mining: store mining args in NodeContext

Instead of parsing arguments like -blockmaxweight each time a block
template is generated, do it once in ReadMiningArgs().

These arguments are stored in a new struct MiningArgs.

The BlockAssembler::Options struct is removed in favor of passing
both MiningArgs and BlockCreateOptions.

This disentangles node configuration from client (or test) options.

Author: Sjors

src/init.cpp

@@ -1055,7 +1055,8 @@ bool AppInitParameterInteraction(const ArgsManager& args)
         return InitError(Untranslated("peertimeout must be a positive integer."));
     }
 
-    auto mining_result{node::ReadMiningArgs(args)};
+    node::MiningArgs mining_args_dummy;
+    auto mining_result{node::ReadMiningArgs(args, mining_args_dummy)};
     if (!mining_result) {
         return InitError(util::ErrorString(mining_result));
     }
@@ -1293,6 +1294,7 @@ static ChainstateLoadResult InitAndLoadChainstate(
     if (!mempool_error.empty()) {
         return {ChainstateLoadStatus::FAILURE_FATAL, mempool_error};
     }
+    Assert(ReadMiningArgs(args, node.mining_args)); // no error can happen, already checked in AppInitParameterInteraction
     LogInfo("* Using %.1f MiB for in-memory UTXO set (plus up to %.1f MiB of unused mempool space)",
             cache_sizes.coins * (1.0 / 1024 / 1024),
             mempool_opts.max_size_bytes * (1.0 / 1024 / 1024));

src/interfaces/mining.h

@@ -152,7 +152,7 @@ class Mining
      * @retval BlockTemplate a block template.
      * @retval std::nullptr if the node is shut down.
      */
-    virtual std::unique_ptr<BlockTemplate> createNewBlock(const node::BlockCreateOptions& options = {}) = 0;
+    virtual std::unique_ptr<BlockTemplate> createNewBlock(node::BlockCreateOptions options = {}) = 0;
 
     /**
      * Checks if a given block is valid.

src/node/context.h

@@ -5,6 +5,8 @@
 #ifndef BITCOIN_NODE_CONTEXT_H
 #define BITCOIN_NODE_CONTEXT_H
 
+#include <node/mining_args.h>
+
 #include <atomic>
 #include <cstdlib>
 #include <functional>
@@ -79,6 +81,7 @@ struct NodeContext {
     //! Reference to chain client that should used to load or create wallets
     //! opened by the gui.
     std::unique_ptr<interfaces::Mining> mining;
+    MiningArgs mining_args;
     interfaces::WalletLoader* wallet_loader{nullptr};
     std::unique_ptr<CScheduler> scheduler;
     std::function<void()> rpc_interruption_point = [] {};

src/node/interfaces.cpp

@@ -84,6 +84,8 @@ using interfaces::Node;
 using interfaces::WalletLoader;
 using kernel::ChainstateRole;
 using node::BlockAssembler;
+using node::BlockCreateOptions;
+using node::MiningArgs;
 using node::BlockWaitOptions;
 using node::CoinbaseTx;
 using util::Join;
@@ -861,9 +863,9 @@ class ChainImpl : public Chain
 class BlockTemplateImpl : public BlockTemplate
 {
 public:
-    explicit BlockTemplateImpl(BlockAssembler::Options assemble_options,
+    explicit BlockTemplateImpl(BlockCreateOptions create_options,
                                std::unique_ptr<CBlockTemplate> block_template,
-                               NodeContext& node) : m_assemble_options(std::move(assemble_options)),
+                               NodeContext& node) : m_create_options(std::move(create_options)),
                                                     m_block_template(std::move(block_template)),
                                                     m_node(node)
     {
@@ -923,8 +925,15 @@ class BlockTemplateImpl : public BlockTemplate
 
     std::unique_ptr<BlockTemplate> waitNext(BlockWaitOptions options) override
     {
-        auto new_template = WaitAndCreateNewBlock(chainman(), notifications(), m_node.mempool.get(), m_block_template, options, m_assemble_options, m_interrupt_wait);
-        if (new_template) return std::make_unique<BlockTemplateImpl>(m_assemble_options, std::move(new_template), m_node);
+        auto new_template = WaitAndCreateNewBlock(chainman(),
+                                                  notifications(),
+                                                  m_node.mempool.get(),
+                                                  m_block_template,
+                                                  options,
+                                                  m_mining_args,
+                                                  m_create_options,
+                                                  m_interrupt_wait);
+        if (new_template) return std::make_unique<BlockTemplateImpl>(m_create_options, std::move(new_template), m_node);
         return nullptr;
     }
 
@@ -933,7 +942,8 @@ class BlockTemplateImpl : public BlockTemplate
         InterruptWait(notifications(), m_interrupt_wait);
     }
 
-    const BlockAssembler::Options m_assemble_options;
+    const MiningArgs m_mining_args;
+    const BlockCreateOptions m_create_options;
 
     const std::unique_ptr<CBlockTemplate> m_block_template;
 
@@ -968,7 +978,7 @@ class MinerImpl : public Mining
         return WaitTipChanged(chainman(), notifications(), current_tip, timeout);
     }
 
-    std::unique_ptr<BlockTemplate> createNewBlock(const BlockCreateOptions& options) override
+    std::unique_ptr<BlockTemplate> createNewBlock(BlockCreateOptions options) override
     {
         // Reject too-small values instead of clamping so callers don't silently
         // end up mining with different options than requested. This matches the
@@ -982,10 +992,15 @@ class MinerImpl : public Mining
 
         // Ensure m_tip_block is set so consumers of BlockTemplate can rely on that.
         if (!waitTipChanged(uint256::ZERO, MillisecondsDouble::max())) return {};
-
-        BlockAssembler::Options assemble_options{options};
-        ApplyArgsManOptions(*Assert(m_node.args), assemble_options);
-        return std::make_unique<BlockTemplateImpl>(assemble_options, BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), assemble_options}.CreateNewBlock(), m_node);
+        ApplyMiningDefaults(m_node.mining_args, options);
+        return std::make_unique<BlockTemplateImpl>(options,
+                                                   BlockAssembler{
+                                                       chainman().ActiveChainstate(),
+                                                       context()->mempool.get(),
+                                                       context()->mining_args,
+                                                       options,
+                                                    }.CreateNewBlock(),
+                                                   m_node);
     }
 
     bool checkBlock(const CBlock& block, const node::BlockCheckOptions& options, std::string& reason, std::string& debug) override

src/node/miner.cpp

@@ -76,9 +76,11 @@ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman)
     block.hashMerkleRoot = BlockMerkleRoot(block);
 }
 
-static BlockAssembler::Options ClampOptions(BlockAssembler::Options options)
+static BlockCreateOptions ClampOptions(BlockCreateOptions options)
 {
-    // Apply DEFAULT_BLOCK_RESERVED_WEIGHT and DEFAULT_BLOCK_MAX_WEIGHT when the caller left it unset.
+    // Typically block_reserved_weight and block_max_weight are set by
+    // ApplyMiningDefaults before the constructor calls this; value_or(DEFAULT_...)
+    // only affects (test) call sites that don't go through the Mining interface.
     options.block_reserved_weight = std::clamp<size_t>(options.block_reserved_weight.value_or(DEFAULT_BLOCK_RESERVED_WEIGHT), MINIMUM_BLOCK_RESERVED_WEIGHT, MAX_BLOCK_WEIGHT);
     options.coinbase_output_max_additional_sigops = std::clamp<size_t>(options.coinbase_output_max_additional_sigops, 0, MAX_BLOCK_SIGOPS_COST);
     // Limit weight to between block_reserved_weight and MAX_BLOCK_WEIGHT for sanity:
@@ -87,26 +89,26 @@ static BlockAssembler::Options ClampOptions(BlockAssembler::Options options)
     return options;
 }
 
-BlockAssembler::BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool, const Options& options)
+BlockAssembler::BlockAssembler(Chainstate& chainstate,
+                               const CTxMemPool* mempool,
+                               MiningArgs mining_args,
+                               BlockCreateOptions options)
     : chainparams{chainstate.m_chainman.GetParams()},
       m_mempool{options.use_mempool ? mempool : nullptr},
       m_chainstate{chainstate},
+      m_mining_args{mining_args},
       m_options{ClampOptions(options)}
 {
 }
 
-void ApplyArgsManOptions(const ArgsManager& args, BlockAssembler::Options& options)
+void ApplyMiningDefaults(const MiningArgs& args, BlockCreateOptions& options)
 {
     // Block resource limits
     if (!options.block_max_weight) {
-        options.block_max_weight = args.GetIntArg("-blockmaxweight");
+        options.block_max_weight = args.default_block_max_weight;
     }
-    if (const auto blockmintxfee{args.GetArg("-blockmintxfee")}) {
-        if (const auto parsed{ParseMoney(*blockmintxfee)}) options.blockMinFeeRate = CFeeRate{*parsed};
-    }
-    options.print_modified_fee = args.GetBoolArg("-printpriority", options.print_modified_fee);
     if (!options.block_reserved_weight) {
-        options.block_reserved_weight = args.GetIntArg("-blockreservedweight");
+        options.block_reserved_weight = args.default_block_reserved_weight;
     }
 }
 
@@ -271,7 +273,7 @@ void BlockAssembler::AddToBlock(const CTxMemPoolEntry& entry)
     nBlockSigOpsCost += entry.GetSigOpCost();
     nFees += entry.GetFee();
 
-    if (m_options.print_modified_fee) {
+    if (m_mining_args.print_modified_fee) {
         LogInfo("fee rate %s txid %s\n",
                   CFeeRate(entry.GetModifiedFee(), entry.GetTxSize()).ToString(),
                   entry.GetTx().GetHash().ToString());
@@ -297,7 +299,7 @@ void BlockAssembler::addChunks()
 
     while (selected_transactions.size() > 0) {
         // Check to see if min fee rate is still respected.
-        if (chunk_feerate_vsize << m_options.blockMinFeeRate.GetFeePerVSize()) {
+        if (chunk_feerate_vsize << m_mining_args.blockMinFeeRate.GetFeePerVSize()) {
             // Everything else we might consider has a lower feerate
             return;
         }
@@ -365,7 +367,8 @@ std::unique_ptr<CBlockTemplate> WaitAndCreateNewBlock(ChainstateManager& chainma
                                                       CTxMemPool* mempool,
                                                       const std::unique_ptr<CBlockTemplate>& block_template,
                                                       const BlockWaitOptions& options,
-                                                      const BlockAssembler::Options& assemble_options,
+                                                      const MiningArgs& mining_args,
+                                                      const BlockCreateOptions& assemble_options,
                                                       bool& interrupt_wait)
 {
     // Delay calculating the current template fees, just in case a new block
@@ -426,8 +429,9 @@ std::unique_ptr<CBlockTemplate> WaitAndCreateNewBlock(ChainstateManager& chainma
             auto new_tmpl{BlockAssembler{
                 chainman.ActiveChainstate(),
                 mempool,
-                assemble_options}
-                              .CreateNewBlock()};
+                mining_args,
+                assemble_options
+                }.CreateNewBlock()};
 
             // If the tip changed, return the new template regardless of its fees.
             if (tip_changed) return new_tmpl;

src/node/miner.h

@@ -9,6 +9,7 @@
 #include <interfaces/types.h>
 #include <node/types.h>
 #include <node/mining_types.h>
+#include <node/mining_args.h>
 #include <primitives/block.h>
 #include <txmempool.h>
 #include <util/feefrac.h>
@@ -23,7 +24,6 @@
 #include <boost/multi_index/tag.hpp>
 #include <boost/multi_index_container.hpp>
 
-class ArgsManager;
 class CBlockIndex;
 class CChainParams;
 class CScript;
@@ -35,10 +35,9 @@ namespace Consensus { struct Params; };
 using interfaces::BlockRef;
 
 namespace node {
+struct NodeContext;
 class KernelNotifications;
 
-static const bool DEFAULT_PRINT_MODIFIED_FEE = false;
-
 struct CBlockTemplate
 {
     CBlock block;
@@ -79,12 +78,10 @@ class BlockAssembler
     Chainstate& m_chainstate;
 
 public:
-    struct Options : BlockCreateOptions {
-        CFeeRate blockMinFeeRate{DEFAULT_BLOCK_MIN_TX_FEE};
-        bool print_modified_fee{DEFAULT_PRINT_MODIFIED_FEE};
-    };
-
-    explicit BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool, const Options& options);
+    explicit BlockAssembler(Chainstate& chainstate,
+                            const CTxMemPool* mempool,
+                            MiningArgs mining_args,
+                            BlockCreateOptions create_options);
 
     /** Construct a new block template */
     std::unique_ptr<CBlockTemplate> CreateNewBlock();
@@ -95,7 +92,8 @@ class BlockAssembler
     inline static std::optional<int64_t> m_last_block_weight{};
 
 private:
-    const Options m_options;
+    const MiningArgs m_mining_args;
+    const BlockCreateOptions m_options;
 
     // utility functions
     /** Clear the block's state and prepare for assembling a new block */
@@ -131,8 +129,8 @@ int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParam
 /** Update an old GenerateCoinbaseCommitment from CreateNewBlock after the block txs have changed */
 void RegenerateCommitments(CBlock& block, ChainstateManager& chainman);
 
-/** Apply -blockmintxfee and -blockmaxweight options from ArgsManager to BlockAssembler options. */
-void ApplyArgsManOptions(const ArgsManager& gArgs, BlockAssembler::Options& options);
+/** Apply -blockmaxweight and -blockreservedweight arguments from MiningArgs to BlockCreateOptions options. */
+void ApplyMiningDefaults(const MiningArgs& args, BlockCreateOptions& options);
 
 /* Compute the block's merkle root, insert or replace the coinbase transaction and the merkle root into the block */
 void AddMerkleRootAndCoinbase(CBlock& block, CTransactionRef coinbase, uint32_t version, uint32_t timestamp, uint32_t nonce);
@@ -149,7 +147,8 @@ std::unique_ptr<CBlockTemplate> WaitAndCreateNewBlock(ChainstateManager& chainma
                                                       CTxMemPool* mempool,
                                                       const std::unique_ptr<CBlockTemplate>& block_template,
                                                       const BlockWaitOptions& options,
-                                                      const BlockAssembler::Options& assemble_options,
+                                                      const MiningArgs& mining_args,
+                                                      const BlockCreateOptions& assemble_options,
                                                       bool& interrupt_wait);
 
 /* Locks cs_main and returns the block hash and block height of the active chain if it exists; otherwise, returns nullopt.*/

src/node/mining_args.cpp

@@ -16,26 +16,30 @@ using common::AmountErrMsg;
 
 namespace node {
 
-util::Result<void> ReadMiningArgs(const ArgsManager& args)
+util::Result<void> ReadMiningArgs(const ArgsManager& args, MiningArgs& mining_args)
 {
     if (const auto arg{args.GetArg("-blockmintxfee")}) {
-        if (!ParseMoney(*arg)) {
-            return util::Error{AmountErrMsg("blockmintxfee", *arg)};
-        }
+        std::optional<CAmount> block_min_tx_fee{ParseMoney(*arg)};
+        if (!block_min_tx_fee) return util::Error{AmountErrMsg("blockmintxfee", *arg)};
+        mining_args.blockMinFeeRate = CFeeRate{*block_min_tx_fee};
     }
 
-    const auto max_block_weight = args.GetIntArg("-blockmaxweight", DEFAULT_BLOCK_MAX_WEIGHT);
+    const size_t max_block_weight = args.GetIntArg("-blockmaxweight", DEFAULT_BLOCK_MAX_WEIGHT);
     if (max_block_weight > MAX_BLOCK_WEIGHT) {
         return util::Error{strprintf(_("Specified -blockmaxweight (%d) exceeds consensus maximum block weight (%d)"), max_block_weight, MAX_BLOCK_WEIGHT)};
     }
+    mining_args.default_block_max_weight = max_block_weight;
 
-    const auto block_reserved_weight = args.GetIntArg("-blockreservedweight", DEFAULT_BLOCK_RESERVED_WEIGHT);
+    const size_t block_reserved_weight = args.GetIntArg("-blockreservedweight", DEFAULT_BLOCK_RESERVED_WEIGHT);
     if (block_reserved_weight > MAX_BLOCK_WEIGHT) {
         return util::Error{strprintf(_("Specified -blockreservedweight (%d) exceeds consensus maximum block weight (%d)"), block_reserved_weight, MAX_BLOCK_WEIGHT)};
     }
     if (block_reserved_weight < MINIMUM_BLOCK_RESERVED_WEIGHT) {
         return util::Error{strprintf(_("Specified -blockreservedweight (%d) is lower than minimum safety value of (%d)"), block_reserved_weight, MINIMUM_BLOCK_RESERVED_WEIGHT)};
     }
+    mining_args.default_block_reserved_weight = block_reserved_weight;
+
+    mining_args.print_modified_fee = args.GetBoolArg("-printpriority", mining_args.print_modified_fee);
 
     return {};
 }

src/node/mining_args.h

@@ -5,13 +5,41 @@
 #ifndef BITCOIN_NODE_MINING_ARGS_H
 #define BITCOIN_NODE_MINING_ARGS_H
 
+#include <policy/feerate.h>
+#include <policy/policy.h>
 #include <util/result.h>
 
 class ArgsManager;
 
 namespace node {
 
-[[nodiscard]] util::Result<void> ReadMiningArgs(const ArgsManager& args);
+static const bool DEFAULT_PRINT_MODIFIED_FEE = false;
+
+/**
+ * Block template creation defaults and limits configured for the node.
+ */
+struct MiningArgs {
+    CFeeRate blockMinFeeRate{DEFAULT_BLOCK_MIN_TX_FEE};
+    bool print_modified_fee{DEFAULT_PRINT_MODIFIED_FEE};
+    /**
+     * The default maximum block weight.
+     */
+    size_t default_block_max_weight{DEFAULT_BLOCK_MAX_WEIGHT};
+    /**
+     * The default reserved weight for the fixed-size block header,
+     * transaction count and coinbase transaction.
+     */
+    size_t default_block_reserved_weight{DEFAULT_BLOCK_RESERVED_WEIGHT};
+};
+
+/**
+ * Overlay the options set in \p args on top of corresponding members in
+ * \p mining_args. Returns an error if one was encountered.
+ *
+ * @param[in]  args The ArgsManager in which to check set options.
+ * @param[in,out] mining_args The MiningArgs to modify according to \p args.
+ */
+[[nodiscard]] util::Result<void> ReadMiningArgs(const ArgsManager& args, MiningArgs& mining_args);
 
 } // namespace node
 

src/node/mining_types.h

@@ -25,6 +25,10 @@
 
 namespace node {
 
+/**
+ * Block template creation options. These override node defaults, but can't
+ * exceed node limits (e.g. block_reserved_weight can't exceed max block weight).
+ */
 struct BlockCreateOptions {
     /**
      * Set false to omit mempool transactions

src/rpc/mining.cpp

@@ -462,17 +462,15 @@ static RPCHelpMan getmininginfo()
     CBlockIndex& tip{*CHECK_NONFATAL(active_chain.Tip())};
 
     UniValue obj(UniValue::VOBJ);
-    obj.pushKV("blocks",           active_chain.Height());
+    obj.pushKV("blocks", active_chain.Height());
     if (BlockAssembler::m_last_block_weight) obj.pushKV("currentblockweight", *BlockAssembler::m_last_block_weight);
     if (BlockAssembler::m_last_block_num_txs) obj.pushKV("currentblocktx", *BlockAssembler::m_last_block_num_txs);
     obj.pushKV("bits", strprintf("%08x", tip.nBits));
     obj.pushKV("difficulty", GetDifficulty(tip));
     obj.pushKV("target", GetTarget(tip, chainman.GetConsensus().powLimit).GetHex());
     obj.pushKV("networkhashps",    getnetworkhashps().HandleRequest(request));
     obj.pushKV("pooledtx", mempool.size());
-    BlockAssembler::Options assembler_options;
-    ApplyArgsManOptions(*node.args, assembler_options);
-    obj.pushKV("blockmintxfee", ValueFromAmount(assembler_options.blockMinFeeRate.GetFeePerK()));
+    obj.pushKV("blockmintxfee", ValueFromAmount(node.mining_args.blockMinFeeRate.GetFeePerK()));
     obj.pushKV("chain", chainman.GetParams().GetChainTypeString());
 
     UniValue next(UniValue::VOBJ);