From 3f8d33b9465eeb9756d7f9d20a72cc93e908b60d Mon Sep 17 00:00:00 2001 From: Murisi Tarusenga Date: Thu, 16 May 2024 14:41:55 +0200 Subject: [PATCH 1/2] Use the decoupled num-traits. --- Cargo.lock | 23 +++++--- masp_primitives/Cargo.toml | 1 + .../src/transaction/components/amount.rs | 52 +++++++++++-------- 3 files changed, 46 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eb6072d4..a295ddcc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -436,7 +436,7 @@ dependencies = [ "criterion-plot", "itertools 0.10.5", "lazy_static", - "num-traits", + "num-traits 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)", "oorandom", "plotters", "rayon", @@ -617,7 +617,7 @@ dependencies = [ "libm", "num-bigint", "num-integer", - "num-traits", + "num-traits 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -920,7 +920,8 @@ dependencies = [ "masp_note_encryption", "memuse", "nonempty", - "num-traits", + "num-traits 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)", + "num-traits 0.2.19 (git+https://github.com/heliaxdev/num-traits?rev=e3e712fc4ecbf9d95399b0b98ee9f3d6b9973e38)", "proptest", "rand", "rand_core", @@ -1021,7 +1022,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c165a9ab64cf766f73521c0dd2cfdff64f488b8f0b3e621face3462d3db536d7" dependencies = [ "num-integer", - "num-traits", + "num-traits 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -1040,7 +1041,7 @@ version = "0.1.46" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7969661fd2958a5cb096e56c8e1ad0444ac2bbcd0061bd28660485a44879858f" dependencies = [ - "num-traits", + "num-traits 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -1053,6 +1054,14 @@ dependencies = [ "libm", ] +[[package]] +name = "num-traits" +version = "0.2.19" +source = "git+https://github.com/heliaxdev/num-traits?rev=e3e712fc4ecbf9d95399b0b98ee9f3d6b9973e38#e3e712fc4ecbf9d95399b0b98ee9f3d6b9973e38" +dependencies = [ + "autocfg", +] + [[package]] name = "num_cpus" version = "1.16.0" @@ -1174,7 +1183,7 @@ version = "0.3.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d2c224ba00d7cadd4d5c660deaf2098e5e80e07846537c51f9cfa4be50c1fd45" dependencies = [ - "num-traits", + "num-traits 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)", "plotters-backend", "plotters-svg", "wasm-bindgen", @@ -1286,7 +1295,7 @@ dependencies = [ "bit-vec", "bitflags 2.5.0", "lazy_static", - "num-traits", + "num-traits 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)", "rand", "rand_chacha", "rand_xorshift", diff --git a/masp_primitives/Cargo.toml b/masp_primitives/Cargo.toml index e78dbf41..186e380a 100644 --- a/masp_primitives/Cargo.toml +++ b/masp_primitives/Cargo.toml @@ -38,6 +38,7 @@ memuse = "0.2.1" # - Checked arithmetic num-traits = "0.2.14" +num-traits-decoupled = { package = "num-traits", version = "0.2.19", git = "https://github.com/heliaxdev/num-traits", rev = "e3e712fc4ecbf9d95399b0b98ee9f3d6b9973e38" } # - Secret management subtle = "2.2.3" diff --git a/masp_primitives/src/transaction/components/amount.rs b/masp_primitives/src/transaction/components/amount.rs index cf9888ae..f35daf92 100644 --- a/masp_primitives/src/transaction/components/amount.rs +++ b/masp_primitives/src/transaction/components/amount.rs @@ -4,7 +4,7 @@ use borsh::schema::Fields; use borsh::schema::{Declaration, Definition}; use borsh::BorshSchema; use borsh::{BorshDeserialize, BorshSerialize}; -use num_traits::{CheckedAdd, CheckedMul, CheckedNeg, CheckedSub, One}; +use num_traits_decoupled::{CheckedAdd, CheckedMul, CheckedNeg, CheckedSub, One}; use std::cmp::Ordering; use std::collections::btree_map::Keys; use std::collections::btree_map::{IntoIter, Iter}; @@ -422,7 +422,7 @@ where + Copy + Default + PartialOrd - + CheckedMul, + + CheckedMul, { fn mul_assign(&mut self, rhs: Value) { *self = self.clone() * rhs; @@ -440,18 +440,19 @@ where + Default + PartialOrd + CheckedMul, + ::Output : Default + BorshSerialize + BorshDeserialize + Eq, { - type Output = ValueSum; + type Output = ValueSum::Output>; fn mul(self, rhs: Value) -> 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"), + amount.checked_mul(rhs).expect("overflow detected"), ); } - comps.retain(|_, v| *v != Value::default()); + comps.retain(|_, v| *v != ::Output::default()); ValueSum(comps) } } @@ -466,7 +467,7 @@ where + Copy + Default + PartialOrd - + CheckedAdd, + + CheckedAdd, { fn add_assign(&mut self, rhs: &ValueSum) { *self = self.clone() + rhs; @@ -483,7 +484,7 @@ where + Copy + Default + PartialOrd - + CheckedAdd, + + CheckedAdd, { fn add_assign(&mut self, rhs: ValueSum) { *self += &rhs @@ -500,7 +501,7 @@ where + Copy + Default + PartialOrd - + CheckedAdd, + + CheckedAdd, { type Output = ValueSum; @@ -519,7 +520,7 @@ where + Copy + Default + PartialOrd - + CheckedAdd, + + CheckedAdd, { type Output = ValueSum; @@ -528,7 +529,7 @@ where } } -impl CheckedAdd for ValueSum +impl CheckedAdd for &ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, Value: BorshSerialize @@ -538,12 +539,14 @@ where + Copy + Default + PartialOrd - + CheckedAdd, + + CheckedAdd, { - fn checked_add(&self, v: &Self) -> Option { + type Output = ValueSum; + + fn checked_add(self, v: Self) -> Option { let mut comps = self.0.clone(); for (atype, amount) in v.components() { - comps.insert(atype.clone(), self.get(atype).checked_add(amount)?); + comps.insert(atype.clone(), self.get(atype).checked_add(*amount)?); } comps.retain(|_, v| *v != Value::default()); Some(ValueSum(comps)) @@ -560,7 +563,7 @@ where + Copy + Default + PartialOrd - + CheckedSub, + + CheckedSub, { fn sub_assign(&mut self, rhs: &ValueSum) { *self = self.clone() - rhs @@ -577,7 +580,7 @@ where + Copy + Default + PartialOrd - + CheckedSub, + + CheckedSub, { fn sub_assign(&mut self, rhs: ValueSum) { *self -= &rhs @@ -595,8 +598,9 @@ where + Default + PartialOrd + CheckedNeg, + ::Output: BorshSerialize + BorshDeserialize + Eq + Default, { - type Output = ValueSum; + type Output = ValueSum::Output>; fn neg(mut self) -> Self::Output { let mut comps = BTreeMap::new(); @@ -606,7 +610,7 @@ where amount.checked_neg().expect("overflow detected"), ); } - comps.retain(|_, v| *v != Value::default()); + comps.retain(|_, v| *v != ::Output::default()); ValueSum(comps) } } @@ -614,7 +618,7 @@ where impl Sub<&ValueSum> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, { type Output = ValueSum; @@ -626,7 +630,7 @@ where impl Sub> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, { type Output = ValueSum; @@ -635,15 +639,17 @@ where } } -impl CheckedSub for ValueSum +impl CheckedSub for &ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, + Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, { - fn checked_sub(&self, v: &Self) -> Option { + type Output = ValueSum; + + fn checked_sub(self, v: Self) -> Option { let mut comps = self.0.clone(); for (atype, amount) in v.components() { - comps.insert(atype.clone(), self.get(atype).checked_sub(amount)?); + comps.insert(atype.clone(), self.get(atype).checked_sub(*amount)?); } comps.retain(|_, v| *v != Value::default()); Some(ValueSum(comps)) From c62d4fe0eecfde013225b8ece944542a8fd8f407 Mon Sep 17 00:00:00 2001 From: Murisi Tarusenga Date: Thu, 16 May 2024 17:34:52 +0200 Subject: [PATCH 2/2] Made the ValueSum trait implementations more generic. --- Cargo.lock | 5 +- masp_primitives/Cargo.toml | 3 +- masp_primitives/src/lib.rs | 1 + .../src/transaction/components/amount.rs | 171 +++++++++--------- .../transaction/components/sapling/builder.rs | 4 +- 5 files changed, 87 insertions(+), 97 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a295ddcc..6fab48aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -920,8 +920,7 @@ dependencies = [ "masp_note_encryption", "memuse", "nonempty", - "num-traits 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)", - "num-traits 0.2.19 (git+https://github.com/heliaxdev/num-traits?rev=e3e712fc4ecbf9d95399b0b98ee9f3d6b9973e38)", + "num-traits 0.2.19 (git+https://github.com/heliaxdev/num-traits?rev=3f3657caa34b8e116fdf3f8a3519c4ac29f012fe)", "proptest", "rand", "rand_core", @@ -1057,7 +1056,7 @@ dependencies = [ [[package]] name = "num-traits" version = "0.2.19" -source = "git+https://github.com/heliaxdev/num-traits?rev=e3e712fc4ecbf9d95399b0b98ee9f3d6b9973e38#e3e712fc4ecbf9d95399b0b98ee9f3d6b9973e38" +source = "git+https://github.com/heliaxdev/num-traits?rev=3f3657caa34b8e116fdf3f8a3519c4ac29f012fe#3f3657caa34b8e116fdf3f8a3519c4ac29f012fe" dependencies = [ "autocfg", ] diff --git a/masp_primitives/Cargo.toml b/masp_primitives/Cargo.toml index 186e380a..1c696803 100644 --- a/masp_primitives/Cargo.toml +++ b/masp_primitives/Cargo.toml @@ -37,8 +37,7 @@ sha2 = "0.10" memuse = "0.2.1" # - Checked arithmetic -num-traits = "0.2.14" -num-traits-decoupled = { package = "num-traits", version = "0.2.19", git = "https://github.com/heliaxdev/num-traits", rev = "e3e712fc4ecbf9d95399b0b98ee9f3d6b9973e38" } +num-traits = { version = "0.2.19", git = "https://github.com/heliaxdev/num-traits", rev = "3f3657caa34b8e116fdf3f8a3519c4ac29f012fe" } # - Secret management subtle = "2.2.3" diff --git a/masp_primitives/src/lib.rs b/masp_primitives/src/lib.rs index 308e9323..07765909 100644 --- a/masp_primitives/src/lib.rs +++ b/masp_primitives/src/lib.rs @@ -30,6 +30,7 @@ pub use bls12_381; pub use ff; pub use group; pub use jubjub; +pub use num_traits; #[cfg(test)] mod test_vectors; diff --git a/masp_primitives/src/transaction/components/amount.rs b/masp_primitives/src/transaction/components/amount.rs index f35daf92..3ed1141d 100644 --- a/masp_primitives/src/transaction/components/amount.rs +++ b/masp_primitives/src/transaction/components/amount.rs @@ -4,7 +4,7 @@ use borsh::schema::Fields; use borsh::schema::{Declaration, Definition}; use borsh::BorshSchema; use borsh::{BorshDeserialize, BorshSerialize}; -use num_traits_decoupled::{CheckedAdd, CheckedMul, CheckedNeg, CheckedSub, One}; +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}; @@ -412,39 +412,33 @@ impl_index!(i128); impl_index!(u128); -impl MulAssign for ValueSum +impl MulAssign for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Value: BorshSerialize + Lhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default - + PartialOrd - + CheckedMul, + + CheckedMul, + Rhs: Copy, { - fn mul_assign(&mut self, rhs: Value) { + fn mul_assign(&mut self, rhs: Rhs) { *self = self.clone() * rhs; } } -impl Mul for ValueSum +impl Mul for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Value: BorshSerialize - + BorshDeserialize - + PartialEq - + Eq - + Copy - + Default - + PartialOrd - + CheckedMul, - ::Output : Default + BorshSerialize + BorshDeserialize + Eq, + Lhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedMul, + Rhs: Copy, + >::Output: Default + BorshSerialize + BorshDeserialize + Eq, { - type Output = ValueSum::Output>; + type Output = ValueSum>::Output>; - fn mul(self, rhs: Value) -> Self::Output { + fn mul(self, rhs: Rhs) -> Self::Output { let mut comps = BTreeMap::new(); for (atype, amount) in self.0.iter() { comps.insert( @@ -452,137 +446,125 @@ where amount.checked_mul(rhs).expect("overflow detected"), ); } - comps.retain(|_, v| *v != ::Output::default()); + comps.retain(|_, v| *v != >::Output::default()); ValueSum(comps) } } -impl AddAssign<&ValueSum> for ValueSum +impl AddAssign<&ValueSum> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Value: BorshSerialize + Lhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default - + PartialOrd - + CheckedAdd, + + CheckedAdd, + Rhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, { - 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, - Value: BorshSerialize + Lhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default - + PartialOrd - + CheckedAdd, + + CheckedAdd, + Rhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, { - 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, - Value: BorshSerialize - + BorshDeserialize - + PartialEq - + Eq - + Copy - + Default - + PartialOrd - + CheckedAdd, + Lhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedAdd, + Rhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, + >::Output: BorshSerialize + BorshDeserialize + Eq + Default, { - type Output = ValueSum; + type Output = ValueSum>::Output>; - fn add(self, rhs: &ValueSum) -> Self::Output { + fn add(self, rhs: &ValueSum) -> Self::Output { self.checked_add(rhs).expect("overflow detected") } } -impl Add> for ValueSum +impl Add> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Value: BorshSerialize - + BorshDeserialize - + PartialEq - + Eq - + Copy - + Default - + PartialOrd - + CheckedAdd, + Lhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedAdd, + Rhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, + >::Output: BorshSerialize + BorshDeserialize + Eq + Default, { - type Output = ValueSum; + type Output = ValueSum>::Output>; - fn add(self, rhs: ValueSum) -> Self::Output { + fn add(self, rhs: ValueSum) -> Self::Output { self + &rhs } } -impl CheckedAdd for &ValueSum +impl CheckedAdd<&ValueSum> for &ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Value: BorshSerialize - + BorshDeserialize - + PartialEq - + Eq - + Copy - + Default - + PartialOrd - + CheckedAdd, + Lhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedAdd, + Rhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, + >::Output: BorshSerialize + BorshDeserialize + Eq + Default, { - type Output = ValueSum; - - fn checked_add(self, v: Self) -> Option { - let mut comps = self.0.clone(); + type Output = ValueSum>::Output>; + + fn checked_add(self, v: &ValueSum) -> Option { + let mut comps = BTreeMap::new(); + for (atype, amount) in self.components() { + comps.insert(atype.clone(), amount.checked_add(Rhs::default())?); + } for (atype, amount) in v.components() { comps.insert(atype.clone(), self.get(atype).checked_add(*amount)?); } - comps.retain(|_, v| *v != Value::default()); + comps.retain(|_, v| *v != >::Output::default()); Some(ValueSum(comps)) } } -impl SubAssign<&ValueSum> for ValueSum +impl SubAssign<&ValueSum> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Value: BorshSerialize + Lhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default - + PartialOrd - + CheckedSub, + + CheckedSub, + Rhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, { - 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, - Value: BorshSerialize + Lhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default - + PartialOrd - + CheckedSub, + + CheckedSub, + Rhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, { - fn sub_assign(&mut self, rhs: ValueSum) { + fn sub_assign(&mut self, rhs: ValueSum) { *self -= &rhs } } @@ -615,43 +597,52 @@ where } } -impl Sub<&ValueSum> for ValueSum +impl Sub<&ValueSum> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, + Lhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, + Rhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, + >::Output: BorshSerialize + BorshDeserialize + Eq + Default, { - type Output = ValueSum; + type Output = ValueSum>::Output>; - fn sub(self, rhs: &ValueSum) -> Self::Output { + fn sub(self, rhs: &ValueSum) -> Self::Output { self.checked_sub(rhs).expect("underflow detected") } } -impl Sub> for ValueSum +impl Sub> for ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, + Lhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, + Rhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, + >::Output: BorshSerialize + BorshDeserialize + Eq + Default, { - type Output = ValueSum; + type Output = ValueSum>::Output>; - fn sub(self, rhs: ValueSum) -> Self::Output { + fn sub(self, rhs: ValueSum) -> Self::Output { self - &rhs } } -impl CheckedSub for &ValueSum +impl CheckedSub<&ValueSum> for &ValueSum where Unit: Hash + Ord + BorshSerialize + BorshDeserialize + Clone, - Value: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, + Lhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default + CheckedSub, + Rhs: BorshSerialize + BorshDeserialize + PartialEq + Eq + Copy + Default, + >::Output: BorshSerialize + BorshDeserialize + Eq + Default, { - type Output = ValueSum; - - fn checked_sub(self, v: Self) -> Option { - let mut comps = self.0.clone(); + type Output = ValueSum>::Output>; + + fn checked_sub(self, v: &ValueSum) -> Option { + let mut comps = BTreeMap::new(); + for (atype, amount) in self.components() { + comps.insert(atype.clone(), amount.checked_sub(Rhs::default())?); + } for (atype, amount) in v.components() { comps.insert(atype.clone(), self.get(atype).checked_sub(*amount)?); } - comps.retain(|_, v| *v != Value::default()); + comps.retain(|_, v| *v != >::Output::default()); Some(ValueSum(comps)) } } diff --git a/masp_primitives/src/transaction/components/sapling/builder.rs b/masp_primitives/src/transaction/components/sapling/builder.rs index df4a2b03..eb150197 100644 --- a/masp_primitives/src/transaction/components/sapling/builder.rs +++ b/masp_primitives/src/transaction/components/sapling/builder.rs @@ -652,7 +652,7 @@ impl SaplingBuilder

{ self.spend_anchor = Some(merkle_path.root(node).into()) } - self.value_balance += ValueSum::from_pair(note.asset_type, note.value.into()); + self.value_balance += ValueSum::from_pair(note.asset_type, i128::from(note.value)); self.spends.push(SpendDescriptionInfo { extsk, @@ -710,7 +710,7 @@ impl SaplingBuilder

{ ) -> Result<(), Error> { let output = SaplingOutputInfo::new_internal(ovk, to, asset_type, value, memo)?; - self.value_balance -= ValueSum::from_pair(asset_type, value.into()); + self.value_balance -= ValueSum::from_pair(asset_type, i128::from(value)); self.outputs.push(output);