Merge #19107: p2p: Move all header verification into the network layer, extend logging

deb52711a17236d0fca302701b5af585341ab42a Remove header checks out of net_processing (Troy Giorshev)
52d4ae46ab822d0f54e246a6f2364415cda149bd Give V1TransportDeserializer CChainParams& member (Troy Giorshev)
5bceef6b12fa16d20287693be377dace3dfec3e5 Change CMessageHeader Constructor (Troy Giorshev)
1ca20c1af8f08f07c407c3183c37b467ddf0f413 Add doxygen comment for ReceiveMsgBytes (Troy Giorshev)
890b1d7c2b8312d41d048d2db124586c5dbc8a49 Move checksum check from net_processing to net (Troy Giorshev)
2716647ebf60cea05fc9edce6a18dcce4e7727ad Give V1TransportDeserializer an m_node_id member (Troy Giorshev)

Pull request description:

  Inspired by #15206 and #15197, this PR moves all message header verification from the message processing layer and into the network/transport layer.

  In the previous PRs there is a change in behavior, where we would disconnect from peers upon a single failed checksum check.  In various discussions there was concern over whether this was the right choice, and some expressed a desire to see how this would look if it was made to be a pure refactor.

  For more context, see https://bitcoincore.reviews/15206.html#l-81.

  This PR improves the separation between the p2p layers, helping improvements like [BIP324](https://github.com/bitcoin/bitcoin/pull/18242) and #18989.

ACKs for top commit:
  ryanofsky:
    Code review ACK deb52711a17236d0fca302701b5af585341ab42a just rebase due to conflict on adjacent line
  jnewbery:
    Code review ACK deb52711a17236d0fca302701b5af585341ab42a.

Tree-SHA512: 1a3b7ae883b020cfee1bef968813e04df651ffdad9dd961a826bd80654f2c98676ce7f4721038a1b78d8790e4cebe8060419e3d8affc97ce2b9b4e4b72e6fa9f

2 files changed, 11 insertions, 14 deletions

diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp
index 54793c890f..b799d3b43b 100644
--- a/src/test/fuzz/deserialize.cpp
+++ b/src/test/fuzz/deserialize.cpp
@@ -189,10 +189,9 @@ void test_one_input(const std::vector<uint8_t>& buffer)
         DeserializeFromFuzzingInput(buffer, s);
         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);
+        (void)mh.IsCommandValid();
 #elif ADDRESS_DESERIALIZE
         CAddress a;
         DeserializeFromFuzzingInput(buffer, a);
diff --git a/src/test/fuzz/p2p_transport_deserializer.cpp b/src/test/fuzz/p2p_transport_deserializer.cpp
index 6fba2bfaba..7e216e16fe 100644
--- 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(), (NodeId)0, SER_NETWORK, INIT_PROTO_VERSION};
     const char* pch = (const char*)buffer.data();
     size_t n_bytes = buffer.size();
     while (n_bytes > 0) {
@@ -31,16 +32,13 @@ void test_one_input(const std::vector<uint8_t>& buffer)
         n_bytes -= handled;
         if (deserializer.Complete()) {
             const std::chrono::microseconds m_time{std::numeric_limits<int64_t>::max()};
-            const CNetMessage msg = deserializer.GetMessage(Params().MessageStart(), m_time);
-            assert(msg.m_command.size() <= CMessageHeader::COMMAND_SIZE);
-            assert(msg.m_raw_message_size <= buffer.size());
-            assert(msg.m_raw_message_size == CMessageHeader::HEADER_SIZE + msg.m_message_size);
-            assert(msg.m_time == m_time);
-            if (msg.m_valid_header) {
-                assert(msg.m_valid_netmagic);
-            }
-            if (!msg.m_valid_netmagic) {
-                assert(!msg.m_valid_header);
+            uint32_t out_err_raw_size{0};
+            Optional<CNetMessage> result{deserializer.GetMessage(m_time, out_err_raw_size)};
+            if (result) {
+                assert(result->m_command.size() <= CMessageHeader::COMMAND_SIZE);
+                assert(result->m_raw_message_size <= buffer.size());
+                assert(result->m_raw_message_size == CMessageHeader::HEADER_SIZE + result->m_message_size);
+                assert(result->m_time == m_time);
             }
         }
     }