Skip to content

fix!: do fee math in f64 - #88

Merged
evanlinjin merged 1 commit into
bitcoindevkit:masterfrom
evanlinjin:dv/replace-87
Oct 22, 2026
Merged

evanlinjin merged 1 commit into
bitcoindevkit:masterfrom
evanlinjin:dv/replace-87

Conversation

@evanlinjin

Copy link
Copy Markdown
Member

Moves the crate's fee math from f32 to f64. With f32, rounding at ordinary amounts makes LowestFee miss the cheapest selection:

  • LowestFee::bound cast selected_value and target.value() to f32 separately before subtracting them. At ~1 BTC, f32's spacing is 8 sats, so the bound could come out several sats above the true fee, and BnB pruned the cheapest selection.
  • LowestFee's score cast the fee to f32, so fees of 2^24 sats or more that differ by a few sats scored the same (LowestFee score loses precision for fees above 2^24 sats due to f32 conversion #82).

Patching only bound would leave every other f32 path rounding, so the whole float layer changes. The next release breaks the API anyway:

  • FeeRate wraps an Ordf64. Its constructors and as_sat_vb/spwu take and return f64.
  • Candidate::effective_value, value_pwu, effective_value_pwu, implied_fee and fee_per_value, CoinSelector::waste and DrainWeights::waste return f64.
  • BnbMetric::score/bound, run_bnb and bnb_solutions use Ordf64.
  • FloatExt::ceil goes through i64 rather than i32, so it is correct for f64.
  • LowestFee::bound subtracts the values as integers and needs no helper or final cast.

Tests: bound_is_admissible_at_large_values calls bound on a 1 BTC + 99 sat input (on master the bound is 104 while selecting both costs 101). score_tells_apart_fees_above_2_pow_24 checks that fees of 2^25 and 2^25 + 1 score differently. The round-count asserts in tests/bnb.rs are unchanged.

Fixes #82

Replaces #87. Differences from it:

`FeeRate`, `Candidate`'s value methods, the waste functions and the
`BnbMetric` score were all `f32`. At ~1 BTC values f32's spacing is 8
sats, so `LowestFee::bound` (which cast `selected_value` and
`target.value()` separately before subtracting them) could land several
sats above the true fee and BnB pruned the cheapest selection. Fees of
2^24 sats or more also scored the same, so BnB could keep the dearer of
two selections (bitcoindevkit#82).

Move the crate's float layer to f64: `FeeRate` wraps an `Ordf64`,
`BnbMetric::score`/`bound` return `Ordf64`, and `FloatExt::ceil` goes
through `i64` so it is correct for f64. `LowestFee::bound` also
subtracts the selected and target values as integers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@evanlinjin
evanlinjin marked this pull request as ready for review October 22, 2026 02:40
@evanlinjin
evanlinjin merged commit ba83476 into bitcoindevkit:master Oct 22, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LowestFee score loses precision for fees above 2^24 sats due to f32 conversion

2 participants