Merge branch 'develop' into bthomee/node_depth

Signed-off-by: JCW <a1q123456@users.noreply.github.com>
Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>
Co-authored-by: Copilot <copilot@github.com>

.github/scripts/levelization/results/ordering.txt

@@ -93,7 +93,6 @@ test.core > xrpl.basics
 test.core > xrpl.core
 test.core > xrpld.core
 test.core > xrpl.json
-test.core > xrpl.protocol
 test.core > xrpl.rdb
 test.core > xrpl.server
 test.csf > xrpl.basics
@@ -188,10 +187,16 @@ test.toplevel > xrpl.json
 test.unit_test > xrpl.basics
 test.unit_test > xrpl.protocol
 tests.libxrpl > xrpl.basics
+tests.libxrpl > xrpl.core
 tests.libxrpl > xrpl.json
+tests.libxrpl > xrpl.ledger
 tests.libxrpl > xrpl.net
+tests.libxrpl > xrpl.nodestore
 tests.libxrpl > xrpl.protocol
 tests.libxrpl > xrpl.protocol_autogen
+tests.libxrpl > xrpl.server
+tests.libxrpl > xrpl.shamap
+tests.libxrpl > xrpl.tx
 xrpl.conditions > xrpl.basics
 xrpl.conditions > xrpl.protocol
 xrpl.core > xrpl.basics

include/xrpl/basics/Slice.h

@@ -11,6 +11,7 @@
 #include <limits>
 #include <stdexcept>
 #include <string>
+#include <string_view>
 #include <type_traits>
 #include <vector>
 
@@ -231,4 +232,11 @@ makeSlice(std::basic_string<char, Traits, Alloc> const& s)
     return Slice(s.data(), s.size());
 }
 
+template <class Traits>
+Slice
+makeSlice(std::basic_string_view<char, Traits> s)
+{
+    return Slice(s.data(), s.size());
+}
+
 }  // namespace xrpl

include/xrpl/proto/xrpl.proto

@@ -246,7 +246,15 @@ message TMGetObjectByHash {
 
 message TMLedgerNode {
   required bytes nodedata = 1;
-  optional bytes nodeid = 2;  // missing for ledger base data
+
+  // Used when protocol version <2.3. Not set for ledger base data.
+  optional bytes nodeid = 2;
+
+  // Used when protocol version >=2.3. Neither value is set for ledger base data.
+  oneof reference {
+    bytes id = 3;      // Set for inner nodes.
+    uint32 depth = 4;  // Set for leaf nodes.
+  }
 }
 
 enum TMLedgerInfoType {

include/xrpl/protocol_autogen/LedgerEntryBase.h

@@ -130,6 +130,19 @@ public:
         return sle_->at(sfFlags);
     }
 
+    /**
+     * @brief Check if a specific flag is set.
+     *
+     * @param f The flag bitmask to check
+     * @return true if all bits in f are set in the flags field
+     */
+    [[nodiscard]]
+    bool
+    isFlag(std::uint32_t f) const
+    {
+        return sle_->isFlag(f);
+    }
+
     /**
      * @brief Get the underlying SLE object.
      *

include/xrpl/shamap/SHAMap.h

@@ -16,6 +16,7 @@
 
 #include <set>
 #include <stack>
+#include <tuple>
 #include <vector>
 
 namespace xrpl {
@@ -73,6 +74,22 @@ enum class SHAMapState {
 
     See https://en.wikipedia.org/wiki/Merkle_tree
  */
+
+/** Holds a SHAMap node's identity, serialized data, and leaf status.
+    Used by getNodeFat to return node data for peer synchronization.
+*/
+struct SHAMapNodeData
+{
+    SHAMapNodeID nodeID;
+    Blob data;
+    bool isLeaf;
+
+    SHAMapNodeData(SHAMapNodeID const& id, Blob d, bool leaf)
+        : nodeID(id), data(std::move(d)), isLeaf(leaf)
+    {
+    }
+};
+
 class SHAMap
 {
 private:
@@ -85,7 +102,7 @@ private:
     /** The sequence of the ledger that this map references, if any. */
     std::uint32_t ledgerSeq_ = 0;
 
-    intr_ptr::SharedPtr<SHAMapTreeNode> root_;
+    SHAMapTreeNodePtr root_;
     mutable SHAMapState state_;
     SHAMapType const type_;
     bool backed_ = true;         // Map is backed by the database
@@ -250,10 +267,10 @@ public:
     std::vector<std::pair<SHAMapNodeID, uint256>>
     getMissingNodes(int maxNodes, SHAMapSyncFilter* filter);
 
-    bool
+    [[nodiscard]] bool
     getNodeFat(
         SHAMapNodeID const& wanted,
-        std::vector<std::pair<SHAMapNodeID, Blob>>& data,
+        std::vector<SHAMapNodeData>& data,
         bool fatLeaves,
         std::uint32_t depth) const;
 
@@ -280,10 +297,42 @@ public:
     void
     serializeRoot(Serializer& s) const;
 
+    /** Add a root node to the SHAMap during synchronization.
+     *
+     * This function is used when receiving the root node of a SHAMap from a peer during ledger
+     * synchronization. The node must already have been deserialized.
+     *
+     * @param hash The expected hash of the root node.
+     * @param rootNode A deserialized root node to add.
+     * @param filter Optional sync filter to track received nodes.
+     * @return Status indicating whether the node was useful, duplicate, or invalid.
+     *
+     * @note This function expects the rootNode to be a valid, deserialized SHAMapTreeNode. The
+     *       caller is responsible for deserialization and basic validation before calling this
+     *       function.
+     */
     SHAMapAddNode
-    addRootNode(SHAMapHash const& hash, Slice const& rootNode, SHAMapSyncFilter* filter);
+    addRootNode(SHAMapHash const& hash, SHAMapTreeNodePtr rootNode, SHAMapSyncFilter const* filter);
+
+    /** Add a known node at a specific position in the SHAMap during synchronization.
+     *
+     * This function is used when receiving nodes from peers during ledger synchronization. The node
+     * is inserted at the position specified by nodeID. The node must already have been
+     * deserialized.
+     *
+     * @param nodeID The position in the tree where this node belongs.
+     * @param treeNode A deserialized tree node to add.
+     * @param filter Optional sync filter to track received nodes.
+     * @return Status indicating whether the node was useful, duplicate, or invalid.
+     *
+     * @note This function expects that the caller has already validated that the nodeID is
+     * consistent with the node's content.
+     */
     SHAMapAddNode
-    addKnownNode(SHAMapNodeID const& nodeID, Slice const& rawNode, SHAMapSyncFilter* filter);
+    addKnownNode(
+        SHAMapNodeID const& nodeID,
+        SHAMapTreeNodePtr treeNode,
+        SHAMapSyncFilter const* filter);
 
     // status functions
     void
@@ -326,36 +375,32 @@ public:
     invariants() const;
 
 private:
-    using SharedPtrNodeStack =
-        std::stack<std::pair<intr_ptr::SharedPtr<SHAMapTreeNode>, SHAMapNodeID>>;
+    using SharedPtrNodeStack = std::stack<std::pair<SHAMapTreeNodePtr, SHAMapNodeID>>;
     using DeltaRef =
         std::pair<boost::intrusive_ptr<SHAMapItem const>, boost::intrusive_ptr<SHAMapItem const>>;
 
     // tree node cache operations
-    intr_ptr::SharedPtr<SHAMapTreeNode>
+    SHAMapTreeNodePtr
     cacheLookup(SHAMapHash const& hash) const;
 
     void
-    canonicalize(SHAMapHash const& hash, intr_ptr::SharedPtr<SHAMapTreeNode>&) const;
+    canonicalize(SHAMapHash const& hash, SHAMapTreeNodePtr&) const;
 
     // database operations
-    intr_ptr::SharedPtr<SHAMapTreeNode>
+    SHAMapTreeNodePtr
     fetchNodeFromDB(SHAMapHash const& hash) const;
-    intr_ptr::SharedPtr<SHAMapTreeNode>
+    SHAMapTreeNodePtr
     fetchNodeNT(SHAMapHash const& hash) const;
-    intr_ptr::SharedPtr<SHAMapTreeNode>
-    fetchNodeNT(SHAMapHash const& hash, SHAMapSyncFilter* filter) const;
-    intr_ptr::SharedPtr<SHAMapTreeNode>
+    SHAMapTreeNodePtr
+    fetchNodeNT(SHAMapHash const& hash, SHAMapSyncFilter const* filter) const;
+    SHAMapTreeNodePtr
     fetchNode(SHAMapHash const& hash) const;
-    intr_ptr::SharedPtr<SHAMapTreeNode>
-    checkFilter(SHAMapHash const& hash, SHAMapSyncFilter* filter) const;
+    SHAMapTreeNodePtr
+    checkFilter(SHAMapHash const& hash, SHAMapSyncFilter const* filter) const;
 
     /** Update hashes up to the root */
     void
-    dirtyUp(
-        SharedPtrNodeStack& stack,
-        uint256 const& target,
-        intr_ptr::SharedPtr<SHAMapTreeNode> terminal);
+    dirtyUp(SharedPtrNodeStack& stack, uint256 const& target, SHAMapTreeNodePtr terminal);
 
     /** Walk towards the specified id, returning the node.  Caller must check
         if the return is nullptr, and if not, if the node->peekItem()->key() ==
@@ -377,25 +422,21 @@ private:
     preFlushNode(intr_ptr::SharedPtr<Node> node) const;
 
     /** write and canonicalize modified node */
-    intr_ptr::SharedPtr<SHAMapTreeNode>
-    writeNode(NodeObjectType t, intr_ptr::SharedPtr<SHAMapTreeNode> node) const;
+    SHAMapTreeNodePtr
+    writeNode(NodeObjectType t, SHAMapTreeNodePtr node) const;
 
     // returns the first item at or below this node
     SHAMapLeafNode*
-    firstBelow(intr_ptr::SharedPtr<SHAMapTreeNode>, SharedPtrNodeStack& stack, int branch = 0)
-        const;
+    firstBelow(SHAMapTreeNodePtr node, SharedPtrNodeStack& stack, int branch = 0) const;
 
     // returns the last item at or below this node
     SHAMapLeafNode*
-    lastBelow(
-        intr_ptr::SharedPtr<SHAMapTreeNode> node,
-        SharedPtrNodeStack& stack,
-        int branch = branchFactor) const;
+    lastBelow(SHAMapTreeNodePtr node, SharedPtrNodeStack& stack, int branch = branchFactor) const;
 
     // helper function for firstBelow and lastBelow
     SHAMapLeafNode*
     belowHelper(
-        intr_ptr::SharedPtr<SHAMapTreeNode> node,
+        SHAMapTreeNodePtr node,
         SharedPtrNodeStack& stack,
         int branch,
         std::tuple<int, std::function<bool(int)>, std::function<void(int&)>> const& loopParams)
@@ -407,20 +448,19 @@ private:
     descend(SHAMapInnerNode*, int branch) const;
     SHAMapTreeNode*
     descendThrow(SHAMapInnerNode*, int branch) const;
-    intr_ptr::SharedPtr<SHAMapTreeNode>
+    SHAMapTreeNodePtr
     descend(SHAMapInnerNode&, int branch) const;
-    intr_ptr::SharedPtr<SHAMapTreeNode>
+    SHAMapTreeNodePtr
     descendThrow(SHAMapInnerNode&, int branch) const;
 
     // Descend with filter
     // If pending, callback is called as if it called fetchNodeNT
-    using descendCallback =
-        std::function<void(intr_ptr::SharedPtr<SHAMapTreeNode>, SHAMapHash const&)>;
+    using descendCallback = std::function<void(SHAMapTreeNodePtr, SHAMapHash const&)>;
     SHAMapTreeNode*
     descendAsync(
         SHAMapInnerNode* parent,
         int branch,
-        SHAMapSyncFilter* filter,
+        SHAMapSyncFilter const* filter,
         bool& pending,
         descendCallback&&) const;
 
@@ -429,11 +469,11 @@ private:
         SHAMapInnerNode* parent,
         SHAMapNodeID const& parentID,
         int branch,
-        SHAMapSyncFilter* filter) const;
+        SHAMapSyncFilter const* filter) const;
 
     // Non-storing
     // Does not hook the returned node to its parent
-    intr_ptr::SharedPtr<SHAMapTreeNode>
+    SHAMapTreeNodePtr
     descendNoStore(SHAMapInnerNode&, int branch) const;
 
     /** If there is only one leaf below this node, get its contents */
@@ -495,10 +535,10 @@ private:
 
         // nodes we may have acquired from deferred reads
         using DeferredNode = std::tuple<
-            SHAMapInnerNode*,                      // parent node
-            SHAMapNodeID,                          // parent node ID
-            int,                                   // branch
-            intr_ptr::SharedPtr<SHAMapTreeNode>>;  // node
+            SHAMapInnerNode*,    // parent node
+            SHAMapNodeID,        // parent node ID
+            int,                 // branch
+            SHAMapTreeNodePtr>;  // node
 
         int deferred_;
         std::mutex deferLock_;
@@ -524,7 +564,7 @@ private:
     gmn_ProcessDeferredReads(MissingNodes&);
 
     // fetch from DB helper function
-    intr_ptr::SharedPtr<SHAMapTreeNode>
+    SHAMapTreeNodePtr
     finishFetch(SHAMapHash const& hash, std::shared_ptr<NodeObject> const& object) const;
 };
 

include/xrpl/shamap/SHAMapAccountStateLeafNode.h

@@ -27,7 +27,7 @@ public:
     {
     }
 
-    intr_ptr::SharedPtr<SHAMapTreeNode>
+    SHAMapTreeNodePtr
     clone(std::uint32_t cowid) const final
     {
         return intr_ptr::make_shared<SHAMapAccountStateLeafNode>(item_, cowid, hash_);

include/xrpl/shamap/SHAMapInnerNode.h

@@ -87,7 +87,7 @@ public:
     void
     partialDestructor() override;
 
-    intr_ptr::SharedPtr<SHAMapTreeNode>
+    SHAMapTreeNodePtr
     clone(std::uint32_t cowid) const override;
 
     SHAMapNodeType
@@ -121,19 +121,19 @@ public:
     getChildHash(int m) const;
 
     void
-    setChild(int m, intr_ptr::SharedPtr<SHAMapTreeNode> child);
+    setChild(int m, SHAMapTreeNodePtr child);
 
     void
-    shareChild(int m, intr_ptr::SharedPtr<SHAMapTreeNode> const& child);
+    shareChild(int m, SHAMapTreeNodePtr const& child);
 
     SHAMapTreeNode*
     getChildPointer(int branch);
 
-    intr_ptr::SharedPtr<SHAMapTreeNode>
+    SHAMapTreeNodePtr
     getChild(int branch);
 
-    intr_ptr::SharedPtr<SHAMapTreeNode>
-    canonicalizeChild(int branch, intr_ptr::SharedPtr<SHAMapTreeNode> node);
+    SHAMapTreeNodePtr
+    canonicalizeChild(int branch, SHAMapTreeNodePtr node);
 
     // sync functions
     bool
@@ -161,10 +161,10 @@ public:
     void
     invariants(bool is_root = false) const override;
 
-    static intr_ptr::SharedPtr<SHAMapTreeNode>
+    static SHAMapTreeNodePtr
     makeFullInner(Slice data, SHAMapHash const& hash, bool hashValid);
 
-    static intr_ptr::SharedPtr<SHAMapTreeNode>
+    static SHAMapTreeNodePtr
     makeCompressedInner(Slice data);
 };
 

include/xrpl/shamap/SHAMapTreeNode.h

@@ -166,4 +166,6 @@ private:
     makeTransactionWithMeta(Slice data, SHAMapHash const& hash, bool hashValid);
 };
 
+using SHAMapTreeNodePtr = intr_ptr::SharedPtr<SHAMapTreeNode>;
+
 }  // namespace xrpl

include/xrpl/shamap/SHAMapTxLeafNode.h

@@ -26,7 +26,7 @@ public:
     {
     }
 
