refactor: Remove support for protocol version 2.1 (#7432)

Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

src/test/app/ValidatorList_test.cpp

@@ -2253,8 +2253,7 @@ private:
     {
         testcase("Sha512 hashing");
         // Tests that ValidatorList hash_append helpers with a single blob
-        // returns the same result as xrpl::Sha512Half used by the
-        // TMValidatorList protocol message handler
+        // return the same result as xrpl::Sha512Half
         std::string const manifest = "This is not really a manifest";
         std::string const blob = "This is not really a blob";
         std::string const signature = "This is not really a signature";
@@ -2275,17 +2274,6 @@ private:
             BEAST_EXPECT(global != sha512Half(blob, blobMap, version));
         }
 
-        {
-            protocol::TMValidatorList msg1;
-            msg1.set_manifest(manifest);
-            msg1.set_blob(blob);
-            msg1.set_signature(signature);
-            msg1.set_version(version);
-            BEAST_EXPECT(global == sha512Half(msg1));
-            msg1.set_signature(blob);
-            BEAST_EXPECT(global != sha512Half(msg1));
-        }
-
         {
             protocol::TMValidatorListCollection msg2;
             msg2.set_manifest(manifest);
@@ -2323,19 +2311,7 @@ private:
             BEAST_EXPECT(!ec);
             return std::make_pair(header, buffers);
         };
-        auto extractProtocolMessage1 = [this, &extractHeader](Message& message) {
-            auto [header, buffers] = extractHeader(message);
-            if (BEAST_EXPECT(header) &&
-                BEAST_EXPECT(header->messageType == protocol::mtVALIDATOR_LIST))
-            {
-                auto const msg =
-                    detail::parseMessageContent<protocol::TMValidatorList>(*header, buffers.data());
-                BEAST_EXPECT(msg);
-                return msg;
-            }
-            return std::shared_ptr<protocol::TMValidatorList>();
-        };
-        auto extractProtocolMessage2 = [this, &extractHeader](Message& message) {
+        auto extractProtocolMessage = [this, &extractHeader](Message& message) {
             auto [header, buffers] = extractHeader(message);
             if (BEAST_EXPECT(header) &&
                 BEAST_EXPECT(header->messageType == protocol::mtVALIDATOR_LIST_COLLECTION))
@@ -2347,92 +2323,55 @@ private:
             }
             return std::shared_ptr<protocol::TMValidatorListCollection>();
         };
-        auto verifyMessage =
-            [this, manifestCutoff, &extractProtocolMessage1, &extractProtocolMessage2](
-                auto const version,
-                auto const& manifest,
-                auto const& blobInfos,
-                auto const& messages,
-                std::vector<std::pair<std::size_t, std::vector<std::uint32_t>>> expectedInfo) {
-                BEAST_EXPECT(messages.size() == expectedInfo.size());
-                auto msgIter = expectedInfo.begin();
-                for (auto const& messageWithHash : messages)
+        auto verifyMessage = [this, manifestCutoff, &extractProtocolMessage](
+                                 auto const version,
+                                 auto const& manifest,
+                                 auto const& blobInfos,
+                                 auto const& messages,
+                                 std::vector<std::vector<std::uint32_t>> expectedInfo) {
+            BEAST_EXPECT(messages.size() == expectedInfo.size());
+            auto msgIter = expectedInfo.begin();
+            for (auto const& messageWithHash : messages)
+            {
+                if (!BEAST_EXPECT(msgIter != expectedInfo.end()))
+                    break;
+                if (!BEAST_EXPECT(messageWithHash.message))
+                    continue;
+                auto const& expectedSeqs = *msgIter;
+                auto seqIter = expectedSeqs.begin();
                 {
-                    if (!BEAST_EXPECT(msgIter != expectedInfo.end()))
-                        break;
-                    if (!BEAST_EXPECT(messageWithHash.message))
-                        continue;
-                    auto const& expectedSeqs = msgIter->second;
-                    auto seqIter = expectedSeqs.begin();
-                    auto const size =
-                        messageWithHash.message->getBuffer(compression::Compressed::Off).size();
-                    // This size is arbitrary, but shouldn't change
-                    BEAST_EXPECT(size == msgIter->first);
-                    if (expectedSeqs.size() == 1)
+                    std::vector<ValidatorBlobInfo> hashingBlobs;
+                    hashingBlobs.reserve(expectedSeqs.size());
+
+                    auto const msg = extractProtocolMessage(*messageWithHash.message);
+                    if (BEAST_EXPECT(msg))
                     {
-                        auto const msg = extractProtocolMessage1(*messageWithHash.message);
-                        auto const expectedVersion = 1;
-                        if (BEAST_EXPECT(msg))
+                        BEAST_EXPECT(msg->version() == version);
+                        BEAST_EXPECT(msg->manifest() == manifest);
+                        for (auto const& blobInfo : msg->blobs())
                         {
-                            BEAST_EXPECT(msg->version() == expectedVersion);
                             if (!BEAST_EXPECT(seqIter != expectedSeqs.end()))
-                                continue;
+                                break;
                             auto const& expectedBlob = blobInfos.at(*seqIter);
-                            BEAST_EXPECT((*seqIter < manifestCutoff) == !!expectedBlob.manifest);
-                            auto const expectedManifest =
-                                *seqIter < manifestCutoff && expectedBlob.manifest
-                                ? *expectedBlob.manifest
-                                : manifest;
-                            BEAST_EXPECT(msg->manifest() == expectedManifest);
-                            BEAST_EXPECT(msg->blob() == expectedBlob.blob);
-                            BEAST_EXPECT(msg->signature() == expectedBlob.signature);
+                            hashingBlobs.push_back(expectedBlob);
+                            BEAST_EXPECT(blobInfo.has_manifest() == !!expectedBlob.manifest);
+                            BEAST_EXPECT(blobInfo.has_manifest() == (*seqIter < manifestCutoff));
+
+                            if (*seqIter < manifestCutoff)
+                                BEAST_EXPECT(blobInfo.manifest() == *expectedBlob.manifest);
+                            BEAST_EXPECT(blobInfo.blob() == expectedBlob.blob);
+                            BEAST_EXPECT(blobInfo.signature() == expectedBlob.signature);
                             ++seqIter;
-                            BEAST_EXPECT(seqIter == expectedSeqs.end());
-
-                            BEAST_EXPECT(
-                                messageWithHash.hash ==
-                                sha512Half(
-                                    expectedManifest,
-                                    expectedBlob.blob,
-                                    expectedBlob.signature,
-                                    expectedVersion));
                         }
+                        BEAST_EXPECT(seqIter == expectedSeqs.end());
                     }
-                    else
-                    {
-                        std::vector<ValidatorBlobInfo> hashingBlobs;
-                        hashingBlobs.reserve(msgIter->second.size());
-
-                        auto const msg = extractProtocolMessage2(*messageWithHash.message);
-                        if (BEAST_EXPECT(msg))
-                        {
-                            BEAST_EXPECT(msg->version() == version);
-                            BEAST_EXPECT(msg->manifest() == manifest);
-                            for (auto const& blobInfo : msg->blobs())
-                            {
-                                if (!BEAST_EXPECT(seqIter != expectedSeqs.end()))
-                                    break;
-                                auto const& expectedBlob = blobInfos.at(*seqIter);
-                                hashingBlobs.push_back(expectedBlob);
-                                BEAST_EXPECT(blobInfo.has_manifest() == !!expectedBlob.manifest);
-                                BEAST_EXPECT(
-                                    blobInfo.has_manifest() == (*seqIter < manifestCutoff));
-
-                                if (*seqIter < manifestCutoff)
-                                    BEAST_EXPECT(blobInfo.manifest() == *expectedBlob.manifest);
-                                BEAST_EXPECT(blobInfo.blob() == expectedBlob.blob);
-                                BEAST_EXPECT(blobInfo.signature() == expectedBlob.signature);
-                                ++seqIter;
-                            }
-                            BEAST_EXPECT(seqIter == expectedSeqs.end());
-                        }
-                        BEAST_EXPECT(
-                            messageWithHash.hash == sha512Half(manifest, hashingBlobs, version));
-                    }
-                    ++msgIter;
+                    BEAST_EXPECT(
+                        messageWithHash.hash == sha512Half(manifest, hashingBlobs, version));
                 }
-                BEAST_EXPECT(msgIter == expectedInfo.end());
-            };
+                ++msgIter;
+            }
+            BEAST_EXPECT(msgIter == expectedInfo.end());
+        };
         auto verifyBuildMessages = [this](
                                        std::pair<std::size_t, std::size_t> const& result,
                                        std::size_t expectedSequence,
@@ -2471,66 +2410,10 @@ private:
 
         std::vector<ValidatorList::MessageWithHash> messages;
 
-        // Version 1
-
-        // This peer has a VL ahead of our "current"
-        verifyBuildMessages(
-            ValidatorList::buildValidatorListMessages(
-                1, 8, maxSequence, version, manifest, blobInfos, messages),
-            0,
-            0);
-        BEAST_EXPECT(messages.empty());
-
-        // Don't repeat the work if messages is populated, even though the
-        // peerSequence provided indicates it should. Note that this
-        // situation is contrived for this test and should never happen in
-        // real code.
-        messages.emplace_back();
-        verifyBuildMessages(
-            ValidatorList::buildValidatorListMessages(
-                1, 3, maxSequence, version, manifest, blobInfos, messages),
-            5,
-            0);
-        BEAST_EXPECT(messages.size() == 1 && !messages.front().message);
-
-        // Generate a version 1 message
-        messages.clear();
-        verifyBuildMessages(
-            ValidatorList::buildValidatorListMessages(
-                1, 3, maxSequence, version, manifest, blobInfos, messages),
-            5,
-            1);
-        if (BEAST_EXPECT(messages.size() == 1) && BEAST_EXPECT(messages.front().message))
-        {
-            auto const& messageWithHash = messages.front();
-            auto const msg = extractProtocolMessage1(*messageWithHash.message);
-            auto const size =
-                messageWithHash.message->getBuffer(compression::Compressed::Off).size();
-            // This size is arbitrary, but shouldn't change
-            BEAST_EXPECT(size == 108);
-            auto const& expected = blobInfos.at(5);
-            if (BEAST_EXPECT(msg))
-            {
-                BEAST_EXPECT(msg->version() == 1);
-                // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
-                BEAST_EXPECT(msg->manifest() == *expected.manifest);
-                BEAST_EXPECT(msg->blob() == expected.blob);
-                BEAST_EXPECT(msg->signature() == expected.signature);
-            }
-            BEAST_EXPECT(
-                messageWithHash.hash ==
-                // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
-                sha512Half(*expected.manifest, expected.blob, expected.signature, 1));
-        }
-
-        // Version 2
-
-        messages.clear();
-
         // This peer has a VL ahead of us.
         verifyBuildMessages(
             ValidatorList::buildValidatorListMessages(
-                2, maxSequence * 2, maxSequence, version, manifest, blobInfos, messages),
+                maxSequence * 2, maxSequence, version, manifest, blobInfos, messages),
             0,
             0);
         BEAST_EXPECT(messages.empty());
@@ -2542,19 +2425,19 @@ private:
         messages.emplace_back();
         verifyBuildMessages(
             ValidatorList::buildValidatorListMessages(
-                2, 3, maxSequence, version, manifest, blobInfos, messages),
+                3, maxSequence, version, manifest, blobInfos, messages),
             maxSequence,
             0);
         BEAST_EXPECT(messages.size() == 1 && !messages.front().message);
 
