Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor - #4435

Closed
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation
Closed

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor#4435
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation

Conversation

@joostjager

Copy link
Copy Markdown
Contributor

When ChannelMonitor::update_monitor fails (e.g. a counterparty commitment_signed arrives after funding was spent), ChainMonitor persists the full monitor successfully but overrides the return value to InProgress. If the user's Persist impl only returns Completed, this causes ChannelManager::handle_monitor_update_res to panic on the mode mismatch.

The root cause is that the mode check lived in ChannelManager, which sees the post-override status. By moving the check to ChainMonitor and running it against the rawPersist result (before the update_res.is_err() override), we validate only the user's persister behavior. ChainMonitor's internal override to InProgress for failed monitor updates no longer triggers a false mode violation.

@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from 79be78c to abe9d13CompareFebruary 23, 2026 11:39
…inMonitor
When `ChannelMonitor::update_monitor` fails (e.g. a counterparty
`commitment_signed` arrives after funding was spent), `ChainMonitor`
persists the full monitor successfully but overrides the return value
to `InProgress`. If the user's `Persist` impl only returns `Completed`,
this causes `ChannelManager::handle_monitor_update_res` to panic on
the mode mismatch.
The root cause is that the mode check lived in `ChannelManager`, which
sees the *post-override* status. By moving the check to `ChainMonitor`
and running it against the *raw* `Persist` result (before the
`update_res.is_err()` override), we validate only the user's persister
behavior. ChainMonitor's internal override to `InProgress` for failed
monitor updates no longer triggers a false mode violation.
Also fix the `chanmon_consistency` fuzzer's `reload_node`, which
hardcoded the persister to `Completed` before calling `watch_channel`,
then switched to the node's actual mode afterward. With the mode check
now in `ChainMonitor`, this caused a false violation. Set the persister
to the correct mode before calling `watch_channel` so `ChainMonitor`
sees a consistent persist mode from the start.
Additionally, mirror pending monitor updates into
`TestChainMonitor.latest_monitors` after `watch_channel` returns
`InProgress`. The real `ChainMonitor` tracks the initial `update_id`
as pending, but `TestChainMonitor` was unaware of it since
`watch_channel` is called on the real `ChainMonitor` directly
(bypassing `TestChainMonitor`). Without this,
`complete_all_monitor_updates` could never drain and complete the
update, leaving reloaded node channels permanently stuck.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from abe9d13 to 7e0a4dcCompareFebruary 23, 2026 14:00
@codecov

codecovBot commented Feb 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.86%. Comparing base (2d2151a) to head (7e0a4dc).
⚠️ Report is 2 commits behind head on main.

