diff --git a/doc/release-notes-7485.md b/doc/release-notes-7485.md new file mode 100644 index 000000000000..a9d13a78773d --- /dev/null +++ b/doc/release-notes-7485.md @@ -0,0 +1,8 @@ +P2P and network +--------------- + +- Bound in-memory masternode list caches so unauthenticated historical + `GETMNLISTDIFF` requests can no longer grow memory without limit between + blocks. Recent lists are capped by height-aware eviction; stale historical + mini-snapshots are kept in a small LRU cache so repeated requests do not + re-read large on-disk snapshots on every call. (#7485) diff --git a/src/evo/deterministicmns.cpp b/src/evo/deterministicmns.cpp index d652efaec732..a3938c34fa30 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"); @@ -741,6 +742,7 @@ bool CDeterministicMNManager::UndoBlock(gsl::not_null pindex mnListsCache.erase(blockHash); mnListDiffsCache.erase(blockHash); + EraseStaleList(blockHash); } if (diff.HasChanges()) { CDeterministicMNList curList{prevList}; @@ -765,6 +767,129 @@ 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())) { + CacheStaleMNList(block_hash, list); + 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::CacheStaleMNList(const uint256& block_hash, const CDeterministicMNList& list) +{ + AssertLockHeld(cs); + if (auto it = mnStaleListsCache.find(block_hash); it != mnStaleListsCache.end()) { + mnStaleListsLru.splice(mnStaleListsLru.begin(), mnStaleListsLru, it->second.lru_it); + return; + } + mnStaleListsLru.push_front(block_hash); + mnStaleListsCache.emplace(block_hash, StaleListEntry{list, mnStaleListsLru.begin()}); + while (mnStaleListsCache.size() > MAX_STALE_CACHE_LISTS) { + const uint256& evict_hash = mnStaleListsLru.back(); + if (auto evict_it = mnStaleListsCache.find(evict_hash); evict_it != mnStaleListsCache.end()) { + mnStaleListsCache.erase(evict_it); + } + mnStaleListsLru.pop_back(); + } +} + +std::optional CDeterministicMNManager::GetStaleList(const uint256& block_hash) +{ + AssertLockHeld(cs); + const auto it = mnStaleListsCache.find(block_hash); + if (it == mnStaleListsCache.end()) { + return std::nullopt; + } + mnStaleListsLru.splice(mnStaleListsLru.begin(), mnStaleListsLru, it->second.lru_it); + return it->second.list; +} + +void CDeterministicMNManager::EraseStaleList(const uint256& block_hash) +{ + AssertLockHeld(cs); + const auto it = mnStaleListsCache.find(block_hash); + if (it == mnStaleListsCache.end()) { + return; + } + mnStaleListsLru.erase(it->second.lru_it); + mnStaleListsCache.erase(it); +} + +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; @@ -785,8 +910,14 @@ CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(gsl::not_n break; } + if (auto stale = GetStaleList(pindex->GetBlockHash())) { + snapshot = std::move(*stale); + break; + } + 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; } @@ -813,6 +944,10 @@ CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(gsl::not_n // the current DIP3 activation height (e.g. the functional-test // cached chain) bootstraps an empty list here and rebuilds via // ProcessBlock from that point on. + // This throw is the one exception exit a peer can reach, and it + // skips the post-walk cap enforcement below — re-bound the diffs + // this walk admitted before unwinding. + EnforceDiffsCacheLimit(); throw BlockDataUnavailableError(strprintf( "CDeterministicMNManager::%s -- masternode list diff for block %s %s", __func__, pindex->GetBlockHash().ToString(), BLOCK_DATA_UNAVAILABLE_SUFFIX)); @@ -820,13 +955,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 +989,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 +1006,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; } @@ -1466,6 +1609,7 @@ void CDeterministicMNManager::WriteRepairedDiffs( for (const auto& [block_hash, diff] : recalculated_diffs) { mnListDiffsCache.erase(block_hash); mnListsCache.erase(block_hash); + EraseStaleList(block_hash); } LogPrintf("CDeterministicMNManager::%s -- Successfully repaired %d diffs (caches cleared)\n", __func__, diff --git a/src/evo/deterministicmns.h b/src/evo/deterministicmns.h index 4fb5dee91aef..faac57f15353 100644 --- a/src/evo/deterministicmns.h +++ b/src/evo/deterministicmns.h @@ -24,7 +24,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -752,7 +754,34 @@ 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). List admission uses two tiers: + // - Recent tier (mnListsCache): tip-recent heights only, lowest-height-first + // eviction, cap MAX_CACHE_LISTS. Attacker stale traffic never enters here. + // - Stale tier (mnStaleListsCache): LRU, cap MAX_STALE_CACHE_LISTS. Keeps a + // bounded working set of historical mini-snapshots so repeated stale + // getmnlistd requests do not re-read a multi-MB disk snapshot on every call. + // 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 recent 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; + // One 576-block snapshot interval of mini-snapshots (18) plus margin. + static constexpr size_t MAX_STALE_CACHE_LISTS = 32; + // 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: + struct StaleListEntry { + CDeterministicMNList list; + std::list::iterator lru_it{}; + }; Mutex cs; Mutex cs_cleanup; // We have performed CleanupCache() on this height. @@ -766,6 +795,8 @@ class CDeterministicMNManager Uint256HashMap mnListsCache GUARDED_BY(cs); Uint256HashMap mnListDiffsCache GUARDED_BY(cs); + std::list mnStaleListsLru GUARDED_BY(cs); + Uint256HashMap mnStaleListsCache GUARDED_BY(cs); const CBlockIndex* tipIndex GUARDED_BY(cs) {nullptr}; const CBlockIndex* m_initial_snapshot_index GUARDED_BY(cs) {nullptr}; @@ -795,6 +826,23 @@ class CDeterministicMNManager mnListsCache.insert_or_assign(list.GetBlockHash(), list); } + // 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(); + } + size_t GetStaleListCacheSize() EXCLUSIVE_LOCKS_REQUIRED(!cs) + { + LOCK(cs); + return mnStaleListsCache.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); @@ -834,6 +882,15 @@ 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 CacheStaleMNList(const uint256& block_hash, const CDeterministicMNList& list) EXCLUSIVE_LOCKS_REQUIRED(cs); + [[nodiscard]] std::optional GetStaleList(const uint256& block_hash) EXCLUSIVE_LOCKS_REQUIRED(cs); + void EraseStaleList(const uint256& block_hash) 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 6e9339bd0c7a..a4d703ef6072 100644 --- a/src/test/evo_deterministicmns_tests.cpp +++ b/src/test/evo_deterministicmns_tests.cpp @@ -3234,6 +3234,105 @@ 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 past the recency window and diff cap without running cleanup — mirrors + // the attacker window between blocks when getmnlistd populates the cache. + constexpr size_t recency_window = CDeterministicMNManager::MAX_CACHE_DIFFS - 64; + constexpr size_t n_blocks = recency_window + (40 * 32); + 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 in + // the recent tier; stale heights land in the LRU stale tier instead. + 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); + + // 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); + } + + const size_t list_cache_size = dmnman.GetListCacheSize(); + const size_t diff_cache_size = dmnman.GetListDiffsCacheSize(); + const size_t stale_cache_size = dmnman.GetStaleListCacheSize(); + BOOST_TEST_MESSAGE("mnListsCache size after sweep: " << list_cache_size); + BOOST_TEST_MESSAGE("mnListDiffsCache size after sweep: " << diff_cache_size); + BOOST_TEST_MESSAGE("mnStaleListsCache size after sweep: " << stale_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(diff_cache_size, CDeterministicMNManager::MAX_CACHE_DIFFS); + // n_blocks ProcessBlock admissions exceed MAX_CACHE_DIFFS; the cap must have trimmed. + BOOST_CHECK_MESSAGE(diff_cache_size >= CDeterministicMNManager::MAX_CACHE_DIFFS - 64, + strprintf("mnListDiffsCache size %zu never approached cap %zu", diff_cache_size, + CDeterministicMNManager::MAX_CACHE_DIFFS)); + + // Stale heights from the sweep must have been routed to the LRU stale tier. + BOOST_CHECK_GT(stale_cache_size, 0U); + BOOST_CHECK_LE(stale_cache_size, CDeterministicMNManager::MAX_STALE_CACHE_LISTS); + + // Repeat access to one stale interval must be cache-served, not a full re-walk. + const int stale_target_height = tip->nHeight - static_cast(recency_window) - 100; + BOOST_REQUIRE(stale_target_height >= 0); + const CBlockIndex* stale_pindex = tip->GetAncestor(stale_target_height); + BOOST_REQUIRE(stale_pindex != nullptr); + const auto stale_first = dmnman.GetListForBlock(stale_pindex); + const size_t stale_size_after_first = dmnman.GetStaleListCacheSize(); + const CBlockIndex* stale_neighbor = tip->GetAncestor(stale_target_height + 16); + BOOST_REQUIRE(stale_neighbor != nullptr); + const auto stale_neighbor_result = dmnman.GetListForBlock(stale_neighbor); + const auto stale_second = dmnman.GetListForBlock(stale_pindex); + BOOST_CHECK(stale_first == stale_second); + BOOST_CHECK(stale_neighbor_result == dmnman.GetListForBlock(stale_neighbor)); + BOOST_CHECK_LE(dmnman.GetStaleListCacheSize(), stale_size_after_first + 1); + + // The cache is pure memoisation: bounding it must not change any result. + 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.