From 3845686a6ec5ef9862a8074ccdd49281881dff75 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 20 Dec 2023 19:00:47 -0700 Subject: [PATCH 1/2] Modify `BundleType` to exclude the anchor & allow no bundle to be produced. This adds a flag to `BundleType` that, when set, requires a dummy-only bundle to be produced even if no spends or outputs are added to the builder, and when unset results in standard padding. --- benches/circuit.rs | 6 +-- benches/note_decryption.rs | 6 +-- src/builder.rs | 82 +++++++++++++++++++++++++++----------- src/bundle.rs | 2 +- tests/builder.rs | 10 ++++- 5 files changed, 70 insertions(+), 36 deletions(-) diff --git a/benches/circuit.rs b/benches/circuit.rs index 6dd8e9de..571ce9a0 100644 --- a/benches/circuit.rs +++ b/benches/circuit.rs @@ -8,7 +8,6 @@ use pprof::criterion::{Output, PProfProfiler}; use orchard::{ builder::{Builder, BundleType}, - bundle::Flags, circuit::{ProvingKey, VerifyingKey}, keys::{FullViewingKey, Scope, SpendingKey}, value::NoteValue, @@ -26,10 +25,7 @@ fn criterion_benchmark(c: &mut Criterion) { let pk = ProvingKey::build(); let create_bundle = |num_recipients| { - let mut builder = Builder::new(BundleType::Transactional( - Flags::ENABLED, - Anchor::from_bytes([0; 32]).unwrap(), - )); + let mut builder = Builder::new(BundleType::DEFAULT, Anchor::from_bytes([0; 32]).unwrap()); for _ in 0..num_recipients { builder .add_output(None, recipient, NoteValue::from_raw(10), None) diff --git a/benches/note_decryption.rs b/benches/note_decryption.rs index a4939b12..aa10d1de 100644 --- a/benches/note_decryption.rs +++ b/benches/note_decryption.rs @@ -1,7 +1,6 @@ use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion, Throughput}; use orchard::{ builder::{Builder, BundleType}, - bundle::Flags, circuit::ProvingKey, keys::{FullViewingKey, PreparedIncomingViewingKey, Scope, SpendingKey}, note_encryption::{CompactAction, OrchardDomain}, @@ -45,10 +44,7 @@ fn bench_note_decryption(c: &mut Criterion) { .collect(); let bundle = { - let mut builder = Builder::new(BundleType::Transactional( - Flags::ENABLED, - Anchor::from_bytes([0; 32]).unwrap(), - )); + let mut builder = Builder::new(BundleType::DEFAULT, Anchor::from_bytes([0; 32]).unwrap()); // The builder pads to two actions, and shuffles their order. Add two recipients // so the first action is always decryptable. builder diff --git a/src/builder.rs b/src/builder.rs index a7b1f8fe..d7327569 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -32,12 +32,33 @@ const MIN_ACTIONS: usize = 2; pub enum BundleType { /// A transactional bundle will be padded if necessary to contain at least 2 actions, /// irrespective of whether any genuine actions are required. - Transactional(Flags, Anchor), + Transactional { + /// The flags that control whether spends and/or outputs are enabled for the bundle. + flags: Flags, + /// A flag that, when set to `true`, indicates that a bundle should be produced even if no + /// spends or outputs have been added to the bundle; in such a circumstance, all of the + /// actions in the resulting bundle will be dummies. + bundle_required: bool, + }, /// A coinbase bundle is required to have no non-dummy spends. No 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 { + flags: Flags::ENABLED, + bundle_required: false, + }; + + /// The DISABLED bundle type does not permit any bundle to be produced, and when used in the + /// builder will prevent any spends or outputs from being added. + pub const DISABLED: BundleType = BundleType::Transactional { + flags: Flags::from_parts(false, false), + bundle_required: false, + }; + /// Returns the number of logical actions that builder will produce in constructing a bundle /// of this type, given the specified numbers of spends and outputs. /// @@ -51,13 +72,20 @@ impl BundleType { let num_requested_actions = core::cmp::max(num_spends, num_outputs); match self { - BundleType::Transactional(flags, _) => { + BundleType::Transactional { + flags, + bundle_required, + } => { if !flags.spends_enabled() && num_spends > 0 { Err("Spends are disabled, so num_spends must be zero") } else if !flags.outputs_enabled() && num_outputs > 0 { Err("Outputs are disabled, so num_outputs must be zero") } else { - Ok(core::cmp::max(num_requested_actions, MIN_ACTIONS)) + Ok(if *bundle_required || num_requested_actions > 0 { + core::cmp::max(num_requested_actions, MIN_ACTIONS) + } else { + 0 + }) } } BundleType::Coinbase => { @@ -71,10 +99,10 @@ impl BundleType { } /// Returns the set of flags and the anchor that will be used for bundle construction. - pub fn bundle_config(&self) -> (Flags, Anchor) { + pub fn flags(&self) -> Flags { match self { - BundleType::Transactional(flags, anchor) => (*flags, *anchor), - BundleType::Coinbase => (Flags::SPENDS_DISABLED, Anchor::empty_tree()), + BundleType::Transactional { flags, .. } => *flags, + BundleType::Coinbase => Flags::SPENDS_DISABLED, } } } @@ -224,13 +252,13 @@ impl SpendInfo { } } - fn has_matching_anchor(&self, anchor: Anchor) -> bool { + fn has_matching_anchor(&self, anchor: &Anchor) -> bool { if self.note.value() == NoteValue::zero() { true } else { let cm = self.note.commitment(); let path_root = self.merkle_path.root(cm.into()); - path_root == anchor + &path_root == anchor } } } @@ -352,15 +380,17 @@ pub struct Builder { spends: Vec, outputs: Vec, bundle_type: BundleType, + anchor: Anchor, } impl Builder { /// Constructs a new empty builder for an Orchard bundle. - pub fn new(bundle_type: BundleType) -> Self { + pub fn new(bundle_type: BundleType, anchor: Anchor) -> Self { Builder { spends: vec![], outputs: vec![], bundle_type, + anchor, } } @@ -382,7 +412,7 @@ impl Builder { note: Note, merkle_path: MerklePath, ) -> Result<(), SpendError> { - let (flags, anchor) = self.bundle_type.bundle_config(); + let flags = self.bundle_type.flags(); if !flags.spends_enabled() { return Err(SpendError::SpendsDisabled); } @@ -390,7 +420,7 @@ impl Builder { let spend = SpendInfo::new(fvk, note, merkle_path).ok_or(SpendError::FvkMismatch)?; // Consistency check: all anchors must be equal. - if !spend.has_matching_anchor(anchor) { + if !spend.has_matching_anchor(&self.anchor) { return Err(SpendError::AnchorMismatch); } @@ -407,7 +437,7 @@ impl Builder { value: NoteValue, memo: Option<[u8; 512]>, ) -> Result<(), OutputError> { - let (flags, _) = self.bundle_type.bundle_config(); + let flags = self.bundle_type.flags(); if !flags.outputs_enabled() { return Err(OutputError); } @@ -463,7 +493,13 @@ impl Builder { self, rng: impl RngCore, ) -> Result>, BuildError> { - bundle(rng, self.spends, self.outputs, self.bundle_type) + bundle( + rng, + self.anchor, + self.bundle_type, + self.spends, + self.outputs, + ) } } @@ -473,11 +509,12 @@ impl Builder { /// [`Bundle::create_proof`] and [`Bundle::apply_signatures`] respectively. pub fn bundle>( mut rng: impl RngCore, + anchor: Anchor, + bundle_type: BundleType, mut spends: Vec, mut outputs: Vec, - bundle_type: BundleType, ) -> Result>, BuildError> { - let (flags, anchor) = bundle_type.bundle_config(); + let flags = bundle_type.flags(); let num_requested_spends = spends.len(); if !flags.spends_enabled() && num_requested_spends > 0 { @@ -485,7 +522,7 @@ pub fn bundle>( } for spend in &spends { - if !spend.has_matching_anchor(anchor) { + if !spend.has_matching_anchor(&anchor) { return Err(BuildError::AnchorMismatch); } } @@ -868,7 +905,7 @@ pub mod testing { use crate::{ address::testing::arb_address, - bundle::{Authorized, Bundle, Flags}, + bundle::{Authorized, Bundle}, circuit::ProvingKey, keys::{testing::arb_spending_key, FullViewingKey, SpendAuthorizingKey, SpendingKey}, note::testing::arb_note, @@ -900,8 +937,7 @@ pub mod testing { /// Create a bundle from the set of arbitrary bundle inputs. fn into_bundle>(mut self) -> Bundle { let fvk = FullViewingKey::from(&self.sk); - let flags = Flags::from_parts(true, true); - let mut builder = Builder::new(BundleType::Transactional(flags, self.anchor)); + let mut builder = Builder::new(BundleType::DEFAULT, self.anchor); for (note, path) in self.notes.into_iter() { builder.add_spend(fvk.clone(), note, path).unwrap(); @@ -1001,7 +1037,7 @@ mod tests { use super::Builder; use crate::{ builder::BundleType, - bundle::{Authorized, Bundle, Flags}, + bundle::{Authorized, Bundle}, circuit::ProvingKey, constants::MERKLE_DEPTH_ORCHARD, keys::{FullViewingKey, Scope, SpendingKey}, @@ -1018,10 +1054,10 @@ mod tests { let fvk = FullViewingKey::from(&sk); let recipient = fvk.address_at(0u32, Scope::External); - let mut builder = Builder::new(BundleType::Transactional( - Flags::from_parts(true, true), + let mut builder = Builder::new( + BundleType::DEFAULT, EMPTY_ROOTS[MERKLE_DEPTH_ORCHARD].into(), - )); + ); builder .add_output(None, recipient, NoteValue::from_raw(5000), None) diff --git a/src/bundle.rs b/src/bundle.rs index f5dbb55d..02cd697e 100644 --- a/src/bundle.rs +++ b/src/bundle.rs @@ -64,7 +64,7 @@ const FLAGS_EXPECTED_UNSET: u8 = !(FLAG_SPENDS_ENABLED | FLAG_OUTPUTS_ENABLED); impl Flags { /// Construct a set of flags from its constituent parts - pub(crate) fn from_parts(spends_enabled: bool, outputs_enabled: bool) -> Self { + pub(crate) const fn from_parts(spends_enabled: bool, outputs_enabled: bool) -> Self { Flags { spends_enabled, outputs_enabled, diff --git a/tests/builder.rs b/tests/builder.rs index db5c2ae5..fa35a32f 100644 --- a/tests/builder.rs +++ b/tests/builder.rs @@ -42,7 +42,13 @@ fn bundle_chain() { // Use the empty tree. let anchor = MerkleHashOrchard::empty_root(32.into()).into(); - let mut builder = Builder::new(BundleType::Transactional(Flags::SPENDS_DISABLED, anchor)); + let mut builder = Builder::new( + BundleType::Transactional { + flags: Flags::SPENDS_DISABLED, + bundle_required: false, + }, + anchor, + ); assert_eq!( builder.add_output(None, recipient, NoteValue::from_raw(5000), None), Ok(()) @@ -83,7 +89,7 @@ fn bundle_chain() { let anchor = root.into(); assert_eq!(anchor, merkle_path.root(cmx)); - let mut builder = Builder::new(BundleType::Transactional(Flags::ENABLED, anchor)); + let mut builder = Builder::new(BundleType::DEFAULT, anchor); assert_eq!(builder.add_spend(fvk, note, merkle_path), Ok(())); assert_eq!( builder.add_output(None, recipient, NoteValue::from_raw(5000), None), From 189257391a5726d7f3f2568d547a5ef8603c7156 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 20 Dec 2023 20:08:51 -0700 Subject: [PATCH 2/2] Expose `Anchor::empty_tree`. --- CHANGELOG.md | 1 + src/tree.rs | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffd6aad4..14372c8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to Rust's notion of - `orchard::builder::BundleType` - `orchard::builder::OutputInfo` - `orchard::bundle::Flags::{ENABLED, SPENDS_DISABLED, OUTPUTS_DISABLED}` +- `orchard::tree::Anchor::empty_tree` ### Changed - `orchard::builder::Builder::new` now takes the bundle type to be used diff --git a/src/tree.rs b/src/tree.rs index c93ad695..8f5f0fc6 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -58,7 +58,12 @@ impl From for Anchor { } impl Anchor { - pub(crate) fn empty_tree() -> 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(MerkleHashOrchard::empty_root(Level::from(MERKLE_DEPTH_ORCHARD as u8)).0) }