diff --git a/doc/release-notes-31384.md b/doc/release-notes-31384.md new file mode 100644 index 0000000000..0cfb44956f --- /dev/null +++ b/doc/release-notes-31384.md @@ -0,0 +1,17 @@ +- Node and Mining + +--- + +- PR #31384 fixed an issue where the coinbase transaction weight was being reserved in two different places. + The fix ensures that the reservation happens in a single place. + +- This specifies how many weight units to reserve for the coinbase transaction. + +- Upgrade note: the default of `-maxcoinbaseweight` ensures backward compatibility for users who did + not override `-blockmaxweight`. + +- If you previously configured `-blockmaxweight=4000000` then you can + safely set `-maxcoinbaseweight=4000` to maintain existing behavior. + +- Bitcoin Core will now **fail to start** if the `-blockmaxweight` or `-maxcoinbaseweight` init parameter exceeds + the consensus limit of `4,000,000` weight units. diff --git a/src/init.cpp b/src/init.cpp index b5adcf6476..b2d351f251 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,8 @@ 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("-maxcoinbaseweight=", strprintf("Set the block maximum reserved coinbase transaction weight (default: %d).", node::BlockCreateOptions{}.coinbase_max_additional_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); @@ -1015,6 +1018,20 @@ bool AppInitParameterInteraction(const ArgsManager& args) } } + if (args.IsArgSet("-blockmaxweight")) { + const auto max_block_weight = args.GetIntArg("-blockmaxweight", MAX_BLOCK_WEIGHT); + if (max_block_weight > MAX_BLOCK_WEIGHT) { + return InitError(strprintf(_("Specified -blockmaxweight (%d) exceeds consensus maximum block weight (%d)"), max_block_weight, MAX_BLOCK_WEIGHT)); + } + } + + if (args.IsArgSet("-maxcoinbaseweight")) { + const auto max_coinbase_weight = args.GetIntArg("-maxcoinbaseweight", node::BlockCreateOptions{}.coinbase_max_additional_weight); + if (max_coinbase_weight > MAX_BLOCK_WEIGHT) { + return InitError(strprintf(_("Specified -maxcoinbaseweight (%d) exceeds consensus maximum block weight (%d)"), max_coinbase_weight, MAX_BLOCK_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 5d7304b597..ab17374feb 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; } @@ -90,18 +90,17 @@ void ApplyArgsManOptions(const ArgsManager& args, BlockAssembler::Options& optio 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); + options.coinbase_max_additional_weight = args.GetIntArg("-maxcoinbaseweight", options.coinbase_max_additional_weight); } void BlockAssembler::resetBlock() { inBlock.clear(); - - // Reserve space for coinbase tx - nBlockWeight = m_options.coinbase_max_additional_weight; - nBlockSigOpsCost = m_options.coinbase_output_max_additional_sigops; - // These counters do not include coinbase tx + nBlockWeight = 0; + nBlockSigOpsCost = 0; nBlockTx = 0; nFees = 0; } @@ -158,7 +157,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock() pblocktemplate->vchCoinbaseCommitment = m_chainstate.m_chainman.GenerateCoinbaseCommitment(*pblock, pindexPrev); pblocktemplate->vTxFees[0] = -nFees; - LogPrintf("CreateNewBlock(): block weight: %u txs: %u fees: %ld sigops %d\n", GetBlockWeight(*pblock), nBlockTx, nFees, nBlockSigOpsCost); + LogPrintf("CreateNewBlock(): block weight: %u txs: %u fees: %ld sigops %d. (excluding coinbase tx)\n", nBlockWeight, nBlockTx, nFees, nBlockSigOpsCost); // Fill in header pblock->hashPrevBlock = pindexPrev->GetBlockHash(); @@ -197,10 +196,12 @@ void BlockAssembler::onlyUnconfirmed(CTxMemPool::setEntries& testSet) bool BlockAssembler::TestPackage(uint64_t packageSize, int64_t packageSigOpsCost) const { // TODO: switch to weight-based accounting for packages instead of vsize-based accounting. - if (nBlockWeight + WITNESS_SCALE_FACTOR * packageSize >= m_options.nBlockMaxWeight) { + auto coinbase_adjusted_block_weight = nBlockWeight + m_options.coinbase_max_additional_weight; // Account for coinbase tx weight. + if (coinbase_adjusted_block_weight + WITNESS_SCALE_FACTOR * packageSize >= m_options.nBlockMaxWeight) { return false; } - if (nBlockSigOpsCost + packageSigOpsCost >= MAX_BLOCK_SIGOPS_COST) { + auto coinbase_adjusted_block_sigopcost = nBlockSigOpsCost + m_options.coinbase_output_max_additional_sigops; // Account for the coinbase tx sigop cost. + if (coinbase_adjusted_block_sigopcost + packageSigOpsCost >= MAX_BLOCK_SIGOPS_COST) { return false; } return true; diff --git a/src/node/miner.h b/src/node/miner.h index f6461a8d55..d0316cb016 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}; @@ -172,7 +172,9 @@ public: /** Construct a new block template */ std::unique_ptr CreateNewBlock(); + /** The number of transactions in the last assembled block (excluding coinbase transaction) */ inline static std::optional m_last_block_num_txs{}; + /** The weight of the last assembled block (excluding coinbase transaction) */ inline static std::optional m_last_block_weight{}; private: diff --git a/src/node/types.h b/src/node/types.h index 4b0de084ab..f3d99b0540 100644 --- a/src/node/types.h +++ b/src/node/types.h @@ -38,7 +38,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 4412f2db87..67697adc04 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/rpc/mining.cpp b/src/rpc/mining.cpp index 7e5936fddf..e45be91aa8 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -419,8 +419,8 @@ static RPCHelpMan getmininginfo() RPCResult::Type::OBJ, "", "", { {RPCResult::Type::NUM, "blocks", "The current block"}, - {RPCResult::Type::NUM, "currentblockweight", /*optional=*/true, "The block weight of the last assembled block (only present if a block was ever assembled)"}, - {RPCResult::Type::NUM, "currentblocktx", /*optional=*/true, "The number of block transactions of the last assembled block (only present if a block was ever assembled)"}, + {RPCResult::Type::NUM, "currentblockweight", /*optional=*/true, "The block weight (excluding coinbase) of the last assembled block (only present if a block was ever assembled)"}, + {RPCResult::Type::NUM, "currentblocktx", /*optional=*/true, "The number of block transactions (excluding coinbase) of the last assembled block (only present if a block was ever assembled)"}, {RPCResult::Type::NUM, "difficulty", "The current difficulty"}, {RPCResult::Type::NUM, "networkhashps", "The network hashes per second"}, {RPCResult::Type::NUM, "pooledtx", "The size of the mempool"}, diff --git a/src/test/fuzz/mini_miner.cpp b/src/test/fuzz/mini_miner.cpp index a5bccd103d..664699d10e 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 @@ -150,9 +151,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)); @@ -174,7 +176,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; miner_options.coinbase_output_script = CScript() << OP_0; diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index d367ec122d..60c7cd791e 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -22,7 +22,10 @@ from test_framework.messages import ( CBlock, CBlockHeader, COIN, + DEFAULT_COINBASE_TX_WEIGHT, + MAX_BLOCK_WEIGHT, ser_uint256, + WITNESS_SCALE_FACTOR ) from test_framework.p2p import P2PDataStore from test_framework.test_framework import BitcoinTestFramework @@ -73,7 +76,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'], 0) self.log.info('test blockversion') self.restart_node(0, extra_args=[f'-mocktime={t}', '-blockversion=1337']) @@ -189,6 +192,109 @@ class MiningTest(BitcoinTestFramework): assert_equal(result, "inconclusive") assert_equal(prune_node.getblock(pruned_blockhash, verbosity=0), pruned_block) + + def send_transactions(self, utxos, fee_rate, target_vsize): + """ + Helper to create and send transactions with the specified target virtual size and fee rate. + """ + for utxo in utxos: + self.wallet.send_self_transfer( + from_node=self.nodes[0], + utxo_to_spend=utxo, + target_vsize=target_vsize, + fee_rate=fee_rate, + ) + + def verify_block_template(self, expected_tx_count, expected_weight): + """ + Create a block template and check that it satisfies the expected transaction count and total weight. + """ + response = self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS) + self.log.info(f"Testing block template: contains {expected_tx_count} transactions, and total weight <= {expected_weight}") + assert_equal(len(response["transactions"]), expected_tx_count) + total_weight = sum(transaction["weight"] for transaction in response["transactions"]) + assert_greater_than_or_equal(expected_weight, total_weight) + + def test_block_max_weight(self): + self.log.info("Testing default and custom -blockmaxweight startup options.") + + # Restart the node to allow large transactions + LARGE_TXS_COUNT = 10 + LARGE_VSIZE = int(((MAX_BLOCK_WEIGHT - DEFAULT_COINBASE_TX_WEIGHT) / WITNESS_SCALE_FACTOR) / LARGE_TXS_COUNT) + HIGH_FEERATE = Decimal("0.0003") + self.restart_node(0, extra_args=[f"-datacarriersize={LARGE_VSIZE}"]) + + # Ensure the mempool is empty + assert_equal(len(self.nodes[0].getrawmempool()), 0) + + # Generate UTXOs and send 10 large transactions with a high fee rate + utxos = [self.wallet.get_utxo(confirmed_only=True) for _ in range(LARGE_TXS_COUNT + 4)] # Add 4 more utxos that will be used in the test later + self.send_transactions(utxos[:LARGE_TXS_COUNT], HIGH_FEERATE, LARGE_VSIZE) + + # Send 2 normal transactions with a lower fee rate + NORMAL_VSIZE = int(2000 / WITNESS_SCALE_FACTOR) + NORMAL_FEERATE = Decimal("0.0001") + self.send_transactions(utxos[LARGE_TXS_COUNT:LARGE_TXS_COUNT + 2], NORMAL_FEERATE, NORMAL_VSIZE) + + # Check that the mempool contains all transactions + self.log.info(f"Testing that the mempool contains {LARGE_TXS_COUNT + 2} transactions.") + assert_equal(len(self.nodes[0].getrawmempool()), LARGE_TXS_COUNT + 2) + + # Verify the block template includes only the 10 high-fee transactions + self.log.info("Testing that the block template includes only the 10 large transactions.") + self.verify_block_template( + expected_tx_count=LARGE_TXS_COUNT, + expected_weight=MAX_BLOCK_WEIGHT - DEFAULT_COINBASE_TX_WEIGHT, + ) + + # Test block template creation with custom -blockmaxweight + custom_block_weight = MAX_BLOCK_WEIGHT - 2000 + # Reducing the weight by 2000 units will prevent 1 large transaction from fitting into the block. + self.restart_node(0, extra_args=[f"-datacarriersize={LARGE_VSIZE}", f"-blockmaxweight={custom_block_weight}"]) + + self.log.info("Testing the block template with custom -blockmaxweight to include 9 large and 2 normal transactions.") + self.verify_block_template( + expected_tx_count=11, + expected_weight=MAX_BLOCK_WEIGHT - DEFAULT_COINBASE_TX_WEIGHT - 2000, + ) + + # Ensure the block weight does not exceed the maximum + self.log.info(f"Testing that the block weight will never exceed {MAX_BLOCK_WEIGHT - DEFAULT_COINBASE_TX_WEIGHT}.") + self.restart_node(0, extra_args=[f"-datacarriersize={LARGE_VSIZE}", f"-blockmaxweight={MAX_BLOCK_WEIGHT}"]) + self.log.info("Sending 2 additional normal transactions to fill the mempool to the maximum block weight.") + self.send_transactions(utxos[LARGE_TXS_COUNT + 2:], NORMAL_FEERATE, NORMAL_VSIZE) + self.log.info(f"Testing that the mempool's weight matches the maximum block weight: {MAX_BLOCK_WEIGHT}.") + assert_equal(self.nodes[0].getmempoolinfo()['bytes'] * WITNESS_SCALE_FACTOR, MAX_BLOCK_WEIGHT) + + self.log.info("Testing that the block template includes only 10 transactions and cannot reach full block weight.") + self.verify_block_template( + expected_tx_count=LARGE_TXS_COUNT, + expected_weight=MAX_BLOCK_WEIGHT - DEFAULT_COINBASE_TX_WEIGHT, + ) + + self.log.info("Test -maxcoinbaseweight startup option.") + # Lowering the -maxcoinbaseweight by 4000 will allow for two more transactions. + self.restart_node(0, extra_args=[f"-datacarriersize={LARGE_VSIZE}", "-maxcoinbaseweight=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 consensus invalid -maxcoinbaseweight") + self.stop_node(0) + self.nodes[0].assert_start_raises_init_error( + extra_args=[f"-maxcoinbaseweight={MAX_BLOCK_WEIGHT + 1}"], + expected_msg=f"Error: Specified -maxcoinbaseweight ({MAX_BLOCK_WEIGHT + 1}) exceeds consensus maximum block weight ({MAX_BLOCK_WEIGHT})", + ) + + self.log.info("Test that node will fail to start when user provide consensus invalid -blockmaxweight") + self.stop_node(0) + self.nodes[0].assert_start_raises_init_error( + extra_args=[f"-blockmaxweight={MAX_BLOCK_WEIGHT + 1}"], + expected_msg=f"Error: Specified -blockmaxweight ({MAX_BLOCK_WEIGHT + 1}) exceeds consensus maximum block weight ({MAX_BLOCK_WEIGHT})", + ) + + def run_test(self): node = self.nodes[0] self.wallet = MiniWallet(node) @@ -406,6 +512,7 @@ class MiningTest(BitcoinTestFramework): assert_equal(node.submitblock(hexdata=block.serialize().hex()), 'duplicate') # valid self.test_blockmintxfee_parameter() + self.test_block_max_weight() self.test_timewarp() self.test_pruning() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index b4a5d9d5ef..97f08d8f20 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