Dont use PaymentPathFailed a probe fails without making it out #1704
Conversation
This file contains hidden or 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
wpaulino
previously approved these changes
Sep 7, 2022
jkczyz
reviewed
Sep 8, 2022
TheBlueMatt
force-pushed
the
2022-09-always-probe-failed
branch
from
dd591c2 to
b143b19
Compare
Codecov Report
@@ Coverage Diff @@
## main #1704 +/- ##
==========================================
+ Coverage 90.93% 90.96% +0.02%
==========================================
Files 86 86
Lines 46840 46848 +8
Branches 46840 46848 +8
==========================================
+ Hits 42594 42613 +19
+ Misses 4246 4235 -11
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
`fail_holding_cell_htlcs` calls through to `fail_htlc_backwards_internal` for HTLCs that need to be failed-backwards but opts to generate its own payment failure events for `HTLCSource:;OutboundRoute` HTLCs. There is no reason for that as `fail_htlc_backwards_internal` will also happily generate (now-)equivalent events for `HTLCSource::OutboundRoute` HTLCs. Thus, we can drop the redundant code and always call `fail_htlc_backwards_internal` for each HTLC in `fail_holding_cell_htlcs`.
When we fail to forward a probe HTLC at all and immediately fail it (e.g. due to the first hop channel closing) we'd previously spuriously generate only a `PaymentPathFailed` event. This violates the expected API, as users expect a `ProbeFailed` event instead. This fixes the oversight by ensuring we generate the correct event. Thanks to @jkczyz for pointing out this issue.
|
Squashed without further changes. |
TheBlueMatt
force-pushed
the
2022-09-always-probe-failed
branch
from
b143b19 to
6601192
Compare
wpaulino
approved these changes
Sep 8, 2022
jkczyz
approved these changes
Sep 8, 2022
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.
Based on #1702.
When we fail to forward a probe HTLC at all and immediately fail it
(e.g. due to the first hop channel closing) we'd previously
spuriously generate only a
PaymentPathFailedevent. This violatesthe expected API, as users expect a
ProbeFailedevent instead.This fixes the oversight by ensuring we generate the correct event.
Thanks to @jkczyz for pointing out this issue.