Skip to content

Commit

Permalink
Fix AMM blocking LOB
Browse files Browse the repository at this point in the history
limitOut() in StrandFlow adjusts remainingOut for single path
with AMM and limitQuality so that generated AMM offer matches
limitQuality. If there is LOB offer and this offer is
not considered when calculating quality function then
the following might happen. The quality function is calculated
based on max AMM offer and remainingOut is reduced. When
the strand is executed, AMM offer can't be generated to match
LOB quality and LOB offer is used instead of AMM offer. Because
remainingOut is reduced, possibly substantially, only
a fraction of LOB offer is consumed. The same process may
repeat for many iterations until either LOB offer is consumed
or max iterations is reached. Two fixes address this issue.
One is to factor in the trading fee into AMM spot quality.
This enables to make a decision that AMM offer can't
be genereated to match LOB offer quality. This is done when
LOB quality is compared to AMM spot quality. Another fix,
which is included in 2.2.0, is to consider LOB offer quality
into the quality function calculation.
Added unit-test verifies that the fixes resolve the issue.
  • Loading branch information
gregtatcam committed May 29, 2024
1 parent f2d37da commit 12f0273
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 3 deletions.
3 changes: 3 additions & 0 deletions src/ripple/app/paths/AMMLiquidity.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ class AMMLiquidity
*/
std::optional<AMMOffer<TIn, TOut>>
maxOffer(TAmounts<TIn, TOut> const& balances, Rules const& rules) const;

Quality
adjSpotQualityWithFees(TAmounts<TIn, TOut> const& balances) const;
};

} // namespace ripple
Expand Down
25 changes: 23 additions & 2 deletions src/ripple/app/paths/impl/AMMLiquidity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,17 @@ maxOut(T const& out, Issue const& iss)
}
} // namespace

template <typename TIn, typename TOut>
Quality
AMMLiquidity<TIn, TOut>::adjSpotQualityWithFees(
TAmounts<TIn, TOut> const& balances) const
{
auto const outWithFee =
toAmount<TOut>(issueOut_, balances.out * feeMult(tradingFee_));
return Quality{balances};
return Quality{TAmounts<TIn, TOut>{balances.in, outWithFee}};
}

template <typename TIn, typename TOut>
std::optional<AMMOffer<TIn, TOut>>
AMMLiquidity<TIn, TOut>::maxOffer(
Expand All @@ -135,11 +146,16 @@ AMMLiquidity<TIn, TOut>::maxOffer(
auto const out = maxOut<TOut>(balances.out, issueOut());
if (out <= TOut{0} || out >= balances.out)
return std::nullopt;
auto const spotPriceQ = [&]() {
if (!rules.enabled(fixAMMv1_1))
return Quality{balances};
return adjSpotQualityWithFees(balances);
}();
return AMMOffer<TIn, TOut>(
*this,
{swapAssetOut(balances, out, tradingFee_), out},
balances,
Quality{balances});
spotPriceQ);
}
}

