fuzz: add corpus reachability checks #36098

HowHsu commented at 11:48 AM on August 27, 2026: contributor

This is a follow-up to #35482.

Add ReachabilityGoal() to aggregate whether selected conditions are reached across all inputs processed by a fuzz target. Goals are identified by source location and reported at shutdown. Setting FUZZ_ENFORCE_REACHABILITY=1 makes the process fail if any registered goal was not reached.

Add goals for selected high-value paths, including IBD and non-IBD message processing, completed P2P handshakes and transport messages, successful estimator/mempool/snapshot operations, transaction and package acceptance, and compact block reconstruction.

Testing

Built the fuzz binary and replayed the current qa-assets corpora for all instrumented targets with FUZZ_ENFORCE_REACHABILITY=1.

DrahtBot added the label Fuzzing on Aug 27, 2026

DrahtBot commented at 11:48 AM on August 27, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36098.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	brunoerg

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

HowHsu force-pushed on Aug 27, 2026

DrahtBot added the label CI failed on Aug 27, 2026

DrahtBot commented at 11:53 AM on August 27, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task lint: https://github.com/bitcoin/bitcoin/actions/runs/33069008111/job/98506152003</sub> <sub>LLM reason (✨ experimental): CI failed because the lint-locale-dependence check flagged a locale-dependent std::fprintf usage in src/test/fuzz/util/reachability.cpp.</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

in src/test/fuzz/util/reachability.cpp:69 in 22271f90aa

  64 | +        goal |= reached;
  65 | +    }
  66 | +
  67 | +private:
  68 | +    std::map<GoalId, bool> m_goals;
  69 | +    std::mutex m_mutex;

maflcko commented at 11:55 AM on August 27, 2026:

nit: I wonder if this can use Mutex + the annotations

HowHsu commented at 2:32 PM on August 28, 2026:

Updated.

