Skip to content

feat(abi): support dynamics abi#2

Open
0xbigapple wants to merge 8 commits intorelease_0.11.0from
feature/support-dynamics-abi
Open

feat(abi): support dynamics abi#2
0xbigapple wants to merge 8 commits intorelease_0.11.0from
feature/support-dynamics-abi

Conversation

@0xbigapple
Copy link
Owner

@0xbigapple 0xbigapple commented Mar 2, 2026

What does this PR do?

support abiV2 include static/dynamics arrays, struct

Why are these changes required?

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details

Why are these changes required?

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details


Summary by cubic

Adds ABI v2 support: encode/decode structs (tuples), static/dynamic arrays (including nested), packed encoding, and custom errors. Also fixes decoding/padding bugs and normalizes event/error signature hashes (no 0x).

  • New Features

    • Encode/decode StaticStruct/DynamicStruct and static/dynamic arrays, including nested/multi-dimensional and arrays of structs.
    • Added encodePacked support and FunctionEncoder.encodeWithSelector.
    • Custom errors via CustomError and CustomErrorEncoder (signature hashing).
    • TypeReference supports inner types; added @datatypes.reflection.Parameterized for nested generics.
    • AbiTypes/Utils: getTypeAString for structs/arrays; load custom types via Class.forName; support empty static arrays (StaticArray0) and improved type strings for arrays/structs.
  • Bug Fixes

    • Correct dynamic data offsets and numeric decoding.
    • Fixed multi-dimensional array and nested struct encode/decode, including empty arrays.
    • Correct Bytes and Utf8String padded length.
    • Improved DynamicStruct.getTypeAsString for arrays/nested types.
    • Breaking: EventEncoder.buildEventSignature now returns a hash without the 0x prefix.

Written for commit 1997f15. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Added custom error encoding support with signature hash computation
    • Introduced packed ABI encoding (abi.encodePacked) for multiple types
    • Added new decoding methods for addresses and dynamic bytes
    • Enhanced support for nested and multi-dimensional arrays in encoding/decoding
  • Improvements

    • Improved handling of complex struct types (static and dynamic) in encoding/decoding
    • Better type resolution for struct and array type strings
    • Expanded function encoding to support method selector-based encoding

@coderabbitai
Copy link

coderabbitai bot commented Mar 2, 2026

📝 Walkthrough

Walkthrough

This PR significantly enhances the ABI encoding/decoding library with support for custom Ethereum error encoding, packed value encoding (abi.encodePacked), nested struct and complex array handling, SPI-based encoder/decoder selection, and improved type resolution. Introduces new public APIs and extensive test coverage for complex data structures.

Changes

Cohort / File(s) Summary
Custom Error Support
abi/src/main/java/org/tron/trident/abi/CustomErrorEncoder.java, abi/src/main/java/org/tron/trident/abi/datatypes/CustomError.java, abi/src/test/java/org/tron/trident/abi/CustomErrorEncoderTest.java, abi/src/test/java/org/tron/trident/abi/datatypes/CustomErrorTest.java
New classes to encode and represent Ethereum-style custom errors. CustomErrorEncoder provides encode, buildErrorSignature, and calculateSignatureHash methods. CustomError wraps error name and parameters. Tests validate signature hashing and encoding logic.
Packed Encoding
abi/src/main/java/org/tron/trident/abi/TypeEncoder.java, abi/src/test/java/org/tron/trident/abi/TypeEncoderPackedTest.java
Added encodePacked public method and supporting helpers (removePadding, encodeStaticArrayWithDynamicStruct, encodeDynamicsTypesArraysOffsets, arrayEncodePacked). Comprehensive test suite with 60+ test methods covering all primitive types, arrays, structs, and edge cases.
SPI-Based Encoder/Decoder Selection
abi/src/main/java/org/tron/trident/abi/FunctionEncoder.java, abi/src/main/java/org/tron/trident/abi/FunctionReturnDecoder.java
Replaced lazy-loading pattern with static ServiceLoader initialization. Added new public methods encode(String methodId, List) and decodeDynamicBytes/decodeAddress helpers. Introduced abstract methods encodeWithSelector and encodePackedParameters.
Enhanced Type Handling & Resolution
abi/src/main/java/org/tron/trident/abi/Utils.java, abi/src/main/java/org/tron/trident/abi/TypeReference.java, abi/src/main/java/org/tron/trident/abi/AbiTypes.java
Extended Utils with 7+ new public methods for struct/type introspection (getTypeName, getStructType, findStructConstructor, staticStructNestedPublicFieldsFlatList, etc.). TypeReference now supports innerTypes field and getter. AbiTypes adds getTypeAString and dynamic class loading in getType.
Refactored Decoding Logic
abi/src/main/java/org/tron/trident/abi/DefaultFunctionReturnDecoder.java, abi/src/main/java/org/tron/trident/abi/TypeDecoder.java
Major refactoring of offset calculation to use TypeReference and handle dynamic/static structs and nested arrays. TypeDecoder adds 450+ lines of new logic for inner-type-aware decoding paths, ParameterOffsetTracker, and nested struct support. DefaultFunctionReturnDecoder refines length calculations for StaticArray with nested structs.
Function Encoding Enhancements
abi/src/main/java/org/tron/trident/abi/DefaultFunctionEncoder.java, abi/src/main/java/org/tron/trident/abi/EventEncoder.java
DefaultFunctionEncoder adds encodeWithSelector and encodePackedParameters methods, plus refined getLength logic for StaticArray with structured components. EventEncoder removes 0x prefix from signature hash string.
Datatype Struct & Array Improvements
abi/src/main/java/org/tron/trident/abi/datatypes/DynamicArray.java, abi/src/main/java/org/tron/trident/abi/datatypes/StaticArray.java, abi/src/main/java/org/tron/trident/abi/datatypes/DynamicStruct.java, abi/src/main/java/org/tron/trident/abi/datatypes/StaticStruct.java
Enhanced type string generation to handle nested structs and arrays conditionally. DynamicStruct/StaticStruct now populate itemTypes and support Array type detection. DynamicArray and StaticArray intelligently derive component types from actual values or struct metadata.
Padding & Length Calculations
abi/src/main/java/org/tron/trident/abi/datatypes/BytesType.java, abi/src/main/java/org/tron/trident/abi/datatypes/Utf8String.java, abi/src/main/java/org/tron/trident/abi/datatypes/Array.java
BytesType.bytes32PaddedLength now rounds to nearest 32-byte multiple instead of simple multiplication. Utf8String.bytes32PaddedLength returns MAX_BYTE_LENGTH for empty strings, 2 * MAX_BYTE_LENGTH otherwise. Array adds getNativeValueCopy method.
Type Definition & Annotation
abi/src/main/java/org/tron/trident/abi/datatypes/reflection/Parameterized.java
New runtime retention annotation targeting method parameters to specify concrete parameterized types for reflection-based processing. Single element() member of type Class<?>.
Test Fixture Expansion
abi/src/test/java/org/tron/trident/abi/AbiV2TestFixture.java
Significantly expanded public constants for 40+ function names and introduced 8 public inner data classes (Foo, Bar, Nazzy, Barr, Naz, Qux, BytesStruct, ArrayStruct) extending DynamicStruct/StaticStruct. Added event declarations and complex TypeReference mappings for nested/multi-dimensional structures.
Comprehensive Encoding/Decoding Tests
abi/src/test/java/org/tron/trident/abi/DefaultFunctionEncoderTest.java, abi/src/test/java/org/tron/trident/abi/FunctionReturnDecoderTest.java, abi/src/test/java/org/tron/trident/abi/TypeDecoderTest.java
DefaultFunctionEncoderTest adds 30+ test methods for complex structs, nested arrays, and multi-dimensional encoding. FunctionReturnDecoderTest adds 40+ test methods for decoding dynamic/static structs, nested structures, and arrays. TypeDecoderTest adds tests for nested array scenarios.
Datatype-Specific Tests
abi/src/test/java/org/tron/trident/abi/datatypes/DynamicArrayTest.java, abi/src/test/java/org/tron/trident/abi/datatypes/StaticArrayTest.java, abi/src/test/java/org/tron/trident/abi/datatypes/StaticStructTest.java
New focused test classes validating type string generation for empty arrays, dynamic/static structs, and multi-dimensional arrays.
Event & TypeEncoder Tests
abi/src/test/java/org/tron/trident/abi/EventEncoderTest.java, abi/src/test/java/org/tron/trident/abi/TypeEncoderTest.java
EventEncoderTest removes 0x prefix expectations and adds tests for dynamic struct/array signatures. TypeEncoderTest adds 5 test methods for nested arrays, struct arrays, and dynamic byte handling.
Integration Test
core/src/test/java/org/tron/trident/core/AbiV2ContractTest.java
End-to-end contract test suite with 780 lines covering StaticInfo, DynamicInfo, NestedInfo structs and complex array encoding/decoding. Includes contract deployment, encoding, round-trip verification, and packed encoding comparison with Solidity-generated results.

Sequence Diagram(s)

This change involves encoding/decoding enhancements across multiple heterogeneous areas (SPI patterns, padding calculations, struct handling, packed encoding) that do not form a cohesive sequential interaction flow suitable for a single diagram. The modifications are largely distributed logic enhancements in parallel paths rather than a unified multi-component interaction pattern. Sequence diagrams are not applicable.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 Hoppity-hop through structs so nested,
CustomErrors encoded, carefully tested,
Packed bytes and paddings, now fit just right,
SPI loaders bounce us to the light! 🌟

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.64% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'feat(abi): support dynamics abi' is vague and contains a grammatical error. It uses non-descriptive phrasing ('dynamics abi') that doesn't clearly convey the comprehensive scope of changes including ABI v2 support, struct/array encoding-decoding, custom errors, and packed encoding. Revise the title to be more specific and grammatically correct, such as 'feat(abi): add ABI v2 support for structs, arrays, and packed encoding' or 'feat(abi): support ABI v2 with struct/array encoding-decoding and custom errors'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/support-dynamics-abi
📝 Coding Plan for PR comments
  • Generate coding plan

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

