test: drop scriptPubKeyIn arg from CreateNewBlock

This removes the temporary overload added in the previous commit.

Also drop unneeded custom coinbase output scripts.

Author: Sjors

src/bench/block_assemble.cpp

@@ -20,19 +20,23 @@
 #include <memory>
 #include <vector>
 
+using node::BlockAssembler;
+
 static void AssembleBlock(benchmark::Bench& bench)
 {
     const auto test_setup = MakeNoLogFileContext<const TestingSetup>();
 
     CScriptWitness witness;
     witness.stack.push_back(WITNESS_STACK_ELEM_OP_TRUE);
+    BlockAssembler::Options options;
+    options.coinbase_output_script = P2WSH_OP_TRUE;
 
     // Collect some loose transactions that spend the coinbases of our mined blocks
     constexpr size_t NUM_BLOCKS{200};
     std::array<CTransactionRef, NUM_BLOCKS - COINBASE_MATURITY + 1> txs;
     for (size_t b{0}; b < NUM_BLOCKS; ++b) {
         CMutableTransaction tx;
-        tx.vin.emplace_back(MineBlock(test_setup->m_node, P2WSH_OP_TRUE));
+        tx.vin.emplace_back(MineBlock(test_setup->m_node, options));
         tx.vin.back().scriptWitness = witness;
         tx.vout.emplace_back(1337, P2WSH_OP_TRUE);
         if (NUM_BLOCKS - b >= COINBASE_MATURITY)
@@ -48,19 +52,20 @@ static void AssembleBlock(benchmark::Bench& bench)
     }
 
     bench.run([&] {
-        PrepareBlock(test_setup->m_node, P2WSH_OP_TRUE);
+        PrepareBlock(test_setup->m_node, options);
     });
 }
 static void BlockAssemblerAddPackageTxns(benchmark::Bench& bench)
 {
     FastRandomContext det_rand{true};
     auto testing_setup{MakeNoLogFileContext<TestChain100Setup>()};
     testing_setup->PopulateMempool(det_rand, /*num_transactions=*/1000, /*submit=*/true);
-    node::BlockAssembler::Options assembler_options;
+    BlockAssembler::Options assembler_options;
     assembler_options.test_block_validity = false;
+    assembler_options.coinbase_output_script = P2WSH_OP_TRUE;
 
     bench.run([&] {
-        PrepareBlock(testing_setup->m_node, P2WSH_OP_TRUE, assembler_options);
+        PrepareBlock(testing_setup->m_node, assembler_options);
     });
 }
 

src/node/miner.h

@@ -172,19 +172,11 @@ class BlockAssembler
     /** 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:
-    // TODO: make const again
-    Options m_options;
+    const Options m_options;
 
     // utility functions
     /** Clear the block's state and prepare for assembling a new block */

src/test/blockfilter_index_tests.cpp

