Reject a mempool transaction if it has internal spend conflicts (#2843)

* Reorder imports to follow convention

Place the imports from `std` at the top.

* Add transaction errors for double spends

Add a variant for each pool. They represent a double spend inside a
transaction.

* Add `check::spend_conflicts` implementation

Checks if a transaction has spend conflicts, i.e., if a transaction
spends a UTXO more than once or if it reveals a nullifier more than
once.

* Reject transactions with internal spend conflicts

The transaction verifier should reject transactions that spend the same
transparent UTXO or that reveal the same nullifier.

* Add transparent spend consensus rule

Add it to the documentation to help with understanding and auditing it.

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

* Use different nullifiers by default

Don't use the same nullifier twice when mocking a
`sprout::JoinSplitData` because it will lead to an invalid transaction.

* Test transactions with repeated spend outpoints

Since that represents a spend conflict, they should be rejected.

* Test duplicate nullifiers in joinsplit

Check if a mock transaction with a joinsplit that reveals the same
nullifier twice is rejected.

* Test duplicate nullifiers across joinsplits

Check if a duplicate nullifier in two different joinsplits in the same
transaction is rejected.

* Test V4 transaction with duplicate Sapling spend

Check if a V4 transaction that has a duplicate Sapling spend is
rejected.

* Test V5 transaction with duplicate Sapling spend

Check if a V5 transaction that has a duplicate Sapling spend is
rejected.

* Test V5 transaction with duplicate Orchard actions

Check if a V5 transaction that has duplicate Orchard actions is rejected
by the transaction verifier.

Co-authored-by: teor <teor@riseup.net>
This commit is contained in:
Janito Vaqueiro Ferreira Filho 2021-10-27 23:49:28 -03:00 committed by GitHub
parent f26a60b801
commit 36d488edb4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 524 additions and 4 deletions

View File

@ -7,6 +7,8 @@
use thiserror::Error;
use zebra_chain::{orchard, sapling, sprout, transparent};
use crate::BoxError;
#[cfg(any(test, feature = "proptest-impl"))]
@ -102,6 +104,18 @@ pub enum TransactionError {
#[error("could not calculate the transaction fee")]
IncorrectFee,
#[error("transparent double-spend: {_0:?} is spent twice")]
DuplicateTransparentSpend(transparent::OutPoint),
#[error("sprout double-spend: duplicate nullifier: {_0:?}")]
DuplicateSproutNullifier(sprout::Nullifier),
#[error("sapling double-spend: duplicate nullifier: {_0:?}")]
DuplicateSaplingNullifier(sapling::Nullifier),
#[error("orchard double-spend: duplicate nullifier: {_0:?}")]
DuplicateOrchardNullifier(orchard::Nullifier),
}
impl From<BoxError> for TransactionError {

View File

@ -277,6 +277,8 @@ where
// https://zips.z.cash/protocol/protocol.pdf#joinsplitdesc
check::disabled_add_to_sprout_pool(&tx, req.height(), network)?;
check::spend_conflicts(&tx)?;
// "The consensus rules applied to valueBalance, vShieldedOutput, and bindingSig
// in non-coinbase transactions MUST also be applied to coinbase transactions."
//

View File

@ -2,6 +2,8 @@
//!
//! Code in this file can freely assume that no pre-V4 transactions are present.
use std::{borrow::Cow, collections::HashSet, convert::TryFrom, hash::Hash};
use zebra_chain::{
amount::{Amount, NonNegative},
block::Height,
@ -13,8 +15,6 @@ use zebra_chain::{
use crate::error::TransactionError;
use std::convert::TryFrom;
/// Checks that the transaction has inputs and outputs.
///
/// For `Transaction::V4`:
@ -135,3 +135,62 @@ pub fn disabled_add_to_sprout_pool(
Ok(())
}
/// Check if a transaction has any internal spend conflicts.
///
/// An internal spend conflict happens if the transaction spends a UTXO more than once or if it
/// reveals a nullifier more than once.
///
/// Consensus rules:
///
/// "each output of a particular transaction
/// can only be used as an input once in the block chain.
/// Any subsequent reference is a forbidden double spend-
/// an attempt to spend the same satoshis twice."
///
/// https://developer.bitcoin.org/devguide/block_chain.html#introduction
///
/// A _nullifier_ *MUST NOT* repeat either within a _transaction_, or across _transactions_ in a
/// _valid blockchain_ . *Sprout* and *Sapling* and *Orchard* _nulliers_ are considered disjoint,
/// even if they have the same bit pattern.
///
/// https://zips.z.cash/protocol/protocol.pdf#nullifierset
pub fn spend_conflicts(transaction: &Transaction) -> Result<(), TransactionError> {
use crate::error::TransactionError::*;
let transparent_outpoints = transaction.spent_outpoints().map(Cow::Owned);
let sprout_nullifiers = transaction.sprout_nullifiers().map(Cow::Borrowed);
let sapling_nullifiers = transaction.sapling_nullifiers().map(Cow::Borrowed);
let orchard_nullifiers = transaction.orchard_nullifiers().map(Cow::Borrowed);
check_for_duplicates(transparent_outpoints, DuplicateTransparentSpend)?;
check_for_duplicates(sprout_nullifiers, DuplicateSproutNullifier)?;
check_for_duplicates(sapling_nullifiers, DuplicateSaplingNullifier)?;
check_for_duplicates(orchard_nullifiers, DuplicateOrchardNullifier)?;
Ok(())
}
/// Check for duplicate items in a collection.
///
/// Each item should be wrapped by a [`Cow`] instance so that this helper function can properly
/// handle borrowed items and owned items.
///
/// If a duplicate is found, an error created by the `error_wrapper` is returned.
fn check_for_duplicates<'t, T>(
items: impl IntoIterator<Item = Cow<'t, T>>,
error_wrapper: impl FnOnce(T) -> TransactionError,
) -> Result<(), TransactionError>
where
T: Clone + Eq + Hash + 't,
{
let mut hash_set = HashSet::new();
for item in items {
if let Some(duplicate) = hash_set.replace(item) {
return Err(error_wrapper(duplicate.into_owned()));
}
}
Ok(())
}

View File

@ -7,6 +7,7 @@ use zebra_chain::{
block, orchard,
parameters::{Network, NetworkUpgrade},
primitives::{ed25519, x25519, Groth16Proof},
sapling,
serialization::ZcashDeserialize,
sprout,
transaction::{
@ -428,6 +429,193 @@ async fn v4_transaction_with_transparent_transfer_is_rejected_by_the_script() {
);
}
/// Test if V4 transaction with an internal double spend of transparent funds is rejected.
#[tokio::test]
async fn v4_transaction_with_conflicting_transparent_spend_is_rejected() {
let network = Network::Mainnet;
let canopy_activation_height = NetworkUpgrade::Canopy
.activation_height(network)
.expect("Canopy activation height is specified");
let transaction_block_height =
(canopy_activation_height + 10).expect("transaction block height is too large");
let fake_source_fund_height =
(transaction_block_height - 1).expect("fake source fund block height is too small");
// Create a fake transparent transfer that should succeed
let (input, output, known_utxos) = mock_transparent_transfer(fake_source_fund_height, true);
// Create a V4 transaction
let transaction = Transaction::V4 {
inputs: vec![input.clone(), input.clone()],
outputs: vec![output],
lock_time: LockTime::Height(block::Height(0)),
expiry_height: (transaction_block_height + 1).expect("expiry height is too large"),
joinsplit_data: None,
sapling_shielded_data: None,
};
let state_service =
service_fn(|_| async { unreachable!("State service should not be called") });
let script_verifier = script::Verifier::new(state_service);
let verifier = Verifier::new(network, script_verifier);
let result = verifier
.oneshot(Request::Block {
transaction: Arc::new(transaction),
known_utxos: Arc::new(known_utxos),
height: transaction_block_height,
})
.await;
let expected_outpoint = input.outpoint().expect("Input should have an outpoint");
assert_eq!(
result,
Err(TransactionError::DuplicateTransparentSpend(
expected_outpoint
))
);
}
/// Test if V4 transaction with a joinsplit that has duplicate nullifiers is rejected.
#[test]
fn v4_transaction_with_conflicting_sprout_nullifier_inside_joinsplit_is_rejected() {
zebra_test::init();
zebra_test::RUNTIME.block_on(async {
let network = Network::Mainnet;
let network_upgrade = NetworkUpgrade::Canopy;
let canopy_activation_height = NetworkUpgrade::Canopy
.activation_height(network)
.expect("Canopy activation height is specified");
let transaction_block_height =
(canopy_activation_height + 10).expect("transaction block height is too large");
// Create a fake Sprout join split
let (mut joinsplit_data, signing_key) = mock_sprout_join_split_data();
// Make both nullifiers the same inside the joinsplit transaction
let duplicate_nullifier = joinsplit_data.first.nullifiers[0];
joinsplit_data.first.nullifiers[1] = duplicate_nullifier;
// Create a V4 transaction
let mut transaction = Transaction::V4 {
inputs: vec![],
outputs: vec![],
lock_time: LockTime::Height(block::Height(0)),
expiry_height: (transaction_block_height + 1).expect("expiry height is too large"),
joinsplit_data: Some(joinsplit_data),
sapling_shielded_data: None,
};
// Sign the transaction
let sighash = transaction.sighash(network_upgrade, HashType::ALL, None);
match &mut transaction {
Transaction::V4 {
joinsplit_data: Some(joinsplit_data),
..
} => joinsplit_data.sig = signing_key.sign(sighash.as_ref()),
_ => unreachable!("Mock transaction was created incorrectly"),
}
let state_service =
service_fn(|_| async { unreachable!("State service should not be called") });
let script_verifier = script::Verifier::new(state_service);
let verifier = Verifier::new(network, script_verifier);
let result = verifier
.oneshot(Request::Block {
transaction: Arc::new(transaction),
known_utxos: Arc::new(HashMap::new()),
height: transaction_block_height,
})
.await;
assert_eq!(
result,
Err(TransactionError::DuplicateSproutNullifier(
duplicate_nullifier
))
);
});
}
/// Test if V4 transaction with duplicate nullifiers across joinsplits is rejected.
#[test]
fn v4_transaction_with_conflicting_sprout_nullifier_across_joinsplits_is_rejected() {
zebra_test::init();
zebra_test::RUNTIME.block_on(async {
let network = Network::Mainnet;
let network_upgrade = NetworkUpgrade::Canopy;
let canopy_activation_height = NetworkUpgrade::Canopy
.activation_height(network)
.expect("Canopy activation height is specified");
let transaction_block_height =
(canopy_activation_height + 10).expect("transaction block height is too large");
// Create a fake Sprout join split
let (mut joinsplit_data, signing_key) = mock_sprout_join_split_data();
// Duplicate a nullifier from the created joinsplit
let duplicate_nullifier = joinsplit_data.first.nullifiers[1];
// Add a new joinsplit with the duplicate nullifier
let mut new_joinsplit = joinsplit_data.first.clone();
new_joinsplit.nullifiers[0] = duplicate_nullifier;
new_joinsplit.nullifiers[1] = sprout::note::Nullifier([2u8; 32]);
joinsplit_data.rest.push(new_joinsplit);
// Create a V4 transaction
let mut transaction = Transaction::V4 {
inputs: vec![],
outputs: vec![],
lock_time: LockTime::Height(block::Height(0)),
expiry_height: (transaction_block_height + 1).expect("expiry height is too large"),
joinsplit_data: Some(joinsplit_data),
sapling_shielded_data: None,
};
// Sign the transaction
let sighash = transaction.sighash(network_upgrade, HashType::ALL, None);
match &mut transaction {
Transaction::V4 {
joinsplit_data: Some(joinsplit_data),
..
} => joinsplit_data.sig = signing_key.sign(sighash.as_ref()),
_ => unreachable!("Mock transaction was created incorrectly"),
}
let state_service =
service_fn(|_| async { unreachable!("State service should not be called") });
let script_verifier = script::Verifier::new(state_service);
let verifier = Verifier::new(network, script_verifier);
let result = verifier
.oneshot(Request::Block {
transaction: Arc::new(transaction),
known_utxos: Arc::new(HashMap::new()),
height: transaction_block_height,
})
.await;
assert_eq!(
result,
Err(TransactionError::DuplicateSproutNullifier(
duplicate_nullifier
))
);
});
}
/// Test if V5 transaction with transparent funds is accepted.
#[tokio::test]
async fn v5_transaction_with_transparent_transfer_is_accepted() {
@ -583,6 +771,59 @@ async fn v5_transaction_with_transparent_transfer_is_rejected_by_the_script() {
);
}
/// Test if V5 transaction with an internal double spend of transparent funds is rejected.
#[tokio::test]
async fn v5_transaction_with_conflicting_transparent_spend_is_rejected() {
let network = Network::Mainnet;
let network_upgrade = NetworkUpgrade::Nu5;
let canopy_activation_height = NetworkUpgrade::Canopy
.activation_height(network)
.expect("Canopy activation height is specified");
let transaction_block_height =
(canopy_activation_height + 10).expect("transaction block height is too large");
let fake_source_fund_height =
(transaction_block_height - 1).expect("fake source fund block height is too small");
// Create a fake transparent transfer that should succeed
let (input, output, known_utxos) = mock_transparent_transfer(fake_source_fund_height, true);
// Create a V4 transaction
let transaction = Transaction::V5 {
inputs: vec![input.clone(), input.clone()],
outputs: vec![output],
lock_time: LockTime::Height(block::Height(0)),
expiry_height: (transaction_block_height + 1).expect("expiry height is too large"),
sapling_shielded_data: None,
orchard_shielded_data: None,
network_upgrade,
};
let state_service =
service_fn(|_| async { unreachable!("State service should not be called") });
let script_verifier = script::Verifier::new(state_service);
let verifier = Verifier::new(network, script_verifier);
let result = verifier
.oneshot(Request::Block {
transaction: Arc::new(transaction),
known_utxos: Arc::new(known_utxos),
height: transaction_block_height,
})
.await;
let expected_outpoint = input.outpoint().expect("Input should have an outpoint");
assert_eq!(
result,
Err(TransactionError::DuplicateTransparentSpend(
expected_outpoint
))
);
}
/// Test if signed V4 transaction with a dummy [`sprout::JoinSplit`] is accepted.
/// - Test if an unsigned V4 transaction with a dummy [`sprout::JoinSplit`] is rejected.
///
@ -750,6 +991,52 @@ fn v4_with_sapling_spends() {
});
}
/// Test if a V4 transaction with a duplicate Sapling spend is rejected by the verifier.
#[test]
fn v4_with_duplicate_sapling_spends() {
zebra_test::init();
zebra_test::RUNTIME.block_on(async {
let network = Network::Mainnet;
let (height, mut transaction) = test_transactions(network)
.rev()
.filter(|(_, transaction)| {
!transaction.has_valid_coinbase_transaction_inputs()
&& transaction.inputs().is_empty()
})
.find(|(_, transaction)| transaction.sapling_spends_per_anchor().next().is_some())
.expect("No transaction found with Sapling spends");
// Duplicate one of the spends
let duplicate_nullifier = duplicate_sapling_spend(
Arc::get_mut(&mut transaction).expect("Transaction only has one active reference"),
);
// Initialize the verifier
let state_service =
service_fn(|_| async { unreachable!("State service should not be called") });
let script_verifier = script::Verifier::new(state_service);
let verifier = Verifier::new(network, script_verifier);
// Test the transaction verifier
let result = verifier
.clone()
.oneshot(Request::Block {
transaction,
known_utxos: Arc::new(HashMap::new()),
height,
})
.await;
assert_eq!(
result,
Err(TransactionError::DuplicateSaplingNullifier(
duplicate_nullifier
))
);
});
}
/// Test if a V4 transaction with Sapling outputs but no spends is accepted by the verifier.
#[test]
fn v4_with_sapling_outputs_and_no_spends() {
@ -841,6 +1128,117 @@ fn v5_with_sapling_spends() {
});
}
/// Test if a V5 transaction with a duplicate Sapling spend is rejected by the verifier.
#[test]
fn v5_with_duplicate_sapling_spends() {
zebra_test::init();
zebra_test::RUNTIME.block_on(async {
let network = Network::Mainnet;
let mut transaction =
fake_v5_transactions_for_network(network, zebra_test::vectors::MAINNET_BLOCKS.iter())
.rev()
.filter(|transaction| {
!transaction.has_valid_coinbase_transaction_inputs()
&& transaction.inputs().is_empty()
})
.find(|transaction| transaction.sapling_spends_per_anchor().next().is_some())
.expect("No transaction found with Sapling spends");
let height = transaction
.expiry_height()
.expect("Transaction is missing expiry height");
// Duplicate one of the spends
let duplicate_nullifier = duplicate_sapling_spend(&mut transaction);
// Initialize the verifier
let state_service =
service_fn(|_| async { unreachable!("State service should not be called") });
let script_verifier = script::Verifier::new(state_service);
let verifier = Verifier::new(network, script_verifier);
// Test the transaction verifier
let result = verifier
.clone()
.oneshot(Request::Block {
transaction: Arc::new(transaction),
known_utxos: Arc::new(HashMap::new()),
height,
})
.await;
assert_eq!(
result,
Err(TransactionError::DuplicateSaplingNullifier(
duplicate_nullifier
))
);
});
}
/// Test if a V5 transaction with a duplicate Orchard action is rejected by the verifier.
#[test]
fn v5_with_duplicate_orchard_action() {
zebra_test::init();
zebra_test::RUNTIME.block_on(async {
let network = Network::Mainnet;
// Find a transaction with no inputs or outputs to use as base
let mut transaction =
fake_v5_transactions_for_network(network, zebra_test::vectors::MAINNET_BLOCKS.iter())
.rev()
.find(|transaction| {
transaction.inputs().is_empty()
&& transaction.outputs().is_empty()
&& transaction.sapling_spends_per_anchor().next().is_none()
&& transaction.sapling_outputs().next().is_none()
&& transaction.joinsplit_count() == 0
})
.expect("At least one fake V5 transaction with no inputs and no outputs");
let height = transaction
.expiry_height()
.expect("Transaction is missing expiry height");
// Insert fake Orchard shielded data to the transaction, which has at least one action (this is
// guaranteed structurally by `orchard::ShieldedData`)
let shielded_data = insert_fake_orchard_shielded_data(&mut transaction);
// Enable spends
shielded_data.flags = orchard::Flags::ENABLE_SPENDS | orchard::Flags::ENABLE_OUTPUTS;
// Duplicate the first action
let duplicate_action = shielded_data.actions.first().clone();
let duplicate_nullifier = duplicate_action.action.nullifier;
shielded_data.actions.push(duplicate_action);
// Initialize the verifier
let state_service =
service_fn(|_| async { unreachable!("State service should not be called") });
let script_verifier = script::Verifier::new(state_service);
let verifier = Verifier::new(network, script_verifier);
// Test the transaction verifier
let result = verifier
.clone()
.oneshot(Request::Block {
transaction: Arc::new(transaction),
known_utxos: Arc::new(HashMap::new()),
height,
})
.await;
assert_eq!(
result,
Err(TransactionError::DuplicateOrchardNullifier(
duplicate_nullifier
))
);
});
}
// Utility functions
/// Create a mock transparent transfer to be included in a transaction.
@ -958,7 +1356,8 @@ fn mock_sprout_join_split_data() -> (JoinSplitData<Groth16Proof>, ed25519::Signi
.try_into()
.expect("Invalid JoinSplit transparent input");
let anchor = sprout::tree::Root::default();
let nullifier = sprout::note::Nullifier([0u8; 32]);
let first_nullifier = sprout::note::Nullifier([0u8; 32]);
let second_nullifier = sprout::note::Nullifier([1u8; 32]);
let commitment = sprout::commitment::NoteCommitment::from([0u8; 32]);
let ephemeral_key =
x25519::PublicKey::from(&x25519::EphemeralSecret::new(rand07::thread_rng()));
@ -973,7 +1372,7 @@ fn mock_sprout_join_split_data() -> (JoinSplitData<Groth16Proof>, ed25519::Signi
vpub_old: zero_amount,
vpub_new: zero_amount,
anchor,
nullifiers: [nullifier; 2],
nullifiers: [first_nullifier, second_nullifier],
commitments: [commitment; 2],
ephemeral_key,
random_seed,
@ -997,6 +1396,52 @@ fn mock_sprout_join_split_data() -> (JoinSplitData<Groth16Proof>, ed25519::Signi
(joinsplit_data, signing_key)
}
/// Duplicate a Sapling spend inside a `transaction`.
///
/// Returns the nullifier of the duplicate spend.
///
/// # Panics
///
/// Will panic if the `transaction` does not have Sapling spends.
fn duplicate_sapling_spend(transaction: &mut Transaction) -> sapling::Nullifier {
match transaction {
Transaction::V4 {
sapling_shielded_data: Some(ref mut shielded_data),
..
} => duplicate_sapling_spend_in_shielded_data(shielded_data),
Transaction::V5 {
sapling_shielded_data: Some(ref mut shielded_data),
..
} => duplicate_sapling_spend_in_shielded_data(shielded_data),
_ => unreachable!("Transaction has no Sapling shielded data"),
}
}
/// Duplicates the first spend of the `shielded_data`.
///
/// Returns the nullifier of the duplicate spend.
///
/// # Panics
///
/// Will panic if `shielded_data` has no spends.
fn duplicate_sapling_spend_in_shielded_data<A: sapling::AnchorVariant + Clone>(
shielded_data: &mut sapling::ShieldedData<A>,
) -> sapling::Nullifier {
match shielded_data.transfers {
sapling::TransferData::SpendsAndMaybeOutputs { ref mut spends, .. } => {
let duplicate_spend = spends.first().clone();
let duplicate_nullifier = duplicate_spend.nullifier;
spends.push(duplicate_spend);
duplicate_nullifier
}
sapling::TransferData::JustOutputs { .. } => {
unreachable!("Sapling shielded data has no spends")
}
}
}
#[test]
fn add_to_sprout_pool_after_nu() {
zebra_test::init();