From f1d94c05a32e43e56147d9643bdd65f602030878 Mon Sep 17 00:00:00 2001 From: Ayelet Zilber Date: Thu, 18 Jul 2024 15:09:11 +0300 Subject: [PATCH 1/3] test(mempool): refactor MempoolState to have Partial and Full states --- crates/mempool/src/mempool_test.rs | 78 ++++++++++++++++++++++++------ 1 file changed, 62 insertions(+), 16 deletions(-) diff --git a/crates/mempool/src/mempool_test.rs b/crates/mempool/src/mempool_test.rs index 8f015548..4bbc8083 100644 --- a/crates/mempool/src/mempool_test.rs +++ b/crates/mempool/src/mempool_test.rs @@ -19,32 +19,78 @@ use crate::transaction_queue::TransactionQueue; /// Represents the internal state of the mempool. /// Enables customized (and potentially inconsistent) creation for unit testing. -struct MempoolState { - tx_pool: TransactionPool, - tx_queue: TransactionQueue, +#[derive(Debug)] +struct MempoolState { + tx_pool: Option, + tx_queue: Option, + // PhantomData artificially use generic type, for the compiler. + _phantom: std::marker::PhantomData, } -impl MempoolState { - fn new(pool_txs: PoolTxs, queue_txs: QueueTxs) -> Self +#[derive(Debug)] +struct FullState; +#[allow(dead_code)] +#[derive(Debug)] +struct PartialState; + +impl MempoolState { + fn new(pool_txs: P, queue_txs: Q) -> Self + where + P: IntoIterator, + // TODO(Ayelet): Consider using `&ThinTransaction` instead of `TransactionReference`. + Q: IntoIterator, + { + Self { + tx_pool: Some(pool_txs.into_iter().collect()), + tx_queue: Some(queue_txs.into_iter().collect()), + _phantom: std::marker::PhantomData, + } + } +} + +impl MempoolState { + fn _with_pool

