mining: add new getCoinbaseTx() returning a struct

Introduce a new method intended to replace getCoinbaseRawTx(), which
provides a struct with everything clients need to construct a coinbase.
This is safer than providing a raw dummy coinbase that clients then have
to manipulate.

The CoinbaseTx data is populated during the dummy transaction generation
and stored in struct CBlockTemplate.

Expand the interface_ipc.py functional test to document its usage
and ensure equivalence.

src/interfaces/mining.h

@@ -44,16 +44,26 @@ public:
 
     /**
      * 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;
 

src/ipc/capnp/mining.capnp

@@ -28,6 +28,7 @@ interface BlockTemplate $Proxy.wrap("interfaces::BlockTemplate") {
     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));
@@ -51,3 +52,13 @@ struct BlockCheckOptions $Proxy.wrap("node::BlockCheckOptions") {
     checkMerkleRoot @0 :Bool $Proxy.name("check_merkle_root");
     checkPow @1 :Bool $Proxy.name("check_pow");
 }
+
+struct CoinbaseTx $Proxy.wrap("node::CoinbaseTx") {
+    version @0 :UInt32 $Proxy.name("version");
+    sequence @1 :UInt32 $Proxy.name("sequence");
+    scriptSigPrefix @2 :Data $Proxy.name("script_sig_prefix");
+    witness @3 :Data $Proxy.name("witness");
+    blockRewardRemaining @4 :Int64 $Proxy.name("block_reward_remaining");
+    requiredOutputs @5 :List(Data) $Proxy.name("required_outputs");
+    lockTime @6 :UInt32 $Proxy.name("lock_time");
+}

src/node/interfaces.cpp

@@ -84,6 +84,7 @@ using interfaces::WalletLoader;
 using kernel::ChainstateRole;
 using node::BlockAssembler;
 using node::BlockWaitOptions;
+using node::CoinbaseTx;
 using util::Join;
 
 namespace node {
@@ -893,6 +894,11 @@ public:
         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;

src/node/miner.cpp

@@ -158,18 +158,51 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock()
 
     // Create coinbase transaction.
     CMutableTransaction coinbaseTx;
+
+    // Construct coinbase transaction struct in parallel
+    CoinbaseTx& coinbase_tx{pblocktemplate->m_coinbase_tx};
+    coinbase_tx.version = coinbaseTx.version;
+
     coinbaseTx.vin.resize(1);
     coinbaseTx.vin[0].prevout.SetNull();
     coinbaseTx.vin[0].nSequence = CTxIn::MAX_SEQUENCE_NONFINAL; // Make sure timelock is enforced.
+    coinbase_tx.sequence = coinbaseTx.vin[0].nSequence;
+
+    // Add an output that spends the full coinbase reward.
     coinbaseTx.vout.resize(1);
     coinbaseTx.vout[0].scriptPubKey = m_options.coinbase_output_script;
-    coinbaseTx.vout[0].nValue = nFees + GetBlockSubsidy(nHeight, chainparams.GetConsensus());
+    // Block subsidy + fees
+    const CAmount block_reward{nFees + GetBlockSubsidy(nHeight, chainparams.GetConsensus())};
+    coinbaseTx.vout[0].nValue = block_reward;
+    coinbase_tx.block_reward_remaining = block_reward;
+
+    // Start the coinbase scriptSig with the block height as required by BIP34.
+    // The trailing OP_0 (historically an extranonce) is optional padding and
+    // could be removed without a consensus change. Mining clients are expected
+    // to append extra data to this prefix, so increasing its length would reduce
+    // the space they can use and may break existing clients.
     coinbaseTx.vin[0].scriptSig = CScript() << nHeight << OP_0;
+    coinbase_tx.script_sig_prefix = coinbaseTx.vin[0].scriptSig;
     Assert(nHeight > 0);
     coinbaseTx.nLockTime = static_cast<uint32_t>(nHeight - 1);
+    coinbase_tx.lock_time = coinbaseTx.nLockTime;
+
     pblock->vtx[0] = MakeTransactionRef(std::move(coinbaseTx));
     pblocktemplate->vchCoinbaseCommitment = m_chainstate.m_chainman.GenerateCoinbaseCommitment(*pblock, pindexPrev);
 
+    const CTransactionRef& final_coinbase{pblock->vtx[0]};
+    if (final_coinbase->HasWitness()) {
+        const auto& witness_stack{final_coinbase->vin[0].scriptWitness.stack};
+        // Consensus requires the coinbase witness stack to have exactly one
+        // element of 32 bytes.
+        Assert(witness_stack.size() == 1 && witness_stack[0].size() == 32);
+        coinbase_tx.witness = uint256(witness_stack[0]);
+    }
+    if (const int witness_index = GetWitnessCommitmentIndex(*pblock); witness_index != NO_WITNESS_COMMITMENT) {
+        Assert(witness_index >= 0 && static_cast<size_t>(witness_index) < final_coinbase->vout.size());
+        coinbase_tx.required_outputs.push_back(final_coinbase->vout[witness_index]);
+    }
+
     LogInfo("CreateNewBlock(): block weight: %u txs: %u fees: %ld sigops %d\n", GetBlockWeight(*pblock), nBlockTx, nFees, nBlockSigOpsCost);
 
     // Fill in header
@@ -440,4 +473,5 @@ std::optional<BlockRef> WaitTipChanged(ChainstateManager& chainman, KernelNotifi
     // avoid deadlocks.
     return GetTip(chainman);
 }
+
 } // namespace node

src/node/miner.h

@@ -50,6 +50,11 @@ struct CBlockTemplate
     /* 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;
+    /*
+     * Template containing all coinbase transaction fields that are set by our
+     * miner code.
+     */
+    CoinbaseTx m_coinbase_tx;
 };
 
 /** Generate a new block, without valid proof-of-work */
