zcash_primitives: Avoid passing duplicate diversifier information to Sapling builder.

The note provided to `add_sapling_spend` contains the recipient address,
and we can extract the diversifier from this address, so we should not
pass it separately.

zcash_client_backend/src/data_api/wallet.rs

@@ -605,7 +605,7 @@ where
                     })
                 })?;
 
-                builder.add_sapling_spend(key, selected.diversifier(), note, merkle_path)?;
+                builder.add_sapling_spend(key, note, merkle_path)?;
             }
             Ok(())
         })?;

zcash_extensions/src/transparent/demo.rs

@@ -823,7 +823,7 @@ mod tests {
 
         let mut builder_a = demo_builder(tx_height);
         builder_a
-            .add_sapling_spend(extsk, *to.diversifier(), note1, witness1.path().unwrap())
+            .add_sapling_spend(extsk, note1, witness1.path().unwrap())
             .unwrap();
 
         let value = NonNegativeAmount::const_from_u64(100000);

zcash_primitives/CHANGELOG.md

@@ -111,7 +111,9 @@ and this library adheres to Rust's notion of
     parameter.
   - `builder::SaplingBuilder::new` now takes a `Zip212Enforcement` argument
     instead of a `P: consensus::Parameters` argument and a target height.
-  - `builder::SaplingBuilder::add_spend` now takes `extsk` by reference.
+  - `builder::SaplingBuilder::add_spend` now takes `extsk` by reference. Also,
+    it no longer takes a `diversifier` argument as the diversifier may be obtained
+    from the note.
   - `builder::SaplingBuilder::add_output` now takes an `Option<[u8; 512]>` memo
     instead of a `MemoBytes`.
   - `builder::SaplingBuilder::build` no longer takes a prover, proving context,
@@ -151,6 +153,8 @@ and this library adheres to Rust's notion of
 - `zcash_primitives::transaction`:
   - `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.
   - `components::transparent::TxOut.value` now has type `NonNegativeAmount`
     instead of `Amount`.
   - `Unauthorized::SaplingAuth` now has type `InProgress<Proven, Unsigned>`.

zcash_primitives/src/sapling/builder.rs

@@ -74,7 +74,6 @@ impl fmt::Display for Error {
 #[derive(Debug, Clone)]
 pub struct SpendDescriptionInfo {
     proof_generation_key: ProofGenerationKey,
-    diversifier: Diversifier,
     note: Note,
     alpha: jubjub::Fr,
     merkle_path: MerklePath,
@@ -97,13 +96,11 @@ impl SpendDescriptionInfo {
     fn new_internal<R: RngCore>(
         mut rng: &mut R,
         extsk: &ExtendedSpendingKey,
-        diversifier: Diversifier,
         note: Note,
         merkle_path: MerklePath,
     ) -> Self {
         SpendDescriptionInfo {
             proof_generation_key: extsk.expsk.proof_generation_key(),
-            diversifier,
             note,
             alpha: jubjub::Fr::random(&mut rng),
             merkle_path,
@@ -133,7 +130,7 @@ impl SpendDescriptionInfo {
 
         let zkproof = Pr::prepare_circuit(
             self.proof_generation_key,
-            self.diversifier,
+            *self.note.recipient().diversifier(),
             *self.note.rseed(),
             self.note.value(),
             self.alpha,
@@ -370,7 +367,6 @@ impl SaplingBuilder {
         &mut self,
         mut rng: R,
         extsk: &ExtendedSpendingKey,
-        diversifier: Diversifier,
         note: Note,
         merkle_path: MerklePath,
     ) -> Result<(), Error> {
@@ -388,8 +384,7 @@ impl SaplingBuilder {
         self.value_balance = (self.value_balance + note.value()).ok_or(Error::InvalidAmount)?;
         self.try_value_balance::<i64>()?;
 
-        let spend =
+        let spend = SpendDescriptionInfo::new_internal(&mut rng, extsk, note, merkle_path);
-            SpendDescriptionInfo::new_internal(&mut rng, extsk, diversifier, note, merkle_path);
 
         self.spends.push(spend);
 
@@ -881,7 +876,6 @@ pub mod testing {
         testing::{arb_node, arb_note},
         value::testing::arb_positive_note_value,
         zip32::testing::arb_extended_spending_key,
-        Diversifier,
     };
     use incrementalmerkletree::{
         frontier::testing::arb_commitment_tree, witness::IncrementalWitness,
@@ -907,34 +901,20 @@ pub mod testing {
                             .prop_map(|t| IncrementalWitness::from_tree(t).path().unwrap()),
                         n_notes,
                     ),
-                    vec(
-                        prop::array::uniform11(any::<u8>()).prop_map(Diversifier),
-                        n_notes,
-                    ),
                     prop::array::uniform32(any::<u8>()),
                     prop::array::uniform32(any::<u8>()),
                 )
             })
             .prop_map(
-                move |(
+                move |(extsk, spendable_notes, commitment_trees, rng_seed, fake_sighash_bytes)| {
-                    extsk,
-                    spendable_notes,
-                    commitment_trees,
-                    diversifiers,
-                    rng_seed,
-                    fake_sighash_bytes,
-                )| {
                     let mut builder = SaplingBuilder::new(zip212_enforcement);
                     let mut rng = StdRng::from_seed(rng_seed);
 
-                    for ((note, path), diversifier) in spendable_notes
+                    for (note, path) in spendable_notes
                         .into_iter()
                         .zip(commitment_trees.into_iter())
-                        .zip(diversifiers.into_iter())
                     {
-                        builder
+                        builder.add_spend(&mut rng, &extsk, note, path).unwrap();
-                            .add_spend(&mut rng, &extsk, diversifier, note, path)
-                            .unwrap();
                     }
 
                     let (bundle, _) = builder

zcash_primitives/src/transaction/builder.rs

@@ -15,7 +15,7 @@ use crate::{
         self,
         builder::{self as sapling_builder, SaplingBuilder, SaplingMetadata},
         prover::{OutputProver, SpendProver},
-        redjubjub, Diversifier, Note, PaymentAddress,
+        redjubjub, Note, PaymentAddress,
     },
     transaction::{
         components::{
@@ -328,12 +328,11 @@ impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> {
     pub fn add_sapling_spend(
         &mut self,
         extsk: sapling::zip32::ExtendedSpendingKey,
-        diversifier: Diversifier,
         note: Note,
         merkle_path: sapling::MerklePath,
     ) -> Result<(), sapling_builder::Error> {
         self.sapling_builder
-            .add_spend(&mut self.rng, &extsk, diversifier, note, merkle_path)?;
+            .add_spend(&mut self.rng, &extsk, note, merkle_path)?;
 
         self.sapling_asks
             .push(redjubjub::PrivateKey(extsk.expsk.ask));
@@ -842,7 +841,7 @@ mod tests {
 
         // Create a tx with a sapling spend. binding_sig should be present
         builder
-            .add_sapling_spend(extsk, *to.diversifier(), note1, witness1.path().unwrap())
+            .add_sapling_spend(extsk, note1, witness1.path().unwrap())
             .unwrap();
 
         builder
@@ -933,12 +932,7 @@ mod tests {
         {
             let mut builder = Builder::new(TEST_NETWORK, tx_height, None);
             builder
-                .add_sapling_spend(
+                .add_sapling_spend(extsk.clone(), note1.clone(), witness1.path().unwrap())
-                    extsk.clone(),
-                    *to.diversifier(),
-                    note1.clone(),
-                    witness1.path().unwrap(),
-                )
                 .unwrap();
             builder
                 .add_sapling_output(
@@ -974,15 +968,10 @@ mod tests {
         {
             let mut builder = Builder::new(TEST_NETWORK, tx_height, None);
             builder
-                .add_sapling_spend(
+                .add_sapling_spend(extsk.clone(), note1, witness1.path().unwrap())
-                    extsk.clone(),
-                    *to.diversifier(),
-                    note1,
-                    witness1.path().unwrap(),
-                )
                 .unwrap();
             builder
-                .add_sapling_spend(extsk, *to.diversifier(), note2, witness2.path().unwrap())
+                .add_sapling_spend(extsk, note2, witness2.path().unwrap())
                 .unwrap();
             builder
                 .add_sapling_output(