Refactor Sapling builder to separate out proof generation

Closes #741.

zcash_client_backend/src/data_api/wallet.rs

@@ -8,7 +8,7 @@ use zcash_primitives::{
     sapling::{
         self,
         note_encryption::{try_sapling_note_decryption, PreparedIncomingViewingKey},
-        prover::TxProver as SaplingProver,
+        prover::{OutputProver, SpendProver},
         Node,
     },
     transaction::{
@@ -191,7 +191,8 @@ where
 pub fn create_spend_to_address<DbT, ParamsT>(
     wallet_db: &mut DbT,
     params: &ParamsT,
-    prover: impl SaplingProver,
+    spend_prover: &impl SpendProver,
+    output_prover: &impl OutputProver,
     usk: &UnifiedSpendingKey,
     to: &RecipientAddress,
     amount: NonNegativeAmount,
@@ -231,7 +232,15 @@ where
         change_memo,
     )?;
 
-    create_proposed_transaction(wallet_db, params, prover, usk, ovk_policy, &proposal)
+    create_proposed_transaction(
+        wallet_db,
+        params,
+        spend_prover,
+        output_prover,
+        usk,
+        ovk_policy,
+        &proposal,
+    )
 }
 
 /// Constructs a transaction that sends funds as specified by the `request` argument
@@ -289,7 +298,8 @@ where
 pub fn spend<DbT, ParamsT, InputsT>(
     wallet_db: &mut DbT,
     params: &ParamsT,
-    prover: impl SaplingProver,
+    spend_prover: &impl SpendProver,
+    output_prover: &impl OutputProver,
     input_selector: &InputsT,
     usk: &UnifiedSpendingKey,
     request: zip321::TransactionRequest,
@@ -324,7 +334,15 @@ where
         min_confirmations,
     )?;
 
-    create_proposed_transaction(wallet_db, params, prover, usk, ovk_policy, &proposal)
+    create_proposed_transaction(
+        wallet_db,
+        params,
+        spend_prover,
+        output_prover,
+        usk,
+        ovk_policy,
+        &proposal,
+    )
 }
 
 /// Select transaction inputs, compute fees, and construct a proposal for a transaction
@@ -475,7 +493,8 @@ where
 pub fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT>(
     wallet_db: &mut DbT,
     params: &ParamsT,
-    prover: impl SaplingProver,
+    spend_prover: &impl SpendProver,
+    output_prover: &impl OutputProver,
     usk: &UnifiedSpendingKey,
     ovk_policy: OvkPolicy,
     proposal: &Proposal<FeeRuleT, DbT::NoteRef>,
@@ -663,7 +682,8 @@ where
     }
 
     // Build the transaction with the specified fee rule
-    let (tx, sapling_build_meta) = builder.build(&prover, proposal.fee_rule())?;
+    let (tx, sapling_build_meta) =
+        builder.build(spend_prover, output_prover, proposal.fee_rule())?;
 
     let internal_ivk = PreparedIncomingViewingKey::new(&dfvk.to_ivk(Scope::Internal));
     let sapling_outputs =
@@ -768,7 +788,8 @@ where
 pub fn shield_transparent_funds<DbT, ParamsT, InputsT>(
     wallet_db: &mut DbT,
     params: &ParamsT,
-    prover: impl SaplingProver,
+    spend_prover: &impl SpendProver,
+    output_prover: &impl OutputProver,
     input_selector: &InputsT,
     shielding_threshold: NonNegativeAmount,
     usk: &UnifiedSpendingKey,
@@ -798,7 +819,15 @@ where
         min_confirmations,
     )?;
 
-    create_proposed_transaction(wallet_db, params, prover, usk, OvkPolicy::Sender, &proposal)
+    create_proposed_transaction(
+        wallet_db,
+        params,
+        spend_prover,
+        output_prover,
+        usk,
+        OvkPolicy::Sender,
+        &proposal,
+    )
 }
 
 #[allow(clippy::type_complexity)]

zcash_client_sqlite/src/testing.rs

