Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: refactor MockTransaction #12627

Merged
merged 1 commit into from
Nov 18, 2024
Merged

chore: refactor MockTransaction #12627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/net/network/src/transactions/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>>();

Expand Down
2 changes: 1 addition & 1 deletion crates/net/network/tests/it/big_pooled_txs_req.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion crates/transaction-pool/src/pool/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 5 additions & 7 deletions crates/transaction-pool/src/pool/txpool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
MockTransaction::eip4844().set_sender(tx.get_sender()).inc_price_by(100).inc_limit();
let tx = MockTransaction::eip4844().set_sender(tx.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:?}");
Expand All @@ -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 =
MockTransaction::eip1559().set_sender(tx.get_sender()).inc_price_by(100).inc_limit();
let tx = MockTransaction::eip1559().set_sender(tx.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:?}");
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down
156 changes: 18 additions & 138 deletions crates/transaction-pool/src/test_utils/mock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ macro_rules! get_value {
MockTransaction::Legacy { $field, .. } |
MockTransaction::Eip1559 { $field, .. } |
MockTransaction::Eip4844 { $field, .. } |
MockTransaction::Eip2930 { $field, .. } => $field.clone(),
MockTransaction::Eip2930 { $field, .. } => $field,
}
};
}
Expand All @@ -90,7 +90,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)
}
)*}
Expand Down Expand Up @@ -581,30 +581,15 @@ impl PoolTransaction for MockTransaction {
}

fn hash(&self) -> &TxHash {
match self {
Self::Legacy { hash, .. } |
Self::Eip1559 { hash, .. } |
Self::Eip4844 { hash, .. } |
Self::Eip2930 { hash, .. } => hash,
}
self.get_hash()
}

fn sender(&self) -> Address {
match self {
Self::Legacy { sender, .. } |
Self::Eip1559 { sender, .. } |
Self::Eip4844 { sender, .. } |
Self::Eip2930 { sender, .. } => *sender,
}
*self.get_sender()
}

fn nonce(&self) -> u64 {
match self {
Self::Legacy { nonce, .. } |
Self::Eip1559 { nonce, .. } |
Self::Eip4844 { nonce, .. } |
Self::Eip2930 { nonce, .. } => *nonce,
}
*self.get_nonce()
}

fn cost(&self) -> U256 {
Expand All @@ -621,7 +606,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 {
Expand Down Expand Up @@ -702,22 +687,12 @@ impl PoolTransaction for MockTransaction {

/// Returns the input data associated with the transaction.
fn input(&self) -> &[u8] {
match self {
Self::Legacy { .. } => &[],
Self::Eip1559 { input, .. } |
Self::Eip4844 { input, .. } |
Self::Eip2930 { input, .. } => input,
}
self.get_input()
}

/// Returns the size of the transaction.
fn size(&self) -> usize {
match self {
Self::Legacy { size, .. } |
Self::Eip1559 { size, .. } |
Self::Eip4844 { size, .. } |
Self::Eip2930 { size, .. } => *size,
}
*self.get_size()
}

/// Returns the transaction type as a byte identifier.
Expand Down Expand Up @@ -1006,109 +981,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)>()
.prop_map(|(tx, sender, tx_hash)| match &tx {
Transaction::Legacy(TxLegacy {
chain_id,
nonce,
gas_price,
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!(),

arb::<(TransactionSigned, Address)>()
.prop_map(|(signed_transaction, signer)| {
TransactionSignedEcRecovered::from_signed_transaction(signed_transaction, signer)
.try_into()
.expect(
"Failed to create an Arbitrary MockTransaction via TransactionSignedEcRecovered",
)
})
.boxed()
}
Expand All @@ -1127,8 +1007,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());
TransactionId::new(sender, tx.get_nonce())
let sender = self.ids.sender_id_or_create(tx.sender());
TransactionId::new(sender, tx.nonce())
}

/// Validates a [`MockTransaction`] and returns a [`MockValidTx`].
Expand Down
13 changes: 6 additions & 7 deletions crates/transaction-pool/tests/it/blobs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand All @@ -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()
.set_sender(blob_tx.transaction.get_sender())
.inc_price_by(10_000);
let eip1559_tx =
MockTransaction::eip1559().set_sender(blob_tx.transaction.sender()).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!(),
Expand Down
7 changes: 4 additions & 3 deletions crates/transaction-pool/tests/it/evict.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions crates/transaction-pool/tests/it/listeners.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
}

Expand Down
4 changes: 2 additions & 2 deletions crates/transaction-pool/tests/it/pending.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ 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());
assert_matches!(best_txns.next(), None);
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());
}
Loading