fuzz: add ipc round-trip fuzz target #35118

enirox001 commented at 8:30 AM on April 20, 2026: contributor

As discussed in #23015, this PR adds an IPC fuzz target to exercise the Cap'n Proto/libmultiprocess serialization bridge using an in-process two-way pipe and a reflected test interface.

It covers round-trip serialization for COutPoint, CScript, std::vector<uint8_t>, UniValue, and transactions, and exercises libmultiprocess proxy/server interaction. The target guarantees at least one IPC operation per input and is included by default when IPC is enabled in fuzz builds

DrahtBot added the label Fuzzing on Apr 20, 2026

DrahtBot commented at 8:31 AM on April 20, 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/35118.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Approach ACK	dergoegge
Stale ACK	ryanofsky

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

Conflicts

No conflicts as of last run.

enirox001 force-pushed on Apr 20, 2026

DrahtBot added the label CI failed on Apr 20, 2026

enirox001 force-pushed on Apr 20, 2026

DrahtBot removed the label CI failed on Apr 20, 2026

ViniciusCestarii commented at 6:37 PM on April 20, 2026: contributor

While testing this PR with cmake --preset=libfuzzer, I noticed the new ipc target never makes it into the fuzz binary (FUZZ=ipc build_fuzz/bin/fuzz reports No fuzz target compiled for ipc).

The cause is in the top-level CMakeLists.txt:234: when BUILD_FOR_FUZZING=ON, ENABLE_IPC is unconditionally forced OFF, which makes the if(ENABLE_IPC) guard in src/test/fuzz/CMakeLists.txt skip ipc.cpp. So the target added by this PR can't actually be exercised through the standard fuzz preset.

I believe the fix is to drop set(ENABLE_IPC OFF) from CMakeLists.txt:234.

ViniciusCestarii commented at 7:42 PM on April 20, 2026: contributor

Three of the four methods (passOutPoint, passVectorUint8, passScript) are identity round-trips. add(a, b) is stronger because the server has to actually decode a and b to compute the sum and the client then checks against the original inputs.

With identity, the server never has to understand the value it received, so a broken deserialization can be re-serialized as the same garbage and still compare equal. A small deterministic transform on the server side would close that gap, e.g.:

  std::vector<uint8_t> passVectorUint8(std::vector<uint8_t> v) {
      std::reverse(v.begin(), v.end());
      return v;
  }
  CScript passScript(CScript s) { s << OP_NOP; return s; }
  COutPoint passOutPoint(COutPoint o) { return COutPoint{o.hash, o.n ^ 0xFFFFFFFFu}; }

and then comparing against the reproduced transform on the client just like add(a, b). This matches the "transform, reverse, or shift" idea from #23015.

enirox001 force-pushed on Apr 21, 2026

DrahtBot added the label CI failed on Apr 21, 2026

DrahtBot commented at 3:58 AM on April 21, 2026: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/24702970550/job/72250002067 LLM reason (✨ experimental): CI failed due to a lint error for trailing whitespace detected by the trailing_whitespace check.

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

enirox001 force-pushed on Apr 21, 2026

enirox001 commented at 4:43 AM on April 21, 2026: contributor

Thanks for the review @ViniciusCestarii

While testing this PR with cmake --preset=libfuzzer, I noticed the new ipc target never makes it into the fuzz binary (FUZZ=ipc build_fuzz/bin/fuzz reports No fuzz target compiled for ipc). I believe the fix is to drop set(ENABLE_IPC OFF) from CMakeLists.txt:234.

The fix is indeed to remove set(ENABLE_IPC OFF) from the BUILD_FOR_FUZZING block, that line was added in af4156ab to work around missing capnp in fuzzing builds, but since add_libmultiprocess and add_subdirectory(ipc) are both gated behind ENABLE_IPC in src/CMakeLists.txt, users without capnp will still get ENABLE_IPC=OFF naturally with no configure error.

