mining: Break compatibility with existing IPC mining clients #34568

ryanofsky commented at 4:10 AM on February 12, 2026: contributor

This PR increments the field number of the Init.makeMining method and makes the old makeMining method return an error, so IPC mining clients not using the latest schema file will get an error and not be able to access the Mining interface.

Normally, there shouldn't be a need to break compatibility this way, but the mining interface has evolved a lot since it was first introduced, with old clients using the original methods less stable and performant than newer clients. So now is a good time to introduce a cutoff, drop deprecated methods, and stop supporting old clients which can't function as well.

Bumping the field number is also an opportunity to make other improvements that would be awkward to implement compatibly:

Making Cap'n Proto default parameter and field values match default values of corresponding C++ methods and structs.
Adding missing Context parameters to Mining.createNewBlock and checkBlock methods so these methods will be executed on separate execution threads and not block the Cap'n Proto event loop thread.

More details about these changes are in the commit messages.

rpc refactor: stop using deprecated getCoinbaseCommitment method

There should be no change in behavior

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

df53a3e5ec

test framework: expand expected_stderr, expected_ret_code options

Allow `expected_stderr` option passed to `wait_until_stopped` and
`is_node_stopped` helper functions to be a regex pattern instead of just a
fixed string.

Allow `expected_ret_code` be list of possible exit codes instead of a single
error code to handle the case where exit codes vary depending on OS and libc.

b970cdf20f

ipc test: add workaround to block_reserved_weight exception test

libmultiprocess currently handles uncaught exceptions from IPC methods badly
when an `mp.Context` parameter is passed and the IPC call executes on an a
worker thread, with the uncaught exception leading to a std::terminate call.

https://github.com/bitcoin-core/libmultiprocess/pull/218 was created to fix
this, but before that change is available, update an IPC test which can trigger
this behavior to handle it and recover when mp.Context parameters are added in
the an upcoming commit.

Having this workaround makes the test a little more complicated and less strict
but reduces dependencies between pending PRs so they don't need to be reviewed
or merged in a particular order.

ff995b50cf

ipc mining: declare constants for default field values

This commit only declares constants without using them. They will be applied in
seperate commit since changing struct default field values in cap'n proto is
not backwards compatible.

a4603ac774

ipc mining: provide default option values (incompatible schema change)

This change copies default option values from the C++ mining interface to the
Cap'n Proto interface. Currently, no capnp default values are set, so they are
implicitly all false or 0, which is inconvenient for the rust and python
clients and inconsistent with the C++ client.

Warning: This is an intermediate, review-only commit. Binaries built from it
should not be distributed or used to connect to other clients or servers. It
makes incompatible changes to the `mining.capnp` schema without updating the
`Init.makeMining` version, causing binaries to advertise support for a schema
they do not actually implement. Mixed versions may therefore exchange garbage
requests/responses instead of producing clear errors. The final commit in this
series bumps the mining interface number to ensure mismatches are detected.

git-bisect-skip: yes

c6638fa7c5

ipc mining: remove deprecated methods (incompatible schema change)

This change removes deprecated methods from the ipc mining interface.

Warning: This is an intermediate, review-only commit. Binaries built from it
should not be distributed or used to connect to other clients or servers. It
makes incompatible changes to the `mining.capnp` schema without updating the
`Init.makeMining` version, causing binaries to advertise support for a schema
they do not actually implement. Mixed versions may therefore exchange garbage
requests/responses instead of producing clear errors. The final commit in this
series bumps the mining interface number to ensure mismatches are detected.

git-bisect-skip: yes

2278f017af

ipc mining: pass missing context to BlockTemplate methods (incompatible schema change)

Adding a context parameter ensures that these methods are run in
their own thread and don't block other calls. They were missing
for:

- createNewBlock()
- checkBlock()

