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

Sjors · 2025-02-05T16:04:00Z

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
Various changes to CI: switching on ENABLE_IPC on in most configurations and using bitcoin-node binary 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)
macOS and Linux installation instructions are updated to install capnp by default

This PR doesn't need to do all of 3 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 and OpenBSD are not supported yet, the latter due to a fairly trivial upstream issue. Therefore for those platforms ENABLE_IPC is off by default.

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.

See #31756 for discussion of when this should happen. Supersedes #30975.

Guix hashes:

...

DrahtBot · 2025-02-05T16:04:04Z

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

Code Coverage & Benchmarks

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	TheCharlatan
Stale ACK	ryanofsky

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32586 (ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task by maflcko)
#32262 (build: Restore cross-compilation for Android by hebasto)
#32162 (depends: Switch from multilib to platform-specific toolchains by hebasto)
#31375 (multiprocess: Add bitcoin wrapper executable by ryanofsky)
#31349 (ci: detect outbound internet traffic generated while running tests by vasild)
#30595 (kernel: Introduce initial C header API by TheCharlatan)

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

LLM Linter (✨ experimental)

Possible typos and grammar issues:

sipa · 2025-02-07T21:03:06Z

Some chatter from IRC:

17:21:41 < darosior> It might be confusing to release both a bitcoin-wallet utility and a bitcoin-wallet binary as part of multiprocess?
17: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 
                     30.0, which means if we want to deprecate the bitcoin-wallet utility name we should do it.. now?
17:33:31 < sipa> bitcoin-wallet-util ?
...
23:38:57 < _aj_> darosior: bitcoin-wallet-process, bitcoin-gui-process, etc? it's multi *-process!
03:04:34 < Sjors[m]> darosior: at the moment there is no wallet binary if were to enable multiprocess. That won't happen until #19460.
...
03:05:18 < Sjors[m]> Or maybe already in #10102
...
03:05:50 < Sjors[m]> In any case #31802 only adds bitcoin-node and bitcoin-gui.
...
03:07:19 < Sjors[m]> Though if we want to rename the utility eventually, it's always better to do it early.

Sjors · 2025-02-08T10:58:20Z

@sipa I opened #31827

Reported by Sjors Provoost <[email protected]> bitcoin/bitcoin#31802 (comment) /ci_container_base/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] /ci_container_base/include/mp/proxy-io.h:336:18: 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] /ci_container_base/include/mp/util.h:186:12: 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] https://cirrus-ci.com/task/6187773452877824?logs=ci#L4712

ryanofsky · 2025-05-12T18:10:27Z

re: Sjors #31802 (comment)

@ryanofsky CI complains about std::move in libmultiprocess: https://cirrus-ci.com/task/6187773452877824, e.g.:

Thanks for catching this. I implemented a fix for that error and the other errors in bitcoin-core/libmultiprocess#172

If you wanted to test the fix here you could do:

git fetch https://github.com/bitcoin-core/libmultiprocess 8d8e9f65f1d9faaafe91ca90910c1f96da9d094d
git subtree merge --prefix=src/ipc/libmultiprocess --squash 8d8e9f65f1d9faaafe91ca90910c1f96da9d094d

I'll also try to open a subtree update PR in the next week or so that this PR could be rebased on.

Sjors · 2025-05-13T09:03:54Z

@ryanofsky I added your libmultiprocess patch (and rebased).

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 and OpenBSD.

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.

Install capnp on non-depends CI jobs. Use the bitcoin-node binary in the macOS native non-depends job. Co-authored-by: Ryan Ofsky <[email protected]>

Sjors · 2025-05-15T16:52:00Z

Rebased after #31895. Also marking draft given the dependency on bitcoin-core/libmultiprocess#172.

Reported by Sjors Provoost <[email protected]> bitcoin/bitcoin#31802 (comment) /ci_container_base/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] /ci_container_base/include/mp/proxy-io.h:336:18: 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] /ci_container_base/include/mp/util.h:186:12: 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] https://cirrus-ci.com/task/6187773452877824?logs=ci#L4712

a5bff37ad8 clang-tidy: Suppress bitcoin-nontrivial-threadlocal error e922bb8f6a clang-tidy: Fix bugprone-move-forwarding-reference error c1e8c1a028 clang-tidy: Fix bugprone-move-forwarding-reference errors 0d8012f656 Merge bitcoin-core/libmultiprocess#165: clang-tidy: fix warnings introduced in version 19 aa19285303 use ranges transform a78137ca73 make member function const ca3226ec8a replace custom tuple unpacking code with `std::apply` 949fe85fc9 replace SFINAE trick with `if constexpr` git-subtree-dir: src/ipc/libmultiprocess git-subtree-split: a5bff37ad8f5630a738eb1b0bfa60cf01bedbcdf

…ipc-yea

DrahtBot · 2025-05-22T18:23:21Z

🐙 This pull request conflicts with the target branch and needs rebase.

This was referenced Feb 5, 2025

