Fix pending payments not confirmed due to invalidation or mempool removal

Pending payments that become invalid or are removed from the mempool are now marked as failed. RBF tests were updated to check that only confirmed transactions succeed and all others are failed.only the actually confirmed transactions have the correct

Author: moisesPompilio

src/wallet/mod.rs

@@ -11,7 +11,7 @@ use crate::config::Config;
 use crate::logger::{log_debug, log_error, log_info, log_trace, LdkLogger};
 
 use crate::fee_estimator::{ConfirmationTarget, FeeEstimator};
-use crate::payment::store::ConfirmationStatus;
+use crate::payment::store::{ConfirmationStatus, PaymentDetailsUpdate};
 use crate::payment::{PaymentDetails, PaymentDirection, PaymentStatus};
 use crate::types::PaymentStore;
 use crate::Error;
@@ -165,6 +165,7 @@ where
 	fn update_payment_store<'a>(
 		&self, locked_wallet: &'a mut PersistedWallet<KVStoreWalletPersister>,
 	) -> Result<(), Error> {
+		let mut unconfirmed_txid = Vec::new();
 		for wtx in locked_wallet.transactions() {
 			let id = PaymentId(wtx.tx_node.txid.to_byte_array());
 			let txid = wtx.tx_node.txid;
@@ -178,6 +179,9 @@ where
 					} else {
 						PaymentStatus::Pending
 					};
+					if payment_status == PaymentStatus::Pending {
+						unconfirmed_txid.push(id);
+					}
 					let confirmation_status = ConfirmationStatus::Confirmed {
 						block_hash: anchor.block_id.hash,
 						height: confirmation_height,
@@ -186,6 +190,7 @@ where
 					(payment_status, confirmation_status)
 				},
 				bdk_chain::ChainPosition::Unconfirmed { .. } => {
+					unconfirmed_txid.push(id);
 					(PaymentStatus::Pending, ConfirmationStatus::Unconfirmed)
 				},
 			};
@@ -233,6 +238,31 @@ where
 			self.payment_store.insert_or_update(payment)?;
 		}
 
+		self.update_transactions_unconfirmed(&unconfirmed_txid)?;
+
+		Ok(())
+	}
+
+	fn update_transactions_unconfirmed(
+		&self, canonical_payment: &[PaymentId],
+	) -> Result<(), Error> {
+		let failed_payment = self.payment_store.list_filter(|payment| {
+			payment.status == PaymentStatus::Pending && !canonical_payment.contains(&payment.id)
+		});
+		if failed_payment.is_empty() {
+			return Ok(());
+		}
+
+		for failed_payment in &failed_payment {
+			let id = failed_payment.id;
+			let payment_update = PaymentDetailsUpdate {
+				id,
+				status: Some(PaymentStatus::Failed),
+				..PaymentDetailsUpdate::new(id)
+			};
+			self.payment_store.update(&payment_update)?;
+		}
+
 		Ok(())
 	}
 

tests/integration_tests_rust.rs

@@ -750,6 +750,9 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) {
 	let mut final_amount_sat = 0;
 	let mut original_tx;
 
+	let mut array_original_txid = Vec::new();
+	let mut array_rbf_txid = Vec::new();
+
 	// Step 1: Bump fee and change output address
 	distribute_funds_all_nodes!();
 	validate_balances!(amount_sat, false);
@@ -769,18 +772,21 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) {
 		final_amount_sat += amount_sat;
 	}
 	validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx);
+	array_original_txid.push(original_tx.compute_txid());
 
 	// Step 2: Bump fee only
 	distribute_funds_all_nodes!();
 	validate_total_onchain_balance!(amount_sat + final_amount_sat);
 	(tx, fee_output_index) = prepare_rbf(electrs, txid, &scripts_buf);
 	original_tx = tx.clone();
-	bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block);
+	let rbf_tx = bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block);
 	if is_insertion_original_tx {
 		generate_block_and_insert_transactions(bitcoind, electrs, &[original_tx.clone()]);
 	}
 	final_amount_sat += amount_sat;
 	validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx);
+	array_original_txid.push(original_tx.compute_txid());
+	array_rbf_txid.push(rbf_tx.compute_txid());
 
 	// Step 3: Increase output value
 	let value_sat = 21_000;
@@ -794,7 +800,7 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) {
 		}
 	});
 	tx.output[fee_output_index].value -= Amount::from_sat(scripts_buf.len() as u64 * value_sat);
-	bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block);
+	let rbf_tx = bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block);
 	if is_insertion_original_tx {
 		generate_block_and_insert_transactions(bitcoind, electrs, &[original_tx.clone()]);
 	}
@@ -803,6 +809,8 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) {
 		final_amount_sat += value_sat;
 	}
 	validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx);
+	array_original_txid.push(original_tx.compute_txid());
+	array_rbf_txid.push(rbf_tx.compute_txid());
 
 	// Step 4: Decrease output value
 	distribute_funds_all_nodes!();
@@ -815,7 +823,7 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) {
 		}
 	});
 	tx.output[fee_output_index].value += Amount::from_sat(scripts_buf.len() as u64 * value_sat);
-	bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block);
+	let rbf_tx = bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block);
 	if is_insertion_original_tx {
 		generate_block_and_insert_transactions(bitcoind, electrs, &[original_tx.clone()]);
 	}
@@ -824,10 +832,31 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) {
 		final_amount_sat -= value_sat;
 	}
 	validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx);
+	array_original_txid.push(original_tx.compute_txid());
+	array_rbf_txid.push(rbf_tx.compute_txid());
 
