Skip to content

Commit 40d714a

Browse files
refactor: route per-LLMQ-type caches through PerLlmqTypeCache
Consensus::LLMQType is a uint8_t enum that arrives verbatim over the wire, and the LRU caches keyed by it are only pre-seeded with the types in Params().GetConsensus().llmqs. Indexing one of those std::maps with operator[] on an unregistered key default-constructs an unordered_lru_cache with MaxSize = 0, whose constructor asserts; util/check.h makes compiling with NDEBUG a hard #error, so that assert is live in release builds. The QSIGSHARE handler gate and find()-guarded lookups this branch originally carried landed separately in v23.1.8 (a801d3f) and reached develop via the master back-merge, so this is now the follow-up hardening rather than the fix itself. That fix guarded only the two lookups a QSIGSHARE could reach; every other map keyed by LLMQ type still used operator[]. Replace them all with PerLlmqTypeCache, which owns one LRU per registered type and answers unregistered types as misses, so no caller can size a zero-capacity cache into existence. This covers mapHasMinedCommitmentCache, m_qc_hashes_lru, mapQuorumsCache, scanQuorumsCache, cleanupQuorumsCache, indexed_quorums_cache, mapQuorumMembers and mapIndexedQuorumMembers, and lets InitQuorumsCache go away. Extend llmq_invalid_type_tests to pin the cache contract itself plus the QSIGSHARE ban. The ban needs its own test because no cache-level assertion can catch its removal: ProcessMessageSigShare returns true when GetQuorum yields nullptr, so without the gate the peer is dropped silently and never scored.
1 parent fff8664 commit 40d714a

14 files changed

Lines changed: 328 additions & 145 deletions

‎src/Makefile.am‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ BITCOIN_CORE_H = \
291291
key_io.h \
292292
limitedmap.h \
293293
llmq/blockprocessor.h \
294+
llmq/cache.h \
294295
llmq/commitment.h \
295296
llmq/context.h \
296297
llmq/debug.h \

‎src/llmq/blockprocessor.cpp‎

