Merge rust-bitcoin/rust-bitcoin#3813: Refactor fee calculation code

9d1cba4994 units: Introduce fee module (Tobin C. Harding)
cd908fec51 Use explicit calc getters and setters (Tobin C. Harding)

Pull request description:

  We have a bunch of functions and impl blocks scattered around the place for calculating fee from fee rate and weight.

  In an effort to make the code easier to read/understand and also easier to audit introduce a private `fee` module and move all the code that is related to this calculation into it.

  This is in internal change only.

ACKs for top commit:
  apoelstra:
    ACK 9d1cba4994e9ec94850a0e0e49f85fd0c595541e; successfully ran local tests

Tree-SHA512: a33c1ce4a1b62ff29ee65dd3adf2f19384a77f7e18f1c42019973631726cd710c2c8d9c200afb108d4f3f34fcce5cd5383a7ae512caf76c73604db9cdf9eaeda

units/src/amount/unsigned.rs

@@ -15,8 +15,6 @@ use super::{
     parse_signed_to_satoshi, split_amount_and_denomination, Denomination, Display, DisplayStyle,
     OutOfRangeError, ParseAmountError, ParseError, SignedAmount,
 };
-#[cfg(feature = "alloc")]
-use crate::{FeeRate, Weight};
 
 /// An amount.
 ///
@@ -353,59 +351,6 @@ impl Amount {
         }
     }
 
-    /// Checked weight ceiling division.
-    ///
-    /// Be aware that integer division loses the remainder if no exact division
-    /// can be made. This method rounds up ensuring the transaction fee-rate is
-    /// sufficient. See also [`Self::checked_div_by_weight_floor`].
-    ///
-    /// Returns [`None`] if overflow occurred.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// # use bitcoin_units::{Amount, FeeRate, Weight};
-    /// let amount = Amount::from_sat(10);
-    /// let weight = Weight::from_wu(300);
-    /// let fee_rate = amount.checked_div_by_weight_ceil(weight).expect("Division by weight failed");
-    /// assert_eq!(fee_rate, FeeRate::from_sat_per_kwu(34));
-    /// ```
-    #[cfg(feature = "alloc")]
-    #[must_use]
-    pub const fn checked_div_by_weight_ceil(self, weight: Weight) -> Option<FeeRate> {
-        let wu = weight.to_wu();
-        // No `?` operator in const context.
-        if let Some(sats) = self.0.checked_mul(1_000) {
-            if let Some(wu_minus_one) = wu.checked_sub(1) {
-                if let Some(sats_plus_wu_minus_one) = sats.checked_add(wu_minus_one) {
-                    if let Some(fee_rate) = sats_plus_wu_minus_one.checked_div(wu) {
-                        return Some(FeeRate::from_sat_per_kwu(fee_rate));
-                    }
-                }
-            }
-        }
-        None
-    }
-
-    /// Checked weight floor division.
-    ///
-    /// Be aware that integer division loses the remainder if no exact division
-    /// can be made. See also [`Self::checked_div_by_weight_ceil`].
-    ///
-    /// Returns [`None`] if overflow occurred.
-    #[cfg(feature = "alloc")]
-    #[must_use]
-    pub const fn checked_div_by_weight_floor(self, weight: Weight) -> Option<FeeRate> {
-        // No `?` operator in const context.
-        match self.0.checked_mul(1_000) {
-            Some(res) => match res.checked_div(weight.to_wu()) {
-                Some(fee_rate) => Some(FeeRate::from_sat_per_kwu(fee_rate)),
-                None => None,
-            },
-            None => None,
-        }
-    }
-
     /// Checked remainder.
     ///
     /// Returns [`None`] if overflow occurred.

units/src/fee.rs

