Skip to content

feat: Add fee rate handling and bump fee functionality #571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 23, 2025
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 115 additions & 2 deletions crates/utils/src/bitcoin_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ use bitcoin::{
};
use bitcoincore_rpc::{
bitcoin::{BlockHash, Transaction},
json::TestMempoolAcceptResult,
json::{EstimateMode, TestMempoolAcceptResult},
jsonrpc::{error::RpcError, Error as JsonRpcError},
Auth, Client, Error as BitcoinError, RpcApi,
};
use num_derive::FromPrimitive;
use serde_json::error::Category as SerdeJsonCategory;
use serde::{Deserialize, Serialize};
use serde_json::{error::Category as SerdeJsonCategory, json};
use std::{sync::Arc, time::Duration};
use tokio::time::{error::Elapsed, sleep, timeout};
use tracing::*;
Expand Down Expand Up @@ -85,6 +86,99 @@ pub enum BitcoinRpcError {
RpcUnknownError = 0,
}

/// A representation of a fee rate. Bitcoin Core uses different units in different
/// versions. To avoid burdening the user with using the correct unit, this struct
/// provides an umambiguous way to represent the fee rate, and the lib will perform
/// the necessary conversions.
#[derive(Copy, Clone, PartialEq, Eq, Debug, Default)]
pub struct FeeRate(Amount);

impl FeeRate {
/// Construct FeeRate from the amount per vbyte
pub fn per_vbyte(amount_per_vbyte: Amount) -> Self {
// internal representation is amount per vbyte
Self(amount_per_vbyte)
}

/// Construct FeeRate from the amount per kilo-vbyte
pub fn per_kvbyte(amount_per_kvbyte: Amount) -> Self {
// internal representation is amount per vbyte, so divide by 1000
Self::per_vbyte(amount_per_kvbyte / 1000)
}

pub fn to_sat_per_vbyte(&self) -> f64 {
// multiply by the number of decimals to get sat
self.0.to_sat() as f64 // TODO: Changed this
}

pub fn to_btc_per_kvbyte(&self) -> f64 {
// divide by 10^8 to get btc/vbyte, then multiply by 10^3 to get btc/kbyte
self.0.to_sat() as f64 / 100_000.0
}
}
Comment on lines +89 to +118
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

to_sat_per_vbyte / to_btc_per_kvbyte: misleading comments and loss of precision

  1. The comment “multiply by the number of decimals to get sat” no longer
    matches the implementation – nothing is multiplied.
  2. Returning f64 while the input is discrete satoshis discards information
    about sub‑sat/vB values before they reach the JSON layer. Bitcoin Core
    accepts sub‑sat values (e.g. 0.75) in bumpfee options.
-    pub fn to_sat_per_vbyte(&self) -> f64 {
-        // multiply by the number of decimals to get sat
-        self.0.to_sat() as f64 // TODO: Changed this
+    pub fn to_sat_per_vbyte(&self) -> f64 {
+        self.0.to_sat_f64()
     }

You can add

trait ToSatF64 {
    fn to_sat_f64(self) -> f64;
}
impl ToSatF64 for Amount {
    fn to_sat_f64(self) -> f64 {
        self.to_sat() as f64 / 1.0 // placeholder – implement once upstream exposes sub‑sat support
    }
}

to preserve eventual sub‑sat support while keeping the public API stable.


#[derive(Clone, PartialEq, Eq, Debug, Default)]
pub struct BumpFeeOptions {
/// Confirmation target in blocks.
pub conf_target: Option<u16>,
/// Specify a fee rate instead of relying on the built-in fee estimator.
pub fee_rate: Option<FeeRate>,
/// Whether this transaction could be replaced due to BIP125 (replace-by-fee)
pub replaceable: Option<bool>,
/// The fee estimate mode
pub estimate_mode: Option<EstimateMode>,
}

#[derive(Serialize, Clone, PartialEq, Debug, Default)]
#[serde(rename_all = "camelCase")]
pub struct SerializableBumpFeeOptions {
#[serde(rename = "conf_target", skip_serializing_if = "Option::is_none")]
/// Confirmation target in blocks.
pub conf_target: Option<u16>,
/// Specify a fee rate instead of relying on the built-in fee estimator.
#[serde(rename = "fee_rate")]
pub fee_rate: Option<f64>,
/// Whether this transaction could be replaced due to BIP125 (replace-by-fee)
#[serde(skip_serializing_if = "Option::is_none")]
pub replaceable: Option<bool>,
/// The fee estimate mode
#[serde(rename = "estimate_mode", skip_serializing_if = "Option::is_none")]
pub estimate_mode: Option<EstimateMode>,
}

impl BumpFeeOptions {
pub fn to_serializable(&self, version: usize) -> SerializableBumpFeeOptions {
let fee_rate = self.fee_rate.map(|x| {
if version < 210000 {
x.to_btc_per_kvbyte()
} else {
x.to_sat_per_vbyte()
}
});

SerializableBumpFeeOptions {
fee_rate,
conf_target: self.conf_target,
replaceable: self.replaceable,
estimate_mode: self.estimate_mode,
}
}
}
Comment on lines +149 to +166
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid sending null when no options are provided

bumpfee treats the absence of the second parameter differently from a
null value (the latter triggers a type error in ≥ 25.0). Assemble the
parameter list lazily:

- let args = vec![serde_json::to_value(txid)?, serde_json::to_value(opts)?];
+ let mut args = vec![serde_json::to_value(txid)?];
+ if let Some(o) = opts {
+     args.push(serde_json::to_value(o)?);
+ }

This mirrors the CLI behaviour (bitcoin-cli bumpfee <txid> [options]) and
keeps the code forward‑compatible.

Committable suggestion skipped: line range outside the PR's diff.


#[derive(Deserialize, Clone, PartialEq, Eq, Debug)]
#[serde(rename_all = "camelCase")]
pub struct BumpFeeResult {
/// The base64-encoded unsigned PSBT of the new transaction. Only returned when wallet private
/// keys are disabled.
pub psbt: Option<String>,
/// The id of the new transaction. Only returned when wallet private keys are enabled.
pub txid: Option<bitcoin::Txid>,
pub origfee: Amount,
pub fee: Amount,
/// Errors encountered during processing.
pub errors: Vec<String>,
}

impl From<RpcError> for BitcoinRpcError {
fn from(err: RpcError) -> Self {
match num::FromPrimitive::from_i32(err.code) {
Expand Down Expand Up @@ -323,6 +417,25 @@ impl BitcoinClient {
let transaction = signed_tx.transaction()?;
Ok(transaction)
}

fn bump_fee(
&self,
txid: &Txid,
options: Option<&BumpFeeOptions>,
) -> Result<BumpFeeResult, Error> {
let opts = match options {
Some(options) => Some(options.to_serializable(self.rpc.version()?)),
None => None,
};

let params = match opts {
Some(o) => json!([txid, o]),
None => json!([txid]),
};

let result = self.rpc.call("bumpfee", &[params])?;
Ok(serde_json::from_value(result)?)
}
}

fn merklize(left: Sha256dHash, right: Sha256dHash) -> Sha256dHash {
Expand Down
Loading