Reject conflicting mempool transactions (#2765)

* Add `Transaction::spent_outpoints` getter method

Returns an iterator over the UTXO `OutPoint`s spent by the transaction.

* Add `mempool::Error::Conflict` variant

An error representing that a transaction was rejected because it
conflicts with another transaction that's already in the mempool.

* Reject conflicting mempool transactions

Reject including a transaction in the mempool if it spends outputs
already spent by, or reveals nullifiers already revealed by another
transaction in the mempool.

* Fix typo in documentation

Remove the `r` that was incorrectly added.

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

* Specify that the conflict is a spend conflict

Make the situation clearer, because there are other types of conflict.

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

* Clarify that the outpoints are from inputs

Because otherwise it could lead to confusion because it could also mean
the outputs of the transaction represented as `OutPoint` references.

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

* Create `storage::tests::vectors` module

Refactor to follow the convention used for other tests.

* Add an `AtLeastOne::first_mut` method

A getter to allow changing the first element.

* Add an `AtLeastOne::push` method

Allow appending elements to the collection.

* Derive `Arbitrary` for `FieldNotPresent`

This is just to make the code that generates arbitrary anchors a bit
simpler.

* Test if conflicting transactions are rejected

Generate two transactions (either V4 or V5) and insert a conflicting
spend, which can be either a transparent UTXO, or a nullifier for one of
the shielded pools. Check that any attempt to insert both transactions
causes one to be accepted and the other to be rejected.

* Delete a TODO comment that we decided not to do

Co-authored-by: teor <teor@riseup.net>
This commit is contained in:
Janito Vaqueiro Ferreira Filho 2021-09-27 22:03:08 -03:00 committed by GitHub
parent 4567701933
commit a0d45c38f3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 483 additions and 96 deletions

View File

@ -4,6 +4,8 @@
//! The `value_balance` change is handled using the default zero value.
//! The anchor change is handled using the `AnchorVariant` type trait.
#[cfg(any(test, feature = "proptest-impl"))]
use proptest_derive::Arbitrary;
use serde::{de::DeserializeOwned, Serialize};
use crate::{
@ -35,6 +37,7 @@ pub struct SharedAnchor {}
/// This field is not present in this transaction version.
#[derive(Copy, Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
#[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))]
pub struct FieldNotPresent;
impl AnchorVariant for PerSpendAnchor {

View File

@ -185,6 +185,18 @@ impl<T> AtLeastOne<T> {
&self.inner[0]
}
/// Returns a mutable reference to the first element.
///
/// Unlike `Vec` or slice, `AtLeastOne` always has a first element.
pub fn first_mut(&mut self) -> &mut T {
&mut self.inner[0]
}
/// Appends an element to the back of the collection.
pub fn push(&mut self, element: T) {
self.inner.push(element);
}
/// Returns the first and all the rest of the elements of the vector.
///
/// Unlike `Vec` or slice, `AtLeastOne` always has a first element.

View File

@ -349,6 +349,13 @@ impl Transaction {
}
}
/// Access the [`transparent::OutPoint`]s spent by this transaction's [`transparent::Input`]s.
pub fn spent_outpoints(&self) -> impl Iterator<Item = transparent::OutPoint> + '_ {
self.inputs()
.iter()
.filter_map(transparent::Input::outpoint)
}
/// Access the transparent outputs of this transaction, regardless of version.
pub fn outputs(&self) -> &[transparent::Output] {
match self {

View File

@ -60,6 +60,7 @@ tokio = { version = "0.3.6", features = ["full", "test-util"] }
proptest = "0.10"
proptest-derive = "0.3"
zebra-chain = { path = "../zebra-chain", features = ["proptest-impl"] }
zebra-test = { path = "../zebra-test" }
[features]

View File

@ -43,4 +43,11 @@ pub enum MempoolError {
/// The queue's capacity is [`super::downloads::MAX_INBOUND_CONCURRENCY`].
#[error("transaction dropped because the queue is full")]
FullQueue,
/// The transaction has a spend conflict with another transaction already in the mempool.
#[error(
"transaction rejected because another transaction in the mempool has already spent some of \
its inputs"
)]
SpendConflict,
}

View File

@ -1,8 +1,11 @@
use std::collections::{HashMap, HashSet, VecDeque};
use std::{
collections::{HashMap, HashSet, VecDeque},
hash::Hash,
};
use zebra_chain::{
block,
transaction::{UnminedTx, UnminedTxId},
transaction::{Transaction, UnminedTx, UnminedTxId},
};
use zebra_consensus::error::TransactionError;
@ -21,6 +24,8 @@ pub enum State {
/// An otherwise valid mempool transaction was mined into a block, therefore
/// no longer belongs in the mempool.
Confirmed(block::Hash),
/// Rejected because it has a spend conflict with another transaction already in the mempool.
SpendConflict,
/// Stayed in mempool for too long without being mined.
// TODO(2021-09-09): Implement ZIP-203: Validate Transaction Expiry Height.
// TODO(2021-09-09): https://github.com/ZcashFoundation/zebra/issues/2387
@ -58,6 +63,7 @@ impl Storage {
State::Confirmed(block_hash) => MempoolError::InBlock(*block_hash),
State::Excess => MempoolError::Excess,
State::LowFee => MempoolError::LowFee,
State::SpendConflict => MempoolError::SpendConflict,
});
}
@ -69,6 +75,14 @@ impl Storage {
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) {
self.rejected.insert(tx.id, State::SpendConflict);
return Err(MempoolError::Rejected);
}
// Then, we insert into the pool.
self.verified.push_front(tx);
@ -146,4 +160,38 @@ impl Storage {
self.verified.clear();
self.rejected.clear();
}
/// 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))
}
}

