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

Decoding arbitrary length bytes fails due to expected padding #198

Closed
RohitK89 opened this issue Dec 8, 2022 · 2 comments · Fixed by #211
Closed

Decoding arbitrary length bytes fails due to expected padding #198

RohitK89 opened this issue Dec 8, 2022 · 2 comments · Fixed by #211

Comments

@RohitK89
Copy link

RohitK89 commented Dec 8, 2022

If this is a bug report, please fill in the following sections.
If this is a feature request, delete and describe what you would like with examples.

What was wrong?

The code is unable to decode several valid transaction calls because of a mismatch between length of the input stream being parsed and the expected padded_length in the ByteStringDecoder class.

Here's the trace I'm using as an example on Etherscan. It is the one listed as Action[2]. There are multiple other examples that I can provide if needed.

Code that produced the error

tx_input = '0x022c0d9f00000000000000000000000000000000000000000000003050f28c10d7d9b40c0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000b5c7ad3cb6506c65da01f2fac2e667dcb9e66e9c000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000c904853d955acef822db058eb8505911ed77f175b99e1531c1a63a169ac75a2daae399080745fa51de4400000000000000000000000000000000000000000000003050f28c10d7d9b40c7bc2c873190bbaddefe646c35f1ae6cffbfb402059bd6774c22486d9f4fab2d448dce4f892a9ae250d0ab87046fbb341d058f17cbc4c1133f25a20a52f000db63cac384247597756545b500253ff8e607a8020010b00020f0100000e030c01990966d504030200000569b81152c5a8d35a67b32a4d3772795d96cae4da010106'
eth_abi.decode(['uint256', 'uint256', 'address', 'bytes'], bytes.fromhex(tx_input[10:]))

Full error output

---------------------------------------------------------------------------
InsufficientDataBytes                     Traceback (most recent call last)
Cell In[99], line 1
----> 1 eth_abi.decode(['uint256', 'uint256', 'address', 'bytes'], bytes.fromhex(tx_input[10:]))

File ~/miniconda3/envs/abi/lib/python3.8/site-packages/eth_abi/codec.py:210, in ABIDecoder.decode(self, types, data)
    207 decoder = TupleDecoder(decoders=decoders)
    208 stream = self.stream_class(data)
--> 210 return decoder(stream)

File ~/miniconda3/envs/abi/lib/python3.8/site-packages/eth_abi/decoding.py:127, in BaseDecoder.__call__(self, stream)
    126 def __call__(self, stream: ContextFramesBytesIO) -> Any:
--> 127     return self.decode(stream)

File ~/miniconda3/envs/abi/lib/python3.8/site-packages/eth_utils/functional.py:45, in apply_to_return_value.<locals>.outer.<locals>.inner(*args, **kwargs)
     43 @functools.wraps(fn)
     44 def inner(*args, **kwargs) -> T:  # type: ignore
---> 45     return callback(fn(*args, **kwargs))

File ~/miniconda3/envs/abi/lib/python3.8/site-packages/eth_abi/decoding.py:173, in TupleDecoder.decode(self, stream)
    170 @to_tuple
    171 def decode(self, stream):
    172     for decoder in self.decoders:
--> 173         yield decoder(stream)

File ~/miniconda3/envs/abi/lib/python3.8/site-packages/eth_abi/decoding.py:127, in BaseDecoder.__call__(self, stream)
    126 def __call__(self, stream: ContextFramesBytesIO) -> Any:
--> 127     return self.decode(stream)

File ~/miniconda3/envs/abi/lib/python3.8/site-packages/eth_abi/decoding.py:145, in HeadTailDecoder.decode(self, stream)
    142 start_pos = decode_uint_256(stream)
    144 stream.push_frame(start_pos)
--> 145 value = self.tail_decoder(stream)
    146 stream.pop_frame()
    148 return value

File ~/miniconda3/envs/abi/lib/python3.8/site-packages/eth_abi/decoding.py:127, in BaseDecoder.__call__(self, stream)
    126 def __call__(self, stream: ContextFramesBytesIO) -> Any:
