ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-06-03 16:56:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Address AI feedback

Browse Source
This commit is contained in:
Bart
2026-05-03 17:48:25 -04:00
parent 1a35abbb43
commit 7912d112e7
7 changed files with 38 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
src/libxrpl/shamap/SHAMapSync.cpp
View File

@@ -512,12 +512,8 @@ SHAMap::addRootNode(
SHAMapTreeNodePtr rootNode,
SHAMapSyncFilter const* filter)
{
XRPL_ASSERT(cowid_ >= 1, "xrpl::SHAMap::addRootNode : valid cowid");
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())
@@ -527,7 +523,6 @@ SHAMap::addRootNode(
return SHAMapAddNode::duplicate();
}
XRPL_ASSERT(cowid_ >= 1, "xrpl::SHAMap::addRootNode : valid cowid");
if (rootNode->getHash() != hash)
{
JLOG(journal_.warn()) << "Corrupt node received";
@@ -560,17 +555,7 @@ SHAMap::addKnownNode(
SHAMapSyncFilter const* filter)
{
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())
{

35
src/test/app/LedgerNodeHelpers_test.cpp
View File

@@ -44,11 +44,10 @@ class LedgerNodeHelpers_test : public beast::unit_test::Suite
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");
auto const validID = SHAMapNodeID::createID(3, uint256{}).getRawString();
// Invalid: missing all fields.
{
protocol::TMLedgerNode const node;
@@ -58,14 +57,14 @@ class LedgerNodeHelpers_test : public beast::unit_test::Suite
// Invalid: missing `nodedata` field.
{
protocol::TMLedgerNode node;
node.set_nodeid("test_nodeid");
node.set_nodeid(validID);
BEAST_EXPECT(!validateLedgerNode(node));
}
// Invalid: missing `nodedata` field.
{
protocol::TMLedgerNode node;
node.set_id("test_nodeid");
node.set_id(validID);
BEAST_EXPECT(!validateLedgerNode(node));
}
@@ -80,16 +79,24 @@ class LedgerNodeHelpers_test : public beast::unit_test::Suite
{
protocol::TMLedgerNode node;
node.set_nodedata("test_data");
node.set_nodeid("test_nodeid");
node.set_nodeid(validID);
BEAST_EXPECT(validateLedgerNode(node));
}
// Invalid: legacy `nodeid` field with garbage content.
{
protocol::TMLedgerNode node;
node.set_nodedata("test_data");
node.set_nodeid("garbage");
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");
node.set_nodeid(validID);
node.set_id(validID);
BEAST_EXPECT(!validateLedgerNode(node));
}
@@ -97,7 +104,7 @@ class LedgerNodeHelpers_test : public beast::unit_test::Suite
{
protocol::TMLedgerNode node;
node.set_nodedata("test_data");
node.set_nodeid("test_nodeid");
node.set_nodeid(validID);
node.set_depth(5);
BEAST_EXPECT(!validateLedgerNode(node));
}
@@ -106,10 +113,18 @@ class LedgerNodeHelpers_test : public beast::unit_test::Suite
{
protocol::TMLedgerNode node;
node.set_nodedata("test_data");
node.set_id("test_id");
node.set_id(validID);
BEAST_EXPECT(validateLedgerNode(node));
}
// Invalid: new `id` field with garbage content.
{
protocol::TMLedgerNode node;
node.set_nodedata("test_data");
node.set_id("garbage");
BEAST_EXPECT(!validateLedgerNode(node));
}
// Valid: new `depth` field.
{
protocol::TMLedgerNode node;

2
src/xrpld/app/ledger/detail/InboundLedger.cpp
View File

@@ -1150,7 +1150,7 @@ InboundLedger::processData(std::shared_ptr<Peer> peer, protocol::TMLedgerData& p
if (!validateLedgerNode(ledgerNode))
{
JLOG(journal_.warn()) << "Got malformed ledger node";
peer->charge(Resource::kFEE_MALFORMED_REQUEST, "ledgerNode");
peer->charge(Resource::kFEE_MALFORMED_REQUEST, "ledger_node");
return -1;
}
}

2
src/xrpld/app/ledger/detail/InboundTransactions.cpp
View File

@@ -155,7 +155,7 @@ public:
if (!validateLedgerNode(ledgerNode))
{
JLOG(j_.warn()) << "Got malformed ledger node";
peer->charge(Resource::kFEE_MALFORMED_REQUEST, "ledgerNode");
peer->charge(Resource::kFEE_MALFORMED_REQUEST, "ledger_node");
return;
}

9
src/xrpld/app/ledger/detail/LedgerNodeHelpers.cpp
View File

@@ -23,10 +23,13 @@ validateLedgerNode(protocol::TMLedgerNode const& ledgerNode)
return false;
if (ledgerNode.has_nodeid())
return !ledgerNode.has_id() && !ledgerNode.has_depth();
return !ledgerNode.has_id() && !ledgerNode.has_depth() &&
deserializeSHAMapNodeID(ledgerNode.nodeid()).has_value();
return ledgerNode.has_id() ||
(ledgerNode.has_depth() && ledgerNode.depth() <= SHAMap::kLEAF_DEPTH);
if (ledgerNode.has_id())
return deserializeSHAMapNodeID(ledgerNode.id()).has_value();
return ledgerNode.has_depth() && ledgerNode.depth() <= SHAMap::kLEAF_DEPTH;
}
SHAMapTreeNodePtr

5
src/xrpld/app/ledger/detail/LedgerNodeHelpers.h
View File

@@ -19,10 +19,11 @@ namespace xrpl {
* 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.
* present, and `nodeid` must be a valid serialized SHAMapNodeID.
* - 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).
* - If the `id` field is present then it must be a valid serialized SHAMapNodeID.
* - If the `depth` field is present then it must be between 0 and SHAMap::kLEAF_DEPTH (inclusive).
*
* @param ledgerNode The ledger node to validate.
* @return true if the ledger node has the expected fields, false otherwise.

2
src/xrpld/app/ledger/detail/TransactionAcquire.cpp
View File

@@ -199,7 +199,7 @@ TransactionAcquire::takeNodes(
ConsensusTransSetSF sf(app_, app_.getTempNodeCache());
for (auto const& d : data)
for (auto& d : data)
{
if (d.first.isRoot())
{