From b08eab3f2e957a77c768a8fc256aad01555603b6 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 11 Nov 2025 22:16:32 -0600 Subject: [PATCH 1/4] refactor(cache): migrate unordered_lru_cache::get to return std::optional Change unordered_lru_cache::get() from returning bool with an output parameter to returning std::optional by value. This provides a more modern API that is easier to use and less error-prone. - Add header to unordered_lru_cache.h - Update get() signature: bool get(key, Value&) -> std::optional get(key) - Update all 21 call sites across 11 files to use the new API - Maintain same copy semantics for performance (no performance impact) Files updated: - src/unordered_lru_cache.h - src/llmq/quorums.cpp (5 call sites) - src/llmq/utils.cpp (2 call sites) - src/llmq/snapshot.cpp, signing.cpp, dkgsessionmgr.cpp, blockprocessor.cpp - src/instantsend/db.cpp (3 call sites) - src/evo/mnhftx.cpp, creditpool.cpp, cbtx.cpp - src/masternode/meta.cpp --- src/evo/cbtx.cpp | 4 +++- src/evo/creditpool.cpp | 8 ++++---- src/evo/mnhftx.cpp | 4 ++-- src/instantsend/db.cpp | 31 ++++++++++++++++++------------- src/llmq/blockprocessor.cpp | 7 ++++--- src/llmq/dkgsessionmgr.cpp | 4 +++- src/llmq/quorums.cpp | 24 ++++++++++++++---------- src/llmq/signing.cpp | 21 +++++++++------------ src/llmq/snapshot.cpp | 4 ++-- src/llmq/utils.cpp | 7 ++++--- src/masternode/meta.cpp | 8 +++----- src/unordered_lru_cache.h | 8 ++++---- 12 files changed, 70 insertions(+), 60 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 8954ba5f63cc..2b527453f5ac 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -86,7 +86,9 @@ auto CachedGetQcHashesQcIndexedHashes(const CBlockIndex* pindexPrev, const llmq: uint256 block_hash{blockIndex->GetBlockHash()}; std::pair qc_hash; - if (!qc_hashes_cached[llmqType].get(block_hash, qc_hash)) { + if (auto cached = qc_hashes_cached[llmqType].get(block_hash)) { + qc_hash = *cached; + } else { auto [pqc, dummy_hash] = quorum_block_processor.GetMinedCommitment(llmqType, block_hash); if (dummy_hash == uint256::ZERO) { // this should never happen diff --git a/src/evo/creditpool.cpp b/src/evo/creditpool.cpp index 7a82653b4bc5..c553c96d0bc9 100644 --- a/src/evo/creditpool.cpp +++ b/src/evo/creditpool.cpp @@ -70,8 +70,8 @@ static std::optional GetCreditDataFromBlock(const gsl::n static Mutex cache_mutex; static Uint256LruHashMap block_data_cache GUARDED_BY(cache_mutex){ static_cast(Params().CreditPoolPeriodBlocks()) * 2}; - if (LOCK(cache_mutex); block_data_cache.get(block_index->GetBlockHash(), blockData)) { - return blockData; + if (LOCK(cache_mutex); auto cached = block_data_cache.get(block_index->GetBlockHash())) { + return *cached; } CBlock block; @@ -123,8 +123,8 @@ std::optional CCreditPoolManager::GetFromCache(const CBlockIndex& b CCreditPool pool; { LOCK(cache_mutex); - if (creditPoolCache.get(block_hash, pool)) { - return pool; + if (auto cached = creditPoolCache.get(block_hash)) { + return *cached; } } if (block_index.nHeight % DISK_SNAPSHOT_PERIOD == 0) { diff --git a/src/evo/mnhftx.cpp b/src/evo/mnhftx.cpp index 54e5d0c29ce0..88f7d9983724 100644 --- a/src/evo/mnhftx.cpp +++ b/src/evo/mnhftx.cpp @@ -325,8 +325,8 @@ std::optional CMNHFManager::GetFromCache(const CBlockInde const uint256& blockHash = pindex->GetBlockHash(); { LOCK(cs_cache); - if (mnhfCache.get(blockHash, signals)) { - return signals; + if (auto cached = mnhfCache.get(blockHash)) { + return *cached; } } { diff --git a/src/instantsend/db.cpp b/src/instantsend/db.cpp index 778596851385..cb9f4f41b9a0 100644 --- a/src/instantsend/db.cpp +++ b/src/instantsend/db.cpp @@ -271,11 +271,14 @@ InstantSendLockPtr CInstantSendDb::GetInstantSendLockByHashInternal(const uint25 return nullptr; } - InstantSendLockPtr ret; - if (use_cache && islockCache.get(hash, ret)) { - return ret; + if (use_cache) { + if (auto cached = islockCache.get(hash)) { + return *cached; + } } + InstantSendLockPtr ret; + ret = std::make_shared(); bool exists = db->Read(std::make_tuple(DB_ISLOCK_BY_HASH, hash), *ret); if (!exists || (::SerializeHash(*ret) != hash)) { @@ -292,13 +295,14 @@ InstantSendLockPtr CInstantSendDb::GetInstantSendLockByHashInternal(const uint25 uint256 CInstantSendDb::GetInstantSendLockHashByTxidInternal(const uint256& txid) const { AssertLockHeld(cs_db); + if (auto cached = txidCache.get(txid)) { + return *cached; + } uint256 islockHash; - if (!txidCache.get(txid, islockHash)) { - if (!db->Read(std::make_tuple(DB_HASH_BY_TXID, txid), islockHash)) { - return {}; - } - txidCache.insert(txid, islockHash); + if (!db->Read(std::make_tuple(DB_HASH_BY_TXID, txid), islockHash)) { + return {}; } + txidCache.insert(txid, islockHash); return islockHash; } @@ -311,13 +315,14 @@ InstantSendLockPtr CInstantSendDb::GetInstantSendLockByTxid(const uint256& txid) InstantSendLockPtr CInstantSendDb::GetInstantSendLockByInput(const COutPoint& outpoint) const { LOCK(cs_db); + if (auto cached = outpointCache.get(outpoint)) { + return GetInstantSendLockByHashInternal(*cached); + } uint256 islockHash; - if (!outpointCache.get(outpoint, islockHash)) { - if (!db->Read(std::make_tuple(DB_HASH_BY_OUTPOINT, outpoint), islockHash)) { - return nullptr; - } - outpointCache.insert(outpoint, islockHash); + if (!db->Read(std::make_tuple(DB_HASH_BY_OUTPOINT, outpoint), islockHash)) { + return nullptr; } + outpointCache.insert(outpoint, islockHash); return GetInstantSendLockByHashInternal(islockHash); } diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index 0baf23fb9abf..b28a9908770d 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -513,11 +513,12 @@ uint256 CQuorumBlockProcessor::GetQuorumBlockHash(const Consensus::LLMQParams& l bool CQuorumBlockProcessor::HasMinedCommitment(Consensus::LLMQType llmqType, const uint256& quorumHash) const { - bool fExists; - if (LOCK(minableCommitmentsCs); mapHasMinedCommitmentCache[llmqType].get(quorumHash, fExists)) { - return fExists; + if (LOCK(minableCommitmentsCs); auto cached = mapHasMinedCommitmentCache[llmqType].get(quorumHash)) { + return *cached; } + bool fExists; + fExists = m_evoDb.Exists(std::make_pair(DB_MINED_COMMITMENT, std::make_pair(llmqType, quorumHash))); LOCK(minableCommitmentsCs); diff --git a/src/llmq/dkgsessionmgr.cpp b/src/llmq/dkgsessionmgr.cpp index 3c2fdc135fbc..a282fcc34b4c 100644 --- a/src/llmq/dkgsessionmgr.cpp +++ b/src/llmq/dkgsessionmgr.cpp @@ -149,7 +149,9 @@ MessageProcessingResult CDKGSessionManager::ProcessMessage(CNode& pfrom, bool is if (indexedQuorumsCache.empty()) { utils::InitQuorumsCache(indexedQuorumsCache); } - indexedQuorumsCache[llmqType].get(quorumHash, quorumIndex); + if (auto cached = indexedQuorumsCache[llmqType].get(quorumHash)) { + quorumIndex = *cached; + } } // No luck, try to compute diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index 3c467aca2530..74256e1836ba 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -571,8 +571,8 @@ std::vector CQuorumManager::ScanQuorums(Consensus::LLMQType llmqTyp } } auto& cache = scanQuorumsCache[llmqType]; - bool fCacheExists = cache.get(pindexStore->GetBlockHash(), vecResultQuorums); - if (fCacheExists) { + if (auto cached = cache.get(pindexStore->GetBlockHash())) { + vecResultQuorums = *cached; // We have exactly what requested so just return it if (vecResultQuorums.size() == nCountRequested) { return vecResultQuorums; @@ -642,12 +642,15 @@ CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const uint25 // We cannot hold cs_quorumBaseBlockIndexCache the whole time as that creates lock-order inversion with cs_main; // We cannot acquire cs_main if we have cs_quorumBaseBlockIndexCache held const CBlockIndex* pindex; - if (!WITH_LOCK(cs_quorumBaseBlockIndexCache, return quorumBaseBlockIndexCache.get(quorumHash, pindex))) { + auto cached = WITH_LOCK(cs_quorumBaseBlockIndexCache, return quorumBaseBlockIndexCache.get(quorumHash)); + if (!cached.has_value()) { pindex = WITH_LOCK(::cs_main, return m_chainstate.m_blockman.LookupBlockIndex(quorumHash)); if (pindex) { LOCK(cs_quorumBaseBlockIndexCache); quorumBaseBlockIndexCache.insert(quorumHash, pindex); } + } else { + pindex = *cached; } return pindex; }(); @@ -668,9 +671,8 @@ CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, gsl::not_nul return nullptr; } - CQuorumPtr pQuorum; - if (LOCK(cs_map_quorums); mapQuorumsCache[llmqType].get(quorumHash, pQuorum)) { - return pQuorum; + if (LOCK(cs_map_quorums); auto cached = mapQuorumsCache[llmqType].get(quorumHash)) { + return *cached; } return BuildQuorumFromCommitment(llmqType, pQuorumBaseBlockIndex, populate_cache); @@ -833,9 +835,12 @@ MessageProcessingResult CQuorumManager::ProcessMessage(CNode& pfrom, CConnman& c CQuorumPtr pQuorum; { - if (LOCK(cs_map_quorums); !mapQuorumsCache[request.GetLLMQType()].get(request.GetQuorumHash(), pQuorum)) { + LOCK(cs_map_quorums); + auto cached = mapQuorumsCache[request.GetLLMQType()].get(request.GetQuorumHash()); + if (!cached.has_value()) { return errorHandler("Quorum not found", 0); // Don't bump score because we asked for it } + pQuorum = *cached; } // Check if request has QUORUM_VERIFICATION_VECTOR data @@ -1118,9 +1123,8 @@ void CQuorumManager::StartCleanupOldQuorumDataThread(const CBlockIndex* pIndex) const CBlockIndex* pindex_loop{pIndex}; std::set quorum_keys; while (pindex_loop != nullptr && pIndex->nHeight - pindex_loop->nHeight < params.max_store_depth()) { - uint256 quorum_key; - if (cache.get(pindex_loop->GetBlockHash(), quorum_key)) { - quorum_keys.insert(quorum_key); + if (auto quorum_key = cache.get(pindex_loop->GetBlockHash())) { + quorum_keys.insert(*quorum_key); if (quorum_keys.size() >= static_cast(params.keepOldKeys)) break; // extra safety belt } pindex_loop = pindex_loop->pprev; diff --git a/src/llmq/signing.cpp b/src/llmq/signing.cpp index 2511c1026a79..1eb3f5adaf87 100644 --- a/src/llmq/signing.cpp +++ b/src/llmq/signing.cpp @@ -43,17 +43,16 @@ bool CRecoveredSigsDb::HasRecoveredSig(Consensus::LLMQType llmqType, const uint2 bool CRecoveredSigsDb::HasRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id) const { auto cacheKey = std::make_pair(llmqType, id); - bool ret; { LOCK(cs_cache); - if (hasSigForIdCache.get(cacheKey, ret)) { - return ret; + if (auto cached = hasSigForIdCache.get(cacheKey)) { + return *cached; } } auto k = std::make_tuple(std::string("rs_r"), llmqType, id); - ret = db->Exists(k); + bool ret = db->Exists(k); LOCK(cs_cache); hasSigForIdCache.insert(cacheKey, ret); @@ -62,16 +61,15 @@ bool CRecoveredSigsDb::HasRecoveredSigForId(Consensus::LLMQType llmqType, const bool CRecoveredSigsDb::HasRecoveredSigForSession(const uint256& signHash) const { - bool ret; { LOCK(cs_cache); - if (hasSigForSessionCache.get(signHash, ret)) { - return ret; + if (auto cached = hasSigForSessionCache.get(signHash)) { + return *cached; } } auto k = std::make_tuple(std::string("rs_s"), signHash); - ret = db->Exists(k); + bool ret = db->Exists(k); LOCK(cs_cache); hasSigForSessionCache.insert(signHash, ret); @@ -80,16 +78,15 @@ bool CRecoveredSigsDb::HasRecoveredSigForSession(const uint256& signHash) const bool CRecoveredSigsDb::HasRecoveredSigForHash(const uint256& hash) const { - bool ret; { LOCK(cs_cache); - if (hasSigForHashCache.get(hash, ret)) { - return ret; + if (auto cached = hasSigForHashCache.get(hash)) { + return *cached; } } auto k = std::make_tuple(std::string("rs_h"), hash); - ret = db->Exists(k); + bool ret = db->Exists(k); LOCK(cs_cache); hasSigForHashCache.insert(hash, ret); diff --git a/src/llmq/snapshot.cpp b/src/llmq/snapshot.cpp index 123ad4c90194..0631eb6aec50 100644 --- a/src/llmq/snapshot.cpp +++ b/src/llmq/snapshot.cpp @@ -352,8 +352,8 @@ std::optional CQuorumSnapshotManager::GetSnapshotForBlock(const LOCK(snapshotCacheCs); // try using cache before reading from disk - if (quorumSnapshotCache.get(snapshotHash, snapshot)) { - return snapshot; + if (auto cached = quorumSnapshotCache.get(snapshotHash)) { + return *cached; } if (m_evoDb.Read(std::make_pair(DB_QUORUM_SNAPSHOT, snapshotHash), snapshot)) { quorumSnapshotCache.insert(snapshotHash, snapshot); diff --git a/src/llmq/utils.cpp b/src/llmq/utils.cpp index 09d2d4fddc7b..327c05189ee5 100644 --- a/src/llmq/utils.cpp +++ b/src/llmq/utils.cpp @@ -215,8 +215,8 @@ std::vector GetAllQuorumMembers(Consensus::LLMQType llmqTy } if (reset_cache) { mapQuorumMembers[llmqType].clear(); - } else if (mapQuorumMembers[llmqType].get(pQuorumBaseBlockIndex->GetBlockHash(), quorumMembers)) { - return quorumMembers; + } else if (auto cached = mapQuorumMembers[llmqType].get(pQuorumBaseBlockIndex->GetBlockHash())) { + return *cached; } } @@ -251,7 +251,8 @@ std::vector GetAllQuorumMembers(Consensus::LLMQType llmqTy if (reset_cache) { LOCK(cs_indexed_members); mapIndexedQuorumMembers[llmqType].clear(); - } else if (LOCK(cs_indexed_members); mapIndexedQuorumMembers[llmqType].get(std::pair(pCycleQuorumBaseBlockIndex->GetBlockHash(), quorumIndex), quorumMembers)) { + } else if (LOCK(cs_indexed_members); auto cached = mapIndexedQuorumMembers[llmqType].get(std::pair(pCycleQuorumBaseBlockIndex->GetBlockHash(), quorumIndex))) { + quorumMembers = *cached; LOCK(cs_members); mapQuorumMembers[llmqType].insert(pQuorumBaseBlockIndex->GetBlockHash(), quorumMembers); return quorumMembers; diff --git a/src/masternode/meta.cpp b/src/masternode/meta.cpp index dee9c08397dc..5c5e2484f5a3 100644 --- a/src/masternode/meta.cpp +++ b/src/masternode/meta.cpp @@ -139,12 +139,10 @@ bool CMasternodeMetaMan::AlreadyHavePlatformBan(const uint256& inv_hash) const std::optional CMasternodeMetaMan::GetPlatformBan(const uint256& inv_hash) const { LOCK(cs); - PlatformBanMessage ret; - if (!m_seen_platform_bans.get(inv_hash, ret)) { - return std::nullopt; + if (auto cached = m_seen_platform_bans.get(inv_hash)) { + return *cached; } - - return ret; + return std::nullopt; } void CMasternodeMetaMan::RememberPlatformBan(const uint256& inv_hash, PlatformBanMessage&& msg) diff --git a/src/unordered_lru_cache.h b/src/unordered_lru_cache.h index 036c955d67a6..c1624ee0a370 100644 --- a/src/unordered_lru_cache.h +++ b/src/unordered_lru_cache.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -56,15 +57,14 @@ class unordered_lru_cache _emplace(key, v); } - bool get(const Key& key, Value& value) + std::optional get(const Key& key) { auto it = cacheMap.find(key); if (it != cacheMap.end()) { it->second.second = accessCounter++; - value = it->second.first; - return true; + return std::make_optional(it->second.first); } - return false; + return std::nullopt; } bool exists(const Key& key) From 4b85c94052beb1b19010f3074b1e1b0a9c3c5874 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 11 Nov 2025 22:45:36 -0600 Subject: [PATCH 2/4] test(unordered_lru_cache): add comprehensive unit tests Add Boost.Test suite with 17 test cases covering: - Construction and configuration (runtime/compile-time sizing) - Basic API behavior (insert, get, emplace, exists, erase, clear) - LRU and truncation semantics (threshold behavior, recency tracking) - Edge cases (maxSize=1, various threshold combinations) Also optimize truncate_if_needed() to use std::nth_element instead of std::sort for O(N) average complexity instead of O(N log N). --- src/Makefile.test.include | 1 + src/test/unordered_lru_cache_tests.cpp | 402 +++++++++++++++++++++++++ src/unordered_lru_cache.h | 7 +- 3 files changed, 407 insertions(+), 3 deletions(-) create mode 100644 src/test/unordered_lru_cache_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index ecce78db38f1..b7ce5f0a736a 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -196,6 +196,7 @@ BITCOIN_TESTS =\ test/txvalidation_tests.cpp \ test/txvalidationcache_tests.cpp \ test/uint256_tests.cpp \ + test/unordered_lru_cache_tests.cpp \ test/util_tests.cpp \ test/validation_block_tests.cpp \ test/validation_chainstate_tests.cpp \ diff --git a/src/test/unordered_lru_cache_tests.cpp b/src/test/unordered_lru_cache_tests.cpp new file mode 100644 index 000000000000..122301389c59 --- /dev/null +++ b/src/test/unordered_lru_cache_tests.cpp @@ -0,0 +1,402 @@ +// Copyright (c) 2024 The Dash Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include + +struct IntHasher { + size_t operator()(int v) const noexcept { + return std::hash{}(v); + } +}; + +BOOST_AUTO_TEST_SUITE(unordered_lru_cache_tests) + +BOOST_AUTO_TEST_CASE(construction_and_configuration) +{ + // Runtime-sized cache with default truncateThreshold + unordered_lru_cache c1(5); + BOOST_CHECK_EQUAL(c1.max_size(), 5U); + + // Custom truncateThreshold + unordered_lru_cache c2(5, 10); + BOOST_CHECK_EQUAL(c2.max_size(), 5U); + + // truncateThreshold less than maxSize + unordered_lru_cache c3(10, 5); + BOOST_CHECK_EQUAL(c3.max_size(), 10U); + + // truncateThreshold equal to maxSize + unordered_lru_cache c4(5, 5); + BOOST_CHECK_EQUAL(c4.max_size(), 5U); +} + +BOOST_AUTO_TEST_CASE(compile_time_maxsize) +{ + // Compile-time MaxSize template parameter + unordered_lru_cache c; + BOOST_CHECK_EQUAL(c.max_size(), 10U); +} + +BOOST_AUTO_TEST_CASE(basic_insert_and_get) +{ + unordered_lru_cache c(10); + + // Insert new key + c.insert(1, 10); + auto opt = c.get(1); + BOOST_CHECK(opt.has_value()); + BOOST_CHECK_EQUAL(opt.value(), 10); + + // Get non-existent key + auto opt2 = c.get(2); + BOOST_CHECK(!opt2.has_value()); + + // Insert another key + c.insert(2, 20); + opt2 = c.get(2); + BOOST_CHECK(opt2.has_value()); + BOOST_CHECK_EQUAL(opt2.value(), 20); +} + +BOOST_AUTO_TEST_CASE(emplace_behavior) +{ + unordered_lru_cache c(10); + + // Emplace new key + c.emplace(1, 10); + auto opt = c.get(1); + BOOST_CHECK(opt.has_value()); + BOOST_CHECK_EQUAL(opt.value(), 10); + + // Emplace existing key updates value + c.emplace(1, 15); + opt = c.get(1); + BOOST_CHECK(opt.has_value()); + BOOST_CHECK_EQUAL(opt.value(), 15); +} + +BOOST_AUTO_TEST_CASE(exists_touches_recency) +{ + unordered_lru_cache c(3, 5); + + // Insert keys 1-5 + for (int i = 1; i <= 5; ++i) { + c.insert(i, i * 10); + } + + // Touch key 1 via exists + BOOST_CHECK(c.exists(1)); + + // Insert key 6, which should trigger truncation + // Key 1 should survive because it was touched + c.insert(6, 60); + + BOOST_CHECK(c.exists(1)); // Key 1 should still exist + BOOST_CHECK(!c.exists(2)); // Key 2 should be evicted +} + +BOOST_AUTO_TEST_CASE(erase_behavior) +{ + unordered_lru_cache c(10); + + c.insert(1, 10); + BOOST_CHECK(c.exists(1)); + + c.erase(1); + BOOST_CHECK(!c.exists(1)); + BOOST_CHECK(!c.get(1).has_value()); + + // Erasing non-existent key is a no-op + c.erase(999); +} + +BOOST_AUTO_TEST_CASE(clear_empties_cache) +{ + unordered_lru_cache c(10); + + for (int i = 1; i <= 5; ++i) { + c.insert(i, i * 10); + } + + c.clear(); + + for (int i = 1; i <= 5; ++i) { + BOOST_CHECK(!c.exists(i)); + BOOST_CHECK(!c.get(i).has_value()); + } +} + +BOOST_AUTO_TEST_CASE(get_returns_copy) +{ + unordered_lru_cache c(10); + + c.insert(1, 10); + auto opt = c.get(1); + BOOST_REQUIRE(opt.has_value()); + + // Mutate returned value + opt.value() = 99; + + // Original value should be unchanged + auto opt2 = c.get(1); + BOOST_CHECK(opt2.has_value()); + BOOST_CHECK_EQUAL(opt2.value(), 10); +} + +BOOST_AUTO_TEST_CASE(no_truncation_below_threshold) +{ + unordered_lru_cache c(5, 10); + + // Insert up to threshold + for (int i = 1; i <= 10; ++i) { + c.insert(i, i * 10); + } + + // All keys should still exist + for (int i = 1; i <= 10; ++i) { + BOOST_CHECK(c.exists(i)); + } +} + +BOOST_AUTO_TEST_CASE(truncation_on_first_overflow) +{ + unordered_lru_cache c(5, 10); + + // Insert up to threshold + for (int i = 1; i <= 10; ++i) { + c.insert(i, i * 10); + } + + // Insert one more to trigger truncation + c.insert(11, 110); + + // Should have exactly maxSize entries + int count = 0; + for (int i = 1; i <= 11; ++i) { + if (c.exists(i)) { + count++; + } + } + BOOST_CHECK_EQUAL(count, 5U); + + // Most recent entries should survive + BOOST_CHECK(c.exists(11)); // Most recent + BOOST_CHECK(c.exists(10)); + BOOST_CHECK(c.exists(9)); + BOOST_CHECK(c.exists(8)); + BOOST_CHECK(c.exists(7)); +} + +BOOST_AUTO_TEST_CASE(sequential_inserts_preserve_most_recent) +{ + unordered_lru_cache c(3, 5); + + // Insert 5 items + for (int i = 1; i <= 5; ++i) { + c.insert(i, i * 10); + } + + // Insert 6th item triggers truncation + c.insert(6, 60); + + // Last 3 inserted should survive (4, 5, 6) + BOOST_CHECK(!c.exists(1)); + BOOST_CHECK(!c.exists(2)); + BOOST_CHECK(!c.exists(3)); + BOOST_CHECK(c.exists(4)); + BOOST_CHECK(c.exists(5)); + BOOST_CHECK(c.exists(6)); +} + +BOOST_AUTO_TEST_CASE(touching_older_entry_preserves_it) +{ + unordered_lru_cache c(3, 5); + + // Insert 5 items + for (int i = 1; i <= 5; ++i) { + c.insert(i, i * 10); + } + + // Touch key 2 (older entry) + c.get(2); + + // Insert 6th item triggers truncation + c.insert(6, 60); + + // Key 2 should survive because it was touched (most recent before 6) + BOOST_CHECK(c.exists(2)); + // Keys 5 and 6 should also survive (most recent) + BOOST_CHECK(c.exists(5)); + BOOST_CHECK(c.exists(6)); + + // Keys 1, 3, and 4 should be evicted + BOOST_CHECK(!c.exists(1)); + BOOST_CHECK(!c.exists(3)); + BOOST_CHECK(!c.exists(4)); +} + +BOOST_AUTO_TEST_CASE(reinserting_updates_recency) +{ + unordered_lru_cache c(3, 5); + + // Insert 5 items + for (int i = 1; i <= 5; ++i) { + c.insert(i, i * 10); + } + + // Re-insert key 1 (oldest) + c.insert(1, 100); + + // Insert 6th item triggers truncation + c.insert(6, 60); + + // Key 1 should survive because it was re-inserted + BOOST_CHECK(c.exists(1)); + BOOST_CHECK_EQUAL(c.get(1).value(), 100); // Value should be updated + + // Keys 5 and 6 should also survive + BOOST_CHECK(c.exists(5)); + BOOST_CHECK(c.exists(6)); + + // Keys 2, 3, 4 should be evicted + BOOST_CHECK(!c.exists(2)); + BOOST_CHECK(!c.exists(3)); + BOOST_CHECK(!c.exists(4)); +} + +BOOST_AUTO_TEST_CASE(multiple_truncation_triggers) +{ + unordered_lru_cache c(3, 5); + + // Insert 5 items + for (int i = 1; i <= 5; ++i) { + c.insert(i, i * 10); + } + + // Insert 6th item triggers truncation (6 > threshold 5) + c.insert(6, 60); + + // After truncation, we have 3 items. Insert more to exceed threshold again + c.insert(7, 70); + c.insert(8, 80); + c.insert(9, 90); + // Now we have 6 items (after inserting 7, 8, 9), which exceeds threshold 5 + // This triggers truncation back to 3 + + // Should have exactly 3 entries + int count = 0; + for (int i = 1; i <= 9; ++i) { + if (c.exists(i)) { + count++; + } + } + BOOST_CHECK_EQUAL(count, 3U); + + // Most recent 3 should survive (7, 8, 9) + BOOST_CHECK(c.exists(7)); + BOOST_CHECK(c.exists(8)); + BOOST_CHECK(c.exists(9)); +} + +BOOST_AUTO_TEST_CASE(edge_case_maxsize_one) +{ + unordered_lru_cache c(1, 2); + + // Insert first item + c.insert(1, 10); + BOOST_CHECK(c.exists(1)); + + // Insert second item (no truncation yet) + c.insert(2, 20); + BOOST_CHECK(c.exists(1)); + BOOST_CHECK(c.exists(2)); + + // Insert third item triggers truncation + c.insert(3, 30); + BOOST_CHECK(!c.exists(1)); + BOOST_CHECK(!c.exists(2)); + BOOST_CHECK(c.exists(3)); + + // Get preserves the only entry + c.get(3); + BOOST_CHECK(c.exists(3)); +} + +BOOST_AUTO_TEST_CASE(edge_case_threshold_equals_maxsize) +{ + unordered_lru_cache c(5, 5); + + // Insert 5 items (no truncation yet) + for (int i = 1; i <= 5; ++i) { + c.insert(i, i * 10); + } + + // All should exist + for (int i = 1; i <= 5; ++i) { + BOOST_CHECK(c.exists(i)); + } + + // Insert 6th item triggers immediate truncation + c.insert(6, 60); + + // Should have exactly 5 entries + int count = 0; + for (int i = 1; i <= 6; ++i) { + if (c.exists(i)) { + count++; + } + } + BOOST_CHECK_EQUAL(count, 5U); + + // Last 5 should survive (2, 3, 4, 5, 6) + BOOST_CHECK(!c.exists(1)); + BOOST_CHECK(c.exists(2)); + BOOST_CHECK(c.exists(3)); + BOOST_CHECK(c.exists(4)); + BOOST_CHECK(c.exists(5)); + BOOST_CHECK(c.exists(6)); +} + +BOOST_AUTO_TEST_CASE(edge_case_threshold_less_than_maxsize) +{ + unordered_lru_cache c(10, 5); + + // Insert 5 items (no truncation yet) + for (int i = 1; i <= 5; ++i) { + c.insert(i, i * 10); + } + + // All should exist + for (int i = 1; i <= 5; ++i) { + BOOST_CHECK(c.exists(i)); + } + + // Insert 6th item triggers truncation + c.insert(6, 60); + + // Should truncate to maxSize (10), but we only have 6 items + // So all should still exist + for (int i = 1; i <= 6; ++i) { + BOOST_CHECK(c.exists(i)); + } + + // Insert more items to exceed maxSize + for (int i = 7; i <= 15; ++i) { + c.insert(i, i * 10); + } + + // Should have exactly 10 entries + int count = 0; + for (int i = 1; i <= 15; ++i) { + if (c.exists(i)) { + count++; + } + } + BOOST_CHECK_EQUAL(count, 10U); +} + +BOOST_AUTO_TEST_SUITE_END() + diff --git a/src/unordered_lru_cache.h b/src/unordered_lru_cache.h index c1624ee0a370..a39c42752c41 100644 --- a/src/unordered_lru_cache.h +++ b/src/unordered_lru_cache.h @@ -102,11 +102,12 @@ class unordered_lru_cache vec.emplace_back(it); } // sort by last access time (descending order) - std::sort(vec.begin(), vec.end(), [](const Iterator& it1, const Iterator& it2) { + const size_t keep = std::min(maxSize, vec.size()); + std::nth_element(vec.begin(), vec.begin() + keep, vec.end(), [](const Iterator& it1, const Iterator& it2) { return it1->second.second > it2->second.second; }); - - for (size_t i = maxSize; i < vec.size(); i++) { + // Erase all but the first `keep` most recently accessed entries + for (size_t i = keep; i < vec.size(); i++) { cacheMap.erase(vec[i]); } } From 49ee1bdad048857c252ce9329dcc95390adb5381 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 12 Nov 2025 09:28:53 -0600 Subject: [PATCH 3/4] test: remove trailing whitespace from unordered_lru_cache_tests.cpp --- src/test/unordered_lru_cache_tests.cpp | 122 ++++++++++++------------- 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/src/test/unordered_lru_cache_tests.cpp b/src/test/unordered_lru_cache_tests.cpp index 122301389c59..f374198cefa1 100644 --- a/src/test/unordered_lru_cache_tests.cpp +++ b/src/test/unordered_lru_cache_tests.cpp @@ -19,15 +19,15 @@ BOOST_AUTO_TEST_CASE(construction_and_configuration) // Runtime-sized cache with default truncateThreshold unordered_lru_cache c1(5); BOOST_CHECK_EQUAL(c1.max_size(), 5U); - + // Custom truncateThreshold unordered_lru_cache c2(5, 10); BOOST_CHECK_EQUAL(c2.max_size(), 5U); - + // truncateThreshold less than maxSize unordered_lru_cache c3(10, 5); BOOST_CHECK_EQUAL(c3.max_size(), 10U); - + // truncateThreshold equal to maxSize unordered_lru_cache c4(5, 5); BOOST_CHECK_EQUAL(c4.max_size(), 5U); @@ -43,17 +43,17 @@ BOOST_AUTO_TEST_CASE(compile_time_maxsize) BOOST_AUTO_TEST_CASE(basic_insert_and_get) { unordered_lru_cache c(10); - + // Insert new key c.insert(1, 10); auto opt = c.get(1); BOOST_CHECK(opt.has_value()); BOOST_CHECK_EQUAL(opt.value(), 10); - + // Get non-existent key auto opt2 = c.get(2); BOOST_CHECK(!opt2.has_value()); - + // Insert another key c.insert(2, 20); opt2 = c.get(2); @@ -64,13 +64,13 @@ BOOST_AUTO_TEST_CASE(basic_insert_and_get) BOOST_AUTO_TEST_CASE(emplace_behavior) { unordered_lru_cache c(10); - + // Emplace new key c.emplace(1, 10); auto opt = c.get(1); BOOST_CHECK(opt.has_value()); BOOST_CHECK_EQUAL(opt.value(), 10); - + // Emplace existing key updates value c.emplace(1, 15); opt = c.get(1); @@ -81,19 +81,19 @@ BOOST_AUTO_TEST_CASE(emplace_behavior) BOOST_AUTO_TEST_CASE(exists_touches_recency) { unordered_lru_cache c(3, 5); - + // Insert keys 1-5 for (int i = 1; i <= 5; ++i) { c.insert(i, i * 10); } - + // Touch key 1 via exists BOOST_CHECK(c.exists(1)); - + // Insert key 6, which should trigger truncation // Key 1 should survive because it was touched c.insert(6, 60); - + BOOST_CHECK(c.exists(1)); // Key 1 should still exist BOOST_CHECK(!c.exists(2)); // Key 2 should be evicted } @@ -101,14 +101,14 @@ BOOST_AUTO_TEST_CASE(exists_touches_recency) BOOST_AUTO_TEST_CASE(erase_behavior) { unordered_lru_cache c(10); - + c.insert(1, 10); BOOST_CHECK(c.exists(1)); - + c.erase(1); BOOST_CHECK(!c.exists(1)); BOOST_CHECK(!c.get(1).has_value()); - + // Erasing non-existent key is a no-op c.erase(999); } @@ -116,13 +116,13 @@ BOOST_AUTO_TEST_CASE(erase_behavior) BOOST_AUTO_TEST_CASE(clear_empties_cache) { unordered_lru_cache c(10); - + for (int i = 1; i <= 5; ++i) { c.insert(i, i * 10); } - + c.clear(); - + for (int i = 1; i <= 5; ++i) { BOOST_CHECK(!c.exists(i)); BOOST_CHECK(!c.get(i).has_value()); @@ -132,14 +132,14 @@ BOOST_AUTO_TEST_CASE(clear_empties_cache) BOOST_AUTO_TEST_CASE(get_returns_copy) { unordered_lru_cache c(10); - + c.insert(1, 10); auto opt = c.get(1); BOOST_REQUIRE(opt.has_value()); - + // Mutate returned value opt.value() = 99; - + // Original value should be unchanged auto opt2 = c.get(1); BOOST_CHECK(opt2.has_value()); @@ -149,12 +149,12 @@ BOOST_AUTO_TEST_CASE(get_returns_copy) BOOST_AUTO_TEST_CASE(no_truncation_below_threshold) { unordered_lru_cache c(5, 10); - + // Insert up to threshold for (int i = 1; i <= 10; ++i) { c.insert(i, i * 10); } - + // All keys should still exist for (int i = 1; i <= 10; ++i) { BOOST_CHECK(c.exists(i)); @@ -164,15 +164,15 @@ BOOST_AUTO_TEST_CASE(no_truncation_below_threshold) BOOST_AUTO_TEST_CASE(truncation_on_first_overflow) { unordered_lru_cache c(5, 10); - + // Insert up to threshold for (int i = 1; i <= 10; ++i) { c.insert(i, i * 10); } - + // Insert one more to trigger truncation c.insert(11, 110); - + // Should have exactly maxSize entries int count = 0; for (int i = 1; i <= 11; ++i) { @@ -181,7 +181,7 @@ BOOST_AUTO_TEST_CASE(truncation_on_first_overflow) } } BOOST_CHECK_EQUAL(count, 5U); - + // Most recent entries should survive BOOST_CHECK(c.exists(11)); // Most recent BOOST_CHECK(c.exists(10)); @@ -193,15 +193,15 @@ BOOST_AUTO_TEST_CASE(truncation_on_first_overflow) BOOST_AUTO_TEST_CASE(sequential_inserts_preserve_most_recent) { unordered_lru_cache c(3, 5); - + // Insert 5 items for (int i = 1; i <= 5; ++i) { c.insert(i, i * 10); } - + // Insert 6th item triggers truncation c.insert(6, 60); - + // Last 3 inserted should survive (4, 5, 6) BOOST_CHECK(!c.exists(1)); BOOST_CHECK(!c.exists(2)); @@ -214,24 +214,24 @@ BOOST_AUTO_TEST_CASE(sequential_inserts_preserve_most_recent) BOOST_AUTO_TEST_CASE(touching_older_entry_preserves_it) { unordered_lru_cache c(3, 5); - + // Insert 5 items for (int i = 1; i <= 5; ++i) { c.insert(i, i * 10); } - + // Touch key 2 (older entry) c.get(2); - + // Insert 6th item triggers truncation c.insert(6, 60); - + // Key 2 should survive because it was touched (most recent before 6) BOOST_CHECK(c.exists(2)); // Keys 5 and 6 should also survive (most recent) BOOST_CHECK(c.exists(5)); BOOST_CHECK(c.exists(6)); - + // Keys 1, 3, and 4 should be evicted BOOST_CHECK(!c.exists(1)); BOOST_CHECK(!c.exists(3)); @@ -241,26 +241,26 @@ BOOST_AUTO_TEST_CASE(touching_older_entry_preserves_it) BOOST_AUTO_TEST_CASE(reinserting_updates_recency) { unordered_lru_cache c(3, 5); - + // Insert 5 items for (int i = 1; i <= 5; ++i) { c.insert(i, i * 10); } - + // Re-insert key 1 (oldest) c.insert(1, 100); - + // Insert 6th item triggers truncation c.insert(6, 60); - + // Key 1 should survive because it was re-inserted BOOST_CHECK(c.exists(1)); BOOST_CHECK_EQUAL(c.get(1).value(), 100); // Value should be updated - + // Keys 5 and 6 should also survive BOOST_CHECK(c.exists(5)); BOOST_CHECK(c.exists(6)); - + // Keys 2, 3, 4 should be evicted BOOST_CHECK(!c.exists(2)); BOOST_CHECK(!c.exists(3)); @@ -270,22 +270,22 @@ BOOST_AUTO_TEST_CASE(reinserting_updates_recency) BOOST_AUTO_TEST_CASE(multiple_truncation_triggers) { unordered_lru_cache c(3, 5); - + // Insert 5 items for (int i = 1; i <= 5; ++i) { c.insert(i, i * 10); } - + // Insert 6th item triggers truncation (6 > threshold 5) c.insert(6, 60); - + // After truncation, we have 3 items. Insert more to exceed threshold again c.insert(7, 70); c.insert(8, 80); c.insert(9, 90); // Now we have 6 items (after inserting 7, 8, 9), which exceeds threshold 5 // This triggers truncation back to 3 - + // Should have exactly 3 entries int count = 0; for (int i = 1; i <= 9; ++i) { @@ -294,7 +294,7 @@ BOOST_AUTO_TEST_CASE(multiple_truncation_triggers) } } BOOST_CHECK_EQUAL(count, 3U); - + // Most recent 3 should survive (7, 8, 9) BOOST_CHECK(c.exists(7)); BOOST_CHECK(c.exists(8)); @@ -304,22 +304,22 @@ BOOST_AUTO_TEST_CASE(multiple_truncation_triggers) BOOST_AUTO_TEST_CASE(edge_case_maxsize_one) { unordered_lru_cache c(1, 2); - + // Insert first item c.insert(1, 10); BOOST_CHECK(c.exists(1)); - + // Insert second item (no truncation yet) c.insert(2, 20); BOOST_CHECK(c.exists(1)); BOOST_CHECK(c.exists(2)); - + // Insert third item triggers truncation c.insert(3, 30); BOOST_CHECK(!c.exists(1)); BOOST_CHECK(!c.exists(2)); BOOST_CHECK(c.exists(3)); - + // Get preserves the only entry c.get(3); BOOST_CHECK(c.exists(3)); @@ -328,20 +328,20 @@ BOOST_AUTO_TEST_CASE(edge_case_maxsize_one) BOOST_AUTO_TEST_CASE(edge_case_threshold_equals_maxsize) { unordered_lru_cache c(5, 5); - + // Insert 5 items (no truncation yet) for (int i = 1; i <= 5; ++i) { c.insert(i, i * 10); } - + // All should exist for (int i = 1; i <= 5; ++i) { BOOST_CHECK(c.exists(i)); } - + // Insert 6th item triggers immediate truncation c.insert(6, 60); - + // Should have exactly 5 entries int count = 0; for (int i = 1; i <= 6; ++i) { @@ -350,7 +350,7 @@ BOOST_AUTO_TEST_CASE(edge_case_threshold_equals_maxsize) } } BOOST_CHECK_EQUAL(count, 5U); - + // Last 5 should survive (2, 3, 4, 5, 6) BOOST_CHECK(!c.exists(1)); BOOST_CHECK(c.exists(2)); @@ -363,31 +363,31 @@ BOOST_AUTO_TEST_CASE(edge_case_threshold_equals_maxsize) BOOST_AUTO_TEST_CASE(edge_case_threshold_less_than_maxsize) { unordered_lru_cache c(10, 5); - + // Insert 5 items (no truncation yet) for (int i = 1; i <= 5; ++i) { c.insert(i, i * 10); } - + // All should exist for (int i = 1; i <= 5; ++i) { BOOST_CHECK(c.exists(i)); } - + // Insert 6th item triggers truncation c.insert(6, 60); - + // Should truncate to maxSize (10), but we only have 6 items // So all should still exist for (int i = 1; i <= 6; ++i) { BOOST_CHECK(c.exists(i)); } - + // Insert more items to exceed maxSize for (int i = 7; i <= 15; ++i) { c.insert(i, i * 10); } - + // Should have exactly 10 entries int count = 0; for (int i = 1; i <= 15; ++i) { From aafef954294829fae1830a821d47db8aafee9f1e Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 12 Nov 2025 09:29:53 -0600 Subject: [PATCH 4/4] refactor: improve optional usage --- src/evo/cbtx.cpp | 15 ++++++--------- src/evo/creditpool.cpp | 7 ++----- src/instantsend/db.cpp | 4 +--- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 2b527453f5ac..1ed3c9ecf28c 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -85,23 +85,20 @@ auto CachedGetQcHashesQcIndexedHashes(const CBlockIndex* pindexPrev, const llmq: for (const auto& blockIndex : vecBlockIndexes) { uint256 block_hash{blockIndex->GetBlockHash()}; - std::pair qc_hash; - if (auto cached = qc_hashes_cached[llmqType].get(block_hash)) { - qc_hash = *cached; - } else { + std::optional> qc_hash = qc_hashes_cached[llmqType].get(block_hash); + if (!qc_hash.has_value()) { auto [pqc, dummy_hash] = quorum_block_processor.GetMinedCommitment(llmqType, block_hash); if (dummy_hash == uint256::ZERO) { // this should never happen return std::nullopt; } - qc_hash.first = ::SerializeHash(pqc); - qc_hash.second = rotation_enabled ? pqc.quorumIndex : 0; - qc_hashes_cached[llmqType].insert(block_hash, qc_hash); + qc_hash = {::SerializeHash(pqc), rotation_enabled ? pqc.quorumIndex : 0}; + qc_hashes_cached[llmqType].insert(block_hash, *qc_hash); } if (rotation_enabled) { - map_indexed_hashes[qc_hash.second] = qc_hash.first; + map_indexed_hashes[qc_hash->second] = qc_hash->first; } else { - vec_hashes.emplace_back(qc_hash.first); + vec_hashes.emplace_back(qc_hash->first); } } } diff --git a/src/evo/creditpool.cpp b/src/evo/creditpool.cpp index c553c96d0bc9..059272df3edd 100644 --- a/src/evo/creditpool.cpp +++ b/src/evo/creditpool.cpp @@ -65,8 +65,6 @@ static std::optional GetCreditDataFromBlock(const gsl::n return std::nullopt; } - CreditPoolDataPerBlock blockData; - static Mutex cache_mutex; static Uint256LruHashMap block_data_cache GUARDED_BY(cache_mutex){ static_cast(Params().CreditPoolPeriodBlocks()) * 2}; @@ -84,7 +82,7 @@ static std::optional GetCreditDataFromBlock(const gsl::n return std::nullopt; } - + CreditPoolDataPerBlock blockData; if (const auto opt_cbTx = GetTxPayload(block.vtx[0]->vExtraPayload); opt_cbTx) { blockData.credit_pool = opt_cbTx->creditPoolBalance; } else { @@ -120,7 +118,6 @@ std::optional CCreditPoolManager::GetFromCache(const CBlockIndex& b if (!DeploymentActiveAt(block_index, Params().GetConsensus(), Consensus::DEPLOYMENT_V20)) return CCreditPool{}; const uint256 block_hash = block_index.GetBlockHash(); - CCreditPool pool; { LOCK(cache_mutex); if (auto cached = creditPoolCache.get(block_hash)) { @@ -128,7 +125,7 @@ std::optional CCreditPoolManager::GetFromCache(const CBlockIndex& b } } if (block_index.nHeight % DISK_SNAPSHOT_PERIOD == 0) { - if (evoDb.Read(std::make_pair(DB_CREDITPOOL_SNAPSHOT, block_hash), pool)) { + if (CCreditPool pool; evoDb.Read(std::make_pair(DB_CREDITPOOL_SNAPSHOT, block_hash), pool)) { LOCK(cache_mutex); creditPoolCache.insert(block_hash, pool); return pool; diff --git a/src/instantsend/db.cpp b/src/instantsend/db.cpp index cb9f4f41b9a0..1ef55e6cca52 100644 --- a/src/instantsend/db.cpp +++ b/src/instantsend/db.cpp @@ -277,9 +277,7 @@ InstantSendLockPtr CInstantSendDb::GetInstantSendLockByHashInternal(const uint25 } } - InstantSendLockPtr ret; - - ret = std::make_shared(); + InstantSendLockPtr ret{std::make_shared()}; bool exists = db->Read(std::make_tuple(DB_ISLOCK_BY_HASH, hash), *ret); if (!exists || (::SerializeHash(*ret) != hash)) { ret = std::make_shared();