Add check that GetSortedScoreWithTopology() agrees with CompareMiningScoreWithTopology()
What the developer wrote
Authored by Suhas Daftuar
Add check that GetSortedScoreWithTopology() agrees with CompareMiningScoreWithTopology()
We use CompareMiningScoreWithTopology() for sorting transaction announcements
during tx relay, and we use GetSortedScoreWithTopology() in
CTxMemPool::check().
What changed, and why it matters
This commit adds an internal consistency check inside Bitcoin Core's memory pool validation routine. It verifies that two different ways of ranking transactions by mining priority produce the same order. It is a defensive assertion, not a fix for a known bug or vulnerability, and it does not change network behavior.
No action required. Treat as routine hardening/test-coverage improvement. Reviewers may want to confirm the assertion is not enabled in production release builds.
Security signals we found
Defensive consistency assertion between two mempool scoring functions
No change to consensus, P2P protocol, or transaction acceptance logic
No patch of an exploitable bug; adds verification only
Evidence from the diff
In CTxMemPool::check(), the code now iterates over GetSortedScoreWithTopology() and asserts that each successive pair of transactions is also ordered according to CompareMiningScoreWithTopology(). This is a sanity assertion linking the sort used in CTxMemPool::check() with the comparator used during transaction relay. It only runs when mempool consistency checks are enabled and aborts on mismatch.
Changed components
src/txmempool.cppCTxMemPool::check()GetSortedScoreWithTopology()CompareMiningScoreWithTopology()Inspect captured patch +9 / −0
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 5197fb01..592f6194 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -425,11 +425,20 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
CCoinsViewCache mempoolDuplicate(const_cast<CCoinsViewCache*>(&active_coins_tip));
+ std::optional<Wtxid> last_wtxid = std::nullopt;
+
for (const auto& it : GetSortedScoreWithTopology()) {
checkTotal += it->GetTxSize();
check_total_fee += it->GetFee();
innerUsage += it->DynamicMemoryUsage();
const CTransaction& tx = it->GetTx();
+
+ // CompareMiningScoreWithTopology should agree with GetSortedScoreWithTopology()
+ if (last_wtxid) {
+ assert(CompareMiningScoreWithTopology(*last_wtxid, tx.GetWitnessHash()));
+ }
+ last_wtxid = tx.GetWitnessHash();
+
std::set<CTxMemPoolEntry::CTxMemPoolEntryRef, CompareIteratorByHash> setParentCheck;
std::set<CTxMemPoolEntry::CTxMemPoolEntryRef, CompareIteratorByHash> setParentsStored;
for (const CTxIn &txin : tx.vin) {
Why this scored 16/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.