diff --git a/include/xrpl/protocol/SField.h b/include/xrpl/protocol/SField.h index 5b505fc3399..a0810f3c7b5 100644 --- a/include/xrpl/protocol/SField.h +++ b/include/xrpl/protocol/SField.h @@ -588,7 +588,6 @@ extern SF_ACCOUNT const sfRegularKey; extern SF_ACCOUNT const sfNFTokenMinter; extern SF_ACCOUNT const sfEmitCallback; extern SF_ACCOUNT const sfHolder; -extern SF_ACCOUNT const sfAMMAccount; // account (uncommon) extern SF_ACCOUNT const sfHookAccount; diff --git a/include/xrpl/protocol/jss.h b/include/xrpl/protocol/jss.h index f56858a161a..0702d692f9f 100644 --- a/include/xrpl/protocol/jss.h +++ b/include/xrpl/protocol/jss.h @@ -48,7 +48,6 @@ JSS(AccountDelete); // transaction type. JSS(AccountRoot); // ledger type. JSS(AccountSet); // transaction type. JSS(AMM); // ledger type -JSS(AMMAccount); // field JSS(AMMBid); // transaction type JSS(AMMClawback); // transaction type. JSS(AMMID); // field diff --git a/src/test/app/AMMClawback_test.cpp b/src/test/app/AMMClawback_test.cpp index f1a44d347a2..812193932a5 100644 --- a/src/test/app/AMMClawback_test.cpp +++ b/src/test/app/AMMClawback_test.cpp @@ -93,6 +93,11 @@ class AMMClawback_test : public jtx::AMMTest // Issuer can not clawback from himself. env(amm::ammClawback(gw, gw, USD, XRP, std::nullopt, std::nullopt), ter(temMALFORMED)); + + // Holder can not clawback from himself. + env(amm::ammClawback( + alice, alice, USD, XRP, std::nullopt, std::nullopt), + ter(temMALFORMED)); } // Test if the Asset field matches the Account field. @@ -640,11 +645,11 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(!amm.ammExists()); } - // Test AMMClawback for USD/XRP pool. The assets are issued by different - // issuer. Claw back USD for multiple times, and XRP goes back to the - // holder. The last AMMClawback transaction exceeds the holder's USD - // balance in AMM pool. In this case, gw creates the AMM pool USD/XRP, - // both alice and bob deposit into it. gw2 creates the AMM pool EUR/XRP. + // Test AMMClawback for USD/XRP pool. Claw back USD for multiple times, + // and XRP goes back to the holder. The last AMMClawback transaction + // exceeds the holder's USD balance in AMM pool. In this case, gw + // creates the AMM pool USD/XRP, both alice and bob deposit into it. gw2 + // creates the AMM pool EUR/XRP. { Env env(*this, features); Account gw{"gateway"}; @@ -1000,8 +1005,8 @@ class AMMClawback_test : public jtx::AMMTest BEAST_EXPECT(!amm.ammExists()); } - // Test AMMClawback for USD/XRP pool. The assets are issued by different - // issuer. Claw back all the USD for different users. + // Test AMMClawback for USD/XRP pool. Claw back all the USD for + // different users. { Env env(*this, features); Account gw{"gateway"}; diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 26c18d3f4b4..feee7eae603 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -6951,44 +6951,45 @@ struct AMM_test : public jtx::AMMTest using namespace jtx; // This lambda function is used to create trustlines - // between gw and alice, and create and return an AMM account. - auto setupAMM = [&](Env& env) -> AMM* { + // between gw and alice, and create an AMM account. + // And also test the callback function. + auto testAMMDeposit = [&](Env& env, std::function cb) { env.fund(XRP(1'000), gw); fund(env, gw, {alice}, XRP(1'000), {USD(1'000)}, Fund::Acct); env.close(); AMM* amm = new AMM(env, alice, XRP(100), USD(100), ter(tesSUCCESS)); env(trust(gw, alice["USD"](0), tfSetFreeze)); - return amm; + cb(*amm); }; + // Deposit two assets, one of which is frozen, + // then we should get tecFROZEN error. { - // Deposit two assets, one of which is frozen, - // then we should get tecFROZEN error. Env env(*this, features); - auto amm = setupAMM(env); - amm->deposit( - alice, - USD(100), - XRP(100), - std::nullopt, - tfTwoAsset, - ter(tecFROZEN)); - delete amm; + testAMMDeposit(env, [&](AMM& amm) { + amm.deposit( + alice, + USD(100), + XRP(100), + std::nullopt, + tfTwoAsset, + ter(tecFROZEN)); + }); } + // Deposit one asset, which is the frozen token, + // then we should get tecFROZEN error. { - // Deposit one asset, which is the frozen token, - // then we should get tecFROZEN error. Env env(*this, features); - auto amm = setupAMM(env); - amm->deposit( - alice, - USD(100), - std::nullopt, - std::nullopt, - tfSingleAsset, - ter(tecFROZEN)); - delete amm; + testAMMDeposit(env, [&](AMM& amm) { + amm.deposit( + alice, + USD(100), + std::nullopt, + std::nullopt, + tfSingleAsset, + ter(tecFROZEN)); + }); } if (features[featureAMMClawback]) @@ -6997,14 +6998,15 @@ struct AMM_test : public jtx::AMMTest // but the other asset is frozen. We should get tecFROZEN error // when feature AMMClawback is enabled. Env env(*this, features); - auto amm = setupAMM(env); - amm->deposit( - alice, - XRP(100), - std::nullopt, - std::nullopt, - tfSingleAsset, - ter(tecFROZEN)); + testAMMDeposit(env, [&](AMM& amm) { + amm.deposit( + alice, + XRP(100), + std::nullopt, + std::nullopt, + tfSingleAsset, + ter(tecFROZEN)); + }); } else { @@ -7012,14 +7014,15 @@ struct AMM_test : public jtx::AMMTest // but the other asset is frozen. We will get tecSUCCESS // when feature AMMClawback is not enabled. Env env(*this, features); - auto amm = setupAMM(env); - amm->deposit( - alice, - XRP(100), - std::nullopt, - std::nullopt, - tfSingleAsset, - ter(tesSUCCESS)); + testAMMDeposit(env, [&](AMM& amm) { + amm.deposit( + alice, + XRP(100), + std::nullopt, + std::nullopt, + tfSingleAsset, + ter(tesSUCCESS)); + }); } } diff --git a/src/xrpld/app/misc/AMMUtils.h b/src/xrpld/app/misc/AMMUtils.h index e02ef6158ad..52fe819a28e 100644 --- a/src/xrpld/app/misc/AMMUtils.h +++ b/src/xrpld/app/misc/AMMUtils.h @@ -123,31 +123,6 @@ isOnlyLiquidityProvider( Issue const& ammIssue, AccountID const& lpAccount); -std::tuple> -withdraw( - Sandbox& view, - AccountID const& ammAccount, - AccountID const& account, - SLE const& ammSle, - STAmount const& amountBalance, - STAmount const& amountWithdraw, - std::optional const& amount2Withdraw, - STAmount const& lpTokensAMMBalance, - STAmount const& lpTokensWithdraw, - std::uint16_t tfee, - beast::Journal const& journal, - STTx const& tx, - bool withdrawAll); - -std::pair -deleteAMMAccountIfEmpty( - Sandbox& sb, - std::shared_ptr const ammSle, - STAmount const& lpTokenBalance, - Issue const& issue1, - Issue const& issue2, - beast::Journal const& journal); - } // namespace ripple #endif // RIPPLE_APP_MISC_AMMUTILS_H_INLCUDED diff --git a/src/xrpld/app/misc/detail/AMMUtils.cpp b/src/xrpld/app/misc/detail/AMMUtils.cpp index b5aeb10326f..d5af0ee02ef 100644 --- a/src/xrpld/app/misc/detail/AMMUtils.cpp +++ b/src/xrpld/app/misc/detail/AMMUtils.cpp @@ -432,204 +432,4 @@ isOnlyLiquidityProvider( return Unexpected(tecINTERNAL); // LCOV_EXCL_LINE } -std::tuple> -withdraw( - Sandbox& view, - AccountID const& ammAccount, - AccountID const& account, - SLE const& ammSle, - STAmount const& amountBalance, - STAmount const& amountWithdraw, - std::optional const& amount2Withdraw, - STAmount const& lpTokensAMMBalance, - STAmount const& lpTokensWithdraw, - std::uint16_t tfee, - beast::Journal const& journal, - STTx const& tx, - bool withdrawAll) -{ - auto const lpTokens = ammLPHolds(view, ammSle, account, journal); - auto const expected = ammHolds( - view, - ammSle, - amountWithdraw.issue(), - std::nullopt, - FreezeHandling::fhZERO_IF_FROZEN, - journal); - // LCOV_EXCL_START - if (!expected) - return {expected.error(), STAmount{}, STAmount{}, STAmount{}}; - // LCOV_EXCL_STOP - auto const [curBalance, curBalance2, _] = *expected; - (void)_; - - auto const - [amountWithdrawActual, amount2WithdrawActual, lpTokensWithdrawActual] = - [&]() -> std::tuple, STAmount> { - if (!withdrawAll) - return adjustAmountsByLPTokens( - amountBalance, - amountWithdraw, - amount2Withdraw, - lpTokensAMMBalance, - lpTokensWithdraw, - tfee, - false); - return std::make_tuple( - amountWithdraw, amount2Withdraw, lpTokensWithdraw); - }(); - - if (lpTokensWithdrawActual <= beast::zero || - lpTokensWithdrawActual > lpTokens) - { - JLOG(journal.debug()) - << "AMM Withdraw: failed to withdraw, invalid LP tokens: " - << lpTokensWithdrawActual << " " << lpTokens << " " - << lpTokensAMMBalance; - return {tecAMM_INVALID_TOKENS, STAmount{}, STAmount{}, STAmount{}}; - } - - // Should not happen since the only LP on last withdraw - // has the balance set to the lp token trustline balance. - if (view.rules().enabled(fixAMMv1_1) && - lpTokensWithdrawActual > lpTokensAMMBalance) - { - // LCOV_EXCL_START - JLOG(journal.debug()) - << "AMM Withdraw: failed to withdraw, unexpected LP tokens: " - << lpTokensWithdrawActual << " " << lpTokens << " " - << lpTokensAMMBalance; - return {tecINTERNAL, STAmount{}, STAmount{}, STAmount{}}; - // LCOV_EXCL_STOP - } - - // Withdrawing one side of the pool - if ((amountWithdrawActual == curBalance && - amount2WithdrawActual != curBalance2) || - (amount2WithdrawActual == curBalance2 && - amountWithdrawActual != curBalance)) - { - JLOG(journal.debug()) - << "AMM Withdraw: failed to withdraw one side of the pool " - << " curBalance: " << curBalance << " " << amountWithdrawActual - << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " - << lpTokensAMMBalance; - return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; - } - - // May happen if withdrawing an amount close to one side of the pool - if (lpTokensWithdrawActual == lpTokensAMMBalance && - (amountWithdrawActual != curBalance || - amount2WithdrawActual != curBalance2)) - { - JLOG(journal.debug()) - << "AMM Withdraw: failed to withdraw all tokens " - << " curBalance: " << curBalance << " " << amountWithdrawActual - << " curBalance2: " << amount2WithdrawActual.value_or(STAmount{0}) - << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " - << lpTokensAMMBalance; - return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; - } - - // Withdrawing more than the pool's balance - if (amountWithdrawActual > curBalance || - amount2WithdrawActual > curBalance2) - { - JLOG(journal.debug()) - << "AMM Withdraw: withdrawing more than the pool's balance " - << " curBalance: " << curBalance << " " << amountWithdrawActual - << " curBalance2: " << curBalance2 << " " - << (amount2WithdrawActual ? *amount2WithdrawActual : STAmount{}) - << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " - << lpTokensAMMBalance; - return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; - } - - // Withdraw amountWithdraw - auto res = accountSend( - view, - ammAccount, - account, - amountWithdrawActual, - journal, - WaiveTransferFee::Yes); - if (res != tesSUCCESS) - { - // LCOV_EXCL_START - JLOG(journal.debug()) - << "AMM Withdraw: failed to withdraw " << amountWithdrawActual; - return {res, STAmount{}, STAmount{}, STAmount{}}; - // LCOV_EXCL_STOP - } - - // Withdraw amount2Withdraw - if (amount2WithdrawActual) - { - res = accountSend( - view, - ammAccount, - account, - *amount2WithdrawActual, - journal, - WaiveTransferFee::Yes); - if (res != tesSUCCESS) - { - // LCOV_EXCL_START - JLOG(journal.debug()) << "AMM Withdraw: failed to withdraw " - << *amount2WithdrawActual; - return {res, STAmount{}, STAmount{}, STAmount{}}; - // LCOV_EXCL_STOP - } - } - - // Withdraw LP tokens - res = redeemIOU( - view, - account, - lpTokensWithdrawActual, - lpTokensWithdrawActual.issue(), - journal); - if (res != tesSUCCESS) - { - // LCOV_EXCL_START - JLOG(journal.debug()) << "AMM Withdraw: failed to withdraw LPTokens"; - return {res, STAmount{}, STAmount{}, STAmount{}}; - // LCOV_EXCL_STOP - } - - return std::make_tuple( - tesSUCCESS, - lpTokensAMMBalance - lpTokensWithdrawActual, - amountWithdrawActual, - amount2WithdrawActual); -} - -std::pair -deleteAMMAccountIfEmpty( - Sandbox& sb, - std::shared_ptr const ammSle, - STAmount const& lpTokenBalance, - Issue const& issue1, - Issue const& issue2, - beast::Journal const& journal) -{ - TER ter; - bool updateBalance = true; - if (lpTokenBalance == beast::zero) - { - ter = deleteAMMAccount(sb, issue1, issue2, journal); - if (ter != tesSUCCESS && ter != tecINCOMPLETE) - return {ter, false}; // LCOV_EXCL_LINE - else - updateBalance = (ter == tecINCOMPLETE); - } - - if (updateBalance) - { - ammSle->setFieldAmount(sfLPTokenBalance, lpTokenBalance); - sb.update(ammSle); - } - - return {ter, true}; -} } // namespace ripple diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index 82b7c78de3c..f952d5c3ec7 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -84,12 +84,11 @@ AMMClawback::preflight(PreflightContext const& ctx) TER AMMClawback::preclaim(PreclaimContext const& ctx) { - auto const issuer = ctx.tx[sfAccount]; - auto const holder = ctx.tx[sfHolder]; auto const asset = ctx.tx[sfAsset]; auto const asset2 = ctx.tx[sfAsset2]; - auto const sleIssuer = ctx.view.read(keylet::account(issuer)); - auto const sleHolder = ctx.view.read(keylet::account(holder)); + auto const sleIssuer = ctx.view.read(keylet::account(ctx.tx[sfAccount])); + if (!sleIssuer) + return terNO_ACCOUNT; // LCOV_EXCL_LINE auto const ammSle = ctx.view.read(keylet::amm(asset, asset2)); if (!ammSle) @@ -107,15 +106,12 @@ AMMClawback::preclaim(PreclaimContext const& ctx) return tecNO_PERMISSION; auto const flags = ctx.tx.getFlags(); - if (flags & tfClawTwoAssets) + if (flags & tfClawTwoAssets && asset.account != asset2.account) { - if (asset.account != asset2.account) - { - JLOG(ctx.j.trace()) - << "AMMClawback: tfClawTwoAssets can only be enabled when two " - "assets in the AMM pool are both issued by the issuer"; - return tecNO_PERMISSION; - } + JLOG(ctx.j.trace()) + << "AMMClawback: tfClawTwoAssets can only be enabled when two " + "assets in the AMM pool are both issued by the issuer"; + return tecNO_PERMISSION; } return tesSUCCESS; @@ -188,9 +184,8 @@ AMMClawback::applyGuts(Sandbox& sb) holdLPtokens, holdLPtokens, 0, - ctx_.journal, - ctx_.tx, - true); + WithdrawAll::Yes, + ctx_.journal); } else std::tie(result, newLPTokenBalance, amountWithdraw, amount2Withdraw) = @@ -207,7 +202,7 @@ AMMClawback::applyGuts(Sandbox& sb) if (result != tesSUCCESS) return result; // LCOV_EXCL_LINE - auto const res = deleteAMMAccountIfEmpty( + auto const res = AMMWithdraw::deleteAMMAccountIfEmpty( sb, ammSle, newLPTokenBalance, asset, asset2, j_); if (!res.second) return res.first; // LCOV_EXCL_LINE @@ -265,13 +260,12 @@ AMMClawback::equalWithdrawMatchingOneAmount( holdLPtokens, holdLPtokens, 0, - ctx_.journal, - ctx_.tx, - true); + WithdrawAll::Yes, + ctx_.journal); // Because we are doing a two-asset withdrawal, // tfee is actually not used, so pass tfee as 0. - return withdraw( + return AMMWithdraw::withdrawAmounts( sb, ammAccount, holder, @@ -282,9 +276,8 @@ AMMClawback::equalWithdrawMatchingOneAmount( lptAMMBalance, toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac), 0, - ctx_.journal, - ctx_.tx, - false); + WithdrawAll::No, + ctx_.journal); } } // namespace ripple diff --git a/src/xrpld/app/tx/detail/AMMDeposit.cpp b/src/xrpld/app/tx/detail/AMMDeposit.cpp index d9bd22a0097..c73b833f9c6 100644 --- a/src/xrpld/app/tx/detail/AMMDeposit.cpp +++ b/src/xrpld/app/tx/detail/AMMDeposit.cpp @@ -248,30 +248,21 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) { // Check if either of the assets is frozen, AMMDeposit is not allowed // if either asset is frozen - auto checkAsset = [&](std::optional const& asset) -> TER { - if (!asset.has_value()) - return temMALFORMED; // LCOV_EXCL_LINE - - if (isFrozen(ctx.view, accountID, asset.value())) + auto checkAsset = [&](Issue const& asset) -> bool { + if (isFrozen(ctx.view, accountID, asset)) { - JLOG(ctx.j.debug()) << "AMM Deposit: account is frozen, " - << to_string(accountID) << " " - << to_string(asset->currency); + JLOG(ctx.j.debug()) + << "AMM Deposit: account or currency is frozen, " + << to_string(accountID) << " " << to_string(asset.currency); - return tecFROZEN; + return false; } - return tesSUCCESS; + return true; }; - auto const asset = ctx.tx[~sfAsset]; - auto const asset2 = ctx.tx[~sfAsset2]; - - if (auto const ter = checkAsset(asset)) - return ter; - - if (auto const ter = checkAsset(asset2)) - return ter; + if (!checkAsset(ctx.tx[sfAsset]) || !checkAsset(ctx.tx[sfAsset2])) + return tecFROZEN; } auto const amount = ctx.tx[~sfAmount]; diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp index 9dd71d1f6f8..90c0597a0ac 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.cpp +++ b/src/xrpld/app/tx/detail/AMMWithdraw.cpp @@ -399,8 +399,6 @@ AMMWithdraw::applyGuts(Sandbox& sb) { TER result; STAmount newLPTokenBalance; - bool withdrawAll = - ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = equalWithdrawTokens( sb, @@ -413,9 +411,8 @@ AMMWithdraw::applyGuts(Sandbox& sb) lpTokens, *lpTokensWithdraw, tfee, - ctx_.journal, - ctx_.tx, - withdrawAll); + isWithdrawAll(ctx_.tx), + ctx_.journal); return {result, newLPTokenBalance}; } // should not happen. @@ -456,6 +453,240 @@ AMMWithdraw::doApply() return result.first; } +std::tuple +AMMWithdraw::withdraw( + Sandbox& view, + AccountID const& ammAccount, + AccountID const& account, + SLE const& ammSle, + STAmount const& amountBalance, + STAmount const& amountWithdraw, + std::optional const& amount2Withdraw, + STAmount const& lpTokensAMMBalance, + STAmount const& lpTokensWithdraw, + std::uint16_t tfee, + WithdrawAll withdrawAll, + beast::Journal const& journal) +{ + TER ter; + STAmount newLPTokenBalance; + std::tie(ter, newLPTokenBalance, std::ignore, std::ignore) = + withdrawAmounts( + view, + ammAccount, + account, + ammSle, + amountBalance, + amountWithdraw, + amount2Withdraw, + lpTokensAMMBalance, + lpTokensWithdraw, + tfee, + withdrawAll, + journal); + return {ter, newLPTokenBalance}; +} + +std::tuple> +AMMWithdraw::withdrawAmounts( + Sandbox& view, + AccountID const& ammAccount, + AccountID const& account, + SLE const& ammSle, + STAmount const& amountBalance, + STAmount const& amountWithdraw, + std::optional const& amount2Withdraw, + STAmount const& lpTokensAMMBalance, + STAmount const& lpTokensWithdraw, + std::uint16_t tfee, + WithdrawAll withdrawAll, + beast::Journal const& journal) +{ + auto const lpTokens = ammLPHolds(view, ammSle, account, journal); + auto const expected = ammHolds( + view, + ammSle, + amountWithdraw.issue(), + std::nullopt, + FreezeHandling::fhZERO_IF_FROZEN, + journal); + // LCOV_EXCL_START + if (!expected) + return {expected.error(), STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + auto const [curBalance, curBalance2, _] = *expected; + (void)_; + + auto const + [amountWithdrawActual, amount2WithdrawActual, lpTokensWithdrawActual] = + [&]() -> std::tuple, STAmount> { + if (withdrawAll == WithdrawAll::No) + return adjustAmountsByLPTokens( + amountBalance, + amountWithdraw, + amount2Withdraw, + lpTokensAMMBalance, + lpTokensWithdraw, + tfee, + false); + return std::make_tuple( + amountWithdraw, amount2Withdraw, lpTokensWithdraw); + }(); + + if (lpTokensWithdrawActual <= beast::zero || + lpTokensWithdrawActual > lpTokens) + { + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw, invalid LP tokens: " + << lpTokensWithdrawActual << " " << lpTokens << " " + << lpTokensAMMBalance; + return {tecAMM_INVALID_TOKENS, STAmount{}, STAmount{}, STAmount{}}; + } + + // Should not happen since the only LP on last withdraw + // has the balance set to the lp token trustline balance. + if (view.rules().enabled(fixAMMv1_1) && + lpTokensWithdrawActual > lpTokensAMMBalance) + { + // LCOV_EXCL_START + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw, unexpected LP tokens: " + << lpTokensWithdrawActual << " " << lpTokens << " " + << lpTokensAMMBalance; + return {tecINTERNAL, STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + } + + // Withdrawing one side of the pool + if ((amountWithdrawActual == curBalance && + amount2WithdrawActual != curBalance2) || + (amount2WithdrawActual == curBalance2 && + amountWithdrawActual != curBalance)) + { + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw one side of the pool " + << " curBalance: " << curBalance << " " << amountWithdrawActual + << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " + << lpTokensAMMBalance; + return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; + } + + // May happen if withdrawing an amount close to one side of the pool + if (lpTokensWithdrawActual == lpTokensAMMBalance && + (amountWithdrawActual != curBalance || + amount2WithdrawActual != curBalance2)) + { + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw all tokens " + << " curBalance: " << curBalance << " " << amountWithdrawActual + << " curBalance2: " << amount2WithdrawActual.value_or(STAmount{0}) + << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " + << lpTokensAMMBalance; + return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; + } + + // Withdrawing more than the pool's balance + if (amountWithdrawActual > curBalance || + amount2WithdrawActual > curBalance2) + { + JLOG(journal.debug()) + << "AMM Withdraw: withdrawing more than the pool's balance " + << " curBalance: " << curBalance << " " << amountWithdrawActual + << " curBalance2: " << curBalance2 << " " + << (amount2WithdrawActual ? *amount2WithdrawActual : STAmount{}) + << " lpTokensBalance: " << lpTokensWithdraw << " lptBalance " + << lpTokensAMMBalance; + return {tecAMM_BALANCE, STAmount{}, STAmount{}, STAmount{}}; + } + + // Withdraw amountWithdraw + auto res = accountSend( + view, + ammAccount, + account, + amountWithdrawActual, + journal, + WaiveTransferFee::Yes); + if (res != tesSUCCESS) + { + // LCOV_EXCL_START + JLOG(journal.debug()) + << "AMM Withdraw: failed to withdraw " << amountWithdrawActual; + return {res, STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + } + + // Withdraw amount2Withdraw + if (amount2WithdrawActual) + { + res = accountSend( + view, + ammAccount, + account, + *amount2WithdrawActual, + journal, + WaiveTransferFee::Yes); + if (res != tesSUCCESS) + { + // LCOV_EXCL_START + JLOG(journal.debug()) << "AMM Withdraw: failed to withdraw " + << *amount2WithdrawActual; + return {res, STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + } + } + + // Withdraw LP tokens + res = redeemIOU( + view, + account, + lpTokensWithdrawActual, + lpTokensWithdrawActual.issue(), + journal); + if (res != tesSUCCESS) + { + // LCOV_EXCL_START + JLOG(journal.debug()) << "AMM Withdraw: failed to withdraw LPTokens"; + return {res, STAmount{}, STAmount{}, STAmount{}}; + // LCOV_EXCL_STOP + } + + return std::make_tuple( + tesSUCCESS, + lpTokensAMMBalance - lpTokensWithdrawActual, + amountWithdrawActual, + amount2WithdrawActual); +} + +std::pair +AMMWithdraw::deleteAMMAccountIfEmpty( + Sandbox& sb, + std::shared_ptr const ammSle, + STAmount const& lpTokenBalance, + Issue const& issue1, + Issue const& issue2, + beast::Journal const& journal) +{ + TER ter; + bool updateBalance = true; + if (lpTokenBalance == beast::zero) + { + ter = deleteAMMAccount(sb, issue1, issue2, journal); + if (ter != tesSUCCESS && ter != tecINCOMPLETE) + return {ter, false}; // LCOV_EXCL_LINE + else + updateBalance = (ter == tecINCOMPLETE); + } + + if (updateBalance) + { + ammSle->setFieldAmount(sfLPTokenBalance, lpTokenBalance); + sb.update(ammSle); + } + + return {ter, true}; +} + /** Proportional withdrawal of pool assets for the amount of LPTokens. */ std::tuple> @@ -470,16 +701,15 @@ AMMWithdraw::equalWithdrawTokens( STAmount const& lpTokens, STAmount const& lpTokensWithdraw, std::uint16_t tfee, - beast::Journal const& journal, - STTx const& tx, - bool withdrawAll) + WithdrawAll withdrawAll, + beast::Journal const& journal) { try { // Withdrawing all tokens in the pool if (lpTokensWithdraw == lptAMMBalance) { - return withdraw( + return withdrawAmounts( view, ammAccount, account, @@ -490,9 +720,8 @@ AMMWithdraw::equalWithdrawTokens( lptAMMBalance, lpTokensWithdraw, tfee, - journal, - tx, - true); + WithdrawAll::Yes, + journal); } auto const frac = divide(lpTokensWithdraw, lptAMMBalance, noIssue()); @@ -507,7 +736,7 @@ AMMWithdraw::equalWithdrawTokens( if (withdrawAmount == beast::zero || withdraw2Amount == beast::zero) return {tecAMM_FAILED, STAmount{}, STAmount{}, STAmount{}}; - return withdraw( + return withdrawAmounts( view, ammAccount, account, @@ -518,9 +747,8 @@ AMMWithdraw::equalWithdrawTokens( lptAMMBalance, lpTokensWithdraw, tfee, - journal, - tx, - withdrawAll); + withdrawAll, + journal); } // LCOV_EXCL_START catch (std::exception const& e) @@ -569,36 +797,29 @@ AMMWithdraw::equalWithdrawLimit( STAmount const& amount2, std::uint16_t tfee) { - TER result; - STAmount newLPTokenBalance; auto frac = Number{amount} / amountBalance; auto const amount2Withdraw = amount2Balance * frac; - bool withdrawAll = - ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); if (amount2Withdraw <= amount2) { - std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = - withdraw( - view, - ammAccount, - account_, - ammSle, - amountBalance, - amount, - toSTAmount(amount2.issue(), amount2Withdraw), - lptAMMBalance, - toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac), - tfee, - ctx_.journal, - ctx_.tx, - withdrawAll); - return {result, newLPTokenBalance}; + return withdraw( + view, + ammAccount, + account_, + ammSle, + amountBalance, + amount, + toSTAmount(amount2.issue(), amount2Withdraw), + lptAMMBalance, + toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac), + tfee, + isWithdrawAll(ctx_.tx), + ctx_.journal); } frac = Number{amount2} / amount2Balance; auto const amountWithdraw = amountBalance * frac; assert(amountWithdraw <= amount); - std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = withdraw( + return withdraw( view, ammAccount, account_, @@ -609,10 +830,8 @@ AMMWithdraw::equalWithdrawLimit( lptAMMBalance, toSTAmount(lptAMMBalance.issue(), lptAMMBalance * frac), tfee, - ctx_.journal, - ctx_.tx, - withdrawAll); - return {result, newLPTokenBalance}; + isWithdrawAll(ctx_.tx), + ctx_.journal); } /** Withdraw single asset equivalent to the amount specified in Asset1Out. @@ -634,11 +853,7 @@ AMMWithdraw::singleWithdraw( if (tokens == beast::zero) return {tecAMM_FAILED, STAmount{}}; - TER result; - STAmount newLPTokenBalance; - bool withdrawAll = - ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); - std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = withdraw( + return withdraw( view, ammAccount, account_, @@ -649,11 +864,8 @@ AMMWithdraw::singleWithdraw( lptAMMBalance, tokens, tfee, - ctx_.journal, - ctx_.tx, - withdrawAll); - - return {result, newLPTokenBalance}; + isWithdrawAll(ctx_.tx), + ctx_.journal); } /** withdrawal of single asset specified in Asset1Out proportional @@ -681,27 +893,19 @@ AMMWithdraw::singleWithdrawTokens( withdrawByTokens(amountBalance, lptAMMBalance, lpTokensWithdraw, tfee); if (amount == beast::zero || amountWithdraw >= amount) { - TER result; - STAmount newLPTokenBalance; - bool withdrawAll = - ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); - std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = - withdraw( - view, - ammAccount, - account_, - ammSle, - amountBalance, - amountWithdraw, - std::nullopt, - lptAMMBalance, - lpTokensWithdraw, - tfee, - ctx_.journal, - ctx_.tx, - withdrawAll); - - return {result, newLPTokenBalance}; + return withdraw( + view, + ammAccount, + account_, + ammSle, + amountBalance, + amountWithdraw, + std::nullopt, + lptAMMBalance, + lpTokensWithdraw, + tfee, + isWithdrawAll(ctx_.tx), + ctx_.journal); } return {tecAMM_FAILED, STAmount{}}; @@ -756,30 +960,29 @@ AMMWithdraw::singleWithdrawEPrice( auto const amountWithdraw = toSTAmount(amount.issue(), tokens / ePrice); if (amount == beast::zero || amountWithdraw >= amount) { - TER result; - STAmount newLPTokenBalance; - bool withdrawAll = - ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll); - std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = - withdraw( - view, - ammAccount, - account_, - ammSle, - amountBalance, - amountWithdraw, - std::nullopt, - lptAMMBalance, - toSTAmount(lptAMMBalance.issue(), tokens), - tfee, - ctx_.journal, - ctx_.tx, - withdrawAll); - - return {result, newLPTokenBalance}; + return withdraw( + view, + ammAccount, + account_, + ammSle, + amountBalance, + amountWithdraw, + std::nullopt, + lptAMMBalance, + toSTAmount(lptAMMBalance.issue(), tokens), + tfee, + isWithdrawAll(ctx_.tx), + ctx_.journal); } return {tecAMM_FAILED, STAmount{}}; } +WithdrawAll +AMMWithdraw::isWithdrawAll(STTx const& tx) +{ + if (tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll)) + return WithdrawAll::Yes; + return WithdrawAll::No; +} } // namespace ripple diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.h b/src/xrpld/app/tx/detail/AMMWithdraw.h index e8147ec65d8..3f04fc0d7ac 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.h +++ b/src/xrpld/app/tx/detail/AMMWithdraw.h @@ -62,6 +62,9 @@ class Sandbox; * @see [XLS30d:AMMWithdraw * transaction](https://github.com/XRPLF/XRPL-Standards/discussions/78) */ + +enum class WithdrawAll : bool { No = false, Yes }; + class AMMWithdraw : public Transactor { public: @@ -91,6 +94,7 @@ class AMMWithdraw : public Transactor * @param lpTokens current LPT balance * @param lpTokensWithdraw amount of tokens to withdraw * @param tfee trading fee in basis points + * @param withdrawAll if withdrawing all lptokens * @return */ static std::tuple> @@ -105,14 +109,79 @@ class AMMWithdraw : public Transactor STAmount const& lpTokens, STAmount const& lpTokensWithdraw, std::uint16_t tfee, - beast::Journal const& journal, - STTx const& tx, - bool withdrawAll); + WithdrawAll withdrawAll, + beast::Journal const& journal); + + /** Withdraw requested assets and token from AMM into LP account. + * Return new total LPToken balance and the withdrawn amounts for both + * assets. + * @param view + * @param ammAccount + * @param amountBalance current LP asset1 balance + * @param amountWithdraw asset1 withdraw amount + * @param amount2Withdraw asset2 withdraw amount + * @param lpTokensAMMBalance current AMM LPT balance + * @param lpTokensWithdraw amount of lptokens to withdraw + * @param tfee trading fee in basis points + * @param withdrawAll if withdraw all lptokens + * @return + */ + static std::tuple> + withdrawAmounts( + Sandbox& view, + AccountID const& ammAccount, + AccountID const& account, + SLE const& ammSle, + STAmount const& amountBalance, + STAmount const& amountWithdraw, + std::optional const& amount2Withdraw, + STAmount const& lpTokensAMMBalance, + STAmount const& lpTokensWithdraw, + std::uint16_t tfee, + WithdrawAll withdrawAll, + beast::Journal const& journal); + + static std::pair + deleteAMMAccountIfEmpty( + Sandbox& sb, + std::shared_ptr const ammSle, + STAmount const& lpTokenBalance, + Issue const& issue1, + Issue const& issue2, + beast::Journal const& journal); private: std::pair applyGuts(Sandbox& view); + /** Withdraw requested assets and token from AMM into LP account. + * Return new total LPToken balance. + * @param view + * @param ammAccount + * @param amountBalance current LP asset1 balance + * @param amountWithdraw asset1 withdraw amount + * @param amount2Withdraw asset2 withdraw amount + * @param lpTokensAMMBalance current AMM LPT balance + * @param lpTokensWithdraw amount of lptokens to withdraw + * @param tfee trading fee in basis points + * @param withdrawAll if withdraw all lptokens + * @return + */ + std::tuple + withdraw( + Sandbox& view, + AccountID const& ammAccount, + AccountID const& account, + SLE const& ammSle, + STAmount const& amountBalance, + STAmount const& amountWithdraw, + std::optional const& amount2Withdraw, + STAmount const& lpTokensAMMBalance, + STAmount const& lpTokensWithdraw, + std::uint16_t tfee, + WithdrawAll withdrawAll, + beast::Journal const& journal); + /** Withdraw both assets (Asset1Out, Asset2Out) with the constraints * on the maximum amount of each asset that the trader is willing * to withdraw. The trading fee is not charged. @@ -201,6 +270,10 @@ class AMMWithdraw : public Transactor STAmount const& amount, STAmount const& ePrice, std::uint16_t tfee); + + /** Check from the flags if it's withdraw all */ + WithdrawAll + isWithdrawAll(STTx const& tx); }; } // namespace ripple