Delete AMM MPToken on AMM delete.

Update for MPT isOnlyLiquidityProvider().
Update checkReserve() in AMMWithdraw.
Update invariant for AMMWithdraw.
Add deleteMPToken to View.

include/xrpl/protocol/Indexes.h

@@ -310,7 +310,7 @@ Keylet
 mptIssuance(std::uint32_t seq, AccountID const& issuer) noexcept;
 
 Keylet
-mptIssuance(MPTID const& issuanceID) noexcept;
+mptIssuance(MPTIssue const& mptIssue) noexcept;
 
 inline Keylet
 mptIssuance(uint256 const& issuanceKey)

include/xrpl/protocol/STAmount.h

@@ -568,6 +568,8 @@ STAmount::clear(Asset const& asset)
 inline void
 STAmount::setIssuer(AccountID const& uIssuer)
 {
+    if (!mAsset.holds<Issue>())
+        Throw<std::runtime_error>("Can't set issuer for non-Issue");
     mAsset.get<Issue>().account = uIssuer;
 }
 

src/libxrpl/protocol/Indexes.cpp

@@ -549,11 +549,11 @@ mptIssuance(std::uint32_t seq, AccountID const& issuer) noexcept
 }
 
 Keylet
-mptIssuance(MPTID const& issuanceID) noexcept
+mptIssuance(MPTIssue const& mptIssue) noexcept
 {
     return {
         ltMPTOKEN_ISSUANCE,
-        indexHash(LedgerNameSpace::MPTOKEN_ISSUANCE, issuanceID)};
+        indexHash(LedgerNameSpace::MPTOKEN_ISSUANCE, mptIssue.getMptID())};
 }
 
 Keylet

src/test/app/MPToken_test.cpp

@@ -3583,6 +3583,51 @@ class MPToken_test : public beast::unit_test::suite
         }
     }
 
