From 4df77f4ac0bd159ca0cbbbeaefb3b0948a7fc5f9 Mon Sep 17 00:00:00 2001 From: tsk Date: Thu, 20 Aug 2026 14:05:05 +0000 Subject: [PATCH] Ignore late failures for successful payments rust-lightning documents that PaymentFailed can arrive after PaymentSent in rare cases. In that ordering, the failure must be ignored and the payment must be treated as successful: https://github.com/lightningdevkit/rust-lightning/blob/9174965af9437196c527a9aa0df36bbcf050c8bb/lightning/src/events/mod.rs#L1230-L1233 Keep succeeded outbound Lightning records monotonic and suppress the contradictory user-facing PaymentFailed event. Cover both BOLT11 and BOLT12 on the persistence-backed store path. Developed with assistance from OpenAI Codex. --- src/event.rs | 33 ++++++++++++++++++++------ src/payment/store.rs | 55 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 8 deletions(-) diff --git a/src/event.rs b/src/event.rs index 0a3569755..abca70b1d 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1457,13 +1457,6 @@ where }; }, LdkEvent::PaymentFailed { payment_id, payment_hash, reason, .. } => { - log_info!( - self.logger, - "Failed to send payment with ID {} due to {:?}.", - payment_id, - reason - ); - let update = PaymentDetailsUpdate { hash: Some(payment_hash), status: Some(PaymentStatus::Failed), @@ -1477,6 +1470,32 @@ where }, }; + // LDK may emit `PaymentFailed` after `PaymentSent` in exceedingly rare cases. + // The payment-store update above preserves success in that case; re-read the + // resulting state so we also avoid surfacing a contradictory public event. + match self.payment_store.get(&payment_id).await { + Ok(Some(payment)) if payment.status == PaymentStatus::Succeeded => { + log_info!( + self.logger, + "Ignoring late payment failure for already-succeeded payment with ID {}.", + payment_id + ); + return Ok(()); + }, + Ok(_) => {}, + Err(e) => { + log_error!(self.logger, "Failed to access payment store: {}", e); + return Err(ReplayEvent()); + }, + } + + log_info!( + self.logger, + "Failed to send payment with ID {} due to {:?}.", + payment_id, + reason + ); + let event = Event::PaymentFailed { payment_id, payment_hash, reason }; match self.event_queue.add_event(event).await { Ok(_) => return Ok(()), diff --git a/src/payment/store.rs b/src/payment/store.rs index 3163ed15b..54a17ee8d 100644 --- a/src/payment/store.rs +++ b/src/payment/store.rs @@ -317,7 +317,20 @@ impl StorableObject for PaymentDetails { } if let Some(status) = update.status { - update_if_necessary!(self.status, status); + // LDK may, in exceedingly rare cases, emit `PaymentFailed` after + // `PaymentSent` for the same outbound payment. In that case the + // failure must be ignored and the payment must remain succeeded. + // + // Keep this invariant scoped to outbound Lightning payments as + // on-chain payment state may legitimately be revised after a reorg. + let is_outbound_lightning_payment = self.direction == PaymentDirection::Outbound + && !matches!(self.kind, PaymentKind::Onchain { .. }); + let is_late_failure = is_outbound_lightning_payment + && self.status == PaymentStatus::Succeeded + && status == PaymentStatus::Failed; + if !is_late_failure { + update_if_necessary!(self.status, status); + } } if let Some(confirmation_status) = update.confirmation_status { @@ -1589,6 +1602,23 @@ mod bounded_cache_tests { ) } + fn bolt12_payment(seed: u8) -> PaymentDetails { + PaymentDetails::new( + PaymentId([seed; 32]), + PaymentKind::Bolt12Refund { + hash: Some(PaymentHash([seed; 32])), + preimage: Some(PaymentPreimage([seed.wrapping_add(1); 32])), + secret: Some(PaymentSecret([seed.wrapping_add(2); 32])), + payer_note: None, + quantity: None, + }, + Some(seed as u64 * 1_000), + Some(seed as u64 * 3), + PaymentDirection::Outbound, + PaymentStatus::Succeeded, + ) + } + #[tokio::test] async fn evicted_payments_survive_a_round_trip_through_the_store() { // A bounded store hands back objects it deserialized rather than ones it kept, so every @@ -1614,6 +1644,10 @@ mod bounded_cache_tests { let data_store = new_bounded_payment_store(1); let mut stored = bolt11_payment(1); + stored.status = PaymentStatus::Pending; + if let PaymentKind::Bolt11 { ref mut preimage, .. } = stored.kind { + *preimage = None; + } stored.fee_paid_msat = Some(4_242); data_store.insert(stored.clone()).await.unwrap(); @@ -1631,6 +1665,25 @@ mod bounded_cache_tests { assert_eq!(stored.amount_msat, updated.amount_msat); } + #[tokio::test] + async fn late_failure_does_not_downgrade_succeeded_outbound_lightning_payment() { + let data_store = new_bounded_payment_store(1); + + for succeeded in [bolt11_payment(1), bolt12_payment(2)] { + data_store.insert(succeeded.clone()).await.unwrap(); + + // Exercise the persistence-backed update path rather than relying on the cache. + data_store.insert(bolt11_payment(3)).await.unwrap(); + + let mut update = PaymentDetailsUpdate::new(succeeded.id); + update.status = Some(PaymentStatus::Failed); + assert_eq!(Ok(DataStoreUpdateResult::Unchanged), data_store.update(update).await); + + let stored = data_store.get(&succeeded.id).await.unwrap().unwrap(); + assert_eq!(PaymentStatus::Succeeded, stored.status); + } + } + #[tokio::test] async fn listing_covers_payments_the_cache_cannot_hold() { let data_store = new_bounded_payment_store(3);