From 76fca5f32f53b9a874406838827611359415feb1 Mon Sep 17 00:00:00 2001 From: Janito Vaqueiro Ferreira Filho Date: Wed, 30 Jun 2021 23:17:37 -0300 Subject: [PATCH] Refactor validation of Sapling shielded data in `transaction::Verifier` (#2419) * Refactor to create `verify_sapling_shielded_data` Move the code to verify Sapling shielded data into a new helper method that returns `AsyncChecks`. * Test verifying a Sapling transaction with spends Use the test vectors to find a transaction that has Sapling spends and test if it the verifier considers it valid. * Create a helper method to list test transactions Transforms the block test vectors into a list of transactions and block heights for each transaction. * Use new helper function in V4 Sapling spend test Also use the block height for that transaction as specified in the test vector. * Test V4 tx. with Sapling outputs but no spends Find a transaction V4 vector that has Sapling outputs but no spends, and check that the verifier accepts it. --- zebra-chain/Cargo.toml | 2 +- zebra-chain/src/transaction/arbitrary.rs | 21 +++ zebra-consensus/src/transaction.rs | 209 ++++++++++++----------- zebra-consensus/src/transaction/tests.rs | 81 ++++++++- 4 files changed, 213 insertions(+), 100 deletions(-) diff --git a/zebra-chain/Cargo.toml b/zebra-chain/Cargo.toml index 7562bfb0..03a2e6ce 100644 --- a/zebra-chain/Cargo.toml +++ b/zebra-chain/Cargo.toml @@ -9,7 +9,7 @@ edition = "2018" [features] default = [] -proptest-impl = ["proptest", "proptest-derive", "itertools"] +proptest-impl = ["proptest", "proptest-derive", "itertools", "zebra-test"] bench = ["zebra-test"] [dependencies] diff --git a/zebra-chain/src/transaction/arbitrary.rs b/zebra-chain/src/transaction/arbitrary.rs index 06f976e6..d3cde508 100644 --- a/zebra-chain/src/transaction/arbitrary.rs +++ b/zebra-chain/src/transaction/arbitrary.rs @@ -567,6 +567,27 @@ fn sapling_spend_v4_to_fake_v5( } } +/// Iterate over V4 transactions in the block test vectors for the specified `network`. +pub fn test_transactions( + network: Network, +) -> impl DoubleEndedIterator)> { + let blocks = match network { + Network::Mainnet => zebra_test::vectors::MAINNET_BLOCKS.iter(), + Network::Testnet => zebra_test::vectors::TESTNET_BLOCKS.iter(), + }; + + blocks.flat_map(|(&block_height, &block_bytes)| { + let block = block_bytes + .zcash_deserialize_into::() + .expect("block is structurally valid"); + + block + .transactions + .into_iter() + .map(move |transaction| (block::Height(block_height), transaction)) + }) +} + /// Generate an iterator over fake V5 transactions. /// /// These transactions are converted from non-V5 transactions that exist in the provided network diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs index da8ab87f..7130e279 100644 --- a/zebra-consensus/src/transaction.rs +++ b/zebra-consensus/src/transaction.rs @@ -252,11 +252,6 @@ where joinsplit_data: &Option>, sapling_shielded_data: &Option>, ) -> Result { - let mut spend_verifier = primitives::groth16::SPEND_VERIFIER.clone(); - let mut output_verifier = primitives::groth16::OUTPUT_VERIFIER.clone(); - - let mut redjubjub_verifier = primitives::redjubjub::VERIFIER.clone(); - // A set of asynchronous checks which must all succeed. // We finish by waiting on these below. let mut async_checks = AsyncChecks::new(); @@ -279,99 +274,9 @@ where &shielded_sighash, )); - if let Some(sapling_shielded_data) = sapling_shielded_data { - for spend in sapling_shielded_data.spends_per_anchor() { - // Consensus rule: cv and rk MUST NOT be of small - // order, i.e. [h_J]cv MUST NOT be 𝒪_J and [h_J]rk - // MUST NOT be 𝒪_J. - // - // https://zips.z.cash/protocol/protocol.pdf#spenddesc - check::spend_cv_rk_not_small_order(&spend)?; - - // Consensus rule: The proof π_ZKSpend MUST be valid - // given a primary input formed from the other - // fields except spendAuthSig. - // - // Queue the verification of the Groth16 spend proof - // for each Spend description while adding the - // resulting future to our collection of async - // checks that (at a minimum) must pass for the - // transaction to verify. - let spend_rsp = spend_verifier - .ready_and() - .await? - .call(primitives::groth16::ItemWrapper::from(&spend).into()); - - async_checks.push(spend_rsp.boxed()); - - // Consensus rule: The spend authorization signature - // MUST be a valid SpendAuthSig signature over - // SigHash using rk as the validating key. - // - // Queue the validation of the RedJubjub spend - // authorization signature for each Spend - // description while adding the resulting future to - // our collection of async checks that (at a - // minimum) must pass for the transaction to verify. - let rsp = redjubjub_verifier - .ready_and() - .await? - .call((spend.rk, spend.spend_auth_sig, &shielded_sighash).into()); - - async_checks.push(rsp.boxed()); - } - - for output in sapling_shielded_data.outputs() { - // Consensus rule: cv and wpk MUST NOT be of small - // order, i.e. [h_J]cv MUST NOT be 𝒪_J and [h_J]wpk - // MUST NOT be 𝒪_J. - // - // https://zips.z.cash/protocol/protocol.pdf#outputdesc - check::output_cv_epk_not_small_order(output)?; - - // Consensus rule: The proof π_ZKOutput MUST be - // valid given a primary input formed from the other - // fields except C^enc and C^out. - // - // Queue the verification of the Groth16 output - // proof for each Output description while adding - // the resulting future to our collection of async - // checks that (at a minimum) must pass for the - // transaction to verify. - let output_rsp = output_verifier - .ready_and() - .await? - .call(primitives::groth16::ItemWrapper::from(output).into()); - - async_checks.push(output_rsp.boxed()); - } - - let bvk = sapling_shielded_data.binding_verification_key(); - - // TODO: enable async verification and remove this block - #1939 - { - let item: zebra_chain::primitives::redjubjub::batch::Item = - (bvk, sapling_shielded_data.binding_sig, &shielded_sighash).into(); - item.verify_single().unwrap_or_else(|binding_sig_error| { - let binding_sig_error = binding_sig_error.to_string(); - tracing::warn!(%binding_sig_error, "ignoring"); - metrics::counter!("zebra.error.sapling.binding", - 1, - "kind" => binding_sig_error); - }); - // Ignore errors until binding signatures are fixed - //.map_err(|e| BoxError::from(Box::new(e)))?; - } - - let _rsp = redjubjub_verifier - .ready_and() - .await? - .call((bvk, sapling_shielded_data.binding_sig, &shielded_sighash).into()) - .boxed(); - - // TODO: stop ignoring binding signature errors - #1939 - // async_checks.push(rsp); - } + async_checks.extend( + Self::verify_sapling_shielded_data(sapling_shielded_data, &shielded_sighash).await?, + ); Ok(async_checks) } @@ -518,6 +423,114 @@ where checks } + /// Verifies a transaction's Sapling shielded data. + async fn verify_sapling_shielded_data( + sapling_shielded_data: &Option>, + shielded_sighash: &blake2b_simd::Hash, + ) -> Result { + let async_checks = AsyncChecks::new(); + + if let Some(sapling_shielded_data) = sapling_shielded_data { + let mut spend_verifier = primitives::groth16::SPEND_VERIFIER.clone(); + let mut output_verifier = primitives::groth16::OUTPUT_VERIFIER.clone(); + let mut redjubjub_verifier = primitives::redjubjub::VERIFIER.clone(); + + for spend in sapling_shielded_data.spends_per_anchor() { + // Consensus rule: cv and rk MUST NOT be of small + // order, i.e. [h_J]cv MUST NOT be 𝒪_J and [h_J]rk + // MUST NOT be 𝒪_J. + // + // https://zips.z.cash/protocol/protocol.pdf#spenddesc + check::spend_cv_rk_not_small_order(&spend)?; + + // Consensus rule: The proof π_ZKSpend MUST be valid + // given a primary input formed from the other + // fields except spendAuthSig. + // + // Queue the verification of the Groth16 spend proof + // for each Spend description while adding the + // resulting future to our collection of async + // checks that (at a minimum) must pass for the + // transaction to verify. + let spend_rsp = spend_verifier + .ready_and() + .await? + .call(primitives::groth16::ItemWrapper::from(&spend).into()); + + async_checks.push(spend_rsp.boxed()); + + // Consensus rule: The spend authorization signature + // MUST be a valid SpendAuthSig signature over + // SigHash using rk as the validating key. + // + // Queue the validation of the RedJubjub spend + // authorization signature for each Spend + // description while adding the resulting future to + // our collection of async checks that (at a + // minimum) must pass for the transaction to verify. + let rsp = redjubjub_verifier + .ready_and() + .await? + .call((spend.rk, spend.spend_auth_sig, &shielded_sighash).into()); + + async_checks.push(rsp.boxed()); + } + + for output in sapling_shielded_data.outputs() { + // Consensus rule: cv and wpk MUST NOT be of small + // order, i.e. [h_J]cv MUST NOT be 𝒪_J and [h_J]wpk + // MUST NOT be 𝒪_J. + // + // https://zips.z.cash/protocol/protocol.pdf#outputdesc + check::output_cv_epk_not_small_order(output)?; + + // Consensus rule: The proof π_ZKOutput MUST be + // valid given a primary input formed from the other + // fields except C^enc and C^out. + // + // Queue the verification of the Groth16 output + // proof for each Output description while adding + // the resulting future to our collection of async + // checks that (at a minimum) must pass for the + // transaction to verify. + let output_rsp = output_verifier + .ready_and() + .await? + .call(primitives::groth16::ItemWrapper::from(output).into()); + + async_checks.push(output_rsp.boxed()); + } + + let bvk = sapling_shielded_data.binding_verification_key(); + + // TODO: enable async verification and remove this block - #1939 + { + let item: zebra_chain::primitives::redjubjub::batch::Item = + (bvk, sapling_shielded_data.binding_sig, &shielded_sighash).into(); + item.verify_single().unwrap_or_else(|binding_sig_error| { + let binding_sig_error = binding_sig_error.to_string(); + tracing::warn!(%binding_sig_error, "ignoring"); + metrics::counter!("zebra.error.sapling.binding", + 1, + "kind" => binding_sig_error); + }); + // Ignore errors until binding signatures are fixed + //.map_err(|e| BoxError::from(Box::new(e)))?; + } + + let _rsp = redjubjub_verifier + .ready_and() + .await? + .call((bvk, sapling_shielded_data.binding_sig, &shielded_sighash).into()) + .boxed(); + + // TODO: stop ignoring binding signature errors - #1939 + // async_checks.push(rsp); + } + + Ok(async_checks) + } + /// Await a set of checks that should all succeed. /// /// If any of the checks fail, this method immediately returns the error and cancels all other diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index 651fb4ff..631cfe73 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -10,7 +10,9 @@ use zebra_chain::{ serialization::ZcashDeserialize, sprout, transaction::{ - arbitrary::{fake_v5_transactions_for_network, insert_fake_orchard_shielded_data}, + arbitrary::{ + fake_v5_transactions_for_network, insert_fake_orchard_shielded_data, test_transactions, + }, Hash, HashType, JoinSplitData, LockTime, Transaction, }, transparent::{self, CoinbaseData}, @@ -696,6 +698,83 @@ fn v4_with_unsigned_sprout_transfer_is_rejected() { }); } +/// Test if a V4 transaction with Sapling spends is accepted by the verifier. +#[test] +fn v4_with_sapling_spends() { + zebra_test::init(); + zebra_test::RUNTIME.block_on(async { + let network = Network::Mainnet; + + let (height, transaction) = test_transactions(network) + .rev() + .filter(|(_, transaction)| { + !transaction.is_coinbase() && transaction.inputs().is_empty() + }) + .find(|(_, transaction)| transaction.sapling_spends_per_anchor().next().is_some()) + .expect("No transaction found with Sapling spends"); + + let expected_hash = transaction.hash(); + + // 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, Ok(expected_hash)); + }); +} + +/// 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() { + zebra_test::init(); + zebra_test::RUNTIME.block_on(async { + let network = Network::Mainnet; + + let (height, transaction) = test_transactions(network) + .rev() + .filter(|(_, transaction)| { + !transaction.is_coinbase() && transaction.inputs().is_empty() + }) + .find(|(_, transaction)| { + transaction.sapling_spends_per_anchor().next().is_none() + && transaction.sapling_outputs().next().is_some() + }) + .expect("No transaction found with Sapling spends"); + + let expected_hash = transaction.hash(); + + // 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, Ok(expected_hash)); + }); +} + // Utility functions /// Create a mock transparent transfer to be included in a transaction.