Lines changed: 12 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ CQuorumBlockProcessor::CQuorumBlockProcessor(const ChainstateManager& chainman,
5252
m_evoDb{evoDb},
5353
m_qsnapman{qsnapman}
5454
{
55-
utils::InitQuorumsCache(mapHasMinedCommitmentCache, m_chainman.GetConsensus());
55+
mapHasMinedCommitmentCache.Init(m_chainman.GetConsensus());
5656
LogPrintf("BLS verification uses %d additional threads\n", bls_threads);
5757
m_bls_queue.StartWorkerThreads(bls_threads);
5858
}
@@ -387,7 +387,7 @@ bool CQuorumBlockProcessor::ProcessCommitment(int nHeight, const uint256& blockH
387387

388388
{
389389
LOCK(minableCommitmentsCs);
390-
mapHasMinedCommitmentCache[qc.llmqType].erase(qc.quorumHash);
390+
mapHasMinedCommitmentCache.erase(qc.llmqType, qc.quorumHash);
391391
minableCommitmentsByQuorum.erase(cacheKey);
392392
minableCommitments.erase(::SerializeHash(qc));
393393
}
@@ -404,11 +404,7 @@ void CQuorumBlockProcessor::DropQcHashesCache()
404404
m_quorums_cached.clear();
405405
m_qc_hashes_cached.clear();
406406
m_qc_indexed_hashes_cached.clear();
407-
// Clear per-type LRU contents but keep the map entries so InitQuorumsCache is not
408-
// required on every subsequent miss.
409-
for (auto& [_, cache] : m_qc_hashes_lru) {
410-
cache.clear();
411-
}
407+
m_qc_hashes_lru.clear();
412408
}
413409

414410
std::optional<std::pair<QcHashMap, QcIndexedHashMap>> CQuorumBlockProcessor::GetQcHashes(const CBlockIndex* pindexPrev) const
@@ -424,8 +420,8 @@ std::optional<std::pair<QcHashMap, QcIndexedHashMap>> CQuorumBlockProcessor::Get
424420
m_quorums_cached.clear();
425421
m_qc_hashes_cached.clear();
426422
m_qc_indexed_hashes_cached.clear();
427-
if (m_qc_hashes_lru.empty()) {
428-
utils::InitQuorumsCache(m_qc_hashes_lru, Params().GetConsensus());
423+
if (!m_qc_hashes_lru.IsInitialized()) {
424+
m_qc_hashes_lru.Init(Params().GetConsensus());
429425
}
430426

431427
for (const auto& [llmqType, vecBlockIndexes] : quorums) {
@@ -439,15 +435,15 @@ std::optional<std::pair<QcHashMap, QcIndexedHashMap>> CQuorumBlockProcessor::Get
439435
uint256 block_hash{blockIndex->GetBlockHash()};
440436

441437
std::pair<uint256, int> qc_hash;
442-
if (!m_qc_hashes_lru[llmqType].get(block_hash, qc_hash)) {
438+
if (!m_qc_hashes_lru.get(llmqType, block_hash, qc_hash)) {
443439
auto [pqc, dummy_hash] = GetMinedCommitment(llmqType, block_hash);
444440
if (dummy_hash == uint256::ZERO) {
445441
// this should never happen
446442
return std::nullopt;
447443
}
448444
qc_hash.first = ::SerializeHash(pqc);
449445
qc_hash.second = rotation_enabled ? pqc.quorumIndex : 0;
450-
m_qc_hashes_lru[llmqType].insert(block_hash, qc_hash);
446+
m_qc_hashes_lru.insert(llmqType, block_hash, qc_hash);
451447
}
452448
if (rotation_enabled) {
453449
map_indexed_hashes[qc_hash.second] = qc_hash.first;
@@ -491,7 +487,7 @@ bool CQuorumBlockProcessor::UndoBlock(const CBlock& block, gsl::not_null<const C
491487
// Only once this commitment's state change is complete; see ProcessCommitment.
492488
DropQcHashesCache();
493489

494-
WITH_LOCK(minableCommitmentsCs, mapHasMinedCommitmentCache[qc.llmqType].erase(qc.quorumHash));
490+
WITH_LOCK(minableCommitmentsCs, mapHasMinedCommitmentCache.erase(qc.llmqType, qc.quorumHash));
495491

496492
// if a reorg happened, we should allow to mine this commitment later
497493
AddMineableCommitment(qc);
@@ -586,31 +582,14 @@ uint256 CQuorumBlockProcessor::GetQuorumBlockHash(const Consensus::LLMQParams& l
586582
bool CQuorumBlockProcessor::HasMinedCommitment(Consensus::LLMQType llmqType, const uint256& quorumHash) const
587583
{
588584
bool fExists;
589-
{
590-
// Defence-in-depth: this map is only pre-seeded by InitQuorumsCache() with the LLMQ types
591-
// from the chain's consensus params. operator[] with any other type would insert a
592-
// default-constructed, zero-capacity cache and abort in its constructor, so treat an
593-
// unregistered type as "no mined commitment" rather than indexing the map.
594-
LOCK(minableCommitmentsCs);
595-
auto it = mapHasMinedCommitmentCache.find(llmqType);
596-
if (it == mapHasMinedCommitmentCache.end()) {
597-
return false;
598-
}
599-
if (it->second.get(quorumHash, fExists)) {
600-
return fExists;
601-
}
585+
if (LOCK(minableCommitmentsCs); mapHasMinedCommitmentCache.get(llmqType, quorumHash, fExists)) {
586+
return fExists;
602587
}
603588

604589
fExists = m_evoDb.Exists(std::make_pair(DB_MINED_COMMITMENT, std::make_pair(llmqType, quorumHash)));
605590

606-
{
607-
LOCK(minableCommitmentsCs);
608-
// The key set is fixed at construction, so this can only miss if the type was unregistered,
609-
// which the check above already returned on.
610-
if (auto it = mapHasMinedCommitmentCache.find(llmqType); it != mapHasMinedCommitmentCache.end()) {
611-
it->second.insert(quorumHash, fExists);
612-
}
613-
}
591+
LOCK(minableCommitmentsCs);
592+
mapHasMinedCommitmentCache.insert(llmqType, quorumHash, fExists);
614593

615594
return fExists;
616595
}

‎src/llmq/blockprocessor.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66
#define BITCOIN_LLMQ_BLOCKPROCESSOR_H
77

88
#include <bls/bls.h>
9+
#include <llmq/cache.h>
910
#include <llmq/params.h>
1011
#include <llmq/utils.h>
1112
#include <msg_result.h>
12-
#include <unordered_lru_cache.h>
1313

1414
#include <checkqueue.h>
1515
#include <protocol.h>
@@ -58,7 +58,7 @@ class CQuorumBlockProcessor
5858
std::map<std::pair<Consensus::LLMQType, uint256>, uint256> minableCommitmentsByQuorum GUARDED_BY(minableCommitmentsCs);
5959
std::map<uint256, CFinalCommitment> minableCommitments GUARDED_BY(minableCommitmentsCs);
6060

61-
mutable std::map<Consensus::LLMQType, Uint256LruHashMap<bool>> mapHasMinedCommitmentCache GUARDED_BY(minableCommitmentsCs);
61+
mutable PerLlmqTypeCache<bool> mapHasMinedCommitmentCache GUARDED_BY(minableCommitmentsCs);
6262

6363
// Memoizes GetQcHashes(). The whole-result cache is keyed on the set of active
6464
// quorum base blocks, the LRU on those base-block hashes; neither key identifies
@@ -68,7 +68,7 @@ class CQuorumBlockProcessor
6868
// block index whose CBlockIndex* the outer cache stores.
6969
mutable Mutex m_qc_hashes_cache_mutex;
7070
mutable std::map<Consensus::LLMQType, std::vector<const CBlockIndex*>> m_quorums_cached GUARDED_BY(m_qc_hashes_cache_mutex);
71-
mutable std::map<Consensus::LLMQType, Uint256LruHashMap<std::pair<uint256, int>>> m_qc_hashes_lru GUARDED_BY(m_qc_hashes_cache_mutex);
71+
mutable PerLlmqTypeCache<std::pair<uint256, int>> m_qc_hashes_lru GUARDED_BY(m_qc_hashes_cache_mutex);
7272
mutable QcHashMap m_qc_hashes_cached GUARDED_BY(m_qc_hashes_cache_mutex);
7373
mutable QcIndexedHashMap m_qc_indexed_hashes_cached GUARDED_BY(m_qc_hashes_cache_mutex);
7474

‎src/llmq/cache.h‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
// Copyright (c) 2026 The Dash Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#ifndef BITCOIN_LLMQ_CACHE_H
6+
#define BITCOIN_LLMQ_CACHE_H
7+
8+
#include <consensus/params.h>
9+
#include <llmq/params.h>
10+
#include <saltedhasher.h>
11+
#include <uint256.h>
12+
#include <unordered_lru_cache.h>
13+
14+
#include <map>
15+
#include <utility>
16+
17+
namespace llmq {
18+
19+
//! A separate LRU cache per LLMQ type, sized from that type's consensus parameters.
20+
//!
21+
//! Only the types registered for the active chain get a cache. Consensus::LLMQType is a
22+
//! uint8_t enum that arrives over the wire, so callers may pass a type this chain does not
23+
//! use: those lookups miss and those writes are dropped, which is the same answer a cache
24+
//! that has never held such an entry would give.
25+
template <typename Value, typename Key = uint256>
26+
class PerLlmqTypeCache
27+
{
28+
private:
29+
using CacheType = unordered_lru_cache<Key, Value, StaticSaltedHasher>;
30+
31+
std::map<Consensus::LLMQType, CacheType> m_caches;
32+
33+
public:
34+
//! Creates a cache per registered type, sized by size_fn. Must be called before use;
35+
//! until then every type reads as absent.
36+
template <typename SizeFn>
37+
void Init(const Consensus::Params& consensus_params, SizeFn size_fn)
38+
{
39+
for (const auto& llmq : consensus_params.llmqs) {
40+
m_caches.emplace(std::piecewise_construct, std::forward_as_tuple(llmq.type),
41+
std::forward_as_tuple(size_fn(llmq)));
42+
}
43+
}
44+
45+
void Init(const Consensus::Params& consensus_params, bool limit_by_connections = true)
46+
{
47+
Init(consensus_params, [limit_by_connections](const Consensus::LLMQParams& llmq) {
48+
return limit_by_connections ? llmq.keepOldConnections : llmq.keepOldKeys;
49+
});
50+
}
51+
52+
bool IsInitialized() const { return !m_caches.empty(); }
53+
54+
bool get(Consensus::LLMQType llmqType, const Key& key, Value& value)
55+
{
56+
auto it = m_caches.find(llmqType);
57+
return it != m_caches.end() && it->second.get(key, value);
58+
}
59+
60+
void insert(Consensus::LLMQType llmqType, const Key& key, const Value& value)
61+
{
62+
if (auto it = m_caches.find(llmqType); it != m_caches.end()) {
63+
it->second.insert(key, value);
64+
}
65+
}
66+
67+
void emplace(Consensus::LLMQType llmqType, const Key& key, Value&& value)
68+
{
69+
if (auto it = m_caches.find(llmqType); it != m_caches.end()) {
70+
it->second.emplace(key, std::move(value));
71+
}
72+
}
73+
74+
void erase(Consensus::LLMQType llmqType, const Key& key)
75+
{
76+
if (auto it = m_caches.find(llmqType); it != m_caches.end()) {
77+
it->second.erase(key);
78+
}
79+
}
80+
81+
//! Drops cached entries but keeps the per-type caches, so Init is not needed again.
82+
void clear()
83+
{
84+
for (auto& [_, cache] : m_caches) {
85+
cache.clear();
86+
}
87+
}
88+
89+
void clear(Consensus::LLMQType llmqType)
90+
{
91+
if (auto it = m_caches.find(llmqType); it != m_caches.end()) {
92+
it->second.clear();
93+
}
94+
}
95+
96+
//! Capacity of a type's cache, or 0 if this chain does not use it.
97+
size_t max_size(Consensus::LLMQType llmqType) const
98+
{
99+
auto it = m_caches.find(llmqType);
100+
return it != m_caches.end() ? it->second.max_size() : 0;
101+
}
102+
};
103+
104+
} // namespace llmq
105+
106+
#endif // BITCOIN_LLMQ_CACHE_H

‎src/llmq/net_dkg.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -420,10 +420,10 @@ void NetDKG::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStre
420420
int quorumIndex{-1};
421421
{
422422
LOCK(cs_indexed_quorums_cache);
423-
if (indexed_quorums_cache.empty()) {
424-
utils::InitQuorumsCache(indexed_quorums_cache, m_chainman.GetConsensus());
423+
if (!indexed_quorums_cache.IsInitialized()) {
424+
indexed_quorums_cache.Init(m_chainman.GetConsensus());
425425
}
426-
indexed_quorums_cache[llmqType].get(quorumHash, quorumIndex);
426+
indexed_quorums_cache.get(llmqType, quorumHash, quorumIndex);
427427
}
428428

429429
if (quorumIndex == -1) {
@@ -532,7 +532,7 @@ void NetDKG::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStre
532532
return;
533533
}
534534

535-
WITH_LOCK(cs_indexed_quorums_cache, indexed_quorums_cache[llmqType].insert(quorumHash, quorumIndex));
535+
WITH_LOCK(cs_indexed_quorums_cache, indexed_quorums_cache.insert(llmqType, quorumHash, quorumIndex));
536536
}
537537

538538
bool NetDKG::AlreadyHave(const CInv& inv)

‎src/llmq/net_dkg.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66
#define BITCOIN_LLMQ_NET_DKG_H
77

88
#include <consensus/params.h>
9+
#include <llmq/cache.h>
910
#include <net_processing.h>
1011
#include <sync.h>
1112
#include <uint256.h>
12-
#include <unordered_lru_cache.h>
1313

1414
#include <map>
1515
#include <memory>
@@ -100,7 +100,7 @@ class NetDKG final : public NetHandler
100100

101101
/** Cache: quorum hash → quorum index, populated lazily by ProcessMessage. */
102102
mutable Mutex cs_indexed_quorums_cache;
103-
mutable std::map<Consensus::LLMQType, Uint256LruHashMap<int>> indexed_quorums_cache GUARDED_BY(cs_indexed_quorums_cache);
103+
mutable PerLlmqTypeCache<int> indexed_quorums_cache GUARDED_BY(cs_indexed_quorums_cache);
104104

105105
std::vector<std::thread> m_phase_threads;
106106
};

‎src/llmq/net_quorum.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -688,20 +688,19 @@ void NetQuorum::StartCleanupOldQuorumDataThread(gsl::not_null<const CBlockIndex*
688688
workerPool.push([pIndex, t, this](int threadId) {
689689
Uint256HashSet dbKeysToSkip;
690690

691-
if (LOCK(cs_cleanup); cleanupQuorumsCache.empty()) {
692-
utils::InitQuorumsCache(cleanupQuorumsCache, m_chainman.GetConsensus(), /*limit_by_connections=*/false);
691+
if (LOCK(cs_cleanup); !cleanupQuorumsCache.IsInitialized()) {
692+
cleanupQuorumsCache.Init(m_chainman.GetConsensus(), /*limit_by_connections=*/false);
693693
}
694694
for (const auto& params : Params().GetConsensus().llmqs) {
695695
if (quorumThreadInterrupt) {
696696
break;
697697
}
698698
LOCK(cs_cleanup);
699-
auto& cache = cleanupQuorumsCache[params.type];
700699
const CBlockIndex* pindex_loop{pIndex};
701700
Uint256HashSet quorum_keys;
702701
while (pindex_loop != nullptr && pIndex->nHeight - pindex_loop->nHeight < params.max_store_depth()) {
703702
uint256 quorum_key;
704-
if (cache.get(pindex_loop->GetBlockHash(), quorum_key)) {
703+
if (cleanupQuorumsCache.get(params.type, pindex_loop->GetBlockHash(), quorum_key)) {
705704
quorum_keys.insert(quorum_key);
706705
if (quorum_keys.size() >= static_cast<size_t>(params.keepOldKeys)) break; // extra safety belt
707706
}
@@ -710,7 +709,8 @@ void NetQuorum::StartCleanupOldQuorumDataThread(gsl::not_null<const CBlockIndex*
710709
for (const auto& pQuorum : m_qman.ScanQuorums(params.type, pIndex, params.keepOldKeys - quorum_keys.size())) {
711710
const uint256 quorum_key = MakeQuorumKey(*pQuorum);
712711
quorum_keys.insert(quorum_key);
713-
cache.insert(pQuorum->m_quorum_base_block_index->GetBlockHash(), quorum_key);
712+
cleanupQuorumsCache.insert(params.type, pQuorum->m_quorum_base_block_index->GetBlockHash(),
713+
quorum_key);
714714
}
715715
dbKeysToSkip.merge(quorum_keys);
716716
}

‎src/llmq/net_quorum.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55
#ifndef BITCOIN_LLMQ_NET_QUORUM_H
66
#define BITCOIN_LLMQ_NET_QUORUM_H
77

8+
#include <llmq/cache.h>
89
#include <llmq/options.h>
910
#include <llmq/quorums.h>
1011
#include <net_processing.h>
1112
#include <sync.h>
12-
#include <unordered_lru_cache.h>
1313
#include <util/threadinterrupt.h>
1414
#include <validationinterface.h>
1515

@@ -117,7 +117,7 @@ class NetQuorum final : public NetHandler, public CValidationInterface
117117
const bool m_quorums_recovery;
118118

119119
mutable Mutex cs_cleanup;
120-
mutable std::map<Consensus::LLMQType, Uint256LruHashMap<uint256>> cleanupQuorumsCache
120+
mutable PerLlmqTypeCache<uint256> cleanupQuorumsCache
121121
GUARDED_BY(cs_cleanup);
122122

123123
mutable ctpl::thread_pool workerPool;

0 commit comments

Comments
 (0)