-    intr_ptr::SharedPtr<SHAMapTreeNode>
+    SHAMapTreeNodePtr
     clone(std::uint32_t cowid) const final
     {
         return intr_ptr::make_shared<SHAMapTxLeafNode>(item_, cowid, hash_);

include/xrpl/shamap/SHAMapTxPlusMetaLeafNode.h

@@ -27,7 +27,7 @@ public:
     {
     }
 
-    intr_ptr::SharedPtr<SHAMapTreeNode>
+    SHAMapTreeNodePtr
     clone(std::uint32_t cowid) const override
     {
         return intr_ptr::make_shared<SHAMapTxPlusMetaLeafNode>(item_, cowid, hash_);

include/xrpl/shamap/TreeNodeCache.h

@@ -11,5 +11,5 @@ using TreeNodeCache = TaggedCache<
     SHAMapTreeNode,
     /*IsKeyCache*/ false,
     intr_ptr::SharedWeakUnionPtr<SHAMapTreeNode>,
-    intr_ptr::SharedPtr<SHAMapTreeNode>>;
+    SHAMapTreeNodePtr>;
 }  // namespace xrpl

include/xrpl/shamap/detail/TaggedPointer.h

@@ -148,7 +148,7 @@ public:
     /** Get the number of elements in each array and a pointer to the start
         of each array.
     */
-    [[nodiscard]] std::tuple<std::uint8_t, SHAMapHash*, intr_ptr::SharedPtr<SHAMapTreeNode>*>
+    [[nodiscard]] std::tuple<std::uint8_t, SHAMapHash*, SHAMapTreeNodePtr*>
     getHashesAndChildren() const;
 
     /** Get the `hashes` array */
@@ -156,7 +156,7 @@ public:
     getHashes() const;
 
     /** Get the `children` array */
-    [[nodiscard]] intr_ptr::SharedPtr<SHAMapTreeNode>*
+    [[nodiscard]] SHAMapTreeNodePtr*
     getChildren() const;
 
     /** Call the `f` callback for all 16 (branchFactor) branches - even if

include/xrpl/shamap/detail/TaggedPointer.ipp

@@ -25,8 +25,7 @@ static_assert(
 // Terminology: A chunk is the memory being allocated from a block. A block
 // contains multiple chunks. This is the terminology the boost documentation
 // uses. Pools use "Simple Segregated Storage" as their storage format.
-constexpr size_t elementSizeBytes =
-    (sizeof(SHAMapHash) + sizeof(intr_ptr::SharedPtr<SHAMapTreeNode>));
+constexpr size_t elementSizeBytes = (sizeof(SHAMapHash) + sizeof(SHAMapTreeNodePtr));
 
 constexpr size_t blockSizeBytes = kilobytes(512);
 
@@ -363,8 +362,7 @@ inline TaggedPointer::TaggedPointer(
                 // keep
                 new (&dstHashes[dstIndex]) SHAMapHash{srcHashes[srcIndex]};
 
-                new (&dstChildren[dstIndex])
-                    intr_ptr::SharedPtr<SHAMapTreeNode>{std::move(srcChildren[srcIndex])};
+                new (&dstChildren[dstIndex]) SHAMapTreeNodePtr{std::move(srcChildren[srcIndex])};
                 ++dstIndex;
                 ++srcIndex;
             }
@@ -375,7 +373,7 @@ inline TaggedPointer::TaggedPointer(
                 if (dstIsDense)
                 {
                     new (&dstHashes[dstIndex]) SHAMapHash{};
-                    new (&dstChildren[dstIndex]) intr_ptr::SharedPtr<SHAMapTreeNode>{};
+                    new (&dstChildren[dstIndex]) SHAMapTreeNodePtr{};
                     ++dstIndex;
                 }
             }
@@ -383,7 +381,7 @@ inline TaggedPointer::TaggedPointer(
             {
                 // add
                 new (&dstHashes[dstIndex]) SHAMapHash{};
-                new (&dstChildren[dstIndex]) intr_ptr::SharedPtr<SHAMapTreeNode>{};
+                new (&dstChildren[dstIndex]) SHAMapTreeNodePtr{};
                 ++dstIndex;
                 if (srcIsDense)
                 {
@@ -396,7 +394,7 @@ inline TaggedPointer::TaggedPointer(
                 if (dstIsDense)
                 {
                     new (&dstHashes[dstIndex]) SHAMapHash{};
-                    new (&dstChildren[dstIndex]) intr_ptr::SharedPtr<SHAMapTreeNode>{};
+                    new (&dstChildren[dstIndex]) SHAMapTreeNodePtr{};
                     ++dstIndex;
                 }
                 if (srcIsDense)
@@ -413,7 +411,7 @@ inline TaggedPointer::TaggedPointer(
         for (int i = dstIndex; i < dstNumAllocated; ++i)
         {
             new (&dstHashes[i]) SHAMapHash{};
-            new (&dstChildren[i]) intr_ptr::SharedPtr<SHAMapTreeNode>{};
+            new (&dstChildren[i]) SHAMapTreeNodePtr{};
         }
         *this = std::move(dst);
     }
@@ -433,7 +431,7 @@ inline TaggedPointer::TaggedPointer(
     // allocate hashes and children, but do not run constructors
     TaggedPointer newHashesAndChildren{RawAllocateTag{}, toAllocate};
     SHAMapHash *newHashes, *oldHashes;
-    intr_ptr::SharedPtr<SHAMapTreeNode>*newChildren, *oldChildren;
+    SHAMapTreeNodePtr *newChildren, *oldChildren;
     std::uint8_t newNumAllocated;
     // structured bindings can't be captured in c++ 17; use tie instead
     std::tie(newNumAllocated, newHashes, newChildren) = newHashesAndChildren.getHashesAndChildren();
@@ -444,8 +442,7 @@ inline TaggedPointer::TaggedPointer(
         // new arrays are dense, old arrays are sparse
         iterNonEmptyChildIndexes(isBranch, [&](auto branchNum, auto indexNum) {
             new (&newHashes[branchNum]) SHAMapHash{oldHashes[indexNum]};
-            new (&newChildren[branchNum])
-                intr_ptr::SharedPtr<SHAMapTreeNode>{std::move(oldChildren[indexNum])};
+            new (&newChildren[branchNum]) SHAMapTreeNodePtr{std::move(oldChildren[indexNum])};
         });
         // Run the constructors for the remaining elements
         for (int i = 0; i < SHAMapInnerNode::branchFactor; ++i)
@@ -453,7 +450,7 @@ inline TaggedPointer::TaggedPointer(
             if ((1 << i) & isBranch)
                 continue;
             new (&newHashes[i]) SHAMapHash{};
-            new (&newChildren[i]) intr_ptr::SharedPtr<SHAMapTreeNode>{};
+            new (&newChildren[i]) SHAMapTreeNodePtr{};
         }
     }
     else
@@ -463,14 +460,14 @@ inline TaggedPointer::TaggedPointer(
         iterNonEmptyChildIndexes(isBranch, [&](auto branchNum, auto indexNum) {
             new (&newHashes[curCompressedIndex]) SHAMapHash{oldHashes[indexNum]};
             new (&newChildren[curCompressedIndex])
-                intr_ptr::SharedPtr<SHAMapTreeNode>{std::move(oldChildren[indexNum])};
+                SHAMapTreeNodePtr{std::move(oldChildren[indexNum])};
             ++curCompressedIndex;
         });
         // Run the constructors for the remaining elements
         for (int i = curCompressedIndex; i < newNumAllocated; ++i)
         {
             new (&newHashes[i]) SHAMapHash{};
-            new (&newChildren[i]) intr_ptr::SharedPtr<SHAMapTreeNode>{};
+            new (&newChildren[i]) SHAMapTreeNodePtr{};
         }
     }
 
@@ -484,7 +481,7 @@ inline TaggedPointer::TaggedPointer(std::uint8_t numChildren)
     for (std::size_t i = 0; i < numAllocated; ++i)
     {
         new (&hashes[i]) SHAMapHash{};
-        new (&children[i]) intr_ptr::SharedPtr<SHAMapTreeNode>{};
+        new (&children[i]) SHAMapTreeNodePtr{};
     }
 }
 
@@ -522,14 +519,13 @@ TaggedPointer::isDense() const
     return (tp_ & tagMask) == boundaries.size() - 1;
 }
 
-[[nodiscard]] inline std::tuple<std::uint8_t, SHAMapHash*, intr_ptr::SharedPtr<SHAMapTreeNode>*>
+[[nodiscard]] inline std::tuple<std::uint8_t, SHAMapHash*, SHAMapTreeNodePtr*>
 TaggedPointer::getHashesAndChildren() const
 {
     auto const [tag, ptr] = decode();
     auto const hashes = reinterpret_cast<SHAMapHash*>(ptr);
     std::uint8_t numAllocated = boundaries[tag];
-    auto const children =
-        reinterpret_cast<intr_ptr::SharedPtr<SHAMapTreeNode>*>(hashes + numAllocated);
+    auto const children = reinterpret_cast<SHAMapTreeNodePtr*>(hashes + numAllocated);
     return {numAllocated, hashes, children};
 };
 
@@ -539,7 +535,7 @@ TaggedPointer::getHashes() const
     return reinterpret_cast<SHAMapHash*>(tp_ & ptrMask);
 };
 
-[[nodiscard]] inline intr_ptr::SharedPtr<SHAMapTreeNode>*
+[[nodiscard]] inline SHAMapTreeNodePtr*
 TaggedPointer::getChildren() const
 {
     auto [unused1, unused2, result] = getHashesAndChildren();

src/libxrpl/shamap/SHAMap.cpp

@@ -97,10 +97,7 @@ SHAMap::snapShot(bool isMutable) const
 }
 
 void
-SHAMap::dirtyUp(
-    SharedPtrNodeStack& stack,
-    uint256 const& target,
-    intr_ptr::SharedPtr<SHAMapTreeNode> child)
+SHAMap::dirtyUp(SharedPtrNodeStack& stack, uint256 const& target, SHAMapTreeNodePtr child)
 {
     // walk the tree up from through the inner nodes to the root_
     // update hashes and links
@@ -165,7 +162,7 @@ SHAMap::findKey(uint256 const& id) const
     return leaf;
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMap::fetchNodeFromDB(SHAMapHash const& hash) const
 {
     XRPL_ASSERT(backed_, "xrpl::SHAMap::fetchNodeFromDB : is backed");
@@ -173,7 +170,7 @@ SHAMap::fetchNodeFromDB(SHAMapHash const& hash) const
     return finishFetch(hash, obj);
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMap::finishFetch(SHAMapHash const& hash, std::shared_ptr<NodeObject> const& object) const
 {
     XRPL_ASSERT(backed_, "xrpl::SHAMap::finishFetch : is backed");
@@ -208,8 +205,8 @@ SHAMap::finishFetch(SHAMapHash const& hash, std::shared_ptr<NodeObject> const& o
 }
 
 // See if a sync filter has a node
-intr_ptr::SharedPtr<SHAMapTreeNode>
-SHAMap::checkFilter(SHAMapHash const& hash, SHAMapSyncFilter* filter) const
+SHAMapTreeNodePtr
+SHAMap::checkFilter(SHAMapHash const& hash, SHAMapSyncFilter const* filter) const
 {
     if (auto nodeData = filter->getNode(hash))
     {
@@ -234,8 +231,8 @@ SHAMap::checkFilter(SHAMapHash const& hash, SHAMapSyncFilter* filter) const
 
 // Get a node without throwing
 // Used on maps where missing nodes are expected
-intr_ptr::SharedPtr<SHAMapTreeNode>
-SHAMap::fetchNodeNT(SHAMapHash const& hash, SHAMapSyncFilter* filter) const
+SHAMapTreeNodePtr
+SHAMap::fetchNodeNT(SHAMapHash const& hash, SHAMapSyncFilter const* filter) const
 {
     auto node = cacheLookup(hash);
     if (node)
@@ -257,7 +254,7 @@ SHAMap::fetchNodeNT(SHAMapHash const& hash, SHAMapSyncFilter* filter) const
     return node;
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMap::fetchNodeNT(SHAMapHash const& hash) const
 {
     auto node = cacheLookup(hash);
@@ -269,7 +266,7 @@ SHAMap::fetchNodeNT(SHAMapHash const& hash) const
 }
 
 // Throw if the node is missing
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMap::fetchNode(SHAMapHash const& hash) const
 {
     auto node = fetchNodeNT(hash);
@@ -291,10 +288,10 @@ SHAMap::descendThrow(SHAMapInnerNode* parent, int branch) const
     return ret;
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMap::descendThrow(SHAMapInnerNode& parent, int branch) const
 {
-    intr_ptr::SharedPtr<SHAMapTreeNode> ret = descend(parent, branch);
+    SHAMapTreeNodePtr ret = descend(parent, branch);
 
     if (!ret && !parent.isEmptyBranch(branch))
         Throw<SHAMapMissingNode>(type_, parent.getChildHash(branch));
@@ -309,7 +306,7 @@ SHAMap::descend(SHAMapInnerNode* parent, int branch) const
     if ((ret != nullptr) || !backed_)
         return ret;
 
-    intr_ptr::SharedPtr<SHAMapTreeNode> node = fetchNodeNT(parent->getChildHash(branch));
+    SHAMapTreeNodePtr node = fetchNodeNT(parent->getChildHash(branch));
     if (!node)
         return nullptr;
 
@@ -317,10 +314,10 @@ SHAMap::descend(SHAMapInnerNode* parent, int branch) const
     return node.get();
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMap::descend(SHAMapInnerNode& parent, int branch) const
 {
-    intr_ptr::SharedPtr<SHAMapTreeNode> node = parent.getChild(branch);
+    SHAMapTreeNodePtr node = parent.getChild(branch);
     if (node || !backed_)
         return node;
 
@@ -334,10 +331,10 @@ SHAMap::descend(SHAMapInnerNode& parent, int branch) const
 
 // Gets the node that would be hooked to this branch,
 // but doesn't hook it up.
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMap::descendNoStore(SHAMapInnerNode& parent, int branch) const
 {
-    intr_ptr::SharedPtr<SHAMapTreeNode> ret = parent.getChild(branch);
+    SHAMapTreeNodePtr ret = parent.getChild(branch);
     if (!ret && backed_)
         ret = fetchNode(parent.getChildHash(branch));
     return ret;
@@ -348,7 +345,7 @@ SHAMap::descend(
     SHAMapInnerNode* parent,
     SHAMapNodeID const& parentID,
     int branch,
-    SHAMapSyncFilter* filter) const
+    SHAMapSyncFilter const* filter) const
 {
     XRPL_ASSERT(parent->isInner(), "xrpl::SHAMap::descend : valid parent input");
     XRPL_ASSERT(
@@ -361,7 +358,7 @@ SHAMap::descend(
     if (child == nullptr)
     {
         auto const& childHash = parent->getChildHash(branch);
-        intr_ptr::SharedPtr<SHAMapTreeNode> childNode = fetchNodeNT(childHash, filter);
+        SHAMapTreeNodePtr childNode = fetchNodeNT(childHash, filter);
 
         if (childNode)
         {
@@ -377,7 +374,7 @@ SHAMapTreeNode*
 SHAMap::descendAsync(
     SHAMapInnerNode* parent,
     int branch,
-    SHAMapSyncFilter* filter,
+    SHAMapSyncFilter const* filter,
     bool& pending,
     descendCallback&& callback) const
 {
@@ -434,7 +431,7 @@ SHAMap::unshareNode(intr_ptr::SharedPtr<Node> node, SHAMapNodeID const& nodeID)
 
 SHAMapLeafNode*
 SHAMap::belowHelper(
-    intr_ptr::SharedPtr<SHAMapTreeNode> node,
+    SHAMapTreeNodePtr node,
     SharedPtrNodeStack& stack,
     int branch,
     std::tuple<int, std::function<bool(int)>, std::function<void(int&)>> const& loopParams) const
@@ -479,8 +476,7 @@ SHAMap::belowHelper(
     return nullptr;
 }
 SHAMapLeafNode*
-SHAMap::lastBelow(intr_ptr::SharedPtr<SHAMapTreeNode> node, SharedPtrNodeStack& stack, int branch)
-    const
+SHAMap::lastBelow(SHAMapTreeNodePtr node, SharedPtrNodeStack& stack, int branch) const
 {
     auto init = branchFactor - 1;
     auto cmp = [](int i) { return i >= 0; };
@@ -489,8 +485,7 @@ SHAMap::lastBelow(intr_ptr::SharedPtr<SHAMapTreeNode> node, SharedPtrNodeStack&
     return belowHelper(node, stack, branch, {init, cmp, incr});
 }
 SHAMapLeafNode*
-SHAMap::firstBelow(intr_ptr::SharedPtr<SHAMapTreeNode> node, SharedPtrNodeStack& stack, int branch)
-    const
+SHAMap::firstBelow(SHAMapTreeNodePtr node, SharedPtrNodeStack& stack, int branch) const
 {
     auto init = 0;
     auto cmp = [](int i) { return i <= branchFactor; };
@@ -699,10 +694,8 @@ SHAMap::delItem(uint256 const& id)
 
     SHAMapNodeType const type = leaf->getType();
 
-    using TreeNodeType = intr_ptr::SharedPtr<SHAMapTreeNode>;
-
     // What gets attached to the end of the chain (For now, nothing, since we deleted the leaf)
-    TreeNodeType prevNode;
+    SHAMapTreeNodePtr prevNode;
 
     while (!stack.empty())
     {
@@ -728,7 +721,7 @@ SHAMap::delItem(uint256 const& id)
                 // no children below this branch
                 //
                 // Note: This is unnecessary due to the std::move above but left here for safety
-                prevNode = TreeNodeType{};
+                prevNode = SHAMapTreeNodePtr{};
             }
             else if (bc == 1)
             {
@@ -741,7 +734,7 @@ SHAMap::delItem(uint256 const& id)
                     {
                         if (!node->isEmptyBranch(i))
                         {
-                            node->setChild(i, TreeNodeType{});
+                            node->setChild(i, SHAMapTreeNodePtr{});
                             break;
                         }
                     }
@@ -937,8 +930,8 @@ SHAMap::fetchRoot(SHAMapHash const& hash, SHAMapSyncFilter* filter)
     @note The node must have already been unshared by having the caller
           first call SHAMapTreeNode::unshare().
  */
-intr_ptr::SharedPtr<SHAMapTreeNode>
-SHAMap::writeNode(NodeObjectType t, intr_ptr::SharedPtr<SHAMapTreeNode> node) const
+SHAMapTreeNodePtr
+SHAMap::writeNode(NodeObjectType t, SHAMapTreeNodePtr node) const
 {
     XRPL_ASSERT(node->cowid() == 0, "xrpl::SHAMap::writeNode : valid input node");
     XRPL_ASSERT(backed_, "xrpl::SHAMap::writeNode : is backed");
@@ -1155,7 +1148,7 @@ SHAMap::dump(bool hash) const
     JLOG(journal_.info()) << leafCount << " resident leaves";
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMap::cacheLookup(SHAMapHash const& hash) const
 {
     auto ret = f_.getTreeNodeCache()->fetch(hash.as_uint256());
@@ -1164,7 +1157,7 @@ SHAMap::cacheLookup(SHAMapHash const& hash) const
 }
 
 void
-SHAMap::canonicalize(SHAMapHash const& hash, intr_ptr::SharedPtr<SHAMapTreeNode>& node) const
+SHAMap::canonicalize(SHAMapHash const& hash, SHAMapTreeNodePtr& node) const
 {
     XRPL_ASSERT(backed_, "xrpl::SHAMap::canonicalize : is backed");
     XRPL_ASSERT(node->cowid() == 0, "xrpl::SHAMap::canonicalize : valid node input");

src/libxrpl/shamap/SHAMapDelta.cpp

@@ -261,7 +261,7 @@ SHAMap::walkMap(std::vector<SHAMapMissingNode>& missingNodes, int maxMissing) co
         {
             if (!node->isEmptyBranch(i))
             {
-                intr_ptr::SharedPtr<SHAMapTreeNode> const nextNode = descendNoStore(*node, i);
+                SHAMapTreeNodePtr const nextNode = descendNoStore(*node, i);
 
                 if (nextNode)
                 {
@@ -286,7 +286,7 @@ SHAMap::walkMapParallel(std::vector<SHAMapMissingNode>& missingNodes, int maxMis
         return false;
 
     using StackEntry = intr_ptr::SharedPtr<SHAMapInnerNode>;
-    std::array<intr_ptr::SharedPtr<SHAMapTreeNode>, 16> topChildren;
+    std::array<SHAMapTreeNodePtr, 16> topChildren;
     {
         auto const& innerRoot = intr_ptr::static_pointer_cast<SHAMapInnerNode>(root_);
         for (int i = 0; i < 16; ++i)
@@ -331,8 +331,7 @@ SHAMap::walkMapParallel(std::vector<SHAMapMissingNode>& missingNodes, int maxMis
                         {
                             if (node->isEmptyBranch(i))
                                 continue;
-                            intr_ptr::SharedPtr<SHAMapTreeNode> const nextNode =
-                                descendNoStore(*node, i);
+                            SHAMapTreeNodePtr const nextNode = descendNoStore(*node, i);
 
                             if (nextNode)
                             {

src/libxrpl/shamap/SHAMapInnerNode.cpp

@@ -37,7 +37,7 @@ SHAMapInnerNode::~SHAMapInnerNode() = default;
 void
 SHAMapInnerNode::partialDestructor()
 {
-    intr_ptr::SharedPtr<SHAMapTreeNode>* children = nullptr;
+    SHAMapTreeNodePtr* children = nullptr;
     // structured bindings can't be captured in c++ 17; use tie instead
     std::tie(std::ignore, std::ignore, children) = hashesAndChildren_.getHashesAndChildren();
     iterNonEmptyChildIndexes([&](auto branchNum, auto indexNum) { children[indexNum].reset(); });
@@ -69,7 +69,7 @@ SHAMapInnerNode::getChildIndex(int i) const
     return hashesAndChildren_.getChildIndex(isBranch_, i);
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMapInnerNode::clone(std::uint32_t cowid) const
 {
     auto const branchCount = getBranchCount();
@@ -79,7 +79,7 @@ SHAMapInnerNode::clone(std::uint32_t cowid) const
     p->isBranch_ = isBranch_;
     p->fullBelowGen_ = fullBelowGen_;
     SHAMapHash *cloneHashes = nullptr, *thisHashes = nullptr;
-    intr_ptr::SharedPtr<SHAMapTreeNode>*cloneChildren = nullptr, *thisChildren = nullptr;
+    SHAMapTreeNodePtr *cloneChildren = nullptr, *thisChildren = nullptr;
     // structured bindings can't be captured in c++ 17; use tie instead
     std::tie(std::ignore, cloneHashes, cloneChildren) =
         p->hashesAndChildren_.getHashesAndChildren();
@@ -118,7 +118,7 @@ SHAMapInnerNode::clone(std::uint32_t cowid) const
     return p;
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMapInnerNode::makeFullInner(Slice data, SHAMapHash const& hash, bool hashValid)
 {
     // A full inner node is serialized as 16 256-bit hashes, back to back:
@@ -153,7 +153,7 @@ SHAMapInnerNode::makeFullInner(Slice data, SHAMapHash const& hash, bool hashVali
     return ret;
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMapInnerNode::makeCompressedInner(Slice data)
 {
     // A compressed inner node is serialized as a series of 33 byte chunks,
@@ -207,7 +207,7 @@ void
 SHAMapInnerNode::updateHashDeep()
 {
     SHAMapHash* hashes = nullptr;
-    intr_ptr::SharedPtr<SHAMapTreeNode>* children = nullptr;
+    SHAMapTreeNodePtr* children = nullptr;
     // structured bindings can't be captured in c++ 17; use tie instead
     std::tie(std::ignore, hashes, children) = hashesAndChildren_.getHashesAndChildren();
     iterNonEmptyChildIndexes([&](auto branchNum, auto indexNum) {
@@ -265,7 +265,7 @@ SHAMapInnerNode::getString(SHAMapNodeID const& id) const
 
 // We are modifying an inner node
 void
-SHAMapInnerNode::setChild(int m, intr_ptr::SharedPtr<SHAMapTreeNode> child)
+SHAMapInnerNode::setChild(int m, SHAMapTreeNodePtr child)
 {
     XRPL_ASSERT(
         (m >= 0) && (m < branchFactor), "xrpl::SHAMapInnerNode::setChild : valid branch input");
@@ -307,7 +307,7 @@ SHAMapInnerNode::setChild(int m, intr_ptr::SharedPtr<SHAMapTreeNode> child)
 
 // finished modifying, now make shareable
 void
-SHAMapInnerNode::shareChild(int m, intr_ptr::SharedPtr<SHAMapTreeNode> const& child)
+SHAMapInnerNode::shareChild(int m, SHAMapTreeNodePtr const& child)
 {
     XRPL_ASSERT(
         (m >= 0) && (m < branchFactor), "xrpl::SHAMapInnerNode::shareChild : valid branch input");
@@ -337,7 +337,7 @@ SHAMapInnerNode::getChildPointer(int branch)
     return hashesAndChildren_.getChildren()[index].get();
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMapInnerNode::getChild(int branch)
 {
     XRPL_ASSERT(
@@ -364,8 +364,8 @@ SHAMapInnerNode::getChildHash(int m) const
     return zeroSHAMapHash;
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
-SHAMapInnerNode::canonicalizeChild(int branch, intr_ptr::SharedPtr<SHAMapTreeNode> node)
+SHAMapTreeNodePtr
+SHAMapInnerNode::canonicalizeChild(int branch, SHAMapTreeNodePtr node)
 {
     XRPL_ASSERT(
         branch >= 0 && branch < branchFactor,

src/libxrpl/shamap/SHAMapNodeID.cpp

@@ -129,7 +129,9 @@ selectBranch(SHAMapNodeID const& id, uint256 const& hash)
 SHAMapNodeID
 SHAMapNodeID::createID(int depth, uint256 const& key)
 {
-    XRPL_ASSERT((depth >= 0) && (depth < 65), "xrpl::SHAMapNodeID::createID : valid branch input");
+    XRPL_ASSERT(
+        depth >= 0 && depth <= SHAMap::leafDepth,
+        "xrpl::SHAMapNodeID::createID : valid branch input");
     return SHAMapNodeID(depth, key & depthMask(depth));
 }
 

src/libxrpl/shamap/SHAMapSync.cpp

@@ -66,7 +66,7 @@ SHAMap::visitNodes(std::function<bool(SHAMapTreeNode&)> const& function) const
         {
             if (!node->isEmptyBranch(pos))
             {
-                intr_ptr::SharedPtr<SHAMapTreeNode> const child = descendNoStore(*node, pos);
+                SHAMapTreeNodePtr const child = descendNoStore(*node, pos);
                 if (!function(*child))
                     return;
 
@@ -204,8 +204,7 @@ SHAMap::gmn_ProcessNodes(MissingNodes& mn, MissingNodes::StackEntry& se)
                 branch,
                 mn.filter_,
                 pending,
-                [node, nodeID, branch, &mn](
-                    intr_ptr::SharedPtr<SHAMapTreeNode> found, SHAMapHash const&) {
+                [node, nodeID, branch, &mn](SHAMapTreeNodePtr found, SHAMapHash const&) {
                     // a read completed asynchronously
                     std::unique_lock<std::mutex> const lock{mn.deferLock_};
                     mn.finishedReads_.emplace_back(node, nodeID, branch, std::move(found));
@@ -268,8 +267,7 @@ SHAMap::gmn_ProcessDeferredReads(MissingNodes& mn)
     int complete = 0;
     while (complete != mn.deferred_)
     {
-        std::tuple<SHAMapInnerNode*, SHAMapNodeID, int, intr_ptr::SharedPtr<SHAMapTreeNode>>
-            deferredNode;
+        std::tuple<SHAMapInnerNode*, SHAMapNodeID, int, SHAMapTreeNodePtr> deferredNode;
         {
             std::unique_lock<std::mutex> lock{mn.deferLock_};
 
@@ -417,7 +415,7 @@ SHAMap::getMissingNodes(int max, SHAMapSyncFilter* filter)
 bool
 SHAMap::getNodeFat(
     SHAMapNodeID const& wanted,
-    std::vector<std::pair<SHAMapNodeID, Blob>>& data,
+    std::vector<SHAMapNodeData>& data,
     bool fatLeaves,
     std::uint32_t depth) const
 {
@@ -463,7 +461,7 @@ SHAMap::getNodeFat(
         // Add this node to the reply
         s.erase();
         node->serializeForWire(s);
-        data.emplace_back(nodeID, s.getData());
+        data.emplace_back(nodeID, s.getData(), node->isLeaf());
 
         if (node->isInner())
         {
@@ -493,7 +491,7 @@ SHAMap::getNodeFat(
                             // Just include this node
                             s.erase();
                             childNode->serializeForWire(s);
-                            data.emplace_back(childID, s.getData());
+                            data.emplace_back(childID, s.getData(), childNode->isLeaf());
                         }
                     }
                 }
@@ -511,8 +509,18 @@ SHAMap::serializeRoot(Serializer& s) const
 }
 
 SHAMapAddNode
-SHAMap::addRootNode(SHAMapHash const& hash, Slice const& rootNode, SHAMapSyncFilter* filter)
+SHAMap::addRootNode(
+    SHAMapHash const& hash,
+    SHAMapTreeNodePtr rootNode,
+    SHAMapSyncFilter const* filter)
 {
+    XRPL_ASSERT(rootNode, "xrpl::SHAMap::addRootNode : non-null root node");
+    if (!rootNode)
+    {
+        JLOG(journal_.error()) << "Null node received";
+        return SHAMapAddNode::invalid();
+    }
+
     // we already have a root_ node
     if (root_->getHash().isNonZero())
     {
@@ -522,14 +530,16 @@ SHAMap::addRootNode(SHAMapHash const& hash, Slice const& rootNode, SHAMapSyncFil
     }
 
     XRPL_ASSERT(cowid_ >= 1, "xrpl::SHAMap::addRootNode : valid cowid");
-    auto node = SHAMapTreeNode::makeFromWire(rootNode);
-    if (!node || node->getHash() != hash)
+    if (rootNode->getHash() != hash)
+    {
+        JLOG(journal_.warn()) << "Corrupt node received";
         return SHAMapAddNode::invalid();
+    }
 
     if (backed_)
-        canonicalize(hash, node);
+        canonicalize(hash, rootNode);
 
-    root_ = node;
+    root_ = std::move(rootNode);
 
     if (root_->isLeaf())
         clearSynching();
@@ -546,9 +556,23 @@ SHAMap::addRootNode(SHAMapHash const& hash, Slice const& rootNode, SHAMapSyncFil
 }
 
 SHAMapAddNode
-SHAMap::addKnownNode(SHAMapNodeID const& node, Slice const& rawNode, SHAMapSyncFilter* filter)
+SHAMap::addKnownNode(
+    SHAMapNodeID const& nodeID,
+    SHAMapTreeNodePtr treeNode,
+    SHAMapSyncFilter const* filter)
 {
-    XRPL_ASSERT(!node.isRoot(), "xrpl::SHAMap::addKnownNode : valid node input");
+    XRPL_ASSERT(!nodeID.isRoot(), "xrpl::SHAMap::addKnownNode : valid node input");
+    if (nodeID.isRoot())
+    {
+        JLOG(journal_.error()) << "Root node received";
+        return SHAMapAddNode::invalid();
+    }
+    XRPL_ASSERT(treeNode, "xrpl::SHAMap::addKnownNode : non-null tree node");
+    if (!treeNode)
+    {
+        JLOG(journal_.error()) << "Null node received";
+        return SHAMapAddNode::invalid();
+    }
 
     if (!isSynching())
     {
@@ -562,14 +586,14 @@ SHAMap::addKnownNode(SHAMapNodeID const& node, Slice const& rawNode, SHAMapSyncF
 
     while (currNode->isInner() &&
            !safe_downcast<SHAMapInnerNode*>(currNode)->isFullBelow(generation) &&
-           (currNodeID.getDepth() < node.getDepth()))
+           (currNodeID.getDepth() < nodeID.getDepth()))
     {
-        int const branch = selectBranch(currNodeID, node.getNodeID());
+        int const branch = selectBranch(currNodeID, nodeID.getNodeID());
         XRPL_ASSERT(branch >= 0, "xrpl::SHAMap::addKnownNode : valid branch");
         auto inner = safe_downcast<SHAMapInnerNode*>(currNode);
         if (inner->isEmptyBranch(branch))
         {
-            JLOG(journal_.warn()) << "Add known node for empty branch" << node;
+            JLOG(journal_.warn()) << "Add known node for empty branch" << nodeID;
             return SHAMapAddNode::invalid();
         }
 
@@ -585,67 +609,44 @@ SHAMap::addKnownNode(SHAMapNodeID const& node, Slice const& rawNode, SHAMapSyncF
         if (currNode != nullptr)
             continue;
 
-        auto newNode = SHAMapTreeNode::makeFromWire(rawNode);
-
-        if (!newNode || childHash != newNode->getHash())
+        if (childHash != treeNode->getHash())
         {
             JLOG(journal_.warn()) << "Corrupt node received";
             return SHAMapAddNode::invalid();
         }
 
-        // In rare cases, a node can still be corrupt even after hash
-        // validation. For leaf nodes, we perform an additional check to
-        // ensure the node's position in the tree is consistent with its
-        // content to prevent inconsistencies that could
-        // propagate further down the line.
-        if (newNode->isLeaf())
-        {
-            auto const& actualKey =
-                safe_downcast<SHAMapLeafNode const*>(newNode.get())->peekItem()->key();
-
-            // Validate that this leaf belongs at the target position
-            auto const expectedNodeID = SHAMapNodeID::createID(node.getDepth(), actualKey);
-            if (expectedNodeID.getNodeID() != node.getNodeID())
-            {
-                JLOG(journal_.debug())
-                    << "Leaf node position mismatch: "
-                    << "expected=" << expectedNodeID.getNodeID() << ", actual=" << node.getNodeID();
-                return SHAMapAddNode::invalid();
-            }
-        }
-
         // Inner nodes must be at a level strictly less than 64
         // but leaf nodes (while notionally at level 64) can be
         // at any depth up to and including 64:
         if ((currNodeID.getDepth() > leafDepth) ||
-            (newNode->isInner() && currNodeID.getDepth() == leafDepth))
+            (treeNode->isInner() && currNodeID.getDepth() == leafDepth))
         {
             // Map is provably invalid
             state_ = SHAMapState::Invalid;
             return SHAMapAddNode::useful();
         }
 
-        if (currNodeID != node)
+        if (currNodeID != nodeID)
         {
             // Either this node is broken or we didn't request it (yet)
-            JLOG(journal_.warn()) << "unable to hook node " << node;
+            JLOG(journal_.warn()) << "unable to hook node " << nodeID;
             JLOG(journal_.info()) << " stuck at " << currNodeID;
-            JLOG(journal_.info()) << "got depth=" << node.getDepth()
+            JLOG(journal_.info()) << "got depth=" << nodeID.getDepth()
                                   << ", walked to= " << currNodeID.getDepth();
             return SHAMapAddNode::useful();
         }
 
         if (backed_)
-            canonicalize(childHash, newNode);
+            canonicalize(childHash, treeNode);
 
-        newNode = prevNode->canonicalizeChild(branch, std::move(newNode));
+        treeNode = prevNode->canonicalizeChild(branch, std::move(treeNode));
 
         if (filter != nullptr)
         {
             Serializer s;
-            newNode->serializeWithPrefix(s);
+            treeNode->serializeWithPrefix(s);
             filter->gotNode(
-                false, childHash, ledgerSeq_, std::move(s.modData()), newNode->getType());
+                false, childHash, ledgerSeq_, std::move(s.modData()), treeNode->getType());
         }
 
         return SHAMapAddNode::useful();

src/libxrpl/shamap/SHAMapTreeNode.cpp

@@ -25,7 +25,7 @@
 
 namespace xrpl {
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMapTreeNode::makeTransaction(Slice data, SHAMapHash const& hash, bool hashValid)
 {
     auto item = make_shamapitem(sha512Half(HashPrefix::transactionID, data), data);
@@ -36,7 +36,7 @@ SHAMapTreeNode::makeTransaction(Slice data, SHAMapHash const& hash, bool hashVal
     return intr_ptr::make_shared<SHAMapTxLeafNode>(std::move(item), 0);
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMapTreeNode::makeTransactionWithMeta(Slice data, SHAMapHash const& hash, bool hashValid)
 {
     Serializer s(data.data(), data.size());
@@ -60,7 +60,7 @@ SHAMapTreeNode::makeTransactionWithMeta(Slice data, SHAMapHash const& hash, bool
     return intr_ptr::make_shared<SHAMapTxPlusMetaLeafNode>(std::move(item), 0);
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMapTreeNode::makeAccountState(Slice data, SHAMapHash const& hash, bool hashValid)
 {
     Serializer s(data.data(), data.size());
@@ -87,7 +87,7 @@ SHAMapTreeNode::makeAccountState(Slice data, SHAMapHash const& hash, bool hashVa
     return intr_ptr::make_shared<SHAMapAccountStateLeafNode>(std::move(item), 0);
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMapTreeNode::makeFromWire(Slice rawNode)
 {
     if (rawNode.empty())
@@ -118,7 +118,7 @@ SHAMapTreeNode::makeFromWire(Slice rawNode)
     Throw<std::runtime_error>("wire: Unknown type (" + std::to_string(type) + ")");
 }
 
-intr_ptr::SharedPtr<SHAMapTreeNode>
+SHAMapTreeNodePtr
 SHAMapTreeNode::makeFromPrefix(Slice rawNode, SHAMapHash const& hash)
 {
     if (rawNode.size() < 4)

src/test/app/LedgerNodeHelpers_test.cpp

@@ -0,0 +1,350 @@
+#include <xrpld/app/ledger/detail/LedgerNodeHelpers.h>
+
+#include <xrpl/basics/IntrusivePointer.h>
+#include <xrpl/basics/base_uint.h>
+#include <xrpl/beast/unit_test/suite.h>
+#include <xrpl/protocol/Serializer.h>
+#include <xrpl/shamap/SHAMap.h>
+#include <xrpl/shamap/SHAMapAccountStateLeafNode.h>
+#include <xrpl/shamap/SHAMapInnerNode.h>
+#include <xrpl/shamap/SHAMapItem.h>
+#include <xrpl/shamap/SHAMapTreeNode.h>
+
+#include <boost/smart_ptr/intrusive_ptr.hpp>
+
+#include <xrpl.pb.h>
+
+#include <bit>
+#include <cstdint>
+#include <string>
+
+namespace xrpl::tests {
+
+class LedgerNodeHelpers_test : public beast::unit_test::suite
+{
+    static boost::intrusive_ptr<SHAMapItem>
+    makeTestItem(std::uint32_t seed)
+    {
+        Serializer s;
+        s.add32(seed);
+        s.add32(seed + 1);
+        s.add32(seed + 2);
+        return make_shamapitem(s.getSHA512Half(), s.slice());
+    }
+
+    static std::string
+    serializeNode(SHAMapTreeNodePtr const& node)
+    {
+        Serializer s;
+        node->serializeForWire(s);
+        auto const slice = s.slice();
+        return std::string(std::bit_cast<char const*>(slice.data()), slice.size());
+    }
+
+    void
+    testValidateLedgerNode()
+    {
+        // In the tests below the validity of the content of the node data and ID fields is not
+        // checked - only that the fields have values when expected. The content of the fields is
+        // verified in the other tests in this file.
+        testcase("validateLedgerNode");
+
+        // Invalid: missing all fields.
+        {
+            protocol::TMLedgerNode const node;
+            BEAST_EXPECT(!validateLedgerNode(node));
+        }
+
+        // Invalid: missing `nodedata` field.
+        {
+            protocol::TMLedgerNode node;
+            node.set_nodeid("test_nodeid");
+            BEAST_EXPECT(!validateLedgerNode(node));
+        }
+
+        // Invalid: missing `nodedata` field.
+        {
+            protocol::TMLedgerNode node;
+            node.set_id("test_nodeid");
+            BEAST_EXPECT(!validateLedgerNode(node));
+        }
+
+        // Invalid: missing `nodedata` field.
+        {
+            protocol::TMLedgerNode node;
+            node.set_depth(1);
+            BEAST_EXPECT(!validateLedgerNode(node));
+        }
+
+        // Valid: legacy `nodeid` field.
+        {
+            protocol::TMLedgerNode node;
+            node.set_nodedata("test_data");
+            node.set_nodeid("test_nodeid");
+            BEAST_EXPECT(validateLedgerNode(node));
+        }
+
+        // Invalid: has both legacy `nodeid` and new `id` fields.
+        {
+            protocol::TMLedgerNode node;
+            node.set_nodedata("test_data");
+            node.set_nodeid("test_nodeid");
+            node.set_id("test_nodeid");
+            BEAST_EXPECT(!validateLedgerNode(node));
+        }
+
+        // Invalid: has both legacy `nodeid` and new `depth` fields.
+        {
+            protocol::TMLedgerNode node;
+            node.set_nodedata("test_data");
+            node.set_nodeid("test_nodeid");
+            node.set_depth(5);
+            BEAST_EXPECT(!validateLedgerNode(node));
+        }
+
+        // Valid: new `id` field.
+        {
+            protocol::TMLedgerNode node;
+            node.set_nodedata("test_data");
+            node.set_id("test_id");
+            BEAST_EXPECT(validateLedgerNode(node));
+        }
+
+        // Valid: new `depth` field.
+        {
+            protocol::TMLedgerNode node;
+            node.set_nodedata("test_data");
+            node.set_depth(5);
+            BEAST_EXPECT(validateLedgerNode(node));
+        }
+
+        // Valid: `depth` at minimum depth.
+        {
+            protocol::TMLedgerNode node;
+            node.set_nodedata("test_data");
+            node.set_depth(0);
+            BEAST_EXPECT(validateLedgerNode(node));
+        }
+
+        // Valid: `depth` at arbitrary depth between minimum and maximum.
+        {
+            protocol::TMLedgerNode node;
+            node.set_nodedata("test_data");
+            node.set_depth(10);
+            BEAST_EXPECT(validateLedgerNode(node));
+        }
+
+        // Valid: `depth` at maximum depth.
+        {
+            protocol::TMLedgerNode node;
+            node.set_nodedata("test_data");
+            node.set_depth(SHAMap::leafDepth);
+            BEAST_EXPECT(validateLedgerNode(node));
+        }
+
+        // Invalid: `depth` is greater than maximum depth.
+        {
+            protocol::TMLedgerNode node;
+            node.set_nodedata("test_data");
+            node.set_depth(SHAMap::leafDepth + 1);
+            BEAST_EXPECT(!validateLedgerNode(node));
+        }
+    }
+
+    void
+    testGetTreeNode()
+    {
+        testcase("getTreeNode");
+
+        // Valid: inner node. It must have at least one child for `serializeNode` to work.
+        {
+            auto const innerNode = intr_ptr::make_shared<SHAMapInnerNode>(1);
+            auto const childNode = intr_ptr::make_shared<SHAMapInnerNode>(1);
+            innerNode->setChild(0, childNode);
+            auto const innerData = serializeNode(innerNode);
+            auto const result = getTreeNode(innerData);
+            BEAST_EXPECT(result && result->isInner());
+        }
+
+        // Valid: leaf node.
+        {
+            auto const leafItem = makeTestItem(12345);
+            auto const leafNode = intr_ptr::make_shared<SHAMapAccountStateLeafNode>(leafItem, 1);
+            auto const leafData = serializeNode(leafNode);
+            auto const result = getTreeNode(leafData);
+            BEAST_EXPECT(result && result->isLeaf());
+        }
+
+        // Invalid: empty data.
+        {
+            auto const result = getTreeNode("");
+            BEAST_EXPECT(!result);
+        }
+
+        // Invalid: garbage data.
+        {
+            auto const result = getTreeNode("invalid");
+            BEAST_EXPECT(!result);
+        }
+
+        // Invalid: truncated data.
+        {
+            auto const leafItem = makeTestItem(54321);
+            auto const leafNode = intr_ptr::make_shared<SHAMapAccountStateLeafNode>(leafItem, 1);
+            // Truncate the data to trigger an exception in SHAMapTreeNode::makeAccountState when
+            // the data is used to deserialize the node.
+            uint256 const tag;
+            auto const leafData = serializeNode(leafNode).substr(0, tag.bytes - 1);
+            auto const result = getTreeNode(leafData);
+            BEAST_EXPECT(!result);
+        }
+    }
+
+    void
+    testGetSHAMapNodeID()
+    {
+        testcase("getSHAMapNodeID");
+
+        {
+            // Tests using inner nodes at various depths.
+            auto const innerNode = intr_ptr::make_shared<SHAMapInnerNode>(1);
+            auto const childNode = intr_ptr::make_shared<SHAMapInnerNode>(1);
+            innerNode->setChild(0, childNode);
+            auto const innerData = serializeNode(innerNode);
+
+            // Valid: legacy `nodeid` field at arbitrary depth.
+            {
+                auto const innerDepth = 3;
+                auto const innerID = SHAMapNodeID::createID(innerDepth, uint256{});
+
+                protocol::TMLedgerNode node;
+                node.set_nodedata(innerData);
+                node.set_nodeid(innerID.getRawString());
+                auto const result = getSHAMapNodeID(node, innerNode);
+                BEAST_EXPECT(result == innerID);
+            }
+
+            // Valid: new `id` field at minimum depth.
+            {
+                auto const innerDepth = 0;
+                auto const innerID = SHAMapNodeID::createID(innerDepth, uint256{});
+
+                protocol::TMLedgerNode node;
+                node.set_nodedata(innerData);
+                node.set_id(innerID.getRawString());
+                auto const result = getSHAMapNodeID(node, innerNode);
+                BEAST_EXPECT(result == innerID);
+            }
+
+            // Invalid: new `depth` field should not be used for inner nodes.
+            {
+                protocol::TMLedgerNode node;
+                node.set_nodedata(innerData);
+                node.set_depth(10);
+                auto const result = getSHAMapNodeID(node, innerNode);
+                BEAST_EXPECT(!result);
+            }
+        }
+
+        {
+            // Tests using leaf nodes at various depths.
+            auto const leafItem = makeTestItem(12345);
+            auto const leafNode = intr_ptr::make_shared<SHAMapAccountStateLeafNode>(leafItem, 1);
+            auto const leafData = serializeNode(leafNode);
+            auto const leafKey = leafItem->key();
+
+            // Valid: legacy `nodeid` field at arbitrary depth.
+            {
+                auto const leafDepth = 5;
+                auto const leafID = SHAMapNodeID::createID(leafDepth, leafKey);
+
+                protocol::TMLedgerNode ledgerNode;
+                ledgerNode.set_nodedata(leafData);
+                ledgerNode.set_nodeid(leafID.getRawString());
+                auto const result = getSHAMapNodeID(ledgerNode, leafNode);
+                BEAST_EXPECT(result == leafID);
+            }
+
+            // Invalid: new `id` field should not be used for leaf nodes.
+            {
+                auto const leafDepth = 5;
+                auto const leafID = SHAMapNodeID::createID(leafDepth, leafKey);
+
+                protocol::TMLedgerNode ledgerNode;
+                ledgerNode.set_nodedata(leafData);
+                ledgerNode.set_id(leafID.getRawString());
+                auto const result = getSHAMapNodeID(ledgerNode, leafNode);
+                BEAST_EXPECT(!result);
+            }
+
+            // Valid: new `depth` field at minimum depth.
+            {
+                auto const leafDepth = 0;
+                auto const leafID = SHAMapNodeID::createID(leafDepth, leafKey);
+
+                protocol::TMLedgerNode node;
+                node.set_nodedata(leafData);
+                node.set_depth(leafDepth);
+                auto const result = getSHAMapNodeID(node, leafNode);
+                BEAST_EXPECT(result == leafID);
+            }
+
+            // Valid: new `depth` field at arbitrary depth between minimum and maximum.
+            {
+                auto const leafDepth = 10;
+                auto const leafID = SHAMapNodeID::createID(leafDepth, leafKey);
+
+                protocol::TMLedgerNode ledgerNode;
+                ledgerNode.set_nodedata(leafData);
+                ledgerNode.set_depth(leafDepth);
+                auto const result = getSHAMapNodeID(ledgerNode, leafNode);
+                BEAST_EXPECT(result == leafID);
+            }
+
+            // Valid: new `depth` field at maximum depth.
+            // Note that we do not test a depth greater than the maximum depth, because the proto
+            // message is assumed to have been validated by the time the getSHAMapNodeID function is
+            // called.
+            {
+                auto const leafDepth = SHAMap::leafDepth;
+                auto const leafID = SHAMapNodeID::createID(leafDepth, leafKey);
+
+                protocol::TMLedgerNode node;
+                node.set_nodedata(leafData);
+                node.set_depth(leafDepth);
+                auto const result = getSHAMapNodeID(node, leafNode);
+                BEAST_EXPECT(result == leafID);
+            }
+
+            // Invalid: legacy `nodeid` field where the node ID is inconsistent with the key.
+            {
+                auto const otherItem = makeTestItem(54321);
+                auto const otherNode =
+                    intr_ptr::make_shared<SHAMapAccountStateLeafNode>(otherItem, 1);
+                auto const otherData = serializeNode(otherNode);
+                auto const otherKey = otherItem->key();
+                auto const otherDepth = 1;
+                auto const otherID = SHAMapNodeID::createID(otherDepth, otherKey);
+
+                protocol::TMLedgerNode ledgerNode;
+                ledgerNode.set_nodedata(otherData);
+                ledgerNode.set_nodeid(otherID.getRawString());
+                auto const result = getSHAMapNodeID(ledgerNode, leafNode);
+                BEAST_EXPECT(!result);
+            }
+        }
+    }
+
+public:
+    void
+    run() override
+    {
+        testValidateLedgerNode();
+        testGetTreeNode();
+        testGetSHAMapNodeID();
+    }
+};
+
+BEAST_DEFINE_TESTSUITE(LedgerNodeHelpers, app, xrpl);
+
+}  // namespace xrpl::tests

src/test/core/Config_test.cpp

@@ -7,7 +7,6 @@
 #include <xrpl/basics/BasicConfig.h>
 #include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/beast/utility/temp_dir.h>
-#include <xrpl/protocol/SystemParameters.h>  // IWYU pragma: keep
 #include <xrpl/server/Port.h>
 
 #include <boost/filesystem/operations.hpp>

src/test/overlay/ProtocolVersion_test.cpp

@@ -63,8 +63,8 @@ public:
                 negotiateProtocolVersion("RTXP/1.2, XRPL/2.0, XRPL/2.1") == make_protocol(2, 1));
             BEAST_EXPECT(negotiateProtocolVersion("XRPL/2.2") == make_protocol(2, 2));
             BEAST_EXPECT(
-                negotiateProtocolVersion("RTXP/1.2, XRPL/2.2, XRPL/2.3, XRPL/999.999") ==
-                make_protocol(2, 2));
+                negotiateProtocolVersion("RTXP/1.2, XRPL/2.3, XRPL/2.4, XRPL/999.999") ==
+                make_protocol(2, 3));
             BEAST_EXPECT(negotiateProtocolVersion("XRPL/999.999, WebSocket/1.0") == std::nullopt);
             BEAST_EXPECT(negotiateProtocolVersion("") == std::nullopt);
         }

src/test/shamap/SHAMapSync_test.cpp

@@ -1,7 +1,6 @@
 #include <test/shamap/common.h>
 #include <test/unit_test/SuiteJournal.h>
 
-#include <xrpl/basics/Blob.h>
 #include <xrpl/basics/SHAMapHash.h>
 #include <xrpl/basics/Slice.h>
 #include <xrpl/basics/base_uint.h>
@@ -115,14 +114,17 @@ public:
         destination.setSynching();
 
         {
-            std::vector<std::pair<SHAMapNodeID, Blob>> a;
+            std::vector<SHAMapNodeData> a;
 
             BEAST_EXPECT(source.getNodeFat(SHAMapNodeID(), a, rand_bool(eng_), rand_int(eng_, 2)));
 
             unexpected(a.empty(), "NodeSize");
 
-            BEAST_EXPECT(destination.addRootNode(source.getHash(), makeSlice(a[0].second), nullptr)
-                             .isGood());
+            auto node = SHAMapTreeNode::makeFromWire(makeSlice(a[0].data));
+            if (!node)
+                fail("", __FILE__, __LINE__);
+            BEAST_EXPECT(
+                destination.addRootNode(source.getHash(), std::move(node), nullptr).isGood());
         }
 
         do
@@ -136,7 +138,7 @@ public:
                 break;
 
             // get as many nodes as possible based on this information
-            std::vector<std::pair<SHAMapNodeID, Blob>> b;
+            std::vector<SHAMapNodeData> b;
 
             for (auto& it : nodesMissing)
             {
@@ -158,8 +160,10 @@ public:
                 // Don't use BEAST_EXPECT here b/c it will be called a
                 // non-deterministic number of times and the number of tests run
                 // should be deterministic
-                if (!destination.addKnownNode(b[i].first, makeSlice(b[i].second), nullptr)
-                         .isUseful())
+                auto node = SHAMapTreeNode::makeFromWire(makeSlice(b[i].data));
+                if (!node)
+                    fail("", __FILE__, __LINE__);
+                if (!destination.addKnownNode(b[i].nodeID, std::move(node), nullptr).isUseful())
                     fail("", __FILE__, __LINE__);
             }
         } while (true);

src/tests/libxrpl/CMakeLists.txt

@@ -8,9 +8,12 @@ add_custom_target(xrpl.tests)
 
 # Test helpers
 add_library(xrpl.helpers.test STATIC)
-target_sources(xrpl.helpers.test PRIVATE helpers/TestSink.cpp)
+target_sources(
+    xrpl.helpers.test
+    PRIVATE helpers/Account.cpp helpers/TestSink.cpp helpers/TxTest.cpp
+)
 target_include_directories(xrpl.helpers.test PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})
-target_link_libraries(xrpl.helpers.test PRIVATE xrpl.libxrpl)
+target_link_libraries(xrpl.helpers.test PUBLIC xrpl.libxrpl gtest::gtest)
 
 # Common library dependencies for the rest of the tests.
 add_library(xrpl.imports.test INTERFACE)
@@ -32,6 +35,10 @@ xrpl_add_test(json)
 target_link_libraries(xrpl.test.json PRIVATE xrpl.imports.test)
 add_dependencies(xrpl.tests xrpl.test.json)
 
+xrpl_add_test(tx)
+target_link_libraries(xrpl.test.tx PRIVATE xrpl.imports.test)
+add_dependencies(xrpl.tests xrpl.test.tx)
+
 xrpl_add_test(protocol_autogen)
 target_link_libraries(xrpl.test.protocol_autogen PRIVATE xrpl.imports.test)
 add_dependencies(xrpl.tests xrpl.test.protocol_autogen)

src/tests/libxrpl/helpers/Account.cpp

@@ -0,0 +1,19 @@
+#include <helpers/Account.h>
+
+#include <xrpl/protocol/KeyType.h>
+#include <xrpl/protocol/PublicKey.h>
+#include <xrpl/protocol/SecretKey.h>
+#include <xrpl/protocol/Seed.h>
+
+namespace xrpl::test {
+
+Account const Account::master{"masterpassphrase"};
+
+Account::Account(std::string_view name, KeyType type)
+    : name_(name)
+    , keyPair_(generateKeyPair(type, generateSeed(name_)))
+    , id_(calcAccountID(keyPair_.first))
+{
+}
+
+}  // namespace xrpl::test

src/tests/libxrpl/helpers/Account.h

@@ -0,0 +1,81 @@
+#pragma once
+
+#include <xrpl/protocol/AccountID.h>
+#include <xrpl/protocol/KeyType.h>
+#include <xrpl/protocol/PublicKey.h>
+#include <xrpl/protocol/SecretKey.h>
+
+#include <string>
+#include <string_view>
+#include <utility>
+
+namespace xrpl::test {
+
+/**
+ * @brief A test account with cryptographic keys.
+ *
+ * Generates keys deterministically from a name, making tests reproducible.
+ * The same name always produces the same AccountID and keys.
+ */
+class Account
+{
+public:
+    /**
+     * @brief The master account that holds all XRP in genesis.
+     *
+     * This account is created in the genesis ledger with all 100 billion XRP.
+     * It uses the well-known seed "masterpassphrase".
+     */
+    static Account const master;
+
+    /**
+     * @brief Create an account from a name.
+     *
+     * Keys are derived deterministically from the name.
+     *
+     * @param name Human-readable name for the account.
+     * @param type Key type to use (defaults to secp256k1).
+     */
+    explicit Account(std::string_view name, KeyType type = KeyType::secp256k1);
+
+    /** @brief Return the human-readable name. */
+    std::string const&
+    name() const noexcept
+    {
+        return name_;
+    }
+
+    /** @brief Return the AccountID. */
+    AccountID const&
+    id() const noexcept
+    {
+        return id_;
+    }
+
+    /** @brief Return the public key. */
+    PublicKey const&
+    pk() const noexcept
+    {
+        return keyPair_.first;
+    }
+
+    /** @brief Return the secret key. */
+    SecretKey const&
+    sk() const noexcept
+    {
+        return keyPair_.second;
+    }
+
+    /** @brief Implicit conversion to AccountID. */
+    operator AccountID const&() const noexcept
+    {
+        return id_;
+    }
+
+private:
+    std::string name_;
+    std::pair<PublicKey, SecretKey> keyPair_;
+    AccountID id_;
+};
+
+}  // namespace xrpl::test

src/tests/libxrpl/helpers/IOU.h

@@ -0,0 +1,132 @@
+#pragma once
+
+#include <xrpl/basics/Number.h>
+#include <xrpl/protocol/Asset.h>
+#include <xrpl/protocol/Issue.h>
+#include <xrpl/protocol/STAmount.h>
+#include <xrpl/protocol/UintTypes.h>
+
+#include <helpers/Account.h>
+
+#include <concepts>
+#include <string>
+#include <string_view>
+#include <type_traits>
+
+namespace xrpl::test {
+
+/**
+ * @brief Represents an IOU (issued currency) for testing.
+ *
+ * Provides a clear, explicit API for creating currencies issued by an account.
+ * This replaces the cryptic `Account::operator[]` from the jtx framework.
+ *
+ * @code
+ *     Account gw("gateway");
+ *     IOU USD("USD", gw);
+ *
+ *     auto issue = USD.issue();      // Get the Issue
+ *     auto asset = USD.asset();      // Get the Asset
+ *     auto amt = USD.amount(100);    // Get STAmount of 100 USD
+ * @endcode
+ */
+class IOU
+{
+public:
+    /**
+     * @brief Construct an IOU from a currency code and issuing account.
+     * @param currencyCode A 3-character ISO currency code (e.g., "USD").
+     * @param issuer The account that issues this currency.
+     */
+    IOU(std::string_view currencyCode, Account const& issuer)
+        : currency_(to_currency(std::string(currencyCode))), issuer_(issuer.id())
+    {
+        XRPL_ASSERT(!isXRP(currency_), "IOU: currency code must not resolve to XRP");
+    }
+
+    /**
+     * @brief Construct an IOU from a Currency and issuing account.
+     * @param currency The Currency object.
+     * @param issuer The account that issues this currency.
+     */
+    IOU(Currency currency, Account const& issuer)
+        : currency_(std::move(currency)), issuer_(issuer.id())
+    {
+        XRPL_ASSERT(!isXRP(currency_), "IOU: currency code must not resolve to XRP");
+    }
+
+    /**
+     * @brief Get the Issue (currency + issuer pair).
+     * @return An Issue object representing this IOU.
+     */
+    [[nodiscard]] Issue
+    issue() const
+    {
+        return Issue{currency_, issuer_};
+    }
+
+    /**
+     * @brief Get the Asset.
+     * @return An Asset object representing this IOU.
+     */
+    [[nodiscard]] Asset
+    asset() const
+    {
+        return Asset{issue()};
+    }
+
+    /**
+     * @brief Create an STAmount of this IOU.
+     *
+     * Works with any arithmetic type (int, double, etc.) by converting
+     * to string and parsing. This matches the jtx IOU behaviour.
+     *
+     * @tparam T An arithmetic type.
+     * @param value The amount as any arithmetic type.
+     * @return An STAmount representing value units of this IOU.
+     */
+    template <typename T>
+        requires std::is_arithmetic_v<T>
+    [[nodiscard]] STAmount
+    amount(T value) const
+    {
+        return amountFromString(issue(), to_string(value));
+    }
+
+    /**
+     * @brief Create an STAmount of this IOU from a Number.
+     * @param value The amount as a Number.
+     * @return An STAmount representing value units of this IOU.
+     */
+    [[nodiscard]] STAmount
+    amount(Number const& value) const
+    {
+        return STAmount{issue(), value};
+    }
+
+    /**
+     * @brief Get the currency.
+     * @return The currency.
+     */
+    [[nodiscard]] Currency const&
+    currency() const
+    {
+        return currency_;
+    }
+
+    /**
+     * @brief Get the issuer account ID.
+     * @return The issuer's AccountID.
+     */
+    [[nodiscard]] AccountID const&
+    issuer() const
+    {
+        return issuer_;
+    }
+
+private:
+    Currency currency_;
+    AccountID issuer_;
+};
+
+}  // namespace xrpl::test

src/tests/libxrpl/helpers/TestFamily.h

@@ -0,0 +1,111 @@
+#pragma once
+
+#include <xrpl/basics/chrono.h>
+#include <xrpl/nodestore/DummyScheduler.h>
+#include <xrpl/nodestore/Manager.h>
+#include <xrpl/shamap/Family.h>
+
+#include <memory>
+
+namespace xrpl {
+namespace test {
+
+/** Test implementation of Family for unit tests.
+
+    Uses an in-memory NodeStore database and simple caches.
+    The missingNode methods throw since tests shouldn't encounter missing nodes.
+*/
+class TestFamily : public Family
+{
+private:
+    std::unique_ptr<NodeStore::Database> db_;
+    TestStopwatch clock_;
+    std::shared_ptr<FullBelowCache> fbCache_;
+    std::shared_ptr<TreeNodeCache> tnCache_;
+    NodeStore::DummyScheduler scheduler_;
+    beast::Journal j_;
+
+public:
+    explicit TestFamily(beast::Journal j)
+        : fbCache_(std::make_shared<FullBelowCache>("TestFamily full below cache", clock_, j))
+        , tnCache_(
+              std::make_shared<TreeNodeCache>(
+                  "TestFamily tree node cache",
+                  65536,
+                  std::chrono::minutes{1},
+                  clock_,
+                  j))
+        , j_(j)
+    {
+        Section config;
+        config.set("type", "memory");
+        config.set("path", "TestFamily");
+        db_ = NodeStore::Manager::instance().make_Database(megabytes(4), scheduler_, 1, config, j);
+    }
+
+    NodeStore::Database&
+    db() override
+    {
+        return *db_;
+    }
+
+    NodeStore::Database const&
+    db() const override
+    {
+        return *db_;
+    }
+
+    beast::Journal const&
+    journal() override
+    {
+        return j_;
+    }
+
+    std::shared_ptr<FullBelowCache>
+    getFullBelowCache() override
+    {
+        return fbCache_;
+    }
+
+    std::shared_ptr<TreeNodeCache>
+    getTreeNodeCache() override
+    {
+        return tnCache_;
+    }
+
+    void
+    sweep() override
+    {
+        fbCache_->sweep();
+        tnCache_->sweep();
+    }
+
+    void
+    missingNodeAcquireBySeq(std::uint32_t refNum, uint256 const& nodeHash) override
+    {
+        Throw<std::runtime_error>("TestFamily: missing node (by seq)");
+    }
+
+    void
+    missingNodeAcquireByHash(uint256 const& refHash, std::uint32_t refNum) override
+    {
+        Throw<std::runtime_error>("TestFamily: missing node (by hash)");
+    }
+
+    void
+    reset() override
+    {
+        fbCache_->reset();
+        tnCache_->reset();
+    }
+
+    /** Access the test clock for time manipulation in tests. */
+    TestStopwatch&
+    clock()
+    {
+        return clock_;
+    }
+};
+
+}  // namespace test
+}  // namespace xrpl

src/tests/libxrpl/helpers/TestServiceRegistry.h

@@ -0,0 +1,378 @@
+#pragma once
+
+#include <xrpl/basics/Log.h>
+#include <xrpl/basics/chrono.h>
+#include <xrpl/core/HashRouter.h>
+#include <xrpl/core/NetworkIDService.h>
+#include <xrpl/core/ServiceRegistry.h>
+#include <xrpl/ledger/PendingSaves.h>
+#include <xrpl/server/LoadFeeTrack.h>
+
+#include <boost/asio/io_context.hpp>
+
+#include <helpers/TestFamily.h>
+#include <helpers/TestSink.h>
+
+#include <optional>
+#include <stdexcept>
+
+namespace xrpl {
+namespace test {
+
+/** Logs implementation that creates TestSink instances. */
+class TestLogs : public Logs
+{
+public:
+    explicit TestLogs(beast::severities::Severity level = beast::severities::kWarning) : Logs(level)
+    {
+    }
+
+    std::unique_ptr<beast::Journal::Sink>
+    makeSink(std::string const&, beast::severities::Severity threshold) override
+    {
+        return std::make_unique<TestSink>(threshold);
+    }
+};
+
+/** Simple NetworkIDService implementation for tests. */
+class TestNetworkIDService final : public NetworkIDService
+{
+public:
+    explicit TestNetworkIDService(std::uint32_t networkID = 0) : networkID_(networkID)
+    {
+    }
+
+    [[nodiscard]] std::uint32_t
+    getNetworkID() const noexcept override
+    {
+        return networkID_;
+    }
+
+private:
+    std::uint32_t networkID_;
+};
+
+/** Test implementation of ServiceRegistry for unit tests.
+
+    This class provides real implementations for services that can be
+    instantiated from libxrpl (such as Logs, io_context, caches), and
+    throws std::logic_error for services that require the full Application.
+
+    Tests can subclass this to provide additional services they need.
+*/
+class TestServiceRegistry : public ServiceRegistry
+{
+    TestLogs logs_{beast::severities::kWarning};
+    boost::asio::io_context io_context_;
+    TestFamily family_{logs_.journal("TestFamily")};
+    LoadFeeTrack feeTrack_{logs_.journal("LoadFeeTrack")};
+    TestNetworkIDService networkIDService_;
+    HashRouter hashRouter_{HashRouter::Setup{}, stopwatch()};
+    NodeCache tempNodeCache_{
+        "TempNodeCache",
+        16384,
+        std::chrono::minutes{1},
+        stopwatch(),
+        logs_.journal("TaggedCache")};
+    CachedSLEs cachedSLEs_{
+        "CachedSLEs",
+        16384,
+        std::chrono::minutes{1},
+        stopwatch(),
+        logs_.journal("TaggedCache")};
+    PendingSaves pendingSaves_;
+    std::optional<uint256> trapTxID_;
+
+public:
+    TestServiceRegistry() = default;
+    ~TestServiceRegistry() override = default;
+
+    // Core infrastructure services
+    CollectorManager&
+    getCollectorManager() override
+    {
+        throw std::logic_error("TestServiceRegistry::getCollectorManager() not implemented");
+    }
+
+    Family&
+    getNodeFamily() override
+    {
+        return family_;
+    }
+
+    TimeKeeper&
+    getTimeKeeper() override
+    {
+        throw std::logic_error("TestServiceRegistry::timeKeeper() not implemented");
+    }
+
+    JobQueue&
+    getJobQueue() override
+    {
+        throw std::logic_error("TestServiceRegistry::getJobQueue() not implemented");
+    }
+
+    NodeCache&
+    getTempNodeCache() override
+    {
+        return tempNodeCache_;
+    }
+
+    CachedSLEs&
+    getCachedSLEs() override
+    {
+        return cachedSLEs_;
+    }
+
+    NetworkIDService&
+    getNetworkIDService() override
+    {
+        return networkIDService_;
+    }
+
+    // Protocol and validation services
+    AmendmentTable&
+    getAmendmentTable() override
+    {
+        throw std::logic_error("TestServiceRegistry::getAmendmentTable() not implemented");
+    }
+
+    HashRouter&
+    getHashRouter() override
+    {
+        return hashRouter_;
+    }
+
+    LoadFeeTrack&
+    getFeeTrack() override
+    {
+        return feeTrack_;
+    }
+
+    LoadManager&
+    getLoadManager() override
+    {
+        throw std::logic_error("TestServiceRegistry::getLoadManager() not implemented");
+    }
+
+    RCLValidations&
+    getValidations() override
+    {
+        throw std::logic_error("TestServiceRegistry::getValidations() not implemented");
+    }
+
+    ValidatorList&
+    getValidators() override
+    {
+        throw std::logic_error("TestServiceRegistry::validators() not implemented");
+    }
+
+    ValidatorSite&
+    getValidatorSites() override
+    {
+        throw std::logic_error("TestServiceRegistry::validatorSites() not implemented");
+    }
+
+    ManifestCache&
+    getValidatorManifests() override
+    {
+        throw std::logic_error("TestServiceRegistry::validatorManifests() not implemented");
+    }
+
+    ManifestCache&
+    getPublisherManifests() override
+    {
+        throw std::logic_error("TestServiceRegistry::publisherManifests() not implemented");
+    }
+
+    // Network services
+    Overlay&
+    getOverlay() override
+    {
+        throw std::logic_error("TestServiceRegistry::overlay() not implemented");
+    }
+
+    Cluster&
+    getCluster() override
+    {
+        throw std::logic_error("TestServiceRegistry::cluster() not implemented");
+    }
+
+    PeerReservationTable&
+    getPeerReservations() override
+    {
+        throw std::logic_error("TestServiceRegistry::peerReservations() not implemented");
+    }
+
+    Resource::Manager&
+    getResourceManager() override
+    {
+        throw std::logic_error("TestServiceRegistry::getResourceManager() not implemented");
+    }
+
+    // Storage services
+    NodeStore::Database&
+    getNodeStore() override
+    {
+        throw std::logic_error("TestServiceRegistry::getNodeStore() not implemented");
+    }
+
+    SHAMapStore&
+    getSHAMapStore() override
+    {
+        throw std::logic_error("TestServiceRegistry::getSHAMapStore() not implemented");
+    }
+
+    RelationalDatabase&
+    getRelationalDatabase() override
+    {
+        throw std::logic_error("TestServiceRegistry::getRelationalDatabase() not implemented");
+    }
+
+    // Ledger services
+    InboundLedgers&
+    getInboundLedgers() override
+    {
+        throw std::logic_error("TestServiceRegistry::getInboundLedgers() not implemented");
+    }
+
+    InboundTransactions&
+    getInboundTransactions() override
+    {
+        throw std::logic_error("TestServiceRegistry::getInboundTransactions() not implemented");
+    }
+
+    TaggedCache<uint256, AcceptedLedger>&
+    getAcceptedLedgerCache() override
+    {
+        throw std::logic_error("TestServiceRegistry::getAcceptedLedgerCache() not implemented");
+    }
+
+    LedgerMaster&
+    getLedgerMaster() override
+    {
+        throw std::logic_error("TestServiceRegistry::getLedgerMaster() not implemented");
+    }
+
+    LedgerCleaner&
+    getLedgerCleaner() override
+    {
+        throw std::logic_error("TestServiceRegistry::getLedgerCleaner() not implemented");
+    }
+
+    LedgerReplayer&
+    getLedgerReplayer() override
+    {
+        throw std::logic_error("TestServiceRegistry::getLedgerReplayer() not implemented");
+    }
+
+    PendingSaves&
+    getPendingSaves() override
+    {
+        return pendingSaves_;
+    }
+
+    OpenLedger&
+    getOpenLedger() override
+    {
+        throw std::logic_error("TestServiceRegistry::openLedger() not implemented");
+    }
+
+    OpenLedger const&
+    getOpenLedger() const override
+    {
+        throw std::logic_error("TestServiceRegistry::openLedger() const not implemented");
+    }
+
+    // Transaction and operation services
+    NetworkOPs&
+    getOPs() override
+    {
+        throw std::logic_error("TestServiceRegistry::getOPs() not implemented");
+    }
+
+    OrderBookDB&
+    getOrderBookDB() override
+    {
+        throw std::logic_error("TestServiceRegistry::getOrderBookDB() not implemented");
+    }
+
+    TransactionMaster&
+    getMasterTransaction() override
+    {
+        throw std::logic_error("TestServiceRegistry::getMasterTransaction() not implemented");
+    }
+
+    TxQ&
+    getTxQ() override
+    {
+        throw std::logic_error("TestServiceRegistry::getTxQ() not implemented");
+    }
+
+    PathRequestManager&
+    getPathRequestManager() override
+    {
+        throw std::logic_error("TestServiceRegistry::getPathRequestManager() not implemented");
+    }
+
+    // Server services
+    ServerHandler&
+    getServerHandler() override
+    {
+        throw std::logic_error("TestServiceRegistry::getServerHandler() not implemented");
+    }
+
+    perf::PerfLog&
+    getPerfLog() override
+    {
+        throw std::logic_error("TestServiceRegistry::getPerfLog() not implemented");
+    }
+
+    // Configuration and state
+    bool
+    isStopping() const override
+    {
+        return false;
+    }
+
+    beast::Journal
+    getJournal(std::string const& name) override
+    {
+        return logs_.journal(name);
+    }
+
+    boost::asio::io_context&
+    getIOContext() override
+    {
+        return io_context_;
+    }
+
+    Logs&
+    getLogs() override
+    {
+        return logs_;
+    }
+
+    std::optional<uint256> const&
+    getTrapTxID() const override
+    {
+        return trapTxID_;
+    }
+
+    DatabaseCon&
+    getWalletDB() override
+    {
+        throw std::logic_error("TestServiceRegistry::getWalletDB() not implemented");
+    }
+
+    // Temporary: Get the underlying Application
+    Application&
+    getApp() override
+    {
+        throw std::logic_error(
+            "TestServiceRegistry::app() not implemented - no Application available in tests");
+    }
+};
+
+}  // namespace test
+}  // namespace xrpl

src/tests/libxrpl/helpers/TxTest.cpp

@@ -0,0 +1,252 @@
+#include <helpers/TxTest.h>
+
+#include <xrpl/basics/base_uint.h>
+#include <xrpl/basics/chrono.h>
+#include <xrpl/basics/contract.h>
+#include <xrpl/ledger/ApplyView.h>
+#include <xrpl/ledger/CanonicalTXSet.h>
+#include <xrpl/ledger/Ledger.h>
+#include <xrpl/ledger/OpenView.h>
+#include <xrpl/ledger/ReadView.h>
+#include <xrpl/protocol/AccountID.h>
+#include <xrpl/protocol/Feature.h>
+#include <xrpl/protocol/Fees.h>
+#include <xrpl/protocol/Indexes.h>
+#include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/STLedgerEntry.h>
+#include <xrpl/protocol/STTx.h>
+#include <xrpl/protocol/TER.h>
+#include <xrpl/protocol_autogen/ledger_entries/AccountRoot.h>
+#include <xrpl/protocol_autogen/ledger_entries/RippleState.h>
+#include <xrpl/protocol_autogen/transactions/AccountSet.h>
+#include <xrpl/protocol_autogen/transactions/Payment.h>
+#include <xrpl/tx/apply.h>
+
+#include <helpers/Account.h>
+#include <helpers/IOU.h>
+
+#include <cstdint>
+#include <memory>
+#include <optional>
+#include <stdexcept>
+#include <utility>
+#include <vector>
+
+namespace xrpl::test {
+
+//------------------------------------------------------------------------------
+// Feature helpers
+//------------------------------------------------------------------------------
+
+FeatureBitset
+allFeatures()
+{
+    static FeatureBitset const features = [] {
+        auto const& sa = allAmendments();
+        std::vector<uint256> feats;
+        feats.reserve(sa.size());
+        for ([[maybe_unused]] auto const& [name, _] : sa)
+        {
+            if (auto const f = getRegisteredFeature(name); f.has_value())
+                feats.push_back(*f);
+        }
+        return FeatureBitset(feats);
+    }();
+    return features;
+}
+
+//------------------------------------------------------------------------------
+// TxTest
+//------------------------------------------------------------------------------
+
+TxTest::TxTest(std::optional<FeatureBitset> features)
+{
+    // Convert FeatureBitset to unordered_set for Rules constructor
+    auto const featureBits = features.value_or(allFeatures());
+    foreachFeature(featureBits, [&](uint256 const& f) { featureSet_.insert(f); });
+
+    // Create rules with the specified features
+    rules_.emplace(featureSet_);
+
+    // Default fees for testing
+    Fees const fees{XRPAmount{10}, XRPAmount{10000000}, XRPAmount{2000000}};
+
+    // Create a genesis ledger as the base
+    closedLedger_ = std::make_shared<Ledger>(
+        create_genesis,
+        // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+        *rules_,
+        fees,
+        std::vector<uint256>{featureSet_.begin(), featureSet_.end()},
+        registry_.getNodeFamily());
+
+    // Initialize time from the genesis ledger
+    now_ = closedLedger_->header().closeTime;
+
+    // Create an open view on top of the genesis ledger
+    openLedger_ =
+        std::make_shared<OpenView>(open_ledger, closedLedger_.get(), *rules_, closedLedger_);
+}
+
+bool
+TxTest::isEnabled(uint256 const& feature) const
+{
+    // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+    return rules_->enabled(feature);
+}
+
+Rules const&
+TxTest::getRules() const
+{
+    // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+    return *rules_;
+}
+
+[[nodiscard]] TxResult
+TxTest::submit(std::shared_ptr<STTx const> stx)
+{
+    auto result = apply(registry_, *openLedger_, *stx, tapNONE, registry_.getJournal("apply"));
+
+    // Track successfully applied transactions for canonical reordering on close
+    // We make a copy since the TransactionBase doesn't own the STTx
+    if (result.applied)
+        pendingTxs_.push_back(stx);
+
+    return TxResult{
+        .ter = result.ter,
+        .applied = result.applied,
+        .metadata = std::move(result).metadata,
+        .tx = std::move(stx)};
+}
+
+void
+TxTest::createAccount(Account const& account, XRPAmount xrp, uint32_t accountFlags)
+{
+    auto const paymentTer =
+        submit(transactions::PaymentBuilder{Account::master, account, xrp}, Account::master).ter;
+
+    if (paymentTer != tesSUCCESS)
+    {
+        throw std::runtime_error("TxTest::createAccount: failed to create account");
+    }
+
+    close();
+
+    if (accountFlags != 0)
+    {
+        auto const accountSetTer =
+            submit(transactions::AccountSetBuilder{account}.setSetFlag(accountFlags), account).ter;
+        if (accountSetTer != tesSUCCESS)
+        {
+            throw std::runtime_error("TxTest::createAccount: failed to set account flags");
+        }
+        close();
+    }
+}
+
+ledger_entries::AccountRoot
+TxTest::getAccountRoot(AccountID const& id) const
+{
+    auto const sle = getOpenLedger().read(keylet::account(id));
+    if (!sle)
+        Throw<std::runtime_error>("TxTest::getAccountRoot: account not found");
+    return ledger_entries::AccountRoot{std::const_pointer_cast<SLE const>(sle)};
+}
+
+OpenView&
+TxTest::getOpenLedger()
+{
+    return *openLedger_;
+}
+
+OpenView const&
+TxTest::getOpenLedger() const
+{
+    return *openLedger_;
+}
+
+ReadView const&
+TxTest::getClosedLedger() const
+{
+    return *closedLedger_;
+}
+
+void
+TxTest::close()
+{
+    // Build a new closed ledger from the previous closed ledger,
+    // similar to how buildLedgerImpl works:
+    // 1. Create a new Ledger from the previous closed ledger
+    // 2. Re-apply transactions in canonical order
+    // 3. Mark it as accepted/immutable
+
+    auto const& prevLedger = *closedLedger_;
+
+    auto const ledgerCloseTime = now_ + prevLedger.header().closeTimeResolution;
+
+    now_ = ledgerCloseTime;
+
+    auto newLedger = std::make_shared<Ledger>(prevLedger, ledgerCloseTime);
+
+    CanonicalTXSet txSet(prevLedger.header().hash);
+    for (auto const& tx : pendingTxs_)
+        txSet.insert(tx);
+
+    {
+        OpenView accum(&*newLedger);
+        for (auto const& [key, tx] : txSet)
+        {
+            auto result = apply(registry_, accum, *tx, tapNONE, registry_.getJournal("apply"));
+            if (!result.applied)
+            {
+                throw std::runtime_error("TxTest::close: failed to apply transaction");
+            }
+        }
+        accum.apply(*newLedger);
+    }
+
+    newLedger->setAccepted(ledgerCloseTime, newLedger->header().closeTimeResolution, true);
+
+    closedLedger_ = newLedger;
+
+    pendingTxs_.clear();
+
+    openLedger_ =
+        // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+        std::make_shared<OpenView>(open_ledger, closedLedger_.get(), *rules_, closedLedger_);
+}
+
+void
+TxTest::advanceTime(NetClock::duration duration)
+{
+    now_ += duration;
+}
+
+NetClock::time_point
+TxTest::getCloseTime() const
+{
+    return now_;
+}
+
+STAmount
+TxTest::getBalance(AccountID const& account, IOU const& iou) const
+{
+    auto const sle = openLedger_->read(keylet::line(account, iou.issue()));
+    if (!sle)
+        return STAmount{iou.issue(), 0};
+
+    auto const rippleState = ledger_entries::RippleState{sle};
+
+    auto balance = rippleState.getBalance();
+    if (iou.issue().account == account)
+    {
+        throw std::logic_error("TxTest::getBalance: account is issuer");
+    }
+
+    balance.get<Issue>().account = iou.issue().account;
+    if (account > iou.issue().account)
+        balance.negate();
+    return balance;
+}
+
+}  // namespace xrpl::test

src/tests/libxrpl/helpers/TxTest.h

@@ -0,0 +1,364 @@
+#pragma once
+
+#include <xrpl/basics/Number.h>
+#include <xrpl/basics/chrono.h>
+#include <xrpl/beast/utility/Journal.h>
+#include <xrpl/core/ServiceRegistry.h>
+#include <xrpl/ledger/ApplyViewImpl.h>
+#include <xrpl/ledger/Ledger.h>
+#include <xrpl/ledger/OpenView.h>
+#include <xrpl/protocol/Feature.h>
+#include <xrpl/protocol/LedgerFormats.h>
+#include <xrpl/protocol/Rules.h>
+#include <xrpl/protocol/STTx.h>
+#include <xrpl/protocol/TER.h>
+#include <xrpl/protocol/TxFlags.h>
+#include <xrpl/protocol/XRPAmount.h>
+#include <xrpl/protocol_autogen/TransactionBuilderBase.h>
+#include <xrpl/protocol_autogen/ledger_entries/AccountRoot.h>
+#include <xrpl/tx/applySteps.h>
+
+#include <helpers/Account.h>
+#include <helpers/IOU.h>
+#include <helpers/TestServiceRegistry.h>
+
+#include <cmath>
+#include <concepts>
+#include <memory>
+#include <optional>
+#include <stdexcept>
+#include <type_traits>
+#include <unordered_set>
+#include <vector>
+
+namespace xrpl::test {
+
+//------------------------------------------------------------------------------
+// Amount helpers
+//------------------------------------------------------------------------------
+
+/**
+ * @brief Convert XRP to drops (integral types).
+ * @param xrp The amount in XRP.
+ * @return The equivalent amount in drops as XRPAmount.
+ */
+template <std::integral T>
+constexpr XRPAmount
+XRP(T xrp)
+{
+    return XRPAmount{static_cast<std::int64_t>(xrp) * DROPS_PER_XRP.drops()};
+}
+
+/**
+ * @brief Convert XRP to drops (floating point types).
+ * @param xrp The amount in XRP (may be fractional).
+ * @return The equivalent amount in drops as XRPAmount.
+ */
+template <std::floating_point T>
+XRPAmount
+XRP(T xrp)
+{
+    return XRPAmount{static_cast<std::int64_t>(std::round(xrp * DROPS_PER_XRP.drops()))};
+}
+
+/**
+ * @brief Convert XRP to drops (Number type).
+ * @param xrp The amount in XRP as a Number.
+ * @return The equivalent amount in drops as XRPAmount.
+ */
+inline XRPAmount
+XRP(Number const& xrp)
+{
+    return XRPAmount{static_cast<std::int64_t>(xrp * DROPS_PER_XRP.drops())};
+}
+
+//------------------------------------------------------------------------------
+// Flag helpers
+//------------------------------------------------------------------------------
+
+/**
+ * @brief Convert AccountSet flag (asf) to LedgerState flag (lsf).
+ * @param asf The AccountSet flag value.
+ * @return The corresponding LedgerState flag.
+ * @throws std::runtime_error if the flag is not supported.
+ *
+ * Supported flags:
+ *   asfRequireDest, asfRequireAuth, asfDisallowXRP, asfDisableMaster,
+ *   asfNoFreeze, asfGlobalFreeze, asfDefaultRipple, asfDepositAuth,
+ *   asfAllowTrustLineClawback, asfDisallowIncomingCheck,
+ *   asfDisallowIncomingNFTokenOffer, asfDisallowIncomingPayChan,
+ *   asfDisallowIncomingTrustline, asfAllowTrustLineLocking
+ */
+constexpr std::uint32_t
+asfToLsf(std::uint32_t asf)
+{
+    switch (asf)
+    {
+        case asfRequireDest:
+            return lsfRequireDestTag;
+        case asfRequireAuth:
+            return lsfRequireAuth;
+        case asfDisallowXRP:
+            return lsfDisallowXRP;
+        case asfDisableMaster:
+            return lsfDisableMaster;
+        case asfNoFreeze:
+            return lsfNoFreeze;
+        case asfGlobalFreeze:
+            return lsfGlobalFreeze;
+        case asfDefaultRipple:
+            return lsfDefaultRipple;
+        case asfDepositAuth:
+            return lsfDepositAuth;
+        case asfAllowTrustLineClawback:
+            return lsfAllowTrustLineClawback;
+        case asfDisallowIncomingCheck:
+            return lsfDisallowIncomingCheck;
+        case asfDisallowIncomingNFTokenOffer:
+            return lsfDisallowIncomingNFTokenOffer;
+        case asfDisallowIncomingPayChan:
+            return lsfDisallowIncomingPayChan;
+        case asfDisallowIncomingTrustline:
+            return lsfDisallowIncomingTrustline;
+        case asfAllowTrustLineLocking:
+            return lsfAllowTrustLineLocking;
+        default:
+            throw std::runtime_error("Unknown asf flag");
+    }
+}
+
+//------------------------------------------------------------------------------
+// Feature helpers
+//------------------------------------------------------------------------------
+
+/**
+ * @brief Returns all testable amendments.
+ * @note This is similar to jtx::testable_amendments() but for the TxTest framework.
+ */
+FeatureBitset
+allFeatures();
+
+//------------------------------------------------------------------------------
+// TxResult
+//------------------------------------------------------------------------------
+
+/**
+ * @brief Result of a transaction submission in TxTest.
+ *
+ * Contains the TER code, whether the transaction was applied,
+ * optional metadata, and a reference to the submitted transaction.
+ * Use standard gtest macros (EXPECT_EQ, EXPECT_TRUE, etc.) to verify results.
+ */
+struct TxResult
+{
+    TER ter;                        /**< The transaction engine result code. */
+    bool applied;                   /**< Whether the transaction was applied to the ledger. */
+    std::optional<TxMeta> metadata; /**< Transaction metadata, if available. */
+    std::shared_ptr<STTx const> tx; /**< Pointer to the submitted transaction. */
+};
+
+/**
+ * @brief A lightweight transaction testing harness.
+ *
+ * Unlike the JTx framework which requires a full Application and RPC layer,
+ * TxTest applies transactions directly to an OpenView using the transactor
+ * pipeline (preflight -> preclaim -> doApply).
+ *
+ * This makes it suitable for:
+ * - Unit testing individual transactors
+ * - Testing transaction validation logic
+ * - Fast, focused tests without full server infrastructure
+ *
+ * @code
+ *     TxTest env;
+ *     env.submit(paymentTx).expectSuccess();
+ *     env.submit(badTx).expectTer(tecNO_ENTRY);
+ * @endcode
+ */
+class TxTest
+{
+public:
+    /**
+     * @brief Construct a TxTest environment.
+     *
+     * Creates a genesis ledger and an open view on top of it.
+     *
+     * @param features Optional set of features to enable. If not specified,
+     *                 uses all testable amendments.
+     */
+    explicit TxTest(std::optional<FeatureBitset> features = std::nullopt);
+
+    /**
+     * @brief Check if a feature is enabled.
+     * @param feature The feature to check.
+     * @return True if the feature is enabled.
+     */
+    [[nodiscard]] bool
+    isEnabled(uint256 const& feature) const;
+
+    /**
+     * @brief Get the current rules.
+     * @return The current consensus rules.
+     */
+    [[nodiscard]] Rules const&
+    getRules() const;
+
+    /**
+     * @brief Submit a transaction from a builder.
+     *
+     * Convenience overload that accepts transaction builders.
+     * Automatically sets sequence and fee before submission.
+     *
+     * @tparam T A type derived from TransactionBuilderBase.
+     * @param builder The transaction builder.
+     * @param signer The account to sign with.
+     * @return TxResult containing the result code, applied status, and metadata.
+     */
+    template <typename T>
+        requires std::
+            derived_from<std::decay_t<T>, transactions::TransactionBuilderBase<std::decay_t<T>>>
+        [[nodiscard]] TxResult
+        submit(T&& builder, Account const& signer)
+    {
+        auto const& obj = builder.getSTObject();
+        auto accountId = obj[sfAccount];
+        // Only set sequence if not using a ticket (ticket sets sequence to 0)
+        if (!obj.isFieldPresent(sfTicketSequence))
+        {
+            builder.setSequence(getAccountRoot(accountId).getSequence());
+        }
+        else
+        {
+            builder.setSequence(0);
+        }
+        builder.setFee(XRPAmount(10));
+        return submit(builder.build(signer.pk(), signer.sk()).getSTTx());
+    }
+
+    /**
+     * @brief Submit a transaction to the open ledger.
+     *
+     * Applies the transaction through the full transactor pipeline:
+     * preflight -> preclaim -> doApply -> invariant checks
+     *
+     * Invariant checks are automatically run after doApply. If any
+     * invariant fails, the result will be tecINVARIANT_FAILED.
+     *
+     * @param stx The transaction to submit.
+     * @return TxResult containing the result code, applied status, and metadata.
+     */
+    [[nodiscard]] TxResult
+    submit(std::shared_ptr<STTx const> stx);
+
+    /**
+     * @brief Create a new account in the ledger.
+     *
+     * Sends a Payment from the master account to create and fund the account.
+     * Closes the ledger after creation. If accountFlags is non-zero, submits
+     * an AccountSet transaction and closes again.
+     *
+     * @param account The account to create.
+     * @param xrp The initial XRP balance.
+     * @param accountFlags Optional account flags to set. Defaults to 0
+     *        (no flags).
+     */
+    void
+    createAccount(Account const& account, XRPAmount xrp, uint32_t accountFlags = 0);
+
+    /**
+     * @brief Get the account root object from the current open ledger.
+     * @param id The account ID.
+     * @return The AccountRoot ledger entry.
+     * @throws std::runtime_error if the account does not exist.
+     * @todo Once we make keylet strongly typed, we can ditch this method.
+     */
+    [[nodiscard]] ledger_entries::AccountRoot
+    getAccountRoot(AccountID const& id) const;
+
+    /**
+     * @brief Get the current open ledger view.
+     * @return A mutable reference to the open ledger.
+     */
+    [[nodiscard]] OpenView&
+    getOpenLedger();
+
+    /**
+     * @brief Get the current open ledger view (const).
+     * @return A const reference to the open ledger.
+     */
+    [[nodiscard]] OpenView const&
+    getOpenLedger() const;
+
+    /**
+     * @brief Get the closed (base) ledger view.
+     * @return A const reference to the closed ledger.
+     */
+    [[nodiscard]] ReadView const&
+    getClosedLedger() const;
+
+    /**
+     * @brief Close the current ledger.
+     *
+     * Creates a new closed ledger from the current open ledger.
+     * All pending transactions are re-applied in canonical order.
+     */
+    void
+    close();
+
+    /**
+     * @brief Advance time without closing the ledger.
+     *
+     * Useful for testing time-dependent features like escrow release
+     * times or offer expirations.
+     *
+     * @param duration The amount of time to advance.
+     */
+    void
+    advanceTime(NetClock::duration duration);
+
+    /**
+     * @brief Get the current ledger close time.
+     * @return The current close time.
+     */
+    [[nodiscard]] NetClock::time_point
+    getCloseTime() const;
+
+    /**
+     * @brief Get the balance of an IOU for an account.
+     *
+     * Returns the balance from the perspective of the specified account.
+     * If the trust line doesn't exist, returns zero.
+     *
+     * @param account The account to check.
+     * @param iou The IOU to check the balance for.
+     * @return The balance as an STAmount.
+     * @todo Once we make keylet strongly typed, we can ditch this method.
+     */
+    [[nodiscard]] STAmount
+    getBalance(AccountID const& account, IOU const& iou) const;
+
+    /**
+     * @brief Get the service registry.
+     * @return A reference to the service registry.
+     */
+    ServiceRegistry&
+    getServiceRegistry()
+    {
+        return registry_;
+    }
+
+private:
+    TestServiceRegistry registry_;
+    std::unordered_set<uint256, beast::uhash<>> featureSet_;
+    std::optional<Rules> rules_;
+    std::shared_ptr<Ledger const> closedLedger_;
+    std::shared_ptr<OpenView> openLedger_;
+
+    /** Transactions submitted to the open ledger, for canonical reordering on close. */
+    std::vector<std::shared_ptr<STTx const>> pendingTxs_;
+
+    /** Current time (can be advanced arbitrarily for testing). */
+    NetClock::time_point now_;
+};
+
+}  // namespace xrpl::test

src/tests/libxrpl/tx/AccountSet.cpp

@@ -0,0 +1,804 @@
+#include <xrpl/protocol_autogen/transactions/AccountSet.h>
+
+#include <xrpl/basics/Slice.h>
+#include <xrpl/basics/base_uint.h>
+#include <xrpl/beast/utility/Zero.h>
+#include <xrpl/core/ServiceRegistry.h>
+#include <xrpl/ledger/helpers/DirectoryHelpers.h>
+#include <xrpl/protocol/Indexes.h>
+#include <xrpl/protocol/KeyType.h>
+#include <xrpl/protocol/LedgerFormats.h>
+#include <xrpl/protocol/Quality.h>
+#include <xrpl/protocol/Rate.h>
+#include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/STArray.h>
+#include <xrpl/protocol/STObject.h>
+#include <xrpl/protocol/STTx.h>
+#include <xrpl/protocol/SecretKey.h>
+#include <xrpl/protocol/TER.h>
+#include <xrpl/protocol/TxFlags.h>
+#include <xrpl/protocol_autogen/ledger_entries/AccountRoot.h>
+#include <xrpl/protocol_autogen/transactions/Payment.h>
+#include <xrpl/protocol_autogen/transactions/SetRegularKey.h>
+#include <xrpl/protocol_autogen/transactions/SignerListSet.h>
+#include <xrpl/protocol_autogen/transactions/TicketCreate.h>
+#include <xrpl/protocol_autogen/transactions/TrustSet.h>
+
+#include <gtest/gtest.h>
+#include <helpers/Account.h>
+#include <helpers/IOU.h>
+#include <helpers/TxTest.h>
+
+#include <algorithm>
+#include <cstddef>
+#include <cstdint>
+#include <initializer_list>
+#include <limits>
+#include <memory>
+#include <string>
+#include <string_view>
+#include <utility>
+#include <vector>
+
+namespace xrpl::test {
+
+TEST(AccountSet, NullAccountSet)
+{
+    TxTest env;
+
+    Account const alice("alice");
+    env.createAccount(alice, XRP(10));
+
+    auto& view = env.getOpenLedger();
+
+    // ask for the ledger entry - account root, to check its flags
+    auto sle = view.read(keylet::account(alice));
+
+    EXPECT_NE(sle, nullptr);
+    ledger_entries::AccountRoot const accountRoot(sle);
+    EXPECT_EQ(accountRoot.getFlags(), 0);
+}
+
+TEST(AccountSet, MostFlags)
+{
+    Account const alice("alice");
+
+    TxTest env;
+    env.createAccount(alice, XRP(10000));
+
+    // Give alice a regular key so she can legally set and clear
+    // her asfDisableMaster flag.
+    Account const aliceRegularKey{"aliceRegularKey", KeyType::secp256k1};
+
+    env.createAccount(aliceRegularKey, XRP(10000));
+    env.close();
+
+    EXPECT_EQ(
+        env.submit(transactions::SetRegularKeyBuilder{alice}.setRegularKey(aliceRegularKey), alice)
+            .ter,
+        tesSUCCESS);
+    env.close();
+
+    auto testFlags = [&alice, &aliceRegularKey, &env](
+                         std::initializer_list<std::uint32_t> goodFlags) {
+        std::uint32_t const orig_flags = env.getAccountRoot(alice).getFlags();
+        for (std::uint32_t flag{1u}; flag < std::numeric_limits<std::uint32_t>::digits; ++flag)
+        {
+            if (flag == asfNoFreeze)
+            {
+                // The asfNoFreeze flag can't be cleared.  It is tested
+                // elsewhere.
+                continue;
+            }
+            if (flag == asfAuthorizedNFTokenMinter)
+            {
+                // The asfAuthorizedNFTokenMinter flag requires the
+                // presence or absence of the sfNFTokenMinter field in
+                // the transaction.  It is tested elsewhere.
+                continue;
+            }
+
+            if (flag == asfDisallowIncomingCheck || flag == asfDisallowIncomingPayChan ||
+                flag == asfDisallowIncomingNFTokenOffer || flag == asfDisallowIncomingTrustline)
+            {
+                // These flags are part of the DisallowIncoming amendment
+                // and are tested elsewhere
+                continue;
+            }
+            if (flag == asfAllowTrustLineClawback)
+            {
+                // The asfAllowTrustLineClawback flag can't be cleared.  It
+                // is tested elsewhere.
+                continue;
+            }
+            if (flag == asfAllowTrustLineLocking)
+            {
+                // These flags are part of the AllowTokenLocking amendment
+                // and are tested elsewhere
+                continue;
+            }
+            if (std::ranges::find(goodFlags, flag) != goodFlags.end())
+            {
+                // Good flag
+                EXPECT_FALSE(env.getAccountRoot(alice).isFlag(asfToLsf(flag)));
+
+                EXPECT_EQ(
+                    env.submit(transactions::AccountSetBuilder{alice}.setSetFlag(flag), alice).ter,
+                    tesSUCCESS);
+                env.close();
+
+                EXPECT_TRUE(env.getAccountRoot(alice).isFlag(asfToLsf(flag)));
+
+                EXPECT_EQ(
+                    env.submit(
+                           transactions::AccountSetBuilder{alice}.setClearFlag(flag),
+                           aliceRegularKey)
+                        .ter,
+                    tesSUCCESS);
+                env.close();
+
+                EXPECT_FALSE(env.getAccountRoot(alice).isFlag(asfToLsf(flag)));
+
+                std::uint32_t const now_flags = env.getAccountRoot(alice).getFlags();
+                EXPECT_EQ(now_flags, orig_flags);
+            }
+            else
+            {
+                // Bad flag
+                EXPECT_EQ(env.getAccountRoot(alice).getFlags(), orig_flags);
+                EXPECT_EQ(
+                    env.submit(transactions::AccountSetBuilder{alice}.setSetFlag(flag), alice).ter,
+                    tesSUCCESS);
+                env.close();
+                EXPECT_EQ(env.getAccountRoot(alice).getFlags(), orig_flags);
+
+                EXPECT_EQ(
+                    env.submit(
+                           transactions::AccountSetBuilder{alice}.setClearFlag(flag),
+                           aliceRegularKey)
+                        .ter,
+                    tesSUCCESS);
+                env.close();
+                EXPECT_EQ(env.getAccountRoot(alice).getFlags(), orig_flags);
+            }
+        }
+    };
+    testFlags({
+        asfRequireDest,
+        asfRequireAuth,
+        asfDisallowXRP,
+        asfGlobalFreeze,
+        asfDisableMaster,
+        asfDefaultRipple,
+        asfDepositAuth,
+    });
+}
+
+TEST(AccountSet, SetAndResetAccountTxnID)
+{
+    TxTest env;
+    Account const alice("alice");
+
+    env.createAccount(alice, XRP(10000));
+
+    std::uint32_t const orig_flags = env.getAccountRoot(alice).getFlags();
+
+    // asfAccountTxnID is special and not actually set as a flag,
+    // so we check the field presence instead
+    EXPECT_FALSE(env.getAccountRoot(alice).hasAccountTxnID());
+
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setSetFlag(asfAccountTxnID), alice).ter,
+        tesSUCCESS);
+    env.close();
+
+    EXPECT_TRUE(env.getAccountRoot(alice).hasAccountTxnID());
+
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setClearFlag(asfAccountTxnID), alice).ter,
+        tesSUCCESS);
+    env.close();
+
+    EXPECT_FALSE(env.getAccountRoot(alice).hasAccountTxnID());
+
+    std::uint32_t const now_flags = env.getAccountRoot(alice).getFlags();
+    EXPECT_EQ(now_flags, orig_flags);
+}
+
+TEST(AccountSet, SetNoFreeze)
+{
+    TxTest env;
+    Account const alice("alice");
+    Account const eric("eric");
+
+    env.createAccount(alice, XRP(10000));
+    env.close();
+
+    // Set eric as alice's regular key (eric doesn't need to be funded)
+    EXPECT_EQ(
+        env.submit(transactions::SetRegularKeyBuilder{alice}.setRegularKey(eric), alice).ter,
+        tesSUCCESS);
+    env.close();
+
+    // Verify alice doesn't have NoFreeze flag
+    EXPECT_FALSE(env.getAccountRoot(alice).isFlag(lsfNoFreeze));
+
+    // Setting NoFreeze with regular key should fail - requires master key
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setSetFlag(asfNoFreeze), eric).ter,
+        tecNEED_MASTER_KEY);
+    env.close();
+
+    // Setting NoFreeze with master key should succeed
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setSetFlag(asfNoFreeze), alice).ter,
+        tesSUCCESS);
+    env.close();
+
+    // Verify alice now has NoFreeze flag
+    EXPECT_TRUE(env.getAccountRoot(alice).isFlag(lsfNoFreeze));
+
+    // Try to clear NoFreeze - transaction succeeds but flag remains set
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setClearFlag(asfNoFreeze), alice).ter,
+        tesSUCCESS);
+    env.close();
+
+    // Verify flag is still set (NoFreeze cannot be cleared once set)
+    EXPECT_TRUE(env.getAccountRoot(alice).isFlag(lsfNoFreeze));
+}
+
+TEST(AccountSet, Domain)
+{
+    TxTest env;
+    Account const alice("alice");
+
+    env.createAccount(alice, XRP(10000));
+    env.close();
+
+    // The Domain field is represented as the hex string of the lowercase
+    // ASCII of the domain. For example, the domain example.com would be
+    // represented as "6578616d706c652e636f6d".
+    //
+    // To remove the Domain field from an account, send an AccountSet with
+    // the Domain set to an empty string.
+    std::string const domain = "example.com";
+
+    // Set domain
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setDomain(makeSlice(domain)), alice).ter,
+        tesSUCCESS);
+    env.close();
+
+    EXPECT_TRUE(env.getAccountRoot(alice).hasDomain());
+    // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+    EXPECT_EQ(*env.getAccountRoot(alice).getDomain(), makeSlice(domain));
+
+    // Clear domain by setting empty
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setDomain(Slice{}), alice).ter,
+        tesSUCCESS);
+    env.close();
+
+    EXPECT_FALSE(env.getAccountRoot(alice).hasDomain());
+
+    // The upper limit on the length is 256 bytes
+    // (defined as DOMAIN_BYTES_MAX in SetAccount)
+    // test the edge cases: 255, 256, 257.
+    std::size_t const maxLength = 256;
+    for (std::size_t len = maxLength - 1; len <= maxLength + 1; ++len)
+    {
+        std::string const domain2 = std::string(len - domain.length() - 1, 'a') + "." + domain;
+
+        EXPECT_EQ(domain2.length(), len);
+
+        if (len <= maxLength)
+        {
+            EXPECT_EQ(
+                env.submit(
+                       transactions::AccountSetBuilder{alice}.setDomain(makeSlice(domain2)), alice)
+                    .ter,
+                tesSUCCESS);
+            env.close();
+
+            // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+            EXPECT_EQ(*env.getAccountRoot(alice).getDomain(), makeSlice(domain2));
+        }
+        else
+        {
+            EXPECT_EQ(
+                env.submit(
+                       transactions::AccountSetBuilder{alice}.setDomain(makeSlice(domain2)), alice)
+                    .ter,
+                telBAD_DOMAIN);
+            env.close();
+        }
+    }
+}
+
+TEST(AccountSet, MessageKey)
+{
+    TxTest env;
+    Account const alice("alice");
+
+    env.createAccount(alice, XRP(10000));
+    env.close();
+
+    // Generate a random ed25519 key pair for the message key
+    auto const rkp = randomKeyPair(KeyType::ed25519);
+
+    // Set the message key
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setMessageKey(rkp.first.slice()), alice)
+            .ter,
+        tesSUCCESS);
+    env.close();
+
+    EXPECT_TRUE(env.getAccountRoot(alice).hasMessageKey());
+    // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+    EXPECT_EQ(*env.getAccountRoot(alice).getMessageKey(), rkp.first.slice());
+
+    // Clear the message key by setting to empty
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setMessageKey(Slice{}), alice).ter,
+        tesSUCCESS);
+    env.close();
+
+    EXPECT_FALSE(env.getAccountRoot(alice).hasMessageKey());
+
+    // Try to set an invalid public key - should fail
+    using namespace std::string_literals;
+    EXPECT_EQ(
+        env.submit(
+               transactions::AccountSetBuilder{alice}.setMessageKey(
+                   makeSlice("NOT_REALLY_A_PUBKEY"s)),
+               alice)
+            .ter,
+        telBAD_PUBLIC_KEY);
+}
+
+TEST(AccountSet, WalletID)
+{
+    TxTest env;
+    Account const alice("alice");
+
+    env.createAccount(alice, XRP(10000));
+    env.close();
+
+    std::string_view const locator =
+        "9633EC8AF54F16B5286DB1D7B519EF49EEFC050C0C8AC4384F1D88ACD1BFDF05";
+    uint256 locatorHash{};
+    EXPECT_TRUE(locatorHash.parseHex(locator));
+
+    // Set the wallet locator
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setWalletLocator(locatorHash), alice).ter,
+        tesSUCCESS);
+    env.close();
+
+    EXPECT_TRUE(env.getAccountRoot(alice).hasWalletLocator());
+    // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+    EXPECT_EQ(*env.getAccountRoot(alice).getWalletLocator(), locatorHash);
+
+    // Clear the wallet locator by setting to zero
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setWalletLocator(beast::zero), alice).ter,
+        tesSUCCESS);
+    env.close();
+
+    EXPECT_FALSE(env.getAccountRoot(alice).hasWalletLocator());
+}
+
+TEST(AccountSet, EmailHash)
+{
+    TxTest env;
+    Account const alice("alice");
+
+    env.createAccount(alice, XRP(10000));
+    env.close();
+
+    std::string_view const mh = "5F31A79367DC3137FADA860C05742EE6";
+    uint128 emailHash{};
+    EXPECT_TRUE(emailHash.parseHex(mh));
+
+    // Set the email hash
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setEmailHash(emailHash), alice).ter,
+        tesSUCCESS);
+    env.close();
+
+    EXPECT_TRUE(env.getAccountRoot(alice).hasEmailHash());
+    // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+    EXPECT_EQ(*env.getAccountRoot(alice).getEmailHash(), emailHash);
+
+    // Clear the email hash by setting to zero
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setEmailHash(beast::zero), alice).ter,
+        tesSUCCESS);
+    env.close();
+
+    EXPECT_FALSE(env.getAccountRoot(alice).hasEmailHash());
+}
+
+TEST(AccountSet, TransferRate)
+{
+    struct TestCase
+    {
+        double set;
+        TER code;
+        double get;
+    };
+
+    // Test data: {rate to set, expected TER, expected stored rate}
+    std::vector<TestCase> const testData = {
+        {1.0, tesSUCCESS, 1.0},
+        {1.1, tesSUCCESS, 1.1},
+        {2.0, tesSUCCESS, 2.0},
+        {2.1, temBAD_TRANSFER_RATE, 2.0},  // > 2.0 is invalid
+        {0.0, tesSUCCESS, 1.0},            // 0 clears the rate (default = 1.0)
+        {2.0, tesSUCCESS, 2.0},
+        {0.9, temBAD_TRANSFER_RATE, 2.0},  // < 1.0 is invalid
+    };
+
+    TxTest env;
+    Account const alice("alice");
+
+    env.createAccount(alice, XRP(10000));
+    env.close();
+
+    for (auto const& r : testData)
+    {
+        auto const rateValue = static_cast<std::uint32_t>(QUALITY_ONE * r.set);
+
+        EXPECT_EQ(
+            env.submit(transactions::AccountSetBuilder{alice}.setTransferRate(rateValue), alice)
+                .ter,
+            r.code);
+        env.close();
+
+        // If the field is not present, expect the default value (1.0)
+        if (!env.getAccountRoot(alice).hasTransferRate())
+        {
+            EXPECT_EQ(r.get, 1.0);
+        }
+        else
+        {
+            // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
+            EXPECT_EQ(
+                *env.getAccountRoot(alice).getTransferRate(),
+                static_cast<std::uint32_t>(r.get * QUALITY_ONE));
+        }
+    }
+}
+
+TEST(AccountSet, BadInputs)
+{
+    TxTest env;
+    Account const alice("alice");
+
+    env.createAccount(alice, XRP(10000));
+    env.close();
+
+    // Setting and clearing the same flag is invalid
+    EXPECT_EQ(
+        env.submit(
+               transactions::AccountSetBuilder{alice}
+                   .setSetFlag(asfDisallowXRP)
+                   .setClearFlag(asfDisallowXRP),
+               alice)
+            .ter,
+        temINVALID_FLAG);
+
+    EXPECT_EQ(
+        env.submit(
+               transactions::AccountSetBuilder{alice}
+                   .setSetFlag(asfRequireAuth)
+                   .setClearFlag(asfRequireAuth),
+               alice)
+            .ter,
+        temINVALID_FLAG);
+
+    EXPECT_EQ(
+        env.submit(
+               transactions::AccountSetBuilder{alice}
+                   .setSetFlag(asfRequireDest)
+                   .setClearFlag(asfRequireDest),
+               alice)
+            .ter,
+        temINVALID_FLAG);
+
+    // Setting asf flag while also using corresponding tf flag is invalid
+    EXPECT_EQ(
+        env.submit(
+               transactions::AccountSetBuilder{alice}
+                   .setSetFlag(asfDisallowXRP)
+                   .setFlags(tfAllowXRP),
+               alice)
+            .ter,
+        temINVALID_FLAG);
+
+    EXPECT_EQ(
+        env.submit(
+               transactions::AccountSetBuilder{alice}
+                   .setSetFlag(asfRequireAuth)
+                   .setFlags(tfOptionalAuth),
+               alice)
+            .ter,
+        temINVALID_FLAG);
+
+    EXPECT_EQ(
+        env.submit(
+               transactions::AccountSetBuilder{alice}
+                   .setSetFlag(asfRequireDest)
+                   .setFlags(tfOptionalDestTag),
+               alice)
+            .ter,
+        temINVALID_FLAG);
+
+    // Using invalid flags (mask) is invalid
+    EXPECT_EQ(
+        env.submit(
+               transactions::AccountSetBuilder{alice}
+                   .setSetFlag(asfRequireDest)
+                   .setFlags(tfAccountSetMask),
+               alice)
+            .ter,
+        temINVALID_FLAG);
+
+    // Disabling master key without an alternative key is invalid
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setSetFlag(asfDisableMaster), alice).ter,
+        tecNO_ALTERNATIVE_KEY);
+}
+
+TEST(AccountSet, RequireAuthWithDir)
+{
+    TxTest env;
+    Account const alice("alice");
+    Account const bob("bob");
+
+    env.createAccount(alice, XRP(10000));
+    env.close();
+
+    // alice should have an empty directory
+    EXPECT_TRUE(dirIsEmpty(env.getClosedLedger(), keylet::ownerDir(alice.id())));
+
+    // Give alice a signer list, then there will be stuff in the directory
+    // Build the SignerEntries array
+    STArray signerEntries(1);
+    {
+        signerEntries.push_back(STObject::makeInnerObject(sfSignerEntry));
+        STObject& entry = signerEntries.back();
+        entry[sfAccount] = bob.id();
+        entry[sfSignerWeight] = std::uint16_t{1};
+    }
+
+    EXPECT_EQ(
+        env.submit(
+               transactions::SignerListSetBuilder{alice, 1}.setSignerEntries(signerEntries), alice)
+            .ter,
+        tesSUCCESS);
+    env.close();
+
+    EXPECT_FALSE(dirIsEmpty(env.getClosedLedger(), keylet::ownerDir(alice.id())));
+
+    // Setting RequireAuth should fail because alice has owner objects
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setSetFlag(asfRequireAuth), alice).ter,
+        tecOWNERS);
+
+    // Remove the signer list (quorum = 0, no entries)
+    EXPECT_EQ(env.submit(transactions::SignerListSetBuilder{alice, 0}, alice).ter, tesSUCCESS);
+    env.close();
+
+    EXPECT_TRUE(dirIsEmpty(env.getClosedLedger(), keylet::ownerDir(alice.id())));
+
+    // Now setting RequireAuth should succeed
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setSetFlag(asfRequireAuth), alice).ter,
+        tesSUCCESS);
+}
+
+TEST(AccountSet, Ticket)
+{
+    TxTest env;
+    Account const alice("alice");
+
+    env.createAccount(alice, XRP(10000));
+    env.close();
+
+    // Get alice's current sequence - the ticket will be created at seq + 1
+    std::uint32_t const aliceSeqBefore = env.getAccountRoot(alice.id()).getSequence();
+    std::uint32_t const ticketSeq = aliceSeqBefore + 1;
+
+    // Create a ticket
+    EXPECT_EQ(env.submit(transactions::TicketCreateBuilder{alice, 1}, alice).ter, tesSUCCESS);
+    env.close();
+
+    // Verify alice has 1 owner object (the ticket)
+    EXPECT_EQ(env.getAccountRoot(alice.id()).getOwnerCount(), 1u);
+    // Verify ticket exists
+    EXPECT_TRUE(env.getClosedLedger().exists(keylet::ticket(alice.id(), ticketSeq)));
+
+    // Try using a ticket that alice doesn't have
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setTicketSequence(ticketSeq + 1), alice)
+            .ter,
+        terPRE_TICKET);
+    env.close();
+
+    // Verify ticket still exists
+    EXPECT_TRUE(env.getClosedLedger().exists(keylet::ticket(alice.id(), ticketSeq)));
+
+    // Get alice's sequence before using the ticket
+    std::uint32_t const aliceSeq = env.getAccountRoot(alice.id()).getSequence();
+
+    // Actually use alice's ticket (noop AccountSet)
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setTicketSequence(ticketSeq), alice).ter,
+        tesSUCCESS);
+    env.close();
+
+    // Verify ticket is consumed (no owner objects)
+    EXPECT_EQ(env.getAccountRoot(alice.id()).getOwnerCount(), 0u);
+    EXPECT_FALSE(env.getClosedLedger().exists(keylet::ticket(alice.id(), ticketSeq)));
+
+    // Verify alice's sequence did NOT advance (ticket use doesn't increment seq)
+    EXPECT_EQ(env.getAccountRoot(alice.id()).getSequence(), aliceSeq);
+
+    // Try re-using a ticket that alice already used
+    EXPECT_EQ(
+        env.submit(transactions::AccountSetBuilder{alice}.setTicketSequence(ticketSeq), alice).ter,
+        tefNO_TICKET);
+}
+
+TEST(AccountSet, BadSigningKey)
+{
+    TxTest env;
+    Account const alice("alice");
+
+    env.createAccount(alice, XRP(10000));
+    env.close();
+
+    // Build a valid transaction first, then corrupt the signing key
+    auto stx = transactions::AccountSetBuilder{alice}
+                   .setSequence(env.getAccountRoot(alice.id()).getSequence())
+                   .setFee(XRPAmount{10})
+                   .build(alice.pk(), alice.sk())
+                   .getSTTx();
+
+    // Create a copy with a bad signing key
+    STObject obj = *stx;
+    obj.setFieldVL(sfSigningPubKey, makeSlice(std::string("badkey")));
+
+    auto result = env.submit(std::make_shared<STTx>(std::move(obj)));
+    EXPECT_EQ(result.ter, temBAD_SIGNATURE);
+    EXPECT_FALSE(result.applied);
+}
+
+TEST(AccountSet, Gateway)
+{
+    Account const alice("alice");
+    Account const bob("bob");
+    Account const gw("gateway");
+    IOU const USD("USD", gw);
+
+    // Test gateway with a variety of allowed transfer rates
+    for (double transferRate = 1.0; transferRate <= 2.0; transferRate += 0.03125)
+    {
+        TxTest env;
+
+        env.createAccount(gw, XRP(10000), asfDefaultRipple);
+        env.createAccount(alice, XRP(10000), asfDefaultRipple);
+        env.createAccount(bob, XRP(10000), asfDefaultRipple);
+        env.close();
+
+        // Set up trust lines: alice and bob trust gw for USD
+        EXPECT_EQ(
+            env.submit(transactions::TrustSetBuilder{alice}.setLimitAmount(USD.amount(10)), alice)
+                .ter,
+            tesSUCCESS);
+        EXPECT_EQ(
+            env.submit(transactions::TrustSetBuilder{bob}.setLimitAmount(USD.amount(10)), bob).ter,
+            tesSUCCESS);
+        env.close();
+
+        // Set transfer rate on the gateway
+        EXPECT_EQ(
+            env.submit(
+                   transactions::AccountSetBuilder{gw}.setTransferRate(
+                       static_cast<std::uint32_t>(transferRate * QUALITY_ONE)),
+                   gw)
+                .ter,
+            tesSUCCESS);
+        env.close();
+
+        // Calculate the amount with transfer rate applied
+        auto const amount = USD.amount(1);
+        Rate const rate(static_cast<std::uint32_t>(transferRate * QUALITY_ONE));
+        auto const amountWithRate = multiply(amount, rate);
+
+        // Gateway pays alice 10 USD
+        EXPECT_EQ(
+            env.submit(transactions::PaymentBuilder{gw, alice, USD.amount(10)}, gw).ter,
+            tesSUCCESS);
+        env.close();
+
+        // Alice pays bob 1 USD (with sendmax to cover transfer fee)
+        EXPECT_EQ(
+            env.submit(
+                   transactions::PaymentBuilder{alice, bob, USD.amount(1)}.setSendMax(
+                       USD.amount(10)),
+                   alice)
+                .ter,
+            tesSUCCESS);
+        env.close();
+
+        // Check balances
+        EXPECT_EQ(env.getBalance(alice.id(), USD), USD.amount(10) - amountWithRate);
+        EXPECT_EQ(env.getBalance(bob.id(), USD), USD.amount(1));
+    }
+
+    // Test out-of-bounds legacy transfer rates (4.0 and 4.294967295)
+    // These require direct ledger modification since the transactor blocks them
+    for (std::uint32_t const transferRate : {4000000000U, 4294967295U})
+    {
+        TxTest env;
+        env.createAccount(gw, XRP(10000), asfDefaultRipple);
+        env.createAccount(alice, XRP(10000), asfDefaultRipple);
+        env.createAccount(bob, XRP(10000), asfDefaultRipple);
+        env.close();
+
+        // Set up trust lines
+        EXPECT_EQ(
+            env.submit(transactions::TrustSetBuilder{alice}.setLimitAmount(USD.amount(10)), alice)
+                .ter,
+            tesSUCCESS);
+        EXPECT_EQ(
+            env.submit(transactions::TrustSetBuilder{bob}.setLimitAmount(USD.amount(10)), bob).ter,
+            tesSUCCESS);
+        env.close();
+
+        // Set an acceptable transfer rate first (we'll hack it later)
+        EXPECT_EQ(
+            env.submit(
+                   transactions::AccountSetBuilder{gw}.setTransferRate(
+                       static_cast<std::uint32_t>(2.0 * QUALITY_ONE)),
+                   gw)
+                .ter,
+            tesSUCCESS);
+        env.close();
+
+        // Directly modify the ledger to set an out-of-bounds transfer rate
+        // This bypasses the transactor's validation
+        auto& view = env.getOpenLedger();
+        auto slePtr = view.read(keylet::account(gw.id()));
+        ASSERT_NE(slePtr, nullptr);
+        auto sleCopy = std::make_shared<SLE>(*slePtr);
+        (*sleCopy)[sfTransferRate] = transferRate;
+        view.rawReplace(sleCopy);
+
+        // Calculate the amount with the legacy transfer rate
+        auto const amount = USD.amount(1);
+        auto const amountWithRate = multiply(amount, Rate(transferRate));
+
+        // Gateway pays alice 10 USD
+        EXPECT_EQ(
+            env.submit(transactions::PaymentBuilder{gw, alice, USD.amount(10)}, gw).ter,
+            tesSUCCESS);
+
+        // Alice pays bob 1 USD
+        EXPECT_EQ(
+            env.submit(
+                   transactions::PaymentBuilder{alice, bob, amount}.setSendMax(USD.amount(10)),
+                   alice)
+                .ter,
+            tesSUCCESS);
+
+        // Check balances
+        EXPECT_EQ(env.getBalance(alice.id(), USD), USD.amount(10) - amountWithRate);
+        EXPECT_EQ(env.getBalance(bob.id(), USD), amount);
+    }
+}
+
+}  // namespace xrpl::test

src/tests/libxrpl/tx/main.cpp

@@ -0,0 +1,8 @@
+#include <gtest/gtest.h>
+
+int
+main(int argc, char** argv)
+{
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}

src/xrpld/app/ledger/InboundLedger.h

@@ -9,6 +9,7 @@
 
 #include <mutex>
 #include <set>
+#include <string_view>
 #include <utility>
 
 namespace xrpl {
@@ -131,16 +132,16 @@ private:
     processData(std::shared_ptr<Peer> peer, protocol::TMLedgerData& data);
 
     bool
-    takeHeader(std::string const& data);
+    takeHeader(std::string_view data);
 
     void
-    receiveNode(protocol::TMLedgerData& packet, SHAMapAddNode&);
+    receiveNode(protocol::TMLedgerData& packet, SHAMapAddNode& san);
 
     bool
-    takeTxRootNode(Slice const& data, SHAMapAddNode&);
+    takeTxRootNode(std::string_view data, SHAMapAddNode& san);
 
     bool
-    takeAsRootNode(Slice const& data, SHAMapAddNode&);
+    takeAsRootNode(std::string_view data, SHAMapAddNode& san);
 
     std::vector<uint256>
     neededTxHashes(int max, SHAMapSyncFilter* filter) const;

src/xrpld/app/ledger/detail/InboundLedger.cpp

@@ -4,6 +4,7 @@
 #include <xrpld/app/ledger/InboundLedgers.h>
 #include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpld/app/ledger/TransactionStateSF.h>
+#include <xrpld/app/ledger/detail/LedgerNodeHelpers.h>
 #include <xrpld/app/ledger/detail/TimeoutCounter.h>
 #include <xrpld/app/main/Application.h>
 #include <xrpld/overlay/Message.h>
@@ -42,8 +43,8 @@
 #include <mutex>
 #include <random>
 #include <sstream>
-#include <stdexcept>
 #include <string>
+#include <string_view>
 #include <tuple>
 #include <unordered_map>
 #include <utility>
@@ -793,7 +794,7 @@ InboundLedger::filterNodes(
 */
 // data must not have hash prefix
 bool
-InboundLedger::takeHeader(std::string const& data)
+InboundLedger::takeHeader(std::string_view data)
 {
     // Return value: true=normal, false=bad data
     JLOG(journal_.trace()) << "got header acquiring ledger " << hash_;
@@ -881,20 +882,31 @@ InboundLedger::receiveNode(protocol::TMLedgerData& packet, SHAMapAddNode& san)
     {
         auto const f = filter.get();
 
-        for (auto const& node : packet.nodes())
+        for (auto const& ledgerNode : packet.nodes())
         {
-            auto const nodeID = deserializeSHAMapNodeID(node.nodeid());
+            auto treeNode = getTreeNode(ledgerNode.nodedata());
+            if (!treeNode)
+            {
+                JLOG(journal_.warn()) << "Got invalid node data";
+                san.incInvalid();
+                return;
+            }
 
+            auto const nodeID = getSHAMapNodeID(ledgerNode, treeNode);
             if (!nodeID)
-                throw std::runtime_error("data does not properly deserialize");
+            {
+                JLOG(journal_.warn()) << "Got invalid node id";
+                san.incInvalid();
+                return;
+            }
 
             if (nodeID->isRoot())
             {
-                san += map.addRootNode(rootHash, makeSlice(node.nodedata()), f);
+                san += map.addRootNode(rootHash, std::move(treeNode), f);
             }
             else
             {
-                san += map.addKnownNode(*nodeID, makeSlice(node.nodedata()), f);
+                san += map.addKnownNode(*nodeID, std::move(treeNode), f);
             }
 
             if (!san.isGood())
@@ -934,7 +946,7 @@ InboundLedger::receiveNode(protocol::TMLedgerData& packet, SHAMapAddNode& san)
     Call with a lock
 */
 bool
-InboundLedger::takeAsRootNode(Slice const& data, SHAMapAddNode& san)
+InboundLedger::takeAsRootNode(std::string_view data, SHAMapAddNode& san)
 {
     if (failed_ || mHaveState)
     {
@@ -950,9 +962,17 @@ InboundLedger::takeAsRootNode(Slice const& data, SHAMapAddNode& san)
         // LCOV_EXCL_STOP
     }
 
+    auto treeNode = getTreeNode(data);
+    if (!treeNode)
+    {
+        JLOG(journal_.warn()) << "Got invalid node data";
+        san.incInvalid();
+        return false;
+    }
+
     AccountStateSF filter(mLedger->stateMap().family().db(), app_.getLedgerMaster());
-    san +=
-        mLedger->stateMap().addRootNode(SHAMapHash{mLedger->header().accountHash}, data, &filter);
+    san += mLedger->stateMap().addRootNode(
+        SHAMapHash{mLedger->header().accountHash}, std::move(treeNode), &filter);
     return san.isGood();
 }
 
@@ -960,7 +980,7 @@ InboundLedger::takeAsRootNode(Slice const& data, SHAMapAddNode& san)
     Call with a lock
 */
 bool
-InboundLedger::takeTxRootNode(Slice const& data, SHAMapAddNode& san)
+InboundLedger::takeTxRootNode(std::string_view data, SHAMapAddNode& san)
 {
     if (failed_ || mHaveTransactions)
     {
@@ -976,8 +996,17 @@ InboundLedger::takeTxRootNode(Slice const& data, SHAMapAddNode& san)
         // LCOV_EXCL_STOP
     }
 
+    auto treeNode = getTreeNode(data);
+    if (!treeNode)
+    {
+        JLOG(journal_.warn()) << "Got invalid node data";
+        san.incInvalid();
+        return false;
+    }
+
     TransactionStateSF filter(mLedger->txMap().family().db(), app_.getLedgerMaster());
-    san += mLedger->txMap().addRootNode(SHAMapHash{mLedger->header().txHash}, data, &filter);
+    san += mLedger->txMap().addRootNode(
+        SHAMapHash{mLedger->header().txHash}, std::move(treeNode), &filter);
     return san.isGood();
 }
 
@@ -1074,13 +1103,13 @@ InboundLedger::processData(std::shared_ptr<Peer> peer, protocol::TMLedgerData& p
             }
 
             if (!mHaveState && (packet.nodes().size() > 1) &&
-                !takeAsRootNode(makeSlice(packet.nodes(1).nodedata()), san))
+                !takeAsRootNode(packet.nodes(1).nodedata(), san))
             {
                 JLOG(journal_.warn()) << "Included AS root invalid";
             }
 
             if (!mHaveTransactions && (packet.nodes().size() > 2) &&
-                !takeTxRootNode(makeSlice(packet.nodes(2).nodedata()), san))
+                !takeTxRootNode(packet.nodes(2).nodedata(), san))
             {
                 JLOG(journal_.warn()) << "Included TX root invalid";
             }
@@ -1111,13 +1140,13 @@ InboundLedger::processData(std::shared_ptr<Peer> peer, protocol::TMLedgerData& p
 
         ScopedLockType const sl(mtx_);
 
-        // Verify node IDs and data are complete
-        for (auto const& node : packet.nodes())
+        // Verify nodes are complete
+        for (auto const& ledgerNode : packet.nodes())
         {
-            if (!node.has_nodeid() || !node.has_nodedata())
+            if (!validateLedgerNode(ledgerNode))
             {
-                JLOG(journal_.warn()) << "Got bad node";
-                peer->charge(Resource::feeMalformedRequest, "ledger_data bad node");
+                JLOG(journal_.warn()) << "Got malformed ledger node";
+                peer->charge(Resource::feeMalformedRequest, "ledgerNode");
                 return -1;
             }
         }

src/xrpld/app/ledger/detail/InboundLedgers.cpp

@@ -2,13 +2,13 @@
 
 #include <xrpld/app/ledger/InboundLedger.h>
 #include <xrpld/app/ledger/LedgerMaster.h>
+#include <xrpld/app/ledger/detail/LedgerNodeHelpers.h>
 #include <xrpld/app/main/Application.h>
 #include <xrpld/overlay/PeerSet.h>
 
 #include <xrpl/basics/Blob.h>
 #include <xrpl/basics/DecayingSample.h>
 #include <xrpl/basics/Log.h>
-#include <xrpl/basics/Slice.h>
 #include <xrpl/basics/UnorderedContainers.h>
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/basics/scope.h>
@@ -248,23 +248,20 @@ public:
         Serializer s;
         try
         {
-            for (int i = 0; i < packet_ptr->nodes().size(); ++i)
+            for (auto const& ledgerNode : packet_ptr->nodes())
             {
-                auto const& node = packet_ptr->nodes(i);
-
-                if (!node.has_nodeid() || !node.has_nodedata())
+                if (!validateLedgerNode(ledgerNode))
                     return;
 
-                auto newNode = SHAMapTreeNode::makeFromWire(makeSlice(node.nodedata()));
-
-                if (!newNode)
+                auto const treeNode = getTreeNode(ledgerNode.nodedata());
+                if (!treeNode)
                     return;
 
                 s.erase();
-                newNode->serializeWithPrefix(s);
+                treeNode->serializeWithPrefix(s);
 
                 app_.getLedgerMaster().addFetchPack(
-                    newNode->getHash().as_uint256(), std::make_shared<Blob>(s.begin(), s.end()));
+                    treeNode->getHash().as_uint256(), std::make_shared<Blob>(s.begin(), s.end()));
             }
         }
         catch (std::exception const&)  // NOLINT(bugprone-empty-catch)

src/xrpld/app/ledger/detail/InboundTransactions.cpp

@@ -1,11 +1,11 @@
 #include <xrpld/app/ledger/InboundTransactions.h>
 
+#include <xrpld/app/ledger/detail/LedgerNodeHelpers.h>
 #include <xrpld/app/ledger/detail/TransactionAcquire.h>
 #include <xrpld/app/main/Application.h>
 #include <xrpld/overlay/PeerSet.h>
 
 #include <xrpl/basics/Log.h>
-#include <xrpl/basics/Slice.h>
 #include <xrpl/basics/UnorderedContainers.h>
 #include <xrpl/beast/insight/Collector.h>
 #include <xrpl/protocol/RippleLedgerHash.h>
@@ -14,6 +14,7 @@
 #include <xrpl/shamap/SHAMap.h>
 #include <xrpl/shamap/SHAMapMissingNode.h>
 #include <xrpl/shamap/SHAMapNodeID.h>
+#include <xrpl/shamap/SHAMapTreeNode.h>
 
 #include <xrpl.pb.h>
 
@@ -144,29 +145,38 @@ public:
             return;
         }
 
-        std::vector<std::pair<SHAMapNodeID, Slice>> data;
+        std::vector<std::pair<SHAMapNodeID, SHAMapTreeNodePtr>> data;
         data.reserve(packet.nodes().size());
 
-        for (auto const& node : packet.nodes())
+        for (auto const& ledgerNode : packet.nodes())
         {
-            if (!node.has_nodeid() || !node.has_nodedata())
+            if (!validateLedgerNode(ledgerNode))
             {
-                peer->charge(Resource::feeMalformedRequest, "ledger_data");
+                JLOG(j_.warn()) << "Got malformed ledger node";
+                peer->charge(Resource::feeMalformedRequest, "ledgerNode");
                 return;
             }
 
-            auto const id = deserializeSHAMapNodeID(node.nodeid());
-
-            if (!id)
+            auto treeNode = getTreeNode(ledgerNode.nodedata());
+            if (!treeNode)
             {
-                peer->charge(Resource::feeInvalidData, "ledger_data");
+                JLOG(j_.warn()) << "Got invalid node data";
+                peer->charge(Resource::feeInvalidData, "node_data");
                 return;
             }
 
-            data.emplace_back(*id, makeSlice(node.nodedata()));
+            auto const nodeID = getSHAMapNodeID(ledgerNode, treeNode);
+            if (!nodeID)
+            {
+                JLOG(j_.warn()) << "Got invalid node id";
+                peer->charge(Resource::feeInvalidData, "node_id");
+                return;
+            }
+
+            data.emplace_back(*nodeID, std::move(treeNode));
         }
 
-        if (!ta->takeNodes(data, peer).isUseful())
+        if (!ta->takeNodes(std::move(data), peer).isUseful())
             peer->charge(Resource::feeUselessData, "ledger_data not useful");
     }
 

src/xrpld/app/ledger/detail/LedgerNodeHelpers.cpp

@@ -0,0 +1,91 @@
+#include <xrpld/app/ledger/detail/LedgerNodeHelpers.h>
+
+#include <xrpl/basics/Slice.h>
+#include <xrpl/basics/safe_cast.h>
+#include <xrpl/beast/utility/instrumentation.h>
+#include <xrpl/shamap/SHAMap.h>
+#include <xrpl/shamap/SHAMapLeafNode.h>
+#include <xrpl/shamap/SHAMapNodeID.h>
+#include <xrpl/shamap/SHAMapTreeNode.h>
+
+#include <xrpl.pb.h>
+
+#include <exception>
+#include <optional>
+#include <string_view>
+
+namespace xrpl {
+
+bool
+validateLedgerNode(protocol::TMLedgerNode const& ledgerNode)
+{
+    if (!ledgerNode.has_nodedata())
+        return false;
+
+    if (ledgerNode.has_nodeid())
+        return !ledgerNode.has_id() && !ledgerNode.has_depth();
+
+    return ledgerNode.has_id() ||
+        (ledgerNode.has_depth() && ledgerNode.depth() <= SHAMap::leafDepth);
+}
+
+SHAMapTreeNodePtr
+getTreeNode(std::string_view data)
+{
+    auto const slice = makeSlice(data);
+    try
+    {
+        return SHAMapTreeNode::makeFromWire(slice);
+    }
+    catch (std::exception const&)
+    {
+        return {};
+    }
+}
+
+std::optional<SHAMapNodeID>
+getSHAMapNodeID(protocol::TMLedgerNode const& ledgerNode, SHAMapTreeNodePtr const& treeNode)
+{
+    if (ledgerNode.has_id() || ledgerNode.has_depth())
+    {
+        if (treeNode->isInner())
+        {
+            if (!ledgerNode.has_id())
+                return std::nullopt;
+
+            return deserializeSHAMapNodeID(ledgerNode.id());
+        }
+
+        if (treeNode->isLeaf())
+        {
+            if (!ledgerNode.has_depth())
+                return std::nullopt;
+
+            auto const key =
+                safe_downcast<SHAMapLeafNode const*>(treeNode.get())->peekItem()->key();
+            return SHAMapNodeID::createID(ledgerNode.depth(), key);
+        }
+
+        UNREACHABLE("xrpl::getSHAMapNodeID : tree node is neither inner nor leaf");
+        return std::nullopt;
+    }
+
+    if (!ledgerNode.has_nodeid())
+        return std::nullopt;
+
+    auto nodeID = deserializeSHAMapNodeID(ledgerNode.nodeid());
+    if (!nodeID.has_value())
+        return std::nullopt;
+
+    if (treeNode->isLeaf())
+    {
+        auto const key = safe_downcast<SHAMapLeafNode const*>(treeNode.get())->peekItem()->key();
+        auto const expected_id = SHAMapNodeID::createID(static_cast<int>(nodeID->getDepth()), key);
+        if (nodeID->getNodeID() != expected_id.getNodeID())
+            return std::nullopt;
+    }
+
+    return nodeID;
+}
+
+}  // namespace xrpl

src/xrpld/app/ledger/detail/LedgerNodeHelpers.h

@@ -0,0 +1,72 @@
+#pragma once
+
+#include <xrpl/basics/IntrusivePointer.h>
+#include <xrpl/shamap/SHAMapNodeID.h>
+#include <xrpl/shamap/SHAMapTreeNode.h>
+
+#include <optional>
+#include <string_view>
+
+namespace protocol {
+class TMLedgerNode;
+}  // namespace protocol
+
+namespace xrpl {
+
+/**
+ * @brief Validates a ledger node proto message.
+ *
+ * This function checks whether a ledger node has the expected fields (for non-ledger base data):
+ * - The node must have `nodedata`.
+ * - If the legacy `nodeid` field is present then the new `id` and `depth` fields must not be
+ *   present.
+ * - If the new `id` or `depth` fields are present (it is a oneof field, so only one of the two can
+ *   be set) then the legacy `nodeid` must not be present.
+ * - If the `depth` field is present then it must be between 0 and SHAMap::leafDepth (inclusive).
+ *
+ * @param ledgerNode The ledger node to validate.
+ * @return true if the ledger node has the expected fields, false otherwise.
+ */
+[[nodiscard]] bool
+validateLedgerNode(protocol::TMLedgerNode const& ledgerNode);
+
+/**
+ * @brief Deserializes a SHAMapTreeNode from wire format data.
+ *
+ * This function attempts to create a SHAMapTreeNode from the provided data string. If the data is
+ * malformed or deserialization fails, the function returns a nullptr instead of throwing an
+ * exception.
+ *
+ * @param data The serialized node data in wire format.
+ * @return The deserialized tree node if successful, or a nullptr if deserialization fails.
+ */
+[[nodiscard]] SHAMapTreeNodePtr
+getTreeNode(std::string_view data);
+
+/**
+ * @brief Extracts or reconstructs the SHAMapNodeID from a ledger node proto message.
+ *
+ * This function retrieves the SHAMapNodeID for a tree node, with behavior that depends on which
+ * field is set and the node type (inner vs. leaf).
+ *
+ * When the legacy `nodeid` field is set in the message:
+ * - For all nodes: Deserializes the node ID from the field.
+ * - For leaf nodes: Validates that the node ID is consistent with the leaf's key.
+ *
+ * When the new `id` or `depth` field is set in the message:
+ * - For inner nodes: Deserializes the node ID from the `id` field.
+ * - For leaf nodes: Reconstructs the node ID using both the depth from the `depth` field and the
+ *   key from the leaf node's item.
+ * Note that root nodes may be inner nodes or leaf nodes.
+ *
+ * @param ledgerNode The validated protocol message containing the ledger node data.
+ * @param treeNode The deserialized tree node (inner or leaf node).
+ * @return An optional containing the node ID if extraction/reconstruction succeeds, or std::nullopt
+ *         if the required fields are missing or validation fails.
+ * @note This function expects that the caller has already validated the ledger node by calling the
+ *       `validateLedgerNode` function and obtained a valid tree node by calling `getTreeNode`.
+ */
+[[nodiscard]] std::optional<SHAMapNodeID>
+getSHAMapNodeID(protocol::TMLedgerNode const& ledgerNode, SHAMapTreeNodePtr const& treeNode);
+
+}  // namespace xrpl

src/xrpld/app/ledger/detail/TransactionAcquire.cpp

@@ -7,13 +7,13 @@
 #include <xrpld/overlay/PeerSet.h>
 
 #include <xrpl/basics/Log.h>
-#include <xrpl/basics/Slice.h>
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/core/Job.h>
 #include <xrpl/server/NetworkOPs.h>
 #include <xrpl/shamap/SHAMap.h>
 #include <xrpl/shamap/SHAMapAddNode.h>
 #include <xrpl/shamap/SHAMapMissingNode.h>
+#include <xrpl/shamap/SHAMapTreeNode.h>
 
 #include <xrpl.pb.h>
 
@@ -173,7 +173,7 @@ TransactionAcquire::trigger(std::shared_ptr<Peer> const& peer)
 
 SHAMapAddNode
 TransactionAcquire::takeNodes(
-    std::vector<std::pair<SHAMapNodeID, Slice>> const& data,
+    std::vector<std::pair<SHAMapNodeID, SHAMapTreeNodePtr>> data,
     std::shared_ptr<Peer> const& peer)
 {
     ScopedLockType const sl(mtx_);

src/xrpld/app/ledger/detail/TransactionAcquire.h

@@ -21,8 +21,8 @@ public:
 
     SHAMapAddNode
     takeNodes(
-        std::vector<std::pair<SHAMapNodeID, Slice>> const& data,
-        std::shared_ptr<Peer> const&);
+        std::vector<std::pair<SHAMapNodeID, SHAMapTreeNodePtr>> data,
+        std::shared_ptr<Peer> const& peer);
 
     void
     init(int startPeers);

src/xrpld/overlay/Peer.h

@@ -17,6 +17,7 @@ enum class ProtocolFeature {
     ValidatorListPropagation,
     ValidatorList2Propagation,
     LedgerReplay,
+    LedgerNodeDepth,
 };
 
 /** Represents a peer connection in the overlay. */

src/xrpld/overlay/detail/PeerImp.cpp

@@ -61,6 +61,7 @@
 #include <xrpl/server/Handoff.h>
 #include <xrpl/server/LoadFeeTrack.h>
 #include <xrpl/server/NetworkOPs.h>
+#include <xrpl/shamap/SHAMap.h>
 #include <xrpl/shamap/SHAMapNodeID.h>
 #include <xrpl/tx/apply.h>
 
@@ -565,6 +566,8 @@ PeerImp::supportsFeature(ProtocolFeature f) const
             return protocol_ >= make_protocol(2, 1);
         case ProtocolFeature::ValidatorList2Propagation:
             return protocol_ >= make_protocol(2, 2);
+        case ProtocolFeature::LedgerNodeDepth:
+            return protocol_ >= make_protocol(2, 3);
         case ProtocolFeature::LedgerReplay:
             return ledgerReplayEnabled_;
     }
@@ -1611,7 +1614,8 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMGetLedger> const& m)
         }
     }
 
