diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 5288e12aed2..7c924209ef3 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -6,6 +6,27 @@ This document contains the release notes for `rippled`, the reference server imp Have new ideas? Need help with setting up your node? [Please open an issue here](https://github.com/xrplf/rippled/issues/new/choose). +## Version 2.1.1 + +The `rippled` 2.1.1 release fixes a critical bug in the integration of AMMs with the payment engine. + +[Sign Up for Future Release Announcements](https://groups.google.com/g/ripple-server) + + + + +## Action Required + +One new amendment is now open for voting according to the XRP Ledger's [amendment process](https://xrpl.org/amendments.html), which enables protocol changes following two weeks of >80% support from trusted validators. + +If you operate an XRP Ledger server, upgrade to version 2.1.1 by April 8, 2024 to ensure service continuity. The exact time that protocol changes take effect depends on the voting decisions of the decentralized network. + +## Changelog + +### Amendments + +- **fixAMMOverflowOffer**: Fix improper handling of large synthetic AMM offers in the payment engine. Due to the importance of this fix, the default vote in the source code has been set to YES. For information on how to configure your validator's amendment voting, see [Configure Amendment Voting](https://xrpl.org/docs/infrastructure/configuration/configure-amendment-voting). + # Introducing XRP Ledger version 2.1.0 Version 2.1.0 of `rippled`, the reference server implementation of the XRP Ledger protocol, is now available. This release adds a bug fix, build improvements, and introduces the `fixNFTokenReserve` and `fixInnerObjTemplate` amendments. diff --git a/src/ripple/app/misc/AMMHelpers.h b/src/ripple/app/misc/AMMHelpers.h index 24c25922800..e84c6c535ea 100644 --- a/src/ripple/app/misc/AMMHelpers.h +++ b/src/ripple/app/misc/AMMHelpers.h @@ -134,7 +134,7 @@ withinRelativeDistance( template requires( std::is_same_v || std::is_same_v || - std::is_same_v) + std::is_same_v || std::is_same_v) bool withinRelativeDistance(Amt const& calc, Amt const& req, Number const& dist) { diff --git a/src/ripple/app/paths/AMMLiquidity.h b/src/ripple/app/paths/AMMLiquidity.h index 7757bd4684d..f1be112b2d6 100644 --- a/src/ripple/app/paths/AMMLiquidity.h +++ b/src/ripple/app/paths/AMMLiquidity.h @@ -137,10 +137,17 @@ class AMMLiquidity TAmounts generateFibSeqOffer(TAmounts const& balances) const; - /** Generate max offer + /** Generate max offer. + * If `fixAMMOverflowOffer` is active, the offer is generated as: + * takerGets = 99% * balances.out takerPays = swapOut(takerGets). + * Return nullopt if takerGets is 0 or takerGets == balances.out. + * + * If `fixAMMOverflowOffer` is not active, the offer is generated as: + * takerPays = max input amount; + * takerGets = swapIn(takerPays). */ - AMMOffer - maxOffer(TAmounts const& balances) const; + std::optional> + maxOffer(TAmounts const& balances, Rules const& rules) const; }; } // namespace ripple diff --git a/src/ripple/app/paths/AMMOffer.h b/src/ripple/app/paths/AMMOffer.h index 10e6017dd96..426ba96a772 100644 --- a/src/ripple/app/paths/AMMOffer.h +++ b/src/ripple/app/paths/AMMOffer.h @@ -50,9 +50,8 @@ class AMMOffer // the swap out of the entire side of the pool, in which case // the swap in amount is infinite. TAmounts const amounts_; - // If seated then current pool balances. Used in one-path limiting steps - // to swap in/out. - std::optional> const balances_; + // Current pool balances. + TAmounts const balances_; // The Spot Price quality if balances != amounts // else the amounts quality Quality const quality_; @@ -63,7 +62,7 @@ class AMMOffer AMMOffer( AMMLiquidity const& ammLiquidity, TAmounts const& amounts, - std::optional> const& balances, + TAmounts const& balances, Quality const& quality); Quality @@ -142,6 +141,12 @@ class AMMOffer // AMM doesn't pay transfer fee on Payment tx return {ofrInRate, QUALITY_ONE}; } + + /** Check the new pool product is greater or equal to the old pool + * product or if decreases then within some threshold. + */ + bool + checkInvariant(TAmounts const& consumed, beast::Journal j) const; }; } // namespace ripple diff --git a/src/ripple/app/paths/impl/AMMLiquidity.cpp b/src/ripple/app/paths/impl/AMMLiquidity.cpp index 3f22ebacec5..bcc086e23da 100644 --- a/src/ripple/app/paths/impl/AMMLiquidity.cpp +++ b/src/ripple/app/paths/impl/AMMLiquidity.cpp @@ -93,6 +93,7 @@ AMMLiquidity::generateFibSeqOffer( return cur; } +namespace { template constexpr T maxAmount() @@ -105,16 +106,41 @@ maxAmount() return STAmount(STAmount::cMaxValue / 2, STAmount::cMaxOffset); } +template +T +maxOut(T const& out, Issue const& iss) +{ + Number const res = out * Number{99, -2}; + return toAmount(iss, res, Number::rounding_mode::downward); +} +} // namespace + template -AMMOffer -AMMLiquidity::maxOffer(TAmounts const& balances) const +std::optional> +AMMLiquidity::maxOffer( + TAmounts const& balances, + Rules const& rules) const { - return AMMOffer( - *this, - {maxAmount(), - swapAssetIn(balances, maxAmount(), tradingFee_)}, - balances, - Quality{balances}); + if (!rules.enabled(fixAMMOverflowOffer)) + { + return AMMOffer( + *this, + {maxAmount(), + swapAssetIn(balances, maxAmount(), tradingFee_)}, + balances, + Quality{balances}); + } + else + { + auto const out = maxOut(balances.out, issueOut()); + if (out <= TOut{0} || out >= balances.out) + return std::nullopt; + return AMMOffer( + *this, + {swapAssetOut(balances, out, tradingFee_), out}, + balances, + Quality{balances}); + } } template @@ -167,15 +193,16 @@ AMMLiquidity::getOffer( if (clobQuality && Quality{amounts} < clobQuality) return std::nullopt; return AMMOffer( - *this, amounts, std::nullopt, Quality{amounts}); + *this, amounts, balances, Quality{amounts}); } else if (!clobQuality) { // If there is no CLOB to compare against, return the largest // amount, which doesn't overflow. The size is going to be // changed in BookStep per either deliver amount limit, or - // sendmax, or available output or input funds. - return maxOffer(balances); + // sendmax, or available output or input funds. Might return + // nullopt if the pool is small. + return maxOffer(balances, view.rules()); } else if ( auto const amounts = @@ -188,7 +215,10 @@ AMMLiquidity::getOffer( catch (std::overflow_error const& e) { JLOG(j_.error()) << "AMMLiquidity::getOffer overflow " << e.what(); - return maxOffer(balances); + if (!view.rules().enabled(fixAMMOverflowOffer)) + return maxOffer(balances, view.rules()); + else + return std::nullopt; } catch (std::exception const& e) { diff --git a/src/ripple/app/paths/impl/AMMOffer.cpp b/src/ripple/app/paths/impl/AMMOffer.cpp index 10b75b78565..759851b7afe 100644 --- a/src/ripple/app/paths/impl/AMMOffer.cpp +++ b/src/ripple/app/paths/impl/AMMOffer.cpp @@ -27,7 +27,7 @@ template AMMOffer::AMMOffer( AMMLiquidity const& ammLiquidity, TAmounts const& amounts, - std::optional> const& balances, + TAmounts const& balances, Quality const& quality) : ammLiquidity_(ammLiquidity) , amounts_(amounts) @@ -110,7 +110,7 @@ AMMOffer::limitOut( // Change the offer size according to the conservation function. The offer // quality is increased in this case, but it doesn't matter since there is // only one path. - return {swapAssetOut(*balances_, limit, ammLiquidity_.tradingFee()), limit}; + return {swapAssetOut(balances_, limit, ammLiquidity_.tradingFee()), limit}; } template @@ -122,7 +122,7 @@ AMMOffer::limitIn( // See the comments above in limitOut(). if (ammLiquidity_.multiPath()) return quality().ceil_in(offrAmt, limit); - return {limit, swapAssetIn(*balances_, limit, ammLiquidity_.tradingFee())}; + return {limit, swapAssetIn(balances_, limit, ammLiquidity_.tradingFee())}; } template @@ -132,7 +132,45 @@ AMMOffer::getQualityFunc() const if (ammLiquidity_.multiPath()) return QualityFunction{quality(), QualityFunction::CLOBLikeTag{}}; return QualityFunction{ - *balances_, ammLiquidity_.tradingFee(), QualityFunction::AMMTag{}}; + balances_, ammLiquidity_.tradingFee(), QualityFunction::AMMTag{}}; +} + +template +bool +AMMOffer::checkInvariant( + TAmounts const& consumed, + beast::Journal j) const +{ + if (consumed.in > amounts_.in || consumed.out > amounts_.out) + { + JLOG(j.error()) << "AMMOffer::checkInvariant failed: consumed " + << to_string(consumed.in) << " " + << to_string(consumed.out) << " amounts " + << to_string(amounts_.in) << " " + << to_string(amounts_.out); + + return false; + } + + Number const product = balances_.in * balances_.out; + auto const newBalances = TAmounts{ + balances_.in + consumed.in, balances_.out - consumed.out}; + Number const newProduct = newBalances.in * newBalances.out; + + if (newProduct >= product || + withinRelativeDistance(product, newProduct, Number{1, -7})) + return true; + + JLOG(j.error()) << "AMMOffer::checkInvariant failed: balances " + << to_string(balances_.in) << " " + << to_string(balances_.out) << " new balances " + << to_string(newBalances.in) << " " + << to_string(newBalances.out) << " product/newProduct " + << product << " " << newProduct << " diff " + << (product != Number{0} + ? to_string((product - newProduct) / product) + : "undefined"); + return false; } template class AMMOffer; diff --git a/src/ripple/app/paths/impl/BookStep.cpp b/src/ripple/app/paths/impl/BookStep.cpp index dd6abe577f5..358dac4c796 100644 --- a/src/ripple/app/paths/impl/BookStep.cpp +++ b/src/ripple/app/paths/impl/BookStep.cpp @@ -793,6 +793,17 @@ BookStep::consumeOffer( TAmounts const& stepAmt, TOut const& ownerGives) const { + if (!offer.checkInvariant(ofrAmt, j_)) + { + // purposely written as separate if statements so we get logging even + // when the amendment isn't active. + if (sb.rules().enabled(fixAMMOverflowOffer)) + { + Throw( + tecINVARIANT_FAILED, "AMM pool product invariant failed."); + } + } + // The offer owner gets the ofrAmt. The difference between ofrAmt and // stepAmt is a transfer fee that goes to book_.in.account { diff --git a/src/ripple/app/tx/impl/Offer.h b/src/ripple/app/tx/impl/Offer.h index 027030ec8d0..bdae4d2b155 100644 --- a/src/ripple/app/tx/impl/Offer.h +++ b/src/ripple/app/tx/impl/Offer.h @@ -163,6 +163,15 @@ class TOffer : private TOfferBase // CLOB offer pays the transfer fee return {ofrInRate, ofrOutRate}; } + + /** Check any required invariant. Limit order book offer + * always returns true. + */ + bool + checkInvariant(TAmounts const&, beast::Journal j) const + { + return true; + } }; using Offer = TOffer<>; diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index ffe8d0adc6b..d00dd1555f2 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -74,7 +74,7 @@ namespace detail { // Feature.cpp. Because it's only used to reserve storage, and determine how // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than // the actual number of amendments. A LogicError on startup will verify this. -static constexpr std::size_t numFeatures = 70; +static constexpr std::size_t numFeatures = 71; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -354,6 +354,7 @@ extern uint256 const featureDID; extern uint256 const fixFillOrKill; extern uint256 const fixNFTokenReserve; extern uint256 const fixInnerObjTemplate; +extern uint256 const fixAMMOverflowOffer; extern uint256 const featurePriceOracle; extern uint256 const fixEmptyDID; extern uint256 const fixXChainRewardRounding; diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index f7a8018c393..8bd4b7aea27 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -461,6 +461,7 @@ REGISTER_FEATURE(DID, Supported::yes, VoteBehavior::De REGISTER_FIX (fixFillOrKill, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixNFTokenReserve, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixInnerObjTemplate, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FIX (fixAMMOverflowOffer, Supported::yes, VoteBehavior::DefaultYes); REGISTER_FEATURE(PriceOracle, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixEmptyDID, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixXChainRewardRounding, Supported::yes, VoteBehavior::DefaultNo);