build: Enable ENABLE_IPC option by default #33190

pull ryanofsky wants to merge 2 commits into bitcoin:master from ryanofsky:pr/ipc-default changing 13 files +44 −28
  1. ryanofsky commented at 8:33 pm on August 14, 2025: contributor

    This change toggles -DENABLE_IPC default value from OFF to ON so IPC features will be compiled by default in source builds.

    The main feature this provides is an -ipcbind option that lets the node listen on a unix socket and expose a mining interface to support Stratum v2 mining software.

    This change doesn’t affect bitcoind or bitcoin-qt since IPC features are implemented in separate binaries accessible through a new bitcoin command.


    This PR is a minimal change that just enables IPC by default in cmake. #31802 is more comprehensive and additionally enables IPC by default in depends, uses it in more CI jobs, and updates documentation. If you like this PR, you will love #31802!

    I prefer the approach in #31802 of enabling IPC in source builds, release binaries, and CI at the same time, without an intermediate state where source and binary releases are different. But there was a comment in todays irc meeting “i think it’d be nice if we had IPC enabled in from-source builds by default for a while before we add it to releases” and there have been similar comments previously, so maybe this PR will be of interest if we want to take that approach.


    This PR is part of the process separation project.

  2. cmake: Enable ENABLE_IPC option by default 7f2d161d5c
  3. doc: update build and dependencies docs for IPC
    OpenBSD does not have this package, so recommend building from
    source for now.
    5a34156ab6
  4. DrahtBot added the label Build system on Aug 14, 2025
  5. DrahtBot commented at 8:33 pm on August 14, 2025: contributor

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

    Code Coverage & Benchmarks

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

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK TheCharlatan, Sjors
    Stale ACK ismaelsadeeq, josibake, sipa

    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)
    • #31802 (Add bitcoin-{node,gui} to release binaries for IPC by Sjors)

    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.

  6. TheCharlatan commented at 8:37 pm on August 14, 2025: contributor
    Concept ACK
  7. DrahtBot added the label CI failed on Aug 14, 2025
  8. ryanofsky force-pushed on Aug 15, 2025
  9. DrahtBot removed the label CI failed on Aug 15, 2025
  10. Sjors commented at 6:44 am on August 15, 2025: member

    Concept ACK. I prefer #31802 but that’s a simple rebase after this lands.

    You’ll probably want to include 71f29d4fa90aaeb6472b3ce9d4f4e97f85ed487b from that PR here to update the build docs.

  11. ismaelsadeeq commented at 1:44 pm on August 15, 2025: member

    ACK 732c134bfc1208377f449e5956e01dd8b78d73d9 as well

    This is more cautious I think.

  12. DrahtBot requested review from Sjors on Aug 15, 2025
  13. DrahtBot requested review from TheCharlatan on Aug 15, 2025
  14. ryanofsky commented at 1:46 pm on August 15, 2025: contributor

    I want to elaborate on why I prefer #31802 to this change.

    The idea of enabling IPC in source builds before binary releases sounds nice at first, but more thought, stops really making sense.

    For one thing, this is not how we’ve rolled out any other features I’m aware of. Most features aren’t toggled by build flags, but theoretically any new feature could be gated by a build flag, enabled first in source builds, and later in binary releases. We don’t roll out features this way because it’s cumbersome and offers no clear benefits. I don’t think IPC is different from other features in this respect. Even looking at the last feature that was toggled by a build flag, SQLite, there was never a point where it was enabled in source builds but disabled in depends and binary releases (see #19077).

    I think some people like the idea of turning IPC on in source builds but leaving it off in depends and binary releases because they think it would lead to more feature testing before inclusion in a binary release. But IPC has already been enabled in the “dev-mode” CMake preset for as long as that preset has existed. And since the only real functionality added by ENABLE_IPC is the third-party mining interface, I doubt changing its default will meaningfully increase testing, unless there’s a user or developer who (1) is willing to write or download a program that connects to the interface, (2) builds Bitcoin Core from source with default CMake options, but (3) is unwilling to manually enable ENABLE_IPC. That seems like a narrow group.

    The only possible benefit I see from changing the default to ON in source builds is that it would confirm IPC builds and tests work on more platforms and with a wider variety of build options beyond what CI covers. That has some value, but I don’t see build flag compatibility or platform-specific bugs as major concerns, so putting the feature in a half-enabled state just to get a few more builds and automated tests doesn’t seem appealing.

    I have no objection to this change if it helps us move forward. I just want to explain why I don’t think it’s very useful on its own, and why I dropped it from #31741 after initially implementing it there.

  15. josibake commented at 2:05 pm on August 15, 2025: member

    ACK https://github.com/bitcoin/bitcoin/pull/33190/commits/732c134bfc1208377f449e5956e01dd8b78d73d9

    The only possible benefit I see from changing the default to ON in source builds is that it would confirm IPC builds and tests work on more platforms and with a wider variety of build options beyond what CI covers

    While I also prefer #31802 (and largely agree with everything you said in the rest of your comment), I think there is one additional benefit, namely signalling progress to external projects. I do think this PR would be most impactful if it also included a release note for v30 to inform users this is now on by default when compiling from source, with the expectation it will be fully turned on in the v31 release.

  16. ryanofsky commented at 2:25 pm on August 15, 2025: contributor

    I think there is one additional benefit, namely signalling progress

    Yes that’s a great point, and reminds me that even without this PR or #33190 we could add a release note saying that in 30.0 the Mining interface has new checkBlock and waitNext methods (see git diff v29.0..origin/master src/ipc/capnp/mining.capnp) and the WITH_MULTIPROCESS cmake option has been renamed to ENABLE_IPC.

  17. sipa commented at 1:24 pm on August 18, 2025: member

    ACK 732c134bfc1208377f449e5956e01dd8b78d73d9. See also tests added in #33201.

    I think #31802 should be seen as a successor to this PR, regardless of which release it goes into.

  18. fanquake commented at 1:52 pm on August 18, 2025: member

    You’ll probably want to include https://github.com/bitcoin/bitcoin/commit/71f29d4fa90aaeb6472b3ce9d4f4e97f85ed487b from that PR here to update the build docs.

    You’ll also need to update the valgrind and valgrind fuzz CI jobs (run in qa-assets / elsewhere), otherwise they wont work, after this is merged.

  19. ryanofsky force-pushed on Aug 18, 2025
  20. ryanofsky commented at 5:32 pm on August 18, 2025: contributor
    Updated 732c134bfc1208377f449e5956e01dd8b78d73d9 -> 5a34156ab65ac7460c26141bb29651477e000d19 (pr/ipc-default.3 -> pr/ipc-default.4, compare applying fanquake’s suggestion #33190 (comment) to update valgrind jobs, and sjors suggestion #33190 (comment) to cherry-pick documentation from #31802.
  21. ryanofsky commented at 6:48 pm on August 18, 2025: contributor

    re: #33190 (comment)

    I think #31802 should be seen as a successor to this PR, regardless of which release it goes into. @sipa if you are saying you see this PR as a prerequisite to #31802, that seems ok, but I would be interested to know why.

    As far as I can tell, there haven’t been any other features in bitcoin core enabled by default in non-depends builds but disabled by default by default in depends builds and releases, so it seems like this PR leaves the build configuration in a strange state without a justification for what it is trying to achieve.

    I could imagine it making sense to enable a feature in ordinary source builds that is disabled in depends builds if the feature changed default behavior of the node or wallet, and we wanted to opt developers into the change before rolling it out more widely. But the IPC feature features are off by default so flipping this switch isn’t opting anyone into actually using IPC features. All it is doing is compiling some extra files and enabling some extra automated tests in the non-depends build. Which is nice, but does not seem like a compelling reason to leave this feature in an unusual half-enabled state.

    Basically the more I think about this PR the less it seems to make sense to me, and I think I’d like to close it, but no problem keeping it open if others think this is a good idea.

  22. achow101 commented at 7:48 pm on August 18, 2025: member

    Thinking on this further, I think I agree that there isn’t a good reason to be making this change. When building from source, we already don’t enable several optional features, including the GUI, that we ship in the release binaries. Since the switch to cmake, having the dependencies installed doesn’t enable those features automatically, they still need to be enabled explicitly during configuration. Developers should probably be using the dev-mode preset, or similar, which enables a bunch of the optional features so this shouldn’t make a difference to them.

    Since this PR defaults the option to on, it isn’t really an optional feature anymore. It can be turned off, but if you just do the default configuration, you will need to have capnproto installed.

  23. BrandonOdiwuor commented at 9:48 am on August 19, 2025: contributor

    Verified that ENABLE_IPC is enabled by default and that both bitcoin-node and bitcoin-gui (when built with -DBUILD_GUI=ON) are included in build/bin.

    0$ cmake -B build -DENABLE_GUI=ON
    

    (system: macOS 15.6)

  24. BrandonOdiwuor commented at 9:49 am on August 19, 2025: contributor
    I still don’t understand the value of enabling these binaries in the build tree without also including them in release artifact compared to https://github.com/bitcoin/bitcoin/pull/31802
  25. ryanofsky commented at 2:00 pm on August 19, 2025: contributor
    Thanks everybody who commented! Will close this in favor of #31802. If anybody thinks this should be reopened, I’d be curious and can reopen it. But balance of opinion seems to be in favor of not doing this.
  26. ryanofsky closed this on Aug 19, 2025

  27. ryanofsky commented at 4:59 pm on August 19, 2025: contributor

    Note: achow101 commented against enabling ENABLE_IPC by default in #31802#pullrequestreview-3129705803 and sipa commented in favor in #31802 (comment), so more discussion about this is happening in the other PR.

    I think the difference comes down to short vs. long term perspective. In the short term it doesn’t make a huge amount of sense to turn this one feature on by default while comparable features are turned off. But in the longer term it does not make sense to support and maintain the IPC feature at all if it is not used for more things and turned on by default.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-08-21 03:12 UTC

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