fix(rpc): null v/r/s for undecodable EVM tx sigs instead of silent zeros

[satyakwok]

Audit M-6 follow-up against #692.

Problem

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.

Fix

Surface v/r/s as null instead of zero strings. 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.

Impact

No behavior change for healthy EVM txs (the steady state). Failed-decode path goes from "silent zeros that look like a valid signature" to "explicit null + warn log".

Test plan

• cargo check --release passes
• Confirm Sentrix testnet tx returns proper v/r/s post-deploy
• Confirm null v/r/s + warn log on a deliberately-corrupted signature row

Summary by CodeRabbit

• Chores

  • Version bumped to 2.2.13

• Bug Fixes

  • Ethereum transaction signature fields now return null and emit diagnostic warnings when decoding fails, replacing previous silent default fallbacks

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR contains two changes: a workspace version bump from 2.2.12 to 2.2.13, and a logic modification in the tx_to_evm_json function that changes how Ethereum signature fields (v, r, s) are handled. Previously, when RLP decoding of the stored signature failed, the function returned zero-filled values ("0x0"). Now it returns null instead and emits a warning log (for EVM transactions) that includes the transaction ID and failure reason.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• sentrix-labs/sentrix#695: Both PRs modify crates/sentrix-rpc/src/jsonrpc/eth.rs's tx_to_evm_json response-shaping logic for eth_getTransactionByHash (this PR changes v/r/s nulling on signature decode issues, while the related PR changes pending field nulling and COINBASE→zero-address sender handling).
• sentrix-labs/sentrix#692: Both PRs modify crates/sentrix-rpc/src/jsonrpc/eth.rs's tx_to_evm_json logic for EVM transaction signature fields, changing how v/r/s are decoded/fallback-populated in the JSON response.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The pull request description provides a thorough explanation of the problem, fix, and impact, but does not follow the repository's required template structure with Scope and Checks sections.	Restructure the description to follow the template: add Scope section (clearly mark which type of change this is), and include a Checks section documenting verification steps.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: replacing silent zero values with null for undecodable EVM transaction signatures.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/rpc-vrs-no-silent-zero

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 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 338-345: The current match assumes extract_vrs_from_rlp returns
Option and logs only txid on failure; change extract_vrs_from_rlp to return
Result<(String,String,String), E> (or propagate its error type), update the call
in eth.rs to match on Ok((v,r,s)) and Err(err), and include the err in the
tracing::warn message (e.g., tracing::warn!(txid, error = %err,
"eth_getTransactionByHash: failed to decode v/r/s from stored EVM-tx signature
field; returning null sig")). Ensure callers of extract_vrs_from_rlp are updated
to handle Result and that the error type implements Display for logging.

🪄 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: 133c732f-b3ae-4f55-b18a-07ccf6dcc6d2

📥 Commits

Reviewing files that changed from the base of the PR and between a06fa94 and 46e89c9.

📒 Files selected for processing (2)

• Cargo.toml
• crates/sentrix-rpc/src/jsonrpc/eth.rs

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Include the decode failure reason in the warning log.

On Line 341, the warning includes txid but not the decode error reason, which misses part of the stated behavior and weakens diagnosability. Return an error from extract_vrs_from_rlp and log it here.

Proposed fix

-    let (v_value, r_value, s_value): (Value, Value, Value) = if is_evm {
-        match extract_vrs_from_rlp(tx_obj["signature"].as_str().unwrap_or("")) {
+    let (v_value, r_value, s_value): (Value, Value, Value) = if is_evm {
+        match extract_vrs_from_rlp(tx_obj["signature"].as_str().unwrap_or("")) {
             Some((v, r, s)) => (Value::String(v), Value::String(r), Value::String(s)),
-            None => {
+            Err(err) => {
                 tracing::warn!(
                     txid,
+                    error = %err,
                     "eth_getTransactionByHash: failed to decode v/r/s from stored \
                      EVM-tx signature field; returning null sig"
                 );
                 (Value::Null, Value::Null, Value::Null)
             }
         }
     } else {
         (Value::Null, Value::Null, Value::Null)
     };

-fn extract_vrs_from_rlp(sig_hex: &str) -> Option<(String, String, String)> {
+fn extract_vrs_from_rlp(sig_hex: &str) -> Result<(String, String, String), String> {
     use alloy_consensus::TxEnvelope;
     use alloy_eips::eip2718::Decodable2718;

-    let raw_bytes = hex::decode(sig_hex).ok()?;
-    let envelope = TxEnvelope::decode_2718(&mut raw_bytes.as_slice()).ok()?;
+    let raw_bytes = hex::decode(sig_hex).map_err(|e| format!("hex decode failed: {e}"))?;
+    let envelope = TxEnvelope::decode_2718(&mut raw_bytes.as_slice())
+        .map_err(|e| format!("2718 decode failed: {e}"))?;
     let sig = envelope.signature();
     let v_val: u64 = if sig.v() { 1 } else { 0 };
     let r_u256 = sig.r();
     let s_u256 = sig.s();
-    Some((to_hex(v_val), format!("0x{r_u256:x}"), format!("0x{s_u256:x}")))
+    Ok((to_hex(v_val), format!("0x{r_u256:x}"), format!("0x{s_u256:x}")))
 }

🤖 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 338 - 345, The current
match assumes extract_vrs_from_rlp returns Option and logs only txid on failure;
change extract_vrs_from_rlp to return Result<(String,String,String), E> (or
propagate its error type), update the call in eth.rs to match on Ok((v,r,s)) and
Err(err), and include the err in the tracing::warn message (e.g.,
tracing::warn!(txid, error = %err, "eth_getTransactionByHash: failed to decode
v/r/s from stored EVM-tx signature field; returning null sig")). Ensure callers
of extract_vrs_from_rlp are updated to handle Result and that the error type
implements Display for logging.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!