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

change(mempool) reject transactions with spent outpoints or nullifiers (#5434) 

* adds transactions to ChainTipBlock and rejects transactions from the mempool that have been invalidated by the latest block commit

* merges remove_same_effects in with reject_invalidated_transactions, moves nullifier retrieval to the new Storage method, rejects mined_ids, and updates tests

* updates DuplicateSpend's error message

* fixes tests

* Update zebrad/src/components/mempool/storage.rs

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

* adds comment

* formatting for the proptest

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: teor <teor@riseup.net>
This commit is contained in:
Arya 2022-10-24 15:48:45 -04:00 committed by GitHub
parent 737fbac3fc
commit e9452d9a6f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 185 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
zebra-state/src/arbitrary.rs
View File

@ -59,10 +59,12 @@ impl From<PreparedBlock> for ChainTipBlock {
new_outputs: _,
transaction_hashes,
} = prepared;
Self {
hash,
height,
time: block.header.time,
transactions: block.transactions.clone(),
transaction_hashes,
previous_block_hash: block.header.previous_block_hash,
}

8
zebra-state/src/service/chain_tip.rs
View File

@ -20,7 +20,7 @@ use zebra_chain::{
block,
chain_tip::ChainTip,
parameters::{Network, NetworkUpgrade},
transaction,
transaction::{self, Transaction},
};
use crate::{
@ -56,6 +56,9 @@ pub struct ChainTipBlock {
)]
pub time: DateTime<Utc>,
/// The block transactions.
pub transactions: Vec<Arc<Transaction>>,
/// The mined transaction IDs of the transactions in `block`,
/// in the same order as `block.transactions`.
pub transaction_hashes: Arc<[transaction::Hash]>,
@ -84,6 +87,7 @@ impl From<ContextuallyValidBlock> for ChainTipBlock {
hash,
height,
time: block.header.time,
transactions: block.transactions.clone(),
transaction_hashes,
previous_block_hash: block.header.previous_block_hash,
}
@ -99,10 +103,12 @@ impl From<FinalizedBlock> for ChainTipBlock {
transaction_hashes,
..
} = finalized;
Self {
hash,
height,
time: block.header.time,
transactions: block.transactions.clone(),
transaction_hashes,
previous_block_hash: block.header.previous_block_hash,
}

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

@ -369,7 +369,7 @@ impl Service<Request> for Mempool {
// with the same mined IDs as recently mined transactions.
let mined_ids = block.transaction_hashes.iter().cloned().collect();
tx_downloads.cancel(&mined_ids);
storage.remove_same_effects(&mined_ids);
storage.reject_and_remove_same_effects(&mined_ids, block.transactions);
storage.clear_tip_rejections();
}

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

@ -10,12 +10,15 @@
use std::{
collections::{HashMap, HashSet},
mem::size_of,
sync::Arc,
time::Duration,
};
use thiserror::Error;
use zebra_chain::transaction::{self, Hash, UnminedTx, UnminedTxId, VerifiedUnminedTx};
use zebra_chain::transaction::{
self, Hash, Transaction, UnminedTx, UnminedTxId, VerifiedUnminedTx,
};
use self::{eviction_list::EvictionList, verified_set::VerifiedSet};
use super::{config, downloads::TransactionDownloadVerifyError, MempoolError};
@ -78,6 +81,12 @@ pub enum SameEffectsChainRejectionError {
#[error("best chain tip has reached transaction expiry height")]
Expired,
#[error("transaction inputs were spent, or nullifiers were revealed, in the best chain")]
DuplicateSpend,
#[error("transaction was committed to the best chain")]
Mined,
/// Otherwise valid transaction removed from mempool due to [ZIP-401] random
/// eviction.
///
@ -276,22 +285,89 @@ impl Storage {
.remove_all_that(|tx| exact_wtxids.contains(&tx.transaction.id))
}
/// Remove transactions from the mempool via non-malleable [`transaction::Hash`].
///
/// For v5 transactions, transactions are matched by TXID,
/// Reject and remove transactions from the mempool via non-malleable [`transaction::Hash`].
/// - For v5 transactions, transactions are matched by TXID,
/// using only the non-malleable transaction ID.
/// This matches any transaction with the same effect on the blockchain state,
/// even if its signatures and proofs are different.
///
/// Returns the number of transactions which were removed.
///
/// Removes from the 'verified' set, if present.
/// - Returns the number of transactions which were removed.
/// - Removes from the 'verified' set, if present.
/// Maintains the order in which the other unmined transactions have been inserted into the mempool.
///
/// Does not add or remove from the 'rejected' tracking set.
pub fn remove_same_effects(&mut self, mined_ids: &HashSet<transaction::Hash>) -> usize {
self.verified
.remove_all_that(|tx| mined_ids.contains(&tx.transaction.id.mined_id()))
/// Reject and remove transactions from the mempool that contain any outpoints or nullifiers in
/// the `spent_outpoints` or `nullifiers` collections that are passed in.
///
/// Returns the number of transactions that were removed.
pub fn reject_and_remove_same_effects(
&mut self,
mined_ids: &HashSet<transaction::Hash>,
transactions: Vec<Arc<Transaction>>,
) -> usize {
let num_removed_mined = self
.verified
.remove_all_that(|tx| mined_ids.contains(&tx.transaction.id.mined_id()));
let spent_outpoints: HashSet<_> = transactions
.iter()
.flat_map(|tx| tx.spent_outpoints())
.collect();
let sprout_nullifiers: HashSet<_> = transactions
.iter()
.flat_map(|transaction| transaction.sprout_nullifiers())
.collect();
let sapling_nullifiers: HashSet<_> = transactions
.iter()
.flat_map(|transaction| transaction.sapling_nullifiers())
.collect();
let orchard_nullifiers: HashSet<_> = transactions
.iter()
.flat_map(|transaction| transaction.orchard_nullifiers())
.collect();
let duplicate_spend_ids: HashSet<_> = self
.verified
.transactions()
.filter_map(|tx| {
(tx.transaction
.spent_outpoints()
.any(|outpoint| spent_outpoints.contains(&outpoint))
|| tx
.transaction
.sprout_nullifiers()
.any(|nullifier| sprout_nullifiers.contains(nullifier))
|| tx
.transaction
.sapling_nullifiers()
.any(|nullifier| sapling_nullifiers.contains(nullifier))
|| tx
.transaction
.orchard_nullifiers()
.any(|nullifier| orchard_nullifiers.contains(nullifier)))
.then_some(tx.id)
})
.collect();
let num_removed_duplicate_spend = self
.verified
.remove_all_that(|tx| duplicate_spend_ids.contains(&tx.transaction.id));
for &mined_id in mined_ids {
self.reject(
// the reject and rejection_error fns that store and check `SameEffectsChainRejectionError`s
// only use the mined id, so using `Legacy` ids will apply to v5 transactions as well.
UnminedTxId::Legacy(mined_id),
SameEffectsChainRejectionError::Mined.into(),
);
}
for duplicate_spend_id in duplicate_spend_ids {
self.reject(
duplicate_spend_id,
SameEffectsChainRejectionError::DuplicateSpend.into(),
);
}
num_removed_mined + num_removed_duplicate_spend
}
/// Clears the whole mempool storage.
@ -520,7 +596,8 @@ impl Storage {
}
// expiry height is effecting data, so we match by non-malleable TXID
self.remove_same_effects(&txid_set);
self.verified
.remove_all_that(|tx| txid_set.contains(&tx.transaction.id.mined_id()));
// also reject it
for id in unmined_id_set.iter() {

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

@ -364,10 +364,14 @@ proptest! {
// 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 inserted_transactions: HashSet<_> = input
.transactions()
.filter_map(|transaction| {
let id = transaction.transaction.id;
storage.insert(transaction.clone()).ok().map(|_| id)}).collect();
storage.insert(transaction.clone()).ok().map(|_| id)
})
.collect();
// Check that the inserted transactions are still there.
for transaction_id in &inserted_transactions {
@ -377,7 +381,16 @@ proptest! {
// 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),
RejectAndRemoveSameEffects { mined_ids_to_remove, .. } => {
let num_removals = storage.reject_and_remove_same_effects(mined_ids_to_remove, vec![]);
for &removed_transaction_id in mined_ids_to_remove.iter() {
prop_assert_eq!(
storage.rejection_error(&UnminedTxId::Legacy(removed_transaction_id)),
Some(SameEffectsChainRejectionError::Mined.into())
);
}
num_removals
},
};
// Check that the removed transactions are not in the storage.
@ -911,7 +924,7 @@ pub enum MultipleTransactionRemovalTestInput {
wtx_ids_to_remove: SummaryDebug<HashSet<UnminedTxId>>,
},
RemoveSameEffects {
RejectAndRemoveSameEffects {
transactions: SummaryDebug<Vec<VerifiedUnminedTx>>,
mined_ids_to_remove: SummaryDebug<HashSet<transaction::Hash>>,
},
@ -947,7 +960,7 @@ impl Arbitrary for MultipleTransactionRemovalTestInput {
.collect();
prop_oneof![
Just(RemoveSameEffects {
Just(RejectAndRemoveSameEffects {
transactions: transactions.clone().into(),
mined_ids_to_remove: mined_ids_to_remove.into(),
}),
@ -967,7 +980,7 @@ impl MultipleTransactionRemovalTestInput {
/// Iterate over all transactions generated as input.
pub fn transactions(&self) -> impl Iterator<Item = &VerifiedUnminedTx> + '_ {
match self {
RemoveExact { transactions, .. } | RemoveSameEffects { transactions, .. } => {
RemoveExact { transactions, .. } | RejectAndRemoveSameEffects { transactions, .. } => {
transactions.iter()
}
}
@ -980,7 +993,7 @@ impl MultipleTransactionRemovalTestInput {
wtx_ids_to_remove, ..
} => wtx_ids_to_remove.0.clone(),
RemoveSameEffects {
RejectAndRemoveSameEffects {
transactions,
mined_ids_to_remove,
} => transactions

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

@ -162,23 +162,75 @@ fn mempool_storage_crud_same_effects_mainnet() {
});
// Get one (1) unmined transaction
let unmined_tx = unmined_transactions_in_blocks(.., network)
let unmined_tx_1 = unmined_transactions_in_blocks(.., network)
.next()
.expect("at least one unmined transaction");
// Insert unmined tx into the mempool.
let _ = storage.insert(unmined_tx.clone());
let _ = storage.insert(unmined_tx_1.clone());
// Check that it is in the mempool, and not rejected.
assert!(storage.contains_transaction_exact(&unmined_tx.transaction.id));
assert!(storage.contains_transaction_exact(&unmined_tx_1.transaction.id));
// Remove tx
let removal_count =
storage.remove_same_effects(&iter::once(unmined_tx.transaction.id.mined_id()).collect());
// Reject and remove mined tx
let removal_count = storage.reject_and_remove_same_effects(
&iter::once(unmined_tx_1.transaction.id.mined_id()).collect(),
vec![unmined_tx_1.transaction.transaction.clone()],
);
// Check that it is /not/ in the mempool.
// Check that it is /not/ in the mempool as a verified transaction.
assert_eq!(removal_count, 1);
assert!(!storage.contains_transaction_exact(&unmined_tx.transaction.id));
assert!(!storage.contains_transaction_exact(&unmined_tx_1.transaction.id));
// Check that it's rejection is cached in the chain_rejected_same_effects' `Mined` eviction list.
assert_eq!(
storage.rejection_error(&unmined_tx_1.transaction.id),
Some(SameEffectsChainRejectionError::Mined.into())
);
assert_eq!(
storage.insert(unmined_tx_1),
Err(SameEffectsChainRejectionError::Mined.into())
);
// Get a different unmined transaction
let unmined_tx_2 = unmined_transactions_in_blocks(1.., network)
.find(|tx| {
tx.transaction
.transaction
.spent_outpoints()
.next()
.is_some()
})
.expect("at least one unmined transaction with at least 1 spent outpoint");
// Insert unmined tx into the mempool.
assert_eq!(
storage.insert(unmined_tx_2.clone()),
Ok(unmined_tx_2.transaction.id)
);
// Check that it is in the mempool, and not rejected.
assert!(storage.contains_transaction_exact(&unmined_tx_2.transaction.id));
// Reject and remove duplicate spend tx
let removal_count = storage.reject_and_remove_same_effects(
&HashSet::new(),
vec![unmined_tx_2.transaction.transaction.clone()],
);
// Check that it is /not/ in the mempool as a verified transaction.
assert_eq!(removal_count, 1);
assert!(!storage.contains_transaction_exact(&unmined_tx_2.transaction.id));
// Check that it's rejection is cached in the chain_rejected_same_effects' `SpendConflict` eviction list.
assert_eq!(
storage.rejection_error(&unmined_tx_2.transaction.id),
Some(SameEffectsChainRejectionError::DuplicateSpend.into())
);
assert_eq!(
storage.insert(unmined_tx_2),
Err(SameEffectsChainRejectionError::DuplicateSpend.into())
);
}
#[test]

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

@ -288,6 +288,7 @@ impl FakeChainTip {
hash: chain_tip_block.hash,
height,
time: previous.time + mock_block_time_delta,
transactions: chain_tip_block.transactions.clone(),
transaction_hashes: chain_tip_block.transaction_hashes.clone(),
previous_block_hash: previous.hash,
}