Fix last Liquidity Provider withdrawal:

Due to the rounding, LPTokenBalance of the last
Liquidity Provider (LP), might not match this LP's
trustline balance. This fix sets LPTokenBalance on
last LP withdrawal to this LP's LPToken trustline
balance.

src/ripple/app/misc/AMMUtils.h

@@ -113,6 +113,16 @@ initializeFeeAuctionVote(
     Issue const& lptIssue,
     std::uint16_t tfee);
 
+/** Return true if the Liquidity Provider is the only AMM provider, false
+ * otherwise. Return tecINTERNAL if encountered an unexpected condition,
+ * for instance Liquidity Provider has more than one LPToken trustline.
+ */
+Expected<bool, TER>
+isOnlyLiquidityProvider(
+    ReadView const& view,
+    Issue const& ammIssue,
+    AccountID const& lpAccount);
+
 }  // namespace ripple
 
 #endif  // RIPPLE_APP_MISC_AMMUTILS_H_INLCUDED

src/ripple/app/misc/impl/AMMHelpers.cpp

@@ -167,7 +167,7 @@ adjustAmountsByLPTokens(
     {
         bool const ammRoundingEnabled = [&]() {
             if (auto const& rules = getCurrentTransactionRules();
-                rules && rules->enabled(fixAMMRounding))
+                rules && rules->enabled(fixAMMv1_1))
                 return true;
             return false;
         }();

src/ripple/app/misc/impl/AMMUtils.cpp

@@ -348,4 +348,86 @@ initializeFeeAuctionVote(
         auctionSlot.makeFieldAbsent(sfDiscountedFee);  // LCOV_EXCL_LINE
 }
 
+Expected<bool, TER>
+isOnlyLiquidityProvider(
+    ReadView const& view,
+    Issue const& ammIssue,
+    AccountID const& lpAccount)
+{
+    // Liquidity Provider (LP) must have one LPToken trustline
+    std::uint8_t nLPTokenTrustLines = 0;
+    // There are at most two IOU trustlines. One or both could be to the LP
+    // if LP is the issuer, or a different account if LP is not an issuer.
+    // 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 is only one AMM object
+    bool hasAMM = false;
+    // AMM LP has at most three trustlines and only one AMM object must exist.
+    // If there are more than five objects then it's either an error or
+    // there are more than one LP. Ten pages should be sufficient to include
+    // five objects.
+    std::uint8_t limit = 10;
+    auto const root = keylet::ownerDir(ammIssue.account);
+    auto currentIndex = root;
+
+    // Iterate over AMM owner directory objects.
+    while (limit-- >= 1)
+    {
+        auto const ownerDir = view.read(currentIndex);
+        if (!ownerDir)
+            return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
+        for (auto const& key : ownerDir->getFieldV256(sfIndexes))
+        {
+            auto const sle = view.read(keylet::child(key));
+            if (!sle)
+                return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
+            // Only one AMM object
+            if (sle->getFieldU16(sfLedgerEntryType) == ltAMM)
+            {
+                if (hasAMM)
+                    return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
+                hasAMM = true;
+                continue;
+            }
+            if (sle->getFieldU16(sfLedgerEntryType) != ltRIPPLE_STATE)
+                return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
+            auto const lowLimit = sle->getFieldAmount(sfLowLimit);
+            auto const highLimit = sle->getFieldAmount(sfHighLimit);
+            auto const isLPTrustline = lowLimit.getIssuer() == lpAccount ||
+                highLimit.getIssuer() == lpAccount;
+            auto const isLPTokenTrustline =
+                lowLimit.issue() == ammIssue || highLimit.issue() == ammIssue;
+
+            // Liquidity Provider trustline
+            if (isLPTrustline)
+            {
+                // LPToken trustline
+                if (isLPTokenTrustline)
+                {
+                    if (++nLPTokenTrustLines > 1)
+                        return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
+                }
+                else if (++nIOUTrustLines > 2)
+                    return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
+            }
+            // Another Liquidity Provider LPToken trustline
+            else if (isLPTokenTrustline)
+                return false;
+            else if (++nIOUTrustLines > 2)
+                return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
+        }
+        auto const uNodeNext = ownerDir->getFieldU64(sfIndexNext);
+        if (uNodeNext == 0)
+        {
+            if (nLPTokenTrustLines != 1 || nIOUTrustLines == 0 ||
+                nIOUTrustLines > 2)
+                return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
+            return true;
+        }
+        currentIndex = keylet::page(root, uNodeNext);
+    }
+    return Unexpected<TER>(tecINTERNAL);  // LCOV_EXCL_LINE
+}
+
 }  // namespace ripple

