From 5be248341a8e9ad97143647e7f480eecf1f90eb4 Mon Sep 17 00:00:00 2001 From: ismaelsadeeq Date: Mon, 22 Jun 2026 10:53:54 +0100 Subject: [PATCH] bugfix: compare real chunk weight against block weight limit TestChunkBlockLimits() compared the chunk's sigops-adjusted weight against block_max_weight, while nBlockWeight tracks real transaction weight. This over-counted sigop-dense chunks and could skip ones that actually fit, losing fees; the block sigop limit is enforced separately on the next line. Pass the chunk's real weight (sum of GetTxWeight()) instead, and update the test to show the chunk is now included. --- src/node/miner.cpp | 8 +++++--- src/node/miner.h | 2 +- src/test/miner_tests.cpp | 3 ++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 32a21440c4e..b3fd020820e 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -243,11 +243,11 @@ std::unique_ptr BlockAssembler::CreateNewBlock() return std::move(pblocktemplate); } -bool BlockAssembler::TestChunkBlockLimits(FeePerWeight chunk_feerate, int64_t chunk_sigops_cost) const +bool BlockAssembler::TestChunkBlockLimits(int64_t chunk_weight, int64_t chunk_sigops_cost) const { // block_max_weight has been flattened before block assembly limit checks. Assert(m_options.block_max_weight); - if (nBlockWeight + chunk_feerate.size >= *m_options.block_max_weight) { + if (nBlockWeight + chunk_weight >= m_options.block_max_weight) { return false; } if (nBlockSigOpsCost + chunk_sigops_cost >= MAX_BLOCK_SIGOPS_COST) { @@ -310,12 +310,14 @@ void BlockAssembler::addChunks() } int64_t chunk_sig_ops = 0; + int64_t chunk_weight = 0; for (const auto& tx : selected_transactions) { chunk_sig_ops += tx.get().GetSigOpCost(); + chunk_weight += tx.get().GetTxWeight(); } // Check to see if this chunk will fit. - if (!TestChunkBlockLimits(chunk_feerate, chunk_sig_ops) || !TestChunkTransactions(selected_transactions)) { + if (!TestChunkBlockLimits(chunk_weight, chunk_sig_ops) || !TestChunkTransactions(selected_transactions)) { // This chunk won't fit, so we skip it and will try the next best one. m_mempool->SkipBuilderChunk(); ++nConsecutiveFailed; diff --git a/src/node/miner.h b/src/node/miner.h index af327307329..08fa3f1d2af 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -108,7 +108,7 @@ private: // helper functions for addChunks() /** Test if a new chunk would "fit" in the block */ - bool TestChunkBlockLimits(FeePerWeight chunk_feerate, int64_t chunk_sigops_cost) const; + bool TestChunkBlockLimits(int64_t chunk_weight, int64_t chunk_sigops_cost) const; /** Perform locktime checks on each transaction in a chunk: * This check should always succeed, and is here * only as an extra check in case of a bug */ diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index fc80e7dd8ad..b91ec8b8b6f 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -368,7 +368,8 @@ void MinerTestingSetup::TestSigOpsAdjustedWeightChunkLimit(const CScript& script .coinbase_output_script = scriptPubKey, }; const CBlock block{mining->createNewBlock(options, /*cooldown=*/false)->getBlock()}; - BOOST_CHECK_EQUAL(block.vtx.size(), 1U); + BOOST_CHECK_EQUAL(block.vtx.size(), 2U); + BOOST_CHECK(block.vtx[1]->GetHash() == tx.GetHash()); } void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::vector& txFirst, int baseheight)