bitcoindevkit
feat!: convert fees from BTC/kB to sats/vB
I was just emulating what we already do in rust-esplora-client:
/// Get a fee value in sats/vbytes from the estimates
/// that matches the confirmation target set as parameter.
pub fn convert_fee_rate(target: usize, estimates: HashMap<u16, f64>) -> Result<f32, Error> {
let fee_val = {
let mut pairs = estimates.into_iter().collect::<Vec<(u16, f64)>>();
pairs.sort_unstable_by_key(|(k, _)| std::cmp::Reverse(*k));
pairs
.into_iter()
.find(|(k, _)| *k as usize <= target)
.map(|(_, v)| v)
.unwrap_or(1.0)
};
Ok(fee_val as f32)
}
Shall we use FeeRate instead?
I was just emulating what we already do in
rust-esplora-client:/// Get a fee value in sats/vbytes from the estimates /// that matches the confirmation target set as parameter. pub fn convert_fee_rate(target: usize, estimates: HashMap<u16, f64>) -> Result<f32, Error> { let fee_val = { let mut pairs = estimates.into_iter().collect::<Vec<(u16, f64)>>(); pairs.sort_unstable_by_key(|(k, _)| std::cmp::Reverse(*k)); pairs .into_iter() .find(|(k, _)| *k as usize <= target) .map(|(_, v)| v) .unwrap_or(1.0) }; Ok(fee_val as f32) }Shall we use
FeeRateinstead?
I think it would be preferable, yes. At least a lot less error-prone and unambiguous than using a f32 or similar.
I was unable to get a correct fee estimation
let res = client.estimate_fee(1).unwrap().to_sat_per_vb_ceil();
println!("{:#?}", res);
returns
100
when the current recommended fee from mempool.space (I'd expect a similar, if not exact value) is
{"fastestFee":5,"halfHourFee":5,"hourFee":5,"economyFee":4,"minimumFee":2}
I think the convert fee rate function needs work.
I think the math is right, but the existing test might need to be updated. What you think?
I think the math is right, but the existing test might need to be updated. What you think?
As discussed offline, I think the tests need fixing.
The tests are CRAZY! They are even not reproducible. They pretty much ping a live server electrum.blockstream.info:50001. So I added the tests for the fees to make sure that the fee is > 0.
The tests are CRAZY! They are even not reproducible. They pretty much ping a live server
electrum.blockstream.info:50001. So I added the tests for the fees to make sure that the fee is> 0.
Maybe we should switch to use a local regtest electrs that we pre-populate with a certain amount of transactions and blocks to make them ~reproducible? Or, we could consider building a mock Electrum server, which shouldn't be hard to do? Especially given the confusion around fee rate conversion it would be good to test it properly in CI?
@oleonardolima Are you interested in picking this up? Otherwise I might, although it may be while until I get to it.
@oleonardolima Are you interested in picking this up? Otherwise I might, although it may be while until I get to it.
No, feel free to pick it up.
