Merge branch 'develop' into mvadari/refactor-tx-settings

.cspell.config.yaml

@@ -365,6 +365,7 @@ words:
   - xchain
   - ximinez
   - XMACRO
+  - xored
   - xrpkuwait
   - xrpl
   - xrpld

API-CHANGELOG.md

@@ -54,6 +54,8 @@ This section contains changes targeting a future version.
 - `submit`: The `fail_hard` field now returns an error if the value is not a boolean. [#6529](https://github.com/XRPLF/rippled/pull/6529)
 - `subscribe`: The `taker` field in the `books` array now returns `actMalformed` instead of `badIssuer` if the value is not a valid account. [#6529](https://github.com/XRPLF/rippled/pull/6529)
 - Fixed a bug in `Forwarded` HTTP header parsing where the extracted IP address could be incorrect when no comma or semicolon delimiter follows the address. This could cause the server to misidentify a client's IP address when operating behind a reverse proxy. [#6529](https://github.com/XRPLF/rippled/pull/6529)
+- `gateway_balances`: The `account` and `ident` fields now return an `invalidParams` error if the value is not a string, instead of an `internal` error. [#7655](https://github.com/XRPLF/rippled/pull/7655)
+- `account_lines`: The `peer` field now returns an error if the value is not a string. [#7728](https://github.com/XRPLF/rippled/pull/7728)
 
 ## XRP Ledger server version 3.1.0
 

cmake/XrplCompiler.cmake

@@ -188,6 +188,32 @@ else()
     endif()
 endif()
 
+# Linker warnings are errors where we control the toolchain and the dependencies: CI and the Nix dev shell.
+# On non-Nix macOS we suppress the deployment target warning: an old Conan profile may not pin os.version.
+if(is_macos OR is_linux)
+    if(is_ci OR is_nix_compiler)
+        if(is_macos)
+            set(fatal_warnings_flag "-Wl,-fatal_warnings")
+        else()
+            set(fatal_warnings_flag "-Wl,--fatal-warnings")
+        endif()
+        message(
+            STATUS
+            "Treating all linker warnings as errors (${fatal_warnings_flag})"
+        )
+        target_link_options(common INTERFACE "${fatal_warnings_flag}")
+        unset(fatal_warnings_flag)
+    elseif(is_macos)
+        set(silence_flag "-Wl,-deployment_target_mismatches,suppress")
+        message(
+            STATUS
+            "Silencing macOS deployment target mismatch warnings (${silence_flag})"
+        )
+        target_link_options(common INTERFACE "${silence_flag}")
+        unset(silence_flag)
+    endif()
+endif()
+
 # Antithesis instrumentation will only be built and deployed using machines running Linux.
 if(voidstar)
     if(NOT CMAKE_BUILD_TYPE STREQUAL "Debug")

cmake/XrplCov.cmake

@@ -44,6 +44,7 @@ setup_target_for_coverage_gcovr(
     EXCLUDE
         "src/test"
         "src/tests"
+        "src/benchmarks"
         "include/xrpl/beast/test"
         "include/xrpl/beast/unit_test"
         "${CMAKE_BINARY_DIR}/pb-xrpl.libpb"

include/xrpl/basics/Buffer.h

@@ -3,6 +3,7 @@
 #include <xrpl/basics/Slice.h>
 #include <xrpl/beast/utility/instrumentation.h>
 
+#include <algorithm>
 #include <cstdint>
 #include <cstring>
 #include <memory>
@@ -156,6 +157,19 @@ public:
     }
     /** @} */
 
+    /**
+     * Set every byte in the buffer to the given value.
+     *
+     * The size is unchanged, and this is a no-op on an empty buffer.
+     *
+     * @param value the byte to write to every position.
+     */
+    void
+    fill(std::uint8_t value) noexcept
+    {
+        std::fill_n(p_.get(), size_, value);
+    }
+
     /**
      * Reset the buffer.
      * All memory is deallocated. The resulting size is 0.

include/xrpl/json/json_value.h

@@ -4,6 +4,7 @@
 #include <xrpl/json/json_forwards.h>
 
 #include <cstring>
+#include <iterator>
 #include <limits>
 #include <map>
 #include <string>
@@ -524,6 +525,7 @@ public:
 class ValueIteratorBase
 {
 public:
+    using iterator_category = std::bidirectional_iterator_tag;
     using size_t = unsigned int;
     using difference_type = int;
     using SelfType = ValueIteratorBase;

include/xrpl/tx/invariants/FreezeInvariant.h

@@ -69,7 +69,8 @@ private:
         IssuerChanges const& changes,
         STTx const& tx,
         beast::Journal const& j,
-        bool enforce);
+        bool enforce,
+        bool fixOverrideFreeze);
 
     static bool
     validateFrozenState(
@@ -78,7 +79,8 @@ private:
         STTx const& tx,
         beast::Journal const& j,
         bool enforce,
-        bool globalFreeze);
+        bool globalFreeze,
+        bool fixOverrideFreeze);
 };
 
 }  // namespace xrpl

src/benchmarks/libxrpl/nodestore/Backend.cpp

@@ -41,10 +41,11 @@ struct RunState
     release()
     {
         harness.reset();
-        Batch{}.swap(present);
-        Batch{}.swap(recent);
-        std::vector<uint256>{}.swap(missing);
-        std::vector<std::size_t>{}.swap(shuffle);
+        present = Batch{};
+        recent = Batch{};
+        missing = std::vector<uint256>{};
+        shuffle = std::vector<std::size_t>{};
+        avgPayload = 0;
     }
 };
 
@@ -239,9 +240,13 @@ registerWorkload(BackendConfig const& bc, Workload const& w)
     if (!w.pinToPool)
     {
         auto rs = std::make_shared<RunState>();
-        auto* b = benchmark::RegisterBenchmark(name, makeRunner(w, cfg, rs));
-        b->RangeMultiplier(10)->Range(kPoolSizes.front(), kPoolSizes.back());
-        b->Threads(1)->Threads(4)->Threads(8)->UseRealTime();
+        benchmark::RegisterBenchmark(name, makeRunner(w, cfg, rs))
+            ->RangeMultiplier(10)
+            ->Range(kPoolSizes.front(), kPoolSizes.back())
+            ->Threads(1)
+            ->Threads(4)
+            ->Threads(8)
+            ->UseRealTime();
 
         return;
     }

src/benchmarks/libxrpl/nodestore/NodeStoreBench.h

@@ -297,12 +297,11 @@ struct BackendConfig
 inline std::vector<BackendConfig> const&
 backendConfigs()
 {
+    // Use factory settings for each DB
     static std::vector<BackendConfig> const kConfigs = {
         {.name = "nudb", .config = "type=nudb"},
 #if XRPL_ROCKSDB_AVAILABLE
-        {.name = "rocksdb",
-         .config = "type=rocksdb,open_files=2000,filter_bits=12,cache_mb=256,"
-                   "file_size_mb=8,file_size_mult=2"},
+        {.name = "rocksdb", .config = "type=rocksdb"},
 #endif
     };
     return kConfigs;

src/libxrpl/protocol/ConfidentialTransfer.cpp

@@ -4,6 +4,7 @@
 #include <xrpl/basics/Slice.h>
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/basics/contract.h>
+#include <xrpl/beast/utility/instrumentation.h>
 #include <xrpl/protocol/AccountID.h>
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
@@ -124,7 +125,12 @@ std::optional<EcPair>
 makeEcPair(Slice const& buffer)
 {
     if (buffer.length() != 2 * kEcCiphertextComponentLength)
-        return std::nullopt;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE("xrpl::makeEcPair : callers must pre-validate ciphertext length");
+        return std::nullopt;
+        // LCOV_EXCL_STOP
+    }
 
     auto parsePubKey = [](Slice const& slice, secp256k1_pubkey& out) {
         return secp256k1_ec_pubkey_parse(secp256k1Context(), &out, slice.data(), slice.length());
@@ -266,7 +272,13 @@ std::optional<Buffer>
 encryptCanonicalZeroAmount(Slice const& pubKeySlice, AccountID const& account, MPTID const& mptId)
 {
     if (pubKeySlice.size() != kEcPubKeyLength)
-        return std::nullopt;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::encryptCanonicalZeroAmount : callers must pre-validate public key length");
+        return std::nullopt;
+        // LCOV_EXCL_STOP
+    }
 
     EcPair pair{};
     secp256k1_pubkey pubKey;
@@ -274,14 +286,24 @@ encryptCanonicalZeroAmount(Slice const& pubKeySlice, AccountID const& account, M
             secp256k1Context(), &pubKey, pubKeySlice.data(), kEcPubKeyLength);
         res != 1)
     {
-        return std::nullopt;  // LCOV_EXCL_LINE
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::encryptCanonicalZeroAmount : public key read from the ledger must already be "
+            "valid");
+        return std::nullopt;
+        // LCOV_EXCL_STOP
     }
 
     if (auto res = generate_canonical_encrypted_zero(
             secp256k1Context(), &pair.c1, &pair.c2, &pubKey, account.data(), mptId.data());
         res != 1)
     {
-        return std::nullopt;  // LCOV_EXCL_LINE
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::encryptCanonicalZeroAmount : canonical zero generation cannot fail for a "
+            "valid public key");
+        return std::nullopt;
+        // LCOV_EXCL_STOP
     }
 
     return serializeEcPair(pair);
@@ -301,7 +323,11 @@ verifyRevealedAmount(
         issuer.publicKey.size() != kEcPubKeyLength ||
         issuer.encryptedAmount.size() != kEcGamalEncryptedTotalLength)
     {
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::verifyRevealedAmount : callers must pre-validate holder/issuer field lengths");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
     }
 
     auto const holderP = toParticipant(holder);
@@ -313,7 +339,11 @@ verifyRevealedAmount(
         if (auditor->publicKey.size() != kEcPubKeyLength ||
             auditor->encryptedAmount.size() != kEcGamalEncryptedTotalLength)
         {
-            return tecINTERNAL;  // LCOV_EXCL_LINE
+            // LCOV_EXCL_START
+            UNREACHABLE(
+                "xrpl::verifyRevealedAmount : callers must pre-validate auditor field lengths");
+            return tecINTERNAL;
+            // LCOV_EXCL_STOP
         }
         auditorP = toParticipant(*auditor);
         auditorPtr = &auditorP;
@@ -337,7 +367,12 @@ checkEncryptedAmountFormat(STObject const& object)
     if (!object.isFieldPresent(sfHolderEncryptedAmount) ||
         !object.isFieldPresent(sfIssuerEncryptedAmount))
     {
-        return temMALFORMED;  // LCOV_EXCL_LINE
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::checkEncryptedAmountFormat : callers already enforce that these fields are "
+            "present");
+        return temMALFORMED;
+        // LCOV_EXCL_STOP
     }
 
     if (object[sfHolderEncryptedAmount].length() != kEcGamalEncryptedTotalLength ||
@@ -366,7 +401,12 @@ TER
 verifySchnorrProof(Slice const& pubKeySlice, Slice const& proofSlice, uint256 const& contextHash)
 {
     if (proofSlice.size() != kEcSchnorrProofLength || pubKeySlice.size() != kEcPubKeyLength)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE("xrpl::verifySchnorrProof : callers must pre-validate proof/public key length");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     if (mpt_verify_convert_proof(proofSlice.data(), pubKeySlice.data(), contextHash.data()) != 0)
         return tecBAD_PROOF;
@@ -385,7 +425,12 @@ verifyClawbackProof(
     if (ciphertext.size() != kEcGamalEncryptedTotalLength ||
         pubKeySlice.size() != kEcPubKeyLength || proof.size() != kEcClawbackProofLength)
     {
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::verifyClawbackProof : callers must pre-validate ciphertext/public "
+            "key/proof length");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
     }
 
     if (mpt_verify_clawback_proof(
@@ -420,7 +465,12 @@ verifySendProof(
         amountCommitment.size() != kEcPedersenCommitmentLength ||
         balanceCommitment.size() != kEcPedersenCommitmentLength)
     {
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::verifySendProof : callers must pre-validate proof/participant/commitment "
+            "lengths");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
     }
 
     std::vector<mpt_confidential_participant> participants;
@@ -433,12 +483,22 @@ verifySendProof(
         if (auditor->publicKey.size() != kEcPubKeyLength ||
             auditor->encryptedAmount.size() != kEcGamalEncryptedTotalLength)
         {
-            return tecINTERNAL;  // LCOV_EXCL_LINE
+            // LCOV_EXCL_START
+            UNREACHABLE("xrpl::verifySendProof : callers must pre-validate auditor field lengths");
+            return tecINTERNAL;
+            // LCOV_EXCL_STOP
         }
         participants.push_back(toParticipant(*auditor));
     }
     if (participants.size() != recipientCount)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::verifySendProof : participant count must match the requested recipient "
+            "count");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     if (mpt_verify_send_proof(
             proof.data(),
@@ -468,7 +528,12 @@ verifyConvertBackProof(
         spendingBalance.size() != kEcGamalEncryptedTotalLength ||
         balanceCommitment.size() != kEcPedersenCommitmentLength)
     {
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::verifyConvertBackProof : callers must pre-validate proof/public "
+            "key/balance/commitment lengths");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
     }
 
     if (mpt_verify_convert_back_proof(

src/libxrpl/tx/invariants/FreezeInvariant.cpp

@@ -73,6 +73,7 @@ TransfersNotFrozen::finalize(
      *           view.rules().enabled(fixFreezeExploit);
      */
     [[maybe_unused]] bool const enforce = view.rules().enabled(featureDeepFreeze);
+    bool const fixOverrideFreeze = view.rules().enabled(fixCleanup3_4_0);
 
     return std::ranges::all_of(balanceChanges_, [&](auto const& entry) {
         auto const& [issue, changes] = entry;
@@ -90,7 +91,7 @@ TransfersNotFrozen::finalize(
             return !enforce;
         }
 
-        return validateIssuerChanges(issuerSle, changes, tx, j, enforce);
+        return validateIssuerChanges(issuerSle, changes, tx, j, enforce, fixOverrideFreeze);
     });
 }
 
@@ -199,7 +200,8 @@ TransfersNotFrozen::validateIssuerChanges(
     IssuerChanges const& changes,
     STTx const& tx,
     beast::Journal const& j,
-    bool enforce)
+    bool enforce,
+    bool fixOverrideFreeze)
 {
     if (!issuer)
     {
@@ -225,7 +227,7 @@ TransfersNotFrozen::validateIssuerChanges(
         {
             bool const high = change.line->at(sfLowLimit).getIssuer() == issuer->at(sfAccount);
 
-            if (!validateFrozenState(change, high, tx, j, enforce, globalFreeze))
+            if (!validateFrozenState(change, high, tx, j, enforce, globalFreeze, fixOverrideFreeze))
             {
                 return false;
             }
@@ -241,26 +243,29 @@ TransfersNotFrozen::validateFrozenState(
     STTx const& tx,
     beast::Journal const& j,
     bool enforce,
-    bool globalFreeze)
+    bool globalFreeze,
+    bool fixOverrideFreeze)
 {
     bool const freeze =
         change.balanceChangeSign < 0 && change.line->isFlag(high ? lsfLowFreeze : lsfHighFreeze);
     bool const deepFreeze = change.line->isFlag(high ? lsfLowDeepFreeze : lsfHighDeepFreeze);
     bool const frozen = globalFreeze || deepFreeze || freeze;
 
-    bool const isAMMLine = change.line->isFlag(lsfAMMNode);
-
     if (!frozen)
     {
         return true;
     }
 
-    // AMMClawbacks are allowed to override some freeze rules
-    if ((!isAMMLine || globalFreeze) && hasPrivilege(tx, OverrideFreeze))
+    // Pre-fixCleanup3_4_0: the isAMMLine check incorrectly blocked clawback on
+    // individually-frozen or deep-frozen AMM trust lines.
+    // Post-fixCleanup3_4_0: AMMClawbacks are allowed to override all freeze types.
+    bool const isAMMLine = change.line->isFlag(lsfAMMNode);
+    if ((fixOverrideFreeze || !isAMMLine || globalFreeze) && hasPrivilege(tx, OverrideFreeze))
     {
         JLOG(j.debug()) << "Invariant check allowing funds to be moved "
                         << (change.balanceChangeSign > 0 ? "to" : "from")
-                        << " a frozen trustline for AMMClawback " << tx.getTransactionID();
+                        << " a frozen trustline for a freeze privileged transaction "
+                        << tx.getTransactionID();
         return true;
     }
 

src/libxrpl/tx/transactors/dex/AMMClawback.cpp

@@ -256,7 +256,7 @@ AMMClawback::applyGuts(Sandbox& sb)
     }
 
     if (!isTesSuccess(result))
-        return result;  // LCOV_EXCL_LINE
+        return result;
 
     if (sb.rules().enabled(fixCleanup3_3_0) && sb.rules().enabled(fixAMMv1_3))
     {
@@ -353,6 +353,13 @@ AMMClawback::equalWithdrawMatchingOneAmount(
 
         auto amountRounded = getRoundedAsset(rules, amountBalance, frac, IsDeposit::No);
 
+        // The requested clawback amount is likely too small and results in
+        // one-sided pool withdrawal due to round off. Fail so the issuer can
+        // clawback a larger amount.
+        if (rules.enabled(fixCleanup3_4_0) &&
+            (amountRounded == beast::kZero || amount2Rounded == beast::kZero))
+            return {tecAMM_FAILED, STAmount{}, STAmount{}, STAmount{}};
+
         return AMMWithdraw::withdraw(
             sb,
             ammSle,

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

@@ -1,6 +1,7 @@
 #include <xrpl/tx/transactors/token/ConfidentialMPTClawback.h>
 
 #include <xrpl/beast/utility/Journal.h>
+#include <xrpl/beast/utility/instrumentation.h>
 #include <xrpl/core/ServiceRegistry.h>
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/protocol/ConfidentialTransfer.h>
@@ -70,7 +71,14 @@ ConfidentialMPTClawback::preclaim(PreclaimContext const& ctx)
 
     // Sanity check: account must be the same as issuer
     if (sleIssuance->getAccountID(sfIssuer) != account)
-        return tefINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTClawback::preclaim : preflight already validated the "
+            "submitter is the issuer");
+        return tefINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     // Check if issuance has issuer ElGamal public key
     if (!sleIssuance->isFieldPresent(sfIssuerEncryptionKey))
@@ -127,7 +135,14 @@ ConfidentialMPTClawback::doApply()
     auto sleHolderMPToken = view().peek(keylet::mptoken(mptIssuanceID, holder));
 
     if (!sleIssuance || !sleHolderMPToken)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTClawback::doApply : preclaim already validated these "
+            "objects exist");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     auto const clawAmount = ctx_.tx[sfMPTAmount];
 
@@ -137,11 +152,25 @@ ConfidentialMPTClawback::doApply()
     // After clawback, the balance should be encrypted zero.
     auto const encZeroForHolder = encryptCanonicalZeroAmount(holderPubKey, holder, mptIssuanceID);
     if (!encZeroForHolder)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTClawback::doApply : canonical zero encryption cannot fail "
+            "for an already-valid holder public key");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     auto encZeroForIssuer = encryptCanonicalZeroAmount(issuerPubKey, holder, mptIssuanceID);
     if (!encZeroForIssuer)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTClawback::doApply : canonical zero encryption cannot fail "
+            "for an already-valid issuer public key");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     // Set holder's confidential balances to encrypted zero
     (*sleHolderMPToken)[sfConfidentialBalanceInbox] = *encZeroForHolder;
@@ -154,14 +183,28 @@ ConfidentialMPTClawback::doApply()
         // Sanity check: the issuance must have an auditor public key if
         // auditing is enabled.
         if (!sleIssuance->isFieldPresent(sfAuditorEncryptionKey))
-            return tecINTERNAL;  // LCOV_EXCL_LINE
+        {
+            // LCOV_EXCL_START
+            UNREACHABLE(
+                "xrpl::ConfidentialMPTClawback::doApply : the holder's auditor balance implies "
+                "the issuance has an auditor public key");
+            return tecINTERNAL;
+            // LCOV_EXCL_STOP
+        }
 
         auto const auditorPubKey = (*sleIssuance)[sfAuditorEncryptionKey];
 
         auto encZeroForAuditor = encryptCanonicalZeroAmount(auditorPubKey, holder, mptIssuanceID);
 
         if (!encZeroForAuditor)
-            return tecINTERNAL;  // LCOV_EXCL_LINE
+        {
+            // LCOV_EXCL_START
+            UNREACHABLE(
+                "xrpl::ConfidentialMPTClawback::doApply : canonical zero encryption cannot "
+                "fail for an already-valid auditor public key");
+            return tecINTERNAL;
+            // LCOV_EXCL_STOP
+        }
 
         (*sleHolderMPToken)[sfAuditorEncryptedBalance] = std::move(*encZeroForAuditor);
     }

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

@@ -3,6 +3,7 @@
 #include <xrpl/basics/Log.h>
 #include <xrpl/basics/Slice.h>
 #include <xrpl/beast/utility/Journal.h>
+#include <xrpl/beast/utility/instrumentation.h>
 #include <xrpl/core/ServiceRegistry.h>
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/ledger/helpers/TokenHelpers.h>
@@ -89,7 +90,14 @@ ConfidentialMPTConvert::preclaim(PreclaimContext const& ctx)
     // already checked in preflight, but should also check that issuer on the
     // issuance isn't the account either
     if (sleIssuance->getAccountID(sfIssuer) == account)
-        return tefINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTConvert::preclaim : issuer derived from the MPT ID must "
+            "match the ledger's stored issuer");
+        return tefINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     bool const hasAuditor = ctx.tx.isFieldPresent(sfAuditorEncryptedAmount);
     bool const requiresAuditor = sleIssuance->isFieldPresent(sfAuditorEncryptionKey);
@@ -207,11 +215,25 @@ ConfidentialMPTConvert::doApply()
 
     auto sleMptoken = view().peek(keylet::mptoken(mptIssuanceID, accountID_));
     if (!sleMptoken)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTConvert::doApply : preclaim already validated the MPToken "
+            "exists");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     auto sleIssuance = view().peek(keylet::mptokenIssuance(mptIssuanceID));
     if (!sleIssuance)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTConvert::doApply : preclaim already validated the issuance "
+            "exists");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     auto const amtToConvert = ctx_.tx[sfMPTAmount];
     auto const amt = (*sleMptoken)[~sfMPTAmount].valueOr(0);
