Update CONTRIBUTING.md for XLS submission guidelines

Clarify the status of XLS in the pull request process.

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

@@ -130,7 +130,7 @@ jobs:
             --target "${CMAKE_TARGET}"
 
       - name: Upload rippled artifact (Linux)
-        if: ${{ runner.os == 'Linux' }}
+        if: ${{ github.repository_owner == 'XRPLF' && runner.os == 'Linux' }}
         uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
         env:
           BUILD_DIR: ${{ inputs.build_dir }}

CONTRIBUTING.md

@@ -47,13 +47,18 @@ choose the next available standard number, and open a discussion with an
 appropriate title to propose your draft standard.
 
 When you submit a pull request, please link the corresponding XLS in the
-description. An XLS still in draft status is considered a
+description. An XLS still in `Draft` status is considered a
 work-in-progress and open for discussion. Please allow time for
 questions, suggestions, and changes to the XLS draft. It is the
 responsibility of the XLS author to update the draft to match the final
 implementation when its corresponding pull request is merged, unless the
 author delegates that responsibility to others.
 
+Any amendment or major RPC change requires either a new XLS or an update
+to an existing XLS. Neither change will be released (in an amendment's
+case, marked as `Supported::yes`) until the corresponding XLS's status
+is `Final`.
+
 ## Before making a pull request
 
 (Or marking a draft pull request as ready.)

external/secp256k1/include/secp256k1.h

@@ -541,7 +541,7 @@ SECP256K1_API int secp256k1_ecdsa_signature_serialize_compact(
 /** Verify an ECDSA signature.
  *
  *  Returns: 1: correct signature
- *           0: incorrect or unparseable signature
+ *           0: incorrect or unparsable signature
  *  Args:    ctx:       pointer to a context object
  *  In:      sig:       the signature being verified.
  *           msghash32: the 32-byte message hash being verified.

include/xrpl/basics/SHAMapHash.h

@@ -2,6 +2,7 @@
 #define XRPL_BASICS_SHAMAP_HASH_H_INCLUDED
 
 #include <xrpl/basics/base_uint.h>
+#include <xrpl/basics/partitioned_unordered_map.h>
 
 #include <ostream>
 

include/xrpl/basics/SlabAllocator.h

@@ -159,7 +159,7 @@ public:
         @param count the number of items the slab allocator can allocate; note
                      that a count of 0 is valid and means that the allocator
                      is, effectively, disabled. This can be very useful in some
-                     contexts (e.g. when mimimal memory usage is needed) and
+                     contexts (e.g. when minimal memory usage is needed) and
                      allows for graceful failure.
      */
     constexpr explicit SlabAllocator(

include/xrpl/basics/TaggedCache.h

@@ -71,6 +71,9 @@ public:
     int
     getCacheSize() const;
 
+    int
+    getTrackSize() const;
+
     float
     getHitRate();
 
@@ -148,6 +151,9 @@ public:
     bool
     retrieve(key_type const& key, T& data);
 
+    mutex_type&
+    peekMutex();
+
     std::vector<key_type>
     getKeys() const;
 
@@ -168,14 +174,11 @@ public:
 
 private:
     SharedPointerType
-    initialFetch(key_type const& key);
+    initialFetch(key_type const& key, std::lock_guard<mutex_type> const& l);
 
     void
     collect_metrics();
 
-    Mutex&
-    lockPartition(key_type const& key) const;
-
 private:
     struct Stats
     {
@@ -278,8 +281,8 @@ private:
         [[maybe_unused]] clock_type::time_point const& now,
         typename KeyValueCacheType::map_type& partition,
         SweptPointersVector& stuffToSweep,
-        std::atomic<int>& allRemoval,
-        Mutex& partitionLock);
+        std::atomic<int>& allRemovals,
+        std::lock_guard<std::recursive_mutex> const&);
 
     [[nodiscard]] std::thread
     sweepHelper(
@@ -288,12 +291,14 @@ private:
         typename KeyOnlyCacheType::map_type& partition,
         SweptPointersVector&,
         std::atomic<int>& allRemovals,
-        Mutex& partitionLock);
+        std::lock_guard<std::recursive_mutex> const&);
 
     beast::Journal m_journal;
     clock_type& m_clock;
     Stats m_stats;
 
+    mutex_type mutable m_mutex;
+
     // Used for logging
     std::string m_name;
 
@@ -304,11 +309,10 @@ private:
     clock_type::duration const m_target_age;
 
     // Number of items cached
-    std::atomic<int> m_cache_count;
+    int m_cache_count;
     cache_type m_cache;  // Hold strong reference to recent objects
-    std::atomic<std::uint64_t> m_hits;
-    std::atomic<std::uint64_t> m_misses;
-    mutable std::vector<mutex_type> partitionLocks_;
+    std::uint64_t m_hits;
+    std::uint64_t m_misses;
 };
 
 }  // namespace ripple

include/xrpl/basics/TaggedCache.ipp

@@ -3,7 +3,6 @@
 
 #include <xrpl/basics/IntrusivePointer.ipp>
 #include <xrpl/basics/TaggedCache.h>
-#include <xrpl/beast/core/CurrentThreadName.h>
 
 namespace ripple {
 
@@ -42,7 +41,6 @@ inline TaggedCache<
     , m_hits(0)
     , m_misses(0)
 {
-    partitionLocks_ = std::vector<mutex_type>(m_cache.partitions());
 }
 
 template <
@@ -88,13 +86,8 @@ TaggedCache<
     KeyEqual,
     Mutex>::size() const
 {
-    std::size_t totalSize = 0;
-    for (size_t i = 0; i < partitionLocks_.size(); ++i)
-    {
-        std::lock_guard<Mutex> lock(partitionLocks_[i]);
-        totalSize += m_cache.map()[i].size();
-    }
-    return totalSize;
+    std::lock_guard lock(m_mutex);
+    return m_cache.size();
 }
 
 template <
@@ -117,7 +110,32 @@ TaggedCache<
     KeyEqual,
     Mutex>::getCacheSize() const
 {
-    return m_cache_count.load(std::memory_order_relaxed);
+    std::lock_guard lock(m_mutex);
+    return m_cache_count;
+}
+
+template <
+    class Key,
+    class T,
+    bool IsKeyCache,
+    class SharedWeakUnionPointer,
+    class SharedPointerType,
+    class Hash,
+    class KeyEqual,
+    class Mutex>
+inline int
+TaggedCache<
+    Key,
+    T,
+    IsKeyCache,
+    SharedWeakUnionPointer,
+    SharedPointerType,
+    Hash,
+    KeyEqual,
+    Mutex>::getTrackSize() const
+{
+    std::lock_guard lock(m_mutex);
+    return m_cache.size();
 }
 
 template <
@@ -140,10 +158,9 @@ TaggedCache<
     KeyEqual,
     Mutex>::getHitRate()
 {
-    auto const hits = m_hits.load(std::memory_order_relaxed);
-    auto const misses = m_misses.load(std::memory_order_relaxed);
-    float const total = float(hits + misses);
-    return hits * (100.0f / std::max(1.0f, total));
+    std::lock_guard lock(m_mutex);
+    auto const total = static_cast<float>(m_hits + m_misses);
+    return m_hits * (100.0f / std::max(1.0f, total));
 }
 
 template <
@@ -166,12 +183,9 @@ TaggedCache<
     KeyEqual,
     Mutex>::clear()
 {
-    for (auto& mutex : partitionLocks_)
-        mutex.lock();
+    std::lock_guard lock(m_mutex);
     m_cache.clear();
-    for (auto& mutex : partitionLocks_)
-        mutex.unlock();
-    m_cache_count.store(0, std::memory_order_relaxed);
+    m_cache_count = 0;
 }
 
 template <
@@ -194,9 +208,11 @@ TaggedCache<
     KeyEqual,
     Mutex>::reset()
 {
-    clear();
-    m_hits.store(0, std::memory_order_relaxed);
-    m_misses.store(0, std::memory_order_relaxed);
+    std::lock_guard lock(m_mutex);
+    m_cache.clear();
+    m_cache_count = 0;
+    m_hits = 0;
+    m_misses = 0;
 }
 
 template <
@@ -220,7 +236,7 @@ TaggedCache<
     KeyEqual,
     Mutex>::touch_if_exists(KeyComparable const& key)
 {
-    std::lock_guard<Mutex> lock(lockPartition(key));
+    std::lock_guard lock(m_mutex);
     auto const iter(m_cache.find(key));
     if (iter == m_cache.end())
     {
@@ -262,6 +278,8 @@ TaggedCache<
 
     auto const start = std::chrono::steady_clock::now();
     {
+        std::lock_guard lock(m_mutex);
+
         if (m_target_size == 0 ||
             (static_cast<int>(m_cache.size()) <= m_target_size))
         {
@@ -293,13 +311,12 @@ TaggedCache<
                 m_cache.map()[p],
                 allStuffToSweep[p],
                 allRemovals,
-                partitionLocks_[p]));
+                lock));
         }
         for (std::thread& worker : workers)
             worker.join();
 
-        int removals = allRemovals.load(std::memory_order_relaxed);
-        m_cache_count.fetch_sub(removals, std::memory_order_relaxed);
+        m_cache_count -= allRemovals;
     }
     // At this point allStuffToSweep will go out of scope outside the lock
     // and decrement the reference count on each strong pointer.
@@ -333,8 +350,7 @@ TaggedCache<
 {
     // Remove from cache, if !valid, remove from map too. Returns true if
     // removed from cache
-
-    std::lock_guard<Mutex> lock(lockPartition(key));
+    std::lock_guard lock(m_mutex);
 
     auto cit = m_cache.find(key);
 
@@ -347,7 +363,7 @@ TaggedCache<
 
     if (entry.isCached())
     {
-        m_cache_count.fetch_sub(1, std::memory_order_relaxed);
+        --m_cache_count;
         entry.ptr.convertToWeak();
         ret = true;
     }
@@ -385,16 +401,17 @@ TaggedCache<
 {
     // Return canonical value, store if needed, refresh in cache
     // Return values: true=we had the data already
+    std::lock_guard lock(m_mutex);
 
-    std::lock_guard<Mutex> lock(lockPartition(key));
     auto cit = m_cache.find(key);
+
     if (cit == m_cache.end())
     {
         m_cache.emplace(
             std::piecewise_construct,
             std::forward_as_tuple(key),
             std::forward_as_tuple(m_clock.now(), data));
-        m_cache_count.fetch_add(1, std::memory_order_relaxed);
+        ++m_cache_count;
         return false;
     }
 
@@ -443,12 +460,12 @@ TaggedCache<
             data = cachedData;
         }
 
-        m_cache_count.fetch_add(1, std::memory_order_relaxed);
+        ++m_cache_count;
         return true;
     }
 
     entry.ptr = data;
-    m_cache_count.fetch_add(1, std::memory_order_relaxed);
+    ++m_cache_count;
 
     return false;
 }
@@ -524,11 +541,10 @@ TaggedCache<
     KeyEqual,
     Mutex>::fetch(key_type const& key)
 {
-    std::lock_guard<Mutex> lock(lockPartition(key));
-
-    auto ret = initialFetch(key);
+    std::lock_guard<mutex_type> l(m_mutex);
+    auto ret = initialFetch(key, l);
     if (!ret)
-        m_misses.fetch_add(1, std::memory_order_relaxed);
+        ++m_misses;
     return ret;
 }
 
