Decimal/Decimal256: Refactor checked_from_ratio implementation to use its own error type

Author: ueco-jb

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;

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("Multiplication overflow")]
+    MulOverflow,
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

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::{

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, StdError};
 use crate::OverflowError;
 
 use super::Fraction;
@@ -120,28 +120,31 @@ impl Decimal {
     pub fn from_ratio(numerator: impl Into<Uint128>, denominator: impl Into<Uint128>) -> 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::MulOverflow) => panic!("Multiplication overflow"),
         }
     }
 
     /// Returns the ratio (numerator / denominator) as a Decimal
     pub fn checked_from_ratio(
         numerator: impl Into<Uint128>,
         denominator: impl Into<Uint128>,
-    ) -> Result<Self, CheckedMultiplyRatioError> {
+    ) -> Result<Self, CheckedFromRatioError> {
         let numerator: Uint128 = numerator.into();
         let denominator: Uint128 = denominator.into();
         if denominator.is_zero() {
-            return Err(CheckedMultiplyRatioError::DivideByZero);
+            return Err(CheckedFromRatioError::DivideByZero);
         }
 
-        Ok(Decimal(
-            // numerator * DECIMAL_FRACTIONAL / denominator
-            numerator.checked_multiply_ratio(Self::DECIMAL_FRACTIONAL, denominator)?,
-        ))
+        match numerator.checked_multiply_ratio(Self::DECIMAL_FRACTIONAL, denominator) {
+            Ok(ratio) => {
+                // numerator * DECIMAL_FRACTIONAL / denominator
+                Ok(Decimal(ratio))
+            }
+            Err(_) => Err(CheckedFromRatioError::MulOverflow),
+        }
     }
 
     pub const fn is_zero(&self) -> bool {
@@ -672,16 +675,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::MulOverflow)
         );
     }
 

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, StdError};
 use crate::{OverflowError, Uint512};
 
 use super::Fraction;
@@ -132,28 +132,31 @@ impl Decimal256 {
     pub fn from_ratio(numerator: impl Into<Uint256>, denominator: impl Into<Uint256>) -> 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::MulOverflow) => panic!("Multiplication overflow"),
         }
     }
 
     /// Returns the ratio (numerator / denominator) as a Decimal256
     pub fn checked_from_ratio(
         numerator: impl Into<Uint256>,
         denominator: impl Into<Uint256>,
-    ) -> Result<Self, CheckedMultiplyRatioError> {
+    ) -> Result<Self, CheckedFromRatioError> {
         let numerator: Uint256 = numerator.into();
         let denominator: Uint256 = denominator.into();
         if denominator.is_zero() {
-            return Err(CheckedMultiplyRatioError::DivideByZero);
+            return Err(CheckedFromRatioError::DivideByZero);
         }
 
-        Ok(Self(
-            // numerator * DECIMAL_FRACTIONAL / denominator
-            numerator.checked_multiply_ratio(Self::DECIMAL_FRACTIONAL, denominator)?,
-        ))
+        match numerator.checked_multiply_ratio(Self::DECIMAL_FRACTIONAL, denominator) {
+            Ok(ratio) => {
+                // numerator * DECIMAL_FRACTIONAL / denominator
+                Ok(Self(ratio))
+            }
+            Err(_) => Err(CheckedFromRatioError::MulOverflow),
+        }
     }
 
     pub const fn is_zero(&self) -> bool {
@@ -704,16 +707,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::MulOverflow)
         );
     }