Attempt to unblock blocked monitor updates on startup by TheBlueMatt · Pull Request #4520 · lightningdevkit/rust-lightning

TheBlueMatt · 2026-03-30T01:15:33Z

When we make an MPP claim we push RAA blockers for each chanel to ensure we don't allow any single channel to make too much progress until all channels have the preimage durably on disk. We don't have to store those RAA blockers on disk in the ChannelManager as there's no point - if the ChannelManager gets to disk with the RAA blockers it also brought with it the pending ChannelMonitorUpdates that contain the preimages and will now be replayed, ensuring the preimage makes it to all ChannelMonitors.

However, just because those RAA blockers dissapear on reload doesn't mean the implications of them does too - if a later ChannelMonitorUpdate was blocked in the channel we don't have logic to unblock it on startup.

Here we add such logic, simply attempting to unblock all blocked ChannelMonitorUpdates that existed on startup.

Code written by Claude.

Fixes #4518

Needs a test, I have a start on one but need to clean it up

ldk-reviews-bot · 2026-03-30T01:15:36Z

👋 I see @tankyleo was un-assigned.
If you'd like another reviewer assignment, please click here.

joostjager · 2026-05-06T11:11:32Z

There was a unit test already in issue linked in the description. On branch main...joostjager:2026-03-startup-mpp-unblock-with-test, it's added as a commit, and shows that your fix makes the test pass.

joostjager · 2026-05-06T13:00:20Z

Discussed offline and improved unit test by ignoring some ignored read outs and 'loops until settled'.

When we make an MPP claim we push RAA blockers for each chanel to ensure we don't allow any single channel to make too much progress until all channels have the preimage durably on disk. We don't have to store those RAA blockers on disk in the ChannelManager as there's no point - if the ChannelManager gets to disk with the RAA blockers it also brought with it the pending ChannelMonitorUpdates that contain the preimages and will now be replayed, ensuring the preimage makes it to all ChannelMonitors. However, just because those RAA blockers dissapear on reload doesn't mean the implications of them does too - if a later ChannelMonitorUpdate was blocked in the channel we don't have logic to unblock it on startup. Here we add such logic, simply attempting to unblock all blocked `ChannelMonitorUpdate`s that existed on startup. Code written by Claude. Fixes lightningdevkit#4518

TheBlueMatt · 2026-05-06T20:38:52Z

Thanks! Rebased and included the test.

ldk-claude-review-bot · 2026-05-06T20:47:33Z

Good — takes msgs::UpdateFulfillHTLC by value, so the clone() in the test is needed and correct.

I've done a thorough review of the implementation and test. My prior review was correct — no issues found.

No issues found.

The implementation is correct:

New BackgroundEvent::AttemptUnblockMonitorUpdates variant — all pattern matches across the codebase are exhaustive. The variant is never serialized (background events are hardcoded to count 0 at line 18223).
Deserialization logic (lines 19471-19477) — correctly queued after MonitorUpdateRegeneratedOnStartup/MonitorUpdatesComplete events, ensuring preimage replays are processed first. Only triggers for channels with blocked_monitor_updates_pending() > 0.
Event processing (lines 8803-8808) — handle_monitor_update_release(counterparty_node_id, channel_id, None) safely gates on raa_monitor_updates_held(), which correctly checks both actions_blocking_raa_monitor_updates (empty on fresh startup) and pending_events for any ReleaseRAAChannelMonitorUpdate completion actions that may have been created during the same process_background_events pass. If a PaymentClaimed event with such an action exists, the blocked updates are held until the user processes it; otherwise they're released immediately.
Assertion at line 9765 — AttemptUnblockMonitorUpdates { .. } => false is correct; this debug assertion checks for preimage replay events for the claiming channel.
Test — comprehensive coverage of the full cycle: two-channel MPP claim with asymmetric monitor state on reload, verifying both the startup-released fulfill (channel A) and the event-completion-released fulfill (channel B).

codecov · 2026-05-06T22:18:13Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.14%. Comparing base (1d36f7b) to head (01d55dc).
⚠️ Report is 76 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4520      +/-   ##
==========================================
- Coverage   86.16%   86.14%   -0.03%     
==========================================
  Files         156      157       +1     
  Lines      108669   108958     +289     
  Branches   108669   108958     +289     
==========================================
+ Hits        93638    93857     +219     
- Misses      12420    12484      +64     
- Partials     2611     2617       +6

