Rework CTxMemPool::GetChildren() to not use epochs
What changed, and why it matters
This commit is a small internal cleanup in Bitcoin Core's memory pool (mempool) code. It replaces a fast but complex deduplication trick ('epochs') with a simpler, slightly slower approach using sorting. The change saves 8 bytes per mempool entry and removes unused code. There is no indication this fixes a security bug; it appears to be ordinary maintenance/refactoring.
No security action required. Treat as normal code-review/maintenance commit.
Security signals we found
No security-relevant signals in commit message or diff
Refactoring/cleanup only: removal of Epoch deduplication in favor of sort+unique
No input validation, cryptography, network parsing, or consensus code changed
No bug pattern (use-after-free, overflow, race, etc.) is corrected by the diff
Evidence from the diff
CTxMemPool::GetChildren() previously used Epoch::Marker/Guard to avoid returning duplicate child entries when iterating mapNextTx. The patch removes the epoch machinery, collects all matches, sorts by hash, and removes duplicates by pointer equality. It also removes the m_epoch_marker field from CTxMemPoolEntry and the m_epoch member from CTxMemPool, shrinking entries by 8 bytes. The lock scope is narrowed to just the map lookup. The commit message explicitly frames this as a performance/memory trade-off, not a security fix.
Changed components
src/txmempool.cppsrc/txmempool.hsrc/kernel/mempool_entry.hInspect captured patch +9 / −29
diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h
index 3053a77c..58824f7a 100644
--- a/src/kernel/mempool_entry.h
+++ b/src/kernel/mempool_entry.h
@@ -12,7 +12,6 @@
#include <policy/settings.h>
#include <primitives/transaction.h>
#include <txgraph.h>
-#include <util/epochguard.h>
#include <util/overflow.h>
#include <chrono>
@@ -138,7 +137,6 @@ public:
bool GetSpendsCoinbase() const { return spendsCoinbase; }
mutable size_t idx_randomized; //!< Index in mempool's txns_randomized
- mutable Epoch::Marker m_epoch_marker; //!< epoch when last touched, useful for graph algorithms
};
using CTxMemPoolEntryRef = CTxMemPoolEntry::CTxMemPoolEntryRef;
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 4dc692bf..2486270b 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -56,15 +56,18 @@ bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp)
std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef> CTxMemPool::GetChildren(const CTxMemPoolEntry& entry) const
{
- LOCK(cs);
std::vector<CTxMemPoolEntry::CTxMemPoolEntryRef> ret;
- WITH_FRESH_EPOCH(m_epoch);
- auto iter = mapNextTx.lower_bound(COutPoint(entry.GetTx().GetHash(), 0));
- for (; iter != mapNextTx.end() && iter->first->hash == entry.GetTx().GetHash(); ++iter) {
- if (!visited(iter->second)) {
+ const auto& hash = entry.GetTx().GetHash();
+ {
+ LOCK(cs);
+ auto iter = mapNextTx.lower_bound(COutPoint(hash, 0));
+ for (; iter != mapNextTx.end() && iter->first->hash == hash; ++iter) {
ret.emplace_back(*(iter->second));
}
}
+ std::ranges::sort(ret, CompareIteratorByHash{});
+ auto removed = std::ranges::unique(ret, [](auto& a, auto& b) noexcept { return &a.get() == &b.get(); });
+ ret.erase(removed.begin(), removed.end());
return ret;
}
diff --git a/src/txmempool.h b/src/txmempool.h
index 996a1fe0..9feeee3e 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -20,7 +20,6 @@
#include <primitives/transaction_identifier.h>
#include <sync.h>
#include <txgraph.h>
-#include <util/epochguard.h>
#include <util/feefrac.h>
#include <util/hasher.h>
#include <util/result.h>
@@ -197,7 +196,6 @@ protected:
mutable int64_t lastRollingFeeUpdate GUARDED_BY(cs){GetTime()};
mutable bool blockSinceLastRollingFeeBump GUARDED_BY(cs){false};
mutable double rollingMinimumFeeRate GUARDED_BY(cs){0}; //!< minimum fee to get into the pool, decreases exponentially
- mutable Epoch m_epoch GUARDED_BY(cs){};
// In-memory counter for external mempool tracking purposes.
// This number is incremented once every time a transaction
@@ -394,7 +392,7 @@ public:
* @param[in] vHashesToUpdate The set of txids from the
* disconnected block that have been accepted back into the mempool.
*/
- void UpdateTransactionsFromBlock(const std::vector<Txid>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main) LOCKS_EXCLUDED(m_epoch);
+ void UpdateTransactionsFromBlock(const std::vector<Txid>& vHashesToUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
std::vector<FeePerWeight> GetFeerateDiagram() const EXCLUSIVE_LOCKS_REQUIRED(cs);
FeePerWeight GetMainChunkFeerate(const CTxMemPoolEntry& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs) {
@@ -592,25 +590,6 @@ private:
/* Removal from the mempool also triggers removal of the entry's Ref from txgraph. */
void removeUnchecked(txiter entry, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
public:
- /** visited marks a CTxMemPoolEntry as having been traversed
- * during the lifetime of the most recently created Epoch::Guard
- * and returns false if we are the first visitor, true otherwise.
- *
- * An Epoch::Guard must be held when visited is called or an assert will be
- * triggered.
- *
- */
- bool visited(const txiter it) const EXCLUSIVE_LOCKS_REQUIRED(cs, m_epoch)
- {
- return m_epoch.visited(it->m_epoch_marker);
- }
-
- bool visited(std::optional<txiter> it) const EXCLUSIVE_LOCKS_REQUIRED(cs, m_epoch)
- {
- assert(m_epoch.guarded()); // verify guard even when it==nullopt
- return !it || visited(*it);
- }
-
/*
* CTxMemPool::ChangeSet:
*
Why this scored 18/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.