miner: bugfix: remove duplicate space reservation for coinbase tx

src/init.cpp

@@ -19,6 +19,7 @@
 #include <common/args.h>
 #include <common/system.h>
 #include <consensus/amount.h>
+#include <consensus/consensus.h>
 #include <deploymentstatus.h>
 #include <hash.h>
 #include <httprpc.h>
@@ -54,6 +55,7 @@
 #include <node/mempool_persist_args.h>
 #include <node/miner.h>
 #include <node/peerman_args.h>
+#include <node/types.h>
 #include <policy/feerate.h>
 #include <policy/fees.h>
 #include <policy/fees_args.h>
@@ -647,7 +649,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
     argsman.AddArg("-whitelistrelay", strprintf("Add 'relay' permission to whitelisted peers with default permissions. This will accept relayed transactions even when not relaying transactions (default: %d)", DEFAULT_WHITELISTRELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
 
 
-    argsman.AddArg("-blockmaxweight=<n>", strprintf("Set maximum BIP141 block weight (default: %d)", DEFAULT_BLOCK_MAX_WEIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION);
+    argsman.AddArg("-blockmaxweight=<n>", strprintf("Set maximum BIP141 block weight (default: %d).", MAX_BLOCK_WEIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION);
     argsman.AddArg("-blockmintxfee=<amt>", strprintf("Set lowest fee rate (in %s/kvB) for transactions to be included in block creation. (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_BLOCK_MIN_TX_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION);
     argsman.AddArg("-blockversion=<n>", "Override block version to test forking scenarios", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::BLOCK_CREATION);
 

src/node/miner.cpp

@@ -67,11 +67,11 @@ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman)
 
 static BlockAssembler::Options ClampOptions(BlockAssembler::Options options)
 {
-    Assert(options.coinbase_max_additional_weight <= DEFAULT_BLOCK_MAX_WEIGHT);
+    Assert(options.coinbase_max_additional_weight <= MAX_BLOCK_WEIGHT);
     Assert(options.coinbase_output_max_additional_sigops <= MAX_BLOCK_SIGOPS_COST);
-    // Limit weight to between coinbase_max_additional_weight and DEFAULT_BLOCK_MAX_WEIGHT for sanity:
+    // Limit weight to between coinbase_max_additional_weight and MAX_BLOCK_WEIGHT for sanity:
     // Coinbase (reserved) outputs can safely exceed -blockmaxweight, but the rest of the block template will be empty.
-    options.nBlockMaxWeight = std::clamp<size_t>(options.nBlockMaxWeight, options.coinbase_max_additional_weight, DEFAULT_BLOCK_MAX_WEIGHT);
+    options.nBlockMaxWeight = std::clamp<size_t>(options.nBlockMaxWeight, options.coinbase_max_additional_weight, MAX_BLOCK_WEIGHT);
     return options;
 }
 

src/node/miner.h

@@ -6,8 +6,8 @@
 #ifndef BITCOIN_NODE_MINER_H
 #define BITCOIN_NODE_MINER_H
 
+#include <consensus/consensus.h>
 #include <node/types.h>
-#include <policy/policy.h>
 #include <primitives/block.h>
 #include <txmempool.h>
 
@@ -160,7 +160,7 @@ private:
 public:
     struct Options : BlockCreateOptions {
         // Configuration parameters for the block size
-        size_t nBlockMaxWeight{DEFAULT_BLOCK_MAX_WEIGHT};
+        size_t nBlockMaxWeight{MAX_BLOCK_WEIGHT};
         CFeeRate blockMinFeeRate{DEFAULT_BLOCK_MIN_TX_FEE};
         // Whether to call TestBlockValidity() at the end of CreateNewBlock().
         bool test_block_validity{true};

src/node/types.h

@@ -37,7 +37,7 @@ struct BlockCreateOptions {
      * scriptSig, witness and outputs. This must include any additional
      * weight needed for larger CompactSize encoded lengths.
      */
-    size_t coinbase_max_additional_weight{4000};
+    size_t coinbase_max_additional_weight{8000};
     /**
      * The maximum additional sigops which the pool will add in coinbase
      * transaction outputs.

src/policy/policy.h

@@ -19,8 +19,6 @@ class CCoinsViewCache;
 class CFeeRate;
 class CScript;
 
-/** Default for -blockmaxweight, which controls the range of block weights the mining code will create **/
-static constexpr unsigned int DEFAULT_BLOCK_MAX_WEIGHT{MAX_BLOCK_WEIGHT - 4000};
 /** Default for -blockmintxfee, which sets the minimum feerate for a transaction in blocks created by mining code **/
 static constexpr unsigned int DEFAULT_BLOCK_MIN_TX_FEE{1000};
 /** The maximum weight for transactions we're willing to relay/mine */

src/test/fuzz/mini_miner.cpp

@@ -9,6 +9,7 @@
 
 #include <node/miner.h>
 #include <node/mini_miner.h>
+#include <node/types.h>
 #include <primitives/transaction.h>
 #include <random.h>
 #include <txmempool.h>
@@ -148,9 +149,10 @@ FUZZ_TARGET(mini_miner_selection, .init = initialize_miner)
             }
         }
 
-        // Stop if pool reaches DEFAULT_BLOCK_MAX_WEIGHT because BlockAssembler will stop when the
+        const auto block_adjusted_max_weight = MAX_BLOCK_WEIGHT - node::BlockCreateOptions{}.coinbase_max_additional_weight;
+        // Stop if pool reaches block_adjusted_max_weight because BlockAssembler will stop when the
         // block template reaches that, but the MiniMiner will keep going.
-        if (pool.GetTotalTxSize() + GetVirtualTransactionSize(*tx) >= DEFAULT_BLOCK_MAX_WEIGHT) break;
+        if (pool.GetTotalTxSize() + GetVirtualTransactionSize(*tx) >= block_adjusted_max_weight) break;
         TestMemPoolEntryHelper entry;
         const CAmount fee{ConsumeMoney(fuzzed_data_provider, /*max=*/MAX_MONEY/100000)};
         assert(MoneyRange(fee));
@@ -172,7 +174,7 @@ FUZZ_TARGET(mini_miner_selection, .init = initialize_miner)
 
     node::BlockAssembler::Options miner_options;
     miner_options.blockMinFeeRate = target_feerate;
-    miner_options.nBlockMaxWeight = DEFAULT_BLOCK_MAX_WEIGHT;
+    miner_options.nBlockMaxWeight = MAX_BLOCK_WEIGHT;
     miner_options.test_block_validity = false;
 
     node::BlockAssembler miner{g_setup->m_node.chainman->ActiveChainstate(), &pool, miner_options};

test/functional/mining_basic.py

@@ -22,6 +22,7 @@ from test_framework.messages import (
     CBlock,
     CBlockHeader,
     COIN,
+    DEFAULT_COINBASE_TX_WEIGHT,
     ser_uint256,
 )
 from test_framework.p2p import P2PDataStore
@@ -73,7 +74,7 @@ class MiningTest(BitcoinTestFramework):
         mining_info = self.nodes[0].getmininginfo()
         assert_equal(mining_info['blocks'], 200)
         assert_equal(mining_info['currentblocktx'], 0)
-        assert_equal(mining_info['currentblockweight'], 4000)
+        assert_equal(mining_info['currentblockweight'], DEFAULT_COINBASE_TX_WEIGHT)
 
         self.log.info('test blockversion')
         self.restart_node(0, extra_args=[f'-mocktime={t}', '-blockversion=1337'])

test/functional/test_framework/messages.py

@@ -33,6 +33,7 @@ from test_framework.util import assert_equal
 
 MAX_LOCATOR_SZ = 101
 MAX_BLOCK_WEIGHT = 4000000
+DEFAULT_COINBASE_TX_WEIGHT = 8000
 MAX_BLOOM_FILTER_SIZE = 36000
 MAX_BLOOM_HASH_FUNCS = 50