From db0cdb74ff3e147a7a3717ccb40cfc2ac36260bf Mon Sep 17 00:00:00 2001 From: Janito Vaqueiro Ferreira Filho Date: Tue, 1 Jun 2021 22:32:52 -0300 Subject: [PATCH] Update `has_inputs_and_outputs` to check V5 transactions (#2229) * Fix documentation comment Was missing a slash to become documentation. * Add documentation link to type reference Just to help navigation a bit. * Implement `Transaction::orchard_actions()` getter Returns an iterator to iterator over the actions in the Orchard shielded data (if there is one, otherwise it returns an empty iterator). * Add V5 support for `has_inputs_and_outputs` Checks if the transaction has Orchard actions. If it does, it is considered to have inputs and outputs. * Refactor transaction test vectors Make it easier to reuse the fake V5 transaction converter in other test vectors. * Move helper function to `zebra-chain` crate Place it together with some other helper functions, including the one that actually creates the fake V5 transaction. * Test transaction with no inputs `check::has_inputs_and_outputs` should return an error indicating that the transaction has no inputs. * Test transaction with no outputs `check::has_inputs_and_outputs` should return an error indicating that the transaction has no outputs. * Note that transaction is fake in `expect` message Should make the message easier to find, and also gives emphasis to the fact that the transaction is a fake conversion to V5. Co-authored-by: teor Co-authored-by: teor --- zebra-chain/src/transaction.rs | 25 +++++- zebra-chain/src/transaction/arbitrary.rs | 27 +++++- zebra-consensus/src/transaction/check.rs | 10 +-- zebra-consensus/src/transaction/tests.rs | 107 +++++++++++++---------- 4 files changed, 115 insertions(+), 54 deletions(-) diff --git a/zebra-chain/src/transaction.rs b/zebra-chain/src/transaction.rs index ee28f655a..91bccf7b8 100644 --- a/zebra-chain/src/transaction.rs +++ b/zebra-chain/src/transaction.rs @@ -112,7 +112,7 @@ pub enum Transaction { outputs: Vec, /// The sapling shielded data for this transaction, if any. sapling_shielded_data: Option>, - // The orchard data for this transaction, if any. + /// The orchard data for this transaction, if any. orchard_shielded_data: Option, }, } @@ -395,7 +395,28 @@ impl Transaction { // orchard - /// Access the orchard::Nullifiers in this transaction, regardless of version. + /// Iterate over the [`orchard::Action`]s in this transaction, if there are any. + pub fn orchard_actions(&self) -> Box + '_> { + match self { + // Actions + Transaction::V5 { + orchard_shielded_data: Some(orchard_shielded_data), + .. + } => Box::new(orchard_shielded_data.actions()), + + // No Actions + Transaction::V1 { .. } + | Transaction::V2 { .. } + | Transaction::V3 { .. } + | Transaction::V4 { .. } + | Transaction::V5 { + orchard_shielded_data: None, + .. + } => Box::new(std::iter::empty()), + } + } + + /// Access the [`orchard::Nullifier`]s in this transaction, regardless of version. pub fn orchard_nullifiers(&self) -> Box + '_> { // This function returns a boxed iterator because the different // transaction variants can have different iterator types diff --git a/zebra-chain/src/transaction/arbitrary.rs b/zebra-chain/src/transaction/arbitrary.rs index 430f4bf33..50fb318bb 100644 --- a/zebra-chain/src/transaction/arbitrary.rs +++ b/zebra-chain/src/transaction/arbitrary.rs @@ -11,7 +11,9 @@ use crate::{ redpallas::{Binding, Signature}, Bctv14Proof, Groth16Proof, Halo2Proof, ZkSnarkProof, }, - sapling, sprout, transparent, LedgerState, + sapling, + serialization::ZcashDeserializeInto, + sprout, transparent, LedgerState, }; use itertools::Itertools; @@ -531,3 +533,26 @@ fn sapling_spend_v4_to_fake_v5( spend_auth_sig: v4_spend.spend_auth_sig, } } + +/// Generate an iterator over fake V5 transactions. +/// +/// These transactions are converted from non-V5 transactions that exist in the provided network +/// blocks. +pub fn fake_v5_transactions_for_network<'b>( + network: Network, + blocks: impl DoubleEndedIterator + 'b, +) -> impl DoubleEndedIterator + 'b { + blocks.flat_map(move |(height, original_bytes)| { + let original_block = original_bytes + .zcash_deserialize_into::() + .expect("block is structurally valid"); + + original_block + .transactions + .into_iter() + .map(move |transaction| { + transaction_to_fake_v5(&*transaction, network, block::Height(*height)) + }) + .map(Transaction::from) + }) +} diff --git a/zebra-consensus/src/transaction/check.rs b/zebra-consensus/src/transaction/check.rs index 35705315c..f728c0e64 100644 --- a/zebra-consensus/src/transaction/check.rs +++ b/zebra-consensus/src/transaction/check.rs @@ -28,15 +28,11 @@ pub fn has_inputs_and_outputs(tx: &Transaction) -> Result<(), TransactionError> let n_joinsplit = tx.joinsplit_count(); let n_spends_sapling = tx.sapling_spends_per_anchor().count(); let n_outputs_sapling = tx.sapling_outputs().count(); + let n_actions_orchard = tx.orchard_actions().count(); - // TODO: Orchard validation (#1980) - // For `Transaction::V5`: - // * at least one of `tx_in_count`, `nSpendsSapling`, and `nActionsOrchard` MUST be non-zero. - // * at least one of `tx_out_count`, `nOutputsSapling`, and `nActionsOrchard` MUST be non-zero. - - if tx_in_count + n_spends_sapling + n_joinsplit == 0 { + if tx_in_count + n_spends_sapling + n_joinsplit + n_actions_orchard == 0 { Err(TransactionError::NoInputs) - } else if tx_out_count + n_outputs_sapling + n_joinsplit == 0 { + } else if tx_out_count + n_outputs_sapling + n_joinsplit + n_actions_orchard == 0 { Err(TransactionError::NoOutputs) } else { Ok(()) diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index 999483414..f55f443b7 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -1,59 +1,32 @@ use zebra_chain::{ - block::{Block, Height}, parameters::Network, - serialization::ZcashDeserializeInto, - transaction::{arbitrary::transaction_to_fake_v5, Transaction}, + transaction::{arbitrary::fake_v5_transactions_for_network, Transaction}, }; -use crate::error::TransactionError::*; +use super::check; + +use crate::error::TransactionError; use color_eyre::eyre::Report; #[test] fn v5_fake_transactions() -> Result<(), Report> { zebra_test::init(); - v5_fake_transactions_for_network(Network::Mainnet)?; - v5_fake_transactions_for_network(Network::Testnet)?; + let networks = vec![ + (Network::Mainnet, zebra_test::vectors::MAINNET_BLOCKS.iter()), + (Network::Testnet, zebra_test::vectors::TESTNET_BLOCKS.iter()), + ]; - Ok(()) -} - -fn v5_fake_transactions_for_network(network: Network) -> Result<(), Report> { - zebra_test::init(); - - // get all the blocks we have available - let block_iter = match network { - Network::Mainnet => zebra_test::vectors::MAINNET_BLOCKS.iter(), - Network::Testnet => zebra_test::vectors::TESTNET_BLOCKS.iter(), - }; - - for (height, original_bytes) in block_iter { - let original_block = original_bytes - .zcash_deserialize_into::() - .expect("block is structurally valid"); - - // convert all transactions from the block to V5 - let transactions: Vec = original_block - .transactions - .iter() - .map(AsRef::as_ref) - .map(|t| transaction_to_fake_v5(t, network, Height(*height))) - .map(Into::into) - .collect(); - - // after the conversion some transactions end up with no inputs nor outputs. - for transaction in transactions { - match super::check::has_inputs_and_outputs(&transaction) { - Err(e) => { - if e != NoInputs && e != NoOutputs { - panic!("error must be NoInputs or NoOutputs") - } - } + for (network, blocks) in networks { + for transaction in fake_v5_transactions_for_network(network, blocks) { + match check::has_inputs_and_outputs(&transaction) { Ok(()) => (), + Err(TransactionError::NoInputs) | Err(TransactionError::NoOutputs) => (), + Err(_) => panic!("error must be NoInputs or NoOutputs"), }; // make sure there are no joinsplits nor spends in coinbase - super::check::coinbase_tx_no_prevout_joinsplit_spend(&transaction)?; + check::coinbase_tx_no_prevout_joinsplit_spend(&transaction)?; // validate the sapling shielded data match transaction { @@ -62,13 +35,13 @@ fn v5_fake_transactions_for_network(network: Network) -> Result<(), Report> { .. } => { if let Some(s) = sapling_shielded_data { - super::check::sapling_balances_match(&s)?; + check::sapling_balances_match(&s)?; for spend in s.spends_per_anchor() { - super::check::spend_cv_rk_not_small_order(&spend)? + check::spend_cv_rk_not_small_order(&spend)? } for output in s.outputs() { - super::check::output_cv_epk_not_small_order(&output)?; + check::output_cv_epk_not_small_order(&output)?; } } } @@ -76,5 +49,51 @@ fn v5_fake_transactions_for_network(network: Network) -> Result<(), Report> { } } } + Ok(()) } + +#[test] +fn v5_transaction_with_no_inputs_fails_validation() { + let transaction = fake_v5_transactions_for_network( + Network::Mainnet, + zebra_test::vectors::MAINNET_BLOCKS.iter(), + ) + .rev() + .find(|transaction| { + transaction.inputs().is_empty() + && transaction.sapling_spends_per_anchor().next().is_none() + && transaction.orchard_actions().next().is_none() + && transaction.joinsplit_count() == 0 + && (!transaction.outputs().is_empty() || transaction.sapling_outputs().next().is_some()) + }) + .expect("At least one fake v5 transaction with no inputs in the test vectors"); + + assert_eq!( + check::has_inputs_and_outputs(&transaction), + Err(TransactionError::NoInputs) + ); +} + +#[test] +fn v5_transaction_with_no_outputs_fails_validation() { + let transaction = fake_v5_transactions_for_network( + Network::Mainnet, + zebra_test::vectors::MAINNET_BLOCKS.iter(), + ) + .rev() + .find(|transaction| { + transaction.outputs().is_empty() + && transaction.sapling_outputs().next().is_none() + && transaction.orchard_actions().next().is_none() + && transaction.joinsplit_count() == 0 + && (!transaction.inputs().is_empty() + || transaction.sapling_spends_per_anchor().next().is_some()) + }) + .expect("At least one fake v5 transaction with no outputs in the test vectors"); + + assert_eq!( + check::has_inputs_and_outputs(&transaction), + Err(TransactionError::NoOutputs) + ); +}