Skip to content

Log each condition that was violated for a stale monitor #2557

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

waterson
Copy link
Contributor

@waterson waterson commented Sep 6, 2023

There are several conditions that can be violated which indicate a stale monitor. This logs each that doesn't hold.

There are several conditions that can be violated which indicate a stale
monitor. This logs each that doesn't hold.
TheBlueMatt
TheBlueMatt previously approved these changes Sep 6, 2023
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

I'm not 100% sure that any of these are reachable without also hitting the update_id check, but, sure.

@codecov-commenter
Copy link

codecov-commenter commented Sep 6, 2023

Codecov Report

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

Comparison is base (eb44d99) 90.61% compared to head (ff8c5db) 90.60%.
Report is 6 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2557      +/-   ##
==========================================
- Coverage   90.61%   90.60%   -0.01%     
==========================================
  Files         110      110              
  Lines       57573    57795     +222     
  Branches    57573    57795     +222     
==========================================
+ Hits        52167    52367     +200     
- Misses       5406     5428      +22     
Files Changed Coverage Δ
lightning/src/ln/channelmanager.rs 87.20% <100.00%> (+0.02%) ⬆️

... and 11 files with indirect coverage changes

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

@waterson
Copy link
Contributor Author

waterson commented Sep 6, 2023

I'm not 100% sure that any of these are reachable without also hitting the update_id check, but, sure.

In the steady state, you're probably right, but it's actually how I figured out what was wrong with #2554. :)

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.

LGTM

@TheBlueMatt TheBlueMatt merged commit 82d92dd into lightningdevkit:main Sep 7, 2023
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.

5 participants