-        // Generate a version 2 message. Don't send the current
+        // Generate a message. Don't send the current
         messages.clear();
         verifyBuildMessages(
             ValidatorList::buildValidatorListMessages(
-                2, 5, maxSequence, version, manifest, blobInfos, messages),
+                5, maxSequence, version, manifest, blobInfos, messages),
             maxSequence,
             4);
-        verifyMessage(version, manifest, blobInfos, messages, {{372, {6, 7, 10, 12}}});
+        verifyMessage(version, manifest, blobInfos, messages, {{6, 7, 10, 12}});
 
         // Test message splitting on size limits.
 
@@ -2562,50 +2445,39 @@ private:
         messages.clear();
         verifyBuildMessages(
             ValidatorList::buildValidatorListMessages(
-                2, 5, maxSequence, version, manifest, blobInfos, messages, 300),
+                5, maxSequence, version, manifest, blobInfos, messages, 300),
             maxSequence,
             4);
-        verifyMessage(version, manifest, blobInfos, messages, {{212, {6, 7}}, {192, {10, 12}}});
+        verifyMessage(version, manifest, blobInfos, messages, {{6, 7}, {10, 12}});
 
         // Set a limit between the size of the two earlier messages so one
         // will split and the other won't
         messages.clear();
         verifyBuildMessages(
             ValidatorList::buildValidatorListMessages(
-                2, 5, maxSequence, version, manifest, blobInfos, messages, 200),
+                5, maxSequence, version, manifest, blobInfos, messages, 200),
             maxSequence,
             4);