@@ -273,7 +295,14 @@ ConfidentialMPTConvert::doApply()
         if (auditorEc)
         {
             if (!sleMptoken->isFieldPresent(sfAuditorEncryptedBalance))
-                return tecINTERNAL;  // LCOV_EXCL_LINE
+            {
+                // LCOV_EXCL_START
+                UNREACHABLE(
+                    "xrpl::ConfidentialMPTConvert::doApply : issuance-level auditing implies "
+                    "the MPToken already carries an auditor balance");
+                return tecINTERNAL;
+                // LCOV_EXCL_STOP
+            }
 
             auto sum = homomorphicAdd(*auditorEc, (*sleMptoken)[sfAuditorEncryptedBalance]);
             if (!sum)
@@ -308,7 +337,14 @@ ConfidentialMPTConvert::doApply()
             (*sleMptoken)[sfHolderEncryptionKey], accountID_, mptIssuanceID);
 
         if (!zeroBalance)
-            return tecINTERNAL;  // LCOV_EXCL_LINE
+        {
+            // LCOV_EXCL_START
+            UNREACHABLE(
+                "xrpl::ConfidentialMPTConvert::doApply : canonical zero encryption cannot fail "
+                "for an already-valid holder public key");
+            return tecINTERNAL;
+            // LCOV_EXCL_STOP
+        }
 
         (*sleMptoken)[sfConfidentialBalanceSpending] = std::move(*zeroBalance);
     }
@@ -316,7 +352,12 @@ ConfidentialMPTConvert::doApply()
     {
         // both sfIssuerEncryptedBalance and sfConfidentialBalanceInbox should
         // exist together
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTConvert::doApply : confidential balance fields must be all "
+            "present or all absent");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
     }
 
     view().update(sleIssuance);

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

@@ -2,6 +2,7 @@
 
 #include <xrpl/basics/Log.h>
 #include <xrpl/beast/utility/Journal.h>
+#include <xrpl/beast/utility/instrumentation.h>
 #include <xrpl/core/ServiceRegistry.h>
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/ledger/helpers/TokenHelpers.h>
@@ -72,7 +73,14 @@ verifyProofs(
     std::shared_ptr<SLE const> const& mptoken)
 {
     if (!mptoken->isFieldPresent(sfHolderEncryptionKey))
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::verifyProofs : preclaim already validated the holder encryption key is "
+            "present");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     auto const mptIssuanceID = tx[sfMPTokenIssuanceID];
     auto const account = tx[sfAccount];
@@ -169,7 +177,14 @@ ConfidentialMPTConvertBack::preclaim(PreclaimContext const& ctx)
     // already checked in preflight, but should also check that issuer on
     // the issuance isn't the account either
     if (sleIssuance->getAccountID(sfIssuer) == account)
-        return tefINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTConvertBack::preclaim : issuer derived from the MPT ID must "
+            "match the ledger's stored issuer");
+        return tefINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     auto const sleMptoken = ctx.view.read(keylet::mptoken(mptIssuanceID, account));
     if (!sleMptoken)
