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

Some thoughts regarding IllegalArgumentException("unconsumed bytes: ...") #34

Closed
hawkaa opened this issue Feb 15, 2022 · 5 comments
Closed

Comments

@hawkaa
Copy link

hawkaa commented Feb 15, 2022

Hi @esaulpaugh ,

Thank you so much for a great library!

We've reached an edge case in decoding where we hit the exception mentioned in the title, IllegalArgumentException("unconsumed bytes: ..."), when decoding a function's output.

We have the following input and output data data:

Input: 0xc37f68e200000000000000000000000037cab7add0393689fa50e979cc9bd8db7a9905aa
Output: 0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000052f1a067938685bf66f0a400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

ABI:

     {
        "constant": true,
        "inputs": [
            {
                "internalType": "address",
                "name": "account",
                "type": "address"
            }
        ],
        "name": "getAccountSnapshot",
        "outputs": [
            {
                "internalType": "uint256",
                "name": "",
                "type": "uint256"
            },
            {
                "internalType": "uint256",
                "name": "",
                "type": "uint256"
            },
            {
                "internalType": "uint256",
                "name": "",
                "type": "uint256"
            },
            {
                "internalType": "uint256",
                "name": "",
                "type": "uint256"
            }
        ],
        "payable": false,
        "stateMutability": "view",
        "type": "function"
    }

The library throws the following exception with stack trace:

java.lang.IllegalArgumentException: unconsumed bytes: 64 remaining
	at com.esaulpaugh.headlong.abi.ABIType.decode(ABIType.java:190)
	at com.esaulpaugh.headlong.abi.Function.decodeReturn(Function.java:240)

The problem was quite easy to identify. The output data contains 6 x 32 bytes, while the decoder only expects 4 x 32 bytes ((uint256,uint256,uint256,uint256)). This causes the exception.

One could argue that we do have the wrong data in the first place. Which might be the case, but I've triple checked our node provider API and this is what we get.

We do have a golang implementation of decoding which uses geth which doesn't crash on this data. I've also created a quick test script which uses the official eth-abi library. Both implementations consume the data they need and disregard the last 64 bytes.

This begs the question: Does throwing an exception on unconsumed bytes adhere to the standard? Should we consider removing that check?

If not, we would be happy to discuss alternative APIs for us to use which doesn't do this check.

Let me know what you think!

Håkon

@esaulpaugh
Copy link
Owner

if the bytes are given as a ByteBuffer it should not throw. can you try that?

the intention is that the byte[] APIs consume the entire input while the ByteBuffer APIs allow more flexibility at the cost of some additional complexity.

@esaulpaugh
Copy link
Owner

esaulpaugh commented Feb 15, 2022

try

byte[] data = FastHex.decode("00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000052f1a067938685bf66f0a400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000");
TupleType tt = TupleType.parse("(int,int,int,int)");
Tuple t = tt.decode(ByteBuffer.wrap(data));
System.out.println(t);

EDIT: for function output it would actually be foo.decodeReturn(ByteBuffer.wrap(data))

@esaulpaugh
Copy link
Owner

esaulpaugh commented Feb 15, 2022

As of version 6.0.0, the other alternative is specifying the indices to decode with foo.decodeReturn(data, 0, 1, 2, 3) where data is either a byte[] or a ByteBuffer

@hawkaa
Copy link
Author

hawkaa commented Feb 15, 2022

Thank you for a quick response! You are absolutely right. That solved our problems!

I don't have the expertise or have followed the #33 PR that my colleague @devictr worked on, but it seems like this only works for Function for now as decodeCall and decodeReturn are overloaded to handle ByteBuffer. decodeArgs, on the other hands, doesn't seem to have this.

Anyway, feel free to close this issue. Thank you so much for being so responsive and maintaining this amazing library!

@esaulpaugh
Copy link
Owner

esaulpaugh commented Feb 16, 2022

No problem.

For now it seems that any method that accepts a byte[] but not an offset and length should not support (silent) partial decodes as that could lead to surprising and difficult-to-detect behavior. Decode methods accepting a ByteBuffer as an argument should advance the position() the number of bytes read so the number of bytes consumed can be calculated. An exception to this is if specific indices (into the Tuple) are specified. I will consider adding a ByteBuffer version of decodeArgs if there is a significant need.

Let me know if you have any further issues.

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

2 participants