From 16b8544a8f9121ca0c9ca8aba145b507338ecb0d Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Sat, 13 Jun 2026 07:52:05 -0400 Subject: [PATCH] Fix CheckCash hard caps MPT DeliverMin at half of maxMPTokenAmount --- .../tx/transactors/check/CheckCash.cpp | 26 ++++++--- src/test/app/CheckMPT_test.cpp | 53 +++++++++++++++++++ 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/src/libxrpl/tx/transactors/check/CheckCash.cpp b/src/libxrpl/tx/transactors/check/CheckCash.cpp index c5813ae42d..c5f23624c9 100644 --- a/src/libxrpl/tx/transactors/check/CheckCash.cpp +++ b/src/libxrpl/tx/transactors/check/CheckCash.cpp @@ -16,8 +16,9 @@ #include #include #include +#include #include -#include +#include #include #include #include @@ -368,18 +369,29 @@ CheckCash::doApply() else { // Note that for DeliverMin we don't know exactly how much - // currency we want flow to deliver. We can't ask for the - // maximum possible currency because there might be a gateway - // transfer rate to account for. Since the transfer rate cannot - // exceed 200%, we use 1/2 maxValue as our limit. + // currency we want flow to deliver. For IOUs, use a value + // higher than any real delivery as the request. MPTs are + // bounded integral amounts, so use the maximum output the check + // can actually deliver without exceeding SendMax. auto const maxDeliverMin = [&]() { return optDeliverMin->asset().visit( [&](Issue const&) { return STAmount( optDeliverMin->asset(), STAmount::kMaxValue / 2, STAmount::kMaxOffset); }, - [&](MPTIssue const&) { - return STAmount(optDeliverMin->asset(), kMaxMpTokenAmount / 2); + [&](MPTIssue const& issue) { + MPTAmount maxDeliver = sendMax.mpt(); + auto const& issuer = issue.getIssuer(); + if (srcId != issuer && accountID_ != issuer) + { + auto const rate = transferRate(psb, issue.getMptID()); + // Request at most floor(SendMax / rate). The endpoint reverse pass + // will quote ceil(output * rate), so this keeps the input + // representable and within SendMax. + maxDeliver = + mulRatio(maxDeliver, QUALITY_ONE, rate.value, /*roundUp*/ false); + } + return STAmount(maxDeliver, issue); }); }; STAmount const flowDeliver{ diff --git a/src/test/app/CheckMPT_test.cpp b/src/test/app/CheckMPT_test.cpp index 861a115fc9..d4abe7f495 100644 --- a/src/test/app/CheckMPT_test.cpp +++ b/src/test/app/CheckMPT_test.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -653,6 +654,32 @@ class CheckMPT_test : public beast::unit_test::Suite BEAST_EXPECT(ownerCount(env, alice) == 1); BEAST_EXPECT(ownerCount(env, bob) == 1); } + + { + Env env{*this, features}; + + env.fund(XRP(1'000), gw, alice, bob); + + // MPT DeliverMin should not be capped at half of the legal range. + std::uint64_t constexpr deliverMin = (kMaxMpTokenAmount / 2) + 1; + MPT const usd = MPTTester( + {.env = env, .issuer = gw, .holders = {alice, bob}, .maxAmt = kMaxMpTokenAmount}); + + env(pay(gw, alice, usd(deliverMin))); + env.close(); + + uint256 const chkId{getCheckIndex(alice, env.seq(alice))}; + env(check::create(alice, bob, usd(deliverMin))); + env.close(); + + env(check::cash(bob, chkId, check::DeliverMin(usd(deliverMin)))); + verifyDeliveredAmount(env, usd(deliverMin)); + env.require(Balance(alice, usd(0))); + env.require(Balance(bob, usd(deliverMin))); + BEAST_EXPECT(checksOnAccount(env, alice).empty()); + BEAST_EXPECT(checksOnAccount(env, bob).empty()); + } + { // Examine the effects of the asfRequireAuth flag. Env env(*this, features); @@ -831,6 +858,32 @@ class CheckMPT_test : public beast::unit_test::Suite BEAST_EXPECT(checksOnAccount(env, alice).size() == 0); BEAST_EXPECT(checksOnAccount(env, bob).size() == 0); #endif + + // With the maximum transfer fee, this is the largest output whose + // fee-adjusted debit is still within SendMax. + std::uint64_t constexpr maxDeliver = (kMaxMpTokenAmount / 3) * 2; + MPT const eur = MPTTester( + {.env = env, + .issuer = gw, + .holders = {alice, bob}, + .transferFee = kMaxTransferFee, + .maxAmt = kMaxMpTokenAmount}); + + env(pay(gw, alice, eur(kMaxMpTokenAmount))); + env.close(); + + uint256 const chkIdMax{getCheckIndex(alice, env.seq(alice))}; + env(check::create(alice, bob, eur(kMaxMpTokenAmount))); + env.close(); + + // The DeliverMin cap must divide SendMax by the rate before flow() + // computes the fee-adjusted input. + env(check::cash(bob, chkIdMax, check::DeliverMin(eur(maxDeliver)))); + verifyDeliveredAmount(env, eur(maxDeliver)); + env.require(Balance(alice, eur(1))); + env.require(Balance(bob, eur(maxDeliver))); + BEAST_EXPECT(checksOnAccount(env, alice).empty()); + BEAST_EXPECT(checksOnAccount(env, bob).empty()); } void