src/ripple/app/paths/impl/BookStep.cpp

@@ -532,7 +532,7 @@ class BookOfferCrossingStep
         // Single path AMM offer has to factor in the transfer in rate
         // when calculating the upper bound quality and the quality function
         // because single path AMM's offer quality is not constant.
-        if (!rules.enabled(fixAMMRounding))
+        if (!rules.enabled(fixAMMv1_1))
             return ofrQ;
         else if (
             offerType == OfferType::CLOB ||

src/ripple/app/tx/impl/AMMWithdraw.cpp

@@ -310,6 +310,31 @@ AMMWithdraw::applyGuts(Sandbox& sb)
     auto const lpTokensWithdraw =
         tokensWithdraw(lpTokens, ctx_.tx[~sfLPTokenIn], ctx_.tx.getFlags());
 
+    // Due to rounding, the LPTokenBalance of the last LP
+    // might not match the LP's trustline balance
+    if (sb.rules().enabled(fixAMMv1_1))
+    {
+        if (auto const res =
+                isOnlyLiquidityProvider(sb, lpTokens.issue(), account_);
+            !res)
+            return {res.error(), false};
+        else if (res.value())
+        {
+            if (withinRelativeDistance(
+                    lpTokens,
+                    ammSle->getFieldAmount(sfLPTokenBalance),
+                    Number{1, -3}))
+            {
+                ammSle->setFieldAmount(sfLPTokenBalance, lpTokens);
+                sb.update(ammSle);
+            }
+            else
+            {
+                return {tecAMM_INVALID_TOKENS, false};
+            }
+        }
+    }
+
     auto const tfee = getTradingFee(ctx_.view(), *ammSle, account_);
 
     auto const expected = ammHolds(
@@ -467,12 +492,24 @@ AMMWithdraw::withdraw(
         lpTokensWithdrawActual > lpTokens)
     {
         JLOG(ctx_.journal.debug())
-            << "AMM Withdraw: failed to withdraw, invalid LP tokens "
-            << " tokens: " << lpTokensWithdrawActual << " " << lpTokens << " "
+            << "AMM Withdraw: failed to withdraw, invalid LP tokens: "
+            << lpTokensWithdrawActual << " " << lpTokens << " "
             << lpTokensAMMBalance;
         return {tecAMM_INVALID_TOKENS, STAmount{}};
     }
 
+    // Should not happen since the only LP on last withdraw
+    // has the balance set to the lp token trustline balance.
+    if (view.rules().enabled(fixAMMv1_1) &&
+        lpTokensWithdrawActual > lpTokensAMMBalance)
+    {
+        JLOG(ctx_.journal.debug())
+            << "AMM Withdraw: failed to withdraw, unexpected LP tokens: "
+            << lpTokensWithdrawActual << " " << lpTokens << " "
+            << lpTokensAMMBalance;
+        return {tecINTERNAL, STAmount{}};
+    }
+
     // Withdrawing one side of the pool
     if ((amountWithdrawActual == curBalance &&
          amount2WithdrawActual != curBalance2) ||

src/ripple/protocol/impl/Feature.cpp

@@ -466,7 +466,7 @@ REGISTER_FEATURE(PriceOracle,                   Supported::yes, VoteBehavior::De
 REGISTER_FIX    (fixEmptyDID,                   Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX    (fixXChainRewardRounding,       Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX    (fixPreviousTxnID,              Supported::yes, VoteBehavior::DefaultNo);
-REGISTER_FIX    (fixAMMv1_1,                Supported::yes, VoteBehavior::DefaultNo);
+REGISTER_FIX    (fixAMMv1_1,                    Supported::yes, VoteBehavior::DefaultNo);
 
 // The following amendments are obsolete, but must remain supported
 // because they could potentially get enabled.

src/test/app/AMM_test.cpp

@@ -17,6 +17,7 @@
 */
 //==============================================================================
 #include <ripple/app/misc/AMMHelpers.h>
+#include <ripple/app/misc/AMMUtils.h>
 #include <ripple/app/paths/AMMContext.h>
 #include <ripple/app/paths/AMMOffer.h>
 #include <ripple/app/tx/impl/AMMBid.h>
@@ -3814,7 +3815,7 @@ struct AMM_test : public jtx::AMMTest
                 env.close();
                 env(offer(carol, XRP(100), USD(55)));
                 env.close();
-                if (!features[fixAMMRounding])
+                if (!features[fixAMMv1_1])
                 {
                     // Pre-amendment the transfer fee is not taken into
                     // account when calculating the limit out based on
@@ -3865,7 +3866,7 @@ struct AMM_test : public jtx::AMMTest
                 env.close();
                 env(offer(carol, XRP(10), USD(5.5)));
                 env.close();
-                if (!features[fixAMMRounding])
+                if (!features[fixAMMv1_1])
                 {
                     BEAST_EXPECT(amm.expectBalances(
                         XRP(990),
@@ -3910,7 +3911,7 @@ struct AMM_test : public jtx::AMMTest
                 env.close();
                 env(offer(carol, EUR(100), GBP(100)));
                 env.close();
-                if (!features[fixAMMRounding])
+                if (!features[fixAMMv1_1])
                 {
                     // After the auto-bridge offers are consumed, single path
                     // AMM offer is generated with the limit out not taking
@@ -6737,6 +6738,124 @@ struct AMM_test : public jtx::AMMTest
         }
     }
 
+    void
+    testLPTokenBalance(FeatureBitset features)
+    {
+        using namespace jtx;
+
+        // Last Liquidity Provider is the issuer of one token
+        {
+            Env env(*this, features);
+            fund(
+                env,
+                gw,
+                {alice, carol},
+                XRP(1'000'000'000),
+                {USD(1'000'000'000)});
+            AMM amm(env, gw, XRP(2), USD(1));
+            amm.deposit(alice, IOUAmount{1'876123487565916, -15});
+            amm.deposit(carol, IOUAmount{1'000'000});
+            amm.withdrawAll(alice);
+            amm.withdrawAll(carol);
+            auto const lpToken = getAccountLines(
+                env, gw, amm.lptIssue())[jss::lines][0u][jss::balance];
+            auto const lpTokenBalance =
+                amm.ammRpcInfo()[jss::amm][jss::lp_token][jss::value];
+            BEAST_EXPECT(
+                lpToken == "1414.213562373095" &&
+                lpTokenBalance == "1414.213562373");
+            if (!features[fixAMMv1_1])
+            {
+                amm.withdrawAll(gw, std::nullopt, ter(tecAMM_BALANCE));
+                BEAST_EXPECT(amm.ammExists());
+            }
+            else
+            {
+                amm.withdrawAll(gw);
+                BEAST_EXPECT(!amm.ammExists());
+            }
+        }
+
+        // Last Liquidity Provider is the issuer of two tokens, or not
+        // the issuer
+        for (auto const& lp : {gw, bob})
+        {
+            Env env(*this, features);
+            auto const ABC = gw["ABC"];
+            fund(
+                env,
+                gw,
+                {alice, carol, bob},
+                XRP(1'000),
+                {USD(1'000'000'000), ABC(1'000'000'000'000)});
+            AMM amm(env, lp, ABC(2'000'000), USD(1));
+            amm.deposit(alice, IOUAmount{1'876123487565916, -15});
+            amm.deposit(carol, IOUAmount{1'000'000});
+            amm.withdrawAll(alice);
+            amm.withdrawAll(carol);
+            auto const lpToken = getAccountLines(
+                env, lp, amm.lptIssue())[jss::lines][0u][jss::balance];
+            auto const lpTokenBalance =
+                amm.ammRpcInfo()[jss::amm][jss::lp_token][jss::value];
+            BEAST_EXPECT(
+                lpToken == "1414.213562373095" &&
+                lpTokenBalance == "1414.213562373");
+            if (!features[fixAMMv1_1])
+            {
+                amm.withdrawAll(lp, std::nullopt, ter(tecAMM_BALANCE));
+                BEAST_EXPECT(amm.ammExists());
+            }
+            else
+            {
+                amm.withdrawAll(lp);
+                BEAST_EXPECT(!amm.ammExists());
+            }
+        }
+
+        // More than one Liquidity Provider
+        // XRP/IOU
+        {
+            Env env(*this, features);
+            fund(env, gw, {alice}, XRP(1'000), {USD(1'000)});
+            AMM amm(env, gw, XRP(10), USD(10));
+            amm.deposit(alice, 1'000);
+            auto res =
+                isOnlyLiquidityProvider(*env.current(), amm.lptIssue(), gw);
+            BEAST_EXPECT(res && !res.value());
+            res =
+                isOnlyLiquidityProvider(*env.current(), amm.lptIssue(), alice);
+            BEAST_EXPECT(res && !res.value());
+        }
+        // IOU/IOU, issuer of both IOU
+        {
+            Env env(*this, features);
+            fund(env, gw, {alice}, XRP(1'000), {USD(1'000), EUR(1'000)});
+            AMM amm(env, gw, EUR(10), USD(10));
+            amm.deposit(alice, 1'000);
+            auto res =
+                isOnlyLiquidityProvider(*env.current(), amm.lptIssue(), gw);
+            BEAST_EXPECT(res && !res.value());
+            res =
+                isOnlyLiquidityProvider(*env.current(), amm.lptIssue(), alice);
+            BEAST_EXPECT(res && !res.value());
+        }
+        // IOU/IOU, issuer of one IOU
+        {
+            Env env(*this, features);
+            Account const gw1("gw1");
+            auto const YAN = gw1["YAN"];
+            fund(env, gw, {gw1}, XRP(1'000), {USD(1'000)});
+            fund(env, gw1, {gw}, XRP(1'000), {YAN(1'000)}, Fund::IOUOnly);
+            AMM amm(env, gw1, YAN(10), USD(10));
+            amm.deposit(gw, 1'000);
+            auto res =
+                isOnlyLiquidityProvider(*env.current(), amm.lptIssue(), gw);
+            BEAST_EXPECT(res && !res.value());
+            res = isOnlyLiquidityProvider(*env.current(), amm.lptIssue(), gw1);
+            BEAST_EXPECT(res && !res.value());
+        }
+    }
+
     void
     run() override
     {
@@ -6779,6 +6898,8 @@ struct AMM_test : public jtx::AMMTest
         testFixChangeSpotPriceQuality(all - fixAMMv1_1);
         testFixAMMOfferBlockedByLOB(all);
         testFixAMMOfferBlockedByLOB(all - fixAMMv1_1);
+        testLPTokenBalance(all);
+        testLPTokenBalance(all - fixAMMv1_1);
     }
 };
 

src/test/jtx/impl/AMMTest.cpp

@@ -137,10 +137,15 @@ AMMTestBase::testAMM(
         else if (asset2.native())
             fund(env, gw, {alice, carol}, toFund2, {toFund1}, Fund::All);
 
-        AMM ammAlice(env, alice, asset1, asset2, false, tfee);
-        BEAST_EXPECT(
-            ammAlice.expectBalances(asset1, asset2, ammAlice.tokens()));
-        cb(ammAlice, env);
+        AMM ammAlice(
+            env,
+            alice,
+            asset1,
+            asset2,
+            CreateArg{.log = false, .tfee = tfee, .err = ter});
+        if (BEAST_EXPECT(
+                ammAlice.expectBalances(asset1, asset2, ammAlice.tokens())))
+            cb(ammAlice, env);
     }
 }