-    // Verify ledger node IDs
+    // Verify and parse ledger node IDs
+    std::vector<SHAMapNodeID> nodeIDs;
     if (itype != protocol::liBASE)
     {
         if (m->nodeids_size() <= 0)
@@ -1620,13 +1624,16 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMGetLedger> const& m)
             return;
         }
 
+        nodeIDs.reserve(m->nodeids_size());
         for (auto const& nodeId : m->nodeids())
         {
-            if (deserializeSHAMapNodeID(nodeId) == std::nullopt)
+            auto parsed = deserializeSHAMapNodeID(nodeId);
+            if (!parsed)
             {
                 badData("Invalid SHAMap node ID");
                 return;
             }
+            nodeIDs.push_back(std::move(*parsed));
         }
     }
 
@@ -1649,10 +1656,11 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMGetLedger> const& m)
 
     // Queue a job to process the request
     std::weak_ptr<PeerImp> const weak = shared_from_this();
-    app_.getJobQueue().addJob(jtLEDGER_REQ, "RcvGetLedger", [weak, m]() {
-        if (auto peer = weak.lock())
-            peer->processLedgerRequest(m);
-    });
+    app_.getJobQueue().addJob(
+        jtLEDGER_REQ, "RcvGetLedger", [weak, m, nodeIDs = std::move(nodeIDs)]() mutable {
+            if (auto peer = weak.lock())
+                peer->processLedgerRequest(m, std::move(nodeIDs));
+        });
 }
 
 void
