From f1c6d178794ae9147273be539c3b0bdad2d7d65a Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 26 Sep 2024 18:39:11 +0700 Subject: [PATCH] refactor: remove dependency of chainlocks on PeerManager --- src/llmq/chainlocks.cpp | 45 ++++++------------------- src/llmq/chainlocks.h | 12 +++---- src/llmq/context.cpp | 2 +- src/net_processing.cpp | 13 ++++++- src/rpc/quorums.cpp | 4 ++- test/lint/lint-circular-dependencies.sh | 3 +- 6 files changed, 32 insertions(+), 47 deletions(-) diff --git a/src/llmq/chainlocks.cpp b/src/llmq/chainlocks.cpp index e46b3bd7f5..1ef7d22b91 100644 --- a/src/llmq/chainlocks.cpp +++ b/src/llmq/chainlocks.cpp @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include @@ -34,8 +33,7 @@ std::unique_ptr chainLocksHandler; CChainLocksHandler::CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSigningManager& _sigman, CSigSharesManager& _shareman, CSporkManager& sporkman, CTxMemPool& _mempool, - const CMasternodeSync& mn_sync, const std::unique_ptr& peerman, - bool is_masternode) : + const CMasternodeSync& mn_sync, bool is_masternode) : m_chainstate(chainstate), qman(_qman), sigman(_sigman), @@ -43,10 +41,10 @@ CChainLocksHandler::CChainLocksHandler(CChainState& chainstate, CQuorumManager& spork_manager(sporkman), mempool(_mempool), m_mn_sync(mn_sync), - m_peerman(peerman), m_is_masternode{is_masternode}, scheduler(std::make_unique()), - scheduler_thread(std::make_unique(std::thread(util::TraceThread, "cl-schdlr", [&] { scheduler->serviceQueue(); }))) + scheduler_thread( + std::make_unique(std::thread(util::TraceThread, "cl-schdlr", [&] { scheduler->serviceQueue(); }))) { } @@ -98,31 +96,11 @@ CChainLockSig CChainLocksHandler::GetBestChainLock() const return bestChainLock; } -PeerMsgRet CChainLocksHandler::ProcessMessage(const CNode& pfrom, const std::string& msg_type, CDataStream& vRecv) -{ - if (!AreChainLocksEnabled(spork_manager)) { - return {}; - } - - if (msg_type == NetMsgType::CLSIG) { - CChainLockSig clsig; - vRecv >> clsig; - - return ProcessNewChainLock(pfrom.GetId(), clsig, ::SerializeHash(clsig)); - } - return {}; -} - -PeerMsgRet CChainLocksHandler::ProcessNewChainLock(const NodeId from, const llmq::CChainLockSig& clsig, const uint256& hash) +MessageProcessingResult CChainLocksHandler::ProcessNewChainLock(const NodeId from, const llmq::CChainLockSig& clsig, + const uint256& hash) { CheckActiveState(); - CInv clsigInv(MSG_CLSIG, hash); - - if (from != -1) { - WITH_LOCK(::cs_main, Assert(m_peerman)->EraseObjectRequest(from, clsigInv)); - } - { LOCK(cs); if (!seenChainLocks.emplace(hash, GetTimeMillis()).second) { @@ -138,7 +116,7 @@ PeerMsgRet CChainLocksHandler::ProcessNewChainLock(const NodeId from, const llmq if (const auto ret = VerifyChainLock(clsig); ret != VerifyRecSigStatus::Valid) { LogPrint(BCLog::CHAINLOCKS, "CChainLocksHandler::%s -- invalid CLSIG (%s), status=%d peer=%d\n", __func__, clsig.ToString(), ToUnderlying(ret), from); if (from != -1) { - return tl::unexpected{10}; + return MisbehavingError{10}; } return {}; } @@ -167,14 +145,12 @@ PeerMsgRet CChainLocksHandler::ProcessNewChainLock(const NodeId from, const llmq // Note: make sure to still relay clsig further. } - // Note: do not hold cs while calling RelayInv - AssertLockNotHeld(cs); - Assert(m_peerman)->RelayInv(clsigInv); + CInv clsigInv(MSG_CLSIG, hash); if (pindex == nullptr) { // we don't know the block/header for this CLSIG yet, so bail out for now // when the block or the header later comes in, we will enforce the correct chain - return {}; + return clsigInv; } scheduler->scheduleFromNow([&]() { @@ -184,7 +160,7 @@ PeerMsgRet CChainLocksHandler::ProcessNewChainLock(const NodeId from, const llmq LogPrint(BCLog::CHAINLOCKS, "CChainLocksHandler::%s -- processed new CLSIG (%s), peer=%d\n", __func__, clsig.ToString(), from); - return {}; + return clsigInv; } void CChainLocksHandler::AcceptedBlockHeader(gsl::not_null pindexNew) @@ -547,8 +523,7 @@ MessageProcessingResult CChainLocksHandler::HandleNewRecoveredSig(const llmq::CR clsig = CChainLockSig(lastSignedHeight, lastSignedMsgHash, recoveredSig.sig.Get()); } - ProcessNewChainLock(-1, clsig, ::SerializeHash(clsig)); - return {}; + return ProcessNewChainLock(-1, clsig, ::SerializeHash(clsig)); } bool CChainLocksHandler::HasChainLock(int nHeight, const uint256& blockHash) const diff --git a/src/llmq/chainlocks.h b/src/llmq/chainlocks.h index 7a7682fa47..2c44fb1ec6 100644 --- a/src/llmq/chainlocks.h +++ b/src/llmq/chainlocks.h @@ -29,7 +29,6 @@ class CMasternodeSync; class CScheduler; class CSporkManager; class CTxMemPool; -class PeerManager; namespace llmq { @@ -53,7 +52,6 @@ private: CSporkManager& spork_manager; CTxMemPool& mempool; const CMasternodeSync& m_mn_sync; - const std::unique_ptr& m_peerman; const bool m_is_masternode; std::unique_ptr scheduler; @@ -89,8 +87,7 @@ private: public: explicit CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSigningManager& _sigman, CSigSharesManager& _shareman, CSporkManager& sporkman, CTxMemPool& _mempool, - const CMasternodeSync& mn_sync, const std::unique_ptr& peerman, - bool is_masternode); + const CMasternodeSync& mn_sync, bool is_masternode); ~CChainLocksHandler(); void Start(); @@ -100,8 +97,8 @@ public: bool GetChainLockByHash(const uint256& hash, CChainLockSig& ret) const EXCLUSIVE_LOCKS_REQUIRED(!cs); CChainLockSig GetBestChainLock() const EXCLUSIVE_LOCKS_REQUIRED(!cs); - PeerMsgRet ProcessMessage(const CNode& pfrom, const std::string& msg_type, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs); - PeerMsgRet ProcessNewChainLock(NodeId from, const CChainLockSig& clsig, const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(!cs); + [[nodiscard]] MessageProcessingResult ProcessNewChainLock(NodeId from, const CChainLockSig& clsig, + const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(!cs); void AcceptedBlockHeader(gsl::not_null pindexNew) EXCLUSIVE_LOCKS_REQUIRED(!cs); void UpdatedBlockTip(); @@ -111,7 +108,8 @@ public: void CheckActiveState() EXCLUSIVE_LOCKS_REQUIRED(!cs); void TrySignChainTip() EXCLUSIVE_LOCKS_REQUIRED(!cs); void EnforceBestChainLock() EXCLUSIVE_LOCKS_REQUIRED(!cs); - [[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override EXCLUSIVE_LOCKS_REQUIRED(!cs); + [[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override + EXCLUSIVE_LOCKS_REQUIRED(!cs); bool HasChainLock(int nHeight, const uint256& blockHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); bool HasConflictingChainLock(int nHeight, const uint256& blockHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); diff --git a/src/llmq/context.cpp b/src/llmq/context.cpp index aee90f9f8b..bd6ab30f66 100644 --- a/src/llmq/context.cpp +++ b/src/llmq/context.cpp @@ -36,7 +36,7 @@ LLMQContext::LLMQContext(CChainState& chainstate, CConnman& connman, CDeterminis shareman{std::make_unique(connman, *sigman, mn_activeman, *qman, sporkman, peerman)}, clhandler{[&]() -> llmq::CChainLocksHandler* const { assert(llmq::chainLocksHandler == nullptr); - llmq::chainLocksHandler = std::make_unique(chainstate, *qman, *sigman, *shareman, sporkman, mempool, mn_sync, peerman, is_masternode); + llmq::chainLocksHandler = std::make_unique(chainstate, *qman, *sigman, *shareman, sporkman, mempool, mn_sync, is_masternode); return llmq::chainLocksHandler.get(); }()}, isman{[&]() -> llmq::CInstantSendManager* const { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c7338ebec7..18a8e8bbb0 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4999,7 +4999,18 @@ void PeerManagerImpl::ProcessMessage( ProcessPeerMsgRet(m_llmq_ctx->qman->ProcessMessage(pfrom, msg_type, vRecv), pfrom); m_llmq_ctx->shareman->ProcessMessage(pfrom, m_sporkman, msg_type, vRecv); ProcessPeerMsgRet(m_llmq_ctx->sigman->ProcessMessage(pfrom, msg_type, vRecv), pfrom); - ProcessPeerMsgRet(m_llmq_ctx->clhandler->ProcessMessage(pfrom, msg_type, vRecv), pfrom); + + if (msg_type == NetMsgType::CLSIG) { + if (llmq::AreChainLocksEnabled(m_sporkman)) { + llmq::CChainLockSig clsig; + vRecv >> clsig; + const uint256& hash = ::SerializeHash(clsig); + WITH_LOCK(::cs_main, EraseObjectRequest(pfrom.GetId(), CInv{MSG_CLSIG, hash})); + PostProcessMessage(m_llmq_ctx->clhandler->ProcessNewChainLock(pfrom.GetId(), clsig, hash), pfrom.GetId()); + } + return; // CLSIG + } + ProcessPeerMsgRet(m_llmq_ctx->isman->ProcessMessage(pfrom, msg_type, vRecv), pfrom); return; } diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index 0ccff814a9..763d89bbd4 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -1121,7 +1122,8 @@ static RPCHelpMan submitchainlock() throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature"); } - llmq_ctx.clhandler->ProcessNewChainLock(-1, clsig, ::SerializeHash(clsig)); + PeerManager& peerman = EnsurePeerman(node); + peerman.PostProcessMessage(llmq_ctx.clhandler->ProcessNewChainLock(-1, clsig, ::SerializeHash(clsig))); return llmq_ctx.clhandler->GetBestChainLock().getHeight(); }, }; diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh index e14704c0a9..937e206d2d 100755 --- a/test/lint/lint-circular-dependencies.sh +++ b/test/lint/lint-circular-dependencies.sh @@ -90,13 +90,12 @@ EXPECTED_CIRCULAR_DEPENDENCIES=( "coinjoin/server -> net_processing -> coinjoin/server" "llmq/context -> llmq/ehf_signals -> net_processing -> llmq/context" "llmq/blockprocessor -> net_processing -> llmq/blockprocessor" - "llmq/chainlocks -> net_processing -> llmq/chainlocks" + "llmq/chainlocks -> llmq/instantsend -> net_processing -> llmq/chainlocks" "net_processing -> spork -> net_processing" "evo/simplifiedmns -> llmq/blockprocessor -> net_processing -> evo/simplifiedmns" "governance/governance -> net_processing -> governance/governance" "llmq/blockprocessor -> net_processing -> llmq/context -> llmq/blockprocessor" "llmq/blockprocessor -> net_processing -> llmq/quorums -> llmq/blockprocessor" - "llmq/chainlocks -> net_processing -> llmq/context -> llmq/chainlocks" "rpc/blockchain -> rpc/server -> rpc/blockchain" )