From f83a1355a8c87c5e9d70fc956a09b5fef4b95994 Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 21 Nov 2025 12:47:48 -0600 Subject: [PATCH 1/2] refactor: segregate CreateSigShare for single member handling --- src/llmq/signing_shares.cpp | 62 +++++++++++++++++++++---------------- src/llmq/signing_shares.h | 2 ++ 2 files changed, 37 insertions(+), 27 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 43bc21af4af9..b6098e7b6f99 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -1645,46 +1645,54 @@ void CSigSharesManager::SignPendingSigShares() } } -std::optional CSigSharesManager::CreateSigShare(const CQuorum& quorum, const uint256& id, const uint256& msgHash) const +std::optional CSigSharesManager::CreateSigShareForSingleMember(const CQuorum& quorum, const uint256& id, const uint256& msgHash) const { cxxtimer::Timer t(true); auto activeMasterNodeProTxHash = m_mn_activeman.GetProTxHash(); - if (!quorum.IsValidMember(activeMasterNodeProTxHash)) { + int memberIdx = quorum.GetMemberIndex(activeMasterNodeProTxHash); + if (memberIdx == -1) { + // this should really not happen (IsValidMember gave true) return std::nullopt; } - 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), {}); + uint256 signHash = sigShare.buildSignHash().Get(); - 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 + // see TODO in CDKGSession::FinalizeSingleCommitment for details + auto bls_scheme = bls::bls_legacy_scheme.load(); + sigShare.sigShare.Set(m_mn_activeman.Sign(signHash, bls_scheme), bls_scheme); - // TODO: This one should be SIGN by QUORUM key, not by OPERATOR key - // see TODO in CDKGSession::FinalizeSingleCommitment for details - auto bls_scheme = bls::bls_legacy_scheme.load(); - sigShare.sigShare.Set(m_mn_activeman.Sign(signHash, bls_scheme), bls_scheme); + if (!sigShare.sigShare.Get().IsValid()) { + LogPrintf("CSigSharesManager::%s -- failed to sign sigShare. signHash=%s, id=%s, msgHash=%s, time=%s\n", + __func__, signHash.ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), + t.count()); + return std::nullopt; + } - if (!sigShare.sigShare.Get().IsValid()) { - LogPrintf("CSigSharesManager::%s -- failed to sign sigShare. signHash=%s, id=%s, msgHash=%s, time=%s\n", - __func__, signHash.ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), - t.count()); - return std::nullopt; - } + sigShare.UpdateKey(); - sigShare.UpdateKey(); + LogPrint(BCLog::LLMQ_SIGS, /* Continued */ + "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()); - LogPrint(BCLog::LLMQ_SIGS, /* Continued */ - "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()); + return sigShare; +} - return sigShare; +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)) { + return std::nullopt; + } + + if (quorum.params.is_single_member()) { + return CreateSigShareForSingleMember(quorum, id, msgHash); } const CBLSSecretKey& skShare = quorum.GetSkShare(); if (!skShare.IsValid()) { diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index d729fe31f442..24865fec98a8 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -452,6 +452,8 @@ class CSigSharesManager : public CRecoveredSigsListener void NotifyRecoveredSig(const std::shared_ptr& sig) const EXCLUSIVE_LOCKS_REQUIRED(!cs); private: + std::optional CreateSigShareForSingleMember(const CQuorum& quorum, const uint256& id, const uint256& msgHash) const; + // all of these return false when the currently processed message should be aborted (as each message actually contains multiple messages) bool ProcessMessageSigSesAnn(const CNode& pfrom, const CSigSesAnn& ann) EXCLUSIVE_LOCKS_REQUIRED(!cs); bool ProcessMessageSigSharesInv(const CNode& pfrom, const CSigSharesInv& inv) EXCLUSIVE_LOCKS_REQUIRED(!cs); From b21644657f4649610571916c08e60f81691db148 Mon Sep 17 00:00:00 2001 From: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com> Date: Mon, 24 Nov 2025 08:51:04 -0600 Subject: [PATCH 2/2] fix: avoid two timers Co-authored-by: Konstantin Akimov --- src/llmq/signing_shares.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index b6098e7b6f99..90c9ea3a95ed 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -1684,7 +1684,6 @@ std::optional CSigSharesManager::CreateSigShareForSingleMember(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)) { @@ -1694,6 +1693,7 @@ std::optional CSigSharesManager::CreateSigShare(const CQuorum& quorum if (quorum.params.is_single_member()) { return CreateSigShareForSingleMember(quorum, id, msgHash); } + cxxtimer::Timer t(true); 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());