View File

@ -1,103 +1,11 @@
use std::ops::RangeBounds;
use super::*;
use zebra_chain::{
block::Block, parameters::Network, serialization::ZcashDeserializeInto, transaction::UnminedTx,
};
use color_eyre::eyre::Result;
#[test]
fn mempool_storage_crud_mainnet() {
zebra_test::init();
let network = Network::Mainnet;
// Create an empty storage instance
let mut storage: Storage = Default::default();
// Get one (1) unmined transaction
let unmined_tx = unmined_transactions_in_blocks(.., network)
.next()
.expect("at least one unmined transaction");
// Insert unmined tx into the mempool.
let _ = storage.insert(unmined_tx.clone());
// Check that it is in the mempool, and not rejected.
assert!(storage.contains(&unmined_tx.id));
// Remove tx
let _ = storage.remove(&unmined_tx.id);
// Check that it is /not/ in the mempool.
assert!(!storage.contains(&unmined_tx.id));
}
#[test]
fn mempool_storage_basic() -> Result<()> {
zebra_test::init();
mempool_storage_basic_for_network(Network::Mainnet)?;
mempool_storage_basic_for_network(Network::Testnet)?;
Ok(())
}
fn mempool_storage_basic_for_network(network: Network) -> Result<()> {
// Create an empty storage
let mut storage: Storage = Default::default();
// Get transactions from the first 10 blocks of the Zcash blockchain
let unmined_transactions: Vec<_> = unmined_transactions_in_blocks(..=10, network).collect();
let total_transactions = unmined_transactions.len();
// Insert them all to the storage
for unmined_transaction in unmined_transactions.clone() {
storage.insert(unmined_transaction)?;
}
// Separate transactions into the ones expected to be in the mempool and those expected to be
// rejected.
let rejected_transaction_count = total_transactions - MEMPOOL_SIZE;
let expected_to_be_rejected = &unmined_transactions[..rejected_transaction_count];
let expected_in_mempool = &unmined_transactions[rejected_transaction_count..];
// Only MEMPOOL_SIZE should land in verified
assert_eq!(storage.verified.len(), MEMPOOL_SIZE);
// The rest of the transactions will be in rejected
assert_eq!(storage.rejected.len(), rejected_transaction_count);
// Make sure the last MEMPOOL_SIZE transactions we sent are in the verified
for tx in expected_in_mempool {
assert!(storage.contains(&tx.id));
}
// Anything greater should not be in the verified
for tx in expected_to_be_rejected {
assert!(!storage.contains(&tx.id));
}
// Query all the ids we have for rejected, get back `total - MEMPOOL_SIZE`
let all_ids: HashSet<UnminedTxId> = unmined_transactions.iter().map(|tx| tx.id).collect();
// Convert response to a `HashSet` as we need a fixed order to compare.
let rejected_response: HashSet<UnminedTxId> =
storage.rejected_transactions(all_ids).into_iter().collect();
let rejected_ids = expected_to_be_rejected.iter().map(|tx| tx.id).collect();
assert_eq!(rejected_response, rejected_ids);
// Use `contains_rejected` to make sure the first id stored is now rejected
assert!(storage.contains_rejected(&expected_to_be_rejected[0].id));
// Use `contains_rejected` to make sure the last id stored is not rejected
assert!(!storage.contains_rejected(&expected_in_mempool[0].id));
Ok(())
}
mod prop;
mod vectors;
pub fn unmined_transactions_in_blocks(
block_height_range: impl RangeBounds<u32>,

View File

@ -0,0 +1,305 @@
use std::fmt::Debug;
use proptest::prelude::*;
use proptest_derive::Arbitrary;
use zebra_chain::{
at_least_one, orchard,
primitives::Groth16Proof,
sapling,
transaction::{self, Transaction, UnminedTx},
transparent, LedgerState,
};
use super::super::{MempoolError, Storage};
proptest! {
/// Test if a transaction that has a spend conflict with a transaction already in the mempool
/// is rejected.
///
/// A spend conflict in this case is when two transactions spend the same UTXO or reveal the
/// same nullifier.
#[test]
fn conflicting_transactions_are_rejected(input in any::<SpendConflictTestInput>()) {
let mut storage = Storage::default();
let (first_transaction, second_transaction) = input.conflicting_transactions();
let input_permutations = vec![
(first_transaction.clone(), second_transaction.clone()),
(second_transaction, first_transaction),
];
for (transaction_to_accept, transaction_to_reject) in input_permutations {
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_reject),
Err(MempoolError::Rejected)
);
assert!(storage.contains_rejected(&id_to_reject));
storage.clear();
}
}
}
/// 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)]
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())")]
second: 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())")]
second: Transaction,
conflict: SpendConflictForTransactionV5,
},
}
impl SpendConflictTestInput {
/// Return two transactions that have a spend conflict.
pub fn conflicting_transactions(self) -> (UnminedTx, UnminedTx) {
let (first, second) = match self {
SpendConflictTestInput::V4 {
mut first,
mut second,
conflict,
} => {
conflict.clone().apply_to(&mut first);
conflict.apply_to(&mut second);
(first, second)
}
SpendConflictTestInput::V5 {
mut first,
mut second,
conflict,
} => {
conflict.clone().apply_to(&mut first);
conflict.apply_to(&mut second);
(first, second)
}
};
(first.into(), second.into())
}
}
/// A spend conflict valid for V4 transactions.
#[derive(Arbitrary, Clone, Debug)]
enum SpendConflictForTransactionV4 {
Transparent(Box<TransparentSpendConflict>),
Sprout(Box<SproutSpendConflict>),
Sapling(Box<SaplingSpendConflict<sapling::PerSpendAnchor>>),
}
/// A spend conflict valid for V5 transactions.
#[derive(Arbitrary, Clone, Debug)]
enum SpendConflictForTransactionV5 {
Transparent(Box<TransparentSpendConflict>),
Sapling(Box<SaplingSpendConflict<sapling::SharedAnchor>>),
Orchard(Box<OrchardSpendConflict>),
}
/// A conflict caused by spending the same UTXO.
#[derive(Arbitrary, Clone, Debug)]
struct TransparentSpendConflict {
new_input: transparent::Input,
}
/// A conflict caused by revealing the same Sprout nullifier.
#[derive(Arbitrary, Clone, Debug)]
struct SproutSpendConflict {
new_joinsplit_data: 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_shared_anchor: A::Shared,
fallback_shielded_data: sapling::ShieldedData<A>,
}
/// A conflict caused by revealing the same Orchard nullifier.
#[derive(Arbitrary, Clone, Debug)]
struct OrchardSpendConflict {
new_shielded_data: orchard::ShieldedData,
}
impl SpendConflictForTransactionV4 {
/// Apply a spend conflict to a V4 transaction.
///
/// Changes the `transaction_v4` to include the spend that will result in a conflict.
pub fn apply_to(self, transaction_v4: &mut Transaction) {
let (inputs, joinsplit_data, sapling_shielded_data) = match transaction_v4 {
Transaction::V4 {
inputs,
joinsplit_data,
sapling_shielded_data,
..
} => (inputs, joinsplit_data, sapling_shielded_data),
_ => unreachable!("incorrect transaction version generated for test"),
};
use SpendConflictForTransactionV4::*;
match self {
Transparent(transparent_conflict) => transparent_conflict.apply_to(inputs),
Sprout(sprout_conflict) => sprout_conflict.apply_to(joinsplit_data),
Sapling(sapling_conflict) => sapling_conflict.apply_to(sapling_shielded_data),
}
}
}
impl SpendConflictForTransactionV5 {
/// Apply a spend conflict to a V5 transaction.
///
/// Changes the `transaction_v5` to include the spend that will result in a conflict.
pub fn apply_to(self, transaction_v5: &mut Transaction) {
let (inputs, sapling_shielded_data, orchard_shielded_data) = match transaction_v5 {
Transaction::V5 {
inputs,
sapling_shielded_data,
orchard_shielded_data,
..
} => (inputs, sapling_shielded_data, orchard_shielded_data),
_ => unreachable!("incorrect transaction version generated for test"),
};
use SpendConflictForTransactionV5::*;
match self {
Transparent(transparent_conflict) => transparent_conflict.apply_to(inputs),
Sapling(sapling_conflict) => sapling_conflict.apply_to(sapling_shielded_data),
Orchard(orchard_conflict) => orchard_conflict.apply_to(orchard_shielded_data),
}
}
}
impl TransparentSpendConflict {
/// Apply a transparent spend conflict.
///
/// 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);
}
}
impl SproutSpendConflict {
/// Apply a Sprout spend conflict.
///
/// Ensures that a transaction's `joinsplit_data` has a nullifier used to represent a conflict.
/// If the transaction already has Sprout joinsplits, the first nullifier is replaced with the
/// new nullifier. Otherwise, a joinsplit is inserted with that new nullifier in the
/// transaction.
///
/// The transaction will then conflict with any other transaction with the same new nullifier.
pub fn apply_to(self, joinsplit_data: &mut Option<transaction::JoinSplitData<Groth16Proof>>) {
if let Some(existing_joinsplit_data) = joinsplit_data.as_mut() {
existing_joinsplit_data.first.nullifiers[0] =
self.new_joinsplit_data.first.nullifiers[0];
} else {
*joinsplit_data = Some(self.new_joinsplit_data);
}
}
}
/// Generate arbitrary [`SaplingSpendConflict`]s.
///
/// This had to be implemented manually because of the constraints required as a consequence of the
/// generic type parameter.
impl<A> Arbitrary for SaplingSpendConflict<A>
where
A: sapling::AnchorVariant + Clone + Debug + 'static,
A::Shared: Arbitrary,
sapling::Spend<A>: Arbitrary,
sapling::TransferData<A>: Arbitrary,
{
type Parameters = ();
fn arbitrary_with(_: Self::Parameters) -> Self::Strategy {
any::<(sapling::Spend<A>, A::Shared, sapling::ShieldedData<A>)>()
.prop_map(|(new_spend, new_shared_anchor, fallback_shielded_data)| {
SaplingSpendConflict {
new_spend,
new_shared_anchor,
fallback_shielded_data,
}
})
.boxed()
}
type Strategy = BoxedStrategy<Self>;
}
impl<A: sapling::AnchorVariant + Clone> SaplingSpendConflict<A> {
/// Apply a Sapling spend conflict.
///
/// Ensures that a transaction's `sapling_shielded_data` has a nullifier used to represent a
/// conflict. If the transaction already has Sapling shielded data, a new spend is added with
/// the new nullifier. Otherwise, a fallback instance of Sapling shielded data is inserted in
/// the transaction, and then the spend is added.
///
/// The transaction will then conflict with any other transaction with the same new nullifier.
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);
match &mut shielded_data.transfers {
SpendsAndMaybeOutputs { ref mut spends, .. } => spends.push(self.new_spend),
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],
maybe_outputs: new_outputs.into_vec(),
};
}
}
}
}
impl OrchardSpendConflict {
/// Apply a Orchard spend conflict.
///
/// Ensures that a transaction's `orchard_shielded_data` has a nullifier used to represent a
/// conflict. If the transaction already has Orchard shielded data, a new action is added with
/// the new nullifier. Otherwise, a fallback instance of Orchard shielded data that contains
/// the new action is inserted in the transaction.
///
/// The transaction will then conflict with any other transaction with the same new nullifier.
pub fn apply_to(self, orchard_shielded_data: &mut Option<orchard::ShieldedData>) {
if let Some(shielded_data) = orchard_shielded_data.as_mut() {
shielded_data.actions.first_mut().action.nullifier =
self.new_shielded_data.actions.first().action.nullifier;
} else {
*orchard_shielded_data = Some(self.new_shielded_data);
}
}
}

