Allow claiming a payment if a channel with an HTLC has closed #2148
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #2148 +/- ##
==========================================
+ Coverage 91.35% 91.98% +0.63%
==========================================
Files 102 102
Lines 49909 54805 +4896
Branches 49909 54805 +4896
==========================================
+ Hits 45592 50413 +4821
- Misses 4317 4392 +75
... and 8 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
wpaulino
reviewed
Apr 4, 2023
Yea, I think its fine if we put the timeout in the event? We could guarantee the timeout will be at least two blocks away but I'm not sure if we need to? |
TheBlueMatt
force-pushed
the
2023-04-claim-from-closed
branch
from
d4448f5
to
3c5c6a1
Compare
|
LGTM after squash |
Previously, LDK would refuse to claim a payment if a channel on which the payment was received had been closed between when the HTLC was received and when we went to claim it. This makes sense in the payment case - why pay an on-chain fee to claim the HTLC when presumably the sender may retry later. Long ago it also reduced total code in the claim pipeline. However, this doesn't make sense if you're trying to do an atomic swap or some other protocol that requires atomicity with some other action - if your money got claimed elsewhere you need to be able to claim the HTLC in lightning no matter what. Further, this is an over-optimization - there should be a very, very low likelihood that a channel closes between when we receive the last HTLC for a payment and the user goes to claim the payment. Since we now have code to handle this anyway we should allow it. Fixes lightningdevkit#2017.
TheBlueMatt
force-pushed
the
2023-04-claim-from-closed
branch
from
3c5c6a1
to
8b086ae
Compare
|
Squashed without further changes. |
tnull
previously approved these changes
Apr 5, 2023
|
Feel free to squash the fixup! |
Now that we guarantee `claim_payment` will always succeed we have to let the user know what the deadline is. We still fail payments if they haven't been claimed in time, which we now expose in `PaymentClaimable`.
TheBlueMatt
force-pushed
the
2023-04-claim-from-closed
branch
from
8be723c
to
4a8d01d
Compare
wpaulino
approved these changes
Apr 6, 2023
tnull
approved these changes
Apr 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, LDK would refuse to claim a payment if a channel on
which the payment was received had been closed between when the
HTLC was received and when we went to claim it. This makes sense in
the payment case - why pay an on-chain fee to claim the HTLC when
presumably the sender may retry later. Long ago it also reduced
total code in the claim pipeline.
However, this doesn't make sense if you're trying to do an atomic
swap or some other protocol that requires atomicity with some other
action - if your money got claimed elsewhere you need to be able to
claim the HTLC in lightning no matter what. Further, this is an
over-optimization - there should be a very, very low likelihood
that a channel closes between when we receive the last HTLC for a
payment and the user goes to claim the payment. Since we now have
code to handle this anyway we should allow it.
Fixes #2017.