Add fuzzers for UTXO validation and gossip verification#4438
Add fuzzers for UTXO validation and gossip verification#4438tnull wants to merge 2 commits intolightningdevkit:mainfrom
Conversation
Add a dedicated fuzz target exercising the UTXO verification logic in `routing::utxo`, covering the `PendingChecks` state machine through the `NetworkGraph` and `P2PGossipSync` public APIs. The `FuzzChainSource` returns fuzz-driven `UtxoResult` variants including sync success/failure, async with immediate resolution, and async with deferred resolution. The fuzz loop drives channel announcements (with and without UTXO lookup), node announcements, channel updates, deferred future resolution (success and failure), completed-check processing, and future drops without resolution. Co-Authored-By: HAL 9000
Add a dedicated fuzz target exercising the `GossipVerifier` from `lightning-block-sync`, which implements `UtxoLookup` by fetching blocks and checking UTXOs via the `BlockSource`/`UtxoSource` traits. The `FuzzBlockSource` drives all async methods from fuzz input, covering `BlockSourceError` variants (persistent and transient), `HeaderOnly` vs `FullBlock` responses, variable block shapes (transaction and output counts), the 5-confirmation depth check, and the `is_output_unspent` spent/unspent paths. The fuzz loop feeds channel announcements through the `GossipVerifier`, interleaved with node announcements, channel updates, and `check_resolved_futures` processing via `P2PGossipSync`. Co-Authored-By: HAL 9000
|
👋 Hi! Please choose at least one reviewer by assigning them on the right bar. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4438 +/- ##
==========================================
- Coverage 85.88% 85.87% -0.01%
==========================================
Files 159 159
Lines 104302 104302
Branches 104302 104302
==========================================
- Hits 89578 89568 -10
- Misses 12222 12232 +10
Partials 2502 2502
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
I'm not quite sure that each of these adds independent test coverage? Should we skip utxo_validation and just do gossip_verifier?
| use std::sync::{Arc, Mutex}; | ||
|
|
||
| #[inline] | ||
| pub fn slice_to_be16(v: &[u8]) -> u16 { |
There was a problem hiding this comment.
heh, these are in std now, we should use std.
|
|
||
| macro_rules! decode_msg_with_len16 { | ||
| ($MsgType: path, $excess: expr) => {{ | ||
| let extra_len = slice_to_be16(get_slice_nonadvancing!(2)); |
There was a problem hiding this comment.
I assume this was copied from somewhere that's buggy but this is weird? We read 2 bytes of length, but don't advance the buffer, then in decode_msg read those two bytes again as a part of the message.
| let idx_byte = get_slice!(1)[0] as usize; | ||
| let idx = idx_byte % futures.len(); | ||
| let future = futures.remove(idx); | ||
| let script_byte = get_slice!(1)[0]; |
There was a problem hiding this comment.
Can we just use the byte that was already read when we called get_utxo so we skip the extra read? Same below, we can decide the ultimate future result when we read in get_utxo rather than here.
| // Channel announcement with UTXO lookup | ||
| 0 => { | ||
| let msg = decode_msg_with_len16!( | ||
| msgs::UnsignedChannelAnnouncement, |
There was a problem hiding this comment.
I do wonder if there's any actual value in decoding whole messages - if we instead just read a few fixed bytes and use it to decide between a set of valid messages, can we get the same coverage?
| // Process completed checks (triggers check_resolved_futures) | ||
| 3 => { | ||
| // Yield first so any in-flight tokio tasks can resolve their futures. | ||
| tokio::task::yield_now().await; |
There was a problem hiding this comment.
This should only be deterministic if we are using a single-threaded runtime, no? (and even then not really...)
3 participants
I hit a
debug_assertin the gossip verification flow (see #4439) that I'm still trying to reproduce. As a side product I added fuzz targets for UTXO validation and gossip verification.