fuzz: skip broken splicing paths and handle broadcast channel updates - #4561

Merged
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices
Apr 14, 2026
Merged

fuzz: skip broken splicing paths and handle broadcast channel updates#4561
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices

Conversation

@joostjager

@joostjagerjoostjager commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

This PR tightens chanmon_consistency so it stays focused on failures we can currently act on.

Splicing fuzzing has been broken for quite a while, and leaving splice opcodes active in the default configuration makes it harder to spot unrelated regressions because runs terminate in known-bad splicing paths instead of continuing toward other failures. This change makes those opcodes bail out unless the target is built with cfg(splicing) enabled, while still keeping the opcode slots in place for dedicated splicing fuzzing later.

This PR also corrects a message-passing issue in chanmon_consistency introduced by #4508. After broadcast channel announcements started flowing through the broadcast queue, the fuzz target was no longer handling that path correctly, which meant it could trip over a known routing mismatch instead of exercising the rest of the state machine.

@ldk-reviews-bot

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@joostjagerjoostjager changed the title Fuzz disable splicesfuzz: skip broken splicing paths and handle broadcast channel updatesApr 14, 2026
@joostjagerjoostjager mentioned this pull request Apr 14, 2026
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 523a70a to 5ede3fdCompareApril 14, 2026 06:59
@joostjager
joostjager marked this pull request as ready for review April 14, 2026 08:00
@codecov