Files with missing linesPatch %Lines
lightning/src/chain/chainmonitor.rs72.22%5 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4435 +/- ##
==========================================
- Coverage 85.86% 85.86% -0.01% 
==========================================
Files 159 159 Lines 104302 104308 +6 Branches 104302 104308 +6 ==========================================
+ Hits 89558 89563 +5 + Misses 12246 12242 -4 - Partials 2498 2503 +5 
FlagCoverage Δ
tests85.86% <73.68%> (-0.01%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joostjager

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of alternative fix: #4436

Sign up for freeto 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.

2 participants

@joostjager@ldk-reviews-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor - #4435

Closed
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation
Closed

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor#4435
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation

Conversation

@joostjager

Copy link
Copy Markdown
Contributor

When ChannelMonitor::update_monitor fails (e.g. a counterparty commitment_signed arrives after funding was spent), ChainMonitor persists the full monitor successfully but overrides the return value to InProgress. If the user's Persist impl only returns Completed, this causes ChannelManager::handle_monitor_update_res to panic on the mode mismatch.

The root cause is that the mode check lived in ChannelManager, which sees the post-override status. By moving the check to ChainMonitor and running it against the rawPersist result (before the update_res.is_err() override), we validate only the user's persister behavior. ChainMonitor's internal override to InProgress for failed monitor updates no longer triggers a false mode violation.

@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from 79be78c to abe9d13CompareFebruary 23, 2026 11:39
…inMonitor
When `ChannelMonitor::update_monitor` fails (e.g. a counterparty
`commitment_signed` arrives after funding was spent), `ChainMonitor`
persists the full monitor successfully but overrides the return value
to `InProgress`. If the user's `Persist` impl only returns `Completed`,
this causes `ChannelManager::handle_monitor_update_res` to panic on
the mode mismatch.
The root cause is that the mode check lived in `ChannelManager`, which
sees the *post-override* status. By moving the check to `ChainMonitor`
and running it against the *raw* `Persist` result (before the
`update_res.is_err()` override), we validate only the user's persister
behavior. ChainMonitor's internal override to `InProgress` for failed
monitor updates no longer triggers a false mode violation.
Also fix the `chanmon_consistency` fuzzer's `reload_node`, which
hardcoded the persister to `Completed` before calling `watch_channel`,
then switched to the node's actual mode afterward. With the mode check
now in `ChainMonitor`, this caused a false violation. Set the persister
to the correct mode before calling `watch_channel` so `ChainMonitor`
sees a consistent persist mode from the start.
Additionally, mirror pending monitor updates into
`TestChainMonitor.latest_monitors` after `watch_channel` returns
`InProgress`. The real `ChainMonitor` tracks the initial `update_id`
as pending, but `TestChainMonitor` was unaware of it since
`watch_channel` is called on the real `ChainMonitor` directly
(bypassing `TestChainMonitor`). Without this,
`complete_all_monitor_updates` could never drain and complete the
update, leaving reloaded node channels permanently stuck.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from abe9d13 to 7e0a4dcCompareFebruary 23, 2026 14:00
@codecov

codecovBot commented Feb 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.86%. Comparing base (2d2151a) to head (7e0a4dc).
⚠️ Report is 2 commits behind head on main.

Files with missing linesPatch %Lines
lightning/src/chain/chainmonitor.rs72.22%5 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4435 +/- ##
==========================================
- Coverage 85.86% 85.86% -0.01% 
==========================================
Files 159 159 Lines 104302 104308 +6 Branches 104302 104308 +6 ==========================================
+ Hits 89558 89563 +5 + Misses 12246 12242 -4 - Partials 2498 2503 +5 
FlagCoverage Δ
tests85.86% <73.68%> (-0.01%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joostjager

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of alternative fix: #4436

Sign up for freeto 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.

2 participants

@joostjager@ldk-reviews-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor - #4435

Closed
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation
Closed

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor#4435
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation

Conversation

@joostjager

Copy link
Copy Markdown
Contributor

When ChannelMonitor::update_monitor fails (e.g. a counterparty commitment_signed arrives after funding was spent), ChainMonitor persists the full monitor successfully but overrides the return value to InProgress. If the user's Persist impl only returns Completed, this causes ChannelManager::handle_monitor_update_res to panic on the mode mismatch.

The root cause is that the mode check lived in ChannelManager, which sees the post-override status. By moving the check to ChainMonitor and running it against the rawPersist result (before the update_res.is_err() override), we validate only the user's persister behavior. ChainMonitor's internal override to InProgress for failed monitor updates no longer triggers a false mode violation.

@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from 79be78c to abe9d13CompareFebruary 23, 2026 11:39
…inMonitor
When `ChannelMonitor::update_monitor` fails (e.g. a counterparty
`commitment_signed` arrives after funding was spent), `ChainMonitor`
persists the full monitor successfully but overrides the return value
to `InProgress`. If the user's `Persist` impl only returns `Completed`,
this causes `ChannelManager::handle_monitor_update_res` to panic on
the mode mismatch.
The root cause is that the mode check lived in `ChannelManager`, which
sees the *post-override* status. By moving the check to `ChainMonitor`
and running it against the *raw* `Persist` result (before the
`update_res.is_err()` override), we validate only the user's persister
behavior. ChainMonitor's internal override to `InProgress` for failed
monitor updates no longer triggers a false mode violation.
Also fix the `chanmon_consistency` fuzzer's `reload_node`, which
hardcoded the persister to `Completed` before calling `watch_channel`,
then switched to the node's actual mode afterward. With the mode check
now in `ChainMonitor`, this caused a false violation. Set the persister
to the correct mode before calling `watch_channel` so `ChainMonitor`
sees a consistent persist mode from the start.
Additionally, mirror pending monitor updates into
`TestChainMonitor.latest_monitors` after `watch_channel` returns
`InProgress`. The real `ChainMonitor` tracks the initial `update_id`
as pending, but `TestChainMonitor` was unaware of it since
`watch_channel` is called on the real `ChainMonitor` directly
(bypassing `TestChainMonitor`). Without this,
`complete_all_monitor_updates` could never drain and complete the
update, leaving reloaded node channels permanently stuck.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from abe9d13 to 7e0a4dcCompareFebruary 23, 2026 14:00
@codecov

codecovBot commented Feb 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.86%. Comparing base (2d2151a) to head (7e0a4dc).
⚠️ Report is 2 commits behind head on main.

Files with missing linesPatch %Lines
lightning/src/chain/chainmonitor.rs72.22%5 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4435 +/- ##
==========================================
- Coverage 85.86% 85.86% -0.01% 
==========================================
Files 159 159 Lines 104302 104308 +6 Branches 104302 104308 +6 ==========================================
+ Hits 89558 89563 +5 + Misses 12246 12242 -4 - Partials 2498 2503 +5 
FlagCoverage Δ
tests85.86% <73.68%> (-0.01%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joostjager

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of alternative fix: #4436

Sign up for freeto 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.

2 participants

@joostjager@ldk-reviews-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor - #4435

Closed
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation
Closed

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor#4435
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation

Conversation

@joostjager

Copy link
Copy Markdown
Contributor

When ChannelMonitor::update_monitor fails (e.g. a counterparty commitment_signed arrives after funding was spent), ChainMonitor persists the full monitor successfully but overrides the return value to InProgress. If the user's Persist impl only returns Completed, this causes ChannelManager::handle_monitor_update_res to panic on the mode mismatch.

The root cause is that the mode check lived in ChannelManager, which sees the post-override status. By moving the check to ChainMonitor and running it against the rawPersist result (before the update_res.is_err() override), we validate only the user's persister behavior. ChainMonitor's internal override to InProgress for failed monitor updates no longer triggers a false mode violation.

@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from 79be78c to abe9d13CompareFebruary 23, 2026 11:39
…inMonitor
When `ChannelMonitor::update_monitor` fails (e.g. a counterparty
`commitment_signed` arrives after funding was spent), `ChainMonitor`
persists the full monitor successfully but overrides the return value
to `InProgress`. If the user's `Persist` impl only returns `Completed`,
this causes `ChannelManager::handle_monitor_update_res` to panic on
the mode mismatch.
The root cause is that the mode check lived in `ChannelManager`, which
sees the *post-override* status. By moving the check to `ChainMonitor`
and running it against the *raw* `Persist` result (before the
`update_res.is_err()` override), we validate only the user's persister
behavior. ChainMonitor's internal override to `InProgress` for failed
monitor updates no longer triggers a false mode violation.
Also fix the `chanmon_consistency` fuzzer's `reload_node`, which
hardcoded the persister to `Completed` before calling `watch_channel`,
then switched to the node's actual mode afterward. With the mode check
now in `ChainMonitor`, this caused a false violation. Set the persister
to the correct mode before calling `watch_channel` so `ChainMonitor`
sees a consistent persist mode from the start.
Additionally, mirror pending monitor updates into
`TestChainMonitor.latest_monitors` after `watch_channel` returns
`InProgress`. The real `ChainMonitor` tracks the initial `update_id`
as pending, but `TestChainMonitor` was unaware of it since
`watch_channel` is called on the real `ChainMonitor` directly
(bypassing `TestChainMonitor`). Without this,
`complete_all_monitor_updates` could never drain and complete the
update, leaving reloaded node channels permanently stuck.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from abe9d13 to 7e0a4dcCompareFebruary 23, 2026 14:00
@codecov

codecovBot commented Feb 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.86%. Comparing base (2d2151a) to head (7e0a4dc).
⚠️ Report is 2 commits behind head on main.

Files with missing linesPatch %Lines
lightning/src/chain/chainmonitor.rs72.22%5 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4435 +/- ##
==========================================
- Coverage 85.86% 85.86% -0.01% 
==========================================
Files 159 159 Lines 104302 104308 +6 Branches 104302 104308 +6 ==========================================
+ Hits 89558 89563 +5 + Misses 12246 12242 -4 - Partials 2498 2503 +5 
FlagCoverage Δ
tests85.86% <73.68%> (-0.01%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joostjager

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of alternative fix: #4436

Sign up for freeto 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.

2 participants

@joostjager@ldk-reviews-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor - #4435

Closed
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation
Closed

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor#4435
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation

Conversation

@joostjager

Copy link
Copy Markdown
Contributor

When ChannelMonitor::update_monitor fails (e.g. a counterparty commitment_signed arrives after funding was spent), ChainMonitor persists the full monitor successfully but overrides the return value to InProgress. If the user's Persist impl only returns Completed, this causes ChannelManager::handle_monitor_update_res to panic on the mode mismatch.

The root cause is that the mode check lived in ChannelManager, which sees the post-override status. By moving the check to ChainMonitor and running it against the rawPersist result (before the update_res.is_err() override), we validate only the user's persister behavior. ChainMonitor's internal override to InProgress for failed monitor updates no longer triggers a false mode violation.

@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from 79be78c to abe9d13CompareFebruary 23, 2026 11:39
…inMonitor
When `ChannelMonitor::update_monitor` fails (e.g. a counterparty
`commitment_signed` arrives after funding was spent), `ChainMonitor`
persists the full monitor successfully but overrides the return value
to `InProgress`. If the user's `Persist` impl only returns `Completed`,
this causes `ChannelManager::handle_monitor_update_res` to panic on
the mode mismatch.
The root cause is that the mode check lived in `ChannelManager`, which
sees the *post-override* status. By moving the check to `ChainMonitor`
and running it against the *raw* `Persist` result (before the
`update_res.is_err()` override), we validate only the user's persister
behavior. ChainMonitor's internal override to `InProgress` for failed
monitor updates no longer triggers a false mode violation.
Also fix the `chanmon_consistency` fuzzer's `reload_node`, which
hardcoded the persister to `Completed` before calling `watch_channel`,
then switched to the node's actual mode afterward. With the mode check
now in `ChainMonitor`, this caused a false violation. Set the persister
to the correct mode before calling `watch_channel` so `ChainMonitor`
sees a consistent persist mode from the start.
Additionally, mirror pending monitor updates into
`TestChainMonitor.latest_monitors` after `watch_channel` returns
`InProgress`. The real `ChainMonitor` tracks the initial `update_id`
as pending, but `TestChainMonitor` was unaware of it since
`watch_channel` is called on the real `ChainMonitor` directly
(bypassing `TestChainMonitor`). Without this,
`complete_all_monitor_updates` could never drain and complete the
update, leaving reloaded node channels permanently stuck.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from abe9d13 to 7e0a4dcCompareFebruary 23, 2026 14:00
@codecov

codecovBot commented Feb 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.86%. Comparing base (2d2151a) to head (7e0a4dc).
⚠️ Report is 2 commits behind head on main.

Files with missing linesPatch %Lines
lightning/src/chain/chainmonitor.rs72.22%5 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4435 +/- ##
==========================================
- Coverage 85.86% 85.86% -0.01% 
==========================================
Files 159 159 Lines 104302 104308 +6 Branches 104302 104308 +6 ==========================================
+ Hits 89558 89563 +5 + Misses 12246 12242 -4 - Partials 2498 2503 +5 
FlagCoverage Δ
tests85.86% <73.68%> (-0.01%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joostjager

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of alternative fix: #4436

Sign up for freeto 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.

2 participants

@joostjager@ldk-reviews-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor - #4435

Closed
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation
Closed

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor#4435
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation

Conversation

@joostjager

Copy link
Copy Markdown
Contributor

When ChannelMonitor::update_monitor fails (e.g. a counterparty commitment_signed arrives after funding was spent), ChainMonitor persists the full monitor successfully but overrides the return value to InProgress. If the user's Persist impl only returns Completed, this causes ChannelManager::handle_monitor_update_res to panic on the mode mismatch.

The root cause is that the mode check lived in ChannelManager, which sees the post-override status. By moving the check to ChainMonitor and running it against the rawPersist result (before the update_res.is_err() override), we validate only the user's persister behavior. ChainMonitor's internal override to InProgress for failed monitor updates no longer triggers a false mode violation.

@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from 79be78c to abe9d13CompareFebruary 23, 2026 11:39
…inMonitor
When `ChannelMonitor::update_monitor` fails (e.g. a counterparty
`commitment_signed` arrives after funding was spent), `ChainMonitor`
persists the full monitor successfully but overrides the return value
to `InProgress`. If the user's `Persist` impl only returns `Completed`,
this causes `ChannelManager::handle_monitor_update_res` to panic on
the mode mismatch.
The root cause is that the mode check lived in `ChannelManager`, which
sees the *post-override* status. By moving the check to `ChainMonitor`
and running it against the *raw* `Persist` result (before the
`update_res.is_err()` override), we validate only the user's persister
behavior. ChainMonitor's internal override to `InProgress` for failed
monitor updates no longer triggers a false mode violation.
Also fix the `chanmon_consistency` fuzzer's `reload_node`, which
hardcoded the persister to `Completed` before calling `watch_channel`,
then switched to the node's actual mode afterward. With the mode check
now in `ChainMonitor`, this caused a false violation. Set the persister
to the correct mode before calling `watch_channel` so `ChainMonitor`
sees a consistent persist mode from the start.
Additionally, mirror pending monitor updates into
`TestChainMonitor.latest_monitors` after `watch_channel` returns
`InProgress`. The real `ChainMonitor` tracks the initial `update_id`
as pending, but `TestChainMonitor` was unaware of it since
`watch_channel` is called on the real `ChainMonitor` directly
(bypassing `TestChainMonitor`). Without this,
`complete_all_monitor_updates` could never drain and complete the
update, leaving reloaded node channels permanently stuck.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from abe9d13 to 7e0a4dcCompareFebruary 23, 2026 14:00
@codecov

codecovBot commented Feb 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.86%. Comparing base (2d2151a) to head (7e0a4dc).
⚠️ Report is 2 commits behind head on main.

Files with missing linesPatch %Lines
lightning/src/chain/chainmonitor.rs72.22%5 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4435 +/- ##
==========================================
- Coverage 85.86% 85.86% -0.01% 
==========================================
Files 159 159 Lines 104302 104308 +6 Branches 104302 104308 +6 ==========================================
+ Hits 89558 89563 +5 + Misses 12246 12242 -4 - Partials 2498 2503 +5 
FlagCoverage Δ
tests85.86% <73.68%> (-0.01%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joostjager

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of alternative fix: #4436

Sign up for freeto 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.

2 participants

@joostjager@ldk-reviews-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor - #4435

Closed
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation
Closed

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor#4435
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation

Conversation

@joostjager

Copy link
Copy Markdown
Contributor

When ChannelMonitor::update_monitor fails (e.g. a counterparty commitment_signed arrives after funding was spent), ChainMonitor persists the full monitor successfully but overrides the return value to InProgress. If the user's Persist impl only returns Completed, this causes ChannelManager::handle_monitor_update_res to panic on the mode mismatch.

The root cause is that the mode check lived in ChannelManager, which sees the post-override status. By moving the check to ChainMonitor and running it against the rawPersist result (before the update_res.is_err() override), we validate only the user's persister behavior. ChainMonitor's internal override to InProgress for failed monitor updates no longer triggers a false mode violation.

@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from 79be78c to abe9d13CompareFebruary 23, 2026 11:39
…inMonitor
When `ChannelMonitor::update_monitor` fails (e.g. a counterparty
`commitment_signed` arrives after funding was spent), `ChainMonitor`
persists the full monitor successfully but overrides the return value
to `InProgress`. If the user's `Persist` impl only returns `Completed`,
this causes `ChannelManager::handle_monitor_update_res` to panic on
the mode mismatch.
The root cause is that the mode check lived in `ChannelManager`, which
sees the *post-override* status. By moving the check to `ChainMonitor`
and running it against the *raw* `Persist` result (before the
`update_res.is_err()` override), we validate only the user's persister
behavior. ChainMonitor's internal override to `InProgress` for failed
monitor updates no longer triggers a false mode violation.
Also fix the `chanmon_consistency` fuzzer's `reload_node`, which
hardcoded the persister to `Completed` before calling `watch_channel`,
then switched to the node's actual mode afterward. With the mode check
now in `ChainMonitor`, this caused a false violation. Set the persister
to the correct mode before calling `watch_channel` so `ChainMonitor`
sees a consistent persist mode from the start.
Additionally, mirror pending monitor updates into
`TestChainMonitor.latest_monitors` after `watch_channel` returns
`InProgress`. The real `ChainMonitor` tracks the initial `update_id`
as pending, but `TestChainMonitor` was unaware of it since
`watch_channel` is called on the real `ChainMonitor` directly
(bypassing `TestChainMonitor`). Without this,
`complete_all_monitor_updates` could never drain and complete the
update, leaving reloaded node channels permanently stuck.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from abe9d13 to 7e0a4dcCompareFebruary 23, 2026 14:00
@codecov

codecovBot commented Feb 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.86%. Comparing base (2d2151a) to head (7e0a4dc).
⚠️ Report is 2 commits behind head on main.

Files with missing linesPatch %Lines
lightning/src/chain/chainmonitor.rs72.22%5 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4435 +/- ##
==========================================
- Coverage 85.86% 85.86% -0.01% 
==========================================
Files 159 159 Lines 104302 104308 +6 Branches 104302 104308 +6 ==========================================
+ Hits 89558 89563 +5 + Misses 12246 12242 -4 - Partials 2498 2503 +5 
FlagCoverage Δ
tests85.86% <73.68%> (-0.01%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joostjager

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of alternative fix: #4436

Sign up for freeto 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.

2 participants

@joostjager@ldk-reviews-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor - #4435

Closed
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation
Closed

Move ChannelMonitorUpdateStatus mode check from ChannelManager to ChainMonitor#4435
joostjager wants to merge 1 commit into
lightningdevkit:mainfrom
joostjager:fix-chainmonitor-mode-violation

Conversation

@joostjager

Copy link
Copy Markdown
Contributor

When ChannelMonitor::update_monitor fails (e.g. a counterparty commitment_signed arrives after funding was spent), ChainMonitor persists the full monitor successfully but overrides the return value to InProgress. If the user's Persist impl only returns Completed, this causes ChannelManager::handle_monitor_update_res to panic on the mode mismatch.

The root cause is that the mode check lived in ChannelManager, which sees the post-override status. By moving the check to ChainMonitor and running it against the rawPersist result (before the update_res.is_err() override), we validate only the user's persister behavior. ChainMonitor's internal override to InProgress for failed monitor updates no longer triggers a false mode violation.

@ldk-reviews-bot

Copy link
Copy Markdown

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from 79be78c to abe9d13CompareFebruary 23, 2026 11:39
…inMonitor
When `ChannelMonitor::update_monitor` fails (e.g. a counterparty
`commitment_signed` arrives after funding was spent), `ChainMonitor`
persists the full monitor successfully but overrides the return value
to `InProgress`. If the user's `Persist` impl only returns `Completed`,
this causes `ChannelManager::handle_monitor_update_res` to panic on
the mode mismatch.
The root cause is that the mode check lived in `ChannelManager`, which
sees the *post-override* status. By moving the check to `ChainMonitor`
and running it against the *raw* `Persist` result (before the
`update_res.is_err()` override), we validate only the user's persister
behavior. ChainMonitor's internal override to `InProgress` for failed
monitor updates no longer triggers a false mode violation.
Also fix the `chanmon_consistency` fuzzer's `reload_node`, which
hardcoded the persister to `Completed` before calling `watch_channel`,
then switched to the node's actual mode afterward. With the mode check
now in `ChainMonitor`, this caused a false violation. Set the persister
to the correct mode before calling `watch_channel` so `ChainMonitor`
sees a consistent persist mode from the start.
Additionally, mirror pending monitor updates into
`TestChainMonitor.latest_monitors` after `watch_channel` returns
`InProgress`. The real `ChainMonitor` tracks the initial `update_id`
as pending, but `TestChainMonitor` was unaware of it since
`watch_channel` is called on the real `ChainMonitor` directly
(bypassing `TestChainMonitor`). Without this,
`complete_all_monitor_updates` could never drain and complete the
update, leaving reloaded node channels permanently stuck.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@joostjager
joostjagerforce-pushed the fix-chainmonitor-mode-violation branch from abe9d13 to 7e0a4dcCompareFebruary 23, 2026 14:00
@codecov

codecovBot commented Feb 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.86%. Comparing base (2d2151a) to head (7e0a4dc).
⚠️ Report is 2 commits behind head on main.

Files with missing linesPatch %Lines
lightning/src/chain/chainmonitor.rs72.22%5 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #4435 +/- ##
==========================================
- Coverage 85.86% 85.86% -0.01% 
==========================================
Files 159 159 Lines 104302 104308 +6 Branches 104302 104308 +6 ==========================================
+ Hits 89558 89563 +5 + Misses 12246 12242 -4 - Partials 2498 2503 +5 
FlagCoverage Δ
tests85.86% <73.68%> (-0.01%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@joostjager

Copy link
Copy Markdown
ContributorAuthor

Closing in favor of alternative fix: #4436

Sign up for freeto 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.

2 participants

@joostjager@ldk-reviews-bot