zecfoundation
/
zebra
mirror of https://github.com/ZcashFoundation/zebra.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Refactor mempool spend conflict checks to increase performance (#2826) 

* Add `HashSet`s to help spend conflict detection

Keep track of the spent transparent outpoints and the revealed
nullifiers.

Clippy complained that the `ActiveState` had variants with large size
differences, but that was expected, so I disabled that lint on that
`enum`.

* Clear the `HashSet`s when clearing the mempool

Clear them so that they remain consistent with the set of verified
transactions.

* Use `HashSet`s to check for spend conflicts

Store new outputs into its respective `HashSet`, and abort if a
duplicate output is found.

* Remove inserted outputs when aborting

Restore the `HashSet` to its previous state.

* Remove tracked outputs when removing a transaction

Keep the mempool storage in a consistent state when a transaction is
removed.

* Remove tracked outputs when evicting from mempool

Ensure eviction also keeps the tracked outputs consistent with the
verified transactions.

* Refactor to create a `VerifiedSet` helper type

Move the code to handle the output caches into the new type. Also move
the eviction code to make things a little simpler.

* Refactor to have a single `remove` method

Centralize the code that handles the removal of a transaction to avoid
mistakes.

* Move mempool size limiting back to `Storage`

Because the evicted transactions must be added to the rejected list.

* Remove leftover `dbg!` statement

Leftover from some temporary testing code.

Co-authored-by: teor <teor@riseup.net>

* Remove unnecessary `TODO`

It is more speculation than planning, so it doesn't add much value.

Co-authored-by: teor <teor@riseup.net>

* Fix typo in documentation

The verb should match the subject "transactions" which is plural.

Co-authored-by: teor <teor@riseup.net>

* Add a comment to warn about correctness

There's a subtle but important detail in the implementation that should
be made more visible to avoid mistakes in the future.

Co-authored-by: teor <teor@riseup.net>

* Remove outdated comment

Left-over from the attempt to move the eviction into the `VerifiedSet`.

* Improve comment explaining lint removal

Rewrite the comment explaining why the Clippy lint was ignored.

* Check for spend conflicts in `VerifiedSet`

Refactor to avoid API misuse.

* Test rejected transaction rollback

Using two transactions, perform the same test adding a conflict to both
of them to check if the second inserted transaction is properly
rejected. Then remove any conflicts from the second transaction and add
it again. That should work, because if it doesn't it means that when the
second transaction was rejected it left things it shouldn't in the
cache.

* Test removal of multiple transactions

When removing multiple transactions from the mempool storage, all of the
ones requested should be removed and any other transaction should be
still be there afterwards.

* Increase mempool size to 4, so that spend conflict tests work

If the mempool size is smaller than 4,
these tests don't fail on a trivial removal bug.
Because we need a minimum number of transactions in the mempool
to trigger the bug.

Also commit a proptest seed that fails on a trivial removal bug.
(This seed fails if we remove indexes in order,
because every index past the first removes the wrong transaction.)

* Summarise transaction data in proptest error output

* Summarise spend conflict field data in proptest error output

* Summarise multiple removal field data in proptest error output

And replace the very large proptest debug output with the new summary.

Co-authored-by: teor <teor@riseup.net>
This commit is contained in:
Janito Vaqueiro Ferreira Filho 2021-10-10 20:54:46 -03:00 committed by GitHub
parent dcf281efff
commit 9e78a8af40
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 834 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
zebra-chain/src/block.rs
View File

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

18
zebra-chain/src/orchard/shielded_data.rs
View File

@ -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<Binding>,
}
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.

23
zebra-chain/src/sapling/shielded_data.rs
View File

@ -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<AnchorV> fmt::Display for ShieldedData<AnchorV>
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::<AnchorV>()
)
.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<AnchorV> ShieldedData<AnchorV>
where
AnchorV: AnchorVariant + Clone,

17
zebra-chain/src/sapling/spend.rs
View File