@@ -67,8 +67,9 @@ CBlock BuildChainTestingSetup::CreateBlock(const CBlockIndex* prev,
     const std::vector<CMutableTransaction>& txns,
     const CScript& scriptPubKey)
 {
-     BlockAssembler::Options options;
-    std::unique_ptr<CBlockTemplate> pblocktemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock(scriptPubKey);
+    BlockAssembler::Options options;
+    options.coinbase_output_script = scriptPubKey;
+    std::unique_ptr<CBlockTemplate> pblocktemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock();
     CBlock& block = pblocktemplate->block;
     block.hashPrevBlock = prev->GetBlockHash();
     block.nTime = prev->nTime + 1;

src/test/fuzz/mini_miner.cpp

@@ -174,15 +174,15 @@ FUZZ_TARGET(mini_miner_selection, .init = initialize_miner)
     miner_options.blockMinFeeRate = target_feerate;
     miner_options.nBlockMaxWeight = DEFAULT_BLOCK_MAX_WEIGHT;
     miner_options.test_block_validity = false;
+    miner_options.coinbase_output_script = CScript() << OP_0;
 
     node::BlockAssembler miner{g_setup->m_node.chainman->ActiveChainstate(), &pool, miner_options};
     node::MiniMiner mini_miner{pool, outpoints};
     assert(mini_miner.IsReadyToCalculate());
 
-    CScript spk_placeholder = CScript() << OP_0;
     // Use BlockAssembler as oracle. BlockAssembler and MiniMiner should select the same
     // transactions, stopping once packages do not meet target_feerate.
-    const auto blocktemplate{miner.CreateNewBlock(spk_placeholder)};
+    const auto blocktemplate{miner.CreateNewBlock()};
     mini_miner.BuildMockTemplate(target_feerate);
     assert(!mini_miner.IsReadyToCalculate());
     auto mock_template_txids = mini_miner.GetMockTemplateTxids();

src/test/fuzz/package_eval.cpp

@@ -21,6 +21,7 @@
 #include <validation.h>
 #include <validationinterface.h>
 
+using node::BlockAssembler;
 using node::NodeContext;
 
 namespace {
@@ -42,8 +43,11 @@ void initialize_tx_pool()
     static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>();
     g_setup = testing_setup.get();
 
+    BlockAssembler::Options options;
+    options.coinbase_output_script = P2WSH_EMPTY;
+
     for (int i = 0; i < 2 * COINBASE_MATURITY; ++i) {
-        COutPoint prevout{MineBlock(g_setup->m_node, P2WSH_EMPTY)};
+        COutPoint prevout{MineBlock(g_setup->m_node, options)};
         if (i < COINBASE_MATURITY) {
             // Remember the txids to avoid expensive disk access later on
             g_outpoints_coinbase_init_mature.push_back(prevout);

src/test/fuzz/process_message.cpp

@@ -45,7 +45,7 @@ void initialize_process_message()
             {.extra_args = {"-txreconciliation"}});
     g_setup = testing_setup.get();
     for (int i = 0; i < 2 * COINBASE_MATURITY; i++) {
-        MineBlock(g_setup->m_node, CScript() << OP_TRUE);
+        MineBlock(g_setup->m_node, {});
     }
     g_setup->m_node.validation_signals->SyncWithValidationInterfaceQueue();
 }

src/test/fuzz/process_messages.cpp

@@ -35,7 +35,7 @@ void initialize_process_messages()
             {.extra_args = {"-txreconciliation"}});
     g_setup = testing_setup.get();
     for (int i = 0; i < 2 * COINBASE_MATURITY; i++) {
-        MineBlock(g_setup->m_node, CScript() << OP_TRUE);
+        MineBlock(g_setup->m_node, {});
     }
     g_setup->m_node.validation_signals->SyncWithValidationInterfaceQueue();
 }

src/test/fuzz/tx_pool.cpp

@@ -45,8 +45,11 @@ void initialize_tx_pool()
     static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>();
     g_setup = testing_setup.get();
 
+    BlockAssembler::Options options;
+    options.coinbase_output_script = P2WSH_OP_TRUE;
+
     for (int i = 0; i < 2 * COINBASE_MATURITY; ++i) {
-        COutPoint prevout{MineBlock(g_setup->m_node, P2WSH_OP_TRUE)};
+        COutPoint prevout{MineBlock(g_setup->m_node, options)};
         // Remember the txids to avoid expensive disk access later on
         auto& outpoints = i < COINBASE_MATURITY ?
                               g_outpoints_coinbase_init_mature :
@@ -98,7 +101,7 @@ void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, Cha
         options.nBlockMaxWeight = fuzzed_data_provider.ConsumeIntegralInRange(0U, MAX_BLOCK_WEIGHT);
         options.blockMinFeeRate = CFeeRate{ConsumeMoney(fuzzed_data_provider, /*max=*/COIN)};
         auto assembler = BlockAssembler{chainstate, &tx_pool, options};
-        auto block_template = assembler.CreateNewBlock(CScript{} << OP_TRUE);
+        auto block_template = assembler.CreateNewBlock();
         Assert(block_template->block.vtx.size() >= 1);
     }
     const auto info_all = tx_pool.infoAll();

src/test/fuzz/utxo_total_supply.cpp

@@ -17,6 +17,8 @@
 #include <util/chaintype.h>
 #include <validation.h>
 
+using node::BlockAssembler;
+
 FUZZ_TARGET(utxo_total_supply)
 {
     /** The testing setup that creates a chainman only (no chainstate) */
@@ -36,9 +38,11 @@ FUZZ_TARGET(utxo_total_supply)
         LOCK(chainman.GetMutex());
         return chainman.ActiveHeight();
     };
+    BlockAssembler::Options options;
+    options.coinbase_output_script = CScript() << OP_FALSE;
     const auto PrepareNextBlock = [&]() {
         // Use OP_FALSE to avoid BIP30 check from hitting early
-        auto block = PrepareBlock(node, CScript{} << OP_FALSE);
+        auto block = PrepareBlock(node, options);
         // Replace OP_FALSE with OP_TRUE
         {
             CMutableTransaction tx{*block->vtx.back()};