merge: bring develop forward into phase-3 tx tracing

Resolved src/libxrpl/tx/applySteps.cpp. calculateBaseFee now returns
std::expected<XRPAmount, TER>, so invokePreclaim rejects the transaction when
the fee cannot be computed. Kept that inside this branch's preclaimTer wrapper,
so the returned error also reaches the span's ter_result attribute.

include/xrpl/basics/StringUtilities.h

@@ -162,4 +162,89 @@ toUInt64(std::string const& s);
 bool
 isProperlyFormedTomlDomain(std::string_view domain);
 
+/**
+ * Whether a view can be passed on as a C string.
+ *
+ * A reader given only data() stops at the first null, so the view must reach the
+ * terminating null. The test rebuilds the view from data() and compares: a view
+ * that stops earlier rebuilds longer, and so compares unequal.
+ *
+ * consteval because reading the byte after the view is only defined when @p str
+ * points into storage holding a null at or after its end, such as a string
+ * literal. An unterminated view is then a compile error, not an out-of-bounds
+ * read.
+ *
+ * @param str The view to test.
+ * @return Whether @p str is null-terminated. A view with no data is not.
+ */
+consteval bool
+isNullTerminated(std::string_view str)
+{
+    if (str.data() == nullptr)
+        return false;
+
+    // Reading past the view is the point, so the usual data() warning does not
+    // apply.
+    // NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
+    return std::string_view{str.data()} == str;
+}
+
+/**
+ * A string that is known to reach its terminating null.
+ *
+ * Converts to std::string_view, so it compares and hashes as one. Unlike a
+ * view, asCString() may be handed to a reader that expects a C string, such
+ * as json::StaticString.
+ *
+ * The only constructor is consteval and rejects a view that stops before the
+ * null, so the property holds by construction and no caller asserts it.
+ */
+class NullTerminatedView
+{
+public:
+    /**
+     * Build a view from one that reaches its terminating null.
+     *
+     * Explicit, so that a plain view cannot become a proof of termination by
+     * accident. The conversion the other way stays implicit.
+     *
+     * @param view The string to hold. Rejected at compile time if it stops
+     *        before its terminating null, or has no data.
+     */
+    explicit consteval NullTerminatedView(std::string_view view)
+        : data_(view.data()), size_(view.size())
+    {
+        if (!isNullTerminated(view))
+            throw "xrpl::NullTerminatedView : view does not reach a null";
+    }
+
+    constexpr
+    operator std::string_view() const noexcept
+    {
+        return view();
+    }
+
+    /**
+     * @return The string as a view.
+     */
+    [[nodiscard]] constexpr std::string_view
+    view() const noexcept
+    {
+        return {data_, size_};
+    }
+
+    /**
+     * @return The string as a C string. Never null.
+     */
+    [[nodiscard]] constexpr char const*
+    asCString() const noexcept
+    {
+        return data_;
+    }
+
+private:
+    char const* data_;
+    std::size_t size_;
+};
+
 }  // namespace xrpl

include/xrpl/core/PerfLog.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <xrpl/basics/Log.h>
+#include <xrpl/basics/StringUtilities.h>
 #include <xrpl/core/Job.h>
 #include <xrpl/json/json_value.h>
 
@@ -9,7 +10,8 @@
 #include <filesystem>
 #include <functional>
 #include <memory>
