miner: fix empty mempool case for waitNext()
What the developer wrote
Authored by Sjors Provoost
miner: fix empty mempool case for waitNext()
Block template fees are calculated by looping over new_tmpl->vTxFees
and return (early) once the fee_threshold is exceeded.
This left an edge case when the mempool is empty, which this commit
fixes and adds a test for. It does so by using std::accumulate instead
of manual loops.
Also update interface_ipc.py to account for the new behavior.
Co-authored-by: Raimo33 <claudio.raimondi@protonmail.com>
What changed, and why it matters
This commit fixes a small logic bug in Bitcoin Core's block-template waiting code. When the mempool is empty, the code that decides whether a newly created block template is 'better' than the old one would never finish its comparison loop, so it could fail to return a result or behave unexpectedly. The fix replaces the manual loop with a simple sum of all fees, and adds tests for the empty-mempool case. It is a correctness fix rather than a critical security vulnerability.
No immediate action required beyond normal review and merge. The change is a straightforward bug fix with accompanying tests. Operators using the mining IPC interface should ensure they run a version containing this fix to avoid unexpected waitNext() behavior when the mempool is empty.
Security signals we found
Logic bug in fee-threshold comparison loop
Empty mempool edge case left unhandled
Potential denial-of-service or hang in miner IPC interface
Fix verified by new unit and functional tests
Evidence from the diff
In WaitAndCreateNewBlock(), the original code computed new_fees by iterating over new_tmpl->vTxFees and returning early once new_fees >= current_fees + fee_threshold. When the mempool is empty, vTxFees contains only the coinbase transaction fee (which is typically zero or negative, i.e., not a fee paid by users), so the early-return condition could never be satisfied, leaving waitNext() hanging or returning an unexpected result. The patch replaces the manual loop with std::accumulate over the entire vector and performs a single comparison. Tests are added to verify that waitNext() returns nullptr on an empty mempool when fee_threshold > 0, and returns a template when fee_threshold == 0. The functional interface_ipc.py test is updated to explicitly set feeThreshold values.
Changed components
src/node/miner.cppsrc/test/miner_tests.cpptest/functional/interface_ipc.pyInspect captured patch +27 / −13
diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index e75bc3a6..28e9048a 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -29,6 +29,7 @@
#include <algorithm>
#include <utility>
+#include <numeric>
namespace node {
@@ -522,18 +523,13 @@ std::unique_ptr<CBlockTemplate> WaitAndCreateNewBlock(ChainstateManager& chainma
// Calculate the original template total fees if we haven't already
if (current_fees == -1) {
- current_fees = 0;
- for (CAmount fee : block_template->vTxFees) {
- current_fees += fee;
- }
+ current_fees = std::accumulate(block_template->vTxFees.begin(), block_template->vTxFees.end(), CAmount{0});
}
- CAmount new_fees = 0;
- for (CAmount fee : new_tmpl->vTxFees) {
- new_fees += fee;
- Assume(options.fee_threshold != MAX_MONEY);
- if (new_fees >= current_fees + options.fee_threshold) return new_tmpl;
- }
+ // Check if fees increased enough to return the new template
+ const CAmount new_fees = std::accumulate(new_tmpl->vTxFees.begin(), new_tmpl->vTxFees.end(), CAmount{0});
+ Assume(options.fee_threshold != MAX_MONEY);
+ if (new_fees >= current_fees + options.fee_threshold) return new_tmpl;
}
now = NodeClock::now();
diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp
index 652ec25f..94ade598 100644
--- a/src/test/miner_tests.cpp
+++ b/src/test/miner_tests.cpp
@@ -113,6 +113,22 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
options.coinbase_output_script = scriptPubKey;
LOCK(tx_mempool.cs);
+ BOOST_CHECK(tx_mempool.size() == 0);
+
+ // Block template should only have a coinbase when there's nothing in the mempool
+ std::unique_ptr<BlockTemplate> block_template = mining->createNewBlock(options);
+ BOOST_REQUIRE(block_template);
+ CBlock block{block_template->getBlock()};
+ BOOST_REQUIRE_EQUAL(block.vtx.size(), 1U);
+
+ // waitNext() on an empty mempool should return nullptr because there is no better template
+ auto should_be_nullptr = block_template->waitNext({.timeout = MillisecondsDouble{0}, .fee_threshold = 1});
+ BOOST_REQUIRE(should_be_nullptr == nullptr);
+
+ // Unless fee_threshold is 0
+ block_template = block_template->waitNext({.timeout = MillisecondsDouble{0}, .fee_threshold = 0});
+ BOOST_REQUIRE(block_template);
+
// Test the ancestor feerate transaction selection.
TestMemPoolEntryHelper entry;
@@ -144,9 +160,9 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
const auto high_fee_tx{entry.Fee(50000).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx)};
AddToMempool(tx_mempool, high_fee_tx);
- std::unique_ptr<BlockTemplate> block_template = mining->createNewBlock(options);
+ block_template = mining->createNewBlock(options);
BOOST_REQUIRE(block_template);
- CBlock block{block_template->getBlock()};
+ block = block_template->getBlock();
BOOST_REQUIRE_EQUAL(block.vtx.size(), 4U);
BOOST_CHECK(block.vtx[1]->GetHash() == hashParentTx);
BOOST_CHECK(block.vtx[2]->GetHash() == hashHighFeeTx);
@@ -184,7 +200,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
AddToMempool(tx_mempool, entry.Fee(feeToUse).FromTx(tx));
// waitNext() should return nullptr because there is no better template
- auto should_be_nullptr = block_template->waitNext({.timeout = MillisecondsDouble{0}, .fee_threshold = 1});
+ should_be_nullptr = block_template->waitNext({.timeout = MillisecondsDouble{0}, .fee_threshold = 1});
BOOST_REQUIRE(should_be_nullptr == nullptr);
block = block_template->getBlock();
diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
index 4231e95b..abcc4d6b 100755
--- a/test/functional/interface_ipc.py
+++ b/test/functional/interface_ipc.py
@@ -153,6 +153,7 @@ class IPCInterfaceTest(BitcoinTestFramework):
self.log.debug("Wait for a new template")
waitoptions = self.capnp_modules['mining'].BlockWaitOptions()
waitoptions.timeout = timeout
+ waitoptions.feeThreshold = 1
waitnext = template.result.waitNext(ctx, waitoptions)
self.generate(self.nodes[0], 1)
template2 = await waitnext
@@ -168,6 +169,7 @@ class IPCInterfaceTest(BitcoinTestFramework):
block3 = await self.parse_and_deserialize_block(template4, ctx)
assert_equal(len(block3.vtx), 2)
self.log.debug("Wait again, this should return the same template, since the fee threshold is zero")
+ waitoptions.feeThreshold = 0
template5 = await template4.result.waitNext(ctx, waitoptions)
block4 = await self.parse_and_deserialize_block(template5, ctx)
assert_equal(len(block4.vtx), 2)
Why this scored 23/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.