Skip to content

Commit c635cb4

Browse files
moisesPompiliomoisesPompilio
committed
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
1 parent fe805cf commit c635cb4

File tree

3 files changed

+92
-14
lines changed

3 files changed

+92
-14
lines changed

‎src/wallet/mod.rs‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ use persist::KVStoreWalletPersister;
4444
use crate::config::Config;
4545
use crate::fee_estimator::{ConfirmationTarget, FeeEstimator, OnchainFeeEstimator};
4646
use crate::logger::{log_debug, log_error, log_info, log_trace, LdkLogger, Logger};
47-
use crate::payment::store::ConfirmationStatus;
47+
use crate::payment::store::{ConfirmationStatus, PaymentDetailsUpdate};
4848
use crate::payment::{PaymentDetails, PaymentDirection, PaymentStatus};
4949
use crate::types::{Broadcaster, PaymentStore};
5050
use crate::Error;
@@ -151,6 +151,7 @@ impl Wallet {
151151
fn update_payment_store<'a>(
152152
&self, locked_wallet: &'a mut PersistedWallet<KVStoreWalletPersister>,
153153
) -> Result<(), Error> {
154+
let mut unconfirmed_txid = Vec::new();
154155
for wtx in locked_wallet.transactions() {
155156
let id = PaymentId(wtx.tx_node.txid.to_byte_array());
156157
let txid = wtx.tx_node.txid;
@@ -162,6 +163,7 @@ impl Wallet {
162163
{
163164
PaymentStatus::Succeeded
164165
} else {
166+
unconfirmed_txid.push(id);
165167
PaymentStatus::Pending
166168
};
167169
let confirmation_status = ConfirmationStatus::Confirmed {
@@ -172,6 +174,7 @@ impl Wallet {
172174
(payment_status, confirmation_status)
173175
},
174176
bdk_chain::ChainPosition::Unconfirmed { .. } => {
177+
unconfirmed_txid.push(id);
175178
(PaymentStatus::Pending, ConfirmationStatus::Unconfirmed)
176179
},
177180
};
@@ -219,6 +222,31 @@ impl Wallet {
219222
self.payment_store.insert_or_update(payment)?;
220223
}
221224

225+
self.update_transactions_unconfirmed(&unconfirmed_txid)?;
226+
227+
Ok(())
228+
}
229+
230+
fn update_transactions_unconfirmed(
231+
&self, canonical_payment: &[PaymentId],
232+
) -> Result<(), Error> {
233+
let failed_payment = self.payment_store.list_filter(|payment| {
234+
payment.status == PaymentStatus::Pending && !canonical_payment.contains(&payment.id)
235+
});
236+
if failed_payment.is_empty() {
237+
return Ok(());
238+
}
239+
240+
for failed_payment in &failed_payment {
241+
let id = failed_payment.id;
242+
let payment_update = PaymentDetailsUpdate {
243+
id,
244+
status: Some(PaymentStatus::Failed),
245+
..PaymentDetailsUpdate::new(id)
246+
};
247+
self.payment_store.update(&payment_update)?;
248+
}
249+
222250
Ok(())
223251
}
224252

‎tests/integration_tests_rust.rs‎

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -748,6 +748,9 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) {
748748
let mut final_amount_sat = 0;
749749
let mut original_tx;
750750

751+
let mut array_original_txid = Vec::new();
752+
let mut array_rbf_txid = Vec::new();
753+
751754
// Step 1: Bump fee and change output address
752755
distribute_funds_all_nodes!();
753756
validate_balances!(amount_sat, false);
@@ -767,18 +770,21 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) {
767770
final_amount_sat += amount_sat;
768771
}
769772
validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx);
773+
array_original_txid.push(original_tx.compute_txid());
770774

771775
// Step 2: Bump fee only
772776
distribute_funds_all_nodes!();
773777
validate_total_onchain_balance!(amount_sat + final_amount_sat);
774778
(tx, fee_output_index) = prepare_rbf(electrs, txid, &scripts_buf);
775779
original_tx = tx.clone();
776-
bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block);
780+
let rbf_tx = bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block);
777781
if is_insertion_original_tx {
778782
generate_block_and_insert_transactions(bitcoind, electrs, &[original_tx.clone()]);
779783
}
780784
final_amount_sat += amount_sat;
781785
validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx);
786+
array_original_txid.push(original_tx.compute_txid());
787+
array_rbf_txid.push(rbf_tx.compute_txid());
782788