@@ -185,7 +200,14 @@ ConfidentialMPTConvertBack::preclaim(PreclaimContext const& ctx)
     // Sanity check: holder's MPToken must have auditor balance field if auditing
     // is enabled
     if (requiresAuditor && !sleMptoken->isFieldPresent(sfAuditorEncryptedBalance))
-        return tefINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTConvertBack::preclaim : issuance-level auditing implies the "
+            "MPToken already carries an auditor balance");
+        return tefINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     // if the total circulating confidential balance is smaller than what the
     // holder is trying to convert back, we know for sure this txn should
@@ -215,11 +237,25 @@ ConfidentialMPTConvertBack::doApply()
 
     auto sleMptoken = view().peek(keylet::mptoken(mptIssuanceID, accountID_));
     if (!sleMptoken)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTConvertBack::doApply : preclaim already validated the "
+            "MPToken exists");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     auto sleIssuance = view().peek(keylet::mptokenIssuance(mptIssuanceID));
     if (!sleIssuance)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTConvertBack::doApply : preclaim already validated the "
+            "issuance exists");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     auto const amtToConvertBack = ctx_.tx[sfMPTAmount];
     auto const amt = (*sleMptoken)[~sfMPTAmount].valueOr(0);

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

@@ -2,6 +2,7 @@
 
 #include <xrpl/basics/Log.h>
 #include <xrpl/beast/utility/Journal.h>
+#include <xrpl/beast/utility/instrumentation.h>
 #include <xrpl/core/ServiceRegistry.h>
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/ledger/helpers/TokenHelpers.h>
@@ -49,7 +50,14 @@ ConfidentialMPTMergeInbox::preclaim(PreclaimContext const& ctx)
     // already checked in preflight, but should also check that issuer on the
     // issuance isn't the account either
     if (sleIssuance->getAccountID(sfIssuer) == ctx.tx[sfAccount])
-        return tefINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTMergeInbox::preclaim : issuer derived from the MPT ID must "
+            "match the ledger's stored issuer");
+        return tefINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     auto const sleMptoken =
         ctx.view.read(keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], ctx.tx[sfAccount]));
@@ -82,14 +90,26 @@ ConfidentialMPTMergeInbox::doApply()
     auto const mptIssuanceID = ctx_.tx[sfMPTokenIssuanceID];
     auto sleMptoken = view().peek(keylet::mptoken(mptIssuanceID, accountID_));
     if (!sleMptoken)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTMergeInbox::doApply : preclaim already validated the "
+            "MPToken exists");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     // sanity check
     if (!sleMptoken->isFieldPresent(sfConfidentialBalanceSpending) ||
         !sleMptoken->isFieldPresent(sfConfidentialBalanceInbox) ||
         !sleMptoken->isFieldPresent(sfHolderEncryptionKey))
     {
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTMergeInbox::doApply : preclaim already validated these "
+            "fields are present");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
     }
 
     // Merge inbox into spending: spending = spending + inbox
@@ -114,7 +134,14 @@ ConfidentialMPTMergeInbox::doApply()
         encryptCanonicalZeroAmount((*sleMptoken)[sfHolderEncryptionKey], accountID_, mptIssuanceID);
 
     if (!zeroEncryption)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTMergeInbox::doApply : canonical zero encryption cannot fail "
+            "for an already-valid holder public key");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     (*sleMptoken)[sfConfidentialBalanceInbox] = std::move(*zeroEncryption);
 

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

@@ -2,6 +2,7 @@
 
 #include <xrpl/basics/Log.h>
 #include <xrpl/basics/Slice.h>
+#include <xrpl/beast/utility/instrumentation.h>
 #include <xrpl/core/ServiceRegistry.h>
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/ledger/helpers/CredentialHelpers.h>
@@ -105,7 +106,14 @@ verifySendProofs(
 {
     // Sanity check
     if (!sleSenderMPToken || !sleDestinationMPToken || !sleIssuance)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::detail::verifySendProofs : caller must pre-validate sender/destination/"
+            "issuance existence");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     auto const hasAuditor = ctx.tx.isFieldPresent(sfAuditorEncryptedAmount);
 
@@ -204,7 +212,14 @@ ConfidentialMPTSend::preclaim(PreclaimContext const& ctx)
 
     // Sanity check: issuer isn't the sender
     if (sleIssuance->getAccountID(sfIssuer) == ctx.tx[sfAccount])
-        return tefINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTSend::preclaim : issuer derived from the MPT ID must match "
+            "the ledger's stored issuer");
+        return tefINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     // Check sender's MPToken existence
     auto const sleSenderMPToken = ctx.view.read(keylet::mptoken(mptIssuanceID, account));
@@ -238,7 +253,12 @@ ConfidentialMPTSend::preclaim(PreclaimContext const& ctx)
         (!sleSenderMPToken->isFieldPresent(sfAuditorEncryptedBalance) ||
          !sleDestinationMPToken->isFieldPresent(sfAuditorEncryptedBalance)))
     {
-        return tefINTERNAL;  // LCOV_EXCL_LINE
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTSend::preclaim : issuance-level auditing implies both "
+            "MPTokens already carry an auditor balance");
+        return tefINTERNAL;
+        // LCOV_EXCL_STOP
     }
 
     // Check lock
@@ -283,7 +303,14 @@ ConfidentialMPTSend::doApply()
     auto const sleDestAcct = view().read(keylet::account(destination));
 
     if (!sleSenderMPToken || !sleDestinationMPToken || !sleIssuance || !sleDestAcct)
-        return tecINTERNAL;  // LCOV_EXCL_LINE
+    {
+        // LCOV_EXCL_START
+        UNREACHABLE(
+            "xrpl::ConfidentialMPTSend::doApply : preclaim already validated these objects "
+            "exist");
+        return tecINTERNAL;
+        // LCOV_EXCL_STOP
+    }
 
     // Deposit preauth authorization was already verified in preclaim.
     // Remove any expired credentials.
@@ -353,7 +380,13 @@ ConfidentialMPTSend::doApply()
         auto rerandomizedDestEc = rerandomizeCiphertext(
             destEc, (*sleDestinationMPToken)[sfHolderEncryptionKey], sendChallenge);
         if (!rerandomizedDestEc)
-            return tecINTERNAL;  // LCOV_EXCL_LINE
+        {
+            // LCOV_EXCL_START
+            JLOG(ctx_.journal.error())
+                << "ConfidentialMPTSend failed to rerandomize destination inbox ciphertext.";
+            return tecINTERNAL;
+            // LCOV_EXCL_STOP
+        }
 
         auto const curInbox = (*sleDestinationMPToken)[sfConfidentialBalanceInbox];
         auto newInbox = homomorphicAdd(curInbox, *rerandomizedDestEc);
@@ -374,7 +407,13 @@ ConfidentialMPTSend::doApply()
         auto rerandomizedIssuerEc =
             rerandomizeCiphertext(issuerEc, (*sleIssuance)[sfIssuerEncryptionKey], sendChallenge);
         if (!rerandomizedIssuerEc)
-            return tecINTERNAL;  // LCOV_EXCL_LINE
+        {
+            // LCOV_EXCL_START
+            JLOG(ctx_.journal.error())
+                << "ConfidentialMPTSend failed to rerandomize destination issuer ciphertext.";
+            return tecINTERNAL;
+            // LCOV_EXCL_STOP
+        }
 
         auto const curIssuerEnc = (*sleDestinationMPToken)[sfIssuerEncryptedBalance];
         auto newIssuerEnc = homomorphicAdd(curIssuerEnc, *rerandomizedIssuerEc);
@@ -396,7 +435,13 @@ ConfidentialMPTSend::doApply()
         auto rerandomizedAuditorEc = rerandomizeCiphertext(
             *auditorEc, (*sleIssuance)[sfAuditorEncryptionKey], sendChallenge);
         if (!rerandomizedAuditorEc)
-            return tecINTERNAL;  // LCOV_EXCL_LINE
+        {
+            // LCOV_EXCL_START
+            JLOG(ctx_.journal.error())
+                << "ConfidentialMPTSend failed to rerandomize destination auditor ciphertext.";
+            return tecINTERNAL;
+            // LCOV_EXCL_STOP
+        }
 
         auto const curAuditorEnc = (*sleDestinationMPToken)[sfAuditorEncryptedBalance];
         auto newAuditorEnc = homomorphicAdd(curAuditorEnc, *rerandomizedAuditorEc);

src/test/app/AMMClawbackMPT_test.cpp

@@ -137,7 +137,6 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite
             AMM amm(env, gw, btc(100), usd(100));
             env.close();
             amm.deposit(alice, 1'000);
-            env.close();
 
             // can not clawback when tfMPTCanClawback is not enabled
             env(amm::ammClawback(gw, alice, btc, usd, std::nullopt), Ter(tecNO_PERMISSION));
@@ -503,6 +502,150 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite
         }
     }
 
