From df53a3e5ec8781833c29682ff9e459fca489fa7b Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 11 Feb 2026 21:34:08 -0500 Subject: [PATCH 1/9] rpc refactor: stop using deprecated getCoinbaseCommitment method There should be no change in behavior Co-authored-by: Sjors Provoost --- src/rpc/mining.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index ee80b903248..e21d0d137c0 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -1015,8 +1015,9 @@ static RPCHelpMan getblocktemplate() result.pushKV("signet_challenge", HexStr(consensusParams.signet_challenge)); } - if (!block_template->getCoinbaseCommitment().empty()) { - result.pushKV("default_witness_commitment", HexStr(block_template->getCoinbaseCommitment())); + if (auto coinbase{block_template->getCoinbaseTx()}; coinbase.required_outputs.size() > 0) { + CHECK_NONFATAL(coinbase.required_outputs.size() == 1); // Only one output is currently expected + result.pushKV("default_witness_commitment", HexStr(coinbase.required_outputs[0].scriptPubKey)); } return result; From b970cdf20fce43fb58dde1cbf713e97ff21d7a2e Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 14 Jan 2026 08:37:37 -0500 Subject: [PATCH 2/9] 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. --- test/functional/test_framework/test_node.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index b56cf64ce99..89665ab232f 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -22,6 +22,7 @@ import collections import shlex import shutil import sys +from collections.abc import Iterable from pathlib import Path from .authproxy import ( @@ -469,6 +470,12 @@ class TestNode(): """Checks whether the node has stopped. Returns True if the node has stopped. False otherwise. + + If the process has exited, asserts that the exit code matches + `expected_ret_code` (which may be a single value or an iterable of values), + and that stderr matches `expected_stderr` exactly or, if a regex pattern is + provided, contains the pattern. + This method is responsible for freeing resources (self.process).""" if not self.running: return True @@ -477,12 +484,17 @@ class TestNode(): return False # process has stopped. Assert that it didn't return an error code. - assert return_code == expected_ret_code, self._node_msg( + if not isinstance(expected_ret_code, Iterable): + expected_ret_code = (expected_ret_code,) + assert return_code in expected_ret_code, self._node_msg( f"Node returned unexpected exit code ({return_code}) vs ({expected_ret_code}) when stopping") # Check that stderr is as expected self.stderr.seek(0) stderr = self.stderr.read().decode('utf-8').strip() - if stderr != expected_stderr: + if isinstance(expected_stderr, re.Pattern): + if not expected_stderr.search(stderr): + raise AssertionError(f"Unexpected stderr {stderr!r} does not contain {expected_stderr.pattern!r}") + elif stderr != expected_stderr: raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr)) self.stdout.close() From ff995b50cf9e1ea521f3cf546339f05d10b79a4d Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 11 Feb 2026 21:34:08 -0500 Subject: [PATCH 3/9] 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. --- test/functional/interface_ipc_mining.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py index 61c050e74fe..d4a0d87e767 100755 --- a/test/functional/interface_ipc_mining.py +++ b/test/functional/interface_ipc_mining.py @@ -6,6 +6,7 @@ import asyncio from contextlib import AsyncExitStack from io import BytesIO +import re from test_framework.blocktools import NULL_OUTPOINT from test_framework.messages import ( MAX_BLOCK_WEIGHT, @@ -257,12 +258,21 @@ class IPCMiningTest(BitcoinTestFramework): empty_block = await mining_get_block(empty_template, ctx) assert_equal(len(empty_block.vtx), 1) - self.log.debug("Enforce minimum reserved weight for IPC clients too") - opts.blockReservedWeight = 0 - try: - await mining.createNewBlock(opts) - raise AssertionError("createNewBlock unexpectedly succeeded") - except capnp.lib.capnp.KjException as e: + self.log.debug("Enforce minimum reserved weight for IPC clients too") + opts.blockReservedWeight = 0 + try: + await mining.createNewBlock(opts) + raise AssertionError("createNewBlock unexpectedly succeeded") + except capnp.lib.capnp.KjException as e: + if e.type == "DISCONNECTED": + # The remote exception isn't caught currently and leads to a + # std::terminate call. Just detect and restart in this case. + # This bug is fixed with + # https://github.com/bitcoin-core/libmultiprocess/pull/218 + assert_equal(e.description, "Peer disconnected.") + self.nodes[0].wait_until_stopped(expected_ret_code=(-11, -6, 1, 66), expected_stderr=re.compile("")) + self.start_node(0) + else: assert_equal(e.description, "remote exception: std::exception: block_reserved_weight (0) must be at least 2000 weight units") assert_equal(e.type, "FAILED") From a4603ac77412790b6498ab1750017e31353740bf Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Mon, 9 Feb 2026 15:44:27 -0500 Subject: [PATCH 4/9] 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. --- src/ipc/capnp/mining.capnp | 5 +++++ src/ipc/test/ipc_test.cpp | 6 ++++++ src/node/types.h | 2 +- src/policy/policy.h | 2 ++ 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/ipc/capnp/mining.capnp b/src/ipc/capnp/mining.capnp index 25df1186331..7e5cced6f31 100644 --- a/src/ipc/capnp/mining.capnp +++ b/src/ipc/capnp/mining.capnp @@ -12,6 +12,11 @@ using Proxy = import "/mp/proxy.capnp"; $Proxy.include("interfaces/mining.h"); $Proxy.includeTypes("ipc/capnp/mining-types.h"); +const maxMoney :Int64 = 2100000000000000; +const maxDouble :Float64 = 1.7976931348623157e308; +const defaultBlockReservedWeight :UInt32 = 8000; +const defaultCoinbaseOutputMaxAdditionalSigops :UInt32 = 400; + interface Mining $Proxy.wrap("interfaces::Mining") { isTestChain @0 (context :Proxy.Context) -> (result: Bool); isInitialBlockDownload @1 (context :Proxy.Context) -> (result: Bool); diff --git a/src/ipc/test/ipc_test.cpp b/src/ipc/test/ipc_test.cpp index bec288813ad..31607299ae7 100644 --- a/src/ipc/test/ipc_test.cpp +++ b/src/ipc/test/ipc_test.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -23,6 +24,11 @@ #include +static_assert(ipc::capnp::messages::MAX_MONEY == MAX_MONEY); +static_assert(ipc::capnp::messages::MAX_DOUBLE == std::numeric_limits::max()); +static_assert(ipc::capnp::messages::DEFAULT_BLOCK_RESERVED_WEIGHT == DEFAULT_BLOCK_RESERVED_WEIGHT); +static_assert(ipc::capnp::messages::DEFAULT_COINBASE_OUTPUT_MAX_ADDITIONAL_SIGOPS == DEFAULT_COINBASE_OUTPUT_MAX_ADDITIONAL_SIGOPS); + //! Remote init class. class TestInit : public interfaces::Init { diff --git a/src/node/types.h b/src/node/types.h index e3ee05dd0d9..01e74528e06 100644 --- a/src/node/types.h +++ b/src/node/types.h @@ -55,7 +55,7 @@ struct BlockCreateOptions { * The maximum additional sigops which the pool will add in coinbase * transaction outputs. */ - size_t coinbase_output_max_additional_sigops{400}; + size_t coinbase_output_max_additional_sigops{DEFAULT_COINBASE_OUTPUT_MAX_ADDITIONAL_SIGOPS}; /** * Script to put in the coinbase transaction. The default is an * anyone-can-spend dummy. diff --git a/src/policy/policy.h b/src/policy/policy.h index 35189d71335..0bc7b327708 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -24,6 +24,8 @@ class CScript; static constexpr unsigned int DEFAULT_BLOCK_MAX_WEIGHT{MAX_BLOCK_WEIGHT}; /** Default for -blockreservedweight **/ static constexpr unsigned int DEFAULT_BLOCK_RESERVED_WEIGHT{8000}; +/** Default sigops cost to reserve for coinbase transaction outputs when creating block templates. */ +static constexpr unsigned int DEFAULT_COINBASE_OUTPUT_MAX_ADDITIONAL_SIGOPS{400}; /** This accounts for the block header, var_int encoding of the transaction count and a minimally viable * coinbase transaction. It adds an additional safety margin, because even with a thorough understanding * of block serialization, it's easy to make a costly mistake when trying to squeeze every last byte. From c6638fa7c5e97f9fd7a5ea8feb29f8caeac788bd Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Mon, 9 Feb 2026 15:44:27 -0500 Subject: [PATCH 5/9] 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 --- src/ipc/capnp/mining.capnp | 16 ++++++++-------- test/functional/interface_ipc_mining.py | 10 +--------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/ipc/capnp/mining.capnp b/src/ipc/capnp/mining.capnp index 7e5cced6f31..1811716ff54 100644 --- a/src/ipc/capnp/mining.capnp +++ b/src/ipc/capnp/mining.capnp @@ -21,7 +21,7 @@ interface Mining $Proxy.wrap("interfaces::Mining") { isTestChain @0 (context :Proxy.Context) -> (result: Bool); isInitialBlockDownload @1 (context :Proxy.Context) -> (result: Bool); getTip @2 (context :Proxy.Context) -> (result: Common.BlockRef, hasResult: Bool); - waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64) -> (result: Common.BlockRef); + waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64 = .maxDouble) -> (result: Common.BlockRef); createNewBlock @4 (options: BlockCreateOptions) -> (result: BlockTemplate); checkBlock @5 (block: Data, options: BlockCheckOptions) -> (reason: Text, debug: Text, result: Bool); } @@ -43,19 +43,19 @@ interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") { } struct BlockCreateOptions $Proxy.wrap("node::BlockCreateOptions") { - useMempool @0 :Bool $Proxy.name("use_mempool"); - blockReservedWeight @1 :UInt64 $Proxy.name("block_reserved_weight"); - coinbaseOutputMaxAdditionalSigops @2 :UInt64 $Proxy.name("coinbase_output_max_additional_sigops"); + useMempool @0 :Bool = true $Proxy.name("use_mempool"); + blockReservedWeight @1 :UInt64 = .defaultBlockReservedWeight $Proxy.name("block_reserved_weight"); + coinbaseOutputMaxAdditionalSigops @2 :UInt64 = .defaultCoinbaseOutputMaxAdditionalSigops $Proxy.name("coinbase_output_max_additional_sigops"); } struct BlockWaitOptions $Proxy.wrap("node::BlockWaitOptions") { - timeout @0 : Float64 $Proxy.name("timeout"); - feeThreshold @1 : Int64 $Proxy.name("fee_threshold"); + timeout @0 : Float64 = .maxDouble $Proxy.name("timeout"); + feeThreshold @1 : Int64 = .maxMoney $Proxy.name("fee_threshold"); } struct BlockCheckOptions $Proxy.wrap("node::BlockCheckOptions") { - checkMerkleRoot @0 :Bool $Proxy.name("check_merkle_root"); - checkPow @1 :Bool $Proxy.name("check_pow"); + checkMerkleRoot @0 :Bool = true $Proxy.name("check_merkle_root"); + checkPow @1 :Bool = true $Proxy.name("check_pow"); } struct CoinbaseTx $Proxy.wrap("node::CoinbaseTx") { diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py index d4a0d87e767..a02727bb4c9 100755 --- a/test/functional/interface_ipc_mining.py +++ b/test/functional/interface_ipc_mining.py @@ -243,9 +243,6 @@ class IPCMiningTest(BitcoinTestFramework): async with AsyncExitStack() as stack: opts = self.capnp_modules['mining'].BlockCreateOptions() - opts.useMempool = True - opts.blockReservedWeight = 4000 - opts.coinbaseOutputMaxAdditionalSigops = 0 template = await mining_create_block_template(mining, stack, ctx, opts) assert template is not None block = await mining_get_block(template, ctx) @@ -351,12 +348,7 @@ class IPCMiningTest(BitcoinTestFramework): def run_test(self): self.miniwallet = MiniWallet(self.nodes[0]) - self.default_block_create_options = self.capnp_modules['mining'].BlockCreateOptions( - useMempool=True, - blockReservedWeight=4000, - coinbaseOutputMaxAdditionalSigops=0 - ) - + self.default_block_create_options = self.capnp_modules['mining'].BlockCreateOptions() self.run_mining_interface_test() self.run_block_template_test() self.run_coinbase_and_submission_test() From 2278f017afad4d2c570527b15df776ee64fc1ee2 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Mon, 9 Feb 2026 15:44:27 -0500 Subject: [PATCH 6/9] 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 --- src/interfaces/mining.h | 22 ---------------------- src/ipc/capnp/mining.capnp | 13 +++++-------- src/node/interfaces.cpp | 15 --------------- src/node/miner.cpp | 2 +- src/node/miner.h | 1 - src/validation.cpp | 5 +---- src/validation.h | 2 +- test/functional/interface_ipc_mining.py | 18 +----------------- test/functional/test_framework/ipc_util.py | 10 +--------- 9 files changed, 10 insertions(+), 78 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 993f70bd7aa..42002284d11 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -42,31 +42,9 @@ public: // Sigop cost per transaction, not including coinbase transaction. virtual std::vector getTxSigops() = 0; - /** - * Return serialized dummy coinbase transaction. - * - * @note deprecated: use getCoinbaseTx() - */ - virtual CTransactionRef getCoinbaseRawTx() = 0; - /** Return fields needed to construct a coinbase transaction */ virtual node::CoinbaseTx getCoinbaseTx() = 0; - /** - * Return scriptPubKey with SegWit OP_RETURN. - * - * @note deprecated: use getCoinbaseTx() - */ - virtual std::vector getCoinbaseCommitment() = 0; - - /** - * Return which output in the dummy coinbase contains the SegWit OP_RETURN. - * - * @note deprecated. Scan outputs from getCoinbaseTx() outputs field for the - * SegWit marker. - */ - virtual int getWitnessCommitmentIndex() = 0; - /** * Compute merkle path to the coinbase transaction * diff --git a/src/ipc/capnp/mining.capnp b/src/ipc/capnp/mining.capnp index 1811716ff54..0727fcc0a50 100644 --- a/src/ipc/capnp/mining.capnp +++ b/src/ipc/capnp/mining.capnp @@ -32,14 +32,11 @@ interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") { getBlock @2 (context: Proxy.Context) -> (result: Data); getTxFees @3 (context: Proxy.Context) -> (result: List(Int64)); getTxSigops @4 (context: Proxy.Context) -> (result: List(Int64)); - getCoinbaseRawTx @5 (context: Proxy.Context) -> (result: Data); - getCoinbaseTx @12 (context: Proxy.Context) -> (result: CoinbaseTx); - getCoinbaseCommitment @6 (context: Proxy.Context) -> (result: Data); - getWitnessCommitmentIndex @7 (context: Proxy.Context) -> (result: Int32); - getCoinbaseMerklePath @8 (context: Proxy.Context) -> (result: List(Data)); - submitSolution @9 (context: Proxy.Context, version: UInt32, timestamp: UInt32, nonce: UInt32, coinbase :Data) -> (result: Bool); - waitNext @10 (context: Proxy.Context, options: BlockWaitOptions) -> (result: BlockTemplate); - interruptWait @11() -> (); + getCoinbaseTx @5 (context: Proxy.Context) -> (result: CoinbaseTx); + getCoinbaseMerklePath @6 (context: Proxy.Context) -> (result: List(Data)); + submitSolution @7 (context: Proxy.Context, version: UInt32, timestamp: UInt32, nonce: UInt32, coinbase :Data) -> (result: Bool); + waitNext @8 (context: Proxy.Context, options: BlockWaitOptions) -> (result: BlockTemplate); + interruptWait @9() -> (); } struct BlockCreateOptions $Proxy.wrap("node::BlockCreateOptions") { diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index af9388d4b9f..8f5406ab3c0 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -890,26 +890,11 @@ public: return m_block_template->vTxSigOpsCost; } - CTransactionRef getCoinbaseRawTx() override - { - return m_block_template->block.vtx[0]; - } - CoinbaseTx getCoinbaseTx() override { return m_block_template->m_coinbase_tx; } - std::vector getCoinbaseCommitment() override - { - return m_block_template->vchCoinbaseCommitment; - } - - int getWitnessCommitmentIndex() override - { - return GetWitnessCommitmentIndex(m_block_template->block); - } - std::vector getCoinbaseMerklePath() override { return TransactionMerklePath(m_block_template->block, 0); diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 7bdd8b690d7..fe26794528b 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -197,7 +197,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock() coinbase_tx.lock_time = coinbaseTx.nLockTime; pblock->vtx[0] = MakeTransactionRef(std::move(coinbaseTx)); - pblocktemplate->vchCoinbaseCommitment = m_chainstate.m_chainman.GenerateCoinbaseCommitment(*pblock, pindexPrev); + m_chainstate.m_chainman.GenerateCoinbaseCommitment(*pblock, pindexPrev); const CTransactionRef& final_coinbase{pblock->vtx[0]}; if (final_coinbase->HasWitness()) { diff --git a/src/node/miner.h b/src/node/miner.h index 0c268f1826d..5a338dfb917 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -46,7 +46,6 @@ struct CBlockTemplate std::vector vTxFees; // Sigops per transaction, not including coinbase transaction (unlike CBlock::vtx). std::vector vTxSigOpsCost; - std::vector vchCoinbaseCommitment; /* A vector of package fee rates, ordered by the sequence in which * packages are selected for inclusion in the block template.*/ std::vector m_package_feerates; diff --git a/src/validation.cpp b/src/validation.cpp index c200c3d6cd8..b7a113ea4b9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4040,9 +4040,8 @@ void ChainstateManager::UpdateUncommittedBlockStructures(CBlock& block, const CB } } -std::vector ChainstateManager::GenerateCoinbaseCommitment(CBlock& block, const CBlockIndex* pindexPrev) const +void ChainstateManager::GenerateCoinbaseCommitment(CBlock& block, const CBlockIndex* pindexPrev) const { - std::vector commitment; int commitpos = GetWitnessCommitmentIndex(block); std::vector ret(32, 0x00); if (commitpos == NO_WITNESS_COMMITMENT) { @@ -4058,13 +4057,11 @@ std::vector ChainstateManager::GenerateCoinbaseCommitment(CBlock& out.scriptPubKey[4] = 0xa9; out.scriptPubKey[5] = 0xed; memcpy(&out.scriptPubKey[6], witnessroot.begin(), 32); - commitment = std::vector(out.scriptPubKey.begin(), out.scriptPubKey.end()); CMutableTransaction tx(*block.vtx[0]); tx.vout.push_back(out); block.vtx[0] = MakeTransactionRef(std::move(tx)); } UpdateUncommittedBlockStructures(block, pindexPrev); - return commitment; } bool HasValidProofOfWork(std::span headers, const Consensus::Params& consensusParams) diff --git a/src/validation.h b/src/validation.h index 675d7061ab1..c1468836925 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1310,7 +1310,7 @@ public: void UpdateUncommittedBlockStructures(CBlock& block, const CBlockIndex* pindexPrev) const; /** Produce the necessary coinbase commitment for a block (modifies the hash, don't call for mined blocks). */ - std::vector GenerateCoinbaseCommitment(CBlock& block, const CBlockIndex* pindexPrev) const; + void GenerateCoinbaseCommitment(CBlock& block, const CBlockIndex* pindexPrev) const; /** This is used by net_processing to report pre-synchronization progress of headers, as * headers are not yet fed to validation during that time, but validation is (for now) diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py index a02727bb4c9..0a530827cd1 100755 --- a/test/functional/interface_ipc_mining.py +++ b/test/functional/interface_ipc_mining.py @@ -34,7 +34,6 @@ from test_framework.ipc_util import ( make_capnp_init_ctx, mining_get_block, mining_get_coinbase_tx, - mining_get_coinbase_raw_tx, mining_wait_next_template, wait_and_do, ) @@ -93,27 +92,12 @@ class IPCMiningTest(BitcoinTestFramework): coinbase_tx.vout[0].nValue = coinbase_res.blockRewardRemaining # Add SegWit OP_RETURN. This is currently always present even for # empty blocks, but this may change. - found_witness_op_return = False - # Compare SegWit OP_RETURN to getCoinbaseCommitment() - coinbase_commitment = (await template.getCoinbaseCommitment(ctx)).result for output_data in coinbase_res.requiredOutputs: output = CTxOut() output.deserialize(BytesIO(output_data)) coinbase_tx.vout.append(output) - if output.scriptPubKey == coinbase_commitment: - found_witness_op_return = True - - assert_equal(has_witness, found_witness_op_return) coinbase_tx.nLockTime = coinbase_res.lockTime - # Compare to dummy coinbase transaction provided by the deprecated - # getCoinbaseRawTx() - coinbase_legacy = await mining_get_coinbase_raw_tx(template, ctx) - assert_equal(coinbase_legacy.vout[0].nValue, coinbase_res.blockRewardRemaining) - # Swap dummy output for our own - coinbase_legacy.vout[0].scriptPubKey = coinbase_tx.vout[0].scriptPubKey - assert_equal(coinbase_tx.serialize().hex(), coinbase_legacy.serialize().hex()) - return coinbase_tx async def make_mining_ctx(self): @@ -276,7 +260,7 @@ class IPCMiningTest(BitcoinTestFramework): asyncio.run(capnp.run(async_routine())) def run_coinbase_and_submission_test(self): - """Test coinbase construction (getCoinbaseTx, getCoinbaseCommitment) and block submission (submitSolution).""" + """Test coinbase construction (getCoinbaseTx) and block submission (submitSolution).""" self.log.info("Running coinbase construction and submission test") async def async_routine(): diff --git a/test/functional/test_framework/ipc_util.py b/test/functional/test_framework/ipc_util.py index 1b1ad7a55aa..2ab74abad96 100644 --- a/test/functional/test_framework/ipc_util.py +++ b/test/functional/test_framework/ipc_util.py @@ -12,7 +12,7 @@ from pathlib import Path import shutil from typing import Optional -from test_framework.messages import CBlock, CTransaction +from test_framework.messages import CBlock # Test may be skipped and not have capnp installed try: @@ -129,14 +129,6 @@ async def mining_get_block(block_template, ctx): return block -async def mining_get_coinbase_raw_tx(block_template, ctx): - assert block_template is not None - coinbase_data = BytesIO((await block_template.getCoinbaseRawTx(ctx)).result) - tx = CTransaction() - tx.deserialize(coinbase_data) - return tx - - async def mining_get_coinbase_tx(block_template, ctx) -> CoinbaseTxData: assert block_template is not None # Note: the template_capnp struct will be garbage-collected when this From 70de5cc2d205672743379f2e1a94290ee8b4b84b Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 24 Nov 2025 15:34:45 +0100 Subject: [PATCH 7/9] 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 --- src/ipc/capnp/mining.capnp | 4 ++-- test/functional/interface_ipc_mining.py | 10 +++++----- test/functional/test_framework/ipc_util.py | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/ipc/capnp/mining.capnp b/src/ipc/capnp/mining.capnp index 0727fcc0a50..47cd242b81d 100644 --- a/src/ipc/capnp/mining.capnp +++ b/src/ipc/capnp/mining.capnp @@ -22,8 +22,8 @@ interface Mining $Proxy.wrap("interfaces::Mining") { isInitialBlockDownload @1 (context :Proxy.Context) -> (result: Bool); getTip @2 (context :Proxy.Context) -> (result: Common.BlockRef, hasResult: Bool); waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64 = .maxDouble) -> (result: Common.BlockRef); - createNewBlock @4 (options: BlockCreateOptions) -> (result: BlockTemplate); - checkBlock @5 (block: Data, options: BlockCheckOptions) -> (reason: Text, debug: Text, result: Bool); + createNewBlock @4 (context :Proxy.Context, options: BlockCreateOptions) -> (result: BlockTemplate); + checkBlock @5 (context :Proxy.Context, block: Data, options: BlockCheckOptions) -> (reason: Text, debug: Text, result: Bool); } interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") { diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py index 0a530827cd1..2221d462879 100755 --- a/test/functional/interface_ipc_mining.py +++ b/test/functional/interface_ipc_mining.py @@ -242,7 +242,7 @@ class IPCMiningTest(BitcoinTestFramework): self.log.debug("Enforce minimum reserved weight for IPC clients too") opts.blockReservedWeight = 0 try: - await mining.createNewBlock(opts) + await mining.createNewBlock(ctx, opts) raise AssertionError("createNewBlock unexpectedly succeeded") except capnp.lib.capnp.KjException as e: if e.type == "DISCONNECTED": @@ -269,7 +269,7 @@ class IPCMiningTest(BitcoinTestFramework): current_block_height = self.nodes[0].getchaintips()[0]["height"] check_opts = self.capnp_modules['mining'].BlockCheckOptions() - async with destroying((await mining.createNewBlock(self.default_block_create_options)).result, ctx) as template: + async with destroying((await mining.createNewBlock(ctx, self.default_block_create_options)).result, ctx) as template: block = await mining_get_block(template, ctx) balance = self.miniwallet.get_balance() coinbase = await self.build_coinbase_test(template, ctx, self.miniwallet) @@ -282,7 +282,7 @@ class IPCMiningTest(BitcoinTestFramework): self.log.debug("Submit a block with a bad version") block.nVersion = 0 block.solve() - check = await mining.checkBlock(block.serialize(), check_opts) + check = await mining.checkBlock(ctx, block.serialize(), check_opts) assert_equal(check.result, False) assert_equal(check.reason, "bad-version(0x00000000)") submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())).result @@ -292,7 +292,7 @@ class IPCMiningTest(BitcoinTestFramework): block.solve() self.log.debug("First call checkBlock()") - block_valid = (await mining.checkBlock(block.serialize(), check_opts)).result + block_valid = (await mining.checkBlock(ctx, block.serialize(), check_opts)).result assert_equal(block_valid, True) # The remote template block will be mutated, capture the original: @@ -324,7 +324,7 @@ class IPCMiningTest(BitcoinTestFramework): self.miniwallet.rescan_utxos() assert_equal(self.miniwallet.get_balance(), balance + 1) self.log.debug("Check block should fail now, since it is a duplicate") - check = await mining.checkBlock(block.serialize(), check_opts) + check = await mining.checkBlock(ctx, block.serialize(), check_opts) assert_equal(check.result, False) assert_equal(check.reason, "inconclusive-not-best-prevblk") diff --git a/test/functional/test_framework/ipc_util.py b/test/functional/test_framework/ipc_util.py index 2ab74abad96..11497463eb3 100644 --- a/test/functional/test_framework/ipc_util.py +++ b/test/functional/test_framework/ipc_util.py @@ -108,7 +108,7 @@ async def make_capnp_init_ctx(self): async def mining_create_block_template(mining, stack, ctx, opts): """Call mining.createNewBlock() and return template, then call template.destroy() when stack exits.""" - response = await mining.createNewBlock(opts) + response = await mining.createNewBlock(ctx, opts) if not response._has("result"): return None return await stack.enter_async_context(destroying(response.result, ctx)) From 9453c153612ae9b30308362048099bc53afcde6f Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 24 Nov 2025 15:34:45 +0100 Subject: [PATCH 8/9] 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 --- src/interfaces/init.h | 1 + src/ipc/capnp/init.capnp | 5 ++++- test/functional/interface_ipc.py | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/interfaces/init.h b/src/interfaces/init.h index b909c9e6f6b..463d43e7c2e 100644 --- a/src/interfaces/init.h +++ b/src/interfaces/init.h @@ -39,6 +39,7 @@ public: virtual Ipc* ipc() { return nullptr; } virtual bool canListenIpc() { return false; } virtual const char* exeName() { return nullptr; } + virtual void makeMiningOld2() { throw std::runtime_error("Old mining interface (@2) not supported. Please update your client!"); } }; //! Return implementation of Init interface for the node process. If the argv diff --git a/src/ipc/capnp/init.capnp b/src/ipc/capnp/init.capnp index 64a7bf9b2b4..a20ef2fcaf3 100644 --- a/src/ipc/capnp/init.capnp +++ b/src/ipc/capnp/init.capnp @@ -19,5 +19,8 @@ using Mining = import "mining.capnp"; interface Init $Proxy.wrap("interfaces::Init") { construct @0 (threadMap: Proxy.ThreadMap) -> (threadMap :Proxy.ThreadMap); makeEcho @1 (context :Proxy.Context) -> (result :Echo.Echo); - makeMining @2 (context :Proxy.Context) -> (result :Mining.Mining); + makeMining @3 (context :Proxy.Context) -> (result :Mining.Mining); + + # DEPRECATED: no longer supported; server returns an error. + makeMiningOld2 @2 () -> (); } diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py index f2c75f6e09f..2fc4497a426 100755 --- a/test/functional/interface_ipc.py +++ b/test/functional/interface_ipc.py @@ -68,9 +68,23 @@ class IPCInterfaceTest(BitcoinTestFramework): asyncio.run(capnp.run(async_routine())) + def run_deprecated_mining_test(self): + self.log.info("Running deprecated mining interface test") + async def async_routine(): + ctx, init = await make_capnp_init_ctx(self) + self.log.debug("Calling deprecated makeMiningOld2 should raise an error") + try: + await init.makeMiningOld2() + raise AssertionError("makeMiningOld2 unexpectedly succeeded") + except capnp.KjException as e: + assert_equal(e.description, "remote exception: std::exception: Old mining interface (@2) not supported. Please update your client!") + assert_equal(e.type, "FAILED") + asyncio.run(capnp.run(async_routine())) + def run_test(self): self.run_echo_test() self.run_mining_test() + self.run_deprecated_mining_test() if __name__ == '__main__': IPCInterfaceTest(__file__).main() From f700609e8ada3b48fd45ec19979cd72d943d47a6 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 12 Feb 2026 08:39:11 -0500 Subject: [PATCH 9/9] doc: Release notes for mining IPC interface bump --- doc/release-notes-33819.md | 8 -------- doc/release-notes-34568.md | 11 +++++++++++ 2 files changed, 11 insertions(+), 8 deletions(-) delete mode 100644 doc/release-notes-33819.md create mode 100644 doc/release-notes-34568.md diff --git a/doc/release-notes-33819.md b/doc/release-notes-33819.md deleted file mode 100644 index 79ed1f70749..00000000000 --- a/doc/release-notes-33819.md +++ /dev/null @@ -1,8 +0,0 @@ -Mining IPC ----------- - -- The `getCoinbaseTx()` method is renamed to `getCoinbaseRawTx()` and deprecated. - IPC clients do not use the function name, so they're not affected. (#33819) -- Adds `getCoinbaseTx()` which clients should use instead of `getCoinbaseRawTx()`. It - contains all fields required to construct a coinbase transaction, and omits the - dummy output which Bitcoin Core uses internally. (#33819) diff --git a/doc/release-notes-34568.md b/doc/release-notes-34568.md new file mode 100644 index 00000000000..e48772330c1 --- /dev/null +++ b/doc/release-notes-34568.md @@ -0,0 +1,11 @@ +Mining IPC +---------- + +The IPC mining interface now requires mining clients to use the latest `mining.capnp` schema. Clients built against older schemas will fail when calling `Init.makeMining` and receive an RPC error indicating the old mining interface is no longer supported. Mining clients must update to the latest schema and regenerate bindings to continue working. (#34568) + +Notable IPC mining interface changes since the last release: +- `Mining.createNewBlock` and `Mining.checkBlock` now require a `context` parameter. +- `Mining.waitTipChanged` now has a default `timeout` (effectively infinite / `maxDouble`) if the client omits it. +- `BlockTemplate.getCoinbaseTx()` now returns a structured `CoinbaseTx` instead of raw bytes. +- Removed `BlockTemplate.getCoinbaseCommitment()` and `BlockTemplate.getWitnessCommitmentIndex()`. +- Cap’n Proto default values were updated to match the corresponding C++ defaults for mining-related option structs (e.g. `BlockCreateOptions`, `BlockWaitOptions`, `BlockCheckOptions`).