Skip to content

Conversation

@thunderbiscuit
Copy link
Member

@thunderbiscuit thunderbiscuit commented Jan 8, 2026

Fixes #932.

The current implementation doesn't allow to calculate more precise versions of the feerate and always gives back the ceiling version of the feerate integer in sat/vb. This lets users do the calculations they need on the type.

It's too bad but this is actually a breaking change, so can only ship with 3.0.

Documentation

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing
  • I've added a changelog in the next release tracking issue (see example)
  • I've linked the relevant upstream docs or specs above

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

@thunderbiscuit
Copy link
Member Author

cc @AlexBrou

@busayo-OD
Copy link

I’m new to the project and wanted to help test this change locally. I confirmed that TxDetails now uses the FeeRate type instead of an integer, which fixes the rounding issue on the Rust side.
For UniFFI consumers (Kotlin/Swift), I noticed that FeeRate currently exposes only integer-based accessors (e.g. to_sat_per_kwu() returning sat/kwu).
Do you expect consumers to perform the sat/kwu → sat/vB conversion themselves, or would it make sense to expose a helper that returns sat/vB as a float to make fractional fee rates directly accessible?

Copy link
Collaborator

@ItoroD ItoroD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 0ef0c63

@thunderbiscuit
Copy link
Member Author

@busayo-OD

Do you expect consumers to perform the sat/kwu → sat/vB conversion themselves, or would it make sense to expose a helper that returns sat/vB as a float to make fractional fee rates directly accessible?

In theory I do like this and wish that rust-bitcoin had more utilities to work with sats/vb, but the design of the type was chosen because they wanted to circumvent dealing with floating-point arithmetic completely (pushing that complexity onto the user instead of within the FeeRate type). In this library we only expose what's available in the Rust equivalent libraries we support, and so in this case I don't want to add a custom wrapper.

@ItoroD thanks for the review! Just FYI I think those 3.0 PRs will sit here for a while before we start merging breaking changes on master. But it's good to know they're reviewed and ready to go!

@busayo-OD
Copy link

@busayo-OD

Do you expect consumers to perform the sat/kwu → sat/vB conversion themselves, or would it make sense to expose a helper that returns sat/vB as a float to make fractional fee rates directly accessible?

In theory I do like this and wish that rust-bitcoin had more utilities to work with sats/vb, but the design of the type was chosen because they wanted to circumvent dealing with floating-point arithmetic completely (pushing that complexity onto the user instead of within the FeeRate type). In this library we only expose what's available in the Rust equivalent libraries we support, and so in this case I don't want to add a custom wrapper.

@ItoroD thanks for the review! Just FYI I think those 3.0 PRs will sit here for a while before we start merging breaking changes on master. But it's good to know they're reviewed and ready to go!

Makes sense to keep this aligned with the Rust APIs and avoid floating-point handling. Thanks for the explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Return FeeRate type in TxDetail instead of FeeRate::to_sat_per_vb_ceil() (f32)

3 participants