in src/test/fuzz/util/reachability.h:11 in 22271f90aa outdated

   6 | +#define BITCOIN_TEST_FUZZ_UTIL_REACHABILITY_H
   7 | +
   8 | +#include <source_location>
   9 | +
  10 | +/**
  11 | + * Record whether a reachability goal is satisfied by at least one fuzz input.

maflcko commented at 11:56 AM on August 27, 2026:

/// Record whether a reachability goal is satisfied by at least one fuzz input.

nit: Would be nice to use the /// for new doxygen comments, to avoid the formatting inconsistencies around /** comments.

HowHsu commented at 2:32 PM on August 28, 2026:

Updated.

maflcko commented at 7:37 AM on August 31, 2026:

Updated.

This was reverted in the last push?

HowHsu commented at 8:04 AM on August 31, 2026:

Updated.

This was reverted in the last push?

Thanks, fixed.

in src/test/fuzz/process_messages.cpp:140 in 22271f90aa outdated

 135 | @@ -132,6 +136,8 @@ FUZZ_TARGET(process_messages, .init = initialize_process_messages)
 136 |              node.peerman->SendMessages(random_node);
 137 |          }
 138 |      }
 139 | +    ReachabilityGoal(saw_ibd_message);
 140 | +    ReachabilityGoal(saw_non_ibd_message);

maflcko commented at 11:59 AM on August 27, 2026:

Seems fine, but this seems a bit low-level. I am thinking that goals may be best higher level?

Eg. mempool.size()>=1, which implies non-ibd message?

HowHsu commented at 1:46 PM on August 27, 2026:

Seems fine, but this seems a bit low-level. I am thinking that goals may be best higher level?

Eg. mempool.size()>=1, which implies non-ibd message?

~Not sure if the current corpus data can pass this.~ Tested it, current corpus data fails on it. We have to first construct a valid Tx entry into corpus set first.

maflcko commented at 1:59 PM on August 27, 2026:

Tested it, current corpus data fails on it. We have to first construct a valid Tx entry into corpus set first.

Huh, I am pretty sure I generated those manually last week and uploaded the fuzz inputs for that?

$ FUZZ=process_messages ./bld-cmake/bin/fuzz -runs=1 ../qa-assets/fuzz_corpora/process_messages/273fac680feaaa99ede7542dbd7af07db80821e2 
INFO: Running with entropic power schedule (0xFF, 100).

test/fuzz/process_messages.cpp:139 void process_messages_fuzz_target(FuzzBufferType): Assertion `initial_sequence==end_sequence' failed.

HowHsu commented at 2:25 PM on August 27, 2026:

Tested it, current corpus data fails on it. We have to first construct a valid Tx entry into corpus set first.

Huh, I am pretty sure I generated those manually last week and uploaded the fuzz inputs for that?
$ FUZZ=process_messages ./bld-cmake/bin/fuzz -runs=1 ../qa-assets/fuzz_corpora/process_messages/273fac680feaaa99ede7542dbd7af07db80821e2 
INFO: Running with entropic power schedule (0xFF, 100).

test/fuzz/process_messages.cpp:139 void process_messages_fuzz_target(FuzzBufferType): Assertion `initial_sequence==end_sequence' failed.

My bad, forgot to update the data set. But the thing is an input of add tx + del tx in process_messages is also valid. so perhaps end_sequence != initial_sequence is better?

maflcko commented at 2:29 PM on August 27, 2026:

Right, the assertion was just to trigger/find the input. The goal should obviously be inverted :)

HowHsu commented at 3:51 AM on August 28, 2026:

Right, the assertion was just to trigger/find the input. The goal should obviously be inverted :)

I meant "compare to use mempool.size() >= 1, end_sequence != initial_sequence may be a better choice?"

maflcko commented at 10:16 AM on August 28, 2026:

I've also uploaded block messages, so marking them as another goal could also make sense?

HowHsu commented at 1:03 PM on August 28, 2026:

Seems fine, but this seems a bit low-level. I am thinking that goals may be best higher level?

Eg. mempool.size()>=1, which implies non-ibd message?

One problem is we still need a goal to represent at least one input goes the IBD path, while mempool.size() == 0 is not accurate for that. If we keep ReachabilityGoal(saw_ibd_message, ...);, then seems keep both saw_ibd_message and saw_non_ibd_message would be more symmetrical

maflcko commented at 1:28 PM on August 28, 2026:

JumpOutOfIbd and ResetIbd already have internal assertions (both ways), so now that those function calls are added back, it is unlikely to regress, unless they are removed?

If you really want, you can add the redundant goals for it, but I don't think we need to turn every assertion into a goal.

HowHsu commented at 2:31 PM on August 28, 2026:

Updated.

maflcko approved

maflcko commented at 12:00 PM on August 27, 2026: member

lgtm. I wonder if reachability goals can be formulated at a higher level. Maybe taking past issues or CVEs into account, to ensure they are covered and remain covered?

in src/test/fuzz/util/reachability.h:19 in eb6d22affb outdated

  14 | + * of the fuzz process. A goal is tracked only after its call site is executed,
  15 | + * so calls should generally be unconditional and near the end of the target.
  16 | + * Set FUZZ_ENFORCE_REACHABILITY=1 to fail at shutdown if a tracked goal was
  17 | + * never satisfied.
  18 | + */
  19 | +void ReachabilityGoal(

dergoegge commented at 12:20 PM on August 27, 2026:

I think it'd be great to add a message to this, so failures are more verbose.

It could also make sense to provide multiple reachability functions for different condition types, where the parameters of the comparison are passed directly e.g. ReachabilityGoalGt(a, b, "sometimes a is greater than b");, this can be used to guide fuzzing in addition to asserting reachability. For example, the assertion feedback in this https://github.com/oss-garage/fuzzamoto/pull/69 PR could make use of the these reachability checks for guidance.

Maybe (although this is likely more controversial) we could also place these checks into production code, to get this feedback for all our tests not just fuzzing.

maflcko commented at 1:17 PM on August 27, 2026:

It could also make sense to provide multiple reachability functions for different condition types, where the parameters of the comparison are passed directly e.g. ReachabilityGoalGt(a, b

I'd presume that -use_value_profile=1 of libFuzzer takes care of this for e.g. mempool.size() >= 200, but I haven't really benchmarked any of this. Is there some gist or background on how to best guide a fuzz engine toward a set goal? I know there is libFuzzers focus_function, but it is experimental and last time I tried it didn't work. Also, I haven't tried other fuzzing engines, so I wonder if others have nice findings here.

Crypt-iQ commented at 1:28 PM on August 27, 2026:

Also, I haven't tried other fuzzing engines, so I wonder if others have nice findings here.

AFL++ does have IJON annotations which I think can just be added to the harness, though it requires specific macros (maybe there is a way to hide them though?). I tested them but since I did not properly benchmark it compared to not using it, I could not tell whether it actually guided AFL++ to the goal more efficiently.

dergoegge commented at 1:32 PM on August 27, 2026:

I was more thinking of custom fuzzers that could make use of this (e.g. that fuzzamoto PR I linked), or AFL++ also has an IJON implementation now, and Antithesis has their Sometimes Assertions, which could both be used under the hood in ReachabilityGoal*. For Antithesis, this only makes sense if we put these goal annotations in non-test code, as the fuzz tests aren't running there.

So overall I think splitting into different functions and explicitly passing the parameters gives us: better error messages, as well as more flexibility for different fuzzers. This can of course also be added later. I think what's in this PR already provides value.

dergoegge commented at 1:34 PM on August 27, 2026:

The same argument could also be made for our Assert and Assume macros (e.g. having Assume{Gt,Lt,...})

maflcko commented at 2:13 PM on August 27, 2026:

For Antithesis, this only makes sense if we put these goal annotations in non-test code, as the fuzz tests aren't running there.

Hmm, I'd presume that reachability goals are test-case dependant. Let's say you want to have a "full mempool". For a test that manually fills the mempool with dummy transactions (possibly just as an initial setup for a later test), this goal is mostly meaningless and would be better off not being checked globally. However, if there is a mempool-sepcific test, it could make sense to have a reachability goal toward a "full mempool" for this specific test.

Moreover, even if the goals are similar for different test-cases, the specific goal could still differ: E.g. the connman test could have a goal of 1000 peers, whereas the p2p test could have a goal of 10 peers (or so)...

HowHsu commented at 2:27 PM on August 27, 2026:

I think it'd be great to add a message to this, so failures are more verbose.

Added.

It could also make sense to provide multiple reachability functions for different condition types, where the parameters of the comparison are passed directly e.g. ReachabilityGoalGt(a, b, "sometimes a is greater than b");, this can be used to guide fuzzing in addition to asserting reachability. For example, the assertion feedback in this oss-garage/fuzzamoto#69 PR could make use of the these reachability checks for guidance.

Beyond my knowledges, I'll take some time to get to know this first.

Maybe (although this is likely more controversial) we could also place these checks into production code, to get this feedback for all our tests not just fuzzing.

It is a good follow-up task, thanks.

HowHsu commented at 12:25 PM on August 27, 2026: contributor

lgtm. I wonder if reachability goals can be formulated at a higher level. Maybe taking past issues or CVEs into account, to ensure they are covered and remain covered?

Sure, I'll look into it.

HowHsu force-pushed on Aug 27, 2026

DrahtBot removed the label CI failed on Aug 27, 2026

brunoerg commented at 9:41 PM on August 27, 2026: contributor

Concept ACK

in src/test/fuzz/partially_downloaded_block.cpp:58 in 1e1144a468 outdated

  57 | -        return;
  58 | -    }
  59 | +    const bool suitable_block{
  60 | +        block && block->vtx.size() > 0 &&
  61 | +        block->vtx.size() < std::numeric_limits<uint16_t>::max()};
  62 | +    ReachabilityGoal(suitable_block, "a suitable block is deserialized");

brunoerg commented at 12:08 AM on August 28, 2026:

1e1144a468fed566dddd732886a0a5b78ad9df99: Is this deserialization a good candidate for it? I think the other ones for this target would implicitly catch it.

HowHsu commented at 2:59 PM on August 28, 2026:

right, but currently the Goal is registered while ReachabilityGoal() is called at the first time. Thus removing this causes the early return on line 59 escaped. This reminds me about the design, maybe a explicit Goal registration is better, otherwise we have to look out all the early returns? @maflcko any thoughts on this?

maflcko commented at 5:28 PM on August 28, 2026:

Well, I'd say this shouldn't be needed, but maybe I am wrong. There could be options:

One hacky, where one globally looks at the coverage with a tool and then makes the tool complain when it sees uncovered ReachabilityGoals. The benefit would be that everything here stays as-is and the tool sits somewhere else?
Make reachability goals global and register them. (Probably with some kind of context to select/check only the active ones?). This will probably increase the state/code/logic a bit, but should be fine.

No strong opinion. I guess anything is fine here?

HowHsu commented at 6:04 AM on August 29, 2026:

Yes, I'm implementing the second way, will push the code soon.

HowHsu commented at 6:31 AM on August 31, 2026:

Updated to "two-phase register/observe protocol".

brunoerg commented at 12:35 AM on August 28, 2026: contributor

Just playing with the idea, thought on coinselection:

diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp
index 8949a6c20d..56703ecf0e 100644
--- a/src/wallet/test/fuzz/coinselection.cpp
+++ b/src/wallet/test/fuzz/coinselection.cpp
@@ -8,6 +8,7 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <test/fuzz/util/reachability.h>
 #include <test/util/setup_common.h>
 #include <wallet/coinselection.h>
 
@@ -395,6 +396,7 @@ void FuzzCoinSelectionAlgorithm(std::span<const uint8_t> buffer) {
                 (void)result_bnb->GetInputSet();
             }
         }
