From 89bc5cd17a4da03bcb9a0f85aa877fd38e50acc0 Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris@nutty.land>
Date: Wed, 13 Dec 2023 11:45:03 -0700
Subject: [PATCH] Make `PaddingRule` an argument to `build` rather than to
 `Builder::new`

---
 CHANGELOG.md     |  4 ++--
 src/builder.rs   | 16 ++++++----------
 tests/builder.rs | 15 ++++++++++-----
 3 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4b7216e08..81f8c7aad 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,11 +10,11 @@ and this project adheres to Rust's notion of
 - `orchard::builder::PaddingRule`
 
 ### Changed
-- `orchard::builder::Builder::new` now takes an additional `PaddingRule` argument
-  that specifies how actions should be padded, instead of using hardcoded padding.
 - `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.
 - `orchard::builder::Builder::build` now returns a `Result<Option<Bundle<...>>, ...>`
   instead of a `Result<Bundle<...>, ...>` to support `PaddingRule::PadTo` and
   to avoid a possible panic in the case of `PadTo(0)` or `Require(0)`.
diff --git a/src/builder.rs b/src/builder.rs
index 68046be77..e1d2bfb03 100644
--- a/src/builder.rs
+++ b/src/builder.rs
@@ -301,18 +301,16 @@ pub struct Builder {
     outputs: Vec<OutputInfo>,
     flags: Flags,
     anchor: Anchor,
-    padding_rule: PaddingRule,
 }
 
 impl Builder {
     /// Constructs a new empty builder for an Orchard bundle.
-    pub fn new(flags: Flags, anchor: Anchor, padding_rule: PaddingRule) -> Self {
+    pub fn new(flags: Flags, anchor: Anchor) -> Self {
         Builder {
             spends: vec![],
             outputs: vec![],
             flags,
             anchor,
-            padding_rule,
         }
     }
 
@@ -428,12 +426,11 @@ impl Builder {
     pub fn build<V: TryFrom<i64>>(
         mut self,
         mut rng: impl RngCore,
+        padding_rule: &PaddingRule,
     ) -> Result<Option<Bundle<InProgress<Unproven, Unauthorized>, V>>, BuildError> {
         let num_real_spends = self.spends.len();
         let num_real_outputs = self.outputs.len();
-        let num_actions = self
-            .padding_rule
-            .num_actions(num_real_spends, num_real_outputs);
+        let num_actions = padding_rule.num_actions(num_real_spends, num_real_outputs);
 
         // Pair up the spends and outputs, extending with dummy values as necessary.
         let pre_actions: Vec<_> = {
@@ -842,7 +839,7 @@ pub mod testing {
         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(flags, self.anchor, PaddingRule::DEFAULT);
+            let mut builder = Builder::new(flags, self.anchor);
 
             for (note, path) in self.notes.into_iter() {
                 builder.add_spend(fvk.clone(), note, path).unwrap();
@@ -859,7 +856,7 @@ pub mod testing {
 
             let pk = ProvingKey::build();
             builder
-                .build(&mut self.rng)
+                .build(&mut self.rng, &PaddingRule::DEFAULT)
                 .unwrap()
                 .unwrap()
                 .create_proof(&pk, &mut self.rng)
@@ -962,7 +959,6 @@ mod tests {
         let mut builder = Builder::new(
             Flags::from_parts(true, true),
             EMPTY_ROOTS[MERKLE_DEPTH_ORCHARD].into(),
-            PaddingRule::DEFAULT,
         );
 
         builder
@@ -972,7 +968,7 @@ mod tests {
         assert_eq!(balance, -5000);
 
         let bundle: Bundle<Authorized, i64> = builder
-            .build(&mut rng)
+            .build(&mut rng, &PaddingRule::DEFAULT)
             .unwrap()
             .unwrap()
             .create_proof(&pk, &mut rng)
diff --git a/tests/builder.rs b/tests/builder.rs
index 6f438c74b..1cef95b64 100644
--- a/tests/builder.rs
+++ b/tests/builder.rs
@@ -42,13 +42,15 @@ fn bundle_chain() {
         // Use the empty tree.
         let anchor = MerkleHashOrchard::empty_root(32.into()).into();
 
-        let mut builder =
-            Builder::new(Flags::from_parts(false, true), anchor, PaddingRule::DEFAULT);
+        let mut builder = Builder::new(Flags::from_parts(false, true), anchor);
         assert_eq!(
             builder.add_output(None, recipient, NoteValue::from_raw(5000), None),
             Ok(())
         );
-        let unauthorized = builder.build(&mut rng).unwrap().unwrap();
+        let unauthorized = builder
+            .build(&mut rng, &PaddingRule::DEFAULT)
+            .unwrap()
+            .unwrap();
         let sighash = unauthorized.commitment().into();
         let proven = unauthorized.create_proof(&pk, &mut rng).unwrap();
         proven.apply_signatures(rng, sighash, &[]).unwrap()
@@ -84,13 +86,16 @@ fn bundle_chain() {
         let anchor = root.into();
         assert_eq!(anchor, merkle_path.root(cmx));
 
-        let mut builder = Builder::new(Flags::from_parts(true, true), anchor, PaddingRule::DEFAULT);
+        let mut builder = Builder::new(Flags::from_parts(true, true), anchor);
         assert_eq!(builder.add_spend(fvk, note, merkle_path), Ok(()));
         assert_eq!(
             builder.add_output(None, recipient, NoteValue::from_raw(5000), None),
             Ok(())
         );
-        let unauthorized = builder.build(&mut rng).unwrap().unwrap();
+        let unauthorized = builder
+            .build(&mut rng, &PaddingRule::DEFAULT)
+            .unwrap()
+            .unwrap();
         let sighash = unauthorized.commitment().into();
         let proven = unauthorized.create_proof(&pk, &mut rng).unwrap();
         proven