From 37f16a75d10b67bece3cb86afbb5902c830f27a6 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Sun, 5 May 2024 07:23:26 -0400 Subject: [PATCH] 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 | 8 ++++ src/ripple/app/misc/impl/AMMUtils.cpp | 52 ++++++++++++++++++++++++++ src/ripple/app/tx/impl/AMMWithdraw.cpp | 27 ++++++++++++- src/test/app/AMM_test.cpp | 33 ++++++++++++++++ 4 files changed, 118 insertions(+), 2 deletions(-) diff --git a/src/ripple/app/misc/AMMUtils.h b/src/ripple/app/misc/AMMUtils.h index c25503ceb9c..a77b3061bb8 100644 --- a/src/ripple/app/misc/AMMUtils.h +++ b/src/ripple/app/misc/AMMUtils.h @@ -113,6 +113,14 @@ initializeFeeAuctionVote( Issue const& lptIssue, std::uint16_t tfee); +/** Return true if the liquidity provider is the only AMM provider. + */ +bool +isOnlyLiquidityProvider( + ReadView const& view, + AccountID const& ammAccount, + AccountID const& lpAccount); + } // namespace ripple #endif // RIPPLE_APP_MISC_AMMUTILS_H_INLCUDED diff --git a/src/ripple/app/misc/impl/AMMUtils.cpp b/src/ripple/app/misc/impl/AMMUtils.cpp index cf4da2c5d85..908ca6232db 100644 --- a/src/ripple/app/misc/impl/AMMUtils.cpp +++ b/src/ripple/app/misc/impl/AMMUtils.cpp @@ -348,4 +348,56 @@ initializeFeeAuctionVote( auctionSlot.makeFieldAbsent(sfDiscountedFee); // LCOV_EXCL_LINE } +bool +isOnlyLiquidityProvider( + ReadView const& view, + AccountID const& ammAccount, + AccountID const& lpAccount) +{ + std::uint8_t nTrustLines = 0; + std::uint8_t limit = 10; + auto const root = keylet::ownerDir(ammAccount); + auto currentIndex = root; + + // Iterate over AMM owner directory objects. + // AMM Liquidity Provider has at most three + // trustlines. One for LPToken, and at most + // two trustlines for IOU: one if XRP/IOU + // AMM and two if IOU/IOU AMM. + while (limit-- >= 1) + { + auto const ownerDir = view.read(currentIndex); + if (!ownerDir) + return false; + for (auto const& key : ownerDir->getFieldV256(sfIndexes)) + { + auto const sle = view.read(keylet::child(key)); + if (!sle) + { + assert(false); + return false; + } + // Only one AMM object + if (sle->getFieldU16(sfLedgerEntryType) == ltAMM) + continue; + if (sle->getFieldU16(sfLedgerEntryType) != ltRIPPLE_STATE) + { + assert(false); + return false; + } + auto const lowLimit = sle->getFieldAmount(sfLowLimit); + auto const highLimit = sle->getFieldAmount(sfHighLimit); + if ((lowLimit.getIssuer() != lpAccount && + highLimit.getIssuer() != lpAccount)) + return false; + ++nTrustLines; + } + auto const uNodeNext = ownerDir->getFieldU64(sfIndexNext); + if (uNodeNext == 0) + return nTrustLines == 2 || nTrustLines == 3; + currentIndex = keylet::page(root, uNodeNext); + } + return nTrustLines == 2 || nTrustLines == 3; +} + } // namespace ripple diff --git a/src/ripple/app/tx/impl/AMMWithdraw.cpp b/src/ripple/app/tx/impl/AMMWithdraw.cpp index 839c3b433aa..860c1b9fbcb 100644 --- a/src/ripple/app/tx/impl/AMMWithdraw.cpp +++ b/src/ripple/app/tx/impl/AMMWithdraw.cpp @@ -310,6 +310,17 @@ 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(fixAMMRounding) && + isOnlyLiquidityProvider(sb, ammAccountID, account_) && + withinRelativeDistance( + lpTokens, ammSle->getFieldAmount(sfLPTokenBalance), Number{1, -7})) + { + ammSle->setFieldAmount(sfLPTokenBalance, lpTokens); + sb.update(ammSle); + } + auto const tfee = getTradingFee(ctx_.view(), *ammSle, account_); auto const expected = ammHolds( @@ -467,12 +478,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(fixAMMRounding) && + 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) || diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 5d8cff4e299..46ac6ca7291 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -6299,6 +6299,37 @@ struct AMM_test : public jtx::AMMTest {jtx::supported_amendments() | fixAMMRounding}); } + void + testLPTokenBalance(FeatureBitset features) + { + using namespace jtx; + 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[fixAMMRounding]) + { + amm.withdrawAll(gw, std::nullopt, ter(tecAMM_BALANCE)); + BEAST_EXPECT(amm.ammExists()); + } + else + { + amm.withdrawAll(gw); + BEAST_EXPECT(!amm.ammExists()); + } + } + void run() override { @@ -6337,6 +6368,8 @@ struct AMM_test : public jtx::AMMTest testFixOverflowOffer(all); testFixOverflowOffer(all - fixAMMRounding); testSwapRounding(); + testLPTokenBalance(all); + testLPTokenBalance(all - fixAMMRounding); } };