Return Result from Watch::update_channel to separate monitor failure from persistence status#4445
Closed
joostjager wants to merge 1 commit intolightningdevkit:mainfrom
Closed
Return Result from Watch::update_channel to separate monitor failure from persistence status#4445joostjager wants to merge 1 commit intolightningdevkit:mainfrom
joostjager wants to merge 1 commit intolightningdevkit:mainfrom
Conversation
…from persistence status Change Watch::update_channel to return Result<ChannelMonitorUpdateStatus, ()> instead of bare ChannelMonitorUpdateStatus. This cleanly separates two distinct concerns that were previously conflated: - Ok(status): the persistence status (Completed, InProgress, or UnrecoverableError), representing whether the monitor was durably persisted. - Err(()): ChannelMonitor::update_monitor failed internally, generally implying the channel has been closed on-chain. Previously, ChainMonitor would override the Persist result to InProgress when update_monitor failed, repurposing the async persistence mechanism to freeze the channel. This was problematic because it could switch a synchronous persister's channel into async mode, violating the expectation that Completed and InProgress modes are not mixed. Now, ChainMonitor returns Err(()) when update_monitor fails, and ChannelManager maps this to InProgress behavior internally to freeze the channel with minimal diff. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
👋 Hi! I see this is a draft PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4445 +/- ##
==========================================
+ Coverage 85.94% 85.96% +0.02%
==========================================
Files 159 159
Lines 104644 104649 +5
Branches 104644 104649 +5
==========================================
+ Hits 89934 89960 +26
+ Misses 12204 12185 -19
+ Partials 2506 2504 -2
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:
|
Contributor
Author
|
Closing because of incompatibility with future remote chain monitor impls |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.