With identity, the server never has to understand the value it received, so a broken deserialization can be re-serialized as the same garbage and still compare equal. A small deterministic transform on the server side would close that gap, e.g.:
 std::vector<uint8_t> passVectorUint8(std::vector<uint8_t> v) {
 std::reverse(v.begin(), v.end());
 return v;
 }
 CScript passScript(CScript s) { s << OP_NOP; return s; }
 COutPoint passOutPoint(COutPoint o) { return COutPoint{o.hash, o.n ^ 0xFFFFFFFFu}; }
and then comparing against the reproduced transform on the client just like add(a, b). This matches the "transform, reverse, or shift" idea from #23015.

Good point. Updated the three identity methods to use deterministic transforms instead:

passOutPoint: XORs n with 0xFFFFFFFF
passVectorUint8: reverses the vector
passScript: appends OP_NOP

The client-side asserts now reproduce the same transform and compare against that

enirox001 force-pushed on Apr 21, 2026

DrahtBot removed the label CI failed on Apr 21, 2026

enirox001 renamed this:
~~fuzz: Add IPC round-trip fuzz target~~
fuzz: add ipc round-trip fuzz target
on Apr 23, 2026

enirox001 force-pushed on Apr 23, 2026

enirox001 commented at 9:22 AM on April 23, 2026: contributor

Updated the branch to keep the IPC fuzz target opt-in instead of enabling IPC for all fuzz builds.

The issue was that BUILD_FOR_FUZZING=ON intentionally disables IPC, so the new ipc target was not compiled even when using the standard libFuzzer preset. Simply dropping set(ENABLE_IPC OFF) would make cmake --preset=libfuzzer require IPC/libmultiprocess dependencies by default, which seems unnecessarily disruptive for existing fuzz builds.

The current approach preserves the existing default behavior:

cmake --preset=libfuzzer keeps IPC disabled.
cmake --preset=libfuzzer -DENABLE_IPC=ON builds the IPC fuzz target.

I also removed the unrelated fuzz target linking cleanup from this PR. The remaining diff is focused on making IPC fuzzing explicitly buildable and adding the initial ipc round-trip target.

enirox001 force-pushed on Apr 23, 2026

DrahtBot added the label CI failed on Apr 23, 2026

DrahtBot commented at 9:33 AM on April 23, 2026: contributor

🚧 At least one of the CI tasks failed. Task 32 bit ARM: https://github.com/bitcoin/bitcoin/actions/runs/24827161441/job/72666022727 LLM reason (✨ experimental): CI failed because the fuzz target build stopped with a missing include file (mp/proxy.capnp.h) causing a fatal compilation error.

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

enirox001 force-pushed on Apr 24, 2026

DrahtBot removed the label CI failed on Apr 24, 2026

sedited requested review from marcofleon on Apr 24, 2026

ViniciusCestarii commented at 12:39 PM on April 29, 2026: contributor

cmake --preset=libfuzzer keeps IPC disabled.

cmake --preset=libfuzzer -DENABLE_IPC=ON builds the IPC fuzz target.

I don't believe this is the correct approach. The preset libfuzzer is a preset to build everything necessary to run all fuzz targets and not partially. This way it can silently skips coverage, which is confusing and sets a precedent where every future subsystem fuzzer becomes another flag to remember.

build: allow ipc fuzz builds

Do not force ENABLE_IPC=OFF when BUILD_FOR_FUZZING is enabled.

The libfuzzer preset is expected to build the full fuzz surface, and the
ipc target should not require an extra opt-in flag to be compiled.

IPC already depends on the normal top-level ENABLE_IPC option and will fail
naturally when its Capn Proto dependencies are unavailable. Keeping the
fuzz-only override would silently skip IPC coverage in standard fuzz builds.

8a739a5510

enirox001 force-pushed on Apr 30, 2026

enirox001 commented at 8:03 AM on April 30, 2026: contributor

I don't believe this is the correct approach. The preset libfuzzer is a preset to build everything necessary to run all fuzz targets and not partially. This way it can silently skips coverage, which is confusing and sets a precedent where every future subsystem fuzzer becomes another flag to remember.

