refactor: new type of message processing result for resolving circular dependency over PeerManager

Author: knst

src/coinjoin/client.h

@@ -8,7 +8,7 @@
 #include <coinjoin/util.h>
 #include <coinjoin/coinjoin.h>
 
-#include <net_types.h>
+#include <protocol.h>
 #include <util/ranges.h>
 #include <util/translation.h>
 

src/coinjoin/server.h

@@ -7,7 +7,7 @@
 
 #include <coinjoin/coinjoin.h>
 
-#include <net_types.h>
+#include <protocol.h>
 
 class CActiveMasternodeManager;
 class CChainState;

src/evo/mnauth.h

@@ -6,7 +6,7 @@
 #define BITCOIN_EVO_MNAUTH_H
 
 #include <bls/bls.h>
-#include <net_types.h>
+#include <protocol.h>
 #include <serialize.h>
 
 class CActiveMasternodeManager;

src/governance/governance.h

@@ -10,7 +10,7 @@
 
 #include <cachemap.h>
 #include <cachemultimap.h>
-#include <net_types.h>
+#include <protocol.h>
 #include <util/check.h>
 
 #include <optional>

src/llmq/blockprocessor.h

@@ -9,8 +9,8 @@
 
 #include <chain.h>
 #include <consensus/params.h>
-#include <net_types.h>
 #include <primitives/block.h>
+#include <protocol.h>
 #include <saltedhasher.h>
 #include <sync.h>
 

src/llmq/quorums.h

@@ -13,8 +13,8 @@
 
 #include <bls/bls.h>
 #include <bls/bls_worker.h>
+#include <protocol.h>
 
-#include <net_types.h>
 #include <gsl/pointers.h>
 
 #include <atomic>

src/llmq/signing.h

@@ -8,7 +8,7 @@
 #include <bls/bls.h>
 #include <consensus/params.h>
 #include <gsl/pointers.h>
-#include <net_types.h>
+#include <protocol.h>
 #include <random.h>
 #include <saltedhasher.h>
 #include <sync.h>

src/net_processing.cpp

@@ -584,8 +584,9 @@ class PeerManagerImpl final : public PeerManager
     bool IsInvInFilter(NodeId nodeid, const uint256& hash) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
 
 private:
-    /** Helper to process result of external handlers of message */
+    /** Helpers to process result of external handlers of message */
     void ProcessPeerMsgRet(const PeerMsgRet& ret, CNode& pfrom) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
+    void PostProcessMessage(MessageProcessingResult&& ret, NodeId node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
 
     /** Consider evicting an outbound peer based on the amount of time they've been behind our tip */
     void ConsiderEviction(CNode& pto, Peer& peer, std::chrono::seconds time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_msgproc_mutex);
@@ -3309,6 +3310,16 @@ void PeerManagerImpl::ProcessPeerMsgRet(const PeerMsgRet& ret, CNode& pfrom)
     if (!ret) Misbehaving(pfrom.GetId(), ret.error().score, ret.error().message);
 }
 
+void PeerManagerImpl::PostProcessMessage(MessageProcessingResult&& ret, NodeId node)
+{
+    if (ret.m_error) {
+        Misbehaving(node, ret.m_error->score, ret.m_error->message);
+    }
+    if (ret.m_inventory) {
+        RelayInv(ret.m_inventory.value(), MIN_PEER_PROTO_VERSION);
+    }
+}
+
 void PeerManagerImpl::ProcessMessage(
     CNode& pfrom,
     const std::string& msg_type,

src/net_processing.h

@@ -127,6 +127,9 @@ class PeerManager : public CValidationInterface, public NetEventsInterface
     virtual void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv,
                                 const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0;
 
+    /** Finish message processing. Used for some specific messages */
+    virtual void PostProcessMessage(MessageProcessingResult&& ret, NodeId node = -1) = 0;
+
     /** This function is used for testing the stale tip eviction logic, see denialofservice_tests.cpp */
     virtual void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) = 0;
 

src/net_types.h

@@ -5,8 +5,6 @@
 #ifndef BITCOIN_NET_TYPES_H
 #define BITCOIN_NET_TYPES_H
 
-#include <util/expected.h>
-
 #include <cstdint>
 #include <map>
 #include <string>
@@ -60,21 +58,5 @@ UniValue BanMapToJson(const banmap_t& bans);
  */
 void BanMapFromJson(const UniValue& bans_json, banmap_t& bans);
 
-struct MisbehavingError
-{
-    int score;
-    std::string message;
-
-    MisbehavingError(int s) : score{s} {}
-
-     // Constructor does a perfect forwarding reference
-    template <typename T>
-    MisbehavingError(int s, T&& msg) :
-        score{s},
-        message{std::forward<T>(msg)}
-    {}
-};
-
-using PeerMsgRet = tl::expected<void, MisbehavingError>;
 
 #endif // BITCOIN_NET_TYPES_H