fix: (DRAFT) Combine #5531 and #7763

BUILD.md

@@ -41,7 +41,10 @@ Our Linux CI tooling is distro-independent and uses a Nix-based environment, so
 
 ### macOS
 
-Many `xrpld` engineers use macOS for development.
+Many `xrpld` engineers use macOS for development. The minimum supported version
+is macOS 26, which is also what our CI builds and tests against. The build
+defaults `CMAKE_OSX_DEPLOYMENT_TARGET` accordingly, so you do not need to pass it
+yourself.
 
 ### Windows
 

CMakeLists.txt

@@ -13,6 +13,21 @@ if(DEFINED CMAKE_MODULE_PATH)
 endif()
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
 
+# Default the macOS deployment target so it does not have to be passed on the
+# command line. Must be set before project() because project() consumes it when
+# configuring the compiler and SDK. A user-provided -DCMAKE_OSX_DEPLOYMENT_TARGET
+# still takes precedence.
+if(
+    CMAKE_HOST_SYSTEM_NAME STREQUAL "Darwin"
+    AND NOT DEFINED CMAKE_OSX_DEPLOYMENT_TARGET
+)
+    set(CMAKE_OSX_DEPLOYMENT_TARGET
+        "26.0"
+        CACHE STRING
+        "Minimum macOS deployment version"
+    )
+endif()
+
 project(xrpl)
 set(CMAKE_CXX_EXTENSIONS OFF)
 set(CMAKE_CXX_STANDARD 23)

include/xrpl/basics/TaggedCache.ipp

@@ -623,10 +623,11 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
             // Allocate the current size plus a little extra, in case the cache grows while
             // allocating. Each time another allocation is needed, the extra also gets bigger until
             // it ultimately doubles the size + 1.
-            constexpr std::size_t baseShift = 5;
+            constexpr std::size_t baseShift = 63;
             auto const bufferOffset = std::min(allocationIterations, std::size_t{baseShift});
             auto const bufferShift = baseShift - bufferOffset;
-            size += (size >> bufferShift) + 1;
+            auto const buffer = (size >> bufferShift) + 1;
+            size += buffer;
             v.reserve(size);
             ++allocationIterations;
         }

include/xrpl/nodestore/DatabaseRotating.h

@@ -4,6 +4,7 @@
 #include <xrpl/nodestore/Backend.h>
 #include <xrpl/nodestore/Database.h>
 #include <xrpl/nodestore/Scheduler.h>
+#include <xrpl/protocol/Protocol.h>
 
 #include <functional>
 #include <memory>
@@ -53,7 +54,7 @@ class DatabaseRotating : public Database
      * dropped.
      */
     virtual void
-    setRotationInFlight(bool inFlight) = 0;
+    setRotationInFlight(LedgerIndex inFlight) = 0;
 };
 
 }  // namespace xrpl::NodeStore

include/xrpl/nodestore/detail/DatabaseRotatingImp.h

@@ -8,6 +8,7 @@
 #include <xrpl/nodestore/DatabaseRotating.h>
 #include <xrpl/nodestore/NodeObject.h>
 #include <xrpl/nodestore/Scheduler.h>
+#include <xrpl/protocol/Protocol.h>
 
 #include <atomic>
 #include <cstdint>
@@ -71,20 +72,25 @@ class DatabaseRotatingImp : public DatabaseRotating
     sweep() override;
 
     void
-    setRotationInFlight(bool inFlight) override;
+    setRotationInFlight(LedgerIndex inFlight) override;
 
 private:
     std::shared_ptr<Backend> writableBackend_;
     std::shared_ptr<Backend> archiveBackend_;
     mutable std::mutex mutex_;
 
