Skip to content

Commit

Permalink
Add the fixAMMOfferRounding amendment: (XRPLF#4983)
Browse files Browse the repository at this point in the history
* Fix AMM offer rounding and low quality LOB offer blocking AMM:

A single-path AMM offer with account offer on DEX, is always generated
starting with the takerPays first, which is rounded up, and then
the takerGets, which is rounded down. This rounding ensures that the pool's
product invariant is maintained. However, when one of the offer's side
is XRP, this rounding can result in the AMM offer having a lower
quality, potentially causing offer generation to fail if the quality
is lower than the account's offer quality.

To address this issue, the proposed fix adjusts the offer generation process
to start with the XRP side first and always rounds it down. This results
in a smaller offer size, improving the offer's quality. Regardless if the offer
has XRP or not, the rounding is done so that the offer size is minimized.
This change still ensures the product invariant, as the other generated
side is the exact result of the swap-in or swap-out equations.

If a liquidity can be provided by both AMM and LOB offer on offer crossing
then AMM offer is generated so that it matches LOB offer quality. If LOB
offer quality is less than limit quality then generated AMM offer quality
is also less than limit quality and the offer doesn't cross. To address
this issue, if LOB quality is better than limit quality then use LOB
quality to generate AMM offer. Otherwise, don't use the quality to generate
AMM offer. In this case, limitOut() function in StrandFlow limits
the out amount to match strand's quality to limit quality and consume
maximum AMM liquidity.
  • Loading branch information
gregtatcam authored May 14, 2024
1 parent 244ac5e commit 2705109
Show file tree
Hide file tree
Showing 13 changed files with 986 additions and 222 deletions.
329 changes: 282 additions & 47 deletions src/ripple/app/misc/AMMHelpers.h

Large diffs are not rendered by default.

15 changes: 15 additions & 0 deletions src/ripple/app/misc/impl/AMMHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,4 +203,19 @@ solveQuadraticEq(Number const& a, Number const& b, Number const& c)
return (-b + root2(b * b - 4 * a * c)) / (2 * a);
}

// Minimize takerGets or takerPays
std::optional<Number>
solveQuadraticEqSmallest(Number const& a, Number const& b, Number const& c)
{
auto const d = b * b - 4 * a * c;
if (d < 0)
return std::nullopt;
// use numerically stable citardauq formula for quadratic equation solution
// https://people.csail.mit.edu/bkph/articles/Quadratics.pdf
if (b > 0)
return (2 * c) / (-b - root2(d));
else
return (2 * c) / (-b + root2(d));
}

} // namespace ripple
11 changes: 8 additions & 3 deletions src/ripple/app/paths/impl/AMMLiquidity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ AMMLiquidity<TIn, TOut>::getOffer(
return maxOffer(balances, view.rules());
}
else if (
auto const amounts =
changeSpotPriceQuality(balances, *clobQuality, tradingFee_))
auto const amounts = changeSpotPriceQuality(
balances, *clobQuality, tradingFee_, view.rules(), j_))
{
return AMMOffer<TIn, TOut>(
*this, *amounts, balances, Quality{*amounts});
Expand Down Expand Up @@ -239,7 +239,12 @@ AMMLiquidity<TIn, TOut>::getOffer(
return offer;
}

JLOG(j_.error()) << "AMMLiquidity::getOffer, failed";
JLOG(j_.error()) << "AMMLiquidity::getOffer, failed "
<< ammContext_.multiPath() << " "
<< ammContext_.curIters() << " "
<< (clobQuality ? clobQuality->rate() : STAmount{})
<< " " << to_string(balances.in) << " "
<< to_string(balances.out);
}

return std::nullopt;
Expand Down
79 changes: 67 additions & 12 deletions src/ripple/app/paths/impl/BookStep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,14 @@ class BookPaymentStep : public BookStep<TIn, TOut, BookPaymentStep<TIn, TOut>>
return true;
}

// A payment doesn't use quality threshold (limitQuality)
// since the strand's quality doesn't directly relate to the step's quality.
std::optional<Quality>
qualityThreshold(Quality const& lobQuality) const
{
return lobQuality;
}

// For a payment ofrInRate is always the same as trIn.
std::uint32_t
getOfrInRate(Step const*, AccountID const&, std::uint32_t trIn) const
Expand Down Expand Up @@ -450,6 +458,25 @@ class BookOfferCrossingStep
return !defaultPath_ || quality >= qualityThreshold_;
}

