Skip to content

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

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

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

Conversation

@joostjager
Copy link
Copy Markdown
Contributor

@joostjager joostjager commented Apr 14, 2026

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
Copy link
Copy Markdown

ldk-reviews-bot commented Apr 14, 2026

👋 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.

@joostjager joostjager changed the title Fuzz disable splices fuzz: skip broken splicing paths and handle broadcast channel updates Apr 14, 2026
@joostjager joostjager force-pushed the fuzz-disable-splices branch from 523a70a to 5ede3fd Compare April 14, 2026 06:59
@joostjager joostjager marked this pull request as ready for review April 14, 2026 08:00
@joostjager joostjager requested a review from TheBlueMatt April 14, 2026 08:00
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 14, 2026

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     
Flag Coverage Δ
fuzzing 38.09% <ø> (-2.20%) ⬇️
tests 86.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
Copy link
Copy Markdown
Collaborator

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

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 joostjager force-pushed the fuzz-disable-splices branch from 5ede3fd to 9f59b33 Compare April 14, 2026 11:41
@joostjager joostjager requested a review from jkczyz April 14, 2026 14:44
@jkczyz jkczyz merged commit 423c1dc into lightningdevkit:main Apr 14, 2026
24 of 25 checks passed
Sign up for free to 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.

4 participants