ipc: add bitcoin-mine test program #30437

ryanofsky · 2024-07-12T10:37:28Z

Add bitcoin-mine executable to test connecting to the bitcoin-node process over a unix socket and calling interface::Mining methods.

This is useful as discussed in #29432 (comment) to work on getting mining code working in an external process.

This PR initially supported spawning a new bitcoin-node process if a connection could not be established with an existing process, but it was dropped to simplify the code. A branch building on this PR and adding that support is
pr/mine-detach.

This PR is part of the process separation project.

DrahtBot · 2024-07-12T10:37:31Z

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/30437.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Stale ACK	ismaelsadeeq, Sjors, BrandonOdiwuor

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)
#33142 (test: Run bench sanity checks in parallel with functional tests by maflcko)
#32297 (bitcoin-cli: Add -ipcconnect option by ryanofsky)
#31723 (qa debug: Add --debug_runs/-waitfordebugger [DRAFT] by hodlinator)
#30595 (kernel: Introduce initial C header API by TheCharlatan)
#29838 (Feature: Use different datadirs for different signets by BrandonOdiwuor)

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.

Sjors · 2024-07-12T11:47:37Z

Thanks. I'll try to get the Mining interface in good shape as well, which this can then be rebased on:

DrahtBot · 2024-07-12T12:32:20Z

