feat: decode rlp directly from tx_input

[f3l1ph3s]

PR Type

Enhancement, Tests

---

Description

• Add signature_hash() method to TransactionInput

  • Supports Legacy and EIP-2930/1559/4844/7702 types

• Refactor RLP decoding flow

  • Introduce build_transaction_input_from_envelope()
  • Replace try_from_alloy_transaction() logic

• Remove redundant transaction conversion code

• Add unit tests for signature_hash() correctness

---

Diagram Walkthrough

flowchart LR
  A["RLP bytes"] -- "Decodable.decode()" --> B["TxEnvelope"]
  B -- "build_transaction_input_from_envelope()" --> C["TransactionInput"]
  C -- "signature_hash()" --> D["B256 signature hash"]

Loading

File Walkthrough

	Relevant files
Enhancement	transaction_input.rs Add signature_hash and refactor decoding                                  src/eth/types/transaction/transaction_input.rs Introduce signature_hash() computing B256 from stored fields Refactor RLP decoding into build_transaction_input_from_envelope() Replace old try_from_alloy_transaction() conversion path Add unit tests verifying signature hash matches envelope +144/-26

---

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 6b243b5)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Incomplete signature hash The new signature_hash() method defaults several important fields—access_list, blob‐related fields (blob_versioned_hashes, max_fee_per_blob_gas), and authorization_list—to empty or zero. It also uses the same gas_price for both max_fee_per_gas and max_priority_fee_per_gas. As a result, transactions that include a non‐empty access list, blob data, or distinct priority fees will produce an incorrect signing hash. This mismatch will only surface on real transactions with those fields populated. fn signature_hash(&self) -> B256 { match self.transaction_info.tx_type.map(|t| t.as_u64()).unwrap_or(0) { // EIP-2930 1 => TxEip2930 { chain_id: self.execution_info.chain_id.unwrap_or_default().into(), nonce: self.execution_info.nonce.into(), gas_price: self.execution_info.gas_price, gas_limit: self.execution_info.gas_limit.into(), to: TxKind::from(self.execution_info.to.map(Into::into)), value: self.execution_info.value.into(), input: self.execution_info.input.clone().into(), access_list: AccessList::default(), } .signature_hash(), // EIP-1559 2 => TxEip1559 { chain_id: self.execution_info.chain_id.unwrap_or_default().into(), nonce: self.execution_info.nonce.into(), max_fee_per_gas: self.execution_info.gas_price, max_priority_fee_per_gas: self.execution_info.gas_price, gas_limit: self.execution_info.gas_limit.into(), to: TxKind::from(self.execution_info.to.map(Into::into)), value: self.execution_info.value.into(), input: self.execution_info.input.clone().into(), access_list: AccessList::default(), } .signature_hash(), // EIP-4844 3 => TxEip4844 { chain_id: self.execution_info.chain_id.unwrap_or_default().into(), nonce: self.execution_info.nonce.into(), max_fee_per_gas: self.execution_info.gas_price, max_priority_fee_per_gas: self.execution_info.gas_price, gas_limit: self.execution_info.gas_limit.into(), to: self.execution_info.to.map(Into::into).unwrap_or_default(), value: self.execution_info.value.into(), input: self.execution_info.input.clone().into(), access_list: AccessList::default(), blob_versioned_hashes: Vec::default(), max_fee_per_blob_gas: 0, } .signature_hash(), // EIP-7702 4 => TxEip7702 { chain_id: self.execution_info.chain_id.unwrap_or_default().into(), nonce: self.execution_info.nonce.into(), gas_limit: self.execution_info.gas_limit.into(), max_fee_per_gas: self.execution_info.gas_price, max_priority_fee_per_gas: self.execution_info.gas_price, to: self.execution_info.to.map(Into::into).unwrap_or_default(), value: self.execution_info.value.into(), input: self.execution_info.input.clone().into(), access_list: AccessList::default(), authorization_list: Vec::default(), } .signature_hash(), // Legacy (default) _ => TxLegacy { chain_id: self.execution_info.chain_id.map(Into::into), nonce: self.execution_info.nonce.into(), gas_price: self.execution_info.gas_price, gas_limit: self.execution_info.gas_limit.into(), to: TxKind::from(self.execution_info.to.map(Into::into)), value: self.execution_info.value.into(), input: self.execution_info.input.clone().into(), } .signature_hash(), } }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 6b243b5
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Use TxKind for to field Use a TxKind conversion for the to field in EIP-4844 and EIP-7702 arms to match how other types are handled and ensure the signature hash aligns with to_tx_envelope. src/eth/types/transaction/transaction_input.rs [187-203] // EIP-4844 3 => TxEip4844 { ... - to: self.execution_info.to.map(Into::into).unwrap_or_default(), + to: TxKind::from(self.execution_info.to.map(Into::into)), ... } // EIP-7702 4 => TxEip7702 { ... - to: self.execution_info.to.map(Into::into).unwrap_or_default(), + to: TxKind::from(self.execution_info.to.map(Into::into)), ... } Suggestion importance[1-10]: 8 __ Why: Converting to with TxKind aligns EIP-4844 and EIP-7702 arms with other transaction types, ensuring consistent signature hash logic and preventing subtle bugs.	Medium
General	Qualify AccessList path Qualify AccessList with its full path to avoid missing import errors and ensure the code compiles without adding a new import. This makes it explicit where AccessList is coming from. src/eth/types/transaction/transaction_input.rs [162] -access_list: AccessList::default(), +access_list: alloy_consensus::AccessList::default(), Suggestion importance[1-10]: 6 __ Why: Qualifying AccessList ensures the code compiles without adding a new import and prevents ambiguity, improving reliability with minimal change.	Low

---

Previous suggestions

Suggestions up to commit 7db6dee

Category	Suggestion	Impact
Possible issue	Separate EIP-1559 fee fields Collapsing both fee fields into gas_price loses the original max and priority fee distinction and will break EIP-1559 signature hashes when they differ. Split ExecutionInfo.gas_price into max_fee_per_gas and max_priority_fee_per_gas, and use them here. src/eth/types/transaction/transaction_input.rs [170-171] -// Inside the EIP-1559 arm of signature_hash: -max_fee_per_gas: self.execution_info.gas_price, -max_priority_fee_per_gas: self.execution_info.gas_price, +max_fee_per_gas: self.execution_info.max_fee_per_gas, +max_priority_fee_per_gas: self.execution_info.max_priority_fee_per_gas, Suggestion importance[1-10]: 8 __ Why: Using the same gas_price for both EIP-1559 fee fields will produce incorrect signature hashes when original max_fee_per_gas ≠ max_priority_fee_per_gas, so this is an important correctness fix.	Medium

[cloudwalk-review-agent]

Summary

Nice refactor overall: decoding now builds TransactionInput directly from TxEnvelope, and the new signature_hash() path is covered by a focused consistency test against to_tx_envelope().signature_hash() across tx types 0–4.

I reviewed for correctness around signer recovery and hash derivation, and the implementation stays behaviorally aligned with the previous envelope-based hash flow for the fields currently persisted in TransactionInput.

[github-actions]

Persistent review updated to latest commit 6b243b5

[carneiro-cw]

Wrong idea. The idea is to decode transaction input directly from the received rlp. This means that we never even create the alloy types in the first place, the rlp gets decoded directly to TransactionInput, this means we don't ever even decode to TxEnvelope.

Before doing this however, a better issue to solve first is #2548 . Since then when receiving an eth_sendRawTransaction we wouldn't have to do any conversions with alloy at all.

[cloudwalk-review-agent]

Thanks for the refactor — moving decode toward TransactionInput directly is a good direction. I found one blocking correctness issue in signer recovery:

Blocking: signature_hash() now reconstructs typed txs with fields that are not persisted in TransactionInput (access list, 1559 priority fee, 4844 blob fields, 7702 authorization list) using defaults. For any real tx where those fields are non-default, recovered prehash will differ from the original signed prehash, so signer recovery can fail or recover the wrong address.

Concretely in this diff:

• type 1/2: access_list: AccessList::default()
• type 2/3/4: max_priority_fee_per_gas = gas_price
• type 3: blob_versioned_hashes = [], max_fee_per_blob_gas = 0
• type 4: authorization_list = []

Because build_transaction_input_from_envelope() now calls recover_signer_address() (which uses this new hash path), this can break immediately on decode of valid raw txs carrying those fields.

Suggested fix options:

1. Keep using envelope-native signing hash for recovery when decoding from raw tx (envelope.signature_hash()), and only use field-derived hash where you can guarantee complete persisted fields; or
2. Extend TransactionInput/ExecutionInfo to store all signing-relevant fields per tx type, then compute hash from fully faithful data.

Also, current test only checks signature_hash() vs to_tx_envelope() built from the same reduced fields, so it can’t catch this class of mismatch. Please add a test with a real typed tx (e.g. 1559 with non-empty access list / different priority fee, and/or 4844 with blob hashes) and assert recovered signer matches the signer from the original envelope/raw tx.