HarryR / panautomata

Cross-chain proofs and atomic transactions
GNU General Public License v3.0
18 stars 2 forks source link

Migrate to ABIEncoderV2, Solidity 0.5.0 #3

Closed HarryR closed 6 years ago

HarryR commented 6 years ago

As many complex structs will need to be passed around to make life easier the ABIEncoderV2 flag should be used.

I would rather get a working example of things, then port to ABIEncoderV2 and Solidity 0.5.0 in one go.

The eth-abi module supports encoding tuples as per the ABIEncoderV2 specification, so it shouldn't be a problem.

However the EthJsonRpc class will need to support converting the ABI specification to ABIEncoderV2.

See: https://eth-abi.readthedocs.io/en/latest/encoding.html

Are signatures different for ABI v2 encoding?

HarryR commented 6 years ago

Have migrated the ExampleSwap contract to ABIEncoderV2, seems to have fixed a lot of the stack related problems. However, need to start writing the tests for EthJsonRpc and the proxy interface to make sure the contract can be called when passed an ABI that uses tuples as parameters.

HarryR commented 6 years ago

This is mostly done, see:

Required using the eth-abi package, and fixing the rlp version in requirements.txt

However, there is some uncleanly/hacky Python at: https://github.com/HarryR/panautomata/blob/master/python/panautomata/ethrpc.py#L222

This requires an extra parameter to be passed with an array of the types, however it can't be an array of arrays - each item of an array can be a type, or a tuple type (or even a tuple of tuples) etc.

Need to think of a cleaner way to do this

HarryR commented 6 years ago

There is a difference with ABI Encoder V2 that I'm currently debugging.

The transaction input seems to have the full 32 bytes of the signature hash, whereas previously it was truncated to 4 bytes. The code in Panautoma.sol uses a 4 byte signature.

From tx.input (from receipt / details)

0x79a821d96689592d1d26831d0189883d04880b7bb55ba56d7986f4b78bbddff3f16994860000000000000000000000000290fb167208af455bb137780163b7b7a9a10c160000000000000000000000000000000000000000000000000000000000000001000000000000000000000000e982e462b094850f12af94d21d470e21be9d0e9c0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c16000000000000000000000000000000000000000000000000000000000000000100000000000000000000000059d3631c86bbe35ef041872d502f218a39fba1500000000000000000000000000000000000000000000000000000000000000001

From l_input in Panautoma.sol via abi.encodePacked(in_selector, in_args):

0x79a821d90000000000000000000000000290fb167208af455bb137780163b7b7a9a10c160000000000000000000000000000000000000000000000000000000000000001000000000000000000000000e982e462b094850f12af94d21d470e21be9d0e9c0000000000000000000000000290fb167208af455bb137780163b7b7a9a10c16000000000000000000000000000000000000000000000000000000000000000100000000000000000000000059d3631c86bbe35ef041872d502f218a39fba1500000000000000000000000000000000000000000000000000000000000000001

It's the same, apart from the first 32 bytes.

0x79a821d96689592d1d26831d0189883d04880b7bb55ba56d7986f4b78bbddff3f1699486 vs 0x79a821d9

The contract seems to work fine, with the correct parameters in the right place, so I'm thinking maybe they just expanded the signature to 32 bytes? But I can't find an official reference for this.

The code in ethrpc.py is:

The problem is - how do we handle compatibility between both ABI v1 and ABI v2 if there's a fundamental difference like that which will mess up all the arguments to V1 calls if V2 is used and visa versa...

But... how on earth is it working now?

HarryR commented 6 years ago

I'm pretty sure the selector thing is irrelevant, and that I'm just being mistaken, I'm sure that the extra bytes must be the in_guid which isn't being added properly.

Yup it was just me being stupid, the selector was correct, and I thought that a ticket from ethereum/solidity about changing the selector from 4 to 32 bytes had gone through in ABI encoder v2, when infact it was just the in_guid parameter wasn't being encoded in the ReceiveStart method :man_facepalming:

HarryR commented 6 years ago

I'm happy that ABIEncoderV2 support and Solidity 0.5.0 works fine.