From e470ed00e6284db104d8a9bd008b5e5e17c82832 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 7 Oct 2021 08:45:15 +1000 Subject: [PATCH] Return rejected transaction ID iterators from mempool storage Also rename methods for consistency. --- zebrad/src/components/mempool.rs | 7 ++- zebrad/src/components/mempool/storage.rs | 54 +++++++++++-------- .../components/mempool/storage/tests/prop.rs | 2 +- .../mempool/storage/tests/vectors.rs | 16 +++--- 4 files changed, 44 insertions(+), 35 deletions(-) diff --git a/zebrad/src/components/mempool.rs b/zebrad/src/components/mempool.rs index e777dcbf..e7d6fb3c 100644 --- a/zebrad/src/components/mempool.rs +++ b/zebrad/src/components/mempool.rs @@ -184,7 +184,7 @@ impl Mempool { if storage.contains_transaction_exact(&txid) { return Err(MempoolError::InMempool); } - if let Some(error) = storage.rejection_error(&txid) { + if let Some(error) = storage.rejection_error_exact(&txid) { return Err(error); } Ok(()) @@ -266,9 +266,8 @@ impl Service for Mempool { async move { Ok(Response::Transactions(res)) }.boxed() } Request::RejectedTransactionIds(ids) => { - let rsp = Ok(storage.rejected_transactions(ids)) - .map(Response::RejectedTransactionIds); - async move { rsp }.boxed() + let res = storage.rejected_transactions_exact(ids).collect(); + async move { Ok(Response::RejectedTransactionIds(res)) }.boxed() } Request::Queue(gossiped_txs) => { let rsp: Vec> = gossiped_txs diff --git a/zebrad/src/components/mempool/storage.rs b/zebrad/src/components/mempool/storage.rs index 1a7540b8..1a5aac6e 100644 --- a/zebrad/src/components/mempool/storage.rs +++ b/zebrad/src/components/mempool/storage.rs @@ -78,7 +78,7 @@ impl Storage { let tx_id = tx.id; // First, check if we have a cached rejection for this transaction. - if let Some(error) = self.rejection_error(&tx.id) { + if let Some(error) = self.rejection_error_exact(&tx.id) { return Err(error); } @@ -165,6 +165,13 @@ impl Storage { original_size - self.verified.len() } + /// Clears the whole mempool storage. + pub fn clear(&mut self) { + self.verified.clear(); + self.rejected_exact.clear(); + self.rejected_same_effects.clear(); + } + /// Returns the set of [`UnminedTxId`]s in the mempool. pub fn tx_ids(&self) -> impl Iterator + '_ { self.verified.iter().map(|tx| tx.id) @@ -201,16 +208,17 @@ impl Storage { self.verified.iter().any(|tx| &tx.id == txid) } - /// Returns `true` if a [`UnminedTx`] matching the supplied [`UnminedTxId`] is in - /// the mempool rejected list. - pub fn contains_rejected(&self, txid: &UnminedTxId) -> bool { - self.rejected_exact.contains_key(txid) - || self.rejected_same_effects.contains_key(&txid.mined_id()) + /// Returns the number of rejected [`UnminedTxId`]s or [`transaction::Hash`]es. + #[allow(dead_code)] + pub fn rejected_transaction_count(&self) -> usize { + self.rejected_exact.len() + self.rejected_same_effects.len() } - /// Returns `true` if a [`UnminedTx`] matching an [`UnminedTxId`] is in + /// Returns `true` if a [`UnminedTx`] exactly matching an [`UnminedTxId`] is in /// the mempool rejected list. - pub fn rejection_error(&self, txid: &UnminedTxId) -> Option { + /// + /// This matches the exact transaction, with identical blockchain effects, signatures, and proofs. + pub fn rejection_error_exact(&self, txid: &UnminedTxId) -> Option { if let Some(exact_error) = self.rejected_exact.get(txid) { return Some(exact_error.clone().into()); } @@ -222,25 +230,25 @@ impl Storage { None } - /// Returns the set of [`UnminedTxId`]s matching ids in the rejected list. - pub fn rejected_transactions(&self, tx_ids: HashSet) -> Vec { + /// Returns the set of [`UnminedTxId`]s exactly matching ids in the rejected list. + /// + /// This matches the exact transaction, with identical blockchain effects, signatures, and proofs. + pub fn rejected_transactions_exact( + &self, + tx_ids: HashSet, + ) -> impl Iterator + '_ { tx_ids .into_iter() - .filter(|txid| self.contains_rejected(txid)) - .collect() + .filter(move |txid| self.contains_rejected_exact(txid)) } - /// Returns the number of rejected [`UnminedTxId`]s or [`transaction::Hash`]es. - #[allow(dead_code)] - pub fn rejected_transaction_count(&self) -> usize { - self.rejected_exact.len() + self.rejected_same_effects.len() - } - - /// Clears the whole mempool storage. - pub fn clear(&mut self) { - self.verified.clear(); - self.rejected_exact.clear(); - self.rejected_same_effects.clear(); + /// Returns `true` if a [`UnminedTx`] exactly matching the supplied [`UnminedTxId`] is in + /// the mempool rejected list. + /// + /// This matches the exact transaction, with identical blockchain effects, signatures, and proofs. + pub fn contains_rejected_exact(&self, txid: &UnminedTxId) -> bool { + self.rejected_exact.contains_key(txid) + || self.rejected_same_effects.contains_key(&txid.mined_id()) } /// Checks if the `tx` transaction has spend conflicts with another transaction in the mempool. diff --git a/zebrad/src/components/mempool/storage/tests/prop.rs b/zebrad/src/components/mempool/storage/tests/prop.rs index bcaf3f17..afeaf508 100644 --- a/zebrad/src/components/mempool/storage/tests/prop.rs +++ b/zebrad/src/components/mempool/storage/tests/prop.rs @@ -45,7 +45,7 @@ proptest! { Err(MempoolError::StorageEffects(SameEffectsRejectionError::SpendConflict)) ); - assert!(storage.contains_rejected(&id_to_reject)); + assert!(storage.contains_rejected_exact(&id_to_reject)); storage.clear(); } diff --git a/zebrad/src/components/mempool/storage/tests/vectors.rs b/zebrad/src/components/mempool/storage/tests/vectors.rs index 31b2ba23..ea33b1ff 100644 --- a/zebrad/src/components/mempool/storage/tests/vectors.rs +++ b/zebrad/src/components/mempool/storage/tests/vectors.rs @@ -114,18 +114,20 @@ fn mempool_storage_basic_for_network(network: Network) -> Result<()> { // Query all the ids we have for rejected, get back `total - MEMPOOL_SIZE` let all_ids: HashSet = unmined_transactions.iter().map(|tx| tx.id).collect(); - // Convert response to a `HashSet` as we need a fixed order to compare. - let rejected_response: HashSet = - storage.rejected_transactions(all_ids).into_iter().collect(); + // Convert response to a `HashSet`, because the order of the response doesn't matter. + let rejected_response: HashSet = storage + .rejected_transactions_exact(all_ids) + .into_iter() + .collect(); let rejected_ids = expected_to_be_rejected.iter().map(|tx| tx.id).collect(); assert_eq!(rejected_response, rejected_ids); - // Use `contains_rejected` to make sure the first id stored is now rejected - assert!(storage.contains_rejected(&expected_to_be_rejected[0].id)); - // Use `contains_rejected` to make sure the last id stored is not rejected - assert!(!storage.contains_rejected(&expected_in_mempool[0].id)); + // Make sure the first id stored is now rejected + assert!(storage.contains_rejected_exact(&expected_to_be_rejected[0].id)); + // Make sure the last id stored is not rejected + assert!(!storage.contains_rejected_exact(&expected_in_mempool[0].id)); Ok(()) }