mining: fix -blockreservedweight shadows IPC option #33965

1. 
   Sjors commented at 1:38 pm on November 28, 2025: member

   The -blockreservedweight startup option should only affect RPC code, because IPC clients currently do not have a way to signal their intent to use the node default (the BlockCreateOptions struct defaults merely document a recommendation for client software).

   Before this PR however, if the user set -blockreservedweight then ApplyArgsManOptions would cause the block_reserved_weight option passed by IPC clients to be ignored. Users who don’t set this value were not affected.

   Fix this by having ApplyArgsManOptions not touch block_reserved_weight. Update RPC callers, in particular getblocktemplate, to explicitly set this option since it’s no longer automatic.

   Test coverage is added.

   mining_basic.py already ensured -blockreservedweight is enforced by mining RPC methods. This commit adds coverage for Mining interface IPC clients. It also verifies that -blockreservedweight has no effect on them.

2. DrahtBot added the label Mining on Nov 28, 2025
3. 
   DrahtBot commented at 1:38 pm on November 28, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33965.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept ACK	ryanofsky

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #33819 (mining: add getCoinbase() by Sjors)
   • #33795 (test: Ignore error message give from python because of PYTHON_GIL by kevkevinpal)
   • #32468 (rpc: generateblock to allow multiple outputs by polespinasa)
   • #32420 (miner: drop dummy extraNonce in coinbase scriptSig for templates requested via IPC by Sjors)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

4. mining: fix -blockreservedweight shadows IPC option

   The -blockreservedweight startup option should only affect RPC code,
   because IPC clients currently do not have a way to signal their intent
   to use the node default (the BlockCreateOptions struct defaults
   merely document a recommendation for client software).
   
   Before this commit however, if the user set -blockreservedweight
   then ApplyArgsManOptions would cause the block_reserved_weight
   option passed by IPC clients to be ignored. Users who don't set
   this value were not affected.
   
   Fix this by having ApplyArgsManOptions not touch block_reserved_weight.
   Update RPC callers, in particular getblocktemplate, to explicitly set
   this option since it's no longer automatic.
   
   Test coverage is added.
   
   mining_basic.py already ensured -blockreservedweight is enforced by
   mining RPC methods. This commit adds coverage for Mining interface IPC
   clients. It also verifies that -blockreservedweight has no effect on
   them.

   46d901c253
5. Sjors force-pushed on Nov 28, 2025
6. DrahtBot added the label CI failed on Nov 28, 2025
7. 
   DrahtBot commented at 1:49 pm on November 28, 2025: contributor

   🚧 At least one of the CI tasks failed. Task i686, no IPC: https://github.com/bitcoin/bitcoin/actions/runs/19765451883/job/56637015664 LLM reason (✨ experimental): Compile error: ISO C++ reorder-init-list warnings promoted to errors in src/rpc/mining.cpp, causing CI failure.

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

8. in src/rpc/mining.cpp:882 in 46d901c253

   877@@ -871,7 +878,9 @@ static RPCHelpMan getblocktemplate()
   878         time_start = GetTime();
   879 
   880         // Create new block
   881-        block_template = miner.createNewBlock();
   882+        block_template = miner.createNewBlock({
   883+            .block_reserved_weight = size_t(node.args->GetIntArg("-blockreservedweight", DEFAULT_BLOCK_RESERVED_WEIGHT))
   

   ---

   ---

   

   Sjors commented at 1:52 pm on November 28, 2025:

   The big refactor in #33966 simplifies this to node.mining_args.default_block_reserved_weight. But using GetIntArg directly here is easier to backport.
9. DrahtBot removed the label CI failed on Nov 28, 2025
10. 
    ryanofsky commented at 8:22 pm on December 1, 2025: contributor

    Concept ACK. Would suggest updating -blockreservedweight documentation to say it only affects RPC mining clients, not IPC clients.

    I think implementation of this could be improved a bit, depending on if you think it would be useful for IPC clients to “have a way to signal their intent to use the node default”?

    If yes, would suggest just making the field std::optional:

     0--- a/src/node/miner.cpp
     1+++ b/src/node/miner.cpp
     2@@ -78,11 +78,11 @@ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman)
     3 
     4 static BlockAssembler::Options ClampOptions(BlockAssembler::Options options)
     5 {
     6-    options.block_reserved_weight = std::clamp<size_t>(options.block_reserved_weight, MINIMUM_BLOCK_RESERVED_WEIGHT, MAX_BLOCK_WEIGHT);
     7+    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);
     8     options.coinbase_output_max_additional_sigops = std::clamp<size_t>(options.coinbase_output_max_additional_sigops, 0, MAX_BLOCK_SIGOPS_COST);
     9     // Limit weight to between block_reserved_weight and MAX_BLOCK_WEIGHT for sanity:
    10     // block_reserved_weight can safely exceed -blockmaxweight, but the rest of the block template will be empty.
    11-    options.nBlockMaxWeight = std::clamp<size_t>(options.nBlockMaxWeight, options.block_reserved_weight, MAX_BLOCK_WEIGHT);
    12+    options.nBlockMaxWeight = std::clamp<size_t>(options.nBlockMaxWeight, *options.block_reserved_weight, MAX_BLOCK_WEIGHT);
    13     return options;
    14 }
    15 
    16@@ -102,13 +102,15 @@ void ApplyArgsManOptions(const ArgsManager& args, BlockAssembler::Options& optio
    17         if (const auto parsed{ParseMoney(*blockmintxfee)}) options.blockMinFeeRate = CFeeRate{*parsed};
    18     }
    19     options.print_modified_fee = args.GetBoolArg("-printpriority", options.print_modified_fee);
    20-    options.block_reserved_weight = args.GetIntArg("-blockreservedweight", options.block_reserved_weight);
    21+    if (!options.block_reserved_weight) {
    22+        options.block_reserved_weight = args.GetIntArg("-blockreservedweight");
    23+    }
    24 }
    25 
    26 void BlockAssembler::resetBlock()
    27 {
    28     // Reserve space for fixed-size block header, txs count, and coinbase tx.
    29-    nBlockWeight = m_options.block_reserved_weight;
    30+    nBlockWeight = *m_options.block_reserved_weight;
    31     nBlockSigOpsCost = m_options.coinbase_output_max_additional_sigops;
    32 
    33     // These counters do not include coinbase tx
    34--- a/src/node/types.h
    35+++ b/src/node/types.h
    36@@ -42,7 +42,7 @@ struct BlockCreateOptions {
    37      * The default reserved weight for the fixed-size block header,
    38      * transaction count and coinbase transaction.
    39      */
    40-    size_t block_reserved_weight{DEFAULT_BLOCK_RESERVED_WEIGHT};
    41+    std::optional<size_t> block_reserved_weight{};
    42     /**
    43      * The maximum additional sigops which the pool will add in coinbase
    44      * transaction outputs.
    

    If no, would suggest having separate functions to read options which IPC clients can and can’t control:

      0--- a/src/node/interfaces.cpp
      1+++ b/src/node/interfaces.cpp
      2@@ -963,8 +963,12 @@ public:
      3         // Ensure m_tip_block is set so consumers of BlockTemplate can rely on that.
      4         if (!waitTipChanged(uint256::ZERO, MillisecondsDouble::max())) return {};
      5 
      6+        // Construct block assembler options from block create options and
      7+        // ArgsManager preferences. Intentionally do not call
      8+        // ReadBlockCreateArgs, so IPC create options will take precedence over
      9+        // ArgsManager values.
     10         BlockAssembler::Options assemble_options{options};
     11-        ApplyArgsManOptions(*Assert(m_node.args), assemble_options);
     12+        ReadBlockAssemblerArgs(*Assert(m_node.args), assemble_options);
     13         return std::make_unique<BlockTemplateImpl>(assemble_options, BlockAssembler{chainman().ActiveChainstate(), context()->mempool.get(), assemble_options}.CreateNewBlock(), m_node);
     14     }
     15 
     16--- a/src/node/miner.cpp
     17+++ b/src/node/miner.cpp
     18@@ -94,7 +94,7 @@ BlockAssembler::BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool
     19 {
     20 }
     21 
     22-void ApplyArgsManOptions(const ArgsManager& args, BlockAssembler::Options& options)
     23+void ReadBlockAssemblerArgs(const ArgsManager& args, BlockAssembler::Options& options)
     24 {
     25     // Block resource limits
     26     options.nBlockMaxWeight = args.GetIntArg("-blockmaxweight", options.nBlockMaxWeight);
     27@@ -102,6 +102,10 @@ void ApplyArgsManOptions(const ArgsManager& args, BlockAssembler::Options& optio
     28         if (const auto parsed{ParseMoney(*blockmintxfee)}) options.blockMinFeeRate = CFeeRate{*parsed};
     29     }
     30     options.print_modified_fee = args.GetBoolArg("-printpriority", options.print_modified_fee);
     31+}
     32+
     33+void ReadBlockCreateArgs(const ArgsManager& args, BlockCreateOptions& options)
     34+{
     35     options.block_reserved_weight = args.GetIntArg("-blockreservedweight", options.block_reserved_weight);
     36 }
     37 
     38--- a/src/node/miner.h
     39+++ b/src/node/miner.h
     40@@ -131,8 +131,11 @@ int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParam
     41 /** Update an old GenerateCoinbaseCommitment from CreateNewBlock after the block txs have changed */
     42 void RegenerateCommitments(CBlock& block, ChainstateManager& chainman);
     43 
     44+/** Apply -blockreservedweight options from ArgsManager to BlockCreateOptions. */
     45+void ReadBlockCreateArgs(const ArgsManager& args, BlockCreateOptions& options);
     46+
     47 /** Apply -blockmintxfee and -blockmaxweight options from ArgsManager to BlockAssembler options. */
     48-void ApplyArgsManOptions(const ArgsManager& gArgs, BlockAssembler::Options& options);
     49+void ReadBlockAssemblerArgs(const ArgsManager& args, BlockAssembler::Options& options);
     50 
     51 /* Compute the block's merkle root, insert or replace the coinbase transaction and the merkle root into the block */
     52 void AddMerkleRootAndCoinbase(CBlock& block, CTransactionRef coinbase, uint32_t version, uint32_t timestamp, uint32_t nonce);
     53--- a/src/rpc/mining.cpp
     54+++ b/src/rpc/mining.cpp
     55@@ -161,11 +161,13 @@ static bool GenerateBlock(ChainstateManager& chainman, CBlock&& block, uint64_t&
     56     return true;
     57 }
     58 
     59-static UniValue generateBlocks(ChainstateManager& chainman, Mining& miner, const CScript& coinbase_output_script, int nGenerate, uint64_t nMaxTries)
     60+static UniValue generateBlocks(ChainstateManager& chainman, ArgsManager& args, Mining& miner, const CScript& coinbase_output_script, int nGenerate, uint64_t nMaxTries)
     61 {
     62     UniValue blockHashes(UniValue::VARR);
     63+    node::BlockCreateOptions options{ .coinbase_output_script = coinbase_output_script };
     64+    ReadBlockCreateArgs(args, options);
     65     while (nGenerate > 0 && !chainman.m_interrupt) {
     66-        std::unique_ptr<BlockTemplate> block_template(miner.createNewBlock({ .coinbase_output_script = coinbase_output_script }));
     67+        std::unique_ptr<BlockTemplate> block_template(miner.createNewBlock(options));
     68         CHECK_NONFATAL(block_template);
     69 
     70         std::shared_ptr<const CBlock> block_out;
     71@@ -247,9 +249,10 @@ static RPCHelpMan generatetodescriptor()
     72 
     73     NodeContext& node = EnsureAnyNodeContext(request.context);
     74     Mining& miner = EnsureMining(node);
     75+    ArgsManager& args = EnsureArgsman(node);
     76     ChainstateManager& chainman = EnsureChainman(node);
     77 
     78-    return generateBlocks(chainman, miner, coinbase_output_script, num_blocks, max_tries);
     79+    return generateBlocks(chainman, args, miner, coinbase_output_script, num_blocks, max_tries);
     80 },
     81     };
     82 }
     83@@ -293,11 +296,12 @@ static RPCHelpMan generatetoaddress()
     84 
     85     NodeContext& node = EnsureAnyNodeContext(request.context);
     86     Mining& miner = EnsureMining(node);
     87+    ArgsManager& args = EnsureArgsman(node);
     88     ChainstateManager& chainman = EnsureChainman(node);
     89 
     90     CScript coinbase_output_script = GetScriptForDestination(destination);
     91 
     92-    return generateBlocks(chainman, miner, coinbase_output_script, num_blocks, max_tries);
     93+    return generateBlocks(chainman, args, miner, coinbase_output_script, num_blocks, max_tries);
     94 },
     95     };
     96 }
     97@@ -345,6 +349,7 @@ static RPCHelpMan generateblock()
     98 
     99     NodeContext& node = EnsureAnyNodeContext(request.context);
    100     Mining& miner = EnsureMining(node);
    101+    ArgsManager& args = EnsureArgsman(node);
    102     const CTxMemPool& mempool = EnsureMemPool(node);
    103 
    104     std::vector<CTransactionRef> txs;
    105@@ -374,9 +379,11 @@ static RPCHelpMan generateblock()
    106 
    107     ChainstateManager& chainman = EnsureChainman(node);
    108     {
    109+        node::BlockCreateOptions options{.use_mempool = false, .coinbase_output_script = coinbase_output_script};
    110+        ReadBlockCreateArgs(args, options);
    111         LOCK(chainman.GetMutex());
    112         {
    113-            std::unique_ptr<BlockTemplate> block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script})};
    114+            std::unique_ptr<BlockTemplate> block_template{miner.createNewBlock(options)};
    115             CHECK_NONFATAL(block_template);
    116 
    117             block = block_template->getBlock();
    118@@ -471,7 +478,8 @@ static RPCHelpMan getmininginfo()
    119     obj.pushKV("networkhashps",    getnetworkhashps().HandleRequest(request));
    120     obj.pushKV("pooledtx",         (uint64_t)mempool.size());
    121     BlockAssembler::Options assembler_options;
    122-    ApplyArgsManOptions(*node.args, assembler_options);
    123+    ReadBlockCreateArgs(*node.args, assembler_options);
    124+    ReadBlockAssemblerArgs(*node.args, assembler_options);
    125     obj.pushKV("blockmintxfee", ValueFromAmount(assembler_options.blockMinFeeRate.GetFeePerK()));
    126     obj.pushKV("chain", chainman.GetParams().GetChainTypeString());
    127 
    128@@ -871,7 +879,10 @@ static RPCHelpMan getblocktemplate()
    129         time_start = GetTime();
    130 
    131         // Create new block
    132-        block_template = miner.createNewBlock();
    133+        ArgsManager& args = EnsureArgsman(node);
    134+        node::BlockCreateOptions options;
    135+        ReadBlockCreateArgs(args, options);
    136+        block_template = miner.createNewBlock(options);
    137         CHECK_NONFATAL(block_template);
    138 
    139 
    140--- a/src/test/util/mining.cpp
    141+++ b/src/test/util/mining.cpp
    142@@ -138,6 +138,7 @@ std::shared_ptr<CBlock> PrepareBlock(const NodeContext& node, const CScript& coi
    143 {
    144     BlockAssembler::Options assembler_options;
    145     assembler_options.coinbase_output_script = coinbase_scriptPubKey;
    146-    ApplyArgsManOptions(*node.args, assembler_options);
    147+    ReadBlockCreateArgs(*node.args, assembler_options);
    148+    ReadBlockAssemblerArgs(*node.args, assembler_options);
    149     return PrepareBlock(node, assembler_options);
    150 }
    

    The current implementation 46d901c253107ffba14ddce6c3154b46bd471fc2 seems a little less ideal using repeated GetIntArg calls instead a function to read the options.

Contributors
Sjors DrahtBot ryanofsky

Labels
Mining