From 3c1505a29d5c8f9bdee449319009734a023bbdfd Mon Sep 17 00:00:00 2001 From: Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> Date: Tue, 3 Feb 2026 17:26:51 +0000 Subject: [PATCH] avoid repeated normalizations --- include/xrpl/protocol/STAmount.h | 34 ++++++- src/libxrpl/protocol/STAmount.cpp | 6 +- src/test/protocol/STAmount_test.cpp | 145 ++++++++++++++++++++++++++++ 3 files changed, 178 insertions(+), 7 deletions(-) diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index c678205882..fa9bd97b18 100644 --- a/include/xrpl/protocol/STAmount.h +++ b/include/xrpl/protocol/STAmount.h @@ -45,6 +45,9 @@ public: static int const cMinOffset = -96; static int const cMaxOffset = 80; + // The -100 is used to allow 0 to sort less than small positive values + // which will have a large negative exponent. + static int const cZeroOffset = -100; // Maximum native value supported by the code constexpr static std::uint64_t cMinValue = 1'000'000'000'000'000ull; @@ -524,7 +527,32 @@ STAmount::fromNumber(A const& a, Number const& number) auto const [mantissa, exponent] = working.normalizeToRange(cMinValue, cMaxValue); - return STAmount{asset, mantissa, exponent, negative}; + // Special case: normalizeToRange returns mantissa=0 with Number's default + // exponent (std::numeric_limits::lowest()), but STAmount expects zero + // IOUs to have the canonical zero offset. Handle this explicitly. + if (mantissa == 0) + { + return STAmount{asset, 0, cZeroOffset, false, unchecked{}}; + } + + // Handle underflow: if exponent is below minimum or mantissa is too small, + // the value underflows to zero. + if ((exponent < cMinOffset) || (mantissa < cMinValue)) + { + return STAmount{asset, 0, cZeroOffset, false, unchecked{}}; + } + + // Handle overflow: if exponent exceeds maximum, throw. + if (exponent > cMaxOffset) + Throw("value overflow"); + + // normalizeToRange already produced canonical mantissa/exponent in the range + // [cMinValue, cMaxValue], so bypass canonicalize() to avoid redundant work. + XRPL_ASSERT( + mantissa >= cMinValue && mantissa <= cMaxValue, "xrpl::STAmount::fromNumber : mantissa in canonical range"); + XRPL_ASSERT( + exponent >= cMinOffset && exponent <= cMaxOffset, "xrpl::STAmount::fromNumber : exponent in canonical range"); + return STAmount{asset, static_cast(mantissa), exponent, negative, unchecked{}}; } inline void @@ -537,9 +565,7 @@ STAmount::negate() inline void STAmount::clear() { - // The -100 is used to allow 0 to sort less than a small positive values - // which have a negative exponent. - mOffset = integral() ? 0 : -100; + mOffset = integral() ? 0 : cZeroOffset; mValue = 0; mIsNegative = false; } diff --git a/src/libxrpl/protocol/STAmount.cpp b/src/libxrpl/protocol/STAmount.cpp index b7c33bb907..682f73fcf3 100644 --- a/src/libxrpl/protocol/STAmount.cpp +++ b/src/libxrpl/protocol/STAmount.cpp @@ -866,7 +866,7 @@ STAmount::canonicalize() if (mValue == 0) { - mOffset = -100; + mOffset = cZeroOffset; mIsNegative = false; return; } @@ -890,7 +890,7 @@ STAmount::canonicalize() { mValue = 0; mIsNegative = false; - mOffset = -100; + mOffset = cZeroOffset; return; } @@ -903,7 +903,7 @@ STAmount::canonicalize() XRPL_ASSERT( (mValue == 0) || ((mOffset >= cMinOffset) && (mOffset <= cMaxOffset)), "xrpl::STAmount::canonicalize : offset inside range"); - XRPL_ASSERT((mValue != 0) || (mOffset != -100), "xrpl::STAmount::canonicalize : value or offset set"); + XRPL_ASSERT((mValue != 0) || (mOffset == cZeroOffset), "xrpl::STAmount::canonicalize : value or offset set"); } void diff --git a/src/test/protocol/STAmount_test.cpp b/src/test/protocol/STAmount_test.cpp index 10e159dd8d..6f1ff4c4a5 100644 --- a/src/test/protocol/STAmount_test.cpp +++ b/src/test/protocol/STAmount_test.cpp @@ -1,7 +1,9 @@ #include +#include #include #include +#include #include #include @@ -1143,6 +1145,148 @@ public: } } + void + testNumberConversion() + { + testcase("Number to STAmount conversions"); + + Issue const usd{Currency(0x5553440000000000), AccountID(0x4985601)}; + NumberSO stNumberSO{true}; + + // Test zero conversion + { + Number const zero{}; + STAmount const result{usd, zero}; + BEAST_EXPECT(result.mantissa() == 0); + BEAST_EXPECT(result.exponent() == STAmount::cZeroOffset); + BEAST_EXPECT(!result.negative()); + } + + // Test positive zero + { + Number const zero{0, 0}; + STAmount const result{usd, zero}; + BEAST_EXPECT(result.mantissa() == 0); + BEAST_EXPECT(result.exponent() == STAmount::cZeroOffset); + } + + // Test negative zero (should become positive zero) + { + Number const negZero{-0, 0}; + STAmount const result{usd, negZero}; + BEAST_EXPECT(result.mantissa() == 0); + BEAST_EXPECT(!result.negative()); + } + + // Test minimum positive IOU amount + { + Number const minPos{STAmount::cMinValue, STAmount::cMinOffset}; + STAmount const result{usd, minPos}; + BEAST_EXPECT(result.mantissa() == STAmount::cMinValue); + BEAST_EXPECT(result.exponent() == STAmount::cMinOffset); + BEAST_EXPECT(!result.negative()); + } + + // Test maximum positive IOU amount + { + Number const maxPos{STAmount::cMaxValue, STAmount::cMaxOffset}; + STAmount const result{usd, maxPos}; + BEAST_EXPECT(result.mantissa() == STAmount::cMaxValue); + BEAST_EXPECT(result.exponent() == STAmount::cMaxOffset); + BEAST_EXPECT(!result.negative()); + } + + // Test negative amounts + { + Number const neg{-static_cast(STAmount::cMinValue), STAmount::cMinOffset}; + STAmount const result{usd, neg}; + BEAST_EXPECT(result.mantissa() == STAmount::cMinValue); + BEAST_EXPECT(result.exponent() == STAmount::cMinOffset); + BEAST_EXPECT(result.negative()); + } + + // Test value requiring scale up (mantissa too small) + { + Number const small{1000000000000000ull / 10, -95}; // Will scale up + STAmount const result{usd, small}; + BEAST_EXPECT(result.mantissa() >= STAmount::cMinValue); + BEAST_EXPECT(result.mantissa() <= STAmount::cMaxValue); + BEAST_EXPECT(result.exponent() >= STAmount::cMinOffset); + BEAST_EXPECT(result.exponent() <= STAmount::cMaxOffset); + } + + // Test value requiring scale down (mantissa too large) + { + Number const large{9999999999999999ull * 10, 79}; // Will scale down + STAmount const result{usd, large}; + BEAST_EXPECT(result.mantissa() >= STAmount::cMinValue); + BEAST_EXPECT(result.mantissa() <= STAmount::cMaxValue); + BEAST_EXPECT(result.exponent() >= STAmount::cMinOffset); + BEAST_EXPECT(result.exponent() <= STAmount::cMaxOffset); + } + + // Test boundary mantissa values + { + Number const atMin{STAmount::cMinValue, 0}; + STAmount const result{usd, atMin}; + BEAST_EXPECT(result.mantissa() == STAmount::cMinValue); + } + + { + Number const atMax{STAmount::cMaxValue, 0}; + STAmount const result{usd, atMax}; + BEAST_EXPECT(result.mantissa() == STAmount::cMaxValue); + } + + // Test typical amounts + { + Number const typical{1234567890123456ull, -10}; + STAmount const result{usd, typical}; + BEAST_EXPECT(result.mantissa() >= STAmount::cMinValue); + BEAST_EXPECT(result.mantissa() <= STAmount::cMaxValue); + BEAST_EXPECT(result.exponent() >= STAmount::cMinOffset); + BEAST_EXPECT(result.exponent() <= STAmount::cMaxOffset); + } + + // Test round-trip conversion (Number -> STAmount -> Number) + { + Number const original{5000000000000000ull, 5}; + STAmount const st{usd, original}; + Number const recovered{st}; + BEAST_EXPECT(original == recovered); + } + + // Test various exponents + for (int exp = STAmount::cMinOffset; exp <= STAmount::cMaxOffset; exp += 10) + { + Number const n{STAmount::cMinValue, exp}; + STAmount const result{usd, n}; + BEAST_EXPECT(result.mantissa() >= STAmount::cMinValue); + BEAST_EXPECT(result.mantissa() <= STAmount::cMaxValue); + BEAST_EXPECT(result.exponent() >= STAmount::cMinOffset); + BEAST_EXPECT(result.exponent() <= STAmount::cMaxOffset); + } + + // Test both mantissa scales (if applicable) + { + // Small mantissa scale test + NumberMantissaScaleGuard guard{MantissaRange::small}; + Number const n{5000000000000000ull, 5}; + STAmount const result{usd, n}; + BEAST_EXPECT(result.mantissa() >= STAmount::cMinValue); + BEAST_EXPECT(result.mantissa() <= STAmount::cMaxValue); + } + + { + // Large mantissa scale test + NumberMantissaScaleGuard guard{MantissaRange::large}; + Number const n{5000000000000000ull, 5}; + STAmount const result{usd, n}; + BEAST_EXPECT(result.mantissa() >= STAmount::cMinValue); + BEAST_EXPECT(result.mantissa() <= STAmount::cMaxValue); + } + } + //-------------------------------------------------------------------------- void @@ -1157,6 +1301,7 @@ public: testParseJson(); testConvertXRP(); testConvertIOU(); + testNumberConversion(); testCanAddXRP(); testCanAddIOU(); testCanAddMPT();