@@ -0,0 +1,202 @@
+// SPDX-License-Identifier: CC0-1.0
+
+//! Calculate transaction fee ([`Amount`]) from a [`FeeRate`] and [`Weight`].
+//!
+//! The total fee for a transaction can be calculated by multiplying the transaction weight by the
+//! fee rate used to send the transaction.
+//!
+//! Either the weight or fee rate can be calculated if one knows the total fee and either of the
+//! other values. Note however that such calculations truncate (as for integer division).
+//!
+//! We provide `fee.checked_div_by_weight_ceil(weight)` to calculate a minimum threshold fee rate
+//! required to pay at least `fee` for transaction with `weight`.
+
+use core::ops;
+
+use crate::{Amount, FeeRate, Weight};
+
+impl Amount {
+    /// Checked weight ceiling division.
+    ///
+    /// Be aware that integer division loses the remainder if no exact division
+    /// can be made. This method rounds up ensuring the transaction fee-rate is
+    /// sufficient. See also [`Self::checked_div_by_weight_floor`].
+    ///
+    /// Returns [`None`] if overflow occurred.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use bitcoin_units::{Amount, FeeRate, Weight};
+    /// let amount = Amount::from_sat(10);
+    /// let weight = Weight::from_wu(300);
+    /// let fee_rate = amount.checked_div_by_weight_ceil(weight).expect("Division by weight failed");
+    /// assert_eq!(fee_rate, FeeRate::from_sat_per_kwu(34));
+    /// ```
+    #[cfg(feature = "alloc")]
+    #[must_use]
+    pub const fn checked_div_by_weight_ceil(self, weight: Weight) -> Option<FeeRate> {
+        let wu = weight.to_wu();
+        // No `?` operator in const context.
+        if let Some(sats) = self.to_sat().checked_mul(1_000) {
+            if let Some(wu_minus_one) = wu.checked_sub(1) {
+                if let Some(sats_plus_wu_minus_one) = sats.checked_add(wu_minus_one) {
+                    if let Some(fee_rate) = sats_plus_wu_minus_one.checked_div(wu) {
+                        return Some(FeeRate::from_sat_per_kwu(fee_rate));
+                    }
+                }
+            }
+        }
+        None
+    }
+
+    /// Checked weight floor division.
+    ///
+    /// Be aware that integer division loses the remainder if no exact division
+    /// can be made. See also [`Self::checked_div_by_weight_ceil`].
+    ///
+    /// Returns [`None`] if overflow occurred.
+    #[cfg(feature = "alloc")]
+    #[must_use]
+    pub const fn checked_div_by_weight_floor(self, weight: Weight) -> Option<FeeRate> {
+        // No `?` operator in const context.
+        match self.to_sat().checked_mul(1_000) {
+            Some(res) => match res.checked_div(weight.to_wu()) {
+                Some(fee_rate) => Some(FeeRate::from_sat_per_kwu(fee_rate)),
+                None => None,
+            },
+            None => None,
+        }
+    }
+}
+
+impl FeeRate {
+    /// Calculates the fee by multiplying this fee rate by weight, in weight units, returning [`None`]
+    /// if an overflow occurred.
+    ///
+    /// This is equivalent to `Self::checked_mul_by_weight()`.
+    #[must_use]
+    pub fn fee_wu(self, weight: Weight) -> Option<Amount> { self.checked_mul_by_weight(weight) }
+
+    /// Calculates the fee by multiplying this fee rate by weight, in virtual bytes, returning [`None`]
+    /// if an overflow occurred.
+    ///
+    /// This is equivalent to converting `vb` to [`Weight`] using [`Weight::from_vb`] and then calling
+    /// `Self::fee_wu(weight)`.
+    #[must_use]
+    pub fn fee_vb(self, vb: u64) -> Option<Amount> {
+        Weight::from_vb(vb).and_then(|w| self.fee_wu(w))
+    }
+
+    /// Checked weight multiplication.
+    ///
+    /// Computes the absolute fee amount for a given [`Weight`] at this fee rate.
+    /// When the resulting 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.
+    ///
+    /// [`None`] is returned if an overflow occurred.
+    #[must_use]
+    pub const fn checked_mul_by_weight(self, weight: Weight) -> Option<Amount> {
+        // No `?` operator in const context.
+        match self.to_sat_per_kwu().checked_mul(weight.to_wu()) {
+            Some(mul_res) => match mul_res.checked_add(999) {
+                Some(add_res) => Some(Amount::from_sat(add_res / 1000)),
+                None => None,
+            },
+            None => None,
+        }
+    }
+}
+
+/// Computes the ceiling so that the fee computation is conservative.
+impl ops::Mul<FeeRate> for Weight {
+    type Output = Amount;
+
+    fn mul(self, rhs: FeeRate) -> Self::Output {
+        Amount::from_sat((rhs.to_sat_per_kwu() * self.to_wu() + 999) / 1000)
+    }
+}
+
+impl ops::Mul<Weight> for FeeRate {
+    type Output = Amount;
+
+    fn mul(self, rhs: Weight) -> Self::Output { rhs * self }
+}
+
+impl ops::Div<Weight> for Amount {
+    type Output = FeeRate;
+
+    /// Truncating integer division.
+    ///
+    /// This is likely the wrong thing for a user dividing an amount by a weight. Consider using
+    /// `checked_div_by_weight` instead.
+    fn div(self, rhs: Weight) -> Self::Output {
+        FeeRate::from_sat_per_kwu(self.to_sat() * 1000 / rhs.to_wu())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn fee_rate_div_by_weight() {
+        let fee_rate = Amount::from_sat(329) / Weight::from_wu(381);
+        assert_eq!(fee_rate, FeeRate::from_sat_per_kwu(863));
+    }
+
+    #[test]
+    fn fee_wu() {
+        let fee_overflow = FeeRate::from_sat_per_kwu(10).fee_wu(Weight::MAX);
+        assert!(fee_overflow.is_none());
+
+        let fee_rate = FeeRate::from_sat_per_vb(2).unwrap();
+        let weight = Weight::from_vb(3).unwrap();
+        assert_eq!(fee_rate.fee_wu(weight).unwrap(), Amount::from_sat(6));
+    }
+
+    #[test]
+    fn fee_vb() {
+        let fee_overflow = FeeRate::from_sat_per_kwu(10).fee_vb(Weight::MAX.to_wu());
+        assert!(fee_overflow.is_none());
+
+        let fee_rate = FeeRate::from_sat_per_vb(2).unwrap();
+        assert_eq!(fee_rate.fee_vb(3).unwrap(), Amount::from_sat(6));
+    }
+
+    #[test]
+    fn checked_weight_mul() {
+        let weight = Weight::from_vb(10).unwrap();
+        let fee: Amount = FeeRate::from_sat_per_vb(10)
+            .unwrap()
+            .checked_mul_by_weight(weight)
+            .expect("expected Amount");
+        assert_eq!(Amount::from_sat(100), fee);
+
+        let fee = FeeRate::from_sat_per_kwu(10).checked_mul_by_weight(Weight::MAX);
+        assert!(fee.is_none());
+
+        let weight = Weight::from_vb(3).unwrap();
+        let fee_rate = FeeRate::from_sat_per_vb(3).unwrap();
+        let fee = fee_rate.checked_mul_by_weight(weight).unwrap();
+        assert_eq!(Amount::from_sat(9), fee);
+
+        let weight = Weight::from_wu(381);
+        let fee_rate = FeeRate::from_sat_per_kwu(864);
+        let fee = fee_rate.checked_mul_by_weight(weight).unwrap();
+        // 381 * 0.864 yields 329.18.
+        // The result is then rounded up to 330.
+        assert_eq!(fee, Amount::from_sat(330));
+    }
+
+    #[test]
+    #[allow(clippy::op_ref)]
+    fn multiply() {
+        let two = FeeRate::from_sat_per_vb(2).unwrap();
+        let three = Weight::from_vb(3).unwrap();
+        let six = Amount::from_sat(6);
+
+        assert_eq!(two * three, six);
+    }
+}

units/src/fee_rate/mod.rs

@@ -10,9 +10,6 @@ use core::{fmt, ops};
 #[cfg(feature = "arbitrary")]
 use arbitrary::{Arbitrary, Unstructured};
 
-use crate::amount::Amount;
-use crate::weight::Weight;
-
 /// Represents fee rate.
 ///
 /// This is an integer newtype representing fee rate in `sat/kwu`. It provides protection against mixing
@@ -101,26 +98,6 @@ impl FeeRate {
         }
     }
 
