fix: eth_sendRawTransaction now handles malformed RLP data correctly #3765
Conversation
Signed-off-by: Simeon Nakov <simeon.nakov@limechain.tech>
Signed-off-by: Simeon Nakov <simeon.nakov@limechain.tech>
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
Test Results 22 files + 2 316 suites +61 35m 38s ⏱️ - 1m 41s For more details on these failures, see this check. Results for commit 2e41c24. ± Comparison against base commit 53b6777. ♻️ This comment has been updated with latest results. |
…ed behaviour Signed-off-by: Simeon Nakov <simeon.nakov@limechain.tech>
Signed-off-by: Simeon Nakov <simeon.nakov@limechain.tech>
Signed-off-by: Simeon Nakov <simeon.nakov@limechain.tech>
Signed-off-by: Simeon Nakov <simeon.nakov@limechain.tech>
Signed-off-by: Simeon Nakov <simeon.nakov@limechain.tech>
Signed-off-by: Simeon Nakov <simeon.nakov@limechain.tech>
| } catch (e: any) { | ||
| if (e.message.toString().includes('unexpected junk after rlp payload')) { | ||
| throw predefined.INVALID_ARGUMENTS(e.message.toString()); | ||
| } | ||
| throw predefined.INTERNAL_ERROR(e.message.toString()); | ||
| } | ||
| } |
There was a problem hiding this comment.
I’d suggest removing this overhead and simply returning INVALID_ARGUMENTS. At this point, the only operation that throws an error is the Transaction.from(), which functions as a parsing engine. If it fails, it indicates an issue with the transaction hash, which is the argument itself—so I believe there’s no need to use INTERNAL_ERROR here.
| const invalidTx = | ||
| '0xf8748201280585800e8dfc0085800e8dfc00832dc6c094aca85ef7e1fce27079bbf99b60fcf6fd19b99b248502540be40080c001a0210446cfb671c3174392410d52fa3cd58723d8417e40cc67c6225b8f7e3ff693a02674b392846c59f783ea96655d39560956dd987051972064a5d853cea0b6f6d711'; | ||
| await RelayAssertions.assertRejection( | ||
| predefined.INVALID_ARGUMENTS('invalid hex string'), |
There was a problem hiding this comment.
Hmmm I wanna see the unexpected junk after rlp payload to be covered since this PR is aiming to fix that, right? How about we use the provided signed transaction for this?
| expect(() => Precheck.parseRawTransaction(invalidTx)).to.throw('Error invoking RPC: invalid BytesLike value'); | ||
| }); |
There was a problem hiding this comment.
Let's test the unexpected junk after RLP payload issue.
Description:
In this PR, the
eth_sendRawTransactionlogic now handles malformed RLP data correctly by checking for INVALID_ARGUMENT error and returning a predifined error and 400 status code.Related issue(s):
Fixes #3652
Checklist