docs: "Fuzzing the Bitcoin Core P2P layer using Honggfuzz NetDriver" is outdated #30957

Similar to #28019. The following instruction is outdated and doesn't work:

$ git apply << "EOF"
diff --git a/src/compat/compat.h b/src/compat/compat.h
index 8195bceaec..cce2b31ff0 100644
--- a/src/compat/compat.h
+++ b/src/compat/compat.h
@@ -90,8 +90,12 @@ typedef char* sockopt_arg_type;
 // building with a binutils < 2.36 is subject to this ld bug.
 #define MAIN_FUNCTION __declspec(dllexport) int main(int argc, char* argv[])
 #else
+#ifdef HFND_FUZZING_ENTRY_FUNCTION_CXX
+#define MAIN_FUNCTION HFND_FUZZING_ENTRY_FUNCTION_CXX(int argc, char* argv[])
+#else
 #define MAIN_FUNCTION int main(int argc, char* argv[])
 #endif
+#endif

 // Note these both should work with the current usage of poll, but best to be safe
 // WIN32 poll is broken https://daniel.haxx.se/blog/2012/10/10/wsapoll-is-broken/
diff --git a/src/net.cpp b/src/net.cpp
index 7601a6ea84..702d0f56ce 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -727,7 +727,7 @@ int V1TransportDeserializer::readHeader(Span<const uint8_t> msg_bytes)
     }

     // Check start string, network magic
-    if (memcmp(hdr.pchMessageStart, m_chain_params.MessageStart(), CMessageHeader::MESSAGE_START_SIZE) != 0) {
+    if (false && memcmp(hdr.pchMessageStart, m_chain_params.MessageStart(), CMessageHeader::MESSAGE_START_SIZE) != 0) { // skip network magic checking
         LogDebug(BCLog::NET, "Header error: Wrong MessageStart %s received, peer=%d\n", HexStr(hdr.pchMessageStart), m_node_id);
         return -1;
     }
@@ -788,7 +788,7 @@ CNetMessage V1TransportDeserializer::GetMessage(const std::chrono::microseconds
     RandAddEvent(ReadLE32(hash.begin()));

     // Check checksum and header message type string
-    if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) {
+    if (false && memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) { // skip checksum checking
         LogDebug(BCLog::NET, "Header error: Wrong checksum (%s, %u bytes), expected %s was %s, peer=%d\n",
                  SanitizeString(msg.m_type), msg.m_message_size,
                  HexStr(Span{hash}.first(CMessageHeader::CHECKSUM_SIZE)),
EOF

Should be trivial to rebase it, no?

diff --git a/src/compat/compat.h b/src/compat/compat.h
index 366c648ae7..1f3bf2b018 100644
--- a/src/compat/compat.h
+++ b/src/compat/compat.h
@@ -92,8 +92,12 @@ typedef char* sockopt_arg_type;
 // building with a binutils < 2.36 is subject to this ld bug.
 #define MAIN_FUNCTION __declspec(dllexport) int main(int argc, char* argv[])
 #else
+#ifdef HFND_FUZZING_ENTRY_FUNCTION_CXX
+#define MAIN_FUNCTION HFND_FUZZING_ENTRY_FUNCTION_CXX(int argc, char* argv[])
+#else
 #define MAIN_FUNCTION int main(int argc, char* argv[])
 #endif
+#endif
 
 // Note these both should work with the current usage of poll, but best to be safe
 // WIN32 poll is broken https://daniel.haxx.se/blog/2012/10/10/wsapoll-is-broken/
diff --git a/src/net.cpp b/src/net.cpp
index d87adcc031..7e5f03ac67 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -727,7 +727,7 @@ int V1Transport::readHeader(Span<const uint8_t> msg_bytes)
     }
 
     // Check start string, network magic
-    if (hdr.pchMessageStart != m_magic_bytes) {
+    if (false && hdr.pchMessageStart != m_magic_bytes) { // skip network magic checking
         LogDebug(BCLog::NET, "Header error: Wrong MessageStart %s received, peer=%d\n", HexStr(hdr.pchMessageStart), m_node_id);
         return -1;
     }
@@ -792,7 +792,7 @@ CNetMessage V1Transport::GetReceivedMessage(const std::chrono::microseconds time
     RandAddEvent(ReadLE32(hash.begin()));
 
     // Check checksum and header message type string
-    if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) {
+    if (false && memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) { // skip checksum checking
         LogDebug(BCLog::NET, "Header error: Wrong checksum (%s, %u bytes), expected %s was %s, peer=%d\n",
                  SanitizeString(msg.m_type), msg.m_message_size,
                  HexStr(Span{hash}.first(CMessageHeader::CHECKSUM_SIZE)),

How about using FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION for these checks? Then we wouldn't have to maintain a patch in the docs.

How about using FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION for these checks? Then we wouldn't have to maintain a patch in the docs.

Sounds good, I can address it if others agree. Just a question, couldn't it affect other targets that also reaches it?

Just a question, couldn't it affect other targets that also reaches it?

Yes I think so but similar to the PoW check it never makes sense to have this check in the way. The p2p_transport_serialization harness currently has a bunch of extra logic for reaching the unhappy/happy paths of these checks. We can achieve the same with the macro (without having to create valid checksum in the test), e.g.:

#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
if (hdr.pchChecksum[0]&1 != 0) {
#else
if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) {
#endif

...
}

Yes I think so but similar to the PoW check it never makes sense to have this check in the way. The p2p_transport_serialization harness currently has a bunch of extra logic for reaching the unhappy/happy paths of these checks. We can achieve the same with the macro (without having to create valid checksum in the test), e.g.:

Looks good to me!