Give V1TransportDeserializer an m_node_id member

troygiorshev · troygiorshev · commit 2716647ebf60 · 2020-09-22T22:01:14.000-04:00
This is intended to only be used for logging.

This will allow log messages in the following commits to keep recording
the peer's ID, even when logging is moved into V1TransportDeserializer.
diff --git a/src/net.cpp b/src/net.cpp
@@ -699,10 +699,11 @@ CNetMessage V1TransportDeserializer::GetMessage(const CMessageHeader::MessageSta
 
     msg.m_valid_checksum = (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) == 0);
     if (!msg.m_valid_checksum) {
-        LogPrint(BCLog::NET, "CHECKSUM ERROR (%s, %u bytes), expected %s was %s\n",
+        LogPrint(BCLog::NET, "CHECKSUM ERROR (%s, %u bytes), expected %s was %s, peer=%d\n",
                  SanitizeString(msg.m_command), msg.m_message_size,
                  HexStr(Span<uint8_t>(hash.begin(), hash.begin() + CMessageHeader::CHECKSUM_SIZE)),
-                 HexStr(hdr.pchChecksum));
+                 HexStr(hdr.pchChecksum),
+                 m_node_id);
     }
 
     // store receive time
@@ -2828,7 +2829,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
         LogPrint(BCLog::NET, "Added connection peer=%d\n", id);
     }
 
-    m_deserializer = MakeUnique<V1TransportDeserializer>(V1TransportDeserializer(Params().MessageStart(), SER_NETWORK, INIT_PROTO_VERSION));
+    m_deserializer = MakeUnique<V1TransportDeserializer>(V1TransportDeserializer(Params().MessageStart(), GetId(), SER_NETWORK, INIT_PROTO_VERSION));
     m_serializer = MakeUnique<V1TransportSerializer>(V1TransportSerializer());
 }
 
diff --git a/src/net.h b/src/net.h
@@ -739,6 +739,7 @@ class TransportDeserializer {
 class V1TransportDeserializer final : public TransportDeserializer
 {
 private:
+    const NodeId m_node_id; // Only for logging
     mutable CHash256 hasher;
     mutable uint256 data_hash;
     bool in_data;                   // parsing header (false) or data (true)
@@ -764,8 +765,12 @@ class V1TransportDeserializer final : public TransportDeserializer
     }
 
 public:
-
-    V1TransportDeserializer(const CMessageHeader::MessageStartChars& pchMessageStartIn, int nTypeIn, int nVersionIn) : hdrbuf(nTypeIn, nVersionIn), hdr(pchMessageStartIn), vRecv(nTypeIn, nVersionIn) {
+    V1TransportDeserializer(const CMessageHeader::MessageStartChars& pchMessageStartIn, const NodeId node_id, int nTypeIn, int nVersionIn)
+        : m_node_id(node_id),
+          hdrbuf(nTypeIn, nVersionIn),
+          hdr(pchMessageStartIn),
+          vRecv(nTypeIn, nVersionIn)
+    {
         Reset();
     }
 
diff --git a/src/test/fuzz/p2p_transport_deserializer.cpp b/src/test/fuzz/p2p_transport_deserializer.cpp
@@ -19,7 +19,8 @@ void initialize()
 
 void test_one_input(const std::vector<uint8_t>& buffer)
 {
-    V1TransportDeserializer deserializer{Params().MessageStart(), SER_NETWORK, INIT_PROTO_VERSION};
+    // Construct deserializer, with a dummy NodeId
+    V1TransportDeserializer deserializer{Params().MessageStart(), (NodeId)0, SER_NETWORK, INIT_PROTO_VERSION};
     const char* pch = (const char*)buffer.data();
     size_t n_bytes = buffer.size();
     while (n_bytes > 0) {

Original file line number	Diff line number	Diff line change
`@@ -699,10 +699,11 @@ CNetMessage V1TransportDeserializer::GetMessage(const CMessageHeader::MessageSta`
`699`	`699`
`700`	`700`	`msg.m_valid_checksum = (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) == 0);`
`701`	`701`	`if (!msg.m_valid_checksum) {`
`702`		`- LogPrint(BCLog::NET, "CHECKSUM ERROR (%s, %u bytes), expected %s was %s\n",`
	`702`	`+ LogPrint(BCLog::NET, "CHECKSUM ERROR (%s, %u bytes), expected %s was %s, peer=%d\n",`
`703`	`703`	`SanitizeString(msg.m_command), msg.m_message_size,`
`704`	`704`	`HexStr(Span<uint8_t>(hash.begin(), hash.begin() + CMessageHeader::CHECKSUM_SIZE)),`
`705`		`- HexStr(hdr.pchChecksum));`
	`705`	`+ HexStr(hdr.pchChecksum),`
	`706`	`+ m_node_id);`
`706`	`707`	`}`
`707`	`708`
`708`	`709`	`// store receive time`
`@@ -2828,7 +2829,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn`
`2828`	`2829`	`LogPrint(BCLog::NET, "Added connection peer=%d\n", id);`
`2829`	`2830`	`}`
`2830`	`2831`
`2831`		`- m_deserializer = MakeUnique<V1TransportDeserializer>(V1TransportDeserializer(Params().MessageStart(), SER_NETWORK, INIT_PROTO_VERSION));`
	`2832`	`+ m_deserializer = MakeUnique<V1TransportDeserializer>(V1TransportDeserializer(Params().MessageStart(), GetId(), SER_NETWORK, INIT_PROTO_VERSION));`
`2832`	`2833`	`m_serializer = MakeUnique<V1TransportSerializer>(V1TransportSerializer());`
`2833`	`2834`	`}`
`2834`	`2835`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,8 @@ void initialize()`
`19`	`19`
`20`	`20`	`void test_one_input(const std::vector<uint8_t>& buffer)`
`21`	`21`	`{`
`22`		`- V1TransportDeserializer deserializer{Params().MessageStart(), SER_NETWORK, INIT_PROTO_VERSION};`
	`22`	`+ // Construct deserializer, with a dummy NodeId`
	`23`	`+ V1TransportDeserializer deserializer{Params().MessageStart(), (NodeId)0, SER_NETWORK, INIT_PROTO_VERSION};`
`23`	`24`	`const char* pch = (const char*)buffer.data();`
`24`	`25`	`size_t n_bytes = buffer.size();`
`25`	`26`	`while (n_bytes > 0) {`