diff --git a/src/Makefile.am b/src/Makefile.am index 501a695cf852..4c9e042ae194 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -291,6 +291,7 @@ BITCOIN_CORE_H = \ key_io.h \ limitedmap.h \ llmq/blockprocessor.h \ + llmq/cache.h \ llmq/commitment.h \ llmq/context.h \ llmq/debug.h \ diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index b044f7c13dad..3a4dd0c08391 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -52,7 +52,7 @@ CQuorumBlockProcessor::CQuorumBlockProcessor(const ChainstateManager& chainman, m_evoDb{evoDb}, m_qsnapman{qsnapman} { - utils::InitQuorumsCache(mapHasMinedCommitmentCache, m_chainman.GetConsensus()); + mapHasMinedCommitmentCache.Init(m_chainman.GetConsensus()); LogPrintf("BLS verification uses %d additional threads\n", bls_threads); m_bls_queue.StartWorkerThreads(bls_threads); } @@ -387,7 +387,7 @@ bool CQuorumBlockProcessor::ProcessCommitment(int nHeight, const uint256& blockH { LOCK(minableCommitmentsCs); - mapHasMinedCommitmentCache[qc.llmqType].erase(qc.quorumHash); + mapHasMinedCommitmentCache.erase(qc.llmqType, qc.quorumHash); minableCommitmentsByQuorum.erase(cacheKey); minableCommitments.erase(::SerializeHash(qc)); } @@ -404,11 +404,7 @@ void CQuorumBlockProcessor::DropQcHashesCache() m_quorums_cached.clear(); m_qc_hashes_cached.clear(); m_qc_indexed_hashes_cached.clear(); - // Clear per-type LRU contents but keep the map entries so InitQuorumsCache is not - // required on every subsequent miss. - for (auto& [_, cache] : m_qc_hashes_lru) { - cache.clear(); - } + m_qc_hashes_lru.clear(); } std::optional> CQuorumBlockProcessor::GetQcHashes(const CBlockIndex* pindexPrev) const @@ -424,8 +420,8 @@ std::optional> CQuorumBlockProcessor::Get m_quorums_cached.clear(); m_qc_hashes_cached.clear(); m_qc_indexed_hashes_cached.clear(); - if (m_qc_hashes_lru.empty()) { - utils::InitQuorumsCache(m_qc_hashes_lru, Params().GetConsensus()); + if (!m_qc_hashes_lru.IsInitialized()) { + m_qc_hashes_lru.Init(Params().GetConsensus()); } for (const auto& [llmqType, vecBlockIndexes] : quorums) { @@ -439,7 +435,7 @@ std::optional> CQuorumBlockProcessor::Get uint256 block_hash{blockIndex->GetBlockHash()}; std::pair qc_hash; - if (!m_qc_hashes_lru[llmqType].get(block_hash, qc_hash)) { + if (!m_qc_hashes_lru.get(llmqType, block_hash, qc_hash)) { auto [pqc, dummy_hash] = GetMinedCommitment(llmqType, block_hash); if (dummy_hash == uint256::ZERO) { // this should never happen @@ -447,7 +443,7 @@ std::optional> CQuorumBlockProcessor::Get } qc_hash.first = ::SerializeHash(pqc); qc_hash.second = rotation_enabled ? pqc.quorumIndex : 0; - m_qc_hashes_lru[llmqType].insert(block_hash, qc_hash); + m_qc_hashes_lru.insert(llmqType, block_hash, qc_hash); } if (rotation_enabled) { map_indexed_hashes[qc_hash.second] = qc_hash.first; @@ -491,7 +487,7 @@ bool CQuorumBlockProcessor::UndoBlock(const CBlock& block, gsl::not_nullsecond.get(quorumHash, fExists)) { - return fExists; - } + if (LOCK(minableCommitmentsCs); mapHasMinedCommitmentCache.get(llmqType, quorumHash, fExists)) { + return fExists; } fExists = m_evoDb.Exists(std::make_pair(DB_MINED_COMMITMENT, std::make_pair(llmqType, quorumHash))); - { - LOCK(minableCommitmentsCs); - // The key set is fixed at construction, so this can only miss if the type was unregistered, - // which the check above already returned on. - if (auto it = mapHasMinedCommitmentCache.find(llmqType); it != mapHasMinedCommitmentCache.end()) { - it->second.insert(quorumHash, fExists); - } - } + LOCK(minableCommitmentsCs); + mapHasMinedCommitmentCache.insert(llmqType, quorumHash, fExists); return fExists; } diff --git a/src/llmq/blockprocessor.h b/src/llmq/blockprocessor.h index 2932c2596c17..657578678919 100644 --- a/src/llmq/blockprocessor.h +++ b/src/llmq/blockprocessor.h @@ -6,10 +6,10 @@ #define BITCOIN_LLMQ_BLOCKPROCESSOR_H #include +#include #include #include #include -#include #include #include @@ -58,7 +58,7 @@ class CQuorumBlockProcessor std::map, uint256> minableCommitmentsByQuorum GUARDED_BY(minableCommitmentsCs); std::map minableCommitments GUARDED_BY(minableCommitmentsCs); - mutable std::map> mapHasMinedCommitmentCache GUARDED_BY(minableCommitmentsCs); + mutable PerLlmqTypeCache mapHasMinedCommitmentCache GUARDED_BY(minableCommitmentsCs); // Memoizes GetQcHashes(). The whole-result cache is keyed on the set of active // quorum base blocks, the LRU on those base-block hashes; neither key identifies @@ -68,7 +68,7 @@ class CQuorumBlockProcessor // block index whose CBlockIndex* the outer cache stores. mutable Mutex m_qc_hashes_cache_mutex; mutable std::map> m_quorums_cached GUARDED_BY(m_qc_hashes_cache_mutex); - mutable std::map>> m_qc_hashes_lru GUARDED_BY(m_qc_hashes_cache_mutex); + mutable PerLlmqTypeCache> m_qc_hashes_lru GUARDED_BY(m_qc_hashes_cache_mutex); mutable QcHashMap m_qc_hashes_cached GUARDED_BY(m_qc_hashes_cache_mutex); mutable QcIndexedHashMap m_qc_indexed_hashes_cached GUARDED_BY(m_qc_hashes_cache_mutex); diff --git a/src/llmq/cache.h b/src/llmq/cache.h new file mode 100644 index 000000000000..5fa54625e44b --- /dev/null +++ b/src/llmq/cache.h @@ -0,0 +1,106 @@ +// Copyright (c) 2026 The Dash Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_LLMQ_CACHE_H +#define BITCOIN_LLMQ_CACHE_H + +#include +#include +#include +#include +#include + +#include +#include + +namespace llmq { + +//! A separate LRU cache per LLMQ type, sized from that type's consensus parameters. +//! +//! Only the types registered for the active chain get a cache. Consensus::LLMQType is a +//! uint8_t enum that arrives over the wire, so callers may pass a type this chain does not +//! use: those lookups miss and those writes are dropped, which is the same answer a cache +//! that has never held such an entry would give. +template +class PerLlmqTypeCache +{ +private: + using CacheType = unordered_lru_cache; + + std::map m_caches; + +public: + //! Creates a cache per registered type, sized by size_fn. Must be called before use; + //! until then every type reads as absent. + template + void Init(const Consensus::Params& consensus_params, SizeFn size_fn) + { + for (const auto& llmq : consensus_params.llmqs) { + m_caches.emplace(std::piecewise_construct, std::forward_as_tuple(llmq.type), + std::forward_as_tuple(size_fn(llmq))); + } + } + + void Init(const Consensus::Params& consensus_params, bool limit_by_connections = true) + { + Init(consensus_params, [limit_by_connections](const Consensus::LLMQParams& llmq) { + return limit_by_connections ? llmq.keepOldConnections : llmq.keepOldKeys; + }); + } + + bool IsInitialized() const { return !m_caches.empty(); } + + bool get(Consensus::LLMQType llmqType, const Key& key, Value& value) + { + auto it = m_caches.find(llmqType); + return it != m_caches.end() && it->second.get(key, value); + } + + void insert(Consensus::LLMQType llmqType, const Key& key, const Value& value) + { + if (auto it = m_caches.find(llmqType); it != m_caches.end()) { + it->second.insert(key, value); + } + } + + void emplace(Consensus::LLMQType llmqType, const Key& key, Value&& value) + { + if (auto it = m_caches.find(llmqType); it != m_caches.end()) { + it->second.emplace(key, std::move(value)); + } + } + + void erase(Consensus::LLMQType llmqType, const Key& key) + { + if (auto it = m_caches.find(llmqType); it != m_caches.end()) { + it->second.erase(key); + } + } + + //! Drops cached entries but keeps the per-type caches, so Init is not needed again. + void clear() + { + for (auto& [_, cache] : m_caches) { + cache.clear(); + } + } + + void clear(Consensus::LLMQType llmqType) + { + if (auto it = m_caches.find(llmqType); it != m_caches.end()) { + it->second.clear(); + } + } + + //! Capacity of a type's cache, or 0 if this chain does not use it. + size_t max_size(Consensus::LLMQType llmqType) const + { + auto it = m_caches.find(llmqType); + return it != m_caches.end() ? it->second.max_size() : 0; + } +}; + +} // namespace llmq + +#endif // BITCOIN_LLMQ_CACHE_H diff --git a/src/llmq/net_dkg.cpp b/src/llmq/net_dkg.cpp index 8659b3be6530..d7b11f480d68 100644 --- a/src/llmq/net_dkg.cpp +++ b/src/llmq/net_dkg.cpp @@ -420,10 +420,10 @@ void NetDKG::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStre int quorumIndex{-1}; { LOCK(cs_indexed_quorums_cache); - if (indexed_quorums_cache.empty()) { - utils::InitQuorumsCache(indexed_quorums_cache, m_chainman.GetConsensus()); + if (!indexed_quorums_cache.IsInitialized()) { + indexed_quorums_cache.Init(m_chainman.GetConsensus()); } - indexed_quorums_cache[llmqType].get(quorumHash, quorumIndex); + indexed_quorums_cache.get(llmqType, quorumHash, quorumIndex); } if (quorumIndex == -1) { @@ -532,7 +532,7 @@ void NetDKG::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStre return; } - WITH_LOCK(cs_indexed_quorums_cache, indexed_quorums_cache[llmqType].insert(quorumHash, quorumIndex)); + WITH_LOCK(cs_indexed_quorums_cache, indexed_quorums_cache.insert(llmqType, quorumHash, quorumIndex)); } bool NetDKG::AlreadyHave(const CInv& inv) diff --git a/src/llmq/net_dkg.h b/src/llmq/net_dkg.h index 2b1d6988878a..350a170bc6ac 100644 --- a/src/llmq/net_dkg.h +++ b/src/llmq/net_dkg.h @@ -6,10 +6,10 @@ #define BITCOIN_LLMQ_NET_DKG_H #include +#include #include #include #include -#include #include #include @@ -100,7 +100,7 @@ class NetDKG final : public NetHandler /** Cache: quorum hash → quorum index, populated lazily by ProcessMessage. */ mutable Mutex cs_indexed_quorums_cache; - mutable std::map> indexed_quorums_cache GUARDED_BY(cs_indexed_quorums_cache); + mutable PerLlmqTypeCache indexed_quorums_cache GUARDED_BY(cs_indexed_quorums_cache); std::vector m_phase_threads; }; diff --git a/src/llmq/net_quorum.cpp b/src/llmq/net_quorum.cpp index c5a1c3320886..698fc41f4cb7 100644 --- a/src/llmq/net_quorum.cpp +++ b/src/llmq/net_quorum.cpp @@ -688,20 +688,19 @@ void NetQuorum::StartCleanupOldQuorumDataThread(gsl::not_nullnHeight - pindex_loop->nHeight < params.max_store_depth()) { uint256 quorum_key; - if (cache.get(pindex_loop->GetBlockHash(), quorum_key)) { + if (cleanupQuorumsCache.get(params.type, pindex_loop->GetBlockHash(), quorum_key)) { quorum_keys.insert(quorum_key); if (quorum_keys.size() >= static_cast(params.keepOldKeys)) break; // extra safety belt } @@ -710,7 +709,8 @@ void NetQuorum::StartCleanupOldQuorumDataThread(gsl::not_nullm_quorum_base_block_index->GetBlockHash(), quorum_key); + cleanupQuorumsCache.insert(params.type, pQuorum->m_quorum_base_block_index->GetBlockHash(), + quorum_key); } dbKeysToSkip.merge(quorum_keys); } diff --git a/src/llmq/net_quorum.h b/src/llmq/net_quorum.h index 4e5f91749b99..b64d69f67b56 100644 --- a/src/llmq/net_quorum.h +++ b/src/llmq/net_quorum.h @@ -5,11 +5,11 @@ #ifndef BITCOIN_LLMQ_NET_QUORUM_H #define BITCOIN_LLMQ_NET_QUORUM_H +#include #include #include #include #include -#include #include #include @@ -117,7 +117,7 @@ class NetQuorum final : public NetHandler, public CValidationInterface const bool m_quorums_recovery; mutable Mutex cs_cleanup; - mutable std::map> cleanupQuorumsCache + mutable PerLlmqTypeCache cleanupQuorumsCache GUARDED_BY(cs_cleanup); mutable ctpl::thread_pool workerPool; diff --git a/src/llmq/quorumsman.cpp b/src/llmq/quorumsman.cpp index d340631f6636..fbbf26d49b99 100644 --- a/src/llmq/quorumsman.cpp +++ b/src/llmq/quorumsman.cpp @@ -38,7 +38,7 @@ CQuorumManager::CQuorumManager(CBLSWorker& _blsWorker, CDeterministicMNManager& m_chainman{chainman}, db{util::MakeDbWrapper({db_params.path / "llmq" / "quorumdb", db_params.memory, db_params.wipe, /*cache_size=*/1 << 20})} { - utils::InitQuorumsCache(mapQuorumsCache, m_chainman.GetConsensus(), /*limit_by_connections=*/false); + mapQuorumsCache.Init(m_chainman.GetConsensus(), /*limit_by_connections=*/false); m_cache_interrupt.reset(); m_cache_thread = std::thread(&util::TraceThread, "q-cache", [this] { CacheWarmingThreadMain(); }); MigrateOldQuorumDB(_evoDb); @@ -81,7 +81,7 @@ CQuorumPtr CQuorumManager::BuildQuorumFromCommitment(const Consensus::LLMQType l std::make_unique(std::move(qc)), pQuorumBaseBlockIndex, minedBlockHash, members); if (populate_cache && llmq_params_opt->size == 1) { - WITH_LOCK(m_cs_maps, mapQuorumsCache[llmqType].insert(quorumHash, quorum)); + WITH_LOCK(m_cs_maps, mapQuorumsCache.insert(llmqType, quorumHash, quorum)); return quorum; } @@ -105,7 +105,7 @@ CQuorumPtr CQuorumManager::BuildQuorumFromCommitment(const Consensus::LLMQType l QueueQuorumForWarming(quorum); } - WITH_LOCK(m_cs_maps, mapQuorumsCache[llmqType].insert(quorumHash, quorum)); + WITH_LOCK(m_cs_maps, mapQuorumsCache.insert(llmqType, quorumHash, quorum)); return quorum; } @@ -187,18 +187,17 @@ std::vector CQuorumManager::ScanQuorums(Consensus::LLMQType llmqTyp { LOCK(m_cs_maps); - if (scanQuorumsCache.empty()) { - for (const auto& llmq : Params().GetConsensus().llmqs) { - // NOTE: We store it for each block hash in the DKG mining phase here - // and not for a single quorum hash per quorum like we do for other caches. - // And we only do this for max_cycles() of the most recent quorums - // because signing by old quorums requires the exact quorum hash to be specified - // and quorum scanning isn't needed there. - scanQuorumsCache.try_emplace(llmq.type, llmq.max_cycles(llmq.keepOldConnections) * (llmq.dkgMiningWindowEnd - llmq.dkgMiningWindowStart)); - } + if (!scanQuorumsCache.IsInitialized()) { + // NOTE: We store it for each block hash in the DKG mining phase here + // and not for a single quorum hash per quorum like we do for other caches. + // And we only do this for max_cycles() of the most recent quorums + // because signing by old quorums requires the exact quorum hash to be specified + // and quorum scanning isn't needed there. + scanQuorumsCache.Init(Params().GetConsensus(), [](const Consensus::LLMQParams& llmq) { + return llmq.max_cycles(llmq.keepOldConnections) * (llmq.dkgMiningWindowEnd - llmq.dkgMiningWindowStart); + }); } - auto& cache = scanQuorumsCache[llmqType]; - bool fCacheExists = cache.get(pindexStore->GetBlockHash(), vecResultQuorums); + bool fCacheExists = scanQuorumsCache.get(llmqType, pindexStore->GetBlockHash(), vecResultQuorums); if (fCacheExists) { // We have exactly what requested so just return it if (vecResultQuorums.size() == nCountRequested) { @@ -253,9 +252,9 @@ std::vector CQuorumManager::ScanQuorums(Consensus::LLMQType llmqTyp // Don't cache more than keepOldConnections elements // because signing by old quorums requires the exact quorum hash // to be specified and quorum scanning isn't needed there. - auto& cache = scanQuorumsCache[llmqType]; const size_t nCacheEndIndex = std::min(nCountResult, static_cast(llmq_params_opt->keepOldConnections)); - cache.emplace(pindexStore->GetBlockHash(), {vecResultQuorums.begin(), vecResultQuorums.begin() + nCacheEndIndex}); + scanQuorumsCache.emplace(llmqType, pindexStore->GetBlockHash(), + {vecResultQuorums.begin(), vecResultQuorums.begin() + nCacheEndIndex}); } // Don't return more than nCountRequested elements const size_t nResultEndIndex = std::min(nCountResult, nCountRequested); @@ -356,13 +355,8 @@ CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, gsl::not_nul CQuorumPtr pQuorum; { - // Defence-in-depth: mapQuorumsCache only holds the LLMQ types InitQuorumsCache() seeded - // from the chain's consensus params. operator[] on any other type would insert a - // default-constructed, zero-capacity cache and abort in its constructor, so look up - // without inserting and fall through for unknown types. LOCK(m_cs_maps); - auto it = mapQuorumsCache.find(llmqType); - if (it != mapQuorumsCache.end() && it->second.get(quorumHash, pQuorum)) { + if (mapQuorumsCache.get(llmqType, quorumHash, pQuorum)) { return pQuorum; } } @@ -406,11 +400,8 @@ CQuorumManager::DataResponseValidation CQuorumManager::ValidateDataResponse( CQuorumPtr CQuorumManager::GetCachedMutableQuorum(Consensus::LLMQType llmqType, const uint256& quorumHash) const { CQuorumPtr pQuorum; - // See GetQuorum(): never operator[] this map with a wire-supplied LLMQ type. LOCK(m_cs_maps); - if (auto it = mapQuorumsCache.find(llmqType); it != mapQuorumsCache.end()) { - it->second.get(quorumHash, pQuorum); - } + mapQuorumsCache.get(llmqType, quorumHash, pQuorum); return pQuorum; } diff --git a/src/llmq/quorumsman.h b/src/llmq/quorumsman.h index 1a35b1cf0ab0..9dc24c8d948f 100644 --- a/src/llmq/quorumsman.h +++ b/src/llmq/quorumsman.h @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -72,10 +73,8 @@ class CQuorumManager final GUARDED_BY(cs_data_requests); mutable Mutex m_cs_maps; - mutable std::map> mapQuorumsCache - GUARDED_BY(m_cs_maps); - mutable std::map>> scanQuorumsCache - GUARDED_BY(m_cs_maps); + mutable PerLlmqTypeCache mapQuorumsCache GUARDED_BY(m_cs_maps); + mutable PerLlmqTypeCache> scanQuorumsCache GUARDED_BY(m_cs_maps); // On mainnet, we have around 62 quorums active at any point; let's cache a little more than double that to be safe. // it maps `quorum_hash` to `pindex` diff --git a/src/llmq/utils.cpp b/src/llmq/utils.cpp index 7155a1a98dce..a742886aed22 100644 --- a/src/llmq/utils.cpp +++ b/src/llmq/utils.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -633,9 +634,9 @@ std::optional> ComputeQuorumMembersFromWorkBlo QuorumMembers GetAllQuorumMembers(Consensus::LLMQType llmqType, const UtilParameters& util_params, bool reset_cache) { static RecursiveMutex cs_members; - static std::map> mapQuorumMembers GUARDED_BY(cs_members); + static PerLlmqTypeCache mapQuorumMembers GUARDED_BY(cs_members); static RecursiveMutex cs_indexed_members; - static std::map, QuorumMembers, StaticSaltedHasher>> mapIndexedQuorumMembers GUARDED_BY(cs_indexed_members); + static PerLlmqTypeCache> mapIndexedQuorumMembers GUARDED_BY(cs_indexed_members); // A parentless base index (genesis) can never host a quorum. IsQuorumTypeEnabled() handles the // null, but say so explicitly here: this is reached with an attacker-supplied quorumHash via @@ -648,12 +649,12 @@ QuorumMembers GetAllQuorumMembers(Consensus::LLMQType llmqType, const UtilParame std::vector quorumMembers; { LOCK(cs_members); - if (mapQuorumMembers.empty()) { - InitQuorumsCache(mapQuorumMembers, util_params.m_chainman.GetConsensus()); + if (!mapQuorumMembers.IsInitialized()) { + mapQuorumMembers.Init(util_params.m_chainman.GetConsensus()); } if (reset_cache) { - mapQuorumMembers[llmqType].clear(); - } else if (mapQuorumMembers[llmqType].get(util_params.m_base_index->GetBlockHash(), quorumMembers)) { + mapQuorumMembers.clear(llmqType); + } else if (mapQuorumMembers.get(llmqType, util_params.m_base_index->GetBlockHash(), quorumMembers)) { return quorumMembers; } } @@ -663,8 +664,8 @@ QuorumMembers GetAllQuorumMembers(Consensus::LLMQType llmqType, const UtilParame const auto& llmq_params = llmq_params_opt.value(); if (IsQuorumRotationEnabled(llmq_params, util_params.m_base_index)) { - if (LOCK(cs_indexed_members); mapIndexedQuorumMembers.empty()) { - InitQuorumsCache(mapIndexedQuorumMembers, util_params.m_chainman.GetConsensus()); + if (LOCK(cs_indexed_members); !mapIndexedQuorumMembers.IsInitialized()) { + mapIndexedQuorumMembers.Init(util_params.m_chainman.GetConsensus()); } /* * Quorums created with rotation are now created in a different way. All signingActiveQuorumCount are created @@ -689,11 +690,11 @@ QuorumMembers GetAllQuorumMembers(Consensus::LLMQType llmqType, const UtilParame */ 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)) { + mapIndexedQuorumMembers.clear(llmqType); + } else if (LOCK(cs_indexed_members); mapIndexedQuorumMembers.get( + llmqType, std::pair(pCycleQuorumBaseBlockIndex->GetBlockHash(), quorumIndex), quorumMembers)) { LOCK(cs_members); - mapQuorumMembers[llmqType].insert(util_params.m_base_index->GetBlockHash(), quorumMembers); + mapQuorumMembers.insert(llmqType, util_params.m_base_index->GetBlockHash(), quorumMembers); return quorumMembers; } @@ -708,8 +709,8 @@ QuorumMembers GetAllQuorumMembers(Consensus::LLMQType llmqType, const UtilParame LOCK(cs_indexed_members); for (const size_t i : util::irange(q.size())) { - mapIndexedQuorumMembers[llmqType].emplace(std::make_pair(pCycleQuorumBaseBlockIndex->GetBlockHash(), i), - std::move(q[i])); + mapIndexedQuorumMembers.emplace(llmqType, std::make_pair(pCycleQuorumBaseBlockIndex->GetBlockHash(), i), + std::move(q[i])); } } else { const CBlockIndex* pWorkBlockIndex = DeploymentActiveAfter(util_params.m_base_index, @@ -724,7 +725,7 @@ QuorumMembers GetAllQuorumMembers(Consensus::LLMQType llmqType, const UtilParame } LOCK(cs_members); - mapQuorumMembers[llmqType].insert(util_params.m_base_index->GetBlockHash(), quorumMembers); + mapQuorumMembers.insert(llmqType, util_params.m_base_index->GetBlockHash(), quorumMembers); return quorumMembers; } diff --git a/src/llmq/utils.h b/src/llmq/utils.h index 01ab95316117..ef9b11987bb4 100644 --- a/src/llmq/utils.h +++ b/src/llmq/utils.h @@ -81,15 +81,6 @@ Uint256HashSet GetQuorumConnections(const Consensus::LLMQParams& llmqParams, con Uint256HashSet GetQuorumRelayMembers(const Consensus::LLMQParams& llmqParams, const UtilParameters& util_params, const uint256& forMember, bool onlyOutbound); - -template -inline void InitQuorumsCache(CacheType& cache, const Consensus::Params& consensus_params, bool limit_by_connections = true) -{ - for (const auto& llmq : consensus_params.llmqs) { - cache.emplace(std::piecewise_construct, std::forward_as_tuple(llmq.type), - std::forward_as_tuple(limit_by_connections ? llmq.keepOldConnections : llmq.keepOldKeys)); - } -} } // namespace utils } // namespace llmq diff --git a/src/test/llmq_invalid_type_tests.cpp b/src/test/llmq_invalid_type_tests.cpp index 943299dbf2d2..0e267706a6f1 100644 --- a/src/test/llmq_invalid_type_tests.cpp +++ b/src/test/llmq_invalid_type_tests.cpp @@ -4,83 +4,201 @@ #include +#include +#include #include #include +#include #include +#include #include #include -#include -#include -#include +#include +#include +#include +#include +#include +#include +#include +#include #include #include #include -#include +#include +#include -// An LLMQ type arriving in a P2P QSIGSHARE message is an unvalidated uint8_t. It used to reach -// CQuorumManager::GetQuorum() -> CQuorumBlockProcessor::HasMinedCommitment(), where indexing -// mapHasMinedCommitmentCache with std::map::operator[] inserted a default-constructed -// Uint256LruHashMap. Its default MaxSize is 0, and unordered_lru_cache's constructor asserts -// maxSize != 0, so any of the ~250 unregistered type values aborted the node. +// Consensus::LLMQType is a uint8_t enum serialised verbatim over the wire, so a peer can name +// a type this chain does not use. PerLlmqTypeCache holds a cache only for the registered +// types and answers for the rest as misses; before it existed these were plain +// std::map, and operator[] on an unregistered key default-constructed a +// zero-capacity LRU whose constructor asserts -- a remote abort wherever a handler missed the +// Params().GetLLMQ() gate. // -// These caches are only ever seeded by InitQuorumsCache() with the LLMQ types in the active -// chain's consensus params, so every lookup keyed by untrusted input must use find() rather than -// operator[]. The message handler now rejects unregistered types up front as well. +// These tests cover the cache contract itself plus the two lookup paths a wire-supplied type +// reaches: HasMinedCommitment directly, and GetQuorum, which funnels into it via HasQuorum. BOOST_AUTO_TEST_SUITE(llmq_invalid_type_tests) -// Not a named enumerator, and never present in any chain's consensus params. +// Unassigned enum values; each test re-asserts they are unregistered, so this stays honest +// if a future chainparams change starts using one. static constexpr Consensus::LLMQType UNKNOWN_LLMQ_TYPE{static_cast(0)}; -// LLMQ_25_67. A real enumerator, but registered on testnet only, so it is unknown under regtest. -static constexpr Consensus::LLMQType UNREGISTERED_LLMQ_TYPE{Consensus::LLMQType::LLMQ_25_67}; +static constexpr Consensus::LLMQType UNKNOWN_LLMQ_TYPE_ALT{static_cast(55)}; -BOOST_FIXTURE_TEST_CASE(init_quorums_cache_does_not_seed_unknown_types, BasicTestingSetup) +BOOST_FIXTURE_TEST_CASE(per_llmq_type_cache_ignores_unknown_types, BasicTestingSetup) { - std::map> cache; - llmq::utils::InitQuorumsCache(cache, Params().GetConsensus()); - BOOST_REQUIRE(!cache.empty()); + llmq::PerLlmqTypeCache cache; + BOOST_CHECK(!cache.IsInitialized()); + cache.Init(Params().GetConsensus()); + BOOST_REQUIRE(cache.IsInitialized()); BOOST_REQUIRE(!Params().GetLLMQ(UNKNOWN_LLMQ_TYPE).has_value()); - BOOST_CHECK(cache.find(UNKNOWN_LLMQ_TYPE) == cache.end()); + BOOST_CHECK_EQUAL(cache.max_size(UNKNOWN_LLMQ_TYPE), 0U); + + // Writes for an unregistered type are dropped rather than sized into existence, and reads + // stay misses -- the value below must be left alone. + bool value{true}; + cache.insert(UNKNOWN_LLMQ_TYPE, uint256::ONE, false); + BOOST_CHECK(!cache.get(UNKNOWN_LLMQ_TYPE, uint256::ONE, value)); + BOOST_CHECK(value); + cache.erase(UNKNOWN_LLMQ_TYPE, uint256::ONE); + cache.clear(UNKNOWN_LLMQ_TYPE); + BOOST_CHECK_EQUAL(cache.max_size(UNKNOWN_LLMQ_TYPE), 0U); - // Every seeded type has a non-zero capacity; anything else would have to be - // default-constructed, which is exactly what aborts. for (const auto& llmq : Params().GetConsensus().llmqs) { - auto it = cache.find(llmq.type); - BOOST_REQUIRE(it != cache.end()); - BOOST_CHECK_GT(it->second.max_size(), 0U); + BOOST_CHECK_GT(cache.max_size(llmq.type), 0U); + + value = false; + cache.insert(llmq.type, uint256::ONE, true); + BOOST_CHECK(cache.get(llmq.type, uint256::ONE, value)); + BOOST_CHECK(value); + + cache.erase(llmq.type, uint256::ONE); + BOOST_CHECK(!cache.get(llmq.type, uint256::ONE, value)); } } -// Crash site. Pre-fix this aborted with `Assertion failed: (_maxSize != 0)`; it must report that -// there is no mined commitment instead. +// The crash site itself: an unregistered type must answer false, not abort. BOOST_FIXTURE_TEST_CASE(has_mined_commitment_unknown_llmq_type_is_safe, RegTestingSetup) { - const auto& qbp = *Assert(Assert(m_node.llmq_ctx)->quorum_block_processor); - const uint256 tip_hash = WITH_LOCK(::cs_main, return Assert(m_node.chainman->ActiveTip())->GetBlockHash()); + auto& qbp = *Assert(m_node.llmq_ctx)->quorum_block_processor; + const uint256 hash = WITH_LOCK(::cs_main, return m_node.chainman->ActiveTip()->GetBlockHash()); BOOST_REQUIRE(!Params().GetLLMQ(UNKNOWN_LLMQ_TYPE).has_value()); - BOOST_REQUIRE(!Params().GetLLMQ(UNREGISTERED_LLMQ_TYPE).has_value()); + BOOST_REQUIRE(!Params().GetLLMQ(UNKNOWN_LLMQ_TYPE_ALT).has_value()); - BOOST_CHECK(!qbp.HasMinedCommitment(UNKNOWN_LLMQ_TYPE, tip_hash)); - BOOST_CHECK(!qbp.HasMinedCommitment(UNREGISTERED_LLMQ_TYPE, tip_hash)); + BOOST_CHECK(!qbp.HasMinedCommitment(UNKNOWN_LLMQ_TYPE, hash)); + BOOST_CHECK(!qbp.HasMinedCommitment(UNKNOWN_LLMQ_TYPE_ALT, hash)); + + for (const auto& llmq : Params().GetConsensus().llmqs) { + BOOST_CHECK(!qbp.HasMinedCommitment(llmq.type, hash)); + } } -// The exact call ProcessMessageSigShare() makes with wire-supplied values: LookupBlockIndex() -// succeeds for a real block hash, so HasQuorum() -> HasMinedCommitment() runs with the -// attacker-chosen LLMQ type. Pre-fix this aborted; it must return nullptr instead. +// The path a wire-supplied type actually travels: GetQuorum resolves the block index from the +// hash, then consults HasQuorum -> HasMinedCommitment. Must return nullptr, not abort. BOOST_FIXTURE_TEST_CASE(get_quorum_unknown_llmq_type_is_safe, RegTestingSetup) { - const auto& qman = *Assert(Assert(m_node.llmq_ctx)->qman); - const uint256 tip_hash = WITH_LOCK(::cs_main, return Assert(m_node.chainman->ActiveTip())->GetBlockHash()); + auto& qman = *Assert(m_node.llmq_ctx)->qman; + const uint256 tip_hash = WITH_LOCK(::cs_main, return m_node.chainman->ActiveTip()->GetBlockHash()); BOOST_REQUIRE(!Params().GetLLMQ(UNKNOWN_LLMQ_TYPE).has_value()); - BOOST_REQUIRE(!Params().GetLLMQ(UNREGISTERED_LLMQ_TYPE).has_value()); + BOOST_REQUIRE(!Params().GetLLMQ(UNKNOWN_LLMQ_TYPE_ALT).has_value()); BOOST_CHECK(qman.GetQuorum(UNKNOWN_LLMQ_TYPE, tip_hash) == nullptr); - BOOST_CHECK(qman.GetQuorum(UNREGISTERED_LLMQ_TYPE, tip_hash) == nullptr); + BOOST_CHECK(qman.GetQuorum(UNKNOWN_LLMQ_TYPE_ALT, tip_hash) == nullptr); + + // Same map, reached by the QDATA handler with a wire-supplied type. + BOOST_CHECK(qman.GetCachedMutableQuorum(UNKNOWN_LLMQ_TYPE, tip_hash) == nullptr); + BOOST_CHECK(qman.GetCachedMutableQuorum(UNKNOWN_LLMQ_TYPE_ALT, tip_hash) == nullptr); +} + +namespace { + +CBLSSecretKey MakeSecretKey() +{ + CBLSSecretKey sk; + sk.MakeNewKey(); + return sk; +} + +std::unique_ptr MakePeer(NodeId id) +{ + in_addr peer_in_addr{}; + peer_in_addr.s_addr = htonl(0x0a000001 + static_cast(id)); + auto peer{std::make_unique(id, + /*sock=*/nullptr, + /*addrIn=*/CAddress{CService{peer_in_addr, 9999}, NODE_NETWORK}, + /*nKeyedNetGroupIn=*/0, + /*nLocalHostNonceIn=*/0, + /*addrBindIn=*/CAddress{}, + /*addrNameIn=*/std::string{}, + /*conn_type_in=*/ConnectionType::INBOUND, + /*inbound_onion=*/false)}; + peer->nVersion = PROTOCOL_VERSION; + peer->SetCommonVersion(PROTOCOL_VERSION); + peer->fSuccessfullyConnected = true; + return peer; +} + +//! One CSigShare on the wire -- llmqType | quorumHash | quorumMember | id | msgHash | sigShare -- +//! wrapped in the CompactSize-prefixed vector the QSIGSHARE handler reads. +CDataStream MakeQSigShareStream(Consensus::LLMQType llmq_type) +{ + CDataStream stream{SER_NETWORK, PROTOCOL_VERSION}; + WriteCompactSize(stream, 1); + stream << static_cast(llmq_type); + stream << uint256::ONE; // quorumHash + stream << static_cast(0); // quorumMember + stream << uint256::ONE; // id + stream << uint256::ONE; // msgHash + stream << CBLSLazySignature{}; // sigShare + return stream; +} + +} // namespace + +/** + * The handler gate itself, not just the caches behind it. + * + * The cache hardening above makes an unregistered type a safe miss, but a miss alone is not the + * contract: CSigSharesManager::ProcessMessageSigShare returns true when GetQuorum yields nullptr, + * so without the gate the sender is dropped silently and never scored. Deleting the gate would + * leave every other test in this file green, so pin the ban here. + */ +BOOST_FIXTURE_TEST_CASE(qsigshare_unknown_llmq_type_bans_peer, RegTestingSetup) +{ + BOOST_REQUIRE(!Params().GetLLMQ(UNKNOWN_LLMQ_TYPE).has_value()); + BOOST_REQUIRE(m_node.peerman); + + // The QSIGSHARE branch sits behind this spork and behind a non-null shares manager. The + // fixture builds a bare CSporkManager, so wire up the regtest signer before setting it. + constexpr const char* REGTEST_SPORK_PRIVKEY{"cP4EKFyJsHT39LDqgdcB43Y3YXjNyjb5Fuas1GQSeAtjnZWmZEQK"}; + BOOST_REQUIRE(Assert(m_node.sporkman)->SetSporkAddress(Params().SporkAddress())); + BOOST_REQUIRE(m_node.sporkman->SetPrivKey(REGTEST_SPORK_PRIVKEY)); + BOOST_REQUIRE(m_node.sporkman->UpdateSpork(SPORK_21_QUORUM_ALL_CONNECTED, 0).has_value()); + + CActiveMasternodeManager mn_activeman{*Assert(m_node.connman), *Assert(m_node.dmnman), MakeSecretKey()}; + auto& llmq_ctx = *Assert(m_node.llmq_ctx); + llmq::CSigSharesManager shareman{*Assert(m_node.connman), *Assert(m_node.chainman), *llmq_ctx.sigman, + mn_activeman, *llmq_ctx.qman, *Assert(m_node.sporkman)}; + llmq::NetSigning net_signing{m_node.peerman.get(), *llmq_ctx.sigman, &shareman, *Assert(m_node.sporkman)}; + + auto peer{MakePeer(/*id=*/1)}; + m_node.peerman->InitializeNode(*peer, NODE_NETWORK); + + CNodeStateStats stats; + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); + BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 0); + + auto stream = MakeQSigShareStream(UNKNOWN_LLMQ_TYPE); + net_signing.ProcessMessage(*peer, NetMsgType::QSIGSHARE, stream); + + BOOST_REQUIRE(m_node.peerman->GetNodeStateStats(peer->GetId(), stats)); + BOOST_CHECK_EQUAL(stats.m_misbehavior_score, 100); + + m_node.peerman->FinalizeNode(*peer); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/llmq_utils_tests.cpp b/src/test/llmq_utils_tests.cpp index a3f80e5aa1f9..4b0af70d579a 100644 --- a/src/test/llmq_utils_tests.cpp +++ b/src/test/llmq_utils_tests.cpp @@ -308,9 +308,6 @@ BOOST_AUTO_TEST_CASE(deterministic_outbound_connection_edge_cases_test) // Note: CalcDeterministicWatchConnections requires CBlockIndex which is complex to mock // Testing is deferred to functional tests -// Note: InitQuorumsCache requires specific cache types with LLMQ consensus parameters -// Testing is deferred to integration tests - BOOST_AUTO_TEST_CASE(deterministic_connection_symmetry_test) { // Test interesting properties of DeterministicOutboundConnection