Improve log message

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

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

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

@@ -22,8 +22,7 @@ on:
   workflow_dispatch:
 
 concurrency:
-  # 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 }}
+  group: ${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: true
 
 defaults:

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

@@ -20,8 +20,7 @@ on:
   workflow_dispatch:
 
 concurrency:
-  # 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 }}
+  group: ${{ github.workflow }}-${{ 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@312aaab296060ff89d7f798dcab59f019bea6e02
+    uses: XRPLF/actions/.github/workflows/pre-commit.yml@cba1f0891650baf1a9c88624dc2d72573be2eb81
     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-63ffdc3
+    container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-8abe82e
     steps:
       - name: Checkout repository
         uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
 
       - name: Prepare runner
-        uses: XRPLF/actions/prepare-runner@c47daebb2f9db64ffbac71b47d68a661498d5ce8
+        uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
         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@c47daebb2f9db64ffbac71b47d68a661498d5ce8
+        uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
         with:
           enable_ccache: ${{ inputs.ccache_enabled }}
 
@@ -370,7 +370,7 @@ jobs:
 
       - name: Upload coverage report
         if: ${{ github.repository == 'XRPLF/rippled' && !inputs.build_only && env.COVERAGE_ENABLED == 'true' }}
-        uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0
+        uses: codecov/codecov-action@e79a6962e0d4c0c17b229090214935d2e33f8354 # v6.0.1
         with:
           disable_search: true
           disable_telem: true

.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@312aaab296060ff89d7f798dcab59f019bea6e02
+    uses: XRPLF/actions/.github/workflows/determine-tidy-files.yml@224f3c48d3014d082a1129237b8291ff0b0a331f
 
   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-63ffdc3"
+    container: "ghcr.io/xrplf/xrpld/nix-debian:sha-8abe82e"
     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@c47daebb2f9db64ffbac71b47d68a661498d5ce8
+        uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
         with:
           enable_ccache: false
 

.github/workflows/reusable-package.yml

@@ -68,6 +68,31 @@ 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-63ffdc3
+    container: ghcr.io/xrplf/xrpld/nix-ubuntu:sha-8abe82e
     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@c47daebb2f9db64ffbac71b47d68a661498d5ce8
+        uses: XRPLF/actions/prepare-runner@90f11ee655d1687824fb8793db770477d52afbab
         with:
           enable_ccache: false
 

src/test/overlay/ProtocolVersion_test.cpp

@@ -36,6 +36,7 @@ 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");
 
         {
@@ -58,10 +59,11 @@ 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") == std::nullopt);
+            BEAST_EXPECT(
+                negotiateProtocolVersion("RTXP/1.2, XRPL/2.0, XRPL/2.1") == makeProtocol(2, 1));
             BEAST_EXPECT(negotiateProtocolVersion("XRPL/2.2") == makeProtocol(2, 2));
             BEAST_EXPECT(
-                negotiateProtocolVersion("RTXP/1.2, XRPL/2.1, XRPL/2.2, XRPL/2.3, XRPL/999.999") ==
+                negotiateProtocolVersion("RTXP/1.2, 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 DISCOVERY_TIMEOUT 60)
+gtest_discover_tests(xrpl_tests)

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

@@ -727,8 +727,17 @@ ValidatorList::sendValidatorList(
     HashRouter& hashRouter,
     beast::Journal j)
 {
-    // v1 messages are no longer supported.
-    std::size_t const messageVersion = 2;
+    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);
     if (newPeerSequence != 0u)
@@ -757,11 +766,24 @@ ValidatorList::sendValidatorList(
             "xrpl::ValidatorList::sendValidatorList : sent or one message");
         if (sent)
         {
-            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();
+            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();
+            }
         }
     }
 }
@@ -836,9 +858,16 @@ ValidatorList::broadcastBlobs(
 
     if (toSkip)
     {
-        // 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.
+        // 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.
         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.
@@ -858,6 +887,8 @@ ValidatorList::broadcastBlobs(
                 {
                     if (blobInfos.empty())
                         buildBlobInfos(blobInfos, lists);
+                    auto const v2 =
+                        peer->supportsFeature(ProtocolFeature::ValidatorList2Propagation);
                     sendValidatorList(
                         *peer,
                         peerSequence,
@@ -866,10 +897,11 @@ ValidatorList::broadcastBlobs(
                         lists.rawVersion,
                         lists.rawManifest,
                         blobInfos,
-                        messages2[peerSequence],
+                        v2 ? messages2[peerSequence] : messages1,
                         hashRouter,
                         j);
-                    // Don't send it next time.
+                    // Even if the peer doesn't support the messages,
+                    // suppress it so it'll be ignored next time.
                     hashRouter.addSuppressionPeer(hash, peer->id());
                 }
             }

src/xrpld/overlay/Peer.h

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

src/xrpld/overlay/detail/PeerImp.cpp

@@ -61,6 +61,7 @@
 #include <xrpl/server/Handoff.h>
 #include <xrpl/server/LoadFeeTrack.h>
 #include <xrpl/server/NetworkOPs.h>
+#include <xrpl/shamap/SHAMap.h>
 #include <xrpl/shamap/SHAMapNodeID.h>
 #include <xrpl/tx/apply.h>
 
@@ -545,6 +546,10 @@ 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_;
     }
@@ -888,7 +893,7 @@ PeerImp::doProtocolStart()
     onReadMessage(error_code(), 0);
 
     // Send all the validator lists that have been loaded
-    if (inbound_)
+    if (inbound_ && supportsFeature(ProtocolFeature::ValidatorListPropagation))
     {
         app_.getValidators().forEachAvailable(
             [&](std::string const& manifest,
@@ -1489,23 +1494,12 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMGetLedger> const& m)
         }
     }
 
-    // Verify ledger node IDs
-    if (itype != protocol::liBASE)
+    // Verify ledger node counts. Full parsing of the node IDs is deferred to the job, so the I/O
+    // thread is not burdened with SHAMapNodeID deserialization for every TMGetLedger message.
+    if (itype != protocol::liBASE && m->nodeids_size() <= 0)
     {
-        if (m->nodeids_size() <= 0)
-        {
-            badData("Invalid ledger node IDs");
-            return;
-        }
-
-        for (auto const& nodeId : m->nodeids())
-        {
-            if (deserializeSHAMapNodeID(nodeId) == std::nullopt)
-            {
-                badData("Invalid SHAMap node ID");
-                return;
-            }
-        }
+        badData("Invalid ledger node IDs");
+        return;
     }
 
     // Verify query type
@@ -1525,11 +1519,40 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMGetLedger> const& m)
         }
     }
 
-    // Queue a job to process the request
+    // Queue a job to process the request.
     std::weak_ptr<PeerImp> const weak = shared_from_this();
-    app_.getJobQueue().addJob(JtLedgerReq, "RcvGetLedger", [weak, m]() {
-        if (auto peer = weak.lock())
-            peer->processLedgerRequest(m);
+    app_.getJobQueue().addJob(JtLedgerReq, "RcvGetLedger", [weak, m, itype]() {
+        auto peer = weak.lock();
+        if (!peer)
+            return;
+
+        std::vector<SHAMapNodeID> nodeIDs;
+        if (itype != protocol::liBASE)
+        {
+            nodeIDs.reserve(std::min(m->nodeids_size(), Tuning::kSoftMaxReplyNodes));
+            for (auto const& nodeId : m->nodeids())
+            {
+                if (nodeIDs.size() >= Tuning::kSoftMaxReplyNodes)
+                {
+                    // Charge the peer for sending too many node IDs, but continue processing the
+                    // received node IDs up to the limit. If the request is legitimate then at least
+                    // they will get a response and won't have to resend these nodes in their next
+                    // request.
+                    peer->charge(
+                        Resource::kFeeModerateBurdenPeer, "TMGetLedger: too many node IDs");
+                    break;
+                }
+                auto parsed = deserializeSHAMapNodeID(nodeId);
+                if (!parsed)
+                {
+                    peer->charge(Resource::kFeeInvalidData, "TMGetLedger: Invalid node ID");
+                    return;
+                }
+                nodeIDs.push_back(std::move(*parsed));
+            }
+        }
+
+        peer->processLedgerRequest(m, std::move(nodeIDs));
     });
 }
 
@@ -2271,6 +2294,14 @@ 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));
     }
