diff --git a/zebrad/src/components/mempool.rs b/zebrad/src/components/mempool.rs index 86f8d44d..5203ac95 100644 --- a/zebrad/src/components/mempool.rs +++ b/zebrad/src/components/mempool.rs @@ -33,7 +33,9 @@ mod tests; pub use self::crawler::Crawler; pub use self::error::MempoolError; -pub use self::storage::{ExactRejectionError, SameEffectsRejectionError}; +pub use self::storage::{ + ExactTipRejectionError, SameEffectsChainRejectionError, SameEffectsTipRejectionError, +}; #[cfg(test)] pub use self::storage::tests::unmined_transactions_in_blocks; @@ -218,6 +220,7 @@ impl Service for Mempool { let mined_ids = block.transaction_hashes.iter().cloned().collect(); tx_downloads.cancel(&mined_ids); storage.remove_same_effects(&mined_ids); + storage.clear_tip_rejections(); } } } diff --git a/zebrad/src/components/mempool/error.rs b/zebrad/src/components/mempool/error.rs index 763156f3..1b1ad6cd 100644 --- a/zebrad/src/components/mempool/error.rs +++ b/zebrad/src/components/mempool/error.rs @@ -5,17 +5,26 @@ use thiserror::Error; #[cfg(any(test, feature = "proptest-impl"))] use proptest_derive::Arbitrary; -use super::storage::{ExactRejectionError, SameEffectsRejectionError}; +use super::storage::{ + ExactTipRejectionError, SameEffectsChainRejectionError, SameEffectsTipRejectionError, +}; #[derive(Error, Clone, Debug, PartialEq, Eq)] #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] #[allow(dead_code)] pub enum MempoolError { - #[error("mempool storage has a cached rejection for this exact transaction")] - StorageExact(#[from] ExactRejectionError), + #[error("mempool storage has a cached tip rejection for this exact transaction")] + StorageExactTip(#[from] ExactTipRejectionError), - #[error("mempool storage has a cached rejection for any transaction with the same effects")] - StorageEffects(#[from] SameEffectsRejectionError), + #[error( + "mempool storage has a cached tip rejection for any transaction with the same effects" + )] + StorageEffectsTip(#[from] SameEffectsTipRejectionError), + + #[error( + "mempool storage has a cached chain rejection for any transaction with the same effects" + )] + StorageEffectsChain(#[from] SameEffectsChainRejectionError), #[error("transaction already exists in mempool")] InMempool, diff --git a/zebrad/src/components/mempool/storage.rs b/zebrad/src/components/mempool/storage.rs index 65c7273c..798f997a 100644 --- a/zebrad/src/components/mempool/storage.rs +++ b/zebrad/src/components/mempool/storage.rs @@ -17,27 +17,52 @@ pub mod tests; const MEMPOOL_SIZE: usize = 2; +/// The size limit for mempool transaction rejection lists. +/// +/// > The size of RecentlyEvicted SHOULD never exceed `eviction_memory_entries` entries, +/// > which is the constant 40000. +/// +/// https://zips.z.cash/zip-0401#specification +/// +/// We use the specified value for all lists for consistency. +const MAX_EVICTION_MEMORY_ENTRIES: usize = 40_000; + /// Transactions rejected based on transaction authorizing data (scripts, proofs, signatures), -/// or for other reasons. +/// These rejections are only valid for the current tip. +/// +/// Each committed block clears these rejections, because new blocks can supply missing inputs. #[derive(Error, Clone, Debug, PartialEq, Eq)] #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] #[allow(dead_code)] -pub enum ExactRejectionError { +pub enum ExactTipRejectionError { #[error("transaction did not pass consensus validation")] FailedVerification(#[from] zebra_consensus::error::TransactionError), } /// Transactions rejected based only on their effects (spends, outputs, transaction header). +/// These rejections are only valid for the current tip. +/// +/// Each committed block clears these rejections, because new blocks can evict other transactions. #[derive(Error, Clone, Debug, PartialEq, Eq)] #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] #[allow(dead_code)] -pub enum SameEffectsRejectionError { +pub enum SameEffectsTipRejectionError { #[error( "transaction rejected because another transaction in the mempool has already spent some of \ its inputs" )] SpendConflict, +} +/// Transactions rejected based only on their effects (spends, outputs, transaction header). +/// These rejections are valid while the current chain continues to grow. +/// +/// 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)] +#[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] +#[allow(dead_code)] +pub enum SameEffectsChainRejectionError { #[error("best chain tip has reached transaction expiry height")] Expired, @@ -58,15 +83,22 @@ pub struct Storage { verified: VecDeque, /// The set of transactions rejected due to bad authorizations, or for other reasons, - /// and their rejection reasons. + /// and their rejection reasons. These rejections only apply to the current tip. /// /// Only transactions with the exact `UnminedTxId` are invalid. - rejected_exact: HashMap, + tip_rejected_exact: HashMap, - /// The set of transactions rejected for their effects, and their rejection reasons. + /// A set of transactions rejected for their effects, and their rejection reasons. + /// These rejections only apply to the current tip. /// /// Any transaction with the same `transaction::Hash` is invalid. - rejected_same_effects: HashMap, + tip_rejected_same_effects: HashMap, + + /// The set of transactions rejected for their effects, and their rejection reasons. + /// These rejections apply until a rollback or network upgrade. + /// + /// Any transaction with the same `transaction::Hash` is invalid. + chain_rejected_same_effects: HashMap, } impl Storage { @@ -94,29 +126,34 @@ impl Storage { // nullifier already revealed by another transaction in the mempool, reject that // transaction. if self.check_spend_conflicts(&tx) { - self.rejected_same_effects - .insert(tx.id.mined_id(), SameEffectsRejectionError::SpendConflict); - return Err(SameEffectsRejectionError::SpendConflict.into()); + let error = SameEffectsTipRejectionError::SpendConflict; + self.tip_rejected_same_effects + .insert(tx.id.mined_id(), error.clone()); + return Err(error.into()); } // Then, we insert into the pool. self.verified.push_front(tx); + // Security: stop the transaction or rejection lists using too much memory + // Once inserted, we evict transactions over the pool size limit in FIFO // order. // // 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.rejected_same_effects.insert( + let _ = self.chain_rejected_same_effects.insert( evicted_tx.id.mined_id(), - SameEffectsRejectionError::RandomlyEvicted, + SameEffectsChainRejectionError::RandomlyEvicted, ); } assert_eq!(self.verified.len(), MEMPOOL_SIZE); } + self.limit_rejection_list_memory(); + Ok(tx_id) } @@ -168,8 +205,34 @@ impl Storage { /// Clears the whole mempool storage. pub fn clear(&mut self) { self.verified.clear(); - self.rejected_exact.clear(); - self.rejected_same_effects.clear(); + self.tip_rejected_exact.clear(); + self.tip_rejected_same_effects.clear(); + self.chain_rejected_same_effects.clear(); + } + + /// Clears rejections that only apply to the current tip. + pub fn clear_tip_rejections(&mut self) { + self.tip_rejected_exact.clear(); + self.tip_rejected_same_effects.clear(); + } + + /// Clears rejections that only apply to the current tip. + /// + /// # Security + /// + /// This method must be called at the end of every method that adds rejections. + /// Otherwise, peers could make our reject lists use a lot of RAM. + fn limit_rejection_list_memory(&mut self) { + // These lists are an optimisation - it's ok to totally clear them as needed. + if self.tip_rejected_exact.len() > MAX_EVICTION_MEMORY_ENTRIES { + self.tip_rejected_exact.clear(); + } + 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(); + } } /// Returns the set of [`UnminedTxId`]s in the mempool. @@ -211,7 +274,9 @@ impl Storage { /// 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() + self.tip_rejected_exact.len() + + self.tip_rejected_same_effects.len() + + self.chain_rejected_same_effects.len() } /// Returns `true` if a [`UnminedTx`] matching an [`UnminedTxId`] is in @@ -219,12 +284,16 @@ impl Storage { /// /// This matches transactions based on each rejection list's matching rule. pub fn rejection_error(&self, txid: &UnminedTxId) -> Option { - if let Some(exact_error) = self.rejected_exact.get(txid) { - return Some(exact_error.clone().into()); + if let Some(error) = self.tip_rejected_exact.get(txid) { + return Some(error.clone().into()); } - if let Some(effects_error) = self.rejected_same_effects.get(&txid.mined_id()) { - return Some(effects_error.clone().into()); + if let Some(error) = self.tip_rejected_same_effects.get(&txid.mined_id()) { + return Some(error.clone().into()); + } + + if let Some(error) = self.chain_rejected_same_effects.get(&txid.mined_id()) { + return Some(error.clone().into()); } None @@ -247,8 +316,13 @@ impl Storage { /// /// This matches transactions based on each rejection list's matching rule. pub fn contains_rejected(&self, txid: &UnminedTxId) -> bool { - self.rejected_exact.contains_key(txid) - || self.rejected_same_effects.contains_key(&txid.mined_id()) + self.tip_rejected_exact.contains_key(txid) + || self + .tip_rejected_same_effects + .contains_key(&txid.mined_id()) + || self + .chain_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..a0aec569 100644 --- a/zebrad/src/components/mempool/storage/tests/prop.rs +++ b/zebrad/src/components/mempool/storage/tests/prop.rs @@ -11,7 +11,7 @@ use zebra_chain::{ transparent, LedgerState, }; -use crate::components::mempool::storage::SameEffectsRejectionError; +use crate::components::mempool::storage::SameEffectsTipRejectionError; use super::super::{MempoolError, Storage}; @@ -42,7 +42,7 @@ proptest! { assert_eq!( storage.insert(transaction_to_reject), - Err(MempoolError::StorageEffects(SameEffectsRejectionError::SpendConflict)) + Err(MempoolError::StorageEffectsTip(SameEffectsTipRejectionError::SpendConflict)) ); assert!(storage.contains_rejected(&id_to_reject)); diff --git a/zebrad/src/components/mempool/tests/vector.rs b/zebrad/src/components/mempool/tests/vector.rs index 06711c6f..e7c7fce2 100644 --- a/zebrad/src/components/mempool/tests/vector.rs +++ b/zebrad/src/components/mempool/tests/vector.rs @@ -231,8 +231,8 @@ async fn mempool_queue() -> Result<(), Report> { assert_eq!(queued_responses.len(), 1); assert_eq!( queued_responses[0], - Err(MempoolError::StorageEffects( - SameEffectsRejectionError::RandomlyEvicted + Err(MempoolError::StorageEffectsChain( + SameEffectsChainRejectionError::RandomlyEvicted )) );