diff --git a/src/test/rpc/AccountLines_test.cpp b/src/test/rpc/AccountLines_test.cpp index dc00183bad..c87f5bd1cc 100644 --- a/src/test/rpc/AccountLines_test.cpp +++ b/src/test/rpc/AccountLines_test.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -32,6 +33,7 @@ #include #include #include +#include #include namespace xrpl::rpc { @@ -517,6 +519,71 @@ public: rpc::makeError(RpcInvalidParams)[jss::error_message]); } + void + testMarkerNewObjectTypes() + { + // Regression: pagination across account_lines / account_offers / + // account_channels must not reject a marker whose SLE is a + // Credential (or any post-2023 owner-directory entry). + testcase("Marker on new owner-directory entry types (Credential)"); + + using namespace test::jtx; + + // Walk the owner directory via `rpcMethod` at limit=1 and return + // the total number of paged calls plus whether the RPC ever + // returned "invalidParams" during pagination. + auto walkPages = [](Env& env, Account const& account, char const* rpcMethod) { + std::optional marker; + int iterations = 0; + bool hitInvalidParams = false; + for (int guard = 0; guard < 20; ++guard) + { + json::Value params; + params[jss::account] = account.human(); + params[jss::limit] = 1; + if (marker) + params[jss::marker] = *marker; + auto const resp = env.rpc("json", rpcMethod, to_string(params))[jss::result]; + ++iterations; + if (resp.isMember(jss::error)) + { + hitInvalidParams = resp[jss::error].asString() == "invalidParams"; + break; + } + if (!resp.isMember(jss::marker)) + break; + marker = resp[jss::marker].asString(); + } + return std::pair{iterations, hitInvalidParams}; + }; + + for (char const* rpcMethod : {"account_lines", "account_offers", "account_channels"}) + { + Env env(*this); + + Account const alice{"alice"}; + Account const issuer{"issuer"}; + env.fund(XRP(10000), alice, issuer); + env.close(); + + auto const usd = issuer["USD"]; + env(trust(alice, usd(200))); + + for (int i = 0; i < 4; ++i) + { + env(credentials::create(alice, issuer, std::string("Cred") + std::to_string(i))); + } + env.close(); + + auto const [iterations, hitInvalidParams] = walkPages(env, alice, rpcMethod); + BEAST_EXPECTS(!hitInvalidParams, rpcMethod); + // 1 trust line + 4 credentials — pagination walks all owner-dir + // entries regardless of which the RPC includes in its results. + BEAST_EXPECTS( + iterations == 5, std::string(rpcMethod) + ": " + std::to_string(iterations)); + } + } + void testAccountLinesWalkMarkers() { @@ -625,8 +692,8 @@ public: env(ticket::create(alice, 2)); // Add another trustline for good measure - auto const btCbecky = becky["BTC"]; - env(trust(alice, btCbecky(200))); + auto const btcBecky = becky["BTC"]; + env(trust(alice, btcBecky(200))); env.close(); @@ -1325,6 +1392,7 @@ public: testAccountLines(); testAccountLinesMarker(); testAccountLineDelete(); + testMarkerNewObjectTypes(); testAccountLinesWalkMarkers(); testAccountLines2(); testAccountLineDelete2(); diff --git a/src/test/rpc/RPCHelpers_test.cpp b/src/test/rpc/RPCHelpers_test.cpp index 25368235a3..ac57f4e2ac 100644 --- a/src/test/rpc/RPCHelpers_test.cpp +++ b/src/test/rpc/RPCHelpers_test.cpp @@ -5,8 +5,12 @@ #include #include #include +#include #include +#include +#include + namespace xrpl::test { class RPCHelpers_test : public beast::unit_test::Suite @@ -66,10 +70,40 @@ public: BEAST_EXPECT(result.second == 0); } + void + testOwnerDirNodeFieldsCoverage() + { + // Every UINT64 sf*Node field must be classified: either it is an + // owner-directory page hint (rpc::isOwnerDirNodeField()) or it + // belongs to a book directory (exclusion set below). Adding a new + // sf*Node UINT64 without updating one of these lists trips this + // test. + testcase("Owner-directory Node fields coverage"); + + // Book-directory page hints, not owner-directory. sfBookNode: offer + // book. sfNFTokenOfferNode: NFT bid/ask book. + std::set const nonOwnerDirNodes{ + &sfBookNode, + &sfNFTokenOfferNode, + }; + + for (auto const& [_, sf] : SField::getKnownCodeToField()) + { + if (sf->fieldType != STI_UINT64 || !sf->getName().ends_with("Node")) + continue; + BEAST_EXPECTS( + rpc::isOwnerDirNodeField(*sf) || nonOwnerDirNodes.contains(sf), + "sf" + sf->getName() + + ": add to kOwnerDirNodeFields (owner-dir page hint) " + "or to nonOwnerDirNodes (book-dir)."); + } + } + void run() override { testChooseLedgerEntryType(); + testOwnerDirNodeFieldsCoverage(); } }; diff --git a/src/xrpld/rpc/detail/RPCHelpers.cpp b/src/xrpld/rpc/detail/RPCHelpers.cpp index 18a6863b40..c51b35abbe 100644 --- a/src/xrpld/rpc/detail/RPCHelpers.cpp +++ b/src/xrpld/rpc/detail/RPCHelpers.cpp @@ -14,13 +14,13 @@ #include #include #include +#include #include #include #include #include #include #include -#include #include #include #include @@ -66,38 +66,47 @@ getStartHint(SLE::ConstRef sle, AccountID const& accountID) return sle->getFieldU64(sfOwnerNode); } +namespace { + +// UINT64 sf*Node fields that record a page number in an owner directory. +// Keep in sync with sfields.macro; the xrpl.rpc.RPCHelpers test enforces it. +constexpr std::array kOwnerDirNodeFields{ + &sfOwnerNode, + &sfLowNode, + &sfHighNode, + &sfDestinationNode, + &sfIssuerNode, + &sfSubjectNode, + &sfSponseeNode, + &sfLoanBrokerNode, + &sfVaultNode, +}; + +} // namespace + +bool +isOwnerDirNodeField(SField const& field) +{ + return std::ranges::contains(kOwnerDirNodeFields, &field); +} + bool isRelatedToAccount(ReadView const& ledger, SLE::ConstRef sle, AccountID const& accountID) { - if (sle->getType() == ltRIPPLE_STATE) - { - return (sle->getFieldAmount(sfLowLimit).getIssuer() == accountID) || - (sle->getFieldAmount(sfHighLimit).getIssuer() == accountID); - } - if (sle->isFieldPresent(sfAccount)) - { - // If there's an sfAccount present, also test the sfDestination, if - // present. This will match objects such as Escrows (ltESCROW), Payment - // Channels (ltPAYCHAN), and Checks (ltCHECK) because those are added to - // the Destination account's directory. It intentionally EXCLUDES - // NFToken Offers (ltNFTOKEN_OFFER). NFToken Offers are NOT added to the - // Destination account's directory. - return sle->getAccountID(sfAccount) == accountID || - (sle->isFieldPresent(sfDestination) && sle->getAccountID(sfDestination) == accountID); - } - if (sle->getType() == ltSIGNER_LIST) - { - Keylet const accountSignerList = keylet::signerList(accountID); - return sle->key() == accountSignerList.key; - } - if (sle->getType() == ltNFTOKEN_OFFER) - { - // Do not check the sfDestination field. NFToken Offers are NOT added to - // the Destination account's directory. - return sle->getAccountID(sfOwner) == accountID; - } + // Marker validator for account_lines / account_offers / account_channels + // pagination: probes each owner-directory page-hint field on `sle` and + // returns true iff `sle`'s key is present on that page in `accountID`'s + // owner directory. Bounded by kOwnerDirNodeFields.size() ledger reads. + auto const ownerDir = keylet::ownerDir(accountID); - return false; + auto const pageContainsKey = [&](std::uint64_t node) { + auto const page = ledger.read(keylet::page(ownerDir, node)); + return page && std::ranges::contains(page->getFieldV256(sfIndexes), sle->key()); + }; + + return std::ranges::any_of(kOwnerDirNodeFields, [&](SField const* field) { + return sle->isFieldPresent(*field) && pageContainsKey(sle->getFieldU64(*field)); + }); } HashSet diff --git a/src/xrpld/rpc/detail/RPCHelpers.h b/src/xrpld/rpc/detail/RPCHelpers.h index 1d527742b0..adafcf4af3 100644 --- a/src/xrpld/rpc/detail/RPCHelpers.h +++ b/src/xrpld/rpc/detail/RPCHelpers.h @@ -14,6 +14,7 @@ #include #include #include +#include #include // IWYU pragma: keep #include #include @@ -60,6 +61,17 @@ getStartHint(SLE::ConstRef sle, AccountID const& accountID); bool isRelatedToAccount(ReadView const& ledger, SLE::ConstRef sle, AccountID const& accountID); +/** + * @brief Checks whether an SField is a UINT64 sf*Node owner-directory + * page-hint field probed by `isRelatedToAccount`. + * + * @param field The SField to test. + * @return true if the field is one of the owner-directory page-hint + * fields, false otherwise. + */ +bool +isOwnerDirNodeField(SField const& field); + /** * @brief Parses an array of account IDs from a JSON value. *