@@ -448,10 +448,12 @@ impl<Cache> TestState<Cache> {
         >,
     > {
         let params = self.network();
+        let prover = test_prover();
         create_spend_to_address(
             &mut self.db_data,
             &params,
-            test_prover(),
+            &prover,
+            &prover,
             usk,
             to,
             amount,
@@ -484,10 +486,12 @@ impl<Cache> TestState<Cache> {
         InputsT: InputSelector<DataSource = WalletDb<Connection, Network>>,
     {
         let params = self.network();
+        let prover = test_prover();
         spend(
             &mut self.db_data,
             &params,
-            test_prover(),
+            &prover,
+            &prover,
             input_selector,
             usk,
             request,
@@ -614,10 +618,12 @@ impl<Cache> TestState<Cache> {
         FeeRuleT: FeeRule,
     {
         let params = self.network();
+        let prover = test_prover();
         create_proposed_transaction(
             &mut self.db_data,
             &params,
-            test_prover(),
+            &prover,
+            &prover,
             usk,
             ovk_policy,
             proposal,
@@ -647,10 +653,12 @@ impl<Cache> TestState<Cache> {
         InputsT: InputSelector<DataSource = WalletDb<Connection, Network>>,
     {
         let params = self.network();
+        let prover = test_prover();
         shield_transparent_funds(
             &mut self.db_data,
             &params,
-            test_prover(),
+            &prover,
+            &prover,
             input_selector,
             shielding_threshold,
             usk,

zcash_client_sqlite/src/wallet/sapling.rs

@@ -451,8 +451,9 @@ pub(crate) mod tests {
         legacy::TransparentAddress,
         memo::{Memo, MemoBytes},
         sapling::{
-            note_encryption::try_sapling_output_recovery, prover::TxProver, Node, Note,
-            PaymentAddress,
+            note_encryption::try_sapling_output_recovery,
+            prover::{OutputProver, SpendProver},
+            Node, Note, PaymentAddress,
         },
         transaction::{
             components::{amount::NonNegativeAmount, Amount},
@@ -497,7 +498,7 @@ pub(crate) mod tests {
         zcash_primitives::transaction::components::{OutPoint, TxOut},
     };
 
-    pub(crate) fn test_prover() -> impl TxProver {
+    pub(crate) fn test_prover() -> impl SpendProver + OutputProver {
         match LocalTxProver::with_default_location() {
             Some(tx_prover) => tx_prover,
             None => {

zcash_extensions/src/transparent/demo.rs

@@ -839,7 +839,7 @@ mod tests {
             .unwrap();
         let (tx_a, _) = builder_a
             .txn_builder
-            .build_zfuture(&prover, &fee_rule)
+            .build_zfuture(&prover, &prover, &fee_rule)
             .map_err(|e| format!("build failure: {:?}", e))
             .unwrap();
         let tze_a = tx_a.tze_bundle().unwrap();
@@ -857,7 +857,7 @@ mod tests {
             .unwrap();
         let (tx_b, _) = builder_b
             .txn_builder
-            .build_zfuture(&prover, &fee_rule)
+            .build_zfuture(&prover, &prover, &fee_rule)
             .map_err(|e| format!("build failure: {:?}", e))
             .unwrap();
         let tze_b = tx_b.tze_bundle().unwrap();
@@ -882,7 +882,7 @@ mod tests {
 
         let (tx_c, _) = builder_c
             .txn_builder
-            .build_zfuture(&prover, &fee_rule)
+            .build_zfuture(&prover, &prover, &fee_rule)
             .map_err(|e| format!("build failure: {:?}", e))
             .unwrap();
         let tze_c = tx_c.tze_bundle().unwrap();

zcash_primitives/benches/note_decryption.rs

@@ -12,13 +12,11 @@ use zcash_primitives::{
             try_sapling_compact_note_decryption, try_sapling_note_decryption,
             PreparedIncomingViewingKey, SaplingDomain,
         },
-        prover::mock::MockTxProver,
+        prover::mock::{MockOutputProver, MockSpendProver},
         value::NoteValue,
         Diversifier, SaplingIvk,
     },
-    transaction::components::sapling::{
-        builder::SaplingBuilder, CompactOutputDescription, GrothProofBytes, OutputDescription,
-    },
+    transaction::components::sapling::{builder::SaplingBuilder, CompactOutputDescription},
 };
 
 #[cfg(unix)]
@@ -32,7 +30,7 @@ fn bench_note_decryption(c: &mut Criterion) {
     let invalid_ivk = SaplingIvk(jubjub::Fr::random(&mut rng));
 
     // Construct a Sapling output.
-    let output: OutputDescription<GrothProofBytes> = {
+    let output = {
         let diversifier = Diversifier([0; 11]);
         let pa = valid_ivk.to_payment_address(diversifier).unwrap();
 
@@ -46,8 +44,8 @@ fn bench_note_decryption(c: &mut Criterion) {
                 MemoBytes::empty(),
             )
             .unwrap();
-        let bundle = builder
-            .build(&MockTxProver, &mut (), &mut rng, height, None)
+        let (bundle, _) = builder
+            .build::<MockSpendProver, MockOutputProver, _>(&mut rng, height)
             .unwrap()
             .unwrap();
         bundle.shielded_outputs()[0].clone()

zcash_primitives/src/sapling.rs

@@ -36,11 +36,11 @@ pub fn spend_sig<R: RngCore + CryptoRng>(
     sighash: &[u8; 32],
     rng: &mut R,
 ) -> Signature {
-    spend_sig_internal(ask, ar, sighash, rng)
+    spend_sig_internal(&ask, ar, sighash, rng)
 }
 
 pub(crate) fn spend_sig_internal<R: RngCore>(
-    ask: PrivateKey,
+    ask: &PrivateKey,
     ar: jubjub::Fr,
     sighash: &[u8; 32],
     rng: &mut R,
@@ -60,6 +60,29 @@ pub(crate) fn spend_sig_internal<R: RngCore>(
     rsk.sign(&data_to_be_signed, rng, SPENDING_KEY_GENERATOR)
 }
 
+/// Verifies a spendAuthSig.
+///
+/// This only exists because the RedJubjub implementation inside `zcash_primitives` does
+/// not implement key prefixing (which was added in response to a Sapling audit). This
+/// will be fixed by migrating to the redjubjub crate.
+pub(crate) fn verify_spend_sig(
+    ak: &PublicKey,
+    alpha: jubjub::Fr,
+    sighash: &[u8; 32],
+    sig: &Signature,
+) -> bool {
+    // We compute `rk` (needed for key prefixing)
+    let rk = ak.randomize(alpha, SPENDING_KEY_GENERATOR);
+
+    // Compute the signature's message for rk/spend_auth_sig
+    let mut data_to_be_signed = [0u8; 64];
+    data_to_be_signed[0..32].copy_from_slice(&rk.0.to_bytes());
+    data_to_be_signed[32..64].copy_from_slice(&sighash[..]);
+
+    // Do the verifying
+    rk.verify(&data_to_be_signed, sig, SPENDING_KEY_GENERATOR)
+}
+
 #[cfg(any(test, feature = "test-dependencies"))]
 pub mod testing {
     pub use super::{

zcash_primitives/src/sapling/value.rs

@@ -38,7 +38,7 @@
 //! [Rust documentation]: https://doc.rust-lang.org/stable/std/primitive.i64.html
 
 use bitvec::{array::BitArray, order::Lsb0};
-use ff::Field;
+use ff::{Field, PrimeField};
 use group::GroupEncoding;
 use rand::RngCore;
 use subtle::CtOption;
@@ -86,6 +86,21 @@ impl ValueCommitTrapdoor {
         ValueCommitTrapdoor(jubjub::Scalar::random(rng))
     }
 
+    /// Constructs `ValueCommitTrapdoor` from the byte representation of a scalar.
+    ///
+    /// Returns a `None` [`CtOption`] if `bytes` is not a canonical representation of a
+    /// Jubjub scalar.
+    ///
+    /// This is a low-level API, requiring a detailed understanding of the
+    /// [use of value commitment trapdoors][saplingbalance] in the Zcash protocol
+    /// to use correctly and securely. It is intended to be used in combination
+    /// with [`ValueCommitment::derive`].
+    ///
+    /// [saplingbalance]: https://zips.z.cash/protocol/protocol.pdf#saplingbalance
+    pub fn from_bytes(bytes: [u8; 32]) -> CtOption<Self> {
+        jubjub::Scalar::from_repr(bytes).map(ValueCommitTrapdoor)
+    }
+
     /// Returns the inner Jubjub scalar representing this trapdoor.
     ///
     /// This is public for access by `zcash_proofs`.

zcash_primitives/src/sapling/value/sums.rs

@@ -123,6 +123,14 @@ impl Sub<&ValueCommitTrapdoor> for ValueCommitTrapdoor {
     }
 }
 
+impl Sub<TrapdoorSum> for TrapdoorSum {
+    type Output = TrapdoorSum;
+
+    fn sub(self, rhs: Self) -> Self::Output {
+        TrapdoorSum(self.0 - rhs.0)
+    }
+}
+
 impl SubAssign<&ValueCommitTrapdoor> for TrapdoorSum {
     fn sub_assign(&mut self, rhs: &ValueCommitTrapdoor) {
         self.0 -= rhs.0;
@@ -207,6 +215,14 @@ impl SubAssign<&ValueCommitment> for CommitmentSum {
     }
 }
 
+impl Sub<CommitmentSum> for CommitmentSum {
+    type Output = CommitmentSum;
+
+    fn sub(self, rhs: Self) -> Self::Output {
+        CommitmentSum(self.0 - rhs.0)
+    }
+}
+
 impl Sum<ValueCommitment> for CommitmentSum {
     fn sum<I: Iterator<Item = ValueCommitment>>(iter: I) -> Self {
         iter.fold(CommitmentSum::zero(), |acc, cv| acc + &cv)