From 416294a78e632a4c0a07ee58ee3dd09c807cd853 Mon Sep 17 00:00:00 2001 From: pasta Date: Sat, 8 Aug 2026 14:23:02 -0500 Subject: [PATCH] fix: bound DMN list/diff caches to stop getmnlistd memory exhaustion CDeterministicMNManager's in-memory caches (mnListsCache, mnListDiffsCache) are only trimmed by CleanupCache(), which runs when a new block arrives. Between blocks there is no bound: GETMNLISTDIFF accepts an arbitrary historical baseBlockHash and GetListForBlock appends a cache entry per requested block, so an unauthenticated peer requesting many distinct historical blocks drives cache growth without ceiling (a full mainnet MN list is several MB). Bound admission through new CacheMNList()/CacheMNListDiff() helpers: entries older than the recency window CleanupCache would drop anyway (height + LIST_DIFFS_CACHE_SIZE < tip) are not retained at all, and hard caps evict the lowest-height entries in a single pass (std::nth_element), never the tip snapshot. MAX_CACHE_LISTS = DISK_SNAPSHOT_PERIOD * 2: lists are rebuilt by applying up to DISK_SNAPSHOT_PERIOD - 1 diffs from the previous on-disk snapshot, so validation/invalidation spanning a snapshot boundary can keep two snapshot periods of lists resident without eviction thrash. MAX_CACHE_DIFFS = LIST_DIFFS_CACHE_SIZE + 64. The rebuild walk in GetListForBlockInternal() admits every diff it reads unconditionally so the apply loop can resolve every walked hash; the diff cap is enforced once after the walk completes, so eviction can never drop a diff the walk still needs. Add mn_lists_cache_bounded regression test: drive GetListForBlock over more distinct historical heights than the cap without running cleanup, assert both caches stay bounded, and prove eviction never changes a returned list by re-querying entries guaranteed to have been evicted. --- src/evo/deterministicmns.cpp | 108 ++++++++++++++++++++++-- src/evo/deterministicmns.h | 36 ++++++++ src/test/evo_deterministicmns_tests.cpp | 81 ++++++++++++++++++ 3 files changed, 217 insertions(+), 8 deletions(-) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 0eed499ba976..f7a9d630f219 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -24,6 +24,7 @@ #include +#include #include #include #include @@ -662,7 +663,7 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_nullnHeight; - mnListDiffsCache.emplace(pindex->GetBlockHash(), diff); - mnListsCache.emplace(newList.GetBlockHash(), newList); + CacheMNListDiff(pindex->GetBlockHash(), diff); + CacheMNList(newList.GetBlockHash(), newList); } catch (const std::exception& e) { LogPrintf("CDeterministicMNManager::%s -- internal error: %s\n", __func__, e.what()); return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "failed-dmn-block"); @@ -765,6 +766,88 @@ void CDeterministicMNManager::UpdatedBlockTip(gsl::not_null tipIndex = pindex; } +bool CDeterministicMNManager::ShouldRetainCacheHeight(int height) +{ + AssertLockHeld(cs); + // Before tip is known, retain freely (early startup / first connect). + if (!tipIndex) return true; + // Same recency window CleanupCache uses for the "too old" drop predicate. + return height + LIST_DIFFS_CACHE_SIZE >= tipIndex->nHeight; +} + +void CDeterministicMNManager::EnforceListsCacheLimit() +{ + AssertLockHeld(cs); + if (mnListsCache.size() <= MAX_CACHE_LISTS) { + return; + } + // Evict the lowest-height entries, but never the tip snapshot. Single pass: + // partition the candidate iterators so the excess-many lowest heights come + // first, then erase exactly those. + std::vector candidates; + candidates.reserve(mnListsCache.size()); + for (auto it = mnListsCache.begin(); it != mnListsCache.end(); ++it) { + if (tipIndex != nullptr && it->first == tipIndex->GetBlockHash()) { + continue; + } + candidates.emplace_back(it); + } + const size_t excess = std::min(mnListsCache.size() - MAX_CACHE_LISTS, candidates.size()); + if (excess == 0) { + return; + } + std::nth_element(candidates.begin(), candidates.begin() + (excess - 1), candidates.end(), + [](const auto& a, const auto& b) { return a->second.GetHeight() < b->second.GetHeight(); }); + for (size_t i = 0; i < excess; ++i) { + mnListsCache.erase(candidates[i]); + } +} + +void CDeterministicMNManager::EnforceDiffsCacheLimit() +{ + AssertLockHeld(cs); + if (mnListDiffsCache.size() <= MAX_CACHE_DIFFS) { + return; + } + const size_t excess = mnListDiffsCache.size() - MAX_CACHE_DIFFS; + std::vector candidates; + candidates.reserve(mnListDiffsCache.size()); + for (auto it = mnListDiffsCache.begin(); it != mnListDiffsCache.end(); ++it) { + candidates.emplace_back(it); + } + std::nth_element(candidates.begin(), candidates.begin() + (excess - 1), candidates.end(), + [](const auto& a, const auto& b) { return a->second.nHeight < b->second.nHeight; }); + for (size_t i = 0; i < excess; ++i) { + mnListDiffsCache.erase(candidates[i]); + } +} + +void CDeterministicMNManager::CacheMNList(const uint256& block_hash, const CDeterministicMNList& list) +{ + AssertLockHeld(cs); + if (!ShouldRetainCacheHeight(list.GetHeight())) { + return; + } + // Prefer emplace over assign: CDeterministicMNList::operator= locks m_cached_sml_mutex + // and must not run while cs is held (lock-order checker). + const auto [_, inserted] = mnListsCache.emplace(block_hash, list); + if (inserted) { + EnforceListsCacheLimit(); + } +} + +void CDeterministicMNManager::CacheMNListDiff(const uint256& block_hash, CDeterministicMNListDiff diff) +{ + AssertLockHeld(cs); + if (!ShouldRetainCacheHeight(diff.nHeight)) { + return; + } + const auto [_, inserted] = mnListDiffsCache.emplace(block_hash, std::move(diff)); + if (inserted) { + EnforceDiffsCacheLimit(); + } +} + CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(gsl::not_null pindex) { CDeterministicMNList snapshot; @@ -786,7 +869,8 @@ CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(gsl::not_n } if (m_evoDb.Read(std::make_pair(DB_LIST_SNAPSHOT, pindex->GetBlockHash()), snapshot)) { - mnListsCache.emplace(pindex->GetBlockHash(), snapshot); + // Use the list; only retain it in the cache if it is tip-recent. + CacheMNList(pindex->GetBlockHash(), snapshot); break; } @@ -820,13 +904,17 @@ CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(gsl::not_n // no snapshot and no diff on disk means that it's the initial snapshot m_initial_snapshot_index = pindex; snapshot = CDeterministicMNList(pindex->GetBlockHash(), pindex->nHeight, 0); - mnListsCache.emplace(pindex->GetBlockHash(), snapshot); + CacheMNList(pindex->GetBlockHash(), snapshot); LogPrintf("CDeterministicMNManager::%s -- initial snapshot. blockHash=%s nHeight=%d\n", __func__, snapshot.GetBlockHash().ToString(), snapshot.GetHeight()); break; } diff.nHeight = pindex->nHeight; + // Cache for this rebuild pass even if older than the retention window, so that + // the apply loop below can resolve every walked hash via mnListDiffsCache. The + // hard bound is enforced once after the apply loop, so eviction can never drop + // a diff this walk still needs. mnListDiffsCache.emplace(pindex->GetBlockHash(), std::move(diff)); listDiffIndexes.emplace_front(pindex); pindex = pindex->pprev; @@ -850,14 +938,14 @@ CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(gsl::not_n // There is also separate in-memory caching for the current tip and active quorums, // but this mini-snapshot cache specifically speeds up repeated requests // for nearby historical blocks. - mnListsCache.emplace(snapshot.GetBlockHash(), snapshot); + CacheMNList(snapshot.GetBlockHash(), snapshot); } } if (tipIndex) { // always keep a snapshot for the tip if (const auto snapshot_hash = snapshot.GetBlockHash(); snapshot_hash == tipIndex->GetBlockHash()) { - mnListsCache.emplace(snapshot_hash, snapshot); + CacheMNList(snapshot_hash, snapshot); } else { // keep snapshots for yet alive quorums if (std::ranges::any_of(Params().GetConsensus().llmqs, [&snapshot, this]( @@ -867,11 +955,15 @@ CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(gsl::not_n (snapshot.GetHeight() + params.dkgInterval * (params.keepOldConnections + 1) >= tipIndex->nHeight); })) { - mnListsCache.emplace(snapshot_hash, snapshot); + CacheMNList(snapshot_hash, snapshot); } } } + // The rebuild walk above admits every diff it reads unconditionally, so that the + // apply loop can resolve them. Enforce the hard bound now that the walk is done. + EnforceDiffsCacheLimit(); + assert(snapshot.GetHeight() != -1); return snapshot; } diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h index b4a886f39eed..c372cc21a46f 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -742,6 +742,24 @@ class CDeterministicMNManager static constexpr int DISK_SNAPSHOTS = llmq_max_blocks() / DISK_SNAPSHOT_PERIOD + 1; static constexpr int LIST_DIFFS_CACHE_SIZE = DISK_SNAPSHOT_PERIOD * DISK_SNAPSHOTS; +public: + // Hard caps on the in-memory caches. CleanupCache() alone is not enough: it only + // runs once a new block has arrived, so between blocks an unauthenticated peer + // spamming getmnlistd for historical blocks could append entries without bound + // (a full mainnet list is several MB). Admission is bounded two ways: entries + // older than the window CleanupCache would drop anyway are not retained at all, + // and these caps evict the lowest-height entries when exceeded. + // Lists are rebuilt by applying up to DISK_SNAPSHOT_PERIOD - 1 diffs from the + // previous on-disk snapshot, so block validation / invalidation spanning a + // snapshot boundary can legitimately keep up to two snapshot periods of lists + // (per-block lists plus mini-snapshots) resident. Size the cap to hold that + // whole window so bounding admission never slows the (dis)connect hot path; + // it is well above honest steady-state usage (tip + live quorum bases + + // mini-snapshots within LIST_DIFFS_CACHE_SIZE of the tip). + static constexpr size_t MAX_CACHE_LISTS = static_cast(DISK_SNAPSHOT_PERIOD) * 2; + // Diffs are small; allow a full recency window plus a margin for one rebuild walk. + static constexpr size_t MAX_CACHE_DIFFS = static_cast(LIST_DIFFS_CACHE_SIZE) + 64; + private: Mutex cs; Mutex cs_cleanup; @@ -779,6 +797,18 @@ class CDeterministicMNManager }; CDeterministicMNList GetListAtChainTip() EXCLUSIVE_LOCKS_REQUIRED(!cs); + // In-memory list/diff cache sizes (for tests and diagnostics). + size_t GetListCacheSize() EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + LOCK(cs); + return mnListsCache.size(); + } + size_t GetListDiffsCacheSize() EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + LOCK(cs); + return mnListDiffsCache.size(); + } + // Test if given TX is a ProRegTx which also contains the collateral at index n static bool IsProTxWithCollateral(const CTransactionRef& tx, uint32_t n); @@ -818,6 +848,12 @@ class CDeterministicMNManager private: void CleanupCache(int nHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); CDeterministicMNList GetListForBlockInternal(gsl::not_null pindex) EXCLUSIVE_LOCKS_REQUIRED(cs); + // Retain only tip-recent heights (same window CleanupCache uses for "too old"). + [[nodiscard]] bool ShouldRetainCacheHeight(int height) EXCLUSIVE_LOCKS_REQUIRED(cs); + void CacheMNList(const uint256& block_hash, const CDeterministicMNList& list) EXCLUSIVE_LOCKS_REQUIRED(cs); + void CacheMNListDiff(const uint256& block_hash, CDeterministicMNListDiff diff) EXCLUSIVE_LOCKS_REQUIRED(cs); + void EnforceListsCacheLimit() EXCLUSIVE_LOCKS_REQUIRED(cs); + void EnforceDiffsCacheLimit() EXCLUSIVE_LOCKS_REQUIRED(cs); // Helper methods for RecalculateAndRepairDiffs static std::vector CollectSnapshotBlocks(const CBlockIndex* start_index, diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 8ca40a2d2909..650b88ea7539 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -3088,6 +3088,87 @@ BOOST_AUTO_TEST_CASE(field_bit_migration_validation) BOOST_CHECK_EQUAL(usedBits.size(), 19); } +// Unauthenticated getmnlistd can force arbitrary historical MN lists +// into mnListsCache. Between CleanupCache runs the map was append-only, so N +// distinct heights produced N retained full lists. Bound retention at insert. +BOOST_AUTO_TEST_CASE(mn_lists_cache_bounded) +{ + TestChainDIP3Setup setup; + auto& dmnman = *Assert(setup.m_node.dmnman); + auto& chainman = *Assert(setup.m_node.chainman.get()); + const CScript coinbase_pk = GetScriptForRawPubKey(setup.coinbaseKey.GetPubKey()); + auto tip_index = [&] { return WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip()); }; + + dmnman.UpdatedBlockTip(tip_index()); + dmnman.DoMaintenance(); + + // Mine more than the hard cap without running cleanup — mirrors the + // attacker window between blocks when getmnlistd populates the cache. + constexpr size_t n_blocks = CDeterministicMNManager::MAX_CACHE_LISTS + 64; + for (size_t i = 0; i < n_blocks; ++i) { + setup.CreateAndProcessBlock({}, coinbase_pk); + dmnman.UpdatedBlockTip(tip_index()); + } + + // Record the expected list for a spread of historical heights, and for every + // height in the lowest 64 of the range. Eviction is lowest-height-first, so + // after the descending sweep below (which touches MAX_CACHE_LISTS newer + // heights after each of these), the lowest-64 entries are guaranteed to have + // been evicted; re-querying them proves eviction never changes what is + // returned. + const CBlockIndex* tip = tip_index(); + BOOST_REQUIRE(tip != nullptr); + const int lowest_height = tip->nHeight - static_cast(n_blocks) + 1; + std::vector> expected; + for (int h = tip->nHeight; h >= 0 && h > tip->nHeight - static_cast(n_blocks); h -= 37) { + const CBlockIndex* pindex = tip->GetAncestor(h); + BOOST_REQUIRE(pindex != nullptr); + expected.emplace_back(pindex, dmnman.GetListForBlock(pindex)); + } + for (int h = lowest_height; h < lowest_height + 64; ++h) { + BOOST_REQUIRE(h >= 0); + const CBlockIndex* pindex = tip->GetAncestor(h); + BOOST_REQUIRE(pindex != nullptr); + expected.emplace_back(pindex, dmnman.GetListForBlock(pindex)); + } + BOOST_REQUIRE(expected.size() > 64); + + // Now exercise GetListForBlock over every distinct historical height — the + // getmnlistd / BuildSimplifiedMNListDiff path an unauthenticated peer drives. + for (int h = tip->nHeight; h >= 0 && h > tip->nHeight - static_cast(n_blocks); --h) { + const CBlockIndex* pindex = tip->GetAncestor(h); + BOOST_REQUIRE(pindex != nullptr); + (void)dmnman.GetListForBlock(pindex); + } + + // Pre-fix: each ProcessBlock / historical load appends freely → size > MAX. + // Post-fix: insert-time retention + hard cap keep the cache bounded. + // (The diffs cache stays far below its cap here — its bound only bites on + // walks that read stale diffs back from disk — so only the lists cap is + // driven past its limit by this test.) + const size_t list_cache_size = dmnman.GetListCacheSize(); + BOOST_TEST_MESSAGE("mnListsCache size after sweep: " << list_cache_size); + BOOST_CHECK_MESSAGE(list_cache_size <= CDeterministicMNManager::MAX_CACHE_LISTS, + strprintf("mnListsCache size %zu exceeds hard cap %zu", list_cache_size, + CDeterministicMNManager::MAX_CACHE_LISTS)); + BOOST_CHECK_LE(dmnman.GetListDiffsCacheSize(), CDeterministicMNManager::MAX_CACHE_DIFFS); + + // The cache is pure memoisation: bounding it must not change any result. The + // lowest-64 entries recorded above have certainly been evicted by now, so + // they are recomputed from disk here — the values must still match what the + // cache returned above. + for (const auto& [pindex, want] : expected) { + const auto got = dmnman.GetListForBlock(pindex); + BOOST_CHECK_MESSAGE(got == want, + strprintf("GetListForBlock(%d) differs after cache eviction", pindex->nHeight)); + } + + // Cleanup must still drop everything outside the recency window, and must not + // resurrect unbounded growth. + dmnman.DoMaintenance(); + BOOST_CHECK_LE(dmnman.GetListCacheSize(), CDeterministicMNManager::MAX_CACHE_LISTS); +} + BOOST_AUTO_TEST_CASE(migration_logic_validation) { // Test the database migration logic for nVersion-first format conversion.