From 8306ac7710d86bdfd26a84597fc7769afbd3f539 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Fri, 10 Jul 2026 20:18:31 -0400 Subject: [PATCH] fix: Improve Number addition/subtraction rounding (#7369) Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- include/xrpl/basics/Number.h | 123 +++--- src/libxrpl/basics/Number.cpp | 593 ++++++++++++++++++---------- src/libxrpl/protocol/Rules.cpp | 45 ++- src/libxrpl/protocol/STNumber.cpp | 5 +- src/test/app/Invariants_test.cpp | 7 +- src/tests/libxrpl/basics/Number.cpp | 575 +++++++++++++++++++++++---- 6 files changed, 994 insertions(+), 354 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 073da12f89..28c7b1dbda 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -14,7 +14,6 @@ #include #include #include -#include #include namespace xrpl { @@ -55,37 +54,43 @@ namespace detail { * compile time. Doing it at runtime would be pretty wasteful and * inefficient. */ -constexpr std::size_t kInt64Digits = 20; -consteval std::array +constexpr std::size_t kUint64Digits = 20; +[[maybe_unused]] constexpr std::size_t kUint128Digits = 39; + +template +consteval std::array buildPowersOfTen() { - std::array result{}; + std::array result{}; - std::uint64_t power = 1; + T power = 1; std::size_t exponent = 0; // end the loop early so it doesn't overflow; for (; exponent < result.size() - 1; ++exponent, power *= 10) { result[exponent] = power; - if (power > std::numeric_limits::max() / 10) + if (power > std::numeric_limits::max() / 10) throw std::logic_error("Power of 10 table is too big"); } result[exponent] = power; - if (power < std::numeric_limits::max() / 10) - throw std::logic_error("Power of 10 table is not big enough for the uint64_t type"); + if (power < std::numeric_limits::max() / 10) + throw std::logic_error("Power of 10 table is not big enough for the given type"); return result; } } // namespace detail -constexpr std::array kPowerOfTen = detail::buildPowersOfTen(); +template +constexpr std::array kPowerOfTenImpl = detail::buildPowersOfTen(); + +constexpr auto kPowerOfTen = kPowerOfTenImpl; static_assert(kPowerOfTen[0] == 1); static_assert(kPowerOfTen[1] == 10); static_assert(kPowerOfTen[10] == 10'000'000'000); static_assert( - isPowerOfTen(kPowerOfTen.back()) && *logTen(kPowerOfTen.back()) == detail::kInt64Digits - 1); + isPowerOfTen(kPowerOfTen.back()) && *logTen(kPowerOfTen.back()) == detail::kUint64Digits - 1); /** MantissaRange defines a range for the mantissa of a normalized Number. * @@ -124,17 +129,37 @@ struct MantissaRange final { using rep = std::uint64_t; + // NOLINTBEGIN(readability-enum-initial-value) + // The values don't matter, except for Large enum class MantissaScale { + // Small can be removed when either featureSingleAssetVault or featureLendingProtocol are + // retired Small, // LargeLegacy can be removed when fixCleanup3_2_0 is retired LargeLegacy, - Large, + // Large320 can be removed when fixCleanup3_3_0 is retired + Large320, + // If Large330 is ever the only remaining "Large*" entry, it can be renamed to just "Large". + Large330, + // Large is a de-facto alias for "the latest", and is only here for backward compatibility + // in the extremely unlikely case that a downstream project made use of it. Note that + // because the behavior changed, this may still be a breaking change. + Large = Large330, }; + // NOLINTEND(readability-enum-initial-value) - // This entire enum can be removed when fixCleanup3_2_0 is retired - enum class CuspRoundingFix : bool { - Disabled = false, - Enabled = true, + // This entire enum can be removed when the last relevant amendment is retired + enum class CuspRoundingFix : std::uint8_t { + // Disabled can be removed when fixCleanup3_2_0 is retired + Disabled = 0, + // Enabled320 can be removed when fixCleanup3_3_0 is retired + Enabled320 = 1, + // If we ever get to the point that there's only one entry, remove the entire enum + Enabled330 = 2, + // Enabled is a de-facto alias for "the latest", and is only here for backward compatibility + // in the extremely unlikely case that a downstream project made use of it. Note that + // because the behavior changed, this may still be a breaking change. + Enabled = Enabled330, }; explicit constexpr MantissaRange(MantissaScale sc) : scale(sc) @@ -145,13 +170,27 @@ struct MantissaRange final int const log{getExponent(scale)}; rep const min{getMin(scale, log)}; rep const max{(min * 10) - 1}; - CuspRoundingFix const cuspRoundingFixEnabled{isCuspFixEnabled(scale)}; - - static MantissaRange const& - getMantissaRange(MantissaScale scale); + CuspRoundingFix const cuspRoundingFix{isCuspFixEnabled(scale)}; static std::set const& - getAllScales(); + getAllScales() + { + static std::set const kScales = { + MantissaRange::MantissaScale::Small, + MantissaRange::MantissaScale::LargeLegacy, + MantissaRange::MantissaScale::Large320, + MantissaRange::MantissaScale::Large330, + }; + return kScales; + } + + class Access + { + static constexpr MantissaRange const& + mantissaRange(MantissaScale scale); + + friend Number; + }; private: static constexpr int @@ -162,7 +201,8 @@ private: case MantissaScale::Small: return 15; case MantissaScale::LargeLegacy: - case MantissaScale::Large: + case MantissaScale::Large320: + case MantissaScale::Large330: return 18; // LCOV_EXCL_START default: @@ -191,17 +231,16 @@ private: case MantissaScale::Small: case MantissaScale::LargeLegacy: return CuspRoundingFix::Disabled; - case MantissaScale::Large: - return CuspRoundingFix::Enabled; + case MantissaScale::Large320: + return CuspRoundingFix::Enabled320; + case MantissaScale::Large330: + return CuspRoundingFix::Enabled330; default: // If called in a constexpr context, this throw assures that the build fails if an // invalid scale is used. throw std::runtime_error("Unknown mantissa scale"); // LCOV_EXCL_LINE } } - - static std::unordered_map const& - getRanges(); }; // Like std::integral, but only 64-bit integral types. @@ -554,9 +593,15 @@ private: // changing the values inside the range. static thread_local std::reference_wrapper kRange; + class Guard; + void normalize(MantissaRange const& range); + // Guard has the fields that we need, as well as MantissaRange, so if we have a guard, use that + void + normalize(Guard const& guard); + /** Normalize Number components to an arbitrary range. * * min/maxMantissa are parameters because this function is used by both @@ -571,7 +616,7 @@ private: int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled); + MantissaRange::CuspRoundingFix cuspRoundingFix); template friend void @@ -581,7 +626,7 @@ private: int& exponent, MantissaRange::rep const& minMantissa, MantissaRange::rep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, + MantissaRange::CuspRoundingFix cuspRoundingFix, bool dropped); [[nodiscard]] bool @@ -599,8 +644,6 @@ private: // UB, and can vary across compilers. static internalrep externalToInternal(rep mantissa); - - class Guard; }; constexpr Number::Number(bool negative, internalrep mantissa, int exponent, Unchecked) noexcept @@ -862,21 +905,11 @@ squelch(Number const& x, Number const& limit) noexcept return x; } -inline std::string -to_string(MantissaRange::MantissaScale const& scale) -{ - switch (scale) - { - case MantissaRange::MantissaScale::Small: - return "small"; - case MantissaRange::MantissaScale::LargeLegacy: - return "largeLegacy"; - case MantissaRange::MantissaScale::Large: - return "large"; - default: - throw std::runtime_error("Bad scale"); - } -} +std::string +to_string(MantissaRange::MantissaScale const& scale); + +std::string +to_string(Number::RoundingMode const& round); class SaveNumberRoundMode { diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 4d7a821040..0a1d558421 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #ifdef _MSC_VER @@ -31,74 +30,103 @@ namespace xrpl { thread_local Number::RoundingMode Number::mode = Number::RoundingMode::ToNearest; thread_local std::reference_wrapper Number::kRange = - MantissaRange::getMantissaRange(MantissaRange::MantissaScale::Large); + MantissaRange::Access::mantissaRange(MantissaRange::MantissaScale::Large330); -std::set const& -MantissaRange::getAllScales() +std::string +to_string(MantissaRange::MantissaScale const& scale) { - static std::set const kScales = { - MantissaRange::MantissaScale::Small, - MantissaRange::MantissaScale::LargeLegacy, - MantissaRange::MantissaScale::Large, - }; - return kScales; + switch (scale) + { + case MantissaRange::MantissaScale::Small: + return "Small"; + case MantissaRange::MantissaScale::LargeLegacy: + return "LargeLegacy"; + case MantissaRange::MantissaScale::Large320: + return "Large320"; + case MantissaRange::MantissaScale::Large330: + return "Large330"; + default: + throw std::runtime_error("Bad scale"); // LCOV_EXCL_LINE + } } -std::unordered_map const& -MantissaRange::getRanges() +std::string +to_string(Number::RoundingMode const& round) { - static auto const kMap = []() { - std::unordered_map map; - for (auto const scale : getAllScales()) - { - map.emplace(scale, scale); - } - - // Use these constexpr declarations to do static_asserts to verify the MantissaRanges are - // created correctly, but nothing else. - { - [[maybe_unused]] - constexpr static MantissaRange kRange{MantissaRange::MantissaScale::Small}; - static_assert(isPowerOfTen(kRange.min)); - static_assert(kRange.min == 1'000'000'000'000'000LL); - static_assert(kRange.max == 9'999'999'999'999'999LL); - static_assert(kRange.log == 15); - static_assert(kRange.min < Number::kMaxRep); - static_assert(kRange.max < Number::kMaxRep); - static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); - } - { - [[maybe_unused]] - constexpr static MantissaRange kRange{MantissaRange::MantissaScale::LargeLegacy}; - static_assert(isPowerOfTen(kRange.min)); - static_assert(kRange.min == 1'000'000'000'000'000'000ULL); - static_assert(kRange.max == rep(9'999'999'999'999'999'999ULL)); - static_assert(kRange.log == 18); - static_assert(kRange.min < Number::kMaxRep); - static_assert(kRange.max > Number::kMaxRep); - static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); - } - { - [[maybe_unused]] - constexpr static MantissaRange kRange{MantissaRange::MantissaScale::Large}; - static_assert(isPowerOfTen(kRange.min)); - static_assert(kRange.min == 1'000'000'000'000'000'000ULL); - static_assert(kRange.max == rep(9'999'999'999'999'999'999ULL)); - static_assert(kRange.log == 18); - static_assert(kRange.min < Number::kMaxRep); - static_assert(kRange.max > Number::kMaxRep); - static_assert(kRange.cuspRoundingFixEnabled == CuspRoundingFix::Enabled); - } - return map; - }(); - - return kMap; + switch (round) + { + case Number::RoundingMode::ToNearest: + return "ToNearest"; + case Number::RoundingMode::TowardsZero: + return "TowardsZero"; + case Number::RoundingMode::Downward: + return "Downward"; + case Number::RoundingMode::Upward: + return "Upward"; + default: + throw std::runtime_error("Bad rounding mode"); // LCOV_EXCL_LINE + } } -MantissaRange const& -MantissaRange::getMantissaRange(MantissaScale scale) +constexpr MantissaRange const& +MantissaRange::Access::mantissaRange(MantissaScale scale) { - return getRanges().at(scale); + static constexpr MantissaRange kSmall{MantissaScale::Small}; + static constexpr MantissaRange kLegacy{MantissaScale::LargeLegacy}; + static constexpr MantissaRange kLarge320{MantissaScale::Large320}; + static constexpr MantissaRange kLarge330{MantissaScale::Large330}; + + switch (scale) + { + case MantissaScale::Small: + return kSmall; + case MantissaScale::LargeLegacy: + return kLegacy; + case MantissaScale::Large320: + return kLarge320; + case MantissaScale::Large330: + return kLarge330; + } + throw std::logic_error("Unknown mantissa scale"); + + // static_asserts are checked at compile time, so it doesn't matter where in the function they + // are located. For readability of the main body, put them after it. + + // Small + static_assert(isPowerOfTen(kSmall.min)); + static_assert(kSmall.min == 1'000'000'000'000'000LL); + static_assert(kSmall.max == 9'999'999'999'999'999LL); + static_assert(kSmall.log == 15); + static_assert(kSmall.min < Number::kMaxRep); + static_assert(kSmall.max < Number::kMaxRep); + static_assert(kSmall.cuspRoundingFix == CuspRoundingFix::Disabled); + + // LargeLegacy + static_assert(isPowerOfTen(kLegacy.min)); + static_assert(kLegacy.min == 1'000'000'000'000'000'000ULL); + static_assert(kLegacy.max == rep(9'999'999'999'999'999'999ULL)); + static_assert(kLegacy.log == 18); + static_assert(kLegacy.min < Number::kMaxRep); + static_assert(kLegacy.max > Number::kMaxRep); + static_assert(kLegacy.cuspRoundingFix == CuspRoundingFix::Disabled); + + // Large320 + static_assert(isPowerOfTen(kLarge320.min)); + static_assert(kLarge320.min == 1'000'000'000'000'000'000ULL); + static_assert(kLarge320.max == rep(9'999'999'999'999'999'999ULL)); + static_assert(kLarge320.log == 18); + static_assert(kLarge320.min < Number::kMaxRep); + static_assert(kLarge320.max > Number::kMaxRep); + static_assert(kLarge320.cuspRoundingFix == CuspRoundingFix::Enabled320); + + // Large330 + static_assert(isPowerOfTen(kLarge330.min)); + static_assert(kLarge330.min == 1'000'000'000'000'000'000ULL); + static_assert(kLarge330.max == rep(9'999'999'999'999'999'999ULL)); + static_assert(kLarge330.log == 18); + static_assert(kLarge330.min < Number::kMaxRep); + static_assert(kLarge330.max > Number::kMaxRep); + static_assert(kLarge330.cuspRoundingFix == CuspRoundingFix::Enabled330); } Number::RoundingMode @@ -124,7 +152,7 @@ Number::setMantissaScale(MantissaRange::MantissaScale scale) { if (!MantissaRange::getAllScales().contains(scale)) logicError("Unknown mantissa scale"); - kRange = MantissaRange::getMantissaRange(scale); + kRange = MantissaRange::Access::mantissaRange(scale); } // Optimization equivalent to: @@ -155,15 +183,38 @@ divu10(uint128_t& u) return r; } -// Guard - -// The Guard class is used to temporarily add extra digits of -// precision to an operation. This enables the final result -// to be correctly rounded to the internal precision of Number. - template concept UnsignedMantissa = std::is_unsigned_v || std::is_same_v; +/** Guard + + The Guard class is used to temporarily add extra digits of + precision to an operation. This enables the final result + to be correctly rounded to the internal precision of Number. + + At its core, the Guard really only needs three pieces of information to determine how to round: + 1. The rounding mode + 2. The last digit dropped from the mantissa (i.e. the first digit after the decimal point). + (first byte of digits_) + 3. Whether any other non-zero digits were dropped from the mantissa. (remaining bytes of digits_ + and xbit_) + + Upward and Downward rounding modes round the unsigned mantissa toward or away from zero + depending on whether the sign is negative (sbit_). For positive values, Upward is away, and + Downward is toward. For negative values, that's reversed. For simplicity, I'm going to describe + the logic using "TowardZero" and "AwayFromZero". + + * TowardZero is the easiest rounding mode. It always rounds down. digits_ and xbit_ are + irrelevant. + * AwayFromZero is almost as simple. If both "digits_" and "xbit_" are zero (0), it rounds down. + Else it rounds up. + * ToNearest is only a little more complicated. If the last dropped digit is < 5, then round + down. If it is > 5, round up. If it is exactly 5, and there are _any_ other digits (the + remainder of "digits_" or "xbit_"), round up, else round to even. + + The current implementation stores 16 digits in "digits_" so that digits can be "pop"ped back + out if needed during subtraction (negative addition) operations. +*/ class Number::Guard { std::uint64_t digits_{0}; // 16 decimal guard digits @@ -171,7 +222,21 @@ class Number::Guard std::uint8_t sbit_ : 1 {0}; // the sign of the guard digits public: - explicit Guard() = default; + internalrep const minMantissa; + internalrep const maxMantissa; + MantissaRange::CuspRoundingFix const cuspRoundingFix; + + explicit Guard( + internalrep const& minMantissa, + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFix) + : minMantissa(minMantissa), maxMantissa(maxMantissa), cuspRoundingFix(cuspRoundingFix) + { + } + + explicit Guard(MantissaRange const& range) : Guard(range.min, range.max, range.cuspRoundingFix) + { + } // set & test the sign bit void @@ -194,6 +259,10 @@ public: unsigned pop() noexcept; + // if true, there are no digits in the guard, including dropped digits (xbit_) + [[nodiscard]] bool + empty() const noexcept; + /** Drop a digit from the mantissa, and increment the exponent, storing the dropped digit in * this Guard. * @@ -206,28 +275,37 @@ public: void doDropDigit(T& mantissa, int& exponent) noexcept; + enum class Round { + // The result is exact. No rounding is needed. Only used if cuspRoundingFix is Enabled330 or + // higher. + Exact = -2, + // Round down. Since we use integer math, that usually means no change is needed. + // Exceptions are for when the result is between kMaxRep and kMaxRepUp (round to kMaxRep), + // or after subtraction where _any_ remainder will modify the result. The latter is what + // distinguishes Exact from Down. + Down = -1, + // The result was exactly half-way between two integers. This will round to even. + Even = 0, + // Round up. Always adds 1 (or subtracts 1 in some cases if cuspRoundingFix is not + // Enabled) + Up = 1, + }; + // Indicate round direction: 1 is up, -1 is down, 0 is even // This enables the client to round towards nearest, and on // tie, round towards even. - [[nodiscard]] int + [[nodiscard]] Round round() const noexcept; // Modify the result to the correctly rounded value template void - doRoundUp( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa, - internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, - std::string location); + doRoundUp(bool& negative, T& mantissa, int& exponent, std::string location); // Modify the result to the correctly rounded value template void - doRoundDown(bool& negative, T& mantissa, int& exponent, internalrep const& minMantissa); + doRoundDown(bool& negative, T& mantissa, int& exponent); // Modify the result to the correctly rounded value void @@ -239,7 +317,7 @@ private: template void - bringIntoRange(bool& negative, T& mantissa, int& exponent, internalrep const& minMantissa); + bringIntoRange(bool& negative, T& mantissa, int& exponent); }; inline void @@ -289,6 +367,12 @@ Number::Guard::pop() noexcept return d; } +inline bool +Number::Guard::empty() const noexcept +{ + return digits_ == 0 && !xbit_; +} + template void Number::Guard::doDropDigit(T& mantissa, int& exponent) noexcept @@ -314,50 +398,52 @@ Number::Guard::doDropDigit(uint128_t& mantissa, int& exponent) noexce // -1 if Guard is less than half // 0 if Guard is exactly half // 1 if Guard is greater than half -int +Number::Guard::Round Number::Guard::round() const noexcept { - auto mode = Number::getround(); + // Local "mode" shadows and has the same value as the static thread_local "Number::mode". + // This ensures the overhead of loading the thread_local is only incurred once. + auto const mode = Number::getround(); + + if (cuspRoundingFix >= MantissaRange::CuspRoundingFix::Enabled330 && empty()) + { + // No remainder + return Round::Exact; + } if (mode == RoundingMode::TowardsZero) - return -1; + return Round::Down; - if (mode == RoundingMode::Downward) + // Also Towards Zero + if ((mode == RoundingMode::Downward && !sbit_) || (mode == RoundingMode::Upward && sbit_)) { - if (sbit_) - { - if (digits_ > 0 || xbit_) - return 1; - } - return -1; + return Round::Down; } - if (mode == RoundingMode::Upward) + // Away from Zero. Since we checked sbit_ in the previous block, we don't need to check it + // again. + if (mode == RoundingMode::Downward || mode == RoundingMode::Upward) { - if (sbit_) - return -1; - if (digits_ > 0 || xbit_) - return 1; - return -1; + if (empty()) + return Round::Down; + return Round::Up; } + XRPL_ASSERT( + mode == RoundingMode::ToNearest, "xrpl::Number::Guard::Round : fallthrough to ToNearest"); // assume round to nearest if mode is not one of the predefined values if (digits_ > 0x5000'0000'0000'0000) - return 1; + return Round::Up; if (digits_ < 0x5000'0000'0000'0000) - return -1; + return Round::Down; if (xbit_) - return 1; - return 0; + return Round::Up; + return Round::Even; } template void -Number::Guard::bringIntoRange( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa) +Number::Guard::bringIntoRange(bool& negative, T& mantissa, int& exponent) { // Bring mantissa back into the minMantissa / maxMantissa range AFTER // rounding @@ -378,22 +464,15 @@ Number::Guard::bringIntoRange( template void -Number::Guard::doRoundUp( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa, - internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, - std::string location) +Number::Guard::doRoundUp(bool& negative, T& mantissa, int& exponent, std::string location) { auto r = round(); - if (r == 1 || (r == 0 && (mantissa & 1) == 1)) + if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) { - auto const safeToIncrement = [&maxMantissa](auto const& mantissa) { + auto const safeToIncrement = [this](auto const& mantissa) { return mantissa < maxMantissa && mantissa < kMaxRep; }; - if (cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled) + if (cuspRoundingFix != MantissaRange::CuspRoundingFix::Disabled) { // Ensure mantissa after incrementing fits within both the // min/maxMantissa range and is a valid "rep". @@ -414,14 +493,7 @@ Number::Guard::doRoundUp( safeToIncrement(mantissa), "xrpl::Number::Guard::doRoundUp", "can't recurse more than once"); - doRoundUp( - negative, - mantissa, - exponent, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - location); + doRoundUp(negative, mantissa, exponent, location); return; } } @@ -440,30 +512,43 @@ Number::Guard::doRoundUp( } } } - bringIntoRange(negative, mantissa, exponent, minMantissa); + bringIntoRange(negative, mantissa, exponent); if (exponent > kMaxExponent) Throw(std::string(location)); } template void -Number::Guard::doRoundDown( - bool& negative, - T& mantissa, - int& exponent, - internalrep const& minMantissa) +Number::Guard::doRoundDown(bool& negative, T& mantissa, int& exponent) { auto r = round(); - if (r == 1 || (r == 0 && (mantissa & 1) == 1)) + if (cuspRoundingFix >= MantissaRange::CuspRoundingFix::Enabled330) { - --mantissa; - if (mantissa < minMantissa) + // If there was any remainder, subtract 1 from the result. This is sufficient to get the + // best rounding. + XRPL_ASSERT( + r == Round::Exact || mantissa > maxMantissa, + "xrpl::Number::Guard::doRoundDown : mantissa is expected size"); + if (r != Round::Exact) { - mantissa *= 10; - --exponent; + --mantissa; } } - bringIntoRange(negative, mantissa, exponent, minMantissa); + else + { + // Need to preserve the incorrect behavior until the fix amendment can be retired, + // because otherwise would risk an unplanned ledger fork. + if (r == Round::Up || (r == Round::Even && (mantissa & 1) == 1)) + { + --mantissa; + if (mantissa < minMantissa) + { + mantissa *= 10; + --exponent; + } + } + } + bringIntoRange(negative, mantissa, exponent); } // Modify the result to the correctly rounded value @@ -471,7 +556,7 @@ void Number::Guard::doRound(rep& drops, std::string location) const { auto r = round(); - if (r == 1 || (r == 0 && (drops & 1) == 1)) + if (r == Round::Up || (r == Round::Even && (drops & 1) == 1)) { if (drops >= kMaxRep) { @@ -530,7 +615,7 @@ doNormalize( int& exponent, MantissaRange::rep const& minMantissa, MantissaRange::rep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, + MantissaRange::CuspRoundingFix cuspRoundingFix, bool dropped) { static constexpr auto kMinExponent = Number::kMinExponent; @@ -553,7 +638,7 @@ doNormalize( m *= 10; --exponent; } - Guard g; + Guard g(minMantissa, maxMantissa, cuspRoundingFix); if (negative) g.setNegative(); if (dropped) @@ -598,14 +683,7 @@ doNormalize( XRPL_ASSERT_PARTS(m <= kMaxRep, "xrpl::doNormalize", "intermediate mantissa fits in int64"); mantissa = m; - g.doRoundUp( - negative, - mantissa, - exponent, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - "Number::normalize 2"); + g.doRoundUp(negative, mantissa, exponent, "Number::normalize 2"); XRPL_ASSERT_PARTS( mantissa >= minMantissa && mantissa <= maxMantissa, "xrpl::doNormalize", @@ -620,13 +698,12 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) + MantissaRange::CuspRoundingFix cuspRoundingFix) { // Not used by every compiler version, and thus not necessarily // counted by coverage build // LCOV_EXCL_START - doNormalize( - negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); // LCOV_EXCL_STOP } @@ -638,13 +715,12 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) + MantissaRange::CuspRoundingFix cuspRoundingFix) { // Not used by every compiler version, and thus not necessarily // counted by coverage build // LCOV_EXCL_START - doNormalize( - negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); // LCOV_EXCL_STOP } @@ -656,16 +732,27 @@ Number::normalize( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, - MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) + MantissaRange::CuspRoundingFix cuspRoundingFix) { - doNormalize( - negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled, false); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFix, false); } void Number::normalize(MantissaRange const& range) { - normalize(negative_, mantissa_, exponent_, range.min, range.max, range.cuspRoundingFixEnabled); + normalize(negative_, mantissa_, exponent_, range.min, range.max, range.cuspRoundingFix); +} + +void +Number::normalize(Guard const& guard) +{ + normalize( + negative_, + mantissa_, + exponent_, + guard.minMantissa, + guard.maxMantissa, + guard.cuspRoundingFix); } // Copy the number, but set a new exponent. Because the mantissa doesn't change, @@ -719,46 +806,102 @@ Number::operator+=(Number const& y) bool const yn = y.negative_; uint128_t ym = y.mantissa_; auto ye = y.exponent_; - Guard g; + Guard g(kRange); + + auto const& minMantissa = g.minMantissa; + auto const& maxMantissa = g.maxMantissa; + auto const cuspRoundingFix = g.cuspRoundingFix; + + // Bring the exponents of both values into agreement, so the mantissas are on the same scale + // and can be added directly together. + + auto const upperLimit = static_cast(g.minMantissa) * 1000; + // For the "adjust" lambda + // expandM / expandE: The values for which the mantissa will be expanded, and the exponent + // decreased to match. Mantissa won't be expanded beyond upperLimit. + // (37e8 == 37000e5 == 37000000e2) + // shrinkM / shrinkE: The values for which the mantissa will be shrunk, and exponent increased + // to match, if necessary. + auto const adjust = [&g, &upperLimit]( + uint128_t& expandM, int& expandE, uint128_t& shrinkM, int& shrinkE) { + // Adjust up and down until the exponents match + if (g.cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled330) + { + // For Enabled330, there are three steps. + // 1. First, shrink the mantissa of shrinkM/shrinkE while shrinkM ends in 0. + while (shrinkE < expandE && shrinkM % 10 == 0) + { + g.doDropDigit(shrinkM, shrinkE); + } + + // 2. Then expand the mantissa of expandM/expandE, with a limit for expandM a few orders + // of magnitude above the MantissaRange. This will leave a few extra digits for rounding + // later, but nothing excessive. + while (shrinkE < expandE && expandE > kMinExponent && expandM < upperLimit) + { + expandM *= 10; + --expandE; + } + } + + // 3. Finally, shrink the mantissa of shrinkM/shrinkE until the exponents match. Any removed + // digits will be put into the Guard. This is the only step for non-Enabled330 modes. + while (shrinkE < expandE) + { + g.doDropDigit(shrinkM, shrinkE); + } + }; + + // Shrink the mantissa and raise the exponent of the value with the lower exponent. Store any + // dropped digits in the Guard. if (xe < ye) { if (xn) g.setNegative(); - do - { - g.doDropDigit(xm, xe); - } while (xe < ye); + + adjust(ym, ye, xm, xe); } else if (xe > ye) { if (yn) g.setNegative(); - do - { - g.doDropDigit(ym, ye); - } while (xe > ye); - } - auto const& range = kRange.get(); - auto const& minMantissa = range.min; - auto const& maxMantissa = range.max; - auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; + adjust(xm, xe, ym, ye); + } + else if (g.cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled330) + { + // Both values have the same exponent. + // Set the sign of the Guard based on the sign of the Number with the smallest + // unsigned _mantissa_ + if ((xm < ym && xn) || (ym < xm && yn)) + g.setNegative(); + } if (xn == yn) { xm += ym; - if (xm > maxMantissa || xm > kMaxRep) + + if (g.cuspRoundingFix >= MantissaRange::CuspRoundingFix::Enabled330) { - g.doDropDigit(xm, xe); + // Don't do any adjustments for Enabled330. Normalize will take care of it + // Because of "adjust", the only way there can be data in the Guard is if we first grew + // the mantissa past the maxMantissa. Since we added here, it can only get bigger. + // If xm > maxMantissa, then doNormalize has all the data it needs from the last 3-4 + // digits, plus the "dropped" flag that will be passed in. + // If not, then the mantissa will only need to be padded out with 0s and won't need to + // round. + XRPL_ASSERT( + xm > maxMantissa || g.empty(), + "xrpl::Number::operator+ : rounding state expected after add"); + } + else + { + if (xm > maxMantissa || xm > kMaxRep) + { + g.doDropDigit(xm, xe); + } + g.doRoundUp(xn, xm, xe, "Number::addition overflow"); } - g.doRoundUp( - xn, - xm, - xe, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - "Number::addition overflow"); } else { @@ -772,19 +915,67 @@ Number::operator+=(Number const& y) xe = ye; xn = yn; } - while (xm < minMantissa && xm * 10 <= kMaxRep) + if (cuspRoundingFix >= MantissaRange::CuspRoundingFix::Enabled330) { - xm *= 10; - xm -= g.pop(); - --xe; + // Because we subtracted, xm can have any number of digits from 1 up to + // upperLimit * 10, and g can be in any state. (Note that xm can't be zero, because that + // special case was tested earlier.) + + // Grow xm/xe and pull digits out of the Guard until xm reaches upperLimit, but stop if + // the Guard empties out, because no rounding will be necessary. This will ensure that + // normalize will have enough information to make an accurate rounding decision. + // (Normalize will pad a small mantissa back into range.) Note that if any digits were + // lost (xbit_), the Guard will never be empty, so xm will grow larger than upperLimit. + while (xm < upperLimit && !g.empty()) + { + xm *= 10; + xm -= g.pop(); + --xe; + } + XRPL_ASSERT( + xm > maxMantissa || g.empty(), + "xrpl::Number::operator+ : rounding state expected after subtract"); } - g.doRoundDown(xn, xm, xe, minMantissa); + else + { + // Grow xm/xe and pull digits out of the Guard until it's back in the + // minMantissa/maxMantissa range. + while (xm < minMantissa && xm * 10 <= kMaxRep) + { + xm *= 10; + xm -= g.pop(); + --xe; + } + } + // Rounding down can result in decrementing xm, based on whether there is any data left in + // the Guard (depending on cuspRoundingFix). Note that if that happens, then the Guard is + // not empty. For Enabled330, that will also result in the "dropped" flag being passed to + // doNormalize, which may result in the mantissa being incremented again. It doesn't matter + // what the dropped digits are, only that they exist. This is because subtracting one + // "overcorrects", so we know there are still trailing digits to be accounted for in the + // rounding. + // + // This works because + // 1. The rounding up will be done _after_ the mantissa is brought into range. It may not + // be in range right now, and + // 2. The "dropped" flag is only ever used as a tie-breaker, specifically when rounding + // away from zero, and the dropped digits are 0, or when rounding to nearest, and + // the dropped digits represent exactly 0.5. + g.doRoundDown(xn, xm, xe); } + doNormalize( + xn, + xm, + xe, + minMantissa, + maxMantissa, + cuspRoundingFix, + cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled330 && !g.empty()); negative_ = xn; mantissa_ = static_cast(xm); exponent_ = xe; - normalize(range); + XRPL_ASSERT(isnormal(), "xrpl::Number::operator+= : result is normal"); return *this; } @@ -818,14 +1009,11 @@ Number::operator*=(Number const& y) auto ze = xe + ye; auto zs = xs * ys; bool zn = (zs == -1); - Guard g; + Guard g(kRange); if (zn) g.setNegative(); - auto const& range = kRange.get(); - auto const& minMantissa = range.min; - auto const& maxMantissa = range.max; - auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; + auto const& maxMantissa = g.maxMantissa; while (zm > maxMantissa || zm > kMaxRep) { @@ -834,19 +1022,12 @@ Number::operator*=(Number const& y) xm = static_cast(zm); xe = ze; - g.doRoundUp( - zn, - xm, - xe, - minMantissa, - maxMantissa, - cuspRoundingFixEnabled, - "Number::multiplication overflow : exponent is " + std::to_string(xe)); + g.doRoundUp(zn, xm, xe, "Number::multiplication overflow : exponent is " + std::to_string(xe)); negative_ = zn; mantissa_ = xm; exponent_ = xe; - normalize(range); + normalize(g); return *this; } @@ -882,7 +1063,7 @@ Number::operator/=(Number const& y) auto const& range = kRange.get(); auto const& minMantissa = range.min; auto const& maxMantissa = range.max; - auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; + auto const cuspRoundingFix = range.cuspRoundingFix; // Division operates on two large integers (16-digit for small // mantissas, 19-digit for large) using integer math. If the values @@ -930,7 +1111,7 @@ Number::operator/=(Number const& y) // This is equivalent to if we had used an initial factor of 10^22, // a couple digits more than we actually need. // - // Stage 3: If there is still a remainder, and the CuspRoundingFix + // Stage 3: If there is still a remainder, and the cuspRoundingFix // is enabled, pass a flag indicating such to doNormalize. The Guard // in doNormalize will treat that flag as if non-zero digits had // been dropped from the mantissa when shrinking it into range. @@ -1014,14 +1195,14 @@ Number::operator/=(Number const& y) // rounding fix is enabled, flag if there is still // a remainder from stage 2. bool const useTrailingRemainder = - cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled; + cuspRoundingFix != MantissaRange::CuspRoundingFix::Disabled; if (useTrailingRemainder) { dropped = partialNumerator % dm != 0; } } } - doNormalize(zp, zm, ze, minMantissa, maxMantissa, cuspRoundingFixEnabled, dropped); + doNormalize(zp, zm, ze, minMantissa, maxMantissa, cuspRoundingFix, dropped); negative_ = zp; mantissa_ = static_cast(zm); exponent_ = ze; @@ -1035,7 +1216,7 @@ operator rep() const { rep drops = mantissa(); int offset = exponent(); - Guard g; + Guard g(kRange); if (drops != 0) { if (negative_) diff --git a/src/libxrpl/protocol/Rules.cpp b/src/libxrpl/protocol/Rules.cpp index e0968ea868..d71bb77f66 100644 --- a/src/libxrpl/protocol/Rules.cpp +++ b/src/libxrpl/protocol/Rules.cpp @@ -39,22 +39,30 @@ setCurrentTransactionRules(std::optional r) // Push the appropriate setting, instead of having the class pull every time // the value is needed. That could get expensive fast. - // If any new conditions with new amendments are added, those amendments must also be added to - // useRulesGuards. - bool const enableVaultNumbers = - !r || (r->enabled(featureSingleAssetVault) || r->enabled(featureLendingProtocol)); - bool const enableCuspRoundingFix = !r || r->enabled(fixCleanup3_2_0); - XRPL_ASSERT( - !r || useRulesGuards(*r) == (enableCuspRoundingFix || enableVaultNumbers), - "setCurrentTransactionRules : rule decisions match"); - // Declare the range this way to keep clang-tidy from complaining - auto const range = [enableCuspRoundingFix, enableVaultNumbers]() { - if (enableVaultNumbers) + auto const range = [&r]() { + // If any new conditions with new amendments are added to "enableLargeNumbers", those + // amendments must also be added to useRulesGuards. + bool const enableLargeNumbers = + !r || (r->enabled(featureSingleAssetVault) || r->enabled(featureLendingProtocol)); + // If enableLargeNumbers is true, then useRulesGuard must also return true. + // However, the reverse is not true. Other amendments can cause the rules guard to be used, + // even though large numbers are _not_ used. + XRPL_ASSERT( + !r || !enableLargeNumbers || useRulesGuards(*r), + "setCurrentTransactionRules : rule decisions match"); + + if (enableLargeNumbers) { - if (enableCuspRoundingFix) + static_assert( + MantissaRange::MantissaScale::Large == MantissaRange::MantissaScale::Large330); + if (!r || r->enabled(fixCleanup3_3_0)) { - return MantissaRange::MantissaScale::Large; + return MantissaRange::MantissaScale::Large330; + } + if (r->enabled(fixCleanup3_2_0)) + { + return MantissaRange::MantissaScale::Large320; } return MantissaRange::MantissaScale::LargeLegacy; } @@ -69,14 +77,14 @@ bool useRulesGuards(Rules const& rules) { // The list of amendments used here - to decide whether to create a RulesGuard - must be a - // superset of the list used to figure out which mantissa scale to use in - // setCurrentTransactionRules. Additional amendments can be added if desired. + // superset of the list used to determine "enableLargeNumbers" in setCurrentTransactionRules. + // Additional amendments can be added if desired. // // As soon as any one of these amendments is retired, this whole function can be removed, along // with createGuards, and any other callers, and the first set of guards can be created directly // at the call site, without using optional. - return rules.enabled(fixCleanup3_2_0) || rules.enabled(featureSingleAssetVault) || - rules.enabled(featureLendingProtocol); + return rules.enabled(featureSingleAssetVault) || rules.enabled(featureLendingProtocol) || + rules.enabled(fixCleanup3_2_0) || rules.enabled(fixCleanup3_3_0); } void @@ -87,7 +95,8 @@ createGuards( { if (useRulesGuards(rules)) { - // raii classes for the current ledger rules. + // raii classes for the current ledger rules. If the rules are set, the MantissaRange will + // be updated, too. rulesGuard.emplace(rules); } else diff --git a/src/libxrpl/protocol/STNumber.cpp b/src/libxrpl/protocol/STNumber.cpp index 79f7655869..bd7f67649b 100644 --- a/src/libxrpl/protocol/STNumber.cpp +++ b/src/libxrpl/protocol/STNumber.cpp @@ -88,7 +88,6 @@ STNumber::add(Serializer& s) const } else { -#if !NDEBUG // There are circumstances where an already-rounded Number is // serialized without being touched by a transactor, and thus // without an asset. We can't know if it's rounded, because it could @@ -96,11 +95,9 @@ STNumber::add(Serializer& s) const // Json. Regardless, the only time we should be serializing an // STNumber is when the scale is large. XRPL_ASSERT_PARTS( - Number::getMantissaScale() == MantissaRange::MantissaScale::LargeLegacy || - Number::getMantissaScale() == MantissaRange::MantissaScale::Large, + Number::getMantissaScale() != MantissaRange::MantissaScale::Small, "xrpl::STNumber::add", "STNumber only used with large mantissa scale"); -#endif } } diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index b70d099ba4..8a1f24e30b 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -5030,11 +5030,10 @@ class Invariants_test : public beast::unit_test::Suite std::vector values; }; - for (auto const mantissaScale : { - MantissaRange::MantissaScale::LargeLegacy, - MantissaRange::MantissaScale::Large, - }) + for (auto const mantissaScale : MantissaRange::getAllScales()) { + if (mantissaScale == MantissaRange::MantissaScale::Small) + continue; NumberMantissaScaleGuard const g{mantissaScale}; auto makeDelta = [&vaultAsset](Number const& n) -> ValidVault::DeltaInfo { diff --git a/src/tests/libxrpl/basics/Number.cpp b/src/tests/libxrpl/basics/Number.cpp index e8c4e4209f..70d6be2da5 100644 --- a/src/tests/libxrpl/basics/Number.cpp +++ b/src/tests/libxrpl/basics/Number.cpp @@ -48,6 +48,22 @@ fmt(BigInt const& value) return out; } +BigInt +toBigInt(Number const& n) +{ + BigInt v = n.mantissa(); + auto e = n.exponent(); + + for (; e > 0; --e) + v *= 10; + for (; e < 0; ++e) + { + EXPECT_EQ(v % 10, 0); + v /= 10; + } + return v; +} + template static T pow10(int n) @@ -177,28 +193,35 @@ TEST(NumberTest, add) auto const scale = Number::getMantissaScale(); - using Case = std::tuple; - auto const cSmall = std::to_array( - {{Number{1'000'000'000'000'000, -15}, - Number{6'555'555'555'555'555, -29}, - Number{1'000'000'000'000'066, -15}}, - {Number{-1'000'000'000'000'000, -15}, - Number{-6'555'555'555'555'555, -29}, - Number{-1'000'000'000'000'066, -15}}, - {Number{-1'000'000'000'000'000, -15}, - Number{6'555'555'555'555'555, -29}, - Number{-9'999'999'999'999'344, -16}}, - {Number{-6'555'555'555'555'555, -29}, - Number{1'000'000'000'000'000, -15}, - Number{9'999'999'999'999'344, -16}}, - {Number{}, Number{5}, Number{5}}, - {Number{5}, Number{}, Number{5}}, - {Number{5'555'555'555'555'555, -32768}, - Number{-5'555'555'555'555'554, -32768}, - Number{0}}, - {Number{-9'999'999'999'999'999, -31}, - Number{1'000'000'000'000'000, -15}, - Number{9'999'999'999'999'990, -16}}}); + using Case = std::tuple; + auto const cSmall = std::to_array({ + {Number{1'000'000'000'000'000, -15}, + Number{6'555'555'555'555'555, -29}, + Number{1'000'000'000'000'066, -15}, + __LINE__}, + {Number{-1'000'000'000'000'000, -15}, + Number{-6'555'555'555'555'555, -29}, + Number{-1'000'000'000'000'066, -15}, + __LINE__}, + {Number{-1'000'000'000'000'000, -15}, + Number{6'555'555'555'555'555, -29}, + Number{-9'999'999'999'999'344, -16}, + __LINE__}, + {Number{-6'555'555'555'555'555, -29}, + Number{1'000'000'000'000'000, -15}, + Number{9'999'999'999'999'344, -16}, + __LINE__}, + {Number{}, Number{5}, Number{5}, __LINE__}, + {Number{5}, Number{}, Number{5}, __LINE__}, + {Number{5'555'555'555'555'555, -32768}, + Number{-5'555'555'555'555'554, -32768}, + Number{0}, + __LINE__}, + {Number{-9'999'999'999'999'999, -31}, + Number{1'000'000'000'000'000, -15}, + Number{9'999'999'999'999'990, -16}, + __LINE__}, + }); auto const cLarge = std::to_array( // Note that items with extremely large mantissas need to be // calculated, because otherwise they overflow uint64. Items from C @@ -206,45 +229,57 @@ TEST(NumberTest, add) { {Number{1'000'000'000'000'000, -15}, Number{6'555'555'555'555'555, -29}, - Number{1'000'000'000'000'065'556, -18}}, + Number{1'000'000'000'000'065'556, -18}, + __LINE__}, {Number{-1'000'000'000'000'000, -15}, Number{-6'555'555'555'555'555, -29}, - Number{-1'000'000'000'000'065'556, -18}}, + Number{-1'000'000'000'000'065'556, -18}, + __LINE__}, {Number{-1'000'000'000'000'000, -15}, Number{6'555'555'555'555'555, -29}, - Number{true, 9'999'999'999'999'344'444ULL, -19, Number::Normalized{}}}, + Number{true, 9'999'999'999'999'344'444ULL, -19, Number::Normalized{}}, + __LINE__}, {Number{-6'555'555'555'555'555, -29}, Number{1'000'000'000'000'000, -15}, - Number{false, 9'999'999'999'999'344'444ULL, -19, Number::Normalized{}}}, - {Number{}, Number{5}, Number{5}}, - {Number{5}, Number{}, Number{5}}, + Number{false, 9'999'999'999'999'344'444ULL, -19, Number::Normalized{}}, + __LINE__}, + {Number{}, Number{5}, Number{5}, __LINE__}, + {Number{5}, Number{}, Number{5}, __LINE__}, {Number{5'555'555'555'555'555'000, -32768}, Number{-5'555'555'555'555'554'000, -32768}, - Number{0}}, + Number{0}, + __LINE__}, {Number{-9'999'999'999'999'999, -31}, Number{1'000'000'000'000'000, -15}, - Number{9'999'999'999'999'990, -16}}, + Number{9'999'999'999'999'990, -16}, + __LINE__}, // Items from cSmall expanded for the larger mantissa {Number{1'000'000'000'000'000'000, -18}, Number{6'555'555'555'555'555'555, -35}, - Number{1'000'000'000'000'000'066, -18}}, + Number{1'000'000'000'000'000'066, -18}, + __LINE__}, {Number{-1'000'000'000'000'000'000, -18}, Number{-6'555'555'555'555'555'555, -35}, - Number{-1'000'000'000'000'000'066, -18}}, + Number{-1'000'000'000'000'000'066, -18}, + __LINE__}, {Number{-1'000'000'000'000'000'000, -18}, Number{6'555'555'555'555'555'555, -35}, - Number{true, 9'999'999'999'999'999'344ULL, -19, Number::Normalized{}}}, + Number{true, 9'999'999'999'999'999'344ULL, -19, Number::Normalized{}}, + __LINE__}, {Number{-6'555'555'555'555'555'555, -35}, Number{1'000'000'000'000'000'000, -18}, - Number{false, 9'999'999'999'999'999'344ULL, -19, Number::Normalized{}}}, - {Number{}, Number{5}, Number{5}}, + Number{false, 9'999'999'999'999'999'344ULL, -19, Number::Normalized{}}, + __LINE__}, + {Number{}, Number{5}, Number{5}, __LINE__}, {Number{5'555'555'555'555'555'555, -32768}, Number{-5'555'555'555'555'555'554, -32768}, - Number{0}}, + Number{0}, + __LINE__}, {Number{true, 9'999'999'999'999'999'999ULL, -37, Number::Normalized{}}, Number{1'000'000'000'000'000'000, -18}, - Number{false, 9'999'999'999'999'999'990ULL, -19, Number::Normalized{}}}, - {Number{Number::kMaxRep - 1}, Number{1, 0}, Number{Number::kMaxRep}}, + Number{false, 9'999'999'999'999'999'990ULL, -19, Number::Normalized{}}, + __LINE__}, + {Number{Number::kMaxRep - 1}, Number{1, 0}, Number{Number::kMaxRep}, __LINE__}, // Test extremes { // Each Number operand rounds up, so the actual mantissa is @@ -252,6 +287,7 @@ TEST(NumberTest, add) Number{false, 9'999'999'999'999'999'999ULL, 0, Number::Normalized{}}, Number{false, 9'999'999'999'999'999'999ULL, 0, Number::Normalized{}}, Number{2, 19}, + __LINE__, }, { // Does not round. Mantissas are going to be > maxRep, so if @@ -262,21 +298,25 @@ TEST(NumberTest, add) Number{false, 9'999'999'999'999'999'990ULL, 0, Number::Normalized{}}, Number{false, 9'999'999'999'999'999'990ULL, 0, Number::Normalized{}}, Number{false, 1'999'999'999'999'999'998ULL, 1, Number::Normalized{}}, + __LINE__, }, }); auto const cLargeLegacy = std::to_array({ - {Number{Number::kMaxRep}, Number{6, -1}, Number{Number::kMaxRep / 10, 1}}, + {Number{Number::kMaxRep}, Number{6, -1}, Number{Number::kMaxRep / 10, 1}, __LINE__}, }); auto const cLargeCorrected = std::to_array({ - {Number{Number::kMaxRep}, Number{6, -1}, Number{(Number::kMaxRep / 10) + 1, 1}}, + {Number{Number::kMaxRep}, + Number{6, -1}, + Number{(Number::kMaxRep / 10) + 1, 1}, + __LINE__}, }); auto test = [](auto const& c) { - for (auto const& [x, y, z] : c) + for (auto const& [x, y, z, line] : c) { auto const result = x + y; std::stringstream ss; ss << x << " + " << y << " = " << result << ". Expected: " << z; - EXPECT_EQ(result, z) << ss.str(); + EXPECT_EQ(result, z) << ss.str() << " Line: " << line; } }; if (scale == MantissaRange::MantissaScale::Small) @@ -319,21 +359,28 @@ TEST(NumberTest, sub) auto const scale = Number::getMantissaScale(); - using Case = std::tuple; + using Case = std::tuple; auto const cSmall = std::to_array( {{Number{1'000'000'000'000'000, -15}, Number{6'555'555'555'555'555, -29}, - Number{9'999'999'999'999'344, -16}}, + Number{9'999'999'999'999'344, -16}, + __LINE__}, {Number{6'555'555'555'555'555, -29}, Number{1'000'000'000'000'000, -15}, - Number{-9'999'999'999'999'344, -16}}, - {Number{1'000'000'000'000'000, -15}, Number{1'000'000'000'000'000, -15}, Number{0}}, + Number{-9'999'999'999'999'344, -16}, + __LINE__}, + {Number{1'000'000'000'000'000, -15}, + Number{1'000'000'000'000'000, -15}, + Number{0}, + __LINE__}, {Number{1'000'000'000'000'000, -15}, Number{1'000'000'000'000'001, -15}, - Number{-1'000'000'000'000'000, -30}}, + Number{-1'000'000'000'000'000, -30}, + __LINE__}, {Number{1'000'000'000'000'001, -15}, Number{1'000'000'000'000'000, -15}, - Number{1'000'000'000'000'000, -30}}}); + Number{1'000'000'000'000'000, -30}, + __LINE__}}); auto const cLarge = std::to_array( // Note that items with extremely large mantissas need to be // calculated, because otherwise they overflow uint64. Items from C @@ -341,49 +388,63 @@ TEST(NumberTest, sub) { {Number{1'000'000'000'000'000, -15}, Number{6'555'555'555'555'555, -29}, - Number{false, 9'999'999'999'999'344'444ULL, -19, Number::Normalized{}}}, + Number{false, 9'999'999'999'999'344'444ULL, -19, Number::Normalized{}}, + __LINE__}, {Number{6'555'555'555'555'555, -29}, Number{1'000'000'000'000'000, -15}, - Number{true, 9'999'999'999'999'344'444ULL, -19, Number::Normalized{}}}, - {Number{1'000'000'000'000'000, -15}, Number{1'000'000'000'000'000, -15}, Number{0}}, + Number{true, 9'999'999'999'999'344'444ULL, -19, Number::Normalized{}}, + __LINE__}, + {Number{1'000'000'000'000'000, -15}, + Number{1'000'000'000'000'000, -15}, + Number{0}, + __LINE__}, {Number{1'000'000'000'000'000, -15}, Number{1'000'000'000'000'001, -15}, - Number{-1'000'000'000'000'000, -30}}, + Number{-1'000'000'000'000'000, -30}, + __LINE__}, {Number{1'000'000'000'000'001, -15}, Number{1'000'000'000'000'000, -15}, - Number{1'000'000'000'000'000, -30}}, + Number{1'000'000'000'000'000, -30}, + __LINE__}, // Items from cSmall expanded for the larger mantissa {Number{1'000'000'000'000'000'000, -18}, Number{6'555'555'555'555'555'555, -32}, - Number{false, 9'999'999'999'999'344'444ULL, -19, Number::Normalized{}}}, + Number{false, 9'999'999'999'999'344'444ULL, -19, Number::Normalized{}}, + __LINE__}, {Number{6'555'555'555'555'555'555, -32}, Number{1'000'000'000'000'000'000, -18}, - Number{true, 9'999'999'999'999'344'444ULL, -19, Number::Normalized{}}}, + Number{true, 9'999'999'999'999'344'444ULL, -19, Number::Normalized{}}, + __LINE__}, {Number{1'000'000'000'000'000'000, -18}, Number{1'000'000'000'000'000'000, -18}, - Number{0}}, + Number{0}, + __LINE__}, {Number{1'000'000'000'000'000'000, -18}, Number{1'000'000'000'000'000'001, -18}, - Number{-1'000'000'000'000'000'000, -36}}, + Number{-1'000'000'000'000'000'000, -36}, + __LINE__}, {Number{1'000'000'000'000'000'001, -18}, Number{1'000'000'000'000'000'000, -18}, - Number{1'000'000'000'000'000'000, -36}}, - {Number{Number::kMaxRep}, Number{6, -1}, Number{Number::kMaxRep - 1}}, + Number{1'000'000'000'000'000'000, -36}, + __LINE__}, + {Number{Number::kMaxRep}, Number{6, -1}, Number{Number::kMaxRep - 1}, __LINE__}, {Number{false, Number::kMaxRep + 1, 0, Number::Normalized{}}, Number{1, 0}, - Number{(Number::kMaxRep / 10) + 1, 1}}, + Number{(Number::kMaxRep / 10) + 1, 1}, + __LINE__}, {Number{false, Number::kMaxRep + 1, 0, Number::Normalized{}}, Number{3, 0}, - Number{Number::kMaxRep}}, - {power(2, 63), Number{3, 0}, Number{Number::kMaxRep}}, + Number{Number::kMaxRep}, + __LINE__}, + {power(2, 63), Number{3, 0}, Number{Number::kMaxRep}, __LINE__}, }); auto test = [](auto const& c) { - for (auto const& [x, y, z] : c) + for (auto const& [x, y, z, line] : c) { auto const result = x - y; std::stringstream ss; ss << x << " - " << y << " = " << result << ". Expected: " << z; - EXPECT_EQ(result, z) << ss.str(); + EXPECT_EQ(result, z) << ss.str() << " Line: " << line; } }; if (scale == MantissaRange::MantissaScale::Small) @@ -1365,8 +1426,7 @@ TEST(NumberTest, to_string) "9223372036854775e3"); } break; - case MantissaRange::MantissaScale::LargeLegacy: - case MantissaRange::MantissaScale::Large: + default: // Test the edges // ((exponent < -(28)) || (exponent > -(8))))) test(Number::min(), "1e-32750"); @@ -1405,8 +1465,6 @@ TEST(NumberTest, to_string) -(Number{std::numeric_limits::max(), 0} + 1), "-9223372036854775810"); break; - default: - EXPECT_TRUE(false); } } } @@ -1787,9 +1845,7 @@ TEST(NumberTest, upward_rounding_produces_value_not_below_exact_at_k_max_rep_cus BigInt const exactProduct = BigInt(kAValue) * BigInt(kBValue); // What Number actually stored. - BigInt storedValue = BigInt(product.mantissa()); - for (int i = 0; i < product.exponent(); ++i) - storedValue *= 10; + BigInt const storedValue = toBigInt(product); BigInt const signedDifference = storedValue - exactProduct; @@ -1809,7 +1865,8 @@ TEST(NumberTest, upward_rounding_produces_value_not_below_exact_at_k_max_rep_cus switch (scale) { - case MantissaRange::MantissaScale::Large: + case MantissaRange::MantissaScale::Large320: + case MantissaRange::MantissaScale::Large330: EXPECT_TRUE(signedDifference >= 0) << message(); EXPECT_TRUE(signedDifference < pow10(product.exponent())) << message(); EXPECT_EQ(product.mantissa(), (std::numeric_limits::max() / 10) + 1); @@ -1897,7 +1954,8 @@ TEST(NumberTest, upward_division_returns_value_not_below_exact_on_large_scale) // Upward invariant: stored >= exact. Bug: stored < exact. switch (scale) { - case MantissaRange::MantissaScale::Large: + case MantissaRange::MantissaScale::Large320: + case MantissaRange::MantissaScale::Large330: EXPECT_TRUE(stored >= exact) << message(); EXPECT_TRUE(diff < pow10(quotient.exponent())) << message(); break; @@ -1951,10 +2009,11 @@ TEST(NumberTest, downward_division_returns_value_not_above_exact_on_large_scale) return os.str(); }; - // Downward invariant: stored <= exact. Bug: stored > exact. + // invariant: stored <= exact. Bug: stored > exact. switch (scale) { - case MantissaRange::MantissaScale::Large: + case MantissaRange::MantissaScale::Large320: + case MantissaRange::MantissaScale::Large330: EXPECT_TRUE(stored <= exact) << message(); EXPECT_TRUE(diff > -pow10(quotient.exponent())) << message(); break; @@ -2018,10 +2077,11 @@ TEST(NumberTest, to_nearest_division_uses_dropped_digits_on_large_scale) return os.str(); }; - // ToNearest should account for dropped digits beyond the visible "5". + // invariant: stored >= exact. Bug: stored < exact. switch (scale) { - case MantissaRange::MantissaScale::Large: + case MantissaRange::MantissaScale::Large320: + case MantissaRange::MantissaScale::Large330: EXPECT_TRUE(stored >= exact) << message(); EXPECT_TRUE(diff < pow10(quotient.exponent())) << message(); break; @@ -2039,4 +2099,365 @@ TEST(NumberTest, to_nearest_division_uses_dropped_digits_on_large_scale) } } +TEST(NumberTest, subtraction_rounding) +{ + for (auto const mantissaScale : MantissaRange::getAllScales()) + { + NumberMantissaScaleGuard const mg{mantissaScale}; + NumberRoundModeGuard const rg{Number::RoundingMode::ToNearest}; + + auto const scale = Number::getMantissaScale(); + + auto const exp = Number::mantissaLog(); + // SubCase is + // * offset: offset from exp + // * extraB: whether to include 1e"exp" in "b" + // * aString: expected string value for "a" + // * bString: expected string value for "b" + // There aren't too many valid combinations for test cases here. If extraB is true, + // offset can really only be 2, because any larger and the mantissa can't be represented + // without loss. Offset can't be less than 2, or there's no error. + using SubCase = std::tuple; + auto const c = std::to_array({ + {2, + true, + scale == MantissaRange::MantissaScale::Small ? "100000000000000000" + : "100000000000000000000", + scale == MantissaRange::MantissaScale::Small ? "-1000000000000001" + : "-1000000000000000001"}, + {2, + false, + scale == MantissaRange::MantissaScale::Small ? "100000000000000000" + : "100000000000000000000", + "-1"}, + {30, + false, + scale == MantissaRange::MantissaScale::Small + ? "1000000000000000000000000000000000000000000000" + : "1000000000000000000000000000000000000000000000000", + "-1"}, + }); + + for (auto const& [offset, extraB, aString, bString] : c) + { + Number const a{1LL, exp + offset}; + Number const b{-((extraB ? Number{1, exp} : kNumZero) + 1)}; + + auto const bigA = toBigInt(a); + auto const bigB = toBigInt(b); + + EXPECT_EQ(bigA, BigInt{aString}); + EXPECT_EQ(bigB, BigInt{bString}); + + auto construct = [&a, &b](Number::RoundingMode r) { + NumberRoundModeGuard const roundGuard{r}; + auto const sum = a + b; + BigInt const stored = toBigInt(sum); + return std::make_pair(r, std::make_pair(stored, sum)); + }; + + BigInt const exact = bigA + bigB; + + auto const sums = [&]() { + std::map> r; + r.emplace(construct(Number::RoundingMode::TowardsZero)); + r.emplace(construct(Number::RoundingMode::Upward)); + r.emplace(construct(Number::RoundingMode::Downward)); + r.emplace(construct(Number::RoundingMode::ToNearest)); + return r; + }(); + + auto const message = [&](auto const& r, auto const& sum) { + std::ostringstream os; + os << "\n a = " << a << " (" << fmt(bigA) + << ")\n b = " << b << " (" << fmt(bigB) + << ")\n exact a + b = " << fmt(exact) << "\n"; + + auto const diff = sum.first - exact; + auto const rLabel = to_string(r); + os << std::string(15 - rLabel.length(), ' ') << rLabel << " = " << fmt(sum.first) + << "\n difference = " << fmt(diff) << "\n"; + + return os.str(); + }; + + auto const expectedExponent = + offset - (scale == MantissaRange::MantissaScale::Small && extraB ? 1 : 0); + auto const epsilon = pow10(expectedExponent); + for (auto const& [r, sum] : sums) + { + auto diff = sum.first - exact; + switch (scale) + { + case MantissaRange::MantissaScale::Small: + case MantissaRange::MantissaScale::LargeLegacy: + case MantissaRange::MantissaScale::Large320: { + // Without the fix, all the results but one round up + if (r == Number::RoundingMode::Downward) + { + // Downward works because the Guard sign is negative, and Downward + // returns Up instead of Down if negative and there's a remainder, + // whereas TowardsZero always returns Down. + EXPECT_LT(sum.first, exact) << message(r, sum); + EXPECT_EQ(diff, -(epsilon - 1)) << message(r, sum); + } + else + { + EXPECT_GT(sum.first, exact) << message(r, sum); + EXPECT_EQ(diff, 1) << message(r, sum); + } + break; + } + default: { + EXPECT_LE(sum.second.exponent(), expectedExponent) << message(r, sum); + switch (r) + { + case Number::RoundingMode::Upward: + case Number::RoundingMode::ToNearest: + EXPECT_GT(sum.first, exact) << message(r, sum); + EXPECT_EQ(diff, 1) << message(r, sum); + break; + default: + EXPECT_LT(sum.first, exact) << message(r, sum); + EXPECT_EQ(diff, -(epsilon - 1)) << message(r, sum); + } + } + } + } + } + } +} + +TEST(NumberTest, number_add_directed_sign_wrong) +{ + for (auto const mantissaScale : MantissaRange::getAllScales()) + { + NumberMantissaScaleGuard const mg{mantissaScale}; + NumberRoundModeGuard const rg{Number::RoundingMode::ToNearest}; + + auto const scale = Number::getMantissaScale(); + { + // Two negative numbers with the same exponent + Number const a{-6, Number::mantissaLog()}; + Number const b{a - 3}; + EXPECT_TRUE(a.exponent() == b.exponent() && abs(b) > abs(a)); + + BigInt const exact = toBigInt(a) + toBigInt(b); + if (scale == MantissaRange::MantissaScale::Small) + { + EXPECT_EQ(exact, BigInt{"-12000000000000003"}); + } + else + { + EXPECT_EQ(exact, BigInt{"-12000000000000000003"}); + } + + Number down, up; + { + NumberRoundModeGuard const g{Number::RoundingMode::Downward}; + down = a + b; + } + { + NumberRoundModeGuard const g{Number::RoundingMode::Upward}; + up = a + b; + } + + auto const valueDown = toBigInt(down); + auto const valueUp = toBigInt(up); + auto message = [&] { + std::ostringstream log; + log << " exact = " << fmt(exact) << "\n downward = " << fmt(valueDown) + << " (correct rounding: <= exact)" + << "\n upward = " << fmt(valueUp) << " (correct rounding: >= exact)\n\n"; + return log.str(); + }; + + if (scale == MantissaRange::MantissaScale::Large330) + { + EXPECT_LE(valueDown, exact) << message(); // Downward should round away from zero + EXPECT_GE(valueUp, exact) << message(); // Upward should round toward 0 + } + else + { + EXPECT_GT(valueDown, exact) + << message(); // Downward rounded toward zero (too high) + EXPECT_LT(valueUp, exact) << message(); // Upward rounded toward -inf (too low) + } + } + + { + // Positive control: the same magnitudes with a positive result round + Number const pa{6, Number::mantissaLog()}; + Number const pb{pa + 3}; + EXPECT_TRUE(pa.exponent() == pb.exponent() && abs(pb) > abs(pa)); + BigInt const pexact = toBigInt(pa) + toBigInt(pb); // 12'000'000'000'000'000'003 + + Number pdown, pup; + { + NumberRoundModeGuard const g{Number::RoundingMode::Downward}; + pdown = pa + pb; + } + { + NumberRoundModeGuard const g{Number::RoundingMode::Upward}; + pup = pa + pb; + } + auto const valuePDown = toBigInt(pdown); + auto const valuePUp = toBigInt(pup); + auto message = [&] { + std::ostringstream log; + log << " exact = " << fmt(pexact) << "\n downward = " << fmt(valuePDown) + << " (correct rounding: <= exact)" + << "\n upward = " << fmt(valuePUp) + << " (correct rounding: >= exact)\n\n"; + return log.str(); + }; + + EXPECT_LE(valuePDown, pexact) << message(); // correct for positive results + EXPECT_GE(valuePUp, pexact) << message(); + } + + { + // Mixed sign numbers with the same exponent: negative second value + Number const a{1, Number::mantissaLog()}; + Number const b{Number{-9, Number::mantissaLog()} - 3}; + EXPECT_TRUE(a.exponent() == b.exponent() && abs(b) > abs(a)); + + BigInt const exact = toBigInt(a) + toBigInt(b); + if (scale == MantissaRange::MantissaScale::Small) + { + EXPECT_EQ(exact, BigInt{"-8000000000000003"}); + } + else + { + EXPECT_EQ(exact, BigInt{"-8000000000000000003"}); + } + + Number down, up; + { + NumberRoundModeGuard const g{Number::RoundingMode::Downward}; + down = a + b; + } + { + NumberRoundModeGuard const g{Number::RoundingMode::Upward}; + up = a + b; + } + + auto const valueDown = toBigInt(down); + auto const valueUp = toBigInt(up); + auto message = [&] { + std::ostringstream log; + log << " exact = " << fmt(exact) << "\n downward = " << fmt(valueDown) + << " (correct rounding: <= exact)" + << "\n upward = " << fmt(valueUp) << " (correct rounding: >= exact)\n\n"; + return log.str(); + }; + + EXPECT_LE(valueDown, exact) << message(); // Downward should round away from zero + EXPECT_GE(valueUp, exact) << message(); // Upward should round toward 0 + } + + { + // Mixed sign numbers with the same exponent: negative first value + Number const a{-1, Number::mantissaLog()}; + Number const b{Number{9, Number::mantissaLog()} + 3}; + EXPECT_TRUE(a.exponent() == b.exponent() && abs(b) > abs(a)); + + BigInt const exact = toBigInt(a) + toBigInt(b); + if (scale == MantissaRange::MantissaScale::Small) + { + EXPECT_EQ(exact, BigInt{"8000000000000003"}); + } + else + { + EXPECT_EQ(exact, BigInt{"8000000000000000003"}); + } + + Number down, up; + { + NumberRoundModeGuard const g{Number::RoundingMode::Downward}; + down = a + b; + } + { + NumberRoundModeGuard const g{Number::RoundingMode::Upward}; + up = a + b; + } + + auto const valueDown = toBigInt(down); + auto const valueUp = toBigInt(up); + auto message = [&] { + std::ostringstream log; + log << " exact = " << fmt(exact) << "\n downward = " << fmt(valueDown) + << " (correct rounding: <= exact)" + << "\n upward = " << fmt(valueUp) << " (correct rounding: >= exact)\n\n"; + return log.str(); + }; + + EXPECT_LE(valueDown, exact) << message(); // Downward should round away from zero + EXPECT_GE(valueUp, exact) << message(); // Upward should round toward 0 + } + } +} + +TEST(NumberTest, number_add_to_nearest_picks_farther) +{ + for (auto const mantissaScale : MantissaRange::getAllScales()) + { + NumberMantissaScaleGuard const mg{mantissaScale}; + NumberRoundModeGuard const rg{Number::RoundingMode::ToNearest}; + + auto const scale = Number::getMantissaScale(); + + // Case is + using Case = std::pair; + + auto const c = std::to_array({ + {Number{5'175'909'259'972'499'745LL, 22}, -1'074'951'375'311'646'003}, + {Number{1}, -1'074'956'551'220'905'975}, + {Number{1, 10}, -1'074'956'551'220'905'975}, + {Number{1, 20}, -1'074'956'551'220'905'975}, + {Number{1, 27}, -1'074'956'551'220'905'975}, + {Number{1, 28}, -1'074'956'551'220'905'974}, + {Number{1, 31}, -1'074'956'551'220'904'975}, + }); + + for (auto const& [y, expectedQ] : c) + { + Number const x{-1'074'956'551'220'905'975LL, 28}; + Number const res = x + y; + + BigInt const exact = toBigInt(x) + toBigInt(y); + BigInt const vres = toBigInt(res); + + BigInt ulp = 1; + for (int i = 0; i < res.exponent(); ++i) + ulp *= 10; + + BigInt const q = (exact - ulp / 2) / ulp; + Number const normalizedExact{static_cast(q), res.exponent()}; + BigInt const norm = toBigInt(normalizedExact); + + auto message = [&](auto const& comp) { + std::ostringstream log; + log << fmt(q) + " != " + fmt(comp) << "\n" + << " x = " << x << "\n y = " << y + << "\n exact = " << fmt(exact) + << "\n result (x + y) = " << fmt(vres) + << "\n normalize(exact) = " << fmt(norm) << "\n\n"; + return log.str(); + }; + + if (scale == MantissaRange::MantissaScale::Small) + { + auto const comp = toBigInt(Number{expectedQ, -3}); + EXPECT_EQ(q, comp) << message(comp); + } + else + { + EXPECT_EQ(q, expectedQ) << message(BigInt(expectedQ)); + } + EXPECT_EQ(normalizedExact, res); + } + } +} + } // namespace xrpl