Add functional test for IPC interface #33201

pull sipa wants to merge 5 commits into bitcoin:master from sipa:202508_ipc_test changing 11 files +269 −17
  1. sipa commented at 8:10 pm on August 15, 2025: member
    This adds support to the functional test framework to run the multiprocess bitcoin-node binary, and then tests it in a new interface_ipc.py functional test through the pycapnp module.
  2. DrahtBot commented at 8:10 pm on August 15, 2025: 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/33201.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK janb84
    Stale ACK josibake, ismaelsadeeq, Sjors, ryanofsky

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #33142 (test: Run bench sanity checks in parallel with functional tests by maflcko)
    • #32989 (ci: Migrate CI to hosted Cirrus Runners by willcl-ark)
    • #32297 (bitcoin-cli: Add -ipcconnect option by ryanofsky)
    • #32162 (depends: Switch from multilib to platform-specific toolchains by hebasto)
    • #31802 (Add bitcoin-{node,gui} to release binaries for IPC by Sjors)
    • #31723 (qa debug: Add –debug_runs/-waitfordebugger [DRAFT] by hodlinator)
    • #30437 (ipc: add bitcoin-mine test program by ryanofsky)
    • #29838 (Feature: Use different datadirs for different signets by BrandonOdiwuor)

    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.

    LLM Linter (✨ experimental)

    Possible typos and grammar issues:

    • in test/README.md: “requires a python IPC library” -> “requires a Python IPC library” [“Python” is a proper noun]
    • in test/functional/interface_ipc.py comment: “have it’s own” -> “have its own” [possessive “its,” not contraction “it’s”]

    No other comprehension-impacting typos were found.

    drahtbot_id_4_m

  3. sipa force-pushed on Aug 15, 2025
  4. DrahtBot added the label CI failed on Aug 15, 2025
  5. DrahtBot commented at 8:14 pm on August 15, 2025: contributor

    🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/48194727681 LLM reason (✨ experimental): The CI failure is caused by errors detected during the linting process, specifically from ruff and mypy, indicating code issues and missing module imports.

    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.

  6. sipa force-pushed on Aug 15, 2025
  7. ryanofsky commented at 8:46 pm on August 15, 2025: contributor

    Concept ACK. 27bd67857dd0e145d7539b949df2b7aaa53c6b1b is really nice and simpler than I would have expected.

    In case it’s useful #30437 adds an interface_ipc_mining.py test, and #32297 adds an interface_ipc_cli.py test which are similar to this test and have similar test framework updates. Code from those PRs might be useful here or vice versa.

  8. sipa force-pushed on Aug 15, 2025
  9. sipa force-pushed on Aug 15, 2025
  10. sipa force-pushed on Aug 15, 2025
  11. sipa force-pushed on Aug 16, 2025
  12. Sjors commented at 7:06 am on August 16, 2025: member

    Concept ACK

    So far I achieved test coverage for the Mining interface with a combination of unit tests and by having RPC methods such as getblocktemplate use the interface internally. But the latter has side-effects (see #32547) and could be avoided once we have enough direct coverage.

    CentOS CI fails with capnp.lib.capnp.KjException: mp/proxy.capnp:6: failed: Import failed: /capnp/c++.capn https://cirrus-ci.com/task/4824947408764928?logs=ci#L3320

  13. in test/functional/interface_ipc.py:119 in b87de46c7a outdated
    69+            mining = init.makeMining(ctx)
    70+            opts = modules['mining'].BlockCreateOptions()
    71+            opts.useMempool = True
    72+            opts.blockReservedWeight = 4000
    73+            opts.coinbaseOutputMaxAdditionalSigops = 0
    74+            template = mining.result.createNewBlock(opts)
    


    Sjors commented at 7:09 am on August 16, 2025:
    It would be great to cover the waitNext() method on template.

    sipa commented at 1:31 pm on August 17, 2025:
    Done.
  14. sipa force-pushed on Aug 16, 2025
  15. sipa force-pushed on Aug 16, 2025
  16. sipa force-pushed on Aug 16, 2025
  17. DrahtBot removed the label CI failed on Aug 16, 2025
  18. sipa force-pushed on Aug 16, 2025
  19. sipa force-pushed on Aug 16, 2025
  20. DrahtBot added the label CI failed on Aug 16, 2025
  21. sipa force-pushed on Aug 17, 2025
  22. sipa force-pushed on Aug 17, 2025
  23. DrahtBot removed the label CI failed on Aug 17, 2025
  24. in ci/test/01_base_install.sh:55 in eb482c1f34 outdated
    51@@ -52,7 +52,7 @@ fi
    52 
    53 if [ -n "$PIP_PACKAGES" ]; then
    54   # shellcheck disable=SC2086
    55-  ${CI_RETRY_EXE} pip3 install --user $PIP_PACKAGES
    56+  ${CI_RETRY_EXE} pip3 install --break-system-packages --user $PIP_PACKAGES
    


    Sjors commented at 7:02 am on August 17, 2025:
    In eb482c1f3423fb5d5b91bd47b8bd6ec290d55d33 ci: add functional test for IPC interface: @maflcko thoughts on how to avoid --break-system-packages? On CI it’s probably fine, but for the unix build instructions it seems less optimal?

    sipa commented at 11:38 am on August 17, 2025:

    @Sjors I’ll expand the functional tests with some more methods now that CI works. It took many tries to find something that works on a reasonable number of platforms.

    I have tried pip3 install pycapnp on macOS CI, but that gives a strange error that I also got when running locally. Locally, I fixed that by installing pycapnp from source, but in macOS CI, that resulted in C++ errors about Python functions that were not found. At this point, I’m going to give up on getting it to work there; someone else can work on adding that.

    Regarding you running locally, where is your c++.capnp installed? The interface_i2p.py script currently just assumes it’s in /usr/include/capnp/c++.capnp, but that will not work for custom installations.


    Sjors commented at 1:05 pm on August 17, 2025:

    where is your c++.capnp installed?

    /opt/homebrew/bin/capnpc-c++

    This makes the test pass:

     0diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
     1index 759fbbbc74..6f34a0dca6 100755
     2--- a/test/functional/interface_ipc.py
     3+++ b/test/functional/interface_ipc.py
     4@@ -33,7 +33,10 @@ class IPCInterfaceTest(BitcoinTestFramework):
     5     def capnp_modules(self):
     6         src_dir = Path(self.config['environment']['SRCDIR']) / "src"
     7         mp_dir = src_dir / "ipc" / "libmultiprocess" / "include"
     8-        imports = ["/usr/include", str(src_dir), str(mp_dir)]
     9+        # c++.capnp path:
    10+        # https://github.com/capnproto/pycapnp/issues/279#issuecomment-2477709244
    11+        cpp_capnp_dir = Path(capnp.__path__[0]).parent
    12+        imports = ["/usr/include", str(src_dir), str(mp_dir), str(cpp_capnp_dir)]
    13         return {
    14             "proxy": capnp.load(str(mp_dir / "mp" / "proxy.capnp"), imports=imports),
    15             "init": capnp.load(str(src_dir / "ipc" / "capnp" / "init.capnp"), imports=imports),
    

    See https://github.com/capnproto/pycapnp/issues/279


    sipa commented at 1:31 pm on August 17, 2025:
    Done.
  25. Sjors commented at 7:08 am on August 17, 2025: member

    On macOS 15.6 with capnp 1.2.0 installed via Homebrew and Python 3.10.14 installed via PyEnv I get:

     0pip3 install pycapnp
     1...
     2Successfully installed pycapnp-2.0.0
     3
     4cmake -B build
     5cmake --build build
     6...
     7
     8% build/test/functional/interface_ipc.py
     92025-08-17T07:05:49.321522Z TestFramework (INFO): PRNG seed is: 6039316184654231320
    102025-08-17T07:05:49.322566Z TestFramework (INFO): Initializing test directory /var/folders/8p/qqdl2fsj7rd6q096jtmsfydc0000gn/T/bitcoin_func_test_z3jtr5u1
    112025-08-17T07:05:50.595292Z TestFramework (ERROR): Unexpected exception
    12Traceback (most recent call last):
    13  File "/Users/sjors/dev/bitcoin/test/functional/test_framework/test_framework.py", line 200, in main
    14    self.run_test()
    15  File "/Users/sjors/dev/bitcoin/build/test/functional/interface_ipc.py", line 82, in run_test
    16    self.run_echo_test()
    17  File "/Users/sjors/dev/bitcoin/build/test/functional/interface_ipc.py", line 63, in run_echo_test
    18    asyncio.run(capnp.run(async_routine()))
    19  File "/Users/sjors/.pyenv/versions/3.10.14/lib/python3.10/asyncio/runners.py", line 44, in run
    20    return loop.run_until_complete(main)
    21  File "/Users/sjors/.pyenv/versions/3.10.14/lib/python3.10/asyncio/base_events.py", line 649, in run_until_complete
    22    return future.result()
    23  File "capnp/lib/capnp.pyx", line 1944, in run
    24  File "capnp/lib/capnp.pyx", line 1945, in capnp.lib.capnp.run
    25  File "/Users/sjors/dev/bitcoin/build/test/functional/interface_ipc.py", line 57, in async_routine
    26    init, ctx = await self.make_capnp_init_ctx()
    27  File "/Users/sjors/dev/bitcoin/build/test/functional/interface_ipc.py", line 45, in make_capnp_init_ctx
    28    modules = self.capnp_modules()
    29  File "/Users/sjors/dev/bitcoin/build/test/functional/interface_ipc.py", line 38, in capnp_modules
    30    "proxy": capnp.load(str(mp_dir / "mp" / "proxy.capnp"), imports=imports),
    31  File "capnp/lib/capnp.pyx", line 4385, in capnp.lib.capnp.load
    32  File "capnp/lib/capnp.pyx", line 3574, in capnp.lib.capnp.SchemaParser.load
    33capnp.lib.capnp.KjException: mp/proxy.capnp:6: failed: Import failed: /capnp/c++.capnp
    

    Maybe related: https://github.com/capnproto/pycapnp/issues/377 (though pycapnp-1.3.0 doesn’t work at all, since it doesn’t have the run method)

    Suggested test doc improvement:

     0diff --git a/test/README.md b/test/README.md
     1index 37f2c07217..3b2979cdf6 100644
     2--- a/test/README.md
     3+++ b/test/README.md
     4@@ -34,6 +34,9 @@ The ZMQ functional test requires a python ZMQ library. To install it:
     5 - on Unix, run `sudo apt-get install python3-zmq`
     6 - on mac OS, run `pip3 install pyzmq`
     7
     8+The IPC functional test requires a python IPC library. To install it:
     9+
    10+- `pip3 install pycapnp`
    11
    12 On Windows the `PYTHONUTF8` environment variable must be set to 1:
    
  26. sipa force-pushed on Aug 17, 2025
  27. sipa commented at 1:33 pm on August 17, 2025: member
    Rebased on #33190, added instructions to test/README.md, added extra paths for finding capnp/c++.capnp, and expanded the tests to cover more methods.
  28. janb84 commented at 3:30 pm on August 17, 2025: contributor

    On macOS 15.6 with capnp 1.2.0 installed via Homebrew and Python 3.10.14 installed via PyEnv I get:

    0% build/test/functional/interface_ipc.py
    12025-08-17T07:05:49.321522Z TestFramework (INFO): PRNG seed is: 6039316184654231320
    22025-08-17T07:05:49.322566Z TestFramework (INFO): Initializing test directory 
    

    Maybe related: capnproto/pycapnp#377 (though pycapnp-1.3.0 doesn’t work at all, since it doesn’t have the run method)

    Experiencing related errors in a nix-shell on macos (the change as suggested by Sjors did not fix it)

     0build/test/functional/test_runner.py -j8 interface_ipc.py
     1Temporary test directory at /private/tmp/nix-shell-46943-2562808985/test_runner_₿_🏃_20250817_171423
     2Remaining jobs: [interface_ipc.py]
     31/1 - interface_ipc.py failed, Duration: 1 s
     4
     5stdout:
     62025-08-17T15:14:23.339367Z TestFramework (INFO): PRNG seed is: 8709597786033281775
     72025-08-17T15:14:23.339826Z TestFramework (INFO): Initializing test directory /private/tmp/nix-shell-46943-2562808985/test_runner_₿_🏃_20250817_171423/interface_ipc_0
     82025-08-17T15:14:24.140276Z TestFramework (INFO): Running echo test
     9
    10
    11stderr:
    12libc++abi: terminating due to uncaught exception of type kj::ExceptionImpl: mp/proxy.capnp:6: failed: Import failed: /capnp/c++.capnp
    13
    14TEST             | STATUS    | DURATION
    15
    16interface_ipc.py | ✖ Failed  | 1 s
    

    location of c++.capnp: /nix/store/qfb31v9la93rp92krx05ndzm5nidsrzd-capnproto-1.2.0/include/capnp

  29. sipa commented at 3:32 pm on August 17, 2025: member
    @janb84 Yeah, the include paths used in the python test are a guess. It’d be nice to pass through the location of the capnp installation from the build system to the tests, but I don’t know how.
  30. janb84 commented at 3:37 pm on August 17, 2025: contributor

    @janb84 Yeah, the include paths used in the python test are a guess. It’d be nice to pass through the location of the capnp installation from the build system to the tests, but I don’t know how.

    I’m also looking into it to get it to work, just a heads-up that there is (possibly) some trouble on nixos/ nix-shells

  31. in test/functional/interface_ipc.py:48 in 3f7ff4849e outdated
    43+            "echo": capnp.load(str(src_dir / "ipc" / "capnp" / "echo.capnp"), imports=imports),
    44+            "mining": capnp.load(str(src_dir / "ipc" / "capnp" / "mining.capnp"), imports=imports),
    45+        }
    46+
    47+    async def make_capnp_init_ctx(self):
    48+        modules = self.capnp_modules()
    


    ismaelsadeeq commented at 3:24 pm on August 18, 2025:

    In “ci: add functional test for IPC interface” 3f7ff4849e6165a6bcc4d5977c35a467fd7fc232

    Should modules be a test parameter initialised in set_test_params so that we can later reuse in mining test?


    sipa commented at 5:17 pm on August 18, 2025:
    Good idea, done.
  32. in test/functional/interface_ipc.py:26 in 3f7ff4849e outdated
    34+    @functools.cache
    35+    def capnp_modules(self):
    36+        cpp_capnp_dir = Path(capnp.__path__[0]).parent
    37+        src_dir = Path(self.config['environment']['SRCDIR']) / "src"
    38+        mp_dir = src_dir / "ipc" / "libmultiprocess" / "include"
    39+        imports = ["/usr/include", str(cpp_capnp_dir), str(src_dir), str(mp_dir)]
    


    ismaelsadeeq commented at 3:30 pm on August 18, 2025:

    In “ci: add functional test for IPC interface” https://github.com/bitcoin/bitcoin/commit/3f7ff4849e6165a6bcc4d5977c35a467fd7fc232

    This currently does not work on macOs when the pycapnp uses system installed capnproto installed via homebrew.

    Editing it to works for me.

    0  imports = ["/opt/homebrew/opt/capnp/include", str(src_dir), str(mp_dir)]
    

    I thought of two ways to fix it but was not successful.

    1. Whether we can get the path to the include directly from pycapnp
    2. Search for the location of capnproto installation for supported platforms and add it.

    josibake commented at 3:57 pm on August 18, 2025:

    FWIW, this worked for me on macos: #33201 (review). I’d recommend we have the docs tell users to install pycapnp from source, since that should “just work” everywhere due to this line: cpp_capnp_dir = Path(capnp.__path__[0]).parent.

    Eventually, we can replace this recommendation once we have CMake automatically finding the include directory for capnproto and making the test framework aware.


    ismaelsadeeq commented at 4:09 pm on August 18, 2025:

    @josibake I did that initially but was having some errors

      0(.venv) abubakarismail@Abubakars-MacBook-Pro pycapnp % cmake --version
      1cmake version 4.1.0
      2
      3CMake suite maintained and supported by Kitware (kitware.com/cmake).
      4(.venv) abubakarismail@Abubakars-MacBook-Pro pycapnp % pip install . -C force-bundled-libcapnp=True
      5
      6Processing /Users/abubakarismail/Desktop/Work/bitcoin-dev/py-bitcoin-ipc-client/pycapnp
      7  Installing build dependencies ... done
      8  Getting requirements to build wheel ... done
      9  Preparing metadata (pyproject.toml) ... done
     10Building wheels for collected packages: pycapnp
     11  Building wheel for pycapnp (pyproject.toml) ... error
     12  error: subprocess-exited-with-error
     13  
     14  × Building wheel for pycapnp (pyproject.toml) did not run successfully.
     15   exit code: 1
     16  ╰─> [89 lines of output]
     17      running build_ext
     18      CMake Deprecation Warning at CMakeLists.txt:1 (cmake_minimum_required):
     19        Compatibility with CMake < 3.10 will be removed from a future version of
     20        CMake.
     21      
     22        Update the VERSION argument <min> value.  Or, use the <min>...<max> syntax
     23        to tell CMake that the project requires at least <min> but has been updated
     24        to work with policies introduced by <max> or earlier.
     25      
     26      
     27      CMake Error at CMakeLists.txt:2 (project):
     28        Running
     29      
     30         '/opt/homebrew/bin/ninja' '--version'
     31      
     32        failed with:
     33      
     34         Traceback (most recent call last):
     35          File "/opt/homebrew/bin/ninja", line 5, in <module>
     36            from ninja import ninja
     37      
     38        ModuleNotFoundError: No module named 'ninja'
     39      
     40      
     41      
     42      CMake Error: CMAKE_CXX_COMPILER not set, after EnableLanguage
     43      -- Configuring incomplete, errors occurred!
     44      *WARNING* no libcapnp detected or rebuild forced. Attempting to build it from source now. If you have C++ Cap'n Proto installed, it may be out of date or is not being detected. This may take a while...
     45      already have /Users/abubakarismail/Desktop/Work/bitcoin-dev/py-bitcoin-ipc-client/pycapnp/bundled/capnproto-c++
     46      Traceback (most recent call last):
     47        File "/Users/abubakarismail/Desktop/Work/bitcoin-dev/py-bitcoin-ipc-client/.venv/lib/python3.13/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 389, in <module>
     48          main()
     49          ~~~~^^
     50        File "/Users/abubakarismail/Desktop/Work/bitcoin-dev/py-bitcoin-ipc-client/.venv/lib/python3.13/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 373, in main
     51          json_out["return_val"] = hook(**hook_input["kwargs"])
     52                                   ~~~~^^^^^^^^^^^^^^^^^^^^^^^^
     53        File "/Users/abubakarismail/Desktop/Work/bitcoin-dev/py-bitcoin-ipc-client/.venv/lib/python3.13/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 280, in build_wheel
     54          return _build_backend().build_wheel(
     55                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
     56              wheel_directory, config_settings, metadata_directory
     57              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     58          )
     59          ^
     60        File "/Users/abubakarismail/Desktop/Work/bitcoin-dev/py-bitcoin-ipc-client/pycapnp/_custom_build/backend.py", line 28, in build_wheel
     61          return super().build_wheel(wheel_directory, config_settings, metadata_directory)
     62                 ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     63        File "/private/var/folders/dj/d8p8jhd172n7wnq81ryfl6rc0000gn/T/pip-build-env-xr1q9l8l/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 435, in build_wheel
     64          return _build(['bdist_wheel', '--dist-info-dir', str(metadata_directory)])
     65        File "/private/var/folders/dj/d8p8jhd172n7wnq81ryfl6rc0000gn/T/pip-build-env-xr1q9l8l/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 423, in _build
     66          return self._build_with_temp_dir(
     67                 ~~~~~~~~~~~~~~~~~~~~~~~~~^
     68              cmd,
     69              ^^^^
     70          ...<3 lines>...
     71              self._arbitrary_args(config_settings),
     72              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     73          )
     74          ^
     75        File "/private/var/folders/dj/d8p8jhd172n7wnq81ryfl6rc0000gn/T/pip-build-env-xr1q9l8l/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 404, in _build_with_temp_dir
     76          self.run_setup()
     77          ~~~~~~~~~~~~~~^^
     78        File "/Users/abubakarismail/Desktop/Work/bitcoin-dev/py-bitcoin-ipc-client/pycapnp/_custom_build/backend.py", line 22, in run_setup
     79          return super().run_setup(setup_script)
     80                 ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^
     81        File "/private/var/folders/dj/d8p8jhd172n7wnq81ryfl6rc0000gn/T/pip-build-env-xr1q9l8l/overlay/lib/python3.13/site-packages/setuptools/build_meta.py", line 317, in run_setup
     82          exec(code, locals())
     83          ~~~~^^^^^^^^^^^^^^^^
     84        File "<string>", line 212, in <module>
     85        File "/private/var/folders/dj/d8p8jhd172n7wnq81ryfl6rc0000gn/T/pip-build-env-xr1q9l8l/overlay/lib/python3.13/site-packages/setuptools/__init__.py", line 115, in setup
     86          return distutils.core.setup(**attrs)
     87                 ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^
     88        File "/private/var/folders/dj/d8p8jhd172n7wnq81ryfl6rc0000gn/T/pip-build-env-xr1q9l8l/overlay/lib/python3.13/site-packages/setuptools/_distutils/core.py", line 186, in setup
     89          return run_commands(dist)
     90        File "/private/var/folders/dj/d8p8jhd172n7wnq81ryfl6rc0000gn/T/pip-build-env-xr1q9l8l/overlay/lib/python3.13/site-packages/setuptools/_distutils/core.py", line 202, in run_commands
     91          dist.run_commands()
     92          ~~~~~~~~~~~~~~~~~^^
     93        File "/private/var/folders/dj/d8p8jhd172n7wnq81ryfl6rc0000gn/T/pip-build-env-xr1q9l8l/overlay/lib/python3.13/site-packages/setuptools/_distutils/dist.py", line 1002, in run_commands
     94          self.run_command(cmd)
     95          ~~~~~~~~~~~~~~~~^^^^^
     96        File "/private/var/folders/dj/d8p8jhd172n7wnq81ryfl6rc0000gn/T/pip-build-env-xr1q9l8l/overlay/lib/python3.13/site-packages/setuptools/dist.py", line 1102, in run_command
     97          super().run_command(command)
     98          ~~~~~~~~~~~~~~~~~~~^^^^^^^^^
     99        File "/private/var/folders/dj/d8p8jhd172n7wnq81ryfl6rc0000gn/T/pip-build-env-xr1q9l8l/overlay/lib/python3.13/site-packages/setuptools/_distutils/dist.py", line 1021, in run_command
    100          cmd_obj.run()
    101          ~~~~~~~~~~~^^
    102        File "<string>", line 169, in run
    103        File "/Users/abubakarismail/Desktop/Work/bitcoin-dev/py-bitcoin-ipc-client/pycapnp/buildutils/build.py", line 65, in build_libcapnp
    104          raise RuntimeError("CMake failed {}".format(returncode))
    105      RuntimeError: CMake failed 1
    106      [end of output]
    107  
    108  note: This error originates from a subprocess, and is likely not a problem with pip.
    109  ERROR: Failed building wheel for pycapnp
    110Failed to build pycapnp
    111
    112[notice] A new release of pip is available: 25.1.1 -> 25.2
    113[notice] To update, run: pip install --upgrade pip
    114ERROR: Failed to build installable wheels for some pyproject.toml based projects (pycapnp)
    

    So I opted into the other approach of

    pip install . which by default, the setup script will automatically use the locally installed Cap’n Proto. If Cap’n Proto is not installed, it will bundle and build the matching Cap’n Proto library.

    So it picked up my homebrew capnproto installation.


    fanquake commented at 4:14 pm on August 18, 2025:

    I did that initially but was having some errors

    It looks like the pycapnp package doesn’t currently work with Python 3.13 (https://github.com/capnproto/pycapnp/issues/372).


    ismaelsadeeq commented at 4:50 pm on August 18, 2025:

    It looks like the pycapnp package doesn’t currently work with Python 3.13 (https://github.com/capnproto/pycapnp/issues/372)

    This issue seems unrelated.

    The actual issue was that my ninja installation was not linked.

    0 brew link --overwrite ninja
    

    And then @josibake recommended documentation should work.

    Thanks @josibake for helping me debug.


    ryanofsky commented at 5:13 pm on August 18, 2025:

    re: #33201 (review)

    On nixos, I needed to change this as follows:

    0-        cpp_capnp_dir = Path(capnp.__path__[0]).parent
    1+        capnp_dir = Path(shutil.which("capnp")).parent.parent / "include"
    2         src_dir = Path(self.config['environment']['SRCDIR']) / "src"
    3         mp_dir = src_dir / "ipc" / "libmultiprocess" / "include"
    4-        imports = ["/usr/include", str(cpp_capnp_dir), str(src_dir), str(mp_dir)]
    5+        imports = [str(capnp_dir), str(src_dir), str(mp_dir)]
    

    Maybe something like this could work with homebrew too.


    sipa commented at 5:17 pm on August 18, 2025:
    I added a comment to explain that Python 3.13 doesn’t currently work.

    Sjors commented at 9:10 am on August 19, 2025:

    As of cd86ddaa1dac104415ab16cc8068fd50d068c12f the test passes for me on macOS with Python 3.10.14 with pycapnp installed via pip and capnp installed via homebrew.

    I also tested Python 3.12.11. With Python 3.13.17 pip3 install pycapnp fails. We can worry about that later.

  33. in test/functional/interface_ipc.py:114 in 3f7ff4849e outdated
    109+            # Test some inspectors of template.
    110+            header = await template.result.getBlockHeader(ctx)
    111+            assert len(header.result) == 80
    112+            block_data = BytesIO((await template.result.getBlock(ctx)).result)
    113+            block = CBlock()
    114+            block.deserialize(block_data)
    


    ismaelsadeeq commented at 3:32 pm on August 18, 2025:

    In “ci: add functional test for IPC interface” https://github.com/bitcoin/bitcoin/commit/3f7ff4849e6165a6bcc4d5977c35a467fd7fc232

    Should be abstracted away as a helper to be reused ?

    0    async def parse_and_deserialize_block(self, block_template, ctx):
    1        block_data = BytesIO((await block_template.result.getBlock(ctx)).result)
    2        block = CBlock()
    3        block.deserialize(block_data)
    4        return block
    
    0            block = await self.parse_and_deserialize_block(template, ctx)
    

    sipa commented at 5:17 pm on August 18, 2025:
    Done.
  34. in test/functional/interface_ipc.py:158 in 3f7ff4849e outdated
    137+            waitnext = template2.result.waitNext(ctx, waitoptions)
    138+            template3 = await waitnext
    139+            assert template3.to_dict() == {}
    140+            # Destroy template objects
    141+            template.result.destroy(ctx)
    142+            template2.result.destroy(ctx)
    


    ismaelsadeeq commented at 3:39 pm on August 18, 2025:

    In “ci: add functional test for IPC interface” https://github.com/bitcoin/bitcoin/commit/3f7ff4849e6165a6bcc4d5977c35a467fd7fc232

    Also add a branch for when we have a better template based on increase in transaction fees in the mempool above the fee threshold.

     0            assert_equal(len(block2.vtx), 1)
     1            # Wait for another, get one after increase in fees in the mempool.
     2            waitnext = template2.result.waitNext(ctx, waitoptions)
     3            self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1, fee_rate=10)
     4            template3 = await waitnext
     5            assert template3
     6            block3 = await self.parse_and_deserialize_block(template3, ctx)
     7            assert_equal(len(block3.vtx), 2)
     8            # Wait for another, but timeout.
     9            waitnext = template3.result.waitNext(ctx, waitoptions)
    10            template4 = await waitnext
    11            assert template4.to_dict() == {}
    12            # Destroy template objects
    13            template.result.destroy(ctx)
    14            template2.result.destroy(ctx)
    15            template3.result.destroy(ctx)
    

    sipa commented at 5:17 pm on August 18, 2025:
    Thanks, added!
  35. in test/functional/interface_ipc.py:135 in 3f7ff4849e outdated
    130+            template2 = await waitnext
    131+            assert template2
    132+            block2_data = BytesIO((await template2.result.getBlock(ctx)).result)
    133+            block2 = CBlock()
    134+            block2.deserialize(block2_data)
    135+            assert int(new_tip[0], 16) == block2.hashPrevBlock
    


    ismaelsadeeq commented at 3:39 pm on August 18, 2025:

    In “ci: add functional test for IPC interface” https://github.com/bitcoin/bitcoin/commit/3f7ff4849e6165a6bcc4d5977c35a467fd7fc232

    nit: use our internal assertion helpers for better errors?

  36. in test/functional/interface_ipc.py:79 in 3f7ff4849e outdated
    74+
    75+    def run_mining_test(self):
    76+        self.log.info("Running mining test")
    77+        modules = self.capnp_modules()
    78+        async def async_routine():
    79+            init, ctx = await self.make_capnp_init_ctx()
    


    ismaelsadeeq commented at 3:40 pm on August 18, 2025:

    In “ci: add functional test for IPC interface” https://github.com/bitcoin/bitcoin/commit/3f7ff4849e6165a6bcc4d5977c35a467fd7fc232

    Is their benefit of having context for each test, else we can just have a global context?

  37. ismaelsadeeq commented at 3:42 pm on August 18, 2025: member

    Approach ACK

    First pass initial comments

  38. in test/README.md:45 in 3f7ff4849e outdated
    40+
    41+Depending on the system, it may be necessary to install and run in a venv:
    42+
    43+- `python -m venv venv`
    44+- `venv/bin/pip3 install pycapnp`
    45+- `venv/bin/python3 test/functional/interface_ipc.py`
    


    josibake commented at 3:42 pm on August 18, 2025:

    These commands for installing via a venv dont work for me on macos or linux. I’m also not sure about installing via pip, since we need to be sure we are installing the package with capnproto bundled. Instead, I think pycapnp should be installed from source (venv or otherwise):

    0git clone https://github.com/capnproto/pycapnp.git
    1cd pycapnp
    2pip install . -C force-bundled-libcapnp=True
    

    This approach worked on macos and linux for me (inside a venv on both).


    fanquake commented at 4:08 pm on August 18, 2025:

    venv/bin/python3 test/functional/interface_ipc.py

    This should be build/test/functional/interface_ipc.py. Otherwise the test wont run.


    ismaelsadeeq commented at 4:12 pm on August 18, 2025:
    When you build using depends with the autotools it will work I think? So both options maybe?

    fanquake commented at 4:15 pm on August 18, 2025:
    Documentation being added to master shouldn’t be for Autotools.

    sipa commented at 5:18 pm on August 18, 2025:
    Cool, used this in the instructions.

    sipa commented at 5:18 pm on August 18, 2025:
    Fixed.
  39. in test/functional/interface_ipc.py:23 in 3f7ff4849e outdated
    31+        self.skip_if_no_py_capnp()
    32+        self.skip_if_no_bitcoind_ipc()
    33+
    34+    @functools.cache
    35+    def capnp_modules(self):
    36+        cpp_capnp_dir = Path(capnp.__path__[0]).parent
    


    josibake commented at 3:43 pm on August 18, 2025:
    AFAICT, this only works when capnproto is bundled inside pycapnp.

    ryanofsky commented at 5:11 pm on August 18, 2025:

    re: #33201 (review)

    AFAICT, this only works when capnproto is bundled inside pycapnp.

    Can confirm at least on nixos there are no .capnp files in this directory. Seems harmless to keep this if it helps other platforms, but would be good to document where it’s necessary.


    josibake commented at 6:25 pm on August 18, 2025:

    Sorry, didn’t mean to imply this breaks things if present, rather that adding this line works because it allows accessing the bundled version. This is why I recommended in a separate comment that our docs prompt the user to always install from source using the bundled version when installing pycapnp.

    Longer term, I am confident we can drop all of this from the functional test in favour of having CMake pipe through the correct info to the config file.


    ryanofsky commented at 7:54 pm on August 18, 2025:

    Thanks, when I saw this I wasn’t aware what a “bundled” capnproto was but agree it makes sense to use it if the python extension includes it.

    I guess I’d be curious what would happen if the docs were changed to use force-system-libcapnp=True instead of force-bundled-libcapnp=True? It seems like since the c++ code already requires capnproto to be built, maybe it’s wasteful for the instructions to recommend downloading it and rebuilding if it’s not necessary?

    Also agree it could be good to get the location of the include directory from cmake. There’s a CAPNP_INCLUDE_DIRS cmake variable that could work for this.

    I’m not sure it is necessarily better to use the cmake value because in theory there’s no reason the python client needs to use the same library as the c++ server, and maybe it’s even good to test with different versions. But getting a location directly from cmake is probably more reliable than having to derive one in python.

    If we don’t use the cmake path, the shutil.which approach suggested #33201 (review) also could be reasonable and seems to match what python setup code does internally.

    Overall anything that works here seems fine to me.

  40. josibake approved
  41. josibake commented at 3:51 pm on August 18, 2025: member

    ACK https://github.com/bitcoin/bitcoin/pull/33201/commits/3f7ff4849e6165a6bcc4d5977c35a467fd7fc232

    Thanks for picking this up! I think the docs and multiplatform support still need a bit of work but that can be tackled in a follow-up(s), considering the main benefit I see here is getting a basic functional test in place that can be extended later.

  42. DrahtBot requested review from Sjors on Aug 18, 2025
  43. DrahtBot requested review from ismaelsadeeq on Aug 18, 2025
  44. DrahtBot requested review from ryanofsky on Aug 18, 2025
  45. josibake commented at 3:53 pm on August 18, 2025: member

    the include paths used in the python test are a guess. It’d be nice to pass through the location of the capnp installation from the build system to the tests

    I took a stab at this, ended up being harder than expected and I don’t yet have something that works. But I think this is the correct longterm approach, instead of trying to guess in the functional test itself.

  46. ismaelsadeeq approved
  47. ismaelsadeeq commented at 4:51 pm on August 18, 2025: member
    ACK 3f7ff4849e6165a6bcc4d5977c35a467fd7fc232
  48. ryanofsky referenced this in commit 6d0103472f on Aug 18, 2025
  49. ryanofsky referenced this in commit c17b20d786 on Aug 18, 2025
  50. in test/functional/interface_ipc.py:28 in 3f7ff4849e outdated
    23+    def set_test_params(self):
    24+        self.num_nodes = 1
    25+        self.use_multiprocess_node = True
    26+        # The default socket path exceeds the maximum socket filename length in CI.
    27+        self.socket_path = Path(tempfile.mkdtemp()) / "ipc.sock"
    28+        self.extra_args = [[f"-ipcbind=unix:{self.socket_path}"]]
    


    ryanofsky commented at 5:09 pm on August 18, 2025:

    In commit “ci: add functional test for IPC interface” (3f7ff4849e6165a6bcc4d5977c35a467fd7fc232)

    This seems like it leaves behind an empty directory after the test runs. In any case it should be possible drop this code and simplify this commit by cherrypicking three commits from #30437:

    • c3d82ef8fa947f108502ec80c522b4bd0084dd4c test: add is_ipc_enabled() and skip_if_no_ipc() functions
    • 3af4d9a50ab008aa3974c580a831095e2ba17042 test: Provide path to bitcoin binary
    • 6d0103472f1d691e8e1a73049ba85163e103ae28 test: Add TestNode ipcbind option

    I posted a branch doing this at https://github.com/ryanofsky/bitcoin/commits/pr/ipc-py.1


    janb84 commented at 5:40 pm on August 18, 2025:
    This branch / patch works for me on Nix on macOS. Hint, do not forget to import shutil when applying the patch below.

    sipa commented at 5:41 pm on August 18, 2025:
    @ryanofsky Thanks! Will address later today.
  51. sipa force-pushed on Aug 18, 2025
  52. sipa force-pushed on Aug 18, 2025
  53. DrahtBot added the label CI failed on Aug 18, 2025
  54. DrahtBot commented at 5:20 pm on August 18, 2025: contributor

    🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/48327091424 LLM reason (✨ experimental): The CI failure is caused by errors detected in the lint test due to unused imports and variables in the Python code.

    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.

  55. fanquake commented at 6:13 pm on August 18, 2025: member

    https://github.com/bitcoin/bitcoin/actions/runs/17047602183/job/48327342413?pr=33201#step:8:3379:

     0stderr:
     1Traceback (most recent call last):
     2  File "/Users/runner/work/bitcoin/bitcoin/ci/scratch/build-aarch64-apple-darwin23.6.0/test/functional/interface_ipc.py", line 161, in <module>
     3    IPCInterfaceTest(__file__).main()
     4    ~~~~~~~~~~~~~~~~^^^^^^^^^^
     5  File "/Users/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 186, in __init__
     6    self.set_test_params()
     7    ~~~~~~~~~~~~~~~~~~~~^^
     8  File "/Users/runner/work/bitcoin/bitcoin/ci/scratch/build-aarch64-apple-darwin23.6.0/test/functional/interface_ipc.py", line 40, in set_test_params
     9    self.capnp_modules = self.load_capnp_modules()
    10                         ~~~~~~~~~~~~~~~~~~~~~~~^^
    11  File "/Users/runner/work/bitcoin/bitcoin/ci/scratch/build-aarch64-apple-darwin23.6.0/test/functional/interface_ipc.py", line 23, in load_capnp_modules
    12    cpp_capnp_dir = Path(capnp.__path__[0]).parent
    13                         ^^^^^
    14NameError: name 'capnp' is not defined
    
  56. sipa force-pushed on Aug 18, 2025
  57. sipa referenced this in commit f4ef6d2ce2 on Aug 18, 2025
  58. sipa force-pushed on Aug 18, 2025
  59. Sjors commented at 9:18 am on August 19, 2025: member

    If this PR is to be based on #31802 or #33190 then it should be marked draft. Otherwise it could be made independent by dropping 7a7f11933b83d290129377523cc4a3cfc45fe9ef and instead enabling IPC for one or more CI machines that run functional tests.

    It should be straightforward for me to rebase #31802 after this lands as long as you leave ci/test/00_setup_env_i686_multiprocess.sh alone (due to 789f7195b2acbc3e50ee75b2b5a88af058cf7b14).

    My preference would be to base it on #31802 because that gives the widest test coverage, including depends builds.

  60. in test/README.md:40 in cd86ddaa1d outdated
    33@@ -34,6 +34,26 @@ The ZMQ functional test requires a python ZMQ library. To install it:
    34 - on Unix, run `sudo apt-get install python3-zmq`
    35 - on mac OS, run `pip3 install pyzmq`
    36 
    37+The IPC functional test requires a python IPC library. `pip3 install pycapnp` may work, but if not, install it from source:
    38+
    39+```sh
    40+git clone https://github.com/capnproto/pycapnp.git`
    


    maflcko commented at 9:19 am on August 19, 2025:

    from the llm:

    0git clone https://github.com/capnproto/pycapnp.git
    

    sipa commented at 7:28 am on August 21, 2025:
    Done.
  61. in test/functional/test_framework/test_framework.py:560 in cd86ddaa1d outdated
    556@@ -550,15 +557,15 @@ def bin_dir_from_version(version):
    557 
    558             bin_dirs.append(bin_dir)
    559 
    560+        extra_init = [{}] * num_nodes if self.extra_init is None else self.extra_init
    


    maflcko commented at 9:25 am on August 19, 2025:

    Not sure why the lint task complains here:

    0[21:49:19.712] test/functional/test_framework/test_framework.py:560: error: Need type annotation for "extra_init"  [var-annotated]
    1[21:49:21.445] Found 1 error in 1 file (checked 301 source files)
    2[21:49:21.473] ^---- ⚠️ Failure generated from lint-python.py
    

    No other variable in this function is required to be annotated and the lint should probably be fixed to not require it here either.


    ryanofsky commented at 2:27 pm on August 19, 2025:
    If we wanted to annotate it, could write as extra_init: list[dict[str, Any]] = ... (with from typing import Any) but would also seem reasonable to suppress this with # type: ignore[var-annotated]

    sipa commented at 5:09 pm on August 19, 2025:
    Added the ignore annotation.
  62. in test/functional/test_framework/test_framework.py:970 in cd86ddaa1d outdated
    963@@ -964,6 +964,13 @@ def skip_if_no_py_sqlite3(self):
    964         except ImportError:
    965             raise SkipTest("sqlite3 module not available.")
    966 
    967+    def skip_if_no_py_capnp(self):
    968+        """Attempt to import the capnp package and skip the test if the import fails."""
    969+        try:
    970+            import capnp  # type: ignore[import] # noqa: F401
    


    Sjors commented at 9:29 am on August 19, 2025:

    In cd86ddaa1dac104415ab16cc8068fd50d068c12f ci: add functional test for IPC interface: is it somehow possible for import to succeed but capnp to remain undefined?

    #33201 (comment)


    sipa commented at 10:31 am on August 19, 2025:
    This was with an earlier version of the PR which called load_capnp_modules from set_test_params (which tried to do so before deciding whether pycapnp was even installed).

    Sjors commented at 11:12 am on August 19, 2025:
    Ah yes, now it’s failing with a very cryptic “Tests successful”

    sipa commented at 5:09 pm on August 19, 2025:
    Fixed, see elsewhere.
  63. in test/functional/test_framework/test_framework.py:215 in 530ca0bc57 outdated
    206@@ -207,6 +207,12 @@ def main(self):
    207         except BaseException:
    208             self.log.exception("Unexpected exception")
    209             self.success = TestStatus.FAILED
    210+        except SystemExit as e:
    211+            self.log.exception(f"System exit: {e.message}")
    212+            self.success = TestStatus.FAILED
    213+        except GeneratorExit as e:
    214+            self.log.exception(f"Generator exit: {e.message}")
    215+            self.success = TestStatus.FAILED
    


    maflcko commented at 1:46 pm on August 19, 2025:

    this is a no-op, no? The BaseException above calls self.log.exception, which fully logs the exception.

    Also, I don’t think the logs contain “unexpected exception”. In https://github.com/bitcoin/bitcoin/actions/runs/17053347607/job/48345965497#step:6:5037 the error is the stderr:

    0
    1
    2stderr:
    3[node 0] Cleaning up ipc directory '/tmp/test-ipc-xj2xyof4'
    

    sipa commented at 1:48 pm on August 19, 2025:

    See IRC:

    009:15:59 < sipa> anyone have any clue what's going on here? https://github.com/bitcoin/bitcoin/actions/runs/17053347607/job/48345965497?pr=33201
    109:16:13 < sipa> 240/272 - interface_ipc.py failed, Duration: 6 s
    209:16:18 < sipa> 2025-08-18T22:28:30.854262Z TestFramework (INFO): Tests successful
    309:28:06 < instagibbs> TIL SystemExit and GeneratorExit don't inherit Exception, so maybe SystemExit is triggering, causing the failure to not be set
    409:46:23 < sipa> instagibbs: let's see!
    

    But it seems that these two do inherit from BaseException, just not from Exception, so nevermind.


    ryanofsky commented at 2:21 pm on August 19, 2025:

    IIUC it’s failing because stderr is nonempty:

    https://github.com/bitcoin/bitcoin/blob/22e689587a7ae02f82ad2464017731f3b23b5363/test/functional/test_runner.py#L760

    because the log (raw) includes “[node 0] Cleaning up ipc directory ‘/tmp/test-ipc-xj2xyof4’” as maflcko mentioned.

    Maybe this line should be changed to use stdout instead of stderr?

    https://github.com/bitcoin/bitcoin/blob/530ca0bc57477b77f60af142695d9bf200ef6e3c/test/functional/test_framework/test_node.py#L232

    (Sorry if this is the bug)


    janb84 commented at 2:39 pm on August 19, 2025:

    Maybe this line should be changed to use stdout instead of stderr?

    https://github.com/bitcoin/bitcoin/blob/530ca0bc57477b77f60af142695d9bf200ef6e3c/test/functional/test_framework/test_node.py#L232

    (Sorry if this is the bug)

    I Could reproduce the bug locally and this change solves it for me. (stderr -> stdout)

    Without change :

     0Remaining jobs: [interface_ipc.py]
     11/1 - interface_ipc.py failed, Duration: 4 s
     2
     3stdout:
     42025-08-19T14:35:47.614991Z TestFramework (INFO): PRNG seed is: 1494337157176280909
     52025-08-19T14:35:47.615519Z TestFramework (INFO): Initializing test directory /private/tmp/nix-shell-71975-4016936104/test_runner_₿_🏃_20250819_163547/interface_ipc_0
     62025-08-19T14:35:49.166272Z TestFramework (INFO): Running echo test
     72025-08-19T14:35:49.168265Z TestFramework (INFO): Running mining test
     82025-08-19T14:35:51.247065Z TestFramework (INFO): Stopping nodes
     92025-08-19T14:35:51.411476Z TestFramework (INFO): Cleaning up /private/tmp/nix-shell-71975-4016936104/test_runner_₿_🏃_20250819_163547/interface_ipc_0 on exit
    102025-08-19T14:35:51.411598Z TestFramework (INFO): Tests successful
    11
    12
    13stderr:
    14[node 0] Cleaning up ipc directory '/private/tmp/nix-shell-71975-4016936104/test-ipc-qbv8akuk'
    15
    16TEST             | STATUS    | DURATION
    17
    18interface_ipc.py | ✖ Failed  | 4 s
    19
    20ALL              | ✖ Failed  | 4 s (accumulated)
    21Runtime: 4 s
    

    with change:

     0
     1Temporary test directory at /private/tmp/nix-shell-71975-4016936104/test_runner_₿_🏃_20250819_163804
     2Remaining jobs: [interface_ipc.py]
     31/1 - interface_ipc.py passed, Duration: 3 s
     4
     5TEST             | STATUS    | DURATION
     6
     7interface_ipc.py | ✓ Passed  | 3 s
     8
     9ALL              | ✓ Passed  | 3 s (accumulated)
    10Runtime: 3 s
    

    sipa commented at 5:08 pm on August 19, 2025:
    Changed to print to stdout, seems to work.
  64. sipa referenced this in commit 0a5734e228 on Aug 19, 2025
  65. sipa force-pushed on Aug 19, 2025
  66. in test/functional/interface_ipc.py:54 in 088dc2c486 outdated
    47+        super().setup_nodes()
    48+        # Use this function to also load the capnp modules (we cannot use set_test_params for this,
    49+        # as it is being called before knowing whether capnp is available).
    50+        self.capnp_modules = self.load_capnp_modules()
    51+
    52+    async def make_capnp_init_ctx(self):
    


    Sjors commented at 5:00 pm on August 19, 2025:
    088dc2c486af0ad6803d919d8114ab29d3cd652f: eventually I’d like to move this and load_capnp_modules into the test framework so that it’s easier to expand e.g. mining_template_verification.py to call both getblocktemplate proposal and checkBlock().

    sipa commented at 7:23 am on August 21, 2025:
    Leaving this for a follow-up.
  67. in test/functional/interface_ipc.py:150 in 088dc2c486 outdated
    145+                self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1, fee_rate=10)
    146+                template3 = await waitnext
    147+                block3 = await self.parse_and_deserialize_block(template3, ctx)
    148+                assert_equal(len(block3.vtx), 2)
    149+                last_template = template3
    150+            # Wait for another, but timeout.
    


    Sjors commented at 5:05 pm on August 19, 2025:
    088dc2c486af0ad6803d919d8114ab29d3cd652f: nit if you have to retouch: these comments are useful as self.log.debug()

    sipa commented at 7:22 am on August 21, 2025:
    Done.
  68. Sjors approved
  69. Sjors commented at 5:06 pm on August 19, 2025: member
    ACK 088dc2c486af0ad6803d919d8114ab29d3cd652f (after base PR lands)
  70. DrahtBot requested review from ismaelsadeeq on Aug 19, 2025
  71. DrahtBot requested review from josibake on Aug 19, 2025
  72. DrahtBot requested review from ryanofsky on Aug 19, 2025
  73. sipa commented at 5:07 pm on August 19, 2025: member

    Addressed a number of comments, and rebased on top of #31802.

    I will however not have (much) time to push this further forward until the end of this month. @Sjors or @ryanofsky, or someone else, feel free to take this over and/or integrate into other PRs.

  74. DrahtBot removed the label CI failed on Aug 19, 2025
  75. janb84 commented at 11:44 am on August 20, 2025: contributor

    ACK 088dc2c486af0ad6803d919d8114ab29d3cd652f (after base #31802)

    This PR adds some functional tests for the IPC interface, setups the CI to use IPC and changes build documentation to match the changes in this PR.

    Current state of the PR works on nix on macOS, builds & test run without trouble.

    • build ✅
    • tested ✅
    • code review✅

    small NIT if retouched, please see the remarks of DrahtBot’s LLM linter.

  76. in test/functional/test_framework/test_framework.py:1083 in eae32d1a9b outdated
    1079@@ -1075,6 +1080,10 @@ def is_usdt_compiled(self):
    1080         """Checks whether the USDT tracepoints were compiled."""
    1081         return self.config["components"].getboolean("ENABLE_USDT_TRACEPOINTS")
    1082 
    1083+    def is_ipc_enabled(self):
    


    ryanofsky commented at 1:25 pm on August 20, 2025:

    In commit “test: add is_ipc_enabled() and skip_if_no_ipc() functions” (eae32d1a9b5c62707de6afd9832d40de859dd42d)

    Not important, but might be better to rename enabled to compiled here to be more similar to surrounding functions which check ENABLE_XXX but are called is_xxx_compiled. (I think your PR actually did this originally but I didn’t notice the pattern it was following)


    sipa commented at 7:22 am on August 21, 2025:
    Done.
  77. in test/README.md:42 in 088dc2c486 outdated
    33@@ -34,6 +34,26 @@ The ZMQ functional test requires a python ZMQ library. To install it:
    34 - on Unix, run `sudo apt-get install python3-zmq`
    35 - on mac OS, run `pip3 install pyzmq`
    36 
    37+The IPC functional test requires a python IPC library. `pip3 install pycapnp` may work, but if not, install it from source:
    38+
    39+```sh
    40+git clone https://github.com/capnproto/pycapnp.git`
    41+cd pycapnp
    42+pip install . -C force-bundled-libcapnp=True
    


    ryanofsky commented at 1:34 pm on August 20, 2025:

    In commit “ci: add functional test for IPC interface” (088dc2c486af0ad6803d919d8114ab29d3cd652f)

    As mentioned #33201 (review), would be curious to know if force-system-libcapnp=True works for people, since it seems wasteful to download and build a dependency that we already know must be installed (or the test would be skipped).


    sipa commented at 7:22 am on August 21, 2025:
    I opted to use system libcapnp in the description here because it’s more likely to work. I’ve added a comment about it being optional now.
  78. in ci/test/00_setup_env_native_tidy.sh:13 in 088dc2c486 outdated
     9@@ -10,7 +10,8 @@ export CI_IMAGE_NAME_TAG="mirror.gcr.io/ubuntu:24.04"
    10 export CONTAINER_NAME=ci_native_tidy
    11 export TIDY_LLVM_V="20"
    12 export APT_LLVM_V="${TIDY_LLVM_V}"
    13-export PACKAGES="clang-${TIDY_LLVM_V} libclang-${TIDY_LLVM_V}-dev llvm-${TIDY_LLVM_V}-dev libomp-${TIDY_LLVM_V}-dev clang-tidy-${TIDY_LLVM_V} jq libevent-dev libboost-dev libzmq3-dev systemtap-sdt-dev qt6-base-dev qt6-tools-dev qt6-l10n-tools libqrencode-dev libsqlite3-dev libcapnp-dev capnproto"
    14+export PACKAGES="clang-${TIDY_LLVM_V} libclang-${TIDY_LLVM_V}-dev llvm-${TIDY_LLVM_V}-dev libomp-${TIDY_LLVM_V}-dev clang-tidy-${TIDY_LLVM_V} jq libevent-dev libboost-dev libzmq3-dev systemtap-sdt-dev qt6-base-dev qt6-tools-dev qt6-l10n-tools libqrencode-dev libsqlite3-dev libcapnp-dev capnproto python3-pip"
    


    ryanofsky commented at 1:37 pm on August 20, 2025:

    In commit “ci: add functional test for IPC interface” (088dc2c486af0ad6803d919d8114ab29d3cd652f)

    Not important, but for easier review and to avoid conflicts in the CI files, it may make sense to split this commit into two commits: one which adds the interface_ipc.py test and allows running it on systems with pycapnp installed, and another commit before or after that installs pycapnp in CI.


    sipa commented at 7:22 am on August 21, 2025:
    Done.
  79. in test/functional/interface_ipc.py:31 in 088dc2c486 outdated
    26+    def load_capnp_modules(self):
    27+        # If pycapnp was installed with bundled capnp, capnp/c++.capnp can be found here.
    28+        bundled_capnp_dir = Path(capnp.__path__[0]).parent
    29+        # Otherwise guess the location of capnp/c++.capnp based on the installation path of the
    30+        # capnp binary itself.
    31+        bin_capnp_dir = Path(shutil.which("capnp")).parent.parent / "include"
    


    ryanofsky commented at 1:45 pm on August 20, 2025:

    In commit “ci: add functional test for IPC interface” (088dc2c486af0ad6803d919d8114ab29d3cd652f)

    This is probably fine in practice, but it does seem possible for this to fail if the capnp binary isn’t on the path because shutil.which will return None. It might be better to write this as:

    0if capnp_bin := shutil.which("capnp"):
    1    # Add the system cap'nproto path so include/capnp/c++.capnp can be found.
    2    capnp_dir = Path(shutil.which("capnp")).parent.parent / "include"
    3else:
    4    # If there is system cap'nproto, the pycapnp module should have it's own "bundled" includes at this location
    5    capnp_dir = Path(capnp.__path__[0]).parent
    6
    7```
    

    sipa commented at 7:21 am on August 21, 2025:
    Done.
  80. in test/functional/interface_ipc.py:58 in 088dc2c486 outdated
    53+        node = self.nodes[0]
    54+        # Establish a connection, and create Init proxy object.
    55+        connection = await capnp.AsyncIoStream.create_unix_connection(node.ipc_socket_path)
    56+        client = capnp.TwoPartyClient(connection)
    57+        init = client.bootstrap().cast_as(self.capnp_modules['init'].Init)
    58+        # Create a thread, and a context object it is registered in.
    


    ryanofsky commented at 1:47 pm on August 20, 2025:

    In commit “ci: add functional test for IPC interface” (088dc2c486af0ad6803d919d8114ab29d3cd652f)

    I might describe this as # Create a remote thread on the server for the IPC calls to be executed in


    sipa commented at 7:21 am on August 21, 2025:
    Done.
  81. ryanofsky approved
  82. ryanofsky commented at 1:57 pm on August 20, 2025: contributor
    Code review ACK 088dc2c486af0ad6803d919d8114ab29d3cd652f. Looks great! Left some comments that you should feel free to ignore. I am shocked and amazed about how much more friendly the python async API is compared to the c++ API, but I guess coroutines can bring similar benefits to c++. It is also nice how this tests covers different scenarios with waitNext()
  83. DrahtBot commented at 10:35 pm on August 20, 2025: contributor
    🐙 This pull request conflicts with the target branch and needs rebase.
  84. DrahtBot added the label Needs rebase on Aug 20, 2025
  85. test: add is_ipc_compiled() and skip_if_no_ipc() functions
    Expose ENABLE_IPC build option to function tests so new tests can test IPC-only features.
    8bf4c00970
  86. test: Provide path to `bitcoin` binary
    Set new `BitcoinTestFramework.binary_paths.bitcoin_bin` property with path to
    the `bitcoin` wrapper binary. This allows new tests for `bitcoin-mine` in
    #30437 and `bitcoin-cli` in #32297 to find the `bitcoin` binary and call
    `bitcoin -m` to start nodes with IPC support. This way the new tests can run
    whenever the ENABLE_IPC build option is enabled, instead of only running when
    the `BITCOIN_CMD` environment variable is set to `bitcoin -m`
    fcf378e36b
  87. test: Add TestNode ipcbind option
    With this change, tests can specify `self.extra_init = [{ipcbind: True}]` to
    start a node listening on an IPC socket, instead of needing to choose which
    node binary to invoke and what `self.extra_args=[["-ipcbind=..."]]` value to
    pass to it.
    
    The eliminates boilerplate code #30437 (interface_ipc_mining.py), #32297
    (interface_ipc_cli.py), and #33201 (interface_ipc.py) previously needed in
    their test setup.
    61b154b1ec
  88. Sjors commented at 7:10 am on August 21, 2025: member
    @sipa let me know if you don’t have to time to rebase, in which case I’ll open a fresh PR next week.
  89. sipa force-pushed on Aug 21, 2025
  90. sipa commented at 7:21 am on August 21, 2025: member

    Rebased, and addressed some of the feedback above.

    Note that the last commit (which enables the tests in CI) may need work after moving to be on top of #31802, so that it matches the CI configurations which have IPC enabled.

  91. tests: add functional tests for IPC interface bb8a9afaf7
  92. ci: enable IPC tests in CI 03c66ee229
  93. sipa force-pushed on Aug 21, 2025
  94. DrahtBot added the label CI failed on Aug 21, 2025
  95. DrahtBot commented at 7:28 am on August 21, 2025: contributor

    🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/48558512670 LLM reason (✨ experimental): Lint check failed due to unused variable capnp_bin.

    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.

  96. in test/functional/interface_ipc.py:31 in 03c66ee229
    26+    def load_capnp_modules(self):
    27+        if capnp_bin := shutil.which("capnp"):
    28+            # Add the system cap'nproto path so include/capnp/c++.capnp can be found.
    29+            capnp_dir = Path(capnp_bin).parent.parent / "include"
    30+        else:
    31+            # If there is system cap'nproto, the pycapnp module should have it's own "bundled" includes at this location
    


    DrahtBot commented at 7:43 am on August 21, 2025:
    in test/functional/interface_ipc.py comment: “have it’s own” -> “have its own” [possessive “its,” not contraction “it’s”]
    
  97. DrahtBot removed the label CI failed on Aug 21, 2025
  98. janb84 commented at 8:59 am on August 21, 2025: contributor

    re ACK 03c66ee229393335fabae1192b871f9ec68adf74

    changes since last ACK:

    • rebase
    • addressed some NIT’s
  99. DrahtBot requested review from Sjors on Aug 21, 2025
  100. DrahtBot requested review from ryanofsky on Aug 21, 2025

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: 2025-08-21 09:12 UTC

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