refactor: drop dependency of spork manager on network code

Author: knst

src/net_processing.cpp

@@ -5462,6 +5462,23 @@ void PeerManagerImpl::ProcessMessage(
         return;
     }
 
+    if (msg_type == NetMsgType::SPORK) {
+        CSporkMessage spork;
+        vRecv >> spork;
+
+        uint256 hash = spork.GetHash();
+        CInv spork_inv{MSG_SPORK, hash};
+        WITH_LOCK(::cs_main, EraseObjectRequest(pfrom.GetId(), spork_inv));
+        if (!m_sporkman.IsSporkValid(spork)) {
+            Misbehaving(pfrom.GetId(), 100);
+            return;
+        }
+        if (m_sporkman.ProcessSpork(spork)) {
+            RelayInv(spork_inv);
+        }
+        return;
+    }
+
     if (msg_type == NetMsgType::GETSPORKS) {
         // For 'getsporks', active sporks is sent to the requesting peer.
         auto active_sporks = m_sporkman.ActiveSporks();
@@ -5470,6 +5487,7 @@ void PeerManagerImpl::ProcessMessage(
                 m_connman.PushMessage(&pfrom, CNetMsgMaker(pfrom.GetCommonVersion()).Make(NetMsgType::SPORK, signerSporkPair.second));
             }
         }
+        return;
     }
 
     if (msg_type == NetMsgType::QUORUMROTATIONINFO) {
@@ -5518,7 +5536,6 @@ void PeerManagerImpl::ProcessMessage(
             PostProcessMessage(m_cj_walletman->processMessage(pfrom, m_chainman.ActiveChainstate(), m_connman, m_mempool, msg_type, vRecv), pfrom.GetId());
         }
         PostProcessMessage(DKGSessionProcessMessage(pfrom, is_masternode, msg_type, vRecv), pfrom.GetId());
-        PostProcessMessage(m_sporkman.ProcessMessage(pfrom, m_connman, msg_type, vRecv), pfrom.GetId());
         PostProcessMessage(CMNAuth::ProcessMessage(pfrom, peer->m_their_services, m_connman, m_mn_metaman, (m_active_ctx ? m_active_ctx->nodeman.get() : nullptr), m_mn_sync, m_dmnman->GetListAtChainTip(), msg_type, vRecv), pfrom.GetId());
         PostProcessMessage(m_llmq_ctx->quorum_block_processor->ProcessMessage(pfrom, msg_type, vRecv), pfrom.GetId());
         PostProcessMessage(ProcessPlatformBanMessage(pfrom.GetId(), msg_type, vRecv), pfrom.GetId());

src/spork.cpp

@@ -4,17 +4,15 @@
 
 #include <spork.h>
 
-#include <flat-database.h>
-#include <util/helpers.h>
-
 #include <chainparams.h>
+#include <flat-database.h>
 #include <key_io.h>
 #include <logging.h>
 #include <messagesigner.h>
-#include <net.h>
 #include <protocol.h>
 #include <script/standard.h>
 #include <timedata.h>
+#include <util/helpers.h>
 #include <util/message.h> // for MESSAGE_MAGIC
 #include <util/string.h>
 
@@ -125,49 +123,37 @@ void CSporkManager::CheckAndRemove()
     }
 }
 
-MessageProcessingResult CSporkManager::ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv)
+bool CSporkManager::IsSporkValid(const CSporkMessage& spork)
 {
-    if (msg_type == NetMsgType::SPORK) {
-        return ProcessSpork(peer.GetId(), vRecv);
-    }
-    return {};
-}
-
-MessageProcessingResult CSporkManager::ProcessSpork(NodeId from, CDataStream& vRecv)
-{
-    CSporkMessage spork;
-    vRecv >> spork;
-
-    uint256 hash = spork.GetHash();
-
-    MessageProcessingResult ret{};
-    ret.m_to_erase = CInv{MSG_SPORK, hash};
-
     if (spork.nTimeSigned > GetAdjustedTime() + 2 * 60 * 60) {
         LogPrint(BCLog::SPORK, "CSporkManager::ProcessSpork -- ERROR: too far into the future\n");
-        ret.m_error = MisbehavingError{100};
-        return ret;
+        return false;
     }
 
     auto opt_keyIDSigner = spork.GetSignerKeyID();
 
     if (opt_keyIDSigner == std::nullopt || WITH_LOCK(cs, return !setSporkPubKeyIDs.count(*opt_keyIDSigner))) {
         LogPrint(BCLog::SPORK, "CSporkManager::ProcessSpork -- ERROR: invalid signature\n");
-        ret.m_error = MisbehavingError{100};
-        return ret;
+        return false;
     }
+    return true;
+}
 
