Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add NFTokenPages to account_objects RPC #4352

Merged
merged 8 commits into from
Apr 5, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/ripple/rpc/handlers/AccountObjects.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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}};

Expand Down
115 changes: 98 additions & 17 deletions src/ripple/rpc/impl/RPCHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,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 {

Expand Down Expand Up @@ -138,10 +138,85 @@ 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)
{
// we do this by shifting the accountid left by 12 bytes
// if it matches 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;
ximinez marked this conversation as resolved.
Show resolved Hide resolved
}

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;
}
}
ck = *npm;
ximinez marked this conversation as resolved.
Show resolved Hide resolved
}

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

Expand All @@ -153,10 +228,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);
Expand All @@ -171,25 +249,27 @@ getAccountObjects(
found = true;
}

// it's possible that the returned NFTPages exactly filled the
// response. Check for that condition.
if (i == mlimit && mlimit < limit)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious why i == mlimit was preferred here over than writing 0 == mlimit

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been a while. As I recall this change was motivated by one of the new unit tests (that failed before this change). FWIW, i is incrementing inside the outer for loop, so I suspect that a comparison to 0 would not give the correct results.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah sorry i misread the code.

{
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())
{
Expand All @@ -212,7 +292,7 @@ getAccountObjects(
if (!dir)
return true;

if (i == limit)
if (i == mlimit)
{
auto const& e = dir->getFieldV256(sfIndexes);
if (!e.empty())
Expand Down Expand Up @@ -883,7 +963,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},
Expand All @@ -898,7 +978,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())
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/rpc/impl/RPCHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Loading