Remove timelimit from Builder and Bundle

benches/circuit.rs

@@ -34,7 +34,6 @@ fn criterion_benchmark<FL: OrchardFlavorBench>(c: &mut Criterion) {
         let mut builder = Builder::new(
             BundleType::DEFAULT_VANILLA,
             Anchor::from_bytes([0; 32]).unwrap(),
-            None,
         );
         for _ in 0..num_recipients {
             builder

benches/note_decryption.rs

@@ -53,7 +53,6 @@ fn bench_note_decryption<FL: OrchardFlavorBench>(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.

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,21 +525,18 @@ pub struct Builder {
     burn: HashMap<AssetBase, NoteValue>,
     bundle_type: BundleType,
     anchor: Anchor,
-    // When timelimit is set, the Builder will build an ActionGroup (burn must be empty)
-    timelimit: Option<u32>,
 }
 
 impl Builder {
     /// Constructs a new empty builder for an Orchard bundle.
-    pub fn new(bundle_type: BundleType, anchor: Anchor, timelimit: Option<u32>) -> Self {
+    pub fn new(bundle_type: BundleType, anchor: Anchor) -> Self {
         Builder {
             spends: vec![],
             outputs: vec![],
             split_notes: HashMap::new(),
             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<ActionGroup<InProgress<Unproven<OrchardZSA>, 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<V: TryFrom<i64>, FL: OrchardFlavor>(
     mut rng: impl RngCore,
     anchor: Anchor,
-    timelimit: Option<u32>,
     bundle_type: BundleType,
     spends: Vec<SpendInfo>,
     outputs: Vec<OutputInfo>,
     split_notes: HashMap<AssetBase, SpendInfo>,
     burn: HashMap<AssetBase, NoteValue>,
+    check_bsk_bvk: bool,
 ) -> Result<Option<UnauthorizedBundleWithMetadata<V, FL>>, 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<V: TryFrom<i64>, FL: OrchardFlavor>(
         })
         .collect::<Result<Vec<(AssetBase, NoteValue)>, 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<V: TryFrom<i64>, 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<Authorized, V, FL> {
             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

src/bundle.rs

@@ -207,10 +207,6 @@ pub struct Bundle<A: Authorization, V, D: OrchardDomainCommon> {
     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<u32>,
     /// The authorization for this bundle.
     authorization: A,
 }
@@ -243,7 +239,6 @@ impl<A: Authorization, V, D: OrchardDomainCommon> Bundle<A, V, D> {
         value_balance: V,
         burn: Vec<(AssetBase, NoteValue)>,
         anchor: Anchor,
-        timelimit: Option<u32>,
         authorization: A,
     ) -> Self {
         Bundle {
@@ -252,7 +247,6 @@ impl<A: Authorization, V, D: OrchardDomainCommon> Bundle<A, V, D> {
             value_balance,
             burn,
             anchor,
-            timelimit,
             authorization,
         }
     }
@@ -284,11 +278,6 @@ impl<A: Authorization, V, D: OrchardDomainCommon> Bundle<A, V, D> {
         &self.anchor
     }
 
-    /// Returns the root of the Orchard commitment tree that this bundle commits to.
-    pub fn timelimit(&self) -> Option<u32> {
-        self.timelimit
-    }
-
     /// Returns the authorization for this bundle.
     ///
     /// In the case of a `Bundle<Authorized>`, this is the proof and binding signature.
@@ -308,7 +297,6 @@ impl<A: Authorization, V, D: OrchardDomainCommon> Bundle<A, V, D> {
             value_balance: f(self.value_balance)?,
             burn: self.burn,
             anchor: self.anchor,
-            timelimit: self.timelimit,
             authorization: self.authorization,
         })
     }
@@ -328,7 +316,6 @@ impl<A: Authorization, V, D: OrchardDomainCommon> Bundle<A, V, D> {
             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<A: Authorization, V, D: OrchardDomainCommon> Bundle<A, V, D> {
             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::<Result<ValueSum, _>>().unwrap(),
                     burn,
                     anchor,
-                    None,
                     Unauthorized,
                 )
             }
@@ -790,7 +775,6 @@ pub mod testing {
                     balances.into_iter().sum::<Result<ValueSum, _>>().unwrap(),
                     burn,
                     anchor,
-                    None,
                     Authorized {
                         proof: Proof::new(fake_proof),
                         binding_signature: sk.sign(rng, &fake_sighash),

tests/builder.rs

@@ -121,7 +121,6 @@ fn bundle_chain<FL: BundleOrchardFlavor>() {
                 bundle_required: false,
             },
             anchor,
-            None,
         );
         let note_value = NoteValue::from_raw(5000);
         assert_eq!(
@@ -177,7 +176,6 @@ fn bundle_chain<FL: BundleOrchardFlavor>() {
                 bundle_required: false,
             },
             anchor,
-            None,
         );
 
         assert!(builder.add_spend(fvk.clone(), note, merkle_path).is_err());
@@ -187,7 +185,7 @@ fn bundle_chain<FL: BundleOrchardFlavor>() {
     let shielded_bundle: Bundle<_, i64, FL> = {
         let (merkle_path, anchor) = build_merkle_path(&note);
 
-        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(

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<ActionGroup<ActionGroupAuthorized, i64>, 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()