From e6044475bec82bd79bfa7c44e6b292036fbb7203 Mon Sep 17 00:00:00 2001 From: Jakub Bogucki Date: Tue, 26 Apr 2022 14:25:26 +0200 Subject: [PATCH 1/2] Decimal/Decimal256: Implement checked_from_ratio --- CHANGELOG.md | 1 + packages/std/src/math/decimal.rs | 37 +++++++++++++++++++++++++---- packages/std/src/math/decimal256.rs | 37 +++++++++++++++++++++++++---- 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 863b428b5a..aeec2f6831 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to - cosmwasm-std: Implement `checked_multiply_ratio` for `Uint64`/`Uint128`/`Uint256` +- cosmwasm-std: Implement `checked_from_ratio` for `Decimal`/`Decimal256` ### Changed diff --git a/packages/std/src/math/decimal.rs b/packages/std/src/math/decimal.rs index 92d260ba45..f31708a6d2 100644 --- a/packages/std/src/math/decimal.rs +++ b/packages/std/src/math/decimal.rs @@ -7,7 +7,7 @@ use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign}; use std::str::FromStr; use thiserror::Error; -use crate::errors::StdError; +use crate::errors::{CheckedMultiplyRatioError, StdError}; use crate::OverflowError; use super::Fraction; @@ -118,16 +118,30 @@ impl Decimal { /// Returns the ratio (numerator / denominator) as a Decimal pub fn from_ratio(numerator: impl Into, denominator: impl Into) -> Self { + match Decimal::checked_from_ratio(numerator, denominator) { + Ok(value) => value, + Err(CheckedMultiplyRatioError::DivideByZero) => { + panic!("Denominator must not be zero") + } + Err(CheckedMultiplyRatioError::Overflow) => panic!("Multiplication overflow"), + } + } + + /// Returns the ratio (numerator / denominator) as a Decimal + pub fn checked_from_ratio( + numerator: impl Into, + denominator: impl Into, + ) -> Result { let numerator: Uint128 = numerator.into(); let denominator: Uint128 = denominator.into(); if denominator.is_zero() { - panic!("Denominator must not be zero"); + return Err(CheckedMultiplyRatioError::DivideByZero); } - Decimal( + Ok(Decimal( // numerator * DECIMAL_FRACTIONAL / denominator - numerator.multiply_ratio(Self::DECIMAL_FRACTIONAL, denominator), - ) + numerator.checked_multiply_ratio(Self::DECIMAL_FRACTIONAL, denominator)?, + )) } pub const fn is_zero(&self) -> bool { @@ -658,6 +672,19 @@ mod tests { Decimal::from_ratio(1u128, 0u128); } + #[test] + fn decimal_checked_from_ratio_does_not_panic() { + assert_eq!( + Decimal::checked_from_ratio(1u128, 0u128), + Err(CheckedMultiplyRatioError::DivideByZero) + ); + + assert_eq!( + Decimal::checked_from_ratio(u128::MAX, 1u128), + Err(CheckedMultiplyRatioError::Overflow) + ); + } + #[test] fn decimal_implements_fraction() { let fraction = Decimal::from_str("1234.567").unwrap(); diff --git a/packages/std/src/math/decimal256.rs b/packages/std/src/math/decimal256.rs index 4dc1ef01ff..6ee0a7a4b2 100644 --- a/packages/std/src/math/decimal256.rs +++ b/packages/std/src/math/decimal256.rs @@ -7,7 +7,7 @@ use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign}; use std::str::FromStr; use thiserror::Error; -use crate::errors::StdError; +use crate::errors::{CheckedMultiplyRatioError, StdError}; use crate::{OverflowError, Uint512}; use super::Fraction; @@ -130,16 +130,30 @@ impl Decimal256 { /// Returns the ratio (numerator / denominator) as a Decimal256 pub fn from_ratio(numerator: impl Into, denominator: impl Into) -> Self { + match Decimal256::checked_from_ratio(numerator, denominator) { + Ok(value) => value, + Err(CheckedMultiplyRatioError::DivideByZero) => { + panic!("Denominator must not be zero") + } + Err(CheckedMultiplyRatioError::Overflow) => panic!("Multiplication overflow"), + } + } + + /// Returns the ratio (numerator / denominator) as a Decimal256 + pub fn checked_from_ratio( + numerator: impl Into, + denominator: impl Into, + ) -> Result { let numerator: Uint256 = numerator.into(); let denominator: Uint256 = denominator.into(); if denominator.is_zero() { - panic!("Denominator must not be zero"); + return Err(CheckedMultiplyRatioError::DivideByZero); } - Self( + Ok(Self( // numerator * DECIMAL_FRACTIONAL / denominator - numerator.multiply_ratio(Self::DECIMAL_FRACTIONAL, denominator), - ) + numerator.checked_multiply_ratio(Self::DECIMAL_FRACTIONAL, denominator)?, + )) } pub const fn is_zero(&self) -> bool { @@ -690,6 +704,19 @@ mod tests { Decimal256::from_ratio(1u128, 0u128); } + #[test] + fn decimal256_checked_from_ratio_does_not_panic() { + assert_eq!( + Decimal256::checked_from_ratio(1u128, 0u128), + Err(CheckedMultiplyRatioError::DivideByZero) + ); + + assert_eq!( + Decimal256::checked_from_ratio(Uint256::MAX, 1u128), + Err(CheckedMultiplyRatioError::Overflow) + ); + } + #[test] fn decimal256_implements_fraction() { let fraction = Decimal256::from_str("1234.567").unwrap(); From de8b8683ec03e762de41d0d11efdb4ec7014d6ab Mon Sep 17 00:00:00 2001 From: Jakub Bogucki Date: Tue, 26 Apr 2022 15:53:17 +0200 Subject: [PATCH 2/2] Decimal/Decimal256: Refactor checked_from_ratio implementation to use its own error type --- packages/std/src/errors/mod.rs | 4 ++-- packages/std/src/errors/std_error.rs | 9 ++++++++ packages/std/src/lib.rs | 5 ++-- packages/std/src/math/decimal.rs | 34 +++++++++++++++++----------- packages/std/src/math/decimal256.rs | 34 +++++++++++++++++----------- 5 files changed, 56 insertions(+), 30 deletions(-) diff --git a/packages/std/src/errors/mod.rs b/packages/std/src/errors/mod.rs index 6af5c0d09a..64b1e96f92 100644 --- a/packages/std/src/errors/mod.rs +++ b/packages/std/src/errors/mod.rs @@ -5,8 +5,8 @@ mod verification_error; pub use recover_pubkey_error::RecoverPubkeyError; pub use std_error::{ - CheckedMultiplyRatioError, ConversionOverflowError, DivideByZeroError, OverflowError, - OverflowOperation, StdError, StdResult, + CheckedFromRatioError, CheckedMultiplyRatioError, ConversionOverflowError, DivideByZeroError, + OverflowError, OverflowOperation, StdError, StdResult, }; pub use system_error::SystemError; pub use verification_error::VerificationError; diff --git a/packages/std/src/errors/std_error.rs b/packages/std/src/errors/std_error.rs index fabc7a2ef0..165ba20fb6 100644 --- a/packages/std/src/errors/std_error.rs +++ b/packages/std/src/errors/std_error.rs @@ -535,6 +535,15 @@ pub enum CheckedMultiplyRatioError { Overflow, } +#[derive(Error, Debug, PartialEq, Eq)] +pub enum CheckedFromRatioError { + #[error("Denominator must not be zero")] + DivideByZero, + + #[error("Overflow")] + Overflow, +} + #[cfg(test)] mod tests { use super::*; diff --git a/packages/std/src/lib.rs b/packages/std/src/lib.rs index 2e3d4c1fe5..10eda460e0 100644 --- a/packages/std/src/lib.rs +++ b/packages/std/src/lib.rs @@ -28,8 +28,9 @@ pub use crate::binary::Binary; pub use crate::coins::{coin, coins, has_coins, Coin}; pub use crate::deps::{Deps, DepsMut, OwnedDeps}; pub use crate::errors::{ - CheckedMultiplyRatioError, ConversionOverflowError, DivideByZeroError, OverflowError, - OverflowOperation, RecoverPubkeyError, StdError, StdResult, SystemError, VerificationError, + CheckedFromRatioError, CheckedMultiplyRatioError, ConversionOverflowError, DivideByZeroError, + OverflowError, OverflowOperation, RecoverPubkeyError, StdError, StdResult, SystemError, + VerificationError, }; #[cfg(feature = "stargate")] pub use crate::ibc::{ diff --git a/packages/std/src/math/decimal.rs b/packages/std/src/math/decimal.rs index f31708a6d2..5b15a9c31a 100644 --- a/packages/std/src/math/decimal.rs +++ b/packages/std/src/math/decimal.rs @@ -7,7 +7,7 @@ use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign}; use std::str::FromStr; use thiserror::Error; -use crate::errors::{CheckedMultiplyRatioError, StdError}; +use crate::errors::{CheckedFromRatioError, CheckedMultiplyRatioError, StdError}; use crate::OverflowError; use super::Fraction; @@ -120,10 +120,10 @@ impl Decimal { pub fn from_ratio(numerator: impl Into, denominator: impl Into) -> Self { match Decimal::checked_from_ratio(numerator, denominator) { Ok(value) => value, - Err(CheckedMultiplyRatioError::DivideByZero) => { + Err(CheckedFromRatioError::DivideByZero) => { panic!("Denominator must not be zero") } - Err(CheckedMultiplyRatioError::Overflow) => panic!("Multiplication overflow"), + Err(CheckedFromRatioError::Overflow) => panic!("Multiplication overflow"), } } @@ -131,17 +131,19 @@ impl Decimal { pub fn checked_from_ratio( numerator: impl Into, denominator: impl Into, - ) -> Result { + ) -> Result { let numerator: Uint128 = numerator.into(); let denominator: Uint128 = denominator.into(); - if denominator.is_zero() { - return Err(CheckedMultiplyRatioError::DivideByZero); + match numerator.checked_multiply_ratio(Self::DECIMAL_FRACTIONAL, denominator) { + Ok(ratio) => { + // numerator * DECIMAL_FRACTIONAL / denominator + Ok(Decimal(ratio)) + } + Err(CheckedMultiplyRatioError::Overflow) => Err(CheckedFromRatioError::Overflow), + Err(CheckedMultiplyRatioError::DivideByZero) => { + Err(CheckedFromRatioError::DivideByZero) + } } - - Ok(Decimal( - // numerator * DECIMAL_FRACTIONAL / denominator - numerator.checked_multiply_ratio(Self::DECIMAL_FRACTIONAL, denominator)?, - )) } pub const fn is_zero(&self) -> bool { @@ -672,16 +674,22 @@ mod tests { Decimal::from_ratio(1u128, 0u128); } + #[test] + #[should_panic(expected = "Multiplication overflow")] + fn decimal_from_ratio_panics_for_mul_overflow() { + Decimal::from_ratio(u128::MAX, 1u128); + } + #[test] fn decimal_checked_from_ratio_does_not_panic() { assert_eq!( Decimal::checked_from_ratio(1u128, 0u128), - Err(CheckedMultiplyRatioError::DivideByZero) + Err(CheckedFromRatioError::DivideByZero) ); assert_eq!( Decimal::checked_from_ratio(u128::MAX, 1u128), - Err(CheckedMultiplyRatioError::Overflow) + Err(CheckedFromRatioError::Overflow) ); } diff --git a/packages/std/src/math/decimal256.rs b/packages/std/src/math/decimal256.rs index 6ee0a7a4b2..a7898203ba 100644 --- a/packages/std/src/math/decimal256.rs +++ b/packages/std/src/math/decimal256.rs @@ -7,7 +7,7 @@ use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign}; use std::str::FromStr; use thiserror::Error; -use crate::errors::{CheckedMultiplyRatioError, StdError}; +use crate::errors::{CheckedFromRatioError, CheckedMultiplyRatioError, StdError}; use crate::{OverflowError, Uint512}; use super::Fraction; @@ -132,10 +132,10 @@ impl Decimal256 { pub fn from_ratio(numerator: impl Into, denominator: impl Into) -> Self { match Decimal256::checked_from_ratio(numerator, denominator) { Ok(value) => value, - Err(CheckedMultiplyRatioError::DivideByZero) => { + Err(CheckedFromRatioError::DivideByZero) => { panic!("Denominator must not be zero") } - Err(CheckedMultiplyRatioError::Overflow) => panic!("Multiplication overflow"), + Err(CheckedFromRatioError::Overflow) => panic!("Multiplication overflow"), } } @@ -143,17 +143,19 @@ impl Decimal256 { pub fn checked_from_ratio( numerator: impl Into, denominator: impl Into, - ) -> Result { + ) -> Result { let numerator: Uint256 = numerator.into(); let denominator: Uint256 = denominator.into(); - if denominator.is_zero() { - return Err(CheckedMultiplyRatioError::DivideByZero); + match numerator.checked_multiply_ratio(Self::DECIMAL_FRACTIONAL, denominator) { + Ok(ratio) => { + // numerator * DECIMAL_FRACTIONAL / denominator + Ok(Self(ratio)) + } + Err(CheckedMultiplyRatioError::Overflow) => Err(CheckedFromRatioError::Overflow), + Err(CheckedMultiplyRatioError::DivideByZero) => { + Err(CheckedFromRatioError::DivideByZero) + } } - - Ok(Self( - // numerator * DECIMAL_FRACTIONAL / denominator - numerator.checked_multiply_ratio(Self::DECIMAL_FRACTIONAL, denominator)?, - )) } pub const fn is_zero(&self) -> bool { @@ -704,16 +706,22 @@ mod tests { Decimal256::from_ratio(1u128, 0u128); } + #[test] + #[should_panic(expected = "Multiplication overflow")] + fn decimal256_from_ratio_panics_for_mul_overflow() { + Decimal256::from_ratio(Uint256::MAX, 1u128); + } + #[test] fn decimal256_checked_from_ratio_does_not_panic() { assert_eq!( Decimal256::checked_from_ratio(1u128, 0u128), - Err(CheckedMultiplyRatioError::DivideByZero) + Err(CheckedFromRatioError::DivideByZero) ); assert_eq!( Decimal256::checked_from_ratio(Uint256::MAX, 1u128), - Err(CheckedMultiplyRatioError::Overflow) + Err(CheckedFromRatioError::Overflow) ); }