mining: Break compatibility with existing IPC mining clients

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

Let expected_stderr option passed to wait_until_stopped and is_node_stopped
helper functions be a regex pattern instead of just a fixed string.
Let 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.

cd2aa85867

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.

f7ca042b6a

ipc mining: declare constants for default field values

The default values will be applied in an upcoming commit since changing them is
not backwards compatible

31f18f5551

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 and is not intended to
stand alone. It violates Cap'n Proto schema-evolution compatibility, so mixed
client/server builds will silently decode IPC messages inconsistently,
producing nonsensical requests/responses. A follow-up commit bumps the mining
interface version and ensures new clients/servers reject older counterparts.

git-bisect-skip: yes

abd9a2fe10

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 and is not intended to
stand alone. It violates Cap'n Proto schema-evolution compatibility, so mixed
client/server builds will silently decode IPC messages inconsistently,
producing nonsensical requests/responses. A follow-up commit bumps the mining
interface version and ensures new clients/servers reject older counterparts.

git-bisect-skip: yes

e2770e2f6b

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 and is not intended to
stand alone. It violates Cap'n Proto schema-evolution compatibility, so mixed
client/server builds will silently decode IPC messages inconsistently,
producing nonsensical requests/responses. A follow-up commit bumps the mining
interface version and ensures new clients/servers reject older counterparts.

git-bisect-skip: yes

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

3afebda53c

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>

