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

@@ -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
      *

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 @5 (context: Proxy.Context) -> (result: CoinbaseTx);
-    getCoinbaseTx @12 (context: Proxy.Context) -> (result: CoinbaseTx);
+    getCoinbaseMerklePath @6 (context: Proxy.Context) -> (result: List(Data));
-    getCoinbaseCommitment @6 (context: Proxy.Context) -> (result: Data);
+    submitSolution @7 (context: Proxy.Context, version: UInt32, timestamp: UInt32, nonce: UInt32, coinbase :Data) -> (result: Bool);
-    getWitnessCommitmentIndex @7 (context: Proxy.Context) -> (result: Int32);
+    waitNext @8 (context: Proxy.Context, options: BlockWaitOptions) -> (result: BlockTemplate);
-    getCoinbaseMerklePath @8 (context: Proxy.Context) -> (result: List(Data));
+    interruptWait @9() -> ();
-    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() -> ();
 }
 
 struct BlockCreateOptions $Proxy.wrap("node::BlockCreateOptions") {

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);

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()) {

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;

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)

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)

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():

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