fix "account_nfts" with unassociated marker returning issue

src/test/rpc/AccountObjects_test.cpp

@@ -1032,13 +1032,152 @@ class AccountObjects_test : public beast::unit_test::suite
         BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::hashes), 0));
     }
 
+    void
+    testNFTsMarker()
+    {
+        // there's some bug found in account_nfts method that it did not
+        // return invalid params when providing unassociated nft marker.
+        // this test tests both situations when providing valid nft marker
+        // and unassociated nft marker.
+        testcase("NFTsMarker");
+
+        using namespace jtx;
+        Env env(*this);
+
+        Account const bob{"bob"};
+        env.fund(XRP(10000), bob);
+
+        // this lambda function is used to create some fake marker using given
+        // taxon and sequence because we want to test some unassociated markers
+        // later
+        auto createFakeNFTMarker = [](AccountID const& issuer,
+                                      uint32_t taxon,
+                                      uint32_t tokenSeq,
+                                      std::uint16_t flags = 0,
+                                      std::uint16_t fee = 0) {
+            taxon = boost::endian::native_to_big(taxon);
+            tokenSeq = boost::endian::native_to_big(tokenSeq);
+            flags = boost::endian::native_to_big(flags);
+            fee = boost::endian::native_to_big(fee);
+            std::array<std::uint8_t, 32> buf{};
+            auto ptr = buf.data();
+            std::memcpy(ptr, &flags, sizeof(flags));
+            ptr += sizeof(flags);
+            std::memcpy(ptr, &fee, sizeof(fee));
+            ptr += sizeof(fee);
+            std::memcpy(ptr, issuer.data(), issuer.size());
+            ptr += issuer.size();
+            std::memcpy(ptr, &taxon, sizeof(taxon));
+            ptr += sizeof(taxon);
+            std::memcpy(ptr, &tokenSeq, sizeof(tokenSeq));
+            ptr += sizeof(tokenSeq);
+            return uint256::fromVoid(buf.data());
+        };
+
+        // this lambda function is used to check if the account_nfts method
+        // returns the correct token information. lastIndex is used to query the
+        // last marker.
+        auto compareNFTs = [&](unsigned const limit,
+                               unsigned const lastIndex,
+                               unsigned const nftsSize,
+                               const std::vector<Json::Value>& tokenIDs) {
+            Json::Value params;
+            params[jss::account] = bob.human();
+            params[jss::limit] = limit;
+            params[jss::marker] = tokenIDs[lastIndex];
+            params[jss::ledger_index] = "validated";
+            auto const resp =
+                env.rpc("json", "account_nfts", to_string(params));
+
+            if (resp[jss::result].isMember(jss::error))
+                return false;
+
+            auto const& nfts = resp[jss::result][jss::account_nfts];
+            auto nftsCount = nftsSize - lastIndex - 1 < limit
+                ? nftsSize - lastIndex - 1
+                : limit;
+
+            if (nfts.size() != nftsCount)
+                return false;
+
+            for (unsigned i = 0; i < nftsCount; i++)
+            {
+                if (nfts[i]["NFTokenID"] != tokenIDs[lastIndex + 1 + i])
+                    return false;
+            }
+
+            return true;
+        };
+
+        unsigned nftsSize = 10;
+        for (unsigned i = 0; i < nftsSize; i++)
+        {
+            env(token::mint(bob, 0));
+        }
+
+        env.close();
+
+        // save the NFTokenIDs to use later
+        std::vector<Json::Value> tokenIDs;
+        {
+            Json::Value params;
+            params[jss::account] = bob.human();
+            params[jss::ledger_index] = "validated";
+            auto const resp =
+                env.rpc("json", "account_nfts", to_string(params));
+            auto const& nfts = resp[jss::result][jss::account_nfts];
+            for (auto const& nft : nfts)
+                tokenIDs.push_back(nft["NFTokenID"]);
+        }
+
+        // test a valid marker which is equal to the third tokenID
+        BEAST_EXPECT(compareNFTs(4, 2, nftsSize, tokenIDs));
+
+        // test a valid marker which is equal to the 8th tokenID
+        BEAST_EXPECT(compareNFTs(4, 7, nftsSize, tokenIDs));
+
+        // test an unassociated marker which does not exist in the NFTokenIDs
+        {
+            Json::Value params;
+            params[jss::account] = bob.human();
+            params[jss::limit] = 4;
+            params[jss::ledger_index] = "validated";
+            auto const marker =
+                createFakeNFTMarker(bob.id(), 0x00000000, 0x00000000);
+            params[jss::marker] = to_string(marker);
+            auto const resp =
+                env.rpc("json", "account_nfts", to_string(params));
+            BEAST_EXPECT(
+                resp[jss::result][jss::error_message] ==
+                "Invalid field \'marker\'.");
+        }
+
+        // test an unassociated marker which exceeds the maximum value of the
+        // existing NFTokenID
+        {
+            Json::Value params;
+            params[jss::account] = bob.human();
+            params[jss::limit] = 4;
+            params[jss::ledger_index] = "validated";
+            auto const marker =
+                createFakeNFTMarker(bob.id(), 0xFFFFFFFF, 0xFFFFFFFF);
+            params[jss::marker] = to_string(marker);
+            auto const resp =
+                env.rpc("json", "account_nfts", to_string(params));
+            BEAST_EXPECT(
+                resp[jss::result][jss::error_message] ==
+                "Invalid field \'marker\'.");
+        }
+    }
+
     void
     run() override
     {
         testErrors();
         testUnsteppedThenStepped();
         testUnsteppedThenSteppedWithNFTs();
         testObjectTypes();
+        testNFTsMarker();
     }
 };
 

src/xrpld/rpc/handlers/AccountObjects.cpp

@@ -75,9 +75,11 @@ doAccountNFTs(RPC::JsonContext& context)
         return *err;
 
     uint256 marker;
+    bool markerSet = false;
 
     if (params.isMember(jss::marker))
     {
+        markerSet = true;
         auto const& m = params[jss::marker];
         if (!m.isString())
             return RPC::expected_field_error(jss::marker, "string");
@@ -98,6 +100,7 @@ doAccountNFTs(RPC::JsonContext& context)
 
     // Continue iteration from the current page:
     bool pastMarker = marker.isZero();
+    bool markerFound = false;
     uint256 const maskedMarker = marker & nft::pageMask;
     while (cp)
     {
@@ -123,9 +126,18 @@ doAccountNFTs(RPC::JsonContext& context)
                 continue;
 
             if (!pastMarker && maskedNftokenID == maskedMarker &&
-                nftokenID <= marker)
+                nftokenID < marker)
                 continue;
 
+            if (nftokenID == marker)
+            {
+                markerFound = true;
+                continue;
+            }
+
+            if (markerSet && !markerFound)
+                return RPC::invalid_field_error(jss::marker);
+
             pastMarker = true;
 
             {
@@ -155,6 +167,9 @@ doAccountNFTs(RPC::JsonContext& context)
             cp = nullptr;
     }
 
+    if (markerSet && !markerFound)
+        return RPC::invalid_field_error(jss::marker);
+
     result[jss::account] = toBase58(accountID);
     context.loadType = Resource::feeMediumBurdenRPC;
     return result;