Merge bitcoin/bitcoin#31163: scripted-diff: get rid of remaining "command" terminology in protocol.{h,cpp}

4120c7543e scripted-diff: get rid of remaining "command" terminology in protocol.{h,cpp} (Sebastian Falbesoner)

Pull request description:

  The confusing "command" terminology for the 12-byte field in the (v1) p2p message header was replaced with the more proper term "message type" in other modules already years ago, see eg #18533, #18937, #24078, #24141. This PR does the same for the protocol.{h,cpp} module to complete the replacements. Note that "GetCommand" is a method name also used in the `ArgsManager` (there it makes much more sense), so the scripted-diff lists for this replacement the files explicitly, rather than using `$(git grep -l ...)`.

ACKs for top commit:
  maflcko:
    review ACK 4120c7543e 🛒
  fjahr:
    Code review ACK 4120c7543e
  rkrux:
    tACK 4120c7543e

Tree-SHA512: 7b4dd30136392a145da95d2f3ba181c18c155ba6f3158e49e622d76811c6a45ef9b5c7539a979a04d8404faf18bb27f11457aa436d4e2998ece3deb2c9e59748

src/net.cpp

@@ -739,7 +739,7 @@ int V1Transport::readHeader(Span<const uint8_t> msg_bytes)
 
     // reject messages larger than MAX_SIZE or MAX_PROTOCOL_MESSAGE_LENGTH
     if (hdr.nMessageSize > MAX_SIZE || hdr.nMessageSize > MAX_PROTOCOL_MESSAGE_LENGTH) {
-        LogDebug(BCLog::NET, "Header error: Size too large (%s, %u bytes), peer=%d\n", SanitizeString(hdr.GetCommand()), hdr.nMessageSize, m_node_id);
+        LogDebug(BCLog::NET, "Header error: Size too large (%s, %u bytes), peer=%d\n", SanitizeString(hdr.GetMessageType()), hdr.nMessageSize, m_node_id);
         return -1;
     }
 
@@ -786,7 +786,7 @@ CNetMessage V1Transport::GetReceivedMessage(const std::chrono::microseconds time
     CNetMessage msg(std::move(vRecv));
 
     // store message type string, time, and sizes
-    msg.m_type = hdr.GetCommand();
+    msg.m_type = hdr.GetMessageType();
     msg.m_time = time;
     msg.m_message_size = hdr.nMessageSize;
     msg.m_raw_message_size = hdr.nMessageSize + CMessageHeader::HEADER_SIZE;
@@ -804,9 +804,9 @@ CNetMessage V1Transport::GetReceivedMessage(const std::chrono::microseconds time
                  HexStr(hdr.pchChecksum),
                  m_node_id);
         reject_message = true;
-    } else if (!hdr.IsCommandValid()) {
+    } else if (!hdr.IsMessageTypeValid()) {
         LogDebug(BCLog::NET, "Header error: Invalid message type (%s, %u bytes), peer=%d\n",
-                 SanitizeString(hdr.GetCommand()), msg.m_message_size, m_node_id);
+                 SanitizeString(hdr.GetMessageType()), msg.m_message_size, m_node_id);
         reject_message = true;
     }
 
@@ -1188,7 +1188,7 @@ bool V2Transport::ProcessReceivedPacketBytes() noexcept
     // - 12 bytes of message type
     // - payload
     static constexpr size_t MAX_CONTENTS_LEN =
-        1 + CMessageHeader::COMMAND_SIZE +
+        1 + CMessageHeader::MESSAGE_TYPE_SIZE +
         std::min<size_t>(MAX_SIZE, MAX_PROTOCOL_MESSAGE_LENGTH);
 
     if (m_recv_buffer.size() == BIP324Cipher::LENGTH_LEN) {
@@ -1403,12 +1403,12 @@ std::optional<std::string> V2Transport::GetMessageType(Span<const uint8_t>& cont
         }
     }
 
