Skip to content

Commit

Permalink
Add more adjustment when amount increases while tokens decrease
Browse files Browse the repository at this point in the history
  • Loading branch information
gregtatcam committed Nov 12, 2024
1 parent f7735ea commit ec4a908
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 64 deletions.
14 changes: 7 additions & 7 deletions src/test/app/AMM_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7392,12 +7392,12 @@ struct AMM_test : public jtx::AMMTest
// both LPTokens and asset-amounts are rounded down in AMMHelpers
// .cpp:adjustAmountsByLPTokens. The AMM Invariant is violated by ~1e-6
for (STAmount const& depositEuro :
{/*STAmount(EUR, 1, 1),
{STAmount(EUR, 1, 1),
STAmount(EUR, 1, 2),
STAmount(EUR, 1, 5),*/
STAmount(EUR, 1, 5),
STAmount(EUR, 1, -3),
/*STAmount(EUR, 1, -6),
STAmount(EUR, 1, -9)*/})
STAmount(EUR, 1, -6),
STAmount(EUR, 1, -9)})
{
testAMM(
[&](AMM& ammAlice, Env& env) {
Expand All @@ -7415,7 +7415,7 @@ struct AMM_test : public jtx::AMMTest
},
{{GBP(30'000), EUR(30'000)}});
}
return;

// two-asset proportional deposit (1:1 pool ratio)
testAMM(
[&](AMM& ammAlice, Env& env) {
Expand Down Expand Up @@ -7684,8 +7684,8 @@ struct AMM_test : public jtx::AMMTest
all - fixAMMv1_1 - fixAMMv1_3 - featureAMMClawback);
testDepositAndWithdrawRoundingV2(all);
testDepositAndWithdrawRoundingV2(all - fixAMMv1_3);
// testDepositRounding();
// testWithdrawRounding();
testDepositRounding();
testWithdrawRounding();
}
};

Expand Down
4 changes: 2 additions & 2 deletions src/xrpld/app/misc/AMMHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -710,10 +710,10 @@ getRoundedLPTokens(
std::function<Number()>&& noRoundCb,
STAmount const& lptAMMBalance,
std::function<Number()>&& productCb,
bool product,
bool productOnly,
bool isDeposit);

