fuzz: add fuzz target for P2PGossipSync gossip message handling

[NishantBansal2003]

Added gossip state machine fuzz tests for gossip discovery state handling. In these fuzz tests, we read bytes from the fuzz input to determine actions such as connecting peers, feeding channel announcements, node announcements, channel updates, or query messages. Both valid and malformed messages are generated to exercise error paths.

The reason for adding these fuzz tests is that P2PGossipSync is the ultimate sink for gossip messages received from peers, so there must not be any crashes or unintended behavior in LDK when handling messages from potentially malicious peers.

I only included the states that LDK currently handles. For example, I did not add reply_channel_range or query_scids messages since LDK does not process them at the moment. Similarly, since P2PGossipSync does not handle gossip_timestamp_filter, it is not included in these fuzz tests. Additionally, there is limited benefit in fuzzing gossip_timestamp_filter in LDK, as it is typically processed only once per peer and involves minimal logical processing.

[ldk-reviews-bot]

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

I've completed a thorough review of the entire diff, checking every file and hunk. I also verified the underlying implementations that the assertions depend on (handle_query_channel_range, update_channel_internal, get_directional_info, and the LengthReadable impls for all four message types).

All three issues from the prior review have been addressed:

• Licensing header added (lines 1-8)
• get_pubkey!() now uses continue instead of return (line 90)
• Assertions use message contents (node_ann.contents.node_id, chan_ann.contents.node_id_1, etc.) instead of extracted peer variables

No new issues found. The fuzz target is correct:

• assert!(reader.is_empty()) in decode_msg! is safe — all four message types (NodeAnnouncement, ChannelAnnouncement, ChannelUpdate, QueryChannelRange) consume all remaining bytes via read_to_end or decode_tlv_stream!
• Channel update assertions are safe — handle_channel_update returning Ok guarantees the directional info was stored
• Query channel range event assertions are correct — the handler always enqueues at least one SendReplyChannelRange event with the last having sync_complete: true
• NetworkUpdate assertions are valid for is_permanent: true
• FuzzUtxoLookup correctly provides synchronous UTXO results
• Generated target file matches the standard template
• All additions are in correct alphabetical order (lib.rs, gen_target.sh, targets.h)

No issues found.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.11%. Comparing base (450c03a) to head (d29150d).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4532   +/-   ##
=======================================
  Coverage   87.10%   87.11%           
=======================================
  Files         163      163           
  Lines      108740   108740           
  Branches   108740   108740           
=======================================
+ Hits        94723    94730    +7     
+ Misses      11531    11526    -5     
+ Partials     2486     2484    -2     

Flag	Coverage Δ
fuzzing	40.35% <ø> (+0.15%)	⬆️
tests	86.20% <ø> (-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.

[TheBlueMatt]

Cool, thanks! Have you run any comparisons on the coverage of this vs the router fuzzer? I'm wondering if it makes sense to add a new fuzzer here or focus more CPU on the router fuzzer given it tests very similar codepaths.

[NishantBansal2003]

Cool, thanks! Have you run any comparisons on the coverage of this vs the router fuzzer? I'm wondering if it makes sense to add a new fuzzer here or focus more CPU on the router fuzzer given it tests very similar codepaths.

Coverage wise, the additional benefit is that P2PGossipSync also gets covered, so that's a plus.

My main goal was to cover all announcement messages in the fuzz tests (which the router fuzzer already does), and additionally support pruning cases like fail channel (already covered), fail node, and stale channel, along with handling gossip queries. However, since LDK only supports query channel range, we effectively get only one additional path to fuzz.

One option is to extend the router.rs fuzzer by adding new states (query channel range, fail node, stale channel) and checking invariants at the end of each state also. But for this, we would also need to include P2P gossip sync there.

OR, we can keep a separate fuzz target (gossip_discovery.rs) dedicated purely to gossip related cases (which was my original intention). The downside is some duplication in CPU usage with router.rs.

WDYT? I'm fine with either approach since both will ultimately achieve full coverage of gossip message processing.

[ldk-reviews-bot]

🔔 1st Reminder

Hey @joostjager! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[ldk-reviews-bot]

🔔 2nd Reminder

Hey @joostjager! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[TheBlueMatt]

Hmm, yea, I suppose the goal is sufficiently different that it makes sense to just ship a separate fuzzer. What could go wrong, anyway? :)

[TheBlueMatt]

I'm not entirely sure its worth all the effort for creative message building - should we instead just read a length and then deserialize N bytes as each message and process it?

[NishantBansal2003]

I followed your suggestion and used a decode method to fill the inputs (mostly taken from router.rs). The reason for doing this is that the code is now somewhat reduced. But, the downside is that some fuzz input gets wasted due to being overwritten. I think it’s fine?

The reason I initially did manual field parsing is i want the fuzz tests to cover P2P gossip sync e2e, including sign verify and raw fuzz input won't penetrate sign verification, so we need to define some valid fields manually

[TheBlueMatt]

including sign verify and raw fuzz input won't penetrate sign verification

We deliberately use broken signatures and hash function in fuzzing so the fuzzer should be able to do this, even if its a bit of work (usually just all-0s and a single byte).

[NishantBansal2003]

Great, I saw the code in rust-secp256k1 and noticed that with the secp256k1_fuzz flag, there is a fuzzed implementation of secp256k1_ecdsa_verify (https://github.com/rust-bitcoin/rust-secp256k1/blob/master/secp256k1-sys/src/lib.rs#L1828-L1845). So, I think it's fine to remove the manual message building. Working on it now

[TheBlueMatt]

What's the point of maybe_set vs just letting the fuzzer pick the values it wants?

[NishantBansal2003]

Yes, I was running the fuzzer, so I hadn't committed the changes. But now committed
So, maybe_set has been removed in the latest commit, and the fuzzer now directly picks values.

[ldk-reviews-bot]

🔔 3rd Reminder

Hey @TheBlueMatt @joostjager! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.