Remove depth, do not include node ID for leaf nodes

This commit is contained in:
Bart
2026-02-13 17:05:05 -05:00
parent b2371c4c02
commit 832a7e7e4a
8 changed files with 70 additions and 44 deletions

View File

@@ -244,8 +244,7 @@ message TMGetObjectByHash {
message TMLedgerNode {
required bytes nodedata = 1;
optional bytes nodeid = 2; // Used when fixLedgerNodeDepth is not active.
optional uint32 nodedepth = 3; // Used when fixLedgerNodeDepth is active.
optional bytes nodeid = 2; // missing for ledger base data
}
enum TMLedgerInfoType {

View File

@@ -15,7 +15,7 @@
// Add new amendments to the top of this list.
// Keep it sorted in reverse chronological order.
XRPL_FIX (LedgerNodeDepth, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FIX (LedgerNodeID, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FIX (PermissionedDomainInvariant, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (BatchInnerSigs, Supported::yes, VoteBehavior::DefaultNo)

View File

@@ -113,7 +113,7 @@ 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));
}

View File

@@ -830,28 +830,26 @@ InboundLedger::receiveNode(protocol::TMLedgerData& packet, SHAMapAddNode& san)
}
auto const node_slice = makeSlice(ledger_node.nodedata());
auto const tree_node_opt = getTreeNode(node_slice);
if (!tree_node_opt)
auto const tree_node = getTreeNode(node_slice);
if (!tree_node)
{
JLOG(journal_.warn()) << "Got invalid node data";
san.incInvalid();
return;
}
auto const tree_node = *tree_node_opt;
auto const node_id_opt = getSHAMapNodeID(app_, ledger_node, tree_node);
if (!node_id_opt)
auto const node_id = getSHAMapNodeID(app_, ledger_node, *tree_node);
if (!node_id)
{
JLOG(journal_.warn()) << "Got invalid node id";
san.incInvalid();
return;
}
auto const& node_id = *node_id_opt;
if (node_id.isRoot())
if (node_id->isRoot())
san += map.addRootNode(rootHash, node_slice, f);
else
san += map.addKnownNode(node_id, node_slice, f);
san += map.addKnownNode(*node_id, node_slice, f);
if (!san.isGood())
{

View File

@@ -222,16 +222,16 @@ public:
return;
auto const node_slice = makeSlice(ledger_node.nodedata());
auto const tree_node_opt = getTreeNode(node_slice);
if (!tree_node_opt)
auto const tree_node = getTreeNode(node_slice);
if (!tree_node)
return;
auto const tree_node = *tree_node_opt;
auto const& tn = *tree_node;
s.erase();
tree_node->serializeWithPrefix(s);
tn->serializeWithPrefix(s);
app_.getLedgerMaster().addFetchPack(
tree_node->getHash().as_uint256(), std::make_shared<Blob>(s.begin(), s.end()));
tn->getHash().as_uint256(), std::make_shared<Blob>(s.begin(), s.end()));
}
}
catch (std::exception const&)

View File

@@ -141,25 +141,23 @@ public:
}
auto const node_slice = makeSlice(ledger_node.nodedata());
auto const tree_node_opt = getTreeNode(node_slice);
if (!tree_node_opt)
auto const tree_node = getTreeNode(node_slice);
if (!tree_node)
{
JLOG(j_.warn()) << "Got invalid node data";
peer->charge(Resource::feeInvalidData, "node_data");
return;
}
auto const tree_node = *tree_node_opt;
auto const node_id_opt = getSHAMapNodeID(app_, ledger_node, tree_node);
if (!node_id_opt)
auto const node_id = getSHAMapNodeID(app_, ledger_node, *tree_node);
if (!node_id)
{
JLOG(j_.warn()) << "Got invalid node id";
peer->charge(Resource::feeInvalidData, "node_id");
return;
}
auto const& node_id = *node_id_opt;
data.emplace_back(std::make_pair(node_id, node_slice));
data.emplace_back(std::make_pair(*node_id, node_slice));
}
if (!ta->takeNodes(data, peer).isUseful())

View File

