From 6e017801435e91036d526ae19634bf1c6f7aeec9 Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Fri, 3 Jan 2025 19:41:37 -0500 Subject: [PATCH] miner: fix block weight and block sigop count accounting - The reserved weight and sigops count of the coinbase transaction is an estimate and may not represent the exact values; it can be lower. Adding these values to the total block weight, and total block sigop count of the assembled block might result in an incorrect total values when the real values are lower than the reserved ones. - Fix this by not adding the reserved values to the total. - Also make it explicit that total block weight, sigops count, and transaction count does not include the coinbase transaction. --- src/node/miner.cpp | 15 +++++++-------- src/node/miner.h | 2 ++ src/rpc/mining.cpp | 4 ++-- test/functional/mining_basic.py | 2 +- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 1cc53468b6..37dfd07d9a 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -96,12 +96,9 @@ void ApplyArgsManOptions(const ArgsManager& args, BlockAssembler::Options& optio 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 +155,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc 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 +194,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 c67419c495..315cd02a72 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -172,7 +172,9 @@ public: /** Construct a new block template with coinbase to scriptPubKeyIn */ std::unique_ptr CreateNewBlock(const CScript& scriptPubKeyIn); + /** 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/rpc/mining.cpp b/src/rpc/mining.cpp index 60828c1711..d202e160ea 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -418,8 +418,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/test/functional/mining_basic.py b/test/functional/mining_basic.py index 973c9cb51a..f160b6e180 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -76,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'], DEFAULT_COINBASE_TX_WEIGHT) + assert_equal(mining_info['currentblockweight'], 0) self.log.info('test blockversion') self.restart_node(0, extra_args=[f'-mocktime={t}', '-blockversion=1337'])