-    if (contents.size() < CMessageHeader::COMMAND_SIZE) {
+    if (contents.size() < CMessageHeader::MESSAGE_TYPE_SIZE) {
         return std::nullopt; // Long encoding needs 12 message type bytes.
     }
 
     size_t msg_type_len{0};
-    while (msg_type_len < CMessageHeader::COMMAND_SIZE && contents[msg_type_len] != 0) {
+    while (msg_type_len < CMessageHeader::MESSAGE_TYPE_SIZE && contents[msg_type_len] != 0) {
         // Verify that message type bytes before the first 0x00 are in range.
         if (contents[msg_type_len] < ' ' || contents[msg_type_len] > 0x7F) {
             return {};
@@ -1416,13 +1416,13 @@ std::optional<std::string> V2Transport::GetMessageType(Span<const uint8_t>& cont
         ++msg_type_len;
     }
     std::string ret{reinterpret_cast<const char*>(contents.data()), msg_type_len};
-    while (msg_type_len < CMessageHeader::COMMAND_SIZE) {
+    while (msg_type_len < CMessageHeader::MESSAGE_TYPE_SIZE) {
         // Verify that message type bytes after the first 0x00 are also 0x00.
         if (contents[msg_type_len] != 0) return {};
         ++msg_type_len;
     }
     // Strip message type bytes of contents.
-    contents = contents.subspan(CMessageHeader::COMMAND_SIZE);
+    contents = contents.subspan(CMessageHeader::MESSAGE_TYPE_SIZE);
     return ret;
 }
 
@@ -1474,9 +1474,9 @@ bool V2Transport::SetMessageToSend(CSerializedNetMsg& msg) noexcept
     } else {
         // Initialize with zeroes, and then write the message type string starting at offset 1.
         // This means contents[0] and the unused positions in contents[1..13] remain 0x00.
-        contents.resize(1 + CMessageHeader::COMMAND_SIZE + msg.data.size(), 0);
+        contents.resize(1 + CMessageHeader::MESSAGE_TYPE_SIZE + msg.data.size(), 0);
         std::copy(msg.m_type.begin(), msg.m_type.end(), contents.data() + 1);
-        std::copy(msg.data.begin(), msg.data.end(), contents.begin() + 1 + CMessageHeader::COMMAND_SIZE);
+        std::copy(msg.data.begin(), msg.data.end(), contents.begin() + 1 + CMessageHeader::MESSAGE_TYPE_SIZE);
     }
     // Construct ciphertext in send buffer.
     m_send_buffer.resize(contents.size() + BIP324Cipher::EXPANSION);
@@ -3940,7 +3940,7 @@ static void CaptureMessageToFile(const CAddress& addr,
 
     ser_writedata64(f, now.count());
     f << Span{msg_type};
-    for (auto i = msg_type.length(); i < CMessageHeader::COMMAND_SIZE; ++i) {
+    for (auto i = msg_type.length(); i < CMessageHeader::MESSAGE_TYPE_SIZE; ++i) {
         f << uint8_t{'\0'};
     }
     uint32_t size = data.size();

src/protocol.cpp

@@ -7,29 +7,29 @@
 
 #include <common/system.h>
 
-CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn, const char* pszCommand, unsigned int nMessageSizeIn)
+CMessageHeader::CMessageHeader(const MessageStartChars& pchMessageStartIn, const char* msg_type, unsigned int nMessageSizeIn)
     : pchMessageStart{pchMessageStartIn}
 {
-    // Copy the command name
+    // Copy the message type name
     size_t i = 0;
-    for (; i < COMMAND_SIZE && pszCommand[i] != 0; ++i) pchCommand[i] = pszCommand[i];
+    for (; i < MESSAGE_TYPE_SIZE && msg_type[i] != 0; ++i) m_msg_type[i] = msg_type[i];
-    assert(pszCommand[i] == 0); // Assert that the command name passed in is not longer than COMMAND_SIZE
+    assert(msg_type[i] == 0); // Assert that the message type name passed in is not longer than MESSAGE_TYPE_SIZE
 
     nMessageSize = nMessageSizeIn;
 }
 
-std::string CMessageHeader::GetCommand() const
+std::string CMessageHeader::GetMessageType() const
 {
-    return std::string(pchCommand, pchCommand + strnlen(pchCommand, COMMAND_SIZE));
+    return std::string(m_msg_type, m_msg_type + strnlen(m_msg_type, MESSAGE_TYPE_SIZE));
 }
 
-bool CMessageHeader::IsCommandValid() const
+bool CMessageHeader::IsMessageTypeValid() const
 {
-    // Check the command string for errors
+    // Check the message type string for errors
-    for (const char* p1 = pchCommand; p1 < pchCommand + COMMAND_SIZE; ++p1) {
+    for (const char* p1 = m_msg_type; p1 < m_msg_type + MESSAGE_TYPE_SIZE; ++p1) {
         if (*p1 == 0) {
             // Must be all zeros after the first zero
-            for (; p1 < pchCommand + COMMAND_SIZE; ++p1) {
+            for (; p1 < m_msg_type + MESSAGE_TYPE_SIZE; ++p1) {
                 if (*p1 != 0) {
                     return false;
                 }
@@ -55,7 +55,7 @@ bool operator<(const CInv& a, const CInv& b)
     return (a.type < b.type || (a.type == b.type && a.hash < b.hash));
 }
 
-std::string CInv::GetCommand() const
+std::string CInv::GetMessageType() const
 {
     std::string cmd;
     if (type & MSG_WITNESS_FLAG)
@@ -70,14 +70,14 @@ std::string CInv::GetCommand() const
     case MSG_FILTERED_BLOCK: return cmd.append(NetMsgType::MERKLEBLOCK);
     case MSG_CMPCT_BLOCK:    return cmd.append(NetMsgType::CMPCTBLOCK);
     default:
-        throw std::out_of_range(strprintf("CInv::GetCommand(): type=%d unknown type", type));
+        throw std::out_of_range(strprintf("CInv::GetMessageType(): type=%d unknown type", type));
     }
 }
 
 std::string CInv::ToString() const
 {
     try {
-        return strprintf("%s %s", GetCommand(), hash.ToString());
+        return strprintf("%s %s", GetMessageType(), hash.ToString());
     } catch(const std::out_of_range &) {
         return strprintf("0x%08x %s", type, hash.ToString());
     }

src/protocol.h

@@ -21,34 +21,34 @@
 
 /** Message header.
  * (4) message start.
- * (12) command.
+ * (12) message type.
  * (4) size.
  * (4) checksum.
  */
 class CMessageHeader
 {
 public:
-    static constexpr size_t COMMAND_SIZE = 12;
+    static constexpr size_t MESSAGE_TYPE_SIZE = 12;
     static constexpr size_t MESSAGE_SIZE_SIZE = 4;
     static constexpr size_t CHECKSUM_SIZE = 4;
-    static constexpr size_t MESSAGE_SIZE_OFFSET = std::tuple_size_v<MessageStartChars> + COMMAND_SIZE;
+    static constexpr size_t MESSAGE_SIZE_OFFSET = std::tuple_size_v<MessageStartChars> + MESSAGE_TYPE_SIZE;
     static constexpr size_t CHECKSUM_OFFSET = MESSAGE_SIZE_OFFSET + MESSAGE_SIZE_SIZE;
-    static constexpr size_t HEADER_SIZE = std::tuple_size_v<MessageStartChars> + COMMAND_SIZE + MESSAGE_SIZE_SIZE + CHECKSUM_SIZE;
+    static constexpr size_t HEADER_SIZE = std::tuple_size_v<MessageStartChars> + MESSAGE_TYPE_SIZE + MESSAGE_SIZE_SIZE + CHECKSUM_SIZE;
 
     explicit CMessageHeader() = default;
 
-    /** Construct a P2P message header from message-start characters, a command and the size of the message.
+    /** Construct a P2P message header from message-start characters, a message type and the size of the message.
-     * @note Passing in a `pszCommand` longer than COMMAND_SIZE will result in a run-time assertion error.
+     * @note Passing in a `msg_type` longer than MESSAGE_TYPE_SIZE will result in a run-time assertion error.
      */
-    CMessageHeader(const MessageStartChars& pchMessageStartIn, const char* pszCommand, unsigned int nMessageSizeIn);
+    CMessageHeader(const MessageStartChars& pchMessageStartIn, const char* msg_type, unsigned int nMessageSizeIn);
 
-    std::string GetCommand() const;
+    std::string GetMessageType() const;
-    bool IsCommandValid() const;
+    bool IsMessageTypeValid() const;
 
-    SERIALIZE_METHODS(CMessageHeader, obj) { READWRITE(obj.pchMessageStart, obj.pchCommand, obj.nMessageSize, obj.pchChecksum); }
+    SERIALIZE_METHODS(CMessageHeader, obj) { READWRITE(obj.pchMessageStart, obj.m_msg_type, obj.nMessageSize, obj.pchChecksum); }
 
     MessageStartChars pchMessageStart{};
-    char pchCommand[COMMAND_SIZE]{};
+    char m_msg_type[MESSAGE_TYPE_SIZE]{};
     uint32_t nMessageSize{std::numeric_limits<uint32_t>::max()};
     uint8_t pchChecksum[CHECKSUM_SIZE]{};
 };
@@ -500,7 +500,7 @@ public:
 
     friend bool operator<(const CInv& a, const CInv& b);
 
-    std::string GetCommand() const;
+    std::string GetMessageType() const;
     std::string ToString() const;
 
     // Single-message helper methods

src/rpc/net.cpp

@@ -1024,7 +1024,7 @@ static RPCHelpMan sendmsgtopeer()
         "This RPC is for testing only.",
         {
             {"peer_id", RPCArg::Type::NUM, RPCArg::Optional::NO, "The peer to send the message to."},
-            {"msg_type", RPCArg::Type::STR, RPCArg::Optional::NO, strprintf("The message type (maximum length %i)", CMessageHeader::COMMAND_SIZE)},
+            {"msg_type", RPCArg::Type::STR, RPCArg::Optional::NO, strprintf("The message type (maximum length %i)", CMessageHeader::MESSAGE_TYPE_SIZE)},
             {"msg", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The serialized message body to send, in hex, without a message header"},
         },
         RPCResult{RPCResult::Type::OBJ, "", "", std::vector<RPCResult>{}},
@@ -1033,8 +1033,8 @@ static RPCHelpMan sendmsgtopeer()
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue {
             const NodeId peer_id{request.params[0].getInt<int64_t>()};
             const std::string& msg_type{request.params[1].get_str()};
-            if (msg_type.size() > CMessageHeader::COMMAND_SIZE) {
+            if (msg_type.size() > CMessageHeader::MESSAGE_TYPE_SIZE) {
-                throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Error: msg_type too long, max length is %i", CMessageHeader::COMMAND_SIZE));
+                throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Error: msg_type too long, max length is %i", CMessageHeader::MESSAGE_TYPE_SIZE));
             }
             auto msg{TryParseHex<unsigned char>(request.params[2].get_str())};
             if (!msg.has_value()) {

src/test/fuzz/connman.cpp

@@ -130,7 +130,7 @@ FUZZ_TARGET(connman, .init = initialize_connman)
             },
             [&] {
                 CSerializedNetMsg serialized_net_msg;
-                serialized_net_msg.m_type = fuzzed_data_provider.ConsumeRandomLengthString(CMessageHeader::COMMAND_SIZE);
+                serialized_net_msg.m_type = fuzzed_data_provider.ConsumeRandomLengthString(CMessageHeader::MESSAGE_TYPE_SIZE);
                 serialized_net_msg.data = ConsumeRandomLengthByteVector(fuzzed_data_provider);
                 connman.PushMessage(&random_node, std::move(serialized_net_msg));
             },

src/test/fuzz/deserialize.cpp

@@ -263,7 +263,7 @@ FUZZ_TARGET(service_deserialize, .init = initialize_deserialize)
 FUZZ_TARGET_DESERIALIZE(messageheader_deserialize, {
     CMessageHeader mh;
     DeserializeFromFuzzingInput(buffer, mh);
-    (void)mh.IsCommandValid();
+    (void)mh.IsMessageTypeValid();
 })
 FUZZ_TARGET(address_deserialize, .init = initialize_deserialize)
 {

src/test/fuzz/p2p_transport_serialization.cpp

@@ -81,7 +81,7 @@ FUZZ_TARGET(p2p_transport_serialization, .init = initialize_p2p_transport_serial
             const std::chrono::microseconds m_time{std::numeric_limits<int64_t>::max()};
             bool reject_message{false};
             CNetMessage msg = recv_transport.GetReceivedMessage(m_time, reject_message);
-            assert(msg.m_type.size() <= CMessageHeader::COMMAND_SIZE);
+            assert(msg.m_type.size() <= CMessageHeader::MESSAGE_TYPE_SIZE);
             assert(msg.m_raw_message_size <= mutable_msg_bytes.size());
             assert(msg.m_raw_message_size == CMessageHeader::HEADER_SIZE + msg.m_message_size);
             assert(msg.m_time == m_time);
@@ -139,9 +139,9 @@ void SimulationTest(Transport& initiator, Transport& responder, R& rng, FuzzedDa
             // If v is 0xFF, construct a valid (but possibly unknown) message type from the fuzz
             // data.
             std::string ret;
-            while (ret.size() < CMessageHeader::COMMAND_SIZE) {
+            while (ret.size() < CMessageHeader::MESSAGE_TYPE_SIZE) {
                 char c = provider.ConsumeIntegral<char>();
-                // Match the allowed characters in CMessageHeader::IsCommandValid(). Any other
+                // Match the allowed characters in CMessageHeader::IsMessageTypeValid(). Any other
                 // character is interpreted as end.
                 if (c < ' ' || c > 0x7E) break;
                 ret += c;

src/test/fuzz/process_message.cpp

@@ -61,7 +61,7 @@ FUZZ_TARGET(process_message, .init = initialize_process_message)
 
     LOCK(NetEventsInterface::g_msgproc_mutex);
 
-    const std::string random_message_type{fuzzed_data_provider.ConsumeBytesAsString(CMessageHeader::COMMAND_SIZE).c_str()};
+    const std::string random_message_type{fuzzed_data_provider.ConsumeBytesAsString(CMessageHeader::MESSAGE_TYPE_SIZE).c_str()};
     if (!LIMIT_TO_MESSAGE_TYPE.empty() && random_message_type != LIMIT_TO_MESSAGE_TYPE) {
         return;
     }

src/test/fuzz/process_messages.cpp

@@ -64,7 +64,7 @@ FUZZ_TARGET(process_messages, .init = initialize_process_messages)
 
     LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 30)
     {
-        const std::string random_message_type{fuzzed_data_provider.ConsumeBytesAsString(CMessageHeader::COMMAND_SIZE).c_str()};
+        const std::string random_message_type{fuzzed_data_provider.ConsumeBytesAsString(CMessageHeader::MESSAGE_TYPE_SIZE).c_str()};
 
         const auto mock_time = ConsumeTime(fuzzed_data_provider);
         SetMockTime(mock_time);

src/test/fuzz/protocol.cpp

@@ -20,7 +20,7 @@ FUZZ_TARGET(protocol)
         return;
     }
     try {
-        (void)inv->GetCommand();
+        (void)inv->GetMessageType();
     } catch (const std::out_of_range&) {
     }
     (void)inv->ToString();

src/test/net_tests.cpp

@@ -1281,7 +1281,7 @@ public:
     void ReceiveMessage(const std::string& m_type, Span<const uint8_t> payload)
     {
         auto ret = ReceivePacket();
-        BOOST_REQUIRE(ret.size() == payload.size() + 1 + CMessageHeader::COMMAND_SIZE);
+        BOOST_REQUIRE(ret.size() == payload.size() + 1 + CMessageHeader::MESSAGE_TYPE_SIZE);
         BOOST_CHECK(ret[0] == 0);
         for (unsigned i = 0; i < 12; ++i) {
             if (i < m_type.size()) {
@@ -1290,7 +1290,7 @@ public:
                 BOOST_CHECK(ret[1 + i] == 0);
             }
         }
-        BOOST_CHECK(std::ranges::equal(Span{ret}.subspan(1 + CMessageHeader::COMMAND_SIZE), payload));
+        BOOST_CHECK(std::ranges::equal(Span{ret}.subspan(1 + CMessageHeader::MESSAGE_TYPE_SIZE), payload));
     }
 
     /** Schedule an encrypted packet with specified message type and payload to be sent to
@@ -1298,9 +1298,9 @@ public:
     void SendMessage(std::string mtype, Span<const uint8_t> payload)
     {
         // Construct contents consisting of 0x00 + 12-byte message type + payload.
-        std::vector<uint8_t> contents(1 + CMessageHeader::COMMAND_SIZE + payload.size());
+        std::vector<uint8_t> contents(1 + CMessageHeader::MESSAGE_TYPE_SIZE + payload.size());
         std::copy(mtype.begin(), mtype.end(), reinterpret_cast<char*>(contents.data() + 1));
-        std::copy(payload.begin(), payload.end(), contents.begin() + 1 + CMessageHeader::COMMAND_SIZE);
+        std::copy(payload.begin(), payload.end(), contents.begin() + 1 + CMessageHeader::MESSAGE_TYPE_SIZE);
         // Send a packet with that as contents.
         SendPacket(contents);
     }
@@ -1459,7 +1459,7 @@ BOOST_AUTO_TEST_CASE(v2transport_test)
         auto msg_data_2 = m_rng.randbytes<uint8_t>(m_rng.randrange(1000));
         tester.SendMessage(uint8_t(13), msg_data_2); // headers short id
         // Send invalidly-encoded message
-        tester.SendMessage(std::string("blocktxn\x00\x00\x00a", CMessageHeader::COMMAND_SIZE), {});
+        tester.SendMessage(std::string("blocktxn\x00\x00\x00a", CMessageHeader::MESSAGE_TYPE_SIZE), {});
         tester.SendMessage("foobar", {}); // test receiving unknown message type
         tester.AddMessage("barfoo", {}); // test sending unknown message type
         ret = tester.Interact();