-
Notifications
You must be signed in to change notification settings - Fork 58
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
Changes from all commits
668f637
aa81677
1449c79
be48698
b941c3d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,11 +9,12 @@ 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::{Deserialize, Serialize}; | ||
use serde_json::error::Category as SerdeJsonCategory; | ||
use std::{sync::Arc, time::Duration}; | ||
use tokio::time::{error::Elapsed, sleep, timeout}; | ||
|
@@ -85,6 +86,101 @@ 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 | ||
} | ||
} | ||
|
||
#[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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid sending
- 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 (
|
||
|
||
#[derive(Deserialize, Clone, 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>, | ||
/// The fee of the original transaction (before bumping), denominated in BTC. | ||
pub origfee: f64, | ||
/// The fee of the newly created bumped transaction, denominated in BTC. | ||
pub fee: f64, | ||
/// 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) { | ||
|
@@ -323,6 +419,41 @@ impl BitcoinClient { | |
let transaction = signed_tx.transaction()?; | ||
Ok(transaction) | ||
} | ||
|
||
pub fn bump_fee( | ||
&self, | ||
txid: &Txid, | ||
options: Option<&BumpFeeOptions>, | ||
) -> Result<BumpFeeResult, Error> { | ||
// Serialize options if provided | ||
let opts = match options { | ||
Some(options) => Some(options.to_serializable(self.rpc.version()?)), | ||
None => None, | ||
}; | ||
|
||
// Prepare arguments | ||
let args = vec![serde_json::to_value(txid)?, serde_json::to_value(opts)?]; | ||
|
||
// Call the "bumpfee" RPC method with borrowed args | ||
let result = self.rpc.call("bumpfee", &args); | ||
|
||
// Handle the result | ||
match result { | ||
Ok(result_value) => { | ||
// Try to deserialize the result into the expected BumpFeeResult | ||
let result: Result<BumpFeeResult, _> = serde_json::from_value(result_value); | ||
|
||
match result { | ||
Ok(bump_fee_result) => Ok(bump_fee_result), | ||
Err(err) => { | ||
println!("Failed to deserialize into BumpFeeResult"); | ||
Err(err.into()) | ||
} | ||
} | ||
} | ||
Err(err) => Err(err.into()), // Handle the case where the RPC call fails | ||
} | ||
} | ||
} | ||
|
||
fn merklize(left: Sha256dHash, right: Sha256dHash) -> Sha256dHash { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -215,6 +215,8 @@ impl BitcoinCore { | |
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
use crate::{BitcoinClient, BumpFeeOptions}; | ||
use bitcoin::Amount; | ||
|
||
#[test] | ||
fn can_launch_bitcoin() { | ||
|
@@ -231,4 +233,70 @@ mod tests { | |
5000000000 | ||
); | ||
} | ||
|
||
#[tokio::test] | ||
async fn test_bump_fee() -> Result<()> { | ||
// Step 1: Create and initialize BitcoinCore instance for test | ||
let bitcoin = BitcoinCore::new().spawn(); | ||
|
||
Comment on lines
+237
to
+241
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Blocking synchronous RPC calls inside
let to_addr = bitcoin_client.rpc.get_new_address(/* … */).unwrap(); Consider off‑loading these calls: - let to_addr = bitcoin_client.rpc.get_new_address(None, None).unwrap().assume_checked();
+ let to_addr = tokio::task::spawn_blocking(move || {
+ bitcoin_client
+ .rpc
+ .get_new_address(None, None)
+ .map(|a| a.assume_checked())
+ })
+ .await
+ .expect("task panicked")?; (or wrap the client in |
||
// Fund Alice's wallet | ||
bitcoin.fund_wallet("Alice").expect("Should fund Alice"); | ||
|
||
// Fund Bob's wallet | ||
bitcoin.fund_wallet("Bob").expect("Should fund Alice"); | ||
|
||
// Check that Bob's balance is 5000000000 satoshis (i.e., 5 BTC) | ||
assert_eq!( | ||
bitcoin.client(Some("Bob")).unwrap().get_balance(None, None).unwrap().to_sat(), | ||
5000000000 | ||
); | ||
|
||
// Initialize BitcoinClient for Alice (make sure Alice's wallet is used) | ||
let bitcoin_client = BitcoinClient::from(bitcoin.client(Some("Alice"))?); | ||
|
||
// Step 2: Send to yourself with very low fee (simulate low fee transaction) | ||
let to_addr = bitcoin_client.rpc.get_new_address(None, None).unwrap().assume_checked(); | ||
|
||
// Set the amount to send (you can adjust the amount as needed) | ||
let amount = Amount::from_sat(100_000); // 0.001 BTC (adjust as necessary) | ||
|
||
// Send the transaction to yourself (low fee expected) | ||
let txid = bitcoin_client | ||
.rpc | ||
.send_to_address(&to_addr, amount, None, None, None, Some(true), None, None) | ||
.unwrap(); | ||
|
||
// Step 3: Bump the fee for the low-fee transaction by calling bump_fee | ||
let bump_fee = bitcoin_client | ||
.bump_fee( | ||
&txid, | ||
Some(&BumpFeeOptions { | ||
conf_target: None, | ||
fee_rate: None, | ||
replaceable: Some(true), // Allow the transaction to be replaceable | ||
estimate_mode: None, | ||
}), | ||
) | ||
.unwrap(); | ||
|
||
// Assert there are no errors when bumping the fee | ||
assert!(bump_fee.errors.is_empty()); | ||
|
||
// Step 4: Generate 100 blocks to confirm the bump fee transaction | ||
bitcoin_client.rpc.generate_to_address(100, &to_addr).unwrap(); | ||
|
||
// Check the original transaction | ||
let tx_info = bitcoin_client.rpc.get_transaction(&txid, None).unwrap(); | ||
|
||
// Assert that the original transaction has negative confirmations | ||
assert!(tx_info.info.confirmations.is_negative()); | ||
|
||
// Get the bumped fee transaction's | ||
let tx_info = bitcoin_client.rpc.get_transaction(&bump_fee.txid.unwrap(), None).unwrap(); | ||
|
||
// Assert that the bumped fee transaction has confirmations | ||
assert!(tx_info.info.confirmations.is_positive()); | ||
|
||
Ok(()) | ||
} | ||
} |
There was a problem hiding this comment.
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 precisionmatches the implementation – nothing is multiplied.
f64
while the input is discrete satoshis discards informationabout sub‑sat/vB values before they reach the JSON layer. Bitcoin Core
accepts sub‑sat values (e.g.
0.75
) inbumpfee
options.You can add
to preserve eventual sub‑sat support while keeping the public API stable.