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 <[email protected]> Co-authored-by: Ed Hennis <[email protected]>
XRPLF · Apr 5, 2023 · f191c91 · f191c91
1 parent e6f4904
commit f191c91
Show file tree

Hide file tree

Showing 5 changed files with 331 additions and 23 deletions.
diff --git 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
@@ -28,13 +28,13 @@
 #include <ripple/net/RPCErr.h>
 #include <ripple/protocol/AccountID.h>
 #include <ripple/protocol/Feature.h>
+#include <ripple/protocol/nftPageMask.h>
+#include <ripple/resource/Fees.h>
 #include <ripple/rpc/Context.h>
 #include <ripple/rpc/DeliveredAmount.h>
 #include <ripple/rpc/impl/RPCHelpers.h>
 #include <boost/algorithm/string/case_conv.hpp>
 
-#include <ripple/resource/Fees.h>
-
 namespace ripple {
 namespace RPC {
 
@@ -153,10 +153,88 @@ getAccountObjects(
     AccountID const& account,
     std::optional<std::vector<LedgerEntryType>> const& typeFilter,
     uint256 dirIndex,
-    uint256 const& entryIndex,
+    uint256 entryIndex,
     std::uint32_t const limit,
     Json::Value& jvResult)
 {
+    auto typeMatchesFilter = [](std::vector<LedgerEntryType> 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<LedgerEntryType> 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<RPC::Status, LedgerEntryType> result{RPC::Status::OK, ltANY};
     if (params.isMember(jss::type))
     {
-        static constexpr std::array<std::pair<char const*, LedgerEntryType>, 14>
+        static constexpr std::array<std::pair<char const*, LedgerEntryType>, 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
@@ -106,7 +106,7 @@ getAccountObjects(
     AccountID const& account,
     std::optional<std::vector<LedgerEntryType>> 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
@@ -679,6 +679,7 @@ class AccountLinesRPC_test : public beast::unit_test::suite
             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 @@ class AccountLinesRPC_test : public beast::unit_test::suite
             // 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(