From 22ea97d07ff2a809dea486deb1601183ada53053 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 29 Oct 2025 01:42:07 +0700 Subject: [PATCH 01/10] refactor: use new NetHandler for llmq/signing module --- src/Makefile.am | 2 + src/init.cpp | 2 + src/llmq/context.cpp | 5 +- src/llmq/net_signing.cpp | 179 ++++++++++++++++++++ src/llmq/net_signing.h | 45 ++++++ src/llmq/signing.cpp | 207 +++--------------------- src/llmq/signing.h | 46 ++---- src/llmq/signing_shares.cpp | 24 ++- src/llmq/signing_shares.h | 1 + src/net_processing.cpp | 8 +- src/net_processing.h | 1 + test/lint/lint-circular-dependencies.py | 25 ++- 12 files changed, 305 insertions(+), 240 deletions(-) create mode 100644 src/llmq/net_signing.cpp create mode 100644 src/llmq/net_signing.h diff --git a/src/Makefile.am b/src/Makefile.am index 29aa4c06a6ca..77b384d144e2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -273,6 +273,7 @@ BITCOIN_CORE_H = \ llmq/quorums.h \ llmq/signhash.h \ llmq/signing.h \ + llmq/net_signing.h \ llmq/signing_shares.h \ llmq/snapshot.h \ llmq/types.h \ @@ -535,6 +536,7 @@ libbitcoin_node_a_SOURCES = \ llmq/dkgsessionhandler.cpp \ llmq/dkgsessionmgr.cpp \ llmq/ehf_signals.cpp \ + llmq/net_signing.cpp \ llmq/options.cpp \ llmq/quorums.cpp \ llmq/signhash.cpp \ diff --git a/src/init.cpp b/src/init.cpp index 5e63df2639be..6170c66817a9 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -93,6 +93,7 @@ #include #include #include +#include #include #include #include @@ -2203,6 +2204,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) RegisterValidationInterface(g_active_notification_interface.get()); } node.peerman->AddExtraHandler(std::make_unique(node.peerman.get(), *node.llmq_ctx->isman, *node.llmq_ctx->qman, chainman.ActiveChainstate())); + node.peerman->AddExtraHandler(std::make_unique(node.peerman.get(), *node.llmq_ctx->sigman)); // ********************************************************* Step 7d: Setup other Dash services diff --git a/src/llmq/context.cpp b/src/llmq/context.cpp index e3c7b4911f1b..00a21fc8c6b7 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, db_params)}, - sigman{std::make_unique(chainman.ActiveChainstate(), *qman, db_params)}, + sigman{std::make_unique(*qman, db_params)}, clhandler{std::make_unique(chainman.ActiveChainstate(), *qman, sporkman, mempool, mn_sync)}, isman{std::make_unique(*clhandler, chainman.ActiveChainstate(), *sigman, sporkman, mempool, mn_sync, db_params)} @@ -44,19 +44,16 @@ LLMQContext::~LLMQContext() { } void LLMQContext::Interrupt() { - sigman->InterruptWorkerThread(); } void LLMQContext::Start(PeerManager& peerman) { qman->Start(); - sigman->StartWorkerThread(peerman); clhandler->Start(*isman); } void LLMQContext::Stop() { clhandler->Stop(); - sigman->StopWorkerThread(); qman->Stop(); } diff --git a/src/llmq/net_signing.cpp b/src/llmq/net_signing.cpp new file mode 100644 index 000000000000..806e99d8b7ea --- /dev/null +++ b/src/llmq/net_signing.cpp @@ -0,0 +1,179 @@ +// Copyright (c) 2025 The Dash Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include + +static bool PreVerifyRecoveredSig(Consensus::LLMQType& llmqType, const llmq::CQuorumManager& quorum_manager, + const llmq::CRecoveredSig& recoveredSig) +{ + auto quorum = quorum_manager.GetQuorum(llmqType, recoveredSig.getQuorumHash()); + + if (!quorum) { + LogPrint(BCLog::LLMQ, "NetSigning::%s -- quorum %s not found\n", __func__, recoveredSig.getQuorumHash().ToString()); + return false; + } + if (!llmq::IsQuorumActive(llmqType, quorum_manager, quorum->qc->quorumHash)) { + return false; + } + + return true; +} + +void NetSigning::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv) +{ + if (msg_type != NetMsgType::QSIGREC) return; + + auto recoveredSig = std::make_shared(); + vRecv >> *recoveredSig; + + WITH_LOCK(cs_main, m_peer_manager->PeerEraseObjectRequest(pfrom.GetId(), + CInv{MSG_QUORUM_RECOVERED_SIG, recoveredSig->GetHash()})); + + auto llmqType = recoveredSig->getLlmqType(); + if (!Params().GetLLMQ(llmqType).has_value()) { + m_peer_manager->PeerMisbehaving(pfrom.GetId(), 100); + } + if (!PreVerifyRecoveredSig(llmqType, m_sig_manager.Qman(), *recoveredSig)) { + return; + } + + m_sig_manager.ProcessRecoveredSig(pfrom.GetId(), std::move(recoveredSig)); +} + +void NetSigning::Start() +{ + // can't start new thread if we have one running already + if (workThread.joinable()) { + assert(false); + } + + workThread = std::thread(&util::TraceThread, "recsigs", [this] { WorkThreadMain(); }); +} + +void NetSigning::Stop() +{ + // make sure to call InterruptWorkerThread() first + if (!workInterrupt) { + assert(false); + } + + if (workThread.joinable()) { + workThread.join(); + } +} + +void NetSigning::ProcessRecoveredSig(std::shared_ptr recoveredSig, bool consider_proactive_relay) +{ + if (!m_sig_manager.ProcessRecoveredSig(recoveredSig)) return; + + auto listeners = m_sig_manager.GetListeners(); + for (auto& l : listeners) { + m_peer_manager->PeerPostProcessMessage(l->HandleNewRecoveredSig(*recoveredSig)); + } + + // TODO refactor to use a better abstraction analogous to IsAllMembersConnectedEnabled + auto proactive_relay = consider_proactive_relay && recoveredSig->getLlmqType() != Consensus::LLMQType::LLMQ_100_67 && + recoveredSig->getLlmqType() != Consensus::LLMQType::LLMQ_400_60 && + recoveredSig->getLlmqType() != Consensus::LLMQType::LLMQ_400_85; + GetMainSignals().NotifyRecoveredSig(recoveredSig, recoveredSig->GetHash().ToString(), proactive_relay); +} + +bool NetSigning::ProcessPendingRecoveredSigs() +{ + Uint256HashMap> pending{m_sig_manager.FetchPendingReconstructed()}; + + for (const auto& p : pending) { + ProcessRecoveredSig(p.second, true); + } + + std::unordered_map>> recSigsByNode; + std::unordered_map, llmq::CQuorumCPtr, StaticSaltedHasher> quorums; + + const size_t nMaxBatchSize{32}; + bool more_work = m_sig_manager.CollectPendingRecoveredSigsToVerify(nMaxBatchSize, recSigsByNode, quorums); + if (recSigsByNode.empty()) { + return false; + } + + // It's ok to perform insecure batched verification here as we verify against the quorum public keys, which are not + // craftable by individual entities, making the rogue public key attack impossible + CBLSBatchVerifier batchVerifier(false, false); + + size_t verifyCount = 0; + for (const auto& p : recSigsByNode) { + NodeId nodeId = p.first; + const auto& v = p.second; + + for (const auto& recSig : v) { + // we didn't verify the lazy signature until now + if (!recSig->sig.Get().IsValid()) { + batchVerifier.badSources.emplace(nodeId); + break; + } + + const auto& quorum = quorums.at(std::make_pair(recSig->getLlmqType(), recSig->getQuorumHash())); + batchVerifier.PushMessage(nodeId, recSig->GetHash(), recSig->buildSignHash().Get(), recSig->sig.Get(), + quorum->qc->quorumPublicKey); + verifyCount++; + } + } + + cxxtimer::Timer verifyTimer(true); + batchVerifier.Verify(); + verifyTimer.stop(); + + LogPrint(BCLog::LLMQ, "CSigningManager::%s -- verified recovered sig(s). count=%d, vt=%d, nodes=%d\n", __func__, + verifyCount, verifyTimer.count(), recSigsByNode.size()); + + Uint256HashSet processed; + for (const auto& p : recSigsByNode) { + NodeId nodeId = p.first; + const auto& v = p.second; + + if (batchVerifier.badSources.count(nodeId)) { + LogPrint(BCLog::LLMQ, "CSigningManager::%s -- invalid recSig from other node, banning peer=%d\n", __func__, + nodeId); + m_peer_manager->PeerMisbehaving(nodeId, 100); + continue; + } + + for (const auto& recSig : v) { + if (!processed.emplace(recSig->GetHash()).second) { + continue; + } + + ProcessRecoveredSig(recSig, nodeId == -1); + } + } + + return more_work; +} + +void NetSigning::WorkThreadMain() +{ + while (!workInterrupt) { + bool fMoreWork = ProcessPendingRecoveredSigs(); + + m_sig_manager.Cleanup(); + + // TODO Wakeup when pending signing is needed? + if (!fMoreWork && !workInterrupt.sleep_for(std::chrono::milliseconds(100))) { + return; + } + } +} diff --git a/src/llmq/net_signing.h b/src/llmq/net_signing.h new file mode 100644 index 000000000000..bb40ec2de9c3 --- /dev/null +++ b/src/llmq/net_signing.h @@ -0,0 +1,45 @@ +// Copyright (c) 2025 The Dash Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_LLMQ_NET_SIGNING_H +#define BITCOIN_LLMQ_NET_SIGNING_H + +#include + +#include + +#include + +namespace llmq { +class CSigningManager; +} // namespace llmq + +class NetSigning final : public NetHandler +{ +public: + NetSigning(PeerManagerInternal* peer_manager, llmq::CSigningManager& sig_manager) : + NetHandler(peer_manager), + m_sig_manager(sig_manager) + { + workInterrupt.reset(); + } + void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv) override; + + bool ProcessPendingRecoveredSigs(); + void ProcessRecoveredSig(std::shared_ptr recoveredSig, bool consider_proactive_relay); + + void Start() override; + void Stop() override; + void Interrupt() override { workInterrupt(); }; + + void WorkThreadMain(); + +private: + llmq::CSigningManager& m_sig_manager; + + std::thread workThread; + CThreadInterrupt workInterrupt; +}; + +#endif // BITCOIN_LLMQ_NET_SIGNING_H diff --git a/src/llmq/signing.cpp b/src/llmq/signing.cpp index d4218f04aad7..195be38b8790 100644 --- a/src/llmq/signing.cpp +++ b/src/llmq/signing.cpp @@ -8,22 +8,14 @@ #include #include #include -#include -#include #include -#include #include -#include -#include -#include #include -#include -#include -#include -#include +#include #include +#include #include namespace llmq @@ -334,10 +326,8 @@ void CRecoveredSigsDb::CleanupOldVotes(int64_t maxAge) ////////////////// -CSigningManager::CSigningManager(const CChainState& chainstate, const CQuorumManager& _qman, - const util::DbWrapperParams& db_params) : +CSigningManager::CSigningManager(const CQuorumManager& _qman, const util::DbWrapperParams& db_params) : db{db_params}, - m_chainstate{chainstate}, qman{_qman} { } @@ -371,55 +361,12 @@ bool CSigningManager::GetRecoveredSigForGetData(const uint256& hash, CRecoveredS return true; } -static bool PreVerifyRecoveredSig(const CQuorumManager& quorum_manager, const CRecoveredSig& recoveredSig, bool& retBan) +void CSigningManager::ProcessRecoveredSig(NodeId from, std::shared_ptr recoveredSig) { - retBan = false; - - auto llmqType = recoveredSig.getLlmqType(); - if (!Params().GetLLMQ(llmqType).has_value()) { - retBan = true; - return false; - } - - auto quorum = quorum_manager.GetQuorum(llmqType, recoveredSig.getQuorumHash()); - - if (!quorum) { - LogPrint(BCLog::LLMQ, "CSigningManager::%s -- quorum %s not found\n", __func__, - recoveredSig.getQuorumHash().ToString()); - return false; - } - if (!IsQuorumActive(llmqType, quorum_manager, quorum->qc->quorumHash)) { - return false; - } - - return true; -} - -MessageProcessingResult CSigningManager::ProcessMessage(NodeId from, std::string_view msg_type, CDataStream& vRecv) -{ - if (msg_type != NetMsgType::QSIGREC) { - return {}; - } - - auto recoveredSig = std::make_shared(); - vRecv >> *recoveredSig; - - MessageProcessingResult ret{}; - ret.m_to_erase = CInv{MSG_QUORUM_RECOVERED_SIG, recoveredSig->GetHash()}; - - bool ban = false; - if (!PreVerifyRecoveredSig(qman, *recoveredSig, ban)) { - if (ban) { - ret.m_error = MisbehavingError{100}; - return ret; - } - return ret; - } - // It's important to only skip seen *valid* sig shares here. See comment for CBatchedSigShare // We don't receive recovered sigs in batches, but we do batched verification per node on these if (db.HasRecoveredSigForHash(recoveredSig->GetHash())) { - return ret; + return; } LogPrint(BCLog::LLMQ, "CSigningManager::%s -- signHash=%s, id=%s, msgHash=%s, node=%d\n", __func__, @@ -430,11 +377,10 @@ MessageProcessingResult CSigningManager::ProcessMessage(NodeId from, std::string // no need to perform full verification LogPrint(BCLog::LLMQ, "CSigningManager::%s -- already pending reconstructed sig, signHash=%s, id=%s, msgHash=%s, node=%d\n", __func__, recoveredSig->buildSignHash().ToString(), recoveredSig->getId().ToString(), recoveredSig->getMsgHash().ToString(), from); - return ret; + return; } - pendingRecoveredSigs[from].emplace_back(recoveredSig); - return ret; + pendingRecoveredSigs[from].emplace_back(std::move(recoveredSig)); } bool CSigningManager::CollectPendingRecoveredSigsToVerify( @@ -512,88 +458,20 @@ bool CSigningManager::CollectPendingRecoveredSigsToVerify( return more_work; } -void CSigningManager::ProcessPendingReconstructedRecoveredSigs(PeerManager& peerman) +Uint256HashMap> CSigningManager::FetchPendingReconstructed() { - decltype(pendingReconstructedRecoveredSigs) m; - WITH_LOCK(cs_pending, swap(m, pendingReconstructedRecoveredSigs)); - - for (const auto& p : m) { - ProcessRecoveredSig(p.second, peerman, /*from=*/-1); - } -} - -bool CSigningManager::ProcessPendingRecoveredSigs(PeerManager& peerman) -{ - std::unordered_map>> recSigsByNode; - std::unordered_map, CQuorumCPtr, StaticSaltedHasher> quorums; - - ProcessPendingReconstructedRecoveredSigs(peerman); - - const size_t nMaxBatchSize{32}; - bool more_work = CollectPendingRecoveredSigsToVerify(nMaxBatchSize, recSigsByNode, quorums); - if (recSigsByNode.empty()) { - return false; - } - - // It's ok to perform insecure batched verification here as we verify against the quorum public keys, which are not - // craftable by individual entities, making the rogue public key attack impossible - CBLSBatchVerifier batchVerifier(false, false); - - size_t verifyCount = 0; - for (const auto& p : recSigsByNode) { - NodeId nodeId = p.first; - const auto& v = p.second; - - for (const auto& recSig : v) { - // we didn't verify the lazy signature until now - if (!recSig->sig.Get().IsValid()) { - batchVerifier.badSources.emplace(nodeId); - break; - } - - const auto& quorum = quorums.at(std::make_pair(recSig->getLlmqType(), recSig->getQuorumHash())); - batchVerifier.PushMessage(nodeId, recSig->GetHash(), recSig->buildSignHash().Get(), recSig->sig.Get(), - quorum->qc->quorumPublicKey); - verifyCount++; - } - } - - cxxtimer::Timer verifyTimer(true); - batchVerifier.Verify(); - verifyTimer.stop(); - - LogPrint(BCLog::LLMQ, "CSigningManager::%s -- verified recovered sig(s). count=%d, vt=%d, nodes=%d\n", __func__, verifyCount, verifyTimer.count(), recSigsByNode.size()); - - Uint256HashSet processed; - for (const auto& p : recSigsByNode) { - NodeId nodeId = p.first; - const auto& v = p.second; - - if (batchVerifier.badSources.count(nodeId)) { - LogPrint(BCLog::LLMQ, "CSigningManager::%s -- invalid recSig from other node, banning peer=%d\n", __func__, nodeId); - peerman.Misbehaving(nodeId, 100); - continue; - } - - for (const auto& recSig : v) { - if (!processed.emplace(recSig->GetHash()).second) { - continue; - } - - ProcessRecoveredSig(recSig, peerman, nodeId); - } - } - - return more_work; + Uint256HashMap> tmp; + WITH_LOCK(cs_pending, swap(tmp, pendingReconstructedRecoveredSigs)); + return tmp; } // signature must be verified already -void CSigningManager::ProcessRecoveredSig(const std::shared_ptr& recoveredSig, PeerManager& peerman, NodeId from) +bool CSigningManager::ProcessRecoveredSig(const std::shared_ptr& recoveredSig) { auto llmqType = recoveredSig->getLlmqType(); if (db.HasRecoveredSigForHash(recoveredSig->GetHash())) { - return; + return false; } auto signHash = recoveredSig->buildSignHash(); @@ -615,7 +493,7 @@ void CSigningManager::ProcessRecoveredSig(const std::shared_ptrGetHash())); - auto listeners = WITH_LOCK(cs_listeners, return recoveredSigsListeners); - for (auto& l : listeners) { - peerman.PostProcessMessage(l->HandleNewRecoveredSig(*recoveredSig)); - } + return true; +} - // TODO refactor to use a better abstraction analogous to IsAllMembersConnectedEnabled - auto proactive_relay = from == -1 && - llmqType != Consensus::LLMQType::LLMQ_100_67 && - llmqType != Consensus::LLMQType::LLMQ_400_60 && - llmqType != Consensus::LLMQType::LLMQ_400_85; - GetMainSignals().NotifyRecoveredSig(recoveredSig, recoveredSig->GetHash().ToString(), proactive_relay); +std::vector CSigningManager::GetListeners() const +{ + LOCK(cs_listeners); + return recoveredSigsListeners; } void CSigningManager::PushReconstructedRecoveredSig(const std::shared_ptr& recoveredSig) @@ -720,47 +594,6 @@ bool CSigningManager::GetVoteForId(Consensus::LLMQType llmqType, const uint256& return db.GetVoteForId(llmqType, id, msgHashRet); } -void CSigningManager::StartWorkerThread(PeerManager& peerman) -{ - // can't start new thread if we have one running already - if (workThread.joinable()) { - assert(false); - } - - workThread = std::thread(&util::TraceThread, "recsigs", [this, &peerman] { WorkThreadMain(peerman); }); -} - -void CSigningManager::StopWorkerThread() -{ - // make sure to call InterruptWorkerThread() first - if (!workInterrupt) { - assert(false); - } - - if (workThread.joinable()) { - workThread.join(); - } -} - -void CSigningManager::InterruptWorkerThread() -{ - workInterrupt(); -} - -void CSigningManager::WorkThreadMain(PeerManager& peerman) -{ - while (!workInterrupt) { - bool fMoreWork = ProcessPendingRecoveredSigs(peerman); - - Cleanup(); - - // TODO Wakeup when pending signing is needed? - if (!fMoreWork && !workInterrupt.sleep_for(std::chrono::milliseconds(100))) { - return; - } - } -} - SignHash CSigBase::buildSignHash() const { return SignHash(llmqType, quorumHash, id, msgHash); } diff --git a/src/llmq/signing.h b/src/llmq/signing.h index e37f79093172..28e6d08930fa 100644 --- a/src/llmq/signing.h +++ b/src/llmq/signing.h @@ -7,23 +7,17 @@ #include #include -#include #include #include -#include - #include #include #include #include -#include +#include #include -#include - #include #include -#include #include class CChainState; @@ -31,7 +25,6 @@ class CDataStream; class CDBBatch; class CDBWrapper; class CInv; -class PeerManager; struct RPCResult; namespace util { struct DbWrapperParams; @@ -42,6 +35,7 @@ class UniValue; namespace llmq { class CQuorumManager; class CSigSharesManager; +class SignHash; // Keep recovered signatures for a week. This is a "-maxrecsigsage" option default. static constexpr int64_t DEFAULT_MAX_RECOVERED_SIGS_AGE{60 * 60 * 24 * 7}; @@ -59,9 +53,7 @@ class CSigBase CSigBase() = default; public: - [[nodiscard]] constexpr auto getLlmqType() const { - return llmqType; - } + [[nodiscard]] constexpr Consensus::LLMQType getLlmqType() const { return llmqType; } [[nodiscard]] constexpr auto getQuorumHash() const -> const uint256& { return quorumHash; @@ -160,6 +152,7 @@ class CRecoveredSigsListener public: virtual ~CRecoveredSigsListener() = default; + // TODO: simplify returned type to std::variant [[nodiscard]] virtual MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) = 0; }; @@ -168,7 +161,6 @@ class CSigningManager private: CRecoveredSigsDb db; - const CChainState& m_chainstate; const CQuorumManager& qman; mutable Mutex cs_pending; @@ -187,14 +179,13 @@ class CSigningManager CSigningManager() = delete; CSigningManager(const CSigningManager&) = delete; CSigningManager& operator=(const CSigningManager&) = delete; - explicit CSigningManager(const CChainState& chainstate, const CQuorumManager& _qman, - const util::DbWrapperParams& db_params); + explicit CSigningManager(const CQuorumManager& _qman, const util::DbWrapperParams& db_params); ~CSigningManager(); bool AlreadyHave(const CInv& inv) const EXCLUSIVE_LOCKS_REQUIRED(!cs_pending); bool GetRecoveredSigForGetData(const uint256& hash, CRecoveredSig& ret) const; - [[nodiscard]] MessageProcessingResult ProcessMessage(NodeId from, std::string_view msg_type, CDataStream& vRecv) + void ProcessRecoveredSig(NodeId from, std::shared_ptr recovered_sig) EXCLUSIVE_LOCKS_REQUIRED(!cs_pending); // This is called when a recovered signature was was reconstructed from another P2P message and is known to be valid @@ -208,20 +199,21 @@ class CSigningManager // DB. This allows AlreadyHave/late-share filtering to keep returning true. Cleanup will later remove the remains void TruncateRecoveredSig(Consensus::LLMQType llmqType, const uint256& id); -private: + // Used by NetSigning: + const CQuorumManager& Qman() { return qman; } + Uint256HashMap> FetchPendingReconstructed() EXCLUSIVE_LOCKS_REQUIRED(!cs_pending); bool CollectPendingRecoveredSigsToVerify( size_t maxUniqueSessions, std::unordered_map>>& retSigShares, std::unordered_map, CQuorumCPtr, StaticSaltedHasher>& retQuorums) EXCLUSIVE_LOCKS_REQUIRED(!cs_pending); - void ProcessPendingReconstructedRecoveredSigs(PeerManager& peerman) - 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 + std::vector GetListeners() const EXCLUSIVE_LOCKS_REQUIRED(!cs_listeners); + // Returns true if recovered sigs should be send to listeners + bool ProcessRecoveredSig(const std::shared_ptr& recoveredSig) + EXCLUSIVE_LOCKS_REQUIRED(!cs_pending); +private: // Used by CSigSharesManager CRecoveredSigsDb& GetDb() { return db; } - void ProcessRecoveredSig(const std::shared_ptr& recoveredSig, PeerManager& peerman, NodeId from) - EXCLUSIVE_LOCKS_REQUIRED(!cs_pending, !cs_listeners); // Needed for access to GetDb() and ProcessRecoveredSig() friend class CSigSharesManager; @@ -239,16 +231,8 @@ class CSigningManager bool GetVoteForId(Consensus::LLMQType llmqType, const uint256& id, uint256& msgHashRet) const; -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); - void StopWorkerThread(); - void InterruptWorkerThread(); + void Cleanup(); }; template diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 8b5e7c375195..c3eb48e7c8d2 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -844,7 +844,16 @@ 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, /*from=*/-1); + if (sigman.ProcessRecoveredSig(singleMemberRecoveredSig)) { + // TODO: remove duplicated code with NetSigning + auto listeners = sigman.GetListeners(); + for (auto& l : listeners) { + m_peerman.PostProcessMessage(l->HandleNewRecoveredSig(*singleMemberRecoveredSig)); + } + + GetMainSignals().NotifyRecoveredSig(singleMemberRecoveredSig, + singleMemberRecoveredSig->GetHash().ToString(), false); + } return; // end of single-quorum processing } @@ -876,7 +885,18 @@ void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id, } } - sigman.ProcessRecoveredSig(rs, m_peerman, /*from=*/-1); + if (sigman.ProcessRecoveredSig(rs)) { + // TODO: remove duplicated code with NetSigning + auto listeners = sigman.GetListeners(); + for (auto& l : listeners) { + m_peerman.PostProcessMessage(l->HandleNewRecoveredSig(*rs)); + } + + auto proactive_relay = rs->getLlmqType() != Consensus::LLMQType::LLMQ_100_67 && + rs->getLlmqType() != Consensus::LLMQType::LLMQ_400_60 && + rs->getLlmqType() != Consensus::LLMQType::LLMQ_400_85; + GetMainSignals().NotifyRecoveredSig(rs, rs->GetHash().ToString(), proactive_relay); + } } CDeterministicMNCPtr CSigSharesManager::SelectMemberForRecovery(const CQuorum& quorum, const uint256 &id, int attempt) diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index 7e8e23eb17a3..b8184f38e2ec 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -7,6 +7,7 @@ #include #include +#include #include #include diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 07aa813c4fe4..0dad56089317 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -652,6 +652,8 @@ class PeerManagerImpl final : public PeerManager void PeerRelayInvFiltered(const CInv& inv, const CTransaction& relatedTx) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void PeerRelayInvFiltered(const CInv& inv, const uint256& relatedTxHash) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void PeerAskPeersForTransaction(const uint256& txid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + void PeerPostProcessMessage(MessageProcessingResult&& ret) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + private: void _RelayTransaction(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_peer_mutex); @@ -5441,7 +5443,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()); - PostProcessMessage(m_llmq_ctx->sigman->ProcessMessage(pfrom.GetId(), msg_type, vRecv), pfrom.GetId()); PostProcessMessage(ProcessPlatformBanMessage(pfrom.GetId(), msg_type, vRecv), pfrom.GetId()); if (msg_type == NetMsgType::CLSIG) { @@ -6567,3 +6568,8 @@ void PeerManagerImpl::PeerAskPeersForTransaction(const uint256& txid) { AskPeersForTransaction(txid); } + +void PeerManagerImpl::PeerPostProcessMessage(MessageProcessingResult&& ret) +{ + PostProcessMessage(std::move(ret), -1); +} diff --git a/src/net_processing.h b/src/net_processing.h index 7dc68e272d53..f0ab54554686 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -64,6 +64,7 @@ class PeerManagerInternal virtual void PeerRelayInvFiltered(const CInv& inv, const CTransaction& relatedTx) = 0; virtual void PeerRelayInvFiltered(const CInv& inv, const uint256& relatedTxHash) = 0; virtual void PeerAskPeersForTransaction(const uint256& txid) = 0; + virtual void PeerPostProcessMessage(MessageProcessingResult&& ret) = 0; }; class NetHandler diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index e591e3166ab6..87a6f11e6fcd 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -23,24 +23,22 @@ # Dash "banman -> common/bloom -> evo/assetlocktx -> llmq/quorums -> net -> banman", "chainlock/chainlock -> instantsend/instantsend -> chainlock/chainlock", - "chainlock/chainlock -> chainlock/signing -> llmq/signing -> net_processing -> chainlock/chainlock", - "chainlock/chainlock -> chainlock/signing -> llmq/signing -> net_processing -> masternode/active/context -> chainlock/chainlock", + "chainlock/chainlock -> chainlock/signing -> llmq/signing_shares -> net_processing -> chainlock/chainlock", + "chainlock/chainlock -> chainlock/signing -> llmq/signing_shares -> net_processing -> llmq/context -> chainlock/chainlock", + "chainlock/chainlock -> chainlock/signing -> llmq/signing_shares -> net_processing -> masternode/active/context -> chainlock/chainlock", "chainlock/chainlock -> instantsend/instantsend -> instantsend/signing -> chainlock/chainlock", "chainlock/chainlock -> llmq/quorums -> msg_result -> coinjoin/coinjoin -> chainlock/chainlock", "chainlock/chainlock -> validation -> chainlock/chainlock", "chainlock/chainlock -> validation -> evo/chainhelper -> chainlock/chainlock", - "chainlock/signing -> llmq/signing -> net_processing -> masternode/active/context -> chainlock/signing", + "chainlock/signing -> llmq/signing_shares -> net_processing -> masternode/active/context -> chainlock/signing", "coinjoin/coinjoin -> instantsend/instantsend -> spork -> msg_result -> coinjoin/coinjoin", - "coinjoin/client -> core_io -> evo/mnhftx -> llmq/signing -> net_processing -> coinjoin/walletman -> coinjoin/client", + "coinjoin/client -> coinjoin/coinjoin -> instantsend/instantsend -> instantsend/signing -> llmq/signing_shares -> net_processing -> coinjoin/walletman -> coinjoin/client", "coinjoin/server -> net_processing -> coinjoin/server", "coinjoin/server -> net_processing -> masternode/active/context -> coinjoin/server", "common/bloom -> evo/assetlocktx -> llmq/commitment -> evo/deterministicmns -> evo/simplifiedmns -> merkleblock -> common/bloom", "common/bloom -> evo/assetlocktx -> llmq/quorums -> net -> common/bloom", "consensus/tx_verify -> evo/assetlocktx -> llmq/commitment -> validation -> consensus/tx_verify", "consensus/tx_verify -> evo/assetlocktx -> llmq/commitment -> validation -> txmempool -> consensus/tx_verify", - "core_io -> evo/mnhftx -> llmq/signing -> net_processing -> evo/smldiff -> core_io", - "core_io -> evo/mnhftx -> llmq/signing -> net_processing -> masternode/active/context -> governance/signing -> governance/classes -> core_io", - "core_io -> evo/mnhftx -> llmq/signing -> net_processing -> masternode/active/context -> governance/signing -> governance/object -> core_io", "evo/assetlocktx -> llmq/commitment -> validation -> txmempool -> evo/assetlocktx", "evo/chainhelper -> evo/specialtxman -> validation -> evo/chainhelper", "evo/deterministicmns -> index/txindex -> validation -> evo/deterministicmns", @@ -53,19 +51,16 @@ "governance/governance -> masternode/sync -> governance/governance", "governance/governance -> net_processing -> masternode/active/context -> governance/governance", "governance/governance -> net_processing -> governance/governance", - "instantsend/instantsend -> instantsend/signing -> llmq/signing -> net_processing -> instantsend/instantsend", - "instantsend/instantsend -> instantsend/signing -> llmq/signing -> net_processing -> masternode/active/context -> instantsend/instantsend", + "instantsend/instantsend -> instantsend/signing -> llmq/signing_shares -> net_processing -> instantsend/instantsend", + "instantsend/instantsend -> instantsend/signing -> llmq/signing_shares -> net_processing -> llmq/context -> instantsend/instantsend", + "instantsend/instantsend -> instantsend/signing -> llmq/signing_shares -> net_processing -> masternode/active/context -> instantsend/instantsend", "instantsend/instantsend -> txmempool -> instantsend/instantsend", - "instantsend/signing -> llmq/signing -> net_processing -> masternode/active/context -> instantsend/signing", + "instantsend/signing -> llmq/signing_shares -> net_processing -> masternode/active/context -> instantsend/signing", "llmq/blockprocessor -> llmq/utils -> llmq/snapshot -> llmq/blockprocessor", "llmq/commitment -> llmq/utils -> llmq/snapshot -> llmq/commitment", - "llmq/context -> llmq/signing -> net_processing -> llmq/context", - "llmq/context -> llmq/signing -> net_processing -> masternode/active/context -> llmq/context", "llmq/dkgsession -> llmq/dkgsessionmgr -> llmq/dkgsessionhandler -> llmq/dkgsession", "llmq/dkgsessionhandler -> net_processing -> llmq/dkgsessionmgr -> llmq/dkgsessionhandler", - "llmq/ehf_signals -> llmq/signing -> net_processing -> masternode/active/context -> llmq/ehf_signals", - "llmq/signing -> llmq/signing_shares -> llmq/signing", - "llmq/signing -> net_processing -> llmq/signing", + "llmq/ehf_signals -> llmq/signing_shares -> net_processing -> masternode/active/context -> llmq/ehf_signals", "llmq/signing_shares -> net_processing -> llmq/signing_shares", "llmq/signing_shares -> net_processing -> masternode/active/context -> llmq/signing_shares", "masternode/payments -> validation -> masternode/payments", From 489090b69374cf362f92c27d8db46c8e235e8eb6 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 14 Nov 2025 06:43:41 +0700 Subject: [PATCH 02/10] refactor: update log records for net_signing --- src/llmq/net_signing.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/llmq/net_signing.cpp b/src/llmq/net_signing.cpp index 806e99d8b7ea..13611304a358 100644 --- a/src/llmq/net_signing.cpp +++ b/src/llmq/net_signing.cpp @@ -137,7 +137,7 @@ bool NetSigning::ProcessPendingRecoveredSigs() batchVerifier.Verify(); verifyTimer.stop(); - LogPrint(BCLog::LLMQ, "CSigningManager::%s -- verified recovered sig(s). count=%d, vt=%d, nodes=%d\n", __func__, + LogPrint(BCLog::LLMQ, "NetSigning::%s -- verified recovered sig(s). count=%d, vt=%d, nodes=%d\n", __func__, verifyCount, verifyTimer.count(), recSigsByNode.size()); Uint256HashSet processed; @@ -146,8 +146,7 @@ bool NetSigning::ProcessPendingRecoveredSigs() const auto& v = p.second; if (batchVerifier.badSources.count(nodeId)) { - LogPrint(BCLog::LLMQ, "CSigningManager::%s -- invalid recSig from other node, banning peer=%d\n", __func__, - nodeId); + LogPrint(BCLog::LLMQ, "NetSigning::%s -- invalid recSig from other node, banning peer=%d\n", __func__, nodeId); m_peer_manager->PeerMisbehaving(nodeId, 100); continue; } From 0e4c80b26965a12730636a4926ee47ba5d4a4b87 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 19 Nov 2025 00:59:27 +0700 Subject: [PATCH 03/10] refactor: duplicated code for single-node quorums and regular quorums inside TryRecoverSig --- src/llmq/signing_shares.cpp | 63 ++++++++++++++++--------------------- src/llmq/signing_shares.h | 3 +- 2 files changed, 29 insertions(+), 37 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index c3eb48e7c8d2..ce1e3d51d05d 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -789,35 +789,50 @@ void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CQuorum } if (canTryRecovery) { - TryRecoverSig(*quorum, sigShare.getId(), sigShare.getMsgHash()); + auto rs = TryRecoverSig(*quorum, sigShare.getId(), sigShare.getMsgHash()); + if (rs != nullptr) { + if (sigman.ProcessRecoveredSig(rs)) { + // TODO: remove duplicated code with NetSigning + auto listeners = sigman.GetListeners(); + for (auto& l : listeners) { + m_peerman.PostProcessMessage(l->HandleNewRecoveredSig(*rs)); + } + + bool proactive_relay = rs->getLlmqType() != Consensus::LLMQType::LLMQ_100_67 && + rs->getLlmqType() != Consensus::LLMQType::LLMQ_400_60 && + rs->getLlmqType() != Consensus::LLMQType::LLMQ_400_85; + GetMainSignals().NotifyRecoveredSig(rs, rs->GetHash().ToString(), proactive_relay); + } + } } } -void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id, const uint256& msgHash) +std::shared_ptr CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id, + const uint256& msgHash) { if (sigman.HasRecoveredSigForId(quorum.params.type, id)) { - return; + return nullptr; } std::vector sigSharesForRecovery; std::vector idsForRecovery; - std::shared_ptr singleMemberRecoveredSig; { LOCK(cs); auto signHash = SignHash(quorum.params.type, quorum.qc->quorumHash, id, msgHash).Get(); const auto* sigSharesForSignHash = sigShares.GetAllForSignHash(signHash); if (sigSharesForSignHash == nullptr) { - return; + return nullptr; } + std::shared_ptr singleMemberRecoveredSig; if (quorum.params.is_single_member()) { if (sigSharesForSignHash->empty()) { LogPrint(BCLog::LLMQ_SIGS, /* Continued */ "CSigSharesManager::%s -- impossible to recover single-node signature - no shares yet. id=%s, " "msgHash=%s\n", __func__, id.ToString(), msgHash.ToString()); - return; + return nullptr; } const auto& sigShare = sigSharesForSignHash->begin()->second; CBLSSignature recoveredSig = sigShare.sigShare.Get(); @@ -838,23 +853,11 @@ void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id, // check if we can recover the final signature if (sigSharesForRecovery.size() < size_t(quorum.params.threshold)) { - return; + return nullptr; } - } - - // Handle single-member quorum case after releasing the lock - if (singleMemberRecoveredSig) { - if (sigman.ProcessRecoveredSig(singleMemberRecoveredSig)) { - // TODO: remove duplicated code with NetSigning - auto listeners = sigman.GetListeners(); - for (auto& l : listeners) { - m_peerman.PostProcessMessage(l->HandleNewRecoveredSig(*singleMemberRecoveredSig)); - } - - GetMainSignals().NotifyRecoveredSig(singleMemberRecoveredSig, - singleMemberRecoveredSig->GetHash().ToString(), false); + if (quorum.params.is_single_member()) { + return singleMemberRecoveredSig; // end of single-quorum processing } - return; // end of single-quorum processing } // now recover it @@ -863,7 +866,7 @@ void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id, if (!recoveredSig.Recover(sigSharesForRecovery, idsForRecovery)) { LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- failed to recover signature. id=%s, msgHash=%s, time=%d\n", __func__, id.ToString(), msgHash.ToString(), t.count()); - return; + return nullptr; } LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- recovered signature. id=%s, msgHash=%s, time=%d\n", __func__, @@ -881,22 +884,10 @@ void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id, // this should really not happen as we have verified all signature shares before LogPrintf("CSigSharesManager::%s -- own recovered signature is invalid. id=%s, msgHash=%s\n", __func__, id.ToString(), msgHash.ToString()); - return; + return nullptr; } } - - if (sigman.ProcessRecoveredSig(rs)) { - // TODO: remove duplicated code with NetSigning - auto listeners = sigman.GetListeners(); - for (auto& l : listeners) { - m_peerman.PostProcessMessage(l->HandleNewRecoveredSig(*rs)); - } - - auto proactive_relay = rs->getLlmqType() != Consensus::LLMQType::LLMQ_100_67 && - rs->getLlmqType() != Consensus::LLMQType::LLMQ_400_60 && - rs->getLlmqType() != Consensus::LLMQType::LLMQ_400_85; - GetMainSignals().NotifyRecoveredSig(rs, rs->GetHash().ToString(), proactive_relay); - } + return rs; } CDeterministicMNCPtr CSigSharesManager::SelectMemberForRecovery(const CQuorum& quorum, const uint256 &id, int attempt) diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index b8184f38e2ec..59fe5caff1b0 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -479,7 +479,8 @@ class CSigSharesManager : public CRecoveredSigsListener EXCLUSIVE_LOCKS_REQUIRED(!cs); 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); + std::shared_ptr TryRecoverSig(const CQuorum& quorum, const uint256& id, const uint256& msgHash) + EXCLUSIVE_LOCKS_REQUIRED(!cs); bool GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo) EXCLUSIVE_LOCKS_REQUIRED(!cs); From 1d34321b4b32941b7aa5896848403d5bff231413 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 19 Nov 2025 04:05:43 +0700 Subject: [PATCH 04/10] refactor: drop unused includes of llmq/signing.h over the codebase --- src/evo/mnhftx.cpp | 1 - src/init.cpp | 1 - src/test/evo_islock_tests.cpp | 1 - src/test/fuzz/process_message.cpp | 9 --------- 4 files changed, 12 deletions(-) diff --git a/src/evo/mnhftx.cpp b/src/evo/mnhftx.cpp index 54e5d0c29ce0..b71e28f2e7b7 100644 --- a/src/evo/mnhftx.cpp +++ b/src/evo/mnhftx.cpp @@ -10,7 +10,6 @@ #include #include #include -#include #include #include diff --git a/src/init.cpp b/src/init.cpp index 6170c66817a9..5bd21994fc6d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -94,7 +94,6 @@ #include #include #include -#include #include #include #include diff --git a/src/test/evo_islock_tests.cpp b/src/test/evo_islock_tests.cpp index 7dc76475941d..4d40b139af58 100644 --- a/src/test/evo_islock_tests.cpp +++ b/src/test/evo_islock_tests.cpp @@ -6,7 +6,6 @@ #include #include #include -#include #include #include #include diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index c765c656ad76..0049ffc3d353 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -4,12 +4,9 @@ #include #include -#include -#include #include #include