From 703373df8c5977ca572e26422eb1752c694361a8 Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Sat, 30 Nov 2024 11:45:24 -0500 Subject: [PATCH] miner: bugfix: remove duplicate space reservation for coinbase tx --- src/init.cpp | 4 +++- src/node/miner.cpp | 6 +++--- src/node/miner.h | 4 ++-- src/node/types.h | 2 +- src/policy/policy.h | 2 -- src/test/fuzz/mini_miner.cpp | 8 +++++--- test/functional/mining_basic.py | 3 ++- test/functional/test_framework/messages.py | 1 + 8 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 19efc69aaa2..89fd586bf6a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -54,6 +55,7 @@ #include #include #include +#include #include #include #include @@ -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=", strprintf("Set maximum BIP141 block weight (default: %d)", DEFAULT_BLOCK_MAX_WEIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION); + argsman.AddArg("-blockmaxweight=", strprintf("Set maximum BIP141 block weight (default: %d).", MAX_BLOCK_WEIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION); argsman.AddArg("-blockmintxfee=", 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=", "Override block version to test forking scenarios", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::BLOCK_CREATION); diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 790ee1c1466..1cc53468b60 100644 --- a/src/node/miner.cpp +++ b/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(options.nBlockMaxWeight, options.coinbase_max_additional_weight, DEFAULT_BLOCK_MAX_WEIGHT); + options.nBlockMaxWeight = std::clamp(options.nBlockMaxWeight, options.coinbase_max_additional_weight, MAX_BLOCK_WEIGHT); return options; } diff --git a/src/node/miner.h b/src/node/miner.h index 25ce110b348..c67419c4956 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -6,8 +6,8 @@ #ifndef BITCOIN_NODE_MINER_H #define BITCOIN_NODE_MINER_H +#include #include -#include #include #include @@ -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}; diff --git a/src/node/types.h b/src/node/types.h index 1302f1b127f..3dbb94147d1 100644 --- a/src/node/types.h +++ b/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. diff --git a/src/policy/policy.h b/src/policy/policy.h index 4412f2db87a..67697adc045 100644 --- a/src/policy/policy.h +++ b/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 */ diff --git a/src/test/fuzz/mini_miner.cpp b/src/test/fuzz/mini_miner.cpp index d06594d5f8d..b2b5291520c 100644 --- a/src/test/fuzz/mini_miner.cpp +++ b/src/test/fuzz/mini_miner.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -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}; diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index d367ec122dd..4ac3dd91c7a 100755 --- a/test/functional/mining_basic.py +++ b/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']) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 1f566a1348a..23dba1fe398 100755 --- a/test/functional/test_framework/messages.py +++ b/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