Add Hex Normalization to prevent leading Zeros in RLP #135
Conversation
WalkthroughThe pull request introduces significant structural changes to the RLP (Recursive Length Prefix) encoding and decoding logic in the Ethereum Virtual Machine (EVM) utilities. It replaces the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/src/crypto/evm/utils/rlp.dart (1)
Line range hint
63-116: Consider adding a comprehensive RLP validation layer.While the hex normalization is a good improvement, consider enhancing the RLP implementation with a dedicated validation layer that:
- Validates input according to the RLP specification
- Checks string length limits
- Verifies encoding format
- Handles edge cases systematically
This would help catch invalid inputs early and improve error reporting.
Would you like me to provide a detailed design proposal for this validation layer?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/src/crypto/evm/utils/rlp.dart(2 hunks)
🔇 Additional comments (1)
lib/src/crypto/evm/utils/rlp.dart (1)
116-116: LGTM! Verify RLP encoding compatibility.The integration of
normalizeHexStringlooks good. However, let's verify that the normalized output remains compatible with Ethereum's RLP encoding specification.Run the following script to find and analyze test cases:
✅ Verification successful
RLP encoding compatibility verified through test suite ✓
The change is safe as verified by existing test cases that confirm:
- Correct RLP encoding/decoding roundtrip
- Proper transaction serialization compatibility
- Accurate handling of hex strings in the context of EVM transactions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for RLP encoding test cases to verify compatibility # Look for test files that might contain RLP encoding test vectors # Search for test files echo "Searching for RLP test files..." fd -e dart test.*rlp # Search for specific RLP test cases with hex strings echo "Searching for hex string test cases..." rg -A 5 "rlpEncode.*0x"Length of output: 210
Script:
#!/bin/bash # Try multiple approaches to find RLP-related tests and usage # Search for test files containing 'rlp' case-insensitive echo "=== Searching for RLP test files (case-insensitive) ===" fd -e dart -i rlp # Search for RLP-related code and tests echo -e "\n=== Searching for RLP test cases and usage ===" rg -l "rlp" --type dart # Look for hex string handling in tests echo -e "\n=== Searching for hex string test cases ===" rg -A 3 "test.*hex" --type dart # Search for RLP encode method usage echo -e "\n=== Searching for RLP encode usage ===" rg -A 2 "encode.*hex" --type dartLength of output: 1323
Script:
#!/bin/bash # Examine RLP implementation and its usage in transaction handling # Check RLP encoding implementation echo "=== RLP Implementation Details ===" rg -A 10 "_encode.*object" lib/src/crypto/evm/utils/rlp.dart # Check transaction parsing test echo -e "\n=== Transaction Parsing Test Details ===" cat test/ci/evm/parsing/parse_hex_transaction_test.dart # Check RLP usage in transaction handling echo -e "\n=== RLP Usage in Transaction Handling ===" rg -A 5 "rlp\." lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dartLength of output: 2659
lib/src/crypto/evm/utils/rlp.dart
Outdated
| /** | ||
| * Normalizes a hex string by removing leading zeros and '0x' prefix | ||
| * @param {String} hexStr The hex string to normalize | ||
| * @returns {String} The normalized hex string | ||
| */ | ||
| String normalizeHexString(String hexStr) { | ||
| // Remove 0x prefix if present | ||
| hexStr = hexStr.toLowerCase().replaceAll('0x', ''); | ||
|
|
||
| // Remove leading zeros | ||
| hexStr = hexStr.replaceAll(RegExp(r'^0+'), ''); | ||
|
|
||
| // Handle special case of zero | ||
| if (hexStr.isEmpty) { | ||
| return '00'; | ||
| } | ||
|
|
||
| // Ensure even length | ||
| if (hexStr.length % 2 != 0) { | ||
| hexStr = '0' + hexStr; | ||
| } | ||
|
|
||
| return hexStr; | ||
| } |
There was a problem hiding this comment.
Add input validation and fix zero handling.
The function has two issues that need to be addressed:
- The zero handling logic could fail for strings containing only zeros
- Missing validation for non-hex characters
Apply this diff to fix the issues:
String normalizeHexString(String hexStr) {
+ // Validate hex string format
+ if (!RegExp(r'^(0x)?[0-9a-fA-F]*$').hasMatch(hexStr)) {
+ throw ArgumentError('Invalid hex string');
+ }
+
// Remove 0x prefix if present
hexStr = hexStr.toLowerCase().replaceAll('0x', '');
- // Remove leading zeros
- hexStr = hexStr.replaceAll(RegExp(r'^0+'), '');
-
- // Handle special case of zero
- if (hexStr.isEmpty) {
+ // Handle special case of zero or empty string
+ if (hexStr.isEmpty || RegExp(r'^0+$').hasMatch(hexStr)) {
return '00';
}
+ // Remove leading zeros
+ hexStr = hexStr.replaceAll(RegExp(r'^0+(?=[1-9a-f])'), '');
+
// Ensure even length
if (hexStr.length % 2 != 0) {
hexStr = '0' + hexStr;
}
return hexStr;
}📝 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.
| /** | |
| * Normalizes a hex string by removing leading zeros and '0x' prefix | |
| * @param {String} hexStr The hex string to normalize | |
| * @returns {String} The normalized hex string | |
| */ | |
| String normalizeHexString(String hexStr) { | |
| // Remove 0x prefix if present | |
| hexStr = hexStr.toLowerCase().replaceAll('0x', ''); | |
| // Remove leading zeros | |
| hexStr = hexStr.replaceAll(RegExp(r'^0+'), ''); | |
| // Handle special case of zero | |
| if (hexStr.isEmpty) { | |
| return '00'; | |
| } | |
| // Ensure even length | |
| if (hexStr.length % 2 != 0) { | |
| hexStr = '0' + hexStr; | |
| } | |
| return hexStr; | |
| } | |
| /** | |
| * Normalizes a hex string by removing leading zeros and '0x' prefix | |
| * @param {String} hexStr The hex string to normalize | |
| * @returns {String} The normalized hex string | |
| */ | |
| String normalizeHexString(String hexStr) { | |
| // Validate hex string format | |
| if (!RegExp(r'^(0x)?[0-9a-fA-F]*$').hasMatch(hexStr)) { | |
| throw ArgumentError('Invalid hex string'); | |
| } | |
| // Remove 0x prefix if present | |
| hexStr = hexStr.toLowerCase().replaceAll('0x', ''); | |
| // Handle special case of zero or empty string | |
| if (hexStr.isEmpty || RegExp(r'^0+$').hasMatch(hexStr)) { | |
| return '00'; | |
| } | |
| // Remove leading zeros | |
| hexStr = hexStr.replaceAll(RegExp(r'^0+(?=[1-9a-f])'), ''); | |
| // Ensure even length | |
| if (hexStr.length % 2 != 0) { | |
| hexStr = '0' + hexStr; | |
| } | |
| return hexStr; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart (1)
Line range hint
2-2: Remove unused import 'bigint_utils.dart'The import
package:walletkit_dart/src/utils/bigint_utils.dartis unused and should be removed to resolve the pipeline warning and keep the codebase clean.
🧹 Nitpick comments (7)
lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart (2)
229-254: Remove commented-out code to improve code readabilityThere are extensive blocks of commented-out code in the
fromUnsignedHexandfromHexmethods. Commented code can clutter the codebase and make maintenance more difficult. Consider removing these comments if they are no longer needed or replace them with proper implementation.Also applies to: 270-291
518-543: Clean up commented-out code in decoding methodsMultiple decoding methods across
RawEVMTransactionType1andRawEVMTransactionType2contain large blocks of commented-out code. This can hinder readability and maintenance. Consider removing the commented code or restoring it with appropriate updates if it's still needed.Also applies to: 562-593, 679-706, 725-758
test/ci/rlp/rlp_test.dart (2)
121-125: Add tests or remove empty test group 'RLP Encoding Integer'The test group
"RLP Encoding Integer"is empty and does not contain any test cases. If integer encoding tests are planned, please add them to ensure comprehensive test coverage. Otherwise, consider removing this empty group to tidy up the test suite.
157-183: Review necessity of the commented-out 'GETH Tests'The "GETH Tests" section is entirely commented out. If these tests are still relevant and provide value, consider uncommenting and updating them to match the current codebase. If they are obsolete, removing them can improve code clarity.
lib/src/crypto/evm/utils/rlp.dart (1)
146-164: Improve documentation and optimize implementation.The function could benefit from better documentation and optimization.
-/** - * - * @param {int} value - * - * This function takes an integer value and converts it to a Uint8List. - * - * @returns {Uint8List} - */ +/// Converts an integer value to its RLP byte representation. +/// +/// If the value is 0, returns [0x80] as per RLP spec. +/// For non-zero values, converts to big-endian byte representation. +/// +/// Example: +/// ```dart +/// arrayifyInteger(0) // returns [0x80] +/// arrayifyInteger(1) // returns [0x01] +/// arrayifyInteger(127) // returns [0x7f] +/// arrayifyInteger(128) // returns [0x80, 0x80] +/// ``` +/// +/// @param value The integer to convert +/// @returns The RLP byte representation Uint8List arrayifyInteger(int value) { if (value == 0) { return Uint8List.fromList([0x80]); } - List<int> result = []; - while (value > 0) { - result.insert(0, value & 0xff); - value >>= 8; + + // Calculate required bytes + int bytes = (value.bitLength + 7) >> 3; + final result = Uint8List(bytes); + + // Fill bytes from right to left + for (int i = bytes - 1; i >= 0; i--) { + result[i] = value & 0xff; + value >>= 8; } return Uint8List.fromList(result); }test/ci/rlp/fixtures/geth.json (1)
1-302: Comprehensive test coverage for RLP encoding/decoding.The test fixtures provide excellent coverage across various scenarios including single bytes, strings, arrays, and nested structures. The consistent format facilitates easy parsing and validation.
However, consider adding test cases for:
- Maximum length strings/arrays to validate length prefix encoding
- Invalid RLP encodings to verify error handling
- Unicode strings to ensure proper encoding
test/ci/rlp/fixtures/rlptest.json (1)
5-173: Well-structured test cases with clear categorization.The test fixtures provide comprehensive coverage with well-named test cases and clear input/output mappings. The organization into categories (strings, integers, lists) enhances maintainability.
A few suggestions for additional test cases:
- Negative integers
- Floating-point numbers
- Invalid UTF-8 sequences
- Maximum nesting depth
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart(10 hunks)lib/src/crypto/evm/utils/rlp.dart(1 hunks)lib/src/domain/extensions.dart(1 hunks)test/ci/evm/parsing/fetch_function_signature_test.dart(1 hunks)test/ci/evm/parsing/parse_hex_transaction_test.dart(1 hunks)test/ci/evm/parsing/reverse-hash-computation_test.dart(3 hunks)test/ci/rlp/fixtures/geth.json(1 hunks)test/ci/rlp/fixtures/invalid.json(1 hunks)test/ci/rlp/fixtures/rlptest.json(1 hunks)test/ci/rlp/rlp_test.dart(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/ci/evm/parsing/parse_hex_transaction_test.dart
🧰 Additional context used
🪛 GitHub Actions: TestSuite
test/ci/evm/parsing/reverse-hash-computation_test.dart
[warning] 1-1: Unused import: 'dart:math'. Try removing the import directive.
lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart
[warning] 2-2: Unused import: 'package:walletkit_dart/src/utils/bigint_utils.dart'. Try removing the import directive.
🔇 Additional comments (10)
lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart (1)
Line range hint
144-166: Verify data types in serialization bufferIn the
serializedgetter ofRawEVMTransactionType0, the properties_nonce,_gasPrice,_gasLimit,_value, and_dataare added directly to the buffer without conversion. Ensure that these properties are correctly serialized according to RLP encoding specifications. Previously, they might have been converted to hex strings or byte arrays. Please verify that the current implementation correctly handles the data types for RLP encoding.lib/src/domain/extensions.dart (1)
46-47: Confirm the use of 'allowMalformed' in 'bytesToUTF8'The
bytesToUTF8getter usesutf8.decodewithallowMalformed: true, which will replace any malformed UTF-8 sequences with the Unicode replacement character (�). Please verify that this behavior is intentional. If strict decoding is required, settingallowMalformedtofalsewill throw an exception on malformed input.test/ci/evm/parsing/reverse-hash-computation_test.dart (2)
24-26: LGTM! Code is more idiomatic.The changes improve code clarity by:
- Using type inference
- Directly accessing RLPItem values
102-104: LGTM! Consistent with new RLP implementation.The changes properly handle type casting and value access with the new RLPItem hierarchy.
test/ci/evm/parsing/fetch_function_signature_test.dart (1)
17-19: LGTM! Consistent with previous file.The changes follow the same pattern of improvements as in reverse-hash-computation_test.dart.
test/ci/rlp/fixtures/invalid.json (2)
5-130: LGTM! Comprehensive test cases for invalid RLP scenarios.The test cases cover important invalid scenarios including:
- Integer overflows
- Wrong size lists
- Incorrect lengths
- Leading zeros
- Non-optimal encodings
4-4:⚠️ Potential issueUpdate the date to the current date.
The date is set to 2025-01-20, which is in the future.
- "date": "2025-01-20", + "date": "2024-01-20",Likely invalid or redundant comment.
lib/src/crypto/evm/utils/rlp.dart (1)
6-33: LGTM! Well-structured base class with type-safe conversion.The
RLPItembase class provides a clean hierarchy for RLP types with type-safe conversion throughfromValue.test/ci/rlp/fixtures/rlptest.json (2)
4-4:⚠️ Potential issueFix the future date in metadata.
The date field is set to "2025-01-20", which is in the future. This should be updated to reflect the actual creation date.
- "date": "2025-01-20", + "date": "2024-01-20",Likely invalid or redundant comment.
70-77: Verify handling of large integers.The test cases for medium integers use string representation with "#" prefix. Ensure that the RLP implementation correctly handles these large numbers and doesn't suffer from integer overflow.
✅ Verification successful
Large integer handling verified successfully.
The RLP implementation properly handles large integers using Dart's BigInt type, which has no practical size limit. The codebase shows robust encoding/decoding implementations with proper test coverage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for integer handling in the codebase ast-grep --pattern 'int $name = $_' # Find integer declarations rg -A 5 'parseInt|BigInt|fromString' # Find number parsingLength of output: 85075
| final rlpDecoded = decodeRLP(rawTxHex.hexToBytes); | ||
|
|
||
| if (rlpDecoded is! List) { | ||
| throw Exception("Error RLP decoding transaction: $rlpDecoded"); | ||
| } | ||
|
|
||
| final decodedList = rlpDecoded.whereType<String>().toList(); | ||
|
|
||
| if (decodedList.length < 6) { | ||
| throw Exception("Invalid transaction, missing fields: $decodedList"); | ||
| } | ||
|
|
||
| final nonce = parseAsHexBigInt(decodedList[0]); | ||
| final gasPrice = parseAsHexBigInt(decodedList[1]); | ||
| final gasLimit = parseAsHexBigInt(decodedList[2]); | ||
| final to = "0x" + decodedList[3]; | ||
| final value = parseAsHexBigInt(decodedList[4]); | ||
| final data = decodedList[5].hexToBytes; | ||
|
|
||
| final chainId = | ||
| decodedList.length > 6 ? parseAsHexInt(decodedList[6]) : null; | ||
|
|
||
| return RawEVMTransactionType0.unsigned( | ||
| nonce: nonce, | ||
| gasPrice: gasPrice, | ||
| gasLimit: gasLimit, | ||
| to: to, | ||
| value: value, | ||
| data: data, | ||
| chainId: chainId, | ||
| ); | ||
| // final decodedList = rlpDecoded.whereType<String>().toList(); | ||
|
|
||
| // if (decodedList.length < 6) { | ||
| // throw Exception("Invalid transaction, missing fields: $decodedList"); | ||
| // } | ||
|
|
||
| // final nonce = parseAsHexBigInt(decodedList[0]); | ||
| // final gasPrice = parseAsHexBigInt(decodedList[1]); | ||
| // final gasLimit = parseAsHexBigInt(decodedList[2]); | ||
| // final to = "0x" + decodedList[3]; | ||
| // final value = parseAsHexBigInt(decodedList[4]); | ||
| // final data = decodedList[5].hexToBytes; | ||
|
|
||
| // final chainId = | ||
| // decodedList.length > 6 ? parseAsHexInt(decodedList[6]) : null; | ||
|
|
||
| // return RawEVMTransactionType0.unsigned( | ||
| // nonce: nonce, | ||
| // gasPrice: gasPrice, | ||
| // gasLimit: gasLimit, | ||
| // to: to, | ||
| // value: value, | ||
| // data: data, | ||
| // chainId: chainId, | ||
| // ); | ||
|
|
||
| throw UnimplementedError(); |
There was a problem hiding this comment.
Implement 'fromUnsignedHex' method or handle unimplemented code properly
The fromUnsignedHex factory constructor currently throws UnimplementedError(), and the previous decoding logic is commented out. This prevents the creation of unsigned transactions from hex strings. Please implement the necessary decoding logic or remove the method if it's not required.
| final rlpDecoded = decodeRLP(rawTxHex.hexToBytes); | ||
|
|
||
| if (rlpDecoded is! List) { | ||
| if (rlpDecoded is! RLPList) { | ||
| throw Exception("Error RLP decoding transaction: $rlpDecoded"); | ||
| } | ||
|
|
||
| final decodedList = rlpDecoded.whereType<String>().toList(); | ||
|
|
||
| if (decodedList.length < 9) { | ||
| throw Exception("Invalid transaction, missing fields: $decodedList"); | ||
| if (rlpDecoded.length < 9) { | ||
| throw Exception("Invalid transaction, missing fields: $rlpDecoded"); | ||
| } | ||
|
|
||
| final nonce = parseAsHexBigInt(decodedList[0]); | ||
| final gasPrice = parseAsHexBigInt(decodedList[1]); | ||
| final gasLimit = parseAsHexBigInt(decodedList[2]); | ||
| final to = "0x" + decodedList[3]; | ||
| final value = parseAsHexBigInt(decodedList[4]); | ||
| final data = decodedList[5].hexToBytes; | ||
| final v = parseAsHexInt(decodedList[6]); | ||
| final r = parseAsHexBigInt(decodedList[7]); | ||
| final s = parseAsHexBigInt(decodedList[8]); | ||
|
|
||
| return RawEVMTransactionType0( | ||
| nonce: nonce, | ||
| gasPrice: gasPrice, | ||
| gasLimit: gasLimit, | ||
| to: to, | ||
| value: value, | ||
| data: data, | ||
| v: v, | ||
| r: r, | ||
| s: s, | ||
| ); | ||
| // final nonce = parseAsHexBigInt(rlpDecoded[0]); | ||
| // final gasPrice = parseAsHexBigInt(rlpDecoded[1]); | ||
| // final gasLimit = parseAsHexBigInt(rlpDecoded[2]); | ||
| // final to = "0x" + rlpDecoded[3]; | ||
| // final value = parseAsHexBigInt(rlpDecoded[4]); | ||
| // final data = rlpDecoded[5].hexToBytes; | ||
| // final v = parseAsHexInt(rlpDecoded[6]); | ||
| // final r = parseAsHexBigInt(rlpDecoded[7]); | ||
| // final s = parseAsHexBigInt(rlpDecoded[8]); | ||
|
|
||
| // return RawEVMTransactionType0( | ||
| // nonce: nonce, | ||
| // gasPrice: gasPrice, | ||
| // gasLimit: gasLimit, | ||
| // to: to, | ||
| // value: value, | ||
| // data: data, | ||
| // v: v, | ||
| // r: r, | ||
| // s: s, | ||
| // ); | ||
|
|
||
| throw UnimplementedError(); |
There was a problem hiding this comment.
Implement 'fromHex' method or handle unimplemented code properly
Similar to fromUnsignedHex, the fromHex factory constructor is incomplete and throws UnimplementedError(), with the decoding logic commented out. This method is essential for decoding signed transactions from hex strings. Please provide the implementation or remove the method if it's unnecessary.
| final rlpDecoded = decodeRLP(rawTxHex.hexToBytes); | ||
|
|
||
| if (rlpDecoded is! List) { | ||
| if (rlpDecoded is! RLPList) { | ||
| throw Exception("Error RLP decoding transaction: $rlpDecoded"); | ||
| } | ||
|
|
||
| if (rlpDecoded.length < 8) { | ||
| throw Exception("Invalid transaction, missing fields: $rlpDecoded"); | ||
| } | ||
|
|
||
| final chainId = parseAsHexInt(rlpDecoded[0]); | ||
| final nonce = parseAsHexBigInt(rlpDecoded[1]); | ||
| final gasPrice = parseAsHexBigInt(rlpDecoded[2]); | ||
| final gasLimit = parseAsHexBigInt(rlpDecoded[3]); | ||
| final to = "0x" + rlpDecoded[4]; | ||
| final value = parseAsHexBigInt(rlpDecoded[5]); | ||
| final data = (rlpDecoded[6] as String).hexToBytes; | ||
| final accessList = [ | ||
| for (final item in rlpDecoded[7]) | ||
| ( | ||
| address: "0x" + item[0], | ||
| storageKeys: (item[1] as List<dynamic>).whereType<String>().toList(), | ||
| ) | ||
| ]; | ||
|
|
||
| return RawEVMTransactionType1.unsigned( | ||
| nonce: nonce, | ||
| gasLimit: gasLimit, | ||
| to: to, | ||
| value: value, | ||
| data: data, | ||
| chainId: chainId, | ||
| gasPrice: gasPrice, | ||
| accessList: accessList, | ||
| ); | ||
| // final chainId = parseAsHexInt(rlpDecoded[0]); | ||
| // final nonce = parseAsHexBigInt(rlpDecoded[1]); | ||
| // final gasPrice = parseAsHexBigInt(rlpDecoded[2]); | ||
| // final gasLimit = parseAsHexBigInt(rlpDecoded[3]); | ||
| // final to = "0x" + rlpDecoded[4]; | ||
| // final value = parseAsHexBigInt(rlpDecoded[5]); | ||
| // final data = (rlpDecoded[6] as String).hexToBytes; | ||
| // final accessList = [ | ||
| // for (final item in rlpDecoded[7]) | ||
| // ( | ||
| // address: "0x" + item[0], | ||
| // storageKeys: (item[1] as List<dynamic>).whereType<String>().toList(), | ||
| // ) | ||
| // ]; | ||
|
|
||
| // return RawEVMTransactionType1.unsigned( | ||
| // nonce: nonce, | ||
| // gasLimit: gasLimit, | ||
| // to: to, | ||
| // value: value, | ||
| // data: data, | ||
| // chainId: chainId, | ||
| // gasPrice: gasPrice, | ||
| // accessList: accessList, | ||
| // ); | ||
|
|
||
| throw UnimplementedError(); |
There was a problem hiding this comment.
Complete the implementation of transaction decoding methods in 'RawEVMTransactionType1'
The decoding methods fromUnsignedHexing and fromHex in RawEVMTransactionType1 are incomplete, throwing UnimplementedError(), and contain commented-out decoding logic. These methods are crucial for handling Type 1 transactions. Please implement the decoding logic to enable proper transaction processing.
Also applies to: 552-593
| // final rlpDecoded = decodeRLP(rawTxHex.hexToBytes, 0).result; | ||
|
|
||
| // if (rlpDecoded is! List) { | ||
| // throw Exception("Error RLP decoding transaction: $rlpDecoded"); | ||
| // } | ||
|
|
||
| // if (rlpDecoded.length < 9) { | ||
| // throw Exception("Invalid transaction, missing fields: $rlpDecoded"); | ||
| // } | ||
|
|
||
| // final chainId = parseAsHexInt(rlpDecoded[0]); | ||
| // final nonce = parseAsHexBigInt(rlpDecoded[1]); | ||
| // final maxPriorityFeePerGas = parseAsHexBigInt(rlpDecoded[2]); | ||
| // final maxFeePerGas = parseAsHexBigInt(rlpDecoded[3]); | ||
| // final gasLimit = parseAsHexBigInt(rlpDecoded[4]); | ||
| // final to = "0x" + rlpDecoded[5]; | ||
| // final value = parseAsHexBigInt(rlpDecoded[6]); | ||
| // final data = (rlpDecoded[7] as String).hexToBytes; | ||
| // final accessList = [ | ||
| // for (final item in rlpDecoded[8]) | ||
| // ( | ||
| // address: "0x" + item[0], | ||
| // storageKeys: (item[1] as List<dynamic>).whereType<String>().toList(), | ||
| // ) | ||
| // ]; | ||
|
|
||
| // return RawEVMTransactionType2.unsigned( | ||
| // nonce: nonce, | ||
| // gasLimit: gasLimit, | ||
| // to: to, | ||
| // value: value, | ||
| // data: data, | ||
| // chainId: chainId, | ||
| // maxFeePerGas: maxFeePerGas, | ||
| // maxPriorityFeePerGas: maxPriorityFeePerGas, | ||
| // accessList: accessList, | ||
| // ); | ||
|
|
||
| throw UnimplementedError(); |
There was a problem hiding this comment.
Complete the implementation of transaction decoding methods in 'RawEVMTransactionType2'
The fromUnsignedHex and fromHex methods in RawEVMTransactionType2 are also incomplete and currently throw UnimplementedError(), with the decoding logic commented out. Implement these methods to support decoding of Type 2 transactions.
Also applies to: 715-759
test/ci/rlp/rlp_test.dart
Outdated
| final _ = decodeRLP(input.hexToBytes); | ||
|
|
||
| fail("Expected to throw but didn't"); | ||
| } on TestFailure { | ||
| fail("Expected to throw but didn't"); | ||
| } catch (e) { | ||
| expect(true, true); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle exceptions appropriately in tests
In the test for invalid Geth cases, catching all exceptions and then asserting expect(true, true); may mask unexpected errors. Instead, consider catching specific exceptions or using expect with a function that should throw, to ensure that only the expected exceptions are caught.
Apply this change to improve exception handling:
- } catch (e) {
- expect(true, true);
+ } catch (e) {
+ // Expected exception, test passes
}Alternatively, use expect with throwsException:
expect(() => decodeRLP(input.hexToBytes), throwsException);| @@ -1,3 +1,4 @@ | |||
| import 'dart:math'; | |||
There was a problem hiding this comment.
Remove unused import.
The import of 'dart:math' is not used in this file.
-import 'dart:math';🧰 Tools
🪛 GitHub Actions: TestSuite
[warning] 1-1: Unused import: 'dart:math'. Try removing the import directive.
lib/src/crypto/evm/utils/rlp.dart
Outdated
| final class RLPList extends RLPItem<List<RLPItem>> { | ||
| RLPList(super.value); | ||
|
|
||
| @override | ||
| Uint8List get encoded { | ||
| throw UnimplementedError(); | ||
| } | ||
|
|
||
| return "0x" + result; | ||
| int get length => value.length; | ||
|
|
||
| RLPItem operator [](int index) { | ||
| return value[index]; | ||
| } | ||
| } |
There was a problem hiding this comment.
Implement missing encoded getter and add documentation.
The RLPList class has two issues:
- The
encodedgetter throws UnimplementedError - Missing documentation for public members
Add implementation and documentation:
final class RLPList extends RLPItem<List<RLPItem>> {
+ /// Creates a new RLP list with the given items
RLPList(super.value);
@override
Uint8List get encoded {
- throw UnimplementedError();
+ final output = List<int>.empty(growable: true);
+ for (final item in value) {
+ output.addAll(encodeRLP(item));
+ }
+ if (output.length < 56) {
+ return Uint8List.fromList([0xc0 + output.length, ...output]);
+ }
+ final lengthBuffer = arrayifyInteger(output.length);
+ return Uint8List.fromList([
+ 0xf7 + lengthBuffer.length,
+ ...lengthBuffer,
+ ...output,
+ ]);
}
+ /// Returns the number of items in the list
int get length => value.length;
+ /// Returns the item at the given index
RLPItem operator [](int index) {
return value[index];
}
}Committable suggestion skipped: line range outside the PR's diff.
lib/src/crypto/evm/utils/rlp.dart
Outdated
| Uint8List rlpEncode(dynamic input) { | ||
| return Uint8List(1); | ||
| } | ||
|
|
||
| final result = hex.encode(data.sublist( | ||
| offset + 1 + lengthLength, offset + 1 + lengthLength + length)); | ||
| return DecodedRLP(consumed: (1 + lengthLength + length), result: result); | ||
| } else if (data[offset] >= 0x80) { | ||
| final length = data[offset] - 0x80; | ||
| RLPItem decodeRLP(Uint8List input) { | ||
| return RLPInt(0); | ||
| } |
There was a problem hiding this comment.
Implement core RLP encoding/decoding functions.
Both rlpEncode and decodeRLP have stub implementations that need to be completed.
Implement the functions:
Uint8List rlpEncode(dynamic input) {
- return Uint8List(1);
+ final item = RLPItem.fromValue(input);
+ return encodeRLP(item);
}
RLPItem decodeRLP(Uint8List input) {
- return RLPInt(0);
+ if (input.isEmpty) {
+ throw ArgumentError('Empty input');
+ }
+
+ final firstByte = input[0];
+ if (firstByte <= 0x7f) {
+ return RLPBytes(Uint8List.fromList([firstByte]));
+ }
+
+ if (firstByte <= 0xb7) {
+ final length = firstByte - 0x80;
+ if (input.length - 1 < length) {
+ throw ArgumentError('Input too short');
+ }
+ return RLPBytes(input.sublist(1, length + 1));
+ }
+
+ if (firstByte <= 0xbf) {
+ final llength = firstByte - 0xb7;
+ if (input.length - 1 < llength) {
+ throw ArgumentError('Input too short');
+ }
+ final length = unarrayifyInteger(input.sublist(1, llength + 1));
+ if (input.length - 1 - llength < length) {
+ throw ArgumentError('Input too short');
+ }
+ return RLPBytes(input.sublist(llength + 1, llength + 1 + length));
+ }
+
+ if (firstByte <= 0xf7) {
+ final length = firstByte - 0xc0;
+ if (input.length - 1 < length) {
+ throw ArgumentError('Input too short');
+ }
+ final items = <RLPItem>[];
+ var offset = 1;
+ while (offset < length + 1) {
+ final item = decodeRLP(input.sublist(offset));
+ items.add(item);
+ offset += item.encoded.length;
+ }
+ return RLPList(items);
+ }
+
+ final llength = firstByte - 0xf7;
+ if (input.length - 1 < llength) {
+ throw ArgumentError('Input too short');
+ }
+ final length = unarrayifyInteger(input.sublist(1, llength + 1));
+ if (input.length - 1 - llength < length) {
+ throw ArgumentError('Input too short');
+ }
+ final items = <RLPItem>[];
+ var offset = llength + 1;
+ while (offset < llength + 1 + length) {
+ final item = decodeRLP(input.sublist(offset));
+ items.add(item);
+ offset += item.encoded.length;
+ }
+ return RLPList(items);
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart (3)
483-519:⚠️ Potential issueImplement
fromUnsignedHexingmethod inRawEVMTransactionType1The
fromUnsignedHexingmethod is incomplete and currently throwsUnimplementedError, with the decoding logic commented out. This prevents the creation of unsigned Type 1 transactions from hex strings. Please implement the necessary decoding logic.🧰 Tools
🪛 GitHub Actions: TestSuite
[warning] 483-483: The value of the local variable 'rlpDecoded' isn't used. Try removing the variable or using it.
527-568:⚠️ Potential issueImplement
fromHexmethod inRawEVMTransactionType1The
fromHexmethod inRawEVMTransactionType1is incomplete and throwsUnimplementedError, with the decoding logic commented out. Implementing this method is essential for decoding signed Type 1 transactions from hex strings.
644-682:⚠️ Potential issueComplete the implementation of transaction decoding methods in
RawEVMTransactionType2The
fromUnsignedHexandfromHexmethods inRawEVMTransactionType2are incomplete and throwUnimplementedError, with the decoding logic commented out. Implement these methods to support decoding of Type 2 transactions.Also applies to: 690-734
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart(10 hunks)lib/src/crypto/evm/utils/rlp.dart(1 hunks)lib/src/domain/extensions.dart(1 hunks)lib/src/utils/bigint_utils.dart(0 hunks)lib/src/utils/var_uint.dart(1 hunks)test/ci/rlp/rlp_test.dart(1 hunks)
💤 Files with no reviewable changes (1)
- lib/src/utils/bigint_utils.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/src/domain/extensions.dart
- test/ci/rlp/rlp_test.dart
🧰 Additional context used
🪛 GitHub Actions: TestSuite
lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart
[error] 435-435: The method 'rlpEncode' isn't defined for the type 'RawEVMTransactionType1'. Try correcting the name to the name of an existing method, or defining a method named 'rlpEncode'.
[error] 471-471: The method 'rlpEncode' isn't defined for the type 'RawEVMTransactionType1'. Try correcting the name to the name of an existing method, or defining a method named 'rlpEncode'.
[error] 802-802: The method 'rlpEncode' isn't defined for the type 'RawEVMTransactionType2'. Try correcting the name to the name of an existing method, or defining a method named 'rlpEncode'.
[error] 840-840: The method 'rlpEncode' isn't defined for the type 'RawEVMTransactionType2'. Try correcting the name to the name of an existing method, or defining a method named 'rlpEncode'.
[warning] 483-483: The value of the local variable 'rlpDecoded' isn't used. Try removing the variable or using it.
🔇 Additional comments (3)
lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart (2)
143-157: Serialization logic correctly updated to use new RLP encodingThe serialization method now appropriately uses
encodeRLPwithRLPListand specificRLPItemsubclasses. This enhances clarity and aligns with the updated RLP encoding implementation.
483-483: 🛠️ Refactor suggestionRemove unused variable or implement decoding logic
At line 483, the variable
rlpDecodedis assigned but never used, leading to a warning in the pipeline. Since the decoding logic is commented out and the method throwsUnimplementedError, please either remove the unused variable or complete the implementation.Apply this diff to remove the unused variable:
- final rlpDecoded = decodeRLP(rawTxHex.hexToBytes);Alternatively, uncomment and complete the decoding logic.
Likely invalid or redundant comment.
🧰 Tools
🪛 GitHub Actions: TestSuite
[warning] 483-483: The value of the local variable 'rlpDecoded' isn't used. Try removing the variable or using it.
lib/src/crypto/evm/utils/rlp.dart (1)
145-184: Ensure correct handling of zero values in RLP encodingIn the
encodeRLPfunction, there is special handling for zero values inRLPIntandRLPBigInt. Ensure that zero is encoded correctly according to the RLP specification. In RLP, an empty string is represented by0x80, and zero as an integer should be encoded as a single zero byte0x00.Please verify that zero values are handled appropriately and adjust the encoding logic if necessary.
|
|
||
| return Uint8List.fromList( | ||
| [0x01, ...rlpEncode(buffer).hexToBytes], | ||
| [0x01, ...rlpEncode(buffer)], |
There was a problem hiding this comment.
Replace rlpEncode with encodeRLP to fix undefined method error
The method rlpEncode is not defined, causing pipeline failures. Replace rlpEncode with encodeRLP to resolve the issue.
Apply this diff to fix the error:
At lines 435 and 471:
- [0x01, ...rlpEncode(buffer)],
+ [0x01, ...encodeRLP(RLPList(buffer))],At lines 802 and 840:
- [0x02, ...rlpEncode(buffer)],
+ [0x02, ...encodeRLP(RLPList(buffer))],Also applies to: 471-471, 802-802, 840-840
🧰 Tools
🪛 GitHub Actions: TestSuite
[error] 435-435: The method 'rlpEncode' isn't defined for the type 'RawEVMTransactionType1'. Try correcting the name to the name of an existing method, or defining a method named 'rlpEncode'.
| int readVarIntFromLength( | ||
| int offset, | ||
| int length, | ||
| Endian endian, | ||
| ) { | ||
| if (length < 0xfd) { | ||
| return getUint8(offset); | ||
| } | ||
| if (length == 0xfd) { | ||
| return getUint16(offset, endian); | ||
| } | ||
| if (length == 0xfe) { | ||
| return getUint32(offset, endian); | ||
| } | ||
| return getUint64(offset, endian); | ||
| } |
There was a problem hiding this comment.
Review the logic of readVarIntFromLength method
The readVarIntFromLength method may have logical errors in handling the length parameter. The comparisons with values like 0xfd (253) are unusual if length represents the number of bytes to read. Typically, these values are used as prefix indicators in variable-length integer encodings, not actual byte lengths.
Please verify and correct the logic. If length represents the length in bytes, the comparisons should be against the actual byte sizes (e.g., 1, 2, 4, 8), not against 0xfd and 0xfe. Consider adjusting the conditions as follows:
if (length == 1) {
return getUint8(offset);
}
if (length == 2) {
return getUint16(offset, endian);
}
if (length == 4) {
return getUint32(offset, endian);
}
if (length == 8) {
return getUint64(offset, endian);
}| Uint8List arrayifyInteger(int value) { | ||
| if (value == 0) { | ||
| return Uint8List.fromList([0x80]); | ||
| } | ||
| List<int> result = []; | ||
| while (value > 0) { | ||
| result.insert(0, value & 0xff); | ||
| value >>= 8; | ||
| } | ||
|
|
||
| return DecodedRLP(consumed: 1, result: hexlifyByte(data[offset])); | ||
| return Uint8List.fromList(result); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate the implementation of arrayifyInteger function
The arrayifyInteger function may not correctly handle the encoding of zero. According to the RLP specification, the integer zero should be encoded as an empty byte array, resulting in a single byte 0x80 when encoded. However, the function currently returns [0x80] when the value is zero, which may not be appropriate.
Please review the function to ensure it aligns with the RLP specification for integer encoding.
Apply this diff to adjust the handling of zero:
- if (value == 0) {
- return Uint8List.fromList([0x80]);
- }
+ if (value == 0) {
+ return Uint8List(0);
+ }📝 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.
| Uint8List arrayifyInteger(int value) { | |
| if (value == 0) { | |
| return Uint8List.fromList([0x80]); | |
| } | |
| List<int> result = []; | |
| while (value > 0) { | |
| result.insert(0, value & 0xff); | |
| value >>= 8; | |
| } | |
| return DecodedRLP(consumed: 1, result: hexlifyByte(data[offset])); | |
| return Uint8List.fromList(result); | |
| Uint8List arrayifyInteger(int value) { | |
| if (value == 0) { | |
| return Uint8List(0); | |
| } | |
| List<int> result = []; | |
| while (value > 0) { | |
| result.insert(0, value & 0xff); | |
| value >>= 8; | |
| } | |
| return Uint8List.fromList(result); |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart (2)
507-523: Simplify access list handling and add null safety.The access list handling could be simplified and made more robust with proper null checks.
Consider this more concise implementation:
- accessList: switch (rlpDecoded[7]) { - RLPList list => list.value - .whereType<RLPList>() - .map((item) { - final subList = item[1]; - if (subList is RLPList) - return ( - address: "0x" + item[0].buffer.toHex, - storageKeys: - subList.value.map((key) => key.buffer.toHex).toList(), - ); - return null; - }) - .nonNulls - .toList(), - _ => [], - }, + accessList: (rlpDecoded[7] as RLPList?)?.value + .whereType<RLPList>() + .map((item) => ( + address: "0x" + (item[0].buffer.toHex), + storageKeys: (item[1] as RLPList) + .value + .map((key) => key.buffer.toHex) + .toList(), + )) + .toList() ?? + [],
Line range hint
404-838: Consider extracting common RLP serialization patterns.The transaction types share similar RLP serialization patterns. Consider creating a shared utility class to reduce code duplication and improve maintainability.
You could:
- Create an abstract base class with common RLP serialization logic
- Extract access list serialization into a shared utility
- Implement type-specific serialization through template methods
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart(11 hunks)test/ci/evm/parsing/parse_hex_transaction_test.dart(1 hunks)test/ci/evm/parsing/reverse-hash-computation_test.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- test/ci/evm/parsing/parse_hex_transaction_test.dart
- test/ci/evm/parsing/reverse-hash-computation_test.dart
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Static code analysis and run unit tests
🔇 Additional comments (1)
lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart (1)
142-156: LGTM! Clean RLP serialization implementation.The serialization logic correctly uses typed RLP classes for better type safety and clarity.
| final rlpDecoded = decodeRLP(rawTxHex.hexToBytes).$1; | ||
|
|
||
| if (rlpDecoded is! List) { | ||
| if (rlpDecoded is! RLPList) { | ||
| throw Exception("Error RLP decoding transaction: $rlpDecoded"); | ||
| } | ||
|
|
||
| final decodedList = rlpDecoded.whereType<String>().toList(); | ||
|
|
||
| if (decodedList.length < 6) { | ||
| throw Exception("Invalid transaction, missing fields: $decodedList"); | ||
| if (rlpDecoded.length < 6) { | ||
| throw Exception("Invalid transaction, missing fields: $rlpDecoded"); | ||
| } | ||
|
|
||
| final nonce = parseAsHexBigInt(decodedList[0]); | ||
| final gasPrice = parseAsHexBigInt(decodedList[1]); | ||
| final gasLimit = parseAsHexBigInt(decodedList[2]); | ||
| final to = "0x" + decodedList[3]; | ||
| final value = parseAsHexBigInt(decodedList[4]); | ||
| final data = decodedList[5].hexToBytes; | ||
|
|
||
| final chainId = | ||
| decodedList.length > 6 ? parseAsHexInt(decodedList[6]) : null; | ||
| final nonce = rlpDecoded[0].buffer.toBigInt(); | ||
| final gasPrice = rlpDecoded[1].buffer.toBigInt(); | ||
| final gasLimit = rlpDecoded[2].buffer.toBigInt(); | ||
| final to = "0x" + rlpDecoded[3].hex; | ||
| final value = rlpDecoded[4].buffer.toBigInt(); | ||
| final data = rlpDecoded[5].buffer; | ||
| final chainId = rlpDecoded.length > 6 ? rlpDecoded[6].buffer.toInt() : null; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add hex normalization to prevent leading zeros.
The RLP decoding should normalize hex strings to prevent issues with leading zeros. This aligns with the PR's objective.
Apply this diff to normalize the hex values:
- final nonce = rlpDecoded[0].buffer.toBigInt();
- final gasPrice = rlpDecoded[1].buffer.toBigInt();
- final gasLimit = rlpDecoded[2].buffer.toBigInt();
- final to = "0x" + rlpDecoded[3].hex;
- final value = rlpDecoded[4].buffer.toBigInt();
+ final nonce = rlpDecoded[0].buffer.toBigInt().toString().replaceFirst(RegExp(r'^0+'), '');
+ final gasPrice = rlpDecoded[1].buffer.toBigInt().toString().replaceFirst(RegExp(r'^0+'), '');
+ final gasLimit = rlpDecoded[2].buffer.toBigInt().toString().replaceFirst(RegExp(r'^0+'), '');
+ final to = "0x" + rlpDecoded[3].hex.replaceFirst(RegExp(r'^0+'), '');
+ final value = rlpDecoded[4].buffer.toBigInt().toString().replaceFirst(RegExp(r'^0+'), '');Committable suggestion skipped: line range outside the PR's diff.
| rawTxHex = rawTxHex.substring(2); // Remove the type prefix | ||
|
|
||
| final rlpDecoded = decodeRLP(rawTxHex.hexToBytes, 0).result; | ||
| final rlpDecoded = decodeRLP(rawTxHex.hexToBytes).$1; |
There was a problem hiding this comment.
Fix incorrect error message in Type 2 transaction validation.
The assertion message incorrectly refers to "Type 1 Transaction" when validating a Type 2 transaction.
Apply this diff to fix the error message:
- assert(rawTxHex.startsWith("02"), "Invalid Type 1 Transaction");
+ assert(rawTxHex.startsWith("02"), "Invalid Type 2 Transaction");Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
lib/src/crypto/evm/utils/rlp.dart (1)
75-77:⚠️ Potential issueFix zero handling in RLPBigInt.
According to RLP specification, zero should be encoded as an empty array, not as
[0x80].- if (value == BigInt.zero) { - return Uint8List.fromList([0x80]); - } + if (value == BigInt.zero) { + return Uint8List(0); + }lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart (1)
645-645:⚠️ Potential issueFix incorrect transaction type in error message.
The error message refers to Type 1 transaction when validating a Type 2 transaction.
- assert(rawTxHex.startsWith("02"), "Invalid Type 1 Transaction"); + assert(rawTxHex.startsWith("02"), "Invalid Type 2 Transaction");
🧹 Nitpick comments (7)
lib/src/crypto/tron/entities/tron_transaction_utils.dart (1)
203-205: Consider consolidating signature component handling.While the conversion to
toBytesUnsignedis correct, consider refactoring this to use a helper method for consistency across the codebase.- final r = padUint8ListTo32(sig.r.toBytesUnsigned); - final s = padUint8ListTo32(sig.s.toBytesUnsigned); - final v = (BigInt.from(sig.v)).toBytesUnsigned; + final signatureBytes = encodeSignatureComponents(sig); + return signatureBytes; +Uint8List encodeSignatureComponents(Signature sig) { + final r = padUint8ListTo32(sig.r.toBytesUnsigned); + final s = padUint8ListTo32(sig.s.toBytesUnsigned); + final v = (BigInt.from(sig.v)).toBytesUnsigned; + return uint8ListFromList(r + s + v); +}lib/src/crypto/evm/utils/rlp.dart (4)
24-24: Add null check in hex getter.The
hexgetter should handle the case wherebufferis null.- String get hex => buffer.toHex; + String get hex => buffer?.toHex ?? '';
50-55: Consider caching the buffer.The
buffergetter recalculates the value each time. Consider caching it for better performance.final class RLPInt extends RLPItem<int> { + Uint8List? _buffer; RLPInt(super.value); @override - Uint8List get buffer => arrayifyInteger(value); + Uint8List get buffer { + return _buffer ??= arrayifyInteger(value); + } }
134-141: Remove redundant zero handling.The zero handling here is redundant as it's already handled in the respective RLP item classes.
- /// Check for int = 0 - if (input is RLPInt && buffer.length == 1 && input.value == 0) { - return buffer; - } - - /// Check for bigint = 0 - if (input is RLPBigInt && buffer.length == 1 && input.value == BigInt.zero) { - return buffer; - }
203-203: Fix typo in comment.The word "stric" should be "strict".
- // For Root Lists we can enforce a stric length check + // For Root Lists we can enforce a strict length checklib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart (2)
214-219: Improve error messages.The error messages could be more descriptive to help with debugging.
- if (rlpDecoded is! RLPList) { - throw Exception("Error RLP decoding transaction: $rlpDecoded"); - } - - if (rlpDecoded.length < 6) { - throw Exception("Invalid transaction, missing fields: $rlpDecoded"); + if (rlpDecoded is! RLPList) { + throw Exception("Expected RLP list, got: ${rlpDecoded.runtimeType}"); + } + + if (rlpDecoded.length < 6) { + throw Exception("Legacy transaction requires at least 6 fields, got: ${rlpDecoded.length}"); }
506-522: Simplify access list parsing.The access list parsing logic can be simplified using pattern matching.
- accessList: switch (rlpDecoded[7]) { - RLPList list => list.value - .whereType<RLPList>() - .map((item) { - final subList = item[1]; - if (subList is RLPList) - return ( - address: "0x" + item[0].buffer.toHex, - storageKeys: - subList.value.map((key) => key.buffer.toHex).toList(), - ); - return null; - }) - .nonNulls - .toList(), - _ => [], - }, + accessList: (rlpDecoded[7] as RLPList?)?.value + .whereType<RLPList>() + .map((item) => ( + address: "0x" + item[0].buffer.toHex, + storageKeys: (item[1] as RLPList) + .value + .map((key) => key.buffer.toHex) + .toList(), + )) + .toList() ?? + [],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
lib/src/crypto/evm/entities/contract/parameter_type/int_parameter_type.dart(1 hunks)lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart(15 hunks)lib/src/crypto/evm/entities/raw_transaction/signature.dart(1 hunks)lib/src/crypto/evm/utils/rlp.dart(1 hunks)lib/src/crypto/tron/entities/tron_parameter_type.dart(1 hunks)lib/src/crypto/tron/entities/tron_transaction_utils.dart(2 hunks)lib/src/domain/extensions.dart(2 hunks)test/ci/evm/parsing/parse_hex_transaction_test.dart(2 hunks)test/ci/rlp/rlp_test.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- test/ci/evm/parsing/parse_hex_transaction_test.dart
- lib/src/domain/extensions.dart
- test/ci/rlp/rlp_test.dart
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test/ci/tron tests
- GitHub Check: test/ci/proof_of_payment tests
- GitHub Check: test/ci/fetching tests
- GitHub Check: test/ci/evm tests
🔇 Additional comments (6)
lib/src/crypto/tron/entities/tron_parameter_type.dart (2)
48-48: LGTM! Improved byte conversion method.The change from
bigIntToBytestotoBytesUnsignedaligns with the PR's objective of preventing leading zeros in hex representation.
51-51: Verify endianness handling.The removal of explicit endianness parameter might affect how the bytes are interpreted. Please ensure this change is consistent with the expected byte order in the Tron protocol.
lib/src/crypto/evm/entities/raw_transaction/signature.dart (1)
27-28: Verify signature byte order consistency.The removal of explicit endianness parameters in signature component conversion should be verified against EVM specifications to ensure compatibility.
✅ Verification successful
Signature byte order handling is correctly implemented
The current implementation maintains EVM signature compatibility:
- Test cases verify correct round-trip serialization/deserialization
- Component extraction preserves the standard EVM signature format (32-byte r, 32-byte s, 1-byte v)
- Signature creation and verification work as expected in tests
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for signature-related byte order handling in EVM code rg "Signature.*fromBytes" --type dart -A 5 ast-grep --pattern 'factory $_.fromBytes($_) { $$$}'Length of output: 1987
Script:
#!/bin/bash # Search for toBigInt extension implementation rg "extension.*toBigInt" --type dart -A 5 # Search for signature test files fd "signature.*test" --extension dart --exec cat {} # Search for any EVM signature encoding/decoding in tests rg "test.*signature.*encoding|test.*signature.*decoding" --type dart -A 5Length of output: 34405
lib/src/crypto/tron/entities/tron_transaction_utils.dart (1)
99-99: LGTM! Consistent byte conversion method.The change to
toBytesUnsignedfor block number conversion aligns with the standardized approach for hex normalization.lib/src/crypto/evm/entities/contract/parameter_type/int_parameter_type.dart (1)
31-31: Verify EVM parameter decoding compatibility.The simplified
toBigIntusage should be verified against EVM ABI specifications to ensure correct parameter decoding.✅ Verification successful
toBigIntimplementation verified as EVM-compatibleThe implementation correctly uses pointycastle's
decodeBigIntfor big-endian decoding, which aligns with EVM ABI specifications. The extension is part of a comprehensive parameter type system and is consistently used across the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar parameter decoding patterns ast-grep --pattern 'decode($_) { return $_.toBigInt; }' rg "toBigInt.*littleEndian" --type dartLength of output: 99
Script:
#!/bin/bash # Search for toBigInt extension and other parameter type implementations ast-grep --pattern 'extension $_ on Uint8List { $$ toBigInt $$}' fd "parameter_type" --type f --exec cat {} rg "class.*ParameterType.*decode" -A 5Length of output: 37281
Script:
#!/bin/bash # Search for toBigInt extension and tests rg "extension.*toBigInt" --type dart fd "test.*int.*parameter" --type f --exec cat {}Length of output: 91
Script:
#!/bin/bash # Search for extensions and BigInt conversions fd "extensions?.dart" --type f --exec cat {} rg "BigInt.*from" --type dart fd "test" --type f -e dart --exec grep -l "ParameterType" {} \;Length of output: 23063
lib/src/crypto/evm/utils/rlp.dart (1)
7-15: LGTM!Clean and standard implementation of an exception class.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
lib/src/domain/extensions.dart (3)
32-32: Remove commented out code.If this method is no longer needed, it should be removed rather than left commented out.
64-71: Consider using pointycastle's decodeBigInt for better performance.The current manual BigInt construction is less efficient than using the crypto library's implementation.
BigInt get toUBigInt { - BigInt result = BigInt.zero; - for (var i = 0; i < length; i++) { - result = result * BigInt.from(256) + BigInt.from(this[i]); - } - return result; + return p_utils.decodeBigIntWithSign(1, this); }
76-76: Document the purpose of allowing malformed UTF-8.Add documentation explaining when to use this method versus strict UTF-8 decoding, and the implications of allowing malformed input.
+ /// Decodes the byte array as UTF-8, allowing malformed sequences. + /// Use this only when dealing with potentially corrupted or partial UTF-8 data. + /// For strict UTF-8 validation, use utf8.decode(this) instead. String get bytesToUTF8 => utf8.decode(this, allowMalformed: true);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
lib/src/crypto/evm/entities/contract/parameter_type/int_parameter_type.dart(1 hunks)lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart(15 hunks)lib/src/crypto/evm/entities/raw_transaction/signature.dart(1 hunks)lib/src/crypto/tron/entities/tron_parameter_type.dart(1 hunks)lib/src/domain/extensions.dart(1 hunks)test/ci/evm/parsing/fetch_function_signature_test.dart(1 hunks)test/ci/evm/parsing/reverse-hash-computation_test.dart(2 hunks)test/ci/rlp/rlp_test.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- lib/src/crypto/evm/entities/raw_transaction/signature.dart
- test/ci/evm/parsing/reverse-hash-computation_test.dart
- lib/src/crypto/evm/entities/contract/parameter_type/int_parameter_type.dart
- test/ci/evm/parsing/fetch_function_signature_test.dart
- test/ci/rlp/rlp_test.dart
- lib/src/crypto/tron/entities/tron_parameter_type.dart
- lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart
🔇 Additional comments (2)
lib/src/domain/extensions.dart (2)
17-24: Well-structured BigInt encoding methods!Good practice using distinct methods for signed and unsigned conversions with proper input validation.
17-71: Verify consistency with RLP encoding implementation.These number encoding/decoding changes affect RLP implementation. Let's verify the consistency across the codebase.
✅ Verification successful
RLP implementation is compatible with the extensions changes
The changes in the extensions file are safe and consistent with the RLP implementation. The only extension method used by RLP (
toBytesUnsigned) remains unchanged, and the commented-out or newly added methods don't affect the RLP encoding/decoding flow.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for RLP-related files and their number handling echo "Searching for RLP implementations..." rg -l "class.*RLP" . echo "Checking number encoding/decoding in RLP..." rg "to(U)?BigInt|toBytesUnsigned|encode.*Int" $(rg -l "class.*RLP" .)Length of output: 385
Script:
#!/bin/bash # Get the full RLP implementation echo "Full RLP implementation:" cat ./lib/src/crypto/evm/utils/rlp.dart echo -e "\nChecking usages of the extensions in RLP:" rg "extension.*on.*BigInt|extension.*on.*List<int>|extension.*on.*Uint8List" ./lib/src/crypto/evm/utils/rlp.dartLength of output: 7460
| /// Decode a unsigned big-endian integer from the buffer. | ||
| int get toUInt { | ||
| int result = 0; | ||
| for (var i = 0; i < length; i++) { | ||
| result = result * 256 + this[i]; | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
Add overflow protection to toUInt.
The current implementation could overflow for large byte arrays. Consider adding a size check to prevent overflow of Dart's int type.
int get toUInt {
+ if (length > 8) {
+ throw ArgumentError('Integer overflow: byte array too long');
+ }
int result = 0;
for (var i = 0; i < length; i++) {
result = result * 256 + this[i];
}
return result;
}📝 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.
| /// Decode a unsigned big-endian integer from the buffer. | |
| int get toUInt { | |
| int result = 0; | |
| for (var i = 0; i < length; i++) { | |
| result = result * 256 + this[i]; | |
| } | |
| return result; | |
| } | |
| /// Decode a unsigned big-endian integer from the buffer. | |
| int get toUInt { | |
| if (length > 8) { | |
| throw ArgumentError('Integer overflow: byte array too long'); | |
| } | |
| int result = 0; | |
| for (var i = 0; i < length; i++) { | |
| result = result * 256 + this[i]; | |
| } | |
| return result; | |
| } |
| /// Decode a big-endian integer from the buffer. | ||
| /// Can be positive or negative. | ||
| int get toInt { | ||
| bool negative = length > 0 && (this[0] & 0x80) == 0x80; | ||
| int result = 0; | ||
|
|
||
| var asHex = hex.encode(buf.toList()); | ||
| for (var i = 0; i < length; i++) { | ||
| result = result * 256 + this[i]; | ||
| } | ||
|
|
||
| return BigInt.parse(asHex, radix: 16); | ||
| return negative ? -(~result + 1) : result; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add overflow protection to toInt and improve readability.
The implementation correctly handles signed integers but needs overflow protection and could benefit from better readability.
int get toInt {
+ if (length > 8) {
+ throw ArgumentError('Integer overflow: byte array too long');
+ }
bool negative = length > 0 && (this[0] & 0x80) == 0x80;
int result = 0;
for (var i = 0; i < length; i++) {
result = result * 256 + this[i];
}
- return negative ? -(~result + 1) : result;
+ if (negative) {
+ // Convert from two's complement
+ return -(~result + 1);
+ }
+ return result;
}📝 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.
| /// Decode a big-endian integer from the buffer. | |
| /// Can be positive or negative. | |
| int get toInt { | |
| bool negative = length > 0 && (this[0] & 0x80) == 0x80; | |
| int result = 0; | |
| var asHex = hex.encode(buf.toList()); | |
| for (var i = 0; i < length; i++) { | |
| result = result * 256 + this[i]; | |
| } | |
| return BigInt.parse(asHex, radix: 16); | |
| return negative ? -(~result + 1) : result; | |
| } | |
| /// Decode a big-endian integer from the buffer. | |
| /// Can be positive or negative. | |
| int get toInt { | |
| if (length > 8) { | |
| throw ArgumentError('Integer overflow: byte array too long'); | |
| } | |
| bool negative = length > 0 && (this[0] & 0x80) == 0x80; | |
| int result = 0; | |
| for (var i = 0; i < length; i++) { | |
| result = result * 256 + this[i]; | |
| } | |
| if (negative) { | |
| // Convert from two's complement | |
| return -(~result + 1); | |
| } | |
| return result; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
lib/src/crypto/evm/utils/rlp.dart (2)
69-79:⚠️ Potential issueFix zero handling in RLPBigInt.
The zero handling in RLPBigInt returns
[0x80]for zero value, which may not align with RLP specification.@override Uint8List get buffer { if (value == BigInt.zero) { - return Uint8List.fromList([0x80]); + return Uint8List(0); } return this.value.toBytesUnsigned; }
281-299:⚠️ Potential issueFix zero handling in arrayifyInteger.
The zero handling returns
[0x80]which may not align with RLP specification.Uint8List arrayifyInteger(int value) { if (value == 0) { - return Uint8List.fromList([0x80]); + return Uint8List(0); }
🧹 Nitpick comments (1)
lib/src/crypto/evm/utils/rlp.dart (1)
170-279: Fix typos and reduce code duplication.
- Fix typo in comments: "stric" → "strict"
- Extract common length validation logic
+ void validateLength(int totalLength, String message) { + if (offset + totalLength > input.length) { + throw RLPException(message); + } + // For Root Lists we can enforce a strict length check + if (offset == 0 && offset + totalLength != input.length) { + throw RLPException("Given input is longer than specified length"); + } + } + switch (firstByte) { case >= 0xf8: final lengthLength = firstByte - 0xf7; if (offset + 1 + lengthLength > input.length) { throw RLPException("insufficient data for length bytes"); } final length = input.sublist(offset + 1, offset + 1 + lengthLength).toUInt; if (length < 56) { throw RLPException("encoded list too short"); } final totalLength = 1 + lengthLength + length; - if (offset + totalLength > input.length) { - throw RLPException("insufficient data length"); - } - - // For Root Lists we can enforce a stric length check - if (offset == 0) { - if (offset + totalLength != input.length) { - throw RLPException("Given input is longer than specified length"); - } - } + validateLength(totalLength, "insufficient data length");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/src/crypto/evm/utils/rlp.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test/ci/tron tests
- GitHub Check: test/ci/proof_of_payment tests
- GitHub Check: test/ci/fetching tests
- GitHub Check: test/ci/evm tests
🔇 Additional comments (2)
lib/src/crypto/evm/utils/rlp.dart (2)
6-14: LGTM! Clean exception implementation.The exception class follows Dart best practices with proper interface implementation and error reporting.
16-46: LGTM! Well-structured type hierarchy.The sealed class hierarchy with generic type parameter provides good type safety and extensibility.
| final class RLPList extends RLPItem<List<RLPItem>> { | ||
| RLPList(super.value); | ||
|
|
||
| @override | ||
| Uint8List get buffer => | ||
| Uint8List.fromList([for (final item in value) ...encodeRLP(item)]); | ||
|
|
||
| int get length => value.length; | ||
|
|
||
| RLPItem operator [](int index) { | ||
| return value[index]; | ||
| } | ||
|
|
||
| return "0x" + result; | ||
| } | ||
| List<String> get hexValues => value.map((e) => e.hex).toList(); | ||
|
|
||
| /** | ||
| * @param {Uint8List} data | ||
| * @param {int} offset | ||
| * | ||
| * This function is the main RLP decoding logic. It recursively decodes RLP-encoded data. | ||
| It checks the first byte of the data to determine the type of encoding (long item, list, long string, short string). | ||
| Depending on the type, it calls _decodeChildren to handle the decoding recursively. | ||
| The decoded result is stored in a List<dynamic>. | ||
| The function returns a Decoded object containing the consumed bytes and the result. | ||
| * | ||
| */ | ||
| DecodedRLP decodeRLP(Uint8List data, int offset) { | ||
| DecodedRLP _decodeChildren( | ||
| Uint8List data, int offset, int childOffset, int length) { | ||
| List<dynamic> result = []; | ||
| @override | ||
| String get hex => "[" + hexValues.join(", ") + "]"; | ||
|
|
||
| factory RLPList.fromBuffer( | ||
| Uint8List buffer, | ||
| int offset, | ||
| int childOffset, | ||
| int length, | ||
| ) { | ||
| final items = <RLPItem>[]; | ||
| while (childOffset < offset + 1 + length) { | ||
| DecodedRLP decoded = decodeRLP(data, childOffset); | ||
| final (item, consumed) = decodeRLP(buffer, offset: childOffset); | ||
| items.add(item); | ||
| childOffset += consumed; | ||
| } | ||
| return RLPList(items); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance RLPList implementation with validation.
The RLPList.fromBuffer factory constructor needs additional validation:
- Verify that childOffset doesn't exceed buffer length
- Ensure the total consumed bytes match the expected length
factory RLPList.fromBuffer(
Uint8List buffer,
int offset,
int childOffset,
int length,
) {
+ if (childOffset >= buffer.length) {
+ throw RLPException('Child offset exceeds buffer length');
+ }
final items = <RLPItem>[];
+ var totalConsumed = 0;
while (childOffset < offset + 1 + length) {
final (item, consumed) = decodeRLP(buffer, offset: childOffset);
items.add(item);
childOffset += consumed;
+ totalConsumed += consumed;
}
+ if (totalConsumed != length) {
+ throw RLPException('Consumed bytes do not match expected length');
+ }
return RLPList(items);
}Committable suggestion skipped: line range outside the PR's diff.
| Uint8List encodeRLP(RLPItem input) { | ||
| final buffer = input.buffer; | ||
|
|
||
| childOffset += decoded.consumed; | ||
| } | ||
| /// Check for int = 0 | ||
| if (input is RLPInt && buffer.length == 1 && input.value == 0) { | ||
| return buffer; | ||
| } | ||
|
|
||
| return DecodedRLP(consumed: (1 + length), result: result); | ||
| /// Check for bigint = 0 | ||
| if (input is RLPBigInt && buffer.length == 1 && input.value == BigInt.zero) { | ||
| return buffer; | ||
| } | ||
| if (input is! RLPList && buffer.length == 1 && buffer[0] < 128) { | ||
| return buffer; | ||
| } | ||
|
|
||
| if (data[offset] >= 0xf8) { | ||
| final lengthLength = data[offset] - 0xf7; | ||
| if (buffer.length < 56) { | ||
| final flag = switch (input) { | ||
| RLPList _ => 0xc0, | ||
| _ => 0x80, | ||
| }; | ||
|
|
||
| final length = unarrayifyInteger(data, offset + 1, lengthLength); | ||
| return Uint8List.fromList([flag + buffer.length, ...buffer]); | ||
| } | ||
|
|
||
| return _decodeChildren( | ||
| data, offset, offset + 1 + lengthLength, lengthLength + length); | ||
| } else if (data[offset] >= 0xc0) { | ||
| final length = data[offset] - 0xc0; | ||
| final lengthBuffer = arrayifyInteger(buffer.length); | ||
|
|
||
| return _decodeChildren(data, offset, offset + 1, length); | ||
| } else if (data[offset] >= 0xb8) { | ||
| final lengthLength = data[offset] - 0xb7; | ||
| final flag = switch (input) { | ||
| RLPList _ => 0xf7, | ||
| _ => 0xb7, | ||
| }; | ||
|
|
||
| final length = unarrayifyInteger(data, offset + 1, lengthLength); | ||
| return Uint8List.fromList( | ||
| [ | ||
| flag + lengthBuffer.length, | ||
| ...lengthBuffer, | ||
| ...buffer, | ||
| ], | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and fix zero handling inconsistency.
The encodeRLP function needs two improvements:
- Add validation for maximum input length (RLP spec limits)
- Ensure consistent zero handling across all types
Uint8List encodeRLP(RLPItem input) {
+ // RLP spec limits total encoded length to 2^64-1
+ if (input.buffer.length > (1 << 64) - 1) {
+ throw RLPException('Input too large for RLP encoding');
+ }
+
final buffer = input.buffer;
/// Check for int = 0
if (input is RLPInt && buffer.length == 1 && input.value == 0) {
- return buffer;
+ return Uint8List(0);
}
/// Check for bigint = 0
if (input is RLPBigInt && buffer.length == 1 && input.value == BigInt.zero) {
- return buffer;
+ return Uint8List(0);
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
lib/src/crypto/evm/utils/rlp.dart (4)
83-85:⚠️ Potential issueFix zero handling in RLPBigInt.
According to the RLP specification, zero should be encoded as an empty byte array.
Apply this diff to fix zero handling:
if (value == BigInt.zero) { - return Uint8List.fromList([0x80]); + return Uint8List(0); }
122-135:⚠️ Potential issueAdd bounds checking in RLPList.fromBuffer.
The method needs validation to prevent buffer overruns.
Apply this diff to add validation:
factory RLPList.fromBuffer( Uint8List buffer, int offset, int childOffset, int length, ) { + if (childOffset >= buffer.length) { + throw RLPException('Child offset exceeds buffer length'); + } final items = <RLPItem>[]; + var totalConsumed = 0; while (childOffset < offset + 1 + length) { final (item, consumed) = decodeRLP(buffer, offset: childOffset); items.add(item); childOffset += consumed; + totalConsumed += consumed; } + if (totalConsumed != length) { + throw RLPException('Consumed bytes do not match expected length'); + } return RLPList(items); }
138-177:⚠️ Potential issueAdd input validation and fix zero handling inconsistency.
The function needs two improvements:
- Add validation for maximum input length (RLP spec limits)
- Ensure consistent zero handling across all types
Apply this diff to fix the issues:
Uint8List encodeRLP(RLPItem input) { + // RLP spec limits total encoded length to 2^64-1 + if (input.buffer.length > (1 << 64) - 1) { + throw RLPException('Input too large for RLP encoding'); + } + final buffer = input.buffer; /// Check for int = 0 if (input is RLPInt && buffer.length == 1 && input.value == 0) { - return buffer; + return Uint8List(0); } /// Check for bigint = 0 if (input is RLPBigInt && buffer.length == 1 && input.value == BigInt.zero) { - return buffer; + return Uint8List(0); }
298-308:⚠️ Potential issueFix zero handling in arrayifyInteger.
The function's zero handling doesn't match the RLP specification.
Apply this diff to fix zero handling:
Uint8List arrayifyInteger(int value) { if (value == 0) { - return Uint8List.fromList([0x80]); + return Uint8List(0); }lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart (1)
649-649:⚠️ Potential issueFix incorrect error message in Type 2 transaction validation.
The assertion message incorrectly refers to "Type 1 Transaction" when validating a Type 2 transaction.
Apply this diff to fix the error message:
- assert(rawTxHex.startsWith("02"), "Invalid Type 1 Transaction"); + assert(rawTxHex.startsWith("02"), "Invalid Type 2 Transaction");
🧹 Nitpick comments (4)
lib/src/crypto/evm/utils/rlp.dart (2)
208-213: Fix typo in comments and improve error messages.The comments contain a typo ("stric" should be "strict") and the error messages could be more descriptive.
Apply this diff to fix the issues:
- // For Root Lists we can enforce a stric length check + // For Root Lists we can enforce a strict length check if (offset == 0) { if (offset + totalLength != input.length) { - throw RLPException("Given input is longer than specified length"); + throw RLPException("Input exceeds expected length: got ${input.length} bytes, expected ${offset + totalLength}"); } }Also applies to: 231-236
290-297: Convert JSDoc to Dart documentation style.The function uses JSDoc style comments which should be converted to Dart's documentation style.
Apply this diff to fix the documentation:
-/** - * - * @param {int} value - * - * This function takes an integer value and converts it to a Uint8List. - * - * @returns {Uint8List} - */ +/// Converts an integer value to its byte representation. +/// +/// The returned bytes are in big-endian order. +/// Returns an empty list for zero values as per RLP spec.lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart (2)
141-155: Add documentation for the serialization format.The RLP encoding structure should be documented to help maintainers understand the transaction format.
Add documentation above the serialized getter:
+ /// Returns the RLP encoded transaction with the following structure: + /// [nonce, gasPrice, gasLimit, to, value, data, v, r, s] + /// where: + /// - nonce: Transaction nonce + /// - gasPrice: Gas price in wei + /// - gasLimit: Maximum gas allowed + /// - to: Recipient address + /// - value: Amount in wei + /// - data: Transaction data + /// - v: Recovery ID + chainId * 2 + 35 + /// - r, s: Signature components @override Uint8List get serialized {
510-526: Extract duplicated access list parsing logic.The access list parsing logic is duplicated across Type 1 and Type 2 transactions.
Create a utility function to handle access list parsing:
List<AccessListItem> parseAccessList(RLPItem item) { return switch (item) { RLPList list => list.value .whereType<RLPList>() .map((item) { final subList = item[1]; if (subList is RLPList) return ( address: "0x" + item[0].buffer.toHex, storageKeys: subList.value.map((key) => key.buffer.toHex).toList(), ); return null; }) .nonNulls .toList(), _ => [], }; }Then use it in both transaction types:
- accessList: switch (rlpDecoded[7]) { - RLPList list => list.value - .whereType<RLPList>() - .map((item) { - final subList = item[1]; - if (subList is RLPList) - return ( - address: "0x" + item[0].buffer.toHex, - storageKeys: - subList.value.map((key) => key.buffer.toHex).toList(), - ); - return null; - }) - .nonNulls - .toList(), - _ => [], - }, + accessList: parseAccessList(rlpDecoded[7]),Also applies to: 670-686, 714-730
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/src/crypto/evm/entities/raw_transaction/raw_evm_transaction.dart(15 hunks)lib/src/crypto/evm/utils/rlp.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test/ci/tron tests
- GitHub Check: test/ci/proof_of_payment tests
- GitHub Check: test/ci/fetching tests
- GitHub Check: test/ci/evm tests
🔇 Additional comments (1)
lib/src/crypto/evm/utils/rlp.dart (1)
6-14: LGTM! Clean exception class implementation.The RLPException class follows best practices with proper interface implementation and error message handling.
| if (value.startsWith("#")) { | ||
| return RLPBigInt(BigInt.parse(value.substring(1))); | ||
| } |
There was a problem hiding this comment.
Add input validation for BigInt parsing.
The string to BigInt conversion could fail if the substring after '#' is not a valid number.
Apply this diff to add validation:
if (value.startsWith("#")) {
+ final numStr = value.substring(1);
+ if (!RegExp(r'^[0-9]+$').hasMatch(numStr)) {
+ throw ArgumentError("Invalid BigInt format: $value");
+ }
return RLPBigInt(BigInt.parse(value.substring(1)));
}📝 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.
| if (value.startsWith("#")) { | |
| return RLPBigInt(BigInt.parse(value.substring(1))); | |
| } | |
| if (value.startsWith("#")) { | |
| final numStr = value.substring(1); | |
| if (!RegExp(r'^[0-9]+$').hasMatch(numStr)) { | |
| throw ArgumentError("Invalid BigInt format: $value"); | |
| } | |
| return RLPBigInt(BigInt.parse(value.substring(1))); | |
| } |
…extraction (check for leading 0)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
lib/src/crypto/evm/utils/rlp.dart (4)
32-34:⚠️ Potential issueAdd input validation for BigInt parsing.
The string to BigInt conversion could fail if the substring after '#' is not a valid number.
Apply this diff to add validation:
if (value.startsWith("#")) { + final numStr = value.substring(1); + if (!RegExp(r'^[0-9]+$').hasMatch(numStr)) { + throw ArgumentError("Invalid BigInt format: $value"); + } return RLPBigInt(BigInt.parse(value.substring(1))); }
122-134: 🛠️ Refactor suggestionAdd validation in fromBuffer factory.
The
fromBufferfactory constructor needs validation for:
- childOffset exceeding buffer length
- total consumed bytes matching expected length
Apply this diff to add validation:
factory RLPList.fromBuffer( Uint8List buffer, int offset, int childOffset, int length, ) { + if (childOffset >= buffer.length) { + throw RLPException('Child offset exceeds buffer length'); + } final items = <RLPItem>[]; + var totalConsumed = 0; while (childOffset < offset + 1 + length) { final (item, consumed) = decodeRLP(buffer, offset: childOffset); items.add(item); childOffset += consumed; + totalConsumed += consumed; } + if (totalConsumed != length) { + throw RLPException('Consumed bytes do not match expected length'); + } return RLPList(items); }
138-176: 🛠️ Refactor suggestionAdd input validation and fix zero handling inconsistency.
The function needs:
- Validation for maximum input length (RLP spec limits)
- Consistent zero handling across all types
Apply this diff to add validation:
Uint8List encodeRLP(RLPItem input) { + // RLP spec limits total encoded length to 2^64-1 + if (input.buffer.length > (1 << 64) - 1) { + throw RLPException('Input too large for RLP encoding'); + } + final buffer = input.buffer; /// Check for int = 0 if (input is RLPInt && buffer.length == 1 && input.value == 0) { - return buffer; + return Uint8List(0); } /// Check for bigint = 0 if (input is RLPBigInt && buffer.length == 1 && input.value == BigInt.zero) { - return buffer; + return Uint8List(0); }
293-304:⚠️ Potential issueValidate the implementation of
arrayifyIntegerfunction.The function may not correctly handle the encoding of zero according to the RLP specification.
Apply this diff to adjust the handling of zero:
- if (value == 0) { - return Uint8List.fromList([0x80]); - } + if (value == 0) { + return Uint8List(0); + }test/ci/rlp/rlp_test.dart (1)
530-539: 🛠️ Refactor suggestionImprove exception handling in tests.
The current exception handling in invalid tests could mask unexpected errors. Instead of asserting
true, verify the specific exception type.Apply this diff to improve the test:
- try { - final result = decodeRLP(out.hexToBytes); - print(result); - fail("Expected to throw but didn't"); - } on TestFailure { - fail("Expected to throw but didn't"); - } catch (e) { - print("$e"); - expect(true, true); - } + expect( + () => decodeRLP(out.hexToBytes), + throwsA(isA<RLPException>()), + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/src/crypto/evm/utils/rlp.dart(1 hunks)test/ci/rlp/rlp_test.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test/ci/tron tests
- GitHub Check: test/ci/proof_of_payment tests
- GitHub Check: test/ci/fetching tests
- GitHub Check: test/ci/evm tests
🔇 Additional comments (6)
lib/src/crypto/evm/utils/rlp.dart (5)
6-14: LGTM!The
RLPExceptionclass is well-implemented with proper message handling and string representation.
49-54: Verify zero handling in buffer getter.The
buffergetter usesarrayifyIntegerwhich may not correctly handle zero values. Please ensure this is fixed when addressing the zero handling issue inarrayifyInteger.
90-95: LGTM!The
RLPNullclass is well-implemented with proper null handling.
97-102: LGTM!The
RLPBytesclass is well-implemented with proper byte array handling.
306-315: LGTM!The
decodeLengthfunction correctly validates leading zeros and handles length decoding according to the RLP specification.test/ci/rlp/rlp_test.dart (1)
579-587: 🛠️ Refactor suggestionUse consistent exception handling pattern.
Similar to the previous comment, improve the exception handling in invalid Geth cases.
Apply this diff to improve the test:
- try { - final _ = decodeRLP(input.hexToBytes).$1; - - fail("Expected to throw but didn't"); - } on TestFailure { - fail("Expected to throw but didn't"); - } catch (e) { - expect(true, true); - } + expect( + () => decodeRLP(input.hexToBytes), + throwsA(isA<RLPException>()), + );Likely invalid or redundant comment.
| if (value == BigInt.zero) { | ||
| return Uint8List.fromList([0x80]); | ||
| } |
There was a problem hiding this comment.
Ensure consistent zero handling.
The zero handling in RLPBigInt should be consistent with other RLP types. Currently, it returns [0x80] for zero, which may not align with the RLP specification.
Apply this diff to align with the specification:
if (value == BigInt.zero) {
- return Uint8List.fromList([0x80]);
+ return Uint8List(0);
}📝 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.
| if (value == BigInt.zero) { | |
| return Uint8List.fromList([0x80]); | |
| } | |
| if (value == BigInt.zero) { | |
| return Uint8List(0); | |
| } |
| final class RLPString extends RLPItem<String> { | ||
| final bool? isHex; | ||
|
|
||
| RLPString(super.value, {this.isHex}); | ||
|
|
||
| @override | ||
| Uint8List get buffer { | ||
| if (isHex == true) { | ||
| if (value.startsWith("0x")) { | ||
| return value.substring(2).hexToBytes; | ||
| } | ||
| return value.hexToBytes; | ||
| } | ||
|
|
||
| if (value.startsWith("0x")) { | ||
| return value.substring(2).hexToBytes; | ||
| } | ||
|
|
||
| return utf8.encode(value); | ||
| } | ||
| List<int> result = []; | ||
| while (value > 0) { | ||
| result.insert(0, value & 0xff); | ||
| value >>= 8; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add hex string validation.
The buffer getter should validate hex strings before conversion to prevent runtime errors.
Apply this diff to add validation:
@override
Uint8List get buffer {
if (isHex == true) {
+ final hexStr = value.startsWith("0x") ? value.substring(2) : value;
+ if (!RegExp(r'^[0-9a-fA-F]*$').hasMatch(hexStr)) {
+ throw ArgumentError('Invalid hex string');
+ }
if (value.startsWith("0x")) {
return value.substring(2).hexToBytes;
}
return value.hexToBytes;
}
if (value.startsWith("0x")) {
+ final hexStr = value.substring(2);
+ if (!RegExp(r'^[0-9a-fA-F]*$').hasMatch(hexStr)) {
+ throw ArgumentError('Invalid hex string');
+ }
return value.substring(2).hexToBytes;
}
return utf8.encode(value);
}📝 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.
| final class RLPString extends RLPItem<String> { | |
| final bool? isHex; | |
| RLPString(super.value, {this.isHex}); | |
| @override | |
| Uint8List get buffer { | |
| if (isHex == true) { | |
| if (value.startsWith("0x")) { | |
| return value.substring(2).hexToBytes; | |
| } | |
| return value.hexToBytes; | |
| } | |
| if (value.startsWith("0x")) { | |
| return value.substring(2).hexToBytes; | |
| } | |
| return utf8.encode(value); | |
| } | |
| List<int> result = []; | |
| while (value > 0) { | |
| result.insert(0, value & 0xff); | |
| value >>= 8; | |
| } | |
| final class RLPString extends RLPItem<String> { | |
| final bool? isHex; | |
| RLPString(super.value, {this.isHex}); | |
| @override | |
| Uint8List get buffer { | |
| if (isHex == true) { | |
| final hexStr = value.startsWith("0x") ? value.substring(2) : value; | |
| if (!RegExp(r'^[0-9a-fA-F]*$').hasMatch(hexStr)) { | |
| throw ArgumentError('Invalid hex string'); | |
| } | |
| if (value.startsWith("0x")) { | |
| return value.substring(2).hexToBytes; | |
| } | |
| return value.hexToBytes; | |
| } | |
| if (value.startsWith("0x")) { | |
| final hexStr = value.substring(2); | |
| if (!RegExp(r'^[0-9a-fA-F]*$').hasMatch(hexStr)) { | |
| throw ArgumentError('Invalid hex string'); | |
| } | |
| return value.substring(2).hexToBytes; | |
| } | |
| return utf8.encode(value); | |
| } | |
| } |
* feat: add ERC1155 support with balance fetching and transfer function… (#133) * feat: add ERC1155 support with balance fetching and transfer functionality * feat: add ERC1155 export to EVM module * refactor: rename 'account' parameter to 'address' for consistency in ERC1155 balance functions * feat: add isErc1155 function to check ERC1155 token support and include optional token ID in TokenInfo * feat: add ERC1155 transaction fetching and balance handling in EVM module * fix: update tokenValue parsing to default to zero instead of -1 in EtherscanTransaction * Add Hex Normalization to prevent leading Zeros in RLP (#135) * Add Hex Normalization to prevent leading Zeros in RLP * Start of Reimplementing RLP & Testing * Implement all Test Cases and Decoding * Finish RawEvmTx Encoding and Decode and fix lint * Refactor Buffer to int/BigInt and vice versa * Refactor BigInt Conversion * Refactor RLP encoding to use sublist for length extraction * Remove unused import * Fix Access List Encoding & add isHex Parameter to RLPString to indicate hex * Add RLP test directory to GitHub Actions workflow * Add decodeRLPCheck function for input validation and refactor length extraction (check for leading 0) --------- Co-authored-by: dev2 <[email protected]> Co-authored-by: thomas.fercher <[email protected]> * FunctionParamWithValue JsonEncoding (#136) * initial * feat: implement JSON serialization and deserialization for function parameter types * Fix Lint --------- Co-authored-by: dev2 <[email protected]> * fix: update toJson method for EvmType2GasPrice to use proper serialization (#138) * Initial * Update PipeTests * Fix Tests * Fix Lint * Update GitHub Actions to use upload-artifact@v4 * Update Tests * Fix Lints --------- Co-authored-by: The Nomo App's core developer team <[email protected]> Co-authored-by: thomas.fercher <[email protected]>
Summary by CodeRabbit
New Features
Bug Fixes
Improvements