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

fa79a88 refactor: P2P transport without serialize version and type (MarcoFalke)
fa9b5f4 refactor: NetMsg::Make() without nVersion (MarcoFalke)
66669da Remove unused Make() overload in netmessagemaker.h (MarcoFalke)
fa0ed07 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 fa79a88

Tree-SHA512: 785b413580d980f51f0d4f70ea5e0a99ce14cd12cb065393de2f5254891be94a14f4266110c8b87bd2dbc37467676655bce13bdb295ab139749fcd8b61bd5110

Author: fanquake

src/blockfilter.cpp

@@ -81,15 +81,15 @@ GCSFilter::GCSFilter(const Params& params, const ElementSet& elements)
     }
     m_F = static_cast<uint64_t>(m_N) * static_cast<uint64_t>(m_params.m_M);
 
-    CVectorWriter stream(GCS_SER_VERSION, m_encoded, 0);
+    VectorWriter stream{m_encoded, 0};
 
     WriteCompactSize(stream, m_N);
 
     if (elements.empty()) {
         return;
     }
 
-    BitStreamWriter<CVectorWriter> bitwriter(stream);
+    BitStreamWriter bitwriter{stream};
 
     uint64_t last_value = 0;
     for (uint64_t value : BuildHashedSet(elements)) {

src/net.cpp

@@ -683,8 +683,8 @@ bool CNode::ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete)
     return true;
 }
 
-V1Transport::V1Transport(const NodeId node_id, int nTypeIn, int nVersionIn) noexcept :
-    m_magic_bytes{Params().MessageStart()}, m_node_id(node_id), hdrbuf(nTypeIn, nVersionIn), vRecv(nTypeIn, nVersionIn)
+V1Transport::V1Transport(const NodeId node_id) noexcept
+    : m_magic_bytes{Params().MessageStart()}, m_node_id{node_id}
 {
     LOCK(m_recv_mutex);
     Reset();
@@ -818,7 +818,7 @@ bool V1Transport::SetMessageToSend(CSerializedNetMsg& msg) noexcept
 
     // serialize header
     m_header_to_send.clear();
-    CVectorWriter{INIT_PROTO_VERSION, m_header_to_send, 0, hdr};
+    VectorWriter{m_header_to_send, 0, hdr};
 
     // update state
     m_message_to_send = std::move(msg);
@@ -968,12 +968,12 @@ void V2Transport::StartSendingHandshake() noexcept
     // We cannot wipe m_send_garbage as it will still be used as AAD later in the handshake.
 }
 
-V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span<const std::byte> ent32, std::vector<uint8_t> garbage) noexcept :
-    m_cipher{key, ent32}, m_initiating{initiating}, m_nodeid{nodeid},
-    m_v1_fallback{nodeid, type_in, version_in}, m_recv_type{type_in}, m_recv_version{version_in},
-    m_recv_state{initiating ? RecvState::KEY : RecvState::KEY_MAYBE_V1},
-    m_send_garbage{std::move(garbage)},
-    m_send_state{initiating ? SendState::AWAITING_KEY : SendState::MAYBE_V1}
+V2Transport::V2Transport(NodeId nodeid, bool initiating, const CKey& key, Span<const std::byte> ent32, std::vector<uint8_t> garbage) noexcept
+    : m_cipher{key, ent32}, m_initiating{initiating}, m_nodeid{nodeid},
+      m_v1_fallback{nodeid},
+      m_recv_state{initiating ? RecvState::KEY : RecvState::KEY_MAYBE_V1},
+      m_send_garbage{std::move(garbage)},
+      m_send_state{initiating ? SendState::AWAITING_KEY : SendState::MAYBE_V1}
 {
     Assume(m_send_garbage.size() <= MAX_GARBAGE_LEN);
     // Start sending immediately if we're the initiator of the connection.
@@ -983,9 +983,9 @@ V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int versio
     }
 }
 
