Merge bitcoin/bitcoin#29421: net: make the list of known message types a compile time constant

b3efb48673 protocol: make message types constexpr (Vasil Dimov)
2fa9de06c2 net: make the list of known message types a compile time constant (Vasil Dimov)

Pull request description:

  Turn the `std::vector` to `std::array` because it is cheaper and allows us to have the number of the messages as a compile time constant: `ALL_NET_MESSAGE_TYPES.size()` which can be used in future code to build other `std::array`s with that size.

  ---

  This change is part of https://github.com/bitcoin/bitcoin/pull/29418 but it makes sense on its own and would be good to have it, regardless of the fate of https://github.com/bitcoin/bitcoin/pull/29418. Also, if this is merged, that would reduce the size of https://github.com/bitcoin/bitcoin/pull/29418, thus the current standalone PR.

ACKs for top commit:
  achow101:
    ACK b3efb48673
  jonatack:
    ACK b3efb48673
  maflcko:
    utACK b3efb48673 🎊
  willcl-ark:
    ACK b3efb48673

Tree-SHA512: 6d3860c138c64514ebab13d97ea67893e2d346dfac30a48c3d9bc769a1970407375ea4170afdb522411ced306a14a9af4eede99e964d1fb1ea3efff5d5eb57af

src/net.cpp

@@ -3736,8 +3736,9 @@ CNode::CNode(NodeId idIn,
 {
     if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND);
 
-    for (const std::string &msg : getAllNetMessageTypes())
+    for (const auto& msg : ALL_NET_MESSAGE_TYPES) {
         mapRecvBytesPerMsgType[msg] = 0;
+    }
     mapRecvBytesPerMsgType[NET_MESSAGE_TYPE_OTHER] = 0;
 
     if (fLogIPs) {

src/protocol.cpp

@@ -7,87 +7,6 @@
 
 #include <common/system.h>
 
-#include <atomic>
-
-namespace NetMsgType {
-const char* VERSION = "version";
-const char* VERACK = "verack";
-const char* ADDR = "addr";
-const char* ADDRV2 = "addrv2";
-const char* SENDADDRV2 = "sendaddrv2";
-const char* INV = "inv";
-const char* GETDATA = "getdata";
-const char* MERKLEBLOCK = "merkleblock";
-const char* GETBLOCKS = "getblocks";
-const char* GETHEADERS = "getheaders";
-const char* TX = "tx";
-const char* HEADERS = "headers";
-const char* BLOCK = "block";
-const char* GETADDR = "getaddr";
-const char* MEMPOOL = "mempool";
-const char* PING = "ping";
-const char* PONG = "pong";
-const char* NOTFOUND = "notfound";
-const char* FILTERLOAD = "filterload";
-const char* FILTERADD = "filteradd";
-const char* FILTERCLEAR = "filterclear";
-const char* SENDHEADERS = "sendheaders";
-const char* FEEFILTER = "feefilter";
-const char* SENDCMPCT = "sendcmpct";
-const char* CMPCTBLOCK = "cmpctblock";
-const char* GETBLOCKTXN = "getblocktxn";
-const char* BLOCKTXN = "blocktxn";
-const char* GETCFILTERS = "getcfilters";
-const char* CFILTER = "cfilter";
-const char* GETCFHEADERS = "getcfheaders";
-const char* CFHEADERS = "cfheaders";
-const char* GETCFCHECKPT = "getcfcheckpt";
-const char* CFCHECKPT = "cfcheckpt";
-const char* WTXIDRELAY = "wtxidrelay";
-const char* SENDTXRCNCL = "sendtxrcncl";
-} // namespace NetMsgType
-
-/** All known message types. Keep this in the same order as the list of
- * messages above and in protocol.h.
- */
-const static std::vector<std::string> g_all_net_message_types{
-    NetMsgType::VERSION,
-    NetMsgType::VERACK,
-    NetMsgType::ADDR,
-    NetMsgType::ADDRV2,
-    NetMsgType::SENDADDRV2,
-    NetMsgType::INV,
-    NetMsgType::GETDATA,
-    NetMsgType::MERKLEBLOCK,
-    NetMsgType::GETBLOCKS,
-    NetMsgType::GETHEADERS,
-    NetMsgType::TX,
-    NetMsgType::HEADERS,
-    NetMsgType::BLOCK,
-    NetMsgType::GETADDR,
-    NetMsgType::MEMPOOL,
-    NetMsgType::PING,
-    NetMsgType::PONG,
-    NetMsgType::NOTFOUND,
-    NetMsgType::FILTERLOAD,
-    NetMsgType::FILTERADD,
-    NetMsgType::FILTERCLEAR,
-    NetMsgType::SENDHEADERS,
-    NetMsgType::FEEFILTER,
-    NetMsgType::SENDCMPCT,
-    NetMsgType::CMPCTBLOCK,
-    NetMsgType::GETBLOCKTXN,
-    NetMsgType::BLOCKTXN,
-    NetMsgType::GETCFILTERS,
-    NetMsgType::CFILTER,
-    NetMsgType::GETCFHEADERS,
-    NetMsgType::CFHEADERS,
-    NetMsgType::GETCFCHECKPT,
-    NetMsgType::CFCHECKPT,
-    NetMsgType::WTXIDRELAY,
-    NetMsgType::SENDTXRCNCL,
-};
-
 CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn, const char* pszCommand, unsigned int nMessageSizeIn)
     : pchMessageStart{pchMessageStartIn}
 {
@@ -164,11 +83,6 @@ std::string CInv::ToString() const
     }
 }
 