🚧 At least one of the CI tasks failed.
_{Debug: https://github.com/bitcoin/bitcoin/runs/27368335663}

Hints

Make sure to run all tests locally, according to the documentation.

The failure may 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.

ryanofsky · 2024-07-12T12:35:56Z

Updated 8e93673 -> 3c0a13c (pr/mine.1 -> pr/mine.2, compare) filling in missing serialization for CBlockTemplate and BlockValidationState types, also fixing link errors for test programs that were causing CI tasks to fail.

ryanofsky · 2024-07-12T12:44:58Z

I'll work on dropping the spawn functionality from this PR next, and just having the bitcoin-mine program print an error when it can't connect to the node socket, instead of starting the node itself. Sjors suggested this in #29432 (comment):

If it helps getting IPC stuff merged quicker, it's fine by me if the miner can't spin up a node process (see e.g. Sjors@b6dfa5f)

This would simplify the PR by allowing 3 commits to be dropped:

Other things I'd like to do:

Write unit tests to cover the new code
Split up the last commit
Fix remaining CI errors
Add -ipconnect option to bitcoin-mine to allow specifying custom path for unix socket.

Sjors · 2024-07-12T13:40:59Z

filling in missing serialization for CBlockTemplate

You won't need that if you include #30440, though maybe it's better to wait until that's merged to avoid too much code churn.

ryanofsky · 2024-07-12T14:19:50Z

filling in missing serialization for CBlockTemplate

You won't need that if you include #30440, though maybe it's better to wait until that's merged to avoid too much code churn.

Yeah definitely some of that should not be needed. Some parts will probably be needed though. For example, the initial version of this PR did not support deserializing CTransaction or CBlock objects (because CTransaction does not have an Unserialize method, and also requires a version parameter to be set).

Sjors · 2024-07-12T14:25:55Z

I'll certainly need CBlock. It's probably good to support CTransaction as well, as it might allow for more efficiently fetching only missing transactions in a template update, but that would be a future optimisation.

Sjors · 2024-07-12T15:41:45Z

I opened draft PRs for all interface changes that I think I will need, see updated comment above: #30437 (comment)

If you can cherry-pick them all, I'll expand the demo app to simulate all the steps a Template Provider would do. That should reveal any remaining gaps in the interface.

Sjors · 2024-07-13T10:18:33Z

I wrote:

I'll expand the demo app to simulate all the steps a Template Provider would do

Once that works the next step for me would be to open an alternative to #29432 which runs the Template Provider in bitcoin-tp. I might wait with that until we ship a release with this interface enabled on the node side, ideally in bitcoind and bitcoin-qt rather than in seperate bitcoin-node and bitcoin-gui binaries.

I could then modify the guix build to only produce bitcoin-tp. This effectively turns it into a separate project that just happens to have access to the entire Bitcoin Core codebase.

That way the user can simply install and run a recent Bitcoin Core release in the manner they're familiar with. They would install bitcoin-tp separately and it should just work(tm), at most having to add a line to bitcoin.conf to turn IPC on.

If later on we can also turn libbitcoin_net into a library (see #30350), I could look into completely separating the codebase (I'd probably copy-paste the Bitcoin Core code and strip everything out I don't need, rather starting a new c++ project and build system from scratch). E.g. it could pull in libbitcoin_net, libbitcoin_util and whatever else it needs, add its own Transport subclass. This lets me reuse #29346, #30315 and #30332, but without having to merge them into Bitcoin Core itself.

This would be in parallel to the alternative approach of writing bitcoin-tp from scratch in rust reusing SRI code.

With both approaches it will take a while to reach feature parity with #29432 so I'll have to keep maintaining that too (using the same interface, but not as a separate process).

ryanofsky · 2024-07-16T19:24:46Z

I simplified PR as suggested so bitcoin-mine no longer spawns bitcoin-node if it can't connect to it. Instead it just prints an error message and suggests a command line to run bitcoin-node manually.

I also added a new commit to make the IPC code compatible with all the interfaces added in #30409, #30356, #30440, and #30443, so this PR is easier to test with those interfaces.

I still need to break up the main commit ("Add bitcoin-mine test program") and add unit tests for the new connecting, listening, and serialization code that's introduced.

Rebased 3c0a13c -> 4e1a434 (pr/mine.2 -> pr/mine.3, compare) with those changes.

For reference, the code that was dropped from this PR allowing bitcoin-mine to spawn a bitcoin-node process is in commits:

09621da multiprocess: Add IPC spawnProcess detach option
3d23332 multiprocess: Add IPC attach and detach methods
3c11262 multiprocess: Allow bitcoin-mine to spawn bitcoin-node

These changes would probably be useful in another context, but aren't needed for this test program.

re: #30437 (comment)

I think your ideas for packaging bitcoin-tp make sense. And maybe it would also be good to try to integrate IPC in bitcoind and bitcoin-qt executables instead of separate bitcoin-node and bitcoin-gui executables. Would have to think about how to implement this and what the tradeoffs would be.

Allows InitError and InitWarning to be used by other executables beside bitcoind and bitcoin-node.

See src/bitcoin-mine.cpp for usage information. Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>

Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

Suggested by Sjors Provoost <sjors@sprovoost.nl> in bitcoin#30437 (comment)

Add a MakeBasicInit() function so simpler standalone IPC clients like bitcoin-mine in bitcoin#30437 and bitcoin-cli in bitcoin#32297 that only initiate IPC connections without exposing any IPC interfaces themselves can to avoid needing to implement their own specialized interfaces::Init subclasses.

Expose ENABLE_IPC build option to function tests so new tests for bitcoin-mine in bitcoin#30437 and bitcoin-cli in bitcoin#32297 can test IPC-only features.

Set new `BitcoinTestFramework.binary_paths.bitcoin_bin` property with path to the `bitcoin` wrapper binary. This allows new tests for `bitcoin-mine` in bitcoin#30437 and `bitcoin-cli` in bitcoin#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`

Suggested by Sjors Provoost <sjors@sprovoost.nl> in bitcoin#30437 (comment)

ryanofsky · 2025-08-18T16:55:12Z

Rebased e1f139b -> 1e8a832 (pr/mine.30 -> pr/mine.31, compare due to conflict with #31679. Also make some framework improvements to simplify the test and avoid duplicating code with #32297 and #33201

Expose ENABLE_IPC build option to function tests so new tests for bitcoin-mine in bitcoin#30437 and bitcoin-cli in bitcoin#32297 can test IPC-only features.

Set new `BitcoinTestFramework.binary_paths.bitcoin_bin` property with path to the `bitcoin` wrapper binary. This allows new tests for `bitcoin-mine` in bitcoin#30437 and `bitcoin-cli` in bitcoin#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`

With this change, tests can specify `self.extra_init = [{ipcbind: True}]` to start a node listening on an IPC socket, instead of needing to choose which node binary to invoke and what `self.extra_args=[["-ipcbind=..."]]` value to pass to it. The eliminates boilerplate code bitcoin#30437 (interface_ipc_mining.py), bitcoin#32297 (interface_ipc_cli.py), and bitcoin#33201 (interface_ipc.py) previously needed in their test setup.

DrahtBot · 2025-08-18T18:28:14Z

🚧 At least one of the CI tasks failed.
_{Task lint: https://github.com/bitcoin/bitcoin/runs/48325274608}
_{LLM reason (✨ experimental): The CI failure is caused by a Python lint error due to a missing explicit type annotation in the test framework code.}

Hints

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.

Expose ENABLE_IPC build option to function tests so new tests for bitcoin-mine in bitcoin#30437 and bitcoin-cli in bitcoin#32297 can test IPC-only features.

Set new `BitcoinTestFramework.binary_paths.bitcoin_bin` property with path to the `bitcoin` wrapper binary. This allows new tests for `bitcoin-mine` in bitcoin#30437 and `bitcoin-cli` in bitcoin#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`

With this change, tests can specify `self.extra_init = [{ipcbind: True}]` to start a node listening on an IPC socket, instead of needing to choose which node binary to invoke and what `self.extra_args=[["-ipcbind=..."]]` value to pass to it. The eliminates boilerplate code bitcoin#30437 (interface_ipc_mining.py), bitcoin#32297 (interface_ipc_cli.py), and bitcoin#33201 (interface_ipc.py) previously needed in their test setup.

Expose ENABLE_IPC build option to function tests so new tests for bitcoin-mine in bitcoin#30437 and bitcoin-cli in bitcoin#32297 can test IPC-only features.

Set new `BitcoinTestFramework.binary_paths.bitcoin_bin` property with path to the `bitcoin` wrapper binary. This allows new tests for `bitcoin-mine` in bitcoin#30437 and `bitcoin-cli` in bitcoin#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`

With this change, tests can specify `self.extra_init = [{ipcbind: True}]` to start a node listening on an IPC socket, instead of needing to choose which node binary to invoke and what `self.extra_args=[["-ipcbind=..."]]` value to pass to it. The eliminates boilerplate code bitcoin#30437 (interface_ipc_mining.py), bitcoin#32297 (interface_ipc_cli.py), and bitcoin#33201 (interface_ipc.py) previously needed in their test setup.

…coin` binary 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 bitcoin#30437 and `bitcoin-cli` in bitcoin#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`

Set new `BitcoinTestFramework.binary_paths.bitcoin_bin` property with path to the `bitcoin` wrapper binary. This allows new tests for `bitcoin-mine` in bitcoin#30437 and `bitcoin-cli` in bitcoin#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`

With this change, tests can specify `self.extra_init = [{ipcbind: True}]` to start a node listening on an IPC socket, instead of needing to choose which node binary to invoke and what `self.extra_args=[["-ipcbind=..."]]` value to pass to it. The eliminates boilerplate code bitcoin#30437 (interface_ipc_mining.py), bitcoin#32297 (interface_ipc_cli.py), and bitcoin#33201 (interface_ipc.py) previously needed in their test setup.

Sjors · 2025-08-25T13:47:51Z

test_framework.py:565: error: Need type annotation for "extra_init"

DrahtBot changed the title ~~multiprocess: add bitcoin-mine test program~~ multiprocess: add bitcoin-mine test program Jul 12, 2024

ryanofsky mentioned this pull request Jul 12, 2024

Stratum v2 Template Provider (take 3) #29432

Closed

24 tasks

ryanofsky force-pushed the pr/mine branch from 8e93673 to 3c0a13c Compare July 12, 2024 12:32

DrahtBot added the CI failed label Jul 12, 2024

Sjors mentioned this pull request Jul 12, 2024

[WIP] Add getCoinbaseMerklePath() to Mining interface #30441

Closed

DrahtBot mentioned this pull request Jul 12, 2024

Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block #30409

Merged

Sjors mentioned this pull request Jul 12, 2024

Introduce waitFeesChanged() mining interface #30443

Closed

1 task

DrahtBot mentioned this pull request Jul 12, 2024

kernel, logging: Pass Logger instances to kernel objects #30342

Draft

This was referenced Jul 13, 2024

[DO NOT MERGE] cmake: Migrate CI scripts to CMake-based build system -- WIP #29790

Closed

Have createNewBlock() return a BlockTemplate interface #30440

Merged

ryanofsky force-pushed the pr/mine branch from 3c0a13c to 4e1a434 Compare July 16, 2024 18:51

DrahtBot removed the CI failed label Jul 16, 2024

This was referenced Jul 17, 2024

build: Introduce CMake-based build system #30454

Merged

refactor: add coinbase constraints to BlockAssembler::Options #30356

Merged

ryanofsky and others added 5 commits August 18, 2025 10:19

interface_ui: move from node to common library

894297c

Allows InitError and InitWarning to be used by other executables beside bitcoind and bitcoin-node.

ipc: Add bitcoin-mine test program

33217f3

See src/bitcoin-mine.cpp for usage information. Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>

test: add interface_ipc_mining.py test calling bitcoin-mine

bb033ef

Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>

bitcoin-mine: Extend example to mine a block

295f9c2

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

ipc: Add bitcoin-mine to bitcoin wrapper interface

1e8a832

Suggested by Sjors Provoost <sjors@sprovoost.nl> in bitcoin#30437 (comment)

ryanofsky force-pushed the pr/mine branch from e1f139b to 1e8a832 Compare August 18, 2025 16:52

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Aug 18, 2025

ipc: Add bitcoin-mine to bitcoin wrapper interface

047a51b

Suggested by Sjors Provoost <sjors@sprovoost.nl> in bitcoin#30437 (comment)

DrahtBot added CI failed and removed Needs rebase labels Aug 18, 2025

This was referenced Aug 20, 2025

qa debug: Add --debug_runs/-waitfordebugger [DRAFT] #31723

Draft

Feature: Use different datadirs for different signets #29838

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ipc: add bitcoin-mine test program #30437

ipc: add bitcoin-mine test program #30437

ryanofsky commented Jul 12, 2024 •

edited

Loading

Uh oh!

DrahtBot commented Jul 12, 2024 •

edited

Loading

Uh oh!

Sjors commented Jul 12, 2024 •

edited

Loading

Uh oh!

DrahtBot commented Jul 12, 2024

Uh oh!

ryanofsky commented Jul 12, 2024

Uh oh!

ryanofsky commented Jul 12, 2024 •

edited

Loading

Uh oh!

Sjors commented Jul 12, 2024 •

edited

Loading

Uh oh!

ryanofsky commented Jul 12, 2024

Uh oh!

Sjors commented Jul 12, 2024

Uh oh!

Sjors commented Jul 12, 2024 •

edited

Loading

Uh oh!

Sjors commented Jul 13, 2024 •

edited

Loading

Uh oh!

ryanofsky commented Jul 16, 2024

Uh oh!

ryanofsky commented Aug 18, 2025

Uh oh!

DrahtBot commented Aug 18, 2025

Uh oh!

Sjors commented Aug 25, 2025

Uh oh!

Uh oh!

ipc: add bitcoin-mine test program #30437

Are you sure you want to change the base?

ipc: add bitcoin-mine test program #30437

Conversation

ryanofsky commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

Sjors commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jul 12, 2024

Uh oh!

ryanofsky commented Jul 12, 2024

Uh oh!

ryanofsky commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sjors commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanofsky commented Jul 12, 2024

Uh oh!

Sjors commented Jul 12, 2024

Uh oh!

Sjors commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sjors commented Jul 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanofsky commented Jul 16, 2024

Uh oh!

ryanofsky commented Aug 18, 2025

Uh oh!

DrahtBot commented Aug 18, 2025

Uh oh!

Sjors commented Aug 25, 2025

Uh oh!

Uh oh!

ryanofsky commented Jul 12, 2024 •

edited

Loading

DrahtBot commented Jul 12, 2024 •

edited

Loading

Sjors commented Jul 12, 2024 •

edited

Loading

ryanofsky commented Jul 12, 2024 •

edited

Loading

Sjors commented Jul 12, 2024 •

edited

Loading

Sjors commented Jul 12, 2024 •

edited

Loading

Sjors commented Jul 13, 2024 •

edited

Loading