+        ReachabilityGoal(result.has_value(), "coinselection_bnb finds a solution");
     }
 
     if constexpr (Algorithm == CoinSelectionAlgorithm::SRD) {
@@ -409,6 +411,7 @@ void FuzzCoinSelectionAlgorithm(std::span<const uint8_t> buffer) {
             (void)result_srd->GetShuffledInputVector();
             (void)result_srd->GetInputSet();
         }
+        ReachabilityGoal(result.has_value(), "coinselection_srd finds a solution");
     }
 
     if constexpr (Algorithm == CoinSelectionAlgorithm::KNAPSACK) {
@@ -436,6 +439,7 @@ void FuzzCoinSelectionAlgorithm(std::span<const uint8_t> buffer) {
             (void)result_knapsack->GetShuffledInputVector();
             (void)result_knapsack->GetInputSet();
         }
+        ReachabilityGoal(result.has_value(), "coinselection_knapsack finds a solution");
     }
 
     std::vector<COutput> utxos;

HowHsu force-pushed on Aug 28, 2026

HowHsu force-pushed on Aug 31, 2026

HowHsu commented at 6:31 AM on August 31, 2026: contributor

Updated to use a two-phase register/observe protocol: each target explicitly registers its reachability goals during target initialization and records observations while processing inputs. This avoids lazy registration at observation sites, where an early return could leave a goal entirely unknown to the tracker and cause enforcement to pass silently instead of reporting it as unreached.

