From 0ccda87bf8ee3937d3a0e194c4db4b67f3402e24 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Mon, 14 Mar 2022 16:08:13 +0100 Subject: [PATCH 1/2] Implement Sub/SubAssign for Uint64 and improve consistency across Uint* types --- CHANGELOG.md | 1 + packages/std/src/math/uint128.rs | 57 +++++++++++++++++--------- packages/std/src/math/uint256.rs | 66 ++++++++++++++++++++++--------- packages/std/src/math/uint512.rs | 68 ++++++++++++++++++++++---------- packages/std/src/math/uint64.rs | 61 +++++++++++++++++++++++++++- 5 files changed, 193 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 496a514e85..adfcc027dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to - cosmwasm-std: Implement `Decimal{,256}::checked_mul` and `Decimal{,256}::checked_pow`. +- cosmwasm-std: Implement `Sub`/`SubAssign` for `Uint64`. - cosmwasm-crypto: Upgrade ed25519-zebra to version 3. ## [1.0.0-beta6] - 2022-03-07 diff --git a/packages/std/src/math/uint128.rs b/packages/std/src/math/uint128.rs index f3b546188f..87b667459c 100644 --- a/packages/std/src/math/uint128.rs +++ b/packages/std/src/math/uint128.rs @@ -1,9 +1,9 @@ -use forward_ref::forward_ref_binop; +use forward_ref::{forward_ref_binop, forward_ref_op_assign}; use schemars::JsonSchema; use serde::{de, ser, Deserialize, Deserializer, Serialize}; use std::convert::{TryFrom, TryInto}; use std::fmt::{self}; -use std::ops::{self, Rem}; +use std::ops::{self, Rem, Sub, SubAssign}; use std::str::FromStr; use crate::errors::{DivideByZeroError, OverflowError, OverflowOperation, StdError}; @@ -266,14 +266,14 @@ impl ops::Sub for Uint128 { ) } } +forward_ref_binop!(impl Sub, sub for Uint128, Uint128); -impl<'a> ops::Sub<&'a Uint128> for Uint128 { - type Output = Self; - - fn sub(self, rhs: &'a Uint128) -> Self { - self - *rhs +impl SubAssign for Uint128 { + fn sub_assign(&mut self, rhs: Uint128) { + *self = *self - rhs; } } +forward_ref_op_assign!(impl SubAssign, sub_assign for Uint128, Uint128); impl ops::Mul for Uint128 { type Output = Self; @@ -347,18 +347,6 @@ impl<'a> ops::AddAssign<&'a Uint128> for Uint128 { } } -impl ops::SubAssign for Uint128 { - fn sub_assign(&mut self, rhs: Uint128) { - *self = *self - rhs; - } -} - -impl<'a> ops::SubAssign<&'a Uint128> for Uint128 { - fn sub_assign(&mut self, rhs: &'a Uint128) { - *self = *self - rhs; - } -} - impl ops::MulAssign for Uint128 { fn mul_assign(&mut self, rhs: Self) { *self = *self * rhs; @@ -672,12 +660,43 @@ mod tests { let _ = almost_max + Uint128(12); } + #[test] + #[allow(clippy::op_ref)] + fn uint128_sub_works() { + assert_eq!(Uint128(2) - Uint128(1), Uint128(1)); + assert_eq!(Uint128(2) - Uint128(0), Uint128(2)); + assert_eq!(Uint128(2) - Uint128(2), Uint128(0)); + + // works for refs + let a = Uint128::new(10); + let b = Uint128::new(3); + let expected = Uint128::new(7); + assert_eq!(a - b, expected); + assert_eq!(a - &b, expected); + assert_eq!(&a - b, expected); + assert_eq!(&a - &b, expected); + } + #[test] #[should_panic] fn uint128_sub_overflow_panics() { let _ = Uint128(1) - Uint128(2); } + #[test] + fn uint128_sub_assign_works() { + let mut a = Uint128(14); + a -= Uint128(2); + assert_eq!(a, Uint128(12)); + + // works for refs + let mut a = Uint128::new(10); + let b = Uint128::new(3); + let expected = Uint128::new(7); + a -= &b; + assert_eq!(a, expected); + } + #[test] fn uint128_multiply_ratio_works() { let base = Uint128(500); diff --git a/packages/std/src/math/uint256.rs b/packages/std/src/math/uint256.rs index 8d571a71ae..bb4675696d 100644 --- a/packages/std/src/math/uint256.rs +++ b/packages/std/src/math/uint256.rs @@ -1,9 +1,9 @@ -use forward_ref::forward_ref_binop; +use forward_ref::{forward_ref_binop, forward_ref_op_assign}; use schemars::JsonSchema; use serde::{de, ser, Deserialize, Deserializer, Serialize}; use std::convert::{TryFrom, TryInto}; use std::fmt; -use std::ops::{self, Rem, Shl, Shr}; +use std::ops::{self, Rem, Shl, Shr, Sub, SubAssign}; use std::str::FromStr; use crate::errors::{ @@ -405,14 +405,14 @@ impl ops::Sub for Uint256 { ) } } +forward_ref_binop!(impl Sub, sub for Uint256, Uint256); -impl<'a> ops::Sub<&'a Uint256> for Uint256 { - type Output = Self; - - fn sub(self, rhs: &'a Uint256) -> Self { - self - *rhs +impl SubAssign for Uint256 { + fn sub_assign(&mut self, rhs: Uint256) { + *self = *self - rhs; } } +forward_ref_op_assign!(impl SubAssign, sub_assign for Uint256, Uint256); impl ops::Div for Uint256 { type Output = Self; @@ -521,18 +521,6 @@ impl<'a> ops::AddAssign<&'a Uint256> for Uint256 { } } -impl ops::SubAssign for Uint256 { - fn sub_assign(&mut self, rhs: Uint256) { - *self = *self - rhs; - } -} - -impl<'a> ops::SubAssign<&'a Uint256> for Uint256 { - fn sub_assign(&mut self, rhs: &'a Uint256) { - *self = *self - rhs; - } -} - impl ops::MulAssign for Uint256 { fn mul_assign(&mut self, rhs: Self) { *self = *self * rhs; @@ -1221,12 +1209,52 @@ mod tests { let _ = max + Uint256::from(12u32); } + #[test] + #[allow(clippy::op_ref)] + fn uint256_sub_works() { + assert_eq!( + Uint256::from(2u32) - Uint256::from(1u32), + Uint256::from(1u32) + ); + assert_eq!( + Uint256::from(2u32) - Uint256::from(0u32), + Uint256::from(2u32) + ); + assert_eq!( + Uint256::from(2u32) - Uint256::from(2u32), + Uint256::from(0u32) + ); + + // works for refs + let a = Uint256::from(10u32); + let b = Uint256::from(3u32); + let expected = Uint256::from(7u32); + assert_eq!(a - b, expected); + assert_eq!(a - &b, expected); + assert_eq!(&a - b, expected); + assert_eq!(&a - &b, expected); + } + #[test] #[should_panic] fn uint256_sub_overflow_panics() { let _ = Uint256::from(1u32) - Uint256::from(2u32); } + #[test] + fn uint256_sub_assign_works() { + let mut a = Uint256::from(14u32); + a -= Uint256::from(2u32); + assert_eq!(a, Uint256::from(12u32)); + + // works for refs + let mut a = Uint256::from(10u32); + let b = Uint256::from(3u32); + let expected = Uint256::from(7u32); + a -= &b; + assert_eq!(a, expected); + } + #[test] fn uint256_multiply_ratio_works() { let base = Uint256::from(500u32); diff --git a/packages/std/src/math/uint512.rs b/packages/std/src/math/uint512.rs index a4fd76f206..7c57b8995a 100644 --- a/packages/std/src/math/uint512.rs +++ b/packages/std/src/math/uint512.rs @@ -1,9 +1,9 @@ -use forward_ref::forward_ref_binop; +use forward_ref::{forward_ref_binop, forward_ref_op_assign}; use schemars::JsonSchema; use serde::{de, ser, Deserialize, Deserializer, Serialize}; use std::convert::{TryFrom, TryInto}; use std::fmt; -use std::ops::{self, Rem, Shr}; +use std::ops::{self, Rem, Shr, Sub, SubAssign}; use std::str::FromStr; use crate::errors::{ @@ -493,21 +493,21 @@ impl<'a> ops::Add<&'a Uint512> for Uint512 { } } -impl ops::Sub for Uint512 { +impl Sub for Uint512 { type Output = Self; fn sub(self, rhs: Self) -> Self { Uint512(self.0.checked_sub(rhs.0).unwrap()) } } +forward_ref_binop!(impl Sub, sub for Uint512, Uint512); -impl<'a> ops::Sub<&'a Uint512> for Uint512 { - type Output = Self; - - fn sub(self, rhs: &'a Uint512) -> Self { - Uint512(self.0.checked_sub(rhs.0).unwrap()) +impl ops::SubAssign for Uint512 { + fn sub_assign(&mut self, rhs: Uint512) { + self.0 = self.0.checked_sub(rhs.0).unwrap(); } } +forward_ref_op_assign!(impl SubAssign, sub_assign for Uint512, Uint512); impl ops::Div for Uint512 { type Output = Self; @@ -587,18 +587,6 @@ impl<'a> ops::AddAssign<&'a Uint512> for Uint512 { } } -impl ops::SubAssign for Uint512 { - fn sub_assign(&mut self, rhs: Uint512) { - self.0 = self.0.checked_sub(rhs.0).unwrap(); - } -} - -impl<'a> ops::SubAssign<&'a Uint512> for Uint512 { - fn sub_assign(&mut self, rhs: &'a Uint512) { - self.0 = self.0.checked_sub(rhs.0).unwrap(); - } -} - impl ops::DivAssign for Uint512 { fn div_assign(&mut self, rhs: Self) { self.0 = self.0.checked_div(rhs.0).unwrap(); @@ -993,12 +981,52 @@ mod tests { let _ = max + Uint512::from(12u32); } + #[test] + #[allow(clippy::op_ref)] + fn uint512_sub_works() { + assert_eq!( + Uint512::from(2u32) - Uint512::from(1u32), + Uint512::from(1u32) + ); + assert_eq!( + Uint512::from(2u32) - Uint512::from(0u32), + Uint512::from(2u32) + ); + assert_eq!( + Uint512::from(2u32) - Uint512::from(2u32), + Uint512::from(0u32) + ); + + // works for refs + let a = Uint512::from(10u32); + let b = Uint512::from(3u32); + let expected = Uint512::from(7u32); + assert_eq!(a - b, expected); + assert_eq!(a - &b, expected); + assert_eq!(&a - b, expected); + assert_eq!(&a - &b, expected); + } + #[test] #[should_panic] fn uint512_sub_overflow_panics() { let _ = Uint512::from(1u32) - Uint512::from(2u32); } + #[test] + fn uint512_sub_assign_works() { + let mut a = Uint512::from(14u32); + a -= Uint512::from(2u32); + assert_eq!(a, Uint512::from(12u32)); + + // works for refs + let mut a = Uint512::from(10u32); + let b = Uint512::from(3u32); + let expected = Uint512::from(7u32); + a -= &b; + assert_eq!(a, expected); + } + #[test] fn uint512_shr_works() { let original = Uint512::new([ diff --git a/packages/std/src/math/uint64.rs b/packages/std/src/math/uint64.rs index d62c13f279..3c455c1ac2 100644 --- a/packages/std/src/math/uint64.rs +++ b/packages/std/src/math/uint64.rs @@ -1,9 +1,9 @@ -use forward_ref::forward_ref_binop; +use forward_ref::{forward_ref_binop, forward_ref_op_assign}; use schemars::JsonSchema; use serde::{de, ser, Deserialize, Deserializer, Serialize}; use std::convert::{TryFrom, TryInto}; use std::fmt::{self}; -use std::ops::{self, Rem}; +use std::ops::{self, Rem, Sub, SubAssign}; use crate::errors::{DivideByZeroError, OverflowError, OverflowOperation, StdError}; use crate::Uint128; @@ -210,6 +210,26 @@ impl<'a> ops::Add<&'a Uint64> for Uint64 { } } +impl ops::Sub for Uint64 { + type Output = Self; + + fn sub(self, rhs: Self) -> Self { + Uint64( + self.u64() + .checked_sub(rhs.u64()) + .expect("attempt to subtract with overflow"), + ) + } +} +forward_ref_binop!(impl Sub, sub for Uint64, Uint64); + +impl SubAssign for Uint64 { + fn sub_assign(&mut self, rhs: Uint64) { + *self = *self - rhs; + } +} +forward_ref_op_assign!(impl SubAssign, sub_assign for Uint64, Uint64); + impl ops::Div for Uint64 { type Output = Self; @@ -518,6 +538,43 @@ mod tests { assert_eq!((operand1, operand2), (a.to_string(), b.to_string())); } + #[test] + #[allow(clippy::op_ref)] + fn uint64_sub_works() { + assert_eq!(Uint64(2) - Uint64(1), Uint64(1)); + assert_eq!(Uint64(2) - Uint64(0), Uint64(2)); + assert_eq!(Uint64(2) - Uint64(2), Uint64(0)); + + // works for refs + let a = Uint64::new(10); + let b = Uint64::new(3); + let expected = Uint64::new(7); + assert_eq!(a - b, expected); + assert_eq!(a - &b, expected); + assert_eq!(&a - b, expected); + assert_eq!(&a - &b, expected); + } + + #[test] + #[should_panic] + fn uint64_sub_overflow_panics() { + let _ = Uint64(1) - Uint64(2); + } + + #[test] + fn uint64_sub_assign_works() { + let mut a = Uint64(14); + a -= Uint64(2); + assert_eq!(a, Uint64(12)); + + // works for refs + let mut a = Uint64::new(10); + let b = Uint64::new(3); + let expected = Uint64::new(7); + a -= &b; + assert_eq!(a, expected); + } + #[test] #[should_panic] fn uint64_math_overflow_panics() { From 4e4ccb2849dc6ed8b5bb22f9a3b5943c7b0bc9b2 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Mon, 14 Mar 2022 16:08:40 +0100 Subject: [PATCH 2/2] Create helper trait to ensure trait implementations --- packages/std/src/math/mod.rs | 42 ++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/packages/std/src/math/mod.rs b/packages/std/src/math/mod.rs index 3a7a4e801e..4c956b0377 100644 --- a/packages/std/src/math/mod.rs +++ b/packages/std/src/math/mod.rs @@ -15,3 +15,45 @@ pub use uint128::Uint128; pub use uint256::Uint256; pub use uint512::Uint512; pub use uint64::Uint64; + +#[cfg(test)] +mod tests { + use super::*; + use std::ops::*; + + /// An trait that ensures other traits are implemented for our number types + trait AllImpl<'a>: + Add + + Add<&'a Self> + + AddAssign + + AddAssign<&'a Self> + + Sub + + Sub<&'a Self> + + SubAssign + + SubAssign<&'a Self> + // Uncomment when implementing Mul/MulAssign for Uint64 + // + Mul + // + Mul<&'a Self> + // + MulAssign + // + MulAssign<&'a Self> + + Div + + Div<&'a Self> + + DivAssign + + DivAssign<&'a Self> + + Rem + + Rem<&'a Self> + // Uncomment when implementing RemAssign + // + RemAssign + // + RemAssign<&'a Self> + + Sized + + Copy + where + Self: 'a, + { + } + + impl AllImpl<'_> for Uint64 {} + impl AllImpl<'_> for Uint128 {} + impl AllImpl<'_> for Uint256 {} + impl AllImpl<'_> for Uint512 {} +}