Skip to content

feat: convert CSigSharesManager to use Mutex instead of RecursiveMutex #6954

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 13, 2025
Merged

feat: convert CSigSharesManager to use Mutex instead of RecursiveMutex #6954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8832775
feat: convert CSigSharesManager to use Mutex instead of RecursiveMutex
PastaPastaPasta Nov 10, 2025
582e6e4
chore: run clang-format
PastaPastaPasta Nov 10, 2025
0a08b2a
refactor: enhance CSigSharesManager for single-member quorum handling
PastaPastaPasta Nov 11, 2025
e98ba12
refactor: streamline signature recovery process in CSigSharesManager
PastaPastaPasta Nov 11, 2025
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
11 changes: 8 additions & 3 deletions src/llmq/signing_shares.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,7 @@ void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id,

std::vector<CBLSSignature> sigSharesForRecovery;
std::vector<CBLSId> idsForRecovery;
std::shared_ptr<CRecoveredSig> singleMemberRecoveredSig;
{
LOCK(cs);

Expand All @@ -802,10 +803,8 @@ void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id,
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<CRecoveredSig>(quorum.params.type, quorum.qc->quorumHash, id, msgHash,
singleMemberRecoveredSig = std::make_shared<CRecoveredSig>(quorum.params.type, quorum.qc->quorumHash, id, msgHash,
recoveredSig);
Comment on lines +806 to 807
Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix clang-format differences.

The pipeline reports clang-format differences on lines 803-809. Please run ./contrib/devtools/clang-format-diff.py -p1 and apply the suggested formatting for the CRecoveredSig construction.

🤖 Prompt for AI Agents 
In src/llmq/signing_shares.cpp around lines 803 to 809, the construction of
CRecoveredSig is misformatted per clang-format; run
./contrib/devtools/clang-format-diff.py -p1 and apply its suggested changes so
the std::make_shared<CRecoveredSig>(...) call matches the project's clang-format
style (line-wrapping and indentation of the argument list), ensuring the opening
parenthesis, arguments, and closing parenthesis are aligned as the tool
prescribes and then re-run the formatter to confirm no remaining diffs.

sigman.ProcessRecoveredSig(rs, m_peerman);
return; // end of single-quorum processing
}

sigSharesForRecovery.reserve((size_t) quorum.params.threshold);
Expand All @@ -822,6 +821,12 @@ void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id,
}
}

// Handle single-member quorum case after releasing the lock
if (singleMemberRecoveredSig) {
sigman.ProcessRecoveredSig(singleMemberRecoveredSig, m_peerman);
return; // end of single-quorum processing
}

// now recover it
cxxtimer::Timer t(true);
CBLSSignature recoveredSig;
Expand Down
68 changes: 37 additions & 31 deletions src/llmq/signing_shares.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ class CSigSharesManager : public CRecoveredSigsListener
static constexpr int64_t MAX_SEND_FOR_RECOVERY_TIMEOUT{10000};
static constexpr size_t MAX_MSGS_SIG_SHARES{32};

RecursiveMutex cs;
Mutex cs;

std::thread workThread;
CThreadInterrupt workInterrupt;
Expand Down Expand Up @@ -424,73 +424,79 @@ class CSigSharesManager : public CRecoveredSigsListener
const CQuorumManager& _qman, const CSporkManager& sporkman);
~CSigSharesManager() override;

void StartWorkerThread();
void StopWorkerThread();
void RegisterAsRecoveredSigsListener();
void UnregisterAsRecoveredSigsListener();
void InterruptWorkerThread();
void StartWorkerThread() EXCLUSIVE_LOCKS_REQUIRED(!cs);
void StopWorkerThread() EXCLUSIVE_LOCKS_REQUIRED(!cs);
void RegisterAsRecoveredSigsListener() EXCLUSIVE_LOCKS_REQUIRED(!cs);
void UnregisterAsRecoveredSigsListener() EXCLUSIVE_LOCKS_REQUIRED(!cs);
void InterruptWorkerThread() EXCLUSIVE_LOCKS_REQUIRED(!cs);

void ProcessMessage(const CNode& pnode, const std::string& msg_type, CDataStream& vRecv);
void ProcessMessage(const CNode& pnode, const std::string& msg_type, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs);

void AsyncSign(CQuorumCPtr quorum, const uint256& id, const uint256& msgHash)
EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns);
std::optional<CSigShare> 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);
EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns, !cs);
std::optional<CSigShare> CreateSigShare(const CQuorum& quorum, const uint256& id, const uint256& msgHash) const
EXCLUSIVE_LOCKS_REQUIRED(!cs);
void ForceReAnnouncement(const CQuorum& quorum, Consensus::LLMQType llmqType, const uint256& id,
const uint256& msgHash) EXCLUSIVE_LOCKS_REQUIRED(!cs);

[[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override;
[[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override
EXCLUSIVE_LOCKS_REQUIRED(!cs);

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,
bool allowDiffMsgHashSigning = false)
EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns);
bool allowDiffMsgHashSigning = false) EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns, !cs);

