Merge bitcoin/bitcoin#28892: refactor: P2P transport without serialize version and type

fa79a881ce0537e1d74da296a7513730438d2a02 refactor: P2P transport without serialize version and type (MarcoFalke)
fa9b5f4fe32c0cfe2e477bb11912756f84a52cfe refactor: NetMsg::Make() without nVersion (MarcoFalke)
66669da4a5ca9edf2a40d20879d9a8aaf2b9e2ee Remove unused Make() overload in netmessagemaker.h (MarcoFalke)
fa0ed0794161d937d2d3385963c1aa5624b60d17 refactor: VectorWriter without nVersion (MarcoFalke)

Pull request description:

  Now that the serialize framework ignores the serialize version and serialize type, everything related to it can be removed from the code.

  This is the first step, removing dead code from the P2P stack. A different pull will remove it from the wallet and other parts.

ACKs for top commit:
  ajtowns:
    reACK fa79a881ce0537e1d74da296a7513730438d2a02

Tree-SHA512: 785b413580d980f51f0d4f70ea5e0a99ce14cd12cb065393de2f5254891be94a14f4266110c8b87bd2dbc37467676655bce13bdb295ab139749fcd8b61bd5110

5 files changed, 29 insertions, 31 deletions

diff --git a/src/test/fuzz/golomb_rice.cpp b/src/test/fuzz/golomb_rice.cpp
index f3073c5c97..8b395f3ea9 100644
--- a/src/test/fuzz/golomb_rice.cpp
+++ b/src/test/fuzz/golomb_rice.cpp
@@ -51,9 +51,9 @@ FUZZ_TARGET(golomb_rice)
         for (int i = 0; i < n; ++i) {
             elements.insert(ConsumeRandomLengthByteVector(fuzzed_data_provider, 16));
         }
-        CVectorWriter stream{0, golomb_rice_data, 0};
+        VectorWriter stream{golomb_rice_data, 0};
         WriteCompactSize(stream, static_cast<uint32_t>(elements.size()));
