Repository: rust-bitcoin/rust-bitcoin
Pull request #4076 (2025-02-18 18:45:21)

units: macroize the op implementations
This introduces a general macro which takes some number of `impl ops::Whatever<A> for <B>` and replicates them with all the permutations of references. It also takes a syntax which resembles the code for the initial impl block.

Uses it for all the binary opcodes on `Amount`, `SignedAmount`, `Weight`, `FeeRate`, as well as the numeric mul/div on those types, as well as generic impls on `NumResultOp<T>` (which are expanded to cover more cases).

coveralls: ## Pull Request Test Coverage Report for [Build 13397854829](https://coveralls.io/builds/72301439)


### Details

* **39** of **40**   **(97.5%)**  changed or added relevant lines in **3** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage increased (+**0.1%**) to **82.875%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [units/src/amount/result.rs](https://coveralls.io/builds/72301439/source?filename=units%2Fsrc%2Famount%2Fresult.rs#L181) | 1 | 2 | 50.0%
<!-- | **Total:** | **39** | **40** | **97.5%** | -->


|  Totals | [![Coverage Status](https://coveralls.io/builds/72301439/badge)](https://coveralls.io/builds/72301439) |
| :-- | --: |
| Change from base [Build 13392597058](https://coveralls.io/builds/72295742): |  0.1% |
| Covered Lines: | 21139 |
| Relevant Lines: | 25507 |

---
##### 💛  - [Coveralls](https://coveralls.io)

tcharding: I grabbed the test updates out of #4056. Raised https://github.com/apoelstra/rust-bitcoin/pull/1 for you to see if you want it.
apoelstra: Nice, thanks! I think it'd be fine if you just pushed that commit onto my PR.

If you wanna be nice, I'd prefer if you split it into two -- one that adds the impls and one that adds the tests.
tcharding: So red now GitHub, you are stressing me out unnecessarily.
tcharding: Found another cut'n'pasta bug.

tcharding: Just flagging that I should have put the `self - rhs` change in a separate patch not in c47de029. We have two acks now though so not changing.
apoelstra: I think there's just one ACK -- my comment is not an ACK since it's my PR. If Kix doesn't jump in, I'll try to clean up this PR a bit in the next 24 hours (to split out your commits and maybe merge parts into my own commits).
tcharding: Ah woops, you right. It was a bit weird acking my own commits too.
tcharding: lol, acking my own broken commits.
tcharding: Have you got time to push the fixes @apoelstra? This PR is holding up the MAX_MONEY one which is dragging on forever. I can do it if not, didn't want to force push to your branch without asking.
Kixunil: Oh, crap I haven't found the time to review this either. Hopefully tomorrow.
apoelstra: >I can do it if not, didn't want to force push to your branch without asking.

Yes, please do. No, I have not found the time to do it.

Patch:
diff --git a/units/src/amount/result.rs b/units/src/amount/result.rs
index 645b3edd..72423083 100644
--- a/units/src/amount/result.rs
+++ b/units/src/amount/result.rs
@@ -12,7 +12,7 @@
 /// Result of an operation on [`Amount`] or [`SignedAmount`].
 ///
 /// The type parameter `T` should be normally `Amout` or `SignedAmount`.
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
 #[must_use]
 pub enum NumOpResult<T> {
     /// Result of a successful mathematical operation.
@@ -123,282 +123,177 @@ impl From<&SignedAmount> for NumOpResult<SignedAmount> {
     fn from(a: &SignedAmount) -> Self { Self::Valid(*a) }
 }
 
-impl ops::Add for Amount {
-    type Output = NumOpResult<Amount>;
+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() }
-}
-crate::internal_macros::impl_add_for_amount_references!(Amount);
-
-impl ops::Add<NumOpResult<Amount>> for Amount {
-    type Output = NumOpResult<Amount>;
-
-    fn add(self, rhs: NumOpResult<Amount>) -> Self::Output { rhs.and_then(|a| a + self) }
-}
-impl ops::Add<NumOpResult<Amount>> for &Amount {
-    type Output = NumOpResult<Amount>;
-
-    fn add(self, rhs: NumOpResult<Amount>) -> Self::Output { rhs.and_then(|a| a + self) }
-}
-impl ops::Add<Amount> for NumOpResult<Amount> {
-    type Output = NumOpResult<Amount>;
-
-    fn add(self, rhs: Amount) -> Self::Output { rhs + self }
-}
-impl ops::Add<&Amount> for NumOpResult<Amount> {
-    type Output = NumOpResult<Amount>;
-
-    fn add(self, rhs: &Amount) -> Self::Output { rhs + self }
-}
-
-impl ops::Sub for Amount {
-    type Output = NumOpResult<Amount>;
-
-    fn sub(self, rhs: Amount) -> Self::Output { self.checked_sub(rhs).valid_or_error() }
-}
-crate::internal_macros::impl_sub_for_amount_references!(Amount);
-
-impl ops::Sub<NumOpResult<Amount>> for Amount {
-    type Output = NumOpResult<Amount>;
-
-    fn sub(self, rhs: NumOpResult<Amount>) -> Self::Output {
-        match rhs {
-            R::Valid(amount) => self - amount,
-            R::Error(_) => rhs,
-        }
+        fn add(self, rhs: Amount) -> Self::Output { self.checked_add(rhs).valid_or_error() }
     }
-}
-impl ops::Sub<NumOpResult<Amount>> for &Amount {
-    type Output = NumOpResult<Amount>;
+    impl ops::Add<NumOpResult<Amount>> for Amount {
+        type Output = NumOpResult<Amount>;
 
-    fn sub(self, rhs: NumOpResult<Amount>) -> Self::Output {
-        match rhs {
-            R::Valid(amount) => self - amount,
-            R::Error(_) => rhs,
-        }
+        fn add(self, rhs: NumOpResult<Amount>) -> Self::Output { rhs.and_then(|a| a + self) }
     }
-}
-impl ops::Sub<Amount> for NumOpResult<Amount> {
-    type Output = NumOpResult<Amount>;
 
-    fn sub(self, rhs: Amount) -> Self::Output {
-        match self {
-            R::Valid(amount) => amount - rhs,
-            R::Error(_) => self,
-        }
-    }
-}
-impl ops::Sub<&Amount> for NumOpResult<Amount> {
-    type Output = NumOpResult<Amount>;
+    impl ops::Sub<Amount> for Amount {
+        type Output = NumOpResult<Amount>;
 
-    fn sub(self, rhs: &Amount) -> Self::Output {
-        match self {
-            R::Valid(amount) => amount - (*rhs),
-            R::Error(_) => self,
+        fn sub(self, rhs: Amount) -> Self::Output { self.checked_sub(rhs).valid_or_error() }
+    }
+    impl ops::Sub<NumOpResult<Amount>> for Amount {
+        type Output = NumOpResult<Amount>;
+
+        fn sub(self, rhs: NumOpResult<Amount>) -> Self::Output {
+            match rhs {
+                R::Valid(amount) => self - amount,
+                R::Error(_) => rhs,
+            }
         }
     }
-}
-
-impl ops::Mul<u64> for Amount {
-    type Output = NumOpResult<Amount>;
-
-    fn mul(self, rhs: u64) -> Self::Output { self.checked_mul(rhs).valid_or_error() }
-}
-impl ops::Mul<&u64> for Amount {
-    type Output = NumOpResult<Amount>;
-
-    fn mul(self, rhs: &u64) -> Self::Output { self.mul(*rhs) }
-}
-impl ops::Mul<u64> for &Amount {
-    type Output = NumOpResult<Amount>;
-
-    fn mul(self, rhs: u64) -> Self::Output { (*self).mul(rhs) }
-}
-impl ops::Mul<&u64> for &Amount {
-    type Output = NumOpResult<Amount>;
-
-    fn mul(self, rhs: &u64) -> Self::Output { self.mul(*rhs) }
-}
-
-impl ops::Div<u64> for Amount {
-    type Output = NumOpResult<Amount>;
 
-    fn div(self, rhs: u64) -> Self::Output { self.checked_div(rhs).valid_or_error() }
-}
-impl ops::Div<&u64> for Amount {
-    type Output = NumOpResult<Amount>;
-
-    fn div(self, rhs: &u64) -> Self::Output { self.div(*rhs) }
-}
-impl ops::Div<u64> for &Amount {
-    type Output = NumOpResult<Amount>;
-
-    fn div(self, rhs: u64) -> Self::Output { (*self).div(rhs) }
-}
-impl ops::Div<&u64> for &Amount {
-    type Output = NumOpResult<Amount>;
-
-    fn div(self, rhs: &u64) -> Self::Output { (*self).div(*rhs) }
-}
-
-impl ops::Rem<u64> for Amount {
-    type Output = NumOpResult<Amount>;
-
-    fn rem(self, modulus: u64) -> Self::Output { self.checked_rem(modulus).valid_or_error() }
-}
-impl ops::Rem<&u64> for Amount {
-    type Output = NumOpResult<Amount>;
-
-    fn rem(self, modulus: &u64) -> Self::Output { self.rem(*modulus) }
-}
-impl ops::Rem<u64> for &Amount {
-    type Output = NumOpResult<Amount>;
+    impl ops::Mul<u64> for Amount {
+        type Output = NumOpResult<Amount>;
 
-    fn rem(self, modulus: u64) -> Self::Output { (*self).rem(modulus) }
-}
-impl ops::Rem<&u64> for &Amount {
-    type Output = NumOpResult<Amount>;
+        fn mul(self, rhs: u64) -> Self::Output { self.checked_mul(rhs).valid_or_error() }
+    }
+    impl ops::Mul<u64> for NumOpResult<Amount> {
+        type Output = NumOpResult<Amount>;
 
-    fn rem(self, modulus: &u64) -> Self::Output { (*self).rem(*modulus) }
-}
+        fn mul(self, rhs: u64) -> Self::Output { self.and_then(|lhs| lhs * rhs) }
+    }
 
-impl ops::Add for SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+    impl ops::Div<u64> for Amount {
+        type Output = NumOpResult<Amount>;
 
-    fn add(self, rhs: SignedAmount) -> Self::Output { self.checked_add(rhs).valid_or_error() }
-}
-crate::internal_macros::impl_add_for_amount_references!(SignedAmount);
+        fn div(self, rhs: u64) -> Self::Output { self.checked_div(rhs).valid_or_error() }
+    }
+    impl ops::Div<u64> for NumOpResult<Amount> {
+        type Output = NumOpResult<Amount>;
 
-impl ops::Add<NumOpResult<SignedAmount>> for SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+        fn div(self, rhs: u64) -> Self::Output { self.and_then(|lhs| lhs / rhs) }
+    }
 
-    fn add(self, rhs: NumOpResult<SignedAmount>) -> Self::Output { rhs.and_then(|a| a + self) }
-}
-impl ops::Add<NumOpResult<SignedAmount>> for &SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+    impl ops::Rem<u64> for Amount {
+        type Output = NumOpResult<Amount>;
 
-    fn add(self, rhs: NumOpResult<SignedAmount>) -> Self::Output { rhs.and_then(|a| a + self) }
-}
-impl ops::Add<SignedAmount> for NumOpResult<SignedAmount> {
-    type Output = NumOpResult<SignedAmount>;
+        fn rem(self, modulus: u64) -> Self::Output { self.checked_rem(modulus).valid_or_error() }
+    }
+    impl ops::Rem<u64> for NumOpResult<Amount> {
+        type Output = NumOpResult<Amount>;
 
-    fn add(self, rhs: SignedAmount) -> Self::Output { rhs + self }
-}
-impl ops::Add<&SignedAmount> for NumOpResult<SignedAmount> {
-    type Output = NumOpResult<SignedAmount>;
+        fn rem(self, modulus: u64) -> Self::Output { self.and_then(|lhs| lhs % modulus) }
+    }
 
-    fn add(self, rhs: &SignedAmount) -> Self::Output { rhs + self }
-}
+    impl ops::Add<SignedAmount> for SignedAmount {
+        type Output = NumOpResult<SignedAmount>;
 
-impl ops::Sub for SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+        fn add(self, rhs: SignedAmount) -> Self::Output { self.checked_add(rhs).valid_or_error() }
+    }
+    impl ops::Add<NumOpResult<SignedAmount>> for SignedAmount {
+        type Output = NumOpResult<SignedAmount>;
 
-    fn sub(self, rhs: SignedAmount) -> Self::Output { self.checked_sub(rhs).valid_or_error() }
-}
-crate::internal_macros::impl_sub_for_amount_references!(SignedAmount);
+        fn add(self, rhs: NumOpResult<SignedAmount>) -> Self::Output { rhs.and_then(|a| a + self) }
+    }
 
-impl ops::Sub<NumOpResult<SignedAmount>> for SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+    impl ops::Sub<SignedAmount> for SignedAmount {
+        type Output = NumOpResult<SignedAmount>;
 
-    fn sub(self, rhs: NumOpResult<SignedAmount>) -> Self::Output {
-        match rhs {
-            R::Valid(amount) => amount - rhs,
-            R::Error(_) => rhs,
-        }
+        fn sub(self, rhs: SignedAmount) -> Self::Output { self.checked_sub(rhs).valid_or_error() }
     }
-}
-impl ops::Sub<NumOpResult<SignedAmount>> for &SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
-
-    fn sub(self, rhs: NumOpResult<SignedAmount>) -> Self::Output {
-        match rhs {
-            R::Valid(amount) => amount - rhs,
-            R::Error(_) => rhs,
+    impl ops::Sub<NumOpResult<SignedAmount>> for SignedAmount {
+        type Output = NumOpResult<SignedAmount>;
+
+        fn sub(self, rhs: NumOpResult<SignedAmount>) -> Self::Output {
+            match rhs {
+                R::Valid(amount) => self - amount,
+                R::Error(_) => rhs,
+            }
         }
     }
-}
-impl ops::Sub<SignedAmount> for NumOpResult<SignedAmount> {
-    type Output = NumOpResult<SignedAmount>;
 
-    fn sub(self, rhs: SignedAmount) -> Self::Output {
-        match self {
-            R::Valid(amount) => amount - rhs,
-            R::Error(_) => self,
-        }
-    }
-}
-impl ops::Sub<&SignedAmount> for NumOpResult<SignedAmount> {
-    type Output = NumOpResult<SignedAmount>;
+    impl ops::Mul<i64> for SignedAmount {
+        type Output = NumOpResult<SignedAmount>;
 
-    fn sub(self, rhs: &SignedAmount) -> Self::Output {
-        match self {
-            R::Valid(amount) => amount - *rhs,
-            R::Error(_) => self,
-        }
+        fn mul(self, rhs: i64) -> Self::Output { self.checked_mul(rhs).valid_or_error() }
     }
-}
+    impl ops::Mul<i64> for NumOpResult<SignedAmount> {
+        type Output = NumOpResult<SignedAmount>;
 
-impl ops::Mul<i64> for SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+        fn mul(self, rhs: i64) -> Self::Output { self.and_then(|lhs| lhs * rhs) }
+    }
 
-    fn mul(self, rhs: i64) -> Self::Output { self.checked_mul(rhs).valid_or_error() }
-}
-impl ops::Mul<&i64> for SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+    impl ops::Div<i64> for SignedAmount {
+        type Output = NumOpResult<SignedAmount>;
 
-    fn mul(self, rhs: &i64) -> Self::Output { self.mul(*rhs) }
-}
-impl ops::Mul<i64> for &SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+        fn div(self, rhs: i64) -> Self::Output { self.checked_div(rhs).valid_or_error() }
+    }
+    impl ops::Div<i64> for NumOpResult<SignedAmount> {
+        type Output = NumOpResult<SignedAmount>;
 
-    fn mul(self, rhs: i64) -> Self::Output { (*self).mul(rhs) }
-}
-impl ops::Mul<&i64> for &SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+        fn div(self, rhs: i64) -> Self::Output { self.and_then(|lhs| lhs / rhs) }
+    }
 
-    fn mul(self, rhs: &i64) -> Self::Output { self.mul(*rhs) }
-}
+    impl ops::Rem<i64> for SignedAmount {
+        type Output = NumOpResult<SignedAmount>;
 
-impl ops::Div<i64> for SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+        fn rem(self, modulus: i64) -> Self::Output { self.checked_rem(modulus).valid_or_error() }
+    }
+    impl ops::Rem<i64> for NumOpResult<SignedAmount> {
+        type Output = NumOpResult<SignedAmount>;
 
-    fn div(self, rhs: i64) -> Self::Output { self.checked_div(rhs).valid_or_error() }
-}
-impl ops::Div<&i64> for SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+        fn rem(self, modulus: i64) -> Self::Output { self.and_then(|lhs| lhs % modulus) }
+    }
 