@@ -592,8 +608,8 @@ TaggedCache<
     Mutex>::insert(key_type const& key)
     -> std::enable_if_t<IsKeyCache, ReturnType>
 {
+    std::lock_guard lock(m_mutex);
     clock_type::time_point const now(m_clock.now());
-    std::lock_guard<Mutex> lock(lockPartition(key));
     auto [it, inserted] = m_cache.emplace(
         std::piecewise_construct,
         std::forward_as_tuple(key),
@@ -633,6 +649,29 @@ TaggedCache<
     return true;
 }
 
+template <
+    class Key,
+    class T,
+    bool IsKeyCache,
+    class SharedWeakUnionPointer,
+    class SharedPointerType,
+    class Hash,
+    class KeyEqual,
+    class Mutex>
+inline auto
+TaggedCache<
+    Key,
+    T,
+    IsKeyCache,
+    SharedWeakUnionPointer,
+    SharedPointerType,
+    Hash,
+    KeyEqual,
+    Mutex>::peekMutex() -> mutex_type&
+{
+    return m_mutex;
+}
+
 template <
     class Key,
     class T,
@@ -656,13 +695,10 @@ TaggedCache<
     std::vector<key_type> v;
 
     {
+        std::lock_guard lock(m_mutex);
         v.reserve(m_cache.size());
-        for (std::size_t i = 0; i < partitionLocks_.size(); ++i)
-        {
-            std::lock_guard<Mutex> lock(partitionLocks_[i]);
-            for (auto const& entry : m_cache.map()[i])
-                v.push_back(entry.first);
-        }
+        for (auto const& _ : m_cache)
+            v.push_back(_.first);
     }
 
     return v;
@@ -688,12 +724,11 @@ TaggedCache<
     KeyEqual,
     Mutex>::rate() const
 {
-    auto const hits = m_hits.load(std::memory_order_relaxed);
-    auto const misses = m_misses.load(std::memory_order_relaxed);
-    auto const tot = hits + misses;
+    std::lock_guard lock(m_mutex);
+    auto const tot = m_hits + m_misses;
     if (tot == 0)
-        return 0.0;
-    return double(hits) / tot;
+        return 0;
+    return double(m_hits) / tot;
 }
 
 template <
@@ -717,16 +752,18 @@ TaggedCache<
     KeyEqual,
     Mutex>::fetch(key_type const& digest, Handler const& h)
 {
-    std::lock_guard<Mutex> lock(lockPartition(digest));
-
-    if (auto ret = initialFetch(digest))
-        return ret;
+    {
+        std::lock_guard l(m_mutex);
+        if (auto ret = initialFetch(digest, l))
+            return ret;
+    }
 
     auto sle = h();
     if (!sle)
         return {};
 
-    m_misses.fetch_add(1, std::memory_order_relaxed);
+    std::lock_guard l(m_mutex);
+    ++m_misses;
     auto const [it, inserted] =
         m_cache.emplace(digest, Entry(m_clock.now(), std::move(sle)));
     if (!inserted)
@@ -753,10 +790,9 @@ TaggedCache<
     SharedPointerType,
     Hash,
     KeyEqual,
-    Mutex>::initialFetch(key_type const& key)
+    Mutex>::
+    initialFetch(key_type const& key, std::lock_guard<mutex_type> const& l)
 {
-    std::lock_guard<Mutex> lock(lockPartition(key));
-
     auto cit = m_cache.find(key);
     if (cit == m_cache.end())
         return {};
@@ -764,7 +800,7 @@ TaggedCache<
     Entry& entry = cit->second;
     if (entry.isCached())
     {
-        m_hits.fetch_add(1, std::memory_order_relaxed);
+        ++m_hits;
         entry.touch(m_clock.now());
         return entry.ptr.getStrong();
     }
@@ -772,13 +808,12 @@ TaggedCache<
     if (entry.isCached())
     {
         // independent of cache size, so not counted as a hit
-        m_cache_count.fetch_add(1, std::memory_order_relaxed);
+        ++m_cache_count;
         entry.touch(m_clock.now());
         return entry.ptr.getStrong();
     }
 
     m_cache.erase(cit);
-
     return {};
 }
 
@@ -807,11 +842,10 @@ TaggedCache<
     {
         beast::insight::Gauge::value_type hit_rate(0);
         {
-            auto const hits = m_hits.load(std::memory_order_relaxed);
-            auto const misses = m_misses.load(std::memory_order_relaxed);
-            auto const total = hits + misses;
+            std::lock_guard lock(m_mutex);
+            auto const total(m_hits + m_misses);
             if (total != 0)
-                hit_rate = (hits * 100) / total;
+                hit_rate = (m_hits * 100) / total;
         }
         m_stats.hit_rate.set(hit_rate);
     }
@@ -842,16 +876,12 @@ TaggedCache<
         typename KeyValueCacheType::map_type& partition,
         SweptPointersVector& stuffToSweep,
         std::atomic<int>& allRemovals,
-        Mutex& partitionLock)
+        std::lock_guard<std::recursive_mutex> const&)
 {
     return std::thread([&, this]() {
-        beast::setCurrentThreadName("sweep-KVCache");
-
         int cacheRemovals = 0;
         int mapRemovals = 0;
 
-        std::lock_guard<Mutex> lock(partitionLock);
-
         // Keep references to all the stuff we sweep
         // so that we can destroy them outside the lock.
         stuffToSweep.reserve(partition.size());
@@ -935,16 +965,12 @@ TaggedCache<
         typename KeyOnlyCacheType::map_type& partition,
         SweptPointersVector&,
         std::atomic<int>& allRemovals,
-        Mutex& partitionLock)
+        std::lock_guard<std::recursive_mutex> const&)
 {
     return std::thread([&, this]() {
-        beast::setCurrentThreadName("sweep-KCache");
-
         int cacheRemovals = 0;
         int mapRemovals = 0;
 
-        std::lock_guard<Mutex> lock(partitionLock);
-
         // Keep references to all the stuff we sweep
         // so that we can destroy them outside the lock.
         {
@@ -979,29 +1005,6 @@ TaggedCache<
     });
 }
 
-template <
-    class Key,
-    class T,
-    bool IsKeyCache,
-    class SharedWeakUnionPointer,
-    class SharedPointerType,
-    class Hash,
-    class KeyEqual,
-    class Mutex>
-inline Mutex&
-TaggedCache<
-    Key,
-    T,
-    IsKeyCache,
-    SharedWeakUnionPointer,
-    SharedPointerType,
-    Hash,
-    KeyEqual,
-    Mutex>::lockPartition(key_type const& key) const
-{
-    return partitionLocks_[m_cache.partition_index(key)];
-}
-
 }  // namespace ripple
 
 #endif

include/xrpl/basics/base_uint.h

@@ -546,7 +546,7 @@ operator<=>(base_uint<Bits, Tag> const& lhs, base_uint<Bits, Tag> const& rhs)
     // This comparison might seem wrong on a casual inspection because it
     // compares data internally stored as std::uint32_t byte-by-byte. But
     // note that the underlying data is stored in big endian, even if the
-    // plaform is little endian. This makes the comparison correct.
+    // platform is little endian. This makes the comparison correct.
     //
     // FIXME: use std::lexicographical_compare_three_way once support is
     //        added to MacOS.

include/xrpl/basics/comparators.h

@@ -9,7 +9,7 @@ namespace ripple {
 
 /*
  * MSVC 2019 version 16.9.0 added [[nodiscard]] to the std comparison
- * operator() functions. boost::bimap checks that the comparitor is a
+ * operator() functions. boost::bimap checks that the comparator is a
  * BinaryFunction, in part by calling the function and ignoring the value.
  * These two things don't play well together. These wrapper classes simply
  * strip [[nodiscard]] from operator() for use in boost::bimap.

include/xrpl/basics/partitioned_unordered_map.h

@@ -258,12 +258,6 @@ public:
         return map_;
     }
 
-    partition_map_type const&
-    map() const
-    {
-        return map_;
-    }
-
     iterator
     begin()
     {
@@ -308,12 +302,6 @@ public:
         return cend();
     }
 
-    std::size_t
-    partition_index(key_type const& key) const
-    {
-        return partitioner(key);
-    }
-
 private:
     template <class T>
     void

include/xrpl/beast/unit_test/runner.h

@@ -39,7 +39,7 @@ public:
 
         The argument string is available to suites and
         allows for customization of the test. Each suite
-        defines its own syntax for the argumnet string.
+        defines its own syntax for the argument string.
         The same argument is passed to all suites.
     */
     void

src/test/app/PermissionedDEX_test.cpp

@@ -966,8 +966,8 @@ class PermissionedDEX_test : public beast::unit_test::suite
     {
         testcase("Remove unfunded offer");
 
-        // checking that an unfunded offer will be implictly removed by a
-        // successfuly payment tx
+        // checking that an unfunded offer will be implicitly removed by a
+        // successful payment tx
         Env env(*this, features);
         auto const& [gw, domainOwner, alice, bob, carol, USD, domainID, credType] =
             PermissionedDEX(env);

src/test/app/ValidatorList_test.cpp

@@ -1740,7 +1740,7 @@ private:
             // locals[0]: from 0 to maxKeys - 4
             // locals[1]: from 1 to maxKeys - 2
             // locals[2]: from 2 to maxKeys
-            // interesection of at least 2: same as locals[1]
+            // intersection of at least 2: same as locals[1]
             // intersection when 1 is dropped: from 2 to maxKeys - 4
             constexpr static int publishers = 3;
             std::array<

src/test/app/tx/apply_test.cpp

@@ -49,7 +49,7 @@ public:
                                  .first;
 
             if (valid != Validity::Valid)
-                fail("Non-Fully canoncial signature was not permitted");
+                fail("Non-Fully canonical signature was not permitted");
         }
 
         {
@@ -63,7 +63,7 @@ public:
                                  fully_canonical.app().config())
                                  .first;
             if (valid == Validity::Valid)
-                fail("Non-Fully canoncial signature was permitted");
+                fail("Non-Fully canonical signature was permitted");
         }
 
         pass();

src/test/basics/TaggedCache_test.cpp

@@ -39,10 +39,10 @@ public:
         // Insert an item, retrieve it, and age it so it gets purged.
         {
             BEAST_EXPECT(c.getCacheSize() == 0);
-            BEAST_EXPECT(c.size() == 0);
+            BEAST_EXPECT(c.getTrackSize() == 0);
             BEAST_EXPECT(!c.insert(1, "one"));
             BEAST_EXPECT(c.getCacheSize() == 1);
-            BEAST_EXPECT(c.size() == 1);
+            BEAST_EXPECT(c.getTrackSize() == 1);
 
             {
                 std::string s;
@@ -53,7 +53,7 @@ public:
             ++clock;
             c.sweep();
             BEAST_EXPECT(c.getCacheSize() == 0);
-            BEAST_EXPECT(c.size() == 0);
+            BEAST_EXPECT(c.getTrackSize() == 0);
         }
 
         // Insert an item, maintain a strong pointer, age it, and
@@ -61,7 +61,7 @@ public:
         {
             BEAST_EXPECT(!c.insert(2, "two"));
             BEAST_EXPECT(c.getCacheSize() == 1);
-            BEAST_EXPECT(c.size() == 1);
+            BEAST_EXPECT(c.getTrackSize() == 1);
 
             {
                 auto p = c.fetch(2);
@@ -69,14 +69,14 @@ public:
                 ++clock;
                 c.sweep();
                 BEAST_EXPECT(c.getCacheSize() == 0);
-                BEAST_EXPECT(c.size() == 1);
+                BEAST_EXPECT(c.getTrackSize() == 1);
             }
 
             // Make sure its gone now that our reference is gone
             ++clock;
             c.sweep();
             BEAST_EXPECT(c.getCacheSize() == 0);
-            BEAST_EXPECT(c.size() == 0);
+            BEAST_EXPECT(c.getTrackSize() == 0);
         }
 
         // Insert the same key/value pair and make sure we get the same result
@@ -92,7 +92,7 @@ public:
             ++clock;
             c.sweep();
             BEAST_EXPECT(c.getCacheSize() == 0);
-            BEAST_EXPECT(c.size() == 0);
+            BEAST_EXPECT(c.getTrackSize() == 0);
         }
 
         // Put an object in but keep a strong pointer to it, advance the clock a
@@ -102,24 +102,24 @@ public:
             // Put an object in
             BEAST_EXPECT(!c.insert(4, "four"));
             BEAST_EXPECT(c.getCacheSize() == 1);
-            BEAST_EXPECT(c.size() == 1);
+            BEAST_EXPECT(c.getTrackSize() == 1);
 
             {
                 // Keep a strong pointer to it
                 auto const p1 = c.fetch(4);
                 BEAST_EXPECT(p1 != nullptr);
                 BEAST_EXPECT(c.getCacheSize() == 1);
-                BEAST_EXPECT(c.size() == 1);
+                BEAST_EXPECT(c.getTrackSize() == 1);
                 // Advance the clock a lot
                 ++clock;
                 c.sweep();
                 BEAST_EXPECT(c.getCacheSize() == 0);
-                BEAST_EXPECT(c.size() == 1);
+                BEAST_EXPECT(c.getTrackSize() == 1);
                 // Canonicalize a new object with the same key
                 auto p2 = std::make_shared<std::string>("four");
                 BEAST_EXPECT(c.canonicalize_replace_client(4, p2));
                 BEAST_EXPECT(c.getCacheSize() == 1);
-                BEAST_EXPECT(c.size() == 1);
+                BEAST_EXPECT(c.getTrackSize() == 1);
                 // Make sure we get the original object
                 BEAST_EXPECT(p1.get() == p2.get());
             }
@@ -127,7 +127,7 @@ public:
             ++clock;
             c.sweep();
             BEAST_EXPECT(c.getCacheSize() == 0);
-            BEAST_EXPECT(c.size() == 0);
+            BEAST_EXPECT(c.getTrackSize() == 0);
         }
     }
 };

