multiprocess: Don't require bitcoin -m argument when IPC options are used #33229

ryanofsky · 2025-08-20T20:08:36Z

sipa and theuni in #31802 pointed out that users shouldn't be exposed to multiprocess implementation details just to use IPC features, so current need to specify the bitcoin -m option in conjunction with -ipcbind could be seen as a design mistake and not just a usage inconvenience. This is easy to fix by having the bitcoin wrapper respect IPC settings. Was planning to do this anyway, but this provides a good excuse to do it now.

Expose ENABLE_IPC build option to functional tests so new tests can test IPC-only features.

Set new `BitcoinTestFramework.binary_paths.bitcoin_bin` property with path to the `bitcoin` wrapper binary. This allows new tests for `bitcoin-mine` in bitcoin#30437 and `bitcoin-cli` in bitcoin#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`

…le to distinguish these in tests

Choose the right binary by default if an IPC option is specified

DrahtBot · 2025-08-20T20:08:42Z

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/33229.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	Sjors

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:

#33247 (build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings by 151henry151)
#33142 (test: Run bench sanity checks in parallel with functional tests by maflcko)
#32380 (Modernize use of UTF-8 in Windows code by hebasto)
#32297 (bitcoin-cli: Add -ipcconnect option by ryanofsky)
#31723 (qa debug: Add --debug_runs/-waitfordebugger [DRAFT] by hodlinator)
#31507 (build: Use clang-cl to build on Windows natively by hebasto)
#30595 (kernel: Introduce initial C header API by TheCharlatan)
#30437 (ipc: add bitcoin-mine test program by ryanofsky)
#28802 (ArgsManager: support subcommand-specific options by ajtowns)

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.

DrahtBot · 2025-08-20T21:13:48Z

🚧 At least one of the CI tasks failed.
_{Task lint: https://github.com/bitcoin/bitcoin/runs/48525913081}
_{LLM reason (✨ experimental): The CI failure is caused by lint errors related to unused imports and formatting issues in Python code.}

Hints

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.

Sjors · 2025-08-21T07:09:27Z

Concept ACK

This also makes it easier to document instructions for miners, they just need to drop the "d" from bitcoind and then -ipcbind behaves like any other setting.

The help text should explain that -m is assumed if any IPC arguments are passed.

Sjors · 2025-09-01T13:18:37Z

The following patches fix the linter:

iff --git a/src/bitcoin.cpp b/src/bitcoin.cpp
index c1a5fce33a..aa74f3e54e 100644
--- a/src/bitcoin.cpp
+++ b/src/bitcoin.cpp
@@ -166,7 +166,7 @@ bool UseMultiprocess(const CommandLine& cmd)
 
     // If any -ipc* options are set these need to be processed by a
     // multiprocess-capable binary.
-    return args.IsArgSet("-ipcbind") || args.IsArgSet("-ipcconnect") || args.IsArgSet("-ipcfd");
+    return args.IsArgSet("-ipcbind");
 }
 
 //! Execute the specified bitcoind, bitcoin-qt or other command line in `args`
