Avoid writing ChannelManager when hitting lnd bug 6039
#2937
Conversation
|
Build failure: ln::shutdown_tests::test_lnd_bug_6039 |
When we hit lnd bug 6039, we end up sending error messages to peers in a loop. This should be fine, but because we used the generic `PersistenceNotifierGuard::notify_on_drop` lock above the specific handling, we end up writing `ChannelManager` every time we manage a round-trip to our peer. This can add up quite quickly, and isn't actually changing, so we really need to avoid writing the `ChannelManager` in this case.
TheBlueMatt
force-pushed
the
2024-03-no-wake-on-shutdown
branch
from
dadd363 to
03425e4
Compare
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2937 +/- ##
==========================================
- Coverage 89.20% 89.20% -0.01%
==========================================
Files 117 117
Lines 95513 95522 +9
Branches 95513 95522 +9
==========================================
+ Hits 85203 85209 +6
- Misses 7822 7831 +9
+ Partials 2488 2482 -6 ☔ View full report in Codecov by Sentry. |
G8XSU
left a comment
G8XSU
left a comment
There was a problem hiding this comment.
Lgtm!
Waiting for CI to approve.
To me it looks safe, your call to wait for another reviewer or not.
|
|
||
| match &msg.data as &str { | ||
| "cannot co-op close channel w/ active htlcs"| | ||
| "link failed to shutdown" => |
There was a problem hiding this comment.
unrelated: is it safe to rely on these string messages?
There was a problem hiding this comment.
In the worst case lnd changes the messages and we force-close instead of sending a repeated shutdown, so...it should be?
5 participants
When we hit lnd bug 6039, we end up sending error messages to peers in a loop. This should be fine, but because we used the generic
PersistenceNotifierGuard::notify_on_droplock above the specific handling, we end up writingChannelManagerevery time we manage a round-trip to our peer.This can add up quite quickly, and isn't actually changing, so we really need to avoid writing the
ChannelManagerin this case.