-        verifyMessage(
-            version, manifest, blobInfos, messages, {{108, {6}}, {108, {7}}, {192, {10, 12}}});
+        verifyMessage(version, manifest, blobInfos, messages, {{6}, {7}, {10, 12}});
 
         // Set a limit so that all the VLs are sent individually
         messages.clear();
         verifyBuildMessages(
             ValidatorList::buildValidatorListMessages(
-                2, 5, maxSequence, version, manifest, blobInfos, messages, 150),
+                5, maxSequence, version, manifest, blobInfos, messages, 150),
             maxSequence,
             4);
-        verifyMessage(
-            version,
-            manifest,
-            blobInfos,
-            messages,
-            {{108, {6}}, {108, {7}}, {110, {10}}, {110, {12}}});
+        verifyMessage(version, manifest, blobInfos, messages, {{6}, {7}, {10}, {12}});
 
         // Set a limit smaller than some of the messages. Because single
         // messages send regardless, they will all still be sent
         messages.clear();
         verifyBuildMessages(
             ValidatorList::buildValidatorListMessages(
-                2, 5, maxSequence, version, manifest, blobInfos, messages, 108),
+                5, maxSequence, version, manifest, blobInfos, messages, 108),
             maxSequence,
             4);
-        verifyMessage(
-            version,
-            manifest,
-            blobInfos,
-            messages,
-            {{108, {6}}, {108, {7}}, {110, {10}}, {110, {12}}});
+        verifyMessage(version, manifest, blobInfos, messages, {{6}, {7}, {10}, {12}});
     }
 
     void

src/test/overlay/ProtocolVersion_test.cpp

