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 adds a test for the low-fee rejection provided by user, 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)?;
 				},
@@ -1125,13 +1125,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 +1156,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,
+) {
+	exponential_backoff_poll(|| {
+		electrs.ping().unwrap();
+		let history = electrs.script_get_history(script).ok()?;
+		if history.iter().any(|item| item.tx_hash == txid) {
+			Some(())
+		} else {
+			None
+		}
+	})
+	.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

@@ -14,7 +14,7 @@ use std::sync::Arc;
 use bitcoin::address::NetworkUnchecked;
 use bitcoin::hashes::sha256::Hash as Sha256Hash;
 use bitcoin::hashes::Hash;
-use bitcoin::{Address, Amount, ScriptBuf, Txid};
+use bitcoin::{Address, Amount, FeeRate, ScriptBuf, Txid};
 use common::logging::{init_log_logger, validate_log_entry, MultiNodeLogger, TestLogWriter};
 use common::{
 	bump_fee_and_broadcast, distribute_funds_unconfirmed, do_channel_full_cycle,
@@ -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;
@@ -2512,12 +2515,15 @@ async fn onchain_fee_bump_rbf() {
 		node_a.onchain_payment().bump_fee_rbf(payment_id, None)
 	);
 
+	// Bump a payment with a low fee (should fail as fee must increase)
+	assert_eq!(
+		Err(NodeError::InvalidFeeRate),
+		node_b.onchain_payment().bump_fee_rbf(payment_id, Some(FeeRate::from_sat_per_kwu(1)))
+	);
+
 	// 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 +2561,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();