Skip to content

Commit

Permalink
Fix last Liquidity Provider withdrawal:
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
gregtatcam committed May 10, 2024
1 parent f45364f commit 37f16a7
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 2 deletions.
8 changes: 8 additions & 0 deletions src/ripple/app/misc/AMMUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
52 changes: 52 additions & 0 deletions src/ripple/app/misc/impl/AMMUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
27 changes: 25 additions & 2 deletions src/ripple/app/tx/impl/AMMWithdraw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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) ||
Expand Down
33 changes: 33 additions & 0 deletions src/test/app/AMM_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -6337,6 +6368,8 @@ struct AMM_test : public jtx::AMMTest
testFixOverflowOffer(all);
testFixOverflowOffer(all - fixAMMRounding);
testSwapRounding();
testLPTokenBalance(all);
testLPTokenBalance(all - fixAMMRounding);
}
};

Expand Down

0 comments on commit 37f16a7

Please sign in to comment.