Skip to content

Fix update_id gap during force_shutdown #3858

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

whfuyn
Copy link
Contributor

@whfuyn whfuyn commented Jun 13, 2025

When a channel is force-closed, there might be blocked monitor updates not yet applied. But latest_monitor_update_id has been incremented and assigned to these updates. This results in a panic when trying to apply the ChannelForceClosed update. Use the unblocked update id instead.

Resolves: #3857

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jun 13, 2025

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

@ldk-reviews-bot ldk-reviews-bot requested a review from jkczyz June 13, 2025 14:42
Copy link
Contributor

@wpaulino wpaulino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This should work, but do you mind including a test?

There are other scenarios where we also increment by 1, but those should not result in a panic as they are queued in blocked_monitor_updates. The force close update is the only one we let fly through regardless of blocked_monitor_updates.

@wpaulino wpaulino removed the request for review from jkczyz June 13, 2025 16:25
When a channel is force-closed, there might be blocked monitor updates
not yet applied. But `latest_monitor_update_id` has been incremented and
assigned to these updates. This results in a panic when trying to apply
the `ChannelForceClosed` update. Use the unblocked update id instead.

Resolves: lightningdevkit#3857
@whfuyn whfuyn force-pushed the fix-update-id-gap branch from 446155b to 5e6e74c Compare June 13, 2025 17:23
@whfuyn
Copy link
Contributor Author

whfuyn commented Jun 13, 2025

Got it! Will add a test later.

Copy link

codecov bot commented Jun 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.91%. Comparing base (3333b6d) to head (5e6e74c).
Report is 13 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3858      +/-   ##
==========================================
- Coverage   89.94%   89.91%   -0.04%     
==========================================
  Files         163      163              
  Lines      131655   131652       -3     
  Branches   131655   131652       -3     
==========================================
- Hits       118421   118371      -50     
- Misses      10550    10583      +33     
- Partials     2684     2698      +14     

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

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.

Panic when applying monitor update during channel force close
3 participants