fixAMMClawbackRounding: adjust last holder's LPToken balance (#5513)

Due to rounding, the LPTokenBalance of the last LP might not match the LP's trustline balance. This was fixed for `AMMWithdraw` in `fixAMMv1_1` by adjusting the LPTokenBalance to be the same as the trustline balance. Since `AMMClawback` is also performing a withdrawal, we need to adjust LPTokenBalance as well in `AMMClawback.`

This change includes:
1. Refactored `verifyAndAdjustLPTokenBalance` function in `AMMUtils`, which both`AMMWithdraw` and `AMMClawback` call to adjust LPTokenBalance.
2. Added the unit test `testLastHolderLPTokenBalance` to test the scenario.
3. Modify the existing unit tests for `fixAMMClawbackRounding`.
This commit is contained in:
yinyiqian1
2025-07-11 16:03:28 -04:00
committed by GitHub
parent 258ba71363
commit 8aa94ea09a
6 changed files with 693 additions and 205 deletions

View File

@@ -35,6 +35,7 @@
// If you add an amendment here, then do not forget to increment `numFeatures`
// in include/xrpl/protocol/Feature.h.
XRPL_FIX (AMMClawbackRounding, Supported::no, VoteBehavior::DefaultNo)
XRPL_FEATURE(TokenEscrow, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (EnforceNFTokenTrustlineV2, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (AMMv1_3, Supported::yes, VoteBehavior::DefaultNo)

File diff suppressed because it is too large Load Diff

View File

@@ -125,6 +125,17 @@ isOnlyLiquidityProvider(
Issue const& ammIssue,
AccountID const& lpAccount);
/** Due to rounding, the LPTokenBalance of the last LP might
* not match the LP's trustline balance. If it's within the tolerance,
* update LPTokenBalance to match the LP's trustline balance.
*/
Expected<bool, TER>
verifyAndAdjustLPTokenBalance(
Sandbox& sb,
STAmount const& lpTokens,
std::shared_ptr<SLE>& ammSle,
AccountID const& account);
} // namespace ripple
#endif // RIPPLE_APP_MISC_AMMUTILS_H_INCLUDED

View File

@@ -17,6 +17,7 @@
*/
//==============================================================================
#include <xrpld/app/misc/AMMHelpers.h>
#include <xrpld/app/misc/AMMUtils.h>
#include <xrpld/ledger/Sandbox.h>
@@ -464,4 +465,32 @@ isOnlyLiquidityProvider(
return Unexpected<TER>(tecINTERNAL); // LCOV_EXCL_LINE
}
Expected<bool, TER>
verifyAndAdjustLPTokenBalance(
Sandbox& sb,
STAmount const& lpTokens,
std::shared_ptr<SLE>& ammSle,
AccountID const& account)
{
if (auto const res = isOnlyLiquidityProvider(sb, lpTokens.issue(), account);
!res)
return Unexpected<TER>(res.error());
else if (res.value())
{
if (withinRelativeDistance(
lpTokens,
ammSle->getFieldAmount(sfLPTokenBalance),
Number{1, -3}))
{
ammSle->setFieldAmount(sfLPTokenBalance, lpTokens);
sb.update(ammSle);
}
else
{
return Unexpected<TER>(tecAMM_INVALID_TOKENS);
}
}
return true;
}
} // namespace ripple

View File

@@ -151,6 +151,20 @@ AMMClawback::applyGuts(Sandbox& sb)
if (!accountSle)
return tecINTERNAL; // LCOV_EXCL_LINE
if (sb.rules().enabled(fixAMMClawbackRounding))
{
// retrieve LP token balance inside the amendment gate to avoid
// inconsistent error behavior
auto const lpTokenBalance = ammLPHolds(sb, *ammSle, holder, j_);
if (lpTokenBalance == beast::zero)
return tecAMM_BALANCE;
if (auto const res = verifyAndAdjustLPTokenBalance(
sb, lpTokenBalance, ammSle, holder);
!res)
return res.error(); // LCOV_EXCL_LINE
}
auto const expected = ammHolds(
sb,
*ammSle,
@@ -248,10 +262,11 @@ AMMClawback::equalWithdrawMatchingOneAmount(
STAmount const& amount)
{
auto frac = Number{amount} / amountBalance;
auto const amount2Withdraw = amount2Balance * frac;
auto amount2Withdraw = amount2Balance * frac;
auto const lpTokensWithdraw =
toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac);
if (lpTokensWithdraw > holdLPtokens)
// if lptoken balance less than what the issuer intended to clawback,
// clawback all the tokens. Because we are doing a two-asset withdrawal,
@@ -272,6 +287,42 @@ AMMClawback::equalWithdrawMatchingOneAmount(
mPriorBalance,
ctx_.journal);
auto const& rules = sb.rules();
if (rules.enabled(fixAMMClawbackRounding))
{
auto tokensAdj =
getRoundedLPTokens(rules, lptAMMBalance, frac, IsDeposit::No);
// LCOV_EXCL_START
if (tokensAdj == beast::zero)
return {
tecAMM_INVALID_TOKENS, STAmount{}, STAmount{}, std::nullopt};
// LCOV_EXCL_STOP
frac = adjustFracByTokens(rules, lptAMMBalance, tokensAdj, frac);
auto amount2Rounded =
getRoundedAsset(rules, amount2Balance, frac, IsDeposit::No);
auto amountRounded =
getRoundedAsset(rules, amountBalance, frac, IsDeposit::No);
return AMMWithdraw::withdraw(
sb,
ammSle,
ammAccount,
holder,
amountBalance,
amountRounded,
amount2Rounded,
lptAMMBalance,
tokensAdj,
0,
FreezeHandling::fhIGNORE_FREEZE,
WithdrawAll::No,
mPriorBalance,
ctx_.journal);
}
// Because we are doing a two-asset withdrawal,
// tfee is actually not used, so pass tfee as 0.
return AMMWithdraw::withdraw(

View File

@@ -311,24 +311,9 @@ AMMWithdraw::applyGuts(Sandbox& sb)
if (sb.rules().enabled(fixAMMv1_1))
{
if (auto const res =
isOnlyLiquidityProvider(sb, lpTokens.issue(), account_);
verifyAndAdjustLPTokenBalance(sb, lpTokens, ammSle, 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_);