The missing parameters were first pointed out by plebhash in
https://github.com/bitcoin/bitcoin/issues/33575#issuecomment-3383290115 and
adding them should prevent possible performance problems and lockups,
especially with #34184 which can make the createNewBlock method block for a
long time before returning. It would be straightforward to make this change in
a backward compatible way
(https://github.com/bitcoin/bitcoin/pull/34184#discussion_r2770232149) but nice
to not need to go through the trouble.

Warning: This is an intermediate, review-only commit. Binaries built from it
should not be distributed or used to connect to other clients or servers. It
makes incompatible changes to the `mining.capnp` schema without updating the
`Init.makeMining` version, causing binaries to advertise support for a schema
they do not actually implement. Mixed versions may therefore exchange garbage
requests/responses instead of producing clear errors. The final commit in this
series bumps the mining interface number to ensure mismatches are detected.

git-bisect-skip: yes

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

70de5cc2d2

ipc mining: break compatibility with existing clients (version bump)

This increments the field number of the `Init.makeMining` method and makes the
old `makeMining` method return an error, so existing IPC mining clients not
using the latest schema file will get an error and not be able to access the
Mining interface.

Normally, there shouldn't be a need to break compatibility this way, but the
mining interface has evolved a lot since it was first introduced, with old
clients using the original methods less stable and performant than newer
clients. So now is a good time to introduce a cutoff, drop deprecated methods,
and stop supporting old clients which can't function as well.

Bumping the field number is also an opportunity to make other improvements that
would be awkward to implement compatibly, so a few of these were implemented in
commits immediately preceding this one.

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

9453c15361

DrahtBot added the label Mining on Feb 12, 2026

DrahtBot commented at 4:10 AM on February 12, 2026: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	Sjors, enirox001, ismaelsadeeq, sedited

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34422 (Update libmultiprocess subtree to be more stable with rust IPC client by ryanofsky)
#34284 (ipc, test: Add tests for unclean disconnect and thread busy behavior by ryanofsky)
#34184 (mining: add cooldown to createNewBlock() immediately after IBD by Sjors)
#34038 (logging: replace -loglevel with -trace, various API improvements by ajtowns)
#33966 (refactor: disentangle miner startup defaults from runtime options by Sjors)
#33922 (mining: add getMemoryLoad() and track template non-mempool memory footprint by Sjors)
#32297 (bitcoin-cli: Add -ipcconnect option by ryanofsky)
#29409 (multiprocess: Add capnp wrapper for Chain interface by ryanofsky)

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.

DrahtBot added the label CI failed on Feb 12, 2026

DrahtBot commented at 5:04 AM on February 12, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task lint: https://github.com/bitcoin/bitcoin/actions/runs/21933214675/job/63341541522</sub> <sub>LLM reason (✨ experimental): Linting failed: ruff found an unused import (CTransaction) in test_framework/ipc_util.py, causing the lint step to fail.</sub>

<details><summary>Hints</summary>

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.

</details>

Sjors commented at 1:28 PM on February 12, 2026: member

Concept ACK

Thanks, maybe split out a separate commit that drops the deprecated methods (first).

If #34184 lands first, I would suggest adding a separate commit to switch its default to true, which will churn quite a few tests but should nonetheless be easy to review.

doc: Release notes for mining IPC interface bump f700609e8a

ryanofsky force-pushed on Feb 12, 2026

ryanofsky commented at 1:53 PM on February 12, 2026: contributor

Updated e555eb86e9cd85eb00ec68c28b9ed6db72c8af35 -> 773091182cd3cece43257da70a6b08dc62153b30 (pr/mbreak.1 -> pr/mbreak.2, compare) fixing lint error (https://github.com/bitcoin/bitcoin/actions/runs/21933214675/job/63341541522), splitting commits, adding release notes, and adding checks to make sure c++/capnp constant values are consistent

Sjors commented at 1:54 PM on February 12, 2026: member

Can you try absorbing eaa1209ea4462d1221e049891fb588adb5cf60d3 from #34184? It creates a non-trivial issue that distracts from the cooldown PR core: #34184 (comment)

ryanofsky commented at 2:05 PM on February 12, 2026: contributor

Thanks, maybe split out a separate commit that drops the deprecated methods (first).

Yeah commit was definitely too big and is spilt up now. I do feel like it is good to make backwards incompatible changes atomically, so you can check out any commit in bitcoin core history and build bitcoin-node binary and check out any other commit in bitcoin core history and use the schema files to build an external client, and IPC calls made by that client will succeed or fail but they will never send corrupt parameters & return values.

This would mean practically that you should never change a default value without bumping the mining interface number in the same commit, which should be a reasonable rule to follow.

If #34184 lands first, I would suggest adding a separate commit to switch its default to true, which will churn quite a few tests but should nonetheless be easy to review.

Can do this but I think I'm missing something. if you think true is an more appropriate default, why not just set the default value to true in #34184? Would this break something?

Sjors commented at 2:08 PM on February 12, 2026: member

why not just set the default value to true in #34184? Would this break something?

It would create a lot of churn which makes review in that PR more difficult. I'd rather churn in a refactor PR like this.

ryanofsky commented at 2:08 PM on February 12, 2026: contributor

Can you try absorbing eaa1209 from #34184? It creates a non-trivial issue that distracts from the cooldown PR core: #34184 (comment)

I think all the changes there are incorporated here. That commit was my starting point for this PR. But let me know if anything is missing

in src/ipc/capnp/mining.capnp:25 in 6c614ba31a outdated

  20 | @@ -21,9 +21,9 @@ interface Mining $Proxy.wrap("interfaces::Mining") {
  21 |      isTestChain @0 (context :Proxy.Context) -> (result: Bool);
  22 |      isInitialBlockDownload @1 (context :Proxy.Context) -> (result: Bool);
  23 |      getTip @2 (context :Proxy.Context) -> (result: Common.BlockRef, hasResult: Bool);
  24 | -    waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64) -> (result: Common.BlockRef);
  25 | -    createNewBlock @4 (options: BlockCreateOptions) -> (result: BlockTemplate);
  26 | -    checkBlock @5 (block: Data, options: BlockCheckOptions) -> (reason: Text, debug: Text, result: Bool);
  27 | +    waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64 = .maxDouble) -> (result: Common.BlockRef);
  28 | +    createNewBlock @4 (context :Proxy.Context, options: BlockCreateOptions) -> (result: BlockTemplate);

Sjors commented at 2:09 PM on February 12, 2026:

Oh I see, this change doesn't break the test for you after #33965 merged?

in test/functional/interface_ipc_mining.py:251 in 6c614ba31a outdated

 252 | +            opts.blockReservedWeight = 0
 253 | +            try:
 254 | +                await mining.createNewBlock(ctx, opts)
 255 | +                raise AssertionError("createNewBlock unexpectedly succeeded")
 256 | +            except capnp.lib.capnp.KjException as e:
 257 | +                if e.type == "DISCONNECTED":

Sjors commented at 2:10 PM on February 12, 2026:

Oh I see, you added a workaround here.

Sjors commented at 2:15 PM on February 12, 2026:

If that can go in its own commit, I can cherry-pick it in to #34184, so the merge order doesn't matter.

ryanofsky commented at 2:43 PM on February 12, 2026:

re: #34568 (review)

If that can go in its own commit, I can cherry-pick it in to #34184, so the merge order doesn't matter.

Sure, done in latest push!

ryanofsky commented at 2:11 PM on February 12, 2026: contributor

why not just set the default value to true in #34184? Would this break something?

It would create a lot of churn which makes review in that PR more difficult. I'd rather churn in a refactor PR like this.

Is that true? If it's an entirely new parameter I'd think it would just require adding , false to a few existing calls.

Another thing I was wondering about is if this PR should get rid of the waitTipChanged method or if that method is still useful even with waitNext

Sjors commented at 2:13 PM on February 12, 2026: member

I prefer to keep waitTipChanged(), it's potentially useful before you create a template or even for applications that don't need a template.

Sjors commented at 2:14 PM on February 12, 2026: member

just require adding , false to a few existing calls

All of the tests (functional, unit, fuzz, bench) have to opt out of cooldown. Though I haven't tried to see how big the actual diff is.

Update: it's a big diff, though perhaps can be shortened because not all tests will break if use cooldown.

diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h
index 14327c01c0..2c7aac7b0f 100644
--- a/src/interfaces/mining.h
+++ b/src/interfaces/mining.h
@@ -151,11 +151,11 @@ public:
      *                     tip to catch up. Recommended, except for regtest and
      *                     signets with only one miner.
      * [@retval](/bitcoin-bitcoin/contributor/retval/) BlockTemplate a block template.
      * [@retval](/bitcoin-bitcoin/contributor/retval/) std::nullptr if the node is shut down or interrupt() is called.
      */
-    virtual std::unique_ptr<BlockTemplate> createNewBlock(const node::BlockCreateOptions& options = {}, bool cooldown = false) = 0;
+    virtual std::unique_ptr<BlockTemplate> createNewBlock(const node::BlockCreateOptions& options = {}, bool cooldown = true) = 0;

     /**
      * Interrupts createNewBlock and waitTipChanged.
      */
     virtual void interrupt() = 0;
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index ee80b90324..d8a9690481 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -163,11 +163,11 @@ static bool GenerateBlock(ChainstateManager& chainman, CBlock&& block, uint64_t&

 static UniValue generateBlocks(ChainstateManager& chainman, Mining& miner, const CScript& coinbase_output_script, int nGenerate, uint64_t nMaxTries)
 {
     UniValue blockHashes(UniValue::VARR);
     while (nGenerate > 0 && !chainman.m_interrupt) {
-        std::unique_ptr<BlockTemplate> block_template(miner.createNewBlock({ .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true }));
+        std::unique_ptr<BlockTemplate> block_template(miner.createNewBlock({ .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true }, /*cooldown=*/false));
         CHECK_NONFATAL(block_template);

         std::shared_ptr<const CBlock> block_out;
         if (!GenerateBlock(chainman, block_template->getBlock(), nMaxTries, block_out, /*process_new_block=*/true)) {
             break;
@@ -374,11 +374,11 @@ static RPCHelpMan generateblock()

     ChainstateManager& chainman = EnsureChainman(node);
     {
         LOCK(chainman.GetMutex());
         {
-            std::unique_ptr<BlockTemplate> block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true})};
+            std::unique_ptr<BlockTemplate> block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true}, /*cooldown=*/false)};
             CHECK_NONFATAL(block_template);

             block = block_template->getBlock();
         }