Looking at this again, you are correct.

My earlier rationale was to keep cmake --preset=libfuzzer from pulling in IPC by default, and require -DENABLE_IPC=ON to opt into this target.

But that seems to be the wrong tradeoff here, because it makes the libfuzzer preset silently build only a partial fuzz surface. IPC is already governed by the normal top-level ENABLE_IPC option, so treating fuzz builds specially in this case does not seem justified.

I updated the branch to remove the fuzz-only ENABLE_IPC=OFF override. With this change, cmake --preset=libfuzzer follows the normal ENABLE_IPC behavior and includes the IPC fuzz target by default when IPC is enabled for the build.

Also updated the PR description to match changes made

enirox001 force-pushed on Apr 30, 2026

DrahtBot added the label CI failed on Apr 30, 2026

DrahtBot commented at 8:36 AM on April 30, 2026: contributor

🚧 At least one of the CI tasks failed. Task fuzzer,address,undefined,integer: https://github.com/bitcoin/bitcoin/actions/runs/25154022524/job/73731223345 LLM reason (✨ experimental): Fuzz testing crashed under AddressSanitizer (memory corruption) causing the CI job to abort with exit code 1.

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

enirox001 force-pushed on Apr 30, 2026

enirox001 commented at 9:13 AM on April 30, 2026: contributor

Updated the harness after sanitizer failures in the direct-call version.

IPC client calls are now routed through a dedicated worker thread so libmultiprocess thread-local client state is not initialized on the main fuzzing thread. This avoids teardown-time sanitizer failures without changing the target's intended IPC coverage.

DrahtBot removed the label CI failed on Apr 30, 2026

enirox001 force-pushed on May 4, 2026

DrahtBot added the label CI failed on May 4, 2026

DrahtBot commented at 5:39 PM on May 4, 2026: contributor

🚧 At least one of the CI tasks failed. Task i686, no IPC: https://github.com/bitcoin/bitcoin/actions/runs/25330582475/job/74262991276 LLM reason (✨ experimental): CI failed because the sock_tests CTest module hit fatal socket errors (critical check s != (SOCKET)-1 has failed), causing 6 test failures.

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

enirox001 force-pushed on May 4, 2026

enirox001 force-pushed on May 5, 2026

DrahtBot removed the label CI failed on May 5, 2026

enirox001 force-pushed on May 5, 2026

ryanofsky commented at 5:17 PM on May 14, 2026: contributor

Concept ACK 9ac410a662914684aca3e8a28313863270a86279. This seems like a good first step that tests the IPC framework end-to-end and could uncover some issues. I don't have a lot of confidence that this will be the best or most useful approach (mostly due to my inexperience with fuzzing), but this definitely seems like something we can build on.

I do have some notes about the implementation:

