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

@@ -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;

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) {