amm_info: fetch by amm account id; add AMM object entry (#4682)

- Update amm_info to fetch AMM by amm account id.
  - This is an additional way to retrieve an AMM object.
  - Alternatively, AMM can still be fetched by the asset pair as well.
- Add owner directory entry for AMM object.

Context:

- Add back the AMM object directory entry, which was deleted by #4626.
  - This fixes `account_objects` for `amm` type.

src/test/jtx/AMM.h

@@ -105,7 +105,10 @@ public:
     ammRpcInfo(
         std::optional<AccountID> const& account = std::nullopt,
         std::optional<std::string> const& ledgerIndex = std::nullopt,
-        std::optional<std::pair<Issue, Issue>> tokens = std::nullopt) const;
+        std::optional<Issue> issue1 = std::nullopt,
+        std::optional<Issue> issue2 = std::nullopt,
+        std::optional<AccountID> const& ammAccount = std::nullopt,
+        bool ignoreParams = false) const;
 
     /** Verify the AMM balances.
      */
@@ -150,7 +153,8 @@ public:
         STAmount const& asset2,
         IOUAmount const& balance,
         std::optional<AccountID> const& account = std::nullopt,
-        std::optional<std::string> const& ledger_index = std::nullopt) const;
+        std::optional<std::string> const& ledger_index = std::nullopt,
+        std::optional<AccountID> const& ammAccount = std::nullopt) const;
 
     [[nodiscard]] bool
     ammExists() const;

src/test/jtx/impl/AMM.cpp

@@ -136,26 +136,36 @@ Json::Value
 AMM::ammRpcInfo(
     std::optional<AccountID> const& account,
     std::optional<std::string> const& ledgerIndex,
-    std::optional<std::pair<Issue, Issue>> tokens) const
+    std::optional<Issue> issue1,
+    std::optional<Issue> issue2,
+    std::optional<AccountID> const& ammAccount,
+    bool ignoreParams) const
 {
     Json::Value jv;
     if (account)
         jv[jss::account] = to_string(*account);
     if (ledgerIndex)
         jv[jss::ledger_index] = *ledgerIndex;
-    if (tokens)
+    if (!ignoreParams)
     {
-        jv[jss::asset] =
-            STIssue(sfAsset, tokens->first).getJson(JsonOptions::none);
-        jv[jss::asset2] =
-            STIssue(sfAsset2, tokens->second).getJson(JsonOptions::none);
-    }
-    else
-    {
-        jv[jss::asset] =
-            STIssue(sfAsset, asset1_.issue()).getJson(JsonOptions::none);
-        jv[jss::asset2] =
-            STIssue(sfAsset2, asset2_.issue()).getJson(JsonOptions::none);
+        if (issue1 || issue2)
+        {
+            if (issue1)
+                jv[jss::asset] =
+                    STIssue(sfAsset, *issue1).getJson(JsonOptions::none);
+            if (issue2)
+                jv[jss::asset2] =
+                    STIssue(sfAsset2, *issue2).getJson(JsonOptions::none);
+        }
+        else if (!ammAccount)
+        {
+            jv[jss::asset] =
+                STIssue(sfAsset, asset1_.issue()).getJson(JsonOptions::none);
+            jv[jss::asset2] =
+                STIssue(sfAsset2, asset2_.issue()).getJson(JsonOptions::none);
+        }
+        if (ammAccount)
+            jv[jss::amm_account] = to_string(*ammAccount);
     }
     auto jr = env_.rpc("json", "amm_info", to_string(jv));
     if (jr.isObject() && jr.isMember(jss::result) &&
@@ -292,9 +302,11 @@ AMM::expectAmmRpcInfo(
     STAmount const& asset2,
     IOUAmount const& balance,
     std::optional<AccountID> const& account,
-    std::optional<std::string> const& ledger_index) const
+    std::optional<std::string> const& ledger_index,
+    std::optional<AccountID> const& ammAccount) const
 {
-    auto const jv = ammRpcInfo(account, ledger_index);
+    auto const jv = ammRpcInfo(
+        account, ledger_index, std::nullopt, std::nullopt, ammAccount);
     return expectAmmInfo(asset1, asset2, balance, jv);
 }
 

src/test/rpc/AMMInfo_test.cpp

@@ -42,7 +42,7 @@ public:
             Account const gw("gw");
             auto const USD = gw["USD"];
             auto const jv =
-                ammAlice.ammRpcInfo({}, {}, {{USD.issue(), USD.issue()}});
+                ammAlice.ammRpcInfo({}, {}, USD.issue(), USD.issue());
             BEAST_EXPECT(jv[jss::error_message] == "Account not found.");
         });
 