The leaking of IpcFuzzSetup and use of CallOnClientThreadAndWait seem very hacky and should be avoidable. If these are only needed because the fuzzing framework does not provide a cleanup hook for safe shutdown, it would be pretty easy to add a cleanup hook (for example grep for cleanup_threadpool_test in 6e7439bf9c5d0db4332ad71949ffad5cf0c91329 from #30342 for a small change to the framework that allows cleaning up after a test).
Another issue is the empty catch for std::exception in the test. I'm not sure what exceptions are expected to be thrown during the test but it would be good to narrow the catch so all unexpected errors are detected.
Another observation is that value.read(fuzzed_data_provider.ConsumeRandomLengthString(512)) call seems likely to just pass null UniValue values repeatedly because not many valid json strings will be generated. It would probably make sense to define a ConsumeUnivalue utility function (similar to ConsumeScript) to avoid this problem and generate more valid json strings.
Right now the test is using always libmultiprocess to call IpcFuzzInterface methods on the client side, and always using libmultiprocess to handle calls on the server-side. This limits what the test can cover because the client will always send valid bitcoin-serialized Data values and valid JSON-serialized Text values, and the server will similarly always return valid values. It could make sense to extend the test with a non-libmultiprocess client that can send invalid Data/Text values to a libmultiprocess server, and with a non-libmultiprocess server that can return invalid Data/Text values to a libmultiprocess client, to make sure only expected errors occur. This would add extra complexity though so might make more sense for a followup.

enirox001 force-pushed on May 15, 2026

DrahtBot added the label CI failed on May 15, 2026

DrahtBot commented at 9:24 AM on May 15, 2026: contributor

🚧 At least one of the CI tasks failed. Task fuzzer,address,undefined,integer: https://github.com/bitcoin/bitcoin/actions/runs/25910067631/job/76152694979 LLM reason (✨ experimental): Fuzz test failed because libFuzzer hit a deadly crash (std::out_of_range from vector::_M_range_check) in ipc_fuzz_target/JSONUTF8StringFilter::push_back_u during the fuzz run.

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

enirox001 force-pushed on May 15, 2026

enirox001 commented at 2:33 PM on May 15, 2026: contributor

Thanks for the review @ryanofsky

The leaking of IpcFuzzSetup and use of CallOnClientThreadAndWait seem very hacky and should be avoidable. If these are only needed because the fuzzing framework does not provide a cleanup hook for safe shutdown, it would be pretty easy to add a cleanup hook (for example grep for cleanup_threadpool_test in https://github.com/bitcoin/bitcoin/commit/6e7439bf9c5d0db4332ad71949ffad5cf0c91329 from #30342 for a small change to the framework that allows cleaning up after a test).

Updated the target to avoid the leaked setup and client-thread workaround. IpcFuzzSetup is now scoped to each fuzz input, so it is torn down normally before the target returns.

Another issue is the empty catch for std::exception in the test. I'm not sure what exceptions are expected to be thrown during the test but it would be good to narrow the catch so all unexpected errors are detected.

Another observation is that value.read(fuzzed_data_provider.ConsumeRandomLengthString(512)) call seems likely to just pass null UniValue values repeatedly because not many valid json strings will be generated. It would probably make sense to define a ConsumeUnivalue utility function (similar to ConsumeScript) to avoid this problem and generate more valid json strings.

Also removed the broad/empty exception catch, and changed the UniValue case to construct a valid object with fuzzed fields instead of parsing mostly-invalid random JSON.

Right now the test is using always libmultiprocess to call IpcFuzzInterface methods on the client side, and always using libmultiprocess to handle calls on the server-side. This limits what the test can cover because the client will always send valid bitcoin-serialized Data values and valid JSON-serialized Text values, and the server will similarly always return valid values. It could make sense to extend the test with a non-libmultiprocess client that can send invalid Data/Text values to a libmultiprocess server, and with a non-libmultiprocess server that can return invalid Data/Text values to a libmultiprocess client, to make sure only expected errors occur. This would add extra complexity though so might make more sense for a followup.

I left the non-libmultiprocess invalid Data/Text client/server cases for a follow-up, since that seems like a separate increase in complexity.

DrahtBot removed the label CI failed on May 15, 2026

ryanofsky commented at 7:26 PM on May 18, 2026: contributor

Code review ACK 5691f8ccfb142f0d3f7432375dea87785dd5d2dd. This test now seems much simpler without CallOnClientThreadAndWait and leaked objects from the earlier version (9ac410a662914684aca3e8a28313863270a86279) .

But in the new version it seems inefficient to initialize and tear down the whole IPC framework with each fuzz input. So I'm still wondering if the cleanup hook approach I suggested #35118 (comment) could avoid this overhead while still providing a deterministic destruction order and allowing a clean shutdown. Claude implemented it as:

--- a/src/test/fuzz/fuzz.cpp
+++ b/src/test/fuzz/fuzz.cpp
@@ -80,6 +80,25 @@ void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target,
 static std::string_view g_fuzz_target;
 static const TypeTestOneInput* g_test_one_input{nullptr};
 
+class FuzzTestSetup
+{
+public:
+    FuzzTestSetup(const FuzzTargetOptions& options) : m_options{options}
+    {
+        if (m_options.init) m_options.init();
+    }
+    ~FuzzTestSetup()
+    {
+        if (m_options.cleanup) m_options.cleanup();
+    }
+    const FuzzTargetOptions& m_options;
+};
+
+static void test_setup(const FuzzTargetOptions& options)
+{
+    static FuzzTestSetup setup{options};
+}
+
 static void test_one_input(FuzzBufferType buffer)
 {
     CheckGlobals check{};
@@ -162,7 +181,7 @@ static void initialize()
     }
     Assert(!g_test_one_input);
     g_test_one_input = &it->second.test_one_input;
-    it->second.opts.init();
+    test_setup(it->second.opts);
 
     ResetCoverageCounters();
 }
--- a/src/test/fuzz/fuzz.h
+++ b/src/test/fuzz/fuzz.h
@@ -26,7 +26,8 @@ using FuzzBufferType = std::span<const uint8_t>;
 
 using TypeTestOneInput = std::function<void(FuzzBufferType)>;
 struct FuzzTargetOptions {
-    std::function<void()> init{[] {}};
+    std::function<void()> init{};
+    std::function<void()> cleanup{};
     bool hidden{false};
 };
 
--- a/src/test/fuzz/ipc.cpp
+++ b/src/test/fuzz/ipc.cpp
@@ -17,6 +17,7 @@
 #include <mp/proxy-io.h>
 #include <future>
 #include <memory>
+#include <optional>
 #include <stdexcept>
 #include <thread>
 
@@ -99,15 +100,28 @@ private:
     std::thread m_loop_thread;
 };
 
