From cd908fec51b171635c4d5dc5494717ff1d796eeb Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 27 Dec 2024 12:37:43 +1100 Subject: [PATCH 1/2] Use explicit calc getters and setters We have a bunch of functions and impl blocks scattered around the place for calculating fee from fee rate and weight. In preparation for adding a `calc` module use getters and setters in code that will move to the `calc` module. (Remember, the `FeeRate` uses an inner sats per kwu value.) Internal change only. --- units/src/amount/unsigned.rs | 4 ++-- units/src/fee_rate/mod.rs | 14 ++++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index f6cb57eb5..96e372302 100644 --- a/units/src/amount/unsigned.rs +++ b/units/src/amount/unsigned.rs @@ -375,7 +375,7 @@ impl Amount { pub const fn checked_div_by_weight_ceil(self, weight: Weight) -> Option { let wu = weight.to_wu(); // No `?` operator in const context. - if let Some(sats) = self.0.checked_mul(1_000) { + 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) { @@ -397,7 +397,7 @@ impl Amount { #[must_use] pub const fn checked_div_by_weight_floor(self, weight: Weight) -> Option { // No `?` operator in const context. - match self.0.checked_mul(1_000) { + 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, diff --git a/units/src/fee_rate/mod.rs b/units/src/fee_rate/mod.rs index 2b5fe8205..938a518ff 100644 --- a/units/src/fee_rate/mod.rs +++ b/units/src/fee_rate/mod.rs @@ -112,7 +112,7 @@ impl FeeRate { #[must_use] pub const fn checked_mul_by_weight(self, weight: Weight) -> Option { // No `?` operator in const context. - match self.0.checked_mul(weight.to_wu()) { + 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, @@ -216,7 +216,9 @@ impl ops::Div for Amount { /// /// 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()) } + fn div(self, rhs: Weight) -> Self::Output { + FeeRate::from_sat_per_kwu(self.to_sat() * 1000 / rhs.to_wu()) + } } impl core::iter::Sum for FeeRate { @@ -324,7 +326,7 @@ mod tests { #[test] fn fee_rate_div_by_weight() { let fee_rate = Amount::from_sat(329) / Weight::from_wu(381); - assert_eq!(fee_rate, FeeRate(863)); + assert_eq!(fee_rate, FeeRate::from_sat_per_kwu(863)); } #[test] @@ -402,7 +404,7 @@ mod tests { #[test] fn fee_wu() { - let fee_overflow = FeeRate(10).fee_wu(Weight::MAX); + 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(); @@ -412,7 +414,7 @@ mod tests { #[test] fn fee_vb() { - let fee_overflow = FeeRate(10).fee_vb(Weight::MAX.to_wu()); + 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(); @@ -428,7 +430,7 @@ mod tests { .expect("expected Amount"); assert_eq!(Amount::from_sat(100), fee); - let fee = FeeRate(10).checked_mul_by_weight(Weight::MAX); + 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(); From 9d1cba4994e9ec94850a0e0e49f85fd0c595541e Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 27 Dec 2024 12:40:41 +1100 Subject: [PATCH 2/2] units: Introduce fee module 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. --- units/src/amount/unsigned.rs | 55 ---------- units/src/fee.rs | 202 +++++++++++++++++++++++++++++++++++ units/src/fee_rate/mod.rs | 127 ---------------------- units/src/lib.rs | 1 + 4 files changed, 203 insertions(+), 182 deletions(-) create mode 100644 units/src/fee.rs diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs index 96e372302..9ac7e4497 100644 --- a/units/src/amount/unsigned.rs +++ b/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 { - 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 { - // 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, - } - } - /// Checked remainder. /// /// Returns [`None`] if overflow occurred. diff --git a/units/src/fee.rs b/units/src/fee.rs new file mode 100644 index 000000000..fba121055 --- /dev/null +++ b/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 { + 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 { + // 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 { 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 { + 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 { + // 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 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 for FeeRate { + type Output = Amount; + + fn mul(self, rhs: Weight) -> Self::Output { rhs * self } +} + +impl ops::Div 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); + } +} diff --git a/units/src/fee_rate/mod.rs b/units/src/fee_rate/mod.rs index 938a518ff..c0baeb9df 100644 --- a/units/src/fee_rate/mod.rs +++ b/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 { - // 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, - } - } - /// 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 { 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 { - Weight::from_vb(vb).and_then(|w| self.fee_wu(w)) - } } /// Alternative will display the unit. @@ -194,33 +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 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 for FeeRate { - type Output = Amount; - - fn mul(self, rhs: Weight) -> Self::Output { rhs * self } -} - -impl ops::Div 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()) - } -} - impl core::iter::Sum for FeeRate { fn sum(iter: I) -> Self where @@ -291,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); @@ -323,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::from_sat_per_kwu(863)); - } - #[test] fn checked_add() { let f = FeeRate(1).checked_add(2).unwrap(); @@ -402,50 +319,6 @@ mod tests { assert!(fee_rate.is_none()); } - #[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] fn checked_div() { let fee_rate = FeeRate(10).checked_div(10).expect("expected feerate in sat/kwu"); diff --git a/units/src/lib.rs b/units/src/lib.rs index e4bd1254b..4d3be98ac 100644 --- a/units/src/lib.rs +++ b/units/src/lib.rs @@ -18,6 +18,7 @@ extern crate alloc; #[cfg(feature = "std")] extern crate std; +mod fee; mod internal_macros; #[doc(hidden)]