chore: refactor MockTransaction (#12627)

crates/net/network/src/transactions/mod.rs

@@ -2230,7 +2230,7 @@ mod tests {
             .add_transaction(reth_transaction_pool::TransactionOrigin::External, tx.clone())
             .await;
 
-        let request = GetPooledTransactions(vec![tx.get_hash()]);
+        let request = GetPooledTransactions(vec![*tx.get_hash()]);
 
         let (send, receive) = oneshot::channel::<RequestResult<PooledTransactions>>();
 

crates/net/network/tests/it/big_pooled_txs_req.rs

@@ -35,7 +35,7 @@ async fn test_large_tx_req() {
             tx
         })
         .collect();
-    let txs_hashes: Vec<B256> = txs.iter().map(|tx| tx.get_hash()).collect();
+    let txs_hashes: Vec<B256> = txs.iter().map(|tx| *tx.get_hash()).collect();
 
     // setup testnet
     let mut net = Testnet::create_with(2, MockEthProvider::default()).await;

crates/transaction-pool/src/pool/mod.rs

@@ -1302,7 +1302,7 @@ mod tests {
 
             // Insert the sidecar into the blob store if the current index is within the blob limit.
             if n < blob_limit.max_txs {
-                blob_store.insert(tx.get_hash(), sidecar.clone()).unwrap();
+                blob_store.insert(*tx.get_hash(), sidecar.clone()).unwrap();
             }
 
             // Add the transaction to the pool with external origin and valid outcome.

crates/transaction-pool/src/pool/txpool.rs

@@ -2486,8 +2486,7 @@ mod tests {
         let tx = MockTransaction::eip1559().inc_price().inc_limit();
         let first = f.validated(tx.clone());
         pool.insert_tx(first, on_chain_balance, on_chain_nonce).unwrap();
-        let tx =
+        let tx = MockTransaction::eip4844().set_sender(tx.sender()).inc_price_by(100).inc_limit();
-            MockTransaction::eip4844().set_sender(tx.get_sender()).inc_price_by(100).inc_limit();
         let blob = f.validated(tx);
         let err = pool.insert_tx(blob, on_chain_balance, on_chain_nonce).unwrap_err();
         assert!(matches!(err, InsertErr::TxTypeConflict { .. }), "{err:?}");
@@ -2502,8 +2501,7 @@ mod tests {
         let tx = MockTransaction::eip4844().inc_price().inc_limit();
         let first = f.validated(tx.clone());
         pool.insert_tx(first, on_chain_balance, on_chain_nonce).unwrap();
-        let tx =
+        let tx = MockTransaction::eip1559().set_sender(tx.sender()).inc_price_by(100).inc_limit();
-            MockTransaction::eip1559().set_sender(tx.get_sender()).inc_price_by(100).inc_limit();
         let tx = f.validated(tx);
         let err = pool.insert_tx(tx, on_chain_balance, on_chain_nonce).unwrap_err();
         assert!(matches!(err, InsertErr::TxTypeConflict { .. }), "{err:?}");
@@ -2622,7 +2620,7 @@ mod tests {
 
         assert_eq!(
             pool.max_account_slots,
-            pool.tx_count(f.ids.sender_id(&tx.get_sender()).unwrap())
+            pool.tx_count(f.ids.sender_id(tx.get_sender()).unwrap())
         );
 
         let err =
@@ -2654,7 +2652,7 @@ mod tests {
 
         assert_eq!(
             pool.max_account_slots,
-            pool.tx_count(f.ids.sender_id(&tx.get_sender()).unwrap())
+            pool.tx_count(f.ids.sender_id(tx.get_sender()).unwrap())
         );
 
         pool.insert_tx(
@@ -2829,7 +2827,7 @@ mod tests {
         let mut changed_senders = HashMap::default();
         changed_senders.insert(
             id.sender,
-            SenderInfo { state_nonce: next.get_nonce(), balance: U256::from(1_000) },
+            SenderInfo { state_nonce: next.nonce(), balance: U256::from(1_000) },
         );
         let outcome = pool.update_accounts(changed_senders);
         assert_eq!(outcome.discarded.len(), 1);

crates/transaction-pool/src/test_utils/mock.rs

@@ -69,7 +69,7 @@ macro_rules! get_value {
             MockTransaction::Legacy { $field, .. } |
             MockTransaction::Eip1559 { $field, .. } |
             MockTransaction::Eip4844 { $field, .. } |
-            MockTransaction::Eip2930 { $field, .. } => $field.clone(),
+            MockTransaction::Eip2930 { $field, .. } => $field,
         }
     };
 }
@@ -91,7 +91,7 @@ macro_rules! make_setters_getters {
             }
 
             /// Gets the value of the specified field.
-            pub fn [<get_ $name>](&self) -> $t {
+            pub const fn [<get_ $name>](&self) -> &$t {
                 get_value!(self => $name)
             }
         )*}
@@ -582,30 +582,15 @@ impl PoolTransaction for MockTransaction {
     }
 
     fn hash(&self) -> &TxHash {
-        match self {
+        self.get_hash()
-            Self::Legacy { hash, .. } |
-            Self::Eip1559 { hash, .. } |
-            Self::Eip4844 { hash, .. } |
-            Self::Eip2930 { hash, .. } => hash,
-        }
     }
 
     fn sender(&self) -> Address {
-        match self {
+        *self.get_sender()
-            Self::Legacy { sender, .. } |
-            Self::Eip1559 { sender, .. } |
-            Self::Eip4844 { sender, .. } |
-            Self::Eip2930 { sender, .. } => *sender,
-        }
     }
 
     fn nonce(&self) -> u64 {
-        match self {
+        *self.get_nonce()
-            Self::Legacy { nonce, .. } |
-            Self::Eip1559 { nonce, .. } |
-            Self::Eip4844 { nonce, .. } |
-            Self::Eip2930 { nonce, .. } => *nonce,
-        }
     }
 
     fn cost(&self) -> U256 {
@@ -622,7 +607,7 @@ impl PoolTransaction for MockTransaction {
     }
 
     fn gas_limit(&self) -> u64 {
-        self.get_gas_limit()
+        *self.get_gas_limit()
     }
 
     fn max_fee_per_gas(&self) -> u128 {
@@ -703,22 +688,12 @@ impl PoolTransaction for MockTransaction {
 
     /// Returns the input data associated with the transaction.
     fn input(&self) -> &[u8] {
-        match self {
+        self.get_input()
-            Self::Legacy { .. } => &[],
-            Self::Eip1559 { input, .. } |
-            Self::Eip4844 { input, .. } |
-            Self::Eip2930 { input, .. } => input,
-        }
     }
 
     /// Returns the size of the transaction.
     fn size(&self) -> usize {
-        match self {
+        *self.get_size()
-            Self::Legacy { size, .. } |
-            Self::Eip1559 { size, .. } |
-            Self::Eip4844 { size, .. } |
-            Self::Eip2930 { size, .. } => *size,
-        }
     }
 
     /// Returns the transaction type as a byte identifier.
@@ -1007,109 +982,14 @@ impl proptest::arbitrary::Arbitrary for MockTransaction {
     fn arbitrary_with(_: Self::Parameters) -> Self::Strategy {
         use proptest::prelude::Strategy;
         use proptest_arbitrary_interop::arb;
-        use reth_primitives_traits::size::InMemorySize;
 
-        arb::<(Transaction, Address, B256)>()
+        arb::<(TransactionSigned, Address)>()
-            .prop_map(|(tx, sender, tx_hash)| match &tx {
+            .prop_map(|(signed_transaction, signer)| {
-                Transaction::Legacy(TxLegacy {
+                TransactionSignedEcRecovered::from_signed_transaction(signed_transaction, signer)
-                    chain_id,
+                    .try_into()
-                    nonce,
+                    .expect(
-                    gas_price,
+                        "Failed to create an Arbitrary MockTransaction via TransactionSignedEcRecovered",
-                    gas_limit,
+                    )
-                    to,
-                    value,
-                    input,
-                }) => Self::Legacy {
-                    chain_id: *chain_id,
-                    sender,
-                    hash: tx_hash,
-                    nonce: *nonce,
-                    gas_price: *gas_price,
-                    gas_limit: { *gas_limit },
-                    to: *to,
-                    value: *value,
-                    input: input.clone(),
-                    size: tx.size(),
-                },
-
-                Transaction::Eip2930(TxEip2930 {
-                    chain_id,
-                    nonce,
-                    gas_price,
-                    gas_limit,
-                    to,
-                    value,
-                    access_list,
-                    input,
-                }) => Self::Eip2930 {
-                    chain_id: *chain_id,
-                    sender,
-                    hash: tx_hash,
-                    nonce: *nonce,
-                    gas_price: *gas_price,
-                    gas_limit: { *gas_limit },
-                    to: *to,
-                    value: *value,
-                    input: input.clone(),
-                    access_list: access_list.clone(),
-                    size: tx.size(),
-                },
-                Transaction::Eip1559(TxEip1559 {
-                    chain_id,
-                    nonce,
-                    gas_limit,
-                    max_fee_per_gas,
-                    max_priority_fee_per_gas,
-                    to,
-                    value,
-                    input,
-                    access_list,
-                }) => Self::Eip1559 {
-                    chain_id: *chain_id,
-                    sender,
-                    hash: tx_hash,
-                    nonce: *nonce,
-                    max_fee_per_gas: *max_fee_per_gas,
-                    max_priority_fee_per_gas: *max_priority_fee_per_gas,
-                    gas_limit: { *gas_limit },
-                    to: *to,
-                    value: *value,
-                    input: input.clone(),
-                    access_list: access_list.clone(),
-                    size: tx.size(),
-                },
-                Transaction::Eip4844(TxEip4844 {
-                    chain_id,
-                    nonce,
-                    gas_limit,
-                    max_fee_per_gas,
-                    max_priority_fee_per_gas,
-                    to,
-                    value,
-                    input,
-                    max_fee_per_blob_gas,
-                    access_list,
-                    blob_versioned_hashes: _,
-                }) => Self::Eip4844 {
-                    chain_id: *chain_id,
-                    sender,
-                    hash: tx_hash,
-                    nonce: *nonce,
-                    max_fee_per_gas: *max_fee_per_gas,
-                    max_priority_fee_per_gas: *max_priority_fee_per_gas,
-                    max_fee_per_blob_gas: *max_fee_per_blob_gas,
-                    gas_limit: { *gas_limit },
-                    to: *to,
-                    value: *value,
-                    input: input.clone(),
-                    access_list: access_list.clone(),
-                    // only generate a sidecar if it is a 4844 tx - also for the sake of
-                    // performance just use a default sidecar
-                    sidecar: BlobTransactionSidecar::default(),
-                    size: tx.size(),
-                },
-                #[allow(unreachable_patterns)]
-                _ => unimplemented!(),
             })
             .boxed()
     }
@@ -1128,8 +1008,8 @@ pub struct MockTransactionFactory {
 impl MockTransactionFactory {
     /// Generates a transaction ID for the given [`MockTransaction`].
     pub fn tx_id(&mut self, tx: &MockTransaction) -> TransactionId {
-        let sender = self.ids.sender_id_or_create(tx.get_sender());
+        let sender = self.ids.sender_id_or_create(tx.sender());
-        TransactionId::new(sender, tx.get_nonce())
+        TransactionId::new(sender, tx.nonce())
     }
 
     /// Validates a [`MockTransaction`] and returns a [`MockValidTx`].

crates/transaction-pool/tests/it/blobs.rs

@@ -3,7 +3,7 @@
 use reth_transaction_pool::{
     error::PoolErrorKind,
     test_utils::{MockTransaction, MockTransactionFactory, TestPoolBuilder},
-    TransactionOrigin, TransactionPool,
+    PoolTransaction, TransactionOrigin, TransactionPool,
 };
 
 #[tokio::test(flavor = "multi_thread")]
@@ -16,23 +16,22 @@ async fn blobs_exclusive() {
         .add_transaction(TransactionOrigin::External, blob_tx.transaction.clone())
         .await
         .unwrap();
-    assert_eq!(hash, blob_tx.transaction.get_hash());
+    assert_eq!(hash, *blob_tx.transaction.get_hash());
 
     let mut best_txns = txpool.best_transactions();
     assert_eq!(best_txns.next().unwrap().transaction.get_hash(), blob_tx.transaction.get_hash());
     assert!(best_txns.next().is_none());
 
-    let eip1559_tx = MockTransaction::eip1559()
+    let eip1559_tx =
-        .set_sender(blob_tx.transaction.get_sender())
+        MockTransaction::eip1559().set_sender(blob_tx.transaction.sender()).inc_price_by(10_000);
-        .inc_price_by(10_000);
 
     let res =
         txpool.add_transaction(TransactionOrigin::External, eip1559_tx.clone()).await.unwrap_err();
 
-    assert_eq!(res.hash, eip1559_tx.get_hash());
+    assert_eq!(res.hash, *eip1559_tx.get_hash());
     match res.kind {
         PoolErrorKind::ExistingConflictingTransactionType(addr, tx_type) => {
-            assert_eq!(addr, eip1559_tx.get_sender());
+            assert_eq!(addr, eip1559_tx.sender());
             assert_eq!(tx_type, eip1559_tx.tx_type());
         }
         _ => unreachable!(),

crates/transaction-pool/tests/it/evict.rs

@@ -8,7 +8,8 @@ use reth_transaction_pool::{
     test_utils::{
         MockFeeRange, MockTransactionDistribution, MockTransactionRatio, TestPool, TestPoolBuilder,
     },
-    BlockInfo, PoolConfig, SubPoolLimit, TransactionOrigin, TransactionPool, TransactionPoolExt,
+    BlockInfo, PoolConfig, PoolTransaction, SubPoolLimit, TransactionOrigin, TransactionPool,
+    TransactionPoolExt,
 };
 
 #[tokio::test(flavor = "multi_thread")]
@@ -87,7 +88,7 @@ async fn only_blobs_eviction() {
             let set = set.into_vec();
 
             // ensure that the first nonce is 0
-            assert_eq!(set[0].get_nonce(), 0);
+            assert_eq!(set[0].nonce(), 0);
 
             // and finally insert it into the pool
             let results = pool.add_transactions(TransactionOrigin::External, set).await;
@@ -194,7 +195,7 @@ async fn mixed_eviction() {
             );
 
             let set = set.into_inner().into_vec();
-            assert_eq!(set[0].get_nonce(), 0);
+            assert_eq!(set[0].nonce(), 0);
 
             let results = pool.add_transactions(TransactionOrigin::External, set).await;
             for (i, result) in results.iter().enumerate() {

crates/transaction-pool/tests/it/listeners.rs

@@ -33,11 +33,11 @@ async fn txpool_listener_all() {
 
     let added_result =
         txpool.add_transaction(TransactionOrigin::External, transaction.transaction.clone()).await;
-    assert_matches!(added_result, Ok(hash) if hash == transaction.transaction.get_hash());
+    assert_matches!(added_result, Ok(hash) if hash == *transaction.transaction.get_hash());
 
     assert_matches!(
         all_tx_events.next().await,
-        Some(FullTransactionEvent::Pending(hash)) if hash == transaction.transaction.get_hash()
+        Some(FullTransactionEvent::Pending(hash)) if hash == *transaction.transaction.get_hash()
     );
 }
 

crates/transaction-pool/tests/it/pending.rs

@@ -12,7 +12,7 @@ async fn txpool_new_pending_txs() {
 
     let added_result =
         txpool.add_transaction(TransactionOrigin::External, transaction.transaction.clone()).await;
-    assert_matches!(added_result, Ok(hash) if hash == transaction.transaction.get_hash());
+    assert_matches!(added_result, Ok(hash) if hash == *transaction.transaction.get_hash());
 
     let mut best_txns = txpool.best_transactions();
     assert_matches!(best_txns.next(), Some(tx) if tx.transaction.get_hash() == transaction.transaction.get_hash());
@@ -20,6 +20,6 @@ async fn txpool_new_pending_txs() {
     let transaction = mock_tx_factory.create_eip1559();
     let added_result =
         txpool.add_transaction(TransactionOrigin::External, transaction.transaction.clone()).await;
-    assert_matches!(added_result, Ok(hash) if hash == transaction.transaction.get_hash());
+    assert_matches!(added_result, Ok(hash) if hash == *transaction.transaction.get_hash());
     assert_matches!(best_txns.next(), Some(tx) if tx.transaction.get_hash() == transaction.transaction.get_hash());
 }