Merge #7082: feat: add protocol version-based v2 p2p short ID negotiation

ddeca21 refactor: use message type as key in V2_NEW_SHORT_ID_VERSIONS (UdjinM6)
14a085c docs: add release notes for v2 short ID version negotiation (UdjinM6)
2373ab3 test: add unit tests for v2 short ID version negotiation (UdjinM6)
11f9a41 feat: add protocol version-based v2 p2p short ID negotiation (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  Implement backwards-compatible version negotiation for BIP324 v2 P2P short message IDs, allowing new short IDs to be added incrementally without breaking compatibility with older peers.

  ## What was done?
  - Implement backwards-compatible version negotiation for BIP324 v2 P2P short message IDs
  - Bump `PROTOCOL_VERSION` to 70240 and add `platformban` message to the list of short message IDs for nodes with protocol version 70240+
  - Add unit tests
  - Add release notes

  ## How Has This Been Tested?
  Run tests

  ## Breaking Changes
  None

  Backwards Compatibility:
  - Old nodes (v70238) receive PLATFORMBAN with long encoding (13 bytes)
  - New nodes (v70240) use short encoding (1 byte) when both support it
  - Automatic per-connection negotiation based on protocol version
  - No network-wide activation needed
  - MIN_MASTERNODE_PROTO_VERSION unchanged (70238) - masternodes can
    upgrade gradually since the version negotiation handles mixed versions

  ## Checklist:
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK ddeca21
  kwvg:
    utACK ddeca21

Tree-SHA512: 9e55c0261914cfed3cdcf10110915ce66884cc2ab1a6bd07af6fbc9339240e82bbd77ce6542811e104901fbd61ec18df957ae42dec723db4cecb11c90efa5ac2

Author: PastaPastaPasta

doc/release-notes-7082.md

@@ -0,0 +1,7 @@
+# v2 P2P Short ID Version Negotiation
+
+The network protocol version has been updated to `70240` (`PLATFORMBAN_V2_SHORT_ID_VERSION`).
+
+The `PLATFORMBAN` message has been added to the v2 P2P short ID mapping (short ID 168). When communicating with peers supporting version 70240+, this message uses 1-byte encoding instead of 13-byte encoding, reducing bandwidth.
+
+The v2 transport layer now automatically negotiates short ID support per connection. Compatible peers use compact encoding, while older v2 peers automatically fall back to long encoding. This enables gradual rollout of new short IDs without breaking backwards compatibility.

src/net.cpp

@@ -931,7 +931,7 @@ const std::array<std::string, 33> V2_BITCOIN_IDS = {
  * by a protocol upgrade, the old message is no longer supported by the client
  * and a new slot wasn't already allotted for the message.
  */
-const std::array<std::string, 40> V2_DASH_IDS = {
+const std::array<std::string, 41> V2_DASH_IDS = {
     NetMsgType::SPORK,
     NetMsgType::GETSPORKS,
     NetMsgType::SENDDSQUEUE,
@@ -971,9 +971,39 @@ const std::array<std::string, 40> V2_DASH_IDS = {
     NetMsgType::SENDHEADERS2,
     NetMsgType::HEADERS2,
     NetMsgType::GETQUORUMROTATIONINFO,
-    NetMsgType::QUORUMROTATIONINFO
+    NetMsgType::QUORUMROTATIONINFO,
+    NetMsgType::PLATFORMBAN
 };
 
+ /** Explicit version requirements for Dash v2 short IDs added after baseline.
+ *
+ * Maps message type to the minimum protocol version required to use its short ID.
+ * Only contains messages that were added AFTER the initial v2 implementation.
+ * Messages not in this map are baseline messages available at BIP324_DASH_BASELINE_VERSION.
+ *
+ * IMPORTANT:
+ * - Never remove entries (historical compatibility)
+ * - Add new messages as they are introduced
+ */
+static const std::map<std::string, int> V2_NEW_SHORT_ID_VERSIONS = {
+    {NetMsgType::PLATFORMBAN, PLATFORMBAN_V2_SHORT_ID_VERSION},
+};
+
+/** Get the minimum protocol version required for a message's short ID.
+ *
+ * @param message_type The message type to check
+ * @return Minimum protocol version required
+ */
+int GetMessageMinVersion(const std::string& message_type)
+{
+    auto it = V2_NEW_SHORT_ID_VERSIONS.find(message_type);
+    if (it != V2_NEW_SHORT_ID_VERSIONS.end()) {
+        return it->second;
+    }
+    // Not in map means it's a baseline message
+    return BIP324_DASH_BASELINE_VERSION;
+}
+
 /** A complete set of short IDs
  *
  * Bitcoin takes up short IDs up to 128 (lower half) while Dash can take
@@ -1551,11 +1581,14 @@ bool V2Transport::SetMessageToSend(CSerializedNetMsg& msg) noexcept
     // Construct contents (encoding message type + payload).
     std::vector<uint8_t> contents;
     auto short_message_id = V2_MESSAGE_MAP(msg.m_type);
-    if (short_message_id) {
+
+    if (short_message_id.has_value() && m_peer_version >= GetMessageMinVersion(msg.m_type)) {
+        // Use short encoding (1 byte)
         contents.resize(1 + msg.data.size());
         contents[0] = *short_message_id;
         std::copy(msg.data.begin(), msg.data.end(), contents.begin() + 1);
     } else {
+        // Use long encoding (13 bytes for message type)
         // 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);
@@ -1571,6 +1604,13 @@ bool V2Transport::SetMessageToSend(CSerializedNetMsg& msg) noexcept
     return true;
 }
 
+void V2Transport::SetPeerVersion(int version) noexcept
+{
+    AssertLockNotHeld(m_send_mutex);
+    LOCK(m_send_mutex);
+    m_peer_version = version;
+}
+
 Transport::BytesToSend V2Transport::GetBytesToSend(bool have_next_message) const noexcept
 {
     AssertLockNotHeld(m_send_mutex);

src/net.h

@@ -389,6 +389,13 @@ class Transport {
     /** Return the memory usage of this transport attributable to buffered data to send. */
     virtual size_t GetSendMemoryUsage() const noexcept = 0;
 
+    /** Set the peer's protocol version (used for v2 short ID negotiation).
+     *
+     * This is a no-op for V1 transport. V2 transport uses it to determine which
+     * short IDs are supported by the peer.
+     */
+    virtual void SetPeerVersion(int version) noexcept {}
+
     // 3. Miscellaneous functions.
 
     /** Whether upon disconnections, a reconnect with V1 is warranted. */
@@ -645,6 +652,8 @@ class V2Transport final : public Transport
     SendState m_send_state GUARDED_BY(m_send_mutex);
     /** Whether we've sent at least 24 bytes (which would trigger disconnect for V1 peers). */
     bool m_sent_v1_header_worth GUARDED_BY(m_send_mutex) {false};
+    /** Peer's protocol version for encoding decisions (e.g., v2 short ID negotiation). */
+    int m_peer_version GUARDED_BY(m_send_mutex) {INIT_PROTO_VERSION};
 
     /** Change the receive state. */
     void SetReceiveState(RecvState recv_state) noexcept EXCLUSIVE_LOCKS_REQUIRED(m_recv_mutex);
@@ -690,6 +699,7 @@ class V2Transport final : public Transport
     BytesToSend GetBytesToSend(bool have_next_message) const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
     void MarkBytesSent(size_t bytes_sent) noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
     size_t GetSendMemoryUsage() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
+    void SetPeerVersion(int version) noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
 
     // Miscellaneous functions.
     bool ShouldReconnectV1() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_recv_mutex, !m_send_mutex);
@@ -1006,6 +1016,8 @@ class CNode
     {
         Assume(m_greatest_common_version == INIT_PROTO_VERSION);
         m_greatest_common_version = greatest_common_version;
+        // Also update transport's peer version for v2 short ID negotiation
+        m_transport->SetPeerVersion(greatest_common_version);
     }
     int GetCommonVersion() const
     {

src/test/net_tests.cpp

@@ -1054,7 +1054,11 @@ class V2TransportTester
     V2TransportTester(bool test_initiator) :
         m_transport(0, test_initiator, SER_NETWORK, INIT_PROTO_VERSION),
         m_cipher{GenerateRandomTestKey(), MakeByteSpan(InsecureRand256())},
-        m_test_initiator(test_initiator) {}
+        m_test_initiator(test_initiator)
+    {
+        // Set peer version for v2 short ID negotiation
+        m_transport.SetPeerVersion(PROTOCOL_VERSION);
+    }
 
     /** Data type returned by Interact:
      *
@@ -1121,6 +1125,9 @@ class V2TransportTester
     /** Expose the cipher. */
     BIP324Cipher& GetCipher() { return m_cipher; }
 
+    /** Expose the transport. */
+    V2Transport& GetTransport() { return m_transport; }
+
     /** Schedule bytes to be sent to the transport. */
     void Send(Span<const uint8_t> data)
     {
@@ -1575,4 +1582,84 @@ BOOST_AUTO_TEST_CASE(v2transport_test)
     }
 }
 
+BOOST_AUTO_TEST_CASE(v2_short_id_version_negotiation)
+{
+    // Test that v2 short ID encoding respects peer protocol version.
+    // This ensures backwards compatibility when adding new short IDs.
+
+    // Test 1: Baseline messages (short IDs 0-167) work with any v2-capable peer
+    {
+        V2TransportTester tester(true);
+        tester.GetTransport().SetPeerVersion(BIP324_DASH_BASELINE_VERSION); // v70235
+
+        auto ret = tester.Interact();
+        BOOST_REQUIRE(ret && ret->empty());
+        tester.SendKey();
+        tester.SendGarbage();
+        tester.ReceiveKey();
+        tester.SendGarbageTerm();
+        tester.SendVersion();
+        ret = tester.Interact();
+        BOOST_REQUIRE(ret && ret->empty());
+        tester.ReceiveGarbage();
+        tester.ReceiveVersion();
+
+        // SPORK (short ID 128) should use short encoding
+        auto msg_data = g_insecure_rand_ctx.randbytes<uint8_t>(100);
+        tester.AddMessage("spork", msg_data);
+        ret = tester.Interact();
+        BOOST_REQUIRE(ret && ret->empty());
+        tester.ReceiveMessage(uint8_t(128), msg_data);
+    }
+
+    // Test 2: New short IDs (168+) require version negotiation
+    {
+        // Old peer (v70238) - doesn't know about PLATFORMBAN short ID 168
+        V2TransportTester tester_old(true);
+        tester_old.GetTransport().SetPeerVersion(70238);
+
+        auto ret = tester_old.Interact();
+        BOOST_REQUIRE(ret && ret->empty());
+        tester_old.SendKey();
+        tester_old.SendGarbage();
+        tester_old.ReceiveKey();
+        tester_old.SendGarbageTerm();
+        tester_old.SendVersion();
+        ret = tester_old.Interact();
+        BOOST_REQUIRE(ret && ret->empty());
+        tester_old.ReceiveGarbage();
+        tester_old.ReceiveVersion();
+
+        // Old peer gets long encoding for PLATFORMBAN
+        auto msg_data_old = g_insecure_rand_ctx.randbytes<uint8_t>(100);
+        tester_old.AddMessage("platformban", msg_data_old);
+        ret = tester_old.Interact();
+        BOOST_REQUIRE(ret && ret->empty());
+        tester_old.ReceiveMessage("platformban", msg_data_old); // long encoding
+
+        // New peer (v70240) - knows about PLATFORMBAN short ID 168
+        V2TransportTester tester_new(true);
+        // Uses PROTOCOL_VERSION (70240) by default
+
+        ret = tester_new.Interact();
+        BOOST_REQUIRE(ret && ret->empty());
+        tester_new.SendKey();
+        tester_new.SendGarbage();
+        tester_new.ReceiveKey();
+        tester_new.SendGarbageTerm();
+        tester_new.SendVersion();
+        ret = tester_new.Interact();
+        BOOST_REQUIRE(ret && ret->empty());
+        tester_new.ReceiveGarbage();
+        tester_new.ReceiveVersion();
+
+        // New peer gets short encoding for PLATFORMBAN
+        auto msg_data_new = g_insecure_rand_ctx.randbytes<uint8_t>(100);
+        tester_new.AddMessage("platformban", msg_data_new);
+        ret = tester_new.Interact();
+        BOOST_REQUIRE(ret && ret->empty());
+        tester_new.ReceiveMessage(uint8_t(168), msg_data_new); // short encoding
+    }
+}
+
 BOOST_AUTO_TEST_SUITE_END()

src/version.h

@@ -10,7 +10,8 @@
  * network protocol versioning
  */
 
-static const int PROTOCOL_VERSION = 70239;
+
+static const int PROTOCOL_VERSION = 70240;
 
 //! initial proto version, to be increased after version/verack negotiation
 static const int INIT_PROTO_VERSION = 209;
@@ -48,6 +49,9 @@ static const int DSQ_INV_VERSION = 70234;
 //! Maximum header count for HEADRES2 message was increased from 2000 to 8000 in this version
 static const int INCREASE_MAX_HEADERS2_VERSION = 70235;
 
+//! BIP324 v2 transport with Dash short IDs (128-167) introduced
+static const int BIP324_DASH_BASELINE_VERSION = 70235;
+
 //! Behavior of QRINFO is changed in this protocol version
 static const int EFFICIENT_QRINFO_VERSION = 70236;
 
@@ -60,6 +64,9 @@ static const int PLATFORM_BAN_VERSION = 70238;
 //! Ban of re-propagation of old QFCOMMIT enforcement
 static const int QFCOMMIT_STALE_REPROP_BAN_VERSION = 70239;
 
+//! PLATFORMBAN added to v2 short IDs (short ID 168)
+static const int PLATFORMBAN_V2_SHORT_ID_VERSION = 70240;
+
 // Make sure that none of the values above collide with `ADDRV2_FORMAT`.
 
 #endif // BITCOIN_VERSION_H

test/functional/test_framework/p2p.py

@@ -100,8 +100,8 @@
 # The minimum P2P version that this test framework supports
 MIN_P2P_VERSION_SUPPORTED = 60001
 # The P2P version that this test framework implements and sends in its `version` message
-# Version 70238 introduced platform PoSe ban (dip-0031)
-P2P_VERSION = 70238
+# Version 70240 introduced PLATFORMBAN to v2 short IDs
+P2P_VERSION = 70240
 # The services that this test framework offers in its `version` message
 P2P_SERVICES = NODE_NETWORK | NODE_HEADERS_COMPRESSED
 # The P2P user agent string that this test framework sends in its `version` message

test/functional/test_framework/v2_p2p.py

@@ -90,6 +90,7 @@
     165: b"headers2",
     166: b"getqrinfo",
     167: b"qrinfo",
+    168: b"platformban",
 }
 
 # Dictionary which contains short message type ID for the P2P message