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

@@ -338,6 +338,7 @@ impl Wallet {
 				WalletEvent::TxUnconfirmed { txid, tx, old_block_time: None } => {
 					let payment_id = self
 						.find_payment_by_txid(txid)
+						.or_else(|| self.find_payment_by_tx_conflicts(locked_wallet, &tx))
 						.unwrap_or_else(|| PaymentId(txid.to_byte_array()));
 
 					let payment = self.create_payment_from_tx(
@@ -378,8 +379,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 +1043,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))
@@ -1053,6 +1064,15 @@ impl Wallet {
 		None
 	}
 
+	fn find_payment_by_tx_conflicts(
+		&self, locked_wallet: &PersistedWallet<KVStoreWalletPersister>, tx: &Transaction,
+	) -> Option<PaymentId> {
+		locked_wallet
+			.tx_graph()
+			.direct_conflicts(tx)
+			.find_map(|(_, conflict_txid)| self.find_payment_by_txid(conflict_txid))
+	}
+
 	#[allow(deprecated)]
 	pub(crate) fn bump_fee_rbf(
 		&self, payment_id: PaymentId, fee_rate: Option<FeeRate>,
@@ -1125,13 +1145,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 +1176,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/common/mod.rs

@@ -538,6 +538,21 @@ pub(crate) async fn wait_for_tx<E: ElectrumApi>(electrs: &E, txid: Txid) {
 	.await;
 }
 
+pub(crate) async fn wait_for_tx_in_script_history<E: ElectrumApi>(
+	electrs: &E, script: &bitcoin::Script, txid: Txid,
+) {
+	if electrs.script_get_history(script).unwrap().iter().any(|h| h.tx_hash == txid) {
+		return;
+	}
+
+	exponential_backoff_poll(|| {
+		electrs.ping().unwrap();
+		let found = electrs.script_get_history(script).unwrap().iter().any(|h| h.tx_hash == txid);
+		found.then_some(())
+	})
+	.await;
+}
+
 pub(crate) async fn wait_for_outpoint_spend<E: ElectrumApi>(electrs: &E, outpoint: OutPoint) {
 	let tx = electrs.transaction_get(&outpoint.txid).unwrap();
 	let txout_script = tx.output.get(outpoint.vout as usize).unwrap().clone().script_pubkey;

tests/integration_tests_rust.rs

@@ -40,6 +40,8 @@ use lightning_invoice::{Bolt11InvoiceDescription, Description};
 use lightning_types::payment::{PaymentHash, PaymentPreimage};
 use log::LevelFilter;
 
+use crate::common::wait_for_tx_in_script_history;
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
 async fn channel_full_cycle() {
 	let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
@@ -2466,11 +2468,12 @@ 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
 	let addr_a = node_a.onchain_payment().new_address().unwrap();
+	let addr_a_script = addr_a.script_pubkey();
 	let addr_b = node_b.onchain_payment().new_address().unwrap();
 
 	let premine_amount_sat = 500_000;
@@ -2514,10 +2517,7 @@ 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;
+	wait_for_tx_in_script_history(&electrsd.client, &addr_a_script, new_txid).await;
 
 	node_a.sync_wallets().unwrap();
 	node_b.sync_wallets().unwrap();
@@ -2555,10 +2555,7 @@ async fn onchain_fee_bump_rbf() {
 
 	// 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;
+	wait_for_tx_in_script_history(&electrsd.client, &addr_a_script, second_bump_txid).await;
 
 	node_a.sync_wallets().unwrap();
 	node_b.sync_wallets().unwrap();