Stop tracking parents/children outside of txgraph
What changed, and why it matters
This commit is a code cleanup in Bitcoin Core's memory pool (mempool). It removes duplicate bookkeeping: parent/child relationships between transactions used to be tracked both inside each mempool entry object and inside a newer 'txgraph' module. The change makes txgraph the single source of truth for those relationships. There is no direct evidence in the commit or supplied references that this fixes a security vulnerability; it appears to be a refactoring to simplify data structures and reduce maintenance risk.
Treat as a normal refactoring commit. Reviewers should verify that txgraph correctly maintains all parent/child relationships previously held in CTxMemPoolEntry, especially around reorg handling (UpdateTransactionsFromBlock), eviction (TrimToSize/RemoveStaged), and the mempool check assertions. No immediate security response is indicated by the available evidence.
Security signals we found
Removal of duplicated mutable state that could become inconsistent with txgraph
No new input validation, bounds checks, or cryptographic operations introduced
No mention of vulnerability, CVE, bug bounty, or security fix in commit message or diff
Evidence from the diff
The patch deletes the mutable Parents/Children sets from CTxMemPoolEntry and removes the corresponding UpdateParent/UpdateChild/UpdateAncestorsOf/UpdateForRemoveFromMempool helpers from CTxMemPool. Parent/child links are now obtained via TxGraph::Ref methods (GetParents/GetChildren). The mempool’s consistency check is updated to compare mapNextTx and mapTx against the txgraph-derived sets instead of the removed entry-local sets. The change is purely structural: dependency information is still maintained, just in one place (m_txgraph) rather than two.
Changed components
src/kernel/mempool_entry.hsrc/txmempool.cppsrc/txmempool.hInspect captured patch +14 / −110
diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h
index e12047de..b274e1f7 100644
--- a/src/kernel/mempool_entry.h
+++ b/src/kernel/mempool_entry.h
@@ -67,16 +67,11 @@ class CTxMemPoolEntry : public TxGraph::Ref
{
public:
typedef std::reference_wrapper<const CTxMemPoolEntry> CTxMemPoolEntryRef;
- // two aliases, should the types ever diverge
- typedef std::set<CTxMemPoolEntryRef, CompareIteratorByHash> Parents;
- typedef std::set<CTxMemPoolEntryRef, CompareIteratorByHash> Children;
private:
CTxMemPoolEntry(const CTxMemPoolEntry&) = delete;
const CTransactionRef tx;
- mutable Parents m_parents;
- mutable Children m_children;
const CAmount nFee; //!< Cached to avoid expensive parent-transaction lookups
const int32_t nTxWeight; //!< ... and avoid recomputing tx weight (also used for GetTxSize())
const size_t nUsageSize; //!< ... and total memory usage
@@ -142,11 +137,6 @@ public:
bool GetSpendsCoinbase() const { return spendsCoinbase; }
- const Parents& GetMemPoolParentsConst() const { return m_parents; }
- const Children& GetMemPoolChildrenConst() const { return m_children; }
- Parents& GetMemPoolParents() const { return m_parents; }
- Children& GetMemPoolChildren() const { return m_children; }
-
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
};
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 53d92b8d..64acf952 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -90,10 +90,6 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<Txid>& vHashesToU
{
AssertLockHeld(cs);
- // Use a set for lookups into vHashesToUpdate (these entries are already
- // accounted for in the state of their ancestors)
- std::set<Txid> setAlreadyIncluded(vHashesToUpdate.begin(), vHashesToUpdate.end());
-
// Iterate in reverse, so that whenever we are looking at a transaction
// we are sure that all in-mempool descendants have already been processed.
for (const Txid& hash : vHashesToUpdate | std::views::reverse) {
@@ -103,20 +99,10 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<Txid>& vHashesToU
continue;
}
auto iter = mapNextTx.lower_bound(COutPoint(hash, 0));
- // First calculate the children, and update CTxMemPoolEntry::m_children to
- // include them, and update their CTxMemPoolEntry::m_parents to include this tx.
- // we cache the in-mempool children to avoid duplicate updates
{
- WITH_FRESH_EPOCH(m_epoch);
for (; iter != mapNextTx.end() && iter->first->hash == hash; ++iter) {
txiter childIter = iter->second;
assert(childIter != mapTx.end());
- // We can skip updating entries we've encountered before or that
- // are in the block (which are already accounted for).
- if (!visited(childIter) && !setAlreadyIncluded.count(iter->second->GetTx().GetHash())) {
- UpdateChild(it, childIter, true);
- UpdateParent(childIter, it, true);
- }
// Add dependencies that are discovered between transactions in the
// block and transactions that were in the mempool to txgraph.
m_txgraph->AddDependency(/*parent=*/*it, /*child=*/*childIter);
@@ -158,8 +144,6 @@ CTxMemPool::setEntries CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEnt
const CTransaction &tx = entry.GetTx();
// Get parents of this transaction that are in the mempool
- // GetMemPoolParents() is only valid for entries in the mempool, so we
- // iterate mapTx to find parents.
for (unsigned int i = 0; i < tx.vin.size(); i++) {
std::optional<txiter> piter = GetIter(tx.vin[i].prevout.hash);
if (piter) {
@@ -177,38 +161,6 @@ CTxMemPool::setEntries CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEnt
return ret;
}
-void CTxMemPool::UpdateAncestorsOf(bool add, txiter it)
-{
- const CTxMemPoolEntry::Parents& parents = it->GetMemPoolParentsConst();
- // add or remove this tx as a child of each parent
- for (const CTxMemPoolEntry& parent : parents) {
- UpdateChild(mapTx.iterator_to(parent), it, add);
- }
-}
-
-void CTxMemPool::UpdateChildrenForRemoval(txiter it)
-{
- const CTxMemPoolEntry::Children& children = it->GetMemPoolChildrenConst();
- for (const CTxMemPoolEntry& updateIt : children) {
- UpdateParent(mapTx.iterator_to(updateIt), it, false);
- }
-}
-
-void CTxMemPool::UpdateForRemoveFromMempool(const setEntries &entriesToRemove, bool updateDescendants)
-{
- for (txiter removeIt : entriesToRemove) {
- // Note that UpdateAncestorsOf severs the child links that point to
- // removeIt in the entries for the parents of removeIt.
- UpdateAncestorsOf(false, removeIt);
- }
- // After updating all the ancestor sizes, we can now sever the link between each
- // transaction being removed and any mempool children (ie, update CTxMemPoolEntry::m_parents
- // for each direct child of a transaction being removed).
- for (txiter removeIt : entriesToRemove) {
- UpdateChildrenForRemoval(removeIt);
- }
-}
-
static CTxMemPool::Options&& Flatten(CTxMemPool::Options&& opts, bilingual_str& error)
{
opts.check_ratio = std::clamp<int>(opts.check_ratio, 0, 1'000'000);
@@ -271,10 +223,8 @@ void CTxMemPool::addNewTransaction(CTxMemPool::txiter newit)
cachedInnerUsage += entry.DynamicMemoryUsage();
const CTransaction& tx = newit->GetTx();
- std::set<Txid> setParentTransactions;
for (unsigned int i = 0; i < tx.vin.size(); i++) {
mapNextTx.insert(std::make_pair(&tx.vin[i].prevout, newit));
- setParentTransactions.insert(tx.vin[i].prevout.hash);
}
// Don't bother worrying about child transactions of this one.
// Normal case of a new transaction arriving is that there can't be any
@@ -283,12 +233,6 @@ void CTxMemPool::addNewTransaction(CTxMemPool::txiter newit)
// In that case, our disconnect block logic will call UpdateTransactionsFromBlock
// to clean up the mess we're leaving here.
- // Update ancestors with information about this tx
- for (const auto& pit : GetIterSet(setParentTransactions)) {
- UpdateParent(newit, pit, true);
- }
- UpdateAncestorsOf(true, newit);
-
nTransactionsUpdated++;
totalTxSize += entry.GetTxSize();
m_total_fee += entry.GetFee();
@@ -344,7 +288,6 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
totalTxSize -= it->GetTxSize();
m_total_fee -= it->GetFee();
cachedInnerUsage -= it->DynamicMemoryUsage();
- cachedInnerUsage -= memusage::DynamicUsage(it->GetMemPoolParentsConst()) + memusage::DynamicUsage(it->GetMemPoolChildrenConst());
mapTx.erase(it);
nTransactionsUpdated++;
}
@@ -483,8 +426,8 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
check_total_fee += it->GetFee();
innerUsage += it->DynamicMemoryUsage();
const CTransaction& tx = it->GetTx();
- innerUsage += memusage::DynamicUsage(it->GetMemPoolParentsConst()) + memusage::DynamicUsage(it->GetMemPoolChildrenConst());
- CTxMemPoolEntry::Parents setParentCheck;
+ std::set<CTxMemPoolEntry::CTxMemPoolEntryRef, CompareIteratorByHash> setParentCheck;
+ std::set<CTxMemPoolEntry::CTxMemPoolEntryRef, CompareIteratorByHash> setParentsStored;
for (const CTxIn &txin : tx.vin) {
// Check that every mempool transaction's inputs refer to available coins, or other mempool tx's.
indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash);
@@ -507,19 +450,26 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
auto comp = [](const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) -> bool {
return a.GetTx().GetHash() == b.GetTx().GetHash();
};
- assert(setParentCheck.size() == it->GetMemPoolParentsConst().size());
- assert(std::equal(setParentCheck.begin(), setParentCheck.end(), it->GetMemPoolParentsConst().begin(), comp));
+ for (auto &txentry : GetParents(*it)) {
+ setParentsStored.insert(dynamic_cast<const CTxMemPoolEntry&>(txentry.get()));
+ }
+ assert(setParentCheck.size() == setParentsStored.size());
+ assert(std::equal(setParentCheck.begin(), setParentCheck.end(), setParentsStored.begin(), comp));
// Check children against mapNextTx
- CTxMemPoolEntry::Children setChildrenCheck;
+ std::set<CTxMemPoolEntry::CTxMemPoolEntryRef, CompareIteratorByHash> setChildrenCheck;
+ std::set<CTxMemPoolEntry::CTxMemPoolEntryRef, CompareIteratorByHash> setChildrenStored;
auto iter = mapNextTx.lower_bound(COutPoint(it->GetTx().GetHash(), 0));
for (; iter != mapNextTx.end() && iter->first->hash == it->GetTx().GetHash(); ++iter) {
txiter childit = iter->second;
assert(childit != mapTx.end()); // mapNextTx points to in-mempool transactions
setChildrenCheck.insert(*childit);
}
- assert(setChildrenCheck.size() == it->GetMemPoolChildrenConst().size());
- assert(std::equal(setChildrenCheck.begin(), setChildrenCheck.end(), it->GetMemPoolChildrenConst().begin(), comp));
+ for (auto &txentry : GetChildren(*it)) {
+ setChildrenStored.insert(dynamic_cast<const CTxMemPoolEntry&>(txentry.get()));
+ }
+ assert(setChildrenCheck.size() == setChildrenStored.size());
+ assert(std::equal(setChildrenCheck.begin(), setChildrenCheck.end(), setChildrenStored.begin(), comp));
TxValidationState dummy_state; // Not used. CheckTxInputs() should always pass
CAmount txfee = 0;
@@ -777,7 +727,6 @@ void CTxMemPool::RemoveUnbroadcastTx(const Txid& txid, const bool unchecked) {
void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason) {
AssertLockHeld(cs);
- UpdateForRemoveFromMempool(stage, updateDescendants);
for (txiter it : stage) {
removeUnchecked(it, reason);
}
@@ -812,28 +761,6 @@ int CTxMemPool::Expire(std::chrono::seconds time)
return stage.size();
}
-void CTxMemPool::UpdateChild(txiter entry, txiter child, bool add)
-{
- AssertLockHeld(cs);
- CTxMemPoolEntry::Children s;
- if (add && entry->GetMemPoolChildren().insert(*child).second) {
- cachedInnerUsage += memusage::IncrementalDynamicUsage(s);
- } else if (!add && entry->GetMemPoolChildren().erase(*child)) {
- cachedInnerUsage -= memusage::IncrementalDynamicUsage(s);
- }
-}
-
-void CTxMemPool::UpdateParent(txiter entry, txiter parent, bool add)
-{
- AssertLockHeld(cs);
- CTxMemPoolEntry::Parents s;
- if (add && entry->GetMemPoolParents().insert(*parent).second) {
- cachedInnerUsage += memusage::IncrementalDynamicUsage(s);
- } else if (!add && entry->GetMemPoolParents().erase(*parent)) {
- cachedInnerUsage -= memusage::IncrementalDynamicUsage(s);
- }
-}
-
CFeeRate CTxMemPool::GetMinFee(size_t sizelimit) const {
LOCK(cs);
if (!blockSinceLastRollingFeeBump || rollingMinimumFeeRate == 0)
@@ -900,7 +827,6 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpends
for (auto ref : worst_chunk) {
stage.insert(mapTx.iterator_to(static_cast<const CTxMemPoolEntry&>(*ref)));
}
- UpdateForRemoveFromMempool(stage, false);
for (auto e : stage) {
removeUnchecked(e, MemPoolRemovalReason::SIZELIMIT);
}
diff --git a/src/txmempool.h b/src/txmempool.h
index 0c0ec241..1891c8d4 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -296,9 +296,6 @@ private:
typedef std::map<txiter, setEntries, CompareIteratorByHash> cacheMap;
- void UpdateParent(txiter entry, txiter parent, bool add) EXCLUSIVE_LOCKS_REQUIRED(cs);
- void UpdateChild(txiter entry, txiter child, bool add) EXCLUSIVE_LOCKS_REQUIRED(cs);
-
std::vector<indexed_transaction_set::const_iterator> GetSortedScoreWithTopology() const EXCLUSIVE_LOCKS_REQUIRED(cs);
/**
@@ -584,15 +581,6 @@ private:
*/
void RemoveStaged(setEntries& stage, bool updateDescendants, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
- /** Update ancestors of hash to add/remove it as a descendant transaction. */
- void UpdateAncestorsOf(bool add, txiter hash) EXCLUSIVE_LOCKS_REQUIRED(cs);
- /** For each transaction being removed, update ancestors and any direct children.
- * If updateDescendants is true, then also update in-mempool descendants'
- * ancestor state. */
- void UpdateForRemoveFromMempool(const setEntries &entriesToRemove, bool updateDescendants) EXCLUSIVE_LOCKS_REQUIRED(cs);
- /** Sever link between specified transaction and direct children. */
- void UpdateChildrenForRemoval(txiter entry) EXCLUSIVE_LOCKS_REQUIRED(cs);
-
/** Before calling removeUnchecked for a given transaction,
* UpdateForRemoveFromMempool must be called on the entire (dependent) set
* of transactions being removed at the same time. We use each
Why this scored 12/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.