Skip to content

Commit d31fc06

Browse files
committed
Merge #483: Fix BTC/kB conversion logic
2bb341b Add test for BTC/kb conversion (Tobin C. Harding) eea11c3 Fix BTC/kb conversion (Tobin C. Harding) Pull request description: Old RPC calls use BTC/kB, we need a `FeeRate` but `v0.32` cannot elegantly handle the conversion so we do it manually - and surprise surprise its buggy. This bug is why we spent so much time on the `FeeRate` API in `rust-bitcoin`. The original code was written by me, was untested, and was buggy (even after I worked on the API in `rust-bitcoin`) - bad Tobin no biscuit. Patch 2 adds a unit test and can be put before patch 1 to see the bug. Fix: #482 ACKs for top commit: jamillambert: ACK 2bb341b Tree-SHA512: f40b7d7201d723e0b747c7af6601b6e2609792476d9f5a57706c5848f74aa415bcf2c3bdd74264e74c0d1aacb530da18e740320e1f8a2e0c02ac0bff1597a510
2 parents b6b2fea + 2bb341b commit d31fc06

1 file changed

Lines changed: 16 additions & 7 deletions

File tree

types/src/lib.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,9 @@ impl std::error::Error for NumericError {}
9090

9191
/// Converts `fee_rate` in BTC/kB to `FeeRate`.
9292
fn btc_per_kb(btc_per_kb: f64) -> Result<Option<FeeRate>, ParseAmountError> {
93-
let sats_per_kb = Amount::from_btc(btc_per_kb)?;
94-
let sats_per_byte = sats_per_kb.to_sat() / 1000;
95-
96-
// Virtual bytes equal bytes before segwit.
97-
let rate = FeeRate::from_sat_per_vb(sats_per_byte);
98-
99-
Ok(rate)
93+
// TODO: After upgrade to bitcoin `v0.33` use `FeeRate::from_sat_per_kvb()`.
94+
let per_kb = Amount::from_btc(btc_per_kb)?;
95+
Ok(FeeRate::from_sat_per_vb(per_kb.to_sat()).and_then(|fee_rate| fee_rate.checked_div(1000)))
10096
}
10197

10298
// TODO: Remove this function if a new `Witness` constructor gets added.
@@ -292,3 +288,16 @@ impl ScriptSig {
292288
ScriptBuf::from_hex(&self.hex)
293289
}
294290
}
291+
292+
#[cfg(test)]
293+
mod tests {
294+
use super::*;
295+
296+
#[test]
297+
fn convert_btc_per_kb() {
298+
// per kB = per kvB because this is a conversion of legacy transaction weights.
299+
let f: f64 = 0.000001;
300+
let got = btc_per_kb(f).unwrap();
301+
assert_eq!(got, Some(FeeRate::from_sat_per_kwu(25)))
302+
}
303+
}

0 commit comments

Comments
 (0)