diff --git a/src/libxrpl/protocol/STAmount.cpp b/src/libxrpl/protocol/STAmount.cpp index 20d3db45c0..82da721b9d 100644 --- a/src/libxrpl/protocol/STAmount.cpp +++ b/src/libxrpl/protocol/STAmount.cpp @@ -1530,6 +1530,45 @@ 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}; + + if (roundUp && !resultNegative && !result) + { + // Preserve existing mulRound/divRound behavior for a positive result + // too small to represent in the target asset. + if (asset.integral()) + return STAmount{asset, 1}; + return STAmount{asset, STAmount::kMIN_VALUE, STAmount::kMIN_OFFSET, false}; + } + + return result; +} + } // anonymous namespace // Pass the canonicalizeRound function pointer as a template parameter. @@ -1571,6 +1610,23 @@ 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) && + getSTNumberSwitchover()) + { + // 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(); @@ -1591,9 +1647,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 @@ -1660,6 +1713,23 @@ 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) && + getSTNumberSwitchover()) + { + // 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(); @@ -1681,8 +1751,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/test/app/AMMExtendedMPT_test.cpp b/src/test/app/AMMExtendedMPT_test.cpp index 9583318382..ae0926482f 100644 --- a/src/test/app/AMMExtendedMPT_test.cpp +++ b/src/test/app/AMMExtendedMPT_test.cpp @@ -1086,7 +1086,7 @@ private: BEAST_EXPECT(ammCarol.expectBalances( 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'419), aBux(200'215'509'984'420)}}})); + env, cam, 1, {{Amounts{bBux(200'215'509'984'419), aBux(200'215'509'984'419)}}})); } void diff --git a/src/test/app/AMMMPT_test.cpp b/src/test/app/AMMMPT_test.cpp index a591410146..309a796384 100644 --- a/src/test/app/AMMMPT_test.cpp +++ b/src/test/app/AMMMPT_test.cpp @@ -5038,15 +5038,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))); } diff --git a/src/test/app/OfferMPT_test.cpp b/src/test/app/OfferMPT_test.cpp index 15decf5f1b..12585db49c 100644 --- a/src/test/app/OfferMPT_test.cpp +++ b/src/test/app/OfferMPT_test.cpp @@ -3177,10 +3177,36 @@ public: auto const issuer = Account("issuer"); auto const taker = Account("taker"); - // Each scenario below targets a specific 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(); + + 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 = kMAX_MP_TOKEN_AMOUNT}}; + + // 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(), 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); diff --git a/src/test/protocol/STAmount_test.cpp b/src/test/protocol/STAmount_test.cpp index 7c37d3e9bc..38d6921047 100644 --- a/src/test/protocol/STAmount_test.cpp +++ b/src/test/protocol/STAmount_test.cpp @@ -6,11 +6,14 @@ #include #include #include +#include #include #include #include #include #include +#include +#include #include #include #include @@ -24,6 +27,7 @@ #include #include #include +#include namespace xrpl { @@ -1039,6 +1043,70 @@ 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, 1'230'000'000'000'000'000LL}; + STAmount const scaledAmount{asset, 1'845'000'000'000'000'000LL}; + + 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 kNO_FEATURES; + static std::unordered_set> const kMPT_V2_FEATURES{ + featureMPTokensV2}; + return Rules{mptV2 ? kMPT_V2_FEATURES : kNO_FEATURES}; + }; + + auto throwsOverflow = [&](auto&& f) { + bool threw = false; + try + { + f(); + } + catch (std::overflow_error const&) + { + threw = true; + } + BEAST_EXPECT(threw); + }; + + { + CurrentTransactionRulesGuard const rg(rules(false)); + NumberSO const numberSO{true}; + + throwsOverflow([&] { (void)multiplyRound(largeAmount, transferRate, asset, true); }); + throwsOverflow([&] { (void)divideRound(scaledAmount, transferRate, asset, true); }); + } + + { + CurrentTransactionRulesGuard const rg(rules(true)); + NumberSO const numberSO{false}; + + throwsOverflow([&] { (void)multiplyRound(largeAmount, transferRate, asset, true); }); + throwsOverflow([&] { (void)divideRound(scaledAmount, transferRate, asset, true); }); + } + + { + CurrentTransactionRulesGuard const rg(rules(true)); + NumberSO const numberSO{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); + } + } + void testCanSubtractXRP() { @@ -1224,6 +1292,7 @@ public: testCanAddXRP(); testCanAddIOU(); testCanAddMPT(); + testMPTRateRounding(); testCanSubtractXRP(); testCanSubtractIOU(); testCanSubtractMPT();