void NotifyRecoveredSig(const std::shared_ptr<const CRecoveredSig>& sig) const;
void NotifyRecoveredSig(const std::shared_ptr<const CRecoveredSig>& sig) const EXCLUSIVE_LOCKS_REQUIRED(!cs);

private:
// 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);
bool ProcessMessageSigSharesInv(const CNode& pfrom, const CSigSharesInv& inv);
bool ProcessMessageGetSigShares(const CNode& pfrom, const CSigSharesInv& inv);
bool ProcessMessageBatchedSigShares(const CNode& pfrom, const CBatchedSigShares& batchedSigShares);
void ProcessMessageSigShare(NodeId fromId, const CSigShare& sigShare);
bool ProcessMessageSigSesAnn(const CNode& pfrom, const CSigSesAnn& ann) EXCLUSIVE_LOCKS_REQUIRED(!cs);
bool ProcessMessageSigSharesInv(const CNode& pfrom, const CSigSharesInv& inv) EXCLUSIVE_LOCKS_REQUIRED(!cs);
bool ProcessMessageGetSigShares(const CNode& pfrom, const CSigSharesInv& inv) EXCLUSIVE_LOCKS_REQUIRED(!cs);
bool ProcessMessageBatchedSigShares(const CNode& pfrom, const CBatchedSigShares& batchedSigShares)
EXCLUSIVE_LOCKS_REQUIRED(!cs);
void ProcessMessageSigShare(NodeId fromId, const CSigShare& sigShare) EXCLUSIVE_LOCKS_REQUIRED(!cs);

static bool VerifySigSharesInv(Consensus::LLMQType llmqType, const CSigSharesInv& inv);
static bool PreVerifyBatchedSigShares(const CActiveMasternodeManager& mn_activeman, const CQuorumManager& quorum_manager,
const CSigSharesNodeState::SessionInfo& session, const CBatchedSigShares& batchedSigShares, bool& retBan);

bool CollectPendingSigSharesToVerify(
size_t maxUniqueSessions, std::unordered_map<NodeId, std::vector<CSigShare>>& retSigShares,
std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CQuorumCPtr, StaticSaltedHasher>& retQuorums);
bool ProcessPendingSigShares();
std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CQuorumCPtr, StaticSaltedHasher>& retQuorums)
EXCLUSIVE_LOCKS_REQUIRED(!cs);
bool ProcessPendingSigShares() EXCLUSIVE_LOCKS_REQUIRED(!cs);

void ProcessPendingSigShares(
const std::vector<CSigShare>& sigSharesToProcess,
const std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CQuorumCPtr, StaticSaltedHasher>& quorums);
const std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CQuorumCPtr, StaticSaltedHasher>& quorums)
EXCLUSIVE_LOCKS_REQUIRED(!cs);

void ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum);
void TryRecoverSig(const CQuorum& quorum, const uint256& id, const uint256& msgHash);
void ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum) EXCLUSIVE_LOCKS_REQUIRED(!cs);
void TryRecoverSig(const CQuorum& quorum, const uint256& id, const uint256& msgHash) EXCLUSIVE_LOCKS_REQUIRED(!cs);

bool GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo);
bool GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo)
EXCLUSIVE_LOCKS_REQUIRED(!cs);
static CSigShare RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const std::pair<uint16_t, CBLSLazySignature>& in);

void Cleanup();
void Cleanup() EXCLUSIVE_LOCKS_REQUIRED(!cs);
void RemoveSigSharesForSession(const uint256& signHash) EXCLUSIVE_LOCKS_REQUIRED(cs);
void RemoveBannedNodeStates();
void RemoveBannedNodeStates() EXCLUSIVE_LOCKS_REQUIRED(!cs);

void BanNode(NodeId nodeId);
void BanNode(NodeId nodeId) EXCLUSIVE_LOCKS_REQUIRED(!cs);

bool SendMessages();
bool SendMessages() EXCLUSIVE_LOCKS_REQUIRED(!cs);
void CollectSigSharesToRequest(std::unordered_map<NodeId, Uint256HashMap<CSigSharesInv>>& sigSharesToRequest)
EXCLUSIVE_LOCKS_REQUIRED(cs);
void CollectSigSharesToSend(std::unordered_map<NodeId, Uint256HashMap<CBatchedSigShares>>& sigSharesToSend)
EXCLUSIVE_LOCKS_REQUIRED(cs);
void CollectSigSharesToSendConcentrated(std::unordered_map<NodeId, std::vector<CSigShare>>& sigSharesToSend, const std::vector<CNode*>& vNodes) EXCLUSIVE_LOCKS_REQUIRED(cs);
void CollectSigSharesToAnnounce(std::unordered_map<NodeId, Uint256HashMap<CSigSharesInv>>& sigSharesToAnnounce)
EXCLUSIVE_LOCKS_REQUIRED(cs);
void SignPendingSigShares() EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns);
void WorkThreadMain() EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns);
void SignPendingSigShares() EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns, !cs);
void WorkThreadMain() EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns, !cs);
};
} // namespace llmq

Expand Down
Loading