+std::optional<IpcFuzzSetup>* g_ipc;
+
 void initialize_ipc()
 {
     static const auto testing_setup = MakeNoLogFileContext<>();
     (void)testing_setup;
+    static std::optional<IpcFuzzSetup> ipc;
+    ipc.emplace();
+    g_ipc = &ipc;
 }
 
-FUZZ_TARGET(ipc, .init = initialize_ipc)
+void cleanup_ipc()
 {
-    IpcFuzzSetup ipc;
+    // Disconnect and join the event loop thread before fuzz framework objects
+    // are destroyed. Cross-TU static destruction order is undefined, so this
+    // cleanup hook is needed to guarantee correct ordering.
+    g_ipc->reset();
+}
+
+FUZZ_TARGET(ipc, .init = initialize_ipc, .cleanup = cleanup_ipc)
+{
+    auto& ipc = **g_ipc;
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
     const size_t iterations = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 64);

</details>

enirox001 commented at 8:45 AM on May 19, 2026: contributor

But in the new version it seems inefficient to initialize and tear down the whole IPC framework with each fuzz input. So I'm still wondering if the cleanup hook approach I suggested #35118 (comment) could avoid this overhead while still providing a deterministic destruction order and allowing a clean shutdown.

Thanks for the suggestion @ryanofsky, i had tried this approach previously and it seems to hit a teardown ordering issue under ASan.

With persistent IpcFuzzSetup + cleanup hook, the input runs successfully, but process exit triggers a use-after free during:

FuzzTestSetup::~FuzzTestSetup() -> cleanup_ipc() -> IpcFuzzSetup::~IpcFuzzSetup() -> ProxyClient::~ProxyClient()

So the cleanup hook does not seem to run early enough to avoid teardown ordering issues with thread-local state used by libmultiprocess. I kept the per-input setup/teardown because it gives deterministic lifetime and passes the ASan/UBSan reproducers.

ryanofsky commented at 8:46 PM on May 19, 2026: contributor

re: #35118 (comment)

So the cleanup hook does not seem to run early enough to avoid teardown ordering issues with thread-local state used by libmultiprocess.

Thanks for explaining. I could confirm by running the fuzz test with the suggested change locally. The test crashes on cleanup because the cleanup_ipc function is called after the g_thread_context thread-local variable is destroyed.

