Drop script_pub_key arg from createNewBlock

This is a very sloppy merge conflict resultation
between bitcoin#31283 and bitcoin#31318.

A temporary overload is added so that the test can be modified in the
next commit.

src/interfaces/mining.h

@@ -101,11 +101,10 @@ class Mining
    /**
      * Construct a new block template
      *
-     * @param[in] script_pub_key the coinbase output
      * @param[in] options options for creating the block
      * @returns a block template
      */
-    virtual std::unique_ptr<BlockTemplate> createNewBlock(const CScript& script_pub_key, const node::BlockCreateOptions& options = {}) = 0;
+    virtual std::unique_ptr<BlockTemplate> createNewBlock(const node::BlockCreateOptions& options = {}) = 0;
 
     /**
      * Processes new block. A valid new block is automatically relayed to peers.

src/ipc/capnp/mining.capnp

@@ -17,7 +17,7 @@ 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) -> (result: Common.BlockRef);
-    createNewBlock @4 (scriptPubKey: Data, options: BlockCreateOptions) -> (result: BlockTemplate);
+    createNewBlock @4 (options: BlockCreateOptions) -> (result: BlockTemplate);
     processNewBlock @5 (context :Proxy.Context, block: Data) -> (newBlock: Bool, result: Bool);
     getTransactionsUpdated @6 (context :Proxy.Context) -> (result: UInt32);
     testBlockValidity @7 (context :Proxy.Context, block: Data, checkMerkleRoot: Bool) -> (state: BlockValidationState, result: Bool);

src/node/interfaces.cpp

@@ -871,7 +871,7 @@ class ChainImpl : public Chain
 class BlockTemplateImpl : public BlockTemplate
 {
 public:
-    explicit BlockTemplateImpl(CScript script_pub_key, BlockAssembler::Options assemble_options, std::unique_ptr<CBlockTemplate> block_template, NodeContext& node) : m_script_pub_key(script_pub_key), m_assemble_options(std::move(assemble_options)), m_block_template(std::move(block_template)), m_node(node)
+    explicit BlockTemplateImpl(BlockAssembler::Options assemble_options, std::unique_ptr<CBlockTemplate> block_template, NodeContext& node) : m_assemble_options(std::move(assemble_options)), m_block_template(std::move(block_template)), m_node(node)
     {
         assert(m_block_template);
     }
@@ -977,7 +977,7 @@ class BlockTemplateImpl : public BlockTemplate
              * We'll also create a new template if the tip changed during the last tick.
              */
             if (fee_threshold < MAX_MONEY || tip_changed) {
-                auto block_template{std::make_unique<BlockTemplateImpl>(m_script_pub_key, m_assemble_options, BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), m_assemble_options}.CreateNewBlock(m_script_pub_key), m_node)};
+                auto block_template{std::make_unique<BlockTemplateImpl>(m_assemble_options, BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), m_assemble_options}.CreateNewBlock(), m_node)};
 
                 // If the tip changed, return the new template regardless of its fees.
                 if (block_template->m_block_template->block.hashPrevBlock != m_block_template->block.hashPrevBlock) {
@@ -1009,7 +1009,6 @@ class BlockTemplateImpl : public BlockTemplate
         return nullptr;
     }
 
-    const CScript m_script_pub_key;
     const BlockAssembler::Options m_assemble_options;
 
     const std::unique_ptr<CBlockTemplate> m_block_template;
@@ -1083,11 +1082,11 @@ class MinerImpl : public Mining
         return TestBlockValidity(state, chainman().GetParams(), chainman().ActiveChainstate(), block, tip, /*fCheckPOW=*/false, check_merkle_root);
     }
 
-    std::unique_ptr<BlockTemplate> createNewBlock(const CScript& script_pub_key, const BlockCreateOptions& options) override
+    std::unique_ptr<BlockTemplate> createNewBlock(const BlockCreateOptions& options) override
     {
         BlockAssembler::Options assemble_options{options};
         ApplyArgsManOptions(*Assert(m_node.args), assemble_options);
-        return std::make_unique<BlockTemplateImpl>(script_pub_key, assemble_options, BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), assemble_options}.CreateNewBlock(script_pub_key), m_node);
+        return std::make_unique<BlockTemplateImpl>(assemble_options, BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), assemble_options}.CreateNewBlock(), m_node);
     }
 
     NodeContext* context() override { return &m_node; }

src/node/miner.cpp

