Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 1 addition & 5 deletions benches/circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand Down
6 changes: 1 addition & 5 deletions benches/note_decryption.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand Down Expand Up @@ -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
Expand Down
82 changes: 59 additions & 23 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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 => {
Expand All @@ -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,
}
}
}
Expand Down Expand Up @@ -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
}
}
}
Expand Down Expand Up @@ -352,15 +380,17 @@ pub struct Builder {
spends: Vec<SpendInfo>,
outputs: Vec<OutputInfo>,
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,
}
}

Expand All @@ -382,15 +412,15 @@ 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);
}

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);
}

Expand All @@ -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);
}
Expand Down Expand Up @@ -463,7 +493,13 @@ impl Builder {
self,
rng: impl RngCore,
) -> Result<Option<UnauthorizedBundle<V>>, BuildError> {
bundle(rng, self.spends, self.outputs, self.bundle_type)
bundle(
rng,
self.anchor,
self.bundle_type,
self.spends,
self.outputs,
)
}
}

Expand All @@ -473,19 +509,20 @@ impl Builder {
/// [`Bundle::create_proof`] and [`Bundle::apply_signatures`] respectively.
pub fn bundle<V: TryFrom<i64>>(
mut rng: impl RngCore,
anchor: Anchor,
bundle_type: BundleType,
mut spends: Vec<SpendInfo>,
mut outputs: Vec<OutputInfo>,
bundle_type: BundleType,
) -> Result<Option<UnauthorizedBundle<V>>, 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 {
return Err(BuildError::SpendsDisabled);
}

for spend in &spends {
if !spend.has_matching_anchor(anchor) {
if !spend.has_matching_anchor(&anchor) {
return Err(BuildError::AnchorMismatch);
}
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -900,8 +937,7 @@ pub mod testing {
/// Create a bundle from the set of arbitrary bundle inputs.
fn into_bundle<V: TryFrom<i64>>(mut self) -> Bundle<Authorized, V> {
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();
Expand Down Expand Up @@ -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},
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion src/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,12 @@ impl From<MerkleHashOrchard> 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.
Comment on lines +63 to +65
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is entirely valid to use the empty tree in regular bundles. Doing so is equivalent to setting enable_spends = false - it proves publicly that the Orchard bundle spends no notes (but may still have real outputs). I think this comment is fine however because callers should reach for enable_spends in this case, rather than the empty anchor.

pub fn empty_tree() -> Anchor {
Anchor(MerkleHashOrchard::empty_root(Level::from(MERKLE_DEPTH_ORCHARD as u8)).0)
}

Expand Down
10 changes: 8 additions & 2 deletions tests/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this publicly announces that spends are disabled, which in a real shielding bundle may be highly undesirable. This doesn't matter in a test, but we should avoid doing this in examples that downstream crate users are likely to look at for reference.

bundle_required: false,
},
anchor,
);
assert_eq!(
builder.add_output(None, recipient, NoteValue::from_raw(5000), None),
Ok(())
Expand Down Expand Up @@ -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),
Expand Down