From 75d29aca249fda0d24fcef09a99c5a858da7e0fd Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 27 Apr 2021 21:43:00 -0300 Subject: [PATCH] Add V5 transparent and sapling to transaction::check, add missing coinbase PrevOut check (#2070) * validate sapling v5 tx * Make itertools dependency optional We only need itertools when the `proptest-impl` feature is enabled. * Check if V4 and V5 coinbase transactions contain PrevOut transparent inputs This is a bugfix on V4 transaction validation. The PrevOut consensus rule was not explicitly stated in the Zcash spec until April 2021. (But it was implied by Bitcoin, and partially implemented by Zebra.) Also do the shielded sapling input check for V5 transactions. * Add spec and orchard TODOs to has_inputs_and_outputs Also make the variable names match the spec. * Sort transaction functions to match v5 data order * Simplify transaction input and output checks Move counts or iterators into `Transaction` methods, so we can remove duplicate code, and make the consensus rule logic clearer. * Update sapling_balances_match for Transaction v5 - Quote from the spec - Explain why the function is redunant for v5 - Rename the function so it's clear that it is sapling-specific Co-authored-by: teor --- zebra-chain/Cargo.toml | 6 +- zebra-chain/src/transaction.rs | 233 ++++++++++++++----- zebra-chain/src/transaction/arbitrary.rs | 129 ++++++++++ zebra-chain/src/transaction/tests/vectors.rs | 132 +---------- zebra-consensus/Cargo.toml | 1 + zebra-consensus/src/block/check.rs | 2 +- zebra-consensus/src/block/tests.rs | 2 +- zebra-consensus/src/error.rs | 5 +- zebra-consensus/src/transaction.rs | 6 +- zebra-consensus/src/transaction/check.rs | 134 +++++------ zebra-consensus/src/transaction/tests.rs | 65 ++++++ 11 files changed, 442 insertions(+), 273 deletions(-) create mode 100644 zebra-consensus/src/transaction/tests.rs diff --git a/zebra-chain/Cargo.toml b/zebra-chain/Cargo.toml index ea72120beb9..c281ada144c 100644 --- a/zebra-chain/Cargo.toml +++ b/zebra-chain/Cargo.toml @@ -10,7 +10,7 @@ edition = "2018" [features] default = [] -proptest-impl = ["proptest", "proptest-derive"] +proptest-impl = ["proptest", "proptest-derive", "itertools"] bench = ["zebra-test"] [dependencies] @@ -40,6 +40,7 @@ x25519-dalek = { version = "1.1", features = ["serde"] } proptest = { version = "0.10", optional = true } proptest-derive = { version = "0.3.0", optional = true } +itertools = { version = "0.10.0", optional = true } # ZF deps ed25519-zebra = "2.2.0" @@ -55,10 +56,9 @@ criterion = { version = "0.3", features = ["html_reports"] } spandoc = "0.2" tracing = "0.1.25" -itertools = "0.10.0" - proptest = "0.10" proptest-derive = "0.3" +itertools = "0.10.0" zebra-test = { path = "../zebra-test/" } diff --git a/zebra-chain/src/transaction.rs b/zebra-chain/src/transaction.rs index 929563fd74a..6b8b1042ddc 100644 --- a/zebra-chain/src/transaction.rs +++ b/zebra-chain/src/transaction.rs @@ -10,7 +10,7 @@ mod serialize; mod sighash; #[cfg(any(test, feature = "proptest-impl"))] -mod arbitrary; +pub mod arbitrary; #[cfg(test)] mod tests; @@ -113,11 +113,63 @@ pub enum Transaction { } impl Transaction { - /// Compute the hash of this transaction. + // hashes + + /// Compute the hash (id) of this transaction. pub fn hash(&self) -> Hash { Hash::from(self) } + /// Calculate the sighash for the current transaction + /// + /// # Details + /// + /// The `input` argument indicates the transparent Input for which we are + /// producing a sighash. It is comprised of the index identifying the + /// transparent::Input within the transaction and the transparent::Output + /// representing the UTXO being spent by that input. + /// + /// # Panics + /// + /// - if passed in any NetworkUpgrade from before NetworkUpgrade::Overwinter + /// - if called on a v1 or v2 transaction + /// - if the input index points to a transparent::Input::CoinBase + /// - if the input index is out of bounds for self.inputs() + pub fn sighash( + &self, + network_upgrade: NetworkUpgrade, + hash_type: sighash::HashType, + input: Option<(u32, transparent::Output)>, + ) -> blake2b_simd::Hash { + sighash::SigHasher::new(self, hash_type, network_upgrade, input).sighash() + } + + // header + + /// Get this transaction's lock time. + pub fn lock_time(&self) -> LockTime { + match self { + Transaction::V1 { lock_time, .. } => *lock_time, + Transaction::V2 { lock_time, .. } => *lock_time, + Transaction::V3 { lock_time, .. } => *lock_time, + Transaction::V4 { lock_time, .. } => *lock_time, + Transaction::V5 { lock_time, .. } => *lock_time, + } + } + + /// Get this transaction's expiry height, if any. + pub fn expiry_height(&self) -> Option { + match self { + Transaction::V1 { .. } => None, + Transaction::V2 { .. } => None, + Transaction::V3 { expiry_height, .. } => Some(*expiry_height), + Transaction::V4 { expiry_height, .. } => Some(*expiry_height), + Transaction::V5 { expiry_height, .. } => Some(*expiry_height), + } + } + + // transparent + /// Access the transparent inputs of this transaction, regardless of version. pub fn inputs(&self) -> &[transparent::Input] { match self { @@ -140,25 +192,65 @@ impl Transaction { } } - /// Get this transaction's lock time. - pub fn lock_time(&self) -> LockTime { - match self { - Transaction::V1 { lock_time, .. } => *lock_time, - Transaction::V2 { lock_time, .. } => *lock_time, - Transaction::V3 { lock_time, .. } => *lock_time, - Transaction::V4 { lock_time, .. } => *lock_time, - Transaction::V5 { lock_time, .. } => *lock_time, - } + /// Returns `true` if this transaction is a coinbase transaction. + pub fn is_coinbase(&self) -> bool { + self.inputs().len() == 1 + && matches!( + self.inputs().get(0), + Some(transparent::Input::Coinbase { .. }) + ) } - /// Get this transaction's expiry height, if any. - pub fn expiry_height(&self) -> Option { + /// Returns `true` if transaction contains any coinbase inputs. + pub fn contains_coinbase_input(&self) -> bool { + self.inputs() + .iter() + .any(|input| matches!(input, transparent::Input::Coinbase { .. })) + } + + /// Returns `true` if transaction contains any `PrevOut` inputs. + /// + /// `PrevOut` inputs are also known as `transparent` inputs in the spec. + pub fn contains_prevout_input(&self) -> bool { + self.inputs() + .iter() + .any(|input| matches!(input, transparent::Input::PrevOut { .. })) + } + + // sprout + + /// Returns the number of `JoinSplit`s in this transaction, regardless of version. + pub fn joinsplit_count(&self) -> usize { match self { - Transaction::V1 { .. } => None, - Transaction::V2 { .. } => None, - Transaction::V3 { expiry_height, .. } => Some(*expiry_height), - Transaction::V4 { expiry_height, .. } => Some(*expiry_height), - Transaction::V5 { expiry_height, .. } => Some(*expiry_height), + // JoinSplits with Bctv14 Proofs + Transaction::V2 { + joinsplit_data: Some(joinsplit_data), + .. + } + | Transaction::V3 { + joinsplit_data: Some(joinsplit_data), + .. + } => joinsplit_data.joinsplits().count(), + // JoinSplits with Groth Proofs + Transaction::V4 { + joinsplit_data: Some(joinsplit_data), + .. + } => joinsplit_data.joinsplits().count(), + // No JoinSplits + Transaction::V1 { .. } + | Transaction::V2 { + joinsplit_data: None, + .. + } + | Transaction::V3 { + joinsplit_data: None, + .. + } + | Transaction::V4 { + joinsplit_data: None, + .. + } + | Transaction::V5 { .. } => 0, } } @@ -201,6 +293,71 @@ impl Transaction { } } + // sapling + + /// Iterate over the sapling [`Spend`](sapling::Spend)s for this transaction, + /// returning `Spend` regardless of the underlying + /// transaction version. + /// + /// # Correctness + /// + /// Do not use this function for serialization. + pub fn sapling_spends_per_anchor( + &self, + ) -> Box> + '_> { + match self { + Transaction::V4 { + sapling_shielded_data: Some(sapling_shielded_data), + .. + } => Box::new(sapling_shielded_data.spends_per_anchor()), + Transaction::V5 { + sapling_shielded_data: Some(sapling_shielded_data), + .. + } => Box::new(sapling_shielded_data.spends_per_anchor()), + + // No Spends + Transaction::V1 { .. } + | Transaction::V2 { .. } + | Transaction::V3 { .. } + | Transaction::V4 { + sapling_shielded_data: None, + .. + } + | Transaction::V5 { + sapling_shielded_data: None, + .. + } => Box::new(std::iter::empty()), + } + } + + /// Iterate over the sapling [`Output`](sapling::Output)s for this + /// transaction + pub fn sapling_outputs(&self) -> Box + '_> { + match self { + Transaction::V4 { + sapling_shielded_data: Some(sapling_shielded_data), + .. + } => Box::new(sapling_shielded_data.outputs()), + Transaction::V5 { + sapling_shielded_data: Some(sapling_shielded_data), + .. + } => Box::new(sapling_shielded_data.outputs()), + + // No Outputs + Transaction::V1 { .. } + | Transaction::V2 { .. } + | Transaction::V3 { .. } + | Transaction::V4 { + sapling_shielded_data: None, + .. + } + | Transaction::V5 { + sapling_shielded_data: None, + .. + } => Box::new(std::iter::empty()), + } + } + /// Access the sapling::Nullifiers in this transaction, regardless of version. pub fn sapling_nullifiers(&self) -> Box + '_> { // This function returns a boxed iterator because the different @@ -231,43 +388,5 @@ impl Transaction { } } - /// Returns `true` if transaction contains any coinbase inputs. - pub fn contains_coinbase_input(&self) -> bool { - self.inputs() - .iter() - .any(|input| matches!(input, transparent::Input::Coinbase { .. })) - } - - /// Returns `true` if this transaction is a coinbase transaction. - pub fn is_coinbase(&self) -> bool { - self.inputs().len() == 1 - && matches!( - self.inputs().get(0), - Some(transparent::Input::Coinbase { .. }) - ) - } - - /// Calculate the sighash for the current transaction - /// - /// # Details - /// - /// The `input` argument indicates the transparent Input for which we are - /// producing a sighash. It is comprised of the index identifying the - /// transparent::Input within the transaction and the transparent::Output - /// representing the UTXO being spent by that input. - /// - /// # Panics - /// - /// - if passed in any NetworkUpgrade from before NetworkUpgrade::Overwinter - /// - if called on a v1 or v2 transaction - /// - if the input index points to a transparent::Input::CoinBase - /// - if the input index is out of bounds for self.inputs() - pub fn sighash( - &self, - network_upgrade: NetworkUpgrade, - hash_type: sighash::HashType, - input: Option<(u32, transparent::Output)>, - ) -> blake2b_simd::Hash { - sighash::SigHasher::new(self, hash_type, network_upgrade, input).sighash() - } + // TODO: orchard } diff --git a/zebra-chain/src/transaction/arbitrary.rs b/zebra-chain/src/transaction/arbitrary.rs index 623de13d787..e5a37b80581 100644 --- a/zebra-chain/src/transaction/arbitrary.rs +++ b/zebra-chain/src/transaction/arbitrary.rs @@ -11,6 +11,8 @@ use crate::{ sapling, sprout, transparent, LedgerState, }; +use itertools::Itertools; + use super::{FieldNotPresent, JoinSplitData, LockTime, Memo, Transaction}; use sapling::{AnchorVariant, PerSpendAnchor, SharedAnchor}; @@ -321,3 +323,130 @@ impl Arbitrary for Transaction { type Strategy = BoxedStrategy; } + +/// Transaction utility tests functions + +/// Convert `trans` into a fake v5 transaction, +/// converting sapling shielded data from v4 to v5 if possible. +pub fn transaction_to_fake_v5(trans: &Transaction) -> Transaction { + use Transaction::*; + + match trans { + V1 { + inputs, + outputs, + lock_time, + } => V5 { + inputs: inputs.to_vec(), + outputs: outputs.to_vec(), + lock_time: *lock_time, + expiry_height: block::Height(0), + sapling_shielded_data: None, + }, + V2 { + inputs, + outputs, + lock_time, + .. + } => V5 { + inputs: inputs.to_vec(), + outputs: outputs.to_vec(), + lock_time: *lock_time, + expiry_height: block::Height(0), + sapling_shielded_data: None, + }, + V3 { + inputs, + outputs, + lock_time, + expiry_height, + .. + } => V5 { + inputs: inputs.to_vec(), + outputs: outputs.to_vec(), + lock_time: *lock_time, + expiry_height: *expiry_height, + sapling_shielded_data: None, + }, + V4 { + inputs, + outputs, + lock_time, + expiry_height, + sapling_shielded_data, + .. + } => V5 { + inputs: inputs.to_vec(), + outputs: outputs.to_vec(), + lock_time: *lock_time, + expiry_height: *expiry_height, + sapling_shielded_data: sapling_shielded_data + .clone() + .map(sapling_shielded_v4_to_fake_v5) + .flatten(), + }, + v5 @ V5 { .. } => v5.clone(), + } +} + +/// Convert a v4 sapling shielded data into a fake v5 sapling shielded data, +/// if possible. +fn sapling_shielded_v4_to_fake_v5( + v4_shielded: sapling::ShieldedData, +) -> Option> { + use sapling::ShieldedData; + use sapling::TransferData::*; + + let unique_anchors: Vec<_> = v4_shielded + .spends() + .map(|spend| spend.per_spend_anchor) + .unique() + .collect(); + + let fake_spends: Vec<_> = v4_shielded + .spends() + .cloned() + .map(sapling_spend_v4_to_fake_v5) + .collect(); + + let transfers = match v4_shielded.transfers { + SpendsAndMaybeOutputs { maybe_outputs, .. } => { + let shared_anchor = match unique_anchors.as_slice() { + [unique_anchor] => *unique_anchor, + // Multiple different anchors, can't convert to v5 + _ => return None, + }; + + SpendsAndMaybeOutputs { + shared_anchor, + spends: fake_spends.try_into().unwrap(), + maybe_outputs, + } + } + JustOutputs { outputs } => JustOutputs { outputs }, + }; + + let fake_shielded_v5 = ShieldedData:: { + value_balance: v4_shielded.value_balance, + transfers, + binding_sig: v4_shielded.binding_sig, + }; + + Some(fake_shielded_v5) +} + +/// Convert a v4 sapling spend into a fake v5 sapling spend. +fn sapling_spend_v4_to_fake_v5( + v4_spend: sapling::Spend, +) -> sapling::Spend { + use sapling::Spend; + + Spend:: { + cv: v4_spend.cv, + per_spend_anchor: FieldNotPresent, + nullifier: v4_spend.nullifier, + rk: v4_spend.rk, + zkproof: v4_spend.zkproof, + spend_auth_sig: v4_spend.spend_auth_sig, + } +} diff --git a/zebra-chain/src/transaction/tests/vectors.rs b/zebra-chain/src/transaction/tests/vectors.rs index 1dc154a441f..90ade53361d 100644 --- a/zebra-chain/src/transaction/tests/vectors.rs +++ b/zebra-chain/src/transaction/tests/vectors.rs @@ -2,12 +2,9 @@ use super::super::*; use crate::{ block::{Block, MAX_BLOCK_BYTES}, - sapling::{PerSpendAnchor, SharedAnchor}, serialization::{ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize}, }; -use itertools::Itertools; - use std::convert::TryInto; #[test] @@ -186,7 +183,7 @@ fn fake_v5_round_trip() { .transactions .iter() .map(AsRef::as_ref) - .map(transaction_to_fake_v5) + .map(arbitrary::transaction_to_fake_v5) .map(Into::into) .collect(); @@ -264,130 +261,3 @@ fn fake_v5_round_trip() { ); } } - -// Utility functions - -/// Convert `trans` into a fake v5 transaction, -/// converting sapling shielded data from v4 to v5 if possible. -fn transaction_to_fake_v5(trans: &Transaction) -> Transaction { - use Transaction::*; - - match trans { - V1 { - inputs, - outputs, - lock_time, - } => V5 { - inputs: inputs.to_vec(), - outputs: outputs.to_vec(), - lock_time: *lock_time, - expiry_height: block::Height(0), - sapling_shielded_data: None, - }, - V2 { - inputs, - outputs, - lock_time, - .. - } => V5 { - inputs: inputs.to_vec(), - outputs: outputs.to_vec(), - lock_time: *lock_time, - expiry_height: block::Height(0), - sapling_shielded_data: None, - }, - V3 { - inputs, - outputs, - lock_time, - expiry_height, - .. - } => V5 { - inputs: inputs.to_vec(), - outputs: outputs.to_vec(), - lock_time: *lock_time, - expiry_height: *expiry_height, - sapling_shielded_data: None, - }, - V4 { - inputs, - outputs, - lock_time, - expiry_height, - sapling_shielded_data, - .. - } => V5 { - inputs: inputs.to_vec(), - outputs: outputs.to_vec(), - lock_time: *lock_time, - expiry_height: *expiry_height, - sapling_shielded_data: sapling_shielded_data - .clone() - .map(sapling_shielded_v4_to_fake_v5) - .flatten(), - }, - v5 @ V5 { .. } => v5.clone(), - } -} - -/// Convert a v4 sapling shielded data into a fake v5 sapling shielded data, -/// if possible. -fn sapling_shielded_v4_to_fake_v5( - v4_shielded: sapling::ShieldedData, -) -> Option> { - use sapling::ShieldedData; - use sapling::TransferData::*; - - let unique_anchors: Vec<_> = v4_shielded - .spends() - .map(|spend| spend.per_spend_anchor) - .unique() - .collect(); - - let fake_spends: Vec<_> = v4_shielded - .spends() - .cloned() - .map(sapling_spend_v4_to_fake_v5) - .collect(); - - let transfers = match v4_shielded.transfers { - SpendsAndMaybeOutputs { maybe_outputs, .. } => { - let shared_anchor = match unique_anchors.as_slice() { - [unique_anchor] => *unique_anchor, - // Multiple different anchors, can't convert to v5 - _ => return None, - }; - - SpendsAndMaybeOutputs { - shared_anchor, - spends: fake_spends.try_into().unwrap(), - maybe_outputs, - } - } - JustOutputs { outputs } => JustOutputs { outputs }, - }; - - let fake_shielded_v5 = ShieldedData:: { - value_balance: v4_shielded.value_balance, - transfers, - binding_sig: v4_shielded.binding_sig, - }; - - Some(fake_shielded_v5) -} - -/// Convert a v4 sapling spend into a fake v5 sapling spend. -fn sapling_spend_v4_to_fake_v5( - v4_spend: sapling::Spend, -) -> sapling::Spend { - use sapling::Spend; - - Spend:: { - cv: v4_spend.cv, - per_spend_anchor: FieldNotPresent, - nullifier: v4_spend.nullifier, - rk: v4_spend.rk, - zkproof: v4_spend.zkproof, - spend_auth_sig: v4_spend.spend_auth_sig, - } -} diff --git a/zebra-consensus/Cargo.toml b/zebra-consensus/Cargo.toml index e0022b49e34..96878d68f59 100644 --- a/zebra-consensus/Cargo.toml +++ b/zebra-consensus/Cargo.toml @@ -42,4 +42,5 @@ tokio = { version = "0.3.6", features = ["full"] } tracing-error = "0.1.2" tracing-subscriber = "0.2.17" +zebra-chain = { path = "../zebra-chain", features = ["proptest-impl"] } zebra-test = { path = "../zebra-test/" } diff --git a/zebra-consensus/src/block/check.rs b/zebra-consensus/src/block/check.rs index 6e7e1a1fd66..f62afb2ca11 100644 --- a/zebra-consensus/src/block/check.rs +++ b/zebra-consensus/src/block/check.rs @@ -31,7 +31,7 @@ pub fn coinbase_is_first(block: &Block) -> Result<(), BlockError> { return Err(TransactionError::CoinbasePosition)?; } if rest.any(|tx| tx.contains_coinbase_input()) { - return Err(TransactionError::CoinbaseInputFound)?; + return Err(TransactionError::CoinbaseAfterFirst)?; } Ok(()) diff --git a/zebra-consensus/src/block/tests.rs b/zebra-consensus/src/block/tests.rs index 2c1ceb56c35..45359a57d86 100644 --- a/zebra-consensus/src/block/tests.rs +++ b/zebra-consensus/src/block/tests.rs @@ -354,7 +354,7 @@ fn coinbase_validation_failure() -> Result<(), Report> { // Validate the block using coinbase_is_first let result = check::coinbase_is_first(&block).unwrap_err(); - let expected = BlockError::Transaction(TransactionError::CoinbaseInputFound); + let expected = BlockError::Transaction(TransactionError::CoinbaseAfterFirst); assert_eq!(expected, result); // Validate the block using subsidy_is_valid, which does not detect this error diff --git a/zebra-consensus/src/error.rs b/zebra-consensus/src/error.rs index 48cd020720e..d4e0d6a636a 100644 --- a/zebra-consensus/src/error.rs +++ b/zebra-consensus/src/error.rs @@ -24,7 +24,10 @@ pub enum TransactionError { CoinbasePosition, #[error("coinbase input found in non-coinbase transaction")] - CoinbaseInputFound, + CoinbaseAfterFirst, + + #[error("coinbase transaction MUST NOT have any transparent (PrevOut) inputs")] + CoinbaseHasPrevOutInput, #[error("coinbase transaction MUST NOT have any JoinSplit descriptions")] CoinbaseHasJoinSplit, diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs index 7d8da38f372..cd3a1b024f9 100644 --- a/zebra-consensus/src/transaction.rs +++ b/zebra-consensus/src/transaction.rs @@ -26,6 +26,8 @@ use zebra_state as zs; use crate::{error::TransactionError, primitives, script, BoxError}; mod check; +#[cfg(test)] +mod tests; /// Asynchronous transaction verification. #[derive(Debug, Clone)] @@ -160,7 +162,7 @@ where // Handle transparent inputs and outputs. if tx.is_coinbase() { - check::coinbase_tx_no_joinsplit_or_spend(&tx)?; + check::coinbase_tx_no_prevout_joinsplit_spend(&tx)?; } else { // feed all of the inputs to the script and shielded verifiers // the script_verifier also checks transparent sighashes, using its own implementation @@ -213,7 +215,7 @@ where } if let Some(sapling_shielded_data) = sapling_shielded_data { - check::shielded_balances_match(&sapling_shielded_data)?; + check::sapling_balances_match(&sapling_shielded_data)?; for spend in sapling_shielded_data.spends_per_anchor() { // Consensus rule: cv and rk MUST NOT be of small diff --git a/zebra-consensus/src/transaction/check.rs b/zebra-consensus/src/transaction/check.rs index 8f72f443fe5..35705315ca4 100644 --- a/zebra-consensus/src/transaction/check.rs +++ b/zebra-consensus/src/transaction/check.rs @@ -11,67 +11,56 @@ use crate::error::TransactionError; /// Checks that the transaction has inputs and outputs. /// -/// More specifically: +/// For `Transaction::V4`: +/// * at least one of `tx_in_count`, `nSpendsSapling`, and `nJoinSplit` MUST be non-zero. +/// * at least one of `tx_out_count`, `nOutputsSapling`, and `nJoinSplit` MUST be non-zero. /// -/// * at least one of tx_in_count, nShieldedSpend, and nJoinSplit MUST be non-zero. -/// * at least one of tx_out_count, nShieldedOutput, and nJoinSplit MUST be non-zero. +/// 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. +/// +/// This check counts both `Coinbase` and `PrevOut` transparent inputs. /// /// https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus pub fn has_inputs_and_outputs(tx: &Transaction) -> Result<(), TransactionError> { - // The consensus rule is written in terms of numbers, but our transactions - // hold enum'd data. Mixing pattern matching and numerical checks is risky, - // so convert everything to counts and sum up. - match tx { - Transaction::V4 { - inputs, - outputs, - joinsplit_data, - sapling_shielded_data, - .. - } => { - let tx_in_count = inputs.len(); - let tx_out_count = outputs.len(); - let n_joinsplit = joinsplit_data - .as_ref() - .map(|d| d.joinsplits().count()) - .unwrap_or(0); - let n_shielded_spend = sapling_shielded_data - .as_ref() - .map(|d| d.spends().count()) - .unwrap_or(0); - let n_shielded_output = sapling_shielded_data - .as_ref() - .map(|d| d.outputs().count()) - .unwrap_or(0); + let tx_in_count = tx.inputs().len(); + let tx_out_count = tx.outputs().len(); + let n_joinsplit = tx.joinsplit_count(); + let n_spends_sapling = tx.sapling_spends_per_anchor().count(); + let n_outputs_sapling = tx.sapling_outputs().count(); - if tx_in_count + n_shielded_spend + n_joinsplit == 0 { - Err(TransactionError::NoInputs) - } else if tx_out_count + n_shielded_output + n_joinsplit == 0 { - Err(TransactionError::NoOutputs) - } else { - Ok(()) - } - } - Transaction::V1 { .. } | Transaction::V2 { .. } | Transaction::V3 { .. } => { - unreachable!("tx version is checked first") - } - Transaction::V5 { .. } => { - unimplemented!("v5 transaction format as specified in ZIP-225") - } + // 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 { + Err(TransactionError::NoInputs) + } else if tx_out_count + n_outputs_sapling + n_joinsplit == 0 { + Err(TransactionError::NoOutputs) + } else { + Ok(()) } } -/// Check that if there are no Spends or Outputs, that valueBalance is also 0. +/// Check that if there are no Spends or Outputs, the Sapling valueBalance is also 0. +/// +/// If effectiveVersion = 4 and there are no Spend descriptions or Output descriptions, +/// then valueBalanceSapling MUST be 0. +/// +/// This check is redundant for `Transaction::V5`, because the transaction format +/// omits `valueBalanceSapling` when there are no spends and no outputs. But it's +/// simpler to just do the redundant check anyway. /// -/// https://zips.z.cash/protocol/protocol.pdf#consensusfrombitcoin -pub fn shielded_balances_match( - shielded_data: &ShieldedData, +/// https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus +pub fn sapling_balances_match( + sapling_shielded_data: &ShieldedData, ) -> Result<(), TransactionError> where AnchorV: AnchorVariant + Clone, { - if (shielded_data.spends().count() + shielded_data.outputs().count() != 0) - || i64::from(shielded_data.value_balance) == 0 + if (sapling_shielded_data.spends().count() + sapling_shielded_data.outputs().count() != 0) + || i64::from(sapling_shielded_data.value_balance) == 0 { Ok(()) } else { @@ -79,40 +68,31 @@ where } } -/// Check that a coinbase tx does not have any JoinSplit or Spend descriptions. +/// Check that a coinbase transaction has no PrevOut inputs, JoinSplits, or spends. +/// +/// A coinbase transaction MUST NOT have any transparent inputs, JoinSplit descriptions, +/// or Spend descriptions. +/// +/// In a version 5 coinbase transaction, the enableSpendsOrchard flag MUST be 0. +/// +/// This check only counts `PrevOut` transparent inputs. /// /// https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus -pub fn coinbase_tx_no_joinsplit_or_spend(tx: &Transaction) -> Result<(), TransactionError> { +pub fn coinbase_tx_no_prevout_joinsplit_spend(tx: &Transaction) -> Result<(), TransactionError> { if tx.is_coinbase() { - match tx { - // Check if there is any JoinSplitData. - Transaction::V4 { - joinsplit_data: Some(_), - .. - } => Err(TransactionError::CoinbaseHasJoinSplit), - - // The ShieldedData contains both Spends and Outputs, and Outputs - // are allowed post-Heartwood, so we have to count Spends. - Transaction::V4 { - sapling_shielded_data: Some(sapling_shielded_data), - .. - } if sapling_shielded_data.spends().count() > 0 => { - Err(TransactionError::CoinbaseHasSpend) - } - - Transaction::V4 { .. } => Ok(()), - - Transaction::V1 { .. } | Transaction::V2 { .. } | Transaction::V3 { .. } => { - unreachable!("tx version is checked first") - } - - Transaction::V5 { .. } => { - unimplemented!("v5 coinbase validation as specified in ZIP-225 and the draft spec") - } + if tx.contains_prevout_input() { + return Err(TransactionError::CoinbaseHasPrevOutInput); + } else if tx.joinsplit_count() > 0 { + return Err(TransactionError::CoinbaseHasJoinSplit); + } else if tx.sapling_spends_per_anchor().count() > 0 { + return Err(TransactionError::CoinbaseHasSpend); } - } else { - Ok(()) + + // TODO: Orchard validation (#1980) + // In a version 5 coinbase transaction, the enableSpendsOrchard flag MUST be 0. } + + Ok(()) } /// Check that a Spend description's cv and rk are not of small order, diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs new file mode 100644 index 00000000000..3481e5fa066 --- /dev/null +++ b/zebra-consensus/src/transaction/tests.rs @@ -0,0 +1,65 @@ +use zebra_chain::{ + block::Block, + serialization::ZcashDeserializeInto, + transaction::{arbitrary::transaction_to_fake_v5, Transaction}, +}; + +use crate::error::TransactionError::*; +use color_eyre::eyre::Report; + +#[test] +fn v5_fake_transactions() -> Result<(), Report> { + zebra_test::init(); + + // get all the blocks we have available + for original_bytes in zebra_test::vectors::BLOCKS.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(transaction_to_fake_v5) + .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") + } + } + Ok(()) => (), + }; + + // make sure there are no joinsplits nor spends in coinbase + super::check::coinbase_tx_no_prevout_joinsplit_spend(&transaction)?; + + // validate the sapling shielded data + match transaction { + Transaction::V5 { + sapling_shielded_data, + .. + } => { + if let Some(s) = sapling_shielded_data { + super::check::sapling_balances_match(&s)?; + + for spend in s.spends_per_anchor() { + super::check::spend_cv_rk_not_small_order(&spend)? + } + for output in s.outputs() { + super::check::output_cv_epk_not_small_order(&output)?; + } + } + } + _ => panic!("we should have no tx other than 5"), + } + } + } + Ok(()) +}