From f5bc5279cac645646212da978384678306b34fe6 Mon Sep 17 00:00:00 2001 From: Janito Vaqueiro Ferreira Filho Date: Fri, 2 Jul 2021 13:48:53 -0300 Subject: [PATCH] Validate V5 transactions with Sapling shielded data (#2437) * Make `verify_sapling_shielded_data` more generic Prepare to support V5 transactions which have a shared anchor. * Verify Sapling shielded data in V5 transactions Call the `verify_sapling_shielded_data` method and add the respective asynchronous checks to the set of V5 checks. * Fix expect message in V4 transaction test It was using the same message as the previous test, even though the test searches with different criteria. * Test V5 transaction with Sapling spends Create a fake V5 transaction that has Sapling spends and check that the verifier accepts the transaction. * Ignore rejected V5 transaction test for now Because now it needs the `sighash` implementation for V5 to be ready. * Reference V5 `sighash` PR in comment So that it is easier to check if it's possible to remove the `should_panic` or not. Co-authored-by: Alfredo Garcia * Remove `sapling shielded pool` TODO V5 transactions now have Sapling shielded pool properly validated. * Link to some extra issues in TODO comment Some other issues are also necessary for full V5 validation. * Add a TODO in the main code to fix the tests Some tests are blocked due to missing features required for full V5 validation. Once those features are implemented, they should be updated to remove the `#[should_panic]` attribute so that they actually run and check the code correctly. Co-authored-by: Alfredo Garcia --- zebra-consensus/src/transaction.rs | 44 ++++++++++++++++------- zebra-consensus/src/transaction/tests.rs | 45 +++++++++++++++++++++++- 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs index f421c704e41..5b3f69ed316 100644 --- a/zebra-consensus/src/transaction.rs +++ b/zebra-consensus/src/transaction.rs @@ -206,9 +206,17 @@ where joinsplit_data, sapling_shielded_data, )?, - Transaction::V5 { inputs, .. } => { - Self::verify_v5_transaction(req, network, script_verifier, inputs)? - } + Transaction::V5 { + inputs, + sapling_shielded_data, + .. + } => Self::verify_v5_transaction( + req, + network, + script_verifier, + inputs, + sapling_shielded_data, + )?, }; async_checks.check().await?; @@ -295,24 +303,32 @@ where network: Network, script_verifier: script::Verifier, inputs: &[transparent::Input], + sapling_shielded_data: &Option>, ) -> Result { - Self::verify_v5_transaction_network_upgrade( - &request.transaction(), - request.upgrade(network), - )?; + let transaction = request.transaction(); + let upgrade = request.upgrade(network); + let shielded_sighash = transaction.sighash(upgrade, HashType::ALL, None); + + Self::verify_v5_transaction_network_upgrade(&transaction, upgrade)?; let _async_checks = Self::verify_transparent_inputs_and_outputs( &request, network, inputs, script_verifier, - )?; + )? + .and(Self::verify_sapling_shielded_data( + sapling_shielded_data, + &shielded_sighash, + )?); // TODO: - // - verify sapling shielded pool (#1981) // - verify orchard shielded pool (ZIP-224) (#2105) // - ZIP-216 (#1798) // - ZIP-244 (#1874) + // - validate bindingSigOrchard (#2103) + // - remaining consensus rules (#2379) + // - remove `should_panic` from tests unimplemented!("V5 transaction validation is not yet complete"); } @@ -416,10 +432,14 @@ where } /// Verifies a transaction's Sapling shielded data. - fn verify_sapling_shielded_data( - sapling_shielded_data: &Option>, + fn verify_sapling_shielded_data( + sapling_shielded_data: &Option>, shielded_sighash: &blake2b_simd::Hash, - ) -> Result { + ) -> Result + where + A: sapling::AnchorVariant + Clone, + sapling::Spend: From<(sapling::Spend, A::Shared)>, + { let mut async_checks = AsyncChecks::new(); if let Some(sapling_shielded_data) = sapling_shielded_data { diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs index 2dde3cd3a2b..9be4ddb63ba 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -201,6 +201,8 @@ fn v5_coinbase_transaction_with_enable_spends_flag_fails_validation() { } #[tokio::test] +// TODO: Remove `should_panic` once V5 transaction sighash is implemened by the merge of #2165. +#[should_panic] async fn v5_transaction_is_rejected_before_nu5_activation() { const V5_TRANSACTION_VERSION: u32 = 5; @@ -751,7 +753,7 @@ fn v4_with_sapling_outputs_and_no_spends() { transaction.sapling_spends_per_anchor().next().is_none() && transaction.sapling_outputs().next().is_some() }) - .expect("No transaction found with Sapling spends"); + .expect("No transaction found with Sapling outputs and no Sapling spends"); let expected_hash = transaction.hash(); @@ -775,6 +777,47 @@ fn v4_with_sapling_outputs_and_no_spends() { }); } +/// Test if a V5 transaction with Sapling spends is accepted by the verifier. +#[test] +// TODO: Remove `should_panic` once V5 transaction verification is complete. +#[should_panic] +fn v5_with_sapling_spends() { + zebra_test::init(); + zebra_test::RUNTIME.block_on(async { + let network = Network::Mainnet; + + let transaction = + fake_v5_transactions_for_network(network, zebra_test::vectors::MAINNET_BLOCKS.iter()) + .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(); + let height = transaction + .expiry_height() + .expect("Transaction is missing expiry height"); + + // 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, Ok(expected_hash)); + }); +} + // Utility functions /// Create a mock transparent transfer to be included in a transaction.