-    fn div(self, rhs: &i64) -> Self::Output { self.div(*rhs) }
-}
-impl ops::Div<i64> for &SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+    impl<T> ops::Add<NumOpResult<T>> for NumOpResult<T>
+    where
+        (T: Copy + ops::Add<Output = NumOpResult<T>>)
+    {
+        type Output = NumOpResult<T>;
 
-    fn div(self, rhs: i64) -> Self::Output { (*self).div(rhs) }
-}
-impl ops::Div<&i64> for &SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+        fn add(self, rhs: Self) -> Self::Output {
+            match (self, rhs) {
+                (R::Valid(lhs), R::Valid(rhs)) => lhs + rhs,
+                (_, _) => R::Error(NumOpError {}),
+            }
+        }
+    }
 
-    fn div(self, rhs: &i64) -> Self::Output { (*self).div(*rhs) }
-}
+    impl<T> ops::Add<T> for NumOpResult<T>
+    where
+        (T: Copy + ops::Add<NumOpResult<T>, Output = NumOpResult<T>>)
+    {
+        type Output = NumOpResult<T>;
 
-impl ops::Rem<i64> for SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+        fn add(self, rhs: T) -> Self::Output { rhs + self }
+    }
 
-    fn rem(self, modulus: i64) -> Self::Output { self.checked_rem(modulus).valid_or_error() }
-}
-impl ops::Rem<&i64> for SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+    impl<T> ops::Sub<NumOpResult<T>> for NumOpResult<T>
+    where
+        (T: Copy + ops::Sub<Output = NumOpResult<T>>)
+    {
+        type Output = NumOpResult<T>;
 
-    fn rem(self, modulus: &i64) -> Self::Output { self.rem(*modulus) }
-}
-impl ops::Rem<i64> for &SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+        fn sub(self, rhs: Self) -> Self::Output {
+            match (self, rhs) {
+                (R::Valid(lhs), R::Valid(rhs)) => lhs - rhs,
+                (_, _) => R::Error(NumOpError {}),
+            }
+        }
+    }
 