@@ -106,7 +106,7 @@ void BlockAssembler::resetBlock()
     nFees = 0;
 }
 
-std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn)
+std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock()
 {
     const auto time_start{SteadyClock::now()};
 
@@ -151,7 +151,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
     coinbaseTx.vin.resize(1);
     coinbaseTx.vin[0].prevout.SetNull();
     coinbaseTx.vout.resize(1);
-    coinbaseTx.vout[0].scriptPubKey = scriptPubKeyIn;
+    coinbaseTx.vout[0].scriptPubKey = m_options.coinbase_output_script;
     coinbaseTx.vout[0].nValue = nFees + GetBlockSubsidy(nHeight, chainparams.GetConsensus());
     coinbaseTx.vin[0].scriptSig = CScript() << nHeight << OP_0;
     pblock->vtx[0] = MakeTransactionRef(std::move(coinbaseTx));

src/node/miner.h

@@ -169,14 +169,22 @@ class BlockAssembler
 
     explicit BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool, const Options& options);
 
-    /** Construct a new block template with coinbase to scriptPubKeyIn */
-    std::unique_ptr<CBlockTemplate> CreateNewBlock(const CScript& scriptPubKeyIn);
+    /** Construct a new block template */
+    std::unique_ptr<CBlockTemplate> CreateNewBlock();
+
+    /** Temporary overload for tests */
+    std::unique_ptr<CBlockTemplate> CreateNewBlock(const CScript& scriptPubKeyIn)
+    {
+        m_options.coinbase_output_script = scriptPubKeyIn;
+        return CreateNewBlock();
+    };
 
     inline static std::optional<int64_t> m_last_block_num_txs{};
     inline static std::optional<int64_t> m_last_block_weight{};
 
 private:
-    const Options m_options;
+    // TODO: make const again
+    Options m_options;
 
     // utility functions
     /** Clear the block's state and prepare for assembling a new block */

src/node/types.h

@@ -3,8 +3,8 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 //! @file node/types.h is a home for public enum and struct type definitions
-//! that are used by internally by node code, but also used externally by wallet
-//! or GUI code.
+//! that are used by internally by node code, but also used externally by wallet,
+//! mining or GUI code.
 //!
 //! This file is intended to define only simple types that do not have external
 //! dependencies. More complicated types should be defined in dedicated header
@@ -14,6 +14,7 @@
 #define BITCOIN_NODE_TYPES_H
 
 #include <cstddef>
+#include <script/script.h>
 
 namespace node {
 enum class TransactionError {
@@ -43,6 +44,22 @@ struct BlockCreateOptions {
      * transaction outputs.
      */
     size_t coinbase_output_max_additional_sigops{400};
+    /**
+     * Script to put in the coinbase transaction. The default is an
+     * anyone-can-spend dummy.
+     *
+     * Should only be used for tests, when the default doesn't suffice.
+     *
+     * Note that higher level code like the getblocktemplate RPC may omit the
+     * coinbase transaction entirely. It's instead constructed by pool software
+     * using fields like coinbasevalue, coinbaseaux and default_witness_commitment.
+     * This software typically also controls the payout outputs, even for solo
+     * mining.
+     *
+     * The size and sigops are not checked against
+     * coinbase_max_additional_weight and coinbase_output_max_additional_sigops.
+     */
+    CScript coinbase_output_script{CScript() << OP_TRUE};
 };
 } // namespace node
 

src/rpc/mining.cpp

@@ -162,7 +162,7 @@ static UniValue generateBlocks(ChainstateManager& chainman, Mining& miner, const
 {
     UniValue blockHashes(UniValue::VARR);
     while (nGenerate > 0 && !chainman.m_interrupt) {
-        std::unique_ptr<BlockTemplate> block_template(miner.createNewBlock(coinbase_script));
+        std::unique_ptr<BlockTemplate> block_template(miner.createNewBlock({ .coinbase_output_script = coinbase_script }));
         CHECK_NONFATAL(block_template);
 
         std::shared_ptr<const CBlock> block_out;
@@ -371,7 +371,7 @@ static RPCHelpMan generateblock()
 
     ChainstateManager& chainman = EnsureChainman(node);
     {
-        std::unique_ptr<BlockTemplate> block_template{miner.createNewBlock(coinbase_script, {.use_mempool = false})};
+        std::unique_ptr<BlockTemplate> block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_script})};
         CHECK_NONFATAL(block_template);
 
         block = block_template->getBlock();
@@ -814,8 +814,7 @@ static RPCHelpMan getblocktemplate()
         time_start = GetTime();
 
         // Create new block
-        CScript scriptDummy = CScript() << OP_TRUE;
-        block_template = miner.createNewBlock(scriptDummy);
+        block_template = miner.createNewBlock();
         CHECK_NONFATAL(block_template);
 
 

src/sv2/template_provider.cpp

@@ -197,7 +197,7 @@ void Sv2TemplateProvider::ThreadSv2Handler()
             uint32_t additional_coinbase_weight{(client.m_coinbase_tx_outputs_size + 100 + 0 + 2) * 4};
 
             const auto time_start{SteadyClock::now()};
-            auto block_template = m_mining.createNewBlock(CScript(), {.use_mempool = true, .coinbase_max_additional_weight = additional_coinbase_weight});
+            auto block_template = m_mining.createNewBlock({.use_mempool = true, .coinbase_max_additional_weight = additional_coinbase_weight});
             LogPrintLevel(BCLog::SV2, BCLog::Level::Trace, "Assemble template: %.2fms\n",
                 Ticks<MillisecondsDouble>(SteadyClock::now() - time_start));
 

src/test/miner_tests.cpp

@@ -133,7 +133,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
     Txid hashHighFeeTx = tx.GetHash();
     AddToMempool(tx_mempool, entry.Fee(50000).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx));
 
-    std::unique_ptr<BlockTemplate> block_template = miner->createNewBlock(scriptPubKey);
+    std::unique_ptr<BlockTemplate> block_template = miner->createNewBlock();
     CBlock block{block_template->getBlock()};
     BOOST_REQUIRE_EQUAL(block.vtx.size(), 4U);
     BOOST_CHECK(block.vtx[1]->GetHash() == hashParentTx);
@@ -204,7 +204,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
     tx.vout[0].nValue = 5000000000LL - 100000000 - feeToUse;
     Txid hashLowFeeTx2 = tx.GetHash();
     AddToMempool(tx_mempool, entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx));
