From f49efdf3f751e3992700841d0888a2993beb5956 Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me@tobin.cc>
Date: Mon, 7 Apr 2025 10:55:09 +1000
Subject: [PATCH 1/6] units: Remove mention of amount in result module

We currently use the `NumOpResult` for operations involving more than
just amount types (e.g. `FeeRate`) however when the `result` module was
written we only used amount types.

To make the docs and code clearer use 'numeric type' instead of
'amount' in docs. And for local variables use `x` instead of `amount`.

This is docs and internal changes only.
---
 units/src/amount/result.rs | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/units/src/amount/result.rs b/units/src/amount/result.rs
index ab06f207f..c36584b1c 100644
--- a/units/src/amount/result.rs
+++ b/units/src/amount/result.rs
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: CC0-1.0
 
-//! Provides a monodic type used when mathematical operations (`core::ops`) return an amount type.
+//! Provides a monodic type returned by mathematical operations (`core::ops`).
 
 use core::{fmt, ops};
 
@@ -8,9 +8,7 @@ use NumOpResult as R;
 
 use super::{Amount, SignedAmount};
 
-/// Result of an operation on [`Amount`] or [`SignedAmount`].
-///
-/// The type parameter `T` should be normally `Amount` or `SignedAmount`.
+/// Result of a mathematical operation on two numeric types.
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 #[must_use]
 pub enum NumOpResult<T> {
@@ -21,7 +19,7 @@ pub enum NumOpResult<T> {
 }
 
 impl<T: fmt::Debug> NumOpResult<T> {
-    /// Returns the contained valid amount, consuming `self`.
+    /// Returns the contained valid numeric type, consuming `self`.
     ///
     /// # Panics
     ///
@@ -30,12 +28,12 @@ impl<T: fmt::Debug> NumOpResult<T> {
     #[track_caller]
     pub fn expect(self, msg: &str) -> T {
         match self {
-            R::Valid(amount) => amount,
+            R::Valid(x) => x,
             R::Error(_) => panic!("{}", msg),
         }
     }
 
-    /// Returns the contained valid amount, consuming `self`.
+    /// Returns the contained valid numeric type, consuming `self`.
     ///
     /// # Panics
     ///
@@ -44,7 +42,7 @@ impl<T: fmt::Debug> NumOpResult<T> {
     #[track_caller]
     pub fn unwrap(self) -> T {
         match self {
-            R::Valid(amount) => amount,
+            R::Valid(x) => x,
             R::Error(e) => panic!("tried to unwrap an invalid numeric result: {:?}", e),
         }
     }
@@ -53,7 +51,7 @@ impl<T: fmt::Debug> NumOpResult<T> {
     ///
     /// # Panics
     ///
-    /// Panics if the numeric result is a valid amount.
+    /// Panics if the numeric result is valid.
     #[inline]
     #[track_caller]
     pub fn unwrap_err(self) -> NumOpError {
@@ -67,7 +65,7 @@ impl<T: fmt::Debug> NumOpResult<T> {
     #[inline]
     pub fn ok(self) -> Option<T> {
         match self {
-            R::Valid(amount) => Some(amount),
+            R::Valid(x) => Some(x),
             R::Error(_) => None,
         }
     }
@@ -77,7 +75,7 @@ impl<T: fmt::Debug> NumOpResult<T> {
     #[allow(clippy::missing_errors_doc)]
     pub fn into_result(self) -> Result<T, NumOpError> {
         match self {
-            R::Valid(amount) => Ok(amount),
+            R::Valid(x) => Ok(x),
             R::Error(e) => Err(e),
         }
     }
@@ -89,12 +87,12 @@ impl<T: fmt::Debug> NumOpResult<T> {
         F: FnOnce(T) -> NumOpResult<T>,
     {
         match self {
-            R::Valid(amount) => op(amount),
+            R::Valid(x) => op(x),
             R::Error(e) => R::Error(e),
         }
     }
 
-    /// Returns `true` if the numeric result is a valid amount.
+    /// Returns `true` if the numeric result is valid.
     #[inline]
     pub fn is_valid(&self) -> bool {
         match self {
@@ -103,7 +101,7 @@ impl<T: fmt::Debug> NumOpResult<T> {
         }
     }
 
-    /// Returns `true` if the numeric result is an invalid amount.
+    /// Returns `true` if the numeric result is invalid.
     #[inline]
     pub fn is_error(&self) -> bool { !self.is_valid() }
 }
@@ -410,7 +408,7 @@ pub struct NumOpError;
 
 impl fmt::Display for NumOpError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "a math operation on amounts gave an invalid numeric result")
+        write!(f, "a math operation gave an invalid numeric result")
     }
 }
 

From f5b54e5fe065181081cfb68c6e9ef09528498e9e Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me@tobin.cc>
Date: Mon, 7 Apr 2025 11:01:57 +1000
Subject: [PATCH 2/6] units: Move general result stuff to a separate module

We currently use the `NumOpResult` for operations involving more than
just amount types (e.g. `FeeRate`) however when the `result` module was
written we only used amount types.

To make the intention of the custom result types more clear introduce a
top level `result` module and move the general code there. Leave the
amount implementations in the `amount` module. Note that both `result`
modules are private.

Move the `OptionExt` impls because later we will add a bunch more of them.

Internal change only, no logic changes.
---
 units/src/amount/mod.rs    |   2 -
 units/src/amount/result.rs | 139 +----------------------------------
 units/src/amount/tests.rs  |   1 +
 units/src/fee.rs           |   3 +-
 units/src/lib.rs           |   3 +
 units/src/result.rs        | 145 +++++++++++++++++++++++++++++++++++++
 6 files changed, 152 insertions(+), 141 deletions(-)
 create mode 100644 units/src/result.rs

diff --git a/units/src/amount/mod.rs b/units/src/amount/mod.rs
index 8928cea9d..6fb09fbcf 100644
--- a/units/src/amount/mod.rs
+++ b/units/src/amount/mod.rs
@@ -35,11 +35,9 @@ pub use self::{
         OutOfRangeError, ParseAmountError, ParseDenominationError, ParseError,
         PossiblyConfusingDenominationError, TooPreciseError, UnknownDenominationError,
     },
-    result::{NumOpError, NumOpResult},
     signed::SignedAmount,
     unsigned::Amount,
 };
-pub(crate) use self::result::OptionExt;
 
 /// A set of denominations in which amounts can be expressed.
 ///
diff --git a/units/src/amount/result.rs b/units/src/amount/result.rs
index c36584b1c..a0ad26954 100644
--- a/units/src/amount/result.rs
+++ b/units/src/amount/result.rs
@@ -2,109 +2,12 @@
 
 //! Provides a monodic type returned by mathematical operations (`core::ops`).
 
-use core::{fmt, ops};
+use core::ops;
 
 use NumOpResult as R;
 
 use super::{Amount, SignedAmount};
-
-/// Result of a mathematical operation on two numeric types.
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
-#[must_use]
-pub enum NumOpResult<T> {
-    /// Result of a successful mathematical operation.
-    Valid(T),
-    /// Result of an unsuccessful mathematical operation.
-    Error(NumOpError),
-}
-
-impl<T: fmt::Debug> NumOpResult<T> {
-    /// Returns the contained valid numeric type, consuming `self`.
-    ///
-    /// # Panics
-    ///
-    /// Panics with `msg` if the numeric result is an `Error`.
-    #[inline]
-    #[track_caller]
-    pub fn expect(self, msg: &str) -> T {
-        match self {
-            R::Valid(x) => x,
-            R::Error(_) => panic!("{}", msg),
-        }
-    }
-
-    /// Returns the contained valid numeric type, consuming `self`.
-    ///
-    /// # Panics
-    ///
-    /// Panics if the numeric result is an `Error`.
-    #[inline]
-    #[track_caller]
-    pub fn unwrap(self) -> T {
-        match self {
-            R::Valid(x) => x,
-            R::Error(e) => panic!("tried to unwrap an invalid numeric result: {:?}", e),
-        }
-    }
-
-    /// Returns the contained error, consuming `self`.
-    ///
-    /// # Panics
-    ///
-    /// Panics if the numeric result is valid.
-    #[inline]
-    #[track_caller]
-    pub fn unwrap_err(self) -> NumOpError {
-        match self {
-            R::Error(e) => e,
-            R::Valid(a) => panic!("tried to unwrap a valid numeric result: {:?}", a),
-        }
-    }
-
-    /// Converts this `NumOpResult` to an `Option<T>`.
-    #[inline]
-    pub fn ok(self) -> Option<T> {
-        match self {
-            R::Valid(x) => Some(x),
-            R::Error(_) => None,
-        }
-    }
-
-    /// Converts this `NumOpResult` to a `Result<T, NumOpError>`.
-    #[inline]
-    #[allow(clippy::missing_errors_doc)]
-    pub fn into_result(self) -> Result<T, NumOpError> {
-        match self {
-            R::Valid(x) => Ok(x),
-            R::Error(e) => Err(e),
-        }
-    }
-
-    /// Calls `op` if the numeric result is `Valid`, otherwise returns the `Error` value of `self`.
-    #[inline]
-    pub fn and_then<F>(self, op: F) -> NumOpResult<T>
-    where
-        F: FnOnce(T) -> NumOpResult<T>,
-    {
-        match self {
-            R::Valid(x) => op(x),
-            R::Error(e) => R::Error(e),
-        }
-    }
-
-    /// Returns `true` if the numeric result is valid.
-    #[inline]
-    pub fn is_valid(&self) -> bool {
-        match self {
-            R::Valid(_) => true,
-            R::Error(_) => false,
-        }
-    }
-
-    /// Returns `true` if the numeric result is invalid.
-    #[inline]
-    pub fn is_error(&self) -> bool { !self.is_valid() }
-}
+use crate::{NumOpError, NumOpResult, OptionExt};
 
 impl From<Amount> for NumOpResult<Amount> {
     fn from(a: Amount) -> Self { Self::Valid(a) }
@@ -376,41 +279,3 @@ impl<'a> core::iter::Sum<&'a NumOpResult<SignedAmount>> for NumOpResult<SignedAm
         })
     }
 }
-
-pub(crate) trait OptionExt<T> {
-    fn valid_or_error(self) -> NumOpResult<T>;
-}
-
-impl OptionExt<Amount> for Option<Amount> {
-    #[inline]
-    fn valid_or_error(self) -> NumOpResult<Amount> {
-        match self {
-            Some(amount) => R::Valid(amount),
-            None => R::Error(NumOpError {}),
-        }
-    }
-}
-
-impl OptionExt<SignedAmount> for Option<SignedAmount> {
-    #[inline]
-    fn valid_or_error(self) -> NumOpResult<SignedAmount> {
-        match self {
-            Some(amount) => R::Valid(amount),
-            None => R::Error(NumOpError {}),
-        }
-    }
-}
-
-/// An error occurred while doing a mathematical operation.
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
-#[non_exhaustive]
-pub struct NumOpError;
-
-impl fmt::Display for NumOpError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "a math operation gave an invalid numeric result")
-    }
-}
-
-#[cfg(feature = "std")]
-impl std::error::Error for NumOpError {}
diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs
index f8aba133f..2dbefee14 100644
--- a/units/src/amount/tests.rs
+++ b/units/src/amount/tests.rs
@@ -13,6 +13,7 @@ use std::panic;
 use ::serde::{Deserialize, Serialize};
 
 use super::*;