// Return quality threshold or nullopt to use when generating AMM offer.
// AMM synthetic offer is generated to match LOB offer quality.
// If LOB tip offer quality is less than qualityThreshold
// then generated AMM offer quality is also less than qualityThreshold and
// the offer is not crossed even though AMM might generate a better quality
// offer. To address this, if qualityThreshold is greater than lobQuality
// then don't use quality to generate the AMM offer. The limit out value
// generates the maximum AMM offer in this case, which matches
// the quality threshold. This only applies to single path scenario.
// Multi-path AMM offers work the same as LOB offers.
std::optional<Quality>
qualityThreshold(Quality const& lobQuality) const
{
if (this->ammLiquidity_ && !this->ammLiquidity_->multiPath() &&
qualityThreshold_ > lobQuality)
return std::nullopt;
return lobQuality;
}

// For offer crossing don't pay the transfer fee if alice is paying alice.
// A regular (non-offer-crossing) payment does not apply this rule.
std::uint32_t
Expand Down Expand Up @@ -758,8 +785,16 @@ BookStep<TIn, TOut, TDerived>::forEachOffer(
};

// At any payment engine iteration, AMM offer can only be consumed once.
auto tryAMM = [&](std::optional<Quality> const& quality) -> bool {
auto ammOffer = getAMMOffer(sb, quality);
auto tryAMM = [&](std::optional<Quality> const& lobQuality) -> bool {
// If offer crossing then use either LOB quality or nullopt
// to prevent AMM being blocked by a lower quality LOB.
auto const qualityThreshold = [&]() -> std::optional<Quality> {
if (sb.rules().enabled(fixAMMv1_1) && lobQuality)
return static_cast<TDerived const*>(this)->qualityThreshold(
*lobQuality);
return lobQuality;
}();
auto ammOffer = getAMMOffer(sb, qualityThreshold);
return !ammOffer || execOffer(*ammOffer);
};

Expand All @@ -776,7 +811,7 @@ BookStep<TIn, TOut, TDerived>::forEachOffer(
}
else
{
// Might have AMM offer if there is no CLOB offers.
// Might have AMM offer if there are no LOB offers.
tryAMM(std::nullopt);
}

Expand Down Expand Up @@ -851,17 +886,37 @@ BookStep<TIn, TOut, TDerived>::tip(ReadView const& view) const
// This can be simplified (and sped up) if directories are never empty.
Sandbox sb(&view, tapNONE);
BookTip bt(sb, book_);
auto const clobQuality =
auto const lobQuality =
bt.step(j_) ? std::optional<Quality>(bt.quality()) : std::nullopt;
// Don't pass in clobQuality. For one-path it returns the offer as
// the pool balances and the resulting quality is Spot Price Quality.
// For multi-path it returns the actual offer.
// AMM quality is better or no CLOB offer
if (auto const ammOffer = getAMMOffer(view, std::nullopt); ammOffer &&
((clobQuality && ammOffer->quality() > clobQuality) || !clobQuality))
// Multi-path offer generates an offer with the quality
// calculated from the offer size and the quality is constant in this case.
// Single path offer quality changes with the offer size. Spot price quality
// (SPQ) can't be used in this case as the upper bound quality because
// even if SPQ quality is better than LOB quality, it might not be possible
// to generate AMM offer at or better quality than LOB quality. Another
// factor to consider is limit quality on offer crossing. If LOB quality
// is greater than limit quality then use LOB quality when generating AMM
// offer, otherwise don't use quality threshold when generating AMM offer.
// AMM or LOB offer, whether multi-path or single path then can be selected
// based on the best offer quality. Using the quality to generate AMM offer
// in this case also prevents the payment engine from going into multiple
// iterations to cross a LOB offer. This happens when AMM changes
// the out amount at the start of iteration to match the limitQuality
// on offer crossing but AMM can't generate the offer at this quality,
// as the result a LOB offer is partially crossed, and it might take a few
// iterations to fully cross the offer.
auto const qualityThreshold = [&]() -> std::optional<Quality> {
if (view.rules().enabled(fixAMMv1_1) && lobQuality)
return static_cast<TDerived const*>(this)->qualityThreshold(
*lobQuality);
return std::nullopt;
}();
// AMM quality is better or no LOB offer
if (auto const ammOffer = getAMMOffer(view, qualityThreshold); ammOffer &&
((lobQuality && ammOffer->quality() > lobQuality) || !lobQuality))
return ammOffer;
// CLOB quality is better or nullopt
return clobQuality;
// LOB quality is better or nullopt
return lobQuality;
}

