fix: add basic support for multiple transaction types in transaction conversion

[gabriel-aranha-cw]

User description

Enhance transaction input and mined transaction conversion to support various Ethereum transaction types:

• Add basic support for EIP-2930, EIP-1559, EIP-4844, and EIP-7702 transaction types
• Improve transaction type handling in conversion logic
• Update receipt envelope generation to match transaction types

---

PR Type

Enhancement, Tests

---

Description

• Support multiple Ethereum transaction types

• Update transaction conversion logic

• Implement receipt envelope generation

• Add comprehensive transaction type tests

---

Changes walkthrough 📝

	Relevant files
Enhancement	transaction_input.rs Add support for multiple Ethereum transaction types            src/eth/primitives/transaction_input.rs Added support for EIP-2930, EIP-1559, EIP-4844, and EIP-7702 transaction types Updated From implementation for AlloyTransaction Improved transaction type handling in conversion logic +95/-15  transaction_mined.rs Update mined transaction and receipt conversions                  src/eth/primitives/transaction_mined.rs Updated From implementation for AlloyTransaction Modified From implementation for AlloyReceipt Improved receipt envelope generation to match transaction types +3/-3
Tests	e2e-json-rpc.test.ts Add end-to-end tests for transaction types                              e2e/test/automine/e2e-json-rpc.test.ts Added new test suite for different transaction types Implemented tests for legacy, EIP-2930, EIP-1559, and EIP-4844 transactions Verified correct transaction type handling in JSON-RPC interface +47/-0    rpc.ts Add helper functions for transaction type testing                e2e/test/helpers/rpc.ts Added helper functions to create and sign different transaction types Implemented createLegacyTransaction, createEIP2930Transaction, createEIP1559Transaction, and createEIP4844Transaction Updated pollReceipt function to handle new transaction types +76/-3
Configuration changes	justfile Update block comparison command in justfile                            justfile Removed --ignore type flag from block comparison command +1/-1

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 28b1068)