-    fn rem(self, modulus: i64) -> Self::Output { (*self).rem(modulus) }
-}
-impl ops::Rem<&i64> for &SignedAmount {
-    type Output = NumOpResult<SignedAmount>;
+    impl<T> ops::Sub<T> for NumOpResult<T>
+    where
+        (T: Copy + ops::Sub<Output = NumOpResult<T>>)
+    {
+        type Output = NumOpResult<T>;
 
-    fn rem(self, modulus: &i64) -> Self::Output { (*self).rem(*modulus) }
+        fn sub(self, rhs: T) -> Self::Output {
+            match self {
+                R::Valid(amount) => amount - rhs,
+                R::Error(_) => self,
+            }
+        }
+    }
 }
 
 impl ops::Neg for SignedAmount {
@@ -407,112 +302,6 @@ impl ops::Neg for SignedAmount {
     fn neg(self) -> Self::Output { Self::from_sat(self.to_sat().neg()) }
 }
 
-impl<T> ops::Add for NumOpResult<T>
-where
-    T: ops::Add<Output = NumOpResult<T>>,
-{
-    type Output = NumOpResult<T>;
-
-    fn add(self, rhs: Self) -> Self::Output {
-        match (self, rhs) {
-            (R::Valid(lhs), R::Valid(rhs)) => lhs + rhs,
-            (_, _) => R::Error(NumOpError {}),
-        }
-    }
-}
-impl<T> ops::Add<NumOpResult<T>> for &NumOpResult<T>
-where
-    T: ops::Add<Output = NumOpResult<T>> + Copy,
-{
-    type Output = NumOpResult<T>;
-
-    fn add(self, rhs: NumOpResult<T>) -> Self::Output {
-        match (self, rhs) {
-            (R::Valid(lhs), R::Valid(rhs)) => *lhs + rhs,
-            (_, _) => R::Error(NumOpError {}),
-        }
-    }
-}
-impl<T> ops::Add<&NumOpResult<T>> for NumOpResult<T>
-where
-    T: ops::Add<Output = NumOpResult<T>> + Copy,
-{
-    type Output = NumOpResult<T>;
-
-    fn add(self, rhs: &NumOpResult<T>) -> Self::Output {
-        match (self, rhs) {
-            (R::Valid(lhs), R::Valid(rhs)) => lhs + *rhs,
-            (_, _) => R::Error(NumOpError {}),
-        }
-    }
-}
-impl<T> ops::Add for &NumOpResult<T>
-where
-    T: ops::Add<Output = NumOpResult<T>> + Copy,
-{
-    type Output = NumOpResult<T>;
-
-    fn add(self, rhs: &NumOpResult<T>) -> Self::Output {
-        match (self, rhs) {
-            (R::Valid(lhs), R::Valid(rhs)) => *lhs + *rhs,
-            (_, _) => R::Error(NumOpError {}),
-        }
-    }
-}
-
-impl<T> ops::Sub for NumOpResult<T>
-where
-    T: ops::Sub<Output = NumOpResult<T>>,
-{
-    type Output = NumOpResult<T>;
-
-    fn sub(self, rhs: Self) -> Self::Output {
-        match (self, rhs) {
-            (R::Valid(lhs), R::Valid(rhs)) => lhs - rhs,
-            (_, _) => R::Error(NumOpError {}),
-        }
-    }
-}
-impl<T> ops::Sub<NumOpResult<T>> for &NumOpResult<T>
-where
-    T: ops::Sub<Output = NumOpResult<T>> + Copy,
-{
-    type Output = NumOpResult<T>;
-
-    fn sub(self, rhs: NumOpResult<T>) -> Self::Output {
-        match (self, rhs) {
-            (R::Valid(lhs), R::Valid(rhs)) => *lhs - rhs,
-            (_, _) => R::Error(NumOpError {}),
-        }
-    }
-}
-impl<T> ops::Sub<&NumOpResult<T>> for NumOpResult<T>
-where
-    T: ops::Sub<Output = NumOpResult<T>> + Copy,
-{
-    type Output = NumOpResult<T>;
-
-    fn sub(self, rhs: &NumOpResult<T>) -> Self::Output {
-        match (self, rhs) {
-            (R::Valid(lhs), R::Valid(rhs)) => lhs - *rhs,
-            (_, _) => R::Error(NumOpError {}),
-        }
-    }
-}
-impl<T> ops::Sub for &NumOpResult<T>
-where
-    T: ops::Sub<Output = NumOpResult<T>> + Copy,
-{
-    type Output = NumOpResult<T>;
-
-    fn sub(self, rhs: Self) -> Self::Output {
-        match (self, rhs) {
-            (R::Valid(lhs), R::Valid(rhs)) => *lhs - *rhs,
-            (_, _) => R::Error(NumOpError {}),
-        }
-    }
-}
-
 impl core::iter::Sum<NumOpResult<Amount>> for NumOpResult<Amount> {
     fn sum<I>(iter: I) -> Self
     where
@@ -584,7 +373,7 @@ fn valid_or_error(self) -> NumOpResult<SignedAmount> {
 }
 
 /// An error occurred while doing a mathematical operation.
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq)]
 #[non_exhaustive]
 pub struct NumOpError;
 
