#30437 ipc: add bitcoin-mine test program
| Reviewability | Reviews | Agreement | Size | |||||||
|---|---|---|---|---|---|---|---|---|---|---|
| #30437 ipc: add bitcoin-mine test program | Waiting on author | 0 (+3) | Blocked | 219 + 41 tests | ||||||
|
|
|
|
| ||||||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | ||||||||||
Goal
Adds an experimental `bitcoin-mine` executable and an accompanying functional test that connects to `bitcoin-node` over IPC and calls `interfaces::Mining` methods. Moves `interface_ui.cpp` from the node library to the common library so standalone utility binaries can report `InitError` and `InitWarning`. Adds a `mine` subcommand to the `bitcoin` wrapper executable.
Problem: multiprocess Bitcoin Core development and external mining implementations (such as Stratum v2 template providers) lack an end-to-end test binary to exercise and demonstrate the IPC mining interface outside the node process.
Evidence of importance:
- PR description: 'This is useful as discussed in https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2222418506 to work on getting mining code working in an external process.'
- PR description notes it is 'part of the process separation project (https://github.com/bitcoin/bitcoin/issues/28722)'.
Scope: Moves node/interface_ui.cpp into the bitcoin_common library, clearing a suppression in contrib/devtools/check-deps.sh. Adds a new internal executable bitcoin-mine built only when both IPC and daemon building are enabled.
Reviewability
Paused · Waiting on author
Open objections from w0xlt with no author reply for 30 days; review would repeat what has been asked.
Waiting on (from the thread read):
- author: address unaddressed review comments from w0xlt and push agreed build variable change 2026-10-02
Agreement
Blocked
objection that needs a change, nobody has replied (w0xlt)
Thread notes: Added c:5952564494 by sedited noting unaddressed review and suggesting converting to draft. This is a process comment and does not add a new claim; the PR remains waiting on the author.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| fanquake (member) | objection, scope | Shipping experimental unreleased binaries/maintenance scripts in Guix and release packages prematurely | fixed · fix pushed · author moved line to avoid merge conflict and did not add binary to Guix or release scripts | changed | yes | 2024-11-28: 'Nack. We can add things to Guix and the release process when they are actually part of a release.' Status from: 2024-12-03: ryanofsky: 'Updated 27072547bb22cdd2080d1014eba9c30bc9d47650 -> 99f2f335b6a8c2b5fe518a0c0056eb2a42c0496b' |
| Sjors (member) | objection, correctness | Functional test fails on macOS when tmpdir path exceeds 104 bytes | fixed · fix pushed · author applied suggested test fix | changed | yes | 2025-03-21: 'On macOS the limit is 104, which I run into when using a RAM disk with --tmpdir=/Volumes/RAMDisk/tmp.' Status from: 2025-03-25: ryanofsky: 'adding suggested test fix' |
| sedited (member) | suggestion, usefulness | Explain why having an in-tree example client is still needed given that external Rust and C++ clients exist in the SV2 project | accepted · sedited agreed that framing/renaming as a test and making compilation conditional would be enough | answered | yes | 2026-02-02: 'I think this motivation is a bit stale at this point. We both have a rust and c++ client for the existing mining interface under the umbrella of the sv2 project and it seems less pressing to have our ' Status from: 2026-04-20: sedited: 'Yes, I think that would be enough (next to making its compilation conditional).' |
| sedited (member) | objection, scope | Shipping test/example binary in binary release packages without dedicated build option | answered · author agreed it should get its own variable and not be part of binary releases, but change has not yet been pushed | changed | yes | 2026-02-02: 'I think this should get its own variable. If I read this correctly, we would ship this as part of a release with the current configuration?' Status from: 2026-02-02: ryanofsky: 'Yes good catch. This should get it's own variable and not be part of binary releases.' |
| fanquake (member) | objection, scope | Shipping unneeded experimental test binary in release builds | answered · author agreed that the binary should not ship in binary releases | changed | yes | 2026-02-02: 'I don't think this is something that needs to be shipped in releases.' Status from: 2026-02-02: ryanofsky: 'This should get it's own variable and not be part of binary releases.' |
| w0xlt (contributor) | objection, correctness | Crash when Mining::createNewBlock() returns nullptr during shutdown or interruption | open | changed | no | 2026-06-03: 'Mining::createNewBlock() can legally return nullptr when bitcoin-node is shutting down or mining is interrupted.' |
| w0xlt (contributor) | objection, correctness | Values below DEFAULT_MAX_TRIES cannot be passed to -maxtries as they are ignored and clamped to DEFAULT_MAX_TRIES | open | changed | no | 2026-06-03: 'With std::max(DEFAULT_MAX_TRIES, ...), max_tries can never be lower than DEFAULT_MAX_TRIES, so values like -maxtries=0 or -maxtries=100 are ignored and become 10000. Is this intentional ?' |
| w0xlt (contributor) | objection, correctness | Negative values passed to -maxtries wrap to large unsigned integers because GetIntArg() signed integer is converted to uint64_t | open | changed | no | 2026-06-03: 'Also, GetIntArg() returns a signed integer, but std::max<uint64_t>() converts it to unsigned. A negative value like -maxtries=-1 can become a huge uint64_t.' |
Support:
- Sjors (member) (Tested ACK, 2024-10-01) [tested]: Tested the standalone application
- Sjors (member) (ACK, 2024-10-01) [read]: Logging works now.
- Sjors (member) (ACK, 2024-11-28) [both: gen-manpages.py]: Tested that gen-manpages.py generates something useful and approved the functional test
- ismaelsadeeq (member) (Concept ACK, 2025-05-19) [read]: This can serve as a usage example for the interfaces.
- Sjors (member) (ACK, 2025-06-04): (no reason given) [not substantive]
- Sjors (member) (ACK, 2025-07-10): (no reason given) [not substantive]
- ismaelsadeeq (member) (ACK, 2025-07-29) [read]: re-ACK after clarification on test coverage scope and mining comments
- Sjors (member) (ACK, 2025-07-29): (no reason given) [not substantive]
- BrandonOdiwuor (contributor) (Tested ACK, 2025-08-12) [tested]: Tested IPC connection, tip hash retrieval, and valid block mining/submission on macOS in regtest mode without errors
- Sjors (member) (ACK, 2025-11-04) [read]: re-ACK after functional test refactoring
- Sjors (member) (ACK, 2026-01-05): (no reason given) [not substantive]
Participants:
- Sjors (member, support, 55 statements 2024-07-12 to 2026-06-30): Supported throughout development, reviewed repeatedly, provided bug fixes and rationale for IPC client test coverage
- hebasto (member, neutral, 1 statements 2024-10-07 to 2024-10-07): Clarified CMake installable_targets and maintenance targets scopes
- fanquake (member, objection, 2 statements 2024-11-28 to 2026-02-02): Objected to adding experimental test binary to release packages and guix
- maflcko (member, neutral, 4 statements 2025-03-25 to 2025-07-24): Reported typos and nits on python test framework and cmake rebase
- ismaelsadeeq (member, support, 21 statements 2025-05-19 to 2025-07-29): Concept ACK and reviewed, questioned expanding mining scope but withdrew objection after clarification
- sedited (member, objection, 6 statements 2025-06-18 to 2026-10-02): Questioned binary purpose, objected to shipping it in releases without a dedicated build variable, and suggested converting to draft
- BrandonOdiwuor (contributor, support, 1 statements 2025-08-12 to 2025-08-12): Tested ACK on macOS regtest
- ROLANDSIHOMBING1 (none, neutral, 1 statements 2025-10-13 to 2025-10-13): Off-topic comment
- w0xlt (contributor, objection, 3 statements 2026-06-03 to 2026-06-03): Pointed out missing nullptr check on Mining::createNewBlock() and argument parsing bugs for -maxtries
Dependencies
Enables:
- External mining process separation and Stratum v2 template provider integration (#29432)
Files
47 lines under test/bench/ci.
- src/bitcoin-mine.cpp +168/-0
- src/bitcoin-mine-res.rc +33/-0
- test/functional/feature_mine.py +33/-0
- src/CMakeLists.txt +14/-1
- contrib/devtools/check-deps.sh +1/-6
- test/functional/test_framework/util.py +5/-0
- src/bitcoin.cpp +3/-0
- CMakeLists.txt +1/-0
- contrib/devtools/gen-manpages.py +1/-0
- test/functional/test_runner.py +1/-0
Card
The short text the ranking pass compares across PRs.
This pull request adds an experimental bitcoin-mine executable and functional test to exercise the Mining IPC interface against bitcoin-node over a Unix domain socket. It provides a working example and test harness for out-of-process mining workflows, including querying chain tips, generating block templates, solving proof of work, and submitting solutions. This helps developers building external mining services, such as Stratum v2 template providers, verify and integrate IPC mining interfaces. The change also moves interface_ui.cpp from the node library to common, resolving an internal dependency issue so standalone binaries can use InitError and InitWarning. It is part of the multiprocess separation project and unblocks external mining architectures.
Data
thread read through 2026-10-02 (146 statements), head 154acb6507
labels: IPC
Categories
IPC / multiprocess
P2 · unblocks #29432
Provides an end-to-end test executable and IPC consumer needed to exercise and unblock external mining process separation in #29432. As noted in the description, this directly serves the process separation project (#28722) by validating interfaces::Mining across the process boundary.
Membership: Labeled IPC; adds bitcoin-mine binary connecting to bitcoin-node over IPC, updates bitcoin wrapper, and adds end-to-end functional test for IPC mining interface.
Factors: security/stability 0, bug 0, performance 0, user value 1, leverage 2
Mining
P3 · test coverage (ranking pass: Moved down: developer-facing tooling ranks last in this category's ordering, below the behavioural fixes it now sits behind.)
Adds developer-facing tooling and test coverage for block template mining over interfaces::Mining. Developer tooling is useful for downstream Stratum v2 integration but ranks behind template validity and interface stability.
Membership: Explicitly covered by the category definition ('the bitcoin-mine program, and the tests for these'); exercises interfaces::Mining methods.
Factors: security/stability 0, bug 0, performance 0, user value 1, leverage 2