Merge bitcoin/bitcoin#25591: move-only: Version handshake to libtest_util

fa4be8e7c3 move-only: InitializeNode to handshake helper (MacroFake)
fa7098947c move-only: Version handshake to libtest_util (MacroFake)

Pull request description:

  The version handshake after setting up a peer is an integral part of (unit) testing net processing logic.

  Thus, make the helper accessible in libtest_util.

  Also, remove the peerman argument from `FillNode`, as it must be equal to connman's peerman, which can then be used instead.

ACKs for top commit:
  dergoegge:
    ACK fa4be8e7c3

Tree-SHA512: 8296399dc2c29196bd56584c9b61f1c5a088f96dd3438b07b84e1acf525d867f1e37fdfdeede8a831add25848cda0c221ce3fb873e5ae5ca805a1765aa08eb12

src/test/fuzz/process_message.cpp

@@ -80,8 +80,7 @@ void fuzz_target(FuzzBufferType buffer, const std::string& LIMIT_TO_MESSAGE_TYPE
     CNode& p2p_node = *ConsumeNodeAsUniquePtr(fuzzed_data_provider).release();
 
     connman.AddTestNode(p2p_node);
-    g_setup->m_node.peerman->InitializeNode(&p2p_node);
+    FillNode(fuzzed_data_provider, connman, p2p_node);
-    FillNode(fuzzed_data_provider, connman, *g_setup->m_node.peerman, p2p_node);
 
     const auto mock_time = ConsumeTime(fuzzed_data_provider);
     SetMockTime(mock_time);

src/test/fuzz/process_messages.cpp

@@ -46,8 +46,7 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages)
         peers.push_back(ConsumeNodeAsUniquePtr(fuzzed_data_provider, i).release());
         CNode& p2p_node = *peers.back();
 
-        g_setup->m_node.peerman->InitializeNode(&p2p_node);
+        FillNode(fuzzed_data_provider, connman, p2p_node);
-        FillNode(fuzzed_data_provider, connman, *g_setup->m_node.peerman, p2p_node);
 
         connman.AddTestNode(p2p_node);
     }

src/test/fuzz/util.cpp

@@ -289,57 +289,14 @@ bool FuzzedSock::IsConnected(std::string& errmsg) const
     return false;
 }
 
-void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, PeerManager& peerman, CNode& node) noexcept
+void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, CNode& node) noexcept
 {
-    const bool successfully_connected{fuzzed_data_provider.ConsumeBool()};
+    connman.Handshake(node,
-    const ServiceFlags remote_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS);
+                      /*successfully_connected=*/fuzzed_data_provider.ConsumeBool(),
-    const NetPermissionFlags permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS);
+                      /*remote_services=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS),
-    const int32_t version = fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max());
+                      /*permission_flags=*/ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS),
-    const bool relay_txs{fuzzed_data_provider.ConsumeBool()};
+                      /*version=*/fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max()),
-
+                      /*relay_txs=*/fuzzed_data_provider.ConsumeBool());
-    const CNetMsgMaker mm{0};
-
-    CSerializedNetMsg msg_version{
-        mm.Make(NetMsgType::VERSION,
-                version,                                        //
-                Using<CustomUintFormatter<8>>(remote_services), //
-                int64_t{},                                      // dummy time
-                int64_t{},                                      // ignored service bits
-                CService{},                                     // dummy
-                int64_t{},                                      // ignored service bits
-                CService{},                                     // ignored
-                uint64_t{1},                                    // dummy nonce
-                std::string{},                                  // dummy subver
-                int32_t{},                                      // dummy starting_height
-                relay_txs),
-    };
-
-    (void)connman.ReceiveMsgFrom(node, msg_version);
-    node.fPauseSend = false;
-    connman.ProcessMessagesOnce(node);
-    {
-        LOCK(node.cs_sendProcessing);
-        peerman.SendMessages(&node);
-    }
-    if (node.fDisconnect) return;
-    assert(node.nVersion == version);
-    assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION));
-    assert(node.nServices == remote_services);
-    CNodeStateStats statestats;
-    assert(peerman.GetNodeStateStats(node.GetId(), statestats));
-    assert(statestats.m_relay_txs == (relay_txs && !node.IsBlockOnlyConn()));
-    node.m_permissionFlags = permission_flags;
-    if (successfully_connected) {
-        CSerializedNetMsg msg_verack{mm.Make(NetMsgType::VERACK)};
-        (void)connman.ReceiveMsgFrom(node, msg_verack);
-        node.fPauseSend = false;
-        connman.ProcessMessagesOnce(node);
-        {
-            LOCK(node.cs_sendProcessing);
-            peerman.SendMessages(&node);
-        }
-        assert(node.fSuccessfullyConnected == true);
-    }
 }
 
 CAmount ConsumeMoney(FuzzedDataProvider& fuzzed_data_provider, const std::optional<CAmount>& max) noexcept