diff --git a/units/src/fee_rate/mod.rs b/units/src/fee_rate/mod.rs
index f9dca7b8..f3ec8fd7 100644
--- a/units/src/fee_rate/mod.rs
+++ b/units/src/fee_rate/mod.rs
@@ -135,20 +135,20 @@ impl From<FeeRate> for u64 {
     fn from(value: FeeRate) -> Self { value.to_sat_per_kwu() }
 }
 
-impl ops::Add for FeeRate {
-    type Output = FeeRate;
+crate::internal_macros::impl_op_for_references! {
+    impl ops::Add<FeeRate> for FeeRate {
+        type Output = FeeRate;
 
-    fn add(self, rhs: FeeRate) -> Self::Output { FeeRate(self.0 + rhs.0) }
-}
-crate::internal_macros::impl_add_for_references!(FeeRate);
-crate::internal_macros::impl_add_assign!(FeeRate);
+        fn add(self, rhs: FeeRate) -> Self::Output { FeeRate(self.0 + rhs.0) }
+    }
 
-impl ops::Sub for FeeRate {
-    type Output = FeeRate;
+    impl ops::Sub<FeeRate> for FeeRate {
+        type Output = FeeRate;
 
-    fn sub(self, rhs: FeeRate) -> Self::Output { FeeRate(self.0 - rhs.0) }
+        fn sub(self, rhs: FeeRate) -> Self::Output { FeeRate(self.0 - rhs.0) }
+    }
 }
-crate::internal_macros::impl_sub_for_references!(FeeRate);
+crate::internal_macros::impl_add_assign!(FeeRate);
 crate::internal_macros::impl_sub_assign!(FeeRate);
 
 impl core::iter::Sum for FeeRate {
diff --git a/units/src/internal_macros.rs b/units/src/internal_macros.rs
index 1bae6898..c5f0ac6f 100644
--- a/units/src/internal_macros.rs
+++ b/units/src/internal_macros.rs
@@ -4,65 +4,70 @@
 //!
 //! Macros meant to be used inside the `bitcoin-units` library.
 
-/// Implements `ops::Add` for various references.
+/// Implements an opcode for various reference combinations.
 ///
-/// Requires `$ty` it implement `Add` e.g. 'impl Add<T> for T'. Adds impls of:
+/// 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`:
 ///
-/// - Add<T> for &T
-/// - Add<&T> for T
-/// - Add<&T> for &T
-macro_rules! impl_add_for_references {
-    ($ty:ident) => {
-        impl core::ops::Add<$ty> for &$ty {
-            type Output = $ty;
-
-            fn add(self, rhs: $ty) -> Self::Output { *self + rhs }
-        }
-
-        impl core::ops::Add<&$ty> for $ty {
-            type Output = $ty;
-
-            fn add(self, rhs: &$ty) -> Self::Output { self + *rhs }
-        }
-
-        impl<'a> core::ops::Add<&'a $ty> for &$ty {
-            type Output = $ty;
-
-            fn add(self, rhs: &'a $ty) -> Self::Output { *self + *rhs }
-        }
-    };
-}
-pub(crate) use impl_add_for_references;
-
-/// Implements `ops::Add` for various amount references.
+/// - `Add<$other_ty> for &$ty`
+/// - `Add<&$other_ty> for $ty`
+/// - `Add<&$other_ty> for &$ty`
 ///
-/// Requires `$ty` it implement `Add` e.g. 'impl Add<T> for T'. Adds impls of:
+/// # Limitations
 ///
-/// - Add<T> for &T
-/// - Add<&T> for T
-/// - Add<&T> for &T
-macro_rules! impl_add_for_amount_references {
-    ($ty:ident) => {
-        impl core::ops::Add<$ty> for &$ty {
-            type Output = NumOpResult<$ty>;
-
-            fn add(self, rhs: $ty) -> Self::Output { *self + rhs }
-        }
-
-        impl core::ops::Add<&$ty> for $ty {
-            type Output = NumOpResult<$ty>;
-
-            fn add(self, rhs: &$ty) -> Self::Output { self + *rhs }
-        }
-
-        impl<'a> core::ops::Add<&'a $ty> for &$ty {
-            type Output = NumOpResult<$ty>;
-
-            fn add(self, rhs: &'a $ty) -> Self::Output { *self + *rhs }
-        }
-    };
+/// You must specify `$other_ty` and you may not use `Self`. So e.g. you need
+/// to write `impl ops::Add<Amount> for Amount { ... }` when calling this macro.
+///
+/// Your where clause must include extra parenthesis, like `where (T: Copy)`.
+macro_rules! impl_op_for_references {
+    ($(
+        impl$(<$gen:ident>)? $($op_trait:ident)::+<$other_ty:ty> for $ty:ty
+        $(where ($($bounds:tt)*))?
+        {
+            type Output = $($main_output:ty)*;
+            fn $op:ident($($main_args:tt)*) -> Self::Output {
+                $($main_impl:tt)*
+            }
+        }
+    )+) => {$(
+        impl$(<$gen>)?  $($op_trait)::+<$other_ty> for $ty
+        $(where $($bounds)*)?
+        {
+            type Output = $($main_output)*;
+            fn $op($($main_args)*) -> Self::Output {
+                $($main_impl)*
+            }
+        }
+
+        impl$(<$gen>)?  $($op_trait)::+<$other_ty> for &$ty
+        $(where $($bounds)*)?
+        {
+            type Output = <$ty as $($op_trait)::+<$other_ty>>::Output;
+            fn $op(self, rhs: $other_ty) -> Self::Output {
+                (*self).$op(rhs)
+            }
+        }
+
+        impl$(<$gen>)?  $($op_trait)::+<&$other_ty> for $ty
+        $(where $($bounds)*)?
+        {
+            type Output = <$ty as $($op_trait)::+<$other_ty>>::Output;
+            fn $op(self, rhs: &$other_ty) -> Self::Output {
+                self.$op(*rhs)
+            }
+        }
+
+        impl<'a, $($gen)?> $($op_trait)::+<&'a $other_ty> for &$ty
+        $(where $($bounds)*)?
+        {
+            type Output = <$ty as $($op_trait)::+<$other_ty>>::Output;
+            fn $op(self, rhs: &$other_ty) -> Self::Output {
+                (*self).$op(*rhs)
+            }
+        }
+    )+};
 }
-pub(crate) use impl_add_for_amount_references;
+pub(crate) use impl_op_for_references;
 
 /// Implement `ops::AddAssign` for `$ty` and `&$ty`.
 macro_rules! impl_add_assign {
@@ -78,66 +83,6 @@ fn add_assign(&mut self, rhs: &$ty) { *self = *self + *rhs }
 }
 pub(crate) use impl_add_assign;
 
-/// Implement `ops::Sub` for various references.
-///
-/// Requires `$ty` it implement `Sub` e.g. 'impl Sub<T> for T'. Adds impls of:
-///
-/// - Sub<T> for &T
-/// - Sub<&T> for T
-/// - Sub<&T> for &T
-macro_rules! impl_sub_for_references {
-    ($ty:ident) => {
-        impl core::ops::Sub<$ty> for &$ty {
-            type Output = $ty;
-
-            fn sub(self, rhs: $ty) -> Self::Output { *self - rhs }
-        }
-
-        impl core::ops::Sub<&$ty> for $ty {
-            type Output = $ty;
-
-            fn sub(self, rhs: &$ty) -> Self::Output { self - *rhs }
-        }
-
-        impl<'a> core::ops::Sub<&'a $ty> for &$ty {
-            type Output = $ty;
-
-            fn sub(self, rhs: &'a $ty) -> Self::Output { *self - *rhs }
-        }
-    };
-}
-pub(crate) use impl_sub_for_references;
-
-/// Implement `ops::Sub` for various amount references.
-///
-/// Requires `$ty` it implement `Sub` e.g. 'impl Sub<T> for T'. Adds impls of:
-///
-/// - Sub<T> for &T
-/// - Sub<&T> for T
-/// - Sub<&T> for &T
-macro_rules! impl_sub_for_amount_references {
-    ($ty:ident) => {
-        impl core::ops::Sub<$ty> for &$ty {
-            type Output = NumOpResult<$ty>;
-
-            fn sub(self, rhs: $ty) -> Self::Output { *self - rhs }
-        }
-
-        impl core::ops::Sub<&$ty> for $ty {
-            type Output = NumOpResult<$ty>;
-
-            fn sub(self, rhs: &$ty) -> Self::Output { self - *rhs }
-        }
-
-        impl<'a> core::ops::Sub<&'a $ty> for &$ty {
-            type Output = NumOpResult<$ty>;
-
-            fn sub(self, rhs: &'a $ty) -> Self::Output { *self - *rhs }
-        }
-    };
-}
-pub(crate) use impl_sub_for_amount_references;
-
 /// Implement `ops::SubAssign` for `$ty` and `&$ty`.
 macro_rules! impl_sub_assign {
     ($ty:ident) => {
diff --git a/units/src/weight.rs b/units/src/weight.rs
index df654e9d..720dfd38 100644
--- a/units/src/weight.rs
+++ b/units/src/weight.rs
@@ -166,50 +166,46 @@ impl From<Weight> for u64 {
     fn from(value: Weight) -> Self { value.to_wu() }
 }
 
-impl ops::Add for Weight {
-    type Output = Weight;
+crate::internal_macros::impl_op_for_references! {
+    impl ops::Add<Weight> for Weight {
+        type Output = Weight;
 
-    fn add(self, rhs: Weight) -> Self::Output { Weight(self.0 + rhs.0) }
-}
-crate::internal_macros::impl_add_for_references!(Weight);
-crate::internal_macros::impl_add_assign!(Weight);
+        fn add(self, rhs: Weight) -> Self::Output { Weight(self.0 + rhs.0) }
+    }
+    impl ops::Sub<Weight> for Weight {
+        type Output = Weight;
 
-impl ops::Sub for Weight {
-    type Output = Weight;
+        fn sub(self, rhs: Weight) -> Self::Output { Weight(self.0 - rhs.0) }
+    }
 
-    fn sub(self, rhs: Weight) -> Self::Output { Weight(self.0 - rhs.0) }
-}
-crate::internal_macros::impl_sub_for_references!(Weight);
-crate::internal_macros::impl_sub_assign!(Weight);
+    impl ops::Mul<u64> for Weight {
+        type Output = Weight;
 
-impl ops::Mul<u64> for Weight {
-    type Output = Weight;
+        fn mul(self, rhs: u64) -> Self::Output { Weight(self.0 * rhs) }
+    }
+    impl ops::Mul<Weight> for u64 {
+        type Output = Weight;
 
-    fn mul(self, rhs: u64) -> Self::Output { Weight(self.0 * rhs) }
-}
+        fn mul(self, rhs: Weight) -> Self::Output { Weight(self * rhs.0) }
+    }
+    impl ops::Div<u64> for Weight {
+        type Output = Weight;
 
-impl ops::Mul<Weight> for u64 {
-    type Output = Weight;
+        fn div(self, rhs: u64) -> Self::Output { Weight(self.0 / rhs) }
+    }
+    impl ops::Div<Weight> for Weight {
+        type Output = u64;
 
-    fn mul(self, rhs: Weight) -> Self::Output { Weight(self * rhs.0) }
+        fn div(self, rhs: Weight) -> Self::Output { self.to_wu() / rhs.to_wu() }
+    }
 }
+crate::internal_macros::impl_add_assign!(Weight);
+crate::internal_macros::impl_sub_assign!(Weight);
 
 impl ops::MulAssign<u64> for Weight {
     fn mul_assign(&mut self, rhs: u64) { self.0 *= rhs }
 }
 
-impl ops::Div<u64> for Weight {
-    type Output = Weight;
-
-    fn div(self, rhs: u64) -> Self::Output { Weight(self.0 / rhs) }
-}
-
-impl ops::Div<Weight> for Weight {
-    type Output = u64;
-
-    fn div(self, rhs: Weight) -> Self::Output { self.to_wu() / rhs.to_wu() }
-}
-
 impl ops::DivAssign<u64> for Weight {
     fn div_assign(&mut self, rhs: u64) { self.0 /= rhs }
 }


Test patch:
diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs
index f5b450fd..d3463660 100644
--- a/units/src/amount/tests.rs
+++ b/units/src/amount/tests.rs
@@ -1196,184 +1196,133 @@ fn check_const() {
     assert_eq!(Amount::MAX_MONEY.to_sat() as i64, SignedAmount::MAX_MONEY.to_sat());
 }
 
-// Verify we have implemented all combinations of ops for `Amount` and `SignedAmount`.
-// It's easier to read this test that check the code.
+// Sanity check than stdlib supports the set of reference combinations for the ops we want.
 #[test]
 #[allow(clippy::op_ref)] // We are explicitly testing the references work with ops.
-fn amount_tyes_all_ops() {
-    // Sanity check than stdlib supports the set of reference combinations for the ops we want.
-    {
-        let x = 127;
+fn sanity_all_ops() {
+    let x = 127;
 
-        let _ = x + x;
-        let _ = &x + x;
-        let _ = x + &x;
-        let _ = &x + &x;
+    let _ = x + x;
+    let _ = &x + x;
+    let _ = x + &x;
+    let _ = &x + &x;
 
-        let _ = x - x;
-        let _ = &x - x;
-        let _ = x - &x;
-        let _ = &x - &x;
+    let _ = x - x;
+    let _ = &x - x;
+    let _ = x - &x;
+    let _ = &x - &x;
 
-        let _ = -x;
-    }
+    let _ = -x;
+}
 
+// Verify we have implemented all combinations of ops for the amount types and `NumOpResult` type.
+// It's easier to read this test than check the code.
+#[test]
+#[allow(clippy::op_ref)] // We are explicitly testing the references work with ops.
+fn num_op_result_ops() {
     let sat = Amount::from_sat(1);
     let ssat = SignedAmount::from_sat(1);
 
-    // Add
-    let _ = sat + sat;
-    let _ = &sat + sat;
-    let _ = sat + &sat;
-    let _ = &sat + &sat;
-
-    // let _ = ssat + sat;
-    // let _ = &ssat + sat;
-    // let _ = ssat + &sat;
-    // let _ = &ssat + &sat;
-
-    // let _ = sat + ssat;
-    // let _ = &sat + ssat;
-    // let _ = sat + &ssat;
-    // let _ = &sat + &ssat;
-
-    let _ = ssat + ssat;
-    let _ = &ssat + ssat;
-    let _ = ssat + &ssat;
-    let _ = &ssat + &ssat;
-
-    // Sub
-    let _ = sat - sat;
-    let _ = &sat - sat;
-    let _ = sat - &sat;
-    let _ = &sat - &sat;
-
-    // let _ = ssat - sat;
-    // let _ = &ssat - sat;
-    // let _ = ssat - &sat;
-    // let _ = &ssat - &sat;
-
-    // let _ = sat - ssat;
-    // let _ = &sat - ssat;
-    // let _ = sat - &ssat;
-    // let _ = &sat - &ssat;
-
-    let _ = ssat - ssat;
-    let _ = &ssat - ssat;
-    let _ = ssat - &ssat;
-    let _ = &ssat - &ssat;
-
-    // let _ = sat * sat;  // Intentionally not supported.
-
-    // Mul
-    let _ = sat * 3;
-    let _ = sat * &3;
-    let _ = &sat * 3;
-    let _ = &sat * &3;
-
-    let _ = ssat * 3_i64; // Explicit type for the benefit of the reader.
-    let _ = ssat * &3;
-    let _ = &ssat * 3;
-    let _ = &ssat * &3;
-
-    // Div
-    let _ = sat / 3;
-    let _ = &sat / 3;
-    let _ = sat / &3;
-    let _ = &sat / &3;
-
-    let _ = ssat / 3_i64; // Explicit type for the benefit of the reader.
-    let _ = &ssat / 3;
-    let _ = ssat / &3;
-    let _ = &ssat / &3;
-
-    // Rem
-    let _ = sat % 3;
-    let _ = &sat % 3;
-    let _ = sat % &3;
-    let _ = &sat % &3;
-
-    let _ = ssat % 3;
-    let _ = &ssat % 3;
-    let _ = ssat % &3;
-    let _ = &ssat % &3;
-
-    // FIXME: Do we want to support this?
-    // let _ = sat / sat;
-    //
-    // "How many times does this amount go into that amount?" - seems
-    // like a reasonable question to ask.
-
-    // FIXME: Do we want to support these?
-    // let _ = -sat;
-    // let _ = -ssat;
-}
+    // Explicit type as sanity check.
+    let res: NumOpResult<Amount> = sat + sat;
+    let sres: NumOpResult<SignedAmount> = ssat + ssat;
+
+    macro_rules! check_op {
+        ($(let _ = $lhs:ident $op:tt $rhs:ident);* $(;)?) => {
+            $(
+                let _ = $lhs $op $rhs;
+                let _ = &$lhs $op $rhs;
+                let _ = $lhs $op &$rhs;
+                let _ = &$lhs $op &$rhs;
+            )*
+        }
+    }
 
-// FIXME: Should we support this sort of thing?
-// It will be a lot more code for possibly not that much benefit.
-#[test]
-fn can_ops_on_amount_and_signed_amount() {
-    // let res: NumOpResult<SignedAmount> = sat + ssat;
+    // We do not currently support division involving `NumOpResult` and an amount type.
+    check_op! {
+        // Operations where RHS is the result of another operation.
+        let _ = sat + res;
+        let _ = sat - res;
+        // let _ = sat / res;
+        let _ = ssat + sres;
+        let _ = ssat - sres;
+        // let _ = ssat / sres;
+
+        // Operations where LHS is the result of another operation.
+        let _ = res + sat;
+        let _ = res - sat;
+        // let _ = res / sat;
+        let _ = sres + ssat;
+        let _ = sres - ssat;
+        // let _ = sres / ssat;
+
+        // Operations that where both sides are the result of another operation.
+        let _ = res + res;
+        let _ = res - res;
+        // let _ = res / res;
+        let _ = sres + sres;
+        let _ = sres - sres;
+        // let _ = sres / sres;
+    };
 }
 
-// Verify we have implemented all combinations of ops for the `NumOpResult` type.
-// It's easier to read this test that check the code.
+// Verify we have implemented all combinations of ops for the `NumOpResult` type and an integer.
+// It's easier to read this test than check the code.
 #[test]
 #[allow(clippy::op_ref)] // We are explicitly testing the references work with ops.
-fn amount_op_result_all_ops() {
+fn num_op_result_ops_integer() {
     let sat = Amount::from_sat(1);
-    // let ssat = SignedAmount::from_sat(1);
+    let ssat = SignedAmount::from_sat(1);
 
     // Explicit type as sanity check.
     let res: NumOpResult<Amount> = sat + sat;
-    // let sres: NumOpResult<SignedAmount> = ssat + ssat;
-
-    // Operations that where RHS is the result of another operation.
-    let _ = sat + res.clone();
-    let _ = &sat + res.clone();
-    // let _ = sat + &res.clone();
-    // let _ = &sat + &res.clone();
-
-    let _ = sat - res.clone();
-    let _ = &sat - res.clone();
-    // let _ = sat - &res.clone();
-    // let _ = &sat - &res.clone();
-
-    // Operations that where LHS is the result of another operation.
-    let _ = res.clone() + sat;
-    // let _ = &res.clone() + sat;
-    let _ = res.clone() + &sat;
-    // let _ = &res.clone() + &sat;
-
-    let _ = res.clone() - sat;
-    // let _ = &res.clone() - sat;
-    let _ = res.clone() - &sat;
-    // let _ = &res.clone() - &sat;
-
-    // Operations that where both sides are the result of another operation.
-    let _ = res.clone() + res.clone();
-    // let _ = &res.clone() + res.clone();
-    // let _ = res.clone() + &res.clone();
-    // let _ = &res.clone() + &res.clone();
-
-    let _ = res.clone() - res.clone();
-    // let _ = &res.clone() - res.clone();
-    // let _ = res.clone() - &res.clone();
-    // let _ = &res.clone() - &res.clone();
+    let sres: NumOpResult<SignedAmount> = ssat + ssat;
+
+    macro_rules! check_op {
+        ($(let _ = $lhs:ident $op:tt $rhs:literal);* $(;)?) => {
+            $(
+                let _ = $lhs $op $rhs;
+                let _ = &$lhs $op $rhs;
+                let _ = $lhs $op &$rhs;
+                let _ = &$lhs $op &$rhs;
+            )*
+        }
+    }
+    check_op! {
+        // Operations on a `NumOpResult` and integer.
+        let _ = res * 3_u64; // Explicit type for the benefit of the reader.
+        let _ = res / 3;
+        let _ = res % 3;
+
+        let _ = sres * 3_i64; // Explicit type for the benefit of the reader.
+        let _ = sres / 3;
+        let _ = sres % 3;
+    };
+}
+
+// Verify we have implemented all `Neg` for the amount types.
+#[test]
+fn amount_op_result_neg() {
+    // TODO: Implement Neg all round.
+
+    // let sat = Amount::from_sat(1);
+    let ssat = SignedAmount::from_sat(1);
+
+    // let _ = -sat;
+    let _ = -ssat;
+    // let _ = -res;
+    // let _ = -sres;
 }
 
 // Verify we have implemented all `Sum` for the `NumOpResult` type.
 #[test]
 fn amount_op_result_sum() {
     let res = Amount::from_sat(1) + Amount::from_sat(1);
-    let amounts = [res.clone(), res.clone()];
+    let amounts = [res, res];
     let amount_refs = [&res, &res];
 
     // Sum iterators.
     let _ = amounts.iter().sum::<NumOpResult<Amount>>();
     let _ = amount_refs.iter().copied().sum::<NumOpResult<Amount>>();
     let _ = amount_refs.into_iter().sum::<NumOpResult<Amount>>();
-
-    // FIXME: Should we support this? I don't think so (Tobin).
-    // let _ = amount_refs.iter().sum::<NumOpResult<&Amount>>();
 }