codecovBot commented Apr 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.98%. Comparing base (78df66d) to head (9f59b33).
⚠️ Report is 25 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #4561 +/- ##
==========================================
- Coverage 86.99% 86.98% -0.02% 
==========================================
Files 163 163 Lines 108744 108744 Branches 108744 108744 ==========================================
- Hits 94605 94589 -16 - Misses 11658 11672 +14 - Partials 2481 2483 +2 
FlagCoverage Δ
fuzzing38.09% <ø> (-2.20%)⬇️
tests86.09% <ø> (+<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.

@ldk-claude-review-bot

ldk-claude-review-bot commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

The PR is clean. After thorough review:

  • test_return!() is the normal termination path for the fuzz loop (also used in get_slice! when input is exhausted), so calling it from the splice opcode gates doesn't introduce false panics.
  • BroadcastChannelUpdate handling is correctly added in all three places that needed it (push_excess_b_events, both branches of drain_msg_events_on_disconnect), and it was already handled in process_msg_events.
  • The #[allow(unused_variables)] annotations are correct since loggers and fee_estimators are only used inside the now-gated splice arms.
  • All 8 splice opcodes (0xa0-0xa7) are consistently gated.

No issues found.

The regression was introduced in d627ce1. That change
switched fee update opcodes in chanmon_consistency from
maybe_update_chan_fees() to timer_tick_occurred(), which can
enqueue BroadcastChannelUpdate events while peers are
disconnected.
The harness already tolerated those events in one delivery
path, but still treated them as unreachable in
push_excess_b_events and disconnect draining. Accept
BroadcastChannelUpdate in those match arms so the fuzz target
no longer panics on valid timer tick driven message queues.
AI tools were used in preparing this commit.
Keep the splice opcodes in chanmon_consistency available
only when the crate is built with cfg(splicing). When
splicing is disabled, return early from those opcode
handlers instead of calling splice helpers that are not
compiled in.
Add cfg(splicing) to fuzz/Cargo.toml check-cfg so the
guarded code builds cleanly in the fuzz crate.
AI tools were used in preparing this commit.
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 5ede3fd to 9f59b33CompareApril 14, 2026 11:41
@joostjager
joostjager requested a review from jkczyzApril 14, 2026 14:44
@jkczyz
jkczyz merged commit 423c1dc into lightningdevkit:mainApr 14, 2026
24 of 25 checks passed
@joostjagerjoostjager self-assigned this Apr 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants

@joostjager@ldk-reviews-bot@ldk-claude-review-bot@jkczyz
, '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

fuzz: skip broken splicing paths and handle broadcast channel updates - #4561

Merged
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices
Apr 14, 2026
Merged

fuzz: skip broken splicing paths and handle broadcast channel updates#4561
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices

Conversation

@joostjager

@joostjagerjoostjager commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

This PR tightens chanmon_consistency so it stays focused on failures we can currently act on.

Splicing fuzzing has been broken for quite a while, and leaving splice opcodes active in the default configuration makes it harder to spot unrelated regressions because runs terminate in known-bad splicing paths instead of continuing toward other failures. This change makes those opcodes bail out unless the target is built with cfg(splicing) enabled, while still keeping the opcode slots in place for dedicated splicing fuzzing later.

This PR also corrects a message-passing issue in chanmon_consistency introduced by #4508. After broadcast channel announcements started flowing through the broadcast queue, the fuzz target was no longer handling that path correctly, which meant it could trip over a known routing mismatch instead of exercising the rest of the state machine.

@ldk-reviews-bot

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@joostjagerjoostjager changed the title Fuzz disable splicesfuzz: skip broken splicing paths and handle broadcast channel updatesApr 14, 2026
@joostjagerjoostjager mentioned this pull request Apr 14, 2026
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 523a70a to 5ede3fdCompareApril 14, 2026 06:59
@joostjager
joostjager marked this pull request as ready for review April 14, 2026 08:00
@codecov

codecovBot commented Apr 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.98%. Comparing base (78df66d) to head (9f59b33).
⚠️ Report is 25 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #4561 +/- ##
==========================================
- Coverage 86.99% 86.98% -0.02% 
==========================================
Files 163 163 Lines 108744 108744 Branches 108744 108744 ==========================================
- Hits 94605 94589 -16 - Misses 11658 11672 +14 - Partials 2481 2483 +2 
FlagCoverage Δ
fuzzing38.09% <ø> (-2.20%)⬇️
tests86.09% <ø> (+<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.

@ldk-claude-review-bot

ldk-claude-review-bot commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

The PR is clean. After thorough review:

  • test_return!() is the normal termination path for the fuzz loop (also used in get_slice! when input is exhausted), so calling it from the splice opcode gates doesn't introduce false panics.
  • BroadcastChannelUpdate handling is correctly added in all three places that needed it (push_excess_b_events, both branches of drain_msg_events_on_disconnect), and it was already handled in process_msg_events.
  • The #[allow(unused_variables)] annotations are correct since loggers and fee_estimators are only used inside the now-gated splice arms.
  • All 8 splice opcodes (0xa0-0xa7) are consistently gated.

No issues found.

The regression was introduced in d627ce1. That change
switched fee update opcodes in chanmon_consistency from
maybe_update_chan_fees() to timer_tick_occurred(), which can
enqueue BroadcastChannelUpdate events while peers are
disconnected.
The harness already tolerated those events in one delivery
path, but still treated them as unreachable in
push_excess_b_events and disconnect draining. Accept
BroadcastChannelUpdate in those match arms so the fuzz target
no longer panics on valid timer tick driven message queues.
AI tools were used in preparing this commit.
Keep the splice opcodes in chanmon_consistency available
only when the crate is built with cfg(splicing). When
splicing is disabled, return early from those opcode
handlers instead of calling splice helpers that are not
compiled in.
Add cfg(splicing) to fuzz/Cargo.toml check-cfg so the
guarded code builds cleanly in the fuzz crate.
AI tools were used in preparing this commit.
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 5ede3fd to 9f59b33CompareApril 14, 2026 11:41
@joostjager
joostjager requested a review from jkczyzApril 14, 2026 14:44
@jkczyz
jkczyz merged commit 423c1dc into lightningdevkit:mainApr 14, 2026
24 of 25 checks passed
@joostjagerjoostjager self-assigned this Apr 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants

@joostjager@ldk-reviews-bot@ldk-claude-review-bot@jkczyz
, '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

fuzz: skip broken splicing paths and handle broadcast channel updates - #4561

Merged
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices
Apr 14, 2026
Merged

fuzz: skip broken splicing paths and handle broadcast channel updates#4561
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices

Conversation

@joostjager

@joostjagerjoostjager commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

This PR tightens chanmon_consistency so it stays focused on failures we can currently act on.

Splicing fuzzing has been broken for quite a while, and leaving splice opcodes active in the default configuration makes it harder to spot unrelated regressions because runs terminate in known-bad splicing paths instead of continuing toward other failures. This change makes those opcodes bail out unless the target is built with cfg(splicing) enabled, while still keeping the opcode slots in place for dedicated splicing fuzzing later.

This PR also corrects a message-passing issue in chanmon_consistency introduced by #4508. After broadcast channel announcements started flowing through the broadcast queue, the fuzz target was no longer handling that path correctly, which meant it could trip over a known routing mismatch instead of exercising the rest of the state machine.

@ldk-reviews-bot

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@joostjagerjoostjager changed the title Fuzz disable splicesfuzz: skip broken splicing paths and handle broadcast channel updatesApr 14, 2026
@joostjagerjoostjager mentioned this pull request Apr 14, 2026
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 523a70a to 5ede3fdCompareApril 14, 2026 06:59
@joostjager
joostjager marked this pull request as ready for review April 14, 2026 08:00
@codecov

codecovBot commented Apr 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.98%. Comparing base (78df66d) to head (9f59b33).
⚠️ Report is 25 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #4561 +/- ##
==========================================
- Coverage 86.99% 86.98% -0.02% 
==========================================
Files 163 163 Lines 108744 108744 Branches 108744 108744 ==========================================
- Hits 94605 94589 -16 - Misses 11658 11672 +14 - Partials 2481 2483 +2 
FlagCoverage Δ
fuzzing38.09% <ø> (-2.20%)⬇️
tests86.09% <ø> (+<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.

@ldk-claude-review-bot

ldk-claude-review-bot commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

The PR is clean. After thorough review:

  • test_return!() is the normal termination path for the fuzz loop (also used in get_slice! when input is exhausted), so calling it from the splice opcode gates doesn't introduce false panics.
  • BroadcastChannelUpdate handling is correctly added in all three places that needed it (push_excess_b_events, both branches of drain_msg_events_on_disconnect), and it was already handled in process_msg_events.
  • The #[allow(unused_variables)] annotations are correct since loggers and fee_estimators are only used inside the now-gated splice arms.
  • All 8 splice opcodes (0xa0-0xa7) are consistently gated.

No issues found.

The regression was introduced in d627ce1. That change
switched fee update opcodes in chanmon_consistency from
maybe_update_chan_fees() to timer_tick_occurred(), which can
enqueue BroadcastChannelUpdate events while peers are
disconnected.
The harness already tolerated those events in one delivery
path, but still treated them as unreachable in
push_excess_b_events and disconnect draining. Accept
BroadcastChannelUpdate in those match arms so the fuzz target
no longer panics on valid timer tick driven message queues.
AI tools were used in preparing this commit.
Keep the splice opcodes in chanmon_consistency available
only when the crate is built with cfg(splicing). When
splicing is disabled, return early from those opcode
handlers instead of calling splice helpers that are not
compiled in.
Add cfg(splicing) to fuzz/Cargo.toml check-cfg so the
guarded code builds cleanly in the fuzz crate.
AI tools were used in preparing this commit.
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 5ede3fd to 9f59b33CompareApril 14, 2026 11:41
@joostjager
joostjager requested a review from jkczyzApril 14, 2026 14:44
@jkczyz
jkczyz merged commit 423c1dc into lightningdevkit:mainApr 14, 2026
24 of 25 checks passed
@joostjagerjoostjager self-assigned this Apr 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants

@joostjager@ldk-reviews-bot@ldk-claude-review-bot@jkczyz
, '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

fuzz: skip broken splicing paths and handle broadcast channel updates - #4561

Merged
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices
Apr 14, 2026
Merged

fuzz: skip broken splicing paths and handle broadcast channel updates#4561
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices

Conversation

@joostjager

@joostjagerjoostjager commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

This PR tightens chanmon_consistency so it stays focused on failures we can currently act on.

Splicing fuzzing has been broken for quite a while, and leaving splice opcodes active in the default configuration makes it harder to spot unrelated regressions because runs terminate in known-bad splicing paths instead of continuing toward other failures. This change makes those opcodes bail out unless the target is built with cfg(splicing) enabled, while still keeping the opcode slots in place for dedicated splicing fuzzing later.

This PR also corrects a message-passing issue in chanmon_consistency introduced by #4508. After broadcast channel announcements started flowing through the broadcast queue, the fuzz target was no longer handling that path correctly, which meant it could trip over a known routing mismatch instead of exercising the rest of the state machine.

@ldk-reviews-bot

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@joostjagerjoostjager changed the title Fuzz disable splicesfuzz: skip broken splicing paths and handle broadcast channel updatesApr 14, 2026
@joostjagerjoostjager mentioned this pull request Apr 14, 2026
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 523a70a to 5ede3fdCompareApril 14, 2026 06:59
@joostjager
joostjager marked this pull request as ready for review April 14, 2026 08:00
@codecov

codecovBot commented Apr 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.98%. Comparing base (78df66d) to head (9f59b33).
⚠️ Report is 25 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #4561 +/- ##
==========================================
- Coverage 86.99% 86.98% -0.02% 
==========================================
Files 163 163 Lines 108744 108744 Branches 108744 108744 ==========================================
- Hits 94605 94589 -16 - Misses 11658 11672 +14 - Partials 2481 2483 +2 
FlagCoverage Δ
fuzzing38.09% <ø> (-2.20%)⬇️
tests86.09% <ø> (+<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.

@ldk-claude-review-bot

ldk-claude-review-bot commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

The PR is clean. After thorough review:

  • test_return!() is the normal termination path for the fuzz loop (also used in get_slice! when input is exhausted), so calling it from the splice opcode gates doesn't introduce false panics.
  • BroadcastChannelUpdate handling is correctly added in all three places that needed it (push_excess_b_events, both branches of drain_msg_events_on_disconnect), and it was already handled in process_msg_events.
  • The #[allow(unused_variables)] annotations are correct since loggers and fee_estimators are only used inside the now-gated splice arms.
  • All 8 splice opcodes (0xa0-0xa7) are consistently gated.

No issues found.

The regression was introduced in d627ce1. That change
switched fee update opcodes in chanmon_consistency from
maybe_update_chan_fees() to timer_tick_occurred(), which can
enqueue BroadcastChannelUpdate events while peers are
disconnected.
The harness already tolerated those events in one delivery
path, but still treated them as unreachable in
push_excess_b_events and disconnect draining. Accept
BroadcastChannelUpdate in those match arms so the fuzz target
no longer panics on valid timer tick driven message queues.
AI tools were used in preparing this commit.
Keep the splice opcodes in chanmon_consistency available
only when the crate is built with cfg(splicing). When
splicing is disabled, return early from those opcode
handlers instead of calling splice helpers that are not
compiled in.
Add cfg(splicing) to fuzz/Cargo.toml check-cfg so the
guarded code builds cleanly in the fuzz crate.
AI tools were used in preparing this commit.
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 5ede3fd to 9f59b33CompareApril 14, 2026 11:41
@joostjager
joostjager requested a review from jkczyzApril 14, 2026 14:44
@jkczyz
jkczyz merged commit 423c1dc into lightningdevkit:mainApr 14, 2026
24 of 25 checks passed
@joostjagerjoostjager self-assigned this Apr 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants

@joostjager@ldk-reviews-bot@ldk-claude-review-bot@jkczyz
, '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

fuzz: skip broken splicing paths and handle broadcast channel updates - #4561

Merged
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices
Apr 14, 2026
Merged

fuzz: skip broken splicing paths and handle broadcast channel updates#4561
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices

Conversation

@joostjager

@joostjagerjoostjager commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

This PR tightens chanmon_consistency so it stays focused on failures we can currently act on.

Splicing fuzzing has been broken for quite a while, and leaving splice opcodes active in the default configuration makes it harder to spot unrelated regressions because runs terminate in known-bad splicing paths instead of continuing toward other failures. This change makes those opcodes bail out unless the target is built with cfg(splicing) enabled, while still keeping the opcode slots in place for dedicated splicing fuzzing later.

This PR also corrects a message-passing issue in chanmon_consistency introduced by #4508. After broadcast channel announcements started flowing through the broadcast queue, the fuzz target was no longer handling that path correctly, which meant it could trip over a known routing mismatch instead of exercising the rest of the state machine.

@ldk-reviews-bot

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@joostjagerjoostjager changed the title Fuzz disable splicesfuzz: skip broken splicing paths and handle broadcast channel updatesApr 14, 2026
@joostjagerjoostjager mentioned this pull request Apr 14, 2026
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 523a70a to 5ede3fdCompareApril 14, 2026 06:59
@joostjager
joostjager marked this pull request as ready for review April 14, 2026 08:00
@codecov

codecovBot commented Apr 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.98%. Comparing base (78df66d) to head (9f59b33).
⚠️ Report is 25 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #4561 +/- ##
==========================================
- Coverage 86.99% 86.98% -0.02% 
==========================================
Files 163 163 Lines 108744 108744 Branches 108744 108744 ==========================================
- Hits 94605 94589 -16 - Misses 11658 11672 +14 - Partials 2481 2483 +2 
FlagCoverage Δ
fuzzing38.09% <ø> (-2.20%)⬇️
tests86.09% <ø> (+<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.

@ldk-claude-review-bot

ldk-claude-review-bot commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

The PR is clean. After thorough review:

  • test_return!() is the normal termination path for the fuzz loop (also used in get_slice! when input is exhausted), so calling it from the splice opcode gates doesn't introduce false panics.
  • BroadcastChannelUpdate handling is correctly added in all three places that needed it (push_excess_b_events, both branches of drain_msg_events_on_disconnect), and it was already handled in process_msg_events.
  • The #[allow(unused_variables)] annotations are correct since loggers and fee_estimators are only used inside the now-gated splice arms.
  • All 8 splice opcodes (0xa0-0xa7) are consistently gated.

No issues found.

The regression was introduced in d627ce1. That change
switched fee update opcodes in chanmon_consistency from
maybe_update_chan_fees() to timer_tick_occurred(), which can
enqueue BroadcastChannelUpdate events while peers are
disconnected.
The harness already tolerated those events in one delivery
path, but still treated them as unreachable in
push_excess_b_events and disconnect draining. Accept
BroadcastChannelUpdate in those match arms so the fuzz target
no longer panics on valid timer tick driven message queues.
AI tools were used in preparing this commit.
Keep the splice opcodes in chanmon_consistency available
only when the crate is built with cfg(splicing). When
splicing is disabled, return early from those opcode
handlers instead of calling splice helpers that are not
compiled in.
Add cfg(splicing) to fuzz/Cargo.toml check-cfg so the
guarded code builds cleanly in the fuzz crate.
AI tools were used in preparing this commit.
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 5ede3fd to 9f59b33CompareApril 14, 2026 11:41
@joostjager
joostjager requested a review from jkczyzApril 14, 2026 14:44
@jkczyz
jkczyz merged commit 423c1dc into lightningdevkit:mainApr 14, 2026
24 of 25 checks passed
@joostjagerjoostjager self-assigned this Apr 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants

@joostjager@ldk-reviews-bot@ldk-claude-review-bot@jkczyz
, '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

fuzz: skip broken splicing paths and handle broadcast channel updates - #4561

Merged
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices
Apr 14, 2026
Merged

fuzz: skip broken splicing paths and handle broadcast channel updates#4561
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices

Conversation

@joostjager

@joostjagerjoostjager commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

This PR tightens chanmon_consistency so it stays focused on failures we can currently act on.

Splicing fuzzing has been broken for quite a while, and leaving splice opcodes active in the default configuration makes it harder to spot unrelated regressions because runs terminate in known-bad splicing paths instead of continuing toward other failures. This change makes those opcodes bail out unless the target is built with cfg(splicing) enabled, while still keeping the opcode slots in place for dedicated splicing fuzzing later.

This PR also corrects a message-passing issue in chanmon_consistency introduced by #4508. After broadcast channel announcements started flowing through the broadcast queue, the fuzz target was no longer handling that path correctly, which meant it could trip over a known routing mismatch instead of exercising the rest of the state machine.

@ldk-reviews-bot

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@joostjagerjoostjager changed the title Fuzz disable splicesfuzz: skip broken splicing paths and handle broadcast channel updatesApr 14, 2026
@joostjagerjoostjager mentioned this pull request Apr 14, 2026
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 523a70a to 5ede3fdCompareApril 14, 2026 06:59
@joostjager
joostjager marked this pull request as ready for review April 14, 2026 08:00
@codecov

codecovBot commented Apr 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.98%. Comparing base (78df66d) to head (9f59b33).
⚠️ Report is 25 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #4561 +/- ##
==========================================
- Coverage 86.99% 86.98% -0.02% 
==========================================
Files 163 163 Lines 108744 108744 Branches 108744 108744 ==========================================
- Hits 94605 94589 -16 - Misses 11658 11672 +14 - Partials 2481 2483 +2 
FlagCoverage Δ
fuzzing38.09% <ø> (-2.20%)⬇️
tests86.09% <ø> (+<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.

@ldk-claude-review-bot

ldk-claude-review-bot commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

The PR is clean. After thorough review:

  • test_return!() is the normal termination path for the fuzz loop (also used in get_slice! when input is exhausted), so calling it from the splice opcode gates doesn't introduce false panics.
  • BroadcastChannelUpdate handling is correctly added in all three places that needed it (push_excess_b_events, both branches of drain_msg_events_on_disconnect), and it was already handled in process_msg_events.
  • The #[allow(unused_variables)] annotations are correct since loggers and fee_estimators are only used inside the now-gated splice arms.
  • All 8 splice opcodes (0xa0-0xa7) are consistently gated.

No issues found.

The regression was introduced in d627ce1. That change
switched fee update opcodes in chanmon_consistency from
maybe_update_chan_fees() to timer_tick_occurred(), which can
enqueue BroadcastChannelUpdate events while peers are
disconnected.
The harness already tolerated those events in one delivery
path, but still treated them as unreachable in
push_excess_b_events and disconnect draining. Accept
BroadcastChannelUpdate in those match arms so the fuzz target
no longer panics on valid timer tick driven message queues.
AI tools were used in preparing this commit.
Keep the splice opcodes in chanmon_consistency available
only when the crate is built with cfg(splicing). When
splicing is disabled, return early from those opcode
handlers instead of calling splice helpers that are not
compiled in.
Add cfg(splicing) to fuzz/Cargo.toml check-cfg so the
guarded code builds cleanly in the fuzz crate.
AI tools were used in preparing this commit.
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 5ede3fd to 9f59b33CompareApril 14, 2026 11:41
@joostjager
joostjager requested a review from jkczyzApril 14, 2026 14:44
@jkczyz
jkczyz merged commit 423c1dc into lightningdevkit:mainApr 14, 2026
24 of 25 checks passed
@joostjagerjoostjager self-assigned this Apr 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants

@joostjager@ldk-reviews-bot@ldk-claude-review-bot@jkczyz
, '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

fuzz: skip broken splicing paths and handle broadcast channel updates - #4561

Merged
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices
Apr 14, 2026
Merged

fuzz: skip broken splicing paths and handle broadcast channel updates#4561
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices

Conversation

@joostjager

@joostjagerjoostjager commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

This PR tightens chanmon_consistency so it stays focused on failures we can currently act on.

Splicing fuzzing has been broken for quite a while, and leaving splice opcodes active in the default configuration makes it harder to spot unrelated regressions because runs terminate in known-bad splicing paths instead of continuing toward other failures. This change makes those opcodes bail out unless the target is built with cfg(splicing) enabled, while still keeping the opcode slots in place for dedicated splicing fuzzing later.

This PR also corrects a message-passing issue in chanmon_consistency introduced by #4508. After broadcast channel announcements started flowing through the broadcast queue, the fuzz target was no longer handling that path correctly, which meant it could trip over a known routing mismatch instead of exercising the rest of the state machine.

@ldk-reviews-bot

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@joostjagerjoostjager changed the title Fuzz disable splicesfuzz: skip broken splicing paths and handle broadcast channel updatesApr 14, 2026
@joostjagerjoostjager mentioned this pull request Apr 14, 2026
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 523a70a to 5ede3fdCompareApril 14, 2026 06:59
@joostjager
joostjager marked this pull request as ready for review April 14, 2026 08:00
@codecov

codecovBot commented Apr 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.98%. Comparing base (78df66d) to head (9f59b33).
⚠️ Report is 25 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #4561 +/- ##
==========================================
- Coverage 86.99% 86.98% -0.02% 
==========================================
Files 163 163 Lines 108744 108744 Branches 108744 108744 ==========================================
- Hits 94605 94589 -16 - Misses 11658 11672 +14 - Partials 2481 2483 +2 
FlagCoverage Δ
fuzzing38.09% <ø> (-2.20%)⬇️
tests86.09% <ø> (+<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.

@ldk-claude-review-bot

ldk-claude-review-bot commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

The PR is clean. After thorough review:

  • test_return!() is the normal termination path for the fuzz loop (also used in get_slice! when input is exhausted), so calling it from the splice opcode gates doesn't introduce false panics.
  • BroadcastChannelUpdate handling is correctly added in all three places that needed it (push_excess_b_events, both branches of drain_msg_events_on_disconnect), and it was already handled in process_msg_events.
  • The #[allow(unused_variables)] annotations are correct since loggers and fee_estimators are only used inside the now-gated splice arms.
  • All 8 splice opcodes (0xa0-0xa7) are consistently gated.

No issues found.

The regression was introduced in d627ce1. That change
switched fee update opcodes in chanmon_consistency from
maybe_update_chan_fees() to timer_tick_occurred(), which can
enqueue BroadcastChannelUpdate events while peers are
disconnected.
The harness already tolerated those events in one delivery
path, but still treated them as unreachable in
push_excess_b_events and disconnect draining. Accept
BroadcastChannelUpdate in those match arms so the fuzz target
no longer panics on valid timer tick driven message queues.
AI tools were used in preparing this commit.
Keep the splice opcodes in chanmon_consistency available
only when the crate is built with cfg(splicing). When
splicing is disabled, return early from those opcode
handlers instead of calling splice helpers that are not
compiled in.
Add cfg(splicing) to fuzz/Cargo.toml check-cfg so the
guarded code builds cleanly in the fuzz crate.
AI tools were used in preparing this commit.
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 5ede3fd to 9f59b33CompareApril 14, 2026 11:41
@joostjager
joostjager requested a review from jkczyzApril 14, 2026 14:44
@jkczyz
jkczyz merged commit 423c1dc into lightningdevkit:mainApr 14, 2026
24 of 25 checks passed
@joostjagerjoostjager self-assigned this Apr 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants

@joostjager@ldk-reviews-bot@ldk-claude-review-bot@jkczyz
, '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

fuzz: skip broken splicing paths and handle broadcast channel updates - #4561

Merged
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices
Apr 14, 2026
Merged

fuzz: skip broken splicing paths and handle broadcast channel updates#4561
jkczyz merged 2 commits into
lightningdevkit:mainfrom
joostjager:fuzz-disable-splices

Conversation

@joostjager

@joostjagerjoostjager commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

This PR tightens chanmon_consistency so it stays focused on failures we can currently act on.

Splicing fuzzing has been broken for quite a while, and leaving splice opcodes active in the default configuration makes it harder to spot unrelated regressions because runs terminate in known-bad splicing paths instead of continuing toward other failures. This change makes those opcodes bail out unless the target is built with cfg(splicing) enabled, while still keeping the opcode slots in place for dedicated splicing fuzzing later.

This PR also corrects a message-passing issue in chanmon_consistency introduced by #4508. After broadcast channel announcements started flowing through the broadcast queue, the fuzz target was no longer handling that path correctly, which meant it could trip over a known routing mismatch instead of exercising the rest of the state machine.

@ldk-reviews-bot

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@joostjagerjoostjager changed the title Fuzz disable splicesfuzz: skip broken splicing paths and handle broadcast channel updatesApr 14, 2026
@joostjagerjoostjager mentioned this pull request Apr 14, 2026
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 523a70a to 5ede3fdCompareApril 14, 2026 06:59
@joostjager
joostjager marked this pull request as ready for review April 14, 2026 08:00
@codecov

codecovBot commented Apr 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.98%. Comparing base (78df66d) to head (9f59b33).
⚠️ Report is 25 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #4561 +/- ##
==========================================
- Coverage 86.99% 86.98% -0.02% 
==========================================
Files 163 163 Lines 108744 108744 Branches 108744 108744 ==========================================
- Hits 94605 94589 -16 - Misses 11658 11672 +14 - Partials 2481 2483 +2 
FlagCoverage Δ
fuzzing38.09% <ø> (-2.20%)⬇️
tests86.09% <ø> (+<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.

@ldk-claude-review-bot

ldk-claude-review-bot commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

The PR is clean. After thorough review:

  • test_return!() is the normal termination path for the fuzz loop (also used in get_slice! when input is exhausted), so calling it from the splice opcode gates doesn't introduce false panics.
  • BroadcastChannelUpdate handling is correctly added in all three places that needed it (push_excess_b_events, both branches of drain_msg_events_on_disconnect), and it was already handled in process_msg_events.
  • The #[allow(unused_variables)] annotations are correct since loggers and fee_estimators are only used inside the now-gated splice arms.
  • All 8 splice opcodes (0xa0-0xa7) are consistently gated.

No issues found.

The regression was introduced in d627ce1. That change
switched fee update opcodes in chanmon_consistency from
maybe_update_chan_fees() to timer_tick_occurred(), which can
enqueue BroadcastChannelUpdate events while peers are
disconnected.
The harness already tolerated those events in one delivery
path, but still treated them as unreachable in
push_excess_b_events and disconnect draining. Accept
BroadcastChannelUpdate in those match arms so the fuzz target
no longer panics on valid timer tick driven message queues.
AI tools were used in preparing this commit.
Keep the splice opcodes in chanmon_consistency available
only when the crate is built with cfg(splicing). When
splicing is disabled, return early from those opcode
handlers instead of calling splice helpers that are not
compiled in.
Add cfg(splicing) to fuzz/Cargo.toml check-cfg so the
guarded code builds cleanly in the fuzz crate.
AI tools were used in preparing this commit.
@joostjager
joostjagerforce-pushed the fuzz-disable-splices branch from 5ede3fd to 9f59b33CompareApril 14, 2026 11:41
@joostjager
joostjager requested a review from jkczyzApril 14, 2026 14:44
@jkczyz
jkczyz merged commit 423c1dc into lightningdevkit:mainApr 14, 2026
24 of 25 checks passed
@joostjagerjoostjager self-assigned this Apr 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants

@joostjager@ldk-reviews-bot@ldk-claude-review-bot@jkczyz