Merge pull request #79 from 'murisi/guarantee-value-sum'

masp_primitives/src/convert.rs

@@ -223,12 +223,11 @@ mod tests {
     #[test]
     fn test_homomorphism() {
         // Left operand
-        let a = ValueSum::from_pair(zec(), 5i128).unwrap()
-            + ValueSum::from_pair(btc(), 6i128).unwrap()
-            + ValueSum::from_pair(xan(), 7i128).unwrap();
+        let a = ValueSum::from_pair(zec(), 5i128)
+            + ValueSum::from_pair(btc(), 6i128)
+            + ValueSum::from_pair(xan(), 7i128);
         // Right operand
-        let b = ValueSum::from_pair(zec(), 2i128).unwrap()
-            + ValueSum::from_pair(xan(), 10i128).unwrap();
+        let b = ValueSum::from_pair(zec(), 2i128) + ValueSum::from_pair(xan(), 10i128);
         // Test homomorphism
         assert_eq!(
             AllowedConversion::from(a.clone() + b.clone()),
@@ -238,9 +237,9 @@ mod tests {
     #[test]
     fn test_serialization() {
         // Make conversion
-        let a: AllowedConversion = (ValueSum::from_pair(zec(), 5i128).unwrap()
-            + ValueSum::from_pair(btc(), 6i128).unwrap()
-            + ValueSum::from_pair(xan(), 7i128).unwrap())
+        let a: AllowedConversion = (ValueSum::from_pair(zec(), 5i128)
+            + ValueSum::from_pair(btc(), 6i128)
+            + ValueSum::from_pair(xan(), 7i128))
         .into();
         // Serialize conversion
         let mut data = Vec::new();

masp_primitives/src/transaction/builder.rs

@@ -277,13 +277,13 @@ impl<P: consensus::Parameters> Builder<P> {
     }
 
     /// Returns the sum of the transparent, Sapling, and TZE value balances.
-    pub fn value_balance(&self) -> Result<I128Sum, BalanceError> {
+    pub fn value_balance(&self) -> I128Sum {
         let value_balances = [
-            self.transparent_builder.value_balance()?,
+            self.transparent_builder.value_balance(),
             self.sapling_builder.value_balance(),
         ];
 
-        Ok(value_balances.into_iter().sum::<I128Sum>())
+        value_balances.into_iter().sum::<I128Sum>()
     }
 
     /// Builds a transaction from the configured spends and outputs.
@@ -326,7 +326,7 @@ impl<P: consensus::Parameters> Builder<P> {
         //
 
         // After fees are accounted for, the value balance of the transaction must be zero.
-        let balance_after_fees = self.value_balance()? - I128Sum::from_sum(fee);
+        let balance_after_fees = self.value_balance() - I128Sum::from_sum(fee);
 
         if balance_after_fees != ValueSum::zero() {
             return Err(Error::InsufficientFunds(-balance_after_fees));
@@ -595,8 +595,7 @@ mod tests {
             assert_eq!(
                 builder.mock_build(&mut OsRng, &mut build_s::RngBuildParams::new(OsRng)),
                 Err(Error::InsufficientFunds(
-                    I128Sum::from_pair(zec(), 50000).unwrap()
-                        + &I128Sum::from_sum(DEFAULT_FEE.clone())
+                    I128Sum::from_pair(zec(), 50000) + &I128Sum::from_sum(DEFAULT_FEE.clone())
                 ))
             );
         }
@@ -611,8 +610,7 @@ mod tests {
             assert_eq!(
                 builder.mock_build(&mut OsRng, &mut build_s::RngBuildParams::new(OsRng)),
                 Err(Error::InsufficientFunds(
-                    I128Sum::from_pair(zec(), 50000).unwrap()
-                        + &I128Sum::from_sum(DEFAULT_FEE.clone())
+                    I128Sum::from_pair(zec(), 50000) + &I128Sum::from_sum(DEFAULT_FEE.clone())
                 ))
             );
         }
@@ -644,9 +642,7 @@ mod tests {
                 .unwrap();
             assert_eq!(
                 builder.mock_build(&mut OsRng, &mut build_s::RngBuildParams::new(OsRng)),
-                Err(Error::InsufficientFunds(
-                    ValueSum::from_pair(zec(), 1).unwrap()
-                ))
+                Err(Error::InsufficientFunds(ValueSum::from_pair(zec(), 1)))
             );
         }
 

masp_primitives/src/transaction/components/amount.rs

@@ -17,7 +17,7 @@ use zcash_encoding::Vector;
 
 pub const MAX_MONEY: u64 = u64::MAX;
 lazy_static::lazy_static! {
-pub static ref DEFAULT_FEE: U64Sum = ValueSum::from_pair(zec(), 1000).unwrap();
+pub static ref DEFAULT_FEE: U64Sum = ValueSum::from_pair(zec(), 1000);
 }
 /// A type-safe representation of some quantity of Zcash.
 ///
@@ -99,20 +99,20 @@ where
     Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default,
 {
     /// Creates an ValueSum from a Value.
-    pub fn from_pair(atype: Unit, amount: Value) -> Result<Self, ()> {
+    pub fn from_pair(atype: Unit, amount: Value) -> Self {
         if amount == Value::default() {
-            Ok(Self::zero())
+            Self::zero()
         } else {
             let mut ret = BTreeMap::new();
             ret.insert(atype, amount);
-            Ok(ValueSum(ret))
+            ValueSum(ret)
         }
     }
 
     /// Filters out everything but the given AssetType from this ValueSum
     pub fn project(&self, index: Unit) -> Self {
         let val = self.0.get(&index).copied().unwrap_or_default();
-        Self::from_pair(index, val).unwrap()
+        Self::from_pair(index, val)
     }
 
     /// Get the given AssetType within this ValueSum
@@ -252,9 +252,7 @@ impl ValueSum<AssetType, i32> {
         })?;
         let mut ret = Self::zero();
         for (atype, amt) in vec {
-            ret += Self::from_pair(atype, amt).map_err(|_| {
-                std::io::Error::new(std::io::ErrorKind::InvalidData, "amount out of range")
-            })?;
+            ret += Self::from_pair(atype, amt);
         }
         Ok(ret)
     }
@@ -283,9 +281,7 @@ impl ValueSum<AssetType, i64> {
         })?;
         let mut ret = Self::zero();
         for (atype, amt) in vec {
-            ret += Self::from_pair(atype, amt).map_err(|_| {
-                std::io::Error::new(std::io::ErrorKind::InvalidData, "amount out of range")
-            })?;
+            ret += Self::from_pair(atype, amt);
         }
         Ok(ret)
     }
@@ -314,9 +310,7 @@ impl ValueSum<AssetType, i128> {
         })?;
         let mut ret = Self::zero();
         for (atype, amt) in vec {
-            ret += Self::from_pair(atype, amt).map_err(|_| {
-                std::io::Error::new(std::io::ErrorKind::InvalidData, "amount out of range")
-            })?;
+            ret += Self::from_pair(atype, amt);
         }
         Ok(ret)
     }
@@ -731,7 +725,7 @@ pub fn zec() -> AssetType {
 }
 
 pub fn default_fee() -> ValueSum<AssetType, i64> {
-    ValueSum::from_pair(zec(), 10000).unwrap()
+    ValueSum::from_pair(zec(), 10000)
 }
 
 #[cfg(any(test, feature = "test-dependencies"))]
@@ -743,25 +737,25 @@ pub mod testing {
 
     prop_compose! {
         pub fn arb_i64_sum()(asset_type in arb_asset_type(), amt in i64::MIN..i64::MAX) -> I64Sum {
-            ValueSum::from_pair(asset_type, amt).unwrap()
+            ValueSum::from_pair(asset_type, amt)
         }
     }
 
     prop_compose! {
         pub fn arb_i128_sum()(asset_type in arb_asset_type(), amt in i128::MIN..i128::MAX) -> I128Sum {
-            ValueSum::from_pair(asset_type, amt).unwrap()
+            ValueSum::from_pair(asset_type, amt)
         }
     }
 
     prop_compose! {
         pub fn arb_nonnegative_amount()(asset_type in arb_asset_type(), amt in 0u64..MAX_MONEY) -> U64Sum {
-            ValueSum::from_pair(asset_type, amt).unwrap()
+            ValueSum::from_pair(asset_type, amt)
         }
     }
 
     prop_compose! {
         pub fn arb_positive_amount()(asset_type in arb_asset_type(), amt in 1u64..MAX_MONEY) -> U64Sum {
-            ValueSum::from_pair(asset_type, amt).unwrap()
+            ValueSum::from_pair(asset_type, amt)
         }
     }
 }
@@ -792,31 +786,31 @@ mod tests {
         }
         macro_rules! value_amount {
             ($t:ty, $val:expr) => {{
-                let mut amount = <$t>::from_pair(zec(), 1).unwrap();
+                let mut amount = <$t>::from_pair(zec(), 1);
                 *amount.0.get_mut(&zec()).unwrap() = $val;
                 amount
             }};
         }
 
         let test_amounts_i32 = [
             value_amount!(I32Sum, 0), // zec() asset with value 0
-            I32Sum::from_pair(zec(), -1).unwrap(),
-            I32Sum::from_pair(zec(), i32::MAX).unwrap(),
-            I32Sum::from_pair(zec(), -i32::MAX).unwrap(),
+            I32Sum::from_pair(zec(), -1),
+            I32Sum::from_pair(zec(), i32::MAX),
+            I32Sum::from_pair(zec(), -i32::MAX),
         ];
 
         let test_amounts_i64 = [
             value_amount!(I64Sum, 0), // zec() asset with value 0
-            I64Sum::from_pair(zec(), -1).unwrap(),
-            I64Sum::from_pair(zec(), i64::MAX).unwrap(),
-            I64Sum::from_pair(zec(), -i64::MAX).unwrap(),
+            I64Sum::from_pair(zec(), -1),
+            I64Sum::from_pair(zec(), i64::MAX),
+            I64Sum::from_pair(zec(), -i64::MAX),
         ];
 
         let test_amounts_i128 = [
             value_amount!(I128Sum, 0), // zec() asset with value 0
-            I128Sum::from_pair(zec(), MAX_MONEY as i128).unwrap(),
+            I128Sum::from_pair(zec(), MAX_MONEY as i128),
             value_amount!(I128Sum, MAX_MONEY as i128 + 1),
-            I128Sum::from_pair(zec(), -(MAX_MONEY as i128)).unwrap(),
+            I128Sum::from_pair(zec(), -(MAX_MONEY as i128)),
             value_amount!(I128Sum, -(MAX_MONEY as i128 - 1)),
         ];
 
@@ -898,28 +892,28 @@ mod tests {
     #[test]
     #[should_panic]
     fn add_panics_on_overflow() {
-        let v = ValueSum::from_pair(zec(), MAX_MONEY).unwrap();
-        let _sum = v + ValueSum::from_pair(zec(), 1).unwrap();
+        let v = ValueSum::from_pair(zec(), MAX_MONEY);
+        let _sum = v + ValueSum::from_pair(zec(), 1);
     }
 
     #[test]
     #[should_panic]
     fn add_assign_panics_on_overflow() {
-        let mut a = ValueSum::from_pair(zec(), MAX_MONEY).unwrap();
-        a += ValueSum::from_pair(zec(), 1).unwrap();
+        let mut a = ValueSum::from_pair(zec(), MAX_MONEY);
+        a += ValueSum::from_pair(zec(), 1);
     }
 
     #[test]
     #[should_panic]
     fn sub_panics_on_underflow() {
-        let v = ValueSum::from_pair(zec(), 0u64).unwrap();
-        let _diff = v - ValueSum::from_pair(zec(), 1).unwrap();
+        let v = ValueSum::from_pair(zec(), 0u64);
+        let _diff = v - ValueSum::from_pair(zec(), 1);
     }
 
     #[test]
     #[should_panic]
     fn sub_assign_panics_on_underflow() {
-        let mut a = ValueSum::from_pair(zec(), 0u64).unwrap();
-        a -= ValueSum::from_pair(zec(), 1).unwrap();
+        let mut a = ValueSum::from_pair(zec(), 0u64);
+        a -= ValueSum::from_pair(zec(), 1);
     }
 }

masp_primitives/src/transaction/components/sapling/builder.rs

@@ -652,8 +652,7 @@ impl<P: consensus::Parameters> SaplingBuilder<P> {
             self.spend_anchor = Some(merkle_path.root(node).into())
         }
 
-        self.value_balance += ValueSum::from_pair(note.asset_type, note.value.into())
-            .map_err(|_| Error::InvalidAmount)?;
+        self.value_balance += ValueSum::from_pair(note.asset_type, note.value.into());
 
         self.spends.push(SpendDescriptionInfo {
             extsk,
@@ -711,8 +710,7 @@ impl<P: consensus::Parameters> SaplingBuilder<P> {
     ) -> Result<(), Error> {
         let output = SaplingOutputInfo::new_internal(ovk, to, asset_type, value, memo)?;
 
-        self.value_balance -=
-            ValueSum::from_pair(asset_type, value.into()).map_err(|_| Error::InvalidAmount)?;
+        self.value_balance -= ValueSum::from_pair(asset_type, value.into());
 
         self.outputs.push(output);
 

masp_primitives/src/transaction/components/transparent.rs

@@ -63,26 +63,24 @@ 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>(&self) -> Result<I128Sum, E>
+    pub fn value_balance<E, F>(&self) -> I128Sum
     where
         E: From<BalanceError>,
     {
         let input_sum = self
             .vin
             .iter()
             .map(|p| ValueSum::from_pair(p.asset_type, p.value as i128))
-            .sum::<Result<I128Sum, ()>>()
-            .map_err(|_| BalanceError::Overflow)?;
+            .sum::<I128Sum>();
 
         let output_sum = self
             .vout
             .iter()
             .map(|p| ValueSum::from_pair(p.asset_type, p.value as i128))
-            .sum::<Result<I128Sum, ()>>()
-            .map_err(|_| BalanceError::Overflow)?;
+            .sum::<I128Sum>();
 
         // Cannot panic when subtracting two positive i64
-        Ok(input_sum - output_sum)
+        input_sum - output_sum
     }
 }
 

masp_primitives/src/transaction/components/transparent/builder.rs

@@ -6,7 +6,7 @@ use crate::{
     asset_type::AssetType,
     transaction::{
         components::{
-            amount::{BalanceError, I128Sum, ValueSum, MAX_MONEY},
+            amount::{I128Sum, ValueSum, MAX_MONEY},
             transparent::{self, fees, Authorization, Authorized, Bundle, TxIn, TxOut},
         },
         sighash::TransparentAuthorizingContext,
@@ -130,14 +130,13 @@ impl TransparentBuilder {
         Ok(())
     }
 
-    pub fn value_balance(&self) -> Result<I128Sum, BalanceError> {
+    pub fn value_balance(&self) -> I128Sum {
         #[cfg(feature = "transparent-inputs")]
         let input_sum = self
             .inputs
             .iter()
             .map(|input| ValueSum::from_pair(input.coin.asset_type, input.coin.value as i128))
-            .sum::<Result<I128Sum, ()>>()
-            .map_err(|_| BalanceError::Overflow)?;
+            .sum::<I128Sum>();
 
         #[cfg(not(feature = "transparent-inputs"))]
         let input_sum = ValueSum::zero();
@@ -146,11 +145,10 @@ impl TransparentBuilder {
             .vout
             .iter()
             .map(|vo| ValueSum::from_pair(vo.asset_type, vo.value as i128))
-            .sum::<Result<I128Sum, ()>>()
-            .map_err(|_| BalanceError::Overflow)?;
+            .sum::<I128Sum>();
 
         // Cannot panic when subtracting two positive i64
-        Ok(input_sum - output_sum)
+        input_sum - output_sum
     }
 
     pub fn build(self) -> Option<transparent::Bundle<Unauthorized>> {

masp_proofs/benches/convert.rs

@@ -39,9 +39,8 @@ fn criterion_benchmark(c: &mut Criterion) {
         let mint_value = i as i128 + 1;
 
         let allowed_conversion: AllowedConversion = (ValueSum::from_pair(spend_asset, spend_value)
-            .unwrap()
-            + ValueSum::from_pair(output_asset, output_value).unwrap()
-            + ValueSum::from_pair(mint_asset, mint_value).unwrap())
+            + ValueSum::from_pair(output_asset, output_value)
+            + ValueSum::from_pair(mint_asset, mint_value))
         .into();
 
         let value = rng.next_u64();

masp_proofs/src/circuit/convert.rs

@@ -155,9 +155,8 @@ fn test_convert_circuit_with_bls12_381() {
         let mint_value = i as i128 + 1;
 
         let allowed_conversion: AllowedConversion = (ValueSum::from_pair(spend_asset, spend_value)
-            .unwrap()
-            + ValueSum::from_pair(output_asset, output_value).unwrap()
-            + ValueSum::from_pair(mint_asset, mint_value).unwrap())
+            + ValueSum::from_pair(output_asset, output_value)
+            + ValueSum::from_pair(mint_asset, mint_value))
         .into();
 
         let value = rng.next_u64();