Skip to content

Commit

Permalink
Merge #678: Implement conversion for Lightning fee rate
Browse files Browse the repository at this point in the history
de358f8 Implement conversion for Lightning fee rate (Vladimir Fomene)

Pull request description:

  This PR fixes #608.

  ### Description

  Lightning denotes transaction fee rate sats / 1000 weight units and sats / 1000 vbytes.
  Here we add support for creating BDK FeeRate from lightning fee rate. We also move all FeeRate tests to
  types.rs and rename as_sat_vb to as_sat_per_vb.

  ### Notes to the reviewers

  Matt was concerned that we might round down value in fee calculation in such a way that a transaction may not be relayed because it is below Bitcoin Core's min relay fee (1 sat/vbyte). I don't think we need to worry about that because we [round up(ceil)](https://github.com/bitcoindevkit/bdk/blob/master/src/types.rs#L91) during fee calculation, we don't round down. I will love to hear what you think. Is there something I'm missing? @johncantrell97, I will appreciate your review on this one.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### New Features:

  * [x] I've added tests for the new feature
  * [x] I've added docs for the new feature
  * [x] I've updated `CHANGELOG.md`

  #### Bugfixes:

  * [ ] This pull request breaks the existing API
  * [ ] I've added tests to reproduce the issue which are now passing
  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  danielabrozzoni:
    ACK de358f8

Tree-SHA512: aaa7da8284b668d15ad9c92168c149c4b3ee0f8faee9b7eb159745d23e38835189eaf5c336da14ba9272ee07cd366718eefb8365da9ddf53014e122b6393a087
  • Loading branch information
danielabrozzoni committed Aug 29, 2022
2 parents 0a3734e + de358f8 commit 12507c7
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 27 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
- Add capacity to create FeeRate from sats/kvbytes and sats/kwu.
- Rename `as_sat_vb` to `as_sat_per_vb`. Move all `FeeRate` test to `types.rs`.

## [v0.21.0] - [v0.20.0]

Expand All @@ -19,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- New `RpcBlockchain` implementation with various fixes.
- Return balance in separate categories, namely `confirmed`, `trusted_pending`, `untrusted_pending` & `immature`.


## [v0.20.0] - [v0.19.0]

- New MSRV set to `1.56.1`
Expand Down
44 changes: 42 additions & 2 deletions src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ impl FeeRate {
FeeRate(value)
}

/// Create a new instance of [`FeeRate`] given a float fee rate in sats/kwu
pub fn from_sat_per_kwu(sat_per_kwu: f32) -> Self {
FeeRate::new_checked(sat_per_kwu / 250.0_f32)
}

/// Create a new instance of [`FeeRate`] given a float fee rate in sats/kvb
pub fn from_sat_per_kvb(sat_per_kvb: f32) -> Self {
FeeRate::new_checked(sat_per_kvb / 1000.0_f32)
}

/// Create a new instance of [`FeeRate`] given a float fee rate in btc/kvbytes
///
/// ## Panics
Expand Down Expand Up @@ -98,7 +108,7 @@ impl FeeRate {
}

/// Return the value as satoshi/vbyte
pub fn as_sat_vb(&self) -> f32 {
pub fn as_sat_per_vb(&self) -> f32 {
self.0
}

Expand All @@ -109,7 +119,7 @@ impl FeeRate {

/// Calculate absolute fee in Satoshis using size in virtual bytes.
pub fn fee_vb(&self, vbytes: usize) -> u64 {
(self.as_sat_vb() * vbytes as f32).ceil() as u64
(self.as_sat_per_vb() * vbytes as f32).ceil() as u64
}
}

Expand Down Expand Up @@ -358,4 +368,34 @@ mod tests {
fn test_valid_feerate_pos_zero() {
let _ = FeeRate::from_sat_per_vb(0.0);
}

#[test]
fn test_fee_from_btc_per_kvb() {
let fee = FeeRate::from_btc_per_kvb(1e-5);
assert!((fee.as_sat_per_vb() - 1.0).abs() < f32::EPSILON);
}

#[test]
fn test_fee_from_sat_per_vbyte() {
let fee = FeeRate::from_sat_per_vb(1.0);
assert!((fee.as_sat_per_vb() - 1.0).abs() < f32::EPSILON);
}

#[test]
fn test_fee_default_min_relay_fee() {
let fee = FeeRate::default_min_relay_fee();
assert!((fee.as_sat_per_vb() - 1.0).abs() < f32::EPSILON);
}

#[test]
fn test_fee_from_sat_per_kvb() {
let fee = FeeRate::from_sat_per_kvb(1000.0);
assert!((fee.as_sat_per_vb() - 1.0).abs() < f32::EPSILON);
}

#[test]
fn test_fee_from_sat_per_kwu() {
let fee = FeeRate::from_sat_per_kwu(250.0);
assert!((fee.as_sat_per_vb() - 1.0).abs() < f32::EPSILON);
}
}
9 changes: 5 additions & 4 deletions src/wallet/coin_selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ impl<D: Database> CoinSelectionAlgorithm<D> for BranchAndBoundCoinSelection {
.iter()
.fold(0, |acc, x| acc + x.effective_value);

let cost_of_change = self.size_of_change as f32 * fee_rate.as_sat_vb();
let cost_of_change = self.size_of_change as f32 * fee_rate.as_sat_per_vb();

// `curr_value` and `curr_available_value` are both the sum of *effective_values* of
// the UTXOs. For the optional UTXOs (curr_available_value) we filter out UTXOs with
Expand Down Expand Up @@ -1360,7 +1360,8 @@ mod test {
let curr_available_value = utxos.iter().fold(0, |acc, x| acc + x.effective_value);

let size_of_change = 31;
let cost_of_change = size_of_change as f32 * fee_rate.as_sat_vb();
let cost_of_change = size_of_change as f32 * fee_rate.as_sat_per_vb();

let drain_script = Script::default();
let target_amount = 20_000 + FEE_AMOUNT;
BranchAndBoundCoinSelection::new(size_of_change)
Expand Down Expand Up @@ -1389,7 +1390,7 @@ mod test {
let curr_available_value = utxos.iter().fold(0, |acc, x| acc + x.effective_value);

let size_of_change = 31;
let cost_of_change = size_of_change as f32 * fee_rate.as_sat_vb();
let cost_of_change = size_of_change as f32 * fee_rate.as_sat_per_vb();
let target_amount = 20_000 + FEE_AMOUNT;

let drain_script = Script::default();
Expand All @@ -1413,7 +1414,7 @@ mod test {
fn test_bnb_function_almost_exact_match_with_fees() {
let fee_rate = FeeRate::from_sat_per_vb(1.0);
let size_of_change = 31;
let cost_of_change = size_of_change as f32 * fee_rate.as_sat_vb();
let cost_of_change = size_of_change as f32 * fee_rate.as_sat_per_vb();

let utxos: Vec<_> = generate_same_value_utxos(50_000, 10)
.into_iter()
Expand Down
4 changes: 2 additions & 2 deletions src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1067,7 +1067,7 @@ where
utxos: original_utxos,
bumping_fee: Some(tx_builder::PreviousFee {
absolute: details.fee.ok_or(Error::FeeRateUnavailable)?,
rate: feerate.as_sat_vb(),
rate: feerate.as_sat_per_vb(),
}),
..Default::default()
};
Expand Down Expand Up @@ -2095,7 +2095,7 @@ pub(crate) mod test {
let fee_rate = $fee_rate;

if !dust_change {
assert!(tx_fee_rate >= fee_rate && (tx_fee_rate - fee_rate).as_sat_vb().abs() < 0.5, "Expected fee rate of {:?}, the tx has {:?}", fee_rate, tx_fee_rate);
assert!(tx_fee_rate >= fee_rate && (tx_fee_rate - fee_rate).as_sat_per_vb().abs() < 0.5, "Expected fee rate of {:?}, the tx has {:?}", fee_rate, tx_fee_rate);
} else {
assert!(tx_fee_rate >= fee_rate, "Expected fee rate of at least {:?}, the tx has {:?}", fee_rate, tx_fee_rate);
}
Expand Down
19 changes: 0 additions & 19 deletions src/wallet/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ mod test {
SEQUENCE_LOCKTIME_TYPE_FLAG,
};
use crate::bitcoin::Address;
use crate::types::FeeRate;
use std::str::FromStr;

#[test]
Expand All @@ -164,24 +163,6 @@ mod test {
assert!(!294.is_dust(&script_p2wpkh));
}

#[test]
fn test_fee_from_btc_per_kb() {
let fee = FeeRate::from_btc_per_kvb(1e-5);
assert!((fee.as_sat_vb() - 1.0).abs() < 0.0001);
}

#[test]
fn test_fee_from_sats_vbyte() {
let fee = FeeRate::from_sat_per_vb(1.0);
assert!((fee.as_sat_vb() - 1.0).abs() < 0.0001);
}

#[test]
fn test_fee_default_min_relay_fee() {
let fee = FeeRate::default_min_relay_fee();
assert!((fee.as_sat_vb() - 1.0).abs() < 0.0001);
}

#[test]
fn test_check_nsequence_rbf_msb_set() {
let result = check_nsequence_rbf(0x80000000, 5000);
Expand Down

0 comments on commit 12507c7

Please sign in to comment.