@@ -52,6 +52,40 @@ public:
             auto const jv = ammAlice.ammRpcInfo(bogie.id());
             BEAST_EXPECT(jv[jss::error_message] == "Account malformed.");
         });
+
+        // Invalid parameters
+        testAMM([&](AMM& ammAlice, Env&) {
+            std::vector<std::tuple<
+                std::optional<Issue>,
+                std::optional<Issue>,
+                std::optional<AccountID>,
+                bool>>
+                vals = {
+                    {xrpIssue(), std::nullopt, std::nullopt, false},
+                    {std::nullopt, USD.issue(), std::nullopt, false},
+                    {xrpIssue(), std::nullopt, ammAlice.ammAccount(), false},
+                    {std::nullopt, USD.issue(), ammAlice.ammAccount(), false},
+                    {xrpIssue(), USD.issue(), ammAlice.ammAccount(), false},
+                    {std::nullopt, std::nullopt, std::nullopt, true}};
+            for (auto const& [iss1, iss2, acct, ignoreParams] : vals)
+            {
+                auto const jv = ammAlice.ammRpcInfo(
+                    std::nullopt, std::nullopt, iss1, iss2, acct, ignoreParams);
+                BEAST_EXPECT(jv[jss::error_message] == "Invalid parameters.");
+            }
+        });
+
+        // Invalid AMM account id
+        testAMM([&](AMM& ammAlice, Env&) {
+            Account bogie("bogie");
+            auto const jv = ammAlice.ammRpcInfo(
+                std::nullopt,
+                std::nullopt,
+                std::nullopt,
+                std::nullopt,
+                bogie.id());
+            BEAST_EXPECT(jv[jss::error_message] == "Account malformed.");
+        });
     }
 
     void
@@ -63,6 +97,13 @@ public:
         testAMM([&](AMM& ammAlice, Env&) {
             BEAST_EXPECT(ammAlice.expectAmmRpcInfo(
                 XRP(10000), USD(10000), IOUAmount{10000000, 0}));
+            BEAST_EXPECT(ammAlice.expectAmmRpcInfo(
+                XRP(10000),
+                USD(10000),
+                IOUAmount{10000000, 0},
+                std::nullopt,
+                std::nullopt,
+                ammAlice.ammAccount()));
         });
     }
 
@@ -91,53 +132,71 @@ public:
             env.fund(XRP(1000), bob, ed, bill);
             ammAlice.bid(alice, 100, std::nullopt, {carol, bob, ed, bill});
             BEAST_EXPECT(ammAlice.expectAmmRpcInfo(
-                XRP(80000), USD(80000), IOUAmount{79994400}));
-            std::unordered_set<std::string> authAccounts = {
-                carol.human(), bob.human(), ed.human(), bill.human()};
-            auto const ammInfo = ammAlice.ammRpcInfo();
-            auto const& amm = ammInfo[jss::amm];
-            try
+                XRP(80000),
+                USD(80000),
+                IOUAmount{79994400},
+                std::nullopt,
+                std::nullopt,
+                ammAlice.ammAccount()));
+            for (auto i = 0; i < 2; ++i)
             {
-                // votes
-                auto const voteSlots = amm[jss::vote_slots];
-                for (std::uint8_t i = 0; i < 8; ++i)
+                std::unordered_set<std::string> authAccounts = {
+                    carol.human(), bob.human(), ed.human(), bill.human()};
+                auto const ammInfo = i ? ammAlice.ammRpcInfo()
+                                       : ammAlice.ammRpcInfo(
+                                             std::nullopt,
+                                             std::nullopt,
+                                             std::nullopt,
+                                             std::nullopt,
+                                             ammAlice.ammAccount());
+                auto const& amm = ammInfo[jss::amm];
+                try
                 {
-                    if (!BEAST_EXPECT(
-                            votes[voteSlots[i][jss::account].asString()] ==
-                                voteSlots[i][jss::trading_fee].asUInt() &&
-                            voteSlots[i][jss::vote_weight].asUInt() == 12500))
+                    // votes
+                    auto const voteSlots = amm[jss::vote_slots];
+                    auto votesCopy = votes;
+                    for (std::uint8_t i = 0; i < 8; ++i)
+                    {
+                        if (!BEAST_EXPECT(
+                                votes[voteSlots[i][jss::account].asString()] ==
+                                    voteSlots[i][jss::trading_fee].asUInt() &&
+                                voteSlots[i][jss::vote_weight].asUInt() ==
+                                    12500))
+                            return;
+                        votes.erase(voteSlots[i][jss::account].asString());
+                    }
+                    if (!BEAST_EXPECT(votes.empty()))
                         return;
-                    votes.erase(voteSlots[i][jss::account].asString());
-                }
-                if (!BEAST_EXPECT(votes.empty()))
-                    return;
+                    votes = votesCopy;
 
-                // bid
-                auto const auctionSlot = amm[jss::auction_slot];
-                for (std::uint8_t i = 0; i < 4; ++i)
-                {
-                    if (!BEAST_EXPECT(authAccounts.contains(
+                    // bid
+                    auto const auctionSlot = amm[jss::auction_slot];
+                    for (std::uint8_t i = 0; i < 4; ++i)
+                    {
+                        if (!BEAST_EXPECT(authAccounts.contains(
+                                auctionSlot[jss::auth_accounts][i][jss::account]
+                                    .asString())))
+                            return;
+                        authAccounts.erase(
                             auctionSlot[jss::auth_accounts][i][jss::account]
-                                .asString())))
+                                .asString());
+                    }
+                    if (!BEAST_EXPECT(authAccounts.empty()))
                         return;
-                    authAccounts.erase(
-                        auctionSlot[jss::auth_accounts][i][jss::account]
-                            .asString());
+                    BEAST_EXPECT(
+                        auctionSlot[jss::account].asString() == alice.human() &&
+                        auctionSlot[jss::discounted_fee].asUInt() == 17 &&
+                        auctionSlot[jss::price][jss::value].asString() ==
+                            "5600" &&
+                        auctionSlot[jss::price][jss::currency].asString() ==
+                            to_string(ammAlice.lptIssue().currency) &&
+                        auctionSlot[jss::price][jss::issuer].asString() ==
+                            to_string(ammAlice.lptIssue().account));
+                }
+                catch (std::exception const& e)
+                {
+                    fail(e.what(), __FILE__, __LINE__);
                 }
-                if (!BEAST_EXPECT(authAccounts.empty()))
-                    return;
-                BEAST_EXPECT(
-                    auctionSlot[jss::account].asString() == alice.human() &&
-                    auctionSlot[jss::discounted_fee].asUInt() == 17 &&
-                    auctionSlot[jss::price][jss::value].asString() == "5600" &&
-                    auctionSlot[jss::price][jss::currency].asString() ==
-                        to_string(ammAlice.lptIssue().currency) &&
-                    auctionSlot[jss::price][jss::issuer].asString() ==
-                        to_string(ammAlice.lptIssue().account));
-            }
-            catch (std::exception const& e)
-            {
-                fail(e.what(), __FILE__, __LINE__);
             }
         });
     }

