refactor: Introduce preflight helper functions

Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>

include/xrpl/basics/TaggedCache.ipp

@@ -3,6 +3,9 @@
 #include <xrpl/basics/IntrusivePointer.ipp>
 #include <xrpl/basics/Log.h>  // IWYU pragma: keep
 #include <xrpl/basics/TaggedCache.h>
+#include <xrpl/basics/scope.h>
+
+#include <algorithm>
 
 namespace xrpl {
 
@@ -601,8 +604,42 @@ TaggedCache<Key, T, IsKeyCache, SharedWeakUnionPointer, SharedPointerType, Hash,
     std::vector<key_type> v;
 
     {
-        std::scoped_lock const lock(mutex_);
-        v.reserve(cache_.size());
+        // Keep track of how many iterations are needed. Exit the loop if the number of retries gets
+        // absurd. (Note that if this somehow ever happens, one more allocation will be done under
+        // lock, which is undesirable, but really should be almost impossible.)
+        std::size_t allocationIterations = 0;
+        std::unique_lock lock(mutex_);
+        for (auto size = cache_.size(); v.capacity() < size && allocationIterations < 20;
+             size = cache_.size())
+        {
+            ScopeUnlock const unlock(lock);
+            if (allocationIterations > 0)
+            {
+                JLOG(journal_.info())
+                    << "getKeys(): Cache grew beyond allocated capacity after "
+                    << allocationIterations << " prior attempt(s). Have " << v.capacity()
+                    << ", need " << size << ". Retrying allocation";
+            }
+            // 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;
+            auto const bufferOffset = std::min(allocationIterations, std::size_t{baseShift});
+            auto const bufferShift = baseShift - bufferOffset;
+            size += (size >> bufferShift) + 1;
+            v.reserve(size);
+            ++allocationIterations;
+        }
+        if (v.capacity() < cache_.size())
+        {
+            // LCOV_EXCL_START
+            UNREACHABLE("xrpl::TaggedCache::getKeys(): failed to allocate sufficient capacity");
+            v.reserve(cache_.size());
+            // LCOV_EXCL_STOP
+        }
+        XRPL_ASSERT(lock.owns_lock(), "xrpl::TaggedCache::getKeys(): owns lock");
+        XRPL_ASSERT(
+            v.capacity() >= cache_.size(), "xrpl::TaggedCache::getKeys(): sufficient capacity");
         for (auto const& _ : cache_)
             v.push_back(_.first);
     }

include/xrpl/nodestore/DatabaseRotating.h

@@ -41,6 +41,19 @@ public:
         std::unique_ptr<NodeStore::Backend>&& newBackend,
         std::function<void(std::string const& writableName, std::string const& archiveName)> const&
             f) = 0;
+
+    /**
+     * Marks an online-delete rotation as in progress (or completed).
+     *
+     * While in flight, a read served by the archive backend is copied
+     * forward into the writable backend even for ordinary
+     * (duplicate == false) fetches: the archive is about to be deleted,
+     * and a node body canonicalized into caches during the rotation
+     * window would otherwise survive only in RAM once the archive is
+     * dropped.
+     */
+    virtual void
+    setRotationInFlight(bool inFlight) = 0;
 };
 
 }  // namespace xrpl::NodeStore

include/xrpl/nodestore/detail/DatabaseRotatingImp.h

@@ -9,6 +9,7 @@
 #include <xrpl/nodestore/NodeObject.h>
 #include <xrpl/nodestore/Scheduler.h>
 
+#include <atomic>
 #include <cstdint>
 #include <functional>
 #include <memory>
@@ -69,11 +70,22 @@ public:
     void
     sweep() override;
 
