ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-06-03 08:46:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix MPT transfer-rate rounding overflow in STAmount

Browse Source
This commit is contained in:
Gregory Tsipenyuk
2026-05-18 14:19:11 -04:00
parent a27596f988
commit 22fbf4d060
5 changed files with 177 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

78
src/libxrpl/protocol/STAmount.cpp
View File

@@ -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<MPTIssue>() && 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<MPTIssue>() && 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

2
src/test/app/AMMExtendedMPT_test.cpp
View File

@@ -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

8
src/test/app/AMMMPT_test.cpp
View File

@@ -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)));
}

34
src/test/app/OfferMPT_test.cpp
View File

@@ -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);

69
src/test/protocol/STAmount_test.cpp
View File

@@ -6,11 +6,14 @@
#include <xrpl/json/json_forwards.h>
#include <xrpl/json/json_value.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/IOUAmount.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/Issue.h>
#include <xrpl/protocol/MPTAmount.h>
#include <xrpl/protocol/MPTIssue.h>
#include <xrpl/protocol/Rate.h>
#include <xrpl/protocol/Rules.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STAmount.h>
#include <xrpl/protocol/Serializer.h>
@@ -24,6 +27,7 @@
#include <string>
#include <type_traits>
#include <typeinfo>
#include <unordered_set>
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<uint256, beast::Uhash<>> const kNO_FEATURES;
static std::unordered_set<uint256, beast::Uhash<>> 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();