Add expired transactions to the mempool rejected list

zebrad/src/components/inbound/tests.rs

@@ -273,7 +273,7 @@ async fn mempool_transaction_expiration() -> Result<(), crate::BoxError> {
     // Get services
     let (
         inbound_service,
-        _mempool_guard,
+        mempool,
         _committed_blocks,
         _added_transactions,
         mut tx_verifier,
@@ -402,6 +402,23 @@ async fn mempool_transaction_expiration() -> Result<(), crate::BoxError> {
             "`MempoolTransactionIds` requests should always respond `Ok(Vec<UnminedTxId>)`"
         ),
     };
+    // Check if tx1 was added to the rejected list as well
+    let response = mempool
+        .clone()
+        .oneshot(mempool::Request::Queue(vec![tx1_id.into()]))
+        .await
+        .unwrap();
+    let queued_responses = match response {
+        mempool::Response::Queued(queue_responses) => queue_responses,
+        _ => unreachable!("will never happen in this test"),
+    };
+    assert_eq!(queued_responses.len(), 1);
+    assert_eq!(
+        queued_responses[0],
+        Err(mempool::MempoolError::StorageEffectsChain(
+            mempool::SameEffectsChainRejectionError::Expired
+        ))
+    );
 
     // Test transaction 2 is gossiped
     let mut hs = HashSet::new();

zebrad/src/components/mempool.rs

@@ -364,17 +364,25 @@ fn remove_expired_transactions(
     tip_height: zebra_chain::block::Height,
 ) {
     let mut txid_set = HashSet::new();
+    // we need a separate set since we need the original unmined ID for reject()
+    let mut unmined_id_set = HashSet::new();
 
     for t in storage.transactions() {
         if let Some(expiry_height) = t.transaction.expiry_height() {
             if tip_height >= expiry_height {
                 txid_set.insert(t.id.mined_id());
+                unmined_id_set.insert(t.id);
             }
         }
     }
 
     // expiry height is effecting data, so we match by non-malleable TXID
     storage.remove_same_effects(&txid_set);
+
+    // also reject it
+    for id in unmined_id_set {
+        storage.reject(id, SameEffectsChainRejectionError::Expired.into());
+    }
 }
 
 /// Add a transaction that failed download and verification to the rejected list

zebrad/src/components/mempool/storage.rs

@@ -59,7 +59,7 @@ pub enum SameEffectsTipRejectionError {
 ///
 /// Rollbacks and network upgrades clear these rejections, because they can lower the tip height,
 /// or change the consensus rules.
-#[derive(Error, Clone, Debug, PartialEq, Eq)]
+#[derive(Error, Clone, Debug, PartialEq, Eq, Hash)]
 #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))]
 #[allow(dead_code)]
 pub enum SameEffectsChainRejectionError {
@@ -106,11 +106,12 @@ pub struct Storage {
     /// Any transaction with the same `transaction::Hash` is invalid.
     tip_rejected_same_effects: HashMap<transaction::Hash, SameEffectsTipRejectionError>,
 
-    /// The set of transactions rejected for their effects, and their rejection reasons.
+    /// Sets of transactions rejected for their effects, keyed by rejection reason.
     /// These rejections apply until a rollback or network upgrade.
     ///
     /// Any transaction with the same `transaction::Hash` is invalid.
-    chain_rejected_same_effects: HashMap<transaction::Hash, SameEffectsChainRejectionError>,
+    chain_rejected_same_effects:
+        HashMap<SameEffectsChainRejectionError, HashSet<transaction::Hash>>,
 }
 
 impl Storage {
@@ -155,10 +156,11 @@ impl Storage {
         // TODO: use random weighted eviction as specified in ZIP-401 (#2780)
         if self.verified.len() > MEMPOOL_SIZE {
             for evicted_tx in self.verified.drain(MEMPOOL_SIZE..) {
-                let _ = self.chain_rejected_same_effects.insert(
-                    evicted_tx.id.mined_id(),
-                    SameEffectsChainRejectionError::RandomlyEvicted,
-                );
+                let _ = self
+                    .chain_rejected_same_effects
+                    .entry(SameEffectsChainRejectionError::RandomlyEvicted)
+                    .or_default()
+                    .insert(evicted_tx.id.mined_id());
             }
 
             assert_eq!(self.verified.len(), MEMPOOL_SIZE);
@@ -242,8 +244,10 @@ impl Storage {
         if self.tip_rejected_same_effects.len() > MAX_EVICTION_MEMORY_ENTRIES {
             self.tip_rejected_same_effects.clear();
         }
-        if self.chain_rejected_same_effects.len() > MAX_EVICTION_MEMORY_ENTRIES {
-            self.chain_rejected_same_effects.clear();
+        for (_, map) in self.chain_rejected_same_effects.iter_mut() {
+            if map.len() > MAX_EVICTION_MEMORY_ENTRIES {
+                map.clear();
+            }
         }
     }
 
@@ -288,7 +292,11 @@ impl Storage {
     pub fn rejected_transaction_count(&self) -> usize {
         self.tip_rejected_exact.len()
             + self.tip_rejected_same_effects.len()
-            + self.chain_rejected_same_effects.len()
+            + self
+                .chain_rejected_same_effects
+                .iter()
+                .map(|(_, map)| map.len())
+                .sum::<usize>()
     }
 
     /// Add a transaction to the rejected list for the given reason.
@@ -301,7 +309,10 @@ impl Storage {
                 self.tip_rejected_same_effects.insert(txid.mined_id(), e);
             }
             RejectionError::SameEffectsChain(e) => {
-                self.chain_rejected_same_effects.insert(txid.mined_id(), e);
+                self.chain_rejected_same_effects
+                    .entry(e)
+                    .or_default()
+                    .insert(txid.mined_id());
             }
         }
         self.limit_rejection_list_memory();
@@ -320,8 +331,10 @@ impl Storage {
             return Some(error.clone().into());
         }
 
-        if let Some(error) = self.chain_rejected_same_effects.get(&txid.mined_id()) {
-            return Some(error.clone().into());
+        for (error, set) in self.chain_rejected_same_effects.iter() {
+            if set.contains(&txid.mined_id()) {
+                return Some(error.clone().into());
+            }
         }
 
         None
@@ -350,7 +363,8 @@ impl Storage {
                 .contains_key(&txid.mined_id())
             || self
                 .chain_rejected_same_effects
-                .contains_key(&txid.mined_id())
+                .iter()
+                .any(|(_, map)| map.contains(&txid.mined_id()))
     }
 
     /// Checks if the `tx` transaction has spend conflicts with another transaction in the mempool.