783789
// Step 3: Increase output value
784790
let value_sat = 21_000;
@@ -792,7 +798,7 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) {
792798
}
793799
});
794800
tx.output[fee_output_index].value -= Amount::from_sat(scripts_buf.len() as u64 * value_sat);
795-
bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block);
801+
let rbf_tx = bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block);
796802
if is_insertion_original_tx {
797803
generate_block_and_insert_transactions(bitcoind, electrs, &[original_tx.clone()]);
798804
}
@@ -801,6 +807,8 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) {
801807
final_amount_sat += value_sat;
802808
}
803809
validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx);
810+
array_original_txid.push(original_tx.compute_txid());
811+
array_rbf_txid.push(rbf_tx.compute_txid());
804812

805813
// Step 4: Decrease output value
806814
distribute_funds_all_nodes!();
@@ -813,7 +821,7 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) {
813821
}
814822
});
815823
tx.output[fee_output_index].value += Amount::from_sat(scripts_buf.len() as u64 * value_sat);
816-
bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block);
824+
let rbf_tx = bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_insert_block);
817825
if is_insertion_original_tx {
818826
generate_block_and_insert_transactions(bitcoind, electrs, &[original_tx.clone()]);
819827
}
@@ -822,10 +830,31 @@ fn run_rbf_test(is_insert_block: bool, is_insertion_original_tx: bool) {
822830
final_amount_sat -= value_sat;
823831
}
824832
validate_balances!(final_amount_sat, is_insert_block || is_insertion_original_tx);
833+
array_original_txid.push(original_tx.compute_txid());
834+
array_rbf_txid.push(rbf_tx.compute_txid());
825835

