Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2.0] Remove allocations from math errors #1896

Merged
merged 2 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,13 @@ and this project adheres to
- cosmwasm-std: `Coin::new` now takes `Into<Uint128>` instead of `u128` as the
first argument and `DecCoin::new` takes `Into<Decimal256>` instead of
`Decimal256`. ([#1902])
- cosmwasm-std: Remove operand strings from `OverflowError`,
`ConversionOverflowError` and `DivideByZeroError`. ([#1896])

[#1874]: https://github.com/CosmWasm/cosmwasm/pull/1874
[#1879]: https://github.com/CosmWasm/cosmwasm/pull/1879
[#1890]: https://github.com/CosmWasm/cosmwasm/pull/1890
[#1896]: https://github.com/CosmWasm/cosmwasm/pull/1896
[#1898]: https://github.com/CosmWasm/cosmwasm/pull/1898
[#1902]: https://github.com/CosmWasm/cosmwasm/pull/1902

Expand Down
7 changes: 1 addition & 6 deletions packages/std/src/coins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,7 @@ impl Coins {
if coin.amount.is_zero() {
return Ok(());
}
return Err(OverflowError::new(
OverflowOperation::Sub,
Uint128::zero(),
coin.amount,
)
.into());
return Err(OverflowError::new(OverflowOperation::Sub).into());
}
}

Expand Down
114 changes: 40 additions & 74 deletions packages/std/src/errors/std_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,24 +493,14 @@ impl fmt::Display for OverflowOperation {
}

#[derive(Error, Debug, PartialEq, Eq)]
#[error("Cannot {operation} with {operand1} and {operand2}")]
#[error("Cannot {operation} with given operands")]
pub struct OverflowError {
pub operation: OverflowOperation,
pub operand1: String,
pub operand2: String,
}

impl OverflowError {
pub fn new(
operation: OverflowOperation,
operand1: impl ToString,
operand2: impl ToString,
) -> Self {
Self {
operation,
operand1: operand1.to_string(),
operand2: operand2.to_string(),
}
pub fn new(operation: OverflowOperation) -> Self {
Self { operation }
}
}

Expand All @@ -521,38 +511,28 @@ impl OverflowError {
/// [`Uint256`]: crate::Uint256
/// [`Uint128`]: crate::Uint128
#[derive(Error, Debug, PartialEq, Eq)]
#[error("Error converting {source_type} to {target_type} for {value}")]
#[error("Error converting {source_type} to {target_type}")]
pub struct ConversionOverflowError {
pub source_type: &'static str,
pub target_type: &'static str,
pub value: String,
}

impl ConversionOverflowError {
pub fn new(
source_type: &'static str,
target_type: &'static str,
value: impl Into<String>,
) -> Self {
pub fn new(source_type: &'static str, target_type: &'static str) -> Self {
Self {
source_type,
target_type,
value: value.into(),
}
}
}

#[derive(Error, Debug, PartialEq, Eq)]
#[error("Cannot divide {operand} by zero")]
pub struct DivideByZeroError {
pub operand: String,
}
#[derive(Error, Debug, Default, PartialEq, Eq)]
#[error("Cannot divide by zero")]
pub struct DivideByZeroError;

impl DivideByZeroError {
pub fn new(operand: impl ToString) -> Self {
Self {
operand: operand.to_string(),
}
pub fn new() -> Self {
Self
}
}

Expand Down Expand Up @@ -789,87 +769,73 @@ mod tests {

#[test]
fn underflow_works_for_u128() {
let error =
StdError::overflow(OverflowError::new(OverflowOperation::Sub, 123u128, 456u128));
match error {
let error = StdError::overflow(OverflowError::new(OverflowOperation::Sub));
assert!(matches!(
error,
StdError::Overflow {
source:
OverflowError {
operation,
operand1,
operand2,
},
source: OverflowError {
operation: OverflowOperation::Sub
},
..
} => {
assert_eq!(operation, OverflowOperation::Sub);
assert_eq!(operand1, "123");
assert_eq!(operand2, "456");
}
_ => panic!("expect different error"),
}
));
}

#[test]
fn overflow_works_for_i64() {
let error = StdError::overflow(OverflowError::new(OverflowOperation::Sub, 777i64, 1234i64));
match error {
let error = StdError::overflow(OverflowError::new(OverflowOperation::Sub));
assert!(matches!(
error,
StdError::Overflow {
source:
OverflowError {
operation,
operand1,
operand2,
},
source: OverflowError {
operation: OverflowOperation::Sub
},
..
} => {
assert_eq!(operation, OverflowOperation::Sub);
assert_eq!(operand1, "777");
assert_eq!(operand2, "1234");
}
_ => panic!("expect different error"),
}
));
}

#[test]
fn divide_by_zero_works() {
let error = StdError::divide_by_zero(DivideByZeroError::new(123u128));
match error {
let error = StdError::divide_by_zero(DivideByZeroError);
assert!(matches!(
error,
StdError::DivideByZero {
source: DivideByZeroError { operand },
source: DivideByZeroError,
..
} => assert_eq!(operand, "123"),
_ => panic!("expect different error"),
}
}
));
}

#[test]
fn implements_debug() {
let error: StdError = StdError::from(OverflowError::new(OverflowOperation::Sub, 3, 5));
let error: StdError = StdError::from(OverflowError::new(OverflowOperation::Sub));
let embedded = format!("Debug: {error:?}");
#[cfg(not(feature = "backtraces"))]
let expected = r#"Debug: Overflow { source: OverflowError { operation: Sub, operand1: "3", operand2: "5" } }"#;
let expected = r#"Debug: Overflow { source: OverflowError { operation: Sub } }"#;
#[cfg(feature = "backtraces")]
let expected = r#"Debug: Overflow { source: OverflowError { operation: Sub, operand1: "3", operand2: "5" }, backtrace: <disabled> }"#;
let expected = r#"Debug: Overflow { source: OverflowError { operation: Sub }, backtrace: <disabled> }"#;
assert_eq!(embedded, expected);
}

#[test]
fn implements_display() {
let error: StdError = StdError::from(OverflowError::new(OverflowOperation::Sub, 3, 5));
let error: StdError = StdError::from(OverflowError::new(OverflowOperation::Sub));
let embedded = format!("Display: {error}");
assert_eq!(embedded, "Display: Overflow: Cannot Sub with 3 and 5");
assert_eq!(
embedded,
"Display: Overflow: Cannot Sub with given operands"
);
}

#[test]
fn implements_partial_eq() {
let u1 = StdError::from(OverflowError::new(OverflowOperation::Sub, 3, 5));
let u2 = StdError::from(OverflowError::new(OverflowOperation::Sub, 3, 5));
let u3 = StdError::from(OverflowError::new(OverflowOperation::Sub, 3, 7));
let u1 = StdError::from(OverflowError::new(OverflowOperation::Sub));
let u2 = StdError::from(OverflowError::new(OverflowOperation::Sub));
let s1 = StdError::serialize_err("Book", "Content too long");
let s2 = StdError::serialize_err("Book", "Content too long");
let s3 = StdError::serialize_err("Book", "Title too long");
assert_eq!(u1, u2);
assert_ne!(u1, u3);
assert_ne!(u1, s1);
assert_eq!(s1, s2);
assert_ne!(s1, s3);
Expand Down
42 changes: 8 additions & 34 deletions packages/std/src/math/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ macro_rules! forward_try_from {
.0
.try_into()
.map(Self)
.map_err(|_| Self::Error::new(stringify!($input), stringify!($output), value))
.map_err(|_| Self::Error::new(stringify!($input), stringify!($output)))
}
}
};
Expand All @@ -93,7 +93,7 @@ macro_rules! try_from_int_to_int {

fn try_from(value: $input) -> Result<Self, Self::Error> {
$crate::math::conversion::shrink_be_int(value.to_be_bytes())
.ok_or_else(|| Self::Error::new(stringify!($input), stringify!($output), value))
.ok_or_else(|| Self::Error::new(stringify!($input), stringify!($output)))
.map(Self::from_be_bytes)
}
}
Expand Down Expand Up @@ -123,11 +123,7 @@ macro_rules! try_from_uint_to_int {
use bnum::prelude::As;
// $input::MAX has to be bigger than $output::MAX, so we can just cast it
if value.0 > Self::MAX.0.as_() {
return Err(Self::Error::new(
stringify!($input),
stringify!($output),
value,
));
return Err(Self::Error::new(stringify!($input), stringify!($output)));
}

// at this point we know it fits
Expand Down Expand Up @@ -157,11 +153,7 @@ where
let v = I::MAX;
assert_eq!(
O::try_from(v),
Err(crate::ConversionOverflowError::new(
input_type,
output_type,
v
)),
Err(crate::ConversionOverflowError::new(input_type, output_type)),
"input::MAX value should not fit"
);

Expand All @@ -172,11 +164,7 @@ where
let v = max + I::ONE;
assert_eq!(
O::try_from(v),
Err(crate::ConversionOverflowError::new(
input_type,
output_type,
v
)),
Err(crate::ConversionOverflowError::new(input_type, output_type)),
"output::MAX + 1 should not fit"
);

Expand Down Expand Up @@ -222,11 +210,7 @@ where
let v = I::MAX;
assert_eq!(
O::try_from(v),
Err(crate::ConversionOverflowError::new(
input_type,
output_type,
v
)),
Err(crate::ConversionOverflowError::new(input_type, output_type)),
"input::MAX value should not fit"
);
// but `O::MAX` should fit
Expand All @@ -240,11 +224,7 @@ where
let v = max + I::ONE;
assert_eq!(
O::try_from(v),
Err(crate::ConversionOverflowError::new(
input_type,
output_type,
v
)),
Err(crate::ConversionOverflowError::new(input_type, output_type)),
"output::MAX + 1 should not fit"
);
}
Expand All @@ -253,11 +233,7 @@ where
let v = I::from(-42i32);
assert_eq!(
O::try_from(v),
Err(crate::ConversionOverflowError::new(
input_type,
output_type,
v
)),
Err(crate::ConversionOverflowError::new(input_type, output_type,)),
"negative numbers should not fit"
);

Expand Down Expand Up @@ -294,7 +270,6 @@ macro_rules! try_from_int_to_uint {
return Err(ConversionOverflowError::new(
stringify!($input),
stringify!($output),
value,
));
}

Expand All @@ -308,7 +283,6 @@ macro_rules! try_from_int_to_uint {
return Err(ConversionOverflowError::new(
stringify!($input),
stringify!($output),
value,
));
}

Expand Down
Loading