diff --git a/src/ripple/rpc/handlers/AccountChannels.cpp b/src/ripple/rpc/handlers/AccountChannels.cpp index e5059d3ffc5..9e5c9ca2c46 100644 --- a/src/ripple/rpc/handlers/AccountChannels.cpp +++ b/src/ripple/rpc/handlers/AccountChannels.cpp @@ -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); } diff --git a/src/ripple/rpc/handlers/AccountLines.cpp b/src/ripple/rpc/handlers/AccountLines.cpp index 364d40673fa..adba2acaa72 100644 --- a/src/ripple/rpc/handlers/AccountLines.cpp +++ b/src/ripple/rpc/handlers/AccountLines.cpp @@ -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); } diff --git a/src/ripple/rpc/handlers/AccountOffers.cpp b/src/ripple/rpc/handlers/AccountOffers.cpp index e957fe8a8e0..409d071fb02 100644 --- a/src/ripple/rpc/handlers/AccountOffers.cpp +++ b/src/ripple/rpc/handlers/AccountOffers.cpp @@ -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); } diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index 3d1bfe6375b..ad84d7b12f5 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -109,7 +110,7 @@ getStartHint(std::shared_ptr const& sle, AccountID const& accountID) } bool -isOwnedByAccount( +isRelatedToAccount( ReadView const& ledger, std::shared_ptr const& sle, AccountID const& accountID) @@ -121,13 +122,27 @@ isOwnedByAccount( } else if (sle->isFieldPresent(sfAccount)) { - return sle->getAccountID(sfAccount) == accountID; + // If there's an sfAccount present, also test the sfDestination, if + // present. This will match objects such as Escrows (ltESCROW), Payment + // Channels (ltPAYCHAN), and Checks (ltCHECK) because those are added to + // the Destination account's directory. It intentionally EXCLUDES + // NFToken Offers (ltNFTOKEN_OFFER). NFToken Offers are NOT added to the + // Destination account's directory. + 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) + { + // Do not check the sfDestination field. NFToken Offers are NOT added to + // the Destination account's directory. + return sle->getAccountID(sfOwner) == accountID; + } return false; } diff --git a/src/ripple/rpc/impl/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h index 2aa62f3474a..12f27641ddf 100644 --- a/src/ripple/rpc/impl/RPCHelpers.h +++ b/src/ripple/rpc/impl/RPCHelpers.h @@ -86,7 +86,7 @@ getStartHint(std::shared_ptr const& sle, AccountID const& accountID); * @param account - The account being tested for SLE ownership. */ bool -isOwnedByAccount( +isRelatedToAccount( ReadView const& ledger, std::shared_ptr const& sle, AccountID const& accountID); diff --git a/src/test/rpc/AccountLinesRPC_test.cpp b/src/test/rpc/AccountLinesRPC_test.cpp index bdd376b3aae..cdc61922097 100644 --- a/src/test/rpc/AccountLinesRPC_test.cpp +++ b/src/test/rpc/AccountLinesRPC_test.cpp @@ -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); @@ -524,11 +524,244 @@ 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)); + + env(token::createOffer(becky, beckyNFtokenID, drops(1)), + txflags(tfSellNFToken), + token::destination(alice)); + env(token::createOffer(alice, aliceNFtokenID, drops(1)), + txflags(tfSellNFToken), + token::destination(becky)); + + env(token::createOffer(gw1, beckyNFtokenID, drops(1)), + token::owner(becky), + token::destination(alice)); + env(token::createOffer(gw1, aliceNFtokenID, drops(1)), + token::owner(alice), + token::destination(becky)); + + env(token::createOffer(becky, beckyNFtokenID, drops(1)), + txflags(tfSellNFToken)); + env(token::createOffer(alice, aliceNFtokenID, drops(1)), + txflags(tfSellNFToken)); + + // 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 getNextLine = [](Env& env, + Account const& alice, + std::optional const marker) { + Json::Value params(Json::objectValue); + params[jss::account] = alice.human(); + params[jss::limit] = 1; + if (marker) + params[jss::marker] = *marker; + + return env.rpc("json", "account_lines", to_string(params)); + }; + + auto aliceLines = getNextLine(env, alice, std::nullopt); + constexpr std::size_t expectedIterations = 16; + constexpr std::size_t 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 = getNextLine(env, alice, marker(aliceLines)); + 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 == expectedIterations, std::to_string(iterations)); + + // Get becky's objects just to confirm that they're symmetrical + Json::Value const beckyObjects = env.rpc( + "json", + "account_objects", + R"({"account": ")" + becky.human() + + R"(", )" + R"("limit": 200})"); + BEAST_EXPECT(beckyObjects.isMember(jss::result)); + BEAST_EXPECT( + !beckyObjects[jss::result].isMember(jss::error_message)); + BEAST_EXPECT( + beckyObjects[jss::result].isMember(jss::account_objects)); + BEAST_EXPECT( + beckyObjects[jss::result][jss::account_objects].isArray()); + // becky should have the same number of objects as alice, except the + // 2 tickets that only alice created. + BEAST_EXPECT( + beckyObjects[jss::result][jss::account_objects].size() == + aliceObjects[jss::result][jss::account_objects].size() - 2); + } + } + // test API V2 void testAccountLines2() { - testcase("V2: acccount_lines"); + testcase("V2: account_lines"); using namespace test::jtx; Env env(*this); @@ -1234,6 +1467,7 @@ class AccountLinesRPC_test : public beast::unit_test::suite testAccountLines(); testAccountLinesMarker(); testAccountLineDelete(); + testAccountLinesWalkMarkers(); testAccountLines2(); testAccountLineDelete2(); }