From d36cf0371ccf384c2720cfc008ada9706632a22f Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 19 Dec 2023 20:37:52 -0700 Subject: [PATCH 1/9] Remove unused type parameter from `SaplingBuilder::add_output` --- src/builder.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 22fd55e..aab8544 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -441,8 +441,7 @@ impl SaplingBuilder { } /// Adds a Sapling address to send funds to. - #[allow(clippy::too_many_arguments)] - pub fn add_output( + pub fn add_output( &mut self, ovk: Option, to: PaymentAddress, From 485966bc816e6436522ebac4a97b3af0c8d20699 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 19 Dec 2023 20:39:56 -0700 Subject: [PATCH 2/9] Rename `SaplingBuilder` to `Builder` for consitency with the Orchard API. --- CHANGELOG.md | 13 +++++++------ src/builder.rs | 26 ++++++++++++-------------- src/value.rs | 8 ++++---- 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dfeccc..aa45607 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,21 +91,22 @@ The entries below are relative to the `zcash_primitives::sapling` module as of - `sapling_crypto::address::PaymentAddress::create_note` now takes its `value` argument as a `NoteValue` instead of as a bare `u64`. - `sapling_crypto::builder`: - - `SaplingBuilder` no longer has a `P: zcash_primitives::consensus::Parameters` + - `SaplingBuilder` has been renamed to `Builder` + - `Builder` no longer has a `P: zcash_primitives::consensus::Parameters` type parameter. - - `SaplingBuilder::new` now takes a `Zip212Enforcement` argument instead of a + - `Builder::new` now takes a `Zip212Enforcement` argument instead of a `P: zcash_primitives::consensus::Parameters` argument and a target height. - - `SaplingBuilder::add_spend` now takes `extsk` by reference. Also, it no + - `Builder::add_spend` now takes `extsk` by reference. Also, it no longer takes a `diversifier` argument as the diversifier may be obtained from the note. - - `SaplingBuilder::add_output` now takes an `Option<[u8; 512]>` memo instead + - `Builder::add_output` now takes an `Option<[u8; 512]>` memo instead of a `MemoBytes`. - - `SaplingBuilder::build` no longer takes a prover, proving context, progress + - `Builder::build` no longer takes a prover, proving context, progress notifier, or target height. Instead, it has `SpendProver, OutputProver` generic parameters and returns `(UnauthorizedBundle, SaplingMetadata)`. The caller can then use `Bundle::>::create_proofs` to create spend and output proofs for the bundle. - - `SaplingBuilder::build` now takes a `BundleType` argument that instructs + - `Builder::build` now takes a `BundleType` argument that instructs it how to pad the bundle with dummy outputs. - `Error` has new error variants: - `Error::DuplicateSignature` diff --git a/src/builder.rs b/src/builder.rs index aab8544..af95b12 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -323,7 +323,7 @@ impl PreparedOutputInfo { } } -/// Metadata about a transaction created by a [`SaplingBuilder`]. +/// Metadata about a transaction created by a [`Builder`]. #[derive(Debug, Clone, PartialEq, Eq)] pub struct SaplingMetadata { spend_indices: Vec, @@ -339,22 +339,22 @@ impl SaplingMetadata { } /// Returns the index within the transaction of the [`SpendDescription`] corresponding - /// to the `n`-th call to [`SaplingBuilder::add_spend`]. + /// to the `n`-th call to [`Builder::add_spend`]. /// /// Note positions are randomized when building transactions for indistinguishability. /// This means that the transaction consumer cannot assume that e.g. the first spend - /// they added (via the first call to [`SaplingBuilder::add_spend`]) is the first + /// they added (via the first call to [`Builder::add_spend`]) is the first /// [`SpendDescription`] in the transaction. pub fn spend_index(&self, n: usize) -> Option { self.spend_indices.get(n).copied() } /// Returns the index within the transaction of the [`OutputDescription`] corresponding - /// to the `n`-th call to [`SaplingBuilder::add_output`]. + /// to the `n`-th call to [`Builder::add_output`]. /// /// Note positions are randomized when building transactions for indistinguishability. /// This means that the transaction consumer cannot assume that e.g. the first output - /// they added (via the first call to [`SaplingBuilder::add_output`]) is the first + /// they added (via the first call to [`Builder::add_output`]) is the first /// [`OutputDescription`] in the transaction. pub fn output_index(&self, n: usize) -> Option { self.output_indices.get(n).copied() @@ -362,7 +362,7 @@ impl SaplingMetadata { } /// A mutable builder type for constructing Sapling bundles. -pub struct SaplingBuilder { +pub struct Builder { value_balance: ValueSum, spends: Vec, outputs: Vec, @@ -370,9 +370,9 @@ pub struct SaplingBuilder { bundle_type: BundleType, } -impl SaplingBuilder { +impl Builder { pub fn new(zip212_enforcement: Zip212Enforcement, bundle_type: BundleType) -> Self { - SaplingBuilder { + Builder { value_balance: ValueSum::zero(), spends: vec![], outputs: vec![], @@ -606,7 +606,7 @@ pub fn bundle>( /// Type alias for an in-progress bundle that has no proofs or signatures. /// -/// This is returned by [`SaplingBuilder::build`]. +/// This is returned by [`Builder::build`]. pub type UnauthorizedBundle = Bundle, V>; /// Marker trait representing bundle proofs in the process of being created. @@ -975,7 +975,7 @@ pub mod testing { frontier::testing::arb_commitment_tree, witness::IncrementalWitness, Hashable, Level, }; - use super::{BundleType, SaplingBuilder}; + use super::{Builder, BundleType}; #[allow(dead_code)] fn arb_bundle>( @@ -1011,10 +1011,8 @@ pub mod testing { Node::from_scalar(*tree.root(node).inner()) }, ); - let mut builder = SaplingBuilder::new( - zip212_enforcement, - BundleType::Transactional { anchor }, - ); + let mut builder = + Builder::new(zip212_enforcement, BundleType::Transactional { anchor }); let mut rng = StdRng::from_seed(rng_seed); for (note, path) in spendable_notes diff --git a/src/value.rs b/src/value.rs index f0d3406..474e075 100644 --- a/src/value.rs +++ b/src/value.rs @@ -8,7 +8,7 @@ //! single [`Bundle`]. //! - `valueBalanceSapling`, which is a signed 63-bit integer. This is represented //! by a user-defined type parameter on [`Bundle`], returned by -//! [`Bundle::value_balance`] and [`SaplingBuilder::value_balance`]. +//! [`Bundle::value_balance`] and [`Builder::value_balance`]. //! //! If your specific instantiation of the Sapling protocol requires a smaller bound on //! valid note values (for example, Zcash's `MAX_MONEY` fits into a 51-bit integer), you @@ -17,7 +17,7 @@ //! - Define your `valueBalanceSapling` type to enforce your valid value range. This can //! be checked in its `TryFrom` implementation. //! - Define your own "amount" type for note values, and convert it to `NoteValue` prior -//! to calling [`SaplingBuilder::add_output`]. +//! to calling [`Builder::add_output`]. //! //! Inside the circuit, note values are constrained to be unsigned 64-bit integers. //! @@ -33,8 +33,8 @@ //! //! [`Bundle`]: crate::Bundle //! [`Bundle::value_balance`]: crate::Bundle::value_balance -//! [`SaplingBuilder::value_balance`]: crate::builder::SaplingBuilder::value_balance -//! [`SaplingBuilder::add_output`]: crate::builder::SaplingBuilder::add_output +//! [`Builder::value_balance`]: crate::builder::Builder::value_balance +//! [`Builder::add_output`]: crate::builder::Builder::add_output //! [Rust documentation]: https://doc.rust-lang.org/stable/std/primitive.i64.html use bitvec::{array::BitArray, order::Lsb0}; From 43d4133af47176a4a5ea052cb7dce9a8f02f2d7a Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 20 Dec 2023 19:13:27 -0700 Subject: [PATCH 3/9] Modify `BundleType` to exclude the anchor & allow no bundle to be produced. This adds a flag to `BundleType` that, when set, requires a dummy outputs to be produced even if no outputs are added to the builder, and when unset results in standard padding. --- CHANGELOG.md | 5 ++++- src/builder.rs | 55 +++++++++++++++++++++++++++++++++++--------------- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa45607..c8fcaa2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,9 +96,12 @@ The entries below are relative to the `zcash_primitives::sapling` module as of type parameter. - `Builder::new` now takes a `Zip212Enforcement` argument instead of a `P: zcash_primitives::consensus::Parameters` argument and a target height. + It also now takes as an argument the Sapling anchor to be used for all + spends in the bundle. - `Builder::add_spend` now takes `extsk` by reference. Also, it no longer takes a `diversifier` argument as the diversifier may be obtained - from the note. + from the note. All calls to `add_spend` are now required to use an anchor + that corresponds to the anchor provided at builder construction. - `Builder::add_output` now takes an `Option<[u8; 512]>` memo instead of a `MemoBytes`. - `Builder::build` no longer takes a prover, proving context, progress diff --git a/src/builder.rs b/src/builder.rs index af95b12..74671a7 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -34,12 +34,24 @@ const MIN_SHIELDED_OUTPUTS: usize = 2; pub enum BundleType { /// A transactional bundle will be padded if necessary to contain at least 2 outputs, /// irrespective of whether any genuine outputs are required. - Transactional { anchor: Node }, + Transactional { + /// A flag that, when set to `true`, indicates that the resulting bundle should be produced + /// with the minimum required outputs irrespective of whether any outputs have been + /// requested; if no explicit outputs have been added, all of the outputs in the resulting + /// bundle will be dummies. + outputs_required: bool, + }, /// A coinbase bundle is required to have no spends. No output padding is performed. Coinbase, } impl BundleType { + /// The default bundle type has all flags enabled, and does not require a bundle to be produced + /// if no spends or outputs have been added to the bundle. + pub const DEFAULT: BundleType = BundleType::Transactional { + outputs_required: false, + }; + /// Returns the number of logical outputs that a builder will produce in constructing a bundle /// of this type, given the specified numbers of spends and outputs. /// @@ -51,8 +63,12 @@ impl BundleType { num_outputs: usize, ) -> Result { match self { - BundleType::Transactional { .. } => { - Ok(core::cmp::max(num_outputs, MIN_SHIELDED_OUTPUTS)) + BundleType::Transactional { outputs_required } => { + Ok(if *outputs_required || num_outputs > 0 { + core::cmp::max(num_outputs, MIN_SHIELDED_OUTPUTS) + } else { + 0 + }) } BundleType::Coinbase => { if num_spends == 0 { @@ -130,12 +146,12 @@ impl SpendInfo { self.note.value() } - fn has_matching_anchor(&self, anchor: Node) -> bool { + fn has_matching_anchor(&self, anchor: &Node) -> bool { if self.note.value() == NoteValue::ZERO { true } else { let node = Node::from_cmu(&self.note.cmu()); - self.merkle_path.root(node) == anchor + &self.merkle_path.root(node) == anchor } } @@ -368,16 +384,22 @@ pub struct Builder { outputs: Vec, zip212_enforcement: Zip212Enforcement, bundle_type: BundleType, + anchor: Node, } impl Builder { - pub fn new(zip212_enforcement: Zip212Enforcement, bundle_type: BundleType) -> Self { + pub fn new( + zip212_enforcement: Zip212Enforcement, + bundle_type: BundleType, + anchor: Node, + ) -> Self { Builder { value_balance: ValueSum::zero(), spends: vec![], outputs: vec![], zip212_enforcement, bundle_type, + anchor, } } @@ -422,8 +444,8 @@ impl Builder { // Consistency check: all anchors must equal the first one match self.bundle_type { - BundleType::Transactional { anchor } => { - if !spend.has_matching_anchor(anchor) { + BundleType::Transactional { .. } => { + if !spend.has_matching_anchor(&self.anchor) { return Err(Error::AnchorMismatch); } } @@ -465,10 +487,11 @@ impl Builder { ) -> Result, SaplingMetadata)>, Error> { bundle::( rng, - self.spends, - self.outputs, self.bundle_type, self.zip212_enforcement, + self.anchor, + self.spends, + self.outputs, ) } } @@ -477,15 +500,16 @@ impl Builder { /// and outputs. pub fn bundle>( mut rng: R, - spends: Vec, - outputs: Vec, bundle_type: BundleType, zip212_enforcement: Zip212Enforcement, + anchor: Node, + spends: Vec, + outputs: Vec, ) -> Result, SaplingMetadata)>, Error> { match bundle_type { - BundleType::Transactional { anchor } => { + BundleType::Transactional { .. } => { for spend in &spends { - if !spend.has_matching_anchor(anchor) { + if !spend.has_matching_anchor(&anchor) { return Err(Error::AnchorMismatch); } } @@ -1011,8 +1035,7 @@ pub mod testing { Node::from_scalar(*tree.root(node).inner()) }, ); - let mut builder = - Builder::new(zip212_enforcement, BundleType::Transactional { anchor }); + let mut builder = Builder::new(zip212_enforcement, BundleType::DEFAULT, anchor); let mut rng = StdRng::from_seed(rng_seed); for (note, path) in spendable_notes From 954a27ee9b274413465e2da838bb9740eb4c997b Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 20 Dec 2023 20:36:21 -0700 Subject: [PATCH 4/9] Add `Anchor` type for symmetry with Orchard. --- CHANGELOG.md | 1 + src/builder.rs | 20 ++++++++++---------- src/lib.rs | 3 ++- src/tree.rs | 37 +++++++++++++++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8fcaa2..b4b3a49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The entries below are relative to the `zcash_primitives::sapling` module as of `zcash_primitives 0.13.0`. ### Added +- `sapling_crypto::Anchor` - `sapling_crypto::BatchValidator` (moved from `zcash_proofs::sapling`). - `sapling_crypto::SaplingVerificationContext` (moved from `zcash_proofs::sapling`). diff --git a/src/builder.rs b/src/builder.rs index 74671a7..a94e7df 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -22,7 +22,7 @@ use crate::{ CommitmentSum, NoteValue, TrapdoorSum, ValueCommitTrapdoor, ValueCommitment, ValueSum, }, zip32::ExtendedSpendingKey, - Diversifier, MerklePath, Node, Note, PaymentAddress, ProofGenerationKey, SaplingIvk, + Anchor, Diversifier, MerklePath, Node, Note, PaymentAddress, ProofGenerationKey, SaplingIvk, }; /// If there are any shielded inputs, always have at least two shielded outputs, padding @@ -146,12 +146,12 @@ impl SpendInfo { self.note.value() } - fn has_matching_anchor(&self, anchor: &Node) -> bool { + fn has_matching_anchor(&self, anchor: &Anchor) -> bool { if self.note.value() == NoteValue::ZERO { true } else { let node = Node::from_cmu(&self.note.cmu()); - &self.merkle_path.root(node) == anchor + &Anchor::from(self.merkle_path.root(node)) == anchor } } @@ -384,14 +384,14 @@ pub struct Builder { outputs: Vec, zip212_enforcement: Zip212Enforcement, bundle_type: BundleType, - anchor: Node, + anchor: Anchor, } impl Builder { pub fn new( zip212_enforcement: Zip212Enforcement, bundle_type: BundleType, - anchor: Node, + anchor: Anchor, ) -> Self { Builder { value_balance: ValueSum::zero(), @@ -502,7 +502,7 @@ pub fn bundle>( mut rng: R, bundle_type: BundleType, zip212_enforcement: Zip212Enforcement, - anchor: Node, + anchor: Anchor, spends: Vec, outputs: Vec, ) -> Result, SaplingMetadata)>, Error> { @@ -993,10 +993,10 @@ pub mod testing { testing::{arb_node, arb_note}, value::testing::arb_positive_note_value, zip32::testing::arb_extended_spending_key, - Node, NOTE_COMMITMENT_TREE_DEPTH, + Anchor, Node, }; use incrementalmerkletree::{ - frontier::testing::arb_commitment_tree, witness::IncrementalWitness, Hashable, Level, + frontier::testing::arb_commitment_tree, witness::IncrementalWitness, }; use super::{Builder, BundleType}; @@ -1029,10 +1029,10 @@ pub mod testing { .first() .zip(commitment_trees.first()) .map_or_else( - || Node::empty_root(Level::from(NOTE_COMMITMENT_TREE_DEPTH)), + || Anchor::empty_tree(), |(note, tree)| { let node = Node::from_cmu(¬e.cmu()); - Node::from_scalar(*tree.root(node).inner()) + Anchor::from(*tree.root(node).inner()) }, ); let mut builder = Builder::new(zip212_enforcement, BundleType::DEFAULT, anchor); diff --git a/src/lib.rs b/src/lib.rs index 16d438b..5efd031 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -34,7 +34,8 @@ pub use bundle::Bundle; pub use keys::{Diversifier, NullifierDerivingKey, ProofGenerationKey, SaplingIvk, ViewingKey}; pub use note::{nullifier::Nullifier, Note, Rseed}; pub use tree::{ - merkle_hash, CommitmentTree, IncrementalWitness, MerklePath, Node, NOTE_COMMITMENT_TREE_DEPTH, + merkle_hash, Anchor, CommitmentTree, IncrementalWitness, MerklePath, Node, + NOTE_COMMITMENT_TREE_DEPTH, }; pub use verifier::{BatchValidator, SaplingVerificationContext}; diff --git a/src/tree.rs b/src/tree.rs index f98f499..61e5ac1 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -67,6 +67,43 @@ fn merkle_hash_field(depth: usize, lhs: &[u8; 32], rhs: &[u8; 32]) -> jubjub::Ba .get_u() } +/// The root of a Sapling commitment tree. +#[derive(Eq, PartialEq, Clone, Copy, Debug)] +pub struct Anchor(jubjub::Base); + +impl From for Anchor { + fn from(anchor_field: jubjub::Base) -> Anchor { + Anchor(anchor_field) + } +} + +impl From for Anchor { + fn from(anchor: Node) -> Anchor { + Anchor(anchor.0) + } +} + +impl Anchor { + /// The anchor of the empty Orchard note commitment tree. + /// + /// This anchor does not correspond to any valid anchor for a spend, so it + /// may only be used for coinbase bundles or in circumstances where Orchard + /// functionality is not active. + pub fn empty_tree() -> Anchor { + Anchor(Node::empty_root(NOTE_COMMITMENT_TREE_DEPTH.into()).0) + } + + /// Parses an Orchard anchor from a byte encoding. + pub fn from_bytes(bytes: [u8; 32]) -> CtOption { + jubjub::Base::from_repr(bytes).map(Self) + } + + /// Returns the byte encoding of this anchor. + pub fn to_bytes(self) -> [u8; 32] { + self.0.to_repr() + } +} + /// A node within the Sapling commitment tree. #[derive(Clone, Copy, PartialEq, Eq)] pub struct Node(jubjub::Base); From 6f02b62c8eef36dbcd203eb6193dccd09816508d Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 21 Dec 2023 16:08:51 -0700 Subject: [PATCH 5/9] Add the capability to generate dummy spends (internal to the Builder) --- CHANGELOG.md | 12 ++-- src/builder.rs | 157 +++++++++++++++++++++++++++++++------------------ src/bundle.rs | 87 ++++----------------------- src/note.rs | 27 +++++++++ 4 files changed, 147 insertions(+), 136 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4b3a49..ea7b776 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ The entries below are relative to the `zcash_primitives::sapling` module as of - `OutputInfo` - `ProverProgress` - `BundleType` + - `SigningMetadata` - `bundle` bundle builder function. - `sapling_crypto::bundle` module: - The following types moved from @@ -33,7 +34,7 @@ The entries below are relative to the `zcash_primitives::sapling` module as of - `Bundle` - `SpendDescription, SpendDescriptionV5` - `OutputDescription, OutputDescriptionV5` - - `Authorization, Authorized, MapAuth` + - `Authorization, Authorized` - `GrothProofBytes` - `Bundle::>::create_proofs` - `Bundle::>::prepare` @@ -42,9 +43,6 @@ The entries below are relative to the `zcash_primitives::sapling` module as of - `Bundle::>::apply_signatures` - `Bundle::try_map_authorization` - `TryMapAuth` - - `impl {MapAuth, TryMapAuth} for (FnMut, FnMut, FnMut, FnMut)` - helpers to enable calling `Bundle::{map_authorization, try_map_authorization}` - with a set of closures. - `testing` module, containing the following functions moved from `zcash_primitives::transaction::components::sapling::testing`: - `arb_output_description` @@ -93,6 +91,7 @@ The entries below are relative to the `zcash_primitives::sapling` module as of argument as a `NoteValue` instead of as a bare `u64`. - `sapling_crypto::builder`: - `SaplingBuilder` has been renamed to `Builder` + - `MaybeSigned::SigningMetadata` has been renamed to `MaybeSigned::SigningParts` - `Builder` no longer has a `P: zcash_primitives::consensus::Parameters` type parameter. - `Builder::new` now takes a `Zip212Enforcement` argument instead of a @@ -121,6 +120,9 @@ The entries below are relative to the `zcash_primitives::sapling` module as of - `Bundle` now has a second generic parameter `V`. - `Bundle::value_balance` now returns `&V` instead of `&zcash_primitives::transaction::components::Amount`. + - `Bundle::map_authorization` now takes a context argument and explicit + functions for each mappable field, rather than a `MapAuth` value, in + order to simplify handling of context values. - `Authorized::binding_sig` now has type `redjubjub::Signature`. - `Authorized::AuthSig` now has type `redjubjub::Signature`. - `SpendDescription::temporary_zcashd_from_parts` now takes `rk` as @@ -131,7 +133,6 @@ The entries below are relative to the `zcash_primitives::sapling` module as of `redjubjub::Signature` instead of `sapling_crypto::redjubjub::Signature`. - `testing::arb_bundle` now takes a `value_balance: V` argument. - - `MapAuth` trait methods now take `&mut self` instead of `&self`. - `sapling_crypto::circuit::ValueCommitmentOpening::value` is now represented as a `NoteValue` instead of as a bare `u64`. - `sapling_crypto::keys`: @@ -175,6 +176,7 @@ The entries below are relative to the `zcash_primitives::sapling` module as of - `OutputDescription::read` - `OutputDescription::{write_v4, write_v5_without_proof}` - `OutputDescriptionV5::read` + - `MapAuth` trait - `sapling_crypto::builder`: - `SpendDescriptionInfo` - `sapling_crypto::note_encryption::SaplingDomain::for_height` (use diff --git a/src/builder.rs b/src/builder.rs index a94e7df..4bcd98b 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -4,14 +4,14 @@ use core::fmt; use std::{iter, marker::PhantomData}; use group::ff::Field; +use incrementalmerkletree::Position; use rand::{seq::SliceRandom, RngCore}; use rand_core::CryptoRng; use redjubjub::{Binding, SpendAuth}; use crate::{ bundle::{ - Authorization, Authorized, Bundle, GrothProofBytes, MapAuth, OutputDescription, - SpendDescription, + Authorization, Authorized, Bundle, GrothProofBytes, OutputDescription, SpendDescription, }, circuit, keys::{OutgoingViewingKey, SpendAuthorizingKey, SpendValidatingKey}, @@ -23,6 +23,7 @@ use crate::{ }, zip32::ExtendedSpendingKey, Anchor, Diversifier, MerklePath, Node, Note, PaymentAddress, ProofGenerationKey, SaplingIvk, + NOTE_COMMITMENT_TREE_DEPTH, }; /// If there are any shielded inputs, always have at least two shielded outputs, padding @@ -39,7 +40,7 @@ pub enum BundleType { /// with the minimum required outputs irrespective of whether any outputs have been /// requested; if no explicit outputs have been added, all of the outputs in the resulting /// bundle will be dummies. - outputs_required: bool, + bundle_required: bool, }, /// A coinbase bundle is required to have no spends. No output padding is performed. Coinbase, @@ -49,7 +50,7 @@ impl BundleType { /// The default bundle type has all flags enabled, and does not require a bundle to be produced /// if no spends or outputs have been added to the bundle. pub const DEFAULT: BundleType = BundleType::Transactional { - outputs_required: false, + bundle_required: false, }; /// Returns the number of logical outputs that a builder will produce in constructing a bundle @@ -63,8 +64,8 @@ impl BundleType { num_outputs: usize, ) -> Result { match self { - BundleType::Transactional { outputs_required } => { - Ok(if *outputs_required || num_outputs > 0 { + BundleType::Transactional { bundle_required } => { + Ok(if *bundle_required || num_outputs > 0 { core::cmp::max(num_outputs, MIN_SHIELDED_OUTPUTS) } else { 0 @@ -125,6 +126,7 @@ pub struct SpendInfo { proof_generation_key: ProofGenerationKey, note: Note, merkle_path: MerklePath, + dummy_ask: Option, } impl SpendInfo { @@ -138,6 +140,7 @@ impl SpendInfo { proof_generation_key, note, merkle_path, + dummy_ask: None, } } @@ -146,6 +149,27 @@ impl SpendInfo { self.note.value() } + /// Defined in [Zcash Protocol Spec § 4.8.3: Dummy Notes (Sapling)][orcharddummynotes]. + /// + /// [orcharddummynotes]: https://zips.z.cash/protocol/nu5.pdf#orcharddummynotes + fn dummy(mut rng: R) -> Self { + let (sk, _, note) = Note::dummy(&mut rng); + let merkle_path = MerklePath::from_parts( + iter::repeat_with(|| Node::from_scalar(jubjub::Base::random(&mut rng))) + .take(NOTE_COMMITMENT_TREE_DEPTH.into()) + .collect(), + Position::from(rng.next_u64()), + ) + .expect("The path length corresponds to the length of the generated vector."); + + SpendInfo { + proof_generation_key: sk.proof_generation_key(), + note, + merkle_path, + dummy_ask: Some(sk.ask), + } + } + fn has_matching_anchor(&self, anchor: &Anchor) -> bool { if self.note.value() == NoteValue::ZERO { true @@ -161,6 +185,7 @@ impl SpendInfo { note: self.note, merkle_path: self.merkle_path, rcv: ValueCommitTrapdoor::random(rng), + dummy_ask: self.dummy_ask, } } } @@ -171,6 +196,7 @@ struct PreparedSpendInfo { note: Note, merkle_path: MerklePath, rcv: ValueCommitTrapdoor, + dummy_ask: Option, } impl PreparedSpendInfo { @@ -213,7 +239,10 @@ impl PreparedSpendInfo { nullifier, rk, zkproof, - SigningParts { ak, alpha }, + SigningMetadata { + dummy_ask: self.dummy_ask, + parts: SigningParts { ak, alpha }, + }, )) } } @@ -742,17 +771,7 @@ impl<'a, SP: SpendProver, OP: OutputProver, R: RngCore, U: ProverProgress> self.progress_notifier .update(self.progress, self.total_progress); } -} -impl< - 'a, - S: InProgressSignatures, - SP: SpendProver, - OP: OutputProver, - R: RngCore, - U: ProverProgress, - > MapAuth, InProgress> for CreateProofs<'a, SP, OP, R, U> -{ fn map_spend_proof(&mut self, spend: circuit::Spend) -> GrothProofBytes { let proof = self.spend_prover.create_proof(spend, &mut self.rng); self.update_progress(); @@ -765,11 +784,10 @@ impl< OP::encode_proof(proof) } - fn map_auth_sig(&mut self, s: S::AuthSig) -> S::AuthSig { - s - } - - fn map_authorization(&mut self, a: InProgress) -> InProgress { + fn map_authorization( + &mut self, + a: InProgress, + ) -> InProgress { InProgress { sigs: a.sigs, _proof_state: PhantomData::default(), @@ -788,13 +806,21 @@ impl Bundle, V> { ) -> Bundle, V> { let total_progress = self.shielded_spends().len() as u32 + self.shielded_outputs().len() as u32; - self.map_authorization(CreateProofs::new( + let mut cp = CreateProofs::new( spend_prover, output_prover, rng, progress_notifier, total_progress, - )) + ); + + self.map_authorization( + &mut cp, + |cp, spend| cp.map_spend_proof(spend), + |cp, output| cp.map_output_proof(output), + |_cp, sig| sig, + |cp, auth| cp.map_authorization(auth), + ) } } @@ -811,7 +837,7 @@ impl fmt::Debug for Unsigned { } impl InProgressSignatures for Unsigned { - type AuthSig = SigningParts; + type AuthSig = SigningMetadata; } /// The parts needed to sign a [`SpendDescription`]. @@ -831,6 +857,18 @@ pub struct PartiallyAuthorized { sighash: [u8; 32], } +/// Container for metadata needed to sign a Sapling input. +#[derive(Clone, Debug)] +pub struct SigningMetadata { + /// If this action is spending a dummy note, this field holds that note's spend + /// authorizing key. + /// + /// These keys are used automatically in [`Bundle::prepare`] or + /// [`Bundle::apply_signatures`] to sign dummy spends. + dummy_ask: Option, + parts: SigningParts, +} + impl InProgressSignatures for PartiallyAuthorized { type AuthSig = MaybeSigned; } @@ -841,7 +879,7 @@ impl InProgressSignatures for PartiallyAuthorized { #[derive(Clone, Debug)] pub enum MaybeSigned { /// The information needed to sign this [`SpendDescription`]. - SigningMetadata(SigningParts), + SigningParts(SigningParts), /// The signature for this [`SpendDescription`]. Signature(redjubjub::Signature), } @@ -864,18 +902,24 @@ impl Bundle, V> { mut rng: R, sighash: [u8; 32], ) -> Bundle, V> { - self.map_authorization(( - |proof| proof, - |proof| proof, - MaybeSigned::SigningMetadata, - |auth: InProgress| InProgress { + self.map_authorization( + &mut rng, + |_, proof| proof, + |_, proof| proof, + |rng, SigningMetadata { dummy_ask, parts }| match dummy_ask { + None => MaybeSigned::SigningParts(parts), + Some(ask) => { + MaybeSigned::Signature(ask.randomize(&parts.alpha).sign(rng, &sighash)) + } + }, + |rng, auth: InProgress| InProgress { sigs: PartiallyAuthorized { - binding_signature: auth.sigs.bsk.sign(&mut rng, &sighash), + binding_signature: auth.sigs.bsk.sign(rng, &sighash), sighash, }, _proof_state: PhantomData::default(), }, - )) + ) } } @@ -906,17 +950,18 @@ impl Bundle, V> { pub fn sign(self, mut rng: R, ask: &SpendAuthorizingKey) -> Self { let expected_ak = ask.into(); let sighash = self.authorization().sigs.sighash; - self.map_authorization(( - |proof| proof, - |proof| proof, - |maybe| match maybe { - MaybeSigned::SigningMetadata(parts) if parts.ak == expected_ak => { - MaybeSigned::Signature(ask.randomize(&parts.alpha).sign(&mut rng, &sighash)) + self.map_authorization( + &mut rng, + |_, proof| proof, + |_, proof| proof, + |rng, maybe| match maybe { + MaybeSigned::SigningParts(parts) if parts.ak == expected_ak => { + MaybeSigned::Signature(ask.randomize(&parts.alpha).sign(rng, &sighash)) } s => s, }, - |partial| partial, - )) + |_rng, partial| partial, + ) } /// Appends externally computed [`redjubjub::Signature`]s. @@ -934,24 +979,25 @@ impl Bundle, V> { fn append_signature(self, signature: &redjubjub::Signature) -> Result { let sighash = self.authorization().sigs.sighash; let mut signature_valid_for = 0usize; - let bundle = self.map_authorization(( - |proof| proof, - |proof| proof, - |maybe| match maybe { - MaybeSigned::SigningMetadata(parts) => { + let bundle = self.map_authorization( + &mut signature_valid_for, + |_, proof| proof, + |_, proof| proof, + |ctx, maybe| match maybe { + MaybeSigned::SigningParts(parts) => { let rk = parts.ak.randomize(&parts.alpha); if rk.verify(&sighash, signature).is_ok() { - signature_valid_for += 1; + **ctx += 1; MaybeSigned::Signature(*signature) } else { // Signature isn't for this input. - MaybeSigned::SigningMetadata(parts) + MaybeSigned::SigningParts(parts) } } s => s, }, - |partial| partial, - )); + |_, partial| partial, + ); match signature_valid_for { 0 => Err(Error::InvalidExternalSignature), 1 => Ok(bundle), @@ -1028,13 +1074,10 @@ pub mod testing { let anchor = spendable_notes .first() .zip(commitment_trees.first()) - .map_or_else( - || Anchor::empty_tree(), - |(note, tree)| { - let node = Node::from_cmu(¬e.cmu()); - Anchor::from(*tree.root(node).inner()) - }, - ); + .map_or_else(Anchor::empty_tree, |(note, tree)| { + let node = Node::from_cmu(¬e.cmu()); + Anchor::from(*tree.root(node).inner()) + }); let mut builder = Builder::new(zip212_enforcement, BundleType::DEFAULT, anchor); let mut rng = StdRng::from_seed(rng_seed); diff --git a/src/bundle.rs b/src/bundle.rs index ff44346..75941f9 100644 --- a/src/bundle.rs +++ b/src/bundle.rs @@ -1,6 +1,7 @@ use core::fmt::Debug; use memuse::DynamicUsage; + use redjubjub::{Binding, SpendAuth}; use zcash_note_encryption::{ @@ -38,75 +39,6 @@ impl Authorization for Authorized { type AuthSig = redjubjub::Signature; } -/// A map from one bundle authorization to another. -/// -/// For use with [`Bundle::map_authorization`]. -pub trait MapAuth { - fn map_spend_proof(&mut self, p: A::SpendProof) -> B::SpendProof; - fn map_output_proof(&mut self, p: A::OutputProof) -> B::OutputProof; - fn map_auth_sig(&mut self, s: A::AuthSig) -> B::AuthSig; - fn map_authorization(&mut self, a: A) -> B; -} - -/// The identity map. -/// -/// This can be used with [`Bundle::map_authorization`] when you want to map the -/// authorization of a subset of a transaction's bundles (excluding the Sapling bundle) in -/// a higher-level transaction type. -impl MapAuth for () { - fn map_spend_proof( - &mut self, - p: ::SpendProof, - ) -> ::SpendProof { - p - } - - fn map_output_proof( - &mut self, - p: ::OutputProof, - ) -> ::OutputProof { - p - } - - fn map_auth_sig( - &mut self, - s: ::AuthSig, - ) -> ::AuthSig { - s - } - - fn map_authorization(&mut self, a: Authorized) -> Authorized { - a - } -} - -/// A helper for implementing `MapAuth` with a set of closures. -impl MapAuth for (F, G, H, I) -where - A: Authorization, - B: Authorization, - F: FnMut(A::SpendProof) -> B::SpendProof, - G: FnMut(A::OutputProof) -> B::OutputProof, - H: FnMut(A::AuthSig) -> B::AuthSig, - I: FnMut(A) -> B, -{ - fn map_spend_proof(&mut self, p: A::SpendProof) -> B::SpendProof { - self.0(p) - } - - fn map_output_proof(&mut self, p: A::OutputProof) -> B::OutputProof { - self.1(p) - } - - fn map_auth_sig(&mut self, s: A::AuthSig) -> B::AuthSig { - self.2(s) - } - - fn map_authorization(&mut self, a: A) -> B { - self.3(a) - } -} - /// A fallible map from one bundle authorization to another. /// /// For use with [`Bundle::try_map_authorization`]. @@ -200,7 +132,14 @@ impl Bundle { } /// Transitions this bundle from one authorization state to another. - pub fn map_authorization>(self, mut f: F) -> Bundle { + pub fn map_authorization( + self, + mut context: R, + mut spend_proof: impl FnMut(&mut R, A::SpendProof) -> B::SpendProof, + mut output_proof: impl FnMut(&mut R, A::OutputProof) -> B::OutputProof, + mut auth_sig: impl FnMut(&mut R, A::AuthSig) -> B::AuthSig, + mut auth: impl FnMut(&mut R, A) -> B, + ) -> Bundle { Bundle { shielded_spends: self .shielded_spends @@ -210,8 +149,8 @@ impl Bundle { anchor: d.anchor, nullifier: d.nullifier, rk: d.rk, - zkproof: f.map_spend_proof(d.zkproof), - spend_auth_sig: f.map_auth_sig(d.spend_auth_sig), + zkproof: spend_proof(&mut context, d.zkproof), + spend_auth_sig: auth_sig(&mut context, d.spend_auth_sig), }) .collect(), shielded_outputs: self @@ -223,11 +162,11 @@ impl Bundle { ephemeral_key: o.ephemeral_key, enc_ciphertext: o.enc_ciphertext, out_ciphertext: o.out_ciphertext, - zkproof: f.map_output_proof(o.zkproof), + zkproof: output_proof(&mut context, o.zkproof), }) .collect(), value_balance: self.value_balance, - authorization: f.map_authorization(self.authorization), + authorization: auth(&mut context, self.authorization), } } diff --git a/src/note.rs b/src/note.rs index 3d802b4..70d1473 100644 --- a/src/note.rs +++ b/src/note.rs @@ -2,6 +2,11 @@ use group::{ff::Field, GroupEncoding}; use rand_core::{CryptoRng, RngCore}; use zcash_spec::PrfExpand; +use crate::{ + keys::{ExpandedSpendingKey, FullViewingKey}, + zip32::ExtendedSpendingKey, +}; + use super::{ keys::EphemeralSecretKey, value::NoteValue, Nullifier, NullifierDerivingKey, PaymentAddress, }; @@ -148,6 +153,28 @@ impl Note { ))), } } + + /// Generates a dummy spent note. + /// + /// Defined in [Zcash Protocol Spec § 4.8.2: Dummy Notes (Sapling)][saplingdummynotes]. + /// + /// [saplingdummynotes]: https://zips.z.cash/protocol/nu5.pdf#saplingdummynotes + pub(crate) fn dummy(mut rng: R) -> (ExpandedSpendingKey, FullViewingKey, Self) { + let mut sk_bytes = [0; 32]; + rng.fill_bytes(&mut sk_bytes); + + let extsk = ExtendedSpendingKey::master(&sk_bytes[..]); + let fvk = extsk.to_diversifiable_full_viewing_key().fvk().clone(); + let recipient = extsk.default_address(); + + let mut rseed_bytes = [0; 32]; + rng.fill_bytes(&mut rseed_bytes); + let rseed = Rseed::AfterZip212(rseed_bytes); + + let note = Note::from_parts(recipient.1, NoteValue::ZERO, rseed); + + (extsk.expsk, fvk, note) + } } #[cfg(any(test, feature = "test-dependencies"))] From 5c84c147867a29249afe25b433cc3fa84563a82c Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 21 Dec 2023 16:27:54 -0700 Subject: [PATCH 6/9] Add a dummy spend to the bundle if the bundle is required to be present. --- src/builder.rs | 55 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 4bcd98b..839fd87 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -53,6 +53,30 @@ impl BundleType { bundle_required: false, }; + /// Returns the number of logical spends that a builder will produce in constructing a bundle + /// of this type, given the specified numbers of spends and outputs. + /// + /// Returns an error if the specified number of spends and outputs is incompatible with + /// this bundle type. + pub fn num_spends(&self, requested_spends: usize) -> Result { + match self { + BundleType::Transactional { bundle_required } => { + Ok(if *bundle_required || requested_spends > 0 { + core::cmp::max(requested_spends, 1) + } else { + 0 + }) + } + BundleType::Coinbase => { + if requested_spends == 0 { + Ok(0) + } else { + Err("Spends not allowed in coinbase bundles") + } + } + } + } + /// Returns the number of logical outputs that a builder will produce in constructing a bundle /// of this type, given the specified numbers of spends and outputs. /// @@ -60,20 +84,20 @@ impl BundleType { /// this bundle type. pub fn num_outputs( &self, - num_spends: usize, - num_outputs: usize, + requested_spends: usize, + requested_outputs: usize, ) -> Result { match self { BundleType::Transactional { bundle_required } => { - Ok(if *bundle_required || num_outputs > 0 { - core::cmp::max(num_outputs, MIN_SHIELDED_OUTPUTS) + Ok(if *bundle_required || requested_outputs > 0 { + core::cmp::max(requested_outputs, MIN_SHIELDED_OUTPUTS) } else { 0 }) } BundleType::Coinbase => { - if num_spends == 0 { - Ok(num_outputs) + if requested_spends == 0 { + Ok(requested_outputs) } else { Err("Spends not allowed in coinbase bundles") } @@ -550,6 +574,11 @@ pub fn bundle>( } } + let requested_spend_count = spends.len(); + let bundle_spend_count = bundle_type + .num_spends(requested_spend_count) + .map_err(|_| Error::BundleTypeNotSatisfiable)?; + let requested_output_count = outputs.len(); let bundle_output_count = bundle_type .num_outputs(spends.len(), requested_output_count) @@ -562,8 +591,14 @@ pub fn bundle>( tx_metadata.spend_indices.resize(spends.len(), 0); tx_metadata.output_indices.resize(requested_output_count, 0); - // Record initial spend positions - let mut indexed_spends: Vec<_> = spends.into_iter().enumerate().collect(); + // Create any required dummy spends and record initial spend positions + let mut indexed_spends: Vec<_> = spends + .into_iter() + .chain(iter::repeat_with(|| SpendInfo::dummy(&mut rng))) + .enumerate() + .take(bundle_spend_count) + .collect(); + // Create any required dummy outputs and record initial output positions let mut indexed_outputs: Vec<_> = outputs .into_iter() @@ -582,7 +617,9 @@ pub fn bundle>( .enumerate() .map(|(i, (pos, spend))| { // Record the post-randomized spend location - tx_metadata.spend_indices[pos] = i; + if pos < requested_spend_count { + tx_metadata.spend_indices[pos] = i; + } spend.prepare(&mut rng) }) From ef431050173575735e7e8f4e43247c5975529362 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 21 Dec 2023 17:30:45 -0700 Subject: [PATCH 7/9] Strengthen bounds on `map_authorization` arguments. --- src/bundle.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bundle.rs b/src/bundle.rs index 75941f9..bf6f5fc 100644 --- a/src/bundle.rs +++ b/src/bundle.rs @@ -135,10 +135,10 @@ impl Bundle { pub fn map_authorization( self, mut context: R, - mut spend_proof: impl FnMut(&mut R, A::SpendProof) -> B::SpendProof, - mut output_proof: impl FnMut(&mut R, A::OutputProof) -> B::OutputProof, - mut auth_sig: impl FnMut(&mut R, A::AuthSig) -> B::AuthSig, - mut auth: impl FnMut(&mut R, A) -> B, + spend_proof: impl Fn(&mut R, A::SpendProof) -> B::SpendProof, + output_proof: impl Fn(&mut R, A::OutputProof) -> B::OutputProof, + auth_sig: impl Fn(&mut R, A::AuthSig) -> B::AuthSig, + auth: impl FnOnce(&mut R, A) -> B, ) -> Bundle { Bundle { shielded_spends: self From a3ce3b7628db1ac2b198d0682452c1dd889be2bd Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 21 Dec 2023 17:38:56 -0700 Subject: [PATCH 8/9] Make `Bundle::try_map_authorization` work the same as `Bundle::map_authorization` --- CHANGELOG.md | 1 - src/builder.rs | 13 +++++------ src/bundle.rs | 58 ++++++++++---------------------------------------- 3 files changed, 18 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea7b776..0355385 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,7 +42,6 @@ The entries below are relative to the `zcash_primitives::sapling` module as of - `Bundle::>::finalize` - `Bundle::>::apply_signatures` - `Bundle::try_map_authorization` - - `TryMapAuth` - `testing` module, containing the following functions moved from `zcash_primitives::transaction::components::sapling::testing`: - `arb_output_description` diff --git a/src/builder.rs b/src/builder.rs index 839fd87..8c3b64a 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -1048,16 +1048,17 @@ impl Bundle, V> { /// /// Returns an error if any signatures are missing. pub fn finalize(self) -> Result, Error> { - self.try_map_authorization(( - Ok, - Ok, - |maybe: MaybeSigned| maybe.finalize(), - |partial: InProgress| { + self.try_map_authorization::<(), _, _>( + (), + |_, v| Ok(v), + |_, v| Ok(v), + |_, maybe: MaybeSigned| maybe.finalize(), + |_, partial: InProgress| { Ok(Authorized { binding_sig: partial.sigs.binding_signature, }) }, - )) + ) } } diff --git a/src/bundle.rs b/src/bundle.rs index bf6f5fc..0567c5e 100644 --- a/src/bundle.rs +++ b/src/bundle.rs @@ -39,46 +39,6 @@ impl Authorization for Authorized { type AuthSig = redjubjub::Signature; } -/// A fallible map from one bundle authorization to another. -/// -/// For use with [`Bundle::try_map_authorization`]. -pub trait TryMapAuth { - type Error; - fn try_map_spend_proof(&mut self, p: A::SpendProof) -> Result; - fn try_map_output_proof(&mut self, p: A::OutputProof) -> Result; - fn try_map_auth_sig(&mut self, s: A::AuthSig) -> Result; - fn try_map_authorization(&mut self, a: A) -> Result; -} - -/// A helper for implementing `TryMapAuth` with a set of closures. -impl TryMapAuth for (F, G, H, I) -where - A: Authorization, - B: Authorization, - F: FnMut(A::SpendProof) -> Result, - G: FnMut(A::OutputProof) -> Result, - H: FnMut(A::AuthSig) -> Result, - I: FnMut(A) -> Result, -{ - type Error = E; - - fn try_map_spend_proof(&mut self, p: A::SpendProof) -> Result { - self.0(p) - } - - fn try_map_output_proof(&mut self, p: A::OutputProof) -> Result { - self.1(p) - } - - fn try_map_auth_sig(&mut self, s: A::AuthSig) -> Result { - self.2(s) - } - - fn try_map_authorization(&mut self, a: A) -> Result { - self.3(a) - } -} - #[derive(Debug, Clone)] pub struct Bundle { shielded_spends: Vec>, @@ -171,10 +131,14 @@ impl Bundle { } /// Transitions this bundle from one authorization state to another. - pub fn try_map_authorization>( + pub fn try_map_authorization( self, - mut f: F, - ) -> Result, F::Error> { + mut context: R, + spend_proof: impl Fn(&mut R, A::SpendProof) -> Result, + output_proof: impl Fn(&mut R, A::OutputProof) -> Result, + auth_sig: impl Fn(&mut R, A::AuthSig) -> Result, + auth: impl Fn(&mut R, A) -> Result, + ) -> Result, Error> { Ok(Bundle { shielded_spends: self .shielded_spends @@ -185,8 +149,8 @@ impl Bundle { anchor: d.anchor, nullifier: d.nullifier, rk: d.rk, - zkproof: f.try_map_spend_proof(d.zkproof)?, - spend_auth_sig: f.try_map_auth_sig(d.spend_auth_sig)?, + zkproof: spend_proof(&mut context, d.zkproof)?, + spend_auth_sig: auth_sig(&mut context, d.spend_auth_sig)?, }) }) .collect::>()?, @@ -200,12 +164,12 @@ impl Bundle { ephemeral_key: o.ephemeral_key, enc_ciphertext: o.enc_ciphertext, out_ciphertext: o.out_ciphertext, - zkproof: f.try_map_output_proof(o.zkproof)?, + zkproof: output_proof(&mut context, o.zkproof)?, }) }) .collect::>()?, value_balance: self.value_balance, - authorization: f.try_map_authorization(self.authorization)?, + authorization: auth(&mut context, self.authorization)?, }) } } From 93d369fd0a0a1162425c40f4128d90e18b6206ff Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 2 Jan 2024 12:01:52 -0700 Subject: [PATCH 9/9] Apply suggestions from code review Co-authored-by: str4d --- src/builder.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 8c3b64a..b516cb5 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -36,10 +36,12 @@ pub enum BundleType { /// A transactional bundle will be padded if necessary to contain at least 2 outputs, /// irrespective of whether any genuine outputs are required. Transactional { - /// A flag that, when set to `true`, indicates that the resulting bundle should be produced - /// with the minimum required outputs irrespective of whether any outputs have been - /// requested; if no explicit outputs have been added, all of the outputs in the resulting - /// bundle will be dummies. + /// A flag that, when set to `true`, indicates that the resulting bundle should be + /// produced with the minimum required number of spends (1) and outputs (2 with + /// padding) to be usable on its own in a transaction, irrespective of whether any + /// spends or outputs have been requested. If no explicit spends or outputs have + /// been added, all of the spends and outputs in the resulting bundle will be + /// dummies. bundle_required: bool, }, /// A coinbase bundle is required to have no spends. No output padding is performed. @@ -47,8 +49,8 @@ pub enum BundleType { } impl BundleType { - /// The default bundle type has all flags enabled, and does not require a bundle to be produced - /// if no spends or outputs have been added to the bundle. + /// The default bundle type allows both spends and outputs, and does not require a + /// bundle to be produced if no spends or outputs have been added to the bundle. pub const DEFAULT: BundleType = BundleType::Transactional { bundle_required: false, }; @@ -173,16 +175,16 @@ impl SpendInfo { self.note.value() } - /// Defined in [Zcash Protocol Spec § 4.8.3: Dummy Notes (Sapling)][orcharddummynotes]. + /// Defined in [Zcash Protocol Spec § 4.8.2: Dummy Notes (Sapling)][saplingdummynotes]. /// - /// [orcharddummynotes]: https://zips.z.cash/protocol/nu5.pdf#orcharddummynotes + /// [saplingdummynotes]: https://zips.z.cash/protocol/protocol.pdf#saplingdummynotes fn dummy(mut rng: R) -> Self { let (sk, _, note) = Note::dummy(&mut rng); let merkle_path = MerklePath::from_parts( iter::repeat_with(|| Node::from_scalar(jubjub::Base::random(&mut rng))) .take(NOTE_COMMITMENT_TREE_DEPTH.into()) .collect(), - Position::from(rng.next_u64()), + Position::from(0), ) .expect("The path length corresponds to the length of the generated vector."); @@ -997,7 +999,7 @@ impl Bundle, V> { } s => s, }, - |_rng, partial| partial, + |_, partial| partial, ) } @@ -1048,7 +1050,7 @@ impl Bundle, V> { /// /// Returns an error if any signatures are missing. pub fn finalize(self) -> Result, Error> { - self.try_map_authorization::<(), _, _>( + self.try_map_authorization( (), |_, v| Ok(v), |_, v| Ok(v),