Skip to content

Conversation

@TheBlueMatt
Copy link
Collaborator

@TheBlueMatt TheBlueMatt commented Nov 20, 2025

If we complete a ChannelMonitorUpdate persistence but there are blocked ChannelMonitorUpdates in the channel, we'll skip all the post-monitor-update logic entirely. While its correct that we can't resume the channel (as it expected the monitor updates it generated to complete, even if they ended up blocked), the post-update actions are a channelmanager.rs concept - they cannot be tied to blocked updates because channelmanager.rs doesn't even see blocked updates.

This can lead to a channel getting stuck waiting on itself. In a production environment, an LDK user saw a case where:
(a) an MPP payment was received over several channels, let's call
them A + B.
(b) channel B got into AwaitingRAA due to unrelated operations,
(c) the MPP payment was claimed, with async monitor updating,
(d) the revoke_and_ack we were waiting on was delivered, but the
resulting ChannelMonitorUpdate was blocked due to the
pending claim having inserted an RAA-blocking action,
(e) the preimage ChannelMonitorUpdate generated for channel B
completed persistence, which did nothing due to the blocked
ChannelMonitorUpdate.
(f) the Event::PaymentClaimed event was handled but it, too,
failed to unblock the channel.

Instead, here, we simply process post-update actions when an update completes, even if there are pending blocked updates. We do not fully unblock the channel, of course.

Backport of #4236 (that needs to go first so I can update the commit hash), plus a backport of #4172.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Nov 20, 2025

I've assigned @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@codecov
Copy link

codecov bot commented Nov 20, 2025

Codecov Report

❌ Patch coverage is 93.12977% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.49%. Comparing base (4c1aa13) to head (52528c7).

Files with missing lines Patch % Lines
lightning/src/ln/functional_test_utils.rs 86.04% 1 Missing and 5 partials ⚠️
lightning/src/ln/chanmon_update_fail_tests.rs 97.50% 2 Missing ⚠️
lightning/src/util/ser.rs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##              0.1    #4235   +/-   ##
=======================================
  Coverage   87.48%   87.49%           
=======================================
  Files         149      149           
  Lines      101834   101924   +90     
  Branches   101834   101924   +90     
=======================================
+ Hits        89092    89180   +88     
+ Misses      10479    10473    -6     
- Partials     2263     2271    +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@TheBlueMatt TheBlueMatt force-pushed the 2025-11-post-update-even-when-blocked-0.1 branch from 0596cb2 to 23a3719 Compare November 20, 2025 14:07
`Duration::new` adds any nanoseconds in excess of a second to the
second part. This can overflow, however, panicking. In 0.2 we
introduced a few further cases where we store `Duration`s,
specifically some when handling network messages.

Sadly, that introduced a remotely-triggerable crash where someone
can send us, for example, a malicious blinded path context which
can cause us to panic.

Found by the `onion_message` fuzzer

Backport of 7b9bde1
If we complete a `ChannelMonitorUpdate` persistence but there are
blocked `ChannelMonitorUpdate`s in the channel, we'll skip all the
post-monitor-update logic entirely. While its correct that we can't
resume the channel (as it expected the monitor updates it generated
to complete, even if they ended up blocked), the post-update
actions are a `channelmanager.rs` concept - they cannot be tied to
blocked updates because `channelmanager.rs` doesn't even see
blocked updates.

This can lead to a channel getting stuck waiting on itself. In a
production environment, an LDK user saw a case where:
 (a) an MPP payment was received over several channels, let's call
     them A + B.
 (b) channel B got into `AwaitingRAA` due to unrelated operations,
 (c) the MPP payment was claimed, with async monitor updating,
 (d) the `revoke_and_ack` we were waiting on was delivered, but the
     resulting `ChannelMonitorUpdate` was blocked due to the
     pending claim having inserted an RAA-blocking action,
 (e) the preimage `ChannelMonitorUpdate` generated for channel B
     completed persistence, which did nothing due to the blocked
     `ChannelMonitorUpdate`.
 (f) the `Event::PaymentClaimed` event was handled but it, too,
     failed to unblock the channel.

Instead, here, we simply process post-update actions when an update
completes, even if there are pending blocked updates. We do not
fully unblock the channel, of course.

XXX: Backport of XXX

Fixed conflicts in:
 * lightning/src/ln/chanmon_update_fail_tests.rs
 * lightning/src/ln/channelmanager.rs
 * lightning/src/ln/functional_test_utils.rs
@TheBlueMatt TheBlueMatt force-pushed the 2025-11-post-update-even-when-blocked-0.1 branch from 23a3719 to bad3b46 Compare November 20, 2025 14:12
@TheBlueMatt TheBlueMatt force-pushed the 2025-11-post-update-even-when-blocked-0.1 branch from bad3b46 to 35af975 Compare November 20, 2025 14:33
@TheBlueMatt TheBlueMatt reopened this Nov 20, 2025
@TheBlueMatt TheBlueMatt force-pushed the 2025-11-post-update-even-when-blocked-0.1 branch from 35af975 to 52528c7 Compare November 20, 2025 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants