From 2d3ea512d0b43bf335568bf279c074a93d342194 Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Sun, 7 Jun 2026 13:36:32 -0400 Subject: [PATCH] Fix AMMWithdraw reserve check ignores intra-transaction XRP credits --- .../tx/transactors/dex/AMMWithdraw.cpp | 23 +++--- src/test/app/AMMMPT_test.cpp | 70 +++++++++++++++++-- 2 files changed, 76 insertions(+), 17 deletions(-) diff --git a/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp b/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp index e57f8558ff..1ad97eafdb 100644 --- a/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp @@ -602,17 +602,16 @@ AMMWithdraw::withdraw( mptokenKey = std::nullopt; if (!enabledFixAmMv12 || isXRP(asset)) return tesSUCCESS; - bool const isIssue = asset.holds(); - bool const assetNotExists = [&] { - if (isIssue) - return !view.exists(keylet::line(account, asset.get())); - auto const issuanceKey = keylet::mptIssuance(asset.get()); - mptokenKey = keylet::mptoken(issuanceKey.key, account); - if (!view.exists(*mptokenKey)) - return true; - mptokenKey = std::nullopt; - return false; - }(); + bool const assetNotExists = asset.visit( + [&](Issue const& issue) { return !view.exists(keylet::line(account, issue)); }, + [&](MPTIssue const& issue) { + auto const issuanceKey = keylet::mptIssuance(issue); + mptokenKey = keylet::mptoken(issuanceKey.key, account); + if (!view.exists(*mptokenKey)) + return true; + mptokenKey = std::nullopt; + return false; + }); if (assetNotExists) { auto sleAccount = view.peek(keylet::account(account)); @@ -626,7 +625,7 @@ AMMWithdraw::withdraw( (ownerCount < 2) ? XRPAmount(beast::kZero) : view.fees().accountReserve(ownerCount + 1)); - auto const balanceAdj = isIssue ? std::max(priorBalance, balance.xrp()) : priorBalance; + auto const balanceAdj = std::max(priorBalance, balance.xrp()); if (balanceAdj < reserve) return tecINSUFFICIENT_RESERVE; } diff --git a/src/test/app/AMMMPT_test.cpp b/src/test/app/AMMMPT_test.cpp index 31b54ceee0..80d3525fa4 100644 --- a/src/test/app/AMMMPT_test.cpp +++ b/src/test/app/AMMMPT_test.cpp @@ -3241,6 +3241,65 @@ private: {{XRP(10'000), gAmmmpt(10'000)}}); } + void + testWithdrawReserveUsesLiveBalance() + { + testcase("Withdraw reserve check uses live balance"); + + using namespace jtx; + + auto const test = [&](auto&& makeToken) { + Env env(*this); + env.fund(XRP(30'000), gw_, alice_, bob_); + env.close(); + + auto const token = makeToken(env); + AMM amm(env, gw_, XRP(100), token(100)); + + // The EUR trustline is an unrelated owner object. The XRP-only + // AMM deposit adds the LP token trustline, so Alice has 2 owners. + env.trust(gw_["EUR"](1), alice_); + amm.deposit(DepositArg{.account = alice_, .asset1In = XRP(10)}); + BEAST_EXPECT(env.ownerCount(alice_) == 2); + env.require(Balance(alice_, token(kNone))); + + // Drain Alice to one drop below the reserve for a third owner + // object, accounting for the fee on the drain payment. + auto const reserveForToken = reserve(env, 3); + auto const targetBalance = reserveForToken - XRPAmount{1}; + auto const baseFee = env.current()->fees().base; + auto const currentBalance = env.balance(alice_).value().xrp(); + auto const drainAmount = currentBalance - targetBalance - baseFee; + BEAST_EXPECT(drainAmount > XRPAmount{0}); + env(pay(alice_, bob_, drops(drainAmount))); + env.close(); + + // AMMWithdraw captures priorBalance before the fee, then the XRP + // leg raises the live sandbox balance before the token leg. + // XRP(2) keeps the integral MPT side positive after rounding. + auto const xrpOut = XRP(2); + auto const tokenOut = token(2); + auto const priorBalance = env.balance(alice_).value().xrp(); + auto const liveBalanceAfterXrpLeg = priorBalance - baseFee + xrpOut.value().xrp(); + BEAST_EXPECT(priorBalance < reserveForToken); + BEAST_EXPECT(liveBalanceAfterXrpLeg > priorBalance); + BEAST_EXPECT(liveBalanceAfterXrpLeg >= reserveForToken); + + // The XRP leg runs first, so the missing IOU trustline or MPToken + // is reserved against the updated sandbox balance. + amm.withdraw( + WithdrawArg{.account = alice_, .asset1Out = xrpOut, .asset2Out = tokenOut}); + + // The withdrawal succeeds only if the missing token holding can be + // reserved from the live balance after the XRP leg. + BEAST_EXPECT(env.ownerCount(alice_) == 3); + BEAST_EXPECT(env.balance(alice_, token).value().signum() > 0); + }; + + test([&](Env&) -> PrettyAsset { return gw_["USD"]; }); + test([&](Env& env) -> PrettyAsset { return MPTTester({.env = env, .issuer = gw_}); }); + } + void testInvalidFeeVote() { @@ -4011,9 +4070,9 @@ private: { auto jtx = env.jt(tx, Seq(1), Fee(10)); env.app().config().features.erase(featureMPTokensV2); - PreflightContext const pfctx( + PreflightContext const ctx( env.app(), *jtx.stx, env.current()->rules(), TapNone, env.journal); - auto pf = AMMBid::checkExtraFeatures(pfctx); + auto pf = AMMBid::checkExtraFeatures(ctx); BEAST_EXPECT(pf == false); env.app().config().features.insert(featureMPTokensV2); } @@ -4023,9 +4082,9 @@ private: jtx.jv["Asset2"]["currency"] = "XRP"; jtx.jv["Asset2"].removeMember("mpt_issuance_id"); jtx.stx = env.ust(jtx); - PreflightContext const pfctx( + PreflightContext const ctx( env.app(), *jtx.stx, env.current()->rules(), TapNone, env.journal); - auto pf = AMMBid::preflight(pfctx); + auto pf = AMMBid::preflight(ctx); BEAST_EXPECT(pf == temBAD_AMM_TOKENS); } } @@ -4871,7 +4930,7 @@ private: XRP(10'100), MPT(ammAlice[1])(10'000'000000000001), ammAlice.tokens())); env.require(Balance(carol_, MPT(ammAlice[1])(30'199'999999999999))); - // Initial 30,000 - 10000(AMM pool LP) - 100(AMMoffer) - + // Initial 30,000 - 10000(AMM pool LP) - 100(AMM offer) - // - 100(offer) - 10(tx fee) - 10(tx fee of MPTTester init as // holder) - one reserve BEAST_EXPECT(expectLedgerEntryRoot( @@ -7086,6 +7145,7 @@ private: testDeposit(); testInvalidWithdraw(); testWithdraw(); + testWithdrawReserveUsesLiveBalance(); testInvalidFeeVote(); testFeeVote(); testInvalidBid();