@ -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<SpendAuth>,
}
impl<AnchorV> fmt::Display for Spend<AnchorV>
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::<AnchorV>()).as_str());
fmter.field("per_spend_anchor", &self.per_spend_anchor);
fmter.field("nullifier", &self.nullifier);
fmter.finish()
}
}
impl From<(Spend<SharedAnchor>, tree::Root)> for Spend<PerSpendAnchor> {
/// Convert a `Spend<SharedAnchor>` and its shared anchor, into a
/// `Spend<PerSpendAnchor>`.

24
zebra-chain/src/transaction.rs
View File

@ -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

14
zebra-chain/src/transaction/joinsplit.rs
View File

@ -1,3 +1,5 @@
use std::fmt;
use serde::{Deserialize, Serialize};
use crate::{
@ -46,6 +48,18 @@ pub struct JoinSplitData<P: ZkSnarkProof> {
pub sig: ed25519::Signature,
}
impl<P: ZkSnarkProof> fmt::Display for JoinSplitData<P> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let mut fmter =
f.debug_struct(format!("JoinSplitData<{}>", std::any::type_name::<P>()).as_str());
fmter.field("joinsplits", &self.joinsplits().count());
fmter.field("value_balance", &self.value_balance());
fmter.finish()
}
}
impl<P: ZkSnarkProof> JoinSplitData<P> {
/// Iterate over the [`JoinSplit`]s in `self`, in the order they appear
/// in the transaction.

29
zebra-chain/src/transparent.rs
View File

@ -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`.

7
zebrad/proptest-regressions/components/mempool/storage/tests/prop.txt Normal file
View File

@ -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<zebra_chain::transaction::unmined::UnminedTx><zebra_chain::transaction::unmined::UnminedTx>, len=2, mined_ids_to_remove: std::collections::hash::set::HashSet<zebra_chain::transaction::hash::Hash><zebra_chain::transaction::hash::Hash>, len=2 }

1
zebrad/src/components/mempool.rs
View File

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

117
zebrad/src/components/mempool/storage.rs
View File

@ -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<UnminedTx>,
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<UnminedTxId>) -> 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<transaction::Hash>) -> 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<Item = UnminedTxId> + '_ {
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<Item = &UnminedTx> {
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<UnminedTxId>,
) -> impl Iterator<Item = &UnminedTx> {
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))
}
}

499
zebrad/src/components/mempool/storage/tests/prop.rs
View File

@ -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::<SpendConflictTestInput>())
{
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::<MultipleTransactionRemovalTestInput>()) {
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<Transaction>,
#[proptest(strategy = "Transaction::v4_strategy(LedgerState::default())")]
second: Transaction,
#[proptest(
strategy = "Transaction::v4_strategy(LedgerState::default()).prop_map(DisplayToDebug)"
)]
second: DisplayToDebug<Transaction>,
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<Transaction>,
#[proptest(strategy = "Transaction::v5_strategy(LedgerState::default())")]
second: Transaction,
#[proptest(
strategy = "Transaction::v5_strategy(LedgerState::default()).prop_map(DisplayToDebug)"
)]
second: DisplayToDebug<Transaction>,
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<P: ZkSnarkProof>(
maybe_joinsplit_data: &mut Option<JoinSplitData<P>>,
conflicts: &HashSet<sprout::Nullifier>,
) {
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<A>(
maybe_shielded_data: &mut Option<sapling::ShieldedData<A>>,
conflicts: &HashSet<sapling::Nullifier>,
) 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<orchard::ShieldedData>,
conflicts: &HashSet<orchard::Nullifier>,
) {
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<transparent::Input>,
}
/// A conflict caused by revealing the same Sprout nullifier.
#[derive(Arbitrary, Clone, Debug)]
struct SproutSpendConflict {
new_joinsplit_data: transaction::JoinSplitData<Groth16Proof>,
new_joinsplit_data: DisplayToDebug<transaction::JoinSplitData<Groth16Proof>>,
}
/// A conflict caused by revealing the same Sapling nullifier.
#[derive(Clone, Debug)]
struct SaplingSpendConflict<A: sapling::AnchorVariant + Clone> {
new_spend: sapling::Spend<A>,
new_spend: DisplayToDebug<sapling::Spend<A>>,
new_shared_anchor: A::Shared,
fallback_shielded_data: sapling::ShieldedData<A>,
fallback_shielded_data: DisplayToDebug<sapling::ShieldedData<A>>,
}
/// A conflict caused by revealing the same Orchard nullifier.
#[derive(Arbitrary, Clone, Debug)]
struct OrchardSpendConflict {
new_shielded_data: orchard::ShieldedData,
new_shielded_data: DisplayToDebug<orchard::ShieldedData>,
}
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<transparent::Input>) {
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>, A::Shared, sapling::ShieldedData<A>)>()
.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<A: sapling::AnchorVariant + Clone> SaplingSpendConflict<A> {
pub fn apply_to(self, sapling_shielded_data: &mut Option<sapling::ShieldedData<A>>) {
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<Vec<UnminedTx>>,
wtx_ids_to_remove: SummaryDebug<HashSet<UnminedTxId>>,
},
RemoveSameEffects {
transactions: SummaryDebug<Vec<UnminedTx>>,
mined_ids_to_remove: SummaryDebug<HashSet<transaction::Hash>>,
},
}
impl Arbitrary for MultipleTransactionRemovalTestInput {
type Parameters = ();
fn arbitrary_with(_: Self::Parameters) -> Self::Strategy {
vec(any::<UnminedTx>(), 1..MEMPOOL_SIZE)
.prop_flat_map(|transactions| {
let indices_to_remove =
vec(any::<bool>(), 1..=transactions.len()).prop_map(|removal_markers| {
removal_markers
.into_iter()
.enumerate()
.filter(|(_, should_remove)| *should_remove)
.map(|(index, _)| index)
.collect::<HashSet<usize>>()
});
(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<transaction::Hash> = 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<Self>;
}
impl MultipleTransactionRemovalTestInput {
/// Iterate over all transactions generated as input.
pub fn transactions(&self) -> impl Iterator<Item = &UnminedTx> + '_ {
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<UnminedTxId> {
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(),
}
}
}

2
zebrad/src/components/mempool/storage/tests/vectors.rs
View File

@ -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!(

199
zebrad/src/components/mempool/storage/verified_set.rs Normal file
View File

@ -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<UnminedTx>,
/// The set of spent out points by the verified transactions.
spent_outpoints: HashSet<transparent::OutPoint>,
/// The set of revealed Sprout nullifiers.
sprout_nullifiers: HashSet<sprout::Nullifier>,
/// The set of revealed Sapling nullifiers.
sapling_nullifiers: HashSet<sapling::Nullifier>,
/// The set of revealed Orchard nullifiers.
orchard_nullifiers: HashSet<orchard::Nullifier>,
}
impl VerifiedSet {
/// Returns an iterator over the transactions in the set.
pub fn transactions(&self) -> impl Iterator<Item = &UnminedTx> + '_ {
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<UnminedTx> {
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<T>(set: &HashSet<T>, mut list: impl Iterator<Item = T>) -> 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<T>, items: impl IntoIterator<Item = Cow<'t, T>>)
where
T: Clone + Eq + Hash + 't,
{
for item in items {
set.remove(&item);
}
}
}