+use crate::NumOpResult;
 #[cfg(feature = "alloc")]
 use crate::{FeeRate, Weight};
 
diff --git a/units/src/fee.rs b/units/src/fee.rs
index 21d223c9c..5d277111e 100644
--- a/units/src/fee.rs
+++ b/units/src/fee.rs
@@ -13,8 +13,7 @@
 
 use core::ops;
 
-use crate::amount::{NumOpResult, OptionExt};
-use crate::{Amount, FeeRate, Weight};
+use crate::{Amount, FeeRate, NumOpResult, OptionExt, Weight};
 
 impl Amount {
     /// Checked weight ceiling division.
diff --git a/units/src/lib.rs b/units/src/lib.rs
index 4a166ebb2..9d44f5ad8 100644
--- a/units/src/lib.rs
+++ b/units/src/lib.rs
@@ -20,6 +20,7 @@ extern crate std;
 
 mod fee;
 mod internal_macros;
+mod result;
 
 #[doc(hidden)]
 pub mod _export {
@@ -43,6 +44,8 @@ pub use self::{
     amount::{Amount, SignedAmount},
     block::{BlockHeight, BlockInterval},
     fee_rate::FeeRate,
+    result::{NumOpError, NumOpResult},
     time::BlockTime,
     weight::Weight
 };
+pub(crate) use self::result::OptionExt;
diff --git a/units/src/result.rs b/units/src/result.rs
new file mode 100644
index 000000000..afe268875
--- /dev/null
+++ b/units/src/result.rs
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: CC0-1.0
+
+//! Provides a monodic type returned by mathematical operations (`core::ops`).
+
+use core::fmt;
+
+use NumOpResult as R;
+
+use crate::{Amount, SignedAmount};
+
+/// Result of a mathematical operation on two numeric types.
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[must_use]
+pub enum NumOpResult<T> {
+    /// Result of a successful mathematical operation.
+    Valid(T),
+    /// Result of an unsuccessful mathematical operation.
+    Error(NumOpError),
+}
+
+impl<T: fmt::Debug> NumOpResult<T> {
+    /// Returns the contained valid numeric type, consuming `self`.
+    ///
+    /// # Panics
+    ///
+    /// Panics with `msg` if the numeric result is an `Error`.
+    #[inline]
+    #[track_caller]
+    pub fn expect(self, msg: &str) -> T {
+        match self {
+            R::Valid(x) => x,
+            R::Error(_) => panic!("{}", msg),
+        }
+    }
+
+    /// Returns the contained valid numeric type, consuming `self`.
+    ///
+    /// # Panics
+    ///
+    /// Panics if the numeric result is an `Error`.
+    #[inline]
+    #[track_caller]
+    pub fn unwrap(self) -> T {
+        match self {
+            R::Valid(x) => x,
+            R::Error(e) => panic!("tried to unwrap an invalid numeric result: {:?}", e),
+        }
+    }
+
+    /// Returns the contained error, consuming `self`.
+    ///
+    /// # Panics
+    ///
+    /// Panics if the numeric result is valid.
+    #[inline]
+    #[track_caller]
+    pub fn unwrap_err(self) -> NumOpError {
+        match self {
+            R::Error(e) => e,
+            R::Valid(a) => panic!("tried to unwrap a valid numeric result: {:?}", a),
+        }
+    }
+
+    /// Converts this `NumOpResult` to an `Option<T>`.
+    #[inline]
+    pub fn ok(self) -> Option<T> {
+        match self {
+            R::Valid(x) => Some(x),
+            R::Error(_) => None,
+        }
+    }
+
+    /// Converts this `NumOpResult` to a `Result<T, NumOpError>`.
+    #[inline]
+    #[allow(clippy::missing_errors_doc)]
+    pub fn into_result(self) -> Result<T, NumOpError> {
+        match self {
+            R::Valid(x) => Ok(x),
+            R::Error(e) => Err(e),
+        }
+    }
+
+    /// Calls `op` if the numeric result is `Valid`, otherwise returns the `Error` value of `self`.
+    #[inline]
+    pub fn and_then<F>(self, op: F) -> NumOpResult<T>
+    where
+        F: FnOnce(T) -> NumOpResult<T>,
+    {
+        match self {
+            R::Valid(x) => op(x),
+            R::Error(e) => R::Error(e),
+        }
+    }
+
+    /// Returns `true` if the numeric result is valid.
+    #[inline]
+    pub fn is_valid(&self) -> bool {
+        match self {
+            R::Valid(_) => true,
+            R::Error(_) => false,
+        }
+    }
+
+    /// Returns `true` if the numeric result is invalid.
+    #[inline]
+    pub fn is_error(&self) -> bool { !self.is_valid() }
+}
+
+pub(crate) trait OptionExt<T> {
+    fn valid_or_error(self) -> NumOpResult<T>;
+}
+
+impl OptionExt<Amount> for Option<Amount> {
+    #[inline]
+    fn valid_or_error(self) -> NumOpResult<Amount> {
+        match self {
+            Some(amount) => R::Valid(amount),
+            None => R::Error(NumOpError {}),
+        }
+    }
+}
+
+impl OptionExt<SignedAmount> for Option<SignedAmount> {
+    #[inline]
+    fn valid_or_error(self) -> NumOpResult<SignedAmount> {
+        match self {
+            Some(amount) => R::Valid(amount),
+            None => R::Error(NumOpError {}),
+        }
+    }
+}
+
+/// An error occurred while doing a mathematical operation.
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[non_exhaustive]
+pub struct NumOpError;
+
+impl fmt::Display for NumOpError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "a math operation gave an invalid numeric result")
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for NumOpError {}

From 512326b8b997cbdfbbf9746252a2a52264a139f8 Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me@tobin.cc>
Date: Mon, 7 Apr 2025 11:42:03 +1000
Subject: [PATCH 3/6] units: Macroize implementing OptionExt

We are going to add implementations of `OptionExt` for various other
types and all impls are almost identical. To make doing so easier
macroize the implementation for `Amount` and `SignedAmount`.

Internal change only, no logic changes.
---
 units/src/result.rs | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/units/src/result.rs b/units/src/result.rs
index afe268875..0663a6231 100644
--- a/units/src/result.rs
+++ b/units/src/result.rs
@@ -110,25 +110,22 @@ pub(crate) trait OptionExt<T> {
     fn valid_or_error(self) -> NumOpResult<T>;
 }
 
-impl OptionExt<Amount> for Option<Amount> {
-    #[inline]
-    fn valid_or_error(self) -> NumOpResult<Amount> {
-        match self {
-            Some(amount) => R::Valid(amount),
-            None => R::Error(NumOpError {}),
-        }
-    }
-}
-
-impl OptionExt<SignedAmount> for Option<SignedAmount> {
-    #[inline]
-    fn valid_or_error(self) -> NumOpResult<SignedAmount> {
-        match self {
-            Some(amount) => R::Valid(amount),
-            None => R::Error(NumOpError {}),
-        }
+macro_rules! impl_opt_ext {
+    ($($ty:ident),* $(,)?) => {
+        $(
+            impl OptionExt<$ty> for Option<$ty> {
+                #[inline]
+                fn valid_or_error(self) -> NumOpResult<$ty> {
+                    match self {
+                        Some(amount) => R::Valid(amount),
+                        None => R::Error(NumOpError {}),
+                    }
+                }
+            }
+        )*
     }
 }
+impl_opt_ext!(Amount, SignedAmount);
 
 /// An error occurred while doing a mathematical operation.
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]

From dba61c9efecc9c8ce9dd04d4def957a3cf49ad27 Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me@tobin.cc>
Date: Mon, 7 Apr 2025 11:14:10 +1000
Subject: [PATCH 4/6] units: Fix internal docs

The `impl_op_for_references` macro implements an `ops` trait, nothing to
do with opcodes.
---
 units/src/internal_macros.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/units/src/internal_macros.rs b/units/src/internal_macros.rs
index c5f0ac6fc..ee6fc33d6 100644
--- a/units/src/internal_macros.rs
+++ b/units/src/internal_macros.rs
@@ -4,7 +4,7 @@
 //!
 //! Macros meant to be used inside the `bitcoin-units` library.
 
-/// Implements an opcode for various reference combinations.
+/// Implements a mathematical operation for various reference combinations.
 ///
 /// Given `$ty`, assumes the `$op_trait<$other_ty>` trait is implemented on it,
 /// and implements the same trait with the full matrix of `&$ty` and `&$other_ty`:

From 5fb64953c55cc22120471b5a02f6be4a0a2b9415 Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me@tobin.cc>
Date: Mon, 7 Apr 2025 11:29:12 +1000
Subject: [PATCH 5/6] units: Return NumOpResult when implementing Div

Currently we use a std numeric type for the output of various `Div`
implementations while other ops use `NumOpResult`. This makes it
difficult to chain operations.

Throughout the crate use `Output = NumOpResult<Foo>` when implementing
`Div`.

Later we want to enable users differentiating between an overflow and a
div-by-zero. Explicitly do not implement that yet, done separately to
assist review.
---
 units/src/amount/result.rs | 12 ++++++++----
 units/src/amount/tests.rs  | 26 +++++++++++++++-----------
 units/src/fee.rs           | 20 ++++++++++----------
 units/src/result.rs        |  4 ++--
 4 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/units/src/amount/result.rs b/units/src/amount/result.rs
index a0ad26954..ccbc2150d 100644
--- a/units/src/amount/result.rs
+++ b/units/src/amount/result.rs
@@ -83,9 +83,11 @@ crate::internal_macros::impl_op_for_references! {
         fn div(self, rhs: u64) -> Self::Output { self.and_then(|lhs| lhs / rhs) }
     }
     impl ops::Div<Amount> for Amount {
-        type Output = u64;
+        type Output = NumOpResult<u64>;
 
-        fn div(self, rhs: Amount) -> Self::Output { self.to_sat() / rhs.to_sat() }
+        fn div(self, rhs: Amount) -> Self::Output {
+            self.to_sat().checked_div(rhs.to_sat()).valid_or_error()
+        }
     }
 
     impl ops::Rem<u64> for Amount {
@@ -158,9 +160,11 @@ crate::internal_macros::impl_op_for_references! {
         fn div(self, rhs: i64) -> Self::Output { self.and_then(|lhs| lhs / rhs) }
     }
     impl ops::Div<SignedAmount> for SignedAmount {
-        type Output = i64;
+        type Output = NumOpResult<i64>;
 
-        fn div(self, rhs: SignedAmount) -> Self::Output { self.to_sat() / rhs.to_sat() }
+        fn div(self, rhs: SignedAmount) -> Self::Output {
+            self.to_sat().checked_div(rhs.to_sat()).valid_or_error()
+        }
     }
 
     impl ops::Rem<i64> for SignedAmount {
diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs
index 2dbefee14..a2a3bec07 100644
--- a/units/src/amount/tests.rs
+++ b/units/src/amount/tests.rs
@@ -1231,27 +1231,31 @@ fn op_int_combos() {
 
 #[test]
 fn unsigned_amount_div_by_amount() {
-    assert_eq!(sat(0) / sat(7), 0);
-    assert_eq!(sat(1897) / sat(7), 271);
+    assert_eq!((sat(0) / sat(7)).unwrap(), 0);
+    assert_eq!((sat(1897) / sat(7)).unwrap(), 271);
 }
 
 #[test]
-#[should_panic(expected = "attempt to divide by zero")]
-fn unsigned_amount_div_by_amount_zero() { let _ = sat(1897) / Amount::ZERO; }
+fn unsigned_amount_div_by_amount_zero() {
+    let res = sat(1897) / Amount::ZERO;
+    assert!(res.into_result().is_err());
+}
 
 #[test]
 fn signed_amount_div_by_amount() {
-    assert_eq!(ssat(0) / ssat(7), 0);
+    assert_eq!((ssat(0) / ssat(7)).unwrap(), 0);
 
-    assert_eq!(ssat(1897) / ssat(7), 271);
-    assert_eq!(ssat(1897) / ssat(-7), -271);
-    assert_eq!(ssat(-1897) / ssat(7), -271);
-    assert_eq!(ssat(-1897) / ssat(-7), 271);
+    assert_eq!((ssat(1897) / ssat(7)).unwrap(), 271);
+    assert_eq!((ssat(1897) / ssat(-7)).unwrap(), -271);
+    assert_eq!((ssat(-1897) / ssat(7)).unwrap(), -271);
+    assert_eq!((ssat(-1897) / ssat(-7)).unwrap(), 271);
 }
 
 #[test]
-#[should_panic(expected = "attempt to divide by zero")]
-fn signed_amount_div_by_amount_zero() { let _ = ssat(1897) / SignedAmount::ZERO; }
+fn signed_amount_div_by_amount_zero() {
+    let res = ssat(1897) / SignedAmount::ZERO;
+    assert!(res.into_result().is_err());
+}
 
 #[test]
 fn check_const() {
diff --git a/units/src/fee.rs b/units/src/fee.rs
index 5d277111e..e01f31571 100644
--- a/units/src/fee.rs
+++ b/units/src/fee.rs
@@ -178,18 +178,18 @@ crate::internal_macros::impl_op_for_references! {
     }
 
     impl ops::Div<Weight> for Amount {
-        type Output = FeeRate;
+        type Output = NumOpResult<FeeRate>;
 
         fn div(self, rhs: Weight) -> Self::Output {
-            FeeRate::from_sat_per_kwu(self.to_sat() * 1000 / rhs.to_wu())
+            self.checked_div_by_weight_floor(rhs).valid_or_error()
         }
     }
 
     impl ops::Div<FeeRate> for Amount {
-        type Output = Weight;
+        type Output = NumOpResult<Weight>;
 
         fn div(self, rhs: FeeRate) -> Self::Output {
-            self.checked_div_by_fee_rate_floor(rhs).unwrap()
+            self.checked_div_by_fee_rate_floor(rhs).valid_or_error()
         }
     }
 }
@@ -215,7 +215,7 @@ mod tests {
 
     #[test]
     fn fee_rate_div_by_weight() {
-        let fee_rate = Amount::from_sat_u32(329) / Weight::from_wu(381);
+        let fee_rate = (Amount::from_sat_u32(329) / Weight::from_wu(381)).unwrap();
         assert_eq!(fee_rate, FeeRate::from_sat_per_kwu(863));
     }
 
@@ -275,21 +275,21 @@ mod tests {
         // Test exact division
         let amount = Amount::from_sat_u32(1000);
         let fee_rate = FeeRate::from_sat_per_kwu(2);
-        let weight = amount / fee_rate;
+        let weight = (amount / fee_rate).unwrap();
         assert_eq!(weight, Weight::from_wu(500_000));
 
         // Test reference division
-        let weight_ref = &amount / fee_rate;
+        let weight_ref = (&amount / fee_rate).unwrap();
         assert_eq!(weight_ref, Weight::from_wu(500_000));
-        let weight_ref2 = amount / &fee_rate;
+        let weight_ref2 = (amount / &fee_rate).unwrap();
         assert_eq!(weight_ref2, Weight::from_wu(500_000));
-        let weight_ref3 = &amount / &fee_rate;
+        let weight_ref3 = (&amount / &fee_rate).unwrap();
         assert_eq!(weight_ref3, Weight::from_wu(500_000));
 
         // Test truncation behavior
         let amount = Amount::from_sat_u32(1000);
         let fee_rate = FeeRate::from_sat_per_kwu(3);
-        let weight = amount / fee_rate;
+        let weight = (amount / fee_rate).unwrap();
         // 1000 * 1000 = 1,000,000 msats
         // 1,000,000 / 3 = 333,333.33... wu
         // Should truncate down to 333,333 wu
diff --git a/units/src/result.rs b/units/src/result.rs
index 0663a6231..3cf626e40 100644
--- a/units/src/result.rs
+++ b/units/src/result.rs
@@ -6,7 +6,7 @@ use core::fmt;
 
 use NumOpResult as R;
 
-use crate::{Amount, SignedAmount};
+use crate::{Amount, FeeRate, SignedAmount, Weight};
 
 /// Result of a mathematical operation on two numeric types.
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
@@ -125,7 +125,7 @@ macro_rules! impl_opt_ext {
         )*
     }
 }
-impl_opt_ext!(Amount, SignedAmount);
+impl_opt_ext!(Amount, SignedAmount, u64, i64, FeeRate, Weight);
 
 /// An error occurred while doing a mathematical operation.
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]

From d6881ff5f861b8a85883ec88ce453cc02191ba14 Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me@tobin.cc>
Date: Mon, 7 Apr 2025 12:12:20 +1000
Subject: [PATCH 6/6] 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<Amount> for NumOpResult<Amount> {
     fn from(a: Amount) -> Self { Self::Valid(a) }
@@ -27,7 +27,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() }
+        fn add(self, rhs: Amount) -> Self::Output { self.checked_add(rhs).valid_or_error(MathOp::Add) }
     }
     impl ops::Add<NumOpResult<Amount>> for Amount {
         type Output = NumOpResult<Amount>;
@@ -38,7 +38,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() }
+        fn sub(self, rhs: Amount) -> Self::Output { self.checked_sub(rhs).valid_or_error(MathOp::Sub) }
     }
     impl ops::Sub<NumOpResult<Amount>> for Amount {
         type Output = NumOpResult<Amount>;
@@ -54,7 +54,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() }
+        fn mul(self, rhs: u64) -> Self::Output { self.checked_mul(rhs).valid_or_error(MathOp::Mul) }
     }
     impl ops::Mul<u64> for NumOpResult<Amount> {
         type Output = NumOpResult<Amount>;
@@ -64,7 +64,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() }
+        fn mul(self, rhs: Amount) -> Self::Output { rhs.checked_mul(self).valid_or_error(MathOp::Mul) }
     }
     impl ops::Mul<NumOpResult<Amount>> for u64 {
         type Output = NumOpResult<Amount>;
@@ -75,7 +75,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() }
+        fn div(self, rhs: u64) -> Self::Output { self.checked_div(rhs).valid_or_error(MathOp::Div) }
     }
     impl ops::Div<u64> for NumOpResult<Amount> {
         type Output = NumOpResult<Amount>;
@@ -86,14 +86,14 @@ 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()
+            self.to_sat().checked_div(rhs.to_sat()).valid_or_error(MathOp::Div)
         }
     }
 
     impl ops::Rem<u64> for Amount {
         type Output = NumOpResult<Amount>;
 
-        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<u64> for NumOpResult<Amount> {
         type Output = NumOpResult<Amount>;
@@ -104,7 +104,7 @@ 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() }
+        fn add(self, rhs: SignedAmount) -> Self::Output { self.checked_add(rhs).valid_or_error(MathOp::Add) }
     }
     impl ops::Add<NumOpResult<SignedAmount>> for SignedAmount {
         type Output = NumOpResult<SignedAmount>;
@@ -115,7 +115,7 @@ 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() }
+        fn sub(self, rhs: SignedAmount) -> Self::Output { self.checked_sub(rhs).valid_or_error(MathOp::Sub) }
     }
     impl ops::Sub<NumOpResult<SignedAmount>> for SignedAmount {
         type Output = NumOpResult<SignedAmount>;
@@ -131,7 +131,7 @@ 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() }
+        fn mul(self, rhs: i64) -> Self::Output { self.checked_mul(rhs).valid_or_error(MathOp::Mul) }
     }
     impl ops::Mul<i64> for NumOpResult<SignedAmount> {
         type Output = NumOpResult<SignedAmount>;
@@ -141,7 +141,7 @@ 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() }
+        fn mul(self, rhs: SignedAmount) -> Self::Output { rhs.checked_mul(self).valid_or_error(MathOp::Mul) }
     }
     impl ops::Mul<NumOpResult<SignedAmount>> for i64 {
         type Output = NumOpResult<SignedAmount>;
@@ -152,7 +152,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() }
+        fn div(self, rhs: i64) -> Self::Output { self.checked_div(rhs).valid_or_error(MathOp::Div) }
     }
     impl ops::Div<i64> for NumOpResult<SignedAmount> {
         type Output = NumOpResult<SignedAmount>;
@@ -163,14 +163,14 @@ 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()
+            self.to_sat().checked_div(rhs.to_sat()).valid_or_error(MathOp::Div)
         }
     }
 
     impl ops::Rem<i64> for SignedAmount {
         type Output = NumOpResult<SignedAmount>;
 
-        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<i64> for NumOpResult<SignedAmount> {
         type Output = NumOpResult<SignedAmount>;
@@ -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<NumOpResult<Amount>> for NumOpResult<Amount> {
     {
         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<Amount>> for NumOpResult<Amount> {
     {
         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<NumOpResult<SignedAmount>> for NumOpResult<SignedAmount> {
     {
         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<SignedAmount>> for NumOpResult<SignedAm
     {
         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)),
         })
     }
 }
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<FeeRate> for Weight {
         type Output = NumOpResult<Amount>;
         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<Weight> for FeeRate {
         type Output = NumOpResult<Amount>;
         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<FeeRate>;
 
         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<Weight>;
 
         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<T: fmt::Debug> NumOpResult<T> {
 }
 
 pub(crate) trait OptionExt<T> {
-    fn valid_or_error(self) -> NumOpResult<T>;
+    fn valid_or_error(self, op: MathOp) -> NumOpResult<T>;
 }
 
 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"),
+        }
+    }
+}