826-
if !is_insert_block {
827-
generate_blocks_and_wait(bitcoind, electrs, 1);
828-
validate_balances!(final_amount_sat, true);
836+
// Confirm transaction
837+
generate_blocks_and_wait(bitcoind, electrs, 6);
838+
validate_balances!(final_amount_sat, true);
839+
840+
// Validate the list of payments: all must be succeeded and match the confirmed on-chain txids.
841+
let confirmed_onchain_txids =
842+
if is_insertion_original_tx { array_original_txid } else { array_rbf_txid };
843+
for node in &nodes {
844+
let all_payments: Vec<PaymentDetails> = node.list_payments();
845+
let pending: Vec<_> =
846+
all_payments.iter().filter(|p| p.status == PaymentStatus::Pending).collect();
847+
848+
assert!(pending.is_empty());
849+
850+
let succeeded: Vec<_> =
851+
all_payments.iter().filter(|p| p.status == PaymentStatus::Succeeded).collect();
852+
assert_eq!(succeeded.len(), confirmed_onchain_txids.len());
853+
for p in succeeded {
854+
assert_eq!(p.direction, PaymentDirection::Inbound);
855+
assert!(matches!(p.kind, PaymentKind::Onchain { txid: _, .. }));
856+
assert!(confirmed_onchain_txids.contains(&bitcoin::Txid::from_slice(&p.id.0).unwrap()));
857+
}
829858
}
830859

831860
// Check if it is possible to send all funds from the node

‎tests/reorg_test.rs‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
mod common;
2+
use bitcoin::hashes::Hash;
23
use bitcoin::{Amount, ScriptBuf};
3-
use ldk_node::payment::{PaymentDirection, PaymentKind};
4+
use ldk_node::payment::{PaymentDirection, PaymentKind, PaymentStatus};
45
use ldk_node::{Event, LightningBalance, PendingSweepBalance};
6+
use lightning::ln::channelmanager::PaymentId;
57
use proptest::strategy::Strategy;
68
use proptest::strategy::ValueTree;
79
use proptest::{prelude::prop, proptest};
@@ -220,11 +222,26 @@ proptest! {
220222
let txid = distribute_funds_unconfirmed(bitcoind, electrs, all_addrs, Amount::from_sat(amount_sat));
221223

222224
let mut is_spendable = false;
225+
let mut expect_payment_status = PaymentStatus::Pending;
223226
macro_rules! verify_wallet_balances_and_transactions {
224-
($expected_balance_sat: expr, $expected_size_list_payments: expr) => {
227+
($expected_balance_sat: expr, $expected_size_list_payments: expr, $expect_payment_id: expr) => {
225228
let spend_balance = if is_spendable { $expected_balance_sat } else { 0 };
226229
for node in &nodes {
227230
node.sync_wallets().unwrap();
231+
let all_payments = node.list_payments();
232+
assert_eq!(all_payments.len(), $expected_size_list_payments);
233+
234+
let payment: Vec<_> =
235+
all_payments.iter().filter(|p| p.status == expect_payment_status).collect();
236+
assert_eq!(payment.len(), 1);
237+
assert_eq!(payment[0].id, $expect_payment_id);
238+
239+
// There should be one payment with the expected status (Succeeded or Pending),
240+
// and all other payments must be marked as Failed.
241+
let failed: Vec<_> =
242+
all_payments.iter().filter(|p| p.status == PaymentStatus::Failed).collect();
243+
assert_eq!(failed.len() + payment.len(), all_payments.len());
244+
228245
let balances = node.list_balances();
229246
assert_eq!(balances.total_onchain_balance_sats, $expected_balance_sat);
230247
assert_eq!(balances.spendable_onchain_balance_sats, spend_balance);
@@ -235,8 +252,9 @@ proptest! {
235252
let mut tx_to_amount = HashMap::new();
236253
let (mut tx, fee_output_index) = prepare_rbf(electrs, txid, &scripts_buf);
237254
tx_to_amount.insert(tx.clone(), amount_sat);
255+
let mut expected_size_list_payments = 1;
238256
generate_block_and_insert_transactions(bitcoind, electrs, &[]);
239-
verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments);
257+
verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments, PaymentId(tx.compute_txid().to_byte_array()));
240258
for _ in 0..quantity_rbf {
241259
let is_alterable_value = prop::bool::ANY.new_tree(&mut runner).unwrap().current();
242260
if is_alterable_value {
@@ -259,8 +277,10 @@ proptest! {
259277

260278
tx = bump_fee_and_broadcast(bitcoind, electrs, tx, fee_output_index, is_spendable);
261279
tx_to_amount.insert(tx.clone(), amount_sat);
262-
263-
verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments);
280+
expected_size_list_payments += 1;
281+
// Bitcoind needs a block to insert the transactions in list payments
282+
generate_block_and_insert_transactions(bitcoind, electrs, &[]);
283+
verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments, PaymentId(tx.compute_txid().to_byte_array()));
264284
}
265285

266286
is_spendable = true;
@@ -269,7 +289,7 @@ proptest! {
269289
generate_block_and_insert_transactions(bitcoind, electrs, &[tx_to_confirm.0.clone()]);
270290
generate_blocks_and_wait(bitcoind, electrs, reorg_depth - 1);
271291
amount_sat = *tx_to_confirm.1;
272-
verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments);
292+
verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments, PaymentId(tx_to_confirm.0.compute_txid().to_byte_array()));
273293

274294
invalidate_blocks(bitcoind, reorg_depth);
275295
generate_block_and_insert_transactions(bitcoind, electrs, &[]);
@@ -278,7 +298,8 @@ proptest! {
278298
let tx_to_confirm = tx_to_amount.iter().nth(index_tx_confirm).unwrap();
279299
generate_block_and_insert_transactions(bitcoind, electrs, &[tx_to_confirm.0.clone()]);
280300
amount_sat = *tx_to_confirm.1;
301+
expect_payment_status = PaymentStatus::Succeeded;
281302
generate_blocks_and_wait(bitcoind, electrs, 5);
282-
verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments);
303+
verify_wallet_balances_and_transactions!(amount_sat, expected_size_list_payments, PaymentId(tx_to_confirm.0.compute_txid().to_byte_array()));
283304
}
284305
}

0 commit comments

Comments
 (0)