-const std::vector<std::string> &getAllNetMessageTypes()
-{
-    return g_all_net_message_types;
-}
-
 /**
  * Convert a service flag (NODE_*) to a human readable string.
  * It supports unknown service flags which will be returned as "UNKNOWN[...]".

src/protocol.h

@@ -55,106 +55,105 @@ public:
 
 /**
  * Bitcoin protocol message types. When adding new message types, don't forget
- * to update allNetMessageTypes in protocol.cpp.
+ * to update ALL_NET_MESSAGE_TYPES below.
  */
 namespace NetMsgType {
-
 /**
  * The version message provides information about the transmitting node to the
  * receiving node at the beginning of a connection.
  */
-extern const char* VERSION;
+inline constexpr const char* VERSION{"version"};
 /**
  * The verack message acknowledges a previously-received version message,
  * informing the connecting node that it can begin to send other messages.
  */
-extern const char* VERACK;
+inline constexpr const char* VERACK{"verack"};
 /**
  * The addr (IP address) message relays connection information for peers on the
  * network.
  */
-extern const char* ADDR;
+inline constexpr const char* ADDR{"addr"};
 /**
  * The addrv2 message relays connection information for peers on the network just
  * like the addr message, but is extended to allow gossiping of longer node
  * addresses (see BIP155).
  */
-extern const char *ADDRV2;
+inline constexpr const char* ADDRV2{"addrv2"};
 /**
  * The sendaddrv2 message signals support for receiving ADDRV2 messages (BIP155).
  * It also implies that its sender can encode as ADDRV2 and would send ADDRV2
  * instead of ADDR to a peer that has signaled ADDRV2 support by sending SENDADDRV2.
  */
-extern const char *SENDADDRV2;
+inline constexpr const char* SENDADDRV2{"sendaddrv2"};
 /**
  * The inv message (inventory message) transmits one or more inventories of
  * objects known to the transmitting peer.
  */
-extern const char* INV;
+inline constexpr const char* INV{"inv"};
 /**
  * The getdata message requests one or more data objects from another node.
  */
-extern const char* GETDATA;
+inline constexpr const char* GETDATA{"getdata"};
 /**
  * The merkleblock message is a reply to a getdata message which requested a
  * block using the inventory type MSG_MERKLEBLOCK.
  * @since protocol version 70001 as described by BIP37.
  */
-extern const char* MERKLEBLOCK;
+inline constexpr const char* MERKLEBLOCK{"merkleblock"};
 /**
  * The getblocks message requests an inv message that provides block header
  * hashes starting from a particular point in the block chain.
  */
-extern const char* GETBLOCKS;
+inline constexpr const char* GETBLOCKS{"getblocks"};
 /**
  * The getheaders message requests a headers message that provides block
  * headers starting from a particular point in the block chain.
  * @since protocol version 31800.
  */
-extern const char* GETHEADERS;
+inline constexpr const char* GETHEADERS{"getheaders"};
 /**
  * The tx message transmits a single transaction.
  */
-extern const char* TX;
+inline constexpr const char* TX{"tx"};
 /**
  * The headers message sends one or more block headers to a node which
  * previously requested certain headers with a getheaders message.
  * @since protocol version 31800.
  */
-extern const char* HEADERS;
+inline constexpr const char* HEADERS{"headers"};
 /**
  * The block message transmits a single serialized block.
  */
-extern const char* BLOCK;
+inline constexpr const char* BLOCK{"block"};
 /**
  * The getaddr message requests an addr message from the receiving node,
  * preferably one with lots of IP addresses of other receiving nodes.
  */
-extern const char* GETADDR;
+inline constexpr const char* GETADDR{"getaddr"};
 /**
  * The mempool message requests the TXIDs of transactions that the receiving
  * node has verified as valid but which have not yet appeared in a block.
  * @since protocol version 60002 as described by BIP35.
  *   Only available with service bit NODE_BLOOM, see also BIP111.
  */
-extern const char* MEMPOOL;
+inline constexpr const char* MEMPOOL{"mempool"};
 /**
  * The ping message is sent periodically to help confirm that the receiving
  * peer is still connected.
  */
-extern const char* PING;
+inline constexpr const char* PING{"ping"};
 /**
  * The pong message replies to a ping message, proving to the pinging node that
  * the ponging node is still alive.
  * @since protocol version 60001 as described by BIP31.
  */
-extern const char* PONG;
+inline constexpr const char* PONG{"pong"};
 /**
  * The notfound message is a reply to a getdata message which requested an
  * object the receiving node does not have available for relay.
  * @since protocol version 70001.
  */
-extern const char* NOTFOUND;
+inline constexpr const char* NOTFOUND{"notfound"};
 /**
  * The filterload message tells the receiving peer to filter all relayed
  * transactions and requested merkle blocks through the provided filter.
@@ -162,7 +161,7 @@ extern const char* NOTFOUND;
  *   Only available with service bit NODE_BLOOM since protocol version
  *   70011 as described by BIP111.
  */
-extern const char* FILTERLOAD;
+inline constexpr const char* FILTERLOAD{"filterload"};
 /**
  * The filteradd message tells the receiving peer to add a single element to a
  * previously-set bloom filter, such as a new public key.
@@ -170,7 +169,7 @@ extern const char* FILTERLOAD;
  *   Only available with service bit NODE_BLOOM since protocol version
  *   70011 as described by BIP111.
  */
-extern const char* FILTERADD;
+inline constexpr const char* FILTERADD{"filteradd"};
 /**
  * The filterclear message tells the receiving peer to remove a previously-set
  * bloom filter.
@@ -178,19 +177,19 @@ extern const char* FILTERADD;
  *   Only available with service bit NODE_BLOOM since protocol version
  *   70011 as described by BIP111.
  */
-extern const char* FILTERCLEAR;
+inline constexpr const char* FILTERCLEAR{"filterclear"};
 /**
  * Indicates that a node prefers to receive new block announcements via a
  * "headers" message rather than an "inv".
  * @since protocol version 70012 as described by BIP130.
  */
-extern const char* SENDHEADERS;
+inline constexpr const char* SENDHEADERS{"sendheaders"};
 /**
  * The feefilter message tells the receiving peer not to inv us any txs
  * which do not meet the specified min fee rate.
  * @since protocol version 70013 as described by BIP133
  */
-extern const char* FEEFILTER;
+inline constexpr const char* FEEFILTER{"feefilter"};
 /**
  * Contains a 1-byte bool and 8-byte LE version number.
  * Indicates that a node is willing to provide blocks via "cmpctblock" messages.
@@ -198,36 +197,36 @@ extern const char* FEEFILTER;
  * "cmpctblock" message rather than an "inv", depending on message contents.
  * @since protocol version 70014 as described by BIP 152
  */
-extern const char* SENDCMPCT;
+inline constexpr const char* SENDCMPCT{"sendcmpct"};
 /**
  * Contains a CBlockHeaderAndShortTxIDs object - providing a header and
  * list of "short txids".
  * @since protocol version 70014 as described by BIP 152
  */
-extern const char* CMPCTBLOCK;
+inline constexpr const char* CMPCTBLOCK{"cmpctblock"};
 /**
  * Contains a BlockTransactionsRequest
  * Peer should respond with "blocktxn" message.
  * @since protocol version 70014 as described by BIP 152
  */
-extern const char* GETBLOCKTXN;
+inline constexpr const char* GETBLOCKTXN{"getblocktxn"};
 /**
  * Contains a BlockTransactions.
  * Sent in response to a "getblocktxn" message.
  * @since protocol version 70014 as described by BIP 152
  */
-extern const char* BLOCKTXN;
+inline constexpr const char* BLOCKTXN{"blocktxn"};
 /**
  * getcfilters requests compact filters for a range of blocks.
  * Only available with service bit NODE_COMPACT_FILTERS as described by
  * BIP 157 & 158.
  */
-extern const char* GETCFILTERS;
+inline constexpr const char* GETCFILTERS{"getcfilters"};
 /**
  * cfilter is a response to a getcfilters request containing a single compact
  * filter.
  */
-extern const char* CFILTER;
+inline constexpr const char* CFILTER{"cfilter"};
 /**
  * getcfheaders requests a compact filter header and the filter hashes for a
  * range of blocks, which can then be used to reconstruct the filter headers
@@ -235,40 +234,76 @@ extern const char* CFILTER;
  * Only available with service bit NODE_COMPACT_FILTERS as described by
  * BIP 157 & 158.
  */
-extern const char* GETCFHEADERS;
+inline constexpr const char* GETCFHEADERS{"getcfheaders"};
 /**
  * cfheaders is a response to a getcfheaders request containing a filter header
  * and a vector of filter hashes for each subsequent block in the requested range.
  */
-extern const char* CFHEADERS;
+inline constexpr const char* CFHEADERS{"cfheaders"};
 /**
  * getcfcheckpt requests evenly spaced compact filter headers, enabling
  * parallelized download and validation of the headers between them.
  * Only available with service bit NODE_COMPACT_FILTERS as described by
  * BIP 157 & 158.
  */
-extern const char* GETCFCHECKPT;
+inline constexpr const char* GETCFCHECKPT{"getcfcheckpt"};
 /**
  * cfcheckpt is a response to a getcfcheckpt request containing a vector of
  * evenly spaced filter headers for blocks on the requested chain.
  */
-extern const char* CFCHECKPT;
+inline constexpr const char* CFCHECKPT{"cfcheckpt"};
 /**
  * Indicates that a node prefers to relay transactions via wtxid, rather than
  * txid.
  * @since protocol version 70016 as described by BIP 339.
  */
-extern const char* WTXIDRELAY;
+inline constexpr const char* WTXIDRELAY{"wtxidrelay"};
 /**
  * Contains a 4-byte version number and an 8-byte salt.
  * The salt is used to compute short txids needed for efficient
  * txreconciliation, as described by BIP 330.
  */
-extern const char* SENDTXRCNCL;
+inline constexpr const char* SENDTXRCNCL{"sendtxrcncl"};
 }; // namespace NetMsgType
 
