Skip to content

Commit

Permalink
RPC commands understand markers derived from all ledger object types:
Browse files Browse the repository at this point in the history
* Resolves XRPLF#4340 and XRPLF#4354
* Add unit test that walks all the object types and verifies that all of
  their indexes can work as a marker.
  • Loading branch information
ximinez committed Jan 5, 2023
1 parent 019b956 commit 25889a7
Show file tree
Hide file tree
Showing 7 changed files with 204 additions and 8 deletions.
1 change: 1 addition & 0 deletions src/ripple/app/tx/impl/details/NFTokenUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ findTokenAndPage(
}
return std::nullopt;
}

void
removeAllTokenOffers(ApplyView& view, Keylet const& directory)
{
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/rpc/handlers/AccountChannels.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ doAccountChannels(RPC::JsonContext& context)
if (!sle)
return rpcError(rpcINVALID_PARAMS);

if (!RPC::isOwnedByAccount(*ledger, sle, accountID))
if (!RPC::isRelatedToAccount(*ledger, sle, accountID))
return rpcError(rpcINVALID_PARAMS);
}

Expand Down
2 changes: 1 addition & 1 deletion src/ripple/rpc/handlers/AccountLines.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ doAccountLines(RPC::JsonContext& context)
if (!sle)
return rpcError(rpcINVALID_PARAMS);

if (!RPC::isOwnedByAccount(*ledger, sle, accountID))
if (!RPC::isRelatedToAccount(*ledger, sle, accountID))
return rpcError(rpcINVALID_PARAMS);
}

Expand Down
2 changes: 1 addition & 1 deletion src/ripple/rpc/handlers/AccountOffers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ doAccountOffers(RPC::JsonContext& context)
if (!sle)
return rpcError(rpcINVALID_PARAMS);

if (!RPC::isOwnedByAccount(*ledger, sle, accountID))
if (!RPC::isRelatedToAccount(*ledger, sle, accountID))
return rpcError(rpcINVALID_PARAMS);
}

Expand Down
11 changes: 9 additions & 2 deletions src/ripple/rpc/impl/RPCHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <ripple/app/misc/Transaction.h>
#include <ripple/app/paths/TrustLine.h>
#include <ripple/app/rdb/RelationalDatabase.h>
#include <ripple/app/tx/impl/details/NFTokenUtils.h>
#include <ripple/ledger/View.h>
#include <ripple/net/RPCErr.h>
#include <ripple/protocol/AccountID.h>
Expand Down Expand Up @@ -109,7 +110,7 @@ getStartHint(std::shared_ptr<SLE const> const& sle, AccountID const& accountID)
}

bool
isOwnedByAccount(
isRelatedToAccount(
ReadView const& ledger,
std::shared_ptr<SLE const> const& sle,
AccountID const& accountID)
Expand All @@ -121,13 +122,19 @@ isOwnedByAccount(
}
else if (sle->isFieldPresent(sfAccount))
{
return sle->getAccountID(sfAccount) == accountID;
return sle->getAccountID(sfAccount) == accountID ||
(sle->isFieldPresent(sfDestination) &&
sle->getAccountID(sfDestination) == accountID);
}
else if (sle->getType() == ltSIGNER_LIST)
{
Keylet const accountSignerList = keylet::signers(accountID);
return sle->key() == accountSignerList.key;
}
else if (sle->getType() == ltNFTOKEN_OFFER)
{
return sle->getAccountID(sfOwner) == accountID;
}

return false;
}
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 @@ -86,7 +86,7 @@ getStartHint(std::shared_ptr<SLE const> const& sle, AccountID const& accountID);
* @param account - The account being tested for SLE ownership.
*/
bool
isOwnedByAccount(
isRelatedToAccount(
ReadView const& ledger,
std::shared_ptr<SLE const> const& sle,
AccountID const& accountID);
Expand Down
192 changes: 190 additions & 2 deletions src/test/rpc/AccountLinesRPC_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class AccountLinesRPC_test : public beast::unit_test::suite
void
testAccountLines()
{
testcase("acccount_lines");
testcase("account_lines");

using namespace test::jtx;
Env env(*this);
Expand Down Expand Up @@ -524,11 +524,198 @@ class AccountLinesRPC_test : public beast::unit_test::suite
RPC::make_error(rpcINVALID_PARAMS)[jss::error_message]);
}