db05772f2c

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

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> 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)
#34038 (logging: replace -loglevel with -trace, various API improvements by ajtowns)
#33966 (refactor: disentangle miner startup defaults from runtime options 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. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/21933214675/job/63341541522 LLM reason (✨ experimental): Linting failed: ruff found an unused import (CTransaction) in test_framework/ipc_util.py, causing the lint step to fail.

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 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 84372191f8

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.

  0diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h
  1index 14327c01c0..2c7aac7b0f 100644
  2--- a/src/interfaces/mining.h
  3+++ b/src/interfaces/mining.h
  4@@ -151,11 +151,11 @@ public:
  5      *                     tip to catch up. Recommended, except for regtest and
  6      *                     signets with only one miner.
  7      * [@retval](/bitcoin-bitcoin/contributor/retval/) BlockTemplate a block template.
  8      * [@retval](/bitcoin-bitcoin/contributor/retval/) std::nullptr if the node is shut down or interrupt() is called.
  9      */
 10-    virtual std::unique_ptr<BlockTemplate> createNewBlock(const node::BlockCreateOptions& options = {}, bool cooldown = false) = 0;
 11+    virtual std::unique_ptr<BlockTemplate> createNewBlock(const node::BlockCreateOptions& options = {}, bool cooldown = true) = 0;
 12
 13     /**
 14      * Interrupts createNewBlock and waitTipChanged.
 15      */
 16     virtual void interrupt() = 0;
 17diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
 18index ee80b90324..d8a9690481 100644
 19--- a/src/rpc/mining.cpp
 20+++ b/src/rpc/mining.cpp
 21@@ -163,11 +163,11 @@ static bool GenerateBlock(ChainstateManager& chainman, CBlock&& block, uint64_t&
 22
 23 static UniValue generateBlocks(ChainstateManager& chainman, Mining& miner, const CScript& coinbase_output_script, int nGenerate, uint64_t nMaxTries)
 24 {
 25     UniValue blockHashes(UniValue::VARR);
 26     while (nGenerate > 0 && !chainman.m_interrupt) {
 27-        std::unique_ptr<BlockTemplate> block_template(miner.createNewBlock({ .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true }));
 28+        std::unique_ptr<BlockTemplate> block_template(miner.createNewBlock({ .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true }, /*cooldown=*/false));
 29         CHECK_NONFATAL(block_template);
 30
 31         std::shared_ptr<const CBlock> block_out;
 32         if (!GenerateBlock(chainman, block_template->getBlock(), nMaxTries, block_out, /*process_new_block=*/true)) {
 33             break;
 34@@ -374,11 +374,11 @@ static RPCHelpMan generateblock()
 35
 36     ChainstateManager& chainman = EnsureChainman(node);
 37     {
 38         LOCK(chainman.GetMutex());
 39         {
 40-            std::unique_ptr<BlockTemplate> block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true})};
 41+            std::unique_ptr<BlockTemplate> block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true}, /*cooldown=*/false)};
 42             CHECK_NONFATAL(block_template);
 43
 44             block = block_template->getBlock();
 45         }
 46
 47@@ -869,11 +869,11 @@ static RPCHelpMan getblocktemplate()
 48         nTransactionsUpdatedLast = mempool.GetTransactionsUpdated();
 49         CBlockIndex* pindexPrevNew = chainman.m_blockman.LookupBlockIndex(tip);
 50         time_start = GetTime();
 51
 52         // Create new block
 53-        block_template = miner.createNewBlock({.include_dummy_extranonce = true});
 54+        block_template = miner.createNewBlock({.include_dummy_extranonce = true}, /*cooldown=*/false);
 55         CHECK_NONFATAL(block_template);
 56
 57
 58         // Need to update only after we know createNewBlock succeeded
 59         pindexPrev = pindexPrevNew;
 60diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp
 61index 3b4beebeff..7f05e4e9d8 100644
 62--- a/src/test/miner_tests.cpp
 63+++ b/src/test/miner_tests.cpp
 64@@ -120,55 +120,55 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
 65
 66     LOCK(tx_mempool.cs);
 67     BOOST_CHECK(tx_mempool.size() == 0);
 68
 69     // Block template should only have a coinbase when there's nothing in the mempool
 70-    std::unique_ptr<BlockTemplate> block_template = mining->createNewBlock(options);
 71+    std::unique_ptr<BlockTemplate> block_template = mining->createNewBlock(options, /*cooldown=*/false);
 72     BOOST_REQUIRE(block_template);
 73     CBlock block{block_template->getBlock()};
 74     BOOST_REQUIRE_EQUAL(block.vtx.size(), 1U);
 75
 76     // waitNext() on an empty mempool should return nullptr because there is no better template
 77     auto should_be_nullptr = block_template->waitNext({.timeout = MillisecondsDouble{0}, .fee_threshold = 1});
 78     BOOST_REQUIRE(should_be_nullptr == nullptr);
 79
 80     // Unless fee_threshold is 0
 81     block_template = block_template->waitNext({.timeout = MillisecondsDouble{0}, .fee_threshold = 0});
 82     BOOST_REQUIRE(block_template);
 83
 84     // Test the ancestor feerate transaction selection.
 85     TestMemPoolEntryHelper entry;
 86
 87     // Test that a medium fee transaction will be selected after a higher fee
 88     // rate package with a low fee rate parent.
 89     CMutableTransaction tx;
 90     tx.vin.resize(1);
 91     tx.vin[0].scriptSig = CScript() << OP_1;
 92     tx.vin[0].prevout.hash = txFirst[0]->GetHash();
 93     tx.vin[0].prevout.n = 0;
 94     tx.vout.resize(1);
 95     tx.vout[0].nValue = 5000000000LL - 1000;
 96     // This tx has a low fee: 1000 satoshis
 97     Txid hashParentTx = tx.GetHash(); // save this txid for later use
 98     const auto parent_tx{entry.Fee(1000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx)};
 99     TryAddToMempool(tx_mempool, parent_tx);
