-
Notifications
You must be signed in to change notification settings - Fork 421
Don't auto-fail offers payments pre-HTLC lock in #4078
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't auto-fail offers payments pre-HTLC lock in #4078
Conversation
Previously, we had a bug that particularly affected async payments where if an outbound payment was in the state {Static}InvoiceReceived and there was a call to process_pending_htlc_forwards, the payment would be automatically abandoned. We would behave correctly and avoid abandoning if the payment was awaiting an invoice, but not if the payment had an invoice but the HTLCs weren't yet locked in.
👋 Thanks for assigning @joostjager as a reviewer! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4078 +/- ##
==========================================
+ Coverage 88.63% 88.64% +0.01%
==========================================
Files 176 176
Lines 131920 131922 +2
Branches 131920 131922 +2
==========================================
+ Hits 116927 116948 +21
+ Misses 12325 12311 -14
+ Partials 2668 2663 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
match self { | ||
PendingOutboundPayment::AwaitingInvoice { .. } => true, | ||
PendingOutboundPayment::AwaitingInvoice { .. } | ||
| PendingOutboundPayment::InvoiceReceived { .. } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First I thought InvoiceReceived
isn't necessary, because the payment is initiated immediately after receiving the invoice. But on a closer look, I think there are also similar gaps that require this check. There is also manual bolt12 invoice payment. No test coverage though I think.
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Tagging for backporting.
Backported in #4143 |
Previously, we had a bug that particularly affected async payments where if an outbound payment was in the state
{Static}InvoiceReceived
and there was a call toprocess_pending_htlc_forwards
, the payment would be automatically abandoned. We would behave correctly and avoid abandoning if the payment was awaiting an invoice, but not if the payment had an invoice but the HTLCs weren't yet locked in.