Rewrite GatherClusters to use the txgraph implementation
What changed, and why it matters
This commit rewrites an internal Bitcoin Core function called GatherClusters so it uses a newer transaction graph (txgraph) implementation instead of manually walking parent/child relationships. The change is a refactor that preserves the same 500-entry DoS limit but moves where and how that limit is enforced. There is no direct evidence in the commit that this fixes a security bug, but refactors of mempool cluster logic can affect denial-of-service protections.
Review the txgraph GetCluster implementation to confirm the 500-entry cap is still effective and that cluster expansion cannot be induced to perform unbounded work before the size check. Consider whether the post-collection size check changes worst-case CPU/memory behavior compared to the old early-exit BFS.
Security signals we found
DoS protection limit (500 entries) relocated from inside the traversal to after cluster collection
Refactor of mempool cluster traversal logic, a historically sensitive area for resource exhaustion
Removal of epoch-based visited tracking in favor of txgraph cluster representatives
No explicit security bug or CVE mentioned in commit message or diff
Evidence from the diff
GatherClusters previously built a vector of mempool entries by BFS over GetMemPoolParentsConst/GetMemPoolChildrenConst, marking visited entries with an epoch and returning early if the vector exceeded 500 entries. The new implementation asks m_txgraph for precomputed clusters, deduplicates by cluster representative, flattens clusters into a result vector, and only checks the size limit after all clusters are gathered. The functional behavior (return connected component entries for the given txids, capped at 500) is intended to be equivalent, but the timing and scope of the size check differ: the old code aborted during expansion, while the new code aborts after full expansion.
Changed components
src/txmempool.cppCTxMemPool::GatherClustersmempool cluster/ancestor-descendant traversalInspect captured patch +14 / −18
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 921765de..53d92b8d 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -977,28 +977,24 @@ void CTxMemPool::SetLoadTried(bool load_tried)
std::vector<CTxMemPool::txiter> CTxMemPool::GatherClusters(const std::vector<Txid>& txids) const
{
AssertLockHeld(cs);
- std::vector<txiter> clustered_txs{GetIterVec(txids)};
- // Use epoch: visiting an entry means we have added it to the clustered_txs vector. It does not
- // necessarily mean the entry has been processed.
- WITH_FRESH_EPOCH(m_epoch);
- for (const auto& it : clustered_txs) {
- visited(it);
- }
- // i = index of where the list of entries to process starts
- for (size_t i{0}; i < clustered_txs.size(); ++i) {
- // DoS protection: if there are 500 or more entries to process, just quit.
- if (clustered_txs.size() > 500) return {};
- const txiter& tx_iter = clustered_txs.at(i);
- for (const auto& entries : {tx_iter->GetMemPoolParentsConst(), tx_iter->GetMemPoolChildrenConst()}) {
- for (const CTxMemPoolEntry& entry : entries) {
- const auto entry_it = mapTx.iterator_to(entry);
- if (!visited(entry_it)) {
- clustered_txs.push_back(entry_it);
+
+ std::vector<CTxMemPool::txiter> ret;
+ std::set<const CTxMemPoolEntry*> unique_cluster_representatives;
+ for (auto txid : txids) {
+ auto it = mapTx.find(txid);
+ if (it != mapTx.end()) {
+ auto cluster = m_txgraph->GetCluster(*it, TxGraph::Level::MAIN);
+ if (unique_cluster_representatives.insert(static_cast<const CTxMemPoolEntry*>(&(**cluster.begin()))).second) {
+ for (auto tx : cluster) {
+ ret.emplace_back(mapTx.iterator_to(static_cast<const CTxMemPoolEntry&>(*tx)));
}
}
}
}
- return clustered_txs;
+ if (ret.size() > 500) {
+ return {};
+ }
+ return ret;
}
util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool::ChangeSet::CalculateChunksForRBF()
Why this scored 28/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.