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 <scott@ripple.com>
Co-authored-by: Ed Hennis <ed@ripple.com>

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}};
 

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())

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);
 

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(

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<std::string> 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();
     }
 };