Some thoughts regarding `IllegalArgumentException("unconsumed bytes: ...")`

[hawkaa]

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]

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]

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]

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]

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]

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.