From 3eaa0a3b663782bb1bd874ea881b21649f1db767 Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Mon, 3 Feb 2025 12:48:34 -0500 Subject: [PATCH] miner: init: add `-blockreservedweight` startup option - Prevent setting the value of `-blockreservedweight` below a safety value of 2000. --- src/init.cpp | 11 +++++++++++ src/node/miner.cpp | 2 ++ src/policy/policy.h | 6 ++++++ test/functional/mining_basic.py | 23 ++++++++++++++++++++++ test/functional/test_framework/messages.py | 1 + 5 files changed, 43 insertions(+) diff --git a/src/init.cpp b/src/init.cpp index c293ef23acc..03c525d5b49 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -649,6 +649,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) argsman.AddArg("-blockmaxweight=", strprintf("Set maximum BIP141 block weight (default: %d)", DEFAULT_BLOCK_MAX_WEIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION); + argsman.AddArg("-blockreservedweight=", strprintf("Reserve space for the fixed-size block header plus the largest coinbase transaction the mining software may add to the block. (default: %d).", DEFAULT_BLOCK_RESERVED_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); @@ -1023,6 +1024,16 @@ bool AppInitParameterInteraction(const ArgsManager& args) } } + if (args.IsArgSet("-blockreservedweight")) { + const auto block_reserved_weight = args.GetIntArg("-blockreservedweight", DEFAULT_BLOCK_RESERVED_WEIGHT); + if (block_reserved_weight > MAX_BLOCK_WEIGHT) { + return InitError(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 InitError(strprintf(_("Specified -blockreservedweight (%d) is lower than minimum safety value of (%d)"), block_reserved_weight, MINIMUM_BLOCK_RESERVED_WEIGHT)); + } + } + nBytesPerSigOp = args.GetIntArg("-bytespersigop", nBytesPerSigOp); if (!g_wallet_init_interface.ParameterInteraction()) return false; diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 4f4a273bc9c..b595d91ed97 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -68,6 +68,7 @@ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman) static BlockAssembler::Options ClampOptions(BlockAssembler::Options options) { Assert(options.block_reserved_weight <= MAX_BLOCK_WEIGHT); + Assert(options.block_reserved_weight >= MINIMUM_BLOCK_RESERVED_WEIGHT); Assert(options.coinbase_output_max_additional_sigops <= MAX_BLOCK_SIGOPS_COST); // Limit weight to between block_reserved_weight and MAX_BLOCK_WEIGHT for sanity: // block_reserved_weight can safely exceed -blockmaxweight, but the rest of the block template will be empty. @@ -91,6 +92,7 @@ void ApplyArgsManOptions(const ArgsManager& args, BlockAssembler::Options& optio if (const auto parsed{ParseMoney(*blockmintxfee)}) options.blockMinFeeRate = CFeeRate{*parsed}; } options.print_modified_fee = args.GetBoolArg("-printpriority", options.print_modified_fee); + options.block_reserved_weight = args.GetIntArg("-blockreservedweight", options.block_reserved_weight); } void BlockAssembler::resetBlock() diff --git a/src/policy/policy.h b/src/policy/policy.h index bb2c5240851..2151ec13dd0 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -21,7 +21,13 @@ 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}; +/** Default for -blockreservedweight **/ static constexpr unsigned int DEFAULT_BLOCK_RESERVED_WEIGHT{8000}; +/** This accounts for the block header, var_int encoding of the transaction count and a minimally viable + * coinbase transaction. It adds an additional safety margin, because even with a thorough understanding + * of block serialization, it's easy to make a costly mistake when trying to squeeze every last byte. + * Setting a lower value is prevented at startup. */ +static constexpr unsigned int MINIMUM_BLOCK_RESERVED_WEIGHT{2000}; /** 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/test/functional/mining_basic.py b/test/functional/mining_basic.py index 1c3a46e5d19..29fb8241e33 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -28,6 +28,7 @@ from test_framework.messages import ( COIN, DEFAULT_BLOCK_RESERVED_WEIGHT, MAX_BLOCK_WEIGHT, + MINIMUM_BLOCK_RESERVED_WEIGHT, ser_uint256, WITNESS_SCALE_FACTOR ) @@ -276,6 +277,28 @@ class MiningTest(BitcoinTestFramework): expected_weight=MAX_BLOCK_WEIGHT - DEFAULT_BLOCK_RESERVED_WEIGHT, ) + self.log.info("Test -blockreservedweight startup option.") + # Lowering the -blockreservedweight by 4000 will allow for two more transactions. + self.restart_node(0, extra_args=[f"-datacarriersize={LARGE_VSIZE}", "-blockreservedweight=4000"]) + self.verify_block_template( + expected_tx_count=12, + expected_weight=MAX_BLOCK_WEIGHT - 4000, + ) + + self.log.info("Test that node will fail to start when user provide invalid -blockreservedweight") + self.stop_node(0) + self.nodes[0].assert_start_raises_init_error( + extra_args=[f"-blockreservedweight={MAX_BLOCK_WEIGHT + 1}"], + expected_msg=f"Error: Specified -blockreservedweight ({MAX_BLOCK_WEIGHT + 1}) exceeds consensus maximum block weight ({MAX_BLOCK_WEIGHT})", + ) + + self.log.info(f"Test that node will fail to start when user provide -blockreservedweight below {MINIMUM_BLOCK_RESERVED_WEIGHT}") + self.stop_node(0) + self.nodes[0].assert_start_raises_init_error( + extra_args=[f"-blockreservedweight={MINIMUM_BLOCK_RESERVED_WEIGHT - 1}"], + expected_msg=f"Error: Specified -blockreservedweight ({MINIMUM_BLOCK_RESERVED_WEIGHT - 1}) is lower than minimum safety value of ({MINIMUM_BLOCK_RESERVED_WEIGHT})", + ) + self.log.info("Test that node will fail to start when user provide invalid -blockmaxweight") self.stop_node(0) self.nodes[0].assert_start_raises_init_error( diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index eb905bf16ca..9ebb683a9db 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -34,6 +34,7 @@ from test_framework.util import assert_equal MAX_LOCATOR_SZ = 101 MAX_BLOCK_WEIGHT = 4000000 DEFAULT_BLOCK_RESERVED_WEIGHT = 8000 +MINIMUM_BLOCK_RESERVED_WEIGHT = 2000 MAX_BLOOM_FILTER_SIZE = 36000 MAX_BLOOM_HASH_FUNCS = 50