← Watch feed
units: Replace NumOpError internals with MathErrorKind
Public commit record
78/100 · Adequate
What the developer wrote
Authored by Mitchell Bagot
units: Replace NumOpError internals with MathErrorKind
The NumOpResult doesn't have a way to distinguish the cause of an error
from the operation that was performed. Some operations can have
multiple failure modes, such as division (div-by-zero vs overflow on
signed values). Rather than clutter the operation type, a new error kind
type which captures the failure mode should be introduced. Where a failure
mode applies to multiple operation types (e.g. overflow), the existing
MathOp is wrapped.
Introduce MathErrorKind to replace MathOp private field of NumOpError,
adjusting constructors as needed.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Names security-relevant behavior explicitly
What changed, and why it matters
This commit is a code-quality refactor inside the rust-bitcoin library's 'units' crate. It changes how arithmetic errors (overflow, divide-by-zero, remainder-by-zero) are described internally, making error messages more specific and preserving the original error when combining error values. It does not add or remove any security checks; it only re-labels the existing checked-math failures.
Recommended action
No security action required. Treat as a normal library refactor; review only if your code depends on the exact internal structure or `Display` output of `NumOpError`.
Security signals we found
01
Refactor of error representation only; no new arithmetic bounds checks introduced
02
Existing checked_add/checked_sub/checked_mul/checked_div/checked_rem paths remain unchanged
03
Error-preservation change in AddAssign/SubAssign could be considered a minor bugfix for error reporting but does not alter overflow/division-by-zero protection
Evidence from the diff
The patch replaces the private MathOp field inside NumOpError with a new MathErrorKind enum that distinguishes Overflow { op, is_negative }, DivByZero, and RemByZero. All call sites that previously created NumOpError::while_doing(MathOp::*) are updated to pass the appropriate kind. A notable behavior change is in AddAssign/SubAssign for NumOpResult: previously any error on the left-hand side was overwritten with a new Add/Sub error; now the existing error is preserved. This is a correctness/consistency improvement, not a vulnerability fix, and the underlying arithmetic remains checked.
Changed components
units/src/result.rsunits/src/amount/ops.rsunits/src/amount/unsigned.rsunits/src/fee_rate/mod.rsunits/src/internal_macros.rsunits/src/amount/tests.rsInspect captured patch +262 / −99
### units/src/amount/ops.rs
@@ -12,7 +12,7 @@ use crate::internal_macros::{
impl_add_assign_for_results, impl_div_assign, impl_mul_assign, impl_rem_assign,
impl_sub_assign_for_results,
};
-use crate::result::{MathOp, NumOpError, NumOpResult, OptionExt};
+use crate::result::{MathErrorKind, MathOp, NumOpError, NumOpResult, OptionExt};
impl From<Amount> for NumOpResult<Amount> {
#[inline]
@@ -36,7 +36,7 @@ crate::internal_macros::impl_op_for_references! {
impl ops::Add<Amount> for Amount {
type Output = NumOpResult<Amount>;
- fn add(self, rhs: Amount) -> Self::Output { self.checked_add(rhs).valid_or_error(MathOp::Add) }
+ fn add(self, rhs: Amount) -> Self::Output { self.checked_add(rhs).valid_or_error(MathErrorKind::Overflow { op: MathOp::Add, is_negative: false }) }
}
impl ops::Add<NumOpResult<Amount>> for Amount {
type Output = NumOpResult<Amount>;
@@ -47,7 +47,7 @@ crate::internal_macros::impl_op_for_references! {
impl ops::Sub<Amount> for Amount {
type Output = NumOpResult<Amount>;
- fn sub(self, rhs: Amount) -> Self::Output { self.checked_sub(rhs).valid_or_error(MathOp::Sub) }
+ fn sub(self, rhs: Amount) -> Self::Output { self.checked_sub(rhs).valid_or_error(MathErrorKind::Overflow { op: MathOp::Sub, is_negative: true }) }
}
impl ops::Sub<NumOpResult<Amount>> for Amount {
type Output = NumOpResult<Amount>;
@@ -63,7 +63,7 @@ crate::internal_macros::impl_op_for_references! {
impl ops::Mul<u64> for Amount {
type Output = NumOpResult<Amount>;
- fn mul(self, rhs: u64) -> Self::Output { self.checked_mul(rhs).valid_or_error(MathOp::Mul) }
+ fn mul(self, rhs: u64) -> Self::Output { self.checked_mul(rhs).valid_or_error(MathErrorKind::Overflow { op: MathOp::Mul, is_negative: false }) }
}
impl ops::Mul<u64> for NumOpResult<Amount> {
type Output = NumOpResult<Amount>;
@@ -73,7 +73,7 @@ crate::internal_macros::impl_op_for_references! {
impl ops::Mul<Amount> for u64 {
type Output = NumOpResult<Amount>;
- fn mul(self, rhs: Amount) -> Self::Output { rhs.checked_mul(self).valid_or_error(MathOp::Mul) }
+ fn mul(self, rhs: Amount) -> Self::Output { rhs.checked_mul(self).valid_or_error(MathErrorKind::Overflow { op: MathOp::Mul, is_negative: false }) }
}
impl ops::Mul<NumOpResult<Amount>> for u64 {
type Output = NumOpResult<Amount>;
@@ -84,7 +84,7 @@ crate::internal_macros::impl_op_for_references! {
impl ops::Div<u64> for Amount {
type Output = NumOpResult<Amount>;
- fn div(self, rhs: u64) -> Self::Output { self.checked_div(rhs).valid_or_error(MathOp::Div) }
+ fn div(self, rhs: u64) -> Self::Output { self.checked_div(rhs).valid_or_error(MathErrorKind::DivByZero) }
}
impl ops::Div<u64> for NumOpResult<Amount> {
type Output = NumOpResult<Amount>;
@@ -95,7 +95,7 @@ crate::internal_macros::impl_op_for_references! {
type Output = NumOpResult<u64>;
fn div(self, rhs: Amount) -> Self::Output {
- self.to_sat().checked_div(rhs.to_sat()).valid_or_error(MathOp::Div)
+ self.to_sat().checked_div(rhs.to_sat()).valid_or_error(MathErrorKind::DivByZero)
}
}
impl ops::Div<NonZeroU64> for Amount {
@@ -111,7 +111,7 @@ crate::internal_macros::impl_op_for_references! {
impl ops::Rem<u64> for Amount {
type Output = NumOpResult<Amount>;
- fn rem(self, modulus: u64) -> Self::Output { self.checked_rem(modulus).valid_or_error(MathOp::Rem) }
+ fn rem(self, modulus: u64) -> Self::Output { self.checked_rem(modulus).valid_or_error(MathErrorKind::RemByZero) }
}
impl ops::Rem<NonZeroU64> for Amount {
type Output = Amount;
@@ -132,7 +132,10 @@ crate::internal_macros::impl_op_for_references! {
impl ops::Add<SignedAmount> for SignedAmount {
type Output = NumOpResult<SignedAmount>;
- fn add(self, rhs: SignedAmount) -> Self::Output { self.checked_add(rhs).valid_or_error(MathOp::Add) }
+ fn add(self, rhs: SignedAmount) -> Self::Output {
+ let kind = MathErrorKind::Overflow { op: MathOp::Add, is_negative: rhs.is_negative() };
+ self.checked_add(rhs).valid_or_error(kind)
+ }
}
impl ops::Add<NumOpResult<SignedAmount>> for SignedAmount {
type Output = NumOpResult<SignedAmount>;
@@ -143,7 +146,10 @@ crate::internal_macros::impl_op_for_references! {
impl ops::Sub<SignedAmount> for SignedAmount {
type Output = NumOpResult<SignedAmount>;
- fn sub(self, rhs: SignedAmount) -> Self::Output { self.checked_sub(rhs).valid_or_error(MathOp::Sub) }
+ fn sub(self, rhs: SignedAmount) -> Self::Output {
+ let kind = MathErrorKind::Overflow { op: MathOp::Sub, is_negative: !rhs.is_negative() };
+ self.checked_sub(rhs).valid_or_error(kind)
+ }
}
impl ops::Sub<NumOpResult<SignedAmount>> for SignedAmount {
type Output = NumOpResult<SignedAmount>;
@@ -159,7 +165,10 @@ crate::internal_macros::impl_op_for_references! {
impl ops::Mul<i64> for SignedAmount {
type Output = NumOpResult<SignedAmount>;
- fn mul(self, rhs: i64) -> Self::Output { self.checked_mul(rhs).valid_or_error(MathOp::Mul) }
+ fn mul(self, rhs: i64) -> Self::Output {
+ let is_negative = self.is_negative() != rhs.is_negative();
+ self.checked_mul(rhs).valid_or_error(MathErrorKind::Overflow { op: MathOp::Mul, is_negative })
+ }
}
impl ops::Mul<i64> for NumOpResult<SignedAmount> {
type Output = NumOpResult<SignedAmount>;
@@ -169,7 +178,10 @@ crate::internal_macros::impl_op_for_references! {
impl ops::Mul<SignedAmount> for i64 {
type Output = NumOpResult<SignedAmount>;
- fn mul(self, rhs: SignedAmount) -> Self::Output { rhs.checked_mul(self).valid_or_error(MathOp::Mul) }
+ fn mul(self, rhs: SignedAmount) -> Self::Output {
+ let is_negative = self.is_negative() != rhs.is_negative();
+ rhs.checked_mul(self).valid_or_error(MathErrorKind::Overflow { op: MathOp::Mul, is_negative })
+ }
}
impl ops::Mul<NumOpResult<SignedAmount>> for i64 {
type Output = NumOpResult<SignedAmount>;
@@ -180,7 +192,7 @@ crate::internal_macros::impl_op_for_references! {
impl ops::Div<i64> for SignedAmount {
type Output = NumOpResult<SignedAmount>;
- fn div(self, rhs: i64) -> Self::Output { self.checked_div(rhs).valid_or_error(MathOp::Div) }
+ fn div(self, rhs: i64) -> Self::Output { self.checked_div(rhs).valid_or_error(MathErrorKind::DivByZero) }
}
impl ops::Div<i64> for NumOpResult<SignedAmount> {
type Output = NumOpResult<SignedAmount>;
@@ -191,7 +203,7 @@ crate::internal_macros::impl_op_for_references! {
type Output = NumOpResult<i64>;
fn div(self, rhs: SignedAmount) -> Self::Output {
- self.to_sat().checked_div(rhs.to_sat()).valid_or_error(MathOp::Div)
+ self.to_sat().checked_div(rhs.to_sat()).valid_or_error(MathErrorKind::DivByZero)
}
}
impl ops::Div<NonZeroI64> for SignedAmount {
@@ -207,7 +219,7 @@ crate::internal_macros::impl_op_for_references! {
impl ops::Rem<i64> for SignedAmount {
type Output = NumOpResult<SignedAmount>;
- fn rem(self, modulus: i64) -> Self::Output { self.checked_rem(modulus).valid_or_error(MathOp::Rem) }
+ fn rem(self, modulus: i64) -> Self::Output { self.checked_rem(modulus).valid_or_error(MathErrorKind::RemByZero) }
}
impl ops::Rem<NonZeroI64> for SignedAmount {
type Output = SignedAmount;
@@ -283,7 +295,10 @@ impl<T: Into<Self>> core::iter::Sum<T> for NumOpResult<Amount> {
{
iter.fold(Self::Valid(Amount::ZERO), |acc, amount| match (acc, amount.into()) {
(Self::Valid(lhs), Self::Valid(rhs)) => lhs + rhs,
- (_, _) => Self::Error(NumOpError::while_doing(MathOp::Add)),
+ (_, _) => Self::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ })),
})
}
}
@@ -294,7 +309,10 @@ impl<'a> core::iter::Sum<&'a Self> for NumOpResult<Amount> {
{
iter.fold(Self::Valid(Amount::ZERO), |acc, amount| match (acc, amount) {
(Self::Valid(lhs), Self::Valid(rhs)) => lhs + rhs,
- (_, _) => Self::Error(NumOpError::while_doing(MathOp::Add)),
+ (_, _) => Self::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ })),
})
}
}
@@ -306,7 +324,10 @@ impl<T: Into<Self>> core::iter::Sum<T> for NumOpResult<SignedAmount> {
{
iter.fold(Self::Valid(SignedAmount::ZERO), |acc, amount| match (acc, amount.into()) {
(Self::Valid(lhs), Self::Valid(rhs)) => lhs + rhs,
- (_, _) => Self::Error(NumOpError::while_doing(MathOp::Add)),
+ (_, _) => Self::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ })),
})
}
}
@@ -317,7 +338,10 @@ impl<'a> core::iter::Sum<&'a Self> for NumOpResult<SignedAmount> {
{
iter.fold(Self::Valid(SignedAmount::ZERO), |acc, amount| match (acc, amount) {
(Self::Valid(lhs), Self::Valid(rhs)) => lhs + rhs,
- (_, _) => Self::Error(NumOpError::while_doing(MathOp::Add)),
+ (_, _) => Self::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ })),
})
}
}
@@ -362,7 +386,10 @@ mod tests {
fn sum_amount_with_error_propagation() {
let amounts = [
NumOpResult::Valid(Amount::from_sat_u32(100)),
- NumOpResult::Error(NumOpError::while_doing(MathOp::Add)),
+ NumOpResult::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ })),
NumOpResult::Valid(Amount::from_sat_u32(200)),
];
@@ -410,7 +437,10 @@ mod tests {
fn sum_signed_amount_with_error_propagation() {
let amounts = [
NumOpResult::Valid(SignedAmount::from_sat_i32(100)),
- NumOpResult::Error(NumOpError::while_doing(MathOp::Add)),
+ NumOpResult::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ })),
NumOpResult::Valid(SignedAmount::from_sat_i32(200)),
];
@@ -426,15 +456,21 @@ mod tests {
res += Amount::from_sat_u32(50);
assert_eq!(res, NumOpResult::Valid(Amount::from_sat_u32(150)));
- let add_err = NumOpResult::Error(NumOpError::while_doing(MathOp::Add));
+ let add_err = NumOpResult::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ }));
res += add_err; // Add an error result
assert_eq!(res, add_err);
let mut res = sat + sat;
res -= Amount::from_sat_u32(20);
assert_eq!(res, NumOpResult::Valid(Amount::from_sat_u32(80)));
- let sub_err = NumOpResult::Error(NumOpError::while_doing(MathOp::Sub));
+ let sub_err = NumOpResult::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Sub,
+ is_negative: true,
+ }));
res -= sub_err; // Subtract an error result
assert_eq!(res, sub_err);
}
@@ -447,15 +483,21 @@ mod tests {
res += SignedAmount::from_sat_i32(-30);
assert_eq!(res, NumOpResult::Valid(SignedAmount::from_sat_i32(70)));
- let add_err = NumOpResult::Error(NumOpError::while_doing(MathOp::Add));
+ let add_err = NumOpResult::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ }));
res += add_err; // Add an error result
assert_eq!(res, add_err);
let mut res = ssat + ssat;
res -= SignedAmount::from_sat_i32(25);
assert_eq!(res, NumOpResult::Valid(SignedAmount::from_sat_i32(75)));
- let sub_err = NumOpResult::Error(NumOpError::while_doing(MathOp::Sub));
+ let sub_err = NumOpResult::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Sub,
+ is_negative: true,
+ }));
res -= sub_err; // Subtract an error result
assert_eq!(res, sub_err);
}
@@ -471,10 +513,10 @@ mod tests {
assert_eq!(res, NumOpResult::Valid(Amount::from_sat_u32(2)));
res %= 0_u64;
- assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathOp::Rem)));
+ assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathErrorKind::RemByZero)));
res %= 5_u64;
- assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathOp::Rem)));
+ assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathErrorKind::RemByZero)));
}
#[test]
@@ -504,10 +546,10 @@ mod tests {
assert_eq!(res, NumOpResult::Valid(SignedAmount::from_sat_i32(-2)));
res %= 0_i64;
- assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathOp::Rem)));
+ assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathErrorKind::RemByZero)));
res %= 5_i64;
- assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathOp::Rem)));
+ assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathErrorKind::RemByZero)));
}
#[test]
@@ -540,9 +582,19 @@ mod tests {
assert_eq!(res, NumOpResult::Valid(Amount::from_sat_u32(25)));
// An error result stays an error, keeping the original operation.
- let mut res: NumOpResult<Amount> = NumOpResult::Error(NumOpError::while_doing(MathOp::Add));
+ let mut res: NumOpResult<Amount> =
+ NumOpResult::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ }));
res /= NonZeroU64::new(2).unwrap();
- assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathOp::Add)));
+ assert_eq!(
+ res,
+ NumOpResult::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false
+ }))
+ );
}
#[test]
@@ -563,29 +615,46 @@ mod tests {
assert_eq!(res, NumOpResult::Valid(SignedAmount::from_sat_i32(-25)));
let mut res: NumOpResult<SignedAmount> =
- NumOpResult::Error(NumOpError::while_doing(MathOp::Sub));
+ NumOpResult::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Sub,
+ is_negative: true,
+ }));
res /= NonZeroI64::new(2).unwrap();
- assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathOp::Sub)));
+ assert_eq!(
+ res,
+ NumOpResult::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Sub,
+ is_negative: true
+ }))
+ );
}
#[test]
fn op_assign_amount_error() {
- let mut res: NumOpResult<Amount> = NumOpResult::Error(NumOpError::while_doing(MathOp::Add));
-
- // Adding a valid amount to an error should make an Add error
+ let mut res: NumOpResult<Amount> =
+ NumOpResult::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Mul,
+ is_negative: false,
+ }));
+ let orig_res = res;
+
+ // All assign ops to an error should preserve the error
res += Amount::from_sat_u32(10);
- assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathOp::Add)));
+ assert_eq!(res, orig_res);
- // Adding an error to an error change to an Add error
- res += NumOpResult::Error(NumOpError::while_doing(MathOp::Sub));
- assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathOp::Add)));
+ res += NumOpResult::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Sub,
+ is_negative: true,
+ }));
+ assert_eq!(res, orig_res);
- // Subtracting a valid amount from an error should make a Sub error
res -= Amount::from_sat_u32(10);
- assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathOp::Sub)));
+ assert_eq!(res, orig_res);
- // Subtracting an error from an error change to a Sub error
- res -= NumOpResult::Error(NumOpError::while_doing(MathOp::Add));
- assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathOp::Sub)));
+ res -= NumOpResult::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ }));
+ assert_eq!(res, orig_res);
}
}
### units/src/amount/tests.rs
@@ -11,7 +11,7 @@ use core::num::{NonZeroI64, NonZeroU64};
use std::panic;
use super::*;
-use crate::result::{MathOp, NumOpError, NumOpResult};
+use crate::result::{MathErrorKind, MathOp, NumOpError, NumOpResult};
#[cfg(feature = "alloc")]
use crate::FeeRate;
use crate::Weight;
@@ -1451,13 +1451,15 @@ fn math_op_errors() {
let overflow = Amount::MAX + Amount::from_sat(1).unwrap();
if let NumOpResult::Error(err) = overflow {
assert!(matches!(err.operation(), MathOp::Add));
+ assert!(matches!(err.0, MathErrorKind::Overflow { .. }));
} else {
panic!("Expected an overflow error, but got a valid result");
}
let div_by_zero = Amount::from_sat(10).unwrap() / Amount::ZERO;
if let NumOpResult::Error(err) = div_by_zero {
assert!(matches!(err.operation(), MathOp::Div));
+ assert!(matches!(err.0, MathErrorKind::DivByZero));
} else {
panic!("Expected a division by zero error, but got a valid result");
}
@@ -1648,20 +1650,26 @@ fn checked_rem() {
fn amount_div_by_weight_floor_error() {
// Division by zero weight returns error
let err = sat(100).div_by_weight_floor(Weight::ZERO).unwrap_err();
- assert_eq!(err, NumOpError::while_doing(MathOp::Div));
+ assert_eq!(err, NumOpError::while_doing(MathErrorKind::DivByZero));
// Overflow case: Amount::MAX * 1000 overflows
let err = Amount::MAX.div_by_weight_floor(Weight::from_wu(1)).unwrap_err();
- assert_eq!(err, NumOpError::while_doing(MathOp::Div));
+ assert_eq!(
+ err,
+ NumOpError::while_doing(MathErrorKind::Overflow { op: MathOp::Div, is_negative: false })
+ );
}
#[test]
fn amount_div_by_weight_ceil_error() {
// Division by zero weight returns error
let err = sat(100).div_by_weight_ceil(Weight::ZERO).unwrap_err();
- assert_eq!(err, NumOpError::while_doing(MathOp::Div));
+ assert_eq!(err, NumOpError::while_doing(MathErrorKind::DivByZero));
// Overflow case: Amount::MAX * 1000 overflows
let err = Amount::MAX.div_by_weight_ceil(Weight::from_wu(1)).unwrap_err();
- assert_eq!(err, NumOpError::while_doing(MathOp::Div));
+ assert_eq!(
+ err,
+ NumOpError::while_doing(MathErrorKind::Overflow { op: MathOp::Div, is_negative: false })
+ );
}
### units/src/amount/unsigned.rs
@@ -19,7 +19,7 @@ use super::{
parse_signed_to_satoshi, split_amount_and_denomination, Denomination, Display, DisplayStyle,
OutOfRangeError, ParseAmountError, ParseError, SignedAmount,
};
-use crate::result::{MathOp, NumOpError as E, NumOpResult};
+use crate::result::{MathErrorKind, MathOp, NumOpError as E, NumOpResult};
use crate::{parse_int, FeeRate, Weight};
mod encapsulate {
@@ -471,10 +471,10 @@ impl Amount {
if let Ok(amount) = Self::from_sat(fee_rate) {
return FeeRate::from_per_kwu(amount);
},
- None => return R::Error(E::while_doing(MathOp::Div)),
+ None => return R::Error(E::while_doing(MathErrorKind::DivByZero)),
}
}
- R::Error(E::while_doing(MathOp::Div))
+ R::Error(E::while_doing(MathErrorKind::Overflow { op: MathOp::Div, is_negative: false }))
}
/// Checked weight ceiling division.
@@ -497,7 +497,7 @@ impl Amount {
pub const fn div_by_weight_ceil(self, weight: Weight) -> NumOpResult<FeeRate> {
let wu = weight.to_wu();
if wu == 0 {
- return R::Error(E::while_doing(MathOp::Div));
+ return R::Error(E::while_doing(MathErrorKind::DivByZero));
}
// Mul by 1,000 because we use per/kwu.
@@ -508,7 +508,7 @@ impl Amount {
return FeeRate::from_per_kwu(amount);
}
}
- R::Error(E::while_doing(MathOp::Div))
+ R::Error(E::while_doing(MathErrorKind::Overflow { op: MathOp::Div, is_negative: false }))
}
/// Checked fee rate floor division.
@@ -522,7 +522,7 @@ impl Amount {
let msats = self.to_sat() * 1_000;
match msats.checked_div(fee_rate.to_sat_per_kwu_ceil()) {
Some(wu) => R::Valid(Weight::from_wu(wu)),
- None => R::Error(E::while_doing(MathOp::Div)),
+ None => R::Error(E::while_doing(MathErrorKind::DivByZero)),
}
}
@@ -536,7 +536,7 @@ impl Amount {
let rate = fee_rate.to_sat_per_kwu_ceil();
// Early return so we do not have to use checked arithmetic below.
if rate == 0 {
- return R::Error(E::while_doing(MathOp::Div));
+ return R::Error(E::while_doing(MathErrorKind::DivByZero));
}
debug_assert!(Self::MAX.to_sat().checked_mul(1_000).is_some());
### units/src/fee_rate/mod.rs
@@ -12,7 +12,7 @@ use core::ops;
use arbitrary::{Arbitrary, Unstructured};
use NumOpResult as R;
-use crate::result::{MathOp, NumOpError as E, NumOpResult};
+use crate::result::{MathErrorKind, MathOp, NumOpError as E, NumOpResult};
use crate::{Amount, Weight};
mod encapsulate {
@@ -80,7 +80,10 @@ impl FeeRate {
// No `map()` in const context.
match rate.checked_mul(4_000) {
Some(per_mvb) => R::Valid(Self::from_sat_per_mvb(per_mvb.to_sat())),
- None => R::Error(E::while_doing(MathOp::Mul)),
+ None => R::Error(E::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Mul,
+ is_negative: false,
+ })),
}
}
@@ -97,7 +100,10 @@ impl FeeRate {
// No `map()` in const context.
match rate.checked_mul(1_000_000) {
Some(per_mvb) => R::Valid(Self::from_sat_per_mvb(per_mvb.to_sat())),
- None => R::Error(E::while_doing(MathOp::Mul)),
+ None => R::Error(E::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Mul,
+ is_negative: false,
+ })),
}
}
@@ -114,7 +120,10 @@ impl FeeRate {
// No `map()` in const context.
match rate.checked_mul(1_000) {
Some(per_mvb) => R::Valid(Self::from_sat_per_mvb(per_mvb.to_sat())),
- None => R::Error(E::while_doing(MathOp::Mul)),
+ None => R::Error(E::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Mul,
+ is_negative: false,
+ })),
}
}
@@ -218,7 +227,10 @@ impl FeeRate {
return NumOpResult::Valid(fee_amount);
}
}
- NumOpResult::Error(E::while_doing(MathOp::Mul))
+ NumOpResult::Error(E::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Mul,
+ is_negative: false,
+ }))
}
}
### units/src/internal_macros.rs
@@ -103,9 +103,8 @@ macro_rules! impl_add_assign_for_results {
impl ops::AddAssign<$ty> for NumOpResult<$ty> {
#[inline]
fn add_assign(&mut self, rhs: $ty) {
- match self {
- Self::Error(_) => *self = Self::Error(NumOpError::while_doing(MathOp::Add)),
- Self::Valid(ref lhs) => *self = lhs + rhs,
+ if let Self::Valid(ref lhs) = self {
+ *self = lhs + rhs
}
}
}
@@ -115,7 +114,8 @@ macro_rules! impl_add_assign_for_results {
fn add_assign(&mut self, rhs: Self) {
match (&self, rhs) {
(Self::Valid(_), Self::Valid(rhs)) => *self += rhs,
- (_, _) => *self = Self::Error(NumOpError::while_doing(MathOp::Add)),
+ (Self::Valid(_), Self::Error(err)) => *self = Self::Error(err),
+ (Self::Error(_), _) => (), // If the lhs is an error, preserve it
}
}
}
@@ -137,9 +137,8 @@ macro_rules! impl_sub_assign_for_results {
impl ops::SubAssign<$ty> for NumOpResult<$ty> {
#[inline]
fn sub_assign(&mut self, rhs: $ty) {
- match self {
- Self::Error(_) => *self = Self::Error(NumOpError::while_doing(MathOp::Sub)),
- Self::Valid(ref lhs) => *self = lhs - rhs,
+ if let Self::Valid(ref lhs) = self {
+ *self = lhs - rhs
}
}
}
@@ -149,7 +148,8 @@ macro_rules! impl_sub_assign_for_results {
fn sub_assign(&mut self, rhs: Self) {
match (&self, rhs) {
(Self::Valid(_), Self::Valid(rhs)) => *self -= rhs,
- (_, _) => *self = Self::Error(NumOpError::while_doing(MathOp::Sub)),
+ (Self::Valid(_), Self::Error(err)) => *self = Self::Error(err),
+ (Self::Error(_), _) => (), // If the lhs is an error, preserve it
}
}
}
### units/src/result.rs
@@ -248,7 +248,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::while_doing(MathOp::Add)),
+ (_, _) => R::Error(NumOpError::while_doing(MathErrorKind::Overflow { op: MathOp::Add, is_negative: false })),
}
}
}
@@ -271,7 +271,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::while_doing(MathOp::Sub)),
+ (_, _) => R::Error(NumOpError::while_doing(MathErrorKind::Overflow { op: MathOp::Sub, is_negative: true })),
}
}
}
@@ -306,7 +306,8 @@ impl<T: ops::AddAssign + Copy> ops::AddAssign<Self> for NumOpResult<T> {
fn add_assign(&mut self, rhs: Self) {
match (&self, rhs) {
(Self::Valid(_), Self::Valid(rhs)) => *self += rhs,
- (_, _) => *self = Self::Error(NumOpError::while_doing(MathOp::Add)),
+ (Self::Valid(_), Self::Error(err)) => *self = Self::Error(err),
+ (Self::Error(_), _) => (), // If the lhs is an error, preserve it
}
}
}
@@ -326,24 +327,25 @@ impl<T: ops::SubAssign + Copy> ops::SubAssign<Self> for NumOpResult<T> {
fn sub_assign(&mut self, rhs: Self) {
match (&self, rhs) {
(Self::Valid(_), Self::Valid(rhs)) => *self -= rhs,
- (_, _) => *self = Self::Error(NumOpError::while_doing(MathOp::Sub)),
+ (Self::Valid(_), Self::Error(err)) => *self = Self::Error(err),
+ (Self::Error(_), _) => (), // If the lhs is an error, preserve it
}
}
}
pub(crate) trait OptionExt<T> {
- fn valid_or_error(self, op: MathOp) -> NumOpResult<T>;
+ fn valid_or_error(self, kind: MathErrorKind) -> NumOpResult<T>;
}
macro_rules! impl_opt_ext {
($($ty:ident),* $(,)?) => {
$(
impl OptionExt<$ty> for Option<$ty> {
#[inline]
- fn valid_or_error(self, op: MathOp) -> NumOpResult<$ty> {
+ fn valid_or_error(self, kind: MathErrorKind) -> NumOpResult<$ty> {
match self {
Some(amount) => R::Valid(amount),
- None => R::Error(NumOpError(op)),
+ None => R::Error(NumOpError::while_doing(kind)),
}
}
}
@@ -415,26 +417,46 @@ impl fmt::Display for MathOp {
}
}
+/// The kind of error that happened during a mathematical operation failure.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) enum MathErrorKind {
+ // The result overflowed.
+ Overflow {
+ op: MathOp,
+ is_negative: bool,
+ },
+ /// A division by zero was performed.
+ DivByZero,
+ /// A remainder by zero was performed.
+ RemByZero,
+}
+
/// Error types for mathematical operations.
pub mod error {
use core::convert::Infallible;
use core::fmt;
- use super::MathOp;
+ use super::{MathErrorKind, MathOp};
/// Error returned when a mathematical operation fails.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
#[non_exhaustive]
- pub struct NumOpError(pub(super) MathOp);
+ pub struct NumOpError(pub(crate) MathErrorKind);
impl NumOpError {
- /// Constructs a [`NumOpError`] caused by `op`.
+ /// Constructs a [`NumOpError`] caused by `kind`.
#[inline]
- pub(crate) const fn while_doing(op: MathOp) -> Self { Self(op) }
+ pub(crate) const fn while_doing(kind: MathErrorKind) -> Self { Self(kind) }
/// Returns the [`MathOp`] that caused this error.
#[inline]
- pub fn operation(self) -> MathOp { self.0 }
+ pub fn operation(self) -> MathOp {
+ match self.0 {
+ MathErrorKind::DivByZero => MathOp::Div,
+ MathErrorKind::RemByZero => MathOp::Rem,
+ MathErrorKind::Overflow { op, is_negative: _ } => op,
+ }
+ }
}
impl From<Infallible> for NumOpError {
@@ -445,7 +467,15 @@ pub mod error {
impl fmt::Display for NumOpError {
#[inline]
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
- write!(f, "math operation '{}' gave an invalid numeric result", self.operation())
+ match self.0 {
+ MathErrorKind::Overflow { op, is_negative: false } =>
+ write!(f, "failed to {}: the result would be too large", op),
+ MathErrorKind::Overflow { op, is_negative: true } =>
+ write!(f, "failed to {}: the result would be too small", op),
+ MathErrorKind::DivByZero => write!(f, "division by zero"),
+ MathErrorKind::RemByZero =>
+ write!(f, "attempt to compute the remainder of division by zero"),
+ }
}
}
@@ -465,7 +495,7 @@ impl<'a, T: Arbitrary<'a>> Arbitrary<'a> for NumOpResult<T> {
fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
match bool::arbitrary(u)? {
true => Ok(Self::Valid(T::arbitrary(u)?)),
- false => Ok(Self::Error(NumOpError(MathOp::arbitrary(u)?))),
+ false => Ok(Self::Error(NumOpError(MathErrorKind::arbitrary(u)?))),
}
}
}
@@ -486,14 +516,27 @@ impl<'a> Arbitrary<'a> for MathOp {
}
}
+#[cfg(feature = "arbitrary")]
+impl<'a> Arbitrary<'a> for MathErrorKind {
+ #[inline]
+ fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
+ let choice = u.int_in_range(0..=2)?;
+ match choice {
+ 0 => Ok(Self::Overflow { op: MathOp::arbitrary(u)?, is_negative: bool::arbitrary(u)? }),
+ 1 => Ok(Self::RemByZero),
+ _ => Ok(Self::DivByZero),
+ }
+ }
+}
+
#[cfg(test)]
mod tests {
#[cfg(feature = "alloc")]
use alloc::string::ToString;
#[cfg(feature = "std")]
use std::error::Error;
- use crate::result::{MathOp, NumOpError, NumOpResult};
+ use crate::result::{MathErrorKind, MathOp, NumOpError, NumOpResult};
use crate::{Amount, FeeRate, Weight};
#[test]
@@ -525,7 +568,10 @@ mod tests {
assert_eq!(new_value, NumOpResult::Valid(Weight::from_wu(10_000)));
// op is not evaluated for error results
- let res = NumOpResult::<Weight>::Error(NumOpError::while_doing(MathOp::Add));
+ let res = NumOpResult::<Weight>::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ }));
let res_err = res.map(|_| {
panic!("map should not evaluate for wrapped error values");
});
@@ -551,8 +597,11 @@ mod tests {
#[test]
#[should_panic(expected = "test error message")]
fn mathop_expect_panics_on_error() {
- NumOpResult::<Amount>::Error(NumOpError::while_doing(MathOp::Add))
- .expect("test error message");
+ NumOpResult::<Amount>::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ }))
+ .expect("test error message");
}
#[test]
@@ -574,18 +623,31 @@ mod tests {
#[test]
#[should_panic(expected = "")]
fn mathop_unwrap_panics_on_err() {
- NumOpResult::<Amount>::Error(NumOpError::while_doing(MathOp::Add)).unwrap();
+ NumOpResult::<Amount>::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ }))
+ .unwrap();
}
#[test]
fn mathop_unwrap_err() {
let errs = [
- NumOpError::while_doing(MathOp::Add),
- NumOpError::while_doing(MathOp::Sub),
- NumOpError::while_doing(MathOp::Mul),
- NumOpError::while_doing(MathOp::Div),
- NumOpError::while_doing(MathOp::Neg),
- NumOpError::while_doing(MathOp::Rem),
+ NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ }),
+ NumOpError::while_doing(MathErrorKind::Overflow { op: MathOp::Sub, is_negative: true }),
+ NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Mul,
+ is_negative: false,
+ }),
+ NumOpError::while_doing(MathErrorKind::DivByZero),
+ NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Neg,
+ is_negative: false,
+ }),
+ NumOpError::while_doing(MathErrorKind::RemByZero),
];
for err in errs {
assert_eq!(NumOpResult::<Amount>::Error(err).unwrap_err(), err);
@@ -604,7 +666,10 @@ mod tests {
let base_amount = Amount::from_sat_u32(100);
// default is returned for error results
- let res = NumOpResult::<Amount>::Error(NumOpError::while_doing(MathOp::Add));
+ let res = NumOpResult::<Amount>::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ }));
let res_default = res.unwrap_or(base_amount);
assert_eq!(res_default, base_amount);
@@ -619,7 +684,10 @@ mod tests {
let base_amount = Amount::from_sat_u32(100);
// op is evaluated for error results
- let res = NumOpResult::<Amount>::Error(NumOpError::while_doing(MathOp::Add));
+ let res = NumOpResult::<Amount>::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ }));
let res_default = res.unwrap_or_else(|| base_amount);
assert_eq!(res_default, base_amount);
@@ -636,7 +704,10 @@ mod tests {
let amt = Amount::from_sat_u32(150);
assert_eq!(NumOpResult::Valid(amt).ok(), Some(amt));
- let err = NumOpError::while_doing(MathOp::Add);
+ let err = NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ });
assert_eq!(NumOpResult::<Amount>::Error(err).ok(), None);
}
@@ -648,7 +719,10 @@ mod tests {
assert_eq!(new_value, NumOpResult::Valid(Amount::from_sat_u32(150)));
// op is not evaluated for error results
- let res = NumOpResult::<Amount>::Error(NumOpError::while_doing(MathOp::Add));
+ let res = NumOpResult::<Amount>::Error(NumOpError::while_doing(MathErrorKind::Overflow {
+ op: MathOp::Add,
+ is_negative: false,
+ }));
let res_err = res.and_then(|_| {
panic!("and_then should not evaluate for wrapped error values");
});Risk score
Our methodology →Why this scored 19/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.