Skip to content

Commit

Permalink
Make the Builder::build result type consistent with the Sapling bui…
Browse files Browse the repository at this point in the history
…lder.

This avoids a panic in the case of `PaddingRule::Require(0)` or
`PaddingRule::PadTo(0)`.
  • Loading branch information
nuttycom committed Dec 12, 2023
1 parent 6c7648d commit 5d72a86
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 18 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ and this project adheres to Rust's notion of
- `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 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)`.

## [0.6.0] - 2023-09-08
### Changed
Expand Down
34 changes: 18 additions & 16 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,12 @@ const MIN_ACTIONS: usize = 2;
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum PaddingRule {
/// A rule that requires that at least the specified number of Orchard actions be constructed,
/// irrespective of whether any genuine actions are included.
/// irrespective of whether any genuine actions are included. If no padding is required,
/// then `Require(0)` may be used to indicate this.
Require(usize),
/// A rule that requires that at least the specified number of Orchard actions be constructed,
/// iff any genuine actions are included.
PadTo(usize),
/// A padding rule that specifies that no additional dummy Orchard actions are to be
/// constructed.
None,
}

impl PaddingRule {
Expand All @@ -62,7 +60,6 @@ impl PaddingRule {
core::cmp::max(num_real_actions, *n)
}
}
PaddingRule::None => num_real_actions,
}
}
}
Expand Down Expand Up @@ -427,10 +424,11 @@ impl Builder {
///
/// The returned bundle will have no proof or signatures; these can be applied with
/// [`Bundle::create_proof`] and [`Bundle::apply_signatures`] respectively.
#[allow(clippy::type_complexity)]
pub fn build<V: TryFrom<i64>>(
mut self,
mut rng: impl RngCore,
) -> Result<Bundle<InProgress<Unproven, Unauthorized>, V>, BuildError> {
) -> 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
Expand Down Expand Up @@ -494,16 +492,18 @@ impl Builder {
.into_bvk();
assert_eq!(redpallas::VerificationKey::from(&bsk), bvk);

Ok(Bundle::from_parts(
NonEmpty::from_vec(actions).unwrap(),
flags,
result_value_balance,
anchor,
InProgress {
proof: Unproven { circuits },
sigs: Unauthorized { bsk },
},
))
Ok(NonEmpty::from_vec(actions).map(|actions| {
Bundle::from_parts(
actions,
flags,
result_value_balance,
anchor,
InProgress {
proof: Unproven { circuits },
sigs: Unauthorized { bsk },
},
)
}))
}
}

Expand Down Expand Up @@ -861,6 +861,7 @@ pub mod testing {
builder
.build(&mut self.rng)
.unwrap()
.unwrap()
.create_proof(&pk, &mut self.rng)
.unwrap()
.prepare(&mut self.rng, [0; 32])
Expand Down Expand Up @@ -973,6 +974,7 @@ mod tests {
let bundle: Bundle<Authorized, i64> = builder
.build(&mut rng)
.unwrap()
.unwrap()
.create_proof(&pk, &mut rng)
.unwrap()
.prepare(rng, [0; 32])
Expand Down
4 changes: 2 additions & 2 deletions tests/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,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).unwrap().unwrap();
let sighash = unauthorized.commitment().into();
let proven = unauthorized.create_proof(&pk, &mut rng).unwrap();
proven.apply_signatures(rng, sighash, &[]).unwrap()
Expand Down Expand Up @@ -90,7 +90,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).unwrap().unwrap();
let sighash = unauthorized.commitment().into();
let proven = unauthorized.create_proof(&pk, &mut rng).unwrap();
proven
Expand Down

0 comments on commit 5d72a86

Please sign in to comment.