Expand Down Expand Up @@ -176,7 +192,12 @@ AMMLiquidity<TIn, TOut>::getOffer(
// to the requested clobQuality but not exactly and potentially SPQ may keep
// on approaching clobQuality for many iterations. Checking for the quality
// threshold prevents this scenario.
if (auto const spotPriceQ = Quality{balances}; clobQuality &&
auto const spotPriceQ = [&]() {
if (!view.rules().enabled(fixAMMv1_1))
return Quality{balances};
return adjSpotQualityWithFees(balances);
}();
if (clobQuality &&
(spotPriceQ <= clobQuality ||
withinRelativeDistance(spotPriceQ, *clobQuality, Number(1, -7))))
{
Expand Down
103 changes: 103 additions & 0 deletions src/test/app/AMM_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6741,6 +6741,7 @@ struct AMM_test : public jtx::AMMTest
void
testLPTokenBalance(FeatureBitset features)
{
testcase("Fix LPToken Balance");
using namespace jtx;

// Last Liquidity Provider is the issuer of one token
Expand Down Expand Up @@ -6856,6 +6857,106 @@ struct AMM_test : public jtx::AMMTest
}
}

void
testFixAMMBlockingLOB(FeatureBitset features)
{
testcase("Fix AMM Offer Blocking LOB");
using namespace jtx;

{
Env env(*this, features);
auto const XPM = gw["XPM"];

fund(
env,
gw,
{alice, carol},
XRP(1'000'000'000),
{XPM(1'000'000'000)});

env(offer(alice, STAmount{XPM, UINT64_C(1), -9}, XRPAmount{1'000}));
env(offer(alice, STAmount{XPM, UINT64_C(1), -8}, XRPAmount{10}));
env(offer(alice, STAmount{XPM, UINT64_C(1), -5}, XRPAmount{1'000}));
env(offer(
alice,
STAmount{XPM, UINT64_C(5'00001605), -8},
XRPAmount{5'065'000}));
env(offer(
alice,
STAmount{XPM, UINT64_C(196'547002735), -9},
XRPAmount{196'527'350}));
env(offer(
alice,
STAmount{XPM, UINT64_C(1'05'314740196), -9},
XRPAmount{57'822'092}));
env(offer(
alice, STAmount{XPM, UINT64_C(100)}, XRPAmount{3'600'879}));
env(offer(
alice,
STAmount{XPM, UINT64_C(500'00555), -5},
XRPAmount{18'002'000}));
env(offer(
alice,
STAmount{XPM, UINT64_C(360'00288), -5},
XRPAmount{12'960'000}));
env(offer(
alice,
STAmount{XPM, UINT64_C(2'500'02), -2},
XRPAmount{90'000'000}));
env(offer(
alice,
STAmount{XPM, UINT64_C(400'009107432), -9},
XRPAmount{14'001'999}));
env(offer(
alice,
STAmount{XPM, UINT64_C(10'000'0714), -4},
XRPAmount{350'020'000}));
env(offer(
alice, STAmount{XPM, UINT64_C(142860)}, XRPAmount{5000175003}));
env(offer(
alice,
STAmount{XPM, UINT64_C(28'572), -3},
XRPAmount{1'000'000}));
env(offer(
alice,
STAmount{XPM, UINT64_C(924'828553344), -9},
XRPAmount{32'368'352}));
env(offer(
alice,
STAmount{XPM, UINT64_C(136'175266308), -9},
XRPAmount{4'766'039}));
env.close();

AMM amm(env, gw, XPM(1), XRPAmount{35'000}, false, 50);

env.close();

if (!features[fixAMMv1_1])
{
env(offer(carol, XRPAmount{34902}, XPM(25'714'285)),
txflags(tfSell | tfImmediateOrCancel),
ter(tecKILLED));
env.close();
BEAST_EXPECT(amm.expectBalances(
XPM(1), XRPAmount{35'000}, amm.tokens()));
BEAST_EXPECT(expectOffers(env, alice, 16));
BEAST_EXPECT(expectOffers(env, carol, 0));
}
else
{
env(offer(carol, XRPAmount{34902}, XPM(25'714'285)),
txflags(tfSell | tfImmediateOrCancel));
env.close();
BEAST_EXPECT(amm.expectBalances(
STAmount{XPM, UINT64_C(8'758'662723307628), -12},
XRPAmount{4},
amm.tokens()));
BEAST_EXPECT(expectOffers(env, alice, 0));
BEAST_EXPECT(expectOffers(env, carol, 0));
}
}
}

void
run() override
{
Expand Down Expand Up @@ -6900,6 +7001,8 @@ struct AMM_test : public jtx::AMMTest
testFixAMMOfferBlockedByLOB(all - fixAMMv1_1);
testLPTokenBalance(all);
testLPTokenBalance(all - fixAMMv1_1);
testFixAMMBlockingLOB(all);
testFixAMMBlockingLOB(all - fixAMMv1_1);
}
};

Expand Down
3 changes: 2 additions & 1 deletion src/test/jtx/impl/TestHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ expectOffers(
}
return true;
});
return size == cnt && matched == toMatch.size();
return size == cnt &&
(toMatch.empty() || (!toMatch.empty() && matched == toMatch.size()));
}

Json::Value
Expand Down

0 comments on commit 12f0273

Please sign in to comment.