Remove unreachable code

Co-authored-by: Bart <bthomee@users.noreply.github.com>

.github/scripts/strategy-matrix/linux.json

@@ -1,5 +1,5 @@
 {
-  "image_tag": "sha-8abe82e",
+  "image_tag": "sha-63ffdc3",
   "configs": {
     "ubuntu": [
       {
@@ -67,7 +67,7 @@
         "compiler": ["gcc"],
         "build_type": ["Release"],
         "arch": ["amd64"],
-        "image": "debian:bookworm"
+        "image": "ghcr.io/xrplf/xrpld/packaging-debian:sha-63ffdc3"
       }
     ],
 
@@ -76,7 +76,7 @@
         "compiler": ["gcc"],
         "build_type": ["Release"],
         "arch": ["amd64"],
-        "image": "registry.access.redhat.com/ubi9/ubi:latest"
+        "image": "ghcr.io/xrplf/xrpld/packaging-rhel:sha-63ffdc3"
       }
     ]
   }

.github/workflows/build-nix-images.yml

@@ -22,7 +22,8 @@ on:
   workflow_dispatch:
 
 concurrency:
-  group: ${{ github.workflow }}-${{ github.ref }}
+  # Read `on-trigger.yml` for the rationale behind this concurrency group name.
+  group: ${{ github.workflow }}-${{ github.event_name == 'push' && github.ref == 'refs/heads/develop' && github.sha || github.ref }}
   cancel-in-progress: true
 
 defaults:

.github/workflows/build-packaging-images.yml

@@ -20,7 +20,8 @@ on:
   workflow_dispatch:
 
 concurrency:
-  group: ${{ github.workflow }}-${{ github.ref }}
+  # Read `on-trigger.yml` for the rationale behind this concurrency group name.
+  group: ${{ github.workflow }}-${{ github.event_name == 'push' && github.ref == 'refs/heads/develop' && github.sha || github.ref }}
   cancel-in-progress: true
 
 defaults:

.github/workflows/pre-commit.yml

@@ -14,7 +14,7 @@ on:
 jobs:
   # Call the workflow in the XRPLF/actions repo that runs the pre-commit hooks.
   run-hooks:
-    uses: XRPLF/actions/.github/workflows/pre-commit.yml@cba1f0891650baf1a9c88624dc2d72573be2eb81
+    uses: XRPLF/actions/.github/workflows/pre-commit.yml@312aaab296060ff89d7f798dcab59f019bea6e02
     with:
       runs_on: ubuntu-latest
       container: '{ "image": "ghcr.io/xrplf/ci/tools-rippled-pre-commit:sha-41ec7c1" }'

.github/workflows/publish-docs.yml

@@ -41,13 +41,13 @@ env:
 jobs:
   build:
     runs-on: ubuntu-latest
-    container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-8abe82e
+    container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-63ffdc3
     steps:
       - name: Checkout repository
         uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
 
       - name: Prepare runner
-        uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
+        uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8
         with:
           enable_ccache: false
 

.github/workflows/reusable-build-test-config.yml

@@ -113,7 +113,7 @@ jobs:
         uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
 
       - name: Prepare runner
-        uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
+        uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8
         with:
           enable_ccache: ${{ inputs.ccache_enabled }}
 

.github/workflows/reusable-clang-tidy.yml

@@ -29,14 +29,14 @@ jobs:
     if: ${{ inputs.check_only_changed }}
     permissions:
       contents: read
-    uses: XRPLF/actions/.github/workflows/determine-tidy-files.yml@224f3c48d3014d082a1129237b8291ff0b0a331f
+    uses: XRPLF/actions/.github/workflows/determine-tidy-files.yml@312aaab296060ff89d7f798dcab59f019bea6e02
 
   run-clang-tidy:
     name: Run clang tidy
     needs: [determine-files]
     if: ${{ always() && !cancelled() && (!inputs.check_only_changed || needs.determine-files.outputs.cpp_changed_files != '' || needs.determine-files.outputs.clang_tidy_config_changed == 'true') }}
     runs-on: ["self-hosted", "Linux", "X64", "heavy"]