void
testAccountLinesWalkMarkers()
{
testcase("Marker can point to any appropriate ledger entry type");
using namespace test::jtx;
using namespace std::chrono_literals;
Env env(*this);

// The goal of this test is observe account_lines RPC calls return an
// error message when the SLE pointed to by the marker is not owned by
// the Account being traversed.
//
// To start, we'll create an environment with some trust lines, offers
// and a signers list.
Account const alice{"alice"};
Account const becky{"becky"};
Account const gw1{"gw1"};
env.fund(XRP(10000), alice, becky, gw1);
env.close();

// A couple of helper lambdas
auto escrow = [&env](
Account const& account,
Account const& to,
STAmount const& amount) {
Json::Value jv;
jv[jss::TransactionType] = jss::EscrowCreate;
jv[jss::Flags] = tfUniversal;
jv[jss::Account] = account.human();
jv[jss::Destination] = to.human();
jv[jss::Amount] = amount.getJson(JsonOptions::none);
NetClock::time_point finish = env.now() + 1s;
jv[sfFinishAfter.jsonName] = finish.time_since_epoch().count();
return jv;
};

auto payChan = [](Account const& account,
Account const& to,
STAmount const& amount,
NetClock::duration const& settleDelay,
PublicKey const& pk) {
Json::Value jv;
jv[jss::TransactionType] = jss::PaymentChannelCreate;
jv[jss::Flags] = tfUniversal;
jv[jss::Account] = account.human();
jv[jss::Destination] = to.human();
jv[jss::Amount] = amount.getJson(JsonOptions::none);
jv["SettleDelay"] = settleDelay.count();
jv["PublicKey"] = strHex(pk.slice());
return jv;
};

// Test all available object types. Not all of these objects will be
// included in the search, nor found by `account_objects`. If that ever
// changes for any reason, this test will help catch that.
//
// SignerList, for alice
Account const bogie{"bogie"};
env(signers(alice, 2, {{bogie, 3}}));
env.close();

// SignerList, includes alice
env(signers(becky, 2, {{alice, 3}}));
env.close();

// Trust lines
auto const EUR = gw1["EUR"];
env(trust(alice, EUR(200)));
env(trust(becky, EUR(200)));
env.close();

// Escrow, in each direction
env(escrow(alice, becky, XRP(1000)));
env(escrow(becky, alice, XRP(1000)));

// Pay channels, in each direction
env(payChan(alice, becky, XRP(1000), 100s, alice.pk()));
env(payChan(becky, alice, XRP(1000), 100s, becky.pk()));

// Mint NFTs, for each account
uint256 const aliceNFtokenID =
token::getNextID(env, alice, 0, tfTransferable);
env(token::mint(alice, 0), txflags(tfTransferable));

uint256 const beckyNFtokenID =
token::getNextID(env, becky, 0, tfTransferable);
env(token::mint(becky, 0), txflags(tfTransferable));

// NFT Offers, for each other's NFTs
env(token::createOffer(alice, beckyNFtokenID, drops(1)),
token::owner(becky));
env(token::createOffer(becky, aliceNFtokenID, drops(1)),
token::owner(alice));

// Checks, in each direction
env(check::create(alice, becky, XRP(50)));
env(check::create(becky, alice, XRP(50)));

// Deposit preauth, in each direction
env(deposit::auth(alice, becky));
env(deposit::auth(becky, alice));

// Offers, one where alice is the owner, and one where alice is the
// issuer
auto const USDalice = alice["USD"];
env(offer(alice, EUR(10), XRP(100)));
env(offer(becky, USDalice(10), XRP(100)));

// Tickets
env(ticket::create(alice, 2));

// Add another trustline for good measure
auto const BTCbecky = becky["BTC"];
env(trust(alice, BTCbecky(200)));

env.close();

{
// Now make repeated calls to `account_lines` with a limit of 1.
// That should iterate all of alice's relevant objects, even though
// the list will be empty for most calls.
auto aliceLines = env.rpc(
"json",
"account_lines",
R"({"account": ")" + alice.human() + R"(", "limit": 1})");
std::size_t const expectedLines = 2;
std::size_t foundLines = 0;

auto hasMarker = [](auto const& aliceLines) {
return aliceLines[jss::result].isMember(jss::marker);
};
auto marker = [](auto const& aliceLines) {
return aliceLines[jss::result][jss::marker].asString();
};
auto checkLines = [](auto const& aliceLines) {
return aliceLines.isMember(jss::result) &&
!aliceLines[jss::result].isMember(jss::error_message) &&
aliceLines[jss::result].isMember(jss::lines) &&
aliceLines[jss::result][jss::lines].isArray() &&
aliceLines[jss::result][jss::lines].size() <= 1;
};

BEAST_EXPECT(hasMarker(aliceLines));
BEAST_EXPECT(checkLines(aliceLines));
BEAST_EXPECT(aliceLines[jss::result][jss::lines].size() == 0);

int iterations = 1;

while (hasMarker(aliceLines))
{
// Iterate through the markers
aliceLines = env.rpc(
"json",
"account_lines",
R"({"account": ")" + alice.human() + R"(", "marker": ")" +
marker(aliceLines) + R"(", "limit": 1})");
BEAST_EXPECT(checkLines(aliceLines));
foundLines += aliceLines[jss::result][jss::lines].size();
++iterations;
}
BEAST_EXPECT(expectedLines == foundLines);

Json::Value const aliceObjects = env.rpc(
"json",
"account_objects",
R"({"account": ")" + alice.human() +
R"(", )"
R"("limit": 200})");
BEAST_EXPECT(aliceObjects.isMember(jss::result));
BEAST_EXPECT(
!aliceObjects[jss::result].isMember(jss::error_message));
BEAST_EXPECT(
aliceObjects[jss::result].isMember(jss::account_objects));
BEAST_EXPECT(
aliceObjects[jss::result][jss::account_objects].isArray());
// account_objects does not currently return NFTPages. If
// that ever changes, without also changing account_lines,
// this test will need to be updated.
BEAST_EXPECT(
aliceObjects[jss::result][jss::account_objects].size() ==
iterations);
// If ledger object association ever changes, for whatever
// reason, this test will need to be updated.
BEAST_EXPECTS(iterations == 14, std::to_string(iterations));
}
}

// test API V2
void
testAccountLines2()
{
testcase("V2: acccount_lines");
testcase("V2: account_lines");

using namespace test::jtx;
Env env(*this);
Expand Down Expand Up @@ -1234,6 +1421,7 @@ class AccountLinesRPC_test : public beast::unit_test::suite
testAccountLines();
testAccountLinesMarker();
testAccountLineDelete();
testAccountLinesWalkMarkers();
testAccountLines2();
testAccountLineDelete2();
}
Expand Down

0 comments on commit 25889a7

Please sign in to comment.