fselmo
commented
1 year ago @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
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
Full error output
Expected Result
I've tested decoding this data using
ethersjsas well as on Etherscan and both pass without issue. Here's the decoded data fromethersjs.This is the function signature, btw:
"swap(uint256,uint256,address,bytes)"Environment
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 howethersjsis handling this in JS. Theeth_abicode successfully parses the last input to determine its length as201 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 toceil32is224 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 howweb3andethersjsare handling this on the JS side of the ecosystem.