refactor: remove dependency of chainlocks on PeerManager

Author: knst

src/llmq/chainlocks.cpp

@@ -11,7 +11,6 @@
 #include <chainparams.h>
 #include <consensus/validation.h>
 #include <masternode/sync.h>
-#include <net_processing.h>
 #include <node/blockstorage.h>
 #include <node/ui_interface.h>
 #include <scheduler.h>
@@ -34,19 +33,18 @@ std::unique_ptr<CChainLocksHandler> chainLocksHandler;
 
 CChainLocksHandler::CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSigningManager& _sigman,
                                        CSigSharesManager& _shareman, CSporkManager& sporkman, CTxMemPool& _mempool,
-                                       const CMasternodeSync& mn_sync, const std::unique_ptr<PeerManager>& peerman,
-                                       bool is_masternode) :
+                                       const CMasternodeSync& mn_sync, bool is_masternode) :
     m_chainstate(chainstate),
     qman(_qman),
     sigman(_sigman),
     shareman(_shareman),
     spork_manager(sporkman),
     mempool(_mempool),
     m_mn_sync(mn_sync),
-    m_peerman(peerman),
     m_is_masternode{is_masternode},
     scheduler(std::make_unique<CScheduler>()),
-    scheduler_thread(std::make_unique<std::thread>(std::thread(util::TraceThread, "cl-schdlr", [&] { scheduler->serviceQueue(); })))
+    scheduler_thread(
+        std::make_unique<std::thread>(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<const CBlockIndex*> 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

src/llmq/chainlocks.h

@@ -29,7 +29,6 @@ class CMasternodeSync;
 class CScheduler;
 class CSporkManager;
 class CTxMemPool;
-class PeerManager;
 
 namespace llmq
 {
@@ -53,7 +52,6 @@ class CChainLocksHandler : public CRecoveredSigsListener
     CSporkManager& spork_manager;
     CTxMemPool& mempool;
     const CMasternodeSync& m_mn_sync;
-    const std::unique_ptr<PeerManager>& m_peerman;
 
     const bool m_is_masternode;
     std::unique_ptr<CScheduler> scheduler;
@@ -89,8 +87,7 @@ class CChainLocksHandler : public CRecoveredSigsListener
 public:
     explicit CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSigningManager& _sigman,
                                 CSigSharesManager& _shareman, CSporkManager& sporkman, CTxMemPool& _mempool,
-                                const CMasternodeSync& mn_sync, const std::unique_ptr<PeerManager>& peerman,
-                                bool is_masternode);
+                                const CMasternodeSync& mn_sync, bool is_masternode);
     ~CChainLocksHandler();
 
     void Start();
@@ -100,8 +97,8 @@ class CChainLocksHandler : public CRecoveredSigsListener
     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<const CBlockIndex*> pindexNew) EXCLUSIVE_LOCKS_REQUIRED(!cs);
     void UpdatedBlockTip();
@@ -111,7 +108,8 @@ class CChainLocksHandler : public CRecoveredSigsListener
     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);

src/llmq/context.cpp

@@ -36,7 +36,7 @@ LLMQContext::LLMQContext(CChainState& chainstate, CConnman& connman, CDeterminis
     shareman{std::make_unique<llmq::CSigSharesManager>(connman, *sigman, mn_activeman, *qman, sporkman, peerman)},
     clhandler{[&]() -> llmq::CChainLocksHandler* const {
         assert(llmq::chainLocksHandler == nullptr);
-        llmq::chainLocksHandler = std::make_unique<llmq::CChainLocksHandler>(chainstate, *qman, *sigman, *shareman, sporkman, mempool, mn_sync, peerman, is_masternode);
+        llmq::chainLocksHandler = std::make_unique<llmq::CChainLocksHandler>(chainstate, *qman, *sigman, *shareman, sporkman, mempool, mn_sync, is_masternode);
         return llmq::chainLocksHandler.get();
     }()},
     isman{[&]() -> llmq::CInstantSendManager* const {

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;
     }

src/rpc/quorums.cpp

@@ -5,6 +5,7 @@
 #include <chainparams.h>
 #include <deploymentstatus.h>
 #include <index/txindex.h>
+#include <net_processing.h>
 #include <node/context.h>
 #include <rpc/blockchain.h>
 #include <rpc/server.h>
@@ -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();
 },
     };

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"
 )