Merge #19791: [net processing] Move Misbehaving() to PeerManager

bb6a32ce9983c72afa90f41a43a47ffd703ca006 [net processing] Move Misbehaving() to PeerManager (John Newbery)
aa114b1c9b06c2bd3ed936bbb9fb32b31f75bdb2 [net_processing] Move SendBlockTransactions into PeerManager (John Newbery)
3115e00f75b41d9765dcbb376e367b25f61a1d58 [net processing] Move MaybePunishPeerForTx to PeerManager (John Newbery)
e662e2d42afaf9c67c898634a0f3bc200255b6ea [net processing] Move ProcessOrphanTx to PeerManager (John Newbery)
b70cd890e375e904b7f36b3d959e5656f5a5cbcd [net processing] Move MaybePunishNodeForBlock into PeerManager (John Newbery)
d7778351bf60a21925a97b7fc4e9df5541b6d995 [net processing] Move ProcessHeadersMessage to PeerManager (John Newbery)
64f6162651420be2f4aa1498f0378a86780bc089 [whitespace] tidy up indentation after scripted diff (John Newbery)
58bd369b0ddd3383f7bdf7840912d18b96545f91 scripted-diff: [net processing] Rename PeerLogicValidation to PeerManager (John Newbery)
2297b26b3ce95e935c0ebb8c38dabf19965054a5 [net_processing] Pass chainparams to PeerLogicValidation constructor (John Newbery)
824bbd1ffba3df7ffa6f5bfaa31298cd484473b1 [move only] Collect all private members of PeerLogicValidation together (John Newbery)

Pull request description:

  Continues the work of moving net_processing logic into PeerLogicValidation. See https://github.com/bitcoin/bitcoin/pull/19704 and https://github.com/bitcoin/bitcoin/pull/19607#discussion_r462032894 for motivation.

  This PR also renames `PeerLogicValidation` to `PeerManager` as suggested in https://github.com/bitcoin/bitcoin/pull/10756#pullrequestreview-53892618.

ACKs for top commit:
  MarcoFalke:
    re-ACK bb6a32ce99 only change is rebase due to conflict in struct NodeContext and variable rename 🤸
  hebasto:
    re-ACK bb6a32ce9983c72afa90f41a43a47ffd703ca006, only rebased, and added renaming `s/peer_logic/peerman/` into scripted-diff since my [previous](https://github.com/bitcoin/bitcoin/pull/19791#pullrequestreview-483118079) review (verified with `git range-diff`).

Tree-SHA512: a2de4a521688fd25125b401e5575402c52b328a0fa27b3010567008d4f596b960aabbd02b2d81f42658f88f4365443fadb1008150a62fbcea123fb42d85a2c21

1 files changed, 56 insertions, 14 deletions

diff --git a/src/net_processing.h b/src/net_processing.h
index 4f1c52fc17..520f7489e8 100644
--- a/src/net_processing.h
+++ b/src/net_processing.h
@@ -11,9 +11,13 @@
 #include <sync.h>
 #include <validationinterface.h>
 
+class BlockTransactionsRequest;
+class BlockValidationState;
+class CBlockHeader;
 class CChainParams;
 class CTxMemPool;
 class ChainstateManager;
+class TxValidationState;
 
 extern RecursiveMutex cs_main;
 extern RecursiveMutex g_cs_orphans;
@@ -27,18 +31,10 @@ static const bool DEFAULT_PEERBLOCKFILTERS = false;
 /** Threshold for marking a node to be discouraged, e.g. disconnected and added to the discouragement filter. */
 static const int DISCOURAGEMENT_THRESHOLD{100};
 
-class PeerLogicValidation final : public CValidationInterface, public NetEventsInterface {
-private:
-    CConnman& m_connman;
-    /** Pointer to this node's banman. May be nullptr - check existence before dereferencing. */
-    BanMan* const m_banman;
-    ChainstateManager& m_chainman;
-    CTxMemPool& m_mempool;
-
-    bool MaybeDiscourageAndDisconnect(CNode& pnode);
-
+class PeerManager final : public CValidationInterface, public NetEventsInterface {
 public:
-    PeerLogicValidation(CConnman& connman, BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, CTxMemPool& pool);
+    PeerManager(const CChainParams& chainparams, CConnman& connman, BanMan* banman,
+                CScheduler& scheduler, ChainstateManager& chainman, CTxMemPool& pool);
 
     /**
      * Overridden from CValidationInterface.
@@ -88,12 +84,58 @@ public:
 
     /** Process a single message from a peer. Public for fuzz testing */
     void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv,
-                        const std::chrono::microseconds time_received, const CChainParams& chainparams,
-                        const std::atomic<bool>& interruptMsgProc);
+                        const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc);
+
+    /**
+     * Increment peer's misbehavior score. If the new value >= DISCOURAGEMENT_THRESHOLD, mark the node
+     * to be discouraged, meaning the peer might be disconnected and added to the discouragement filter.
+     * Public for unit testing.
+     */
+    void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message);
 
 private:
-    int64_t m_stale_tip_check_time; //!< Next time to check for stale tip
+    /**
+     * Potentially mark a node discouraged based on the contents of a BlockValidationState object
+     *
+     * @param[in] via_compact_block this bool is passed in because net_processing should
+     * punish peers differently depending on whether the data was provided in a compact
+     * block message or not. If the compact block had a valid header, but contained invalid
+     * txs, the peer should not be punished. See BIP 152.
+     *
+     * @return Returns true if the peer was punished (probably disconnected)
+     */
+    bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& state,
+                                 bool via_compact_block, const std::string& message = "");
 
+    /**
+     * Potentially disconnect and discourage a node based on the contents of a TxValidationState object
+     *
+     * @return Returns true if the peer was punished (probably disconnected)
+     */
+    bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, const std::string& message = "");
+
+    /** Maybe disconnect a peer and discourage future connections from its address.
+     *
+     * @param[in]   pnode     The node to check.
+     * @return                True if the peer was marked for disconnection in this function
+     */
+    bool MaybeDiscourageAndDisconnect(CNode& pnode);
+
+    void ProcessOrphanTx(std::set<uint256>& orphan_work_set, std::list<CTransactionRef>& removed_txn)
+        EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_cs_orphans);
+    /** Process a single headers message from a peer. */
+    void ProcessHeadersMessage(CNode& pfrom, const std::vector<CBlockHeader>& headers, bool via_compact_block);
+
+    void SendBlockTransactions(CNode& pfrom, const CBlock& block, const BlockTransactionsRequest& req);
+
+    const CChainParams& m_chainparams;
+    CConnman& m_connman;
+    /** Pointer to this node's banman. May be nullptr - check existence before dereferencing. */
+    BanMan* const m_banman;
+    ChainstateManager& m_chainman;
+    CTxMemPool& m_mempool;
+
+    int64_t m_stale_tip_check_time; //!< Next time to check for stale tip
 };
 
 struct CNodeStateStats {