txmempool: Drop CompareMiningScoreWithTopology
What changed, and why it matters
This commit removes an unused helper function called CompareMiningScoreWithTopology from Bitcoin Core's transaction memory pool code. It also updates an internal consistency check to use a different, already-existing comparison method. There is no security issue here—this is ordinary code cleanup after a replacement function was introduced.
No action required. This is a benign cleanup/refactoring commit.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch deletes CTxMemPool::CompareMiningScoreWithTopology and its declaration. The only caller was an assertion inside CTxMemPool::check, which now compares mempool iterators directly using m_txgraph->CompareMainOrder instead of comparing Wtxids through the removed helper. The change is a straightforward refactoring: the old function is no longer used because ExtractBestByMiningScoreWithTopology performs the same job more efficiently.
Changed components
src/txmempool.cppsrc/txmempool.hInspect captured patch +4 / −22
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 4bb86c9b..e5ec3b32 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -458,7 +458,7 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
assert(diagram.size() <= score_with_topo.size() + 1);
assert(diagram.size() >= 1);
- std::optional<Wtxid> last_wtxid = std::nullopt;
+ std::optional<txiter> last_iter = std::nullopt;
auto diagram_iter = diagram.cbegin();
for (const auto& it : score_with_topo) {
@@ -480,11 +480,10 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
innerUsage += it->DynamicMemoryUsage();
const CTransaction& tx = it->GetTx();
- // CompareMiningScoreWithTopology should agree with GetSortedScoreWithTopology()
- if (last_wtxid) {
- assert(CompareMiningScoreWithTopology(*last_wtxid, tx.GetWitnessHash()));
+ if (last_iter) {
+ assert(m_txgraph->CompareMainOrder(**last_iter, *it) < 0);
}
- last_wtxid = tx.GetWitnessHash();
+ last_iter = it;
std::set<CTxMemPoolEntry::CTxMemPoolEntryRef, CompareIteratorByHash> setParentCheck;
std::set<CTxMemPoolEntry::CTxMemPoolEntryRef, CompareIteratorByHash> setParentsStored;
@@ -611,22 +610,6 @@ std::vector<CTxMemPool::txiter> CTxMemPool::ExtractBestByMiningScoreWithTopology
return res;
}
-bool CTxMemPool::CompareMiningScoreWithTopology(const Wtxid& hasha, const Wtxid& hashb) const
-{
- /* Return `true` if hasha should be considered sooner than hashb, namely when:
- * a is not in the mempool but b is, or
- * both are in the mempool but a is sorted before b in the total mempool ordering
- * (which takes dependencies and (chunk) feerates into account).
- */
- LOCK(cs);
- auto j{GetIter(hashb)};
- if (!j.has_value()) return false;
- auto i{GetIter(hasha)};
- if (!i.has_value()) return true;
-
- return m_txgraph->CompareMainOrder(*i.value(), *j.value()) < 0;
-}
-
std::vector<CTxMemPool::indexed_transaction_set::const_iterator> CTxMemPool::GetSortedScoreWithTopology() const
{
std::vector<indexed_transaction_set::const_iterator> iters;
diff --git a/src/txmempool.h b/src/txmempool.h
index dcc9ed87..d2324d1c 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -344,7 +344,6 @@ public:
* mempool.cs is released.
*/
std::vector<txiter> ExtractBestByMiningScoreWithTopology(std::vector<Wtxid>& wtxids, size_t n_to_sort) const EXCLUSIVE_LOCKS_REQUIRED(cs);
- bool CompareMiningScoreWithTopology(const Wtxid& hasha, const Wtxid& hashb) const;
bool isSpent(const COutPoint& outpoint) const;
unsigned int GetTransactionsUpdated() const;
void AddTransactionsUpdated(unsigned int n);
Why this scored 15/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.