From bc046909b78f5b82c05d55f8237835613f2f86a0 Mon Sep 17 00:00:00 2001 From: Murisi Tarusenga Date: Thu, 29 Jun 2023 16:46:39 +0200 Subject: [PATCH 1/8] Made Amount more generic. --- masp_primitives/Cargo.toml | 3 + masp_primitives/src/convert.rs | 26 +- masp_primitives/src/sapling/prover.rs | 8 +- masp_primitives/src/transaction.rs | 6 +- masp_primitives/src/transaction/builder.rs | 12 +- masp_primitives/src/transaction/components.rs | 2 +- .../src/transaction/components/amount.rs | 364 +++++++++++------- .../src/transaction/components/sapling.rs | 4 +- .../transaction/components/sapling/builder.rs | 31 +- .../src/transaction/components/transparent.rs | 8 +- .../components/transparent/builder.rs | 8 +- masp_primitives/src/transaction/fees.rs | 4 +- masp_primitives/src/transaction/fees/fixed.rs | 10 +- masp_proofs/src/prover.rs | 4 +- masp_proofs/src/sapling/prover.rs | 4 +- masp_proofs/src/sapling/verifier.rs | 4 +- masp_proofs/src/sapling/verifier/single.rs | 4 +- 17 files changed, 299 insertions(+), 203 deletions(-) diff --git a/masp_primitives/Cargo.toml b/masp_primitives/Cargo.toml index 31dd3c75..9eb2bf48 100644 --- a/masp_primitives/Cargo.toml +++ b/masp_primitives/Cargo.toml @@ -36,6 +36,9 @@ sha2 = "0.9" # - Metrics memuse = "0.2.1" +# - Checked arithmetic +num-traits = "0.2.14" + # - Secret management subtle = "2.2.3" diff --git a/masp_primitives/src/convert.rs b/masp_primitives/src/convert.rs index 82d78a36..ed0d47ec 100644 --- a/masp_primitives/src/convert.rs +++ b/masp_primitives/src/convert.rs @@ -3,7 +3,7 @@ use crate::{ pedersen_hash::{pedersen_hash, Personalization}, Node, ValueCommitment, }, - transaction::components::amount::Amount, + transaction::components::amount::{Amount, I64Amt}, }; use borsh::{BorshDeserialize, BorshSerialize}; use group::{Curve, GroupEncoding}; @@ -16,7 +16,7 @@ use std::{ #[derive(Clone, Debug, PartialEq, Eq)] pub struct AllowedConversion { /// The asset type that the note represents - assets: Amount, + assets: I64Amt, /// Memorize generator because it's expensive to recompute generator: jubjub::ExtendedPoint, } @@ -71,15 +71,15 @@ impl AllowedConversion { } } -impl From for Amount { - fn from(allowed_conversion: AllowedConversion) -> Amount { +impl From for I64Amt { + fn from(allowed_conversion: AllowedConversion) -> I64Amt { allowed_conversion.assets } } -impl From for AllowedConversion { +impl From for AllowedConversion { /// Produces an asset generator without cofactor cleared - fn from(assets: Amount) -> Self { + fn from(assets: I64Amt) -> Self { let mut asset_generator = jubjub::ExtendedPoint::identity(); for (asset, value) in assets.components() { // Compute the absolute value (failing if -i64::MAX is @@ -199,11 +199,11 @@ mod tests { #[test] fn test_homomorphism() { // Left operand - let a = Amount::from_pair(zec(), 5).unwrap() - + Amount::from_pair(btc(), 6).unwrap() - + Amount::from_pair(xan(), 7).unwrap(); + let a = Amount::from_pair(zec(), 5i64).unwrap() + + Amount::from_pair(btc(), 6i64).unwrap() + + Amount::from_pair(xan(), 7i64).unwrap(); // Right operand - let b = Amount::from_pair(zec(), 2).unwrap() + Amount::from_pair(xan(), 10).unwrap(); + let b = Amount::from_pair(zec(), 2i64).unwrap() + Amount::from_pair(xan(), 10i64).unwrap(); // Test homomorphism assert_eq!( AllowedConversion::from(a.clone() + b.clone()), @@ -213,9 +213,9 @@ mod tests { #[test] fn test_serialization() { // Make conversion - let a: AllowedConversion = (Amount::from_pair(zec(), 5).unwrap() - + Amount::from_pair(btc(), 6).unwrap() - + Amount::from_pair(xan(), 7).unwrap()) + let a: AllowedConversion = (Amount::from_pair(zec(), 5i64).unwrap() + + Amount::from_pair(btc(), 6i64).unwrap() + + Amount::from_pair(xan(), 7i64).unwrap()) .into(); // Serialize conversion let mut data = Vec::new(); diff --git a/masp_primitives/src/sapling/prover.rs b/masp_primitives/src/sapling/prover.rs index 03a442fb..c4faec38 100644 --- a/masp_primitives/src/sapling/prover.rs +++ b/masp_primitives/src/sapling/prover.rs @@ -8,7 +8,7 @@ use crate::{ redjubjub::{PublicKey, Signature}, Node, }, - transaction::components::{Amount, GROTH_PROOF_SIZE}, + transaction::components::{I64Amt, GROTH_PROOF_SIZE}, }; use super::{Diversifier, PaymentAddress, ProofGenerationKey, Rseed}; @@ -73,7 +73,7 @@ pub trait TxProver { fn binding_sig( &self, ctx: &mut Self::SaplingProvingContext, - amount: &Amount, + amount: &I64Amt, sighash: &[u8; 32], ) -> Result; } @@ -92,7 +92,7 @@ pub mod mock { redjubjub::{PublicKey, Signature}, Diversifier, Node, PaymentAddress, ProofGenerationKey, Rseed, }, - transaction::components::{Amount, GROTH_PROOF_SIZE}, + transaction::components::{I64Amt, GROTH_PROOF_SIZE}, }; use super::TxProver; @@ -169,7 +169,7 @@ pub mod mock { fn binding_sig( &self, _ctx: &mut Self::SaplingProvingContext, - _value: &Amount, + _value: &I64Amt, _sighash: &[u8; 32], ) -> Result { Err(()) diff --git a/masp_primitives/src/transaction.rs b/masp_primitives/src/transaction.rs index 3938ee30..9a6213a9 100644 --- a/masp_primitives/src/transaction.rs +++ b/masp_primitives/src/transaction.rs @@ -25,7 +25,7 @@ use crate::{ use self::{ components::{ - amount::Amount, + amount::{Amount, I64Amt}, sapling::{ self, ConvertDescriptionV5, OutputDescriptionV5, SpendDescription, SpendDescriptionV5, }, @@ -269,7 +269,7 @@ impl TransactionData { } impl TransactionData { - pub fn sapling_value_balance(&self) -> Amount { + pub fn sapling_value_balance(&self) -> I64Amt { self.sapling_bundle .as_ref() .map_or(Amount::zero(), |b| b.value_balance.clone()) @@ -355,7 +355,7 @@ impl Transaction { }) } - fn read_amount(mut reader: R) -> io::Result { + fn read_amount(mut reader: R) -> io::Result { Amount::read(&mut reader).map_err(|_| { io::Error::new( io::ErrorKind::InvalidData, diff --git a/masp_primitives/src/transaction/builder.rs b/masp_primitives/src/transaction/builder.rs index 63681b3b..c6ed82e3 100644 --- a/masp_primitives/src/transaction/builder.rs +++ b/masp_primitives/src/transaction/builder.rs @@ -19,7 +19,7 @@ use crate::{ sapling::{prover::TxProver, Diversifier, Node, Note, PaymentAddress}, transaction::{ components::{ - amount::{Amount, BalanceError, MAX_MONEY}, + amount::{Amount, BalanceError, I64Amt, MAX_MONEY}, sapling::{ self, builder::{SaplingBuilder, SaplingMetadata}, @@ -43,10 +43,10 @@ const DEFAULT_TX_EXPIRY_DELTA: u32 = 20; pub enum Error { /// Insufficient funds were provided to the transaction builder; the given /// additional amount is required in order to construct the transaction. - InsufficientFunds(Amount), + InsufficientFunds(I64Amt), /// The transaction has inputs in excess of outputs and fees; the user must /// add a change output. - ChangeRequired(Amount), + ChangeRequired(I64Amt), /// An error occurred in computing the fees for a transaction. Fee(FeeError), /// An overflow or underflow occurred when computing value balances @@ -293,13 +293,13 @@ impl Builder { } /// Returns the sum of the transparent, Sapling, and TZE value balances. - pub fn value_balance(&self) -> Result { + pub fn value_balance(&self) -> Result { let value_balances = [ self.transparent_builder.value_balance()?, self.sapling_builder.value_balance(), ]; - Ok(value_balances.into_iter().sum::()) + Ok(value_balances.into_iter().sum::()) } /// Builds a transaction from the configured spends and outputs. @@ -326,7 +326,7 @@ impl Builder { fn build_internal( self, prover: &impl TxProver, - fee: Amount, + fee: I64Amt, ) -> Result<(Transaction, SaplingMetadata), Error> { let consensus_branch_id = BranchId::for_height(&self.params, self.target_height); diff --git a/masp_primitives/src/transaction/components.rs b/masp_primitives/src/transaction/components.rs index db07c9a5..36f28a57 100644 --- a/masp_primitives/src/transaction/components.rs +++ b/masp_primitives/src/transaction/components.rs @@ -4,7 +4,7 @@ pub mod amount; pub mod sapling; pub mod transparent; pub use self::{ - amount::Amount, + amount::{Amount, I64Amt}, sapling::{ConvertDescription, OutputDescription, SpendDescription}, transparent::{TxIn, TxOut}, }; diff --git a/masp_primitives/src/transaction/components/amount.rs b/masp_primitives/src/transaction/components/amount.rs index 49da8501..cea4d71e 100644 --- a/masp_primitives/src/transaction/components/amount.rs +++ b/masp_primitives/src/transaction/components/amount.rs @@ -1,10 +1,10 @@ use crate::asset_type::AssetType; use borsh::{BorshDeserialize, BorshSerialize}; +use num_traits::{CheckedAdd, CheckedMul, CheckedNeg, CheckedSub}; use std::cmp::Ordering; use std::collections::btree_map::Keys; use std::collections::btree_map::{IntoIter, Iter}; use std::collections::BTreeMap; -use std::convert::TryInto; use std::hash::Hash; use std::io::{Read, Write}; use std::iter::Sum; @@ -12,8 +12,9 @@ use std::ops::{Add, AddAssign, Index, Mul, MulAssign, Neg, Sub, SubAssign}; use zcash_encoding::Vector; pub const MAX_MONEY: i64 = i64::MAX; +pub const MIN_MONEY: i64 = i64::MIN; lazy_static::lazy_static! { -pub static ref DEFAULT_FEE: Amount = Amount::from_pair(zec(), 1000).unwrap(); +pub static ref DEFAULT_FEE: Amount = Amount::from_pair(zec(), 1000).unwrap(); } /// A type-safe representation of some quantity of Zcash. /// @@ -25,12 +26,25 @@ pub static ref DEFAULT_FEE: Amount = Amount::from_pair(zec(), 1000).unwrap(); /// particular, a `Transaction` containing serialized invalid Amounts will be rejected /// by the network consensus rules. /// + +pub type I64Amt = Amount; + +pub type U64Amt = Amount; + +pub type I128Amt = Amount; + #[derive(Clone, Default, Debug, PartialEq, Eq, BorshSerialize, BorshDeserialize, Hash)] -pub struct Amount( - pub BTreeMap, +pub struct Amount< + Unit: Hash + Ord + BorshSerialize + BorshDeserialize, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq, + > ( + pub BTreeMap, ); -impl memuse::DynamicUsage for Amount { +impl memuse::DynamicUsage for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, +{ #[inline(always)] fn dynamic_usage(&self) -> usize { unimplemented!() @@ -44,20 +58,17 @@ impl memuse::DynamicUsage for Amount { } } -impl Amount { - /// Returns a zero-valued Amount. - pub fn zero() -> Self { - Amount(BTreeMap::new()) - } - +impl Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, +{ /// Creates a non-negative Amount from an i64. /// /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. - pub fn from_nonnegative>(atype: Unit, amount: Amt) -> Result { - let amount = amount.try_into().map_err(|_| ())?; - if amount == 0 { + pub fn from_nonnegative(atype: Unit, amount: Magnitude) -> Result { + if amount == Magnitude::default() { Ok(Self::zero()) - } else if 0 <= amount && amount <= MAX_MONEY { + } else if Magnitude::default() <= amount { let mut ret = BTreeMap::new(); ret.insert(atype, amount); Ok(Amount(ret)) @@ -65,50 +76,70 @@ impl Amount Err(()) } } +} + +impl Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, +{ /// Creates an Amount from a type convertible to i64. /// /// Returns an error if the amount is outside the range `{-MAX_MONEY..MAX_MONEY}`. - pub fn from_pair>(atype: Unit, amount: Amt) -> Result { - let amount = amount.try_into().map_err(|_| ())?; - if amount == 0 { + pub fn from_pair(atype: Unit, amount: Magnitude) -> Result { + if amount == Magnitude::default() { Ok(Self::zero()) - } else if -MAX_MONEY <= amount && amount <= MAX_MONEY { + } else { let mut ret = BTreeMap::new(); ret.insert(atype, amount); Ok(Amount(ret)) - } else { - Err(()) } } + /// Filters out everything but the given AssetType from this Amount + pub fn project(&self, index: Unit) -> Self { + let val = self.0.get(&index).copied().unwrap_or(Magnitude::default()); + Self::from_pair(index, val).unwrap() + } + + /// Get the given AssetType within this Amount + pub fn get(&self, index: &Unit) -> Magnitude { + *self.0.get(index).unwrap_or(&Magnitude::default()) + } +} + +impl Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, +{ + /// Returns a zero-valued Amount. + pub fn zero() -> Self { + Amount(BTreeMap::new()) + } + /// Returns an iterator over the amount's non-zero asset-types - pub fn asset_types(&self) -> Keys<'_, Unit, i64> { + pub fn asset_types(&self) -> Keys<'_, Unit, Magnitude> { self.0.keys() } /// Returns an iterator over the amount's non-zero components - pub fn components(&self) -> Iter<'_, Unit, i64> { + pub fn components(&self) -> Iter<'_, Unit, Magnitude> { self.0.iter() } /// Returns an iterator over the amount's non-zero components - pub fn into_components(self) -> IntoIter { + pub fn into_components(self) -> IntoIter { self.0.into_iter() } - /// Filters out everything but the given AssetType from this Amount - pub fn project(&self, index: Unit) -> Self { - let val = self.0.get(&index).copied().unwrap_or(0); - Self::from_pair(index, val).unwrap() - } - /// Filters out the given AssetType from this Amount pub fn reject(&self, index: Unit) -> Self { - self.clone() - self.project(index) + let mut val = self.clone(); + val.0.remove(&index); + val } } -impl Amount { +impl Amount { /// Deserialize an Amount object from a list of amounts denominated by /// different assets pub fn read(reader: &mut R) -> std::io::Result { @@ -143,32 +174,30 @@ impl Amount { } } -impl From for Amount { +impl From for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + From { fn from(atype: Unit) -> Self { let mut ret = BTreeMap::new(); - ret.insert(atype, 1); + ret.insert(atype, true.into()); Amount(ret) } } -impl PartialOrd for Amount { +impl PartialOrd for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, + Self: Sub +{ /// One Amount is more than or equal to another if each corresponding /// coordinate is more than the other's. fn partial_cmp(&self, other: &Self) -> Option { - let mut diff = other.clone(); - for (atype, amount) in self.components() { - let ent = diff[atype] - amount; - if ent == 0 { - diff.0.remove(atype); - } else { - diff.0.insert(atype.clone(), ent); - } - } - if diff.0.values().all(|x| *x == 0) { + let diff = other.clone() - self.clone(); + if diff.0.values().all(|x| *x == Default::default()) { Some(Ordering::Equal) - } else if diff.0.values().all(|x| *x >= 0) { + } else if diff.0.values().all(|x| *x >= Default::default()) { Some(Ordering::Less) - } else if diff.0.values().all(|x| *x <= 0) { + } else if diff.0.values().all(|x| *x <= Default::default()) { Some(Ordering::Greater) } else { None @@ -176,147 +205,203 @@ impl PartialOrd fo } } -impl Index<&Unit> for Amount { - type Output = i64; - /// Query how much of the given asset this amount contains - fn index(&self, index: &Unit) -> &Self::Output { - self.0.get(index).unwrap_or(&0) - } -} - -impl MulAssign for Amount { - fn mul_assign(&mut self, rhs: i64) { - for (_atype, amount) in self.0.iter_mut() { - let ent = *amount * rhs; - if -MAX_MONEY <= ent && ent <= MAX_MONEY { - *amount = ent; - } else { - panic!("multiplication should remain in range"); +macro_rules! impl_index { + ($struct_type:ty) => { + impl Index<&Unit> for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize, + { + type Output = $struct_type; + /// Query how much of the given asset this amount contains + fn index(&self, index: &Unit) -> &Self::Output { + self.0.get(index).unwrap_or(&0) } } } } -impl Mul for Amount { - type Output = Self; +impl_index!(i64); + +impl_index!(u64); - fn mul(mut self, rhs: i64) -> Self { - self *= rhs; - self +impl_index!(i128); + +impl MulAssign for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedMul, +{ + fn mul_assign(&mut self, rhs: Magnitude) { + *self = self.clone() * rhs; } } -impl AddAssign<&Amount> - for Amount +impl Mul for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedMul, { - fn add_assign(&mut self, rhs: &Self) { - for (atype, amount) in rhs.components() { - let ent = self[atype] + amount; - if ent == 0 { - self.0.remove(atype); - } else if -MAX_MONEY <= ent && ent <= MAX_MONEY { - self.0.insert(atype.clone(), ent); - } else { - panic!("addition should remain in range"); - } + type Output = Amount; + + fn mul(self, rhs: Magnitude) -> Self::Output { + let mut comps = BTreeMap::new(); + for (atype, amount) in self.0.iter() { + comps.insert(atype.clone(), amount.checked_mul(&rhs).expect("overflow detected")); } + comps.retain(|_, v| *v != Magnitude::default()); + Amount(comps) } } -impl AddAssign> - for Amount +impl AddAssign<&Amount> for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedAdd, { - fn add_assign(&mut self, rhs: Self) { + fn add_assign(&mut self, rhs: &Amount) { + *self = self.clone() + rhs; + } +} + +impl AddAssign> for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedAdd, +{ + fn add_assign(&mut self, rhs: Amount) { *self += &rhs } } -impl Add<&Amount> - for Amount +impl Add<&Amount> for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedAdd, { - type Output = Self; + type Output = Amount; - fn add(mut self, rhs: &Self) -> Self { - self += rhs; - self + fn add(self, rhs: &Amount) -> Self::Output { + let mut comps = self.0.clone(); + for (atype, amount) in rhs.components() { + comps.insert(atype.clone(), self.get(atype).checked_add(amount).expect("overflow detected")); + } + comps.retain(|_, v| *v != Magnitude::default()); + Amount(comps) } } -impl Add> - for Amount +impl Add> for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedAdd, { - type Output = Self; + type Output = Amount; - fn add(mut self, rhs: Self) -> Self { - self += &rhs; - self + fn add(self, rhs: Amount) -> Self::Output { + self + &rhs } } -impl SubAssign<&Amount> - for Amount +impl SubAssign<&Amount> for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedSub, { - fn sub_assign(&mut self, rhs: &Self) { - for (atype, amount) in rhs.components() { - let ent = self[atype] - amount; - if ent == 0 { - self.0.remove(atype); - } else if -MAX_MONEY <= ent && ent <= MAX_MONEY { - self.0.insert(atype.clone(), ent); - } else { - panic!("subtraction should remain in range"); - } - } + fn sub_assign(&mut self, rhs: &Amount) { + *self = self.clone() - rhs } } -impl SubAssign> - for Amount +impl SubAssign> for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedSub, { - fn sub_assign(&mut self, rhs: Self) { + fn sub_assign(&mut self, rhs: Amount) { *self -= &rhs } } -impl Neg for Amount { - type Output = Self; +impl Neg for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedNeg, +{ + type Output = Amount; - fn neg(mut self) -> Self { - for (_, amount) in self.0.iter_mut() { - *amount = -*amount; + fn neg(mut self) -> Self::Output { + let mut comps = BTreeMap::new(); + for (atype, amount) in self.0.iter_mut() { + comps.insert(atype.clone(), amount.checked_neg().expect("overflow detected")); } - self + comps.retain(|_, v| *v != Magnitude::default()); + Amount(comps) } } -impl Sub<&Amount> - for Amount +impl Sub<&Amount> for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, { - type Output = Self; + type Output = Amount; - fn sub(mut self, rhs: &Self) -> Self { - self -= rhs; - self + fn sub(self, rhs: &Amount) -> Self::Output { + let mut comps = self.0.clone(); + for (atype, amount) in rhs.components() { + comps.insert(atype.clone(), self.get(atype).checked_sub(&amount).expect("overflow detected")); + } + comps.retain(|_, v| *v != Magnitude::default()); + Amount(comps) } } -impl Sub> - for Amount +impl Sub> for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, { - type Output = Self; + type Output = Amount; - fn sub(mut self, rhs: Self) -> Self { - self -= &rhs; - self + fn sub(self, rhs: Amount) -> Self::Output { + self - &rhs } } -impl Sum for Amount { +impl Sum for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, + Self: Add, +{ fn sum>(iter: I) -> Self { iter.fold(Self::zero(), Add::add) } } +/// Workaround for the blanket implementation of TryFrom +pub struct TryFromNt(pub X); + +impl TryFrom>> for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, + Output: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + TryFrom, +{ + type Error = >::Error; + + fn try_from(x: TryFromNt>) -> Result { + let mut comps = BTreeMap::new(); + for (atype, amount) in x.0.0 { + comps.insert(atype, amount.try_into()?); + } + Ok(Self(comps)) + } +} + +/// Workaround for the blanket implementation of TryFrom +pub struct FromNt(pub X); + +impl From>> for Amount where + Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, + Output: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + From, +{ + fn from(x: FromNt>) -> Self { + let mut comps = BTreeMap::new(); + for (atype, amount) in x.0.0 { + comps.insert(atype, amount.into()); + } + Self(comps) + } +} + /// A type for balance violations in amount addition and subtraction /// (overflow and underflow of allowed ranges) #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -346,7 +431,7 @@ pub fn zec() -> AssetType { AssetType::new(b"ZEC").unwrap() } -pub fn default_fee() -> Amount { +pub fn default_fee() -> Amount { Amount::from_pair(zec(), 10000).unwrap() } @@ -354,23 +439,23 @@ pub fn default_fee() -> Amount { pub mod testing { use proptest::prelude::prop_compose; - use super::{Amount, MAX_MONEY}; + use super::{Amount, I64Amt, MAX_MONEY}; use crate::asset_type::testing::arb_asset_type; prop_compose! { - pub fn arb_amount()(asset_type in arb_asset_type(), amt in -MAX_MONEY..MAX_MONEY) -> Amount { + pub fn arb_amount()(asset_type in arb_asset_type(), amt in -MAX_MONEY..MAX_MONEY) -> I64Amt { Amount::from_pair(asset_type, amt).unwrap() } } prop_compose! { - pub fn arb_nonnegative_amount()(asset_type in arb_asset_type(), amt in 0i64..MAX_MONEY) -> Amount { + pub fn arb_nonnegative_amount()(asset_type in arb_asset_type(), amt in 0i64..MAX_MONEY) -> I64Amt { Amount::from_pair(asset_type, amt).unwrap() } } prop_compose! { - pub fn arb_positive_amount()(asset_type in arb_asset_type(), amt in 1i64..MAX_MONEY) -> Amount { + pub fn arb_positive_amount()(asset_type in arb_asset_type(), amt in 1i64..MAX_MONEY) -> I64Amt { Amount::from_pair(asset_type, amt).unwrap() } } @@ -378,7 +463,7 @@ pub mod testing { #[cfg(test)] mod tests { - use super::{zec, Amount, MAX_MONEY}; + use super::{zec, Amount, MAX_MONEY, MIN_MONEY}; #[test] fn amount_in_range() { @@ -412,9 +497,6 @@ mod tests { Amount::read(&mut neg_max_money.as_ref()).unwrap(), Amount::from_pair(zec(), -MAX_MONEY).unwrap() ); - - let neg_max_money_m1 = b"\x01\x94\xf3O\xfdd\xef\n\xc3i\x08\xfd\xdf\xec\x05hX\x06)\xc4Vq\x0f\xa1\x86\x83\x12\xa8\x7f\xbf\n\xa5\t\x00\x00\x00\x00\x00\x00\x00\x80"; - assert!(Amount::read(&mut neg_max_money_m1.as_ref()).is_err()); } #[test] @@ -434,14 +516,14 @@ mod tests { #[test] #[should_panic] fn sub_panics_on_underflow() { - let v = Amount::from_pair(zec(), -MAX_MONEY).unwrap(); + let v = Amount::from_pair(zec(), MIN_MONEY).unwrap(); let _diff = v - Amount::from_pair(zec(), 1).unwrap(); } #[test] #[should_panic] fn sub_assign_panics_on_underflow() { - let mut a = Amount::from_pair(zec(), -MAX_MONEY).unwrap(); + let mut a = Amount::from_pair(zec(), MIN_MONEY).unwrap(); a -= Amount::from_pair(zec(), 1).unwrap(); } } diff --git a/masp_primitives/src/transaction/components/sapling.rs b/masp_primitives/src/transaction/components/sapling.rs index 2048abd9..282de9f4 100644 --- a/masp_primitives/src/transaction/components/sapling.rs +++ b/masp_primitives/src/transaction/components/sapling.rs @@ -23,7 +23,7 @@ use crate::{ }, }; -use super::{amount::Amount, GROTH_PROOF_SIZE}; +use super::{amount::I64Amt, GROTH_PROOF_SIZE}; pub type GrothProofBytes = [u8; GROTH_PROOF_SIZE]; @@ -90,7 +90,7 @@ pub struct Bundle>, pub shielded_converts: Vec>, pub shielded_outputs: Vec>, - pub value_balance: Amount, + pub value_balance: I64Amt, pub authorization: A, } diff --git a/masp_primitives/src/transaction/components/sapling/builder.rs b/masp_primitives/src/transaction/components/sapling/builder.rs index 381295f0..e907e918 100644 --- a/masp_primitives/src/transaction/components/sapling/builder.rs +++ b/masp_primitives/src/transaction/components/sapling/builder.rs @@ -25,7 +25,7 @@ use crate::{ transaction::{ builder::Progress, components::{ - amount::{Amount, MAX_MONEY}, + amount::{Amount, I64Amt, I128Amt, TryFromNt, FromNt, MAX_MONEY}, sapling::{ fees, Authorization, Authorized, Bundle, ConvertDescription, GrothProofBytes, OutputDescription, SpendDescription, @@ -272,7 +272,7 @@ pub struct SaplingBuilder { params: P, spend_anchor: Option, target_height: BlockHeight, - value_balance: Amount, + value_balance: Amount, convert_anchor: Option, spends: Vec>, converts: Vec, @@ -303,7 +303,7 @@ impl BorshDeserialize for SaplingBui .map(|x| x.ok_or_else(|| std::io::Error::from(std::io::ErrorKind::InvalidData))) .transpose()?; let target_height = BlockHeight::deserialize(buf)?; - let value_balance: Amount = Amount::deserialize(buf)?; + let value_balance = Amount::::deserialize(buf)?; let convert_anchor: Option> = Option::<[u8; 32]>::deserialize(buf)?.map(|x| bls12_381::Scalar::from_bytes(&x).into()); let convert_anchor = convert_anchor @@ -369,9 +369,19 @@ impl SaplingBuilder { &self.outputs } + /// Returns the net value represented by the spends and outputs added to this builder, + /// or an error if the values added to this builder overflow the range of a Zcash + /// monetary amount. + fn try_value_balance(&self) -> Result { + TryFromNt(self.value_balance.clone()) + .try_into() + .map_err(|_| Error::InvalidAmount) + } + /// Returns the net value represented by the spends and outputs added to this builder. - pub fn value_balance(&self) -> Amount { - self.value_balance.clone() + pub fn value_balance(&self) -> I64Amt { + self.try_value_balance() + .expect("we check this when mutating self.value_balance") } } @@ -402,7 +412,7 @@ impl SaplingBuilder

