From 9dc71855a8878ce460c75de4e596459fbea293e4 Mon Sep 17 00:00:00 2001
From: trantorian <114066155+Trantorian1@users.noreply.github.com>
Date: Wed, 8 Jan 2025 19:14:37 +0100
Subject: [PATCH] test(mempool): added aged tx removal test

---
 .../madara/client/mempool/src/inner/limits.rs |   1 +
 crates/madara/client/mempool/src/inner/mod.rs | 116 ++---
 crates/madara/client/mempool/src/lib.rs       | 403 +++++++++++++++++-
 3 files changed, 462 insertions(+), 58 deletions(-)

diff --git a/crates/madara/client/mempool/src/inner/limits.rs b/crates/madara/client/mempool/src/inner/limits.rs
index 0484963e3..e57f3bfcd 100644
--- a/crates/madara/client/mempool/src/inner/limits.rs
+++ b/crates/madara/client/mempool/src/inner/limits.rs
@@ -47,6 +47,7 @@ pub enum MempoolLimitReached {
     Age { max: Duration },
 }
 
+#[derive(Debug)]
 pub(crate) struct TransactionCheckedLimits {
     check_tx_limit: bool,
     check_declare_limit: bool,
diff --git a/crates/madara/client/mempool/src/inner/mod.rs b/crates/madara/client/mempool/src/inner/mod.rs
index 6890f74ef..0cbf225e9 100644
--- a/crates/madara/client/mempool/src/inner/mod.rs
+++ b/crates/madara/client/mempool/src/inner/mod.rs
@@ -8,12 +8,9 @@ use blockifier::transaction::transaction_execution::Transaction;
 use deployed_contracts::DeployedContracts;
 use mc_db::mempool_db::{NonceInfo, NonceStatus};
 use mp_convert::ToFelt;
-use starknet_api::{
-    core::{ContractAddress, Nonce},
-    StarknetApiError,
-};
+use starknet_api::core::{ContractAddress, Nonce};
 use starknet_types_core::felt::Felt;
-use std::collections::{hash_map, BTreeSet, HashMap};
+use std::collections::{btree_map, hash_map, BTreeSet, HashMap};
 
 mod deployed_contracts;
 mod intent;
@@ -199,6 +196,8 @@ impl CheckInvariants for MempoolInner {
         }
 
         for (contract_address, queue) in self.tx_intent_queue_pending.iter() {
+            assert!(!queue.is_empty());
+
             for intent in queue.iter() {
                 // TODO: check the nonce against the db to make sure this intent is
                 // really pending
@@ -274,11 +273,9 @@ impl MempoolInner {
         // delete age-exceeded txs from the mempool
         // todo(perf): this may want to limit this check once every few seconds
         // to avoid it being in the hot path?
-        self.remove_age_exceeded_txs();
-
-        // check limits
         let limits_for_tx = TransactionCheckedLimits::limits_for(&mempool_tx);
         if !force {
+            self.remove_age_exceeded_txs();
             self.limiter.check_insert_limits(&limits_for_tx)?;
         }
 
@@ -338,7 +335,7 @@ impl MempoolInner {
                         // it is split into individual sub-queues for each
                         // contract address
                         let queue =
-                            self.tx_intent_queue_pending.entry(contract_address).or_insert_with(|| BTreeSet::default());
+                            self.tx_intent_queue_pending.entry(contract_address).or_insert_with(BTreeSet::default);
 
                         // Remove old value (if collision and force == true)
                         if let ReplacedState::Replaced { previous } = replaced {
@@ -381,17 +378,17 @@ impl MempoolInner {
                         nonce_next: nonce_info.nonce_next,
                         phantom: Default::default(),
                     }),
-                    NonceStatus::Pending => self
-                        .tx_intent_queue_pending
-                        .entry(contract_address)
-                        .or_insert_with(|| BTreeSet::default())
-                        .insert(TransactionIntentPending {
-                            contract_address,
-                            timestamp: arrived_at,
-                            nonce: nonce_info.nonce,
-                            nonce_next: nonce_info.nonce_next,
-                            phantom: Default::default(),
-                        }),
+                    NonceStatus::Pending => {
+                        self.tx_intent_queue_pending.entry(contract_address).or_insert_with(BTreeSet::default).insert(
+                            TransactionIntentPending {
+                                contract_address,
+                                timestamp: arrived_at,
+                                nonce: nonce_info.nonce,
+                                nonce_next: nonce_info.nonce_next,
+                                phantom: Default::default(),
+                            },
+                        )
+                    }
                 };
                 debug_assert!(inserted);
 
@@ -418,27 +415,25 @@ impl MempoolInner {
         // ordered by timestamp, so as soon as we find a transaction which has
         // not exceeded its max age (and that transaction supports age limits)
         // we know no more transactions can be removed.
-        //
-        // INFO: we traverse this in reverse as the intents at the end of the
-        // queue have the highest chance of pointing to transactions which have
-        // exceeded their age limit. It is very unlikely that intents at the
-        // front of the queue point to such transactions. If that were the case,
-        // it would be handled by pop_next anyway. This should help with
-        // maximizing the number of removals and keeping the mempool from being
-        // congested.
-        while let Some(intent) = self.tx_intent_queue_ready.last() {
-            let tx = self
-                .nonce_mapping
-                .get_mut(&intent.contract_address)
-                .expect("Nonce chain does not match tx queue")
-                .transactions
-                .get(&intent.nonce)
-                .expect("Nonce chain without a tx");
+        while let Some(intent) = self.tx_intent_queue_ready.first() {
+            let hash_map::Entry::Occupied(mut entry) = self.nonce_mapping.entry(intent.contract_address) else {
+                unreachable!("Nonce chain does not match tx queue");
+            };
 
-            let limits = TransactionCheckedLimits::limits_for(tx);
+            let nonce_mapping = entry.get_mut();
+            let btree_map::Entry::Occupied(nonce) = nonce_mapping.transactions.entry(intent.nonce) else {
+                unreachable!("Nonce chain without a tx");
+            };
+
+            let limits = TransactionCheckedLimits::limits_for(nonce.get());
 
             if self.limiter.tx_age_exceeded(&limits) {
-                self.tx_intent_queue_ready.pop_last();
+                nonce.remove();
+                if nonce_mapping.transactions.is_empty() {
+                    entry.remove();
+                }
+
+                self.tx_intent_queue_ready.pop_first();
             } else if limits.checks_age() {
                 break;
             }
@@ -462,24 +457,45 @@ impl MempoolInner {
         //
         // How do we reconcile (3.) and (4.)? We need a queue wich is sometimes
         // sorted by account, and sometimes sorted by timestamp!
-        for queue in self.tx_intent_queue_pending.values_mut() {
-            while let Some(intent) = queue.last() {
-                let tx = self
-                    .nonce_mapping
-                    .get_mut(&intent.contract_address)
-                    .expect("Nonce chain does not match tx queue")
-                    .transactions
-                    .get(&intent.nonce)
-                    .expect("Nonce chain without a tx");
-
-                let limits = TransactionCheckedLimits::limits_for(tx);
+        // PERF: remove or limit this allocation, perhaps by keeping a count
+        // of the current number of pending transactions. Maybe this is an
+        // indication that we need to change the underlying data structure
+        let mut to_remove = Vec::default();
+        for (contract_address, queue) in self.tx_intent_queue_pending.iter_mut() {
+            while let Some(intent) = queue.first() {
+                let hash_map::Entry::Occupied(mut entry) = self.nonce_mapping.entry(intent.contract_address) else {
+                    unreachable!("Nonce chain does not match tx queue");
+                };
+
+                let nonce_mapping = entry.get_mut();
+                let btree_map::Entry::Occupied(nonce) = nonce_mapping.transactions.entry(intent.nonce) else {
+                    unreachable!("Nonce chain without a tx");
+                };
+
+                let limits = TransactionCheckedLimits::limits_for(nonce.get());
 
                 if self.limiter.tx_age_exceeded(&limits) {
-                    queue.pop_last();
+                    nonce.remove();
+                    if nonce_mapping.transactions.is_empty() {
+                        entry.remove();
+                    }
+
+                    queue.pop_first();
                 } else if limits.checks_age() {
                     break;
                 }
             }
+
+            if queue.is_empty() {
+                to_remove.push(*contract_address);
+            }
+        }
+
+        // We can't do this inside of the loop above due to rules on mutable
+        // ownership
+        for contract_address in to_remove.iter() {
+            let removed = self.tx_intent_queue_pending.remove(contract_address);
+            debug_assert!(removed.is_some());
         }
     }
 
diff --git a/crates/madara/client/mempool/src/lib.rs b/crates/madara/client/mempool/src/lib.rs
index cc99186a6..2cc292dc6 100644
--- a/crates/madara/client/mempool/src/lib.rs
+++ b/crates/madara/client/mempool/src/lib.rs
@@ -436,6 +436,7 @@ impl MempoolProvider for Mempool {
         }
         let mut inner = self.inner.write().expect("Poisoned lock");
         inner.insert_txs(txs, force)?;
+
         Ok(())
     }
     fn chain_id(&self) -> Felt {
@@ -519,6 +520,8 @@ pub(crate) fn clone_transaction(tx: &Transaction) -> Transaction {
 
 #[cfg(test)]
 mod test {
+    use std::time::Duration;
+
     use super::*;
 
     #[rstest::fixture]
@@ -541,12 +544,17 @@ mod test {
     }
 
     #[rstest::fixture]
-    fn tx_account_v0_valid() -> blockifier::transaction::transaction_execution::Transaction {
+    fn tx_account_v0_valid(
+        #[default(Felt::ZERO)] contract_address: Felt,
+    ) -> blockifier::transaction::transaction_execution::Transaction {
         blockifier::transaction::transaction_execution::Transaction::AccountTransaction(
             blockifier::transaction::account_transaction::AccountTransaction::Invoke(
                 blockifier::transaction::transactions::InvokeTransaction {
                     tx: starknet_api::transaction::InvokeTransaction::V0(
-                        starknet_api::transaction::InvokeTransactionV0::default(),
+                        starknet_api::transaction::InvokeTransactionV0 {
+                            contract_address: ContractAddress::try_from(contract_address).unwrap(),
+                            ..Default::default()
+                        },
                     ),
                     tx_hash: starknet_api::transaction::TransactionHash::default(),
                     only_query: false,
@@ -613,17 +621,396 @@ mod test {
         mempool.inner.read().expect("Poisoned lock").check_invariants();
     }
 
+    /// This test makes sure that old transactions are removed from the mempool,
+    /// whether they be represented by ready or pending intents.
+    ///
+    /// # Setup
+    ///
+    /// - We assume `tx_*_n `are from the same contract but with increasing
+    ///   nonces.
+    ///
+    /// - `tx_new` are transactions which _should not_ be removed from the
+    ///   mempool, `tx_old` are transactions which _should_ be removed from the
+    ///   mempool
+    ///
+    /// - `tx_new_3`, `tx_old_3` and `tx_new_2_bis` and `tx_old_4` are in the
+    ///   pending queue, all other transactions are in the ready queue.
     #[rstest::rstest]
     fn mempool_remove_aged_tx_pass(
         backend: Arc<mc_db::MadaraBackend>,
         l1_data_provider: Arc<MockL1DataProvider>,
-        #[from(tx_account_v0_valid)] tx_new_1: blockifier::transaction::transaction_execution::Transaction,
-        #[from(tx_account_v0_valid)] tx_new_2: blockifier::transaction::transaction_execution::Transaction,
-        #[from(tx_account_v0_valid)] tx_old_1: blockifier::transaction::transaction_execution::Transaction,
-        #[from(tx_account_v0_valid)] tx_old_2: blockifier::transaction::transaction_execution::Transaction,
-        #[from(tx_account_v0_valid)] tx_old_3: blockifier::transaction::transaction_execution::Transaction,
+        #[from(tx_account_v0_valid)] // We are reusing the tx_account_v0_valid fixture...
+        #[with(Felt::ZERO)] // ...with different arguments
+        tx_new_1: blockifier::transaction::transaction_execution::Transaction,
+        #[from(tx_account_v0_valid)]
+        #[with(Felt::ONE)]
+        tx_new_2: blockifier::transaction::transaction_execution::Transaction,
+        #[from(tx_account_v0_valid)]
+        #[with(Felt::ONE)]
+        tx_new_3: blockifier::transaction::transaction_execution::Transaction,
+        #[from(tx_account_v0_valid)]
+        #[with(Felt::ZERO)]
+        tx_old_1: blockifier::transaction::transaction_execution::Transaction,
+        #[from(tx_account_v0_valid)]
+        #[with(Felt::ONE)]
+        tx_old_2: blockifier::transaction::transaction_execution::Transaction,
+        #[from(tx_account_v0_valid)]
+        #[with(Felt::TWO)]
+        tx_old_3: blockifier::transaction::transaction_execution::Transaction,
+        #[from(tx_account_v0_valid)]
+        #[with(Felt::THREE)]
+        tx_old_4: blockifier::transaction::transaction_execution::Transaction,
     ) {
-        todo!()
+        let mempool = Mempool::new(
+            backend,
+            l1_data_provider,
+            MempoolLimits { max_age: Some(Duration::from_secs(3600)), ..MempoolLimits::for_testing() },
+        );
+
+        // ================================================================== //
+        //                                STEP 1                              //
+        // ================================================================== //
+
+        // First, we begin by inserting all our transactions and making sure
+        // they are in the ready as well as the pending intent queues.
+        let arrived_at = ArrivedAtTimestamp::now();
+        let tx_new_1_mempool = MempoolTransaction {
+            tx: tx_new_1,
+            arrived_at,
+            converted_class: None,
+            nonce: Nonce(Felt::ZERO),
+            nonce_next: Nonce(Felt::ONE),
+        };
+        let res = mempool.inner.write().expect("Poisoned lock").insert_tx(
+            tx_new_1_mempool.clone(),
+            true,
+            false,
+            NonceInfo::ready(Nonce(Felt::ZERO), Nonce(Felt::ONE)),
+        );
+        assert!(res.is_ok());
+        assert!(
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready.contains(&TransactionIntentReady {
+                contract_address: **tx_new_1_mempool.contract_address(),
+                timestamp: tx_new_1_mempool.arrived_at,
+                nonce: tx_new_1_mempool.nonce,
+                nonce_next: tx_new_1_mempool.nonce_next,
+                phantom: std::marker::PhantomData
+            }),
+            "ready transaction intents are: {:#?}\npending transaction intents are: {:#?}",
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready,
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_pending
+        );
+
+        let arrived_at = ArrivedAtTimestamp::now();
+        let tx_new_2_mempool = MempoolTransaction {
+            tx: tx_new_2,
+            arrived_at,
+            converted_class: None,
+            nonce: Nonce(Felt::ZERO),
+            nonce_next: Nonce(Felt::ONE),
+        };
+        let res = mempool.inner.write().expect("Poisoned lock").insert_tx(
+            tx_new_2_mempool.clone(),
+            true,
+            false,
+            NonceInfo::ready(Nonce(Felt::ZERO), Nonce(Felt::ONE)),
+        );
+        assert!(res.is_ok());
+        assert!(
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready.contains(&TransactionIntentReady {
+                contract_address: **tx_new_2_mempool.contract_address(),
+                timestamp: tx_new_2_mempool.arrived_at,
+                nonce: tx_new_2_mempool.nonce,
+                nonce_next: tx_new_2_mempool.nonce_next,
+                phantom: std::marker::PhantomData
+            }),
+            "ready transaction intents are: {:#?}\npending transaction intents are: {:#?}",
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready,
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_pending
+        );
+
+        let arrived_at = ArrivedAtTimestamp::now();
+        let tx_new_3_mempool = MempoolTransaction {
+            tx: tx_new_3,
+            arrived_at,
+            converted_class: None,
+            nonce: Nonce(Felt::ONE),
+            nonce_next: Nonce(Felt::TWO),
+        };
+        let res = mempool.inner.write().expect("Poisoned lock").insert_tx(
+            tx_new_3_mempool.clone(),
+            true,
+            false,
+            NonceInfo::pending(Nonce(Felt::ONE), Nonce(Felt::TWO)),
+        );
+        assert!(res.is_ok());
+        assert!(
+            mempool
+                .inner
+                .read()
+                .expect("Poisoned lock")
+                .tx_intent_queue_pending
+                .get(&**tx_new_3_mempool.contract_address())
+                .expect("Missing nonce mapping for tx_new_3")
+                .contains(&TransactionIntentPending {
+                    contract_address: **tx_new_3_mempool.contract_address(),
+                    timestamp: tx_new_3_mempool.arrived_at,
+                    nonce: tx_new_3_mempool.nonce,
+                    nonce_next: tx_new_3_mempool.nonce_next,
+                    phantom: std::marker::PhantomData
+                }),
+            "ready transaction intents are: {:#?}\npending transaction intents are: {:#?}",
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready,
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_pending
+        );
+
+        let arrived_at = ArrivedAtTimestamp::UNIX_EPOCH;
+        let tx_old_1_mempool = MempoolTransaction {
+            tx: tx_old_1,
+            arrived_at,
+            converted_class: None,
+            nonce: Nonce(Felt::ONE),
+            nonce_next: Nonce(Felt::TWO),
+        };
+        let res = mempool.inner.write().expect("Poisoned lock").insert_tx(
+            tx_old_1_mempool.clone(),
+            true,
+            false,
+            NonceInfo::ready(Nonce(Felt::ONE), Nonce(Felt::TWO)),
+        );
+        assert!(res.is_ok());
+        assert!(
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready.contains(&TransactionIntentReady {
+                contract_address: **tx_old_1_mempool.contract_address(),
+                timestamp: tx_old_1_mempool.arrived_at,
+                nonce: tx_old_1_mempool.nonce,
+                nonce_next: tx_old_1_mempool.nonce_next,
+                phantom: std::marker::PhantomData
+            }),
+            "ready transaction intents are: {:#?}\npending transaction intents are: {:#?}",
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready,
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_pending
+        );
+
+        let arrived_at = ArrivedAtTimestamp::UNIX_EPOCH;
+        let tx_old_2_mempool = MempoolTransaction {
+            tx: tx_old_2,
+            arrived_at,
+            converted_class: None,
+            nonce: Nonce(Felt::ONE),
+            nonce_next: Nonce(Felt::TWO),
+        };
+        let res = mempool.inner.write().expect("Poisoned lock").insert_tx(
+            tx_old_2_mempool.clone(),
+            true,
+            false,
+            NonceInfo::ready(Nonce(Felt::ONE), Nonce(Felt::TWO)),
+        );
+        assert!(res.is_ok());
+        assert!(
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready.contains(&TransactionIntentReady {
+                contract_address: **tx_old_2_mempool.contract_address(),
+                timestamp: tx_old_2_mempool.arrived_at,
+                nonce: tx_old_2_mempool.nonce,
+                nonce_next: tx_old_2_mempool.nonce_next,
+                phantom: std::marker::PhantomData
+            }),
+            "ready transaction intents are: {:#?}\npending transaction intents are: {:#?}",
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready,
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_pending
+        );
+
+        let arrived_at = ArrivedAtTimestamp::UNIX_EPOCH;
+        let tx_old_3_mempool = MempoolTransaction {
+            tx: tx_old_3,
+            arrived_at,
+            converted_class: None,
+            nonce: Nonce(Felt::TWO),
+            nonce_next: Nonce(Felt::THREE),
+        };
+        let res = mempool.inner.write().expect("Poisoned lock").insert_tx(
+            tx_old_3_mempool.clone(),
+            true,
+            false,
+            NonceInfo::pending(Nonce(Felt::TWO), Nonce(Felt::THREE)),
+        );
+        assert!(res.is_ok());
+        assert!(
+            mempool
+                .inner
+                .read()
+                .expect("Poisoned lock")
+                .tx_intent_queue_pending
+                .get(&**tx_old_3_mempool.contract_address())
+                .expect("Missing nonce mapping for tx_old_3")
+                .contains(&TransactionIntentPending {
+                    contract_address: **tx_old_3_mempool.contract_address(),
+                    timestamp: tx_old_3_mempool.arrived_at,
+                    nonce: tx_old_3_mempool.nonce,
+                    nonce_next: tx_old_3_mempool.nonce_next,
+                    phantom: std::marker::PhantomData
+                }),
+            "ready transaction intents are: {:#?}\npending transaction intents are: {:#?}",
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready,
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_pending
+        );
+
+        let arrived_at = ArrivedAtTimestamp::UNIX_EPOCH;
+        let tx_old_4_mempool = MempoolTransaction {
+            tx: tx_old_4,
+            arrived_at,
+            converted_class: None,
+            nonce: Nonce(Felt::ONE),
+            nonce_next: Nonce(Felt::TWO),
+        };
+        let res = mempool.inner.write().expect("Poisoned lock").insert_tx(
+            tx_old_4_mempool.clone(),
+            true,
+            false,
+            NonceInfo::pending(Nonce(Felt::ONE), Nonce(Felt::TWO)),
+        );
+        assert!(res.is_ok());
+        assert!(
+            mempool
+                .inner
+                .read()
+                .expect("Poisoned lock")
+                .tx_intent_queue_pending
+                .get(&**tx_old_4_mempool.contract_address())
+                .expect("Missing nonce_mapping for tx_old_4")
+                .contains(&TransactionIntentPending {
+                    contract_address: **tx_old_4_mempool.contract_address(),
+                    timestamp: tx_old_4_mempool.arrived_at,
+                    nonce: tx_old_4_mempool.nonce,
+                    nonce_next: tx_old_4_mempool.nonce_next,
+                    phantom: std::marker::PhantomData
+                }),
+            "ready transaction intents are: {:#?}\npending transaction intents are: {:#?}",
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready,
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_pending
+        );
+
+        // Make sure we have not entered an invalid state.
+        mempool.inner.read().expect("Poisoned lock").check_invariants();
+
+        // ================================================================== //
+        //                                STEP 2                              //
+        // ================================================================== //
+
+        // Now we actually remove the transactions. All the old transactions
+        // should be removed.
+        mempool.inner.write().expect("Poisoned lock").remove_age_exceeded_txs();
+
+        // tx_new_1 and tx_new_2 should still be in the mempool
+        assert!(
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready.contains(&TransactionIntentReady {
+                contract_address: **tx_new_1_mempool.contract_address(),
+                timestamp: tx_new_1_mempool.arrived_at,
+                nonce: tx_new_1_mempool.nonce,
+                nonce_next: tx_new_1_mempool.nonce_next,
+                phantom: std::marker::PhantomData
+            }),
+            "ready transaction intents are: {:#?}\npending transaction intents are: {:#?}",
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready,
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_pending
+        );
+        assert!(
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready.contains(&TransactionIntentReady {
+                contract_address: **tx_new_2_mempool.contract_address(),
+                timestamp: tx_new_2_mempool.arrived_at,
+                nonce: tx_new_2_mempool.nonce,
+                nonce_next: tx_new_2_mempool.nonce_next,
+                phantom: std::marker::PhantomData
+            }),
+            "ready transaction intents are: {:#?}\npending transaction intents are: {:#?}",
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready,
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_pending
+        );
+
+        // tx_old_1 and tx_old_2 should no longer be in the ready queue
+        assert!(
+            !mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready.contains(&TransactionIntentReady {
+                contract_address: **tx_old_1_mempool.contract_address(),
+                timestamp: tx_old_1_mempool.arrived_at,
+                nonce: tx_old_1_mempool.nonce,
+                nonce_next: tx_old_1_mempool.nonce_next,
+                phantom: std::marker::PhantomData
+            }),
+            "ready transaction intents are: {:#?}\npending transaction intents are: {:#?}",
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready,
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_pending
+        );
+        assert!(
+            !mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready.contains(&TransactionIntentReady {
+                contract_address: **tx_old_2_mempool.contract_address(),
+                timestamp: tx_old_2_mempool.arrived_at,
+                nonce: tx_old_2_mempool.nonce,
+                nonce_next: tx_old_2_mempool.nonce_next,
+                phantom: std::marker::PhantomData
+            }),
+            "ready transaction intents are: {:#?}\npending transaction intents are: {:#?}",
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready,
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_pending
+        );
+
+        // tx_new_3 should still be in the pending queue but tx_old_3 should not
+        assert!(
+            mempool
+                .inner
+                .read()
+                .expect("Poisoned lock")
+                .tx_intent_queue_pending
+                .get(&**tx_new_3_mempool.contract_address())
+                .expect("Missing nonce mapping for tx_new_3")
+                .contains(&TransactionIntentPending {
+                    contract_address: **tx_new_3_mempool.contract_address(),
+                    timestamp: tx_new_3_mempool.arrived_at,
+                    nonce: tx_new_3_mempool.nonce,
+                    nonce_next: tx_new_3_mempool.nonce_next,
+                    phantom: std::marker::PhantomData
+                }),
+            "ready transaction intents are: {:#?}\npending transaction intents are: {:#?}",
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready,
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_pending
+        );
+        assert!(
+            !mempool
+                .inner
+                .read()
+                .expect("Poisoned lock")
+                .tx_intent_queue_pending
+                .get(&**tx_old_3_mempool.contract_address())
+                .expect("Missing nonce mapping for tx_old_3")
+                .contains(&TransactionIntentPending {
+                    contract_address: **tx_old_3_mempool.contract_address(),
+                    timestamp: tx_old_3_mempool.arrived_at,
+                    nonce: tx_old_3_mempool.nonce,
+                    nonce_next: tx_old_3_mempool.nonce_next,
+                    phantom: std::marker::PhantomData
+                }),
+            "ready transaction intents are: {:#?}\npending transaction intents are: {:#?}",
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready,
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_pending
+        );
+
+        // tx_old_4 should no longer be in the pending queue and since it was
+        // the only transaction for that contract address, that pending queue
+        // should have been emptied
+        assert!(
+            mempool
+                .inner
+                .read()
+                .expect("Poisoned lock")
+                .tx_intent_queue_pending
+                .get(&**tx_old_4_mempool.contract_address())
+                .is_none(),
+            "ready transaction intents are: {:#?}\npending transaction intents are: {:#?}",
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_ready,
+            mempool.inner.read().expect("Poisoned lock").tx_intent_queue_pending
+        );
+
+        // Make sure we have not entered an invalid state.
+        mempool.inner.read().expect("Poisoned lock").check_invariants();
     }
 
     #[rstest::rstest]