Use new protocol version instead of amendment, add tests

This commit is contained in:
Bart
2026-02-28 13:54:00 -05:00
parent c9aa1094a7
commit 29b0076fa8
11 changed files with 366 additions and 312 deletions

View File

@@ -245,10 +245,10 @@ message TMGetObjectByHash {
message TMLedgerNode {
required bytes nodedata = 1;
// Used when fixLedgerNodeID is disabled.
// Used when protocol version <2.3.
optional bytes nodeid = 2; // missing for ledger base data
// Used when fixLedgerNodeID is enabled. Neither value is set for ledger base data.
// 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.

View File

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

View File

@@ -1,24 +1,24 @@
#include <test/jtx.h>
#include <test/jtx/Env.h>
#include <test/shamap/common.h>
#include <test/unit_test/SuiteJournal.h>
#include <xrpld/app/ledger/detail/LedgerNodeHelpers.h>
#include <xrpl/basics/random.h>
#include <xrpl/beast/unit_test.h>
#include <xrpl/proto/xrpl.pb.h>
#include <xrpl/protocol/messages.h>
#include <xrpl/shamap/SHAMap.h>
#include <xrpl/shamap/SHAMapAccountStateLeafNode.h>
#include <xrpl/shamap/SHAMapInnerNode.h>
#include <xrpl/shamap/SHAMapItem.h>
#include <xrpl/shamap/SHAMapLeafNode.h>
#include <xrpl/shamap/SHAMapTreeNode.h>
namespace xrpl {
namespace tests {
class LedgerNodeHelpers_test : public beast::unit_test::suite
{
// Helper to create a simple SHAMapItem for testing
boost::intrusive_ptr<SHAMapItem>
// Helper function to create a simple SHAMapItem for testing.
static boost::intrusive_ptr<SHAMapItem>
makeTestItem(std::uint32_t seed)
{
Serializer s;
@@ -28,152 +28,181 @@ class LedgerNodeHelpers_test : public beast::unit_test::suite
return make_shamapitem(s.getSHA512Half(), s.slice());
}
// Helper to serialize a tree node to wire format
std::string
// Helper function to serialize a tree node to wire format.
static std::string
serializeNode(intr_ptr::SharedPtr<SHAMapTreeNode> const& node)
{
Serializer s;
node->serializeWithPrefix(s);
node->serializeForWire(s);
auto const slice = s.slice();
return std::string(reinterpret_cast<char const*>(slice.data()), slice.size());
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 field.
testcase("validateLedgerNode");
using namespace test::jtx;
/*// Test with amendment disabled.
// Invalid: missing all fields.
{
Env env{*this, testable_amendments() - fixLedgerNodeID};
auto& app = env.app();
// Valid node with nodeid.
protocol::TMLedgerNode node1;
node1.set_nodedata("test_data");
node1.set_nodeid("test_nodeid");
BEAST_EXPECT(validateLedgerNode(app, node1));
// Invalid: missing nodedata.
protocol::TMLedgerNode node2;
node2.set_nodeid("test_nodeid");
BEAST_EXPECT(!validateLedgerNode(app, node2));
// Invalid: has new field (id).
protocol::TMLedgerNode node3;
node3.set_nodedata("test_data");
node3.set_nodeid("test_nodeid");
node3.set_id("test_id");
BEAST_EXPECT(!validateLedgerNode(app, node3));
// Invalid: has new field (depth).
protocol::TMLedgerNode node4;
node4.set_nodedata("test_data");
node4.set_nodeid("test_nodeid");
node4.set_depth(5);
BEAST_EXPECT(!validateLedgerNode(app, node4));
protocol::TMLedgerNode node;
BEAST_EXPECT(!validateLedgerNode(node));
}
// Test with amendment enabled.
// Invalid: missing `nodedata` field.
{
Env env{*this, testable_amendments() | fixLedgerNodeID};
auto& app = env.app();
protocol::TMLedgerNode node;
node.set_nodeid("test_nodeid");
BEAST_EXPECT(!validateLedgerNode(node));
}
// Valid inner node with id.
protocol::TMLedgerNode node1;
node1.set_nodedata("test_data");
node1.set_id("test_id");
BEAST_EXPECT(validateLedgerNode(app, node1));
// Invalid: missing `nodedata` field.
{
protocol::TMLedgerNode node;
node.set_id("test_nodeid");
BEAST_EXPECT(!validateLedgerNode(node));
}
// Valid leaf node with depth.
protocol::TMLedgerNode node2;
node2.set_nodedata("test_data");
node2.set_depth(5);
BEAST_EXPECT(validateLedgerNode(app, node2));
// Invalid: missing `nodedata` field.
{
protocol::TMLedgerNode node;
node.set_depth(1);
BEAST_EXPECT(!validateLedgerNode(node));
}
// Valid leaf node at max depth.
protocol::TMLedgerNode node3;
node3.set_nodedata("test_data");
node3.set_depth(SHAMap::leafDepth);
BEAST_EXPECT(validateLedgerNode(app, node3));
// Valid: legacy `nodeid` field.
{
protocol::TMLedgerNode node;
node.set_nodedata("test_data");
node.set_nodeid("test_nodeid");
BEAST_EXPECT(validateLedgerNode(node));
}
// Invalid: depth exceeds max depth.
protocol::TMLedgerNode node4;
node4.set_nodedata("test_data");
node4.set_depth(SHAMap::leafDepth + 1);
BEAST_EXPECT(!validateLedgerNode(app, node4));
// 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 legacy field (nodeid).
protocol::TMLedgerNode node5;
node5.set_nodedata("test_data");
node5.set_nodeid("test_nodeid");
BEAST_EXPECT(!validateLedgerNode(app, node5));
// 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));
}
// Invalid: missing both id and depth.
protocol::TMLedgerNode node6;
node6.set_nodedata("test_data");
BEAST_EXPECT(!validateLedgerNode(app, node6));
// Valid: new `id` feld.
{
protocol::TMLedgerNode node;
node.set_nodedata("test_data");
node.set_id("test_id");
BEAST_EXPECT(validateLedgerNode(node));
}
// Invalid: missing nodedata.
protocol::TMLedgerNode node7;
node7.set_id("test_id");
BEAST_EXPECT(!validateLedgerNode(app, node7));
}*/
// Valid: new `depth` field.
{
protocol::TMLedgerNode node;
node.set_nodedata("test_data");
node.set_depth(5);
BEAST_EXPECT(validateLedgerNode(node));
}
// Invalid: `depth` is less than minimum depth.
{
protocol::TMLedgerNode node;
node.set_nodedata("test_data");
node.set_depth(0);
BEAST_EXPECT(!validateLedgerNode(node));
}
// Valid: `depth` at minimum depth.
{
protocol::TMLedgerNode node;
node.set_nodedata("test_data");
node.set_depth(1);
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
void
testGetTreeNode()
{
testcase("getTreeNode");
using namespace test::jtx;
test::SuiteJournal journal("LedgerNodeHelpers_test", *this);
TestNodeFamily f(journal);
// Test with valid inner node
// Valid: inner node.
{
auto innerNode = std::make_shared<SHAMapInnerNode>(1);
auto serialized = serializeNode(innerNode);
auto result = getTreeNode(serialized);
auto const rootNode = intr_ptr::make_shared<SHAMapInnerNode>(1);
auto const rootData = serializeNode(rootNode);
auto const result = getTreeNode(rootData);
BEAST_EXPECT(result.has_value());
BEAST_EXPECT((*result)->isInner());
}
// Test with valid leaf node
// Valid: leaf node.
{
auto item = makeTestItem(12345);
auto leafNode =
std::make_shared<SHAMapLeafNode>(item, SHAMapNodeType::tnACCOUNT_STATE);
auto serialized = serializeNode(leafNode);
auto result = getTreeNode(serialized);
auto const leafItem = makeTestItem(12345);
auto const leafNode =
intr_ptr::make_shared<SHAMapAccountStateLeafNode>(std::move(leafItem), 1);
auto const leafData = serializeNode(leafNode);
auto result = getTreeNode(leafData);
BEAST_EXPECT(result.has_value());
BEAST_EXPECT((*result)->isLeaf());
}
// Test with invalid data - empty string
// Invalid: empty data.
{
auto result = getTreeNode("");
auto const result = getTreeNode("");
BEAST_EXPECT(!result.has_value());
}
// Test with invalid data - garbage data
// Invalid: garbage data.
{
std::string garbage = "This is not a valid serialized node!";
auto result = getTreeNode(garbage);
auto const result = getTreeNode("invalid");
BEAST_EXPECT(!result.has_value());
}
// Test with malformed data - truncated
// Invalid: truncated data.
{
auto item = makeTestItem(54321);
auto leafNode =
std::make_shared<SHAMapLeafNode>(item, SHAMapNodeType::tnACCOUNT_STATE);
auto serialized = serializeNode(leafNode);
// Truncate the data
serialized = serialized.substr(0, 5);
auto result = getTreeNode(serialized);
auto const leafItem = makeTestItem(54321);
auto const leafNode =
intr_ptr::make_shared<SHAMapAccountStateLeafNode>(std::move(leafItem), 1);
// Truncate the data to trigger an exception in SHAMapTreeNode::makeAccountState when
// the data is used to deserialize the node.
uint256 tag;
auto const leafData = serializeNode(leafNode).substr(0, tag.bytes - 1);
auto const result = getTreeNode(leafData);
BEAST_EXPECT(!result.has_value());
}
}
@@ -183,166 +212,206 @@ class LedgerNodeHelpers_test : public beast::unit_test::suite
{
testcase("getSHAMapNodeID");
using namespace test::jtx;
test::SuiteJournal journal("LedgerNodeHelpers_test", *this);
TestNodeFamily f(journal);
auto item = makeTestItem(99999);
auto leafNode =
std::make_shared<SHAMapLeafNode>(item, SHAMapNodeType::tnACCOUNT_STATE);
auto innerNode = std::make_shared<SHAMapInnerNode>(1);
// Test with amendment disabled
{
Env env{*this};
auto& app = env.app();
// Tests using a root node (=inner node at depth 0).
auto const rootNode = intr_ptr::make_shared<SHAMapInnerNode>(1);
auto const rootData = serializeNode(rootNode);
auto const rootNodeID = SHAMapNodeID{};
// Test with leaf node - valid nodeid
// Valid: legacy `nodeid` field.
{
auto nodeID = SHAMapNodeID::createID(5, item->key());
protocol::TMLedgerNode ledgerNode;
ledgerNode.set_nodedata("test_data");
ledgerNode.set_nodeid(nodeID.getRawString());
auto result = getSHAMapNodeID(app, ledgerNode, leafNode);
protocol::TMLedgerNode node;
node.set_nodedata(rootData);
node.set_nodeid(rootNodeID.getRawString());
auto const result = getSHAMapNodeID(node, rootNode);
BEAST_EXPECT(result.has_value());
BEAST_EXPECT(*result == nodeID);
BEAST_EXPECT(*result == rootNodeID);
}
// Test with leaf node - invalid nodeid (wrong depth)
// Valid: new `id` field.
{
auto nodeID = SHAMapNodeID::createID(7, item->key());
protocol::TMLedgerNode ledgerNode;
ledgerNode.set_nodedata("test_data");
ledgerNode.set_nodeid(nodeID.getRawString());
protocol::TMLedgerNode node;
node.set_nodedata(rootData);
node.set_id(rootNodeID.getRawString());
auto const result = getSHAMapNodeID(node, rootNode);
BEAST_EXPECT(result.has_value());
BEAST_EXPECT(*result == rootNodeID);
}
auto wrongNodeID = SHAMapNodeID::createID(5, item->key());
ledgerNode.set_nodeid(wrongNodeID.getRawString());
auto result = getSHAMapNodeID(app, ledgerNode, leafNode);
// Should fail because nodeid doesn't match the leaf's key at the
// given depth
// Invalid: Missing `nodeid` or `id` field.
{
protocol::TMLedgerNode node;
node.set_nodedata(rootData);
auto const result = getSHAMapNodeID(node, rootNode);
BEAST_EXPECT(!result.has_value());
}
// Test with inner node
// Invalid: `depth` field is present but should not be used for inner nodes.
{
auto nodeID = SHAMapNodeID::createID(3, uint256{});
protocol::TMLedgerNode ledgerNode;
ledgerNode.set_nodedata("test_data");
ledgerNode.set_nodeid(nodeID.getRawString());
auto result = getSHAMapNodeID(app, ledgerNode, innerNode);
BEAST_EXPECT(result.has_value());
BEAST_EXPECT(*result == nodeID);
}
// Test missing nodeid
{
protocol::TMLedgerNode ledgerNode;
ledgerNode.set_nodedata("test_data");
auto result = getSHAMapNodeID(app, ledgerNode, leafNode);
protocol::TMLedgerNode node;
node.set_nodedata(rootData);
node.set_depth(0);
auto const result = getSHAMapNodeID(node, rootNode);
BEAST_EXPECT(!result.has_value());
}
}
// Test with amendment enabled
{
Env env{*this, testable_amendments() | fixLedgerNodeID};
auto& app = env.app();
// Tests using an inner node at arbitrary depth.
auto const innerNode = intr_ptr::make_shared<SHAMapInnerNode>(1);
auto const innerData = serializeNode(innerNode);
auto const innerDepth = 3;
auto const innerNodeID = SHAMapNodeID::createID(innerDepth, uint256{});
// Test with leaf node - valid depth
// Valid: legacy `nodeid` field.
{
std::uint32_t depth = 5;
protocol::TMLedgerNode ledgerNode;
ledgerNode.set_nodedata("test_data");
ledgerNode.set_depth(depth);
auto result = getSHAMapNodeID(app, ledgerNode, leafNode);
protocol::TMLedgerNode node;
node.set_nodedata(innerData);
node.set_nodeid(innerNodeID.getRawString());
auto const result = getSHAMapNodeID(node, innerNode);
BEAST_EXPECT(result.has_value());
auto expectedID = SHAMapNodeID::createID(depth, item->key());
BEAST_EXPECT(*result == expectedID);
BEAST_EXPECT(*result == innerNodeID);
}
// Test with leaf node - missing depth
// Valid: new `id` field.
{
protocol::TMLedgerNode ledgerNode;
ledgerNode.set_nodedata("test_data");
protocol::TMLedgerNode node;
node.set_nodedata(innerData);
node.set_id(innerNodeID.getRawString());
auto const result = getSHAMapNodeID(node, innerNode);
BEAST_EXPECT(result.has_value());
BEAST_EXPECT(*result == innerNodeID);
}
auto result = getSHAMapNodeID(app, ledgerNode, leafNode);
// Invalid: Missing `nodeid` or `id` field.
{
protocol::TMLedgerNode node;
node.set_nodedata(innerData);
auto const result = getSHAMapNodeID(node, innerNode);
BEAST_EXPECT(!result.has_value());
}
// Test with inner node - valid id
// Invalid: new `depth` field should not be used for inner nodes.
{
auto nodeID = SHAMapNodeID::createID(3, uint256{});
protocol::TMLedgerNode ledgerNode;
ledgerNode.set_nodedata("test_data");
ledgerNode.set_id(nodeID.getRawString());
auto result = getSHAMapNodeID(app, ledgerNode, innerNode);
BEAST_EXPECT(result.has_value());
BEAST_EXPECT(*result == nodeID);
}
// Test with inner node - missing id
{
protocol::TMLedgerNode ledgerNode;
ledgerNode.set_nodedata("test_data");
auto result = getSHAMapNodeID(app, ledgerNode, innerNode);
protocol::TMLedgerNode node;
node.set_nodedata(innerData);
node.set_depth(innerDepth);
auto const result = getSHAMapNodeID(node, innerNode);
BEAST_EXPECT(!result.has_value());
}
// Test that nodeid is rejected when amendment is enabled
{
auto nodeID = SHAMapNodeID::createID(5, item->key());
protocol::TMLedgerNode ledgerNode;
ledgerNode.set_nodedata("test_data");
ledgerNode.set_nodeid(nodeID.getRawString());
// Should fail validation before getSHAMapNodeID is called,
// but let's verify getSHAMapNodeID behavior
// Note: validateLedgerNode should be called first in practice
BEAST_EXPECT(!validateLedgerNode(app, ledgerNode));
}
// Test with root node (inner node at depth 0)
{
auto rootNode = std::make_shared<SHAMapInnerNode>(1);
auto nodeID = SHAMapNodeID{}; // root node ID
protocol::TMLedgerNode ledgerNode;
ledgerNode.set_nodedata("test_data");
ledgerNode.set_id(nodeID.getRawString());
auto result = getSHAMapNodeID(app, ledgerNode, rootNode);
BEAST_EXPECT(result.has_value());
BEAST_EXPECT(*result == nodeID);
}
// Test with leaf at maximum depth
{
std::uint32_t depth = SHAMap::leafDepth;
protocol::TMLedgerNode ledgerNode;
ledgerNode.set_nodedata("test_data");
ledgerNode.set_depth(depth);
auto result = getSHAMapNodeID(app, ledgerNode, leafNode);
BEAST_EXPECT(result.has_value());
auto expectedID = SHAMapNodeID::createID(depth, item->key());
BEAST_EXPECT(*result == expectedID);
}
}
}*/
{
// Tests using leaf nodes at various depths, but with the same key.
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 result = getSHAMapNodeID(ledgerNode, leafNode);
BEAST_EXPECT(result.has_value());
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 result = getSHAMapNodeID(ledgerNode, leafNode);
BEAST_EXPECT(!result.has_value());
}
// Valid: new `depth` field at minimum depth.
// Note that we do not test a depth less than the minimum depth, because the proto
// message is assumed to have been validated by the time the getSHAMapNodeID function is
// called.
{
auto const leafDepth = 1;
auto const leafID = SHAMapNodeID::createID(leafDepth, leafKey);
protocol::TMLedgerNode node;
node.set_nodedata(leafData);
node.set_depth(leafDepth);
auto result = getSHAMapNodeID(node, leafNode);
BEAST_EXPECT(result.has_value());
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 result = getSHAMapNodeID(ledgerNode, leafNode);
BEAST_EXPECT(result.has_value());
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 result = getSHAMapNodeID(node, leafNode);
BEAST_EXPECT(result.has_value());
BEAST_EXPECT(*result == leafID);
}
// Invalid: Missing `depth` field.
{
protocol::TMLedgerNode node;
node.set_nodedata(leafData);
auto const result = getSHAMapNodeID(node, leafNode);
BEAST_EXPECT(!result.has_value());
}
// 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 result = getSHAMapNodeID(ledgerNode, leafNode);
BEAST_EXPECT(!result.has_value());
}
}
}
public:
void
run() override
{
testValidateLedgerNode();
// testGetTreeNode();
// testGetSHAMapNodeID();
testGetTreeNode();
testGetSHAMapNodeID();
}
};

View File

@@ -840,7 +840,7 @@ InboundLedger::receiveNode(protocol::TMLedgerData& packet, SHAMapAddNode& san)
for (auto const& ledger_node : packet.nodes())
{
if (!validateLedgerNode(app_, ledger_node))
if (!validateLedgerNode(ledger_node))
{
JLOG(journal_.warn()) << "Got malformed ledger node";
san.incInvalid();
@@ -855,7 +855,7 @@ InboundLedger::receiveNode(protocol::TMLedgerData& packet, SHAMapAddNode& san)
return;
}
auto const nodeID = getSHAMapNodeID(app_, ledger_node, *treeNode);
auto const nodeID = getSHAMapNodeID(ledger_node, *treeNode);
if (!nodeID)
{
JLOG(journal_.warn()) << "Got invalid node id";
@@ -1092,7 +1092,7 @@ InboundLedger::processData(std::shared_ptr<Peer> peer, protocol::TMLedgerData& p
// Verify nodes are complete
for (auto const& ledger_node : packet.nodes())
{
if (!validateLedgerNode(app_, ledger_node))
if (!validateLedgerNode(ledger_node))
{
JLOG(journal_.warn()) << "Got malformed ledger node";
peer->charge(Resource::feeMalformedRequest, "ledger_node");

View File

@@ -223,7 +223,7 @@ public:
Serializer s;
for (auto const& ledger_node : packet_ptr->nodes())
{
if (!validateLedgerNode(app_, ledger_node))
if (!validateLedgerNode(ledger_node))
return;
auto const treeNode = getTreeNode(ledger_node.nodedata());

View File

@@ -138,7 +138,7 @@ public:
for (auto const& ledger_node : packet.nodes())
{
if (!validateLedgerNode(app_, ledger_node))
if (!validateLedgerNode(ledger_node))
{
JLOG(j_.warn()) << "Got malformed ledger node";
peer->charge(Resource::feeMalformedRequest, "ledger_node");
@@ -153,7 +153,7 @@ public:
return;
}
auto const nodeID = getSHAMapNodeID(app_, ledger_node, *treeNode);
auto const nodeID = getSHAMapNodeID(ledger_node, *treeNode);
if (!nodeID)
{
JLOG(j_.warn()) << "Got invalid node id";

View File

@@ -1,36 +1,31 @@
#include <xrpld/app/ledger/detail/LedgerNodeHelpers.h>
#include <xrpld/app/main/Application.h>
#include <xrpl/basics/IntrusivePointer.h>
#include <xrpl/basics/Slice.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/ledger/AmendmentTable.h>
#include <xrpl/protocol/messages.h>
#include <xrpl/shamap/SHAMap.h>
#include <xrpl/shamap/SHAMapLeafNode.h>
#include <xrpl/shamap/SHAMapNodeID.h>
#include <xrpl/shamap/SHAMapTreeNode.h>
#include <optional>
#include <string>
namespace xrpl {
bool
validateLedgerNode(Application& app, protocol::TMLedgerNode const& ledger_node)
validateLedgerNode(protocol::TMLedgerNode const& ledger_node)
{
if (!ledger_node.has_nodedata())
return false;
if (app.getAmendmentTable().isEnabled(fixLedgerNodeID))
{
// Note that we cannot confirm here whether the node is actually an
// inner or leaf node, and will need to perform additional checks
// separately.
if (ledger_node.has_nodeid())
return false;
if (ledger_node.has_id())
return true;
return ledger_node.has_depth() && ledger_node.depth() <= SHAMap::leafDepth;
}
if (ledger_node.has_nodeid())
return !ledger_node.has_id() && !ledger_node.has_depth();
if (ledger_node.has_id() || ledger_node.has_depth())
return false;
return ledger_node.has_nodeid();
return ledger_node.has_id() ||
(ledger_node.has_depth() && ledger_node.depth() >= 1 &&
ledger_node.depth() <= SHAMap::leafDepth);
}
std::optional<intr_ptr::SharedPtr<SHAMapTreeNode>>
@@ -39,7 +34,10 @@ getTreeNode(std::string const& data)
auto const slice = makeSlice(data);
try
{
return SHAMapTreeNode::makeFromWire(slice);
auto treeNode = SHAMapTreeNode::makeFromWire(slice);
if (!treeNode)
return std::nullopt;
return treeNode;
}
catch (std::exception const&)
{
@@ -49,12 +47,10 @@ getTreeNode(std::string const& data)
std::optional<SHAMapNodeID>
getSHAMapNodeID(
Application& app,
protocol::TMLedgerNode const& ledger_node,
intr_ptr::SharedPtr<SHAMapTreeNode> const& treeNode)
{
// When the amendment is enabled and a node depth is present, we can calculate the node ID.
if (app.getAmendmentTable().isEnabled(fixLedgerNodeID))
if (ledger_node.has_id() || ledger_node.has_depth())
{
if (treeNode->isInner())
{
@@ -79,9 +75,6 @@ getSHAMapNodeID(
return std::nullopt;
}
// 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;

View File

@@ -1,82 +1,71 @@
#pragma once
#include <xrpld/app/main/Application.h>
#include <xrpl/basics/IntrusivePointer.h>
#include <xrpl/shamap/SHAMapLeafNode.h>
#include <xrpl/shamap/SHAMapNodeID.h>
#include <xrpl/shamap/SHAMapTreeNode.h>
#include <optional>
namespace protocol {
class TMLedgerNode;
} // namespace protocol
namespace xrpl {
/**
* @brief Validates a ledger node based on the fixLedgerNodeID amendment status.
* @brief Validates a ledger node proto message.
*
* This function checks whether a ledger node has the expected fields based on
* whether the fixLedgerNodeID amendment is enabled. The validation rules differ
* depending on the amendment state:
*
* When the amendment is enabled:
* This function checks whether a ledger node has the expected fields:
* - The node must have `nodedata`.
* - The legacy `nodeid` field must NOT be present.
* - For inner nodes: the `id` field must be present.
* - For leaf nodes: the `depth` field must be present and <= SHAMap::leafDepth.
* - 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 1 and SHAMap::leafDepth (inclusive).
*
* When the amendment is disabled:
* - The node must have `nodedata`.
* - The `nodeid` field must be present.
* - The new `id` and `depth` fields must NOT be present.
*
* @param app The application instance used to check amendment status.
* @param ledger_node The ledger node to validate.
* @return true if the ledger node has the expected fields, false otherwise.
*/
bool
validateLedgerNode(Application& app, protocol::TMLedgerNode const& ledger_node);
validateLedgerNode(protocol::TMLedgerNode const& ledger_node);
/**
* @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 std::nullopt instead of throwing an exception.
* This function attempts to create a SHAMapTreeNode from the provided data string. If the data is
* malformed or deserialization fails, the function returns std::nullopt instead of throwing an
* exception.
*
* @param data The serialized node data in wire format.
* @return An optional containing the deserialized tree node if successful, or
* std::nullopt if deserialization fails.
* @return An optional containing the deserialized tree node if successful, or std::nullopt if
* deserialization fails.
*/
std::optional<intr_ptr::SharedPtr<SHAMapTreeNode>>
[[nodiscard]] std::optional<intr_ptr::SharedPtr<SHAMapTreeNode>>
getTreeNode(std::string const& data);
/**
* @brief Extracts or reconstructs the SHAMapNodeID from a ledger node.
* @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 the fixLedgerNodeID amendment status and the node type (inner vs.
* leaf).
* 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 fixLedgerNodeID amendment is enabled:
* - For inner nodes: Deserializes the node ID from the `ledger_node.id` field.
* Note that root nodes are also inner nodes.
* - For leaf nodes: Reconstructs the node ID using both the depth from the
* `ledger_node.depth` field and the key from the leaf node's item.
* 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 amendment is disabled:
* - For all nodes: Deserializes the node ID from the `ledger_node.nodeid`
* 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. Note that root nodes are also
* inner nodes.
* - For leaf nodes: Reconstructs the node ID using both the depth from the `depth` field and the
* key from the leaf node's item.
*
* @param app The application instance used to check amendment status.
* @param ledger_node The protocol message containing the ledger node data.
* @param ledger_node 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.
* @return An optional containing the node ID if extraction/reconstruction succeeds, or std::nullopt
* if the required fields are missing or validation fails.
*/
std::optional<SHAMapNodeID>
[[nodiscard]] std::optional<SHAMapNodeID>
getSHAMapNodeID(
Application& app,
protocol::TMLedgerNode const& ledger_node,
intr_ptr::SharedPtr<SHAMapTreeNode> const& treeNode);

View File

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

View File

@@ -472,6 +472,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_;
}
@@ -3266,11 +3268,11 @@ PeerImp::processLedgerRequest(std::shared_ptr<protocol::TMGetLedger> const& m)
auto const& node_data = std::get<1>(d);
node->set_nodedata(node_data.data(), node_data.size());
// When the amendment is disabled, we always set the node ID. However, when
// the amendment is enabled, we only set it for inner nodes, while for leaf
// nodes we set the node depth instead.
// When the LedgerNode protocol feature is not supported by the peer, we
// always set the node ID. However, when it is supported then we only set it
// for inner nodes, while for leaf nodes we set the node depth instead.
auto const& nodeID = std::get<0>(d);
if (!app_.getAmendmentTable().isEnabled(fixLedgerNodeID))
if (!supportsFeature(ProtocolFeature::LedgerNodeDepth))
node->set_nodeid(nodeID.getRawString());
else if (std::get<2>(d))
node->set_depth(nodeID.getDepth());

View File

@@ -21,7 +21,8 @@ namespace xrpl {
constexpr ProtocolVersion const supportedProtocolList[]
{
{2, 1},
{2, 2}
{2, 2},
{2, 3},
};
// clang-format on