Skip to content

Commit 0a08b2a

Browse files
refactor: enhance CSigSharesManager for single-member quorum handling
Added a new method, ProcessRecoveredSigUnlocked, to handle recovered signatures outside of the lock to improve thread safety. Updated TryRecoverSig to utilize this new method for single-member quorum cases, ensuring proper lock management during signature processing.
1 parent 582e6e4 commit 0a08b2a

File tree

2 files changed

+19
-4
lines changed

2 files changed

+19
-4
lines changed

src/llmq/signing_shares.cpp

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,7 @@ void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id,
780780

781781
std::vector<CBLSSignature> sigSharesForRecovery;
782782
std::vector<CBLSId> idsForRecovery;
783+
std::shared_ptr<CRecoveredSig> singleMemberRecoveredSig;
783784
{
784785
LOCK(cs);
785786

@@ -802,10 +803,10 @@ void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id,
802803
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- recover single-node signature. id=%s, msgHash=%s\n",
803804
__func__, id.ToString(), msgHash.ToString());
804805

805-
auto rs = std::make_shared<CRecoveredSig>(quorum.params.type, quorum.qc->quorumHash, id, msgHash,
806+
singleMemberRecoveredSig = std::make_shared<CRecoveredSig>(quorum.params.type, quorum.qc->quorumHash, id, msgHash,
806807
recoveredSig);
807-
sigman.ProcessRecoveredSig(rs, m_peerman);
808-
return; // end of single-quorum processing
808+
// Release lock before calling ProcessRecoveredSig to avoid double lock in HandleNewRecoveredSig
809+
// ProcessRecoveredSig will synchronously call HandleNewRecoveredSig which requires the lock
809810
}
810811

811812
sigSharesForRecovery.reserve((size_t) quorum.params.threshold);
@@ -822,6 +823,12 @@ void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id,
822823
}
823824
}
824825

826+
// Handle single-member quorum case after releasing the lock
827+
if (singleMemberRecoveredSig) {
828+
ProcessRecoveredSigUnlocked(singleMemberRecoveredSig);
829+
return; // end of single-quorum processing
830+
}
831+
825832
// now recover it
826833
cxxtimer::Timer t(true);
827834
CBLSSignature recoveredSig;
@@ -850,7 +857,12 @@ void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id,
850857
}
851858
}
852859

853-
sigman.ProcessRecoveredSig(rs, m_peerman);
860+
ProcessRecoveredSigUnlocked(rs);
861+
}
862+
863+
void CSigSharesManager::ProcessRecoveredSigUnlocked(const std::shared_ptr<const CRecoveredSig>& recoveredSig)
864+
{
865+
sigman.ProcessRecoveredSig(recoveredSig, m_peerman);
854866
}
855867

856868
CDeterministicMNCPtr CSigSharesManager::SelectMemberForRecovery(const CQuorum& quorum, const uint256 &id, int attempt)

src/llmq/signing_shares.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,9 @@ class CSigSharesManager : public CRecoveredSigsListener
477477
void ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum) EXCLUSIVE_LOCKS_REQUIRED(!cs);
478478
void TryRecoverSig(const CQuorum& quorum, const uint256& id, const uint256& msgHash) EXCLUSIVE_LOCKS_REQUIRED(!cs);
479479

480+
private:
481+
void ProcessRecoveredSigUnlocked(const std::shared_ptr<const CRecoveredSig>& recoveredSig) LOCKS_EXCLUDED(cs);
482+
480483
bool GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo)
481484
EXCLUSIVE_LOCKS_REQUIRED(!cs);
482485
static CSigShare RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const std::pair<uint16_t, CBLSLazySignature>& in);

0 commit comments

Comments
 (0)