STAmount
std::pair<STAmount, STAmount>
adjustAssetInByTokens(
Rules const& rules,
STAmount const& balance,
Expand Down
42 changes: 39 additions & 3 deletions src/xrpld/app/misc/detail/AMMHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ getRoundedLPTokens(
return adjustLPTokens(lptAMMBalance, tokens, isDeposit);
}

STAmount
std::pair<STAmount, STAmount>
adjustAssetInByTokens(
Rules const& rules,
STAmount const& balance,
Expand All @@ -357,8 +357,26 @@ adjustAssetInByTokens(
std::uint16_t tfee)
{
if (!rules.enabled(fixAMMv1_3))
return amount;
return std::min(amount, ammAssetIn(balance, lptAMMBalance, tokens, tfee));
return {tokens, amount};
auto adjAsset = ammAssetIn(balance, lptAMMBalance, tokens, tfee);
auto adjTokens = tokens;
// Rounding didn't work the right way.
// Try to adjust the original deposit amount by difference
// in adjust and original amount. Then adjust tokens and deposit amount.
if (adjAsset > amount)
{
auto const adjAmount = [&] {
NumberRoundModeGuard g(Number::getround());
Number::setround(Number::downward);
auto const diff = adjAsset - amount;
Number::setround(Number::upward);
return amount - diff;
}();
auto const t = lpTokensOut(balance, adjAmount, lptAMMBalance, tfee);
adjTokens = adjustLPTokens(lptAMMBalance, t, /*isDeposit*/ true);
adjAsset = ammAssetIn(balance, lptAMMBalance, tokens, tfee);
}
return {adjTokens, std::min(amount, adjAsset)};
}

STAmount
Expand All @@ -372,6 +390,24 @@ adjustAssetOutByTokens(
{
if (!rules.enabled(fixAMMv1_3))
return amount;
auto adjAsset = ammAssetOut(balance, lptAMMBalance, tokens, tfee);
auto adjTokens = tokens;
// Rounding didn't work the right way.
// Try to adjust the original deposit amount by difference
// in adjust and original amount. Then adjust tokens and deposit amount.
if (adjAsset > amount)
{
auto const adjAmount = [&] {
NumberRoundModeGuard g(Number::getround());
Number::setround(Number::upward);
auto const diff = adjAsset - amount;
Number::setround(Number::downward);
return amount - diff;
}();
auto const t = lpTokensIn(balance, adjAmount, lptAMMBalance, tfee);
adjTokens = adjustLPTokens(lptAMMBalance, t, /*isDeposit*/ false);
adjAsset = ammAssetOut(balance, lptAMMBalance, tokens, tfee);
}
return std::min(amount, ammAssetOut(balance, lptAMMBalance, tokens, tfee));
}

Expand Down
46 changes: 29 additions & 17 deletions src/xrpld/app/tx/detail/AMMDeposit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -658,10 +658,10 @@ AMMDeposit::equalDepositTokens(
adjustLPTokensOut(view.rules(), lptAMMBalance, lpTokensDeposit);
auto const frac = divide(tokens, lptAMMBalance, lptAMMBalance.issue());
// amounts factor in the adjusted tokens
auto const amountDeposit =
getRoundedAsset(view.rules(), amountBalance, frac, true);
auto const amount2Deposit =
getRoundedAsset(view.rules(), amount2Balance, frac, true);
auto const amountDeposit = getRoundedAsset(
view.rules(), amountBalance, frac, /*isDeposit*/ true);
auto const amount2Deposit = getRoundedAsset(
view.rules(), amount2Balance, frac, /*isDeposit*/ true);
return deposit(
view,
ammAccount,
Expand Down Expand Up @@ -726,7 +726,8 @@ AMMDeposit::equalDepositLimit(
std::uint16_t tfee)
{
auto frac = Number{amount} / amountBalance;
auto tokens = getRoundedLPTokens(view.rules(), lptAMMBalance, frac, true);
auto tokens = getRoundedLPTokens(
view.rules(), lptAMMBalance, frac, /*isDeposit*/ true);
if (tokens == beast::zero)
{
if (!view.rules().enabled(fixAMMv1_3))
Expand All @@ -737,7 +738,7 @@ AMMDeposit::equalDepositLimit(
// factor in the adjusted tokens
frac = adjustFracByTokens(view.rules(), lptAMMBalance, tokens, frac);
auto const amount2Deposit =
getRoundedAsset(view.rules(), amount2Balance, frac, true);
getRoundedAsset(view.rules(), amount2Balance, frac, /*isDeposit*/ true);
if (amount2Deposit <= amount2)
return deposit(
view,
Expand All @@ -752,7 +753,8 @@ AMMDeposit::equalDepositLimit(
lpTokensDepositMin,
tfee);
frac = Number{amount2} / amount2Balance;
tokens = getRoundedLPTokens(view.rules(), lptAMMBalance, frac, true);
tokens = getRoundedLPTokens(
view.rules(), lptAMMBalance, frac, /*isDeposit*/ true);
if (tokens == beast::zero)
{
if (!view.rules().enabled(fixAMMv1_3))
Expand All @@ -763,7 +765,7 @@ AMMDeposit::equalDepositLimit(
// factor in the adjusted tokens
frac = adjustFracByTokens(view.rules(), lptAMMBalance, tokens, frac);
auto const amountDeposit =
getRoundedAsset(view.rules(), amountBalance, frac, true);
getRoundedAsset(view.rules(), amountBalance, frac, /*isDeposit*/ true);
if (amountDeposit <= amount)
return deposit(
view,
Expand Down Expand Up @@ -810,7 +812,7 @@ AMMDeposit::singleDeposit(
return {tecAMM_INVALID_TOKENS, STAmount{}};
}
// factor in the adjusted tokens
auto const amountDeposit = adjustAssetInByTokens(
auto const [tokensActual, amountDeposit] = adjustAssetInByTokens(
view.rules(), amountBalance, amount, lptAMMBalance, tokens, tfee);
return deposit(
view,
Expand All @@ -819,7 +821,7 @@ AMMDeposit::singleDeposit(
amountDeposit,
std::nullopt,
lptAMMBalance,
tokens,
tokensActual,
std::nullopt,
std::nullopt,
lpTokensDepositMin,
Expand Down Expand Up @@ -913,9 +915,9 @@ AMMDeposit::singleDepositEPrice(
return {tecAMM_INVALID_TOKENS, STAmount{}};
}
// factor in the adjusted tokens
auto const amountDeposit = adjustAssetInByTokens(
auto const [tokensActual, amountDeposit] = adjustAssetInByTokens(
view.rules(), amountBalance, amount, lptAMMBalance, tokens, tfee);
auto const ep = Number{amountDeposit} / tokens;
auto const ep = Number{amountDeposit} / tokensActual;
if (ep <= ePrice)
return deposit(
view,
Expand All @@ -924,7 +926,7 @@ AMMDeposit::singleDepositEPrice(
amountDeposit,
std::nullopt,
lptAMMBalance,
tokens,
tokensActual,
std::nullopt,
std::nullopt,
std::nullopt,
Expand Down Expand Up @@ -960,15 +962,25 @@ AMMDeposit::singleDepositEPrice(
};
auto amtProdCb = [&] { return f1 * solveQuadraticEq(a1, b1, c1); };
auto const amountDeposit = getRoundedAsset(
view.rules(), amtNoRoundCb, amountBalance, amtProdCb, false, true);
view.rules(),
amtNoRoundCb,
amountBalance,
amtProdCb,
/*productOnly*/ false,
/*isDeposit*/ true);
if (amountDeposit <= beast::zero)
return {tecAMM_FAILED, STAmount{}};
auto tokNoRoundCb = [&] { return amountDeposit / ePrice; };
auto tokProdCb = [&] { return amountDeposit / ePrice; };
auto const tokens = getRoundedLPTokens(
view.rules(), tokNoRoundCb, lptAMMBalance, tokProdCb, true, true);
view.rules(),
tokNoRoundCb,
lptAMMBalance,
tokProdCb,
/*productOnly*/ true,
/*isDeposit*/ true);
// factor in the adjusted tokens
auto const amountDepositActual = adjustAssetInByTokens(
auto const [tokensActual, amountDepositActual] = adjustAssetInByTokens(
view.rules(),
amountBalance,
amountDeposit,
Expand All @@ -983,7 +995,7 @@ AMMDeposit::singleDepositEPrice(
amountDepositActual,
std::nullopt,
lptAMMBalance,
tokens,
tokensActual,
std::nullopt,
std::nullopt,
std::nullopt,
Expand Down
41 changes: 27 additions & 14 deletions src/xrpld/app/tx/detail/AMMWithdraw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -766,10 +766,10 @@ AMMWithdraw::equalWithdrawTokens(
view.rules(), lptAMMBalance, lpTokensWithdraw, withdrawAll);
// the adjusted tokens are factored in
auto const frac = divide(tokens, lptAMMBalance, noIssue());
auto const withdrawAmount =
getRoundedAsset(view.rules(), amountBalance, frac, false);
auto const withdraw2Amount =
getRoundedAsset(view.rules(), amount2Balance, frac, false);
auto const withdrawAmount = getRoundedAsset(
view.rules(), amountBalance, frac, /*isDeposit*/ false);
auto const withdraw2Amount = getRoundedAsset(
view.rules(), amount2Balance, frac, /*isDeposit*/ false);
// LP is making equal withdrawal by tokens but the requested amount
// of LP tokens is likely too small and results in one-sided pool
// withdrawal due to round off. Fail so the user withdraws
Expand Down Expand Up @@ -840,13 +840,14 @@ AMMWithdraw::equalWithdrawLimit(
std::uint16_t tfee)
{
auto frac = Number{amount} / amountBalance;
auto amount2Withdraw =
getRoundedAsset(view.rules(), amount2Balance, frac, false);
auto tokens = getRoundedLPTokens(view.rules(), lptAMMBalance, frac, false);
auto amount2Withdraw = getRoundedAsset(
view.rules(), amount2Balance, frac, /*isDeposit*/ false);
auto tokens = getRoundedLPTokens(
view.rules(), lptAMMBalance, frac, /*isDeposit*/ false);
// factor in the adjusted tokens
frac = adjustFracByTokens(view.rules(), lptAMMBalance, tokens, frac);
amount2Withdraw =
getRoundedAsset(view.rules(), amount2Balance, frac, false);
amount2Withdraw = getRoundedAsset(
view.rules(), amount2Balance, frac, /*isDeposit*/ false);
if (amount2Withdraw <= amount2)
{
return withdraw(
Expand All @@ -863,11 +864,13 @@ AMMWithdraw::equalWithdrawLimit(

frac = Number{amount2} / amount2Balance;
auto amountWithdraw =
getRoundedAsset(view.rules(), amountBalance, frac, false);
tokens = getRoundedLPTokens(view.rules(), lptAMMBalance, frac, false);
getRoundedAsset(view.rules(), amountBalance, frac, /*isDeposit*/ false);
tokens = getRoundedLPTokens(
view.rules(), lptAMMBalance, frac, /*isDeposit*/ false);
// factor in the adjusted tokens
frac = adjustFracByTokens(view.rules(), lptAMMBalance, tokens, frac);
amountWithdraw = getRoundedAsset(view.rules(), amountBalance, frac, false);
amountWithdraw =
getRoundedAsset(view.rules(), amountBalance, frac, /*isDeposit*/ false);
if (!view.rules().enabled(fixAMMv1_3))
assert(amountWithdraw <= amount);
else if (amountWithdraw > amount)
Expand Down Expand Up @@ -1019,7 +1022,12 @@ AMMWithdraw::singleWithdrawEPrice(
return (lptAMMBalance + ae * (f - 2)) / (lptAMMBalance * f - ae);
};
auto const tokens = getRoundedLPTokens(
view.rules(), tokNoRoundCb, lptAMMBalance, tokProdCb, false, false);
view.rules(),
tokNoRoundCb,
lptAMMBalance,
tokProdCb,
/*productOnly*/ false,
/*isDeposit*/ false);
if (tokens <= beast::zero)
{
if (!view.rules().enabled(fixAMMv1_3))
Expand All @@ -1031,7 +1039,12 @@ AMMWithdraw::singleWithdrawEPrice(
auto amtProdCb = [&] { return tokens / ePrice; };
// the adjusted tokens are factored in
auto const amountWithdraw = getRoundedAsset(
view.rules(), amtNoRoundCb, amount, amtProdCb, true, false);
view.rules(),
amtNoRoundCb,
amount,
amtProdCb,
/*productOnly*/ true,
/*isDeposit*/ false);
if (amount == beast::zero || amountWithdraw >= amount)
{
return withdraw(
Expand Down
Loading

0 comments on commit ec4a908

Please sign in to comment.