+    void
+    setRotationInFlight(bool 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
+    // summary line logged at swap.
+    std::atomic<bool> rotationInFlight_{false};
+    std::atomic<std::uint64_t> copyForwardCount_{0};
+
     std::shared_ptr<NodeObject>
     fetchNodeObject(uint256 const& hash, std::uint32_t, FetchReport& fetchReport, bool duplicate)
         override;

src/libxrpl/nodestore/DatabaseRotatingImp.cpp

@@ -13,6 +13,7 @@
 #include <xrpl/nodestore/Scheduler.h>
 #include <xrpl/nodestore/Types.h>
 
+#include <atomic>
 #include <cstdint>
 #include <exception>
 #include <functional>
@@ -52,6 +53,7 @@ DatabaseRotatingImp::rotate(
     // callback finishes. Only then will the archive directory be
     // deleted.
     std::shared_ptr<NodeStore::Backend> oldArchiveBackend;
+    std::uint64_t copyForwards = 0;
     {
         std::scoped_lock const lock(mutex_);
 
@@ -62,11 +64,28 @@ DatabaseRotatingImp::rotate(
         newArchiveBackendName = archiveBackend_->getName();
 
         writableBackend_ = std::move(newBackend);
+
+        copyForwards = copyForwardCount_.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";
     }
 
     f(newWritableBackendName, newArchiveBackendName);
 }
 
+void
+DatabaseRotatingImp::setRotationInFlight(bool inFlight)
+{
+    rotationInFlight_.store(inFlight, std::memory_order_release);
+    JLOG(j_.debug()) << "Rotating: copy-forward on archive reads "
+                     << (inFlight ? "enabled" : "disabled");
+}
+
 std::string
 DatabaseRotatingImp::getName() const
 {
@@ -177,9 +196,18 @@ DatabaseRotatingImp::fetchNodeObject(
                 writable = writableBackend_;
             }
 
-            // Update writable backend with data from the archive backend
-            if (duplicate)
+            // 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))
+            {
+                if (!duplicate)
+                    copyForwardCount_.fetch_add(1, std::memory_order_relaxed);
                 writable->store(nodeObject);
+            }
         }
     }
 

src/libxrpl/tx/PreflightHelpers.h

@@ -0,0 +1,68 @@
+#pragma once
+
+#include <xrpl/beast/utility/Zero.h>
+#include <xrpl/protocol/Rules.h>
+#include <xrpl/protocol/STAmount.h>
+#include <xrpl/protocol/TER.h>
+#include <xrpl/protocol/UintTypes.h>
+
+#include <optional>
+
+template <class T>
+inline bool
+checkBounds(T const& value, T const& min, T const& max)
+{
+    return value >= min && value <= max;
+}
+
+template <class T>
+inline bool
+checkMax(T const& value, T const& max)
+{
+    return value <= max;
+}
+
+template <class T>
+inline bool
+checkSize(T const& value, std::size_t const max)
+{
+    return value.size() <= max;
+}
+
+template <class T>
+inline bool
+checkSizeNonEmpty(T const& value, std::size_t const max)
+{
+    return value.size() <= max && !value.empty();
+}
+
+// Checks whether a hash-like identifier field (e.g. a uint256 object ID) is
+// unset/zero.
+template <class T>
+inline bool
+isZeroId(T const& id)
+{
+    return id == beast::kZero;
+}
+
+// Checks whether an amount is a strictly positive XRP amount.
+inline bool
+checkPositiveXRPAmount(xrpl::STAmount const& amount)
+{
+    return xrpl::isXRP(amount) && amount > beast::kZero;
+}
+
+// Checks whether an amount (of any asset type) is strictly positive.
+inline bool
+checkPositiveAmount(xrpl::STAmount const& amount)
+{
+    return amount > beast::kZero;
+}
+
+// Checks whether a currency code is the reserved "bad"/XRP currency code,
+// i.e. not a valid IOU currency.
+inline bool
+isBadCurrency(xrpl::Currency const& currency)
+{
+    return xrpl::badCurrency() == currency;
+}

src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp

@@ -33,6 +33,8 @@
 #include <xrpl/tx/Transactor.h>
 #include <xrpl/tx/applySteps.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 #include <memory>
 #include <system_error>
 #include <variant>
@@ -101,10 +103,10 @@ NotTEC
 escrowCreatePreflightHelper<Issue>(PreflightContext const& ctx)
 {
     STAmount const amount = ctx.tx[sfAmount];
-    if (amount.native() || amount <= beast::kZero)
+    if (amount.native() || !checkPositiveAmount(amount))
         return temBAD_AMOUNT;
 
-    if (badCurrency() == amount.get<Issue>().currency)
+    if (isBadCurrency(amount.get<Issue>().currency))
         return temBAD_CURRENCY;
 
     return tesSUCCESS;
@@ -118,7 +120,8 @@ escrowCreatePreflightHelper<MPTIssue>(PreflightContext const& ctx)
         return temDISABLED;
 
     auto const amount = ctx.tx[sfAmount];
-    if (amount.native() || amount.mpt() > MPTAmount{kMaxMpTokenAmount} || amount <= beast::kZero)
+    if (amount.native() || amount.mpt() > MPTAmount{kMaxMpTokenAmount} ||
+        !checkPositiveAmount(amount))
         return temBAD_AMOUNT;
 
     return tesSUCCESS;
@@ -141,7 +144,7 @@ EscrowCreate::preflight(PreflightContext const& ctx)
     }
     else
     {
-        if (amount <= beast::kZero)
+        if (!checkPositiveXRPAmount(amount))
             return temBAD_AMOUNT;
     }
 

src/libxrpl/tx/transactors/lending/LoanBrokerCoverClawback.cpp

@@ -27,6 +27,8 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 #include <expected>
 #include <optional>
 #include <variant>
@@ -48,7 +50,7 @@ LoanBrokerCoverClawback::preflight(PreflightContext const& ctx)
     if (!brokerID && !amount)
         return temINVALID;
 
-    if (brokerID && *brokerID == beast::kZero)
+    if (brokerID && isZeroId(*brokerID))
         return temINVALID;
 
     if (amount)

src/libxrpl/tx/transactors/lending/LoanBrokerCoverDeposit.cpp

@@ -16,6 +16,8 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 namespace xrpl {
 
 bool
@@ -27,11 +29,11 @@ LoanBrokerCoverDeposit::checkExtraFeatures(PreflightContext const& ctx)
 NotTEC
 LoanBrokerCoverDeposit::preflight(PreflightContext const& ctx)
 {
-    if (ctx.tx[sfLoanBrokerID] == beast::kZero)
+    if (isZeroId(ctx.tx[sfLoanBrokerID]))
         return temINVALID;
 
     auto const dstAmount = ctx.tx[sfAmount];
-    if (dstAmount <= beast::kZero)
+    if (!checkPositiveAmount(dstAmount))
         return temBAD_AMOUNT;
 
     if (!isLegalNet(dstAmount))

src/libxrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.cpp

@@ -20,6 +20,8 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 namespace xrpl {
 
 bool
@@ -31,11 +33,11 @@ LoanBrokerCoverWithdraw::checkExtraFeatures(PreflightContext const& ctx)
 NotTEC
 LoanBrokerCoverWithdraw::preflight(PreflightContext const& ctx)
 {
-    if (ctx.tx[sfLoanBrokerID] == beast::kZero)
+    if (isZeroId(ctx.tx[sfLoanBrokerID]))
         return temINVALID;
 
     auto const dstAmount = ctx.tx[sfAmount];
-    if (dstAmount <= beast::kZero)
+    if (!checkPositiveAmount(dstAmount))
         return temBAD_AMOUNT;
 
     if (!isLegalNet(dstAmount))
@@ -43,7 +45,7 @@ LoanBrokerCoverWithdraw::preflight(PreflightContext const& ctx)
 
     if (auto const destination = ctx.tx[~sfDestination])
     {
-        if (*destination == beast::kZero)
+        if (isZeroId(*destination))
         {
             return temMALFORMED;
         }

src/libxrpl/tx/transactors/lending/LoanBrokerDelete.cpp

@@ -18,6 +18,8 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 namespace xrpl {
 
 bool
@@ -29,7 +31,7 @@ LoanBrokerDelete::checkExtraFeatures(PreflightContext const& ctx)
 NotTEC
 LoanBrokerDelete::preflight(PreflightContext const& ctx)
 {
-    if (ctx.tx[sfLoanBrokerID] == beast::kZero)
+    if (isZeroId(ctx.tx[sfLoanBrokerID]))
         return temINVALID;
 
     return tesSUCCESS;

src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp

@@ -20,6 +20,8 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 #include <memory>
 #include <vector>
 
@@ -57,13 +59,13 @@ LoanBrokerSet::preflight(PreflightContext const& ctx)
             tx.isFieldPresent(sfCoverRateLiquidation))
             return temINVALID;
 
-        if (tx[sfLoanBrokerID] == beast::kZero)
+        if (isZeroId(tx[sfLoanBrokerID]))
             return temINVALID;
     }
 
     if (auto const vaultID = tx.at(~sfVaultID))
     {
-        if (*vaultID == beast::kZero)
+        if (isZeroId(*vaultID))
             return temINVALID;
     }
 

src/libxrpl/tx/transactors/lending/LoanDelete.cpp

@@ -16,6 +16,8 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 namespace xrpl {
 
 bool
@@ -27,7 +29,7 @@ LoanDelete::checkExtraFeatures(PreflightContext const& ctx)
 NotTEC
 LoanDelete::preflight(PreflightContext const& ctx)
 {
-    if (ctx.tx[sfLoanID] == beast::kZero)
+    if (isZeroId(ctx.tx[sfLoanID]))
         return temINVALID;
 
     return tesSUCCESS;

src/libxrpl/tx/transactors/lending/LoanManage.cpp

@@ -24,6 +24,8 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 #include <algorithm>
 #include <cstdint>
 namespace xrpl {
@@ -43,7 +45,7 @@ LoanManage::getFlagsMask(PreflightContext const& ctx)
 NotTEC
 LoanManage::preflight(PreflightContext const& ctx)
 {
-    if (ctx.tx[sfLoanID] == beast::kZero)
+    if (isZeroId(ctx.tx[sfLoanID]))
         return temINVALID;
 
     // Flags are mutually exclusive

src/libxrpl/tx/transactors/lending/LoanPay.cpp

@@ -25,6 +25,8 @@
 #include <xrpl/tx/Transactor.h>
 #include <xrpl/tx/transactors/lending/LoanManage.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 #include <algorithm>
 #include <bit>
 #include <cstdint>
@@ -48,10 +50,10 @@ LoanPay::getFlagsMask(PreflightContext const& ctx)
 NotTEC
 LoanPay::preflight(PreflightContext const& ctx)
 {
-    if (ctx.tx[sfLoanID] == beast::kZero)
+    if (isZeroId(ctx.tx[sfLoanID]))
         return temINVALID;
 
-    if (ctx.tx[sfAmount] <= beast::kZero)
+    if (!checkPositiveAmount(ctx.tx[sfAmount]))
         return temBAD_AMOUNT;
 
     // The loan payment flags are all mutually exclusive. If more than one is

src/libxrpl/tx/transactors/lending/LoanSet.cpp

@@ -28,6 +28,8 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 #include <cstddef>
 #include <cstdint>
 #include <limits>
@@ -138,7 +140,7 @@ LoanSet::preflight(PreflightContext const& ctx)
             return *ret;
     }
 
-    if (auto const brokerID = ctx.tx[~sfLoanBrokerID]; brokerID && *brokerID == beast::kZero)
+    if (auto const brokerID = ctx.tx[~sfLoanBrokerID]; brokerID && isZeroId(*brokerID))
         return temINVALID;
 
     return tesSUCCESS;

src/libxrpl/tx/transactors/payment/Payment.cpp

@@ -36,6 +36,8 @@
 #include <xrpl/tx/applySteps.h>
 #include <xrpl/tx/paths/RippleCalc.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 #include <algorithm>
 #include <cstdint>
 #include <limits>
@@ -143,7 +145,7 @@ Payment::preflight(PreflightContext const& ctx)
     // A zero DomainID is invalid for a PermissionedDomain ledger entry because
     // keylet::permissionedDomain(uint256) uses the DomainID as the ledger key.
     if (auto const domainID = tx[~sfDomainID];
-        ctx.rules.enabled(fixCleanup3_2_0) && domainID && *domainID == beast::kZero)
+        ctx.rules.enabled(fixCleanup3_2_0) && domainID && isZeroId(*domainID))
         return temMALFORMED;
 
     bool const partialPaymentAllowed = tx.isFlag(tfPartialPayment);
@@ -183,13 +185,13 @@ Payment::preflight(PreflightContext const& ctx)
                         << "Payment destination account not specified.";
         return temDST_NEEDED;
     }
-    if (hasMax && maxSourceAmount <= beast::kZero)
+    if (hasMax && !checkPositiveAmount(maxSourceAmount))
     {
         JLOG(j.trace()) << "Malformed transaction: bad max amount: "
                         << maxSourceAmount.getFullText();
         return temBAD_AMOUNT;
     }
-    if (dstAmount <= beast::kZero)
+    if (!checkPositiveAmount(dstAmount))
     {
         JLOG(j.trace()) << "Malformed transaction: bad dst amount: " << dstAmount.getFullText();
         return temBAD_AMOUNT;

src/libxrpl/tx/transactors/payment_channel/PaymentChannelClaim.cpp

@@ -22,6 +22,8 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 #include <cstdint>
 #include <optional>
 
@@ -42,15 +44,15 @@ PaymentChannelClaim::getFlagsMask(PreflightContext const&)
 NotTEC
 PaymentChannelClaim::preflight(PreflightContext const& ctx)
 {
-    if (ctx.rules.enabled(fixCleanup3_2_0) && ctx.tx[sfChannel] == beast::kZero)
+    if (ctx.rules.enabled(fixCleanup3_2_0) && isZeroId(ctx.tx[sfChannel]))
         return temMALFORMED;
 
     auto const bal = ctx.tx[~sfBalance];
-    if (bal && (!isXRP(*bal) || *bal <= beast::kZero))
+    if (bal && !checkPositiveXRPAmount(*bal))
         return temBAD_AMOUNT;
 
     auto const amt = ctx.tx[~sfAmount];
-    if (amt && (!isXRP(*amt) || *amt <= beast::kZero))
+    if (amt && !checkPositiveXRPAmount(*amt))
         return temBAD_AMOUNT;
 
     if (bal && amt && *bal > *amt)

src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp

@@ -22,6 +22,8 @@
 #include <xrpl/tx/Transactor.h>
 #include <xrpl/tx/applySteps.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 #include <memory>
 
 namespace xrpl {
@@ -57,7 +59,7 @@ PaymentChannelCreate::makeTxConsequences(PreflightContext const& ctx)
 NotTEC
 PaymentChannelCreate::preflight(PreflightContext const& ctx)
 {
-    if (!isXRP(ctx.tx[sfAmount]) || (ctx.tx[sfAmount] <= beast::kZero))
+    if (!checkPositiveXRPAmount(ctx.tx[sfAmount]))
         return temBAD_AMOUNT;
 
     if (ctx.tx[sfAccount] == ctx.tx[sfDestination])

src/libxrpl/tx/transactors/payment_channel/PaymentChannelFund.cpp

@@ -20,6 +20,8 @@
 #include <xrpl/tx/Transactor.h>
 #include <xrpl/tx/applySteps.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 namespace xrpl {
 
 TxConsequences
@@ -31,10 +33,10 @@ PaymentChannelFund::makeTxConsequences(PreflightContext const& ctx)
 NotTEC
 PaymentChannelFund::preflight(PreflightContext const& ctx)
 {
-    if (ctx.rules.enabled(fixCleanup3_2_0) && ctx.tx[sfChannel] == beast::kZero)
+    if (ctx.rules.enabled(fixCleanup3_2_0) && isZeroId(ctx.tx[sfChannel]))
         return temMALFORMED;
 
-    if (!isXRP(ctx.tx[sfAmount]) || (ctx.tx[sfAmount] <= beast::kZero))
+    if (!checkPositiveXRPAmount(ctx.tx[sfAmount]))
         return temBAD_AMOUNT;
 
     return tesSUCCESS;

src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainDelete.cpp

@@ -12,13 +12,15 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 namespace xrpl {
 
 NotTEC
 PermissionedDomainDelete::preflight(PreflightContext const& ctx)
 {
     auto const domain = ctx.tx.getFieldH256(sfDomainID);
-    if (domain == beast::kZero)
+    if (isZeroId(domain))
         return temMALFORMED;
 
     return tesSUCCESS;

src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp

@@ -17,6 +17,8 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 #include <memory>
 #include <utility>
 
@@ -39,7 +41,7 @@ PermissionedDomainSet::preflight(PreflightContext const& ctx)
         return err;
 
     auto const domain = ctx.tx.at(~sfDomainID);
-    if (domain && *domain == beast::kZero)
+    if (domain && isZeroId(*domain))
         return temMALFORMED;
 
     return tesSUCCESS;

src/libxrpl/tx/transactors/token/Clawback.cpp

@@ -22,6 +22,8 @@
 #include <xrpl/tx/ApplyContext.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 #include <algorithm>
 #include <variant>
 
@@ -44,7 +46,7 @@ preflightHelper<Issue>(PreflightContext const& ctx)
     // The issuer field is used for the token holder instead
     AccountID const& holder = clawAmount.getIssuer();
 
-    if (issuer == holder || isXRP(clawAmount) || clawAmount <= beast::kZero)
+    if (issuer == holder || isXRP(clawAmount) || !checkPositiveAmount(clawAmount))
         return temBAD_AMOUNT;
 
     return tesSUCCESS;
@@ -67,7 +69,7 @@ preflightHelper<MPTIssue>(PreflightContext const& ctx)
     if (ctx.tx[sfAccount] == *mptHolder)
         return temMALFORMED;
 
-    if (clawAmount.mpt() > MPTAmount{kMaxMpTokenAmount} || clawAmount <= beast::kZero)
+    if (clawAmount.mpt() > MPTAmount{kMaxMpTokenAmount} || !checkPositiveAmount(clawAmount))
         return temBAD_AMOUNT;
 
     return tesSUCCESS;

src/libxrpl/tx/transactors/token/MPTokenIssuanceCreate.cpp

@@ -21,6 +21,8 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 #include <cstdint>
 #include <expected>
 #include <memory>
@@ -87,7 +89,7 @@ MPTokenIssuanceCreate::preflight(PreflightContext const& ctx)
 
     if (auto const domain = ctx.tx[~sfDomainID])
     {
-        if (*domain == beast::kZero)
+        if (isZeroId(*domain))
             return temMALFORMED;
 
         // Domain present implies that MPTokenIssuance is not public

src/libxrpl/tx/transactors/token/TrustSet.cpp

@@ -25,6 +25,8 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 #include <cstdint>
 #include <unordered_set>
 
@@ -98,7 +100,7 @@ TrustSet::preflight(PreflightContext const& ctx)
         return temBAD_LIMIT;
     }
 
-    if (badCurrency() == saLimitAmount.get<Issue>().currency)
+    if (isBadCurrency(saLimitAmount.get<Issue>().currency))
     {
         JLOG(j.trace()) << "Malformed transaction: specifies XRP as IOU";
         return temBAD_CURRENCY;

src/libxrpl/tx/transactors/vault/VaultDelete.cpp

@@ -19,12 +19,14 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 namespace xrpl {
 
 NotTEC
 VaultDelete::preflight(PreflightContext const& ctx)
 {
-    if (ctx.tx[sfVaultID] == beast::kZero)
+    if (isZeroId(ctx.tx[sfVaultID]))
     {
         JLOG(ctx.j.debug()) << "VaultDelete: zero/empty vault ID.";
         return temMALFORMED;

src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp

@@ -23,6 +23,8 @@
 #include <xrpl/protocol/XRPAmount.h>
 #include <xrpl/tx/Transactor.h>
 
+#include <libxrpl/tx/PreflightHelpers.h>
+
 #include <stdexcept>
 
 namespace xrpl {
@@ -42,18 +44,18 @@ shouldWaiveWithdrawal(ReadView const& view, AccountID const& account, SLE::const
 NotTEC
 VaultWithdraw::preflight(PreflightContext const& ctx)
 {
-    if (ctx.tx[sfVaultID] == beast::kZero)
+    if (isZeroId(ctx.tx[sfVaultID]))
     {
         JLOG(ctx.j.debug()) << "VaultWithdraw: zero/empty vault ID.";
         return temMALFORMED;
     }
 
-    if (ctx.tx[sfAmount] <= beast::kZero)
+    if (!checkPositiveAmount(ctx.tx[sfAmount]))
         return temBAD_AMOUNT;
 
     if (auto const destination = ctx.tx[~sfDestination])
     {
-        if (*destination == beast::kZero)
+        if (isZeroId(*destination))
         {
             return temMALFORMED;
         }

src/xrpld/app/misc/SHAMapStoreImp.cpp

@@ -16,9 +16,11 @@
 #include <xrpl/ledger/Ledger.h>
 #include <xrpl/nodestore/Database.h>
 #include <xrpl/nodestore/Manager.h>
+#include <xrpl/nodestore/NodeObject.h>
 #include <xrpl/nodestore/Scheduler.h>
 #include <xrpl/nodestore/detail/DatabaseRotatingImp.h>
 #include <xrpl/protocol/Protocol.h>
+#include <xrpl/protocol/Serializer.h>
 #include <xrpl/server/NetworkOPs.h>
 #include <xrpl/server/State.h>
 #include <xrpl/shamap/SHAMapMissingNode.h>
@@ -254,8 +256,24 @@ bool
 SHAMapStoreImp::copyNode(std::uint64_t& nodeCount, SHAMapTreeNode const& node)
 {
     // Copy a single record from node to dbRotating_
-    dbRotating_->fetchNodeObject(
+    auto obj = dbRotating_->fetchNodeObject(
         node.getHash().asUInt256(), 0, NodeStore::FetchType::Synchronous, true);
+    if (!obj)
+    {
+        XRPL_ASSERT(node.cowid() == 0, "SHAMapStoreImp::copyNode : rescued node must be clean");
+        // Reachable from the validated state map in memory, but present in
+        // neither backend: its only on-disk copy lived in a backend removed by
+        // an earlier rotation, and it was never rewritten because it is clean
+        // (cowid == 0, so flushDirty skips it). Persist the in-memory body
+        // directly into the writable backend so it survives this rotation
+        // instead of later surfacing as an unresolvable SHAMapMissingNode.
+        auto const hash = node.getHash().asUInt256();
+        Serializer s;
+        node.serializeWithPrefix(s);
+        dbRotating_->store(NodeObjectType::AccountNode, std::move(s.modData()), hash, 0);
+        JLOG(journal_.warn()) << "copyNode: re-stored node missing from both backends, hash="
+                              << hash << " type=" << static_cast<int>(node.getType());
+    }
     if ((++nodeCount % checkHealthInterval_) == 0u)
     {
         if (healthWait() == HealthResult::Stopping)
@@ -348,6 +366,23 @@ SHAMapStoreImp::run()
             JLOG(journal_.debug())
                 << "copied ledger " << validatedSeq << " nodecount " << nodeCount;
 
+            // Close the getKeys()->swap exposure window: from here until
+            // rotate() completes, an ordinary read served by the archive is
+            // copied forward into the writable backend, so a node fetched
+            // from the doomed archive cannot be left RAM-only when the
+            // archive is deleted. RAII so the early returns below (and any
+            // exception) also clear the flag.
+            struct RotationExposureGuard
+            {
+                NodeStore::DatabaseRotating& db;
+                ~RotationExposureGuard()
+                {
+                    db.setRotationInFlight(false);
+                }
+            };
+            RotationExposureGuard const rotationExposureGuard{*dbRotating_};
+            dbRotating_->setRotationInFlight(true);
+
             JLOG(journal_.debug()) << "freshening caches";
             freshenCaches();
             if (healthWait() == HealthResult::Stopping)