search

rust-bitcoin / rust-bitcoincore-rpc

Rust RPC client library for the Bitcoin Core JSON-RPC API.
343 stars 257 forks source link

Add comment clarifying fee rate in FundRawTransactionOptions #321

Closed casey closed 7 months ago

casey commented 1 year ago

I think this is potentially a bit of a footgun and deserves some clarification.

fundrawtransaction has two different ways to specify fee rate, fee_rate, and feeRate. From the docs:

"fee_rate" -  Specify a fee rate in sat/vB.
"feeRate" - Specify a fee rate in BTC/kvB.

This is extra confusing because the field in FundRawTransactionOptions is called fee_rate, but is serde renamed to camelCase, so becomes feeRate.

This PR adds a comment to FundRawTransactionOptions::fee_rate clarifying that it's in kvB and not vB, and corresponds to the feeRate argument to fundrawtransaction.

I actually think it would it would probably be best if the rust field were renamed to fee_rate_per_kvb, and manually renamed to feeRate when serializing, but this is good quick fix.

apoelstra commented 1 year ago

See also #320 which suggests a type-level fix.

casey commented 12 months ago

I think the type level fix is way better, although this could be merged in the mean time. Although it's only really useful if it makes it into a release sooner than the type level fix.

tcharding commented 7 months ago

I'm going to merge this, its docs only and documents a know problem. FTR after recently being made a maintainer here in this crate I am going to attempt to put some work into the crate and get a bunch of things into a new release soon-ish. Merging this PR will be a test run of my permissions to do so.

tcharding commented 7 months ago

BOOM!

casey commented 7 months ago

Nice!