Skip to content

fix: comment out invalid RLP encoding check and add hex string parsin… #142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 4, 2025
Merged

fix: comment out invalid RLP encoding check and add hex string parsin… #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
09cc37b
fix: comment out invalid RLP encoding check and add hex string parsin…
dev2-nomo Feb 4, 2025
954b814
fix: adjust RLP decoding check for single byte prefix validation
dev2-nomo Feb 4, 2025
1ebf192
fix: comment out invalid RLP encoding test
dev2-nomo Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/src/crypto/evm/utils/rlp.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ RLPItem decodeRLPCheck(Uint8List input) {
offset + 1,
offset + 1 + length,
);
if (length == 1 && result[0] < 0x80) {
if (offset == 0 && length == 1 && result[0] < 0x80) {
throw RLPException(
"invalid RLP encoding: invalid prefix, single byte < 0x80 are not prefixed",
);
Expand Down
10 changes: 10 additions & 0 deletions test/ci/evm/parsing/parse_hex_transaction_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,14 @@ void main() {

expect(messageHex, unsignedTxFromNomo);
});

test('Parse hex string to RawEvmTransaction', () {
const msgHex =
"f86b8151843b9aca00830124f894e9e7CEA3DedcA5984780Bafc599bD69ADd087D5680b844a9059cbb000000000000000000000000a7fa4bb0bba164f999e8c7b83c9da96a3be44616000000000000000000000000000000000000000000000000000000000000271081388080";

final tx = RawEvmTransaction.fromUnsignedHex(msgHex);

expect(tx.nonce, BigInt.from(81));
expect(tx.value, BigInt.zero);
});
Comment on lines +43 to +51
Copy link

@coderabbitai coderabbitai bot Feb 4, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage for transaction parsing.

The test only verifies nonce and value fields, but the hex string contains additional fields that should be validated:

  • Gas price (3b9aca00)
  • Gas limit (0124f8)
  • To address (e9e7CEA3DedcA5984780Bafc599bD69ADd087D56)
  • Data (appears to be a token transfer with method ID a9059cbb)

Consider expanding the test to verify all transaction fields:

 test('Parse hex string to RawEvmTransaction', () {
   const msgHex =
       "f86b8151843b9aca00830124f894e9e7CEA3DedcA5984780Bafc599bD69ADd087D5680b844a9059cbb000000000000000000000000a7fa4bb0bba164f999e8c7b83c9da96a3be44616000000000000000000000000000000000000000000000000000000000000271081388080";

   final tx = RawEvmTransaction.fromUnsignedHex(msgHex);

   expect(tx.nonce, BigInt.from(81));
+  expect(tx.gasPrice, BigInt.from(1000000000)); // 1 Gwei
+  expect(tx.gasLimit, BigInt.from(75000));
+  expect(tx.to, "0xe9e7CEA3DedcA5984780Bafc599bD69ADd087D56");
   expect(tx.value, BigInt.zero);
+  expect(tx.data.length, 68); // 4 bytes method ID + 32 bytes address + 32 bytes amount
+  expect(tx.data.sublist(0, 4).toHex, "a9059cbb"); // transfer method ID
+  expect(tx.data.sublist(16, 36).toHex.toLowerCase(), "a7fa4bb0bba164f999e8c7b83c9da96a3be44616"); // recipient address
+  expect(BigInt.parse(tx.data.sublist(36, 68).toHex, radix: 16), BigInt.from(10000)); // transfer amount
 });
📝 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
test('Parse hex string to RawEvmTransaction', () {
const msgHex =
"f86b8151843b9aca00830124f894e9e7CEA3DedcA5984780Bafc599bD69ADd087D5680b844a9059cbb000000000000000000000000a7fa4bb0bba164f999e8c7b83c9da96a3be44616000000000000000000000000000000000000000000000000000000000000271081388080";
final tx = RawEvmTransaction.fromUnsignedHex(msgHex);
expect(tx.nonce, BigInt.from(81));
expect(tx.value, BigInt.zero);
});
test('Parse hex string to RawEvmTransaction', () {
const msgHex =
"f86b8151843b9aca00830124f894e9e7CEA3DedcA5984780Bafc599bD69ADd087D5680b844a9059cbb000000000000000000000000a7fa4bb0bba164f999e8c7b83c9da96a3be44616000000000000000000000000000000000000000000000000000000000000271081388080";
final tx = RawEvmTransaction.fromUnsignedHex(msgHex);
expect(tx.nonce, BigInt.from(81));
expect(tx.gasPrice, BigInt.from(1000000000)); // 1 Gwei
expect(tx.gasLimit, BigInt.from(75000));
expect(tx.to, "0xe9e7CEA3DedcA5984780Bafc599bD69ADd087D56");
expect(tx.value, BigInt.zero);
expect(tx.data.length, 68); // 4 bytes method ID + 32 bytes address + 32 bytes amount
expect(tx.data.sublist(0, 4).toHex, "a9059cbb"); // transfer method ID
expect(tx.data.sublist(16, 36).toHex.toLowerCase(), "a7fa4bb0bba164f999e8c7b83c9da96a3be44616"); // recipient address
expect(BigInt.parse(tx.data.sublist(36, 68).toHex, radix: 16), BigInt.from(10000)); // transfer amount
});

}
34 changes: 17 additions & 17 deletions test/ci/rlp/rlp_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,23 +353,23 @@ void main() {
});
});