-    /// Checked weight multiplication.
-    ///
-    /// Computes the absolute fee amount for a given [`Weight`] at this fee rate.
-    /// When the resulting 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.
-    ///
-    /// [`None`] is returned if an overflow occurred.
-    #[must_use]
-    pub const fn checked_mul_by_weight(self, weight: Weight) -> Option<Amount> {
-        // No `?` operator in const context.
-        match self.0.checked_mul(weight.to_wu()) {
-            Some(mul_res) => match mul_res.checked_add(999) {
-                Some(add_res) => Some(Amount::from_sat(add_res / 1000)),
-                None => None,
-            },
-            None => None,
-        }
-    }
-
     /// Checked addition.
     ///
     /// Computes `self + rhs` returning [`None`] if overflow occurred.
@@ -144,23 +121,6 @@ impl FeeRate {
             None => None,
         }
     }
-
-    /// Calculates the fee by multiplying this fee rate by weight, in weight units, returning [`None`]
-    /// if an overflow occurred.
-    ///
-    /// This is equivalent to `Self::checked_mul_by_weight()`.
-    #[must_use]
-    pub fn fee_wu(self, weight: Weight) -> Option<Amount> { self.checked_mul_by_weight(weight) }
-
-    /// Calculates the fee by multiplying this fee rate by weight, in virtual bytes, returning [`None`]
-    /// if an overflow occurred.
-    ///
-    /// This is equivalent to converting `vb` to [`Weight`] using [`Weight::from_vb`] and then calling
-    /// `Self::fee_wu(weight)`.
-    #[must_use]
-    pub fn fee_vb(self, vb: u64) -> Option<Amount> {
-        Weight::from_vb(vb).and_then(|w| self.fee_wu(w))
-    }
 }
 
 /// Alternative will display the unit.
