Merge rust-bitcoin/rust-bitcoin#2182: fix: FeeRate::checked_mul_by_weight should scale output down by 1000
6c6c08ca50
add second test case (conduition)0c56131819
fix: FeeRate::checked_mul_by_weight should scale output down by 1000 (conduition) Pull request description: Fixes a bug in https://github.com/rust-bitcoin/rust-bitcoin/pull/1864. The `FeeRate::checked_mul_by_weight` and by extension the `FeeRate::fee_wu` methods were returning fee amounts scaled up by a factor of 1000, since the internal representation in `FeeRate` is sats per 1000 weight units. This PR fixes `checked_mul_by_weight` and its unit tests by scaling the output of these methods down by 1000, so that `X sat/vbyte * Y vbytes = X * Y sats`, instead of `X * Y * 1000 sats` as before. ## Before This code would pass without panic before this PR. ```rust let weight = Weight::from_vb(3).unwrap(); let rate = FeeRate::from_sat_per_vb(3).unwrap(); assert_eq!(weight * rate, Amount::from_sat(9)); assert_eq!(rate.checked_mul_by_weight(weight), Some(Amount::from_sat(9000))); ``` ## After ```rust let weight = Weight::from_vb(3).unwrap(); let rate = FeeRate::from_sat_per_vb(3).unwrap(); assert_eq!(weight * rate, Amount::from_sat(9)); assert_eq!(rate.checked_mul_by_weight(weight), Some(Amount::from_sat(9))); ``` ACKs for top commit: Kixunil: ACK6c6c08ca50
Tree-SHA512: aa7b0b6237d9b18057dd7c5df12740f87bd9601acc0cac588b73a2d7a1c35b1581532d0de888230296623fa166baff4b9df89d1f2f73399f44a24dcb9c75eac4
This commit is contained in:
commit
ee787c6481
|
@ -82,10 +82,12 @@ impl FeeRate {
|
||||||
|
|
||||||
/// Checked weight multiplication.
|
/// Checked weight multiplication.
|
||||||
///
|
///
|
||||||
/// Computes `self * rhs` where rhs is of type Weight. `None` is returned if an overflow
|
/// Computes the absolute fee amount for a given [`Weight`] at this fee rate.
|
||||||
/// occurred.
|
///
|
||||||
|
/// `None` is returned if an overflow occurred.
|
||||||
pub fn checked_mul_by_weight(self, rhs: Weight) -> Option<Amount> {
|
pub fn checked_mul_by_weight(self, rhs: Weight) -> Option<Amount> {
|
||||||
self.0.checked_mul(rhs.to_wu()).map(Amount::from_sat)
|
let sats = self.0.checked_mul(rhs.to_wu())?.checked_add(999)? / 1000;
|
||||||
|
Some(Amount::from_sat(sats))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Calculates fee by multiplying this fee rate by weight, in weight units, returning `None`
|
/// Calculates fee by multiplying this fee rate by weight, in weight units, returning `None`
|
||||||
|
@ -95,13 +97,14 @@ impl FeeRate {
|
||||||
///
|
///
|
||||||
/// # Examples
|
/// # Examples
|
||||||
///
|
///
|
||||||
/// ```no_run
|
/// ```
|
||||||
/// # use bitcoin::{absolute, transaction, FeeRate, Transaction};
|
/// # use bitcoin::{absolute, transaction, FeeRate, Transaction};
|
||||||
/// # // Dummy transaction.
|
/// # // Dummy transaction.
|
||||||
/// # let tx = Transaction { version: transaction::Version::ONE, lock_time: absolute::LockTime::ZERO, input: vec![], output: vec![] };
|
/// # let tx = Transaction { version: transaction::Version::ONE, lock_time: absolute::LockTime::ZERO, input: vec![], output: vec![] };
|
||||||
///
|
///
|
||||||
/// let rate = FeeRate::from_sat_per_vb(1).expect("1 sat/vbyte is valid");
|
/// let rate = FeeRate::from_sat_per_vb(1).expect("1 sat/vbyte is valid");
|
||||||
/// let fee = rate.fee_wu(tx.weight());
|
/// let fee = rate.fee_wu(tx.weight()).unwrap();
|
||||||
|
/// assert_eq!(fee.to_sat(), tx.vsize() as u64);
|
||||||
/// ```
|
/// ```
|
||||||
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) }
|
||||||
|
|
||||||
|
@ -209,12 +212,20 @@ mod tests {
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn checked_weight_mul_test() {
|
fn checked_weight_mul_test() {
|
||||||
let weight = Weight::from_wu(10);
|
let weight = Weight::from_vb(10).unwrap();
|
||||||
let fee: Amount = FeeRate(10).checked_mul_by_weight(weight).expect("expected Amount");
|
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);
|
assert_eq!(Amount::from_sat(100), fee);
|
||||||
|
|
||||||
let fee = FeeRate(10).checked_mul_by_weight(Weight::MAX);
|
let fee = FeeRate(10).checked_mul_by_weight(Weight::MAX);
|
||||||
assert!(fee.is_none());
|
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);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|
Loading…
Reference in New Issue