diff --git a/src/ripple/app/misc/AMMHelpers.h b/src/ripple/app/misc/AMMHelpers.h index 7bdaf23d6..787bb2300 100644 --- a/src/ripple/app/misc/AMMHelpers.h +++ b/src/ripple/app/misc/AMMHelpers.h @@ -21,7 +21,9 @@ #define RIPPLE_APP_MISC_AMMHELPERS_H_INCLUDED #include +#include #include +#include #include #include #include @@ -35,6 +37,20 @@ namespace ripple { +namespace detail { + +Number +reduceOffer(auto const& amount) +{ + static Number const reducedOfferPct(9999, -4); + + // Make sure the result is always less than amount or zero. + NumberRoundModeGuard mg(Number::towards_zero); + return amount * reducedOfferPct; +} + +} // namespace detail + /** Calculate LP Tokens given AMM pool reserves. * @param asset1 AMM one side of the pool reserve * @param asset2 AMM another side of the pool reserve @@ -147,12 +163,165 @@ withinRelativeDistance(Amt const& calc, Amt const& req, Number const& dist) } // clang-format on -/** Finds takerPays (i) and takerGets (o) such that given pool composition - * poolGets(I) and poolPays(O): (O - o) / (I + i) = quality. - * Where takerGets is calculated as the swapAssetIn (see below). - * The above equation produces the quadratic equation: - * i^2*(1-fee) + i*I*(2-fee) + I^2 - I*O/quality, - * which is solved for i, and o is found with swapAssetIn(). +/** Solve quadratic equation to find takerGets or takerPays. Round + * to minimize the amount in order to maximize the quality. + */ +std::optional +solveQuadraticEqSmallest(Number const& a, Number const& b, Number const& c); + +/** Generate AMM offer starting with takerGets when AMM pool + * from the payment perspective is IOU(in)/XRP(out) + * Equations: + * Spot Price Quality after the offer is consumed: + * Qsp = (O - o) / (I + i) -- equation (1) + * where O is poolPays, I is poolGets, o is takerGets, i is takerPays + * Swap out: + * i = (I * o) / (O - o) * f -- equation (2) + * where f is (1 - tfee/100000), tfee is in basis points + * Effective price targetQuality: + * Qep = o / i -- equation (3) + * There are two scenarios to consider + * A) Qsp = Qep. Substitute i in (1) with (2) and solve for o + * and Qsp = targetQuality(Qt): + * o**2 + o * (I * Qt * (1 - 1 / f) - 2 * O) + O**2 - Qt * I * O = 0 + * B) Qep = Qsp. Substitute i in (3) with (2) and solve for o + * and Qep = targetQuality(Qt): + * o = O - I * Qt / f + * Since the scenario is not known a priori, both A and B are solved and + * the lowest value of o is takerGets. takerPays is calculated with + * swap out eq (2). If o is less or equal to 0 then the offer can't + * be generated. + */ +template +std::optional> +getAMMOfferStartWithTakerGets( + TAmounts const& pool, + Quality const& targetQuality, + std::uint16_t const& tfee) +{ + if (targetQuality.rate() == beast::zero) + return std::nullopt; + + NumberRoundModeGuard mg(Number::to_nearest); + auto const f = feeMult(tfee); + auto const a = 1; + auto const b = pool.in * (1 - 1 / f) / targetQuality.rate() - 2 * pool.out; + auto const c = + pool.out * pool.out - (pool.in * pool.out) / targetQuality.rate(); + + auto nTakerGets = solveQuadraticEqSmallest(a, b, c); + if (!nTakerGets || *nTakerGets <= 0) + return std::nullopt; // LCOV_EXCL_LINE + + auto const nTakerGetsConstraint = + pool.out - pool.in / (targetQuality.rate() * f); + if (nTakerGetsConstraint <= 0) + return std::nullopt; + + // Select the smallest to maximize the quality + if (nTakerGetsConstraint < *nTakerGets) + nTakerGets = nTakerGetsConstraint; + + auto getAmounts = [&pool, &tfee](Number const& nTakerGetsProposed) { + // Round downward to minimize the offer and to maximize the quality. + // This has the most impact when takerGets is XRP. + auto const takerGets = toAmount( + getIssue(pool.out), nTakerGetsProposed, Number::downward); + return TAmounts{ + swapAssetOut(pool, takerGets, tfee), takerGets}; + }; + + // Try to reduce the offer size to improve the quality. + // The quality might still not match the targetQuality for a tiny offer. + if (auto const amounts = getAmounts(*nTakerGets); + Quality{amounts} < targetQuality) + return getAmounts(detail::reduceOffer(amounts.out)); + else + return amounts; +} + +/** Generate AMM offer starting with takerPays when AMM pool + * from the payment perspective is XRP(in)/IOU(out) or IOU(in)/IOU(out). + * Equations: + * Spot Price Quality after the offer is consumed: + * Qsp = (O - o) / (I + i) -- equation (1) + * where O is poolPays, I is poolGets, o is takerGets, i is takerPays + * Swap in: + * o = (O * i * f) / (I + i * f) -- equation (2) + * where f is (1 - tfee/100000), tfee is in basis points + * Effective price quality: + * Qep = o / i -- equation (3) + * There are two scenarios to consider + * A) Qsp = Qep. Substitute o in (1) with (2) and solve for i + * and Qsp = targetQuality(Qt): + * i**2 * f + i * I * (1 + f) + I**2 - I * O / Qt = 0 + * B) Qep = Qsp. Substitute i in (3) with (2) and solve for i + * and Qep = targetQuality(Qt): + * i = O / Qt - I / f + * Since the scenario is not known a priori, both A and B are solved and + * the lowest value of i is takerPays. takerGets is calculated with + * swap in eq (2). If i is less or equal to 0 then the offer can't + * be generated. + */ +template +std::optional> +getAMMOfferStartWithTakerPays( + TAmounts const& pool, + Quality const& targetQuality, + std::uint16_t tfee) +{ + if (targetQuality.rate() == beast::zero) + return std::nullopt; + + NumberRoundModeGuard mg(Number::to_nearest); + auto const f = feeMult(tfee); + auto const& a = f; + auto const b = pool.in * (1 + f); + auto const c = + pool.in * pool.in - pool.in * pool.out * targetQuality.rate(); + + auto nTakerPays = solveQuadraticEqSmallest(a, b, c); + if (!nTakerPays || nTakerPays <= 0) + return std::nullopt; // LCOV_EXCL_LINE + + auto const nTakerPaysConstraint = + pool.out * targetQuality.rate() - pool.in / f; + if (nTakerPaysConstraint <= 0) + return std::nullopt; + + // Select the smallest to maximize the quality + if (nTakerPaysConstraint < *nTakerPays) + nTakerPays = nTakerPaysConstraint; + + auto getAmounts = [&pool, &tfee](Number const& nTakerPaysProposed) { + // Round downward to minimize the offer and to maximize the quality. + // This has the most impact when takerPays is XRP. + auto const takerPays = toAmount( + getIssue(pool.in), nTakerPaysProposed, Number::downward); + return TAmounts{ + takerPays, swapAssetIn(pool, takerPays, tfee)}; + }; + + // Try to reduce the offer size to improve the quality. + // The quality might still not match the targetQuality for a tiny offer. + if (auto const amounts = getAmounts(*nTakerPays); + Quality{amounts} < targetQuality) + return getAmounts(detail::reduceOffer(amounts.in)); + else + return amounts; +} + +/** Generate AMM offer so that either updated Spot Price Quality (SPQ) + * is equal to LOB quality (in this case AMM offer quality is + * better than LOB quality) or AMM offer is equal to LOB quality + * (in this case SPQ is better than LOB quality). + * Pre-amendment code calculates takerPays first. If takerGets is XRP, + * it is rounded down, which results in worse offer quality than + * LOB quality, and the offer might fail to generate. + * Post-amendment code calculates the XRP offer side first. The result + * is rounded down, which makes the offer quality better. + * It might not be possible to match either SPQ or AMM offer to LOB + * quality. This generally happens at higher fees. * @param pool AMM pool balances * @param quality requested quality * @param tfee trading fee in basis points @@ -163,43 +332,111 @@ std::optional> changeSpotPriceQuality( TAmounts const& pool, Quality const& quality, - std::uint16_t tfee) + std::uint16_t tfee, + Rules const& rules, + beast::Journal j) { - auto const f = feeMult(tfee); // 1 - fee - auto const& a = f; - auto const b = pool.in * (1 + f); - Number const c = pool.in * pool.in - pool.in * pool.out * quality.rate(); - if (auto const res = b * b - 4 * a * c; res < 0) - return std::nullopt; - else if (auto const nTakerPaysPropose = (-b + root2(res)) / (2 * a); - nTakerPaysPropose > 0) + if (!rules.enabled(fixAMMv1_1)) { - auto const nTakerPays = [&]() { - // The fee might make the AMM offer quality less than CLOB quality. - // Therefore, AMM offer has to satisfy this constraint: o / i >= q. - // Substituting o with swapAssetIn() gives: - // i <= O / q - I / (1 - fee). - auto const nTakerPaysConstraint = - pool.out * quality.rate() - pool.in / f; - if (nTakerPaysPropose > nTakerPaysConstraint) - return nTakerPaysConstraint; - return nTakerPaysPropose; - }(); - if (nTakerPays <= 0) - return std::nullopt; - auto const takerPays = toAmount( - getIssue(pool.in), nTakerPays, Number::rounding_mode::upward); - // should not fail - if (auto const amounts = - TAmounts{ - takerPays, swapAssetIn(pool, takerPays, tfee)}; - Quality{amounts} < quality && - !withinRelativeDistance(Quality{amounts}, quality, Number(1, -7))) - Throw("changeSpotPriceQuality failed"); - else - return amounts; + // Finds takerPays (i) and takerGets (o) such that given pool + // composition poolGets(I) and poolPays(O): (O - o) / (I + i) = quality. + // Where takerGets is calculated as the swapAssetIn (see below). + // The above equation produces the quadratic equation: + // i^2*(1-fee) + i*I*(2-fee) + I^2 - I*O/quality, + // which is solved for i, and o is found with swapAssetIn(). + auto const f = feeMult(tfee); // 1 - fee + auto const& a = f; + auto const b = pool.in * (1 + f); + Number const c = + pool.in * pool.in - pool.in * pool.out * quality.rate(); + if (auto const res = b * b - 4 * a * c; res < 0) + return std::nullopt; // LCOV_EXCL_LINE + else if (auto const nTakerPaysPropose = (-b + root2(res)) / (2 * a); + nTakerPaysPropose > 0) + { + auto const nTakerPays = [&]() { + // The fee might make the AMM offer quality less than CLOB + // quality. Therefore, AMM offer has to satisfy this constraint: + // o / i >= q. Substituting o with swapAssetIn() gives: i <= O / + // q - I / (1 - fee). + auto const nTakerPaysConstraint = + pool.out * quality.rate() - pool.in / f; + if (nTakerPaysPropose > nTakerPaysConstraint) + return nTakerPaysConstraint; + return nTakerPaysPropose; + }(); + if (nTakerPays <= 0) + { + JLOG(j.trace()) + << "changeSpotPriceQuality calc failed: " + << to_string(pool.in) << " " << to_string(pool.out) << " " + << quality << " " << tfee; + return std::nullopt; + } + auto const takerPays = + toAmount(getIssue(pool.in), nTakerPays, Number::upward); + // should not fail + if (auto const amounts = + TAmounts{ + takerPays, swapAssetIn(pool, takerPays, tfee)}; + Quality{amounts} < quality && + !withinRelativeDistance( + Quality{amounts}, quality, Number(1, -7))) + { + JLOG(j.error()) + << "changeSpotPriceQuality failed: " << to_string(pool.in) + << " " << to_string(pool.out) << " " + << " " << quality << " " << tfee << " " + << to_string(amounts.in) << " " << to_string(amounts.out); + Throw("changeSpotPriceQuality failed"); + } + else + { + JLOG(j.trace()) + << "changeSpotPriceQuality succeeded: " + << to_string(pool.in) << " " << to_string(pool.out) << " " + << " " << quality << " " << tfee << " " + << to_string(amounts.in) << " " << to_string(amounts.out); + return amounts; + } + } + JLOG(j.trace()) << "changeSpotPriceQuality calc failed: " + << to_string(pool.in) << " " << to_string(pool.out) + << " " << quality << " " << tfee; + return std::nullopt; } - return std::nullopt; + + // Generate the offer starting with XRP side. Return seated offer amounts + // if the offer can be generated, otherwise nullopt. + auto const amounts = [&]() { + if (isXRP(getIssue(pool.out))) + return getAMMOfferStartWithTakerGets(pool, quality, tfee); + return getAMMOfferStartWithTakerPays(pool, quality, tfee); + }(); + if (!amounts) + { + JLOG(j.trace()) << "changeSpotPrice calc failed: " << to_string(pool.in) + << " " << to_string(pool.out) << " " << quality << " " + << tfee << std::endl; + return std::nullopt; + } + + if (Quality{*amounts} < quality) + { + JLOG(j.error()) << "changeSpotPriceQuality failed: " + << to_string(pool.in) << " " << to_string(pool.out) + << " " << quality << " " << tfee << " " + << to_string(amounts->in) << " " + << to_string(amounts->out); + return std::nullopt; + } + + JLOG(j.trace()) << "changeSpotPriceQuality succeeded: " + << to_string(pool.in) << " " << to_string(pool.out) << " " + << " " << quality << " " << tfee << " " + << to_string(amounts->in) << " " << to_string(amounts->out); + + return amounts; } /** AMM pool invariant - the product (A * B) after swap in/out has to remain @@ -231,7 +468,7 @@ swapAssetIn( std::uint16_t tfee) { if (auto const& rules = getCurrentTransactionRules(); - rules && rules->enabled(fixAMMRounding)) + rules && rules->enabled(fixAMMv1_1)) { // set rounding to always favor the amm. Clip to zero. // calculate: @@ -275,8 +512,7 @@ swapAssetIn( if (swapOut.signum() < 0) return toAmount(getIssue(pool.out), 0); - return toAmount( - getIssue(pool.out), swapOut, Number::rounding_mode::downward); + return toAmount(getIssue(pool.out), swapOut, Number::downward); } else { @@ -284,7 +520,7 @@ swapAssetIn( getIssue(pool.out), pool.out - (pool.in * pool.out) / (pool.in + assetIn * feeMult(tfee)), - Number::rounding_mode::downward); + Number::downward); } } @@ -305,7 +541,7 @@ swapAssetOut( std::uint16_t tfee) { if (auto const& rules = getCurrentTransactionRules(); - rules && rules->enabled(fixAMMRounding)) + rules && rules->enabled(fixAMMv1_1)) { // set rounding to always favor the amm. Clip to zero. // calculate: @@ -349,8 +585,7 @@ swapAssetOut( if (swapIn.signum() < 0) return toAmount(getIssue(pool.in), 0); - return toAmount( - getIssue(pool.in), swapIn, Number::rounding_mode::upward); + return toAmount(getIssue(pool.in), swapIn, Number::upward); } else { @@ -358,7 +593,7 @@ swapAssetOut( getIssue(pool.in), ((pool.in * pool.out) / (pool.out - assetOut) - pool.in) / feeMult(tfee), - Number::rounding_mode::upward); + Number::upward); } } diff --git a/src/ripple/app/misc/impl/AMMHelpers.cpp b/src/ripple/app/misc/impl/AMMHelpers.cpp index 736743eaa..57b3d3a07 100644 --- a/src/ripple/app/misc/impl/AMMHelpers.cpp +++ b/src/ripple/app/misc/impl/AMMHelpers.cpp @@ -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 +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 diff --git a/src/ripple/app/paths/impl/AMMLiquidity.cpp b/src/ripple/app/paths/impl/AMMLiquidity.cpp index bcc086e23..9ec23d08a 100644 --- a/src/ripple/app/paths/impl/AMMLiquidity.cpp +++ b/src/ripple/app/paths/impl/AMMLiquidity.cpp @@ -205,8 +205,8 @@ AMMLiquidity::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( *this, *amounts, balances, Quality{*amounts}); @@ -239,7 +239,12 @@ AMMLiquidity::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; diff --git a/src/ripple/app/paths/impl/BookStep.cpp b/src/ripple/app/paths/impl/BookStep.cpp index e0b573bcc..b56b77cba 100644 --- a/src/ripple/app/paths/impl/BookStep.cpp +++ b/src/ripple/app/paths/impl/BookStep.cpp @@ -297,6 +297,14 @@ public: 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 + 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 @@ -450,6 +458,25 @@ public: 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 + 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 @@ -758,8 +785,16 @@ BookStep::forEachOffer( }; // At any payment engine iteration, AMM offer can only be consumed once. - auto tryAMM = [&](std::optional const& quality) -> bool { - auto ammOffer = getAMMOffer(sb, quality); + auto tryAMM = [&](std::optional 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 { + if (sb.rules().enabled(fixAMMv1_1) && lobQuality) + return static_cast(this)->qualityThreshold( + *lobQuality); + return lobQuality; + }(); + auto ammOffer = getAMMOffer(sb, qualityThreshold); return !ammOffer || execOffer(*ammOffer); }; @@ -776,7 +811,7 @@ BookStep::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); } @@ -851,17 +886,37 @@ BookStep::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(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 { + if (view.rules().enabled(fixAMMv1_1) && lobQuality) + return static_cast(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 diff --git a/src/ripple/app/tx/impl/AMMCreate.cpp b/src/ripple/app/tx/impl/AMMCreate.cpp index 739a96a43..70fcc5c69 100644 --- a/src/ripple/app/tx/impl/AMMCreate.cpp +++ b/src/ripple/app/tx/impl/AMMCreate.cpp @@ -254,8 +254,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, diff --git a/src/ripple/basics/Number.h b/src/ripple/basics/Number.h index 48cea443e..cdc25b3b2 100644 --- a/src/ripple/basics/Number.h +++ b/src/ripple/basics/Number.h @@ -387,6 +387,26 @@ public: 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 diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index a3e41df2c..72fc9703e 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -1242,7 +1242,7 @@ accountSend( WaiveTransferFee waiveFee, bool const senderPaysXferFees) { - if (view.rules().enabled(fixAMMRounding)) + if (view.rules().enabled(fixAMMv1_1)) { if (saAmount < beast::zero) { diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index fbf45ac50..e704e4b94 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -383,7 +383,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 diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index d5aada3d1..6048cfc5c 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -489,7 +489,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. diff --git a/src/ripple/protocol/impl/STAmount.cpp b/src/ripple/protocol/impl/STAmount.cpp index 201bcb6b6..a02dc9e89 100644 --- a/src/ripple/protocol/impl/STAmount.cpp +++ b/src/ripple/protocol/impl/STAmount.cpp @@ -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 diff --git a/src/test/app/AMMCalc_test.cpp b/src/test/app/AMMCalc_test.cpp index cdb57c3b4..e230ed4d3 100644 --- a/src/test/app/AMMCalc_test.cpp +++ b/src/test/app/AMMCalc_test.cpp @@ -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) diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index 4a3db2301..d5803271e 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -94,7 +94,7 @@ private: sendmax(BTC(1'000)), txflags(tfPartialPayment)); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { BEAST_EXPECT(ammCarol.expectBalances( STAmount{BTC, UINT64_C(1'001'000000374812), -12}, @@ -720,7 +720,7 @@ private: auto const jrr = env.rpc("json", "submit", to_string(payment)); BEAST_EXPECT(jrr[jss::result][jss::status] == "success"); BEAST_EXPECT(jrr[jss::result][jss::engine_result] == "tesSUCCESS"); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { BEAST_EXPECT(ammAlice.expectBalances( STAmount(XTS, UINT64_C(101'010101010101), -12), @@ -1291,17 +1291,34 @@ private: env(offer(cam, B_BUX(30), A_BUX(30))); // AMM is consumed up to the first cam Offer quality - BEAST_EXPECT(ammCarol.expectBalances( - STAmount{A_BUX, UINT64_C(309'3541659651605), -13}, - STAmount{B_BUX, UINT64_C(320'0215509984417), -13}, - ammCarol.tokens())); - BEAST_EXPECT(expectOffers( - env, - cam, - 1, - {{Amounts{ - STAmount{B_BUX, UINT64_C(20'0215509984417), -13}, - STAmount{A_BUX, UINT64_C(20'0215509984417), -13}}}})); + if (!features[fixAMMv1_1]) + { + BEAST_EXPECT(ammCarol.expectBalances( + STAmount{A_BUX, UINT64_C(309'3541659651605), -13}, + STAmount{B_BUX, UINT64_C(320'0215509984417), -13}, + ammCarol.tokens())); + BEAST_EXPECT(expectOffers( + env, + cam, + 1, + {{Amounts{ + STAmount{B_BUX, UINT64_C(20'0215509984417), -13}, + STAmount{A_BUX, UINT64_C(20'0215509984417), -13}}}})); + } + else + { + BEAST_EXPECT(ammCarol.expectBalances( + STAmount{A_BUX, UINT64_C(309'3541659651604), -13}, + STAmount{B_BUX, UINT64_C(320'0215509984419), -13}, + ammCarol.tokens())); + BEAST_EXPECT(expectOffers( + env, + cam, + 1, + {{Amounts{ + STAmount{B_BUX, UINT64_C(20'0215509984419), -13}, + STAmount{A_BUX, UINT64_C(20'0215509984419), -13}}}})); + } } void @@ -1427,7 +1444,7 @@ private: using namespace jtx; FeatureBitset const all{supported_amendments()}; testRmFundedOffer(all); - testRmFundedOffer(all - fixAMMRounding); + testRmFundedOffer(all - fixAMMv1_1); testEnforceNoRipple(all); testFillModes(all); testOfferCrossWithXRP(all); @@ -1441,28 +1458,17 @@ private: testOfferCreateThenCross(all); testSellFlagExceedLimit(all); testGatewayCrossCurrency(all); - testGatewayCrossCurrency(all - fixAMMRounding); - // testPartialCross - // testXRPDirectCross - // testDirectCross + testGatewayCrossCurrency(all - fixAMMv1_1); testBridgedCross(all); - // testSellOffer testSellWithFillOrKill(all); testTransferRateOffer(all); testSelfIssueOffer(all); testBadPathAssert(all); testSellFlagBasic(all); testDirectToDirectPath(all); - // testSelfCrossLowQualityOffer - // testOfferInScaling - // testOfferInScalingWithXferRate - // testOfferThresholdWithReducedFunds - // testTinyOffer - // testSelfPayXferFeeOffer - // testSelfPayXferFeeOffer + testDirectToDirectPath(all - fixAMMv1_1); testRequireAuth(all); testMissingAuth(all); - // testRCSmoketest } void @@ -2320,7 +2326,7 @@ private: txflags(tfNoRippleDirect | tfPartialPayment | tfLimitQuality)); env.close(); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { // alice buys 77.2727USD with 75.5555GBP and pays 25% tr fee // on 75.5555GBP @@ -2367,7 +2373,7 @@ private: env(offer(alice, EUR(100), USD(100))); env.close(); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { // 95.2380USD is swapped in for 100EUR BEAST_EXPECT(amm.expectBalances( @@ -2420,7 +2426,7 @@ private: env(pay(gw, dan, USD(1'000))); AMM ammDan(env, dan, USD(1'000), EUR(1'050)); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { // alice -> bob -> gw -> carol. $50 should have transfer fee; // $10, no fee @@ -2489,7 +2495,7 @@ private: // alice buys 107.1428USD with 120GBP and pays 25% tr fee on 120GBP // 1,000 - 120*1.25 = 850GBP BEAST_EXPECT(expectLine(env, alice, GBP(850))); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { // 120GBP is swapped in for 107.1428USD BEAST_EXPECT(amm.expectBalances( @@ -2578,7 +2584,7 @@ private: env.close(); BEAST_EXPECT(expectLine(env, alice, GBP(850))); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { // alice buys 107.1428EUR with 120GBP and pays 25% tr fee on // 120GBP 1,000 - 120*1.25 = 850GBP 120GBP is swapped in for @@ -2695,7 +2701,7 @@ private: txflags(tfNoRippleDirect | tfPartialPayment | tfLimitQuality)); env.close(); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { // alice buys 28.125USD with 24GBP and pays 25% tr fee // on 24GBP @@ -2752,7 +2758,7 @@ private: txflags(tfNoRippleDirect | tfPartialPayment | tfLimitQuality)); env.close(); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { // alice buys 70.4210EUR with 70.4210GBP via the offer // and pays 25% tr fee on 70.4210GBP @@ -2844,7 +2850,7 @@ private: txflags(tfNoRippleDirect | tfPartialPayment | tfLimitQuality)); env.close(); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { // alice buys 53.3322EUR with 56.3368GBP via the amm // and pays 25% tr fee on 56.3368GBP @@ -2922,7 +2928,7 @@ private: txflags(tfNoRippleDirect | tfPartialPayment | tfLimitQuality)); env.close(); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { // alice buys 53.3322EUR with 107.5308GBP // 25% on 86.0246GBP is paid in tr fee @@ -2993,7 +2999,7 @@ private: txflags(tfNoRippleDirect | tfPartialPayment | tfLimitQuality)); env.close(); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { // 108.1481GBP is swapped in for 97.5935EUR BEAST_EXPECT(amm1.expectBalances( @@ -3166,8 +3172,12 @@ private: // Alice offers to buy 1000 XRP for 1000 USD. She takes Bob's first // offer, removes 999 more as unfunded, then hits the step limit. env(offer(alice, USD(1'000), XRP(1'000))); - env.require( - balance(alice, STAmount{USD, UINT64_C(2'050126257867561), -15})); + if (!features[fixAMMv1_1]) + env.require(balance( + alice, STAmount{USD, UINT64_C(2'050126257867561), -15})); + else + env.require(balance( + alice, STAmount{USD, UINT64_C(2'050125257867587), -15})); env.require(owners(alice, 2)); env.require(balance(bob, USD(0))); env.require(owners(bob, 1'001)); @@ -3273,7 +3283,7 @@ private: env(offer(bob, XRP(100), USD(100))); env(offer(bob, XRP(1'000), USD(100))); AMM ammDan(env, dan, XRP(1'000), USD(1'100)); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { env(pay(alice, carol, USD(10'000)), paths(XRP), @@ -4090,9 +4100,9 @@ private: testBookStep(all); testBookStep(all | ownerPaysFee); testTransferRate(all | ownerPaysFee); - testTransferRate((all - fixAMMRounding) | ownerPaysFee); + testTransferRate((all - fixAMMv1_1) | ownerPaysFee); testTransferRateNoOwnerFee(all); - testTransferRateNoOwnerFee(all - fixAMMRounding); + testTransferRateNoOwnerFee(all - fixAMMv1_1); testLimitQuality(); testXRPPathLoop(); } @@ -4103,6 +4113,7 @@ private: using namespace jtx; FeatureBitset const all{supported_amendments()}; testStepLimit(all); + testStepLimit(all - fixAMMv1_1); } void @@ -4111,7 +4122,7 @@ private: using namespace jtx; FeatureBitset const all{supported_amendments()}; test_convert_all_of_an_asset(all); - test_convert_all_of_an_asset(all - fixAMMRounding); + test_convert_all_of_an_asset(all - fixAMMv1_1); } void diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 6605bb25e..baa39ae43 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -37,6 +37,8 @@ #include #include +#include + namespace ripple { namespace test { @@ -2955,7 +2957,7 @@ private: // alice pays ~1.011USD in fees, which is ~10 times more // than carol's fee // 100.099431529USD swapped in for 100XRP - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { BEAST_EXPECT(ammAlice.expectBalances( XRPAmount{13'000'000'668}, @@ -2984,7 +2986,7 @@ private: } // carol pays ~9.94USD in fees, which is ~10 times more in // trading fees vs discounted fee. - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { BEAST_EXPECT( env.balance(carol, USD) == @@ -3009,7 +3011,7 @@ private: // carol pays ~1.008XRP in trading fee, which is // ~10 times more than the discounted fee. // 99.815876XRP is swapped in for 100USD - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { BEAST_EXPECT(ammAlice.expectBalances( XRPAmount(13'100'824'790), @@ -3111,7 +3113,7 @@ private: IOUAmount{1'004'487'562112089, -9})); // Bob pays the full fee ~0.1USD env(pay(bob, alice, XRP(10)), path(~XRP), sendmax(USD(11))); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { BEAST_EXPECT(amm.expectBalances( XRPAmount{1'000'010'011}, @@ -3520,7 +3522,7 @@ private: XRPAmount(10'030'082'730), STAmount(EUR, UINT64_C(9'970'007498125468), -12), ammEUR_XRP.tokens())); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { BEAST_EXPECT(ammUSD_EUR.expectBalances( STAmount(USD, UINT64_C(9'970'097277662122), -12), @@ -3621,7 +3623,7 @@ private: sendmax(XRP(200)), txflags(tfPartialPayment)); env.close(); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { BEAST_EXPECT(ammAlice.expectBalances( XRP(10'100), USD(10'000), ammAlice.tokens())); @@ -3833,7 +3835,7 @@ private: path(~USD), path(~ETH, ~EUR, ~USD), sendmax(XRP(200))); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { // XRP-ETH-EUR-USD // This path provides ~26.06USD/26.2XRP @@ -3914,7 +3916,7 @@ private: path(~EUR, ~BTC, ~USD), path(~ETH, ~EUR, ~BTC, ~USD), sendmax(XRP(200))); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { // XRP-EUR-BTC-USD path provides ~17.8USD/~18.7XRP // XRP-ETH-EUR-BTC-USD path provides ~82.2USD/82.4XRP @@ -3981,7 +3983,7 @@ private: path(~XRP, ~USD), sendmax(EUR(400)), txflags(tfPartialPayment | tfNoRippleDirect)); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { // Carol gets ~29.91USD because of the AMM offers limit BEAST_EXPECT(ammAlice.expectBalances( @@ -4028,7 +4030,7 @@ private: txflags(tfPartialPayment | tfNoRippleDirect)); BEAST_EXPECT(ammAlice.expectBalances( XRPAmount{10'101'010'102}, USD(9'900), ammAlice.tokens())); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { // Carol gets ~100USD BEAST_EXPECT(expectLine( @@ -4060,17 +4062,33 @@ private: env(offer(bob, XRP(100), USD(100.001))); AMM ammAlice(env, alice, XRP(10'000), USD(10'100)); env(offer(carol, USD(100), XRP(100))); - BEAST_EXPECT(ammAlice.expectBalances( - XRPAmount{10'049'825'373}, - STAmount{USD, UINT64_C(10'049'92586949302), -11}, - ammAlice.tokens())); - BEAST_EXPECT(expectOffers( - env, - bob, - 1, - {{{XRPAmount{50'074'629}, - STAmount{USD, UINT64_C(50'07513050698), -11}}}})); - BEAST_EXPECT(expectLine(env, carol, USD(30'100))); + if (!features[fixAMMv1_1]) + { + BEAST_EXPECT(ammAlice.expectBalances( + XRPAmount{10'049'825'373}, + STAmount{USD, UINT64_C(10'049'92586949302), -11}, + ammAlice.tokens())); + BEAST_EXPECT(expectOffers( + env, + bob, + 1, + {{{XRPAmount{50'074'629}, + STAmount{USD, UINT64_C(50'07513050698), -11}}}})); + } + else + { + BEAST_EXPECT(ammAlice.expectBalances( + XRPAmount{10'049'825'372}, + STAmount{USD, UINT64_C(10'049'92587049303), -11}, + ammAlice.tokens())); + BEAST_EXPECT(expectOffers( + env, + bob, + 1, + {{{XRPAmount{50'074'628}, + STAmount{USD, UINT64_C(50'07512950697), -11}}}})); + BEAST_EXPECT(expectLine(env, carol, USD(30'100))); + } } // Individually frozen account @@ -4342,7 +4360,7 @@ private: // Execute with CLOB offer prep( [&](Env& env) { - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) env(offer( LP1, XRPAmount{18'095'133}, @@ -4352,7 +4370,7 @@ private: env(offer( LP1, XRPAmount{18'095'132}, - STAmount{TST, UINT64_C(1'68737984885387), -14}), + STAmount{TST, UINT64_C(1'68737976189735), -14}), txflags(tfPassive)); }, [&](Env& env) { @@ -4584,7 +4602,7 @@ private: {{Amounts{ STAmount{EUR, UINT64_C(5'025125628140703), -15}, STAmount{USD, UINT64_C(5'025125628140703), -15}}}})); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { BEAST_EXPECT(ammAlice.expectBalances( STAmount{USD, UINT64_C(1'004'974874371859), -12}, @@ -4624,7 +4642,7 @@ private: sendmax(EUR(15)), txflags(tfNoRippleDirect)); BEAST_EXPECT(expectLine(env, ed, USD(2'010))); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { BEAST_EXPECT(expectLine(env, bob, EUR(1'990))); BEAST_EXPECT(ammAlice.expectBalances( @@ -4661,7 +4679,7 @@ private: sendmax(EUR(15)), txflags(tfNoRippleDirect)); BEAST_EXPECT(expectLine(env, ed, USD(2'010))); - if (!features[fixAMMRounding]) + if (!features[fixAMMv1_1]) { BEAST_EXPECT(expectLine( env, @@ -4677,10 +4695,10 @@ private: BEAST_EXPECT(expectLine( env, bob, - STAmount{EUR, UINT64_C(1'989'987453007616), -12})); + STAmount{EUR, UINT64_C(1'989'987453007628), -12})); BEAST_EXPECT(ammAlice.expectBalances( USD(1'000), - STAmount{EUR, UINT64_C(1'005'012546992384), -12}, + STAmount{EUR, UINT64_C(1'005'012546992372), -12}, ammAlice.tokens())); } BEAST_EXPECT(expectOffers(env, carol, 0)); @@ -5192,35 +5210,69 @@ private: BEAST_EXPECT(!amm->expectBalances( USD(1'000), ETH(1'000), amm->tokens())); } - if (i == 2 && !features[fixAMMRounding]) + if (i == 2 && !features[fixAMMv1_1]) { if (rates.first == 1.5) { - BEAST_EXPECT(expectOffers( - env, - ed, - 1, - {{Amounts{ - STAmount{ - ETH, UINT64_C(378'6327949540823), -13}, - STAmount{ - USD, - UINT64_C(283'9745962155617), - -13}}}})); + if (!features[fixAMMv1_1]) + BEAST_EXPECT(expectOffers( + env, + ed, + 1, + {{Amounts{ + STAmount{ + ETH, + UINT64_C(378'6327949540823), + -13}, + STAmount{ + USD, + UINT64_C(283'9745962155617), + -13}}}})); + else + BEAST_EXPECT(expectOffers( + env, + ed, + 1, + {{Amounts{ + STAmount{ + ETH, + UINT64_C(378'6327949540813), + -13}, + STAmount{ + USD, + UINT64_C(283'974596215561), + -12}}}})); } else { - BEAST_EXPECT(expectOffers( - env, - ed, - 1, - {{Amounts{ - STAmount{ - ETH, UINT64_C(325'299461620749), -12}, - STAmount{ - USD, - UINT64_C(243'9745962155617), - -13}}}})); + if (!features[fixAMMv1_1]) + BEAST_EXPECT(expectOffers( + env, + ed, + 1, + {{Amounts{ + STAmount{ + ETH, + UINT64_C(325'299461620749), + -12}, + STAmount{ + USD, + UINT64_C(243'9745962155617), + -13}}}})); + else + BEAST_EXPECT(expectOffers( + env, + ed, + 1, + {{Amounts{ + STAmount{ + ETH, + UINT64_C(325'299461620748), + -12}, + STAmount{ + USD, + UINT64_C(243'974596215561), + -12}}}})); } } else if (i == 2) @@ -5292,29 +5344,71 @@ private: { if (rates.first == 1.5) { - BEAST_EXPECT(expectOffers( - env, ed, 1, {{Amounts{ETH(400), USD(250)}}})); - BEAST_EXPECT(expectOffers( - env, - alice, - 1, - {{Amounts{ - STAmount{USD, UINT64_C(40'5694150420947), -13}, - STAmount{ETH, UINT64_C(64'91106406735152), -14}, - }}})); + if (!features[fixAMMv1_1]) + { + BEAST_EXPECT(expectOffers( + env, ed, 1, {{Amounts{ETH(400), USD(250)}}})); + BEAST_EXPECT(expectOffers( + env, + alice, + 1, + {{Amounts{ + STAmount{ + USD, UINT64_C(40'5694150420947), -13}, + STAmount{ + ETH, UINT64_C(64'91106406735152), -14}, + }}})); + } + else + { + // Ed offer is partially crossed. + // The updated rounding makes limitQuality + // work if both amendments are enabled + BEAST_EXPECT(expectOffers( + env, + ed, + 1, + {{Amounts{ + STAmount{ + ETH, UINT64_C(335'0889359326475), -13}, + STAmount{ + USD, UINT64_C(209'4305849579047), -13}, + }}})); + BEAST_EXPECT(expectOffers(env, alice, 0)); + } } else { - // Ed offer is partially crossed. - BEAST_EXPECT(expectOffers( - env, - ed, - 1, - {{Amounts{ - STAmount{ETH, UINT64_C(335'0889359326485), -13}, - STAmount{USD, UINT64_C(209'4305849579053), -13}, - }}})); - BEAST_EXPECT(expectOffers(env, alice, 0)); + if (!features[fixAMMv1_1]) + { + // Ed offer is partially crossed. + BEAST_EXPECT(expectOffers( + env, + ed, + 1, + {{Amounts{ + STAmount{ + ETH, UINT64_C(335'0889359326485), -13}, + STAmount{ + USD, UINT64_C(209'4305849579053), -13}, + }}})); + BEAST_EXPECT(expectOffers(env, alice, 0)); + } + else + { + // Ed offer is partially crossed. + BEAST_EXPECT(expectOffers( + env, + ed, + 1, + {{Amounts{ + STAmount{ + ETH, UINT64_C(335'0889359326475), -13}, + STAmount{ + USD, UINT64_C(209'4305849579047), -13}, + }}})); + BEAST_EXPECT(expectOffers(env, alice, 0)); + } } } } @@ -5361,7 +5455,7 @@ private: BEAST_EXPECT(expectLine(env, bob, USD(2'100))); - if (i == 2 && !features[fixAMMRounding]) + if (i == 2 && !features[fixAMMv1_1]) { if (rates.first == 1.5) { @@ -5557,6 +5651,222 @@ private: false); } + void + testFixChangeSpotPriceQuality(FeatureBitset features) + { + testcase("Fix changeSpotPriceQuality"); + using namespace jtx; + + enum class Status { + SucceedShouldSucceedResize, // Succeed in pre-fix because + // error allowance, succeed post-fix + // because of offer resizing + FailShouldSucceed, // Fail in pre-fix due to rounding, + // succeed after fix because of XRP + // side is generated first + SucceedShouldFail, // Succeed in pre-fix, fail after fix + // due to small quality difference + Fail, // Both fail because the quality can't be matched + Succeed // Both succeed + }; + using enum Status; + auto const xrpIouAmounts10_100 = + TAmounts{XRPAmount{10}, IOUAmount{100}}; + auto const iouXrpAmounts10_100 = + TAmounts{IOUAmount{10}, XRPAmount{100}}; + // clang-format off + std::vector> tests = { + //Pool In , Pool Out, Quality , Fee, Status + {"0.001519763260828713", "1558701", Quality{5414253689393440221}, 1000, FailShouldSucceed}, + {"0.01099814367603737", "1892611", Quality{5482264816516900274}, 1000, FailShouldSucceed}, + {"0.78", "796599", Quality{5630392334958379008}, 1000, FailShouldSucceed}, + {"105439.2955578965", "49398693", Quality{5910869983721805038}, 400, FailShouldSucceed}, + {"12408293.23445213", "4340810521", Quality{5911611095910090752}, 997, FailShouldSucceed}, + {"1892611", "0.01099814367603737", Quality{6703103457950430139}, 1000, FailShouldSucceed}, + {"423028.8508101858", "3392804520", Quality{5837920340654162816}, 600, FailShouldSucceed}, + {"44565388.41001027", "73890647", Quality{6058976634606450001}, 1000, FailShouldSucceed}, + {"66831.68494832662", "16", Quality{6346111134641742975}, 0, FailShouldSucceed}, + {"675.9287302203422", "1242632304", Quality{5625960929244093294}, 300, FailShouldSucceed}, + {"7047.112186735699", "1649845866", Quality{5696855348026306945}, 504, FailShouldSucceed}, + {"840236.4402981238", "47419053", Quality{5982561601648018688}, 499, FailShouldSucceed}, + {"992715.618909774", "189445631733", Quality{5697835648288106944}, 815, SucceedShouldSucceedResize}, + {"504636667521", "185545883.9506651", Quality{6343802275337659280}, 503, SucceedShouldSucceedResize}, + {"992706.7218636649", "189447316000", Quality{5697835648288106944}, 797, SucceedShouldSucceedResize}, + {"1.068737911388205", "127860278877", Quality{5268604356368739396}, 293, SucceedShouldSucceedResize}, + {"17932506.56880419", "189308.6043676173", Quality{6206460598195440068}, 311, SucceedShouldSucceedResize}, + {"1.066379294658174", "128042251493", Quality{5268559341368739328}, 270, SucceedShouldSucceedResize}, + {"350131413924", "1576879.110907892", Quality{6487411636539049449}, 650, Fail}, + {"422093460", "2.731797662057464", Quality{6702911108534394924}, 1000, Fail}, + {"76128132223", "367172.7148422662", Quality{6487263463413514240}, 548, Fail}, + {"132701839250", "280703770.7695443", Quality{6273750681188885075}, 562, Fail}, + {"994165.7604612011", "189551302411", Quality{5697835592690668727}, 815, Fail}, + {"45053.33303227917", "86612695359", Quality{5625695218943638190}, 500, Fail}, + {"199649.077043865", "14017933007", Quality{5766034667318524880}, 324, Fail}, + {"27751824831.70903", "78896950", Quality{6272538159621630432}, 500, Fail}, + {"225.3731275781907", "156431793648", Quality{5477818047604078924}, 989, Fail}, + {"199649.077043865", "14017933007", Quality{5766036094462806309}, 324, Fail}, + {"3.590272027140361", "20677643641", Quality{5406056147042156356}, 808, Fail}, + {"1.070884664490231", "127604712776", Quality{5268620608623825741}, 293, Fail}, + {"3272.448829820197", "6275124076", Quality{5625710328924117902}, 81, Fail}, + {"0.009059512633902926", "7994028", Quality{5477511954775533172}, 1000, Fail}, + {"1", "1.0", Quality{0}, 100, Fail}, + {"1.0", "1", Quality{0}, 100, Fail}, + {"10", "10.0", Quality{xrpIouAmounts10_100}, 100, Fail}, + {"10.0", "10", Quality{iouXrpAmounts10_100}, 100, Fail}, + {"69864389131", "287631.4543025075", Quality{6487623473313516078}, 451, Succeed}, + {"4328342973", "12453825.99247381", Quality{6272522264364865181}, 997, Succeed}, + {"32347017", "7003.93031579449", Quality{6347261126087916670}, 1000, Succeed}, + {"61697206161", "36631.4583206413", Quality{6558965195382476659}, 500, Succeed}, + {"1654524979", "7028.659825511603", Quality{6487551345110052981}, 504, Succeed}, + {"88621.22277293179", "5128418948", Quality{5766347291552869205}, 380, Succeed}, + {"1892611", "0.01099814367603737", Quality{6703102780512015436}, 1000, Succeed}, + {"4542.639373338766", "24554809", Quality{5838994982188783710}, 0, Succeed}, + {"5132932546", "88542.99750172683", Quality{6419203342950054537}, 380, Succeed}, + {"78929964.1549083", "1506494795", Quality{5986890029845558688}, 589, Succeed}, + {"10096561906", "44727.72453735605", Quality{6487455290284644551}, 250, Succeed}, + {"5092.219565514988", "8768257694", Quality{5626349534958379008}, 503, Succeed}, + {"1819778294", "8305.084302902864", Quality{6487429398998540860}, 415, Succeed}, + {"6970462.633911943", "57359281", Quality{6054087899185946624}, 850, Succeed}, + {"3983448845", "2347.543644281467", Quality{6558965195382476659}, 856, Succeed}, + // This is a tiny offer 12drops/19321952e-15 it succeeds pre-amendment because of the error allowance. + // Post amendment it is resized to 11drops/17711789e-15 but the quality is still less than + // the target quality and the offer fails. + {"771493171", "1.243473020567508", Quality{6707566798038544272}, 100, SucceedShouldFail}, + }; + // clang-format on + + boost::regex rx("^\\d+$"); + boost::smatch match; + // tests that succeed should have the same amounts pre-fix and post-fix + std::vector> successAmounts; + Env env(*this, features); + auto rules = env.current()->rules(); + CurrentTransactionRulesGuard rg(rules); + for (auto const& t : tests) + { + auto getPool = [&](std::string const& v, bool isXRP) { + if (isXRP) + return amountFromString(xrpIssue(), v); + return amountFromString(noIssue(), v); + }; + auto const& quality = std::get(t); + auto const tfee = std::get(t); + auto const status = std::get(t); + auto const poolInIsXRP = + boost::regex_search(std::get<0>(t), match, rx); + auto const poolOutIsXRP = + boost::regex_search(std::get<1>(t), match, rx); + assert(!(poolInIsXRP && poolOutIsXRP)); + auto const poolIn = getPool(std::get<0>(t), poolInIsXRP); + auto const poolOut = getPool(std::get<1>(t), poolOutIsXRP); + try + { + auto const amounts = changeSpotPriceQuality( + Amounts{poolIn, poolOut}, + quality, + tfee, + env.current()->rules(), + env.journal); + if (amounts) + { + if (status == SucceedShouldSucceedResize) + { + if (!features[fixAMMv1_1]) + BEAST_EXPECT(Quality{*amounts} < quality); + else + BEAST_EXPECT(Quality{*amounts} >= quality); + } + else if (status == Succeed) + { + if (!features[fixAMMv1_1]) + BEAST_EXPECT( + Quality{*amounts} >= quality || + withinRelativeDistance( + Quality{*amounts}, quality, Number{1, -7})); + else + BEAST_EXPECT(Quality{*amounts} >= quality); + } + else if (status == FailShouldSucceed) + { + BEAST_EXPECT( + features[fixAMMv1_1] && + Quality{*amounts} >= quality); + } + else if (status == SucceedShouldFail) + { + BEAST_EXPECT( + !features[fixAMMv1_1] && + Quality{*amounts} < quality && + withinRelativeDistance( + Quality{*amounts}, quality, Number{1, -7})); + } + } + else + { + // Fails pre- and post-amendment because the quality can't + // be matched. Verify by generating a tiny offer, which + // doesn't match the quality. Exclude zero quality since + // no offer is generated in this case. + if (status == Fail && quality != Quality{0}) + { + auto tinyOffer = [&]() { + if (isXRP(poolIn)) + { + auto const takerPays = STAmount{xrpIssue(), 1}; + return Amounts{ + takerPays, + swapAssetIn( + Amounts{poolIn, poolOut}, + takerPays, + tfee)}; + } + else if (isXRP(poolOut)) + { + auto const takerGets = STAmount{xrpIssue(), 1}; + return Amounts{ + swapAssetOut( + Amounts{poolIn, poolOut}, + takerGets, + tfee), + takerGets}; + } + auto const takerPays = toAmount( + getIssue(poolIn), Number{1, -10} * poolIn); + return Amounts{ + takerPays, + swapAssetIn( + Amounts{poolIn, poolOut}, takerPays, tfee)}; + }(); + BEAST_EXPECT(Quality(tinyOffer) < quality); + } + else if (status == FailShouldSucceed) + { + BEAST_EXPECT(!features[fixAMMv1_1]); + } + else if (status == SucceedShouldFail) + { + BEAST_EXPECT(features[fixAMMv1_1]); + } + } + } + catch (std::runtime_error const& e) + { + BEAST_EXPECT( + !strcmp(e.what(), "changeSpotPriceQuality failed")); + BEAST_EXPECT( + !features[fixAMMv1_1] && status == FailShouldSucceed); + } + } + + // Test negative discriminant + { + // b**2 - 4 * a * c -> 1 * 1 - 4 * 1 * 1 = -3 + auto const res = + solveQuadraticEqSmallest(Number{1}, Number{1}, Number{1}); + BEAST_EXPECT(!res.has_value()); + } + } + void testMalformed() { @@ -5895,18 +6205,14 @@ private: txflags(tfPartialPayment)); env.close(); - auto const failUsdGH = features[fixAMMRounding] - ? input.failUsdGHr - : input.failUsdGH; - auto const failUsdBIT = features[fixAMMRounding] - ? input.failUsdBITr - : input.failUsdBIT; - auto const goodUsdGH = features[fixAMMRounding] - ? input.goodUsdGHr - : input.goodUsdGH; - auto const goodUsdBIT = features[fixAMMRounding] - ? input.goodUsdBITr - : input.goodUsdBIT; + auto const failUsdGH = + features[fixAMMv1_1] ? input.failUsdGHr : input.failUsdGH; + auto const failUsdBIT = + features[fixAMMv1_1] ? input.failUsdBITr : input.failUsdBIT; + auto const goodUsdGH = + features[fixAMMv1_1] ? input.goodUsdGHr : input.goodUsdGH; + auto const goodUsdBIT = + features[fixAMMv1_1] ? input.goodUsdBITr : input.goodUsdBIT; if (!features[fixAMMOverflowOffer]) { BEAST_EXPECT(amm.expectBalances( @@ -5977,7 +6283,135 @@ private: {{xrpPool, iouPool}}, 889, std::nullopt, - {jtx::supported_amendments() | fixAMMRounding}); + {jtx::supported_amendments() | fixAMMv1_1}); + } + + void + testFixAMMOfferBlockedByLOB(FeatureBitset features) + { + testcase("AMM Offer Blocked By LOB"); + using namespace jtx; + + // Low quality LOB offer blocks AMM liquidity + + // USD/XRP crosses AMM + { + Env env(*this, features); + + fund(env, gw, {alice, carol}, XRP(1'000'000), {USD(1'000'000)}); + // This offer blocks AMM offer in pre-amendment + env(offer(alice, XRP(1), USD(0.01))); + env.close(); + + AMM amm(env, gw, XRP(200'000), USD(100'000)); + + // The offer doesn't cross AMM in pre-amendment code + // It crosses AMM in post-amendment code + env(offer(carol, USD(0.49), XRP(1))); + env.close(); + + if (!features[fixAMMv1_1]) + { + BEAST_EXPECT(amm.expectBalances( + XRP(200'000), USD(100'000), amm.tokens())); + BEAST_EXPECT(expectOffers( + env, alice, 1, {{Amounts{XRP(1), USD(0.01)}}})); + // Carol's offer is blocked by alice's offer + BEAST_EXPECT(expectOffers( + env, carol, 1, {{Amounts{USD(0.49), XRP(1)}}})); + } + else + { + BEAST_EXPECT(amm.expectBalances( + XRPAmount(200'000'980'005), USD(99'999.51), amm.tokens())); + BEAST_EXPECT(expectOffers( + env, alice, 1, {{Amounts{XRP(1), USD(0.01)}}})); + // Carol's offer crosses AMM + BEAST_EXPECT(expectOffers(env, carol, 0)); + } + } + + // There is no blocking offer, the same AMM liquidity is consumed + // pre- and post-amendment. + { + Env env(*this, features); + + fund(env, gw, {alice, carol}, XRP(1'000'000), {USD(1'000'000)}); + // There is no blocking offer + // env(offer(alice, XRP(1), USD(0.01))); + + AMM amm(env, gw, XRP(200'000), USD(100'000)); + + // The offer crosses AMM + env(offer(carol, USD(0.49), XRP(1))); + env.close(); + + // The same result as with the blocking offer + BEAST_EXPECT(amm.expectBalances( + XRPAmount(200'000'980'005), USD(99'999.51), amm.tokens())); + // Carol's offer crosses AMM + BEAST_EXPECT(expectOffers(env, carol, 0)); + } + + // XRP/USD crosses AMM + { + Env env(*this, features); + fund(env, gw, {alice, carol, bob}, XRP(10'000), {USD(1'000)}); + + // This offer blocks AMM offer in pre-amendment + // It crosses AMM in post-amendment code + env(offer(bob, USD(1), XRPAmount(500))); + env.close(); + AMM amm(env, alice, XRP(1'000), USD(500)); + env(offer(carol, XRP(100), USD(55))); + env.close(); + if (!features[fixAMMv1_1]) + { + BEAST_EXPECT( + amm.expectBalances(XRP(1'000), USD(500), amm.tokens())); + BEAST_EXPECT(expectOffers( + env, bob, 1, {{Amounts{USD(1), XRPAmount(500)}}})); + BEAST_EXPECT(expectOffers( + env, carol, 1, {{Amounts{XRP(100), USD(55)}}})); + } + else + { + BEAST_EXPECT(amm.expectBalances( + XRPAmount(909'090'909), + STAmount{USD, UINT64_C(550'000000055), -9}, + amm.tokens())); + BEAST_EXPECT(expectOffers( + env, + carol, + 1, + {{Amounts{ + XRPAmount{9'090'909}, + STAmount{USD, 4'99999995, -8}}}})); + BEAST_EXPECT(expectOffers( + env, bob, 1, {{Amounts{USD(1), XRPAmount(500)}}})); + } + } + + // There is no blocking offer, the same AMM liquidity is consumed + // pre- and post-amendment. + { + Env env(*this, features); + fund(env, gw, {alice, carol, bob}, XRP(10'000), {USD(1'000)}); + + AMM amm(env, alice, XRP(1'000), USD(500)); + env(offer(carol, XRP(100), USD(55))); + env.close(); + BEAST_EXPECT(amm.expectBalances( + XRPAmount(909'090'909), + STAmount{USD, UINT64_C(550'000000055), -9}, + amm.tokens())); + BEAST_EXPECT(expectOffers( + env, + carol, + 1, + {{Amounts{ + XRPAmount{9'090'909}, STAmount{USD, 4'99999995, -8}}}})); + } } void @@ -5994,29 +6428,33 @@ private: testFeeVote(); testInvalidBid(); testBid(all); - testBid(all - fixAMMRounding); + testBid(all - fixAMMv1_1); testInvalidAMMPayment(); testBasicPaymentEngine(all); - testBasicPaymentEngine(all - fixAMMRounding); + testBasicPaymentEngine(all - fixAMMv1_1); testAMMTokens(); testAmendment(); testFlags(); testRippling(); testAMMAndCLOB(all); - testAMMAndCLOB(all - fixAMMRounding); + testAMMAndCLOB(all - fixAMMv1_1); testTradingFee(all); - testTradingFee(all - fixAMMRounding); + testTradingFee(all - fixAMMv1_1); testAdjustedTokens(); testAutoDelete(); testClawback(); testAMMID(); testSelection(all); - testSelection(all - fixAMMRounding); + testSelection(all - fixAMMv1_1); testFixDefaultInnerObj(); testMalformed(); testFixOverflowOffer(all); - testFixOverflowOffer(all - fixAMMRounding); + testFixOverflowOffer(all - fixAMMv1_1); testSwapRounding(); + testFixChangeSpotPriceQuality(all); + testFixChangeSpotPriceQuality(all - fixAMMv1_1); + testFixAMMOfferBlockedByLOB(all); + testFixAMMOfferBlockedByLOB(all - fixAMMv1_1); } };