100
101     // This tx has a medium fee: 10000 satoshis
102     tx.vin[0].prevout.hash = txFirst[1]->GetHash();
103     tx.vout[0].nValue = 5000000000LL - 10000;
104     Txid hashMediumFeeTx = tx.GetHash();
105     const auto medium_fee_tx{entry.Fee(10000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx)};
106     TryAddToMempool(tx_mempool, medium_fee_tx);
107
108     // This tx has a high fee, but depends on the first transaction
109     tx.vin[0].prevout.hash = hashParentTx;
110     tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 50k satoshi fee
111     Txid hashHighFeeTx = tx.GetHash();
112     const auto high_fee_tx{entry.Fee(50000).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx)};
113     TryAddToMempool(tx_mempool, high_fee_tx);
114
115-    block_template = mining->createNewBlock(options);
116+    block_template = mining->createNewBlock(options, /*cooldown=*/false);
117     BOOST_REQUIRE(block_template);
118     block = block_template->getBlock();
119     BOOST_REQUIRE_EQUAL(block.vtx.size(), 4U);
120     BOOST_CHECK(block.vtx[1]->GetHash() == hashParentTx);
121     BOOST_CHECK(block.vtx[2]->GetHash() == hashHighFeeTx);
122@@ -251,26 +251,26 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
123     tx.vout.resize(1);
124     feeToUse = blockMinFeeRate.GetFee(freeTxSize);
125     tx.vout[0].nValue = 5000000000LL - 100000000 - feeToUse;
126     Txid hashLowFeeTx2 = tx.GetHash();
127     TryAddToMempool(tx_mempool, entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx));
128-    block_template = mining->createNewBlock(options);
129+    block_template = mining->createNewBlock(options, /*cooldown=*/false);
130     BOOST_REQUIRE(block_template);
131     block = block_template->getBlock();
132
133     // Verify that this tx isn't selected.
134     for (size_t i=0; i<block.vtx.size(); ++i) {
135         BOOST_CHECK(block.vtx[i]->GetHash() != hashFreeTx2);
136         BOOST_CHECK(block.vtx[i]->GetHash() != hashLowFeeTx2);
137     }
138
139     // This tx will be mineable, and should cause hashLowFeeTx2 to be selected
140     // as well.
141     tx.vin[0].prevout.n = 1;
142     tx.vout[0].nValue = 100000000 - 10000; // 10k satoshi fee
143     TryAddToMempool(tx_mempool, entry.Fee(10000).FromTx(tx));
144-    block_template = mining->createNewBlock(options);
145+    block_template = mining->createNewBlock(options, /*cooldown=*/false);
146     BOOST_REQUIRE(block_template);
147     block = block_template->getBlock();
148     BOOST_REQUIRE_EQUAL(block.vtx.size(), 9U);
149     BOOST_CHECK(block.vtx[8]->GetHash() == hashLowFeeTx2);
150 }
151@@ -340,195 +340,195 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
152     {
153         CTxMemPool& tx_mempool{MakeMempool()};
154         LOCK(tx_mempool.cs);
155
156         // Just to make sure we can still make simple blocks
157-        auto block_template{mining->createNewBlock(options)};
158+        auto block_template{mining->createNewBlock(options, /*cooldown=*/false)};
159         BOOST_REQUIRE(block_template);
160         CBlock block{block_template->getBlock()};
161
162         auto txs = CreateBigSigOpsCluster(txFirst[0]);
163
164         int64_t legacy_sigops = 0;
165         for (auto& t : txs) {
166             // If we don't set the number of sigops in the CTxMemPoolEntry,
167             // template creation fails during sanity checks.
168             TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(t));
169             legacy_sigops += GetLegacySigOpCount(*t);
170             BOOST_CHECK(tx_mempool.GetIter(t->GetHash()).has_value());
171         }
172         assert(tx_mempool.mapTx.size() == 51);
173         assert(legacy_sigops == 20001);
174-        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-blk-sigops"));
175+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("bad-blk-sigops"));
176     }
177
178     {
179         CTxMemPool& tx_mempool{MakeMempool()};
180         LOCK(tx_mempool.cs);
181
182         // Check that the mempool is empty.
183         assert(tx_mempool.mapTx.empty());
184
185         // Just to make sure we can still make simple blocks
186-        auto block_template{mining->createNewBlock(options)};
187+        auto block_template{mining->createNewBlock(options, /*cooldown=*/false)};
188         BOOST_REQUIRE(block_template);
189         CBlock block{block_template->getBlock()};
190
191         auto txs = CreateBigSigOpsCluster(txFirst[0]);
192
193         int64_t legacy_sigops = 0;
194         for (auto& t : txs) {
195             TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).SigOpsCost(GetLegacySigOpCount(*t)*WITNESS_SCALE_FACTOR).FromTx(t));
196             legacy_sigops += GetLegacySigOpCount(*t);
197             BOOST_CHECK(tx_mempool.GetIter(t->GetHash()).has_value());
198         }
199         assert(tx_mempool.mapTx.size() == 51);
200         assert(legacy_sigops == 20001);
201
202-        BOOST_REQUIRE(mining->createNewBlock(options));
203+        BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false));
204     }
205
206     {
207         CTxMemPool& tx_mempool{MakeMempool()};
208         LOCK(tx_mempool.cs);
209
210         // block size > limit
211         tx.vin.resize(1);
212         tx.vout.resize(1);
213         tx.vout[0].nValue = BLOCKSUBSIDY;
214         // 36 * (520char + DROP) + OP_1 = 18757 bytes
215         std::vector<unsigned char> vchData(520);
216         for (unsigned int i = 0; i < 18; ++i) {
217             tx.vin[0].scriptSig << vchData << OP_DROP;
218             tx.vout[0].scriptPubKey << vchData << OP_DROP;
219         }
220         tx.vin[0].scriptSig << OP_1;
221         tx.vout[0].scriptPubKey << OP_1;
222         tx.vin[0].prevout.hash = txFirst[0]->GetHash();
223         tx.vin[0].prevout.n = 0;
224         tx.vout[0].nValue = BLOCKSUBSIDY;
225         for (unsigned int i = 0; i < 63; ++i) {
226             tx.vout[0].nValue -= LOWFEE;
227             hash = tx.GetHash();
228             bool spendsCoinbase = i == 0; // only first tx spends coinbase
229             TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(spendsCoinbase).FromTx(tx));
230             BOOST_CHECK(tx_mempool.GetIter(hash).has_value());
231             tx.vin[0].prevout.hash = hash;
232         }
233-        BOOST_REQUIRE(mining->createNewBlock(options));
234+        BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false));
235     }
236
237     {
238         CTxMemPool& tx_mempool{MakeMempool()};
239         LOCK(tx_mempool.cs);
240
241         // orphan in tx_mempool, template creation fails
242         hash = tx.GetHash();
243         TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).FromTx(tx));
244-        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
245+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
246     }
247
248     {
249         CTxMemPool& tx_mempool{MakeMempool()};
250         LOCK(tx_mempool.cs);
251
252         // child with higher feerate than parent
253         tx.vin[0].scriptSig = CScript() << OP_1;
254         tx.vin[0].prevout.hash = txFirst[1]->GetHash();
255         tx.vout[0].nValue = BLOCKSUBSIDY - HIGHFEE;
256         hash = tx.GetHash();
257         TryAddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
258         tx.vin[0].prevout.hash = hash;
259         tx.vin.resize(2);
260         tx.vin[1].scriptSig = CScript() << OP_1;
261         tx.vin[1].prevout.hash = txFirst[0]->GetHash();
262         tx.vin[1].prevout.n = 0;
263         tx.vout[0].nValue = tx.vout[0].nValue + BLOCKSUBSIDY - HIGHERFEE; // First txn output + fresh coinbase - new txn fee
264         hash = tx.GetHash();
265         TryAddToMempool(tx_mempool, entry.Fee(HIGHERFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
266-        BOOST_REQUIRE(mining->createNewBlock(options));
267+        BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false));
268     }
269
270     {
271         CTxMemPool& tx_mempool{MakeMempool()};
272         LOCK(tx_mempool.cs);
273
274         // coinbase in tx_mempool, template creation fails
275         tx.vin.resize(1);
276         tx.vin[0].prevout.SetNull();
277         tx.vin[0].scriptSig = CScript() << OP_0 << OP_1;
278         tx.vout[0].nValue = 0;
279         hash = tx.GetHash();
280         // give it a fee so it'll get mined
281         TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx));
282         // Should throw bad-cb-multiple
283-        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-cb-multiple"));
284+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("bad-cb-multiple"));
285     }
286
287     {
288         CTxMemPool& tx_mempool{MakeMempool()};
289         LOCK(tx_mempool.cs);
290
291         // double spend txn pair in tx_mempool, template creation fails
292         tx.vin[0].prevout.hash = txFirst[0]->GetHash();
293         tx.vin[0].scriptSig = CScript() << OP_1;
294         tx.vout[0].nValue = BLOCKSUBSIDY - HIGHFEE;
295         tx.vout[0].scriptPubKey = CScript() << OP_1;
296         hash = tx.GetHash();
297         TryAddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
298         tx.vout[0].scriptPubKey = CScript() << OP_2;
299         hash = tx.GetHash();
300         TryAddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
301-        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
302+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
303     }
304
305     {
306         CTxMemPool& tx_mempool{MakeMempool()};
307         LOCK(tx_mempool.cs);
308
309         // subsidy changing
310         int nHeight = m_node.chainman->ActiveChain().Height();
311         // Create an actual 209999-long block chain (without valid blocks).
312         while (m_node.chainman->ActiveChain().Tip()->nHeight < 209999) {
313             CBlockIndex* prev = m_node.chainman->ActiveChain().Tip();
314             CBlockIndex* next = new CBlockIndex();
315             next->phashBlock = new uint256(m_rng.rand256());
316             m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(next->GetBlockHash());
317             next->pprev = prev;
318             next->nHeight = prev->nHeight + 1;
319             next->BuildSkip();
320             m_node.chainman->ActiveChain().SetTip(*next);
321         }
322-        BOOST_REQUIRE(mining->createNewBlock(options));
323+        BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false));
324         // Extend to a 210000-long block chain.
325         while (m_node.chainman->ActiveChain().Tip()->nHeight < 210000) {
326             CBlockIndex* prev = m_node.chainman->ActiveChain().Tip();
327             CBlockIndex* next = new CBlockIndex();
328             next->phashBlock = new uint256(m_rng.rand256());
329             m_node.chainman->ActiveChainstate().CoinsTip().SetBestBlock(next->GetBlockHash());
330             next->pprev = prev;
331             next->nHeight = prev->nHeight + 1;
332             next->BuildSkip();
333             m_node.chainman->ActiveChain().SetTip(*next);
334         }
335-        BOOST_REQUIRE(mining->createNewBlock(options));
336+        BOOST_REQUIRE(mining->createNewBlock(options, /*cooldown=*/false));
337
338         // invalid p2sh txn in tx_mempool, template creation fails
339         tx.vin[0].prevout.hash = txFirst[0]->GetHash();
340         tx.vin[0].prevout.n = 0;
341         tx.vin[0].scriptSig = CScript() << OP_1;
342         tx.vout[0].nValue = BLOCKSUBSIDY - LOWFEE;
343         CScript script = CScript() << OP_0;
344         tx.vout[0].scriptPubKey = GetScriptForDestination(ScriptHash(script));
345         hash = tx.GetHash();
346         TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
347         tx.vin[0].prevout.hash = hash;
348         tx.vin[0].scriptSig = CScript() << std::vector<unsigned char>(script.begin(), script.end());
349         tx.vout[0].nValue -= LOWFEE;
350         hash = tx.GetHash();
351         TryAddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx));
352-        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options), std::runtime_error, HasReason("block-script-verify-flag-failed"));
353+        BOOST_CHECK_EXCEPTION(mining->createNewBlock(options, /*cooldown=*/false), std::runtime_error, HasReason("block-script-verify-flag-failed"));
354
355         // Delete the dummy blocks again.
356         while (m_node.chainman->ActiveChain().Tip()->nHeight > nHeight) {
357             CBlockIndex* del = m_node.chainman->ActiveChain().Tip();
358             m_node.chainman->ActiveChain().SetTip(*Assert(del->pprev));
359@@ -630,28 +630,28 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
360     tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG;
361     BOOST_CHECK(TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks pass
362     tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG | 1;
363     BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail
364
365-    auto block_template = mining->createNewBlock(options);
366+    auto block_template = mining->createNewBlock(options, /*cooldown=*/false);
367     BOOST_REQUIRE(block_template);
368
369     // None of the of the absolute height/time locked tx should have made
370     // it into the template because we still check IsFinalTx in CreateNewBlock,
371     // but relative locked txs will if inconsistently added to mempool.
372     // For now these will still generate a valid template until BIP68 soft fork
373     CBlock block{block_template->getBlock()};
374     BOOST_CHECK_EQUAL(block.vtx.size(), 3U);
375     // However if we advance height by 1 and time by SEQUENCE_LOCK_TIME, all of them should be mined
376     for (int i = 0; i < CBlockIndex::nMedianTimeSpan; ++i) {
377         CBlockIndex* ancestor{Assert(m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i))};
378         ancestor->nTime += SEQUENCE_LOCK_TIME; // Trick the MedianTimePast
379     }
380     m_node.chainman->ActiveChain().Tip()->nHeight++;
381     SetMockTime(m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1);
382
383-    block_template = mining->createNewBlock(options);
384+    block_template = mining->createNewBlock(options, /*cooldown=*/false);
385     BOOST_REQUIRE(block_template);
386     block = block_template->getBlock();
387     BOOST_CHECK_EQUAL(block.vtx.size(), 5U);
388 }
389
390@@ -723,92 +723,92 @@ void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const
391     tx.vin[0].prevout.hash = hashFreeChild;
392     tx.vout[0].nValue = 5000000000LL; // 0 fee
393     Txid hashFreeGrandchild = tx.GetHash();
394     TryAddToMempool(tx_mempool, entry.Fee(0).SpendsCoinbase(false).FromTx(tx));
395
396-    auto block_template = mining->createNewBlock(options);
397+    auto block_template = mining->createNewBlock(options, /*cooldown=*/false);
398     BOOST_REQUIRE(block_template);
399     CBlock block{block_template->getBlock()};
400     BOOST_REQUIRE_EQUAL(block.vtx.size(), 6U);
401     BOOST_CHECK(block.vtx[1]->GetHash() == hashFreeParent);
402     BOOST_CHECK(block.vtx[2]->GetHash() == hashFreePrioritisedTx);
403     BOOST_CHECK(block.vtx[3]->GetHash() == hashParentTx);
404     BOOST_CHECK(block.vtx[4]->GetHash() == hashPrioritsedChild);
405     BOOST_CHECK(block.vtx[5]->GetHash() == hashFreeChild);
406     for (size_t i=0; i<block.vtx.size(); ++i) {
407         // The FreeParent and FreeChild's prioritisations should not impact the child.
408         BOOST_CHECK(block.vtx[i]->GetHash() != hashFreeGrandchild);
409         // De-prioritised transaction should not be included.
410         BOOST_CHECK(block.vtx[i]->GetHash() != hashMediumFeeTx);
411     }
412 }
413
414 // NOTE: These tests rely on CreateNewBlock doing its own self-validation!
415 BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
416 {
417     auto mining{MakeMining()};
418     BOOST_REQUIRE(mining);
419
420     // Note that by default, these tests run with size accounting enabled.
421     CScript scriptPubKey = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex << OP_CHECKSIG;
422     BlockAssembler::Options options;
423     options.coinbase_output_script = scriptPubKey;
424     options.include_dummy_extranonce = true;
425
426     // Create and check a simple template
427-    std::unique_ptr<BlockTemplate> block_template = mining->createNewBlock(options);
428+    std::unique_ptr<BlockTemplate> block_template = mining->createNewBlock(options, /*cooldown=*/false);
429     BOOST_REQUIRE(block_template);
430     {
431         CBlock block{block_template->getBlock()};
432         {
433             std::string reason;
434             std::string debug;
435             BOOST_REQUIRE(!mining->checkBlock(block, {.check_pow = false}, reason, debug));
436             BOOST_REQUIRE_EQUAL(reason, "bad-txnmrklroot");
437             BOOST_REQUIRE_EQUAL(debug, "hashMerkleRoot mismatch");
438         }
439
440         block.hashMerkleRoot = BlockMerkleRoot(block);
441
442         {
443             std::string reason;
444             std::string debug;
445             BOOST_REQUIRE(mining->checkBlock(block, {.check_pow = false}, reason, debug));
446             BOOST_REQUIRE_EQUAL(reason, "");
447             BOOST_REQUIRE_EQUAL(debug, "");
448         }
449
450         {
451             // A block template does not have proof-of-work, but it might pass
452             // verification by coincidence. Grind the nonce if needed:
453             while (CheckProofOfWork(block.GetHash(), block.nBits, Assert(m_node.chainman)->GetParams().GetConsensus())) {
454                 block.nNonce++;
455             }
456
457             std::string reason;
458             std::string debug;
459             BOOST_REQUIRE(!mining->checkBlock(block, {.check_pow = true}, reason, debug));
460             BOOST_REQUIRE_EQUAL(reason, "high-hash");
461             BOOST_REQUIRE_EQUAL(debug, "proof of work failed");
462         }
463     }
464
465     // We can't make transactions until we have inputs
466     // Therefore, load 110 blocks :)
467     static_assert(std::size(BLOCKINFO) == 110, "Should have 110 blocks to import");
468     int baseheight = 0;
469     std::vector<CTransactionRef> txFirst;
470     for (const auto& bi : BLOCKINFO) {
471         const int current_height{mining->getTip()->height};
472
473         /**
474          * Simple block creation, nothing special yet.
475          * If current_height is odd, block_template will have already been
476          * set at the end of the previous loop.
477          */
478         if (current_height % 2 == 0) {
479-            block_template = mining->createNewBlock(options);
480+            block_template = mining->createNewBlock(options, /*cooldown=*/false);
481             BOOST_REQUIRE(block_template);
482         }
483
484         CBlock block{block_template->getBlock()};
485         CMutableTransaction txCoinbase(*block.vtx[0]);
486diff --git a/src/test/testnet4_miner_tests.cpp b/src/test/testnet4_miner_tests.cpp
487index 33e028a5c9..614d2fd63a 100644
488--- a/src/test/testnet4_miner_tests.cpp
489+++ b/src/test/testnet4_miner_tests.cpp
490@@ -40,11 +40,11 @@ BOOST_AUTO_TEST_CASE(MiningInterface)
491
492     // Set node time a few minutes past the testnet4 genesis block
493     const int64_t genesis_time{WITH_LOCK(cs_main, return m_node.chainman->ActiveChain().Tip()->GetBlockTime())};
494     SetMockTime(genesis_time + 3 * 60);
495
496-    block_template = mining->createNewBlock(options);
497+    block_template = mining->createNewBlock(options, /*cooldown=*/false);
498     BOOST_REQUIRE(block_template);
499
500     // The template should use the mocked system time
501     BOOST_REQUIRE_EQUAL(block_template->getBlockHeader().nTime, genesis_time + 3 * 60);

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

0Assume(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

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

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.

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.

mining: Break compatibility with existing IPC mining clients #34568

Reviews

Conflicts