{ let alpha = jubjub::Fr::random(&mut rng); self.value_balance += - Amount::from_pair(note.asset_type, note.value).map_err(|_| Error::InvalidAmount)?; + I128Amt::from(FromNt(Amount::from_pair(note.asset_type, note.value).map_err(|_| Error::InvalidAmount)?)); self.spends.push(SpendDescriptionInfo { extsk, @@ -437,8 +447,8 @@ impl SaplingBuilder

{ self.convert_anchor = Some(merkle_path.root(node).into()) } - let allowed_amt: Amount = allowed.clone().into(); - self.value_balance += allowed_amt * value.try_into().unwrap(); + let allowed_amt: I64Amt = allowed.clone().into(); + self.value_balance += I128Amt::from(FromNt(allowed_amt * (value as i64))); self.converts.push(ConvertDescriptionInfo { allowed, @@ -472,7 +482,7 @@ impl SaplingBuilder

{ )?; self.value_balance -= - Amount::from_pair(asset_type, value).map_err(|_| Error::InvalidAmount)?; + I128Amt::from(FromNt(Amount::from_pair(asset_type, value).map_err(|_| Error::InvalidAmount)?)); self.outputs.push(output); @@ -488,6 +498,7 @@ impl SaplingBuilder

{ progress_notifier: Option<&Sender>, ) -> Result>, Error> { // Record initial positions of spends and outputs + let value_balance = self.try_value_balance()?; let params = self.params; let mut indexed_spends: Vec<_> = self.spends.into_iter().enumerate().collect(); let mut indexed_converts: Vec<_> = self.converts.into_iter().enumerate().collect(); @@ -723,7 +734,7 @@ impl SaplingBuilder

{ shielded_spends, shielded_converts, shielded_outputs, - value_balance: self.value_balance, + value_balance, authorization: Unauthorized { tx_metadata }, }) }; diff --git a/masp_primitives/src/transaction/components/transparent.rs b/masp_primitives/src/transaction/components/transparent.rs index 06efcaee..a34ce73f 100644 --- a/masp_primitives/src/transaction/components/transparent.rs +++ b/masp_primitives/src/transaction/components/transparent.rs @@ -7,7 +7,7 @@ use std::io::{self, Read, Write}; use crate::asset_type::AssetType; use crate::transaction::TransparentAddress; -use super::amount::{Amount, BalanceError, MAX_MONEY}; +use super::amount::{Amount, BalanceError, I64Amt, MAX_MONEY}; pub mod builder; pub mod fees; @@ -58,7 +58,7 @@ impl Bundle { /// 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(&self) -> Result + pub fn value_balance(&self) -> Result where E: From, { @@ -72,7 +72,7 @@ impl Bundle { Err(()) } }) - .sum::>() + .sum::>() .map_err(|_| BalanceError::Overflow)?; let output_sum = self @@ -85,7 +85,7 @@ impl Bundle { Err(()) } }) - .sum::>() + .sum::>() .map_err(|_| BalanceError::Overflow)?; // Cannot panic when subtracting two positive i64 diff --git a/masp_primitives/src/transaction/components/transparent/builder.rs b/masp_primitives/src/transaction/components/transparent/builder.rs index 6c0cc642..dfd1c3d6 100644 --- a/masp_primitives/src/transaction/components/transparent/builder.rs +++ b/masp_primitives/src/transaction/components/transparent/builder.rs @@ -6,7 +6,7 @@ use crate::{ asset_type::AssetType, transaction::{ components::{ - amount::{Amount, BalanceError, MAX_MONEY}, + amount::{Amount, BalanceError, I64Amt, MAX_MONEY}, transparent::{self, fees, Authorization, Authorized, Bundle, TxIn, TxOut}, }, sighash::TransparentAuthorizingContext, @@ -133,7 +133,7 @@ impl TransparentBuilder { Ok(()) } - pub fn value_balance(&self) -> Result { + pub fn value_balance(&self) -> Result { #[cfg(feature = "transparent-inputs")] let input_sum = self .inputs @@ -145,7 +145,7 @@ impl TransparentBuilder { Err(()) } }) - .sum::>() + .sum::>() .map_err(|_| BalanceError::Overflow)?; #[cfg(not(feature = "transparent-inputs"))] @@ -161,7 +161,7 @@ impl TransparentBuilder { Err(()) } }) - .sum::>() + .sum::>() .map_err(|_| BalanceError::Overflow)?; // Cannot panic when subtracting two positive i64 diff --git a/masp_primitives/src/transaction/fees.rs b/masp_primitives/src/transaction/fees.rs index 0108e20f..b4af61aa 100644 --- a/masp_primitives/src/transaction/fees.rs +++ b/masp_primitives/src/transaction/fees.rs @@ -2,7 +2,7 @@ use crate::{ consensus::{self, BlockHeight}, - transaction::components::{amount::Amount, transparent::fees as transparent}, + transaction::components::{amount::I64Amt, transparent::fees as transparent}, }; pub mod fixed; @@ -24,5 +24,5 @@ pub trait FeeRule { transparent_outputs: &[impl transparent::OutputView], sapling_input_count: usize, sapling_output_count: usize, - ) -> Result; + ) -> Result; } diff --git a/masp_primitives/src/transaction/fees/fixed.rs b/masp_primitives/src/transaction/fees/fixed.rs index 02e3bd93..6dbea37d 100644 --- a/masp_primitives/src/transaction/fees/fixed.rs +++ b/masp_primitives/src/transaction/fees/fixed.rs @@ -1,7 +1,7 @@ use crate::{ consensus::{self, BlockHeight}, transaction::components::{ - amount::{Amount, DEFAULT_FEE}, + amount::{I64Amt, DEFAULT_FEE}, transparent::fees as transparent, }, }; @@ -10,12 +10,12 @@ use crate::{ /// the transaction being constructed. #[derive(Clone, Debug)] pub struct FeeRule { - fixed_fee: Amount, + fixed_fee: I64Amt, } impl FeeRule { /// Creates a new nonstandard fixed fee rule with the specified fixed fee. - pub fn non_standard(fixed_fee: Amount) -> Self { + pub fn non_standard(fixed_fee: I64Amt) -> Self { Self { fixed_fee } } @@ -27,7 +27,7 @@ impl FeeRule { } /// Returns the fixed fee amount which which this rule was configured. - pub fn fixed_fee(&self) -> Amount { + pub fn fixed_fee(&self) -> I64Amt { self.fixed_fee.clone() } } @@ -42,7 +42,7 @@ impl super::FeeRule for FeeRule { _transparent_outputs: &[impl transparent::OutputView], _sapling_input_count: usize, _sapling_output_count: usize, - ) -> Result { + ) -> Result { Ok(self.fixed_fee.clone()) } } diff --git a/masp_proofs/src/prover.rs b/masp_proofs/src/prover.rs index b61fbc1b..77fdf187 100644 --- a/masp_proofs/src/prover.rs +++ b/masp_proofs/src/prover.rs @@ -11,7 +11,7 @@ use masp_primitives::{ redjubjub::{PublicKey, Signature}, Diversifier, Node, PaymentAddress, ProofGenerationKey, Rseed, }, - transaction::components::{Amount, GROTH_PROOF_SIZE}, + transaction::components::{I64Amt, GROTH_PROOF_SIZE}, }; use std::path::Path; @@ -247,7 +247,7 @@ impl TxProver for LocalTxProver { fn binding_sig( &self, ctx: &mut Self::SaplingProvingContext, - assets_and_values: &Amount, //&[(AssetType, i64)], + assets_and_values: &I64Amt, //&[(AssetType, i64)], sighash: &[u8; 32], ) -> Result { ctx.binding_sig(assets_and_values, sighash) diff --git a/masp_proofs/src/sapling/prover.rs b/masp_proofs/src/sapling/prover.rs index ad5eaf7b..21a5a914 100644 --- a/masp_proofs/src/sapling/prover.rs +++ b/masp_proofs/src/sapling/prover.rs @@ -13,7 +13,7 @@ use masp_primitives::{ redjubjub::{PrivateKey, PublicKey, Signature}, Diversifier, Node, Note, PaymentAddress, ProofGenerationKey, Rseed, }, - transaction::components::Amount, + transaction::components::I64Amt, }; use rand_core::OsRng; use std::ops::{AddAssign, Neg}; @@ -284,7 +284,7 @@ impl SaplingProvingContext { /// and output_proof() must be completed before calling this function. pub fn binding_sig( &self, - assets_and_values: &Amount, + assets_and_values: &I64Amt, sighash: &[u8; 32], ) -> Result { // Initialize secure RNG diff --git a/masp_proofs/src/sapling/verifier.rs b/masp_proofs/src/sapling/verifier.rs index 22cceafb..d31e4259 100644 --- a/masp_proofs/src/sapling/verifier.rs +++ b/masp_proofs/src/sapling/verifier.rs @@ -5,7 +5,7 @@ use bls12_381::Bls12; use group::{Curve, GroupEncoding}; use masp_primitives::{ sapling::redjubjub::{PublicKey, Signature}, - transaction::components::Amount, + transaction::components::I64Amt, }; use super::masp_compute_value_balance; @@ -172,7 +172,7 @@ impl SaplingVerificationContextInner { /// have been checked before calling this function. fn final_check( &self, - value_balance: Amount, + value_balance: I64Amt, sighash_value: &[u8; 32], binding_sig: Signature, binding_sig_verifier: impl FnOnce(PublicKey, [u8; 64], Signature) -> bool, diff --git a/masp_proofs/src/sapling/verifier/single.rs b/masp_proofs/src/sapling/verifier/single.rs index 2df7dfae..9cd59071 100644 --- a/masp_proofs/src/sapling/verifier/single.rs +++ b/masp_proofs/src/sapling/verifier/single.rs @@ -3,7 +3,7 @@ use bls12_381::Bls12; use masp_primitives::{ constants::{SPENDING_KEY_GENERATOR, VALUE_COMMITMENT_RANDOMNESS_GENERATOR}, sapling::redjubjub::{PublicKey, Signature}, - transaction::components::Amount, + transaction::components::I64Amt, }; use super::SaplingVerificationContextInner; @@ -98,7 +98,7 @@ impl SaplingVerificationContext { /// have been checked before calling this function. pub fn final_check( &self, - value_balance: Amount, + value_balance: I64Amt, sighash_value: &[u8; 32], binding_sig: Signature, ) -> bool { From bf7f9199295e4fb7204e204b90140044c028b394 Mon Sep 17 00:00:00 2001 From: Murisi Tarusenga Date: Thu, 29 Jun 2023 20:16:08 +0200 Subject: [PATCH 2/8] Fixed clippy and formatting issues. --- .../src/transaction/components/amount.rs | 187 +++++++++++++----- .../transaction/components/sapling/builder.rs | 12 +- .../transaction/components/sapling/fees.rs | 2 +- .../components/transparent/fees.rs | 2 +- 4 files changed, 152 insertions(+), 51 deletions(-) diff --git a/masp_primitives/src/transaction/components/amount.rs b/masp_primitives/src/transaction/components/amount.rs index cea4d71e..78814d72 100644 --- a/masp_primitives/src/transaction/components/amount.rs +++ b/masp_primitives/src/transaction/components/amount.rs @@ -35,13 +35,12 @@ pub type I128Amt = Amount; #[derive(Clone, Default, Debug, PartialEq, Eq, BorshSerialize, BorshDeserialize, Hash)] pub struct Amount< - Unit: Hash + Ord + BorshSerialize + BorshDeserialize, + Unit: Hash + Ord + BorshSerialize + BorshDeserialize, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq, - > ( - pub BTreeMap, -); +>(pub BTreeMap); -impl memuse::DynamicUsage for Amount where +impl memuse::DynamicUsage for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, { @@ -58,7 +57,8 @@ impl memuse::DynamicUsage for Amount where } } -impl Amount where +impl Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, { @@ -78,7 +78,8 @@ impl Amount where } } -impl Amount where +impl Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, { @@ -97,7 +98,7 @@ impl Amount where /// Filters out everything but the given AssetType from this Amount pub fn project(&self, index: Unit) -> Self { - let val = self.0.get(&index).copied().unwrap_or(Magnitude::default()); + let val = self.0.get(&index).copied().unwrap_or_default(); Self::from_pair(index, val).unwrap() } @@ -107,7 +108,8 @@ impl Amount where } } -impl Amount where +impl Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, { @@ -174,9 +176,11 @@ impl Amount { } } -impl From for Amount where +impl From for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + From { + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + From, +{ fn from(atype: Unit) -> Self { let mut ret = BTreeMap::new(); ret.insert(atype, true.into()); @@ -184,10 +188,11 @@ impl From for Amount where } } -impl PartialOrd for Amount where +impl PartialOrd for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, - Self: Sub + Self: Sub, { /// One Amount is more than or equal to another if each corresponding /// coordinate is more than the other's. @@ -207,7 +212,8 @@ impl PartialOrd for Amount where macro_rules! impl_index { ($struct_type:ty) => { - impl Index<&Unit> for Amount where + impl Index<&Unit> for Amount + where Unit: Hash + Ord + BorshSerialize + BorshDeserialize, { type Output = $struct_type; @@ -216,7 +222,7 @@ macro_rules! impl_index { self.0.get(index).unwrap_or(&0) } } - } + }; } impl_index!(i64); @@ -225,68 +231,124 @@ impl_index!(u64); impl_index!(i128); -impl MulAssign for Amount where +impl MulAssign for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedMul, + Magnitude: BorshSerialize + + BorshDeserialize + + PartialEq + + Eq + + Copy + + Default + + PartialOrd + + CheckedMul, { fn mul_assign(&mut self, rhs: Magnitude) { *self = self.clone() * rhs; } } -impl Mul for Amount where +impl Mul for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedMul, + Magnitude: BorshSerialize + + BorshDeserialize + + PartialEq + + Eq + + Copy + + Default + + PartialOrd + + CheckedMul, { type Output = Amount; fn mul(self, rhs: Magnitude) -> Self::Output { let mut comps = BTreeMap::new(); for (atype, amount) in self.0.iter() { - comps.insert(atype.clone(), amount.checked_mul(&rhs).expect("overflow detected")); + comps.insert( + atype.clone(), + amount.checked_mul(&rhs).expect("overflow detected"), + ); } comps.retain(|_, v| *v != Magnitude::default()); Amount(comps) } } -impl AddAssign<&Amount> for Amount where +impl AddAssign<&Amount> for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedAdd, + Magnitude: BorshSerialize + + BorshDeserialize + + PartialEq + + Eq + + Copy + + Default + + PartialOrd + + CheckedAdd, { fn add_assign(&mut self, rhs: &Amount) { *self = self.clone() + rhs; } } -impl AddAssign> for Amount where +impl AddAssign> for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedAdd, + Magnitude: BorshSerialize + + BorshDeserialize + + PartialEq + + Eq + + Copy + + Default + + PartialOrd + + CheckedAdd, { fn add_assign(&mut self, rhs: Amount) { *self += &rhs } } -impl Add<&Amount> for Amount where +impl Add<&Amount> for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedAdd, + Magnitude: BorshSerialize + + BorshDeserialize + + PartialEq + + Eq + + Copy + + Default + + PartialOrd + + CheckedAdd, { type Output = Amount; fn add(self, rhs: &Amount) -> Self::Output { let mut comps = self.0.clone(); for (atype, amount) in rhs.components() { - comps.insert(atype.clone(), self.get(atype).checked_add(amount).expect("overflow detected")); + comps.insert( + atype.clone(), + self.get(atype) + .checked_add(amount) + .expect("overflow detected"), + ); } comps.retain(|_, v| *v != Magnitude::default()); Amount(comps) } } -impl Add> for Amount where +impl Add> for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedAdd, + Magnitude: BorshSerialize + + BorshDeserialize + + PartialEq + + Eq + + Copy + + Default + + PartialOrd + + CheckedAdd, { type Output = Amount; @@ -295,41 +357,69 @@ impl Add> for Amount w } } -impl SubAssign<&Amount> for Amount where +impl SubAssign<&Amount> for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedSub, + Magnitude: BorshSerialize + + BorshDeserialize + + PartialEq + + Eq + + Copy + + Default + + PartialOrd + + CheckedSub, { fn sub_assign(&mut self, rhs: &Amount) { *self = self.clone() - rhs } } -impl SubAssign> for Amount where +impl SubAssign> for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedSub, + Magnitude: BorshSerialize + + BorshDeserialize + + PartialEq + + Eq + + Copy + + Default + + PartialOrd + + CheckedSub, { fn sub_assign(&mut self, rhs: Amount) { *self -= &rhs } } -impl Neg for Amount where +impl Neg for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd + CheckedNeg, + Magnitude: BorshSerialize + + BorshDeserialize + + PartialEq + + Eq + + Copy + + Default + + PartialOrd + + CheckedNeg, { type Output = Amount; fn neg(mut self) -> Self::Output { let mut comps = BTreeMap::new(); for (atype, amount) in self.0.iter_mut() { - comps.insert(atype.clone(), amount.checked_neg().expect("overflow detected")); + comps.insert( + atype.clone(), + amount.checked_neg().expect("overflow detected"), + ); } comps.retain(|_, v| *v != Magnitude::default()); Amount(comps) } } -impl Sub<&Amount> for Amount where +impl Sub<&Amount> for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, { @@ -338,14 +428,20 @@ impl Sub<&Amount> for Amount fn sub(self, rhs: &Amount) -> Self::Output { let mut comps = self.0.clone(); for (atype, amount) in rhs.components() { - comps.insert(atype.clone(), self.get(atype).checked_sub(&amount).expect("overflow detected")); + comps.insert( + atype.clone(), + self.get(atype) + .checked_sub(amount) + .expect("overflow detected"), + ); } comps.retain(|_, v| *v != Magnitude::default()); Amount(comps) } } -impl Sub> for Amount where +impl Sub> for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, { @@ -356,7 +452,8 @@ impl Sub> for Amount w } } -impl Sum for Amount where +impl Sum for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, Self: Add, @@ -369,7 +466,8 @@ impl Sum for Amount where /// Workaround for the blanket implementation of TryFrom pub struct TryFromNt(pub X); -impl TryFrom>> for Amount where +impl TryFrom>> for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, Output: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + TryFrom, @@ -378,7 +476,7 @@ impl TryFrom>> for Am fn try_from(x: TryFromNt>) -> Result { let mut comps = BTreeMap::new(); - for (atype, amount) in x.0.0 { + for (atype, amount) in x.0 .0 { comps.insert(atype, amount.try_into()?); } Ok(Self(comps)) @@ -388,14 +486,15 @@ impl TryFrom>> for Am /// Workaround for the blanket implementation of TryFrom pub struct FromNt(pub X); -impl From>> for Amount where +impl From>> for Amount +where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, Output: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + From, { fn from(x: FromNt>) -> Self { let mut comps = BTreeMap::new(); - for (atype, amount) in x.0.0 { + for (atype, amount) in x.0 .0 { comps.insert(atype, amount.into()); } Self(comps) diff --git a/masp_primitives/src/transaction/components/sapling/builder.rs b/masp_primitives/src/transaction/components/sapling/builder.rs index e907e918..40c306df 100644 --- a/masp_primitives/src/transaction/components/sapling/builder.rs +++ b/masp_primitives/src/transaction/components/sapling/builder.rs @@ -25,7 +25,7 @@ use crate::{ transaction::{ builder::Progress, components::{ - amount::{Amount, I64Amt, I128Amt, TryFromNt, FromNt, MAX_MONEY}, + amount::{Amount, FromNt, I128Amt, I64Amt, TryFromNt, MAX_MONEY}, sapling::{ fees, Authorization, Authorized, Bundle, ConvertDescription, GrothProofBytes, OutputDescription, SpendDescription, @@ -411,8 +411,9 @@ impl SaplingBuilder

{ let alpha = jubjub::Fr::random(&mut rng); - self.value_balance += - I128Amt::from(FromNt(Amount::from_pair(note.asset_type, note.value).map_err(|_| Error::InvalidAmount)?)); + self.value_balance += I128Amt::from(FromNt( + Amount::from_pair(note.asset_type, note.value).map_err(|_| Error::InvalidAmount)?, + )); self.spends.push(SpendDescriptionInfo { extsk, @@ -481,8 +482,9 @@ impl SaplingBuilder

{ memo, )?; - self.value_balance -= - I128Amt::from(FromNt(Amount::from_pair(asset_type, value).map_err(|_| Error::InvalidAmount)?)); + self.value_balance -= I128Amt::from(FromNt( + Amount::from_pair(asset_type, value).map_err(|_| Error::InvalidAmount)?, + )); self.outputs.push(output); diff --git a/masp_primitives/src/transaction/components/sapling/fees.rs b/masp_primitives/src/transaction/components/sapling/fees.rs index c43395cb..93bf9497 100644 --- a/masp_primitives/src/transaction/components/sapling/fees.rs +++ b/masp_primitives/src/transaction/components/sapling/fees.rs @@ -2,8 +2,8 @@ //! of a transaction. use crate::asset_type::AssetType; -use crate::sapling::PaymentAddress; use crate::convert::AllowedConversion; +use crate::sapling::PaymentAddress; /// A trait that provides a minimized view of a Sapling input suitable for use in /// fee and change calculation. diff --git a/masp_primitives/src/transaction/components/transparent/fees.rs b/masp_primitives/src/transaction/components/transparent/fees.rs index 94836f05..cff0a544 100644 --- a/masp_primitives/src/transaction/components/transparent/fees.rs +++ b/masp_primitives/src/transaction/components/transparent/fees.rs @@ -2,8 +2,8 @@ //! of a transaction. use super::TxOut; -use crate::transaction::TransparentAddress; use crate::asset_type::AssetType; +use crate::transaction::TransparentAddress; /// This trait provides a minimized view of a transparent input suitable for use in /// fee and change computation. From 2c3229e7f0c3921d8cc00880903c453cfedd7ee5 Mon Sep 17 00:00:00 2001 From: Murisi Tarusenga Date: Wed, 26 Jul 2023 16:39:35 +0200 Subject: [PATCH 3/8] Changed valueBalance into a i128 in order to be able to hold sums of u64s. --- masp_primitives/src/convert.rs | 33 +-- masp_primitives/src/sapling/prover.rs | 8 +- masp_primitives/src/transaction.rs | 14 +- masp_primitives/src/transaction/builder.rs | 30 ++- masp_primitives/src/transaction/components.rs | 2 +- .../src/transaction/components/amount.rs | 222 ++++++++++++------ .../src/transaction/components/sapling.rs | 8 +- .../transaction/components/sapling/builder.rs | 36 +-- .../src/transaction/components/transparent.rs | 12 +- .../components/transparent/builder.rs | 14 +- masp_primitives/src/transaction/fees.rs | 4 +- masp_primitives/src/transaction/fees/fixed.rs | 10 +- masp_proofs/benches/convert.rs | 8 +- masp_proofs/src/circuit/convert.rs | 8 +- masp_proofs/src/prover.rs | 4 +- masp_proofs/src/sapling/mod.rs | 11 +- masp_proofs/src/sapling/prover.rs | 6 +- masp_proofs/src/sapling/verifier.rs | 4 +- masp_proofs/src/sapling/verifier/single.rs | 4 +- 19 files changed, 253 insertions(+), 185 deletions(-) diff --git a/masp_primitives/src/convert.rs b/masp_primitives/src/convert.rs index ed0d47ec..841d80ee 100644 --- a/masp_primitives/src/convert.rs +++ b/masp_primitives/src/convert.rs @@ -3,7 +3,7 @@ use crate::{ pedersen_hash::{pedersen_hash, Personalization}, Node, ValueCommitment, }, - transaction::components::amount::{Amount, I64Amt}, + transaction::components::amount::{I64Sum, ValueSum}, }; use borsh::{BorshDeserialize, BorshSerialize}; use group::{Curve, GroupEncoding}; @@ -16,7 +16,7 @@ use std::{ #[derive(Clone, Debug, PartialEq, Eq)] pub struct AllowedConversion { /// The asset type that the note represents - assets: I64Amt, + assets: I64Sum, /// Memorize generator because it's expensive to recompute generator: jubjub::ExtendedPoint, } @@ -71,15 +71,15 @@ impl AllowedConversion { } } -impl From for I64Amt { - fn from(allowed_conversion: AllowedConversion) -> I64Amt { +impl From for I64Sum { + fn from(allowed_conversion: AllowedConversion) -> I64Sum { allowed_conversion.assets } } -impl From for AllowedConversion { +impl From for AllowedConversion { /// Produces an asset generator without cofactor cleared - fn from(assets: I64Amt) -> Self { + fn from(assets: I64Sum) -> Self { let mut asset_generator = jubjub::ExtendedPoint::identity(); for (asset, value) in assets.components() { // Compute the absolute value (failing if -i64::MAX is @@ -123,7 +123,7 @@ impl BorshDeserialize for AllowedConversion { /// computation of checking whether the asset generator corresponds to the /// deserialized amount. fn deserialize(buf: &mut &[u8]) -> borsh::maybestd::io::Result { - let assets = Amount::read(buf)?; + let assets = I64Sum::read(buf)?; let gen_bytes = <::Repr as BorshDeserialize>::deserialize(buf)?; let generator = Option::from(jubjub::ExtendedPoint::from_bytes(&gen_bytes)) @@ -174,7 +174,7 @@ impl SubAssign for AllowedConversion { impl Sum for AllowedConversion { fn sum>(iter: I) -> Self { - iter.fold(AllowedConversion::from(Amount::zero()), Add::add) + iter.fold(AllowedConversion::from(ValueSum::zero()), Add::add) } } @@ -182,7 +182,7 @@ impl Sum for AllowedConversion { mod tests { use crate::asset_type::AssetType; use crate::convert::AllowedConversion; - use crate::transaction::components::amount::Amount; + use crate::transaction::components::amount::ValueSum; /// Generate ZEC asset type fn zec() -> AssetType { @@ -199,11 +199,12 @@ mod tests { #[test] fn test_homomorphism() { // Left operand - let a = Amount::from_pair(zec(), 5i64).unwrap() - + Amount::from_pair(btc(), 6i64).unwrap() - + Amount::from_pair(xan(), 7i64).unwrap(); + let a = ValueSum::from_pair(zec(), 5i64).unwrap() + + ValueSum::from_pair(btc(), 6i64).unwrap() + + ValueSum::from_pair(xan(), 7i64).unwrap(); // Right operand - let b = Amount::from_pair(zec(), 2i64).unwrap() + Amount::from_pair(xan(), 10i64).unwrap(); + let b = + ValueSum::from_pair(zec(), 2i64).unwrap() + ValueSum::from_pair(xan(), 10i64).unwrap(); // Test homomorphism assert_eq!( AllowedConversion::from(a.clone() + b.clone()), @@ -213,9 +214,9 @@ mod tests { #[test] fn test_serialization() { // Make conversion - let a: AllowedConversion = (Amount::from_pair(zec(), 5i64).unwrap() - + Amount::from_pair(btc(), 6i64).unwrap() - + Amount::from_pair(xan(), 7i64).unwrap()) + let a: AllowedConversion = (ValueSum::from_pair(zec(), 5i64).unwrap() + + ValueSum::from_pair(btc(), 6i64).unwrap() + + ValueSum::from_pair(xan(), 7i64).unwrap()) .into(); // Serialize conversion let mut data = Vec::new(); diff --git a/masp_primitives/src/sapling/prover.rs b/masp_primitives/src/sapling/prover.rs index c4faec38..946641d1 100644 --- a/masp_primitives/src/sapling/prover.rs +++ b/masp_primitives/src/sapling/prover.rs @@ -8,7 +8,7 @@ use crate::{ redjubjub::{PublicKey, Signature}, Node, }, - transaction::components::{I64Amt, GROTH_PROOF_SIZE}, + transaction::components::{I128Sum, GROTH_PROOF_SIZE}, }; use super::{Diversifier, PaymentAddress, ProofGenerationKey, Rseed}; @@ -73,7 +73,7 @@ pub trait TxProver { fn binding_sig( &self, ctx: &mut Self::SaplingProvingContext, - amount: &I64Amt, + amount: &I128Sum, sighash: &[u8; 32], ) -> Result; } @@ -92,7 +92,7 @@ pub mod mock { redjubjub::{PublicKey, Signature}, Diversifier, Node, PaymentAddress, ProofGenerationKey, Rseed, }, - transaction::components::{I64Amt, GROTH_PROOF_SIZE}, + transaction::components::{I128Sum, GROTH_PROOF_SIZE}, }; use super::TxProver; @@ -169,7 +169,7 @@ pub mod mock { fn binding_sig( &self, _ctx: &mut Self::SaplingProvingContext, - _value: &I64Amt, + _value: &I128Sum, _sighash: &[u8; 32], ) -> Result { Err(()) diff --git a/masp_primitives/src/transaction.rs b/masp_primitives/src/transaction.rs index 9a6213a9..4525b425 100644 --- a/masp_primitives/src/transaction.rs +++ b/masp_primitives/src/transaction.rs @@ -25,7 +25,7 @@ use crate::{ use self::{ components::{ - amount::{Amount, I64Amt}, + amount::{I128Sum, ValueSum}, sapling::{ self, ConvertDescriptionV5, OutputDescriptionV5, SpendDescription, SpendDescriptionV5, }, @@ -269,10 +269,10 @@ impl TransactionData { } impl TransactionData { - pub fn sapling_value_balance(&self) -> I64Amt { + pub fn sapling_value_balance(&self) -> I128Sum { self.sapling_bundle .as_ref() - .map_or(Amount::zero(), |b| b.value_balance.clone()) + .map_or(ValueSum::zero(), |b| b.value_balance.clone()) } } @@ -355,8 +355,8 @@ impl Transaction { }) } - fn read_amount(mut reader: R) -> io::Result { - Amount::read(&mut reader).map_err(|_| { + fn read_i128_sum(mut reader: R) -> io::Result { + I128Sum::read(&mut reader).map_err(|_| { io::Error::new( io::ErrorKind::InvalidData, "Amount valueBalance out of range", @@ -407,9 +407,9 @@ impl Transaction { let n_converts = cd_v5s.len(); let n_outputs = od_v5s.len(); let value_balance = if n_spends > 0 || n_outputs > 0 { - Self::read_amount(&mut reader)? + Self::read_i128_sum(&mut reader)? } else { - Amount::zero() + ValueSum::zero() }; let spend_anchor = if n_spends > 0 { diff --git a/masp_primitives/src/transaction/builder.rs b/masp_primitives/src/transaction/builder.rs index c6ed82e3..bcbb8fbf 100644 --- a/masp_primitives/src/transaction/builder.rs +++ b/masp_primitives/src/transaction/builder.rs @@ -19,7 +19,7 @@ use crate::{ sapling::{prover::TxProver, Diversifier, Node, Note, PaymentAddress}, transaction::{ components::{ - amount::{Amount, BalanceError, I64Amt, MAX_MONEY}, + amount::{BalanceError, FromNt, I128Sum, I64Sum, ValueSum, MAX_MONEY}, sapling::{ self, builder::{SaplingBuilder, SaplingMetadata}, @@ -43,10 +43,10 @@ const DEFAULT_TX_EXPIRY_DELTA: u32 = 20; pub enum Error { /// Insufficient funds were provided to the transaction builder; the given /// additional amount is required in order to construct the transaction. - InsufficientFunds(I64Amt), + InsufficientFunds(I128Sum), /// The transaction has inputs in excess of outputs and fees; the user must /// add a change output. - ChangeRequired(I64Amt), + ChangeRequired(I64Sum), /// An error occurred in computing the fees for a transaction. Fee(FeeError), /// An overflow or underflow occurred when computing value balances @@ -293,13 +293,13 @@ impl Builder { } /// Returns the sum of the transparent, Sapling, and TZE value balances. - pub fn value_balance(&self) -> Result { + pub fn value_balance(&self) -> Result { let value_balances = [ self.transparent_builder.value_balance()?, self.sapling_builder.value_balance(), ]; - Ok(value_balances.into_iter().sum::()) + Ok(value_balances.into_iter().sum::()) } /// Builds a transaction from the configured spends and outputs. @@ -326,7 +326,7 @@ impl Builder { fn build_internal( self, prover: &impl TxProver, - fee: I64Amt, + fee: I64Sum, ) -> Result<(Transaction, SaplingMetadata), Error> { let consensus_branch_id = BranchId::for_height(&self.params, self.target_height); @@ -338,9 +338,9 @@ impl Builder { // // After fees are accounted for, the value balance of the transaction must be zero. - let balance_after_fees = self.value_balance()? - fee; + let balance_after_fees = self.value_balance()? - I128Sum::from(FromNt(fee)); - if balance_after_fees != Amount::zero() { + if balance_after_fees != ValueSum::zero() { return Err(Error::InsufficientFunds(-balance_after_fees)); }; @@ -480,7 +480,7 @@ mod tests { merkle_tree::{CommitmentTree, IncrementalWitness}, sapling::Rseed, transaction::{ - components::amount::{Amount, DEFAULT_FEE, MAX_MONEY}, + components::amount::{FromNt, I128Sum, ValueSum, DEFAULT_FEE, MAX_MONEY}, sapling::builder::{self as build_s}, transparent::builder::{self as build_t}, TransparentAddress, @@ -597,7 +597,9 @@ mod tests { let builder = Builder::new(TEST_NETWORK, tx_height); assert_eq!( builder.mock_build(), - Err(Error::InsufficientFunds(DEFAULT_FEE.clone())) + Err(Error::InsufficientFunds(I128Sum::from(FromNt( + DEFAULT_FEE.clone() + )))) ); } @@ -615,7 +617,8 @@ mod tests { assert_eq!( builder.mock_build(), Err(Error::InsufficientFunds( - Amount::from_pair(zec(), 50000).unwrap() + &*DEFAULT_FEE + I128Sum::from_pair(zec(), 50000).unwrap() + + &I128Sum::from(FromNt(DEFAULT_FEE.clone())) )) ); } @@ -630,7 +633,8 @@ mod tests { assert_eq!( builder.mock_build(), Err(Error::InsufficientFunds( - Amount::from_pair(zec(), 50000).unwrap() + &*DEFAULT_FEE + I128Sum::from_pair(zec(), 50000).unwrap() + + &I128Sum::from(FromNt(DEFAULT_FEE.clone())) )) ); } @@ -663,7 +667,7 @@ mod tests { assert_eq!( builder.mock_build(), Err(Error::InsufficientFunds( - Amount::from_pair(zec(), 1).unwrap() + ValueSum::from_pair(zec(), 1).unwrap() )) ); } diff --git a/masp_primitives/src/transaction/components.rs b/masp_primitives/src/transaction/components.rs index 36f28a57..2cb438fe 100644 --- a/masp_primitives/src/transaction/components.rs +++ b/masp_primitives/src/transaction/components.rs @@ -4,7 +4,7 @@ pub mod amount; pub mod sapling; pub mod transparent; pub use self::{ - amount::{Amount, I64Amt}, + amount::{I128Sum, I64Sum, ValueSum}, sapling::{ConvertDescription, OutputDescription, SpendDescription}, transparent::{TxIn, TxOut}, }; diff --git a/masp_primitives/src/transaction/components/amount.rs b/masp_primitives/src/transaction/components/amount.rs index 78814d72..4eeef23f 100644 --- a/masp_primitives/src/transaction/components/amount.rs +++ b/masp_primitives/src/transaction/components/amount.rs @@ -14,7 +14,7 @@ use zcash_encoding::Vector; pub const MAX_MONEY: i64 = i64::MAX; pub const MIN_MONEY: i64 = i64::MIN; lazy_static::lazy_static! { -pub static ref DEFAULT_FEE: Amount = Amount::from_pair(zec(), 1000).unwrap(); +pub static ref DEFAULT_FEE: I64Sum = ValueSum::from_pair(zec(), 1000).unwrap(); } /// A type-safe representation of some quantity of Zcash. /// @@ -27,19 +27,33 @@ pub static ref DEFAULT_FEE: Amount = Amount::from_pair(zec(), 10 /// by the network consensus rules. /// -pub type I64Amt = Amount; +pub type I8Sum = ValueSum; -pub type U64Amt = Amount; +pub type U8Sum = ValueSum; -pub type I128Amt = Amount; +pub type I16Sum = ValueSum; + +pub type U16Sum = ValueSum; + +pub type I32Sum = ValueSum; + +pub type U32Sum = ValueSum; + +pub type I64Sum = ValueSum; + +pub type U64Sum = ValueSum; + +pub type I128Sum = ValueSum; + +pub type U128Sum = ValueSum; #[derive(Clone, Default, Debug, PartialEq, Eq, BorshSerialize, BorshDeserialize, Hash)] -pub struct Amount< +pub struct ValueSum< Unit: Hash + Ord + BorshSerialize + BorshDeserialize, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq, >(pub BTreeMap); -impl memuse::DynamicUsage for Amount +impl memuse::DynamicUsage for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, @@ -57,7 +71,7 @@ where } } -impl Amount +impl ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, @@ -71,14 +85,14 @@ where } else if Magnitude::default() <= amount { let mut ret = BTreeMap::new(); ret.insert(atype, amount); - Ok(Amount(ret)) + Ok(ValueSum(ret)) } else { Err(()) } } } -impl Amount +impl ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, @@ -92,7 +106,7 @@ where } else { let mut ret = BTreeMap::new(); ret.insert(atype, amount); - Ok(Amount(ret)) + Ok(ValueSum(ret)) } } @@ -108,14 +122,14 @@ where } } -impl Amount +impl ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, { /// Returns a zero-valued Amount. pub fn zero() -> Self { - Amount(BTreeMap::new()) + ValueSum(BTreeMap::new()) } /// Returns an iterator over the amount's non-zero asset-types @@ -141,7 +155,7 @@ where } } -impl Amount { +impl ValueSum { /// Deserialize an Amount object from a list of amounts denominated by /// different assets pub fn read(reader: &mut R) -> std::io::Result { @@ -176,7 +190,42 @@ impl Amount { } } -impl From for Amount +impl ValueSum { + /// Deserialize an Amount object from a list of amounts denominated by + /// different assets + pub fn read(reader: &mut R) -> std::io::Result { + let vec = Vector::read(reader, |reader| { + let mut atype = [0; 32]; + let mut value = [0; 16]; + reader.read_exact(&mut atype)?; + reader.read_exact(&mut value)?; + let atype = AssetType::from_identifier(&atype).ok_or_else(|| { + std::io::Error::new(std::io::ErrorKind::InvalidData, "invalid asset type") + })?; + Ok((atype, i128::from_le_bytes(value))) + })?; + 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") + })?; + } + Ok(ret) + } + + /// Serialize an Amount object into a list of amounts denominated by + /// distinct asset types + pub fn write(&self, writer: &mut W) -> std::io::Result<()> { + let vec: Vec<_> = self.components().collect(); + Vector::write(writer, vec.as_ref(), |writer, elt| { + writer.write_all(elt.0.get_identifier())?; + writer.write_all(elt.1.to_le_bytes().as_ref())?; + Ok(()) + }) + } +} + +impl From for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + From, @@ -184,11 +233,11 @@ where fn from(atype: Unit) -> Self { let mut ret = BTreeMap::new(); ret.insert(atype, true.into()); - Amount(ret) + ValueSum(ret) } } -impl PartialOrd for Amount +impl PartialOrd for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, @@ -212,7 +261,7 @@ where macro_rules! impl_index { ($struct_type:ty) => { - impl Index<&Unit> for Amount + impl Index<&Unit> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize, { @@ -225,13 +274,27 @@ macro_rules! impl_index { }; } +impl_index!(i8); + +impl_index!(u8); + +impl_index!(i16); + +impl_index!(u16); + +impl_index!(i32); + +impl_index!(u32); + impl_index!(i64); impl_index!(u64); impl_index!(i128); -impl MulAssign for Amount +impl_index!(u128); + +impl MulAssign for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize @@ -248,7 +311,7 @@ where } } -impl Mul for Amount +impl Mul for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize @@ -260,7 +323,7 @@ where + PartialOrd + CheckedMul, { - type Output = Amount; + type Output = ValueSum; fn mul(self, rhs: Magnitude) -> Self::Output { let mut comps = BTreeMap::new(); @@ -271,11 +334,11 @@ where ); } comps.retain(|_, v| *v != Magnitude::default()); - Amount(comps) + ValueSum(comps) } } -impl AddAssign<&Amount> for Amount +impl AddAssign<&ValueSum> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize @@ -287,12 +350,12 @@ where + PartialOrd + CheckedAdd, { - fn add_assign(&mut self, rhs: &Amount) { + fn add_assign(&mut self, rhs: &ValueSum) { *self = self.clone() + rhs; } } -impl AddAssign> for Amount +impl AddAssign> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize @@ -304,12 +367,12 @@ where + PartialOrd + CheckedAdd, { - fn add_assign(&mut self, rhs: Amount) { + fn add_assign(&mut self, rhs: ValueSum) { *self += &rhs } } -impl Add<&Amount> for Amount +impl Add<&ValueSum> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize @@ -321,9 +384,9 @@ where + PartialOrd + CheckedAdd, { - type Output = Amount; + type Output = ValueSum; - fn add(self, rhs: &Amount) -> Self::Output { + fn add(self, rhs: &ValueSum) -> Self::Output { let mut comps = self.0.clone(); for (atype, amount) in rhs.components() { comps.insert( @@ -334,11 +397,11 @@ where ); } comps.retain(|_, v| *v != Magnitude::default()); - Amount(comps) + ValueSum(comps) } } -impl Add> for Amount +impl Add> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize @@ -350,14 +413,14 @@ where + PartialOrd + CheckedAdd, { - type Output = Amount; + type Output = ValueSum; - fn add(self, rhs: Amount) -> Self::Output { + fn add(self, rhs: ValueSum) -> Self::Output { self + &rhs } } -impl SubAssign<&Amount> for Amount +impl SubAssign<&ValueSum> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize @@ -369,12 +432,12 @@ where + PartialOrd + CheckedSub, { - fn sub_assign(&mut self, rhs: &Amount) { + fn sub_assign(&mut self, rhs: &ValueSum) { *self = self.clone() - rhs } } -impl SubAssign> for Amount +impl SubAssign> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize @@ -386,12 +449,12 @@ where + PartialOrd + CheckedSub, { - fn sub_assign(&mut self, rhs: Amount) { + fn sub_assign(&mut self, rhs: ValueSum) { *self -= &rhs } } -impl Neg for Amount +impl Neg for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize @@ -403,7 +466,7 @@ where + PartialOrd + CheckedNeg, { - type Output = Amount; + type Output = ValueSum; fn neg(mut self) -> Self::Output { let mut comps = BTreeMap::new(); @@ -414,18 +477,18 @@ where ); } comps.retain(|_, v| *v != Magnitude::default()); - Amount(comps) + ValueSum(comps) } } -impl Sub<&Amount> for Amount +impl Sub<&ValueSum> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, { - type Output = Amount; + type Output = ValueSum; - fn sub(self, rhs: &Amount) -> Self::Output { + fn sub(self, rhs: &ValueSum) -> Self::Output { let mut comps = self.0.clone(); for (atype, amount) in rhs.components() { comps.insert( @@ -436,23 +499,23 @@ where ); } comps.retain(|_, v| *v != Magnitude::default()); - Amount(comps) + ValueSum(comps) } } -impl Sub> for Amount +impl Sub> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, { - type Output = Amount; + type Output = ValueSum; - fn sub(self, rhs: Amount) -> Self::Output { + fn sub(self, rhs: ValueSum) -> Self::Output { self - &rhs } } -impl Sum for Amount +impl Sum for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, @@ -466,7 +529,8 @@ where /// Workaround for the blanket implementation of TryFrom pub struct TryFromNt(pub X); -impl TryFrom>> for Amount +impl TryFrom>> + for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, @@ -474,7 +538,7 @@ where { type Error = >::Error; - fn try_from(x: TryFromNt>) -> Result { + fn try_from(x: TryFromNt>) -> Result { let mut comps = BTreeMap::new(); for (atype, amount) in x.0 .0 { comps.insert(atype, amount.try_into()?); @@ -486,13 +550,13 @@ where /// Workaround for the blanket implementation of TryFrom pub struct FromNt(pub X); -impl From>> for Amount +impl From>> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, Output: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + From, { - fn from(x: FromNt>) -> Self { + fn from(x: FromNt>) -> Self { let mut comps = BTreeMap::new(); for (atype, amount) in x.0 .0 { comps.insert(atype, amount.into()); @@ -530,56 +594,62 @@ pub fn zec() -> AssetType { AssetType::new(b"ZEC").unwrap() } -pub fn default_fee() -> Amount { - Amount::from_pair(zec(), 10000).unwrap() +pub fn default_fee() -> ValueSum { + ValueSum::from_pair(zec(), 10000).unwrap() } #[cfg(any(test, feature = "test-dependencies"))] pub mod testing { use proptest::prelude::prop_compose; - use super::{Amount, I64Amt, MAX_MONEY}; + use super::{I128Sum, I64Sum, ValueSum, MAX_MONEY}; use crate::asset_type::testing::arb_asset_type; prop_compose! { - pub fn arb_amount()(asset_type in arb_asset_type(), amt in -MAX_MONEY..MAX_MONEY) -> I64Amt { - Amount::from_pair(asset_type, amt).unwrap() + pub fn arb_i64_sum()(asset_type in arb_asset_type(), amt in -MAX_MONEY..MAX_MONEY) -> I64Sum { + ValueSum::from_pair(asset_type, amt).unwrap() + } + } + + prop_compose! { + pub fn arb_i128_sum()(asset_type in arb_asset_type(), amt in -MAX_MONEY..MAX_MONEY) -> I128Sum { + ValueSum::from_pair(asset_type, amt as i128).unwrap() } } prop_compose! { - pub fn arb_nonnegative_amount()(asset_type in arb_asset_type(), amt in 0i64..MAX_MONEY) -> I64Amt { - Amount::from_pair(asset_type, amt).unwrap() + pub fn arb_nonnegative_amount()(asset_type in arb_asset_type(), amt in 0i64..MAX_MONEY) -> I64Sum { + ValueSum::from_pair(asset_type, amt).unwrap() } } prop_compose! { - pub fn arb_positive_amount()(asset_type in arb_asset_type(), amt in 1i64..MAX_MONEY) -> I64Amt { - Amount::from_pair(asset_type, amt).unwrap() + pub fn arb_positive_amount()(asset_type in arb_asset_type(), amt in 1i64..MAX_MONEY) -> I64Sum { + ValueSum::from_pair(asset_type, amt).unwrap() } } } #[cfg(test)] mod tests { - use super::{zec, Amount, MAX_MONEY, MIN_MONEY}; + use super::{zec, I64Sum, ValueSum, MAX_MONEY, MIN_MONEY}; #[test] fn amount_in_range() { let zero = b"\x01\x94\xf3O\xfdd\xef\n\xc3i\x08\xfd\xdf\xec\x05hX\x06)\xc4Vq\x0f\xa1\x86\x83\x12\xa8\x7f\xbf\n\xa5\t\x00\x00\x00\x00\x00\x00\x00\x00"; - assert_eq!(Amount::read(&mut zero.as_ref()).unwrap(), Amount::zero()); + assert_eq!(I64Sum::read(&mut zero.as_ref()).unwrap(), ValueSum::zero()); let neg_one = b"\x01\x94\xf3O\xfdd\xef\n\xc3i\x08\xfd\xdf\xec\x05hX\x06)\xc4Vq\x0f\xa1\x86\x83\x12\xa8\x7f\xbf\n\xa5\t\xff\xff\xff\xff\xff\xff\xff\xff"; assert_eq!( - Amount::read(&mut neg_one.as_ref()).unwrap(), - Amount::from_pair(zec(), -1).unwrap() + I64Sum::read(&mut neg_one.as_ref()).unwrap(), + I64Sum::from_pair(zec(), -1).unwrap() ); let max_money = b"\x01\x94\xf3O\xfdd\xef\n\xc3i\x08\xfd\xdf\xec\x05hX\x06)\xc4Vq\x0f\xa1\x86\x83\x12\xa8\x7f\xbf\n\xa5\t\xff\xff\xff\xff\xff\xff\xff\x7f"; assert_eq!( - Amount::read(&mut max_money.as_ref()).unwrap(), - Amount::from_pair(zec(), MAX_MONEY).unwrap() + I64Sum::read(&mut max_money.as_ref()).unwrap(), + I64Sum::from_pair(zec(), MAX_MONEY).unwrap() ); //let max_money_p1 = b"\x01\x94\xf3O\xfdd\xef\n\xc3i\x08\xfd\xdf\xec\x05hX\x06)\xc4Vq\x0f\xa1\x86\x83\x12\xa8\x7f\xbf\n\xa5\t\x01\x40\x07\x5a\xf0\x75\x07\x00"; @@ -593,36 +663,36 @@ mod tests { let neg_max_money = b"\x01\x94\xf3O\xfdd\xef\n\xc3i\x08\xfd\xdf\xec\x05hX\x06)\xc4Vq\x0f\xa1\x86\x83\x12\xa8\x7f\xbf\n\xa5\t\x01\x00\x00\x00\x00\x00\x00\x80"; assert_eq!( - Amount::read(&mut neg_max_money.as_ref()).unwrap(), - Amount::from_pair(zec(), -MAX_MONEY).unwrap() + I64Sum::read(&mut neg_max_money.as_ref()).unwrap(), + I64Sum::from_pair(zec(), -MAX_MONEY).unwrap() ); } #[test] #[should_panic] fn add_panics_on_overflow() { - let v = Amount::from_pair(zec(), MAX_MONEY).unwrap(); - let _sum = v + Amount::from_pair(zec(), 1).unwrap(); + let v = ValueSum::from_pair(zec(), MAX_MONEY).unwrap(); + let _sum = v + ValueSum::from_pair(zec(), 1).unwrap(); } #[test] #[should_panic] fn add_assign_panics_on_overflow() { - let mut a = Amount::from_pair(zec(), MAX_MONEY).unwrap(); - a += Amount::from_pair(zec(), 1).unwrap(); + let mut a = ValueSum::from_pair(zec(), MAX_MONEY).unwrap(); + a += ValueSum::from_pair(zec(), 1).unwrap(); } #[test] #[should_panic] fn sub_panics_on_underflow() { - let v = Amount::from_pair(zec(), MIN_MONEY).unwrap(); - let _diff = v - Amount::from_pair(zec(), 1).unwrap(); + let v = ValueSum::from_pair(zec(), MIN_MONEY).unwrap(); + let _diff = v - ValueSum::from_pair(zec(), 1).unwrap(); } #[test] #[should_panic] fn sub_assign_panics_on_underflow() { - let mut a = Amount::from_pair(zec(), MIN_MONEY).unwrap(); - a -= Amount::from_pair(zec(), 1).unwrap(); + let mut a = ValueSum::from_pair(zec(), MIN_MONEY).unwrap(); + a -= ValueSum::from_pair(zec(), 1).unwrap(); } } diff --git a/masp_primitives/src/transaction/components/sapling.rs b/masp_primitives/src/transaction/components/sapling.rs index 282de9f4..7ce04153 100644 --- a/masp_primitives/src/transaction/components/sapling.rs +++ b/masp_primitives/src/transaction/components/sapling.rs @@ -23,7 +23,7 @@ use crate::{ }, }; -use super::{amount::I64Amt, GROTH_PROOF_SIZE}; +use super::{amount::I128Sum, GROTH_PROOF_SIZE}; pub type GrothProofBytes = [u8; GROTH_PROOF_SIZE]; @@ -90,7 +90,7 @@ pub struct Bundle>, pub shielded_converts: Vec>, pub shielded_outputs: Vec>, - pub value_balance: I64Amt, + pub value_balance: I128Sum, pub authorization: A, } @@ -535,7 +535,7 @@ pub mod testing { Nullifier, }, transaction::{ - components::{amount::testing::arb_amount, GROTH_PROOF_SIZE}, + components::{amount::testing::arb_i128_sum, GROTH_PROOF_SIZE}, TxVersion, }, }; @@ -614,7 +614,7 @@ pub mod testing { shielded_spends in vec(arb_spend_description(), 0..30), shielded_converts in vec(arb_convert_description(), 0..30), shielded_outputs in vec(arb_output_description(), 0..30), - value_balance in arb_amount(), + value_balance in arb_i128_sum(), rng_seed in prop::array::uniform32(prop::num::u8::ANY), fake_bvk_bytes in prop::array::uniform32(prop::num::u8::ANY), ) -> Option> { diff --git a/masp_primitives/src/transaction/components/sapling/builder.rs b/masp_primitives/src/transaction/components/sapling/builder.rs index 40c306df..b0403066 100644 --- a/masp_primitives/src/transaction/components/sapling/builder.rs +++ b/masp_primitives/src/transaction/components/sapling/builder.rs @@ -25,7 +25,7 @@ use crate::{ transaction::{ builder::Progress, components::{ - amount::{Amount, FromNt, I128Amt, I64Amt, TryFromNt, MAX_MONEY}, + amount::{FromNt, I128Sum, I64Sum, ValueSum, MAX_MONEY}, sapling::{ fees, Authorization, Authorized, Bundle, ConvertDescription, GrothProofBytes, OutputDescription, SpendDescription, @@ -272,7 +272,7 @@ pub struct SaplingBuilder { params: P, spend_anchor: Option, target_height: BlockHeight, - value_balance: Amount, + value_balance: I128Sum, convert_anchor: Option, spends: Vec>, converts: Vec, @@ -303,7 +303,7 @@ impl BorshDeserialize for SaplingBui .map(|x| x.ok_or_else(|| std::io::Error::from(std::io::ErrorKind::InvalidData))) .transpose()?; let target_height = BlockHeight::deserialize(buf)?; - let value_balance = Amount::::deserialize(buf)?; + let value_balance = I128Sum::deserialize(buf)?; let convert_anchor: Option> = Option::<[u8; 32]>::deserialize(buf)?.map(|x| bls12_381::Scalar::from_bytes(&x).into()); let convert_anchor = convert_anchor @@ -347,7 +347,7 @@ impl SaplingBuilder { params, spend_anchor: None, target_height, - value_balance: Amount::zero(), + value_balance: ValueSum::zero(), convert_anchor: None, spends: vec![], converts: vec![], @@ -369,19 +369,9 @@ impl SaplingBuilder { &self.outputs } - /// Returns the net value represented by the spends and outputs added to this builder, - /// or an error if the values added to this builder overflow the range of a Zcash - /// monetary amount. - fn try_value_balance(&self) -> Result { - TryFromNt(self.value_balance.clone()) - .try_into() - .map_err(|_| Error::InvalidAmount) - } - /// Returns the net value represented by the spends and outputs added to this builder. - pub fn value_balance(&self) -> I64Amt { - self.try_value_balance() - .expect("we check this when mutating self.value_balance") + pub fn value_balance(&self) -> I128Sum { + self.value_balance.clone() } } @@ -411,8 +401,8 @@ impl SaplingBuilder

{ let alpha = jubjub::Fr::random(&mut rng); - self.value_balance += I128Amt::from(FromNt( - Amount::from_pair(note.asset_type, note.value).map_err(|_| Error::InvalidAmount)?, + self.value_balance += I128Sum::from(FromNt( + ValueSum::from_pair(note.asset_type, note.value).map_err(|_| Error::InvalidAmount)?, )); self.spends.push(SpendDescriptionInfo { @@ -448,8 +438,8 @@ impl SaplingBuilder

{ self.convert_anchor = Some(merkle_path.root(node).into()) } - let allowed_amt: I64Amt = allowed.clone().into(); - self.value_balance += I128Amt::from(FromNt(allowed_amt * (value as i64))); + let allowed_amt: I64Sum = allowed.clone().into(); + self.value_balance += I128Sum::from(FromNt(allowed_amt * (value as i64))); self.converts.push(ConvertDescriptionInfo { allowed, @@ -482,8 +472,8 @@ impl SaplingBuilder

{ memo, )?; - self.value_balance -= I128Amt::from(FromNt( - Amount::from_pair(asset_type, value).map_err(|_| Error::InvalidAmount)?, + self.value_balance -= I128Sum::from(FromNt( + ValueSum::from_pair(asset_type, value).map_err(|_| Error::InvalidAmount)?, )); self.outputs.push(output); @@ -500,7 +490,7 @@ impl SaplingBuilder

{ progress_notifier: Option<&Sender>, ) -> Result>, Error> { // Record initial positions of spends and outputs - let value_balance = self.try_value_balance()?; + let value_balance = self.value_balance(); let params = self.params; let mut indexed_spends: Vec<_> = self.spends.into_iter().enumerate().collect(); let mut indexed_converts: Vec<_> = self.converts.into_iter().enumerate().collect(); diff --git a/masp_primitives/src/transaction/components/transparent.rs b/masp_primitives/src/transaction/components/transparent.rs index a34ce73f..d2553fc5 100644 --- a/masp_primitives/src/transaction/components/transparent.rs +++ b/masp_primitives/src/transaction/components/transparent.rs @@ -7,7 +7,7 @@ use std::io::{self, Read, Write}; use crate::asset_type::AssetType; use crate::transaction::TransparentAddress; -use super::amount::{Amount, BalanceError, I64Amt, MAX_MONEY}; +use super::amount::{BalanceError, I128Sum, ValueSum, MAX_MONEY}; pub mod builder; pub mod fees; @@ -58,7 +58,7 @@ impl Bundle { /// 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(&self) -> Result + pub fn value_balance(&self) -> Result where E: From, { @@ -67,12 +67,12 @@ impl Bundle { .iter() .map(|p| { if p.value >= 0 { - Amount::from_pair(p.asset_type, p.value) + ValueSum::from_pair(p.asset_type, p.value as i128) } else { Err(()) } }) - .sum::>() + .sum::>() .map_err(|_| BalanceError::Overflow)?; let output_sum = self @@ -80,12 +80,12 @@ impl Bundle { .iter() .map(|p| { if p.value >= 0 { - Amount::from_pair(p.asset_type, p.value) + ValueSum::from_pair(p.asset_type, p.value as i128) } else { Err(()) } }) - .sum::>() + .sum::>() .map_err(|_| BalanceError::Overflow)?; // Cannot panic when subtracting two positive i64 diff --git a/masp_primitives/src/transaction/components/transparent/builder.rs b/masp_primitives/src/transaction/components/transparent/builder.rs index dfd1c3d6..b5aa9fe9 100644 --- a/masp_primitives/src/transaction/components/transparent/builder.rs +++ b/masp_primitives/src/transaction/components/transparent/builder.rs @@ -6,7 +6,7 @@ use crate::{ asset_type::AssetType, transaction::{ components::{ - amount::{Amount, BalanceError, I64Amt, MAX_MONEY}, + amount::{BalanceError, I128Sum, ValueSum, MAX_MONEY}, transparent::{self, fees, Authorization, Authorized, Bundle, TxIn, TxOut}, }, sighash::TransparentAuthorizingContext, @@ -133,35 +133,35 @@ impl TransparentBuilder { Ok(()) } - pub fn value_balance(&self) -> Result { + pub fn value_balance(&self) -> Result { #[cfg(feature = "transparent-inputs")] let input_sum = self .inputs .iter() .map(|input| { if input.coin.value >= 0 { - Amount::from_pair(input.coin.asset_type, input.coin.value) + ValueSum::from_pair(input.coin.asset_type, input.coin.value as i128) } else { Err(()) } }) - .sum::>() + .sum::>() .map_err(|_| BalanceError::Overflow)?; #[cfg(not(feature = "transparent-inputs"))] - let input_sum = Amount::zero(); + let input_sum = ValueSum::zero(); let output_sum = self .vout .iter() .map(|vo| { if vo.value >= 0 { - Amount::from_pair(vo.asset_type, vo.value) + ValueSum::from_pair(vo.asset_type, vo.value as i128) } else { Err(()) } }) - .sum::>() + .sum::>() .map_err(|_| BalanceError::Overflow)?; // Cannot panic when subtracting two positive i64 diff --git a/masp_primitives/src/transaction/fees.rs b/masp_primitives/src/transaction/fees.rs index b4af61aa..eed0f838 100644 --- a/masp_primitives/src/transaction/fees.rs +++ b/masp_primitives/src/transaction/fees.rs @@ -2,7 +2,7 @@ use crate::{ consensus::{self, BlockHeight}, - transaction::components::{amount::I64Amt, transparent::fees as transparent}, + transaction::components::{amount::I64Sum, transparent::fees as transparent}, }; pub mod fixed; @@ -24,5 +24,5 @@ pub trait FeeRule { transparent_outputs: &[impl transparent::OutputView], sapling_input_count: usize, sapling_output_count: usize, - ) -> Result; + ) -> Result; } diff --git a/masp_primitives/src/transaction/fees/fixed.rs b/masp_primitives/src/transaction/fees/fixed.rs index 6dbea37d..92d434da 100644 --- a/masp_primitives/src/transaction/fees/fixed.rs +++ b/masp_primitives/src/transaction/fees/fixed.rs @@ -1,7 +1,7 @@ use crate::{ consensus::{self, BlockHeight}, transaction::components::{ - amount::{I64Amt, DEFAULT_FEE}, + amount::{I64Sum, DEFAULT_FEE}, transparent::fees as transparent, }, }; @@ -10,12 +10,12 @@ use crate::{ /// the transaction being constructed. #[derive(Clone, Debug)] pub struct FeeRule { - fixed_fee: I64Amt, + fixed_fee: I64Sum, } impl FeeRule { /// Creates a new nonstandard fixed fee rule with the specified fixed fee. - pub fn non_standard(fixed_fee: I64Amt) -> Self { + pub fn non_standard(fixed_fee: I64Sum) -> Self { Self { fixed_fee } } @@ -27,7 +27,7 @@ impl FeeRule { } /// Returns the fixed fee amount which which this rule was configured. - pub fn fixed_fee(&self) -> I64Amt { + pub fn fixed_fee(&self) -> I64Sum { self.fixed_fee.clone() } } @@ -42,7 +42,7 @@ impl super::FeeRule for FeeRule { _transparent_outputs: &[impl transparent::OutputView], _sapling_input_count: usize, _sapling_output_count: usize, - ) -> Result { + ) -> Result { Ok(self.fixed_fee.clone()) } } diff --git a/masp_proofs/benches/convert.rs b/masp_proofs/benches/convert.rs index da496ad1..a295cf00 100644 --- a/masp_proofs/benches/convert.rs +++ b/masp_proofs/benches/convert.rs @@ -6,7 +6,7 @@ use bls12_381::Bls12; use criterion::Criterion; use group::ff::Field; use masp_primitives::{ - asset_type::AssetType, convert::AllowedConversion, transaction::components::Amount, + asset_type::AssetType, convert::AllowedConversion, transaction::components::ValueSum, }; use masp_proofs::circuit::convert::{Convert, TREE_DEPTH}; use rand_core::{RngCore, SeedableRng}; @@ -38,10 +38,10 @@ fn criterion_benchmark(c: &mut Criterion) { let output_value = i as i64 + 1; let mint_value = i as i64 + 1; - let allowed_conversion: AllowedConversion = (Amount::from_pair(spend_asset, spend_value) + let allowed_conversion: AllowedConversion = (ValueSum::from_pair(spend_asset, spend_value) .unwrap() - + Amount::from_pair(output_asset, output_value).unwrap() - + Amount::from_pair(mint_asset, mint_value).unwrap()) + + ValueSum::from_pair(output_asset, output_value).unwrap() + + ValueSum::from_pair(mint_asset, mint_value).unwrap()) .into(); let value = rng.next_u64(); diff --git a/masp_proofs/src/circuit/convert.rs b/masp_proofs/src/circuit/convert.rs index d555ed67..d0c08be1 100644 --- a/masp_proofs/src/circuit/convert.rs +++ b/masp_proofs/src/circuit/convert.rs @@ -132,7 +132,7 @@ fn test_convert_circuit_with_bls12_381() { use group::{ff::Field, ff::PrimeField, ff::PrimeFieldBits, Curve}; use masp_primitives::{ asset_type::AssetType, convert::AllowedConversion, sapling::pedersen_hash, - transaction::components::Amount, + transaction::components::ValueSum, }; use rand_core::{RngCore, SeedableRng}; use rand_xorshift::XorShiftRng; @@ -154,10 +154,10 @@ fn test_convert_circuit_with_bls12_381() { let output_value = i as i64 + 1; let mint_value = i as i64 + 1; - let allowed_conversion: AllowedConversion = (Amount::from_pair(spend_asset, spend_value) + let allowed_conversion: AllowedConversion = (ValueSum::from_pair(spend_asset, spend_value) .unwrap() - + Amount::from_pair(output_asset, output_value).unwrap() - + Amount::from_pair(mint_asset, mint_value).unwrap()) + + ValueSum::from_pair(output_asset, output_value).unwrap() + + ValueSum::from_pair(mint_asset, mint_value).unwrap()) .into(); let value = rng.next_u64(); diff --git a/masp_proofs/src/prover.rs b/masp_proofs/src/prover.rs index 77fdf187..f9621f6e 100644 --- a/masp_proofs/src/prover.rs +++ b/masp_proofs/src/prover.rs @@ -11,7 +11,7 @@ use masp_primitives::{ redjubjub::{PublicKey, Signature}, Diversifier, Node, PaymentAddress, ProofGenerationKey, Rseed, }, - transaction::components::{I64Amt, GROTH_PROOF_SIZE}, + transaction::components::{I128Sum, GROTH_PROOF_SIZE}, }; use std::path::Path; @@ -247,7 +247,7 @@ impl TxProver for LocalTxProver { fn binding_sig( &self, ctx: &mut Self::SaplingProvingContext, - assets_and_values: &I64Amt, //&[(AssetType, i64)], + assets_and_values: &I128Sum, //&[(AssetType, i64)], sighash: &[u8; 32], ) -> Result { ctx.binding_sig(assets_and_values, sighash) diff --git a/masp_proofs/src/sapling/mod.rs b/masp_proofs/src/sapling/mod.rs index eb14fc60..16d90e6e 100644 --- a/masp_proofs/src/sapling/mod.rs +++ b/masp_proofs/src/sapling/mod.rs @@ -9,11 +9,11 @@ pub use self::prover::SaplingProvingContext; pub use self::verifier::{BatchValidator, SaplingVerificationContext}; // This function computes `value` in the exponent of the value commitment base -fn masp_compute_value_balance(asset_type: AssetType, value: i64) -> Option { - // Compute the absolute value (failing if -i64::MAX is +fn masp_compute_value_balance(asset_type: AssetType, value: i128) -> Option { + // Compute the absolute value (failing if -i128::MAX is // the value) let abs = match value.checked_abs() { - Some(a) => a as u64, + Some(a) => a as u128, None => return None, }; @@ -21,7 +21,10 @@ fn masp_compute_value_balance(asset_type: AssetType, value: i64) -> Option Result { // Initialize secure RNG @@ -304,7 +304,7 @@ impl SaplingProvingContext { .components() .map(|(asset_type, value_balance)| { // Compute value balance for each asset - // Error for bad value balances (-INT64_MAX value) + // Error for bad value balances (-INT128_MAX value) masp_compute_value_balance(*asset_type, *value_balance) }) .try_fold(self.cv_sum, |tmp, value_balance| { diff --git a/masp_proofs/src/sapling/verifier.rs b/masp_proofs/src/sapling/verifier.rs index d31e4259..c9c297dd 100644 --- a/masp_proofs/src/sapling/verifier.rs +++ b/masp_proofs/src/sapling/verifier.rs @@ -5,7 +5,7 @@ use bls12_381::Bls12; use group::{Curve, GroupEncoding}; use masp_primitives::{ sapling::redjubjub::{PublicKey, Signature}, - transaction::components::I64Amt, + transaction::components::I128Sum, }; use super::masp_compute_value_balance; @@ -172,7 +172,7 @@ impl SaplingVerificationContextInner { /// have been checked before calling this function. fn final_check( &self, - value_balance: I64Amt, + value_balance: I128Sum, sighash_value: &[u8; 32], binding_sig: Signature, binding_sig_verifier: impl FnOnce(PublicKey, [u8; 64], Signature) -> bool, diff --git a/masp_proofs/src/sapling/verifier/single.rs b/masp_proofs/src/sapling/verifier/single.rs index 9cd59071..8abedb48 100644 --- a/masp_proofs/src/sapling/verifier/single.rs +++ b/masp_proofs/src/sapling/verifier/single.rs @@ -3,7 +3,7 @@ use bls12_381::Bls12; use masp_primitives::{ constants::{SPENDING_KEY_GENERATOR, VALUE_COMMITMENT_RANDOMNESS_GENERATOR}, sapling::redjubjub::{PublicKey, Signature}, - transaction::components::I64Amt, + transaction::components::I128Sum, }; use super::SaplingVerificationContextInner; @@ -98,7 +98,7 @@ impl SaplingVerificationContext { /// have been checked before calling this function. pub fn final_check( &self, - value_balance: I64Amt, + value_balance: I128Sum, sighash_value: &[u8; 32], binding_sig: Signature, ) -> bool { From 27749604ba011ff9286b8b5006f4a3146868c15f Mon Sep 17 00:00:00 2001 From: Murisi Tarusenga Date: Thu, 27 Jul 2023 09:45:58 +0200 Subject: [PATCH 4/8] ValueSum comparisons no longer depend upon subtraction being possible. Removed ValueSum from bool implementation. --- .../src/transaction/components/amount.rs | 46 +++++++++++++------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/masp_primitives/src/transaction/components/amount.rs b/masp_primitives/src/transaction/components/amount.rs index 4eeef23f..d1a59924 100644 --- a/masp_primitives/src/transaction/components/amount.rs +++ b/masp_primitives/src/transaction/components/amount.rs @@ -1,6 +1,6 @@ use crate::asset_type::AssetType; use borsh::{BorshDeserialize, BorshSerialize}; -use num_traits::{CheckedAdd, CheckedMul, CheckedNeg, CheckedSub}; +use num_traits::{CheckedAdd, CheckedMul, CheckedNeg, CheckedSub, One}; use std::cmp::Ordering; use std::collections::btree_map::Keys; use std::collections::btree_map::{IntoIter, Iter}; @@ -228,11 +228,11 @@ impl ValueSum { impl From for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + From, + Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + One, { fn from(atype: Unit) -> Self { let mut ret = BTreeMap::new(); - ret.insert(atype, true.into()); + ret.insert(atype, Magnitude::one()); ValueSum(ret) } } @@ -241,21 +241,37 @@ impl PartialOrd for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, - Self: Sub, { - /// One Amount is more than or equal to another if each corresponding - /// coordinate is more than the other's. + /// One ValueSum is more than or equal to another if each corresponding + /// coordinate is more than or equal to the other's. fn partial_cmp(&self, other: &Self) -> Option { - let diff = other.clone() - self.clone(); - if diff.0.values().all(|x| *x == Default::default()) { - Some(Ordering::Equal) - } else if diff.0.values().all(|x| *x >= Default::default()) { - Some(Ordering::Less) - } else if diff.0.values().all(|x| *x <= Default::default()) { - Some(Ordering::Greater) - } else { - None + let zero = Magnitude::default(); + let mut ordering = Some(Ordering::Equal); + for k in self.0.keys().chain(other.0.keys()) { + let v1 = self.0.get(k).unwrap_or(&zero); + let v2 = other.0.get(k).unwrap_or(&zero); + match (v1.partial_cmp(v2), ordering) { + // Sums cannot be compared if even a single coordinate cannot be + // compared + (None, _) => ordering = None, + // If sums are uncomparable, less, greater, or equal, another + // equal coordinate will not change that + (Some(Ordering::Equal), _) => {} + // A lesser coordinate is inconsistent with the sum being + // greater, and vice-versa + (Some(Ordering::Less), Some(Ordering::Greater) | None) => ordering = None, + (Some(Ordering::Greater), Some(Ordering::Less) | None) => ordering = None, + // It only takes one lesser coordinate, to make a sum that + // otherwise would have been equal, to be lesser + (Some(Ordering::Less), Some(Ordering::Less | Ordering::Equal)) => { + ordering = Some(Ordering::Less) + } + (Some(Ordering::Greater), Some(Ordering::Greater | Ordering::Equal)) => { + ordering = Some(Ordering::Greater) + } + } } + ordering } } From 55e24dd0331e0b7fb5842cb22cae836f9545333c Mon Sep 17 00:00:00 2001 From: Murisi Tarusenga Date: Thu, 27 Jul 2023 12:07:53 +0200 Subject: [PATCH 5/8] Reduced usage of ValueSum conversions. --- masp_primitives/src/transaction/builder.rs | 4 +--- .../src/transaction/components/sapling/builder.rs | 10 ++++------ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/masp_primitives/src/transaction/builder.rs b/masp_primitives/src/transaction/builder.rs index bcbb8fbf..c8353730 100644 --- a/masp_primitives/src/transaction/builder.rs +++ b/masp_primitives/src/transaction/builder.rs @@ -597,9 +597,7 @@ mod tests { let builder = Builder::new(TEST_NETWORK, tx_height); assert_eq!( builder.mock_build(), - Err(Error::InsufficientFunds(I128Sum::from(FromNt( - DEFAULT_FEE.clone() - )))) + Err(Error::InsufficientFunds(FromNt(DEFAULT_FEE.clone()).into())) ); } diff --git a/masp_primitives/src/transaction/components/sapling/builder.rs b/masp_primitives/src/transaction/components/sapling/builder.rs index b0403066..a8187f63 100644 --- a/masp_primitives/src/transaction/components/sapling/builder.rs +++ b/masp_primitives/src/transaction/components/sapling/builder.rs @@ -401,9 +401,8 @@ impl SaplingBuilder

{ let alpha = jubjub::Fr::random(&mut rng); - self.value_balance += I128Sum::from(FromNt( - ValueSum::from_pair(note.asset_type, note.value).map_err(|_| Error::InvalidAmount)?, - )); + self.value_balance += + ValueSum::from_pair(note.asset_type, note.value.into()).map_err(|_| Error::InvalidAmount)?; self.spends.push(SpendDescriptionInfo { extsk, @@ -472,9 +471,8 @@ impl SaplingBuilder

{ memo, )?; - self.value_balance -= I128Sum::from(FromNt( - ValueSum::from_pair(asset_type, value).map_err(|_| Error::InvalidAmount)?, - )); + self.value_balance -= + ValueSum::from_pair(asset_type, value.into()).map_err(|_| Error::InvalidAmount)?; self.outputs.push(output); From 438369a2a762e9ac53a05b9ff8d4b099cd81fe80 Mon Sep 17 00:00:00 2001 From: Murisi Tarusenga Date: Thu, 27 Jul 2023 12:41:32 +0200 Subject: [PATCH 6/8] Now use i32s in AllowedConversions. --- masp_primitives/src/convert.rs | 28 +++++++-------- .../src/transaction/components/amount.rs | 35 +++++++++++++++++++ .../transaction/components/sapling/builder.rs | 10 +++--- masp_proofs/src/circuit/convert.rs | 6 ++-- 4 files changed, 57 insertions(+), 22 deletions(-) diff --git a/masp_primitives/src/convert.rs b/masp_primitives/src/convert.rs index 841d80ee..5a345cf5 100644 --- a/masp_primitives/src/convert.rs +++ b/masp_primitives/src/convert.rs @@ -3,7 +3,7 @@ use crate::{ pedersen_hash::{pedersen_hash, Personalization}, Node, ValueCommitment, }, - transaction::components::amount::{I64Sum, ValueSum}, + transaction::components::amount::{I32Sum, ValueSum}, }; use borsh::{BorshDeserialize, BorshSerialize}; use group::{Curve, GroupEncoding}; @@ -16,7 +16,7 @@ use std::{ #[derive(Clone, Debug, PartialEq, Eq)] pub struct AllowedConversion { /// The asset type that the note represents - assets: I64Sum, + assets: I32Sum, /// Memorize generator because it's expensive to recompute generator: jubjub::ExtendedPoint, } @@ -71,15 +71,15 @@ impl AllowedConversion { } } -impl From for I64Sum { - fn from(allowed_conversion: AllowedConversion) -> I64Sum { +impl From for I32Sum { + fn from(allowed_conversion: AllowedConversion) -> I32Sum { allowed_conversion.assets } } -impl From for AllowedConversion { +impl From for AllowedConversion { /// Produces an asset generator without cofactor cleared - fn from(assets: I64Sum) -> Self { + fn from(assets: I32Sum) -> Self { let mut asset_generator = jubjub::ExtendedPoint::identity(); for (asset, value) in assets.components() { // Compute the absolute value (failing if -i64::MAX is @@ -123,7 +123,7 @@ impl BorshDeserialize for AllowedConversion { /// computation of checking whether the asset generator corresponds to the /// deserialized amount. fn deserialize(buf: &mut &[u8]) -> borsh::maybestd::io::Result { - let assets = I64Sum::read(buf)?; + let assets = I32Sum::read(buf)?; let gen_bytes = <::Repr as BorshDeserialize>::deserialize(buf)?; let generator = Option::from(jubjub::ExtendedPoint::from_bytes(&gen_bytes)) @@ -199,12 +199,12 @@ mod tests { #[test] fn test_homomorphism() { // Left operand - let a = ValueSum::from_pair(zec(), 5i64).unwrap() - + ValueSum::from_pair(btc(), 6i64).unwrap() - + ValueSum::from_pair(xan(), 7i64).unwrap(); + let a = ValueSum::from_pair(zec(), 5i32).unwrap() + + ValueSum::from_pair(btc(), 6i32).unwrap() + + ValueSum::from_pair(xan(), 7i32).unwrap(); // Right operand let b = - ValueSum::from_pair(zec(), 2i64).unwrap() + ValueSum::from_pair(xan(), 10i64).unwrap(); + ValueSum::from_pair(zec(), 2i32).unwrap() + ValueSum::from_pair(xan(), 10i32).unwrap(); // Test homomorphism assert_eq!( AllowedConversion::from(a.clone() + b.clone()), @@ -214,9 +214,9 @@ mod tests { #[test] fn test_serialization() { // Make conversion - let a: AllowedConversion = (ValueSum::from_pair(zec(), 5i64).unwrap() - + ValueSum::from_pair(btc(), 6i64).unwrap() - + ValueSum::from_pair(xan(), 7i64).unwrap()) + let a: AllowedConversion = (ValueSum::from_pair(zec(), 5i32).unwrap() + + ValueSum::from_pair(btc(), 6i32).unwrap() + + ValueSum::from_pair(xan(), 7i32).unwrap()) .into(); // Serialize conversion let mut data = Vec::new(); diff --git a/masp_primitives/src/transaction/components/amount.rs b/masp_primitives/src/transaction/components/amount.rs index d1a59924..4c09cc8c 100644 --- a/masp_primitives/src/transaction/components/amount.rs +++ b/masp_primitives/src/transaction/components/amount.rs @@ -155,6 +155,41 @@ where } } +impl ValueSum { + /// Deserialize an Amount object from a list of amounts denominated by + /// different assets + pub fn read(reader: &mut R) -> std::io::Result { + let vec = Vector::read(reader, |reader| { + let mut atype = [0; 32]; + let mut value = [0; 4]; + reader.read_exact(&mut atype)?; + reader.read_exact(&mut value)?; + let atype = AssetType::from_identifier(&atype).ok_or_else(|| { + std::io::Error::new(std::io::ErrorKind::InvalidData, "invalid asset type") + })?; + Ok((atype, i32::from_le_bytes(value))) + })?; + 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") + })?; + } + Ok(ret) + } + + /// Serialize an Amount object into a list of amounts denominated by + /// distinct asset types + pub fn write(&self, writer: &mut W) -> std::io::Result<()> { + let vec: Vec<_> = self.components().collect(); + Vector::write(writer, vec.as_ref(), |writer, elt| { + writer.write_all(elt.0.get_identifier())?; + writer.write_all(elt.1.to_le_bytes().as_ref())?; + Ok(()) + }) + } +} + impl ValueSum { /// Deserialize an Amount object from a list of amounts denominated by /// different assets diff --git a/masp_primitives/src/transaction/components/sapling/builder.rs b/masp_primitives/src/transaction/components/sapling/builder.rs index a8187f63..ad0bf9b4 100644 --- a/masp_primitives/src/transaction/components/sapling/builder.rs +++ b/masp_primitives/src/transaction/components/sapling/builder.rs @@ -25,7 +25,7 @@ use crate::{ transaction::{ builder::Progress, components::{ - amount::{FromNt, I128Sum, I64Sum, ValueSum, MAX_MONEY}, + amount::{FromNt, I128Sum, I32Sum, ValueSum, MAX_MONEY}, sapling::{ fees, Authorization, Authorized, Bundle, ConvertDescription, GrothProofBytes, OutputDescription, SpendDescription, @@ -401,8 +401,8 @@ impl SaplingBuilder

{ let alpha = jubjub::Fr::random(&mut rng); - 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()) + .map_err(|_| Error::InvalidAmount)?; self.spends.push(SpendDescriptionInfo { extsk, @@ -437,8 +437,8 @@ impl SaplingBuilder

{ self.convert_anchor = Some(merkle_path.root(node).into()) } - let allowed_amt: I64Sum = allowed.clone().into(); - self.value_balance += I128Sum::from(FromNt(allowed_amt * (value as i64))); + let allowed_amt: I32Sum = allowed.clone().into(); + self.value_balance += I128Sum::from(FromNt(allowed_amt)) * (value as i128); self.converts.push(ConvertDescriptionInfo { allowed, diff --git a/masp_proofs/src/circuit/convert.rs b/masp_proofs/src/circuit/convert.rs index d0c08be1..c8bcb300 100644 --- a/masp_proofs/src/circuit/convert.rs +++ b/masp_proofs/src/circuit/convert.rs @@ -150,9 +150,9 @@ fn test_convert_circuit_with_bls12_381() { let output_asset = AssetType::new(format!("asset {}", i + 1).as_bytes()).unwrap(); let mint_asset = AssetType::new(b"reward").unwrap(); - let spend_value = -(i as i64 + 1); - let output_value = i as i64 + 1; - let mint_value = i as i64 + 1; + let spend_value = -(i as i32 + 1); + let output_value = i as i32 + 1; + let mint_value = i as i32 + 1; let allowed_conversion: AllowedConversion = (ValueSum::from_pair(spend_asset, spend_value) .unwrap() From 4862ae7a704e490c0455cecbacce36dffbacbd3f Mon Sep 17 00:00:00 2001 From: Murisi Tarusenga Date: Thu, 27 Jul 2023 13:57:16 +0200 Subject: [PATCH 7/8] Allow TxOut and TxIn values to take the value u64::MAX. --- masp_primitives/src/transaction/builder.rs | 39 ++++++------------- masp_primitives/src/transaction/components.rs | 5 ++- .../src/transaction/components/amount.rs | 33 +++++++--------- .../transaction/components/sapling/builder.rs | 2 +- .../src/transaction/components/transparent.rs | 28 ++++--------- .../components/transparent/builder.rs | 26 +++---------- .../components/transparent/fees.rs | 4 +- masp_primitives/src/transaction/fees.rs | 4 +- masp_primitives/src/transaction/fees/fixed.rs | 10 ++--- masp_primitives/src/transaction/sighash.rs | 2 +- masp_proofs/benches/convert.rs | 8 ++-- 11 files changed, 57 insertions(+), 104 deletions(-) diff --git a/masp_primitives/src/transaction/builder.rs b/masp_primitives/src/transaction/builder.rs index c8353730..1745ef39 100644 --- a/masp_primitives/src/transaction/builder.rs +++ b/masp_primitives/src/transaction/builder.rs @@ -1,6 +1,5 @@ //! Structs for building transactions. -use std::convert::TryInto; use std::error; use std::fmt; use std::sync::mpsc::Sender; @@ -19,7 +18,7 @@ use crate::{ sapling::{prover::TxProver, Diversifier, Node, Note, PaymentAddress}, transaction::{ components::{ - amount::{BalanceError, FromNt, I128Sum, I64Sum, ValueSum, MAX_MONEY}, + amount::{BalanceError, FromNt, I128Sum, U64Sum, ValueSum, MAX_MONEY}, sapling::{ self, builder::{SaplingBuilder, SaplingMetadata}, @@ -46,7 +45,7 @@ pub enum Error { InsufficientFunds(I128Sum), /// The transaction has inputs in excess of outputs and fees; the user must /// add a change output. - ChangeRequired(I64Sum), + ChangeRequired(U64Sum), /// An error occurred in computing the fees for a transaction. Fee(FeeError), /// An overflow or underflow occurred when computing value balances @@ -251,7 +250,7 @@ impl Builder { value: u64, memo: MemoBytes, ) -> Result<(), sapling::builder::Error> { - if value > MAX_MONEY.try_into().unwrap() { + if value > MAX_MONEY { return Err(sapling::builder::Error::InvalidAmount); } self.sapling_builder @@ -273,9 +272,9 @@ impl Builder { &mut self, to: &TransparentAddress, asset_type: AssetType, - value: i64, + value: u64, ) -> Result<(), transparent::builder::Error> { - if value < 0 || value > MAX_MONEY { + if value > MAX_MONEY { return Err(transparent::builder::Error::InvalidAmount); } @@ -326,7 +325,7 @@ impl Builder { fn build_internal( self, prover: &impl TxProver, - fee: I64Sum, + fee: U64Sum, ) -> Result<(Transaction, SaplingMetadata), Error> { let consensus_branch_id = BranchId::for_height(&self.params, self.target_height); @@ -480,9 +479,8 @@ mod tests { merkle_tree::{CommitmentTree, IncrementalWitness}, sapling::Rseed, transaction::{ - components::amount::{FromNt, I128Sum, ValueSum, DEFAULT_FEE, MAX_MONEY}, - sapling::builder::{self as build_s}, - transparent::builder::{self as build_t}, + components::amount::{FromNt, I128Sum, ValueSum, DEFAULT_FEE}, + sapling::builder as build_s, TransparentAddress, }, zip32::ExtendedSpendingKey, @@ -490,7 +488,7 @@ mod tests { use super::{Builder, Error}; - #[test] + /*#[test] fn fails_on_overflow_output() { let extsk = ExtendedSpendingKey::master(&[]); let dfvk = extsk.to_diversifiable_full_viewing_key(); @@ -507,12 +505,12 @@ mod tests { Some(ovk), to, zec(), - MAX_MONEY as u64 + 1, + MAX_MONEY + 1, MemoBytes::empty() ), Err(build_s::Error::InvalidAmount) ); - } + }*/ /// Generate ZEC asset type fn zec() -> AssetType { @@ -565,21 +563,6 @@ mod tests { ); } - #[test] - fn fails_on_negative_transparent_output() { - let mut rng = OsRng; - - let transparent_address = TransparentAddress(rng.gen::<[u8; 20]>()); - let tx_height = TEST_NETWORK - .activation_height(NetworkUpgrade::MASP) - .unwrap(); - let mut builder = Builder::new(TEST_NETWORK, tx_height); - assert_eq!( - builder.add_transparent_output(&transparent_address, zec(), -1,), - Err(build_t::Error::InvalidAmount) - ); - } - #[test] fn fails_on_negative_change() { let mut rng = OsRng; diff --git a/masp_primitives/src/transaction/components.rs b/masp_primitives/src/transaction/components.rs index 2cb438fe..7e76b55b 100644 --- a/masp_primitives/src/transaction/components.rs +++ b/masp_primitives/src/transaction/components.rs @@ -4,7 +4,10 @@ pub mod amount; pub mod sapling; pub mod transparent; pub use self::{ - amount::{I128Sum, I64Sum, ValueSum}, + amount::{ + FromNt, I128Sum, I16Sum, I32Sum, I64Sum, I8Sum, TryFromNt, U128Sum, U16Sum, U32Sum, U64Sum, + U8Sum, ValueSum, + }, sapling::{ConvertDescription, OutputDescription, SpendDescription}, transparent::{TxIn, TxOut}, }; diff --git a/masp_primitives/src/transaction/components/amount.rs b/masp_primitives/src/transaction/components/amount.rs index 4c09cc8c..56f2c287 100644 --- a/masp_primitives/src/transaction/components/amount.rs +++ b/masp_primitives/src/transaction/components/amount.rs @@ -11,10 +11,9 @@ use std::iter::Sum; use std::ops::{Add, AddAssign, Index, Mul, MulAssign, Neg, Sub, SubAssign}; use zcash_encoding::Vector; -pub const MAX_MONEY: i64 = i64::MAX; -pub const MIN_MONEY: i64 = i64::MIN; +pub const MAX_MONEY: u64 = u64::MAX; lazy_static::lazy_static! { -pub static ref DEFAULT_FEE: I64Sum = ValueSum::from_pair(zec(), 1000).unwrap(); +pub static ref DEFAULT_FEE: U64Sum = ValueSum::from_pair(zec(), 1000).unwrap(); } /// A type-safe representation of some quantity of Zcash. /// @@ -76,9 +75,7 @@ where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, { - /// Creates a non-negative Amount from an i64. - /// - /// Returns an error if the amount is outside the range `{0..MAX_MONEY}`. + /// Creates a non-negative Amount from a Magnitude. pub fn from_nonnegative(atype: Unit, amount: Magnitude) -> Result { if amount == Magnitude::default() { Ok(Self::zero()) @@ -97,9 +94,7 @@ where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, { - /// Creates an Amount from a type convertible to i64. - /// - /// Returns an error if the amount is outside the range `{-MAX_MONEY..MAX_MONEY}`. + /// Creates an Amount from a Magnitude. pub fn from_pair(atype: Unit, amount: Magnitude) -> Result { if amount == Magnitude::default() { Ok(Self::zero()) @@ -653,29 +648,29 @@ pub fn default_fee() -> ValueSum { pub mod testing { use proptest::prelude::prop_compose; - use super::{I128Sum, I64Sum, ValueSum, MAX_MONEY}; + use super::{I128Sum, I64Sum, U64Sum, ValueSum, MAX_MONEY}; use crate::asset_type::testing::arb_asset_type; prop_compose! { - pub fn arb_i64_sum()(asset_type in arb_asset_type(), amt in -MAX_MONEY..MAX_MONEY) -> I64Sum { + 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() } } prop_compose! { - pub fn arb_i128_sum()(asset_type in arb_asset_type(), amt in -MAX_MONEY..MAX_MONEY) -> I128Sum { + pub fn arb_i128_sum()(asset_type in arb_asset_type(), amt in i128::MIN..i128::MAX) -> I128Sum { ValueSum::from_pair(asset_type, amt as i128).unwrap() } } prop_compose! { - pub fn arb_nonnegative_amount()(asset_type in arb_asset_type(), amt in 0i64..MAX_MONEY) -> I64Sum { + pub fn arb_nonnegative_amount()(asset_type in arb_asset_type(), amt in 0u64..MAX_MONEY) -> U64Sum { ValueSum::from_pair(asset_type, amt).unwrap() } } prop_compose! { - pub fn arb_positive_amount()(asset_type in arb_asset_type(), amt in 1i64..MAX_MONEY) -> I64Sum { + pub fn arb_positive_amount()(asset_type in arb_asset_type(), amt in 1u64..MAX_MONEY) -> U64Sum { ValueSum::from_pair(asset_type, amt).unwrap() } } @@ -683,7 +678,7 @@ pub mod testing { #[cfg(test)] mod tests { - use super::{zec, I64Sum, ValueSum, MAX_MONEY, MIN_MONEY}; + use super::{zec, I64Sum, ValueSum, MAX_MONEY}; #[test] fn amount_in_range() { @@ -700,7 +695,7 @@ mod tests { assert_eq!( I64Sum::read(&mut max_money.as_ref()).unwrap(), - I64Sum::from_pair(zec(), MAX_MONEY).unwrap() + I64Sum::from_pair(zec(), i64::MAX).unwrap() ); //let max_money_p1 = b"\x01\x94\xf3O\xfdd\xef\n\xc3i\x08\xfd\xdf\xec\x05hX\x06)\xc4Vq\x0f\xa1\x86\x83\x12\xa8\x7f\xbf\n\xa5\t\x01\x40\x07\x5a\xf0\x75\x07\x00"; @@ -715,7 +710,7 @@ mod tests { let neg_max_money = b"\x01\x94\xf3O\xfdd\xef\n\xc3i\x08\xfd\xdf\xec\x05hX\x06)\xc4Vq\x0f\xa1\x86\x83\x12\xa8\x7f\xbf\n\xa5\t\x01\x00\x00\x00\x00\x00\x00\x80"; assert_eq!( I64Sum::read(&mut neg_max_money.as_ref()).unwrap(), - I64Sum::from_pair(zec(), -MAX_MONEY).unwrap() + I64Sum::from_pair(zec(), -i64::MAX).unwrap() ); } @@ -736,14 +731,14 @@ mod tests { #[test] #[should_panic] fn sub_panics_on_underflow() { - let v = ValueSum::from_pair(zec(), MIN_MONEY).unwrap(); + let v = ValueSum::from_pair(zec(), 0u64).unwrap(); let _diff = v - ValueSum::from_pair(zec(), 1).unwrap(); } #[test] #[should_panic] fn sub_assign_panics_on_underflow() { - let mut a = ValueSum::from_pair(zec(), MIN_MONEY).unwrap(); + let mut a = ValueSum::from_pair(zec(), 0u64).unwrap(); a -= ValueSum::from_pair(zec(), 1).unwrap(); } } diff --git a/masp_primitives/src/transaction/components/sapling/builder.rs b/masp_primitives/src/transaction/components/sapling/builder.rs index ad0bf9b4..ceda98a8 100644 --- a/masp_primitives/src/transaction/components/sapling/builder.rs +++ b/masp_primitives/src/transaction/components/sapling/builder.rs @@ -146,7 +146,7 @@ impl SaplingOutputInfo { memo: MemoBytes, ) -> Result { let g_d = to.g_d().ok_or(Error::InvalidAddress)?; - if value > MAX_MONEY.try_into().unwrap() { + if value > MAX_MONEY { return Err(Error::InvalidAmount); } diff --git a/masp_primitives/src/transaction/components/transparent.rs b/masp_primitives/src/transaction/components/transparent.rs index d2553fc5..85391e8c 100644 --- a/masp_primitives/src/transaction/components/transparent.rs +++ b/masp_primitives/src/transaction/components/transparent.rs @@ -65,26 +65,14 @@ impl Bundle { let input_sum = self .vin .iter() - .map(|p| { - if p.value >= 0 { - ValueSum::from_pair(p.asset_type, p.value as i128) - } else { - Err(()) - } - }) + .map(|p| ValueSum::from_pair(p.asset_type, p.value as i128)) .sum::>() .map_err(|_| BalanceError::Overflow)?; let output_sum = self .vout .iter() - .map(|p| { - if p.value >= 0 { - ValueSum::from_pair(p.asset_type, p.value as i128) - } else { - Err(()) - } - }) + .map(|p| ValueSum::from_pair(p.asset_type, p.value as i128)) .sum::>() .map_err(|_| BalanceError::Overflow)?; @@ -96,7 +84,7 @@ impl Bundle { #[derive(Debug, Clone, PartialEq, Eq)] pub struct TxIn { pub asset_type: AssetType, - pub value: i64, + pub value: u64, pub address: TransparentAddress, pub transparent_sig: A::TransparentSig, } @@ -112,9 +100,9 @@ impl TxIn { let value = { let mut tmp = [0u8; 8]; reader.read_exact(&mut tmp)?; - i64::from_le_bytes(tmp) + u64::from_le_bytes(tmp) }; - if value < 0 || value > MAX_MONEY { + if value > MAX_MONEY { return Err(io::Error::new( io::ErrorKind::InvalidData, "value out of range", @@ -144,7 +132,7 @@ impl TxIn { #[derive(Clone, Debug, Hash, PartialOrd, PartialEq, Ord, Eq)] pub struct TxOut { pub asset_type: AssetType, - pub value: i64, + pub value: u64, pub address: TransparentAddress, } @@ -159,9 +147,9 @@ impl TxOut { let value = { let mut tmp = [0u8; 8]; reader.read_exact(&mut tmp)?; - i64::from_le_bytes(tmp) + u64::from_le_bytes(tmp) }; - if value < 0 || value > MAX_MONEY { + if value > MAX_MONEY { return Err(io::Error::new( io::ErrorKind::InvalidData, "value out of range", diff --git a/masp_primitives/src/transaction/components/transparent/builder.rs b/masp_primitives/src/transaction/components/transparent/builder.rs index b5aa9fe9..f97eb491 100644 --- a/masp_primitives/src/transaction/components/transparent/builder.rs +++ b/masp_primitives/src/transaction/components/transparent/builder.rs @@ -105,10 +105,6 @@ impl TransparentBuilder { /// Adds a coin (the output of a previous transaction) to be spent to the transaction. #[cfg(feature = "transparent-inputs")] pub fn add_input(&mut self, coin: TxOut) -> Result<(), Error> { - if coin.value.is_negative() { - return Err(Error::InvalidAmount); - } - self.inputs.push(TransparentInputInfo { coin }); Ok(()) @@ -118,9 +114,9 @@ impl TransparentBuilder { &mut self, to: &TransparentAddress, asset_type: AssetType, - value: i64, + value: u64, ) -> Result<(), Error> { - if value < 0 || value > MAX_MONEY { + if value > MAX_MONEY { return Err(Error::InvalidAmount); } @@ -138,13 +134,7 @@ impl TransparentBuilder { let input_sum = self .inputs .iter() - .map(|input| { - if input.coin.value >= 0 { - ValueSum::from_pair(input.coin.asset_type, input.coin.value as i128) - } else { - Err(()) - } - }) + .map(|input| ValueSum::from_pair(input.coin.asset_type, input.coin.value as i128)) .sum::>() .map_err(|_| BalanceError::Overflow)?; @@ -154,13 +144,7 @@ impl TransparentBuilder { let output_sum = self .vout .iter() - .map(|vo| { - if vo.value >= 0 { - ValueSum::from_pair(vo.asset_type, vo.value as i128) - } else { - Err(()) - } - }) + .map(|vo| ValueSum::from_pair(vo.asset_type, vo.value as i128)) .sum::>() .map_err(|_| BalanceError::Overflow)?; @@ -208,7 +192,7 @@ impl TransparentAuthorizingContext for Unauthorized { #[cfg(feature = "transparent-inputs")] impl TransparentAuthorizingContext for Unauthorized { - fn input_amounts(&self) -> Vec<(AssetType, i64)> { + fn input_amounts(&self) -> Vec<(AssetType, u64)> { return self .inputs .iter() diff --git a/masp_primitives/src/transaction/components/transparent/fees.rs b/masp_primitives/src/transaction/components/transparent/fees.rs index cff0a544..99522494 100644 --- a/masp_primitives/src/transaction/components/transparent/fees.rs +++ b/masp_primitives/src/transaction/components/transparent/fees.rs @@ -16,7 +16,7 @@ pub trait InputView { /// fee and change computation. pub trait OutputView { /// Returns the value of the output being created. - fn value(&self) -> i64; + fn value(&self) -> u64; /// Returns the asset type of the output being created. fn asset_type(&self) -> AssetType; /// Returns the script corresponding to the newly created output. @@ -24,7 +24,7 @@ pub trait OutputView { } impl OutputView for TxOut { - fn value(&self) -> i64 { + fn value(&self) -> u64 { self.value } diff --git a/masp_primitives/src/transaction/fees.rs b/masp_primitives/src/transaction/fees.rs index eed0f838..24d50aad 100644 --- a/masp_primitives/src/transaction/fees.rs +++ b/masp_primitives/src/transaction/fees.rs @@ -2,7 +2,7 @@ use crate::{ consensus::{self, BlockHeight}, - transaction::components::{amount::I64Sum, transparent::fees as transparent}, + transaction::components::{amount::U64Sum, transparent::fees as transparent}, }; pub mod fixed; @@ -24,5 +24,5 @@ pub trait FeeRule { transparent_outputs: &[impl transparent::OutputView], sapling_input_count: usize, sapling_output_count: usize, - ) -> Result; + ) -> Result; } diff --git a/masp_primitives/src/transaction/fees/fixed.rs b/masp_primitives/src/transaction/fees/fixed.rs index 92d434da..8ea28c72 100644 --- a/masp_primitives/src/transaction/fees/fixed.rs +++ b/masp_primitives/src/transaction/fees/fixed.rs @@ -1,7 +1,7 @@ use crate::{ consensus::{self, BlockHeight}, transaction::components::{ - amount::{I64Sum, DEFAULT_FEE}, + amount::{U64Sum, DEFAULT_FEE}, transparent::fees as transparent, }, }; @@ -10,12 +10,12 @@ use crate::{ /// the transaction being constructed. #[derive(Clone, Debug)] pub struct FeeRule { - fixed_fee: I64Sum, + fixed_fee: U64Sum, } impl FeeRule { /// Creates a new nonstandard fixed fee rule with the specified fixed fee. - pub fn non_standard(fixed_fee: I64Sum) -> Self { + pub fn non_standard(fixed_fee: U64Sum) -> Self { Self { fixed_fee } } @@ -27,7 +27,7 @@ impl FeeRule { } /// Returns the fixed fee amount which which this rule was configured. - pub fn fixed_fee(&self) -> I64Sum { + pub fn fixed_fee(&self) -> U64Sum { self.fixed_fee.clone() } } @@ -42,7 +42,7 @@ impl super::FeeRule for FeeRule { _transparent_outputs: &[impl transparent::OutputView], _sapling_input_count: usize, _sapling_output_count: usize, - ) -> Result { + ) -> Result { Ok(self.fixed_fee.clone()) } } diff --git a/masp_primitives/src/transaction/sighash.rs b/masp_primitives/src/transaction/sighash.rs index e6c9d3c4..c4d4f5d7 100644 --- a/masp_primitives/src/transaction/sighash.rs +++ b/masp_primitives/src/transaction/sighash.rs @@ -55,7 +55,7 @@ pub trait TransparentAuthorizingContext: transparent::Authorization { /// so that wallets can commit to the transparent input breakdown /// without requiring the full data of the previous transactions /// providing these inputs. - fn input_amounts(&self) -> Vec<(AssetType, i64)>; + fn input_amounts(&self) -> Vec<(AssetType, u64)>; } /// Computes the signature hash for an input to a transaction, given diff --git a/masp_proofs/benches/convert.rs b/masp_proofs/benches/convert.rs index a295cf00..60fbbc74 100644 --- a/masp_proofs/benches/convert.rs +++ b/masp_proofs/benches/convert.rs @@ -29,14 +29,14 @@ fn criterion_benchmark(c: &mut Criterion) { .unwrap(); c.bench_function("convert", |b| { - let i = rng.next_u32(); + let i = rng.next_u32() >> 1; let spend_asset = AssetType::new(format!("asset {}", i).as_bytes()).unwrap(); let output_asset = AssetType::new(format!("asset {}", i + 1).as_bytes()).unwrap(); let mint_asset = AssetType::new(b"reward").unwrap(); - let spend_value = -(i as i64 + 1); - let output_value = i as i64 + 1; - let mint_value = i as i64 + 1; + let spend_value = -(i as i32 + 1); + let output_value = i as i32 + 1; + let mint_value = i as i32 + 1; let allowed_conversion: AllowedConversion = (ValueSum::from_pair(spend_asset, spend_value) .unwrap() From 2f185600365e5cfcfc25296ca2a6720a081d9f9f Mon Sep 17 00:00:00 2001 From: Murisi Tarusenga Date: Mon, 7 Aug 2023 11:15:48 +0200 Subject: [PATCH 8/8] Renamed Magnitude type parameter to Value. Added is_zero function ValueSum. Corrected comments. --- masp_primitives/src/transaction/builder.rs | 14 +- masp_primitives/src/transaction/components.rs | 3 +- .../src/transaction/components/amount.rs | 224 +++++++++--------- .../transaction/components/sapling/builder.rs | 4 +- 4 files changed, 122 insertions(+), 123 deletions(-) diff --git a/masp_primitives/src/transaction/builder.rs b/masp_primitives/src/transaction/builder.rs index 1745ef39..fc65dcfd 100644 --- a/masp_primitives/src/transaction/builder.rs +++ b/masp_primitives/src/transaction/builder.rs @@ -18,7 +18,7 @@ use crate::{ sapling::{prover::TxProver, Diversifier, Node, Note, PaymentAddress}, transaction::{ components::{ - amount::{BalanceError, FromNt, I128Sum, U64Sum, ValueSum, MAX_MONEY}, + amount::{BalanceError, I128Sum, U64Sum, ValueSum, MAX_MONEY}, sapling::{ self, builder::{SaplingBuilder, SaplingMetadata}, @@ -337,7 +337,7 @@ impl Builder { // // After fees are accounted for, the value balance of the transaction must be zero. - let balance_after_fees = self.value_balance()? - I128Sum::from(FromNt(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)); @@ -479,7 +479,7 @@ mod tests { merkle_tree::{CommitmentTree, IncrementalWitness}, sapling::Rseed, transaction::{ - components::amount::{FromNt, I128Sum, ValueSum, DEFAULT_FEE}, + components::amount::{I128Sum, ValueSum, DEFAULT_FEE}, sapling::builder as build_s, TransparentAddress, }, @@ -580,7 +580,9 @@ mod tests { let builder = Builder::new(TEST_NETWORK, tx_height); assert_eq!( builder.mock_build(), - Err(Error::InsufficientFunds(FromNt(DEFAULT_FEE.clone()).into())) + Err(Error::InsufficientFunds(I128Sum::from_sum( + DEFAULT_FEE.clone() + ))) ); } @@ -599,7 +601,7 @@ mod tests { builder.mock_build(), Err(Error::InsufficientFunds( I128Sum::from_pair(zec(), 50000).unwrap() - + &I128Sum::from(FromNt(DEFAULT_FEE.clone())) + + &I128Sum::from_sum(DEFAULT_FEE.clone()) )) ); } @@ -615,7 +617,7 @@ mod tests { builder.mock_build(), Err(Error::InsufficientFunds( I128Sum::from_pair(zec(), 50000).unwrap() - + &I128Sum::from(FromNt(DEFAULT_FEE.clone())) + + &I128Sum::from_sum(DEFAULT_FEE.clone()) )) ); } diff --git a/masp_primitives/src/transaction/components.rs b/masp_primitives/src/transaction/components.rs index 7e76b55b..cc041955 100644 --- a/masp_primitives/src/transaction/components.rs +++ b/masp_primitives/src/transaction/components.rs @@ -5,8 +5,7 @@ pub mod sapling; pub mod transparent; pub use self::{ amount::{ - FromNt, I128Sum, I16Sum, I32Sum, I64Sum, I8Sum, TryFromNt, U128Sum, U16Sum, U32Sum, U64Sum, - U8Sum, ValueSum, + I128Sum, I16Sum, I32Sum, I64Sum, I8Sum, U128Sum, U16Sum, U32Sum, U64Sum, U8Sum, ValueSum, }, sapling::{ConvertDescription, OutputDescription, SpendDescription}, transparent::{TxIn, TxOut}, diff --git a/masp_primitives/src/transaction/components/amount.rs b/masp_primitives/src/transaction/components/amount.rs index 56f2c287..fac323c5 100644 --- a/masp_primitives/src/transaction/components/amount.rs +++ b/masp_primitives/src/transaction/components/amount.rs @@ -17,12 +17,12 @@ pub static ref DEFAULT_FEE: U64Sum = ValueSum::from_pair(zec(), 1000).unwrap(); } /// A type-safe representation of some quantity of Zcash. /// -/// An Amount can only be constructed from an integer that is within the valid monetary +/// An ValueSum can only be constructed from an integer that is within the valid monetary /// range of `{-MAX_MONEY..MAX_MONEY}` (where `MAX_MONEY` = i64::MAX). /// However, this range is not preserved as an invariant internally; it is possible to -/// add two valid Amounts together to obtain an invalid Amount. It is the user's -/// responsibility to handle the result of serializing potentially-invalid Amounts. In -/// particular, a `Transaction` containing serialized invalid Amounts will be rejected +/// add two valid ValueSums together to obtain an invalid ValueSum. It is the user's +/// responsibility to handle the result of serializing potentially-invalid ValueSums. In +/// particular, a `Transaction` containing serialized invalid ValueSums will be rejected /// by the network consensus rules. /// @@ -49,13 +49,13 @@ pub type U128Sum = ValueSum; #[derive(Clone, Default, Debug, PartialEq, Eq, BorshSerialize, BorshDeserialize, Hash)] pub struct ValueSum< Unit: Hash + Ord + BorshSerialize + BorshDeserialize, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq, ->(pub BTreeMap); + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq, +>(pub BTreeMap); -impl memuse::DynamicUsage for ValueSum +impl memuse::DynamicUsage for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, { #[inline(always)] fn dynamic_usage(&self) -> usize { @@ -70,16 +70,16 @@ where } } -impl ValueSum +impl ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, { - /// Creates a non-negative Amount from a Magnitude. - pub fn from_nonnegative(atype: Unit, amount: Magnitude) -> Result { - if amount == Magnitude::default() { + /// Creates a non-negative ValueSum from a Value. + pub fn from_nonnegative(atype: Unit, amount: Value) -> Result { + if amount == Value::default() { Ok(Self::zero()) - } else if Magnitude::default() <= amount { + } else if Value::default() <= amount { let mut ret = BTreeMap::new(); ret.insert(atype, amount); Ok(ValueSum(ret)) @@ -89,14 +89,14 @@ where } } -impl ValueSum +impl ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, { - /// Creates an Amount from a Magnitude. - pub fn from_pair(atype: Unit, amount: Magnitude) -> Result { - if amount == Magnitude::default() { + /// Creates an ValueSum from a Value. + pub fn from_pair(atype: Unit, amount: Value) -> Result { + if amount == Value::default() { Ok(Self::zero()) } else { let mut ret = BTreeMap::new(); @@ -105,44 +105,49 @@ where } } - /// Filters out everything but the given AssetType from this Amount + /// 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() } - /// Get the given AssetType within this Amount - pub fn get(&self, index: &Unit) -> Magnitude { - *self.0.get(index).unwrap_or(&Magnitude::default()) + /// Get the given AssetType within this ValueSum + pub fn get(&self, index: &Unit) -> Value { + *self.0.get(index).unwrap_or(&Value::default()) } } -impl ValueSum +impl ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, { - /// Returns a zero-valued Amount. + /// Returns a zero-valued ValueSum. pub fn zero() -> Self { ValueSum(BTreeMap::new()) } + /// Check if ValueSum is zero + pub fn is_zero(&self) -> bool { + self.0.is_empty() + } + /// Returns an iterator over the amount's non-zero asset-types - pub fn asset_types(&self) -> Keys<'_, Unit, Magnitude> { + pub fn asset_types(&self) -> Keys<'_, Unit, Value> { self.0.keys() } /// Returns an iterator over the amount's non-zero components - pub fn components(&self) -> Iter<'_, Unit, Magnitude> { + pub fn components(&self) -> Iter<'_, Unit, Value> { self.0.iter() } /// Returns an iterator over the amount's non-zero components - pub fn into_components(self) -> IntoIter { + pub fn into_components(self) -> IntoIter { self.0.into_iter() } - /// Filters out the given AssetType from this Amount + /// Filters out the given AssetType from this ValueSum pub fn reject(&self, index: Unit) -> Self { let mut val = self.clone(); val.0.remove(&index); @@ -151,7 +156,7 @@ where } impl ValueSum { - /// Deserialize an Amount object from a list of amounts denominated by + /// Deserialize an ValueSum object from a list of amounts denominated by /// different assets pub fn read(reader: &mut R) -> std::io::Result { let vec = Vector::read(reader, |reader| { @@ -173,7 +178,7 @@ impl ValueSum { Ok(ret) } - /// Serialize an Amount object into a list of amounts denominated by + /// Serialize an ValueSum object into a list of amounts denominated by /// distinct asset types pub fn write(&self, writer: &mut W) -> std::io::Result<()> { let vec: Vec<_> = self.components().collect(); @@ -186,7 +191,7 @@ impl ValueSum { } impl ValueSum { - /// Deserialize an Amount object from a list of amounts denominated by + /// Deserialize an ValueSum object from a list of amounts denominated by /// different assets pub fn read(reader: &mut R) -> std::io::Result { let vec = Vector::read(reader, |reader| { @@ -208,7 +213,7 @@ impl ValueSum { Ok(ret) } - /// Serialize an Amount object into a list of amounts denominated by + /// Serialize an ValueSum object into a list of amounts denominated by /// distinct asset types pub fn write(&self, writer: &mut W) -> std::io::Result<()> { let vec: Vec<_> = self.components().collect(); @@ -221,7 +226,7 @@ impl ValueSum { } impl ValueSum { - /// Deserialize an Amount object from a list of amounts denominated by + /// Deserialize an ValueSum object from a list of amounts denominated by /// different assets pub fn read(reader: &mut R) -> std::io::Result { let vec = Vector::read(reader, |reader| { @@ -243,7 +248,7 @@ impl ValueSum { Ok(ret) } - /// Serialize an Amount object into a list of amounts denominated by + /// Serialize an ValueSum object into a list of amounts denominated by /// distinct asset types pub fn write(&self, writer: &mut W) -> std::io::Result<()> { let vec: Vec<_> = self.components().collect(); @@ -255,27 +260,27 @@ impl ValueSum { } } -impl From for ValueSum +impl From for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + One, + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + One, { fn from(atype: Unit) -> Self { let mut ret = BTreeMap::new(); - ret.insert(atype, Magnitude::one()); + ret.insert(atype, Value::one()); ValueSum(ret) } } -impl PartialOrd for ValueSum +impl PartialOrd for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, { /// One ValueSum is more than or equal to another if each corresponding /// coordinate is more than or equal to the other's. fn partial_cmp(&self, other: &Self) -> Option { - let zero = Magnitude::default(); + let zero = Value::default(); let mut ordering = Some(Ordering::Equal); for k in self.0.keys().chain(other.0.keys()) { let v1 = self.0.get(k).unwrap_or(&zero); @@ -340,10 +345,10 @@ impl_index!(i128); impl_index!(u128); -impl MulAssign for ValueSum +impl MulAssign for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq @@ -352,15 +357,15 @@ where + PartialOrd + CheckedMul, { - fn mul_assign(&mut self, rhs: Magnitude) { + fn mul_assign(&mut self, rhs: Value) { *self = self.clone() * rhs; } } -impl Mul for ValueSum +impl Mul for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq @@ -369,9 +374,9 @@ where + PartialOrd + CheckedMul, { - type Output = ValueSum; + type Output = ValueSum; - fn mul(self, rhs: Magnitude) -> Self::Output { + fn mul(self, rhs: Value) -> Self::Output { let mut comps = BTreeMap::new(); for (atype, amount) in self.0.iter() { comps.insert( @@ -379,15 +384,15 @@ where amount.checked_mul(&rhs).expect("overflow detected"), ); } - comps.retain(|_, v| *v != Magnitude::default()); + comps.retain(|_, v| *v != Value::default()); ValueSum(comps) } } -impl AddAssign<&ValueSum> for ValueSum +impl AddAssign<&ValueSum> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq @@ -396,15 +401,15 @@ where + PartialOrd + CheckedAdd, { - fn add_assign(&mut self, rhs: &ValueSum) { + fn add_assign(&mut self, rhs: &ValueSum) { *self = self.clone() + rhs; } } -impl AddAssign> for ValueSum +impl AddAssign> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq @@ -413,15 +418,15 @@ where + PartialOrd + CheckedAdd, { - fn add_assign(&mut self, rhs: ValueSum) { + fn add_assign(&mut self, rhs: ValueSum) { *self += &rhs } } -impl Add<&ValueSum> for ValueSum +impl Add<&ValueSum> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq @@ -430,9 +435,9 @@ where + PartialOrd + CheckedAdd, { - type Output = ValueSum; + type Output = ValueSum; - fn add(self, rhs: &ValueSum) -> Self::Output { + fn add(self, rhs: &ValueSum) -> Self::Output { let mut comps = self.0.clone(); for (atype, amount) in rhs.components() { comps.insert( @@ -442,15 +447,15 @@ where .expect("overflow detected"), ); } - comps.retain(|_, v| *v != Magnitude::default()); + comps.retain(|_, v| *v != Value::default()); ValueSum(comps) } } -impl Add> for ValueSum +impl Add> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq @@ -459,17 +464,17 @@ where + PartialOrd + CheckedAdd, { - type Output = ValueSum; + type Output = ValueSum; - fn add(self, rhs: ValueSum) -> Self::Output { + fn add(self, rhs: ValueSum) -> Self::Output { self + &rhs } } -impl SubAssign<&ValueSum> for ValueSum +impl SubAssign<&ValueSum> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq @@ -478,15 +483,15 @@ where + PartialOrd + CheckedSub, { - fn sub_assign(&mut self, rhs: &ValueSum) { + fn sub_assign(&mut self, rhs: &ValueSum) { *self = self.clone() - rhs } } -impl SubAssign> for ValueSum +impl SubAssign> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq @@ -495,15 +500,15 @@ where + PartialOrd + CheckedSub, { - fn sub_assign(&mut self, rhs: ValueSum) { + fn sub_assign(&mut self, rhs: ValueSum) { *self -= &rhs } } -impl Neg for ValueSum +impl Neg for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq @@ -512,7 +517,7 @@ where + PartialOrd + CheckedNeg, { - type Output = ValueSum; + type Output = ValueSum; fn neg(mut self) -> Self::Output { let mut comps = BTreeMap::new(); @@ -522,19 +527,19 @@ where amount.checked_neg().expect("overflow detected"), ); } - comps.retain(|_, v| *v != Magnitude::default()); + comps.retain(|_, v| *v != Value::default()); ValueSum(comps) } } -impl Sub<&ValueSum> for ValueSum +impl Sub<&ValueSum> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, { - type Output = ValueSum; + type Output = ValueSum; - fn sub(self, rhs: &ValueSum) -> Self::Output { + fn sub(self, rhs: &ValueSum) -> Self::Output { let mut comps = self.0.clone(); for (atype, amount) in rhs.components() { comps.insert( @@ -544,27 +549,27 @@ where .expect("overflow detected"), ); } - comps.retain(|_, v| *v != Magnitude::default()); + comps.retain(|_, v| *v != Value::default()); ValueSum(comps) } } -impl Sub> for ValueSum +impl Sub> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, { - type Output = ValueSum; + type Output = ValueSum; - fn sub(self, rhs: ValueSum) -> Self::Output { + fn sub(self, rhs: ValueSum) -> Self::Output { self - &rhs } } -impl Sum for ValueSum +impl Sum for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + PartialOrd, Self: Add, { fn sum>(iter: I) -> Self { @@ -572,39 +577,32 @@ where } } -/// Workaround for the blanket implementation of TryFrom -pub struct TryFromNt(pub X); - -impl TryFrom>> - for ValueSum +impl ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, - Output: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + TryFrom, + Output: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, { - type Error = >::Error; - - fn try_from(x: TryFromNt>) -> Result { + pub fn try_from_sum( + x: ValueSum, + ) -> Result>::Error> + where + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, + Output: TryFrom, + { let mut comps = BTreeMap::new(); - for (atype, amount) in x.0 .0 { + for (atype, amount) in x.0 { comps.insert(atype, amount.try_into()?); } Ok(Self(comps)) } -} -/// Workaround for the blanket implementation of TryFrom -pub struct FromNt(pub X); - -impl From>> for ValueSum -where - Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Magnitude: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, - Output: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + From, -{ - fn from(x: FromNt>) -> Self { + pub fn from_sum(x: ValueSum) -> Self + where + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy, + Output: From, + { let mut comps = BTreeMap::new(); - for (atype, amount) in x.0 .0 { + for (atype, amount) in x.0 { comps.insert(atype, amount.into()); } Self(comps) @@ -625,12 +623,12 @@ impl std::fmt::Display for BalanceError { BalanceError::Overflow => { write!( f, - "Amount addition resulted in a value outside the valid range." + "ValueSum addition resulted in a value outside the valid range." ) } BalanceError::Underflow => write!( f, - "Amount subtraction resulted in a value outside the valid range." + "ValueSum subtraction resulted in a value outside the valid range." ), } } @@ -699,10 +697,10 @@ mod tests { ); //let max_money_p1 = b"\x01\x94\xf3O\xfdd\xef\n\xc3i\x08\xfd\xdf\xec\x05hX\x06)\xc4Vq\x0f\xa1\x86\x83\x12\xa8\x7f\xbf\n\xa5\t\x01\x40\x07\x5a\xf0\x75\x07\x00"; - //assert!(Amount::read(&mut max_money_p1.as_ref()).is_err()); + //assert!(ValueSum::read(&mut max_money_p1.as_ref()).is_err()); //let mut neg_max_money = [0u8; 41]; - //let mut amount = Amount::from_pair(zec(), -MAX_MONEY).unwrap(); + //let mut amount = ValueSum::from_pair(zec(), -MAX_MONEY).unwrap(); //*amount.0.get_mut(&zec()).unwrap() = i64::MIN; //amount.write(&mut neg_max_money.as_mut()); //dbg!(std::str::from_utf8(&neg_max_money.as_ref().iter().map(|b| std::ascii::escape_default(*b)).flatten().collect::>()).unwrap()); diff --git a/masp_primitives/src/transaction/components/sapling/builder.rs b/masp_primitives/src/transaction/components/sapling/builder.rs index ceda98a8..9d5594ca 100644 --- a/masp_primitives/src/transaction/components/sapling/builder.rs +++ b/masp_primitives/src/transaction/components/sapling/builder.rs @@ -25,7 +25,7 @@ use crate::{ transaction::{ builder::Progress, components::{ - amount::{FromNt, I128Sum, I32Sum, ValueSum, MAX_MONEY}, + amount::{I128Sum, I32Sum, ValueSum, MAX_MONEY}, sapling::{ fees, Authorization, Authorized, Bundle, ConvertDescription, GrothProofBytes, OutputDescription, SpendDescription, @@ -438,7 +438,7 @@ impl SaplingBuilder

{ } let allowed_amt: I32Sum = allowed.clone().into(); - self.value_balance += I128Sum::from(FromNt(allowed_amt)) * (value as i128); + self.value_balance += I128Sum::from_sum(allowed_amt) * (value as i128); self.converts.push(ConvertDescriptionInfo { allowed,