-#include <string>
+#include <span>
+#include <string_view>
 
 namespace beast {
 class Journal;
@@ -67,7 +69,7 @@ public:
      * @param requestId Unique identifier to track command
      */
     virtual void
-    rpcStart(std::string const& method, std::uint64_t requestId) = 0;
+    rpcStart(std::string_view method, std::uint64_t requestId) = 0;
 
     /**
      * Log successful finish of RPC call
@@ -76,7 +78,7 @@ public:
      * @param requestId Unique identifier to track command
      */
     virtual void
-    rpcFinish(std::string const& method, std::uint64_t requestId) = 0;
+    rpcFinish(std::string_view method, std::uint64_t requestId) = 0;
 
     /**
      * Log errored RPC call
@@ -85,7 +87,7 @@ public:
      * @param requestId Unique identifier to track command
      */
     virtual void
-    rpcError(std::string const& method, std::uint64_t requestId) = 0;
+    rpcError(std::string_view method, std::uint64_t requestId) = 0;
 
     /**
      * Log queued job
@@ -150,10 +152,20 @@ public:
 PerfLog::Setup
 setupPerfLog(Section const& section, std::filesystem::path const& configDir);
 
+/**
+ * @param methodNames The RPC methods to count, one counter per name. Reported
+ *        as JSON keys that borrow each name and read it as a C string, which is
+ *        why the parameter type requires one that reaches its terminating null.
+ *        The names must outlive the returned object, which holds views of them.
+ *        The range itself need not: it is copied.
+ *        Passed in rather than looked up here, so that this layer needs no
+ *        knowledge of the dispatch table.
+ */
 std::unique_ptr<PerfLog>
 makePerfLog(
     PerfLog::Setup const& setup,
     Application& app,
+    std::span<NullTerminatedView const> methodNames,
     beast::Journal journal,
     std::function<void()>&& signalStop);
 
@@ -161,7 +173,7 @@ template <typename Func, class Rep, class Period>
 auto
 measureDurationAndLog(
     Func&& func,
-    std::string const& actionDescription,
+    std::string_view actionDescription,
     std::chrono::duration<Rep, Period> maxDelay,
     beast::Journal const& journal)
 {

include/xrpl/ledger/helpers/LendingHelpers.h

@@ -338,9 +338,9 @@ struct AccountingDeltas
     Number debtTotalDelta;
 };
 
-// Whole-life (pre-LendingProtocolV1_1) recognition model: interest is
-// recognized into AssetsTotal/DebtTotal up front, at origination.
-namespace accrual {
+// Instant interest recognition (pre-LendingProtocolV1_1): interest is
+// recognized into AssetsTotal/DebtTotal immediately, at origination.
+namespace instant_recognition {
 
 // LoanSet origination: what's added to Vault.AssetsTotal and LoanBroker.DebtTotal
 AccountingDeltas
@@ -362,7 +362,7 @@ loanVaultExposure(SLE::const_ref loanSle);
 AccountingDeltas
 loanPaymentDeltas(LoanPaymentParts const& parts);
 
-}  // namespace accrual
+}  // namespace instant_recognition
 
 // Cash-basis (LendingProtocolV1_1) recognition model: AssetsTotal/DebtTotal
 // are principal-only, interest is recognized only as it's actually paid.
@@ -381,7 +381,7 @@ loanPaymentDeltas(LoanPaymentParts const& parts);
 
 // Public dispatchers: pick cash_basis:: if featureLendingProtocolV1_1 is
 // enabled AND the Vault's LEVersion (VaultHelpers::getVaultVersion) is
-// VaultVersion::CashBasis, else accrual::. These are the only entry points
+// VaultVersion::CashBasis, else instant_recognition::. These are the only entry points
 // transactors call.
 AccountingDeltas
 loanOriginationDeltas(

include/xrpl/ledger/helpers/VaultHelpers.h

@@ -172,8 +172,8 @@ isSoleShareholder(ReadView const& view, AccountID const& account, SLE::const_ref
 
 /**
  * Resolves a Vault's LEVersion, the single point every accounting touch
- * point should call to determine which recognition model (accrual vs.
- * cash-basis) a Vault uses. Vaults created before featureLendingProtocolV1_1
+ * point should call to determine which recognition model (instant interest
+ * recognition vs. cash-basis) a Vault uses. Vaults created before featureLendingProtocolV1_1
  * activated never have sfLEVersion set, which resolves here to
  * VaultVersion::Legacy.
  *

include/xrpl/nodestore/detail/DatabaseNodeImp.h

@@ -92,7 +92,7 @@ public:
     void
     importDatabase(Database& source) override
     {
-        importInternal(*backend_.get(), source);
+        importInternal(*backend_, source);
     }
 
     void

include/xrpl/protocol/PathAsset.h

@@ -5,9 +5,11 @@
 #include <xrpl/protocol/Concepts.h>
 #include <xrpl/protocol/UintTypes.h>
 
+#include <cstdint>
 #include <ostream>
 #include <stdexcept>
 #include <string>
+#include <type_traits>
 #include <variant>
 
 namespace xrpl {
@@ -121,9 +123,32 @@ operator==(PathAsset const& lhs, PathAsset const& rhs)
 
 template <typename Hasher>
 void
-hash_append(Hasher& h, PathAsset const& pathAsset)
+hash_append(Hasher& h, PathAsset const& pathAsset) noexcept
 {
-    std::visit([&]<ValidPathAsset T>(T const& e) { hash_append(h, e); }, pathAsset.value());
+    using beast::hash_append;
+    using Variant = std::remove_cvref_t<decltype(pathAsset.value())>;
+
+    static_assert(
+        std::variant_size_v<Variant> < 0xFFu,
+        "PathAsset's discriminant must fit in a byte, leaving 0xFF reserved.");
+
+    // std::visit is not noexcept: it throws bad_variant_access when the variant
+    // is valueless_by_exception.
+    if (pathAsset.value().valueless_by_exception()) [[unlikely]]
+    {
+        hash_append(h, static_cast<std::uint8_t>(0xFFu));
+        return;
+    }
+
+    hash_append(h, static_cast<std::uint8_t>(pathAsset.value().index()));
+    std::visit(
+        [&]<ValidPathAsset T>(T const& e) noexcept {
+            static_assert(
+                noexcept(hash_append(h, e)),
+                "Every PathAsset alternative must be nothrow-hashable.");
+            hash_append(h, e);
+        },
+        pathAsset.value());
 }
 
 inline bool

include/xrpl/protocol/Protocol.h

@@ -322,7 +322,7 @@ constexpr std::uint8_t kVaultMaximumIouScale = 18;
  * Vault ledger-entry schema versions. Assigned to newly created
  * Vaults once featureLendingProtocolV1_1 is enabled. Vaults created before
  * activation are left without LEVersion (implicit legacy version 0,
- * accrual-basis accounting).
+ * instant interest recognition).
  */
 enum class VaultVersion : uint8_t {
     Legacy = 0,

include/xrpl/protocol/STPathSet.h

@@ -12,6 +12,8 @@
 #include <xrpl/protocol/UintTypes.h>
 
 #include <cstddef>
+#include <cstdint>
+#include <memory>
 #include <optional>
 #include <utility>
 #include <vector>
@@ -65,7 +67,7 @@ public:
         PathAsset const& asset,
         AccountID const& issuer);
 
-    [[nodiscard]] auto
+    [[nodiscard]] std::uint32_t
     getNodeType() const;
 
     [[nodiscard]] bool
@@ -109,9 +111,6 @@ public:
     [[nodiscard]] bool
     isType(Type const& pe) const;
 
-    [[nodiscard]] size_t
-    getHash() const;
-
     bool
     operator==(STPathElement const& t) const;
 
@@ -120,6 +119,17 @@ private:
     getHash(STPathElement const& element);
 };
 
+template <class Hasher>
+void
+hash_append(Hasher& h, STPathElement const& e) noexcept
+{
+    using beast::hash_append;
+    hash_append(h, (e.getNodeType() & STPathElement::TypeAccount) != 0u);
+    hash_append(h, e.getAccountID());
+    hash_append(h, e.getPathAsset());
+    hash_append(h, e.getIssuerID());
+}
+
 class STPath final : public CountedObject<STPath>
 {
     std::vector<STPathElement> path_;
@@ -176,9 +186,10 @@ template <class Hasher>
 void
 hash_append(Hasher& h, STPath const& p) noexcept
 {
+    using beast::hash_append;
     for (auto const& e : p)
     {
-        beast::hash_append(h, e.getHash());
+        hash_append(h, e);
     }
 }
 
@@ -188,13 +199,39 @@ hash_append(Hasher& h, STPath const& p) noexcept
 class STPathSet final : public STBase, public CountedObject<STPathSet>
 {
     std::vector<STPath> value_;
-    xrpl::hardened_hash_set<STPath> seenHashes_;
+
+    /**
+     * Deduplication index over `value_`, for pathfinding.
+     * The use of a std::unique_ptr is intentional as it
+     * only requires 8 additional bytes of storage for the pointer
+     * as opposed to 64 bytes with an optional.  This keeps the size
+     * of the STPathSet to within the `STVar::kMaxSize` limit of 72 bytes.
+     */
+    std::unique_ptr<hardened_hash_set<STPath>> seen_;
 
 public:
+    struct DeduplicationTag
+    {
+    };
+
     STPathSet() = default;
+    /**
+     * Deduplication tagged constructor.
+     * Use when you want to ensure that the STPathSet does not contain duplicate paths.
+     */
+    explicit STPathSet(DeduplicationTag);
 
     STPathSet(SField const& n);
     STPathSet(SerialIter& sit, SField const& name);
+    STPathSet(STPathSet const& other);
+    STPathSet(STPathSet&&) = default;
+
+    STPathSet&
+    operator=(STPathSet const& other);
+    STPathSet&
+    operator=(STPathSet&&) = default;
+
+    ~STPathSet() override = default;
 
     void
     add(Serializer& s) const override;
@@ -204,6 +241,16 @@ public:
     [[nodiscard]] SerializedTypeID
     getSType() const override;
 
+    /**
+     * @brief assembleAdd adds a path to the set by combining a base path and a tail element.
+     *
+     * @param base The base path.
+     * @param tail The tail element.
+     * @return true if the path was added, false if it was a duplicate and not added.
+     * @remarks Requires the STPathSet to be constructed with the DeduplicationTag. The return value
+     * indicates whether the combined path was inserted (true) or rejected as a duplicate (false).
+     * It is fine for callers to ignore the return value.
+     */
     bool
     assembleAdd(STPath const& base, STPathElement const& tail);
 
@@ -229,22 +276,61 @@ public:
     [[nodiscard]] bool
     empty() const;
 
-    void
+    /**
+     * @brief pushBack adds a path to the set.
+     *
+     * @param e The path to add.
+     * @return true if the path was added, false if it was a duplicate and not added.
+     * @remarks If the STPathSet was constructed with the DeduplicationTag, then this method will
+     *          check for duplicates and only add the path if it is not already present in the
+     *          set. If the STPathSet was constructed without the DeduplicationTag,
+     *          then this method will always add the path to the set, regardless of duplicates.
+     *          It is fine for callers to ignore the return value.
+     */
+    bool
     pushBack(STPath const& e);
 
+    /**
+     * @brief emplaceBack adds a path to the set.
+     *
+     * @param args The arguments to construct the path with.
+     * @return true if the path was added, false if it was a duplicate and not added.
+     * @remarks If the STPathSet was constructed with the DeduplicationTag, then this method will
+     *          check for duplicates and only add the path if it is not already present in the
+     *          set. If the STPathSet was constructed without the DeduplicationTag,
+     *          then this method will always add the path to the set, regardless of duplicates.
+     *          It is fine for callers to ignore the return value.
+     * @note The path is constructed before the duplicate check, so on a false
+     *       return the constructed path is discarded and any argument
+     *       forwarded as an rvalue is left in a moved-from state.  Use
+     *       pushBack when the caller needs to keep its path on rejection.
+     */
     template <typename... Args>
-    void
+    bool
     emplaceBack(Args&&... args);
 
-    [[nodiscard]] bool
-    contains(STPath const& path) const;
-
 private:
     STBase*
     copy(std::size_t n, void* buf) const override;
     STBase*
     move(std::size_t n, void* buf) override;
 
+    /**
+     * @brief Append a path via `append`, then register it in the deduplication index.
+     *
+     * @param append Invoked with `value_`; must append exactly one path to it.
+     * @return true if the path was kept, false if it was a duplicate and was rolled back.
+     * @remarks Appends to the vector before touching the index, so that a failed allocation
+     *          there leaves both containers untouched rather than leaving the index holding
+     *          a path the vector does not.  If the index insert reports a duplicate, or
+     *          throws, the append is rolled back so the two containers stay consistent; in
+     *          the throwing case the exception propagates.  With no index (constructed
+     *          without the DeduplicationTag) the append is unconditional.
+     */
+    template <typename Append>
+    bool
+    appendUnique(Append&& append);
+
     friend class detail::STVar;
 };
 
@@ -336,7 +422,7 @@ inline STPathElement::STPathElement(
     hashValue_ = getHash(*this);
 }
 
-inline auto
+inline std::uint32_t
 STPathElement::getNodeType() const
 {
     return type_;
@@ -545,25 +631,50 @@ STPathSet::empty() const
     return value_.empty();
 }
 
-inline void
-STPathSet::pushBack(STPath const& e)
+template <typename Append>
+inline bool
+STPathSet::appendUnique(Append&& append)
 {
-    value_.push_back(e);
-    seenHashes_.emplace(value_.back());
-}
+    // Append to the vector first, so that a failed allocation there leaves both
+    // containers untouched rather than leaving the index holding a path the
+    // vector does not.
+    append(value_);
 
-template <typename... Args>
-inline void
-STPathSet::emplaceBack(Args&&... args)
-{
-    value_.emplace_back(std::forward<Args>(args)...);
-    seenHashes_.emplace(value_.back());
+    if (seen_ == nullptr)
+    {
+        return true;
+    }
+
+    try
+    {
+        if (!seen_->insert(value_.back()).second)
+        {
+            // Already present: roll back the append.
+            value_.pop_back();
+            return false;
+        }
+    }
+    catch (...)
+    {
+        // The index insert failed, so roll back the append to keep the vector
+        // and the index consistent.
+        value_.pop_back();
+        throw;
+    }
+    return true;
 }
 
 inline bool
-STPathSet::contains(STPath const& path) const
+STPathSet::pushBack(STPath const& e)
 {
-    return seenHashes_.contains(path);
+    return appendUnique([&](auto& value) { value.push_back(e); });
+}
+
+template <typename... Args>
+inline bool
+STPathSet::emplaceBack(Args&&... args)
+{
+    return appendUnique([&](auto& value) { value.emplace_back(std::forward<Args>(args)...); });
 }
 
 }  // namespace xrpl

include/xrpl/protocol/STValidation.h

@@ -124,6 +124,13 @@ public:
     [[nodiscard]] NodeID const&
     getNodeID() const noexcept;
 
+    /**
+     * Whether this validation carries a good signature.
+     *
+     * Reports false if the signature cannot be checked at all, so a caller
+     * cannot tell that apart from a bad signature. Either way the validation is
+     * unusable, and the reason is logged. Only a computed answer is remembered.
+     */
     [[nodiscard]] bool
     isValid() const noexcept;
 

include/xrpl/protocol/Serializer.h

@@ -10,6 +10,7 @@
 #include <xrpl/protocol/HashPrefix.h>
 #include <xrpl/protocol/SField.h>
 
+#include <cstddef>
 #include <cstdint>
 #include <cstring>
 #include <stdexcept>
@@ -25,6 +26,101 @@ private:
     Blob data_;
 
 public:
+    /**
+     * A header is never longer than this. The encoder fills a buffer of this
+     * size and writes only the bytes it used.
+     */
+    static constexpr int kMaxNumberOfBytesInHeader = 3;
+
+    // A field whose size varies is stored as a header holding its length, then
+    // the field data. The header is 1, 2 or 3 bytes long. Nothing outside it says
+    // which, so the decoder reads the first byte and its value says how long the
+    // header is:
+    //
+    //     0 ... 192   kMin/kMaxValueOfFirstByteFor1ByteHeader
+    //   193 ... 240   kMin/kMaxValueOfFirstByteFor2ByteHeader
+    //   241 ... 254   kMin/kMaxValueOfFirstByteFor3ByteHeader
+    //         255     belongs to no header
+    //
+    // Each range starts one past the end of the range before it.
+
+    static constexpr int kMinValueOfFirstByteFor1ByteHeader = 0;
+    static constexpr int kMaxValueOfFirstByteFor1ByteHeader = 192;
+
+    static constexpr int kMinValueOfFirstByteFor2ByteHeader =
+        kMaxValueOfFirstByteFor1ByteHeader + 1;
+    static constexpr int kMaxValueOfFirstByteFor2ByteHeader = 240;
+
+    static constexpr int kMinValueOfFirstByteFor3ByteHeader =
+        kMaxValueOfFirstByteFor2ByteHeader + 1;
+
+    static constexpr int kMaxValueOfFirstByteFor3ByteHeader = 254;
+
+    // A length x too big for one byte is split across the header. For 2 bytes:
+    //
+    //     first byte  = 193 + (x - 193) / 256
+    //     second byte = (x - 193) % 256
+    //
+    // so 300 is stored as 193, 107. For 3 bytes it is the same, from 241, with
+    // the remainder split across two bytes: 20,000 is stored as 241, 29, 95.
+
+    static constexpr int kNumberOfValuesInOneByte = 256;
+    static constexpr int kNumberOfValuesInTwoBytes =
+        kNumberOfValuesInOneByte * kNumberOfValuesInOneByte;
+
+    // Each header length therefore covers a range of field lengths:
+    //
+    //        0 ...     192   kMin/kMaxValueOfLengthFor1ByteHeader
+    //      193 ...  12,480   kMin/kMaxValueOfLengthFor2ByteHeader
+    //   12,481 ... 918,744   kMin/kMaxValueOfLengthFor3ByteHeader
+    //
+    // The encoder always uses the shortest header that fits.
+
+    /**
+     * A 1 byte header holds the length in the byte itself, so both ends of
+     * this range are the same numbers as the first byte's own range.
+     */
+    static constexpr int kMinValueOfLengthFor1ByteHeader = kMinValueOfFirstByteFor1ByteHeader;
+    static constexpr int kMaxValueOfLengthFor1ByteHeader = kMaxValueOfFirstByteFor1ByteHeader;
+
+    static constexpr int kMinValueOfLengthFor2ByteHeader = kMaxValueOfLengthFor1ByteHeader + 1;
+
+    /**
+     * 48 values of the first byte mean a 2 byte header, and each of them covers
+     * 256 lengths. The 48 is worked out from the two range ends above, so it
+     * stays right if either of them changes.
+     */
+    static constexpr int kMaxValueOfLengthFor2ByteHeader = kMinValueOfLengthFor2ByteHeader +
+        ((kMaxValueOfFirstByteFor2ByteHeader - kMaxValueOfFirstByteFor1ByteHeader) *
+         kNumberOfValuesInOneByte) -
+        1;
+
+    static constexpr int kMinValueOfLengthFor3ByteHeader = kMaxValueOfLengthFor2ByteHeader + 1;
+
+    /**
+     * 14 values of the first byte mean a 3 byte header, and each of them covers
+     * 65,536 lengths. Counted the same way, that gives the largest length any
+     * header can state.
+     *
+     * Nothing is accepted or rejected against this. The assertion below uses it
+     * to check that every length the encoder writes is one a header can state.
+     */
+    static constexpr int kMaxRepresentableLength = kMinValueOfLengthFor3ByteHeader +
+        ((kMaxValueOfFirstByteFor3ByteHeader - kMaxValueOfFirstByteFor2ByteHeader) *
+         kNumberOfValuesInTwoBytes) -
+        1;
+
+    /**
+     * The largest length the encoder will write. This is the one number here
+     * that is picked rather than worked out. The decoder accepts nothing above
+     * it, so both sides agree on the same set of lengths.
+     */
+    static constexpr int kMaxValueOfLengthFor3ByteHeader = 918744;
+
+    static_assert(
+        kMaxValueOfLengthFor3ByteHeader <= kMaxRepresentableLength,
+        "a length the encoder writes must be one a header can state");
+
     explicit Serializer(int n = 256)
     {
         data_.reserve(n);
@@ -61,7 +157,7 @@ public:
 
     // assemble functions
     int
-    add8(unsigned char i);
+    add8(unsigned char byteValue);
     int
     add16(std::uint16_t i);
 
@@ -270,18 +366,90 @@ public:
         return v.data_ == data_;
     }
 
+    /**
+     * Works out how long a header is, from its first byte.
+     *
+     * Each overload of decodeVLLength below reads one header length, so call
+     * this first to learn which of them to call.
+     *
+     * @param firstByte First byte of the header, as read from the stream.
+     * @return How many bytes the whole header takes, counting firstByte: 1, 2
+     * or 3.
+     * @throws std::overflow_error if firstByte is the one value that starts no
+     * header.
+     */
     static int
-    decodeLengthLength(int b1);
+    decodeLengthLength(std::byte firstByte);
+
+    /**
+     * Reads the field length out of a 1 byte header.
+     *
+     * @param firstByte The single header byte, which is the length itself.
+     * @return Field length in bytes, from kMinValueOfLengthFor1ByteHeader to
+     * kMaxValueOfLengthFor1ByteHeader.
+     * @throws std::overflow_error if firstByte is big enough to mean a longer
+     * header, in which case it is not a length by itself.
+     */
     static int
-    decodeVLLength(int b1);
+    decodeVLLength(std::byte firstByte);
+
+    /**
+     * Reads the field length out of a 2 byte header.
+     *
+     * @param firstByte First header byte. Its value means a 2 byte header, and
+     * how far it sits into that range gives the top part of the length.
+     * @param secondByte Second header byte, holding the rest of the length.
+     * @return Field length in bytes, from kMinValueOfLengthFor2ByteHeader to
+     * kMaxValueOfLengthFor2ByteHeader.
+     * @throws std::overflow_error if firstByte is outside the range that means
+     * a 2 byte header.
+     */
     static int
-    decodeVLLength(int b1, int b2);
+    decodeVLLength(std::byte firstByte, std::byte secondByte);
+
+    /**
+     * Reads the field length out of a 3 byte header.
+     *
+     * @param firstByte First header byte. Its value means a 3 byte header, and
+     * how far it sits into that range gives the top part of the length.
+     * @param secondByte Second header byte, holding the middle part of the
+     * length.
+     * @param thirdByte Third header byte, holding the low part.
+     * @return Field length in bytes, from kMinValueOfLengthFor3ByteHeader to
+     * kMaxValueOfLengthFor3ByteHeader.
+     * @throws std::overflow_error if firstByte is outside the range that means
+     * a 3 byte header, or if the three bytes together state a length above
+     * kMaxValueOfLengthFor3ByteHeader, which the encoder would not write back.
+     */
     static int
-    decodeVLLength(int b1, int b2, int b3);
+    decodeVLLength(std::byte firstByte, std::byte secondByte, std::byte thirdByte);
 
 private:
+    /**
+     * Works out how many bytes the header needs for the given length.
+     *
+     * This deliberately repeats the width choice addEncoded makes, so that
+     * addVL's assertion can compare the two. It has no other caller; do not
+     * reach for it as a utility.
+     *
+     * @param length Field length in bytes.
+     * @return How many header bytes it needs: 1, 2 or 3.
+     * @throws std::overflow_error if length is negative, or above
+     * kMaxValueOfLengthFor3ByteHeader.
+     */
     static int
-    encodeLengthLength(int length);  // length to encode length
+    encodeLengthLength(int length);
+
+    /**
+     * Appends the length header for a field of the given length.
+     *
+     * The field's own data is not written; the caller appends it next.
+     *
+     * @param length Field length in bytes.
+     * @return Offset within this Serializer at which the header was written.
+     * @throws std::overflow_error if length is negative, or above
+     * kMaxValueOfLengthFor3ByteHeader.
+     */
     int
     addEncoded(int length);
 };
@@ -390,9 +558,15 @@ public:
     void
     getFieldID(int& type, int& name);
 
-    // Returns the size of the VL if the
-    // next object is a VL. Advances the iterator
-    // to the beginning of the VL.
+    /**
+     * Reads the length header at the read position and steps past it.
+     *
+     * @return Field length in bytes. The iterator is left on the first byte of
+     * the field data.
+     * @throws std::overflow_error if the header states a length the encoder could
+     * not have written.
+     * @throws std::runtime_error if the data runs out before the header does.
+     */
     int
     getVLDataLength();
 

include/xrpl/protocol/detail/STVar.h

@@ -34,10 +34,11 @@ concept ValidConstructSTArgs =
 // and includes a small-object allocation optimization.
 class STVar
 {
-private:
+public:
     // The largest "small object" we can accommodate
     static constexpr std::size_t kMaxSize = 72;
 
+private:
     alignas(std::max_align_t) std::byte d_[kMaxSize] = {};
     STBase* p_ = nullptr;
 

include/xrpl/tx/ApplyContext.h

@@ -8,6 +8,7 @@
 #include <xrpl/ledger/ApplyViewImpl.h>
 #include <xrpl/ledger/OpenView.h>
 #include <xrpl/ledger/RawView.h>
+#include <xrpl/protocol/Book.h>
 #include <xrpl/protocol/STAmount.h>
 #include <xrpl/protocol/STTx.h>
 #include <xrpl/protocol/TER.h>
@@ -129,6 +130,14 @@ public:
         view_->rawDestroyXRP(fee);
     }
 
+    /**
+     * Registers a newly-created order book directory with the shared,
+     * process-wide OrderBookDB, unless this transaction is being applied
+     * under TapDryRun.
+     */
+    void
+    addOrderBook(Book const& book);
+
     ApplyViewContext
     getApplyViewContext()
     {

include/xrpl/tx/applySteps.h

@@ -12,6 +12,7 @@
 #include <xrpl/protocol/XRPAmount.h>
 
 #include <cstdint>
+#include <expected>
 #include <optional>
 #include <utility>
 
@@ -393,16 +394,21 @@ preclaim(PreflightResult const& preflightResult, ServiceRegistry& registry, Open
  *
  * No validation is done or implied by this function.
  *
- * Caller is responsible for handling any exceptions.
- * Since none should be thrown, that will usually
- * mean terminating.
- *
+ * Callers do not expect this function to throw; exceptions from a transactor's
+ * `calculateBaseFee` are caught and reported as an error instead.
  * @param view The current open ledger.
  * @param tx The transaction to be checked.
  *
- * @return The base fee.
+ * @return The base fee on success. Returns `std::unexpected(temUNKNOWN)` if the transaction
+ * type is not recognized, and `std::unexpected(tefEXCEPTION)` if the transactor's
+ * `calculateBaseFee` threw.
+ *
+ * @note Failure is reported as an error rather than a fee of zero because a
+ * zero (or default) fee would pass checkFee and let the transaction be
+ * applied for less than it owes. Callers that only need a fee hint may fall
+ * back to a default; callers deciding whether to apply should reject.
  */
-XRPAmount
+[[nodiscard]] std::expected<XRPAmount, TER>
 calculateBaseFee(ReadView const& view, STTx const& tx);
 
 /**

include/xrpl/tx/invariants/LoanInvariant.h

@@ -38,9 +38,11 @@ namespace xrpl {
  *    f. A Loan must reference a live `ltLOAN_BROKER`, and that broker must
  *       reference a live `ltVAULT`.
  *    g. Post-conditions for the Loan paid down by a successful `ttLOAN_PAY`:
- *       `PaymentRemaining > 0` after: `PrincipalOutstanding` and
- *          `PaymentRemaining` strictly decrease; `NextPaymentDueDate`
- *          advances by N * `PaymentInterval`, N > 0.
+ *       `PaymentRemaining > 0` after: neither `PrincipalOutstanding` nor
+ *          `TotalValueOutstanding` increases, and at least one of them
+ *          strictly decreases;
+ *          `PaymentRemaining` strictly decreases;
+ *          `NextPaymentDueDate` advances by N * `PaymentInterval`, N > 0.
  *       `PaymentRemaining == 0` after: pinned by checks 1 and 5b.
  *
  */