@@ -869,11 +869,11 @@ static RPCHelpMan getblocktemplate()
         nTransactionsUpdatedLast = mempool.GetTransactionsUpdated();
         CBlockIndex* pindexPrevNew = chainman.m_blockman.LookupBlockIndex(tip);
         time_start = GetTime();

         // Create new block
-        block_template = miner.createNewBlock({.include_dummy_extranonce = true});
+        block_template = miner.createNewBlock({.include_dummy_extranonce = true}, /*cooldown=*/false);
         CHECK_NONFATAL(block_template);


         // Need to update only after we know createNewBlock succeeded
         pindexPrev = pindexPrevNew;
diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp
index 3b4beebeff..7f05e4e9d8 100644
--- a/src/test/miner_tests.cpp
+++ b/src/test/miner_tests.cpp
@@ -120,55 +120,55 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const

     LOCK(tx_mempool.cs);
     BOOST_CHECK(tx_mempool.size() == 0);

     // Block template should only have a coinbase when there's nothing in the mempool
-    std::unique_ptr<BlockTemplate> block_template = mining->createNewBlock(options);
+    std::unique_ptr<BlockTemplate> block_template = mining->createNewBlock(options, /*cooldown=*/false);
     BOOST_REQUIRE(block_template);
     CBlock block{block_template->getBlock()};
     BOOST_REQUIRE_EQUAL(block.vtx.size(), 1U);

     // waitNext() on an empty mempool should return nullptr because there is no better template
     auto should_be_nullptr = block_template->waitNext({.timeout = MillisecondsDouble{0}, .fee_threshold = 1});
     BOOST_REQUIRE(should_be_nullptr == nullptr);

     // Unless fee_threshold is 0
     block_template = block_template->waitNext({.timeout = MillisecondsDouble{0}, .fee_threshold = 0});
     BOOST_REQUIRE(block_template);

     // Test the ancestor feerate transaction selection.
     TestMemPoolEntryHelper entry;

     // Test that a medium fee transaction will be selected after a higher fee
     // rate package with a low fee rate parent.
     CMutableTransaction tx;
     tx.vin.resize(1);
     tx.vin[0].scriptSig = CScript() << OP_1;
     tx.vin[0].prevout.hash = txFirst[0]->GetHash();
     tx.vin[0].prevout.n = 0;
     tx.vout.resize(1);
     tx.vout[0].nValue = 5000000000LL - 1000;
     // This tx has a low fee: 1000 satoshis
     Txid hashParentTx = tx.GetHash(); // save this txid for later use
     const auto parent_tx{entry.Fee(1000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx)};
     TryAddToMempool(tx_mempool, parent_tx);

     // This tx has a medium fee: 10000 satoshis
     tx.vin[0].prevout.hash = txFirst[1]->GetHash();
     tx.vout[0].nValue = 5000000000LL - 10000;
     Txid hashMediumFeeTx = tx.GetHash();
     const auto medium_fee_tx{entry.Fee(10000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx)};
     TryAddToMempool(tx_mempool, medium_fee_tx);

     // This tx has a high fee, but depends on the first transaction
     tx.vin[0].prevout.hash = hashParentTx;
     tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 50k satoshi fee
     Txid hashHighFeeTx = tx.GetHash();
     const auto high_fee_tx{entry.Fee(50000).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx)};
     TryAddToMempool(tx_mempool, high_fee_tx);

-    block_template = mining->createNewBlock(options);
+    block_template = mining->createNewBlock(options, /*cooldown=*/false);
     BOOST_REQUIRE(block_template);
     block = block_template->getBlock();
     BOOST_REQUIRE_EQUAL(block.vtx.size(), 4U);
     BOOST_CHECK(block.vtx[1]->GetHash() == hashParentTx);
     BOOST_CHECK(block.vtx[2]->GetHash() == hashHighFeeTx);
@@ -251,26 +251,26 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
     tx.vout.resize(1);
     feeToUse = blockMinFeeRate.GetFee(freeTxSize);
     tx.vout[0].nValue = 5000000000LL - 100000000 - feeToUse;
     Txid hashLowFeeTx2 = tx.GetHash();
     TryAddToMempool(tx_mempool, entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx));
-    block_template = mining->createNewBlock(options);
+    block_template = mining->createNewBlock(options, /*cooldown=*/false);
     BOOST_REQUIRE(block_template);
     block = block_template->getBlock();

     // Verify that this tx isn't selected.
     for (size_t i=0; i<block.vtx.size(); ++i) {
         BOOST_CHECK(block.vtx[i]->GetHash() != hashFreeTx2);
         BOOST_CHECK(block.vtx[i]->GetHash() != hashLowFeeTx2);
     }

     // This tx will be mineable, and should cause hashLowFeeTx2 to be selected
     // as well.
     tx.vin[0].prevout.n = 1;
     tx.vout[0].nValue = 100000000 - 10000; // 10k satoshi fee
     TryAddToMempool(tx_mempool, entry.Fee(10000).FromTx(tx));
-    block_template = mining->createNewBlock(options);
+    block_template = mining->createNewBlock(options, /*cooldown=*/false);
     BOOST_REQUIRE(block_template);
     block = block_template->getBlock();
     BOOST_REQUIRE_EQUAL(block.vtx.size(), 9U);
     BOOST_CHECK(block.vtx[8]->GetHash() == hashLowFeeTx2);
 }