@@ -33,22 +33,30 @@ public:
     void
     run() override
     {
-        testcase("Convert protocol version to string");
-        BEAST_EXPECT(to_string(makeProtocol(1, 3)) == "XRPL/1.3");
-        BEAST_EXPECT(to_string(makeProtocol(2, 0)) == "XRPL/2.0");
-        BEAST_EXPECT(to_string(makeProtocol(2, 1)) == "XRPL/2.1");
-        BEAST_EXPECT(to_string(makeProtocol(10, 10)) == "XRPL/10.10");
+        {
+            testcase("Convert protocol version to string");
+
+            BEAST_EXPECT(to_string(makeProtocol(0, 0)) == "XRPL/0.0");
+            BEAST_EXPECT(to_string(makeProtocol(0, 1)) == "XRPL/0.1");
+            BEAST_EXPECT(to_string(makeProtocol(1, 3)) == "XRPL/1.3");
+            BEAST_EXPECT(to_string(makeProtocol(2, 0)) == "XRPL/2.0");
+            BEAST_EXPECT(to_string(makeProtocol(2, 1)) == "XRPL/2.1");
+            BEAST_EXPECT(to_string(makeProtocol(10, 10)) == "XRPL/10.10");
+            BEAST_EXPECT(to_string(makeProtocol(65535, 65535)) == "XRPL/65535.65535");
+        }
 
         {
             testcase("Convert strings to protocol versions");
 
-            // Empty string
+            // Invalid versions, either they do not parse as XRPL/N.M or are unsupported.
             check("", "");
+            check("RTXP/1.1,RTXP/1.2,RTXP/1.3", "");
+            check("XRPL/-2.1,XRPL/0.3,XRPL/2,XRPL/2.01,websocket", "");
 
-            check("RTXP/1.1,RTXP/1.2,RTXP/1.3,XRPL/2.1,XRPL/2.0,/XRPL/3.0", "XRPL/2.0,XRPL/2.1");
-            check("RTXP/0.9,RTXP/1.01,XRPL/0.3,XRPL/2.01,websocket", "");
+            // Mixture of valid, duplicate, and invalid versions.
+            check("RTXP/1.3,XRPL/2.1,XRPL/2.0,/XRPL/3.0", "XRPL/2.0,XRPL/2.1");
             check(
-                "XRPL/2.0,XRPL/2.0,XRPL/19.4,XRPL/7.89,XRPL/XRPL/3.0,XRPL/2.01",
+                "XRPL/2.0,XRPL/2.0,XRPL/19.4,XRPL/7.89,XRPL/XRPL/3.0,XRPL/2.01,XRPL/-65535.65535",
                 "XRPL/2.0,XRPL/7.89,XRPL/19.4");
             check(
                 "XRPL/2.0,XRPL/3.0,XRPL/4,XRPL/,XRPL,OPT XRPL/2.2,XRPL/5.67",
@@ -58,15 +66,17 @@ public:
         {
             testcase("Protocol version negotiation");
 
-            BEAST_EXPECT(negotiateProtocolVersion("RTXP/1.2") == std::nullopt);
+            // Only the highest supported protocol version, if any, is returned.
+            BEAST_EXPECT(negotiateProtocolVersion("") == std::nullopt);
+            BEAST_EXPECT(negotiateProtocolVersion("XRPL/0.0") == std::nullopt);
+            BEAST_EXPECT(negotiateProtocolVersion("RTXP/1.2,XRPL/0.1") == std::nullopt);
             BEAST_EXPECT(
-                negotiateProtocolVersion("RTXP/1.2, XRPL/2.0, XRPL/2.1") == makeProtocol(2, 1));
+                negotiateProtocolVersion("XRPL/999.999, XRPL/-2.2,WebSocket/1.0") == std::nullopt);
             BEAST_EXPECT(negotiateProtocolVersion("XRPL/2.2") == makeProtocol(2, 2));
             BEAST_EXPECT(
-                negotiateProtocolVersion("RTXP/1.2, XRPL/2.3, XRPL/2.4, XRPL/999.999") ==
+                negotiateProtocolVersion(
+                    "RTXP/1.2, XRPL/2.1, XRPL/2.2, XRPL/2.3, XRPL/2.4, XRPL/999.999") ==
                 makeProtocol(2, 3));
-            BEAST_EXPECT(negotiateProtocolVersion("XRPL/999.999, WebSocket/1.0") == std::nullopt);
-            BEAST_EXPECT(negotiateProtocolVersion("") == std::nullopt);
         }
     }
 };

src/test/overlay/compression_test.cpp

@@ -292,33 +292,6 @@ public:
         return getObject;
     }
 
