From 2adffaef724f0180ffc44fb0a91c6bb854a2ebaf Mon Sep 17 00:00:00 2001 From: Bart Date: Fri, 14 Aug 2026 15:36:47 +0000 Subject: [PATCH] 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 --- include/xrpl/proto/xrpl.proto | 15 +- src/test/app/ValidatorList_test.cpp | 238 +++++-------------- src/test/overlay/ProtocolVersion_test.cpp | 38 +-- src/test/overlay/compression_test.cpp | 30 --- src/xrpld/app/misc/ValidatorList.h | 13 - src/xrpld/app/misc/detail/ValidatorList.cpp | 167 +++---------- src/xrpld/overlay/Peer.h | 2 - src/xrpld/overlay/detail/Message.cpp | 1 - src/xrpld/overlay/detail/PeerImp.cpp | 38 +-- src/xrpld/overlay/detail/PeerImp.h | 2 - src/xrpld/overlay/detail/ProtocolMessage.h | 5 - src/xrpld/overlay/detail/ProtocolVersion.cpp | 1 - src/xrpld/overlay/detail/TrafficCount.cpp | 1 - 13 files changed, 114 insertions(+), 437 deletions(-) diff --git a/include/xrpl/proto/xrpl.proto b/include/xrpl/proto/xrpl.proto index b9cb94e668..644e099179 100644 --- a/include/xrpl/proto/xrpl.proto +++ b/include/xrpl/proto/xrpl.proto @@ -1,10 +1,10 @@ syntax = "proto2"; package protocol; -// Unused numbers in the list below may have been used previously. Please don't -// reassign them for reuse unless you are 100% certain that there won't be a -// conflict. Even if you're sure, it's probably best to assign a new type. enum MessageType { + // Previously used - don't reuse. + reserved 0 to 1, 4, 6 to 14, 16 to 29, 36 to 40, 43 to 54, 61 to 62; + mtMANIFESTS = 2; mtPING = 3; mtCLUSTER = 5; @@ -17,7 +17,6 @@ enum MessageType { mtHAVE_SET = 35; mtVALIDATION = 41; mtGET_OBJECTS = 42; - mtVALIDATOR_LIST = 54; mtSQUELCH = 55; mtVALIDATOR_LIST_COLLECTION = 56; mtPROOF_PATH_REQ = 57; @@ -162,14 +161,6 @@ message TMHaveTransactionSet { required bytes hash = 2; } -// Validator list (UNL) -message TMValidatorList { - required bytes manifest = 1; - required bytes blob = 2; - required bytes signature = 3; - required uint32 version = 4; -} - // Validator List v2 message ValidatorBlobInfo { optional bytes manifest = 1; diff --git a/src/test/app/ValidatorList_test.cpp b/src/test/app/ValidatorList_test.cpp index 323c77c780..d2e6cb24aa 100644 --- a/src/test/app/ValidatorList_test.cpp +++ b/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(*header, buffers.data()); - BEAST_EXPECT(msg); - return msg; - } - return std::shared_ptr(); - }; - 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(); }; - auto verifyMessage = - [this, manifestCutoff, &extractProtocolMessage1, &extractProtocolMessage2]( - auto const version, - auto const& manifest, - auto const& blobInfos, - auto const& messages, - std::vector>> 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> 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 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 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 const& result, std::size_t expectedSequence, @@ -2471,66 +2410,10 @@ private: std::vector 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 diff --git a/src/test/overlay/ProtocolVersion_test.cpp b/src/test/overlay/ProtocolVersion_test.cpp index e31a574502..e7b63a34cb 100644 --- a/src/test/overlay/ProtocolVersion_test.cpp +++ b/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); } } }; diff --git a/src/test/overlay/compression_test.cpp b/src/test/overlay/compression_test.cpp index a583a3aeab..40dee96c75 100644 --- a/src/test/overlay/compression_test.cpp +++ b/src/test/overlay/compression_test.cpp @@ -292,33 +292,6 @@ public: return getObject; } - static std::shared_ptr - buildValidatorList() - { - auto list = std::make_shared(); - - 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 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, diff --git a/src/xrpld/app/misc/ValidatorList.h b/src/xrpld/app/misc/ValidatorList.h index abec6cf4e0..4e001affe8 100644 --- a/src/xrpld/app/misc/ValidatorList.h +++ b/src/xrpld/app/misc/ValidatorList.h @@ -30,7 +30,6 @@ #include namespace protocol { -class TMValidatorList; class TMValidatorListCollection; } // namespace protocol @@ -371,9 +370,6 @@ public: static std::vector parseBlobs(std::uint32_t version, json::Value const& body); - static std::vector - parseBlobs(protocol::TMValidatorList const& body); - static std::vector parseBlobs(protocol::TMValidatorListCollection const& body); @@ -391,7 +387,6 @@ public: [[nodiscard]] static std::pair 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 const& blobs) namespace protocol { -template -void -hash_append(Hasher& h, TMValidatorList const& msg) -{ - using beast::hash_append; - hash_append(h, msg.manifest(), msg.blob(), msg.signature(), msg.version()); -} - template void hash_append(Hasher& h, TMValidatorListCollection const& msg) diff --git a/src/xrpld/app/misc/detail/ValidatorList.cpp b/src/xrpld/app/misc/detail/ValidatorList.cpp index 0ada8ed55f..f099ebf059 100644 --- a/src/xrpld/app/misc/detail/ValidatorList.cpp +++ b/src/xrpld/app/misc/detail/ValidatorList.cpp @@ -449,13 +449,6 @@ ValidatorList::parseBlobs(std::uint32_t version, json::Value const& body) } } -// static -std::vector -ValidatorList::parseBlobs(protocol::TMValidatorList const& body) -{ - return {{.blob = body.blob(), .signature = body.signature(), .manifest = {}}}; -} - // static std::vector 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(smallMsg, protocol::mtVALIDATOR_LIST), - sha512Half(smallMsg), - 1); - return messages.back().numVLs; - } std::optional 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(*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& 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(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 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 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> 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()); } } diff --git a/src/xrpld/overlay/Peer.h b/src/xrpld/overlay/Peer.h index 87750ed40e..6c4cf1dff1 100644 --- a/src/xrpld/overlay/Peer.h +++ b/src/xrpld/overlay/Peer.h @@ -20,8 +20,6 @@ class Charge; } // namespace resource enum class ProtocolFeature { - ValidatorListPropagation, - ValidatorList2Propagation, LedgerReplay, LedgerNodeDepth, }; diff --git a/src/xrpld/overlay/detail/Message.cpp b/src/xrpld/overlay/detail/Message.cpp index c6e0511515..a6af525620 100644 --- a/src/xrpld/overlay/detail/Message.cpp +++ b/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: diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 726002fce4..3f0b4453b8 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/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 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 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()) diff --git a/src/xrpld/overlay/detail/PeerImp.h b/src/xrpld/overlay/detail/PeerImp.h index 7078d6fb56..0f229bf9d8 100644 --- a/src/xrpld/overlay/detail/PeerImp.h +++ b/src/xrpld/overlay/detail/PeerImp.h @@ -623,8 +623,6 @@ public: void onMessage(std::shared_ptr const& m); void - onMessage(std::shared_ptr const& m); - void onMessage(std::shared_ptr const& m); void onMessage(std::shared_ptr const& m); diff --git a/src/xrpld/overlay/detail/ProtocolMessage.h b/src/xrpld/overlay/detail/ProtocolMessage.h index f7d5e26272..88f50e1e2e 100644 --- a/src/xrpld/overlay/detail/ProtocolMessage.h +++ b/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(*header, buffers, handler); break; - case protocol::mtVALIDATOR_LIST: - success = detail::invoke(*header, buffers, handler); - break; case protocol::mtVALIDATOR_LIST_COLLECTION: success = detail::invoke(*header, buffers, handler); diff --git a/src/xrpld/overlay/detail/ProtocolVersion.cpp b/src/xrpld/overlay/detail/ProtocolVersion.cpp index 93d4fae156..1296041ad5 100644 --- a/src/xrpld/overlay/detail/ProtocolVersion.cpp +++ b/src/xrpld/overlay/detail/ProtocolVersion.cpp @@ -28,7 +28,6 @@ namespace xrpl { */ constexpr ProtocolVersion const kSupportedProtocolList[]{ - {2, 1}, {2, 2}, {2, 3}, }; diff --git a/src/xrpld/overlay/detail/TrafficCount.cpp b/src/xrpld/overlay/detail/TrafficCount.cpp index bdce9e68f0..90d5c0b4ff 100644 --- a/src/xrpld/overlay/detail/TrafficCount.cpp +++ b/src/xrpld/overlay/detail/TrafficCount.cpp @@ -14,7 +14,6 @@ std::unordered_map 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},