zcash_primitives: Add Orchard bundle metadata to transaction builder.

Cargo.toml

@@ -116,4 +116,4 @@ codegen-units = 1
 
 [patch.crates-io]
 sapling = { package = "sapling-crypto", git = "https://github.com/zcash/sapling-crypto.git", rev = "4ec6a48daab0af1fe6cb930f6a150030ce91d0e9" }
-orchard = { git = "https://github.com/zcash/orchard.git", rev = "189257391a5726d7f3f2568d547a5ef8603c7156" }
+orchard = { git = "https://github.com/zcash/orchard.git", rev = "9a85034ce932ca398da16529482e5efecc474c50" }

zcash_client_backend/src/data_api/wallet.rs

@@ -751,35 +751,38 @@ where
     }
 
     // Build the transaction with the specified fee rule
-    let (tx, sapling_build_meta) =
-        builder.build(OsRng, spend_prover, output_prover, proposal.fee_rule())?;
+    let build_result = builder.build(OsRng, spend_prover, output_prover, proposal.fee_rule())?;
 
     let internal_ivk = PreparedIncomingViewingKey::new(&dfvk.to_ivk(Scope::Internal));
     let sapling_outputs =
         sapling_output_meta
             .into_iter()
             .enumerate()
             .map(|(i, (recipient, value, memo))| {
-                let output_index = sapling_build_meta
+                let output_index = build_result
+                    .sapling_meta()
                     .output_index(i)
-                    .expect("An output should exist in the transaction for each shielded payment.");
+                    .expect("An output should exist in the transaction for each Sapling payment.");
 
                 let received_as = if let Recipient::InternalAccount(
                     account,
                     PoolType::Shielded(ShieldedProtocol::Sapling),
                 ) = recipient
                 {
-                    tx.sapling_bundle().and_then(|bundle| {
-                        try_sapling_note_decryption(
-                            &internal_ivk,
-                            &bundle.shielded_outputs()[output_index],
-                            consensus::sapling_zip212_enforcement(
-                                params,
-                                proposal.min_target_height(),
-                            ),
-                        )
-                        .map(|(note, _, _)| (account, note))
-                    })
+                    build_result
+                        .transaction()
+                        .sapling_bundle()
+                        .and_then(|bundle| {
+                            try_sapling_note_decryption(
+                                &internal_ivk,
+                                &bundle.shielded_outputs()[output_index],
+                                consensus::sapling_zip212_enforcement(
+                                    params,
+                                    proposal.min_target_height(),
+                                ),
+                            )
+                            .map(|(note, _, _)| (account, note))
+                        })
                 } else {
                     None
                 };
@@ -789,7 +792,8 @@ where
 
     let transparent_outputs = transparent_output_meta.into_iter().map(|(addr, value)| {
         let script = addr.script();
-        let output_index = tx
+        let output_index = build_result
+            .transaction()
             .transparent_bundle()
             .and_then(|b| {
                 b.vout
@@ -811,7 +815,7 @@ where
 
     wallet_db
         .store_sent_tx(&SentTransaction {
-            tx: &tx,
+            tx: build_result.transaction(),
             created: time::OffsetDateTime::now_utc(),
             account,
             outputs: sapling_outputs.chain(transparent_outputs).collect(),
@@ -821,7 +825,7 @@ where
         })
         .map_err(Error::DataSource)?;
 
-    Ok(tx.txid())
+    Ok(build_result.transaction().txid())
 }
 
 /// Constructs a transaction that consumes available transparent UTXOs belonging to

zcash_extensions/src/transparent/demo.rs

@@ -846,37 +846,43 @@ mod tests {
             .demo_open(value.into(), h1)
             .map_err(|e| format!("open failure: {:?}", e))
             .unwrap();
-        let (tx_a, _) = builder_a
+        let res_a = builder_a
             .txn_builder
             .build_zfuture(OsRng, &prover, &prover, &fee_rule)
             .map_err(|e| format!("build failure: {:?}", e))
             .unwrap();
-        let tze_a = tx_a.tze_bundle().unwrap();
+        let tze_a = res_a.transaction().tze_bundle().unwrap();
 
         //
         // Transfer
         //
 
         let mut builder_b = demo_builder(tx_height + 1, sapling::Anchor::empty_tree());
-        let prevout_a = (OutPoint::new(tx_a.txid(), 0), tze_a.vout[0].clone());
+        let prevout_a = (
+            OutPoint::new(res_a.transaction().txid(), 0),
+            tze_a.vout[0].clone(),
+        );
         let value_xfr = (value - fee_rule.fixed_fee()).unwrap();
         builder_b
             .demo_transfer_to_close(prevout_a, value_xfr.into(), preimage_1, h2)
             .map_err(|e| format!("transfer failure: {:?}", e))
             .unwrap();
-        let (tx_b, _) = builder_b
+        let res_b = builder_b
             .txn_builder
             .build_zfuture(OsRng, &prover, &prover, &fee_rule)
             .map_err(|e| format!("build failure: {:?}", e))
             .unwrap();
-        let tze_b = tx_b.tze_bundle().unwrap();
+        let tze_b = res_b.transaction().tze_bundle().unwrap();
 
         //
         // Closing transaction
         //
 
         let mut builder_c = demo_builder(tx_height + 2, sapling::Anchor::empty_tree());
-        let prevout_b = (OutPoint::new(tx_a.txid(), 0), tze_b.vout[0].clone());
+        let prevout_b = (
+            OutPoint::new(res_a.transaction().txid(), 0),
+            tze_b.vout[0].clone(),
+        );
         builder_c
             .demo_close(prevout_b, preimage_2)
             .map_err(|e| format!("close failure: {:?}", e))
@@ -889,22 +895,26 @@ mod tests {
             )
             .unwrap();
 
-        let (tx_c, _) = builder_c
+        let res_c = builder_c
             .txn_builder
             .build_zfuture(OsRng, &prover, &prover, &fee_rule)
             .map_err(|e| format!("build failure: {:?}", e))
             .unwrap();
-        let tze_c = tx_c.tze_bundle().unwrap();
+        let tze_c = res_c.transaction().tze_bundle().unwrap();
 
         // Verify tx_b
-        let ctx0 = Ctx { tx: &tx_b };
+        let ctx0 = Ctx {
+            tx: res_b.transaction(),
+        };
         assert_eq!(
             Program.verify(&tze_a.vout[0].precondition, &tze_b.vin[0].witness, &ctx0),
             Ok(())
         );
 
         // Verify tx_c
-        let ctx1 = Ctx { tx: &tx_c };
+        let ctx1 = Ctx {
+            tx: res_c.transaction(),
+        };
         assert_eq!(
             Program.verify(&tze_b.vout[0].precondition, &tze_c.vin[0].witness, &ctx1),
             Ok(())

zcash_primitives/CHANGELOG.md

@@ -10,7 +10,7 @@ and this library adheres to Rust's notion of
 - Dependency on `bellman 0.14`.
 - `zcash_primitives::consensus::sapling_zip212_enforcement`
 - `zcash_primitives::transaction`:
-  - `builder::{BuildConfig, FeeError, get_fee}`
+  - `builder::{BuildConfig, FeeError, get_fee, BuildResult}`
   - `builder::Error::SaplingBuilderNotAvailable`
   - `components::sapling`:
     - Sapling bundle component parsers, behind the `temporary-zcashd` feature
@@ -78,6 +78,8 @@ and this library adheres to Rust's notion of
     `Builder` typed on the provided channel.
   - `builder::Builder::get_fee` now returns a `builder::FeeError` instead of the
     bare `FeeRule::Error` when returning `Err`.
+  - `builder::Builder::build` now returns a `Result<BuildResult, ...>` instead of 
+    using a tuple to return the constructed transaction and build metadata.
   - `builder::Error::OrchardAnchorNotAvailable` has been renamed to
     `OrchardBuilderNotAvailable`.
   - `builder::{build, build_zfuture}` each now take an additional `rng` argument.

zcash_primitives/src/transaction/builder.rs

@@ -239,6 +239,35 @@ impl BuildConfig {
     }
 }
 
+/// The result of a transaction build operation, which includes the resulting transaction along
+/// with metadata describing how spends and outputs were shuffled in creating the transaction's
+/// shielded bundles.
+#[derive(Debug)]
+pub struct BuildResult {
+    transaction: Transaction,
+    sapling_meta: SaplingMetadata,
+    orchard_meta: orchard::builder::BundleMetadata,
+}
+
+impl BuildResult {
+    /// Returns the transaction that was constructed by the builder.
+    pub fn transaction(&self) -> &Transaction {
+        &self.transaction
+    }
+
+    /// Returns the mapping from Sapling inputs and outputs to their randomized positions in the
+    /// Sapling bundle in the newly constructed transaction.
+    pub fn sapling_meta(&self) -> &SaplingMetadata {
+        &self.sapling_meta
+    }
+
+    /// Returns the mapping from Orchard inputs and outputs to the randomized positions of the
+    /// Actions that contain them in the Orchard bundle in the newly constructed transaction.
+    pub fn orchard_meta(&self) -> &orchard::builder::BundleMetadata {
+        &self.orchard_meta
+    }
+}
+
 /// Generates a [`Transaction`] from its inputs and outputs.
 pub struct Builder<'a, P, U: sapling::builder::ProverProgress> {
     params: P,
@@ -602,7 +631,7 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder<
         spend_prover: &SP,
         output_prover: &OP,
         fee_rule: &FR,
-    ) -> Result<(Transaction, SaplingMetadata), Error<FR::Error>> {
+    ) -> Result<BuildResult, Error<FR::Error>> {
         let fee = self.get_fee(fee_rule).map_err(Error::Fee)?;
         self.build_internal(rng, spend_prover, output_prover, fee)
     }
@@ -623,7 +652,7 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder<
         spend_prover: &SP,
         output_prover: &OP,
         fee_rule: &FR,
-    ) -> Result<(Transaction, SaplingMetadata), Error<FR::Error>> {
+    ) -> Result<BuildResult, Error<FR::Error>> {
         let fee = self.get_fee_zfuture(fee_rule).map_err(Error::Fee)?;
         self.build_internal(rng, spend_prover, output_prover, fee)
     }
@@ -634,7 +663,7 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder<
         spend_prover: &SP,
         output_prover: &OP,
         fee: NonNegativeAmount,
-    ) -> Result<(Transaction, SaplingMetadata), Error<FE>> {
+    ) -> Result<BuildResult, Error<FE>> {
         let consensus_branch_id = BranchId::for_height(&self.params, self.target_height);
 
         // determine transaction version
@@ -660,15 +689,15 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder<
 
         let transparent_bundle = self.transparent_builder.build();
 
-        let (sapling_bundle, tx_metadata) = match self
+        let (sapling_bundle, sapling_meta) = match self
             .sapling_builder
             .and_then(|builder| {
                 builder
                     .build::<SP, OP, _, _>(&mut rng)
                     .map_err(Error::SaplingBuild)
                     .transpose()
                     .map(|res| {
-                        res.map(|(bundle, tx_metadata)| {
+                        res.map(|(bundle, sapling_meta)| {
                             // We need to create proofs before signatures, because we still support
                             // creating V4 transactions, which commit to the Sapling proofs in the
                             // transaction digest.
@@ -679,7 +708,7 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder<
                                     &mut rng,
                                     self.progress_notifier,
                                 ),
-                                tx_metadata,
+                                sapling_meta,
                             )
                         })
                     })
@@ -690,15 +719,19 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder<
             None => (None, SaplingMetadata::empty()),
         };
 
-        let orchard_bundle: Option<orchard::Bundle<_, Amount>> = self
+        let (orchard_bundle, orchard_meta) = match self
             .orchard_builder
             .and_then(|builder| {
                 builder
                     .build(&mut rng)
                     .map_err(Error::OrchardBuild)
                     .transpose()
             })
-            .transpose()?;
+            .transpose()?
+        {
+            Some((bundle, meta)) => (Some(bundle), meta),
+            None => (None, orchard::builder::BundleMetadata::empty()),
+        };
 
         #[cfg(feature = "zfuture")]
         let (tze_bundle, tze_signers) = self.tze_builder.build();
@@ -786,7 +819,11 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder<
 
         // The unwrap() here is safe because the txid hashing
         // of freeze() should be infalliable.
-        Ok((authorized_tx.freeze().unwrap(), tx_metadata))
+        Ok(BuildResult {
+            transaction: authorized_tx.freeze().unwrap(),
+            sapling_meta,
+            orchard_meta,
+        })
     }
 }
 
@@ -829,24 +866,20 @@ mod testing {
     use rand_core::CryptoRng;
     use std::convert::Infallible;
 
-    use super::{Builder, Error, SaplingMetadata};
+    use super::{BuildResult, Builder, Error};
     use crate::{
         consensus,
         sapling::{
             self,
             prover::mock::{MockOutputProver, MockSpendProver},
         },
         transaction::fees::fixed,
-        transaction::Transaction,
     };
 
     impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder<'a, P, U> {
         /// Build the transaction using mocked randomness and proving capabilities.
         /// DO NOT USE EXCEPT FOR UNIT TESTING.
-        pub fn mock_build<R: RngCore>(
-            self,
-            rng: R,
-        ) -> Result<(Transaction, SaplingMetadata), Error<Infallible>> {
+        pub fn mock_build<R: RngCore>(self, rng: R) -> Result<BuildResult, Error<Infallible>> {
             struct FakeCryptoRng<R: RngCore>(R);
             impl<R: RngCore> CryptoRng for FakeCryptoRng<R> {}
             impl<R: RngCore> RngCore for FakeCryptoRng<R> {
@@ -971,9 +1004,9 @@ mod tests {
             )
             .unwrap();
 
-        let (tx, _) = builder.mock_build(OsRng).unwrap();
+        let res = builder.mock_build(OsRng).unwrap();
         // No binding signature, because only t input and outputs
-        assert!(tx.sapling_bundle.is_none());
+        assert!(res.transaction().sapling_bundle.is_none());
     }
 
     #[test]
@@ -1016,8 +1049,8 @@ mod tests {
             .unwrap();
 
         // A binding signature (and bundle) is present because there is a Sapling spend.
-        let (tx, _) = builder.mock_build(OsRng).unwrap();
-        assert!(tx.sapling_bundle().is_some());
+        let res = builder.mock_build(OsRng).unwrap();
+        assert!(res.transaction().sapling_bundle().is_some());
     }
 
     #[test]
@@ -1173,7 +1206,7 @@ mod tests {
                 .unwrap();
             assert_matches!(
                 builder.mock_build(OsRng),
-                Ok((tx, _)) if tx.fee_paid(|_| Err(BalanceError::Overflow)).unwrap() == Amount::const_from_i64(10_000)
+                Ok(res) if res.transaction().fee_paid(|_| Err(BalanceError::Overflow)).unwrap() == Amount::const_from_i64(10_000)
             );
         }
     }