perf: Replace node ID by depth in TMLedgerNode

bthomee · bthomee · commit 2d78d41f7bfc · 2026-02-11T15:55:16.000-05:00
diff --git a/include/xrpl/proto/xrpl.proto b/include/xrpl/proto/xrpl.proto
@@ -244,7 +244,8 @@ message TMGetObjectByHash {
 
 message TMLedgerNode {
   required bytes nodedata = 1;
-  optional bytes nodeid = 2;  // missing for ledger base data
+  optional bytes nodeid = 2 [deprecated = true];
+  optional uint32 nodedepth = 3;  // missing for ledger base data
 }
 
 enum TMLedgerInfoType {
diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro
@@ -15,6 +15,7 @@
 
 // Add new amendments to the top of this list.
 // Keep it sorted in reverse chronological order.
+XRPL_FIX    (LedgerNodeDepth,            Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX   (PermissionedDomainInvariant, Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (BatchInnerSigs,             Supported::yes, VoteBehavior::DefaultNo)
diff --git a/src/libxrpl/shamap/SHAMapSync.cpp b/src/libxrpl/shamap/SHAMapSync.cpp
@@ -547,25 +547,6 @@ SHAMap::addKnownNode(SHAMapNodeID const& node, Slice const& rawNode, SHAMapSyncF
             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 = static_cast<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:
diff --git a/src/xrpld/app/ledger/detail/InboundLedger.cpp b/src/xrpld/app/ledger/detail/InboundLedger.cpp
@@ -4,6 +4,7 @@
 #include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpld/app/ledger/TransactionStateSF.h>
 #include <xrpld/app/main/Application.h>
+#include <xrpld/app/misc/AmendmentTable.h>
 #include <xrpld/overlay/Overlay.h>
 
 #include <xrpl/basics/Log.h>
@@ -819,22 +820,54 @@ 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());
-
-            if (!nodeID)
-                throw std::runtime_error("data does not properly deserialize");
+            auto nodeSlice = makeSlice(ledgerNode.nodedata());
+            auto treeNode = SHAMapTreeNode::makeFromWire(nodeSlice);
+            if (!treeNode)
+            {
+                JLOG(journal_.warn()) << "Got invalid node data";
+                san.incInvalid();
+                return;
+            }
+            auto const nodeKey = static_cast<SHAMapLeafNode const*>(treeNode.get())->peekItem()->key();
 
-            if (nodeID->isRoot())
+            SHAMapNodeID nodeID;
+            if (app_.getAmendmentTable().isSupported(fixLedgerNodeDepth))
             {
-                san += map.addRootNode(rootHash, makeSlice(node.nodedata()), f);
+                nodeID = SHAMapNodeID::createID(ledgerNode.nodedepth(), nodeKey);
             }
             else
             {
-                san += map.addKnownNode(*nodeID, makeSlice(node.nodedata()), f);
+                auto const nid = deserializeSHAMapNodeID(ledgerNode.nodeid());
+                if (!nid)
+                {
+                    JLOG(journal_.warn()) << "Got invalid node id";
+                    san.incInvalid();
+                    return;
+                }
+                nodeID = *nid;
+
+                // For leaf nodes, verify that the passed-in node ID is actually
+                // the same as what the node ID should be, given the position of
+                // the node in the SHAMap.
+                if (treeNode->isLeaf())
+                {
+                    auto const expectedID = SHAMapNodeID::createID(nodeID.getDepth(), nodeKey);
+                    if (nodeID.getNodeID() != expectedID.getNodeID())
+                    {
+                        JLOG(journal_.warn()) << "Got invalid node id";
+                        san.incInvalid();
+                        return;
+                    }
+                }
             }
 
+            if (nodeID.isRoot())
+                san += map.addRootNode(rootHash, nodeSlice, f);
+            else
+                san += map.addKnownNode(nodeID, nodeSlice, f);
+
             if (!san.isGood())
             {
                 JLOG(journal_.warn()) << "Received bad node data";
@@ -1044,13 +1077,15 @@ InboundLedger::processData(std::shared_ptr<Peer> peer, protocol::TMLedgerData& p
 
         ScopedLockType 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 (!ledgerNode.has_nodedata() ||
+                (app_.getAmendmentTable().isSupported(fixLedgerNodeDepth) && !ledgerNode.has_nodedepth()) ||
+                (!app_.getAmendmentTable().isSupported(fixLedgerNodeDepth) && !ledgerNode.has_nodeid()))
             {
-                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, "ledger_node");
                 return -1;
             }
         }
diff --git a/src/xrpld/app/ledger/detail/InboundLedgers.cpp b/src/xrpld/app/ledger/detail/InboundLedgers.cpp
@@ -1,6 +1,7 @@
 #include <xrpld/app/ledger/InboundLedgers.h>
 #include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpld/app/main/Application.h>
+#include <xrpld/app/misc/AmendmentTable.h>
 #include <xrpld/app/misc/NetworkOPs.h>
 
 #include <xrpl/basics/DecayingSample.h>
@@ -217,21 +218,22 @@ class InboundLedgersImp : public InboundLedgers
         {
             for (int i = 0; i < packet_ptr->nodes().size(); ++i)
             {
-                auto const& node = packet_ptr->nodes(i);
+                auto const& ledgerNode = packet_ptr->nodes(i);
 
-                if (!node.has_nodeid() || !node.has_nodedata())
+                if (!ledgerNode.has_nodedata() ||
+                    (app_.getAmendmentTable().isSupported(fixLedgerNodeDepth) && !ledgerNode.has_nodedepth()) ||
+                    (!app_.getAmendmentTable().isSupported(fixLedgerNodeDepth) && !ledgerNode.has_nodeid()))
                     return;
 
-                auto newNode = SHAMapTreeNode::makeFromWire(makeSlice(node.nodedata()));
-
-                if (!newNode)
+                auto treeNode = SHAMapTreeNode::makeFromWire(makeSlice(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&)
diff --git a/src/xrpld/app/ledger/detail/InboundTransactions.cpp b/src/xrpld/app/ledger/detail/InboundTransactions.cpp
@@ -2,6 +2,7 @@
 #include <xrpld/app/ledger/InboundTransactions.h>
 #include <xrpld/app/ledger/detail/TransactionAcquire.h>
 #include <xrpld/app/main/Application.h>
+#include <xrpld/app/misc/AmendmentTable.h>
 #include <xrpld/app/misc/NetworkOPs.h>
 
 #include <xrpl/basics/Log.h>
@@ -130,23 +131,57 @@ class InboundTransactionsImp : public InboundTransactions
         std::vector<std::pair<SHAMapNodeID, Slice>> 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 (!ledgerNode.has_nodedata() ||
+                (app_.getAmendmentTable().isSupported(fixLedgerNodeDepth) && !ledgerNode.has_nodedepth()) ||
+                (!app_.getAmendmentTable().isSupported(fixLedgerNodeDepth) && !ledgerNode.has_nodeid()))
             {
-                peer->charge(Resource::feeMalformedRequest, "ledger_data");
+                JLOG(j_.warn()) << "Got malformed ledger node";
+                peer->charge(Resource::feeMalformedRequest, "ledger_node");
                 return;
             }
 
-            auto const id = deserializeSHAMapNodeID(node.nodeid());
-
-            if (!id)
+            auto nodeSlice = makeSlice(ledgerNode.nodedata());
+            auto treeNode = SHAMapTreeNode::makeFromWire(nodeSlice);
+            if (!treeNode)
             {
+                JLOG(j_.warn()) << "Got invalid node data";
                 peer->charge(Resource::feeInvalidData, "ledger_data");
                 return;
             }
+            auto const nodeKey = static_cast<SHAMapLeafNode const*>(treeNode.get())->peekItem()->key();
+
+            SHAMapNodeID nodeID;
+            if (app_.getAmendmentTable().isSupported(fixLedgerNodeDepth))
+            {
+                nodeID = SHAMapNodeID::createID(ledgerNode.nodedepth(), nodeKey);
+            }
+            else
+            {
+                auto const nid = deserializeSHAMapNodeID(ledgerNode.nodeid());
+                if (!nid)
+                {
+                    peer->charge(Resource::feeInvalidData, "ledger_id");
+                    return;
+                }
+                nodeID = *nid;
+
+                // For leaf nodes, verify that the passed-in node ID is actually
+                // the same as what the node ID should be, given the position of
+                // the node in the SHAMap.
+                if (treeNode->isLeaf())
+                {
+                    auto const expectedID = SHAMapNodeID::createID(nodeID.getDepth(), nodeKey);
+                    if (nodeID.getNodeID() != expectedID.getNodeID())
+                    {
+                        peer->charge(Resource::feeInvalidData, "ledger_id");
+                        return;
+                    }
+                }
+            }
 
-            data.emplace_back(std::make_pair(*id, makeSlice(node.nodedata())));
+            data.emplace_back(std::make_pair(nodeID, nodeSlice));
         }
 
         if (!ta->takeNodes(data, peer).isUseful())
diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp
@@ -3,6 +3,7 @@
 #include <xrpld/app/ledger/InboundTransactions.h>
 #include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpld/app/ledger/TransactionMaster.h>
+#include <xrpld/app/misc/AmendmentTable.h>
 #include <xrpld/app/misc/HashRouter.h>
 #include <xrpld/app/misc/LoadFeeTrack.h>
 #include <xrpld/app/misc/NetworkOPs.h>
@@ -3152,8 +3153,11 @@ 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());
+                        if (app_.getAmendmentTable().isSupported(fixLedgerNodeDepth))
+                            node->set_nodedepth(d.first.getDepth());
+                        else
+                            node->set_nodeid(d.first.getRawString());
                     }
                 }
                 else

Original file line number	Diff line number	Diff line change
`@@ -244,7 +244,8 @@ message TMGetObjectByHash {`
`244`	`244`
`245`	`245`	`message TMLedgerNode {`
`246`	`246`	`required bytes nodedata = 1;`
`247`		`- optional bytes nodeid = 2; // missing for ledger base data`
	`247`	`+ optional bytes nodeid = 2 [deprecated = true];`
	`248`	`+ optional uint32 nodedepth = 3; // missing for ledger base data`
`248`	`249`	`}`
`249`	`250`
`250`	`251`	`enum TMLedgerInfoType {`