diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index 18291bf2b95..efa9e10cc34 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -20,10 +20,12 @@ #include #include #include +#include #include #include #include #include +#include #include @@ -1032,6 +1034,128 @@ 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); + + static constexpr 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 tokenIDs; + { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::ledger_index] = "validated"; + Json::Value const resp = + env.rpc("json", "account_nfts", to_string(params)); + Json::Value const& nfts = resp[jss::result][jss::account_nfts]; + for (Json::Value const& nft : nfts) + tokenIDs.push_back(nft["NFTokenID"]); + } + + // 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 = [&tokenIDs, &env, &bob]( + unsigned const limit, unsigned const lastIndex) { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = limit; + params[jss::marker] = tokenIDs[lastIndex]; + params[jss::ledger_index] = "validated"; + Json::Value const resp = + env.rpc("json", "account_nfts", to_string(params)); + + if (resp[jss::result].isMember(jss::error)) + return false; + + Json::Value const& nfts = resp[jss::result][jss::account_nfts]; + unsigned const nftsCount = tokenIDs.size() - lastIndex - 1 < limit + ? tokenIDs.size() - 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; + }; + + // test a valid marker which is equal to the third tokenID + BEAST_EXPECT(compareNFTs(4, 2)); + + // test a valid marker which is equal to the 8th tokenID + BEAST_EXPECT(compareNFTs(4, 7)); + + // lambda that holds common code for invalid cases. + auto testInvalidMarker = [&env, &bob]( + auto marker, char const* errorMessage) { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = 4; + params[jss::ledger_index] = jss::validated; + params[jss::marker] = marker; + Json::Value const resp = + env.rpc("json", "account_nfts", to_string(params)); + return resp[jss::result][jss::error_message] == errorMessage; + }; + + // test an invalid marker that is not a string + BEAST_EXPECT( + testInvalidMarker(17, "Invalid field \'marker\', not string.")); + + // test an invalid marker that has a non-hex character + BEAST_EXPECT(testInvalidMarker( + "00000000F51DFC2A09D62CBBA1DFBDD4691DAC96AD98B900000000000000000G", + "Invalid field \'marker\'.")); + + // 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, + std::uint32_t taxon, + std::uint32_t tokenSeq, + std::uint16_t flags = 0, + std::uint16_t fee = 0) { + // the marker has the exact same format as an NFTokenID + return to_string(NFTokenMint::createNFTokenID( + flags, fee, issuer, nft::toTaxon(taxon), tokenSeq)); + }; + + // test an unassociated marker which does not exist in the NFTokenIDs + BEAST_EXPECT(testInvalidMarker( + createFakeNFTMarker(bob.id(), 0x000000000, 0x00000000), + "Invalid field \'marker\'.")); + + // test an unassociated marker which exceeds the maximum value of the + // existing NFTokenID + BEAST_EXPECT(testInvalidMarker( + createFakeNFTMarker(bob.id(), 0xFFFFFFFF, 0xFFFFFFFF), + "Invalid field \'marker\'.")); + } + void run() override { @@ -1039,6 +1163,7 @@ class AccountObjects_test : public beast::unit_test::suite testUnsteppedThenStepped(); testUnsteppedThenSteppedWithNFTs(); testObjectTypes(); + testNFTsMarker(); } }; diff --git a/src/xrpld/app/tx/detail/NFTokenMint.h b/src/xrpld/app/tx/detail/NFTokenMint.h index e0eb54bbc93..c95fd5944e4 100644 --- a/src/xrpld/app/tx/detail/NFTokenMint.h +++ b/src/xrpld/app/tx/detail/NFTokenMint.h @@ -22,6 +22,7 @@ #include #include +#include namespace ripple { diff --git a/src/xrpld/rpc/handlers/AccountObjects.cpp b/src/xrpld/rpc/handlers/AccountObjects.cpp index dfc4e9b3388..32fe15ec264 100644 --- a/src/xrpld/rpc/handlers/AccountObjects.cpp +++ b/src/xrpld/rpc/handlers/AccountObjects.cpp @@ -75,8 +75,9 @@ doAccountNFTs(RPC::JsonContext& context) return *err; uint256 marker; + bool const markerSet = params.isMember(jss::marker); - if (params.isMember(jss::marker)) + if (markerSet) { auto const& m = params[jss::marker]; if (!m.isString()) @@ -98,6 +99,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) { @@ -119,12 +121,23 @@ doAccountNFTs(RPC::JsonContext& context) uint256 const nftokenID = o[sfNFTokenID]; uint256 const maskedNftokenID = nftokenID & nft::pageMask; - if (!pastMarker && maskedNftokenID < maskedMarker) - continue; + if (!pastMarker) + { + if (maskedNftokenID < maskedMarker) + continue; - if (!pastMarker && maskedNftokenID == maskedMarker && - nftokenID <= marker) - continue; + if (maskedNftokenID == maskedMarker && nftokenID < marker) + continue; + + if (nftokenID == marker) + { + markerFound = true; + continue; + } + } + + if (markerSet && !markerFound) + return RPC::invalid_field_error(jss::marker); pastMarker = true; @@ -155,6 +168,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;