@@ -340,195 +340,195 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
     {
         CTxMemPool& tx_mempool{MakeMempool()};
         LOCK(tx_mempool.cs);

         // Just to make sure we can still make simple blocks
-        auto block_template{mining->createNewBlock(options)};
+        auto block_template{mining->createNewBlock(options, /*cooldown=*/false)};
         BOOST_REQUIRE(block_template);
         CBlock block{block_template->getBlock()};

         auto txs = CreateBigSigOpsCluster(txFirst[0]);

         int64_t legacy_sigops = 0;
         for (auto& t : txs) {
             // If we don't set the number of sigops in the CTxMemPoolEntry,
             // template creation fails during sanity checks.
             TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(t));
             legacy_sigops += GetLegacySigOpCount(*t);
             BOOST_CHECK(tx_mempool.GetIter(t->GetHash()).has_value());
         }
         assert(tx_mempool.mapTx.size() == 51);
         assert(legacy_sigops == 20001);
-        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-blk-sigops"));
+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("bad-blk-sigops"));
     }

     {
         CTxMemPool& tx_mempool{MakeMempool()};
         LOCK(tx_mempool.cs);

         // Check that the mempool is empty.
         assert(tx_mempool.mapTx.empty());

         // Just to make sure we can still make simple blocks
-        auto block_template{mining->createNewBlock(options)};
+        auto block_template{mining->createNewBlock(options, /*cooldown=*/false)};
         BOOST_REQUIRE(block_template);
         CBlock block{block_template->getBlock()};

         auto txs = CreateBigSigOpsCluster(txFirst[0]);

         int64_t legacy_sigops = 0;
         for (auto& t : txs) {
             TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).SigOpsCost(GetLegacySigOpCount(*t)*WITNESS_SCALE_FACTOR).FromTx(t));
             legacy_sigops += GetLegacySigOpCount(*t);
             BOOST_CHECK(tx_mempool.GetIter(t->GetHash()).has_value());
         }
         assert(tx_mempool.mapTx.size() == 51);
         assert(legacy_sigops == 20001);

-        BOOST_REQUIRE(mining->createNewBlock(options));
+        BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false));
     }

     {
         CTxMemPool& tx_mempool{MakeMempool()};
         LOCK(tx_mempool.cs);

         // block size > limit
         tx.vin.resize(1);
         tx.vout.resize(1);
         tx.vout[0].nValue = BLOCKSUBSIDY;
         // 36 * (520char + DROP) + OP_1 = 18757 bytes
         std::vector<unsigned char> vchData(520);
         for (unsigned int i = 0; i < 18; ++i) {
             tx.vin[0].scriptSig << vchData << OP_DROP;
             tx.vout[0].scriptPubKey << vchData << OP_DROP;
         }
         tx.vin[0].scriptSig << OP_1;
         tx.vout[0].scriptPubKey << OP_1;
         tx.vin[0].prevout.hash = txFirst[0]->GetHash();
         tx.vin[0].prevout.n = 0;
         tx.vout[0].nValue = BLOCKSUBSIDY;
         for (unsigned int i = 0; i < 63; ++i) {
             tx.vout[0].nValue -= LOWFEE;
             hash = tx.GetHash();
             bool spendsCoinbase = i == 0; // only first tx spends coinbase
             TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(spendsCoinbase).FromTx(tx));
             BOOST_CHECK(tx_mempool.GetIter(hash).has_value());
             tx.vin[0].prevout.hash = hash;
         }
-        BOOST_REQUIRE(mining->createNewBlock(options));
+        BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false));
     }

     {
         CTxMemPool& tx_mempool{MakeMempool()};
         LOCK(tx_mempool.cs);

         // orphan in tx_mempool, template creation fails
         hash = tx.GetHash();
         TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).FromTx(tx));
-        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
     }

     {
         CTxMemPool& tx_mempool{MakeMempool()};
         LOCK(tx_mempool.cs);

         // child with higher feerate than parent
         tx.vin[0].scriptSig = CScript() << OP_1;
         tx.vin[0].prevout.hash = txFirst[1]->GetHash();
         tx.vout[0].nValue = BLOCKSUBSIDY - HIGHFEE;
         hash = tx.GetHash();
         TryAddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
         tx.vin[0].prevout.hash = hash;
         tx.vin.resize(2);
         tx.vin[1].scriptSig = CScript() << OP_1;
         tx.vin[1].prevout.hash = txFirst[0]->GetHash();
         tx.vin[1].prevout.n = 0;
         tx.vout[0].nValue = tx.vout[0].nValue + BLOCKSUBSIDY - HIGHERFEE; // First txn output + fresh coinbase - new txn fee
         hash = tx.GetHash();
         TryAddToMempool(tx_mempool, entry.Fee(HIGHERFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
-        BOOST_REQUIRE(mining->createNewBlock(options));
+        BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false));
     }

     {
         CTxMemPool& tx_mempool{MakeMempool()};
         LOCK(tx_mempool.cs);

         // coinbase in tx_mempool, template creation fails
         tx.vin.resize(1);
         tx.vin[0].prevout.SetNull();
         tx.vin[0].scriptSig = CScript() << OP_0 << OP_1;
         tx.vout[0].nValue = 0;
         hash = tx.GetHash();
         // give it a fee so it'll get mined
         TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx));
         // Should throw bad-cb-multiple
-        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-cb-multiple"));
+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("bad-cb-multiple"));
     }

     {
         CTxMemPool& tx_mempool{MakeMempool()};
         LOCK(tx_mempool.cs);

         // double spend txn pair in tx_mempool, template creation fails
         tx.vin[0].prevout.hash = txFirst[0]->GetHash();
         tx.vin[0].scriptSig = CScript() << OP_1;
         tx.vout[0].nValue = BLOCKSUBSIDY - HIGHFEE;
         tx.vout[0].scriptPubKey = CScript() << OP_1;
         hash = tx.GetHash();
         TryAddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
         tx.vout[0].scriptPubKey = CScript() << OP_2;
         hash = tx.GetHash();
         TryAddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
-        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
     }

     {
         CTxMemPool& tx_mempool{MakeMempool()};
         LOCK(tx_mempool.cs);

         // subsidy changing
         int nHeight = m_node.chainman->ActiveChain().Height();
         // Create an actual 209999-long block chain (without valid blocks).
         while (m_node.chainman->ActiveChain().Tip()->nHeight < 209999) {
             CBlockIndex* prev = m_node.chainman->ActiveChain().Tip();
             CBlockIndex* next = new CBlockIndex();
             next->phashBlock = new uint256(m_rng.rand256());
             m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(next->GetBlockHash());
             next->pprev = prev;
             next->nHeight = prev->nHeight + 1;
             next->BuildSkip();
             m_node.chainman->ActiveChain().SetTip(*next);
         }
-        BOOST_REQUIRE(mining->createNewBlock(options));
+        BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false));
         // Extend to a 210000-long block chain.
         while (m_node.chainman->ActiveChain().Tip()->nHeight < 210000) {
             CBlockIndex* prev = m_node.chainman->ActiveChain().Tip();
             CBlockIndex* next = new CBlockIndex();
             next->phashBlock = new uint256(m_rng.rand256());
             m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(next->GetBlockHash());
             next->pprev = prev;
             next->nHeight = prev->nHeight + 1;
             next->BuildSkip();
             m_node.chainman->ActiveChain().SetTip(*next);
         }
