diff --git a/zebra-chain/src/block.rs b/zebra-chain/src/block.rs index ecea4a1c..759030ef 100644 --- a/zebra-chain/src/block.rs +++ b/zebra-chain/src/block.rs @@ -55,11 +55,14 @@ pub struct Block { impl fmt::Display for Block { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut fmter = f.debug_struct("Block"); + if let Some(height) = self.coinbase_height() { fmter.field("height", &height); } + fmter.field("transactions", &self.transactions.len()); + fmter.field("hash", &DisplayToDebug(self.hash())); - fmter.field("hash", &DisplayToDebug(self.hash())).finish() + fmter.finish() } } diff --git a/zebra-chain/src/orchard/shielded_data.rs b/zebra-chain/src/orchard/shielded_data.rs index 02636259..366dec12 100644 --- a/zebra-chain/src/orchard/shielded_data.rs +++ b/zebra-chain/src/orchard/shielded_data.rs @@ -2,7 +2,7 @@ use std::{ cmp::{Eq, PartialEq}, - fmt::Debug, + fmt::{self, Debug}, io, }; @@ -39,6 +39,22 @@ pub struct ShieldedData { pub binding_sig: Signature, } +impl fmt::Display for ShieldedData { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut fmter = f.debug_struct("orchard::ShieldedData"); + + fmter.field("actions", &self.actions.len()); + fmter.field("value_balance", &self.value_balance); + fmter.field("flags", &self.flags); + + fmter.field("proof_len", &self.proof.zcash_serialized_size()); + + fmter.field("shared_anchor", &self.shared_anchor); + + fmter.finish() + } +} + impl ShieldedData { /// Iterate over the [`Action`]s for the [`AuthorizedAction`]s in this /// transaction, in the order they appear in it. diff --git a/zebra-chain/src/sapling/shielded_data.rs b/zebra-chain/src/sapling/shielded_data.rs index d923c3af..8879c873 100644 --- a/zebra-chain/src/sapling/shielded_data.rs +++ b/zebra-chain/src/sapling/shielded_data.rs @@ -23,7 +23,7 @@ use crate::{ use std::{ cmp::{max, Eq, PartialEq}, - fmt::Debug, + fmt::{self, Debug}, }; /// Per-Spend Sapling anchors, used in Transaction V4 and the @@ -179,6 +179,27 @@ where }, } +impl fmt::Display for ShieldedData +where + AnchorV: AnchorVariant + Clone, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut fmter = f.debug_struct( + format!( + "sapling::ShieldedData<{}>", + std::any::type_name::() + ) + .as_str(), + ); + + fmter.field("spends", &self.transfers.spends().count()); + fmter.field("outputs", &self.transfers.outputs().count()); + fmter.field("value_balance", &self.value_balance); + + fmter.finish() + } +} + impl ShieldedData where AnchorV: AnchorVariant + Clone, diff --git a/zebra-chain/src/sapling/spend.rs b/zebra-chain/src/sapling/spend.rs index 732781eb..000515d9 100644 --- a/zebra-chain/src/sapling/spend.rs +++ b/zebra-chain/src/sapling/spend.rs @@ -3,7 +3,7 @@ //! Zebra uses a generic spend type for `V4` and `V5` transactions. //! The anchor change is handled using the `AnchorVariant` type trait. -use std::io; +use std::{fmt, io}; use crate::{ block::MAX_BLOCK_BYTES, @@ -73,6 +73,21 @@ pub struct SpendPrefixInTransactionV5 { pub rk: redjubjub::VerificationKeyBytes, } +impl fmt::Display for Spend +where + AnchorV: AnchorVariant + Clone, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut fmter = f + .debug_struct(format!("sapling::Spend<{}>", std::any::type_name::()).as_str()); + + fmter.field("per_spend_anchor", &self.per_spend_anchor); + fmter.field("nullifier", &self.nullifier); + + fmter.finish() + } +} + impl From<(Spend, tree::Root)> for Spend { /// Convert a `Spend` and its shared anchor, into a /// `Spend`. diff --git a/zebra-chain/src/transaction.rs b/zebra-chain/src/transaction.rs index ef5fbdb4..44e85c71 100644 --- a/zebra-chain/src/transaction.rs +++ b/zebra-chain/src/transaction.rs @@ -40,7 +40,7 @@ use crate::{ value_balance::{ValueBalance, ValueBalanceError}, }; -use std::{collections::HashMap, iter}; +use std::{collections::HashMap, fmt, iter}; /// A Zcash transaction. /// @@ -131,6 +131,28 @@ pub enum Transaction { }, } +impl fmt::Display for Transaction { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut fmter = f.debug_struct("Transaction"); + + fmter.field("version", &self.version()); + if let Some(network_upgrade) = self.network_upgrade() { + fmter.field("network_upgrade", &network_upgrade); + } + + fmter.field("transparent_inputs", &self.inputs().len()); + fmter.field("transparent_outputs", &self.outputs().len()); + fmter.field("sprout_joinsplits", &self.joinsplit_count()); + fmter.field("sapling_spends", &self.sapling_spends_per_anchor().count()); + fmter.field("sapling_outputs", &self.sapling_outputs().count()); + fmter.field("orchard_actions", &self.orchard_actions().count()); + + fmter.field("unmined_id", &self.unmined_id()); + + fmter.finish() + } +} + impl Transaction { // identifiers and hashes diff --git a/zebra-chain/src/transaction/joinsplit.rs b/zebra-chain/src/transaction/joinsplit.rs index fe7cc160..523c062a 100644 --- a/zebra-chain/src/transaction/joinsplit.rs +++ b/zebra-chain/src/transaction/joinsplit.rs @@ -1,3 +1,5 @@ +use std::fmt; + use serde::{Deserialize, Serialize}; use crate::{ @@ -46,6 +48,18 @@ pub struct JoinSplitData { pub sig: ed25519::Signature, } +impl fmt::Display for JoinSplitData

{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut fmter = + f.debug_struct(format!("JoinSplitData<{}>", std::any::type_name::

()).as_str()); + + fmter.field("joinsplits", &self.joinsplits().count()); + fmter.field("value_balance", &self.value_balance()); + + fmter.finish() + } +} + impl JoinSplitData