-    block_template = miner->createNewBlock(scriptPubKey);
+    block_template = miner->createNewBlock();
     BOOST_REQUIRE(block_template);
     block = block_template->getBlock();
 
@@ -219,7 +219,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
     tx.vin[0].prevout.n = 1;
     tx.vout[0].nValue = 100000000 - 10000; // 10k satoshi fee
     AddToMempool(tx_mempool, entry.Fee(10000).FromTx(tx));
-    block_template = miner->createNewBlock(scriptPubKey);
+    block_template = miner->createNewBlock();
     BOOST_REQUIRE(block_template);
     block = block_template->getBlock();
     BOOST_REQUIRE_EQUAL(block.vtx.size(), 9U);
@@ -247,7 +247,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
         LOCK(tx_mempool.cs);
 
         // Just to make sure we can still make simple blocks
-        auto block_template{miner->createNewBlock(scriptPubKey)};
+        auto block_template{miner->createNewBlock()};
         BOOST_REQUIRE(block_template);
         CBlock block{block_template->getBlock()};
 
@@ -268,7 +268,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
             tx.vin[0].prevout.hash = hash;
         }
 
-        BOOST_CHECK_EXCEPTION(miner->createNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-blk-sigops"));
+        BOOST_CHECK_EXCEPTION(miner->createNewBlock(), std::runtime_error, HasReason("bad-blk-sigops"));
     }
 
     {
@@ -285,7 +285,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
             AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(spendsCoinbase).SigOpsCost(80).FromTx(tx));
             tx.vin[0].prevout.hash = hash;
         }
-        BOOST_CHECK(miner->createNewBlock(scriptPubKey));
+        BOOST_CHECK(miner->createNewBlock());
     }
 
     {
@@ -309,7 +309,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
             AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(spendsCoinbase).FromTx(tx));
             tx.vin[0].prevout.hash = hash;
         }
-        BOOST_CHECK(miner->createNewBlock(scriptPubKey));
+        BOOST_CHECK(miner->createNewBlock());
     }
 
     {
@@ -319,7 +319,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
         // orphan in tx_mempool, template creation fails
         hash = tx.GetHash();
         AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).FromTx(tx));