-    static std::shared_ptr<protocol::TMValidatorList>
-    buildValidatorList()
-    {
-        auto list = std::make_shared<protocol::TMValidatorList>();
-
-        auto master = randomKeyPair(KeyType::Ed25519);
-        auto signing = randomKeyPair(KeyType::Ed25519);
-        STObject st(sfGeneric);
-        st[sfSequence] = 0;
-        st[sfPublicKey] = std::get<0>(master);
-        st[sfSigningPubKey] = std::get<0>(signing);
-        st[sfDomain] = makeSlice(std::string("example.com"));
-        sign(st, HashPrefix::Manifest, KeyType::Ed25519, std::get<1>(master), sfMasterSignature);
-        sign(st, HashPrefix::Manifest, KeyType::Ed25519, std::get<1>(signing));
-        Serializer s;
-        st.add(s);
-        list->set_manifest(s.data(), s.size());
-        list->set_version(3);
-        STObject const signature(sfSignature);
-        xrpl::sign(st, HashPrefix::Manifest, KeyType::Ed25519, std::get<1>(signing));
-        Serializer s1;
-        st.add(s1);
-        list->set_signature(s1.data(), s1.size());
-        list->set_blob(strHex(s.slice()));
-        return list;
-    }
-
     static std::shared_ptr<protocol::TMValidatorListCollection>
     buildValidatorListCollection()
     {
@@ -359,7 +332,6 @@ public:
         protocol::TMGetLedger const getLedger;
         protocol::TMLedgerData const ledgerData;
         protocol::TMGetObjectByHash const getObject;
-        protocol::TMValidatorList const validatorList;
         protocol::TMValidatorListCollection const validatorListCollection;
 
         // 4.5KB
@@ -386,8 +358,6 @@ public:
         doTest(buildLedgerData(500000, *logs), protocol::mtLEDGER_DATA, 100, "TMLedgerData500000");
         // 7.7KB
         doTest(buildGetObjectByHash(), protocol::mtGET_OBJECTS, 4, "TMGetObjectByHash");
-        // 895B
-        doTest(buildValidatorList(), protocol::mtVALIDATOR_LIST, 4, "TMValidatorList");
         doTest(
             buildValidatorListCollection(),
             protocol::mtVALIDATOR_LIST_COLLECTION,

src/xrpld/app/misc/ValidatorList.h

@@ -30,7 +30,6 @@
 #include <vector>
 
 namespace protocol {
-class TMValidatorList;
 class TMValidatorListCollection;
 }  // namespace protocol
 
@@ -371,9 +370,6 @@ public:
     static std::vector<ValidatorBlobInfo>
     parseBlobs(std::uint32_t version, json::Value const& body);
 
-    static std::vector<ValidatorBlobInfo>
-    parseBlobs(protocol::TMValidatorList const& body);
-
     static std::vector<ValidatorBlobInfo>
     parseBlobs(protocol::TMValidatorListCollection const& body);
 
@@ -391,7 +387,6 @@ public:
 
     [[nodiscard]] static std::pair<std::size_t, std::size_t>
     buildValidatorListMessages(
-        std::size_t messageVersion,
         std::uint64_t peerSequence,
         std::size_t maxSequence,
         std::uint32_t rawVersion,
@@ -987,14 +982,6 @@ hash_append(Hasher& h, std::map<std::size_t, ValidatorBlobInfo> const& blobs)
 
 namespace protocol {
 
-template <class Hasher>
-void
-hash_append(Hasher& h, TMValidatorList const& msg)
-{
-    using beast::hash_append;
-    hash_append(h, msg.manifest(), msg.blob(), msg.signature(), msg.version());
-}
-
 template <class Hasher>
 void
 hash_append(Hasher& h, TMValidatorListCollection const& msg)

src/xrpld/app/misc/detail/ValidatorList.cpp

@@ -449,13 +449,6 @@ ValidatorList::parseBlobs(std::uint32_t version, json::Value const& body)
     }
 }
 
-// static
-std::vector<ValidatorBlobInfo>
-ValidatorList::parseBlobs(protocol::TMValidatorList const& body)
-{
-    return {{.blob = body.blob(), .signature = body.signature(), .manifest = {}}};
-}
-
 // static
 std::vector<ValidatorBlobInfo>
 ValidatorList::parseBlobs(protocol::TMValidatorListCollection const& body)
@@ -476,7 +469,7 @@ ValidatorList::parseBlobs(protocol::TMValidatorListCollection const& body)
     }
     XRPL_ASSERT(
         result.size() == body.blobs_size(),
-        "xrpl::ValidatorList::parseBlobs(TMValidatorList) : result size "
+        "xrpl::ValidatorList::parseBlobs(TMValidatorListCollection) : result size "
         "match");
     return result;
 }
