Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ BITCOIN_CORE_H = \
dsnotificationinterface.h \
evo/assetlocktx.h \
evo/cbtx.h \
evo/cbtx_cache.h \
evo/chainhelper.h \
evo/creditpool.h \
evo/deterministicmns.h \
Expand Down
68 changes: 47 additions & 21 deletions src/evo/cbtx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include <evo/cbtx.h>

#include <evo/cbtx_cache.h>

#include <evo/specialtx.h>
#include <llmq/blockprocessor.h>
#include <llmq/commitment.h>
Expand Down Expand Up @@ -49,6 +51,35 @@ bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, TxValidationSta
using QcHashMap = std::map<Consensus::LLMQType, std::vector<uint256>>;
using QcIndexedHashMap = std::map<Consensus::LLMQType, std::map<int16_t, uint256>>;

// Process-lifetime caches for CalcCbTxMerkleRootQuorums.
//
// The outer whole-result cache is keyed only by the set of active quorum *base*
// blocks. The inner LRU is keyed only by those base-block hashes. Neither key
// includes the serialized CFinalCommitment that was actually mined for that
// base on the active chain. Different valid branches can therefore mine
// different commitments for the same base list, so these caches must be dropped
// whenever mined commitment state is undone (see InvalidateCachedQcHashes).
namespace {
GlobalMutex g_qc_hashes_cache_mutex;
std::map<Consensus::LLMQType, std::vector<const CBlockIndex*>> g_quorums_cached GUARDED_BY(g_qc_hashes_cache_mutex);
std::map<Consensus::LLMQType, Uint256LruHashMap<std::pair<uint256, int>>> g_qc_hashes_lru GUARDED_BY(g_qc_hashes_cache_mutex);
QcHashMap g_qcHashes_cached GUARDED_BY(g_qc_hashes_cache_mutex);
QcIndexedHashMap g_qcIndexedHashes_cached GUARDED_BY(g_qc_hashes_cache_mutex);
} // anonymous namespace

void InvalidateCachedQcHashes()
{
LOCK(g_qc_hashes_cache_mutex);
g_quorums_cached.clear();
g_qcHashes_cached.clear();
g_qcIndexedHashes_cached.clear();
// Clear per-type LRU contents but keep the map entries so InitQuorumsCache is
// not required on every post-invalidation miss.
for (auto& [_, cache] : g_qc_hashes_lru) {
cache.clear();
}
}

