Add `builder::BundleType` for explicit control over output padding.

CHANGELOG.md

@@ -24,6 +24,7 @@ The entries below are relative to the `zcash_primitives::sapling` module as of
   - `SpendDescriptionInfo::value`
   - `SaplingOutputInfo`
   - `ProverProgress`
+  - `BundleType`
 - `sapling_crypto::bundle` module:
   - The following types moved from
     `zcash_primitives::transaction::components::sapling`:
@@ -102,10 +103,18 @@ The entries below are relative to the `zcash_primitives::sapling` module as of
     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
+    it how to pad the bundle with dummy outputs.
+  - `SaplingBuilder::bundle_output_count` has been changed to use a padding
+    rule to compute its result. It also now returns a `Result<usize, &'static str>`
+    instead of a bare `usize` in order to be able to indicate that current
+    state of the builder will produce a bundle that is incompatible with
+    the specified bundle type.
   - `Error` has new error variants:
     - `Error::DuplicateSignature`
     - `Error::InvalidExternalSignature`
     - `Error::MissingSignatures`
+    - `Error::BundleTypeNotSatisfiable`
 - `sapling_crypto::bundle`:
   - `Bundle` now has a second generic parameter `V`.
   - `Bundle::value_balance` now returns `&V` instead of

src/builder.rs

@@ -29,6 +29,41 @@ use crate::{
 /// with dummy outputs if necessary. See <https://github.com/zcash/zcash/issues/3615>.
 const MIN_SHIELDED_OUTPUTS: usize = 2;
 
+/// An enumeration of rules for construction of dummy actions that may be applied to Sapling bundle
+/// construction.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum BundleType {
+    /// A transactional bundle will be padded if necessary to contain at least 2 actions,
+    /// irrespective of whether any genuine actions are required.
+    Transactional,
+    /// A coinbase bundle is required to have no non-dummy spends. No padding is performed.
+    Coinbase,
+}
+
+impl BundleType {
+    /// Returns the number of logical actions that builder will produce in constructing a bundle
+    /// of this type, given the specified numbers of spends and outputs.
+    ///
+    /// Returns an error if the specified number of spends and outputs is incompatible with
+    /// this bundle type.
+    pub fn num_outputs(
+        &self,
+        num_spends: usize,
+        num_outputs: usize,
+    ) -> Result<usize, &'static str> {
+        match self {
+            BundleType::Transactional => Ok(core::cmp::max(num_outputs, MIN_SHIELDED_OUTPUTS)),
+            BundleType::Coinbase => {
+                if num_spends == 0 {
+                    Ok(num_outputs)
+                } else {
+                    Err("Spends not allowed in coinbase bundles")
+                }
+            }
+        }
+    }
+}
+
 #[derive(Debug, PartialEq, Eq)]
 pub enum Error {
     AnchorMismatch,
@@ -43,6 +78,8 @@ pub enum Error {
     /// A bundle could not be built because required signatures were missing.
     MissingSignatures,
     SpendProof,
+    /// The bundle being constructed violated the construction rules for the requested bundle type.
+    BundleTypeNotSatisfiable,
 }
 
 impl fmt::Display for Error {
@@ -58,6 +95,9 @@ impl fmt::Display for Error {
             Error::InvalidExternalSignature => write!(f, "External signature was invalid"),
             Error::MissingSignatures => write!(f, "Required signatures were missing during build"),
             Error::SpendProof => write!(f, "Failed to create Sapling spend proof"),
+            Error::BundleTypeNotSatisfiable => {
+                f.write_str("Bundle structure did not conform to requested bundle type.")
+            }
         }
     }
 }
@@ -318,12 +358,10 @@ impl SaplingBuilder {
     ///
     /// This may be larger than the number of outputs that have been added to the builder,
     /// depending on whether padding is going to be applied.
-    pub fn bundle_output_count(&self) -> usize {
-        // This matches the padding behaviour in `Self::build`.
-        match self.spends.len() {
-            0 => self.outputs.len(),
-            _ => std::cmp::max(MIN_SHIELDED_OUTPUTS, self.outputs.len()),
-        }
+    pub fn bundle_output_count(&self, padding_rule: &BundleType) -> Result<usize, Error> {
+        padding_rule
+            .num_outputs(self.spends.len(), self.outputs.len())
+            .map_err(|_| Error::BundleTypeNotSatisfiable)
     }
 
     /// Returns the net value represented by the spends and outputs added to this builder,
@@ -405,8 +443,10 @@ impl SaplingBuilder {
     pub fn build<SP: SpendProver, OP: OutputProver, R: RngCore, V: TryFrom<i64>>(
         self,
         mut rng: R,
+        padding_rule: &BundleType,
     ) -> Result<Option<(UnauthorizedBundle<V>, SaplingMetadata)>, Error> {
         let value_balance = self.try_value_balance()?;
+        let bundle_output_count = self.bundle_output_count(padding_rule)?;
 
         // Record initial positions of spends and outputs
         let mut indexed_spends: Vec<_> = self.spends.into_iter().enumerate().collect();
@@ -424,10 +464,8 @@ impl SaplingBuilder {
         tx_metadata.output_indices.resize(indexed_outputs.len(), 0);
 
         // Pad Sapling outputs
-        if !indexed_spends.is_empty() {
-            while indexed_outputs.len() < MIN_SHIELDED_OUTPUTS {
-                indexed_outputs.push(None);
-            }
+        while indexed_outputs.len() < bundle_output_count {
+            indexed_outputs.push(None);
         }
 
         // Randomize order of inputs and outputs
@@ -876,7 +914,7 @@ pub mod testing {
         frontier::testing::arb_commitment_tree, witness::IncrementalWitness,
     };
 
-    use super::SaplingBuilder;
+    use super::{BundleType, SaplingBuilder};
 
     #[allow(dead_code)]
     fn arb_bundle<V: fmt::Debug + From<i64>>(
@@ -913,7 +951,10 @@ pub mod testing {
                     }
 
                     let (bundle, _) = builder
-                        .build::<MockSpendProver, MockOutputProver, _, _>(&mut rng)
+                        .build::<MockSpendProver, MockOutputProver, _, _>(
+                            &mut rng,
+                            &BundleType::Transactional,
+                        )
                         .unwrap()
                         .unwrap();