-
Notifications
You must be signed in to change notification settings - Fork 385
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
Add persistence test for htlc in the LocalRemoved state #3645
base: main
Are you sure you want to change the base?
Add persistence test for htlc in the LocalRemoved state #3645
Conversation
Increase coverage and prepare for attributable failures which are going to extend the update_fail_htlc message with an additional field that needs to be persisted as well.
I've assigned @valentinewallace as a reviewer! |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3645 +/- ##
==========================================
+ Coverage 89.16% 90.25% +1.08%
==========================================
Files 152 155 +3
Lines 118791 127106 +8315
Branches 118791 127106 +8315
==========================================
+ Hits 105921 114715 +8794
+ Misses 10312 9940 -372
+ Partials 2558 2451 -107 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
@@ -15306,6 +15306,80 @@ mod tests { | |||
nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "Payment preimage didn't match payment hash", 1); | |||
} | |||
|
|||
#[test] | |||
fn test_htlc_localremoved_persistence() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're generally trying to avoid adding any tests in channelmanager.rs
since it's already massive. Could we move this to reload_tests.rs
or another test file?
@@ -15306,6 +15306,80 @@ mod tests { | |||
nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "Payment preimage didn't match payment hash", 1); | |||
} | |||
|
|||
#[test] | |||
fn test_htlc_localremoved_persistence() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind adding a comment with a high level summary of what the test is covering?
👋 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. |
Increase coverage and prepare for attributable failures which are going to extend the update_fail_htlc message with an additional field that needs to be persisted as well (#3611)