From 5c0e82932180b40b225872ace6d4eb5a7579069b Mon Sep 17 00:00:00 2001 From: 0x0 Date: Sat, 2 Mar 2024 23:01:23 -0800 Subject: [PATCH 1/8] . --- crates/net/eth-wire/src/types/transactions.rs | 759 +++++++++--------- crates/primitives/src/transaction/error.rs | 11 + crates/primitives/src/transaction/mod.rs | 2 +- crates/primitives/src/transaction/pooled.rs | 139 ++-- crates/rpc/rpc/src/eth/api/transactions.rs | 6 +- crates/rpc/rpc/src/eth/error.rs | 6 +- crates/transaction-pool/src/pool/mod.rs | 11 +- 7 files changed, 478 insertions(+), 456 deletions(-) diff --git a/crates/net/eth-wire/src/types/transactions.rs b/crates/net/eth-wire/src/types/transactions.rs index dd593c5c7bb6..7d03e8b61412 100644 --- a/crates/net/eth-wire/src/types/transactions.rs +++ b/crates/net/eth-wire/src/types/transactions.rs @@ -3,7 +3,9 @@ use alloy_rlp::{RlpDecodableWrapper, RlpEncodableWrapper}; use derive_more::{Constructor, Deref, IntoIterator}; use reth_codecs::derive_arbitrary; -use reth_primitives::{PooledTransactionsElement, TransactionSigned, B256}; +use reth_primitives::{ + transaction::TransactionConversionError, PooledTransactionsElement, TransactionSigned, B256, +}; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; @@ -59,9 +61,20 @@ impl PooledTransactions { } } -impl From> for PooledTransactions { - fn from(txs: Vec) -> Self { - PooledTransactions(txs.into_iter().map(Into::into).collect()) +impl TryFrom> for PooledTransactions { + type Error = TransactionConversionError; // Assume this is defined elsewhere + + fn try_from(txs: Vec) -> Result { + let mut pooled_txs = Vec::new(); + + for tx in txs { + match PooledTransactionsElement::try_from(tx) { + Ok(pooled_tx) => pooled_txs.push(pooled_tx), + Err(e) => return Err(e), + } + } + + Ok(PooledTransactions(pooled_txs)) } } @@ -76,7 +89,8 @@ mod tests { use crate::{message::RequestPair, GetPooledTransactions, PooledTransactions}; use alloy_rlp::{Decodable, Encodable}; use reth_primitives::{ - hex, Signature, Transaction, TransactionKind, TransactionSigned, TxEip1559, TxLegacy, U256, + hex, PooledTransactionsElement, Signature, Transaction, TransactionKind, TransactionSigned, + TxEip1559, TxLegacy, U256, }; use std::str::FromStr; @@ -118,59 +132,67 @@ mod tests { fn encode_pooled_transactions() { let expected = hex!("f8d7820457f8d2f867088504a817c8088302e2489435353535353535353535353535353535353535358202008025a064b1702d9298fee62dfeccc57d322a463ad55ca201256d01f62b45b2e1c21c12a064b1702d9298fee62dfeccc57d322a463ad55ca201256d01f62b45b2e1c21c10f867098504a817c809830334509435353535353535353535353535353535353535358202d98025a052f8f61201b2b11a78d6e866abc9c3db2ae8631fa656bfe5cb53668255367afba052f8f61201b2b11a78d6e866abc9c3db2ae8631fa656bfe5cb53668255367afb"); let mut data = vec![]; + let txs = vec![ + TransactionSigned::from_transaction_and_signature( + Transaction::Legacy(TxLegacy { + chain_id: Some(1), + nonce: 0x8u64, + gas_price: 0x4a817c808, + gas_limit: 0x2e248u64, + to: TransactionKind::Call( + hex!("3535353535353535353535353535353535353535").into(), + ), + value: U256::from(0x200u64), + input: Default::default(), + }), + Signature { + odd_y_parity: false, + r: U256::from_str( + "0x64b1702d9298fee62dfeccc57d322a463ad55ca201256d01f62b45b2e1c21c12", + ) + .unwrap(), + s: U256::from_str( + "0x64b1702d9298fee62dfeccc57d322a463ad55ca201256d01f62b45b2e1c21c10", + ) + .unwrap(), + }, + ), + TransactionSigned::from_transaction_and_signature( + Transaction::Legacy(TxLegacy { + chain_id: Some(1), + nonce: 0x09u64, + gas_price: 0x4a817c809, + gas_limit: 0x33450u64, + to: TransactionKind::Call( + hex!("3535353535353535353535353535353535353535").into(), + ), + value: U256::from(0x2d9u64), + input: Default::default(), + }), + Signature { + odd_y_parity: false, + r: U256::from_str( + "0x52f8f61201b2b11a78d6e866abc9c3db2ae8631fa656bfe5cb53668255367afb", + ) + .unwrap(), + s: U256::from_str( + "0x52f8f61201b2b11a78d6e866abc9c3db2ae8631fa656bfe5cb53668255367afb", + ) + .unwrap(), + }, + ), + ]; + let message: Vec = txs + .into_iter() + .map(|tx| { + PooledTransactionsElement::try_from(tx) + .expect("Failed to convert TransactionSigned to PooledTransactionsElement") + }) + .collect(); let request = RequestPair:: { request_id: 1111, - message: vec![ - TransactionSigned::from_transaction_and_signature( - Transaction::Legacy(TxLegacy { - chain_id: Some(1), - nonce: 0x8u64, - gas_price: 0x4a817c808, - gas_limit: 0x2e248u64, - to: TransactionKind::Call( - hex!("3535353535353535353535353535353535353535").into(), - ), - value: U256::from(0x200u64), - input: Default::default(), - }), - Signature { - odd_y_parity: false, - r: U256::from_str( - "0x64b1702d9298fee62dfeccc57d322a463ad55ca201256d01f62b45b2e1c21c12", - ) - .unwrap(), - s: U256::from_str( - "0x64b1702d9298fee62dfeccc57d322a463ad55ca201256d01f62b45b2e1c21c10", - ) - .unwrap(), - }, - ), - TransactionSigned::from_transaction_and_signature( - Transaction::Legacy(TxLegacy { - chain_id: Some(1), - nonce: 0x09u64, - gas_price: 0x4a817c809, - gas_limit: 0x33450u64, - to: TransactionKind::Call( - hex!("3535353535353535353535353535353535353535").into(), - ), - value: U256::from(0x2d9u64), - input: Default::default(), - }), - Signature { - odd_y_parity: false, - r: U256::from_str( - "0x52f8f61201b2b11a78d6e866abc9c3db2ae8631fa656bfe5cb53668255367afb", - ) - .unwrap(), - s: U256::from_str( - "0x52f8f61201b2b11a78d6e866abc9c3db2ae8631fa656bfe5cb53668255367afb", - ) - .unwrap(), - }, - ), - ] - .into(), + message: PooledTransactions(message), /* Assuming PooledTransactions wraps a + * Vec */ }; request.encode(&mut data); assert_eq!(data, expected); @@ -180,59 +202,66 @@ mod tests { // Test vector from: https://eips.ethereum.org/EIPS/eip-2481 fn decode_pooled_transactions() { let data = hex!("f8d7820457f8d2f867088504a817c8088302e2489435353535353535353535353535353535353535358202008025a064b1702d9298fee62dfeccc57d322a463ad55ca201256d01f62b45b2e1c21c12a064b1702d9298fee62dfeccc57d322a463ad55ca201256d01f62b45b2e1c21c10f867098504a817c809830334509435353535353535353535353535353535353535358202d98025a052f8f61201b2b11a78d6e866abc9c3db2ae8631fa656bfe5cb53668255367afba052f8f61201b2b11a78d6e866abc9c3db2ae8631fa656bfe5cb53668255367afb"); + let txs = vec![ + TransactionSigned::from_transaction_and_signature( + Transaction::Legacy(TxLegacy { + chain_id: Some(1), + nonce: 0x8u64, + gas_price: 0x4a817c808, + gas_limit: 0x2e248u64, + to: TransactionKind::Call( + hex!("3535353535353535353535353535353535353535").into(), + ), + value: U256::from(0x200u64), + input: Default::default(), + }), + Signature { + odd_y_parity: false, + r: U256::from_str( + "0x64b1702d9298fee62dfeccc57d322a463ad55ca201256d01f62b45b2e1c21c12", + ) + .unwrap(), + s: U256::from_str( + "0x64b1702d9298fee62dfeccc57d322a463ad55ca201256d01f62b45b2e1c21c10", + ) + .unwrap(), + }, + ), + TransactionSigned::from_transaction_and_signature( + Transaction::Legacy(TxLegacy { + chain_id: Some(1), + nonce: 0x09u64, + gas_price: 0x4a817c809, + gas_limit: 0x33450u64, + to: TransactionKind::Call( + hex!("3535353535353535353535353535353535353535").into(), + ), + value: U256::from(0x2d9u64), + input: Default::default(), + }), + Signature { + odd_y_parity: false, + r: U256::from_str( + "0x52f8f61201b2b11a78d6e866abc9c3db2ae8631fa656bfe5cb53668255367afb", + ) + .unwrap(), + s: U256::from_str( + "0x52f8f61201b2b11a78d6e866abc9c3db2ae8631fa656bfe5cb53668255367afb", + ) + .unwrap(), + }, + ), + ]; + let message: Vec = txs + .into_iter() + .map(|tx| { + PooledTransactionsElement::try_from(tx) + .expect("Failed to convert TransactionSigned to PooledTransactionsElement") + }) + .collect(); let expected = RequestPair:: { request_id: 1111, - message: vec![ - TransactionSigned::from_transaction_and_signature( - Transaction::Legacy(TxLegacy { - chain_id: Some(1), - nonce: 0x8u64, - gas_price: 0x4a817c808, - gas_limit: 0x2e248u64, - to: TransactionKind::Call( - hex!("3535353535353535353535353535353535353535").into(), - ), - value: U256::from(0x200u64), - input: Default::default(), - }), - Signature { - odd_y_parity: false, - r: U256::from_str( - "0x64b1702d9298fee62dfeccc57d322a463ad55ca201256d01f62b45b2e1c21c12", - ) - .unwrap(), - s: U256::from_str( - "0x64b1702d9298fee62dfeccc57d322a463ad55ca201256d01f62b45b2e1c21c10", - ) - .unwrap(), - }, - ), - TransactionSigned::from_transaction_and_signature( - Transaction::Legacy(TxLegacy { - chain_id: Some(1), - nonce: 0x09u64, - gas_price: 0x4a817c809, - gas_limit: 0x33450u64, - to: TransactionKind::Call( - hex!("3535353535353535353535353535353535353535").into(), - ), - value: U256::from(0x2d9u64), - input: Default::default(), - }), - Signature { - odd_y_parity: false, - r: U256::from_str( - "0x52f8f61201b2b11a78d6e866abc9c3db2ae8631fa656bfe5cb53668255367afb", - ) - .unwrap(), - s: U256::from_str( - "0x52f8f61201b2b11a78d6e866abc9c3db2ae8631fa656bfe5cb53668255367afb", - ) - .unwrap(), - }, - ), - ] - .into(), + message: PooledTransactions(message), }; let request = RequestPair::::decode(&mut &data[..]).unwrap(); @@ -244,134 +273,140 @@ mod tests { let data = hex!("f9022980f90225f8650f84832156008287fb94cf7f9e66af820a19257a2108375b180b0ec491678204d2802ca035b7bfeb9ad9ece2cbafaaf8e202e706b4cfaeb233f46198f00b44d4a566a981a0612638fb29427ca33b9a3be2a0a561beecfe0269655be160d35e72d366a6a860b87502f872041a8459682f008459682f0d8252089461815774383099e24810ab832a5b2a5425c154d58829a2241af62c000080c001a059e6b67f48fb32e7e570dfb11e042b5ad2e55e3ce3ce9cd989c7e06e07feeafda0016b83f4f980694ed2eee4d10667242b1f40dc406901b34125b008d334d47469f86b0384773594008398968094d3e8763675e4c425df46cc3b5c0f6cbdac39604687038d7ea4c68000802ba0ce6834447c0a4193c40382e6c57ae33b241379c5418caac9cdc18d786fd12071a03ca3ae86580e94550d7c071e3a02eadb5a77830947c9225165cf9100901bee88f86b01843b9aca00830186a094d3e8763675e4c425df46cc3b5c0f6cbdac3960468702769bb01b2a00802ba0e24d8bd32ad906d6f8b8d7741e08d1959df021698b19ee232feba15361587d0aa05406ad177223213df262cb66ccbb2f46bfdccfdfbbb5ffdda9e2c02d977631daf86b02843b9aca00830186a094d3e8763675e4c425df46cc3b5c0f6cbdac39604687038d7ea4c68000802ba00eb96ca19e8a77102767a41fc85a36afd5c61ccb09911cec5d3e86e193d9c5aea03a456401896b1b6055311536bf00a718568c744d8c1f9df59879e8350220ca18"); let decoded_transactions = RequestPair::::decode(&mut &data[..]).unwrap(); - + let txs = vec![ + TransactionSigned::from_transaction_and_signature( + Transaction::Legacy(TxLegacy { + chain_id: Some(4), + nonce: 15u64, + gas_price: 2200000000, + gas_limit: 34811u64, + to: TransactionKind::Call( + hex!("cf7f9e66af820a19257a2108375b180b0ec49167").into(), + ), + value: U256::from(1234u64), + input: Default::default(), + }), + Signature { + odd_y_parity: true, + r: U256::from_str( + "0x35b7bfeb9ad9ece2cbafaaf8e202e706b4cfaeb233f46198f00b44d4a566a981", + ) + .unwrap(), + s: U256::from_str( + "0x612638fb29427ca33b9a3be2a0a561beecfe0269655be160d35e72d366a6a860", + ) + .unwrap(), + }, + ), + TransactionSigned::from_transaction_and_signature( + Transaction::Eip1559(TxEip1559 { + chain_id: 4, + nonce: 26u64, + max_priority_fee_per_gas: 1500000000, + max_fee_per_gas: 1500000013, + gas_limit: 21000u64, + to: TransactionKind::Call( + hex!("61815774383099e24810ab832a5b2a5425c154d5").into(), + ), + value: U256::from(3000000000000000000u64), + input: Default::default(), + access_list: Default::default(), + }), + Signature { + odd_y_parity: true, + r: U256::from_str( + "0x59e6b67f48fb32e7e570dfb11e042b5ad2e55e3ce3ce9cd989c7e06e07feeafd", + ) + .unwrap(), + s: U256::from_str( + "0x016b83f4f980694ed2eee4d10667242b1f40dc406901b34125b008d334d47469", + ) + .unwrap(), + }, + ), + TransactionSigned::from_transaction_and_signature( + Transaction::Legacy(TxLegacy { + chain_id: Some(4), + nonce: 3u64, + gas_price: 2000000000, + gas_limit: 10000000u64, + to: TransactionKind::Call( + hex!("d3e8763675e4c425df46cc3b5c0f6cbdac396046").into(), + ), + value: U256::from(1000000000000000u64), + input: Default::default(), + }), + Signature { + odd_y_parity: false, + r: U256::from_str( + "0xce6834447c0a4193c40382e6c57ae33b241379c5418caac9cdc18d786fd12071", + ) + .unwrap(), + s: U256::from_str( + "0x3ca3ae86580e94550d7c071e3a02eadb5a77830947c9225165cf9100901bee88", + ) + .unwrap(), + }, + ), + TransactionSigned::from_transaction_and_signature( + Transaction::Legacy(TxLegacy { + chain_id: Some(4), + nonce: 1u64, + gas_price: 1000000000, + gas_limit: 100000u64, + to: TransactionKind::Call( + hex!("d3e8763675e4c425df46cc3b5c0f6cbdac396046").into(), + ), + value: U256::from(693361000000000u64), + input: Default::default(), + }), + Signature { + odd_y_parity: false, + r: U256::from_str( + "0xe24d8bd32ad906d6f8b8d7741e08d1959df021698b19ee232feba15361587d0a", + ) + .unwrap(), + s: U256::from_str( + "0x5406ad177223213df262cb66ccbb2f46bfdccfdfbbb5ffdda9e2c02d977631da", + ) + .unwrap(), + }, + ), + TransactionSigned::from_transaction_and_signature( + Transaction::Legacy(TxLegacy { + chain_id: Some(4), + nonce: 2u64, + gas_price: 1000000000, + gas_limit: 100000u64, + to: TransactionKind::Call( + hex!("d3e8763675e4c425df46cc3b5c0f6cbdac396046").into(), + ), + value: U256::from(1000000000000000u64), + input: Default::default(), + }), + Signature { + odd_y_parity: false, + r: U256::from_str( + "0xeb96ca19e8a77102767a41fc85a36afd5c61ccb09911cec5d3e86e193d9c5ae", + ) + .unwrap(), + s: U256::from_str( + "0x3a456401896b1b6055311536bf00a718568c744d8c1f9df59879e8350220ca18", + ) + .unwrap(), + }, + ), + ]; + let message: Vec = txs + .into_iter() + .map(|tx| { + PooledTransactionsElement::try_from(tx) + .expect("Failed to convert TransactionSigned to PooledTransactionsElement") + }) + .collect(); let expected_transactions = RequestPair:: { request_id: 0, - message: vec![ - TransactionSigned::from_transaction_and_signature( - Transaction::Legacy(TxLegacy { - chain_id: Some(4), - nonce: 15u64, - gas_price: 2200000000, - gas_limit: 34811u64, - to: TransactionKind::Call( - hex!("cf7f9e66af820a19257a2108375b180b0ec49167").into(), - ), - value: U256::from(1234u64), - input: Default::default(), - }), - Signature { - odd_y_parity: true, - r: U256::from_str( - "0x35b7bfeb9ad9ece2cbafaaf8e202e706b4cfaeb233f46198f00b44d4a566a981", - ) - .unwrap(), - s: U256::from_str( - "0x612638fb29427ca33b9a3be2a0a561beecfe0269655be160d35e72d366a6a860", - ) - .unwrap(), - }, - ), - TransactionSigned::from_transaction_and_signature( - Transaction::Eip1559(TxEip1559 { - chain_id: 4, - nonce: 26u64, - max_priority_fee_per_gas: 1500000000, - max_fee_per_gas: 1500000013, - gas_limit: 21000u64, - to: TransactionKind::Call( - hex!("61815774383099e24810ab832a5b2a5425c154d5").into(), - ), - value: U256::from(3000000000000000000u64), - input: Default::default(), - access_list: Default::default(), - }), - Signature { - odd_y_parity: true, - r: U256::from_str( - "0x59e6b67f48fb32e7e570dfb11e042b5ad2e55e3ce3ce9cd989c7e06e07feeafd", - ) - .unwrap(), - s: U256::from_str( - "0x016b83f4f980694ed2eee4d10667242b1f40dc406901b34125b008d334d47469", - ) - .unwrap(), - }, - ), - TransactionSigned::from_transaction_and_signature( - Transaction::Legacy(TxLegacy { - chain_id: Some(4), - nonce: 3u64, - gas_price: 2000000000, - gas_limit: 10000000u64, - to: TransactionKind::Call( - hex!("d3e8763675e4c425df46cc3b5c0f6cbdac396046").into(), - ), - value: U256::from(1000000000000000u64), - input: Default::default(), - }), - Signature { - odd_y_parity: false, - r: U256::from_str( - "0xce6834447c0a4193c40382e6c57ae33b241379c5418caac9cdc18d786fd12071", - ) - .unwrap(), - s: U256::from_str( - "0x3ca3ae86580e94550d7c071e3a02eadb5a77830947c9225165cf9100901bee88", - ) - .unwrap(), - }, - ), - TransactionSigned::from_transaction_and_signature( - Transaction::Legacy(TxLegacy { - chain_id: Some(4), - nonce: 1u64, - gas_price: 1000000000, - gas_limit: 100000u64, - to: TransactionKind::Call( - hex!("d3e8763675e4c425df46cc3b5c0f6cbdac396046").into(), - ), - value: U256::from(693361000000000u64), - input: Default::default(), - }), - Signature { - odd_y_parity: false, - r: U256::from_str( - "0xe24d8bd32ad906d6f8b8d7741e08d1959df021698b19ee232feba15361587d0a", - ) - .unwrap(), - s: U256::from_str( - "0x5406ad177223213df262cb66ccbb2f46bfdccfdfbbb5ffdda9e2c02d977631da", - ) - .unwrap(), - }, - ), - TransactionSigned::from_transaction_and_signature( - Transaction::Legacy(TxLegacy { - chain_id: Some(4), - nonce: 2u64, - gas_price: 1000000000, - gas_limit: 100000u64, - to: TransactionKind::Call( - hex!("d3e8763675e4c425df46cc3b5c0f6cbdac396046").into(), - ), - value: U256::from(1000000000000000u64), - input: Default::default(), - }), - Signature { - odd_y_parity: false, - r: U256::from_str( - "0xeb96ca19e8a77102767a41fc85a36afd5c61ccb09911cec5d3e86e193d9c5ae", - ) - .unwrap(), - s: U256::from_str( - "0x3a456401896b1b6055311536bf00a718568c744d8c1f9df59879e8350220ca18", - ) - .unwrap(), - }, - ), - ] - .into(), + message: PooledTransactions(message), }; // checking tx by tx for easier debugging if there are any regressions @@ -387,134 +422,140 @@ mod tests { #[test] fn encode_pooled_transactions_network() { let expected = hex!("f9022980f90225f8650f84832156008287fb94cf7f9e66af820a19257a2108375b180b0ec491678204d2802ca035b7bfeb9ad9ece2cbafaaf8e202e706b4cfaeb233f46198f00b44d4a566a981a0612638fb29427ca33b9a3be2a0a561beecfe0269655be160d35e72d366a6a860b87502f872041a8459682f008459682f0d8252089461815774383099e24810ab832a5b2a5425c154d58829a2241af62c000080c001a059e6b67f48fb32e7e570dfb11e042b5ad2e55e3ce3ce9cd989c7e06e07feeafda0016b83f4f980694ed2eee4d10667242b1f40dc406901b34125b008d334d47469f86b0384773594008398968094d3e8763675e4c425df46cc3b5c0f6cbdac39604687038d7ea4c68000802ba0ce6834447c0a4193c40382e6c57ae33b241379c5418caac9cdc18d786fd12071a03ca3ae86580e94550d7c071e3a02eadb5a77830947c9225165cf9100901bee88f86b01843b9aca00830186a094d3e8763675e4c425df46cc3b5c0f6cbdac3960468702769bb01b2a00802ba0e24d8bd32ad906d6f8b8d7741e08d1959df021698b19ee232feba15361587d0aa05406ad177223213df262cb66ccbb2f46bfdccfdfbbb5ffdda9e2c02d977631daf86b02843b9aca00830186a094d3e8763675e4c425df46cc3b5c0f6cbdac39604687038d7ea4c68000802ba00eb96ca19e8a77102767a41fc85a36afd5c61ccb09911cec5d3e86e193d9c5aea03a456401896b1b6055311536bf00a718568c744d8c1f9df59879e8350220ca18"); - + let txs = vec![ + TransactionSigned::from_transaction_and_signature( + Transaction::Legacy(TxLegacy { + chain_id: Some(4), + nonce: 15u64, + gas_price: 2200000000, + gas_limit: 34811u64, + to: TransactionKind::Call( + hex!("cf7f9e66af820a19257a2108375b180b0ec49167").into(), + ), + value: U256::from(1234u64), + input: Default::default(), + }), + Signature { + odd_y_parity: true, + r: U256::from_str( + "0x35b7bfeb9ad9ece2cbafaaf8e202e706b4cfaeb233f46198f00b44d4a566a981", + ) + .unwrap(), + s: U256::from_str( + "0x612638fb29427ca33b9a3be2a0a561beecfe0269655be160d35e72d366a6a860", + ) + .unwrap(), + }, + ), + TransactionSigned::from_transaction_and_signature( + Transaction::Eip1559(TxEip1559 { + chain_id: 4, + nonce: 26u64, + max_priority_fee_per_gas: 1500000000, + max_fee_per_gas: 1500000013, + gas_limit: 21000u64, + to: TransactionKind::Call( + hex!("61815774383099e24810ab832a5b2a5425c154d5").into(), + ), + value: U256::from(3000000000000000000u64), + input: Default::default(), + access_list: Default::default(), + }), + Signature { + odd_y_parity: true, + r: U256::from_str( + "0x59e6b67f48fb32e7e570dfb11e042b5ad2e55e3ce3ce9cd989c7e06e07feeafd", + ) + .unwrap(), + s: U256::from_str( + "0x016b83f4f980694ed2eee4d10667242b1f40dc406901b34125b008d334d47469", + ) + .unwrap(), + }, + ), + TransactionSigned::from_transaction_and_signature( + Transaction::Legacy(TxLegacy { + chain_id: Some(4), + nonce: 3u64, + gas_price: 2000000000, + gas_limit: 10000000u64, + to: TransactionKind::Call( + hex!("d3e8763675e4c425df46cc3b5c0f6cbdac396046").into(), + ), + value: U256::from(1000000000000000u64), + input: Default::default(), + }), + Signature { + odd_y_parity: false, + r: U256::from_str( + "0xce6834447c0a4193c40382e6c57ae33b241379c5418caac9cdc18d786fd12071", + ) + .unwrap(), + s: U256::from_str( + "0x3ca3ae86580e94550d7c071e3a02eadb5a77830947c9225165cf9100901bee88", + ) + .unwrap(), + }, + ), + TransactionSigned::from_transaction_and_signature( + Transaction::Legacy(TxLegacy { + chain_id: Some(4), + nonce: 1u64, + gas_price: 1000000000, + gas_limit: 100000u64, + to: TransactionKind::Call( + hex!("d3e8763675e4c425df46cc3b5c0f6cbdac396046").into(), + ), + value: U256::from(693361000000000u64), + input: Default::default(), + }), + Signature { + odd_y_parity: false, + r: U256::from_str( + "0xe24d8bd32ad906d6f8b8d7741e08d1959df021698b19ee232feba15361587d0a", + ) + .unwrap(), + s: U256::from_str( + "0x5406ad177223213df262cb66ccbb2f46bfdccfdfbbb5ffdda9e2c02d977631da", + ) + .unwrap(), + }, + ), + TransactionSigned::from_transaction_and_signature( + Transaction::Legacy(TxLegacy { + chain_id: Some(4), + nonce: 2u64, + gas_price: 1000000000, + gas_limit: 100000u64, + to: TransactionKind::Call( + hex!("d3e8763675e4c425df46cc3b5c0f6cbdac396046").into(), + ), + value: U256::from(1000000000000000u64), + input: Default::default(), + }), + Signature { + odd_y_parity: false, + r: U256::from_str( + "0xeb96ca19e8a77102767a41fc85a36afd5c61ccb09911cec5d3e86e193d9c5ae", + ) + .unwrap(), + s: U256::from_str( + "0x3a456401896b1b6055311536bf00a718568c744d8c1f9df59879e8350220ca18", + ) + .unwrap(), + }, + ), + ]; + let message: Vec = txs + .into_iter() + .map(|tx| { + PooledTransactionsElement::try_from(tx) + .expect("Failed to convert TransactionSigned to PooledTransactionsElement") + }) + .collect(); let transactions = RequestPair:: { request_id: 0, - message: vec![ - TransactionSigned::from_transaction_and_signature( - Transaction::Legacy(TxLegacy { - chain_id: Some(4), - nonce: 15u64, - gas_price: 2200000000, - gas_limit: 34811u64, - to: TransactionKind::Call( - hex!("cf7f9e66af820a19257a2108375b180b0ec49167").into(), - ), - value: U256::from(1234u64), - input: Default::default(), - }), - Signature { - odd_y_parity: true, - r: U256::from_str( - "0x35b7bfeb9ad9ece2cbafaaf8e202e706b4cfaeb233f46198f00b44d4a566a981", - ) - .unwrap(), - s: U256::from_str( - "0x612638fb29427ca33b9a3be2a0a561beecfe0269655be160d35e72d366a6a860", - ) - .unwrap(), - }, - ), - TransactionSigned::from_transaction_and_signature( - Transaction::Eip1559(TxEip1559 { - chain_id: 4, - nonce: 26u64, - max_priority_fee_per_gas: 1500000000, - max_fee_per_gas: 1500000013, - gas_limit: 21000u64, - to: TransactionKind::Call( - hex!("61815774383099e24810ab832a5b2a5425c154d5").into(), - ), - value: U256::from(3000000000000000000u64), - input: Default::default(), - access_list: Default::default(), - }), - Signature { - odd_y_parity: true, - r: U256::from_str( - "0x59e6b67f48fb32e7e570dfb11e042b5ad2e55e3ce3ce9cd989c7e06e07feeafd", - ) - .unwrap(), - s: U256::from_str( - "0x016b83f4f980694ed2eee4d10667242b1f40dc406901b34125b008d334d47469", - ) - .unwrap(), - }, - ), - TransactionSigned::from_transaction_and_signature( - Transaction::Legacy(TxLegacy { - chain_id: Some(4), - nonce: 3u64, - gas_price: 2000000000, - gas_limit: 10000000u64, - to: TransactionKind::Call( - hex!("d3e8763675e4c425df46cc3b5c0f6cbdac396046").into(), - ), - value: U256::from(1000000000000000u64), - input: Default::default(), - }), - Signature { - odd_y_parity: false, - r: U256::from_str( - "0xce6834447c0a4193c40382e6c57ae33b241379c5418caac9cdc18d786fd12071", - ) - .unwrap(), - s: U256::from_str( - "0x3ca3ae86580e94550d7c071e3a02eadb5a77830947c9225165cf9100901bee88", - ) - .unwrap(), - }, - ), - TransactionSigned::from_transaction_and_signature( - Transaction::Legacy(TxLegacy { - chain_id: Some(4), - nonce: 1u64, - gas_price: 1000000000, - gas_limit: 100000u64, - to: TransactionKind::Call( - hex!("d3e8763675e4c425df46cc3b5c0f6cbdac396046").into(), - ), - value: U256::from(693361000000000u64), - input: Default::default(), - }), - Signature { - odd_y_parity: false, - r: U256::from_str( - "0xe24d8bd32ad906d6f8b8d7741e08d1959df021698b19ee232feba15361587d0a", - ) - .unwrap(), - s: U256::from_str( - "0x5406ad177223213df262cb66ccbb2f46bfdccfdfbbb5ffdda9e2c02d977631da", - ) - .unwrap(), - }, - ), - TransactionSigned::from_transaction_and_signature( - Transaction::Legacy(TxLegacy { - chain_id: Some(4), - nonce: 2u64, - gas_price: 1000000000, - gas_limit: 100000u64, - to: TransactionKind::Call( - hex!("d3e8763675e4c425df46cc3b5c0f6cbdac396046").into(), - ), - value: U256::from(1000000000000000u64), - input: Default::default(), - }), - Signature { - odd_y_parity: false, - r: U256::from_str( - "0xeb96ca19e8a77102767a41fc85a36afd5c61ccb09911cec5d3e86e193d9c5ae", - ) - .unwrap(), - s: U256::from_str( - "0x3a456401896b1b6055311536bf00a718568c744d8c1f9df59879e8350220ca18", - ) - .unwrap(), - }, - ), - ] - .into(), + message: PooledTransactions(message), }; let mut encoded = vec![]; diff --git a/crates/primitives/src/transaction/error.rs b/crates/primitives/src/transaction/error.rs index 21fc575186be..000e71bab211 100644 --- a/crates/primitives/src/transaction/error.rs +++ b/crates/primitives/src/transaction/error.rs @@ -52,3 +52,14 @@ pub enum InvalidTransactionError { #[error("transaction signer has bytecode set")] SignerAccountHasBytecode, } + +/// Represents error variants that can happen when trying to convert a transaction to +/// [`PooledTransactionsElement`](crate::PooledTransactionsElement) +#[derive(Debug, Clone, Eq, PartialEq, thiserror::Error)] +pub enum TransactionConversionError { + /// This error variant is used when a transaction cannot be converted into a + /// [`PooledTransactionsElement`](crate::PooledTransactionsElement) because it is not supported + /// for broadcasting over the P2P network. + #[error("transaction cannot be casted to PooledTransactionsElement since it's unsupported for p2p broadcast")] + UnsupportedForBroadcast, +} diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index bf0594ea8ca0..411d811bf62a 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -18,7 +18,7 @@ pub use eip1559::TxEip1559; pub use eip2930::TxEip2930; pub use eip4844::TxEip4844; -pub use error::InvalidTransactionError; +pub use error::{InvalidTransactionError, TransactionConversionError}; pub use legacy::TxLegacy; pub use meta::TransactionMeta; #[cfg(feature = "c-kzg")] diff --git a/crates/primitives/src/transaction/pooled.rs b/crates/primitives/src/transaction/pooled.rs index 44efbab2b5bb..f210505bd359 100644 --- a/crates/primitives/src/transaction/pooled.rs +++ b/crates/primitives/src/transaction/pooled.rs @@ -15,6 +15,8 @@ use derive_more::{AsRef, Deref}; use reth_codecs::add_arbitrary_tests; use serde::{Deserialize, Serialize}; +use super::error::TransactionConversionError; + /// A response to `GetPooledTransactions`. This can include either a blob transaction, or a /// non-4844 signed transaction. #[add_arbitrary_tests] @@ -49,16 +51,6 @@ pub enum PooledTransactionsElement { }, /// A blob transaction, which includes the transaction, blob data, commitments, and proofs. BlobTransaction(BlobTransaction), - /// An Optimism deposit transaction - #[cfg(feature = "optimism")] - Deposit { - /// The inner transaction - transaction: crate::TxDeposit, - /// The signature - signature: Signature, - /// The hash of the transaction - hash: TxHash, - }, } impl PooledTransactionsElement { @@ -70,7 +62,8 @@ impl PooledTransactionsElement { if tx.is_eip4844() { return Err(tx) } - Ok(tx.into()) + // todo this is bad additional clone, better method? + PooledTransactionsElement::try_from(tx.clone()).map_err(|_| tx) } /// Converts from an EIP-4844 [TransactionSignedEcRecovered] to a @@ -107,8 +100,6 @@ impl PooledTransactionsElement { Self::Eip2930 { transaction, .. } => transaction.signature_hash(), Self::Eip1559 { transaction, .. } => transaction.signature_hash(), Self::BlobTransaction(blob_tx) => blob_tx.transaction.signature_hash(), - #[cfg(feature = "optimism")] - Self::Deposit { .. } => B256::ZERO, } } @@ -119,8 +110,6 @@ impl PooledTransactionsElement { PooledTransactionsElement::Eip2930 { hash, .. } | PooledTransactionsElement::Eip1559 { hash, .. } => hash, PooledTransactionsElement::BlobTransaction(tx) => &tx.hash, - #[cfg(feature = "optimism")] - PooledTransactionsElement::Deposit { hash, .. } => hash, } } @@ -131,10 +120,6 @@ impl PooledTransactionsElement { Self::Eip2930 { signature, .. } | Self::Eip1559 { signature, .. } => signature, Self::BlobTransaction(blob_tx) => &blob_tx.signature, - #[cfg(feature = "optimism")] - Self::Deposit { .. } => { - panic!("Deposit transactions do not have a signature! This is a bug.") - } } } @@ -145,8 +130,6 @@ impl PooledTransactionsElement { Self::Eip2930 { transaction, .. } => transaction.nonce, Self::Eip1559 { transaction, .. } => transaction.nonce, Self::BlobTransaction(blob_tx) => blob_tx.transaction.nonce, - #[cfg(feature = "optimism")] - Self::Deposit { .. } => 0, } } @@ -249,12 +232,6 @@ impl PooledTransactionsElement { signature: typed_tx.signature, hash: typed_tx.hash, }), - #[cfg(feature = "optimism")] - Transaction::Deposit(tx) => Ok(PooledTransactionsElement::Deposit { - transaction: tx, - signature: typed_tx.signature, - hash: typed_tx.hash, - }), } } } @@ -283,12 +260,6 @@ impl PooledTransactionsElement { hash, }, Self::BlobTransaction(blob_tx) => blob_tx.into_parts().0, - #[cfg(feature = "optimism")] - Self::Deposit { transaction, signature, hash } => TransactionSigned { - transaction: Transaction::Deposit(transaction), - signature, - hash, - }, } } @@ -311,8 +282,6 @@ impl PooledTransactionsElement { // the encoding does not use a header, so we set `with_header` to false blob_tx.payload_len_with_type(false) } - #[cfg(feature = "optimism")] - Self::Deposit { transaction, .. } => transaction.payload_len_without_header(), } } @@ -355,10 +324,6 @@ impl PooledTransactionsElement { // `tx_type || rlp([transaction_payload_body, blobs, commitments, proofs]))` blob_tx.encode_with_type_inner(out, false); } - #[cfg(feature = "optimism")] - Self::Deposit { transaction, .. } => { - transaction.encode(out, false); - } } } } @@ -395,10 +360,6 @@ impl Encodable for PooledTransactionsElement { // `rlp(tx_type || rlp([transaction_payload_body, blobs, commitments, proofs]))` blob_tx.encode_with_type_inner(out, true); } - #[cfg(feature = "optimism")] - Self::Deposit { transaction, .. } => { - transaction.encode(out, true); - } } } @@ -420,11 +381,6 @@ impl Encodable for PooledTransactionsElement { // the encoding uses a header, so we set `with_header` to true blob_tx.payload_len_with_type(true) } - #[cfg(feature = "optimism")] - Self::Deposit { transaction, .. } => { - // method computes the payload len with a RLP header - transaction.payload_len() - } } } } @@ -526,47 +482,38 @@ impl Decodable for PooledTransactionsElement { signature: typed_tx.signature, hash: typed_tx.hash, }), - #[cfg(feature = "optimism")] - Transaction::Deposit(tx) => Ok(PooledTransactionsElement::Deposit { - transaction: tx, - signature: typed_tx.signature, - hash: typed_tx.hash, - }), } } } } } -impl From for PooledTransactionsElement { - /// Converts from a [TransactionSigned] to a [PooledTransactionsElement]. - /// - /// NOTE: For EIP-4844 transactions, this will return an empty sidecar. - fn from(tx: TransactionSigned) -> Self { +impl TryFrom for PooledTransactionsElement { + type Error = TransactionConversionError; + + fn try_from(tx: TransactionSigned) -> Result { let TransactionSigned { transaction, signature, hash } = tx; match transaction { Transaction::Legacy(tx) => { - PooledTransactionsElement::Legacy { transaction: tx, signature, hash } + Ok(PooledTransactionsElement::Legacy { transaction: tx, signature, hash }) } Transaction::Eip2930(tx) => { - PooledTransactionsElement::Eip2930 { transaction: tx, signature, hash } + Ok(PooledTransactionsElement::Eip2930 { transaction: tx, signature, hash }) } Transaction::Eip1559(tx) => { - PooledTransactionsElement::Eip1559 { transaction: tx, signature, hash } + Ok(PooledTransactionsElement::Eip1559 { transaction: tx, signature, hash }) } Transaction::Eip4844(tx) => { - PooledTransactionsElement::BlobTransaction(BlobTransaction { + Ok(PooledTransactionsElement::BlobTransaction(BlobTransaction { transaction: tx, signature, hash, - // This is empty - just for the conversion! + // Assuming BlobTransaction has a sidecar that needs to be defaulted sidecar: Default::default(), - }) + })) } #[cfg(feature = "optimism")] - Transaction::Deposit(tx) => { - PooledTransactionsElement::Deposit { transaction: tx, signature, hash } - } + Transaction::Deposit(_) => Err(TransactionConversionError::UnsupportedForBroadcast), } } } @@ -582,43 +529,46 @@ impl<'a> arbitrary::Arbitrary<'a> for PooledTransactionsElement { /// `PooledTransactionsElement`. fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result { // Attempt to create a `TransactionSigned` with arbitrary data. - Ok(match PooledTransactionsElement::from(TransactionSigned::arbitrary(u)?) { - // If the generated `PooledTransactionsElement` is a blob transaction... - PooledTransactionsElement::BlobTransaction(mut tx) => { - // Generate a sidecar for the blob transaction using arbitrary data. + let tx_signed = TransactionSigned::arbitrary(u)?; + // Attempt to create a `PooledTransactionsElement` with arbitrary data, handling the Result. + match PooledTransactionsElement::try_from(tx_signed) { + Ok(PooledTransactionsElement::BlobTransaction(mut tx)) => { + // Successfully converted to a BlobTransaction, now generate a sidecar. tx.sidecar = crate::BlobTransactionSidecar::arbitrary(u)?; - // Return the blob transaction with the generated sidecar. - PooledTransactionsElement::BlobTransaction(tx) + Ok(PooledTransactionsElement::BlobTransaction(tx)) } - // If the generated `PooledTransactionsElement` is not a blob transaction... - tx => tx, - }) + Ok(tx) => Ok(tx), // Successfully converted, but not a BlobTransaction. + Err(_) => Err(arbitrary::Error::IncorrectFormat), /* Conversion failed, return an + * arbitrary error. */ + } } } #[cfg(any(test, feature = "arbitrary"))] impl proptest::arbitrary::Arbitrary for PooledTransactionsElement { type Parameters = (); + type Strategy = proptest::strategy::BoxedStrategy; + fn arbitrary_with(_: Self::Parameters) -> Self::Strategy { use proptest::prelude::{any, Strategy}; any::<(TransactionSigned, crate::BlobTransactionSidecar)>() .prop_map(move |(transaction, sidecar)| { - // this will have an empty sidecar - let pooled_txs_element = PooledTransactionsElement::from(transaction); - - // generate a sidecar for blob txs - if let PooledTransactionsElement::BlobTransaction(mut tx) = pooled_txs_element { - tx.sidecar = sidecar; - PooledTransactionsElement::BlobTransaction(tx) - } else { - pooled_txs_element + match PooledTransactionsElement::try_from(transaction) { + Ok(PooledTransactionsElement::BlobTransaction(mut tx)) => { + tx.sidecar = sidecar; + PooledTransactionsElement::BlobTransaction(tx) + } + Ok(tx) => tx, + Err(_) => PooledTransactionsElement::Eip1559 { + transaction: Default::default(), + signature: Default::default(), + hash: Default::default(), + }, // Gen an Eip1559 as arbitrary for testing purpose } }) .boxed() } - - type Strategy = proptest::strategy::BoxedStrategy; } /// A signed pooled transaction with recovered signer. @@ -682,9 +632,16 @@ impl PooledTransactionsElementEcRecovered { } /// Converts a `TransactionSignedEcRecovered` into a `PooledTransactionsElementEcRecovered`. -impl From for PooledTransactionsElementEcRecovered { - fn from(tx: TransactionSignedEcRecovered) -> Self { - Self { transaction: tx.signed_transaction.into(), signer: tx.signer } +impl TryFrom for PooledTransactionsElementEcRecovered { + type Error = TransactionConversionError; + + fn try_from(tx: TransactionSignedEcRecovered) -> Result { + match PooledTransactionsElement::try_from(tx.signed_transaction) { + Ok(pooled_transaction) => { + Ok(Self { transaction: pooled_transaction, signer: tx.signer }) + } + Err(_) => Err(TransactionConversionError::UnsupportedForBroadcast), + } } } diff --git a/crates/rpc/rpc/src/eth/api/transactions.rs b/crates/rpc/rpc/src/eth/api/transactions.rs index 307847adfdf8..32c4857a75d3 100644 --- a/crates/rpc/rpc/src/eth/api/transactions.rs +++ b/crates/rpc/rpc/src/eth/api/transactions.rs @@ -756,8 +756,10 @@ where let recovered = signed_tx.into_ecrecovered().ok_or(EthApiError::InvalidTransactionSignature)?; - let pool_transaction = - ::from_recovered_pooled_transaction(recovered.into()); + let pool_transaction = match recovered.try_into() { + Ok(converted) => ::from_recovered_pooled_transaction(converted), + Err(_) => return Err(EthApiError::TransactionConversionError), + }; // submit the transaction to the pool with a `Local` origin let hash = self.pool().add_transaction(TransactionOrigin::Local, pool_transaction).await?; diff --git a/crates/rpc/rpc/src/eth/error.rs b/crates/rpc/rpc/src/eth/error.rs index 7a5e55033053..b7754a4f99ef 100644 --- a/crates/rpc/rpc/src/eth/error.rs +++ b/crates/rpc/rpc/src/eth/error.rs @@ -113,6 +113,9 @@ pub enum EthApiError { /// Evm generic purpose error. #[error("Revm error: {0}")] EvmCustom(String), + /// Error encountered when converting a transaction type + #[error("Transaction conversion error")] + TransactionConversionError, } /// Eth Optimism Api Error @@ -146,7 +149,8 @@ impl From for ErrorObject<'static> { EthApiError::ConflictingFeeFieldsInRequest | EthApiError::Signing(_) | EthApiError::BothStateAndStateDiffInOverride(_) | - EthApiError::InvalidTracerConfig => invalid_params_rpc_err(error.to_string()), + EthApiError::InvalidTracerConfig | + EthApiError::TransactionConversionError => invalid_params_rpc_err(error.to_string()), EthApiError::InvalidTransaction(err) => err.into(), EthApiError::PoolError(err) => err.into(), EthApiError::PrevrandaoNotSet | diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 017e0e594a34..e45ba6815907 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -327,7 +327,11 @@ where continue } } else { - PooledTransactionsElement::from(tx) + match PooledTransactionsElement::try_from(tx) { + Ok(element) => element, + Err(_) => continue, /* Skip transactions that fail to convert, since they + * won't be broadcast on p2p anyway */ + } }; size += encoded_len; @@ -351,7 +355,10 @@ where if tx.is_eip4844() { self.get_blob_transaction(tx).map(PooledTransactionsElement::BlobTransaction) } else { - Some(PooledTransactionsElement::from(tx)) + match PooledTransactionsElement::try_from(tx) { + Ok(element) => Some(element), + Err(_) => None, // Conversion failed, return None + } } }) } From ce68b4778f9bb3cd091b2103b6356f977d37d1f1 Mon Sep 17 00:00:00 2001 From: 0x0 Date: Sat, 2 Mar 2024 23:04:37 -0800 Subject: [PATCH 2/8] . --- crates/primitives/src/transaction/pooled.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/primitives/src/transaction/pooled.rs b/crates/primitives/src/transaction/pooled.rs index f210505bd359..4fb6b834f492 100644 --- a/crates/primitives/src/transaction/pooled.rs +++ b/crates/primitives/src/transaction/pooled.rs @@ -539,7 +539,7 @@ impl<'a> arbitrary::Arbitrary<'a> for PooledTransactionsElement { } Ok(tx) => Ok(tx), // Successfully converted, but not a BlobTransaction. Err(_) => Err(arbitrary::Error::IncorrectFormat), /* Conversion failed, return an - * arbitrary error. */ + * arbitrary error. */ } } } From 0c46fe92e8557e8aef78a477530dde9b842c89d6 Mon Sep 17 00:00:00 2001 From: 0x0 Date: Sat, 2 Mar 2024 23:40:58 -0800 Subject: [PATCH 3/8] . --- crates/primitives/src/transaction/pooled.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/primitives/src/transaction/pooled.rs b/crates/primitives/src/transaction/pooled.rs index 4fb6b834f492..6cee11f2b6da 100644 --- a/crates/primitives/src/transaction/pooled.rs +++ b/crates/primitives/src/transaction/pooled.rs @@ -232,6 +232,8 @@ impl PooledTransactionsElement { signature: typed_tx.signature, hash: typed_tx.hash, }), + #[cfg(feature = "optimism")] + Transaction::Deposit(_) => Err(RlpError::Custom("Optimism deposit transaction cannot be decoded to PooledTransactionsElement")) } } } @@ -482,6 +484,8 @@ impl Decodable for PooledTransactionsElement { signature: typed_tx.signature, hash: typed_tx.hash, }), + #[cfg(feature = "optimism")] + Transaction::Deposit(_) => Err(RlpError::Custom("Optimism deposit transaction cannot be decoded to PooledTransactionsElement")) } } } From 44adb5c02bf85f150676610a55eae11bec7fe770 Mon Sep 17 00:00:00 2001 From: 0x0 Date: Sat, 2 Mar 2024 23:51:50 -0800 Subject: [PATCH 4/8] . --- crates/net/network/src/transactions/fetcher.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/net/network/src/transactions/fetcher.rs b/crates/net/network/src/transactions/fetcher.rs index 326c8c2b0c1a..d9d8185b43c1 100644 --- a/crates/net/network/src/transactions/fetcher.rs +++ b/crates/net/network/src/transactions/fetcher.rs @@ -1369,10 +1369,10 @@ mod test { fn verify_response_hashes() { let input = hex!("02f871018302a90f808504890aef60826b6c94ddf4c5025d1a5742cf12f74eec246d4432c295e487e09c3bbcc12b2b80c080a0f21a4eacd0bf8fea9c5105c543be5a1d8c796516875710fafafdf16d16d8ee23a001280915021bb446d1973501a67f93d2b38894a514b976e7b46dc2fe54598daa"); let signed_tx_1: PooledTransactionsElement = - TransactionSigned::decode(&mut &input[..]).unwrap().into(); + TransactionSigned::decode(&mut &input[..]).unwrap().try_into().unwrap(); let input = hex!("02f871018302a90f808504890aef60826b6c94ddf4c5025d1a5742cf12f74eec246d4432c295e487e09c3bbcc12b2b80c080a0f21a4eacd0bf8fea9c5105c543be5a1d8c796516875710fafafdf16d16d8ee23a001280915021bb446d1973501a67f93d2b38894a514b976e7b46dc2fe54598d76"); let signed_tx_2: PooledTransactionsElement = - TransactionSigned::decode(&mut &input[..]).unwrap().into(); + TransactionSigned::decode(&mut &input[..]).unwrap().try_into().unwrap(); // only tx 1 is requested let request_hashes = [ From fb107338d79ceaf7f4d3c8136bdc5bcb4752363b Mon Sep 17 00:00:00 2001 From: 0x0 Date: Sun, 3 Mar 2024 10:15:55 -0800 Subject: [PATCH 5/8] . --- crates/net/eth-wire/src/types/transactions.rs | 13 ++----------- crates/net/network/src/transactions/mod.rs | 2 +- crates/primitives/src/transaction/error.rs | 6 +++--- crates/primitives/src/transaction/pooled.rs | 7 ++++--- crates/transaction-pool/src/pool/mod.rs | 5 +---- 5 files changed, 11 insertions(+), 22 deletions(-) diff --git a/crates/net/eth-wire/src/types/transactions.rs b/crates/net/eth-wire/src/types/transactions.rs index 7d03e8b61412..5d48211be5bd 100644 --- a/crates/net/eth-wire/src/types/transactions.rs +++ b/crates/net/eth-wire/src/types/transactions.rs @@ -62,19 +62,10 @@ impl PooledTransactions { } impl TryFrom> for PooledTransactions { - type Error = TransactionConversionError; // Assume this is defined elsewhere + type Error = TransactionConversionError; fn try_from(txs: Vec) -> Result { - let mut pooled_txs = Vec::new(); - - for tx in txs { - match PooledTransactionsElement::try_from(tx) { - Ok(pooled_tx) => pooled_txs.push(pooled_tx), - Err(e) => return Err(e), - } - } - - Ok(PooledTransactions(pooled_txs)) + txs.into_iter().map(PooledTransactionsElement::try_from).collect() } } diff --git a/crates/net/network/src/transactions/mod.rs b/crates/net/network/src/transactions/mod.rs index 7879954c4e88..fac62605ebdf 100644 --- a/crates/net/network/src/transactions/mod.rs +++ b/crates/net/network/src/transactions/mod.rs @@ -857,7 +857,7 @@ where // ensure we didn't receive any blob transactions as these are disallowed to be // broadcasted in full - let has_blob_txs = msg.has_eip4844(); + let has_blob_txs: bool = msg.has_eip4844(); let non_blob_txs = msg .0 diff --git a/crates/primitives/src/transaction/error.rs b/crates/primitives/src/transaction/error.rs index 000e71bab211..cf979f022d6e 100644 --- a/crates/primitives/src/transaction/error.rs +++ b/crates/primitives/src/transaction/error.rs @@ -59,7 +59,7 @@ pub enum InvalidTransactionError { pub enum TransactionConversionError { /// This error variant is used when a transaction cannot be converted into a /// [`PooledTransactionsElement`](crate::PooledTransactionsElement) because it is not supported - /// for broadcasting over the P2P network. - #[error("transaction cannot be casted to PooledTransactionsElement since it's unsupported for p2p broadcast")] - UnsupportedForBroadcast, + /// for P2P network. + #[error("Transaction is not supported for p2p")] + UnsupportedForP2P, } diff --git a/crates/primitives/src/transaction/pooled.rs b/crates/primitives/src/transaction/pooled.rs index 6cee11f2b6da..1e590b072b0c 100644 --- a/crates/primitives/src/transaction/pooled.rs +++ b/crates/primitives/src/transaction/pooled.rs @@ -56,8 +56,9 @@ pub enum PooledTransactionsElement { impl PooledTransactionsElement { /// Tries to convert a [TransactionSigned] into a [PooledTransactionsElement]. /// - /// [BlobTransaction] are disallowed from being propagated, hence this returns an error if the - /// `tx` is [Transaction::Eip4844] + /// This function used as a helper to convert from a decoded p2p broadcast message to + /// [PooledTransactio'nsElement]. Since [BlobTransaction] is disallowed to be broadcasted on + /// p2p, return an err if `tx` is [Transaction::Eip4844]. pub fn try_from_broadcast(tx: TransactionSigned) -> Result { if tx.is_eip4844() { return Err(tx) @@ -644,7 +645,7 @@ impl TryFrom for PooledTransactionsElementEcRecove Ok(pooled_transaction) => { Ok(Self { transaction: pooled_transaction, signer: tx.signer }) } - Err(_) => Err(TransactionConversionError::UnsupportedForBroadcast), + Err(_) => Err(TransactionConversionError::UnsupportedForP2P), } } } diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index e45ba6815907..95971ba57b3f 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -355,10 +355,7 @@ where if tx.is_eip4844() { self.get_blob_transaction(tx).map(PooledTransactionsElement::BlobTransaction) } else { - match PooledTransactionsElement::try_from(tx) { - Ok(element) => Some(element), - Err(_) => None, // Conversion failed, return None - } + PooledTransactionsElement::try_from(tx).ok() } }) } From a74d14548d68439a9ebde024420bf8161f95fdce Mon Sep 17 00:00:00 2001 From: 0x0 Date: Sun, 3 Mar 2024 10:19:28 -0800 Subject: [PATCH 6/8] . --- crates/primitives/src/transaction/pooled.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/primitives/src/transaction/pooled.rs b/crates/primitives/src/transaction/pooled.rs index 1e590b072b0c..5ff4c424fc5e 100644 --- a/crates/primitives/src/transaction/pooled.rs +++ b/crates/primitives/src/transaction/pooled.rs @@ -518,7 +518,7 @@ impl TryFrom for PooledTransactionsElement { })) } #[cfg(feature = "optimism")] - Transaction::Deposit(_) => Err(TransactionConversionError::UnsupportedForBroadcast), + Transaction::Deposit(_) => Err(TransactionConversionError::UnsupportedForP2P), } } } From 592bb428bf8d2ff56473619ec842c299ffc737c7 Mon Sep 17 00:00:00 2001 From: 0x0 Date: Mon, 4 Mar 2024 13:48:14 -0800 Subject: [PATCH 7/8] . --- crates/net/network/src/transactions/mod.rs | 2 +- crates/primitives/src/transaction/pooled.rs | 45 ++++++++------------- 2 files changed, 17 insertions(+), 30 deletions(-) diff --git a/crates/net/network/src/transactions/mod.rs b/crates/net/network/src/transactions/mod.rs index fac62605ebdf..7879954c4e88 100644 --- a/crates/net/network/src/transactions/mod.rs +++ b/crates/net/network/src/transactions/mod.rs @@ -857,7 +857,7 @@ where // ensure we didn't receive any blob transactions as these are disallowed to be // broadcasted in full - let has_blob_txs: bool = msg.has_eip4844(); + let has_blob_txs = msg.has_eip4844(); let non_blob_txs = msg .0 diff --git a/crates/primitives/src/transaction/pooled.rs b/crates/primitives/src/transaction/pooled.rs index 5ff4c424fc5e..cdea1304a68c 100644 --- a/crates/primitives/src/transaction/pooled.rs +++ b/crates/primitives/src/transaction/pooled.rs @@ -4,6 +4,7 @@ #![cfg(feature = "c-kzg")] #![cfg_attr(docsrs, doc(cfg(feature = "c-kzg")))] +use super::error::TransactionConversionError; use crate::{ Address, BlobTransaction, BlobTransactionSidecar, Bytes, Signature, Transaction, TransactionSigned, TransactionSignedEcRecovered, TxEip1559, TxEip2930, TxHash, TxLegacy, B256, @@ -15,8 +16,6 @@ use derive_more::{AsRef, Deref}; use reth_codecs::add_arbitrary_tests; use serde::{Deserialize, Serialize}; -use super::error::TransactionConversionError; - /// A response to `GetPooledTransactions`. This can include either a blob transaction, or a /// non-4844 signed transaction. #[add_arbitrary_tests] @@ -60,11 +59,20 @@ impl PooledTransactionsElement { /// [PooledTransactio'nsElement]. Since [BlobTransaction] is disallowed to be broadcasted on /// p2p, return an err if `tx` is [Transaction::Eip4844]. pub fn try_from_broadcast(tx: TransactionSigned) -> Result { - if tx.is_eip4844() { - return Err(tx) + match tx { + TransactionSigned { transaction: Transaction::Legacy(tx), signature, hash } => { + Ok(PooledTransactionsElement::Legacy { transaction: tx, signature, hash }) + } + TransactionSigned { transaction: Transaction::Eip2930(tx), signature, hash } => { + Ok(PooledTransactionsElement::Eip2930 { transaction: tx, signature, hash }) + } + TransactionSigned { transaction: Transaction::Eip1559(tx), signature, hash } => { + Ok(PooledTransactionsElement::Eip1559 { transaction: tx, signature, hash }) + } + tx @ TransactionSigned { transaction: Transaction::Eip4844(_), .. } => Err(tx), + #[cfg(feature = "optimism")] + tx @ TransactionSigned { transaction: Transaction::Deposit(_), .. } => Err(tx), } - // todo this is bad additional clone, better method? - PooledTransactionsElement::try_from(tx.clone()).map_err(|_| tx) } /// Converts from an EIP-4844 [TransactionSignedEcRecovered] to a @@ -497,29 +505,8 @@ impl TryFrom for PooledTransactionsElement { type Error = TransactionConversionError; fn try_from(tx: TransactionSigned) -> Result { - let TransactionSigned { transaction, signature, hash } = tx; - match transaction { - Transaction::Legacy(tx) => { - Ok(PooledTransactionsElement::Legacy { transaction: tx, signature, hash }) - } - Transaction::Eip2930(tx) => { - Ok(PooledTransactionsElement::Eip2930 { transaction: tx, signature, hash }) - } - Transaction::Eip1559(tx) => { - Ok(PooledTransactionsElement::Eip1559 { transaction: tx, signature, hash }) - } - Transaction::Eip4844(tx) => { - Ok(PooledTransactionsElement::BlobTransaction(BlobTransaction { - transaction: tx, - signature, - hash, - // Assuming BlobTransaction has a sidecar that needs to be defaulted - sidecar: Default::default(), - })) - } - #[cfg(feature = "optimism")] - Transaction::Deposit(_) => Err(TransactionConversionError::UnsupportedForP2P), - } + PooledTransactionsElement::try_from_broadcast(tx) + .map_err(|_| TransactionConversionError::UnsupportedForP2P) } } From c56521a4eaeb8c271b5529d17b47c94f4b05ac60 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Tue, 5 Mar 2024 11:37:30 +0100 Subject: [PATCH 8/8] chore: touchups --- crates/primitives/src/transaction/pooled.rs | 4 +++- crates/transaction-pool/src/pool/mod.rs | 10 ++++++++-- crates/transaction-pool/src/traits.rs | 3 +++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/crates/primitives/src/transaction/pooled.rs b/crates/primitives/src/transaction/pooled.rs index cdea1304a68c..ee7a39b6c8a4 100644 --- a/crates/primitives/src/transaction/pooled.rs +++ b/crates/primitives/src/transaction/pooled.rs @@ -56,7 +56,7 @@ impl PooledTransactionsElement { /// Tries to convert a [TransactionSigned] into a [PooledTransactionsElement]. /// /// This function used as a helper to convert from a decoded p2p broadcast message to - /// [PooledTransactio'nsElement]. Since [BlobTransaction] is disallowed to be broadcasted on + /// [PooledTransactionsElement]. Since [BlobTransaction] is disallowed to be broadcasted on /// p2p, return an err if `tx` is [Transaction::Eip4844]. pub fn try_from_broadcast(tx: TransactionSigned) -> Result { match tx { @@ -69,8 +69,10 @@ impl PooledTransactionsElement { TransactionSigned { transaction: Transaction::Eip1559(tx), signature, hash } => { Ok(PooledTransactionsElement::Eip1559 { transaction: tx, signature, hash }) } + // Not supported because missing blob sidecar tx @ TransactionSigned { transaction: Transaction::Eip4844(_), .. } => Err(tx), #[cfg(feature = "optimism")] + // Not supported because deposit transactions are never pooled tx @ TransactionSigned { transaction: Transaction::Deposit(_), .. } => Err(tx), } } diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 95971ba57b3f..e5d7c6dc1be4 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/crates/transaction-pool/src/pool/mod.rs @@ -321,6 +321,7 @@ where let encoded_len = transaction.encoded_length(); let tx = transaction.to_recovered_transaction().into_signed(); let pooled = if tx.is_eip4844() { + // for EIP-4844 transactions, we need to fetch the blob sidecar from the blob store if let Some(blob) = self.get_blob_transaction(tx) { PooledTransactionsElement::BlobTransaction(blob) } else { @@ -329,8 +330,13 @@ where } else { match PooledTransactionsElement::try_from(tx) { Ok(element) => element, - Err(_) => continue, /* Skip transactions that fail to convert, since they - * won't be broadcast on p2p anyway */ + Err(err) => { + debug!( + target: "txpool", %err, + "failed to convert transaction to pooled element; skipping", + ); + continue + } } }; diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index e36e8a3db898..8e974057b12a 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -915,6 +915,9 @@ pub enum EthBlobTransactionSidecar { impl EthPooledTransaction { /// Create new instance of [Self]. + /// + /// Caution: In case of blob transactions, this does marks the blob sidecar as + /// [EthBlobTransactionSidecar::Missing] pub fn new(transaction: TransactionSignedEcRecovered, encoded_length: usize) -> Self { let mut blob_sidecar = EthBlobTransactionSidecar::None; let gas_cost = match &transaction.transaction {