Skip to content

Ignore late failures for successful payments - #1065

Open
thesimplekid wants to merge 1 commit into
lightningdevkit:mainfrom
thesimplekid:late_failures
Open

Ignore late failures for successful payments#1065
thesimplekid wants to merge 1 commit into
lightningdevkit:mainfrom
thesimplekid:late_failures

Conversation

@thesimplekid

Copy link
Copy Markdown

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.

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.
@ldk-reviews-bot

ldk-reviews-bot commented Aug 20, 2026

Copy link
Copy Markdown

I've assigned @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@ldk-reviews-bot
ldk-reviews-bot requested a review from tnull August 20, 2026 14:46
Comment thread src/event.rs
Comment on lines 1457 to 1501
@@ -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(()),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is again related to the split-brain situation with persistence, but can the initial payment store write fail after LDK accepts the payment? PaymentSent then accepts Ok(NotFound) and emits success, so a replayed late failure could escape this check after restart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants