From a93c9d334e88969561fcc6119c58195ea01bf502 Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris.nuttycombe@gmail.com>
Date: Wed, 7 Sep 2022 11:10:34 -0600
Subject: [PATCH] Apply suggestions from code review

Co-authored-by: str4d <thestr4d@gmail.com>
---
 zcash_primitives/src/transaction/components/sprout.rs  | 10 ++++------
 .../src/transaction/components/transparent.rs          |  6 ++----
 zcash_primitives/src/transaction/mod.rs                |  3 ++-
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/zcash_primitives/src/transaction/components/sprout.rs b/zcash_primitives/src/transaction/components/sprout.rs
index 6484c8164..b3e2370f7 100644
--- a/zcash_primitives/src/transaction/components/sprout.rs
+++ b/zcash_primitives/src/transaction/components/sprout.rs
@@ -25,9 +25,7 @@ impl Bundle {
     pub fn value_balance(&self) -> Option<Amount> {
         self.joinsplits
             .iter()
-            .try_fold(Amount::zero(), |total, js| {
-                js.value_balance().and_then(|b| total + b)
-            })
+            .try_fold(Amount::zero(), |total, js| total + js.net_value())
     }
 }
 
@@ -187,11 +185,11 @@ impl JsDescription {
         writer.write_all(&self.ciphertexts[1])
     }
 
-    /// The value balance for the JoinSplit. When this is positive,
+    /// The net value for the JoinSplit. When this is positive,
     /// its value is added to the transparent value pool; when it
     /// is negative, its value is subtracted from the transparent
     /// value pool.
-    pub fn value_balance(&self) -> Option<Amount> {
-        self.vpub_new - self.vpub_old
+    pub fn net_value(&self) -> Amount {
+        (self.vpub_new - self.vpub_old).expect("difference is in range [-MAX_MONEY..=MAX_MONEY]")
     }
 }
diff --git a/zcash_primitives/src/transaction/components/transparent.rs b/zcash_primitives/src/transaction/components/transparent.rs
index 301a4f111..bae708158 100644
--- a/zcash_primitives/src/transaction/components/transparent.rs
+++ b/zcash_primitives/src/transaction/components/transparent.rs
@@ -68,12 +68,10 @@ impl<A: Authorization> Bundle<A> {
     /// transferred out of the transparent pool into shielded pools or to fees; a negative value
     /// means that the containing transaction has funds being transferred into the transparent pool
     /// from the shielded pools.
-    pub fn value_balance<E, F: FnMut(&OutPoint) -> Result<Amount, E>>(
-        &self,
-        mut get_prevout_value: F,
-    ) -> Result<Amount, E>
+    pub fn value_balance<E, F>(&self, mut get_prevout_value: F) -> Result<Amount, E>
     where
         E: From<BalanceError>,
+        F: FnMut(&OutPoint) -> Result<Amount, E>,
     {
         let input_sum = self.vin.iter().try_fold(Amount::zero(), |total, txin| {
             get_prevout_value(&txin.prevout)
diff --git a/zcash_primitives/src/transaction/mod.rs b/zcash_primitives/src/transaction/mod.rs
index fa0600f1b..d82528da3 100644
--- a/zcash_primitives/src/transaction/mod.rs
+++ b/zcash_primitives/src/transaction/mod.rs
@@ -420,7 +420,8 @@ impl<A: Authorization> TransactionData<A> {
                 .map_or_else(Amount::zero, |b| *b.value_balance()),
         ];
 
-        IntoIterator::into_iter(&value_balances)
+        value_balances
+            .iter()
             .sum::<Option<_>>()
             .ok_or_else(|| BalanceError::Overflow.into())
     }