/**
* Handles the calculation or caching of qcHashes and qcIndexedHashes
* @param pindexPrev The const CBlockIndex* (ie a block) of a block. Both the Quorum list and quorum rotation activation status will be retrieved based on this block.
Expand All @@ -58,45 +89,40 @@ auto CachedGetQcHashesQcIndexedHashes(const CBlockIndex* pindexPrev, const llmq:
std::optional<std::pair<QcHashMap /*qcHashes*/, QcIndexedHashMap /*qcIndexedHashes*/>> {
auto quorums = quorum_block_processor.GetMinedAndActiveCommitmentsUntilBlock(pindexPrev);

static Mutex cs_cache;
static std::map<Consensus::LLMQType, std::vector<const CBlockIndex*>> quorums_cached GUARDED_BY(cs_cache);
static std::map<Consensus::LLMQType, Uint256LruHashMap<std::pair<uint256, int>>> qc_hashes_cached GUARDED_BY(cs_cache);
static QcHashMap qcHashes_cached GUARDED_BY(cs_cache);
static QcIndexedHashMap qcIndexedHashes_cached GUARDED_BY(cs_cache);

LOCK(cs_cache);
if (quorums == quorums_cached) {
return std::make_pair(qcHashes_cached, qcIndexedHashes_cached);
LOCK(g_qc_hashes_cache_mutex);
if (quorums == g_quorums_cached) {
return std::make_pair(g_qcHashes_cached, g_qcIndexedHashes_cached);
}

// Quorums set is different, reset cached values
quorums_cached.clear();
qcHashes_cached.clear();
qcIndexedHashes_cached.clear();
if (qc_hashes_cached.empty()) {
llmq::utils::InitQuorumsCache(qc_hashes_cached, Params().GetConsensus());
// Quorums set changed: rebuild whole-result caches. Keep the per-base LRU;
// branch-dependent staleness is handled by InvalidateCachedQcHashes().
g_quorums_cached.clear();
g_qcHashes_cached.clear();
g_qcIndexedHashes_cached.clear();
if (g_qc_hashes_lru.empty()) {
llmq::utils::InitQuorumsCache(g_qc_hashes_lru, Params().GetConsensus());
}

for (const auto& [llmqType, vecBlockIndexes] : quorums) {
const auto& llmq_params_opt = Params().GetLLMQ(llmqType);
assert(llmq_params_opt.has_value());
bool rotation_enabled = llmq::IsQuorumRotationEnabled(llmq_params_opt.value(), pindexPrev);
auto& vec_hashes = qcHashes_cached[llmqType];
auto& vec_hashes = g_qcHashes_cached[llmqType];
vec_hashes.reserve(vecBlockIndexes.size());
auto& map_indexed_hashes = qcIndexedHashes_cached[llmqType];
auto& map_indexed_hashes = g_qcIndexedHashes_cached[llmqType];
for (const auto& blockIndex : vecBlockIndexes) {
uint256 block_hash{blockIndex->GetBlockHash()};

std::pair<uint256, int> qc_hash;
if (!qc_hashes_cached[llmqType].get(block_hash, qc_hash)) {
if (!g_qc_hashes_lru[llmqType].get(block_hash, qc_hash)) {
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);
g_qc_hashes_lru[llmqType].insert(block_hash, qc_hash);
}
if (rotation_enabled) {
map_indexed_hashes[qc_hash.second] = qc_hash.first;
Expand All @@ -105,8 +131,8 @@ auto CachedGetQcHashesQcIndexedHashes(const CBlockIndex* pindexPrev, const llmq:
}
}
}
std::swap(quorums_cached, quorums);
return std::make_pair(qcHashes_cached, qcIndexedHashes_cached);
std::swap(g_quorums_cached, quorums);
return std::make_pair(g_qcHashes_cached, g_qcIndexedHashes_cached);
}

auto CalcHashCountFromQCHashes(const QcHashMap& qcHashes)
Expand Down
17 changes: 17 additions & 0 deletions src/evo/cbtx_cache.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// 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_EVO_CBTX_CACHE_H
#define BITCOIN_EVO_CBTX_CACHE_H

/**
* Drop process-lifetime CbTx quorum-commitment hash caches.
*
* Required when mined commitment data for a quorum base block can change without the
* active base-block list changing (e.g. disconnect/reorg of the block that mined a
* different valid CFinalCommitment for the same quorumHash).
*/
void InvalidateCachedQcHashes();

#endif // BITCOIN_EVO_CBTX_CACHE_H
9 changes: 9 additions & 0 deletions src/llmq/blockprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include <llmq/blockprocessor.h>

#include <evo/cbtx_cache.h>
#include <evo/evodb.h>
#include <evo/specialtx.h>
#include <llmq/commitment.h>
Expand Down Expand Up @@ -391,12 +392,14 @@ bool CQuorumBlockProcessor::UndoBlock(const CBlock& block, gsl::not_null<const C
return false;
}

bool undone_commitment{false};
for (auto& [_, qc2] : qcs) {
auto& qc = qc2; // cannot capture structured binding into lambda
if (qc.IsNull()) {
continue;
}

undone_commitment = true;
m_evoDb.Erase(std::make_pair(DB_MINED_COMMITMENT, std::make_pair(qc.llmqType, qc.quorumHash)));

const auto& llmq_params_opt = Params().GetLLMQ(qc.llmqType);
Expand All @@ -414,6 +417,12 @@ bool CQuorumBlockProcessor::UndoBlock(const CBlock& block, gsl::not_null<const C
AddMineableCommitment(qc);
}

// Drop both CbTx qc-hash cache layers: mined commitments are branch-dependent
// even when the active quorum base-block list is unchanged.
if (undone_commitment) {
InvalidateCachedQcHashes();
}
Comment on lines 417 to +424

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Squash the two iterative follow-up commits into the primary fix commit

I diffed all three commits directly. 5a142a8 introduces the cache-invalidation fix, an explanatory comment at the UndoBlock call site, and the regression tests. c39393a then trims that same comment down and simplifies the tests it just added (including removing an EraseMinedCommitment helper and inlining CBlockIndex/genesis setup). 52abbf3 immediately re-adds a shortened version of the identical comment c39393a had just deleted, word-for-word restating the same rationale ("branch-dependent even when the active quorum base-block list is unchanged"). None of these three commits is independently meaningful in permanent history: c39393a's comment removal is undone one commit later, and 52abbf3 exists solely to patch what c39393a just deleted. A future git blame/git log -p on this line will show a comment added, deleted, and re-added within the same PR — pure noise for bisect/blame with no reviewable content difference. Squash all three into one commit via interactive rebase before merge.

source: ['claude', 'codex']

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8a897dee04f: squashed the two iterative follow-ups into the primary fix, leaving one signed commit. The resulting tree is byte-identical to the reviewed 52abbf390d3d head; evo_cbtx_tests and git diff --check pass.


m_evoDb.Write(DB_BEST_BLOCK_UPGRADE, pindex->pprev->GetBlockHash());

return true;
Expand Down
Loading
Loading