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

pull ryanofsky wants to merge 4 commits into bitcoin:master from ryanofsky:pr/ipc-auto changing 14 files +160 −9
  1. ryanofsky commented at 8:08 pm on August 20, 2025: contributor
    sipa and theuni in #31802 pointed out that users shouldn’t be exposed to multiprocess implementation details just to use IPC features, so current need to specify the bitcoin -m option in conjunction with -ipcbind could be seen as a design mistake and not just a usage inconvenience. This is easy to fix by having the bitcoin wrapper respect IPC settings. Was planning to do this anyway, but this provides a good excuse to do it now.
  2. test: Provide path to `bitcoin` binary
    Set new `BitcoinTestFramework.binary_paths.bitcoin_bin` property with path to
    the `bitcoin` wrapper binary. This allows new tests for `bitcoin-mine` in
    #30437 and `bitcoin-cli` in #32297 to find the `bitcoin` binary and call
    `bitcoin -m` to start nodes with IPC support. This way the new tests can run
    whenever the ENABLE_IPC build option is enabled, instead of only running when
    the `BITCOIN_CMD` environment variable is set to `bitcoin -m`
    83c8d62b3b
  3. init: add exe name to bitcoind, bitcoin-node -version output to be able to distinguish these in tests 231fba1f06
  4. test: add tool_bitcoin to test bitcoin wrapper behavior 75cdfb49b2
  5. bitcoin: Make wrapper not require -m
    Choose the right binary by default if an IPC option is specified
    5a28d0b027
  6. DrahtBot added the label interfaces on Aug 20, 2025
  7. DrahtBot commented at 8:08 pm on August 20, 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/33229.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK Sjors

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #33142 (test: Run bench sanity checks in parallel with functional tests by maflcko)
    • #32297 (bitcoin-cli: Add -ipcconnect option by ryanofsky)
    • #28802 (ArgsManager: support subcommand-specific options by ajtowns)

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

    LLM Linter (✨ experimental)

    Possible typos and grammar issues:

    • In tool_bitcoin.py docstring:
      “Test the IPC interface.” -> “Test the IPC interface.” [extra space between “IPC” and “interface”]
    • In tool_bitcoin.py log message:
      “Ensure bitcoin -m does accept ipcbind” -> “Ensure bitcoin -m does accept -ipcbind” [missing dash on “-ipcbind”]

    drahtbot_id_4_m

  8. DrahtBot added the label CI failed on Aug 20, 2025
  9. DrahtBot commented at 9:13 pm on August 20, 2025: contributor

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

    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.

  10. Sjors commented at 7:09 am on August 21, 2025: member

    Concept ACK

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

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


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-29 15:13 UTC

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