- Pointer of next element in dynamic array decoding fixed

[6od9i]

Dublicate check to subType.isStatic removed in decoding array with dynamic size and dynamic subtype value Increment pointer of next element in array added, without this incrementing in array decoded only first element with thero address, second element with type.memoryUsage address and all next elements is decoded at type.memoryUsage - address and equal second element

issue - https://github.com/skywinder/web3swift/issues/565

[JeneaVranceanu]

This needs to be thoroughly reviewed as I did exactly the same and remember that this solution broke the decoding of tuples or something around. I'll join the review later this evening and will write what exactly was the issue.

[mloit]

Look at #440 which had a similar fix and resultant problem, and ultimately #498 for the final fix. A similar solution may be applicable here.

[JeneaVranceanu]

@mloit, thanks for the references! @6od9i, thanks for your contribution! I've found locally the tests I wrote while a month ago working on fixing this decoding issue. There are two simple tests that I've executed against the latest development and received the following:

1. decoding array with dynamic bytes entries (fails in development); Array<Data>
2. decoding array of tuples with dynamic bytes entries (succeeds). Array<(Data, Data)>

After checking the history I was able to find info that someone has added what you've removed exactly because the decoding of an array with tuples of dynamic bytes was failing.

No matter how rare this case of nesting arrays and tuples but it should work absolutely fine no matter the level of nesting. I'll devote some time to check this issue once again and will share the results with you soon.

[6od9i]

Hi I think i have answer to question why - decoding array of tuples with dynamic bytes entries (succeeds). Because it is - only two elements with zero-address and type.size-address. If it will be more than two elements in tuple, i think, it will be fail

Try to test this case please

[6od9i]

And "if (subType.isStatic) " is not use, because in this block this value is checked in early "if subType.isStatic " in line 108, and if i understand right, subtype not changed, then we need to delete if, but leave another line with commentary,

May be use and test second solution of the problem -

2 - when dynamicBytes type decoded in "decodeSignleType" method we can return in tuple - "nextElementPointer" value, to save it in "subpointer" and incement at next step of decoding element cycle in "followTheData" - method

[mloit]

So yes the check is redundant as subype indeed does not change. However you appear to be reducing to the wrong result. Here we need to go by subpointer = consumedUnwrapped since consumedUnwrapped is not actually the count of unwrapped bytes, but a pointer to the next value. (non-static type). To be clear, as it is currently written, if (subType.isStatic) on line 138 will always evaluate to false, thus subpointer = consumedUnwrapped will always be used.

The whole ABI code needs to be refactored I think. The fact that we're returning anonymous tuples, and having inconsistent use of the return parameters makes this very hard to maintain, and is the likely source of this bug at a deeper level.