-    // True between SHAMapStore starting the cache-freshen phase and the
-    // completion of rotate(). While true, archive hits on ordinary
-    // (duplicate == false) fetches are copied forward into the writable
-    // backend; copyForwardCount_ tallies them per rotation for the
+    // Set to the index of the last rotated ledger between SHAMapStore
+    // starting the cache-freshen phase and the completion of rotate().
+    // While non-zero, archive hits on ordinary (duplicate == false)
+    // fetches are copied forward into the writable backend if they are
+    // for that ledger or later, since those are the ones we'll keep.
+    // To be safe, copy forward if the provided ledger index is 0.
+    // copyForwardCount_ tallies them per rotation for the
     // summary line logged at swap.
-    std::atomic<bool> rotationInFlight_{false};
+    // copyRejectCount_ tallies the ones that weren't copied.
+    std::atomic<LedgerIndex> rotationInFlight_{0};
     std::atomic<std::uint64_t> copyForwardCount_{0};
+    std::atomic<std::uint64_t> copyRejectCount_{0};
 
     std::shared_ptr<NodeObject>
     fetchNodeObject(uint256 const& hash, std::uint32_t, FetchReport& fetchReport, bool duplicate)

include/xrpl/shamap/SHAMapMissingNode.h

@@ -35,13 +35,15 @@
 class SHAMapMissingNode : public std::runtime_error
 {
 public:
-    SHAMapMissingNode(SHAMapType t, SHAMapHash const& hash)
-        : std::runtime_error("Missing Node: " + to_string(t) + ": hash " + to_string(hash))
+    SHAMapMissingNode(SHAMapType t, SHAMapHash const& hash, std::string const& location)
+        : std::runtime_error(
+              "Missing Node: " + to_string(t) + ": hash " + to_string(hash) + " in: " + location)
     {
     }
 
-    SHAMapMissingNode(SHAMapType t, uint256 const& id)
-        : std::runtime_error("Missing Node: " + to_string(t) + ": id " + to_string(id))
+    SHAMapMissingNode(SHAMapType t, uint256 const& id, std::string const& location)
+        : std::runtime_error(
+              "Missing Node: " + to_string(t) + ": id " + to_string(id) + " in: " + location)
     {
     }
 };

src/libxrpl/ledger/Ledger.cpp

