Drop forwarded HTLCs which were still pending at persist-time #1915
Conversation
There was a problem hiding this comment.
Looks good!
Just to improve the test's readability, it might be nice to leave a quick note on/before check_closed_event! call (https://github.com/TheBlueMatt/rust-lightning/blob/236780d261d76afe28e784bf43d9f75d3c71ecfa/lightning/src/ln/reload_tests.rs#L897) mentioning that this implies checking no HTLCIntercepted event is emitted in the use_intercept case?
There was a problem hiding this comment.
CI isn't happy on older versions of rustc. LGTM otherwise.
error[E0070]: invalid left-hand side of assignment
--> lightning/src/ln/reload_tests.rs:865:49
|
865 | (intercept_id, expected_outbound_amount_msat) = match events[0] {
| --------------------------------------------- ^
| |
| cannot assign to this expression
|
= note: destructuring assignments are not currently supported
= note: for more information, see https://github.com/rust-lang/rfcs/issues/372
Codecov ReportBase: 90.71% // Head: 90.72% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1915 +/- ##
==========================================
+ Coverage 90.71% 90.72% +0.01%
==========================================
Files 94 94
Lines 49559 49603 +44
Branches 49559 49603 +44
==========================================
+ Hits 44955 45001 +46
+ Misses 4604 4602 -2
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 at Codecov. |
|
Feel free to squash, I think. |
If, after forwarding an intercepted payment to our counterparty, we restart with a ChannelMonitor update having been persisted, but the corresponding ChannelManager update not having been persisted, we'll still have the intercepted HTLC in the `pending_intercepted_htlcs` map on start (and potentially a pending `HTLCIntercepted` event). This will cause us to allow the user to handle the forwarded HTLC twice, potentially double-forwarding it. This builds on 0bb87dd, which provided a preemptive fix for the general relay case (though it was not an actual issue at the time). We simply check for the HTLCs having been forwarded on startup and remove them from the map. Fixes lightningdevkit#1858
TheBlueMatt
force-pushed
the
2022-12-jit-reload-consistency
branch
from
7889856 to
2d68183
Compare
|
Squashed. |
If, after forwarding an intercepted payment to our counterparty, we
restart with a ChannelMonitor update having been persisted, but the
corresponding ChannelManager update not having been persisted,
we'll still have the intercepted HTLC in the
pending_intercepted_htlcsmap on start (and potentially a pendingHTLCInterceptedevent). This will cause us to allow the user tohandle the forwarded HTLC twice, potentially double-forwarding it.
This builds on 0bb87dd, which
provided a preemptive fix for the general relay case (though it was
not an actual issue at the time). We simply check for the HTLCs
having been forwarded on startup and remove them from the map.
Fixes #1858