@@ -4,6 +4,7 @@
#include <xrpld/app/misc/AmendmentTable.h>
#include <xrpl/basics/IntrusivePointer.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/shamap/SHAMapLeafNode.h>
#include <xrpl/shamap/SHAMapNodeID.h>
#include <xrpl/shamap/SHAMapTreeNode.h>
@@ -16,9 +17,14 @@ validateLedgerNode(Application& app, protocol::TMLedgerNode const& ledger_node)
if (!ledger_node.has_nodedata())
return false;
if (app.getAmendmentTable().isEnabled(fixLedgerNodeDepth))
return ledger_node.has_nodedepth() && ledger_node.nodedepth() < 65;
// When the amendment is enabled, we expect the node ID to be present in the ledger node for
// inner nodes, while we expect it to not be present in the ledger node for leaf nodes. However,
// at this point we don't yet know whether the node is an inner node or a leaf node, so we
// allow both cases.
if (app.getAmendmentTable().isEnabled(fixLedgerNodeID))
return true;
// When the amendment is disabled, we expect the node ID to always be present.
return ledger_node.has_nodeid();
}
@@ -41,26 +47,48 @@ getSHAMapNodeID(
protocol::TMLedgerNode const& ledger_node,
intr_ptr::SharedPtr<SHAMapTreeNode> const& tree_node)
{
SHAMapNodeID node_id;
if (app.getAmendmentTable().isEnabled(fixLedgerNodeDepth))
// When the amendment is enabled, we can get the node ID directly from the ledger node for inner
// nodes, while we compute it for leaf nodes.
if (app.getAmendmentTable().isEnabled(fixLedgerNodeID))
{
node_id = SHAMapNodeID(ledger_node.nodedepth(), tree_node->getHash().as_uint256());
}
else
{
auto const nid = deserializeSHAMapNodeID(ledger_node.nodeid());
if (!nid)
return std::nullopt;
node_id = *nid;
if (tree_node->isInner())
{
XRPL_ASSERT(ledger_node.has_nodeid(), "xrpl::getSHAMapNodeID : node ID is present");
if (!ledger_node.has_nodeid())
return std::nullopt;
return deserializeSHAMapNodeID(ledger_node.nodeid());
}
if (tree_node->isLeaf())
{
XRPL_ASSERT(!ledger_node.has_nodeid(), "xrpl::getSHAMapNodeID : node ID is not present");
if (ledger_node.has_nodeid())
return std::nullopt;
auto const key = static_cast<SHAMapLeafNode const*>(tree_node.get())->peekItem()->key();
return SHAMapNodeID::createID(SHAMap::leafDepth, key);
}
return std::nullopt;
}
// For leaf nodes, verify that the node ID is actually the same as what the node ID should be,
// given the position of the node in the SHAMap.
// When the amendment is disabled, we expect the node ID to always be present. For leaf nodes
// we perform an extra check to ensure the node's position in the tree is consistent with its
// content.
XRPL_ASSERT(ledger_node.has_nodeid(), "xrpl::getSHAMapNodeID : node ID is present");
if (!ledger_node.has_nodeid())
return std::nullopt;
auto const node_id = deserializeSHAMapNodeID(ledger_node.nodeid());
if (!node_id)
return std::nullopt;
if (tree_node->isLeaf())
{
auto const nodeKey = dynamic_cast<SHAMapLeafNode const*>(tree_node.get())->peekItem()->key();
auto const expectedID = SHAMapNodeID::createID(static_cast<int>(node_id.getDepth()), nodeKey);
if (node_id.getNodeID() != expectedID.getNodeID())
auto const key = static_cast<SHAMapLeafNode const*>(tree_node.get())->peekItem()->key();
auto const expected_id = SHAMapNodeID::createID(static_cast<int>(node_id->getDepth()), key);
if (node_id->getNodeID() != expected_id.getNodeID())
return std::nullopt;
}

View File

@@ -3154,9 +3154,12 @@ PeerImp::processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m)
break;
protocol::TMLedgerNode* node{ledgerData.add_nodes()};
node->set_nodedata(d.second.data(), d.second.size());
if (app_.getAmendmentTable().isEnabled(fixLedgerNodeDepth))
node->set_nodedepth(d.first.getDepth());
else
// When the amendment is enabled, we only include the node ID in the ledger node for inner
// nodes, while we do not include it for leaf nodes as it can be calculated from the data. When
// the amendment is disabled, we always need to include the node ID in the ledger node.
if ((app_.getAmendmentTable().isEnabled(fixLedgerNodeID) &&
d.first.getDepth() != SHAMap::leafDepth) ||
!app_.getAmendmentTable().isEnabled(fixLedgerNodeID))
node->set_nodeid(d.first.getRawString());
}
}