Add ethers v5 target #250

zemse · 2020-06-20T22:00:52Z

This PR adds a target-ethers-v5 to TypeChain packages and intends to close #205.

Thanks to @krzkaczor and @quezak for v4 target. The v5 target in this PR builds on top of it to update the types changed in ethers v5.

To try it: by cloning repository:

git clone [email protected]:zemse/TypeChain.git or git clone https://github.com/zemse/TypeChain.git
cd TypeChain
yarn (you need yarn installed if you don't)
yarn build (this generates dist dir which is imp)
Link this custom target to your project by placing the appropriate path to the target: typechain --target ../TypeChain/packages/target-ethers-v5/ (Docs)

Just in case if it's too much, I have temporarily published it to npm for testing:

npm install typechain-target-ethers-v5 --save-dev (later you can just replace the package with official one)
typechain --target ethers-v5 (Docs)

Useage

import { Erc20Factory } from '../../interfaces'; // there is an index.ts file which exports all factories

// Deploying
// you only need to pass a signer (abi and bytecode is managed)
const erc20Factory = new Erc20Factory(signer); 
const erc20Instance = await erc20Factory.deploy(); // you will get type suggestions in VSCode.

// Connecting to Existing
const erc20Instance = Erc20Factory.connect(contractAddress, signer);

// For some reason if you need interface of the contract
import { Erc20 } from '../../interfaces/Erc20';
interface Global {
  tokenInstance: Erc20;
}

Please have a look at this if it is fine and let me know of any change I need to make.

abarmat · 2020-06-22T20:51:54Z

I tried this PR on a repo where I have many tests I migrated from ethers-v4 to ethers-v5 using waffle-v3 and worked for me.

krzkaczor · 2020-06-22T21:01:59Z

.gitignore

@@ -14,3 +14,4 @@ examples/truffle-v4/types
 examples/truffle-v4/migrations
 examples/truffle-v5/types
 examples/truffle-v5/migrations
+.DS_Store


this should go to your global gitignore file. otherwise we will end up with huge list of unrelevant OS files in this file.

That makes sense. Removing this from gitignore file right away.

krzkaczor · 2020-06-22T21:03:35Z

This looks great @zemse! Thanks!

I guess more tests would be nice (in a separate directory as we do for other targets).

abarmat · 2020-06-22T21:47:21Z

packages/target-ethers-v5/src/codegen/index.ts

+export function codegenAbstractContractFactory(contract: Contract, abi: any): string {
+  return `
+  import { Contract, Signer } from "ethers";
+  import { Provider } from "ethers/providers";


@krzkaczor is this ok, or it should be import { Provider } from '@ethersproject/providers'; ?

uhh good call. That's why we need tests + isolated example to catch such problems 😆

Oh my bad.. fixing this for now. @krzkaczor I'll have a look at how other tests are written and add the same for v5 here.

krzkaczor · 2020-06-23T12:49:30Z

@zemse awesome work! Yeah I would like to hear some feedback from the users. Tests would also be great! We can't publish an official release without them.

interface.encodeFunctionData("transfer", [ to, amount ])

I think it's possible to implement it by using function overloads where first arg type would be string literal "transfer". But yes, it's a little bit more complicated.

zemse · 2020-06-23T12:53:14Z

I think it's possible to implement it by using function overloads where first arg type would be string literal "transfer". But yes, it's a little bit more complicated.

Okay, it's possible then I'll give it a try because it'd be nice to have this too.

quezak · 2020-06-23T15:44:42Z

I'll take a look tomorrow!

quezak

I've reviewed a diff between this and the v4 target package, code looks good!
Just a nitpick plus a few questions, since I didn't try ethers v5 myself yet.

quezak · 2020-06-24T10:25:26Z

packages/target-ethers-v5/src/codegen/types.ts

+export interface GenerateFunctionOptions {
+  returnResultObject?: boolean
+  isStaticCall?: boolean
+}
+


I'd move this interface to the functions.ts file, since it's only used there and doesn't have to be exported. (this file's name types.ts refers to handling code generation for solidity types, not TS types for this package ;) )

Very much thanks for the review. This makes sense and I'll make this change.

quezak · 2020-06-24T10:32:54Z

packages/target-ethers-v5/src/codegen/index.ts

+  if (allFunctions.match(/\W Overrides(\W|$)/)) contractImports.push('Overrides')
+  if (allFunctions.match(/\WPayableOverrides(\W|$)/)) contractImports.push('PayableOverrides')
+  if (allFunctions.match(/\WCallOverrides(\W|$)/)) contractImports.push('CallOverrides')


why is the first regex different than the 2nd & 3rd one? (space between \W and the name)

maybe is it worth extracting a helper fn to increase readability and avoid typos in the regexes? like

function pushImportIfUsed(importName: string, generatedCode: string, importArray: string[]): void { if (new RegExp(`\\W${importName}(\\W|$)`).test(generatedCode)) importArray.push(importName); }

I was afraid that /\WOverrides(\W|$)/ would match PayableOverrides and CallOverrides too when it shouldn't and should only match Overrides. Is there a better way to do the same? I think would face the same issue having this as a method as the pushImportIfUsed example

I've noticed that problem when I wrote those regexes in the ethers-v4 target :) \W means a non-word character, and it's there specifically so \Wxyz doesn't match abcxyz, so it's safe to drop the extra space.

Example: https://regex101.com/r/r6Cjn4/1

packages/target-ethers-v5/src/codegen/index.ts

quezak · 2020-06-24T10:40:59Z

packages/target-ethers-v5/src/codegen/index.ts

+    ? `${constructorArgNamesWithoutOverrides}, overrides || {}`
+    : 'overrides || {}'


does the ContractFactory require passing empty overrides in v5, since you added the explicit || {} here?

When we don't pass any override using typechain's overrides, the overrides is set as undefined. But it looks like ethers v4 was fine with undefined overrides value but in v5 it throws an error. We can make it not pass an undefined override (+1 or 2 lines) or just fall-backing to an empty override as done corrently. Is it good doing like this?

This is weird, not passing an optional argument should be handled exactly the same as explicitly passing undefined there. TBH, if passing undefined overrides throws in ethers v5, it's likely a bug there. We can leave the explicit empty overrides for now.

zemse · 2020-06-28T13:07:42Z

I think it's possible to implement it by using function overloads

Thanks for pointers. I've tried to add that.

I've updated the tests for ethers v5. Please have a look and let me know for any changes required.

krzkaczor · 2020-06-30T19:34:26Z

Thanks for all your hard work! It's moved to internal branch fork/ethers-v5-target. For now.

krzkaczor · 2020-06-30T20:12:30Z

It's published as @typechain/ethers-v5, still in beta. Thanks again for your work.

I added an example to my branch and it seems like everything works fine 👍

zemse · 2020-06-30T21:41:18Z

@krzkaczor Thanks for moving it forward! I'm very happy I gave this a shot, this was my first PR in OSS. I have marked the demo published package as deprecated. Let me know if I need to do anything more.

BTW saw the typedAssert while writing tests, It might be common for rest of you guys but for someone like me without a programing background and barely over 1 year exp, honestly I'm was impressed with the cool stuff that I saw in this repo.

One more thing which were exiting for me were the reviews (thanks to everyone for taking time to review the work and guiding me). Sorry if I sound too crazy, but I'm happy that I'm on GitHub, I have access to so many amazing people all around the world. I observed that in preparing PRs on GitHub, we get to learn a lot of best practices since we get to see a lot. So I believe I'll find more opportunities to do.

Hopefully, I'll get to see you all again.

abarmat · 2020-06-30T21:54:08Z

@zemse thank you for your time putting this code together. This PR is super useful for all us using Typechain and Ethers.

zemse · 2020-07-02T20:19:05Z

Hi @HenryNguyen5, I just encountered a problem with staticCall. ethers-io/ethers.js#924

krzkaczor · 2020-07-02T20:26:02Z

@zemse wow great work! Especially that it was your first PR to OSS.

I gotta admit that TypeChain is not the easiest project to contribute to, so you should be proud of yourself even more ;)

zemse added 24 commits June 20, 2020 01:08

Ignore macOS .DS_Store files from git

54cf942

Duplicate ethers v4 directory

92c8d55

Add ethers v5 target to tsconfig reference path

7c58831

Rename contract estimate to estimateGas

3b1916c

Update Arrayish to BytesLike

ea81db9

Update imports from ethers v5

b8b0bdf

Replace interface events and function fragments

e383c4d

Update getDeployTransaction signature

3412c1c

Update Contract self return on helper methods

1eeffb7

Remove index.d.ts static file

22b3dbd

Rename Factory file suffix to Contract

e755f86

Export an existing contract constructor

04c0678

Fix PayableOverrides if payable constructor

8fa1ea7

Fix PayableOverrides if payable contract method

c759bad

Add populateTransaction

865d89c

Fix functions Result obj (ethers-io/ethers.js#869)

abe4215

Add CallOverrides optional param for constant fns

6805bf4

Fix unused overrides import

5eb4631

Add index file that exports all contracts

4548a0a

Run yarn test:fix and fix errors

72553b5

Rename to v5

01954da

Rename Contract file suffix back to Factory

59e38bd

Remove existing contract constructor

d298842

Fix ethers constructor too many arguments error

a34fe8d

zemse mentioned this pull request Jun 20, 2020

Add target for ethers v5 #205

Closed

abarmat mentioned this pull request Jun 22, 2020

build: update to ethers v5 graphprotocol/contracts#224

Merged

krzkaczor reviewed Jun 22, 2020

View reviewed changes

abarmat reviewed Jun 22, 2020

View reviewed changes

quezak self-requested a review June 23, 2020 15:45

quezak reviewed Jun 24, 2020

View reviewed changes

zemse added 11 commits June 28, 2020 14:43

Add encodeFunctionData function overloads

1614cb3

Add decodeFunctionResult function overloads

170806b

Add getEvent function overloads

c25f9bf

Move GenerateFunctionOptions to functions.ts

6c1ed8c

Add a helper method to push optional imports

654e976

Duplicate ethers v4 tests directory

f6ded62

Install ethers v5

79c3fb5

Update types

bc554ad

Update tests for ethers v5

4c57d75

Fix bug of empty parameter type

7c1d122

Update types

0ca6e89

zemse requested review from krzkaczor and quezak June 28, 2020 12:50

zemse mentioned this pull request Jun 28, 2020

Adding event type for ethers #249

Closed

krzkaczor changed the base branch from master to fork/ethers-v5-target June 30, 2020 19:23

krzkaczor merged commit 12141da into dethcrypto:fork/ethers-v5-target Jun 30, 2020

zemse mentioned this pull request Sep 11, 2020

[ethers-v5] Contract types not exported in generated index.ts #278

Closed

Hawthorne001 mentioned this pull request Mar 13, 2024

[Snyk] Upgrade bignumber.js from 9.0.1 to 9.1.2 Hawthorne001/TypeChain#4

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ethers v5 target #250

Add ethers v5 target #250

zemse commented Jun 20, 2020 •

edited

Loading

abarmat commented Jun 22, 2020

krzkaczor Jun 22, 2020

zemse Jun 23, 2020

krzkaczor commented Jun 22, 2020

abarmat Jun 22, 2020

krzkaczor Jun 23, 2020

zemse Jun 23, 2020

krzkaczor commented Jun 23, 2020

zemse commented Jun 23, 2020

quezak commented Jun 23, 2020

quezak left a comment

quezak Jun 24, 2020 •

edited

Loading

zemse Jun 24, 2020

quezak Jun 24, 2020

quezak Jun 24, 2020 •

edited

Loading

zemse Jun 24, 2020

quezak Jun 24, 2020 •

edited

Loading

quezak Jun 24, 2020

quezak Jun 24, 2020 •

edited

Loading

zemse Jun 24, 2020

quezak Jun 24, 2020 •

edited

Loading

zemse commented Jun 28, 2020

krzkaczor commented Jun 30, 2020

krzkaczor commented Jun 30, 2020

zemse commented Jun 30, 2020

abarmat commented Jun 30, 2020

zemse commented Jul 2, 2020

krzkaczor commented Jul 2, 2020

		? `${constructorArgNamesWithoutOverrides}, overrides \|\| {}`
		: 'overrides \|\| {}'

Add ethers v5 target #250

Add ethers v5 target #250

Conversation

zemse commented Jun 20, 2020 • edited Loading

Useage

abarmat commented Jun 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krzkaczor commented Jun 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krzkaczor commented Jun 23, 2020

zemse commented Jun 23, 2020

quezak commented Jun 23, 2020

quezak left a comment

Choose a reason for hiding this comment

quezak Jun 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quezak Jun 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quezak Jun 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quezak Jun 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

quezak Jun 24, 2020 • edited Loading

Choose a reason for hiding this comment

zemse commented Jun 28, 2020

krzkaczor commented Jun 30, 2020

krzkaczor commented Jun 30, 2020

zemse commented Jun 30, 2020

abarmat commented Jun 30, 2020

zemse commented Jul 2, 2020

krzkaczor commented Jul 2, 2020

zemse commented Jun 20, 2020 •

edited

Loading

quezak Jun 24, 2020 •

edited

Loading

quezak Jun 24, 2020 •

edited

Loading

quezak Jun 24, 2020 •

edited

Loading

quezak Jun 24, 2020 •

edited

Loading

quezak Jun 24, 2020 •

edited

Loading