-    container: "ghcr.io/xrplf/xrpld/nix-debian:sha-8abe82e"
+    container: "ghcr.io/xrplf/xrpld/nix-debian:sha-63ffdc3"
     permissions:
       contents: read
       issues: write
@@ -45,7 +45,7 @@ jobs:
         uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
 
       - name: Prepare runner
-        uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
+        uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8
         with:
           enable_ccache: false
 

.github/workflows/reusable-package.yml

@@ -68,31 +68,6 @@ jobs:
     timeout-minutes: 30
 
     steps:
-      # Packaging runs in a vanilla distro image, so the tooling has to come
-      # from the distro's archive: debhelper for deb, rpm-build (and the
-      # systemd / find-debuginfo macros it depends on) for rpm. Run this
-      # before actions/checkout so the latter can use git (real history) for
-      # build_pkg.sh's SOURCE_DATE_EPOCH; otherwise it falls back to a tarball
-      # download and the timestamp comes from wall-clock time.
-      - name: Install packaging tooling (deb)
-        if: ${{ matrix.distro == 'debian' }}
-        run: |
-          export DEBIAN_FRONTEND=noninteractive
-          apt-get update
-          apt-get install -y --no-install-recommends \
-              ca-certificates \
-              debhelper \
-              git
-
-      - name: Install packaging tooling (rpm)
-        if: ${{ matrix.distro == 'rhel' }}
-        run: |
-          dnf install -y --setopt=install_weak_deps=False \
-              git \
-              rpm-build \
-              redhat-rpm-config \
-              systemd-rpm-macros
-
       - name: Checkout repository
         uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
 

.github/workflows/reusable-upload-recipe.yml

@@ -40,7 +40,7 @@ defaults:
 jobs:
   upload:
     runs-on: ubuntu-latest
-    container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-8abe82e
+    container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-63ffdc3
     steps:
       - name: Checkout repository
         uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3

.github/workflows/upload-conan-deps.yml

@@ -67,7 +67,7 @@ jobs:
         uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
 
       - name: Prepare runner
-        uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
+        uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8
         with:
           enable_ccache: false
 

include/xrpl/basics/TaggedCache.h

@@ -9,7 +9,6 @@
 #include <xrpl/beast/insight/Insight.h>
 
 #include <atomic>
-#include <cstddef>
 #include <functional>
 #include <mutex>
 #include <thread>