fuzz: add reachability goal tracking

Add reachability goal descriptors that fuzz targets register during
initialization. Track whether each condition becomes true across all inputs,
and report registered goals when the process exits.

Set FUZZ_ENFORCE_REACHABILITY=1 to exit unsuccessfully if a registered
goal is never satisfied. This allows corpus CI to detect lost path coverage
without a separate build configuration.

Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>

bcf97a3cc4

fuzz: track process_messages reachability 9cc79514d1

fuzz: track process_message reachability 193101ba20

fuzz: track p2p handshake reachability 5e8da56fbe

fuzz: track successful block policy estimator reads 1a1ef162bf

fuzz: track successful mempool loads 56a89c4e1d

fuzz: track successful UTXO snapshot activation 0302981df8

fuzz: track tx_pool_standard acceptance e2ba005dad

fuzz: track tx_pool acceptance 84ec1276cb

fuzz: track valid ephemeral packages bb09525fee

fuzz: track valid transaction packages 96ab84c13e

fuzz: track compact block reachability 454f917d02

fuzz: track complete v1 transport messages b3e4cdcf02

HowHsu force-pushed on Aug 31, 2026

in src/test/fuzz/validation_load_mempool.cpp:67 in b3e4cdcf02

  66 | +                    {
  67 | +                        .mockable_fopen_function = fuzzed_fopen,
  68 | +                    })};
  69 |      pool.SetLoadTried(true);
  70 |      (void)DumpMempool(pool, MempoolPath(g_setup->m_args), fuzzed_fopen, true);
  71 | +    ObserveReachabilityGoal(load_success, MEMPOOL_LOAD_SUCCEEDS);

maflcko commented at 11:02 AM on August 31, 2026:

Instead of a plain "success" (which probably happens on an empty file), it may be better to ensure all features are covered:

txs
unbroadcast set
fee deltas
... etc?