From 5e888c4251df06b574a9d102eb58defb2f8910d8 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 20 Oct 2025 22:40:51 -0500 Subject: [PATCH 1/6] refactor: optimize AsyncSign by using std::move for quorum parameter --- src/llmq/signing_shares.cpp | 8 ++++---- src/llmq/signing_shares.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 4b070416b9bb6..bd26063f094e1 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -876,7 +876,7 @@ bool CSigSharesManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigning if (m_mn_activeman.GetProTxHash().IsNull()) return false; - const auto quorum = [&]() { + auto quorum = [&]() { if (quorumHash.IsNull()) { // This might end up giving different results on different members // This might happen when we are on the brink of confirming a new quorum @@ -943,7 +943,7 @@ bool CSigSharesManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigning // make us re-announce all known shares (other nodes might have run into a timeout) ForceReAnnouncement(quorum, llmqType, id, msgHash); } - AsyncSign(quorum, id, msgHash); + AsyncSign(std::move(quorum), id, msgHash); return true; } @@ -1599,10 +1599,10 @@ void CSigSharesManager::WorkThreadMain() } } -void CSigSharesManager::AsyncSign(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) +void CSigSharesManager::AsyncSign(CQuorumCPtr quorum, const uint256& id, const uint256& msgHash) { LOCK(cs_pendingSigns); - pendingSigns.emplace_back(quorum, id, msgHash); + pendingSigns.emplace_back(std::move(quorum), id, msgHash); } void CSigSharesManager::SignPendingSigShares() diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index fb2f692306fa3..b49a25c12c5d9 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -431,7 +431,7 @@ class CSigSharesManager : public CRecoveredSigsListener void ProcessMessage(const CNode& pnode, const std::string& msg_type, CDataStream& vRecv); - void AsyncSign(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) + void AsyncSign(CQuorumCPtr quorum, const uint256& id, const uint256& msgHash) EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns); std::optional CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) const; void ForceReAnnouncement(const CQuorumCPtr& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash); From 46856fc16d570d8706c0c43c348c51553c0d5296 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 20 Oct 2025 23:20:04 -0500 Subject: [PATCH 2/6] refactor: update RequestQuorumData and GetQuorumRecoveryStartOffset to use CQuorum reference --- src/llmq/quorums.cpp | 14 +++++++------- src/llmq/quorums.h | 4 ++-- src/rpc/quorums.cpp | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index c73395a010714..9fa8b8e1018f5 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -475,7 +475,7 @@ bool CQuorumManager::HasQuorum(Consensus::LLMQType llmqType, const CQuorumBlockP return quorum_block_processor.HasMinedCommitment(llmqType, quorumHash); } -bool CQuorumManager::RequestQuorumData(CNode* pfrom, CConnman& connman, CQuorumCPtr pQuorum, uint16_t nDataMask, +bool CQuorumManager::RequestQuorumData(CNode* pfrom, CConnman& connman, const CQuorum& quorum, uint16_t nDataMask, const uint256& proTxHash) const { if (pfrom == nullptr) { @@ -486,12 +486,12 @@ bool CQuorumManager::RequestQuorumData(CNode* pfrom, CConnman& connman, CQuorumC LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- pfrom is not a verified masternode\n", __func__); return false; } - const Consensus::LLMQType llmqType = pQuorum->qc->llmqType; + const Consensus::LLMQType llmqType = quorum.qc->llmqType; if (!Params().GetLLMQ(llmqType).has_value()) { LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Invalid llmqType: %d\n", __func__, ToUnderlying(llmqType)); return false; } - const CBlockIndex* pindex{pQuorum->m_quorum_base_block_index}; + const CBlockIndex* pindex{quorum.m_quorum_base_block_index}; if (pindex == nullptr) { LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Invalid m_quorum_base_block_index : nullptr\n", __func__); return false; @@ -673,7 +673,7 @@ CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, gsl::not_nul return BuildQuorumFromCommitment(llmqType, pQuorumBaseBlockIndex, populate_cache); } -size_t CQuorumManager::GetQuorumRecoveryStartOffset(const CQuorumCPtr pQuorum, const CBlockIndex* pIndex) const +size_t CQuorumManager::GetQuorumRecoveryStartOffset(const CQuorum& quorum, const CBlockIndex* pIndex) const { assert(m_mn_activeman); @@ -695,7 +695,7 @@ size_t CQuorumManager::GetQuorumRecoveryStartOffset(const CQuorumCPtr pQuorum, c } } } - return nIndex % pQuorum->qc->validMembers.size(); + return nIndex % quorum.qc->validMembers.size(); } MessageProcessingResult CQuorumManager::ProcessMessage(CNode& pfrom, CConnman& connman, std::string_view msg_type, CDataStream& vRecv) @@ -930,7 +930,7 @@ void CQuorumManager::StartQuorumDataRecoveryThread(CConnman& connman, const CQuo int64_t nTimeLastSuccess{0}; uint256* pCurrentMemberHash{nullptr}; std::vector vecMemberHashes; - const size_t nMyStartOffset{GetQuorumRecoveryStartOffset(pQuorum, pIndex)}; + const size_t nMyStartOffset{GetQuorumRecoveryStartOffset(*pQuorum, pIndex)}; const int64_t nRequestTimeout{10}; auto printLog = [&](const std::string& strMessage) { @@ -1006,7 +1006,7 @@ void CQuorumManager::StartQuorumDataRecoveryThread(CConnman& connman, const CQuo return; } - if (RequestQuorumData(pNode, connman, pQuorum, nDataMask, proTxHash)) { + if (RequestQuorumData(pNode, connman, *pQuorum, nDataMask, proTxHash)) { nTimeLastSuccess = GetTime().count(); printLog("Requested"); } else { diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h index 01a1e75dad755..ef31dfb002f17 100644 --- a/src/llmq/quorums.h +++ b/src/llmq/quorums.h @@ -280,7 +280,7 @@ class CQuorumManager static bool HasQuorum(Consensus::LLMQType llmqType, const CQuorumBlockProcessor& quorum_block_processor, const uint256& quorumHash); - bool RequestQuorumData(CNode* pfrom, CConnman& connman, const CQuorumCPtr pQuorum, uint16_t nDataMask, + bool RequestQuorumData(CNode* pfrom, CConnman& connman, const CQuorum& quorum, uint16_t nDataMask, const uint256& proTxHash = uint256()) const; // all these methods will lock cs_main for a short period of time @@ -309,7 +309,7 @@ class CQuorumManager /// Returns the start offset for the masternode with the given proTxHash. This offset is applied when picking data recovery members of a quorum's /// memberlist and is calculated based on a list of all member of all active quorums for the given llmqType in a way that each member /// should receive the same number of request if all active llmqType members requests data from one llmqType quorum. - size_t GetQuorumRecoveryStartOffset(const CQuorumCPtr pQuorum, const CBlockIndex* pIndex) const; + size_t GetQuorumRecoveryStartOffset(const CQuorum& quorum, const CBlockIndex* pIndex) const; void StartCachePopulatorThread(const CQuorumCPtr pQuorum) const; void StartQuorumDataRecoveryThread(CConnman& connman, const CQuorumCPtr pQuorum, const CBlockIndex* pIndex, diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index 16ce3eabc16ac..9cc576ba9ecd1 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -902,7 +902,7 @@ static RPCHelpMan quorum_getdata() throw JSONRPCError(RPC_INVALID_PARAMETER, "quorum not found"); } return connman.ForNode(nodeId, [&](CNode* pNode) { - return llmq_ctx.qman->RequestQuorumData(pNode, connman, quorum, nDataMask, proTxHash); + return llmq_ctx.qman->RequestQuorumData(pNode, connman, *quorum, nDataMask, proTxHash); }); }, }; From 5a6e64d773b8d0e0a1f052b3b32caec4cafb1ec3 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 20 Oct 2025 23:27:42 -0500 Subject: [PATCH 3/6] refactor: use std::move for CQuorumCPtr in StartQuorumDataRecoveryThread --- src/llmq/quorums.cpp | 6 +++--- src/llmq/quorums.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index 9fa8b8e1018f5..3bcc192b393fd 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -293,7 +293,7 @@ void CQuorumManager::TriggerQuorumDataRecoveryThreads(CConnman& connman, const C } // Finally start the thread which triggers the requests for this quorum - StartQuorumDataRecoveryThread(connman, pQuorum, pIndex, nDataMask); + StartQuorumDataRecoveryThread(connman, std::move(pQuorum), pIndex, nDataMask); } } } @@ -913,7 +913,7 @@ void CQuorumManager::StartCachePopulatorThread(const CQuorumCPtr pQuorum) const }); } -void CQuorumManager::StartQuorumDataRecoveryThread(CConnman& connman, const CQuorumCPtr pQuorum, +void CQuorumManager::StartQuorumDataRecoveryThread(CConnman& connman, CQuorumCPtr pQuorum, const CBlockIndex* pIndex, uint16_t nDataMaskIn) const { assert(m_mn_activeman); @@ -924,7 +924,7 @@ void CQuorumManager::StartQuorumDataRecoveryThread(CConnman& connman, const CQuo return; } - workerPool.push([&connman, pQuorum, pIndex, nDataMaskIn, this](int threadId) { + workerPool.push([&connman, pQuorum = std::move(pQuorum), pIndex, nDataMaskIn, this](int threadId) { size_t nTries{0}; uint16_t nDataMask{nDataMaskIn}; int64_t nTimeLastSuccess{0}; diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h index ef31dfb002f17..7029deb2aef60 100644 --- a/src/llmq/quorums.h +++ b/src/llmq/quorums.h @@ -312,7 +312,7 @@ class CQuorumManager size_t GetQuorumRecoveryStartOffset(const CQuorum& quorum, const CBlockIndex* pIndex) const; void StartCachePopulatorThread(const CQuorumCPtr pQuorum) const; - void StartQuorumDataRecoveryThread(CConnman& connman, const CQuorumCPtr pQuorum, const CBlockIndex* pIndex, + void StartQuorumDataRecoveryThread(CConnman& connman, CQuorumCPtr pQuorum, const CBlockIndex* pIndex, uint16_t nDataMask) const; void StartCleanupOldQuorumDataThread(const CBlockIndex* pIndex) const; From 382fd3c3e8b4bc4da6726abeb44d85f81632e8b9 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 20 Oct 2025 23:44:52 -0500 Subject: [PATCH 4/6] refactor: use std::move for CQuorumCPtr in StartCachePopulatorThread --- src/llmq/quorums.cpp | 4 ++-- src/llmq/quorums.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index 3bcc192b393fd..fd35d69299e55 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -883,7 +883,7 @@ MessageProcessingResult CQuorumManager::ProcessMessage(CNode& pfrom, CConnman& c return {}; } -void CQuorumManager::StartCachePopulatorThread(const CQuorumCPtr pQuorum) const +void CQuorumManager::StartCachePopulatorThread(CQuorumCPtr pQuorum) const { if (!pQuorum->HasVerificationVector()) { return; @@ -896,7 +896,7 @@ void CQuorumManager::StartCachePopulatorThread(const CQuorumCPtr pQuorum) const pQuorum->m_quorum_base_block_index->GetBlockHash().ToString()); // when then later some other thread tries to get keys, it will be much faster - workerPool.push([pQuorum, t, this](int threadId) { + workerPool.push([pQuorum = std::move(pQuorum), t, this](int threadId) { for (const auto i : irange::range(pQuorum->members.size())) { if (quorumThreadInterrupt) { break; diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h index 7029deb2aef60..cbbbdd5dccbfe 100644 --- a/src/llmq/quorums.h +++ b/src/llmq/quorums.h @@ -311,7 +311,7 @@ class CQuorumManager /// should receive the same number of request if all active llmqType members requests data from one llmqType quorum. size_t GetQuorumRecoveryStartOffset(const CQuorum& quorum, const CBlockIndex* pIndex) const; - void StartCachePopulatorThread(const CQuorumCPtr pQuorum) const; + void StartCachePopulatorThread(CQuorumCPtr pQuorum) const; void StartQuorumDataRecoveryThread(CConnman& connman, CQuorumCPtr pQuorum, const CBlockIndex* pIndex, uint16_t nDataMask) const; From 35a4f07371ad81e025b2553e919fbdd8a08ae88a Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 20 Oct 2025 23:45:44 -0500 Subject: [PATCH 5/6] refactor: replace CQuorumCPtr with CQuorum references in quorum-related functions --- src/llmq/signing_shares.cpp | 66 ++++++++++++++++++------------------- src/llmq/signing_shares.h | 8 ++--- src/rpc/quorums.cpp | 42 +++++++++++------------ 3 files changed, 58 insertions(+), 58 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index bd26063f094e1..f6d5ada3c0045 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -768,13 +768,13 @@ void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CQuorum } if (canTryRecovery) { - TryRecoverSig(quorum, sigShare.getId(), sigShare.getMsgHash()); + TryRecoverSig(*quorum, sigShare.getId(), sigShare.getMsgHash()); } } -void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) +void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id, const uint256& msgHash) { - if (sigman.HasRecoveredSigForId(quorum->params.type, id)) { + if (sigman.HasRecoveredSigForId(quorum.params.type, id)) { return; } @@ -783,13 +783,13 @@ void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256& { LOCK(cs); - auto signHash = SignHash(quorum->params.type, quorum->qc->quorumHash, id, msgHash).Get(); + auto signHash = SignHash(quorum.params.type, quorum.qc->quorumHash, id, msgHash).Get(); const auto* sigSharesForSignHash = sigShares.GetAllForSignHash(signHash); if (sigSharesForSignHash == nullptr) { return; } - if (quorum->params.is_single_member()) { + if (quorum.params.is_single_member()) { if (sigSharesForSignHash->empty()) { LogPrint(BCLog::LLMQ_SIGS, /* Continued */ "CSigSharesManager::%s -- impossible to recover single-node signature - no shares yet. id=%s, " @@ -802,22 +802,22 @@ void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256& LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- recover single-node signature. id=%s, msgHash=%s\n", __func__, id.ToString(), msgHash.ToString()); - auto rs = std::make_shared(quorum->params.type, quorum->qc->quorumHash, id, msgHash, + auto rs = std::make_shared(quorum.params.type, quorum.qc->quorumHash, id, msgHash, recoveredSig); sigman.ProcessRecoveredSig(rs, m_peerman); return; // end of single-quorum processing } - sigSharesForRecovery.reserve((size_t) quorum->params.threshold); - idsForRecovery.reserve((size_t) quorum->params.threshold); - for (auto it = sigSharesForSignHash->begin(); it != sigSharesForSignHash->end() && sigSharesForRecovery.size() < size_t(quorum->params.threshold); ++it) { + sigSharesForRecovery.reserve((size_t) quorum.params.threshold); + idsForRecovery.reserve((size_t) quorum.params.threshold); + for (auto it = sigSharesForSignHash->begin(); it != sigSharesForSignHash->end() && sigSharesForRecovery.size() < size_t(quorum.params.threshold); ++it) { const auto& sigShare = it->second; sigSharesForRecovery.emplace_back(sigShare.sigShare.Get()); - idsForRecovery.emplace_back(quorum->members[sigShare.getQuorumMember()]->proTxHash); + idsForRecovery.emplace_back(quorum.members[sigShare.getQuorumMember()]->proTxHash); } // check if we can recover the final signature - if (sigSharesForRecovery.size() < size_t(quorum->params.threshold)) { + if (sigSharesForRecovery.size() < size_t(quorum.params.threshold)) { return; } } @@ -834,14 +834,14 @@ void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256& LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- recovered signature. id=%s, msgHash=%s, time=%d\n", __func__, id.ToString(), msgHash.ToString(), t.count()); - auto rs = std::make_shared(quorum->params.type, quorum->qc->quorumHash, id, msgHash, recoveredSig); + auto rs = std::make_shared(quorum.params.type, quorum.qc->quorumHash, id, msgHash, recoveredSig); // There should actually be no need to verify the self-recovered signatures as it should always succeed. Let's // however still verify it from time to time, so that we have a chance to catch bugs. We do only this sporadic // verification because this is unbatched and thus slow verification that happens here. if (((recoveredSigsCounter++) % 100) == 0) { auto signHash = rs->buildSignHash(); - bool valid = recoveredSig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash.Get()); + bool valid = recoveredSig.VerifyInsecure(quorum.qc->quorumPublicKey, signHash.Get()); if (!valid) { // this should really not happen as we have verified all signature shares before LogPrintf("CSigSharesManager::%s -- own recovered signature is invalid. id=%s, msgHash=%s\n", __func__, @@ -853,13 +853,13 @@ void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256& sigman.ProcessRecoveredSig(rs, m_peerman); } -CDeterministicMNCPtr CSigSharesManager::SelectMemberForRecovery(const CQuorumCPtr& quorum, const uint256 &id, int attempt) +CDeterministicMNCPtr CSigSharesManager::SelectMemberForRecovery(const CQuorum& quorum, const uint256 &id, int attempt) { - assert(attempt < quorum->params.recoveryMembers); + assert(attempt < quorum.params.recoveryMembers); std::vector> v; - v.reserve(quorum->members.size()); - for (const auto& dmn : quorum->members) { + v.reserve(quorum.members.size()); + for (const auto& dmn : quorum.members) { auto h = ::SerializeHash(std::make_pair(dmn->proTxHash, id)); v.emplace_back(h, dmn); } @@ -941,7 +941,7 @@ bool CSigSharesManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigning if (allowReSign) { // make us re-announce all known shares (other nodes might have run into a timeout) - ForceReAnnouncement(quorum, llmqType, id, msgHash); + ForceReAnnouncement(*quorum, llmqType, id, msgHash); } AsyncSign(std::move(quorum), id, msgHash); @@ -1128,7 +1128,7 @@ void CSigSharesManager::CollectSigSharesToSendConcentrated(std::unordered_mapsigShare.Get().IsValid()) { auto sigShare = *opt_sigShare; @@ -1629,23 +1629,23 @@ void CSigSharesManager::SignPendingSigShares() } } -std::optional CSigSharesManager::CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) const +std::optional CSigSharesManager::CreateSigShare(const CQuorum& quorum, const uint256& id, const uint256& msgHash) const { cxxtimer::Timer t(true); auto activeMasterNodeProTxHash = m_mn_activeman.GetProTxHash(); - if (!quorum->IsValidMember(activeMasterNodeProTxHash)) { + if (!quorum.IsValidMember(activeMasterNodeProTxHash)) { return std::nullopt; } - if (quorum->params.is_single_member()) { - int memberIdx = quorum->GetMemberIndex(activeMasterNodeProTxHash); + if (quorum.params.is_single_member()) { + int memberIdx = quorum.GetMemberIndex(activeMasterNodeProTxHash); if (memberIdx == -1) { // this should really not happen (IsValidMember gave true) return std::nullopt; } - CSigShare sigShare(quorum->params.type, quorum->qc->quorumHash, id, msgHash, uint16_t(memberIdx), {}); + CSigShare sigShare(quorum.params.type, quorum.qc->quorumHash, id, msgHash, uint16_t(memberIdx), {}); uint256 signHash = sigShare.buildSignHash().Get(); // TODO: This one should be SIGN by QUORUM key, not by OPERATOR key @@ -1665,23 +1665,23 @@ std::optional CSigSharesManager::CreateSigShare(const CQuorumCPtr& qu "CSigSharesManager::%s -- created sigShare. signHash=%s, id=%s, msgHash=%s, llmqType=%d, quorum=%s, " "time=%s\n", __func__, signHash.ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), - ToUnderlying(quorum->params.type), quorum->qc->quorumHash.ToString(), t.count()); + ToUnderlying(quorum.params.type), quorum.qc->quorumHash.ToString(), t.count()); return sigShare; } - const CBLSSecretKey& skShare = quorum->GetSkShare(); + const CBLSSecretKey& skShare = quorum.GetSkShare(); if (!skShare.IsValid()) { - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have our skShare for quorum %s\n", __func__, quorum->qc->quorumHash.ToString()); + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have our skShare for quorum %s\n", __func__, quorum.qc->quorumHash.ToString()); return std::nullopt; } - int memberIdx = quorum->GetMemberIndex(activeMasterNodeProTxHash); + int memberIdx = quorum.GetMemberIndex(activeMasterNodeProTxHash); if (memberIdx == -1) { // this should really not happen (IsValidMember gave true) return std::nullopt; } - CSigShare sigShare(quorum->params.type, quorum->qc->quorumHash, id, msgHash, uint16_t(memberIdx), {}); + CSigShare sigShare(quorum.params.type, quorum.qc->quorumHash, id, msgHash, uint16_t(memberIdx), {}); uint256 signHash = sigShare.buildSignHash().Get(); sigShare.sigShare.Set(skShare.Sign(signHash, bls::bls_legacy_scheme.load()), bls::bls_legacy_scheme.load()); @@ -1694,20 +1694,20 @@ std::optional CSigSharesManager::CreateSigShare(const CQuorumCPtr& qu sigShare.UpdateKey(); LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- created sigShare. signHash=%s, id=%s, msgHash=%s, llmqType=%d, quorum=%s, time=%s\n", __func__, - signHash.ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), ToUnderlying(quorum->params.type), quorum->qc->quorumHash.ToString(), t.count()); + signHash.ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), ToUnderlying(quorum.params.type), quorum.qc->quorumHash.ToString(), t.count()); return sigShare; } // causes all known sigShares to be re-announced -void CSigSharesManager::ForceReAnnouncement(const CQuorumCPtr& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash) +void CSigSharesManager::ForceReAnnouncement(const CQuorum& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash) { if (IsAllMembersConnectedEnabled(llmqType, m_sporkman)) { return; } LOCK(cs); - auto signHash = SignHash(llmqType, quorum->qc->quorumHash, id, msgHash).Get(); + auto signHash = SignHash(llmqType, quorum.qc->quorumHash, id, msgHash).Get(); if (const auto *const sigs = sigShares.GetAllForSignHash(signHash)) { for (const auto& [quorumMemberIndex, _] : *sigs) { // re-announce every sigshare to every node diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index b49a25c12c5d9..9d3084002667a 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -433,12 +433,12 @@ class CSigSharesManager : public CRecoveredSigsListener void AsyncSign(CQuorumCPtr quorum, const uint256& id, const uint256& msgHash) EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns); - std::optional CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) const; - void ForceReAnnouncement(const CQuorumCPtr& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash); + std::optional CreateSigShare(const CQuorum& quorum, const uint256& id, const uint256& msgHash) const; + void ForceReAnnouncement(const CQuorum& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash); [[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override; - static CDeterministicMNCPtr SelectMemberForRecovery(const CQuorumCPtr& quorum, const uint256& id, int attempt); + static CDeterministicMNCPtr SelectMemberForRecovery(const CQuorum& quorum, const uint256& id, int attempt); bool AsyncSignIfMember(Consensus::LLMQType llmqType, CSigningManager& sigman, const uint256& id, const uint256& msgHash, const uint256& quorumHash = uint256(), bool allowReSign = false, @@ -469,7 +469,7 @@ class CSigSharesManager : public CRecoveredSigsListener const std::unordered_map, CQuorumCPtr, StaticSaltedHasher>& quorums); void ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum); - void TryRecoverSig(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash); + void TryRecoverSig(const CQuorum& quorum, const uint256& id, const uint256& msgHash); bool GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo); static CSigShare RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const std::pair& in); diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index 9cc576ba9ecd1..fb837dd0f1c34 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -180,20 +180,20 @@ static RPCHelpMan quorum_list_extended() } static UniValue BuildQuorumInfo(const llmq::CQuorumBlockProcessor& quorum_block_processor, - const llmq::CQuorumCPtr& quorum, bool includeMembers, bool includeSkShare) + const llmq::CQuorum& quorum, bool includeMembers, bool includeSkShare) { UniValue ret(UniValue::VOBJ); - ret.pushKV("height", quorum->m_quorum_base_block_index->nHeight); - ret.pushKV("type", std::string(quorum->params.name)); - ret.pushKV("quorumHash", quorum->qc->quorumHash.ToString()); - ret.pushKV("quorumIndex", quorum->qc->quorumIndex); - ret.pushKV("minedBlock", quorum->minedBlockHash.ToString()); + ret.pushKV("height", quorum.m_quorum_base_block_index->nHeight); + ret.pushKV("type", std::string(quorum.params.name)); + ret.pushKV("quorumHash", quorum.qc->quorumHash.ToString()); + ret.pushKV("quorumIndex", quorum.qc->quorumIndex); + ret.pushKV("minedBlock", quorum.minedBlockHash.ToString()); - if (quorum->params.useRotation) { - auto previousActiveCommitment = quorum_block_processor.GetLastMinedCommitmentsByQuorumIndexUntilBlock(quorum->params.type, quorum->m_quorum_base_block_index, quorum->qc->quorumIndex, 0); + if (quorum.params.useRotation) { + auto previousActiveCommitment = quorum_block_processor.GetLastMinedCommitmentsByQuorumIndexUntilBlock(quorum.params.type, quorum.m_quorum_base_block_index, quorum.qc->quorumIndex, 0); if (previousActiveCommitment.has_value()) { - int previousConsecutiveDKGFailures = (quorum->m_quorum_base_block_index->nHeight - previousActiveCommitment.value()->nHeight) / quorum->params.dkgInterval - 1; + int previousConsecutiveDKGFailures = (quorum.m_quorum_base_block_index->nHeight - previousActiveCommitment.value()->nHeight) / quorum.params.dkgInterval - 1; ret.pushKV("previousConsecutiveDKGFailures", previousConsecutiveDKGFailures); } else { @@ -203,19 +203,19 @@ static UniValue BuildQuorumInfo(const llmq::CQuorumBlockProcessor& quorum_block_ if (includeMembers) { UniValue membersArr(UniValue::VARR); - for (size_t i = 0; i < quorum->members.size(); i++) { - const auto& dmn = quorum->members[i]; + for (size_t i = 0; i < quorum.members.size(); i++) { + const auto& dmn = quorum.members[i]; UniValue mo(UniValue::VOBJ); mo.pushKV("proTxHash", dmn->proTxHash.ToString()); mo.pushKV("service", dmn->pdmnState->netInfo->GetPrimary().ToStringAddrPort()); mo.pushKV("addresses", GetNetInfoWithLegacyFields(*dmn->pdmnState, dmn->nType)); mo.pushKV("pubKeyOperator", dmn->pdmnState->pubKeyOperator.ToString()); - mo.pushKV("valid", static_cast(quorum->qc->validMembers[i])); - if (quorum->qc->validMembers[i]) { - if (quorum->params.is_single_member()) { + mo.pushKV("valid", static_cast(quorum.qc->validMembers[i])); + if (quorum.qc->validMembers[i]) { + if (quorum.params.is_single_member()) { mo.pushKV("pubKeyShare", dmn->pdmnState->pubKeyOperator.ToString()); } else { - CBLSPublicKey pubKey = quorum->GetPubKeyShare(i); + CBLSPublicKey pubKey = quorum.GetPubKeyShare(i); if (pubKey.IsValid()) { mo.pushKV("pubKeyShare", pubKey.ToString()); } @@ -226,8 +226,8 @@ static UniValue BuildQuorumInfo(const llmq::CQuorumBlockProcessor& quorum_block_ ret.pushKV("members", membersArr); } - ret.pushKV("quorumPublicKey", quorum->qc->quorumPublicKey.ToString()); - const CBLSSecretKey& skShare = quorum->GetSkShare(); + ret.pushKV("quorumPublicKey", quorum.qc->quorumPublicKey.ToString()); + const CBLSSecretKey& skShare = quorum.GetSkShare(); if (includeSkShare && skShare.IsValid()) { ret.pushKV("secretKeyShare", skShare.ToString()); } @@ -271,7 +271,7 @@ static RPCHelpMan quorum_info() throw JSONRPCError(RPC_INVALID_PARAMETER, "quorum not found"); } - return BuildQuorumInfo(*llmq_ctx.quorum_block_processor, quorum, true, includeSkShare); + return BuildQuorumInfo(*llmq_ctx.quorum_block_processor, *quorum, true, includeSkShare); }, }; } @@ -467,7 +467,7 @@ static RPCHelpMan quorum_memberof() auto quorums = llmq_ctx.qman->ScanQuorums(llmq_params_opt->type, count); for (auto& quorum : quorums) { if (quorum->IsMember(dmn->proTxHash)) { - auto json = BuildQuorumInfo(*llmq_ctx.quorum_block_processor, quorum, false, false); + auto json = BuildQuorumInfo(*llmq_ctx.quorum_block_processor, *quorum, false, false); json.pushKV("isValidMember", quorum->IsValidMember(dmn->proTxHash)); json.pushKV("memberIndex", quorum->GetMemberIndex(dmn->proTxHash)); result.push_back(json); @@ -521,7 +521,7 @@ static UniValue quorum_sign_helper(const JSONRPCRequest& request, Consensus::LLM throw JSONRPCError(RPC_INVALID_PARAMETER, "quorum not found"); } - auto sigShare = CHECK_NONFATAL(node.active_ctx)->shareman->CreateSigShare(pQuorum, id, msgHash); + auto sigShare = CHECK_NONFATAL(node.active_ctx)->shareman->CreateSigShare(*pQuorum, id, msgHash); if (!sigShare.has_value() || !sigShare->sigShare.Get().IsValid()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "failed to create sigShare"); @@ -815,7 +815,7 @@ static RPCHelpMan quorum_selectquorum() UniValue recoveryMembers(UniValue::VARR); for (int i = 0; i < quorum->params.recoveryMembers; ++i) { - auto dmn = llmq::CSigSharesManager::SelectMemberForRecovery(quorum, id, i); + auto dmn = llmq::CSigSharesManager::SelectMemberForRecovery(*quorum, id, i); recoveryMembers.push_back(dmn->proTxHash.ToString()); } ret.pushKV("recoveryMembers", recoveryMembers); From 6b4973046a768b2ed29d59795b8cf93be90d8c46 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 21 Oct 2025 09:06:35 -0500 Subject: [PATCH 6/6] refactor: use non-const reference for pQuorum in quorum iteration loop to allow moves --- src/llmq/quorums.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index fd35d69299e55..cf8d01db4ac96 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -257,7 +257,7 @@ void CQuorumManager::TriggerQuorumDataRecoveryThreads(CConnman& connman, const C LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- Process block %s\n", __func__, pIndex->GetBlockHash().ToString()); for (const auto& params : Params().GetConsensus().llmqs) { - const auto vecQuorums = ScanQuorums(params.type, pIndex, params.keepOldConnections); + auto vecQuorums = ScanQuorums(params.type, pIndex, params.keepOldConnections); // First check if we are member of any quorum of this type const uint256 proTxHash = m_mn_activeman != nullptr ? m_mn_activeman->GetProTxHash() : uint256(); @@ -266,7 +266,7 @@ void CQuorumManager::TriggerQuorumDataRecoveryThreads(CConnman& connman, const C return pQuorum->IsValidMember(proTxHash); }); - for (const auto& pQuorum : vecQuorums) { + for (auto& pQuorum : vecQuorums) { // If there is already a thread running for this specific quorum skip it if (pQuorum->fQuorumDataRecoveryThreadRunning) { continue;