From 35ffa71c0bae3e791f46f3811bb8a0c324178290 Mon Sep 17 00:00:00 2001 From: Bart <11445373+bthomee@users.noreply.github.com> Date: Mon, 11 May 2026 17:53:43 -0400 Subject: [PATCH] Review feedback --- src/test/app/LedgerNodeHelpers_test.cpp | 18 +++++++++--------- src/xrpld/app/ledger/detail/InboundLedger.cpp | 2 +- .../app/ledger/detail/InboundTransactions.cpp | 2 +- .../app/ledger/detail/LedgerNodeHelpers.cpp | 14 +++++++------- .../app/ledger/detail/LedgerNodeHelpers.h | 2 +- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/test/app/LedgerNodeHelpers_test.cpp b/src/test/app/LedgerNodeHelpers_test.cpp index 0e614d1ad5..02ae2584c2 100644 --- a/src/test/app/LedgerNodeHelpers_test.cpp +++ b/src/test/app/LedgerNodeHelpers_test.cpp @@ -235,7 +235,7 @@ class LedgerNodeHelpers_test : public beast::unit_test::Suite protocol::TMLedgerNode node; node.set_nodedata(innerData); node.set_nodeid(innerID.getRawString()); - auto const result = getSHAMapNodeID(node, innerNode); + auto const result = getSHAMapNodeID(node, *innerNode); BEAST_EXPECT(result == innerID); } @@ -247,7 +247,7 @@ class LedgerNodeHelpers_test : public beast::unit_test::Suite protocol::TMLedgerNode node; node.set_nodedata(innerData); node.set_id(innerID.getRawString()); - auto const result = getSHAMapNodeID(node, innerNode); + auto const result = getSHAMapNodeID(node, *innerNode); BEAST_EXPECT(result == innerID); } @@ -256,7 +256,7 @@ class LedgerNodeHelpers_test : public beast::unit_test::Suite protocol::TMLedgerNode node; node.set_nodedata(innerData); node.set_depth(10); - auto const result = getSHAMapNodeID(node, innerNode); + auto const result = getSHAMapNodeID(node, *innerNode); BEAST_EXPECT(!result); } } @@ -276,7 +276,7 @@ class LedgerNodeHelpers_test : public beast::unit_test::Suite protocol::TMLedgerNode ledgerNode; ledgerNode.set_nodedata(leafData); ledgerNode.set_nodeid(leafID.getRawString()); - auto const result = getSHAMapNodeID(ledgerNode, leafNode); + auto const result = getSHAMapNodeID(ledgerNode, *leafNode); BEAST_EXPECT(result == leafID); } @@ -288,7 +288,7 @@ class LedgerNodeHelpers_test : public beast::unit_test::Suite protocol::TMLedgerNode ledgerNode; ledgerNode.set_nodedata(leafData); ledgerNode.set_id(leafID.getRawString()); - auto const result = getSHAMapNodeID(ledgerNode, leafNode); + auto const result = getSHAMapNodeID(ledgerNode, *leafNode); BEAST_EXPECT(!result); } @@ -300,7 +300,7 @@ class LedgerNodeHelpers_test : public beast::unit_test::Suite protocol::TMLedgerNode node; node.set_nodedata(leafData); node.set_depth(kLeafDepth); - auto const result = getSHAMapNodeID(node, leafNode); + auto const result = getSHAMapNodeID(node, *leafNode); BEAST_EXPECT(result == leafID); } @@ -312,7 +312,7 @@ class LedgerNodeHelpers_test : public beast::unit_test::Suite protocol::TMLedgerNode ledgerNode; ledgerNode.set_nodedata(leafData); ledgerNode.set_depth(kLeafDepth); - auto const result = getSHAMapNodeID(ledgerNode, leafNode); + auto const result = getSHAMapNodeID(ledgerNode, *leafNode); BEAST_EXPECT(result == leafID); } @@ -327,7 +327,7 @@ class LedgerNodeHelpers_test : public beast::unit_test::Suite protocol::TMLedgerNode node; node.set_nodedata(leafData); node.set_depth(kLeafDepth); - auto const result = getSHAMapNodeID(node, leafNode); + auto const result = getSHAMapNodeID(node, *leafNode); BEAST_EXPECT(result == leafID); } @@ -344,7 +344,7 @@ class LedgerNodeHelpers_test : public beast::unit_test::Suite protocol::TMLedgerNode ledgerNode; ledgerNode.set_nodedata(otherData); ledgerNode.set_nodeid(otherID.getRawString()); - auto const result = getSHAMapNodeID(ledgerNode, leafNode); + auto const result = getSHAMapNodeID(ledgerNode, *leafNode); BEAST_EXPECT(!result); } } diff --git a/src/xrpld/app/ledger/detail/InboundLedger.cpp b/src/xrpld/app/ledger/detail/InboundLedger.cpp index 0bfe153ad9..4aad048413 100644 --- a/src/xrpld/app/ledger/detail/InboundLedger.cpp +++ b/src/xrpld/app/ledger/detail/InboundLedger.cpp @@ -874,7 +874,7 @@ InboundLedger::receiveNode(protocol::TMLedgerData& packet, SHAMapAddNode& san) return; } - auto const nodeID = getSHAMapNodeID(ledgerNode, treeNode); + auto const nodeID = getSHAMapNodeID(ledgerNode, *treeNode); if (!nodeID) { JLOG(journal_.warn()) << "Got invalid node id"; diff --git a/src/xrpld/app/ledger/detail/InboundTransactions.cpp b/src/xrpld/app/ledger/detail/InboundTransactions.cpp index 8eab20e005..c394007b0c 100644 --- a/src/xrpld/app/ledger/detail/InboundTransactions.cpp +++ b/src/xrpld/app/ledger/detail/InboundTransactions.cpp @@ -161,7 +161,7 @@ public: return; } - auto const nodeID = getSHAMapNodeID(ledgerNode, treeNode); + auto const nodeID = getSHAMapNodeID(ledgerNode, *treeNode); if (!nodeID) { JLOG(j_.warn()) << "Got invalid node id"; diff --git a/src/xrpld/app/ledger/detail/LedgerNodeHelpers.cpp b/src/xrpld/app/ledger/detail/LedgerNodeHelpers.cpp index 68a36bbac5..1881b395c9 100644 --- a/src/xrpld/app/ledger/detail/LedgerNodeHelpers.cpp +++ b/src/xrpld/app/ledger/detail/LedgerNodeHelpers.cpp @@ -24,7 +24,7 @@ validateLedgerNode(protocol::TMLedgerNode const& ledgerNode) if (ledgerNode.has_nodeid()) { - return !ledgerNode.has_id() && !ledgerNode.has_depth() && + return ledgerNode.reference_case() == ledgerNode.REFERENCE_NOT_SET && deserializeSHAMapNodeID(ledgerNode.nodeid()).has_value(); } @@ -49,11 +49,11 @@ getTreeNode(std::string_view data) } std::optional -getSHAMapNodeID(protocol::TMLedgerNode const& ledgerNode, SHAMapTreeNodePtr const& treeNode) +getSHAMapNodeID(protocol::TMLedgerNode const& ledgerNode, SHAMapTreeNode const& treeNode) { if (ledgerNode.has_id() || ledgerNode.has_depth()) { - if (treeNode->isInner()) + if (treeNode.isInner()) { if (!ledgerNode.has_id()) return std::nullopt; @@ -61,12 +61,12 @@ getSHAMapNodeID(protocol::TMLedgerNode const& ledgerNode, SHAMapTreeNodePtr cons return deserializeSHAMapNodeID(ledgerNode.id()); } - if (treeNode->isLeaf()) + if (treeNode.isLeaf()) { if (!ledgerNode.has_depth()) return std::nullopt; - auto const key = safeDowncast(treeNode.get())->peekItem()->key(); + auto const key = safeDowncast(&treeNode)->peekItem()->key(); return SHAMapNodeID::createID(ledgerNode.depth(), key); } @@ -81,9 +81,9 @@ getSHAMapNodeID(protocol::TMLedgerNode const& ledgerNode, SHAMapTreeNodePtr cons if (!nodeID.has_value()) return std::nullopt; - if (treeNode->isLeaf()) + if (treeNode.isLeaf()) { - auto const key = safeDowncast(treeNode.get())->peekItem()->key(); + auto const key = safeDowncast(&treeNode)->peekItem()->key(); auto const expectedID = SHAMapNodeID::createID(static_cast(nodeID->getDepth()), key); if (nodeID->getNodeID() != expectedID.getNodeID()) return std::nullopt; diff --git a/src/xrpld/app/ledger/detail/LedgerNodeHelpers.h b/src/xrpld/app/ledger/detail/LedgerNodeHelpers.h index 7a89b09543..03e6133361 100644 --- a/src/xrpld/app/ledger/detail/LedgerNodeHelpers.h +++ b/src/xrpld/app/ledger/detail/LedgerNodeHelpers.h @@ -68,6 +68,6 @@ getTreeNode(std::string_view data); * `validateLedgerNode` function and obtained a valid tree node by calling `getTreeNode`. */ [[nodiscard]] std::optional -getSHAMapNodeID(protocol::TMLedgerNode const& ledgerNode, SHAMapTreeNodePtr const& treeNode); +getSHAMapNodeID(protocol::TMLedgerNode const& ledgerNode, SHAMapTreeNode const& treeNode); } // namespace xrpl