-V2Transport::V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in) noexcept :
-    V2Transport{nodeid, initiating, type_in, version_in, GenerateRandomKey(),
-                MakeByteSpan(GetRandHash()), GenerateRandomGarbage()} { }
+V2Transport::V2Transport(NodeId nodeid, bool initiating) noexcept
+    : V2Transport{nodeid, initiating, GenerateRandomKey(),
+                  MakeByteSpan(GetRandHash()), GenerateRandomGarbage()} {}
 
 void V2Transport::SetReceiveState(RecvState recv_state) noexcept
 {
@@ -1429,8 +1429,7 @@ CNetMessage V2Transport::GetReceivedMessage(std::chrono::microseconds time, bool
     Assume(m_recv_state == RecvState::APP_READY);
     Span<const uint8_t> contents{m_recv_decode_buffer};
     auto msg_type = GetMessageType(contents);
-    CDataStream ret(m_recv_type, m_recv_version);
-    CNetMessage msg{std::move(ret)};
+    CNetMessage msg{DataStream{}};
     // Note that BIP324Cipher::EXPANSION also includes the length descriptor size.
     msg.m_raw_message_size = m_recv_decode_buffer.size() + BIP324Cipher::EXPANSION;
     if (msg_type) {
@@ -3660,9 +3659,9 @@ ServiceFlags CConnman::GetLocalServices() const
 static std::unique_ptr<Transport> MakeTransport(NodeId id, bool use_v2transport, bool inbound) noexcept
 {
     if (use_v2transport) {
-        return std::make_unique<V2Transport>(id, /*initiating=*/!inbound, SER_NETWORK, INIT_PROTO_VERSION);
+        return std::make_unique<V2Transport>(id, /*initiating=*/!inbound);
     } else {
-        return std::make_unique<V1Transport>(id, SER_NETWORK, INIT_PROTO_VERSION);
+        return std::make_unique<V1Transport>(id);
     }
 }
 

src/net.h

@@ -232,15 +232,16 @@ class CNodeStats
  * Ideally it should only contain receive time, payload,
  * type and size.
  */
-class CNetMessage {
+class CNetMessage
+{
 public:
-    CDataStream m_recv;                  //!< received message data
+    DataStream m_recv;                   //!< received message data
     std::chrono::microseconds m_time{0}; //!< time of message receipt
     uint32_t m_message_size{0};          //!< size of the payload
     uint32_t m_raw_message_size{0};      //!< used wire size of the message (including header/checksum)
     std::string m_type;
 
-    CNetMessage(CDataStream&& recv_in) : m_recv(std::move(recv_in)) {}
+    explicit CNetMessage(DataStream&& recv_in) : m_recv(std::move(recv_in)) {}
     // Only one CNetMessage object will exist for the same message on either
     // the receive or processing queue. For performance reasons we therefore
     // delete the copy constructor and assignment operator to avoid the
@@ -249,11 +250,6 @@ class CNetMessage {
     CNetMessage(const CNetMessage&) = delete;
     CNetMessage& operator=(CNetMessage&&) = default;
     CNetMessage& operator=(const CNetMessage&) = delete;
-
-    void SetVersion(int nVersionIn)
-    {
-        m_recv.SetVersion(nVersionIn);
-    }
 };
 
 /** The Transport converts one connection's sent messages to wire bytes, and received bytes back. */
@@ -379,9 +375,9 @@ class V1Transport final : public Transport
     mutable CHash256 hasher GUARDED_BY(m_recv_mutex);
     mutable uint256 data_hash GUARDED_BY(m_recv_mutex);
     bool in_data GUARDED_BY(m_recv_mutex); // parsing header (false) or data (true)
-    CDataStream hdrbuf GUARDED_BY(m_recv_mutex); // partially received header
+    DataStream hdrbuf GUARDED_BY(m_recv_mutex){}; // partially received header
     CMessageHeader hdr GUARDED_BY(m_recv_mutex); // complete header
-    CDataStream vRecv GUARDED_BY(m_recv_mutex); // received message data
+    DataStream vRecv GUARDED_BY(m_recv_mutex){}; // received message data
     unsigned int nHdrPos GUARDED_BY(m_recv_mutex);
     unsigned int nDataPos GUARDED_BY(m_recv_mutex);
 
@@ -420,7 +416,7 @@ class V1Transport final : public Transport
     size_t m_bytes_sent GUARDED_BY(m_send_mutex) {0};
 
 public:
-    V1Transport(const NodeId node_id, int nTypeIn, int nVersionIn) noexcept;
+    explicit V1Transport(const NodeId node_id) noexcept;
 
     bool ReceivedMessageComplete() const override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex)
     {
@@ -598,10 +594,6 @@ class V2Transport final : public Transport
     std::vector<uint8_t> m_recv_aad GUARDED_BY(m_recv_mutex);
     /** Buffer to put decrypted contents in, for converting to CNetMessage. */
     std::vector<uint8_t> m_recv_decode_buffer GUARDED_BY(m_recv_mutex);
-    /** Deserialization type. */
-    const int m_recv_type;
-    /** Deserialization version number. */
-    const int m_recv_version;
     /** Current receiver state. */
     RecvState m_recv_state GUARDED_BY(m_recv_mutex);
 
@@ -647,13 +639,11 @@ class V2Transport final : public Transport
      *
      * @param[in] nodeid      the node's NodeId (only for debug log output).
      * @param[in] initiating  whether we are the initiator side.
-     * @param[in] type_in     the serialization type of returned CNetMessages.
-     * @param[in] version_in  the serialization version of returned CNetMessages.
      */
-    V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in) noexcept;
+    V2Transport(NodeId nodeid, bool initiating) noexcept;
 
     /** Construct a V2 transport with specified keys and garbage (test use only). */
-    V2Transport(NodeId nodeid, bool initiating, int type_in, int version_in, const CKey& key, Span<const std::byte> ent32, std::vector<uint8_t> garbage) noexcept;
+    V2Transport(NodeId nodeid, bool initiating, const CKey& key, Span<const std::byte> ent32, std::vector<uint8_t> garbage) noexcept;
 
     // Receive side functions.
     bool ReceivedMessageComplete() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex);