From d5c5fe63ccacaa8f8fe55fc84d8e1020d1055e97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 4 Oct 2022 10:47:01 +0800 Subject: [PATCH 1/2] `FeeRate` calculation fixes Weight units should always be whole units. However, virtual bytes will need decimal places and the conversion from weight unit to vbytes should not be rounded. This commit has the following changes: * vbytes should always be represented in `f32` * conversion between vbytes and weight units should never be rounded * simple calculations (such as those for feerates) should be explicitly defined (instead of using multiple small functions) --- src/testutils/blockchain_tests.rs | 2 +- src/types.rs | 23 +++++++++++++---------- src/wallet/coin_selection.rs | 5 ++--- src/wallet/mod.rs | 2 +- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/testutils/blockchain_tests.rs b/src/testutils/blockchain_tests.rs index a3d7c2b17..a75d7647a 100644 --- a/src/testutils/blockchain_tests.rs +++ b/src/testutils/blockchain_tests.rs @@ -1021,7 +1021,7 @@ macro_rules! bdk_blockchain_tests { assert_eq!(details.received, 1_000 - details.fee.unwrap_or(0), "incorrect received after send"); let mut builder = wallet.build_fee_bump(details.txid).unwrap(); - builder.fee_rate(FeeRate::from_sat_per_vb(123.0)); + builder.fee_rate(FeeRate::from_sat_per_vb(124.0)); let (mut new_psbt, new_details) = builder.finish().unwrap(); println!("{:#?}", new_details); diff --git a/src/types.rs b/src/types.rs index bae86477f..1ce687356 100644 --- a/src/types.rs +++ b/src/types.rs @@ -98,13 +98,15 @@ impl FeeRate { /// Calculate fee rate from `fee` and weight units (`wu`). pub fn from_wu(fee: u64, wu: usize) -> FeeRate { - Self::from_vb(fee, wu.vbytes()) + let vbytes = wu as f32 / 4.0; + let rate = fee as f32 / vbytes; + Self::new_checked(rate) } /// Calculate fee rate from `fee` and `vbytes`. - pub fn from_vb(fee: u64, vbytes: usize) -> FeeRate { - let rate = fee as f32 / vbytes as f32; - Self::from_sat_per_vb(rate) + pub fn from_vb(fee: u64, vbytes: f32) -> FeeRate { + let rate = fee as f32 / vbytes; + Self::new_checked(rate) } /// Return the value as satoshi/vbyte @@ -114,12 +116,13 @@ impl FeeRate { /// Calculate absolute fee in Satoshis using size in weight units. pub fn fee_wu(&self, wu: usize) -> u64 { - self.fee_vb(wu.vbytes()) + let vbytes = wu as f32 / 4.0; + (self.0 * vbytes).ceil() as u64 } /// Calculate absolute fee in Satoshis using size in virtual bytes. - pub fn fee_vb(&self, vbytes: usize) -> u64 { - (self.as_sat_per_vb() * vbytes as f32).ceil() as u64 + pub fn fee_vb(&self, vbytes: f32) -> u64 { + (self.0 * vbytes).ceil() as u64 } } @@ -140,13 +143,13 @@ impl Sub for FeeRate { /// Trait implemented by types that can be used to measure weight units. pub trait Vbytes { /// Convert weight units to virtual bytes. - fn vbytes(self) -> usize; + fn vbytes(self) -> f32; } impl Vbytes for usize { - fn vbytes(self) -> usize { + fn vbytes(self) -> f32 { // ref: https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#transaction-size-calculations - (self as f32 / 4.0).ceil() as usize + self as f32 / 4.0 } } diff --git a/src/wallet/coin_selection.rs b/src/wallet/coin_selection.rs index 702ba1855..3ef22912f 100644 --- a/src/wallet/coin_selection.rs +++ b/src/wallet/coin_selection.rs @@ -304,9 +304,8 @@ impl CoinSelectionAlgorithm for OldestFirstCoinSelection { /// - `fee_rate`: required fee rate for the current selection /// - `drain_script`: script to consider change creation pub fn decide_change(remaining_amount: u64, fee_rate: FeeRate, drain_script: &Script) -> Excess { - // drain_output_len = size(len(script_pubkey)) + len(script_pubkey) + size(output_value) - let drain_output_len = serialize(drain_script).len() + 8usize; - let change_fee = fee_rate.fee_vb(drain_output_len); + let drain_output_len = serialize(drain_script).len() + 8_usize; + let change_fee = fee_rate.fee_vb(drain_output_len as f32); let drain_val = remaining_amount.saturating_sub(change_fee); if drain_val.is_dust(drain_script) { diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 2e3d9fdff..c43d1c7a4 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -2649,7 +2649,7 @@ pub(crate) mod test { builder .drain_to(addr.script_pubkey()) .drain_wallet() - .fee_rate(FeeRate::from_sat_per_vb(453.0)); + .fee_rate(FeeRate::from_sat_per_vb(455.0)); builder.finish().unwrap(); } From 2f78b34fe264775c1fc953ab53d7babee32e6ec9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BF=97=E5=AE=87?= Date: Tue, 4 Oct 2022 12:24:56 +0800 Subject: [PATCH 2/2] Rename `Vbytes` to `WeightUnits` --- src/types.rs | 8 ++++---- src/wallet/coin_selection.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/types.rs b/src/types.rs index 1ce687356..42e2494cc 100644 --- a/src/types.rs +++ b/src/types.rs @@ -141,13 +141,13 @@ impl Sub for FeeRate { } /// Trait implemented by types that can be used to measure weight units. -pub trait Vbytes { +pub trait WeightUnits { /// Convert weight units to virtual bytes. - fn vbytes(self) -> f32; + fn to_vbytes(self) -> f32; } -impl Vbytes for usize { - fn vbytes(self) -> f32 { +impl WeightUnits for usize { + fn to_vbytes(self) -> f32 { // ref: https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#transaction-size-calculations self as f32 / 4.0 } diff --git a/src/wallet/coin_selection.rs b/src/wallet/coin_selection.rs index 3ef22912f..823dd447a 100644 --- a/src/wallet/coin_selection.rs +++ b/src/wallet/coin_selection.rs @@ -727,7 +727,7 @@ mod test { use super::*; use crate::database::{BatchOperations, MemoryDatabase}; use crate::types::*; - use crate::wallet::Vbytes; + use crate::wallet::WeightUnits; use rand::rngs::StdRng; use rand::seq::SliceRandom; @@ -1318,7 +1318,7 @@ mod test { assert_eq!(result.selected.len(), 1); assert_eq!(result.selected_amount(), 100_000); - let input_size = (TXIN_BASE_WEIGHT + P2WPKH_SATISFACTION_SIZE).vbytes(); + let input_size = (TXIN_BASE_WEIGHT + P2WPKH_SATISFACTION_SIZE).to_vbytes(); // the final fee rate should be exactly the same as the fee rate given assert!((1.0 - (result.fee_amount as f32 / input_size as f32)).abs() < f32::EPSILON); }