Skip to content

Commit

Permalink
Merge #321: Add comment clarifying fee rate in FundRawTransactionOptions
Browse files Browse the repository at this point in the history
f8a1622 Add comment clarifying fee rate in FundRawTransactionOptions (Casey Rodarmor)

Pull request description:

  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](https://developer.bitcoin.org/reference/rpc/fundrawtransaction.html):

  ```
  "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.

ACKs for top commit:
  tcharding:
    ACK f8a1622

Tree-SHA512: 3ec1c4d5dcce51b3f14cc4825667c4fda29b818dbc9b4aa8bc5d909bdaa547d5d19f78a32b2bd406f67a5344ee0574ebcbd239a7301532fb7685b1bfd2b6dc5b
  • Loading branch information
tcharding committed Apr 30, 2024
2 parents 4b3ac19 + f8a1622 commit c0fc7cb
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions json/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1877,6 +1877,9 @@ pub struct FundRawTransactionOptions {
pub include_watching: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub lock_unspents: Option<bool>,
/// The fee rate to pay per kvB. NB. This field is converted to camelCase
/// when serialized, so it is receeived by fundrawtransaction as `feeRate`,
/// which fee rate per kvB, and *not* `fee_rate`, which is per vB.
#[serde(
with = "bitcoin::amount::serde::as_btc::opt",
skip_serializing_if = "Option::is_none"
Expand Down

0 comments on commit c0fc7cb

Please sign in to comment.