From 14282f84314ef8f9df52e25074562d7c22c91036 Mon Sep 17 00:00:00 2001 From: gregtatcam Date: Mon, 28 Aug 2023 13:21:43 +0100 Subject: [PATCH] Fix account_objects command for AMM objects and add AMM unit-test for account_objects. --- src/ripple/rpc/impl/RPCHelpers.cpp | 34 ++++++++++++++ src/test/rpc/AccountObjects_test.cpp | 69 ++++++++++++++++++++++++++-- 2 files changed, 100 insertions(+), 3 deletions(-) diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index 4a517733637..80c0180d0a6 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -236,6 +236,40 @@ getAccountObjects( entryIndex = beast::zero; } + // AMM root account requires special handling to fetch AMM object and + // trustlines + bool const filterAMM = + typeFilter.has_value() && typeMatchesFilter(typeFilter.value(), ltAMM); + // skip amm if marker (dirIndex) is provided + if (dirIndex == uint256{} && (!typeFilter.has_value() || filterAMM)) + { + if (auto const sle = ledger.read(keylet::account(account)); !sle) + return false; + else if (bool const isAMM = sle->isFieldPresent(sfAMMID); + filterAMM && !isAMM) + return false; + else if (isAMM) + { + auto const sleAMM = + ledger.read(keylet::amm(sle->getFieldH256(sfAMMID))); + if (!sleAMM) + return false; + + jvObjects.append(sleAMM->getJson(JsonOptions::none)); + + if (filterAMM) + return true; + if (limit == 1) + { + jvResult[jss::limit] = limit; + jvResult[jss::marker] = "AMM"; + return true; + } + if (limit > 1) + --mlimit; + } + } + auto const root = keylet::ownerDir(account); auto found = false; diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index 7de5b73671e..d09dd1178b3 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include @@ -552,10 +553,19 @@ class AccountObjects_test : public beast::unit_test::suite Env env(*this); // Make a lambda we can use to get "account_objects" easily. - auto acct_objs = [&env](Account const& acct, char const* type) { + auto acct_objs = [&env]( + AccountID const& acct, + std::optional const& type, + std::optional limit = std::nullopt, + std::optional marker = std::nullopt) { Json::Value params; - params[jss::account] = acct.human(); - params[jss::type] = type; + params[jss::account] = to_string(acct); + if (type) + params[jss::type] = *type; + if (limit) + params[jss::limit] = *limit; + if (marker) + params[jss::marker] = *marker; params[jss::ledger_index] = "validated"; return env.rpc("json", "account_objects", to_string(params)); }; @@ -585,6 +595,7 @@ class AccountObjects_test : public beast::unit_test::suite BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::signer_list), 0)); BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::state), 0)); BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::ticket), 0)); + BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::amm), 0)); // gw mints an NFT so we can find it. uint256 const nftID{token::getNextID(env, gw, 0u, tfTransferable)}; @@ -782,6 +793,58 @@ class AccountObjects_test : public beast::unit_test::suite BEAST_EXPECT(aobjs[0u]["LedgerEntryType"] == jss::Escrow); } } + { + // Make a lambda we can use to check the number of fetched + // account objects and their ledger type + auto expectObjects = + [&](Json::Value const& resp, + std::vector const& types) -> bool { + if (!acct_objs_is_size(resp, types.size())) + return false; + auto const objs = resp[jss::result][jss::account_objects]; + for (std::uint32_t i = 0; i < types.size(); ++i) + if (objs[i][sfLedgerEntryType.fieldName].asString() != + types[i]) + return false; + return true; + }; + // Find AMM objects + AMM amm(env, gw, XRP(1'000), USD(1'000)); + amm.deposit(alice, USD(1)); + // AMM account has 4 objects: AMM object and 3 trustlines + auto const lines = getAccountLines(env, amm.ammAccount()); + BEAST_EXPECT(lines[jss::lines].size() == 3); + // request AMM only, doesn't depend on the limit + BEAST_EXPECT( + acct_objs_is_size(acct_objs(amm.ammAccount(), jss::amm), 1)); + BEAST_EXPECT( + acct_objs_is_size(acct_objs(amm.ammAccount(), jss::amm, 1), 1)); + BEAST_EXPECT( + acct_objs_is_size(acct_objs(amm.ammAccount(), jss::amm, 2), 1)); + // no filter and limit 1 returns AMM only + auto resp = acct_objs(amm.ammAccount(), std::nullopt, 1); + BEAST_EXPECT(expectObjects(resp, {jss::AMM})); + // request first two: first two objects are AMM and trustline + resp = acct_objs(amm.ammAccount(), std::nullopt, 2); + BEAST_EXPECT(expectObjects(resp, {jss::AMM, jss::RippleState})); + // request next: next two objects are trustlines + resp = acct_objs( + amm.ammAccount(), + std::nullopt, + 10, + resp[jss::result][jss::marker].asString()); + BEAST_EXPECT( + expectObjects(resp, {jss::RippleState, jss::RippleState})); + // filter by state: there are three trustlines + resp = acct_objs(amm.ammAccount(), jss::state, 10); + BEAST_EXPECT(expectObjects( + resp, {jss::RippleState, jss::RippleState, jss::RippleState})); + // AMM account doesn't own offers + BEAST_EXPECT( + acct_objs_is_size(acct_objs(amm.ammAccount(), jss::offer), 0)); + // gw account doesn't own AMM object + BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::amm), 0)); + } // Run up the number of directory entries so gw has two // directory nodes.