What changed, and why it matters
This commit is a routine internal refactoring in Bitcoin Core's memory pool (mempool) code. It replaces one way of calculating transaction ancestors with a newer shared component called txgraph. The visible behavior for users and network peers should stay the same. There is no indication this fixes a security bug or introduces a vulnerability.
No security action required. Treat as normal code maintenance and review for functional correctness during regular QA.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change removes CTxMemPool::CalculateAncestors and reimplements CalculateMemPoolAncestors on top of m_txgraph->GetAncestors(TxGraph::Level::MAIN). It also removes the fSearchForParents parameter, so callers in RBF and RPC now always use the single-argument form. The new implementation first asks txgraph for ancestors; if the entry is not in the graph it falls back to iterating vin, finding mempool parents, and collecting each parent’s ancestors. The comment is updated to note the returned set no longer includes the transaction itself.
Changed components
src/policy/rbf.cppsrc/rpc/mempool.cppsrc/txmempool.cppsrc/txmempool.hInspect captured patch +34 / −64
diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp
index fdf6f53c..c78fed8e 100644
--- a/src/policy/rbf.cpp
+++ b/src/policy/rbf.cpp
@@ -39,7 +39,7 @@ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool)
// If all the inputs have nSequence >= maxint-1, it still might be
// signaled for RBF if any unconfirmed parents have signaled.
const auto& entry{*Assert(pool.GetEntry(tx.GetHash()))};
- auto ancestors{pool.CalculateMemPoolAncestors(entry, /*fSearchForParents=*/false)};
+ auto ancestors{pool.CalculateMemPoolAncestors(entry)};
for (CTxMemPool::txiter it : ancestors) {
if (SignalsOptInRBF(it->GetTx())) {
diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
index 01870ef3..7a016335 100644
--- a/src/rpc/mempool.cpp
+++ b/src/rpc/mempool.cpp
@@ -474,7 +474,7 @@ static RPCHelpMan getmempoolancestors()
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool");
}
- auto ancestors{mempool.CalculateMemPoolAncestors(*entry, /*fSearchForParents=*/false)};
+ auto ancestors{mempool.CalculateMemPoolAncestors(*entry)};
if (!fVerbose) {
UniValue o(UniValue::VARR);
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index f83b4544..2b8bec25 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -99,31 +99,6 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector<Txid>& vHashesToU
}
}
-CTxMemPool::setEntries CTxMemPool::CalculateAncestors(CTxMemPoolEntry::Parents& staged_ancestors) const
-{
- setEntries ancestors;
-
- while (!staged_ancestors.empty()) {
- const CTxMemPoolEntry& stage = staged_ancestors.begin()->get();
- txiter stageit = mapTx.iterator_to(stage);
-
- ancestors.insert(stageit);
- staged_ancestors.erase(stage);
-
- const CTxMemPoolEntry::Parents& parents = stageit->GetMemPoolParentsConst();
- for (const CTxMemPoolEntry& parent : parents) {
- txiter parent_it = mapTx.iterator_to(parent);
-
- // If this is a new ancestor, add it.
- if (ancestors.count(parent_it) == 0) {
- staged_ancestors.insert(parent);
- }
- }
- }
-
- return ancestors;
-}
-
util::Result<void> CTxMemPool::CheckPackageLimits(const Package& package,
const int64_t total_vsize) const
{
@@ -138,31 +113,42 @@ bool CTxMemPool::HasDescendants(const Txid& txid) const
return m_txgraph->GetDescendants(*entry, TxGraph::Level::MAIN).size() > 1;
}
-CTxMemPool::setEntries CTxMemPool::CalculateMemPoolAncestors(
- const CTxMemPoolEntry &entry,
- bool fSearchForParents /* = true */) const
+CTxMemPool::setEntries CTxMemPool::CalculateMemPoolAncestors(const CTxMemPoolEntry &entry) const
{
- CTxMemPoolEntry::Parents staged_ancestors;
+ auto ancestors = m_txgraph->GetAncestors(entry, TxGraph::Level::MAIN);
+ setEntries ret;
+ if (ancestors.size() > 0) {
+ for (auto ancestor : ancestors) {
+ if (ancestor != &entry) {
+ ret.insert(mapTx.iterator_to(static_cast<const CTxMemPoolEntry&>(*ancestor)));
+ }
+ }
+ return ret;
+ }
+
+ // If we didn't get anything back, the transaction is not in the graph.
+ // Find each parent and call GetAncestors on each.
+ setEntries staged_parents;
const CTransaction &tx = entry.GetTx();
- if (fSearchForParents) {
- // 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) {
- staged_ancestors.insert(**piter);
- }
+ // 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) {
+ staged_parents.insert(*piter);
+ }
+ }
+
+ for (const auto& parent : staged_parents) {
+ auto parent_ancestors = m_txgraph->GetAncestors(*parent, TxGraph::Level::MAIN);
+ for (auto ancestor : parent_ancestors) {
+ ret.insert(mapTx.iterator_to(static_cast<const CTxMemPoolEntry&>(*ancestor)));
}
- } else {
- // If we're not searching for parents, we require this to already be an
- // entry in the mempool and use the entry's cached parents.
- txiter it = mapTx.iterator_to(entry);
- staged_ancestors = it->GetMemPoolParentsConst();
}
- return CalculateAncestors(staged_ancestors);
+ return ret;
}
void CTxMemPool::UpdateAncestorsOf(bool add, txiter it)
diff --git a/src/txmempool.h b/src/txmempool.h
index d2b7f232..3c488215 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -304,17 +304,6 @@ private:
*/
std::set<Txid> m_unbroadcast_txids GUARDED_BY(cs);
-
- /**
- * Helper function to calculate all in-mempool ancestors of staged_ancestors
- *
- * @param[in] staged_ancestors Should contain entries in the mempool.
- *
- * @return all in-mempool ancestors
- */
- setEntries CalculateAncestors(CTxMemPoolEntry::Parents &staged_ancestors)
- const EXCLUSIVE_LOCKS_REQUIRED(cs);
-
static TxMempoolInfo GetInfo(CTxMemPool::indexed_transaction_set::const_iterator it)
{
return TxMempoolInfo{it->GetSharedTx(), it->GetTime(), it->GetFee(), it->GetTxSize(), it->GetModifiedFee() - it->GetFee()};
@@ -429,18 +418,13 @@ public:
}
/**
- * Calculate all in-mempool ancestors of entry.
- * (these are all calculated including the tx itself)
+ * Calculate all in-mempool ancestors of entry (not including the tx itself)
*
* @param[in] entry CTxMemPoolEntry of which all in-mempool ancestors are calculated
- * @param[in] fSearchForParents Whether to search a tx's vin for in-mempool parents, or look
- * up parents from mapLinks. Must be true for entries not in
- * the mempool
*
* @return all in-mempool ancestors
*/
- setEntries CalculateMemPoolAncestors(const CTxMemPoolEntry& entry,
- bool fSearchForParents = true) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+ setEntries CalculateMemPoolAncestors(const CTxMemPoolEntry& entry) const EXCLUSIVE_LOCKS_REQUIRED(cs);
bool HasDescendants(const Txid& txid) const;
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.