@@ -157,6 +162,7 @@ std::optional<BlockRef> GetTip(ChainstateManager& chainman);
 /* Waits for the connected tip to change until timeout has elapsed. During node initialization, this will wait until the tip is connected (regardless of `timeout`).
  * Returns the current tip, or nullopt if the node is shutting down. */
 std::optional<BlockRef> WaitTipChanged(ChainstateManager& chainman, KernelNotifications& kernel_notifications, const uint256& current_tip, MillisecondsDouble& timeout);
+
 } // namespace node
 
 #endif // BITCOIN_NODE_MINER_H

src/node/types.h

@@ -16,10 +16,13 @@
 #include <consensus/amount.h>
 #include <cstddef>
 #include <cstdint>
+#include <optional>
 #include <policy/policy.h>
+#include <primitives/transaction.h>
 #include <script/script.h>
 #include <uint256.h>
 #include <util/time.h>
+#include <vector>
 
 namespace node {
 enum class TransactionError {
@@ -99,6 +102,53 @@ struct BlockCheckOptions {
     bool check_pow{true};
 };
 
+/**
+ * Template containing all coinbase transaction fields that are set by our
+ * miner code. Clients are expected to add their own outputs and typically
+ * also expand the scriptSig.
+ */
+struct CoinbaseTx {
+    /* nVersion */
+    uint32_t version;
+    /* nSequence for the only coinbase transaction input */
+    uint32_t sequence;
+    /**
+     * Prefix which needs to be placed at the beginning of the scriptSig.
+     * Clients may append extra data to this as long as the overall scriptSig
+     * size is 100 bytes or less, to avoid the block being rejected with
+     * "bad-cb-length" error.
+     *
+     * Currently with BIP 34, the prefix is guaranteed to be less than 8 bytes,
+     * but future soft forks could require longer prefixes.
+     */
+    CScript script_sig_prefix;
+    /**
+     * The first (and only) witness stack element of the coinbase input.
+     *
+     * Omitted for block templates without witness data.
+     *
+     * This is currently the BIP 141 witness reserved value, and can be chosen
+     * arbitrarily by the node, but future soft forks may constrain it.
+     */
+    std::optional<uint256> witness;
+    /**
+     * Block subsidy plus fees, minus any non-zero required_outputs.
+     *
+     * Currently there are no non-zero required_outputs, so block_reward_remaining
+     * is the entire block reward. See also required_outputs.
+     */
+    CAmount block_reward_remaining;
+    /*
+     * To be included as the last outputs in the coinbase transaction.
+     * Currently this is only the witness commitment OP_RETURN, but future
+     * softforks or a custom mining patch could add more.
+     *
+     * The dummy output that spends the full reward is excluded.
+     */
+    std::vector<CTxOut> required_outputs;
+    uint32_t lock_time;
+};
+
 /**
  * How to broadcast a local transaction.
  * Used to influence `BroadcastTransaction()` and its callers.