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

Coverage report provided by marcofleon

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
ACK	marcofleon, sedited
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.

ryanofsky commented at 6:31 PM on June 16, 2026:

re: #35118 (review)

I think ipc fuzz tests won't be able to follow the exact same pattern of wallet fuzz tests in cmake due to constraints around the target_capnp_sources function, as it seems like was encountered above.

But I do think it would be good to move IPC fuzz files from src/test/fuzz/ipc* to src/ipc/test/fuzz/ to get closer to that organization for a few reasons:

Consistency with where wallet fuzz tests are located and with where ipc unit tests are located.
Making it possible to reference src/ipc/capnp/*.capnp schema files in fuzz tests. Right now the fuzz test is using its own schema file, but it could definitely be useful to reference other bitcoin capnp schema files when writing new tests. This is the reason the bitcoin_ipc_test ilbrary is declared in src/ipc/ and it seems like it would make sense to declare the bitcoin_ipc_fuzz library there too.
Avoiding potential incompatibilities with https://github.com/bitcoin-core/libmultiprocess/commit/620f297f311718f3d186b716676779e046e3255c from https://github.com/bitcoin-core/libmultiprocess/pull/289 since it looks like right now 72da982a9dda9c2a21318f7ef77085c4a3306882 is passing ${PROJECT_SOURCE_DIR}/src to target_capnp_sources instead of ${CMAKE_CURRENT_SOURCE_DIR} which is different than other code is doing, and might get broken after next libmultiprocess subtree update.

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

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

in src/test/fuzz/ipc.cpp:67 in 72da982a9d

  62 | +    {
  63 | +        m_client.reset();
  64 | +        if (m_loop_thread.joinable()) m_loop_thread.join();
  65 | +    }
  66 | +
  67 | +    int Add(int a, int b)

ryanofsky commented at 6:27 PM on June 16, 2026:

In commit "fuzz: add IPC round-trip target" (72da982a9dda9c2a21318f7ef77085c4a3306882)

Seems like it would be simpler not to have these one-line methods and for callers to just access the client directly. Is there a reason for these to exist?

enirox001 commented at 2:41 PM on June 17, 2026:

Done, they were there previously because they were calling CallOnClientThreadAndWait, but since this has been removed, they are no longer needed.

The callers now access the client directly

ryanofsky approved

ryanofsky commented at 6:35 PM on June 16, 2026: contributor

Code review ACK 72da982a9dda9c2a21318f7ef77085c4a3306882, just adding ConsumeUnivalue and static methods since last review.

This looks good, but think it would also be good to move new code from src/test/fuzz to src/test/ipc/fuzz as suggested by @ekzyis #35118 (review) and I mentioned a few more reasons below. This could also be done as a followup

DrahtBot requested review from dergoegge on Jun 16, 2026

enirox001 force-pushed on Jun 17, 2026

DrahtBot added the label CI failed on Jun 17, 2026

DrahtBot commented at 2:55 PM on June 17, 2026: contributor

🚧 At least one of the CI tasks failed. Task macOS-cross to arm64: https://github.com/bitcoin/bitcoin/actions/runs/27697123515/job/81923242080 LLM reason (✨ experimental): CI failed because the fuzz build failed with a compiler fatal error: mp/proxy.capnp.h header file not found.

<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 commented at 3:50 PM on June 17, 2026: contributor

Thanks for the review @ryanofsky

In the latest push, I removed the one-line methods and moved the files to src/ipc/test/fuzz/ to be consistent with how the IPC unit tests are organized.

DrahtBot removed the label CI failed on Jun 17, 2026

in src/ipc/CMakeLists.txt:50 in 46b2ac7999 outdated

  46 | @@ -47,4 +47,21 @@ if(BUILD_TESTS)
  47 |    )
  48 |  endif()
  49 |  
  50 | +if (BUILD_FUZZ_BINARY)

sedited commented at 9:32 AM on June 21, 2026:

I think this entire commit would make more sense squashed into the previous commit. In my opinion there is little point in just introducing the files if they cannot actually be tested.

enirox001 commented at 3:32 PM on June 25, 2026:

Done, squashed this into the second commit.

sedited commented at 11:56 AM on June 21, 2026: contributor

This is not working for me:

FUZZ=ipc ./build_fuzz_nosan/bin/fuzz -timeout=10 ipc
No fuzz target compiled for ipc.

I think your latest pushes introduced this regression. My guess is this is because the fuzz targets are statically registered and that silently breaks if you link them in an external library. I would suggest the following (which fixes it for me):

<details> <summary>Click to expand diff</summary>

diff --git a/src/ipc/CMakeLists.txt b/src/ipc/CMakeLists.txt
index 8326423d8b..e9bdf0b39b 100644
--- a/src/ipc/CMakeLists.txt
+++ b/src/ipc/CMakeLists.txt
@@ -47,4 +47,19 @@ if(BUILD_TESTS)
   )
 endif()
 
+if (BUILD_FUZZ_BINARY)
+  add_library(bitcoin_ipc_fuzz STATIC EXCLUDE_FROM_ALL)
+  target_capnp_sources(bitcoin_ipc_fuzz ${CMAKE_CURRENT_SOURCE_DIR}
+    test/fuzz/ipc_fuzz.capnp
+  )
+  add_dependencies(bitcoin_ipc_fuzz bitcoin_ipc_headers)
+
+  target_link_libraries(bitcoin_ipc_fuzz
+    PRIVATE
+      core_interface
+      univalue
+      Boost::headers
+  )
+endif()
+
 configure_file(.clang-tidy.in .clang-tidy USE_SOURCE_PERMISSIONS COPYONLY)
diff --git a/src/ipc/test/fuzz/CMakeLists.txt b/src/ipc/test/fuzz/CMakeLists.txt
new file mode 100644
index 0000000000..936e84c485
--- /dev/null
+++ b/src/ipc/test/fuzz/CMakeLists.txt
@@ -0,0 +1,7 @@
+# Copyright (c) The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or https://opensource.org/license/mit/.
+
+target_sources(fuzz PRIVATE ${PROJECT_SOURCE_DIR}/src/ipc/test/fuzz/ipc.cpp)
+target_link_libraries(fuzz bitcoin_ipc_fuzz multiprocess)
+
diff --git a/src/test/fuzz/CMakeLists.txt b/src/test/fuzz/CMakeLists.txt
index fc82fdc03a..2b395e6b3c 100644
--- a/src/test/fuzz/CMakeLists.txt
+++ b/src/test/fuzz/CMakeLists.txt
@@ -138,6 +138,10 @@ add_executable(fuzz
   versionbits.cpp
 )
 
+if(ENABLE_IPC)
+  add_subdirectory(${PROJECT_SOURCE_DIR}/src/ipc/test/fuzz ipc)
+endif()
+
 add_windows_application_manifest(fuzz)
 
 target_link_libraries(fuzz

</details>

in src/ipc/test/fuzz/ipc.cpp:133 in 46b2ac7999

 128 | +                CMutableTransaction mutable_tx;
 129 | +                mutable_tx.version = fuzzed_data_provider.ConsumeBool() ? CTransaction::CURRENT_VERSION : 1;
 130 | +                mutable_tx.nLockTime = fuzzed_data_provider.ConsumeIntegral<uint32_t>();
 131 | +                mutable_tx.vin.emplace_back(COutPoint{Txid::FromUint256(ConsumeUInt256(fuzzed_data_provider)),
 132 | +                                                      fuzzed_data_provider.ConsumeIntegral<uint32_t>()});
 133 | +                mutable_tx.vout.emplace_back(fuzzed_data_provider.ConsumeIntegralInRange<CAmount>(0, COIN), CScript{});

sedited commented at 12:22 PM on June 21, 2026:

I think it would be better to use the ConsumeTransaction utility. If i do that I also run into a fuzz failure pretty much immediately, probably caused by some missing or zero length fields in a transaction.

enirox001 commented at 3:32 PM on June 25, 2026:

Updated this to use the ConsumeTransaction utility as mentioned.

I also hit the failure you had mentioned. Transactions with vin=0 cause a fatal SpanReader::read(): end of data in libmultiprocess's serialization layer rather than a graceful error. Added a guard to skip empty vin transactions for now.

sedited commented at 12:24 PM on June 21, 2026: contributor

Concept ACK

I think it would also be good to assert the invariant on the server side. Maybe you can add some data fields to the IPCFuzzImplementation that get populated in the fuzz test cases and then checked against the passed through values in the interface member functions.

enirox001 force-pushed on Jun 25, 2026

DrahtBot added the label CI failed on Jun 25, 2026

DrahtBot commented at 2:54 PM on June 25, 2026: contributor

🚧 At least one of the CI tasks failed. Task Alpine (musl): https://github.com/bitcoin/bitcoin/actions/runs/28178614348/job/83461736562 LLM reason (✨ experimental): CI failed because the CMake configure “try_compile” crashed with a compiler error: g++ doesn’t recognize the flag -Wunique-object-duplication.

<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 commented at 3:32 PM on June 25, 2026: contributor

This is not working for me:
FUZZ=ipc ./build_fuzz_nosan/bin/fuzz -timeout=10 ipc
No fuzz target compiled for ipc.
I think your latest pushes introduced this regression. My guess is this is because the fuzz targets are statically registered and that silently breaks if you link them in an external library. I would suggest the following (which fixes it for me):

Thanks for catching this. I tested the build with a stale build directory, which is why I didn't catch this. The diff you provided fixed this.

I think it would also be good to assert the invariant on the server side. Maybe you can add some data fields to the IPCFuzzImplementation that get populated in the fuzz test cases and then checked against the passed through values in the interface member functions.

Thanks for the suggestion. Similar to the non-libmultiprocess client/server extension @ryanofsky mentioned here, I'd prefer to keep this PR focused on the basic IPC fuzz infrastructure and address server-side invariant checks in a follow-up.

enirox001 force-pushed on Jun 26, 2026

DrahtBot removed the label CI failed on Jun 26, 2026

in src/ipc/CMakeLists.txt:52 in c4908e5f7b

  46 | @@ -47,4 +47,21 @@ if(BUILD_TESTS)
  47 |    )
  48 |  endif()
  49 |  
  50 | +if (BUILD_FUZZ_BINARY)
  51 | +  add_library(bitcoin_ipc_fuzz STATIC EXCLUDE_FROM_ALL
  52 | +    test/fuzz/ipc.cpp

sedited commented at 1:00 PM on June 26, 2026:

I think this doesn't need to be listed here. It is compiled in through the fuzz target.

enirox001 commented at 10:42 AM on June 27, 2026:

Done, Removed this since it is compiled in through the fuzz target. Thanks

in CMakeLists.txt:234 in c4908e5f7b outdated

 268 | -  else()
 269 | -    find_package(SQLite3 3.7.17 REQUIRED)
 270 | -    if(NOT TARGET SQLite3::SQLite3) # CMake < 4.3
 271 | -      add_library(SQLite3::SQLite3 ALIAS SQLite::SQLite3)
 272 | -    endif()
 273 | -  endif()

sedited commented at 1:07 PM on June 26, 2026:

Just a note: This was set explicitly in pull request #33235. Are there any environment, like oss-fuzz, where this might lead to problems?

enirox001 commented at 10:40 AM on June 27, 2026:

I believe you are speaking about the first commit that removes the ENABLE_IPC flag

The expectation noted in that PR was that it would need to go away once IPC fuzzing exists #33235 (comment)

This should not force IPC on in environments that already explicitly disable it. For example, current oss-fuzz builds Bitcoin Core with NO-IPC=1 as seen here, and the generated depends toolchain will set ENABLE_IPC=OFF in that case. So oss-fuzz should continue to build without capnp

If we want to get this target running in oss-fuzz, we need a follow-up oss-fuzz change to stop using NO_IPC=1. This PR just prevents BUILD_FOR_FUZZING from unconditionally overriding a user provided IPC setting.

enirox001 force-pushed on Jun 27, 2026

sedited approved

sedited commented at 10:49 AM on June 27, 2026: contributor

ACK e3a501242063adbb6e5a3f431007264128091bf5

DrahtBot requested review from ryanofsky on Jun 27, 2026

in src/ipc/test/fuzz/ipc.cpp:114 in e3a5012420

 109 | +                COutPoint expected{outpoint.hash, outpoint.n ^ 0xFFFFFFFFu};
 110 | +                assert(ipc.m_client->passOutPoint(outpoint) == expected);
 111 | +            },
 112 | +            [&] {
 113 | +                std::vector<uint8_t> value = ConsumeRandomLengthByteVector<uint8_t>(fuzzed_data_provider, 512);
 114 | +                if (value.empty()) value.push_back(0);

marcofleon commented at 2:05 PM on July 1, 2026:

I'm not quite seeing why we need this line. Is an empty vector not a case we should be covering?

enirox001 commented at 7:08 PM on July 1, 2026:

This is a workaround to empty input, because of a libmultiprocess sanitizer issue

enirox001 commented at 8:25 PM on July 1, 2026:

Removing this causes the UBSan job to find that the empty std::vector<uint8_t> case currently trips an invalid-null-argument in libmultiprocess' byte-span serializer.

For an empty vector, data() may be null, and libmultiprocess currently calls memcpy(result.begin(), data.data(), data.size()). Even with size 0, UBSan flags the null pointer argument.

So I restored the guard, but added a comment explaining why it is there. This is not because the IPC round-trip cannot logically handle empty data; it is to avoid a known sanitizer issue in libmultiprocess, which I do not think this PR should change.

marcofleon commented at 3:28 PM on July 2, 2026:

So it is real UB? This isn't a blocker, more just trying to understand. Would it make sense in a followup to address it (in type-data.h I'm assuming) and then remove this line at some point?

maflcko commented at 3:46 PM on July 2, 2026:

IIUC this is a GCC bug, see #32016 (comment)

Maybe fixed in GCC-16?

marcofleon commented at 2:19 PM on July 1, 2026: contributor

Nice, good to see an IPC fuzz test. Here's the coverage after fuzzing for a while.

Left a question.

enirox001 force-pushed on Jul 1, 2026

enirox001 commented at 7:08 PM on July 1, 2026: contributor

Nice, good to see an IPC fuzz test. Here's the coverage after fuzzing for a while.

Thanks for giving this a look @marcofleon. Will update the PR description with this coverage report

sedited approved

sedited commented at 7:14 PM on July 1, 2026: contributor

ACK cf2c65e0cc3e75d7ba70e0b78a1515a48f39df76

DrahtBot added the label CI failed on Jul 1, 2026

DrahtBot commented at 8:10 PM on July 1, 2026: contributor

🚧 At least one of the CI tasks failed. Task fuzzer,address,undefined,integer: https://github.com/bitcoin/bitcoin/actions/runs/28541205184/job/84615183126 LLM reason (✨ experimental): Fuzz testing failed due to an AddressSanitizer UndefinedBehaviorSanitizer error: invalid-null-argument (null passed) in mp/type-data.h during IPC fuzzing.

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

Add bitcoin_ipc_fuzz static library and link it to the fuzz target
via a new src/ipc/test/fuzz/CMakeLists.txt.

037ad77071

enirox001 force-pushed on Jul 1, 2026

DrahtBot removed the label CI failed on Jul 1, 2026

in src/ipc/test/fuzz/ipc.cpp:126 in 037ad77071

 121 | +                CScript expected{script};
 122 | +                expected << OP_NOP;
 123 | +                assert(ipc.m_client->passScript(script) == expected);
 124 | +            },
 125 | +            [&] {
 126 | +                UniValue value = ConsumeUniValue(fuzzed_data_provider);

marcofleon commented at 3:25 PM on July 2, 2026:

ConsumeUniValue, especially as a general fuzzing util helper, looks limited with the inputs it's creating. We could drop it entirely and roundtrip test more arbitrary JSON by replacing the above line with something like

UniValue value;
if (!value.read(fuzzed_data_provider.ConsumeRandomLengthString(512))) return;

maflcko commented at 3:49 PM on July 2, 2026:

Agree. It is probably not a good idea to expose this helper in src/test/fuzz/util.h. Otherwise, there could be a risk that other fuzz targets are using it and silently fail to gain any coverage, because the UniValue fields are hard-coded to constants.

sedited commented at 7:05 PM on July 2, 2026:

Lets do this in a tiny follow up.

marcofleon commented at 3:28 PM on July 2, 2026: contributor

ACK 037ad77071419367fd679da71d5c39853d1e622e

Non-blocking suggestion for a potential followup.

DrahtBot requested review from sedited on Jul 2, 2026

sedited approved

sedited commented at 4:04 PM on July 2, 2026: contributor

Re-ACK 037ad77071419367fd679da71d5c39853d1e622e

sedited merged this on Jul 2, 2026

sedited closed this on Jul 2, 2026

sedited referenced this in commit 1835f2fcbf on Jul 3, 2026

in src/ipc/test/fuzz/ipc.cpp:96 in 037ad77071

  91 | +{
  92 | +    auto& ipc = *g_ipc;
  93 | +    FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
  94 | +    const size_t iterations = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 64);
  95 | +
  96 | +    for (size_t i = 0; i < iterations; ++i) {

maflcko commented at 12:10 PM on July 4, 2026:

This is fine, but using the general pattern that all other fuzz tests use seems better:

LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 64)

This ensures:

When no data is available, a simple and single 64 value in the fuzz input will not result in 64 loops over the same body with the same default/fallback values
When no data is available, ConsumeBool falls back to false and breaks the loop early.
When further data is available, the overhead is just a single byte, making it also possibly easier for the fuzz engine to mutate the data, as a single int that influences the whole remainder of the fuzz input can lead to the 'havoc' effect.
When a crash is reduced, deleting bytes will directly influence the execution length, so byte-length of the fuzz input roughly corresponds to run-time length

Idk, it is probably fine here, but I think it would be better to use the common pattern for the given benefits.