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.

Author: Camillarhi

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

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(1)).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
-	tokio::time::sleep(std::time::Duration::from_secs(5)).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(1)).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);
 }