what did i ever do to you clang-tidy

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

src/test/app/PermissionedDomains_test.cpp

@@ -8,18 +8,21 @@
 #include <test/jtx/pay.h>
 #include <test/jtx/permissioned_domains.h>
 #include <test/jtx/ter.h>
+#include <test/jtx/ticket.h>
 #include <test/jtx/txflags.h>
 
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/json/json_value.h>
 #include <xrpl/protocol/Feature.h>
+#include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/TxFlags.h>
 #include <xrpl/protocol/jss.h>
 
 #include <cstddef>
+#include <cstdint>
 #include <exception>
 #include <map>
 #include <optional>
@@ -526,6 +529,47 @@ class PermissionedDomains_test : public beast::unit_test::Suite
         BEAST_EXPECT(env.ownerCount(alice) == 1);
     }
 
+    void
+    testTicket(FeatureBitset features)
+    {
+        testcase("Tickets");
+
+        using namespace test::jtx;
+
+        Env env(*this, features);
+        Account const alice("alice");
+        env.fund(XRP(1000), alice);
+
+        pdomain::Credentials const credentials{
+            {.issuer = alice, .credType = "credential1"},
+        };
+
+        std::uint32_t seq{env.seq(alice)};
+        env(ticket::create(alice, 2));
+
+        {
+            env(pdomain::setTx(alice, credentials), ticket::Use(++seq));
+            auto domain = pdomain::getNewDomain(env.meta());
+            if (features[fixCleanup3_1_3])
+            {
+                BEAST_EXPECT(domain == keylet::permissionedDomain(alice.id(), seq).key);
+            }
+            else
+            {
+                BEAST_EXPECT(domain == keylet::permissionedDomain(alice.id(), 0).key);
+            }
+        }
+
+        if (features[fixCleanup3_1_3])
+        {
+            env(pdomain::setTx(alice, credentials), ticket::Use(++seq));
+        }
+        else
+        {
+            env(pdomain::setTx(alice, credentials), ticket::Use(++seq), Ter(tefEXCEPTION));
+        }
+    }
+
 public:
     void
     run() override
@@ -540,6 +584,8 @@ public:
         testDelete(withFix_);
         testAccountReserve(withFeature_);
         testAccountReserve(withFix_);
+        testTicket(withFeature_);
+        testTicket(withFix_);
     }
 };
 

src/xrpld/overlay/detail/PeerImp.cpp

@@ -61,7 +61,6 @@
 #include <xrpl/resource/Gossip.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>
 
@@ -1478,12 +1477,23 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMGetLedger> const& m)
         }
     }
 
-    // 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)
+    // Verify ledger node IDs
+    if (itype != protocol::liBASE)
     {
-        badData("Invalid ledger node IDs");
-        return;
+        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;
+            }
+        }
     }
 
     // Verify query type
@@ -1503,40 +1513,11 @@ 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, 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 requesting 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));
+    app_.getJobQueue().addJob(JtLedgerReq, "RcvGetLedger", [weak, m]() {
+        if (auto peer = weak.lock())
+            peer->processLedgerRequest(m);
     });
 }
 
@@ -3305,9 +3286,7 @@ PeerImp::getTxSet(std::shared_ptr<protocol::TMGetLedger> const& m) const
 }
 
 void
-PeerImp::processLedgerRequest(
-    std::shared_ptr<protocol::TMGetLedger> const& m,
-    std::vector<SHAMapNodeID> nodeIDs)
+PeerImp::processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m)
 {
     // Do not resource charge a peer responding to a relay
     if (!m->has_requestcookie())
@@ -3392,23 +3371,26 @@ PeerImp::processLedgerRequest(
     }
 
     // Add requested node data to reply
-    if (!nodeIDs.empty())
+    if (m->nodeids_size() > 0)
     {
         std::uint32_t const defaultDepth = isHighLatency() ? 2 : 1;
         auto const queryDepth{m->has_querydepth() ? m->querydepth() : defaultDepth};
 
         std::vector<std::pair<SHAMapNodeID, Blob>> data;
-        data.reserve(Tuning::kSoftMaxReplyNodes);
-        for (auto const& nodeID : nodeIDs)
+
+        for (int i = 0;
+             i < m->nodeids_size() && ledgerData.nodes_size() < Tuning::kSoftMaxReplyNodes;
+             ++i)
         {
-            if (ledgerData.nodes_size() >= Tuning::kSoftMaxReplyNodes)
-                break;
+            auto const shaMapNodeId{deserializeSHAMapNodeID(m->nodeids(i))};
 
             data.clear();
+            data.reserve(Tuning::kSoftMaxReplyNodes);
 
             try
             {
-                if (map->getNodeFat(nodeID, data, fatLeaves, queryDepth))
+                // NOLINTNEXTLINE(bugprone-unchecked-optional-access) nodeids checked in onGetLedger
+                if (map->getNodeFat(*shaMapNodeId, data, fatLeaves, queryDepth))
                 {
                     JLOG(pJournal_.trace())
                         << "processLedgerRequest: getNodeFat got " << data.size() << " nodes";
@@ -3417,9 +3399,9 @@ PeerImp::processLedgerRequest(
                     {
                         if (ledgerData.nodes_size() >= Tuning::kHardMaxReplyNodes)
                             break;
-                        protocol::TMLedgerNode* ledgerNode{ledgerData.add_nodes()};
-                        ledgerNode->set_nodeid(d.first.getRawString());
-                        ledgerNode->set_nodedata(d.second.data(), d.second.size());
+                        protocol::TMLedgerNode* node{ledgerData.add_nodes()};
+                        node->set_nodeid(d.first.getRawString());
+                        node->set_nodedata(d.second.data(), d.second.size());
                     }
                 }
                 else
@@ -3458,13 +3440,13 @@ PeerImp::processLedgerRequest(
                     info += ", no hash specified";
 
                 JLOG(pJournal_.warn())
-                    << "processLedgerRequest: getNodeFat with nodeId " << nodeID
+                    << "processLedgerRequest: getNodeFat with nodeId " << *shaMapNodeId
                     << " and ledger info type " << info << " throws exception: " << e.what();
             }
         }
 
         JLOG(pJournal_.info()) << "processLedgerRequest: Got request for " << m->nodeids_size()
-                               << " node IDs at depth " << queryDepth << ", return "
+                               << " nodes at depth " << queryDepth << ", return "
                                << ledgerData.nodes_size() << " nodes";
     }
 

src/xrpld/overlay/detail/PeerImp.h

@@ -14,7 +14,6 @@
 #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>
@@ -630,9 +629,7 @@ private:
     getTxSet(std::shared_ptr<protocol::TMGetLedger> const& m) const;
 
     void
-    processLedgerRequest(
-        std::shared_ptr<protocol::TMGetLedger> const& m,
-        std::vector<SHAMapNodeID> nodeIDs);
+    processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m);
 
 protected:
     // Kept `protected` so test subclasses (see