--> 127     return self.decode(stream)

File ~/miniconda3/envs/abi/lib/python3.8/site-packages/eth_abi/decoding.py:198, in SingleDecoder.decode(self, stream)
    197 def decode(self, stream):
--> 198     raw_data = self.read_data_from_stream(stream)
    199     data, padding_bytes = self.split_data_and_padding(raw_data)
    200     value = self.decoder_fn(data)

File ~/miniconda3/envs/abi/lib/python3.8/site-packages/eth_abi/decoding.py:519, in ByteStringDecoder.read_data_from_stream(stream)
    516 data = stream.read(padded_length)
    518 if len(data) < padded_length:
--> 519     raise InsufficientDataBytes(
    520         "Tried to read {0} bytes.  Only got {1} bytes".format(
    521             padded_length,
    522             len(data),
    523         )
    524     )
    526 padding_bytes = data[data_length:]
    528 if padding_bytes != b'\x00' * (padded_length - data_length):

InsufficientDataBytes: Tried to read 224 bytes.  Only got 201 bytes

Expected Result

I've tested decoding this data using ethersjs as well as on Etherscan and both pass without issue. Here's the decoded data from ethersjs.

[{"_hex":"0x3050f28c10d7d9b40c"},{"_hex":"0x00"},"0xB5c7Ad3cB6506C65DA01F2fAC2e667DcB9e66e9c","0x04853d955acef822db058eb8505911ed77f175b99e1531c1a63a169ac75a2daae399080745fa51de4400000000000000000000000000000000000000000000003050f28c10d7d9b40c7bc2c873190bbaddefe646c35f1ae6cffbfb402059bd6774c22486d9f4fab2d448dce4f892a9ae250d0ab87046fbb341d058f17cbc4c1133f25a20a52f000db63cac384247597756545b500253ff8e607a8020010b00020f0100000e030c01990966d504030200000569b81152c5a8d35a67b32a4d3772795d96cae4da010106"]

This is the function signature, btw:
"swap(uint256,uint256,address,bytes)"

Environment

# run this:
$ python -m eth_utils

Python version:
3.8.15 | packaged by conda-forge | (default, Nov 22 2022, 08:49:06)
[Clang 14.0.6 ]

Operating System: macOS-12.6-arm64-arm-64bit