-        BOOST_CHECK_EXCEPTION(miner->createNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
+        BOOST_CHECK_EXCEPTION(miner->createNewBlock(), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
     }
 
     {
@@ -340,7 +340,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
         tx.vout[0].nValue = tx.vout[0].nValue + BLOCKSUBSIDY - HIGHERFEE; // First txn output + fresh coinbase - new txn fee
         hash = tx.GetHash();
         AddToMempool(tx_mempool, entry.Fee(HIGHERFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
-        BOOST_CHECK(miner->createNewBlock(scriptPubKey));
+        BOOST_CHECK(miner->createNewBlock());
     }
 
     {
@@ -356,7 +356,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
         // give it a fee so it'll get mined
         AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx));
         // Should throw bad-cb-multiple
-        BOOST_CHECK_EXCEPTION(miner->createNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-cb-multiple"));
+        BOOST_CHECK_EXCEPTION(miner->createNewBlock(), std::runtime_error, HasReason("bad-cb-multiple"));
     }
 
     {
@@ -373,7 +373,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
         tx.vout[0].scriptPubKey = CScript() << OP_2;
         hash = tx.GetHash();
         AddToMempool(tx_mempool, entry.Fee(HIGHFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
-        BOOST_CHECK_EXCEPTION(miner->createNewBlock(scriptPubKey), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
+        BOOST_CHECK_EXCEPTION(miner->createNewBlock(), std::runtime_error, HasReason("bad-txns-inputs-missingorspent"));
     }
 
     {
@@ -393,7 +393,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
             next->BuildSkip();
             m_node.chainman->ActiveChain().SetTip(*next);
         }
-        BOOST_CHECK(miner->createNewBlock(scriptPubKey));
+        BOOST_CHECK(miner->createNewBlock());
         // Extend to a 210000-long block chain.
         while (m_node.chainman->ActiveChain().Tip()->nHeight < 210000) {
             CBlockIndex* prev = m_node.chainman->ActiveChain().Tip();
@@ -405,7 +405,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
             next->BuildSkip();
             m_node.chainman->ActiveChain().SetTip(*next);
         }
-        BOOST_CHECK(miner->createNewBlock(scriptPubKey));
+        BOOST_CHECK(miner->createNewBlock());
 
         // invalid p2sh txn in tx_mempool, template creation fails
         tx.vin[0].prevout.hash = txFirst[0]->GetHash();
@@ -422,7 +422,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
         hash = tx.GetHash();
         AddToMempool(tx_mempool, entry.Fee(LOWFEE).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx));
         // Should throw block-validation-failed
-        BOOST_CHECK_EXCEPTION(miner->createNewBlock(scriptPubKey), std::runtime_error, HasReason("block-validation-failed"));
+        BOOST_CHECK_EXCEPTION(miner->createNewBlock(), std::runtime_error, HasReason("block-validation-failed"));
 
         // Delete the dummy blocks again.
         while (m_node.chainman->ActiveChain().Tip()->nHeight > nHeight) {
@@ -524,7 +524,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
     tx.vin[0].nSequence = CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG | 1;
     BOOST_CHECK(!TestSequenceLocks(CTransaction{tx}, tx_mempool)); // Sequence locks fail
 
-    auto block_template = miner->createNewBlock(scriptPubKey);
+    auto block_template = miner->createNewBlock();
     BOOST_CHECK(block_template);
 
     // None of the of the absolute height/time locked tx should have made
@@ -541,7 +541,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::
     m_node.chainman->ActiveChain().Tip()->nHeight++;
     SetMockTime(m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1);
 
-    block_template = miner->createNewBlock(scriptPubKey);
+    block_template = miner->createNewBlock();
     BOOST_REQUIRE(block_template);
     block = block_template->getBlock();
     BOOST_CHECK_EQUAL(block.vtx.size(), 5U);
@@ -613,7 +613,7 @@ void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const
     Txid hashFreeGrandchild = tx.GetHash();
     AddToMempool(tx_mempool, entry.Fee(0).SpendsCoinbase(false).FromTx(tx));
 
-    auto block_template = miner->createNewBlock(scriptPubKey);
+    auto block_template = miner->createNewBlock();
     CBlock block{block_template->getBlock()};
     BOOST_REQUIRE_EQUAL(block.vtx.size(), 6U);
     BOOST_CHECK(block.vtx[1]->GetHash() == hashFreeParent);
@@ -649,7 +649,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
 
         // Simple block creation, nothing special yet:
         if (current_height % 2 == 0) {
-            block_template = miner->createNewBlock(scriptPubKey);
+            block_template = miner->createNewBlock();
         } // for odd heights the next template is created at the end of this loop
         BOOST_REQUIRE(block_template);