diff --git a/include/xrpl/protocol/Feature.h b/include/xrpl/protocol/Feature.h index a2510c63000..d8353f50a44 100644 --- a/include/xrpl/protocol/Feature.h +++ b/include/xrpl/protocol/Feature.h @@ -80,7 +80,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 = 81; +static constexpr std::size_t numFeatures = 82; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index e5351be11c0..c0d2706cc1a 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -29,6 +29,7 @@ // If you add an amendment here, then do not forget to increment `numFeatures` // in include/xrpl/protocol/Feature.h. +XRPL_FIX (AMMv1_2, Supported::yes, VoteBehavior::DefaultNo) // InvariantsV1_1 will be changes to Supported::yes when all the // invariants expected to be included under it are complete. XRPL_FEATURE(MPTokensV1, Supported::yes, VoteBehavior::DefaultNo) diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 8e764390e9a..f1e81132c5e 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -7064,6 +7064,55 @@ struct AMM_test : public jtx::AMMTest } } + void + testFixReserveCheckOnWithdrawal(FeatureBitset features) + { + testcase("Fix Reserve Check On Withdrawal"); + using namespace jtx; + + auto const err = features[fixAMMv1_2] ? ter(tecINSUFFICIENT_RESERVE) + : ter(tesSUCCESS); + + auto test = [&](auto&& cb) { + Env env(*this, features); + auto const starting_xrp = + reserve(env, 2) + env.current()->fees().base * 5; + env.fund(starting_xrp, gw); + env.fund(starting_xrp, alice); + env.trust(USD(2'000), alice); + env.close(); + env(pay(gw, alice, USD(2'000))); + env.close(); + AMM amm(env, gw, EUR(1'000), USD(1'000)); + amm.deposit(alice, USD(1)); + cb(amm); + }; + + // Equal withdraw + test([&](AMM& amm) { amm.withdrawAll(alice, std::nullopt, err); }); + + // Equal withdraw with a limit + test([&](AMM& amm) { + amm.withdraw(WithdrawArg{ + .account = alice, + .asset1Out = EUR(0.1), + .asset2Out = USD(0.1), + .err = err}); + amm.withdraw(WithdrawArg{ + .account = alice, + .asset1Out = USD(0.1), + .asset2Out = EUR(0.1), + .err = err}); + }); + + // Single withdraw + test([&](AMM& amm) { + amm.withdraw(WithdrawArg{ + .account = alice, .asset1Out = EUR(0.1), .err = err}); + amm.withdraw(WithdrawArg{.account = alice, .asset1Out = USD(0.1)}); + }); + } + void run() override { @@ -7117,6 +7166,8 @@ struct AMM_test : public jtx::AMMTest testAMMDepositWithFrozenAssets(all); testAMMDepositWithFrozenAssets(all - featureAMMClawback); testAMMDepositWithFrozenAssets(all - fixAMMv1_1 - featureAMMClawback); + testFixReserveCheckOnWithdrawal(all); + testFixReserveCheckOnWithdrawal(all - fixAMMv1_2); } }; diff --git a/src/xrpld/app/paths/detail/AMMLiquidity.cpp b/src/xrpld/app/paths/detail/AMMLiquidity.cpp index 8215cdee593..7b1649c649e 100644 --- a/src/xrpld/app/paths/detail/AMMLiquidity.cpp +++ b/src/xrpld/app/paths/detail/AMMLiquidity.cpp @@ -211,6 +211,13 @@ AMMLiquidity::getOffer( return AMMOffer( *this, *amounts, balances, Quality{*amounts}); } + else if (view.rules().enabled(fixAMMv1_2)) + { + if (auto const maxAMMOffer = maxOffer(balances, view.rules()); + maxAMMOffer && + Quality{maxAMMOffer->amount()} > *clobQuality) + return maxAMMOffer; + } } catch (std::overflow_error const& e) { diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index 468a5a4c6a2..64150ded6da 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -188,6 +188,7 @@ AMMClawback::applyGuts(Sandbox& sb) 0, FreezeHandling::fhIGNORE_FREEZE, WithdrawAll::Yes, + mPriorBalance, ctx_.journal); else std::tie(result, newLPTokenBalance, amountWithdraw, amount2Withdraw) = @@ -267,6 +268,7 @@ AMMClawback::equalWithdrawMatchingOneAmount( 0, FreezeHandling::fhIGNORE_FREEZE, WithdrawAll::Yes, + mPriorBalance, ctx_.journal); // Because we are doing a two-asset withdrawal, @@ -284,6 +286,7 @@ AMMClawback::equalWithdrawMatchingOneAmount( 0, FreezeHandling::fhIGNORE_FREEZE, WithdrawAll::No, + mPriorBalance, ctx_.journal); } diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp index 0a6f3291b78..118262905c1 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.cpp +++ b/src/xrpld/app/tx/detail/AMMWithdraw.cpp @@ -472,6 +472,7 @@ AMMWithdraw::withdraw( tfee, FreezeHandling::fhZERO_IF_FROZEN, isWithdrawAll(ctx_.tx), + mPriorBalance, j_); return {ter, newLPTokenBalance}; } @@ -490,6 +491,7 @@ AMMWithdraw::withdraw( std::uint16_t tfee, FreezeHandling freezeHandling, WithdrawAll withdrawAll, + XRPAmount const& priorBalance, beast::Journal const& journal) { auto const lpTokens = ammLPHolds(view, ammSle, account, journal); @@ -589,6 +591,33 @@ AMMWithdraw::withdraw( return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; } + // Check the reserve in case a trustline has to be created + bool const enabledFixAMMv1_2 = view.rules().enabled(fixAMMv1_2); + auto sufficientReserve = [&](Issue const& issue) -> TER { + if (!enabledFixAMMv1_2 || isXRP(issue)) + return tesSUCCESS; + if (!view.exists(keylet::line(account, issue))) + { + auto const sleAccount = view.read(keylet::account(account)); + if (!sleAccount) + return tecINTERNAL; // LCOV_EXCL_LINE + auto const balance = (*sleAccount)[sfBalance].xrp(); + std::uint32_t const ownerCount = sleAccount->at(sfOwnerCount); + + // See also SetTrust::doApply() + XRPAmount const reserve( + (ownerCount < 2) ? XRPAmount(beast::zero) + : view.fees().accountReserve(ownerCount + 1)); + + if (std::max(priorBalance, balance) < reserve) + return tecINSUFFICIENT_RESERVE; + } + return tesSUCCESS; + }; + + if (auto const err = sufficientReserve(amountWithdrawActual.issue())) + return {err, STAmount{}, STAmount{}, STAmount{}}; + // Withdraw amountWithdraw auto res = accountSend( view, @@ -609,6 +638,10 @@ AMMWithdraw::withdraw( // Withdraw amount2Withdraw if (amount2WithdrawActual) { + if (auto const err = sufficientReserve(amount2WithdrawActual->issue()); + err != tesSUCCESS) + return {err, STAmount{}, STAmount{}, STAmount{}}; + res = accountSend( view, ammAccount, @@ -676,6 +709,7 @@ AMMWithdraw::equalWithdrawTokens( tfee, FreezeHandling::fhZERO_IF_FROZEN, isWithdrawAll(ctx_.tx), + mPriorBalance, ctx_.journal); return {ter, newLPTokenBalance}; } @@ -725,6 +759,7 @@ AMMWithdraw::equalWithdrawTokens( std::uint16_t tfee, FreezeHandling freezeHanding, WithdrawAll withdrawAll, + XRPAmount const& priorBalance, beast::Journal const& journal) { try @@ -745,6 +780,7 @@ AMMWithdraw::equalWithdrawTokens( tfee, freezeHanding, WithdrawAll::Yes, + priorBalance, journal); } @@ -773,6 +809,7 @@ AMMWithdraw::equalWithdrawTokens( tfee, freezeHanding, withdrawAll, + priorBalance, journal); } // LCOV_EXCL_START diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.h b/src/xrpld/app/tx/detail/AMMWithdraw.h index f5b6b52e5ba..ae9328cb05e 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.h +++ b/src/xrpld/app/tx/detail/AMMWithdraw.h @@ -96,6 +96,7 @@ class AMMWithdraw : public Transactor * @param lpTokensWithdraw amount of tokens to withdraw * @param tfee trading fee in basis points * @param withdrawAll if withdrawing all lptokens + * @param priorBalance balance before fees * @return */ static std::tuple> @@ -112,6 +113,7 @@ class AMMWithdraw : public Transactor std::uint16_t tfee, FreezeHandling freezeHanding, WithdrawAll withdrawAll, + XRPAmount const& priorBalance, beast::Journal const& journal); /** Withdraw requested assets and token from AMM into LP account. @@ -127,6 +129,7 @@ class AMMWithdraw : public Transactor * @param lpTokensWithdraw amount of lptokens to withdraw * @param tfee trading fee in basis points * @param withdrawAll if withdraw all lptokens + * @param priorBalance balance before fees * @return */ static std::tuple> @@ -143,6 +146,7 @@ class AMMWithdraw : public Transactor std::uint16_t tfee, FreezeHandling freezeHandling, WithdrawAll withdrawAll, + XRPAmount const& priorBalance, beast::Journal const& journal); static std::pair