@@ -18,22 +17,6 @@
 
 namespace xrpl {
 
-namespace detail {
-
-// Replace-policy tags selecting how TaggedCache::canonicalizeImpl resolves a
-// collision when the key already exists (defined in TaggedCache.ipp):
-//   - ReplaceCached: always replace the cached value with `data`. `data` is
-//     never written back and may be const.
-//   - ReplaceClient: keep the cached value and write it back into `data` (the
-//     client's pointer), which must therefore be writable.
-//   - ReplaceDynamically: call the supplied callback to decide per call; `data`
-//     is written back when the cached value is kept, so it must be writable.
-struct ReplaceCached;
-struct ReplaceClient;
-struct ReplaceDynamically;
-
-}  // namespace detail
-
 /** Map/cache combination.
     This class implements a cache and a map. The cache keeps objects alive
     in the map. The map allows multiple code paths that reference objects
@@ -113,32 +96,6 @@ public:
     bool
     del(key_type const& key, bool valid);
 
-private:
-    // Selects the `data` parameter type of canonicalizeImpl from the replace
-    // policy: const for detail::ReplaceCached (never written back), otherwise
-    // writable.
-    template <typename Policy>
-    using CanonicalizeClientPointerType = std::conditional_t<
-        std::is_same_v<detail::ReplaceCached, Policy>,
-        SharedPointerType const&,
-        SharedPointerType&>;
-
-    /** Shared implementation of the canonicalize family.
-
-        `policy` selects how a collision is resolved when `key` already exists:
-        detail::ReplaceCached, detail::ReplaceClient or
-        detail::ReplaceDynamically. For ReplaceDynamically `replaceCallback` is
-        invoked with the existing strong pointer and returns whether to replace
-        the cached value with `data`; for the tag policies it is unused.
-    */
-    template <class Policy, class Callback = std::nullptr_t>
-    bool
-    canonicalizeImpl(
-        key_type const& key,
-        CanonicalizeClientPointerType<Policy> data,
-        Policy policy,
-        Callback&& replaceCallback = nullptr);
-
 public:
     /** Replace aliased objects with originals.
 
@@ -147,49 +104,19 @@ public:
         This routine eliminates the duplicate and performs a replacement
         on the callers shared pointer if needed.
 
-        `replaceCallback` is a callable taking the existing strong pointer and
-        returning whether to replace the cached value with `data` (true) or to
-        keep the cached value and write it back into `data` (false). Because the
-        write-back case mutates `data`, `data` must be writable.
-
         @param key The key corresponding to the object
         @param data A shared pointer to the data corresponding to the object.
-        @param replaceCallback A callable (existing strong pointer -> bool).
+        @param replace Function that decides if cache should be replaced
 
         @return `true` If the key already existed.
     */
-    template <class Callback>
+    template <class R>
     bool
-    canonicalize(key_type const& key, SharedPointerType& data, Callback&& replaceCallback);
+    canonicalize(key_type const& key, SharedPointerType& data, R&& replaceCallback);
 
-    /** Insert/update the canonical entry for `key`, always replacing the
-        cached value with `data`.
-
-        If an entry already exists for `key`, the cached value is unconditionally
-        replaced with `data`; otherwise `data` is inserted. `data` is never
-        written back, so it may be const.
-
-        @param key The key corresponding to the object.
-        @param data A shared pointer to the data corresponding to the object.
-
-        @return `true` If the key already existed.
-    */
     bool
     canonicalizeReplaceCache(key_type const& key, SharedPointerType const& data);
 
-    /** Insert the canonical entry for `key`, keeping any existing cached value.
-
-        If an entry already exists for `key`, the cached value is kept and
-        written back into `data` so the caller ends up with the canonical
-        object; otherwise `data` is inserted. Because `data` may be overwritten
-        it must be writable.
-
-        @param key The key corresponding to the object.
-        @param data A shared pointer to the data corresponding to the object;
-                    updated to the canonical value when one already exists.
-
-        @return `true` If the key already existed.
-    */
     bool
     canonicalizeReplaceClient(key_type const& key, SharedPointerType& data);
 

include/xrpl/basics/TaggedCache.ipp

@@ -5,30 +5,6 @@
 
 namespace xrpl {
 
-namespace detail {
-
-// Replace-policy tags selecting how TaggedCache::canonicalizeImpl resolves a
-// collision when the key already exists:
-//   - ReplaceCached: always replace the cached value with `data`. `data` is
-//     never written back and may be const.
-//   - ReplaceClient: keep the cached value and write it back into `data` (the
-//     client's pointer), which must therefore be writable.
-//   - ReplaceDynamically: call the supplied callback to decide per call; `data`
-//     is written back when the cached value is kept, so it must be writable.
-struct ReplaceCached
-{
-};
-
-struct ReplaceClient
-{
-};
-
-struct ReplaceDynamically
-{
-};
-
-}  // namespace detail
-
 template <
     class Key,
     class T,
@@ -324,29 +300,13 @@ template <
     class Hash,
     class KeyEqual,
     class Mutex>
-template <class Policy, class Callback>
+template <class R>
 inline bool
 TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
-    canonicalizeImpl(
-        key_type const& key,
-        CanonicalizeClientPointerType<Policy> data,
-        [[maybe_unused]] Policy policy,
-        [[maybe_unused]] Callback&& replaceCallback)
+    canonicalize(key_type const& key, SharedPointerType& data, R&& replaceCallback)
 {
     // Return canonical value, store if needed, refresh in cache
     // Return values: true=we had the data already
-
-    // `Policy` is one of:
-    //   - detail::ReplaceCached: always replace the cached value with `data`;
-    //     `data` is never written back and may be const.
-    //   - detail::ReplaceClient: keep the cached value and write it back into
-    //     `data` (the client's pointer), which must therefore be writable.
-    //   - detail::ReplaceDynamically: call `replaceCallback` to decide at run
-    //     time; `data` must be writable.
-    // For the latter two the write-back below requires a mutable `data`, so
-    // passing a const argument is a compile error.
-    constexpr bool replaceCached = std::is_same_v<Policy, detail::ReplaceCached>;
-
     std::scoped_lock const lock(mutex_);
 
     auto cit = cache_.find(key);
@@ -364,14 +324,13 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
     Entry& entry = cit->second;
     entry.touch(clock_.now());
 
-    auto shouldReplaceCached = [&] {
-        if constexpr (replaceCached)
+    auto shouldReplace = [&] {
+        if constexpr (std::is_invocable_r_v<bool, R>)
         {
-            return true;
-        }
-        else if constexpr (std::is_same_v<Policy, detail::ReplaceClient>)
-        {
-            return false;
+            // The reason for this extra complexity is for intrusive
+            // strong/weak combo getting a strong is relatively expensive
+            // and not needed for many cases.
+            return replaceCallback();
         }
         else
         {
@@ -381,11 +340,11 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
 
     if (entry.isCached())
     {
-        if (shouldReplaceCached())
+        if (shouldReplace())
         {
             entry.ptr = data;
         }
-        else if constexpr (!replaceCached)
+        else
         {
             data = entry.ptr.getStrong();
         }
@@ -397,11 +356,11 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
 
     if (cachedData)
     {
-        if (shouldReplaceCached())
+        if (shouldReplace())
         {
             entry.ptr = data;
         }
-        else if constexpr (!replaceCached)
+        else
         {
             entry.ptr.convertToStrong();
             data = cachedData;
@@ -417,24 +376,6 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
     return false;
 }
 
-template <
-    class Key,
-    class T,
-    bool IsKeyCache,
-    class SharedWeakUnionPointer,
-    class SharedPointerType,
-    class Hash,
-    class KeyEqual,
-    class Mutex>
-template <class Callback>
-inline bool
-TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
-    canonicalize(key_type const& key, SharedPointerType& data, Callback&& replaceCallback)
-{
-    return canonicalizeImpl(
-        key, data, detail::ReplaceDynamically{}, std::forward<Callback>(replaceCallback));
-}
-
 template <
     class Key,
     class T,
@@ -448,7 +389,7 @@ inline bool
 TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
     canonicalizeReplaceCache(key_type const& key, SharedPointerType const& data)
 {
-    return canonicalizeImpl(key, data, detail::ReplaceCached{});
+    return canonicalize(key, const_cast<SharedPointerType&>(data), []() { return true; });
 }
 
 template <
@@ -464,7 +405,7 @@ inline bool
 TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash, KeyEqual, Mutex>::
     canonicalizeReplaceClient(key_type const& key, SharedPointerType& data)
 {
-    return canonicalizeImpl(key, data, detail::ReplaceClient{});
+    return canonicalize(key, data, []() { return false; });
 }
 
 template <

src/test/basics/TaggedCache_test.cpp

@@ -1,7 +1,5 @@
 #include <test/unit_test/SuiteJournal.h>
 
-#include <xrpl/basics/IntrusivePointer.h>
-#include <xrpl/basics/IntrusiveRefCounts.h>
 #include <xrpl/basics/TaggedCache.h>
 #include <xrpl/basics/TaggedCache.ipp>  // IWYU pragma: keep
 #include <xrpl/basics/chrono.h>
@@ -10,7 +8,6 @@
 #include <xrpl/protocol/Protocol.h>
 
 #include <memory>
-#include <utility>
 
 namespace xrpl {
 
@@ -136,113 +133,6 @@ public:
             BEAST_EXPECT(c.getCacheSize() == 0);
             BEAST_EXPECT(c.getTrackSize() == 0);
         }
-        {
-            BEAST_EXPECT(!c.insert(5, "five"));
-            BEAST_EXPECT(c.getCacheSize() == 1);
-            BEAST_EXPECT(c.size() == 1);
-
-            {
-                auto const p1 = c.fetch(5);
-                BEAST_EXPECT(p1 != nullptr);
-                BEAST_EXPECT(c.getCacheSize() == 1);
-                BEAST_EXPECT(c.size() == 1);
-
-                // Advance the clock a lot
-                ++clock;
-                c.sweep();
-                BEAST_EXPECT(c.getCacheSize() == 0);
-                BEAST_EXPECT(c.size() == 1);
-
-                auto p2 = std::make_shared<std::string>("five_2");
-                BEAST_EXPECT(c.canonicalizeReplaceCache(5, p2));
-                BEAST_EXPECT(c.getCacheSize() == 1);
-                BEAST_EXPECT(c.size() == 1);
-                // Make sure the caller's original pointer is unchanged
-                BEAST_EXPECT(p1.get() != p2.get());
-                BEAST_EXPECT(*p2 == "five_2");
-
-                auto const p3 = c.fetch(5);
-                BEAST_EXPECT(p3 != nullptr);
-                BEAST_EXPECT(p3.get() == p2.get());
-                BEAST_EXPECT(p3.get() != p1.get());
-            }
-
-            ++clock;
-            c.sweep();
-            BEAST_EXPECT(c.getCacheSize() == 0);
-            BEAST_EXPECT(c.size() == 0);
-        }
-
-        {
-            testcase("intrptr");
-
-            struct MyRefCountObject : IntrusiveRefCounts
-            {
-                std::string data;
-
-                // Needed to support weak intrusive pointers
-                virtual void
-                partialDestructor() {};
-
-                MyRefCountObject() = default;
-                explicit MyRefCountObject(std::string data) : data(std::move(data))
-                {
-                }
-
-                bool
-                operator==(std::string const& other) const
-                {
-                    return data == other;
-                }
-            };
-
-            using IntrPtrCache = TaggedCache<
-                Key,
-                MyRefCountObject,
-                /*IsKeyCache*/ false,
-                intr_ptr::SharedWeakUnionPtr<MyRefCountObject>,
-                intr_ptr::SharedPtr<MyRefCountObject>>;
-
-            IntrPtrCache intrPtrCache("IntrPtrTest", 1, 1s, clock, journal);
-
-            intrPtrCache.canonicalizeReplaceCache(1, intr_ptr::makeShared<MyRefCountObject>("one"));
-            BEAST_EXPECT(intrPtrCache.getCacheSize() == 1);
-            BEAST_EXPECT(intrPtrCache.size() == 1);
-
-            {
-                {
-                    intrPtrCache.canonicalizeReplaceCache(
-                        1, intr_ptr::makeShared<MyRefCountObject>("one_replaced"));
-
-                    auto p = intrPtrCache.fetch(1);
-                    BEAST_EXPECT(*p == "one_replaced");
-
-                    // Advance the clock a lot
-                    ++clock;
-                    intrPtrCache.sweep();
-                    BEAST_EXPECT(intrPtrCache.getCacheSize() == 0);
-                    BEAST_EXPECT(intrPtrCache.size() == 1);
-
-                    intrPtrCache.canonicalizeReplaceCache(
-                        1, intr_ptr::makeShared<MyRefCountObject>("one_replaced_2"));
-
-                    auto p2 = intrPtrCache.fetch(1);
-                    BEAST_EXPECT(*p2 == "one_replaced_2");
-
-                    intrPtrCache.del(1, true);
-                }
-
-                intrPtrCache.canonicalizeReplaceCache(
-                    1, intr_ptr::makeShared<MyRefCountObject>("one_replaced_3"));
-                auto p3 = intrPtrCache.fetch(1);
-                BEAST_EXPECT(*p3 == "one_replaced_3");
-            }
-
-            ++clock;
-            intrPtrCache.sweep();
-            BEAST_EXPECT(intrPtrCache.getCacheSize() == 0);
-            BEAST_EXPECT(intrPtrCache.size() == 0);
-        }
     }
 };
 

src/test/overlay/ProtocolVersion_test.cpp

@@ -36,7 +36,6 @@ public:
         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");
 
         {
@@ -59,11 +58,10 @@ public:
             testcase("Protocol version negotiation");
 
             BEAST_EXPECT(negotiateProtocolVersion("RTXP/1.2") == std::nullopt);
-            BEAST_EXPECT(
-                negotiateProtocolVersion("RTXP/1.2, XRPL/2.0, XRPL/2.1") == makeProtocol(2, 1));
+            BEAST_EXPECT(negotiateProtocolVersion("RTXP/1.2, XRPL/2.0, XRPL/2.1") == std::nullopt);
             BEAST_EXPECT(negotiateProtocolVersion("XRPL/2.2") == makeProtocol(2, 2));
             BEAST_EXPECT(
-                negotiateProtocolVersion("RTXP/1.2, XRPL/2.2, XRPL/2.3, XRPL/999.999") ==
+                negotiateProtocolVersion("RTXP/1.2, XRPL/2.1, XRPL/2.2, XRPL/2.3, XRPL/999.999") ==
                 makeProtocol(2, 2));
             BEAST_EXPECT(negotiateProtocolVersion("XRPL/999.999, WebSocket/1.0") == std::nullopt);
             BEAST_EXPECT(negotiateProtocolVersion("") == std::nullopt);

src/tests/libxrpl/CMakeLists.txt

@@ -53,4 +53,4 @@ foreach(module IN LISTS test_modules)
     )
 endforeach()
 
-gtest_discover_tests(xrpl_tests)
+gtest_discover_tests(xrpl_tests DISCOVERY_TIMEOUT 60)

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

@@ -727,17 +727,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;
+    // v1 messages are no longer supported.
+    std::size_t const messageVersion = 2;
     auto const [newPeerSequence, numVLs] = buildValidatorListMessages(
         messageVersion, peerSequence, maxSequence, rawVersion, rawManifest, blobInfos, messages);
     if (newPeerSequence != 0u)
@@ -766,24 +757,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();
         }
     }
 }
@@ -858,16 +836,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.
@@ -887,8 +858,6 @@ ValidatorList::broadcastBlobs(
                 {
                     if (blobInfos.empty())
                         buildBlobInfos(blobInfos, lists);
-                    auto const v2 =
-                        peer->supportsFeature(ProtocolFeature::ValidatorList2Propagation);
                     sendValidatorList(
                         *peer,
                         peerSequence,
@@ -897,11 +866,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

@@ -14,8 +14,6 @@ class Charge;
 }  // namespace Resource
 
 enum class ProtocolFeature {
-    ValidatorListPropagation,
-    ValidatorList2Propagation,
     LedgerReplay,
 };
 

src/xrpld/overlay/detail/PeerImp.cpp

@@ -545,10 +545,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::LedgerReplay:
             return ledgerReplayEnabled_;
     }
@@ -892,7 +888,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,
@@ -2275,14 +2271,6 @@ 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));
     }
@@ -2299,14 +2287,6 @@ 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/ProtocolVersion.cpp

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