From 2bfeef9430c6ab5c0b188c574325dff09e61f066 Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@electriccoin.co>
Date: Mon, 2 Oct 2023 13:33:33 +0000
Subject: [PATCH] zcash_proofs: Remove immediate verification of created Spend
 proofs

It can be useful to verify proofs after they have been created, but we
were only doing this for spend proofs, not output proofs. It also
duplicated code from the verifier logic. Once the prover and verifier
have been refactored, it will be easier to just call the verifier
immediately after the prover.
---
 zcash_proofs/CHANGELOG.md          |  6 ++++
 zcash_proofs/src/prover.rs         |  4 ++-
 zcash_proofs/src/sapling/prover.rs | 45 ++----------------------------
 3 files changed, 11 insertions(+), 44 deletions(-)

diff --git a/zcash_proofs/CHANGELOG.md b/zcash_proofs/CHANGELOG.md
index 33c36b7be..a35ba5669 100644
--- a/zcash_proofs/CHANGELOG.md
+++ b/zcash_proofs/CHANGELOG.md
@@ -6,6 +6,12 @@ and this library adheres to Rust's notion of
 [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Changed
+- `zcash_proofs::sapling::prover`:
+  - The `verifying_key` argument `SaplingProvingContext::spend_proof` has been
+    removed. Callers should instead use `SaplingVerifyingContext` to verify
+    proofs after they have been created.
+
 ### Removed
 - `zcash_proofs::circuit::sapling` (moved to `zcash_primitives::sapling::circuit`).
 - `zcash_proofs::circuit::{ecc, pedersen_hash}`
diff --git a/zcash_proofs/src/prover.rs b/zcash_proofs/src/prover.rs
index 63c1db79f..40b1240fb 100644
--- a/zcash_proofs/src/prover.rs
+++ b/zcash_proofs/src/prover.rs
@@ -22,6 +22,9 @@ use crate::{default_params_folder, SAPLING_OUTPUT_NAME, SAPLING_SPEND_NAME};
 /// locally-accessible paths.
 pub struct LocalTxProver {
     spend_params: Parameters<Bls12>,
+    // TODO: Either re-introduce verification-after-proving (once the verifier is
+    // refactored), or remove this.
+    #[allow(unused)]
     spend_vk: PreparedVerifyingKey<Bls12>,
     output_params: Parameters<Bls12>,
 }
@@ -164,7 +167,6 @@ impl TxProver for LocalTxProver {
             anchor,
             merkle_path,
             &self.spend_params,
-            &self.spend_vk,
         )?;
 
         let mut zkproof = [0u8; GROTH_PROOF_SIZE];
diff --git a/zcash_proofs/src/sapling/prover.rs b/zcash_proofs/src/sapling/prover.rs
index 05767f348..9c31efc9f 100644
--- a/zcash_proofs/src/sapling/prover.rs
+++ b/zcash_proofs/src/sapling/prover.rs
@@ -1,9 +1,6 @@
-use bellman::{
-    gadgets::multipack,
-    groth16::{create_random_proof, verify_proof, Parameters, PreparedVerifyingKey, Proof},
-};
+use bellman::groth16::{create_random_proof, Parameters, Proof};
 use bls12_381::Bls12;
-use group::{Curve, GroupEncoding};
+use group::GroupEncoding;
 use rand_core::OsRng;
 use zcash_primitives::{
     sapling::{
@@ -52,7 +49,6 @@ impl SaplingProvingContext {
         anchor: bls12_381::Scalar,
         merkle_path: MerklePath,
         proving_key: &Parameters<Bls12>,
-        verifying_key: &PreparedVerifyingKey<Bls12>,
     ) -> Result<(Proof<Bls12>, ValueCommitment, PublicKey), ()> {
         // Initialize secure RNG
         let mut rng = OsRng;
@@ -82,12 +78,6 @@ impl SaplingProvingContext {
         // Let's compute the nullifier while we have the position
         let note = Note::from_parts(payment_address, NoteValue::from_raw(value), rseed);
 
-        let nullifier = note.nf(
-            &viewing_key.nk,
-            u64::try_from(merkle_path.position())
-                .expect("Sapling note commitment tree position must fit into a u64"),
-        );
-
         // We now have the full witness for our circuit
         let pos: u64 = merkle_path.position().into();
         let instance = Spend {
@@ -109,37 +99,6 @@ impl SaplingProvingContext {
         let proof =
             create_random_proof(instance, proving_key, &mut rng).expect("proving should not fail");
 
-        // Try to verify the proof:
-        // Construct public input for circuit
-        let mut public_input = [bls12_381::Scalar::zero(); 7];
-        {
-            let affine = rk.0.to_affine();
-            let (u, v) = (affine.get_u(), affine.get_v());
-            public_input[0] = u;
-            public_input[1] = v;
-        }
-        {
-            let affine = value_commitment.as_inner().to_affine();
-            let (u, v) = (affine.get_u(), affine.get_v());
-            public_input[2] = u;
-            public_input[3] = v;
-        }
-        public_input[4] = anchor;
-
-        // Add the nullifier through multiscalar packing
-        {
-            let nullifier = multipack::bytes_to_bits_le(&nullifier.0);
-            let nullifier = multipack::compute_multipacking(&nullifier);
-
-            assert_eq!(nullifier.len(), 2);
-
-            public_input[5] = nullifier[0];
-            public_input[6] = nullifier[1];
-        }
-
-        // Verify the proof
-        verify_proof(verifying_key, &proof, &public_input[..]).map_err(|_| ())?;
-
         // Accumulate the value commitment in the context
         self.cv_sum += &value_commitment;