-        BOOST_REQUIRE(mining->createNewBlock(options));
+        BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false));

         // invalid p2sh txn in tx_mempool, template creation fails
         tx.vin[0].prevout.hash = txFirst[0]->GetHash();
         tx.vin[0].prevout.n = 0;
         tx.vin[0].scriptSig = CScript() << OP_1;
         tx.vout[0].nValue = BLOCKSUBSIDY - LOWFEE;
         CScript script = CScript() << OP_0;
         tx.vout[0].scriptPubKey = GetScriptForDestination(ScriptHash(script));
         hash = tx.GetHash();
         TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
         tx.vin[0].prevout.hash = hash;
         tx.vin[0].scriptSig = CScript() << std::vector<unsigned char>(script.begin(), script.end());
         tx.vout[0].nValue -= LOWFEE;
         hash = tx.GetHash();
         TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx));
-        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("block-script-verify-flag-failed"));
+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("block-script-verify-flag-failed"));

         // Delete the dummy blocks again.
         while (m_node.chainman->ActiveChain().Tip()->nHeight > nHeight) {
             CBlockIndex* del = m_node.chainman->ActiveChain().Tip();
             m_node.chainman->ActiveChain().SetTip(*Assert(del->pprev));
@@ -630,28 +630,28 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
     tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG;
     BOOST_CHECK(TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks pass
     tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG | 1;
     BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail

-    auto block_template = mining->createNewBlock(options);
+    auto block_template = mining->createNewBlock(options, /*cooldown=*/false);
     BOOST_REQUIRE(block_template);

     // None of the of the absolute height/time locked tx should have made
     // it into the template because we still check IsFinalTx in CreateNewBlock,
     // but relative locked txs will if inconsistently added to mempool.
     // For now these will still generate a valid template until BIP68 soft fork
     CBlock block{block_template->getBlock()};
     BOOST_CHECK_EQUAL(block.vtx.size(), 3U);
     // However if we advance height by 1 and time by SEQUENCE_LOCK_TIME, all of them should be mined
     for (int i = 0; i < CBlockIndex::nMedianTimeSpan; ++i) {
         CBlockIndex* ancestor{Assert(m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i))};
         ancestor->nTime += SEQUENCE_LOCK_TIME; // Trick the MedianTimePast
     }
     m_node.chainman->ActiveChain().Tip()->nHeight++;
     SetMockTime(m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1);

-    block_template = mining->createNewBlock(options);
+    block_template = mining->createNewBlock(options, /*cooldown=*/false);
     BOOST_REQUIRE(block_template);
     block = block_template->getBlock();
     BOOST_CHECK_EQUAL(block.vtx.size(), 5U);
 }

@@ -723,92 +723,92 @@ void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const
     tx.vin[0].prevout.hash = hashFreeChild;
     tx.vout[0].nValue = 5000000000LL; // 0 fee
     Txid hashFreeGrandchild = tx.GetHash();
     TryAddToMempool(tx_mempool, entry.Fee(0).SpendsCoinbase(false).FromTx(tx));

