From 4668db60a2c6abbf138f4d7ce4fd1dd10ec6a98f Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 21 Nov 2024 13:43:59 -0600 Subject: [PATCH 1/2] refactor: create helper function RelayRecoveredSig inside peerman this commit should have a few benefits: 1. previous logic using ForEachNode results in locking m_nodes_mutex, a highly contended RecursiveMutex, AND m_peer_mutex(in GetPeerRef) 2. prior also resulted in calling .find over the m_peer_map for each node. Basically old logic was (probably) O(n(nlogn) the new logic results in acquiring m_peer_mutex once and looping over the list of peers, (probably) O(n) 3. Moves networking logic out of llmq/ and into actual net_processing.cpp --- src/llmq/signing.cpp | 7 +------ src/net.h | 2 -- src/net_processing.cpp | 16 +++++++++++++++- src/net_processing.h | 3 +++ 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/llmq/signing.cpp b/src/llmq/signing.cpp index 68b4949438c07..4258eecc837e0 100644 --- a/src/llmq/signing.cpp +++ b/src/llmq/signing.cpp @@ -633,12 +633,7 @@ void CSigningManager::ProcessRecoveredSig(const std::shared_ptrGetHash())); if (m_mn_activeman != nullptr) { - CInv inv(MSG_QUORUM_RECOVERED_SIG, recoveredSig->GetHash()); - connman.ForEachNode([&](const CNode* pnode) { - if (pnode->fSendRecSigs) { - Assert(m_peerman)->PushInventory(pnode->GetId(), inv); - } - }); + Assert(m_peerman)->RelayRecoveredSig(recoveredSig->GetHash()); } auto listeners = WITH_LOCK(cs_listeners, return recoveredSigsListeners); diff --git a/src/net.h b/src/net.h index 89ea78a4b96a8..22fcc827b01cd 100644 --- a/src/net.h +++ b/src/net.h @@ -976,8 +976,6 @@ class CNode // If true, we will send him CoinJoin queue messages std::atomic fSendDSQueue{false}; - // If true, we will announce/send him plain recovered sigs (usually true for full nodes) - std::atomic fSendRecSigs{false}; // If true, we will send him all quorum related messages, even if he is not a member of our quorums std::atomic qwatch{false}; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b6f75dfa00155..3c10f519f4ea3 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -271,6 +271,8 @@ struct Peer { std::atomic m_ping_start{0us}; /** Whether a ping has been requested by the user */ std::atomic m_ping_queued{false}; + /** Whether the peer has requested to receive llmq recovered signatures */ + std::atomic m_wants_recsigs{false}; struct TxRelay { mutable RecursiveMutex m_bloom_filter_mutex; @@ -607,6 +609,7 @@ class PeerManagerImpl final : public PeerManager void RelayInvFiltered(CInv &inv, const CTransaction &relatedTx, const int minProtoVersion) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void RelayInvFiltered(CInv &inv, const uint256 &relatedTxHash, const int minProtoVersion) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void RelayTransaction(const uint256& txid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + void RelayRecoveredSig(const uint256& sigHash) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void SetBestHeight(int height) override { m_best_height = height; }; void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message = "") override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv, @@ -2339,6 +2342,17 @@ void PeerManagerImpl::RelayTransaction(const uint256& txid) }; } +void PeerManagerImpl::RelayRecoveredSig(const uint256& sigHash) +{ + const CInv inv{MSG_QUORUM_RECOVERED_SIG, sigHash}; + LOCK(m_peer_mutex); + for (const auto& [_, peer] : m_peer_map) { + if (peer->m_wants_recsigs) { + PushInv(*peer, inv); + } + } +} + void PeerManagerImpl::RelayAddress(NodeId originator, const CAddress& addr, bool fReachable) @@ -3948,7 +3962,7 @@ void PeerManagerImpl::ProcessMessage( if (msg_type == NetMsgType::QSENDRECSIGS) { bool b; vRecv >> b; - pfrom.fSendRecSigs = b; + peer->m_wants_recsigs = b; return; } diff --git a/src/net_processing.h b/src/net_processing.h index a857201516053..99144d2335d05 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -109,6 +109,9 @@ class PeerManager : public CValidationInterface, public NetEventsInterface virtual void RelayTransaction(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(cs_main) = 0; + /** Relay recovered sigs to all interested peers */ + virtual void RelayRecoveredSig(const uint256& sigHash) = 0; + /** Set the best height */ virtual void SetBestHeight(int height) = 0; From 86e92c376a71c5cf5b061c0e7c178990c5d61c9f Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Sat, 23 Nov 2024 01:54:18 +0700 Subject: [PATCH 2/2] refactor: drop unused CConnman from CSigningManager --- src/llmq/context.cpp | 2 +- src/llmq/instantsend.h | 1 + src/llmq/signing.cpp | 4 ++-- src/llmq/signing.h | 4 +--- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/llmq/context.cpp b/src/llmq/context.cpp index 94da64087ca36..31857000d508e 100644 --- a/src/llmq/context.cpp +++ b/src/llmq/context.cpp @@ -33,7 +33,7 @@ LLMQContext::LLMQContext(ChainstateManager& chainman, CConnman& connman, CDeterm qman{std::make_unique(*bls_worker, chainman.ActiveChainstate(), connman, dmnman, *qdkgsman, evo_db, *quorum_block_processor, mn_activeman, mn_sync, sporkman, unit_tests, wipe)}, - sigman{std::make_unique(connman, mn_activeman, chainman.ActiveChainstate(), *qman, peerman, + sigman{std::make_unique(mn_activeman, chainman.ActiveChainstate(), *qman, peerman, unit_tests, wipe)}, shareman{std::make_unique(connman, *sigman, mn_activeman, *qman, sporkman, peerman)}, clhandler{[&]() -> llmq::CChainLocksHandler* const { diff --git a/src/llmq/instantsend.h b/src/llmq/instantsend.h index 5f8f9e18bd680..c0a27d74944b8 100644 --- a/src/llmq/instantsend.h +++ b/src/llmq/instantsend.h @@ -22,6 +22,7 @@ class CBlockIndex; class CChainState; +class CConnman; class CDBWrapper; class CMasternodeSync; class CSporkManager; diff --git a/src/llmq/signing.cpp b/src/llmq/signing.cpp index 4258eecc837e0..9aa5a72ec816f 100644 --- a/src/llmq/signing.cpp +++ b/src/llmq/signing.cpp @@ -348,9 +348,9 @@ void CRecoveredSigsDb::CleanupOldVotes(int64_t maxAge) ////////////////// -CSigningManager::CSigningManager(CConnman& _connman, const CActiveMasternodeManager* const mn_activeman, const CChainState& chainstate, +CSigningManager::CSigningManager(const CActiveMasternodeManager* const mn_activeman, const CChainState& chainstate, const CQuorumManager& _qman, const std::unique_ptr& peerman, bool fMemory, bool fWipe) : - db(fMemory, fWipe), connman(_connman), m_mn_activeman(mn_activeman), m_chainstate(chainstate), qman(_qman), m_peerman(peerman) + db(fMemory, fWipe), m_mn_activeman(mn_activeman), m_chainstate(chainstate), qman(_qman), m_peerman(peerman) { } diff --git a/src/llmq/signing.h b/src/llmq/signing.h index e3519cb3abf2f..e044c51bf6e7f 100644 --- a/src/llmq/signing.h +++ b/src/llmq/signing.h @@ -19,7 +19,6 @@ class CActiveMasternodeManager; class CChainState; -class CConnman; class CDataStream; class CDBBatch; class CDBWrapper; @@ -160,7 +159,6 @@ class CSigningManager private: CRecoveredSigsDb db; - CConnman& connman; const CActiveMasternodeManager* const m_mn_activeman; const CChainState& m_chainstate; const CQuorumManager& qman; @@ -179,7 +177,7 @@ class CSigningManager std::vector recoveredSigsListeners GUARDED_BY(cs_listeners); public: - CSigningManager(CConnman& _connman, const CActiveMasternodeManager* const mn_activeman, const CChainState& chainstate, + CSigningManager(const CActiveMasternodeManager* const mn_activeman, const CChainState& chainstate, const CQuorumManager& _qman, const std::unique_ptr& peerman, bool fMemory, bool fWipe); bool AlreadyHave(const CInv& inv) const;