diff --git a/src/main.cpp b/src/main.cpp index 9d881f42a..74187645a 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -414,14 +414,14 @@ namespace { void LimitMempoolSize(CTxMemPool& pool, size_t limit, unsigned long age) { - /* int expired = pool.Expire(GetTime() - age); - if (expired != 0) - LogPrint("mempool", "Expired %i transactions from the memory pool\n", expired); + int expired = pool.Expire(GetTime() - age); + if (expired != 0) + LogPrint("mempool", "Expired %i transactions from the memory pool\n", expired); - std::vector vNoSpendsRemaining; - pool.TrimToSize(limit, &vNoSpendsRemaining); - BOOST_FOREACH(const uint256& removed, vNoSpendsRemaining) - pcoinsTip->Uncache(removed);*/ + // Fee-order trim to the size limit. (Upstream also pcoinsTip->Uncache()s the coins freed + // by eviction, but CCoinsViewCache has no Uncache() in this fork -- it is only a UTXO-cache + // perf hint, not eviction correctness, so it is skipped.) + pool.TrimToSize(limit); } // Requires cs_main. @@ -2072,12 +2072,6 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa { LOCK(pool.cs); - // Bound mempool memory: this fork never ported fee-ordered TrimToSize eviction, so - // instead of evicting we refuse new admissions once the pool exceeds -maxmempool. - // removeExpired() already clears unmineable expired txs on each block connect; this - // caps the total footprint against a flood of otherwise-minable/low-fee txs (OOM DoS). - if ( pool.DynamicMemoryUsage() > (size_t)GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000 ) - return state.DoS(0, error("AcceptToMemoryPool: mempool full, rejecting tx %s", hash.ToString()), REJECT_INSUFFICIENTFEE, "mempool-full"); // Store transaction in memory pool.addUnchecked(hash, entry, !IsInitialBlockDownload()); @@ -2090,6 +2084,17 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa if (fSpentIndex) { pool.addSpentIndex(entry, view); } + + // Bound mempool memory with fee-ordered eviction. Now that the tx is in, if the pool + // exceeds -maxmempool, TrimToSize drops the lowest-feerate txs -- so a higher-fee tx + // DISPLACES lower-fee ones instead of being bluntly rejected. If this very tx was the one + // evicted (its feerate was the lowest in the pool), it does not belong here -- reject it. + size_t maxmempool = (size_t)GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000; + if ( pool.DynamicMemoryUsage() > maxmempool ) { + pool.TrimToSize(maxmempool); + if ( !pool.exists(hash) ) + return state.DoS(0, error("AcceptToMemoryPool: mempool full, tx %s evicted (feerate too low)", hash.ToString()), REJECT_INSUFFICIENTFEE, "mempool-full"); + } } } return true; @@ -4070,6 +4075,10 @@ bool static ConnectTip(CValidationState &state, CBlockIndex *pindexNew, CBlock * uiInterface.NotifyTxExpiration(id); } + // Bound mempool memory on each block: age-expire (-mempoolexpiry) then fee-order trim to + // -maxmempool, evicting the lowest-feerate txs (+ descendants) and uncaching their coins. + LimitMempoolSize(mempool, GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60); + // Update chainActive & related variables. UpdateTip(pindexNew); diff --git a/src/main.h b/src/main.h index 6ecef2f77..ee57a18be 100644 --- a/src/main.h +++ b/src/main.h @@ -65,7 +65,7 @@ class CValidationState; class PrecomputedTransactionData; struct CNodeStateStats; -#define DEFAULT_MEMPOOL_EXPIRY 1 +#define DEFAULT_MEMPOOL_EXPIRY 72 // hours; age-based Expire is now live via LimitMempoolSize -- was 1, too aggressive /** Default for -maxmempool, maximum megabytes of mempool memory usage */ #define DEFAULT_MAX_MEMPOOL_SIZE 300 #define _COINBASE_MATURITY 100 diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 53b971809..6b5d00103 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -491,6 +491,45 @@ std::vector CTxMemPool::removeExpired(unsigned int nBlockHeight) return ids; } +// Age-based eviction: remove txs whose entry time is older than `time`. Distinct from +// removeExpired() (which drops txs past their consensus nExpiryHeight); this is the wall-clock +// sweep LimitMempoolSize wants. Collect-then-remove to avoid iterating mapTx while mutating it. +int CTxMemPool::Expire(int64_t time) +{ + LOCK(cs); + std::list toRemove; + for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { + if (it->GetTime() < time) + toRemove.push_back(it->GetTx()); + } + for (const CTransaction& tx : toRemove) { + std::list removed; + remove(tx, removed, true); + } + return (int)toRemove.size(); +} + +// Fee-ordered eviction: drop the lowest-feerate txs (and their in-mempool descendants, via the +// recursive remove) until DynamicMemoryUsage() is at or below sizelimit. Uses the per-tx feerate +// index (mapTx index 1, sorted feerate DESCENDING, so the worst tx is the tail). NOTE: this is a +// per-tx feerate, not a descendant-aggregate score, so a low-fee parent funded by a high-fee child +// (CPFP) can be evicted -- an accepted simplification (no descendant tracking in this fork). The +// admission cap in AcceptToMemoryPool bounds growth between block connects (no rollingMinFee here). +void CTxMemPool::TrimToSize(size_t sizelimit, std::vector* pvNoSpendsRemaining) +{ + LOCK(cs); + while (DynamicMemoryUsage() > sizelimit && !mapTx.empty()) { + // Re-derive the tail each iteration: remove() invalidates iterators. + CTransaction tx = std::prev(mapTx.get<1>().end())->GetTx(); + std::list removed; + remove(tx, removed, true); + if (pvNoSpendsRemaining) { + for (const CTransaction& r : removed) + pvNoSpendsRemaining->push_back(r.GetHash()); + } + } +} + // Called when a block is connected. Removes from mempool and updates the miner fee estimator. void CTxMemPool::removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, std::list& conflicts, bool fCurrentEstimate) diff --git a/src/txmempool.h b/src/txmempool.h index 72acde9a3..0410a2e76 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -219,6 +219,8 @@ public: void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags); void removeConflicts(const CTransaction &tx, std::list& removed); std::vector removeExpired(unsigned int nBlockHeight); + int Expire(int64_t time); + void TrimToSize(size_t sizelimit, std::vector* pvNoSpendsRemaining = NULL); void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, std::list& conflicts, bool fCurrentEstimate = true); void removeWithoutBranchId(uint32_t nMemPoolBranchId);