fix: Paginate account_lines/offers/channels past new owner-dir types (#8274)

This commit is contained in:
Kassaking7
2026-10-02 14:48:48 +00:00
committed by GitHub
parent cbade49976
commit 84e2a155b9
4 changed files with 154 additions and 31 deletions

View File

@@ -3,6 +3,7 @@
#include <test/jtx/Env.h>
#include <test/jtx/TestHelpers.h>
#include <test/jtx/amount.h>
#include <test/jtx/credentials.h>
#include <test/jtx/deposit.h>
#include <test/jtx/escrow.h>
#include <test/jtx/flags.h>
@@ -32,6 +33,7 @@
#include <cstddef>
#include <optional>
#include <string>
#include <utility>
#include <vector>
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<std::string> 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();

View File

@@ -5,8 +5,12 @@
#include <xrpl/json/json_value.h>
#include <xrpl/protocol/ErrorCodes.h>
#include <xrpl/protocol/LedgerFormats.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/jss.h>
#include <set>
#include <string>
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<SField const*> 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();
}
};

View File

@@ -14,13 +14,13 @@
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/core/ServiceRegistry.h>
#include <xrpl/ledger/ReadView.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Asset.h>
#include <xrpl/protocol/ErrorCodes.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/Issue.h>
#include <xrpl/protocol/KeyType.h>
#include <xrpl/protocol/Keylet.h>
#include <xrpl/protocol/LedgerFormats.h>
#include <xrpl/protocol/PublicKey.h>
#include <xrpl/protocol/RPCErr.h>
@@ -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<SField const*, 9> 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<AccountID>

View File

@@ -14,6 +14,7 @@
#include <xrpl/protocol/ErrorCodes.h>
#include <xrpl/protocol/LedgerFormats.h>
#include <xrpl/protocol/PublicKey.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STLedgerEntry.h> // IWYU pragma: keep
#include <xrpl/protocol/SecretKey.h>
#include <xrpl/protocol/Seed.h>
@@ -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.
*