From 33a3af54d61c7a20a344f1c680f376cc74781977 Mon Sep 17 00:00:00 2001 From: Camillarhi Date: Mon, 23 Feb 2026 12:14:47 +0100 Subject: [PATCH] Minor adjustment to fee rate handling in RBF fee bumping Use user-provided fee rate for bumping transaction and return an error if it is too low, otherwise use system estimated fee rate and retry mechanism. Also switches the RBF test to use `random_chain_source`, and removes unnecessary sleep-based waits. --- src/wallet/mod.rs | 31 +++++++++++++++--- tests/integration_tests_rust.rs | 58 ++++++++++++--------------------- 2 files changed, 47 insertions(+), 42 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 520fff48b..76742a6ca 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -378,8 +378,8 @@ impl Wallet { ); let payment = self.payment_store.get(&payment_id).ok_or(Error::InvalidPaymentId)?; - let pending_payment_details = self - .create_pending_payment_from_tx(payment.clone(), conflict_txids.clone()); + let pending_payment_details = + self.create_pending_payment_from_tx(payment, conflict_txids.clone()); self.pending_payment_store.insert_or_update(pending_payment_details)?; }, @@ -1042,6 +1042,16 @@ impl Wallet { return Some(direct_payment_id); } + if let Some(replaced_details) = self + .pending_payment_store + .list_filter( + |p| matches!(p.details.kind, PaymentKind::Onchain { txid, .. } if txid == target_txid), + ) + .first() + { + return Some(replaced_details.details.id); + } + if let Some(replaced_details) = self .pending_payment_store .list_filter(|p| p.conflicting_txids.contains(&target_txid)) @@ -1125,13 +1135,13 @@ impl Wallet { old_fee_rate.to_sat_per_kwu() + INCREMENTAL_RELAY_FEE_SAT_PER_1000_WEIGHT as u64; let confirmation_target = ConfirmationTarget::OnchainPayment; - let estimated_fee_rate = - fee_rate.unwrap_or_else(|| self.fee_estimator.estimate_fee_rate(confirmation_target)); + let estimated_fee_rate = self.fee_estimator.estimate_fee_rate(confirmation_target); // Use the higher of minimum RBF requirement or current network estimate let final_fee_rate_sat_per_kwu = min_required_fee_rate_sat_per_kwu.max(estimated_fee_rate.to_sat_per_kwu()); - let final_fee_rate = FeeRate::from_sat_per_kwu(final_fee_rate_sat_per_kwu); + let final_fee_rate = + fee_rate.unwrap_or_else(|| FeeRate::from_sat_per_kwu(final_fee_rate_sat_per_kwu)); let mut psbt = { let mut builder = locked_wallet.build_fee_bump(txid).map_err(|e| { @@ -1156,6 +1166,17 @@ impl Wallet { match builder.finish() { Ok(psbt) => Ok(psbt), Err(CreateTxError::FeeRateTooLow { required: required_fee_rate }) => { + if fee_rate.is_some() { + log_error!( + self.logger, + "Provided fee rate {} is too low for RBF fee bump of txid {}, required minimum fee rate: {}", + fee_rate.unwrap(), + txid, + required_fee_rate + ); + return Err(Error::InvalidFeeRate); + } + log_info!(self.logger, "BDK requires higher fee rate: {}", required_fee_rate); // BDK may require a higher fee rate than our estimate due to diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index be84e1543..96f18eeec 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -2466,7 +2466,7 @@ async fn persistence_backwards_compatibility() { #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn onchain_fee_bump_rbf() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let chain_source = TestChainSource::Esplora(&electrsd); + let chain_source = random_chain_source(&bitcoind, &electrsd); let (node_a, node_b) = setup_two_nodes(&chain_source, false, true, false); // Fund both nodes @@ -2490,6 +2490,10 @@ async fn onchain_fee_bump_rbf() { let txid = node_b.onchain_payment().send_to_address(&addr_a, amount_to_send_sats, None).unwrap(); wait_for_tx(&electrsd.client, txid).await; + // Give the chain source time to index the unconfirmed transaction before syncing. + // Without this, Esplora may not yet have the tx, causing sync to miss it and + // leaving the BDK wallet graph empty. + tokio::time::sleep(std::time::Duration::from_secs(5)).await; node_a.sync_wallets().unwrap(); node_b.sync_wallets().unwrap(); @@ -2515,10 +2519,10 @@ async fn onchain_fee_bump_rbf() { // Successful fee bump let new_txid = node_b.onchain_payment().bump_fee_rbf(payment_id, None).unwrap(); wait_for_tx(&electrsd.client, new_txid).await; - - // Sleep to allow for transaction propagation + // Give the chain source time to index the unconfirmed transaction before syncing. + // Without this, Esplora may not yet have the tx, causing sync to miss it and + // leaving the BDK wallet graph empty. tokio::time::sleep(std::time::Duration::from_secs(5)).await; - node_a.sync_wallets().unwrap(); node_b.sync_wallets().unwrap(); @@ -2540,26 +2544,9 @@ async fn onchain_fee_bump_rbf() { _ => panic!("Unexpected payment kind"), } - // Verify node_a has the inbound payment txid updated to the replacement txid - let node_a_inbound_payment = node_a.payment(&payment_id).unwrap(); - assert_eq!(node_a_inbound_payment.direction, PaymentDirection::Inbound); - match &node_a_inbound_payment.kind { - PaymentKind::Onchain { txid: inbound_txid, .. } => { - assert_eq!( - *inbound_txid, new_txid, - "node_a inbound payment txid should be updated to the replacement txid" - ); - }, - _ => panic!("Unexpected payment kind"), - } - // Multiple consecutive bumps let second_bump_txid = node_b.onchain_payment().bump_fee_rbf(payment_id, None).unwrap(); wait_for_tx(&electrsd.client, second_bump_txid).await; - - // Sleep to allow for transaction propagation - tokio::time::sleep(std::time::Duration::from_secs(5)).await; - node_a.sync_wallets().unwrap(); node_b.sync_wallets().unwrap(); @@ -2579,19 +2566,6 @@ async fn onchain_fee_bump_rbf() { _ => panic!("Unexpected payment kind"), } - // Verify node_a has the inbound payment txid updated to the second replacement txid - let node_a_second_inbound_payment = node_a.payment(&payment_id).unwrap(); - assert_eq!(node_a_second_inbound_payment.direction, PaymentDirection::Inbound); - match &node_a_second_inbound_payment.kind { - PaymentKind::Onchain { txid: inbound_txid, .. } => { - assert_eq!( - *inbound_txid, second_bump_txid, - "node_a inbound payment txid should be updated to the second replacement txid" - ); - }, - _ => panic!("Unexpected payment kind"), - } - // Confirm the transaction and try to bump again (should fail) generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; node_a.sync_wallets().unwrap(); @@ -2613,10 +2587,20 @@ async fn onchain_fee_bump_rbf() { } // Verify node A received the funds correctly - let node_a_received_payment = node_a.list_payments_with_filter( - |p| matches!(p.kind, PaymentKind::Onchain { txid, .. } if txid == second_bump_txid), - ); + let node_a_received_payment = node_a.list_payments_with_filter(|p| { + p.id == payment_id && matches!(p.kind, PaymentKind::Onchain { .. }) + }); + assert_eq!(node_a_received_payment.len(), 1); + match &node_a_received_payment[0].kind { + PaymentKind::Onchain { txid: inbound_txid, .. } => { + assert_eq!( + *inbound_txid, second_bump_txid, + "node_a inbound payment txid should be updated to the second replacement txid" + ); + }, + _ => panic!("Unexpected payment kind"), + } assert_eq!(node_a_received_payment[0].amount_msat, Some(amount_to_send_sats * 1000)); assert_eq!(node_a_received_payment[0].status, PaymentStatus::Succeeded); }