pip freeze result:
aiohttp==3.8.3
aiosignal==1.3.1
anyio @ file:///home/conda/feedstock_root/build_artifacts/anyio_1666191106763/work/dist
appnope @ file:///home/conda/feedstock_root/build_artifacts/appnope_1649077682618/work
argon2-cffi @ file:///home/conda/feedstock_root/build_artifacts/argon2-cffi_1640817743617/work
argon2-cffi-bindings @ file:///Users/runner/miniforge3/conda-bld/argon2-cffi-bindings_1666850912616/work
asttokens @ file:///home/conda/feedstock_root/build_artifacts/asttokens_1670263926556/work
async-timeout==4.0.2
attrs @ file:///home/conda/feedstock_root/build_artifacts/attrs_1659291887007/work
Babel @ file:///home/conda/feedstock_root/build_artifacts/babel_1667688356751/work
backcall @ file:///home/conda/feedstock_root/build_artifacts/backcall_1592338393461/work
backports.functools-lru-cache @ file:///home/conda/feedstock_root/build_artifacts/backports.functools_lru_cache_1618230623929/work
base58==2.1.1
beautifulsoup4 @ file:///home/conda/feedstock_root/build_artifacts/beautifulsoup4_1649463573192/work
bitarray==2.6.0
black @ file:///Users/runner/miniforge3/conda-bld/black-recipe_1666900158227/work
bleach @ file:///home/conda/feedstock_root/build_artifacts/bleach_1656355450470/work
brotlipy @ file:///Users/runner/miniforge3/conda-bld/brotlipy_1666764727380/work
certifi==2022.9.24
cffi @ file:///Users/runner/miniforge3/conda-bld/cffi_1666754795865/work
charset-normalizer @ file:///home/conda/feedstock_root/build_artifacts/charset-normalizer_1661170624537/work
click @ file:///home/conda/feedstock_root/build_artifacts/click_1666798198223/work
colorama @ file:///home/conda/feedstock_root/build_artifacts/colorama_1666700638685/work
cryptography @ file:///Users/runner/miniforge3/conda-bld/cryptography_1669593229961/work
cytoolz==0.12.0
debugpy @ file:///Users/runner/miniforge3/conda-bld/debugpy_1669709939034/work
decorator @ file:///home/conda/feedstock_root/build_artifacts/decorator_1641555617451/work
defusedxml @ file:///home/conda/feedstock_root/build_artifacts/defusedxml_1615232257335/work
entrypoints @ file:///home/conda/feedstock_root/build_artifacts/entrypoints_1643888246732/work
eth-abi==2.2.0
eth-account==0.5.9
eth-hash==0.5.1
eth-keyfile==0.5.1
eth-keys==0.3.4
eth-rlp==0.2.1
eth-typing==2.3.0
eth-utils==1.9.5
evm-decoder @ file:///Users/rohit/code/trm-blockchain-etl/evm_decoder
exceptiongroup @ file:///home/conda/feedstock_root/build_artifacts/exceptiongroup_1668523704481/work
executing @ file:///home/conda/feedstock_root/build_artifacts/executing_1667317341051/work
fastjsonschema @ file:///home/conda/feedstock_root/build_artifacts/python-fastjsonschema_1663619548554/work/dist
flit_core @ file:///home/conda/feedstock_root/build_artifacts/flit-core_1667734568827/work/source/flit_core
frozenlist==1.3.3
hexbytes==0.3.0
idna @ file:///home/conda/feedstock_root/build_artifacts/idna_1663625384323/work
importlib-metadata @ file:///home/conda/feedstock_root/build_artifacts/importlib-metadata_1669312071043/work
importlib-resources @ file:///home/conda/feedstock_root/build_artifacts/importlib_resources_1670346715028/work
iniconfig @ file:///home/conda/feedstock_root/build_artifacts/iniconfig_1603384189793/work
ipfshttpclient==0.8.0a2
ipykernel @ file:///Users/runner/miniforge3/conda-bld/ipykernel_1668027175059/work
ipython @ file:///Users/runner/miniforge3/conda-bld/ipython_1669904367927/work
ipython-genutils==0.2.0
jedi @ file:///home/conda/feedstock_root/build_artifacts/jedi_1669134318875/work
Jinja2 @ file:///home/conda/feedstock_root/build_artifacts/jinja2_1654302431367/work
json5 @ file:///home/conda/feedstock_root/build_artifacts/json5_1600692310011/work
jsonschema @ file:///home/conda/feedstock_root/build_artifacts/jsonschema-meta_1669810440410/work
jupyter-server @ file:///home/conda/feedstock_root/build_artifacts/jupyter_server_1669064535452/work
jupyter_client @ file:///home/conda/feedstock_root/build_artifacts/jupyter_client_1670253809910/work
jupyter_core @ file:///Users/runner/miniforge3/conda-bld/jupyter_core_1669776266759/work
jupyterlab @ file:///home/conda/feedstock_root/build_artifacts/jupyterlab_1670246025605/work
jupyterlab-pygments @ file:///home/conda/feedstock_root/build_artifacts/jupyterlab_pygments_1649936611996/work
jupyterlab_server @ file:///home/conda/feedstock_root/build_artifacts/jupyterlab_server_1668171221202/work
lru-dict==1.1.8
MarkupSafe @ file:///Users/runner/miniforge3/conda-bld/markupsafe_1666770321497/work
matplotlib-inline @ file:///home/conda/feedstock_root/build_artifacts/matplotlib-inline_1660814786464/work
mistune @ file:///home/conda/feedstock_root/build_artifacts/mistune_1657892024508/work
multiaddr==0.0.9
multidict==6.0.3
mypy-extensions @ file:///Users/runner/miniforge3/conda-bld/mypy_extensions_1666795208530/work
nbclassic @ file:///home/conda/feedstock_root/build_artifacts/nbclassic_1667492839781/work
nbclient @ file:///home/conda/feedstock_root/build_artifacts/nbclient_1669795076334/work
nbconvert @ file:///home/conda/feedstock_root/build_artifacts/nbconvert-meta_1670253564810/work
nbformat @ file:///home/conda/feedstock_root/build_artifacts/nbformat_1665426034066/work
nest-asyncio @ file:///home/conda/feedstock_root/build_artifacts/nest-asyncio_1664684991461/work
netaddr==0.8.0
notebook @ file:///home/conda/feedstock_root/build_artifacts/notebook_1667565639349/work
notebook_shim @ file:///home/conda/feedstock_root/build_artifacts/notebook-shim_1667478401171/work
packaging @ file:///home/conda/feedstock_root/build_artifacts/packaging_1637239678211/work
pandocfilters @ file:///home/conda/feedstock_root/build_artifacts/pandocfilters_1631603243851/work
parsimonious==0.8.1
parso @ file:///home/conda/feedstock_root/build_artifacts/parso_1638334955874/work
pathspec @ file:///home/conda/feedstock_root/build_artifacts/pathspec_1668325009666/work
pexpect @ file:///home/conda/feedstock_root/build_artifacts/pexpect_1667297516076/work
pickleshare @ file:///home/conda/feedstock_root/build_artifacts/pickleshare_1602536217715/work
pkgutil_resolve_name @ file:///home/conda/feedstock_root/build_artifacts/pkgutil-resolve-name_1633981968097/work
platformdirs @ file:///home/conda/feedstock_root/build_artifacts/platformdirs_1657729053205/work
pluggy @ file:///home/conda/feedstock_root/build_artifacts/pluggy_1667232663820/work
prometheus-client @ file:///home/conda/feedstock_root/build_artifacts/prometheus_client_1665692535292/work
prompt-toolkit @ file:///home/conda/feedstock_root/build_artifacts/prompt-toolkit_1669057097528/work
protobuf==3.19.5
psutil @ file:///Users/runner/miniforge3/conda-bld/psutil_1667885969940/work
ptyprocess @ file:///home/conda/feedstock_root/build_artifacts/ptyprocess_1609419310487/work/dist/ptyprocess-0.7.0-py2.py3-none-any.whl
pure-eval @ file:///home/conda/feedstock_root/build_artifacts/pure_eval_1642875951954/work
pycparser @ file:///home/conda/feedstock_root/build_artifacts/pycparser_1636257122734/work
pycryptodome==3.16.0
Pygments @ file:///home/conda/feedstock_root/build_artifacts/pygments_1660666458521/work
pyOpenSSL @ file:///home/conda/feedstock_root/build_artifacts/pyopenssl_1665350324128/work
pyparsing @ file:///home/conda/feedstock_root/build_artifacts/pyparsing_1652235407899/work
pyrsistent @ file:///Users/runner/miniforge3/conda-bld/pyrsistent_1667498802628/work
PySocks @ file:///home/conda/feedstock_root/build_artifacts/pysocks_1661604839144/work
pytest==7.2.0
python-dateutil @ file:///home/conda/feedstock_root/build_artifacts/python-dateutil_1626286286081/work
pytz @ file:///home/conda/feedstock_root/build_artifacts/pytz_1667391478166/work
pyzmq @ file:///Users/runner/miniforge3/conda-bld/pyzmq_1666828576791/work
requests @ file:///home/conda/feedstock_root/build_artifacts/requests_1661872987712/work
rlp==2.0.1
Send2Trash @ file:///home/conda/feedstock_root/build_artifacts/send2trash_1628511208346/work
six @ file:///home/conda/feedstock_root/build_artifacts/six_1620240208055/work
sniffio @ file:///home/conda/feedstock_root/build_artifacts/sniffio_1662051266223/work
soupsieve @ file:///home/conda/feedstock_root/build_artifacts/soupsieve_1658207591808/work
stack-data @ file:///home/conda/feedstock_root/build_artifacts/stack_data_1669632077133/work
terminado @ file:///Users/runner/miniforge3/conda-bld/terminado_1670254106711/work
tinycss2 @ file:///home/conda/feedstock_root/build_artifacts/tinycss2_1666100256010/work
tomli @ file:///home/conda/feedstock_root/build_artifacts/tomli_1644342247877/work
toolz==0.12.0
tornado @ file:///Users/runner/miniforge3/conda-bld/tornado_1666788903714/work
traitlets @ file:///home/conda/feedstock_root/build_artifacts/traitlets_1669796852779/work
typing_extensions @ file:///home/conda/feedstock_root/build_artifacts/typing_extensions_1665144421445/work
urllib3 @ file:///home/conda/feedstock_root/build_artifacts/urllib3_1669259737463/work
varint==1.0.2
wcwidth @ file:///home/conda/feedstock_root/build_artifacts/wcwidth_1600965781394/work
web3==5.31.3
webencodings==0.5.1
websocket-client @ file:///home/conda/feedstock_root/build_artifacts/websocket-client_1667568040382/work
websockets==9.1
yarl==1.8.2
zipp @ file:///home/conda/feedstock_root/build_artifacts/zipp_1669453021653/work

