Replaces ethereumjs-abi with ethers.js/abi-coder

[theethernaut]

Fix #421
(3rd attempt at fixing it the right way 👍 )

The bug:

Lib's encodeCall helper was failing for any hexadecimal strings that contained the character 'e'. It was interpreting these as scientific notation numbers (e.g. 1.5e9) and seeing their hexadecimal value parsed a decimal instead =S.

As a result, addresses like "0x39af68cF04Abb0eF8f9d8191E1bD9c041E80e18e" were being parsed as some weird exponential number and ending up as "0x0000000000000000000000000000000000000977" in EVM storage.

Beyond the bug:

This PR doesn't only fix the bug, but replaces the dependency used for encoding in favor of ethers/abi-coder, which performs a lot of type/value pair validations internally. Hopefully, this would avoid many errors like #421 in the future. It also adds a few tests for the encodeCall helper.

@spalladino please note a few things:

1. ethers.js/abi-coder is typed 🎉
2. No distinction was made for encodeCall regarding programmatic and cli usage. This is because, given that erhers is what web3 is using, both could be treated equally; unless we want our cli usage to be more permissive of course (or even safer). In that case we could add a flag to the helper function so that it parses some of the incoming values in order to be more flexible in terms of what encodeCall accepts from the command line, and/or perform additional validations.
3. Despite ethers/abi-coder being very robust in terms of the validations it performs on it's incoming values during encoding, it does not support some types listed in the Solidity documentation, such as ufixed, fixed, and functions. I've opened an issue in ethers.js, just to be sure: abi-coder support for fixed, ufixed and function types? ethers-io/ethers.js#389

[spalladino]

@ajsantander looks much cleaner! I want to double-check on this, though:

No distinction was made for encodeCall regarding programmatic and cli usage. This is because, given that erhers is what web3 is using, both could be treated equally; unless we want our cli usage to be more permissive of course (or even safer). In that case we could add a flag to the helper function so that it parses some of the incoming values in order to be more flexible in terms of what encodeCall accepts from the command line, and/or perform additional validations.

Do we have any tests that check that, for every argument type, we can successfully supply them via the command line? In other words, if the create command (or script) is invoked with initArgs ["42", "0x12345","false"] (note that these are all strings, since they were received via the command line) for types [uint256,address,bool], is encodeCall going to work properly?

I'm worried that, if ethers.js enforces too hard validations on the input, we are now preventing our users from initializing certain arguments via the command line.

[spalladino]

Can we add a TODO here to switch to something native of web3 once we migrate to 1.0, such as https://github.com/ethereum/web3.js/blob/1.0/packages/web3-eth-abi/src/index.js#L52?

[spalladino]

Moreover, when we migrate to web3 1.0, can we directly use the web3-eth-abi, and remove the explicit dependency on ethers.js?

[theethernaut]

Sure! TODO note added.
Web3 1.x is actually using exactly the same dependencies we are: https://github.com/ethereum/web3.js/blob/1.0/packages/web3-eth-abi/src/index.js

[theethernaut]

@spalladino, regarding:

Do we have any tests that check that, for every argument type, we can successfully supply them via the command line? In other words, if the create command (or script) is invoked with initArgs ["42", "0x12345","false"] (note that these are all strings, since they were received via the command line) for types [uint256,address,bool], is encodeCall going to work properly?

Most existing tests have an entry that takes in values in string format, except a couple. I'll add to these, and there shouldn't be a problem. The only case in which I see an issue is with booleans, since passing "false" (the string) will result in the abi-coder encoding something like 0x9476a7100000000000000000000000000000000000000000000000000000000000000001. Perhaps for these cases we could re-introduce a simple parseParameters function that, for now, only pre-processes booleans?

[spalladino]

The only case in which I see an issue is with booleans, since passing "false" (the string) will result in the abi-coder encoding something like 0x9476a7100000000000000000000000000000000000000000000000000000000000000001. Perhaps for these cases we could re-introduce a simple parseParameters function that, for now, only pre-processes booleans?

+1, let's try to do that. Also, please make sure that there is at least a test to double check that numbers are handled correctly if passed in as strings.

[theethernaut]

@spalladino, please note:

1. As discussed with @facuspagnuolo, encodeCall will not be parsing any values (like booleans). Pls see: Ability to pass false booleans via the cli #558
2. encodeCall's tests now not only assert that the abi-coder doesn't throw in successful cases, but actually encodes and decodes the data to assert that they match.

[facuspagnuolo]

Looks good! Left some minor comments, please fix them and then merge 👏