From d9945df225a6441dd3ac3ab3a3c5b94f6e1e46e9 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 8 Dec 2023 13:38:52 -0700 Subject: [PATCH] Add explicit control of padding to the Builder API. --- CHANGELOG.md | 6 ++++ benches/circuit.rs | 3 +- benches/note_decryption.rs | 3 +- src/builder.rs | 70 +++++++++++++++++++++++++++++++------- tests/builder.rs | 6 ++-- 5 files changed, 70 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 507ce07cc..2b0aebe51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,16 +6,22 @@ and this project adheres to Rust's notion of [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added +- `orchard::builder::PaddingRule` ### Changed - `orchard::builder::Builder::add_recipient` has been renamed to `add_output` in order to clarify than more than one output of a given transaction may be sent to the same recipient. +- `orchard::builder::Builder::build` now takes an additional `PaddingRule` argument + that specifies how actions should be padded, instead of using hardcoded padding. ## [0.6.0] - 2023-09-08 ### Changed - MSRV is now 1.65.0. - Migrated to `incrementalmerkletree 0.5`. +- `orchard::builder::Builder::new` now takes an additional `PaddingRule` argument + that specifies how actions should be padded, instead of using hardcoded padding. ## [0.5.0] - 2023-06-06 ### Changed diff --git a/benches/circuit.rs b/benches/circuit.rs index 276a4731a..b80a83feb 100644 --- a/benches/circuit.rs +++ b/benches/circuit.rs @@ -7,7 +7,7 @@ use criterion::{BenchmarkId, Criterion}; use pprof::criterion::{Output, PProfProfiler}; use orchard::{ - builder::Builder, + builder::{Builder, PaddingRule}, bundle::Flags, circuit::{ProvingKey, VerifyingKey}, keys::{FullViewingKey, Scope, SpendingKey}, @@ -29,6 +29,7 @@ fn criterion_benchmark(c: &mut Criterion) { let mut builder = Builder::new( Flags::from_parts(true, true), Anchor::from_bytes([0; 32]).unwrap(), + PaddingRule::DEFAULT, ); for _ in 0..num_recipients { builder diff --git a/benches/note_decryption.rs b/benches/note_decryption.rs index 057c0491a..a50b91e57 100644 --- a/benches/note_decryption.rs +++ b/benches/note_decryption.rs @@ -1,6 +1,6 @@ use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion, Throughput}; use orchard::{ - builder::Builder, + builder::{Builder, PaddingRule}, bundle::Flags, circuit::ProvingKey, keys::{FullViewingKey, PreparedIncomingViewingKey, Scope, SpendingKey}, @@ -48,6 +48,7 @@ fn bench_note_decryption(c: &mut Criterion) { let mut builder = Builder::new( Flags::from_parts(true, true), Anchor::from_bytes([0; 32]).unwrap(), + PaddingRule::DEFAULT, ); // The builder pads to two actions, and shuffles their order. Add two recipients // so the first action is always decryptable. diff --git a/src/builder.rs b/src/builder.rs index 496b20807..4cfcb258e 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -27,6 +27,43 @@ use crate::{ const MIN_ACTIONS: usize = 2; +/// An enumeration of rules for construction of dummy actions that may be applied to Orchard bundle +/// construction. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +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, + /// A coinbase bundle is required to have no non-dummy spends. No padding is performed. + Coinbase, +} + +impl BundleType { + /// 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. + /// + /// Returns an error if the specified number of spends and outputs is incompatible with + /// this bundle type. + pub fn num_actions( + &self, + num_spends: usize, + num_outputs: usize, + ) -> Result { + let num_real_actions = core::cmp::max(num_spends, num_outputs); + + match self { + BundleType::Transactional => Ok(core::cmp::max(num_real_actions, MIN_ACTIONS)), + BundleType::Coinbase => { + if num_spends == 0 { + Ok(num_real_actions) + } else { + Err("Spends not allowed in coinbase bundles") + } + } + } + } +} + /// An error type for the kinds of errors that can occur during bundle construction. #[derive(Debug)] pub enum BuildError { @@ -42,6 +79,8 @@ pub enum BuildError { /// A signature is valid for more than one input. This should never happen if `alpha` /// is sampled correctly, and indicates a critical failure in randomness generation. DuplicateSignature, + /// The bundle being constructed violated the construction rules for the requested bundle type. + BundleTypeNotSatisfiable, } impl Display for BuildError { @@ -53,6 +92,9 @@ impl Display for BuildError { ValueSum(_) => f.write_str("Overflow occurred during value construction"), InvalidExternalSignature => f.write_str("External signature was invalid"), DuplicateSignature => f.write_str("Signature valid for more than one input"), + BundleTypeNotSatisfiable => { + f.write_str("Bundle structure did not conform to requested bundle type.") + } } } } @@ -388,22 +430,23 @@ impl Builder { pub fn build>( mut self, mut rng: impl RngCore, + padding_rule: &BundleType, ) -> Result, V>, BuildError> { + let num_real_spends = self.spends.len(); + let num_real_outputs = self.outputs.len(); + let num_actions = padding_rule + .num_actions(num_real_spends, num_real_outputs) + .map_err(|_| BuildError::BundleTypeNotSatisfiable)?; + // Pair up the spends and outputs, extending with dummy values as necessary. let pre_actions: Vec<_> = { - let num_spends = self.spends.len(); - let num_outputs = self.outputs.len(); - let num_actions = [num_spends, num_outputs, MIN_ACTIONS] - .iter() - .max() - .cloned() - .unwrap(); - self.spends.extend( - iter::repeat_with(|| SpendInfo::dummy(&mut rng)).take(num_actions - num_spends), + iter::repeat_with(|| SpendInfo::dummy(&mut rng)) + .take(num_actions - num_real_spends), ); self.outputs.extend( - iter::repeat_with(|| OutputInfo::dummy(&mut rng)).take(num_actions - num_outputs), + iter::repeat_with(|| OutputInfo::dummy(&mut rng)) + .take(num_actions - num_real_outputs), ); // Shuffle the spends and outputs, so that learning the position of a @@ -776,7 +819,7 @@ pub mod testing { Address, Note, }; - use super::Builder; + use super::{Builder, BundleType}; /// An intermediate type used for construction of arbitrary /// bundle values. This type is required because of a limitation @@ -817,7 +860,7 @@ pub mod testing { let pk = ProvingKey::build(); builder - .build(&mut self.rng) + .build(&mut self.rng, &BundleType::Transactional) .unwrap() .create_proof(&pk, &mut self.rng) .unwrap() @@ -898,6 +941,7 @@ mod tests { use super::Builder; use crate::{ + builder::BundleType, bundle::{Authorized, Bundle, Flags}, circuit::ProvingKey, constants::MERKLE_DEPTH_ORCHARD, @@ -927,7 +971,7 @@ mod tests { assert_eq!(balance, -5000); let bundle: Bundle = builder - .build(&mut rng) + .build(&mut rng, &BundleType::Transactional) .unwrap() .create_proof(&pk, &mut rng) .unwrap() diff --git a/tests/builder.rs b/tests/builder.rs index f62f82a0a..40194cd6e 100644 --- a/tests/builder.rs +++ b/tests/builder.rs @@ -1,7 +1,7 @@ use bridgetree::BridgeTree; use incrementalmerkletree::Hashable; use orchard::{ - builder::Builder, + builder::{Builder, BundleType}, bundle::{Authorized, Flags}, circuit::{ProvingKey, VerifyingKey}, keys::{FullViewingKey, PreparedIncomingViewingKey, Scope, SpendAuthorizingKey, SpendingKey}, @@ -47,7 +47,7 @@ fn bundle_chain() { builder.add_output(None, recipient, NoteValue::from_raw(5000), None), Ok(()) ); - let unauthorized = builder.build(&mut rng).unwrap(); + let unauthorized = builder.build(&mut rng, &BundleType::Transactional).unwrap(); let sighash = unauthorized.commitment().into(); let proven = unauthorized.create_proof(&pk, &mut rng).unwrap(); proven.apply_signatures(rng, sighash, &[]).unwrap() @@ -89,7 +89,7 @@ fn bundle_chain() { builder.add_output(None, recipient, NoteValue::from_raw(5000), None), Ok(()) ); - let unauthorized = builder.build(&mut rng).unwrap(); + let unauthorized = builder.build(&mut rng, &BundleType::Transactional).unwrap(); let sighash = unauthorized.commitment().into(); let proven = unauthorized.create_proof(&pk, &mut rng).unwrap(); proven