How can it be fixed?

The root of the issue lies here, with the call to ceil32. I'm not sure why we're padding the expected input length. In this example, I stepped through the code and also compared it line by line to how ethersjs is handling this in JS. The eth_abi code successfully parses the last input to determine its length as 201 bytes (which is exactly how many bytes remain for reading in the stream), and is then able to decode and store the result. However, the padded length after the call to ceil32 is 224 bytes, which of course exceeds the remaining input, causing the exception to be raised. Perhaps I'm missing something here, but it seems like this is a bug, especially when comparing against how web3 and ethersjs are handling this on the JS side of the ecosystem.

@fselmo
Copy link
Contributor

fselmo commented Aug 18, 2023

@RohitK89, @jad-elmourad

I think I understand what's going on. And this shouldn't be a bug. What the original writers of this library seem to have wanted to keep default is the abi "strict mode". In the Solidity ABI's own examples it is shown that abi.encode() will pad to 32 bytes every time. As an example, the very first example in this section. Notice how everything is neatly formatted to 32-byte lengths.

Decoding, however, is a different thing. Solidity doesn't enforce this "strict mode" by default but the libraries that are built on the python ecosystem do seem to want to keep strict mode as the default so as to keep things a bit more by-the-book. Here's the relevant section.

The Solidity ABI decoder currently does not enforce strict mode, but the encoder always creates data in strict mode."

With the above in mind, I'm hesitant to call this a "bug". But I do think the library should be able to behave as Solidity currently does. I tinkered with this a bit and would need to add more tests but I'm a decent way into being able to provide a flag to decode() that allows you to set strict=False and correctly return the same values Solidity does. At least in the few examples I've tried. I am going to add more robust testing around this but does that sound like a nice compromise?

Thanks for starting the discussion on this 👍🏼


edit: @jad-elmourad, if you are open to it, I could push my commits to your PR'd branch and we can collaborate there. Thoughts?

@jad-elmourad
Copy link
Contributor

@fselmo I think adding the optional flag is a great compromise. Please feel free to push your commits there.

fselmo added a commit to jad-elmourad/eth-abi that referenced this issue Aug 21, 2023
fselmo added a commit to jad-elmourad/eth-abi that referenced this issue Aug 21, 2023
fselmo added a commit to jad-elmourad/eth-abi that referenced this issue Aug 22, 2023
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

Successfully merging a pull request may close this issue.

3 participants