ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-04 11:15:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix last Liquidity Provider withdrawal:

Browse Source 
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.
This commit is contained in:
Gregory Tsipenyuk
2024-05-05 07:23:26 -04:00
parent 7f6a079aa4
commit 15390bedd5
8 changed files with 267 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
src/ripple/app/misc/AMMUtils.h
View File

@@ -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

2
src/ripple/app/misc/impl/AMMHelpers.cpp
View File

@@ -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;
}();

82
src/ripple/app/misc/impl/AMMUtils.cpp
View File

@@ -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

2
src/ripple/app/paths/impl/BookStep.cpp
View File

@@ -532,7 +532,7 @@ public:
// 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 ||

41
src/ripple/app/tx/impl/AMMWithdraw.cpp
View File

@@ -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) ||

2
src/ripple/protocol/impl/Feature.cpp
View File

@@ -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.

127
src/test/app/AMM_test.cpp
View File

@@ -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 @@ private:
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 @@ private:
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 @@ private:
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 @@ private:
}
}
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 @@ private:
testFixChangeSpotPriceQuality(all - fixAMMv1_1);
testFixAMMOfferBlockedByLOB(all);
testFixAMMOfferBlockedByLOB(all - fixAMMv1_1);
testLPTokenBalance(all);
testLPTokenBalance(all - fixAMMv1_1);
}
};

13
src/test/jtx/impl/AMMTest.cpp
View File

@@ -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);
}
}