diff --git a/test/functional/tool_bitcoin.py b/test/functional/tool_bitcoin.py
index 7ceb6c85b3..a6cce2339a 100755
--- a/test/functional/tool_bitcoin.py
+++ b/test/functional/tool_bitcoin.py
@@ -3,7 +3,6 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test the IPC  interface."""
-from pathlib import Path
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     append_config,
@@ -63,7 +62,7 @@ class ToolBitcoinTest(BitcoinTestFramework):
         self.test_args([], ["-ipcbind=unix"], expect_exe="bitcoin-node")
 
         self.log.info("Ensure bitcoin recognizes -ipcbind in config file")
-        append_config(node.datadir_path, [f"ipcbind=unix"])
+        append_config(node.datadir_path, ["ipcbind=unix"])
         self.test_args([], [], expect_exe="bitcoin-node")

As for the macOS error I can't reproduce:

2025-08-20T20:26:26.298984Z TestFramework (INFO): Ensure bitcoin -m invokes bitcoin-node
2025-08-20T20:26:26.315700Z TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
  File "/Users/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 195, in main
    self.run_test()
    ~~~~~~~~~~~~~^^
  File "/Users/runner/work/bitcoin/bitcoin/ci/scratch/build-aarch64-apple-darwin23.6.0/test/functional/tool_bitcoin.py", line 54, in run_test
    self.test_args(["-m"], [], expect_exe="bitcoin-node")
    ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/runner/work/bitcoin/bitcoin/ci/scratch/build-aarch64-apple-darwin23.6.0/test/functional/tool_bitcoin.py", line 41, in test_args
    assert_equal(get_exe_name(out), expect_exe.encode())
                 ~~~~~~~~~~~~^^^^^
  File "/Users/runner/work/bitcoin/bitcoin/ci/scratch/build-aarch64-apple-darwin23.6.0/test/functional/tool_bitcoin.py", line 90, in get_exe_name
    return re.match(rb".*\s(\S+)\s*(?:\n|$)", version_str).group(1)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'group'

However you might need the same workaround for long -ipcbind paths as in https://github.com/bitcoin/bitcoin/pull/30437/files#diff-7f22a082e3a80ca2f212e36193f87dd80237035afedb7f15b116ef7fa265f3eeR139

Sjors · 2025-09-01T13:17:40Z

src/bitcoin.cpp

+
+    // If any -ipc* options are set these need to be processed by a
+    // multiprocess-capable binary.
+    return args.IsArgSet("-ipcbind") || args.IsArgSet("-ipcconnect") || args.IsArgSet("-ipcfd");


5a28d0b: did you mean to add -ipcconnect and -ipcfd here?

re: #33229 (comment)

5a28d0b: did you mean to add -ipcconnect and -ipcfd here?

Yeah they are not necessary but listed to be more future-proof. It makes sense for any IPC options to choose the IPC binary.

I asked because it triggered a missing documentation error.

Sjors · 2025-09-01T13:29:06Z

src/init/bitcoin-qt.cpp

@@ -16,6 +16,8 @@

 namespace init {
 namespace {
+const char* EXE_NAME = "bitcoin-node";


231fba1: I'm confused, should this be bitcoin-qt?

I don't see it appear when I run build/bin/bitcoin gui --version

re: #33229 (comment)

Good catch, changed to bitcoin-qt. Also good to know text doesn't appear in gui --version output but I think that is ok as long as the test is only calling bitcoin node. I do think it's probably good for the Init::exeName method to return the right name for consistency even it's not called right now.

Sjors · 2025-09-01T13:30:03Z

test/functional/tool_bitcoin.py

+            assert_equal(err, b"")
+
+    def run_test(self):
+        self.log.info("Ensure bitcoin command invokes bitcoind by default")


bitcoin node command?

re: #33229 (comment)

bitcoin node command?

Thanks, updated

RuntimeError: Unexpected output from ['D:\\a\\bitcoin\\bitcoin\\bin\\bitcoin.exe', 'node', '-datadir=D:\\a\\_temp\\test_runner_₿_🏃_20250904_040915\\tool_bitcoin_29\\node0', '-logtimemicros', '-debug', '-debugexclude=libevent', '-debugexclude=leveldb', '-debugexclude=rand', '-uacomment=testnode0', '-disablewallet', '-logthreadnames', '-logsourcelocations', '-loglevel=trace', '-nologratelimit', '-v2transport=0', '-version']: out=b'' err=b'Assertion failed: base.is_absolute(), file ./util/fs.cpp, line 38\r\n' ret=3 Windows native, VS 2022 https://github.com/bitcoin/bitcoin/actions/runs/17452836196/job/49560783679?pr=33229 Windows, test cross-built https://github.com/bitcoin/bitcoin/actions/runs/17452836196/job/49561115662?pr=33229

gmake: *** No rule to make target 'deploy'. Stop. https://github.com/bitcoin/bitcoin/actions/runs/17464984349/job/49598412758?pr=33229 https://github.com/bitcoin/bitcoin/actions/runs/17464984349/job/49598400710?pr=33229 AssertionError: Unexpected stderr @@@@

@@@@ Cached path '' @@@@ Not directory path 'D:\a\_temp\test_runner_?_??_20250904_143739\tool_bitcoin_0\node0' Assertion failed: base.is_absolute(), file D:\a\bitcoin\bitcoin\src\util\fs.cpp, line 38 https://github.com/bitcoin/bitcoin/actions/runs/17466846483/job/49604704419?pr=33229 also try to fix linux cross>windows build error No target "bitcoin-qt" https://github.com/bitcoin/bitcoin/actions/runs/17466846483/job/49604716292?pr=33229

unicode now working https://github.com/bitcoin/bitcoin/actions/runs/17468061915/job/49608942805?pr=33229 /bin/makensis -V2 /home/admin/actions-runner/_work/_temp/build/bitcoin-win64-setup.nsi File: "/home/admin/actions-runner/_work/_temp/build/release/bitcoin-qt.exe" -> no files found. Usage: File [/nonfatal] [/a] ([/r] [/x filespec [...]] filespec [...] | /oname=outfile one_file_only) Error in script "/home/admin/actions-runner/_work/_temp/build/bitcoin-win64-setup.nsi" on line 75 -- aborting creation process gmake[3]: *** [CMakeFiles/deploy.dir/build.make:81: bitcoin-win64-setup.exe] Error 1 gmake[3]: Leaving directory '/home/admin/actions-runner/_work/_temp/build' https://github.com/bitcoin/bitcoin/actions/runs/17468061915/job/49608953626?pr=33229

empty output https://github.com/bitcoin/bitcoin/actions/runs/17469484758/job/49613880399?pr=33229 skipped test? https://github.com/bitcoin/bitcoin/actions/runs/17469484758/job/49614698195?pr=33229

…ctly still empty output native build https://github.com/bitcoin/bitcoin/actions/runs/17470812198/job/49618470699?pr=33229 still skipped test cross build https://github.com/bitcoin/bitcoin/actions/runs/17470812198/job/49619359209?pr=33229

still empty output native build https://github.com/bitcoin/bitcoin/actions/runs/17471664334/job/49621358922?pr=33229 still skipped test cross build https://github.com/bitcoin/bitcoin/actions/runs/17471664334/job/49622036064?pr=33229

Sjors · 2025-09-05T06:46:31Z

test/functional/test_framework/test_node.py

@@ -446,7 +446,7 @@ def is_node_stopped(self, *, expected_stderr="", expected_ret_code=0):
        self.stderr.seek(0)
        stderr = self.stderr.read().decode('utf-8').strip()
        if stderr != expected_stderr:
-            raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))
+            print("Unexpected stderr {} != {}".format(stderr, expected_stderr))


No access to self.logger here?

No access to self.logger here?

Yeah sorry, this is just a workaround for debugging. This test is just not working on windows and I was trying to debug it using CI to avoid needing to setup a windows build and vm and see if the issue happens there.

So far I've fixed one issue where unicode path CI uses was not properly handled by bitcoin wrapper on windows, but there is another issue where stdout/stderr process of the bitcoind do not seem to be captured by python when it is invoked through bitcoin node. Have more things I want to try to debug this and some ideas for fixing this, so will continue to use this PR to debug a little while, but if it doesn't work out I'll just disable the new test on windows.

try writing to file and changing return code no output from wrapped bitcoind https://github.com/bitcoin/bitcoin/actions/runs/17473672184/job/49628086738?pr=33229 https://github.com/bitcoin/bitcoin/actions/runs/17473672184/job/49628785697?pr=33229

https://github.com/bitcoin/bitcoin/actions/runs/17504927856/job/49726787209?pr=33229 https://github.com/bitcoin/bitcoin/actions/runs/17504927856/job/49726350015?pr=33229

ryanofsky added 5 commits August 20, 2025 04:30

test: add is_ipc_compiled() and skip_if_no_ipc() functions

21df18f

Expose ENABLE_IPC build option to functional tests so new tests can test IPC-only features.

init: add exe name to bitcoind, bitcoin-node -version output to be ab…

e99b775

…le to distinguish these in tests

test: add tool_bitcoin to test bitcoin wrapper behavior

82c2f5c

bitcoin: Make wrapper not require -m

b0e9637

Choose the right binary by default if an IPC option is specified

DrahtBot added the interfaces label Aug 20, 2025

ryanofsky mentioned this pull request Aug 20, 2025

Add bitcoin-{node,gui} to release binaries for IPC #31802

Merged

DrahtBot added the CI failed label Aug 20, 2025

ryanofsky mentioned this pull request Aug 21, 2025

RFC: Multiprocess binaries and packaging options #30983

Open

This was referenced Aug 21, 2025

test: Run bench sanity checks in parallel with functional tests #33142

Open

bitcoin-cli: Add -ipcconnect option #32297

Open

ArgsManager: support subcommand-specific options #28802

Open

Multiprocess bitcoin #10102

Draft

Sjors reviewed Sep 1, 2025

View reviewed changes

ryanofsky mentioned this pull request Aug 15, 2025

Multiprocess tracking issue #28722

Open

89 tasks

ryanofsky force-pushed the pr/ipc-auto branch 2 times, most recently from f325839 to b0e9637 Compare September 4, 2025 03:55

Merge remote-tracking branch 'origin/master' into pr/ipc-auto

fc3fbb5

ryanofsky force-pushed the pr/ipc-auto branch 6 times, most recently from da1ef87 to 9ebbbcb Compare September 4, 2025 13:03

ryanofsky force-pushed the pr/ipc-auto branch 2 times, most recently from 3b3c925 to 0e189dc Compare September 4, 2025 13:09

ryanofsky force-pushed the pr/ipc-auto branch from 0e189dc to f3faaa0 Compare September 4, 2025 13:11

ryanofsky added 5 commits September 4, 2025 10:13

try to debug empty output

51ebaab

empty output https://github.com/bitcoin/bitcoin/actions/runs/17469484758/job/49613880399?pr=33229 skipped test? https://github.com/bitcoin/bitcoin/actions/runs/17469484758/job/49614698195?pr=33229

DrahtBot mentioned this pull request Sep 4, 2025

build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings #33247

Open

This was referenced Sep 5, 2025

build: Use clang-cl to build on Windows natively #31507

Open

kernel: Introduce initial C header API #30595

Open

ipc: add bitcoin-mine test program #30437

Open

Sjors reviewed Sep 5, 2025

View reviewed changes

ryanofsky added 2 commits September 5, 2025 17:28

environment variable being ignored? try to avoid segfault

0f0bf7f

https://github.com/bitcoin/bitcoin/actions/runs/17504927856/job/49726787209?pr=33229 https://github.com/bitcoin/bitcoin/actions/runs/17504927856/job/49726350015?pr=33229

This was referenced Sep 6, 2025

Modernize use of UTF-8 in Windows code #32380

Open

qa debug: Add --debug_runs/-waitfordebugger [DRAFT] #31723

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

multiprocess: Don't require bitcoin -m argument when IPC options are used #33229

multiprocess: Don't require bitcoin -m argument when IPC options are used #33229

ryanofsky commented Aug 20, 2025

Uh oh!

DrahtBot commented Aug 20, 2025 •

edited

Loading

Uh oh!

DrahtBot commented Aug 20, 2025

Uh oh!

Sjors commented Aug 21, 2025

Uh oh!

Sjors commented Sep 1, 2025 •

edited

Loading

Uh oh!

Sjors Sep 1, 2025

Uh oh!

ryanofsky Sep 3, 2025

Uh oh!

Sjors Sep 5, 2025

Uh oh!

Sjors Sep 1, 2025

Uh oh!

ryanofsky Sep 3, 2025

Uh oh!

Sjors Sep 1, 2025

Uh oh!

ryanofsky Sep 3, 2025

Uh oh!

Sjors Sep 5, 2025

Uh oh!

ryanofsky Sep 5, 2025

Uh oh!

Uh oh!

multiprocess: Don't require bitcoin -m argument when IPC options are used #33229

Are you sure you want to change the base?

multiprocess: Don't require bitcoin -m argument when IPC options are used #33229

Conversation

ryanofsky commented Aug 20, 2025

Uh oh!

DrahtBot commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

DrahtBot commented Aug 20, 2025

Uh oh!

Sjors commented Aug 21, 2025

Uh oh!

Sjors commented Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DrahtBot commented Aug 20, 2025 •

edited

Loading

Sjors commented Sep 1, 2025 •

edited

Loading