template <class TIn, class TOut, class TDerived>
Expand Down
4 changes: 2 additions & 2 deletions src/ripple/app/tx/impl/AMMCreate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,8 @@ applyCreate(
: 1};
sleAMMRoot->setFieldU32(sfSequence, seqno);
// Ignore reserves requirement, disable the master key, allow default
// rippling (AMM LPToken can be used as a token in another AMM, which must
// support payments and offer crossing), and enable deposit authorization to
// rippling (AMM LPToken can be used in payments and offer crossing but
// not as a token in another AMM), and enable deposit authorization to
// prevent payments into AMM.
// Note, that the trustlines created by AMM have 0 credit limit.
// This prevents shifting the balance between accounts via AMM,
Expand Down
20 changes: 20 additions & 0 deletions src/ripple/basics/Number.h
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,26 @@ class saveNumberRoundMode
operator=(saveNumberRoundMode const&) = delete;
};

// saveNumberRoundMode doesn't do quite enough for us. What we want is a
// Number::RoundModeGuard that sets the new mode and restores the old mode
// when it leaves scope. Since Number doesn't have that facility, we'll
// build it here.
class NumberRoundModeGuard
{
saveNumberRoundMode saved_;

public:
explicit NumberRoundModeGuard(Number::rounding_mode mode) noexcept
: saved_{Number::setround(mode)}
{
}

NumberRoundModeGuard(NumberRoundModeGuard const&) = delete;

NumberRoundModeGuard&
operator=(NumberRoundModeGuard const&) = delete;
};

} // namespace ripple

#endif // RIPPLE_BASICS_NUMBER_H_INCLUDED
2 changes: 1 addition & 1 deletion src/ripple/ledger/impl/View.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1147,7 +1147,7 @@ accountSend(
beast::Journal j,
WaiveTransferFee waiveFee)
{
if (view.rules().enabled(fixAMMRounding))
if (view.rules().enabled(fixAMMv1_1))
{
if (saAmount < beast::zero)
{
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/protocol/Feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ extern uint256 const featurePriceOracle;
extern uint256 const fixEmptyDID;
extern uint256 const fixXChainRewardRounding;
extern uint256 const fixPreviousTxnID;
extern uint256 const fixAMMRounding;
extern uint256 const fixAMMv1_1;

} // namespace ripple

Expand Down
2 changes: 1 addition & 1 deletion src/ripple/protocol/impl/Feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 (fixAMMRounding, 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.
Expand Down
20 changes: 0 additions & 20 deletions src/ripple/protocol/impl/STAmount.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1367,26 +1367,6 @@ canonicalizeRoundStrict(

namespace {

// saveNumberRoundMode doesn't do quite enough for us. What we want is a
// Number::RoundModeGuard that sets the new mode and restores the old mode
// when it leaves scope. Since Number doesn't have that facility, we'll
// build it here.
class NumberRoundModeGuard
{
saveNumberRoundMode saved_;

public:
explicit NumberRoundModeGuard(Number::rounding_mode mode) noexcept
: saved_{Number::setround(mode)}
{
}

NumberRoundModeGuard(NumberRoundModeGuard const&) = delete;

NumberRoundModeGuard&
operator=(NumberRoundModeGuard const&) = delete;
};

// We need a class that has an interface similar to NumberRoundModeGuard
// but does nothing.
class DontAffectNumberRoundMode
Expand Down
7 changes: 6 additions & 1 deletion src/test/app/AMMCalc_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -413,13 +413,18 @@ class AMMCalc_test : public beast::unit_test::suite
// 10 is AMM trading fee
else if (*p == "changespq")
{
Env env(*this);
if (auto const pool = getAmounts(++p))
{
if (auto const offer = getAmounts(p))
{
auto const fee = getFee(p);
if (auto const ammOffer = changeSpotPriceQuality(
pool->first, Quality{offer->first}, fee);
pool->first,
Quality{offer->first},
fee,
env.current()->rules(),
beast::Journal(beast::Journal::getNullSink()));
ammOffer)
std::cout
<< "amm offer: " << toString(ammOffer->in)
Expand Down
Loading

0 comments on commit 2705109

Please sign in to comment.