Skip to content

fuzz: Add upgrade/downgrade simulation to chanmon_consistency and fix chacha20 build#4499

Draft
Atishyy27 wants to merge 1 commit intolightningdevkit:mainfrom
Atishyy27:feature/fuzzer-upgrade-downgrade
Draft

fuzz: Add upgrade/downgrade simulation to chanmon_consistency and fix chacha20 build#4499
Atishyy27 wants to merge 1 commit intolightningdevkit:mainfrom
Atishyy27:feature/fuzzer-upgrade-downgrade

Conversation

@Atishyy27
Copy link
Contributor

@Atishyy27 Atishyy27 commented Mar 19, 2026

Addresses #4452

This PR introduces real cross-version serialization coverage to the chanmon_consistency fuzzer by depending on a previous version of LDK (lightning_0_2).

Changes:

  • Test Harnesses: Updated FuzzEstimator, TestBroadcaster, and TestChainMonitor to implement both the current traits and the lightning_0_2 versions of those traits.
  • Refactoring: Abstracted common logic (like monitor byte persistence) into private helper methods to avoid code duplication between trait versions.
  • Dependencies: Added lightning_0_2 (v0.2.0) as a dependency in fuzz/Cargo.toml.
  • Fuzzer Logic: Updated the reload_node closure to support a cross-version serialization round-trip path.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Mar 19, 2026

👋 Hi! This PR is now in draft status.
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!

@Atishyy27 Atishyy27 marked this pull request as ready for review March 19, 2026 22:56
@codecov
Copy link

codecov bot commented Mar 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.44%. Comparing base (cb951b4) to head (2358ca7).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4499      +/-   ##
==========================================
+ Coverage   86.18%   87.44%   +1.26%     
==========================================
  Files         160      160              
  Lines      107441   114503    +7062     
  Branches   107441   114503    +7062     
==========================================
+ Hits        92593   100125    +7532     
+ Misses      12229    11755     -474     
- Partials     2619     2623       +4     
Flag Coverage Δ
tests 87.44% <ø> (+1.26%) ⬆️

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.

@TheBlueMatt TheBlueMatt requested review from joostjager and removed request for valentinewallace March 20, 2026 17:44
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Huh? In order to do an upgrade/downgrade we'll need to actually depend on a previous version of LDK.

@TheBlueMatt TheBlueMatt removed the request for review from joostjager March 20, 2026 17:45
@Atishyy27
Copy link
Contributor Author

Ah, I see! I was initially aiming for forward-compatibility coverage by simulating unknown odd TLVs.

If the goal is actual cross-version testing, should I add a previous version (e.g., 0.0.124 or similar) as a renamed dev-dependency in fuzz/Cargo.toml to perform real comparisons? I'd appreciate a pointer if there's an existing target using this pattern I should follow.

@TheBlueMatt
Copy link
Collaborator

See the upgrade tests in tests. Probably just start with 0.2.

@Atishyy27
Copy link
Contributor Author

Got it. I checked upgrade_downgrade_tests.rs and see the pattern of importing lightning_0_2 alongside the current crate.

Before I go too deep into the implementation for the fuzzer: since lightning_0_2's traits (like chain::Watch, FeeEstimator, Router, etc.) are entirely distinct from the current lightning crate's traits, doing a full read/reload cycle with the older version inside chanmon_consistency.rs would require duplicating a lot of the fuzzer's test harness structs (e.g., TestChainMonitor, FuzzEstimator, KeyProvider) to implement the 0_2 traits.

Shall I go ahead and duplicate those test harness implementations for the 0.2 dependency, or is there a lighter-weight serialization round-trip pattern you had in mind for the fuzzer?

@TheBlueMatt
Copy link
Collaborator

I was hoping we'd be able to keep the existing test harnesses but just implement both traits on them - moving common logic to util methods to avoid duplicate code.

@ldk-claude-review-bot
Copy link
Collaborator

@Atishyy27 Atishyy27 force-pushed the feature/fuzzer-upgrade-downgrade branch from 0891df8 to 90980cc Compare March 22, 2026 20:21
@Atishyy27 Atishyy27 marked this pull request as draft March 22, 2026 20:23
@Atishyy27 Atishyy27 force-pushed the feature/fuzzer-upgrade-downgrade branch from 90980cc to 2358ca7 Compare March 22, 2026 20:27
@Atishyy27 Atishyy27 force-pushed the feature/fuzzer-upgrade-downgrade branch 3 times, most recently from 2f5675b to 8c8d44d Compare March 24, 2026 03:22
@Atishyy27 Atishyy27 force-pushed the feature/fuzzer-upgrade-downgrade branch from 8c8d44d to 1ef1d59 Compare March 24, 2026 07:53
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