fix: add tx protected check at eth_sendRawTranasaction api
#144
Conversation
WalkthroughThe pull request introduces a new validation check in the Changes
Suggested reviewers
Possibly related PRs
Poem
Finishing Touches
🪧 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: 2
🧹 Nitpick comments (1)
jsonrpc/backend/tx.go (1)
34-34: Enhance the error message with more details.Consider providing more context in the error message to help API consumers understand and resolve the issue.
- return common.Hash{}, errors.New("only replay-protected (EIP-155) transactions allowed over RPC") + return common.Hash{}, fmt.Errorf("transaction rejected: only replay-protected (EIP-155) transactions allowed over RPC. Please ensure your transaction has a valid chain ID")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
jsonrpc/backend/tx.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run test and upload codecov
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #144 +/- ##
==========================================
+ Coverage 56.96% 56.98% +0.01%
==========================================
Files 110 110
Lines 10150 10154 +4
==========================================
+ Hits 5782 5786 +4
Misses 3529 3529
Partials 839 839
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
jsonrpc/README.md (1)
37-37: Documentation update looks good! Consider adding a reference link.The clarification about EIP-155 replay protection requirement is accurate and aligns well with the implementation. To make it even more helpful for users, consider adding a reference link to the EIP-155 specification.
-| eth | eth_sendRawTransaction | ✅ | Sends a signed transaction to the network. Only replay-protected (EIP-155) transactions are accepted. | +| eth | eth_sendRawTransaction | ✅ | Sends a signed transaction to the network. Only replay-protected ([EIP-155](https://eips.ethereum.org/EIPS/eip-155)) transactions are accepted. |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
jsonrpc/README.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: golangci-lint
- GitHub Check: Run test and upload codecov
- GitHub Check: Minitiad
- GitHub Check: Analyze (go)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
jsonrpc/backend/tx_test.go (2)
19-19: Document the hardcoded transaction hex.The hardcoded transaction hex should be documented to explain its components and why it's non-protected. This helps maintainers understand the test data.
Consider adding a detailed comment:
+// Transaction hex breakdown: +// - First byte (0xf8): RLP list marker +// - Next bytes: nonce, gasPrice, gasLimit, to, value, data +// - Last 65 bytes: signature (v=27/28, r, s) indicating non-EIP-155 format txBz, err := hexutil.Decode("0xf8a58085174876e800830186a08080b853604580600e600039806000f350fe7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe03601600081602082378035828234f58015156039578182fd5b8082525050506014600cf31ba02222222222222222222222222222222222222222222222222222222222222222a02222222222222222222222222222222222222222222222222222222222222222")
22-23: Consider adding assertion for specific error type.The test only checks for the error message substring. Consider asserting the specific error type if available.
_, err = input.backend.SendRawTransaction(txBz) - require.ErrorContains(t, err, "EIP-155") + var expectedErr *evmtypes.ErrTxNotProtected // assuming this type exists + require.ErrorAs(t, err, &expectedErr) + require.ErrorContains(t, err, "EIP-155")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
jsonrpc/backend/tx_test.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run test and upload codecov
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
eth_sendRawTransactionAPI to clarify that only replay-protected (EIP-155) transactions are accepted.