From f191c911d4d9f1d22727dcd60a912bc476efea49 Mon Sep 17 00:00:00 2001 From: RichardAH Date: Wed, 5 Apr 2023 22:58:55 +0200 Subject: [PATCH] Add NFTokenPages to account_objects RPC: (#4352) - Include NFTokenPages in account_objects to make it easier to understand an account's Owner Reserve and simplify app development. - Update related tests and documentation. - Fix #4347. For info about the Owner Reserve, see https://xrpl.org/reserves.html --------- Co-authored-by: Scott Schurr Co-authored-by: Ed Hennis --- src/ripple/rpc/handlers/AccountObjects.cpp | 1 + src/ripple/rpc/impl/RPCHelpers.cpp | 118 +++++++++-- src/ripple/rpc/impl/RPCHelpers.h | 2 +- src/test/rpc/AccountLinesRPC_test.cpp | 3 +- src/test/rpc/AccountObjects_test.cpp | 230 ++++++++++++++++++++- 5 files changed, 331 insertions(+), 23 deletions(-) diff --git a/src/ripple/rpc/handlers/AccountObjects.cpp b/src/ripple/rpc/handlers/AccountObjects.cpp index 6424c3afd..687b87179 100644 --- a/src/ripple/rpc/handlers/AccountObjects.cpp +++ b/src/ripple/rpc/handlers/AccountObjects.cpp @@ -204,6 +204,7 @@ doAccountObjects(RPC::JsonContext& context) } static constexpr deletionBlockers[] = { {jss::check, ltCHECK}, {jss::escrow, ltESCROW}, + {jss::nft_page, ltNFTOKEN_PAGE}, {jss::payment_channel, ltPAYCHAN}, {jss::state, ltRIPPLE_STATE}}; diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index ad84d7b12..44eebdcab 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -28,13 +28,13 @@ #include #include #include +#include +#include #include #include #include #include -#include - namespace ripple { namespace RPC { @@ -153,10 +153,88 @@ getAccountObjects( AccountID const& account, std::optional> const& typeFilter, uint256 dirIndex, - uint256 const& entryIndex, + uint256 entryIndex, std::uint32_t const limit, Json::Value& jvResult) { + auto typeMatchesFilter = [](std::vector const& typeFilter, + LedgerEntryType ledgerType) { + auto it = std::find(typeFilter.begin(), typeFilter.end(), ledgerType); + return it != typeFilter.end(); + }; + + // if dirIndex != 0, then all NFTs have already been returned. only + // iterate NFT pages if the filter says so AND dirIndex == 0 + bool iterateNFTPages = + (!typeFilter.has_value() || + typeMatchesFilter(typeFilter.value(), ltNFTOKEN_PAGE)) && + dirIndex == beast::zero; + + Keylet const firstNFTPage = keylet::nftpage_min(account); + + // we need to check the marker to see if it is an NFTTokenPage index. + if (iterateNFTPages && entryIndex != beast::zero) + { + // if it is we will try to iterate the pages up to the limit + // and then change over to the owner directory + + if (firstNFTPage.key != (entryIndex & ~nft::pageMask)) + iterateNFTPages = false; + } + + auto& jvObjects = (jvResult[jss::account_objects] = Json::arrayValue); + + // this is a mutable version of limit, used to seemlessly switch + // to iterating directory entries when nftokenpages are exhausted + uint32_t mlimit = limit; + + // iterate NFTokenPages preferentially + if (iterateNFTPages) + { + Keylet const first = entryIndex == beast::zero + ? firstNFTPage + : Keylet{ltNFTOKEN_PAGE, entryIndex}; + + Keylet const last = keylet::nftpage_max(account); + + // current key + uint256 ck = ledger.succ(first.key, last.key.next()).value_or(last.key); + + // current page + auto cp = ledger.read(Keylet{ltNFTOKEN_PAGE, ck}); + + while (cp) + { + jvObjects.append(cp->getJson(JsonOptions::none)); + auto const npm = (*cp)[~sfNextPageMin]; + if (npm) + cp = ledger.read(Keylet(ltNFTOKEN_PAGE, *npm)); + else + cp = nullptr; + + if (--mlimit == 0) + { + if (cp) + { + jvResult[jss::limit] = limit; + jvResult[jss::marker] = std::string("0,") + to_string(ck); + return true; + } + } + + if (!npm) + break; + + ck = *npm; + } + + // if execution reaches here then we're about to transition + // to iterating the root directory (and the conventional + // behaviour of this RPC function.) Therefore we should + // zero entryIndex so as not to terribly confuse things. + entryIndex = beast::zero; + } + auto const root = keylet::ownerDir(account); auto found = false; @@ -168,10 +246,13 @@ getAccountObjects( auto dir = ledger.read({ltDIR_NODE, dirIndex}); if (!dir) - return false; + { + // it's possible the user had nftoken pages but no + // directory entries + return mlimit < limit; + } std::uint32_t i = 0; - auto& jvObjects = (jvResult[jss::account_objects] = Json::arrayValue); for (;;) { auto const& entries = dir->getFieldV256(sfIndexes); @@ -186,25 +267,27 @@ getAccountObjects( found = true; } + // it's possible that the returned NFTPages exactly filled the + // response. Check for that condition. + if (i == mlimit && mlimit < limit) + { + jvResult[jss::limit] = limit; + jvResult[jss::marker] = + to_string(dirIndex) + ',' + to_string(*iter); + return true; + } + for (; iter != entries.end(); ++iter) { auto const sleNode = ledger.read(keylet::child(*iter)); - auto typeMatchesFilter = - [](std::vector const& typeFilter, - LedgerEntryType ledgerType) { - auto it = std::find( - typeFilter.begin(), typeFilter.end(), ledgerType); - return it != typeFilter.end(); - }; - if (!typeFilter.has_value() || typeMatchesFilter(typeFilter.value(), sleNode->getType())) { jvObjects.append(sleNode->getJson(JsonOptions::none)); } - if (++i == limit) + if (++i == mlimit) { if (++iter != entries.end()) { @@ -227,7 +310,7 @@ getAccountObjects( if (!dir) return true; - if (i == limit) + if (i == mlimit) { auto const& e = dir->getFieldV256(sfIndexes); if (!e.empty()) @@ -898,7 +981,7 @@ chooseLedgerEntryType(Json::Value const& params) std::pair result{RPC::Status::OK, ltANY}; if (params.isMember(jss::type)) { - static constexpr std::array, 14> + static constexpr std::array, 15> types{ {{jss::account, ltACCOUNT_ROOT}, {jss::amendments, ltAMENDMENTS}, @@ -913,7 +996,8 @@ chooseLedgerEntryType(Json::Value const& params) {jss::signer_list, ltSIGNER_LIST}, {jss::state, ltRIPPLE_STATE}, {jss::ticket, ltTICKET}, - {jss::nft_offer, ltNFTOKEN_OFFER}}}; + {jss::nft_offer, ltNFTOKEN_OFFER}, + {jss::nft_page, ltNFTOKEN_PAGE}}}; auto const& p = params[jss::type]; if (!p.isString()) diff --git a/src/ripple/rpc/impl/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h index 12f27641d..6184b3575 100644 --- a/src/ripple/rpc/impl/RPCHelpers.h +++ b/src/ripple/rpc/impl/RPCHelpers.h @@ -106,7 +106,7 @@ getAccountObjects( AccountID const& account, std::optional> const& typeFilter, uint256 dirIndex, - uint256 const& entryIndex, + uint256 entryIndex, std::uint32_t const limit, Json::Value& jvResult); diff --git a/src/test/rpc/AccountLinesRPC_test.cpp b/src/test/rpc/AccountLinesRPC_test.cpp index cdc619220..1b099f7b7 100644 --- a/src/test/rpc/AccountLinesRPC_test.cpp +++ b/src/test/rpc/AccountLinesRPC_test.cpp @@ -679,6 +679,7 @@ public: auto aliceLines = getNextLine(env, alice, std::nullopt); constexpr std::size_t expectedIterations = 16; constexpr std::size_t expectedLines = 2; + constexpr std::size_t expectedNFTs = 1; std::size_t foundLines = 0; auto hasMarker = [](auto const& aliceLines) { @@ -729,7 +730,7 @@ public: // this test will need to be updated. BEAST_EXPECT( aliceObjects[jss::result][jss::account_objects].size() == - iterations); + iterations + expectedNFTs); // If ledger object association ever changes, for whatever // reason, this test will need to be updated. BEAST_EXPECTS( diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index 64e77b930..07fd5d6dd 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -294,7 +294,7 @@ public: { Json::Value params; params[jss::account] = bob.human(); - params[jss::type] = "state"; + params[jss::type] = jss::state; auto resp = env.rpc("json", "account_objects", to_string(params)); BEAST_EXPECT(!resp.isMember(jss::marker)); @@ -321,14 +321,217 @@ public: auto& aobj = aobjs[0U]; if (i < 3) BEAST_EXPECT(resp[jss::result][jss::limit] == 1); + else + BEAST_EXPECT(!resp[jss::result].isMember(jss::limit)); aobj.removeMember("PreviousTxnID"); aobj.removeMember("PreviousTxnLgrSeq"); BEAST_EXPECT(aobj == bobj[i]); - auto resume_marker = resp[jss::result][jss::marker]; - params[jss::marker] = resume_marker; + params[jss::marker] = resp[jss::result][jss::marker]; + } + } + } + + void + testUnsteppedThenSteppedWithNFTs() + { + // The preceding test case, unsteppedThenStepped(), found a bug in the + // support for NFToken Pages. So we're leaving that test alone when + // adding tests to exercise NFTokenPages. + testcase("unsteppedThenSteppedWithNFTs"); + + using namespace jtx; + Env env(*this); + + Account const gw1{"G1"}; + Account const gw2{"G2"}; + Account const bob{"bob"}; + + auto const USD1 = gw1["USD"]; + auto const USD2 = gw2["USD"]; + + env.fund(XRP(1000), gw1, gw2, bob); + env.close(); + + // Check behavior if there are no account objects. + { + // Unpaged + Json::Value params; + params[jss::account] = bob.human(); + auto resp = env.rpc("json", "account_objects", to_string(params)); + BEAST_EXPECT(!resp.isMember(jss::marker)); + BEAST_EXPECT(resp[jss::result][jss::account_objects].size() == 0); + + // Limit == 1 + params[jss::limit] = 1; + resp = env.rpc("json", "account_objects", to_string(params)); + BEAST_EXPECT(!resp.isMember(jss::marker)); + BEAST_EXPECT(resp[jss::result][jss::account_objects].size() == 0); + } + + // Check behavior if there are only NFTokens. + env(token::mint(bob, 0u), txflags(tfTransferable)); + env.close(); + + // test 'unstepped' + // i.e. request account objects without explicit limit/marker paging + Json::Value unpaged; + { + Json::Value params; + params[jss::account] = bob.human(); + auto resp = env.rpc("json", "account_objects", to_string(params)); + BEAST_EXPECT(!resp.isMember(jss::marker)); + + unpaged = resp[jss::result][jss::account_objects]; + BEAST_EXPECT(unpaged.size() == 1); + } + // test request with type parameter as filter, unstepped + { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::type] = jss::nft_page; + auto resp = env.rpc("json", "account_objects", to_string(params)); + BEAST_EXPECT(!resp.isMember(jss::marker)); + Json::Value& aobjs = resp[jss::result][jss::account_objects]; + BEAST_EXPECT(aobjs.size() == 1); + BEAST_EXPECT( + aobjs[0u][sfLedgerEntryType.jsonName] == jss::NFTokenPage); + BEAST_EXPECT(aobjs[0u][sfNFTokens.jsonName].size() == 1); + } + // test stepped one-at-a-time with limit=1, resume from prev marker + { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = 1; + + Json::Value resp = + env.rpc("json", "account_objects", to_string(params)); + Json::Value& aobjs = resp[jss::result][jss::account_objects]; + BEAST_EXPECT(aobjs.size() == 1); + auto& aobj = aobjs[0U]; + BEAST_EXPECT(!resp[jss::result].isMember(jss::limit)); + BEAST_EXPECT(!resp[jss::result].isMember(jss::marker)); + + BEAST_EXPECT(aobj == unpaged[0u]); + } + + // Add more objects in addition to the NFToken Page. + env.trust(USD1(1000), bob); + env.trust(USD2(1000), bob); + + env(pay(gw1, bob, USD1(1000))); + env(pay(gw2, bob, USD2(1000))); + + env(offer(bob, XRP(100), bob["USD"](1)), txflags(tfPassive)); + env(offer(bob, XRP(100), USD1(1)), txflags(tfPassive)); + env.close(); + + // test 'unstepped' + { + Json::Value params; + params[jss::account] = bob.human(); + auto resp = env.rpc("json", "account_objects", to_string(params)); + BEAST_EXPECT(!resp.isMember(jss::marker)); + + unpaged = resp[jss::result][jss::account_objects]; + BEAST_EXPECT(unpaged.size() == 5); + } + // test request with type parameter as filter, unstepped + { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::type] = jss::nft_page; + auto resp = env.rpc("json", "account_objects", to_string(params)); + BEAST_EXPECT(!resp.isMember(jss::marker)); + Json::Value& aobjs = resp[jss::result][jss::account_objects]; + BEAST_EXPECT(aobjs.size() == 1); + BEAST_EXPECT( + aobjs[0u][sfLedgerEntryType.jsonName] == jss::NFTokenPage); + BEAST_EXPECT(aobjs[0u][sfNFTokens.jsonName].size() == 1); + } + // test stepped one-at-a-time with limit=1, resume from prev marker + { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = 1; + for (int i = 0; i < 5; ++i) + { + Json::Value resp = + env.rpc("json", "account_objects", to_string(params)); + Json::Value& aobjs = resp[jss::result][jss::account_objects]; + BEAST_EXPECT(aobjs.size() == 1); + auto& aobj = aobjs[0U]; + if (i < 4) + { + BEAST_EXPECT(resp[jss::result][jss::limit] == 1); + BEAST_EXPECT(resp[jss::result].isMember(jss::marker)); + } + else + { + BEAST_EXPECT(!resp[jss::result].isMember(jss::limit)); + BEAST_EXPECT(!resp[jss::result].isMember(jss::marker)); + } + + BEAST_EXPECT(aobj == unpaged[i]); + + params[jss::marker] = resp[jss::result][jss::marker]; + } + } + + // Make sure things still work if there is more than 1 NFT Page. + for (int i = 0; i < 32; ++i) + { + env(token::mint(bob, 0u), txflags(tfTransferable)); + env.close(); + } + // test 'unstepped' + { + Json::Value params; + params[jss::account] = bob.human(); + auto resp = env.rpc("json", "account_objects", to_string(params)); + BEAST_EXPECT(!resp.isMember(jss::marker)); + + unpaged = resp[jss::result][jss::account_objects]; + BEAST_EXPECT(unpaged.size() == 6); + } + // test request with type parameter as filter, unstepped + { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::type] = jss::nft_page; + auto resp = env.rpc("json", "account_objects", to_string(params)); + BEAST_EXPECT(!resp.isMember(jss::marker)); + Json::Value& aobjs = resp[jss::result][jss::account_objects]; + BEAST_EXPECT(aobjs.size() == 2); + } + // test stepped one-at-a-time with limit=1, resume from prev marker + { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = 1; + for (int i = 0; i < 6; ++i) + { + Json::Value resp = + env.rpc("json", "account_objects", to_string(params)); + Json::Value& aobjs = resp[jss::result][jss::account_objects]; + BEAST_EXPECT(aobjs.size() == 1); + auto& aobj = aobjs[0U]; + if (i < 5) + { + BEAST_EXPECT(resp[jss::result][jss::limit] == 1); + BEAST_EXPECT(resp[jss::result].isMember(jss::marker)); + } + else + { + BEAST_EXPECT(!resp[jss::result].isMember(jss::limit)); + BEAST_EXPECT(!resp[jss::result].isMember(jss::marker)); + } + + BEAST_EXPECT(aobj == unpaged[i]); + + params[jss::marker] = resp[jss::result][jss::marker]; } } } @@ -376,12 +579,29 @@ public: BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::escrow), 0)); BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::fee), 0)); BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::hashes), 0)); + BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::nft_page), 0)); BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::offer), 0)); BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::payment_channel), 0)); BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::signer_list), 0)); BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::state), 0)); BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::ticket), 0)); + // gw mints an NFT so we can find it. + uint256 const nftID{token::getNextID(env, gw, 0u, tfTransferable)}; + env(token::mint(gw, 0u), txflags(tfTransferable)); + env.close(); + { + // Find the NFToken page and make sure it's the right one. + Json::Value const resp = acct_objs(gw, jss::nft_page); + BEAST_EXPECT(acct_objs_is_size(resp, 1)); + + auto const& nftPage = resp[jss::result][jss::account_objects][0u]; + BEAST_EXPECT(nftPage[sfNFTokens.jsonName].size() == 1); + BEAST_EXPECT( + nftPage[sfNFTokens.jsonName][0u][sfNFToken.jsonName] + [sfNFTokenID.jsonName] == to_string(nftID)); + } + // Set up a trust line so we can find it. env.trust(USD(1000), alice); env.close(); @@ -510,7 +730,7 @@ public: auto const& ticket = resp[jss::result][jss::account_objects][0u]; BEAST_EXPECT(ticket[sfAccount.jsonName] == gw.human()); BEAST_EXPECT(ticket[sfLedgerEntryType.jsonName] == jss::Ticket); - BEAST_EXPECT(ticket[sfTicketSequence.jsonName].asUInt() == 12); + BEAST_EXPECT(ticket[sfTicketSequence.jsonName].asUInt() == 13); } { // See how "deletion_blockers_only" handles gw's directory. @@ -523,6 +743,7 @@ public: std::vector v{ jss::Escrow.c_str(), jss::Check.c_str(), + jss::NFTokenPage.c_str(), jss::RippleState.c_str(), jss::PayChannel.c_str()}; std::sort(v.begin(), v.end()); @@ -583,6 +804,7 @@ public: { testErrors(); testUnsteppedThenStepped(); + testUnsteppedThenSteppedWithNFTs(); testObjectTypes(); } };