src/test/rpc/AccountObjects_test.cpp

@@ -22,6 +22,7 @@
 #include <ripple/json/to_string.h>
 #include <ripple/protocol/jss.h>
 #include <test/jtx.h>
+#include <test/jtx/AMM.h>
 
 #include <boost/utility/string_ref.hpp>
 
@@ -552,10 +553,19 @@ public:
         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<Json::StaticString> const& type,
+                             std::optional<std::uint16_t> limit = std::nullopt,
+                             std::optional<std::string> 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 @@ public:
         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,67 @@ public:
                 BEAST_EXPECT(aobjs[0u]["LedgerEntryType"] == jss::Escrow);
             }
         }
+        {
+            // Make a lambda to get the types
+            auto getTypes = [&](Json::Value const& resp,
+                                std::vector<std::string>& typesOut) {
+                auto const objs = resp[jss::result][jss::account_objects];
+                for (auto const& obj : resp[jss::result][jss::account_objects])
+                    typesOut.push_back(
+                        obj[sfLedgerEntryType.fieldName].asString());
+                std::sort(typesOut.begin(), typesOut.end());
+            };
+            // 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<std::string> const& types) -> bool {
+                if (!acct_objs_is_size(resp, types.size()))
+                    return false;
+                std::vector<std::string> typesOut;
+                getTypes(resp, typesOut);
+                return types == typesOut;
+            };
+            // 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));
+            // request first two objects
+            auto resp = acct_objs(amm.ammAccount(), std::nullopt, 2);
+            std::vector<std::string> typesOut;
+            getTypes(resp, typesOut);
+            // request next two objects
+            resp = acct_objs(
+                amm.ammAccount(),
+                std::nullopt,
+                10,
+                resp[jss::result][jss::marker].asString());
+            getTypes(resp, typesOut);
+            BEAST_EXPECT(
+                (typesOut ==
+                 std::vector<std::string>{
+                     jss::AMM.c_str(),
+                     jss::RippleState.c_str(),
+                     jss::RippleState.c_str(),
+                     jss::RippleState.c_str()}));
+            // filter by state: there are three trustlines
+            resp = acct_objs(amm.ammAccount(), jss::state, 10);
+            BEAST_EXPECT(expectObjects(
+                resp,
+                {jss::RippleState.c_str(),
+                 jss::RippleState.c_str(),
+                 jss::RippleState.c_str()}));
+            // 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.