-        BitStreamWriter<CVectorWriter> bitwriter(stream);
+        BitStreamWriter bitwriter{stream};
         if (!elements.empty()) {
             uint64_t last_value = 0;
             for (const uint64_t value : BuildHashedSet(elements, static_cast<uint64_t>(elements.size()) * static_cast<uint64_t>(BASIC_FILTER_M))) {
diff --git a/src/test/fuzz/p2p_transport_serialization.cpp b/src/test/fuzz/p2p_transport_serialization.cpp
index 21d8dab536..a205ce19f4 100644
--- a/src/test/fuzz/p2p_transport_serialization.cpp
+++ b/src/test/fuzz/p2p_transport_serialization.cpp
@@ -36,8 +36,8 @@ void initialize_p2p_transport_serialization()
 FUZZ_TARGET(p2p_transport_serialization, .init = initialize_p2p_transport_serialization)
 {
     // Construct transports for both sides, with dummy NodeIds.
-    V1Transport recv_transport{NodeId{0}, SER_NETWORK, INIT_PROTO_VERSION};
-    V1Transport send_transport{NodeId{1}, SER_NETWORK, INIT_PROTO_VERSION};
+    V1Transport recv_transport{NodeId{0}};
+    V1Transport send_transport{NodeId{1}};
 
     FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
 
@@ -88,7 +88,7 @@ FUZZ_TARGET(p2p_transport_serialization, .init = initialize_p2p_transport_serial
             assert(msg.m_time == m_time);
 
             std::vector<unsigned char> header;
-            auto msg2 = CNetMsgMaker{msg.m_recv.GetVersion()}.Make(msg.m_type, Span{msg.m_recv});
+            auto msg2 = NetMsg::Make(msg.m_type, Span{msg.m_recv});
             bool queued = send_transport.SetMessageToSend(msg2);
             assert(queued);
             std::optional<bool> known_more;
@@ -335,7 +335,7 @@ void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDa
 
 std::unique_ptr<Transport> MakeV1Transport(NodeId nodeid) noexcept
 {
-    return std::make_unique<V1Transport>(nodeid, SER_NETWORK, INIT_PROTO_VERSION);
+    return std::make_unique<V1Transport>(nodeid);
 }
 
 template<typename RNG>
@@ -369,7 +369,7 @@ std::unique_ptr<Transport> MakeV2Transport(NodeId nodeid, bool initiator, RNG& r
              .Write(garb.data(), garb.size())
              .Finalize(UCharCast(ent.data()));
 
-    return std::make_unique<V2Transport>(nodeid, initiator, SER_NETWORK, INIT_PROTO_VERSION, key, ent, std::move(garb));
+    return std::make_unique<V2Transport>(nodeid, initiator, key, ent, std::move(garb));
 }
 
 } // namespace
diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
index 48e0706a53..508c42cabc 100644
--- a/src/test/net_tests.cpp
+++ b/src/test/net_tests.cpp
@@ -845,7 +845,6 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message)
 
     const uint64_t services{NODE_NETWORK | NODE_WITNESS};
     const int64_t time{0};
-    const CNetMsgMaker msg_maker{PROTOCOL_VERSION};
 
     // Force ChainstateManager::IsInitialBlockDownload() to return false.
     // Otherwise PushAddress() isn't called by PeerManager::ProcessMessage().
@@ -858,13 +857,13 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message)
     std::chrono::microseconds time_received_dummy{0};
 
     const auto msg_version =
-        msg_maker.Make(NetMsgType::VERSION, PROTOCOL_VERSION, services, time, services, CAddress::V1_NETWORK(peer_us));
+        NetMsg::Make(NetMsgType::VERSION, PROTOCOL_VERSION, services, time, services, CAddress::V1_NETWORK(peer_us));
     CDataStream msg_version_stream{msg_version.data, SER_NETWORK, PROTOCOL_VERSION};
 
     m_node.peerman->ProcessMessage(
         peer, NetMsgType::VERSION, msg_version_stream, time_received_dummy, interrupt_dummy);
 
-    const auto msg_verack = msg_maker.Make(NetMsgType::VERACK);
+    const auto msg_verack = NetMsg::Make(NetMsgType::VERACK);
     CDataStream msg_verack_stream{msg_verack.data, SER_NETWORK, PROTOCOL_VERSION};
 
     // Will set peer.fSuccessfullyConnected to true (necessary in SendMessages()).
@@ -1047,10 +1046,10 @@ class V2TransportTester
 
 public:
     /** Construct a tester object. test_initiator: whether the tested transport is initiator. */
-    V2TransportTester(bool test_initiator) :
-        m_transport(0, test_initiator, SER_NETWORK, INIT_PROTO_VERSION),
-        m_cipher{GenerateRandomTestKey(), MakeByteSpan(InsecureRand256())},
-        m_test_initiator(test_initiator) {}
+    explicit V2TransportTester(bool test_initiator)
+        : m_transport{0, test_initiator},
+          m_cipher{GenerateRandomTestKey(), MakeByteSpan(InsecureRand256())},
+          m_test_initiator(test_initiator) {}
 
     /** Data type returned by Interact:
      *
diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp
index d8478ecf5f..693247b8d4 100644
--- a/src/test/streams_tests.cpp
+++ b/src/test/streams_tests.cpp
@@ -74,49 +74,49 @@ BOOST_AUTO_TEST_CASE(streams_vector_writer)
     // point should yield the same results, even if the first test grew the
     // vector.
 
-    CVectorWriter{INIT_PROTO_VERSION, vch, 0, a, b};
+    VectorWriter{vch, 0, a, b};
     BOOST_CHECK((vch == std::vector<unsigned char>{{1, 2}}));
-    CVectorWriter{INIT_PROTO_VERSION, vch, 0, a, b};
+    VectorWriter{vch, 0, a, b};
     BOOST_CHECK((vch == std::vector<unsigned char>{{1, 2}}));
     vch.clear();
 
-    CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, b};
+    VectorWriter{vch, 2, a, b};
     BOOST_CHECK((vch == std::vector<unsigned char>{{0, 0, 1, 2}}));
-    CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, b};
+    VectorWriter{vch, 2, a, b};
     BOOST_CHECK((vch == std::vector<unsigned char>{{0, 0, 1, 2}}));
     vch.clear();
 
     vch.resize(5, 0);
-    CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, b};
+    VectorWriter{vch, 2, a, b};
     BOOST_CHECK((vch == std::vector<unsigned char>{{0, 0, 1, 2, 0}}));
-    CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, b};
+    VectorWriter{vch, 2, a, b};
     BOOST_CHECK((vch == std::vector<unsigned char>{{0, 0, 1, 2, 0}}));
     vch.clear();
 
     vch.resize(4, 0);
-    CVectorWriter{INIT_PROTO_VERSION, vch, 3, a, b};
+    VectorWriter{vch, 3, a, b};
     BOOST_CHECK((vch == std::vector<unsigned char>{{0, 0, 0, 1, 2}}));
-    CVectorWriter{INIT_PROTO_VERSION, vch, 3, a, b};
+    VectorWriter{vch, 3, a, b};
     BOOST_CHECK((vch == std::vector<unsigned char>{{0, 0, 0, 1, 2}}));
     vch.clear();
 
     vch.resize(4, 0);
-    CVectorWriter{INIT_PROTO_VERSION, vch, 4, a, b};
+    VectorWriter{vch, 4, a, b};
     BOOST_CHECK((vch == std::vector<unsigned char>{{0, 0, 0, 0, 1, 2}}));
-    CVectorWriter{INIT_PROTO_VERSION, vch, 4, a, b};
+    VectorWriter{vch, 4, a, b};
     BOOST_CHECK((vch == std::vector<unsigned char>{{0, 0, 0, 0, 1, 2}}));
     vch.clear();
 
-    CVectorWriter{INIT_PROTO_VERSION, vch, 0, bytes};
+    VectorWriter{vch, 0, bytes};
     BOOST_CHECK((vch == std::vector<unsigned char>{{3, 4, 5, 6}}));
-    CVectorWriter{INIT_PROTO_VERSION, vch, 0, bytes};
+    VectorWriter{vch, 0, bytes};
     BOOST_CHECK((vch == std::vector<unsigned char>{{3, 4, 5, 6}}));
     vch.clear();
 
     vch.resize(4, 8);
-    CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, bytes, b};
+    VectorWriter{vch, 2, a, bytes, b};
     BOOST_CHECK((vch == std::vector<unsigned char>{{8, 8, 1, 3, 4, 5, 6, 2}}));
-    CVectorWriter{INIT_PROTO_VERSION, vch, 2, a, bytes, b};
+    VectorWriter{vch, 2, a, bytes, b};
     BOOST_CHECK((vch == std::vector<unsigned char>{{8, 8, 1, 3, 4, 5, 6, 2}}));
     vch.clear();
 }
diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp
index e0404e33ed..9257a4964a 100644
--- a/src/test/util/net.cpp
+++ b/src/test/util/net.cpp
@@ -26,13 +26,12 @@ void ConnmanTestMsg::Handshake(CNode& node,
 {
     auto& peerman{static_cast<PeerManager&>(*m_msgproc)};
     auto& connman{*this};
-    const CNetMsgMaker mm{0};
 
     peerman.InitializeNode(node, local_services);
     FlushSendBuffer(node); // Drop the version message added by InitializeNode.
 
     CSerializedNetMsg msg_version{
-        mm.Make(NetMsgType::VERSION,
+        NetMsg::Make(NetMsgType::VERSION,
                 version,                                        //
                 Using<CustomUintFormatter<8>>(remote_services), //
                 int64_t{},                                      // dummy time
@@ -59,7 +58,7 @@ void ConnmanTestMsg::Handshake(CNode& node,
     assert(statestats.m_relay_txs == (relay_txs && !node.IsBlockOnlyConn()));
     assert(statestats.their_services == remote_services);
     if (successfully_connected) {
-        CSerializedNetMsg msg_verack{mm.Make(NetMsgType::VERACK)};
+        CSerializedNetMsg msg_verack{NetMsg::Make(NetMsgType::VERACK)};
         (void)connman.ReceiveMsgFrom(node, std::move(msg_verack));
         node.fPauseSend = false;
         connman.ProcessMessagesOnce(node);