zcash_primitives: Add `ProverProgress` trait to `sapling::builder`

This breaks the link between the concrete `Sender<Progress>` channel
used by the main builder in `zcash_primitives`, and the proof creation
APIs in what will become the `sapling-crypto` crate.

Part of zcash/librustzcash#1044.

zcash_extensions/src/transparent/demo.rs

@@ -637,7 +637,7 @@ mod tests {
         }
     }
 
-    fn demo_builder<'a>(height: BlockHeight) -> DemoBuilder<Builder<'a, FutureNetwork, OsRng>> {
+    fn demo_builder<'a>(height: BlockHeight) -> DemoBuilder<Builder<'a, FutureNetwork, OsRng, ()>> {
         DemoBuilder {
             txn_builder: Builder::new(FutureNetwork, height, None),
             extension_id: 0,

zcash_primitives/CHANGELOG.md

@@ -19,8 +19,9 @@ and this library adheres to Rust's notion of
   - `builder::{InProgressProofs, Unproven, Proven}`
   - `builder::{InProgressSignatures, Unsigned, PartiallyAuthorized}`
   - `builder::{MaybeSigned, SigningParts}`
-  - `builder::{SpendDescriptionInfo::value}`
+  - `builder::SpendDescriptionInfo::value`
-  - `builder::{SaplingOutputInfo}`
+  - `builder::SaplingOutputInfo`
+  - `builder::ProverProgress`
   - `bundle` module, containing the following types moved from
     `zcash_primitives::transaction::components::sapling`:
     - `Bundle`
@@ -180,10 +181,15 @@ and this library adheres to Rust's notion of
     `redjubjub::VerificationKey<Binding>` instead of
     `zcash_primitives::sapling::redjubjub::PublicKey`.
 - `zcash_primitives::transaction`:
+  - `builder::Builder` now has a generic parameter for the type of progress
+    notifier, which needs to implement `sapling::builder::ProverProgress` in
+    order to build transactions.
   - `builder::Builder::{build, build_zfuture}` now take
     `&impl SpendProver, &impl OutputProver` instead of `&impl TxProver`.
   - `builder::Builder::add_sapling_spend` no longer takes a `diversifier`
     argument as the diversifier may be obtained from the note.
+  - `builder::Builder::with_progress_notifier` now consumes `self` and returns a
+    `Builder` typed on the provided channel.
   - `components::transparent::TxOut.value` now has type `NonNegativeAmount`
     instead of `Amount`.
   - `components::transparent::fees` has been moved to 

zcash_primitives/src/sapling/builder.rs

@@ -1,31 +1,28 @@
 //! Types and functions for building Sapling transaction components.
 
 use core::fmt;
-use std::{marker::PhantomData, sync::mpsc::Sender};
+use std::marker::PhantomData;
 
 use group::ff::Field;
 use rand::{seq::SliceRandom, RngCore};
 use rand_core::CryptoRng;
 use redjubjub::{Binding, SpendAuth};
 
-use crate::{
+use crate::sapling::{
-    sapling::{
+    self,
-        self,
+    bundle::{
-        bundle::{
+        Authorization, Authorized, Bundle, GrothProofBytes, MapAuth, OutputDescription,
-            Authorization, Authorized, Bundle, GrothProofBytes, MapAuth, OutputDescription,
+        SpendDescription,
-            SpendDescription,
-        },
-        keys::{OutgoingViewingKey, SpendAuthorizingKey, SpendValidatingKey},
-        note_encryption::{sapling_note_encryption, Zip212Enforcement},
-        prover::{OutputProver, SpendProver},
-        util::generate_random_rseed_internal,
-        value::{
-            CommitmentSum, NoteValue, TrapdoorSum, ValueCommitTrapdoor, ValueCommitment, ValueSum,
-        },
-        zip32::ExtendedSpendingKey,
-        Diversifier, MerklePath, Node, Note, PaymentAddress, ProofGenerationKey, SaplingIvk,
     },
-    transaction::builder::Progress,
+    keys::{OutgoingViewingKey, SpendAuthorizingKey, SpendValidatingKey},
+    note_encryption::{sapling_note_encryption, Zip212Enforcement},
+    prover::{OutputProver, SpendProver},
+    util::generate_random_rseed_internal,
+    value::{
+        CommitmentSum, NoteValue, TrapdoorSum, ValueCommitTrapdoor, ValueCommitment, ValueSum,
+    },
+    zip32::ExtendedSpendingKey,
+    Diversifier, MerklePath, Node, Note, PaymentAddress, ProofGenerationKey, SaplingIvk,
 };
 
 /// If there are any shielded inputs, always have at least two shielded outputs, padding
@@ -571,21 +568,47 @@ impl InProgressProofs for Proven {
     type OutputProof = GrothProofBytes;
 }
 
-struct CreateProofs<'a, SP: SpendProver, OP: OutputProver, R: RngCore> {
+/// Reports on the progress made towards creating proofs for a bundle.
+pub trait ProverProgress {
+    /// Updates the progress instance with the number of steps completed and the total
+    /// number of steps.
+    fn update(&mut self, cur: u32, end: u32);
+}
+
+impl ProverProgress for () {
+    fn update(&mut self, _: u32, _: u32) {}
+}
+
+impl<U: From<(u32, u32)>> ProverProgress for std::sync::mpsc::Sender<U> {
+    fn update(&mut self, cur: u32, end: u32) {
+        // If the send fails, we should ignore the error, not crash.
+        self.send(U::from((cur, end))).unwrap_or(());
+    }
+}
+
+impl<U: ProverProgress> ProverProgress for &mut U {
+    fn update(&mut self, cur: u32, end: u32) {
+        (*self).update(cur, end);
+    }
+}
+
+struct CreateProofs<'a, SP: SpendProver, OP: OutputProver, R: RngCore, U: ProverProgress> {
     spend_prover: &'a SP,
     output_prover: &'a OP,
     rng: R,
-    progress_notifier: Option<&'a Sender<Progress>>,
+    progress_notifier: U,
     total_progress: u32,
     progress: u32,
 }
 
-impl<'a, SP: SpendProver, OP: OutputProver, R: RngCore> CreateProofs<'a, SP, OP, R> {
+impl<'a, SP: SpendProver, OP: OutputProver, R: RngCore, U: ProverProgress>
+    CreateProofs<'a, SP, OP, R, U>
+{
     fn new(
         spend_prover: &'a SP,
         output_prover: &'a OP,
         rng: R,
-        progress_notifier: Option<&'a Sender<Progress>>,
+        progress_notifier: U,
         total_progress: u32,
     ) -> Self {
         // Keep track of the total number of steps computed
@@ -602,17 +625,19 @@ impl<'a, SP: SpendProver, OP: OutputProver, R: RngCore> CreateProofs<'a, SP, OP,
     fn update_progress(&mut self) {
         // Update progress and send a notification on the channel
         self.progress += 1;
-        if let Some(sender) = self.progress_notifier {
+        self.progress_notifier
-            // If the send fails, we should ignore the error, not crash.
+            .update(self.progress, self.total_progress);
-            sender
-                .send(Progress::new(self.progress, Some(self.total_progress)))
-                .unwrap_or(());
-        }
     }
 }
 
-impl<'a, S: InProgressSignatures, SP: SpendProver, OP: OutputProver, R: RngCore>
+impl<
-    MapAuth<InProgress<Unproven, S>, InProgress<Proven, S>> for CreateProofs<'a, SP, OP, R>
+        'a,
+        S: InProgressSignatures,
+        SP: SpendProver,
+        OP: OutputProver,
+        R: RngCore,
+        U: ProverProgress,
+    > MapAuth<InProgress<Unproven, S>, InProgress<Proven, S>> for CreateProofs<'a, SP, OP, R, U>
 {
     fn map_spend_proof(&mut self, spend: sapling::circuit::Spend) -> GrothProofBytes {
         let proof = self.spend_prover.create_proof(spend, &mut self.rng);
@@ -645,7 +670,7 @@ impl<S: InProgressSignatures, V> Bundle<InProgress<Unproven, S>, V> {
         spend_prover: &SP,
         output_prover: &OP,
         rng: impl RngCore,
-        progress_notifier: Option<&Sender<Progress>>,
+        progress_notifier: impl ProverProgress,
     ) -> Bundle<InProgress<Proven, S>, V> {
         let total_progress =
             self.shielded_spends().len() as u32 + self.shielded_outputs().len() as u32;
@@ -901,7 +926,7 @@ pub mod testing {
                         .unwrap();
 
                     let bundle =
-                        bundle.create_proofs(&MockSpendProver, &MockOutputProver, &mut rng, None);
+                        bundle.create_proofs(&MockSpendProver, &MockOutputProver, &mut rng, ());
 
                     bundle
                         .apply_signatures(&mut rng, fake_sighash_bytes, &[extsk.expsk.ask])

zcash_primitives/src/transaction/builder.rs

@@ -130,11 +130,16 @@ pub struct Progress {
     end: Option<u32>,
 }
 
-impl Progress {
+impl From<(u32, u32)> for Progress {
-    pub fn new(cur: u32, end: Option<u32>) -> Self {
+    fn from((cur, end): (u32, u32)) -> Self {
-        Self { cur, end }
+        Self {
+            cur,
+            end: Some(end),
+        }
     }
+}
 
+impl Progress {
     /// Returns the number of steps completed so far while building the transaction.
     ///
     /// Note that each step may not be of the same complexity/duration.
@@ -152,7 +157,7 @@ impl Progress {
 }
 
 /// Generates a [`Transaction`] from its inputs and outputs.
-pub struct Builder<'a, P, R> {
+pub struct Builder<'a, P, R, U: sapling::builder::ProverProgress> {
     params: P,
     rng: R,
     target_height: BlockHeight,
@@ -170,10 +175,10 @@ pub struct Builder<'a, P, R> {
     tze_builder: TzeBuilder<'a, TransactionData<Unauthorized>>,
     #[cfg(not(feature = "zfuture"))]
     tze_builder: std::marker::PhantomData<&'a ()>,
-    progress_notifier: Option<Sender<Progress>>,
+    progress_notifier: U,
 }
 
-impl<'a, P, R> Builder<'a, P, R> {
+impl<'a, P, R, U: sapling::builder::ProverProgress> Builder<'a, P, R, U> {
     /// Returns the network parameters that the builder has been configured for.
     pub fn params(&self) -> &P {
         &self.params
@@ -210,7 +215,7 @@ impl<'a, P, R> Builder<'a, P, R> {
     }
 }
 
-impl<'a, P: consensus::Parameters> Builder<'a, P, OsRng> {
+impl<'a, P: consensus::Parameters> Builder<'a, P, OsRng, ()> {
     /// Creates a new `Builder` targeted for inclusion in the block with the given height,
     /// using default values for general transaction fields and the default OS random.
     ///
@@ -227,7 +232,7 @@ impl<'a, P: consensus::Parameters> Builder<'a, P, OsRng> {
     }
 }
 
-impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> {
+impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R, ()> {
     /// Creates a new `Builder` targeted for inclusion in the block with the given height
     /// and randomness source, using default values for general transaction fields.
     ///
@@ -240,7 +245,7 @@ impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> {
         target_height: BlockHeight,
         orchard_anchor: Option<orchard::tree::Anchor>,
         rng: R,
-    ) -> Builder<'a, P, R> {
+    ) -> Self {
         let orchard_builder = if params.is_nu_active(NetworkUpgrade::Nu5, target_height) {
             orchard_anchor.map(|anchor| {
                 orchard::builder::Builder::new(
@@ -278,10 +283,39 @@ impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> {
             tze_builder: TzeBuilder::empty(),
             #[cfg(not(feature = "zfuture"))]
             tze_builder: std::marker::PhantomData,
-            progress_notifier: None,
+            progress_notifier: (),
         }
     }
 
+    /// Sets the notifier channel, where progress of building the transaction is sent.
+    ///
+    /// An update is sent after every Sapling Spend or Output is computed, and the `u32`
+    /// sent represents the total steps completed so far. It will eventually send number
+    /// of spends + outputs. If there's an error building the transaction, the channel is
+    /// closed.
+    pub fn with_progress_notifier(
+        self,
+        progress_notifier: Sender<Progress>,
+    ) -> Builder<'a, P, R, Sender<Progress>> {
+        Builder {
+            params: self.params,
+            rng: self.rng,
+            target_height: self.target_height,
+            expiry_height: self.expiry_height,
+            transparent_builder: self.transparent_builder,
+            sapling_builder: self.sapling_builder,
+            orchard_builder: self.orchard_builder,
+            sapling_asks: self.sapling_asks,
+            orchard_saks: self.orchard_saks,
+            tze_builder: self.tze_builder,
+            progress_notifier,
+        }
+    }
+}
+
+impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng, U: sapling::builder::ProverProgress>
+    Builder<'a, P, R, U>
+{
     /// Adds an Orchard note to be spent in this bundle.
     ///
     /// Returns an error if the given Merkle path does not have the required anchor for
@@ -380,16 +414,6 @@ impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> {
         self.transparent_builder.add_output(to, value)
     }
 
-    /// Sets the notifier channel, where progress of building the transaction is sent.
-    ///
-    /// An update is sent after every Spend or Output is computed, and the `u32` sent
-    /// represents the total steps completed so far. It will eventually send number of
-    /// spends + outputs. If there's an error building the transaction, the channel is
-    /// closed.
-    pub fn with_progress_notifier(&mut self, progress_notifier: Sender<Progress>) {
-        self.progress_notifier = Some(progress_notifier);
-    }
-
     /// Returns the sum of the transparent, Sapling, Orchard, and TZE value balances.
     fn value_balance(&self) -> Result<Amount, BalanceError> {
         let value_balances = [
@@ -543,7 +567,7 @@ impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> {
                         spend_prover,
                         output_prover,
                         &mut rng,
-                        self.progress_notifier.as_ref(),
+                        self.progress_notifier,
                     ),
                     tx_metadata,
                 )
@@ -650,8 +674,8 @@ impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> {
 }
 
 #[cfg(feature = "zfuture")]
-impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> ExtensionTxBuilder<'a>
+impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng, U: sapling::builder::ProverProgress>
-    for Builder<'a, P, R>
+    ExtensionTxBuilder<'a> for Builder<'a, P, R, U>
 {
     type BuildCtx = TransactionData<Unauthorized>;
     type BuildError = tze::builder::Error;
@@ -691,12 +715,15 @@ mod testing {
     use super::{Builder, Error, SaplingMetadata};
     use crate::{
         consensus::{self, BlockHeight},
-        sapling::prover::mock::{MockOutputProver, MockSpendProver},
+        sapling::{
+            self,
+            prover::mock::{MockOutputProver, MockSpendProver},
+        },
         transaction::fees::fixed,
         transaction::Transaction,
     };
 
-    impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> {
+    impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R, ()> {
         /// Creates a new `Builder` targeted for inclusion in the block with the given height
         /// and randomness source, using default values for general transaction fields.
         ///
@@ -710,7 +737,7 @@ mod testing {
             params: P,
             height: BlockHeight,
             rng: R,
-        ) -> Builder<'a, P, impl RngCore + CryptoRng> {
+        ) -> Builder<'a, P, impl RngCore + CryptoRng, ()> {
             struct FakeCryptoRng<R: RngCore>(R);
             impl<R: RngCore> CryptoRng for FakeCryptoRng<R> {}
             impl<R: RngCore> RngCore for FakeCryptoRng<R> {
@@ -733,7 +760,13 @@ mod testing {
             Builder::new_internal(params, FakeCryptoRng(rng), height, None)
         }
     }
-    impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> {
+    impl<
+            'a,
+            P: consensus::Parameters,
+            R: RngCore + CryptoRng,
+            U: sapling::builder::ProverProgress,
+        > Builder<'a, P, R, U>
+    {
         pub fn mock_build(self) -> Result<(Transaction, SaplingMetadata), Error<Infallible>> {
             #[allow(deprecated)]
             self.build(
@@ -801,7 +834,7 @@ mod tests {
             tze_builder: TzeBuilder::empty(),
             #[cfg(not(feature = "zfuture"))]
             tze_builder: std::marker::PhantomData,
-            progress_notifier: None,
+            progress_notifier: (),
             orchard_builder: None,
             sapling_asks: vec![],
             orchard_saks: Vec::new(),