-/* Get a vector of all valid message types (see above) */
+/** All known message types (see above). Keep this in the same order as the list of messages above. */
-const std::vector<std::string>& getAllNetMessageTypes();
+inline const std::array ALL_NET_MESSAGE_TYPES{std::to_array<std::string>({
+    NetMsgType::VERSION,
+    NetMsgType::VERACK,
+    NetMsgType::ADDR,
+    NetMsgType::ADDRV2,
+    NetMsgType::SENDADDRV2,
+    NetMsgType::INV,
+    NetMsgType::GETDATA,
+    NetMsgType::MERKLEBLOCK,
+    NetMsgType::GETBLOCKS,
+    NetMsgType::GETHEADERS,
+    NetMsgType::TX,
+    NetMsgType::HEADERS,
+    NetMsgType::BLOCK,
+    NetMsgType::GETADDR,
+    NetMsgType::MEMPOOL,
+    NetMsgType::PING,
+    NetMsgType::PONG,
+    NetMsgType::NOTFOUND,
+    NetMsgType::FILTERLOAD,
+    NetMsgType::FILTERADD,
+    NetMsgType::FILTERCLEAR,
+    NetMsgType::SENDHEADERS,
+    NetMsgType::FEEFILTER,
+    NetMsgType::SENDCMPCT,
+    NetMsgType::CMPCTBLOCK,
+    NetMsgType::GETBLOCKTXN,
+    NetMsgType::BLOCKTXN,
+    NetMsgType::GETCFILTERS,
+    NetMsgType::CFILTER,
+    NetMsgType::GETCFHEADERS,
+    NetMsgType::CFHEADERS,
+    NetMsgType::GETCFCHECKPT,
+    NetMsgType::CFCHECKPT,
+    NetMsgType::WTXIDRELAY,
+    NetMsgType::SENDTXRCNCL,
+})};
 
 /** nServices flags */
 enum ServiceFlags : uint64_t {

src/test/fuzz/p2p_transport_serialization.cpp

@@ -21,13 +21,12 @@
 
 namespace {
 
-std::vector<std::string> g_all_messages;
+auto g_all_messages = ALL_NET_MESSAGE_TYPES;
 
 void initialize_p2p_transport_serialization()
 {
     static ECC_Context ecc_context{};
     SelectParams(ChainType::REGTEST);
-    g_all_messages = getAllNetMessageTypes();
     std::sort(g_all_messages.begin(), g_all_messages.end());
 }
 

src/test/fuzz/process_message.cpp

@@ -37,7 +37,7 @@ void initialize_process_message()
 {
     if (const auto val{std::getenv("LIMIT_TO_MESSAGE_TYPE")}) {
         LIMIT_TO_MESSAGE_TYPE = val;
-        Assert(std::count(getAllNetMessageTypes().begin(), getAllNetMessageTypes().end(), LIMIT_TO_MESSAGE_TYPE)); // Unknown message type passed
+        Assert(std::count(ALL_NET_MESSAGE_TYPES.begin(), ALL_NET_MESSAGE_TYPES.end(), LIMIT_TO_MESSAGE_TYPE)); // Unknown message type passed
     }
 
     static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(

test/fuzz/test_runner.py

@@ -215,12 +215,12 @@ def transform_process_message_target(targets, src_dir):
     p2p_msg_target = "process_message"
     if (p2p_msg_target, {}) in targets:
         lines = subprocess.run(
-            ["git", "grep", "--function-context", "g_all_net_message_types{", src_dir / "src" / "protocol.cpp"],
+            ["git", "grep", "--function-context", "ALL_NET_MESSAGE_TYPES{", src_dir / "src" / "protocol.h"],
             check=True,
             stdout=subprocess.PIPE,
             text=True,
         ).stdout.splitlines()
-        lines = [l.split("::", 1)[1].split(",")[0].lower() for l in lines if l.startswith("src/protocol.cpp-    NetMsgType::")]
+        lines = [l.split("::", 1)[1].split(",")[0].lower() for l in lines if l.startswith("src/protocol.h-    NetMsgType::")]
         assert len(lines)
         targets += [(p2p_msg_target, {"LIMIT_TO_MESSAGE_TYPE": m}) for m in lines]
     return targets