fuzz: improve ipc fuzz coverage #35916

pull enirox001 wants to merge 4 commits into bitcoin:master from enirox001:08-26-fuzz-ipc-coverage changing 3 files +277 −13
  1. enirox001 commented at 1:16 PM on August 6, 2026: contributor

    <!-- *** Please remove the following help text before submitting: *** Pull requests may be closed immediately if they: - do not have a rationale and clear improvement - do not adhere to doc/AI_POLICY.md GUI-related pull requests should be opened against https://github.com/bitcoin-core/gui first. See CONTRIBUTING.md -->

    <!-- Please provide clear motivation for your patch and explain how it improves Bitcoin Core user experience or Bitcoin Core developer experience significantly: * Any test improvements or new tests that improve coverage are always welcome. * All other changes should have accompanying unit tests (see `src/test/`) or functional tests (see `test/`). Contributors should note which tests cover modified code. If no tests exist for a region of modified code, new tests should accompany the change. * Bug fixes are most welcome when they come with steps to reproduce or an explanation of the potential issue as well as reasoning for the way the bug was fixed. * Features are welcome, but might be rejected due to design or scope issues. If a feature is based on a lot of dependencies, contributors should first consider building the system outside of Bitcoin Core, if possible. * Refactoring changes are only accepted if they are required for a feature or bug fix or otherwise improve developer experience significantly. For example, most "code style" refactoring changes require a thorough explanation why they are useful, what downsides they have and why they *significantly* improve developer experience or avoid serious programming bugs. Note that code style is often a subjective matter. Unless they are explicitly mentioned to be preferred in the [developer notes](/doc/developer-notes.md), stylistic code changes are usually rejected. -->

    <!-- Bitcoin Core has a thorough review process and even the most trivial change needs to pass a lot of eyes and requires non-zero or even substantial time effort to review. There is a huge lack of active reviewers on the project, so patches often sit for a long time. -->

    This PR follows up on #35118 and continues the IPC fuzzing work proposed in #23015.

    The initial ipc fuzz target exercised normal calls through a libmultiprocess client and server. This PR extends the target with

    • Server side assertions that verify arguments arrive unchanged (https://github.com/bitcoin/bitcoin/pull/35118#issuecomment-4452992198 and #35118#pullrequestreview-4539477599)
    • A callback call that exercises ipc communication in both directions.
    • Raw capnp requests containing arbitrary transaction and UniValue payloads, which are deserialized by a normal libmultiprocess server.
    • Raw capnp responses containing arbitrary transaction and UniValue payloads, which are deserialized by a normal libmultiprocess client.

    The raw request and response cases allow the fuzz target to exercise deserialization with data that was not first produced by libmultiprocess serialization.

    The ipc setup and callback handling closely follow the existing libmultiprocess tests, particularly their use of EventLoop, ProxyClient, ProxyServer, two-way pipes, and initThreadMap.

    The transaction and UniValue payload cases exercise the serialization hooks defined in common-types.h.

    These files may provide useful background when reviewing the changes.

    Generated coverage report after fuzzing the IPC target for a while using the qa-assets corpus.

  2. fuzz: assert IPC arguments on server side
    Keep a reference to the IPC fuzz implementation so fuzz inputs can be compared
    directly with values deserialized by the proxy server.
    
    This ensures serialization errors are detected on the client to server path instead
    of potentially being hidden by the return trip.
    9270ace9bb
  3. fuzz: exercise IPC callback
    Add a callback interface to the IPC fuzz target so requests exercise communication
    from the server back to the client.
    
    Check the callback argument and return value on both sides.
    7bae915235
  4. DrahtBot added the label Fuzzing on Aug 6, 2026
  5. DrahtBot commented at 1:16 PM on August 6, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

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

    Type Reviewers
    Stale ACK jeanpablojp

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #36087 (util: Add and use AssertUnreachable by maflcko)
    • #35975 (wallet: Fix CWalletTx malleated transaction metadata sync by achow101)
    • #32387 (ipc: add windows support by ryanofsky)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  6. enirox001 renamed this:
    fuzz: improve IPC fuzz coverage
    fuzz: improve ipc fuzz coverage
    on Aug 6, 2026
  7. jeanpablojp commented at 3:40 PM on August 15, 2026: contributor

    tACK bd24122dfbfbaded49ef551c01184b4b5b48e27f

    All green, and it matches the follow-up #35118 asked for.

  8. in src/ipc/test/fuzz/ipc.cpp:180 in bd24122dfb outdated
     175 | +            m_client->m_context.loop->m_task_set->add(request.send().then(
     176 | +                [&](auto&&) {
     177 | +                    done.set_value();
     178 | +                },
     179 | +                [&](kj::Exception&& exception) {
     180 | +                    done.set_exception(std::make_exception_ptr(
    


    jeanpablojp commented at 3:40 PM on August 15, 2026:

    Took me a bit to see why sendTransactionPayload swallows FAILED while sendUniValuePayload treats any kj error as fatal. That's correct, but you only get it by reading the hooks. A one-line comment would have helped, feel free to ignore.

                        // UniValue deserialization never throws (read() failure is ignored), so any kj error is unexpected.
                        done.set_exception(std::make_exception_ptr(
    

    enirox001 commented at 11:33 AM on August 26, 2026:

    Yes, I agree that this can be clarified a bit more, added the suggested comment to clarify why any KJ error is unexpected here. Thanks

  9. enirox001 force-pushed on Aug 26, 2026
  10. fuzz: send malformed IPC request payloads
    Use the underlying capnp client to send arbitrary transaction data and JSON text to the
    libmultiprocess server.
    
    Accept normal transaction deserialization failures while surfacing unexpected IPC errors.
    4a2bdca741
  11. fuzz: return malformed IPC response payloads
    Connect a normal libmultiprocess client to a manual capnp server.
    
    Return arbitrary transaction data and json text to exercise client side
    deserialization.
    82bed574e4
  12. enirox001 force-pushed on Aug 26, 2026
  13. DrahtBot added the label CI failed on Aug 26, 2026
  14. DrahtBot commented at 12:34 PM on August 26, 2026: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

    🚧 At least one of the CI tasks failed. <sub>Task Windows native, fuzz, VS: https://github.com/bitcoin/bitcoin/actions/runs/32963804197/job/98161775031</sub> <sub>LLM reason (✨ experimental): CI failed because the fuzz target rpc crashed with Windows exit code 3221225477 (access violation).</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>

  15. DrahtBot removed the label CI failed on Aug 26, 2026
  16. enirox001 commented at 7:57 AM on August 28, 2026: contributor

    See the generated coverage report after fuzzing the IPC target for a while using the qa-assets corpus.

    Also included in the pr description


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-08-31 18:51 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me