Apply comments from code review.

Co-authored-by: str4d <[email protected]>

CHANGELOG.md

@@ -105,8 +105,8 @@ The entries below are relative to the `zcash_primitives::sapling` module as of
     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>`
+  - `SaplingBuilder::bundle_output_count` has been changed to use a bundle
+    type to compute its result. It also now returns a `Result<usize, Error>`
     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.

src/builder.rs

@@ -29,19 +29,18 @@ 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.
+/// An enumeration of rules for 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.
+    /// A transactional bundle will be padded if necessary to contain at least 2 outputs,
+    /// irrespective of whether any genuine outputs are required.
     Transactional,
-    /// A coinbase bundle is required to have no non-dummy spends. No padding is performed.
+    /// A coinbase bundle is required to have no spends. No output padding is performed.
     Coinbase,
 }
 
 impl BundleType {
-    /// Returns the number of logical actions that builder will produce in constructing a bundle
+    /// Returns the number of logical outputs that a 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
@@ -358,8 +357,8 @@ 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, padding_rule: &BundleType) -> Result<usize, Error> {
-        padding_rule
+    pub fn bundle_output_count(&self, bundle_type: &BundleType) -> Result<usize, Error> {
+        bundle_type
             .num_outputs(self.spends.len(), self.outputs.len())
             .map_err(|_| Error::BundleTypeNotSatisfiable)
     }
@@ -443,10 +442,10 @@ impl SaplingBuilder {
     pub fn build<SP: SpendProver, OP: OutputProver, R: RngCore, V: TryFrom<i64>>(
         self,
         mut rng: R,
-        padding_rule: &BundleType,
+        bundle_type: &BundleType,
     ) -> Result<Option<(UnauthorizedBundle<V>, SaplingMetadata)>, Error> {
         let value_balance = self.try_value_balance()?;
-        let bundle_output_count = self.bundle_output_count(padding_rule)?;
+        let bundle_output_count = self.bundle_output_count(bundle_type)?;
 
         // Record initial positions of spends and outputs
         let mut indexed_spends: Vec<_> = self.spends.into_iter().enumerate().collect();