17 issues found across 37 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="abi/src/test/java/org/tron/trident/abi/DefaultFunctionEncoderTest.java">

<violation number="1" location="abi/src/test/java/org/tron/trident/abi/DefaultFunctionEncoderTest.java:90">
P1: Copy-paste bug: `nazz2` is created but never used in the assertion — the test still passes `nazz` instead of `nazz2`, so the "correct handling of empty list of dynamic struct" case is not actually verified.</violation>
</file>

<file name="abi/src/main/java/org/tron/trident/abi/TypeReference.java">

<violation number="1" location="abi/src/main/java/org/tron/trident/abi/TypeReference.java:45">
P2: `getInnerTypes()` can return `null` since `innerTypes` is only initialized in one constructor, but all callers in `TypeDecoder` dereference it without null checks (e.g., `innerTypes.size()`). Consider initializing the field to an empty list to avoid potential `NullPointerException`.</violation>
</file>

<file name="abi/src/main/java/org/tron/trident/abi/datatypes/DynamicArray.java">

<violation number="1" location="abi/src/main/java/org/tron/trident/abi/datatypes/DynamicArray.java:29">
P1: Inconsistency: the varargs constructor is missing the `Array.class.isAssignableFrom` check that both the List constructor (line 41) and `StaticArray`'s constructor have. Calling `new DynamicArray(nestedArray1, nestedArray2)` with Array-type elements will resolve the wrong component type via `AbiTypes.getType()` instead of using the element's actual class.</violation>

<violation number="2" location="abi/src/main/java/org/tron/trident/abi/datatypes/DynamicArray.java:85">
P1: `DynamicArray.class.isAssignableFrom(...)` should be `Array.class.isAssignableFrom(...)` to also handle `StaticArray` elements (e.g., `uint256[3][]`). The `StaticArray.getTypeAsString()` uses `Array.class` for this check — this should be consistent.</violation>
</file>

<file name="abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java">

<violation number="1" location="abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java:372">
P1: Unchecked cast: `Class.forName(type)` may return a class that does not extend `Type`. Use `Class.forName(type).asSubclass(Type.class)` to validate the type hierarchy at load time and fail fast with a clear error instead of causing a deferred `ClassCastException`.</violation>

<violation number="2" location="abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java:372">
P1: Security concern: `Class.forName(type)` with a raw, potentially user-controlled `type` string allows loading arbitrary classes on the classpath, triggering their static initializers. Unlike other `Class.forName` usages in this codebase that use controlled prefixed strings, this passes external input directly. Consider restricting the input — e.g., validating that `type` starts with an expected package prefix like `org.tron.trident.abi.datatypes`.</violation>
</file>

<file name="abi/src/main/java/org/tron/trident/abi/CustomErrorEncoder.java">

<violation number="1" location="abi/src/main/java/org/tron/trident/abi/CustomErrorEncoder.java:40">
P1: Bug: `calculateSignatureHash` returns the full 32-byte hash (64 hex chars), but Solidity custom error selectors are only 4 bytes (8 hex chars), identical to function selectors. This was likely copied from `EventEncoder.buildEventSignature` where returning the full hash is correct (event topics are 32 bytes), but for error selectors the hash must be truncated to 4 bytes, as done in `FunctionEncoder.buildMethodId` with `.substring(2, 10)`.</violation>
</file>

<file name="abi/src/test/java/org/tron/trident/abi/CustomErrorEncoderTest.java">

<violation number="1" location="abi/src/test/java/org/tron/trident/abi/CustomErrorEncoderTest.java:75">
P1: Wrong class used: this test calls `EventEncoder.buildMethodSignature` instead of `CustomErrorEncoder.buildErrorSignature`. All other tests in this `CustomErrorEncoderTest` class use `CustomErrorEncoder` methods. This looks like a copy-paste mistake — the dynamic-array error-signature path of `CustomErrorEncoder` is left untested.</violation>
</file>

<file name="abi/src/main/java/org/tron/trident/abi/datatypes/DynamicStruct.java">

<violation number="1" location="abi/src/main/java/org/tron/trident/abi/datatypes/DynamicStruct.java:22">
P2: Exposing a mutable `ArrayList` as `public` breaks encapsulation. External code can modify `itemTypes` out of sync with the struct's actual values. Consider keeping the field `private` and providing a getter that returns an unmodifiable list if external access is needed.</violation>

<violation number="2" location="abi/src/main/java/org/tron/trident/abi/datatypes/DynamicStruct.java:55">
P2: The `DynamicArray` and `Array` branches both execute identical code (`getValue().get(i).getTypeAsString()`), and `DynamicArray extends Array`, so the `DynamicArray` check is redundant. These can be merged into a single condition to reduce duplication.</violation>
</file>

<file name="abi/src/test/java/org/tron/trident/abi/datatypes/CustomErrorTest.java">

<violation number="1" location="abi/src/test/java/org/tron/trident/abi/datatypes/CustomErrorTest.java:21">
P2: Arguments to `assertEquals` are swapped: JUnit 5's signature is `assertEquals(expected, actual)`, but here the actual value is passed first. This won't break the test, but if it ever fails, the error message will be inverted and confusing.</violation>
</file>

<file name="abi/src/main/java/org/tron/trident/abi/datatypes/Utf8String.java">

<violation number="1" location="abi/src/main/java/org/tron/trident/abi/datatypes/Utf8String.java:36">
P1: Potential `NullPointerException`: `value` can be `null` (the constructor doesn't prevent it and `equals`/`hashCode` explicitly handle `null`). Calling `value.isEmpty()` will throw NPE. Add a null guard.</violation>
</file>

<file name="abi/src/main/java/org/tron/trident/abi/TypeDecoder.java">

<violation number="1" location="abi/src/main/java/org/tron/trident/abi/TypeDecoder.java:993">
P1: Bug: Offset calculation after decoding a nested `StaticArray` reads array data as a length prefix. `decodeUintAsInt(input, currOffset)` interprets the first element of the already-decoded static array as a byte-length (the formula `(val / MAX_BYTE_LENGTH) + 2` is for length-prefixed DynamicBytes/Utf8String). Static arrays are inline-encoded with no length prefix, so the correct advance should be based on the known `staticLength` and inner element size (e.g., `staticLength * MAX_BYTE_LENGTH_FOR_HEX_STRING` for simple types).</violation>
</file>

<file name="core/src/test/java/org/tron/trident/core/AbiV2ContractTest.java">

<violation number="1" location="core/src/test/java/org/tron/trident/core/AbiV2ContractTest.java:539">
P2: Missing `Thread.sleep()` (or polling for confirmation) between `broadcastTransaction` and the subsequent `triggerConstantContract` read. Without waiting for the transaction to be confirmed, `lastNestedInfo` may return stale data, making this test flaky. The `deployContract` test correctly waits 5 seconds after broadcasting.</violation>
</file>

<file name="abi/src/test/java/org/tron/trident/abi/TypeEncoderPackedTest.java">

<violation number="1" location="abi/src/test/java/org/tron/trident/abi/TypeEncoderPackedTest.java:112">
P2: Swapped `assertEquals` arguments: `assertEquals(expected, actual)` convention requires the expected literal first. This causes confusing failure messages where expected and actual labels are flipped.</violation>
</file>

<file name="abi/src/test/java/org/tron/trident/abi/AbiV2TestFixture.java">

<violation number="1" location="abi/src/test/java/org/tron/trident/abi/AbiV2TestFixture.java:64">
P2: Duplicate constants: `FUNC_IDNARBARFOONARFOODYNAMICARRAY` and `FUNC_IDNARBARFOONARFOOARRAYS` both have value `"idNarBarFooNarFooArrays"`. Consider removing the redundant constant and using a single one to avoid confusion.</violation>
</file>

<file name="abi/src/main/java/org/tron/trident/abi/Utils.java">

<violation number="1" location="abi/src/main/java/org/tron/trident/abi/Utils.java:315">
P1: Bug: Field ordering is broken for structs with nested structs. `canonicalFields` (non-struct) are concatenated before `nestedFields` (expanded struct fields), which destroys the original declaration order. For `struct Baz { Bar { int a, int b }, int c }`, this returns `{c, a, b}` instead of the documented `{a, b, c}`. Since this is used for ABI encoding/decoding, field order matters. The fix should iterate fields in declaration order and either include them directly or recursively expand nested structs inline.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@0xbigapple
Copy link
Owner Author

@claude review this pr

@0xbigapple
Copy link
Owner Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Mar 12, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
abi/src/main/java/org/tron/trident/abi/TypeEncoder.java (1)

353-358: ⚠️ Potential issue | 🟠 Major

Use UTF-8 byte length for string offset calculation in ABI encoding.

Line 356 uses String.length(), which returns the character count in UTF-16, not the byte length required for ABI offset calculation. For strings with multi-byte UTF-8 characters, this will undercount the encoded size and shift all subsequent element offsets. Use getBytes(StandardCharsets.UTF_8).length instead.

Suggested fix
           int bytesLength =
               arrayOfBytes
                   ? ((byte[]) value.getValue().get(i - 1).getValue()).length
-                  : ((String) value.getValue().get(i - 1).getValue()).length();
+                  : ((String) value.getValue().get(i - 1).getValue())
+                      .getBytes(StandardCharsets.UTF_8).length;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/TypeEncoder.java` around lines 353 -
358, In TypeEncoder (the block computing bytesLength/numberOfWords for array
elements), replace use of String.length() with the UTF-8 byte length by calling
((String) value.getValue().get(i -
1).getValue()).getBytes(StandardCharsets.UTF_8).length so offsets use encoded
byte size; add/import java.nio.charset.StandardCharsets if missing and keep
arrayOfBytes branch unchanged (byte[] uses .length). This ensures numberOfWords
and totalBytesLength are computed from UTF-8 bytes rather than UTF-16 char
count.
🧹 Nitpick comments (15)
abi/src/test/java/org/tron/trident/abi/datatypes/CustomErrorTest.java (1)

21-26: JUnit assertion argument order and incomplete iteration check.

  1. JUnit convention is assertEquals(expected, actual). The current order assertEquals(event.getName(), "MyError") has actual first, which will produce confusing failure messages.

  2. The iteration loop doesn't verify that event.getParameters() doesn't have more elements than expected. If the error had extra parameters, the test would pass incorrectly.

♻️ Proposed fix
-    assertEquals(event.getName(), "MyError");
+    assertEquals("MyError", event.getName());
+    assertEquals(parameters.size(), event.getParameters().size());

     Iterator<TypeReference<?>> expectedParameter = parameters.iterator();
     for (TypeReference<?> actualParameter : event.getParameters()) {
-      assertEquals(expectedParameter.next(), actualParameter);
+      assertEquals(expectedParameter.next(), actualParameter);
     }

Or use a more direct comparison:

assertEquals("MyError", event.getName());
assertEquals(parameters, event.getParameters());
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/test/java/org/tron/trident/abi/datatypes/CustomErrorTest.java` around
lines 21 - 26, Swap assertion argument order and strengthen parameter equality:
change assertEquals(event.getName(), "MyError") to assertEquals("MyError",
event.getName()); replace the manual iterator loop over event.getParameters()
with a direct equality/assertion that verifies both contents and size (for
example assertEquals(parameters, event.getParameters()) or
assertEquals(parameters.size(), event.getParameters().size()) followed by
element-by-element comparison) so that extra parameters on event will fail;
update test in CustomErrorTest to use these corrected assertions referencing
event and parameters.
abi/src/main/java/org/tron/trident/abi/FunctionReturnDecoder.java (3)

75-82: Same raw type issue as decodeDynamicBytes.

♻️ Proposed fix
   public static String decodeAddress(String rawInput) {
-    List outputParameters = new ArrayList<TypeReference<Type>>();
+    List<TypeReference<Type>> outputParameters = new ArrayList<>();
     outputParameters.add(new TypeReference<Address>() {});

     List<Type> typeList = decoder.decodeFunctionResult(rawInput, outputParameters);

     return typeList.isEmpty() ? null : ((Address) typeList.get(0)).getValue();
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/FunctionReturnDecoder.java` around
lines 75 - 82, The decodeAddress method currently uses a raw List for
outputParameters (same raw-type problem as decodeDynamicBytes); change the
declaration to a parameterized List<TypeReference<Address>> (e.g.,
List<TypeReference<Address>> outputParameters = new ArrayList<>()) and keep the
TypeReference<Address>(){} element, then call
decoder.decodeFunctionResult(rawInput, outputParameters) as before so generics
are correct for decodeAddress and related decoding logic.

60-65: Missing Javadoc for decodeAddress.

The decodeDynamicBytes method has Javadoc, but decodeAddress (lines 75-82) lacks documentation. Both are new public API methods and should be documented consistently.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/FunctionReturnDecoder.java` around
lines 60 - 65, Add Javadoc for the public method decodeAddress matching the
style and detail of decodeDynamicBytes: describe that it decodes an ABI-encoded
address from rawInput, document the parameters (rawInput) and return value
(String or Address type and null on invalid response), and mention any
exceptions or behavior for invalid input; reference the method name
decodeAddress so reviewers can locate it and ensure consistency with the
existing decodeDynamicBytes Javadoc format.

66-73: Use parameterized type instead of raw List.

The declaration List outputParameters = new ArrayList<TypeReference<Type>>() uses a raw type on the left side, which bypasses generic type checking and generates compiler warnings.

♻️ Proposed fix
   public static byte[] decodeDynamicBytes(String rawInput) {
-    List outputParameters = new ArrayList<TypeReference<Type>>();
+    List<TypeReference<Type>> outputParameters = new ArrayList<>();
     outputParameters.add(new TypeReference<DynamicBytes>() {});

     List<Type> typeList = decoder.decodeFunctionResult(rawInput, outputParameters);

     return typeList.isEmpty() ? null : ((DynamicBytes) typeList.get(0)).getValue();
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/FunctionReturnDecoder.java` around
lines 66 - 73, In decodeDynamicBytes, replace the raw-typed declaration "List
outputParameters = new ArrayList<TypeReference<Type>>()" with a parameterized
type such as "List<TypeReference<Type>> outputParameters = new ArrayList<>()" so
generics are preserved; keep the existing add of new
TypeReference<DynamicBytes>() {} and the subsequent call to
decoder.decodeFunctionResult(rawInput, outputParameters) unchanged.
abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java (1)

386-394: Rename static method for naming consistency: getTypeAStringgetTypeAsString.

The static utility method in AbiTypes has a naming inconsistency—it's missing the 's' in "as" compared to the instance method Type.getTypeAsString() used throughout the codebase. Renaming would improve consistency and discoverability.

This change requires updating 5 call sites:

  • StaticStruct.java:45
  • StaticArray.java:93
  • DynamicStruct.java:61
  • DynamicArray.java:81, 89
♻️ Proposed rename for consistency
-  public static String getTypeAString(Class<? extends Type> type) {
+  public static String getTypeAsString(Class<? extends Type> type) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java` around lines
386 - 394, Rename the static method AbiTypes.getTypeAString to getTypeAsString
and update all callers to use the new name; specifically change the method
declaration in AbiTypes (currently getTypeAString) to getTypeAsString and update
the five call sites in StaticStruct, StaticArray, DynamicStruct, and
DynamicArray to call AbiTypes.getTypeAsString(...) (this aligns the static
helper with the instance method Type.getTypeAsString and preserves existing
behavior for Utf8String and DynamicBytes by returning "string" and "bytes"
respectively).
abi/src/main/java/org/tron/trident/abi/datatypes/DynamicArray.java (1)

74-77: Misleading comment: the code actually handles struct arrays correctly.

The comment states "This will fail if the dynamic array is an array of structs," but lines 78-79 explicitly handle StructType by calling Utils.getStructType(). Consider removing or correcting the comment.

📝 Suggested comment fix
-    // Handle dynamic array of zero length. This will fail if the dynamic array
-    // is an array of structs.
+    // Handle dynamic array of zero length
     if (value.isEmpty()) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/datatypes/DynamicArray.java` around
lines 74 - 77, The comment above the empty dynamic-array handling in
DynamicArray.java is misleading; update or remove the line claiming "This will
fail if the dynamic array is an array of structs" because the subsequent code
handles StructType via Utils.getStructType() (see value.isEmpty() check and
handling of StructType). Edit the comment to accurately reflect that zero-length
dynamic arrays are handled, including struct arrays, or simply remove the
incorrect sentence so the code and comment are consistent.
abi/src/main/java/org/tron/trident/abi/datatypes/DynamicStruct.java (2)

55-59: Redundant conditional branches.

Both if branches (StructType/DynamicArray and Array) execute identical code: getValue().get(i).getTypeAsString(). These can be consolidated.

♻️ Suggested simplification
-      if (StructType.class.isAssignableFrom(cls)
-          || DynamicArray.class.isAssignableFrom(cls)) {
-        type.append(getValue().get(i).getTypeAsString());
-      } else if (Array.class.isAssignableFrom(cls)) {
+      if (StructType.class.isAssignableFrom(cls)
+          || Array.class.isAssignableFrom(cls)) {
         type.append(getValue().get(i).getTypeAsString());
       } else {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/datatypes/DynamicStruct.java` around
lines 55 - 59, The two conditional branches in DynamicStruct (the check using
StructType.class.isAssignableFrom(cls) ||
DynamicArray.class.isAssignableFrom(cls) and the else if
Array.class.isAssignableFrom(cls)) both call
getValue().get(i).getTypeAsString(); simplify by removing the redundant branch
and consolidating into a single branch or unconditional call: inside the method
in class DynamicStruct, replace the duplicated condition logic around cls with a
single call to getValue().get(i).getTypeAsString() (keeping any surrounding
context intact) so only one path uses getTypeAsString().

22-22: Public mutable list exposes internal state.

Exposing itemTypes as a public final field allows external code to mutate the list (add/remove elements). Consider returning an unmodifiable view or making the field private with a getter that returns Collections.unmodifiableList(itemTypes).

♻️ Suggested encapsulation
-  public final List<Class<Type>> itemTypes = new ArrayList<>();
+  private final List<Class<Type>> itemTypes = new ArrayList<>();
+
+  public List<Class<Type>> getItemTypes() {
+    return Collections.unmodifiableList(itemTypes);
+  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/datatypes/DynamicStruct.java` at line
22, The public mutable field itemTypes in class DynamicStruct exposes internal
state; make itemTypes private (e.g., private final List<Class<Type>> itemTypes =
new ArrayList<>();) and add a public accessor getItemTypes() that returns
Collections.unmodifiableList(itemTypes) so callers cannot mutate the internal
list; update any internal usages to reference the private field and adjust
external code to call getItemTypes() where necessary.
abi/src/main/java/org/tron/trident/abi/datatypes/CustomError.java (1)

22-29: Make fields final for immutability.

The name and parameters fields are not declared final, which allows accidental reassignment. Since this is a value-holding class, immutability is preferred.

♻️ Suggested fix
 public class CustomError {
-  private String name;
-  private List<TypeReference<Type>> parameters;
+  private final String name;
+  private final List<TypeReference<Type>> parameters;

   public CustomError(String name, List<TypeReference<?>> parameters) {
     this.name = name;
     this.parameters = convert(parameters);
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/datatypes/CustomError.java` around
lines 22 - 29, Make the CustomError class immutable by declaring the fields name
and parameters as final and ensuring they are only set in the constructor;
update the class declaration to use private final String name and private final
List<TypeReference<Type>> parameters, keep the existing constructor
CustomError(String name, List<TypeReference<?>> parameters) to assign these
finals (using convert(parameters) for parameters), and verify no other code
reassigns these fields or requires setters.
abi/src/test/java/org/tron/trident/abi/datatypes/DynamicArrayTest.java (1)

38-50: Raw type usage in multidimensional array test.

Line 40 uses raw DynamicArray without generic type parameter. While this works for the test, it generates compiler warnings. Consider using DynamicArray<DynamicArray<DynamicArray<Uint256>>> for type safety.

♻️ Suggested type-safe version
   `@Test`
   public void testMultidimensionalDynamicArray() {
-    DynamicArray<DynamicArray> array =
+    `@SuppressWarnings`("unchecked")
+    DynamicArray<DynamicArray<DynamicArray<Uint256>>> array =
         new DynamicArray<>(
             DynamicArray.class,
             Collections.singletonList(
                 new DynamicArray<>(
                     DynamicArray.class,
                     Collections.singletonList(
                         new DynamicArray<>(
                             Uint256.class, new ArrayList<>())))));
     assertEquals("uint256[][][]", array.getTypeAsString());
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/test/java/org/tron/trident/abi/datatypes/DynamicArrayTest.java`
around lines 38 - 50, The testMultidimensionalDynamicArray uses raw types for
DynamicArray which causes compiler warnings; update the declarations and
constructors to use fully parameterized nested generics (e.g.,
DynamicArray<DynamicArray<DynamicArray<Uint256>>> and inner
DynamicArray<DynamicArray<Uint256>> / DynamicArray<Uint256>) so the test remains
the same (assertEquals("uint256[][][]", array.getTypeAsString())) but is
type-safe; adjust the DynamicArray instantiations in the test to use these
generic types rather than raw DynamicArray.
abi/src/main/java/org/tron/trident/abi/TypeEncoder.java (1)

289-297: Reuse the encoded dynamic value.

encode(type) is invoked twice for every dynamic member here. Reusing encodedValue avoids double work on nested structs/arrays and keeps the offset math tied to the exact payload appended.

Suggested fix
         String encodedValue = encode(type);
-        dynamicValues.add(encode(type));
+        dynamicValues.add(encodedValue);
         dynamicOffset += encodedValue.length() >> 1;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/TypeEncoder.java` around lines 289 -
297, The code calls encode(type) twice for dynamic members; capture the result
once and reuse it: replace the second encode(type) call with the local
encodedValue variable so dynamicValues.add(encodedValue) is used and
dynamicOffset is incremented from encodedValue.length() (keeping the current
shift/right-by-1 logic). Update the block in TypeEncoder where isDynamic(type)
is handled (references: dynamicOffset, offsetsAndStaticValues, dynamicValues,
encodedValue, encode(type)) to ensure the offset arithmetic and appended payload
use the exact same encoded bytes.
core/src/test/java/org/tron/trident/core/AbiV2ContractTest.java (1)

718-723: Exercise FunctionEncoder here instead of hardcoded calldata.

The test builds function above but then bypasses it with a raw selector/payload string. That leaves the new multi-dimensional array encoder path untested, so an encoder regression can still keep this test green.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@core/src/test/java/org/tron/trident/core/AbiV2ContractTest.java` around lines
718 - 723, The test currently constructs a Function named `function` but calls
`client.triggerConstantContract(...)` with a hardcoded calldata string; replace
that raw hex with the encoded calldata produced from the constructed Function by
calling the encoder (e.g. FunctionEncoder.encode(function)) and pass that
encoded hex string into `client.triggerConstantContract(testAddress,
contractAddress, /* encoded calldata */)` so the multi-dimensional array
encoding path is exercised; keep the same `testAddress` and `contractAddress`
parameters and assert on the returned `transactionExtention` as before.
abi/src/main/java/org/tron/trident/abi/TypeDecoder.java (2)

766-771: Consider using a more specific exception type.

Throwing a generic RuntimeException at line 768 when the parameter annotation is missing makes it harder to catch and handle this specific error condition programmatically. Consider using IllegalArgumentException or a custom exception type.

♻️ Proposed change
     } else if (DynamicArray.class.isAssignableFrom(declaredField)) {
       if (parameter == null) {
-        throw new RuntimeException(
+        throw new IllegalArgumentException(
             "parameter can not be null, try to use annotation `@Parameterized` "
                 + "to specify the parameter type");
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/TypeDecoder.java` around lines 766 -
771, Replace the generic RuntimeException thrown in the DynamicArray handling
branch of TypeDecoder (the block where
DynamicArray.class.isAssignableFrom(declaredField) and parameter == null) with a
more specific exception; change the throw to IllegalArgumentException (or a
custom exception type) and include the same explanatory message so callers can
catch this specific invalid-argument condition when the `@Parameterized`
annotation is missing.

500-509: The isAssignableFrom direction is correct but semantically confusing.

Lines 501 and 503 use classType.isAssignableFrom(DynamicStruct.class) and classType.isAssignableFrom(StaticStruct.class), which return true only when classType IS DynamicStruct/StaticStruct itself (not for subclasses). This correctly routes base types to their list constructors and custom struct subclasses like Bar, Qux, BytesStruct to the reflection path in the else branch, which is the intended behavior. However, the condition reads backwards from typical pattern matching, which could confuse future maintainers—consider clarifying with a comment or renaming to reflect that this checks for base types specifically.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/TypeDecoder.java` around lines 500 -
509, Replace the confusing use of
classType.isAssignableFrom(DynamicStruct.class) and
classType.isAssignableFrom(StaticStruct.class) in TypeDecoder with explicit
equality checks (e.g., classType.equals(DynamicStruct.class) and
classType.equals(StaticStruct.class)) to make the intention clear that only the
exact base types should match; keep the existing reflection fallback
(findStructConstructor, ctor.newInstance) for custom struct subclasses and
preserve casting of parameters to List<Type> for the DynamicStruct/StaticStruct
branches.
abi/src/test/java/org/tron/trident/abi/AbiV2TestFixture.java (1)

928-961: Multi-dimensional array fixtures use necessary raw type casts.

The raw type casts like (Class)StaticArray2.class at lines 931, 941, etc., are unavoidable due to Java's type erasure limitations. Consider adding @SuppressWarnings("unchecked") or @SuppressWarnings("rawtypes") to suppress compiler warnings for these fixture declarations.

♻️ Proposed annotation
+  `@SuppressWarnings`({"unchecked", "rawtypes"})
   public static final Function setGetMultiDimStaticArrayFunction =
       new Function(FUNC_SETGETMULTIDIMSTATICARRAY,
+  `@SuppressWarnings`({"unchecked", "rawtypes"})
   public static final Function setGetMultiDimDynamicArrayFunction =
       new Function(FUNC_SETGETMULTIDIMDYNAMICARRAY,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/test/java/org/tron/trident/abi/AbiV2TestFixture.java` around lines
928 - 961, The multi-dimensional array fixture declarations
setGetMultiDimStaticArrayFunction and setGetMultiDimDynamicArrayFunction use
unavoidable raw type casts (e.g., (Class)StaticArray2.class and
(Class)DynamicArray.class) that trigger unchecked/rawtypes compiler warnings;
suppress these warnings by annotating the fixture fields with
`@SuppressWarnings`("unchecked") or `@SuppressWarnings`("rawtypes") as appropriate
(place the annotation immediately above the static final field declarations for
setGetMultiDimStaticArrayFunction and setGetMultiDimDynamicArrayFunction) so the
casts remain but the compiler warnings are silenced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@abi/src/main/java/org/tron/trident/abi/CustomErrorEncoder.java`:
- Around line 37-41: The calculateSignatureHash method uses platform default
bytes; change errorSignature.getBytes() to use a deterministic UTF-8 encoding
(e.g., errorSignature.getBytes(StandardCharsets.UTF_8)) and add the required
import (java.nio.charset.StandardCharsets) so the SHA3 hash is computed
consistently across platforms; update the method in
CustomErrorEncoder.calculateSignatureHash accordingly.

In `@abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java`:
- Around line 370-377: In the default branch of AbiTypes (the code that
currently does (Class<? extends Type>) Class.forName(type)), avoid the unchecked
cast and unsafe initialization: load the class without running static
initializers using Class.forName(type, false,
Thread.currentThread().getContextClassLoader()), then validate it implements
org.tron.trident.abi.datatypes.Type by using Class#asSubclass(Type.class) (or
checking Type.class.isAssignableFrom(loaded)), throwing a clear
UnsupportedOperationException if not; return the resulting Class<? extends Type>
from asSubclass so the cast is safe and static initializers are not executed.

In `@abi/src/main/java/org/tron/trident/abi/datatypes/DynamicArray.java`:
- Around line 27-34: The varargs DynamicArray(T... values) constructor (and the
List-based DynamicArray(List<T> values) constructor) access
values.get(0)/values[0] without checking emptiness, which can throw
ArrayIndexOutOfBoundsException; fix by adding a defensive precondition at the
top of both constructors that validates values/values list is non-empty and
throws a clear IllegalArgumentException (e.g., "values must not be empty")
instead of letting an AIOOBE occur—update the DynamicArray(T... values) and
DynamicArray(List<T> values) constructors accordingly.

In `@abi/src/main/java/org/tron/trident/abi/datatypes/StaticArray.java`:
- Around line 51-56: The deprecated StaticArray(List<T> values) constructor
reads values.get(0) and can throw IndexOutOfBoundsException for an empty list;
update the constructor in class StaticArray to guard against empty values by
checking values == null || values.isEmpty() and selecting a sensible default
element type (e.g., using AbiTypes.getType("uint256") or another appropriate
fallback) or by throwing an IllegalArgumentException with a clear message;
ensure the logic that currently branches on
StructType.class.isAssignableFrom(...) || Array.class.isAssignableFrom(...)
still uses the chosen fallback when the list is empty and keep the existing
behavior for non-empty lists (references: constructor StaticArray(List<T>
values), values.get(0), AbiTypes.getType).

In `@abi/src/main/java/org/tron/trident/abi/datatypes/Utf8String.java`:
- Around line 30-41: The bytes32PaddedLength() override in Utf8String is
misleading and unused for dynamic strings; remove the override from class
Utf8String so the class falls back to the default Type.bytes32PaddedLength()
behavior, or if you intend a special-purpose behavior keep the method but update
its Javadoc to clearly state it is not used for ABI dynamic encoding and return
a value that correctly documents that special use; locate the method by name
(bytes32PaddedLength) in Utf8String and consider callers like
TypeEncoder.encodeString() and encodeDynamicBytes() and the base Type
implementation when making the change.

In `@abi/src/main/java/org/tron/trident/abi/DefaultFunctionReturnDecoder.java`:
- Around line 104-144: The TypeReference.StaticArrayTypeReference branch in
DefaultFunctionReturnDecoder currently advances offset as length *
MAX_BYTE_LENGTH_FOR_HEX_STRING unconditionally; change it to compute the
top-level head size consistently with the StaticArray handling: extract the
parameterized element type from the TypeReference.StaticArrayTypeReference (via
the same logic used by getParameterizedTypeFromArray), then if the parameterized
type is DynamicStruct or Utf8String advance by one slot
(MAX_BYTE_LENGTH_FOR_HEX_STRING), if it is StaticStruct advance by
staticStructNestedPublicFieldsFlatList(elementClass).size() * length *
MAX_BYTE_LENGTH_FOR_HEX_STRING, otherwise advance by length *
MAX_BYTE_LENGTH_FOR_HEX_STRING; keep using TypeDecoder.decodeStaticArray for
decoding. Ensure you update the offset calculation in the
TypeReference.StaticArrayTypeReference branch to mirror the StaticArray branch's
branches (DynamicStruct, StaticStruct, Utf8String, fallback).

In `@abi/src/main/java/org/tron/trident/abi/TypeDecoder.java`:
- Around line 391-406: countNestedFields can NPE when
typeReference.getInnerTypes() is null for nested StaticStructs; update
countNestedFields to defensively handle a null return from
TypeReference.getInnerTypes() (for StaticStruct.class cases) by treating it as
empty (i.e. return 0) before calling stream(), so that StaticStructs with
missing inner type info do not cause an NPE; reference the countNestedFields
method, TypeReference.getInnerTypes(), StaticStruct.class and Utils.getTypeName
in your change.
- Around line 988-995: The offset after decoding a nested static array is being
computed as if the array were dynamic by reading a length prefix via
decodeUintAsInt at currOffset; instead, update the logic in TypeDecoder after
the call to TypeDecoder.decodeStaticArray to advance currOffset based on the
known staticLength and the element size (use staticLength and
Type.MAX_BYTE_LENGTH / element byte size logic) rather than reading a length
from the input with decodeUintAsInt; ensure you use
MAX_BYTE_LENGTH_FOR_HEX_STRING and Type.MAX_BYTE_LENGTH consistently to compute
the exact number of hex chars consumed for staticLength * elementSize so
currOffset points to the next ABI value.

In `@abi/src/main/java/org/tron/trident/abi/TypeEncoder.java`:
- Around line 53-60: The isDynamic(Type) method currently only treats a
StaticArray as dynamic when its component type is DynamicStruct; change it to
detect any dynamic component (DynamicBytes, Utf8String, DynamicArray, or any
type that itself returns true from isDynamic) so StaticArray with dynamic
elements returns true; then update the encode() routing so that arrays of those
StaticArray types go through encodeDynamicsTypesArraysOffsets (or the same
dynamic-array offset handling path) instead of inline encoding. Reference:
isDynamic, StaticArray.getComponentType(), DynamicBytes, Utf8String,
DynamicArray, DynamicStruct, encodeDynamicsTypesArraysOffsets, and encode().

In `@abi/src/main/java/org/tron/trident/abi/Utils.java`:
- Around line 124-136: findStructConstructor currently uses
Arrays.stream(...).findAny() which can pick an arbitrary matching constructor;
instead select the most specific constructor (prefer the one with the smallest
parameter count or the one with the fewest parameters equal to raw Type.class)
so typed constructors win over the generic bridge. Modify findStructConstructor
to stream declared constructors, filter by
allMatch(Type.class::isAssignableFrom) as before, then sort or reduce to pick
the constructor with minimal parameter length (and as a tiebreaker minimal count
of parameters exactly equal to Type.class) and return that or throw the same
RuntimeException if none found.

In `@abi/src/test/java/org/tron/trident/abi/CustomErrorEncoderTest.java`:
- Around line 68-76: The test method testBuildErrorSignatureWithDynamicArrays
currently calls EventEncoder.buildMethodSignature but lives in
CustomErrorEncoderTest; change the call to
CustomErrorEncoder.buildErrorSignature (or add a clarifying comment if you
intentionally compare both encoders) so the test asserts
CustomErrorEncoder.buildErrorSignature("DynamicArrayError", convert(parameters))
and remains consistent with the test class name; update only the invocation from
EventEncoder.buildMethodSignature to CustomErrorEncoder.buildErrorSignature in
that test (or add a one-line comment above the assertion explaining the
cross-check intent).

In `@core/src/test/java/org/tron/trident/core/AbiV2ContractTest.java`:
- Around line 449-450: The test in AbiV2ContractTest writes lastNestedInfo to
the public contractAddress and immediately reads it back, which can race with
block finality or external writes; update the test to poll for transaction
confirmation using getTransactionInfoById(txId) (or alternatively deploy an
isolated contract per test run) and only perform the read/assert once
getTransactionInfoById returns a confirmed/valid receipt for the write
transaction; apply the same change to the similar read-after-write assertions
around lines 635-657 to ensure the test waits for the write to be finalized
before asserting shared contract state.

---

Outside diff comments:
In `@abi/src/main/java/org/tron/trident/abi/TypeEncoder.java`:
- Around line 353-358: In TypeEncoder (the block computing
bytesLength/numberOfWords for array elements), replace use of String.length()
with the UTF-8 byte length by calling ((String) value.getValue().get(i -
1).getValue()).getBytes(StandardCharsets.UTF_8).length so offsets use encoded
byte size; add/import java.nio.charset.StandardCharsets if missing and keep
arrayOfBytes branch unchanged (byte[] uses .length). This ensures numberOfWords
and totalBytesLength are computed from UTF-8 bytes rather than UTF-16 char
count.

---

Nitpick comments:
In `@abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java`:
- Around line 386-394: Rename the static method AbiTypes.getTypeAString to
getTypeAsString and update all callers to use the new name; specifically change
the method declaration in AbiTypes (currently getTypeAString) to getTypeAsString
and update the five call sites in StaticStruct, StaticArray, DynamicStruct, and
DynamicArray to call AbiTypes.getTypeAsString(...) (this aligns the static
helper with the instance method Type.getTypeAsString and preserves existing
behavior for Utf8String and DynamicBytes by returning "string" and "bytes"
respectively).

In `@abi/src/main/java/org/tron/trident/abi/datatypes/CustomError.java`:
- Around line 22-29: Make the CustomError class immutable by declaring the
fields name and parameters as final and ensuring they are only set in the
constructor; update the class declaration to use private final String name and
private final List<TypeReference<Type>> parameters, keep the existing
constructor CustomError(String name, List<TypeReference<?>> parameters) to
assign these finals (using convert(parameters) for parameters), and verify no
other code reassigns these fields or requires setters.

In `@abi/src/main/java/org/tron/trident/abi/datatypes/DynamicArray.java`:
- Around line 74-77: The comment above the empty dynamic-array handling in
DynamicArray.java is misleading; update or remove the line claiming "This will
fail if the dynamic array is an array of structs" because the subsequent code
handles StructType via Utils.getStructType() (see value.isEmpty() check and
handling of StructType). Edit the comment to accurately reflect that zero-length
dynamic arrays are handled, including struct arrays, or simply remove the
incorrect sentence so the code and comment are consistent.

In `@abi/src/main/java/org/tron/trident/abi/datatypes/DynamicStruct.java`:
- Around line 55-59: The two conditional branches in DynamicStruct (the check
using StructType.class.isAssignableFrom(cls) ||
DynamicArray.class.isAssignableFrom(cls) and the else if
Array.class.isAssignableFrom(cls)) both call
getValue().get(i).getTypeAsString(); simplify by removing the redundant branch
and consolidating into a single branch or unconditional call: inside the method
in class DynamicStruct, replace the duplicated condition logic around cls with a
single call to getValue().get(i).getTypeAsString() (keeping any surrounding
context intact) so only one path uses getTypeAsString().
- Line 22: The public mutable field itemTypes in class DynamicStruct exposes
internal state; make itemTypes private (e.g., private final List<Class<Type>>
itemTypes = new ArrayList<>();) and add a public accessor getItemTypes() that
returns Collections.unmodifiableList(itemTypes) so callers cannot mutate the
internal list; update any internal usages to reference the private field and
adjust external code to call getItemTypes() where necessary.

In `@abi/src/main/java/org/tron/trident/abi/FunctionReturnDecoder.java`:
- Around line 75-82: The decodeAddress method currently uses a raw List for
outputParameters (same raw-type problem as decodeDynamicBytes); change the
declaration to a parameterized List<TypeReference<Address>> (e.g.,
List<TypeReference<Address>> outputParameters = new ArrayList<>()) and keep the
TypeReference<Address>(){} element, then call
decoder.decodeFunctionResult(rawInput, outputParameters) as before so generics
are correct for decodeAddress and related decoding logic.
- Around line 60-65: Add Javadoc for the public method decodeAddress matching
the style and detail of decodeDynamicBytes: describe that it decodes an
ABI-encoded address from rawInput, document the parameters (rawInput) and return
value (String or Address type and null on invalid response), and mention any
exceptions or behavior for invalid input; reference the method name
decodeAddress so reviewers can locate it and ensure consistency with the
existing decodeDynamicBytes Javadoc format.
- Around line 66-73: In decodeDynamicBytes, replace the raw-typed declaration
"List outputParameters = new ArrayList<TypeReference<Type>>()" with a
parameterized type such as "List<TypeReference<Type>> outputParameters = new
ArrayList<>()" so generics are preserved; keep the existing add of new
TypeReference<DynamicBytes>() {} and the subsequent call to
decoder.decodeFunctionResult(rawInput, outputParameters) unchanged.

In `@abi/src/main/java/org/tron/trident/abi/TypeDecoder.java`:
- Around line 766-771: Replace the generic RuntimeException thrown in the
DynamicArray handling branch of TypeDecoder (the block where
DynamicArray.class.isAssignableFrom(declaredField) and parameter == null) with a
more specific exception; change the throw to IllegalArgumentException (or a
custom exception type) and include the same explanatory message so callers can
catch this specific invalid-argument condition when the `@Parameterized`
annotation is missing.
- Around line 500-509: Replace the confusing use of
classType.isAssignableFrom(DynamicStruct.class) and
classType.isAssignableFrom(StaticStruct.class) in TypeDecoder with explicit
equality checks (e.g., classType.equals(DynamicStruct.class) and
classType.equals(StaticStruct.class)) to make the intention clear that only the
exact base types should match; keep the existing reflection fallback
(findStructConstructor, ctor.newInstance) for custom struct subclasses and
preserve casting of parameters to List<Type> for the DynamicStruct/StaticStruct
branches.

In `@abi/src/main/java/org/tron/trident/abi/TypeEncoder.java`:
- Around line 289-297: The code calls encode(type) twice for dynamic members;
capture the result once and reuse it: replace the second encode(type) call with
the local encodedValue variable so dynamicValues.add(encodedValue) is used and
dynamicOffset is incremented from encodedValue.length() (keeping the current
shift/right-by-1 logic). Update the block in TypeEncoder where isDynamic(type)
is handled (references: dynamicOffset, offsetsAndStaticValues, dynamicValues,
encodedValue, encode(type)) to ensure the offset arithmetic and appended payload
use the exact same encoded bytes.

In `@abi/src/test/java/org/tron/trident/abi/AbiV2TestFixture.java`:
- Around line 928-961: The multi-dimensional array fixture declarations
setGetMultiDimStaticArrayFunction and setGetMultiDimDynamicArrayFunction use
unavoidable raw type casts (e.g., (Class)StaticArray2.class and
(Class)DynamicArray.class) that trigger unchecked/rawtypes compiler warnings;
suppress these warnings by annotating the fixture fields with
`@SuppressWarnings`("unchecked") or `@SuppressWarnings`("rawtypes") as appropriate
(place the annotation immediately above the static final field declarations for
setGetMultiDimStaticArrayFunction and setGetMultiDimDynamicArrayFunction) so the
casts remain but the compiler warnings are silenced.

In `@abi/src/test/java/org/tron/trident/abi/datatypes/CustomErrorTest.java`:
- Around line 21-26: Swap assertion argument order and strengthen parameter
equality: change assertEquals(event.getName(), "MyError") to
assertEquals("MyError", event.getName()); replace the manual iterator loop over
event.getParameters() with a direct equality/assertion that verifies both
contents and size (for example assertEquals(parameters, event.getParameters())
or assertEquals(parameters.size(), event.getParameters().size()) followed by
element-by-element comparison) so that extra parameters on event will fail;
update test in CustomErrorTest to use these corrected assertions referencing
event and parameters.

In `@abi/src/test/java/org/tron/trident/abi/datatypes/DynamicArrayTest.java`:
- Around line 38-50: The testMultidimensionalDynamicArray uses raw types for
DynamicArray which causes compiler warnings; update the declarations and
constructors to use fully parameterized nested generics (e.g.,
DynamicArray<DynamicArray<DynamicArray<Uint256>>> and inner
DynamicArray<DynamicArray<Uint256>> / DynamicArray<Uint256>) so the test remains
the same (assertEquals("uint256[][][]", array.getTypeAsString())) but is
type-safe; adjust the DynamicArray instantiations in the test to use these
generic types rather than raw DynamicArray.

In `@core/src/test/java/org/tron/trident/core/AbiV2ContractTest.java`:
- Around line 718-723: The test currently constructs a Function named `function`
but calls `client.triggerConstantContract(...)` with a hardcoded calldata
string; replace that raw hex with the encoded calldata produced from the
constructed Function by calling the encoder (e.g.
FunctionEncoder.encode(function)) and pass that encoded hex string into
`client.triggerConstantContract(testAddress, contractAddress, /* encoded
calldata */)` so the multi-dimensional array encoding path is exercised; keep
the same `testAddress` and `contractAddress` parameters and assert on the
returned `transactionExtention` as before.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 68ac664c-4ae0-44c1-a1cd-4072a00bbd04

📥 Commits

Reviewing files that changed from the base of the PR and between 45acc6a and 1997f15.

⛔ Files ignored due to path filters (1)
  • abi/src/main/java/org/tron/trident/abi/datatypes/generated/StaticArray0.java is excluded by !**/generated/**
📒 Files selected for processing (36)
  • abi/src/main/java/org/tron/trident/abi/CustomErrorEncoder.java
  • abi/src/main/java/org/tron/trident/abi/DefaultFunctionEncoder.java
  • abi/src/main/java/org/tron/trident/abi/DefaultFunctionReturnDecoder.java
  • abi/src/main/java/org/tron/trident/abi/EventEncoder.java
  • abi/src/main/java/org/tron/trident/abi/FunctionEncoder.java
  • abi/src/main/java/org/tron/trident/abi/FunctionReturnDecoder.java
  • abi/src/main/java/org/tron/trident/abi/TypeDecoder.java
  • abi/src/main/java/org/tron/trident/abi/TypeEncoder.java
  • abi/src/main/java/org/tron/trident/abi/TypeReference.java
  • abi/src/main/java/org/tron/trident/abi/Utils.java
  • abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java
  • abi/src/main/java/org/tron/trident/abi/datatypes/Array.java
  • abi/src/main/java/org/tron/trident/abi/datatypes/BytesType.java
  • abi/src/main/java/org/tron/trident/abi/datatypes/CustomError.java
  • abi/src/main/java/org/tron/trident/abi/datatypes/DynamicArray.java
  • abi/src/main/java/org/tron/trident/abi/datatypes/DynamicStruct.java
  • abi/src/main/java/org/tron/trident/abi/datatypes/Event.java
  • abi/src/main/java/org/tron/trident/abi/datatypes/Function.java
  • abi/src/main/java/org/tron/trident/abi/datatypes/StaticArray.java
  • abi/src/main/java/org/tron/trident/abi/datatypes/StaticStruct.java
  • abi/src/main/java/org/tron/trident/abi/datatypes/Utf8String.java
  • abi/src/main/java/org/tron/trident/abi/datatypes/primitive/Int.java
  • abi/src/main/java/org/tron/trident/abi/datatypes/reflection/Parameterized.java
  • abi/src/test/java/org/tron/trident/abi/AbiV2TestFixture.java
  • abi/src/test/java/org/tron/trident/abi/CustomErrorEncoderTest.java
  • abi/src/test/java/org/tron/trident/abi/DefaultFunctionEncoderTest.java
  • abi/src/test/java/org/tron/trident/abi/EventEncoderTest.java
  • abi/src/test/java/org/tron/trident/abi/FunctionReturnDecoderTest.java
  • abi/src/test/java/org/tron/trident/abi/TypeDecoderTest.java
  • abi/src/test/java/org/tron/trident/abi/TypeEncoderPackedTest.java
  • abi/src/test/java/org/tron/trident/abi/TypeEncoderTest.java
  • abi/src/test/java/org/tron/trident/abi/datatypes/CustomErrorTest.java
  • abi/src/test/java/org/tron/trident/abi/datatypes/DynamicArrayTest.java
  • abi/src/test/java/org/tron/trident/abi/datatypes/StaticArrayTest.java
  • abi/src/test/java/org/tron/trident/abi/datatypes/StaticStructTest.java
  • core/src/test/java/org/tron/trident/core/AbiV2ContractTest.java

Comment on lines +30 to 41
/**
* Returns the Bytes32 Padded length. If the string is empty, we only encode its length. Else,
* we concatenate its length along of its value
*/
@Override
public int bytes32PaddedLength() {
return 2 * MAX_BYTE_LENGTH;
if (value.isEmpty()) {
return MAX_BYTE_LENGTH;
} else {
return 2 * MAX_BYTE_LENGTH;
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find how bytes32PaddedLength is used to understand its contract
echo "=== Searching for bytes32PaddedLength usage ==="
rg -n "bytes32PaddedLength" --type=java -C3

echo "=== Check Type interface definition ==="
ast-grep --pattern 'interface Type {
  $$$
  bytes32PaddedLength($$$)
  $$$
}'

Repository: 0xbigapple/trident

Length of output: 6270


🏁 Script executed:

# Get the full Utf8String class
cat abi/src/main/java/org/tron/trident/abi/datatypes/Utf8String.java

# Also check BytesType for comparison
echo "=== BytesType implementation ==="
cat abi/src/main/java/org/tron/trident/abi/datatypes/BytesType.java

Repository: 0xbigapple/trident

Length of output: 3713


🏁 Script executed:

# Check how dynamic types are identified
rg -n "isDynamic" --type=java -B2 -A2 | head -100

Repository: 0xbigapple/trident

Length of output: 7202


🏁 Script executed:

# Look for how Utf8String is encoded/decoded to understand expected behavior
rg -n "class Utf8String" --type=java -A30

Repository: 0xbigapple/trident

Length of output: 3637


🏁 Script executed:

# Search for string encoding logic
rg -n "encodeString|encode.*Utf8|Utf8String" --type=java -B2 -A5 | head -80

# Check TypeEncoder.encode method to see how it handles Utf8String
rg -n "public static.*encode" abi/src/main/java/org/tron/trident/abi/TypeEncoder.java -A15

Repository: 0xbigapple/trident

Length of output: 9802


🏁 Script executed:

# Get the encodeString method implementation
rg -n "private static.*encodeString|private static String encodeString" abi/src/main/java/org/tron/trident/abi/TypeEncoder.java -A30

Repository: 0xbigapple/trident

Length of output: 44


🏁 Script executed:

# Also check if there's a test for string encoding
rg -n "testString|String.*encode|encodeString" abi/src/test/java/org/tron/trident/abi/TypeEncoderTest.java -B2 -A10 | head -100

Repository: 0xbigapple/trident

Length of output: 609


🏁 Script executed:

# Search for the actual encodeString method definition in TypeEncoder
rg -n "encodeString" abi/src/main/java/org/tron/trident/abi/TypeEncoder.java -A25

# Let's also check the line numbers mentioned in the test
sed -n '1180,1200p' abi/src/main/java/org/tron/trident/abi/TypeEncoderTest.java

Repository: 0xbigapple/trident

Length of output: 2377


🏁 Script executed:

# Get the encodeDynamicBytes method
rg -n "static.*encodeDynamicBytes|private.*encodeDynamicBytes" abi/src/main/java/org/tron/trident/abi/TypeEncoder.java -A30

Repository: 0xbigapple/trident

Length of output: 1240


🏁 Script executed:

# Find where bytes32PaddedLength is actually called in TypeEncoder
rg -n "\.bytes32PaddedLength\(\)" abi/src/main/java/org/tron/trident/abi/TypeEncoder.java -B5 -A5

Repository: 0xbigapple/trident

Length of output: 505


Remove or correct bytes32PaddedLength() in Utf8String—it does not reflect actual encoded size and is unused.

The method returns a fixed 64 bytes for all non-empty strings regardless of content length. However, this is misleading because:

  1. Utf8String is a dynamic type, so bytes32PaddedLength() is never called during actual encoding (see TypeEncoder.encodeString() and encodeDynamicBytes())
  2. For static types in structs, only non-dynamic types use bytes32PaddedLength() (line 281 in TypeEncoder)
  3. The method's Javadoc incorrectly describes the encoding contract for strings

Either remove this override (falling back to the default Type implementation returning MAX_BYTE_LENGTH), or clarify its purpose in the Javadoc if it's meant for a specific use case beyond encoding.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/datatypes/Utf8String.java` around
lines 30 - 41, The bytes32PaddedLength() override in Utf8String is misleading
and unused for dynamic strings; remove the override from class Utf8String so the
class falls back to the default Type.bytes32PaddedLength() behavior, or if you
intend a special-purpose behavior keep the method but update its Javadoc to
clearly state it is not used for ABI dynamic encoding and return a value that
correctly documents that special use; locate the method by name
(bytes32PaddedLength) in Utf8String and consider callers like
TypeEncoder.encodeString() and encodeDynamicBytes() and the base Type
implementation when making the change.

Comment on lines +988 to +995
value =
(T)
TypeDecoder.decodeStaticArray(
input, currOffset, staticReference, staticLength);
currOffset +=
((decodeUintAsInt(input, currOffset) / Type.MAX_BYTE_LENGTH) + 2)
* MAX_BYTE_LENGTH_FOR_HEX_STRING;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for tests that exercise nested static array decoding
rg -n "StaticArray.*StaticArray" --type=java -A5 -B5

Repository: 0xbigapple/trident

Length of output: 50374


🏁 Script executed:

sed -n '858,1026p' abi/src/main/java/org/tron/trident/abi/TypeDecoder.java | cat -n

Repository: 0xbigapple/trident

Length of output: 7888


🏁 Script executed:

rg -n "StaticArray.*Uint" abi/src/test/java/org/tron/trident/abi/TypeDecoderTest.java -A3 | grep -E "(new Uint|StaticArray)" | head -30

Repository: 0xbigapple/trident

Length of output: 843


🏁 Script executed:

rg -n "bytes3d\|bytes2d" abi/src/test/java/org/tron/trident/abi/TypeDecoderTest.java -B5 -A10 | head -80

Repository: 0xbigapple/trident

Length of output: 44


🏁 Script executed:

sed -n '1200,1250p' abi/src/test/java/org/tron/trident/abi/TypeDecoderTest.java

Repository: 0xbigapple/trident

Length of output: 2552


🏁 Script executed:

sed -n '1646,1665p' abi/src/test/java/org/tron/trident/abi/FunctionReturnDecoderTest.java

Repository: 0xbigapple/trident

Length of output: 1004


🏁 Script executed:

sed -n '85,140p' abi/src/main/java/org/tron/trident/abi/TypeDecoder.java | cat -n

Repository: 0xbigapple/trident

Length of output: 2776


🏁 Script executed:

sed -n '916,960p' abi/src/main/java/org/tron/trident/abi/TypeDecoder.java | cat -n

Repository: 0xbigapple/trident

Length of output: 2166


🏁 Script executed:

sed -n '927,975p' abi/src/main/java/org/tron/trident/abi/TypeDecoder.java | cat -n

Repository: 0xbigapple/trident

Length of output: 2396


🏁 Script executed:

sed -n '960,975p' abi/src/main/java/org/tron/trident/abi/TypeDecoder.java | cat -n

Repository: 0xbigapple/trident

Length of output: 720


🏁 Script executed:

sed -n '988,998p' abi/src/main/java/org/tron/trident/abi/TypeDecoder.java | cat -n

Repository: 0xbigapple/trident

Length of output: 575


🏁 Script executed:

rg -n "public static.*decodeStaticArray" abi/src/main/java/org/tron/trident/abi/TypeDecoder.java -A5

Repository: 0xbigapple/trident

Length of output: 44


🏁 Script executed:

rg -n "decodeStaticArray" abi/src/main/java/org/tron/trident/abi/TypeDecoder.java | head -5

Repository: 0xbigapple/trident

Length of output: 236


🏁 Script executed:

sed -n '356,430p' abi/src/main/java/org/tron/trident/abi/TypeDecoder.java | cat -n

Repository: 0xbigapple/trident

Length of output: 3483


🏁 Script executed:

rg -n "MAX_BYTE_LENGTH\s*=" abi/src/main/java/org/tron/trident/abi/TypeDecoder.java

Repository: 0xbigapple/trident

Length of output: 44


🏁 Script executed:

rg -n "MAX_BYTE_LENGTH" abi/src/main/java/org/tron/trident/abi/datatypes/Type.java

Repository: 0xbigapple/trident

Length of output: 139


🏁 Script executed:

sed -n '15,30p' abi/src/main/java/org/tron/trident/abi/datatypes/Type.java

Repository: 0xbigapple/trident

Length of output: 301


Incorrect offset calculation for nested static arrays treats them as dynamic types.

At lines 993-995, the offset calculation ((decodeUintAsInt(input, currOffset) / Type.MAX_BYTE_LENGTH) + 2) attempts to read and decode a length prefix from the input data. However, static arrays in ABI encoding do not have a length prefix—their size is known at compile time (already extracted as staticLength on lines 931-932). This logic is correct for dynamic arrays but incorrect for static arrays.

The calculation will produce correct results when test data contains small values (as in existing tests with values 1-6), but will fail with larger values coincidentally placed at those encoding positions. The offset should instead be calculated from staticLength and the actual element size.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/TypeDecoder.java` around lines 988 -
995, The offset after decoding a nested static array is being computed as if the
array were dynamic by reading a length prefix via decodeUintAsInt at currOffset;
instead, update the logic in TypeDecoder after the call to
TypeDecoder.decodeStaticArray to advance currOffset based on the known
staticLength and the element size (use staticLength and Type.MAX_BYTE_LENGTH /
element byte size logic) rather than reading a length from the input with
decodeUintAsInt; ensure you use MAX_BYTE_LENGTH_FOR_HEX_STRING and
Type.MAX_BYTE_LENGTH consistently to compute the exact number of hex chars
consumed for staticLength * elementSize so currOffset points to the next ABI
value.

Comment on lines 53 to 60
static boolean isDynamic(Type parameter) {
return parameter instanceof DynamicBytes
|| parameter instanceof Utf8String
|| parameter instanceof DynamicArray;
|| parameter instanceof DynamicArray
|| (parameter instanceof StaticArray
&& DynamicStruct.class.isAssignableFrom(
((StaticArray) parameter).getComponentType()));
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "TypeEncoder.java" -type f

Repository: 0xbigapple/trident

Length of output: 119


🏁 Script executed:

# Get the file and examine the structure
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs wc -l

Repository: 0xbigapple/trident

Length of output: 123


🏁 Script executed:

# Read the isDynamic method (lines 53-60)
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs sed -n '50,65p'

Repository: 0xbigapple/trident

Length of output: 617


🏁 Script executed:

# Read lines 76-82
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs sed -n '70,90p'

Repository: 0xbigapple/trident

Length of output: 1088


🏁 Script executed:

# Read lines 337-368 (encodeArrayValuesOffsets method context)
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs sed -n '330,380p'

Repository: 0xbigapple/trident

Length of output: 2545


🏁 Script executed:

# Also get broader context - look at encode method
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs sed -n '1,100p'

Repository: 0xbigapple/trident

Length of output: 4215


🏁 Script executed:

# Read encodeArrayValues method
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs sed -n '100,150p'

Repository: 0xbigapple/trident

Length of output: 2341


🏁 Script executed:

# Search for encodeArrayValues method definition
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs grep -n "encodeArrayValues" | head -10

Repository: 0xbigapple/trident

Length of output: 469


🏁 Script executed:

# Get encodeStaticArrayWithDynamicStruct method
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs sed -n '280,330p'

Repository: 0xbigapple/trident

Length of output: 2102


🏁 Script executed:

# Read more context around line 289 mentioned in original comment
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs sed -n '285,310p'

Repository: 0xbigapple/trident

Length of output: 1127


🏁 Script executed:

# Read the full encodeArrayValues method
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs sed -n '258,275p'

Repository: 0xbigapple/trident

Length of output: 642


🏁 Script executed:

# Check if there are any tests that cover these cases
find . -path "*test*" -name "*TypeEncoder*" -type f | head -5

Repository: 0xbigapple/trident

Length of output: 191


🏁 Script executed:

# Check the test file for coverage of StaticArray with dynamic elements
find . -path "*test*" -name "TypeEncoderTest.java" -type f | head -1 | xargs grep -n "StaticArray\|Utf8String\|DynamicBytes" | head -20

Repository: 0xbigapple/trident

Length of output: 1106


🏁 Script executed:

# Look for any test of string arrays or bytes arrays
find . -path "*test*" -name "TypeEncoderTest.java" -type f | head -1 | xargs grep -A 5 -B 2 "string\[" | head -30

Repository: 0xbigapple/trident

Length of output: 44


🏁 Script executed:

# Look for tests of StaticArray with specific types
find . -path "*test*" -name "TypeEncoderTest.java" -type f | head -1 | xargs grep -n "StaticArray2\|StaticArray3" | head -20

Repository: 0xbigapple/trident

Length of output: 360


🏁 Script executed:

# Get more context - check what the test does with StaticArray2 and Utf8String
find . -path "*test*" -name "TypeEncoderTest.java" -type f | head -1 | xargs sed -n '1190,1230p'

Repository: 0xbigapple/trident

Length of output: 1524


🏁 Script executed:

# Search for any test combining StaticArray with dynamic types
find . -path "*test*" -name "TypeEncoderTest.java" -type f | head -1 | xargs grep -B 5 -A 15 "StaticArray.*Utf8String\|StaticArray.*DynamicBytes"

Repository: 0xbigapple/trident

Length of output: 46


🏁 Script executed:

# Search more carefully for StaticArray tests with dynamic content
find . -path "*test*" -name "TypeEncoderTest.java" -type f | head -1 | xargs sed -n '1226,1350p'

Repository: 0xbigapple/trident

Length of output: 6806


🏁 Script executed:

# Let's understand the actual behavior - create a test case to verify the issue
# First, let's check what methods are available on StaticArray
find . -path "*datatypes*" -name "StaticArray.java" -type f

Repository: 0xbigapple/trident

Length of output: 129


🏁 Script executed:

# Let's verify the issue by examining what happens with a StaticArray of Utf8String
# Check the encode() method again to see how it routes StaticArray
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs sed -n '76,82p'

Repository: 0xbigapple/trident

Length of output: 381


🏁 Script executed:

# Now let's look at encodeStaticArrayWithDynamicStruct to understand the correct pattern
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs grep -A 30 "encodeStaticArrayWithDynamicStruct"

Repository: 0xbigapple/trident

Length of output: 2433


🏁 Script executed:

# Let's check the Solidity ABI specification requirements by looking at comments in the code
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs sed -n '320,340p'

Repository: 0xbigapple/trident

Length of output: 1199


🏁 Script executed:

# Check if isDynamic is used to determine encoding behavior elsewhere
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs grep -n "isDynamic"

Repository: 0xbigapple/trident

Length of output: 175


🏁 Script executed:

# Now let's test the actual issue - create a simple verification with what we know
# Search for any handling of StaticArray with Utf8String or DynamicBytes
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs grep -B 5 -A 10 "StaticArray\|encodeArrayValues\|isDynamic" | head -50

Repository: 0xbigapple/trident

Length of output: 2089


🏁 Script executed:

# Look at the full encodeDynamicsTypesArraysOffsets method
find . -path "*abi*" -name "TypeEncoder.java" -type f | head -1 | xargs sed -n '369,410p'

Repository: 0xbigapple/trident

Length of output: 1841


Generalize dynamic-array detection beyond DynamicStruct.

StaticArray is treated as dynamic only when its component type is DynamicStruct, causing string[2], bytes[2], uint256[][2], and similar arrays with dynamic element types to be encoded inline without offsets. This produces invalid ABI encoding. The offset-handling infrastructure (encodeDynamicsTypesArraysOffsets) is designed to handle any dynamic element type but is only applied to DynamicStruct arrays. Extend isDynamic() to return true for StaticArray with any dynamic component type (Utf8String, DynamicBytes, DynamicArray), and update the encode() routing logic accordingly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/TypeEncoder.java` around lines 53 -
60, The isDynamic(Type) method currently only treats a StaticArray as dynamic
when its component type is DynamicStruct; change it to detect any dynamic
component (DynamicBytes, Utf8String, DynamicArray, or any type that itself
returns true from isDynamic) so StaticArray with dynamic elements returns true;
then update the encode() routing so that arrays of those StaticArray types go
through encodeDynamicsTypesArraysOffsets (or the same dynamic-array offset
handling path) instead of inline encoding. Reference: isDynamic,
StaticArray.getComponentType(), DynamicBytes, Utf8String, DynamicArray,
DynamicStruct, encodeDynamicsTypesArraysOffsets, and encode().

Comment on lines +124 to +136
public static Constructor findStructConstructor(Class classType) {
return Arrays.stream(classType.getDeclaredConstructors())
.filter(
declaredConstructor ->
Arrays.stream(declaredConstructor.getParameterTypes())
.allMatch(Type.class::isAssignableFrom))
.findAny()
.orElseThrow(
() ->
new RuntimeException(
"TypeReferenced struct must contain "
+ "a constructor with types that extend Type"));
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Locate and examine the Utils.java file
find . -type f -name "Utils.java" | grep -E "abi.*src.*main"

Repository: 0xbigapple/trident

Length of output: 113


🏁 Script executed:

# Also search for the test file mentioned
find . -type f -name "AbiV2ContractTest.java"

Repository: 0xbigapple/trident

Length of output: 127


🏁 Script executed:

# Search for getStructType method to understand the dependency
rg "getStructType" --type java -A 5

Repository: 0xbigapple/trident

Length of output: 2845


🏁 Script executed:

# Check the actual findStructConstructor implementation and its usage
rg "findStructConstructor" --type java -B 2 -A 10

Repository: 0xbigapple/trident

Length of output: 7701


🏁 Script executed:

# Examine the test file to see the constructor patterns mentioned
sed -n '458,525p' ./core/src/test/java/org/tron/trident/core/AbiV2ContractTest.java

Repository: 0xbigapple/trident

Length of output: 2275


🏁 Script executed:

# Get more context around the test classes if needed
sed -n '400,600p' ./core/src/test/java/org/tron/trident/core/AbiV2ContractTest.java | head -200

Repository: 0xbigapple/trident

Length of output: 22194


🏁 Script executed:

# Check if there are any sorting or ordering mechanisms in findStructConstructor
cat -n ./abi/src/main/java/org/tron/trident/abi/Utils.java | sed -n '110,150p'

Repository: 0xbigapple/trident

Length of output: 1679


🏁 Script executed:

# Check the entire getStructType method
cat -n ./abi/src/main/java/org/tron/trident/abi/Utils.java | sed -n '100,180p'

Repository: 0xbigapple/trident

Length of output: 3577


🏁 Script executed:

# Check if there are any other usages of findStructConstructor that might shed light on the issue
rg "getDeclaredConstructors" --type java -B 3 -A 3

Repository: 0xbigapple/trident

Length of output: 678


🏁 Script executed:

# Look at TypeDecoder usage of findStructConstructor to confirm the impact
sed -n '1,100p' ./abi/src/main/java/org/tron/trident/abi/TypeDecoder.java | head -50

Repository: 0xbigapple/trident

Length of output: 2232


Prefer the most specific constructor matching the filter.

findAny() returns whichever matching constructor Java reflection provides first, which is undefined. The test classes (StaticInfo, DynamicInfo, NestedInfo) each define both a typed constructor (e.g., (Uint256, Address)) and a generic bridge ((Type, Type, ...)). If the bridge wins, getStructType() extracts Type parameters and generates malformed ABI tuples like (type,...) instead of canonical signatures. Select the constructor with the smallest parameter count or fewest raw Type.class parameters.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@abi/src/main/java/org/tron/trident/abi/Utils.java` around lines 124 - 136,
findStructConstructor currently uses Arrays.stream(...).findAny() which can pick
an arbitrary matching constructor; instead select the most specific constructor
(prefer the one with the smallest parameter count or the one with the fewest
parameters equal to raw Type.class) so typed constructors win over the generic
bridge. Modify findStructConstructor to stream declared constructors, filter by
allMatch(Type.class::isAssignableFrom) as before, then sort or reduce to pick
the constructor with minimal parameter length (and as a tiebreaker minimal count
of parameters exactly equal to Type.class) and return that or throw the same
RuntimeException if none found.

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

Successfully merging this pull request may close these issues.

1 participant