Merge rust-bitcoin/rust-bitcoin#4428: Use NumOpResult instead of Option

13cbead947 Use NumOpResult instead of Option (yancy)
002a0382aa Mark function constant (yancy)

Pull request description:

  Prefer the more descriptive NumOpResult return type over Option where return types are fallible.

  Closes https://github.com/rust-bitcoin/rust-bitcoin/issues/4419

ACKs for top commit:
  apoelstra:
    ACK 13cbead94766987f59482b1fbc1d0ebd0799737c; successfully ran local tests
  tcharding:
    ACK 13cbead947
  Kixunil:
    ACK 13cbead947

Tree-SHA512: 1a870962dcafe901a07abd93bd8075e41696341c1a4b3efef615493c73d5e5728bbc2326f8c2c95b9034ab001d0b3c668c9d64793ab03486d3a19f31df907c96

bitcoin/src/blockdata/transaction.rs

@@ -779,7 +779,7 @@ pub fn effective_value(
     value: Amount,
 ) -> Option<SignedAmount> {
     let weight = input_weight_prediction.total_weight();
-    let signed_input_fee = fee_rate.to_fee(weight)?.to_signed();
+    let signed_input_fee = fee_rate.to_fee(weight).ok()?.to_signed();
     value.to_signed().checked_sub(signed_input_fee)
 }
 

units/src/fee.rs

@@ -14,6 +14,7 @@
 use core::ops;
 
 use NumOpResult as R;
+use crate::NumOpError as E;
 
 use crate::{Amount, FeeRate, MathOp, NumOpResult, OptionExt, Weight};
 
@@ -114,12 +115,11 @@ impl Amount {
 }
 
 impl FeeRate {
-    /// Calculates the fee by multiplying this fee rate by weight, in weight units, returning [`None`]
+    /// Calculates the fee by multiplying this fee rate by weight, in weight units, returning
-    /// if an overflow occurred.
+    /// [`NumOpResult::Error`] if an overflow occurred.
     ///
     /// This is equivalent to `Self::checked_mul_by_weight()`.
-    #[must_use]
+    pub fn to_fee(self, weight: Weight) -> NumOpResult<Amount> { self.checked_mul_by_weight(weight) }
-    pub fn to_fee(self, weight: Weight) -> Option<Amount> { self.checked_mul_by_weight(weight) }
 
     /// Calculates the fee by multiplying this fee rate by weight, in weight units, returning [`None`]
     /// if an overflow occurred.
@@ -127,7 +127,9 @@ impl FeeRate {
     /// This is equivalent to `Self::checked_mul_by_weight()`.
     #[must_use]
     #[deprecated(since = "TBD", note = "use `to_fee()` instead")]
-    pub fn fee_wu(self, weight: Weight) -> Option<Amount> { self.checked_mul_by_weight(weight) }
+    pub fn fee_wu(self, weight: Weight) -> Option<Amount> {
+        self.checked_mul_by_weight(weight).ok()
+    }
 
     /// Calculates the fee by multiplying this fee rate by weight, in virtual bytes, returning [`None`]
     /// if an overflow occurred.
@@ -137,7 +139,7 @@ impl FeeRate {
     #[must_use]
     #[deprecated(since = "TBD", note = "use Weight::from_vb and then `to_fee()` instead")]
     pub fn fee_vb(self, vb: u64) -> Option<Amount> {
-        Weight::from_vb(vb).and_then(|w| self.to_fee(w))
+        Weight::from_vb(vb).and_then(|w| self.to_fee(w).ok())
     }
 
     /// Checked weight multiplication.
@@ -146,20 +148,16 @@ impl FeeRate {
     /// fee is a non-integer amount, the amount is rounded up, ensuring that the transaction fee is
     /// enough instead of falling short if rounded down.
     ///
-    /// Returns [`None`] if overflow occurred.
+    /// Returns [`NumOpResult::Error`] if overflow occurred.
-    #[must_use]
+    pub const fn checked_mul_by_weight(self, weight: Weight) -> NumOpResult<Amount> {
-    pub const fn checked_mul_by_weight(self, weight: Weight) -> Option<Amount> {
+        if let Some(fee) =  self.to_sat_per_kwu().checked_mul(weight.to_wu()) {
-        // No `?` operator in const context.
+            if let Some(round_up) = fee.checked_add(999) {
-        match self.to_sat_per_kwu().checked_mul(weight.to_wu()) {
+                if let Ok(ret) = Amount::from_sat(round_up / 1_000) {
-            Some(mul_res) => match mul_res.checked_add(999) {
+                    return NumOpResult::Valid(ret);
-                Some(add_res) => match Amount::from_sat(add_res / 1000) {
+                }
-                    Ok(fee) => Some(fee),
+            }
-                    Err(_) => None,
-                },
-                None => None,
-            },
-            None => None,
         }
+        NumOpResult::Error(E::while_doing(MathOp::Mul))
     }
 }
 
@@ -167,7 +165,7 @@ 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(MathOp::Mul)
+            rhs.checked_mul_by_weight(self)
         }
     }
     impl ops::Mul<FeeRate> for NumOpResult<Weight> {
@@ -204,7 +202,7 @@ crate::internal_macros::impl_op_for_references! {
     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(MathOp::Mul)
+            self.checked_mul_by_weight(rhs)
         }
     }
     impl ops::Mul<Weight> for NumOpResult<FeeRate> {
@@ -329,8 +327,7 @@ impl Weight {
     /// enough instead of falling short if rounded down.
     ///
     /// Returns [`None`] if overflow occurred.
-    #[must_use]
+    pub const fn checked_mul_by_fee_rate(self, fee_rate: FeeRate) -> NumOpResult<Amount> {
-    pub const fn checked_mul_by_fee_rate(self, fee_rate: FeeRate) -> Option<Amount> {
         fee_rate.checked_mul_by_weight(self)
     }
 }
@@ -348,7 +345,7 @@ mod tests {
     #[test]
     fn fee_wu() {
         let fee_overflow = FeeRate::from_sat_per_kwu(10).to_fee(Weight::MAX);
-        assert!(fee_overflow.is_none());
+        assert!(fee_overflow.is_error());
 
         let fee_rate = FeeRate::from_sat_per_vb(2).unwrap();
         let weight = Weight::from_vb(3).unwrap();
@@ -365,7 +362,7 @@ mod tests {
         assert_eq!(Amount::from_sat_u32(100), fee);
 
         let fee = FeeRate::from_sat_per_kwu(10).checked_mul_by_weight(Weight::MAX);
-        assert!(fee.is_none());
+        assert!(fee.is_error());
 
         let weight = Weight::from_vb(3).unwrap();
         let fee_rate = FeeRate::from_sat_per_vb(3).unwrap();

units/src/result.rs

@@ -203,7 +203,7 @@ pub struct NumOpError(MathOp);
 
 impl NumOpError {
     /// Creates a [`NumOpError`] caused by `op`.
-    pub(crate) fn while_doing(op: MathOp) -> Self { NumOpError(op) }
+    pub(crate) const fn while_doing(op: MathOp) -> Self { NumOpError(op) }
 
     /// Returns `true` if this operation error'ed due to overflow.
     pub fn is_overflow(self) -> bool { self.0.is_overflow() }