Skip to content

Commit e956a4e

Browse files
gregtatcamgregtatcam
committed
[FOLD] Fix valid range for Deposit/Withdraw flags and update Withdraw subTx.
1 parent ffe6a55 commit e956a4e

File tree

6 files changed

+81
-46
lines changed

6 files changed

+81
-46
lines changed

‎src/ripple/app/tx/impl/AMMDeposit.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,11 @@ AMMDeposit::preflight(PreflightContext const& ctx)
6161
// Amount and Amount2
6262
// AssetLPToken and LPTokens
6363
// Amount and EPrice
64-
if (auto const subTxType = std::bitset<32>(flags & tfSubTx);
64+
if (auto const subTxType = std::bitset<32>(flags & tfDepositSubTx);
6565
subTxType.none() || subTxType.count() > 1)
6666
{
6767
JLOG(ctx.j.debug()) << "AMM Deposit: invalid flags.";
68-
return temINVALID_FLAG;
68+
return temBAD_AMM_OPTIONS;
6969
}
7070
else if (flags & tfLPToken)
7171
{
@@ -277,7 +277,7 @@ AMMDeposit::applyGuts(Sandbox& sb)
277277
return {expected.error(), false};
278278
auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected;
279279

280-
auto const subTxType = ctx_.tx.getFlags() & tfSubTx;
280+
auto const subTxType = ctx_.tx.getFlags() & tfDepositSubTx;
281281

282282
auto const [result, depositedTokens] =
283283
[&,

‎src/ripple/app/tx/impl/AMMWithdraw.cpp

+32-18
Original file line numberDiff line numberDiff line change
@@ -51,49 +51,58 @@ AMMWithdraw::preflight(PreflightContext const& ctx)
5151
JLOG(ctx.j.debug()) << "AMM Withdraw: invalid flags.";
5252
return temINVALID_FLAG;
5353
}
54-
bool const withdrawAll = flags & tfWithdrawAll;
5554

5655
auto const amount = ctx.tx[~sfAmount];
5756
auto const amount2 = ctx.tx[~sfAmount2];
5857
auto const ePrice = ctx.tx[~sfEPrice];
5958
auto const lpTokens = ctx.tx[~sfLPTokenIn];
6059
// Valid combinations are:
61-
// LPTokens|tfWithdrawAll
60+
// LPTokens
61+
// tfWithdrawAll
6262
// Amount
63+
// tfOneAssetWithdrawAll & Amount
6364
// Amount and Amount2
64-
// AssetLPToken and [LPTokens|tfWithdrawAll]
65+
// Amount and LPTokens
6566
// Amount and EPrice
66-
if (auto const subTxType = std::bitset<32>(flags & tfSubTx);
67+
if (auto const subTxType = std::bitset<32>(flags & tfWithdrawSubTx);
6768
subTxType.none() || subTxType.count() > 1)
6869
{
6970
JLOG(ctx.j.debug()) << "AMM Withdraw: invalid flags.";
70-
return temINVALID_FLAG;
71+
return temBAD_AMM_OPTIONS;
7172
}
7273
else if (flags & tfLPToken)
7374
{
74-
if ((!lpTokens && !withdrawAll) || (lpTokens && withdrawAll) ||
75-
amount || amount2 || ePrice)
75+
if (!lpTokens || amount || amount2 || ePrice)
76+
return temBAD_AMM_OPTIONS;
77+
}
78+
else if (flags & tfWithdrawAll)
79+
{
80+
if (lpTokens || amount || amount2 || ePrice)
81+
return temBAD_AMM_OPTIONS;
82+
}
83+
else if (flags & tfOneAssetWithdrawAll)
84+
{
85+
if (!amount || lpTokens || amount2 || ePrice)
7686
return temBAD_AMM_OPTIONS;
7787
}
7888
else if (flags & tfSingleAsset)
7989
{
80-
if (!amount || lpTokens || withdrawAll || amount2 || ePrice)
90+
if (!amount || lpTokens || amount2 || ePrice)
8191
return temBAD_AMM_OPTIONS;
8292
}
8393
else if (flags & tfTwoAsset)
8494
{
85-
if (!amount || !amount2 || lpTokens || withdrawAll || ePrice)
95+
if (!amount || !amount2 || lpTokens || ePrice)
8696
return temBAD_AMM_OPTIONS;
8797
}
8898
else if (flags & tfOneAssetLPToken)
8999
{
90-
if (!amount || (!lpTokens && !withdrawAll) ||
91-
(lpTokens && withdrawAll) || amount2 || ePrice)
100+
if (!amount || !lpTokens || amount2 || ePrice)
92101
return temBAD_AMM_OPTIONS;
93102
}
94103
else if (flags & tfLimitLPToken)
95104
{
96-
if (!amount || !ePrice || lpTokens || withdrawAll || amount2)
105+
if (!amount || !ePrice || lpTokens || amount2)
97106
return temBAD_AMM_OPTIONS;
98107
}
99108

@@ -119,7 +128,9 @@ AMMWithdraw::preflight(PreflightContext const& ctx)
119128
}
120129

121130
if (auto const res = invalidAMMAmount(
122-
amount, {{asset, asset2}}, withdrawAll || lpTokens || ePrice))
131+
amount,
132+
{{asset, asset2}},
133+
(flags & (tfOneAssetWithdrawAll | tfOneAssetLPToken)) || ePrice))
123134
{
124135
JLOG(ctx.j.debug()) << "AMM Withdraw: invalid Asset1Out";
125136
return res;
@@ -214,7 +225,8 @@ AMMWithdraw::preclaim(PreclaimContext const& ctx)
214225

215226
auto const lptBalance =
216227
ammLPHolds(ctx.view, **ammSle, ctx.tx[sfAccount], ctx.j);
217-
auto const lpTokens = (ctx.tx.getFlags() & tfWithdrawAll)
228+
auto const lpTokens =
229+
(ctx.tx.getFlags() & (tfWithdrawAll | tfOneAssetWithdrawAll))
218230
? std::optional<STAmount>(lptBalance)
219231
: ctx.tx[~sfLPTokenIn];
220232

@@ -258,7 +270,8 @@ AMMWithdraw::applyGuts(Sandbox& sb)
258270
auto const ammAccountID = (**ammSle)[sfAMMAccount];
259271
auto const lpTokens =
260272
ammLPHolds(ctx_.view(), **ammSle, ctx_.tx[sfAccount], ctx_.journal);
261-
auto const lpTokensWithdraw = (ctx_.tx.getFlags() & tfWithdrawAll)
273+
auto const lpTokensWithdraw =
274+
(ctx_.tx.getFlags() & (tfWithdrawAll | tfOneAssetWithdrawAll))
262275
? std::optional<STAmount>(lpTokens)
263276
: ctx_.tx[~sfLPTokenIn];
264277

@@ -274,7 +287,7 @@ AMMWithdraw::applyGuts(Sandbox& sb)
274287
return {expected.error(), false};
275288
auto const [amountBalance, amount2Balance, lptAMMBalance] = *expected;
276289

277-
auto const subTxType = ctx_.tx.getFlags() & tfSubTx;
290+
auto const subTxType = ctx_.tx.getFlags() & tfWithdrawSubTx;
278291

279292
auto const [result, withdrawnTokens] =
280293
[&,
@@ -290,7 +303,8 @@ AMMWithdraw::applyGuts(Sandbox& sb)
290303
lptAMMBalance,
291304
*amount,
292305
*amount2);
293-
if (subTxType == tfOneAssetLPToken)
306+
if (subTxType == tfOneAssetLPToken ||
307+
subTxType == tfOneAssetWithdrawAll)
294308
return singleWithdrawTokens(
295309
sb,
296310
ammAccountID,
@@ -311,7 +325,7 @@ AMMWithdraw::applyGuts(Sandbox& sb)
311325
if (subTxType == tfSingleAsset)
312326
return singleWithdraw(
313327
sb, ammAccountID, amountBalance, lptAMMBalance, *amount, tfee);
314-
if (subTxType == tfLPToken)
328+
if (subTxType == tfLPToken || subTxType == tfWithdrawAll)
315329
return equalWithdrawTokens(
316330
sb,
317331
ammAccountID,

‎src/ripple/protocol/TxFlags.h

+15-11
Original file line numberDiff line numberDiff line change
@@ -135,17 +135,21 @@ constexpr std::uint32_t const tfNFTokenCancelOfferMask = ~(tfUniversal);
135135
constexpr std::uint32_t const tfNFTokenAcceptOfferMask = ~tfUniversal;
136136

137137
// AMM Flags:
138-
constexpr std::uint32_t tfLPToken = 0x00000001;
139-
constexpr std::uint32_t tfSingleAsset = 0x00000002;
140-
constexpr std::uint32_t tfTwoAsset = 0x00000004;
141-
constexpr std::uint32_t tfOneAssetLPToken = 0x00000008;
142-
constexpr std::uint32_t tfLimitLPToken = 0x00000010;
143-
constexpr std::uint32_t tfWithdrawAll = 0x00000020;
144-
constexpr std::uint32_t tfSubTx =
145-
tfLPToken | tfSingleAsset | tfTwoAsset | tfOneAssetLPToken | tfLimitLPToken;
146-
constexpr std::uint32_t tfWithdrawMask =
147-
~(tfUniversal | tfSubTx | tfWithdrawAll);
148-
constexpr std::uint32_t tfDepositMask = ~(tfUniversal | tfSubTx);
138+
constexpr std::uint32_t tfLPToken = 0x00010000;
139+
constexpr std::uint32_t tfWithdrawAll = 0x00020000;
140+
constexpr std::uint32_t tfOneAssetWithdrawAll = 0x00040000;
141+
constexpr std::uint32_t tfSingleAsset = 0x00080000;
142+
constexpr std::uint32_t tfTwoAsset = 0x00100000;
143+
constexpr std::uint32_t tfOneAssetLPToken = 0x00200000;
144+
constexpr std::uint32_t tfLimitLPToken = 0x00400000;
145+
constexpr std::uint32_t tfWithdrawSubTx =
146+
tfLPToken | tfSingleAsset | tfTwoAsset | tfOneAssetLPToken |
147+
tfLimitLPToken | tfWithdrawAll | tfOneAssetWithdrawAll;
148+
constexpr std::uint32_t tfDepositSubTx =
149+
tfLPToken | tfSingleAsset | tfTwoAsset | tfOneAssetLPToken |
150+
tfLimitLPToken;
151+
constexpr std::uint32_t tfWithdrawMask = ~(tfUniversal | tfWithdrawSubTx);
152+
constexpr std::uint32_t tfDepositMask = ~(tfUniversal | tfDepositSubTx);
149153

150154
// clang-format on
151155

‎src/test/app/AMM_test.cpp

+23-6
Original file line numberDiff line numberDiff line change
@@ -1431,7 +1431,7 @@ struct AMM_test : public Test
14311431
std::nullopt,
14321432
std::nullopt,
14331433
std::nullopt,
1434-
tfPartialPayment,
1434+
tfBurnable,
14351435
std::nullopt,
14361436
std::nullopt,
14371437
ter(temINVALID_FLAG));
@@ -1446,20 +1446,19 @@ struct AMM_test : public Test
14461446
std::optional<std::uint32_t>,
14471447
NotTEC>>
14481448
invalidOptions = {
1449-
// tokens, asset1Out, asset2Out, EPrice, tfWithdrawAll,
1450-
// flags, ter
1449+
// tokens, asset1Out, asset2Out, EPrice, flags, ter
14511450
{std::nullopt,
14521451
std::nullopt,
14531452
std::nullopt,
14541453
std::nullopt,
14551454
std::nullopt,
1456-
temINVALID_FLAG},
1455+
temBAD_AMM_OPTIONS},
14571456
{std::nullopt,
14581457
std::nullopt,
14591458
std::nullopt,
14601459
std::nullopt,
14611460
tfSingleAsset | tfTwoAsset,
1462-
temINVALID_FLAG},
1461+
temBAD_AMM_OPTIONS},
14631462
{1000,
14641463
std::nullopt,
14651464
std::nullopt,
@@ -1478,6 +1477,24 @@ struct AMM_test : public Test
14781477
std::nullopt,
14791478
tfWithdrawAll,
14801479
temBAD_AMM_OPTIONS},
1480+
{std::nullopt,
1481+
std::nullopt,
1482+
std::nullopt,
1483+
std::nullopt,
1484+
tfWithdrawAll | tfOneAssetWithdrawAll,
1485+
temBAD_AMM_OPTIONS},
1486+
{std::nullopt,
1487+
USD(100),
1488+
std::nullopt,
1489+
std::nullopt,
1490+
tfWithdrawAll,
1491+
temBAD_AMM_OPTIONS},
1492+
{std::nullopt,
1493+
std::nullopt,
1494+
std::nullopt,
1495+
std::nullopt,
1496+
tfOneAssetWithdrawAll,
1497+
temBAD_AMM_OPTIONS},
14811498
{1000,
14821499
std::nullopt,
14831500
USD(100),
@@ -1606,7 +1623,7 @@ struct AMM_test : public Test
16061623
USD(0),
16071624
std::nullopt,
16081625
std::nullopt,
1609-
tfWithdrawAll,
1626+
tfOneAssetWithdrawAll,
16101627
std::nullopt,
16111628
std::nullopt,
16121629
ter(tecAMM_FAILED_WITHDRAW));

‎src/test/jtx/AMM.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ class AMM
161161
account,
162162
std::nullopt,
163163
asset1OutDetails,
164-
tfWithdrawAll,
164+
asset1OutDetails ? tfOneAssetWithdrawAll : tfWithdrawAll,
165165
std::nullopt);
166166
}
167167