Flag	Coverage Δ
tests	`86.14% <100.00%> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

joostjager

Even though the fix is just a few lines, it is still hard to fully understand all the consequences. Buthandle_monitor_update_release called with None seems safe with its double-check, and it fixes the bug.

joostjager · 2026-05-07T07:25:14Z

 }

+#[test]
+fn test_mpp_claim_htlc_fulfills_unblocked_on_reload() {


I think you picked the commit message from the first commit on my branch where the test was bug-reproducing ("verifies the bug leaves an htlc stuck"), and combined it with the final test.

Oops, tweaked the commit message. Looks like we didn't get a second reviewer on this so hit the button.

ldk-reviews-bot · 2026-05-07T07:34:40Z

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

joostjager · 2026-05-07T11:28:35Z

Additional verification of this PR: #4601

ldk-reviews-bot · 2026-05-12T13:02:45Z

✅ Added second reviewer: @tankyleo

Add a characterization test for a claimed MPP payment whose preimage monitor updates are only partially persisted before restart. The test drives both channels through a held fee-update commitment dance, claims with async monitor persistence, reloads one fresh and one stale monitor, and verifies that we don't leave a sender-side HTLC stuck after reconnect.

TheBlueMatt added backport 0.1 backport 0.2 labels Mar 30, 2026

TheBlueMatt mentioned this pull request Apr 6, 2026

fuzz: Add upgrade/downgrade simulation to chanmon_consistency and fix chacha20 build #4499

Draft

This was referenced Apr 15, 2026

fuzz: cover deferred writing in chanmon_consistency #4465

Open

MPP claim HTLC fulfills stuck in holding cell after node restart #4518

Open

TheBlueMatt force-pushed the 2026-03-startup-mpp-unblock branch from a38acca to 6977e25 Compare May 6, 2026 20:38

TheBlueMatt marked this pull request as ready for review May 6, 2026 20:38

ldk-reviews-bot requested a review from joostjager May 6, 2026 20:39

TheBlueMatt force-pushed the 2026-03-startup-mpp-unblock branch from 6977e25 to 52a0030 Compare May 6, 2026 21:01

joostjager approved these changes May 7, 2026

View reviewed changes

joostjager mentioned this pull request May 7, 2026

fuzz: add chanmon stuck HTLC invariant #4601

Open

ldk-reviews-bot requested a review from tankyleo May 12, 2026 13:02

TheBlueMatt force-pushed the 2026-03-startup-mpp-unblock branch from 52a0030 to 01d55dc Compare May 12, 2026 13:13

tankyleo requested review from valentinewallace and removed request for tankyleo May 12, 2026 18:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Attempt to unblock blocked monitor updates on startup#4520

Attempt to unblock blocked monitor updates on startup#4520
TheBlueMatt wants to merge 2 commits into
lightningdevkit:mainfrom
TheBlueMatt:2026-03-startup-mpp-unblock

TheBlueMatt commented Mar 30, 2026

Uh oh!

ldk-reviews-bot commented Mar 30, 2026 •

edited

Loading

Uh oh!

joostjager commented May 6, 2026

Uh oh!

joostjager commented May 6, 2026

Uh oh!

TheBlueMatt commented May 6, 2026

Uh oh!

ldk-claude-review-bot commented May 6, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 6, 2026 •

edited

Loading

Uh oh!

joostjager left a comment

Uh oh!

joostjager May 7, 2026

Uh oh!

TheBlueMatt May 12, 2026

Uh oh!

ldk-reviews-bot commented May 7, 2026

Uh oh!

joostjager commented May 7, 2026

Uh oh!

ldk-reviews-bot commented May 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

TheBlueMatt commented Mar 30, 2026

Uh oh!

ldk-reviews-bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joostjager commented May 6, 2026

Uh oh!

joostjager commented May 6, 2026

Uh oh!

TheBlueMatt commented May 6, 2026

Uh oh!

ldk-claude-review-bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

joostjager left a comment

Choose a reason for hiding this comment

Uh oh!

joostjager May 7, 2026

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt May 12, 2026

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented May 7, 2026

Uh oh!

joostjager commented May 7, 2026

Uh oh!

ldk-reviews-bot commented May 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ldk-reviews-bot commented Mar 30, 2026 •

edited

Loading

ldk-claude-review-bot commented May 6, 2026 •

edited

Loading

codecov Bot commented May 6, 2026 •

edited

Loading