From 1ad4590f63855e856d59616d41a87873315c3a2e Mon Sep 17 00:00:00 2001
From: Suhas Daftuar <sdaftuar@chaincode.com>
Date: Wed, 20 Sep 2023 20:55:50 -0400
Subject: [PATCH] Limit mempool size based on chunk feerate

Rather than evicting the transactions with the lowest descendant feerate,
instead evict transactions that have the lowest chunk feerate.

Once mining is implemented based on choosing transactions with highest chunk
feerate (see next commit), mining and eviction will be opposites, so that we
will evict the transactions that would be mined last.
---
 src/test/mempool_tests.cpp | 18 ++++++++++++++----
 src/txmempool.cpp          | 27 ++++++++++++++++++---------
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index baee31e82e8..ec3bbd158fe 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -533,27 +533,37 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
     tx7.vout[1].nValue = 10 * COIN;
 
     AddToMempool(pool, entry.Fee(700LL).FromTx(tx4));
+    auto usage_with_tx4_only = pool.DynamicMemoryUsage();
     AddToMempool(pool, entry.Fee(100LL).FromTx(tx5));
     AddToMempool(pool, entry.Fee(110LL).FromTx(tx6));
     AddToMempool(pool, entry.Fee(900LL).FromTx(tx7));
 
-    // we only require this to remove, at max, 2 txn, because it's not clear what we're really optimizing for aside from that
+    // From the topology above, tx7 must be sorted last, so it should
+    // definitely evicted first if we must trim. tx4 should definitely remain
+    // in the mempool since it has a higher feerate than its descendants and
+    // should be in its own chunk.
     pool.TrimToSize(pool.DynamicMemoryUsage() - 1);
     BOOST_CHECK(pool.exists(tx4.GetHash()));
-    BOOST_CHECK(pool.exists(tx6.GetHash()));
     BOOST_CHECK(!pool.exists(tx7.GetHash()));
 
+    // Tx5 and Tx6 may be removed as well because they're in the same chunk as
+    // tx7, but this behavior need not be guaranteed.
+
     if (!pool.exists(tx5.GetHash()))
         AddToMempool(pool, entry.Fee(100LL).FromTx(tx5));
+    if (!pool.exists(tx6.GetHash()))
+        AddToMempool(pool, entry.Fee(110LL).FromTx(tx6));
     AddToMempool(pool, entry.Fee(900LL).FromTx(tx7));
 
-    pool.TrimToSize(pool.DynamicMemoryUsage() * 0.75); // should maximize mempool size by only removing 5/7
+    // If we trim sufficiently, everything but tx4 should be removed.
+    pool.TrimToSize(usage_with_tx4_only + 1);
     BOOST_CHECK(pool.exists(tx4.GetHash()));
     BOOST_CHECK(!pool.exists(tx5.GetHash()));
-    BOOST_CHECK(pool.exists(tx6.GetHash()));
+    BOOST_CHECK(!pool.exists(tx6.GetHash()));
     BOOST_CHECK(!pool.exists(tx7.GetHash()));
 
     AddToMempool(pool, entry.Fee(100LL).FromTx(tx5));
+    AddToMempool(pool, entry.Fee(110LL).FromTx(tx6));
     AddToMempool(pool, entry.Fee(900LL).FromTx(tx7));
 
     std::vector<CTransactionRef> vtx;
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index e5a318524dd..672198ec2ea 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -1167,29 +1167,38 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector<COutPoint>* pvNoSpends
 
     unsigned nTxnRemoved = 0;
     CFeeRate maxFeeRateRemoved(0);
+
     while (!mapTx.empty() && DynamicMemoryUsage() > sizelimit) {
-        indexed_transaction_set::index<descendant_score>::type::iterator it = mapTx.get<descendant_score>().begin();
+        const auto &[worst_chunk, feeperweight] = m_txgraph->GetWorstMainChunk();
+        FeePerVSize feerate = ToFeePerVSize(feeperweight);
+        CFeeRate removed{feerate.fee, feerate.size};
 
         // We set the new mempool min fee to the feerate of the removed set, plus the
         // "minimum reasonable fee rate" (ie some value under which we consider txn
         // to have 0 fee). This way, we don't allow txn to enter mempool with feerate
         // equal to txn which were removed with no block in between.
-        CFeeRate removed(it->GetModFeesWithDescendants(), it->GetSizeWithDescendants());
         removed += m_opts.incremental_relay_feerate;
         trackPackageRemoved(removed);
         maxFeeRateRemoved = std::max(maxFeeRateRemoved, removed);
 
-        setEntries stage;
-        CalculateDescendants(mapTx.project<0>(it), stage);
-        nTxnRemoved += stage.size();
+        nTxnRemoved += worst_chunk.size();
 
         std::vector<CTransaction> txn;
         if (pvNoSpendsRemaining) {
-            txn.reserve(stage.size());
-            for (txiter iter : stage)
-                txn.push_back(iter->GetTx());
+            txn.reserve(worst_chunk.size());
+            for (auto ref : worst_chunk) {
+                txn.emplace_back(static_cast<const CTxMemPoolEntry&>(*ref).GetTx());
+            }
+        }
+
+        setEntries stage;
+        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);
         }
-        RemoveStaged(stage, false, MemPoolRemovalReason::SIZELIMIT);
         if (pvNoSpendsRemaining) {
             for (const CTransaction& tx : txn) {
                 for (const CTxIn& txin : tx.vin) {