eth-abi icon indicating copy to clipboard operation
eth-abi copied to clipboard

Decoding an array type returns a tuple instead of list

Open tserg opened this issue 2 years ago • 5 comments

What was wrong?

eth_abi.decode of an array returns a tuple instead.

Code that produced the error

>>> eth_abi.decode(["uint256[3]"], b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x03')
((1, 2, 3),)

Expected Result

Shouldn't the result be a list instead?

([1, 2, 3],)

Environment

# run this:
$ python -m eth_utils

# then copy the output here:
eth-abi==3.0.1

How can it be fixed?

Fill this section in if you know how this could or should be fixed.

tserg avatar Oct 04 '22 15:10 tserg

Hey @tserg. Thanks for the submission. I do think that distinction would be nice tbh. Spitting out a tuple can confuse it as decoding as a struct. This is a pretty simple change I wouldn't mind making. I can push a PR up if there are no hesitations. Especially since we are in beta for the new major version.

Thoughts on this @kclowes, @pacrob?

fselmo avatar Oct 04 '22 16:10 fselmo

I do think the flexibility of it not needing to be validated as a python list when we are encoding is nice though. If we make the distinction for decoding then I think it might need to be made for encoding as well and I'm not entirely sure where utility vs flexibility lie here and which I prefer.

i.e. abi.encode(["uint256[3]"], [(1,2,3)]) would yell since (1,2,3) is a struct, not a list.

e.g. you'd need abi.encode(["(uint256,uint256,uint256)"], [(1,2,3)]) to encode the struct properly, even though both encode to the same bytes result.

fselmo avatar Oct 04 '22 16:10 fselmo

I do think the flexibility of it not needing to be validated as a python List when we are encoding is nice though. If we make the distinction for decoding then I think it might need to be made for encoding as well and I'm not entirely sure where utility vs flexibility lie here and which I prefer.

i.e. abi.encode(["uint256[3]"], [(1,2,3)]) would yell since (1,2,3) is a struct, not a list.

e.g. you'd need abi.encode(["(uint256,uint256,uint256)"], [(1,2,3)]) to encode the struct properly, even though both encode to the same bytes result.

I think the fact that uint256 [N] has the same bytes representation as the corresponding tuple is a bit of a red herring - tuple should not be encodable as list and vice versa. I can't really think of a case where the flexibility is needed, and the correctness from validation is probably ultimately a good thing to have (in other words, as a user, the mapping between python and ABI types is probably expected to be strict).

charles-cooper avatar Oct 04 '22 17:10 charles-cooper

Almost tagged you here too @charles-cooper 😃. Thanks for popping in. Yeah I have no issues at all with heading this direction. I can put up a PR asap.

fselmo avatar Oct 04 '22 17:10 fselmo

maybe lists should be encode-able as tuples -- function args are represented as a tuple in the ABI, but often client libraries are dealing with a list of arguments and want to do something like encode_single("(a,b,c)", [a,b,c]).

charles-cooper avatar Oct 14 '22 15:10 charles-cooper