Skip to content

Restore splice coverage in chanmon_consistency fuzz target#4634

Merged
TheBlueMatt merged 3 commits into
lightningdevkit:mainfrom
wpaulino:restore-splicing-fuzzing
May 22, 2026
Merged

Restore splice coverage in chanmon_consistency fuzz target#4634
TheBlueMatt merged 3 commits into
lightningdevkit:mainfrom
wpaulino:restore-splicing-fuzzing

Conversation

@wpaulino
Copy link
Copy Markdown
Contributor

@wpaulino wpaulino commented May 22, 2026 •
edited
Loading

Fixes #4502
Fixes #4504
Fixes #4581

wpaulino added 3 commits May 22, 2026 14:49
@wpaulino
Manually exit quiescence in fuzzing upon disconnect
90621cf 
In certain cases, we may need to terminate quiescence as a result of
some error via a `ChannelError::WarnAndDisconnect`. We don't need to
necessarily reconnect the peers, so we choose to manually terminate
quiescence via the existing `ChannelManager::exit_quiescence` test
helper.
@wpaulino
Restore splice fuzzing by default
836bc38 
This removes the temporary cfg flag that was added while the splice
fuzzer was broken. We also include coverage for the newly supported
async signing of a splice's shared input.
@wpaulino
Fix fuzz build warnings
f0ce340
@wpaulino wpaulino added this to the 0.3 milestone May 22, 2026
@wpaulino wpaulino requested a review from TheBlueMatt May 22, 2026 21:52
@wpaulino wpaulino self-assigned this May 22, 2026
@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented May 22, 2026 •
edited
Loading

👋 Thanks for assigning @TheBlueMatt 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.

@ldk-claude-review-bot
Copy link
Copy Markdown
Collaborator

ldk-claude-review-bot commented May 22, 2026

After a thorough review of every file and hunk in this diff, I found no bugs, security vulnerabilities, or logic errors.

Summary of review:

No issues found.

The PR makes consistent, well-structured changes:

  • Removes cfg(splicing) gating to always enable splice coverage in the fuzz target
  • Adds SignSpliceSharedInput to supported signer ops with matching fuzz opcodes (0xcf-0xd2) following the existing 4-opcode-per-op pattern
  • Refactors assert_action_timeout_awaiting_response → assert_disconnect_action to detect and handle quiescence-related disconnect warnings, correctly calling exit_quiescence on both sides of the channel
  • Fixes fuzz build warnings by properly gating time-related imports/exports with not(fuzzing), consistent with existing usage sites that already had these guards
  • Minor cleanup: unused Filter import removal, encrypt_message visibility tightening to pub(crate)

TheBlueMatt
TheBlueMatt approved these changes May 22, 2026
View reviewed changes
Comment thread fuzz/src/chanmon_consistency.rs
let (msg, is_quiescent) = assert_disconnect_action(action);
let dest_idx = log_peer_message(node_idx, node_id, nodes, out, "warning");
if is_quiescent {
nodes[node_idx].node.exit_quiescence(node_id, &msg.channel_id).unwrap();
Copy link
Copy Markdown
Collaborator

@TheBlueMatt TheBlueMatt May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we just actually disconnect in this case and drop the manual exit_quiescence method?

Copy link
Copy Markdown
Contributor Author

@wpaulino wpaulino May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually use exit_quiescence in production code now. I chose to use it here rather than disconnecting because we're in the middle of the fuzz settle loop and wanted to avoid taking on more complexity.

@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented May 22, 2026

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

@TheBlueMatt TheBlueMatt merged commit 090e09f into lightningdevkit:main May 22, 2026
20 checks passed
@TheBlueMatt
Copy link
Copy Markdown
Collaborator

TheBlueMatt commented May 22, 2026

Landing, but one comment that might be nice to address.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 22, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.74%. Comparing base (4fac0fe) to head (f0ce340).
⚠️ Report is 24 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #4634      +/-   ##
==========================================
+ Coverage   86.69%   86.74%   +0.05%     
==========================================
  Files         159      159              
  Lines      110604   110713     +109     
  Branches   110604   110713     +109     
==========================================
+ Hits        95888    96041     +153     
+ Misses      12198    12165      -33     
+ Partials     2518     2507      -11
Flag Coverage Δ
fuzzing-fake-hashes 7.01% <0.00%> (-0.02%) ⬇️
fuzzing-real-hashes 29.42% <0.00%> (+6.14%) ⬆️
tests 86.26% <100.00%> (+<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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TheBlueMatt TheBlueMatt TheBlueMatt approved these changes

Assignees

@wpaulino wpaulino

Labels

None yet

Projects

None yet

Milestone

0.3

Development

Successfully merging this pull request may close these issues.

fuzz: remove the splicing cfg gate from chanmon_consistency Fuzz failure backlog fuzz: three panics introduced by splice RBF (chanmon_consistency)

4 participants

@wpaulino @ldk-reviews-bot @ldk-claude-review-bot @TheBlueMatt