Sjors
commented at 4:04 pm on February 5, 2025:
member
Have depends make libmultiprocess by default. This PR causes the following behavior changes:
bitcoin-node and bitcoin-gui binaries are included in releases, due to ENABLE_IPC option being switched on by default in depends builds
ENABLE_IPC is also switched on by default in non-depends builds (instructions updated, #33190 does this as a standalone PR)
Various changes to CI: switching on ENABLE_IPC on in most configurations and using bitcoin-node binary (bitcoin -m) for functional tests in two of them.
The bitcoin-node and bitcoin-gui are added to Maintenance.cmake (since they’re now in the release)
This PR doesn’t need to do all of these things at once. However it’s is simpler, avoids code churn (especially in CI), and probably less confusing to make all these changes in the same PR.
Windows is not supported yet, so ENABLE_IPC is off by default for it. It can be enabled after #32387.
The initial main use case for IPC is to enable experimental support for the Mining IPC interface. A working example of a Stratum v2 Template Provider client using this interface can be found here: https://github.com/Sjors/bitcoin/pull/48.
See #31756 for discussion of when this should happen. Supersedes #30975.
Wait what, why?
The Stratum v2 spec has been around for a few years now, mostly stable but with ongoing activity to clarify and fix more subtle issues encountered by implementers. Most of the implementation is built in Rust in a project called the Stratum Reference Implementation (SRI).
Braiins added Stratum v2 support to both their (custom) firmware and pool several years ago, though they have fallen behind on recent spec changes (update: it seems they’ve fixed that). Apparently new hardware is underway that supports Stratum v2 without the need for custom firmware.
DMND pool is Stratum v2 native from the start and employs several of the SRI developers (they haven’t fully launched though). The industry is rather secretive, but apparently there is more underway.
What does Bitcoin Core have to do with this? Well, in Stratum v2 jargon we are the Template Provider.
Or at least, the Template Provider role needs us to make block templates. Initially back in 2023 the plan was to have Bitcoin Core implement this role entirely, see #23049. It would speak the sv2 encrypted message protocol. In fact the spec was designed around this assumption, making sure to only use cryptographic primitives already in our codebase.
I took over that effort in late 2023, but during 2024 it became quite clear there was strong resistance to the idea of including all this new code, opening another network ports, etc.
At the same time there was the long running multiprocess / IPC project #10102, and the idea was born to apply that here: instead of including Stratum v2 specific stuff, we offer a general Mining interface via an IPC connection that can e.g. push out fresh block templates as fees rise above a threshold (something not possible and/or very inefficient with getblocktemplate). A client sidecar application then sits between the Stratum v2 world and our node.
Currently there’s only one such sidecar application, maintained by me, and reusing the same codebase from the integrated approach. An attempt has been made to connect to our interface from Rust https://github.com/bitcoin-core/libmultiprocess/issues/174, which would pave the way for SRI include the Template Provider role. Plebhash below indicates he’s also working on that: #31802 (comment).
So with this new approach in mind, between mid 2024 until spring 2025, I introduced a new Mining interface (#30200 - #31785). At the same time Russ Ryanosky worked on more tight integration of libmultiprocess, including making it a subtree in #31741. See design/multiprocess.md.
one recurring feedback I kept getting regardless of the size/type of miner is that the need to run a forked version of Bitcoin Core remains a significant barrier to adoption
This PR gets rids of that significant barrier. People can download a “pristine” version of Bitcoin Core and the only change is to start it with bitcoin node -m -ipcconnect=unix instead of the usual bitcoind.
Once that’s released, I can dramatically simplify my sidecar codebase (https://github.com/Sjors/bitcoin/pull/48) by removing pretty much all Bitcoin Core code that it doesn’t need. My plan is to then make that a separate repository, which should be much easier to contribute to. I can then also make (deterministically built) signed releases, while making it clear that sidecar code has nothing to do with Bitcoin Core. Perhaps later on SRI implements the same and I can stop maintaining that project.
Conceptually the situation will be a lot clearer;
today: download forked version of bitcoind (or a forked version of bitcoin-node, plus bitcoin-mine), install SRI stuff
tomorrow: download Bitcoin Core v30, install bitcoin-mine and SRI
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:
#33201 (Add functional test for IPC interface by sipa)
#32989 (ci: Migrate CI to hosted Cirrus Runners by willcl-ark)
#32162 (depends: Switch from multilib to platform-specific toolchains by hebasto)
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 added the label
Needs rebase
on Feb 6, 2025
Sjors force-pushed
on Feb 7, 2025
Sjors force-pushed
on Feb 7, 2025
sipa
commented at 9:03 pm on February 7, 2025:
member
Some chatter from IRC:
017:21:41 < darosior> It might be confusing to release both a bitcoin-wallet utility and a bitcoin-wallet binary as part of multiprocess?
117:24:08 < darosior> We could rename the utility, but then it would be nice to at least have one deprecation cycle. Given recent momentum i estimate it's possible we might release multiprocess in
2 30.0, which means if we want to deprecate the bitcoin-wallet utility name we should do it.. now?
317:33:31 < sipa> bitcoin-wallet-util ?
4...
523:38:57 < _aj_> darosior: bitcoin-wallet-process, bitcoin-gui-process, etc? it's multi *-process!
603:04:34 < Sjors[m]> darosior: at the moment there is no wallet binary if were to enable multiprocess. That won't happen until [#19460](/bitcoin-bitcoin/19460/).
7...
803:05:18 < Sjors[m]> Or maybe already in [#10102](/bitcoin-bitcoin/10102/)
9...
1003:05:50 < Sjors[m]> In any case [#31802](/bitcoin-bitcoin/31802/) only adds bitcoin-node and bitcoin-gui.
11...
1203:07:19 < Sjors[m]> Though if we want to rename the utility eventually, it's always better to do it early.
Sjors
commented at 10:58 am on February 8, 2025:
member
DrahtBot removed the label
Needs rebase
on Feb 13, 2025
Sjors force-pushed
on Feb 17, 2025
Sjors force-pushed
on Feb 17, 2025
DrahtBot added the label
Needs rebase
on Feb 20, 2025
ryanofsky referenced this in commit
83e40d3b52
on Feb 24, 2025
ryanofsky referenced this in commit
8619f03ec2
on Feb 24, 2025
ryanofsky referenced this in commit
cbb7b41c20
on Feb 24, 2025
fanquake referenced this in commit
01f7715766
on Feb 25, 2025
fanquake referenced this in commit
ba0a4391ff
on Feb 25, 2025
pablomartin4btc referenced this in commit
bd2453d134
on Feb 26, 2025
pablomartin4btc referenced this in commit
75d5d235a6
on Feb 26, 2025
Sjors force-pushed
on Feb 27, 2025
DrahtBot removed the label
Needs rebase
on Feb 27, 2025
ngn999 referenced this in commit
fd12bfb763
on Feb 27, 2025
DrahtBot added the label
Needs rebase
on Mar 5, 2025
Sjors force-pushed
on Mar 11, 2025
DrahtBot removed the label
Needs rebase
on Mar 11, 2025
DrahtBot added the label
Needs rebase
on Mar 12, 2025
Sjors force-pushed
on Mar 12, 2025
DrahtBot removed the label
Needs rebase
on Mar 12, 2025
DrahtBot added the label
Needs rebase
on Mar 14, 2025
Sjors force-pushed
on Mar 17, 2025
DrahtBot removed the label
Needs rebase
on Mar 17, 2025
DrahtBot added the label
Needs rebase
on Mar 18, 2025
Sjors force-pushed
on Mar 18, 2025
DrahtBot removed the label
Needs rebase
on Mar 18, 2025
DrahtBot added the label
Needs rebase
on Mar 29, 2025
Sjors force-pushed
on Mar 31, 2025
DrahtBot removed the label
Needs rebase
on Mar 31, 2025
Sjors force-pushed
on Mar 31, 2025
DrahtBot added the label
Needs rebase
on Apr 2, 2025
Sjors force-pushed
on Apr 2, 2025
DrahtBot removed the label
Needs rebase
on Apr 2, 2025
DrahtBot added the label
Needs rebase
on Apr 4, 2025
Sjors
commented at 4:32 pm on April 10, 2025:
member
As discussed in IRC, I’ll turn on IPC in the default cmake build (not just the dev preset). Either here on in the previous PR.
ryanofsky
commented at 5:53 pm on April 10, 2025:
contributor
As discussed in IRC, I’ll turn on IPC in the default cmake build (not just the dev preset). Either here on in the previous PR.
In case it helps I actually have an old commit implementing this: a6d9db48249d5d9bd63ea0d99e15066a1299e5c3 (branch) since I was originally thinking of including it in #31741
Sjors force-pushed
on Apr 11, 2025
DrahtBot removed the label
Needs rebase
on Apr 11, 2025
Sjors marked this as ready for review
on Apr 11, 2025
Sjors
commented at 7:32 pm on April 11, 2025:
member
Marked this as ready for review, but see discussion here: #30975 (comment)
ryanofsky
commented at 8:49 pm on April 11, 2025:
contributor
It seems like there are some MULTIPROCESS=1 settings added earlier in this PR that are not removed later and become ignored, so these would be good to clean up
It could be good to simplify this PR and avoid churn by moving the two CI commits last (if they are still needed) after changing the cmake and depends ENABLE_IPC defaults from true to false.
It could make sense to rename the NO_MULTIPROCESS depends option to NO_IPC to be consistent with the cmake ENABLE_IPC option. There are some other depends/cmake options that seem to share the same names like NO_WALLET/ENABLE_WALLET, NO_ZMQ/WITH_ZMQ, NO_USDT/WITH_USDT, etc
Sjors force-pushed
on Apr 11, 2025
Sjors
commented at 9:58 pm on April 11, 2025:
member
I moved the ci commits last and removed the unneeded stuff. So this PR is now independent of #30975, although it overlaps.
Hopefully didn’t break anything…
Does anyone have a strong preference which CI job(s) should build without multiprocess? Right now the Windows and one i686 cirrus job are.
Any which CI job(s) should use bitcoin-node in the functional tests? I picked CentOS (depends) and macOS native (no depends).
Except for Windows, these choices are fairly arbitrary.
Sjors force-pushed
on Apr 11, 2025
DrahtBot added the label
Needs rebase
on Apr 14, 2025
Sjors force-pushed
on Apr 15, 2025
DrahtBot removed the label
Needs rebase
on Apr 15, 2025
in
depends/README.md:121
in
9b46b7d582outdated
117@@ -118,7 +118,7 @@ The following can be set when running make: `make FOO=bar`
118 - `NO_WALLET`: Don't download/build/cache libs needed to enable the wallet (SQLite)
119 - `NO_BDB`: Don't download/build/cache BerkeleyDB
120 - `NO_USDT`: Don't download/build/cache packages needed for enabling USDT tracepoints
121-- `MULTIPROCESS`: Build libmultiprocess (experimental)
122+- `NO_IPC`: Don't build experimental libmultiprocess (default on Windows and OpenBSD)
In commit “build: depends makes libmultiprocess by default” (9b46b7d582edcf61bbec5b01c8e661af804e22d6)
Think it would be good if this description described the full effects of this option. Would maybe say “NO_IPC: don’t build Cap’n Proto and libmultiprocess packages, and disable building IPC binaries (bitcoin-node, bitcoin-gui)”
Also, since this is the most significant commit in the PR, which controls whether IPC binaries are included in releases, it would be good to mention the effect this has on releases in the commit description.
Commit description could also mention effect this has on CI builds: it cause more depends builds to build IPC binaries, but still the only build running functional tests with them is the i686_multiprocess one.
ryanofsky
commented at 5:35 pm on April 15, 2025:
contributor
Code review ACK5d96e656d72abee468f5f49759b1b08eab445d4d. Changes seem pretty clean and simple now. I think it might be good for PR to clearly mention the 3 changes in behavior it causes:
1 - Causes bitcoin-node and bitcoin-gui binaries to be included in releases and ENABLE_IPC option to be switched on by default in depends builds.
2 - Also causes ENABLE_IPC option to be switched on by default in both non-depends builds
3 - Makes various changes to CI: switching on ENABLE_IPC on in most configurations and using bitcoin-node binary for functional tests in two of them.
It would also be good to point out that that this PR doesn’t need to do all of 3 things at once. It is simpler and probably less confusing to make all these changes in the same PR, but if we want to isolate any of these things and make them separate, that’s possible.
All the choices you made about CI jobs options seem reasonable, and seem to test everything we would want. But @maflcko has best understanding of CI jobs and might have input.
DrahtBot added the label
Needs rebase
on Apr 17, 2025
Sjors force-pushed
on Apr 17, 2025
Sjors
commented at 12:42 pm on April 17, 2025:
member
Rebased after #32271, updated PR description, commit description and comment as suggested here: #31802 (review)
Sjors force-pushed
on Apr 17, 2025
DrahtBot added the label
CI failed
on Apr 17, 2025
in
doc/multiprocess.md:19
in
341b2a99e6outdated
15@@ -16,11 +16,11 @@ Specifying `-DENABLE_IPC=ON` requires [Cap'n Proto](https://capnproto.org/) to b
1617 ### Depends installation
1819-Alternately the [depends system](../depends) can be used to avoid need to install local dependencies. A simple way to get started is to pass the `MULTIPROCESS=1` [dependency option](../depends#dependency-options) to make:
20+Alternatively the [depends system](../depends) can be used to avoid needing to local install dependencies:
Alternatively the depends system can be used to avoid needing to local install dependencies:
—> …avoid needing to install local dependencies:
DrahtBot removed the label
Needs rebase
on Apr 17, 2025
DrahtBot removed the label
CI failed
on Apr 17, 2025
Sjors
commented at 7:32 am on April 18, 2025:
member
This or a followup needs to update doc/dependencies.md and update the build instructions to change opt-in wording to opt-out. I’ll update that here if #32293 (comment) lands earlier.
ryanofsky approved
ryanofsky
commented at 6:12 pm on April 21, 2025:
contributor
Code review ACK341b2a99e646246a9e4d3aa283b2b59442cfad27. Only change since last review is making various improvements to comments and rebasing.
Would be nice to update doc/dependencies.md here since #32293 is no longer doing that but not important.
Sjors force-pushed
on Apr 23, 2025
Sjors
commented at 8:19 am on April 23, 2025:
member
Rebased after #32293, updated the documentation it introduced, and added Cap’n Proto to doc/dependencies.md. Also took the LLM fix.
in
doc/dependencies.md:39
in
ce415e06b1outdated
35@@ -36,3 +36,4 @@ Bitcoin Core requires one of the following compilers.
36 | [SQLite](../depends/packages/sqlite.mk) (wallet) | [link](https://sqlite.org) | [3.38.5](https://github.com/bitcoin/bitcoin/pull/25378) | [3.7.17](https://github.com/bitcoin/bitcoin/pull/19077) | No |
37 | Python (scripts, tests) | [link](https://www.python.org) | N/A | [3.10](https://github.com/bitcoin/bitcoin/pull/30527) | No |
38 | [systemtap](../depends/packages/systemtap.mk) ([tracing](tracing.md)) | [link](https://sourceware.org/systemtap/) | [4.8](https://github.com/bitcoin/bitcoin/pull/26945)| N/A | No |
39+| [Cap'n Proto](../depends/packages/capnp.mk) | [link](https://capnproto.org) | [0.6.1](https://capnproto.org/install.html) | 0.5.3 | Yes |
Fixed. @TheCharlatan why was this set to Yes in the original?
TheCharlatan
commented at 8:23 pm on April 29, 2025:
Fixed. @TheCharlatan why was this set to Yes in the original?
It did not make that much sense to include since it was not part of the release binaries yet, but I thought it could be considered a runtime dependency in case it is not linked statically through depends or some other method.
Sjors force-pushed
on Apr 23, 2025
in
doc/build-osx.md:132
in
1eedff84d7outdated
133-Skip if you do not need IPC functionality.
134-
135-```bash
136-brew install capnp
137-```
138+The depedency on `capnp` can be avoided by compiling without IPC-enabled binaries
Sjors
commented at 9:59 am on April 23, 2025:
member
An alternative approach could be to make Cap’n Proto optional and have -DENABLE_IPC default to ON only if its present. Though I vaguely recall that with CMake we wanted to move away from enabling config options depending on whether a library is found.
Sjors force-pushed
on Apr 23, 2025
TheCharlatan
commented at 8:25 pm on April 29, 2025:
contributor
Concept ACK
An alternative approach could be to make Cap’n Proto optional and have -DENABLE_IPC default to ON only if its present. Though I vaguely recall that with CMake we wanted to move away from enabling config options depending on whether a library is found.
Yeah, this is intentionally not done anymore since the cmake migration. I think the current approach is fine.
DrahtBot added the label
Needs rebase
on May 6, 2025
DrahtBot removed the label
Needs rebase
on May 7, 2025
DrahtBot added the label
CI failed
on May 7, 2025
DrahtBot
commented at 10:38 am on May 7, 2025:
contributor
🚧 At least one of the CI tasks failed.
Task tidy: https://github.com/bitcoin/bitcoin/runs/41784006413
LLM reason (✨ experimental): The CI failed due to clang-tidy errors related to incorrect use of std::move and thread_local.
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
commented at 11:34 am on May 7, 2025:
member
0/ci_container_base/src/ipc/libmultiprocess/include/mp/proxy-io.h:252:16: error: forwarding reference passed to std::move(), which may unexpectedly cause lvalues to be moved; use std::forward() instead [bugprone-move-forwarding-reference,-warnings-as-errors]
1[05:29:23.932] 252 | m_fn = std::move(fn);
2[05:29:23.932] | ^~~~~~~~~
3[05:29:23.932] | std::forward<Fn>
DrahtBot added the label
Needs rebase
on May 7, 2025
Although #31679 is orthogonal, it’s also worth reviewing. There should be no impact in user experience, because thanks to #31375 the recommended invocation bitcoin -m will find bitcoin-node whether it’s in /usr/local/bin or /usr/local/libexec. But an upgrade would leave a stale version of bitcoin-node in bin, so it’s nicer to get that PR in the same release as this one. Merge order shouldn’t matter.
Sjors marked this as ready for review
on May 30, 2025
TheCharlatan
commented at 1:26 pm on May 30, 2025:
contributor
0/ci_container_base/src/ipc/libmultiprocess/include/mp/proxy-io.h:252:16: error: forwarding reference passed to std::move(), which may unexpectedly cause lvalues to be moved; use std::forward() instead [bugprone-move-forwarding-reference,-warnings-as-errors]
1[05:29:23.932] 252 | m_fn = std::move(fn);
2[05:29:23.932] | ^~~~~~~~~
3[05:29:23.932] | std::forward<Fn>
My apologies for missing this in d4bc5639829f0c98bd2faeddba9be4e8c6667875. We should treat the libmultiprocess subtree the same as all other subtrees and disable the EXPORT_COMPILE_COMMANDS property for all its targets.
We should treat the libmultiprocess subtree the same as all other subtrees and disable the EXPORT_COMPILE_COMMANDS property for all its targets.
This could be reasonable, though I’m not very sure what the benefits of disabling linting here would be. Also, I think even with EXPORT_COMPILE_COMMANDS disabled, errors similar to #31802 (comment) could still happen because that error comes from a header file included via -I, not -isystem
DrahtBot requested review from TheCharlatan
on Jun 2, 2025
fanquake
commented at 4:02 pm on June 5, 2025:
member
Sjors
commented at 5:10 pm on June 11, 2025:
member
Rebased after #32690 and dropped the OpenBSD exception.
I ran the functional test suite for this PR on my OpenBSD vm.
Sjors force-pushed
on Jun 12, 2025
in
depends/README.md:83
in
5f21cb59a9outdated
79@@ -80,7 +80,7 @@ The following can be set when running make: `make FOO=bar`
80 - `NO_ZMQ`: Don't download/build/cache packages needed for enabling ZeroMQ
81 - `NO_WALLET`: Don't download/build/cache libs needed to enable the wallet (SQLite)
82 - `NO_USDT`: Don't download/build/cache packages needed for enabling USDT tracepoints
83-- `MULTIPROCESS`: Build libmultiprocess (experimental)
84+- `NO_IPC`: Don't build Cap’n Proto and libmultiprocess packages, and disable building IPC binaries (bitcoin-node, bitcoin-gui). Default on Windows.
It seems unavoidable to change ci in the same commit.
Previous iterations were changing BUILD_CONFIG, hence the formatting change. But I’ll just drop them now.
Sjors force-pushed
on Jun 12, 2025
DrahtBot added the label
CI failed
on Jun 12, 2025
DrahtBot
commented at 12:05 pm on June 12, 2025:
contributor
🚧 At least one of the CI tasks failed.
Task tidy: https://github.com/bitcoin/bitcoin/runs/43960756779
LLM reason (✨ experimental): The CI failure is caused by errors in clang-tidy checks related to deprecated header includes, which are treated as fatal errors.
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
commented at 2:37 pm on June 12, 2025:
member
0 node0 2025-06-12T12:34:42.336592Z (mocktime: 2025-06-12T12:34:46Z)[net][net.cpp:2041][InactivityCheck][net] version handshake timeout, disconnecting peer=0 1 test 2025-06-12T12:34:43.290505Z TestFramework.utils (ERROR): wait_until() failed. Predicate: '''' 2 def test_function():
3if check_connected:
4 assert self.is_connected
5return test_function_in() 6'''
7 node0 2025-06-12T12:34:44.168910Z (mocktime: 2025-06-12T12:34:46Z) [net] [net.cpp:2041] [InactivityCheck] [net] version handshake timeout, disconnecting peer=1
8 node0 2025-06-12T12:34:44.168933Z (mocktime: 2025-06-12T12:34:46Z) [net] [net.cpp:2041] [InactivityCheck] [net] version handshake timeout, disconnecting peer=2
9 node0 2025-06-12T12:34:44.168948Z (mocktime: 2025-06-12T12:34:46Z) [net] [net.cpp:558] [CloseSocketDisconnect] [net] Resetting socket for peer=0
10 test 2025-06-12T12:34:44.169006Z TestFramework (ERROR): Assertion failed
11 Traceback (most recent call last):
12 File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 190, in main
13 self.run_test()
14 File "/home/runner/work/bitcoin/bitcoin/build/test/functional/p2p_timeouts.py", line 100, in run_test
15 no_verack_node.wait_for_disconnect(timeout=1)
16 File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/p2p.py", line 609, in wait_for_disconnect
17 self.wait_until(test_function, timeout=timeout, check_connected=False)
18 File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/p2p.py", line 601, in wait_until
19 wait_until_helper_internal(test_function, timeout=timeout, lock=p2p_lock, timeout_factor=self.timeout_factor, check_interval=check_interval)
20 File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/util.py", line 321, in wait_until_helper_internal
21 raise AssertionError("Predicate {} not true after {} seconds".format(predicate_source, timeout))
22 AssertionError: Predicate ''''
23 def test_function():
24 if check_connected:
25 assert self.is_connected
26 return test_function_in()
27 ''' not true after 1 seconds
28 node0 2025-06-12T12:34:44.169056Z (mocktime: 2025-06-12T12:34:46Z)[net][net.cpp:558][CloseSocketDisconnect][net] Resetting socket forpeer=1
DrahtBot removed the label
CI failed
on Jun 13, 2025
Sjors
commented at 11:30 am on June 16, 2025:
member
Added the commit from #32760 to see if bumping capnp (depends) reveals any issues. Marking draft, will rebase after it lands.
I tested this with OpenBSD (x86, as well as aarch64 with 4f10a57671c19cacca630b2401e42a213aacff1b from #32716).
Sjors marked this as a draft
on Jun 16, 2025
ryanofsky approved
ryanofsky
commented at 1:46 pm on June 16, 2025:
contributor
Code review ACK7ecf6bbd84f200dab1ba7cbcae893edc4ed0d581. Since last review rebased, enabled openbsd support by updating capnp, renamed variable and dropped whitespace changes, and avoided c includes to fix tidy modernize-deprecated-headers errors
DrahtBot added the label
CI failed
on Jun 16, 2025
Sjors force-pushed
on Jun 17, 2025
Sjors marked this as ready for review
on Jun 17, 2025
Sjors
commented at 7:31 am on June 17, 2025:
member
Rebased after #32760 (only change is dropping that commit).
DrahtBot removed the label
CI failed
on Jun 17, 2025
ryanofsky approved
ryanofsky
commented at 10:34 am on June 17, 2025:
contributor
Code review ACK5a66729744db797e40f375d62aaebba66884ea6f. No change other than rebase and dropping base commit
vasild approved
vasild
commented at 10:18 am on June 19, 2025:
contributor
ACK5a66729744db797e40f375d62aaebba66884ea6f
Since you are renaming “multiprocess” to “ipc” in some places and you are also renaming 00_setup_env_i686_multiprocess.sh to 00_setup_env_i686_no_multiprocess.sh maybe 00_setup_env_i686_no_ipc.sh would be a better new name of the file?
The last commit refactor: modernize deprecated ipc headers is fine, but I am not sure why it is needed in this PR since those (old) .h includes were not introduced in this PR. Is there not the same problem in master? Or maybe tidy did not look at files in src/ipc/ before? If yes, then maybe that commit should be first, before the one that makes tidy check src/ipc/, so that all intermediate commits are good.
maflcko
commented at 11:16 am on June 19, 2025:
member
The last commit refactor: modernize deprecated ipc headers is fine, but I am not sure why it is needed in this PR since those (old) .h includes were not introduced in this PR. Is there not the same problem in master? Or maybe tidy did not look at files in src/ipc/ before? If yes, then maybe that commit should be first, before the one that makes tidy check src/ipc/, so that all intermediate commits are good.
If you want it to be bisect-nice, I’d guess you’d have to set -DENABLE_IPC=OFF in the tidy CI in the second to last commit and then enable it in the last commit. However I haven’t tested this and I don’t think anyone is bisecting the CI on non-merge commits, so anything should be fine here.
Sjors
commented at 12:21 pm on June 19, 2025:
member
I don’t think anyone is bisecting the CI on non-merge commits
Indeed that’s why I didn’t move it up initially. With this many CI changes it’s hard to be bisect friendly to that level anyway.
@vasild if I need to retouch I’ll move that commit up and rename some more multiprocess things to ipc.
fanquake
commented at 12:53 pm on June 19, 2025:
member
This PR needs release notes.
Note that the Tidy job here is passing, even though it’s generating output (which also seems like a bug?). That should be addressed, to either fix any issues or suppress any false-positives; otherwise it’s going to make the tidy output unusable. i.e (https://cirrus-ci.com/task/6552721135763456):
/usr/include/kj/async-inl.h:609:37: warning: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
Pretty sure this isn’t a bug. Changing [&] to [] should fix it:
/ci_container_base/src/ipc/libmultiprocess/include/mp/proxy-types.h:134:5: warning: Address of stack memory associated with temporary object of type '(lambda at /ci_container_base/src/ipc/libmultiprocess/include/mp/proxy-types.h:134:51)' is still referred to by a temporary object on the stack upon returning to the caller. This will be a dangling reference [clang-analyzer-core.StackAddressEscape]
This might be a test bug:
/ci_container_base/src/ipc/libmultiprocess/include/mp/type-number.h:55:32: warning: The value '0' provided to the cast expression is not in the valid range of values for 'FooEnum' [clang-analyzer-optin.core.EnumCastOutOfRange]
Marking as ready for review, because the tidy fix doesn’t seem blocking to me - now that a fix is being worked on. Happy to rebase after it lands though.
Sjors marked this as ready for review
on Jun 23, 2025
in
doc/release-notes-31802.md:1
in
314b4282e6outdated
0@@ -0,0 +1 @@
1+- Two new binaries are added to the release: `bitcoin-node` (or `bitcoin -m node`) and `bitcoin-gui` (or `bitcoin -m gui`). These enable IPC functionality, initially only for the (experimental) Mining interface. Building the binaries is controlled by `ENABLE_IPC` (enabled by default, except for Windows). When building from source without depends, `capnp` needs to be installed. (#31802)
In commit “doc: add release note” (314b4282e6b741e38f914246557da5787598aa63)
This seems ok but may want to rephrase it at some point (#31679 could be a good place) to be more about the new IPC feature (-ipcbind) than the new binaries. The binaries are kind of an implementation detail, and there should be no need to use them directly if the bitcoin -m is used.
Yea, these need fleshing out, because they don’t give much useful information to users, and will likely leave them with questions: “What’s IPC?” (never defined), “What’s bitcoin-node; should I use that instead of bitcoind?” (same for the GUI), “Why is this enabled by default if it’s for some experimental feature I (and basically all other users) don’t use?”.
I hope the release notes in #31679 (https://github.com/ryanofsky/bitcoin/blob/pr/libexec/doc/release-notes-31375-31679.md) can be a good starting point for a unified description of all the changes. I think all that might be needed there is a paragraph mentioning that the new bitcoin command has a multiprocess-m option enabling features which allow separate processes to communicate with each other. The only feature currently enabled by -m is an -ipcbind option allowing external tools to connect to the node and efficiently generate block templates for mining. But in the future -m may also allow different Bitcoin Core components (such as the wallet, node, and gui) to run in separate processes and be managed independently. The -m option might also be enabled by default in a future release.
ryanofsky approved
ryanofsky
commented at 1:13 pm on June 23, 2025:
contributor
Code review ACK314b4282e6b741e38f914246557da5787598aa63. Just added release notes and renamed CI job since last review.
I tried to include the changes in bitcoin-core/libmultiprocess#184 to see if they resolve all tidy issues, but that requires additional changes on the Bitcoin Core side (will report on that PR).
DrahtBot requested review from vasild
on Jun 23, 2025
ryanofsky referenced this in commit
980ab41ef8
on Jun 23, 2025
ryanofsky referenced this in commit
97f8435ca9
on Jun 23, 2025
ryanofsky referenced this in commit
5049b81657
on Jun 23, 2025
TheCharlatan
commented at 9:42 pm on June 23, 2025:
contributor
Tangentially related to the topic here, have we discussed adding the capnp files to the releases somewhere yet? I’d be interested what the thoughts are around that. I think at least for the mining interface, where some thought has been spent on how to make it usable for an external project this would make sense. The alternative of having to regularly copy them over to the target project seems less ideal to me.
ryanofsky referenced this in commit
53c532e798
on Jun 25, 2025
ryanofsky referenced this in commit
3e6bcf18e5
on Jun 25, 2025
ryanofsky referenced this in commit
d19ee2cf66
on Jun 25, 2025
ryanofsky referenced this in commit
b7ae258b5e
on Jun 25, 2025
ryanofsky referenced this in commit
26d6232f84
on Jun 25, 2025
ryanofsky referenced this in commit
661caf7c1f
on Jun 25, 2025
ryanofsky referenced this in commit
7abe89cc16
on Jun 25, 2025
ryanofsky referenced this in commit
c00938c333
on Jun 25, 2025
ryanofsky referenced this in commit
72ca2472aa
on Jun 25, 2025
ryanofsky referenced this in commit
060a739269
on Jun 25, 2025
ryanofsky referenced this in commit
4896e7fe51
on Jun 25, 2025
ryanofsky referenced this in commit
76313450c2
on Jun 25, 2025
ryanofsky referenced this in commit
8954cc0377
on Jun 26, 2025
DrahtBot added the label
Needs rebase
on Jul 3, 2025
Sjors force-pushed
on Jul 3, 2025
Sjors
commented at 10:12 am on July 3, 2025:
member
Rebased after trivial conflict with #32290 (it touches ci/test/00_setup_env_i686_multiprocess.sh which this PR renames).
DrahtBot removed the label
Needs rebase
on Jul 3, 2025
ryanofsky referenced this in commit
1e9cf763fb
on Jul 3, 2025
l0rinc referenced this in commit
2b3fa59a85
on Jul 4, 2025
ryanofsky referenced this in commit
91d6e146bb
on Jul 4, 2025
ryanofsky referenced this in commit
4d1bd04326
on Jul 4, 2025
ryanofsky referenced this in commit
cd97905ebc
on Jul 7, 2025
BrandonOdiwuor
commented at 1:59 pm on July 23, 2025:
contributor
In 28af4d727b3eeff3da0a3dfab0260c57fb52111c: This should be squashed into the commit that actually changes the Guix build. Ideally CMake would warn/error here, at least in the Guix build. We don’t want it to continue silently if binaries don’t exist/are missing from these checks.
in
.github/workflows/ci.yml:88
in
969a9472a4outdated
Which renaming do you mean? MULTIPROCESS is replaced with NO_IPC but it’s not really a rename. I could rename MULTIPROCESS to IPC in a separate PR, but I don’t think that would reduce the scope of this PR noticeably.
in
depends/Makefile:42
in
d4f18bb9deoutdated
38@@ -39,7 +39,8 @@ NO_QR ?=
39 NO_WALLET ?=
40 NO_ZMQ ?=
41 NO_USDT ?=
42-MULTIPROCESS ?=
43+# Default NO_IPC value is 1 on Windows
In commit “build: add bitcoin-{node,gui} to Maintenance.cmake” (362e233da72b89723cdb9f4f56342716b60976eb)
Note: Since this PR was opened, full windows support (for both connecting to unix sockets and spawning subprocesses) was implemented in #32387. #32387 is a draft just because I don’t want it to be a review burden while there are other changes like #32345 that I think are more important.
ryanofsky approved
ryanofsky
commented at 3:04 pm on July 24, 2025:
contributor
Code review ACK082b416a1bcbfbbdcafb3a3eb1ae800c93385203. Since last review: squashed commit, updated release notes and reverted ci.yml reformatting.
Maybe it’s just me, but I feel like this PR is more comprehensible now that the squashed depends commit is first. And the new release notes are also much better. (Though I would still like to unify them later with the bitcoin wrapper & libexec notes as suggested #31802 (review)).
I could rename MULTIPROCESS to IPC in a separate PR, but I don’t think that would reduce the scope of this PR noticeably.
Tend to agree, but maybe you could rename MULTIPROCESS to NO_IPC in a separate PR and have it default to 1? But I’m not sure much that would reduce complexity here either. I feel like most of the the complexity in this PR comes from updating the CI jobs, so maybe if we had a separate PR enabling IPC in CI jobs first, and then this PR could just flip the depends and cmake defaults and be really simple. But conceptually it seems nice if release & developer & CI defaults are all in sync, so I tend to like the PR in its current form. But would be happy with any approach.
DrahtBot removed the label
Needs rebase
on Jul 24, 2025
Sjors
commented at 3:22 pm on July 24, 2025:
member
so maybe if we had a separate PR enabling IPC in CI jobs first, and then this PR could just flip the depends
That’s what I did before and people didn’t like that: #30975#pullrequestreview-2761133797
In commit “build: depends makes libmultiprocess by default” (d4f18bb9de76d2caf41b6db05c494b333008a3f5)
Notes to help explain changes:
Variable
Description
$(ipc_packages)
Constant list of packages needed to build with IPC features, defined in depends/packages/packages.mk. Always set regardless of NO_IPC.
$(ipc_packages_)
Same as $(ipc_packages), except empty if NO_IPC is set.
@ipc_packages@
Same as $(ipc_packages_), used in toolchain.cmake.in
Depends and toolchain code checks whether IPC is enabled by checking to see if $(ipc_packages_) and @ipc_packages@ values are non-empty, instead of checking NO_IPC directly.
ryanofsky
commented at 1:08 pm on July 29, 2025:
contributor
I’m a little worried this PR is at risk of missing the next release (#32275), and might be caught in a trap where people don’t want to spend time reviewing the code because the level of conceptual support (discussed in #31756) is unclear, and no one is giving conceptual feedback because there are no ACKs for the code. Having review could be a way out of this.
in
doc/release-notes-31802.md:3
in
082b416a1boutdated
0@@ -0,0 +1,3 @@
1+A new (experimental) Mining interface was introduced to support Stratum v2 or other mining client software, see #31098. When the node is started with `bitcoin node -m -ipcbind=unix` the node will listen on a unix socket for IPC client connections. The `-m` option launches a new binary `bitcoin-node` instead of `bitcoind`.
2+
3+IPC connectivity introduces new dependencies (see [multiprocess.md](multiprocess.md)), which are only included in the `bitcoin-node` and `bitcoin-gui` binaries (not in `bitcoind` and `bitcoin-qt`). Those new binaries are now built by default (controlled by `ENABLE_IPC`). If you don't intend to use IPC, nothing changes and there is no need to use `bitcoin -m`, `bitcoin-node` or `bitcoin-gui`(#31802)
ismaelsadeeq
commented at 1:52 pm on August 1, 2025:
nitty-nit: when you click the link to see the multiprocess docs on github on “display rich diff” it is broken, however this is trivial and will go away once merged.
in
doc/release-notes-31802.md:1
in
082b416a1boutdated
0@@ -0,0 +1,3 @@
1+A new (experimental) Mining interface was introduced to support Stratum v2 or other mining client software, see #31098. When the node is started with `bitcoin node -m -ipcbind=unix` the node will listen on a unix socket for IPC client connections. The `-m` option launches a new binary `bitcoin-node` instead of `bitcoind`.
ismaelsadeeq
commented at 1:57 pm on August 1, 2025:
Nit: perhaps just mention that we have two new binaries and then begin the note as done currently..
Tiny note mining interface addition is not done in this PR however still mentioning it in this release is okay.
It might be helpful to clarify what we mean by “experimental” in this context. i.e I will prefer us being explicit for example, we could mention that the binary has undergone rounds of review and testing by a group of core contributors, but since this is its first release, users should still proceed with caution.
Personally, I’m fully on board with labeling reviewed and tested binaries as “experimental” for initial releases. If things go well over the next few release cycle, the label can be dropped.
This should not necessarily be done in this PR, the release notes can be edited later.
ryanofsky
commented at 10:41 pm on August 12, 2025:
fanquake referenced this in commit
2bb06bcaf2
on Aug 7, 2025
ngn999 referenced this in commit
4d2a116ec2
on Aug 8, 2025
fanquake
commented at 10:40 am on August 12, 2025:
member
See #33176 for an issue where the IPC compile hangs indefinitely.
The PR description says macOS and Linux installation instructions are updated to install capnp by default, however that isn’t part of this PR (and should to be).
Sjors force-pushed
on Aug 12, 2025
Sjors
commented at 1:25 pm on August 12, 2025:
member
Rebased after #31679 (for clarity, there was no conflict).
The PR description says macOS and Linux installation instructions are updated to install capnp by default, however that isn’t part of this PR (and should to be).
Not sure if you meant to say “should be” or “shouldn’t have to be”?
It seems better if depends and non-depends have the same defaults. At the same time it may be useful to reduce the scope of this PR a bit, and updating the docs involves quite a bit of churn. So I’ve gone back to only enabling IPC for --preset dev-mode.
Sjors force-pushed
on Aug 12, 2025
fanquake
commented at 1:32 pm on August 12, 2025:
member
Not sure if you meant to say “should be”
I meant “should be”. We shouldn’t change the build system defaults, without simultaneously changing the documentation; otherwise anyone following it, will see a configuration error by default.
Sjors
commented at 1:36 pm on August 12, 2025:
member
Oh ok, maybe it’s better if I open a followup PR that enables it by default in the regular build and updates these docs.
fanquake
commented at 1:48 pm on August 12, 2025:
member
It seems better if depends and non-depends have the same defaults. At the same time it may be useful to reduce the scope of this PR a bit, and updating the docs involves quite a bit of churn. So I’ve gone back to only enabling IPC for –preset dev-mode.
Concept NACK on the current push (ae13d587ab2639c6ab02472cf9b6c6185c2a525f). I don’t think we should start shipping release binaries, that are different from what is, by default, compiled by the build system, or tested by developers, or compiled by end users.
Sjors force-pushed
on Aug 12, 2025
Sjors
commented at 2:42 pm on August 12, 2025:
member
Alright, I switched back to -ENABLE_IPC=ON by default and added a commit with developer instructions.
That commit also updates the wallet instructions, since it’s analogous: both the wallet and IPC are required (in the sense that cmake will complain if it can’t find the dependencies) but trivial to opt-out of with -ENABLE_WALLET=OFF and -ENABLE_IPC=OFF.
Leaving the wallet changes out would make things hard to read, but to potentially reduce churn here I also PR’d the wallet changes separately in #33179.
That said:
different from what is, by default, compiled by the build system, or tested by developers, or compiled by end users
I think that’s a good general rule, but not something we do consistently. E.g. we don’t build the gui and ZMQ by default either, and sqlite was only recently made non-optional.
I documented the minimum capnp version to be 1.2.0 but based on #33176 it seems some distros are way behind (0.8.0?). If those are covered by CI then I guess it’s fine to lower the minimum version? I’m worried though that we update CI and forget to bump the minimum. The current audience for this feature can use modern operating systems, so I’d rather not bother supporting older versions.
fanquake
commented at 4:46 pm on August 12, 2025:
member
and sqlite was only recently made non-optional.
sqlite is still an optional dependency.
I documented the minimum capnp version to be 1.2.0
Capnp 1.2.0 was tagged ~2 months ago, which means it’s not available in the majority of distros we support. That means a default cmake -B, with system installed libraries, and the new ENABLE_IPC default here, wouldn’t work (with the build logic updated). Also, if you are going to set a minimum requirement, you should change the build system to enforce it (which would cause pretty much all the CIs here to fail). However, it’s not clear why you’d create a minimum here, without a reason, given libmultiprocess doesn’t require any particular version: https://github.com/bitcoin-core/libmultiprocess/blob/b4120d34bad2de28141c5770f6e8df8e54898987/CMakeLists.txt#L15.
so I’d rather not bother supporting older versions.
If you’re going to enable this by default, it needs to work and be tested everywhere that we currently expect bitcoind to work.
ryanofsky
commented at 5:09 pm on August 12, 2025:
contributor
However, it’s not clear why you’d create a minium here, without a reason, given libmultiprocess doesn’t require any particular version
Seems reasonable to specify 1.0.1 as a minimum version since many CI jobs in this PR are using ubuntu-24.04 and it looks like it uses that version.
The issue reported #33176 shows that current version of libmultiprocess doesn’t currently support version 0.8.0, though there’s a workaround which I’d like to add in a libmultiprocess PR.
libmultiprocess doesn’t specify a minimum version itself, but I think it could with some CI jobs there to verify compatibility. My understanding has been that it doesn’t any special capnproto features and worked with old versions back to 0.5.0.
If you’re going to enable this by default, it needs to work and be tested everywhere that we currently expect bitcoind to work.
That sounds good to me and I think this PR should be doing that, but you are right to call out the problem with specifying 1.2.0 as a minimum version so thanks for catching that.
ryanofsky
commented at 5:17 pm on August 12, 2025:
contributor
Maybe it would be good to add a cmake check to detect lib capnproto version is less than 1.0.1 (which I think is oldest version tested in CI) and default ENABLE_IPC to off or trigger an error if it is on, or both.
I think we could specify a version before 1.0.1 as the minimum, but there should be CI jobs either here or in libmultiprocess repo testing whatever minimum version is specified.
Sjors
commented at 5:24 pm on August 12, 2025:
member
I’ll try 1.0.1 as the minimum, will update the doc. I opened a PR to libmultiprocess for the version check.
Sjors force-pushed
on Aug 12, 2025
fanquake
commented at 6:40 pm on August 12, 2025:
member
I’ll try 1.0.1 as the minimum
That would prevent compilation using system packages, on Debian Bookworm: https://packages.debian.org/bookworm/capnproto, which was the current stable Debian release, up until 3 days ago.
ryanofsky
commented at 7:02 pm on August 12, 2025:
contributor
That would prevent compilation using system packages, on Debian Bookworm: https://packages.debian.org/bookworm/capnproto, which was the current stable Debian release, up until 3 days ago.
Thanks, 0.9.2 should be fine too. I’m working on adding a libmultiprocess CI job that tests versions back to 0.7.0 0.5.0 (Edit: 0.7.0 is lowest version supported since https://github.com/bitcoin-core/libmultiprocess/pull/88. Probably not worth supporting older versions even though it would not be hard.)
Sjors force-pushed
on Aug 12, 2025
Sjors
commented at 7:17 pm on August 12, 2025:
member
in
doc/release-notes-31802.md:3
in
625d4c9661outdated
0@@ -0,0 +1,3 @@
1+A new (experimental) Mining interface was introduced to support Stratum v2 or other mining client software, see #31098. When the node is started with `bitcoin node -m -ipcbind=unix` the node will listen on a unix socket for IPC client connections. The `-m` option launches a new binary `bitcoin-node` instead of `bitcoind`.
2+
3+IPC connectivity introduces new dependencies (see [multiprocess.md](multiprocess.md)), which are only included in the `bitcoin-node` and `bitcoin-gui` binaries (not in `bitcoind` and `bitcoin-qt`). Those new binaries are now built by with default (controlled by `ENABLE_IPC`). If you don't intend to use IPC, nothing changes and there is no need to use `bitcoin -m`, `bitcoin-node` or `bitcoin-gui`(#31802)
ryanofsky
commented at 10:31 pm on August 12, 2025:
In commit “doc: add release note” (9d6be22d8fc6a8e19d6b8b525eedcd7b3f4c875c)
s/by with default/by default/
in
ci/test/00_setup_env_mac_native.sh:18
in
90f9ec3e94outdated
ryanofsky
commented at 10:46 pm on August 12, 2025:
In commit “cmake: set ENABLE_IPC by default” (90f9ec3e945ae730ea16467c6729d22c56040802)
Not important but I think commit message might have gotten truncated in a recent push. Previously (eb5813a3f00a60594e25a94a69726f7d45c4aea2) it mentioned enabling bitcoin-node in the macos job and installing capnp in the other jobs.
It’s a bit confusing because the default build will fail if it’s missing, but it’s indeed not required. Sqlite is also listed under optional, so I’ll move it there.
I dropped those columns, they were in the original by @TheCharlatan but maybe we changed the layout?
ryanofsky approved
ryanofsky
commented at 11:02 pm on August 12, 2025:
contributor
Code review ACK9d6be22d8fc6a8e19d6b8b525eedcd7b3f4c875c. Since last review this was rebased and new commit was added updating the build-* files and dependencies.md file. The build instruction changes all seem nice and I think make the files more readable. I suggested some more documentation fixes below but they should be not be blocking.
Sjors
commented at 9:26 am on August 13, 2025:
member
Maybe list Cap’n Proto at the bottom of the table instead of top since it’s only currently needed for the IPC mining feature, which I think we’d expect to be less commonly used than things like the wallet, gui, zmq
But since CI spuriously failed, I might as well force push and move it up again…
ryanofsky approved
ryanofsky
commented at 10:55 am on August 13, 2025:
contributor
Code review ACK0d179d39bcb05743d605c0bc5b50a61a0a5aaee9. Just rebased, mentioned netbsd package, tweaked dependencies.md since last review
DrahtBot requested review from ismaelsadeeq
on Aug 13, 2025
Sjors force-pushed
on Aug 13, 2025
Sjors force-pushed
on Aug 13, 2025
achow101 referenced this in commit
e17b5da0d6
on Aug 14, 2025
build: depends makes libmultiprocess by default
This causes IPC binaries (bitcoin-node, bitcoin-gui) to be included
in releases.
The effect on CI is that this causes more depends builds to build IPC
binaries, but still the only build running functional tests with them
is the i686_multiprocess one.
Except for Windows.
16bce9ac4c
ci: build one depends job without multiprocessb333cc14d5
ci: use bitcoin-node for one depends job
The bitcoin-node binary is built on all platforms which have
multiprocess enabled, but for functional tests it's only used in
CentOS native (depends) job. The next commit will also add a
non-depends job.
32a90e1b90
cmake: set ENABLE_IPC by default
Install capnp on non-depends CI jobs.
Use the bitcoin-node binary in the macOS native non-depends job.
3cbf747c32
doc: update build and dependencies docs for IPC
OpenBSD does not have this package, so recommend building from
source for now.
71f29d4fa9
doc: add release notece7d94a492
Sjors force-pushed
on Aug 14, 2025
Sjors
commented at 7:02 pm on August 14, 2025:
member
Rebased after #33179, which shortens the documentation diff in 71f29d4fa90aaeb6472b3ce9d4f4e97f85ed487b.
TheBlueMatt
commented at 7:23 pm on August 14, 2025:
contributor
i’m told this is the main step towards getting IPC available in release binaries which will then, in turn, enable external mining processes (such as Sv2). With today’s Proto announcement of nice mining hardware that’s going to support Sv2 out of the box, and some other advancements I can’t talk about, I think getting robust Sv2 mining logic available without patches to Bitcoin Core in the next few months is important, and waiting another release might materially delay improvements in mining decentralization.
ryanofsky
commented at 1:57 am on August 15, 2025:
contributor
Code review ACKce7d94a492e61f2a43ea315e75be607d6aa71702. This was just rebased to fix a conflict since last review.
Previous reviewers may want to re-ack. The PR still looks the same as it did 6 months ago, just with better documentation.
This PR was also discussed again in today’s IRC meeting so feedback on that would be welcome here or in issue #31756 (to discuss the concept and code changes separately).
pavlenex
commented at 4:49 am on August 15, 2025:
none
Hi everyone, I usually avoid commenting on repos I am not directly contributing to, but I wanted to share some context on the SV2 adoption update as it may be important for the context of this PR.
Over the past few months, my main work on the SV2 project has been engaging with miners, mining pools, and hardware manufacturers.
Stratum V2 is close to reaching a snowball effect in adoption and having this into version 30.0 would help push that adoption. As @TheBlueMatt noted above, mining is a highly competitive industry, so I don’t want to bring up specific names of the companies/farms I’ve interacted with, but one recurring feedback I kept getting regardless of the size/type of miner is that the need to run a forked version of Bitcoin Core remains a significant barrier to adoption.
Small miners for example, are busy trying to keep their ASICS online 24/7 they really don’t have engineering capacity or knowledge to put into reviewing our own forked version of core to use SV2, they prefer a plug and play experience.
Removing a requirement of running a “random fork” to use SV2 would help accelerate SV2 adoption across the ecosystem and delaying that for a few months may harm the momentum.
If there’s something the SRI (Stratum V2 Reference implementation) FOSS community could do to help,, just let us know.
dergoegge
commented at 8:56 am on August 15, 2025:
member
The initial main use case for IPC is to enable experimental support for the Mining IPC interface. A working example of a Stratum v2 Template Provider client using this interface can be found here: https://github.com/Sjors/bitcoin/pull/48.
Are there any production ready clients like this available that don’t build on top of Bitcoin Core? I was under the impression that “put everything into Bitcoin Core” won’t be the direction we’re taking. Or if it is, then that still means that they’ll have to run some “random fork” until bitcoin-mine, sv2 server, etc is merged?
ismaelsadeeq
commented at 9:48 am on August 15, 2025:
member
ACKce7d94a492e61f2a43ea315e75be607d6aa71702 and tested again on macOS by building via depends and source.
sjors made an update that remove building -DENABLE_IPC=ON by default on source build and leaving it only available via the cmake preset builds which fanquake n ack .
That change was reverted and hence I think fanquake n ack does not stand as is.
More documentation improvements were added.
It will be nice if #32345 were to be merged along with this in the same release, but not a blocker for me.
ryanofsky
commented at 11:05 am on August 15, 2025:
contributor
I was under the impression that “put everything into Bitcoin Core” won’t be the direction we’re taking. Or if it is, then that still means that they’ll have to run some “random fork” until bitcoin-mine, sv2 server, etc is merged?
If this PR is merged before 30.0, sv2 software will be able to connect to the node via the IPC interface, eliminating the need to run a fork of Bitcoin Core. My understanding is that running third-party software is not a major barrier to sv2 adoption, but requiring a forked version of Bitcoin Core instead of the official release is a significant barrier.
plebhash
commented at 1:49 pm on August 15, 2025:
none
The initial main use case for IPC is to enable experimental support for the Mining IPC interface. A working example of a Stratum v2 Template Provider client using this interface can be found here: Sjors#48.
Are there any production ready clients like this available that don’t build on top of Bitcoin Core? I was under the impression that “put everything into Bitcoin Core” won’t be the direction we’re taking. Or if it is, then that still means that they’ll have to run some “random fork” until bitcoin-mine, sv2 server, etc is merged?
The key strategic point here is simply having the mining IPC interface available on Bitcoin Core, nothing more than that.
We are planning to write some Rust libs that will allow SRI tooling connect to Bitcoin Core over IPC.
We do not plan to push this to be part of the Bitcoin Core codebase, and bitcoin-mine or sv2 server being merged into Bitcoin Core is not really part of any plan that I’m aware of.
josibake
commented at 2:34 pm on August 15, 2025:
member
I would prefer if #32345 was merged before this, but I wouldn’t consider it a blocker since the main benefit I see for having this in v30 is to encourage more testing. If there is buy-in to include this in v30, I think the release notes could use a bit more work, but I’ll wait to review/give feedback on the release notes since I don’t think the release notes should be a blocker to merging this PR.
Sjors
commented at 4:26 pm on August 15, 2025:
member
Are there any production ready clients like this available that don’t build on top of Bitcoin Core? I was under the impression that “put everything into Bitcoin Core” won’t be the direction we’re taking. Or if it is, then that still means that they’ll have to run some “random fork” until bitcoin-mine, sv2 server, etc is merged?
My plan is to strip all unused Bitcoin Core stuff out of the bitcoin-mine binary in https://github.com/Sjors/bitcoin/pull/48 and turn it into a standalone repo (it uses the demo in #30437 as a scaffold, but that PR doesn’t have to land for this).
So it wouldn’t be a “random fork” of Bitcoin Core, but rather a far smaller (deterministically built) codebase that users can inspect.
Of course every program in the same user-space as Bitcoin Core can control the node and wreak havoc, so arguably the preference of miners to run a “pristine” version of Bitcoin Core along with “other” software (like bitcoin-mine and SRI) is psychological.
@plebhash wrote:
We are planning to write some Rust libs that will allow SRI tooling connect to Bitcoin Core over IPC.
That’s good to know, because think I can eventually stop maintaining this.
The reason I haven’t done this strip-rest-of-bitcoin-core-code-from-bitcoin-mine step yet is that I’m also maintaining a modified bitcoind -sv2 in https://github.com/Sjors/bitcoin/pull/68 which shares the Stratum v2 specific code.
Having a standalone bitcoin-mine binary should substantially lower the barrier to contribution. Currently people do file bug reports (e.g. https://github.com/Sjors/bitcoin/issues/72), but I’m the only one who fixes them. However once I strip all unused Bitcoin Core stuff, there’s no (easy) way back.
At this point I think we’ve made enough progress towards including IPC into Bitcoin Core that I feel comfortable going in that direction - just after the v30 release. I would much prefer that I can instruct people to use the official v30 release, but otherwise they’ll have to use a v30+IPC release from me for a while.
Sjors
commented at 5:28 pm on August 15, 2025:
member
I expanded the PR description with some history.
@josibake I would prefer to improve docs in a followup. As @ryanofsky mentioned #31802 (review), it probably needs to be integrated with docs for #31375.
fanquake
commented at 6:51 pm on August 18, 2025:
member
Note that when this is merged, oss-fuzz will need fixing, to either opt-out of multiprocess, or configure depends to use the pre-built Clang for native package compilation (as Ubuntu 20.04 ships with GCC 9). This isn’t an issue currently, as we don’t build native packages (NO_QT=1), but with multiprocess on by default, we’ll build native_capnp:
0/src/bitcoin-core/depends/work/download/native_capnp-1.2.0/capnproto-cxx-1.2.0.tar.gz.temp: OK
1Extracting native_capnp...
2/src/bitcoin-core/depends/sources/capnproto-cxx-1.2.0.tar.gz: OK
3Preprocessing native_capnp...
4Configuring native_capnp...
5-- The CXX compiler identification is GNU 9.4.0
6-- Detecting CXX compiler ABI info
7-- Detecting CXX compiler ABI info - failed
8-- Check for working CXX compiler: /usr/bin/g++
9-- Check for working CXX compiler: /usr/bin/g++ - broken
10CMake Error at /usr/local/share/cmake-3.29/Modules/CMakeTestCXXCompiler.cmake:60 (message):
11 The C++ compiler
1213"/usr/bin/g++"1415 is not able to compile a simple test program.
1617 It fails with the following output:
1819 Change Dir: '/src/bitcoin-core/depends/work/build/x86_64-pc-linux-gnu/native_capnp/1.2.0-b8ef312d3bf/CMakeFiles/CMakeScratch/TryCompile-8xnY4X'2021 Run Build Command(s): /usr/local/bin/cmake -E env VERBOSE=1 /usr/bin/make -f Makefile cmTC_a7435/fast
22 make[1]: Entering directory '/src/bitcoin-core/depends/work/build/x86_64-pc-linux-gnu/native_capnp/1.2.0-b8ef312d3bf/CMakeFiles/CMakeScratch/TryCompile-8xnY4X'23 /usr/bin/make -f CMakeFiles/cmTC_a7435.dir/build.make CMakeFiles/cmTC_a7435.dir/build
24 make[2]: Entering directory '/src/bitcoin-core/depends/work/build/x86_64-pc-linux-gnu/native_capnp/1.2.0-b8ef312d3bf/CMakeFiles/CMakeScratch/TryCompile-8xnY4X'25 Building CXX object CMakeFiles/cmTC_a7435.dir/testCXXCompiler.cxx.o
26 /usr/bin/g++ -I/src/bitcoin-core/depends/x86_64-pc-linux-gnu/native/include -pipe -std=c++20 -fPIE -o CMakeFiles/cmTC_a7435.dir/testCXXCompiler.cxx.o -c /src/bitcoin-core/depends/work/build/x86_64-pc-linux-gnu/native_capnp/1.2.0-b8ef312d3bf/CMakeFiles/CMakeScratch/TryCompile-8xnY4X/testCXXCompiler.cxx
27 g++: error: unrecognized command line option '-std=c++20'; did you mean '-std=c++2a'?
28 make[2]: *** [CMakeFiles/cmTC_a7435.dir/build.make:78: CMakeFiles/cmTC_a7435.dir/testCXXCompiler.cxx.o] Error 1
in
depends/toolchain.cmake.in:169
in
16bce9ac4coutdated
In 16bce9ac4cd02b2c8308f370bf39d70f9421e69b “build: depends makes libmultiprocess by default”
For the other packages that depends may build, there is a similar if-else block right about this, with a comment that says something about needing to use MATCHES rather than STREQUAL. That seems like something that should be done here?
ryanofsky
commented at 8:54 pm on August 18, 2025:
I think this looks ok, although it would be nice to make the style consistent. The MATCHES checks are treating values like “@zmq_packages@” “@wallet_packages@” as false if they are empty strings or if they are “strings containing only spaces” since apparently such values are treated as false by make & depends.
The check here is doing the same thing but it’s just implemented in a different way. If @ipc_packages@ contains spaces, cmake will ignore them and the ${ipc_packages} variable will be empty.
This seems potentially cleaner than doing a regex match, though I’m not sure what style is better. Either way the behavior should be ok
achow101
commented at 8:01 pm on August 18, 2025:
member
I disagree with defaulting ENABLE_IPC=ON since that makes capnproto almost a required dependency. Instead of it being an optional feature that you can turn on, it now becomes and optional feature that you can turn off, and if you just do a build with the defaults, capnproto will need to be installed.
I think it’s ok to have the default source build not match the release builds or the depends builds since we already do that by excluding optional features like the GUI, or ZMQ, etc.
ryanofsky
commented at 8:41 pm on August 18, 2025:
contributor
I disagree with defaulting ENABLE_IPC=ON since that makes capnproto almost a required dependency. Instead of it being an optional feature that you can turn on, it now becomes and optional feature that you can turn off, and if you just do a build with the defaults, capnproto will need to be installed.
Yes, with the current version of this PR, ENABLE_IPC acts like ENABLE_WALLET: it’s an optional feature that is default-on in both depends and non-depends builds.
I didn’t realize that other options like GUI and USDT and ZMQ were default-off in the non-depends build even though they are default-on in the depends build. I can see how this makes sense and is probably a good example to follow for ENABLE_IPC.
I do think the most important things this PR is doing are (1) making IPC available in binary releases and (2) enabling the IPC build and tests in many more CI jobs. What the cmake default is shouldn’t matter as much since it should be pretty easy for anybody who is building to change, but I tend to agree it would be good if this PR left it off of default or a followup turned it off.
Sjors
commented at 8:49 am on August 19, 2025:
member
Capnp is substantially quicker to install than qt and doesn’t slow down the full build as much as the GUI does, so that’s not the best comparison. Comparison to the wallet isn’t perfect either; they both add to the build time, but sqlite3 is more widely packaged (though so far I’ve only found OpenBSD to be a problem for capnp, where it needs to be installed from source).
My preference is to keep the PR as is and see if capnp actually causes problems for people building on master. We’ll need to know about such issues anyway if we want to continue the bigger multiprocess project.
#32345 has landed. I previously tested that that PR won’t cause CI issues. So to preserve acks I won’t rebase unless I need to retouch for other reasons.
ryanofsky
commented at 1:48 pm on August 19, 2025:
contributor
I’ve summarized current state of this PR below, and think this maybe ready for merge. EDIT: Contents below have been been updated after this was initially posted as discussion progressed.
Review
ryanofsky: current ack, low level reviews
ismaelsadeeq: current ack, testing, and comments on documentation and related prs
josibase: current ack, high level review, no code comments
janb84: current ack, high level review with questions and considerations of tradeoffs
fanquake: low level reviews, most thorough build & CI testing, reporting and fixing openbsd & lint/tidy issues previously, and current gcc 11 / capnproto 0.8 and oss-fuzz issues. Nacked an earlier version of PR that left cmake IPC default off when not using depends, but now it is on again.
achow101: close-seeming review pointing out an inconsistency in cmake code. Current concern about default non-depends cmake option value
charlatan: stale high level ack, testing guix build, suggestion to include .capnp files in binary releases which does seem like a good idea for a followup
vasild: stale review ack, comments about naming and commit structure
maflcko: comments about commit order and spelling, comment suggesting tidy ci fixing
BrandonOdiwuor: stale ack, testing of depends & non-depends builds
marcofleon: full review, discussion about binary/source distribution
TheBlueMatt, pavlenex, plebhash, average-gary (in #31756): recent high level comments in favor of this change to support Sv2, and dergoegge asking a question about this
darosior, sipa, stickies: older higher level comments in #31756 about how and whether we should ship features before they have been widely tested
sipa and theuni have added more recent comments (below) pointing out that if an -ipcbind option were added to bitcoind there would be no need for a bin/bitcoin wrapper and new binaries in libexec/
There’s been recent disagreement about whether the non-depends build cmake ENABLE_IPC default should be on or off
The idea of turning it separately (to encourage more developer testing of IPC) was brought up in last week’s IRC meeting and led to an implementation in #33190
fanquake nacked a recent iteration of this PR that left it off, because that would put us in a position of shipping releases with a feature turned off in source builds if not using the dev-mode preset or depends. So that change was reverted and it is on again in this PR.
achow yesterday pointed out some reasons maybe it should be off: inconvience it could cause and fact that other similar features (GUI, ZMQ, USDT) are also off in non-depends builds
IMO (ryanofsky) it doesn’t matter too much whether this default is on or off, because cmake and ccmake make it pretty easy to toggle, and I feel like we can pretty easily change it if it causes headaches in practice. In the short term I think I agree with achow that since this option is only useful for mining feature it probably does make sense to leave off, but in the future I hope it will be used for more things (#32297, #29409, #32898, #10102) and have more justification for being on.
sipa has commented below saying if the IPC feature is going to be viable in long run, it needs to be used broadly, not just in narrow cases
lack of bitcoind -ipcbind option
This PR only enables -ipcbind for the bitcoin command not the bitcoind command.
sipa and theuni are in favor of supporting bitcoind -ipcbind because it would allow not adding new binaries to libexec/, require all users to run binaries with cap’n proto and libmultiprocess dependencies instead of only some users, and potentially make it more likely that users and developers will test IPC features.
ryanofsky is open to the bitcoind-ipcbind idea but points out ways this could increase medium-term complexity by unbundling the IPC and multiprocess features, and he would not be thrilled about foisting capnproto dependency on users who may not need or want it (especially since a benefit of the bin/bitcoin wrapper and separate libexec/ directory is making it possible to ship smaller binaries with minimal dependencies without affecting the CLI)
This could be discussed further in a new PR or issue or in #30983. (Update: followed up in #30983 (comment))
conflation of different multiprocess features
sipa and theuni pointed out it is bad to require users to explicitly use multiprocess flags or binaries just to use -ipcbind, because it confuses separate IPC listening and process separation features which are both enabled by the multiprocess dependency.
ryanofsky agrees and thinks #33229 would solve this by making the bitcoin -m option optional, since the -m option is the only place the multiprocess term should be currently visible to users.
user confusion from new files
theuni pointed out presence of the new bin/bitcoin file (already added in master) and libexec/bitcoin-node and libexec/bitcoin-gui files (added by this PR) in binary releases could cause confusion for users
ryanofsky pointed out the effects of this should be mitigated by the readme file located in the root directory, and general familiarity with the convention of putting user-facing binaries bin/ but more documentation or different naming might solve potential problems
RFM?
With all that, I feel PR has had enough to discussion and review to be ready for merge given its impact. (Main impact being adding an -ipcbind option in binary releases, and secondary impact changing build system and CI defaults.) So I’m thinking of merging this today, but would appreciate a sanity check and knowing if I’m off-base about anything!
sipa
commented at 2:33 pm on August 19, 2025:
member
With #33190 closed, I’ll comment further here.
@achow101@ryanofsky In my comment in the other PR regarding seeing this PR as a successor to that one, I failed the consider the possibility of enabling it in release builds without making it default in from-source builds, like Qt, ZMQ, USDT, so I saw it as two separate sequential decisions to be made, rather than seeing them as orthogonal.
That said, I think there is a difference with those features. What is being enabled here is nominally an optional feature (in the sense that right now all the multiprocess binaries do is add the mining IPC service), but it’s really the start of moving towards a multiprocess world, where the new binaries at some point become the “normal” way of using the Bitcoin Core daemon. Certainly at that point, I think ENABLE_IPC should be the default, and we should see capnp as a normal expected dependency, rather than an optional extra feature.
So my concern is more about how these binaries are being used than about the actual build system and its defaults. With the multiprocess binary only useful for the mining IPC, it will effectively, for the time being, only be used by people interested in that (either they build from source with -DENABLE_IPC=ON, or they use the release bitcoin-node binary with this PR), while ~all others will continue to use non-IPC from-source builds and non-IPC bitcoind. I think this is an undesirable situation - shipping an entire new replacement set of binaries that only get eyes/practice from a tiny subset of developers and users. I know not everyone agrees on this, but splitting attention this way for a whole new way of doing this means a reduction in assurance we have.
My preference would be to make a decision at some point that we start transitioning to multiprocess as the new way of doing things, so that it gets all eyes. That does not need to mean dropping support for non-IPC, and doesn’t need to be a hard break that happens on any particular point in time, and it does not need to be now. But that decision would come with enabling it by default in developer builds, moving most/all tests over to use it, adapt documentation to suggest those binaries (or the bitcoin binary), etc.
Maybe today is not yet the time to make that decision, but if we don’t, then I think we should treat multiprocess as a second-class and experimental approach.
ryanofsky
commented at 4:25 pm on August 19, 2025:
contributor
I think this is an undesirable situation - shipping an entire new replacement set of binaries that only get eyes/practice from a tiny subset of developers and users.
Thanks for being specific about the negative outcome you see here.
I think I might not understand the significance of the situation you’re describing though. The only difference between the binary that supports the -ipcbind option and the binary that does not support it is that one binary is linked against init/bitcoind.cpp and the other is linked against init/bitcoin-node.cpp and the libbitcoin_ipc library.
Other than that, the two binaries are identical. They contain the same compiled objects, are built with the same tools, and are verified by the same people. In practice, it shouldn’t matter which binary you run if you’re not using the -ipcbind option, either for your own use, or for testing of IPC functionality more broadly.
I do think we may have different starting assumptions about binary distribution in general. To me, it’s a good thing that if you don’t use a feature, you don’t need to load or install it on your system. If you don’t care about IPC you can use bitcoind and rm bitcoin-node without concern. With #10102, if you only want to run the node and don’t care about the wallet or gui, you can rm bitcoin-wallet or rm bitcoin-gui just as easily. I don’t think these things as a huge deal but I do see them as benefits of the modularity that IPC enables.
I can understand how someone might look at this and come to the opposite conclusion, though. We already provide separate bitcoind and bitcoin-qt binaries and it’s true that bitcoin-qt gets less testing than bitcoind. If we only provided bitcoin-qt, we might have fewer GUI bugs. And the benefits of bundling cut both ways: the wallet benefits from being bundled with the node since it gets installed more widely, but the node also benefits from the wallet since it makes certain test cases easier to write and run.
I also agree with you about the current state and the end goal for multiprocess support. In the long term, it makes no sense for multiprocess to remain an optional feature disabled by default. Ultimately, it should either be enabled by default or dropped. In the meantime, it remains an experimental, second-class feature.
Getting back to the current PR, I think turning on the ENABLE_IPC option by default is going in a direction you support. It is not baking -ipcbind option into bitcoind, which I see as a good thing, while you see it as a negative. But importantly, this PR does not prevent us from baking IPC/multiprocess support into the bitcoind binary in the future if we decide. And I hope it’s clear that even if we changed the PR now to bake -ipcbind intobitcoind and never shipped a bitcoin-node binary it would mostly be a symbolic move without much practical effect. It wouldn’t meaningfully increase testing of the IPC feature, for example.
TheBlueMatt
commented at 6:23 pm on August 19, 2025:
contributor
The only difference between the binary that supports the -ipcbind option and the binary that does not support it
Forgive my ignorance, but ISTM a simple solution to @sipa’s concern, then, would be for the only release bitcoind binary to be the bitcoin-node one. Is there some reason why someone should prefer to have run a binary which is simply missing the option (given release builds optionally provide it)? Is there a specific concern with that (you mention you see the separate binary as a “good thing”) but presumably there’s not any (material) risk here for folks who simply don’t turn on the ipc option?
marcofleon
commented at 7:02 pm on August 19, 2025:
contributor
It’s my first time fully reading through this PR (along with #31756 and #33190) and looking at the code changes so my understanding is high level and likely incomplete. If there are details I’m missing or other PRs I should check out, please let me know.
Based on #31756 (comment) and #31802 (comment), it seems that the current users/testers of SV2 are running software (Sjors’s “SV2 integrated into core” fork iiuc) that will probably never be in Bitcoin Core. Would it not make more sense to instead use the DENABLE_IPC=ON option and run bitcoin-node?
The key strategic point here is simply having the mining IPC interface available on Bitcoin Core, nothing more than that.
Is this not already true, with or without this PR in v30? I don’t see how it’s required to use a fork of Core, unless you insist on running pre-built binaries only. This may be somewhat naive, but it doesn’t seem that difficult to encourage users that want SV2 to use v30 with IPC enabled, and then open up issues as bugs turn up. This would give us more confidence to include the new binary in the next official release.
I think if this did end up being in v30, it’s likely to be labeled as experimental and “use at your own risk” until it’s better integrated with CI and fuzzing infrastructure.
edit:
the two binaries are identical. They contain the same compiled objects, are built with the same tools, and are verified by the same people
Just saw this, so I’m not as sure about the experimental label.
ryanofsky
commented at 7:28 pm on August 19, 2025:
contributor
The only difference between the binary that supports the -ipcbind option and the binary that does not support it
Forgive my ignorance, but ISTM a simple solution to @sipa’s concern, then, would be for the only release bitcoind binary to be the bitcoin-node one.
If the only purpose of the ENABLE_IPC option were to enable the IPC mining interface this would make sense. But that’s not the only purpose or even the intended one. The point of the option is to turn on IPC features broadly and in later PRs allow running the node, gui, and wallet in separate processes, even separate hosts, and allow them to stop and stop separately and reconnect freely. It changes the way binaries are used and invoked as well as their runtime behavior. This has benefits for flexibility and security but also can make certain gui & wallet interactions slower since it replaces local calls with remote calls and obviously hasn’t had as much testing. ENABLE_IPC builds separate binaries to allow developing and testing this feature without forcing a switch right away or requiring the new feature have 100% parity and be fully stable before it is enabled at all.
That said, if we want to stick some IPC functionality (-ipcbind only, not multiprocess) into bitcoind instead of only making it available through the bitcoin command we could do that. Then could be a separate ENABLE_MULTIPROCESS option that depends on ENABLE_IPC and controls whether multiprocess features are available.
This PR doesn’t preclude doing that and if it makes sipa or others happy it could be a followup. I will say sipa’s preference seems mostly symbolic to me and as far as I know doesn’t have many practical implications, but that doesn’t mean it’s not worth following up on.
ryanofsky
commented at 7:42 pm on August 19, 2025:
contributor
This would give us more confidence to include the new binary in the next official release.
marcofleon, I think your understanding is all correct but note that -ipcbind feature and mining interface are available in v29 and can be easily toggled on in cmake. If you check discussion starting #31802 (comment) not having the feature enabled in binary releases and depends builds has seemed to be a barrier to adoption. It do think it might not be strictly accurate to refer to builds of bitcoin core with non-standard options as “forks” but I can understand why they’d be called that.
achow101
commented at 8:25 pm on August 19, 2025:
member
What is being enabled here is nominally an optional feature (in the sense that right now all the multiprocess binaries do is add the mining IPC service), but it’s really the start of moving towards a multiprocess world, where the new binaries at some point become the “normal” way of using the Bitcoin Core daemon. Certainly at that point, I think ENABLE_IPC should be the default, and we should see capnp as a normal expected dependency, rather than an optional extra feature.
Right, but my thinking was that we would do that at the same time we switch everything to use multiprocess, but perhaps we will do a more gradual rollout.
However, I don’t feel that strongly about it so I am ok with enabling it now. My main concern is that doing it now may confuse users who are building from source since it does add a new dependency to the default build.
theuni
commented at 8:26 pm on August 19, 2025:
member
(Sorry in advance for the meandering comment, I was reading the others coming in as I was typing this up)
@achow101@ryanofsky In my comment in the other PR regarding seeing this PR as a successor to that one, I failed the consider the possibility of enabling it in release builds without making it default in from-source builds, like Qt, ZMQ, USDT, so I saw it as two separate sequential decisions to be made, rather than seeing them as orthogonal.
That said, I think there is a difference with those features. What is being enabled here is nominally an optional feature (in the sense that right now all the multiprocess binaries do is add the mining IPC service), but it’s really the start of moving towards a multiprocess world, where the new binaries at some point become the “normal” way of using the Bitcoin Core daemon. Certainly at that point, I think ENABLE_IPC should be the default, and we should see capnp as a normal expected dependency, rather than an optional extra feature.
Agreed. I don’t think Qt/ZMQ/etc. are analogous to libmultiprocess. IMO the better example is Boost. Boost is required for bitcoind, so it’s on by default. If multiprocess binaries are the way forward, we’ll need to deal with the headache of that dependency. There’s simply no getting around that.
Having it on by default forces everyone to grapple with busted gcc’s, antiquated system libraries, unsupported operating systems, out-of-date docs, etc. That’s why we turn important features on by default.. to force the project as a whole to deal with those issues as they arise. As sipa said above, that’s the best way we’ve found to get eyes on new features. We want to discover those warts before we rely on new features.
Forgive my ignorance, but ISTM a simple solution to @sipa’s concern, then, would be for the only release bitcoind binary to be the bitcoin-node one. Is there some reason why someone should prefer to have run a binary which is simply missing the option (given release builds optionally provide it)? Is there a specific concern with that (you mention you see the separate binary as a “good thing”) but presumably there’s not any (material) risk here for folks who simply don’t turn on the ipc option?
There’s a compromise here.. shipping the ipc-enabled bitcoin-node, with bitcoind as a symlink to it, and none of the other machinery. No bitcoin, bitcoin-gui, or bitcoin-wallet binaries (yet). As @TheBlueMatt suggested, from a casual user’s POV, this would JUST be bitcoind with an extra option, but it would not include any of the other binaries that have had less eyes/testing. That would:
Be transparent to the average user
Be enough to run sv2
Not incur (in theory) any extra maintenance burden for the project
v31 could then ship the other fun binaries once they’ve been default-on for a release cycle.
…
(Stopping here, because while typing this out, @ryanofsky responded with some helpful points
That said, if we want to stick some IPC functionality (-ipcbind only, not multiprocess) into bitcoind instead of only making it available through the bitcoin command we could do that. Then could be a separate ENABLE_MULTIPROCESS option that depends on ENABLE_IPC and controls whether multiprocess features are available.
I think this is what I was starting to suggest above.
I realize that ipc/multiprocess is much more than just bitcoind+-ipcbind, but we really need to look at this from a user’s/support perspective. What you outlined above is a substantial departure from how users have always interacted with bitcoin core. Doing a bridge v30 release which adds the machinery to take advantage of sv2 mining without complicating anyone else seems like a reasonable and measured approach to me.
Please take a step back and just think like a user. v29 shipped with bitcoind. They’ll now see bitcoin, bitcoind, and bitcoin-node. Even if their behavior doesn’t have to change at all, anyone could be forgiven for being confused by that.
This PR doesn’t preclude doing that and if it makes sipa or others happy it could be a followup. I will say sipa’s preference seems mostly symbolic to me and as far as I know doesn’t have many practical implications, but that doesn’t mean it’s not worth following up on.
I don’t think it’s helpful to be so dismissive. Shipping new binaries (but not on all platforms), recommending new ways of running Core, and deprecating old ones shouldn’t be taken lightly. The experience you described above makes sense to me, but sounds like it would be confusing to the average bitcoind user (even if they don’t have to change anything). Turning ipc/mp on by default would at least force everyday devs to go through the process. I’m not sure how many devs would currently even be able to explain to a user how they should use the new bitcoin binary. That’s why turning some of this stuff on by default first not just symbolic, but necessary.
tl;dr:
For v30, what makes sense to me is turning on IPC by default for a minimal subset of features that would allow for:
Requiring all devs to build (lib)multiprocess or manually turn it off
Exercising the build/release/docs parts of ^^
Running sv2
But not:
Confusing users with a separate set of binaries
Suggesting users change their behavior (yet)
I suppose that basically means changing the defaults to build (and ship) a bitcoind with -ipcbind, and changing little else.
That would be the type of progressive rollout that I would expect from a mature project, at least.
ryanofsky
commented at 8:33 pm on August 19, 2025:
contributor
Please take a step back and just think like a user. v29 shipped with bitcoind. They’ll now see bitcoin, bitcoind, and bitcoin-node. Even if their behavior doesn’t have to change at all, anyone could be forgiven for being confused by that.
No, they will absolutely not see that and it I agree would be a huge problem if that is what were were shipping.
They will see a just one new binary called bitcoin which is a wrapper around internal binaries. bitcoind is unchanged.
sipa
commented at 8:33 pm on August 19, 2025:
member
@ryanofsky I wouldn’t be opposed to this idea (having ENABLE_IPC that enables -ipcbind in bitcoind itself, separate from ENABLE_MULTIPROCESS which enables bitcoin-node, and later the broken-out features as separate binaries).
Multiprocess is more than just IPC, and the mining interface isn’t really (or doesn’t need to be) part of the multiprocess world, it just happens to use the same interface. Furthermore, at this point there really isn’t any user-visible multiprocess functionality to speak of, so it would make sense then to have ENABLE_IPC in release binaries, but wait with turning on ENABLE_MULTIPROCESS until there actually is something to show for.
That said, I’m also not NACKing this PR, but I feel uneasy about starting the transition towards the multiprocess world at this stage, given the limited eyes and use it has had.
theuni
commented at 8:56 pm on August 19, 2025:
member
Please take a step back and just think like a user. v29 shipped with bitcoind. They’ll now see bitcoin, bitcoind, and bitcoin-node. Even if their behavior doesn’t have to change at all, anyone could be forgiven for being confused by that.
No, they will absolutely not see that and it I agree would be a huge problem if that is what were were shipping.
They will see a just one new binary called bitcoin which is a wrapper around internal binaries. bitcoind is unchanged.
Hmm? I just built/installed this branch, and it installed all 3 binaries. bitcoin-node is in libexec, of course, but they’ll all be in the guix tarball (which is what all this is about), no? Am I missing something?
They will see a just one new binary called bitcoin which is a wrapper around internal binaries. bitcoind is unchanged.
You and I know that, yes, but figuring out what’s changed may be non-trivial for a user.
ryanofsky
commented at 8:58 pm on August 19, 2025:
contributor
I don’t think it’s helpful to be so dismissive. Shipping new binaries (but not on all platforms), recommending new ways of running Core, and deprecating old ones shouldn’t be taken lightly.
I don’t think I am being dismissive, but sorry if anything came across that way. I understand there have been a lot of PRs to keep track of and the interactions between them may be confusing, but I feel if anything this approach is more conservative than what sipa is suggesting, and is compatible with his suggestions.
bitcoind is entirely unaffected by the PR. The new bitcoin binary introduced in #31375 is also not directly affected but this PR makes its -m option for starting the node or gui with multiprocess features functional. In binary releases, this PR causes the 2 new binaries which are not intended to be called directly by users to be placed in libexec/.
I’m trying to parse the rest of your comment but fundmentally it is not clear to me how you are connecting “Requiring all devs to build (lib)multiprocess or manually turn it off” to “build (and ship) a bitcoind with -ipcbind”. I feel like this PR is doing all of the things on your list of things it should do and none of the things on your list of things it shouldn’t do.
ryanofsky
commented at 9:02 pm on August 19, 2025:
contributor
You and I know that, yes, but figuring out what’s changed may be non-trivial for a user.
Is it possible to state more concretely what may be confusing here? The new bitcoin binary is already enabled in #31375. The only thing this PR affects is the behavior of the -m option, if specified, and the contents of the libexec directory.
theuni
commented at 9:11 pm on August 19, 2025:
member
I don’t think it’s helpful to be so dismissive. Shipping new binaries (but not on all platforms), recommending new ways of running Core, and deprecating old ones shouldn’t be taken lightly.
I don’t think I am being dismissive, but sorry if anything came across that way. I understand there have been a lot of PRs to keep track of and the interactions between them may be confusing, but I feel if anything this approach is more conservative than what sipa is suggesting, and is compatible with his suggestions.
bitcoind is entirely unaffected by the PR. The new bitcoin binary introduced in #31375 is also not directly affected but this PR makes its -m option for starting the node or gui with multiprocess features functional. In binary releases, this PR causes the 2 new binaries which are not intended to be called directly by users to be placed in libexec/.
I understand this. I am suggesting that the bitcoin binary was defaulted to on prematurely. I am also suggesting (as sipa is, I believe), that it’s unnecessary to ship bitcoin-node, and bitcoin-gui for v30, if the primary goal is to allow for the use of sv2. I think shipping the extra binaries at this stage is unnecessarily confusing for little benefit.
I admit that I am late to this discussion as I have been heads-down elsewhere.
I’m trying to parse the rest of your comment but fundmentally it is not clear to me how you are connecting “Requiring all devs to build (lib)multiprocess or manually turn it off” to “build (and ship) a bitcoind with -ipcbind”. I feel like this PR is doing all of the things on your list of things it should do and none of the things on your list of things it shouldn’t do.
I’m agreeing with sipa that it’s not strictly necessary to ship “the multiprocess world” just yet, when most devs likely have not been building with ipc at all. I would rather see a bitcoind with -ipcbind as part of the default build/release for v30. That would allow us to go through the process of requiring the dependency without disrupting much else. To me, that seems ideal.
TheBlueMatt
commented at 9:55 pm on August 19, 2025:
contributor
I don’t see how it’s required to use a fork of Core, unless you insist on running pre-built binaries only. This may be somewhat naive, but it doesn’t seem that difficult to encourage users that want SV2 to use v30 with IPC enabled, and then open up issues as bugs turn up
Yes, this is somewhat naive. We’ve discussed this more than a few times and the reality is that people just don’t build from source. We’ve been talking to miners for a long time and telling them “just take Sjors’ branch and build it and run that” and basically getting laughed at. Yes, part of the hesitance of miners to doing that is the “branch” part, but a nontrivial part is also just the “build from source” step. We’re not talking about miners who are software engineers, and even building Bitcoin Core from source is a hurdle we don’t want to have to deal with. Ultimately to make mining even slightly decentralized we have to convince miners to overhaul much of the tech stack they rely on as a core part of their business, let alone introduce a number of additional pieces to that stack, any of which being buggy may result in burning energy for nothing in return.
ryanofsky
commented at 10:33 pm on August 19, 2025:
contributor
@theuni Thanks for stating your preference. If I am following, you would like to:
Link cap’n proto and libmultiprocess into bitcoind and expose a new -ipcbind option there instead of only exposing it through the new bitcoin binary.
Revert or partially revert #31375 and stop including the bitcoin wrapper binary in releases because its presence would be too confusing.
Keep all the changes implemented in this PR which toggle the ENABLE_IPC option.
Introduce a new ENABLE_MULTIPROCESS option to toggle building of multiprocess binaries instead of having ENABLE_IPC toggle both IPC and multiprocess features.
Not merge this PR until 4 is done because the existence of two new executables in libexec/ would be confusing to users, even though they are not in bin/ and not on the PATH.
It seems like you are asking for something well beyond what sipa is asking for. I don’t think sipa is complaining about the bitcoin utility. I haven’t heard anyone complain about the bitcoin utility. I also think the changes you are asking for are less conservative than what this PR does because your and sipa’s approach would bake IPC features and dependencies into all binaries when their only practical use is for the mining interface.
I think I have a clear idea of what you want, but I don’t yet have a clear idea of why you want it. I don’t understand what practical harm you think the current PR has. Pointing out that people may notice and be confused by new files sounds like grounds for organizing and documenting the files better as done in #31679, not dictating how widely to roll out IPC features. I think we all want to end up in the same place in the long run but I prefer a narrower rollout in the short run which is what has been implemented and discussed in this PR and related PRs and issues over the past years (particularly #30983, #31756, #31098, #28722)
sipa
commented at 1:04 am on August 20, 2025:
member
Yes, I don’t think the bitcoin binary as such is an issue. If we end up going for a “-ipcbind in bitcoind” approach, and we’d have chosen that earlier, that would have removed some of the impetus for adding the bitcoin binary - but it seems independently useful to have something that wraps even just bitcoind and bitcoin-cli.
your and sipa’s approach would bake IPC features … into all binaries when their only practical use is for the mining interface.
I know you disagree here, but I believe that is a good thing.
… and dependencies …
That’s an argument against enabling it by default in from-source builds, but the release binaries link libcapnp statically.
ryanofsky
commented at 2:24 am on August 20, 2025:
contributor
Thanks again for clarifying sipa. I believe adding the -ipcbind to bitcoind and having separate build flags for IPC and multiprocess features is a reasonable approach and something I want to propose and discuss further in #30983.
I think we both agree that change could be a followup for a future release and does not need to block the current PR, but please let me know if I misunderstood and that is not the case, or if you changed your mind about this.
I think adding -ipcbind to bitcoind could be a nice accompaniment to PR’s potentially exposing other IPC interfaces like #32297 (RPC interface), #29409 (Chain interface), #32898 (Tracing interface) since it would make these interfaces available to users who just want to use bitcoind and are not interested in using bitcoin.
I do think there are some tradeoffs to consider. As mentioned, I would be sad about no longer providing binaries with minimal dependencies. I think that’s a nice thing for the project to provide for its users. I would also be sad about having two build options instead of one because I think build options are a maintenance burden in a way runtime options are not (and as I have discussed with hebasto various times, ideally I would want to eliminate allENABLE_ build options that control which targets are build, and make cmake only have WITH_ options controlling which dependencies are used). From a usability perspective, I also like the idea of treating multiprocess and IPC features as a unit. I like the idea that bitcoind and bitcoin-qt would not be changing in any way, but the bitcoin CLI would offer a superset of their functionality and a bunch of new features. It is true that IPC and multiprocess features do not need to be tied together, just as the sqlite and descriptor wallet features do not need to be tied together. But I think in both cases tying them together has the benefit of reducing long term complexity and setting a clear direction for the project.
ryanofsky
commented at 10:39 am on August 20, 2025:
contributor
With all that, I feel PR has had enough to discussion and review to be ready for merge given its impact.
I think this is still ready for merge but obviously want to give this more time after the discussion yesterday. I would like to merge this today though, if this is reasonable.
To summarize the discussion yesterday I added the following sections to the review summary. Any corrections or other feedback is definitely welcome!
Review
sipa and theuni have added more recent comments pointing out that if an -ipcbind option were added to bitcoind there would be no need for a bin/bitcoin wrapper and new binaries in libexec/
Concerns
lack of bitcoind -ipcbind option
This PR only enables -ipcbind for the bitcoin command not the bitcoind command.
sipa and theuni are in favor of supporting bitcoind -ipcbind because it would allow not adding new binaries to libexec/, require all users to run binaries with cap’n proto and libmultiprocess dependencies instead of only some users, and potentially make it more likely that users and developers will test IPC features.
theuni has additionally expressed a preference for partially reverting #31375 and dropping the bitcoin binary from binary releases
ryanofsky is open to the bitcoind-ipcbind idea but points out ways this could increase medium-term complexity by unbundling the IPC and multiprocess features, and he would not be thrilled about foisting capnproto dependency on users who may not need or want it (especially since a benefit of the bin/bitcoin wrapper and separate libexec/ directory is making it possible to ship smaller binaries with minimal dependencies without affecting the CLI)
Please take a step back and just think like a user. v29 shipped with bitcoind. They’ll now see bitcoin, bitcoind, and bitcoin-node.
As discussed above bitcoin-node is not in PATH since #31679. This has the nice benefit of being able to drop the binary if we go for a different approach later of adding -ipcbind to bitcoind instead (not sure if it’s a good idea).
Once we instruct miners to use bitcoin -m node -ipcbind=unix, ideally that should just keep working. That could be achieved by ignoring the -m flag or special casing the presence of -ipcbind.
I am suggesting that the bitcoin binary was defaulted to on prematurely.
I like it, especially bitcoin rpc as an alias to bitcoin-cli -named. I’ve also been updating documentation to use this, e.g. #33112. The ship hasn’t fully sailed, but it’s leaving the harbour.
sipa
commented at 11:42 am on August 20, 2025:
member
I think we both agree that change could be a followup for a future release and does not need to block the current PR, but please let me know if I misunderstood and that is not the case, or if you changed your mind about this.
I don’t really see the point of doing both.
My (mild) preference, if we are to have IPC in release binaries in 30.0, is to add -ipcbind support to bitcoind, and not include bitcoin-node in the release. This means everyone uses and tests the same binary, isn’t forced into adopting the multiprocess approach to use IPC (though they can use bitcoin wrapper if they like, which will make a future transition easier, which I very much hope happens - but at a time when multiprocess actually has tangible benefits beyond exposing IPC).
If this PR is merged as-is now, and later -ipcbind support is added to bitcoind too, then we end up in a situation where we’re literally shipping two feature-equivalent binaries (bitcoin-node and bitcoind), unless by that time multiprocess has progressed to a point where bitcoin-node has wallet and/or gui split out? That seems pretty silly to me, and loses the benefits I think we can have from not forcing people into using a separate binary/approach just for the one case where they want the mining IPC.
But at this point, I’d really like to hear some other opinions beyond yours and @Sjors’ (who seem to favor this PR) and @theuni and I (who seem to favor the bitcoind -ipcbind approach). If it’s just us who hold opinions on the matter, I really don’t want to be pushing for this last minute change.
ryanofsky
commented at 12:03 pm on August 20, 2025:
contributor
If this PR is merged as-is now, and later -ipcbind support is added to bitcoind too, then we end up in a situation where we’re literally shipping two feature-equivalent binaries (bitcoin-node and bitcoind),
I don’t see how that would make sense. We should simply drop the libexec/bitcoin-node and libexec/bitcoin-gui binaries if they do not have distinguishing features at that point.
I feel like I keep pointing out difference between the bin/ and libexec/ directories, and you and Cory keep not seeing the difference between them. The contents of the libexec/ directory should not be important to most users. We should document the files in libexec/ and probably give them manpages for packagers and interested users, but the typical user should not care about these files. I use git every day, and have never cared about the contents of its libexec/ directory, because I don’t poke around its internals, I just use its CLI. The libexec/ directory gives git developers and maintainers the freedom to add features and change their internal software architecture without affecting end-users, and the same logic extends to bitcoin or any other package with a libexec/ directory.
sipa
commented at 12:11 pm on August 20, 2025:
member
I don’t see how that would make sense. We should simply drop the libexec/bitcoin-node and libexec/bitcoin-gui binaries if they do not have distinguishing features at that point.
Fair enough. With the bitcoin wrapper binary, there is little cost in dropping the libexec/ binaries that are unnecessary again.
I feel like I keep pointing out difference between the bin/ and libexec/ directories, and you and Cory keep not seeing the difference between them.
My concern is not the fact that there are added binaries, but that we force people to use a different one (bitcoin-node, even if it’s not visible) if they want the mining IPC but don’t care about multiprocess - resulting in them using an approach that isn’t tested/looked at/used by anyone else.
I think the mental load is a factor too (though Cory seems to care more about this), but it is about having to explain to people that they now (in some cases) need bitcoin -m node rather than bitcoind. The fact that there are more binaries isn’t nearly as important as explaining how/when to use them, and why/when they should care about this at all.
I’m in favor of multiprocess happening when it’s ready. We should have a point in time where we decide, as a project, to start moving everything to multiprocess. At that point, there will inevitably be a mental cost to users, but at least it can be explained then as “this gives you the benefit of process separation, being able to run wallets elsewhere, stop/start gui independently from node, …”, and it’ll be used and tested by all.
ryanofsky
commented at 12:31 pm on August 20, 2025:
contributor
but it is about having to explain to people that they now (in some cases) need bitcoin -m node rather than bitcoind
Oh yes, I completely agree this is not ideal. It should be possible to just specify ipcbind on the command line or configuration file and the bitcoin command should pick it up and call the right binary without the need for -m. A suggested improvement listed in #31375 is:
Better --multiprocess (-m) / --monolithic (-M) default selection. Right now, default is monolithic but it probably makes sense to chose more intelligently depending on whether -ipc options are enabled and what binaries are available.
#31375 was another issue that had a lot of internal debates and discussion so I was a little burned out from making improvements to it after it was merged, but this would be a nice, simple improvement that would be great to add before releasing more IPC features. It seems less important for the stratum v2 feature, but doing this would improve usability, and remove an unnecessary coupling of multiprocess and IPC features as you suggest.
ryanofsky
commented at 1:03 pm on August 20, 2025:
contributor
My concern is not the fact that there are added binaries, but that we force people to use a different one (bitcoin-node, even if it’s not visible) if they want the mining IPC but don’t care about multiprocess - resulting in them using an approach that isn’t tested/looked at/used by anyone else.
To be clear, this does not sound like a concern currently, but would be a concern if future PR’s do something we both agree would be bad: automatically opt users into the process separation feature merely because they opted into the IPC listening feature.
This is not something I would ever want to do. I think we should separate runtime options for turning off, on, and customizing IPC listening, and for turning off/on spawning of separate processes (whether they are gui/node/wallet as originally designed, or other things like separate indexing processes),
I would push back a little on wanting to have separate build-time options for process separation and IPC listening since they both have the same build time dependency, so having one option to control that dependency makes sense. IMO avoiding unnecessary build options is a good thing to do because they interact in ways that are more complicated than runtime options and harder to adequately test.
janb84
commented at 2:26 pm on August 20, 2025:
contributor
Please take a step back and just think like a user. v29 shipped with bitcoind. They’ll now see bitcoin, bitcoind, and bitcoin-node. Even if their behavior doesn’t have to change at all, anyone could be forgiven for being confused by that.
@theuni could it be that the context you are describing is the <<buid-dir>>/bin directory and not the result of cmake --install result that splits binaries in ${CMAKE_PREFIX_PATH}/bin and ${CMAKE_PREFIX_PATH}/libexec (as per #31679)?
theuni
commented at 2:39 pm on August 20, 2025:
member
@theuni Thanks for stating your preference. If I am following, you would like to:
It seems (as usual :p) that we’re talking past each other.
Above, I described what would’ve been, in my opinion, the ideal features for v30. That would’ve been a bitcoind with build-time opt-out for IPC. That build-time opt-out would’ve added -ipcbind to bitcoind. It would’ve been default-on, so that all devs would’ve encountered IPC as part of their build process, or turned it off manually. By default, there would’ve been no bitcoin util, nor would there have been any multiprocess binaries built by default.
That is what would’ve made sense to me, from a project POV, to ship for v30. That would’ve been a way to exercise the new IPC functionality (and allow sv2), without encouraging users to move to multiprocess. To me, those are separate concerns, and it would’ve made sense to do one and then the other. The v31 cycle would’ve seen the multiprocess binaries turned on by default (though potentially bitcoin-node wouldn’t have been necessary if the functionality were baked into bitcoind).
But that’s not where we are now. bitcoin is currently built by default. Such is life. In my opinion, that was premature because the multiprocess binaries aren’t yet, but as @Sjors points out, it has other uses. So, no, I’m not suggesting that we revert it. I mentioned that because I think it helps to explain my opinion as a whole. I’m just trying to be consistent.
My concern is not the fact that there are added binaries, but that we force people to use a different one (bitcoin-node, even if it’s not visible) if they want the mining IPC but don’t care about multiprocess - resulting in them using an approach that isn’t tested/looked at/used by anyone else.
I think the mental load is a factor too (though Cory seems to care more about this), but it is about having to explain to people that they now (in some cases) need bitcoin -m node rather than bitcoind. The fact that there are more binaries isn’t nearly as important as explaining how/when to use them, and why/when they should care about this at all.
I’m in favor of multiprocess happening when it’s ready. We should have a point in time where we decide, as a project, to start moving everything to multiprocess. At that point, there will inevitably be a mental cost to users, but at least it can be explained then as “this gives you the benefit of process separation, being able to run wallets elsewhere, stop/start gui independently from node, …”, and it’ll be used and tested by all.
This sums up my opinion well. I just don’t see the rush. There’s a demand for sv2, and a low-risk way to ship that with very little fanfare. Multiprocess seems like a different concern.
What’s here isn’t my preference, but I’m not opposed enough to hold it up. So I’ll step out of the conversation.
theuni
commented at 2:51 pm on August 20, 2025:
member
Please take a step back and just think like a user. v29 shipped with bitcoind. They’ll now see bitcoin, bitcoind, and bitcoin-node. Even if their behavior doesn’t have to change at all, anyone could be forgiven for being confused by that.
@theuni could it be that the context you are describing is the <<buid-dir>>/bin directory and not the result of cmake --install result that splits binaries in ${CMAKE_PREFIX_PATH}/bin and ${CMAKE_PREFIX_PATH}/libexec (as per #31679)?
Heh, I promise I understand the libexec/ distinction. Really. I understand its use as a path for subcommand execution (ala git). Really.
I only mentioned it because I was picturing a user downloading from bitcoincore.org (it’s the binary release that we’re all arguing about, right?), unpacking it, and seeing new files.
My concern is not the fact that there are added binaries, but that we force people to use a different one (bitcoin-node, even if it’s not visible) if they want the mining IPC but don’t care about multiprocess - resulting in them using an approach that isn’t tested/looked at/used by anyone else.
I think the mental load is a factor too (though Cory seems to care more about this), but it is about having to explain to people that they now (in some cases) need bitcoin -m node rather than bitcoind. The fact that there are more binaries isn’t nearly as important as explaining how/when to use them, and why/when they should care about this at all.
This. We (on this PR) understand the difference between the binaries. We understand the libexec/ distinction. But thousands of users who download the tarball will not. They’ll encounter the old bitcoind binary, a new bitcoin binary, and a new bitcoin-node binary in a weird directory that seems to do the same thing as bitcoind if they try to run it.
That’s fine. They’ll encounter bitcoin and the binaries in libexec/ eventually. I just would’ve preferred that to be accompanied with a more intentional push (and campaign) for multiprocess. For v30, they’ll enounter them with little added benefit.
janb84
commented at 3:02 pm on August 20, 2025:
contributor
Heh, I promise I understand the libexec/ distinction. Really. I understand its use as a path for subcommand execution (ala git). Really.
I only mentioned it because I was picturing a user downloading from bitcoincore.org (it’s the binary release that we’re all arguing about, right?), unpacking it, and seeing new files.
Thank you for this insight, did not think of this while reviewing the conversation / PR.
marcofleon
commented at 3:16 pm on August 20, 2025:
contributor
@TheBlueMatt I agree that having the experience be seamless for miners is the goal. I more meant for the purposes of SRI contributors and maybe a couple of brave miners who are willing to help with testing of Core and SV2, so we can avoid buggy code in releases.
ryanofsky
commented at 3:21 pm on August 20, 2025:
contributor
I’m in favor of multiprocess happening when it’s ready. We should have a point in time where we decide, as a project, to start moving everything to multiprocess. At that point, there will inevitably be a mental cost to users, but at least it can be explained then as “this gives you the benefit of process separation, being able to run wallets elsewhere, stop/start gui independently from node, …”, and it’ll be used and tested by all.
This sums up my opinion well. I just don’t see the rush. There’s a demand for sv2, and a low-risk way to ship that with very little fanfare. Multiprocess seems like a different concern.
This is useful feedback, and I think it is actionable. It seems like a PR to make the bitcoin -m flag optional when using -ipcbind or maybe dropping the -m flag entirely would solve this issue? I understand this would not satisfy other expressed preferences, but if you look at the current code (with or without this PR) I think the only thing tying the ipc and process separation features together is the bitcoin -m flag.
ryanofsky
commented at 3:42 pm on August 20, 2025:
contributor
They’ll encounter the old bitcoind binary, a new bitcoin binary, and a new bitcoin-node binary in a weird directory that seems to do the same thing as bitcoind if they try to run it.
This also seems like actionable feedback. Note there is also a test_bitcoin binary in the libexec/ directory and a readme file in the root directory to help orient users. We could rename the binaries to have a suffix like -ipc or -internal if that would make things clearer.
janb84
commented at 5:28 pm on August 20, 2025:
contributor
They’ll encounter the old bitcoind binary, a new bitcoin binary, and a new bitcoin-node binary in a weird directory that seems to do the same thing as bitcoind if they try to run it.
This also seems like actionable feedback. Note there is also a test_bitcoin binary in the libexec/ directory and a readme file in the root directory to help orient users. We could rename the binaries to have a suffix like -ipc or -internal if that would make things clearer.
Just to clarify, please correct me if I’m wrong.
It’s not the libexec directory that’s the issue , it’s the bin directory ( packaged ), or will the v30 packaged also contain a libexec directory?
Currently, if one would download it from the website the packaged release, it contains a bin with
bitcoin-cli
bitcoin-qt
bitcoin-tx
bitcoin-util
bitcoin-wallet
bitcoind
test_bitcoin
After this PR how would this look? a bin directory with
bitcoin-cli
bitcoin-qt
bitcoin-tx
bitcoin-util
bitcoin-wallet
bitcoind
test_bitcoin
AND
bitcoin
bitcoin-node
(Which I have to agree to that this is confusing)
Are you suggesting that the last 2 (in this case) would get the suffix -internal or -ipc ? Will the outcome than be that the bin directory (packaged) look like this:
bitcoin-cli
bitcoin-qt
bitcoin-tx
bitcoin-util
bitcoin-wallet
bitcoind
test_bitcoin
bitcoin-internal
bitcoin-node-internal
Sjors
commented at 5:33 pm on August 20, 2025:
member
@janb84 you can see for yourself with both (sudo) cmake --install build and a guix build that the bitcoin-node and bitcoin-gui binaries go into libexec (as is test_bitcoin). They’re only in build/bin (not in PATH) when you build from source without installing.
ryanofsky
commented at 5:40 pm on August 20, 2025:
contributor
It’s not the libexec directory that’s the issue , it’s the bin directory ( packaged ), or will the v30 packaged also contain a libexec directory?
Yep, the v30 package will contain a libexec/ directory alongside the bin/ directory. This was implemented in #31679. In binary releases following that PR, the libexec/ directory just contains a test_bitcoin file. (You can see the complete file list in https://github.com/bitcoin/bitcoin/blob/master/doc/files.md#installed-files.)
This PR adds two new files to binary releases: libexec/bitcoin-node and libexec/bitcoin-gui. These files could be renamed or given suffixes if it would help clarify their purpose.
TheBlueMatt
commented at 6:04 pm on August 20, 2025:
contributor
@TheBlueMatt I agree that having the experience be seamless for miners is the goal. I more meant for the purposes of SRI contributors and maybe a couple of brave miners who are willing to help with testing of Core and SV2, so we can avoid buggy code in releases.
After a few years of work, we’re now at a point where I can (with only modest confidence) say that we need the ability to do proper mining job building in Bitcoin Core in the next six months or so, implying it needs to ship in v30 or we’re gonna end up being mostly blocked on Bitcoin Core, rather than other work. That would be a pretty annoying situation to be in after @ryanofsky has been working on multiprocess for something like 5-10 years now and @Sjors has been trying to push for better job output from Core for at least a few years now as well.
janb84
commented at 7:28 pm on August 20, 2025:
contributor
ACKce7d94a492e61f2a43ea315e75be607d6aa71702
After reading all the comments, I fail to see the risk of the new binaries.
If you want to play safe, just run bitcoind and bitcoin-cli as normal.
if you want to test the more experimental options use the new binaries. eg. bitcoin (and you will accept the risks with that)
People who run bitcoin-core binaries are not novices, they will not get confused by some extra binaries (especially if contained in a separate dir). Novices will run Umbrel or something, shielding them from this change.
Maybe I’m underestimating the risk but I see a clear fallback path and more options for miners and the future. I also agree with not adding extra build options, that will complicate a complicated project even more.
Therefor my ACK for this PR.
(Also thanks for answering my previous questions)
ryanofsky
commented at 8:31 pm on August 20, 2025:
contributor
I’m in favor of multiprocess happening when it’s ready. We should have a point in time where we decide, as a project, to start moving everything to multiprocess. At that point, there will inevitably be a mental cost to users, but at least it can be explained then as “this gives you the benefit of process separation, being able to run wallets elsewhere, stop/start gui independently from node, …”, and it’ll be used and tested by all.
This sums up my opinion well. I just don’t see the rush. There’s a demand for sv2, and a low-risk way to ship that with very little fanfare. Multiprocess seems like a different concern.
I opened #33229 to avoid the need for users to know anything about multiprocess or reference it in any way. With that PR, users can simply add the -ipcbind option to the bitcoin command line, or add it to their bitcoin.conf file and it will be respected without also adding -m.
It was always annoying to have to type -m and I’d planned to make it optional, but your point that -m was referencing an implementation detail and set of features broader than IPC is a reason to drop it sooner.
With #33229 , the -ipcbind option will still be rejected by bitcoind and only accepted by bitcoin node. So users will still see there is a new binary with some new features that are a superset of bitcoind features. If we want to bitcoind to accept -ipcbind too, that would be possible as mentioned and come with the upsides and downsides already discussed.
ryanofsky
commented at 8:47 pm on August 20, 2025:
contributor
RFM?
I’ve been keeping my review summary comment up to date through this discussion and I am more confident this is ready to merge now, and plan to do this shortly.
I’ve just added the following new sections since my last update:
Review
janb84: current ack, high level review with questions and considerations of tradeoffs
Concerns
conflation of different multiprocess features
sipa and theuni pointed out it is bad to require users to explicitly use multiprocess flags or binaries just to use -ipcbind, because it confuses separate IPC listening and process separation features which are both enabled by the multiprocess dependency.
ryanofsky agrees and thinks [multiprocess: Don’t require bitcoin -m argument when IPC options are used #33229](https://github.com/bitcoin/bitcoin/pull/33229) would solve this by making the bitcoin -m option optional, since the -m option is the only place the multiprocess term should be currently visible to users.
user confusion from new files
theuni pointed out presence of the new bin/bitcoin file (already added in master) and libexec/bitcoin-node and libexec/bitcoin-gui files (added by this PR) in binary releases could cause confusion for users
ryanofsky pointed out the the effects of this should be mitigated by the readme file located in the root directory, and general familiarity with the convention of putting user-facing binaries bin/ but more documentation or different naming might solve potential problems
ryanofsky assigned ryanofsky
on Aug 20, 2025
achow101
commented at 9:00 pm on August 20, 2025:
member
ACKce7d94a492e61f2a43ea315e75be607d6aa71702
ryanofsky merged this
on Aug 20, 2025
ryanofsky closed this
on Aug 20, 2025
ryanofsky
commented at 9:42 pm on August 20, 2025:
contributor
Thanks to all who reviewed, tested, and commented on this. This has been very useful and should lead to a lot of good followups. I think all concerns can be addressed in future PRs even if they have not been addressed here and I think there are clear next steps to address all the issues listed in the review summary
TheCharlatan
commented at 9:45 pm on August 20, 2025:
contributor
0... 1Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": /src/bitcoin-core/depends/work/download/native_capnp-1.2.0/capnproto-cxx-1.2.0.tar.gz.temp: OK 2Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": Extracting native_capnp... 3Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": /src/bitcoin-core/depends/sources/capnproto-cxx-1.2.0.tar.gz: OK 4Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": Preprocessing native_capnp... 5Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": Configuring native_capnp... 6Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": -- The CXX compiler identification is GNU 9.4.0 7Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": -- Detecting CXX compiler ABI info 8Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": -- Detecting CXX compiler ABI info - failed 9Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": -- Check for working CXX compiler: /usr/bin/g++10Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": -- Check for working CXX compiler: /usr/bin/g++ - broken11Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": [31mCMake Error at /usr/local/share/cmake-3.29/Modules/CMakeTestCXXCompiler.cmake:60 (message):12Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": The C++ compiler13Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": 14Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": "/usr/bin/g++"15Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": 16Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": is not able to compile a simple test program.17Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": 18Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": It fails with the following output:19Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": 20Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": Change Dir: '/src/bitcoin-core/depends/work/build/x86_64-pc-linux-gnu/native_capnp/1.2.0-5b53bc4e344/CMakeFiles/CMakeScratch/TryCompile-waMe51'21Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": 22Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": Run Build Command(s): /usr/local/bin/cmake -E env VERBOSE=1 /usr/bin/make -f Makefile cmTC_7afd9/fast23Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": make[1]: Entering directory '/src/bitcoin-core/depends/work/build/x86_64-pc-linux-gnu/native_capnp/1.2.0-5b53bc4e344/CMakeFiles/CMakeScratch/TryCompile-waMe51'24Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": /usr/bin/make -f CMakeFiles/cmTC_7afd9.dir/build.make CMakeFiles/cmTC_7afd9.dir/build25Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": make[2]: Entering directory '/src/bitcoin-core/depends/work/build/x86_64-pc-linux-gnu/native_capnp/1.2.0-5b53bc4e344/CMakeFiles/CMakeScratch/TryCompile-waMe51'26Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": Building CXX object CMakeFiles/cmTC_7afd9.dir/testCXXCompiler.cxx.o27Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": /usr/bin/g++ -I/src/bitcoin-core/depends/x86_64-pc-linux-gnu/native/include -pipe -std=c++20 -fPIE -o CMakeFiles/cmTC_7afd9.dir/testCXXCompiler.cxx.o -c /src/bitcoin-core/depends/work/build/x86_64-pc-linux-gnu/native_capnp/1.2.0-5b53bc4e344/CMakeFiles/CMakeScratch/TryCompile-waMe51/testCXXCompiler.cxx28Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": g++: error: unrecognized command line option '-std=c++20'; did you mean '-std=c++2a'?29Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": make[2]: *** [CMakeFiles/cmTC_7afd9.dir/build.make:78: CMakeFiles/cmTC_7afd9.dir/testCXXCompiler.cxx.o] Error 130Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": make[2]: Leaving directory '/src/bitcoin-core/depends/work/build/x86_64-pc-linux-gnu/native_capnp/1.2.0-5b53bc4e344/CMakeFiles/CMakeScratch/TryCompile-waMe51'31Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": make[1]: *** [Makefile:127: cmTC_7afd9/fast] Error 232Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": make[1]: Leaving directory '/src/bitcoin-core/depends/work/build/x86_64-pc-linux-gnu/native_capnp/1.2.0-5b53bc4e344/CMakeFiles/CMakeScratch/TryCompile-waMe51'33Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": 34Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": 35Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": 36Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": 37Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": 38Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": CMake will not be able to correctly generate this project.39Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": Call Stack (most recent call first):40Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": CMakeLists.txt:2 (project)41Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": 42Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": [0m43Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": -- Configuring incomplete, errors occurred!44Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": make: *** [funcs.mk:345: /src/bitcoin-core/depends/x86_64-pc-linux-gnu/.native_capnp_stamp_configured] Error 145Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": ********************************************************************************46Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": Failed to build.47Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": To reproduce, run:48Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": python infra/helper.py build_image bitcoin-core49Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": python infra/helper.py build_fuzzers --sanitizer address --engine afl --architecture x86_64 bitcoin-core50Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64": ********************************************************************************51Finished Step [#3](/bitcoin-bitcoin/3/) - "compile-afl-address-x86_64"52ERROR
53ERROR: build step 3"gcr.io/cloud-builders/docker" failed: step exited with non-zero status: 1
I am still waiting for the oss-fuzz maintainers to give the ok for https://github.com/google/oss-fuzz/pull/13018, so I guess in the meantime one of the above mentioned workarounds could be used.
in
doc/dependencies.md:38
in
ce7d94a492
34@@ -35,6 +35,7 @@ Bitcoin Core requires one of the following compilers.
3536 | Dependency | Releases | Minimum required |
37 | --- | --- | --- |
38+| [Cap'n Proto](../depends/packages/capnp.mk) | [link](https://capnproto.org) | [0.7.1](https://capnproto.org/install.html) |
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-29 09:13 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