‎src/test/jtx/impl/AMM.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -409,15 +409,15 @@ AMM::deposit(
409409
std::uint32_t jvflags = 0;
410410
if (flags)
411411
jvflags = *flags;
412-
if (!(jvflags & tfSubTx))
412+
if (!(jvflags & tfDepositSubTx))
413413
{
414414
if (tokens && !asset1In)
415415
jvflags |= tfLPToken;
416416
else if (tokens && asset1In)
417417
jvflags |= tfOneAssetLPToken;
418418
else if (asset1In && asset2In)
419419
jvflags |= tfTwoAsset;
420-
else if (maxEP)
420+
else if (maxEP && asset1In)
421421
jvflags |= tfLimitLPToken;
422422
else if (asset1In)
423423
jvflags |= tfSingleAsset;
@@ -519,15 +519,15 @@ AMM::withdraw(
519519
std::uint32_t jvflags = 0;
520520
if (flags)
521521
jvflags = *flags;
522-
if (!(jvflags & tfSubTx))
522+
if (!(jvflags & tfWithdrawSubTx))
523523
{
524-
if ((tokens || (jvflags & tfWithdrawAll)) && !asset1Out)
524+
if (tokens && !asset1Out)
525525
jvflags |= tfLPToken;
526-
else if ((tokens || (jvflags & tfWithdrawAll)) && asset1Out)
527-
jvflags |= tfOneAssetLPToken;
528526
else if (asset1Out && asset2Out)
529527
jvflags |= tfTwoAsset;
530-
else if (maxEP)
528+
else if (tokens && asset1Out)
529+
jvflags |= tfOneAssetLPToken;
530+
else if (asset1Out && maxEP)
531531
jvflags |= tfLimitLPToken;
532532
else if (asset1Out)
533533
jvflags |= tfSingleAsset;

0 commit comments

Comments
 (0)