diff --git a/src/ripple/app/tx/impl/AMMDeposit.cpp b/src/ripple/app/tx/impl/AMMDeposit.cpp index de2b047c016..ec5196b20c2 100644 --- a/src/ripple/app/tx/impl/AMMDeposit.cpp +++ b/src/ripple/app/tx/impl/AMMDeposit.cpp @@ -61,11 +61,11 @@ AMMDeposit::preflight(PreflightContext const& ctx) // Amount and Amount2 // AssetLPToken and LPTokens // Amount and EPrice - if (auto const subTxType = std::bitset<32>(flags & tfSubTx); + if (auto const subTxType = std::bitset<32>(flags & tfDepositSubTx); subTxType.none() || subTxType.count() > 1) { JLOG(ctx.j.debug()) << "AMM Deposit: invalid flags."; - return temINVALID_FLAG; + return temBAD_AMM_OPTIONS; } else if (flags & tfLPToken) { @@ -277,7 +277,7 @@ AMMDeposit::applyGuts(Sandbox& sb) return {expected.error(), false}; auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected; - auto const subTxType = ctx_.tx.getFlags() & tfSubTx; + auto const subTxType = ctx_.tx.getFlags() & tfDepositSubTx; auto const [result, depositedTokens] = [&, diff --git a/src/ripple/app/tx/impl/AMMWithdraw.cpp b/src/ripple/app/tx/impl/AMMWithdraw.cpp index 525bf7ee1c5..ca04c6af20f 100644 --- a/src/ripple/app/tx/impl/AMMWithdraw.cpp +++ b/src/ripple/app/tx/impl/AMMWithdraw.cpp @@ -51,49 +51,58 @@ AMMWithdraw::preflight(PreflightContext const& ctx) JLOG(ctx.j.debug()) << "AMM Withdraw: invalid flags."; return temINVALID_FLAG; } - bool const withdrawAll = flags & tfWithdrawAll; auto const amount = ctx.tx[~sfAmount]; auto const amount2 = ctx.tx[~sfAmount2]; auto const ePrice = ctx.tx[~sfEPrice]; auto const lpTokens = ctx.tx[~sfLPTokenIn]; // Valid combinations are: - // LPTokens|tfWithdrawAll + // LPTokens + // tfWithdrawAll // Amount + // tfOneAssetWithdrawAll & Amount // Amount and Amount2 - // AssetLPToken and [LPTokens|tfWithdrawAll] + // Amount and LPTokens // Amount and EPrice - if (auto const subTxType = std::bitset<32>(flags & tfSubTx); + if (auto const subTxType = std::bitset<32>(flags & tfWithdrawSubTx); subTxType.none() || subTxType.count() > 1) { JLOG(ctx.j.debug()) << "AMM Withdraw: invalid flags."; - return temINVALID_FLAG; + return temBAD_AMM_OPTIONS; } else if (flags & tfLPToken) { - if ((!lpTokens && !withdrawAll) || (lpTokens && withdrawAll) || - amount || amount2 || ePrice) + if (!lpTokens || amount || amount2 || ePrice) + return temBAD_AMM_OPTIONS; + } + else if (flags & tfWithdrawAll) + { + if (lpTokens || amount || amount2 || ePrice) + return temBAD_AMM_OPTIONS; + } + else if (flags & tfOneAssetWithdrawAll) + { + if (!amount || lpTokens || amount2 || ePrice) return temBAD_AMM_OPTIONS; } else if (flags & tfSingleAsset) { - if (!amount || lpTokens || withdrawAll || amount2 || ePrice) + if (!amount || lpTokens || amount2 || ePrice) return temBAD_AMM_OPTIONS; } else if (flags & tfTwoAsset) { - if (!amount || !amount2 || lpTokens || withdrawAll || ePrice) + if (!amount || !amount2 || lpTokens || ePrice) return temBAD_AMM_OPTIONS; } else if (flags & tfOneAssetLPToken) { - if (!amount || (!lpTokens && !withdrawAll) || - (lpTokens && withdrawAll) || amount2 || ePrice) + if (!amount || !lpTokens || amount2 || ePrice) return temBAD_AMM_OPTIONS; } else if (flags & tfLimitLPToken) { - if (!amount || !ePrice || lpTokens || withdrawAll || amount2) + if (!amount || !ePrice || lpTokens || amount2) return temBAD_AMM_OPTIONS; } @@ -119,7 +128,9 @@ AMMWithdraw::preflight(PreflightContext const& ctx) } if (auto const res = invalidAMMAmount( - amount, {{asset, asset2}}, withdrawAll || lpTokens || ePrice)) + amount, + {{asset, asset2}}, + (flags & (tfOneAssetWithdrawAll | tfOneAssetLPToken)) || ePrice)) { JLOG(ctx.j.debug()) << "AMM Withdraw: invalid Asset1Out"; return res; @@ -214,7 +225,8 @@ AMMWithdraw::preclaim(PreclaimContext const& ctx) auto const lptBalance = ammLPHolds(ctx.view, **ammSle, ctx.tx[sfAccount], ctx.j); - auto const lpTokens = (ctx.tx.getFlags() & tfWithdrawAll) + auto const lpTokens = + (ctx.tx.getFlags() & (tfWithdrawAll | tfOneAssetWithdrawAll)) ? std::optional(lptBalance) : ctx.tx[~sfLPTokenIn]; @@ -258,7 +270,8 @@ AMMWithdraw::applyGuts(Sandbox& sb) auto const ammAccountID = (**ammSle)[sfAMMAccount]; auto const lpTokens = ammLPHolds(ctx_.view(), **ammSle, ctx_.tx[sfAccount], ctx_.journal); - auto const lpTokensWithdraw = (ctx_.tx.getFlags() & tfWithdrawAll) + auto const lpTokensWithdraw = + (ctx_.tx.getFlags() & (tfWithdrawAll | tfOneAssetWithdrawAll)) ? std::optional(lpTokens) : ctx_.tx[~sfLPTokenIn]; @@ -274,7 +287,7 @@ AMMWithdraw::applyGuts(Sandbox& sb) return {expected.error(), false}; auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected; - auto const subTxType = ctx_.tx.getFlags() & tfSubTx; + auto const subTxType = ctx_.tx.getFlags() & tfWithdrawSubTx; auto const [result, withdrawnTokens] = [&, @@ -290,7 +303,8 @@ AMMWithdraw::applyGuts(Sandbox& sb) lptAMMBalance, *amount, *amount2); - if (subTxType == tfOneAssetLPToken) + if (subTxType == tfOneAssetLPToken || + subTxType == tfOneAssetWithdrawAll) return singleWithdrawTokens( sb, ammAccountID, @@ -311,7 +325,7 @@ AMMWithdraw::applyGuts(Sandbox& sb) if (subTxType == tfSingleAsset) return singleWithdraw( sb, ammAccountID, amountBalance, lptAMMBalance, *amount, tfee); - if (subTxType == tfLPToken) + if (subTxType == tfLPToken || subTxType == tfWithdrawAll) return equalWithdrawTokens( sb, ammAccountID, diff --git a/src/ripple/protocol/TxFlags.h b/src/ripple/protocol/TxFlags.h index 247d247f03d..942d0f8a2e2 100644 --- a/src/ripple/protocol/TxFlags.h +++ b/src/ripple/protocol/TxFlags.h @@ -135,17 +135,21 @@ constexpr std::uint32_t const tfNFTokenCancelOfferMask = ~(tfUniversal); constexpr std::uint32_t const tfNFTokenAcceptOfferMask = ~tfUniversal; // AMM Flags: -constexpr std::uint32_t tfLPToken = 0x00000001; -constexpr std::uint32_t tfSingleAsset = 0x00000002; -constexpr std::uint32_t tfTwoAsset = 0x00000004; -constexpr std::uint32_t tfOneAssetLPToken = 0x00000008; -constexpr std::uint32_t tfLimitLPToken = 0x00000010; -constexpr std::uint32_t tfWithdrawAll = 0x00000020; -constexpr std::uint32_t tfSubTx = - tfLPToken | tfSingleAsset | tfTwoAsset | tfOneAssetLPToken | tfLimitLPToken; -constexpr std::uint32_t tfWithdrawMask = - ~(tfUniversal | tfSubTx | tfWithdrawAll); -constexpr std::uint32_t tfDepositMask = ~(tfUniversal | tfSubTx); +constexpr std::uint32_t tfLPToken = 0x00010000; +constexpr std::uint32_t tfWithdrawAll = 0x00020000; +constexpr std::uint32_t tfOneAssetWithdrawAll = 0x00040000; +constexpr std::uint32_t tfSingleAsset = 0x00080000; +constexpr std::uint32_t tfTwoAsset = 0x00100000; +constexpr std::uint32_t tfOneAssetLPToken = 0x00200000; +constexpr std::uint32_t tfLimitLPToken = 0x00400000; +constexpr std::uint32_t tfWithdrawSubTx = + tfLPToken | tfSingleAsset | tfTwoAsset | tfOneAssetLPToken | + tfLimitLPToken | tfWithdrawAll | tfOneAssetWithdrawAll; +constexpr std::uint32_t tfDepositSubTx = + tfLPToken | tfSingleAsset | tfTwoAsset | tfOneAssetLPToken | + tfLimitLPToken; +constexpr std::uint32_t tfWithdrawMask = ~(tfUniversal | tfWithdrawSubTx); +constexpr std::uint32_t tfDepositMask = ~(tfUniversal | tfDepositSubTx); // clang-format on diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 55633aa17d6..95d971bff34 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -1431,7 +1431,7 @@ struct AMM_test : public Test std::nullopt, std::nullopt, std::nullopt, - tfPartialPayment, + tfBurnable, std::nullopt, std::nullopt, ter(temINVALID_FLAG)); @@ -1446,20 +1446,19 @@ struct AMM_test : public Test std::optional, NotTEC>> invalidOptions = { - // tokens, asset1Out, asset2Out, EPrice, tfWithdrawAll, - // flags, ter + // tokens, asset1Out, asset2Out, EPrice, flags, ter {std::nullopt, std::nullopt, std::nullopt, std::nullopt, std::nullopt, - temINVALID_FLAG}, + temBAD_AMM_OPTIONS}, {std::nullopt, std::nullopt, std::nullopt, std::nullopt, tfSingleAsset | tfTwoAsset, - temINVALID_FLAG}, + temBAD_AMM_OPTIONS}, {1000, std::nullopt, std::nullopt, @@ -1478,6 +1477,24 @@ struct AMM_test : public Test std::nullopt, tfWithdrawAll, temBAD_AMM_OPTIONS}, + {std::nullopt, + std::nullopt, + std::nullopt, + std::nullopt, + tfWithdrawAll | tfOneAssetWithdrawAll, + temBAD_AMM_OPTIONS}, + {std::nullopt, + USD(100), + std::nullopt, + std::nullopt, + tfWithdrawAll, + temBAD_AMM_OPTIONS}, + {std::nullopt, + std::nullopt, + std::nullopt, + std::nullopt, + tfOneAssetWithdrawAll, + temBAD_AMM_OPTIONS}, {1000, std::nullopt, USD(100), @@ -1606,7 +1623,7 @@ struct AMM_test : public Test USD(0), std::nullopt, std::nullopt, - tfWithdrawAll, + tfOneAssetWithdrawAll, std::nullopt, std::nullopt, ter(tecAMM_FAILED_WITHDRAW)); diff --git a/src/test/jtx/AMM.h b/src/test/jtx/AMM.h index 763b7b07505..89395706688 100644 --- a/src/test/jtx/AMM.h +++ b/src/test/jtx/AMM.h @@ -161,7 +161,7 @@ class AMM account, std::nullopt, asset1OutDetails, - tfWithdrawAll, + asset1OutDetails ? tfOneAssetWithdrawAll : tfWithdrawAll, std::nullopt); } diff --git a/src/test/jtx/impl/AMM.cpp b/src/test/jtx/impl/AMM.cpp index c4fc05c551c..2ffe7686c11 100644 --- a/src/test/jtx/impl/AMM.cpp +++ b/src/test/jtx/impl/AMM.cpp @@ -409,7 +409,7 @@ AMM::deposit( std::uint32_t jvflags = 0; if (flags) jvflags = *flags; - if (!(jvflags & tfSubTx)) + if (!(jvflags & tfDepositSubTx)) { if (tokens && !asset1In) jvflags |= tfLPToken; @@ -417,7 +417,7 @@ AMM::deposit( jvflags |= tfOneAssetLPToken; else if (asset1In && asset2In) jvflags |= tfTwoAsset; - else if (maxEP) + else if (maxEP && asset1In) jvflags |= tfLimitLPToken; else if (asset1In) jvflags |= tfSingleAsset; @@ -519,15 +519,15 @@ AMM::withdraw( std::uint32_t jvflags = 0; if (flags) jvflags = *flags; - if (!(jvflags & tfSubTx)) + if (!(jvflags & tfWithdrawSubTx)) { - if ((tokens || (jvflags & tfWithdrawAll)) && !asset1Out) + if (tokens && !asset1Out) jvflags |= tfLPToken; - else if ((tokens || (jvflags & tfWithdrawAll)) && asset1Out) - jvflags |= tfOneAssetLPToken; else if (asset1Out && asset2Out) jvflags |= tfTwoAsset; - else if (maxEP) + else if (tokens && asset1Out) + jvflags |= tfOneAssetLPToken; + else if (asset1Out && maxEP) jvflags |= tfLimitLPToken; else if (asset1Out) jvflags |= tfSingleAsset;