diff --git a/Cargo.lock b/Cargo.lock index ef95098a0..c3f28d2dd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1476,7 +1476,7 @@ checksum = "624a8340c38c1b80fd549087862da4ba43e08858af025b236e509b6649fc13d5" [[package]] name = "orchard" version = "0.6.0" -source = "git+https://github.com/zcash/orchard.git?rev=189257391a5726d7f3f2568d547a5ef8603c7156#189257391a5726d7f3f2568d547a5ef8603c7156" +source = "git+https://github.com/zcash/orchard.git?rev=9a85034ce932ca398da16529482e5efecc474c50#9a85034ce932ca398da16529482e5efecc474c50" dependencies = [ "aes", "bitvec", diff --git a/Cargo.toml b/Cargo.toml index c06009d87..8763ef733 100644 --- a/Cargo.toml +++ b/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" } diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 18dd96eda..535b17ccf 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -751,8 +751,7 @@ 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 = @@ -760,26 +759,30 @@ where .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 diff --git a/zcash_extensions/src/transparent/demo.rs b/zcash_extensions/src/transparent/demo.rs index d3c295ae8..794b93530 100644 --- a/zcash_extensions/src/transparent/demo.rs +++ b/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(()) diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index ddb6765ef..39a07cee6 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/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` 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. diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index 1babaf201..5c9f05398 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/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> { + ) -> Result> { 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> { + ) -> Result> { 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> { + ) -> Result> { let consensus_branch_id = BranchId::for_height(&self.params, self.target_height); // determine transaction version @@ -660,7 +689,7 @@ 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 @@ -668,7 +697,7 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder< .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,7 +719,7 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder< None => (None, SaplingMetadata::empty()), }; - let orchard_bundle: Option> = self + let (orchard_bundle, orchard_meta) = match self .orchard_builder .and_then(|builder| { builder @@ -698,7 +727,11 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder< .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,7 +866,7 @@ mod testing { use rand_core::CryptoRng; use std::convert::Infallible; - use super::{Builder, Error, SaplingMetadata}; + use super::{BuildResult, Builder, Error}; use crate::{ consensus, sapling::{ @@ -837,16 +874,12 @@ mod testing { 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( - self, - rng: R, - ) -> Result<(Transaction, SaplingMetadata), Error> { + pub fn mock_build(self, rng: R) -> Result> { struct FakeCryptoRng(R); impl CryptoRng for FakeCryptoRng {} impl RngCore for FakeCryptoRng { @@ -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) ); } }