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 a38b522 commit 99d3d0a
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 20 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
2 changes: 1 addition & 1 deletion benches/circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ fn criterion_benchmark(c: &mut Criterion) {
.add_output(None, recipient, NoteValue::from_raw(10), None)
.unwrap();
}
let bundle: Bundle<_, i64> = builder.build(rng).unwrap();
let bundle: Bundle<_, i64> = builder.build(rng).unwrap().unwrap();

let instances: Vec<_> = bundle
.actions()
Expand Down
2 changes: 1 addition & 1 deletion benches/note_decryption.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fn bench_note_decryption(c: &mut Criterion) {
builder
.add_output(None, recipient, NoteValue::from_raw(10), None)
.unwrap();
let bundle: Bundle<_, i64> = builder.build(rng).unwrap();
let bundle: Bundle<_, i64> = builder.build(rng).unwrap().unwrap();
bundle
.create_proof(&pk, rng)
.unwrap()
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 99d3d0a

Please sign in to comment.