Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A few minor cleanups for consistency with the orchard builder API #114

Merged
merged 9 commits into from
Jan 2, 2024
13 changes: 7 additions & 6 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,21 +91,22 @@ The entries below are relative to the `zcash_primitives::sapling` module as of
- `sapling_crypto::address::PaymentAddress::create_note` now takes its `value`
argument as a `NoteValue` instead of as a bare `u64`.
- `sapling_crypto::builder`:
- `SaplingBuilder` no longer has a `P: zcash_primitives::consensus::Parameters`
- `SaplingBuilder` has been renamed to `Builder`
- `Builder` no longer has a `P: zcash_primitives::consensus::Parameters`
type parameter.
- `SaplingBuilder::new` now takes a `Zip212Enforcement` argument instead of a
- `Builder::new` now takes a `Zip212Enforcement` argument instead of a
`P: zcash_primitives::consensus::Parameters` argument and a target height.
- `SaplingBuilder::add_spend` now takes `extsk` by reference. Also, it no
- `Builder::add_spend` now takes `extsk` by reference. Also, it no
longer takes a `diversifier` argument as the diversifier may be obtained
from the note.
- `SaplingBuilder::add_output` now takes an `Option<[u8; 512]>` memo instead
- `Builder::add_output` now takes an `Option<[u8; 512]>` memo instead
of a `MemoBytes`.
- `SaplingBuilder::build` no longer takes a prover, proving context, progress
- `Builder::build` no longer takes a prover, proving context, progress
notifier, or target height. Instead, it has `SpendProver, OutputProver`
generic parameters and returns `(UnauthorizedBundle, SaplingMetadata)`. The
caller can then use `Bundle::<InProgress<Unproven, _>>::create_proofs` to
create spend and output proofs for the bundle.
- `SaplingBuilder::build` now takes a `BundleType` argument that instructs
- `Builder::build` now takes a `BundleType` argument that instructs
it how to pad the bundle with dummy outputs.
- `Error` has new error variants:
- `Error::DuplicateSignature`
Expand Down
29 changes: 13 additions & 16 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ impl PreparedOutputInfo {
}
}

/// Metadata about a transaction created by a [`SaplingBuilder`].
/// Metadata about a transaction created by a [`Builder`].
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct SaplingMetadata {
spend_indices: Vec<usize>,
Expand All @@ -339,40 +339,40 @@ impl SaplingMetadata {
}

/// Returns the index within the transaction of the [`SpendDescription`] corresponding
/// to the `n`-th call to [`SaplingBuilder::add_spend`].
/// to the `n`-th call to [`Builder::add_spend`].
///
/// Note positions are randomized when building transactions for indistinguishability.
/// This means that the transaction consumer cannot assume that e.g. the first spend
/// they added (via the first call to [`SaplingBuilder::add_spend`]) is the first
/// they added (via the first call to [`Builder::add_spend`]) is the first
/// [`SpendDescription`] in the transaction.
pub fn spend_index(&self, n: usize) -> Option<usize> {
self.spend_indices.get(n).copied()
}

/// Returns the index within the transaction of the [`OutputDescription`] corresponding
/// to the `n`-th call to [`SaplingBuilder::add_output`].
/// to the `n`-th call to [`Builder::add_output`].
///
/// Note positions are randomized when building transactions for indistinguishability.
/// This means that the transaction consumer cannot assume that e.g. the first output
/// they added (via the first call to [`SaplingBuilder::add_output`]) is the first
/// they added (via the first call to [`Builder::add_output`]) is the first
/// [`OutputDescription`] in the transaction.
pub fn output_index(&self, n: usize) -> Option<usize> {
self.output_indices.get(n).copied()
}
}

/// A mutable builder type for constructing Sapling bundles.
pub struct SaplingBuilder {
pub struct Builder {
value_balance: ValueSum,
spends: Vec<SpendInfo>,
outputs: Vec<OutputInfo>,
zip212_enforcement: Zip212Enforcement,
bundle_type: BundleType,
}

impl SaplingBuilder {
impl Builder {
pub fn new(zip212_enforcement: Zip212Enforcement, bundle_type: BundleType) -> Self {
SaplingBuilder {
Builder {
value_balance: ValueSum::zero(),
spends: vec![],
outputs: vec![],
Expand Down Expand Up @@ -441,8 +441,7 @@ impl SaplingBuilder {
}

/// Adds a Sapling address to send funds to.
#[allow(clippy::too_many_arguments)]
pub fn add_output<R: RngCore>(
pub fn add_output(
&mut self,
ovk: Option<OutgoingViewingKey>,
to: PaymentAddress,
Expand Down Expand Up @@ -607,7 +606,7 @@ pub fn bundle<SP: SpendProver, OP: OutputProver, R: RngCore, V: TryFrom<i64>>(

/// Type alias for an in-progress bundle that has no proofs or signatures.
///
/// This is returned by [`SaplingBuilder::build`].
/// This is returned by [`Builder::build`].
pub type UnauthorizedBundle<V> = Bundle<InProgress<Unproven, Unsigned>, V>;

/// Marker trait representing bundle proofs in the process of being created.
Expand Down Expand Up @@ -976,7 +975,7 @@ pub mod testing {
frontier::testing::arb_commitment_tree, witness::IncrementalWitness, Hashable, Level,
};

use super::{BundleType, SaplingBuilder};
use super::{Builder, BundleType};

#[allow(dead_code)]
fn arb_bundle<V: fmt::Debug + From<i64>>(
Expand Down Expand Up @@ -1012,10 +1011,8 @@ pub mod testing {
Node::from_scalar(*tree.root(node).inner())
},
);
let mut builder = SaplingBuilder::new(
zip212_enforcement,
BundleType::Transactional { anchor },
);
let mut builder =
Builder::new(zip212_enforcement, BundleType::Transactional { anchor });
let mut rng = StdRng::from_seed(rng_seed);

for (note, path) in spendable_notes
Expand Down
8 changes: 4 additions & 4 deletions src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
//! single [`Bundle`].
//! - `valueBalanceSapling`, which is a signed 63-bit integer. This is represented
//! by a user-defined type parameter on [`Bundle`], returned by
//! [`Bundle::value_balance`] and [`SaplingBuilder::value_balance`].
//! [`Bundle::value_balance`] and [`Builder::value_balance`].
//!
//! If your specific instantiation of the Sapling protocol requires a smaller bound on
//! valid note values (for example, Zcash's `MAX_MONEY` fits into a 51-bit integer), you
Expand All @@ -17,7 +17,7 @@
//! - Define your `valueBalanceSapling` type to enforce your valid value range. This can
//! be checked in its `TryFrom<i64>` implementation.
//! - Define your own "amount" type for note values, and convert it to `NoteValue` prior
//! to calling [`SaplingBuilder::add_output`].
//! to calling [`Builder::add_output`].
//!
//! Inside the circuit, note values are constrained to be unsigned 64-bit integers.
//!
Expand All @@ -33,8 +33,8 @@
//!
//! [`Bundle`]: crate::Bundle
//! [`Bundle::value_balance`]: crate::Bundle::value_balance
//! [`SaplingBuilder::value_balance`]: crate::builder::SaplingBuilder::value_balance
//! [`SaplingBuilder::add_output`]: crate::builder::SaplingBuilder::add_output
//! [`Builder::value_balance`]: crate::builder::Builder::value_balance
//! [`Builder::add_output`]: crate::builder::Builder::add_output
//! [Rust documentation]: https://doc.rust-lang.org/stable/std/primitive.i64.html

use bitvec::{array::BitArray, order::Lsb0};
Expand Down