Remove unused mempool errors (#2831)

* Remove unused mempool storage errors

Preparation for ticket #2819.

Removing these errors means that we don't have to decide
which type of transaction ID match we want for them.

* Remove unused mempool errors, and deduplicate storage errors

* rustfmt

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<SubsidyError> for BlockError {
     }
 }
 
-#[derive(Error, Clone, Debug, PartialEq)]
+#[derive(Error, Clone, Debug, PartialEq, Eq)]
 pub enum BlockError {
     #[error("block contains invalid transactions")]
     Transaction(#[from] TransactionError),

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 {

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(())
     }

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,
 }

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<UnminedTx>,
     /// The set of rejected transactions by id, and their rejection reasons.
-    rejected: HashMap<UnminedTxId, State>,
+    rejected: HashMap<UnminedTxId, StorageRejectionError>,
 }
 
 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<UnminedTxId, MempoolError> {
         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<StorageRejectionError> {
+        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<UnminedTxId>) -> Vec<UnminedTxId> {
         tx_ids

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));

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(())
 }