But this turned out not to be very difficult to fix. It could be done by (1) declaring FuzzTestSetup thread-local, because C++ destroys thread-local variables before other types of global variables and (2) accessing g_thread_context in the IpcFuzzSetup constructor, because C++ destroys thread-locals in the reverse order that they complete construction.

With these changes, the same IpcFuzzSetup instance can be shared across the fuzz inputs, and it seems like the fuzz test runs around twice as fast. Suggested change with the two fixes is below:

diff --git a/src/test/fuzz/fuzz.cpp b/src/test/fuzz/fuzz.cpp
index 7fba26ddb0b..56da9cf244d 100644
--- a/src/test/fuzz/fuzz.cpp
+++ b/src/test/fuzz/fuzz.cpp
@@ -80,6 +80,28 @@ void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target,
 static std::string_view g_fuzz_target;
 static const TypeTestOneInput* g_test_one_input{nullptr};
 
+class FuzzTestSetup
+{
+public:
+    FuzzTestSetup(const FuzzTargetOptions& options) : m_options{options}
+    {
+        if (m_options.init) m_options.init();
+    }
+    ~FuzzTestSetup()
+    {
+        if (m_options.cleanup) m_options.cleanup();
+    }
+    const FuzzTargetOptions& m_options;
+};
+
+static void test_setup(const FuzzTargetOptions& options)
+{
+    // It is important for this to be declared thread_local in case
+    // options.cleanup() accesses any thread_local variables, because C++
+    // destroys thread_local variables before destroying other types of globals.
+    static thread_local FuzzTestSetup setup{options};
+}
+
 static void test_one_input(FuzzBufferType buffer)
 {
     CheckGlobals check{};
@@ -162,7 +184,7 @@ static void initialize()
     }
     Assert(!g_test_one_input);
     g_test_one_input = &it->second.test_one_input;
-    it->second.opts.init();
+    test_setup(it->second.opts);
 
     ResetCoverageCounters();
 }
diff --git a/src/test/fuzz/fuzz.h b/src/test/fuzz/fuzz.h
index f51310eb363..ac1f83f97a8 100644
--- a/src/test/fuzz/fuzz.h
+++ b/src/test/fuzz/fuzz.h
@@ -26,7 +26,8 @@ using FuzzBufferType = std::span<const uint8_t>;
 
 using TypeTestOneInput = std::function<void(FuzzBufferType)>;
 struct FuzzTargetOptions {
-    std::function<void()> init{[] {}};
+    std::function<void()> init{};
+    std::function<void()> cleanup{};
     bool hidden{false};
 };
 
diff --git a/src/test/fuzz/ipc.cpp b/src/test/fuzz/ipc.cpp
index f66ee6c1f18..74ff9eb67fb 100644
--- a/src/test/fuzz/ipc.cpp
+++ b/src/test/fuzz/ipc.cpp
@@ -17,6 +17,7 @@
 #include <mp/proxy-io.h>
 #include <future>
 #include <memory>
+#include <optional>
 #include <stdexcept>
 #include <thread>
 
@@ -94,20 +95,41 @@ public:
         return m_client->passTransaction(tx);
     }
 
+    // It is important to access g_thread_context here, before initialize_ipc()
+    // returns, to guarantee that g_thread_context will still exist when
+    // cleanup_ipc() is called. C++ destroys thread_local variables in the
+    // reverse order of completion of their construction, so accessing
+    // g_thread_context here before the FuzzTestSetup thread_local variable is
+    // constructed ensures it is alive when cleanup_ipc() is called.
+    mp::ThreadContext& m_thread_context{mp::g_thread_context};
+
 private:
     std::unique_ptr<mp::ProxyClient<test::fuzz::messages::IpcFuzzInterface>> m_client;
     std::thread m_loop_thread;
 };
 
