fix(rpc): eth_getTransactionByHash returns EVM-standard JSON shape#692
Merged
Conversation
Pre-fix the handler returned the chain-native shape (block_hash,
nested transaction.{amount,chain_id,data:"EVM:..."}) directly,
which ethers.js / viem / alloy / Hyperlane CLI all reject with
"missing from address". eth_sendRawTransaction and
eth_getTransactionReceipt already return EVM-standard responses
on the same chain, so cast send works but anything that polls a
freshly-broadcast tx for confirmation crashes.
Mirror the receipt handler's conversion logic for getTransactionByHash:
- expand from/to/value/gas/gasPrice/input/nonce/type/chainId
- map TOKEN_OP_ADDRESS to to:null for contract creations
- parse EVM:<gas>:<input_hex> envelope for gas + input
- decode stored RLP envelope back into v/r/s via alloy
- amount (sentri) -> value (wei) via *1e10
- type 0x2 + maxFeePerGas for EVM txs, 0x0 for native
Reproducer + workaround discussion isolated 2026-05-20 while running
Hyperlane warp deploy for the Base Sepolia USDC -> Sentrix Testnet
sUSDC route. ProxyAdmin + HypERC20 implementation deployed cleanly
via cast (sendRawTx + getReceipt path), then the CLI crashed once
it tried provider.getTransaction(hash).
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThis PR bumps the Cargo workspace version to 2.2.12 and refactors the Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/sentrix-rpc/src/jsonrpc/eth.rs`:
- Around line 228-232: The code currently maps missing inclusion fields into
genesis-looking defaults (block_index -> 0, block_hash -> "" etc.); change the
parsing in the transaction-building code (where variables block_index,
block_hash, transaction_index are read from tx_data) to produce null/None when
the fields are absent or not present instead of
unwrap_or(0)/unwrap_or_default(). Specifically, for block_index and
transaction_index return an Option/None (or serde_json::Value::Null) when
tx_data["block_index"] / tx_data["transaction_index"] is missing, and for
block_hash return None/null when tx_data["block_hash"] is missing or empty;
apply the same change at the other occurrences you noted (the similar parsing
blocks around lines mentioned) so pending transactions serialize with
blockNumber, blockHash, and transactionIndex set to null until mined.
- Around line 300-317: The code currently hard-codes every EVM transaction as
EIP-1559 (tx_type "0x2", INITIAL_BASE_FEE, zero priority fee) and uses a single
extract_vrs_from_rlp path for signatures; instead decode the original TxEnvelope
variant and populate fields from that envelope: call the transaction decoder
that returns a TxEnvelope (or equivalent) for tx_obj["signature"]/raw RLP, match
on variants (Legacy, EIP2930, EIP1559, EIP4844), set tx_type appropriately
("0x0","0x1","0x2", etc.), fill maxFeePerGas/maxPriorityFeePerGas or gasPrice
from the decoded envelope rather than INITIAL_BASE_FEE, and extract v/r/s from
the decoded signature bytes (use extract_vrs_from_rlp only where appropriate for
legacy/2930 signed envelopes); update both the block response construction
around tx_type/gas fields and the later signature handling (the other affected
block) to use the decoded TxEnvelope data.
- Around line 235-240: The code that builds the sender string from
tx_obj["from_address"] (variables from_raw and from) must normalize "COINBASE"
(case-insensitive) and empty values to the canonical zero EVM address instead of
emitting "0xCOINBASE" or "0x"; update the logic that computes from (and the
analogous occurrence around the other instance noted) to check if from_raw is
empty or equals "COINBASE" (ignoring case) and in those cases set from to
"0x0000000000000000000000000000000000000000", otherwise preserve the existing
0x-prefix handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8ad83114-4750-4724-9acf-abe3fa72dde7
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/*.lock
📒 Files selected for processing (2)
Cargo.tomlcrates/sentrix-rpc/src/jsonrpc/eth.rs
Comment on lines
+228
to
+232
| let block_index = tx_data["block_index"].as_u64().unwrap_or(0); | ||
| let block_hash = tx_data["block_hash"] | ||
| .as_str() | ||
| .map(|h| if h.starts_with("0x") { h.to_string() } else { format!("0x{h}") }) | ||
| .unwrap_or_default(); |
There was a problem hiding this comment.
Return pending location fields as null, not genesis-looking defaults.
This path still turns missing inclusion data into blockNumber = "0x0", blockHash = "", and transactionIndex = "0x0". Given the PR repro is provider.getTransaction(hash) immediately after broadcast, pending transactions will still be mis-shaped here. EIP-1474 clients expect those three fields to be null until the tx is mined.
Proposed fix
- let block_index = tx_data["block_index"].as_u64().unwrap_or(0);
- let block_hash = tx_data["block_hash"]
+ let block_index = tx_data["block_index"].as_u64();
+ let block_hash = tx_data["block_hash"]
.as_str()
.map(|h| if h.starts_with("0x") { h.to_string() } else { format!("0x{h}") })
- .unwrap_or_default();
+ .map(Value::String)
+ .unwrap_or(Value::Null);
...
- let tx_index = bc
- .get_block_any(block_index)
+ let tx_index = block_index.and_then(|block_index| {
+ bc.get_block_any(block_index)
.and_then(|b| b.transactions.iter().position(|t| t.txid == txid))
.map(|i| to_hex(i as u64))
- .unwrap_or_else(|| "0x0".to_string());
+ });
...
- "blockHash": block_hash,
- "blockNumber": to_hex(block_index),
- "transactionIndex": tx_index,
+ "blockHash": block_hash,
+ "blockNumber": block_index.map(to_hex),
+ "transactionIndex": tx_index,Also applies to: 285-289, 321-323
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/sentrix-rpc/src/jsonrpc/eth.rs` around lines 228 - 232, The code
currently maps missing inclusion fields into genesis-looking defaults
(block_index -> 0, block_hash -> "" etc.); change the parsing in the
transaction-building code (where variables block_index, block_hash,
transaction_index are read from tx_data) to produce null/None when the fields
are absent or not present instead of unwrap_or(0)/unwrap_or_default().
Specifically, for block_index and transaction_index return an Option/None (or
serde_json::Value::Null) when tx_data["block_index"] /
tx_data["transaction_index"] is missing, and for block_hash return None/null
when tx_data["block_hash"] is missing or empty; apply the same change at the
other occurrences you noted (the similar parsing blocks around lines mentioned)
so pending transactions serialize with blockNumber, blockHash, and
transactionIndex set to null until mined.
Comment on lines
+235
to
+240
| let from_raw = tx_obj["from_address"].as_str().unwrap_or("").to_string(); | ||
| let from = if from_raw.starts_with("0x") { | ||
| from_raw.clone() | ||
| } else { | ||
| format!("0x{from_raw}") | ||
| }; |
There was a problem hiding this comment.
Normalize coinbase/empty senders to the zero address here too.
eth_getTransactionReceipt already guards from == "COINBASE" / empty because the chain-native value is not a valid EVM address. This helper currently emits 0xCOINBASE or 0x, which will break the same parsers this PR is trying to satisfy for reward/system transactions.
Proposed fix
- let from_raw = tx_obj["from_address"].as_str().unwrap_or("").to_string();
- let from = if from_raw.starts_with("0x") {
- from_raw.clone()
- } else {
- format!("0x{from_raw}")
- };
+ let from_raw = tx_obj["from_address"].as_str().unwrap_or("");
+ let from = if from_raw.is_empty() || from_raw == "COINBASE" {
+ ZERO_HASH_HEX[..42].to_string()
+ } else if from_raw.starts_with("0x") {
+ from_raw.to_string()
+ } else {
+ format!("0x{from_raw}")
+ };Also applies to: 324-324
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/sentrix-rpc/src/jsonrpc/eth.rs` around lines 235 - 240, The code that
builds the sender string from tx_obj["from_address"] (variables from_raw and
from) must normalize "COINBASE" (case-insensitive) and empty values to the
canonical zero EVM address instead of emitting "0xCOINBASE" or "0x"; update the
logic that computes from (and the analogous occurrence around the other instance
noted) to check if from_raw is empty or equals "COINBASE" (ignoring case) and in
those cases set from to "0x0000000000000000000000000000000000000000", otherwise
preserve the existing 0x-prefix handling.
Comment on lines
+300
to
+317
| // Same EIP-1559 type + effectiveGasPrice rationale as receipts. EVM txs | ||
| // go through the base_fee pipeline, native txs do not. | ||
| let (tx_type, gas_price) = if is_evm { | ||
| ("0x2", to_hex(sentrix_evm::gas::INITIAL_BASE_FEE)) | ||
| } else { | ||
| ("0x0", "0x0".to_string()) | ||
| }; | ||
|
|
||
| // For EVM txs, decode the original RLP envelope stored in `signature` | ||
| // (see eth_send_raw_transaction). That gives us the canonical v/r/s | ||
| // ethers needs to round-trip the tx. For native txs we don't have | ||
| // ECDSA-on-EVM-envelope semantics; return zeros so the field is present. | ||
| let (v_hex, r_hex, s_hex) = if is_evm { | ||
| extract_vrs_from_rlp(tx_obj["signature"].as_str().unwrap_or("")) | ||
| .unwrap_or_else(|| ("0x0".to_string(), "0x0".to_string(), "0x0".to_string())) | ||
| } else { | ||
| ("0x0".to_string(), "0x0".to_string(), "0x0".to_string()) | ||
| }; |
There was a problem hiding this comment.
Decode the actual envelope kind before filling type/fee/signature fields.
eth_sendRawTransaction accepts legacy, 2930, 1559, and 4844 envelopes, but this response hard-codes every EVM tx to type = 0x2, maxFeePerGas = INITIAL_BASE_FEE, maxPriorityFeePerGas = 0, and v = 0/1. That makes non-1559 transactions materially wrong, and legacy v loses the signed recovery id. Please branch on the decoded TxEnvelope variant and populate the response from the original envelope instead of from is_evm.
Also applies to: 345-357
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/sentrix-rpc/src/jsonrpc/eth.rs` around lines 300 - 317, The code
currently hard-codes every EVM transaction as EIP-1559 (tx_type "0x2",
INITIAL_BASE_FEE, zero priority fee) and uses a single extract_vrs_from_rlp path
for signatures; instead decode the original TxEnvelope variant and populate
fields from that envelope: call the transaction decoder that returns a
TxEnvelope (or equivalent) for tx_obj["signature"]/raw RLP, match on variants
(Legacy, EIP2930, EIP1559, EIP4844), set tx_type appropriately
("0x0","0x1","0x2", etc.), fill maxFeePerGas/maxPriorityFeePerGas or gasPrice
from the decoded envelope rather than INITIAL_BASE_FEE, and extract v/r/s from
the decoded signature bytes (use extract_vrs_from_rlp only where appropriate for
legacy/2930 signed envelopes); update both the block response construction
around tx_type/gas fields and the later signature handling (the other affected
block) to use the decoded TxEnvelope data.
15 tasks
github-actions Bot
pushed a commit
that referenced
this pull request
May 20, 2026
…onByHash (#695) Two follow-ups on the EVM-shape conversion shipped in #692, both flagged by review after merge: 1. Pending / not-yet-included txs now return null for blockHash, blockNumber, and transactionIndex instead of genesis-looking defaults (block 0, empty string, "0x0"). EIP-1474 clients rely on these three being null to distinguish a mined tx at genesis from one still in the mempool. Pre-fix, ethers.js would happily treat any pending tx as "mined at block 0" and start polling for confirmations against the genesis block. 2. Coinbase / system-emitted txs ("COINBASE" sentinel sender, or empty) now serialize from as the zero address. Receipts already do this; the tx-by-hash path was emitting "0xCOINBASE" or "0x" verbatim, which crashes the same address parsers the original fix tried to unblock. Skipped the third review point — branching tx_type/maxFeePerGas/v on the original TxEnvelope variant — because every EVM tx on Sentrix currently flows through the EIP-1559 base-fee pipeline (cast/forge/ Hyperlane all send type 0x2), so the hard-coded "0x2" matches actual execution semantics. Worth revisiting if/when legacy or 2930 envelopes start landing.
3 tasks
github-actions Bot
pushed a commit
that referenced
this pull request
May 20, 2026
…onByHash (#702) Two follow-ups on the EVM-shape conversion shipped in #692, both flagged by review after merge: 1. Pending / not-yet-included txs now return null for blockHash, blockNumber, and transactionIndex instead of genesis-looking defaults (block 0, empty string, "0x0"). EIP-1474 clients rely on these three being null to distinguish a mined tx at genesis from one still in the mempool. Pre-fix, ethers.js would happily treat any pending tx as "mined at block 0" and start polling for confirmations against the genesis block. 2. Coinbase / system-emitted txs ("COINBASE" sentinel sender, or empty) now serialize from as the zero address. Receipts already do this; the tx-by-hash path was emitting "0xCOINBASE" or "0x" verbatim, which crashes the same address parsers the original fix tried to unblock. Skipped the third review point — branching tx_type/maxFeePerGas/v on the original TxEnvelope variant — because every EVM tx on Sentrix currently flows through the EIP-1559 base-fee pipeline (cast/forge/ Hyperlane all send type 0x2), so the hard-coded "0x2" matches actual execution semantics. Worth revisiting if/when legacy or 2930 envelopes start landing.
3 tasks
github-actions Bot
pushed a commit
that referenced
this pull request
May 21, 2026
…ros (#703) Audit M-6 follow-up against #692. `extract_vrs_from_rlp` returns `Option<(String,String,String)>`; the caller used `unwrap_or_else` to substitute `("0x0","0x0","0x0")` on decode failure. All-zero r/s parses as a valid ECDSA point pair and ecrecover returns a deterministic junk address — a caller doing best-effort sender check via the signature could trust that junk. Surface v/r/s as `null` instead. EIP-1474 clients already treat null sig fields as "not recoverable" (matches the pending-tx convention), so ethers / viem behave correctly without changes. Log a `warn!` when this path fires — successful decode is the steady state for any tx that came in via `eth_sendRawTransaction`, so a failure here means the stored signature field is corrupted or the wire format drifted. For native (non-EVM) txs the same null fields apply; there is no Ethereum-shaped signature to surface, and shape-consistency between the two paths simplifies downstream parsers. No behavior change for healthy EVM txs (which is everything that landed via the standard sender-recovered path).
satyakwok
added a commit
that referenced
this pull request
May 21, 2026
github-actions Bot
pushed a commit
that referenced
this pull request
May 21, 2026
* fix(rpc): eth_getTransactionCount + eth_call + logsBloom length Three discrete chain-RPC fixes bundled because they share one cause (strict-but-non-spec behavior breaking off-the-shelf EVM agents). All discovered together standing up the Hyperlane relayer for the Base Sepolia ↔ Sentrix Testnet bridge (audit fix H-4). 1) eth_getTransactionCount accepts any block tag. Relayer + ethers + viem all pin nonce queries to a recent past block. A stale nonce is self-correcting (chain rejects wrong-nonce tx, caller retries) so this method serves current nonce regardless of block tag. 2) eth_call accepts any block tag. Same pattern: Hyperlane queries Mailbox.delivered(msgId) and recipientIsm(addr) at past blocks. Returns current-state. The strict gate stays on eth_getBalance / eth_getCode / eth_getStorageAt where wrong = wrong protocol decision. 3) Empty logsBloom is now actually 256 bytes (Ethereum spec). The EMPTY_LOGS_BLOOM const was 304 bytes (608 hex chars) because of an off-by-one in the original hand-typed string. The doc comment said 256 but the literal was 304. ethers' fee-oracle middleware strict-parses Block.logsBloom — Hyperlane gas estimation SerdeJson'd on this with "invalid length 608, expected 256 bytes" before any process() tx could be submitted. Bumps workspace 2.2.13 -> 2.2.14. * docs(changelog): add 2.2.14 entry — five RPC compat fixes 2.2.14 went live on mainnet 2026-05-21 17:02 UTC carrying PRs #692/#696/#702/#703/#704. Document the five fixes + flag that the eth_call + logsBloom fixes on this branch are scheduled for 2.2.15.
github-actions Bot
pushed a commit
that referenced
this pull request
May 22, 2026
…bytes (#707) * fix(rpc): eth_getTransactionCount + eth_call + logsBloom length Three discrete chain-RPC fixes bundled because they share one cause (strict-but-non-spec behavior breaking off-the-shelf EVM agents). All discovered together standing up the Hyperlane relayer for the Base Sepolia ↔ Sentrix Testnet bridge (audit fix H-4). 1) eth_getTransactionCount accepts any block tag. Relayer + ethers + viem all pin nonce queries to a recent past block. A stale nonce is self-correcting (chain rejects wrong-nonce tx, caller retries) so this method serves current nonce regardless of block tag. 2) eth_call accepts any block tag. Same pattern: Hyperlane queries Mailbox.delivered(msgId) and recipientIsm(addr) at past blocks. Returns current-state. The strict gate stays on eth_getBalance / eth_getCode / eth_getStorageAt where wrong = wrong protocol decision. 3) Empty logsBloom is now actually 256 bytes (Ethereum spec). The EMPTY_LOGS_BLOOM const was 304 bytes (608 hex chars) because of an off-by-one in the original hand-typed string. The doc comment said 256 but the literal was 304. ethers' fee-oracle middleware strict-parses Block.logsBloom — Hyperlane gas estimation SerdeJson'd on this with "invalid length 608, expected 256 bytes" before any process() tx could be submitted. Bumps workspace 2.2.13 -> 2.2.14. * docs(changelog): add 2.2.14 entry — five RPC compat fixes 2.2.14 went live on mainnet 2026-05-21 17:02 UTC carrying PRs #692/#696/#702/#703/#704. Document the five fixes + flag that the eth_call + logsBloom fixes on this branch are scheduled for 2.2.15.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pre-fix the handler returned the chain-native shape (block_hash, nested transaction.{amount,chain_id,data:"EVM:..."}) directly, which ethers.js / viem / alloy / Hyperlane CLI all reject with "missing from address". eth_sendRawTransaction and eth_getTransactionReceipt already return EVM-standard responses on the same chain, so cast send works but anything that polls a freshly-broadcast tx for confirmation crashes.
Mirror the receipt handler's conversion logic for getTransactionByHash:
Reproducer + workaround discussion isolated 2026-05-20 while running Hyperlane warp deploy for the Base Sepolia USDC -> Sentrix Testnet sUSDC route. ProxyAdmin + HypERC20 implementation deployed cleanly via cast (sendRawTx + getReceipt path), then the CLI crashed once it tried provider.getTransaction(hash).
Summary
Risk tier
Check ONE:
sentrix-core,sentrix-trie,sentrix-staking,sentrix-bft),block_executor,apply_block_*,state_rootpathRequired by tier
🟢 Low — minimum bar
🟡 Medium — adds
#[test]in same PR🟠 High — adds
🔴 Critical — adds
Test plan
Rollback plan
Related
Summary by CodeRabbit
Chores
Bug Fixes