+    void
+    testAMMClawbackAmountRoundsToZero(FeatureBitset features)
+    {
+        // Ensure a clawback that rounds down to zero MPT fails with
+        // tecAMM_FAILED instead of silently burning the holder's LP.
+        testcase("test AMMClawback amount that rounds down to zero");
+        using namespace jtx;
+
+        Env env(*this, features);
+        Account const gw{"gateway"};
+        Account const alice{"alice"};
+        Account const bob{"bob"};
+        env.fund(XRP(10'000'000), gw, alice, bob);
+        env.close();
+
+        env(fset(gw, asfAllowTrustLineClawback));
+        env.close();
+
+        // The clawed asset (amountRounded) rounds to zero while its XRP
+        // counterpart is always large.
+        {
+            MPTTester const mptBtc(
+                {.env = env,
+                 .issuer = gw,
+                 .holders = {alice, bob},
+                 .pay = 1'000,
+                 .flags = tfMPTCanClawback | kMptDexFlags});
+            MPT const btc = mptBtc;
+
+            AMM amm(env, alice, btc(3), XRP(333'000));
+            amm.deposit(bob, btc(3), XRP(333'000));
+
+            [[maybe_unused]] auto const [poolBtcBefore, poolXrpBefore, lptBefore] = amm.balances();
+            BEAST_EXPECT(poolBtcBefore == btc(6));
+
+            auto const issuerOABefore = mptBtc.getBalance(gw);
+            auto const aliceLpBefore = amm.getLPTokensBalance(alice.id());
+            auto const bobLpBefore = amm.getLPTokensBalance(bob.id());
+
+            // Attempt to clawback 1/6th of the BTC pool. When the zero-rounding
+            // guard is active (gated by fixCleanup3_4_0) the rounded amount
+            // drops to 0 and should trigger tecAMM_FAILED.
+            env(amm::ammClawback(gw, alice, btc, XRP, btc(1)),
+                Ter(features[fixCleanup3_4_0] ? TER{tecAMM_FAILED} : TER{tesSUCCESS}));
+            env.close();
+
+            [[maybe_unused]] auto const [poolBtcAfter, poolXrpAfter, lptAfter] = amm.balances();
+            auto const issuerOAAfter = mptBtc.getBalance(gw);
+            auto const aliceLpAfter = amm.getLPTokensBalance(alice.id());
+            auto const bobLpAfter = amm.getLPTokensBalance(bob.id());
+
+            if (features[fixCleanup3_4_0])
+            {
+                // Post-fixCleanup3_4_0: Clawback fails because the BTC balance
+                // would round to zero. All balances must remain untouched.
+                BEAST_EXPECT(poolBtcAfter == poolBtcBefore);
+                BEAST_EXPECT(poolXrpAfter == poolXrpBefore);
+                BEAST_EXPECT(issuerOAAfter == issuerOABefore);
+                BEAST_EXPECT(aliceLpAfter == aliceLpBefore);
+                BEAST_EXPECT(bobLpAfter == bobLpBefore);
+            }
+            else
+            {
+                // Pre-fixCleanup3_4_0: BTC rounds to zero and the clawback
+                // silently burns alice's LP without clawing back any BTC.
+                BEAST_EXPECT(poolBtcAfter == poolBtcBefore);
+                BEAST_EXPECT(poolXrpAfter < poolXrpBefore);
+                BEAST_EXPECT(issuerOAAfter == issuerOABefore);
+                BEAST_EXPECT(aliceLpAfter < aliceLpBefore);
+                BEAST_EXPECT(bobLpAfter == bobLpBefore);
+            }
+        }
+
+        // The pool above only ever rounds the clawed asset (amountRounded) to
+        // zero; its XRP counterpart is always large. Exercise the other operand
+        // of the guard (amount2Rounded == 0) with an MPT/MPT pool where the
+        // *paired* asset is the tiny integer that floors to zero while the
+        // clawed asset still rounds non-zero.
+        {
+            Account const carol{"carol"};
+            Account const dan{"dan"};
+            env.fund(XRP(10'000'000), carol, dan);
+            env.close();
+
+            MPTTester const mptBtc(
+                {.env = env,
+                 .issuer = gw,
+                 .holders = {carol, dan},
+                 .pay = 100'000,
+                 .flags = tfMPTCanClawback | kMptDexFlags});
+            MPT const btc = mptBtc;
+
+            MPTTester const mptEth(
+                {.env = env,
+                 .issuer = gw,
+                 .holders = {carol, dan},
+                 .pay = 1'000,
+                 .flags = tfMPTCanClawback | kMptDexFlags});
+            MPT const eth = mptEth;
+
+            // btc pool dwarfs the eth pool, so a ~1/12th claw withdraws a
+            // non-zero btc amount while the eth counterpart rounds to zero.
+            AMM amm(env, carol, btc(3'000), eth(3));
+            amm.deposit(dan, btc(3'000), eth(3));
+
+            [[maybe_unused]] auto const [poolBtcBefore, poolEthBefore, lptBefore] = amm.balances();
+            BEAST_EXPECT(poolBtcBefore == btc(6'000));
+            BEAST_EXPECT(poolEthBefore == eth(6));
+
+            auto const carolLpBefore = amm.getLPTokensBalance(carol.id());
+            auto const danLpBefore = amm.getLPTokensBalance(dan.id());
+
+            env(amm::ammClawback(gw, carol, btc, eth, btc(500)),
+                Ter(features[fixCleanup3_4_0] ? TER{tecAMM_FAILED} : TER{tesSUCCESS}));
+            env.close();
+
+            [[maybe_unused]] auto const [poolBtcAfter, poolEthAfter, lptAfter] = amm.balances();
+            auto const carolLpAfter = amm.getLPTokensBalance(carol.id());
+            auto const danLpAfter = amm.getLPTokensBalance(dan.id());
+
+            if (features[fixCleanup3_4_0])
+            {
+                // Post-fixCleanup3_4_0: clawback fails because the ETH (Asset2)
+                // balance would round to zero (guard fires via
+                // amount2Rounded == 0). All balances must remain untouched.
+                BEAST_EXPECT(poolBtcAfter == poolBtcBefore);
+                BEAST_EXPECT(poolEthAfter == poolEthBefore);
+                BEAST_EXPECT(carolLpAfter == carolLpBefore);
+                BEAST_EXPECT(danLpAfter == danLpBefore);
+            }
+            else
+            {
+                // Pre-fixCleanup3_4_0: the asymmetric round-off goes through.
+                // btc is clawed (non-zero) but eth rounds to zero, so the eth
+                // pool is untouched while carol's LP is burned. This asymmetry
+                // proves amount2Rounded == 0 is the trigger.
+                BEAST_EXPECT(poolBtcAfter < poolBtcBefore);
+                BEAST_EXPECT(poolEthAfter == poolEthBefore);
+                BEAST_EXPECT(carolLpAfter < carolLpBefore);
+                BEAST_EXPECT(danLpAfter == danLpBefore);
+            }
+        }
+    }
+
     void
     testAMMClawbackAll(FeatureBitset features)
     {
@@ -543,7 +686,6 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite
 
             // gw clawback all BTC from alice
             amm.deposit(bob, btc(1'000'000000), usd(2000));
-            env.close();
             BEAST_EXPECT(amm.expectBalances(btc(3'000'000000), usd(3000), IOUAmount(3000000)));
 
             auto aliceBTC = env.balance(alice, btc);
@@ -921,7 +1063,6 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite
             BEAST_EXPECT(amm.expectBalances(btc(2'000'000000), usd(8'000), IOUAmount(4'000'000)));
 
             amm.deposit(bob, btc(1'000'000000), usd(4'000));
-            env.close();
             BEAST_EXPECT(amm.expectBalances(btc(3'000'000000), usd(12'000), IOUAmount(6'000'000)));
 
             auto aliceBTC = env.balance(alice, btc);
@@ -1361,7 +1502,6 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite
             env.close();
             BEAST_EXPECT(amm.expectBalances(XRP(100), btc(400), IOUAmount(200000)));
             amm.deposit(alice, btc(400));
-            env.close();
             BEAST_EXPECT(amm.expectBalances(XRP(100), btc(800), IOUAmount{282842'712474619, -9}));
 
             auto aliceBTC = env.balance(alice, MPT(btc));
@@ -1407,7 +1547,6 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite
             env.close();
             BEAST_EXPECT(amm.expectBalances(usd(100), btc(400), IOUAmount(200)));
             amm.deposit(alice, btc(400));
-            env.close();
             BEAST_EXPECT(amm.expectBalances(usd(100), btc(800), IOUAmount{282'842712474619, -12}));
 
             auto aliceBTC = env.balance(alice, MPT(btc));
@@ -1462,7 +1601,6 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite
             env.close();
             BEAST_EXPECT(amm.expectBalances(usd(100), btc(400), IOUAmount(200)));
             amm.deposit(alice, btc(400));
-            env.close();
             BEAST_EXPECT(amm.expectBalances(usd(100), btc(800), IOUAmount{282'842712474619, -12}));
 
             auto aliceBTC = env.balance(alice, MPT(btc));
@@ -1669,7 +1807,7 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite
             env(amm::ammClawback(gw, alice, btc, usd, std::nullopt), Ter(tecNO_PERMISSION));
 
             // Although USD is clawable with asfAllowTrustLineClawback.
-            // When tfClawTwoAssets is set, we will claw Asser2 as well.
+            // When tfClawTwoAssets is set, we will claw Asset2 as well.
             // But Asset2 is not clawable. tfMPTCanClawback was not set for BTC.
             env(amm::ammClawback(gw, alice, usd, btc, std::nullopt),
                 Txflags(tfClawTwoAssets),
@@ -1819,6 +1957,9 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite
         testInvalidRequest(all);
         testFeatureDisabled(all);
         testAMMClawbackAmount(all);
+        testAMMClawbackAmount(all - fixCleanup3_4_0);
+        testAMMClawbackAmountRoundsToZero(all);
+        testAMMClawbackAmountRoundsToZero(all - fixCleanup3_4_0);
         testAMMClawbackAll(all);
         testAMMClawbackAmountSameIssuer(all);
         testAMMClawbackAllSameIssuer(all);

src/test/app/AMMClawback_test.cpp

@@ -2155,6 +2155,209 @@ class AMMClawback_test : public beast::unit_test::Suite
             }
             BEAST_EXPECT(env.balance(carol, eur) == eur(7750));
         }
+
+        // gw (USD issuer) individually freezes the AMM-USD trust line.
+        // AMMClawback must still succeed because the freeze invariant
+        // short-circuits before reaching the AMM line check (no receivers in
+        // the USD issuer's change set). Behavior is identical with or without
+        // fixCleanup3_4_0.
+        {
+            Env env(*this, features);
+            Account const gw{"gateway"};
+            Account const gw2{"gateway2"};
+            Account const alice{"alice"};
+            env.fund(XRP(1000000), gw, gw2, alice);
+            env.close();
+
+            env(fset(gw, asfAllowTrustLineClawback));
+            env.close();
+            env.require(Flags(gw, asfAllowTrustLineClawback));
+
+            auto const usd = gw["USD"];
+            env.trust(usd(100000), alice);
+            env(pay(gw, alice, usd(3000)));
+            env.close();
+
+            auto const eur = gw2["EUR"];
+            env.trust(eur(100000), alice);
+            env(pay(gw2, alice, eur(3000)));
+            env.close();
+
+            AMM const amm(env, alice, eur(1000), usd(2000), Ter(tesSUCCESS));
+            env.close();
+
+            BEAST_EXPECT(
+                amm.expectBalances(usd(2000), eur(1000), IOUAmount{1414213562373095, -12}));
+
+            // gw individually freezes the AMM-USD trust line (AMM pseudo-account
+            // <-> gw), not alice's trust line.
+            env(trust(gw, STAmount{Issue{usd.currency, amm.ammAccount()}, 0}, tfSetFreeze));
+            env.close();
+
+            env(amm::ammClawback(gw, alice, usd, eur, usd(1000)), Ter(tesSUCCESS));
+            env.close();
+
+            env.require(Balance(alice, usd(1000)));
+            env.require(Balance(alice, eur(2500)));
+            BEAST_EXPECT(amm.expectBalances(usd(1000), eur(500), IOUAmount{7071067811865475, -13}));
+            BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount{7071067811865475, -13}));
+        }
+
+        // gw2 (EUR issuer) individually freezes the AMM-EUR trust line.
+        // The EUR flow (AMM → alice) is a genuine P2P transfer checked by the
+        // freeze invariant. Pre-fixCleanup3_4_0 the isAMMNode guard incorrectly
+        // blocked AMMClawback's overrideFreeze privilege on that trust line.
+        {
+            Env env(*this, features);
+            Account const gw{"gateway"};
+            Account const gw2{"gateway2"};
+            Account const alice{"alice"};
+            env.fund(XRP(1000000), gw, gw2, alice);
+            env.close();
+
+            env(fset(gw, asfAllowTrustLineClawback));
+            env.close();
+            env.require(Flags(gw, asfAllowTrustLineClawback));
+
+            auto const usd = gw["USD"];
+            env.trust(usd(100000), alice);
+            env(pay(gw, alice, usd(3000)));
+            env.close();
+
+            auto const eur = gw2["EUR"];
+            env.trust(eur(100000), alice);
+            env(pay(gw2, alice, eur(3000)));
+            env.close();
+
+            AMM const amm(env, alice, eur(1000), usd(2000), Ter(tesSUCCESS));
+            env.close();
+
+            BEAST_EXPECT(
+                amm.expectBalances(usd(2000), eur(1000), IOUAmount{1414213562373095, -12}));
+
+            // gw2 individually freezes the AMM-EUR trust line.
+            env(trust(gw2, STAmount{Issue{eur.currency, amm.ammAccount()}, 0}, tfSetFreeze));
+            env.close();
+
+            if (features[fixCleanup3_4_0])
+            {
+                // Post-fixCleanup3_4_0: overrideFreeze privilege applies to
+                // all freeze types on AMM trust lines.
+                env(amm::ammClawback(gw, alice, usd, eur, usd(1000)), Ter(tesSUCCESS));
+                env.close();
+
+                env.require(Balance(alice, usd(1000)));
+                env.require(Balance(alice, eur(2500)));
+                BEAST_EXPECT(
+                    amm.expectBalances(usd(1000), eur(500), IOUAmount{7071067811865475, -13}));
+                BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount{7071067811865475, -13}));
+            }
+            else
+            {
+                // Pre-fixCleanup3_4_0: the isAMMNode guard prevents the
+                // overrideFreeze privilege from applying to individually-frozen
+                // AMM trust lines, so the invariant blocks the clawback.
+                env(amm::ammClawback(gw, alice, usd, eur, usd(1000)), Ter(tecINVARIANT_FAILED));
+            }
+        }
+
+        // gw2 (EUR issuer) globally freezes its issued assets. AMMClawback
+        // must still be able to return EUR from the AMM to alice.
+        {
+            Env env(*this, features);
+            Account const gw{"gateway"};
+            Account const gw2{"gateway2"};
+            Account const alice{"alice"};
+            env.fund(XRP(1000000), gw, gw2, alice);
+            env.close();
+
+            env(fset(gw, asfAllowTrustLineClawback));
+            env.close();
+            env.require(Flags(gw, asfAllowTrustLineClawback));
+
+            auto const usd = gw["USD"];
+            env.trust(usd(100000), alice);
+            env(pay(gw, alice, usd(3000)));
+            env.close();
+
+            auto const eur = gw2["EUR"];
+            env.trust(eur(100000), alice);
+            env(pay(gw2, alice, eur(3000)));
+            env.close();
+
+            AMM const amm(env, alice, eur(1000), usd(2000), Ter(tesSUCCESS));
+            env.close();
+
+            BEAST_EXPECT(
+                amm.expectBalances(usd(2000), eur(1000), IOUAmount{1414213562373095, -12}));
+
+            env(fset(gw2, asfGlobalFreeze));
+            env.close();
+
+            env(amm::ammClawback(gw, alice, usd, eur, usd(1000)), Ter(tesSUCCESS));
+            env.close();
+
+            env.require(Balance(alice, usd(1000)));
+            env.require(Balance(alice, eur(2500)));
+            BEAST_EXPECT(amm.expectBalances(usd(1000), eur(500), IOUAmount{7071067811865475, -13}));
+            BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount{7071067811865475, -13}));
+        }
+
+        // Same as above but gw2 deep-freezes the AMM-EUR trust line.
+        if (features[featureDeepFreeze])
+        {
+            Env env(*this, features);
+            Account const gw{"gateway"};
+            Account const gw2{"gateway2"};
+            Account const alice{"alice"};
+            env.fund(XRP(1000000), gw, gw2, alice);
+            env.close();
+
+            env(fset(gw, asfAllowTrustLineClawback));
+            env.close();
+            env.require(Flags(gw, asfAllowTrustLineClawback));
+
+            auto const usd = gw["USD"];
+            env.trust(usd(100000), alice);
+            env(pay(gw, alice, usd(3000)));
+            env.close();
+
+            auto const eur = gw2["EUR"];
+            env.trust(eur(100000), alice);
+            env(pay(gw2, alice, eur(3000)));
+            env.close();
+
+            AMM const amm(env, alice, eur(1000), usd(2000), Ter(tesSUCCESS));
+            env.close();
+
+            BEAST_EXPECT(
+                amm.expectBalances(usd(2000), eur(1000), IOUAmount{1414213562373095, -12}));
+
+            // gw2 deep-freezes the AMM-EUR trust line.
+            env(trust(
+                gw2,
+                STAmount{Issue{eur.currency, amm.ammAccount()}, 0},
+                tfSetFreeze | tfSetDeepFreeze));
+            env.close();
+
+            if (features[fixCleanup3_4_0])
+            {
+                env(amm::ammClawback(gw, alice, usd, eur, usd(1000)), Ter(tesSUCCESS));
+                env.close();
+
+                env.require(Balance(alice, usd(1000)));
+                env.require(Balance(alice, eur(2500)));
+                BEAST_EXPECT(
+                    amm.expectBalances(usd(1000), eur(500), IOUAmount{7071067811865475, -13}));
+                BEAST_EXPECT(amm.expectLPTokens(alice, IOUAmount{7071067811865475, -13}));
+            }
+            else
+            {
+                // Pre-fixCleanup3_4_0: same isAMMNode guard issue blocks the
+                // clawback on deep-frozen AMM trust lines.
+                env(amm::ammClawback(gw, alice, usd, eur, usd(1000)), Ter(tecINVARIANT_FAILED));
+            }
+        }
     }
 
     void
@@ -2530,6 +2733,7 @@ class AMMClawback_test : public beast::unit_test::Suite
               // precision loss caught in transaction layer -> tecPRECISION_LOSS
               all - fixAMMClawbackRounding - featureMPTokensV2,
               all - featureMPTokensV2,
+              all - fixCleanup3_4_0,
               all})
         {
             testAMMClawbackSpecificAmount(features);

src/test/app/NFToken_test.cpp

@@ -4790,6 +4790,87 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite
         checkOffers("nft_buy_offers", 501, 2, __LINE__);
     }
 
+    void
+    testNftXxxOffersMarkerWrongSide(FeatureBitset features)
+    {
+        // A pagination marker passed to nft_buy_offers / nft_sell_offers must
+        // reference an offer on the same side (buy vs. sell) as the directory
+        // being enumerated.  A wrong-side marker is rejected with invalidParams.
+        //
+        // Note: the pre-fix code also returned invalidParams for a wrong-side
+        // marker, but only after scanning the entire target directory (an
+        // O(directory size) walk usable to burn CPU).  The fix short-circuits
+        // that scan.  The scan-avoidance is not observable from the RPC
+        // response, so this test locks the rejection contract (wrong-side ->
+        // error, same-side -> success) rather than the performance property.
+        testcase("nft_buy_offers and nft_sell_offers wrong-side marker");
+
+        using namespace test::jtx;
+
+        Env env{*this, features};
+
+        Account const issuer{"issuer"};
+        Account const buyer{"buyer"};
+
+        env.fund(XRP(10000), issuer, buyer);
+        env.close();
+
+        // Mint a transferable NFT.
+        uint256 const nftID{token::getNextID(env, issuer, 0u, tfTransferable)};
+        env(token::mint(issuer, 0), Txflags(tfTransferable));
+        env.close();
+
+        // Create one sell offer (from the issuer, who owns the NFT) and one
+        // buy offer (from the buyer) for the same NFT.
+        env(token::createOffer(issuer, nftID, XRP(100)), Txflags(tfSellNFToken));
+        env(token::createOffer(buyer, nftID, XRP(50)), token::Owner(issuer));
+        env.close();
+
+        // Grab the index of the single offer on each side from the RPC
+        // response so we can use it as a marker.
+        auto firstOfferIndex = [this, &env, &nftID](char const* request) {
+            json::Value params;
+            params[jss::nft_id] = to_string(nftID);
+            json::Value const result = env.rpc("json", request, to_string(params))[jss::result];
+            BEAST_EXPECT(result.isMember(jss::offers) && result[jss::offers].size() == 1);
+            return result[jss::offers][0u][jss::nft_offer_index].asString();
+        };
+
+        std::string const sellOfferIndex = firstOfferIndex("nft_sell_offers");
+        std::string const buyOfferIndex = firstOfferIndex("nft_buy_offers");
+
+        auto queryWithMarker = [&env, &nftID](char const* request, std::string const& marker) {
+            json::Value params;
+            params[jss::nft_id] = to_string(nftID);
+            params[jss::marker] = marker;
+            return env.rpc("json", request, to_string(params))[jss::result];
+        };
+
+        // A marker referencing an offer on the wrong side is rejected with
+        // invalidParams.
+        {
+            // Sell-side marker passed to nft_buy_offers.
+            json::Value const result = queryWithMarker("nft_buy_offers", sellOfferIndex);
+            BEAST_EXPECT(result[jss::error].asString() == "invalidParams");
+        }
+        {
+            // Buy-side marker passed to nft_sell_offers.
+            json::Value const result = queryWithMarker("nft_sell_offers", buyOfferIndex);
+            BEAST_EXPECT(result[jss::error].asString() == "invalidParams");
+        }
+
+        // A same-side marker is still accepted.  With a single offer on each
+        // side, resuming after it simply yields no further offers.
+        {
+            json::Value const result = queryWithMarker("nft_buy_offers", buyOfferIndex);
+            BEAST_EXPECT(!result.isMember(jss::error));
+        }
+        {
+            json::Value const result = queryWithMarker("nft_sell_offers", sellOfferIndex);
+            BEAST_EXPECT(!result.isMember(jss::error));
+        }
+    }
+
     void
     testNFTokenNegOffer(FeatureBitset features)
     {
@@ -7305,6 +7386,7 @@ protected:
         testNFTokenWithTickets(features);
         testNFTokenDeleteAccount(features);
         testNftXxxOffers(features);
+        testNftXxxOffersMarkerWrongSide(features);
         testNFTokenNegOffer(features);
         testIOUWithTransferFee(features);
         testBrokeredSaleToSelf(features);

src/test/rpc/AccountLines_test.cpp

@@ -94,6 +94,24 @@ public:
         LedgerHeader const ledger3Info = env.closed()->header();
         BEAST_EXPECT(ledger3Info.seq == 3);
 
+        {
+            // test peer non-string
+            auto testInvalidPeerParam = [&](auto const& param) {
+                json::Value params;
+                params[jss::account] = alice.human();
+                params[jss::peer] = param;
+                auto jrr = env.rpc("json", "account_lines", to_string(params))[jss::result];
+                BEAST_EXPECT(jrr[jss::error] == "invalidParams");
+                BEAST_EXPECT(jrr[jss::error_message] == "Invalid field 'peer'.");
+            };
+
+            testInvalidPeerParam(1);
+            testInvalidPeerParam(1.1);
+            testInvalidPeerParam(true);
+            testInvalidPeerParam(json::Value(json::ValueType::Null));
+            testInvalidPeerParam(json::Value(json::ValueType::Object));
+            testInvalidPeerParam(json::Value(json::ValueType::Array));
+        }
         {
             // alice is funded but has no lines.  An empty array is returned.
             json::Value params;
@@ -775,6 +793,35 @@ public:
         LedgerHeader const ledger3Info = env.closed()->header();
         BEAST_EXPECT(ledger3Info.seq == 3);
 
+        {
+            // test peer non-string
+            auto testInvalidPeerParam = [&](auto const& param) {
+                json::Value params;
+                params[jss::account] = alice.human();
+                params[jss::peer] = param;
+
+                json::Value request;
+                request[jss::method] = "account_lines";
+                request[jss::jsonrpc] = "2.0";
+                request[jss::ripplerpc] = "2.0";
+                request[jss::id] = 5;
+                request[jss::params] = params;
+
+                auto const lines = env.rpc("json2", to_string(request));
+                BEAST_EXPECT(lines[jss::error][jss::error] == "invalidParams");
+                BEAST_EXPECT(lines[jss::error][jss::message] == "Invalid field 'peer'.");
+                BEAST_EXPECT(lines.isMember(jss::jsonrpc) && lines[jss::jsonrpc] == "2.0");
+                BEAST_EXPECT(lines.isMember(jss::ripplerpc) && lines[jss::ripplerpc] == "2.0");
+                BEAST_EXPECT(lines.isMember(jss::id) && lines[jss::id] == 5);
+            };
+
+            testInvalidPeerParam(1);
+            testInvalidPeerParam(1.1);
+            testInvalidPeerParam(true);
+            testInvalidPeerParam(json::Value(json::ValueType::Null));
+            testInvalidPeerParam(json::Value(json::ValueType::Object));
+            testInvalidPeerParam(json::Value(json::ValueType::Array));
+        }
         {
             // alice is funded but has no lines.  An empty array is returned.
             json::Value params;

src/test/rpc/GatewayBalances_test.cpp

@@ -176,6 +176,45 @@ public:
         });
     }
 
+    void
+    testGWBInvalidAccount(FeatureBitset features)
+    {
+        testcase("Gateway Balances with non-string account/ident");
+        using namespace std::chrono_literals;
+        using namespace jtx;
+        Env env(*this, features);
+
+        Account const alice{"alice"};
+        env.fund(XRP(10000), alice);
+        env.close();
+
+        auto wsc = makeWSClient(env.app().config());
+
+        // A non-string "account" must be rejected cleanly with invalidParams
+        // rather than throwing a Json::LogicError that surfaces as internal.
+        json::Value qry;
+        qry[jss::account] = 42;
+        qry[jss::hotwallet] = alice.human();
+
+        forAllApiVersions([&, this](unsigned apiVersion) {
+            qry[jss::api_version] = apiVersion;
+            auto jv = wsc->invoke("gateway_balances", qry);
+            expect(jv[jss::status] == "error");
+            BEAST_EXPECT(jv[jss::result][jss::error] == "invalidParams");
+        });
+
+        // The same applies to a non-string "ident".
+        json::Value qry2;
+        qry2[jss::ident] = 42;
+
+        forAllApiVersions([&, this](unsigned apiVersion) {
+            qry2[jss::api_version] = apiVersion;
+            auto jv = wsc->invoke("gateway_balances", qry2);
+            expect(jv[jss::status] == "error");
+            BEAST_EXPECT(jv[jss::result][jss::error] == "invalidParams");
+        });
+    }
+
     void
     testGWBOverflow()
     {
@@ -280,6 +319,7 @@ public:
         {
             testGWB(feature);
             testGWBApiVersions(feature);
+            testGWBInvalidAccount(feature);
         }
         testGWBWithMPT();
         testGWBOverflow();

src/tests/libxrpl/basics/Buffer.cpp

@@ -4,6 +4,7 @@
 
 #include <gtest/gtest.h>
 
+#include <array>
 #include <cstddef>
 #include <cstdint>
 #include <cstring>
@@ -12,8 +13,18 @@
 
 namespace xrpl::test {
 
+static_assert(std::is_nothrow_move_constructible_v<Buffer>);
+static_assert(std::is_nothrow_move_assignable_v<Buffer>);
+
 struct BufferTest : public ::testing::Test
 {
+    static constexpr auto kRandomData = std::to_array<std::uint8_t>(
+        {0xa8, 0xa1, 0x38, 0x45, 0x23, 0xec, 0xe4, 0x23, 0x71, 0x6d, 0x2a,
+         0x18, 0xb4, 0x70, 0xcb, 0xf5, 0xac, 0x2d, 0x89, 0x4d, 0x19, 0x9c,
+         0xf0, 0x2c, 0x15, 0xd1, 0xf9, 0x9b, 0x66, 0xd2, 0x30, 0xd3});
+
+    static constexpr std::size_t kHalf = kRandomData.size() / 2;
+
     static bool
     sane(Buffer const& b)
     {
@@ -22,239 +33,321 @@ struct BufferTest : public ::testing::Test
 
         return b.data() != nullptr;
     }
+
+    /**
+     * Check the state Buffer documents for a moved-from buffer: "the other buffer is reset", i.e.
+     * empty and sane.
+     *
+     * Zeroing the size is not incidental tidiness. Moving the member unique_ptr nulls the data
+     * pointer whether Buffer wants it or not, so a moved-from buffer that kept its old size would
+     * lie about itself everywhere: alloc() would take its `n == size_` early-out and hand back a
+     * null pointer while still reporting the old size, fill() would run std::fill_n over a null
+     * pointer, and the Slice conversion would publish {nullptr, oldSize} to callers. A moved-from
+     * Buffer has to be a usable empty Buffer rather than a landmine, which is why the tests below
+     * assert this state instead of treating a moved-from buffer as untouchable.
+     */
+    static void
+    checkEmptyAfterMove(Buffer const& buf)
+    {
+        EXPECT_TRUE(sane(buf));
+        EXPECT_TRUE(buf.empty());
+    }
+
+    Buffer const emptyBuffer;
+    Buffer const firstHalf{kRandomData.data(), kHalf};
+    Buffer const secondHalf{kRandomData.data() + kHalf, kHalf};
+    Buffer const whole{kRandomData.data(), kRandomData.size()};
 };
 
-TEST_F(BufferTest, buffer)
+TEST_F(BufferTest, default_constructed_is_empty)
 {
-    std::uint8_t const data[] = {0xa8, 0xa1, 0x38, 0x45, 0x23, 0xec, 0xe4, 0x23, 0x71, 0x6d, 0x2a,
-                                 0x18, 0xb4, 0x70, 0xcb, 0xf5, 0xac, 0x2d, 0x89, 0x4d, 0x19, 0x9c,
-                                 0xf0, 0x2c, 0x15, 0xd1, 0xf9, 0x9b, 0x66, 0xd2, 0x30, 0xd3};
+    Buffer const b;
 
-    Buffer const b0;
-    EXPECT_TRUE(sane(b0));
-    EXPECT_TRUE(b0.empty());
+    EXPECT_TRUE(sane(b));
+    EXPECT_TRUE(b.empty());
+    EXPECT_EQ(b.data(), nullptr);
+}
 
-    Buffer b1{0};
-    EXPECT_TRUE(sane(b1));
-    EXPECT_TRUE(b1.empty());
-    std::memcpy(b1.alloc(16), data, 16);
-    EXPECT_TRUE(sane(b1));
-    EXPECT_FALSE(b1.empty());
-    EXPECT_EQ(b1.size(), 16);
+TEST_F(BufferTest, zero_sized_construction_is_empty)
+{
+    Buffer const b{0};
 
-    Buffer b2{b1.size()};
-    EXPECT_TRUE(sane(b2));
-    EXPECT_FALSE(b2.empty());
-    EXPECT_EQ(b2.size(), b1.size());
-    std::memcpy(b2.data(), data + 16, 16);
+    EXPECT_TRUE(sane(b));
+    EXPECT_TRUE(b.empty());
+}
 
-    Buffer b3{data, sizeof(data)};
-    EXPECT_TRUE(sane(b3));
-    EXPECT_FALSE(b3.empty());
-    EXPECT_EQ(b3.size(), sizeof(data));
-    EXPECT_EQ(std::memcmp(b3.data(), data, b3.size()), 0);
+TEST_F(BufferTest, alloc_grows_an_empty_buffer)
+{
+    Buffer b{0};
+    std::memcpy(b.alloc(kHalf), kRandomData.data(), kHalf);
 
-    // Check equality and inequality comparisons.
-    // For code readability, we want to use general
-    // EXPECT_TRUE instead of specific EXPECT_EQ etc.
-    EXPECT_TRUE(b0 == b0);
-    EXPECT_TRUE(b0 != b1);
-    EXPECT_TRUE(b1 == b1);
-    EXPECT_TRUE(b1 != b2);
-    EXPECT_TRUE(b2 != b3);
+    EXPECT_TRUE(sane(b));
+    EXPECT_FALSE(b.empty());
+    EXPECT_EQ(b.size(), kHalf);
+    EXPECT_EQ(b, firstHalf);
+}
 
-    // Check copy constructors and copy assignments:
-    {
-        Buffer x{b0};
-        EXPECT_EQ(x, b0);
-        EXPECT_TRUE(sane(x));
-        Buffer y{b1};
-        EXPECT_EQ(y, b1);
-        EXPECT_TRUE(sane(y));
-        x = b2;
-        EXPECT_EQ(x, b2);
-        EXPECT_TRUE(sane(x));
-        x = y;
-        EXPECT_EQ(x, y);
-        EXPECT_TRUE(sane(x));
-        y = b3;
-        EXPECT_EQ(y, b3);
-        EXPECT_TRUE(sane(y));
-        x = b0;
-        EXPECT_EQ(x, b0);
-        EXPECT_TRUE(sane(x));
+TEST_F(BufferTest, sized_construction_reserves_without_filling)
+{
+    Buffer b{kHalf};
+
+    EXPECT_TRUE(sane(b));
+    EXPECT_FALSE(b.empty());
+    EXPECT_EQ(b.size(), kHalf);
+
+    std::memcpy(b.data(), kRandomData.data() + kHalf, kHalf);
+    EXPECT_EQ(b, secondHalf);
+}
+
+TEST_F(BufferTest, construction_copies_raw_memory)
+{
+    Buffer const b{kRandomData.data(), kRandomData.size()};
+
+    EXPECT_TRUE(sane(b));
+    EXPECT_FALSE(b.empty());
+    EXPECT_EQ(b.size(), kRandomData.size());
+    EXPECT_EQ(std::memcmp(b.data(), kRandomData.data(), b.size()), 0);
+}
+
+TEST_F(BufferTest, equality_compares_contents)
+{
+    // Uses EXPECT_TRUE rather than EXPECT_EQ/EXPECT_NE because the operators are what is under test
+    // here.
+    EXPECT_TRUE(emptyBuffer == emptyBuffer);
+    EXPECT_TRUE(firstHalf == firstHalf);
+
+    EXPECT_TRUE(emptyBuffer != firstHalf);
+    EXPECT_TRUE(firstHalf != secondHalf);
+    EXPECT_TRUE(secondHalf != whole);
+}
+
+TEST_F(BufferTest, copy_construction)
+{
+    Buffer const fromEmpty{emptyBuffer};
+    EXPECT_TRUE(sane(fromEmpty));
+    EXPECT_EQ(fromEmpty, emptyBuffer);
+
+    Buffer const fromNonEmpty{firstHalf};
+    EXPECT_TRUE(sane(fromNonEmpty));
+    EXPECT_EQ(fromNonEmpty, firstHalf);
+}
+
+TEST_F(BufferTest, copy_assignment)
+{
+    Buffer b{emptyBuffer};
+
+    // empty <- non-empty
+    b = secondHalf;
+    EXPECT_TRUE(sane(b));
+    EXPECT_EQ(b, secondHalf);
+
+    // non-empty <- non-empty of a different size
+    b = whole;
+    EXPECT_TRUE(sane(b));
+    EXPECT_EQ(b, whole);
+
+    // non-empty <- empty
+    b = emptyBuffer;
+    EXPECT_TRUE(sane(b));
+    EXPECT_EQ(b, emptyBuffer);
+}
+
+TEST_F(BufferTest, self_assignment_preserves_contents)
+{
 #ifdef __clang__
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wself-assign-overloaded"
 #endif
 
-        x = x;
-        EXPECT_EQ(x, b0);
-        EXPECT_TRUE(sane(x));
-        y = y;
-        EXPECT_EQ(y, b3);
-        EXPECT_TRUE(sane(y));
+    Buffer emptyCopy{emptyBuffer};
+    emptyCopy = emptyCopy;
+    EXPECT_TRUE(sane(emptyCopy));
+    EXPECT_EQ(emptyCopy, emptyBuffer);
+
+    Buffer wholeCopy{whole};
+    wholeCopy = wholeCopy;
+    EXPECT_TRUE(sane(wholeCopy));
+    EXPECT_EQ(wholeCopy, whole);
 
 #ifdef __clang__
 #pragma clang diagnostic pop
 #endif
-    }
+}
 
-    // Check move constructor & move assignments:
+TEST_F(BufferTest, move_construct_from_empty)
+{
+    Buffer source;
+    Buffer const moved{std::move(source)};
+
+    checkEmptyAfterMove(source);  // NOLINT(bugprone-use-after-move)
+    EXPECT_TRUE(sane(moved));
+    EXPECT_TRUE(moved.empty());
+}
+
+TEST_F(BufferTest, move_construct_from_non_empty)
+{
+    Buffer source{firstHalf};
+    Buffer const moved{std::move(source)};
+
+    checkEmptyAfterMove(source);  // NOLINT(bugprone-use-after-move)
+    EXPECT_TRUE(sane(moved));
+    EXPECT_EQ(moved, firstHalf);
+}
+
+TEST_F(BufferTest, move_assign_empty_to_empty)
+{
+    Buffer target;
+    Buffer source;
+
+    target = std::move(source);
+
+    EXPECT_TRUE(sane(target));
+    EXPECT_TRUE(target.empty());
+    checkEmptyAfterMove(source);  // NOLINT(bugprone-use-after-move)
+}
+
+TEST_F(BufferTest, move_assign_non_empty_to_empty)
+{
+    Buffer target;
+    Buffer source{firstHalf};
+
+    target = std::move(source);
+
+    EXPECT_TRUE(sane(target));
+    EXPECT_EQ(target, firstHalf);
+    checkEmptyAfterMove(source);  // NOLINT(bugprone-use-after-move)
+}
+
+TEST_F(BufferTest, move_assign_empty_to_non_empty)
+{
+    Buffer target{firstHalf};
+    Buffer source;
+
+    target = std::move(source);
+
+    EXPECT_TRUE(sane(target));
+    EXPECT_TRUE(target.empty());
+    checkEmptyAfterMove(source);  // NOLINT(bugprone-use-after-move)
+}
+
+TEST_F(BufferTest, move_assign_non_empty_to_non_empty)
+{
+    Buffer target{firstHalf};
+    Buffer sameSize{secondHalf};
+    Buffer largerSize{whole};
+
+    target = std::move(sameSize);
+    EXPECT_TRUE(sane(target));
+    EXPECT_EQ(target, secondHalf);
+    checkEmptyAfterMove(sameSize);  // NOLINT(bugprone-use-after-move)
+
+    target = std::move(largerSize);
+    EXPECT_TRUE(sane(target));
+    EXPECT_EQ(target, whole);
+    checkEmptyAfterMove(largerSize);  // NOLINT(bugprone-use-after-move)
+}
+
+TEST_F(BufferTest, construction_from_slice)
+{
+    Buffer const fromEmpty{static_cast<Slice>(emptyBuffer)};
+    EXPECT_TRUE(sane(fromEmpty));
+    EXPECT_EQ(fromEmpty, emptyBuffer);
+
+    Buffer const fromNonEmpty{static_cast<Slice>(whole)};
+    EXPECT_TRUE(sane(fromNonEmpty));
+    EXPECT_EQ(fromNonEmpty, whole);
+}
+
+TEST_F(BufferTest, assignment_from_slice)
+{
+    Buffer b;
+
+    // empty <- empty slice
+    b = static_cast<Slice>(emptyBuffer);
+    EXPECT_TRUE(sane(b));
+    EXPECT_EQ(b, emptyBuffer);
+
+    // empty <- non-empty slice
+    b = static_cast<Slice>(firstHalf);
+    EXPECT_TRUE(sane(b));
+    EXPECT_EQ(b, firstHalf);
+
+    // non-empty <- non-empty slice
+    b = static_cast<Slice>(secondHalf);
+    EXPECT_TRUE(sane(b));
+    EXPECT_EQ(b, secondHalf);
+
+    // non-empty <- empty slice
+    b = static_cast<Slice>(emptyBuffer);
+    EXPECT_TRUE(sane(b));
+    EXPECT_EQ(b, emptyBuffer);
+}
+
+TEST_F(BufferTest, resize_allocates_and_clear_releases)
+{
+    auto check = [](Buffer const& original, std::size_t size) {
+        SCOPED_TRACE(::testing::Message() << "size: " << size);
+
+        Buffer b{original};
+
+        // Resizing to zero is equivalent to clearing.
+        b(size);
+        EXPECT_TRUE(sane(b));
+        EXPECT_EQ(b.size(), size);
+        EXPECT_EQ(b.data() == nullptr, size == 0);
+
+        b(size + 1);
+        EXPECT_TRUE(sane(b));
+        EXPECT_EQ(b.size(), size + 1);
+        EXPECT_NE(b.data(), nullptr);
+
+        b.clear();
+        EXPECT_TRUE(sane(b));
+        EXPECT_TRUE(b.empty());
+        EXPECT_EQ(b.data(), nullptr);
+
+        // clear() is idempotent.
+        b.clear();
+        EXPECT_TRUE(sane(b));
+        EXPECT_TRUE(b.empty());
+        EXPECT_EQ(b.data(), nullptr);
+    };
+
+    for (auto size = 0uz; size < kHalf; ++size)
     {
-        static_assert(std::is_nothrow_move_constructible_v<Buffer>);
-        static_assert(std::is_nothrow_move_assignable_v<Buffer>);
-
-        {  // Move-construct from empty buf
-            Buffer x;
-            Buffer const y{std::move(x)};
-            EXPECT_TRUE(sane(x));    // NOLINT(bugprone-use-after-move)
-            EXPECT_TRUE(x.empty());  // NOLINT(bugprone-use-after-move)
-            EXPECT_TRUE(sane(y));
-            EXPECT_TRUE(y.empty());
-            EXPECT_EQ(x, y);  // NOLINT(bugprone-use-after-move)
-        }
-
-        {  // Move-construct from non-empty buf
-            Buffer x{b1};
-            Buffer const y{std::move(x)};
-            EXPECT_TRUE(sane(x));    // NOLINT(bugprone-use-after-move)
-            EXPECT_TRUE(x.empty());  // NOLINT(bugprone-use-after-move)
-            EXPECT_TRUE(sane(y));
-            EXPECT_EQ(y, b1);
-        }
-
-        {  // Move assign empty buf to empty buf
-            Buffer x;
-            Buffer y;
-
-            x = std::move(y);
-            EXPECT_TRUE(sane(x));
-            EXPECT_TRUE(x.empty());
-            EXPECT_TRUE(sane(y));    // NOLINT(bugprone-use-after-move)
-            EXPECT_TRUE(y.empty());  // NOLINT(bugprone-use-after-move)
-        }
-
-        {  // Move assign non-empty buf to empty buf
-            Buffer x;
-            Buffer y{b1};
-
-            x = std::move(y);
-            EXPECT_TRUE(sane(x));
-            EXPECT_EQ(x, b1);
-            EXPECT_TRUE(sane(y));    // NOLINT(bugprone-use-after-move)
-            EXPECT_TRUE(y.empty());  // NOLINT(bugprone-use-after-move)
-        }
-
-        {  // Move assign empty buf to non-empty buf
-            Buffer x{b1};
-            Buffer y;
-
-            x = std::move(y);
-            EXPECT_TRUE(sane(x));
-            EXPECT_TRUE(x.empty());
-            EXPECT_TRUE(sane(y));    // NOLINT(bugprone-use-after-move)
-            EXPECT_TRUE(y.empty());  // NOLINT(bugprone-use-after-move)
-        }
-
-        {  // Move assign non-empty buf to non-empty buf
-            Buffer x{b1};
-            Buffer y{b2};
-            Buffer z{b3};
-
-            x = std::move(y);
-            EXPECT_TRUE(sane(x));
-            EXPECT_FALSE(x.empty());
-            EXPECT_TRUE(sane(y));    // NOLINT(bugprone-use-after-move)
-            EXPECT_TRUE(y.empty());  // NOLINT(bugprone-use-after-move)
-
-            x = std::move(z);
-            EXPECT_TRUE(sane(x));
-            EXPECT_FALSE(x.empty());
-            EXPECT_TRUE(sane(z));    // NOLINT(bugprone-use-after-move)
-            EXPECT_TRUE(z.empty());  // NOLINT(bugprone-use-after-move)
-        }
-    }
-
-    {
-        Buffer w{static_cast<Slice>(b0)};
-        EXPECT_TRUE(sane(w));
-        EXPECT_EQ(w, b0);
-
-        Buffer x{static_cast<Slice>(b1)};
-        EXPECT_TRUE(sane(x));
-        EXPECT_EQ(x, b1);
-
-        Buffer y{static_cast<Slice>(b2)};
-        EXPECT_TRUE(sane(y));
-        EXPECT_EQ(y, b2);
-
-        Buffer z{static_cast<Slice>(b3)};
-        EXPECT_TRUE(sane(z));
-        EXPECT_EQ(z, b3);
-
-        // Assign empty slice to empty buffer
-        w = static_cast<Slice>(b0);
-        EXPECT_TRUE(sane(w));
-        EXPECT_EQ(w, b0);
-
-        // Assign non-empty slice to empty buffer
-        w = static_cast<Slice>(b1);
-        EXPECT_TRUE(sane(w));
-        EXPECT_EQ(w, b1);
-
-        // Assign non-empty slice to non-empty buffer
-        x = static_cast<Slice>(b2);
-        EXPECT_TRUE(sane(x));
-        EXPECT_EQ(x, b2);
-
-        // Assign non-empty slice to non-empty buffer
-        y = static_cast<Slice>(z);
-        EXPECT_TRUE(sane(y));
-        EXPECT_EQ(y, z);
-
-        // Assign empty slice to non-empty buffer:
-        z = static_cast<Slice>(b0);
-        EXPECT_TRUE(sane(z));
-        EXPECT_EQ(z, b0);
-    }
-
-    {
-        auto test = [](Buffer const& b, std::size_t i) {
-            Buffer x{b};
-
-            // Try to allocate some number of bytes, possibly
-            // zero (which means clear) and sanity check
-            x(i);
-            EXPECT_TRUE(sane(x));
-            EXPECT_EQ(x.size(), i);
-            EXPECT_EQ((x.data() == nullptr), (i == 0));
-
-            // Try to allocate some more data (always non-zero)
-            x(i + 1);
-            EXPECT_TRUE(sane(x));
-            EXPECT_EQ(x.size(), i + 1);
-            EXPECT_NE(x.data(), nullptr);
-
-            // Try to clear:
-            x.clear();
-            EXPECT_TRUE(sane(x));
-            EXPECT_TRUE(x.empty());
-            EXPECT_EQ(x.data(), nullptr);
-
-            // Try to clear again:
-            x.clear();
-            EXPECT_TRUE(sane(x));
-            EXPECT_TRUE(x.empty());
-            EXPECT_EQ(x.data(), nullptr);
-        };
-
-        for (std::size_t i = 0; i < 16; ++i)
-        {
-            test(b0, i);
-            test(b1, i);
-        }
+        check(emptyBuffer, size);
+        check(firstHalf, size);
     }
 }
 
+TEST_F(BufferTest, fill_sets_every_byte)
+{
+    Buffer b{4};
+    b.fill(0xab);
+
+    EXPECT_EQ(b.size(), 4);
+    for (auto const byte : Slice{b})
+        EXPECT_EQ(byte, 0xab);
+}
+
+TEST_F(BufferTest, fill_overwrites_and_keeps_size)
+{
+    Buffer b{4};
+    b.fill(0xab);
+    b.fill(0x00);
+
+    EXPECT_EQ(b.size(), 4);
+    for (auto const byte : Slice{b})
+        EXPECT_EQ(byte, 0x00);
+}
+
+TEST_F(BufferTest, fill_on_empty_buffer_is_a_noop)
+{
+    Buffer empty;
+    empty.fill(0xff);
+
+    EXPECT_TRUE(empty.empty());
+    EXPECT_EQ(empty.data(), nullptr);
+}
+
 }  // namespace xrpl::test

src/tests/libxrpl/basics/IntrusiveShared.cpp

@@ -92,6 +92,7 @@ public:
     static constexpr std::size_t kMaxStates = 128;
     static std::array<std::atomic<TrackedState>, kMaxStates> state;
     static std::atomic<std::size_t> nextId;
+
     static TrackedState
     getState(std::size_t id)
     {
@@ -100,13 +101,12 @@ public:
 
         return state[id].load(std::memory_order_acquire);
     }
+
     static void
     resetStates(bool resetCallback)
     {
         for (std::size_t i = 0; i < kMaxStates; ++i)
-        {
             state[i].store(TrackedState::Uninitialized, std::memory_order_release);
-        }
         nextId.store(0, std::memory_order_release);
         if (resetCallback)
             TIBase::tracingCallback = [](TrackedState, std::optional<TrackedState>) {};
@@ -120,6 +120,7 @@ public:
         {
             TIBase::resetStates(resetCallback);
         }
+
         ~ResetStatesGuard()
         {
             TIBase::resetStates(resetCallback);
@@ -130,6 +131,7 @@ public:
     {
         state[id].store(TrackedState::Alive, std::memory_order_relaxed);
     }
+
     ~TIBase() override
     {
         using enum TrackedState;
@@ -218,9 +220,7 @@ TEST(IntrusiveSharedTest, basics)
         EXPECT_EQ(TIBase::getState(id), Alive);
         EXPECT_EQ(b->useCount(), 1);
         for (auto i = 0uz; i < 10; ++i)
-        {
             strong.push_back(b);
-        }
         b.reset();
         EXPECT_EQ(TIBase::getState(id), Alive);
         strong.resize(strong.size() - 1);
@@ -244,8 +244,7 @@ TEST(IntrusiveSharedTest, basics)
         EXPECT_EQ(TIBase::getState(id), PartiallyDeleted);
         while (!weak.empty())
         {
-            weak.resize(weak.size() - 1);
-            if (!weak.empty())
+            if (weak.resize(weak.size() - 1); !weak.empty())
             {
                 EXPECT_EQ(TIBase::getState(id), PartiallyDeleted);
             }

src/tests/libxrpl/basics/base_uint.cpp

@@ -6,6 +6,7 @@
 
 #include <boost/endian/detail/order.hpp>
 
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
 #include <array>
@@ -205,125 +206,119 @@ TEST_F(BaseUintTest, base_uint)
     Blob const raw{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12};
     EXPECT_EQ(BaseUInt96::kBytes, raw.size());
 
-    BaseUInt96 u = BaseUInt96::fromRaw(raw);
-    uset.insert(u);
-    EXPECT_EQ(raw.size(), u.size());
-    EXPECT_EQ(to_string(u), "0102030405060708090A0B0C");
-    EXPECT_EQ(toShortString(u), "01020304...");
-    EXPECT_EQ(*u.data(), 1);
-    EXPECT_EQ(u.signum(), 1);
-    EXPECT_FALSE(!u);
-    EXPECT_FALSE(u.isZero());
-    EXPECT_TRUE(u.isNonZero());
-    unsigned char t = 0;
-    for (auto& d : u)
-    {
-        EXPECT_EQ(d, ++t);
-    }
+    BaseUInt96 ascending = BaseUInt96::fromRaw(raw);
+    uset.insert(ascending);
+    EXPECT_EQ(raw.size(), ascending.size());
+    EXPECT_EQ(to_string(ascending), "0102030405060708090A0B0C");
+    EXPECT_EQ(toShortString(ascending), "01020304...");
+    EXPECT_EQ(*ascending.data(), 1);
+    EXPECT_EQ(ascending.signum(), 1);
+    EXPECT_FALSE(!ascending);
+    EXPECT_FALSE(ascending.isZero());
+    EXPECT_TRUE(ascending.isNonZero());
+    unsigned char expectedByte = 0;
+    for (auto& byte : ascending)
+        EXPECT_EQ(byte, ++expectedByte);
 
-    // Test hash_append by "hashing" with a no-op hasher (h)
+    // Test hash_append by "hashing" with a no-op hasher (hasher)
     // and then extracting the bytes that were written during hashing
-    // back into another base_uint (w) for comparison with the original
-    Nonhash<96> h{};
-    hash_append(h, u);
-    BaseUInt96 const w =
-        BaseUInt96::fromRaw(std::vector<std::uint8_t>(h.data.begin(), h.data.end()));
-    EXPECT_EQ(w, u);
+    // back into another base_uint (rehashed) for comparison with the original
+    Nonhash<96> hasher{};
+    hash_append(hasher, ascending);
+    BaseUInt96 const rehashed =
+        BaseUInt96::fromRaw(std::vector<std::uint8_t>(hasher.data.begin(), hasher.data.end()));
+    EXPECT_EQ(rehashed, ascending);
 
-    BaseUInt96 v{~u};
-    uset.insert(v);
-    EXPECT_EQ(to_string(v), "FEFDFCFBFAF9F8F7F6F5F4F3");
-    EXPECT_EQ(toShortString(v), "FEFDFCFB...");
-    EXPECT_EQ(*v.data(), 0xfe);
-    EXPECT_EQ(v.signum(), 1);
-    EXPECT_FALSE(!v);
-    EXPECT_FALSE(v.isZero());
-    EXPECT_TRUE(v.isNonZero());
+    BaseUInt96 complement{~ascending};
+    uset.insert(complement);
+    EXPECT_EQ(to_string(complement), "FEFDFCFBFAF9F8F7F6F5F4F3");
+    EXPECT_EQ(toShortString(complement), "FEFDFCFB...");
+    EXPECT_EQ(*complement.data(), 0xfe);
+    EXPECT_EQ(complement.signum(), 1);
+    EXPECT_FALSE(!complement);
+    EXPECT_FALSE(complement.isZero());
+    EXPECT_TRUE(complement.isNonZero());
 
-    t = 0xff;
-    for (auto& d : v)
-    {
-        EXPECT_EQ(d, --t);
-    }
+    expectedByte = 0xff;
+    for (auto& byte : complement)
+        EXPECT_EQ(byte, --expectedByte);
 
-    EXPECT_LT(u, v);
-    EXPECT_GT(v, u);
+    EXPECT_LT(ascending, complement);
+    EXPECT_GT(complement, ascending);
 
-    v = u;
-    EXPECT_EQ(v, u);
+    complement = ascending;
+    EXPECT_EQ(complement, ascending);
 
-    BaseUInt96 z{beast::kZero};
-    uset.insert(z);
-    EXPECT_EQ(to_string(z), "000000000000000000000000");
-    EXPECT_EQ(toShortString(z), "00000000...");
-    EXPECT_EQ(*z.data(), 0);
-    EXPECT_EQ(*z.begin(), 0);
-    EXPECT_EQ(*std::prev(z.end(), 1), 0);
-    EXPECT_EQ(z.signum(), 0);
-    EXPECT_TRUE(!z);
-    EXPECT_TRUE(z.isZero());
-    EXPECT_FALSE(z.isNonZero());
-    for (auto& d : z)
-    {
-        EXPECT_EQ(d, 0);
-    }
+    BaseUInt96 zero{beast::kZero};
+    uset.insert(zero);
+    EXPECT_EQ(to_string(zero), "000000000000000000000000");
+    EXPECT_EQ(toShortString(zero), "00000000...");
+    EXPECT_EQ(*zero.data(), 0);
+    EXPECT_EQ(*zero.begin(), 0);
+    EXPECT_EQ(*std::prev(zero.end(), 1), 0);
+    EXPECT_EQ(zero.signum(), 0);
+    EXPECT_TRUE(!zero);
+    EXPECT_TRUE(zero.isZero());
+    EXPECT_FALSE(zero.isNonZero());
+    for (auto& byte : zero)
+        EXPECT_EQ(byte, 0);
 
     {
         // There are several ways to create a zero. beast::kZero is tested above. Test some
         // others.
-        BaseUInt96 const z1;
-        EXPECT_EQ(z1, z) << to_string(z1);
+        BaseUInt96 const defaultZero;
+        EXPECT_EQ(defaultZero, zero) << to_string(defaultZero);
 
-        BaseUInt96 const z2{};
-        EXPECT_EQ(z2, z) << to_string(z2);
+        BaseUInt96 const bracedZero{};
+        EXPECT_EQ(bracedZero, zero) << to_string(bracedZero);
 
-        BaseUInt96 const z3{0u};
-        EXPECT_EQ(z3, z) << to_string(z3);
+        BaseUInt96 const zeroFromUInt{0u};
+        EXPECT_EQ(zeroFromUInt, zero) << to_string(zeroFromUInt);
     }
 
-    BaseUInt96 n{z};
-    n++;
-    EXPECT_EQ(n, BaseUInt96(1));
-    n--;
-    EXPECT_EQ(n, beast::kZero);
-    EXPECT_EQ(n, z);
-    n--;
-    EXPECT_EQ(to_string(n), "FFFFFFFFFFFFFFFFFFFFFFFF");
-    EXPECT_EQ(toShortString(n), "FFFFFFFF...");
-    n = beast::kZero;
-    EXPECT_EQ(n, z);
+    BaseUInt96 counter{zero};
+    counter++;
+    EXPECT_EQ(counter, BaseUInt96(1));
+    counter--;
+    EXPECT_EQ(counter, beast::kZero);
+    EXPECT_EQ(counter, zero);
+    counter--;
+    EXPECT_EQ(to_string(counter), "FFFFFFFFFFFFFFFFFFFFFFFF");
+    EXPECT_EQ(toShortString(counter), "FFFFFFFF...");
+    counter = beast::kZero;
+    EXPECT_EQ(counter, zero);
 
-    BaseUInt96 zp1{z};
-    zp1++;
-    BaseUInt96 zm1{z};
-    zm1--;
-    BaseUInt96 const x{zm1 ^ zp1};
-    uset.insert(x);
-    EXPECT_EQ(to_string(x), "FFFFFFFFFFFFFFFFFFFFFFFE") << to_string(x);
-    EXPECT_EQ(toShortString(x), "FFFFFFFF...") << toShortString(x);
+    BaseUInt96 zeroPlusOne{zero};
+    zeroPlusOne++;
+    BaseUInt96 zeroMinusOne{zero};
+    zeroMinusOne--;
+    BaseUInt96 const xored{zeroMinusOne ^ zeroPlusOne};
+    uset.insert(xored);
+    EXPECT_EQ(to_string(xored), "FFFFFFFFFFFFFFFFFFFFFFFE") << to_string(xored);
+    EXPECT_EQ(toShortString(xored), "FFFFFFFF...") << toShortString(xored);
 
     EXPECT_EQ(uset.size(), 4);
 
-    BaseUInt96 tmp;
-    EXPECT_TRUE(tmp.parseHex(to_string(u)));
-    EXPECT_EQ(tmp, u);
-    tmp = z;
+    BaseUInt96 parsed;
+    EXPECT_TRUE(parsed.parseHex(to_string(ascending)));
+    EXPECT_EQ(parsed, ascending);
+    parsed = zero;
 
     // fails with extra char
-    EXPECT_FALSE(tmp.parseHex("A" + to_string(u)));
-    tmp = z;
+    EXPECT_FALSE(parsed.parseHex("A" + to_string(ascending)));
+    parsed = zero;
 
     // fails with extra char at end
-    EXPECT_FALSE(tmp.parseHex(to_string(u) + "A"));
+    EXPECT_FALSE(parsed.parseHex(to_string(ascending) + "A"));
 
     // fails with a non-hex character at some point in the string:
-    tmp = z;
+    parsed = zero;
 
     for (std::size_t i = 0; i != 24; ++i)
     {
-        std::string x = to_string(z);
-        x[i] = ('G' + (i % 10));
-        EXPECT_FALSE(tmp.parseHex(x));
+        std::string xored = to_string(zero);
+        xored[i] = ('G' + (i % 10));
+        EXPECT_FALSE(parsed.parseHex(xored));
     }
 
     // Walking 1s:
@@ -332,8 +327,8 @@ TEST_F(BaseUintTest, base_uint)
         std::string s1 = "000000000000000000000000";
         s1[i] = '1';
 
-        EXPECT_TRUE(tmp.parseHex(s1));
-        EXPECT_EQ(to_string(tmp), s1);
+        EXPECT_TRUE(parsed.parseHex(s1));
+        EXPECT_EQ(to_string(parsed), s1);
     }
 
     // Walking 0s:
@@ -342,8 +337,8 @@ TEST_F(BaseUintTest, base_uint)
         std::string s1 = "111111111111111111111111";
         s1[i] = '0';
 
-        EXPECT_TRUE(tmp.parseHex(s1));
-        EXPECT_EQ(to_string(tmp), s1);
+        EXPECT_TRUE(parsed.parseHex(s1));
+        EXPECT_EQ(to_string(parsed), s1);
     }
 
     // Constexpr constructors
@@ -357,39 +352,27 @@ TEST_F(BaseUintTest, base_uint)
         // Using the constexpr constructor in a non-constexpr context
         // with an error in the parsing throws an exception.
         {
-            // Invalid length for string.
-            bool caught = false;
-            try
-            {
-                // Try to prevent constant evaluation.
-                std::vector<char> str(23, '7');
+            // Invalid length for string. The vector keeps this out of a constant
+            // expression, so the constructor throws instead of failing to compile.
+            auto tooShort = [] {
+                std::vector<char> const str(23, '7');
                 std::string_view const sView(str.data(), str.size());
                 [[maybe_unused]] BaseUInt96 const t96(sView);
-            }
-            catch (std::invalid_argument const& e)
-            {
-                EXPECT_EQ(e.what(), std::string("invalid length for hex string"));
-                caught = true;
-            }
-            EXPECT_TRUE(caught);
+            };
+            EXPECT_THAT(
+                tooShort,
+                ::testing::ThrowsMessage<std::invalid_argument>("invalid length for hex string"));
         }
         {
             // Invalid character in string.
-            bool caught = false;
-            try
-            {
-                // Try to prevent constant evaluation.
+            auto badCharacter = [] {
                 std::vector<char> str(23, '7');
                 str.push_back('G');
                 std::string_view const sView(str.data(), str.size());
                 [[maybe_unused]] BaseUInt96 const t96(sView);
-            }
-            catch (std::range_error const& e)
-            {
-                EXPECT_EQ(e.what(), std::string("invalid hex character"));
-                caught = true;
-            }
-            EXPECT_TRUE(caught);
+            };
+            EXPECT_THAT(
+                badCharacter, ::testing::ThrowsMessage<std::range_error>("invalid hex character"));
         }
 
         // Verify that constexpr base_uints interpret a string the same
@@ -412,11 +395,11 @@ TEST_F(BaseUintTest, base_uint)
             "fFfFfFfFfFfFfFfFfFfFfFfF",
         });
 
-        for (StrBaseUInt const& t : kTestCases)
+        for (StrBaseUInt const& expectedByte : kTestCases)
         {
             BaseUInt96 t96;
-            EXPECT_TRUE(t96.parseHex(t.str));
-            EXPECT_EQ(t96, t.tst);
+            EXPECT_TRUE(t96.parseHex(expectedByte.str));
+            EXPECT_EQ(t96, expectedByte.tst);
         }
     }
 }

src/tests/libxrpl/shamap/SHAMap.cpp

@@ -16,7 +16,6 @@
 #include <helpers/TestSink.h>
 #include <shamap/common.h>
 
-#include <algorithm>
 #include <array>
 #include <cstddef>
 #include <cstdint>
@@ -113,7 +112,7 @@ protected:
     intToVuc(std::uint8_t v)
     {
         Buffer vuc{32};
-        std::fill_n(vuc.data(), vuc.size(), v);
+        vuc.fill(v);
         return vuc;
     }
 };

src/xrpld/rpc/detail/RPCLedgerHelpers.cpp

@@ -331,6 +331,13 @@ getLedger<>(std::shared_ptr<ReadView const>&, LedgerShortcut shortcut, Context c
 template Status
 getLedger<>(std::shared_ptr<ReadView const>&, uint256 const&, Context const&);
 
+// explicit instantiation of ledgerFromSpecifier
+template Status
+ledgerFromSpecifier<>(
+    std::shared_ptr<ReadView const>&,
+    org::xrpl::rpc::v1::LedgerSpecifier const&,
+    Context const&);
+
 // 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

src/xrpld/rpc/handlers/account/AccountLines.cpp

@@ -107,7 +107,12 @@ doAccountLines(rpc::JsonContext& context)
 
     std::string strPeer;
     if (params.isMember(jss::peer))
+    {
+        if (!params[jss::peer].isString())
+            return rpc::invalidFieldError(jss::peer);
+
         strPeer = params[jss::peer].asString();
+    }
 
     auto const raPeerAccount = [&]() -> std::optional<AccountID> {
         return strPeer.empty() ? std::nullopt : parseBase58<AccountID>(strPeer);

src/xrpld/rpc/handlers/account/GatewayBalances.cpp

@@ -63,6 +63,12 @@ doGatewayBalances(rpc::JsonContext& context)
     if (!(params.isMember(jss::account) || params.isMember(jss::ident)))
         return rpc::missingFieldError(jss::account);
 
+    if (params.isMember(jss::account) && !params[jss::account].isString())
+        return rpc::invalidFieldError(jss::account);
+
+    if (params.isMember(jss::ident) && !params[jss::ident].isString())
+        return rpc::invalidFieldError(jss::ident);
+
     std::string const strIdent(
         params.isMember(jss::account) ? params[jss::account].asString()
                                       : params[jss::ident].asString());

src/xrpld/rpc/handlers/orderbook/NFTOffersHelpers.h

@@ -93,6 +93,17 @@ enumerateNFTOffers(rpc::JsonContext& context, uint256 const& nftId, Keylet const
         if (!sle || nftId != sle->getFieldH256(sfNFTokenID))
             return rpcError(RpcInvalidParams);
 
+        // Reject a marker that references an offer on the opposite side
+        // (buy vs. sell) of the directory being enumerated.  Without this
+        // check the marker's node hint points into the other directory, so
+        // forEachItemAfter never finds `startAfter` and instead scans every
+        // page of `directory` before returning invalidParams -- turning an
+        // O(1) rejection into an O(directory size) walk.
+        auto const offerDir =
+            sle->isFlag(lsfSellNFToken) ? keylet::nftSells(nftId) : keylet::nftBuys(nftId);
+        if (directory.key != offerDir.key)
+            return rpcError(RpcInvalidParams);
+
         startHint = sle->getFieldU64(sfNFTokenOfferNode);
         appendNftOfferJson(context.app, sle, jsonOffers);
         offers.reserve(reserve);