diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index cdb9014a87..fb32b42060 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -7,6 +7,7 @@ #include #include #include +#include #include namespace xrpl { @@ -44,9 +45,9 @@ isPowerOfTen(T value) * * min is a power of 10, and * * max = min * 10 - 1. * - * The mantissa_scale enum indicates whether the range is "small" or "large". - * This intentionally restricts the number of MantissaRanges that can be - * instantiated to two: one for each scale. + * The MantissaScale enum indicates properties of the range: size, and some behavioral + * options. This intentionally restricts the number of unique MantissaRanges that can + * be instantiated: one for each scale. * * The "small" scale is based on the behavior of STAmount for IOUs. It has a min * value of 10^15, and a max value of 10^16-1. This was sufficient for @@ -70,18 +71,39 @@ isPowerOfTen(T value) struct MantissaRange { using rep = std::uint64_t; - enum class MantissaScale { Small, Large }; + enum class MantissaScale { + Small, + // LargeLegacy can be removed when fixCleanup3_2_0 is retired + LargeLegacy, + Large, + }; + + // This entire enum can be removed when fixCleanup3_2_0 is retired + enum class CuspRoundingFix : bool { + Disabled = false, + Enabled = true, + }; explicit constexpr MantissaRange(MantissaScale scale) - : min(getMin(scale)), log(logTen(min).value_or(-1)), scale(scale) + : min(getMin(scale)) + , cuspRoundingFixEnabled(isCuspFixEnabled(scale)) + , log(logTen(min).value_or(-1)) + , scale(scale) { } rep min; rep max{(min * 10) - 1}; + CuspRoundingFix cuspRoundingFixEnabled; int log; MantissaScale scale; + static MantissaRange const& + getMantissaRange(MantissaScale scale); + + static std::set const& + getAllScales(); + private: static constexpr rep getMin(MantissaScale scale) @@ -90,6 +112,7 @@ private: { case MantissaScale::Small: return 1'000'000'000'000'000ULL; + case MantissaScale::LargeLegacy: case MantissaScale::Large: return 1'000'000'000'000'000'000ULL; default: @@ -99,6 +122,27 @@ private: throw std::runtime_error("Unknown mantissa scale"); } } + + static constexpr CuspRoundingFix + isCuspFixEnabled(MantissaScale scale_) + { + switch (scale_) + { + case MantissaScale::Small: + case MantissaScale::LargeLegacy: + return CuspRoundingFix::Disabled; + case MantissaScale::Large: + return CuspRoundingFix::Enabled; + default: + // Since this can never be called outside a non-constexpr + // context, this throw assures that the build fails if an + // invalid scale is used. + throw std::runtime_error("Unknown mantissa scale"); + } + } + + static std::unordered_map const& + getRanges(); }; // Like std::integral, but only 64-bit integral types. @@ -424,49 +468,29 @@ public: return kRANGE.get().log; } - /// oneSmall is needed because the ranges are private - constexpr static Number - oneSmall(); - /// oneLarge is needed because the ranges are private - constexpr static Number - oneLarge(); - - // And one is needed because it needs to choose between oneSmall and - // oneLarge based on the current range static Number one(); - template + template < + auto minMantissa, + auto maxMantissa, + Integral64 T = std::decay_t, + Integral64 TMax = std::decay_t> [[nodiscard]] std::pair - normalizeToRange(T minMantissa, T maxMantissa) const; + normalizeToRange() const; private: static thread_local RoundingMode mode; // The available ranges for mantissa - constexpr static MantissaRange kSMALL_RANGE{MantissaRange::MantissaScale::Small}; - static_assert(isPowerOfTen(kSMALL_RANGE.min)); - static_assert(kSMALL_RANGE.min == 1'000'000'000'000'000LL); - static_assert(kSMALL_RANGE.max == 9'999'999'999'999'999LL); - static_assert(kSMALL_RANGE.log == 15); - static_assert(kSMALL_RANGE.min < kMAX_REP); - static_assert(kSMALL_RANGE.max < kMAX_REP); - constexpr static MantissaRange kLARGE_RANGE{MantissaRange::MantissaScale::Large}; - static_assert(isPowerOfTen(kLARGE_RANGE.min)); - static_assert(kLARGE_RANGE.min == 1'000'000'000'000'000'000ULL); - static_assert(kLARGE_RANGE.max == internalrep(9'999'999'999'999'999'999ULL)); - static_assert(kLARGE_RANGE.log == 18); - static_assert(kLARGE_RANGE.min < kMAX_REP); - static_assert(kLARGE_RANGE.max > kMAX_REP); - // The range for the mantissa when normalized. // Use reference_wrapper to avoid making copies, and prevent accidentally // changing the values inside the range. static thread_local std::reference_wrapper kRANGE; void - normalize(); + normalize(MantissaRange const& range); /** Normalize Number components to an arbitrary range. * @@ -481,7 +505,8 @@ private: T& mantissa, int& exponent, internalrep const& minMantissa, - internalrep const& maxMantissa); + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled); template friend void @@ -490,7 +515,8 @@ private: T& mantissa, int& exponent, MantissaRange::rep const& minMantissa, - MantissaRange::rep const& maxMantissa); + MantissaRange::rep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled); [[nodiscard]] bool isnormal() const noexcept; @@ -526,7 +552,7 @@ constexpr static Number kNUM_ZERO{}; inline Number::Number(bool negative, internalrep mantissa, int exponent, Normalized) : Number(negative, mantissa, exponent, Unchecked{}) { - normalize(); + normalize(kRANGE); } inline Number::Number(internalrep mantissa, int exponent, Normalized) @@ -696,10 +722,19 @@ Number::isnormal() const noexcept kMIN_EXPONENT <= exponent_ && exponent_ <= kMAX_EXPONENT); } -template +template std::pair -Number::normalizeToRange(T minMantissa, T maxMantissa) const +Number::normalizeToRange() const { + static_assert(std::is_same_v || std::is_same_v); + static_assert(std::is_same_v); + auto constexpr min = static_cast(minMantissa); + auto constexpr max = static_cast(maxMantissa); + static_assert(min > 0); + static_assert(min % 10 == 0); + static_assert(max % 10 == 9); + static_assert((max + 1) / 10 == min); + bool negative = negative_; internalrep mantissa = mantissa_; int exponent = exponent_; @@ -711,7 +746,10 @@ Number::normalizeToRange(T minMantissa, T maxMantissa) const "xrpl::Number::normalizeToRange", "Number is non-negative for unsigned range."); } - Number::normalize(negative, mantissa, exponent, minMantissa, maxMantissa); + // Don't need to worry about the cuspRounding fix because rounding up will never take the + // mantissa over maxMantissa with a ones digit value other than 0. 0 can safely be truncated. + Number::normalize( + negative, mantissa, exponent, min, max, MantissaRange::CuspRoundingFix::Disabled); auto const sign = negative ? -1 : 1; return std::make_pair(static_cast(sign * mantissa), exponent); @@ -762,9 +800,11 @@ to_string(MantissaRange::MantissaScale const& scale) switch (scale) { case MantissaRange::MantissaScale::Small: - return "small"; + return "Small"; + case MantissaRange::MantissaScale::LargeLegacy: + return "LargeLegacy"; case MantissaRange::MantissaScale::Large: - return "large"; + return "Large"; default: throw std::runtime_error("Bad scale"); } diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index 7fa6ef88ae..c685ae1e17 100644 --- a/include/xrpl/protocol/STAmount.h +++ b/include/xrpl/protocol/STAmount.h @@ -540,7 +540,7 @@ STAmount::fromNumber(A const& a, Number const& number) return STAmount{asset, intValue, 0, negative}; } - auto const [mantissa, exponent] = working.normalizeToRange(kMIN_VALUE, kMAX_VALUE); + auto const [mantissa, exponent] = working.normalizeToRange(); return STAmount{asset, mantissa, exponent, negative}; } diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 08ead182bf..1d89b05893 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -28,7 +28,76 @@ using int128_t = __int128_t; namespace xrpl { thread_local Number::RoundingMode Number::mode = Number::RoundingMode::ToNearest; -thread_local std::reference_wrapper Number::kRANGE = kLARGE_RANGE; +thread_local std::reference_wrapper Number::kRANGE = + MantissaRange::getMantissaRange(MantissaRange::MantissaScale::Large); + +std::set const& +MantissaRange::getAllScales() +{ + static std::set const scales = { + MantissaRange::MantissaScale::Small, + MantissaRange::MantissaScale::LargeLegacy, + MantissaRange::MantissaScale::Large, + }; + return scales; +} + +std::unordered_map const& +MantissaRange::getRanges() +{ + static auto const map = []() { + 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 range{MantissaRange::MantissaScale::Small}; + static_assert(isPowerOfTen(range.min)); + static_assert(range.min == 1'000'000'000'000'000LL); + static_assert(range.max == 9'999'999'999'999'999LL); + static_assert(range.log == 15); + static_assert(range.min < Number::kMAX_REP); + static_assert(range.max < Number::kMAX_REP); + static_assert(range.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); + } + { + [[maybe_unused]] + constexpr static MantissaRange range{MantissaRange::MantissaScale::LargeLegacy}; + static_assert(isPowerOfTen(range.min)); + static_assert(range.min == 1'000'000'000'000'000'000ULL); + static_assert(range.max == rep(9'999'999'999'999'999'999ULL)); + static_assert(range.log == 18); + static_assert(range.min < Number::kMAX_REP); + static_assert(range.max > Number::kMAX_REP); + static_assert(range.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); + } + { + [[maybe_unused]] + constexpr static MantissaRange range{MantissaRange::MantissaScale::Large}; + static_assert(isPowerOfTen(range.min)); + static_assert(range.min == 1'000'000'000'000'000'000ULL); + static_assert(range.max == rep(9'999'999'999'999'999'999ULL)); + static_assert(range.log == 18); + static_assert(range.min < Number::kMAX_REP); + static_assert(range.max > Number::kMAX_REP); + static_assert(range.cuspRoundingFixEnabled == CuspRoundingFix::Enabled); + } + return map; + }(); + + return map; +} + +MantissaRange const& +MantissaRange::getMantissaRange(MantissaScale scale) +{ + return getRanges().at(scale); +} Number::RoundingMode Number::getround() @@ -51,10 +120,37 @@ Number::getMantissaScale() void Number::setMantissaScale(MantissaRange::MantissaScale scale) { - if (scale != MantissaRange::MantissaScale::Small && - scale != MantissaRange::MantissaScale::Large) + if (!MantissaRange::getAllScales().contains(scale)) logicError("Unknown mantissa scale"); - kRANGE = scale == MantissaRange::MantissaScale::Small ? kSMALL_RANGE : kLARGE_RANGE; + kRANGE = MantissaRange::getMantissaRange(scale); +} + +// Optimization equivalent to: +// auto r = static_cast(u % 10); +// u /= 10; +// return r; +// Derived from Hacker's Delight Second Edition Chapter 10 +// by Henry S. Warren, Jr. +static inline unsigned +divu10(uint128_t& u) +{ + // q = u * 0.75 + auto q = (u >> 1) + (u >> 2); + // iterate towards q = u * 0.8 + q += q >> 4; + q += q >> 8; + q += q >> 16; + q += q >> 32; + q += q >> 64; + // q /= 8 approximately == u / 10 + q >>= 3; + // r = u - q * 10 approximately == u % 10 + auto r = static_cast(u - ((q << 3) + (q << 1))); + // correction c is 1 if r >= 10 else 0 + auto c = (r + 6) >> 4; + u = q + c; + r -= c * 10; + return r; } // Guard @@ -92,6 +188,18 @@ public: unsigned pop() noexcept; + /** Drop a digit from the mantissa, and increment the exponent, storing the dropped digit in + * this Guard. + * + * Substitute for: + push(mantissa % 10); + mantissa /= 10; + ++exponent; + */ + template + void + doDropDigit(T& mantissa, int& exponent) noexcept; + // 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. @@ -107,6 +215,7 @@ public: int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, std::string location); // Modify the result to the correctly rounded value @@ -168,6 +277,27 @@ Number::Guard::pop() noexcept return d; } +template +void +Number::Guard::doDropDigit(T& mantissa, int& exponent) noexcept +{ + push(mantissa % 10); + mantissa /= 10; + ++exponent; +} + +// Use the divu10 optimization for uint128s +template <> +void +Number::Guard::doDropDigit(uint128_t& mantissa, int& exponent) noexcept +{ + // The following is optimization for: + // push(static_cast(mantissa % 10)); + // mantissa /= 10; + push(divu10(mantissa)); + ++exponent; +} + // Returns: // -1 if Guard is less than half // 0 if Guard is exactly half @@ -242,18 +372,58 @@ Number::Guard::doRoundUp( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, std::string location) { auto r = round(); if (r == 1 || (r == 0 && (mantissa & 1) == 1)) { - ++mantissa; - // Ensure mantissa after incrementing fits within both the - // min/maxMantissa range and is a valid "rep". - if (mantissa > maxMantissa || mantissa > kMAX_REP) + if (cuspRoundingFixEnabled == MantissaRange::CuspRoundingFix::Enabled) { - mantissa /= 10; - ++exponent; + // Ensure mantissa after incrementing fits within both the + // min/maxMantissa range and is a valid "rep". + if (mantissa < maxMantissa && mantissa < kMAX_REP) + { + // Nothing unusual here, just increment the mantissa + ++mantissa; + } + else + { + // Incrementing the mantissa will require dividing, which will require rounding. So + // _don't_ increment the mantissa. Instead, divide and round recursively. It should + // be impossible to recurse more than once, because once the mantissa is divided by + // 10, it will be _well_ under maxMantissa and kMAX_REP, so adding 1 will have no + // change of bringing it back over. + doDropDigit(mantissa, exponent); + XRPL_ASSERT_PARTS( + mantissa < maxMantissa && mantissa < kMAX_REP, + "xrpl::Number::Guard::doRoundUp", + "can't recurse more than once"); + // Here be dragons + doRoundUp( + negative, + mantissa, + exponent, + minMantissa, + maxMantissa, + cuspRoundingFixEnabled, + location); + return; + } + } + else + { + // Need to preserve the incorrect behavior until the fix amendment can be retired, + // because otherwise would risk an unplanned ledger fork. + ++mantissa; + // Ensure mantissa after incrementing fits within both the + // min/maxMantissa range and is a valid "rep". + if (mantissa > maxMantissa || mantissa > kMAX_REP) + { + // Don't use doDropDigit here + mantissa /= 10; + ++exponent; + } } } bringIntoRange(negative, mantissa, exponent, minMantissa); @@ -293,9 +463,9 @@ Number::Guard::doRound(rep& drops, std::string location) const { static_assert(sizeof(internalrep) == sizeof(rep)); // This should be impossible, because it's impossible to represent - // "maxRep + 0.6" in Number, regardless of the scale. There aren't - // enough digits available. You'd either get a mantissa of "maxRep" - // or "(maxRep + 1) / 10", neither of which will round up when + // "kMAX_REP + 0.6" in Number, regardless of the scale. There aren't + // enough digits available. You'd either get a mantissa of "kMAX_REP" + // or "(kMAX_REP + 1) / 10", neither of which will round up when // converting to rep, though the latter might overflow _before_ // rounding. Throw(std::string(location)); // LCOV_EXCL_LINE @@ -331,29 +501,11 @@ Number::externalToInternal(rep mantissa) return static_cast(-temp); } -constexpr Number -Number::oneSmall() -{ - return Number{false, Number::kSMALL_RANGE.min, -Number::kSMALL_RANGE.log, Number::Unchecked{}}; -}; - -constexpr Number kONE_SML = Number::oneSmall(); - -constexpr Number -Number::oneLarge() -{ - return Number{false, Number::kLARGE_RANGE.min, -Number::kLARGE_RANGE.log, Number::Unchecked{}}; -}; - -constexpr Number kONE_LRG = Number::oneLarge(); - Number Number::one() { - if (&kRANGE.get() == &kSMALL_RANGE) - return kONE_SML; - XRPL_ASSERT(&kRANGE.get() == &kLARGE_RANGE, "Number::one() : valid range"); - return kONE_LRG; + auto const& range = kRANGE.get(); + return Number{false, range.min, -range.log, Number::Unchecked{}}; } // Use the member names in this static function for now so the diff is cleaner @@ -365,7 +517,8 @@ doNormalize( T& mantissa, int& exponent, MantissaRange::rep const& minMantissa, - MantissaRange::rep const& maxMantissa) + MantissaRange::rep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) { auto constexpr kMIN_EXPONENT = Number::kMIN_EXPONENT; auto constexpr kMAX_EXPONENT = Number::kMAX_EXPONENT; @@ -394,9 +547,7 @@ doNormalize( { if (exponent >= kMAX_EXPONENT) throw std::overflow_error("Number::normalize 1"); - g.push(m % 10); - m /= 10; - ++exponent; + g.doDropDigit(m, exponent); } if ((exponent < kMIN_EXPONENT) || (m < minMantissa)) { @@ -407,7 +558,7 @@ doNormalize( } // When using the largeRange, "m" needs fit within an int64, even if - // the final mantissa_ is going to end up larger to fit within the + // the final mantissa is going to end up larger to fit within the // MantissaRange. Cut it down here so that the rounding will be done while // it's smaller. // @@ -415,26 +566,31 @@ doNormalize( // so "m" will be modified to 990,000,000,000,012,345. Then that value // will be rounded to 990,000,000,000,012,345 or // 990,000,000,000,012,346, depending on the rounding mode. Finally, - // mantissa_ will be "m*10" so it fits within the range, and end up as + // mantissa will be "m*10" so it fits within the range, and end up as // 9,900,000,000,000,123,450 or 9,900,000,000,000,123,460. - // mantissa() will return mantissa_ / 10, and exponent() will return - // exponent_ + 1. + // mantissa() will return mantissa / 10, and exponent() will return + // exponent + 1. if (m > kMAX_REP) { if (exponent >= kMAX_EXPONENT) throw std::overflow_error("Number::normalize 1.5"); - g.push(m % 10); - m /= 10; - ++exponent; + g.doDropDigit(m, exponent); } // Before modification, m should be within the min/max range. After - // modification, it must be less than maxRep. In other words, the original - // value should have been no more than maxRep * 10. - // (maxRep * 10 > maxMantissa) + // modification, it must be less than kMAX_REP. In other words, the original + // value should have been no more than kMAX_REP * 10. + // (kMAX_REP * 10 > maxMantissa) XRPL_ASSERT_PARTS(m <= kMAX_REP, "xrpl::doNormalize", "intermediate mantissa fits in int64"); mantissa = m; - g.doRoundUp(negative, mantissa, exponent, minMantissa, maxMantissa, "Number::normalize 2"); + g.doRoundUp( + negative, + mantissa, + exponent, + minMantissa, + maxMantissa, + cuspRoundingFixEnabled, + "Number::normalize 2"); XRPL_ASSERT_PARTS( mantissa >= minMantissa && mantissa <= maxMantissa, "xrpl::doNormalize", @@ -448,9 +604,10 @@ Number::normalize( uint128_t& mantissa, int& exponent, internalrep const& minMantissa, - internalrep const& maxMantissa) + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) { - doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled); } template <> @@ -460,9 +617,10 @@ Number::normalize( unsigned long long& mantissa, int& exponent, internalrep const& minMantissa, - internalrep const& maxMantissa) + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) { - doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled); } template <> @@ -472,16 +630,16 @@ Number::normalize( unsigned long& mantissa, int& exponent, internalrep const& minMantissa, - internalrep const& maxMantissa) + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled) { - doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa); + doNormalize(negative, mantissa, exponent, minMantissa, maxMantissa, cuspRoundingFixEnabled); } void -Number::normalize() +Number::normalize(MantissaRange const& range) { - auto const& range = kRANGE.get(); - normalize(negative_, mantissa_, exponent_, range.min, range.max); + normalize(negative_, mantissa_, exponent_, range.min, range.max, range.cuspRoundingFixEnabled); } // Copy the number, but set a new exponent. Because the mantissa doesn't change, @@ -542,9 +700,7 @@ Number::operator+=(Number const& y) g.setNegative(); do { - g.push(xm % 10); - xm /= 10; - ++xe; + g.doDropDigit(xm, xe); } while (xe < ye); } else if (xe > ye) @@ -553,26 +709,30 @@ Number::operator+=(Number const& y) g.setNegative(); do { - g.push(ym % 10); - ym /= 10; - ++ye; + 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; if (xn == yn) { xm += ym; if (xm > maxMantissa || xm > kMAX_REP) { - g.push(xm % 10); - xm /= 10; - ++xe; + g.doDropDigit(xm, xe); } - g.doRoundUp(xn, xm, xe, minMantissa, maxMantissa, "Number::addition overflow"); + g.doRoundUp( + xn, + xm, + xe, + minMantissa, + maxMantissa, + cuspRoundingFixEnabled, + "Number::addition overflow"); } else { @@ -598,38 +758,10 @@ Number::operator+=(Number const& y) negative_ = xn; mantissa_ = static_cast(xm); exponent_ = xe; - normalize(); + normalize(range); return *this; } -// Optimization equivalent to: -// auto r = static_cast(u % 10); -// u /= 10; -// return r; -// Derived from Hacker's Delight Second Edition Chapter 10 -// by Henry S. Warren, Jr. -static inline unsigned -divu10(uint128_t& u) -{ - // q = u * 0.75 - auto q = (u >> 1) + (u >> 2); - // iterate towards q = u * 0.8 - q += q >> 4; - q += q >> 8; - q += q >> 16; - q += q >> 32; - q += q >> 64; - // q /= 8 approximately == u / 10 - q >>= 3; - // r = u - q * 10 approximately == u % 10 - auto r = static_cast(u - ((q << 3) + (q << 1))); - // correction c is 1 if r >= 10 else 0 - auto c = (r + 6) >> 4; - u = q + c; - r -= c * 10; - return r; -} - Number& Number::operator*=(Number const& y) { @@ -667,15 +799,13 @@ 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; while (zm > maxMantissa || zm > kMAX_REP) { - // The following is optimization for: - // g.push(static_cast(zm % 10)); - // zm /= 10; - g.push(divu10(zm)); - ++ze; + g.doDropDigit(zm, ze); } + xm = static_cast(zm); xe = ze; g.doRoundUp( @@ -684,12 +814,13 @@ Number::operator*=(Number const& y) xe, minMantissa, maxMantissa, + cuspRoundingFixEnabled, "Number::multiplication overflow : exponent is " + std::to_string(xe)); negative_ = zn; mantissa_ = xm; exponent_ = xe; - normalize(); + normalize(range); return *this; } @@ -721,6 +852,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; // Shift by 10^17 gives greatest precision while not overflowing // uint128_t or the cast back to int64_t @@ -728,8 +860,6 @@ Number::operator/=(Number const& y) // log(2^128,10) ~ 38.5 // largeRange.log = 18, fits in 10^19 // f can be up to 10^(38-19) = 10^19 safely - static_assert(kSMALL_RANGE.log == 15); - static_assert(kLARGE_RANGE.log == 18); bool const small = Number::getMantissaScale() == MantissaRange::MantissaScale::Small; uint128_t const f = small ? 100'000'000'000'000'000 : 10'000'000'000'000'000'000ULL; XRPL_ASSERT_PARTS(f >= minMantissa * 10, "Number::operator/=", "factor expected size"); @@ -779,7 +909,7 @@ Number::operator/=(Number const& y) ze -= 3; } } - normalize(zn, zm, ze, minMantissa, maxMantissa); + normalize(zn, zm, ze, minMantissa, maxMantissa, cuspRoundingFixEnabled); negative_ = zn; mantissa_ = static_cast(zm); exponent_ = ze; @@ -801,10 +931,9 @@ operator rep() const g.setNegative(); drops = -drops; } - for (; offset < 0; ++offset) + while (offset < 0) { - g.push(drops % 10); - drops /= 10; + g.doDropDigit(drops, offset); } for (; offset > 0; --offset) { @@ -831,7 +960,7 @@ Number::truncate() const noexcept } // We are guaranteed that normalize() will never throw an exception // because exponent is either negative or zero at this point. - ret.normalize(); + ret.normalize(kRANGE); return ret; } diff --git a/src/libxrpl/protocol/IOUAmount.cpp b/src/libxrpl/protocol/IOUAmount.cpp index e4326d611e..9e3c024ca0 100644 --- a/src/libxrpl/protocol/IOUAmount.cpp +++ b/src/libxrpl/protocol/IOUAmount.cpp @@ -56,7 +56,7 @@ IOUAmount::fromNumber(Number const& number) // to normalize, which calls fromNumber IOUAmount result{}; std::tie(result.mantissa_, result.exponent_) = - number.normalizeToRange(kMIN_MANTISSA, kMAX_MANTISSA); + number.normalizeToRange(); return result; } diff --git a/src/libxrpl/protocol/Rules.cpp b/src/libxrpl/protocol/Rules.cpp index 2c971749b6..8040571c75 100644 --- a/src/libxrpl/protocol/Rules.cpp +++ b/src/libxrpl/protocol/Rules.cpp @@ -38,11 +38,13 @@ setCurrentTransactionRules(std::optional r) // Make global changes associated with the rules before the value is moved. // Push the appropriate setting, instead of having the class pull every time // the value is needed. That could get expensive fast. - bool const enableLargeNumbers = - !r || (r->enabled(featureSingleAssetVault) || r->enabled(featureLendingProtocol)); + bool const enableCuspRoundingFix = !r || r->enabled(fixCleanup3_2_0); + bool const enableLargeNumbers = enableCuspRoundingFix || + (r->enabled(featureSingleAssetVault) || r->enabled(featureLendingProtocol)); Number::setMantissaScale( - enableLargeNumbers ? MantissaRange::MantissaScale::Large - : MantissaRange::MantissaScale::Small); + enableCuspRoundingFix ? MantissaRange::MantissaScale::Large + : enableLargeNumbers ? MantissaRange::MantissaScale::LargeLegacy + : MantissaRange::MantissaScale::Small); *getCurrentTransactionRulesRef() = std::move(r); } diff --git a/src/libxrpl/protocol/STNumber.cpp b/src/libxrpl/protocol/STNumber.cpp index f6481a4d5d..60008bbe77 100644 --- a/src/libxrpl/protocol/STNumber.cpp +++ b/src/libxrpl/protocol/STNumber.cpp @@ -96,7 +96,8 @@ 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::Large, + Number::getMantissaScale() == MantissaRange::MantissaScale::LargeLegacy || + Number::getMantissaScale() == MantissaRange::MantissaScale::Large, "xrpl::STNumber::add", "STNumber only used with large mantissa scale"); #endif diff --git a/src/test/app/AMMClawbackMPT_test.cpp b/src/test/app/AMMClawbackMPT_test.cpp index 6f0fff0282..523a4af7fc 100644 --- a/src/test/app/AMMClawbackMPT_test.cpp +++ b/src/test/app/AMMClawbackMPT_test.cpp @@ -1829,7 +1829,7 @@ class AMMClawbackMPT_test : public beast::unit_test::Suite testLastHolderLPTokenBalance(all - fixAMMv1_3 - fixAMMClawbackRounding); testLastHolderLPTokenBalance( all - fixAMMv1_3 - fixAMMClawbackRounding - featureSingleAssetVault - - featureLendingProtocol); + featureLendingProtocol - fixCleanup3_2_0); testLastHolderLPTokenBalance(all - fixAMMClawbackRounding); testClawAssetCheck(all); } diff --git a/src/test/app/AMMClawback_test.cpp b/src/test/app/AMMClawback_test.cpp index 9683e8ac17..500fd84ebe 100644 --- a/src/test/app/AMMClawback_test.cpp +++ b/src/test/app/AMMClawback_test.cpp @@ -2506,8 +2506,8 @@ class AMMClawback_test : public beast::unit_test::Suite { // For now, just disable SAV entirely, which locks in the small Number // mantissas - FeatureBitset const all = - jtx::testableAmendments() - featureSingleAssetVault - featureLendingProtocol; + FeatureBitset const all = jtx::testableAmendments() - featureSingleAssetVault - + featureLendingProtocol - fixCleanup3_2_0; testInvalidRequest(all); testInvalidRequest(all - featureMPTokensV2); diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index 8132c012ad..68208f5d36 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -70,6 +70,11 @@ struct AMMExtended_test : public jtx::AMMTest // Use small Number mantissas for the life of this test. NumberMantissaScaleGuard const sg{xrpl::MantissaRange::MantissaScale::Small}; + // For now, just disable SAV entirely, which locks in the small Number + // mantissas + FeatureBitset const all_{ + testableAmendments() - featureSingleAssetVault - featureLendingProtocol - fixCleanup3_2_0}; + private: void testRmFundedOffer(FeatureBitset features) @@ -1348,37 +1353,33 @@ private: testOffers() { using namespace jtx; - // For now, just disable SAV entirely, which locks in the small Number - // mantissas - FeatureBitset const all{ - testableAmendments() - featureSingleAssetVault - featureLendingProtocol}; - testRmFundedOffer(all); - testRmFundedOffer(all - fixAMMv1_1 - fixAMMv1_3); - testEnforceNoRipple(all); - testFillModes(all); - testOfferCrossWithXRP(all); - testOfferCrossWithLimitOverride(all); - testCurrencyConversionEntire(all); - testCurrencyConversionInParts(all); - testCrossCurrencyStartXRP(all); - testCrossCurrencyEndXRP(all); - testCrossCurrencyBridged(all); - testOfferFeesConsumeFunds(all); - testOfferCreateThenCross(all); - testSellFlagExceedLimit(all); - testGatewayCrossCurrency(all); - testGatewayCrossCurrency(all - fixAMMv1_1 - fixAMMv1_3); - testBridgedCross(all); - testSellWithFillOrKill(all); - testTransferRateOffer(all); - testSelfIssueOffer(all); - testBadPathAssert(all); - testSellFlagBasic(all); - testDirectToDirectPath(all); - testDirectToDirectPath(all - fixAMMv1_1 - fixAMMv1_3); - testRequireAuth(all); - testMissingAuth(all); + testRmFundedOffer(all_); + testRmFundedOffer(all_ - fixAMMv1_1 - fixAMMv1_3); + testEnforceNoRipple(all_); + testFillModes(all_); + testOfferCrossWithXRP(all_); + testOfferCrossWithLimitOverride(all_); + testCurrencyConversionEntire(all_); + testCurrencyConversionInParts(all_); + testCrossCurrencyStartXRP(all_); + testCrossCurrencyEndXRP(all_); + testCrossCurrencyBridged(all_); + testOfferFeesConsumeFunds(all_); + testOfferCreateThenCross(all_); + testSellFlagExceedLimit(all_); + testGatewayCrossCurrency(all_); + testGatewayCrossCurrency(all_ - fixAMMv1_1 - fixAMMv1_3); + testBridgedCross(all_); + testSellWithFillOrKill(all_); + testTransferRateOffer(all_); + testSelfIssueOffer(all_); + testBadPathAssert(all_); + testSellFlagBasic(all_); + testDirectToDirectPath(all_); + testDirectToDirectPath(all_ - fixAMMv1_1 - fixAMMv1_3); + testRequireAuth(all_); + testMissingAuth(all_); } void @@ -3513,15 +3514,11 @@ private: testFlow() { using namespace jtx; - // For now, just disable SAV entirely, which locks in the small Number - // mantissas in the transaction engine - FeatureBitset const all{ - testableAmendments() - featureSingleAssetVault - featureLendingProtocol}; - testFalseDry(all); - testBookStep(all); - testTransferRateNoOwnerFee(all); - testTransferRateNoOwnerFee(all - fixAMMv1_1 - fixAMMv1_3); + testFalseDry(all_); + testBookStep(all_); + testTransferRateNoOwnerFee(all_); + testTransferRateNoOwnerFee(all_ - fixAMMv1_1 - fixAMMv1_3); testLimitQuality(); testXRPPathLoop(); } @@ -3530,34 +3527,22 @@ private: testCrossingLimits() { using namespace jtx; - // For now, just disable SAV entirely, which locks in the small Number - // mantissas in the transaction engine - FeatureBitset const all{ - testableAmendments() - featureSingleAssetVault - featureLendingProtocol}; - testStepLimit(all); - testStepLimit(all - fixAMMv1_1 - fixAMMv1_3); + testStepLimit(all_); + testStepLimit(all_ - fixAMMv1_1 - fixAMMv1_3); } void testDeliverMin() { using namespace jtx; - // For now, just disable SAV entirely, which locks in the small Number - // mantissas in the transaction engine - FeatureBitset const all{ - testableAmendments() - featureSingleAssetVault - featureLendingProtocol}; - testConvertAllOfAnAsset(all); - testConvertAllOfAnAsset(all - fixAMMv1_1 - fixAMMv1_3); + testConvertAllOfAnAsset(all_); + testConvertAllOfAnAsset(all_ - fixAMMv1_1 - fixAMMv1_3); } void testDepositAuth() { - // For now, just disable SAV entirely, which locks in the small Number - // mantissas in the transaction engine - FeatureBitset const all{ - jtx::testableAmendments() - featureSingleAssetVault - featureLendingProtocol}; - testPayment(all); + testPayment(all_); testPayIOU(); } @@ -3565,13 +3550,9 @@ private: testFreeze() { using namespace test::jtx; - // For now, just disable SAV entirely, which locks in the small Number - // mantissas in the transaction engine - FeatureBitset const sa{ - testableAmendments() - featureSingleAssetVault - featureLendingProtocol}; - testRippleState(sa); - testGlobalFreeze(sa); - testOffersWhenFrozen(sa); + testRippleState(all_); + testGlobalFreeze(all_); + testOffersWhenFrozen(all_); } void diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 8e2c0255ef..b2792ab66f 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -81,7 +81,8 @@ private: { // For now, just disable SAV entirely, which locks in the small Number // mantissas - return jtx::testableAmendments() - featureSingleAssetVault - featureLendingProtocol; + return jtx::testableAmendments() - featureSingleAssetVault - featureLendingProtocol - + fixCleanup3_2_0; } void @@ -2624,10 +2625,6 @@ private: using namespace jtx; using namespace std::chrono; - // For now, just disable SAV entirely, which locks in the small Number - // mantissas - features = features - featureSingleAssetVault - featureLendingProtocol; - // Auction slot initially is owned by AMM creator, who pays 0 price. // Bid 110 tokens. Pay bidMin. @@ -3337,11 +3334,6 @@ private: testcase("Basic Payment"); using namespace jtx; - // For now, just disable SAV entirely, which locks in the small Number - // mantissas - features = - features - featureSingleAssetVault - featureLendingProtocol - featureLendingProtocol; - // Payment 100USD for 100XRP. // Force one path with tfNoRippleDirect. testAMM( diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 965414553f..3021f192ba 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -4256,109 +4256,119 @@ class Invariants_test : public beast::unit_test::Suite std::vector values; }; - NumberMantissaScaleGuard const g{MantissaRange::MantissaScale::Large}; - - auto makeDelta = [&vaultAsset](Number const& n) -> ValidVault::DeltaInfo { - return {.delta = n, .scale = scale(n, vaultAsset.raw())}; - }; - - auto const testCases = std::vector{ - { - .name = "No values", - .expectedMinScale = 0, - .values = {}, - }, - { - .name = "Mixed integer and Number values", - .expectedMinScale = -15, - .values = {makeDelta(1), makeDelta(-1), makeDelta(Number{10, -1})}, - }, - { - .name = "Mixed scales", - .expectedMinScale = -17, - .values = - {makeDelta(Number{1, -2}), makeDelta(Number{5, -3}), makeDelta(Number{3, -2})}, - }, - { - .name = "Equal scales", - .expectedMinScale = -16, - .values = - {makeDelta(Number{1, -1}), makeDelta(Number{5, -1}), makeDelta(Number{1, -1})}, - }, - { - .name = "Mixed mantissa sizes", - .expectedMinScale = -12, - .values = - {makeDelta(Number{1}), - makeDelta(Number{1234, -3}), - makeDelta(Number{12345, -6}), - makeDelta(Number{123, 1})}, - }, - }; - - for (auto const& tc : testCases) + for (auto const mantissaScale : { + MantissaRange::MantissaScale::LargeLegacy, + MantissaRange::MantissaScale::Large, + }) { - testcase("vault computeCoarsestScale: " + tc.name); + NumberMantissaScaleGuard const g{mantissaScale}; - auto const actualScale = ValidVault::computeCoarsestScale(tc.values); + auto makeDelta = [&vaultAsset](Number const& n) -> ValidVault::DeltaInfo { + return {.delta = n, .scale = scale(n, vaultAsset.raw())}; + }; - BEAST_EXPECTS( - actualScale == tc.expectedMinScale, - "expected: " + std::to_string(tc.expectedMinScale) + - ", actual: " + std::to_string(actualScale)); - for (auto const& num : tc.values) - { - // None of these scales are far enough apart that rounding the - // values would lose information, so check that the rounded - // value matches the original. - auto const actualRounded = roundToAsset(vaultAsset, num.delta, actualScale); - BEAST_EXPECTS( - actualRounded == num.delta, - "number " + to_string(num.delta) + " rounded to scale " + - std::to_string(actualScale) + " is " + to_string(actualRounded)); - } - } - - auto const testCases2 = std::vector{ - { - .name = "False equivalence", - .expectedMinScale = -15, - .values = - { - makeDelta(Number{1234567890123456789, -18}), - makeDelta(Number{12345, -4}), - makeDelta(Number{1}), - }, - }, - }; - - // Unlike the first set of test cases, the values in these test could - // look equivalent if using the wrong scale. - for (auto const& tc : testCases2) - { - testcase("vault computeCoarsestScale: " + tc.name); - - auto const actualScale = ValidVault::computeCoarsestScale(tc.values); - - BEAST_EXPECTS( - actualScale == tc.expectedMinScale, - "expected: " + std::to_string(tc.expectedMinScale) + - ", actual: " + std::to_string(actualScale)); - std::optional first; - Number firstRounded; - for (auto const& num : tc.values) - { - if (!first) + auto const testCases = std::vector{ { - first = num.delta; - firstRounded = roundToAsset(vaultAsset, num.delta, actualScale); - continue; - } - auto const numRounded = roundToAsset(vaultAsset, num.delta, actualScale); + .name = "No values", + .expectedMinScale = 0, + .values = {}, + }, + { + .name = "Mixed integer and Number values", + .expectedMinScale = -15, + .values = {makeDelta(1), makeDelta(-1), makeDelta(Number{10, -1})}, + }, + { + .name = "Mixed scales", + .expectedMinScale = -17, + .values = + {makeDelta(Number{1, -2}), + makeDelta(Number{5, -3}), + makeDelta(Number{3, -2})}, + }, + { + .name = "Equal scales", + .expectedMinScale = -16, + .values = + {makeDelta(Number{1, -1}), + makeDelta(Number{5, -1}), + makeDelta(Number{1, -1})}, + }, + { + .name = "Mixed mantissa sizes", + .expectedMinScale = -12, + .values = + {makeDelta(Number{1}), + makeDelta(Number{1234, -3}), + makeDelta(Number{12345, -6}), + makeDelta(Number{123, 1})}, + }, + }; + + for (auto const& tc : testCases) + { + testcase("vault computeCoarsestScale: " + tc.name); + + auto const actualScale = ValidVault::computeCoarsestScale(tc.values); + BEAST_EXPECTS( - numRounded != firstRounded, - "at a scale of " + std::to_string(actualScale) + " " + to_string(num.delta) + - " == " + to_string(*first)); + actualScale == tc.expectedMinScale, + "expected: " + std::to_string(tc.expectedMinScale) + + ", actual: " + std::to_string(actualScale)); + for (auto const& num : tc.values) + { + // None of these scales are far enough apart that rounding the + // values would lose information, so check that the rounded + // value matches the original. + auto const actualRounded = roundToAsset(vaultAsset, num.delta, actualScale); + BEAST_EXPECTS( + actualRounded == num.delta, + "number " + to_string(num.delta) + " rounded to scale " + + std::to_string(actualScale) + " is " + to_string(actualRounded)); + } + } + + auto const testCases2 = std::vector{ + { + .name = "False equivalence", + .expectedMinScale = -15, + .values = + { + makeDelta(Number{1234567890123456789, -18}), + makeDelta(Number{12345, -4}), + makeDelta(Number{1}), + }, + }, + }; + + // Unlike the first set of test cases, the values in these test could + // look equivalent if using the wrong scale. + for (auto const& tc : testCases2) + { + testcase("vault computeCoarsestScale: " + tc.name); + + auto const actualScale = ValidVault::computeCoarsestScale(tc.values); + + BEAST_EXPECTS( + actualScale == tc.expectedMinScale, + "expected: " + std::to_string(tc.expectedMinScale) + + ", actual: " + std::to_string(actualScale)); + std::optional first; + Number firstRounded; + for (auto const& num : tc.values) + { + if (!first) + { + first = num.delta; + firstRounded = roundToAsset(vaultAsset, num.delta, actualScale); + continue; + } + auto const numRounded = roundToAsset(vaultAsset, num.delta, actualScale); + BEAST_EXPECTS( + numRounded != firstRounded, + "at a scale of " + std::to_string(actualScale) + " " + + to_string(num.delta) + " == " + to_string(*first)); + } } } } diff --git a/src/test/basics/IOUAmount_test.cpp b/src/test/basics/IOUAmount_test.cpp index 738990aac2..275a71cff1 100644 --- a/src/test/basics/IOUAmount_test.cpp +++ b/src/test/basics/IOUAmount_test.cpp @@ -156,8 +156,7 @@ public: BEAST_EXPECTS(result == expected, ss.str()); }; - for (auto const mantissaSize : - {MantissaRange::MantissaScale::Small, MantissaRange::MantissaScale::Large}) + for (auto const mantissaSize : MantissaRange::getAllScales()) { NumberMantissaScaleGuard const mg(mantissaSize); diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 9e06d6de53..c26816348d 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -6,6 +6,8 @@ #include #include +#include + #include #include #include @@ -19,6 +21,24 @@ namespace xrpl { class Number_test : public beast::unit_test::Suite { + using BigInt = boost::multiprecision::cpp_int; + + static std::string + fmt(BigInt const& value) + { + auto s = to_string(value); + std::string out; + int count = 0; + for (auto it = s.rbegin(); it != s.rend(); ++it) + { + if (count && count % 3 == 0) + out.insert(out.begin(), '_'); + out.insert(out.begin(), *it); + ++count; + } + return out; + } + public: void testZero() @@ -178,7 +198,6 @@ public: {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::kMAX_REP}, Number{6, -1}, Number{Number::kMAX_REP / 10, 1}}, {Number{Number::kMAX_REP - 1}, Number{1, 0}, Number{Number::kMAX_REP}}, // Test extremes { @@ -189,16 +208,22 @@ public: Number{2, 19}, }, { - // Does not round. Mantissas are going to be > maxRep, so if + // Does not round. Mantissas are going to be > kMAX_REP, so if // added together as uint64_t's, the result will overflow. // With addition using uint128_t, there's no problem. After // normalizing, the resulting mantissa ends up less than - // maxRep. + // kMAX_REP. 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{}}, }, }); + auto const cLargeLegacy = std::to_array({ + {Number{Number::kMAX_REP}, Number{6, -1}, Number{Number::kMAX_REP / 10, 1}}, + }); + auto const cLargeCorrected = std::to_array({ + {Number{Number::kMAX_REP}, Number{6, -1}, Number{Number::kMAX_REP / 10 + 1, 1}}, + }); auto test = [this](auto const& c) { for (auto const& [x, y, z] : c) { @@ -215,6 +240,10 @@ public: else { test(cLarge); + if (scale == MantissaRange::MantissaScale::LargeLegacy) + test(cLargeLegacy); + else + test(cLargeCorrected); } { bool caught = false; @@ -835,7 +864,7 @@ public: /* auto tests = [&](auto const& cSmall, auto const& cLarge) { test(cSmall); - if (scale != MantissaRange::mantissa_scale::small) + if (scale != MantissaRange::MantissaScale::Small) test(cLarge); }; */ @@ -1266,6 +1295,7 @@ public: "9223372036854775e3"); } break; + case MantissaRange::MantissaScale::LargeLegacy: case MantissaRange::MantissaScale::Large: // Test the edges // ((exponent < -(28)) || (exponent > -(8))))) @@ -1551,11 +1581,49 @@ public: } } + void + testUpwardRoundsDown() + { + testcase << "upward rounding produces a value below exact at kMAX_REP cusp"; + + NumberMantissaScaleGuard const mg{MantissaRange::MantissaScale::Large}; + NumberRoundModeGuard const rg{Number::RoundingMode::Upward}; + + constexpr std::int64_t aValue = 1'000'000'000'000'049'863LL; + constexpr std::int64_t bValue = 9'223'372'036'854'315'903LL; + + // Public conversion operator: STAmount::operator Number() const. + Number const a = aValue; + Number const b = bValue; + Number const product = a * b; + + // Exact reference in BigInt. + BigInt const exactProduct = BigInt(aValue) * BigInt(bValue); + + // What Number actually stored. + BigInt storedValue = BigInt(product.mantissa()); + for (int i = 0; i < product.exponent(); ++i) + storedValue *= 10; + + BigInt const signedDifference = storedValue - exactProduct; + + log << "\n" + << " a = " << fmt(BigInt(aValue)) << "\n" + << " b = " << fmt(BigInt(bValue)) << "\n" + << " exact a*b = " << fmt(exactProduct) << "\n" + << " stored = " << fmt(storedValue) << "\n" + << " stored - exact = " << fmt(signedDifference) << "\n" + << " upward = " << (signedDifference >= 0 ? "held" : "VIOLATED") << "\n"; + + BEAST_EXPECT(signedDifference >= 0); + BEAST_EXPECT(product.mantissa() == (std::numeric_limits::max() / 10) + 1); + BEAST_EXPECT(product.exponent() == 19); + } + void run() override { - for (auto const scale : - {MantissaRange::MantissaScale::Small, MantissaRange::MantissaScale::Large}) + for (auto const scale : MantissaRange::getAllScales()) { NumberMantissaScaleGuard const sg(scale); testZero(); @@ -1580,6 +1648,8 @@ public: testRounding(); testInt64(); } + // This test sets its own number range + testUpwardRoundsDown(); } }; diff --git a/src/test/jtx/AMMTest.h b/src/test/jtx/AMMTest.h index a2e6c38db3..fc8cd1b1e6 100644 --- a/src/test/jtx/AMMTest.h +++ b/src/test/jtx/AMMTest.h @@ -21,7 +21,8 @@ struct TestAMMArg std::vector features = { // For now, just disable SAV entirely, which locks in the small Number // mantissas - jtx::testableAmendments() - featureSingleAssetVault - featureLendingProtocol}; + jtx::testableAmendments() - featureSingleAssetVault - featureLendingProtocol - + fixCleanup3_2_0}; bool noLog = false; }; @@ -87,7 +88,8 @@ public: { // For now, just disable SAV entirely, which locks in the small Number // mantissas - return jtx::testableAmendments() - featureSingleAssetVault - featureLendingProtocol; + return jtx::testableAmendments() - featureSingleAssetVault - featureLendingProtocol - + fixCleanup3_2_0; } protected: diff --git a/src/test/jtx/impl/AMMTest.cpp b/src/test/jtx/impl/AMMTest.cpp index 5801e8fb44..3bb4cca484 100644 --- a/src/test/jtx/impl/AMMTest.cpp +++ b/src/test/jtx/impl/AMMTest.cpp @@ -143,7 +143,7 @@ AMMTestBase::testAMM(std::function const& cb, TestAM // mantissas Env env{ *this, - features - featureSingleAssetVault - featureLendingProtocol, + features - featureSingleAssetVault - featureLendingProtocol - fixCleanup3_2_0, arg.noLog ? std::make_unique(&logs) : nullptr}; auto const [asset1, asset2] = arg.pool ? *arg.pool : std::make_pair(XRP(10000), USD(10000)); diff --git a/src/test/protocol/STNumber_test.cpp b/src/test/protocol/STNumber_test.cpp index 914e2d003f..2f84ccee82 100644 --- a/src/test/protocol/STNumber_test.cpp +++ b/src/test/protocol/STNumber_test.cpp @@ -280,8 +280,7 @@ struct STNumber_test : public beast::unit_test::Suite { static_assert(!std::is_convertible_v); - for (auto const scale : - {MantissaRange::MantissaScale::Small, MantissaRange::MantissaScale::Large}) + for (auto const scale : MantissaRange::getAllScales()) { NumberMantissaScaleGuard const sg(scale); testcase << to_string(Number::getMantissaScale()); diff --git a/src/test/rpc/GetAggregatePrice_test.cpp b/src/test/rpc/GetAggregatePrice_test.cpp index 418b36d703..7505f69851 100644 --- a/src/test/rpc/GetAggregatePrice_test.cpp +++ b/src/test/rpc/GetAggregatePrice_test.cpp @@ -176,10 +176,10 @@ public: // or time threshold { auto const all = testableAmendments(); - for (auto const& feats : {all - featureSingleAssetVault - featureLendingProtocol, all}) + for (auto const& feats : + {all - featureSingleAssetVault - featureLendingProtocol - fixCleanup3_2_0, all}) { - for (auto const mantissaSize : - {MantissaRange::MantissaScale::Small, MantissaRange::MantissaScale::Large}) + for (auto const mantissaSize : MantissaRange::getAllScales()) { // Regardless of the features enabled, RPC is controlled by // the global mantissa size. And since it's a thread-local,