diff --git a/zebra-consensus/src/error.rs b/zebra-consensus/src/error.rs index 333eff10..4fdbaba9 100644 --- a/zebra-consensus/src/error.rs +++ b/zebra-consensus/src/error.rs @@ -12,7 +12,7 @@ use crate::BoxError; #[cfg(any(test, feature = "proptest-impl"))] use proptest_derive::Arbitrary; -#[derive(Error, Copy, Clone, Debug, PartialEq)] +#[derive(Error, Copy, Clone, Debug, PartialEq, Eq)] pub enum SubsidyError { #[error("no coinbase transaction in block")] NoCoinbase, @@ -21,7 +21,7 @@ pub enum SubsidyError { FoundersRewardNotFound, } -#[derive(Error, Clone, Debug, PartialEq)] +#[derive(Error, Clone, Debug, PartialEq, Eq)] #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] pub enum TransactionError { #[error("first transaction must be coinbase")] @@ -128,7 +128,7 @@ impl From for BlockError { } } -#[derive(Error, Clone, Debug, PartialEq)] +#[derive(Error, Clone, Debug, PartialEq, Eq)] pub enum BlockError { #[error("block contains invalid transactions")] Transaction(#[from] TransactionError), diff --git a/zebra-script/src/lib.rs b/zebra-script/src/lib.rs index 15423360..033b78ec 100644 --- a/zebra-script/src/lib.rs +++ b/zebra-script/src/lib.rs @@ -24,7 +24,7 @@ use zebra_chain::{ transparent, }; -#[derive(Copy, Clone, Debug, Display, Error, PartialEq)] +#[derive(Copy, Clone, Debug, Display, Error, PartialEq, Eq)] #[non_exhaustive] /// An Error type representing the error codes returned from zcash_script. pub enum Error { diff --git a/zebrad/src/components/mempool.rs b/zebrad/src/components/mempool.rs index a85dd56a..27c262d2 100644 --- a/zebrad/src/components/mempool.rs +++ b/zebrad/src/components/mempool.rs @@ -33,6 +33,8 @@ mod tests; pub use self::crawler::Crawler; pub use self::error::MempoolError; +pub use self::storage::StorageRejectionError; + #[cfg(test)] pub use self::storage::tests::unmined_transactions_in_blocks; @@ -182,8 +184,8 @@ impl Mempool { if storage.contains(&txid) { return Err(MempoolError::InMempool); } - if storage.contains_rejected(&txid) { - return Err(MempoolError::Rejected); + if let Some(error) = storage.rejection_error(&txid) { + return Err(error.into()); } Ok(()) } diff --git a/zebrad/src/components/mempool/error.rs b/zebrad/src/components/mempool/error.rs index 3c686898..1690bf1f 100644 --- a/zebrad/src/components/mempool/error.rs +++ b/zebrad/src/components/mempool/error.rs @@ -5,34 +5,24 @@ use thiserror::Error; #[cfg(any(test, feature = "proptest-impl"))] use proptest_derive::Arbitrary; -#[derive(Error, Clone, Debug, PartialEq)] +use super::storage::StorageRejectionError; + +#[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 transaction")] + Storage(#[from] StorageRejectionError), + #[error("transaction already exists in mempool")] InMempool, - #[error("transaction did not pass consensus validation")] - Invalid(#[from] zebra_consensus::error::TransactionError), - #[error("transaction is committed in block {0:?}")] InBlock(zebra_chain::block::Hash), - #[error("transaction has expired")] - Expired, - - #[error("transaction fee is too low for the current mempool state")] - LowFee, - #[error("transaction was not found in mempool")] NotInMempool, - #[error("transaction evicted from the mempool due to size restrictions")] - Excess, - - #[error("transaction is in the mempool rejected list")] - Rejected, - /// The transaction hash is already queued, so this request was ignored. /// /// Another peer has already gossiped the same hash to us, or the mempool crawler has fetched it. @@ -48,13 +38,6 @@ pub enum MempoolError { #[error("transaction dropped because the queue is full")] FullQueue, - /// The transaction has a spend conflict with another transaction already in the mempool. - #[error( - "transaction rejected because another transaction in the mempool has already spent some of \ - its inputs" - )] - SpendConflict, - #[error("mempool is disabled since synchronization is behind the chain tip")] Disabled, } diff --git a/zebrad/src/components/mempool/storage.rs b/zebrad/src/components/mempool/storage.rs index b8857cb1..45a5fd85 100644 --- a/zebrad/src/components/mempool/storage.rs +++ b/zebrad/src/components/mempool/storage.rs @@ -3,38 +3,44 @@ use std::{ hash::Hash, }; -use zebra_chain::{ - block, - transaction::{self, Transaction, UnminedTx, UnminedTxId}, -}; -use zebra_consensus::error::TransactionError; +use thiserror::Error; + +use zebra_chain::transaction::{self, Transaction, UnminedTx, UnminedTxId}; use super::MempoolError; +#[cfg(any(test, feature = "proptest-impl"))] +use proptest_derive::Arbitrary; + #[cfg(test)] pub mod tests; const MEMPOOL_SIZE: usize = 2; +#[derive(Error, Clone, Debug, PartialEq, Eq)] +#[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] #[allow(dead_code)] -#[derive(Clone, Debug)] -pub enum State { - /// Rejected because verification failed. - Invalid(TransactionError), - /// An otherwise valid mempool transaction was mined into a block, therefore - /// no longer belongs in the mempool. - Confirmed(block::Hash), - /// Rejected because it has a spend conflict with another transaction already in the mempool. +pub enum StorageRejectionError { + #[error( + "transaction rejected because another transaction in the mempool has already spent some of \ + its inputs" + )] SpendConflict, - /// Stayed in mempool for too long without being mined. - // TODO(2021-09-09): Implement ZIP-203: Validate Transaction Expiry Height. - // TODO(2021-09-09): https://github.com/ZcashFoundation/zebra/issues/2387 + + #[error("best chain tip has reached transaction expiry height")] Expired, - /// Transaction fee is too low for the current mempool state. - LowFee, - /// Otherwise valid transaction removed from mempool, say because of FIFO - /// (first in, first out) policy. - Excess, + + /// Otherwise valid transaction removed from mempool due to ZIP-401 random eviction. + /// + /// Consensus rule: + /// > The txid (rather than the wtxid ...) is used even for version 5 transactions + /// + /// https://zips.z.cash/zip-0401#specification + #[error("transaction evicted from the mempool due to ZIP-401 denial of service limits")] + RandomlyEvicted, + + #[error("transaction did not pass consensus validation")] + FailedVerification(#[from] zebra_consensus::error::TransactionError), } #[derive(Default)] @@ -43,27 +49,20 @@ pub struct Storage { /// cache of size [`MEMPOOL_SIZE`]. verified: VecDeque, /// The set of rejected transactions by id, and their rejection reasons. - rejected: HashMap, + rejected: HashMap, } impl Storage { /// Insert a [`UnminedTx`] into the mempool. /// /// If its insertion results in evicting other transactions, they will be tracked - /// as [`State::Excess`]. + /// as [`StorageRejectionError::RandomlyEvicted`]. pub fn insert(&mut self, tx: UnminedTx) -> Result { let tx_id = tx.id; - // First, check if we should reject this transaction. - if self.rejected.contains_key(&tx.id) { - return Err(match self.rejected.get(&tx.id).unwrap() { - State::Invalid(e) => MempoolError::Invalid(e.clone()), - State::Expired => MempoolError::Expired, - State::Confirmed(block_hash) => MempoolError::InBlock(*block_hash), - State::Excess => MempoolError::Excess, - State::LowFee => MempoolError::LowFee, - State::SpendConflict => MempoolError::SpendConflict, - }); + // First, check if we have a cached rejection for this transaction. + if let Some(error) = self.rejection_error(&tx.id) { + return Err(error.into()); } // If `tx` is already in the mempool, we don't change anything. @@ -78,8 +77,9 @@ impl Storage { // nullifier already revealed by another transaction in the mempool, reject that // transaction. if self.check_spend_conflicts(&tx) { - self.rejected.insert(tx.id, State::SpendConflict); - return Err(MempoolError::Rejected); + self.rejected + .insert(tx.id, StorageRejectionError::SpendConflict); + return Err(StorageRejectionError::SpendConflict.into()); } // Then, we insert into the pool. @@ -87,9 +87,13 @@ impl Storage { // 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.insert(evicted_tx.id, State::Excess); + let _ = self + .rejected + .insert(evicted_tx.id, StorageRejectionError::RandomlyEvicted); } assert_eq!(self.verified.len(), MEMPOOL_SIZE); @@ -170,10 +174,17 @@ impl Storage { /// Returns `true` if a [`UnminedTx`] matching an [`UnminedTxId`] is in /// the mempool rejected list. + #[allow(dead_code)] pub fn contains_rejected(&self, txid: &UnminedTxId) -> bool { self.rejected.contains_key(txid) } + /// Returns `true` if a [`UnminedTx`] matching an [`UnminedTxId`] is in + /// the mempool rejected list. + pub fn rejection_error(&self, txid: &UnminedTxId) -> Option { + self.rejected.get(txid).cloned() + } + /// Returns the set of [`UnminedTxId`]s matching ids in the rejected list. pub fn rejected_transactions(&self, tx_ids: HashSet) -> Vec { tx_ids diff --git a/zebrad/src/components/mempool/storage/tests/prop.rs b/zebrad/src/components/mempool/storage/tests/prop.rs index bbb99f87..135f640e 100644 --- a/zebrad/src/components/mempool/storage/tests/prop.rs +++ b/zebrad/src/components/mempool/storage/tests/prop.rs @@ -11,6 +11,8 @@ use zebra_chain::{ transparent, LedgerState, }; +use crate::components::mempool::storage::StorageRejectionError; + use super::super::{MempoolError, Storage}; proptest! { @@ -40,7 +42,7 @@ proptest! { assert_eq!( storage.insert(transaction_to_reject), - Err(MempoolError::Rejected) + Err(MempoolError::Storage(StorageRejectionError::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 dba5c1e6..5d972148 100644 --- a/zebrad/src/components/mempool/tests/vector.rs +++ b/zebrad/src/components/mempool/tests/vector.rs @@ -229,7 +229,12 @@ async fn mempool_queue() -> Result<(), Report> { _ => unreachable!("will never happen in this test"), }; assert_eq!(queued_responses.len(), 1); - assert_eq!(queued_responses[0], Err(MempoolError::Rejected)); + assert_eq!( + queued_responses[0], + Err(MempoolError::Storage( + StorageRejectionError::RandomlyEvicted + )) + ); Ok(()) }