Skip to content

Prevent ChannelForceClosed monitor update error after detecting spend #2265

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

Merged

Conversation

wpaulino
Copy link
Contributor

@wpaulino wpaulino commented May 4, 2023

If we detected a spend for a channel onchain prior to handling its ChannelForceClosed monitor update, we'd log a concerning error message and return an error unnecessarily. The channel has already been closed, so handling the ChannelForceClosed monitor update at this point should be a no-op.

Fixes #2263.

If we detected a spend for a channel onchain prior to handling its
`ChannelForceClosed` monitor update, we'd log a concerning error
message and return an error unnecessarily. The channel has already been
closed, so handling the `ChannelForceClosed` monitor update at this
point should be a no-op.
@TheBlueMatt
Copy link
Collaborator

Let's also log in ChannelManager when we send the force-close-cause-theres-no-channel-for-a-channelmonitor force-close-update saying something about how we don't have a channel, it was probably closed before, but we're checking again.

TheBlueMatt
TheBlueMatt previously approved these changes May 4, 2023
@wpaulino wpaulino force-pushed the channel-force-closed-update-err branch from a33a1bc to 1aeb821 Compare May 5, 2023 00:46
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01 ⚠️

Comparison is base (e94647c) 91.50% compared to head (a33a1bc) 91.49%.

❗ Current head a33a1bc differs from pull request most recent head 1aeb821. Consider uploading reports for the commit 1aeb821 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2265      +/-   ##
==========================================
- Coverage   91.50%   91.49%   -0.01%     
==========================================
  Files         104      104              
  Lines       52087    52096       +9     
  Branches    52087    52096       +9     
==========================================
+ Hits        47660    47666       +6     
- Misses       4427     4430       +3     
Impacted Files Coverage Δ
lightning/src/chain/channelmonitor.rs 94.84% <100.00%> (+0.01%) ⬆️
lightning/src/ln/channelmanager.rs 87.23% <100.00%> (+<0.01%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@TheBlueMatt TheBlueMatt merged commit 570db8b into lightningdevkit:main May 8, 2023
@wpaulino wpaulino deleted the channel-force-closed-update-err branch May 8, 2023 16:34
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.

Already-force-closed channels generate scary logs on startup
4 participants