Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: introduce two new cache structures to minimize cs_main cont… #6418

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft
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
2 changes: 1 addition & 1 deletion src/llmq/ehf_signals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ void CEHFSignalsHandler::trySignEHFSignal(int bit, const CBlockIndex* const pind
return;
}

const auto quorum = llmq::SelectQuorumForSigning(llmq_params_opt.value(), m_chainman.ActiveChain(), qman, requestId);
const auto quorum = qman.SelectQuorumForSigning(llmq_params_opt.value(), m_chainman.ActiveChain(), requestId);
if (!quorum) {
LogPrintf("CEHFSignalsHandler::trySignEHFSignal no quorum for id=%s\n", requestId.ToString());
return;
Expand Down
2 changes: 1 addition & 1 deletion src/llmq/instantsend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,7 @@ std::unordered_set<uint256, StaticSaltedHasher> CInstantSendManager::ProcessPend
nSignHeight = blockIndex->nHeight + dkgInterval - 1;
}

auto quorum = llmq::SelectQuorumForSigning(llmq_params, m_chainstate.m_chain, qman, id, nSignHeight, signOffset);
auto quorum = qman.SelectQuorumForSigning(llmq_params, m_chainstate.m_chain, id, nSignHeight, signOffset);
if (!quorum) {
// should not happen, but if one fails to select, all others will also fail to select
return {};
Expand Down
47 changes: 33 additions & 14 deletions src/llmq/quorums.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,8 @@ void CQuorumManager::TriggerQuorumDataRecoveryThreads(const CBlockIndex* pIndex)

void CQuorumManager::UpdatedBlockTip(const CBlockIndex* pindexNew, bool fInitialDownload) const
{
cachedChainTip = pindexNew;

if (!m_mn_sync.IsBlockchainSynced()) return;

for (const auto& params : Params().GetConsensus().llmqs) {
Expand Down Expand Up @@ -519,8 +521,7 @@ bool CQuorumManager::RequestQuorumData(CNode* pfrom, Consensus::LLMQType llmqTyp

std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, size_t nCountRequested) const
{
const CBlockIndex* pindex = WITH_LOCK(cs_main, return m_chainstate.m_chain.Tip());
return ScanQuorums(llmqType, pindex, nCountRequested);
return ScanQuorums(llmqType, cachedChainTip, nCountRequested);
}

std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, const CBlockIndex* pindexStart, size_t nCountRequested) const
Expand Down Expand Up @@ -633,7 +634,20 @@ std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqTyp

CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const uint256& quorumHash) const
{
const CBlockIndex* pQuorumBaseBlockIndex = WITH_LOCK(cs_main, return m_chainstate.m_blockman.LookupBlockIndex(quorumHash));
const CBlockIndex* pQuorumBaseBlockIndex = [&]() {
// Lock contention may still be high here; consider using a shared lock
// We cannot hold cs_quorumBaseBlockIndexCache the whole time as that creates lock-order inversion with cs_main;
// We cannot aquire cs_main if we have cs_quorumBaseBlockIndexCache held
const CBlockIndex* pindex;
if (!WITH_LOCK(cs_quorumBaseBlockIndexCache, return quorumBaseBlockIndexCache.get(quorumHash, pindex))) {
pindex = WITH_LOCK(cs_main, return m_chainstate.m_blockman.LookupBlockIndex(quorumHash));
if (pindex) {
LOCK(cs_quorumBaseBlockIndexCache);
quorumBaseBlockIndexCache.insert(quorumHash, pindex);
}
}
return pindex;
}();
if (!pQuorumBaseBlockIndex) {
LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- block %s not found\n", __func__, quorumHash.ToString());
return nullptr;
Expand Down Expand Up @@ -1187,26 +1201,31 @@ void CQuorumManager::MigrateOldQuorumDB(CEvoDB& evoDb) const
LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- done\n", __func__);
}

CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, const CChain& active_chain, const CQuorumManager& qman,
const uint256& selectionHash, int signHeight, int signOffset)
CQuorumCPtr CQuorumManager::SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, const CChain& active_chain,
const uint256& selectionHash, int signHeight, int signOffset) const
{
size_t poolSize = llmq_params.signingActiveQuorumCount;

CBlockIndex* pindexStart;
auto pindexStart = [&]() -> const CBlockIndex*
{
LOCK(cs_main);
auto chainTip = cachedChainTip.load();
Copy link
Collaborator

Choose a reason for hiding this comment

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

if cachedChainTip is null, better to load it from active_chain IMO

if (chainTip == nullptr) return nullptr;
auto cur_height = chainTip->nHeight;
if (signHeight == -1) {
signHeight = active_chain.Height();
signHeight = cur_height;
}
int startBlockHeight = signHeight - signOffset;
if (startBlockHeight > active_chain.Height() || startBlockHeight < 0) {
return {};
if (startBlockHeight > cur_height || startBlockHeight < 0) {
return nullptr;
}
pindexStart = active_chain[startBlockHeight];
return chainTip->GetAncestor(startBlockHeight);
}();
if (pindexStart == nullptr) {
return {};
}

if (IsQuorumRotationEnabled(llmq_params, pindexStart)) {
auto quorums = qman.ScanQuorums(llmq_params.type, pindexStart, poolSize);
auto quorums = ScanQuorums(llmq_params.type, pindexStart, poolSize);
if (quorums.empty()) {
return nullptr;
}
Expand All @@ -1230,7 +1249,7 @@ CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, con
}
return *itQuorum;
} else {
auto quorums = qman.ScanQuorums(llmq_params.type, pindexStart, poolSize);
auto quorums = ScanQuorums(llmq_params.type, pindexStart, poolSize);
if (quorums.empty()) {
return nullptr;
}
Expand All @@ -1255,7 +1274,7 @@ VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain
{
const auto& llmq_params_opt = Params().GetLLMQ(llmqType);
assert(llmq_params_opt.has_value());
auto quorum = SelectQuorumForSigning(llmq_params_opt.value(), active_chain, qman, id, signedAtHeight, signOffset);
auto quorum = qman.SelectQuorumForSigning(llmq_params_opt.value(), active_chain, id, signedAtHeight, signOffset);
if (!quorum) {
return VerifyRecSigStatus::NoQuorum;
}
Expand Down
22 changes: 14 additions & 8 deletions src/llmq/quorums.h
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,11 @@ class CQuorum
bool ReadContributions(const CDBWrapper& db);
};

// when selecting a quorum for signing and verification, we use CQuorumManager::SelectQuorum with this offset as
// starting height for scanning. This is because otherwise the resulting signatures would not be verifiable by nodes
// which are not 100% at the chain tip.
static constexpr int SIGN_HEIGHT_OFFSET{8};

/**
* The quorum manager maintains quorums which were mined on chain. When a quorum is requested from the manager,
* it will lookup the commitment (through CQuorumBlockProcessor) and build a CQuorum object from it.
Expand Down Expand Up @@ -252,6 +257,12 @@ class CQuorumManager
mutable Mutex cs_cleanup;
mutable std::map<Consensus::LLMQType, unordered_lru_cache<uint256, uint256, StaticSaltedHasher>> cleanupQuorumsCache GUARDED_BY(cs_cleanup);

mutable std::atomic<const CBlockIndex*> cachedChainTip{nullptr};

mutable Mutex cs_quorumBaseBlockIndexCache;
// On mainnet, we have around 62 quorums active at any point; let's cache a little more than double that to be safe.
mutable unordered_lru_cache<uint256 /*quorum_hash*/, const CBlockIndex* /*pindex*/, StaticSaltedHasher, 128 /*max_size*/> quorumBaseBlockIndexCache;

mutable ctpl::thread_pool workerPool;
mutable CThreadInterrupt quorumThreadInterrupt;

Expand Down Expand Up @@ -282,6 +293,9 @@ class CQuorumManager
// this one is cs_main-free
std::vector<CQuorumCPtr> ScanQuorums(Consensus::LLMQType llmqType, const CBlockIndex* pindexStart, size_t nCountRequested) const;

CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, const CChain& active_chain,
const uint256& selectionHash, int signHeight = -1 /*chain tip*/, int signOffset = SIGN_HEIGHT_OFFSET) const;

private:
// all private methods here are cs_main-free
void CheckQuorumConnections(const Consensus::LLMQParams& llmqParams, const CBlockIndex *pindexNew) const;
Expand All @@ -302,14 +316,6 @@ class CQuorumManager
void MigrateOldQuorumDB(CEvoDB& evoDb) const;
};

