From 36d488edb423b3c2519584a7e7c7a032a820e785 Mon Sep 17 00:00:00 2001 From: Janito Vaqueiro Ferreira Filho Date: Wed, 27 Oct 2021 23:49:28 -0300 Subject: [PATCH] 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 * 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 --- zebra-consensus/src/error.rs | 14 + zebra-consensus/src/transaction.rs | 2 + zebra-consensus/src/transaction/check.rs | 63 +++- zebra-consensus/src/transaction/tests.rs | 449 ++++++++++++++++++++++- 4 files changed, 524 insertions(+), 4 deletions(-) diff --git a/zebra-consensus/src/error.rs b/zebra-consensus/src/error.rs index fbbe06ddc..6432902bb 100644 --- a/zebra-consensus/src/error.rs +++ b/zebra-consensus/src/error.rs @@ -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 for TransactionError { diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs index e7258d237..201b80810 100644 --- a/zebra-consensus/src/transaction.rs +++ b/zebra-consensus/src/transaction.rs @@ -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." // diff --git a/zebra-consensus/src/transaction/check.rs b/zebra-consensus/src/transaction/check.rs index 3dd667f4a..3345ea03a 100644 --- a/zebra-consensus/src/transaction/check.rs +++ b/zebra-consensus/src/transaction/check.rs @@ -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>, + 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(()) +} diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index fe2d26732..dfe7f8599 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -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, 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, 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, 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( + shielded_data: &mut sapling::ShieldedData, +) -> 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();