Apply suggestions from code review

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

src/builder.rs

@@ -27,7 +27,7 @@ use crate::{
 
 const MIN_ACTIONS: usize = 2;
 
-/// An enumeration of rules Orchard bundle construction.
+/// An enumeration of rules for Orchard 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,
@@ -60,7 +60,13 @@ impl BundleType {
                     Ok(core::cmp::max(num_requested_actions, MIN_ACTIONS))
                 }
             }
-            BundleType::Coinbase => Ok(num_outputs),
+            BundleType::Coinbase => {
+                if num_spends > 0 {
+                    Err("Coinbase bundles have spends disabled, so num_spends must be zero")
+                } else {
+                    Ok(num_outputs)
+                }
+            }
         }
     }
 
@@ -80,7 +86,7 @@ pub enum BuildError {
     SpendsDisabled,
     /// Spends are disabled for the provided bundle type.
     OutputsDisabled,
-    /// The anchor provided to this builder doesn't match the merkle path used to add a spend.
+    /// The anchor provided to this builder doesn't match the Merkle path used to add a spend.
     AnchorMismatch,
     /// A bundle could not be built because required signatures were missing.
     MissingSignatures,
@@ -217,6 +223,16 @@ impl SpendInfo {
             merkle_path,
         }
     }
+
+    fn has_matching_anchor(&self, anchor: Anchor) -> bool {
+        if self.note.value() == NoteValue::zero() {
+            true
+        } else {
+            let cm = self.note.commitment();
+            let path_root = self.merkle_path.root(cm.into());
+            path_root == anchor
+        }
+    }
 }
 
 /// Information about a specific output to receive funds in an [`Action`].
@@ -366,15 +382,14 @@ impl Builder {
             return Err(SpendError::SpendsDisabled);
         }
 
+        let spend = SpendInfo::new(fvk, note, merkle_path).ok_or(SpendError::FvkMismatch)?;
+
         // Consistency check: all anchors must be equal.
-        let cm = note.commitment();
-        let path_root = merkle_path.root(cm.into());
-        if path_root != anchor {
+        if !spend.has_matching_anchor(anchor) {
             return Err(SpendError::AnchorMismatch);
         }
 
-        self.spends
-            .push(SpendInfo::new(fvk, note, merkle_path).ok_or(SpendError::FvkMismatch)?);
+        self.spends.push(spend);
 
         Ok(())
     }
@@ -465,12 +480,8 @@ pub fn bundle<V: TryFrom<i64>>(
     }
 
     for spend in &spends {
-        if spend.note.value() != NoteValue::zero() {
-            let cm = spend.note.commitment();
-            let path_root = spend.merkle_path.root(cm.into());
-            if path_root != anchor {
-                return Err(BuildError::AnchorMismatch);
-            }
+        if !spend.has_matching_anchor(anchor) {
+            return Err(BuildError::AnchorMismatch);
         }
     }