Skip to content

Conversation

@joostjager
Copy link
Contributor

@joostjager joostjager commented Nov 11, 2025

Add a new safe_channels feature flag that gates in-development work toward persisting channel monitors and channels atomically, preventing them from desynchronizing and causing force closures.

This commit begins that transition by storing both pieces together and adding consistency checks during writes. These checks mirror what the channel manager currently validates only on reload, but performing them earlier increases coverage and surfaces inconsistencies sooner.

Builds on #4217

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Nov 11, 2025

👋 Thanks for assigning @valentinewallace 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.

@joostjager joostjager force-pushed the chan-monitor-consistency-check branch 2 times, most recently from d25845e to dda4c5c Compare November 14, 2025 10:40
joostjager and others added 5 commits November 14, 2025 11:44
As a preparatory step for unifying test log output.
Makes sure that 4 and 5 character log levels line up.
Now that we have Display implemented on Record, the various
test logger implementations can make use of that.
Previously, log messages often included the channel ID both in the
message text and in the structured `channel_id` field. This led to
redundant information and made it difficult to quickly identify which
log lines pertain to a given channel, as the ID could appear in
different positions or sometimes not at all.

This change removes the channel ID from the message in all cases where
it is already present in the structured field. A test run was used to
verify that the structured field always appeared in these log calls.
Some exceptions remain—for example, calls where the structured field
contains a temporary channel ID and the message contains the final ID
were left unchanged.
Since most log messages no longer include the channel ID in their text,
this commit updates the test logger to output the structured channel ID
instead. The ID is truncated for readability. Similarly peer id and
payment hash are logged.

Co-authored-by: Matt Corallo <[email protected]>
@joostjager joostjager force-pushed the chan-monitor-consistency-check branch from dda4c5c to b00ff03 Compare November 14, 2025 10:56
@joostjager joostjager changed the title add channel <-> channel monitor consistency check Channel <-> channel monitor consistency check Nov 14, 2025
@joostjager joostjager force-pushed the chan-monitor-consistency-check branch from b00ff03 to c3771b6 Compare November 14, 2025 10:58
Additional trace logs to help with debugging.
@joostjager joostjager force-pushed the chan-monitor-consistency-check branch from c3771b6 to f387f90 Compare November 14, 2025 11:01
@codecov
Copy link

codecov bot commented Nov 14, 2025

Codecov Report

❌ Patch coverage is 90.67164% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.32%. Comparing base (d4828c0) to head (a82719b).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
lightning/src/ln/channelmanager.rs 79.72% 14 Missing and 1 partial ⚠️
lightning/src/util/logger.rs 79.54% 0 Missing and 9 partials ⚠️
lightning/src/chain/channelmonitor.rs 94.44% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4218      +/-   ##
==========================================
- Coverage   89.36%   89.32%   -0.05%     
==========================================
  Files         180      180              
  Lines      138369   138321      -48     
  Branches   138369   138321      -48     
==========================================
- Hits       123652   123550     -102     
- Misses      12114    12157      +43     
- Partials     2603     2614      +11     
Flag Coverage Δ
fuzzing 35.93% <50.98%> (+0.05%) ⬆️
tests 88.68% <90.67%> (-0.04%) ⬇️

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 joostjager force-pushed the chan-monitor-consistency-check branch 2 times, most recently from d83d7de to 00a95f1 Compare November 14, 2025 18:57
Add a new `safe_channels` feature flag that gates in-development work
toward persisting channel monitors and channels atomically, preventing
them from desynchronizing and causing force closures.

This commit begins that transition by storing both pieces together and
adding consistency checks during writes. These checks mirror what the
channel manager currently validates only on reload, but performing them
earlier increases coverage and surfaces inconsistencies sooner.
@joostjager joostjager force-pushed the chan-monitor-consistency-check branch from 00a95f1 to a82719b Compare November 14, 2025 19:36
@joostjager joostjager changed the title Channel <-> channel monitor consistency check Store and check channel data in channel monitors Add a new safe_channels feature flag that gates in-development work toward persisting channel monitors and channels atomically, preventing them from desynchronizing and causing force closures. This commit begins that transition by storing both pieces together and adding consistency checks during writes. These checks mirror what the channel manager currently validates only on reload, but performing them earlier increases coverage and surfaces inconsistencies sooner. Nov 14, 2025
@joostjager joostjager changed the title Store and check channel data in channel monitors Add a new safe_channels feature flag that gates in-development work toward persisting channel monitors and channels atomically, preventing them from desynchronizing and causing force closures. This commit begins that transition by storing both pieces together and adding consistency checks during writes. These checks mirror what the channel manager currently validates only on reload, but performing them earlier increases coverage and surfaces inconsistencies sooner. Store and check channel data in channel monitors Nov 14, 2025
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