Refactor Sprout Join Split validation by transaction verifier (#2371)

* Refactor to create `verify_sprout_shielded_data`

Move the join split verification code into a new
`verify_sprout_shielded_data` helper method that returns an
`AsyncChecks` set.

* Test if signed V4 tx. join splits are accepted

Create a fake V4 transaction with a dummy join split, and sign it
appropriately. Check if the transaction verifier accepts the
transaction.

* Test if unsigned V4 tx. joinsplit data is rejected

Create a fake V4 transaction with a dummy join split. Do NOT sign this
transaction's join split data, and check that the verifier rejects the
transaction.

* Join tests to share Tokio runtime

Otherwise one of the tests might fail incorrectly because of a
limitation in the test environment. `Batch` services spawn a task in the
Tokio runtime, but separate tests can have separate runtimes, so sharing
a `Batch` service can lead to the worker task only being available for
one of the tests.

Cargo.lock

@@ -4550,6 +4550,7 @@ dependencies = [
  "lazy_static",
  "metrics",
  "once_cell",
+ "rand 0.7.3",
  "rand 0.8.4",
  "serde",
  "spandoc",

zebra-consensus/Cargo.toml

@@ -35,6 +35,7 @@ wagyu-zcash-parameters = "0.2.0"
 
 [dev-dependencies]
 color-eyre = "0.5.11"
+rand07 = { package = "rand", version = "0.7" }
 spandoc = "0.2"
 tokio = { version = "0.3.6", features = ["full"] }
 tracing-error = "0.1.2"

zebra-consensus/src/transaction.rs

@@ -250,7 +250,6 @@ where
         let mut spend_verifier = primitives::groth16::SPEND_VERIFIER.clone();
         let mut output_verifier = primitives::groth16::OUTPUT_VERIFIER.clone();
 
-        let mut ed25519_verifier = primitives::ed25519::VERIFIER.clone();
         let mut redjubjub_verifier = primitives::redjubjub::VERIFIER.clone();
 
         // A set of asynchronous checks which must all succeed.
@@ -270,36 +269,10 @@ where
 
         let shielded_sighash = tx.sighash(upgrade, HashType::ALL, None);
 
-        if let Some(joinsplit_data) = joinsplit_data {
+        async_checks.extend(Self::verify_sprout_shielded_data(
-            // XXX create a method on JoinSplitData
+            joinsplit_data,
-            // that prepares groth16::Items with the correct proofs
+            &shielded_sighash,
-            // and proof inputs, handling interstitial treestates
+        ));
-            // correctly.
-
-            // Then, pass those items to self.joinsplit to verify them.
-
-            // Consensus rule: The joinSplitSig MUST represent a
-            // valid signature, under joinSplitPubKey, of the
-            // sighash.
-            //
-            // Queue the validation of the JoinSplit signature while
-            // adding the resulting future to our collection of
-            // async checks that (at a minimum) must pass for the
-            // transaction to verify.
-            //
-            // https://zips.z.cash/protocol/protocol.pdf#sproutnonmalleability
-            // https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus
-            let rsp = ed25519_verifier.ready_and().await?.call(
-                (
-                    joinsplit_data.pub_key,
-                    joinsplit_data.sig,
-                    &shielded_sighash,
-                )
-                    .into(),
-            );
-
-            async_checks.push(rsp.boxed());
-        }
 
         if let Some(sapling_shielded_data) = sapling_shielded_data {
             for spend in sapling_shielded_data.spends_per_anchor() {
@@ -498,6 +471,42 @@ where
         Ok(script_checks)
     }
 
+    /// Verifies a transaction's Sprout shielded join split data.
+    fn verify_sprout_shielded_data(
+        joinsplit_data: &Option<transaction::JoinSplitData<Groth16Proof>>,
+        shielded_sighash: &blake2b_simd::Hash,
+    ) -> AsyncChecks {
+        let checks = AsyncChecks::new();
+
+        if let Some(joinsplit_data) = joinsplit_data {
+            // XXX create a method on JoinSplitData
+            // that prepares groth16::Items with the correct proofs
+            // and proof inputs, handling interstitial treestates
+            // correctly.
+
+            // Then, pass those items to self.joinsplit to verify them.
+
+            // Consensus rule: The joinSplitSig MUST represent a
+            // valid signature, under joinSplitPubKey, of the
+            // sighash.
+            //
+            // Queue the validation of the JoinSplit signature while
+            // adding the resulting future to our collection of
+            // async checks that (at a minimum) must pass for the
+            // transaction to verify.
+            //
+            // https://zips.z.cash/protocol/protocol.pdf#sproutnonmalleability
+            // https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus
+            let ed25519_verifier = primitives::ed25519::VERIFIER.clone();
+            let ed25519_item =
+                (joinsplit_data.pub_key, joinsplit_data.sig, shielded_sighash).into();
+
+            checks.push(ed25519_verifier.oneshot(ed25519_item).boxed());
+        }
+
+        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

zebra-consensus/src/transaction/tests.rs

@@ -1,4 +1,4 @@
-use std::{collections::HashMap, convert::TryFrom, sync::Arc};
+use std::{collections::HashMap, convert::TryFrom, convert::TryInto, sync::Arc};
 
 use tower::{service_fn, ServiceExt};
 
@@ -6,9 +6,12 @@ use zebra_chain::{
     amount::Amount,
     block, orchard,
     parameters::{Network, NetworkUpgrade},
+    primitives::{ed25519, x25519, Groth16Proof},
+    serialization::ZcashDeserialize,
+    sprout,
     transaction::{
         arbitrary::{fake_v5_transactions_for_network, insert_fake_orchard_shielded_data},
-        Hash, LockTime, Transaction,
+        Hash, HashType, JoinSplitData, LockTime, Transaction,
     },
     transparent,
 };
@@ -451,6 +454,86 @@ async fn v5_transaction_with_transparent_transfer_is_rejected_by_the_script() {
     );
 }
 
+/// Tests transactions with Sprout transfers.
+///
+/// This is actually two tests:
+/// - Test if signed V4 transaction with a dummy [`sprout::JoinSplit`] is accepted.
+/// - Test if an unsigned V4 transaction with a dummy [`sprout::JoinSplit`] is rejected.
+///
+/// The first test verifies if the transaction verifier correctly accepts a signed transaction. The
+/// second test verifies if the transaction verifier correctly rejects the transaction because of
+/// the invalid signature.
+///
+/// These tests are grouped together because of a limitation to test shared [`tower_batch::Batch`]
+/// services. Such services spawn a Tokio task in the runtime, and `#[tokio::test]` can create a
+/// separate runtime for each test. This means that the worker task is created for one test and
+/// destroyed before the other gets a chance to use it. (We'll fix this in #2390.)
+#[tokio::test]
+async fn v4_with_sprout_transfers() {
+    let network = Network::Mainnet;
+    let network_upgrade = NetworkUpgrade::Canopy;
+
+    let canopy_activation_height = network_upgrade
+        .activation_height(network)
+        .expect("Canopy activation height is not set");
+
+    let transaction_block_height =
+        (canopy_activation_height + 10).expect("Canopy activation height is too large");
+
+    // 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);
+
+    for should_sign in [true, false] {
+        // Create a fake Sprout join split
+        let (joinsplit_data, signing_key) = mock_sprout_join_split_data();
+
+        let mut transaction = Transaction::V4 {
+            inputs: vec![],
+            outputs: vec![],
+            lock_time: LockTime::Height(block::Height(0)),
+            expiry_height: (transaction_block_height + 1).expect("expiry height is too large"),
+            joinsplit_data: Some(joinsplit_data),
+            sapling_shielded_data: None,
+        };
+
+        let expected_result = if should_sign {
+            // Sign the transaction
+            let sighash = transaction.sighash(network_upgrade, HashType::ALL, None);
+
+            match &mut transaction {
+                Transaction::V4 {
+                    joinsplit_data: Some(joinsplit_data),
+                    ..
+                } => joinsplit_data.sig = signing_key.sign(sighash.as_bytes()),
+                _ => unreachable!("Mock transaction was created incorrectly"),
+            }
+
+            Ok(transaction.hash())
+        } else {
+            // TODO: Fix error downcast
+            // Err(TransactionError::Ed25519(ed25519::Error::InvalidSignature))
+            Err(TransactionError::InternalDowncastError(
+                "downcast to redjubjub::Error failed, original error: InvalidSignature".to_string(),
+            ))
+        };
+
+        // Test the transaction verifier
+        let result = verifier
+            .clone()
+            .oneshot(Request::Block {
+                transaction: Arc::new(transaction),
+                known_utxos: Arc::new(HashMap::new()),
+                height: transaction_block_height,
+            })
+            .await;
+
+        assert_eq!(result, expected_result);
+    }
+}
+
 // Utility functions
 
 /// Create a mock transparent transfer to be included in a transaction.
@@ -524,3 +607,56 @@ fn mock_transparent_transfer(
 
     (input, output, known_utxos)
 }
+
+/// Create a mock [`sprout::JoinSplit`] and include it in a [`transaction::JoinSplitData`].
+///
+/// This creates a dummy join split. By itself it is invalid, but it is useful for including in a
+/// transaction to check the signatures.
+///
+/// The [`transaction::JoinSplitData`] with the dummy [`sprout::JoinSplit`] is returned together
+/// with the [`ed25519::SigningKey`] that can be used to create a signature to later add to the
+/// returned join split data.
+fn mock_sprout_join_split_data() -> (JoinSplitData<Groth16Proof>, ed25519::SigningKey) {
+    // Prepare dummy inputs for the join split
+    let zero_amount = 0_i32
+        .try_into()
+        .expect("Invalid JoinSplit transparent input");
+    let anchor = sprout::tree::Root::default();
+    let nullifier = sprout::note::Nullifier([0u8; 32]);
+    let commitment = sprout::commitment::NoteCommitment::from([0u8; 32]);
+    let ephemeral_key =
+        x25519::PublicKey::from(&x25519::EphemeralSecret::new(rand07::thread_rng()));
+    let random_seed = [0u8; 32];
+    let mac = sprout::note::Mac::zcash_deserialize(&[0u8; 32][..])
+        .expect("Failure to deserialize dummy MAC");
+    let zkproof = Groth16Proof([0u8; 192]);
+    let encrypted_note = sprout::note::EncryptedNote([0u8; 601]);
+
+    // Create an dummy join split
+    let joinsplit = sprout::JoinSplit {
+        vpub_old: zero_amount,
+        vpub_new: zero_amount,
+        anchor,
+        nullifiers: [nullifier; 2],
+        commitments: [commitment; 2],
+        ephemeral_key,
+        random_seed,
+        vmacs: [mac.clone(), mac],
+        zkproof,
+        enc_ciphertexts: [encrypted_note; 2],
+    };
+
+    // Create a usable signing key
+    let signing_key = ed25519::SigningKey::new(rand::thread_rng());
+    let verification_key = ed25519::VerificationKey::from(&signing_key);
+
+    // Populate join split data with the dummy join split.
+    let joinsplit_data = JoinSplitData {
+        first: joinsplit,
+        rest: vec![],
+        pub_key: verification_key.into(),
+        sig: [0u8; 64].into(),
+    };
+
+    (joinsplit_data, signing_key)
+}