// when selecting a quorum for signing and verification, we use CQuorumManager::SelectQuorum with this offset as
// starting height for scanning. This is because otherwise the resulting signatures would not be verifiable by nodes
// which are not 100% at the chain tip.
static constexpr int SIGN_HEIGHT_OFFSET{8};

CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, const CChain& active_chain, const CQuorumManager& qman,
const uint256& selectionHash, int signHeight = -1 /*chain tip*/, int signOffset = SIGN_HEIGHT_OFFSET);

// Verifies a recovered sig that was signed while the chain tip was at signedAtTip
VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain& active_chain, const CQuorumManager& qman,
int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig,
Expand Down
2 changes: 1 addition & 1 deletion src/llmq/signing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigShares
// TODO fix this by re-signing when the next block arrives, but only when that block results in a change of the quorum list and no recovered signature has been created in the mean time
const auto &llmq_params_opt = Params().GetLLMQ(llmqType);
assert(llmq_params_opt.has_value());
return SelectQuorumForSigning(llmq_params_opt.value(), m_chainstate.m_chain, qman, id);
return qman.SelectQuorumForSigning(llmq_params_opt.value(), m_chainstate.m_chain, id);
} else {
return qman.GetQuorum(llmqType, quorumHash);
}
Expand Down
4 changes: 2 additions & 2 deletions src/rpc/quorums.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ static UniValue quorum_sign_helper(const JSONRPCRequest& request, Consensus::LLM
} else {
const auto pQuorum = [&]() {
if (quorumHash.IsNull()) {
return llmq::SelectQuorumForSigning(llmq_params_opt.value(), chainman.ActiveChain(), *llmq_ctx.qman, id);
return llmq_ctx.qman->SelectQuorumForSigning(llmq_params_opt.value(), chainman.ActiveChain(), id);
} else {
return llmq_ctx.qman->GetQuorum(llmqType, quorumHash);
}
Expand Down Expand Up @@ -724,7 +724,7 @@ static RPCHelpMan quorum_selectquorum()

UniValue ret(UniValue::VOBJ);

const auto quorum = llmq::SelectQuorumForSigning(llmq_params_opt.value(), chainman.ActiveChain(), *llmq_ctx.qman, id);
const auto quorum = llmq_ctx.qman->SelectQuorumForSigning(llmq_params_opt.value(), chainman.ActiveChain(), id);
if (!quorum) {
throw JSONRPCError(RPC_MISC_ERROR, "no quorums active");
}
Expand Down
Loading