Here are some key observations to aid the review process:

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Incomplete Implementation The implementation for EIP-2930 and EIP-1559 transactions is using default values for access_list. This might not be the correct behavior for all cases and should be reviewed. access_list: AccessList::default(), Potential Performance Issue The conversion logic creates new instances of TxEnvelope for each transaction type. This could be optimized to avoid unnecessary allocations, especially for frequently used transaction types. let inner = match tx_type { // EIP-2930 1 => TxEnvelope::Eip2930(Signed::new_unchecked( TxEip2930 { chain_id: value.chain_id.unwrap_or_default().into(), nonce: value.nonce.into(), gas_price: value.gas_price.into(), gas_limit: value.gas_limit.into(), to: value.to.map(|a| TxKind::Call(a.into())).unwrap_or(TxKind::Create), value: value.value.into(), input: value.input.clone().into(), access_list: AccessList::default(), }, signature, value.hash.into(), )), // EIP-1559 2 => TxEnvelope::Eip1559(Signed::new_unchecked( TxEip1559 { chain_id: value.chain_id.unwrap_or_default().into(), nonce: value.nonce.into(), max_fee_per_gas: value.gas_price.into(), max_priority_fee_per_gas: value.gas_price.into(), gas_limit: value.gas_limit.into(), to: value.to.map(|a| TxKind::Call(a.into())).unwrap_or(TxKind::Create), value: value.value.into(), input: value.input.clone().into(), access_list: AccessList::default(), }, signature, value.hash.into(), )), // EIP-4844 3 => TxEnvelope::Eip4844(Signed::new_unchecked( TxEip4844Variant::TxEip4844(TxEip4844 { chain_id: value.chain_id.unwrap_or_default().into(), nonce: value.nonce.into(), max_fee_per_gas: value.gas_price.into(), max_priority_fee_per_gas: value.gas_price.into(), gas_limit: value.gas_limit.into(), to: value.to.map(Into::into).unwrap_or_default(), value: value.value.into(), input: value.input.clone().into(), access_list: AccessList::default(), blob_versioned_hashes: Vec::new(), max_fee_per_blob_gas: 0u64.into(), }), signature, value.hash.into(), )), // EIP-7702 4 => TxEnvelope::Eip7702(Signed::new_unchecked( TxEip7702 { chain_id: value.chain_id.unwrap_or_default().into(), nonce: value.nonce.into(), gas_limit: value.gas_limit.into(), max_fee_per_gas: value.gas_price.into(), max_priority_fee_per_gas: value.gas_price.into(), to: value.to.map(Into::into).unwrap_or_default(), value: value.value.into(), input: value.input.clone().into(), access_list: AccessList::default(), authorization_list: Vec::new(), }, signature, value.hash.into(), )), // Legacy (default) _ => TxEnvelope::Legacy(Signed::new_unchecked( TxLegacy { chain_id: value.chain_id.map(Into::into), nonce: value.nonce.into(), gas_price: value.gas_price.into(), gas_limit: value.gas_limit.into(), to: value.to.map(|a| TxKind::Call(a.into())).unwrap_or(TxKind::Create), value: value.value.into(), input: value.input.clone().into(), }, signature, value.hash.into(), )), }; Missing Test There's no test case for EIP-7702 (type 4) transactions, while other transaction types are covered. Consider adding a test for this transaction type to ensure complete coverage. describe("Transaction Types", () => { beforeEach(async () => { await sendReset(); }); it("handles legacy transaction with type 0", async () => { const signedTx = await createLegacyTransaction(ALICE, 0); const txHash = await sendRawTransaction(signedTx); await pollReceipt(txHash); const tx = await ETHERJS.getTransaction(txHash); expect(tx?.type).to.equal(0); }); it("handles EIP-2930 (type 1) transaction", async () => { const signedTx = await createEIP2930Transaction(ALICE); const txHash = await sendRawTransaction(signedTx); await pollReceipt(txHash); const tx = await ETHERJS.getTransaction(txHash); expect(tx?.type).to.equal(1); }); it("handles EIP-1559 (type 2) transaction", async () => { const signedTx = await createEIP1559Transaction(ALICE); const txHash = await sendRawTransaction(signedTx); await pollReceipt(txHash); const tx = await ETHERJS.getTransaction(txHash); expect(tx?.type).to.equal(2); }); it("handles EIP-4844 (type 3) transaction", async function () { const signedTx = await createEIP4844Transaction(ALICE); const txHash = await sendRawTransaction(signedTx); await pollReceipt(txHash); const tx = await ETHERJS.getTransaction(txHash); expect(tx?.type).to.equal(3); }); });

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 28b1068
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Handle unsupported transaction types Implement error handling for unsupported transaction types. Currently, the code defaults to a legacy transaction for any unrecognized type, which could lead to unexpected behavior. src/eth/primitives/transaction_input.rs [262-274] -_ => TxEnvelope::Legacy(Signed::new_unchecked( +0 => TxEnvelope::Legacy(Signed::new_unchecked( TxLegacy { chain_id: value.chain_id.map(Into::into), nonce: value.nonce.into(), gas_price: value.gas_price.into(), gas_limit: value.gas_limit.into(), to: value.to.map(|a| TxKind::Call(a.into())).unwrap_or(TxKind::Create), value: value.value.into(), input: value.input.clone().into(), }, signature, value.hash.into(), )), +_ => return Err(anyhow!("Unsupported transaction type: {}", tx_type)), Suggestion importance[1-10]: 8 __ Why: This suggestion improves error handling for unsupported transaction types, which is crucial for maintaining system stability and providing clear feedback. It replaces the catch-all case with a specific legacy case and an error for unknown types.	Medium
General	Handle unknown transaction types Add a default case to handle unknown transaction types in the receipt envelope creation. This prevents potential panics or unexpected behavior for future transaction types. src/eth/primitives/transaction_mined.rs [131-137] let inner = match value.input.tx_type.map(|tx| tx.as_u64()) { + Some(0) | None => ReceiptEnvelope::Legacy(receipt_with_bloom), Some(1) => ReceiptEnvelope::Eip2930(receipt_with_bloom), Some(2) => ReceiptEnvelope::Eip1559(receipt_with_bloom), Some(3) => ReceiptEnvelope::Eip4844(receipt_with_bloom), Some(4) => ReceiptEnvelope::Eip7702(receipt_with_bloom), - _ => ReceiptEnvelope::Legacy(receipt_with_bloom), + Some(unknown) => { + log::warn!("Unknown transaction type: {}. Defaulting to Legacy.", unknown); + ReceiptEnvelope::Legacy(receipt_with_bloom) + } }; Suggestion importance[1-10]: 7 __ Why: The suggestion enhances robustness by explicitly handling unknown transaction types, logging a warning, and defaulting to Legacy. This approach improves error handling and system observability without breaking existing functionality.	Medium

---

Previous suggestions

Suggestions up to commit d475d7e

Category	Suggestion	Impact
Possible issue	Fix gas fee calculation The max_priority_fee_per_gas is currently set to the same value as max_fee_per_gas. This might not be the intended behavior and could lead to overpayment. Consider using a separate field for max_priority_fee_per_gas if available, or calculate it based on network conditions. src/eth/primitives/transaction_input.rs [212-213] -max_fee_per_gas: value.gas_price.into(), -max_priority_fee_per_gas: value.gas_price.into(), +max_fee_per_gas: value.max_fee_per_gas.unwrap_or(value.gas_price).into(), +max_priority_fee_per_gas: value.max_priority_fee_per_gas.unwrap_or_else(|| value.gas_price.min(value.max_fee_per_gas.unwrap_or(value.gas_price))).into(), Suggestion importance[1-10]: 8 __ Why: This suggestion addresses a critical issue in gas fee calculation that could lead to overpayment. It proposes a more accurate way to set max_fee_per_gas and max_priority_fee_per_gas, which is important for correct transaction processing.	Medium
	Improve chain ID handling Consider handling the case where value.chain_id is None more explicitly. Currently, it's using unwrap_or_default(), which might not be the desired behavior in all cases. Consider using a specific default value or handling the None case separately. src/eth/primitives/transaction_input.rs [194] -chain_id: value.chain_id.unwrap_or_default().into(), +chain_id: value.chain_id.map(Into::into).unwrap_or_else(|| panic!("Chain ID is required for this transaction type")), Suggestion importance[1-10]: 7 __ Why: The suggestion addresses a potential issue with chain ID handling, proposing a more explicit approach that could prevent silent errors. This improvement enhances code robustness and error handling.	Medium
General	Improve transaction type handling The match statement for ReceiptEnvelope is not exhaustive. It's using a catch-all pattern (_) which might hide potential errors if new transaction types are added in the future. Consider adding an explicit None case and possibly a default case with a warning or error log. src/eth/primitives/transaction_mined.rs [131-137] let inner = match value.input.tx_type.map(|tx| tx.as_u64()) { Some(1) => ReceiptEnvelope::Eip2930(receipt_with_bloom), Some(2) => ReceiptEnvelope::Eip1559(receipt_with_bloom), Some(3) => ReceiptEnvelope::Eip4844(receipt_with_bloom), Some(4) => ReceiptEnvelope::Eip7702(receipt_with_bloom), - _ => ReceiptEnvelope::Legacy(receipt_with_bloom), + None => ReceiptEnvelope::Legacy(receipt_with_bloom), + Some(unknown) => { + log::warn!("Unknown transaction type: {}", unknown); + ReceiptEnvelope::Legacy(receipt_with_bloom) + } }; Suggestion importance[1-10]: 7 __ Why: This suggestion enhances the robustness of transaction type handling by making the match statement more exhaustive and adding logging for unknown types. It improves error handling and future-proofs the code against new transaction types.	Medium
	Preserve original access list The AccessList is set to default for all transaction types that support it. This might not be the correct behavior if the original transaction included a non-empty access list. Consider passing the actual access list from the input transaction. src/eth/primitives/transaction_input.rs [201] -access_list: AccessList::default(), +access_list: value.access_list.unwrap_or_default(), Suggestion importance[1-10]: 6 __ Why: The suggestion improves the handling of access lists by preserving the original list if available. This enhancement ensures that important transaction data is not lost during conversion.	Low

[github-actions]

Persistent review updated to latest commit 28b1068

[gabriel-aranha-cw]

/flamegraph

[gabriel-aranha-cw]

/benchmark

[gabriel-aranha-cw]

Benchmark:
Run ID: bench-3593872497

Git Info:

• Branch: fix-input-missing-conversions
• Commit: 758cf52
• PR: fix: add basic support for multiple transaction types in transaction conversion #2025

Configuration:

• Target Account Strategy: Default

Leader Stats:
RPS Stats: Max: 1997.00, Min: 1245.00, Avg: 1836.80, StdDev: 87.84
TPS Stats: Max: 2024.00, Min: 1550.00, Avg: 1783.00, StdDev: 99.94

Follower Stats:
Imported Blocks/s: Max: 2.00, Min: 1.00, Avg: 1.48, StdDev: 0.50
Imported Transactions/s: Max: 3974.00, Min: 196.00, Avg: 2708.23, StdDev: 935.44

Plots:

• View Leader Plot
• View Follower Plot

[gabriel-aranha-cw]

Flamegraphs:
Run ID: bench-347304128

Git Info:

• Branch: fix-input-missing-conversions
• Commit: 758cf52
• PR: fix: add basic support for multiple transaction types in transaction conversion #2025

Flamegraphs:
idle:

• View idle cpu
• View idle offcpu

write:

• View write cpu
• View write offcpu

read:

• View read cpu
• View read offcpu

follower:

• View follower cpu
• View follower offcpu

[arthurmm-cloudwalk]

Nice 👍

[gabriel-aranha-cw]

Final benchmark:
Run ID: bench-2162088880

Git Info:

• Branch: main
• Commit: 2ff6440
• PR: fix: add basic support for multiple transaction types in transaction conversion #2025

Configuration:

• Target Account Strategy: Default

Leader Stats:
RPS Stats: Max: 1937.00, Min: 941.00, Avg: 1703.29, StdDev: 88.62
TPS Stats: Max: 1944.00, Min: 1521.00, Avg: 1654.67, StdDev: 90.95

Follower Stats:
Imported Blocks/s: Max: 2.00, Min: 1.00, Avg: 1.59, StdDev: 0.49
Imported Transactions/s: Max: 3735.00, Min: 567.00, Avg: 2709.86, StdDev: 838.21

Plots:

• View Leader Plot
• View Follower Plot