Change CMessageHeader Constructor

troygiorshev · troygiorshev · commit 5bceef6b12fa · 2020-09-22T22:01:14.000-04:00
This commit removes the single-parameter contructor of CMessageHeader
and replaces it with a default constructor.

The single parameter contructor isn't used anywhere except for tests.
There is no reason to initialize a CMessageHeader with a particular
messagestart.  This messagestart should always be replaced when
deserializing an actual message header so that we can run checks on it.

The default constructor initializes it to zero, just like the command
and checksum.

This also removes a parameter of a V1TransportDeserializer constructor,
as it was only used for this purpose.
diff --git a/src/net.cpp b/src/net.cpp
@@ -2846,7 +2846,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(), GetId(), SER_NETWORK, INIT_PROTO_VERSION));
+    m_deserializer = MakeUnique<V1TransportDeserializer>(V1TransportDeserializer(GetId(), SER_NETWORK, INIT_PROTO_VERSION));
     m_serializer = MakeUnique<V1TransportSerializer>(V1TransportSerializer());
 }
 
diff --git a/src/net.h b/src/net.h
@@ -765,10 +765,9 @@ class V1TransportDeserializer final : public TransportDeserializer
     }
 
 public:
-    V1TransportDeserializer(const CMessageHeader::MessageStartChars& pchMessageStartIn, const NodeId node_id, int nTypeIn, int nVersionIn)
+    V1TransportDeserializer(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/protocol.cpp b/src/protocol.cpp
@@ -84,9 +84,9 @@ const static std::string allNetMessageTypes[] = {
 };
 const static std::vector<std::string> allNetMessageTypesVec(allNetMessageTypes, allNetMessageTypes+ARRAYLEN(allNetMessageTypes));
 
-CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn)
+CMessageHeader::CMessageHeader()
 {
-    memcpy(pchMessageStart, pchMessageStartIn, MESSAGE_START_SIZE);
+    memset(pchMessageStart, 0, MESSAGE_START_SIZE);
     memset(pchCommand, 0, sizeof(pchCommand));
     nMessageSize = -1;
     memset(pchChecksum, 0, CHECKSUM_SIZE);
diff --git a/src/protocol.h b/src/protocol.h
@@ -37,7 +37,7 @@ class CMessageHeader
     static constexpr size_t HEADER_SIZE = MESSAGE_START_SIZE + COMMAND_SIZE + MESSAGE_SIZE_SIZE + CHECKSUM_SIZE;
     typedef unsigned char MessageStartChars[MESSAGE_START_SIZE];
 
-    explicit CMessageHeader(const MessageStartChars& pchMessageStartIn);
+    explicit CMessageHeader();
 
     /** Construct a P2P message header from message-start characters, a command and the size of the message.
      * @note Passing in a `pszCommand` longer than COMMAND_SIZE will result in a run-time assertion error.
diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp
@@ -190,7 +190,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
         AssertEqualAfterSerializeDeserialize(s);
 #elif MESSAGEHEADER_DESERIALIZE
         const CMessageHeader::MessageStartChars pchMessageStart = {0x00, 0x00, 0x00, 0x00};
-        CMessageHeader mh(pchMessageStart);
+        CMessageHeader mh;
         DeserializeFromFuzzingInput(buffer, mh);
         (void)mh.IsValid(pchMessageStart);
 #elif ADDRESS_DESERIALIZE
diff --git a/src/test/fuzz/p2p_transport_deserializer.cpp b/src/test/fuzz/p2p_transport_deserializer.cpp
@@ -20,7 +20,7 @@ void initialize()
 void test_one_input(const std::vector<uint8_t>& buffer)
 {
     // Construct deserializer, with a dummy NodeId
-    V1TransportDeserializer deserializer{Params().MessageStart(), (NodeId)0, SER_NETWORK, INIT_PROTO_VERSION};
+    V1TransportDeserializer deserializer{(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
`@@ -2846,7 +2846,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn`
`2846`	`2846`	`LogPrint(BCLog::NET, "Added connection peer=%d\n", id);`
`2847`	`2847`	`}`
`2848`	`2848`
`2849`		`- m_deserializer = MakeUnique<V1TransportDeserializer>(V1TransportDeserializer(Params().MessageStart(), GetId(), SER_NETWORK, INIT_PROTO_VERSION));`
	`2849`	`+ m_deserializer = MakeUnique<V1TransportDeserializer>(V1TransportDeserializer(GetId(), SER_NETWORK, INIT_PROTO_VERSION));`
`2850`	`2850`	`m_serializer = MakeUnique<V1TransportSerializer>(V1TransportSerializer());`
`2851`	`2851`	`}`
`2852`	`2852`
Original file line number	Diff line number	Diff line change
`@@ -765,10 +765,9 @@ class V1TransportDeserializer final : public TransportDeserializer`
`765`	`765`	`}`
`766`	`766`
`767`	`767`	`public:`
`768`		`- V1TransportDeserializer(const CMessageHeader::MessageStartChars& pchMessageStartIn, const NodeId node_id, int nTypeIn, int nVersionIn)`
	`768`	`+ V1TransportDeserializer(const NodeId node_id, int nTypeIn, int nVersionIn)`
`769`	`769`	`: m_node_id(node_id),`
`770`	`770`	`hdrbuf(nTypeIn, nVersionIn),`
`771`		`- hdr(pchMessageStartIn),`
`772`	`771`	`vRecv(nTypeIn, nVersionIn)`
`773`	`772`	`{`
`774`	`773`	`Reset();`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ void initialize()`
`20`	`20`	`void test_one_input(const std::vector<uint8_t>& buffer)`
`21`	`21`	`{`
`22`	`22`	`// Construct deserializer, with a dummy NodeId`
`23`		`- V1TransportDeserializer deserializer{Params().MessageStart(), (NodeId)0, SER_NETWORK, INIT_PROTO_VERSION};`
	`23`	`+ V1TransportDeserializer deserializer{(NodeId)0, SER_NETWORK, INIT_PROTO_VERSION};`
`24`	`24`	`const char* pch = (const char*)buffer.data();`
`25`	`25`	`size_t n_bytes = buffer.size();`
`26`	`26`	`while (n_bytes > 0) {`