feat(abi): support abiV2 include static/dynamics arrays, struct#5
Open
0xbigapple wants to merge 4 commits intorelease_1.0.0from
Open
feat(abi): support abiV2 include static/dynamics arrays, struct#50xbigapple wants to merge 4 commits intorelease_1.0.0from
0xbigapple wants to merge 4 commits intorelease_1.0.0from
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR extends the abi module to support Solidity ABI v2 features (tuples/structs, nested static/dynamic arrays) and introduces packed encoding plus custom error signature encoding, with expanded unit test coverage.
Changes:
- Add ABI v2 tuple/struct + nested array type-string handling and decoding/encoding support (including empty static arrays via
StaticArray0). - Introduce
abi.encodePacked-style APIs (TypeEncoder.encodePacked,FunctionEncoder.encodeConstructorPacked) and custom errors (CustomError,CustomErrorEncoder). - Change
EventEncoder.buildEventSignature(...)/encode(...)to return topic hash without the0xprefix and update tests accordingly.
Reviewed changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| abi/src/test/java/org/tron/trident/abi/datatypes/StaticStructTest.java | Adds type-string test for static structs containing arrays. |
| abi/src/test/java/org/tron/trident/abi/datatypes/StaticArrayTest.java | Adds empty static array (StaticArray0) type-string test. |
| abi/src/test/java/org/tron/trident/abi/datatypes/DynamicArrayTest.java | Adds dynamic array type-string tests (empty, nested, structs). |
| abi/src/test/java/org/tron/trident/abi/datatypes/CustomErrorTest.java | Adds basic CustomError construction test. |
| abi/src/test/java/org/tron/trident/abi/TypeEncoderTest.java | Adds ABI v2 encoding tests for arrays/structs and dynamic bytes in structs. |
| abi/src/test/java/org/tron/trident/abi/TypeEncoderPackedTest.java | Adds packed-encoding test coverage across many ABI types. |
| abi/src/test/java/org/tron/trident/abi/TypeDecoderTest.java | Updates/extends decoding tests for nested arrays and structs. |
| abi/src/test/java/org/tron/trident/abi/EventEncoderTest.java | Updates expected event topic hashes (no 0x) + adds complex signature tests. |
| abi/src/test/java/org/tron/trident/abi/DefaultFunctionEncoderTest.java | Adds ABI v2 signature + packed-constructor encoding tests. |
| abi/src/test/java/org/tron/trident/abi/CustomErrorEncoderTest.java | Adds custom error signature hashing and signature-building tests. |
| abi/src/test/java/org/tron/trident/abi/AbiV2TestFixture.java | Expands fixture types/functions/events used by ABI v2 tests. |
| abi/src/main/java/org/tron/trident/abi/datatypes/reflection/Parameterized.java | Adds annotation to preserve erased generic element types for reflection. |
| abi/src/main/java/org/tron/trident/abi/datatypes/primitive/Int.java | Adjusts primitive Int generic parameter type. |
| abi/src/main/java/org/tron/trident/abi/datatypes/generated/StaticArray0.java | Introduces zero-length static array type. |
| abi/src/main/java/org/tron/trident/abi/datatypes/Utf8String.java | Alters padded-length calculation for strings. |
| abi/src/main/java/org/tron/trident/abi/datatypes/StaticStruct.java | Updates tuple type-string rendering to handle nested arrays/structs. |
| abi/src/main/java/org/tron/trident/abi/datatypes/StaticArray.java | Updates component-type inference + type-string rendering for arrays/struct elements. |
| abi/src/main/java/org/tron/trident/abi/datatypes/Function.java | Uses static import for Utils.convert(...). |
| abi/src/main/java/org/tron/trident/abi/datatypes/Event.java | Uses static import for Utils.convert(...). |
| abi/src/main/java/org/tron/trident/abi/datatypes/DynamicStruct.java | Updates tuple type-string rendering for nested arrays/structs. |
| abi/src/main/java/org/tron/trident/abi/datatypes/DynamicArray.java | Updates component-type inference + type-string rendering (including empty arrays). |
| abi/src/main/java/org/tron/trident/abi/datatypes/CustomError.java | Adds custom error wrapper type. |
| abi/src/main/java/org/tron/trident/abi/datatypes/BytesType.java | Alters padded-length calculation for bytes types. |
| abi/src/main/java/org/tron/trident/abi/datatypes/Array.java | Adds getNativeValueCopy() helper. |
| abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java | Extends type resolution + clarifies type-string behavior for structs. |
| abi/src/main/java/org/tron/trident/abi/Utils.java | Adds reflection helpers for struct signatures, constructor discovery, nested field flattening. |
| abi/src/main/java/org/tron/trident/abi/TypeReference.java | Adds innerTypes support and improves type-name resolution for older Java. |
| abi/src/main/java/org/tron/trident/abi/TypeEncoder.java | Adds dynamic-type detection improvements + packed encoding + array/struct encoding adjustments. |
| abi/src/main/java/org/tron/trident/abi/TypeDecoder.java | Major ABI v2 decoding updates for structs/arrays (including nested and innerTypes). |
| abi/src/main/java/org/tron/trident/abi/FunctionReturnDecoder.java | Refactors decoder initialization; adds convenience decode methods. |
| abi/src/main/java/org/tron/trident/abi/FunctionEncoder.java | Refactors encoder initialization; adds encode(methodId, params) + packed constructor encoding API. |
| abi/src/main/java/org/tron/trident/abi/EventEncoder.java | Removes 0x prefix from event signature hashes. |
| abi/src/main/java/org/tron/trident/abi/DefaultFunctionReturnDecoder.java | Updates offset computation for ABI v2 structs/arrays. |
| abi/src/main/java/org/tron/trident/abi/DefaultFunctionEncoder.java | Adds encodeWithSelector + packed parameter encoding + ABI v2 tuple-head sizing changes. |
| abi/src/main/java/org/tron/trident/abi/CustomErrorEncoder.java | Adds custom error signature building + hashing. |
Comments suppressed due to low confidence (1)
abi/src/main/java/org/tron/trident/abi/TypeEncoder.java:379
- encodeArrayValuesOffsets() computes Utf8String element sizes using
((String) ...).length(), which counts UTF-16 code units, not UTF-8 bytes. Offsets will be wrong for non-ASCII strings. Use the UTF-8 byte length (e.g.,getBytes(UTF_8).length) when computing words/padding for string elements.
int bytesLength =
arrayOfBytes
? ((byte[]) value.getValue().get(i - 1).getValue()).length
: ((String) value.getValue().get(i - 1).getValue()).length();
int numberOfWords = (bytesLength + MAX_BYTE_LENGTH - 1) / MAX_BYTE_LENGTH;
int totalBytesLength = numberOfWords * MAX_BYTE_LENGTH;
offset += totalBytesLength + MAX_BYTE_LENGTH;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+439
to
+450
| if (isSupportingEncodedPacked(values)) { | ||
| if (values.getValue().isEmpty()) { | ||
| return ""; | ||
| } | ||
| if (values instanceof DynamicArray) { | ||
| return encode(values).substring(64); | ||
| } else if (values instanceof StaticArray) { | ||
| return encode(values); | ||
| } | ||
| } | ||
| throw new UnsupportedOperationException( | ||
| "Type cannot be packed encoded: " + values.getClass()); |
There was a problem hiding this comment.
arrayEncodePacked() currently derives packed array encoding by calling encode(values) and stripping the dynamic array length (or returning the static array encoding as-is). This keeps 32-byte padding per element, which is inconsistent with encodePacked(Type) for scalar numeric/address/bool types (which removes padding based on bit size) and does not implement tight packing for arrays. Consider implementing packed array encoding as concatenation of encodePacked(element) for each element (and for DynamicArray, omitting the length prefix) to match the method’s documented behavior.
Suggested change
| if (isSupportingEncodedPacked(values)) { | |
| if (values.getValue().isEmpty()) { | |
| return ""; | |
| } | |
| if (values instanceof DynamicArray) { | |
| return encode(values).substring(64); | |
| } else if (values instanceof StaticArray) { | |
| return encode(values); | |
| } | |
| } | |
| throw new UnsupportedOperationException( | |
| "Type cannot be packed encoded: " + values.getClass()); | |
| if (!isSupportingEncodedPacked(values)) { | |
| throw new UnsupportedOperationException( | |
| "Type cannot be packed encoded: " + values.getClass()); | |
| } | |
| if (values.getValue().isEmpty()) { | |
| return ""; | |
| } | |
| StringBuilder result = new StringBuilder(); | |
| for (T element : values.getValue()) { | |
| result.append(encodePacked(element)); | |
| } | |
| return result.toString(); |
Comment on lines
+30
to
+40
| /** | ||
| * 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; | ||
| } |
There was a problem hiding this comment.
Utf8String.bytes32PaddedLength() currently returns 32 for empty strings and 64 for all non-empty strings, which is incorrect for strings longer than 32 bytes (and can break offset calculations for ABI v2). bytes32PaddedLength should be 32 (length word) + ceil(utf8ByteLen/32)*32, computed from the UTF-8 byte length.
Comment on lines
+33
to
+38
| if (value.length < MAX_BYTE_LENGTH) { | ||
| return MAX_BYTE_LENGTH; | ||
| } else if (value.length % MAX_BYTE_LENGTH == 0) { | ||
| return value.length; | ||
| } | ||
| return (value.length / MAX_BYTE_LENGTH + 1) * MAX_BYTE_LENGTH; |
There was a problem hiding this comment.
BytesType.bytes32PaddedLength() now returns the padded data length only, but DynamicBytes inherits this and needs to account for the 32-byte length word as well (and fixed-size BytesN types should always be 32 bytes). As-is, bytes32PaddedLength for DynamicBytes is too small and will miscompute offsets/lengths. Consider making BytesType return 32 for fixed bytes, and overriding bytes32PaddedLength in DynamicBytes (or branching on type == "bytes") to return 32 + ceil(len/32)*32.
Suggested change
| if (value.length < MAX_BYTE_LENGTH) { | |
| return MAX_BYTE_LENGTH; | |
| } else if (value.length % MAX_BYTE_LENGTH == 0) { | |
| return value.length; | |
| } | |
| return (value.length / MAX_BYTE_LENGTH + 1) * MAX_BYTE_LENGTH; | |
| // Dynamic bytes ("bytes") are encoded as a 32-byte length word followed by | |
| // the data right-padded to a multiple of 32 bytes. | |
| if ("bytes".equals(type)) { | |
| int dataLength = value != null ? value.length : 0; | |
| int paddedDataLength = | |
| ((dataLength + MAX_BYTE_LENGTH - 1) / MAX_BYTE_LENGTH) * MAX_BYTE_LENGTH; | |
| return MAX_BYTE_LENGTH + paddedDataLength; | |
| } | |
| // Fixed-size bytesN types are always encoded into a single 32-byte word. | |
| return MAX_BYTE_LENGTH; |
Comment on lines
+501
to
+503
| if (classType.isAssignableFrom(DynamicStruct.class)) { | ||
| return (T) new DynamicStruct((List<Type>) parameters); | ||
| } else if (classType.isAssignableFrom(StaticStruct.class)) { |
There was a problem hiding this comment.
TypeDecoder.instantiateStruct() uses reversed assignability checks: classType.isAssignableFrom(DynamicStruct.class) / StaticStruct.class. For user-defined structs that extend DynamicStruct/StaticStruct this will be false, so the special-casing never triggers. This should be DynamicStruct.class.isAssignableFrom(classType) (and similarly for StaticStruct) so subclasses are handled correctly.
Suggested change
| if (classType.isAssignableFrom(DynamicStruct.class)) { | |
| return (T) new DynamicStruct((List<Type>) parameters); | |
| } else if (classType.isAssignableFrom(StaticStruct.class)) { | |
| if (DynamicStruct.class.isAssignableFrom(classType)) { | |
| return (T) new DynamicStruct((List<Type>) parameters); | |
| } else if (StaticStruct.class.isAssignableFrom(classType)) { |
Comment on lines
+543
to
+545
| if (typeReference.getClassType().isAssignableFrom(DynamicStruct.class) | ||
| && typeReference.getInnerTypes() != null) { | ||
| return decodeDynamicStructElementsFromInnerTypes( |
There was a problem hiding this comment.
decodeDynamicStruct() has the same reversed assignability check: typeReference.getClassType().isAssignableFrom(DynamicStruct.class) will be false for structs that extend DynamicStruct, so the innerTypes-based decoding path is skipped even when innerTypes are provided. Flip the check to DynamicStruct.class.isAssignableFrom(typeReference.getClassType()).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (16)
abi/src/main/java/org/tron/trident/abi/datatypes/primitive/Int.java (1)
19-19: Consider using the simple nameIntegerinstead of fully qualifiedjava.lang.Integer.The fully qualified name is functionally identical but unnecessarily verbose. Java convention prefers the simple name when there's no ambiguity, since
java.langis implicitly imported.♻️ Simplify to idiomatic Java style
-public final class Int extends Number<java.lang.Integer> { +public final class Int extends Number<Integer> {🤖 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/primitive/Int.java` at line 19, Replace the fully-qualified type java.lang.Integer with the simple name Integer in the generic declaration of the Int class (the declaration "public final class Int extends Number<java.lang.Integer>"). Update the declaration to use Integer so it follows Java conventions and removes unnecessary verbosity while keeping the same semantics.abi/src/main/java/org/tron/trident/abi/datatypes/DynamicStruct.java (1)
55-62: Consider simplifying the conditional branches.Both the
StructType/DynamicArraybranch and theArraybranch callgetValue().get(i).getTypeAsString(). These could be combined:♻️ Optional 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 {Since
DynamicArrayextendsArray, checkingArray.class.isAssignableFrom(cls)would cover both cases.🤖 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 - 62, The three-branch conditional in DynamicStruct (checking StructType.class, DynamicArray.class, and Array.class) is redundant because DynamicArray is an Array; simplify by first checking Array.class.isAssignableFrom(cls) (which will cover DynamicArray and Array) and calling type.append(getValue().get(i).getTypeAsString()), else fall back to type.append(AbiTypes.getTypeAString(cls)); update the conditional around cls, getValue().get(i).getTypeAsString(), and AbiTypes.getTypeAString to remove the duplicated branch.abi/src/test/java/org/tron/trident/abi/datatypes/StaticStructTest.java (1)
23-31: Make the expected ABI string literal explicit.Lines 24-30 build
expectedfromgetTypeAsString()on the component values. IfAddressorStaticArray4rendering regresses in the same way asStaticStruct, this test still passes. Hardcode the ABI literal so the assertion stays independent.🧪 Suggested change
- // (address,address,uint256[4]) - String expected = - "(" - + address1.getTypeAsString() - + "," - + address2.getTypeAsString() - + "," - + array4.getTypeAsString() - + ")"; + String expected = "(address,address,uint256[4])";🤖 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/StaticStructTest.java` around lines 23 - 31, Replace the dynamically-built `expected` string in StaticStructTest with a hardcoded ABI literal so the test no longer depends on component `getTypeAsString()` implementations: instead of concatenating address1.getTypeAsString(), address2.getTypeAsString(), and array4.getTypeAsString(), set `expected` to the explicit ABI string "(address,address,uint256[4])" (refer to the `expected` variable in StaticStructTest and the Address/StaticArray4 component types to locate the code).abi/src/test/java/org/tron/trident/abi/FunctionReturnDecoderTest.java (1)
1666-1707: Consider removing or tracking the commented-out test.This commented-out test for multi-dimensional dynamic array decoding should either be removed if no longer needed, or have a tracking issue/TODO with context explaining why it's disabled and when it should be re-enabled.
🤖 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/FunctionReturnDecoderTest.java` around lines 1666 - 1707, The test method testDecodeMultiDimDynamicArrayFunction in FunctionReturnDecoderTest is commented out; either remove it or add a TODO/ticket reference explaining why it's disabled and when to re-enable it—update the file by deleting the commented block if obsolete, or replace the comment with a short "// TODO: <tracking-issue-id> - reason and re-enable criteria" referencing the related fixture AbiV2TestFixture.setGetMultiDimDynamicArrayFunction and the decoding logic in FunctionReturnDecoder so future readers know the intent.abi/src/test/java/org/tron/trident/abi/CustomErrorEncoderTest.java (1)
56-77: Test methods have inconsistent visibility modifiers.Tests
testBuildErrorSignatureWithDynamicStructsandtestBuildErrorSignatureWithDynamicArraysuse package-private visibility (void) while other tests in this class usepublic void. Consider making visibility consistent.♻️ Proposed fix for consistency
`@Test` - void testBuildErrorSignatureWithDynamicStructs() { + public void testBuildErrorSignatureWithDynamicStructs() { ... } `@Test` - void testBuildErrorSignatureWithDynamicArrays() { + public void testBuildErrorSignatureWithDynamicArrays() { ... }🤖 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/CustomErrorEncoderTest.java` around lines 56 - 77, Two test methods, testBuildErrorSignatureWithDynamicStructs and testBuildErrorSignatureWithDynamicArrays, use package-private visibility while the rest of the tests are public; update their method signatures in CustomErrorEncoderTest (the two methods named exactly) to be public void so visibility is consistent with other tests and the test framework picks them up consistently.abi/src/main/java/org/tron/trident/abi/datatypes/DynamicArray.java (1)
84-90: Consider consolidating duplicate conditional branches.Lines 84-87 and 86-87 both return
value.get(0).getTypeAsString()forStructTypeandArraycases. These can be merged into a single condition.♻️ Proposed simplification
} else { - if (StructType.class.isAssignableFrom(value.get(0).getClass())) { - type = value.get(0).getTypeAsString(); - } else if (Array.class.isAssignableFrom(value.get(0).getClass())) { + if (StructType.class.isAssignableFrom(value.get(0).getClass()) + || Array.class.isAssignableFrom(value.get(0).getClass())) { type = value.get(0).getTypeAsString(); } else { type = AbiTypes.getTypeAString(getComponentType()); } }🤖 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 84 - 90, In DynamicArray, consolidate the duplicate branch that returns value.get(0).getTypeAsString() by combining the StructType and Array checks into a single condition (e.g., if (StructType.class.isAssignableFrom(...) || Array.class.isAssignableFrom(...))) so both cases use the same return; keep the existing else branch that falls back to AbiTypes.getTypeAString(getComponentType()) and ensure you reference value.get(0).getTypeAsString() and getComponentType() exactly as in the current code.abi/src/test/java/org/tron/trident/abi/datatypes/DynamicArrayTest.java (1)
38-50: Suppress raw type warnings or use proper generics.Line 40-41 uses raw
DynamicArraytype which will generate unchecked/raw type compiler warnings.♻️ Proposed fix to add suppression
`@Test` + `@SuppressWarnings`("unchecked") public void testMultidimensionalDynamicArray() { - DynamicArray<DynamicArray> array = + DynamicArray<DynamicArray<DynamicArray<Uint256>>> array = new DynamicArray<>( - DynamicArray.class, + (Class<DynamicArray<DynamicArray<Uint256>>>) (Class<?>) DynamicArray.class,Or simply add
@SuppressWarnings("rawtypes")to the test method.🤖 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 test method testMultidimensionalDynamicArray uses raw DynamicArray types causing unchecked/raw-type warnings; fix by using proper generics (e.g., parameterize each level as DynamicArray<DynamicArray<Uint256>> and ultimately DynamicArray<DynamicArray<DynamicArray<Uint256>>> when constructing the nested arrays) so all DynamicArray instances are fully generic, or if you prefer to keep the current shape, add `@SuppressWarnings`("rawtypes") to the test method declaration to silence the warning; update the references to DynamicArray and Uint256 in the method accordingly.core/src/test/java/org/tron/trident/core/AbiV2ContractTest.java (3)
492-493: Unchecked cast warning for generic DynamicArray.The cast
(DynamicArray<Uint256>) scoresis unchecked. While this is common with generic types in Java, consider adding@SuppressWarnings("unchecked")to the constructor if this generates compiler warnings.🤖 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 492 - 493, The cast to (DynamicArray<Uint256>) in AbiV2ContractTest (the assignment to this.scores) produces an unchecked cast warning; suppress it by annotating the enclosing constructor (or the smallest enclosing method) with `@SuppressWarnings`("unchecked") so the compiler warning is silenced, keeping the cast as-is and limiting the annotation scope to that constructor/method.
618-619: Remove debug print statements.These
System.out.printlnstatements appear to be debug artifacts and should be removed for cleaner test output.♻️ Suggested fix
assertEquals(1, result.getConstantResultCount()); - System.out.println(FunctionEncoder.encode(function)); - System.out.println(ByteArray.toHexString(result.getConstantResult(0).toByteArray())); List<Type> outputStructs🤖 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 618 - 619, Remove the two debug print statements in the test: delete the System.out.println(FunctionEncoder.encode(function)) and System.out.println(ByteArray.toHexString(result.getConstantResult(0).toByteArray())) lines in AbiV2ContractTest (they are only printing FunctionEncoder.encode(function) and ByteArray.toHexString(result.getConstantResult(0).toByteArray())). Leave the rest of the test intact so assertions/assert logging remain; run tests to confirm no behavioral changes.
677-681: Fragile manual offset prepending for nested struct decoding.The manual prepending of
"0000...0020"to simulate an offset pointer is fragile and may hide issues with the actual decoder. Consider investigating why the decoder requires this workaround or documenting this as expected behavior for the specific contract function output format.🤖 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 677 - 681, The test currently fakes a 32-byte offset by string-prepending ("0000...0020") to inputRaw before calling FunctionReturnDecoder.decode, which is fragile; instead construct the correct ABI-encoded payload by programmatically computing a 32-byte head containing the offset (0x20) and concatenating it with the actual bytes from result.getConstantResult(0) (use ByteArray.toHexString/ByteArray.concat or equivalent) and pass that full byte array into FunctionReturnDecoder.decode for the NestedInfo TypeReference, or if the decoder should accept the raw constant result bytes, remove the manual prepend and update the test to call FunctionReturnDecoder.decode directly with result.getConstantResult(0) and document the expected behavior.abi/src/main/java/org/tron/trident/abi/TypeEncoder.java (1)
149-175: Consider handlingFixedPointTypeexplicitly inremovePadding.At lines 151-153,
UfixedandFixedreturn the full encoded value without padding removal, which is correct since fixed-point types aren't well-supported for packed encoding. However, if someone passes anotherFixedPointTypesubclass, it would fall through to theUnsupportedOperationException. This is fine given the current type hierarchy, but consider usingFixedPointTypein the check for future-proofing:♻️ Suggested improvement
static String removePadding(String encodedValue, Type parameter) { if (parameter instanceof NumericType) { - if (parameter instanceof Ufixed || parameter instanceof Fixed) { + if (parameter instanceof FixedPointType) { return encodedValue; }🤖 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 149 - 175, The removePadding method currently checks for Ufixed and Fixed specifically and returns the full encodedValue for those, but other FixedPointType subclasses would fall through to an exception; update the type check in removePadding to use FixedPointType (or include it alongside Ufixed/Fixed) when deciding to return encodedValue so all fixed-point subclasses are handled uniformly; locate the removePadding method and replace the existing Ufixed/Fixed instanceof branch with a FixedPointType check (or add instanceof FixedPointType) to future-proof the logic.abi/src/main/java/org/tron/trident/abi/FunctionReturnDecoder.java (1)
66-82: Consider using parameterized type foroutputParameterslist.Lines 67 and 76 use raw
ArrayListwhich triggers unchecked warnings. While functional, using the parameterized type improves type safety.♻️ Suggested improvement
public static byte[] decodeDynamicBytes(String rawInput) { - List outputParameters = new ArrayList<TypeReference<Type>>(); + List<TypeReference<Type>> outputParameters = new ArrayList<>(); outputParameters.add(new TypeReference<DynamicBytes>() {}); ... } public static String decodeAddress(String rawInput) { - List outputParameters = new ArrayList<TypeReference<Type>>(); + List<TypeReference<Type>> outputParameters = new ArrayList<>(); outputParameters.add(new TypeReference<Address>() {}); ... }🤖 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 - 82, The outputParameters list in decodeDynamicBytes and decodeAddress is declared with a raw ArrayList causing unchecked warnings; change its declaration to use a parameterized type (e.g., List<TypeReference<Type>> outputParameters = new ArrayList<>()) and ensure the added TypeReference instances (new TypeReference<DynamicBytes>() {} and new TypeReference<Address>() {}) match that generic so decoder.decodeFunctionResult(rawInput, outputParameters) receives a properly typed List; update both methods (decodeDynamicBytes and decodeAddress) accordingly.abi/src/main/java/org/tron/trident/abi/Utils.java (3)
300-317: Minor: Field ordering in flattened list may not match declaration order.The method collects canonical (non-StaticStruct) fields first, then appends nested fields. This may not preserve declaration order (e.g., for
struct { int a; Nested n; int b; }, result would be[a, b, x]instead of[a, x, b]).This appears acceptable since the method is used for counting total fields (for size calculations), not for ordered field access during encoding/decoding. If ordered iteration is ever needed, this would need revision.
🤖 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 300 - 317, The current staticStructsNestedFieldsFlatList method builds canonicalFields then nestedFields separately which breaks original declaration order; change the algorithm to iterate classType.getDeclaredFields() in declaration order and for each Field if StaticStruct.class.isAssignableFrom(field.getType()) recursively call staticStructsNestedFieldsFlatList((Class<Type>) field.getType()) and append its returned list in-place, otherwise append the Field itself — this preserves the original declaration order while keeping the same recursion and return type; update references in the method to use a single List accumulator instead of separate canonicalFields/nestedFields.
319-341: Avoid catchingThrowable.Line 335 catches
Throwablewhich includesErrortypes (likeOutOfMemoryError) that should generally propagate. Consider catchingExceptioninstead if backward compatibility is truly needed.♻️ Suggested fix
} - } catch (Throwable e) { + } catch (Exception e) { /*FALLTHRU*/ }🤖 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 319 - 341, The getClassName method is catching Throwable (which can swallow Errors like OutOfMemoryError); change the catch to a narrower type (e.g., catch Exception) so only recoverable exceptions are handled; update the try/catch around the array-handling block in getClassName (variables cl, dimensions, sb) to catch Exception (or specific reflective exceptions) and let serious Errors propagate.
215-227: PotentialClassCastExceptionif type structure doesn't match expectations.The method assumes the type argument at index 0 is a
ParameterizedType. If the type structure is different (e.g., raw type or unexpected nesting), aClassCastExceptionwill occur at line 225.Consider adding a type check or documenting the expected input constraints.
🤖 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 215 - 227, The getFullParameterizedTypeFromArray method currently casts nested type arguments to ParameterizedType without checks, which can throw ClassCastException for raw or unexpected type structures; update getFullParameterizedTypeFromArray to validate each cast using instanceof before casting (check that type is a ParameterizedType and that the inner typeArgument is a ParameterizedType), handle raw/class cases (e.g., if inner typeArgument is a Class, extract its name directly) and throw a clear IllegalArgumentException or ClassNotFoundException with a descriptive message when the structure is unsupported so callers of getFullParameterizedTypeFromArray receive a deterministic error instead of a ClassCastException.abi/src/main/java/org/tron/trident/abi/TypeDecoder.java (1)
858-1029: Complex but well-structured array element decoding.The rewrite correctly handles:
- Struct types (both static and dynamic) with proper offset tracking
- Nested arrays (both static and dynamic) with recursive decoding
- Primitive types with dynamic/static distinction
One observation: Lines 872-875 and 899-902 use identical
Optionalchains to check forinnerTypes. Consider extracting this to a helper method for clarity.♻️ Optional: Extract repeated Optional chain to helper
private static boolean hasSubTypeInnerTypes(TypeReference<?> typeReference) { return Optional.ofNullable(typeReference) .map(TypeReference::getSubTypeReference) .map(TypeReference::getInnerTypes) .isPresent(); }🤖 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 858 - 1029, The repeated Optional.ofNullable(typeReference).map(x -> x.getSubTypeReference()).map(x -> x.getInnerTypes()).isPresent() logic inside TypeDecoder.decodeArrayElements (used in the DynamicStruct branch and the StaticStruct branch) should be extracted into a helper like hasSubTypeInnerTypes(TypeReference<?>), then replace both identical chains with a call to that helper to improve readability and avoid duplication; update references inside decodeArrayElements to call hasSubTypeInnerTypes(typeReference) where the checks occur.
🤖 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
encoding via errorSignature.getBytes(); update
CustomErrorEncoder.calculateSignatureHash to explicitly encode the
errorSignature as UTF-8 before hashing (use a UTF-8 byte conversion), then pass
that byte[] to Hash.sha3 and return the hex substring as before; reference
calculateSignatureHash, Hash.sha3, and Numeric.toHexString when locating the
change.
In `@abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java`:
- Around line 370-377: Replace the unchecked Class.forName(type) in the AbiTypes
fallback with a non-initializing lookup and subclass validation: call
Class.forName(type, false,
Thread.currentThread().getContextClassLoader()).asSubclass(Type.class) and
return that result; update the exception handling to catch
ClassNotFoundException and ClassCastException (or IllegalArgumentException) and
rethrow the same UnsupportedOperationException("Unsupported type encountered: "
+ type) so only classes that actually implement Type are accepted.
In `@abi/src/main/java/org/tron/trident/abi/datatypes/Array.java`:
- Around line 74-79: The getNativeValueCopy() method in Array<T> currently
unwraps only the outer Type and is therefore shallow; update it to deep-unwrap
nested ABI composites by detecting composite element types and calling their
native-unwrapping methods instead of t.getValue() directly: for each element t
in value, if t is an Array (e.g. instanceof Array) call
((Array<?>)t).getNativeValueCopy(), if t is a struct/composite call that
composite's native-unwrap method (e.g. Struct.getNativeValueCopy() or
equivalent), otherwise fall back to t.getValue(); ensure you reference the Array
class and the getNativeValueCopy() method in your changes and add any needed
imports.
In `@abi/src/main/java/org/tron/trident/abi/datatypes/DynamicArray.java`:
- Around line 38-45: The DynamicArray(List<T> values) constructor currently
dereferences values.get(0) which throws IndexOutOfBoundsException for an empty
list; update the constructor (DynamicArray(List<T> values)) to validate values
is non-empty (e.g., if (values == null || values.isEmpty()) throw new
IllegalArgumentException("values must not be empty")) before using values.get(0)
so callers get a clear, documented error instead of an OOB exception.
- Around line 27-34: The DynamicArray(T... values) constructor dereferences
values[0] which will throw ArrayIndexOutOfBoundsException for empty varargs; add
a guard at the start of the constructor that checks if values == null ||
values.length == 0 and handle it explicitly (e.g., throw a clear
IllegalArgumentException stating empty varargs are not allowed or delegate to an
existing constructor that accepts an explicit element Class), then proceed to
compute the element class as currently done using values[0] and
AbiTypes.getType(values[0].getTypeAsString()).
In `@abi/src/main/java/org/tron/trident/abi/datatypes/StaticArray.java`:
- Around line 87-95: The current logic builds the element type from the first
element in value or falls back to AbiTypes.getTypeAString(getComponentType())
when value is empty, which yields incorrect ABI strings for empty composite
arrays; change the fallback to inspect getComponentType(): if getComponentType()
is a StructType use its getTypeAsString(), if it's an Array use its
getTypeAsString(), otherwise use AbiTypes.getTypeAString(getComponentType());
update the branch in StaticArray (the block referencing value,
getComponentType(), StructType, Array, and AbiTypes.getTypeAString) so
zero-length composite arrays produce correct tuple or nested-array type syntax
before appending "[" + value.size() + "]".
- Around line 51-56: The constructor currently calls values.get(0) unguarded,
which throws for empty lists; fix by computing the element class before calling
super with a guard: if values.isEmpty() then derive the element class via
AbiTypes.getType(...) for the element type (fallback path) otherwise use the
existing logic that checks
StructType.class.isAssignableFrom(values.get(0).getClass()) ||
Array.class.isAssignableFrom(values.get(0).getClass()) ? (Class<T>)
values.get(0).getClass() : (Class<T>)
AbiTypes.getType(values.get(0).getTypeAsString()); then pass that computed
Class<T> and values into super instead of directly calling values.get(0) in the
super call.
In `@abi/src/main/java/org/tron/trident/abi/TypeReference.java`:
- Around line 76-80: The getInnerTypes() method can return null because the
field innerTypes is only set by the constructor that accepts inner types; update
the class so callers don't have to handle null by either initializing innerTypes
to an empty List by default (e.g., new ArrayList<>() in the field declaration or
in all constructors) or annotate and document the nullable contract (add
`@Nullable` to getInnerTypes and Javadoc explaining null return). Locate the
innerTypes field and all constructors (including the one that currently sets
innerTypes) and ensure every constructor initializes innerTypes or add the
`@Nullable` annotation and Javadoc to getInnerTypes to make the contract explicit.
In `@abi/src/main/java/org/tron/trident/abi/Utils.java`:
- Around line 71-78: Remove the unnecessary NoSuchMethodError handling in
Utils.getTypeName: simplify the method to directly return type.getTypeName() and
delete the try/catch and fallback to getClassName((Class) type), since the
project targets Java 8+ where java.lang.reflect.Type.getTypeName() is
guaranteed; update the method signature body for
getTypeName(java.lang.reflect.Type type) accordingly and remove any unused
imports if they become redundant.
In `@abi/src/test/java/org/tron/trident/abi/datatypes/CustomErrorTest.java`:
- Around line 23-26: The test currently only iterates and compares a prefix of
event.getParameters(), so add an explicit size equality assertion (compare
parameters.size() with event.getParameters().size() or assertEquals on those)
before the loop in CustomErrorTest so the full parameter list is validated; keep
the existing iterator/loop (using expectedParameter and event.getParameters())
but fail fast if lengths differ.
---
Nitpick comments:
In `@abi/src/main/java/org/tron/trident/abi/datatypes/DynamicArray.java`:
- Around line 84-90: In DynamicArray, consolidate the duplicate branch that
returns value.get(0).getTypeAsString() by combining the StructType and Array
checks into a single condition (e.g., if (StructType.class.isAssignableFrom(...)
|| Array.class.isAssignableFrom(...))) so both cases use the same return; keep
the existing else branch that falls back to
AbiTypes.getTypeAString(getComponentType()) and ensure you reference
value.get(0).getTypeAsString() and getComponentType() exactly as in the current
code.
In `@abi/src/main/java/org/tron/trident/abi/datatypes/DynamicStruct.java`:
- Around line 55-62: The three-branch conditional in DynamicStruct (checking
StructType.class, DynamicArray.class, and Array.class) is redundant because
DynamicArray is an Array; simplify by first checking
Array.class.isAssignableFrom(cls) (which will cover DynamicArray and Array) and
calling type.append(getValue().get(i).getTypeAsString()), else fall back to
type.append(AbiTypes.getTypeAString(cls)); update the conditional around cls,
getValue().get(i).getTypeAsString(), and AbiTypes.getTypeAString to remove the
duplicated branch.
In `@abi/src/main/java/org/tron/trident/abi/datatypes/primitive/Int.java`:
- Line 19: Replace the fully-qualified type java.lang.Integer with the simple
name Integer in the generic declaration of the Int class (the declaration
"public final class Int extends Number<java.lang.Integer>"). Update the
declaration to use Integer so it follows Java conventions and removes
unnecessary verbosity while keeping the same semantics.
In `@abi/src/main/java/org/tron/trident/abi/FunctionReturnDecoder.java`:
- Around line 66-82: The outputParameters list in decodeDynamicBytes and
decodeAddress is declared with a raw ArrayList causing unchecked warnings;
change its declaration to use a parameterized type (e.g.,
List<TypeReference<Type>> outputParameters = new ArrayList<>()) and ensure the
added TypeReference instances (new TypeReference<DynamicBytes>() {} and new
TypeReference<Address>() {}) match that generic so
decoder.decodeFunctionResult(rawInput, outputParameters) receives a properly
typed List; update both methods (decodeDynamicBytes and decodeAddress)
accordingly.
In `@abi/src/main/java/org/tron/trident/abi/TypeDecoder.java`:
- Around line 858-1029: The repeated Optional.ofNullable(typeReference).map(x ->
x.getSubTypeReference()).map(x -> x.getInnerTypes()).isPresent() logic inside
TypeDecoder.decodeArrayElements (used in the DynamicStruct branch and the
StaticStruct branch) should be extracted into a helper like
hasSubTypeInnerTypes(TypeReference<?>), then replace both identical chains with
a call to that helper to improve readability and avoid duplication; update
references inside decodeArrayElements to call
hasSubTypeInnerTypes(typeReference) where the checks occur.
In `@abi/src/main/java/org/tron/trident/abi/TypeEncoder.java`:
- Around line 149-175: The removePadding method currently checks for Ufixed and
Fixed specifically and returns the full encodedValue for those, but other
FixedPointType subclasses would fall through to an exception; update the type
check in removePadding to use FixedPointType (or include it alongside
Ufixed/Fixed) when deciding to return encodedValue so all fixed-point subclasses
are handled uniformly; locate the removePadding method and replace the existing
Ufixed/Fixed instanceof branch with a FixedPointType check (or add instanceof
FixedPointType) to future-proof the logic.
In `@abi/src/main/java/org/tron/trident/abi/Utils.java`:
- Around line 300-317: The current staticStructsNestedFieldsFlatList method
builds canonicalFields then nestedFields separately which breaks original
declaration order; change the algorithm to iterate classType.getDeclaredFields()
in declaration order and for each Field if
StaticStruct.class.isAssignableFrom(field.getType()) recursively call
staticStructsNestedFieldsFlatList((Class<Type>) field.getType()) and append its
returned list in-place, otherwise append the Field itself — this preserves the
original declaration order while keeping the same recursion and return type;
update references in the method to use a single List accumulator instead of
separate canonicalFields/nestedFields.
- Around line 319-341: The getClassName method is catching Throwable (which can
swallow Errors like OutOfMemoryError); change the catch to a narrower type
(e.g., catch Exception) so only recoverable exceptions are handled; update the
try/catch around the array-handling block in getClassName (variables cl,
dimensions, sb) to catch Exception (or specific reflective exceptions) and let
serious Errors propagate.
- Around line 215-227: The getFullParameterizedTypeFromArray method currently
casts nested type arguments to ParameterizedType without checks, which can throw
ClassCastException for raw or unexpected type structures; update
getFullParameterizedTypeFromArray to validate each cast using instanceof before
casting (check that type is a ParameterizedType and that the inner typeArgument
is a ParameterizedType), handle raw/class cases (e.g., if inner typeArgument is
a Class, extract its name directly) and throw a clear IllegalArgumentException
or ClassNotFoundException with a descriptive message when the structure is
unsupported so callers of getFullParameterizedTypeFromArray receive a
deterministic error instead of a ClassCastException.
In `@abi/src/test/java/org/tron/trident/abi/CustomErrorEncoderTest.java`:
- Around line 56-77: Two test methods, testBuildErrorSignatureWithDynamicStructs
and testBuildErrorSignatureWithDynamicArrays, use package-private visibility
while the rest of the tests are public; update their method signatures in
CustomErrorEncoderTest (the two methods named exactly) to be public void so
visibility is consistent with other tests and the test framework picks them up
consistently.
In `@abi/src/test/java/org/tron/trident/abi/datatypes/DynamicArrayTest.java`:
- Around line 38-50: The test method testMultidimensionalDynamicArray uses raw
DynamicArray types causing unchecked/raw-type warnings; fix by using proper
generics (e.g., parameterize each level as DynamicArray<DynamicArray<Uint256>>
and ultimately DynamicArray<DynamicArray<DynamicArray<Uint256>>> when
constructing the nested arrays) so all DynamicArray instances are fully generic,
or if you prefer to keep the current shape, add `@SuppressWarnings`("rawtypes") to
the test method declaration to silence the warning; update the references to
DynamicArray and Uint256 in the method accordingly.
In `@abi/src/test/java/org/tron/trident/abi/datatypes/StaticStructTest.java`:
- Around line 23-31: Replace the dynamically-built `expected` string in
StaticStructTest with a hardcoded ABI literal so the test no longer depends on
component `getTypeAsString()` implementations: instead of concatenating
address1.getTypeAsString(), address2.getTypeAsString(), and
array4.getTypeAsString(), set `expected` to the explicit ABI string
"(address,address,uint256[4])" (refer to the `expected` variable in
StaticStructTest and the Address/StaticArray4 component types to locate the
code).
In `@abi/src/test/java/org/tron/trident/abi/FunctionReturnDecoderTest.java`:
- Around line 1666-1707: The test method testDecodeMultiDimDynamicArrayFunction
in FunctionReturnDecoderTest is commented out; either remove it or add a
TODO/ticket reference explaining why it's disabled and when to re-enable
it—update the file by deleting the commented block if obsolete, or replace the
comment with a short "// TODO: <tracking-issue-id> - reason and re-enable
criteria" referencing the related fixture
AbiV2TestFixture.setGetMultiDimDynamicArrayFunction and the decoding logic in
FunctionReturnDecoder so future readers know the intent.
In `@core/src/test/java/org/tron/trident/core/AbiV2ContractTest.java`:
- Around line 492-493: The cast to (DynamicArray<Uint256>) in AbiV2ContractTest
(the assignment to this.scores) produces an unchecked cast warning; suppress it
by annotating the enclosing constructor (or the smallest enclosing method) with
`@SuppressWarnings`("unchecked") so the compiler warning is silenced, keeping the
cast as-is and limiting the annotation scope to that constructor/method.
- Around line 618-619: Remove the two debug print statements in the test: delete
the System.out.println(FunctionEncoder.encode(function)) and
System.out.println(ByteArray.toHexString(result.getConstantResult(0).toByteArray()))
lines in AbiV2ContractTest (they are only printing
FunctionEncoder.encode(function) and
ByteArray.toHexString(result.getConstantResult(0).toByteArray())). Leave the
rest of the test intact so assertions/assert logging remain; run tests to
confirm no behavioral changes.
- Around line 677-681: The test currently fakes a 32-byte offset by
string-prepending ("0000...0020") to inputRaw before calling
FunctionReturnDecoder.decode, which is fragile; instead construct the correct
ABI-encoded payload by programmatically computing a 32-byte head containing the
offset (0x20) and concatenating it with the actual bytes from
result.getConstantResult(0) (use ByteArray.toHexString/ByteArray.concat or
equivalent) and pass that full byte array into FunctionReturnDecoder.decode for
the NestedInfo TypeReference, or if the decoder should accept the raw constant
result bytes, remove the manual prepend and update the test to call
FunctionReturnDecoder.decode directly with result.getConstantResult(0) and
document the expected behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6d93dd71-06a7-4e0f-847b-57aae36dea1f
⛔ Files ignored due to path filters (1)
abi/src/main/java/org/tron/trident/abi/datatypes/generated/StaticArray0.javais excluded by!**/generated/**
📒 Files selected for processing (36)
abi/src/main/java/org/tron/trident/abi/CustomErrorEncoder.javaabi/src/main/java/org/tron/trident/abi/DefaultFunctionEncoder.javaabi/src/main/java/org/tron/trident/abi/DefaultFunctionReturnDecoder.javaabi/src/main/java/org/tron/trident/abi/EventEncoder.javaabi/src/main/java/org/tron/trident/abi/FunctionEncoder.javaabi/src/main/java/org/tron/trident/abi/FunctionReturnDecoder.javaabi/src/main/java/org/tron/trident/abi/TypeDecoder.javaabi/src/main/java/org/tron/trident/abi/TypeEncoder.javaabi/src/main/java/org/tron/trident/abi/TypeReference.javaabi/src/main/java/org/tron/trident/abi/Utils.javaabi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.javaabi/src/main/java/org/tron/trident/abi/datatypes/Array.javaabi/src/main/java/org/tron/trident/abi/datatypes/BytesType.javaabi/src/main/java/org/tron/trident/abi/datatypes/CustomError.javaabi/src/main/java/org/tron/trident/abi/datatypes/DynamicArray.javaabi/src/main/java/org/tron/trident/abi/datatypes/DynamicStruct.javaabi/src/main/java/org/tron/trident/abi/datatypes/Event.javaabi/src/main/java/org/tron/trident/abi/datatypes/Function.javaabi/src/main/java/org/tron/trident/abi/datatypes/StaticArray.javaabi/src/main/java/org/tron/trident/abi/datatypes/StaticStruct.javaabi/src/main/java/org/tron/trident/abi/datatypes/Utf8String.javaabi/src/main/java/org/tron/trident/abi/datatypes/primitive/Int.javaabi/src/main/java/org/tron/trident/abi/datatypes/reflection/Parameterized.javaabi/src/test/java/org/tron/trident/abi/AbiV2TestFixture.javaabi/src/test/java/org/tron/trident/abi/CustomErrorEncoderTest.javaabi/src/test/java/org/tron/trident/abi/DefaultFunctionEncoderTest.javaabi/src/test/java/org/tron/trident/abi/EventEncoderTest.javaabi/src/test/java/org/tron/trident/abi/FunctionReturnDecoderTest.javaabi/src/test/java/org/tron/trident/abi/TypeDecoderTest.javaabi/src/test/java/org/tron/trident/abi/TypeEncoderPackedTest.javaabi/src/test/java/org/tron/trident/abi/TypeEncoderTest.javaabi/src/test/java/org/tron/trident/abi/datatypes/CustomErrorTest.javaabi/src/test/java/org/tron/trident/abi/datatypes/DynamicArrayTest.javaabi/src/test/java/org/tron/trident/abi/datatypes/StaticArrayTest.javaabi/src/test/java/org/tron/trident/abi/datatypes/StaticStructTest.javacore/src/test/java/org/tron/trident/core/AbiV2ContractTest.java
Comment on lines
+37
to
+41
| public static String calculateSignatureHash(String errorSignature) { | ||
| byte[] input = errorSignature.getBytes(); | ||
| byte[] hash = Hash.sha3(input); | ||
| return Numeric.toHexString(hash).substring(2); | ||
| } |
There was a problem hiding this comment.
Specify UTF-8 charset explicitly for consistent hashing.
errorSignature.getBytes() uses the platform's default charset, which may vary across systems and could produce different hashes. For ABI signature hashing, UTF-8 should be used explicitly to ensure consistent behavior.
🔧 Proposed fix
+import java.nio.charset.StandardCharsets;
+
public static String calculateSignatureHash(String errorSignature) {
- byte[] input = errorSignature.getBytes();
+ byte[] input = errorSignature.getBytes(StandardCharsets.UTF_8);
byte[] hash = Hash.sha3(input);
return Numeric.toHexString(hash).substring(2);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public static String calculateSignatureHash(String errorSignature) { | |
| byte[] input = errorSignature.getBytes(); | |
| byte[] hash = Hash.sha3(input); | |
| return Numeric.toHexString(hash).substring(2); | |
| } | |
| import java.nio.charset.StandardCharsets; | |
| public static String calculateSignatureHash(String errorSignature) { | |
| byte[] input = errorSignature.getBytes(StandardCharsets.UTF_8); | |
| byte[] hash = Hash.sha3(input); | |
| return Numeric.toHexString(hash).substring(2); | |
| } |
🤖 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/CustomErrorEncoder.java` around lines
37 - 41, The calculateSignatureHash method uses platform-default encoding via
errorSignature.getBytes(); update CustomErrorEncoder.calculateSignatureHash to
explicitly encode the errorSignature as UTF-8 before hashing (use a UTF-8 byte
conversion), then pass that byte[] to Hash.sha3 and return the hex substring as
before; reference calculateSignatureHash, Hash.sha3, and Numeric.toHexString
when locating the change.
Comment on lines
+370
to
+377
| default: { | ||
| try { | ||
| return (Class<? extends Type>) Class.forName(type); | ||
| } catch (ClassNotFoundException e) { | ||
| throw new UnsupportedOperationException( | ||
| "Unsupported type encountered: " + type); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java | sed -n '350,390p'Repository: 0xbigapple/trident
Length of output: 1647
🏁 Script executed:
cat -n abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java | sed -n '320,380p'Repository: 0xbigapple/trident
Length of output: 2066
🏁 Script executed:
cat -n abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java | sed -n '280,320p'Repository: 0xbigapple/trident
Length of output: 1381
🏁 Script executed:
cat -n abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java | sed -n '160,220p'Repository: 0xbigapple/trident
Length of output: 2373
🏁 Script executed:
cat -n abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java | sed -n '140,165p'Repository: 0xbigapple/trident
Length of output: 968
🏁 Script executed:
find . -name "Type.java" -o -name "Type.class" | head -20Repository: 0xbigapple/trident
Length of output: 122
🏁 Script executed:
cat -n abi/src/main/java/org/tron/trident/abi/datatypes/Type.javaRepository: 0xbigapple/trident
Length of output: 1146
🏁 Script executed:
rg -A 5 -B 5 "getType\(" abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java | head -60Repository: 0xbigapple/trident
Length of output: 798
🏁 Script executed:
rg "getType\(" --type java -B 2 -A 2 | grep -v "test" | head -100Repository: 0xbigapple/trident
Length of output: 7065
🏁 Script executed:
rg "AbiTypes\.getType" --type java -B 3 -A 3 | head -150Repository: 0xbigapple/trident
Length of output: 8546
🏁 Script executed:
rg -B 10 "getType\(" abi/src/main/java/org/tron/trident/abi/TypeReference.java | grep -A 10 "getAtomicTypeClass"Repository: 0xbigapple/trident
Length of output: 780
🏁 Script executed:
cat -n abi/src/main/java/org/tron/trident/abi/TypeDecoder.java | sed -n '1,50p'Repository: 0xbigapple/trident
Length of output: 2582
🏁 Script executed:
rg -A 8 "BiFunction.*AbiTypes.getType" abi/src/main/java/org/tron/trident/abi/TypeDecoder.javaRepository: 0xbigapple/trident
Length of output: 44
🏁 Script executed:
rg "AbiTypes.getType" abi/src/main/java/org/tron/trident/abi/TypeDecoder.java -B 2 -A 8Repository: 0xbigapple/trident
Length of output: 475
🏁 Script executed:
rg -B 5 -A 10 "new DynamicArray" abi/src/main/java/org/tron/trident/abi/ --type javaRepository: 0xbigapple/trident
Length of output: 2952
🏁 Script executed:
rg "getType\(" --type java abi/src/main/java/org/tron/trident/abi/ -B 1 -A 1 | grep -E "(getType|solidityType|typeName)" | head -40Repository: 0xbigapple/trident
Length of output: 4029
Add type validation to the Class.forName fallback.
Line 372 will return any class name without checking that it implements Type. Use asSubclass(Type.class) to validate the resolved class, and switch to the non-initializing Class.forName overload so invalid types fail immediately rather than causing issues downstream.
Proposed fix
default: {
try {
- return (Class<? extends Type>) Class.forName(type);
- } catch (ClassNotFoundException e) {
+ return Class.forName(type, false, AbiTypes.class.getClassLoader())
+ .asSubclass(Type.class);
+ } catch (ClassNotFoundException | ClassCastException e) {
throw new UnsupportedOperationException(
"Unsupported type encountered: " + type);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| default: { | |
| try { | |
| return (Class<? extends Type>) Class.forName(type); | |
| } catch (ClassNotFoundException e) { | |
| throw new UnsupportedOperationException( | |
| "Unsupported type encountered: " + type); | |
| } | |
| } | |
| default: { | |
| try { | |
| return Class.forName(type, false, AbiTypes.class.getClassLoader()) | |
| .asSubclass(Type.class); | |
| } catch (ClassNotFoundException | ClassCastException e) { | |
| throw new UnsupportedOperationException( | |
| "Unsupported type encountered: " + 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
370 - 377, Replace the unchecked Class.forName(type) in the AbiTypes fallback
with a non-initializing lookup and subclass validation: call Class.forName(type,
false, Thread.currentThread().getContextClassLoader()).asSubclass(Type.class)
and return that result; update the exception handling to catch
ClassNotFoundException and ClassCastException (or IllegalArgumentException) and
rethrow the same UnsupportedOperationException("Unsupported type encountered: "
+ type) so only classes that actually implement Type are accepted.
Comment on lines
+74
to
+79
| public List<Object> getNativeValueCopy() { | ||
| List<Object> copy = new ArrayList<>(value.size()); | ||
| for (T t : value) { | ||
| copy.add(t.getValue()); | ||
| } | ||
| return Collections.unmodifiableList(copy); |
There was a problem hiding this comment.
getNativeValueCopy() is still shallow for ABI v2 composites.
Line 77 unwraps only the outer Type. For nested arrays/structs, this returns List<Type> again, so callers still receive wrapped ABI objects from a method that promises native values. A StaticArray<StaticArray<Uint256>> would come back as List<List<Uint256>>, not native numbers.
🔁 Proposed fix
public List<Object> getNativeValueCopy() {
List<Object> copy = new ArrayList<>(value.size());
for (T t : value) {
- copy.add(t.getValue());
+ copy.add(t instanceof Array ? ((Array<?>) t).getNativeValueCopy() : t.getValue());
}
return Collections.unmodifiableList(copy);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public List<Object> getNativeValueCopy() { | |
| List<Object> copy = new ArrayList<>(value.size()); | |
| for (T t : value) { | |
| copy.add(t.getValue()); | |
| } | |
| return Collections.unmodifiableList(copy); | |
| public List<Object> getNativeValueCopy() { | |
| List<Object> copy = new ArrayList<>(value.size()); | |
| for (T t : value) { | |
| copy.add(t instanceof Array ? ((Array<?>) t).getNativeValueCopy() : t.getValue()); | |
| } | |
| return Collections.unmodifiableList(copy); |
🤖 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/Array.java` around lines 74
- 79, The getNativeValueCopy() method in Array<T> currently unwraps only the
outer Type and is therefore shallow; update it to deep-unwrap nested ABI
composites by detecting composite element types and calling their
native-unwrapping methods instead of t.getValue() directly: for each element t
in value, if t is an Array (e.g. instanceof Array) call
((Array<?>)t).getNativeValueCopy(), if t is a struct/composite call that
composite's native-unwrap method (e.g. Struct.getNativeValueCopy() or
equivalent), otherwise fall back to t.getValue(); ensure you reference the Array
class and the getNativeValueCopy() method in your changes and add any needed
imports.
Comment on lines
27
to
34
| public DynamicArray(T... values) { | ||
| super((Class<T>) AbiTypes.getType(values[0].getTypeAsString()), values); | ||
| super( | ||
| StructType.class.isAssignableFrom(values[0].getClass()) | ||
| || Array.class.isAssignableFrom(values[0].getClass()) | ||
| ? (Class<T>) values[0].getClass() | ||
| : (Class<T>) AbiTypes.getType(values[0].getTypeAsString()), | ||
| values); | ||
| } |
There was a problem hiding this comment.
Potential ArrayIndexOutOfBoundsException when called with empty array.
The constructor accesses values[0] without checking if the array is empty. If called with zero arguments, this will throw ArrayIndexOutOfBoundsException. Consider adding a guard or documenting that empty arrays are not supported by this constructor variant.
🛡️ Proposed fix to add empty check
`@SafeVarargs`
`@SuppressWarnings`({"unchecked"})
public DynamicArray(T... values) {
+ if (values.length == 0) {
+ throw new IllegalArgumentException("Empty arrays not supported; use DynamicArray(Class<T>, T...) instead");
+ }
super(
StructType.class.isAssignableFrom(values[0].getClass())
|| Array.class.isAssignableFrom(values[0].getClass())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public DynamicArray(T... values) { | |
| super((Class<T>) AbiTypes.getType(values[0].getTypeAsString()), values); | |
| super( | |
| StructType.class.isAssignableFrom(values[0].getClass()) | |
| || Array.class.isAssignableFrom(values[0].getClass()) | |
| ? (Class<T>) values[0].getClass() | |
| : (Class<T>) AbiTypes.getType(values[0].getTypeAsString()), | |
| values); | |
| } | |
| public DynamicArray(T... values) { | |
| if (values.length == 0) { | |
| throw new IllegalArgumentException("Empty arrays not supported; use DynamicArray(Class<T>, T...) instead"); | |
| } | |
| super( | |
| StructType.class.isAssignableFrom(values[0].getClass()) | |
| || Array.class.isAssignableFrom(values[0].getClass()) | |
| ? (Class<T>) values[0].getClass() | |
| : (Class<T>) AbiTypes.getType(values[0].getTypeAsString()), | |
| values); | |
| } |
🤖 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 27 - 34, The DynamicArray(T... values) constructor dereferences values[0]
which will throw ArrayIndexOutOfBoundsException for empty varargs; add a guard
at the start of the constructor that checks if values == null || values.length
== 0 and handle it explicitly (e.g., throw a clear IllegalArgumentException
stating empty varargs are not allowed or delegate to an existing constructor
that accepts an explicit element Class), then proceed to compute the element
class as currently done using values[0] and
AbiTypes.getType(values[0].getTypeAsString()).
Comment on lines
38
to
45
| public DynamicArray(List<T> values) { | ||
| super((Class<T>) AbiTypes.getType(values.get(0).getTypeAsString()), values); | ||
| super( | ||
| StructType.class.isAssignableFrom(values.get(0).getClass()) | ||
| || Array.class.isAssignableFrom(values.get(0).getClass()) | ||
| ? (Class<T>) values.get(0).getClass() | ||
| : (Class<T>) AbiTypes.getType(values.get(0).getTypeAsString()), | ||
| values); | ||
| } |
There was a problem hiding this comment.
Potential IndexOutOfBoundsException when called with empty list.
Similar to the varargs constructor, this method accesses values.get(0) without verifying the list is non-empty. An empty list will cause IndexOutOfBoundsException.
🛡️ Proposed fix to add empty check
`@SuppressWarnings`("unchecked")
public DynamicArray(List<T> values) {
+ if (values.isEmpty()) {
+ throw new IllegalArgumentException("Empty lists not supported; use DynamicArray(Class<T>, List<T>) instead");
+ }
super(
StructType.class.isAssignableFrom(values.get(0).getClass())
|| Array.class.isAssignableFrom(values.get(0).getClass())🤖 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 38 - 45, The DynamicArray(List<T> values) constructor currently
dereferences values.get(0) which throws IndexOutOfBoundsException for an empty
list; update the constructor (DynamicArray(List<T> values)) to validate values
is non-empty (e.g., if (values == null || values.isEmpty()) throw new
IllegalArgumentException("values must not be empty")) before using values.get(0)
so callers get a clear, documented error instead of an OOB exception.
Comment on lines
+51
to
+56
| super( | ||
| StructType.class.isAssignableFrom(values.get(0).getClass()) | ||
| || Array.class.isAssignableFrom(values.get(0).getClass()) | ||
| ? (Class<T>) values.get(0).getClass() | ||
| : (Class<T>) AbiTypes.getType(values.get(0).getTypeAsString()), | ||
| values); |
There was a problem hiding this comment.
Empty static arrays still crash in this constructor path.
Line 52 reads values.get(0) before any emptiness guard, so expectedSize == 0 with an empty list throws IndexOutOfBoundsException. That breaks the new zero-length-array support whenever this overload is used, or at least prevents it from failing with a clear error.
🛠️ Suggested guard
`@Deprecated`
`@SuppressWarnings`("unchecked")
public StaticArray(int expectedSize, List<T> values) {
- super(
- StructType.class.isAssignableFrom(values.get(0).getClass())
- || Array.class.isAssignableFrom(values.get(0).getClass())
- ? (Class<T>) values.get(0).getClass()
- : (Class<T>) AbiTypes.getType(values.get(0).getTypeAsString()),
- values);
+ super(resolveType(values), values);
checkValid(expectedSize);
}
+
+ `@SuppressWarnings`("unchecked")
+ private static <T extends Type> Class<T> resolveType(List<T> values) {
+ if (values.isEmpty()) {
+ throw new UnsupportedOperationException(
+ "Use the typed StaticArray(Class<T>, ...) constructor for empty static arrays.");
+ }
+ return StructType.class.isAssignableFrom(values.get(0).getClass())
+ || Array.class.isAssignableFrom(values.get(0).getClass())
+ ? (Class<T>) values.get(0).getClass()
+ : (Class<T>) AbiTypes.getType(values.get(0).getTypeAsString());
+ }🤖 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/StaticArray.java` around
lines 51 - 56, The constructor currently calls values.get(0) unguarded, which
throws for empty lists; fix by computing the element class before calling super
with a guard: if values.isEmpty() then derive the element class via
AbiTypes.getType(...) for the element type (fallback path) otherwise use the
existing logic that checks
StructType.class.isAssignableFrom(values.get(0).getClass()) ||
Array.class.isAssignableFrom(values.get(0).getClass()) ? (Class<T>)
values.get(0).getClass() : (Class<T>)
AbiTypes.getType(values.get(0).getTypeAsString()); then pass that computed
Class<T> and values into super instead of directly calling values.get(0) in the
super call.
Comment on lines
+87
to
+95
| String type; | ||
| if (!value.isEmpty() && StructType.class.isAssignableFrom(value.get(0).getClass())) { | ||
| type = value.get(0).getTypeAsString(); | ||
| } else if (!value.isEmpty() && Array.class.isAssignableFrom(value.get(0).getClass())) { | ||
| type = value.get(0).getTypeAsString(); | ||
| } else { | ||
| type = AbiTypes.getTypeAString(getComponentType()); | ||
| } | ||
| return type + "[" + value.size() + "]"; |
There was a problem hiding this comment.
Empty composite arrays still produce invalid ABI type strings.
When value is empty, Line 93 falls back to AbiTypes.getTypeAString(getComponentType()). For StaticArray0<MyStruct> or StaticArray0<StaticArray4<Uint256>>, that yields mystruct[0] / staticarray4[0] instead of tuple or nested-array syntax, so selectors and event signatures for zero-length composite arrays are wrong.
🤖 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/StaticArray.java` around
lines 87 - 95, The current logic builds the element type from the first element
in value or falls back to AbiTypes.getTypeAString(getComponentType()) when value
is empty, which yields incorrect ABI strings for empty composite arrays; change
the fallback to inspect getComponentType(): if getComponentType() is a
StructType use its getTypeAsString(), if it's an Array use its
getTypeAsString(), otherwise use AbiTypes.getTypeAString(getComponentType());
update the branch in StaticArray (the block referencing value,
getComponentType(), StructType, Array, and AbiTypes.getTypeAString) so
zero-length composite arrays produce correct tuple or nested-array type syntax
before appending "[" + value.size() + "]".
Comment on lines
+76
to
+80
| public List<TypeReference<?>> getInnerTypes() { | ||
| return this.innerTypes; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
getInnerTypes() may return null; consider documenting or initializing.
The innerTypes field is only initialized when using the new constructor at line 51. When using other constructors, getInnerTypes() will return null. Callers need to handle this, or consider initializing to an empty list by default.
🛡️ Option: Initialize to empty list or document nullability
- protected List<TypeReference<?>> innerTypes;
+ protected List<TypeReference<?>> innerTypes = Collections.emptyList();Or add @Nullable annotation and document the contract.
🤖 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/TypeReference.java` around lines 76 -
80, The getInnerTypes() method can return null because the field innerTypes is
only set by the constructor that accepts inner types; update the class so
callers don't have to handle null by either initializing innerTypes to an empty
List by default (e.g., new ArrayList<>() in the field declaration or in all
constructors) or annotate and document the nullable contract (add `@Nullable` to
getInnerTypes and Javadoc explaining null return). Locate the innerTypes field
and all constructors (including the one that currently sets innerTypes) and
ensure every constructor initializes innerTypes or add the `@Nullable` annotation
and Javadoc to getInnerTypes to make the contract explicit.
Comment on lines
+71
to
+78
| /** Ports {@link java.lang.reflect.Type#getTypeName()}. */ | ||
| public static String getTypeName(java.lang.reflect.Type type) { | ||
| try { | ||
| return type.getTypeName(); | ||
| } catch (NoSuchMethodError e) { | ||
| return getClassName((Class) type); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the Java version requirement in build configuration
fd -t f "build.gradle" --exec cat {} \; 2>/dev/null | grep -E "sourceCompatibility|targetCompatibility|JavaVersion"
fd -t f "pom.xml" --exec grep -A2 -E "<source>|<target>|<release>" {} \; 2>/dev/nullRepository: 0xbigapple/trident
Length of output: 167
Remove unnecessary NoSuchMethodError catch block.
The project targets Java 8 minimum (sourceCompatibility = JavaVersion.VERSION_1_8), so Type.getTypeName() is guaranteed to be available. The catch block and fallback to getClassName() will never execute and should be removed to simplify the code.
🤖 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 71 - 78,
Remove the unnecessary NoSuchMethodError handling in Utils.getTypeName: simplify
the method to directly return type.getTypeName() and delete the try/catch and
fallback to getClassName((Class) type), since the project targets Java 8+ where
java.lang.reflect.Type.getTypeName() is guaranteed; update the method signature
body for getTypeName(java.lang.reflect.Type type) accordingly and remove any
unused imports if they become redundant.
Comment on lines
+23
to
+26
| Iterator<TypeReference<?>> expectedParameter = parameters.iterator(); | ||
| for (TypeReference<?> actualParameter : event.getParameters()) { | ||
| assertEquals(expectedParameter.next(), actualParameter); | ||
| } |
There was a problem hiding this comment.
This test still passes if the implementation drops the last parameter.
Lines 23-26 only compare the prefix of event.getParameters(). If getParameters() returns one entry instead of two, the loop exits cleanly and the test stays green. Add a size assertion so the full parameter list is verified.
✅ Tighten the assertion
+ assertEquals(parameters.size(), event.getParameters().size());
Iterator<TypeReference<?>> expectedParameter = parameters.iterator();
for (TypeReference<?> actualParameter : event.getParameters()) {
assertEquals(expectedParameter.next(), actualParameter);
}🤖 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 23 - 26, The test currently only iterates and compares a prefix of
event.getParameters(), so add an explicit size equality assertion (compare
parameters.size() with event.getParameters().size() or assertEquals on those)
before the loop in CustomErrorTest so the full parameter list is validated; keep
the existing iterator/loop (using expectedParameter and event.getParameters())
but fail fast if lengths differ.
…odeWithOutPrefix/buildEventSignatureWithOutPrefix
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
18 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/main/java/org/tron/trident/abi/CustomErrorEncoder.java">
<violation number="1" location="abi/src/main/java/org/tron/trident/abi/CustomErrorEncoder.java:40">
P1: Custom error selectors should be the first 4 bytes of the hash, not the full 32-byte digest.</violation>
</file>
<file name="abi/src/main/java/org/tron/trident/abi/DefaultFunctionReturnDecoder.java">
<violation number="1" location="abi/src/main/java/org/tron/trident/abi/DefaultFunctionReturnDecoder.java:127">
P1: This offset update still treats `StaticArrayN<DynamicBytes>` and `StaticArrayN<DynamicArray<...>>` as inline arrays. After decoding one of those returns, the next output is read from the wrong tuple offset.</violation>
<violation number="2" location="abi/src/main/java/org/tron/trident/abi/DefaultFunctionReturnDecoder.java:188">
P1: This dynamic-array check is only one level deep. Nested static arrays like `uint256[][3][3]` are still treated as inline, so their function return decoding starts from the pointer word instead of the array payload.</violation>
</file>
<file name="abi/src/main/java/org/tron/trident/abi/datatypes/Array.java">
<violation number="1" location="abi/src/main/java/org/tron/trident/abi/datatypes/Array.java:77">
P2: `getNativeValueCopy()` only unwraps one level, so nested arrays/structs still contain ABI `Type` objects instead of native values.</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:828">
P1: These packed-array expectations keep 32-byte element padding, so they lock in non-packed output for sub-32-byte array elements.</violation>
<violation number="2" location="abi/src/test/java/org/tron/trident/abi/TypeEncoderPackedTest.java:850">
P2: These assertions lock in `encodePacked` rejecting ABI v2 struct arrays, which contradicts the feature this PR is adding.</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:92">
P1: Annotated static-array fields inside structs are emitted as dynamic arrays because this branch always formats them with `[]`.</violation>
<violation number="2" location="abi/src/main/java/org/tron/trident/abi/Utils.java:208">
P2: Nested array type names are reduced to the raw inner array class, so multidimensional `TypeReference`s produce non-canonical ABI names like `staticarray2[3]`.</violation>
<violation number="3" location="abi/src/main/java/org/tron/trident/abi/Utils.java:226">
P1: This helper breaks 3D dynamic-array decoding because `Class.forName` is given a parameterized type string, not a class name.</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:139">
P1: This overload bypasses the TypeReference-aware array/struct decoders, so array or tuple TypeReferences still throw at runtime.</violation>
<violation number="2" location="abi/src/main/java/org/tron/trident/abi/TypeDecoder.java:432">
P1: The new innerTypes tuple path still decodes fixed-array fields as plain classes, which throws instead of decoding the array member.</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:880">
P3: Specify the charset for these string-to-bytes conversions so the ABI fixture is deterministic across JVM default encodings.</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:24">
P2: This loop can pass even when `getParameters()` returns too few elements, because it never asserts that all expected parameters were visited.</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:81">
P1: Empty nested arrays still produce invalid ABI type strings on the zero-length path. `AbiTypes.getTypeAString(getComponentType())` returns raw Java class names for array subclasses, so encoding an empty nested array can generate `dynamicarray[]`/`staticarrayN[]` instead of a canonical ABI type.</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">
P2: Compute the padded length from the UTF-8 byte count; non-empty strings can span more than one 32-byte word.</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">
P2: Reject reflected classes that do not implement `Type`. Accepting any loadable class here turns invalid ABI type names into later runtime failures instead of the expected unsupported-type error.</violation>
</file>
<file name="abi/src/main/java/org/tron/trident/abi/datatypes/StaticArray.java">
<violation number="1" location="abi/src/main/java/org/tron/trident/abi/datatypes/StaticArray.java:54">
P1: Validate all nested array/struct elements before inferring the component type from `values.get(0)`. As written, heterogeneous nested arrays can be encoded under the first element’s ABI type and produce malformed calldata.</violation>
</file>
<file name="abi/src/main/java/org/tron/trident/abi/DefaultFunctionEncoder.java">
<violation number="1" location="abi/src/main/java/org/tron/trident/abi/DefaultFunctionEncoder.java:119">
P1: Static arrays of static structs are mis-sized when the struct contains static array fields, so later dynamic offsets can be encoded incorrectly.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| public static String calculateSignatureHash(String errorSignature) { | ||
| byte[] input = errorSignature.getBytes(); | ||
| byte[] hash = Hash.sha3(input); | ||
| return Numeric.toHexString(hash).substring(2); |
There was a problem hiding this comment.
P1: Custom error selectors should be the first 4 bytes of the hash, not the full 32-byte digest.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At abi/src/main/java/org/tron/trident/abi/CustomErrorEncoder.java, line 40:
<comment>Custom error selectors should be the first 4 bytes of the hash, not the full 32-byte digest.</comment>
<file context>
@@ -0,0 +1,42 @@
+ public static String calculateSignatureHash(String errorSignature) {
+ byte[] input = errorSignature.getBytes();
+ byte[] hash = Hash.sha3(input);
+ return Numeric.toHexString(hash).substring(2);
+ }
+}
</file context>
| return StaticArray.class.isAssignableFrom(type) | ||
| && (DynamicStruct.class.isAssignableFrom( | ||
| getParameterizedTypeFromArray(typeReference)) | ||
| || isDynamic(getParameterizedTypeFromArray(typeReference))); |
There was a problem hiding this comment.
P1: This dynamic-array check is only one level deep. Nested static arrays like uint256[][3][3] are still treated as inline, so their function return decoding starts from the pointer word instead of the array payload.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At abi/src/main/java/org/tron/trident/abi/DefaultFunctionReturnDecoder.java, line 188:
<comment>This dynamic-array check is only one level deep. Nested static arrays like `uint256[][3][3]` are still treated as inline, so their function return decoding starts from the pointer word instead of the array payload.</comment>
<file context>
@@ -137,13 +155,39 @@ private static List<Type> build(String input, List<TypeReference<Type>> outputPa
+ return StaticArray.class.isAssignableFrom(type)
+ && (DynamicStruct.class.isAssignableFrom(
+ getParameterizedTypeFromArray(typeReference))
+ || isDynamic(getParameterizedTypeFromArray(typeReference)));
+ } catch (ClassCastException e) {
+ return false;
</file context>
Comment on lines
+127
to
+144
| if (DynamicStruct.class.isAssignableFrom( | ||
| getParameterizedTypeFromArray(typeReference))) { | ||
| offset += MAX_BYTE_LENGTH_FOR_HEX_STRING; | ||
| } else if (StaticStruct.class.isAssignableFrom( | ||
| getParameterizedTypeFromArray(typeReference))) { | ||
| offset += | ||
| staticStructNestedPublicFieldsFlatList( | ||
| getParameterizedTypeFromArray( | ||
| typeReference)) | ||
| .size() | ||
| * length | ||
| * MAX_BYTE_LENGTH_FOR_HEX_STRING; | ||
| } else if (Utf8String.class.isAssignableFrom( | ||
| getParameterizedTypeFromArray(typeReference))) { | ||
| offset += MAX_BYTE_LENGTH_FOR_HEX_STRING; | ||
| } else { | ||
| offset += length * MAX_BYTE_LENGTH_FOR_HEX_STRING; | ||
| } |
There was a problem hiding this comment.
P1: This offset update still treats StaticArrayN<DynamicBytes> and StaticArrayN<DynamicArray<...>> as inline arrays. After decoding one of those returns, the next output is read from the wrong tuple offset.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At abi/src/main/java/org/tron/trident/abi/DefaultFunctionReturnDecoder.java, line 127:
<comment>This offset update still treats `StaticArrayN<DynamicBytes>` and `StaticArrayN<DynamicArray<...>>` as inline arrays. After decoding one of those returns, the next output is read from the wrong tuple offset.</comment>
<file context>
@@ -78,55 +83,68 @@ private static List<Type> build(String input, List<TypeReference<Type>> outputPa
-
+ TypeDecoder.decodeStaticArray(
+ input, hexStringDataOffset, typeReference, length);
+ if (DynamicStruct.class.isAssignableFrom(
+ getParameterizedTypeFromArray(typeReference))) {
+ offset += MAX_BYTE_LENGTH_FOR_HEX_STRING;
</file context>
Suggested change
| if (DynamicStruct.class.isAssignableFrom( | |
| getParameterizedTypeFromArray(typeReference))) { | |
| offset += MAX_BYTE_LENGTH_FOR_HEX_STRING; | |
| } else if (StaticStruct.class.isAssignableFrom( | |
| getParameterizedTypeFromArray(typeReference))) { | |
| offset += | |
| staticStructNestedPublicFieldsFlatList( | |
| getParameterizedTypeFromArray( | |
| typeReference)) | |
| .size() | |
| * length | |
| * MAX_BYTE_LENGTH_FOR_HEX_STRING; | |
| } else if (Utf8String.class.isAssignableFrom( | |
| getParameterizedTypeFromArray(typeReference))) { | |
| offset += MAX_BYTE_LENGTH_FOR_HEX_STRING; | |
| } else { | |
| offset += length * MAX_BYTE_LENGTH_FOR_HEX_STRING; | |
| } | |
| Class<Type> elementType = getParameterizedTypeFromArray(typeReference); | |
| if (StaticStruct.class.isAssignableFrom(elementType)) { | |
| offset += | |
| staticStructNestedPublicFieldsFlatList(elementType).size() | |
| * length | |
| * MAX_BYTE_LENGTH_FOR_HEX_STRING; | |
| } else if (isDynamic(elementType)) { | |
| offset += MAX_BYTE_LENGTH_FOR_HEX_STRING; | |
| } else { | |
| offset += length * MAX_BYTE_LENGTH_FOR_HEX_STRING; | |
| } |
| @@ -0,0 +1,1082 @@ | |||
| package org.tron.trident.abi; | |||
There was a problem hiding this comment.
P1: These packed-array expectations keep 32-byte element padding, so they lock in non-packed output for sub-32-byte array elements.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At abi/src/test/java/org/tron/trident/abi/TypeEncoderPackedTest.java, line 828:
<comment>These packed-array expectations keep 32-byte element padding, so they lock in non-packed output for sub-32-byte array elements.</comment>
<file context>
@@ -0,0 +1,1082 @@
+ Uint16.class, new Uint16(0x45), new Uint16(0x7), new Uint16(65535));
+
+ assertEquals(
+ "0000000000000000000000000000000000000000000000000000000000000045"
+ + "0000000000000000000000000000000000000000000000000000000000000007"
+ + "000000000000000000000000000000000000000000000000000000000000ffff",
</file context>
| return (Class<T>) | ||
| Class.forName( | ||
| ((ParameterizedType) typeArgument) | ||
| .getActualTypeArguments()[0].getTypeName()); |
There was a problem hiding this comment.
P1: This helper breaks 3D dynamic-array decoding because Class.forName is given a parameterized type string, not a class name.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At abi/src/main/java/org/tron/trident/abi/Utils.java, line 226:
<comment>This helper breaks 3D dynamic-array decoding because `Class.forName` is given a parameterized type string, not a class name.</comment>
<file context>
@@ -94,18 +182,50 @@ static <T extends Type, U extends Type> String getParameterizedTypeName(
+ return (Class<T>)
+ Class.forName(
+ ((ParameterizedType) typeArgument)
+ .getActualTypeArguments()[0].getTypeName());
+ }
+
</file context>
|
|
||
| if (typeArguments[0] instanceof ParameterizedType) { | ||
| return (Class<T>) | ||
| Class.forName(getTypeName(((ParameterizedType) typeArguments[0]).getRawType())); |
There was a problem hiding this comment.
P2: Nested array type names are reduced to the raw inner array class, so multidimensional TypeReferences produce non-canonical ABI names like staticarray2[3].
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At abi/src/main/java/org/tron/trident/abi/Utils.java, line 208:
<comment>Nested array type names are reduced to the raw inner array class, so multidimensional `TypeReference`s produce non-canonical ABI names like `staticarray2[3]`.</comment>
<file context>
@@ -94,18 +182,50 @@ static <T extends Type, U extends Type> String getParameterizedTypeName(
+ if (typeArguments[0] instanceof ParameterizedType) {
+ return (Class<T>)
+ Class.forName(getTypeName(((ParameterizedType) typeArguments[0]).getRawType()));
+ }
+
</file context>
Comment on lines
+24
to
+26
| for (TypeReference<?> actualParameter : event.getParameters()) { | ||
| assertEquals(expectedParameter.next(), actualParameter); | ||
| } |
There was a problem hiding this comment.
P2: This loop can pass even when getParameters() returns too few elements, because it never asserts that all expected parameters were visited.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At abi/src/test/java/org/tron/trident/abi/datatypes/CustomErrorTest.java, line 24:
<comment>This loop can pass even when `getParameters()` returns too few elements, because it never asserts that all expected parameters were visited.</comment>
<file context>
@@ -0,0 +1,29 @@
+ assertEquals(event.getName(), "MyError");
+
+ Iterator<TypeReference<?>> expectedParameter = parameters.iterator();
+ for (TypeReference<?> actualParameter : event.getParameters()) {
+ assertEquals(expectedParameter.next(), actualParameter);
+ }
</file context>
Suggested change
| for (TypeReference<?> actualParameter : event.getParameters()) { | |
| assertEquals(expectedParameter.next(), actualParameter); | |
| } | |
| assertEquals(parameters.size(), event.getParameters().size()); | |
| for (TypeReference<?> actualParameter : event.getParameters()) { | |
| assertEquals(expectedParameter.next(), actualParameter); | |
| } |
| throw new UnsupportedOperationException("Unsupported type encountered: " + type); | ||
| default: { | ||
| try { | ||
| return (Class<? extends Type>) Class.forName(type); |
There was a problem hiding this comment.
P2: Reject reflected classes that do not implement Type. Accepting any loadable class here turns invalid ABI type names into later runtime failures instead of the expected unsupported-type error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At abi/src/main/java/org/tron/trident/abi/datatypes/AbiTypes.java, line 372:
<comment>Reject reflected classes that do not implement `Type`. Accepting any loadable class here turns invalid ABI type names into later runtime failures instead of the expected unsupported-type error.</comment>
<file context>
@@ -367,11 +367,22 @@ public static Class<? extends Type> getType(String type, boolean primitives) {
- throw new UnsupportedOperationException("Unsupported type encountered: " + type);
+ default: {
+ try {
+ return (Class<? extends Type>) Class.forName(type);
+ } catch (ClassNotFoundException e) {
+ throw new UnsupportedOperationException(
</file context>
Suggested change
| return (Class<? extends Type>) Class.forName(type); | |
| Class<?> resolvedType = Class.forName(type); | |
| if (!Type.class.isAssignableFrom(resolvedType)) { | |
| throw new UnsupportedOperationException("Unsupported type encountered: " + type); | |
| } | |
| return (Class<? extends Type>) resolvedType; |
| FUNC_addDynamicBytesArray, | ||
| Arrays.<Type>asList( | ||
| new BytesStruct( | ||
| "dynamic".getBytes(), BigInteger.ZERO, "Bytes".getBytes())), |
There was a problem hiding this comment.
P3: Specify the charset for these string-to-bytes conversions so the ABI fixture is deterministic across JVM default encodings.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At abi/src/test/java/org/tron/trident/abi/AbiV2TestFixture.java, line 880:
<comment>Specify the charset for these string-to-bytes conversions so the ABI fixture is deterministic across JVM default encodings.</comment>
<file context>
@@ -322,5 +817,146 @@ public Wiz(Foo foo, Utf8String data) {
+ FUNC_addDynamicBytesArray,
+ Arrays.<Type>asList(
+ new BytesStruct(
+ "dynamic".getBytes(), BigInteger.ZERO, "Bytes".getBytes())),
+ Collections.<TypeReference<?>>emptyList());
+
</file context>
Suggested change
| "dynamic".getBytes(), BigInteger.ZERO, "Bytes".getBytes())), | |
| "dynamic".getBytes(java.nio.charset.StandardCharsets.UTF_8), BigInteger.ZERO, "Bytes".getBytes(java.nio.charset.StandardCharsets.UTF_8))), |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
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/main/java/org/tron/trident/abi/EventEncoder.java">
<violation number="1" location="abi/src/main/java/org/tron/trident/abi/EventEncoder.java:65">
P1: This changes `buildEventSignature()`/`encode()` to return `0x`-prefixed hashes under the same public API. That is a breaking behavior change and makes event topic encoding inconsistent with the other ABI encoders.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -54,4 +64,14 @@ public static String buildEventSignature(String methodSignature) { | |||
| byte[] hash = Hash.sha3(input); | |||
| return Numeric.toHexString(hash); | |||
There was a problem hiding this comment.
P1: This changes buildEventSignature()/encode() to return 0x-prefixed hashes under the same public API. That is a breaking behavior change and makes event topic encoding inconsistent with the other ABI encoders.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At abi/src/main/java/org/tron/trident/abi/EventEncoder.java, line 65:
<comment>This changes `buildEventSignature()`/`encode()` to return `0x`-prefixed hashes under the same public API. That is a breaking behavior change and makes event topic encoding inconsistent with the other ABI encoders.</comment>
<file context>
@@ -52,6 +62,16 @@ static <T extends Type> String buildMethodSignature(
byte[] input = methodSignature.getBytes();
byte[] hash = Hash.sha3(input);
- return Numeric.toHexString(hash).substring(2);
+ return Numeric.toHexString(hash);
+ }
+
</file context>
…r Encoding, add unitest
…ray elements When encoding DynamicArray<StaticArray<DynamicBytes>> (e.g. bytes[3][]),the encoder was missing offset pointer table for inner elements.StaticArray with dynamic sub-elements was not recognized as needing offsets in encodeArrayValuesOffsets()
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds ABI v2 support for tuples/structs and nested arrays with packed encoding and custom errors.
Why are these changes required?
This PR has been tested by:
Follow up
Extra details
Summary by cubic
Adds ABI v2 support for tuples/structs, nested arrays, and custom errors with packed encoding. Also adds selector/event helpers and fixes encoding bugs with nested dynamic arrays and UTF‑8 handling.
New Features
StaticArray0.CustomErrorandCustomErrorEncoderfor signature hashing and encoding.bytes/stringand complex types inTypeEncoder.DefaultFunctionEncoder.encodeWithSelector(...)to build calls from a precomputed selector.0xprefix:EventEncoder.encodeWithOutPrefix(...)andbuildEventSignatureWithOutPrefix(...).TypeReferenceand class-by-name resolution inAbiTypes.Bug Fixes
DynamicArray<StaticArray<DynamicBytes>>(e.g.,bytes[3][]) so nested dynamic elements encode correctly.Utf8String.bytes32PaddedLength()for empty and multi-chunk strings; fix non-ASCII UTF‑8 length/encoding.BytesType.bytes32PaddedLength()for payloads >32 bytes and exact multiples.Written for commit 83d18d3. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
abi.encodePacked) for multiple data typesImprovements