diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index 20db7a1f08ec..772f642ca0a2 100644 --- a/src/evo/deterministicmns.cpp +++ b/src/evo/deterministicmns.cpp @@ -664,14 +664,14 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, gsl::not_nullpprev == m_initial_snapshot_index) { m_evoDb.Write(std::make_pair(DB_LIST_SNAPSHOT, newList.GetBlockHash()), newList); - mnListsCache.emplace(newList.GetBlockHash(), newList); + CacheMNList(newList.GetBlockHash(), newList); LogPrintf("CDeterministicMNManager::%s -- Wrote snapshot. nHeight=%d, mapCurMNs.allMNsCount=%d\n", __func__, nHeight, newList.GetCounts().total()); } diff.nHeight = pindex->nHeight; - 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"); @@ -750,6 +750,80 @@ 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); + while (mnListsCache.size() > MAX_CACHE_LISTS) { + // Evict the oldest-height entry that is not the tip snapshot. + auto victim = mnListsCache.end(); + for (auto it = mnListsCache.begin(); it != mnListsCache.end(); ++it) { + if (tipIndex != nullptr && it->first == tipIndex->GetBlockHash()) { + continue; + } + if (victim == mnListsCache.end() || it->second.GetHeight() < victim->second.GetHeight()) { + victim = it; + } + } + if (victim == mnListsCache.end()) { + // Only the tip remains; nothing further to drop. + break; + } + mnListsCache.erase(victim); + } +} + +void CDeterministicMNManager::EnforceDiffsCacheLimit() +{ + AssertLockHeld(cs); + while (mnListDiffsCache.size() > MAX_CACHE_DIFFS) { + auto victim = mnListDiffsCache.end(); + for (auto it = mnListDiffsCache.begin(); it != mnListDiffsCache.end(); ++it) { + if (victim == mnListDiffsCache.end() || it->second.nHeight < victim->second.nHeight) { + victim = it; + } + } + if (victim == mnListDiffsCache.end()) { + break; + } + mnListDiffsCache.erase(victim); + } +} + +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; @@ -771,7 +845,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; } @@ -788,13 +863,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; @@ -818,14 +897,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]( @@ -835,11 +914,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 3a684ffa78df..4b681adce2db 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -683,6 +683,19 @@ 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 oldest-height entry when exceeded. + // MAX_CACHE_LISTS is sized 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 = 256; + // 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; @@ -720,6 +733,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); @@ -759,6 +784,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 std::vector CollectSnapshotBlocks(const CBlockIndex* start_index, const CBlockIndex* stop_index, diff --git a/src/test/evo_deterministicmns_tests.cpp b/src/test/evo_deterministicmns_tests.cpp index 24ace3bc7469..e6baa02e0323 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -1680,6 +1680,72 @@ 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 while they are + // still cached, so we can prove eviction does not change what is returned. + const CBlockIndex* tip = tip_index(); + BOOST_REQUIRE(tip != nullptr); + 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)); + } + BOOST_REQUIRE(expected.size() > 1); + + // 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. + 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. Some + // of these entries 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.