View File

@ -0,0 +1,96 @@
use super::{super::*, unmined_transactions_in_blocks};
use zebra_chain::parameters::Network;
use color_eyre::eyre::Result;
#[test]
fn mempool_storage_crud_mainnet() {
zebra_test::init();
let network = Network::Mainnet;
// Create an empty storage instance
let mut storage: Storage = Default::default();
// Get one (1) unmined transaction
let unmined_tx = unmined_transactions_in_blocks(.., network)
.next()
.expect("at least one unmined transaction");
// Insert unmined tx into the mempool.
let _ = storage.insert(unmined_tx.clone());
// Check that it is in the mempool, and not rejected.
assert!(storage.contains(&unmined_tx.id));
// Remove tx
let _ = storage.remove(&unmined_tx.id);
// Check that it is /not/ in the mempool.
assert!(!storage.contains(&unmined_tx.id));
}
#[test]
fn mempool_storage_basic() -> Result<()> {
zebra_test::init();
mempool_storage_basic_for_network(Network::Mainnet)?;
mempool_storage_basic_for_network(Network::Testnet)?;
Ok(())
}
fn mempool_storage_basic_for_network(network: Network) -> Result<()> {
// Create an empty storage
let mut storage: Storage = Default::default();
// Get transactions from the first 10 blocks of the Zcash blockchain
let unmined_transactions: Vec<_> = unmined_transactions_in_blocks(..=10, network).collect();
let total_transactions = unmined_transactions.len();
// Insert them all to the storage
for unmined_transaction in unmined_transactions.clone() {
storage.insert(unmined_transaction)?;
}
// Separate transactions into the ones expected to be in the mempool and those expected to be
// rejected.
let rejected_transaction_count = total_transactions - MEMPOOL_SIZE;
let expected_to_be_rejected = &unmined_transactions[..rejected_transaction_count];
let expected_in_mempool = &unmined_transactions[rejected_transaction_count..];
// Only MEMPOOL_SIZE should land in verified
assert_eq!(storage.verified.len(), MEMPOOL_SIZE);
// The rest of the transactions will be in rejected
assert_eq!(storage.rejected.len(), rejected_transaction_count);
// Make sure the last MEMPOOL_SIZE transactions we sent are in the verified
for tx in expected_in_mempool {
assert!(storage.contains(&tx.id));
}
// Anything greater should not be in the verified
for tx in expected_to_be_rejected {
assert!(!storage.contains(&tx.id));
}
// Query all the ids we have for rejected, get back `total - MEMPOOL_SIZE`
let all_ids: HashSet<UnminedTxId> = unmined_transactions.iter().map(|tx| tx.id).collect();
// Convert response to a `HashSet` as we need a fixed order to compare.
let rejected_response: HashSet<UnminedTxId> =
storage.rejected_transactions(all_ids).into_iter().collect();
let rejected_ids = expected_to_be_rejected.iter().map(|tx| tx.id).collect();
assert_eq!(rejected_response, rejected_ids);
// Use `contains_rejected` to make sure the first id stored is now rejected
assert!(storage.contains_rejected(&expected_to_be_rejected[0].id));
// Use `contains_rejected` to make sure the last id stored is not rejected
assert!(!storage.contains_rejected(&expected_in_mempool[0].id));
Ok(())
}