{ /// Iterate over the [`JoinSplit`]s in `self`, in the order they appear /// in the transaction. diff --git a/zebra-chain/src/transparent.rs b/zebra-chain/src/transparent.rs index b16a74c3..1a64f406 100644 --- a/zebra-chain/src/transparent.rs +++ b/zebra-chain/src/transparent.rs @@ -33,7 +33,7 @@ use crate::{ block, transaction, }; -use std::{collections::HashMap, iter}; +use std::{collections::HashMap, fmt, iter}; /// The maturity threshold for transparent coinbase outputs. /// @@ -123,6 +123,33 @@ pub enum Input { }, } +impl fmt::Display for Input { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Input::PrevOut { + outpoint, + unlock_script, + .. + } => { + let mut fmter = f.debug_struct("transparent::Input::PrevOut"); + + fmter.field("unlock_script_len", &unlock_script.as_raw_bytes().len()); + fmter.field("outpoint", outpoint); + + fmter.finish() + } + Input::Coinbase { height, data, .. } => { + let mut fmter = f.debug_struct("transparent::Input::Coinbase"); + + fmter.field("height", height); + fmter.field("data_len", &data.0.len()); + + fmter.finish() + } + } + } +} + impl Input { /// If this is a `PrevOut` input, returns this input's outpoint. /// Otherwise, returns `None`. diff --git a/zebrad/proptest-regressions/components/mempool/storage/tests/prop.txt b/zebrad/proptest-regressions/components/mempool/storage/tests/prop.txt new file mode 100644 index 00000000..0c305464 --- /dev/null +++ b/zebrad/proptest-regressions/components/mempool/storage/tests/prop.txt @@ -0,0 +1,7 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc b258d507c0b2aef6c2793bdb3fc29e6367e62fba9df378ea6797e9bc97fd2780 # shrinks to input = RemoveSameEffects { transactions: alloc::vec::Vec, len=2, mined_ids_to_remove: std::collections::hash::set::HashSet, len=2 } diff --git a/zebrad/src/components/mempool.rs b/zebrad/src/components/mempool.rs index a83b59f0..cc2c0614 100644 --- a/zebrad/src/components/mempool.rs +++ b/zebrad/src/components/mempool.rs @@ -81,6 +81,7 @@ pub enum Response { /// /// Indicates wether it is enabled or disabled and, if enabled, contains /// the necessary data to run it. +#[allow(clippy::large_enum_variant)] enum ActiveState { /// The Mempool is disabled. Disabled, diff --git a/zebrad/src/components/mempool/storage.rs b/zebrad/src/components/mempool/storage.rs index b6ef49d7..630e98cb 100644 --- a/zebrad/src/components/mempool/storage.rs +++ b/zebrad/src/components/mempool/storage.rs @@ -1,12 +1,10 @@ -use std::{ - collections::{HashMap, HashSet, VecDeque}, - hash::Hash, -}; +use std::collections::{HashMap, HashSet}; use thiserror::Error; -use zebra_chain::transaction::{self, Transaction, UnminedTx, UnminedTxId}; +use zebra_chain::transaction::{self, UnminedTx, UnminedTxId}; +use self::verified_set::VerifiedSet; use super::MempoolError; #[cfg(any(test, feature = "proptest-impl"))] @@ -15,7 +13,10 @@ use proptest_derive::Arbitrary; #[cfg(test)] pub mod tests; -const MEMPOOL_SIZE: usize = 2; +mod verified_set; + +/// The maximum number of verified transactions to store in the mempool. +const MEMPOOL_SIZE: usize = 4; /// The size limit for mempool transaction rejection lists. /// @@ -92,7 +93,7 @@ pub enum RejectionError { pub struct Storage { /// The set of verified transactions in the mempool. This is a /// cache of size [`MEMPOOL_SIZE`]. - verified: VecDeque, + verified: VerifiedSet, /// The set of transactions rejected due to bad authorizations, or for other reasons, /// and their rejection reasons. These rejections only apply to the current tip. @@ -130,40 +131,34 @@ impl Storage { // // Security: transactions must not get refreshed by new queries, // because that allows malicious peers to keep transactions live forever. - if self.verified.contains(&tx) { + if self.verified.contains(&tx.id) { return Err(MempoolError::InMempool); } - // If `tx` spends an UTXO already spent by another transaction in the mempool or reveals a - // nullifier already revealed by another transaction in the mempool, reject that - // transaction. - if self.check_spend_conflicts(&tx) { - let error = SameEffectsTipRejectionError::SpendConflict; + // Then, we try to insert into the pool. If this fails the transaction is rejected. + if let Err(rejection_error) = self.verified.insert(tx) { self.tip_rejected_same_effects - .insert(tx.id.mined_id(), error.clone()); - return Err(error.into()); + .insert(tx_id.mined_id(), rejection_error.clone()); + return Err(rejection_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.chain_rejected_same_effects.insert( - evicted_tx.id.mined_id(), - SameEffectsChainRejectionError::RandomlyEvicted, - ); - } + // Once inserted, we evict transactions over the pool size limit. + while self.verified.transaction_count() > MEMPOOL_SIZE { + let evicted_tx = self + .verified + .evict_one() + .expect("mempool is empty, but was expected to be full"); - assert_eq!(self.verified.len(), MEMPOOL_SIZE); + let _ = self.chain_rejected_same_effects.insert( + evicted_tx.id.mined_id(), + SameEffectsChainRejectionError::RandomlyEvicted, + ); } + assert!(self.verified.transaction_count() <= MEMPOOL_SIZE); + self.limit_rejection_list_memory(); Ok(tx_id) @@ -185,11 +180,8 @@ impl Storage { /// Does not add or remove from the 'rejected' tracking set. #[allow(dead_code)] pub fn remove_exact(&mut self, exact_wtxids: &HashSet) -> usize { - let original_size = self.verified.len(); - - self.verified.retain(|tx| !exact_wtxids.contains(&tx.id)); - - original_size - self.verified.len() + self.verified + .remove_all_that(|tx| exact_wtxids.contains(&tx.id)) } /// Remove [`UnminedTx`]es from the mempool via non-malleable [`transaction::Hash`]. @@ -206,12 +198,8 @@ impl Storage { /// /// Does not add or remove from the 'rejected' tracking set. pub fn remove_same_effects(&mut self, mined_ids: &HashSet) -> usize { - let original_size = self.verified.len(); - self.verified - .retain(|tx| !mined_ids.contains(&tx.id.mined_id())); - - original_size - self.verified.len() + .remove_all_that(|tx| mined_ids.contains(&tx.id.mined_id())) } /// Clears the whole mempool storage. @@ -249,21 +237,22 @@ impl Storage { /// Returns the set of [`UnminedTxId`]s in the mempool. pub fn tx_ids(&self) -> impl Iterator + '_ { - self.verified.iter().map(|tx| tx.id) + self.verified.transactions().map(|tx| tx.id) } - /// Returns the set of [`Transaction`]s in the mempool. + /// Returns the set of [`Transaction`][transaction::Transaction]s in the mempool. pub fn transactions(&self) -> impl Iterator { - self.verified.iter() + self.verified.transactions() } /// Returns the number of transactions in the mempool. #[allow(dead_code)] pub fn transaction_count(&self) -> usize { - self.verified.len() + self.verified.transaction_count() } - /// Returns the set of [`Transaction`]s with exactly matching `tx_ids` in the mempool. + /// Returns the set of [`Transaction`][transaction::Transaction]s with exactly matching + /// `tx_ids` in the mempool. /// /// This matches the exact transaction, with identical blockchain effects, signatures, and proofs. pub fn transactions_exact( @@ -271,7 +260,7 @@ impl Storage { tx_ids: HashSet, ) -> impl Iterator { self.verified - .iter() + .transactions() .filter(move |tx| tx_ids.contains(&tx.id)) } @@ -280,7 +269,7 @@ impl Storage { /// /// This matches the exact transaction, with identical blockchain effects, signatures, and proofs. pub fn contains_transaction_exact(&self, txid: &UnminedTxId) -> bool { - self.verified.iter().any(|tx| &tx.id == txid) + self.verified.transactions().any(|tx| &tx.id == txid) } /// Returns the number of rejected [`UnminedTxId`]s or [`transaction::Hash`]es. @@ -352,38 +341,4 @@ impl Storage { .chain_rejected_same_effects .contains_key(&txid.mined_id()) } - - /// Checks if the `tx` transaction has spend conflicts with another transaction in the mempool. - /// - /// Two transactions have a spend conflict if they spent the same UTXO or if they reveal the - /// same nullifier. - fn check_spend_conflicts(&self, tx: &UnminedTx) -> bool { - self.has_spend_conflicts(tx, Transaction::spent_outpoints) - || self.has_spend_conflicts(tx, Transaction::sprout_nullifiers) - || self.has_spend_conflicts(tx, Transaction::sapling_nullifiers) - || self.has_spend_conflicts(tx, Transaction::orchard_nullifiers) - } - - /// Checks if the `tx` transaction has any spend conflicts with the transactions in the mempool - /// for the provided output type obtained through the `extractor`. - fn has_spend_conflicts<'slf, 'tx, Extractor, Outputs>( - &'slf self, - tx: &'tx UnminedTx, - extractor: Extractor, - ) -> bool - where - 'slf: 'tx, - Extractor: Fn(&'tx Transaction) -> Outputs, - Outputs: IntoIterator, - Outputs::Item: Eq + Hash + 'tx, - { - // TODO: This algorithm should be improved to avoid a performance impact when the mempool - // size is increased (#2784). - let new_outputs: HashSet<_> = extractor(&tx.transaction).into_iter().collect(); - - self.verified - .iter() - .flat_map(|tx| extractor(&tx.transaction)) - .any(|output| new_outputs.contains(&output)) - } } diff --git a/zebrad/src/components/mempool/storage/tests/prop.rs b/zebrad/src/components/mempool/storage/tests/prop.rs index a0aec569..ff6e623f 100644 --- a/zebrad/src/components/mempool/storage/tests/prop.rs +++ b/zebrad/src/components/mempool/storage/tests/prop.rs @@ -1,19 +1,24 @@ -use std::fmt::Debug; +use std::{collections::HashSet, convert::TryFrom, fmt::Debug}; -use proptest::prelude::*; +use proptest::{collection::vec, prelude::*}; use proptest_derive::Arbitrary; use zebra_chain::{ - at_least_one, orchard, - primitives::Groth16Proof, + at_least_one, + fmt::{DisplayToDebug, SummaryDebug}, + orchard, + primitives::{Groth16Proof, ZkSnarkProof}, sapling, - transaction::{self, Transaction, UnminedTx}, + serialization::AtLeastOne, + sprout, + transaction::{self, JoinSplitData, Transaction, UnminedTx, UnminedTxId}, transparent, LedgerState, }; use crate::components::mempool::storage::SameEffectsTipRejectionError; -use super::super::{MempoolError, Storage}; +use self::MultipleTransactionRemovalTestInput::*; +use super::super::{MempoolError, Storage, MEMPOOL_SIZE}; proptest! { /// Test if a transaction that has a spend conflict with a transaction already in the mempool @@ -35,10 +40,7 @@ proptest! { let id_to_accept = transaction_to_accept.id; let id_to_reject = transaction_to_reject.id; - assert_eq!( - storage.insert(transaction_to_accept), - Ok(id_to_accept) - ); + assert_eq!(storage.insert(transaction_to_accept), Ok(id_to_accept)); assert_eq!( storage.insert(transaction_to_reject), @@ -50,32 +52,135 @@ proptest! { storage.clear(); } } + + /// Test if a rejected transaction is properly rolled back. + /// + /// When a transaction is rejected, it should not leave anything in the cache that could lead + /// to false detection of spend conflicts. + #[test] + fn rejected_transactions_are_properly_rolled_back(input in any::()) + { + let mut storage = Storage::default(); + + let (first_unconflicting_transaction, second_unconflicting_transaction) = + input.clone().unconflicting_transactions(); + let (first_transaction, second_transaction) = input.conflicting_transactions(); + + let input_permutations = vec![ + ( + first_transaction.clone(), + second_transaction.clone(), + second_unconflicting_transaction, + ), + ( + second_transaction, + first_transaction, + first_unconflicting_transaction, + ), + ]; + + for (first_transaction_to_accept, transaction_to_reject, second_transaction_to_accept) in + input_permutations + { + let first_id_to_accept = first_transaction_to_accept.id; + let second_id_to_accept = second_transaction_to_accept.id; + let id_to_reject = transaction_to_reject.id; + + assert_eq!( + storage.insert(first_transaction_to_accept), + Ok(first_id_to_accept) + ); + + assert_eq!( + storage.insert(transaction_to_reject), + Err(MempoolError::StorageEffectsTip(SameEffectsTipRejectionError::SpendConflict)) + ); + + assert!(storage.contains_rejected(&id_to_reject)); + + assert_eq!( + storage.insert(second_transaction_to_accept), + Ok(second_id_to_accept) + ); + + storage.clear(); + } + } + + /// Test if multiple transactions are properly removed. + /// + /// Attempting to remove multiple transactions must remove all of them and leave all of the + /// others. + #[test] + fn removal_of_multiple_transactions(input in any::()) { + let mut storage = Storage::default(); + + // Insert all input transactions, and keep track of the IDs of the one that were actually + // inserted. + let inserted_transactions: HashSet<_> = input.transactions().filter_map(|transaction| { + let id = transaction.id; + + storage.insert(transaction.clone()).ok().map(|_| id)}).collect(); + + // Check that the inserted transactions are still there. + for transaction_id in &inserted_transactions { + assert!(storage.contains_transaction_exact(transaction_id)); + } + + // Remove some transactions. + match &input { + RemoveExact { wtx_ids_to_remove, .. } => storage.remove_exact(wtx_ids_to_remove), + RemoveSameEffects { mined_ids_to_remove, .. } => storage.remove_same_effects(mined_ids_to_remove), + }; + + // Check that the removed transactions are not in the storage. + let removed_transactions = input.removed_transaction_ids(); + + for removed_transaction_id in &removed_transactions { + assert!(!storage.contains_transaction_exact(removed_transaction_id)); + } + + // Check that the remaining transactions are still in the storage. + let remaining_transactions = inserted_transactions.difference(&removed_transactions); + + for remaining_transaction_id in remaining_transactions { + assert!(storage.contains_transaction_exact(remaining_transaction_id)); + } + } } /// Test input consisting of two transactions and a conflict to be applied to them. /// /// When the conflict is applied, both transactions will have a shared spend (either a UTXO used as /// an input, or a nullifier revealed by both transactions). -#[derive(Arbitrary, Debug)] +#[derive(Arbitrary, Clone, Debug)] enum SpendConflictTestInput { /// Test V4 transactions to include Sprout nullifier conflicts. V4 { - #[proptest(strategy = "Transaction::v4_strategy(LedgerState::default())")] - first: Transaction, + #[proptest( + strategy = "Transaction::v4_strategy(LedgerState::default()).prop_map(DisplayToDebug)" + )] + first: DisplayToDebug, - #[proptest(strategy = "Transaction::v4_strategy(LedgerState::default())")] - second: Transaction, + #[proptest( + strategy = "Transaction::v4_strategy(LedgerState::default()).prop_map(DisplayToDebug)" + )] + second: DisplayToDebug, conflict: SpendConflictForTransactionV4, }, /// Test V5 transactions to include Orchard nullifier conflicts. V5 { - #[proptest(strategy = "Transaction::v5_strategy(LedgerState::default())")] - first: Transaction, + #[proptest( + strategy = "Transaction::v5_strategy(LedgerState::default()).prop_map(DisplayToDebug)" + )] + first: DisplayToDebug, - #[proptest(strategy = "Transaction::v5_strategy(LedgerState::default())")] - second: Transaction, + #[proptest( + strategy = "Transaction::v5_strategy(LedgerState::default()).prop_map(DisplayToDebug)" + )] + second: DisplayToDebug, conflict: SpendConflictForTransactionV5, }, @@ -107,7 +212,243 @@ impl SpendConflictTestInput { } }; - (first.into(), second.into()) + (first.0.into(), second.0.into()) + } + + /// Return two transactions that have no spend conflicts. + pub fn unconflicting_transactions(self) -> (UnminedTx, UnminedTx) { + let (mut first, mut second) = match self { + SpendConflictTestInput::V4 { first, second, .. } => (first, second), + SpendConflictTestInput::V5 { first, second, .. } => (first, second), + }; + + Self::remove_transparent_conflicts(&mut first, &mut second); + Self::remove_sprout_conflicts(&mut first, &mut second); + Self::remove_sapling_conflicts(&mut first, &mut second); + Self::remove_orchard_conflicts(&mut first, &mut second); + + (first.0.into(), second.0.into()) + } + + /// Find transparent outpoint spends shared by two transactions, then remove them from the + /// transactions. + fn remove_transparent_conflicts(first: &mut Transaction, second: &mut Transaction) { + let first_spent_outpoints: HashSet<_> = first.spent_outpoints().collect(); + let second_spent_outpoints: HashSet<_> = second.spent_outpoints().collect(); + + let conflicts: HashSet<_> = first_spent_outpoints + .intersection(&second_spent_outpoints) + .collect(); + + for transaction in [first, second] { + transaction.inputs_mut().retain(|input| { + input + .outpoint() + .as_ref() + .map(|outpoint| !conflicts.contains(outpoint)) + .unwrap_or(true) + }); + } + } + + /// Find identical Sprout nullifiers revealed by both transactions, then remove the joinsplits + /// that contain them from both transactions. + fn remove_sprout_conflicts(first: &mut Transaction, second: &mut Transaction) { + let first_nullifiers: HashSet<_> = first.sprout_nullifiers().copied().collect(); + let second_nullifiers: HashSet<_> = second.sprout_nullifiers().copied().collect(); + + let conflicts: HashSet<_> = first_nullifiers + .intersection(&second_nullifiers) + .copied() + .collect(); + + for transaction in [first, second] { + match transaction { + // JoinSplits with Bctv14 Proofs + Transaction::V2 { joinsplit_data, .. } | Transaction::V3 { joinsplit_data, .. } => { + Self::remove_joinsplits_with_conflicts(joinsplit_data, &conflicts) + } + + // JoinSplits with Groth Proofs + Transaction::V4 { joinsplit_data, .. } => { + Self::remove_joinsplits_with_conflicts(joinsplit_data, &conflicts) + } + + // No JoinSplits + Transaction::V1 { .. } | Transaction::V5 { .. } => {} + } + } + } + + /// Remove from a transaction's [`JoinSplitData`] the joinsplits that contain nullifiers + /// present in the `conflicts` set. + /// + /// This may clear the entire Sprout joinsplit data. + fn remove_joinsplits_with_conflicts( + maybe_joinsplit_data: &mut Option>, + conflicts: &HashSet, + ) { + let mut should_clear_joinsplit_data = false; + + if let Some(joinsplit_data) = maybe_joinsplit_data.as_mut() { + joinsplit_data.rest.retain(|joinsplit| { + !joinsplit + .nullifiers + .iter() + .any(|nullifier| conflicts.contains(nullifier)) + }); + + let first_joinsplit_should_be_removed = joinsplit_data + .first + .nullifiers + .iter() + .any(|nullifier| conflicts.contains(nullifier)); + + if first_joinsplit_should_be_removed { + if joinsplit_data.rest.is_empty() { + should_clear_joinsplit_data = true; + } else { + joinsplit_data.first = joinsplit_data.rest.remove(0); + } + } + } + + if should_clear_joinsplit_data { + *maybe_joinsplit_data = None; + } + } + + /// Find identical Sapling nullifiers revealed by both transactions, then remove the spends + /// that contain them from both transactions. + fn remove_sapling_conflicts(first: &mut Transaction, second: &mut Transaction) { + let first_nullifiers: HashSet<_> = first.sapling_nullifiers().copied().collect(); + let second_nullifiers: HashSet<_> = second.sapling_nullifiers().copied().collect(); + + let conflicts: HashSet<_> = first_nullifiers + .intersection(&second_nullifiers) + .copied() + .collect(); + + for transaction in [first, second] { + match transaction { + // Spends with Groth Proofs + Transaction::V4 { + sapling_shielded_data, + .. + } => { + Self::remove_sapling_transfers_with_conflicts(sapling_shielded_data, &conflicts) + } + + Transaction::V5 { + sapling_shielded_data, + .. + } => { + Self::remove_sapling_transfers_with_conflicts(sapling_shielded_data, &conflicts) + } + + // No Spends + Transaction::V1 { .. } | Transaction::V2 { .. } | Transaction::V3 { .. } => {} + } + } + } + + /// Remove from a transaction's [`sapling::ShieldedData`] the spends that contain nullifiers + /// present in the `conflicts` set. + /// + /// This may clear the entire shielded data. + fn remove_sapling_transfers_with_conflicts( + maybe_shielded_data: &mut Option>, + conflicts: &HashSet, + ) where + A: sapling::AnchorVariant + Clone, + { + if let Some(shielded_data) = maybe_shielded_data.take() { + match shielded_data.transfers { + sapling::TransferData::JustOutputs { .. } => { + *maybe_shielded_data = Some(shielded_data) + } + + sapling::TransferData::SpendsAndMaybeOutputs { + shared_anchor, + spends, + maybe_outputs, + } => { + let updated_spends: Vec<_> = spends + .into_vec() + .into_iter() + .filter(|spend| !conflicts.contains(&spend.nullifier)) + .collect(); + + if let Ok(spends) = AtLeastOne::try_from(updated_spends) { + *maybe_shielded_data = Some(sapling::ShieldedData { + transfers: sapling::TransferData::SpendsAndMaybeOutputs { + shared_anchor, + spends, + maybe_outputs, + }, + ..shielded_data + }); + } else if let Ok(outputs) = AtLeastOne::try_from(maybe_outputs) { + *maybe_shielded_data = Some(sapling::ShieldedData { + transfers: sapling::TransferData::JustOutputs { outputs }, + ..shielded_data + }); + } + } + } + } + } + + /// Find identical Orchard nullifiers revealed by both transactions, then remove the actions + /// that contain them from both transactions. + fn remove_orchard_conflicts(first: &mut Transaction, second: &mut Transaction) { + let first_nullifiers: HashSet<_> = first.orchard_nullifiers().copied().collect(); + let second_nullifiers: HashSet<_> = second.orchard_nullifiers().copied().collect(); + + let conflicts: HashSet<_> = first_nullifiers + .intersection(&second_nullifiers) + .copied() + .collect(); + + for transaction in [first, second] { + match transaction { + Transaction::V5 { + orchard_shielded_data, + .. + } => Self::remove_orchard_actions_with_conflicts(orchard_shielded_data, &conflicts), + + // No Spends + Transaction::V1 { .. } + | Transaction::V2 { .. } + | Transaction::V3 { .. } + | Transaction::V4 { .. } => {} + } + } + } + + /// Remove from a transaction's [`orchard::ShieldedData`] the actions that contain nullifiers + /// present in the `conflicts` set. + /// + /// This may clear the entire shielded data. + fn remove_orchard_actions_with_conflicts( + maybe_shielded_data: &mut Option, + conflicts: &HashSet, + ) { + if let Some(shielded_data) = maybe_shielded_data.take() { + let updated_actions: Vec<_> = shielded_data + .actions + .into_vec() + .into_iter() + .filter(|action| !conflicts.contains(&action.action.nullifier)) + .collect(); + + if let Ok(actions) = AtLeastOne::try_from(updated_actions) { + *maybe_shielded_data = Some(orchard::ShieldedData { + actions, + ..shielded_data + }); + } + } } } @@ -130,27 +471,27 @@ enum SpendConflictForTransactionV5 { /// A conflict caused by spending the same UTXO. #[derive(Arbitrary, Clone, Debug)] struct TransparentSpendConflict { - new_input: transparent::Input, + new_input: DisplayToDebug, } /// A conflict caused by revealing the same Sprout nullifier. #[derive(Arbitrary, Clone, Debug)] struct SproutSpendConflict { - new_joinsplit_data: transaction::JoinSplitData, + new_joinsplit_data: DisplayToDebug>, } /// A conflict caused by revealing the same Sapling nullifier. #[derive(Clone, Debug)] struct SaplingSpendConflict { - new_spend: sapling::Spend, + new_spend: DisplayToDebug>, new_shared_anchor: A::Shared, - fallback_shielded_data: sapling::ShieldedData, + fallback_shielded_data: DisplayToDebug>, } /// A conflict caused by revealing the same Orchard nullifier. #[derive(Arbitrary, Clone, Debug)] struct OrchardSpendConflict { - new_shielded_data: orchard::ShieldedData, + new_shielded_data: DisplayToDebug, } impl SpendConflictForTransactionV4 { @@ -207,7 +548,7 @@ impl TransparentSpendConflict { /// Adds a new input to a transaction's list of transparent `inputs`. The transaction will then /// conflict with any other transaction that also has that same new input. pub fn apply_to(self, inputs: &mut Vec) { - inputs.push(self.new_input); + inputs.push(self.new_input.0); } } @@ -225,7 +566,7 @@ impl SproutSpendConflict { existing_joinsplit_data.first.nullifiers[0] = self.new_joinsplit_data.first.nullifiers[0]; } else { - *joinsplit_data = Some(self.new_joinsplit_data); + *joinsplit_data = Some(self.new_joinsplit_data.0); } } } @@ -247,9 +588,9 @@ where any::<(sapling::Spend, A::Shared, sapling::ShieldedData)>() .prop_map(|(new_spend, new_shared_anchor, fallback_shielded_data)| { SaplingSpendConflict { - new_spend, + new_spend: new_spend.into(), new_shared_anchor, - fallback_shielded_data, + fallback_shielded_data: fallback_shielded_data.into(), } }) .boxed() @@ -270,16 +611,16 @@ impl SaplingSpendConflict { pub fn apply_to(self, sapling_shielded_data: &mut Option>) { use sapling::TransferData::*; - let shielded_data = sapling_shielded_data.get_or_insert(self.fallback_shielded_data); + let shielded_data = sapling_shielded_data.get_or_insert(self.fallback_shielded_data.0); match &mut shielded_data.transfers { - SpendsAndMaybeOutputs { ref mut spends, .. } => spends.push(self.new_spend), + SpendsAndMaybeOutputs { ref mut spends, .. } => spends.push(self.new_spend.0), JustOutputs { ref mut outputs } => { let new_outputs = outputs.clone(); shielded_data.transfers = SpendsAndMaybeOutputs { shared_anchor: self.new_shared_anchor, - spends: at_least_one![self.new_spend], + spends: at_least_one![self.new_spend.0], maybe_outputs: new_outputs.into_vec(), }; } @@ -301,7 +642,99 @@ impl OrchardSpendConflict { shielded_data.actions.first_mut().action.nullifier = self.new_shielded_data.actions.first().action.nullifier; } else { - *orchard_shielded_data = Some(self.new_shielded_data); + *orchard_shielded_data = Some(self.new_shielded_data.0); + } + } +} + +/// A series of transactions and a sub-set of them to remove. +/// +/// The set of transactions to remove can either be exact WTX IDs to remove exact transactions or +/// mined IDs to remove transactions that have the same effects specified by the ID. +#[derive(Clone, Debug)] +pub enum MultipleTransactionRemovalTestInput { + RemoveExact { + transactions: SummaryDebug>, + wtx_ids_to_remove: SummaryDebug>, + }, + + RemoveSameEffects { + transactions: SummaryDebug>, + mined_ids_to_remove: SummaryDebug>, + }, +} + +impl Arbitrary for MultipleTransactionRemovalTestInput { + type Parameters = (); + + fn arbitrary_with(_: Self::Parameters) -> Self::Strategy { + vec(any::(), 1..MEMPOOL_SIZE) + .prop_flat_map(|transactions| { + let indices_to_remove = + vec(any::(), 1..=transactions.len()).prop_map(|removal_markers| { + removal_markers + .into_iter() + .enumerate() + .filter(|(_, should_remove)| *should_remove) + .map(|(index, _)| index) + .collect::>() + }); + + (Just(transactions), indices_to_remove) + }) + .prop_flat_map(|(transactions, indices_to_remove)| { + let wtx_ids_to_remove: HashSet<_> = indices_to_remove + .iter() + .map(|&index| transactions[index].id) + .collect(); + + let mined_ids_to_remove: HashSet = wtx_ids_to_remove + .iter() + .map(|unmined_id| unmined_id.mined_id()) + .collect(); + + prop_oneof![ + Just(RemoveSameEffects { + transactions: transactions.clone().into(), + mined_ids_to_remove: mined_ids_to_remove.into(), + }), + Just(RemoveExact { + transactions: transactions.into(), + wtx_ids_to_remove: wtx_ids_to_remove.into(), + }), + ] + }) + .boxed() + } + + type Strategy = BoxedStrategy; +} + +impl MultipleTransactionRemovalTestInput { + /// Iterate over all transactions generated as input. + pub fn transactions(&self) -> impl Iterator + '_ { + match self { + RemoveExact { transactions, .. } | RemoveSameEffects { transactions, .. } => { + transactions.iter() + } + } + } + + /// Return a [`HashSet`] of [`UnminedTxId`]s of the transactions to be removed. + pub fn removed_transaction_ids(&self) -> HashSet { + match self { + RemoveExact { + wtx_ids_to_remove, .. + } => wtx_ids_to_remove.0.clone(), + + RemoveSameEffects { + transactions, + mined_ids_to_remove, + } => transactions + .iter() + .map(|transaction| transaction.id) + .filter(|id| mined_ids_to_remove.contains(&id.mined_id())) + .collect(), } } } diff --git a/zebrad/src/components/mempool/storage/tests/vectors.rs b/zebrad/src/components/mempool/storage/tests/vectors.rs index 3a38cece..3aafecd2 100644 --- a/zebrad/src/components/mempool/storage/tests/vectors.rs +++ b/zebrad/src/components/mempool/storage/tests/vectors.rs @@ -93,7 +93,7 @@ fn mempool_storage_basic_for_network(network: Network) -> Result<()> { let expected_in_mempool = &unmined_transactions[rejected_transaction_count..]; // Only MEMPOOL_SIZE should land in verified - assert_eq!(storage.verified.len(), MEMPOOL_SIZE); + assert_eq!(storage.verified.transaction_count(), MEMPOOL_SIZE); // The rest of the transactions will be in rejected assert_eq!( diff --git a/zebrad/src/components/mempool/storage/verified_set.rs b/zebrad/src/components/mempool/storage/verified_set.rs new file mode 100644 index 00000000..55cd61f8 --- /dev/null +++ b/zebrad/src/components/mempool/storage/verified_set.rs @@ -0,0 +1,199 @@ +use std::{ + borrow::Cow, + collections::{HashSet, VecDeque}, + hash::Hash, +}; + +use zebra_chain::{ + orchard, sapling, sprout, + transaction::{Transaction, UnminedTx, UnminedTxId}, + transparent, +}; + +use super::super::SameEffectsTipRejectionError; + +/// The set of verified transactions stored in the mempool. +/// +/// This also caches the all the spent outputs from the transactions in the mempool. The spent +/// outputs include: +/// +/// - the transparent outpoints spent by transactions in the mempool +/// - the Sprout nullifiers revealed by transactions in the mempool +/// - the Sapling nullifiers revealed by transactions in the mempool +/// - the Orchard nullifiers revealed by transactions in the mempool +#[derive(Default)] +pub struct VerifiedSet { + /// The set of verified transactions in the mempool. + transactions: VecDeque, + + /// The set of spent out points by the verified transactions. + spent_outpoints: HashSet, + + /// The set of revealed Sprout nullifiers. + sprout_nullifiers: HashSet, + + /// The set of revealed Sapling nullifiers. + sapling_nullifiers: HashSet, + + /// The set of revealed Orchard nullifiers. + orchard_nullifiers: HashSet, +} + +impl VerifiedSet { + /// Returns an iterator over the transactions in the set. + pub fn transactions(&self) -> impl Iterator + '_ { + self.transactions.iter() + } + + /// Returns the number of verified transactions in the set. + pub fn transaction_count(&self) -> usize { + self.transactions.len() + } + + /// Returns `true` if the set of verified transactions contains the transaction with the + /// specified `id. + pub fn contains(&self, id: &UnminedTxId) -> bool { + self.transactions.iter().any(|tx| &tx.id == id) + } + + /// Clear the set of verified transactions. + /// + /// Also clears all internal caches. + pub fn clear(&mut self) { + self.transactions.clear(); + self.spent_outpoints.clear(); + self.sprout_nullifiers.clear(); + self.sapling_nullifiers.clear(); + self.orchard_nullifiers.clear(); + } + + /// Insert a `transaction` into the set. + /// + /// Returns an error if the `transaction` has spend conflicts with any other transaction + /// already in the set. + /// + /// Two transactions have a spend conflict if they spend the same UTXO or if they reveal the + /// same nullifier. + pub fn insert(&mut self, transaction: UnminedTx) -> Result<(), SameEffectsTipRejectionError> { + if self.has_spend_conflicts(&transaction) { + return Err(SameEffectsTipRejectionError::SpendConflict); + } + + self.cache_outputs_from(&transaction.transaction); + self.transactions.push_front(transaction); + + Ok(()) + } + + /// Evict one transaction from the set to open space for another transaction. + pub fn evict_one(&mut self) -> Option { + if self.transactions.is_empty() { + None + } else { + // TODO: use random weighted eviction as specified in ZIP-401 (#2780) + let last_index = self.transactions.len() - 1; + + Some(self.remove(last_index)) + } + } + + /// Removes all transactions in the set that match the `predicate`. + /// + /// Returns the amount of transactions removed. + pub fn remove_all_that(&mut self, predicate: impl Fn(&UnminedTx) -> bool) -> usize { + // Clippy suggests to remove the `collect` and the `into_iter` further down. However, it is + // unable to detect that when that is done, there is a borrow conflict. What happens is the + // iterator borrows `self.transactions` immutably, but it also need to be borrowed mutably + // in order to remove the transactions while traversing the iterator. + #[allow(clippy::needless_collect)] + let indices_to_remove: Vec<_> = self + .transactions + .iter() + .enumerate() + .filter(|(_, tx)| predicate(tx)) + .map(|(index, _)| index) + .collect(); + + let removed_count = indices_to_remove.len(); + + // Correctness: remove indexes in reverse order, + // so earlier indexes still correspond to the same transactions + for index_to_remove in indices_to_remove.into_iter().rev() { + self.remove(index_to_remove); + } + + removed_count + } + + /// Removes a transaction from the set. + /// + /// Also removes its outputs from the internal caches. + fn remove(&mut self, transaction_index: usize) -> UnminedTx { + let removed_tx = self + .transactions + .remove(transaction_index) + .expect("invalid transaction index"); + + self.remove_outputs(&removed_tx); + + removed_tx + } + + /// Returns `true` if the given `transaction` has any spend conflicts with transactions in the + /// mempool. + /// + /// Two transactions have a spend conflict if they spend the same UTXO or if they reveal the + /// same nullifier. + fn has_spend_conflicts(&self, unmined_tx: &UnminedTx) -> bool { + let tx = &unmined_tx.transaction; + + Self::has_conflicts(&self.spent_outpoints, tx.spent_outpoints()) + || Self::has_conflicts(&self.sprout_nullifiers, tx.sprout_nullifiers().copied()) + || Self::has_conflicts(&self.sapling_nullifiers, tx.sapling_nullifiers().copied()) + || Self::has_conflicts(&self.orchard_nullifiers, tx.orchard_nullifiers().copied()) + } + + /// Inserts the transaction's outputs into the internal caches. + fn cache_outputs_from(&mut self, tx: &Transaction) { + self.spent_outpoints.extend(tx.spent_outpoints()); + self.sprout_nullifiers.extend(tx.sprout_nullifiers()); + self.sapling_nullifiers.extend(tx.sapling_nullifiers()); + self.orchard_nullifiers.extend(tx.orchard_nullifiers()); + } + + /// Removes the tracked transaction outputs from the mempool. + fn remove_outputs(&mut self, unmined_tx: &UnminedTx) { + let tx = &unmined_tx.transaction; + + let spent_outpoints = tx.spent_outpoints().map(Cow::Owned); + let sprout_nullifiers = tx.sprout_nullifiers().map(Cow::Borrowed); + let sapling_nullifiers = tx.sapling_nullifiers().map(Cow::Borrowed); + let orchard_nullifiers = tx.orchard_nullifiers().map(Cow::Borrowed); + + Self::remove_from_set(&mut self.spent_outpoints, spent_outpoints); + Self::remove_from_set(&mut self.sprout_nullifiers, sprout_nullifiers); + Self::remove_from_set(&mut self.sapling_nullifiers, sapling_nullifiers); + Self::remove_from_set(&mut self.orchard_nullifiers, orchard_nullifiers); + } + + /// Returns `true` if the two sets have common items. + fn has_conflicts(set: &HashSet, mut list: impl Iterator) -> bool + where + T: Eq + Hash, + { + list.any(|item| set.contains(&item)) + } + + /// Removes some items from a [`HashSet`]. + /// + /// Each item in the list of `items` should be wrapped in a [`Cow`]. This allows this generic + /// method to support both borrowed and owned items. + fn remove_from_set<'t, T>(set: &mut HashSet, items: impl IntoIterator>) + where + T: Clone + Eq + Hash + 't, + { + for item in items { + set.remove(&item); + } + } +}