+std::optional<IpcFuzzSetup>* g_ipc;
+
 void initialize_ipc()
 {
     static const auto testing_setup = MakeNoLogFileContext<>();
     (void)testing_setup;
+    static std::optional<IpcFuzzSetup> ipc;
+    ipc.emplace();
+    g_ipc = &ipc;
+}
+
+void cleanup_ipc()
+{
+    // Disconnect and join the event loop thread before fuzz framework objects
+    // are destroyed. Cross-TU static destruction order is undefined, so this
+    // cleanup hook is needed to guarantee correct ordering.
+    g_ipc->reset();
 }
 
-FUZZ_TARGET(ipc, .init = initialize_ipc)
+FUZZ_TARGET(ipc, .init = initialize_ipc, .cleanup = cleanup_ipc)
 {
-    IpcFuzzSetup ipc;
+    auto& ipc = **g_ipc;
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
     const size_t iterations = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 64);

</details>

ryanofsky commented at 9:12 AM on May 20, 2026: contributor

I realized there's actually a much simpler way to share IpcFuzzSetup across fuzz inputs, by taking advantage of the fact that c++ destroys thread_local variables from the main thread before it destroys other globals.

The only change needed on top of current commit 5691f8ccfb142f0d3f7432375dea87785dd5d2dd to avoid creating & destroying IpcFuzzSetup with each fuzz input is:

--- a/src/test/fuzz/ipc.cpp
+++ b/src/test/fuzz/ipc.cpp
@@ -99,15 +99,27 @@ private:
     std::thread m_loop_thread;
 };
 
+IpcFuzzSetup* g_ipc;
+
 void initialize_ipc()
 {
     static const auto testing_setup = MakeNoLogFileContext<>();
     (void)testing_setup;
+
+    // Access g_thread_context here before IpcFuzzSetup is constructed to
+    // guarantee that g_thread_context will still exist when IpcFuzzSetup is
+    // destroyed. (C++ destroys thread_local variables in the reverse order of
+    // completion of their construction.)
+    mp::ThreadContext& thread_context{mp::g_thread_context};
+    (void)thread_context;
+
+    thread_local static IpcFuzzSetup ipc;
+    g_ipc = &ipc;
 }
 
 FUZZ_TARGET(ipc, .init = initialize_ipc)
 {
-    IpcFuzzSetup ipc;
+    auto& ipc = *g_ipc;
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
     const size_t iterations = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 64);

</details>

There's no need for a separate cleanup function at all.

enirox001 force-pushed on May 20, 2026

DrahtBot added the label CI failed on May 20, 2026

DrahtBot commented at 12:17 PM on May 20, 2026: contributor

🚧 At least one of the CI tasks failed. Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/26160038573/job/76949303692 LLM reason (✨ experimental): CI failed because clang-tidy (warnings-as-errors) reported a thread_local variable with a non-trivial destructor in test/fuzz/ipc.cpp.

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

DrahtBot removed the label CI failed on May 20, 2026

enirox001 commented at 1:29 PM on May 20, 2026: contributor

Thanks for giving this another look @ryanofsky

I updated it use the thread_local IpcFuzzSetup, so the IPC setup is shared across fuzz inputs without adding a fuzz framework cleanup hook.

Also added a targeted NOLINT(bitcoin-nontrivial-threadlocal) since the non-trivial thread-local is intentional here to preserve teardown ordering with g_thread_context.

in src/test/fuzz/ipc.cpp:102 in 5837733cd9

  97 | +private:
  98 | +    std::unique_ptr<mp::ProxyClient<test::fuzz::messages::IpcFuzzInterface>> m_client;
  99 | +    std::thread m_loop_thread;
 100 | +};
 101 | +
 102 | +IpcFuzzSetup* g_ipc;

ryanofsky commented at 12:50 PM on May 26, 2026:

In commit "fuzz: add IPC round-trip target" (5837733cd91b3f5f954b6af594469e14ab65a445)

Would be good to declare g_ipc and initialize_ipc static or move them to anonymous namespace so they do not conflict with symbols in other fuzz tests.

enirox001 commented at 2:52 PM on June 1, 2026:

Thanks, declared g_ipc and initialize_ipc as static

