Merge bitcoin/bitcoin#34568: mining: Break compatibility with existing IPC mining clients

f700609 doc: Release notes for mining IPC interface bump (Ryan Ofsky)
9453c15 ipc mining: break compatibility with existing clients (version bump) (Sjors Provoost)
70de5cc ipc mining: pass missing context to BlockTemplate methods (incompatible schema change) (Sjors Provoost)
2278f01 ipc mining: remove deprecated methods (incompatible schema change) (Ryan Ofsky)
c6638fa ipc mining: provide default option values (incompatible schema change) (Ryan Ofsky)
a4603ac ipc mining: declare constants for default field values (Ryan Ofsky)
ff995b5 ipc test: add workaround to block_reserved_weight exception test (Ryan Ofsky)
b970cdf test framework: expand expected_stderr, expected_ret_code options (Ryan Ofsky)
df53a3e rpc refactor: stop using deprecated getCoinbaseCommitment method (Ryan Ofsky)

Pull request description:

  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.

ACKs for top commit:
  Sjors:
    ACK f700609
  enirox001:
    ACK f700609
  ismaelsadeeq:
    ACK f700609
  sedited:
    ACK f700609

Tree-SHA512: 0901886af00214c138643b33cec21647de5671dfff2021afe06d78dfd970664a844cde9a1e28f685bb27edccaf6e0c3f2d1e6bb4164bde6b84f42955946e366d

Author: sedited

doc/release-notes-33819.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`).

doc/release-notes-34568.md

@@ -39,6 +39,7 @@ class Init
     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

src/interfaces/init.h

@@ -42,31 +42,9 @@ class BlockTemplate
     // 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/interfaces/mining.h

@@ -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 () -> ();
 }

src/ipc/capnp/init.capnp

@@ -12,13 +12,18 @@ 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);
     getTip @2 (context :Proxy.Context) -> (result: Common.BlockRef, hasResult: Bool);
-    waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64) -> (result: Common.BlockRef);
-    createNewBlock @4 (options: BlockCreateOptions) -> (result: BlockTemplate);
-    checkBlock @5 (block: Data, options: BlockCheckOptions) -> (reason: Text, debug: Text, result: Bool);
+    waitTipChanged @3 (context :Proxy.Context, currentTip: Data, timeout: Float64 = .maxDouble) -> (result: Common.BlockRef);
+    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") {
@@ -27,30 +32,27 @@ 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") {
-    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") {

src/ipc/capnp/mining.capnp

@@ -8,6 +8,7 @@
 #include <ipc/protocol.h>
 #include <logging.h>
 #include <mp/proxy-types.h>
+#include <ipc/capnp/mining.capnp.h>
 #include <ipc/test/ipc_test.capnp.h>
 #include <ipc/test/ipc_test.capnp.proxy.h>
 #include <ipc/test/ipc_test.h>
@@ -23,6 +24,11 @@
 
 #include <boost/test/unit_test.hpp>
 
+static_assert(ipc::capnp::messages::MAX_MONEY == MAX_MONEY);
+static_assert(ipc::capnp::messages::MAX_DOUBLE == std::numeric_limits<double>::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
 {

src/ipc/test/ipc_test.cpp

@@ -890,26 +890,11 @@ class BlockTemplateImpl : public BlockTemplate
         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/interfaces.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.cpp

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