@@ -746,7 +746,8 @@
     if (stateMap_.getHash().isZero() && !header_.accountHash.isZero() &&
         !stateMap_.fetchRoot(SHAMapHash{header_.accountHash}, nullptr))
     {
-        missingNodes1.emplace_back(SHAMapType::STATE, SHAMapHash{header_.accountHash});
+        missingNodes1.emplace_back(
+            SHAMapType::STATE, SHAMapHash{header_.accountHash}, "Ledger::walkLedger");
     }
     else
     {
@@ -770,7 +771,8 @@
     if (txMap_.getHash().isZero() && header_.txHash.isNonZero() &&
         !txMap_.fetchRoot(SHAMapHash{header_.txHash}, nullptr))
     {
-        missingNodes2.emplace_back(SHAMapType::TRANSACTION, SHAMapHash{header_.txHash});
+        missingNodes2.emplace_back(
+            SHAMapType::TRANSACTION, SHAMapHash{header_.txHash}, "Ledger::walkLedger");
     }
     else
     {

src/libxrpl/nodestore/DatabaseRotatingImp.cpp

@@ -12,6 +12,7 @@
 #include <xrpl/nodestore/NodeObject.h>
 #include <xrpl/nodestore/Scheduler.h>
 #include <xrpl/nodestore/Types.h>
+#include <xrpl/protocol/Protocol.h>
 
 #include <atomic>
 #include <cstdint>
@@ -54,6 +55,7 @@
     // deleted.
     std::shared_ptr<NodeStore::Backend> oldArchiveBackend;
     std::uint64_t copyForwards = 0;
+    std::uint64_t copyRejects = 0;
     {
         std::scoped_lock const lock(mutex_);
 
@@ -66,24 +68,25 @@
         writableBackend_ = std::move(newBackend);
 
         copyForwards = copyForwardCount_.exchange(0, std::memory_order_relaxed);
+        copyRejects = copyRejectCount_.exchange(0, std::memory_order_relaxed);
     }
 
     if (copyForwards > 0)
     {
         JLOG(j_.warn()) << "Rotating: copied forward " << copyForwards
                         << " archive-served reads into the writable backend "
-                           "during the rotation window";
+                           "during the rotation window. Rejected "
+                        << copyRejects;
     }
 
     f(newWritableBackendName, newArchiveBackendName);
 }
 
 void
-DatabaseRotatingImp::setRotationInFlight(bool inFlight)
+DatabaseRotatingImp::setRotationInFlight(LedgerIndex inFlight)
 {
     rotationInFlight_.store(inFlight, std::memory_order_release);
-    JLOG(j_.debug()) << "Rotating: copy-forward on archive reads "
-                     << (inFlight ? "enabled" : "disabled");
+    JLOG(j_.debug()) << "Rotating: copy-forward on archive reads from " << inFlight << " forward";
 }
 
 std::string
@@ -141,7 +144,7 @@
 std::shared_ptr<NodeObject>
 DatabaseRotatingImp::fetchNodeObject(
     uint256 const& hash,
-    std::uint32_t,
+    std::uint32_t ledgerSeq,
     FetchReport& fetchReport,
     bool duplicate)
 {
@@ -190,24 +193,35 @@
         nodeObject = fetch(archive);
         if (nodeObject)
         {
-            {
-                // Refresh the writable backend pointer
-                std::scoped_lock const lock(mutex_);
-                writable = writableBackend_;
-            }
-
             // Update writable backend with data from the archive backend.
             // While a rotation is in flight, ordinary (duplicate == false)
             // reads served by the archive are copied forward too: the
             // archive is about to be deleted, and a body canonicalized
             // into the cache after the freshen getKeys() snapshot would
             // otherwise survive only in RAM once the archive is dropped.
-            if (duplicate || rotationInFlight_.load(std::memory_order_acquire))
+            auto const inFlight = rotationInFlight_.load(std::memory_order_acquire);
+            if (duplicate || (inFlight != 0 && (ledgerSeq == 0 || ledgerSeq >= inFlight)))
             {
+                {
+                    // Refresh the writable backend pointer
+                    std::scoped_lock const lock(mutex_);
+                    writable = writableBackend_;
+                }
+
                 if (!duplicate)
+                {
+                    JLOG(j_.warn()) << "Rotating: copy node for ledger " << ledgerSeq
+                                    << " from archive to writable backend: " << hash;
                     copyForwardCount_.fetch_add(1, std::memory_order_relaxed);
+                }
                 writable->store(nodeObject);
             }
+            else if (inFlight != 0)
+            {
+                JLOG(j_.warn()) << "Rotating: DO NOT copy node for ledger " << ledgerSeq
+                                << " from archive to writable backend: " << hash;
+                copyRejectCount_.fetch_add(1, std::memory_order_relaxed);
+            }
         }
     }
 

src/libxrpl/shamap/SHAMap.cpp

@@ -272,7 +272,7 @@
     auto node = fetchNodeNT(hash);
 
     if (!node)
-        Throw<SHAMapMissingNode>(type_, hash);
+        Throw<SHAMapMissingNode>(type_, hash, "fetchNode");
 
     return node;
 }
@@ -283,7 +283,7 @@
     SHAMapTreeNode* ret = descend(parent, branch);  // NOLINT(misc-const-correctness)
 
     if ((ret == nullptr) && !parent->isEmptyBranch(branch))
-        Throw<SHAMapMissingNode>(type_, parent->getChildHash(branch));
+        Throw<SHAMapMissingNode>(type_, parent->getChildHash(branch), "descendThrow");
 
     return ret;
 }
@@ -294,7 +294,7 @@
     SHAMapTreeNodePtr ret = descend(parent, branch);
 
     if (!ret && !parent.isEmptyBranch(branch))