in src/test/fuzz/ipc.cpp:156 in 5837733cd9

 151 | +            },
 152 | +            [&] {
 153 | +                UniValue value{UniValue::VOBJ};
 154 | +                value.pushKV("bool", fuzzed_data_provider.ConsumeBool());
 155 | +                value.pushKV("number", fuzzed_data_provider.ConsumeIntegralInRange<int>(-1'000'000, 1'000'000));
 156 | +                value.pushKV("string", "ipc fuzz");

ryanofsky commented at 12:55 PM on May 26, 2026:

In commit "fuzz: add IPC round-trip target" (5837733cd91b3f5f954b6af594469e14ab65a445)

I think could be worth moving these lines into a ConsumeUnivalue function in test/fuzz/util to declutter this test, and potentially benefit from later util improvements.

enirox001 commented at 2:52 PM on June 1, 2026:

Done, moved them to the test/fuzz/util to declutter the test

ryanofsky approved

ryanofsky commented at 1:00 PM on May 26, 2026: contributor

Code review ACK 5837733cd91b3f5f954b6af594469e14ab65a445. Would still like to see feedback from someone more knowledgeable about fuzzing, but this test seems like a good starting point that makes sure serialization and bidirectional communication are working as expected.

fanquake commented at 1:06 PM on May 26, 2026: member

Would still like to see feedback from someone more knowledgeable about fuzzing, @dergoegge approach ack / nack ?

in src/test/fuzz/CMakeLists.txt:1 in 5837733cd9 outdated

ekzyis commented at 4:11 PM on May 27, 2026:

nit: Should the IPC config be defined in a separate CMakeLists.txt as is done for the wallet? This would mean moving the added files in src/test/fuzz to src/ipc/test/fuzz and only have these changes here:

diff --git a/src/test/fuzz/CMakeLists.txt b/src/test/fuzz/CMakeLists.txt
index fc82fdc03a2..f84c78a4839 100644
--- a/src/test/fuzz/CMakeLists.txt
+++ b/src/test/fuzz/CMakeLists.txt
@@ -155,6 +155,10 @@ target_link_libraries(fuzz
   libevent::extra
 )

+if(ENABLE_IPC)
+  add_subdirectory(${PROJECT_SOURCE_DIR}/src/ipc/test/fuzz ipc)
+endif()
+
 if(ENABLE_WALLET)
   add_subdirectory(${PROJECT_SOURCE_DIR}/src/wallet/test/fuzz wallet)
 endif()

However, I tried this, and it's not as simple as moving the changes here to src/ipc/test/fuzz/CMakeLists.txt because of capnp. I had to do what is done here for bitcoin_ipc_test:

https://github.com/bitcoin/bitcoin/blob/00af5620f01dbd1d2e696487303fad0382b67591/src/ipc/CMakeLists.txt#L27-L33

enirox001 commented at 3:15 PM on June 1, 2026:

Thanks for giving this a look @ekzyis.

Fair point. I'll leave it as is for now, given the capnp complication, but happy to move it to src/ipc/test/fuzz in a follow-up once the target grows.

dergoegge commented at 10:17 AM on June 1, 2026: member

Approach ACK

This seems like a good addition and actually looks less complicated than what I thought it would be.

enirox001 force-pushed on Jun 1, 2026

fuzz: add IPC round-trip target

Add an ipc fuzz target behind ENABLE_IPC.

Set up an in-process two-way pipe and use a small reflected interface to exercise libmultiprocess client/server calls.

Round-trip COutPoint, CScript, std::vector<uint8_t>, UniValue, and transactions.

72da982a9d

enirox001 force-pushed on Jun 1, 2026

DrahtBot added the label CI failed on Jun 1, 2026

DrahtBot commented at 3:05 PM on June 1, 2026: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/26762695083/job/78880436550 LLM reason (✨ experimental): CI failed because the lint “includes” check reported a quote-style include (#include "univalue.h") that should use bracket syntax in src/test/fuzz/util.h.

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

DrahtBot removed the label CI failed on Jun 1, 2026