ipc mining: remove deprecated methods (incompatible schema change)
What changed, and why it matters
This is a Bitcoin Core internal cleanup commit that removes old, deprecated methods from the inter-process communication (IPC) mining interface. The commit itself is explicitly marked as an incomplete, review-only change that should not be used in production. The main risk is that, because the interface version number was not bumped in this intermediate commit, two differently-versioned programs could talk to each other using mismatched method numbers and exchange meaningless data instead of getting a clean error. That mismatch is a protocol-handling bug, not a cryptographic or funds-theft vulnerability, and the commit message says the next commit in the series will fix it by bumping the version.
Do not build or deploy binaries from this intermediate commit. Ensure the final commit in the series that bumps the mining interface version (Init.makeMining) is included before any release or interop testing. Reviewers should verify that the version bump and any needed backward-compatibility or error-handling logic are present in the follow-up commit.
Security signals we found
Incompatible schema change without version bump
Renumbering of capnp interface methods
Removal of deprecated IPC methods
Commit explicitly labeled as intermediate/review-only and git-bisect-skip
Potential garbage request/response exchange between mismatched IPC peers
Evidence from the diff
The commit deletes three deprecated BlockTemplate interface methods (getCoinbaseRawTx, getCoinbaseCommitment, getWitnessCommitmentIndex) and renumbers the remaining capnp schema methods. It also removes the vchCoinbaseCommitment field from CBlockTemplate and changes ChainstateManager::GenerateCoinbaseCommitment to return void. The commit message warns that this is an intermediate review-only commit and that the mining.capnp schema change is incompatible without an Init.makeMining version bump, so mixed-version IPC peers may exchange garbage requests/responses rather than failing cleanly. The actual security impact is low because the affected interface is an experimental, non-default IPC path and the commit is not intended for distribution.
Changed components
src/interfaces/mining.hsrc/ipc/capnp/mining.capnpsrc/node/interfaces.cppsrc/node/miner.cppsrc/node/miner.hsrc/validation.cppsrc/validation.htest/functional/interface_ipc_mining.pytest/functional/test_framework/ipc_util.pyInspect captured patch +10 / −78
diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h
index 993f70bd..42002284 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<int64_t> 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<unsigned char> 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 1811716f..0727fcc0 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 af9388d4..8f5406ab 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<unsigned char> getCoinbaseCommitment() override
- {
- return m_block_template->vchCoinbaseCommitment;
- }
-
- int getWitnessCommitmentIndex() override
- {
- return GetWitnessCommitmentIndex(m_block_template->block);
- }
-
std::vector<uint256> getCoinbaseMerklePath() override
{
return TransactionMerklePath(m_block_template->block, 0);
diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index 7bdd8b69..fe267945 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -197,7 +197,7 @@ std::unique_ptr<CBlockTemplate> 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 0c268f18..5a338dfb 100644
--- a/src/node/miner.h
+++ b/src/node/miner.h
@@ -46,7 +46,6 @@ struct CBlockTemplate
std::vector<CAmount> vTxFees;
// Sigops per transaction, not including coinbase transaction (unlike CBlock::vtx).
std::vector<int64_t> vTxSigOpsCost;
- std::vector<unsigned char> vchCoinbaseCommitment;
/* A vector of package fee rates, ordered by the sequence in which
* packages are selected for inclusion in the block template.*/
std::vector<FeePerVSize> m_package_feerates;
diff --git a/src/validation.cpp b/src/validation.cpp
index c200c3d6..b7a113ea 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4040,9 +4040,8 @@ void ChainstateManager::UpdateUncommittedBlockStructures(CBlock& block, const CB
}
}
-std::vector<unsigned char> ChainstateManager::GenerateCoinbaseCommitment(CBlock& block, const CBlockIndex* pindexPrev) const
+void ChainstateManager::GenerateCoinbaseCommitment(CBlock& block, const CBlockIndex* pindexPrev) const
{
- std::vector<unsigned char> commitment;
int commitpos = GetWitnessCommitmentIndex(block);
std::vector<unsigned char> ret(32, 0x00);
if (commitpos == NO_WITNESS_COMMITMENT) {
@@ -4058,13 +4057,11 @@ std::vector<unsigned char> ChainstateManager::GenerateCoinbaseCommitment(CBlock&
out.scriptPubKey[4] = 0xa9;
out.scriptPubKey[5] = 0xed;
memcpy(&out.scriptPubKey[6], witnessroot.begin(), 32);
- commitment = std::vector<unsigned char>(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<const CBlockHeader> headers, const Consensus::Params& consensusParams)
diff --git a/src/validation.h b/src/validation.h
index 675d7061..c1468836 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<unsigned char> 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 a02727bb..0a530827 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 1b1ad7a5..2ab74aba 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
Why this scored 21/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.