fix(abi): StaticArray offset incorrect for DynamicBytes/DynamicArray elements#2282
fix(abi): StaticArray offset incorrect for DynamicBytes/DynamicArray elements#2282snuderl wants to merge 2 commits intoLFDT-web3j:mainfrom
Conversation
When a StaticArray contains dynamic element types (DynamicBytes, DynamicArray), the array is pointer-encoded and the top-level tuple slot only occupies one 32-byte (64 hex char) pointer. However, both the StaticArrayTypeReference branch and the StaticArray branch incorrectly advance the offset by length * 64, which over-advances past the pointer slot and corrupts decode positions for all subsequent parameters. This also fixes the StaticArrayTypeReference branch which was missing element-type inspection entirely — it unconditionally used length * slotSize regardless of element type. Extract a shared staticArrayOffset helper that handles all element type cases (DynamicStruct, DynamicBytes, DynamicArray, Utf8String, StaticStruct, and plain types) consistently for both branches. Co-Authored-By: Blaz Snuderl <blaz.snuderl@gmail.com>
|
@snuderl thanks for working on this fix the direction makes sense. But this PR was generated by a bot so it would be good to double check the changes manually. I went through the offset logic and there is one important edge case that seems to be missed. Right now staticArrayOffset checks only the immediate element type. This works for cases like bytes[2], but it breaks for nested cases like bytes[2][2]. In that case the outer type is still a StaticArray, so the current check treats it as non dynamic and calculates offset as length * slotSize. But since the inner elements are dynamic, the whole structure should behave like a dynamic type and occupy a single pointer slot. This means the offset calculation will still be incorrect for nested static arrays containing dynamic elements. I think this helper needs to resolve dynamism recursively instead of checking only the top level type. It would also help to add a test for nested cases like bytes[2][2] to confirm the behavior. Once this is handled the fix should be much safer. |
Adds a disabled test for `(bytes[2][2], uint256)` that documents a pre-existing bug in `TypeDecoder.decodeArrayElements` where nested StaticArray of dynamic-element StaticArrays throws `StringIndexOutOfBoundsException`. The bug exists on `main` independent of the offset fix in this PR — kept disabled so it's tracked as a separate follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
When a
StaticArraycontains dynamic-element types (DynamicBytes,DynamicArray) at the top level of a tuple, it is encoded as a single 32-byte pointer slot. However, both branches that handle StaticArrays inDefaultFunctionReturnDecoder.buildadvanced the top-level offset bylength * 64instead of64, over-advancing past the pointer and corrupting the decode position for every subsequent parameter in the tuple.The
StaticArrayTypeReferencebranch was also missing element-type inspection entirely — it always usedlength * slotSizeregardless of element type.This PR consolidates the offset arithmetic into a single
staticArrayOffsethelper that is called from both branches and handlesDynamicStruct,DynamicBytes,DynamicArray,Utf8String,StaticStruct, and plain types consistently.Reproducer
Decoding a tuple of
(bytes[2], uint256)returns the wronguint256. The trailingUint256reads from the next slot in the encoded buffer (thebytes[0]inner offset pointer = 64) instead of the actual uint256 (= 1965).The included regression test
FunctionReturnDecoderTest.testDecodeStaticArrayOfDynamicBytesFollowedByUintfails onmainwith:and passes after the fix.
What changed
staticArrayOffset(typeReference, length, slotSize)helper inDefaultFunctionReturnDecoder.StaticArrayTypeReferencebranch and theStaticArray.class.isAssignableFrom(...)branch) use it.DynamicStruct | DynamicBytes | DynamicArray | Utf8String→ 1 slot;StaticStruct→ fields × length × slot; otherwise length × slot.Test plan
./gradlew :abi:testpassesmainand passes with the fix🤖 Generated with Claude Code