@@ -2287,6 +2318,14 @@ 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())
@@ -3222,7 +3261,9 @@ PeerImp::getTxSet(std::shared_ptr<protocol::TMGetLedger> const& m) const
 }
 
 void
-PeerImp::processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m)
+PeerImp::processLedgerRequest(
+    std::shared_ptr<protocol::TMGetLedger> const& m,
+    std::vector<SHAMapNodeID> nodeIDs)
 {
     // Do not resource charge a peer responding to a relay
     if (!m->has_requestcookie())
@@ -3307,26 +3348,23 @@ PeerImp::processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m)
     }
 
     // Add requested node data to reply
-    if (m->nodeids_size() > 0)
+    if (!nodeIDs.empty())
     {
         std::uint32_t const defaultDepth = isHighLatency() ? 2 : 1;
         auto const queryDepth{m->has_querydepth() ? m->querydepth() : defaultDepth};
 
         std::vector<std::pair<SHAMapNodeID, Blob>> data;
-
-        for (int i = 0;
-             i < m->nodeids_size() && ledgerData.nodes_size() < Tuning::kSoftMaxReplyNodes;
-             ++i)
+        data.reserve(Tuning::kSoftMaxReplyNodes);
+        for (auto const& nodeID : nodeIDs)
         {
-            auto const shaMapNodeId{deserializeSHAMapNodeID(m->nodeids(i))};
+            if (ledgerData.nodes_size() >= Tuning::kSoftMaxReplyNodes)
+                break;
 
             data.clear();
-            data.reserve(Tuning::kSoftMaxReplyNodes);
 
             try
             {
-                // NOLINTNEXTLINE(bugprone-unchecked-optional-access) nodeids checked in onGetLedger
-                if (map->getNodeFat(*shaMapNodeId, data, fatLeaves, queryDepth))
+                if (map->getNodeFat(nodeID, data, fatLeaves, queryDepth))
                 {
                     JLOG(pJournal_.trace())
                         << "processLedgerRequest: getNodeFat got " << data.size() << " nodes";
@@ -3335,9 +3373,9 @@ PeerImp::processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m)
                     {
                         if (ledgerData.nodes_size() >= Tuning::kHardMaxReplyNodes)
                             break;
-                        protocol::TMLedgerNode* node{ledgerData.add_nodes()};
-                        node->set_nodeid(d.first.getRawString());
-                        node->set_nodedata(d.second.data(), d.second.size());
+                        protocol::TMLedgerNode* ledgerNode{ledgerData.add_nodes()};
+                        ledgerNode->set_nodeid(d.first.getRawString());
+                        ledgerNode->set_nodedata(d.second.data(), d.second.size());
                     }
                 }
                 else
@@ -3376,14 +3414,14 @@ PeerImp::processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m)
                     info += ", no hash specified";
 
                 JLOG(pJournal_.warn())
-                    << "processLedgerRequest: getNodeFat with nodeId " << *shaMapNodeId
+                    << "processLedgerRequest: getNodeFat with nodeId " << nodeID
                     << " and ledger info type " << info << " throws exception: " << e.what();
             }
         }
 
         JLOG(pJournal_.info()) << "processLedgerRequest: Got request for " << m->nodeids_size()
-                               << " nodes at depth " << queryDepth << ", return "
-                               << ledgerData.nodes_size() << " nodes";
+                               << " node IDs (processed " << nodeIDs.size() << ") at depth "
+                               << queryDepth << ", return " << ledgerData.nodes_size() << " nodes";
     }
 
     if (ledgerData.nodes_size() == 0)

src/xrpld/overlay/detail/PeerImp.h

@@ -14,6 +14,7 @@
 #include <xrpl/protocol/STTx.h>
 #include <xrpl/protocol/STValidation.h>
 #include <xrpl/resource/Fees.h>
+#include <xrpl/shamap/SHAMapNodeID.h>
 
 #include <boost/circular_buffer.hpp>
 #include <boost/endian/conversion.hpp>
@@ -623,7 +624,9 @@ private:
     getTxSet(std::shared_ptr<protocol::TMGetLedger> const& m) const;
 
     void
-    processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m);
+    processLedgerRequest(
+        std::shared_ptr<protocol::TMGetLedger> const& m,
+        std::vector<SHAMapNodeID> nodeIDs);
 };
 
 //------------------------------------------------------------------------------

src/xrpld/overlay/detail/ProtocolVersion.cpp

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