-	if !is_insert_block {
-		generate_blocks_and_wait(bitcoind, electrs, 1);
-		validate_balances!(final_amount_sat, true);
+	// Confirm transaction
+	generate_blocks_and_wait(bitcoind, electrs, 6);
+	validate_balances!(final_amount_sat, true);
+
+	// Validate the list of payments: all must be succeeded and match the confirmed on-chain txids.
+	let confirmed_onchain_txids =
+		if is_insertion_original_tx { array_original_txid } else { array_rbf_txid };
+	for node in &nodes {
+		let all_payments: Vec<PaymentDetails> = node.list_payments();
+		let pending: Vec<_> =
+			all_payments.iter().filter(|p| p.status == PaymentStatus::Pending).collect();
+
+		assert!(pending.is_empty());
+
+		let succeeded: Vec<_> =
+			all_payments.iter().filter(|p| p.status == PaymentStatus::Succeeded).collect();
+		assert_eq!(succeeded.len(), confirmed_onchain_txids.len());
+		for p in succeeded {
+			assert_eq!(p.direction, PaymentDirection::Inbound);
+			assert!(matches!(p.kind, PaymentKind::Onchain { txid: _, .. }));
+			assert!(confirmed_onchain_txids.contains(&bitcoin::Txid::from_slice(&p.id.0).unwrap()));
+		}
 	}
 
 	// Check if it is possible to send all funds from the node

tests/reorg_test.rs

@@ -1,7 +1,9 @@
 mod common;
+use bitcoin::hashes::Hash;
 use bitcoin::{Amount, ScriptBuf};
-use ldk_node::payment::{PaymentDirection, PaymentKind};
+use ldk_node::payment::{PaymentDirection, PaymentKind, PaymentStatus};
 use ldk_node::{Event, LightningBalance, PendingSweepBalance};
+use lightning::ln::channelmanager::PaymentId;
 use proptest::strategy::Strategy;
 use proptest::strategy::ValueTree;
 use proptest::{prelude::prop, proptest};
@@ -220,11 +222,26 @@ proptest! {
 		let txid = distribute_funds_unconfirmed(bitcoind, electrs, all_addrs, Amount::from_sat(amount_sat));
 
 		let mut is_spendable = false;
+		let mut expect_payment_status = PaymentStatus::Pending;
 		macro_rules! verify_wallet_balances_and_transactions {
-			($expected_balance_sat: expr, $expected_size_list_payments: expr) => {
+			($expected_balance_sat: expr, $expected_size_list_payments: expr, $expect_payment_id: expr) => {
 				let spend_balance = if is_spendable { $expected_balance_sat } else { 0 };
 				for node in &nodes {
 					node.sync_wallets().unwrap();
+					let all_payments = node.list_payments();
+					assert_eq!(all_payments.len(), $expected_size_list_payments);
+
+					let payment: Vec<_> =
+						all_payments.iter().filter(|p| p.status == expect_payment_status).collect();
+					assert_eq!(payment.len(), 1);
+					assert_eq!(payment[0].id, $expect_payment_id);
+
+					// There should be one payment with the expected status (Succeeded or Pending),
+					// and all other payments must be marked as Failed.
+					let failed: Vec<_> =
+						all_payments.iter().filter(|p| p.status == PaymentStatus::Failed).collect();
+					assert_eq!(failed.len() + payment.len(), all_payments.len());
+
 					let balances = node.list_balances();
 					assert_eq!(balances.total_onchain_balance_sats, $expected_balance_sat);
 					assert_eq!(balances.spendable_onchain_balance_sats, spend_balance);
@@ -235,8 +252,9 @@ proptest! {
 		let mut tx_to_amount = HashMap::new();
 		let (mut tx, fee_output_index) = prepare_rbf(electrs, txid, &scripts_buf);
 		tx_to_amount.insert(tx.clone(), amount_sat);
+		let mut expected_size_list_payments = 1;
 		generate_block_and_insert_transactions(bitcoind, electrs, &[]);
-		verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments);
+		verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments, PaymentId(tx.compute_txid().to_byte_array()));
 		for _ in 0..quantity_rbf {
 			let is_alterable_value = prop::bool::ANY.new_tree(&mut runner).unwrap().current();
 			if is_alterable_value {
@@ -259,8 +277,10 @@ proptest! {
 
 			tx = bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_spendable);
 			tx_to_amount.insert(tx.clone(), amount_sat);
-
-			verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments);
+			expected_size_list_payments += 1;
+			// Bitcoind needs a block to insert the transactions in list payments
+			generate_block_and_insert_transactions(bitcoind, electrs, &[]);
+			verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments, PaymentId(tx.compute_txid().to_byte_array()));
 		}
 
 		is_spendable = true;
@@ -269,7 +289,7 @@ proptest! {
 		generate_block_and_insert_transactions(bitcoind, electrs, &[tx_to_confirm.0.clone()]);
 		generate_blocks_and_wait(bitcoind, electrs, reorg_depth - 1);
 		amount_sat = *tx_to_confirm.1;
-		verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments);
+		verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments, PaymentId(tx_to_confirm.0.compute_txid().to_byte_array()));
 
 		invalidate_blocks(bitcoind, reorg_depth);
 		generate_block_and_insert_transactions(bitcoind, electrs, &[]);
@@ -278,7 +298,8 @@ proptest! {
 		let tx_to_confirm = tx_to_amount.iter().nth(index_tx_confirm).unwrap();
 		generate_block_and_insert_transactions(bitcoind, electrs, &[tx_to_confirm.0.clone()]);
 		amount_sat = *tx_to_confirm.1;
+		expect_payment_status = PaymentStatus::Succeeded;
 		generate_blocks_and_wait(bitcoind, electrs, 5);
-		verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments);
+		verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments, PaymentId(tx_to_confirm.0.compute_txid().to_byte_array()));
 	}
 }