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 <oxarbitrage@gmail.com>

* 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 <oxarbitrage@gmail.com>

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<ZS>,
         inputs: &[transparent::Input],
+        sapling_shielded_data: &Option<sapling::ShieldedData<sapling::SharedAnchor>>,
     ) -> Result<AsyncChecks, TransactionError> {
-        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<sapling::ShieldedData<sapling::PerSpendAnchor>>,
+    fn verify_sapling_shielded_data<A>(
+        sapling_shielded_data: &Option<sapling::ShieldedData<A>>,
         shielded_sighash: &blake2b_simd::Hash,
-    ) -> Result<AsyncChecks, TransactionError> {
+    ) -> Result<AsyncChecks, TransactionError>
+    where
+        A: sapling::AnchorVariant + Clone,
+        sapling::Spend<sapling::PerSpendAnchor>: From<(sapling::Spend<A>, A::Shared)>,
+    {
         let mut async_checks = AsyncChecks::new();
 
         if let Some(sapling_shielded_data) = sapling_shielded_data {

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.