From 8f87e2456cc34e10f8c39d5c48619044e205e36c Mon Sep 17 00:00:00 2001 From: Constance Beguier Date: Tue, 15 Oct 2024 17:18:37 +0200 Subject: [PATCH] Remove timelimit from Builder and Bundle --- benches/circuit.rs | 1 - benches/note_decryption.rs | 1 - src/builder.rs | 37 ++++++++++++++++--------------------- src/bundle.rs | 16 ---------------- tests/builder.rs | 4 +--- tests/zsa.rs | 6 +++--- 6 files changed, 20 insertions(+), 45 deletions(-) diff --git a/benches/circuit.rs b/benches/circuit.rs index d99f3230f..668965c4d 100644 --- a/benches/circuit.rs +++ b/benches/circuit.rs @@ -34,7 +34,6 @@ fn criterion_benchmark(c: &mut Criterion) { let mut builder = Builder::new( BundleType::DEFAULT_VANILLA, Anchor::from_bytes([0; 32]).unwrap(), - None, ); for _ in 0..num_recipients { builder diff --git a/benches/note_decryption.rs b/benches/note_decryption.rs index cedc9952a..d17b01e41 100644 --- a/benches/note_decryption.rs +++ b/benches/note_decryption.rs @@ -53,7 +53,6 @@ fn bench_note_decryption(c: &mut Criterion) { let mut builder = Builder::new( BundleType::DEFAULT_VANILLA, Anchor::from_bytes([0; 32]).unwrap(), - None, ); // 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 4bef0cdce..2b301e339 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -151,8 +151,8 @@ pub enum BuildError { BurnDuplicateAsset, /// There is no available split note for this asset. NoSplitNoteAvailable, - /// Timelimit is set (thus it is an ActionGroup builder) but burn is not empty. - TimelimitSetAndBurnNotEmpty, + /// Burn is not empty, but we are building an action group. + BurnNotEmptyInActionGroup, } impl Display for BuildError { @@ -176,9 +176,9 @@ impl Display for BuildError { BurnZero => f.write_str("Burning is not possible for zero values"), BurnDuplicateAsset => f.write_str("Duplicate assets are not allowed when burning"), NoSplitNoteAvailable => f.write_str("No split note has been provided for this asset"), - TimelimitSetAndBurnNotEmpty => f.write_str( - "Timelimit is set (thus it is an ActionGroup builder) but burn is not empty", - ), + BurnNotEmptyInActionGroup => { + f.write_str("Burn is not empty, but we are building an action group") + } } } } @@ -525,13 +525,11 @@ pub struct Builder { burn: HashMap, bundle_type: BundleType, anchor: Anchor, - // When timelimit is set, the Builder will build an ActionGroup (burn must be empty) - timelimit: Option, } impl Builder { /// Constructs a new empty builder for an Orchard bundle. - pub fn new(bundle_type: BundleType, anchor: Anchor, timelimit: Option) -> Self { + pub fn new(bundle_type: BundleType, anchor: Anchor) -> Self { Builder { spends: vec![], outputs: vec![], @@ -539,7 +537,6 @@ impl Builder { burn: HashMap::new(), bundle_type, anchor, - timelimit, } } @@ -684,12 +681,12 @@ impl Builder { bundle( rng, self.anchor, - self.timelimit, self.bundle_type, self.spends, self.outputs, self.split_notes, self.burn, + true, ) } @@ -702,15 +699,18 @@ impl Builder { rng: impl RngCore, timelimit: u32, ) -> Result, Unauthorized>, V>, BuildError> { + if !self.burn.is_empty() { + return Err(BuildError::BurnNotEmptyInActionGroup); + } let action_group = bundle( rng, self.anchor, - self.timelimit, self.bundle_type, self.spends, self.outputs, self.split_notes, self.burn, + false, )? .unwrap() .0; @@ -794,16 +794,13 @@ fn pad_spend( pub fn bundle, FL: OrchardFlavor>( mut rng: impl RngCore, anchor: Anchor, - timelimit: Option, bundle_type: BundleType, spends: Vec, outputs: Vec, split_notes: HashMap, burn: HashMap, + check_bsk_bvk: bool, ) -> Result>, BuildError> { - if timelimit.is_some() && !burn.is_empty() { - return Err(BuildError::TimelimitSetAndBurnNotEmpty); - } let flags = bundle_type.flags(); let num_requested_spends = spends.len(); @@ -941,9 +938,9 @@ pub fn bundle, FL: OrchardFlavor>( }) .collect::, BuildError>>()?; - // Verify that bsk and bvk are consistent except for ActionGroup (when timelimit is set) - if timelimit.is_none() { - // TODO update the check to also do it for swap order by adding value balance for each asset + if check_bsk_bvk { + // Verify that bsk and bvk are consistent + // (they are not consistent in ActionGroup creation) let bvk = derive_bvk(&actions, native_value_balance, burn.iter().cloned()); assert_eq!(redpallas::VerificationKey::from(&bsk), bvk); } @@ -956,7 +953,6 @@ pub fn bundle, FL: OrchardFlavor>( result_value_balance, burn, anchor, - timelimit, InProgress { proof: Unproven { circuits }, sigs: Unauthorized { bsk }, @@ -1431,7 +1427,7 @@ pub mod testing { mut self, ) -> Bundle { let fvk = FullViewingKey::from(&self.sk); - let mut builder = Builder::new(BundleType::DEFAULT_ZSA, self.anchor, None); + let mut builder = Builder::new(BundleType::DEFAULT_ZSA, self.anchor); for (note, path) in self.notes.into_iter() { builder.add_spend(fvk.clone(), note, path).unwrap(); @@ -1565,7 +1561,6 @@ mod tests { let mut builder = Builder::new( BundleType::DEFAULT_VANILLA, EMPTY_ROOTS[MERKLE_DEPTH_ORCHARD].into(), - None, ); builder diff --git a/src/bundle.rs b/src/bundle.rs index e8df03bfe..48b92aaa9 100644 --- a/src/bundle.rs +++ b/src/bundle.rs @@ -207,10 +207,6 @@ pub struct Bundle { burn: Vec<(AssetBase, NoteValue)>, /// The root of the Orchard commitment tree that this bundle commits to. anchor: Anchor, - /// The timelimit for this Bundle (which is an ActionGroup). - /// - /// Burn must be empty when timelimit is set. - timelimit: Option, /// The authorization for this bundle. authorization: A, } @@ -243,7 +239,6 @@ impl Bundle { value_balance: V, burn: Vec<(AssetBase, NoteValue)>, anchor: Anchor, - timelimit: Option, authorization: A, ) -> Self { Bundle { @@ -252,7 +247,6 @@ impl Bundle { value_balance, burn, anchor, - timelimit, authorization, } } @@ -284,11 +278,6 @@ impl Bundle { &self.anchor } - /// Returns the root of the Orchard commitment tree that this bundle commits to. - pub fn timelimit(&self) -> Option { - self.timelimit - } - /// Returns the authorization for this bundle. /// /// In the case of a `Bundle`, this is the proof and binding signature. @@ -308,7 +297,6 @@ impl Bundle { value_balance: f(self.value_balance)?, burn: self.burn, anchor: self.anchor, - timelimit: self.timelimit, authorization: self.authorization, }) } @@ -328,7 +316,6 @@ impl Bundle { flags: self.flags, value_balance: self.value_balance, anchor: self.anchor, - timelimit: self.timelimit, authorization: step(context, authorization), burn: self.burn, } @@ -353,7 +340,6 @@ impl Bundle { flags: self.flags, value_balance: self.value_balance, anchor: self.anchor, - timelimit: self.timelimit, authorization: step(context, authorization)?, burn: self.burn, }) @@ -757,7 +743,6 @@ pub mod testing { balances.into_iter().sum::>().unwrap(), burn, anchor, - None, Unauthorized, ) } @@ -790,7 +775,6 @@ pub mod testing { balances.into_iter().sum::>().unwrap(), burn, anchor, - None, Authorized { proof: Proof::new(fake_proof), binding_signature: sk.sign(rng, &fake_sighash), diff --git a/tests/builder.rs b/tests/builder.rs index bbc08ad55..63ee6f94a 100644 --- a/tests/builder.rs +++ b/tests/builder.rs @@ -121,7 +121,6 @@ fn bundle_chain() { bundle_required: false, }, anchor, - None, ); let note_value = NoteValue::from_raw(5000); assert_eq!( @@ -177,7 +176,6 @@ fn bundle_chain() { bundle_required: false, }, anchor, - None, ); assert!(builder.add_spend(fvk.clone(), note, merkle_path).is_err()); @@ -187,7 +185,7 @@ fn bundle_chain() { let shielded_bundle: Bundle<_, i64, FL> = { let (merkle_path, anchor) = build_merkle_path(¬e); - let mut builder = Builder::new(FL::DEFAULT_BUNDLE_TYPE, anchor, None); + let mut builder = Builder::new(FL::DEFAULT_BUNDLE_TYPE, anchor); assert_eq!(builder.add_spend(fvk, note, merkle_path), Ok(())); assert_eq!( builder.add_output( diff --git a/tests/zsa.rs b/tests/zsa.rs index 7cbc8258f..520c1951b 100644 --- a/tests/zsa.rs +++ b/tests/zsa.rs @@ -236,7 +236,7 @@ fn create_native_note(keys: &Keychain) -> Note { // Use the empty tree. let anchor = MerkleHashOrchard::empty_root(32.into()).into(); - let mut builder = Builder::new(BundleType::Coinbase, anchor, None); + let mut builder = Builder::new(BundleType::Coinbase, anchor); assert_eq!( builder.add_output( None, @@ -291,7 +291,7 @@ fn build_and_verify_bundle( ) -> Result<(), String> { let rng = OsRng; let shielded_bundle: Bundle<_, i64, OrchardZSA> = { - let mut builder = Builder::new(BundleType::DEFAULT_ZSA, anchor, None); + let mut builder = Builder::new(BundleType::DEFAULT_ZSA, anchor); spends .iter() @@ -330,7 +330,7 @@ fn build_and_verify_action_group( ) -> Result, String> { let rng = OsRng; let shielded_bundle: ActionGroup<_, i64> = { - let mut builder = Builder::new(BundleType::DEFAULT_ZSA, anchor, Some(timelimit)); + let mut builder = Builder::new(BundleType::DEFAULT_ZSA, anchor); spends .iter()