From 6563ade3c70dd10572061081cc1991bbb6eb7fde Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 8 Sep 2025 23:27:47 +0000 Subject: [PATCH 1/7] refactor: remove unused `CSigningManager` from `CChainLocksHandler` ctor --- src/chainlock/chainlock.cpp | 4 ++-- src/chainlock/chainlock.h | 4 ++-- src/llmq/context.cpp | 3 +-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/chainlock/chainlock.cpp b/src/chainlock/chainlock.cpp index d87dcb52ba1b1..bb4380170cf0a 100644 --- a/src/chainlock/chainlock.cpp +++ b/src/chainlock/chainlock.cpp @@ -43,8 +43,8 @@ bool AreChainLocksEnabled(const CSporkManager& sporkman) return sporkman.IsSporkActive(SPORK_19_CHAINLOCKS_ENABLED); } -CChainLocksHandler::CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSigningManager& _sigman, - CSporkManager& sporkman, CTxMemPool& _mempool, const CMasternodeSync& mn_sync) : +CChainLocksHandler::CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSporkManager& sporkman, + CTxMemPool& _mempool, const CMasternodeSync& mn_sync) : m_chainstate{chainstate}, qman{_qman}, spork_manager{sporkman}, diff --git a/src/chainlock/chainlock.h b/src/chainlock/chainlock.h index a9f3752236361..ab6b0a1085829 100644 --- a/src/chainlock/chainlock.h +++ b/src/chainlock/chainlock.h @@ -66,8 +66,8 @@ class CChainLocksHandler final : public chainlock::ChainLockSignerParent std::atomic lastCleanupTime{0s}; public: - explicit CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSigningManager& _sigman, - CSporkManager& sporkman, CTxMemPool& _mempool, const CMasternodeSync& mn_sync); + explicit CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSporkManager& sporkman, + CTxMemPool& _mempool, const CMasternodeSync& mn_sync); ~CChainLocksHandler(); void ConnectSigner(gsl::not_null signer) diff --git a/src/llmq/context.cpp b/src/llmq/context.cpp index 75e98f40bbfbe..c6056f1741ab9 100644 --- a/src/llmq/context.cpp +++ b/src/llmq/context.cpp @@ -34,8 +34,7 @@ LLMQContext::LLMQContext(ChainstateManager& chainman, CDeterministicMNManager& d unit_tests, wipe)}, sigman{std::make_unique(mn_activeman, chainman.ActiveChainstate(), *qman, unit_tests, wipe)}, shareman{std::make_unique(*sigman, mn_activeman, *qman, sporkman)}, - clhandler{std::make_unique(chainman.ActiveChainstate(), *qman, *sigman, sporkman, mempool, - mn_sync)}, + clhandler{std::make_unique(chainman.ActiveChainstate(), *qman, sporkman, mempool, mn_sync)}, isman{std::make_unique(*clhandler, chainman.ActiveChainstate(), *qman, *sigman, sporkman, mempool, mn_sync, unit_tests, wipe)} { From 74afa2840331d908e69e0ccaf71a1e59e3b3f3bd Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 22 Oct 2025 17:18:13 +0530 Subject: [PATCH 2/7] refactor: move `CSigSharesManager` to `ActiveContext` --- src/init.cpp | 5 ++++ src/llmq/context.cpp | 7 ------ src/llmq/context.h | 2 -- src/masternode/active/context.cpp | 33 +++++++++++++++++++------ src/masternode/active/context.h | 21 ++++++++++------ src/net_processing.cpp | 2 +- src/rpc/quorums.cpp | 12 ++++++--- test/lint/lint-circular-dependencies.py | 1 + 8 files changed, 56 insertions(+), 27 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 6b73e69bb862e..c83bd17c4581c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -231,6 +231,9 @@ void Interrupt(NodeContext& node) InterruptRPC(); InterruptREST(); InterruptTorControl(); + if (node.active_ctx) { + node.active_ctx->Interrupt(); + } if (node.llmq_ctx) { node.llmq_ctx->Interrupt(); } @@ -266,6 +269,7 @@ void PrepareShutdown(NodeContext& node) StopREST(); StopRPC(); StopHTTPServer(); + if (node.active_ctx) node.active_ctx->Stop(); if (node.llmq_ctx) node.llmq_ctx->Stop(); for (const auto& client : node.chain_clients) { @@ -2253,6 +2257,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 10a: schedule Dash-specific tasks node.llmq_ctx->Start(*node.connman, *node.peerman); + if (node.active_ctx) node.active_ctx->Start(*node.connman, *node.peerman); node.scheduler->scheduleEvery(std::bind(&CNetFulfilledRequestManager::DoMaintenance, std::ref(*node.netfulfilledman)), std::chrono::minutes{1}); node.scheduler->scheduleEvery(std::bind(&CMasternodeSync::DoMaintenance, std::ref(*node.mn_sync), std::cref(*node.peerman), std::cref(*node.govman)), std::chrono::seconds{1}); diff --git a/src/llmq/context.cpp b/src/llmq/context.cpp index c6056f1741ab9..35feaae44e46d 100644 --- a/src/llmq/context.cpp +++ b/src/llmq/context.cpp @@ -12,7 +12,6 @@ #include #include #include -#include #include #include @@ -33,7 +32,6 @@ LLMQContext::LLMQContext(ChainstateManager& chainman, CDeterministicMNManager& d *quorum_block_processor, *qsnapman, mn_activeman, mn_sync, sporkman, unit_tests, wipe)}, sigman{std::make_unique(mn_activeman, chainman.ActiveChainstate(), *qman, unit_tests, wipe)}, - shareman{std::make_unique(*sigman, mn_activeman, *qman, sporkman)}, clhandler{std::make_unique(chainman.ActiveChainstate(), *qman, sporkman, mempool, mn_sync)}, isman{std::make_unique(*clhandler, chainman.ActiveChainstate(), *qman, *sigman, sporkman, mempool, mn_sync, unit_tests, wipe)} @@ -48,7 +46,6 @@ LLMQContext::~LLMQContext() { void LLMQContext::Interrupt() { isman->InterruptWorkerThread(); - shareman->InterruptWorkerThread(); sigman->InterruptWorkerThread(); } @@ -59,8 +56,6 @@ void LLMQContext::Start(CConnman& connman, PeerManager& peerman) } qman->Start(); sigman->StartWorkerThread(peerman); - shareman->RegisterAsRecoveredSigsListener(); - shareman->StartWorkerThread(connman, peerman); clhandler->Start(*isman); isman->Start(peerman); } @@ -68,8 +63,6 @@ void LLMQContext::Start(CConnman& connman, PeerManager& peerman) void LLMQContext::Stop() { isman->Stop(); clhandler->Stop(); - shareman->StopWorkerThread(); - shareman->UnregisterAsRecoveredSigsListener(); sigman->StopWorkerThread(); qman->Stop(); if (is_masternode) { diff --git a/src/llmq/context.h b/src/llmq/context.h index d27b2a7a8137a..e1864e22c17d5 100644 --- a/src/llmq/context.h +++ b/src/llmq/context.h @@ -28,7 +28,6 @@ class CInstantSendManager; class CQuorumBlockProcessor; class CQuorumManager; class CQuorumSnapshotManager; -class CSigSharesManager; class CSigningManager; } @@ -66,7 +65,6 @@ struct LLMQContext { const std::unique_ptr qdkgsman; const std::unique_ptr qman; const std::unique_ptr sigman; - const std::unique_ptr shareman; const std::unique_ptr clhandler; const std::unique_ptr isman; }; diff --git a/src/masternode/active/context.cpp b/src/masternode/active/context.cpp index 23ff1e24ba3d4..6fe4170dd6e24 100644 --- a/src/masternode/active/context.cpp +++ b/src/masternode/active/context.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include ActiveContext::ActiveContext(ChainstateManager& chainman, CConnman& connman, CDeterministicMNManager& dmnman, @@ -21,16 +22,17 @@ ActiveContext::ActiveContext(ChainstateManager& chainman, CConnman& connman, CDe PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, const CMasternodeSync& mn_sync) : m_llmq_ctx{llmq_ctx}, - cl_signer{std::make_unique(chainman.ActiveChainstate(), *llmq_ctx.clhandler, - *llmq_ctx.sigman, *llmq_ctx.shareman, sporkman, mn_sync)}, - is_signer{std::make_unique(chainman.ActiveChainstate(), *llmq_ctx.clhandler, - *llmq_ctx.isman, *llmq_ctx.sigman, *llmq_ctx.shareman, - *llmq_ctx.qman, sporkman, mempool, mn_sync)}, cj_server{std::make_unique(chainman, connman, dmnman, dstxman, mn_metaman, mempool, peerman, mn_activeman, mn_sync, *llmq_ctx.isman)}, gov_signer{std::make_unique(connman, dmnman, govman, mn_activeman, chainman, mn_sync)}, - ehf_sighandler{std::make_unique(chainman, mnhfman, *llmq_ctx.sigman, *llmq_ctx.shareman, - *llmq_ctx.qman)} + shareman{std::make_unique(*llmq_ctx.sigman, &mn_activeman, *llmq_ctx.qman, sporkman)}, + ehf_sighandler{ + std::make_unique(chainman, mnhfman, *llmq_ctx.sigman, *shareman, *llmq_ctx.qman)}, + cl_signer{std::make_unique(chainman.ActiveChainstate(), *llmq_ctx.clhandler, + *llmq_ctx.sigman, *shareman, sporkman, mn_sync)}, + is_signer{std::make_unique(chainman.ActiveChainstate(), *llmq_ctx.clhandler, + *llmq_ctx.isman, *llmq_ctx.sigman, *shareman, + *llmq_ctx.qman, sporkman, mempool, mn_sync)} { m_llmq_ctx.clhandler->ConnectSigner(cl_signer.get()); m_llmq_ctx.isman->ConnectSigner(is_signer.get()); @@ -41,3 +43,20 @@ ActiveContext::~ActiveContext() m_llmq_ctx.clhandler->DisconnectSigner(); m_llmq_ctx.isman->DisconnectSigner(); } + +void ActiveContext::Interrupt() +{ + shareman->InterruptWorkerThread(); +} + +void ActiveContext::Start(CConnman& connman, PeerManager& peerman) +{ + shareman->RegisterAsRecoveredSigsListener(); + shareman->StartWorkerThread(connman, peerman); +} + +void ActiveContext::Stop() +{ + shareman->StopWorkerThread(); + shareman->UnregisterAsRecoveredSigsListener(); +} diff --git a/src/masternode/active/context.h b/src/masternode/active/context.h index ca1cf697ecc9c..d6308b8c0b6ec 100644 --- a/src/masternode/active/context.h +++ b/src/masternode/active/context.h @@ -30,6 +30,7 @@ class InstantSendSigner; } // namespace instantsend namespace llmq { class CEHFSignalsHandler; +class CSigSharesManager; } // namespace llmq struct ActiveContext { @@ -37,13 +38,6 @@ struct ActiveContext { // TODO: Switch to references to members when migration is finished LLMQContext& m_llmq_ctx; - /* - * Entities that are registered with LLMQContext members are not accessible - * and are managed with (Dis)connectSigner() in the (c/d)tor instead - */ - const std::unique_ptr cl_signer; - const std::unique_ptr is_signer; - public: ActiveContext() = delete; ActiveContext(const ActiveContext&) = delete; @@ -53,6 +47,10 @@ struct ActiveContext { PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, const CMasternodeSync& mn_sync); ~ActiveContext(); + void Interrupt(); + void Start(CConnman& connman, PeerManager& peerman); + void Stop(); + /* * Entities that are only utilized when masternode mode is enabled * and are accessible in their own right @@ -60,7 +58,16 @@ struct ActiveContext { */ const std::unique_ptr cj_server; const std::unique_ptr gov_signer; + const std::unique_ptr shareman; const std::unique_ptr ehf_sighandler; + +private: + /* + * Entities that are registered with LLMQContext members are not accessible + * and are managed with (Dis)connectSigner() in the (c/d)tor instead + */ + const std::unique_ptr cl_signer; + const std::unique_ptr is_signer; }; #endif // BITCOIN_MASTERNODE_ACTIVE_CONTEXT_H diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 63d5aac9657b0..495909fe8dc58 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5377,6 +5377,7 @@ void PeerManagerImpl::ProcessMessage( #endif // ENABLE_WALLET if (m_active_ctx) { PostProcessMessage(m_active_ctx->cj_server->ProcessMessage(pfrom, msg_type, vRecv), pfrom.GetId()); + m_active_ctx->shareman->ProcessMessage(pfrom, *this, m_sporkman, msg_type, vRecv); } PostProcessMessage(m_sporkman.ProcessMessage(pfrom, m_connman, msg_type, vRecv), pfrom.GetId()); m_mn_sync.ProcessMessage(pfrom, msg_type, vRecv); @@ -5385,7 +5386,6 @@ void PeerManagerImpl::ProcessMessage( PostProcessMessage(m_llmq_ctx->quorum_block_processor->ProcessMessage(pfrom, msg_type, vRecv), pfrom.GetId()); PostProcessMessage(m_llmq_ctx->qdkgsman->ProcessMessage(pfrom, is_masternode, msg_type, vRecv), pfrom.GetId()); PostProcessMessage(m_llmq_ctx->qman->ProcessMessage(pfrom, m_connman, msg_type, vRecv), pfrom.GetId()); - m_llmq_ctx->shareman->ProcessMessage(pfrom, *this, m_sporkman, msg_type, vRecv); PostProcessMessage(m_llmq_ctx->sigman->ProcessMessage(pfrom.GetId(), msg_type, vRecv), pfrom.GetId()); PostProcessMessage(ProcessPlatformBanMessage(pfrom.GetId(), msg_type, vRecv), pfrom.GetId()); diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index b12b738f9046b..54e36be3a34d0 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -439,6 +440,10 @@ static RPCHelpMan quorum_memberof() static UniValue quorum_sign_helper(const JSONRPCRequest& request, Consensus::LLMQType llmqType) { const NodeContext& node = EnsureAnyNodeContext(request.context); + if (!node.mn_activeman) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Only available in masternode mode."); + } + const ChainstateManager& chainman = EnsureChainman(node); const LLMQContext& llmq_ctx = EnsureLLMQContext(node); @@ -459,7 +464,8 @@ static UniValue quorum_sign_helper(const JSONRPCRequest& request, Consensus::LLM fSubmit = ParseBoolV(request.params[3], "submit"); } if (fSubmit) { - return llmq_ctx.sigman->AsyncSignIfMember(llmqType, *llmq_ctx.shareman, id, msgHash, quorumHash); + return llmq_ctx.sigman->AsyncSignIfMember(llmqType, *CHECK_NONFATAL(node.active_ctx)->shareman, id, msgHash, + quorumHash); } else { const auto pQuorum = [&]() { if (quorumHash.IsNull()) { @@ -473,7 +479,7 @@ static UniValue quorum_sign_helper(const JSONRPCRequest& request, Consensus::LLM throw JSONRPCError(RPC_INVALID_PARAMETER, "quorum not found"); } - auto sigShare = llmq_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"); @@ -742,7 +748,7 @@ static RPCHelpMan quorum_selectquorum() UniValue recoveryMembers(UniValue::VARR); for (int i = 0; i < quorum->params.recoveryMembers; ++i) { - auto dmn = llmq_ctx.shareman->SelectMemberForRecovery(quorum, id, i); + auto dmn = llmq::CSigSharesManager::SelectMemberForRecovery(quorum, id, i); recoveryMembers.push_back(dmn->proTxHash.ToString()); } ret.pushKV("recoveryMembers", recoveryMembers); diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index 32718063789b9..e22949bcffad0 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -69,6 +69,7 @@ "llmq/signing -> llmq/signing_shares -> llmq/signing", "llmq/signing -> net_processing -> llmq/signing", "llmq/signing_shares -> net_processing -> llmq/signing_shares", + "llmq/signing_shares -> net_processing -> masternode/active/context -> llmq/signing_shares", "masternode/payments -> validation -> masternode/payments", "net -> netmessagemaker -> net", "netaddress -> netbase -> netaddress", From f6b02b79ddd6c770bc141e55a488137bb117cc40 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 8 Sep 2025 09:47:34 +0000 Subject: [PATCH 3/7] refactor: move `CDKGSessionManager` thread management to `ActiveContext` --- src/init.cpp | 2 +- src/llmq/context.cpp | 12 +++--------- src/llmq/context.h | 6 +----- src/masternode/active/context.cpp | 3 +++ 4 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index c83bd17c4581c..582f67fcd016f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2256,7 +2256,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 10a: schedule Dash-specific tasks - node.llmq_ctx->Start(*node.connman, *node.peerman); + node.llmq_ctx->Start(*node.peerman); if (node.active_ctx) node.active_ctx->Start(*node.connman, *node.peerman); node.scheduler->scheduleEvery(std::bind(&CNetFulfilledRequestManager::DoMaintenance, std::ref(*node.netfulfilledman)), std::chrono::minutes{1}); diff --git a/src/llmq/context.cpp b/src/llmq/context.cpp index 35feaae44e46d..61680c2c1dbe2 100644 --- a/src/llmq/context.cpp +++ b/src/llmq/context.cpp @@ -19,7 +19,6 @@ LLMQContext::LLMQContext(ChainstateManager& chainman, CDeterministicMNManager& d CMasternodeMetaMan& mn_metaman, CMNHFManager& mnhfman, CSporkManager& sporkman, CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync, bool unit_tests, bool wipe) : - is_masternode{mn_activeman != nullptr}, bls_worker{std::make_shared()}, dkg_debugman{std::make_unique()}, qsnapman{std::make_unique(evo_db)}, @@ -49,23 +48,18 @@ void LLMQContext::Interrupt() { sigman->InterruptWorkerThread(); } -void LLMQContext::Start(CConnman& connman, PeerManager& peerman) +void LLMQContext::Start(PeerManager& peerman) { - if (is_masternode) { - qdkgsman->StartThreads(connman, peerman); - } qman->Start(); sigman->StartWorkerThread(peerman); clhandler->Start(*isman); isman->Start(peerman); } -void LLMQContext::Stop() { +void LLMQContext::Stop() +{ isman->Stop(); clhandler->Stop(); sigman->StopWorkerThread(); qman->Stop(); - if (is_masternode) { - qdkgsman->StopThreads(); - } } diff --git a/src/llmq/context.h b/src/llmq/context.h index e1864e22c17d5..ef6e16009a694 100644 --- a/src/llmq/context.h +++ b/src/llmq/context.h @@ -9,7 +9,6 @@ class CActiveMasternodeManager; class CBLSWorker; -class CConnman; class ChainstateManager; class CDeterministicMNManager; class CEvoDB; @@ -32,9 +31,6 @@ class CSigningManager; } struct LLMQContext { -private: - const bool is_masternode; - public: LLMQContext() = delete; LLMQContext(const LLMQContext&) = delete; @@ -45,7 +41,7 @@ struct LLMQContext { ~LLMQContext(); void Interrupt(); - void Start(CConnman& connman, PeerManager& peerman); + void Start(PeerManager& peerman); void Stop(); /** Guaranteed if LLMQContext is initialized then all members are valid too diff --git a/src/masternode/active/context.cpp b/src/masternode/active/context.cpp index 6fe4170dd6e24..d166b639786e7 100644 --- a/src/masternode/active/context.cpp +++ b/src/masternode/active/context.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -51,6 +52,7 @@ void ActiveContext::Interrupt() void ActiveContext::Start(CConnman& connman, PeerManager& peerman) { + m_llmq_ctx.qdkgsman->StartThreads(connman, peerman); shareman->RegisterAsRecoveredSigsListener(); shareman->StartWorkerThread(connman, peerman); } @@ -59,4 +61,5 @@ void ActiveContext::Stop() { shareman->StopWorkerThread(); shareman->UnregisterAsRecoveredSigsListener(); + m_llmq_ctx.qdkgsman->StopThreads(); } From 489cf6e4c1aac9beba0bbd34f1be63d8f24ce82d Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 22 Oct 2025 23:44:21 +0530 Subject: [PATCH 4/7] refactor: adjust `CSigSharesManager` ctor arguments We initialize the masternode mode context later and as it's explicitly networking-heavy, we can move CConnman and PeerManager back into the ctor and trim down the arguments list a fair bit. Effectively reverts 82d1aed1d6. --- src/llmq/signing_shares.cpp | 164 +++++++++++++++--------------- src/llmq/signing_shares.h | 52 +++++----- src/masternode/active/context.cpp | 5 +- src/net_processing.cpp | 2 +- 4 files changed, 109 insertions(+), 114 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index d20a6ebe0314e..4e1519046d9b8 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -179,15 +179,29 @@ void CSigSharesNodeState::RemoveSession(const uint256& signHash) ////////////////////// -void CSigSharesManager::StartWorkerThread(CConnman& connman, PeerManager& peerman) +CSigSharesManager::CSigSharesManager(CConnman& connman, CSigningManager& _sigman, PeerManager& peerman, + const CActiveMasternodeManager& mn_activeman, const CQuorumManager& _qman, + const CSporkManager& sporkman) : + m_connman{connman}, + sigman{_sigman}, + m_peerman{peerman}, + m_mn_activeman{mn_activeman}, + qman{_qman}, + m_sporkman{sporkman} +{ + workInterrupt.reset(); +} + +CSigSharesManager::~CSigSharesManager() = default; + +void CSigSharesManager::StartWorkerThread() { // can't start new thread if we have one running already if (workThread.joinable()) { assert(false); } - workThread = std::thread(&util::TraceThread, "sigshares", - [this, &connman, &peerman] { WorkThreadMain(connman, peerman); }); + workThread = std::thread(&util::TraceThread, "sigshares", [this] { WorkThreadMain(); }); } void CSigSharesManager::StopWorkerThread() @@ -217,25 +231,23 @@ void CSigSharesManager::InterruptWorkerThread() workInterrupt(); } -void CSigSharesManager::ProcessMessage(const CNode& pfrom, PeerManager& peerman, const CSporkManager& sporkman, - const std::string& msg_type, CDataStream& vRecv) +void CSigSharesManager::ProcessMessage(const CNode& pfrom, const std::string& msg_type, CDataStream& vRecv) { // non-masternodes are not interested in sigshares - if (m_mn_activeman == nullptr) return; - if (m_mn_activeman->GetProTxHash().IsNull()) return; + if (m_mn_activeman.GetProTxHash().IsNull()) return; - if (sporkman.IsSporkActive(SPORK_21_QUORUM_ALL_CONNECTED) && msg_type == NetMsgType::QSIGSHARE) { + if (m_sporkman.IsSporkActive(SPORK_21_QUORUM_ALL_CONNECTED) && msg_type == NetMsgType::QSIGSHARE) { std::vector receivedSigShares; vRecv >> receivedSigShares; if (receivedSigShares.size() > MAX_MSGS_SIG_SHARES) { LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- too many sigs in QSIGSHARE message. cnt=%d, max=%d, node=%d\n", __func__, receivedSigShares.size(), MAX_MSGS_SIG_SHARES, pfrom.GetId()); - BanNode(pfrom.GetId(), peerman); + BanNode(pfrom.GetId()); return; } for (const auto& sigShare : receivedSigShares) { - ProcessMessageSigShare(pfrom.GetId(), peerman, sigShare); + ProcessMessageSigShare(pfrom.GetId(), sigShare); } } @@ -244,12 +256,12 @@ void CSigSharesManager::ProcessMessage(const CNode& pfrom, PeerManager& peerman, vRecv >> msgs; if (msgs.size() > MAX_MSGS_CNT_QSIGSESANN) { LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- too many announcements in QSIGSESANN message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_CNT_QSIGSESANN, pfrom.GetId()); - BanNode(pfrom.GetId(), peerman); + BanNode(pfrom.GetId()); return; } if (!ranges::all_of(msgs, [this, &pfrom](const auto& ann){ return ProcessMessageSigSesAnn(pfrom, ann); })) { - BanNode(pfrom.GetId(), peerman); + BanNode(pfrom.GetId()); return; } } else if (msg_type == NetMsgType::QSIGSHARESINV) { @@ -257,12 +269,12 @@ void CSigSharesManager::ProcessMessage(const CNode& pfrom, PeerManager& peerman, vRecv >> msgs; if (msgs.size() > MAX_MSGS_CNT_QSIGSHARESINV) { LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- too many invs in QSIGSHARESINV message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_CNT_QSIGSHARESINV, pfrom.GetId()); - BanNode(pfrom.GetId(), peerman); + BanNode(pfrom.GetId()); return; } if (!ranges::all_of(msgs, [this, &pfrom](const auto& inv){ return ProcessMessageSigSharesInv(pfrom, inv); })) { - BanNode(pfrom.GetId(), peerman); + BanNode(pfrom.GetId()); return; } } else if (msg_type == NetMsgType::QGETSIGSHARES) { @@ -270,12 +282,12 @@ void CSigSharesManager::ProcessMessage(const CNode& pfrom, PeerManager& peerman, vRecv >> msgs; if (msgs.size() > MAX_MSGS_CNT_QGETSIGSHARES) { LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- too many invs in QGETSIGSHARES message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_CNT_QGETSIGSHARES, pfrom.GetId()); - BanNode(pfrom.GetId(), peerman); + BanNode(pfrom.GetId()); return; } if (!ranges::all_of(msgs, [this, &pfrom](const auto& inv){ return ProcessMessageGetSigShares(pfrom, inv); })) { - BanNode(pfrom.GetId(), peerman); + BanNode(pfrom.GetId()); return; } } else if (msg_type == NetMsgType::QBSIGSHARES) { @@ -287,12 +299,12 @@ void CSigSharesManager::ProcessMessage(const CNode& pfrom, PeerManager& peerman, } if (totalSigsCount > MAX_MSGS_TOTAL_BATCHED_SIGS) { LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- too many sigs in QBSIGSHARES message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_TOTAL_BATCHED_SIGS, pfrom.GetId()); - BanNode(pfrom.GetId(), peerman); + BanNode(pfrom.GetId()); return; } if (!ranges::all_of(msgs, [this, &pfrom](const auto& bs){ return ProcessMessageBatchedSigShares(pfrom, bs); })) { - BanNode(pfrom.GetId(), peerman); + BanNode(pfrom.GetId()); return; } } @@ -410,7 +422,7 @@ bool CSigSharesManager::ProcessMessageBatchedSigShares(const CNode& pfrom, const return true; } - if (bool ban{false}; !PreVerifyBatchedSigShares(*Assert(m_mn_activeman), qman, sessionInfo, batchedSigShares, ban)) { + if (bool ban{false}; !PreVerifyBatchedSigShares(m_mn_activeman, qman, sessionInfo, batchedSigShares, ban)) { return !ban; } @@ -457,10 +469,8 @@ bool CSigSharesManager::ProcessMessageBatchedSigShares(const CNode& pfrom, const return true; } -void CSigSharesManager::ProcessMessageSigShare(NodeId fromId, PeerManager& peerman, const CSigShare& sigShare) +void CSigSharesManager::ProcessMessageSigShare(NodeId fromId, const CSigShare& sigShare) { - assert(m_mn_activeman); - auto quorum = qman.GetQuorum(sigShare.getLlmqType(), sigShare.getQuorumHash()); if (!quorum) { return; @@ -469,7 +479,7 @@ void CSigSharesManager::ProcessMessageSigShare(NodeId fromId, PeerManager& peerm // quorum is too old return; } - if (!quorum->IsMember(m_mn_activeman->GetProTxHash())) { + if (!quorum->IsMember(m_mn_activeman.GetProTxHash())) { // we're not a member so we can't verify it (we actually shouldn't have received it) return; } @@ -482,12 +492,12 @@ void CSigSharesManager::ProcessMessageSigShare(NodeId fromId, PeerManager& peerm if (sigShare.getQuorumMember() >= quorum->members.size()) { LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- quorumMember out of bounds\n", __func__); - BanNode(fromId, peerman); + BanNode(fromId); return; } if (!quorum->qc->validMembers[sigShare.getQuorumMember()]) { LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- quorumMember not valid\n", __func__); - BanNode(fromId, peerman); + BanNode(fromId); return; } @@ -623,7 +633,7 @@ bool CSigSharesManager::CollectPendingSigSharesToVerify( return true; } -bool CSigSharesManager::ProcessPendingSigShares(PeerManager& peerman, const CConnman& connman) +bool CSigSharesManager::ProcessPendingSigShares() { std::unordered_map> sigSharesByNodes; std::unordered_map, CQuorumCPtr, StaticSaltedHasher> quorums; @@ -649,7 +659,7 @@ bool CSigSharesManager::ProcessPendingSigShares(PeerManager& peerman, const CCon // we didn't check this earlier because we use a lazy BLS signature and tried to avoid doing the expensive // deserialization in the message thread if (!sigShare.sigShare.Get().IsValid()) { - BanNode(nodeId, peerman); + BanNode(nodeId); // don't process any additional shares from this node break; } @@ -681,11 +691,11 @@ bool CSigSharesManager::ProcessPendingSigShares(PeerManager& peerman, const CCon LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- invalid sig shares from other node, banning peer=%d\n", __func__, nodeId); // this will also cause re-requesting of the shares that were sent by this node - BanNode(nodeId, peerman); + BanNode(nodeId); continue; } - ProcessPendingSigShares(v, quorums, peerman, connman); + ProcessPendingSigShares(v, quorums); } return sigSharesByNodes.size() >= nMaxBatchSize; @@ -694,13 +704,12 @@ bool CSigSharesManager::ProcessPendingSigShares(PeerManager& peerman, const CCon // It's ensured that no duplicates are passed to this method void CSigSharesManager::ProcessPendingSigShares( const std::vector& sigSharesToProcess, - const std::unordered_map, CQuorumCPtr, StaticSaltedHasher>& quorums, - PeerManager& peerman, const CConnman& connman) + const std::unordered_map, CQuorumCPtr, StaticSaltedHasher>& quorums) { cxxtimer::Timer t(true); for (const auto& sigShare : sigSharesToProcess) { auto quorumKey = std::make_pair(sigShare.getLlmqType(), sigShare.getQuorumHash()); - ProcessSigShare(peerman, sigShare, connman, quorums.at(quorumKey)); + ProcessSigShare(sigShare, quorums.at(quorumKey)); } t.stop(); @@ -709,11 +718,8 @@ void CSigSharesManager::ProcessPendingSigShares( } // sig shares are already verified when entering this method -void CSigSharesManager::ProcessSigShare(PeerManager& peerman, const CSigShare& sigShare, const CConnman& connman, - const CQuorumCPtr& quorum) +void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum) { - assert(m_mn_activeman); - auto llmqType = quorum->params.type; bool canTryRecovery = false; @@ -721,8 +727,9 @@ void CSigSharesManager::ProcessSigShare(PeerManager& peerman, const CSigShare& s // prepare node set for direct-push in case this is our sig share std::vector quorumNodes; - if (!isAllMembersConnectedEnabled && sigShare.getQuorumMember() == quorum->GetMemberIndex(m_mn_activeman->GetProTxHash())) { - quorumNodes = connman.GetMasternodeQuorumNodes(sigShare.getLlmqType(), sigShare.getQuorumHash()); + if (!isAllMembersConnectedEnabled && + sigShare.getQuorumMember() == quorum->GetMemberIndex(m_mn_activeman.GetProTxHash())) { + quorumNodes = m_connman.GetMasternodeQuorumNodes(sigShare.getLlmqType(), sigShare.getQuorumHash()); } if (sigman.HasRecoveredSigForId(llmqType, sigShare.getId())) { @@ -759,12 +766,11 @@ void CSigSharesManager::ProcessSigShare(PeerManager& peerman, const CSigShare& s } if (canTryRecovery) { - TryRecoverSig(peerman, quorum, sigShare.getId(), sigShare.getMsgHash()); + TryRecoverSig(quorum, sigShare.getId(), sigShare.getMsgHash()); } } -void CSigSharesManager::TryRecoverSig(PeerManager& peerman, const CQuorumCPtr& quorum, const uint256& id, - const uint256& msgHash) +void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) { if (sigman.HasRecoveredSigForId(quorum->params.type, id)) { return; @@ -796,7 +802,7 @@ void CSigSharesManager::TryRecoverSig(PeerManager& peerman, const CQuorumCPtr& q auto rs = std::make_shared(quorum->params.type, quorum->qc->quorumHash, id, msgHash, recoveredSig); - sigman.ProcessRecoveredSig(rs, peerman); + sigman.ProcessRecoveredSig(rs, m_peerman); return; // end of single-quorum processing } @@ -842,7 +848,7 @@ void CSigSharesManager::TryRecoverSig(PeerManager& peerman, const CQuorumCPtr& q } } - sigman.ProcessRecoveredSig(rs, peerman); + sigman.ProcessRecoveredSig(rs, m_peerman); } CDeterministicMNCPtr CSigSharesManager::SelectMemberForRecovery(const CQuorumCPtr& quorum, const uint256 &id, int attempt) @@ -1052,8 +1058,7 @@ void CSigSharesManager::CollectSigSharesToSendConcentrated(std::unordered_map>& sigSharesToAnnounce) +void CSigSharesManager::CollectSigSharesToAnnounce(std::unordered_map>& sigSharesToAnnounce) { AssertLockHeld(cs); @@ -1061,8 +1066,8 @@ void CSigSharesManager::CollectSigSharesToAnnounce(const CConnman& connman, // TODO: remove NO_THREAD_SAFETY_ANALYSIS // using here template ForEach makes impossible to use lock annotation - sigSharesQueuedToAnnounce.ForEach([this, &connman, &quorumNodesMap, - &sigSharesToAnnounce](const SigShareKey& sigShareKey, bool) NO_THREAD_SAFETY_ANALYSIS { + sigSharesQueuedToAnnounce.ForEach([this, &quorumNodesMap, &sigSharesToAnnounce](const SigShareKey& sigShareKey, + bool) NO_THREAD_SAFETY_ANALYSIS { AssertLockHeld(cs); const auto& signHash = sigShareKey.first; auto quorumMember = sigShareKey.second; @@ -1075,7 +1080,7 @@ void CSigSharesManager::CollectSigSharesToAnnounce(const CConnman& connman, auto quorumKey = std::make_pair(sigShare->getLlmqType(), sigShare->getQuorumHash()); auto it = quorumNodesMap.find(quorumKey); if (it == quorumNodesMap.end()) { - auto nodeIds = connman.GetMasternodeQuorumNodes(quorumKey.first, quorumKey.second); + auto nodeIds = m_connman.GetMasternodeQuorumNodes(quorumKey.first, quorumKey.second); it = quorumNodesMap.emplace(std::piecewise_construct, std::forward_as_tuple(quorumKey), std::forward_as_tuple(nodeIds.begin(), nodeIds.end())).first; } @@ -1110,7 +1115,7 @@ void CSigSharesManager::CollectSigSharesToAnnounce(const CConnman& connman, sigSharesQueuedToAnnounce.Clear(); } -bool CSigSharesManager::SendMessages(CConnman& connman) +bool CSigSharesManager::SendMessages() { std::unordered_map> sigSharesToRequest; std::unordered_map> sigShareBatchesToSend; @@ -1139,12 +1144,12 @@ bool CSigSharesManager::SendMessages(CConnman& connman) return session->sendSessionId; }; - const CConnman::NodesSnapshot snap{connman, /* cond = */ CConnman::FullyConnectedOnly}; + const CConnman::NodesSnapshot snap{m_connman, /* cond = */ CConnman::FullyConnectedOnly}; { LOCK(cs); CollectSigSharesToRequest(sigSharesToRequest); CollectSigSharesToSend(sigShareBatchesToSend); - CollectSigSharesToAnnounce(connman, sigSharesToAnnounce); + CollectSigSharesToAnnounce(sigSharesToAnnounce); CollectSigSharesToSendConcentrated(sigSharesToSend, snap.Nodes()); for (auto& [nodeId, sigShareMap] : sigSharesToRequest) { @@ -1177,13 +1182,13 @@ bool CSigSharesManager::SendMessages(CConnman& connman) sigSesAnn.buildSignHash().ToString(), sigSesAnn.getSessionId(), pnode->GetId()); msgs.emplace_back(sigSesAnn); if (msgs.size() == MAX_MSGS_CNT_QSIGSESANN) { - connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSESANN, msgs)); + m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSESANN, msgs)); msgs.clear(); didSend = true; } } if (!msgs.empty()) { - connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSESANN, msgs)); + m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSESANN, msgs)); didSend = true; } } @@ -1196,13 +1201,13 @@ bool CSigSharesManager::SendMessages(CConnman& connman) signHash.ToString(), inv.ToString(), pnode->GetId()); msgs.emplace_back(inv); if (msgs.size() == MAX_MSGS_CNT_QGETSIGSHARES) { - connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QGETSIGSHARES, msgs)); + m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QGETSIGSHARES, msgs)); msgs.clear(); didSend = true; } } if (!msgs.empty()) { - connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QGETSIGSHARES, msgs)); + m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QGETSIGSHARES, msgs)); didSend = true; } } @@ -1215,7 +1220,7 @@ bool CSigSharesManager::SendMessages(CConnman& connman) LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::SendMessages -- QBSIGSHARES signHash=%s, inv={%s}, node=%d\n", signHash.ToString(), inv.ToInvString(), pnode->GetId()); if (totalSigsCount + inv.sigShares.size() > MAX_MSGS_TOTAL_BATCHED_SIGS) { - connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QBSIGSHARES, msgs)); + m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QBSIGSHARES, msgs)); msgs.clear(); totalSigsCount = 0; didSend = true; @@ -1224,7 +1229,7 @@ bool CSigSharesManager::SendMessages(CConnman& connman) msgs.emplace_back(inv); } if (!msgs.empty()) { - connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QBSIGSHARES, std::move(msgs))); + m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QBSIGSHARES, std::move(msgs))); didSend = true; } } @@ -1237,13 +1242,13 @@ bool CSigSharesManager::SendMessages(CConnman& connman) signHash.ToString(), inv.ToString(), pnode->GetId()); msgs.emplace_back(inv); if (msgs.size() == MAX_MSGS_CNT_QSIGSHARESINV) { - connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSHARESINV, msgs)); + m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSHARESINV, msgs)); msgs.clear(); didSend = true; } } if (!msgs.empty()) { - connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSHARESINV, msgs)); + m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSHARESINV, msgs)); didSend = true; } } @@ -1256,13 +1261,13 @@ bool CSigSharesManager::SendMessages(CConnman& connman) sigShare.GetSignHash().ToString(), pnode->GetId()); msgs.emplace_back(std::move(sigShare)); if (msgs.size() == MAX_MSGS_SIG_SHARES) { - connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSHARE, msgs)); + m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSHARE, msgs)); msgs.clear(); didSend = true; } } if (!msgs.empty()) { - connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSHARE, msgs)); + m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSHARE, msgs)); didSend = true; } } @@ -1285,7 +1290,7 @@ CSigShare CSigSharesManager::RebuildSigShare(const CSigSharesNodeState::SessionI return sigShare; } -void CSigSharesManager::Cleanup(const CConnman& connman) +void CSigSharesManager::Cleanup() { int64_t now = GetTime().count(); if (now - lastCleanupTime < 5) { @@ -1399,9 +1404,7 @@ void CSigSharesManager::Cleanup(const CConnman& connman) nodeStatesToDelete.emplace(nodeId); } } - connman.ForEachNode([&nodeStatesToDelete](const CNode* pnode) { - nodeStatesToDelete.erase(pnode->GetId()); - }); + m_connman.ForEachNode([&nodeStatesToDelete](const CNode* pnode) { nodeStatesToDelete.erase(pnode->GetId()); }); // Now delete these node states LOCK(cs); @@ -1438,13 +1441,13 @@ void CSigSharesManager::RemoveSigSharesForSession(const uint256& signHash) timeSeenForSessions.erase(signHash); } -void CSigSharesManager::RemoveBannedNodeStates(PeerManager& peerman) +void CSigSharesManager::RemoveBannedNodeStates() { // Called regularly to cleanup local node states for banned nodes LOCK(cs); for (auto it = nodeStates.begin(); it != nodeStates.end();) { - if (peerman.IsBanned(it->first)) { + if (m_peerman.IsBanned(it->first)) { // re-request sigshares from other nodes // TODO: remove NO_THREAD_SAFETY_ANALYSIS // using here template ForEach makes impossible to use lock annotation @@ -1459,13 +1462,13 @@ void CSigSharesManager::RemoveBannedNodeStates(PeerManager& peerman) } } -void CSigSharesManager::BanNode(NodeId nodeId, PeerManager& peerman) +void CSigSharesManager::BanNode(NodeId nodeId) { if (nodeId == -1) { return; } - peerman.Misbehaving(nodeId, 100); + m_peerman.Misbehaving(nodeId, 100); LOCK(cs); auto it = nodeStates.find(nodeId); @@ -1485,22 +1488,22 @@ void CSigSharesManager::BanNode(NodeId nodeId, PeerManager& peerman) nodeState.banned = true; } -void CSigSharesManager::WorkThreadMain(CConnman& connman, PeerManager& peerman) +void CSigSharesManager::WorkThreadMain() { int64_t lastSendTime = 0; while (!workInterrupt) { - RemoveBannedNodeStates(peerman); + RemoveBannedNodeStates(); - bool fMoreWork = ProcessPendingSigShares(peerman, connman); - SignPendingSigShares(connman, peerman); + bool fMoreWork = ProcessPendingSigShares(); + SignPendingSigShares(); if (TicksSinceEpoch(SystemClock::now()) - lastSendTime > 100) { - SendMessages(connman); + SendMessages(); lastSendTime = TicksSinceEpoch(SystemClock::now()); } - Cleanup(connman); + Cleanup(); // TODO Wakeup when pending signing is needed? if (!fMoreWork && !workInterrupt.sleep_for(std::chrono::milliseconds(100))) { @@ -1515,7 +1518,7 @@ void CSigSharesManager::AsyncSign(const CQuorumCPtr& quorum, const uint256& id, pendingSigns.emplace_back(quorum, id, msgHash); } -void CSigSharesManager::SignPendingSigShares(const CConnman& connman, PeerManager& peerman) +void CSigSharesManager::SignPendingSigShares() { std::vector v; WITH_LOCK(cs_pendingSigns, v.swap(pendingSigns)); @@ -1525,7 +1528,7 @@ void CSigSharesManager::SignPendingSigShares(const CConnman& connman, PeerManage if (opt_sigShare.has_value() && opt_sigShare->sigShare.Get().IsValid()) { auto sigShare = *opt_sigShare; - ProcessSigShare(peerman, sigShare, connman, pQuorum); + ProcessSigShare(sigShare, pQuorum); if (IsAllMembersConnectedEnabled(pQuorum->params.type, m_sporkman)) { LOCK(cs); @@ -1541,10 +1544,8 @@ void CSigSharesManager::SignPendingSigShares(const CConnman& connman, PeerManage std::optional CSigSharesManager::CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) const { - assert(m_mn_activeman); - cxxtimer::Timer t(true); - auto activeMasterNodeProTxHash = m_mn_activeman->GetProTxHash(); + auto activeMasterNodeProTxHash = m_mn_activeman.GetProTxHash(); if (!quorum->IsValidMember(activeMasterNodeProTxHash)) { return std::nullopt; @@ -1562,7 +1563,7 @@ std::optional CSigSharesManager::CreateSigShare(const CQuorumCPtr& qu // TODO: This one should be SIGN by QUORUM key, not by OPERATOR key // see TODO in CDKGSession::FinalizeSingleCommitment for details - sigShare.sigShare.Set(m_mn_activeman->Sign(signHash, bls::bls_legacy_scheme.load()), bls::bls_legacy_scheme.load()); + sigShare.sigShare.Set(m_mn_activeman.Sign(signHash, bls::bls_legacy_scheme.load()), bls::bls_legacy_scheme.load()); if (!sigShare.sigShare.Get().IsValid()) { LogPrintf("CSigSharesManager::%s -- failed to sign sigShare. signHash=%s, id=%s, msgHash=%s, time=%s\n", @@ -1644,5 +1645,4 @@ MessageProcessingResult CSigSharesManager::HandleNewRecoveredSig(const llmq::CRe RemoveSigSharesForSession(recoveredSig.buildSignHash().Get()); return {}; } - } // namespace llmq diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index 258a6f2417a66..0a3aec9d6ba59 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -27,6 +27,7 @@ #include #include +class CActiveMasternodeManager; class CNode; class CConnman; class CDeterministicMN; @@ -403,8 +404,10 @@ class CSigSharesManager : public CRecoveredSigsListener FastRandomContext rnd GUARDED_BY(cs); + CConnman& m_connman; CSigningManager& sigman; - const CActiveMasternodeManager* const m_mn_activeman; + PeerManager& m_peerman; + const CActiveMasternodeManager& m_mn_activeman; const CQuorumManager& qman; const CSporkManager& m_sporkman; @@ -412,26 +415,20 @@ class CSigSharesManager : public CRecoveredSigsListener std::atomic recoveredSigsCounter{0}; public: - explicit CSigSharesManager(CSigningManager& _sigman, const CActiveMasternodeManager* const mn_activeman, - const CQuorumManager& _qman, const CSporkManager& sporkman) : - sigman(_sigman), - m_mn_activeman(mn_activeman), - qman(_qman), - m_sporkman(sporkman) - { - workInterrupt.reset(); - }; + explicit CSigSharesManager(CConnman& connman, CSigningManager& _sigman, PeerManager& peerman, + const CActiveMasternodeManager& mn_activeman, const CQuorumManager& _qman, + const CSporkManager& sporkman); + ~CSigSharesManager() override; + CSigSharesManager() = delete; - ~CSigSharesManager() override = default; - void StartWorkerThread(CConnman& connman, PeerManager& peerman); + void StartWorkerThread(); void StopWorkerThread(); void RegisterAsRecoveredSigsListener(); void UnregisterAsRecoveredSigsListener(); void InterruptWorkerThread(); - void ProcessMessage(const CNode& pnode, PeerManager& peerman, const CSporkManager& sporkman, - const std::string& msg_type, CDataStream& vRecv); + void ProcessMessage(const CNode& pnode, const std::string& msg_type, CDataStream& vRecv); void AsyncSign(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns); @@ -448,7 +445,7 @@ class CSigSharesManager : public CRecoveredSigsListener 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, PeerManager& peerman, const CSigShare& sigShare); + void ProcessMessageSigShare(NodeId fromId, const CSigShare& sigShare); static bool VerifySigSharesInv(Consensus::LLMQType llmqType, const CSigSharesInv& inv); static bool PreVerifyBatchedSigShares(const CActiveMasternodeManager& mn_activeman, const CQuorumManager& quorum_manager, @@ -457,37 +454,34 @@ class CSigSharesManager : public CRecoveredSigsListener bool CollectPendingSigSharesToVerify( size_t maxUniqueSessions, std::unordered_map>& retSigShares, std::unordered_map, CQuorumCPtr, StaticSaltedHasher>& retQuorums); - bool ProcessPendingSigShares(PeerManager& peerman, const CConnman& connman); + bool ProcessPendingSigShares(); void ProcessPendingSigShares( const std::vector& sigSharesToProcess, - const std::unordered_map, CQuorumCPtr, StaticSaltedHasher>& quorums, - PeerManager& peerman, const CConnman& connman); + const std::unordered_map, CQuorumCPtr, StaticSaltedHasher>& quorums); - void ProcessSigShare(PeerManager& peerman, const CSigShare& sigShare, const CConnman& connman, - const CQuorumCPtr& quorum); - void TryRecoverSig(PeerManager& peerman, const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash); + void ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum); + void TryRecoverSig(const CQuorumCPtr& 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); - void Cleanup(const CConnman& connman); + void Cleanup(); void RemoveSigSharesForSession(const uint256& signHash) EXCLUSIVE_LOCKS_REQUIRED(cs); - void RemoveBannedNodeStates(PeerManager& peerman); + void RemoveBannedNodeStates(); - void BanNode(NodeId nodeId, PeerManager& peerman); + void BanNode(NodeId nodeId); - bool SendMessages(CConnman& connman); + bool SendMessages(); void CollectSigSharesToRequest(std::unordered_map>& sigSharesToRequest) EXCLUSIVE_LOCKS_REQUIRED(cs); void CollectSigSharesToSend(std::unordered_map>& sigSharesToSend) EXCLUSIVE_LOCKS_REQUIRED(cs); void CollectSigSharesToSendConcentrated(std::unordered_map>& sigSharesToSend, const std::vector& vNodes) EXCLUSIVE_LOCKS_REQUIRED(cs); - void CollectSigSharesToAnnounce(const CConnman& connman, - std::unordered_map>& sigSharesToAnnounce) + void CollectSigSharesToAnnounce(std::unordered_map>& sigSharesToAnnounce) EXCLUSIVE_LOCKS_REQUIRED(cs); - void SignPendingSigShares(const CConnman& connman, PeerManager& peerman) EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns); - void WorkThreadMain(CConnman& connman, PeerManager& peerman) EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns); + void SignPendingSigShares() EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns); + void WorkThreadMain() EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns); }; } // namespace llmq diff --git a/src/masternode/active/context.cpp b/src/masternode/active/context.cpp index d166b639786e7..28d853dc35b27 100644 --- a/src/masternode/active/context.cpp +++ b/src/masternode/active/context.cpp @@ -26,7 +26,8 @@ ActiveContext::ActiveContext(ChainstateManager& chainman, CConnman& connman, CDe cj_server{std::make_unique(chainman, connman, dmnman, dstxman, mn_metaman, mempool, peerman, mn_activeman, mn_sync, *llmq_ctx.isman)}, gov_signer{std::make_unique(connman, dmnman, govman, mn_activeman, chainman, mn_sync)}, - shareman{std::make_unique(*llmq_ctx.sigman, &mn_activeman, *llmq_ctx.qman, sporkman)}, + shareman{std::make_unique(connman, *llmq_ctx.sigman, peerman, mn_activeman, *llmq_ctx.qman, + sporkman)}, ehf_sighandler{ std::make_unique(chainman, mnhfman, *llmq_ctx.sigman, *shareman, *llmq_ctx.qman)}, cl_signer{std::make_unique(chainman.ActiveChainstate(), *llmq_ctx.clhandler, @@ -54,7 +55,7 @@ void ActiveContext::Start(CConnman& connman, PeerManager& peerman) { m_llmq_ctx.qdkgsman->StartThreads(connman, peerman); shareman->RegisterAsRecoveredSigsListener(); - shareman->StartWorkerThread(connman, peerman); + shareman->StartWorkerThread(); } void ActiveContext::Stop() diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 495909fe8dc58..8ad60042cb51d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5377,7 +5377,7 @@ void PeerManagerImpl::ProcessMessage( #endif // ENABLE_WALLET if (m_active_ctx) { PostProcessMessage(m_active_ctx->cj_server->ProcessMessage(pfrom, msg_type, vRecv), pfrom.GetId()); - m_active_ctx->shareman->ProcessMessage(pfrom, *this, m_sporkman, msg_type, vRecv); + m_active_ctx->shareman->ProcessMessage(pfrom, msg_type, vRecv); } PostProcessMessage(m_sporkman.ProcessMessage(pfrom, m_connman, msg_type, vRecv), pfrom.GetId()); m_mn_sync.ProcessMessage(pfrom, msg_type, vRecv); From 0052fcaa59bd0a5c9cde428fd0688c842b4ceec5 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 22 Oct 2025 23:43:26 +0530 Subject: [PATCH 5/7] refactor: move `AsyncSignIfMember()` to `CSigSharesManager` Giving write access of CRecoveredSigsDb to an external manager isn't great but we have to do this if we want to drop CActiveMasternodeManager from CSigSharesManager Review with `git log -p -n1 --color-moved=dimmed_zebra`. --- src/chainlock/signing.cpp | 3 +- src/instantsend/signing.cpp | 5 +- src/llmq/ehf_signals.cpp | 10 ++-- src/llmq/signing.cpp | 74 -------------------------- src/llmq/signing.h | 14 +++-- src/llmq/signing_shares.cpp | 88 +++++++++++++++++++++++++++++-- src/llmq/signing_shares.h | 12 +++-- src/masternode/active/context.cpp | 4 +- src/rpc/quorums.cpp | 3 +- 9 files changed, 114 insertions(+), 99 deletions(-) diff --git a/src/chainlock/signing.cpp b/src/chainlock/signing.cpp index 3524fd2044668..9b574d5cc6ee3 100644 --- a/src/chainlock/signing.cpp +++ b/src/chainlock/signing.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -140,7 +141,7 @@ void ChainLockSigner::TrySignChainTip(const llmq::CInstantSendManager& isman) lastSignedMsgHash = msgHash; } - m_sigman.AsyncSignIfMember(Params().GetConsensus().llmqTypeChainLocks, m_shareman, requestId, msgHash); + m_shareman.AsyncSignIfMember(Params().GetConsensus().llmqTypeChainLocks, m_sigman, requestId, msgHash); } void ChainLockSigner::EraseFromBlockHashTxidMap(const uint256& hash) diff --git a/src/instantsend/signing.cpp b/src/instantsend/signing.cpp index fa6d40f4623e3..789c0135f9685 100644 --- a/src/instantsend/signing.cpp +++ b/src/instantsend/signing.cpp @@ -13,6 +13,7 @@ #include #include +#include #include #include @@ -330,7 +331,7 @@ bool InstantSendSigner::TrySignInputLocks(const CTransaction& tx, bool fRetroact WITH_LOCK(cs_input_requests, inputRequestIds.emplace(id)); LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: trying to vote on input %s with id %s. fRetroactive=%d\n", __func__, tx.GetHash().ToString(), in.prevout.ToStringShort(), id.ToString(), fRetroactive); - if (m_sigman.AsyncSignIfMember(llmqType, m_shareman, id, tx.GetHash(), {}, fRetroactive)) { + if (m_shareman.AsyncSignIfMember(llmqType, m_sigman, id, tx.GetHash(), {}, fRetroactive)) { LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: voted on input %s with id %s\n", __func__, tx.GetHash().ToString(), in.prevout.ToStringShort(), id.ToString()); } @@ -388,6 +389,6 @@ void InstantSendSigner::TrySignInstantSendLock(const CTransaction& tx) txToCreatingInstantSendLocks.emplace(tx.GetHash(), &e.first->second); } - m_sigman.AsyncSignIfMember(llmqType, m_shareman, id, tx.GetHash(), quorum->m_quorum_base_block_index->GetBlockHash()); + m_shareman.AsyncSignIfMember(llmqType, m_sigman, id, tx.GetHash(), quorum->m_quorum_base_block_index->GetBlockHash()); } } // namespace instantsend diff --git a/src/llmq/ehf_signals.cpp b/src/llmq/ehf_signals.cpp index b4689e2d4a3e0..afec0ae3ee940 100644 --- a/src/llmq/ehf_signals.cpp +++ b/src/llmq/ehf_signals.cpp @@ -3,17 +3,19 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include -#include -#include #include #include #include -#include #include // g_txindex #include #include +#include +#include +#include +#include + namespace llmq { CEHFSignalsHandler::CEHFSignalsHandler(ChainstateManager& chainman, CMNHFManager& mnhfman, CSigningManager& sigman, CSigSharesManager& shareman, const CQuorumManager& qman) : @@ -76,7 +78,7 @@ void CEHFSignalsHandler::trySignEHFSignal(int bit, const CBlockIndex* const pind const uint256 msgHash = mnhfPayload.PrepareTx().GetHash(); WITH_LOCK(cs, ids.insert(requestId)); - sigman.AsyncSignIfMember(llmqType, shareman, requestId, msgHash, quorum->qc->quorumHash, false, true); + shareman.AsyncSignIfMember(llmqType, sigman, requestId, msgHash, quorum->qc->quorumHash, false, true); } MessageProcessingResult CEHFSignalsHandler::HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) diff --git a/src/llmq/signing.cpp b/src/llmq/signing.cpp index 55c0ab19db254..e6172efcb2180 100644 --- a/src/llmq/signing.cpp +++ b/src/llmq/signing.cpp @@ -666,80 +666,6 @@ void CSigningManager::UnregisterRecoveredSigsListener(CRecoveredSigsListener* l) recoveredSigsListeners.erase(itRem, recoveredSigsListeners.end()); } -bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigSharesManager& shareman, const uint256& id, - const uint256& msgHash, const uint256& quorumHash, bool allowReSign, - bool allowDiffMsgHashSigning) -{ - if (m_mn_activeman == nullptr) return false; - if (m_mn_activeman->GetProTxHash().IsNull()) return false; - - const 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 - // This gives a slight risk of not getting enough shares to recover a signature - // But at least it shouldn't be possible to get conflicting recovered signatures - // 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); - } else { - return qman.GetQuorum(llmqType, quorumHash); - } - }(); - - if (!quorum) { - LogPrint(BCLog::LLMQ, "CSigningManager::%s -- failed to select quorum. id=%s, msgHash=%s\n", __func__, id.ToString(), msgHash.ToString()); - return false; - } - - if (!quorum->IsValidMember(m_mn_activeman->GetProTxHash())) { - return false; - } - - { - bool hasVoted = db.HasVotedOnId(llmqType, id); - if (hasVoted) { - uint256 prevMsgHash; - db.GetVoteForId(llmqType, id, prevMsgHash); - if (msgHash != prevMsgHash) { - if (allowDiffMsgHashSigning) { - LogPrintf("CSigningManager::%s -- already voted for id=%s and msgHash=%s. Signing for different msgHash=%s\n", - __func__, id.ToString(), prevMsgHash.ToString(), msgHash.ToString()); - hasVoted = false; - } else { - LogPrintf("CSigningManager::%s -- already voted for id=%s and msgHash=%s. Not voting on conflicting msgHash=%s\n", - __func__, id.ToString(), prevMsgHash.ToString(), msgHash.ToString()); - return false; - } - } else if (allowReSign) { - LogPrint(BCLog::LLMQ, "CSigningManager::%s -- already voted for id=%s and msgHash=%s. Resigning!\n", __func__, - id.ToString(), prevMsgHash.ToString()); - } else { - LogPrint(BCLog::LLMQ, "CSigningManager::%s -- already voted for id=%s and msgHash=%s. Not voting again.\n", __func__, - id.ToString(), prevMsgHash.ToString()); - return false; - } - } - - if (db.HasRecoveredSigForId(llmqType, id)) { - // no need to sign it if we already have a recovered sig - return true; - } - if (!hasVoted) { - db.WriteVoteForId(llmqType, id, msgHash); - } - } - - if (allowReSign) { - // make us re-announce all known shares (other nodes might have run into a timeout) - shareman.ForceReAnnouncement(quorum, llmqType, id, msgHash); - } - shareman.AsyncSign(quorum, id, msgHash); - - return true; -} - bool CSigningManager::HasRecoveredSig(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash) const { return db.HasRecoveredSig(llmqType, id, msgHash); diff --git a/src/llmq/signing.h b/src/llmq/signing.h index e49e76a143294..b9aba0d500da3 100644 --- a/src/llmq/signing.h +++ b/src/llmq/signing.h @@ -209,22 +209,20 @@ class CSigningManager EXCLUSIVE_LOCKS_REQUIRED(!cs_pending, !cs_listeners); bool ProcessPendingRecoveredSigs(PeerManager& peerman) EXCLUSIVE_LOCKS_REQUIRED(!cs_pending, !cs_listeners); // called from the worker thread of CSigSharesManager -public: - // TODO - should not be public! + + // Used by CSigSharesManager + CRecoveredSigsDb& GetDb() { return db; } void ProcessRecoveredSig(const std::shared_ptr& recoveredSig, PeerManager& peerman) EXCLUSIVE_LOCKS_REQUIRED(!cs_pending, !cs_listeners); -private: - void Cleanup(); // called from the worker thread of CSigSharesManager + // Needed for access to GetDb() and ProcessRecoveredSig() + friend class CSigSharesManager; public: // public interface void RegisterRecoveredSigsListener(CRecoveredSigsListener* l) EXCLUSIVE_LOCKS_REQUIRED(!cs_listeners); void UnregisterRecoveredSigsListener(CRecoveredSigsListener* l) EXCLUSIVE_LOCKS_REQUIRED(!cs_listeners); - bool AsyncSignIfMember(Consensus::LLMQType llmqType, CSigSharesManager& shareman, const uint256& id, - const uint256& msgHash, const uint256& quorumHash = uint256(), bool allowReSign = false, - bool allowDiffMsgHashSigning = false); bool HasRecoveredSig(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash) const; bool HasRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id) const; bool HasRecoveredSigForSession(const uint256& signHash) const; @@ -236,8 +234,8 @@ class CSigningManager private: std::thread workThread; CThreadInterrupt workInterrupt; + void Cleanup(); // called from the worker thread of CSigSharesManager void WorkThreadMain(PeerManager& peerman) EXCLUSIVE_LOCKS_REQUIRED(!cs_pending, !cs_listeners); - ; public: void StartWorkerThread(PeerManager& peerman); diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 4e1519046d9b8..146e6a729642e 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include @@ -179,10 +180,11 @@ void CSigSharesNodeState::RemoveSession(const uint256& signHash) ////////////////////// -CSigSharesManager::CSigSharesManager(CConnman& connman, CSigningManager& _sigman, PeerManager& peerman, - const CActiveMasternodeManager& mn_activeman, const CQuorumManager& _qman, - const CSporkManager& sporkman) : +CSigSharesManager::CSigSharesManager(CConnman& connman, CChainState& chainstate, CSigningManager& _sigman, + PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, + const CQuorumManager& _qman, const CSporkManager& sporkman) : m_connman{connman}, + m_chainstate{chainstate}, sigman{_sigman}, m_peerman{peerman}, m_mn_activeman{mn_activeman}, @@ -866,6 +868,86 @@ CDeterministicMNCPtr CSigSharesManager::SelectMemberForRecovery(const CQuorumCPt return v[attempt % v.size()].second; } +bool CSigSharesManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigningManager& sigman, const uint256& id, + const uint256& msgHash, const uint256& quorumHash, bool allowReSign, + bool allowDiffMsgHashSigning) +{ + AssertLockNotHeld(cs_pendingSigns); + + if (m_mn_activeman.GetProTxHash().IsNull()) return false; + + const 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 + // This gives a slight risk of not getting enough shares to recover a signature + // But at least it shouldn't be possible to get conflicting recovered signatures + // 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); + } else { + return qman.GetQuorum(llmqType, quorumHash); + } + }(); + + if (!quorum) { + LogPrint(BCLog::LLMQ, "CSigningManager::%s -- failed to select quorum. id=%s, msgHash=%s\n", __func__, + id.ToString(), msgHash.ToString()); + return false; + } + + if (!quorum->IsValidMember(m_mn_activeman.GetProTxHash())) { + return false; + } + + { + auto& db = sigman.GetDb(); + bool hasVoted = db.HasVotedOnId(llmqType, id); + if (hasVoted) { + uint256 prevMsgHash; + db.GetVoteForId(llmqType, id, prevMsgHash); + if (msgHash != prevMsgHash) { + if (allowDiffMsgHashSigning) { + LogPrintf("%s -- already voted for id=%s and msgHash=%s. Signing for different " /* Continued */ + "msgHash=%s\n", + __func__, id.ToString(), prevMsgHash.ToString(), msgHash.ToString()); + hasVoted = false; + } else { + LogPrintf("%s -- already voted for id=%s and msgHash=%s. Not voting on " /* Continued */ + "conflicting msgHash=%s\n", + __func__, id.ToString(), prevMsgHash.ToString(), msgHash.ToString()); + return false; + } + } else if (allowReSign) { + LogPrint(BCLog::LLMQ, "%s -- already voted for id=%s and msgHash=%s. Resigning!\n", __func__, + id.ToString(), prevMsgHash.ToString()); + } else { + LogPrint(BCLog::LLMQ, "%s -- already voted for id=%s and msgHash=%s. Not voting again.\n", __func__, + id.ToString(), prevMsgHash.ToString()); + return false; + } + } + + if (db.HasRecoveredSigForId(llmqType, id)) { + // no need to sign it if we already have a recovered sig + return true; + } + if (!hasVoted) { + db.WriteVoteForId(llmqType, id, msgHash); + } + } + + if (allowReSign) { + // make us re-announce all known shares (other nodes might have run into a timeout) + ForceReAnnouncement(quorum, llmqType, id, msgHash); + } + AsyncSign(quorum, id, msgHash); + + return true; +} + void CSigSharesManager::CollectSigSharesToRequest(std::unordered_map>& sigSharesToRequest) { AssertLockHeld(cs); diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index 0a3aec9d6ba59..85e33a1f1d4dc 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -405,6 +405,7 @@ class CSigSharesManager : public CRecoveredSigsListener FastRandomContext rnd GUARDED_BY(cs); CConnman& m_connman; + CChainState& m_chainstate; CSigningManager& sigman; PeerManager& m_peerman; const CActiveMasternodeManager& m_mn_activeman; @@ -415,9 +416,9 @@ class CSigSharesManager : public CRecoveredSigsListener std::atomic recoveredSigsCounter{0}; public: - explicit CSigSharesManager(CConnman& connman, CSigningManager& _sigman, PeerManager& peerman, - const CActiveMasternodeManager& mn_activeman, const CQuorumManager& _qman, - const CSporkManager& sporkman); + explicit CSigSharesManager(CConnman& connman, CChainState& chainstate, CSigningManager& _sigman, + PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, + const CQuorumManager& _qman, const CSporkManager& sporkman); ~CSigSharesManager() override; CSigSharesManager() = delete; @@ -439,6 +440,11 @@ class CSigSharesManager : public CRecoveredSigsListener static CDeterministicMNCPtr SelectMemberForRecovery(const CQuorumCPtr& 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); + 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); diff --git a/src/masternode/active/context.cpp b/src/masternode/active/context.cpp index 28d853dc35b27..c737d8a03d0ba 100644 --- a/src/masternode/active/context.cpp +++ b/src/masternode/active/context.cpp @@ -26,8 +26,8 @@ ActiveContext::ActiveContext(ChainstateManager& chainman, CConnman& connman, CDe cj_server{std::make_unique(chainman, connman, dmnman, dstxman, mn_metaman, mempool, peerman, mn_activeman, mn_sync, *llmq_ctx.isman)}, gov_signer{std::make_unique(connman, dmnman, govman, mn_activeman, chainman, mn_sync)}, - shareman{std::make_unique(connman, *llmq_ctx.sigman, peerman, mn_activeman, *llmq_ctx.qman, - sporkman)}, + shareman{std::make_unique(connman, chainman.ActiveChainstate(), *llmq_ctx.sigman, peerman, + mn_activeman, *llmq_ctx.qman, sporkman)}, ehf_sighandler{ std::make_unique(chainman, mnhfman, *llmq_ctx.sigman, *shareman, *llmq_ctx.qman)}, cl_signer{std::make_unique(chainman.ActiveChainstate(), *llmq_ctx.clhandler, diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index 54e36be3a34d0..424e587baa209 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -464,8 +464,7 @@ static UniValue quorum_sign_helper(const JSONRPCRequest& request, Consensus::LLM fSubmit = ParseBoolV(request.params[3], "submit"); } if (fSubmit) { - return llmq_ctx.sigman->AsyncSignIfMember(llmqType, *CHECK_NONFATAL(node.active_ctx)->shareman, id, msgHash, - quorumHash); + return CHECK_NONFATAL(node.active_ctx)->shareman->AsyncSignIfMember(llmqType, *llmq_ctx.sigman, id, msgHash, quorumHash); } else { const auto pQuorum = [&]() { if (quorumHash.IsNull()) { From 6c89f0e3e9dade21349c06023aa0f1c2eae490c7 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 8 Sep 2025 23:31:00 +0000 Subject: [PATCH 6/7] refactor: move `RelayRecoveredSig()` call out of `CSigSharesManager` As ProcessRecoveredSig() triggers a NotifyRecoveredSig() notification, we can stick the RelayRecoveredSig() into CSigSharesManager and have the relay request processed through the notification interface instead, lets us finally drop CActiveMasternodeManager from CSigSharesManager. --- src/llmq/context.cpp | 2 +- src/llmq/signing.cpp | 8 +------- src/llmq/signing.h | 5 +---- src/llmq/signing_shares.cpp | 5 +++++ src/llmq/signing_shares.h | 2 ++ src/masternode/active/notificationinterface.cpp | 6 ++++++ src/masternode/active/notificationinterface.h | 6 ++++++ 7 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/llmq/context.cpp b/src/llmq/context.cpp index 61680c2c1dbe2..359d2a23924c6 100644 --- a/src/llmq/context.cpp +++ b/src/llmq/context.cpp @@ -30,7 +30,7 @@ LLMQContext::LLMQContext(ChainstateManager& chainman, CDeterministicMNManager& d qman{std::make_unique(*bls_worker, chainman.ActiveChainstate(), dmnman, *qdkgsman, evo_db, *quorum_block_processor, *qsnapman, mn_activeman, mn_sync, sporkman, unit_tests, wipe)}, - sigman{std::make_unique(mn_activeman, chainman.ActiveChainstate(), *qman, unit_tests, wipe)}, + sigman{std::make_unique(chainman.ActiveChainstate(), *qman, unit_tests, wipe)}, clhandler{std::make_unique(chainman.ActiveChainstate(), *qman, sporkman, mempool, mn_sync)}, isman{std::make_unique(*clhandler, chainman.ActiveChainstate(), *qman, *sigman, sporkman, mempool, mn_sync, unit_tests, wipe)} diff --git a/src/llmq/signing.cpp b/src/llmq/signing.cpp index e6172efcb2180..a2789737e7f20 100644 --- a/src/llmq/signing.cpp +++ b/src/llmq/signing.cpp @@ -332,10 +332,8 @@ void CRecoveredSigsDb::CleanupOldVotes(int64_t maxAge) ////////////////// -CSigningManager::CSigningManager(const CActiveMasternodeManager* const mn_activeman, const CChainState& chainstate, - const CQuorumManager& _qman, bool fMemory, bool fWipe) : +CSigningManager::CSigningManager(const CChainState& chainstate, const CQuorumManager& _qman, bool fMemory, bool fWipe) : db(fMemory, fWipe), - m_mn_activeman(mn_activeman), m_chainstate(chainstate), qman(_qman) { @@ -615,10 +613,6 @@ void CSigningManager::ProcessRecoveredSig(const std::shared_ptrGetHash())); - if (m_mn_activeman != nullptr) { - peerman.RelayRecoveredSig(recoveredSig->GetHash()); - } - auto listeners = WITH_LOCK(cs_listeners, return recoveredSigsListeners); for (auto& l : listeners) { peerman.PostProcessMessage(l->HandleNewRecoveredSig(*recoveredSig)); diff --git a/src/llmq/signing.h b/src/llmq/signing.h index b9aba0d500da3..49839074bf104 100644 --- a/src/llmq/signing.h +++ b/src/llmq/signing.h @@ -23,7 +23,6 @@ #include #include -class CActiveMasternodeManager; class CChainState; class CDataStream; class CDBBatch; @@ -163,7 +162,6 @@ class CSigningManager private: CRecoveredSigsDb db; - const CActiveMasternodeManager* const m_mn_activeman; const CChainState& m_chainstate; const CQuorumManager& qman; @@ -180,8 +178,7 @@ class CSigningManager std::vector recoveredSigsListeners GUARDED_BY(cs_listeners); public: - CSigningManager(const CActiveMasternodeManager* const mn_activeman, const CChainState& chainstate, - const CQuorumManager& _qman, bool fMemory, bool fWipe); + CSigningManager(const CChainState& chainstate, const CQuorumManager& _qman, bool fMemory, bool fWipe); bool AlreadyHave(const CInv& inv) const EXCLUSIVE_LOCKS_REQUIRED(!cs_pending); bool GetRecoveredSigForGetData(const uint256& hash, CRecoveredSig& ret) const; diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 146e6a729642e..fdb7d967dba42 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -948,6 +948,11 @@ bool CSigSharesManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigning return true; } +void CSigSharesManager::NotifyRecoveredSig(const std::shared_ptr& sig) const +{ + m_peerman.RelayRecoveredSig(Assert(sig)->GetHash()); +} + void CSigSharesManager::CollectSigSharesToRequest(std::unordered_map>& sigSharesToRequest) { AssertLockHeld(cs); diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index 85e33a1f1d4dc..426bfbc3a4446 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -445,6 +445,8 @@ class CSigSharesManager : public CRecoveredSigsListener bool allowDiffMsgHashSigning = false) EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns); + void NotifyRecoveredSig(const std::shared_ptr& sig) const; + 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); diff --git a/src/masternode/active/notificationinterface.cpp b/src/masternode/active/notificationinterface.cpp index aee9f1917afcb..5a71a476cd0c9 100644 --- a/src/masternode/active/notificationinterface.cpp +++ b/src/masternode/active/notificationinterface.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -26,4 +27,9 @@ void ActiveNotificationInterface::UpdatedBlockTip(const CBlockIndex* pindexNew, m_active_ctx.gov_signer->UpdatedBlockTip(pindexNew); } +void ActiveNotificationInterface::NotifyRecoveredSig(const std::shared_ptr& sig) +{ + m_active_ctx.shareman->NotifyRecoveredSig(sig); +} + std::unique_ptr g_active_notification_interface; diff --git a/src/masternode/active/notificationinterface.h b/src/masternode/active/notificationinterface.h index ecb3f9fa962cd..4e55bcf216814 100644 --- a/src/masternode/active/notificationinterface.h +++ b/src/masternode/active/notificationinterface.h @@ -7,8 +7,13 @@ #include +#include + class CActiveMasternodeManager; struct ActiveContext; +namespace llmq { +class CRecoveredSig; +} // namespace llmq class ActiveNotificationInterface final : public CValidationInterface { @@ -20,6 +25,7 @@ class ActiveNotificationInterface final : public CValidationInterface protected: // CValidationInterface + void NotifyRecoveredSig(const std::shared_ptr& sig) override; void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) override; private: From af9a15f804f8cb2df0a3658368b0d2f8f4ef2f6e Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 22 Oct 2025 23:30:14 +0530 Subject: [PATCH 7/7] lint: apply review suggestions --- src/chainlock/chainlock.h | 4 ++++ src/llmq/signing.cpp | 2 +- src/llmq/signing.h | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/chainlock/chainlock.h b/src/chainlock/chainlock.h index ab6b0a1085829..14fcad13c5de5 100644 --- a/src/chainlock/chainlock.h +++ b/src/chainlock/chainlock.h @@ -18,7 +18,11 @@ #include #include +#include +#include #include +#include +#include #include class CBlock; diff --git a/src/llmq/signing.cpp b/src/llmq/signing.cpp index a2789737e7f20..8ed8640c60cbb 100644 --- a/src/llmq/signing.cpp +++ b/src/llmq/signing.cpp @@ -711,7 +711,7 @@ void CSigningManager::StartWorkerThread(PeerManager& peerman) assert(false); } - workThread = std::thread(&util::TraceThread, "sigshares", [this, &peerman] { WorkThreadMain(peerman); }); + workThread = std::thread(&util::TraceThread, "recsigs", [this, &peerman] { WorkThreadMain(peerman); }); } void CSigningManager::StopWorkerThread() diff --git a/src/llmq/signing.h b/src/llmq/signing.h index 49839074bf104..fdb37ac10b716 100644 --- a/src/llmq/signing.h +++ b/src/llmq/signing.h @@ -20,7 +20,9 @@ #include +#include #include +#include #include class CChainState;