@@ -194,31 +154,6 @@ impl ops::Sub for FeeRate {
 crate::internal_macros::impl_sub_for_references!(FeeRate);
 crate::internal_macros::impl_sub_assign!(FeeRate);
 
-/// Computes the ceiling so that the fee computation is conservative.
-impl ops::Mul<FeeRate> for Weight {
-    type Output = Amount;
-
-    fn mul(self, rhs: FeeRate) -> Self::Output {
-        Amount::from_sat((rhs.to_sat_per_kwu() * self.to_wu() + 999) / 1000)
-    }
-}
-
-impl ops::Mul<Weight> for FeeRate {
-    type Output = Amount;
-
-    fn mul(self, rhs: Weight) -> Self::Output { rhs * self }
-}
-
-impl ops::Div<Weight> for Amount {
-    type Output = FeeRate;
-
-    /// Truncating integer division.
-    ///
-    /// This is likely the wrong thing for a user dividing an amount by a weight. Consider using
-    /// `checked_div_by_weight` instead.
-    fn div(self, rhs: Weight) -> Self::Output { FeeRate(self.to_sat() * 1000 / rhs.to_wu()) }
-}
-
 impl core::iter::Sum for FeeRate {
     fn sum<I>(iter: I) -> Self
     where
@@ -289,16 +224,6 @@ mod tests {
         assert_eq!(&ten - &seven, three);
     }
 
-    #[test]
-    #[allow(clippy::op_ref)]
-    fn multiply() {
-        let two = FeeRate::from_sat_per_vb(2).unwrap();
-        let three = Weight::from_vb(3).unwrap();
-        let six = Amount::from_sat(6);
-
-        assert_eq!(two * three, six);
-    }
-
     #[test]
     fn add_assign() {
         let mut f = FeeRate(1);
@@ -321,12 +246,6 @@ mod tests {
         assert_eq!(f, FeeRate(1));
     }
 
-    #[test]
-    fn fee_rate_div_by_weight() {
-        let fee_rate = Amount::from_sat(329) / Weight::from_wu(381);
-        assert_eq!(fee_rate, FeeRate(863));
-    }
-
     #[test]
     fn checked_add() {
         let f = FeeRate(1).checked_add(2).unwrap();
@@ -400,50 +319,6 @@ mod tests {
         assert!(fee_rate.is_none());
     }
 
-    #[test]
-    fn fee_wu() {
-        let fee_overflow = FeeRate(10).fee_wu(Weight::MAX);
-        assert!(fee_overflow.is_none());
-
-        let fee_rate = FeeRate::from_sat_per_vb(2).unwrap();
-        let weight = Weight::from_vb(3).unwrap();
-        assert_eq!(fee_rate.fee_wu(weight).unwrap(), Amount::from_sat(6));
-    }
-
-    #[test]
-    fn fee_vb() {
-        let fee_overflow = FeeRate(10).fee_vb(Weight::MAX.to_wu());
-        assert!(fee_overflow.is_none());
-
-        let fee_rate = FeeRate::from_sat_per_vb(2).unwrap();
-        assert_eq!(fee_rate.fee_vb(3).unwrap(), Amount::from_sat(6));
-    }
-
-    #[test]
-    fn checked_weight_mul() {
-        let weight = Weight::from_vb(10).unwrap();
-        let fee: Amount = FeeRate::from_sat_per_vb(10)
-            .unwrap()
-            .checked_mul_by_weight(weight)
-            .expect("expected Amount");
-        assert_eq!(Amount::from_sat(100), fee);
-
-        let fee = FeeRate(10).checked_mul_by_weight(Weight::MAX);
-        assert!(fee.is_none());
-
-        let weight = Weight::from_vb(3).unwrap();
-        let fee_rate = FeeRate::from_sat_per_vb(3).unwrap();
-        let fee = fee_rate.checked_mul_by_weight(weight).unwrap();
-        assert_eq!(Amount::from_sat(9), fee);
-
-        let weight = Weight::from_wu(381);
-        let fee_rate = FeeRate::from_sat_per_kwu(864);
-        let fee = fee_rate.checked_mul_by_weight(weight).unwrap();
-        // 381 * 0.864 yields 329.18.
-        // The result is then rounded up to 330.
-        assert_eq!(fee, Amount::from_sat(330));
-    }
-
     #[test]
     fn checked_div() {
         let fee_rate = FeeRate(10).checked_div(10).expect("expected feerate in sat/kwu");

units/src/lib.rs

@@ -18,6 +18,7 @@ extern crate alloc;
 #[cfg(feature = "std")]
 extern crate std;
 
+mod fee;
 mod internal_macros;
 
 #[doc(hidden)]