diff --git a/include/xrpl/ledger/helpers/AMMHelpers.h b/include/xrpl/ledger/helpers/AMMHelpers.h index 7d41bfce81..a68171c426 100644 --- a/include/xrpl/ledger/helpers/AMMHelpers.h +++ b/include/xrpl/ledger/helpers/AMMHelpers.h @@ -226,7 +226,7 @@ getAMMOfferStartWithTakerGets( auto getAmounts = [&pool, &tfee](Number const& nTakerGetsProposed) { // Round downward to minimize the offer and to maximize the quality. - // This has the most impact when takerGets is XRP. + // This has the most impact when takerGets is integral. auto const takerGets = toAmount(getAsset(pool.out), nTakerGetsProposed, Number::RoundingMode::Downward); return TAmounts{swapAssetOut(pool, takerGets, tfee), takerGets}; @@ -294,7 +294,7 @@ getAMMOfferStartWithTakerPays( auto getAmounts = [&pool, &tfee](Number const& nTakerPaysProposed) { // Round downward to minimize the offer and to maximize the quality. - // This has the most impact when takerPays is XRP. + // This has the most impact when takerPays is integral. auto const takerPays = toAmount(getAsset(pool.in), nTakerPaysProposed, Number::RoundingMode::Downward); return TAmounts{takerPays, swapAssetIn(pool, takerPays, tfee)}; @@ -313,11 +313,11 @@ getAMMOfferStartWithTakerPays( * is equal to LOB quality (in this case AMM offer quality is * better than LOB quality) or AMM offer is equal to LOB quality * (in this case SPQ is better than LOB quality). - * Pre-amendment code calculates takerPays first. If takerGets is XRP, - * it is rounded down, which results in worse offer quality than - * LOB quality, and the offer might fail to generate. - * Post-amendment code calculates the XRP offer side first. The result - * is rounded down, which makes the offer quality better. + * Pre-amendment code calculates takerPays first. If takerGets is the + * economically coarser integral side, it is rounded down, which results in + * worse offer quality than LOB quality, and the offer might fail to generate. + * Post-amendment code calculates the economically coarser integral offer side + * first. The result is rounded down, which makes the offer quality better. * It might not be possible to match either SPQ or AMM offer to LOB * quality. This generally happens at higher fees. * @param pool AMM pool balances @@ -396,10 +396,18 @@ changeSpotPriceQuality( return std::nullopt; } - // Generate the offer starting with XRP side. Return seated offer amounts - // if the offer can be generated, otherwise nullopt. auto amounts = [&]() { - if (isXRP(getAsset(pool.out))) + bool const inIntegral = getAsset(pool.in).integral(); + bool const outIntegral = getAsset(pool.out).integral(); + + // Preserve historical behavior for fractional pairs and XRP/IOU-style + // one-integral-side pairs. For two integral assets, pick the side whose + // minimum unit is economically coarser at this quality. + // + // Quality::rate() is input units per output unit, so one output unit is + // coarser when it costs at least one input unit. Ties use takerGets, + // matching the historical XRP-output behavior. + if (outIntegral && (!inIntegral || Number(quality.rate()) >= 1)) return getAMMOfferStartWithTakerGets(pool, quality, tfee); return getAMMOfferStartWithTakerPays(pool, quality, tfee); }(); diff --git a/include/xrpl/ledger/helpers/MPTokenHelpers.h b/include/xrpl/ledger/helpers/MPTokenHelpers.h index 5418e5b26a..7babefd196 100644 --- a/include/xrpl/ledger/helpers/MPTokenHelpers.h +++ b/include/xrpl/ledger/helpers/MPTokenHelpers.h @@ -261,6 +261,14 @@ checkCreateMPT( xrpl::MPTIssue const& mptIssue, xrpl::AccountID const& holder, SLE::ref sponsorSle, + std::uint32_t flags, + beast::Journal j); + +TER +checkCreateMPT( + xrpl::ApplyView& view, + xrpl::MPTIssue const& mptIssue, + xrpl::AccountID const& holder, beast::Journal j); //------------------------------------------------------------------------------ diff --git a/include/xrpl/protocol/AmountConversions.h b/include/xrpl/protocol/AmountConversions.h index 3bcd80e827..ed68be62fe 100644 --- a/include/xrpl/protocol/AmountConversions.h +++ b/include/xrpl/protocol/AmountConversions.h @@ -154,7 +154,7 @@ T toAmount(Asset const& asset, Number const& n, Number::RoundingMode mode = Number::getround()) { SaveNumberRoundMode const rm(Number::getround()); - if (isXRP(asset)) + if (asset.integral()) Number::setround(mode); if constexpr (std::is_same_v) diff --git a/include/xrpl/protocol/QualityFunction.h b/include/xrpl/protocol/QualityFunction.h index 128b37ce12..4fcc730c42 100644 --- a/include/xrpl/protocol/QualityFunction.h +++ b/include/xrpl/protocol/QualityFunction.h @@ -60,6 +60,15 @@ public: std::optional outFromAvgQ(Quality const& quality); + /** + * Return whether `out` produces at least the requested + * average quality. + * @param quality requested average quality (quality limit) + * @param out output amount to test + */ + [[nodiscard]] bool + satisfiesAvgQ(Quality const& quality, Number const& out) const; + /** * Return true if the quality function is constant */ diff --git a/include/xrpl/tx/paths/detail/StrandFlow.h b/include/xrpl/tx/paths/detail/StrandFlow.h index c932c49cca..fcca97ecfc 100644 --- a/include/xrpl/tx/paths/detail/StrandFlow.h +++ b/include/xrpl/tx/paths/detail/StrandFlow.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include #include @@ -373,7 +374,7 @@ qualityUpperBound(ReadView const& v, Strand const& strand) * increases quality of AMM steps, increasing the strand's composite * quality as the result. */ -template +template inline TOutAmt limitOut( ReadView const& v, @@ -411,21 +412,29 @@ limitOut( auto const out = qf->outFromAvgQ(limitQuality); if (!out) return remainingOut; - if constexpr (std::is_same_v) + if constexpr (std::is_same_v || std::is_same_v) { - return XRPAmount{*out}; + auto const roundedOut = TOutAmt{*out}; + // Integral outputs that round above the continuous target can + // realize worse average quality than the requested limit. Keep the + // default rounded value when it still satisfies the limit, since it + // is the largest matching offer; otherwise round down. + if (v.rules().enabled(featureMPTokensV2) && roundedOut > *out && + !qf->satisfiesAvgQ(limitQuality, roundedOut)) + { + NumberRoundModeGuard const g(Number::RoundingMode::Downward); + return TOutAmt{*out}; + } + return roundedOut; } else if constexpr (std::is_same_v) { return IOUAmount{*out}; } - else if constexpr (std::is_same_v) - { - return MPTAmount{*out}; - } else { - return STAmount{remainingOut.asset(), out->mantissa(), out->exponent()}; + static constexpr bool kAlwaysFalse = !std::is_same_v; + static_assert(kAlwaysFalse, "Unhandled StepAmount type"); } }(); // A tiny difference could be due to the round off diff --git a/include/xrpl/tx/transactors/dex/AMMWithdraw.h b/include/xrpl/tx/transactors/dex/AMMWithdraw.h index 7004dd57c1..6861fa7bc4 100644 --- a/include/xrpl/tx/transactors/dex/AMMWithdraw.h +++ b/include/xrpl/tx/transactors/dex/AMMWithdraw.h @@ -118,6 +118,7 @@ public: Sandbox& view, SLE const& ammSle, AccountID const account, + std::optional const& clawbackIssuer, AccountID const& ammAccount, STAmount const& amountBalance, STAmount const& amount2Balance, @@ -138,6 +139,11 @@ public: * @param view * @param ammSle AMM ledger entry * @param ammAccount AMM account + * @param clawbackIssuer when set (AMMClawback path), the issuer performing + * the clawback. A recreated MPToken is only auto-authorized when the + * asset's issuer matches this account, so a clawback cannot grant + * authorization on behalf of a different (paired-asset) issuer. + * @param account LP account * @param amountBalance current LP asset1 balance * @param amountWithdraw asset1 withdraw amount * @param amount2Withdraw asset2 withdraw amount @@ -153,6 +159,7 @@ public: Sandbox& view, SLE const& ammSle, AccountID const& ammAccount, + std::optional const& clawbackIssuer, AccountID const& account, STAmount const& amountBalance, STAmount const& amountWithdraw, diff --git a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp index 6fe7328fa7..b239d0d3d1 100644 --- a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp @@ -952,6 +952,7 @@ checkCreateMPT( xrpl::MPTIssue const& mptIssue, xrpl::AccountID const& holder, SLE::ref sponsorSle, + std::uint32_t flags, beast::Journal j) { if (mptIssue.getIssuer() == holder) @@ -961,7 +962,7 @@ checkCreateMPT( auto const mptokenID = keylet::mptoken(mptIssuanceID.key, holder); if (!view.exists(mptokenID)) { - if (auto const err = createMPToken(view, mptIssue.getMptID(), holder, sponsorSle, 0); + if (auto const err = createMPToken(view, mptIssue.getMptID(), holder, sponsorSle, flags); !isTesSuccess(err)) { return err; @@ -977,6 +978,16 @@ checkCreateMPT( return tesSUCCESS; } +TER +checkCreateMPT( + xrpl::ApplyView& view, + xrpl::MPTIssue const& mptIssue, + xrpl::AccountID const& holder, + beast::Journal j) +{ + return checkCreateMPT(view, mptIssue, holder, {}, 0, j); +} + std::int64_t maxMPTAmount(SLE const& sleIssuance) { diff --git a/src/libxrpl/protocol/QualityFunction.cpp b/src/libxrpl/protocol/QualityFunction.cpp index e862770406..ffe583b7e1 100644 --- a/src/libxrpl/protocol/QualityFunction.cpp +++ b/src/libxrpl/protocol/QualityFunction.cpp @@ -38,7 +38,22 @@ QualityFunction::outFromAvgQ(Quality const& quality) return std::nullopt; return out; } - return std::nullopt; + // The sole caller (StrandFlow::limitOut) only invokes this on a non-const + // quality function, so m_ != 0 here, and a real payment/offer never yields + // a zero-rate limit quality (it would divide by zero above). This fallback + // is therefore unreachable in practice. + return std::nullopt; // LCOV_EXCL_LINE +} + +bool +QualityFunction::satisfiesAvgQ(Quality const& quality, Number const& out) const +{ + // satisfiesAvgQ is only reached from StrandFlow::limitOut *after* + // outFromAvgQ returned a value, which requires a non-zero rate. So a + // zero-rate quality never reaches here; this guard is defensive. + if (quality.rate() == beast::kZero) + return false; // LCOV_EXCL_LINE + return m_ * out + b_ >= 1 / quality.rate(); } } // namespace xrpl diff --git a/src/libxrpl/protocol/STAmount.cpp b/src/libxrpl/protocol/STAmount.cpp index 212c34322b..83b2983756 100644 --- a/src/libxrpl/protocol/STAmount.cpp +++ b/src/libxrpl/protocol/STAmount.cpp @@ -1445,6 +1445,59 @@ public: operator=(DontAffectNumberRoundMode const&) = delete; }; +Number::RoundingMode +roundMode(bool const resultNegative, bool const roundUp) +{ + using enum Number::RoundingMode; + // STAmount roundUp means "away from zero". The legacy scaled-mantissa + // multiply and divide paths reach that result with slightly different + // mechanics, including a final TowardsZero materialization in multiply. + // + // The MPT/V2 Number path already performs the operation under the directed + // mode below. Use the same mode again when converting back to STAmount so a + // fractional integral result stays consistently rounded after Number + // arithmetic, independent of whether the operation was multiply or divide. + return roundUp ^ resultNegative ? Upward : Downward; +} + +STAmount +roundNumberResult( + Asset const& asset, + bool const resultNegative, + bool const roundUp, + Number const& number) +{ + // MPT/V2 Number arithmetic uses directed rounding both for the operation + // and for materializing the final integral amount. + NumberRoundModeGuard const finalRound(roundMode(resultNegative, roundUp)); + auto result = STAmount{asset, number}; + [[maybe_unused]] bool const nonzeroPositiveRoundUp = + roundUp && !resultNegative && number != beast::kZero; + ALWAYS( + !nonzeroPositiveRoundUp || result != beast::kZero, + "xrpl::roundNumberResult : positive rounded-up MPT result is representable"); + + if (roundUp && !resultNegative && !result) + { + // Intended to preserve existing mulRound/divRound behavior for a + // positive result too small to represent in the target asset. + // + // Unreachable in practice: when roundUp is set, roundMode() above + // selects Upward, and materializing a Number into an STAmount honors + // that mode (Number::operator rep()), so any positive value rounds up + // to at least the smallest representable unit. Hence, a positive result + // is never !result here; the only zero case is a zero operand, which + // the mulRound/divRound callers handle before reaching this function. + // LCOV_EXCL_START + if (asset.integral()) + return STAmount{asset, 1}; + return STAmount{asset, STAmount::kMinValue, STAmount::kMinOffset, false}; + // LCOV_EXCL_STOP + } + + return result; +} + } // anonymous namespace // Pass the canonicalizeRound function pointer as a template parameter. @@ -1486,6 +1539,22 @@ mulRoundImpl(STAmount const& v1, STAmount const& v2, Asset const& asset, bool ro return STAmount(asset, minV * maxV); } + bool const resultNegative = v1.negative() != v2.negative(); + + if (asset.holds() && isFeatureEnabled(featureMPTokensV2, false)) + { + // MPT DEX can combine 63-bit MPT amounts with IOU-shaped transfer + // rates. Use Number arithmetic under MPTokensV2 so the rounded + // operation is not limited by the legacy uint64_t scaled mantissa. + Number result; + { + NumberRoundModeGuard const operationRound(roundMode(resultNegative, roundUp)); + result = Number{v1} * Number{v2}; + } + + return roundNumberResult(asset, resultNegative, roundUp, result); + } + std::uint64_t value1 = v1.mantissa(), value2 = v2.mantissa(); int offset1 = v1.exponent(), offset2 = v2.exponent(); @@ -1506,9 +1575,6 @@ mulRoundImpl(STAmount const& v1, STAmount const& v2, Asset const& asset, bool ro --offset2; } } - - bool const resultNegative = v1.negative() != v2.negative(); - // We multiply the two mantissas (each is between 10^15 // and 10^16), so their product is in the 10^30 to 10^32 // range. Dividing their product by 10^14 maintains the @@ -1575,6 +1641,22 @@ divRoundImpl(STAmount const& num, STAmount const& den, Asset const& asset, bool if (num == beast::kZero) return {asset}; + bool const resultNegative = (num.negative() != den.negative()); + + if (asset.holds() && isFeatureEnabled(featureMPTokensV2, false)) + { + // Match the multiply path above: Number performs the rounded + // operation, then STAmount materializes the final MPT amount using the + // same final rounding mode as the legacy path below. + Number result; + { + NumberRoundModeGuard const operationRound(roundMode(resultNegative, roundUp)); + result = Number{num} / Number{den}; + } + + return roundNumberResult(asset, resultNegative, roundUp, result); + } + std::uint64_t numVal = num.mantissa(), denVal = den.mantissa(); int numOffset = num.exponent(), denOffset = den.exponent(); @@ -1596,8 +1678,6 @@ divRoundImpl(STAmount const& num, STAmount const& den, Asset const& asset, bool } } - bool const resultNegative = (num.negative() != den.negative()); - // We divide the two mantissas (each is between 10^15 // and 10^16). To maintain precision, we multiply the // numerator by 10^17 (the product is in the range of diff --git a/src/libxrpl/tx/invariants/MPTInvariant.cpp b/src/libxrpl/tx/invariants/MPTInvariant.cpp index 77c5ad781e..12ec078c82 100644 --- a/src/libxrpl/tx/invariants/MPTInvariant.cpp +++ b/src/libxrpl/tx/invariants/MPTInvariant.cpp @@ -282,12 +282,13 @@ ValidMPTIssuance::finalize( "but created bad number of mptokens"; return false; } - // At most one MPToken may be created on withdraw/clawback since: + // At most two MPToken may be created on withdraw/clawback since: // - Liquidity Provider must have at least one token in order - // participate in AMM pool liquidity. + // participate in AMM pool liquidity or have LPTokens only. // - At most two MPTokens may be deleted if AMM pool, which has exactly // two tokens, is empty after withdraw/clawback. - if (mptokensCreated_ > 1 || mptokensDeleted_ > 2) + SOMETIMES(mptokensCreated_ == 2, "AMM withdraw/clawback recreated two MPTokens"); + if (mptokensCreated_ > 2 || mptokensDeleted_ > 2) { JLOG(j.fatal()) << "Invariant failed: MPT authorize succeeded " "but created/deleted bad number of mptokens"; diff --git a/src/libxrpl/tx/paths/BookStep.cpp b/src/libxrpl/tx/paths/BookStep.cpp index e7c2e9ee29..2823627108 100644 --- a/src/libxrpl/tx/paths/BookStep.cpp +++ b/src/libxrpl/tx/paths/BookStep.cpp @@ -44,7 +44,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -653,7 +655,15 @@ limitStepIn( // under an amendment. ofrAmt = offer.limitIn(ofrAmt, inLmt, /* roundUp */ false); stpAmt.out = ofrAmt.out; - ownerGives = mulRatio(ofrAmt.out, transferRateOut, QUALITY_ONE, /*roundUp*/ false); + // Round up for MPT output so the offer owner pays the full + // ceil(amount × rate) fee, matching direct Payment semantics. IOU uses + // floating-point arithmetic so the floor/ceil distinction is sub-epsilon + // there; preserve the historical false to avoid changing IOU behavior. + ownerGives = mulRatio( + ofrAmt.out, + transferRateOut, + QUALITY_ONE, + /*roundUp*/ std::is_same_v); } } @@ -672,7 +682,11 @@ limitStepOut( if (limit < stpAmt.out) { stpAmt.out = limit; - ownerGives = mulRatio(stpAmt.out, transferRateOut, QUALITY_ONE, /*roundUp*/ false); + ownerGives = mulRatio( + stpAmt.out, + transferRateOut, + QUALITY_ONE, + /*roundUp*/ std::is_same_v); ofrAmt = offer.limitOut( ofrAmt, stpAmt.out, @@ -727,17 +741,20 @@ BookStep::forEachOffer( bool const isAssetInMPT = assetIn.holds(); auto const& owner = offer.owner(); - if (isAssetInMPT) - { - // Create MPToken for the offer's owner. No need to check - // for the reserve since the offer is removed if it is consumed. - // Therefore, the owner count remains the same. - if (auto const err = checkCreateMPT(sb, assetIn.get(), owner, {}, j_); - !isTesSuccess(err)) + auto removeOffer = [&](std::string_view logMessage = {}) { + auto const key = offer.key(); + if (!logMessage.empty()) { - return true; + JLOG(j_.trace()) << logMessage << (key ? " " + to_string(*key) : ""); } - } + if (key) + offers.permRmOffer(*key); + if (!offerAttempted) + { + // Change quality only if no previous offers were tried. + ofrQ = std::nullopt; + } + }; // It shouldn't matter from auth point of view whether it's sb // or afView. Amendment guard this change just in case. @@ -745,17 +762,15 @@ BookStep::forEachOffer( // Make sure offer owner has authorization to own Assets from issuer // and MPT assets can be traded/transferred. // An account can always own XRP or their own Assets. - if (!isTesSuccess(requireAuth(applyView, assetIn, owner)) || !checkMPTDEX(sb, owner)) + // Missing MPTokens are allowed during offer discovery; they are + // created later if the offer is actually consumed. + auto const authType = isAssetInMPT ? AuthType::WeakAuth : AuthType::Legacy; + if (!isTesSuccess(requireAuth(applyView, assetIn, owner, authType)) || + !checkMPTDEX(sb, owner)) { // Offer owner not authorized to hold IOU/MPT from issuer. // Remove this offer even if no crossing occurs. - if (auto const key = offer.key()) - offers.permRmOffer(*key); - if (!offerAttempted) - { - // Change quality only if no previous offers were tried. - ofrQ = std::nullopt; - } + removeOffer(); // Returning true causes offers.step() to delete the offer. return true; } @@ -768,52 +783,88 @@ BookStep::forEachOffer( static_cast(this)->getOfrOutRate(prevStep_, owner, strandDst_, trOut)); auto ofrAmt = offer.amount(); - TAmounts stpAmt{mulRatio(ofrAmt.in, ofrInRate, QUALITY_ONE, /*roundUp*/ true), ofrAmt.out}; - - // owner pays the transfer fee. - auto ownerGives = mulRatio(ofrAmt.out, ofrOutRate, QUALITY_ONE, /*roundUp*/ false); - - auto const funds = offer.isFunded() - ? ownerGives // Offer owner is issuer; they have unlimited funds - : offers.ownerFunds(); - - // Only if CLOB offer - if (funds < ownerGives) + TAmounts stpAmt{ofrAmt.in, ofrAmt.out}; + auto ownerGives = ofrAmt.out; + try { - // We already know offer.owner()!=offer.issueOut().account - ownerGives = funds; - stpAmt.out = mulRatio(ownerGives, QUALITY_ONE, ofrOutRate, /*roundUp*/ false); - - // It turns out we can prevent order book blocking by (strictly) - // rounding down the ceil_out() result. This adjustment changes - // transaction outcomes, so it must be made under an amendment. - ofrAmt = offer.limitOut(ofrAmt, stpAmt.out, /*roundUp*/ false); - + // All arithmetic in this block runs before the offer is consumed. + // A crafted MPTokensV2 offer can overflow while transfer rates or + // crossing limits are applied; remove that unusable offer instead + // of letting it persist as a tecINTERNAL source. stpAmt.in = mulRatio(ofrAmt.in, ofrInRate, QUALITY_ONE, /*roundUp*/ true); - } - // Limit offer's input if MPT, BookStep is the first step (an issuer - // is making a cross-currency payment), and this offer is not owned - // by the issuer. Otherwise, OutstandingAmount may overflow. - auto const& issuer = assetIn.getIssuer(); - if (isAssetInMPT && !prevStep_ && offer.owner() != issuer) - { - // Funds available to issue - auto const available = toAmount(accountFunds( - sb, - issuer, - assetIn, // STAmount{0}, but the default is not used - FreezeHandling::IgnoreFreeze, - AuthHandling::IgnoreAuth, - j_)); - if (stpAmt.in > available) + // owner pays the transfer fee. + ownerGives = mulRatio( + ofrAmt.out, + ofrOutRate, + QUALITY_ONE, + /*roundUp*/ std::is_same_v); + + auto const funds = offer.isFunded() + ? ownerGives // Offer owner is issuer; they have unlimited funds + : offers.ownerFunds(); + + // Only if CLOB offer + if (funds < ownerGives) { - limitStepIn(offer, ofrAmt, stpAmt, ownerGives, ofrInRate, ofrOutRate, available); - } - } + // We already know offer.owner()!=offer.issueOut().account + ownerGives = funds; + stpAmt.out = mulRatio(ownerGives, QUALITY_ONE, ofrOutRate, /*roundUp*/ false); - offerAttempted = true; - return callback(offer, ofrAmt, stpAmt, ownerGives, ofrInRate, ofrOutRate); + // It turns out we can prevent order book blocking by (strictly) + // rounding down the ceil_out() result. This adjustment changes + // transaction outcomes, so it must be made under an amendment. + ofrAmt = offer.limitOut(ofrAmt, stpAmt.out, /*roundUp*/ false); + + stpAmt.in = mulRatio(ofrAmt.in, ofrInRate, QUALITY_ONE, /*roundUp*/ true); + } + + // Limit offer's input if MPT, BookStep is the first step (an issuer + // is making a cross-currency payment), and this offer is not owned + // by the issuer. Otherwise, OutstandingAmount may overflow. + auto const& issuer = assetIn.getIssuer(); + if (isAssetInMPT && !prevStep_ && offer.owner() != issuer) + { + // Funds available to issue + auto const available = toAmount(accountFunds( + sb, + issuer, + assetIn, // STAmount{0}, but the default is not used + FreezeHandling::IgnoreFreeze, + AuthHandling::IgnoreAuth, + j_)); + if (stpAmt.in > available) + { + limitStepIn( + offer, ofrAmt, stpAmt, ownerGives, ofrInRate, ofrOutRate, available); + } + } + + offerAttempted = true; + return callback(offer, ofrAmt, stpAmt, ownerGives, ofrInRate, ofrOutRate); + } + catch (std::overflow_error const&) + { + if (sb.rules().enabled(featureMPTokensV2)) + { + SOMETIMES( + true, + "BookStep::forEachOffer removed MPT offer after " + "overflow during crossing"); + removeOffer("Removing offer with overflowing amount calculation"); + return true; + } + // An overflow can only be produced by a crafted MPT offer, and MPT + // offers require featureMPTokensV2 (enforced at OfferCreate + // preflight). So the amendment is always enabled when we get here + // and this legacy re-throw is unreachable in practice. + // LCOV_EXCL_START + XRPL_ASSERT( + sb.rules().enabled(featureMPTokensV2), + "xrpl::BookStep::forEachOffer : overflow implies MPTokensV2"); + throw; + // LCOV_EXCL_STOP + } }; // At any payment engine iteration, AMM offer can only be consumed once. @@ -873,6 +924,22 @@ BookStep::consumeOffer( // The offer owner gets the ofrAmt. The difference between ofrAmt and // stepAmt is a transfer fee that goes to book_.in.account { + if constexpr (std::is_same_v) + { + // If the offer's TakerPays asset is an MPT, the offer owner must + // hold an MPToken to receive it. Create one here if it doesn't + // already exist. + if (auto const err = checkCreateMPT(sb, book_.in.get(), offer.owner(), j_); + !isTesSuccess(err)) + { + // checkCreateMPT only fails on tecDIR_FULL (its source line is + // itself LCOV-excluded) or a missing offer-owner account, which + // cannot happen since that account owns the offer being + // consumed. Defensive and unreachable in practice. + Throw(err); // LCOV_EXCL_LINE + } + } + auto const dr = offer.send( sb, book_.in.getIssuer(), offer.owner(), toSTAmount(ofrAmt.in, book_.in), j_); if (!isTesSuccess(dr)) @@ -1043,6 +1110,13 @@ BookStep::revImp( auto ofrAdjAmt = ofrAmt; auto stpAdjAmt = stpAmt; auto ownerGivesAdj = ownerGives; + // This reduction can overflow via the transfer-rate mulRatio() on a + // 63-bit MPT amount (IOU rescales instead of throwing, and XRP stays + // under the int64 limit, so only MPT reaches it today), but + // savedIns/savedOuts are not updated until after it succeeds. The outer + // execOffer() catch can therefore remove the offer under + // featureMPTokensV2 (legacy propagate-the-exception behavior otherwise) + // without rolling back local state. limitStepOut( offer, ofrAdjAmt, @@ -1144,12 +1218,25 @@ BookStep::fwdImp( auto stpAdjAmt = stpAmt; auto ownerGivesAdj = ownerGives; + // limitStepIn()/limitStepOut() can throw std::overflow_error from the + // transfer-rate mulRatio() on a 63-bit MPT amount. (IOUAmount::mulRatio + // rescales rather than throwing, and XRP amounts/rates stay under the + // int64 limit, so in practice only MPT reaches this today.) execOffer() + // catches it: under featureMPTokensV2 the offending offer is removed; + // otherwise the legacy behavior (propagate the exception) is preserved. + // Keep candidate accumulator changes local until those calls succeed so + // the catch path does not observe partially updated state. Re-sum the + // staged sets to preserve historical flat_multiset summing behavior. + auto savedInsAdj = savedIns; + auto savedOutsAdj = savedOuts; + auto resultAdj = result; typename boost::container::flat_multiset::const_iterator lastOut; + if (stpAmt.in <= remainingIn) { - savedIns.insert(stpAmt.in); - lastOut = savedOuts.insert(stpAmt.out); - result = TAmounts(sum(savedIns), sum(savedOuts)); + savedInsAdj.insert(stpAmt.in); + lastOut = savedOutsAdj.insert(stpAmt.out); + resultAdj = TAmounts(sum(savedInsAdj), sum(savedOutsAdj)); // consume the offer even if stepAmt.in == remainingIn processMore = true; } @@ -1163,15 +1250,15 @@ BookStep::fwdImp( transferRateIn, transferRateOut, remainingIn); - savedIns.insert(remainingIn); - lastOut = savedOuts.insert(stpAdjAmt.out); - result.out = sum(savedOuts); - result.in = in; + savedInsAdj.insert(remainingIn); + lastOut = savedOutsAdj.insert(stpAdjAmt.out); + resultAdj.out = sum(savedOutsAdj); + resultAdj.in = in; processMore = false; } - if (result.out > cache_->out && result.in <= cache_->in) + if (resultAdj.out > cache_->out && resultAdj.in <= cache_->in) { // The step produced more output in the forward pass than the // reverse pass while consuming the same input (or less). If we @@ -1181,8 +1268,8 @@ BookStep::fwdImp( // input provided in the forward step and produce the output // requested from the reverse step. auto const lastOutAmt = *lastOut; - savedOuts.erase(lastOut); - auto const remainingOut = cache_->out - sum(savedOuts); + savedOutsAdj.erase(lastOut); + auto const remainingOut = cache_->out - sum(savedOutsAdj); auto ofrAdjAmtRev = ofrAmt; auto stpAdjAmtRev = stpAmt; auto ownerGivesAdjRev = ownerGives; @@ -1197,13 +1284,13 @@ BookStep::fwdImp( if (stpAdjAmtRev.in == remainingIn) { - result.in = in; - result.out = cache_->out; + resultAdj.in = in; + resultAdj.out = cache_->out; - savedIns.clear(); - savedIns.insert(result.in); - savedOuts.clear(); - savedOuts.insert(result.out); + savedInsAdj.clear(); + savedInsAdj.insert(resultAdj.in); + savedOutsAdj.clear(); + savedOutsAdj.insert(resultAdj.out); ofrAdjAmt = ofrAdjAmtRev; stpAdjAmt.in = remainingIn; @@ -1214,10 +1301,15 @@ BookStep::fwdImp( { // This is (likely) a problem case, and will be caught // with later checks - savedOuts.insert(lastOutAmt); + savedOutsAdj.insert(lastOutAmt); } } + // Commit the staged accounting only after limitStepIn()/limitStepOut() + // have succeeded. + savedIns = std::move(savedInsAdj); + savedOuts = std::move(savedOutsAdj); + result = resultAdj; remainingIn = in - result.in; this->consumeOffer(sb, offer, ofrAdjAmt, stpAdjAmt, ownerGivesAdj); diff --git a/src/libxrpl/tx/paths/MPTEndpointStep.cpp b/src/libxrpl/tx/paths/MPTEndpointStep.cpp index 0a0f6a9f27..a47cfa15a5 100644 --- a/src/libxrpl/tx/paths/MPTEndpointStep.cpp +++ b/src/libxrpl/tx/paths/MPTEndpointStep.cpp @@ -410,8 +410,7 @@ MPTEndpointOfferCrossingStep::checkCreateMPT(ApplyView& view, xrpl::DebtDirectio // for the reserve since the offer doesn't go on the books // if crossed. Insufficient reserve is allowed if the offer // crossed. See CreateOffer::applyGuts() for reserve check. - if (auto const err = xrpl::checkCreateMPT(view, mptIssue_, dst_, {}, j_); - !isTesSuccess(err)) + if (auto const err = xrpl::checkCreateMPT(view, mptIssue_, dst_, j_); !isTesSuccess(err)) { JLOG(j_.trace()) << "MPTEndpointStep::checkCreateMPT: failed create MPT"; resetCache(srcDebtDir); diff --git a/src/libxrpl/tx/paths/OfferStream.cpp b/src/libxrpl/tx/paths/OfferStream.cpp index ecc8416a2b..2f2fef49f0 100644 --- a/src/libxrpl/tx/paths/OfferStream.cpp +++ b/src/libxrpl/tx/paths/OfferStream.cpp @@ -29,6 +29,8 @@ #include #include +#include +#include namespace xrpl { @@ -136,17 +138,17 @@ template TOfferStreamBase::shouldRmSmallIncreasedQOffer() const { // Consider removing the offer if: - // o `TakerPays` is XRP (because of XRP drops granularity) or + // o `TakerPays` is integral (because XRP/MPT have indivisible units) or // o `TakerPays` and `TakerGets` are both IOU and `TakerPays`<`TakerGets` - static constexpr bool kInIsXrp = std::is_same_v; - static constexpr bool kOutIsXrp = std::is_same_v; + constexpr bool const kInIsIntegral = !std::is_same_v; + constexpr bool const kOutIsIntegral = !std::is_same_v; - if constexpr (kOutIsXrp) + if constexpr (!kInIsIntegral && kOutIsIntegral) { - // If `TakerGets` is XRP, the worst this offer's quality can change is - // to about 10^-81 `TakerPays` and 1 drop `TakerGets`. This will be - // remarkably good quality for any realistic asset, so these offers - // don't need this extra check. + // If only `TakerGets` is integral, the worst this offer's quality can + // change is to about 10^-81 `TakerPays` and 1 unit `TakerGets`. This + // will be perfect quality for any realistic asset, so these + // offers don't need this extra check. return false; } @@ -156,7 +158,7 @@ TOfferStreamBase::shouldRmSmallIncreasedQOffer() const TAmounts const ofrAmts{ toAmount(offer_.amount().in), toAmount(offer_.amount().out)}; - if constexpr (!kInIsXrp && !kOutIsXrp) + if constexpr (!kInIsIntegral && !kOutIsIntegral) { if (Number(ofrAmts.in) >= Number(ofrAmts.out)) return false; @@ -165,7 +167,12 @@ TOfferStreamBase::shouldRmSmallIncreasedQOffer() const TTakerGets const ownerFunds = toAmount(*ownerFunds_); auto const effectiveAmounts = [&] { - if (offer_.owner() != offer_.assetOut().getIssuer() && ownerFunds < ofrAmts.out) + // Issuer-owned IOU offers are self-funded without a limit. MPT issuer + // offers are bounded by remaining issuance capacity, so they still need + // to be clipped by ownerFunds. + bool const issuerHasUnlimitedFunds = offer_.owner() == offer_.assetOut().getIssuer() && + offer_.assetOut().template holds(); + if (!issuerHasUnlimitedFunds && ownerFunds < ofrAmts.out) { // adjust the amounts by owner funds. // @@ -305,7 +312,41 @@ TOfferStreamBase::step() continue; } - if (shouldRmSmallIncreasedQOffer()) + // Partially funded offers can be reduced before BookStep sees them. + // If that strict reduction overflows under MPTokensV2, remove the + // unusable offer instead of leaving it at the book tip. + bool shouldRemoveSmallIncreasedQOffer = false; + try + { + shouldRemoveSmallIncreasedQOffer = shouldRmSmallIncreasedQOffer(); + } + catch (std::overflow_error const&) + { + if (view_.rules().enabled(featureMPTokensV2)) + { + SOMETIMES( + true, + "OfferStream::step removed MPT offer with overflowing " + "reduced quality"); + permRmOffer(entry->key()); + JLOG(j_.warn()) << "Removing offer with overflowing reduced quality " + << entry->key(); + offer_ = TOffer{}; + continue; + } + // The strict reduction only overflows for a crafted MPT offer, and + // MPT offers require featureMPTokensV2 (enforced at OfferCreate + // preflight). So the amendment is always enabled here and this + // legacy re-throw is unreachable in practice. + // LCOV_EXCL_START + XRPL_ASSERT( + view_.rules().enabled(featureMPTokensV2), + "xrpl::TOfferStreamBase::step : overflow implies MPTokensV2"); + throw; + // LCOV_EXCL_STOP + } + + if (shouldRemoveSmallIncreasedQOffer) { auto const originalFunds = accountFundsHelper( cancelView_, diff --git a/src/libxrpl/tx/transactors/check/CheckCash.cpp b/src/libxrpl/tx/transactors/check/CheckCash.cpp index e4d8f192c0..857f759752 100644 --- a/src/libxrpl/tx/transactors/check/CheckCash.cpp +++ b/src/libxrpl/tx/transactors/check/CheckCash.cpp @@ -528,7 +528,7 @@ CheckCash::doApply() return tecINSUFFICIENT_RESERVE; if (auto const err = - checkCreateMPT(psb, mptID, accountID_, *sponsorSle, j_); + checkCreateMPT(psb, mptID, accountID_, *sponsorSle, 0, j_); !isTesSuccess(err)) { return err; diff --git a/src/libxrpl/tx/transactors/dex/AMMClawback.cpp b/src/libxrpl/tx/transactors/dex/AMMClawback.cpp index 455b2ad5c5..e690cd7693 100644 --- a/src/libxrpl/tx/transactors/dex/AMMClawback.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMClawback.cpp @@ -227,6 +227,7 @@ AMMClawback::applyGuts(Sandbox& sb) sb, *ammSle, holder, + issuer, ammAccount, amountBalance, amount2Balance, @@ -311,6 +312,14 @@ AMMClawback::equalWithdrawMatchingOneAmount( STAmount const& holdLPtokens, STAmount const& amount) { + // The clawback issuer signs for its own asset only. Threaded into the + // withdrawal so a recreated MPToken is auto-authorized only for the + // clawback issuer's asset, never for a paired asset from another issuer. + // preflight guarantees sfAccount is the clawed asset's issuer (it rejects + // the tx as temMALFORMED when sfAsset's issuer != sfAccount), so this is + // the issuer, not just any signer. + AccountID const issuer = ctx_.tx[sfAccount]; + auto frac = Number{amount} / amountBalance; auto amount2Withdraw = amount2Balance * frac; @@ -324,6 +333,7 @@ AMMClawback::equalWithdrawMatchingOneAmount( sb, ammSle, holder, + issuer, ammAccount, amountBalance, amount2Balance, @@ -364,6 +374,7 @@ AMMClawback::equalWithdrawMatchingOneAmount( sb, ammSle, ammAccount, + issuer, holder, amountBalance, amountRounded, @@ -384,6 +395,7 @@ AMMClawback::equalWithdrawMatchingOneAmount( sb, ammSle, ammAccount, + issuer, holder, amountBalance, amount, diff --git a/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp b/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp index 5294dd0c7f..edd2cc2037 100644 --- a/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMWithdraw.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -516,6 +517,7 @@ AMMWithdraw::withdraw( view, ammSle, ammAccount, + std::nullopt, accountID_, amountBalance, amountWithdraw, @@ -536,6 +538,7 @@ AMMWithdraw::withdraw( Sandbox& view, SLE const& ammSle, AccountID const& ammAccount, + std::optional const& clawbackIssuer, AccountID const& account, STAmount const& amountBalance, STAmount const& amountWithdraw, @@ -703,14 +706,48 @@ AMMWithdraw::withdraw( if (mptokenKey && account != asset.getIssuer()) { auto const& mptIssue = asset.get(); + std::uint32_t createFlags = 0; if (auto const err = requireAuth(view, mptIssue, account, AuthType::WeakAuth); !isTesSuccess(err)) - return err; + { + if (authHandling != AuthHandling::IgnoreAuth || err != tecNO_AUTH) + { + // Unreachable in practice. Normal withdraws (authHandling + // != IgnoreAuth) are rejected for unauthorized holders in + // preclaim, so they never get here. Under clawback + // (IgnoreAuth) requireAuth returns a non-tecNO_AUTH error + // (e.g. tecEXPIRED) only for a domain-authorized MPT, but no + // such MPT can be in an AMM pool: a directly domain-gated + // RequireAuth MPT fails AMMCreate/deposit with tecNO_AUTH, + // and vault shares (whose recursive auth could yield + // tecEXPIRED) are rejected by AMMCreate with tecWRONG_ASSET. + return err; // LCOV_EXCL_LINE + } - if (auto const err = checkCreateMPT(view, mptIssue, account, {}, journal); + // AMMClawback ignores authorization so the issuer can recover + // MPT locked in the pool even if the holder deleted their + // MPToken. Only auto-authorize the recreated MPToken for the + // clawback issuer's own asset: authorization is granted by an + // asset's issuer, and the clawback transaction is signed by + // that issuer only for its own asset. For a paired asset issued + // by a different account, recreate the MPToken *unauthorized* so + // the clawback does not grant authorization on behalf of that + // issuer (which would bypass its lsfMPTRequireAuth). The holder + // still receives the paired asset (accountSend only requires the + // MPToken to exist, not to be authorized); the balance remains + // gated by its issuer until that issuer authorizes it. + if (clawbackIssuer && asset.getIssuer() == *clawbackIssuer) + createFlags = lsfMPTAuthorized; + } + + if (auto const err = checkCreateMPT(view, mptIssue, account, {}, createFlags, journal); !isTesSuccess(err)) { - return err; + // checkCreateMPT only fails on tecDIR_FULL (its source line is + // itself LCOV-excluded) or a missing account, which cannot + // happen since `account` is the withdrawing LP. Defensive and + // unreachable in practice. + return err; // LCOV_EXCL_LINE } } return tesSUCCESS; @@ -804,6 +841,7 @@ AMMWithdraw::equalWithdrawTokens( view, ammSle, accountID_, + std::nullopt, ammAccount, amountBalance, amount2Balance, @@ -856,6 +894,7 @@ AMMWithdraw::equalWithdrawTokens( Sandbox& view, SLE const& ammSle, AccountID const account, + std::optional const& clawbackIssuer, AccountID const& ammAccount, STAmount const& amountBalance, STAmount const& amount2Balance, @@ -878,6 +917,7 @@ AMMWithdraw::equalWithdrawTokens( view, ammSle, ammAccount, + clawbackIssuer, account, amountBalance, amountBalance, @@ -913,6 +953,7 @@ AMMWithdraw::equalWithdrawTokens( view, ammSle, ammAccount, + clawbackIssuer, account, amountBalance, amountWithdraw, diff --git a/src/test/app/AMMClawbackMPT_test.cpp b/src/test/app/AMMClawbackMPT_test.cpp index 6c7aa99156..1d75c4db22 100644 --- a/src/test/app/AMMClawbackMPT_test.cpp +++ b/src/test/app/AMMClawbackMPT_test.cpp @@ -16,6 +16,8 @@ #include #include #include +#include +#include #include #include #include @@ -1476,6 +1478,60 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite } } + void + testClawbackCreatesMissingMPToken(FeatureBitset features) + { + testcase("test AMMClawback creates missing MPToken"); + using namespace jtx; + + auto test = [&](std::optional const clawAmount) { + Env env{*this, features}; + Account const gw{"gateway"}; + Account const alice{"alice"}; + env.fund(XRP(1'000'000), gw, alice); + env.close(); + + MPTTester token( + {.env = env, + .issuer = gw, + .holders = {alice}, + .pay = 1'000, + .flags = tfMPTCanClawback | tfMPTRequireAuth | kMptDexFlags, + .authHolder = true}); + + AMM ammAlice(env, alice, token(1'000), XRP(1'000)); + env.close(); + BEAST_EXPECT(env.balance(alice, token) == token(0)); + + // The holder can delete the zero-balance MPToken while still + // holding LP tokens. A regular AMMWithdraw remains subject to + // RequireAuth and cannot recreate the missing token. + token.authorize({.account = alice, .flags = tfMPTUnauthorize}); + env.close(); + BEAST_EXPECT(!env.le(keylet::mptoken(token.issuanceID(), alice.id()))); + ammAlice.withdrawAll(alice, std::nullopt, Ter(tecNO_AUTH)); + env.close(); + BEAST_EXPECT(!env.le(keylet::mptoken(token.issuanceID(), alice.id()))); + + // AMMClawback ignores authorization and must be able to recreate + // the holder MPToken so the issuer can recover MPT from the pool. + std::optional amount; + if (clawAmount) + amount = token(*clawAmount); + env(amm::ammClawback(gw, alice, token, XRP, amount)); + env.close(); + + auto const sleMpt = env.le(keylet::mptoken(token.issuanceID(), alice.id())); + BEAST_EXPECT(sleMpt && sleMpt->isFlag(lsfMPTAuthorized)); + env.require(Balance(alice, token(0))); + + BEAST_EXPECT(clawAmount ? ammAlice.ammExists() : !ammAlice.ammExists()); + }; + + test(std::nullopt); + test(400); + } + void testSingleDepositAndClawback(FeatureBitset features) { @@ -1949,6 +2005,199 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite } } + // Test that AMMClawback succeeds when the LP has previously deleted both + // zero-balance MPToken objects in an MPT/MPT pool. The fix changes the + // ValidMPTIssuance invariant threshold from > 1 to > 2 so that the two + // MPToken creations triggered by the internal AMMWithdraw are permitted. + void + testClawbackAfterDeletingMPTokens(FeatureBitset features) + { + testcase("test AMMClawback after holder deletes zero-balance MPTokens"); + using namespace jtx; + + // Partial clawback (one asset): verify both MPTokens are recreated and + // the non-claw asset is returned to alice. + { + Env env(*this, features); + Account const gw{"gateway"}; + Account const alice{"alice"}; + env.fund(XRP(100'000), gw, alice); + env.close(); + + MPTTester btc( + {.env = env, + .issuer = gw, + .holders = {alice}, + .pay = 10'000, + .flags = tfMPTCanClawback | kMptDexFlags}); + + MPTTester eth( + {.env = env, + .issuer = gw, + .holders = {alice}, + .pay = 10'000, + .flags = tfMPTCanClawback | kMptDexFlags}); + + // Alice deposits everything into the MPT/MPT pool; her MPT + // balances drop to zero. + AMM const amm(env, alice, btc(10'000), eth(10'000)); + env.close(); + BEAST_EXPECT(amm.expectBalances(btc(10'000), eth(10'000), IOUAmount{10'000})); + + auto aliceBTC = env.balance(alice, btc); + auto aliceETH = env.balance(alice, eth); + BEAST_EXPECT(aliceBTC == btc(0)); + BEAST_EXPECT(aliceETH == eth(0)); + + // Alice deletes both zero-balance MPTokens to reclaim reserves. + btc.authorize({.account = alice, .flags = tfMPTUnauthorize}); + eth.authorize({.account = alice, .flags = tfMPTUnauthorize}); + BEAST_EXPECT(!env.le(keylet::mptoken(btc.issuanceID(), alice.id()))); + BEAST_EXPECT(!env.le(keylet::mptoken(eth.issuanceID(), alice.id()))); + + // gw claws back some BTC from alice's share in the pool. + // AMMWithdraw internally creates both missing MPTokens + // (mptokensCreated_ == 2); the invariant (> 2) allows this. + env(amm::ammClawback(gw, alice, btc, eth, btc(1'000))); + env.close(); + + // Both MPToken objects must have been recreated. + BEAST_EXPECT(env.le(keylet::mptoken(btc.issuanceID(), alice.id()))); + BEAST_EXPECT(env.le(keylet::mptoken(eth.issuanceID(), alice.id()))); + + // The non-claw asset (eth) was returned to alice. + BEAST_EXPECT(env.balance(alice, eth) > aliceETH); + // The claw asset (btc) was burned; alice's btc balance stays 0. + env.require(Balance(alice, aliceBTC)); + BEAST_EXPECT(amm.ammExists()); + } + + // Full clawback (two assets, tfClawTwoAssets): verify both MPTokens + // are recreated and the AMM is deleted when fully drained. + { + Env env(*this, features); + Account const gw{"gateway"}; + Account const alice{"alice"}; + env.fund(XRP(100'000), gw, alice); + env.close(); + + MPTTester btc( + {.env = env, + .issuer = gw, + .holders = {alice}, + .pay = 10'000, + .flags = tfMPTCanClawback | kMptDexFlags}); + + MPTTester eth( + {.env = env, + .issuer = gw, + .holders = {alice}, + .pay = 10'000, + .flags = tfMPTCanClawback | kMptDexFlags}); + + AMM const amm(env, alice, btc(10'000), eth(10'000)); + env.close(); + + auto aliceBTC = env.balance(alice, btc); + auto aliceETH = env.balance(alice, eth); + + btc.authorize({.account = alice, .flags = tfMPTUnauthorize}); + eth.authorize({.account = alice, .flags = tfMPTUnauthorize}); + BEAST_EXPECT(!env.le(keylet::mptoken(btc.issuanceID(), alice.id()))); + BEAST_EXPECT(!env.le(keylet::mptoken(eth.issuanceID(), alice.id()))); + + // Full two-asset clawback: both assets are clawed and alice + // receives nothing back. The AMM should be empty and deleted. + env(amm::ammClawback(gw, alice, btc, eth, std::nullopt), Txflags(tfClawTwoAssets)); + env.close(); + + BEAST_EXPECT(!amm.ammExists()); + // Both assets were clawed; alice's balances remain at zero. + env.require(Balance(alice, aliceBTC)); + env.require(Balance(alice, aliceETH)); + } + } + + void + testClawbackCrossIssuerPairedAssetAuth(FeatureBitset features) + { + testcase("test AMMClawback recreates paired-issuer MPToken unauthorized"); + using namespace jtx; + + // Cross-issuer MPT/MPT pool: btc is issued by gw, eth by gw2, and both + // require authorization. Alice deposits her entire balance of both and + // deletes the resulting zero-balance MPTokens. When gw claws back its + // own asset (btc), the two-asset withdrawal must recreate both of + // Alice's MPTokens so the pool can pay her the paired asset. The + // recreated MPToken may only be auto-authorized for the clawback + // issuer's own asset (btc); the paired asset's issuer (gw2) never + // consented, so eth must be recreated *unauthorized*, leaving gw2 in + // control of its own token and preserving its RequireAuth guarantee. + Env env(*this, features); + Account const gw{"gateway"}; + Account const gw2{"gateway2"}; + Account const alice{"alice"}; + env.fund(XRP(100'000), gw, gw2, alice); + env.close(); + + MPTTester btc( + {.env = env, + .issuer = gw, + .holders = {alice}, + .pay = 10'000, + .flags = tfMPTCanClawback | tfMPTRequireAuth | kMptDexFlags, + .authHolder = true}); + + MPTTester eth( + {.env = env, + .issuer = gw2, + .holders = {alice}, + .pay = 10'000, + .flags = tfMPTCanClawback | tfMPTRequireAuth | kMptDexFlags, + .authHolder = true}); + + // Alice deposits everything into the pool; her MPT balances drop to 0. + AMM const amm(env, alice, btc(10'000), eth(10'000)); + env.close(); + BEAST_EXPECT(amm.expectBalances(btc(10'000), eth(10'000), IOUAmount{10'000})); + BEAST_EXPECT(env.balance(alice, btc) == btc(0)); + BEAST_EXPECT(env.balance(alice, eth) == eth(0)); + + // Alice deletes both zero-balance MPTokens to reclaim reserves. + btc.authorize({.account = alice, .flags = tfMPTUnauthorize}); + eth.authorize({.account = alice, .flags = tfMPTUnauthorize}); + BEAST_EXPECT(!env.le(keylet::mptoken(btc.issuanceID(), alice.id()))); + BEAST_EXPECT(!env.le(keylet::mptoken(eth.issuanceID(), alice.id()))); + + // gw (issuer of btc) claws back part of Alice's btc. This is a + // cross-issuer pool, so tfClawTwoAssets is not permitted: only btc is + // clawed back, while the paired eth is returned to Alice. + env(amm::ammClawback(gw, alice, btc, eth, btc(1'000))); + env.close(); + + // Both MPTokens were recreated so the withdrawal could pay Alice. + auto const sleBtc = env.le(keylet::mptoken(btc.issuanceID(), alice.id())); + auto const sleEth = env.le(keylet::mptoken(eth.issuanceID(), alice.id())); + BEAST_EXPECT(sleBtc); + BEAST_EXPECT(sleEth); + + // The clawback issuer's own asset (btc) may be recreated authorized: + // gw has authority over its own token. + BEAST_EXPECT(sleBtc && sleBtc->isFlag(lsfMPTAuthorized)); + + // The paired asset (eth) is issued by gw2, who did not sign this + // transaction. It must be recreated *unauthorized* so gw2's RequireAuth + // is not bypassed. This is the core assertion for the cross-issuer fix. + BEAST_EXPECT(sleEth && !sleEth->isFlag(lsfMPTAuthorized)); + + // The clawback still completed: btc was clawed back (Alice keeps a zero + // btc balance) and the paired eth was delivered into Alice's now + // unauthorized, gw2-gated MPToken (non-zero raw balance). + BEAST_EXPECT(sleBtc && sleBtc->getFieldU64(sfMPTAmount) == 0); + BEAST_EXPECT(sleEth && sleEth->getFieldU64(sfMPTAmount) > 0); + BEAST_EXPECT(amm.ammExists()); + } + void run() override { @@ -1965,6 +2214,9 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite testAMMClawbackAllSameIssuer(all); testAMMClawbackIssuesEachOther(all); testAssetFrozenOrLocked(all); + testClawbackCreatesMissingMPToken(all); + testClawbackAfterDeletingMPTokens(all); + testClawbackCrossIssuerPairedAssetAuth(all); testSingleDepositAndClawback(all); testLastHolderLPTokenBalance(all); testLastHolderLPTokenBalance(all - fixAMMv1_3 - fixAMMClawbackRounding); diff --git a/src/test/app/AMMExtendedMPT_test.cpp b/src/test/app/AMMExtendedMPT_test.cpp index f04ea39f2b..5059128d4b 100644 --- a/src/test/app/AMMExtendedMPT_test.cpp +++ b/src/test/app/AMMExtendedMPT_test.cpp @@ -188,20 +188,28 @@ private: {features}); // tfPassive -- place the offer without crossing it. - testAMM( - [&](AMM& ammAlice, Env& env) { - // Carol creates a passive offer that could cross AMM. - // Carol's offer should stay in the ledger. - auto const& btc = MPT(ammAlice[1]); - env(offer(carol_, XRP(100), btc(100), tfPassive)); - env.close(); - BEAST_EXPECT(ammAlice.expectBalances(XRP(10'100), btc(10'000), ammAlice.tokens())); - BEAST_EXPECT(expectOffers(env, carol_, 1, {{{XRP(100), btc(100)}}})); - }, - {{XRP(10'100), gAmmmpt(10'000)}}, - 0, - std::nullopt, - {features}); + { + Env env{*this, features}; + fund(env, gw_, {alice_, carol_}, XRP(30'000'000)); + + MPTTester const btc( + {.env = env, + .issuer = gw_, + .holders = {alice_, carol_}, + .pay = 30'000'000, + .flags = kMptDexFlags}); + + AMM const ammAlice(env, alice_, XRP(10'100'000), btc(10'000'000)); + + // Scale the exact-quality fixture up so the visual relationship + // stays clear: the passive CLOB offer has the same 1:1 quality as + // the generated AMM offer, so it should not cross. + env(offer(carol_, XRP(100'000), btc(100'000), tfPassive)); + env.close(); + BEAST_EXPECT( + ammAlice.expectBalances(XRP(10'100'000), btc(10'000'000), ammAlice.tokens())); + BEAST_EXPECT(expectOffers(env, carol_, 1, {{{XRP(100'000), btc(100'000)}}})); + } // tfPassive -- cross only offers of better quality. testAMM( @@ -1084,9 +1092,9 @@ private: // AMM is consumed up to the first cam Offer quality BEAST_EXPECT(ammCarol.expectBalances( - aBux(3'093'541'659'651'604), bBux(3'200'215'509'984'418), ammCarol.tokens())); + aBux(3'093'541'659'651'603), bBux(3'200'215'509'984'419), ammCarol.tokens())); BEAST_EXPECT(expectOffers( - env, cam, 1, {{Amounts{bBux(200'215'509'984'418), aBux(200'215'509'984'419)}}})); + env, cam, 1, {{Amounts{bBux(200'215'509'984'419), aBux(200'215'509'984'419)}}})); } void @@ -1241,7 +1249,7 @@ private: BEAST_EXPECT(sa == XRP(100'000'000)); // Bob gets ~99.99e12ETH. This is the amount Bob // can get out of AMM for 100,000,000XRP. - BEAST_EXPECT(equal(da, eth(99'999'900'000'100))); + BEAST_EXPECT(equal(da, eth(99'999'900'000'099))); } // carol holds ETH, sells ETH for XRP @@ -1505,6 +1513,96 @@ private: } } + void + pathFindMPTAMMExecutableSourceAmount() + { + testcase("Path Find: MPT AMM source amount is executable"); + using namespace jtx; + + auto const checkQuote = [&](std::int64_t usdPool, + std::int64_t eurPool, + std::int64_t deliverAmount, + std::int64_t expectedSourceAmount) { + Env env = pathTestEnv(); + env.fund(XRP(30'000), gw_, alice_, bob_, carol_); + env.close(); + + MPTTester const usd( + {.env = env, + .issuer = gw_, + .holders = {alice_, bob_, carol_}, + .pay = usdPool, + .flags = kMptDexFlags}); + + MPTTester const eur( + {.env = env, + .issuer = gw_, + .holders = {alice_, bob_, carol_}, + .pay = eurPool, + .flags = kMptDexFlags}); + + AMM const ammCarol(env, carol_, usd(usdPool), eur(eurPool)); + env.close(); + + STPathSet st; + STAmount sa, da; + auto const deliver = eur(deliverAmount); + std::tie(st, sa, da) = findPaths( + env, + alice_, + bob_, + deliver, + std::nullopt, + usd.issuanceID(), + std::nullopt, + std::nullopt); + + // Each quote must execute when used as an exact-output SendMax. + BEAST_EXPECT(equal(da, deliver)); + BEAST_EXPECT(equal(sa, usd(expectedSourceAmount))); + BEAST_EXPECT(!st.empty()); + + auto const before = eur.getBalance(bob_); + env(pay(alice_, bob_, deliver), + Json(jss::Paths, st.getJson(JsonOptions::Values::None)), + Sendmax(sa), + Txflags(tfNoRippleDirect)); + BEAST_EXPECT(eur.getBalance(bob_) == before + deliverAmount); + }; + + struct TestCase + { + std::int64_t usdPool; + std::int64_t eurPool; + std::int64_t deliverAmount; + std::int64_t expectedSourceAmount; + }; + + // Cover the original 2:1 pool and the same pool scaled down by 1000. + // clang-format off + TestCase const testCases[] = { + {.usdPool = 2'000'000, .eurPool = 1'000'000, .deliverAmount = 1, .expectedSourceAmount = 3}, + {.usdPool = 2'000'000, .eurPool = 1'000'000, .deliverAmount = 2, .expectedSourceAmount = 5}, + {.usdPool = 2'000'000, .eurPool = 1'000'000, .deliverAmount = 10, .expectedSourceAmount = 21}, + {.usdPool = 2'000'000, .eurPool = 1'000'000, .deliverAmount = 100, .expectedSourceAmount = 201}, + {.usdPool = 2'000'000, .eurPool = 1'000'000, .deliverAmount = 1'000, .expectedSourceAmount = 2'003}, + {.usdPool = 2'000, .eurPool = 1'000, .deliverAmount = 1, .expectedSourceAmount = 3}, + {.usdPool = 2'000, .eurPool = 1'000, .deliverAmount = 2, .expectedSourceAmount = 5}, + {.usdPool = 2'000, .eurPool = 1'000, .deliverAmount = 10, .expectedSourceAmount = 21}, + {.usdPool = 2'000, .eurPool = 1'000, .deliverAmount = 100, .expectedSourceAmount = 223}, + }; + // clang-format on + + for (auto const& testCase : testCases) + { + checkQuote( + testCase.usdPool, + testCase.eurPool, + testCase.deliverAmount, + testCase.expectedSourceAmount); + } + } + void testFalseDry(FeatureBitset features) { @@ -3583,6 +3681,7 @@ private: pathFind01(); pathFind02(); pathFind06(); + pathFindMPTAMMExecutableSourceAmount(); } void diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index bb532b361a..83c848b7c4 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -267,20 +267,39 @@ private: {features}); // tfPassive -- place the offer without crossing it. - testAMM( - [&](AMM& ammAlice, Env& env) { - // Carol creates a passive offer that could cross AMM. - // Carol's offer should stay in the ledger. - env(offer(carol_, XRP(100), USD(100), tfPassive)); - env.close(); - BEAST_EXPECT( - ammAlice.expectBalances(XRP(10'100), STAmount{USD, 10'000}, ammAlice.tokens())); - BEAST_EXPECT(expectOffers(env, carol_, 1, {{{XRP(100), STAmount{USD, 100}}}})); - }, - {{XRP(10'100), USD(10'000)}}, - 0, - std::nullopt, - {features}); + if (features[featureMPTokensV2]) + { + Env env{*this, features}; + fund(env, gw_, {alice_, carol_}, XRP(30'000'000), {USD(30'000'000)}); + + AMM const ammAlice(env, alice_, XRP(10'100'000), USD(10'000'000)); + + // Scale the exact-quality fixture up so the visual relationship + // stays clear: the passive CLOB offer has the same 1:1 quality as + // the generated AMM offer, so it should not cross. + env(offer(carol_, XRP(100'000), USD(100'000), tfPassive)); + env.close(); + BEAST_EXPECT( + ammAlice.expectBalances(XRP(10'100'000), USD(10'000'000), ammAlice.tokens())); + BEAST_EXPECT(expectOffers(env, carol_, 1, {{{XRP(100'000), USD(100'000)}}})); + } + else + { + testAMM( + [&](AMM& ammAlice, Env& env) { + // Carol creates a passive offer that could cross AMM. + // Carol's offer should stay in the ledger. + env(offer(carol_, XRP(100), USD(100), tfPassive)); + env.close(); + BEAST_EXPECT(ammAlice.expectBalances( + XRP(10'100), STAmount{USD, 10'000}, ammAlice.tokens())); + BEAST_EXPECT(expectOffers(env, carol_, 1, {{{XRP(100), STAmount{USD, 100}}}})); + }, + {{XRP(10'100), USD(10'000)}}, + 0, + std::nullopt, + {features}); + } // tfPassive -- cross only offers of better quality. testAMM( @@ -1359,6 +1378,7 @@ private: testRmFundedOffer(all_ - fixAMMv1_1 - fixAMMv1_3); testEnforceNoRipple(all_); testFillModes(all_); + testFillModes(all_ - featureMPTokensV2); testOfferCrossWithXRP(all_); testOfferCrossWithLimitOverride(all_); testCurrencyConversionEntire(all_); diff --git a/src/test/app/AMMMPT_test.cpp b/src/test/app/AMMMPT_test.cpp index 7078ea6769..90a267f56f 100644 --- a/src/test/app/AMMMPT_test.cpp +++ b/src/test/app/AMMMPT_test.cpp @@ -32,14 +32,17 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include +#include #include #include #include @@ -3269,6 +3272,48 @@ private: ammAlice.expectBalances(MPT(ammAlice[1])(1), XRP(10'000), IOUAmount{100000})); }, {{XRP(10'000), gAmmmpt(10'000)}}); + + // MPT/MPT equal withdrawal after LP deletes both zero-balance MPTokens. + // AMMWithdraw must recreate both missing MPTokens; the invariant allows + // up to two MPToken creations per AMMWithdraw/AMMClawback (threshold > 2). + { + Env env{*this}; + env.fund(XRP(30'000), gw_, alice_); + env.close(); + MPTTester btc( + {.env = env, + .issuer = gw_, + .holders = {alice_}, + .pay = 10'000, + .flags = kMptDexFlags}); + MPTTester eth( + {.env = env, + .issuer = gw_, + .holders = {alice_}, + .pay = 10'000, + .flags = kMptDexFlags}); + + // Alice deposits everything into the MPT/MPT pool; her MPT + // balances drop to zero. + AMM ammAlice(env, alice_, btc(10'000), eth(10'000)); + BEAST_EXPECT(expectMPT(env, alice_, btc(0))); + BEAST_EXPECT(expectMPT(env, alice_, eth(0))); + + // Alice deletes both zero-balance MPTokens to reclaim reserve. + btc.authorize({.account = alice_, .flags = tfMPTUnauthorize}); + eth.authorize({.account = alice_, .flags = tfMPTUnauthorize}); + BEAST_EXPECT(!env.le(keylet::mptoken(btc.issuanceID(), alice_.id()))); + BEAST_EXPECT(!env.le(keylet::mptoken(eth.issuanceID(), alice_.id()))); + + // Equal withdrawal succeeds: both missing MPTokens are recreated + // (mptokensCreated_ == 2, which satisfies the > 2 invariant check). + ammAlice.withdrawAll(alice_); + BEAST_EXPECT(env.le(keylet::mptoken(btc.issuanceID(), alice_.id()))); + BEAST_EXPECT(env.le(keylet::mptoken(eth.issuanceID(), alice_.id()))); + BEAST_EXPECT(expectMPT(env, alice_, btc(10'000))); + BEAST_EXPECT(expectMPT(env, alice_, eth(10'000))); + BEAST_EXPECT(!ammAlice.ammExists()); + } } void @@ -4041,9 +4086,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); } @@ -4053,9 +4098,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); } } @@ -4901,7 +4946,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( @@ -5010,12 +5055,12 @@ private: env.close(); BEAST_EXPECT( - amm.expectBalances(XRPAmount(909'090'909), btc(550'000000055001), amm.tokens())); - // Offer ~91XRP/49.99e12BTC + amm.expectBalances(XRPAmount(909'090'910), btc(549'999999450001), amm.tokens())); + // Offer ~91XRP/50e12BTC BEAST_EXPECT(expectOffers( - env, carol_, 1, {{Amounts{XRPAmount{9'090'909}, btc(4'999999950000)}}})); - // Carol pays 0.1% fee on 50'000000055000BTC = 50'000000055BTC - env.require(Balance(carol_, btc(29'949'949'999'944'943))); + env, carol_, 1, {{Amounts{XRPAmount{9'090'910}, btc(5'000000500000)}}})); + // Carol pays 0.1% fee on 49'999999450001BTC. + env.require(Balance(carol_, btc(29'949'950'000'550'548))); } { @@ -5065,15 +5110,15 @@ private: env.close(); BEAST_EXPECT(ammAlice.expectBalances( - btc(1'060'6848287928033), eth(1'037'0658372213574), ammAlice.tokens())); + btc(1'060'6848287928025), eth(1'037'0658372213582), ammAlice.tokens())); // Consumed offer ~72.93e13ETH/72.93e13BTC BEAST_EXPECT(expectOffers( - env, carol_, 1, {Amounts{eth(27'0658372213574), btc(27'0658372213575)}})); + env, carol_, 1, {Amounts{eth(27'0658372213582), btc(27'0658372213582)}})); BEAST_EXPECT(expectOffers(env, bob_, 0)); BEAST_EXPECT(expectOffers(env, ed, 0)); - env.require(Balance(carol_, btc(19'116'439'640'089'955))); - env.require(Balance(carol_, eth(20'729'341'627'786'426))); + env.require(Balance(carol_, btc(19'116'439'640'089'965))); + env.require(Balance(carol_, eth(20'729'341'627'786'418))); env.require(Balance(bob_, btc(20'100'000'000'000'000))); env.require(Balance(ed, eth(19'875'000'000'000'000))); } @@ -5672,6 +5717,87 @@ private: }); } + void + testAMMOfferGenerationPolicy(FeatureBitset features) + { + testcase("AMM payment offer generation picks economically coarser integral side"); + + using namespace jtx; + + enum class GeneratedFirst { TakerPays, TakerGets }; + + auto const check = [&](std::uint64_t mptUnitsPerXRP, GeneratedFirst generatedFirst) { + TAmounts const pool{ + XRPAmount{1'000'000}, MPTAmount{1'000'000'125}}; + TAmounts const clobOffer{ + kDropsPerXrp, MPTAmount{static_cast(mptUnitsPerXRP)}}; + Quality const clobQuality{clobOffer}; + + auto const expectedAmounts = generatedFirst == GeneratedFirst::TakerGets + ? getAMMOfferStartWithTakerGets(pool, clobQuality, 0) + : getAMMOfferStartWithTakerPays(pool, clobQuality, 0); + auto const otherAmounts = generatedFirst == GeneratedFirst::TakerGets + ? getAMMOfferStartWithTakerPays(pool, clobQuality, 0) + : getAMMOfferStartWithTakerGets(pool, clobQuality, 0); + BEAST_EXPECT(expectedAmounts); + BEAST_EXPECT(otherAmounts); + if (!expectedAmounts || !otherAmounts) + return; + + // Make the tested branch observable: these cases are chosen so the + // payment consumes different AMM amounts depending on which side + // is generated first. + BEAST_EXPECT(*expectedAmounts != *otherAmounts); + + Env env(*this, features); + auto const gw = Account("gw"); + auto const lp = Account("lp"); + auto const maker = Account("maker"); + auto const taker = Account("taker"); + auto const dst = Account("dst"); + + env.fund(XRP(10'000), gw, lp, maker, taker, dst); + env.close(); + + MPTTester const token( + {.env = env, .issuer = gw, .holders = {lp, maker, dst}, .flags = kMptDexFlags}); + env(pay(gw, lp, token(pool.out.value()))); + env(pay(gw, maker, token(10'000'000))); + env.close(); + + AMM const amm(env, lp, drops(pool.in), token(pool.out.value())); + auto const makerOfferSeq = env.seq(maker); + env(offer(maker, XRP(1), token(mptUnitsPerXRP)), Txflags(tfPassive)); + env.close(); + + env(pay(taker, dst, token(expectedAmounts->out.value())), + Sendmax(drops(expectedAmounts->in))); + env.close(); + + BEAST_EXPECT(amm.expectBalances( + drops(pool.in + expectedAmounts->in), + token((pool.out - expectedAmounts->out).value()), + amm.tokens())); + env.require(Balance(dst, token(expectedAmounts->out.value()))); + BEAST_EXPECT(env.le(keylet::offer(maker.id(), SeqProxy::rawSequence(makerOfferSeq)))); + }; + + // CLOB price: 10'000'000 MPT per 1 XRP, so one raw MPT unit is worth + // 0.1 drops. One drop is the economically coarser unit and the AMM + // offer is generated from takerPays. + check(10 * kDropsPerXrp.drops(), GeneratedFirst::TakerPays); + + // CLOB price: 1'000'000 MPT per 1 XRP, so one raw MPT unit is worth + // one drop. Ties use takerGets to preserve the historical XRP-output + // behavior. + check(kDropsPerXrp.drops(), GeneratedFirst::TakerGets); + + // CLOB price: 100'000 MPT per 1 XRP, so one raw MPT unit is worth + // 10 drops. MPT is the economically coarser unit and the AMM offer is + // generated from takerGets. + check(kDropsPerXrp.drops() / 10, GeneratedFirst::TakerGets); + } + void testTradingFee(FeatureBitset features) { @@ -7242,7 +7368,7 @@ private: // overflow. Deposit has no such bound, which is why only the deposit // path was exposed. // - // These mirror the deposit repros: the same oversized two-asset + // These mirror the deposit tests: the same oversized two-asset // request is rejected cleanly. If the preclaim bound is ever weakened, // equalWithdrawLimit would be reached with a huge frac and // Number::operator rep() would escape as tefEXCEPTION, failing this. @@ -7318,6 +7444,7 @@ private: testAMMTokens(); testAmendment(); testAMMAndCLOB(all); + testAMMOfferGenerationPolicy(all); testTradingFee(all); testTradingFee(all - fixAMMv1_3); testAdjustedTokens(all); diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 8f8079c34a..e1732aaf0e 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -3778,6 +3778,21 @@ private: BEAST_EXPECT(amm.expectBalances(XRP(1'000), USD(500), amm.tokens())); BEAST_EXPECT(expectOffers(env, carol_, 1, {{Amounts{XRP(100), USD(55)}}})); } + else if (!features[featureMPTokensV2]) + { + BEAST_EXPECT(amm.expectBalances( + XRPAmount(909'090'909), + STAmount{USD, UINT64_C(550'000000055), -9}, + amm.tokens())); + BEAST_EXPECT(expectOffers( + env, + carol_, + 1, + {{Amounts{XRPAmount{9'090'909}, STAmount{USD, 4'99999995, -8}}}})); + BEAST_EXPECT( + env.balance(carol_, USD) == + STAmount(USD, UINT64_C(29'949'94999999494), -11)); + } else { // Post-amendment the transfer fee is taken into account @@ -3788,19 +3803,19 @@ private: // quality. // AMM offer ~50USD/91XRP BEAST_EXPECT(amm.expectBalances( - XRPAmount(909'090'909), - STAmount{USD, UINT64_C(550'000000055), -9}, + XRPAmount(909'090'910), + STAmount{USD, UINT64_C(549'99999945), -8}, amm.tokens())); - // Offer ~91XRP/49.99USD + // Offer ~91XRP/50USD BEAST_EXPECT(expectOffers( env, carol_, 1, - {{Amounts{XRPAmount{9'090'909}, STAmount{USD, 4'99999995, -8}}}})); + {{Amounts{XRPAmount{9'090'910}, STAmount{USD, 5'0000005, -7}}}})); // Carol pays 0.1% fee on ~50USD =~ 0.05USD BEAST_EXPECT( env.balance(carol_, USD) == - STAmount(USD, UINT64_C(29'949'94999999494), -11)); + STAmount(USD, UINT64_C(29'949'95000060055), -11)); } }, {{XRP(1'000), USD(500)}}, @@ -6451,7 +6466,7 @@ private: BEAST_EXPECT(expectOffers(env, bob_, 1, {{Amounts{USD(1), XRPAmount(500)}}})); BEAST_EXPECT(expectOffers(env, carol_, 1, {{Amounts{XRP(100), USD(55)}}})); } - else + else if (!features[featureMPTokensV2]) { BEAST_EXPECT(amm.expectBalances( XRPAmount(909'090'909), @@ -6464,6 +6479,19 @@ private: {{Amounts{XRPAmount{9'090'909}, STAmount{USD, 4'99999995, -8}}}})); BEAST_EXPECT(expectOffers(env, bob_, 1, {{Amounts{USD(1), XRPAmount(500)}}})); } + else + { + BEAST_EXPECT(amm.expectBalances( + XRPAmount(909'090'910), + STAmount{USD, UINT64_C(549'99999945), -8}, + amm.tokens())); + BEAST_EXPECT(expectOffers( + env, + carol_, + 1, + {{Amounts{XRPAmount{9'090'910}, STAmount{USD, 5'0000005, -7}}}})); + BEAST_EXPECT(expectOffers(env, bob_, 1, {{Amounts{USD(1), XRPAmount(500)}}})); + } } // There is no blocking offer, the same AMM liquidity is consumed @@ -6475,10 +6503,30 @@ private: AMM const amm(env, alice_, XRP(1'000), USD(500)); env(offer(carol_, XRP(100), USD(55))); env.close(); - BEAST_EXPECT(amm.expectBalances( - XRPAmount(909'090'909), STAmount{USD, UINT64_C(550'000000055), -9}, amm.tokens())); - BEAST_EXPECT(expectOffers( - env, carol_, 1, {{Amounts{XRPAmount{9'090'909}, STAmount{USD, 4'99999995, -8}}}})); + if (!features[featureMPTokensV2]) + { + BEAST_EXPECT(amm.expectBalances( + XRPAmount(909'090'909), + STAmount{USD, UINT64_C(550'000000055), -9}, + amm.tokens())); + BEAST_EXPECT(expectOffers( + env, + carol_, + 1, + {{Amounts{XRPAmount{9'090'909}, STAmount{USD, 4'99999995, -8}}}})); + } + else + { + BEAST_EXPECT(amm.expectBalances( + XRPAmount(909'090'910), + STAmount{USD, UINT64_C(549'99999945), -8}, + amm.tokens())); + BEAST_EXPECT(expectOffers( + env, + carol_, + 1, + {{Amounts{XRPAmount{9'090'910}, STAmount{USD, 5'0000005, -7}}}})); + } } } @@ -7400,6 +7448,7 @@ private: testFlags(); testRippling(); testAMMAndCLOB(all); + testAMMAndCLOB(all - featureMPTokensV2); testAMMAndCLOB(all - fixAMMv1_1 - fixAMMv1_3); testTradingFee(all); testTradingFee(all - fixAMMv1_3); @@ -7419,8 +7468,10 @@ private: testOverflowOffer(all - fixAMMv1_1 - fixAMMv1_3); testSwapRounding(); testFixChangeSpotPriceQuality(all); + testFixChangeSpotPriceQuality(all - featureMPTokensV2); testFixChangeSpotPriceQuality(all - fixAMMv1_1 - fixAMMv1_3); testFixAMMOfferBlockedByLOB(all); + testFixAMMOfferBlockedByLOB(all - featureMPTokensV2); testFixAMMOfferBlockedByLOB(all - fixAMMv1_1 - fixAMMv1_3); testLPTokenBalance(all); testLPTokenBalance(all - fixAMMv1_3); diff --git a/src/test/app/EscrowToken_test.cpp b/src/test/app/EscrowToken_test.cpp index 7e7509c3b7..72db63bd3f 100644 --- a/src/test/app/EscrowToken_test.cpp +++ b/src/test/app/EscrowToken_test.cpp @@ -3749,6 +3749,186 @@ struct EscrowToken_test : public beast::unit_test::Suite BEAST_EXPECT(issuerMPTEscrowed(env, mpt) == 0); } + void + testMPTLargeLockedRate(FeatureBitset features) + { + testcase("MPT large locked rate"); + using namespace test::jtx; + using namespace std::literals; + + auto constexpr escrowAmount = 200'000'000'000'000'000LL; + auto constexpr noOverflowEscrowAmount = 186'000'000'000'000'000LL; + auto const alice = Account("alice"); + auto const bob = Account("bob"); + auto const gw = Account("gw"); + + for (auto const testFeatures : + {features - featureMPTokensV2 - fixCleanup3_4_0, + features - featureMPTokensV2, + (features | featureMPTokensV2) - fixCleanup3_4_0, + features | featureMPTokensV2}) + { + bool const mptV2 = testFeatures[featureMPTokensV2]; + bool const tokenEscrowV1 = testFeatures[fixTokenEscrowV1]; + // The transfer-fee split in EscrowFinish only overflows on the + // legacy divideRound(amount, lockedRate, ...) path, which runs when + // fixCleanup3_4_0 is disabled. With fixCleanup3_4_0 the split uses + // mulRatio (128-bit intermediate), which cannot overflow. Without + // it, this large amount overflows unless the MPTokensV2 Number path + // is active. So the finish succeeds when either amendment is enabled. + bool const cleanup340 = testFeatures[fixCleanup3_4_0]; + bool const noOverflow = cleanup340 || mptV2; + auto const expectedErr = noOverflow ? Ter(tesSUCCESS) : Ter(tefEXCEPTION); + + // Finish with a large MPT amount and non-zero transfer fee. When the + // computation overflows (legacy divideRound path, no MPTokensV2) the + // finish fails with tefEXCEPTION and the escrow is untouched; + // otherwise it unlocks the escrow. + { + Env env{*this, testFeatures}; + env.fund(XRP(1'000), alice, bob, gw); + auto const baseFee = env.current()->fees().base; + + MPTTester const mpt( + {.env = env, + .issuer = gw, + .holders = {alice, bob}, + .transferFee = 1'000, + .flags = tfMPTCanEscrow | tfMPTCanTransfer}); + env(pay(gw, alice, mpt(escrowAmount))); + env.close(); + + auto const preAlice = env.balance(alice, mpt); + auto const preBob = env.balance(bob, mpt); + auto const seq = env.seq(alice); + env(escrow::create(alice, bob, mpt(escrowAmount)), + escrow::kCondition(escrow::kCb1), + escrow::kFinishTime(env.now() + 1s), + escrow::kCancelTime(env.now() + 500s), + Fee(baseFee * 150)); + env.close(); + + BEAST_EXPECT(mptEscrowed(env, alice, mpt) == escrowAmount); + BEAST_EXPECT(issuerMPTEscrowed(env, mpt) == escrowAmount); + + env(escrow::finish(bob, alice, seq), + escrow::kCondition(escrow::kCb1), + escrow::kFulfillment(escrow::kFb1), + Fee(baseFee * 150), + expectedErr); + env.close(); + + if (noOverflow) + { + BEAST_EXPECT(!env.le(keylet::escrow(alice.id(), SeqProxy::rawSequence(seq)))); + BEAST_EXPECT(env.balance(alice, mpt) == preAlice - mpt(escrowAmount)); + auto const postBob = env.balance(bob, mpt); + BEAST_EXPECT(postBob.value() > preBob.value()); + BEAST_EXPECT(postBob.value() < (preBob + mpt(escrowAmount)).value()); + auto const xferFee = escrowAmount - (postBob.value() - preBob.value()); + auto const expectedEscrow = tokenEscrowV1 ? 0 : xferFee; + BEAST_EXPECT(mptEscrowed(env, alice, mpt) == expectedEscrow); + BEAST_EXPECT(issuerMPTEscrowed(env, mpt) == expectedEscrow); + } + else + { + BEAST_EXPECT(env.le(keylet::escrow(alice.id(), SeqProxy::rawSequence(seq)))); + BEAST_EXPECT(env.balance(alice, mpt) == preAlice - mpt(escrowAmount)); + BEAST_EXPECT(env.balance(bob, mpt) == preBob); + BEAST_EXPECT(mptEscrowed(env, alice, mpt) == escrowAmount); + BEAST_EXPECT(issuerMPTEscrowed(env, mpt) == escrowAmount); + } + } + + // Control: a still-large amount below the legacy overflow boundary + // finishes successfully in both feature modes. + { + Env env{*this, testFeatures}; + env.fund(XRP(1'000), alice, bob, gw); + auto const baseFee = env.current()->fees().base; + + MPTTester const mpt( + {.env = env, + .issuer = gw, + .holders = {alice, bob}, + .transferFee = 1'000, + .flags = tfMPTCanEscrow | tfMPTCanTransfer}); + env(pay(gw, alice, mpt(noOverflowEscrowAmount))); + env.close(); + + auto const preAlice = env.balance(alice, mpt); + auto const preBob = env.balance(bob, mpt); + auto const seq = env.seq(alice); + env(escrow::create(alice, bob, mpt(noOverflowEscrowAmount)), + escrow::kCondition(escrow::kCb1), + escrow::kFinishTime(env.now() + 1s), + escrow::kCancelTime(env.now() + 500s), + Fee(baseFee * 150)); + env.close(); + + BEAST_EXPECT(mptEscrowed(env, alice, mpt) == noOverflowEscrowAmount); + BEAST_EXPECT(issuerMPTEscrowed(env, mpt) == noOverflowEscrowAmount); + + env(escrow::finish(bob, alice, seq), + escrow::kCondition(escrow::kCb1), + escrow::kFulfillment(escrow::kFb1), + Fee(baseFee * 150), + Ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::escrow(alice.id(), SeqProxy::rawSequence(seq)))); + BEAST_EXPECT(env.balance(alice, mpt) == preAlice - mpt(noOverflowEscrowAmount)); + auto const postBob = env.balance(bob, mpt); + BEAST_EXPECT(postBob.value() > preBob.value()); + BEAST_EXPECT(postBob.value() < (preBob + mpt(noOverflowEscrowAmount)).value()); + auto const xferFee = noOverflowEscrowAmount - (postBob.value() - preBob.value()); + auto const expectedEscrow = tokenEscrowV1 ? 0 : xferFee; + BEAST_EXPECT(mptEscrowed(env, alice, mpt) == expectedEscrow); + BEAST_EXPECT(issuerMPTEscrowed(env, mpt) == expectedEscrow); + } + + // Cancel returns the escrow to the owner using parity rate, so it + // does not hit the transfer-rate division in either feature mode. + { + Env env{*this, testFeatures}; + env.fund(XRP(1'000), alice, bob, gw); + auto const baseFee = env.current()->fees().base; + + MPTTester const mpt( + {.env = env, + .issuer = gw, + .holders = {alice, bob}, + .transferFee = 1'000, + .flags = tfMPTCanEscrow | tfMPTCanTransfer}); + env(pay(gw, alice, mpt(escrowAmount))); + env.close(); + + auto const preAlice = env.balance(alice, mpt); + auto const preBob = env.balance(bob, mpt); + auto const seq = env.seq(alice); + env(escrow::create(alice, bob, mpt(escrowAmount)), + escrow::kCondition(escrow::kCb1), + escrow::kFinishTime(env.now() + 1s), + escrow::kCancelTime(env.now() + 3s), + Fee(baseFee * 150)); + env.close(); + + BEAST_EXPECT(mptEscrowed(env, alice, mpt) == escrowAmount); + BEAST_EXPECT(issuerMPTEscrowed(env, mpt) == escrowAmount); + + env(escrow::cancel(alice, alice, seq), Fee(baseFee), Ter(tesSUCCESS)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::escrow(alice.id(), SeqProxy::rawSequence(seq)))); + BEAST_EXPECT(env.balance(alice, mpt) == preAlice); + BEAST_EXPECT(env.balance(bob, mpt) == preBob); + BEAST_EXPECT(env.balance(gw, mpt) == -mpt(escrowAmount)); + BEAST_EXPECT(mptEscrowed(env, alice, mpt) == 0); + BEAST_EXPECT(issuerMPTEscrowed(env, mpt) == 0); + } + } + } + void testMPTRequireAuth(FeatureBitset features) { @@ -4047,6 +4227,7 @@ struct EscrowToken_test : public beast::unit_test::Suite testMPTMetaAndOwnership(features); testMPTGateway(features); testMPTLockedRate(features); + testMPTLargeLockedRate(features); testMPTRequireAuth(features); testMPTLock(features); testMPTCanTransfer(features); diff --git a/src/test/app/FlowMPT_test.cpp b/src/test/app/FlowMPT_test.cpp index a94834eb28..49e3f9be94 100644 --- a/src/test/app/FlowMPT_test.cpp +++ b/src/test/app/FlowMPT_test.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -742,6 +743,164 @@ struct FlowMPT_test : public beast::unit_test::Suite return result; } + void + testOfferOwnerMPTCreation(FeatureBitset features) + { + using namespace jtx; + Account const alice("alice"); + Account const bob("bob"); + Account const carol("carol"); + Account const gw("gw"); + + { + testcase("Reserve-edge offer owner cannot create another object"); + + Env env(*this, features); + + auto const baseFee = env.current()->fees().base; + auto const ownerIncrement = reserve(env, 1) - reserve(env, 0); + auto const xrpOffer = ownerIncrement - drops(1); + auto const bobStart = reserve(env, 2) - drops(1) + baseFee; + + env.fund(XRP(10'000), alice, gw); + env.fund(bobStart, bob); + env.close(); + + MPTTester const usd({.env = env, .issuer = gw, .maxAmt = 10}); + + env(offer(bob, usd(1), xrpOffer)); + env.close(); + + env.require(Balance(bob, reserve(env, 2) - drops(1)), Owners(bob, 1)); + + // This mirrors the full-crossing setup below. Bob has enough XRP + // for the resting offer, but not enough to pay a fee and add + // another owner-count object while the offer remains on ledger. + env(check::create(bob, alice, drops(1)), Ter(tecINSUFFICIENT_RESERVE)); + env.close(); + + env.require(Owners(bob, 1)); + BEAST_EXPECT(offersOnAccount(env, bob).size() == 1); + } + + { + testcase("Reserve-edge offer owner creates MPToken during consume"); + + Env env(*this, features); + + auto const baseFee = env.current()->fees().base; + auto const ownerIncrement = reserve(env, 1) - reserve(env, 0); + auto const xrpOffer = ownerIncrement - drops(1); + auto const bobStart = reserve(env, 2) - drops(1) + baseFee; + + env.fund(XRP(10'000), alice, carol, gw); + env.fund(bobStart, bob); + env.close(); + + MPTTester const usd({.env = env, .issuer = gw, .holders = {alice}, .maxAmt = 10}); + + env(pay(gw, alice, usd(1))); + env(offer(bob, usd(1), xrpOffer)); + env.close(); + + env.require(Balance(bob, reserve(env, 2) - drops(1)), Owners(bob, 1)); + BEAST_EXPECT(!env.le(keylet::mptoken(usd.issuanceID(), bob.id()))); + auto const carolXRP = env.balance(carol); + + // Bob has enough XRP for the resting offer but is close to + // reserve. The payment should not create Bob's USD MPToken until + // the offer is actually consumed, otherwise the temporary owner + // count increase can make the offer look underfunded during path + // execution. + env(pay(alice, carol, xrpOffer), + Path(~XRP), + Sendmax(usd(1)), + Txflags(tfNoRippleDirect)); + env.close(); + + env.require(Balance(carol, carolXRP + xrpOffer)); + env.require(Balance(bob, usd(1))); + env.require(Balance(bob, reserve(env, 1)), Owners(bob, 1)); + BEAST_EXPECT(env.le(keylet::mptoken(usd.issuanceID(), bob.id()))); + BEAST_EXPECT(offersOnAccount(env, bob).empty()); + } + + { + testcase("Partial offer owner creates MPToken during consume"); + + Env env(*this, features); + + auto const baseFee = env.current()->fees().base; + auto const ownerIncrement = reserve(env, 1) - reserve(env, 0); + auto const bobStart = reserve(env, 3) + baseFee; + + env.fund(XRP(10'000), alice, carol, gw); + env.fund(bobStart, bob); + env.close(); + + MPTTester const usd({.env = env, .issuer = gw, .holders = {alice}, .maxAmt = 10}); + + env(pay(gw, alice, usd(1))); + env(offer(bob, usd(2), drops(2 * ownerIncrement))); + env.close(); + + env.require(Balance(bob, reserve(env, 3)), Owners(bob, 1)); + BEAST_EXPECT(!env.le(keylet::mptoken(usd.issuanceID(), bob.id()))); + auto const carolXRP = env.balance(carol); + + // Partial consumption leaves Bob's offer on the ledger, so he ends + // up owning both the remaining offer and a newly created MPToken. + // The MPToken is created regardless of reserve; this setup simply + // funds Bob enough that he still meets reserve(2) afterward (the + // under-reserved case is covered in OfferMPT_test's no-reserve-check + // testcase). + env(pay(alice, carol, drops(ownerIncrement)), + Path(~XRP), + Sendmax(usd(1)), + Txflags(tfNoRippleDirect)); + env.close(); + + env.require(Balance(carol, carolXRP + drops(ownerIncrement))); + env.require(Balance(bob, usd(1))); + env.require(Balance(bob, reserve(env, 2)), Owners(bob, 2)); + BEAST_EXPECT(env.le(keylet::mptoken(usd.issuanceID(), bob.id()))); + BEAST_EXPECT(offersOnAccount(env, bob).size() == 1); + BEAST_EXPECT(isOffer(env, bob, usd(1), drops(ownerIncrement))); + } + + { + testcase("Issuer-owned offer does not create issuer MPToken"); + + Env env(*this, features); + + env.fund(XRP(10'000), alice, carol, gw); + env.close(); + + MPTTester const usd({.env = env, .issuer = gw, .holders = {alice}, .maxAmt = 10}); + + env(pay(gw, alice, usd(1))); + env(offer(gw, usd(1), drops(1'000))); + env.close(); + + BEAST_EXPECT(!env.le(keylet::mptoken(usd.issuanceID(), gw.id()))); + auto const carolXRP = env.balance(carol); + + // The issuer can own an offer that receives its own MPT without an + // MPToken. Consuming that offer should keep the issuer side + // tokenless. + env(pay(alice, carol, drops(1'000)), + Path(~XRP), + Sendmax(usd(1)), + Txflags(tfNoRippleDirect)); + env.close(); + + env.require(Balance(alice, usd(0))); + env.require(Balance(carol, carolXRP + drops(1'000))); + BEAST_EXPECT(!env.le(keylet::mptoken(usd.issuanceID(), gw.id()))); + BEAST_EXPECT(offersOnAccount(env, gw).empty()); + } + } + void testSelfPayment1(FeatureBitset features) { @@ -2121,6 +2280,7 @@ struct FlowMPT_test : public beast::unit_test::Suite testFalseDry(features); testDirectStep(features); testBookStep(features); + testOfferOwnerMPTCreation(features); testTransferRate(features); testSelfPayment1(features); testSelfPayment2(features); diff --git a/src/test/app/OfferMPT_test.cpp b/src/test/app/OfferMPT_test.cpp index d03b1b8e93..e262954fdf 100644 --- a/src/test/app/OfferMPT_test.cpp +++ b/src/test/app/OfferMPT_test.cpp @@ -1,3 +1,5 @@ +#include +#include #include #include #include @@ -5,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -22,6 +25,7 @@ #include #include +#include #include #include #include @@ -35,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -46,6 +51,7 @@ #include #include #include +#include #include #include #include @@ -609,6 +615,267 @@ public: testHelper2TokensMix(test); } + void + testMPTIssuerOfferUsesRemainingCapacity(FeatureBitset features) + { + testcase("MPT issuer offer dust removal uses remaining issuance capacity"); + + using namespace jtx; + + Account const issuer{"issuer"}; + Account const carol{"carol"}; + Account const bob{"bob"}; + + Env env{*this, features}; + env.fund(XRP(10'000), issuer, carol, bob); + env.close(); + + MPTTester const musd( + {.env = env, .issuer = issuer, .holders = {carol, bob}, .maxAmt = 101}); + + // The issuer offer is fully fundable when placed. Later issuance leaves + // only one MPT of remaining capacity, so this issuer-owned MPT offer + // must be clipped by owner funds just like a holder-funded offer. + auto const issuerOfferSeq = env.seq(issuer); + env(offer(issuer, drops(1), musd(100))); + env.close(); + + env(pay(issuer, carol, musd(100))); + env.close(); + BEAST_EXPECT(env.balance(issuer, musd) == musd(-100)); + BEAST_EXPECT(env.balance(carol, musd) == musd(100)); + + // Carol's same-quality offer provides the legitimately funded side of + // the crossing. Without the issuer-cap dust-removal check, Bob would + // receive Carol's 100 MPT plus one free self-issued MPT from issuer's + // stale offer while paying only Carol's one drop. + auto const carolOfferSeq = env.seq(carol); + env(offer(carol, drops(1), musd(100))); + env.close(); + + auto const issuerOffer = keylet::offer(issuer.id(), SeqProxy::rawSequence(issuerOfferSeq)); + auto const carolOffer = keylet::offer(carol.id(), SeqProxy::rawSequence(carolOfferSeq)); + BEAST_EXPECT(env.le(issuerOffer) != nullptr); + BEAST_EXPECT(env.le(carolOffer) != nullptr); + + env(offer(bob, musd(101), drops(2), tfImmediateOrCancel)); + env.close(); + + BEAST_EXPECT(env.le(issuerOffer) == nullptr); + BEAST_EXPECT(env.le(carolOffer) == nullptr); + env.require(offers(issuer, 0), offers(carol, 0), offers(bob, 0)); + BEAST_EXPECT(env.balance(issuer, musd) == musd(-100)); + BEAST_EXPECT(env.balance(carol, musd) == musd(0)); + BEAST_EXPECT(env.balance(bob, musd) == musd(100)); + } + + void + testPartiallyFundedMPTInputOfferZeroInput(FeatureBitset features) + { + using namespace jtx; + auto const alice = Account{"alice"}; + auto const bob = Account{"bob"}; + + { + testcase("Partially funded MPT/XRP input offer cannot be consumed for free"); + + Env env{*this, features}; + auto const gw = Account{"gw"}; + + env.fund(XRP(10'000), gw, alice, bob); + env.close(); + + MPTTester const usd({.env = env, .issuer = gw, .holders = {alice}}); + + auto const aliceOfferSeq = env.seq(alice); + env(offer(alice, usd(1), drops(1'000'000))); + env.close(); + + auto const targetBalance = reserve(env, 2) + drops(999'999); + auto const drain = env.balance(alice).value().xrp() - targetBalance.value().xrp() - + env.current()->fees().base; + env(pay(alice, gw, drops(drain))); + env.close(); + + auto const aliceXRPBefore = env.balance(alice); + auto const bobXRPBefore = env.balance(bob); + + env(pay(gw, bob, drops(1'000'000)), + Sendmax(usd(1)), + Path(~XRP), + Txflags(tfNoRippleDirect | tfPartialPayment), + Ter(tecPATH_DRY)); + env.close(); + + // alice's offer sells 1,000,000 drops for usd(1) but she can fund + // only 999,999. Filling the clipped remainder would require a + // fractional usd (MPT) input that rounds down to zero, so without + // the fix the taker could take the funded drops for free. + // shouldRmSmallIncreasedQOffer() now treats the MPT input as + // integral (like XRP) and removes the degraded offer, so the + // payment goes dry. The removal happens only inside the crossing: + // tecPATH_DRY discards everything but the fee, so the offer itself + // stays in the ledger, unconsumed. + BEAST_EXPECT( + env.le(keylet::offer(alice.id(), SeqProxy::rawSequence(aliceOfferSeq))) != nullptr); + BEAST_EXPECT(env.balance(alice) == aliceXRPBefore); + BEAST_EXPECT(env.balance(bob) == bobXRPBefore); + } + + { + testcase("Partially funded MPT/IOU input offer cannot be consumed for free"); + + Env env{*this, features}; + auto const mptIssuer = Account{"mptIssuer"}; + auto const iouIssuer = Account{"iouIssuer"}; + + env.fund(XRP(10'000), mptIssuer, iouIssuer, alice, bob); + env.close(); + + auto const eur = iouIssuer["EUR"]; + env.trust(eur(100), alice, bob); + env(pay(iouIssuer, alice, eur(0.5))); + env.close(); + + MPTTester const usd({.env = env, .issuer = mptIssuer, .holders = {alice}}); + + auto const aliceOfferSeq = env.seq(alice); + env(offer(alice, usd(1), eur(1))); + env.close(); + + auto const aliceEURBefore = env.balance(alice, eur); + auto const bobEURBefore = env.balance(bob, eur); + + env(pay(mptIssuer, bob, eur(1)), + Sendmax(usd(1)), + Path(~eur), + Txflags(tfNoRippleDirect | tfPartialPayment), + Ter(tecPATH_DRY)); + env.close(); + + // Same zero-input regression as the MPT/XRP case above, but with + // an IOU (eur) output leg: the fractional usd (MPT) input rounds + // to zero. The degraded offer is removed during crossing, the + // payment goes dry, and tecPATH_DRY leaves the offer in the ledger. + BEAST_EXPECT( + env.le(keylet::offer(alice.id(), SeqProxy::rawSequence(aliceOfferSeq))) != nullptr); + BEAST_EXPECT(env.balance(alice, eur) == aliceEURBefore); + BEAST_EXPECT(env.balance(bob, eur) == bobEURBefore); + } + + { + testcase("Partially funded MPT/MPT input offer cannot be consumed for free"); + + Env env{*this, features}; + auto const issuerA = Account{"issuerA"}; + auto const issuerB = Account{"issuerB"}; + + env.fund(XRP(10'000), issuerA, issuerB, alice, bob); + env.close(); + + MPTTester const usd({.env = env, .issuer = issuerA, .holders = {alice}}); + MPTTester const eur({.env = env, .issuer = issuerB, .holders = {alice, bob}}); + + env(pay(issuerB, alice, eur(999'999))); + env.close(); + + auto const aliceOfferSeq = env.seq(alice); + env(offer(alice, usd(1), eur(1'000'000))); + env.close(); + + auto const aliceEURBefore = eur.getBalance(alice); + auto const bobEURBefore = eur.getBalance(bob); + + env(pay(issuerA, bob, eur(1'000'000)), + Sendmax(usd(1)), + Path(~eur), + Txflags(tfNoRippleDirect | tfPartialPayment), + Ter(tecPATH_DRY)); + env.close(); + + // Same zero-input regression as above, but with both legs MPT: the + // fractional usd (MPT) input rounds to zero. The degraded offer is + // removed during crossing, the payment goes dry, and tecPATH_DRY + // leaves the offer in the ledger. + BEAST_EXPECT( + env.le(keylet::offer(alice.id(), SeqProxy::rawSequence(aliceOfferSeq))) != nullptr); + BEAST_EXPECT(env.balance(alice, eur) == eur(aliceEURBefore)); + BEAST_EXPECT(env.balance(bob, eur) == eur(bobEURBefore)); + } + + { + // The dry cases above never observe the degraded offer actually + // being removed, because tecPATH_DRY rolls the removal back. Here a + // second, fully funded offer lets the crossing succeed, so the + // removal persists: alice's degraded offer is deleted from the + // book (not taken for free) while carol's good offer fills. + testcase( + "Partially funded MPT input offer is removed, not consumed, " + "when a funded offer crosses"); + + Env env{*this, features}; + auto const gw = Account{"gw"}; + auto const carol = Account{"carol"}; + + env.fund(XRP(10'000), gw, alice, carol, bob); + env.close(); + + MPTTester const usd({.env = env, .issuer = gw, .holders = {alice, carol, bob}}); + + // alice's offer sells 1,000,000 drops for usd(1) but, as in the + // dry cases above, she can fund only 999,999 drops, so filling the + // clipped remainder would require a fractional usd (MPT) input that + // rounds down to zero. + auto const aliceOfferSeq = env.seq(alice); + env(offer(alice, usd(1), drops(1'000'000))); + env.close(); + + auto const targetBalance = reserve(env, 2) + drops(999'999); + auto const drain = env.balance(alice).value().xrp() - targetBalance.value().xrp() - + env.current()->fees().base; + env(pay(alice, gw, drops(drain))); + env.close(); + + // carol's same-quality offer is fully funded and provides the + // legitimate side of the crossing. + auto const carolOfferSeq = env.seq(carol); + env(offer(carol, usd(1), drops(1'000'000))); + env.close(); + + // bob needs usd to buy drops. + env(pay(gw, bob, usd(2))); + env.close(); + + auto const aliceOffer = keylet::offer(alice.id(), SeqProxy::rawSequence(aliceOfferSeq)); + auto const carolOffer = keylet::offer(carol.id(), SeqProxy::rawSequence(carolOfferSeq)); + BEAST_EXPECT(env.le(aliceOffer) != nullptr); + BEAST_EXPECT(env.le(carolOffer) != nullptr); + + auto const aliceXRPBefore = env.balance(alice); + auto const bobXRPBefore = env.balance(bob); + + // bob buys drops with usd, wanting more than carol alone supplies so + // the crossing also reaches alice's offer. carol's offer fills; + // alice's degraded offer is removed rather than taken for free, so + // bob receives only carol's 1,000,000 drops and pays only usd(1). + env(offer(bob, drops(2'000'000), usd(2), tfImmediateOrCancel)); + env.close(); + + BEAST_EXPECT(env.le(aliceOffer) == nullptr); + BEAST_EXPECT(env.le(carolOffer) == nullptr); + env.require(offers(alice, 0), offers(carol, 0), offers(bob, 0)); + + // alice's offer was removed, not consumed: her balances are + // unchanged and none of her funded 999'999 drops leaked to bob. + BEAST_EXPECT(env.balance(alice) == aliceXRPBefore); + BEAST_EXPECT(env.balance(alice, usd) == usd(0)); + BEAST_EXPECT(env.balance(carol, usd) == usd(1)); + BEAST_EXPECT(env.balance(bob, usd) == usd(1)); + BEAST_EXPECT( + env.balance(bob) == bobXRPBefore + drops(1'000'000) - env.current()->fees().base); + } + } + void testInsufficientReserve(FeatureBitset features) { @@ -947,6 +1214,161 @@ public: } } + void + testMPTAMMLimitQualityRounding(FeatureBitset features) + { + testcase("MPT AMM limitQuality checks rounded integral output"); + + using namespace jtx; + + Account const gw{"gateway"}; + Account const alice{"alice"}; + Account const bob{"bob"}; + + // IOC used to reject the AMM strand with tecKILLED. The continuous + // limitQuality target is about 32.88 MPT; rounding to nearest requested + // 33 MPT and made the realized AMM quality miss Bob's limit. The + // discrete fallback takes the largest satisfying integer output: 32. + { + Env env{*this, features}; + + env.fund(XRP(10'000), gw, alice, bob); + env.close(); + + MPTTester const btc( + {.env = env, + .issuer = gw, + .holders = {alice, bob}, + .pay = 100'000, + .flags = kMptDexFlags}); + AMM const amm(env, alice, XRP(100), btc(1'000)); + + auto const bobBTCBefore = btc.getBalance(bob); + auto const [xrpBefore, btcBefore, lpBefore] = amm.balances(); + + env(offer(bob, btc(100), drops(10'340'000)), Txflags(tfImmediateOrCancel)); + env.close(); + + auto const [xrpAfter, btcAfter, lpAfter] = amm.balances(); + BEAST_EXPECT(btc.getBalance(bob) == bobBTCBefore + 32); + BEAST_EXPECT(xrpAfter > xrpBefore); + BEAST_EXPECT(btcAfter < btcBefore); + BEAST_EXPECT(lpAfter == lpBefore); + BEAST_EXPECT(expectOffers(env, bob, 0)); + } + + // A standard OfferCreate at the same limit used to bypass the AMM and + // rest unchanged on the book. It should now take the largest + // satisfying 32-MPT AMM fill first, then leave only the remainder on + // the book. + { + Env env{*this, features}; + + env.fund(XRP(10'000), gw, alice, bob); + env.close(); + + MPTTester const btc( + {.env = env, + .issuer = gw, + .holders = {alice, bob}, + .pay = 100'000, + .flags = kMptDexFlags}); + AMM const amm(env, alice, XRP(100), btc(1'000)); + + auto const bobBTCBefore = btc.getBalance(bob); + auto const [xrpBefore, btcBefore, lpBefore] = amm.balances(); + + env(offer(bob, btc(100), drops(10'340'000))); + env.close(); + + auto const [xrpAfter, btcAfter, lpAfter] = amm.balances(); + BEAST_EXPECT(btc.getBalance(bob) == bobBTCBefore + 32); + BEAST_EXPECT(xrpAfter > xrpBefore); + BEAST_EXPECT(btcAfter < btcBefore); + BEAST_EXPECT(lpAfter == lpBefore); + BEAST_EXPECT(expectOffers(env, bob, 1)); + + auto const bobOffers = offersOnAccount(env, bob); + if (BEAST_EXPECT(bobOffers.size() == 1)) + { + BEAST_EXPECT((*bobOffers[0])[sfTakerPays] != btc(100)); + BEAST_EXPECT((*bobOffers[0])[sfTakerGets] != drops(10'340'000)); + } + } + + // Mirror the IOC case with the integral output flipped from MPT units + // to XRP drops. The same continuous target (~32.88) used to round up + // to 33 drops and miss limitQuality; the discrete fallback allows the + // largest satisfying 32-drop AMM fill. + { + Env env{*this, features}; + + env.fund(XRP(10'000), gw, alice, bob); + env.close(); + + MPTTester const btc( + {.env = env, + .issuer = gw, + .holders = {alice, bob}, + .pay = 200'000'000, + .flags = kMptDexFlags}); + AMM const amm(env, alice, drops(1'000), btc(100'000'000)); + + auto const bobXRPBefore = env.balance(bob, XRP); + auto const baseFee = env.current()->fees().base; + auto const [xrpBefore, btcBefore, lpBefore] = amm.balances(); + + env(offer(bob, drops(100), btc(10'340'000)), Txflags(tfImmediateOrCancel)); + env.close(); + + auto const [xrpAfter, btcAfter, lpAfter] = amm.balances(); + env.require(Balance(bob, bobXRPBefore + drops(32) - baseFee)); + BEAST_EXPECT(xrpAfter < xrpBefore); + BEAST_EXPECT(btcAfter > btcBefore); + BEAST_EXPECT(lpAfter == lpBefore); + BEAST_EXPECT(expectOffers(env, bob, 0)); + } + + // Mirror the standard OfferCreate case as well. It should consume the + // largest satisfying 32-drop AMM fill before leaving only the remainder + // on the book. + { + Env env{*this, features}; + + env.fund(XRP(10'000), gw, alice, bob); + env.close(); + + MPTTester const btc( + {.env = env, + .issuer = gw, + .holders = {alice, bob}, + .pay = 200'000'000, + .flags = kMptDexFlags}); + AMM const amm(env, alice, drops(1'000), btc(100'000'000)); + + auto const bobXRPBefore = env.balance(bob, XRP); + auto const baseFee = env.current()->fees().base; + auto const [xrpBefore, btcBefore, lpBefore] = amm.balances(); + + env(offer(bob, drops(100), btc(10'340'000))); + env.close(); + + auto const [xrpAfter, btcAfter, lpAfter] = amm.balances(); + env.require(Balance(bob, bobXRPBefore + drops(32) - baseFee)); + BEAST_EXPECT(xrpAfter < xrpBefore); + BEAST_EXPECT(btcAfter > btcBefore); + BEAST_EXPECT(lpAfter == lpBefore); + BEAST_EXPECT(expectOffers(env, bob, 1)); + + auto const bobOffers = offersOnAccount(env, bob); + if (BEAST_EXPECT(bobOffers.size() == 1)) + { + BEAST_EXPECT((*bobOffers[0])[sfTakerPays] != drops(100)); + BEAST_EXPECT((*bobOffers[0])[sfTakerGets] != btc(10'340'000)); + } + } + } + void testMalformed(FeatureBitset features) { @@ -2727,6 +3149,50 @@ public: using namespace jtx; auto const gw1 = Account("gateway1"); + { + auto const issuer = Account("issuer"); + auto const sender = Account("sender"); + auto const receiver = Account("receiver"); + auto const seller = Account("seller"); + auto const buyer = Account("buyer"); + + Env env{*this, features}; + env.fund(XRP(10'000), issuer, sender, receiver, seller, buyer); + env.close(); + + MPTTester mpt{ + {.env = env, + .issuer = issuer, + .holders = {sender, receiver, seller, buyer}, + .transferFee = 100}}; + MPT const token = mpt; + + mpt.pay(issuer, sender, 2'000); + mpt.pay(issuer, seller, 2'000); + + // A direct holder-to-holder payment of 999 MPT at a 0.1% fee + // requires 1000 from the sender and burns one MPT. + env(pay(sender, receiver, token(999)), Ter(tecPATH_PARTIAL)); + env.close(); + env(pay(sender, receiver, token(999)), Sendmax(token(1'000))); + env.close(); + + BEAST_EXPECT(mpt.getBalance(sender) == 1'000); + BEAST_EXPECT(mpt.getBalance(receiver) == 999); + BEAST_EXPECT(mpt.getBalance(issuer) == 3'999); + + // CLOB crossing should apply the same fee quantum. The offer + // owner pays ceil(999 * 1.001) = 1000, not floor(...) = 999. + env(offer(seller, XRP(999), token(999))); + env.close(); + env(offer(buyer, token(999), XRP(999))); + env.close(); + + BEAST_EXPECT(mpt.getBalance(seller) == 1'000); + BEAST_EXPECT(mpt.getBalance(buyer) == 999); + BEAST_EXPECT(mpt.getBalance(issuer) == 3'998); + } + auto test = [&](auto&& issue1, auto&& issue2) { Env env{*this, features}; @@ -3102,6 +3568,247 @@ public: } } + void + testTransferRateOverflowOffer(FeatureBitset features) + { + testcase("Transfer Rate Overflow Offer"); + + using namespace jtx; + + auto const issuer = Account("issuer"); + auto const taker = Account("taker"); + + { + Env env{*this, features}; + env.fund(XRP(10'000), issuer, taker); + env.close(); + + auto constexpr takerFunds = 2'000'000'000'000'000'000LL; + MPTTester const token{ + {.env = env, + .issuer = issuer, + .holders = {taker}, + .transferFee = 50'000, + .pay = takerFunds, + .maxAmt = kMaxMpTokenAmount}}; + + // Covers OfferCreate::flowCross() sendMax calculation. A large + // non-issuer MPT offer with a transfer fee used to overflow in + // multiplyRound() before the offer could be placed. + auto constexpr offerAmount = 1'230'000'000'000'000'000LL; + auto const takerSeq = env.seq(taker); + env(offer(taker, XRP(1), token(offerAmount))); + env.close(); + + BEAST_EXPECT( + env.le(keylet::offer(taker.id(), SeqProxy::rawSequence(takerSeq))) != nullptr); + BEAST_EXPECT(env.balance(taker, token) == token(takerFunds)); + } + + // Each scenario below targets a BookStep/OfferStream overflow path. + // The expected behavior is the same in all cases: remove the unusable + // book tip offer and let the taker's crossing offer remain rather than + // returning tecINTERNAL with the poison offer still on-ledger. + { + Env env{*this, features}; + env.fund(XRP(10'000), issuer, taker); + env.close(); + + MPTTester const token{ + {.env = env, .issuer = issuer, .holders = {taker}, .transferFee = 10'000}}; + + // Covers BookStep::forEachOffer() offer preparation, where + // ownerGives = mulRatio(ofrAmt.out, transferRateOut) overflowed + // for an oversized MPT output with a transfer fee. + std::int64_t const poisonAmount = 8'500'000'000'000'000'000LL; + auto const poisonSeq = env.seq(issuer); + env(offer(issuer, XRP(1), token(poisonAmount))); + env.close(); + + auto const poisonKeylet = keylet::offer(issuer.id(), SeqProxy::rawSequence(poisonSeq)); + BEAST_EXPECT(env.le(poisonKeylet) != nullptr); + + auto const takerSeq = env.seq(taker); + env(offer(taker, token(100), XRP(100))); + env.close(); + + BEAST_EXPECT(env.le(poisonKeylet) == nullptr); + BEAST_EXPECT( + env.le(keylet::offer(taker.id(), SeqProxy::rawSequence(takerSeq))) != nullptr); + } + + { + auto const gwA = Account("gatewayA"); + auto const gwB = Account("gatewayB"); + auto const alice = Account("alice"); + auto const mallory = Account("mallory"); + + Env env{*this, features}; + env.fund(XRP(10'000), gwA, gwB, alice, mallory); + env.close(); + + MPTTester const tokenA{ + {.env = env, .issuer = gwA, .holders = {alice, mallory}, .transferFee = 50'000}}; + + MPTTester const tokenB{{.env = env, .issuer = gwB, .holders = {alice, mallory}}}; + + env(pay(gwA, alice, tokenA(1'000))); + + // Covers BookStep::forEachOffer() offer preparation, where + // stpAmt.in = mulRatio(ofrAmt.in, transferRateIn) overflowed. + // The MPT/MPT amounts keep the offer quality reachable while + // applying tokenA's transfer rate overflows the input side. + std::int64_t const poisonPays = 6'148'914'691'236'517'205LL; + std::int64_t const poisonGets = 34'000'000'000'000'000LL; + env(pay(gwB, mallory, tokenB(poisonGets))); + + auto const poisonSeq = env.seq(mallory); + env(offer(mallory, tokenA(poisonPays), tokenB(poisonGets))); + env.close(); + + auto const poisonKeylet = keylet::offer(mallory.id(), SeqProxy::rawSequence(poisonSeq)); + BEAST_EXPECT(env.le(poisonKeylet) != nullptr); + + auto const aliceSeq = env.seq(alice); + env(offer(alice, tokenB(1), tokenA(100))); + env.close(); + + BEAST_EXPECT(env.le(poisonKeylet) == nullptr); + BEAST_EXPECT( + env.le(keylet::offer(alice.id(), SeqProxy::rawSequence(aliceSeq))) != nullptr); + } + + { + Env env{*this, features}; + env.fund(XRP(10'000), issuer, taker); + env.close(); + + MPTTester const token{ + {.env = env, .issuer = issuer, .holders = {taker}, .maxAmt = kMaxMpTokenAmount}}; + + // Give the taker exactly one MPT. If the old rounding overflow + // collapsed the required input to the minimum positive amount, the + // taker could afford the bad fill and the balance checks below + // would catch the economic gain. + env(pay(issuer, taker, token(1))); + env.close(); + + // Covers BookStep::revImp() output reduction. The issuer's offer + // is fully funded and has no transfer fee, so offer preparation + // succeeds. The taker asks for slightly less output, forcing + // limitStepOut() to reduce the offer; that strict reduction used + // to overflow and leave the poison offer on the book. + auto const funded = 1'844'674'407'370'955'162LL; + auto const offerOut = funded + 1; + + auto const poisonSeq = env.seq(issuer); + env(offer(issuer, XRP(1), token(offerOut))); + env.close(); + + auto const poisonKeylet = keylet::offer(issuer.id(), SeqProxy::rawSequence(poisonSeq)); + BEAST_EXPECT(env.le(poisonKeylet) != nullptr); + + auto const issuerXRPBefore = env.balance(issuer, XRP); + auto const takerXRPBefore = env.balance(taker, XRP); + auto const takerMPTBefore = env.balance(taker, token); + auto const fee = env.current()->fees().base; + + auto const takerSeq = env.seq(taker); + env(offer(taker, token(funded), XRP(1))); + env.close(); + + // The former overflow point must not turn into a near-free fill: + // the unusable offer is removed, the taker's offer remains, and no + // value changes hands beyond the taker's transaction fee. + BEAST_EXPECT(env.le(poisonKeylet) == nullptr); + BEAST_EXPECT( + env.le(keylet::offer(taker.id(), SeqProxy::rawSequence(takerSeq))) != nullptr); + BEAST_EXPECT(env.balance(issuer, XRP) == issuerXRPBefore); + BEAST_EXPECT(env.balance(taker, XRP) == takerXRPBefore - fee); + BEAST_EXPECT(env.balance(taker, token) == takerMPTBefore); + } + + { + auto const poisonMaker = Account("poisonMaker"); + + Env env{*this, features}; + env.fund(XRP(10'000), issuer, poisonMaker, taker); + env.close(); + + MPTTester const token{ + {.env = env, + .issuer = issuer, + .holders = {poisonMaker, taker}, + .maxAmt = kMaxMpTokenAmount}}; + + // Covers OfferStream::step() filtering. The offer is mostly + // funded, but reducing it to the actual owner funds inside + // shouldRmSmallIncreasedQOffer() used to overflow before BookStep + // saw the offer. + auto const funded = 1'844'674'407'370'955'162LL; + auto const offerOut = funded + 1; + env(pay(issuer, poisonMaker, token(funded))); + + auto const poisonSeq = env.seq(poisonMaker); + env(offer(poisonMaker, XRP(1), token(offerOut))); + env.close(); + + auto const poisonKeylet = + keylet::offer(poisonMaker.id(), SeqProxy::rawSequence(poisonSeq)); + BEAST_EXPECT(env.le(poisonKeylet) != nullptr); + + auto const takerSeq = env.seq(taker); + env(offer(taker, token(1), XRP(1))); + env.close(); + + BEAST_EXPECT(env.le(poisonKeylet) == nullptr); + BEAST_EXPECT( + env.le(keylet::offer(taker.id(), SeqProxy::rawSequence(takerSeq))) != nullptr); + BEAST_EXPECT(env.balance(poisonMaker, token) == token(funded)); + BEAST_EXPECT(env.balance(taker, token) == token(0)); + } + + { + // Same overflow scenario as the ownerGives case above, but run with + // trace-level logging so BookStep::forEachOffer's removeOffer() + // emits its "Removing offer with overflowing amount calculation" + // trace line. This exercises the JLOG body inside removeOffer, + // which is skipped when logging is above trace severity. + std::string logs; + { + Env env{ + *this, + envconfig(), + features, + std::make_unique(&logs), + beast::Severity::Trace}; + env.fund(XRP(10'000), issuer, taker); + env.close(); + + MPTTester const token{ + {.env = env, .issuer = issuer, .holders = {taker}, .transferFee = 10'000}}; + + std::int64_t const poisonAmount = 8'500'000'000'000'000'000LL; + auto const poisonSeq = env.seq(issuer); + env(offer(issuer, XRP(1), token(poisonAmount))); + env.close(); + + auto const poisonKeylet = + keylet::offer(issuer.id(), SeqProxy::rawSequence(poisonSeq)); + BEAST_EXPECT(env.le(poisonKeylet) != nullptr); + + auto const takerSeq = env.seq(taker); + env(offer(taker, token(100), XRP(100))); + env.close(); + + BEAST_EXPECT(env.le(poisonKeylet) == nullptr); + BEAST_EXPECT( + env.le(keylet::offer(taker.id(), SeqProxy::rawSequence(takerSeq))) != nullptr); + } + BEAST_EXPECT(logs.contains("Removing offer with overflowing amount calculation")); + } + } + void testSelfCrossOffer1(FeatureBitset features) { @@ -4920,6 +5627,7 @@ public: testSellOffer(features); testSellWithFillOrKill(features); testTransferRateOffer(features); + testTransferRateOverflowOffer(features); testSelfCrossOffer(features); testSelfIssueOffer(features); testDirectToDirectPath(features); @@ -4934,8 +5642,11 @@ public: testDeletedOfferIssuer(features); testTicketOffer(features); testTicketCancelOffer(features); + testMPTAMMLimitQualityRounding(features); testRmSmallIncreasedQOffersXRP(features); testRmSmallIncreasedQOffersMPT(features); + testMPTIssuerOfferUsesRemainingCapacity(features); + testPartiallyFundedMPTInputOfferZeroInput(features); testFillOrKill(features); testTickSize(features); testAutoCreateReserve(features); diff --git a/src/test/protocol/STAmount_test.cpp b/src/test/protocol/STAmount_test.cpp index f6c5a94752..c3a681cf01 100644 --- a/src/test/protocol/STAmount_test.cpp +++ b/src/test/protocol/STAmount_test.cpp @@ -1,16 +1,21 @@ #include +#include #include +#include #include #include #include #include #include +#include #include #include #include #include #include +#include +#include #include #include #include @@ -24,6 +29,7 @@ #include #include #include +#include namespace xrpl { @@ -990,6 +996,84 @@ public: } } + void + testMPTRateRounding() + { + testcase("MPT transfer rate rounding uses Number arithmetic"); + + MPTIssue const asset{makeMptID(1, AccountID(0x4985601))}; + Rate const transferRate{1'500'000'000}; + STAmount const largeAmount{asset, UINT64_C(1'230'000'000'000'000'000)}; + STAmount const scaledAmount{asset, UINT64_C(1'845'000'000'000'000'000)}; + + auto rules = [](bool const mptV2) { + // Rules keeps a reference to the presets set, so use static + // storage here rather than a local temporary. + static std::unordered_set> const kNoFeatures; + static std::unordered_set> const kMptV2Features{ + featureMPTokensV2}; + return Rules{mptV2 ? kMptV2Features : kNoFeatures}; + }; + + auto throwsOverflow = [&](auto&& f, bool expected = true) { + bool threw = false; + try + { + f(); + } + catch (std::overflow_error const&) + { + threw = true; + } + BEAST_EXPECT(threw == expected); + }; + + { + CurrentTransactionRulesGuard const rg(rules(false)); + + throwsOverflow([&] { (void)multiplyRound(largeAmount, transferRate, asset, true); }); + throwsOverflow([&] { (void)divideRound(scaledAmount, transferRate, asset, true); }); + } + + { + CurrentTransactionRulesGuard const rg(rules(true)); + + throwsOverflow( + [&] { (void)multiplyRound(largeAmount, transferRate, asset, true); }, false); + throwsOverflow( + [&] { (void)divideRound(scaledAmount, transferRate, asset, true); }, false); + } + + { + CurrentTransactionRulesGuard const rg(rules(true)); + STAmount const one{asset, 1}; + STAmount const two{asset, 2}; + + BEAST_EXPECT(multiplyRound(one, transferRate, asset, true) == two); + BEAST_EXPECT(multiplyRound(one, transferRate, asset, false) == one); + BEAST_EXPECT(divideRound(two, transferRate, asset, true) == two); + BEAST_EXPECT(divideRound(two, transferRate, asset, false) == one); + + BEAST_EXPECT(multiplyRound(largeAmount, transferRate, asset, true) == scaledAmount); + BEAST_EXPECT(divideRound(scaledAmount, transferRate, asset, true) == largeAmount); + } + + { + // mulRound with an integral (XRP) operand whose mantissa is below + // kMinValue exercises the legacy value-scaling loop that normalizes + // the mantissa before multiply. The MPTokensV2 Number path is + // not taken here because the target asset is an IOU. + Issue const usd{Currency(0x5553440000000000), AccountID(0x4985601)}; + STAmount const iouVal{usd, 5}; + STAmount const xrpVal{XRPAmount{7}}; // integral, mantissa < kMinValue + + auto const up = mulRound(iouVal, xrpVal, usd, /*roundUp*/ true); + auto const down = mulRound(iouVal, xrpVal, usd, /*roundUp*/ false); + BEAST_EXPECT(down.signum() > 0); + BEAST_EXPECT(up >= down); + } + } + void testCanSubtractXRP() { @@ -1267,6 +1351,7 @@ public: testCanAddXRP(); testCanAddIOU(); testCanAddMPT(); + testMPTRateRounding(); testCanSubtractXRP(); testCanSubtractIOU(); testCanSubtractMPT();