Repository navigation
LowestFee score loses precision for fees above 2^24 sats due to f32 conversion #82
Description
Describe the bug
LowestFee::fee_score converts the exact integer long-term fee to f32 before wrapping it in Ordf32:
Ordf32((fee_for_the_tx as u64 + fee_for_spending_drain) as f32)f32 only represents integers exactly up to 2^24 (~16.7M sats). Above that, distinct fee totals round to the same f32 and compare as equal. Since branch-and-bound only replaces the incumbent on a strictly smaller score, it can keep a selection that pays more sats in fees than another available selection.
This issue was found by AI.
To Reproduce
use bdk_coin_select::{
metrics::LowestFee, Candidate, CoinSelector, DrainWeights, FeeRate, Target, TargetFee,
TargetOutputs,
};
#[test]
fn bnb_distinguishes_large_fee_scores() {
let exact_fee = 1u64 << 25;
let candidates = [
Candidate { value: exact_fee + 2, weight: 100, input_count: 1, is_segwit: false },
Candidate { value: exact_fee, weight: 100, input_count: 1, is_segwit: false },
];
let target = Target {
outputs: TargetOutputs { value_sum: 0, weight_sum: 0, n_outputs: 0 },
fee: TargetFee { absolute: exact_fee, ..TargetFee::ZERO },
max_weight: None,
};
let metric = LowestFee {
long_term_feerate: FeeRate::from_sat_per_wu(1.0),
dust_relay_feerate: FeeRate::ZERO,
drain_weights: DrainWeights::TR_KEYSPEND,
};
let mut cs = CoinSelector::new(&candidates);
cs.run_bnb(target, metric, usize::MAX).expect("a valid selection exists");
// Fails: selects the first candidate and pays `exact_fee + 2`.
assert_eq!(cs.fee(target.value(), 0), exact_fee as i64);
}Expected behavior
Selections whose fees differ by a whole number of sats should not compare as equal, so branch-and-bound picks the one with the lower fee.
Activity
chukwudiikeh commentedon Oct 9, 2026
I'd like to work on this. Planning to switch the
BnbMetricscore fromOrdf32toOrdf64, do theLowestFee::boundarithmetic inf64, and add a regression test. This changes the public trait, so let me know if prefer a different approach or want it based on #84. @noahjoerisevanlinjin commentedon Oct 19, 2026
Thanks for the clear write-up and repro. You're right that the score loses precision above 2^24 sats. We looked at making the BnB scores exact in #72 and decided against it: a single transaction fee of 16.7M sats or more isn't a range we expect coin selection to handle, and widening
BnbMetric's score type would be a breaking change for every metric implementor. I would close this as won't fix for that reason.@chukwudiikeh, thanks for offering to take it on. If you'd like to help in this area, a smaller, non-breaking fix would be welcome:
LowestFee::boundsubtractsselected_value - target.value()inf32, and that can be off by a few sats at ordinary (~1 BTC) amounts. Doing the subtraction in integers before casting would fix it.fix!: do fee math in f64
Merge bitcoindevkit#88: fix!: do fee math in f64