+    void
+    testBasicAMM(FeatureBitset features)
+    {
+        testcase("Basic AMM");
+        using namespace jtx;
+        Account const gw{"gw"};
+        Account const alice{"alice"};
+        Account const carol{"carol"};
+        auto const USD = gw["USD"];
+        Env env{*this};
+
+        fund(env, gw, {alice, carol}, XRP(1'000), {USD(1'000)});
+
+        MPTTester mpt(env, gw, {.fund = false});
+        mpt.create({.flags = tfMPTCanTransfer | tfMPTCanTrade});
+        auto const MPT = mpt["MPT"];
+        mpt.authorize({.account = alice});
+        mpt.authorize({.account = carol});
+        mpt.pay(gw, alice, 1'000);
+        mpt.pay(gw, carol, 1'000);
+
+        MPTTester mpt1(env, gw, {.fund = false});
+        mpt1.create({.flags = tfMPTCanTransfer | tfMPTCanTrade});
+        auto const MPT1 = mpt1["MPT1"];
+        mpt1.authorize({.account = alice});
+        mpt1.authorize({.account = carol});
+        mpt1.pay(gw, alice, 1'000);
+        mpt1.pay(gw, carol, 1'000);
+
+        std::vector<std::tuple<PrettyAmount, PrettyAmount, IOUAmount>> pools = {
+            {XRP(100), MPT(100), IOUAmount{100'000}},
+            {USD(100), MPT(100), IOUAmount{100}},
+            {MPT(100), MPT1(100), IOUAmount{100}}};
+        for (auto& pool : pools)
+        {
+            AMM amm(env, gw, std::get<0>(pool), std::get<1>(pool));
+            amm.deposit(alice, std::get<2>(pool));
+            amm.deposit(carol, std::get<2>(pool));
+            amm.withdrawAll(alice);
+            amm.withdrawAll(carol);
+            amm.withdrawAll(gw);
+            BEAST_EXPECT(!amm.ammExists());
+        }
+    }
+
 public:
     void
     run() override
@@ -3647,6 +3692,9 @@ class MPToken_test : public beast::unit_test::suite
 
         // Add AMMClawback
         testAMMClawback(all);
+
+        // Test AMM
+        testBasicAMM(all);
     }
 };
 

src/test/rpc/AMMInfo_test.cpp

@@ -210,7 +210,7 @@ class AMMInfo_test : public jtx::AMMTestBase
             auto const MPT1 = mpt1["MPT"];
             std::vector<std::tuple<PrettyAmount, PrettyAmount, IOUAmount>>
                 pools = {
-                    {XRP(100), MPT(100), IOUAmount{100000}},
+                    {XRP(100), MPT(100), IOUAmount{100'000}},
                     {USD(100), MPT(100), IOUAmount{100}},
                     {MPT(100), MPT1(100), IOUAmount{100}}};
             for (auto& pool : pools)

src/xrpld/app/misc/detail/AMMUtils.cpp

@@ -218,7 +218,7 @@ ammAccountHolds(
 }
 
 static TER
-deleteAMMTrustLines(
+deleteAMMObjects(
     Sandbox& sb,
     AccountID const& ammAccountID,
     std::uint16_t maxTrustlinesToDelete,
@@ -233,7 +233,13 @@ deleteAMMTrustLines(
             // Skip AMM
             if (nodeType == LedgerEntryType::ltAMM)
                 return {tesSUCCESS, SkipEntry::Yes};
-            // Should only have the trustlines
+
+            if (nodeType == ltMPTOKEN)
+                return {
+                    deleteAMMMPToken(sb, sleItem, ammAccountID, j),
+                    SkipEntry::No};
+
+            // Only the trustlines should remain
             if (nodeType != LedgerEntryType::ltRIPPLE_STATE)
             {
                 // LCOV_EXCL_START
@@ -292,41 +298,10 @@ deleteAMMAccount(
     }
 
     if (auto const ter =
-            deleteAMMTrustLines(sb, ammAccountID, maxDeletableAMMTrustLines, j);
+            deleteAMMObjects(sb, ammAccountID, maxDeletableAMMTrustLines, j);
         ter != tesSUCCESS)
         return ter;
 
-    auto checkDeleteMPToken = [&](Asset const& asset_) -> TER {
-        if (asset_.holds<MPTIssue>())
-        {
-            auto const mptIssuanceID =
-                keylet::mptIssuance(asset_.get<MPTIssue>().getMptID());
-            auto const mptokenKey =
-                keylet::mptoken(mptIssuanceID.key, ammAccountID);
-
-            auto const sleMpt = sb.peek(mptokenKey);
-            if (!sleMpt)
-                return tecINTERNAL;
-
-            if (!sb.dirRemove(
-                    keylet::ownerDir(ammAccountID),
-                    (*sleMpt)[sfOwnerNode],
-                    sleMpt->key(),
-                    false))
-                return tecINTERNAL;
-
-            sb.erase(sleMpt);
-        }
-
-        return tesSUCCESS;
-    };
-
-    if (auto const err = checkDeleteMPToken(asset))
-        return err;
-
-    if (auto const err = checkDeleteMPToken(asset2))
-        return err;
-
     auto const ownerDirKeylet = keylet::ownerDir(ammAccountID);
     if (!sb.dirRemove(
             ownerDirKeylet, (*ammSle)[sfOwnerNode], ammSle->key(), false))
@@ -411,6 +386,9 @@ isOnlyLiquidityProvider(
     // For instance, if AMM has two tokens USD and EUR and LP is not the issuer
     // of the tokens then the trustlines are between AMM account and the issuer.
     std::uint8_t nIOUTrustLines = 0;
+    // There are at most two MPT objects, one for each side of the pool.
+    // TODO MPT, check logic with MPT introduction
+    std::uint8_t nMPT = 0;
     // There is only one AMM object
     bool hasAMM = false;
     // AMM LP has at most three trustlines and only one AMM object must exist.
@@ -432,15 +410,21 @@ isOnlyLiquidityProvider(
             auto const sle = view.read(keylet::child(key));
             if (!sle)
                 return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
+            auto const entryType = sle->getFieldU16(sfLedgerEntryType);
             // Only one AMM object
-            if (sle->getFieldU16(sfLedgerEntryType) == ltAMM)
+            if (entryType == ltAMM)
             {
                 if (hasAMM)
                     return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
                 hasAMM = true;
                 continue;
             }
-            if (sle->getFieldU16(sfLedgerEntryType) != ltRIPPLE_STATE)
+            if (entryType == ltMPTOKEN)
+            {
+                ++nMPT;
+                continue;
+            }
+            if (entryType != ltRIPPLE_STATE)
                 return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
             auto const lowLimit = sle->getFieldAmount(sfLowLimit);
             auto const highLimit = sle->getFieldAmount(sfHighLimit);
@@ -470,8 +454,8 @@ isOnlyLiquidityProvider(
         auto const uNodeNext = ownerDir->getFieldU64(sfIndexNext);
         if (uNodeNext == 0)
         {
-            if (nLPTokenTrustLines != 1 || nIOUTrustLines == 0 ||
-                nIOUTrustLines > 2)
+            if (nLPTokenTrustLines != 1 || (nIOUTrustLines == 0 && nMPT == 0) ||
+                (nIOUTrustLines > 2 || nMPT > 2))
                 return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
             return true;
         }

src/xrpld/app/tx/detail/AMMCreate.cpp

@@ -368,6 +368,8 @@ applyCreate(
             (*mptoken)[sfFlags] = 0;
             (*mptoken)[sfOwnerNode] = *ownerNode;
             sb.insert(mptoken);
+            // TODO MPT we don't adjust the owner count?
+            // didn't adjust for AMM object
         }
 
         if (auto const res = accountSend(

src/xrpld/app/tx/detail/AMMWithdraw.cpp

@@ -521,7 +521,7 @@ AMMWithdraw::withdraw(
     auto const expected = ammHolds(
         view,
         ammSle,
-        amountWithdraw.issue(),
+        amountWithdraw.asset(),
         std::nullopt,
         freezeHandling,
         AuthHandling::ahIGNORE_AUTH,  // ???
@@ -615,80 +615,80 @@ AMMWithdraw::withdraw(
         return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}};
     }
 
-    // Check the reserve in case a trustline has to be created
+    // Check the reserve in case a trustline or MPT has to be created
     bool const enabledFixAMMv1_2 = view.rules().enabled(fixAMMv1_2);
-    auto sufficientReserve = [&](Issue const& issue) -> TER {
-        if (!enabledFixAMMv1_2 || isXRP(issue))
+    // If seated after a call to sufficientReserve() then MPToken must be
+    // authorized
+    std::optional<Keylet> mptokenKey;
+    auto sufficientReserve = [&](Asset const& asset) -> TER {
+        mptokenKey = std::nullopt;
+        if (!enabledFixAMMv1_2 || isXRP(asset))
             return tesSUCCESS;
-        if (!view.exists(keylet::line(account, issue)))
+        bool const isIssue = asset.holds<Issue>();
+        bool const checkReserve = [&] {
+            if (isIssue)
+                return !view.exists(keylet::line(account, asset.get<Issue>()));
+            auto const issuanceKey = keylet::mptIssuance(asset.get<MPTIssue>());
+            mptokenKey = keylet::mptoken(issuanceKey.key, account);
+            if (!view.exists(keylet::mptoken(issuanceKey.key, account)))
+                return true;
+            mptokenKey = std::nullopt;
+            return false;
+        }();
+        if (checkReserve)
         {
-            auto const sleAccount = view.read(keylet::account(account));
+            auto sleAccount = view.peek(keylet::account(account));
             if (!sleAccount)
                 return tecINTERNAL;  // LCOV_EXCL_LINE
-            auto const balance = (*sleAccount)[sfBalance].xrp();
+            STAmount const balance = (*sleAccount)[sfBalance];
             std::uint32_t const ownerCount = sleAccount->at(sfOwnerCount);
 
             // See also SetTrust::doApply()
             XRPAmount const reserve(
                 (ownerCount < 2) ? XRPAmount(beast::zero)
                                  : view.fees().accountReserve(ownerCount + 1));
 
-            if (std::max(priorBalance, balance) < reserve)
+            auto const balance_ =
+                isIssue ? std::max(priorBalance, balance.xrp()) : balance.xrp();
+            if (balance_ < reserve)
                 return tecINSUFFICIENT_RESERVE;
+
+            // Update owner count.
+            if (!isIssue)
+                adjustOwnerCount(view, sleAccount, 1, journal);
         }
         return tesSUCCESS;
     };
 
-    if (auto const err = sufficientReserve(amountWithdrawActual.issue()))
+    if (auto const err = sufficientReserve(amountWithdrawActual.asset()))
         return {err, STAmount{}, STAmount{}, STAmount{}};
 
     // Create MPToken if doesn't exist
     // TODO make a library, AMMCreate, AMMAuthorize use almost identical code
     auto createMPToken = [&](Asset const& asset) -> TER {
-        if (asset.holds<MPTIssue>())
+        // If mptoken is seated then must authorize
+        if (mptokenKey)
         {
             auto const& mptIssue = asset.get<MPTIssue>();
-            auto const issuanceKey = keylet::mptIssuance(mptIssue.getMptID());
-            auto const mptokenKey = keylet::mptoken(issuanceKey.key, account);
-            if (!view.exists(mptokenKey))
-            {
-                if (auto err = requireAuth(
-                        view, mptIssue, account, MPTAuthType::WeakAuth);
-                    err != tesSUCCESS)
-                    return err;
-            }
-
-            auto const sleAcct = view.peek(keylet::account(account));
-            if (!sleAcct)
-                return tefINTERNAL;
-
-            std::uint32_t const uOwnerCount =
-                sleAcct->getFieldU32(sfOwnerCount);
-            XRPAmount const reserveCreate(
-                (uOwnerCount < 2)
-                    ? XRPAmount(beast::zero)
-                    : view.fees().accountReserve(uOwnerCount + 1));
-
-            if (priorBalance < reserveCreate)
-                return tecINSUFFICIENT_RESERVE;
+            if (auto err =
+                    requireAuth(view, mptIssue, account, MPTAuthType::WeakAuth);
+                err != tesSUCCESS)
+                return err;
 
             auto const ownerNode = view.dirInsert(
                 keylet::ownerDir(account),
-                mptokenKey,
+                *mptokenKey,
                 describeOwnerDir(account));
 
             if (!ownerNode)
                 return tecDIR_FULL;
 
-            auto mptoken = std::make_shared<SLE>(mptokenKey);
+            auto mptoken = std::make_shared<SLE>(*mptokenKey);
             (*mptoken)[sfAccount] = account;
             (*mptoken)[sfMPTokenIssuanceID] = mptIssue.getMptID();
             (*mptoken)[sfFlags] = 0;
             (*mptoken)[sfOwnerNode] = *ownerNode;
             view.insert(mptoken);
-
-            // Update owner count.
-            adjustOwnerCount(view, sleAcct, 1, journal);
         }
         return tesSUCCESS;
     };
@@ -717,7 +717,7 @@ AMMWithdraw::withdraw(
     // Withdraw amount2Withdraw
     if (amount2WithdrawActual)
     {
-        if (auto const err = sufficientReserve(amount2WithdrawActual->issue());
+        if (auto const err = sufficientReserve(amount2WithdrawActual->asset());
             err != tesSUCCESS)
             return {err, STAmount{}, STAmount{}, STAmount{}};