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

Encoding Dynamic Arrays missing location #13

Open
hswick opened this issue Apr 7, 2018 · 10 comments
Open

Encoding Dynamic Arrays missing location #13

hswick opened this issue Apr 7, 2018 · 10 comments

Comments

@hswick
Copy link

hswick commented Apr 7, 2018

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
Copy link
Contributor

hayesgm commented Apr 9, 2018

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
Copy link
Author

hswick commented Apr 10, 2018

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

@hswick
Copy link
Author

hswick commented Apr 13, 2018

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
Copy link
Contributor

hayesgm commented Apr 13, 2018

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
Copy link
Author

hswick commented Apr 22, 2018

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
Copy link

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

@ghbutton
Copy link

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

@ghbutton
Copy link

ghbutton commented Aug 25, 2018

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

@aaroncolaco
Copy link

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
Copy link
Member

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

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

5 participants