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.
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
init: add exe name to bitcoind, bitcoin-node -version output to be able to distinguish these in tests231fba1f06
test: add tool_bitcoin to test bitcoin wrapper behavior75cdfb49b2
bitcoin: Make wrapper not require -m
Choose the right binary by default if an IPC option is specified
5a28d0b027
DrahtBot added the label
interfaces
on Aug 20, 2025
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.
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
DrahtBot added the label
CI failed
on Aug 20, 2025
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.
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.
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