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 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* pvNoSpends unsigned nTxnRemoved = 0; CFeeRate maxFeeRateRemoved(0); + while (!mapTx.empty() && DynamicMemoryUsage() > sizelimit) { - indexed_transaction_set::index::type::iterator it = mapTx.get().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 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(*ref).GetTx()); + } + } + + setEntries stage; + for (auto ref : worst_chunk) { + stage.insert(mapTx.iterator_to(static_cast(*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) {