src/test/fuzz/util.h

@@ -331,7 +331,7 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N
 }
 inline std::unique_ptr<CNode> ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional<NodeId>& node_id_in = std::nullopt) { return ConsumeNode<true>(fdp, node_id_in); }
 
-void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, PeerManager& peerman, CNode& node) noexcept;
+void FillNode(FuzzedDataProvider& fuzzed_data_provider, ConnmanTestMsg& connman, CNode& node) noexcept;
 
 class FuzzedFileProvider
 {

src/test/util/net.cpp

@@ -7,10 +7,68 @@
 #include <chainparams.h>
 #include <node/eviction.h>
 #include <net.h>
+#include <net_processing.h>
+#include <netmessagemaker.h>
 #include <span.h>
 
 #include <vector>
 
+void ConnmanTestMsg::Handshake(CNode& node,
+                               bool successfully_connected,
+                               ServiceFlags remote_services,
+                               NetPermissionFlags permission_flags,
+                               int32_t version,
+                               bool relay_txs)
+{
+    auto& peerman{static_cast<PeerManager&>(*m_msgproc)};
+    auto& connman{*this};
+    const CNetMsgMaker mm{0};
+
+    peerman.InitializeNode(&node);
+
+    CSerializedNetMsg msg_version{
+        mm.Make(NetMsgType::VERSION,
+                version,                                        //
+                Using<CustomUintFormatter<8>>(remote_services), //
+                int64_t{},                                      // dummy time
+                int64_t{},                                      // ignored service bits
+                CService{},                                     // dummy
+                int64_t{},                                      // ignored service bits
+                CService{},                                     // ignored
+                uint64_t{1},                                    // dummy nonce
+                std::string{},                                  // dummy subver
+                int32_t{},                                      // dummy starting_height
+                relay_txs),
+    };
+
+    (void)connman.ReceiveMsgFrom(node, msg_version);
+    node.fPauseSend = false;
+    connman.ProcessMessagesOnce(node);
+    {
+        LOCK(node.cs_sendProcessing);
+        peerman.SendMessages(&node);
+    }
+    if (node.fDisconnect) return;
+    assert(node.nVersion == version);
+    assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION));
+    assert(node.nServices == remote_services);
+    CNodeStateStats statestats;
+    assert(peerman.GetNodeStateStats(node.GetId(), statestats));
+    assert(statestats.m_relay_txs == (relay_txs && !node.IsBlockOnlyConn()));
+    node.m_permissionFlags = permission_flags;
+    if (successfully_connected) {
+        CSerializedNetMsg msg_verack{mm.Make(NetMsgType::VERACK)};
+        (void)connman.ReceiveMsgFrom(node, msg_verack);
+        node.fPauseSend = false;
+        connman.ProcessMessagesOnce(node);
+        {
+            LOCK(node.cs_sendProcessing);
+            peerman.SendMessages(&node);
+        }
+        assert(node.fSuccessfullyConnected == true);
+    }
+}
+
 void ConnmanTestMsg::NodeReceiveMsgBytes(CNode& node, Span<const uint8_t> msg_bytes, bool& complete) const
 {
     assert(node.ReceiveMsgBytes(msg_bytes, complete));

src/test/util/net.h

@@ -39,6 +39,13 @@ struct ConnmanTestMsg : public CConnman {
         m_nodes.clear();
     }
 
+    void Handshake(CNode& node,
+                   bool successfully_connected,
+                   ServiceFlags remote_services,
+                   NetPermissionFlags permission_flags,
+                   int32_t version,
+                   bool relay_txs);
+
     void ProcessMessagesOnce(CNode& node) { m_msgproc->ProcessMessages(&node, flagInterruptMsgProc); }
 
     void NodeReceiveMsgBytes(CNode& node, Span<const uint8_t> msg_bytes, bool& complete) const;