[miniminer] stop assuming ancestor fees >= self fees
What the developer wrote
Authored by glozow
[miniminer] stop assuming ancestor fees >= self fees
Negative fees are possible with prioritisetransaction.
What changed, and why it matters
This commit fixes an internal consistency check in Bitcoin Core's mini-miner, a testing and block-template simulation tool. Previously, the code assumed that a transaction's total fees including ancestors were always at least as large as the transaction's own fee. That assumption can be false when a user uses the prioritisetransaction feature to assign a negative fee to a transaction. The old assumption could cause debug builds to abort (via Assume) or mask incorrect accounting, though this appears limited to the mini-miner and not consensus-critical mining code.
Treat as a low-severity correctness fix. Review whether any other miner or mempool code makes similar ancestor-fee assumptions. No emergency deployment is warranted, but the patch should be included in normal release testing, especially for debug builds using prioritisetransaction.
Security signals we found
Assertion assumption invalidated by negative-fee transaction feature
Potential debug-build crash or incorrect accounting in mini-miner simulation
No direct consensus or network protocol change
Evidence from the diff
In src/node/mini_miner.cpp, two Assume() assertions checked that GetModFeesWithAncestors() >= GetModifiedFee(). Because prioritisetransaction allows negative modified fees, a child transaction’s ancestor-fee total can legitimately be lower than its own modified fee. The patch removes these fee comparisons while keeping the size checks, which remain valid because sizes cannot be negative. This is a correctness fix for assertion logic in the mini-miner, not a consensus or mempool policy change.
Changed components
src/node/mini_miner.cppMiniMiner::DeleteAncestorPackageMiniMiner::SanityCheckInspect captured patch +3 / −5
diff --git a/src/node/mini_miner.cpp b/src/node/mini_miner.cpp
index b847c500..9d85db7d 100644
--- a/src/node/mini_miner.cpp
+++ b/src/node/mini_miner.cpp
@@ -209,8 +209,7 @@ void MiniMiner::DeleteAncestorPackage(const std::set<MockEntryMap::iterator, Ite
// Each entry’s descendant set includes itself
Assume(it != m_descendant_set_by_txid.end());
for (auto& descendant : it->second) {
- // If these fail, we must be double-deducting.
- Assume(descendant->second.GetModFeesWithAncestors() >= anc->second.GetModifiedFee());
+ // If this fails, we must be double-deducting. Don't check fees because negative is possible.
Assume(descendant->second.GetSizeWithAncestors() >= anc->second.GetTxSize());
descendant->second.UpdateAncestorState(-anc->second.GetTxSize(), -anc->second.GetModifiedFee());
}
@@ -234,10 +233,9 @@ void MiniMiner::SanityCheck() const
// m_entries, m_entries_by_txid, and m_descendant_set_by_txid all same size
Assume(m_entries.size() == m_entries_by_txid.size());
Assume(m_entries.size() == m_descendant_set_by_txid.size());
- // Cached ancestor values should be at least as large as the transaction's own fee and size
+ // Cached ancestor values should be at least as large as the transaction's own size
Assume(std::all_of(m_entries.begin(), m_entries.end(), [](const auto& entry) {
- return entry->second.GetSizeWithAncestors() >= entry->second.GetTxSize() &&
- entry->second.GetModFeesWithAncestors() >= entry->second.GetModifiedFee();}));
+ return entry->second.GetSizeWithAncestors() >= entry->second.GetTxSize();}));
// None of the entries should be to-be-replaced transactions
Assume(std::all_of(m_to_be_replaced.begin(), m_to_be_replaced.end(),
[&](const auto& txid){ return !m_entries_by_txid.contains(txid); }));
Why this scored 42/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.