-    auto block_template = mining->createNewBlock(options);
+    auto block_template = mining->createNewBlock(options, /*cooldown=*/false);
     BOOST_REQUIRE(block_template);
     CBlock block{block_template->getBlock()};
     BOOST_REQUIRE_EQUAL(block.vtx.size(), 6U);
     BOOST_CHECK(block.vtx[1]->GetHash() == hashFreeParent);
     BOOST_CHECK(block.vtx[2]->GetHash() == hashFreePrioritisedTx);
     BOOST_CHECK(block.vtx[3]->GetHash() == hashParentTx);
     BOOST_CHECK(block.vtx[4]->GetHash() == hashPrioritsedChild);
     BOOST_CHECK(block.vtx[5]->GetHash() == hashFreeChild);
     for (size_t i=0; i<block.vtx.size(); ++i) {
         // The FreeParent and FreeChild's prioritisations should not impact the child.
         BOOST_CHECK(block.vtx[i]->GetHash() != hashFreeGrandchild);
         // De-prioritised transaction should not be included.
         BOOST_CHECK(block.vtx[i]->GetHash() != hashMediumFeeTx);
     }
 }

 // NOTE: These tests rely on CreateNewBlock doing its own self-validation!
 BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
 {
     auto mining{MakeMining()};
     BOOST_REQUIRE(mining);

     // Note that by default, these tests run with size accounting enabled.
     CScript scriptPubKey = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex << OP_CHECKSIG;
     BlockAssembler::Options options;
     options.coinbase_output_script = scriptPubKey;
     options.include_dummy_extranonce = true;

     // Create and check a simple template
-    std::unique_ptr<BlockTemplate> block_template = mining->createNewBlock(options);
+    std::unique_ptr<BlockTemplate> block_template = mining->createNewBlock(options, /*cooldown=*/false);
     BOOST_REQUIRE(block_template);
     {
         CBlock block{block_template->getBlock()};
         {
             std::string reason;
             std::string debug;
             BOOST_REQUIRE(!mining->checkBlock(block, {.check_pow = false}, reason, debug));
             BOOST_REQUIRE_EQUAL(reason, "bad-txnmrklroot");
             BOOST_REQUIRE_EQUAL(debug, "hashMerkleRoot mismatch");
         }

         block.hashMerkleRoot = BlockMerkleRoot(block);

         {
             std::string reason;
             std::string debug;
             BOOST_REQUIRE(mining->checkBlock(block, {.check_pow = false}, reason, debug));
             BOOST_REQUIRE_EQUAL(reason, "");
             BOOST_REQUIRE_EQUAL(debug, "");
         }

         {
             // A block template does not have proof-of-work, but it might pass
             // verification by coincidence. Grind the nonce if needed:
             while (CheckProofOfWork(block.GetHash(), block.nBits, Assert(m_node.chainman)->GetParams().GetConsensus())) {
                 block.nNonce++;
             }

             std::string reason;
             std::string debug;
             BOOST_REQUIRE(!mining->checkBlock(block, {.check_pow = true}, reason, debug));
             BOOST_REQUIRE_EQUAL(reason, "high-hash");
             BOOST_REQUIRE_EQUAL(debug, "proof of work failed");
         }
     }

     // We can't make transactions until we have inputs
     // Therefore, load 110 blocks :)
     static_assert(std::size(BLOCKINFO) == 110, "Should have 110 blocks to import");
     int baseheight = 0;
     std::vector<CTransactionRef> txFirst;
     for (const auto& bi : BLOCKINFO) {
         const int current_height{mining->getTip()->height};

         /**
          * Simple block creation, nothing special yet.
          * If current_height is odd, block_template will have already been
          * set at the end of the previous loop.
          */
         if (current_height % 2 == 0) {
-            block_template = mining->createNewBlock(options);
+            block_template = mining->createNewBlock(options, /*cooldown=*/false);
             BOOST_REQUIRE(block_template);
         }

         CBlock block{block_template->getBlock()};
         CMutableTransaction txCoinbase(*block.vtx[0]);
diff --git a/src/test/testnet4_miner_tests.cpp b/src/test/testnet4_miner_tests.cpp
index 33e028a5c9..614d2fd63a 100644
--- a/src/test/testnet4_miner_tests.cpp
+++ b/src/test/testnet4_miner_tests.cpp
@@ -40,11 +40,11 @@ BOOST_AUTO_TEST_CASE(MiningInterface)

     // Set node time a few minutes past the testnet4 genesis block
     const int64_t genesis_time{WITH_LOCK(cs_main, return m_node.chainman->ActiveChain().Tip()->GetBlockTime())};
     SetMockTime(genesis_time + 3 * 60);

-    block_template = mining->createNewBlock(options);
+    block_template = mining->createNewBlock(options, /*cooldown=*/false);
     BOOST_REQUIRE(block_template);

     // The template should use the mocked system time
     BOOST_REQUIRE_EQUAL(block_template->getBlockHeader().nTime, genesis_time + 3 * 60);

</details>

ryanofsky force-pushed on Feb 12, 2026

ryanofsky commented at 2:44 PM on February 12, 2026: contributor

Updated 773091182cd3cece43257da70a6b08dc62153b30 -> dfea3246e8fc9c74811fdcc8e747cc593abef250 (pr/mbreak.2 -> pr/mbreak.3, compare) cleaning up some more constant usages and splitting out a test workaround into its own commit

Sjors commented at 4:40 PM on February 12, 2026: member

Looks like one of the intermediate commits broke interface_ipc.py. @maflcko it would be handy if the each-commit job repeated the current commit at the end of its log.

in src/ipc/test/ipc_test.cpp:30 in 766edecb11 outdated

  23 | @@ -23,6 +24,11 @@
  24 |  
  25 |  #include <boost/test/unit_test.hpp>
  26 |  
  27 | +static_assert(ipc::capnp::messages::MAX_MONEY == MAX_MONEY);
  28 | +static_assert(ipc::capnp::messages::MAX_DOUBLE == std::numeric_limits<double>::max());
  29 | +static_assert(ipc::capnp::messages::DEFAULT_BLOCK_RESERVED_WEIGHT == DEFAULT_BLOCK_RESERVED_WEIGHT);
  30 | +static_assert(ipc::capnp::messages::DEFAULT_COINBASE_OUTPUT_MAX_ADDITIONAL_SIGOPS == DEFAULT_COINBASE_OUTPUT_MAX_ADDITIONAL_SIGOPS);

Sjors commented at 5:14 PM on February 12, 2026:

In 766edecb11781375a0bf648a427676f21f0487d8 ipc: Declare constants in mining.capnp for default field values: nice, makes it easy to keep these constants in sync

in src/ipc/capnp/mining.capnp:16 in 766edecb11 outdated

  11 | @@ -12,6 +12,11 @@ using Proxy = import "/mp/proxy.capnp";
  12 |  $Proxy.include("interfaces/mining.h");
  13 |  $Proxy.includeTypes("ipc/capnp/mining-types.h");
  14 |  
  15 | +const maxMoney :Int64 = 2100000000000000;
  16 | +const maxDouble :Float64 = 1.7976931348623157e308;

Sjors commented at 5:20 PM on February 12, 2026:

In 766edecb11781375a0bf648a427676f21f0487d8 ipc: Declare constants in mining.capnp for default field values: maybe make it a nice round 3 trillion milliseconds (~95 years, const century)? Or 32 trillion for a millennium.

ryanofsky commented at 7:53 PM on February 12, 2026:

re: #34568 (review)

In 766edec ipc: Declare constants in mining.capnp for default field values: maybe make it a nice round 3 trillion milliseconds (~95 years, const century)? Or 32 trillion for a millennium.

Yes could switch to something rounder, this is just keeping the c++ and capnproto values the same.

in src/rpc/mining.cpp:1018 in 9c464055ef outdated

1014 | @@ -1015,8 +1015,8 @@ static RPCHelpMan getblocktemplate()
1015 |          result.pushKV("signet_challenge", HexStr(consensusParams.signet_challenge));
1016 |      }
1017 |  
1018 | -    if (!block_template->getCoinbaseCommitment().empty()) {
1019 | -        result.pushKV("default_witness_commitment", HexStr(block_template->getCoinbaseCommitment()));
1020 | +    if (auto coinbase{block_template->getCoinbaseTx()}; coinbase.required_outputs.size() > 0) {

Sjors commented at 5:29 PM on February 12, 2026:

In 9c464055ef529b31d0196ecc8f054f9a95e48d76 rpc refactor: stop using deprecated getCoinbaseCommitment method: let's add

Assume(coinbase.required_outputs.size() == 1);

This could break if we ever soft-fork an additional commitment. And, more likely, if a miner patches a second commitment (e.g. merged mining).

ryanofsky commented at 7:54 PM on February 12, 2026:

re: #34568 (review)

This could break if we ever soft-fork an additional commitment. And, more likely, if a miner patches a second commitment (e.g. merged mining).

Makes sense, applied suggestion.

Sjors commented at 5:39 PM on February 12, 2026: member

Code review dfea3246e8fc9c74811fdcc8e747cc593abef250

Would be nice to further split fc60bd454034ad6b54b03ec2e6982f2761e47b8c:

bump version, with deprecation test
drop deprecated methods
add missing context :Proxy.Context
set defaults

I do feel like it is good to make backwards incompatible changes atomically, so you can check out any commit in bitcoin core history and build bitcoin-node binary and check out any other commit in bitcoin core history and use the schema files to build an external client, and IPC calls made by that client will succeed or fail but they will never send corrupt parameters & return values.

I think it's sufficient that each release behaves this way, not individual commits.

ryanofsky force-pushed on Feb 12, 2026

ryanofsky commented at 8:17 PM on February 12, 2026: contributor

Thanks for the review!

Updated dfea3246e8fc9c74811fdcc8e747cc593abef250 -> 47f688706d0adedf25ab6eecabe0c3e134d62a85 (pr/mbreak.3 -> pr/mbreak.4, compare) spliting commits, adding suggested Assume and making other small tweaks.

re: #34568#pullrequestreview-3792405487

Would be nice to further split fc60bd4:

Thanks, that split does seem to make the changes pretty easy to review.

I think it's sufficient that each release behaves this way, not individual commits.

I think allowing incompatible schema changes between releases without version bumping would create headaches and confusion because it would mean all clients and servers built during development would appear to connect to each other but could actually be sending garbage back and forth. By contrast, just following the rules for compatibility and bumping the interface number whenever compatibility is broken seems more straightforward to me. Maybe it's not necessary to do it atomically in each commit (through this would be nice), but it should at least be safe to switch between PRs and not have to worry about clients and servers communicating unreliably.

ryanofsky force-pushed on Feb 12, 2026

ryanofsky commented at 8:29 PM on February 12, 2026: contributor

Updated 47f688706d0adedf25ab6eecabe0c3e134d62a85 -> 84372191f8d44b23adfa12941b331003e30cb0a3 (pr/mbreak.4 -> pr/mbreak.5, compare) to fix linter complaining about Assume used instead of CHECK_NONFATAL in RPC code https://github.com/bitcoin/bitcoin/actions/runs/21962039339/job/63441798855?pr=34568

DrahtBot removed the label CI failed on Feb 12, 2026

Sjors commented at 8:38 AM on February 13, 2026: member

Maybe it's not necessary to do it atomically in each commit (through this would be nice), but it should at least be safe to switch between PR

Per PR could work, that keeps commits easy to review. I'm worried that having to add a makeMiningDeprecatedV... for every breaking change could get noisy though.

fanquake commented at 8:42 AM on February 13, 2026: member

I'm worried that having to add a makeMiningDeprecatedV... for every breaking change could get noisy though.

Why do we have to add deprecated methods at all, if we are breaking compatility, in an experimental interface?

Sjors commented at 8:49 AM on February 13, 2026: member

@fanquake it's not deprecated in our traditional sense of "works, but will go away", it's actually gone. This just throws a useful error telling the user to upgrade. But it'd be nice if there a more generic way to do that, so we don't have to keep a map of methods that used to exist.

Additionally .capnp files can't have gaps in the method numbers.

It's also useful to have one example of a deleted interface so clients know how to implement handle that in the future (but then presumably we'd keep the old version around as deprecated for one major release).

I'm somewhat optimistic that we can mark the interface as non-experimental by v32.

in test/functional/interface_ipc_mining.py:248 in abd9a2fe10 outdated

 242 | @@ -243,9 +243,6 @@ async def async_routine():
 243 |  
 244 |              async with AsyncExitStack() as stack:
 245 |                  opts = self.capnp_modules['mining'].BlockCreateOptions()
 246 | -                opts.useMempool = True
 247 | -                opts.blockReservedWeight = 4000
 248 | -                opts.coinbaseOutputMaxAdditionalSigops = 0

Sjors commented at 9:00 AM on February 13, 2026:

In abd9a2fe1002995a410d884025b8b2479bc9f194 ipc mining: provide default option values (incompatible schema change): review note: changing this from 0 to the new default 400 is harmless. Ditto for blockReservedWeight.

Using defaults also improves test readability. When custom value shows up, it's now clear that it's relevant to the test.

in src/node/miner.cpp:200 in e2770e2f6b outdated

 196 | @@ -197,7 +197,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock()
 197 |      coinbase_tx.lock_time = coinbaseTx.nLockTime;
 198 |  
 199 |      pblock->vtx[0] = MakeTransactionRef(std::move(coinbaseTx));
 200 | -    pblocktemplate->vchCoinbaseCommitment = m_chainstate.m_chainman.GenerateCoinbaseCommitment(*pblock, pindexPrev);
 201 | +    m_chainstate.m_chainman.GenerateCoinbaseCommitment(*pblock, pindexPrev);

Sjors commented at 9:09 AM on February 13, 2026:

In e2770e2f6ba07b9ad3665b99ad7841a914e70b21 ipc mining: remove deprecated methods (incompatible schema change): let's change GenerateCoinbaseCommitment to void, here or in a followup.

ryanofsky commented at 8:08 PM on February 17, 2026:

re: #34568 (review)

let's change GenerateCoinbaseCommitment to void, here or in a followup.

Nice suggestion, implemented in latest push

Sjors approved

Sjors commented at 9:14 AM on February 13, 2026: member

ACK 84372191f8d44b23adfa12941b331003e30cb0a3

bitcoin deleted a comment on Feb 13, 2026

ryanofsky commented at 12:11 PM on February 13, 2026: contributor

Why do we have to add deprecated methods at all, if we are breaking compatility, in an experimental interface?

We have a few options here with different tradeoffs:

We can freely delete methods from both c++ source code and capnp files breaking capnproto schema update rules. This will cause client and server binaries built from the schemas to appear to be compatible with each other and connect and communicate but send corrupt data back and forth, likely leading to confusion and wasted time during development since there is no clear way to tell which clients and servers are compatible and when they are sending corrupt data.
We can delete the method implementations from c++ source code, but follow schema update rules in capnp files, keeping method numbers the same, but giving them prefixes or suffixes indicating the methods have been removed, deleting the arguments and making calls to those methods throw exceptions. (Right implementing this requires a line of c++ code for each method virtual void removedFoo() { throw std::exception("foo not implemented"); } But this could be simplified by having libmultiprocess support a $Proxy.exception("foo not implmented") annotation in the capnp files so no c++ code is needed. Unlike the first approach this gives clear errors when clients and servers are incompatible with each other, but deleting method implementations will break older clients, and when the methods are simple accessors like the deprecated methods removed here, we might decide not to break clients without a clear reason to do so.
We can keep deprecated methods around for a period of time (like one release) so clients targeted at the most recent release continue to work on master, but might not work with the next release. Then at the end of the period we can delete them by bumping the Init.makeMining number.

This PR is taking approach (3) but we can also delete methods more proactively with (2). I'd like to avoid approach (1) since I don't think I don't think benefits outweigh the costs.

Also should note that the change here removing deprecated methods is only a small part of this PR (just commit e2770e2f6ba07b9ad3665b99ad7841a914e70b21). The bigger part of this PR is making minor schema improvements (context arguments, default option values) we've been putting off but have more of a reason to implement now because of #34184.

fanquake commented at 1:30 PM on February 13, 2026: member

@ryanofsky Thanks for clarifying here.

ryanofsky force-pushed on Feb 17, 2026

ryanofsky commented at 8:13 PM on February 17, 2026: contributor

Updated 84372191f8d44b23adfa12941b331003e30cb0a3 -> f700609e8ada3b48fd45ec19979cd72d943d47a6 (pr/mbreak.5 -> pr/mbreak.6, compare) dropping GenerateCoinbaseCommitment return value and improving commit messages

Sjors commented at 7:51 AM on February 18, 2026: member

ACK f700609e8ada3b48fd45ec19979cd72d943d47a6

enirox001 commented at 8:21 AM on February 19, 2026: contributor

ACK f700609e8ada3b48fd45ec19979cd72d943d47a6

Built the PR and verified the breaking change by connecting an existing Rust SV2 client (using the old schema) to the updated node. The client was rejected gracefully with the expected error (Old mining interface (@2) not supported) and the node remained stable, confirming the version bump logic works safely.

nano nit: The comment in src/node/types.h for BlockCreateOptions states clients "cannot leave this field unset." With the new defaults in mining.capnp, this is no longer true and should be updated.

Sjors commented at 9:38 AM on February 19, 2026: member

cc @ismaelsadeeq

ismaelsadeeq commented at 12:55 PM on February 19, 2026: member

Concept ACK

in src/rpc/mining.cpp:1019 in df53a3e5ec

1014 | @@ -1015,8 +1015,9 @@ static RPCHelpMan getblocktemplate()
1015 |          result.pushKV("signet_challenge", HexStr(consensusParams.signet_challenge));
1016 |      }
1017 |  
1018 | -    if (!block_template->getCoinbaseCommitment().empty()) {
1019 | -        result.pushKV("default_witness_commitment", HexStr(block_template->getCoinbaseCommitment()));
1020 | +    if (auto coinbase{block_template->getCoinbaseTx()}; coinbase.required_outputs.size() > 0) {
1021 | +        CHECK_NONFATAL(coinbase.required_outputs.size() == 1); // Only one output is currently expected

ismaelsadeeq commented at 3:23 PM on February 19, 2026:

In "rpc refactor: stop using deprecated getCoinbaseCommitment method" df53a3e5ec8781833c29682ff9e459fca489fa7b

This comment doesn't add any value here, not a fan :). The CHECK_NON_FATAL made that obvious; a better comment should explain why we expect 1 output or why it's not fatal to continue.

ryanofsky commented at 4:30 PM on February 19, 2026:

re: #34568 (review)

This comment doesn't add any value here, not a fan :).

Agree comment could be better, I just added it because I wanted to be clear this is a "not currently expected to happen" check, not a "this should never happen" check. The designs of the mining API and segwit explicitly allow and anticipate this happening, it's just that the RPC code may need to change if it does.

Thinking about this more though, it would probably be best to generalize the code a little and drop this check. Instead of assuming there is only one output this code could use the GetWitnessCommitmentIndex logic to find the right output and return it. It'd be a slightly bigger change but would be a nice followup.

in test/functional/interface_ipc_mining.py:354 in c6638fa7c5

 347 | @@ -351,12 +348,7 @@ async def async_routine():
 348 |  
 349 |      def run_test(self):
 350 |          self.miniwallet = MiniWallet(self.nodes[0])
 351 | -        self.default_block_create_options = self.capnp_modules['mining'].BlockCreateOptions(

ismaelsadeeq commented at 3:30 PM on February 19, 2026:

Nice to have these gone.

ismaelsadeeq approved

ismaelsadeeq commented at 3:33 PM on February 19, 2026: member

ACK f700609e8ada3b48fd45ec19979cd72d943d47a6

ryanofsky commented at 4:33 PM on February 19, 2026: contributor

Thanks for the reviews! Will leave this PR as-is for now but some nice followups have been suggested:

Updating node/types.h comment (https://github.com/bitcoin/bitcoin/pull/34568#pullrequestreview-3824134962)
Clarifying RPC check or dropping it (https://github.com/bitcoin/bitcoin/pull/34568#discussion_r2828504464)

sedited added this to the milestone 31.0 on Feb 20, 2026

in src/policy/policy.h:36 in a4603ac774

sedited commented at 9:37 AM on February 20, 2026:

Just a question: Why are all these constants in a policy-related file as opposed to e.g. miner.h or its own header?

ismaelsadeeq commented at 10:03 AM on February 20, 2026:

Hmm, the constant they are indeed Bitcoin Core mining defaults, which are whats recommended and seperate from consensus, hence they can be termed as mining policy? see related discussion #31384 (review)

Right now, policy/policy.h encapsulates both mempool policy, mempool, validation, relay and mining policy.

Are you suggesting they be split? It seems beyond the scope of this PR IMO?

ryanofsky commented at 1:10 PM on February 20, 2026:

re: #34568 (review)

Just a question: Why are all these constants in a policy-related file as opposed to e.g. miner.h or its own header?

I also thought this was strange, but you could think of them as a mining policy analogous to normal mempool policy as Abubakar suggests. Since this only adds one constant, this seemed the best place for it to go, but if there are preferences or suggestions for where else these should go, it could be nice to move them. Could do this in a PR with other suggested followups #34568 (comment)

Sjors commented at 1:42 PM on February 20, 2026:

Could also be a followup for #33966. I considered moving mining "policy" things away from policy as well there, but didn't.

sedited approved

sedited commented at 9:52 AM on February 20, 2026: contributor

ACK f700609e8ada3b48fd45ec19979cd72d943d47a6

sedited merged this on Feb 20, 2026

sedited closed this on Feb 20, 2026

sedited referenced this in commit 925759d172 on Feb 23, 2026

sedited referenced this in commit 8640523843 on Feb 23, 2026

in test/functional/interface_ipc_mining.py:271 in ff995b50cf

 272 | +            except capnp.lib.capnp.KjException as e:
 273 | +                if e.type == "DISCONNECTED":
 274 | +                    # The remote exception isn't caught currently and leads to a
 275 | +                    # std::terminate call. Just detect and restart in this case.
 276 | +                    # This bug is fixed with
 277 | +                    # https://github.com/bitcoin-core/libmultiprocess/pull/218

ryanofsky commented at 2:24 AM on February 24, 2026:

In commit "ipc test: add workaround to block_reserved_weight exception test" (ff995b50cf9e1ea521f3cf546339f05d10b79a4d)

I did some more work on https://github.com/bitcoin-core/libmultiprocess/pull/218 and this comment is false. I thought that PR added code to catch uncaught exceptions from IPC server methods running on asynchronous threads, but it actually never did that. The code which does do that was added in https://github.com/bitcoin-core/libmultiprocess/pull/240 commit https://github.com/bitcoin-core/libmultiprocess/pull/240/changes/c4762c7b513abf66e3275b2bd3f2cf0cb980a8f7. That commit was only supposed to be a refactoring, but because it moved a kj::runCatchingExceptions call one level up the stack it fixed this problem inadvertently.