Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic array arguments are serialized incorrectly #378

Closed
niran opened this issue Jan 21, 2016 · 4 comments
Closed

Dynamic array arguments are serialized incorrectly #378

niran opened this issue Jan 21, 2016 · 4 comments

Comments

@niran
Copy link
Contributor

niran commented Jan 21, 2016

Consider this test:

test({ type: 'bool[]', value: [true, true, false],
       expected: '0000000000000000000000000000000000000000000000000000000000000020' + 
                 '0000000000000000000000000000000000000000000000000000000000000003' + 
                 '0000000000000000000000000000000000000000000000000000000000000001' + 
                 '0000000000000000000000000000000000000000000000000000000000000001' + 
                 '0000000000000000000000000000000000000000000000000000000000000000'});

It looks like the algorithm for nested dynamic arrays got confused with the one for normal dynamic arrays. A normal dynamic array (bool[]) only needs the length of the array to know where the array ends. A nested dynamic array (bool[][3], an array of three dynamic arrays of booleans) needs to know the offsets of the three dynamic arrays so the arrays can be indexed without iterating through the dynamic arrays, so the offset of each dynamic array is packed together at the front of the encoded bytes.

To access the second of the three dynamic arrays, you look up the second offset (the second uint256 of the encoded value) and use that as the offset from the beginning. There, you'll find the length of the second dynamic array followed by the values, just like a non-nested dynamic array.

In the data from the test, the first 32 bytes are an offset of 32 bytes to get to the array data itself. It would be redundant, so the ABI doesn't work that way.

This is likely the cause of #301.

axic/ethereumjs-abi also has this bug.

@chriseth
Copy link
Contributor

I would say the encoding is correct. The specification says:

 Definition: The following types are called "dynamic":

      bytes
      string
      T[] for any T
      T[k] for any dynamic T and any k > 0

So bool[] is a dynamic type and thus must be encoded with offset-prefix and not in place. If you change the type to bool[3], the bools are encoded in place.

@niran
Copy link
Contributor Author

niran commented Jan 21, 2016

Yes, bool[] is dynamic. However, offsets are only serialized for T[] or T[k] when T is dynamic. Offsets are redundant when T is static, like bool[].

web3.js makes my contract crash. pyethereum.tester does not.

@niran
Copy link
Contributor Author

niran commented Jan 23, 2016

Ok, perhaps the offset is correct, because I have another contract function working fine with it. The documentation seems pretty clear that my previous comment should be the behavior, but the documentation could just be wrong.

The offsets aren't the only problem with my actual transaction data. My function's signature is function createEvent(bytes32 description_hash, bool is_ranged, int lower_bound, int upper_bound, uint8 outcome_count, address resolver_address, int[] data, uint[] oracle_fees, address currency_address, bytes32 currency_hash). Here's the corresponding transaction data:

function hash
53b3c1b1
bytes32 description_hash
781338c019537798dc1006ea56c645a9280b197cda7b0f3712d5414ebdf238bd
bool is_ranged
0000000000000000000000000000000000000000000000000000000000000000
int lower_bound
0000000000000000000000000000000000000000000000000000000000000000
int upper_bound
0000000000000000000000000000000000000000000000000000000000000000
uint8 outcome_count
0000000000000000000000000000000000000000000000000000000000000002
address resolver_address
0000000000000000000000001b059eda58ae71a1f361df93f49ce9212f810dc8
??? broken offset for int[] data
0000000000000000000000000000000000000000000000000000000000000140
??? broken offset for uint[] oracle_fees
0000000000000000000000000000000000000000000000000000000000000180
??? misplaced address currency_address
0000000000000000000000000000000000000000000000000000000000000000
??? misplaced bytes32 currency_hash
0000000000000000000000000000000000000000000000000000000000000000
dynamic array length
0000000000000000000000000000000000000000000000000000000000000001
int[] data
000000000000000000000000e0931803cd14986c8017aad70618bd5f0b58965a
dynamic array length
0000000000000000000000000000000000000000000000000000000000000004
uint[] oracle_fees
00000000000000000000000000000000000000000000000006f05b59d3b20000
000000000000000000000000000000000000000000000000000000000000001b
081741420b52b66759a5c32cebfda800dad1b112b573ac453af0232d80ade631
88f5366ed2eaaf813c001d9b31a095e0baad99c52e0256c1f01b7663a4846f65

It looks like any arguments that follow dynamic arrays in a function are put in the wrong place.

@niran
Copy link
Contributor Author

niran commented Jan 24, 2016

I was wrong! Sorry, I didn't know that the head and tail parts of the array encoding are handled differently when encoding arguments versus encoding them individually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants