exthereum / abi

The Ethereum ABI Interface
MIT License
20 stars 22 forks source link

Encoding Dynamic Arrays missing location #13

Open hswick opened 6 years ago

hswick commented 6 years ago

It seems the encoding for dynamic arrays is missing the placeholder for the location. Currently the output is this:

ABI.encode("dynamicUint(uint[])", [[1,2,3,4,5]]) |> Hexate.encode

=> "5d4e0342000000000000000000000000000000000000000000000000000000000000000500000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000005"

Whereas I believe it should be something like this:

"5d4e03420000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000500000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000005"

Notice the 0000000000000000000000000000000000000000000000000000000000000020 preceding the length prefix. This is denoting that the dynamic array starts after 32 bytes. Since there is only one argument.

hayesgm commented 6 years ago

I believe this relates to confusion around encoding of tuples or raw data parameters. If you instead encode this as a tuple, you'll get what you're looking for.

ABI.encode("dynamicUint((uint[]))", [{[1,2,3,4,5]}]) |> Base.encode16(case: :lower)
"5b8b2e7c0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000500000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000300000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000005"

Thus, I think the bug is likely in function_selector.ex which tells the encoder that function arguments should be encoded one-by-one instead of as a tuple. I believe that's where we should fix this bug.

Does that make sense?

hswick commented 6 years ago

Ah, yes. Thank you! This makes sense, easy enough to work around for now. I'll stay posted for any updates on this.

hswick commented 6 years ago

So after a bit of data wrangling, I was able to get the dynamic array to be encoded correctly. But the transaction results in a revert.

I compared the results to my ruby project and determined the source of the problem. The issue becomes that the method id "5b8b2e7c" is incorrect, it should be "5d4e0342" like I had it originally. This is because "dynamicUint((uint[]))" is not a valid function signature because of the extra case of parentheses surrounding "uint[]".

So I thought easy enough, I'll use the old function signature. But that results in this error:

** (Protocol.UndefinedError) protocol Enumerable not implemented for {[1, 2, 3, 4, 5]}.

Which I am assuming is due to encoding the data based on how the function signature is parsed.

Once again, I could manually work around this if I had to, but luckily I'm not in that situation. And thus, it will be best if this bug is fixed here.

hayesgm commented 6 years ago

Sure, I believe the fix is as simple as changing how functions are encoded, with the knowledge that they should be considered tuples. I'll see if I can take a crack at this shortly.

hswick commented 6 years ago

I did some digging on my own, and I'm noticing some behavior that seems strange to me:

iex(30)> ABI.Parser.parse!("(uint256[])")           
%ABI.FunctionSelector{
  function: nil,
  returns: nil,
  types: [tuple: [array: {:uint, 256}]]
}
iex(31)> ABI.Parser.parse!("dynamicUint(uint256[])")
%ABI.FunctionSelector{
  function: "dynamicUint",
  returns: nil,
  types: [array: {:uint, 256}]
}

I feel like the results should be similar.

ghbutton commented 6 years ago

Running into this issue and well to want to give it a 👍

ghbutton commented 6 years ago

For now will use something like this: ABI.encode("dynamicUint((uint[]))", [{[1,2,3,4,5]}]) |> Base.encode16(case: :lower)

ghbutton commented 6 years ago

Hmm, I dont know if this is encoding the function the way I am expecting it to

aaroncolaco commented 4 years ago

I'm running into this problem as well. Is there any other way to encode dynamic arrays? I'm trying to encode test(bytes32[], uint256[]) in particular

ayrat555 commented 4 years ago

A couple of months ago I re-wrote encoder and decoder of abi n https://github.com/poanetwork/ex_abi . it should be fixed there