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

Get StringIndexOutOfBoundsException when receive result as a dynamic array with single struct #1858

Open
Gelassen opened this issue Feb 2, 2023 · 6 comments
Labels
needs-review issue/PR needs review from maintainer

Comments

@Gelassen
Copy link

Gelassen commented Feb 2, 2023

Title

I get StringIndexOutOfBoundsException on the call of function getMatches(address userFirst, address userSecond) external view override returns (Match[] memory). Research shows client receives data, but can not parse it properly.

Description

The function getMatches() returns dynamic array with struct items. A test case return a single item array.

The client successfully receives encoded bytecode, but can failed to parse it with error:

Exception(error=java.lang.StringIndexOutOfBoundsException: length=512; index=522084538)

As you might see the index is irrelevant.

The encoded result is:

0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000100000000000000000000000062f8dc8a5c80db6e8fcc042f0cc54a298f8f2ffd000000000000000000000000000000000000000000000000000000000000000000000000000000000000000052e7400ba1b956b11394a5045f8bc3682792e1ac000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001

which match to array code, array size and match struct with params:

0x62F8DC8a5c80db6e8FCc042f0cC54a298F8F2FFd // 1st user
0, // 1st user token id
0x52E7400Ba1B956B11394a5045F8BC3682792E1AC, // 2nd user
1, // 2nd user token id 
0, // approve by 1st user
1  // approve by 2nd user

By debugging I figured out it fails on 128 offset where it tries to represent first item of the struct as a DynamicArray and this is a reason why it returns wrong offset.

Context

struct Match {
    address _userFirst;
    uint256 _valueOfFirstUser; 
    address _userSecond;
    uint256 _valueOfSecondUser; 
    bool _approvedByFirstUser;
    bool _approvedBySecondUser;
}

Relevant TypeDecoder code:

TypeDecoder.decodeDynamicStruct(
                                        input,
                                        offset + getDataOffset(input, currOffset, typeReference), // it will take "00000000000000000000000062f8dc8a5c80db6e8fcc042f0cc54a298f8f2ffd" as substring of input find out next offset
                                        TypeReference.create(cls)
);

Relevant code from Android client:

    override fun getMatches(userFirstAddress: String, userSecondAddress: String): RemoteFunctionCall<List<Match>> {
        val function = Function(
            ISwapChain.Functions.FUNC_GET_MATCHES,
            listOf(
                Address(userFirstAddress),
                Address(userSecondAddress)
            ),
            listOf(
                object: TypeReference<DynamicArray<Match>>() {}
            )
        )
        return RemoteFunctionCall(function) {
            val result = executeCallSingleValueReturn<Type<Match>, List<*>>(function, List::class.java) as List<Type<Match>>
            convertToNative<Type<Match>, Match>(result)
        }

    }

Related PR: #1321
Related issues:
#935
#449

The question is why does it happen? Has been anything changed in dynamic array with struct encoded format over passed 2 years?

@Gelassen Gelassen added the needs-review issue/PR needs review from maintainer label Feb 2, 2023
@Gelassen
Copy link
Author

Gelassen commented Feb 2, 2023

@sweexordious, I know you left the project, but could you please take a look on this? The behaviour is quite odd and I am not sure I got right the idea of bit shift for evaluating next offset.

Why does when it parse the 1st item it use the same typeReference and recognise it again as a DynamicArray instead of Match struct? Is it issue in lib or my malformed request?

@Gelassen
Copy link
Author

Gelassen commented Feb 2, 2023

I have got a similar issue when run this function over etherjs:

Error: call revert exception [ See: https://links.ethers.org/v5-errors-CALL_EXCEPTION ] (method="getMatches(address,address)", data="0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000100000000000000000000000062f8dc8a5c80db6e8fcc042f0cc54a298f8f2ffd000000000000000000000000000000000000000000000000000000000000000000000000000000000000000052e7400ba1b956b11394a5045f8bc3682792e1ac000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001", errorArgs=null, errorName=null, errorSignature=null, reason=null, code=CALL_EXCEPTION, version=abi/5.7.0)
    at Logger.makeError (/home/gelassen/Workspace/Personal/Android/swap/server/node_modules/@ethersproject/logger/lib/index.js:238:21)
    at Logger.throwError (/home/gelassen/Workspace/Personal/Android/swap/server/node_modules/@ethersproject/logger/lib/index.js:247:20)
    at Interface.decodeFunctionResult (/home/gelassen/Workspace/Personal/Android/swap/server/node_modules/@ethersproject/abi/lib/interface.js:388:23)

It is able to execute the request and even return data, but fails on decoding the returned data. Did you and etherjs author worked independently and didn't use the parse algorithm described somewhere else?

@rach-id
Copy link
Contributor

rach-id commented Feb 2, 2023

Is it issue in lib or my malformed request?

Most likely an issue with the library.

I guess that this is one of the limitations of the decoder that Web3J has. Anyway, that portion of code needs to be refactored and corrected with more tests and all. Since I last worked on it, there have been multiple issues with different edge cases where the library fails. A good approach is to gather them all somewhere and test against them and fix.
But generally, the whole logic starting from the V2 decoder up to the arrays/multiple returns decoder need a rework.

I suspect it would be a 1 month work at least to have the decoder working perfectly.

@Gelassen
Copy link
Author

Gelassen commented Feb 2, 2023

Thank you. It is good to localise the issue.

@rach-id
Copy link
Contributor

rach-id commented Feb 2, 2023

You can even be more sure of the issue via adding a test in the abi, but that's a bit of work.

You could create your function similar to this:
https://github.com/web3j/web3j/blob/199914cfffda72f3233abd72d4141fdc62b873d5/abi/src/test/java/org/web3j/abi/AbiV2TestFixture.java#L760-L765

and define your struct like this:
https://github.com/web3j/web3j/blob/199914cfffda72f3233abd72d4141fdc62b873d5/abi/src/test/java/org/web3j/abi/AbiV2TestFixture.java#L678-L694

Probably, you could get it from the java wrappers.

Then add a test similar to this:
https://github.com/web3j/web3j/blob/199914cfffda72f3233abd72d4141fdc62b873d5/abi/src/test/java/org/web3j/abi/FunctionReturnDecoderTest.java#L451-L471

And see if the issue persists.

You have all the ingredients needed to write such thing.

@Gelassen
Copy link
Author

Gelassen commented Feb 3, 2023

@sweexordious , thank you, here is PR to spot this issue.
#1860

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-review issue/PR needs review from maintainer
Projects
None yet
Development

No branches or pull requests

2 participants