Make fuzz targets deterministic by joostjager · Pull Request #4525 · lightningdevkit/rust-lightning

joostjager · 2026-03-31T06:27:09Z

Use deterministic time and hash table ordering when building with cfg(fuzzing) to ensure fuzz test cases reproduce consistently.

ldk-reviews-bot · 2026-03-31T06:27:12Z

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

codecov · 2026-03-31T07:27:19Z

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.10%. Comparing base (450c03a) to head (bb4d148).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channelmanager.rs	0.00%	1 Missing ⚠️
lightning/src/util/hash_tables.rs	75.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4525      +/-   ##
==========================================
- Coverage   87.10%   87.10%   -0.01%     
==========================================
  Files         163      163              
  Lines      108740   108748       +8     
  Branches   108740   108748       +8     
==========================================
+ Hits        94723    94729       +6     
- Misses      11531    11533       +2     
  Partials     2486     2486

Flag	Coverage Δ
fuzzing	`40.24% <20.00%> (+0.04%)`	⬆️
tests	`86.19% <75.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.

lightning/src/ln/channelmanager.rs

ldk-claude-review-bot · 2026-03-31T09:18:59Z

Review Summary

Inline comments posted

lightning/src/routing/gossip.rs:2323 — Split #[cfg(not(fuzzing))] is 17 lines away from the existing #[cfg(feature = "std")] on line 2306; should be combined into one #[cfg(all(feature = "std", not(fuzzing)))] for consistency with the rest of the PR.

Prior review issues (not repeated)

The cross-cutting concerns from my prior review still stand:

Incomplete SystemTime::now() coverage — Several SystemTime::now() calls reachable from fuzz targets remain gated on just #[cfg(feature = "std")] without not(fuzzing), particularly in routing/gossip.rs (channel_failed_permanent, node_failed_permanent, add_channel_announcement_internal, update_channel_internal).
channelmanager.rs:~15000 — 3-branch pattern inconsistent with the 2-branch pattern used at other locations.
fuzz/Cargo.toml feature unification — Transitive dependencies re-enable lightning's default features.

joostjager · 2026-03-31T09:39:43Z

Claude comments addressed

TheBlueMatt · 2026-03-31T13:11:35Z

lightning/src/offers/flow.rs

+		#[cfg(any(not(feature = "std"), fuzzing))]
 		let now = Duration::from_secs(self.highest_seen_timestamp.load(Ordering::Acquire) as u64);
-		#[cfg(feature = "std")]
+		#[cfg(all(feature = "std", not(fuzzing)))]


Shouldn't we just fuzz with no-std, then?

We'd redefine no-std as being deterministic, seems like an undesirable shortcut.

I meant just for the time logic - no-std is already defined as not supporting fetching the current time, seems reasonable to use that.

~~It appears that deterministic hashes were already in the code for fuzzing, so the only change needed is then to switch to no std.~~

Claude review pointed out that no-std wasn't actually enabled. And enabling it runs into the issue that test_utils also needs to be made no-std compatible. Then it becomes a larger change.

I am still in favor of the original change that adds the fuzzing flag to the time code.

ldk-claude-review-bot · 2026-04-01T07:25:19Z

fuzz/Cargo.toml


 [dependencies]
-lightning = { path = "../lightning", features = ["regex", "_test_utils"] }
+lightning = { path = "../lightning", default-features = false, features = ["regex", "_test_utils"] }


Bug: This change is a no-op due to Cargo feature unification.

lightning-persister on line 25 is included without default-features = false:

lightning-persister = { path = "../lightning-persister", features = ["tokio"]}

And lightning-persister/Cargo.toml depends on lightning with default features:

lightning = { version = "0.3.0", path = "../lightning" }

Lightning's defaults are ["std", "grind_signatures"]. Through Cargo feature unification, lightning-persister's dependency re-enables both std and grind_signatures on the unified lightning crate, making your default-features = false here have no effect.

Similarly, lightning-liquidity (line 23) uses default features which include std = ["lightning/std"], also re-enabling lightning/std.

To actually disable grind_signatures, you'd need to ensure no transitive dependency pulls in lightning with default features. At minimum, lightning-persister/Cargo.toml itself would need to be updated to use default-features = false for its lightning dependency (and explicitly enable only the features it needs, likely std).

Good point...

Gate all SystemTime::now() and Instant::now() calls in production code with #[cfg(all(feature = "std", not(fuzzing)))] so that fuzz targets produce consistent results regardless of wall-clock time. For each location, the existing no-std fallback (highest_seen_timestamp, None, or a constant) is reused under fuzzing. Also force deterministic hashing when the fuzzing cfg is active, rather than requiring the LDK_TEST_DETERMINISTIC_HASHES env var. AI tools were used in preparing this commit.

ldk-claude-review-bot · 2026-04-01T08:34:30Z

lightning/src/routing/gossip.rs

-	/// This method is only available with the `std` feature. See
+	/// This method is only available with the `std` feature (and not during fuzzing). See
 	/// [`NetworkGraph::remove_stale_channels_and_tracking_with_time`] for non-`std` use.
+	#[cfg(not(fuzzing))]


The new #[cfg(not(fuzzing))] here is split from the pre-existing #[cfg(feature = "std")] on line 2306, 17 lines above (separated by the entire doc comment). In Rust, multiple #[cfg] attributes are ANDed, so the effective gate is all(feature = "std", not(fuzzing)) — which is correct. But every other location in this PR uses a single combined attribute like #[cfg(all(feature = "std", not(fuzzing)))].

Having the two conditions split across 17 lines makes it easy to overlook one when auditing. Consider merging them into one attribute and moving it closer to the function:

Suggested change

#[cfg(not(fuzzing))]

#[cfg(all(feature = "std", not(fuzzing)))]

(and removing the #[cfg(feature = "std")] on line 2306 above the doc comment)

joostjager mentioned this pull request Mar 31, 2026

Fuzz failure backlog #4504

Open

joostjager marked this pull request as ready for review March 31, 2026 09:13

joostjager requested a review from TheBlueMatt March 31, 2026 09:13

ldk-claude-review-bot reviewed Mar 31, 2026

View reviewed changes

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

joostjager force-pushed the fuzz-deterministic branch from bb4d148 to 1a81a3a Compare March 31, 2026 09:39

TheBlueMatt reviewed Mar 31, 2026

View reviewed changes

joostjager force-pushed the fuzz-deterministic branch from 1a81a3a to 9ffd8b3 Compare April 1, 2026 07:20

ldk-claude-review-bot reviewed Apr 1, 2026

View reviewed changes

joostjager force-pushed the fuzz-deterministic branch from 9ffd8b3 to 1a81a3a Compare April 1, 2026 07:52

joostjager force-pushed the fuzz-deterministic branch from 1a81a3a to 9f4e929 Compare April 1, 2026 08:29

ldk-claude-review-bot reviewed Apr 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make fuzz targets deterministic#4525

Make fuzz targets deterministic#4525
joostjager wants to merge 1 commit intolightningdevkit:mainfrom
joostjager:fuzz-deterministic

joostjager commented Mar 31, 2026 •

edited

Loading

Uh oh!

ldk-reviews-bot commented Mar 31, 2026 •

edited

Loading

Uh oh!

codecov bot commented Mar 31, 2026 •

edited

Loading

Uh oh!

Uh oh!

ldk-claude-review-bot commented Mar 31, 2026 •

edited

Loading

Uh oh!

joostjager commented Mar 31, 2026

Uh oh!

TheBlueMatt Mar 31, 2026

Uh oh!

joostjager Mar 31, 2026

Uh oh!

TheBlueMatt Mar 31, 2026

Uh oh!

joostjager Apr 1, 2026 •

edited

Loading

Uh oh!

joostjager Apr 1, 2026

Uh oh!

ldk-claude-review-bot Apr 1, 2026

Uh oh!

joostjager Apr 1, 2026

Uh oh!

ldk-claude-review-bot Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	#[cfg(not(fuzzing))]
	#[cfg(all(feature = "std", not(fuzzing)))]

Conversation

joostjager commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-reviews-bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

ldk-claude-review-bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review Summary

Inline comments posted

Prior review issues (not repeated)

Uh oh!

joostjager commented Mar 31, 2026

Uh oh!

TheBlueMatt Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

joostjager Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

joostjager Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joostjager Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

ldk-claude-review-bot Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

joostjager Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

ldk-claude-review-bot Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

joostjager commented Mar 31, 2026 •

edited

Loading

ldk-reviews-bot commented Mar 31, 2026 •

edited

Loading

codecov bot commented Mar 31, 2026 •

edited

Loading

ldk-claude-review-bot commented Mar 31, 2026 •

edited

Loading

joostjager Apr 1, 2026 •

edited

Loading