From d6881ff5f861b8a85883ec88ce453cc02191ba14 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 7 Apr 2025 12:12:20 +1000 Subject: [PATCH] units: Enable differentiating div-by-zero Division by zero is a different error class that overflow. Add a `MathOp` enum that enables one to check the error class. --- units/src/amount/result.rs | 42 +++++++++++++-------------- units/src/fee.rs | 10 +++---- units/src/lib.rs | 2 +- units/src/result.rs | 59 ++++++++++++++++++++++++++++++++++---- 4 files changed, 81 insertions(+), 32 deletions(-) diff --git a/units/src/amount/result.rs b/units/src/amount/result.rs index ccbc2150d..614e55bd8 100644 --- a/units/src/amount/result.rs +++ b/units/src/amount/result.rs @@ -7,7 +7,7 @@ use core::ops; use NumOpResult as R; use super::{Amount, SignedAmount}; -use crate::{NumOpError, NumOpResult, OptionExt}; +use crate::{MathOp, NumOpError, NumOpResult, OptionExt}; impl From for NumOpResult { fn from(a: Amount) -> Self { Self::Valid(a) } @@ -27,7 +27,7 @@ crate::internal_macros::impl_op_for_references! { impl ops::Add for Amount { type Output = NumOpResult; - fn add(self, rhs: Amount) -> Self::Output { self.checked_add(rhs).valid_or_error() } + fn add(self, rhs: Amount) -> Self::Output { self.checked_add(rhs).valid_or_error(MathOp::Add) } } impl ops::Add> for Amount { type Output = NumOpResult; @@ -38,7 +38,7 @@ crate::internal_macros::impl_op_for_references! { impl ops::Sub for Amount { type Output = NumOpResult; - fn sub(self, rhs: Amount) -> Self::Output { self.checked_sub(rhs).valid_or_error() } + fn sub(self, rhs: Amount) -> Self::Output { self.checked_sub(rhs).valid_or_error(MathOp::Sub) } } impl ops::Sub> for Amount { type Output = NumOpResult; @@ -54,7 +54,7 @@ crate::internal_macros::impl_op_for_references! { impl ops::Mul for Amount { type Output = NumOpResult; - fn mul(self, rhs: u64) -> Self::Output { self.checked_mul(rhs).valid_or_error() } + fn mul(self, rhs: u64) -> Self::Output { self.checked_mul(rhs).valid_or_error(MathOp::Mul) } } impl ops::Mul for NumOpResult { type Output = NumOpResult; @@ -64,7 +64,7 @@ crate::internal_macros::impl_op_for_references! { impl ops::Mul for u64 { type Output = NumOpResult; - fn mul(self, rhs: Amount) -> Self::Output { rhs.checked_mul(self).valid_or_error() } + fn mul(self, rhs: Amount) -> Self::Output { rhs.checked_mul(self).valid_or_error(MathOp::Mul) } } impl ops::Mul> for u64 { type Output = NumOpResult; @@ -75,7 +75,7 @@ crate::internal_macros::impl_op_for_references! { impl ops::Div for Amount { type Output = NumOpResult; - fn div(self, rhs: u64) -> Self::Output { self.checked_div(rhs).valid_or_error() } + fn div(self, rhs: u64) -> Self::Output { self.checked_div(rhs).valid_or_error(MathOp::Div) } } impl ops::Div for NumOpResult { type Output = NumOpResult; @@ -86,14 +86,14 @@ crate::internal_macros::impl_op_for_references! { type Output = NumOpResult; fn div(self, rhs: Amount) -> Self::Output { - self.to_sat().checked_div(rhs.to_sat()).valid_or_error() + self.to_sat().checked_div(rhs.to_sat()).valid_or_error(MathOp::Div) } } impl ops::Rem for Amount { type Output = NumOpResult; - fn rem(self, modulus: u64) -> Self::Output { self.checked_rem(modulus).valid_or_error() } + fn rem(self, modulus: u64) -> Self::Output { self.checked_rem(modulus).valid_or_error(MathOp::Rem) } } impl ops::Rem for NumOpResult { type Output = NumOpResult; @@ -104,7 +104,7 @@ crate::internal_macros::impl_op_for_references! { impl ops::Add for SignedAmount { type Output = NumOpResult; - fn add(self, rhs: SignedAmount) -> Self::Output { self.checked_add(rhs).valid_or_error() } + fn add(self, rhs: SignedAmount) -> Self::Output { self.checked_add(rhs).valid_or_error(MathOp::Add) } } impl ops::Add> for SignedAmount { type Output = NumOpResult; @@ -115,7 +115,7 @@ crate::internal_macros::impl_op_for_references! { impl ops::Sub for SignedAmount { type Output = NumOpResult; - fn sub(self, rhs: SignedAmount) -> Self::Output { self.checked_sub(rhs).valid_or_error() } + fn sub(self, rhs: SignedAmount) -> Self::Output { self.checked_sub(rhs).valid_or_error(MathOp::Sub) } } impl ops::Sub> for SignedAmount { type Output = NumOpResult; @@ -131,7 +131,7 @@ crate::internal_macros::impl_op_for_references! { impl ops::Mul for SignedAmount { type Output = NumOpResult; - fn mul(self, rhs: i64) -> Self::Output { self.checked_mul(rhs).valid_or_error() } + fn mul(self, rhs: i64) -> Self::Output { self.checked_mul(rhs).valid_or_error(MathOp::Mul) } } impl ops::Mul for NumOpResult { type Output = NumOpResult; @@ -141,7 +141,7 @@ crate::internal_macros::impl_op_for_references! { impl ops::Mul for i64 { type Output = NumOpResult; - fn mul(self, rhs: SignedAmount) -> Self::Output { rhs.checked_mul(self).valid_or_error() } + fn mul(self, rhs: SignedAmount) -> Self::Output { rhs.checked_mul(self).valid_or_error(MathOp::Mul) } } impl ops::Mul> for i64 { type Output = NumOpResult; @@ -152,7 +152,7 @@ crate::internal_macros::impl_op_for_references! { impl ops::Div for SignedAmount { type Output = NumOpResult; - fn div(self, rhs: i64) -> Self::Output { self.checked_div(rhs).valid_or_error() } + fn div(self, rhs: i64) -> Self::Output { self.checked_div(rhs).valid_or_error(MathOp::Div) } } impl ops::Div for NumOpResult { type Output = NumOpResult; @@ -163,14 +163,14 @@ crate::internal_macros::impl_op_for_references! { type Output = NumOpResult; fn div(self, rhs: SignedAmount) -> Self::Output { - self.to_sat().checked_div(rhs.to_sat()).valid_or_error() + self.to_sat().checked_div(rhs.to_sat()).valid_or_error(MathOp::Div) } } impl ops::Rem for SignedAmount { type Output = NumOpResult; - fn rem(self, modulus: i64) -> Self::Output { self.checked_rem(modulus).valid_or_error() } + fn rem(self, modulus: i64) -> Self::Output { self.checked_rem(modulus).valid_or_error(MathOp::Rem) } } impl ops::Rem for NumOpResult { type Output = NumOpResult; @@ -187,7 +187,7 @@ crate::internal_macros::impl_op_for_references! { fn add(self, rhs: Self) -> Self::Output { match (self, rhs) { (R::Valid(lhs), R::Valid(rhs)) => lhs + rhs, - (_, _) => R::Error(NumOpError {}), + (_, _) => R::Error(NumOpError::while_doing(MathOp::Add)), } } } @@ -210,7 +210,7 @@ crate::internal_macros::impl_op_for_references! { fn sub(self, rhs: Self) -> Self::Output { match (self, rhs) { (R::Valid(lhs), R::Valid(rhs)) => lhs - rhs, - (_, _) => R::Error(NumOpError {}), + (_, _) => R::Error(NumOpError::while_doing(MathOp::Sub)), } } } @@ -245,7 +245,7 @@ impl core::iter::Sum> for NumOpResult { { iter.fold(R::Valid(Amount::ZERO), |acc, amount| match (acc, amount) { (R::Valid(lhs), R::Valid(rhs)) => lhs + rhs, - (_, _) => R::Error(NumOpError {}), + (_, _) => R::Error(NumOpError::while_doing(MathOp::Add)), }) } } @@ -256,7 +256,7 @@ impl<'a> core::iter::Sum<&'a NumOpResult> for NumOpResult { { iter.fold(R::Valid(Amount::ZERO), |acc, amount| match (acc, amount) { (R::Valid(lhs), R::Valid(rhs)) => lhs + rhs, - (_, _) => R::Error(NumOpError {}), + (_, _) => R::Error(NumOpError::while_doing(MathOp::Add)), }) } } @@ -268,7 +268,7 @@ impl core::iter::Sum> for NumOpResult { { iter.fold(R::Valid(SignedAmount::ZERO), |acc, amount| match (acc, amount) { (R::Valid(lhs), R::Valid(rhs)) => lhs + rhs, - (_, _) => R::Error(NumOpError {}), + (_, _) => R::Error(NumOpError::while_doing(MathOp::Add)), }) } } @@ -279,7 +279,7 @@ impl<'a> core::iter::Sum<&'a NumOpResult> for NumOpResult lhs + rhs, - (_, _) => R::Error(NumOpError {}), + (_, _) => R::Error(NumOpError::while_doing(MathOp::Add)), }) } } diff --git a/units/src/fee.rs b/units/src/fee.rs index e01f31571..45d18e8dd 100644 --- a/units/src/fee.rs +++ b/units/src/fee.rs @@ -13,7 +13,7 @@ use core::ops; -use crate::{Amount, FeeRate, NumOpResult, OptionExt, Weight}; +use crate::{Amount, FeeRate, MathOp, NumOpResult, OptionExt, Weight}; impl Amount { /// Checked weight ceiling division. @@ -166,14 +166,14 @@ crate::internal_macros::impl_op_for_references! { impl ops::Mul for Weight { type Output = NumOpResult; fn mul(self, rhs: FeeRate) -> Self::Output { - rhs.checked_mul_by_weight(self).valid_or_error() + rhs.checked_mul_by_weight(self).valid_or_error(MathOp::Mul) } } impl ops::Mul for FeeRate { type Output = NumOpResult; fn mul(self, rhs: Weight) -> Self::Output { - self.checked_mul_by_weight(rhs).valid_or_error() + self.checked_mul_by_weight(rhs).valid_or_error(MathOp::Mul) } } @@ -181,7 +181,7 @@ crate::internal_macros::impl_op_for_references! { type Output = NumOpResult; fn div(self, rhs: Weight) -> Self::Output { - self.checked_div_by_weight_floor(rhs).valid_or_error() + self.checked_div_by_weight_floor(rhs).valid_or_error(MathOp::Div) } } @@ -189,7 +189,7 @@ crate::internal_macros::impl_op_for_references! { type Output = NumOpResult; fn div(self, rhs: FeeRate) -> Self::Output { - self.checked_div_by_fee_rate_floor(rhs).valid_or_error() + self.checked_div_by_fee_rate_floor(rhs).valid_or_error(MathOp::Div) } } } diff --git a/units/src/lib.rs b/units/src/lib.rs index 9d44f5ad8..0919efb73 100644 --- a/units/src/lib.rs +++ b/units/src/lib.rs @@ -44,7 +44,7 @@ pub use self::{ amount::{Amount, SignedAmount}, block::{BlockHeight, BlockInterval}, fee_rate::FeeRate, - result::{NumOpError, NumOpResult}, + result::{NumOpError, NumOpResult, MathOp}, time::BlockTime, weight::Weight }; diff --git a/units/src/result.rs b/units/src/result.rs index 3cf626e40..8276e6cc1 100644 --- a/units/src/result.rs +++ b/units/src/result.rs @@ -107,7 +107,7 @@ impl NumOpResult { } pub(crate) trait OptionExt { - fn valid_or_error(self) -> NumOpResult; + fn valid_or_error(self, op: MathOp) -> NumOpResult; } macro_rules! impl_opt_ext { @@ -115,10 +115,10 @@ macro_rules! impl_opt_ext { $( impl OptionExt<$ty> for Option<$ty> { #[inline] - fn valid_or_error(self) -> NumOpResult<$ty> { + fn valid_or_error(self, op: MathOp) -> NumOpResult<$ty> { match self { Some(amount) => R::Valid(amount), - None => R::Error(NumOpError {}), + None => R::Error(NumOpError(op)), } } } @@ -130,13 +130,62 @@ impl_opt_ext!(Amount, SignedAmount, u64, i64, FeeRate, Weight); /// An error occurred while doing a mathematical operation. #[derive(Debug, Copy, Clone, PartialEq, Eq)] #[non_exhaustive] -pub struct NumOpError; +pub struct NumOpError(MathOp); + +impl NumOpError { + /// Creates a [`NumOpError`] caused by `op`. + pub fn while_doing(op: MathOp) -> Self { NumOpError(op) } + + /// Returns the [`MathOp`] that caused this error. + pub fn operation(self) -> MathOp { self.0 } +} impl fmt::Display for NumOpError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "a math operation gave an invalid numeric result") + write!(f, "math operation '{}' gave an invalid numeric result", self.operation()) } } #[cfg(feature = "std")] impl std::error::Error for NumOpError {} + +/// The math operation that caused the error. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum MathOp { + /// Addition failed ([`core::ops::Add`] resulted in an invalid value). + Add, + /// Subtraction failed ([`core::ops::Sub`] resulted in an invalid value). + Sub, + /// Multiplication failed ([`core::ops::Mul`] resulted in an invalid value). + Mul, + /// Division failed ([`core::ops::Div`] attempted div-by-zero). + Div, + /// Calculating the remainder failed ([`core::ops::Rem`] attempted div-by-zero). + Rem, + /// Negation failed ([`core::ops::Neg`] resulted in an invalid value). + Neg, +} + +impl MathOp { + /// Returns `true` if this operation error'ed due to overflow. + pub fn is_overflow(self) -> bool { + matches!(self, MathOp::Add | MathOp::Sub | MathOp::Mul | MathOp::Neg) + } + + /// Returns `true` if this operation error'ed due to division by zero. + pub fn is_div_by_zero(self) -> bool { !self.is_overflow() } +} + +impl fmt::Display for MathOp { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + MathOp::Add => write!(f, "add"), + MathOp::Sub => write!(f, "sub"), + MathOp::Mul => write!(f, "mul"), + MathOp::Div => write!(f, "div"), + MathOp::Rem => write!(f, "rem"), + MathOp::Neg => write!(f, "neg"), + } + } +}