mempool: eliminate accessors to mempool entry ancestor/descendant cached state
What the developer wrote
Authored by Suhas Daftuar
mempool: eliminate accessors to mempool entry ancestor/descendant cached state
What changed, and why it matters
This commit removes several read-only getter methods from Bitcoin Core's memory pool (mempool) transaction entry class and deletes the corresponding consistency checks in the mempool's internal validation routine. It is a code cleanup/refactoring change: the cached ancestor and descendant statistics are no longer exposed through public accessors, and the debug-only `check()` function no longer independently recomputes those statistics to verify them. There is no change to network behavior, consensus rules, or how transactions are accepted.
No security action required. Treat as normal code-maintenance review; verify that downstream callers no longer need the removed accessors and that the remaining mempool invariant checks are sufficient for the project's testing strategy.
Security signals we found
No security-relevant signals present in the diff
Change is purely a code-cleanup/refactoring of internal mempool APIs
Removed assertions are inside an existing debug/consistency check routine, not runtime policy enforcement
Evidence from the diff
The patch deletes eight accessor methods from CTxMemPoolEntry (GetCountWithDescendants, GetSizeWithDescendants, GetModFeesWithDescendants, GetCountWithAncestors, GetSizeWithAncestors, GetModFeesWithAncestors, GetSigOpCostWithAncestors) and removes the assertions in CTxMemPool::check() that recomputed and compared ancestor/descendant counts, sizes, fees, and sigop costs. It also simplifies the child-set construction loop by dropping the running child_sizes sum and the related descendant-size sanity assertion. The remaining cached fields themselves are not removed from the class; only their public accessors and some debug verification logic are eliminated.
Changed components
src/kernel/mempool_entry.hsrc/txmempool.cppInspect captured patch +1 / −33
diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h
index 28c2290c..2e61f102 100644
--- a/src/kernel/mempool_entry.h
+++ b/src/kernel/mempool_entry.h
@@ -166,17 +166,8 @@ public:
lockPoints = lp;
}
- uint64_t GetCountWithDescendants() const { return m_count_with_descendants; }
- int64_t GetSizeWithDescendants() const { return nSizeWithDescendants; }
- CAmount GetModFeesWithDescendants() const { return nModFeesWithDescendants; }
-
bool GetSpendsCoinbase() const { return spendsCoinbase; }
- uint64_t GetCountWithAncestors() const { return m_count_with_ancestors; }
- int64_t GetSizeWithAncestors() const { return nSizeWithAncestors; }
- CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; }
- int64_t GetSigOpCostWithAncestors() const { return nSigOpCostWithAncestors; }
-
const Parents& GetMemPoolParentsConst() const { return m_parents; }
const Children& GetMemPoolChildrenConst() const { return m_children; }
Parents& GetMemPoolParents() const { return m_parents; }
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 7c3be6e1..28a6418b 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -720,40 +720,17 @@ void CTxMemPool::check(const CCoinsViewCache& active_coins_tip, int64_t spendhei
};
assert(setParentCheck.size() == it->GetMemPoolParentsConst().size());
assert(std::equal(setParentCheck.begin(), setParentCheck.end(), it->GetMemPoolParentsConst().begin(), comp));
- // Verify ancestor state is correct.
- auto ancestors{AssumeCalculateMemPoolAncestors(__func__, *it, Limits::NoLimits())};
- uint64_t nCountCheck = ancestors.size() + 1;
- int32_t nSizeCheck = it->GetTxSize();
- CAmount nFeesCheck = it->GetModifiedFee();
- int64_t nSigOpCheck = it->GetSigOpCost();
-
- for (txiter ancestorIt : ancestors) {
- nSizeCheck += ancestorIt->GetTxSize();
- nFeesCheck += ancestorIt->GetModifiedFee();
- nSigOpCheck += ancestorIt->GetSigOpCost();
- }
-
- assert(it->GetCountWithAncestors() == nCountCheck);
- assert(it->GetSizeWithAncestors() == nSizeCheck);
- assert(it->GetSigOpCostWithAncestors() == nSigOpCheck);
- assert(it->GetModFeesWithAncestors() == nFeesCheck);
// Check children against mapNextTx
CTxMemPoolEntry::Children setChildrenCheck;
auto iter = mapNextTx.lower_bound(COutPoint(it->GetTx().GetHash(), 0));
- int32_t child_sizes{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
- if (setChildrenCheck.insert(*childit).second) {
- child_sizes += childit->GetTxSize();
- }
+ setChildrenCheck.insert(*childit);
}
assert(setChildrenCheck.size() == it->GetMemPoolChildrenConst().size());
assert(std::equal(setChildrenCheck.begin(), setChildrenCheck.end(), it->GetMemPoolChildrenConst().begin(), comp));
- // Also check to make sure size is greater than sum with immediate children.
- // just a sanity check, not definitive that this calc is correct...
- assert(it->GetSizeWithDescendants() >= child_sizes + it->GetTxSize());
TxValidationState dummy_state; // Not used. CheckTxInputs() should always pass
CAmount txfee = 0;
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.