From 37e2b011136ca1cf00dfb9e575d12f0d035a6a2c Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Wed, 2 Aug 2023 10:24:20 +0200
Subject: [PATCH 1/7] [refactor] Allow std::array<std::byte, N> in serialize.h

This is already possible for C-style arrays, so allow it for C++11
std::array as well.
---
 src/serialize.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/serialize.h b/src/serialize.h
index 2d790190a0b..f1595077e91 100644
--- a/src/serialize.h
+++ b/src/serialize.h
@@ -285,6 +285,7 @@ template<typename Stream> inline void Serialize(Stream& s, int64_t a ) { ser_wri
 template<typename Stream> inline void Serialize(Stream& s, uint64_t a) { ser_writedata64(s, a); }
 template<typename Stream, int N> inline void Serialize(Stream& s, const char (&a)[N]) { s.write(MakeByteSpan(a)); }
 template<typename Stream, int N> inline void Serialize(Stream& s, const unsigned char (&a)[N]) { s.write(MakeByteSpan(a)); }
+template <typename Stream, typename B, std::size_t N> void Serialize(Stream& s, const std::array<B, N>& a) { (void)/* force byte-type */UCharCast(a.data()); s.write(MakeByteSpan(a)); }
 template <typename Stream, typename B> void Serialize(Stream& s, Span<B> span) { (void)/* force byte-type */UCharCast(span.data()); s.write(AsBytes(span)); }
 
 #ifndef CHAR_EQUALS_INT8
@@ -301,6 +302,7 @@ template<typename Stream> inline void Unserialize(Stream& s, int64_t& a ) { a =
 template<typename Stream> inline void Unserialize(Stream& s, uint64_t& a) { a = ser_readdata64(s); }
 template<typename Stream, int N> inline void Unserialize(Stream& s, char (&a)[N]) { s.read(MakeWritableByteSpan(a)); }
 template<typename Stream, int N> inline void Unserialize(Stream& s, unsigned char (&a)[N]) { s.read(MakeWritableByteSpan(a)); }
+template <typename Stream, typename B, std::size_t N> void Unserialize(Stream& s, std::array<B, N>& a) { (void)/* force byte-type */UCharCast(a.data()); s.read(MakeWritableByteSpan(a)); }
 template <typename Stream, typename B> void Unserialize(Stream& s, Span<B> span) { (void)/* force byte-type */UCharCast(span.data()); s.read(AsWritableBytes(span)); }
 
 template <typename Stream> inline void Serialize(Stream& s, bool a) { uint8_t f = a; ser_writedata8(s, f); }

From 9be330b654cfbd792620295f3867f592059d6a7a Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Tue, 12 Sep 2023 13:01:07 +0200
Subject: [PATCH 2/7] [refactor] Define MessageStartChars as std::array

---
 src/addrdb.cpp             |  4 ++--
 src/kernel/chainparams.cpp |  2 +-
 src/net.cpp                |  6 +++---
 src/node/blockstorage.cpp  |  2 +-
 src/node/blockstorage.h    |  2 +-
 src/protocol.cpp           |  2 +-
 src/protocol.h             | 10 +++++-----
 src/validation.cpp         |  4 ++--
 src/wallet/db.cpp          |  2 +-
 src/wallet/sqlite.cpp      |  4 ++--
 10 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/src/addrdb.cpp b/src/addrdb.cpp
index c5b474339b9..6b21d3a1ed9 100644
--- a/src/addrdb.cpp
+++ b/src/addrdb.cpp
@@ -90,10 +90,10 @@ void DeserializeDB(Stream& stream, Data&& data, bool fCheckSum = true)
 {
     HashVerifier verifier{stream};
     // de-serialize file header (network specific magic number) and ..
-    unsigned char pchMsgTmp[4];
+    CMessageHeader::MessageStartChars pchMsgTmp;
     verifier >> pchMsgTmp;
     // ... verify the network matches ours
-    if (memcmp(pchMsgTmp, Params().MessageStart(), sizeof(pchMsgTmp))) {
+    if (pchMsgTmp != Params().MessageStart()) {
         throw std::runtime_error{"Invalid network magic number"};
     }
 
diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp
index 733a3339b38..c41559ad98f 100644
--- a/src/kernel/chainparams.cpp
+++ b/src/kernel/chainparams.cpp
@@ -359,7 +359,7 @@ public:
         HashWriter h{};
         h << consensus.signet_challenge;
         uint256 hash = h.GetHash();
-        memcpy(pchMessageStart, hash.begin(), 4);
+        std::copy_n(hash.begin(), 4, pchMessageStart.begin());
 
         nDefaultPort = 38333;
         nPruneAfterHeight = 1000;
diff --git a/src/net.cpp b/src/net.cpp
index af2855932de..d091db74c4a 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -730,7 +730,7 @@ V1Transport::V1Transport(const NodeId node_id, int nTypeIn, int nVersionIn) noex
     m_node_id(node_id), hdrbuf(nTypeIn, nVersionIn), vRecv(nTypeIn, nVersionIn)
 {
     assert(std::size(Params().MessageStart()) == std::size(m_magic_bytes));
-    std::copy(std::begin(Params().MessageStart()), std::end(Params().MessageStart()), m_magic_bytes);
+    m_magic_bytes = Params().MessageStart();
     LOCK(m_recv_mutex);
     Reset();
 }
@@ -759,7 +759,7 @@ int V1Transport::readHeader(Span<const uint8_t> msg_bytes)
     }
 
     // Check start string, network magic
-    if (memcmp(hdr.pchMessageStart, m_magic_bytes, CMessageHeader::MESSAGE_START_SIZE) != 0) {
+    if (hdr.pchMessageStart != m_magic_bytes) {
         LogPrint(BCLog::NET, "Header error: Wrong MessageStart %s received, peer=%d\n", HexStr(hdr.pchMessageStart), m_node_id);
         return -1;
     }
@@ -1144,7 +1144,7 @@ bool V2Transport::ProcessReceivedKeyBytes() noexcept
     // they receive our uniformly random key and garbage, but detecting this case specially
     // means we can log it.
     static constexpr std::array<uint8_t, 12> MATCH = {'v', 'e', 'r', 's', 'i', 'o', 'n', 0, 0, 0, 0, 0};
-    static constexpr size_t OFFSET = sizeof(CMessageHeader::MessageStartChars);
+    static constexpr size_t OFFSET = std::tuple_size_v<CMessageHeader::MessageStartChars>;
     if (!m_initiating && m_recv_buffer.size() >= OFFSET + MATCH.size()) {
         if (std::equal(MATCH.begin(), MATCH.end(), m_recv_buffer.begin() + OFFSET)) {
             LogPrint(BCLog::NET, "V2 transport error: V1 peer with wrong MessageStart %s\n",
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index 70f11be5861..2e4e077f187 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -932,7 +932,7 @@ bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatF
 
         filein >> blk_start >> blk_size;
 
-        if (memcmp(blk_start, GetParams().MessageStart(), CMessageHeader::MESSAGE_START_SIZE)) {
+        if (blk_start != GetParams().MessageStart()) {
             return error("%s: Block magic mismatch for %s: %s versus expected %s", __func__, pos.ToString(),
                          HexStr(blk_start),
                          HexStr(GetParams().MessageStart()));
diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
index c79fd2c6a14..4b57637427f 100644
--- a/src/node/blockstorage.h
+++ b/src/node/blockstorage.h
@@ -73,7 +73,7 @@ static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB
 static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB
 
 /** Size of header written by WriteBlockToDisk before a serialized CBlock */
-static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = CMessageHeader::MESSAGE_START_SIZE + sizeof(unsigned int);
+static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = std::tuple_size_v<CMessageHeader::MessageStartChars> + sizeof(unsigned int);
 
 extern std::atomic_bool fReindex;
 
diff --git a/src/protocol.cpp b/src/protocol.cpp
index 2105480c72a..cb956191e4d 100644
--- a/src/protocol.cpp
+++ b/src/protocol.cpp
@@ -92,7 +92,7 @@ const static std::vector<std::string> g_all_net_message_types{
 
 CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn, const char* pszCommand, unsigned int nMessageSizeIn)
 {
-    memcpy(pchMessageStart, pchMessageStartIn, MESSAGE_START_SIZE);
+    pchMessageStart = pchMessageStartIn;
 
     // Copy the command name
     size_t i = 0;
diff --git a/src/protocol.h b/src/protocol.h
index a7ca0c6f3e3..cb7f9666e14 100644
--- a/src/protocol.h
+++ b/src/protocol.h
@@ -13,6 +13,7 @@
 #include <uint256.h>
 #include <util/time.h>
 
+#include <array>
 #include <cstdint>
 #include <limits>
 #include <string>
@@ -26,14 +27,13 @@
 class CMessageHeader
 {
 public:
-    static constexpr size_t MESSAGE_START_SIZE = 4;
+    using MessageStartChars = std::array<uint8_t, 4>;
     static constexpr size_t COMMAND_SIZE = 12;
     static constexpr size_t MESSAGE_SIZE_SIZE = 4;
     static constexpr size_t CHECKSUM_SIZE = 4;
-    static constexpr size_t MESSAGE_SIZE_OFFSET = MESSAGE_START_SIZE + COMMAND_SIZE;
+    static constexpr size_t MESSAGE_SIZE_OFFSET = std::tuple_size_v<MessageStartChars> + COMMAND_SIZE;
     static constexpr size_t CHECKSUM_OFFSET = MESSAGE_SIZE_OFFSET + MESSAGE_SIZE_SIZE;
-    static constexpr size_t HEADER_SIZE = MESSAGE_START_SIZE + COMMAND_SIZE + MESSAGE_SIZE_SIZE + CHECKSUM_SIZE;
-    typedef unsigned char MessageStartChars[MESSAGE_START_SIZE];
+    static constexpr size_t HEADER_SIZE = std::tuple_size_v<MessageStartChars> + COMMAND_SIZE + MESSAGE_SIZE_SIZE + CHECKSUM_SIZE;
 
     explicit CMessageHeader() = default;
 
@@ -47,7 +47,7 @@ public:
 
     SERIALIZE_METHODS(CMessageHeader, obj) { READWRITE(obj.pchMessageStart, obj.pchCommand, obj.nMessageSize, obj.pchChecksum); }
 
-    char pchMessageStart[MESSAGE_START_SIZE]{};
+    MessageStartChars pchMessageStart{};
     char pchCommand[COMMAND_SIZE]{};
     uint32_t nMessageSize{std::numeric_limits<uint32_t>::max()};
     uint8_t pchChecksum[CHECKSUM_SIZE]{};
diff --git a/src/validation.cpp b/src/validation.cpp
index e3a00e4241e..4bb648d1e5b 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4533,11 +4533,11 @@ void ChainstateManager::LoadExternalBlockFile(
             unsigned int nSize = 0;
             try {
                 // locate a header
-                unsigned char buf[CMessageHeader::MESSAGE_START_SIZE];
+                CMessageHeader::MessageStartChars buf;
                 blkdat.FindByte(std::byte(params.MessageStart()[0]));
                 nRewind = blkdat.GetPos() + 1;
                 blkdat >> buf;
-                if (memcmp(buf, params.MessageStart(), CMessageHeader::MESSAGE_START_SIZE)) {
+                if (buf != params.MessageStart()) {
                     continue;
                 }
                 // read size
diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp
index 0c249205162..0ee39d2b5a6 100644
--- a/src/wallet/db.cpp
+++ b/src/wallet/db.cpp
@@ -136,7 +136,7 @@ bool IsSQLiteFile(const fs::path& path)
     }
 
     // Check the application id matches our network magic
-    return memcmp(Params().MessageStart(), app_id, 4) == 0;
+    return memcmp(Params().MessageStart().data(), app_id, 4) == 0;
 }
 
 void ReadDatabaseArgs(const ArgsManager& args, DatabaseOptions& options)
diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp
index ecd34bb96a6..db9989163df 100644
--- a/src/wallet/sqlite.cpp
+++ b/src/wallet/sqlite.cpp
@@ -193,7 +193,7 @@ bool SQLiteDatabase::Verify(bilingual_str& error)
     auto read_result = ReadPragmaInteger(m_db, "application_id", "the application id", error);
     if (!read_result.has_value()) return false;
     uint32_t app_id = static_cast<uint32_t>(read_result.value());
-    uint32_t net_magic = ReadBE32(Params().MessageStart());
+    uint32_t net_magic = ReadBE32(Params().MessageStart().data());
     if (app_id != net_magic) {
         error = strprintf(_("SQLiteDatabase: Unexpected application id. Expected %u, got %u"), net_magic, app_id);
         return false;
@@ -324,7 +324,7 @@ void SQLiteDatabase::Open()
         }
 
         // Set the application id
-        uint32_t app_id = ReadBE32(Params().MessageStart());
+        uint32_t app_id = ReadBE32(Params().MessageStart().data());
         SetPragma(m_db, "application_id", strprintf("%d", static_cast<int32_t>(app_id)),
                   "Failed to set the application id");
 

From 534b314a7401d44f51aabd4565f97be9ee411740 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Wed, 6 Sep 2023 15:55:14 +0200
Subject: [PATCH 3/7] kernel: Move MessageStartChars to its own file

The protocol.h file contains many non-consensus related definitions and
should thus not be part of the libbitcoinkernel. This commit makes
protocol.h no longer a required include for users of the
libbitcoinkernel.

This commit is part of the libbitcoinkernel project, namely its stage 1
step 3: Decouple most non-consensus headers from libbitcoinkernel.

Co-Authored-By: Cory Fields <cory-nospam-@coryfields.com>
---
 src/Makefile.am                |  1 +
 src/addrdb.cpp                 |  2 +-
 src/kernel/chainparams.cpp     |  1 +
 src/kernel/chainparams.h       |  6 +++---
 src/kernel/messagestartchars.h | 13 +++++++++++++
 src/net.cpp                    |  2 +-
 src/net.h                      |  5 +++--
 src/node/blockstorage.cpp      |  3 ++-
 src/node/blockstorage.h        |  4 ++--
 src/protocol.h                 |  2 +-
 src/test/net_tests.cpp         |  2 +-
 src/validation.cpp             |  3 ++-
 12 files changed, 31 insertions(+), 13 deletions(-)
 create mode 100644 src/kernel/messagestartchars.h

diff --git a/src/Makefile.am b/src/Makefile.am
index feed4a0061c..e542a067a47 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -191,6 +191,7 @@ BITCOIN_CORE_H = \
   kernel/mempool_options.h \
   kernel/mempool_persist.h \
   kernel/mempool_removal_reason.h \
+  kernel/messagestartchars.h \
   kernel/notifications_interface.h \
   kernel/validation_cache_sizes.h \
   key.h \
diff --git a/src/addrdb.cpp b/src/addrdb.cpp
index 6b21d3a1ed9..50f576624c6 100644
--- a/src/addrdb.cpp
+++ b/src/addrdb.cpp
@@ -90,7 +90,7 @@ void DeserializeDB(Stream& stream, Data&& data, bool fCheckSum = true)
 {
     HashVerifier verifier{stream};
     // de-serialize file header (network specific magic number) and ..
-    CMessageHeader::MessageStartChars pchMsgTmp;
+    MessageStartChars pchMsgTmp;
     verifier >> pchMsgTmp;
     // ... verify the network matches ours
     if (pchMsgTmp != Params().MessageStart()) {
diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp
index c41559ad98f..6cee379faf9 100644
--- a/src/kernel/chainparams.cpp
+++ b/src/kernel/chainparams.cpp
@@ -10,6 +10,7 @@
 #include <consensus/merkle.h>
 #include <consensus/params.h>
 #include <hash.h>
+#include <kernel/messagestartchars.h>
 #include <logging.h>
 #include <primitives/block.h>
 #include <primitives/transaction.h>
diff --git a/src/kernel/chainparams.h b/src/kernel/chainparams.h
index 2d38af609c9..f1df878f60b 100644
--- a/src/kernel/chainparams.h
+++ b/src/kernel/chainparams.h
@@ -7,9 +7,9 @@
 #define BITCOIN_KERNEL_CHAINPARAMS_H
 
 #include <consensus/params.h>
+#include <kernel/messagestartchars.h>
 #include <netaddress.h>
 #include <primitives/block.h>
-#include <protocol.h>
 #include <uint256.h>
 #include <util/chaintype.h>
 #include <util/hash_type.h>
@@ -87,7 +87,7 @@ public:
     };
 
     const Consensus::Params& GetConsensus() const { return consensus; }
-    const CMessageHeader::MessageStartChars& MessageStart() const { return pchMessageStart; }
+    const MessageStartChars& MessageStart() const { return pchMessageStart; }
     uint16_t GetDefaultPort() const { return nDefaultPort; }
     uint16_t GetDefaultPort(Network net) const
     {
@@ -165,7 +165,7 @@ protected:
     CChainParams() {}
 
     Consensus::Params consensus;
-    CMessageHeader::MessageStartChars pchMessageStart;
+    MessageStartChars pchMessageStart;
     uint16_t nDefaultPort;
     uint64_t nPruneAfterHeight;
     uint64_t m_assumed_blockchain_size;
diff --git a/src/kernel/messagestartchars.h b/src/kernel/messagestartchars.h
new file mode 100644
index 00000000000..829616dc8bd
--- /dev/null
+++ b/src/kernel/messagestartchars.h
@@ -0,0 +1,13 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_KERNEL_MESSAGESTARTCHARS_H
+#define BITCOIN_KERNEL_MESSAGESTARTCHARS_H
+
+#include <array>
+#include <cstdint>
+
+using MessageStartChars = std::array<uint8_t, 4>;
+
+#endif // BITCOIN_KERNEL_MESSAGESTARTCHARS_H
diff --git a/src/net.cpp b/src/net.cpp
index d091db74c4a..ddbf9a6e3c3 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1144,7 +1144,7 @@ bool V2Transport::ProcessReceivedKeyBytes() noexcept
     // they receive our uniformly random key and garbage, but detecting this case specially
     // means we can log it.
     static constexpr std::array<uint8_t, 12> MATCH = {'v', 'e', 'r', 's', 'i', 'o', 'n', 0, 0, 0, 0, 0};
-    static constexpr size_t OFFSET = std::tuple_size_v<CMessageHeader::MessageStartChars>;
+    static constexpr size_t OFFSET = std::tuple_size_v<MessageStartChars>;
     if (!m_initiating && m_recv_buffer.size() >= OFFSET + MATCH.size()) {
         if (std::equal(MATCH.begin(), MATCH.end(), m_recv_buffer.begin() + OFFSET)) {
             LogPrint(BCLog::NET, "V2 transport error: V1 peer with wrong MessageStart %s\n",
diff --git a/src/net.h b/src/net.h
index e1d8995a8eb..669ff68cb4b 100644
--- a/src/net.h
+++ b/src/net.h
@@ -10,15 +10,16 @@
 #include <chainparams.h>
 #include <common/bloom.h>
 #include <compat/compat.h>
-#include <node/connection_types.h>
 #include <consensus/amount.h>
 #include <crypto/siphash.h>
 #include <hash.h>
 #include <i2p.h>
+#include <kernel/messagestartchars.h>
 #include <net_permissions.h>
 #include <netaddress.h>
 #include <netbase.h>
 #include <netgroup.h>
+#include <node/connection_types.h>
 #include <policy/feerate.h>
 #include <protocol.h>
 #include <random.h>
@@ -360,7 +361,7 @@ public:
 class V1Transport final : public Transport
 {
 private:
-    CMessageHeader::MessageStartChars m_magic_bytes;
+    MessageStartChars m_magic_bytes;
     const NodeId m_node_id; // Only for logging
     mutable Mutex m_recv_mutex; //!< Lock for receive state
     mutable CHash256 hasher GUARDED_BY(m_recv_mutex);
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index 2e4e077f187..253dee088e7 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -11,6 +11,7 @@
 #include <flatfile.h>
 #include <hash.h>
 #include <kernel/chainparams.h>
+#include <kernel/messagestartchars.h>
 #include <logging.h>
 #include <pow.h>
 #include <reverse_iterator.h>
@@ -927,7 +928,7 @@ bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatF
     }
 
     try {
-        CMessageHeader::MessageStartChars blk_start;
+        MessageStartChars blk_start;
         unsigned int blk_size;
 
         filein >> blk_start >> blk_size;
diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
index 4b57637427f..b251ece31f2 100644
--- a/src/node/blockstorage.h
+++ b/src/node/blockstorage.h
@@ -11,7 +11,7 @@
 #include <kernel/blockmanager_opts.h>
 #include <kernel/chainparams.h>
 #include <kernel/cs_main.h>
-#include <protocol.h>
+#include <kernel/messagestartchars.h>
 #include <sync.h>
 #include <util/fs.h>
 #include <util/hasher.h>
@@ -73,7 +73,7 @@ static const unsigned int UNDOFILE_CHUNK_SIZE = 0x100000; // 1 MiB
 static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB
 
 /** Size of header written by WriteBlockToDisk before a serialized CBlock */
-static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = std::tuple_size_v<CMessageHeader::MessageStartChars> + sizeof(unsigned int);
+static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = std::tuple_size_v<MessageStartChars> + sizeof(unsigned int);
 
 extern std::atomic_bool fReindex;
 
diff --git a/src/protocol.h b/src/protocol.h
index cb7f9666e14..22e2108afb1 100644
--- a/src/protocol.h
+++ b/src/protocol.h
@@ -6,6 +6,7 @@
 #ifndef BITCOIN_PROTOCOL_H
 #define BITCOIN_PROTOCOL_H
 
+#include <kernel/messagestartchars.h> // IWYU pragma: export
 #include <netaddress.h>
 #include <primitives/transaction.h>
 #include <serialize.h>
@@ -27,7 +28,6 @@
 class CMessageHeader
 {
 public:
-    using MessageStartChars = std::array<uint8_t, 4>;
     static constexpr size_t COMMAND_SIZE = 12;
     static constexpr size_t MESSAGE_SIZE_SIZE = 4;
     static constexpr size_t CHECKSUM_SIZE = 4;
diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
index 3eb7bdec62f..34d78670793 100644
--- a/src/test/net_tests.cpp
+++ b/src/test/net_tests.cpp
@@ -1114,7 +1114,7 @@ public:
     }
 
     /** Send V1 version message header to the transport. */
-    void SendV1Version(const CMessageHeader::MessageStartChars& magic)
+    void SendV1Version(const MessageStartChars& magic)
     {
         CMessageHeader hdr(magic, "version", 126 + InsecureRandRange(11));
         CDataStream ser(SER_NETWORK, CLIENT_VERSION);
diff --git a/src/validation.cpp b/src/validation.cpp
index 4bb648d1e5b..8d4b8386e80 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -23,6 +23,7 @@
 #include <hash.h>
 #include <kernel/chainparams.h>
 #include <kernel/mempool_entry.h>
+#include <kernel/messagestartchars.h>
 #include <kernel/notifications_interface.h>
 #include <logging.h>
 #include <logging/timer.h>
@@ -4533,7 +4534,7 @@ void ChainstateManager::LoadExternalBlockFile(
             unsigned int nSize = 0;
             try {
                 // locate a header
-                CMessageHeader::MessageStartChars buf;
+                MessageStartChars buf;
                 blkdat.FindByte(std::byte(params.MessageStart()[0]));
                 nRewind = blkdat.GetPos() + 1;
                 blkdat >> buf;

From f0d1d8b35c3aa9f2f923f74e3dbbf1e5ece4cd2f Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Wed, 6 Sep 2023 16:59:32 +0200
Subject: [PATCH 4/7] [refactor] Add missing includes for next commit

---
 src/bench/block_assemble.cpp         | 1 +
 src/bench/duplicate_inputs.cpp       | 1 +
 src/bench/mempool_stress.cpp         | 1 +
 src/bitcoin-chainstate.cpp           | 1 +
 src/bitcoin-util.cpp                 | 1 +
 src/key_io.cpp                       | 1 +
 src/node/blockstorage.cpp            | 1 +
 src/test/fuzz/descriptor_parse.cpp   | 1 +
 src/test/txvalidationcache_tests.cpp | 1 +
 9 files changed, 9 insertions(+)

diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp
index 4d032cefc5f..0737144b84f 100644
--- a/src/bench/block_assemble.cpp
+++ b/src/bench/block_assemble.cpp
@@ -6,6 +6,7 @@
 #include <consensus/validation.h>
 #include <crypto/sha256.h>
 #include <node/miner.h>
+#include <random.h>
 #include <test/util/mining.h>
 #include <test/util/script.h>
 #include <test/util/setup_common.h>
diff --git a/src/bench/duplicate_inputs.cpp b/src/bench/duplicate_inputs.cpp
index b3799ad1b72..a1c8d4d9420 100644
--- a/src/bench/duplicate_inputs.cpp
+++ b/src/bench/duplicate_inputs.cpp
@@ -7,6 +7,7 @@
 #include <consensus/merkle.h>
 #include <consensus/validation.h>
 #include <pow.h>
+#include <random.h>
 #include <test/util/setup_common.h>
 #include <txmempool.h>
 #include <validation.h>
diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp
index 1f94461d198..993db666531 100644
--- a/src/bench/mempool_stress.cpp
+++ b/src/bench/mempool_stress.cpp
@@ -5,6 +5,7 @@
 #include <bench/bench.h>
 #include <kernel/mempool_entry.h>
 #include <policy/policy.h>
+#include <random.h>
 #include <test/util/setup_common.h>
 #include <txmempool.h>
 #include <util/chaintype.h>
diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp
index 19c4d36126e..fc83a4ad3a3 100644
--- a/src/bitcoin-chainstate.cpp
+++ b/src/bitcoin-chainstate.cpp
@@ -22,6 +22,7 @@
 #include <node/blockstorage.h>
 #include <node/caches.h>
 #include <node/chainstate.h>
+#include <random.h>
 #include <scheduler.h>
 #include <script/sigcache.h>
 #include <util/chaintype.h>
diff --git a/src/bitcoin-util.cpp b/src/bitcoin-util.cpp
index 582c18c9c60..8b68d04b2bf 100644
--- a/src/bitcoin-util.cpp
+++ b/src/bitcoin-util.cpp
@@ -17,6 +17,7 @@
 #include <core_io.h>
 #include <streams.h>
 #include <util/exception.h>
+#include <util/strencodings.h>
 #include <util/translation.h>
 #include <version.h>
 
diff --git a/src/key_io.cpp b/src/key_io.cpp
index a0611656139..1a0b51a28bc 100644
--- a/src/key_io.cpp
+++ b/src/key_io.cpp
@@ -8,6 +8,7 @@
 #include <bech32.h>
 #include <script/interpreter.h>
 #include <script/solver.h>
+#include <tinyformat.h>
 #include <util/strencodings.h>
 
 #include <algorithm>
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index 253dee088e7..ba612d02046 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -22,6 +22,7 @@
 #include <util/batchpriority.h>
 #include <util/fs.h>
 #include <util/signalinterrupt.h>
+#include <util/strencodings.h>
 #include <util/translation.h>
 #include <validation.h>
 
diff --git a/src/test/fuzz/descriptor_parse.cpp b/src/test/fuzz/descriptor_parse.cpp
index 26c219d6c8a..57129a60b88 100644
--- a/src/test/fuzz/descriptor_parse.cpp
+++ b/src/test/fuzz/descriptor_parse.cpp
@@ -8,6 +8,7 @@
 #include <script/descriptor.h>
 #include <test/fuzz/fuzz.h>
 #include <util/chaintype.h>
+#include <util/strencodings.h>
 
 //! Types are raw (un)compressed pubkeys, raw xonly pubkeys, raw privkeys (WIF), xpubs, xprvs.
 static constexpr uint8_t KEY_TYPES_COUNT{6};
diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp
index c1f62269824..790eabc7c19 100644
--- a/src/test/txvalidationcache_tests.cpp
+++ b/src/test/txvalidationcache_tests.cpp
@@ -4,6 +4,7 @@
 
 #include <consensus/validation.h>
 #include <key.h>
+#include <random.h>
 #include <script/sign.h>
 #include <script/signingprovider.h>
 #include <test/util/setup_common.h>

From 2b08c55f01996e0b05763f05eac50b83ba9d5a8e Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Tue, 12 Sep 2023 13:42:36 +0200
Subject: [PATCH 5/7] [refactor] Add CChainParams member to CConnman

This is done in preparation to the next commit, but has the nice
effect of removing one further data structure relying on the global
`Params()`.
---
 src/init.cpp                       |  2 +-
 src/net.cpp                        | 15 ++++++++-------
 src/net.h                          |  5 ++++-
 src/test/denialofservice_tests.cpp |  8 ++++----
 src/test/fuzz/connman.cpp          |  1 +
 src/test/util/setup_common.cpp     |  2 +-
 6 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/src/init.cpp b/src/init.cpp
index 1515007c548..6dd3d5970bf 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1223,7 +1223,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     assert(!node.connman);
     node.connman = std::make_unique<CConnman>(GetRand<uint64_t>(),
                                               GetRand<uint64_t>(),
-                                              *node.addrman, *node.netgroupman, args.GetBoolArg("-networkactive", true));
+                                              *node.addrman, *node.netgroupman, chainparams, args.GetBoolArg("-networkactive", true));
 
     assert(!node.fee_estimator);
     // Don't initialize fee estimation with old data if we don't relay transactions,
diff --git a/src/net.cpp b/src/net.cpp
index ddbf9a6e3c3..129b049269e 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -470,8 +470,8 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
         Ticks<HoursDouble>(pszDest ? 0h : Now<NodeSeconds>() - addrConnect.nTime));
 
     // Resolve
-    const uint16_t default_port{pszDest != nullptr ? Params().GetDefaultPort(pszDest) :
-                                                     Params().GetDefaultPort()};
+    const uint16_t default_port{pszDest != nullptr ? m_params.GetDefaultPort(pszDest) :
+                                                     m_params.GetDefaultPort()};
     if (pszDest) {
         const std::vector<CService> resolved{Lookup(pszDest, default_port, fNameLookup && !HaveNameProxy(), 256)};
         if (!resolved.empty()) {
@@ -2184,7 +2184,7 @@ void CConnman::WakeMessageHandler()
 void CConnman::ThreadDNSAddressSeed()
 {
     FastRandomContext rng;
-    std::vector<std::string> seeds = Params().DNSSeeds();
+    std::vector<std::string> seeds = m_params.DNSSeeds();
     Shuffle(seeds.begin(), seeds.end(), rng);
     int seeds_right_now = 0; // Number of seeds left before testing if we have enough connections
     int found = 0;
@@ -2275,7 +2275,7 @@ void CConnman::ThreadDNSAddressSeed()
             const auto addresses{LookupHost(host, nMaxIPs, true)};
             if (!addresses.empty()) {
                 for (const CNetAddr& ip : addresses) {
-                    CAddress addr = CAddress(CService(ip, Params().GetDefaultPort()), requiredServiceBits);
+                    CAddress addr = CAddress(CService(ip, m_params.GetDefaultPort()), requiredServiceBits);
                     addr.nTime = rng.rand_uniform_delay(Now<NodeSeconds>() - 3 * 24h, -4 * 24h); // use a random age between 3 and 7 days old
                     vAdd.push_back(addr);
                     found++;
@@ -2480,7 +2480,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
             }
 
             if (add_fixed_seeds_now) {
-                std::vector<CAddress> seed_addrs{ConvertSeeds(Params().FixedSeeds())};
+                std::vector<CAddress> seed_addrs{ConvertSeeds(m_params.FixedSeeds())};
                 // We will not make outgoing connections to peers that are unreachable
                 // (e.g. because of -onlynet configuration).
                 // Therefore, we do not add them to addrman in the first place.
@@ -2769,7 +2769,7 @@ std::vector<AddedNodeInfo> CConnman::GetAddedNodeInfo() const
     }
 
     for (const std::string& strAddNode : lAddresses) {
-        CService service(LookupNumeric(strAddNode, Params().GetDefaultPort(strAddNode)));
+        CService service(LookupNumeric(strAddNode, m_params.GetDefaultPort(strAddNode)));
         AddedNodeInfo addedNode{strAddNode, CService(), false, false};
         if (service.IsValid()) {
             // strAddNode is an IP:port
@@ -3075,11 +3075,12 @@ void CConnman::SetNetworkActive(bool active)
 }
 
 CConnman::CConnman(uint64_t nSeed0In, uint64_t nSeed1In, AddrMan& addrman_in,
-                   const NetGroupManager& netgroupman, bool network_active)
+                   const NetGroupManager& netgroupman, const CChainParams& params, bool network_active)
     : addrman(addrman_in)
     , m_netgroupman{netgroupman}
     , nSeed0(nSeed0In)
     , nSeed1(nSeed1In)
+    , m_params(params)
 {
     SetTryNewOutboundPeer(false);
 
diff --git a/src/net.h b/src/net.h
index 669ff68cb4b..eaa0fa32804 100644
--- a/src/net.h
+++ b/src/net.h
@@ -47,6 +47,7 @@
 
 class AddrMan;
 class BanMan;
+class CChainParams;
 class CNode;
 class CScheduler;
 struct bilingual_str;
@@ -1081,7 +1082,7 @@ public:
     }
 
     CConnman(uint64_t seed0, uint64_t seed1, AddrMan& addrman, const NetGroupManager& netgroupman,
-             bool network_active = true);
+             const CChainParams& params, bool network_active = true);
 
     ~CConnman();
 
@@ -1566,6 +1567,8 @@ private:
         std::vector<CNode*> m_nodes_copy;
     };
 
+    const CChainParams& m_params;
+
     friend struct ConnmanTestMsg;
 };
 
diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index 8c1182b5e13..6e740a4f53a 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -142,7 +142,7 @@ static void AddRandomOutboundPeer(NodeId& id, std::vector<CNode*>& vNodes, PeerM
 BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
 {
     NodeId id{0};
-    auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman);
+    auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params());
     auto peerLogic = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {});
 
     constexpr int max_outbound_full_relay = MAX_OUTBOUND_FULL_RELAY_CONNECTIONS;
@@ -242,7 +242,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
 BOOST_AUTO_TEST_CASE(block_relay_only_eviction)
 {
     NodeId id{0};
-    auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman);
+    auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params());
     auto peerLogic = PeerManager::make(*connman, *m_node.addrman, nullptr, *m_node.chainman, *m_node.mempool, {});
 
     constexpr int max_outbound_block_relay{MAX_BLOCK_RELAY_ONLY_CONNECTIONS};
@@ -305,7 +305,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
     LOCK(NetEventsInterface::g_msgproc_mutex);
 
     auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
-    auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman);
+    auto connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params());
     auto peerLogic = PeerManager::make(*connman, *m_node.addrman, banman.get(), *m_node.chainman, *m_node.mempool, {});
 
     CNetAddr tor_netaddr;
@@ -407,7 +407,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
     LOCK(NetEventsInterface::g_msgproc_mutex);
 
     auto banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
-    auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman);
+    auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params());
     auto peerLogic = PeerManager::make(*connman, *m_node.addrman, banman.get(), *m_node.chainman, *m_node.mempool, {});
 
     banman->ClearBanned();
diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp
index cdf240dc591..e46e085ee77 100644
--- a/src/test/fuzz/connman.cpp
+++ b/src/test/fuzz/connman.cpp
@@ -36,6 +36,7 @@ FUZZ_TARGET(connman, .init = initialize_connman)
                      fuzzed_data_provider.ConsumeIntegral<uint64_t>(),
                      *g_setup->m_node.addrman,
                      *g_setup->m_node.netgroupman,
+                     Params(),
                      fuzzed_data_provider.ConsumeBool()};
     CNetAddr random_netaddr;
     CNode random_node = ConsumeNode(fuzzed_data_provider);
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 331199709e9..2947bc3fcb2 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -253,7 +253,7 @@ TestingSetup::TestingSetup(
                                                /*deterministic=*/false,
                                                m_node.args->GetIntArg("-checkaddrman", 0));
     m_node.banman = std::make_unique<BanMan>(m_args.GetDataDirBase() / "banlist", nullptr, DEFAULT_MISBEHAVING_BANTIME);
-    m_node.connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); // Deterministic randomness for tests.
+    m_node.connman = std::make_unique<ConnmanTestMsg>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman, Params()); // Deterministic randomness for tests.
     PeerManager::Options peerman_opts;
     ApplyArgsManOptions(*m_node.args, peerman_opts);
     m_node.peerman = PeerManager::make(*m_node.connman, *m_node.addrman,

From 36193af47c8dcff53e59498c416b85b59e0d0f91 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Tue, 12 Sep 2023 13:42:52 +0200
Subject: [PATCH 6/7] [refactor] Remove netaddress.h from kernel headers

Move functions requiring the netaddress.h include out of
libbitcoinkernel source files.

The netaddress.h file contains many non-consensus related definitions
and should thus not be part of the libbitcoinkernel. This commit makes
netaddress.h no longer a required include for users of the
libbitcoinkernel.

This commit is part of the libbitcoinkernel project, namely its stage 1
step 3: Decouple most non-consensus headers from libbitcoinkernel.
---
 src/kernel/chainparams.h | 10 ----------
 src/net.cpp              | 14 ++++++++++++--
 src/net.h                |  3 +++
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/src/kernel/chainparams.h b/src/kernel/chainparams.h
index f1df878f60b..63837bb23ed 100644
--- a/src/kernel/chainparams.h
+++ b/src/kernel/chainparams.h
@@ -8,7 +8,6 @@
 
 #include <consensus/params.h>
 #include <kernel/messagestartchars.h>
-#include <netaddress.h>
 #include <primitives/block.h>
 #include <uint256.h>
 #include <util/chaintype.h>
@@ -89,15 +88,6 @@ public:
     const Consensus::Params& GetConsensus() const { return consensus; }
     const MessageStartChars& MessageStart() const { return pchMessageStart; }
     uint16_t GetDefaultPort() const { return nDefaultPort; }
-    uint16_t GetDefaultPort(Network net) const
-    {
-        return net == NET_I2P ? I2P_SAM31_PORT : GetDefaultPort();
-    }
-    uint16_t GetDefaultPort(const std::string& addr) const
-    {
-        CNetAddr a;
-        return a.SetSpecial(addr) ? GetDefaultPort(a.GetNetwork()) : GetDefaultPort();
-    }
 
     const CBlock& GenesisBlock() const { return genesis; }
     /** Default value for -checkmempool and -checkblockindex argument */
diff --git a/src/net.cpp b/src/net.cpp
index 129b049269e..bfa2738e450 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -470,7 +470,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
         Ticks<HoursDouble>(pszDest ? 0h : Now<NodeSeconds>() - addrConnect.nTime));
 
     // Resolve
-    const uint16_t default_port{pszDest != nullptr ? m_params.GetDefaultPort(pszDest) :
+    const uint16_t default_port{pszDest != nullptr ? GetDefaultPort(pszDest) :
                                                      m_params.GetDefaultPort()};
     if (pszDest) {
         const std::vector<CService> resolved{Lookup(pszDest, default_port, fNameLookup && !HaveNameProxy(), 256)};
@@ -2769,7 +2769,7 @@ std::vector<AddedNodeInfo> CConnman::GetAddedNodeInfo() const
     }
 
     for (const std::string& strAddNode : lAddresses) {
-        CService service(LookupNumeric(strAddNode, m_params.GetDefaultPort(strAddNode)));
+        CService service(LookupNumeric(strAddNode, GetDefaultPort(strAddNode)));
         AddedNodeInfo addedNode{strAddNode, CService(), false, false};
         if (service.IsValid()) {
             // strAddNode is an IP:port
@@ -3094,6 +3094,16 @@ NodeId CConnman::GetNewNodeId()
     return nLastNodeId.fetch_add(1, std::memory_order_relaxed);
 }
 
+uint16_t CConnman::GetDefaultPort(Network net) const
+{
+    return net == NET_I2P ? I2P_SAM31_PORT : m_params.GetDefaultPort();
+}
+
+uint16_t CConnman::GetDefaultPort(const std::string& addr) const
+{
+    CNetAddr a;
+    return a.SetSpecial(addr) ? GetDefaultPort(a.GetNetwork()) : m_params.GetDefaultPort();
+}
 
 bool CConnman::Bind(const CService& addr_, unsigned int flags, NetPermissionFlags permissions)
 {
diff --git a/src/net.h b/src/net.h
index eaa0fa32804..145b5cf666f 100644
--- a/src/net.h
+++ b/src/net.h
@@ -1358,6 +1358,9 @@ private:
     // Whether the node should be passed out in ForEach* callbacks
     static bool NodeFullyConnected(const CNode* pnode);
 
+    uint16_t GetDefaultPort(Network net) const;
+    uint16_t GetDefaultPort(const std::string& addr) const;
+
     // Network usage totals
     mutable Mutex m_total_bytes_sent_mutex;
     std::atomic<uint64_t> nTotalBytesRecv{0};

From d5067651991f3e6daf456ba13c7036ddc4545352 Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Thu, 7 Sep 2023 09:03:04 +0200
Subject: [PATCH 7/7] [refactor] Remove compat.h from kernel headers

This commit makes compat.h no longer a required include for users of the
libbitcoinkernel. Including compat.h imports a bunch of
platform-specific definitions.

This commit is part of the libbitcoinkernel project, namely its stage 1
step 3: Decouple most non-consensus headers from libbitcoinkernel.
---
 src/random.cpp    | 1 +
 src/randomenv.cpp | 1 +
 src/util/time.h   | 2 --
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/random.cpp b/src/random.cpp
index 51b8b3ad9d1..9bd8ff9f1a4 100644
--- a/src/random.cpp
+++ b/src/random.cpp
@@ -5,6 +5,7 @@
 
 #include <random.h>
 
+#include <compat/compat.h>
 #include <compat/cpuid.h>
 #include <crypto/chacha20.h>
 #include <crypto/sha256.h>
diff --git a/src/randomenv.cpp b/src/randomenv.cpp
index 581612bccf3..da81a616513 100644
--- a/src/randomenv.cpp
+++ b/src/randomenv.cpp
@@ -10,6 +10,7 @@
 #include <randomenv.h>
 
 #include <clientversion.h>
+#include <compat/compat.h>
 #include <compat/cpuid.h>
 #include <crypto/sha512.h>
 #include <support/cleanse.h>
diff --git a/src/util/time.h b/src/util/time.h
index b6aab615bac..6aa776137c1 100644
--- a/src/util/time.h
+++ b/src/util/time.h
@@ -6,8 +6,6 @@
 #ifndef BITCOIN_UTIL_TIME_H
 #define BITCOIN_UTIL_TIME_H
 
-#include <compat/compat.h>
-
 #include <chrono> // IWYU pragma: export
 #include <cstdint>
 #include <string>