-    std::string strLogMsg{strprintf("SPORK -- hash: %s id: %d value: %10d peer=%d", hash.ToString(), spork.nSporkID,
-                                    spork.nValue, from)};
-    auto keyIDSigner = *opt_keyIDSigner;
+bool CSporkManager::ProcessSpork(const CSporkMessage& spork)
+{
+    uint256 hash = spork.GetHash();
+    std::string strLogMsg{strprintf("SPORK -- hash: %s id: %d value: %10d", hash.ToString(), spork.nSporkID,
+                                    spork.nValue)};
+    assert(spork.GetSignerKeyID().has_value());
+    auto keyIDSigner = spork.GetSignerKeyID().value();
 
     {
         LOCK(cs); // make sure to not lock this together with cs_main
         if (mapSporksActive.count(spork.nSporkID)) {
             if (mapSporksActive[spork.nSporkID].count(keyIDSigner)) {
                 if (mapSporksActive[spork.nSporkID][keyIDSigner].nTimeSigned >= spork.nTimeSigned) {
                     LogPrint(BCLog::SPORK, "%s seen\n", strLogMsg);
-                    return ret;
+                    return false;
                 } else {
                     LogPrintf("%s updated\n", strLogMsg);
                 }
@@ -188,8 +174,7 @@ MessageProcessingResult CSporkManager::ProcessSpork(NodeId from, CDataStream& vR
         }
     }
 
-    ret.m_inventory.emplace_back(MSG_SPORK, hash);
-    return ret;
+    return true;
 }
 
 std::unordered_map<SporkId, std::map<CKeyID, CSporkMessage>> CSporkManager::ActiveSporks() const

src/spork.h

@@ -5,28 +5,23 @@
 #ifndef BITCOIN_SPORK_H
 #define BITCOIN_SPORK_H
 
-#include <msg_result.h>
-
 #include <hash.h>
 #include <key.h>
-#include <net.h>
-#include <net_types.h>
 #include <pubkey.h>
 #include <saltedhasher.h>
 #include <sync.h>
-#include <uint256.h>
 
 #include <array>
 #include <optional>
 #include <string_view>
 #include <unordered_map>
 #include <vector>
 
-class CConnman;
 template<typename T>
 class CFlatDB;
-class CNode;
 class CDataStream;
+class uint256;
+class CInv;
 
 class CSporkMessage;
 class CSporkManager;
@@ -249,18 +244,18 @@ class CSporkManager : public SporkStore
     void CheckAndRemove() EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
     /**
-     * ProcessMessage is used to call ProcessSpork and ProcessGetSporks. See below
+     * IsSporkValid validate signed time and pubkey
+     * If these values mismatch function returns false [spork is invalid]
+     * If spork validation failed, peer should be punished
      */
-    [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type,
-                                                         CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache);
-
+    [[nodiscard]] bool IsSporkValid(const CSporkMessage& spork) EXCLUSIVE_LOCKS_REQUIRED(!cs);
     /**
      * ProcessSpork is used to handle the 'spork' p2p message.
      *
      * For 'spork', it validates the spork and adds it to the internal spork storage and
      * performs any necessary processing.
      */
-    [[nodiscard]] MessageProcessingResult ProcessSpork(NodeId from, CDataStream& vRecv)
+    [[nodiscard]] bool ProcessSpork(const CSporkMessage& spork)
         EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_cache);
 
     /**

test/lint/lint-circular-dependencies.py

@@ -23,7 +23,6 @@
     # Dash
     "active/context -> active/dkgsessionhandler -> llmq/dkgsessionhandler -> net_processing -> active/context",
     "banman -> common/bloom -> evo/assetlocktx -> llmq/quorumsman -> llmq/blockprocessor -> net -> banman",
-    "chainlock/chainlock -> spork -> net -> evo/deterministicmns -> evo/providertx -> validation -> chainlock/chainlock",
     "coinjoin/client -> coinjoin/util -> wallet/wallet -> psbt -> node/transaction -> net_processing -> coinjoin/walletman -> coinjoin/client",
     "common/bloom -> evo/assetlocktx -> llmq/commitment -> evo/deterministicmns -> evo/simplifiedmns -> merkleblock -> common/bloom",
     "common/bloom -> evo/assetlocktx -> llmq/quorumsman -> llmq/blockprocessor -> net -> common/bloom",