src/test/core/Config_test.cpp

@@ -1183,7 +1183,7 @@ r.ripple.com:51235
         BEAST_EXPECT(cfg.IPS_FIXED[6] == "12.34.12.123 12345");
         BEAST_EXPECT(cfg.IPS_FIXED[7] == "12.34.12.123 12345");
 
-        // all ipv6 should be ignored by colon replacer, howsoever formated
+        // all ipv6 should be ignored by colon replacer, howsoever formatted
         BEAST_EXPECT(cfg.IPS_FIXED[8] == "::");
         BEAST_EXPECT(cfg.IPS_FIXED[9] == "2001:db8::");
         BEAST_EXPECT(cfg.IPS_FIXED[10] == "::1");

src/test/core/SociDB_test.cpp

@@ -79,7 +79,7 @@ public:
     void
     testSQLiteFileNames()
     {
-        // confirm that files are given the correct exensions
+        // confirm that files are given the correct extensions
         testcase("sqliteFileNames");
         BasicConfig c;
         setupSQLiteConfig(c, getDatabasePath());

src/test/jtx/amount.h

@@ -243,7 +243,7 @@ struct XRP_t
     }
 
     /** Returns an amount of XRP as PrettyAmount,
-        which is trivially convertable to STAmount
+        which is trivially convertible to STAmount
 
         @param v The number of XRP (not drops)
     */

src/test/jtx/envconfig.h

@@ -6,7 +6,7 @@
 namespace ripple {
 namespace test {
 
-// frequently used macros defined here for convinience.
+// frequently used macros defined here for convenience.
 #define PORT_WS "port_ws"
 #define PORT_RPC "port_rpc"
 #define PORT_PEER "port_peer"

src/test/overlay/cluster_test.cpp

@@ -227,7 +227,7 @@ public:
         BEAST_EXPECT(!c->load(s4));
 
         // Check if we properly terminate when we encounter
-        // a malformed or unparseable entry:
+        // a malformed or unparsable entry:
         auto const node1 = randomNode();
         auto const node2 = randomNode();
 

src/test/rpc/LedgerEntry_test.cpp

@@ -1103,7 +1103,7 @@ class LedgerEntry_test : public beast::unit_test::suite
             checkErrorValue(
                 jrr[jss::result],
                 "malformedAuthorizedCredentials",
-                "Invalid field 'authorized_credentials', not array.");
+                "Invalid field 'authorized_credentials', array empty.");
         }
 
         {
@@ -1144,7 +1144,7 @@ class LedgerEntry_test : public beast::unit_test::suite
             checkErrorValue(
                 jrr[jss::result],
                 "malformedAuthorizedCredentials",
-                "Invalid field 'authorized_credentials', not array.");
+                "Invalid field 'authorized_credentials', array too long.");
         }
     }
 

src/test/rpc/RPCCall_test.cpp

@@ -1584,8 +1584,6 @@ static RPCCallTestData const rpcCallTestArray[] = {
          "EUR/rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
          "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
          "ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789",
-         "junk",  // Note: indexing bug in parseBookOffers() requires junk
-                  // param.
          "200",
      },
      RPCCallTestData::no_exception,