-        Throw<SHAMapMissingNode>(type_, parent.getChildHash(branch));
+        Throw<SHAMapMissingNode>(type_, parent.getChildHash(branch), "descendThrow");
 
     return ret;
 }
@@ -566,7 +566,7 @@
                 node = descendThrow(*inner, i);
                 auto leaf = firstBelow(node, stack, i);
                 if (leaf == nullptr)
-                    Throw<SHAMapMissingNode>(type_, id);
+                    Throw<SHAMapMissingNode>(type_, id, "peekNextItem");
                 XRPL_ASSERT(leaf->isLeaf(), "xrpl::SHAMap::peekNextItem : leaf is valid");
                 return leaf;
             }
@@ -624,7 +624,7 @@
                     node = descendThrow(*inner, branch);
                     auto leaf = firstBelow(node, stack, branch);
                     if (leaf == nullptr)
-                        Throw<SHAMapMissingNode>(type_, id);
+                        Throw<SHAMapMissingNode>(type_, id, "upperBound");
                     return ConstIterator(this, leaf->peekItem().get(), std::move(stack));
                 }
             }
@@ -657,7 +657,7 @@
                     node = descendThrow(*inner, branch);
                     auto leaf = lastBelow(node, stack, branch);
                     if (leaf == nullptr)
-                        Throw<SHAMapMissingNode>(type_, id);
+                        Throw<SHAMapMissingNode>(type_, id, "lowerBound");
                     return ConstIterator(this, leaf->peekItem().get(), std::move(stack));
                 }
             }
@@ -684,7 +684,7 @@
     walkTowardsKey(id, &stack);
 
     if (stack.empty())
-        Throw<SHAMapMissingNode>(type_, id);
+        Throw<SHAMapMissingNode>(type_, id, "delItem");
 
     auto leaf = intr_ptr::dynamicPointerCast<SHAMapLeafNode>(stack.top().first);
     stack.pop();
@@ -770,7 +770,7 @@
     walkTowardsKey(tag, &stack);
 
     if (stack.empty())
-        Throw<SHAMapMissingNode>(type_, tag);
+        Throw<SHAMapMissingNode>(type_, tag, "addGiveItem");
 
     auto [node, nodeID] = stack.top();
     stack.pop();
@@ -857,7 +857,7 @@
     walkTowardsKey(tag, &stack);
 
     if (stack.empty())
-        Throw<SHAMapMissingNode>(type_, tag);
+        Throw<SHAMapMissingNode>(type_, tag, "updateGiveItem");
 
     auto node = intr_ptr::dynamicPointerCast<SHAMapLeafNode>(stack.top().first);
     auto nodeID = stack.top().second;

src/libxrpl/shamap/SHAMapDelta.cpp

@@ -153,7 +153,7 @@ SHAMap::compare(SHAMap const& otherMap, Delta& differences, int maxCount) const
         {
             // LCOV_EXCL_START
             UNREACHABLE("xrpl::SHAMap::compare : missing a node");
-            Throw<SHAMapMissingNode>(type_, uint256());
+            Throw<SHAMapMissingNode>(type_, uint256(), "compare");
             // LCOV_EXCL_STOP
         }
 
@@ -270,7 +270,7 @@ SHAMap::walkMap(std::vector<SHAMapMissingNode>& missingNodes, int maxMissing) co
                 }
                 else
                 {
-                    missingNodes.emplace_back(type_, node->getChildHash(i));
+                    missingNodes.emplace_back(type_, node->getChildHash(i), "walkMap");
                     if (--maxMissing <= 0)
                         return;
                 }
@@ -344,7 +344,8 @@ SHAMap::walkMapParallel(std::vector<SHAMapMissingNode>& missingNodes, int maxMis
                             else
                             {
                                 std::scoped_lock const l{m};
-                                missingNodes.emplace_back(type_, node->getChildHash(i));
+                                missingNodes.emplace_back(
+                                    type_, node->getChildHash(i), "walkMapParallel");
                                 if (--maxMissing <= 0)
                                     return;
                             }