(pool_txs: P) -> Self + where + P: IntoIterator, + { + Self { + tx_pool: Some(pool_txs.into_iter().collect()), + tx_queue: None, + _phantom: std::marker::PhantomData, + } + } + + fn _with_queue(queue_txs: Q) -> Self where - PoolTxs: IntoIterator, - QueueTxs: IntoIterator, + Q: IntoIterator, { - let tx_pool: TransactionPool = pool_txs.into_iter().collect(); - let tx_queue: TransactionQueue = queue_txs.into_iter().collect(); - MempoolState { tx_pool, tx_queue } + Self { + tx_queue: Some(queue_txs.into_iter().collect()), + tx_pool: None, + _phantom: std::marker::PhantomData, + } } +} +impl MempoolState { fn assert_eq_mempool_state(&self, mempool: &Mempool) { - assert_eq!(self.tx_pool, mempool.tx_pool); - assert_eq!(self.tx_queue, mempool.tx_queue); + self.assert_eq_pool_state(mempool); + self.assert_eq_queue_state(mempool); + } + + fn assert_eq_pool_state(&self, mempool: &Mempool) { + assert_eq!(self.tx_pool.as_ref().unwrap(), &mempool.tx_pool); + } + + fn assert_eq_queue_state(&self, mempool: &Mempool) { + assert_eq!(self.tx_queue.as_ref().unwrap(), &mempool.tx_queue); } } -impl From for Mempool { - fn from(mempool_state: MempoolState) -> Mempool { - let MempoolState { tx_pool, tx_queue } = mempool_state; - Mempool { tx_pool, tx_queue } +impl From> for Mempool { + fn from(mempool_state: MempoolState) -> Mempool { + let MempoolState { tx_pool, tx_queue, _phantom: _ } = mempool_state; + Mempool { tx_pool: tx_pool.unwrap_or_default(), tx_queue: tx_queue.unwrap_or_default() } } } From a19df0178dfca255a72d42290c1d009b1814ef0d Mon Sep 17 00:00:00 2001 From: Ayelet Zilber <138376632+ayeletstarkware@users.noreply.github.com> Date: Thu, 25 Jul 2024 12:14:53 +0300 Subject: [PATCH 2/3] test(mempool): refactor test_add_tx_with_duplicate_tx to use mempool state (#444) --- crates/mempool/src/mempool_test.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/mempool/src/mempool_test.rs b/crates/mempool/src/mempool_test.rs index 4bbc8083..c71195c8 100644 --- a/crates/mempool/src/mempool_test.rs +++ b/crates/mempool/src/mempool_test.rs @@ -49,7 +49,7 @@ impl MempoolState { } impl MempoolState { - fn _with_pool

(pool_txs: P) -> Self + fn with_pool

(pool_txs: P) -> Self where P: IntoIterator, { @@ -305,17 +305,20 @@ fn test_new_with_duplicate_tx() { #[rstest] fn test_add_tx_with_duplicate_tx(mut mempool: Mempool) { + // Setup. let input = add_tx_input!(tip: 50, tx_hash: Felt::ONE); - let same_input = input.clone(); + let duplicate_input = input.clone(); + // Test. add_tx(&mut mempool, &input); - assert_matches!( - mempool.add_tx(same_input.clone()), + mempool.add_tx(duplicate_input), Err(MempoolError::DuplicateTransaction { .. }) ); - // Assert that the original tx remains in the pool after the failed attempt. - assert_eq_mempool_queue(&mempool, &[same_input.tx]) + + // Assert: the original transaction remains. + let expected_mempool_state = MempoolState::with_pool([input.tx]); + expected_mempool_state.assert_eq_pool_state(&mempool); } #[rstest] From 232db3d9db049f6feb94595edc8588fed6c8848e Mon Sep 17 00:00:00 2001 From: Ayelet Zilber <138376632+ayeletstarkware@users.noreply.github.com> Date: Thu, 25 Jul 2024 12:18:31 +0300 Subject: [PATCH 3/3] test(mempool): refactor test_tip_priority_over_tx_hash to use mempool state (#446) --- crates/mempool/src/mempool_test.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/mempool/src/mempool_test.rs b/crates/mempool/src/mempool_test.rs index c71195c8..a6af1fa9 100644 --- a/crates/mempool/src/mempool_test.rs +++ b/crates/mempool/src/mempool_test.rs @@ -60,7 +60,7 @@ impl MempoolState { } } - fn _with_queue(queue_txs: Q) -> Self + fn with_queue(queue_txs: Q) -> Self where Q: IntoIterator, { @@ -347,15 +347,25 @@ fn test_add_tx_with_identical_tip_succeeds(mut mempool: Mempool) { #[rstest] fn test_tip_priority_over_tx_hash(mut mempool: Mempool) { + // Setup. let input_big_tip_small_hash = add_tx_input!(tip: 2, tx_hash: Felt::ONE); // Create a transaction with identical tip, it should be allowed through since the priority // queue tie-breaks identical tips by other tx-unique identifiers (for example tx hash). let input_small_tip_big_hash = add_tx_input!(tip: 1, tx_hash: Felt::TWO, sender_address: "0x1"); + // Test. add_tx(&mut mempool, &input_big_tip_small_hash); add_tx(&mut mempool, &input_small_tip_big_hash); - assert_eq_mempool_queue(&mempool, &[input_big_tip_small_hash.tx, input_small_tip_big_hash.tx]) + + // Assert: ensure that the transaction with the higher tip is prioritized higher. + let expected_queue_txs = [ + TransactionReference::new(&input_big_tip_small_hash.tx), + TransactionReference::new(&input_small_tip_big_hash.tx), + ]; + let expected_mempool_state = MempoolState::with_queue(expected_queue_txs); + + expected_mempool_state.assert_eq_queue_state(&mempool); } #[rstest]