@@ -3361,7 +3369,9 @@ PeerImp::getTxSet(std::shared_ptr<protocol::TMGetLedger> const& m) const
 }
 
 void
-PeerImp::processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m)
+PeerImp::processLedgerRequest(
+    std::shared_ptr<protocol::TMGetLedger> const& m,
+    std::vector<SHAMapNodeID> nodeIDs)
 {
     // Do not resource charge a peer responding to a relay
     if (!m->has_requestcookie())
@@ -3446,26 +3456,25 @@ PeerImp::processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m)
     }
 
     // Add requested node data to reply
-    if (m->nodeids_size() > 0)
+    if (!nodeIDs.empty())
     {
         std::uint32_t const defaultDepth = isHighLatency() ? 2 : 1;
         auto const queryDepth{m->has_querydepth() ? m->querydepth() : defaultDepth};
 
-        std::vector<std::pair<SHAMapNodeID, Blob>> data;
+        std::vector<SHAMapNodeData> data;
+        auto const useLedgerNodeDepth = supportsFeature(ProtocolFeature::LedgerNodeDepth);
 
-        for (int i = 0;
-             i < m->nodeids_size() && ledgerData.nodes_size() < Tuning::softMaxReplyNodes;
-             ++i)
+        for (auto const& nodeID : nodeIDs)
         {
-            auto const shaMapNodeId{deserializeSHAMapNodeID(m->nodeids(i))};
+            if (ledgerData.nodes_size() >= Tuning::softMaxReplyNodes)
+                break;
 
             data.clear();
             data.reserve(Tuning::softMaxReplyNodes);
 
             try
             {
-                // NOLINTNEXTLINE(bugprone-unchecked-optional-access) nodeids checked in onGetLedger
-                if (map->getNodeFat(*shaMapNodeId, data, fatLeaves, queryDepth))
+                if (map->getNodeFat(nodeID, data, fatLeaves, queryDepth))
                 {
                     JLOG(p_journal_.trace())
                         << "processLedgerRequest: getNodeFat got " << data.size() << " nodes";
@@ -3474,9 +3483,26 @@ PeerImp::processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m)
                     {
                         if (ledgerData.nodes_size() >= Tuning::hardMaxReplyNodes)
                             break;
+
                         protocol::TMLedgerNode* node{ledgerData.add_nodes()};
-                        node->set_nodeid(d.first.getRawString());
-                        node->set_nodedata(d.second.data(), d.second.size());
+
+                        node->set_nodedata(d.data.data(), d.data.size());
+
+                        // When the LedgerNodeDepth protocol feature is not supported by the peer,
+                        // we always set the `nodeid` field. However, when it is supported then we
+                        // set the `id` field for inner nodes and the `depth` field for leaf nodes.
+                        if (!useLedgerNodeDepth)
+                        {
+                            node->set_nodeid(d.nodeID.getRawString());
+                        }
+                        else if (d.isLeaf)
+                        {
+                            node->set_depth(d.nodeID.getDepth());
+                        }
+                        else
+                        {
+                            node->set_id(d.nodeID.getRawString());
+                        }
                     }
                 }
                 else
@@ -3515,7 +3541,7 @@ PeerImp::processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m)
                     info += ", no hash specified";
 
                 JLOG(p_journal_.warn())
-                    << "processLedgerRequest: getNodeFat with nodeId " << *shaMapNodeId
+                    << "processLedgerRequest: getNodeFat with nodeId " << nodeID
                     << " and ledger info type " << info << " throws exception: " << e.what();
             }
         }

src/xrpld/overlay/detail/PeerImp.h

@@ -14,6 +14,7 @@
 #include <xrpl/protocol/STTx.h>
 #include <xrpl/protocol/STValidation.h>
 #include <xrpl/resource/Fees.h>
+#include <xrpl/shamap/SHAMapNodeID.h>
 
 #include <boost/circular_buffer.hpp>
 #include <boost/endian/conversion.hpp>
@@ -792,7 +793,9 @@ private:
     getTxSet(std::shared_ptr<protocol::TMGetLedger> const& m) const;
 
     void
-    processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m);
+    processLedgerRequest(
+        std::shared_ptr<protocol::TMGetLedger> const& m,
+        std::vector<SHAMapNodeID> nodeIDs);
 };
 
 //------------------------------------------------------------------------------

src/xrpld/overlay/detail/ProtocolVersion.cpp

@@ -28,6 +28,7 @@ namespace xrpl {
 constexpr ProtocolVersion const supportedProtocolList[]{
     {2, 1},
     {2, 2},
+    {2, 3},
 };
 
 // This ugly construct ensures that supportedProtocolList is sorted in strictly