Remove the ancestor and descendant indices from the mempool
What changed, and why it matters
This commit removes two internal sorting indexes from Bitcoin Core's memory pool (mempool) data structure. It is a code cleanup/refactoring change: the ancestor_score and descendant_score indexes are no longer needed because their jobs have been taken over by a newer transaction graph module (m_txgraph). The commit also deletes the unit tests that exercised those old indexes and adjusts a memory-usage estimate to reflect fewer indexes. There is no indication in the commit or supplied references that this fixes a security bug.
No security action required. Treat as routine maintenance/refactoring. Reviewers may want to confirm that the m_txgraph implementation fully replaces the removed ancestor/descendant ordering semantics, but that is a functional correctness concern, not a security issue.
Security signals we found
No security-relevant signals present in the diff or commit metadata.
Change is purely a refactoring/cleanup of internal data structures and tests.
No bounds checks, input validation, cryptographic operations, or network parsing code is modified.
Evidence from the diff
The patch deletes CompareTxMemPoolEntryByDescendantScore, CompareTxMemPoolEntryByAncestorFee, and their associated multi_index tags (descendant_score, ancestor_score) from CTxMemPool’s indexed_transaction_set. It removes ~320 lines of mempool indexing unit tests from src/test/mempool_tests.cpp and updates DynamicMemoryUsage() to estimate 9 pointers of overhead per entry instead of 15. The change is consistent with prior mempool graph refactoring where m_txgraph now provides ancestor/descendant feerate ordering for mining selection and eviction.
Changed components
src/txmempool.hsrc/txmempool.cppsrc/test/mempool_tests.cppInspect captured patch +2 / −395
diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index ec3bbd15..2271e7b8 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -115,322 +115,6 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
BOOST_CHECK_EQUAL(testPool.size(), 0U);
}
-template <typename name>
-static void CheckSort(CTxMemPool& pool, std::vector<std::string>& sortedOrder) EXCLUSIVE_LOCKS_REQUIRED(pool.cs)
-{
- BOOST_CHECK_EQUAL(pool.size(), sortedOrder.size());
- typename CTxMemPool::indexed_transaction_set::index<name>::type::iterator it = pool.mapTx.get<name>().begin();
- int count = 0;
- for (; it != pool.mapTx.get<name>().end(); ++it, ++count) {
- BOOST_CHECK_EQUAL(it->GetTx().GetHash().ToString(), sortedOrder[count]);
- }
-}
-
-BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
-{
- CTxMemPool& pool = *Assert(m_node.mempool);
- LOCK2(cs_main, pool.cs);
- TestMemPoolEntryHelper entry;
-
- /* 3rd highest fee */
- CMutableTransaction tx1 = CMutableTransaction();
- tx1.vout.resize(1);
- tx1.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx1.vout[0].nValue = 10 * COIN;
- AddToMempool(pool, entry.Fee(10000LL).FromTx(tx1));
-
- /* highest fee */
- CMutableTransaction tx2 = CMutableTransaction();
- tx2.vout.resize(1);
- tx2.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx2.vout[0].nValue = 2 * COIN;
- AddToMempool(pool, entry.Fee(20000LL).FromTx(tx2));
-
- /* lowest fee */
- CMutableTransaction tx3 = CMutableTransaction();
- tx3.vout.resize(1);
- tx3.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx3.vout[0].nValue = 5 * COIN;
- AddToMempool(pool, entry.Fee(0LL).FromTx(tx3));
-
- /* 2nd highest fee */
- CMutableTransaction tx4 = CMutableTransaction();
- tx4.vout.resize(1);
- tx4.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx4.vout[0].nValue = 6 * COIN;
- AddToMempool(pool, entry.Fee(15000LL).FromTx(tx4));
-
- /* equal fee rate to tx1, but newer */
- CMutableTransaction tx5 = CMutableTransaction();
- tx5.vout.resize(1);
- tx5.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx5.vout[0].nValue = 11 * COIN;
- entry.time = NodeSeconds{1s};
- AddToMempool(pool, entry.Fee(10000LL).FromTx(tx5));
- BOOST_CHECK_EQUAL(pool.size(), 5U);
-
- std::vector<std::string> sortedOrder;
- sortedOrder.resize(5);
- sortedOrder[0] = tx3.GetHash().ToString(); // 0
- sortedOrder[1] = tx5.GetHash().ToString(); // 10000
- sortedOrder[2] = tx1.GetHash().ToString(); // 10000
- sortedOrder[3] = tx4.GetHash().ToString(); // 15000
- sortedOrder[4] = tx2.GetHash().ToString(); // 20000
- CheckSort<descendant_score>(pool, sortedOrder);
-
- /* low fee but with high fee child */
- /* tx6 -> tx7 -> tx8, tx9 -> tx10 */
- CMutableTransaction tx6 = CMutableTransaction();
- tx6.vout.resize(1);
- tx6.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx6.vout[0].nValue = 20 * COIN;
- AddToMempool(pool, entry.Fee(0LL).FromTx(tx6));
- BOOST_CHECK_EQUAL(pool.size(), 6U);
- // Check that at this point, tx6 is sorted low
- sortedOrder.insert(sortedOrder.begin(), tx6.GetHash().ToString());
- CheckSort<descendant_score>(pool, sortedOrder);
-
- CTxMemPool::setEntries setAncestors;
- setAncestors.insert(pool.GetIter(tx6.GetHash()).value());
- CMutableTransaction tx7 = CMutableTransaction();
- tx7.vin.resize(1);
- tx7.vin[0].prevout = COutPoint(tx6.GetHash(), 0);
- tx7.vin[0].scriptSig = CScript() << OP_11;
- tx7.vout.resize(2);
- tx7.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx7.vout[0].nValue = 10 * COIN;
- tx7.vout[1].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx7.vout[1].nValue = 1 * COIN;
-
- {
- auto ancestors_calculated{pool.CalculateMemPoolAncestors(entry.Fee(2000000LL).FromTx(tx7), CTxMemPool::Limits::NoLimits())};
- BOOST_REQUIRE(ancestors_calculated.has_value());
- BOOST_CHECK(*ancestors_calculated == setAncestors);
- }
-
- AddToMempool(pool, entry.FromTx(tx7));
- BOOST_CHECK_EQUAL(pool.size(), 7U);
-
- // Now tx6 should be sorted higher (high fee child): tx7, tx6, tx2, ...
- sortedOrder.erase(sortedOrder.begin());
- sortedOrder.push_back(tx6.GetHash().ToString());
- sortedOrder.push_back(tx7.GetHash().ToString());
- CheckSort<descendant_score>(pool, sortedOrder);
-
- /* low fee child of tx7 */
- CMutableTransaction tx8 = CMutableTransaction();
- tx8.vin.resize(1);
- tx8.vin[0].prevout = COutPoint(tx7.GetHash(), 0);
- tx8.vin[0].scriptSig = CScript() << OP_11;
- tx8.vout.resize(1);
- tx8.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx8.vout[0].nValue = 10 * COIN;
- setAncestors.insert(pool.GetIter(tx7.GetHash()).value());
- AddToMempool(pool, entry.Fee(0LL).Time(NodeSeconds{2s}).FromTx(tx8));
-
- // Now tx8 should be sorted low, but tx6/tx both high
- sortedOrder.insert(sortedOrder.begin(), tx8.GetHash().ToString());
- CheckSort<descendant_score>(pool, sortedOrder);
-
- /* low fee child of tx7 */
- CMutableTransaction tx9 = CMutableTransaction();
- tx9.vin.resize(1);
- tx9.vin[0].prevout = COutPoint(tx7.GetHash(), 1);
- tx9.vin[0].scriptSig = CScript() << OP_11;
- tx9.vout.resize(1);
- tx9.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx9.vout[0].nValue = 1 * COIN;
- AddToMempool(pool, entry.Fee(0LL).Time(NodeSeconds{3s}).FromTx(tx9));
-
- // tx9 should be sorted low
- BOOST_CHECK_EQUAL(pool.size(), 9U);
- sortedOrder.insert(sortedOrder.begin(), tx9.GetHash().ToString());
- CheckSort<descendant_score>(pool, sortedOrder);
-
- std::vector<std::string> snapshotOrder = sortedOrder;
-
- setAncestors.insert(pool.GetIter(tx8.GetHash()).value());
- setAncestors.insert(pool.GetIter(tx9.GetHash()).value());
- /* tx10 depends on tx8 and tx9 and has a high fee*/
- CMutableTransaction tx10 = CMutableTransaction();
- tx10.vin.resize(2);
- tx10.vin[0].prevout = COutPoint(tx8.GetHash(), 0);
- tx10.vin[0].scriptSig = CScript() << OP_11;
- tx10.vin[1].prevout = COutPoint(tx9.GetHash(), 0);
- tx10.vin[1].scriptSig = CScript() << OP_11;
- tx10.vout.resize(1);
- tx10.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx10.vout[0].nValue = 10 * COIN;
-
- {
- auto ancestors_calculated{pool.CalculateMemPoolAncestors(entry.Fee(200000LL).Time(NodeSeconds{4s}).FromTx(tx10), CTxMemPool::Limits::NoLimits())};
- BOOST_REQUIRE(ancestors_calculated);
- BOOST_CHECK(*ancestors_calculated == setAncestors);
- }
-
- AddToMempool(pool, entry.FromTx(tx10));
-
- /**
- * tx8 and tx9 should both now be sorted higher
- * Final order after tx10 is added:
- *
- * tx3 = 0 (1)
- * tx5 = 10000 (1)
- * tx1 = 10000 (1)
- * tx4 = 15000 (1)
- * tx2 = 20000 (1)
- * tx9 = 200k (2 txs)
- * tx8 = 200k (2 txs)
- * tx10 = 200k (1 tx)
- * tx6 = 2.2M (5 txs)
- * tx7 = 2.2M (4 txs)
- */
- sortedOrder.erase(sortedOrder.begin(), sortedOrder.begin()+2); // take out tx9, tx8 from the beginning
- sortedOrder.insert(sortedOrder.begin()+5, tx9.GetHash().ToString());
- sortedOrder.insert(sortedOrder.begin()+6, tx8.GetHash().ToString());
- sortedOrder.insert(sortedOrder.begin()+7, tx10.GetHash().ToString()); // tx10 is just before tx6
- CheckSort<descendant_score>(pool, sortedOrder);
-
- // there should be 10 transactions in the mempool
- BOOST_CHECK_EQUAL(pool.size(), 10U);
-
- // Now try removing tx10 and verify the sort order returns to normal
- pool.removeRecursive(*Assert(pool.get(tx10.GetHash())), REMOVAL_REASON_DUMMY);
- CheckSort<descendant_score>(pool, snapshotOrder);
-
- pool.removeRecursive(*Assert(pool.get(tx9.GetHash())), REMOVAL_REASON_DUMMY);
- pool.removeRecursive(*Assert(pool.get(tx8.GetHash())), REMOVAL_REASON_DUMMY);
-}
-
-BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
-{
- CTxMemPool& pool = *Assert(m_node.mempool);
- LOCK2(cs_main, pool.cs);
- TestMemPoolEntryHelper entry;
-
- /* 3rd highest fee */
- CMutableTransaction tx1 = CMutableTransaction();
- tx1.vout.resize(1);
- tx1.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx1.vout[0].nValue = 10 * COIN;
- AddToMempool(pool, entry.Fee(10000LL).FromTx(tx1));
-
- /* highest fee */
- CMutableTransaction tx2 = CMutableTransaction();
- tx2.vout.resize(1);
- tx2.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx2.vout[0].nValue = 2 * COIN;
- AddToMempool(pool, entry.Fee(20000LL).FromTx(tx2));
- uint64_t tx2Size = GetVirtualTransactionSize(CTransaction(tx2));
-
- /* lowest fee */
- CMutableTransaction tx3 = CMutableTransaction();
- tx3.vout.resize(1);
- tx3.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx3.vout[0].nValue = 5 * COIN;
- AddToMempool(pool, entry.Fee(0LL).FromTx(tx3));
-
- /* 2nd highest fee */
- CMutableTransaction tx4 = CMutableTransaction();
- tx4.vout.resize(1);
- tx4.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx4.vout[0].nValue = 6 * COIN;
- AddToMempool(pool, entry.Fee(15000LL).FromTx(tx4));
-
- /* equal fee rate to tx1, but newer */
- CMutableTransaction tx5 = CMutableTransaction();
- tx5.vout.resize(1);
- tx5.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx5.vout[0].nValue = 11 * COIN;
- AddToMempool(pool, entry.Fee(10000LL).FromTx(tx5));
- BOOST_CHECK_EQUAL(pool.size(), 5U);
-
- std::vector<std::string> sortedOrder;
- sortedOrder.resize(5);
- sortedOrder[0] = tx2.GetHash().ToString(); // 20000
- sortedOrder[1] = tx4.GetHash().ToString(); // 15000
- // tx1 and tx5 are both 10000
- // Ties are broken by hash, not timestamp, so determine which
- // hash comes first.
- if (tx1.GetHash() < tx5.GetHash()) {
- sortedOrder[2] = tx1.GetHash().ToString();
- sortedOrder[3] = tx5.GetHash().ToString();
- } else {
- sortedOrder[2] = tx5.GetHash().ToString();
- sortedOrder[3] = tx1.GetHash().ToString();
- }
- sortedOrder[4] = tx3.GetHash().ToString(); // 0
-
- CheckSort<ancestor_score>(pool, sortedOrder);
-
- /* low fee parent with high fee child */
- /* tx6 (0) -> tx7 (high) */
- CMutableTransaction tx6 = CMutableTransaction();
- tx6.vout.resize(1);
- tx6.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx6.vout[0].nValue = 20 * COIN;
- uint64_t tx6Size = GetVirtualTransactionSize(CTransaction(tx6));
-
- AddToMempool(pool, entry.Fee(0LL).FromTx(tx6));
- BOOST_CHECK_EQUAL(pool.size(), 6U);
- // Ties are broken by hash
- if (tx3.GetHash() < tx6.GetHash())
- sortedOrder.push_back(tx6.GetHash().ToString());
- else
- sortedOrder.insert(sortedOrder.end()-1,tx6.GetHash().ToString());
-
- CheckSort<ancestor_score>(pool, sortedOrder);
-
- CMutableTransaction tx7 = CMutableTransaction();
- tx7.vin.resize(1);
- tx7.vin[0].prevout = COutPoint(tx6.GetHash(), 0);
- tx7.vin[0].scriptSig = CScript() << OP_11;
- tx7.vout.resize(1);
- tx7.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx7.vout[0].nValue = 10 * COIN;
- uint64_t tx7Size = GetVirtualTransactionSize(CTransaction(tx7));
-
- /* set the fee to just below tx2's feerate when including ancestor */
- CAmount fee = (20000/tx2Size)*(tx7Size + tx6Size) - 1;
-
- AddToMempool(pool, entry.Fee(fee).FromTx(tx7));
- BOOST_CHECK_EQUAL(pool.size(), 7U);
- sortedOrder.insert(sortedOrder.begin()+1, tx7.GetHash().ToString());
- CheckSort<ancestor_score>(pool, sortedOrder);
-
- /* after tx6 is mined, tx7 should move up in the sort */
- std::vector<CTransactionRef> vtx;
- vtx.push_back(MakeTransactionRef(tx6));
- pool.removeForBlock(vtx, 1);
-
- sortedOrder.erase(sortedOrder.begin()+1);
- // Ties are broken by hash
- if (tx3.GetHash() < tx6.GetHash())
- sortedOrder.pop_back();
- else
- sortedOrder.erase(sortedOrder.end()-2);
- sortedOrder.insert(sortedOrder.begin(), tx7.GetHash().ToString());
- CheckSort<ancestor_score>(pool, sortedOrder);
-
- // High-fee parent, low-fee child
- // tx7 -> tx8
- CMutableTransaction tx8 = CMutableTransaction();
- tx8.vin.resize(1);
- tx8.vin[0].prevout = COutPoint(tx7.GetHash(), 0);
- tx8.vin[0].scriptSig = CScript() << OP_11;
- tx8.vout.resize(1);
- tx8.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
- tx8.vout[0].nValue = 10*COIN;
-
- // Check that we sort by min(feerate, ancestor_feerate):
- // set the fee so that the ancestor feerate is above tx1/5,
- // but the transaction's own feerate is lower
- AddToMempool(pool, entry.Fee(5000LL).FromTx(tx8));
- sortedOrder.insert(sortedOrder.end()-1, tx8.GetHash().ToString());
- CheckSort<ancestor_score>(pool, sortedOrder);
-}
-
-
BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
{
auto& pool = static_cast<MemPoolTest&>(*Assert(m_node.mempool));
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 15b25081..f2a63c26 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -1057,8 +1057,8 @@ void CCoinsViewMemPool::Reset()
size_t CTxMemPool::DynamicMemoryUsage() const {
LOCK(cs);
- // Estimate the overhead of mapTx to be 15 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented.
- return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(txns_randomized) + m_txgraph->GetMainMemoryUsage() + cachedInnerUsage;
+ // Estimate the overhead of mapTx to be 9 pointers (3 pointers per index) + an allocation, as no exact formula for boost::multi_index_contained is implemented.
+ return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 9 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(txns_randomized) + m_txgraph->GetMainMemoryUsage() + cachedInnerUsage;
}
void CTxMemPool::RemoveUnbroadcastTx(const Txid& txid, const bool unchecked) {
diff --git a/src/txmempool.h b/src/txmempool.h
index 1750596c..5834cd00 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -90,37 +90,6 @@ struct mempoolentry_wtxid
}
};
-
-/** \class CompareTxMemPoolEntryByDescendantScore
- *
- * Sort an entry by max(score/size of entry's tx, score/size with all descendants).
- */
-class CompareTxMemPoolEntryByDescendantScore
-{
-public:
- bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const
- {
- FeeFrac f1 = GetModFeeAndSize(a);
- FeeFrac f2 = GetModFeeAndSize(b);
-
- if (FeeRateCompare(f1, f2) == 0) {
- return a.GetTime() >= b.GetTime();
- }
- return f1 < f2;
- }
-
- // Return the fee/size we're using for sorting this entry.
- FeeFrac GetModFeeAndSize(const CTxMemPoolEntry &a) const
- {
- // Compare feerate with descendants to feerate of the transaction, and
- // return the fee/size for the max.
- return std::max<FeeFrac>(
- FeeFrac(a.GetModFeesWithDescendants(), a.GetSizeWithDescendants()),
- FeeFrac(a.GetModifiedFee(), a.GetTxSize())
- );
- }
-};
-
/** \class CompareTxMemPoolEntryByScore
*
* Sort by feerate of entry (fee/size) in descending order
@@ -151,42 +120,8 @@ public:
}
};
-/** \class CompareTxMemPoolEntryByAncestorScore
- *
- * Sort an entry by min(score/size of entry's tx, score/size with all ancestors).
- */
-class CompareTxMemPoolEntryByAncestorFee
-{
-public:
- template<typename T>
- bool operator()(const T& a, const T& b) const
- {
- FeeFrac f1 = GetModFeeAndSize(a);
- FeeFrac f2 = GetModFeeAndSize(b);
-
- if (FeeRateCompare(f1, f2) == 0) {
- return a.GetTx().GetHash() < b.GetTx().GetHash();
- }
- return f1 > f2;
- }
-
- // Return the fee/size we're using for sorting this entry.
- template <typename T>
- FeeFrac GetModFeeAndSize(const T &a) const
- {
- // Compare feerate with ancestors to feerate of the transaction, and
- // return the fee/size for the min.
- return std::min<FeeFrac>(
- FeeFrac(a.GetModFeesWithAncestors(), a.GetSizeWithAncestors()),
- FeeFrac(a.GetModifiedFee(), a.GetTxSize())
- );
- }
-};
-
// Multi_index tag names
-struct descendant_score {};
struct entry_time {};
-struct ancestor_score {};
struct index_by_wtxid {};
/**
@@ -321,23 +256,11 @@ public:
mempoolentry_wtxid,
SaltedWtxidHasher
>,
- // sorted by fee rate
- boost::multi_index::ordered_non_unique<
- boost::multi_index::tag<descendant_score>,
- boost::multi_index::identity<CTxMemPoolEntry>,
- CompareTxMemPoolEntryByDescendantScore
- >,
// sorted by entry time
boost::multi_index::ordered_non_unique<
boost::multi_index::tag<entry_time>,
boost::multi_index::identity<CTxMemPoolEntry>,
CompareTxMemPoolEntryByEntryTime
- >,
- // sorted by fee rate with ancestors
- boost::multi_index::ordered_non_unique<
- boost::multi_index::tag<ancestor_score>,
- boost::multi_index::identity<CTxMemPoolEntry>,
- CompareTxMemPoolEntryByAncestorFee
>
>
{};
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.