@@ -520,29 +513,6 @@ splitMessageParts(
 {
     if (end <= begin)
         return 0;
-    if (end - begin == 1)
-    {
-        protocol::TMValidatorList smallMsg;
-        smallMsg.set_version(1);
-        smallMsg.set_manifest(largeMsg.manifest());
-
-        auto const& blob = largeMsg.blobs(begin);
-        smallMsg.set_blob(blob.blob());
-        smallMsg.set_signature(blob.signature());
-        // This is only possible if "downgrading" a v2 UNL to v1.
-        if (blob.has_manifest())
-            smallMsg.set_manifest(blob.manifest());
-
-        XRPL_ASSERT(
-            Message::totalSize(smallMsg) <= kMaximumMessageSize,
-            "xrpl::splitMessageParts : maximum message size");
-
-        messages.emplace_back(
-            std::make_shared<Message>(smallMsg, protocol::mtVALIDATOR_LIST),
-            sha512Half(smallMsg),
-            1);
-        return messages.back().numVLs;
-    }
 
     std::optional<protocol::TMValidatorListCollection> smallMsg;
     smallMsg.emplace();
@@ -554,13 +524,29 @@ splitMessageParts(
         *smallMsg->add_blobs() = largeMsg.blobs(i);
     }
 
-    if (Message::totalSize(*smallMsg) > maxSize)
+    auto const size = Message::totalSize(*smallMsg);
+
+    // Split until each message fits, but a single blob can't be split any
+    // further, so stop recursing at that point regardless of maxSize.
+    if (size > maxSize && end - begin > 1)
     {
         // free up the message space
         smallMsg.reset();
         return splitMessage(messages, largeMsg, maxSize, begin, end);
     }
 
+    // An unsplittable blob is still bounded by the protocol limit: peers drop
+    // messages exceeding it on receipt, so don't waste the bandwidth. maxSize
+    // only ever tightens this (it defaults to kMaximumMessageSize), so a blob
+    // reaching here can exceed maxSize but never the protocol limit.
+    if (size > kMaximumMessageSize)
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE("xrpl::splitMessageParts : maximum message size exceeded");
+        return 0;
+        // LCOV_EXCL_STOP
+    }
+
     messages.emplace_back(
         std::make_shared<Message>(*smallMsg, protocol::mtVALIDATOR_LIST_COLLECTION),
         sha512Half(*smallMsg),
@@ -568,37 +554,6 @@ splitMessageParts(
     return messages.back().numVLs;
 }
 
-// Build a v1 protocol message using only the current VL
-std::size_t
-buildValidatorListMessage(
-    std::vector<ValidatorList::MessageWithHash>& messages,
-    std::uint32_t rawVersion,
-    std::string const& rawManifest,
-    ValidatorBlobInfo const& currentBlob,
-    std::size_t maxSize)
-{
-    XRPL_ASSERT(
-        messages.empty(),
-        "xrpl::buildValidatorListMessage(ValidatorBlobInfo) : empty messages "
-        "input");
-    protocol::TMValidatorList msg;
-    auto const manifest = currentBlob.manifest ? *currentBlob.manifest : rawManifest;
-    auto const version = 1;
-    msg.set_manifest(manifest);
-    msg.set_blob(currentBlob.blob);
-    msg.set_signature(currentBlob.signature);
-    // Override the version
-    msg.set_version(version);
-
-    XRPL_ASSERT(
-        Message::totalSize(msg) <= kMaximumMessageSize,
-        "xrpl::buildValidatorListMessage(ValidatorBlobInfo) : maximum "
-        "message size");
-    messages.emplace_back(
-        std::make_shared<Message>(msg, protocol::mtVALIDATOR_LIST), sha512Half(msg), 1);
-    return 1;
-}
-
 // Build a v2 protocol message using all the VLs with sequence larger than the
 // peer's
 std::size_t
@@ -650,7 +605,6 @@ buildValidatorListMessage(
 // static
 std::pair<std::size_t, std::size_t>
 ValidatorList::buildValidatorListMessages(
-    std::size_t messageVersion,
     std::uint64_t peerSequence,
     std::size_t maxSequence,
     std::uint32_t rawVersion,
@@ -663,14 +617,12 @@ ValidatorList::buildValidatorListMessages(
         !blobInfos.empty(),
         "xrpl::ValidatorList::buildValidatorListMessages : empty messages "
         "input");
-    auto const& [currentSeq, currentBlob] = *blobInfos.begin();
     auto numVLs = std::accumulate(
         messages.begin(), messages.end(), 0, [](std::size_t total, MessageWithHash const& m) {
             return total + m.numVLs;
         });
-    if (messageVersion == 2 && peerSequence < maxSequence)
+    if (peerSequence < maxSequence)
     {
-        // Version 2
         if (messages.empty())
         {
             numVLs = buildValidatorListMessage(
@@ -678,36 +630,13 @@ ValidatorList::buildValidatorListMessages(
             if (messages.empty())
             {
                 // No message was generated. Create an empty placeholder so we
-                // dont' repeat the work later.
+                // don't repeat the work later.
                 messages.emplace_back();
             }
         }
 
-        // Don't send it next time.
         return {maxSequence, numVLs};
     }
-    if (messageVersion == 1 && peerSequence < currentSeq)
-    {
-        // Version 1
-        if (messages.empty())
-        {
-            numVLs = buildValidatorListMessage(
-                messages,
-                rawVersion,
-                currentBlob.manifest ? *currentBlob.manifest : rawManifest,
-                currentBlob,
-                maxSize);
-            if (messages.empty())
-            {
-                // No message was generated. Create an empty placeholder so we
-                // dont' repeat the work later.
-                messages.emplace_back();
-            }
-        }
-
-        // Don't send it next time.
-        return {currentSeq, numVLs};
-    }
     return {0, 0};
 }
 
@@ -725,19 +654,8 @@ ValidatorList::sendValidatorList(
     HashRouter& hashRouter,
     beast::Journal j)
 {
-    std::size_t messageVersion = 0;
-    if (peer.supportsFeature(ProtocolFeature::ValidatorList2Propagation))
-    {
-        messageVersion = 2;
-    }
-    else if (peer.supportsFeature(ProtocolFeature::ValidatorListPropagation))
-    {
-        messageVersion = 1;
-    }
-    if (messageVersion == 0u)
-        return;
     auto const [newPeerSequence, numVLs] = buildValidatorListMessages(
-        messageVersion, peerSequence, maxSequence, rawVersion, rawManifest, blobInfos, messages);
+        peerSequence, maxSequence, rawVersion, rawManifest, blobInfos, messages);
     if (newPeerSequence != 0u)
     {
         XRPL_ASSERT(
@@ -764,24 +682,11 @@ ValidatorList::sendValidatorList(
             "xrpl::ValidatorList::sendValidatorList : sent or one message");
         if (sent)
         {
-            if (messageVersion > 1)
-            {
-                JLOG(j.debug()) << "Sent " << messages.size()
-                                << " validator list collection(s) containing " << numVLs
-                                << " validator list(s) for " << strHex(publisherKey)
-                                << " with sequence range " << peerSequence << ", "
-                                << newPeerSequence << " to " << peer.fingerprint();
-            }
-            else
-            {
-                XRPL_ASSERT(
-                    numVLs == 1,
-                    "xrpl::ValidatorList::sendValidatorList : one validator "
-                    "list");
-                JLOG(j.debug()) << "Sent validator list for " << strHex(publisherKey)
-                                << " with sequence " << newPeerSequence << " to "
-                                << peer.fingerprint();
-            }
+            JLOG(j.debug()) << "Sent " << messages.size()
+                            << " validator list collection(s) containing " << numVLs
+                            << " validator list(s) for " << strHex(publisherKey)
+                            << " with sequence range " << peerSequence << ", " << newPeerSequence
+                            << " to " << peer.fingerprint();
         }
     }
 }
@@ -856,16 +761,9 @@ ValidatorList::broadcastBlobs(
 
     if (toSkip)
     {
-        // We don't know what messages or message versions we're sending
-        // until we examine our peer's properties. Build the message(s) on
-        // demand, but reuse them when possible.
-
-        // This will hold a v1 message with only the current VL if we have
-        // any peers that don't support v2
-        std::vector<ValidatorList::MessageWithHash> messages1;
-        // This will hold v2 messages indexed by the peer's
-        // `publisherListSequence`. For each `publisherListSequence`, we'll
-        // only send the VLs with higher sequences.
+        // Build v2 messages on demand and reuse them when possible. Messages
+        // are indexed by the peer's `publisherListSequence`; for each sequence,
+        // we only send VLs with higher sequences.
         std::map<std::size_t, std::vector<ValidatorList::MessageWithHash>> messages2;
         // If any peers are found that are worth considering, this list will
         // be built to hold info for all of the valid VLs.
@@ -885,8 +783,6 @@ ValidatorList::broadcastBlobs(
                 {
                     if (blobInfos.empty())
                         buildBlobInfos(blobInfos, lists);
-                    auto const v2 =
-                        peer->supportsFeature(ProtocolFeature::ValidatorList2Propagation);
                     sendValidatorList(
                         *peer,
                         peerSequence,
@@ -895,11 +791,10 @@ ValidatorList::broadcastBlobs(
                         lists.rawVersion,
                         lists.rawManifest,
                         blobInfos,
-                        v2 ? messages2[peerSequence] : messages1,
+                        messages2[peerSequence],
                         hashRouter,
                         j);
-                    // Even if the peer doesn't support the messages,
-                    // suppress it so it'll be ignored next time.
+                    // Don't send it next time.
                     hashRouter.addSuppressionPeer(hash, peer->id());
                 }
             }

src/xrpld/overlay/Peer.h

@@ -20,8 +20,6 @@ class Charge;
 }  // namespace resource
 
 enum class ProtocolFeature {
-    ValidatorListPropagation,
-    ValidatorList2Propagation,
     LedgerReplay,
     LedgerNodeDepth,
 };

src/xrpld/overlay/detail/Message.cpp

@@ -82,7 +82,6 @@ Message::compress()
             case protocol::mtGET_LEDGER:
             case protocol::mtLEDGER_DATA:
             case protocol::mtGET_OBJECTS:
-            case protocol::mtVALIDATOR_LIST:
             case protocol::mtVALIDATOR_LIST_COLLECTION:
             case protocol::mtREPLAY_DELTA_RESPONSE:
             case protocol::mtTRANSACTIONS:

src/xrpld/overlay/detail/PeerImp.cpp

@@ -542,10 +542,6 @@ PeerImp::supportsFeature(ProtocolFeature f) const
 {
     switch (f)
     {
-        case ProtocolFeature::ValidatorListPropagation:
-            return protocol_ >= makeProtocol(2, 1);
-        case ProtocolFeature::ValidatorList2Propagation:
-            return protocol_ >= makeProtocol(2, 2);
         case ProtocolFeature::LedgerNodeDepth:
             return protocol_ >= makeProtocol(2, 3);
         case ProtocolFeature::LedgerReplay:
@@ -885,7 +881,7 @@ PeerImp::doProtocolStart()
     onReadMessage(error_code(), 0);
 
     // Send all the validator lists that have been loaded
-    if (inbound_ && supportsFeature(ProtocolFeature::ValidatorListPropagation))
+    if (inbound_)
     {
         app_.getValidators().forEachAvailable(
             [&](std::string const& manifest,
@@ -2422,43 +2418,11 @@ PeerImp::onValidatorListMessage(
     }
 }
 
-void
-PeerImp::onMessage(std::shared_ptr<protocol::TMValidatorList> const& m)
-{
-    try
-    {
-        if (!supportsFeature(ProtocolFeature::ValidatorListPropagation))
-        {
-            JLOG(pJournal_.debug()) << "ValidatorList: received validator list from peer using "
-                                    << "protocol version " << to_string(protocol_)
-                                    << " which shouldn't support this feature.";
-            fee_.update(resource::kFeeUselessData, "unsupported peer");
-            return;
-        }
-        onValidatorListMessage(
-            "ValidatorList", m->manifest(), m->version(), ValidatorList::parseBlobs(*m));
-    }
-    catch (std::exception const& e)
-    {
-        JLOG(pJournal_.warn()) << "ValidatorList: Exception, " << e.what();
-        using namespace std::string_literals;
-        fee_.update(resource::kFeeInvalidData, e.what());
-    }
-}
-
 void
 PeerImp::onMessage(std::shared_ptr<protocol::TMValidatorListCollection> const& m)
 {
     try
     {
-        if (!supportsFeature(ProtocolFeature::ValidatorList2Propagation))
-        {
-            JLOG(pJournal_.debug()) << "ValidatorListCollection: received validator list from peer "
-                                    << "using protocol version " << to_string(protocol_)
-                                    << " which shouldn't support this feature.";
-            fee_.update(resource::kFeeUselessData, "unsupported peer");
-            return;
-        }
         if (m->version() < 2)
         {
             JLOG(pJournal_.debug())

src/xrpld/overlay/detail/PeerImp.h

@@ -623,8 +623,6 @@ public:
     void
     onMessage(std::shared_ptr<protocol::TMHaveTransactionSet> const& m);
     void
-    onMessage(std::shared_ptr<protocol::TMValidatorList> const& m);
-    void
     onMessage(std::shared_ptr<protocol::TMValidatorListCollection> const& m);
     void
     onMessage(std::shared_ptr<protocol::TMValidation> const& m);

src/xrpld/overlay/detail/ProtocolMessage.h

@@ -71,8 +71,6 @@ protocolMessageName(int type)
             return "status";
         case protocol::mtHAVE_SET:
             return "have_set";
-        case protocol::mtVALIDATOR_LIST:
-            return "validator_list";
         case protocol::mtVALIDATOR_LIST_COLLECTION:
             return "validator_list_collection";
         case protocol::mtVALIDATION:
@@ -424,9 +422,6 @@ invokeProtocolMessage(Buffers const& buffers, Handler& handler, std::size_t& hin
         case protocol::mtVALIDATION:
             success = detail::invoke<protocol::TMValidation>(*header, buffers, handler);
             break;
-        case protocol::mtVALIDATOR_LIST:
-            success = detail::invoke<protocol::TMValidatorList>(*header, buffers, handler);
-            break;
         case protocol::mtVALIDATOR_LIST_COLLECTION:
             success =
                 detail::invoke<protocol::TMValidatorListCollection>(*header, buffers, handler);

src/xrpld/overlay/detail/ProtocolVersion.cpp

@@ -28,7 +28,6 @@ namespace xrpl {
  */
 
 constexpr ProtocolVersion const kSupportedProtocolList[]{
-    {2, 1},
     {2, 2},
     {2, 3},
 };

src/xrpld/overlay/detail/TrafficCount.cpp

@@ -14,7 +14,6 @@ std::unordered_map<protocol::MessageType, TrafficCount::Category> const kTypeLoo
     {protocol::mtMANIFESTS, TrafficCount::Category::Manifests},
     {protocol::mtENDPOINTS, TrafficCount::Category::Overlay},
     {protocol::mtTRANSACTION, TrafficCount::Category::Transaction},
-    {protocol::mtVALIDATOR_LIST, TrafficCount::Category::Validatorlist},
     {protocol::mtVALIDATOR_LIST_COLLECTION, TrafficCount::Category::Validatorlist},
     {protocol::mtVALIDATION, TrafficCount::Category::Validation},
     {protocol::mtPROPOSE_LEDGER, TrafficCount::Category::Proposal},