ci: build multiprocess on most jobs #30975

Closed

Stratum v2 Template Provider via IPC interface (multiprocess) Sjors/bitcoin#48

Open

ryanofsky mentioned this pull request Feb 3, 2025

Multiprocess tracking issue #28722

Open

86 tasks

DrahtBot added the Needs rebase label Feb 6, 2025

Sjors mentioned this pull request Feb 7, 2025

multiprocess: Add libmultiprocess git subtree #31741

Merged

Sjors force-pushed the 2025/02/ipc-yea branch 2 times, most recently from ef1e5b0 to 527a77e Compare February 7, 2025 19:55

DrahtBot mentioned this pull request Feb 8, 2025

guix: remove test-security/symbol-check scripts #31818

Merged

DrahtBot mentioned this pull request Feb 8, 2025

build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON #31665

Open

This comment was marked as spam.

Sign in to view

bitcoin deleted a comment from onlinesipahimithu Feb 9, 2025

Sjors force-pushed the 2025/02/ipc-yea branch 2 times, most recently from f43ff56 to 63dffe1 Compare February 13, 2025 19:03

DrahtBot removed the Needs rebase label Feb 13, 2025

This was referenced Feb 14, 2025

ci: switch MSAN to use prebuilt Clang binaries #31850

Closed

cmake: Do not modify CMAKE_TRY_COMPILE_TARGET_TYPE globally #31662

Merged

[POC] cmake: Introduce LLVM's Source-based Code Coverage reports #31394

Closed

Sjors force-pushed the 2025/02/ipc-yea branch from 63dffe1 to 1cb3ff9 Compare February 17, 2025 15:51

Sjors force-pushed the 2025/02/ipc-yea branch from 8b73630 to 5032cac Compare May 7, 2025 14:52

DrahtBot removed the Needs rebase label May 7, 2025

ryanofsky mentioned this pull request May 12, 2025

clang-tidy: fix warnings introduced in version 19 bitcoin-core/libmultiprocess#165

Merged

ryanofsky mentioned this pull request May 12, 2025

refactor: fix warnings from clang-tidy-20 and bitcoin-tidy bitcoin-core/libmultiprocess#172

Open

Sjors force-pushed the 2025/02/ipc-yea branch from 5032cac to 27956b3 Compare May 13, 2025 09:03

DrahtBot added Needs rebase and removed CI failed labels May 13, 2025

Sjors and others added 6 commits May 15, 2025 18:48

build: add bitcoin-{node,gui} to Maintenance.cmake

ea90b25

ci: build one depends job without multiprocess

3a8d2cb

ci: use bitcoin-node for one depends job

da951ce

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.

cmake: enable ENABLE_IPC option by default

d94d295

Install capnp on non-depends CI jobs. Use the bitcoin-node binary in the macOS native non-depends job. Co-authored-by: Ryan Ofsky <[email protected]>

doc: update deps install notes for multiprocess

6c8cccb

Sjors force-pushed the 2025/02/ipc-yea branch from 27956b3 to 7163d44 Compare May 15, 2025 16:51

Sjors marked this pull request as draft May 15, 2025 16:53

DrahtBot removed the Needs rebase label May 15, 2025

Sjors added 2 commits May 16, 2025 09:31

Merge commit '62564a5b1cac95f3c4bafc03d97b56146671b2d1' into 2025/02/…

9622659

…ipc-yea

Sjors force-pushed the 2025/02/ipc-yea branch from 7163d44 to 9622659 Compare May 16, 2025 07:31

DrahtBot added CI failed and removed CI failed labels May 16, 2025

DrahtBot mentioned this pull request May 22, 2025

ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task #32586

Merged

DrahtBot added the Needs rebase label May 22, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

Sjors commented Feb 5, 2025 •

edited

Loading

Uh oh!

DrahtBot commented Feb 5, 2025 •

edited

Loading

Uh oh!

sipa commented Feb 7, 2025

Uh oh!

Sjors commented Feb 8, 2025

Uh oh!

This comment was marked as spam.

Uh oh!

ryanofsky commented May 12, 2025

Uh oh!

Sjors commented May 13, 2025

Uh oh!

Sjors commented May 15, 2025 •

edited

Loading

Uh oh!

DrahtBot commented May 22, 2025

Uh oh!

Uh oh!

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

Are you sure you want to change the base?

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

Uh oh!

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

Uh oh!

sipa commented Feb 7, 2025

Uh oh!

Sjors commented Feb 8, 2025

Uh oh!

This comment was marked as spam.

Uh oh!

ryanofsky commented May 12, 2025

Uh oh!

Sjors commented May 13, 2025

Uh oh!

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

Uh oh!

Uh oh!

DrahtBot commented May 22, 2025

Uh oh!

Uh oh!

Sjors commented Feb 5, 2025 •

edited

Loading

DrahtBot commented Feb 5, 2025 •

edited

Loading

Sjors commented May 15, 2025 •

edited

Loading