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

@@ -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<CBlockTemplate> 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;

src/node/miner.h

@@ -172,7 +172,9 @@ public:
     /** Construct a new block template with coinbase to scriptPubKeyIn */
     std::unique_ptr<CBlockTemplate> CreateNewBlock(const CScript& scriptPubKeyIn);
 
+    /** The number of transactions in the last assembled block (excluding coinbase transaction) */
     inline static std::optional<int64_t> m_last_block_num_txs{};
+    /** The weight of the last assembled block (excluding coinbase transaction) */
     inline static std::optional<int64_t> m_last_block_weight{};
 
 private:

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, "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 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"},

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'])