-
Notifications
You must be signed in to change notification settings - Fork 418
Post-#3897 fixes and cleanups #4022
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
Post-#3897 fixes and cleanups #4022
Conversation
This reverts the changes to `fuzz/src/full_stack.rs` in commit 61bc1e0 which were spuriously included after a rebase and ultimately broke the fuzzing test.
We actually store the latest counterparty revoked commitment transaction number in two places in `Channel` - as `context.cur_counterparty_commitment_transaction_number + 2` and in `commitment_secrets`. Here we add a debug assertion that both values are equal.
This cleans up `test_peer_storage` a bit to clarify what messages are actually being exchanged.
In general we shouldn't be adding new tests in `channelmanager.rs`
...somewhat cleaning up rustfmt crust.
👋 Thanks for assigning @jkczyz as a reviewer! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4022 +/- ##
==========================================
- Coverage 88.82% 88.76% -0.06%
==========================================
Files 175 175
Lines 127767 127753 -14
Branches 127767 127753 -14
==========================================
- Hits 113486 113404 -82
- Misses 11712 11784 +72
+ Partials 2569 2565 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
ACK
Simple enough as most is just reverting/moving/refactoring, going ahead landing this.
No description provided.