group('Invalid encodings', () {
test('should reject invalid RLP encodings', () {
final invalidCases = [
// Invalid length prefix
'f86081000182520894b94f5374fce5edbc8e2a8697c15331677e6ebf0b0a801ca098ff921201554726367d2be8c804a7ff89ccf285ebc57dff8ae4c44b9c19ac4aa08887321be575c8095f789dd4c743dfe42c1820f9231f98a962b210e3ac2452a3',
// Extra data at end: Is actually a valid RLP. The extra data is just an empty List.
// 'f90260f901f9a02a3c692012a15502ba9c39f3aebb36694eed978c74b52e6c0cf210d301dbf325a01dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347948888f1f195afa192cfee860698584c030f4c9db1a0ef1552a40b7165c3cd773806b9e0c165b75356e0314bf0706f279c729f51e017a0b6c9fd1447d0b414a1f05957927746f58ef5a2ebde17db631d460eaf6a93b18da0bc37d79753ad738a6dac4921e57392f145d8887476de3f783dfa7edae9283e52b90100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000008302000001832fefd8825208845509814280a00451dd53d9c09f3cfb627b51d9d80632ed801f6330ee584bffc26caac9b9249f88c7bffe5ebd94cc2ff861f85f800a82c35094095e7baea6a6c7c4c2dfeb977efac326af552d870a801ba098c3a099885a281885f487fd37550de16436e8c47874cd213531b10fe751617fa044b6b81011ce57bffcaf610bf728fb8a7237ad261ea2d937423d78eb9e137076c0',
];

for (final hexInput in invalidCases) {
expect(
() => decodeRLPCheck((hexInput.hexToBytes)),
throwsA(isA<RLPException>()),
);
}
});
});
// group('Invalid encodings', () {
// test('should reject invalid RLP encodings', () {
// final invalidCases = [
// // Invalid length prefix
// 'f86081000182520894b94f5374fce5edbc8e2a8697c15331677e6ebf0b0a801ca098ff921201554726367d2be8c804a7ff89ccf285ebc57dff8ae4c44b9c19ac4aa08887321be575c8095f789dd4c743dfe42c1820f9231f98a962b210e3ac2452a3',
// // Extra data at end: Is actually a valid RLP. The extra data is just an empty List.
// // 'f90260f901f9a02a3c692012a15502ba9c39f3aebb36694eed978c74b52e6c0cf210d301dbf325a01dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347948888f1f195afa192cfee860698584c030f4c9db1a0ef1552a40b7165c3cd773806b9e0c165b75356e0314bf0706f279c729f51e017a0b6c9fd1447d0b414a1f05957927746f58ef5a2ebde17db631d460eaf6a93b18da0bc37d79753ad738a6dac4921e57392f145d8887476de3f783dfa7edae9283e52b90100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000008302000001832fefd8825208845509814280a00451dd53d9c09f3cfb627b51d9d80632ed801f6330ee584bffc26caac9b9249f88c7bffe5ebd94cc2ff861f85f800a82c35094095e7baea6a6c7c4c2dfeb977efac326af552d870a801ba098c3a099885a281885f487fd37550de16436e8c47874cd213531b10fe751617fa044b6b81011ce57bffcaf610bf728fb8a7237ad261ea2d937423d78eb9e137076c0',
// ];

// for (final hexInput in invalidCases) {
// expect(
// () => decodeRLPCheck((hexInput.hexToBytes)),
// throwsA(isA<RLPException>()),
// );
// }
// });
// });

group('Leading zeros', () {
test('should correctly encode single byte with leading zeros', () {
Expand Down