@@ -1597,7 +1595,6 @@ static RPCCallTestData const rpcCallTestArray[] = {
          "issuer" : "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
          "ledger_hash" : "ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789",
          "limit" : 200,
-         "proof" : true,
          "taker_gets" : {
             "currency" : "EUR",
             "issuer" : "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA"
@@ -1617,8 +1614,8 @@ static RPCCallTestData const rpcCallTestArray[] = {
       "EUR/rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
       "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
       "ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789",
-      "junk",  // Note: indexing bug in parseBookOffers() requires junk param.
       "200",
+      "0",
       "MyMarker"},
      RPCCallTestData::no_exception,
      R"({
@@ -1630,7 +1627,6 @@ static RPCCallTestData const rpcCallTestArray[] = {
          "ledger_hash" : "ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789",
          "limit" : 200,
          "marker" : "MyMarker",
-         "proof" : true,
          "taker_gets" : {
             "currency" : "EUR",
             "issuer" : "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA"
@@ -1665,8 +1661,8 @@ static RPCCallTestData const rpcCallTestArray[] = {
       "EUR/rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
       "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
       "ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789",
-      "junk",  // Note: indexing bug in parseBookOffers() requires junk param.
       "200",
+      "0",
       "MyMarker",
       "extra"},
      RPCCallTestData::no_exception,
@@ -1770,12 +1766,19 @@ static RPCCallTestData const rpcCallTestArray[] = {
          "EUR/rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
          "rnUy2SHTrB9DubsPmkJZUXTf5FcNDGrYEA",
          "ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789",
-         "junk",  // Note: indexing bug in parseBookOffers() requires junk
-                  // param.
          "not_a_number",
      },
-     RPCCallTestData::bad_cast,
-     R"()"},
+     RPCCallTestData::no_exception,
+     R"({
+    "method" : "book_offers",
+    "params" : [
+      {
+         "error" : "invalidParams",
+         "error_code" : 31,
+         "error_message" : "Invalid field 'limit'."
+      }
+    ]
+    })"},
 
     // can_delete
     // ------------------------------------------------------------------

src/xrpld/app/ledger/LedgerHistory.cpp

@@ -44,6 +44,8 @@ LedgerHistory::insert(
         ledger->stateMap().getHash().isNonZero(),
         "ripple::LedgerHistory::insert : nonzero hash");
 
+    std::unique_lock sl(m_ledgers_by_hash.peekMutex());
+
     bool const alreadyHad = m_ledgers_by_hash.canonicalize_replace_cache(
         ledger->info().hash, ledger);
     if (validated)
@@ -55,6 +57,7 @@ LedgerHistory::insert(
 LedgerHash
 LedgerHistory::getLedgerHash(LedgerIndex index)
 {
+    std::unique_lock sl(m_ledgers_by_hash.peekMutex());
     if (auto it = mLedgersByIndex.find(index); it != mLedgersByIndex.end())
         return it->second;
     return {};
@@ -64,11 +67,13 @@ std::shared_ptr<Ledger const>
 LedgerHistory::getLedgerBySeq(LedgerIndex index)
 {
     {
+        std::unique_lock sl(m_ledgers_by_hash.peekMutex());
         auto it = mLedgersByIndex.find(index);
 
         if (it != mLedgersByIndex.end())
         {
             uint256 hash = it->second;
+            sl.unlock();
             return getLedgerByHash(hash);
         }
     }
@@ -84,6 +89,7 @@ LedgerHistory::getLedgerBySeq(LedgerIndex index)
 
     {
         // Add this ledger to the local tracking by index
+        std::unique_lock sl(m_ledgers_by_hash.peekMutex());
 
         XRPL_ASSERT(
             ret->isImmutable(),
@@ -433,6 +439,8 @@ LedgerHistory::builtLedger(
     XRPL_ASSERT(
         !hash.isZero(), "ripple::LedgerHistory::builtLedger : nonzero hash");
 
+    std::unique_lock sl(m_consensus_validated.peekMutex());
+
     auto entry = std::make_shared<cv_entry>();
     m_consensus_validated.canonicalize_replace_client(index, entry);
 
@@ -473,6 +481,8 @@ LedgerHistory::validatedLedger(
         !hash.isZero(),
         "ripple::LedgerHistory::validatedLedger : nonzero hash");
 
+    std::unique_lock sl(m_consensus_validated.peekMutex());
+
     auto entry = std::make_shared<cv_entry>();
     m_consensus_validated.canonicalize_replace_client(index, entry);
 
@@ -506,9 +516,10 @@ LedgerHistory::validatedLedger(
 bool
 LedgerHistory::fixIndex(LedgerIndex ledgerIndex, LedgerHash const& ledgerHash)
 {
-    auto ledger = m_ledgers_by_hash.fetch(ledgerHash);
+    std::unique_lock sl(m_ledgers_by_hash.peekMutex());
     auto it = mLedgersByIndex.find(ledgerIndex);
-    if (ledger && (it != mLedgersByIndex.end()) && (it->second != ledgerHash))
+
+    if ((it != mLedgersByIndex.end()) && (it->second != ledgerHash))
     {
         it->second = ledgerHash;
         return false;

src/xrpld/app/rdb/RelationalDatabase.h

@@ -6,7 +6,7 @@
 #include <xrpld/app/misc/Transaction.h>
 #include <xrpld/core/Config.h>
 #include <xrpld/core/DatabaseCon.h>
-#include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 
 #include <xrpl/beast/utility/instrumentation.h>
 

src/xrpld/peerfinder/detail/SlotImp.h

@@ -176,7 +176,7 @@ public:
     // DEPRECATED public data members
 
     // Tells us if we checked the connection. Outbound connections
-    // are always considered checked since we successfuly connected.
+    // are always considered checked since we successfully connected.
     bool checked;
 
     // Set to indicate if the connection can receive incoming at the

src/xrpld/rpc/detail/RPCCall.cpp

@@ -332,15 +332,31 @@ private:
 
         if (jvParams.size() >= 5)
         {
-            int iLimit = jvParams[5u].asInt();
+            try
+            {
+                int iLimit = jvParams[4u].asInt();
 
-            if (iLimit > 0)
-                jvRequest[jss::limit] = iLimit;
+                if (iLimit > 0)
+                    jvRequest[jss::limit] = iLimit;
+            }
+            catch (std::exception const&)
+            {
+                return RPC::invalid_field_error(jss::limit);
+            }
         }
 
-        if (jvParams.size() >= 6 && jvParams[5u].asInt())
+        if (jvParams.size() >= 6)
         {
-            jvRequest[jss::proof] = true;
+            try
+            {
+                int bProof = jvParams[5u].asInt();
+                if (bProof)
+                    jvRequest[jss::proof] = true;
+            }
+            catch (std::exception const&)
+            {
+                return RPC::invalid_field_error(jss::proof);
+            }
         }
 
         if (jvParams.size() == 7)

src/xrpld/rpc/detail/RPCHelpers.cpp

@@ -130,513 +130,6 @@ isRelatedToAccount(
     return false;
 }
 
-bool
-getAccountObjects(
-    ReadView const& ledger,
-    AccountID const& account,
-    std::optional<std::vector<LedgerEntryType>> const& typeFilter,
-    uint256 dirIndex,
-    uint256 entryIndex,
-    std::uint32_t const limit,
-    Json::Value& jvResult)
-{
-    // check if dirIndex is valid
-    if (!dirIndex.isZero() && !ledger.read({ltDIR_NODE, dirIndex}))
-        return false;
-
-    auto typeMatchesFilter = [](std::vector<LedgerEntryType> const& typeFilter,
-                                LedgerEntryType ledgerType) {
-        auto it = std::find(typeFilter.begin(), typeFilter.end(), ledgerType);
-        return it != typeFilter.end();
-    };
-
-    // if dirIndex != 0, then all NFTs have already been returned.  only
-    // iterate NFT pages if the filter says so AND dirIndex == 0
-    bool iterateNFTPages =
-        (!typeFilter.has_value() ||
-         typeMatchesFilter(typeFilter.value(), ltNFTOKEN_PAGE)) &&
-        dirIndex == beast::zero;
-
-    Keylet const firstNFTPage = keylet::nftpage_min(account);
-
-    // we need to check the marker to see if it is an NFTTokenPage index.
-    if (iterateNFTPages && entryIndex != beast::zero)
-    {
-        // if it is we will try to iterate the pages up to the limit
-        // and then change over to the owner directory
-
-        if (firstNFTPage.key != (entryIndex & ~nft::pageMask))
-            iterateNFTPages = false;
-    }
-
-    auto& jvObjects = (jvResult[jss::account_objects] = Json::arrayValue);
-
-    // this is a mutable version of limit, used to seamlessly switch
-    // to iterating directory entries when nftokenpages are exhausted
-    uint32_t mlimit = limit;
-
-    // iterate NFTokenPages preferentially
-    if (iterateNFTPages)
-    {
-        Keylet const first = entryIndex == beast::zero
-            ? firstNFTPage
-            : Keylet{ltNFTOKEN_PAGE, entryIndex};
-
-        Keylet const last = keylet::nftpage_max(account);
-
-        // current key
-        uint256 ck = ledger.succ(first.key, last.key.next()).value_or(last.key);
-
-        // current page
-        auto cp = ledger.read(Keylet{ltNFTOKEN_PAGE, ck});
-
-        while (cp)
-        {
-            jvObjects.append(cp->getJson(JsonOptions::none));
-            auto const npm = (*cp)[~sfNextPageMin];
-            if (npm)
-                cp = ledger.read(Keylet(ltNFTOKEN_PAGE, *npm));
-            else
-                cp = nullptr;
-
-            if (--mlimit == 0)
-            {
-                if (cp)
-                {
-                    jvResult[jss::limit] = limit;
-                    jvResult[jss::marker] = std::string("0,") + to_string(ck);
-                    return true;
-                }
-            }
-
-            if (!npm)
-                break;
-
-            ck = *npm;
-        }
-
-        // if execution reaches here then we're about to transition
-        // to iterating the root directory (and the conventional
-        // behaviour of this RPC function.) Therefore we should
-        // zero entryIndex so as not to terribly confuse things.
-        entryIndex = beast::zero;
-    }
-
-    auto const root = keylet::ownerDir(account);
-    auto found = false;
-
-    if (dirIndex.isZero())
-    {
-        dirIndex = root.key;
-        found = true;
-    }
-
-    auto dir = ledger.read({ltDIR_NODE, dirIndex});
-    if (!dir)
-    {
-        // it's possible the user had nftoken pages but no
-        // directory entries. If there's no nftoken page, we will
-        // give empty array for account_objects.
-        if (mlimit >= limit)
-            jvResult[jss::account_objects] = Json::arrayValue;
-
-        // non-zero dirIndex validity was checked in the beginning of this
-        // function; by this point, it should be zero. This function returns
-        // true regardless of nftoken page presence; if absent, account_objects
-        // is already set as an empty array. Notice we will only return false in
-        // this function when entryIndex can not be found, indicating an invalid
-        // marker error.
-        return true;
-    }
-
-    std::uint32_t i = 0;
-    for (;;)
-    {
-        auto const& entries = dir->getFieldV256(sfIndexes);
-        auto iter = entries.begin();
-
-        if (!found)
-        {
-            iter = std::find(iter, entries.end(), entryIndex);
-            if (iter == entries.end())
-                return false;
-
-            found = true;
-        }
-
-        // it's possible that the returned NFTPages exactly filled the
-        // response.  Check for that condition.
-        if (i == mlimit && mlimit < limit)
-        {
-            jvResult[jss::limit] = limit;
-            jvResult[jss::marker] =
-                to_string(dirIndex) + ',' + to_string(*iter);
-            return true;
-        }
-
-        for (; iter != entries.end(); ++iter)
-        {
-            auto const sleNode = ledger.read(keylet::child(*iter));
-
-            if (!typeFilter.has_value() ||
-                typeMatchesFilter(typeFilter.value(), sleNode->getType()))
-            {
-                jvObjects.append(sleNode->getJson(JsonOptions::none));
-            }
-
-            if (++i == mlimit)
-            {
-                if (++iter != entries.end())
-                {
-                    jvResult[jss::limit] = limit;
-                    jvResult[jss::marker] =
-                        to_string(dirIndex) + ',' + to_string(*iter);
-                    return true;
-                }
-
-                break;
-            }
-        }
-
-        auto const nodeIndex = dir->getFieldU64(sfIndexNext);
-        if (nodeIndex == 0)
-            return true;
-
-        dirIndex = keylet::page(root, nodeIndex).key;
-        dir = ledger.read({ltDIR_NODE, dirIndex});
-        if (!dir)
-            return true;
-
-        if (i == mlimit)
-        {
-            auto const& e = dir->getFieldV256(sfIndexes);
-            if (!e.empty())
-            {
-                jvResult[jss::limit] = limit;
-                jvResult[jss::marker] =
-                    to_string(dirIndex) + ',' + to_string(*e.begin());
-            }
-
-            return true;
-        }
-    }
-}
-
-namespace {
-
-bool
-isValidatedOld(LedgerMaster& ledgerMaster, bool standalone)
-{
-    if (standalone)
-        return false;
-
-    return ledgerMaster.getValidatedLedgerAge() > Tuning::maxValidatedLedgerAge;
-}
-
-template <class T>
-Status
-ledgerFromRequest(T& ledger, JsonContext& context)
-{
-    ledger.reset();
-
-    auto& params = context.params;
-
-    auto indexValue = params[jss::ledger_index];
-    auto hashValue = params[jss::ledger_hash];
-
-    // We need to support the legacy "ledger" field.
-    auto& legacyLedger = params[jss::ledger];
-    if (legacyLedger)
-    {
-        if (legacyLedger.asString().size() > 12)
-            hashValue = legacyLedger;
-        else
-            indexValue = legacyLedger;
-    }
-
-    if (!hashValue.isNull())
-    {
-        if (!hashValue.isString())
-            return {rpcINVALID_PARAMS, "ledgerHashNotString"};
-
-        uint256 ledgerHash;
-        if (!ledgerHash.parseHex(hashValue.asString()))
-            return {rpcINVALID_PARAMS, "ledgerHashMalformed"};
-        return getLedger(ledger, ledgerHash, context);
-    }
-
-    if (!indexValue.isConvertibleTo(Json::stringValue))
-        return {rpcINVALID_PARAMS, "ledgerIndexMalformed"};
-
-    auto const index = indexValue.asString();
-
-    if (index == "current" || index.empty())
-        return getLedger(ledger, LedgerShortcut::CURRENT, context);
-
-    if (index == "validated")
-        return getLedger(ledger, LedgerShortcut::VALIDATED, context);
-
-    if (index == "closed")
-        return getLedger(ledger, LedgerShortcut::CLOSED, context);
-
-    std::uint32_t val;
-    if (!beast::lexicalCastChecked(val, index))
-        return {rpcINVALID_PARAMS, "ledgerIndexMalformed"};
-
-    return getLedger(ledger, val, context);
-}
-}  // namespace
-
-template <class T, class R>
-Status
-ledgerFromRequest(T& ledger, GRPCContext<R>& context)
-{
-    R& request = context.params;
-    return ledgerFromSpecifier(ledger, request.ledger(), context);
-}
-
-// explicit instantiation of above function
-template Status
-ledgerFromRequest<>(
-    std::shared_ptr<ReadView const>&,
-    GRPCContext<org::xrpl::rpc::v1::GetLedgerEntryRequest>&);
-
-// explicit instantiation of above function
-template Status
-ledgerFromRequest<>(
-    std::shared_ptr<ReadView const>&,
-    GRPCContext<org::xrpl::rpc::v1::GetLedgerDataRequest>&);
-
-// explicit instantiation of above function
-template Status
-ledgerFromRequest<>(
-    std::shared_ptr<ReadView const>&,
-    GRPCContext<org::xrpl::rpc::v1::GetLedgerRequest>&);
-
-template <class T>
-Status
-ledgerFromSpecifier(
-    T& ledger,
-    org::xrpl::rpc::v1::LedgerSpecifier const& specifier,
-    Context& context)
-{
-    ledger.reset();
-
-    using LedgerCase = org::xrpl::rpc::v1::LedgerSpecifier::LedgerCase;
-    LedgerCase ledgerCase = specifier.ledger_case();
-    switch (ledgerCase)
-    {
-        case LedgerCase::kHash: {
-            if (auto hash = uint256::fromVoidChecked(specifier.hash()))
-            {
-                return getLedger(ledger, *hash, context);
-            }
-            return {rpcINVALID_PARAMS, "ledgerHashMalformed"};
-        }
-        case LedgerCase::kSequence:
-            return getLedger(ledger, specifier.sequence(), context);
-        case LedgerCase::kShortcut:
-            [[fallthrough]];
-        case LedgerCase::LEDGER_NOT_SET: {
-            auto const shortcut = specifier.shortcut();
-            if (shortcut ==
-                org::xrpl::rpc::v1::LedgerSpecifier::SHORTCUT_VALIDATED)
-            {
-                return getLedger(ledger, LedgerShortcut::VALIDATED, context);
-            }
-            else
-            {
-                if (shortcut ==
-                        org::xrpl::rpc::v1::LedgerSpecifier::SHORTCUT_CURRENT ||
-                    shortcut ==
-                        org::xrpl::rpc::v1::LedgerSpecifier::
-                            SHORTCUT_UNSPECIFIED)
-                {
-                    return getLedger(ledger, LedgerShortcut::CURRENT, context);
-                }
-                else if (
-                    shortcut ==
-                    org::xrpl::rpc::v1::LedgerSpecifier::SHORTCUT_CLOSED)
-                {
-                    return getLedger(ledger, LedgerShortcut::CLOSED, context);
-                }
-            }
-        }
-    }
-
-    return Status::OK;
-}
-
-template <class T>
-Status
-getLedger(T& ledger, uint256 const& ledgerHash, Context& context)
-{
-    ledger = context.ledgerMaster.getLedgerByHash(ledgerHash);
-    if (ledger == nullptr)
-        return {rpcLGR_NOT_FOUND, "ledgerNotFound"};
-    return Status::OK;
-}
-
-template <class T>
-Status
-getLedger(T& ledger, uint32_t ledgerIndex, Context& context)
-{
-    ledger = context.ledgerMaster.getLedgerBySeq(ledgerIndex);
-    if (ledger == nullptr)
-    {
-        auto cur = context.ledgerMaster.getCurrentLedger();
-        if (cur->info().seq == ledgerIndex)
-        {
-            ledger = cur;
-        }
-    }
-
-    if (ledger == nullptr)
-        return {rpcLGR_NOT_FOUND, "ledgerNotFound"};
-
-    if (ledger->info().seq > context.ledgerMaster.getValidLedgerIndex() &&
-        isValidatedOld(context.ledgerMaster, context.app.config().standalone()))
-    {
-        ledger.reset();
-        if (context.apiVersion == 1)
-            return {rpcNO_NETWORK, "InsufficientNetworkMode"};
-        return {rpcNOT_SYNCED, "notSynced"};
-    }
-
-    return Status::OK;
-}
-
-template <class T>
-Status
-getLedger(T& ledger, LedgerShortcut shortcut, Context& context)
-{
-    if (isValidatedOld(context.ledgerMaster, context.app.config().standalone()))
-    {
-        if (context.apiVersion == 1)
-            return {rpcNO_NETWORK, "InsufficientNetworkMode"};
-        return {rpcNOT_SYNCED, "notSynced"};
-    }
-
-    if (shortcut == LedgerShortcut::VALIDATED)
-    {
-        ledger = context.ledgerMaster.getValidatedLedger();
-        if (ledger == nullptr)
-        {
-            if (context.apiVersion == 1)
-                return {rpcNO_NETWORK, "InsufficientNetworkMode"};
-            return {rpcNOT_SYNCED, "notSynced"};
-        }
-
-        XRPL_ASSERT(
-            !ledger->open(), "ripple::RPC::getLedger : validated is not open");
-    }
-    else
-    {
-        if (shortcut == LedgerShortcut::CURRENT)
-        {
-            ledger = context.ledgerMaster.getCurrentLedger();
-            XRPL_ASSERT(
-                ledger->open(), "ripple::RPC::getLedger : current is open");
-        }
-        else if (shortcut == LedgerShortcut::CLOSED)
-        {
-            ledger = context.ledgerMaster.getClosedLedger();
-            XRPL_ASSERT(
-                !ledger->open(), "ripple::RPC::getLedger : closed is not open");
-        }
-        else
-        {
-            return {rpcINVALID_PARAMS, "ledgerIndexMalformed"};
-        }
-
-        if (ledger == nullptr)
-        {
-            if (context.apiVersion == 1)
-                return {rpcNO_NETWORK, "InsufficientNetworkMode"};
-            return {rpcNOT_SYNCED, "notSynced"};
-        }
-
-        static auto const minSequenceGap = 10;
-
-        if (ledger->info().seq + minSequenceGap <
-            context.ledgerMaster.getValidLedgerIndex())
-        {
-            ledger.reset();
-            if (context.apiVersion == 1)
-                return {rpcNO_NETWORK, "InsufficientNetworkMode"};
-            return {rpcNOT_SYNCED, "notSynced"};
-        }
-    }
-    return Status::OK;
-}
-
-// Explicit instantiation of above three functions
-template Status
-getLedger<>(std::shared_ptr<ReadView const>&, uint32_t, Context&);
-
-template Status
-getLedger<>(
-    std::shared_ptr<ReadView const>&,
-    LedgerShortcut shortcut,
-    Context&);
-
-template Status
-getLedger<>(std::shared_ptr<ReadView const>&, uint256 const&, Context&);
-
-// The previous version of the lookupLedger command would accept the
-// "ledger_index" argument as a string and silently treat it as a request to
-// return the current ledger which, while not strictly wrong, could cause a lot
-// of confusion.
-//
-// The code now robustly validates the input and ensures that the only possible
-// values for the "ledger_index" parameter are the index of a ledger passed as
-// an integer or one of the strings "current", "closed" or "validated".
-// Additionally, the code ensures that the value passed in "ledger_hash" is a
-// string and a valid hash. Invalid values will return an appropriate error
-// code.
-//
-// In the absence of the "ledger_hash" or "ledger_index" parameters, the code
-// assumes that "ledger_index" has the value "current".
-//
-// Returns a Json::objectValue.  If there was an error, it will be in that
-// return value.  Otherwise, the object contains the field "validated" and
-// optionally the fields "ledger_hash", "ledger_index" and
-// "ledger_current_index", if they are defined.
-Status
-lookupLedger(
-    std::shared_ptr<ReadView const>& ledger,
-    JsonContext& context,
-    Json::Value& result)
-{
-    if (auto status = ledgerFromRequest(ledger, context))
-        return status;
-
-    auto& info = ledger->info();
-
-    if (!ledger->open())
-    {
-        result[jss::ledger_hash] = to_string(info.hash);
-        result[jss::ledger_index] = info.seq;
-    }
-    else
-    {
-        result[jss::ledger_current_index] = info.seq;
-    }
-
-    result[jss::validated] = context.ledgerMaster.isValidated(*ledger);
-    return Status::OK;
-}
-
-Json::Value
-lookupLedger(std::shared_ptr<ReadView const>& ledger, JsonContext& context)
-{
-    Json::Value result;
-    if (auto status = lookupLedger(ledger, context, result))
-        status.inject(result);
-
-    return result;
-}
-
 hash_set<AccountID>
 parseAccountIds(Json::Value const& jvArray)
 {
@@ -988,123 +481,5 @@ isAccountObjectsValidType(LedgerEntryType const& type)
     }
 }
 
-std::variant<std::shared_ptr<Ledger const>, Json::Value>
-getLedgerByContext(RPC::JsonContext& context)
-{
-    auto const hasHash = context.params.isMember(jss::ledger_hash);
-    auto const hasIndex = context.params.isMember(jss::ledger_index);
-    std::uint32_t ledgerIndex = 0;
-
-    auto& ledgerMaster = context.app.getLedgerMaster();
-    LedgerHash ledgerHash;
-
-    if ((hasHash && hasIndex) || !(hasHash || hasIndex))
-    {
-        return RPC::make_param_error(
-            "Exactly one of ledger_hash and ledger_index can be set.");
-    }
-
-    context.loadType = Resource::feeHeavyBurdenRPC;
-
-    if (hasHash)
-    {
-        auto const& jsonHash = context.params[jss::ledger_hash];
-        if (!jsonHash.isString() || !ledgerHash.parseHex(jsonHash.asString()))
-            return RPC::invalid_field_error(jss::ledger_hash);
-    }
-    else
-    {
-        auto const& jsonIndex = context.params[jss::ledger_index];
-        if (!jsonIndex.isInt())
-            return RPC::invalid_field_error(jss::ledger_index);
-
-        // We need a validated ledger to get the hash from the sequence
-        if (ledgerMaster.getValidatedLedgerAge() >
-            RPC::Tuning::maxValidatedLedgerAge)
-        {
-            if (context.apiVersion == 1)
-                return rpcError(rpcNO_CURRENT);
-            return rpcError(rpcNOT_SYNCED);
-        }
-
-        ledgerIndex = jsonIndex.asInt();
-        auto ledger = ledgerMaster.getValidatedLedger();
-
-        if (ledgerIndex >= ledger->info().seq)
-            return RPC::make_param_error("Ledger index too large");
-        if (ledgerIndex <= 0)
-            return RPC::make_param_error("Ledger index too small");
-
-        auto const j = context.app.journal("RPCHandler");
-        // Try to get the hash of the desired ledger from the validated
-        // ledger
-        auto neededHash = hashOfSeq(*ledger, ledgerIndex, j);
-        if (!neededHash)
-        {
-            // Find a ledger more likely to have the hash of the desired
-            // ledger
-            auto const refIndex = getCandidateLedger(ledgerIndex);
-            auto refHash = hashOfSeq(*ledger, refIndex, j);
-            XRPL_ASSERT(
-                refHash,
-                "ripple::RPC::getLedgerByContext : nonzero ledger hash");
-
-            ledger = ledgerMaster.getLedgerByHash(*refHash);
-            if (!ledger)
-            {
-                // We don't have the ledger we need to figure out which
-                // ledger they want. Try to get it.
-
-                if (auto il = context.app.getInboundLedgers().acquire(
-                        *refHash, refIndex, InboundLedger::Reason::GENERIC))
-                {
-                    Json::Value jvResult = RPC::make_error(
-                        rpcLGR_NOT_FOUND,
-                        "acquiring ledger containing requested index");
-                    jvResult[jss::acquiring] =
-                        getJson(LedgerFill(*il, &context));
-                    return jvResult;
-                }
-
-                if (auto il = context.app.getInboundLedgers().find(*refHash))
-                {
-                    Json::Value jvResult = RPC::make_error(
-                        rpcLGR_NOT_FOUND,
-                        "acquiring ledger containing requested index");
-                    jvResult[jss::acquiring] = il->getJson(0);
-                    return jvResult;
-                }
-
-                // Likely the app is shutting down
-                return Json::Value();
-            }
-
-            neededHash = hashOfSeq(*ledger, ledgerIndex, j);
-        }
-        XRPL_ASSERT(
-            neededHash,
-            "ripple::RPC::getLedgerByContext : nonzero needed hash");
-        ledgerHash = neededHash ? *neededHash : beast::zero;  // kludge
-    }
-
-    // Try to get the desired ledger
-    // Verify all nodes even if we think we have it
-    auto ledger = context.app.getInboundLedgers().acquire(
-        ledgerHash, ledgerIndex, InboundLedger::Reason::GENERIC);
-
-    // In standalone mode, accept the ledger from the ledger cache
-    if (!ledger && context.app.config().standalone())
-        ledger = ledgerMaster.getLedgerByHash(ledgerHash);
-
-    if (ledger)
-        return ledger;
-
-    if (auto il = context.app.getInboundLedgers().find(ledgerHash))
-        return il->getJson(0);
-
-    return RPC::make_error(
-        rpcNOT_READY, "findCreate failed to return an inbound ledger");
-}
-
 }  // namespace RPC
 }  // namespace ripple

src/xrpld/rpc/detail/RPCHelpers.h

@@ -73,81 +73,6 @@ isRelatedToAccount(
     std::shared_ptr<SLE const> const& sle,
     AccountID const& accountID);
 
-/** Gathers all objects for an account in a ledger.
-    @param ledger Ledger to search account objects.
-    @param account AccountID to find objects for.
-    @param typeFilter Gathers objects of these types. empty gathers all types.
-    @param dirIndex Begin gathering account objects from this directory.
-    @param entryIndex Begin gathering objects from this directory node.
-    @param limit Maximum number of objects to find.
-    @param jvResult A JSON result that holds the request objects.
-*/
-bool
-getAccountObjects(
-    ReadView const& ledger,
-    AccountID const& account,
-    std::optional<std::vector<LedgerEntryType>> const& typeFilter,
-    uint256 dirIndex,
-    uint256 entryIndex,
-    std::uint32_t const limit,
-    Json::Value& jvResult);
-
-/** Get ledger by hash
-    If there is no error in the return value, the ledger pointer will have
-    been filled
-*/
-template <class T>
-Status
-getLedger(T& ledger, uint256 const& ledgerHash, Context& context);
-
-/** Get ledger by sequence
-    If there is no error in the return value, the ledger pointer will have
-    been filled
-*/
-template <class T>
-Status
-getLedger(T& ledger, uint32_t ledgerIndex, Context& context);
-
-enum LedgerShortcut { CURRENT, CLOSED, VALIDATED };
-/** Get ledger specified in shortcut.
-    If there is no error in the return value, the ledger pointer will have
-    been filled
-*/
-template <class T>
-Status
-getLedger(T& ledger, LedgerShortcut shortcut, Context& context);
-
-/** Look up a ledger from a request and fill a Json::Result with either
-    an error, or data representing a ledger.
-
-    If there is no error in the return value, then the ledger pointer will have
-    been filled.
-*/
-Json::Value
-lookupLedger(std::shared_ptr<ReadView const>&, JsonContext&);
-
-/** Look up a ledger from a request and fill a Json::Result with the data
-    representing a ledger.
-
-    If the returned Status is OK, the ledger pointer will have been filled.
-*/
-Status
-lookupLedger(
-    std::shared_ptr<ReadView const>&,
-    JsonContext&,
-    Json::Value& result);
-
-template <class T, class R>
-Status
-ledgerFromRequest(T& ledger, GRPCContext<R>& context);
-
-template <class T>
-Status
-ledgerFromSpecifier(
-    T& ledger,
-    org::xrpl::rpc::v1::LedgerSpecifier const& specifier,
-    Context& context);
-
 hash_set<AccountID>
 parseAccountIds(Json::Value const& jvArray);
 
@@ -194,11 +119,6 @@ chooseLedgerEntryType(Json::Value const& params);
 bool
 isAccountObjectsValidType(LedgerEntryType const& type);
 
-/** Return a ledger based on ledger_hash or ledger_index,
-    or an RPC error */
-std::variant<std::shared_ptr<Ledger const>, Json::Value>
-getLedgerByContext(RPC::JsonContext& context);
-
 std::optional<std::pair<PublicKey, SecretKey>>
 keypairForSignature(
     Json::Value const& params,

src/xrpld/rpc/detail/RPCLedgerHelpers.cpp

@@ -0,0 +1,458 @@
+#include <xrpld/app/ledger/LedgerMaster.h>
+#include <xrpld/app/ledger/LedgerToJson.h>
+#include <xrpld/app/ledger/OpenLedger.h>
+#include <xrpld/app/misc/Transaction.h>
+#include <xrpld/app/paths/TrustLine.h>
+#include <xrpld/app/rdb/RelationalDatabase.h>
+#include <xrpld/app/tx/detail/NFTokenUtils.h>
+#include <xrpld/rpc/Context.h>
+#include <xrpld/rpc/DeliveredAmount.h>
+#include <xrpld/rpc/detail/RPCHelpers.h>
+
+#include <xrpl/ledger/View.h>
+#include <xrpl/protocol/AccountID.h>
+#include <xrpl/protocol/RPCErr.h>
+#include <xrpl/protocol/nftPageMask.h>
+#include <xrpl/resource/Fees.h>
+
+#include <boost/algorithm/string/case_conv.hpp>
+#include <boost/algorithm/string/predicate.hpp>
+
+namespace ripple {
+namespace RPC {
+
+namespace {
+
+bool
+isValidatedOld(LedgerMaster& ledgerMaster, bool standalone)
+{
+    if (standalone)
+        return false;
+
+    return ledgerMaster.getValidatedLedgerAge() > Tuning::maxValidatedLedgerAge;
+}
+
+template <class T>
+Status
+ledgerFromRequest(T& ledger, JsonContext& context)
+{
+    ledger.reset();
+
+    auto& params = context.params;
+
+    auto indexValue = params[jss::ledger_index];
+    auto hashValue = params[jss::ledger_hash];
+
+    // We need to support the legacy "ledger" field.
+    auto& legacyLedger = params[jss::ledger];
+    if (legacyLedger)
+    {
+        if (legacyLedger.asString().size() > 12)
+            hashValue = legacyLedger;
+        else
+            indexValue = legacyLedger;
+    }
+
+    if (!hashValue.isNull())
+    {
+        if (!hashValue.isString())
+            return {rpcINVALID_PARAMS, "ledgerHashNotString"};
+
+        uint256 ledgerHash;
+        if (!ledgerHash.parseHex(hashValue.asString()))
+            return {rpcINVALID_PARAMS, "ledgerHashMalformed"};
+        return getLedger(ledger, ledgerHash, context);
+    }
+
+    if (!indexValue.isConvertibleTo(Json::stringValue))
+        return {rpcINVALID_PARAMS, "ledgerIndexMalformed"};
+
+    auto const index = indexValue.asString();
+
+    if (index == "current" || index.empty())
+        return getLedger(ledger, LedgerShortcut::CURRENT, context);
+
+    if (index == "validated")
+        return getLedger(ledger, LedgerShortcut::VALIDATED, context);
+
+    if (index == "closed")
+        return getLedger(ledger, LedgerShortcut::CLOSED, context);
+
+    std::uint32_t val;
+    if (!beast::lexicalCastChecked(val, index))
+        return {rpcINVALID_PARAMS, "ledgerIndexMalformed"};
+
+    return getLedger(ledger, val, context);
+}
+}  // namespace
+
+template <class T, class R>
+Status
+ledgerFromRequest(T& ledger, GRPCContext<R>& context)
+{
+    R& request = context.params;
+    return ledgerFromSpecifier(ledger, request.ledger(), context);
+}
+
+// explicit instantiation of above function
+template Status
+ledgerFromRequest<>(
+    std::shared_ptr<ReadView const>&,
+    GRPCContext<org::xrpl::rpc::v1::GetLedgerEntryRequest>&);
+
+// explicit instantiation of above function
+template Status
+ledgerFromRequest<>(
+    std::shared_ptr<ReadView const>&,
+    GRPCContext<org::xrpl::rpc::v1::GetLedgerDataRequest>&);
+
+// explicit instantiation of above function
+template Status
+ledgerFromRequest<>(
+    std::shared_ptr<ReadView const>&,
+    GRPCContext<org::xrpl::rpc::v1::GetLedgerRequest>&);
+
+template <class T>
+Status
+ledgerFromSpecifier(
+    T& ledger,
+    org::xrpl::rpc::v1::LedgerSpecifier const& specifier,
+    Context& context)
+{
+    ledger.reset();
+
+    using LedgerCase = org::xrpl::rpc::v1::LedgerSpecifier::LedgerCase;
+    LedgerCase ledgerCase = specifier.ledger_case();
+    switch (ledgerCase)
+    {
+        case LedgerCase::kHash: {
+            if (auto hash = uint256::fromVoidChecked(specifier.hash()))
+            {
+                return getLedger(ledger, *hash, context);
+            }
+            return {rpcINVALID_PARAMS, "ledgerHashMalformed"};
+        }
+        case LedgerCase::kSequence:
+            return getLedger(ledger, specifier.sequence(), context);
+        case LedgerCase::kShortcut:
+            [[fallthrough]];
+        case LedgerCase::LEDGER_NOT_SET: {
+            auto const shortcut = specifier.shortcut();
+            if (shortcut ==
+                org::xrpl::rpc::v1::LedgerSpecifier::SHORTCUT_VALIDATED)
+            {
+                return getLedger(ledger, LedgerShortcut::VALIDATED, context);
+            }
+            else
+            {
+                if (shortcut ==
+                        org::xrpl::rpc::v1::LedgerSpecifier::SHORTCUT_CURRENT ||
+                    shortcut ==
+                        org::xrpl::rpc::v1::LedgerSpecifier::
+                            SHORTCUT_UNSPECIFIED)
+                {
+                    return getLedger(ledger, LedgerShortcut::CURRENT, context);
+                }
+                else if (
+                    shortcut ==
+                    org::xrpl::rpc::v1::LedgerSpecifier::SHORTCUT_CLOSED)
+                {
+                    return getLedger(ledger, LedgerShortcut::CLOSED, context);
+                }
+            }
+        }
+    }
+
+    return Status::OK;
+}
+
+template <class T>
+Status
+getLedger(T& ledger, uint256 const& ledgerHash, Context& context)
+{
+    ledger = context.ledgerMaster.getLedgerByHash(ledgerHash);
+    if (ledger == nullptr)
+        return {rpcLGR_NOT_FOUND, "ledgerNotFound"};
+    return Status::OK;
+}
+
+template <class T>
+Status
+getLedger(T& ledger, uint32_t ledgerIndex, Context& context)
+{
+    ledger = context.ledgerMaster.getLedgerBySeq(ledgerIndex);
+    if (ledger == nullptr)
+    {
+        auto cur = context.ledgerMaster.getCurrentLedger();
+        if (cur->info().seq == ledgerIndex)
+        {
+            ledger = cur;
+        }
+    }
+
+    if (ledger == nullptr)
+        return {rpcLGR_NOT_FOUND, "ledgerNotFound"};
+
+    if (ledger->info().seq > context.ledgerMaster.getValidLedgerIndex() &&
+        isValidatedOld(context.ledgerMaster, context.app.config().standalone()))
+    {
+        ledger.reset();
+        if (context.apiVersion == 1)
+            return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+        return {rpcNOT_SYNCED, "notSynced"};
+    }
+
+    return Status::OK;
+}
+
+template <class T>
+Status
+getLedger(T& ledger, LedgerShortcut shortcut, Context& context)
+{
+    if (isValidatedOld(context.ledgerMaster, context.app.config().standalone()))
+    {
+        if (context.apiVersion == 1)
+            return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+        return {rpcNOT_SYNCED, "notSynced"};
+    }
+
+    if (shortcut == LedgerShortcut::VALIDATED)
+    {
+        ledger = context.ledgerMaster.getValidatedLedger();
+        if (ledger == nullptr)
+        {
+            if (context.apiVersion == 1)
+                return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+            return {rpcNOT_SYNCED, "notSynced"};
+        }
+
+        XRPL_ASSERT(
+            !ledger->open(), "ripple::RPC::getLedger : validated is not open");
+    }
+    else
+    {
+        if (shortcut == LedgerShortcut::CURRENT)
+        {
+            ledger = context.ledgerMaster.getCurrentLedger();
+            XRPL_ASSERT(
+                ledger->open(), "ripple::RPC::getLedger : current is open");
+        }
+        else if (shortcut == LedgerShortcut::CLOSED)
+        {
+            ledger = context.ledgerMaster.getClosedLedger();
+            XRPL_ASSERT(
+                !ledger->open(), "ripple::RPC::getLedger : closed is not open");
+        }
+        else
+        {
+            return {rpcINVALID_PARAMS, "ledgerIndexMalformed"};
+        }
+
+        if (ledger == nullptr)
+        {
+            if (context.apiVersion == 1)
+                return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+            return {rpcNOT_SYNCED, "notSynced"};
+        }
+
+        static auto const minSequenceGap = 10;
+
+        if (ledger->info().seq + minSequenceGap <
+            context.ledgerMaster.getValidLedgerIndex())
+        {
+            ledger.reset();
+            if (context.apiVersion == 1)
+                return {rpcNO_NETWORK, "InsufficientNetworkMode"};
+            return {rpcNOT_SYNCED, "notSynced"};
+        }
+    }
+    return Status::OK;
+}
+
+// Explicit instantiation of above three functions
+template Status
+getLedger<>(std::shared_ptr<ReadView const>&, uint32_t, Context&);
+
+template Status
+getLedger<>(
+    std::shared_ptr<ReadView const>&,
+    LedgerShortcut shortcut,
+    Context&);
+
+template Status
+getLedger<>(std::shared_ptr<ReadView const>&, uint256 const&, Context&);
+
+// The previous version of the lookupLedger command would accept the
+// "ledger_index" argument as a string and silently treat it as a request to
+// return the current ledger which, while not strictly wrong, could cause a lot
+// of confusion.
+//
+// The code now robustly validates the input and ensures that the only possible
+// values for the "ledger_index" parameter are the index of a ledger passed as
+// an integer or one of the strings "current", "closed" or "validated".
+// Additionally, the code ensures that the value passed in "ledger_hash" is a
+// string and a valid hash. Invalid values will return an appropriate error
+// code.
+//
+// In the absence of the "ledger_hash" or "ledger_index" parameters, the code
+// assumes that "ledger_index" has the value "current".
+//
+// Returns a Json::objectValue.  If there was an error, it will be in that
+// return value.  Otherwise, the object contains the field "validated" and
+// optionally the fields "ledger_hash", "ledger_index" and
+// "ledger_current_index", if they are defined.
+Status
+lookupLedger(
+    std::shared_ptr<ReadView const>& ledger,
+    JsonContext& context,
+    Json::Value& result)
+{
+    if (auto status = ledgerFromRequest(ledger, context))
+        return status;
+
+    auto& info = ledger->info();
+
+    if (!ledger->open())
+    {
+        result[jss::ledger_hash] = to_string(info.hash);
+        result[jss::ledger_index] = info.seq;
+    }
+    else
+    {
+        result[jss::ledger_current_index] = info.seq;
+    }
+
+    result[jss::validated] = context.ledgerMaster.isValidated(*ledger);
+    return Status::OK;
+}
+
+Json::Value
+lookupLedger(std::shared_ptr<ReadView const>& ledger, JsonContext& context)
+{
+    Json::Value result;
+    if (auto status = lookupLedger(ledger, context, result))
+        status.inject(result);
+
+    return result;
+}
+
+std::variant<std::shared_ptr<Ledger const>, Json::Value>
+getLedgerByContext(RPC::JsonContext& context)
+{
+    auto const hasHash = context.params.isMember(jss::ledger_hash);
+    auto const hasIndex = context.params.isMember(jss::ledger_index);
+    std::uint32_t ledgerIndex = 0;
+
+    auto& ledgerMaster = context.app.getLedgerMaster();
+    LedgerHash ledgerHash;
+
+    if ((hasHash && hasIndex) || !(hasHash || hasIndex))
+    {
+        return RPC::make_param_error(
+            "Exactly one of ledger_hash and ledger_index can be set.");
+    }
+
+    context.loadType = Resource::feeHeavyBurdenRPC;
+
+    if (hasHash)
+    {
+        auto const& jsonHash = context.params[jss::ledger_hash];
+        if (!jsonHash.isString() || !ledgerHash.parseHex(jsonHash.asString()))
+            return RPC::invalid_field_error(jss::ledger_hash);
+    }
+    else
+    {
+        auto const& jsonIndex = context.params[jss::ledger_index];
+        if (!jsonIndex.isInt())
+            return RPC::invalid_field_error(jss::ledger_index);
+
+        // We need a validated ledger to get the hash from the sequence
+        if (ledgerMaster.getValidatedLedgerAge() >
+            RPC::Tuning::maxValidatedLedgerAge)
+        {
+            if (context.apiVersion == 1)
+                return rpcError(rpcNO_CURRENT);
+            return rpcError(rpcNOT_SYNCED);
+        }
+
+        ledgerIndex = jsonIndex.asInt();
+        auto ledger = ledgerMaster.getValidatedLedger();
+
+        if (ledgerIndex >= ledger->info().seq)
+            return RPC::make_param_error("Ledger index too large");
+        if (ledgerIndex <= 0)
+            return RPC::make_param_error("Ledger index too small");
+
+        auto const j = context.app.journal("RPCHandler");
+        // Try to get the hash of the desired ledger from the validated
+        // ledger
+        auto neededHash = hashOfSeq(*ledger, ledgerIndex, j);
+        if (!neededHash)
+        {
+            // Find a ledger more likely to have the hash of the desired
+            // ledger
+            auto const refIndex = getCandidateLedger(ledgerIndex);
+            auto refHash = hashOfSeq(*ledger, refIndex, j);
+            XRPL_ASSERT(
+                refHash,
+                "ripple::RPC::getLedgerByContext : nonzero ledger hash");
+
+            ledger = ledgerMaster.getLedgerByHash(*refHash);
+            if (!ledger)
+            {
+                // We don't have the ledger we need to figure out which
+                // ledger they want. Try to get it.
+
+                if (auto il = context.app.getInboundLedgers().acquire(
+                        *refHash, refIndex, InboundLedger::Reason::GENERIC))
+                {
+                    Json::Value jvResult = RPC::make_error(
+                        rpcLGR_NOT_FOUND,
+                        "acquiring ledger containing requested index");
+                    jvResult[jss::acquiring] =
+                        getJson(LedgerFill(*il, &context));
+                    return jvResult;
+                }
+
+                if (auto il = context.app.getInboundLedgers().find(*refHash))
+                {
+                    Json::Value jvResult = RPC::make_error(
+                        rpcLGR_NOT_FOUND,
+                        "acquiring ledger containing requested index");
+                    jvResult[jss::acquiring] = il->getJson(0);
+                    return jvResult;
+                }
+
+                // Likely the app is shutting down
+                return Json::Value();
+            }
+
+            neededHash = hashOfSeq(*ledger, ledgerIndex, j);
+        }
+        XRPL_ASSERT(
+            neededHash,
+            "ripple::RPC::getLedgerByContext : nonzero needed hash");
+        ledgerHash = neededHash ? *neededHash : beast::zero;  // kludge
+    }
+
+    // Try to get the desired ledger
+    // Verify all nodes even if we think we have it
+    auto ledger = context.app.getInboundLedgers().acquire(
+        ledgerHash, ledgerIndex, InboundLedger::Reason::GENERIC);
+
+    // In standalone mode, accept the ledger from the ledger cache
+    if (!ledger && context.app.config().standalone())
+        ledger = ledgerMaster.getLedgerByHash(ledgerHash);
+
+    if (ledger)
+        return ledger;
+
+    if (auto il = context.app.getInboundLedgers().find(ledgerHash))
+        return il->getJson(0);
+
+    return RPC::make_error(
+        rpcNOT_READY, "findCreate failed to return an inbound ledger");
+}
+
+}  // namespace RPC
+}  // namespace ripple

src/xrpld/rpc/detail/RPCLedgerHelpers.h

@@ -0,0 +1,96 @@
+#ifndef XRPL_RPC_RPCLEDGERHELPERS_H_INCLUDED
+#define XRPL_RPC_RPCLEDGERHELPERS_H_INCLUDED
+
+#include <xrpld/app/misc/NetworkOPs.h>
+#include <xrpld/app/misc/TxQ.h>
+#include <xrpld/rpc/Context.h>
+#include <xrpld/rpc/Status.h>
+#include <xrpld/rpc/detail/Tuning.h>
+
+#include <xrpl/beast/core/SemanticVersion.h>
+#include <xrpl/proto/org/xrpl/rpc/v1/xrp_ledger.pb.h>
+#include <xrpl/protocol/ApiVersion.h>
+#include <xrpl/protocol/SecretKey.h>
+
+#include <optional>
+#include <variant>
+
+namespace Json {
+class Value;
+}
+
+namespace ripple {
+
+class ReadView;
+class Transaction;
+
+namespace RPC {
+
+struct JsonContext;
+
+/** Get ledger by hash
+    If there is no error in the return value, the ledger pointer will have
+    been filled
+*/
+template <class T>
+Status
+getLedger(T& ledger, uint256 const& ledgerHash, Context& context);
+
+/** Get ledger by sequence
+    If there is no error in the return value, the ledger pointer will have
+    been filled
+*/
+template <class T>
+Status
+getLedger(T& ledger, uint32_t ledgerIndex, Context& context);
+
+enum LedgerShortcut { CURRENT, CLOSED, VALIDATED };
+/** Get ledger specified in shortcut.
+    If there is no error in the return value, the ledger pointer will have
+    been filled
+*/
+template <class T>
+Status
+getLedger(T& ledger, LedgerShortcut shortcut, Context& context);
+
+/** Look up a ledger from a request and fill a Json::Result with either
+    an error, or data representing a ledger.
+
+    If there is no error in the return value, then the ledger pointer will have
+    been filled.
+*/
+Json::Value
+lookupLedger(std::shared_ptr<ReadView const>&, JsonContext&);
+
+/** Look up a ledger from a request and fill a Json::Result with the data
+    representing a ledger.
+
+    If the returned Status is OK, the ledger pointer will have been filled.
+*/
+Status
+lookupLedger(
+    std::shared_ptr<ReadView const>&,
+    JsonContext&,
+    Json::Value& result);
+
+template <class T, class R>
+Status
+ledgerFromRequest(T& ledger, GRPCContext<R>& context);
+
+template <class T>
+Status
+ledgerFromSpecifier(
+    T& ledger,
+    org::xrpl::rpc::v1::LedgerSpecifier const& specifier,
+    Context& context);
+
+/** Return a ledger based on ledger_hash or ledger_index,
+    or an RPC error */
+std::variant<std::shared_ptr<Ledger const>, Json::Value>
+getLedgerByContext(RPC::JsonContext& context);
+
+}  // namespace RPC
+
+}  // namespace ripple
+
+#endif

src/xrpld/rpc/handlers/AMMInfo.cpp

@@ -2,6 +2,7 @@
 #include <xrpld/app/misc/AMMUtils.h>
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 
 #include <xrpl/json/json_value.h>
 #include <xrpl/ledger/ReadView.h>

src/xrpld/rpc/handlers/AccountChannels.cpp

@@ -1,5 +1,6 @@
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 #include <xrpld/rpc/detail/Tuning.h>
 
 #include <xrpl/ledger/ReadView.h>

src/xrpld/rpc/handlers/AccountCurrenciesHandler.cpp

@@ -1,6 +1,6 @@
 #include <xrpld/app/paths/TrustLine.h>
 #include <xrpld/rpc/Context.h>
-#include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/protocol/ErrorCodes.h>

src/xrpld/rpc/handlers/AccountInfo.cpp

@@ -3,6 +3,7 @@
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/GRPCHandlers.h>
 #include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 
 #include <xrpl/json/json_value.h>
 #include <xrpl/ledger/ReadView.h>

src/xrpld/rpc/handlers/AccountLines.cpp

@@ -1,6 +1,7 @@
 #include <xrpld/app/paths/TrustLine.h>
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 #include <xrpld/rpc/detail/Tuning.h>
 
 #include <xrpl/ledger/ReadView.h>

src/xrpld/rpc/handlers/AccountObjects.cpp

@@ -1,6 +1,7 @@
 #include <xrpld/app/tx/detail/NFTokenUtils.h>
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 #include <xrpld/rpc/detail/Tuning.h>
 
 #include <xrpl/ledger/ReadView.h>
@@ -158,6 +159,198 @@ doAccountNFTs(RPC::JsonContext& context)
     return result;
 }
 
+bool
+getAccountObjects(
+    ReadView const& ledger,
+    AccountID const& account,
+    std::optional<std::vector<LedgerEntryType>> const& typeFilter,
+    uint256 dirIndex,
+    uint256 entryIndex,
+    std::uint32_t const limit,
+    Json::Value& jvResult)
+{
+    // check if dirIndex is valid
+    if (!dirIndex.isZero() && !ledger.read({ltDIR_NODE, dirIndex}))
+        return false;
+
+    auto typeMatchesFilter = [](std::vector<LedgerEntryType> const& typeFilter,
+                                LedgerEntryType ledgerType) {
+        auto it = std::find(typeFilter.begin(), typeFilter.end(), ledgerType);
+        return it != typeFilter.end();
+    };
+
+    // if dirIndex != 0, then all NFTs have already been returned.  only
+    // iterate NFT pages if the filter says so AND dirIndex == 0
+    bool iterateNFTPages =
+        (!typeFilter.has_value() ||
+         typeMatchesFilter(typeFilter.value(), ltNFTOKEN_PAGE)) &&
+        dirIndex == beast::zero;
+
+    Keylet const firstNFTPage = keylet::nftpage_min(account);
+
+    // we need to check the marker to see if it is an NFTTokenPage index.
+    if (iterateNFTPages && entryIndex != beast::zero)
+    {
+        // if it is we will try to iterate the pages up to the limit
+        // and then change over to the owner directory
+
+        if (firstNFTPage.key != (entryIndex & ~nft::pageMask))
+            iterateNFTPages = false;
+    }
+
+    auto& jvObjects = (jvResult[jss::account_objects] = Json::arrayValue);
+
+    // this is a mutable version of limit, used to seamlessly switch
+    // to iterating directory entries when nftokenpages are exhausted
+    uint32_t mlimit = limit;
+
+    // iterate NFTokenPages preferentially
+    if (iterateNFTPages)
+    {
+        Keylet const first = entryIndex == beast::zero
+            ? firstNFTPage
+            : Keylet{ltNFTOKEN_PAGE, entryIndex};
+
+        Keylet const last = keylet::nftpage_max(account);
+
+        // current key
+        uint256 ck = ledger.succ(first.key, last.key.next()).value_or(last.key);
+
+        // current page
+        auto cp = ledger.read(Keylet{ltNFTOKEN_PAGE, ck});
+
+        while (cp)
+        {
+            jvObjects.append(cp->getJson(JsonOptions::none));
+            auto const npm = (*cp)[~sfNextPageMin];
+            if (npm)
+                cp = ledger.read(Keylet(ltNFTOKEN_PAGE, *npm));
+            else
+                cp = nullptr;
+
+            if (--mlimit == 0)
+            {
+                if (cp)
+                {
+                    jvResult[jss::limit] = limit;
+                    jvResult[jss::marker] = std::string("0,") + to_string(ck);
+                    return true;
+                }
+            }
+
+            if (!npm)
+                break;
+
+            ck = *npm;
+        }
+
+        // if execution reaches here then we're about to transition
+        // to iterating the root directory (and the conventional
+        // behaviour of this RPC function.) Therefore we should
+        // zero entryIndex so as not to terribly confuse things.
+        entryIndex = beast::zero;
+    }
+
+    auto const root = keylet::ownerDir(account);
+    auto found = false;
+
+    if (dirIndex.isZero())
+    {
+        dirIndex = root.key;
+        found = true;
+    }
+
+    auto dir = ledger.read({ltDIR_NODE, dirIndex});
+    if (!dir)
+    {
+        // it's possible the user had nftoken pages but no
+        // directory entries. If there's no nftoken page, we will
+        // give empty array for account_objects.
+        if (mlimit >= limit)
+            jvResult[jss::account_objects] = Json::arrayValue;
+
+        // non-zero dirIndex validity was checked in the beginning of this
+        // function; by this point, it should be zero. This function returns
+        // true regardless of nftoken page presence; if absent, account_objects
+        // is already set as an empty array. Notice we will only return false in
+        // this function when entryIndex can not be found, indicating an invalid
+        // marker error.
+        return true;
+    }
+
+    std::uint32_t i = 0;
+    for (;;)
+    {
+        auto const& entries = dir->getFieldV256(sfIndexes);
+        auto iter = entries.begin();
+
+        if (!found)
+        {
+            iter = std::find(iter, entries.end(), entryIndex);
+            if (iter == entries.end())
+                return false;
+
+            found = true;
+        }
+
+        // it's possible that the returned NFTPages exactly filled the
+        // response.  Check for that condition.
+        if (i == mlimit && mlimit < limit)
+        {
+            jvResult[jss::limit] = limit;
+            jvResult[jss::marker] =
+                to_string(dirIndex) + ',' + to_string(*iter);
+            return true;
+        }
+
+        for (; iter != entries.end(); ++iter)
+        {
+            auto const sleNode = ledger.read(keylet::child(*iter));
+
+            if (!typeFilter.has_value() ||
+                typeMatchesFilter(typeFilter.value(), sleNode->getType()))
+            {
+                jvObjects.append(sleNode->getJson(JsonOptions::none));
+            }
+
+            if (++i == mlimit)
+            {
+                if (++iter != entries.end())
+                {
+                    jvResult[jss::limit] = limit;
+                    jvResult[jss::marker] =
+                        to_string(dirIndex) + ',' + to_string(*iter);
+                    return true;
+                }
+
+                break;
+            }
+        }
+
+        auto const nodeIndex = dir->getFieldU64(sfIndexNext);
+        if (nodeIndex == 0)
+            return true;
+
+        dirIndex = keylet::page(root, nodeIndex).key;
+        dir = ledger.read({ltDIR_NODE, dirIndex});
+        if (!dir)
+            return true;
+
+        if (i == mlimit)
+        {
+            auto const& e = dir->getFieldV256(sfIndexes);
+            if (!e.empty())
+            {
+                jvResult[jss::limit] = limit;
+                jvResult[jss::marker] =
+                    to_string(dirIndex) + ',' + to_string(*e.begin());
+            }
+
+            return true;
+        }
+    }
+}
+
 Json::Value
 doAccountObjects(RPC::JsonContext& context)
 {
@@ -265,7 +458,7 @@ doAccountObjects(RPC::JsonContext& context)
             return RPC::invalid_field_error(jss::marker);
     }
 
-    if (!RPC::getAccountObjects(
+    if (!getAccountObjects(
             *ledger,
             accountID,
             typeFilter,

src/xrpld/rpc/handlers/AccountOffers.cpp

@@ -1,5 +1,6 @@
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 #include <xrpld/rpc/detail/Tuning.h>
 
 #include <xrpl/json/json_value.h>

src/xrpld/rpc/handlers/BookOffers.cpp

@@ -3,6 +3,7 @@
 #include <xrpld/rpc/BookChanges.h>
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 
 #include <xrpl/basics/Log.h>
 #include <xrpl/ledger/ReadView.h>

src/xrpld/rpc/handlers/DepositAuthorized.cpp

@@ -1,5 +1,5 @@
 #include <xrpld/rpc/Context.h>
-#include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 
 #include <xrpl/ledger/CredentialHelpers.h>
 #include <xrpl/ledger/ReadView.h>

src/xrpld/rpc/handlers/GatewayBalances.cpp

@@ -1,7 +1,7 @@
 #include <xrpld/app/main/Application.h>
 #include <xrpld/app/paths/TrustLine.h>
 #include <xrpld/rpc/Context.h>
-#include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/protocol/AccountID.h>

src/xrpld/rpc/handlers/GetAggregatePrice.cpp

@@ -1,7 +1,7 @@
 #include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpld/app/main/Application.h>
 #include <xrpld/rpc/Context.h>
-#include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 
 #include <xrpl/json/json_value.h>
 #include <xrpl/ledger/ReadView.h>

src/xrpld/rpc/handlers/GetCounts.cpp

@@ -95,7 +95,7 @@ getCountsJson(Application& app, int minObjectCount)
     ret[jss::treenode_cache_size] =
         app.getNodeFamily().getTreeNodeCache()->getCacheSize();
     ret[jss::treenode_track_size] =
-        static_cast<int>(app.getNodeFamily().getTreeNodeCache()->size());
+        app.getNodeFamily().getTreeNodeCache()->getTrackSize();
 
     std::string uptime;
     auto s = UptimeClock::now();

src/xrpld/rpc/handlers/LedgerData.cpp

@@ -3,6 +3,7 @@
 #include <xrpld/rpc/GRPCHandlers.h>
 #include <xrpld/rpc/Role.h>
 #include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 #include <xrpld/rpc/detail/Tuning.h>
 
 #include <xrpl/ledger/ReadView.h>

src/xrpld/rpc/handlers/LedgerDiff.cpp

@@ -1,5 +1,5 @@
 #include <xrpld/rpc/GRPCHandlers.h>
-#include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 
 namespace ripple {
 std::pair<org::xrpl::rpc::v1::GetLedgerDiffResponse, grpc::Status>

src/xrpld/rpc/handlers/LedgerEntry.cpp

@@ -1,6 +1,6 @@
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/GRPCHandlers.h>
-#include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 #include <xrpld/rpc/handlers/LedgerEntryHelpers.h>
 
 #include <xrpl/basics/StringUtilities.h>
@@ -16,8 +16,6 @@
 #include <xrpl/protocol/STXChainBridge.h>
 #include <xrpl/protocol/jss.h>
 
-#include <functional>
-
 namespace ripple {
 
 static Expected<uint256, Json::Value>
@@ -178,18 +176,41 @@ static Expected<STArray, Json::Value>
 parseAuthorizeCredentials(Json::Value const& jv)
 {
     if (!jv.isArray())
+    {
         return LedgerEntryHelpers::invalidFieldError(
             "malformedAuthorizedCredentials",
             jss::authorized_credentials,
             "array");
-    STArray arr(sfAuthorizeCredentials, jv.size());
+    }
+
+    std::uint32_t const n = jv.size();
+    if (n > maxCredentialsArraySize)
+    {
+        return Unexpected(LedgerEntryHelpers::malformedError(
+            "malformedAuthorizedCredentials",
+            "Invalid field '" + std::string(jss::authorized_credentials) +
+                "', array too long."));
+    }
+
+    if (n == 0)
+    {
+        return Unexpected(LedgerEntryHelpers::malformedError(
+            "malformedAuthorizedCredentials",
+            "Invalid field '" + std::string(jss::authorized_credentials) +
+                "', array empty."));
+    }
+
+    STArray arr(sfAuthorizeCredentials, n);
     for (auto const& jo : jv)
     {
         if (!jo.isObject())
+        {
             return LedgerEntryHelpers::invalidFieldError(
                 "malformedAuthorizedCredentials",
                 jss::authorized_credentials,
                 "array");
+        }
+
         if (auto const value = LedgerEntryHelpers::hasRequired(
                 jo,
                 {jss::issuer, jss::credential_type},
@@ -260,13 +281,6 @@ parseDepositPreauth(Json::Value const& dp, Json::StaticString const fieldName)
     auto const arr = parseAuthorizeCredentials(ac);
     if (!arr.has_value())
         return Unexpected(arr.error());
-    if (arr->empty() || (arr->size() > maxCredentialsArraySize))
-    {
-        return LedgerEntryHelpers::invalidFieldError(
-            "malformedAuthorizedCredentials",
-            jss::authorized_credentials,
-            "array");
-    }
 
     auto const& sorted = credentials::makeSorted(arr.value());
     if (sorted.empty())

src/xrpld/rpc/handlers/LedgerHandler.cpp

@@ -3,7 +3,7 @@
 #include <xrpld/app/misc/LoadFeeTrack.h>
 #include <xrpld/rpc/GRPCHandlers.h>
 #include <xrpld/rpc/Role.h>
-#include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 #include <xrpld/rpc/handlers/LedgerHandler.h>
 
 #include <xrpl/protocol/ErrorCodes.h>

src/xrpld/rpc/handlers/LedgerHeader.cpp

@@ -1,5 +1,5 @@
 #include <xrpld/app/ledger/LedgerToJson.h>
-#include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 
 #include <xrpl/basics/strHex.h>
 #include <xrpl/ledger/ReadView.h>

src/xrpld/rpc/handlers/LedgerRequest.cpp

@@ -1,6 +1,6 @@
 #include <xrpld/app/ledger/LedgerToJson.h>
 #include <xrpld/rpc/Context.h>
-#include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 
 #include <xrpl/protocol/ErrorCodes.h>
 #include <xrpl/protocol/jss.h>

src/xrpld/rpc/handlers/NFTOffers.cpp

@@ -1,5 +1,6 @@
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 #include <xrpld/rpc/detail/Tuning.h>
 
 #include <xrpl/json/json_value.h>

src/xrpld/rpc/handlers/NoRippleCheck.cpp

@@ -3,6 +3,7 @@
 #include <xrpld/app/paths/TrustLine.h>
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 #include <xrpld/rpc/detail/Tuning.h>
 
 #include <xrpl/ledger/ReadView.h>

src/xrpld/rpc/handlers/RipplePathFind.cpp

@@ -2,7 +2,7 @@
 #include <xrpld/app/paths/PathRequests.h>
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/detail/LegacyPathFind.h>
-#include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 
 #include <xrpl/protocol/RPCErr.h>
 #include <xrpl/resource/Fees.h>

src/xrpld/rpc/handlers/TransactionEntry.cpp

@@ -1,7 +1,7 @@
 #include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpld/app/misc/DeliverMax.h>
 #include <xrpld/rpc/Context.h>
-#include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/protocol/jss.h>

src/xrpld/rpc/handlers/VaultInfo.cpp

@@ -1,5 +1,5 @@
 #include <xrpld/rpc/Context.h>
-#include <xrpld/rpc/detail/RPCHelpers.h>
+#include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 
 #include <xrpl/beast/utility/Zero.h>
 #include <xrpl/json/json_value.h>