From b40d2a8e7d1f81ccee6d9fbd9afdcdf4397b732b Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 28 Apr 2026 22:28:14 -0400 Subject: [PATCH 1/6] fix: Fix a rounding error at the Number::maxRep cusp - Add helper function, doDropDigit, to wrap the common pattern: push(mantissa % 10); mantissa /= 10; ++exponent; - Might have been helpful to catch this issue when developing. --- include/xrpl/basics/Number.h | 120 +++++--- include/xrpl/protocol/STAmount.h | 2 +- src/libxrpl/basics/Number.cpp | 347 ++++++++++++++++-------- src/libxrpl/protocol/IOUAmount.cpp | 2 +- src/libxrpl/protocol/Rules.cpp | 10 +- src/libxrpl/protocol/STNumber.cpp | 3 +- src/test/app/AMMClawbackMPT_test.cpp | 2 +- src/test/app/AMMClawback_test.cpp | 4 +- src/test/app/AMMExtended_test.cpp | 105 +++---- src/test/app/AMM_test.cpp | 12 +- src/test/app/Invariants_test.cpp | 206 +++++++------- src/test/basics/IOUAmount_test.cpp | 3 +- src/test/basics/Number_test.cpp | 82 +++++- src/test/jtx/AMMTest.h | 6 +- src/test/jtx/impl/AMMTest.cpp | 2 +- src/test/protocol/STNumber_test.cpp | 3 +- src/test/rpc/GetAggregatePrice_test.cpp | 6 +- 17 files changed, 570 insertions(+), 345 deletions(-) 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, From a2b21d75ce72837eb0bd87b39ed3d7deb2f9fc86 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 6 May 2026 22:56:07 -0400 Subject: [PATCH 2/6] Fix AI-identified mistakes --- include/xrpl/basics/Number.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index fb32b42060..1db265fa6b 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -9,6 +9,7 @@ #include #include #include +#include namespace xrpl { @@ -475,7 +476,7 @@ public: auto minMantissa, auto maxMantissa, Integral64 T = std::decay_t, - Integral64 TMax = std::decay_t> + Integral64 TMax = std::decay_t> [[nodiscard]] std::pair normalizeToRange() const; From b050c151f89594fb08fbffcdca30ac1895fae2b8 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 6 May 2026 23:26:11 -0400 Subject: [PATCH 3/6] Fix clang-tidy issues, and more AI complaints --- include/xrpl/basics/Number.h | 46 +++++++++++++----------- src/libxrpl/basics/Number.cpp | 58 ++++++++++++++++--------------- src/libxrpl/protocol/Rules.cpp | 12 +++++-- src/test/app/AMMExtended_test.cpp | 2 +- src/test/basics/Number_test.cpp | 22 +++++++----- 5 files changed, 79 insertions(+), 61 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 1db265fa6b..ebdb53aa83 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -50,7 +50,7 @@ isPowerOfTen(T value) * 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 + * 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 * uses before Lending Protocol was implemented, mostly related to AMM. * @@ -61,12 +61,16 @@ isPowerOfTen(T value) * STNumber field type, and for internal calculations. That necessitated the * "large" scale. * - * The "large" scale is intended to represent all values that can be represented + * The "Large" scales are intended to represent all values that can be represented * by an STAmount - IOUs, XRP, and MPTs. It has a min value of 10^18, and a max - * value of 10^19-1. + * value of 10^19-1. "LargeLegacy" is like "Large", but preserves + * a rounding error when a computation results in a mantissa of + * Number::kMAX_REP that needs to be rounded up, but rounds down + * instead. It will maintain consistent behavior until the fixCleanup3_2_0 + * amendment is enabled. * * Note that if the mentioned amendments are eventually retired, this class - * should be left in place, but the "small" scale option should be removed. This + * should be left in place, but the "Small" scale option should be removed. This * will allow for future expansion beyond 64-bits if it is ever needed. */ struct MantissaRange @@ -125,9 +129,9 @@ private: } static constexpr CuspRoundingFix - isCuspFixEnabled(MantissaScale scale_) + isCuspFixEnabled(MantissaScale scale) { - switch (scale_) + switch (scale) { case MantissaScale::Small: case MantissaScale::LargeLegacy: @@ -473,10 +477,10 @@ public: one(); template < - auto minMantissa, - auto maxMantissa, - Integral64 T = std::decay_t, - Integral64 TMax = std::decay_t> + auto MinMantissa, + auto MaxMantissa, + Integral64 T = std::decay_t, + Integral64 TMax = std::decay_t> [[nodiscard]] std::pair normalizeToRange() const; @@ -723,18 +727,18 @@ Number::isnormal() const noexcept kMIN_EXPONENT <= exponent_ && exponent_ <= kMAX_EXPONENT); } -template +template std::pair 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); + auto constexpr kMIN = static_cast(MinMantissa); + auto constexpr kMAX = static_cast(MaxMantissa); + static_assert(kMIN > 0); + static_assert(kMIN % 10 == 0); + static_assert(kMAX % 10 == 9); + static_assert((kMAX + 1) / 10 == kMIN); bool negative = negative_; internalrep mantissa = mantissa_; @@ -750,7 +754,7 @@ Number::normalizeToRange() const // 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); + negative, mantissa, exponent, kMIN, kMAX, MantissaRange::CuspRoundingFix::Disabled); auto const sign = negative ? -1 : 1; return std::make_pair(static_cast(sign * mantissa), exponent); @@ -801,11 +805,11 @@ to_string(MantissaRange::MantissaScale const& scale) switch (scale) { case MantissaRange::MantissaScale::Small: - return "Small"; + return "small"; case MantissaRange::MantissaScale::LargeLegacy: - return "LargeLegacy"; + return "largeLegacy"; case MantissaRange::MantissaScale::Large: - return "Large"; + return "large"; default: throw std::runtime_error("Bad scale"); } diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 1d89b05893..608c7b9a42 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -10,9 +10,11 @@ #include #include #include +#include #include #include #include +#include #include #ifdef _MSC_VER @@ -34,18 +36,18 @@ thread_local std::reference_wrapper Number::kRANGE = std::set const& MantissaRange::getAllScales() { - static std::set const scales = { + static std::set const kSCALES = { MantissaRange::MantissaScale::Small, MantissaRange::MantissaScale::LargeLegacy, MantissaRange::MantissaScale::Large, }; - return scales; + return kSCALES; } std::unordered_map const& MantissaRange::getRanges() { - static auto const map = []() { + static auto const kMAP = []() { std::unordered_map map; for (auto const scale : getAllScales()) { @@ -56,41 +58,41 @@ MantissaRange::getRanges() // 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); + 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::kMAX_REP); + static_assert(kRANGE.max < Number::kMAX_REP); + static_assert(kRANGE.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); + 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::kMAX_REP); + static_assert(kRANGE.max > Number::kMAX_REP); + static_assert(kRANGE.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); + 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::kMAX_REP); + static_assert(kRANGE.max > Number::kMAX_REP); + static_assert(kRANGE.cuspRoundingFixEnabled == CuspRoundingFix::Enabled); } return map; }(); - return map; + return kMAP; } MantissaRange const& diff --git a/src/libxrpl/protocol/Rules.cpp b/src/libxrpl/protocol/Rules.cpp index 8040571c75..8274947e83 100644 --- a/src/libxrpl/protocol/Rules.cpp +++ b/src/libxrpl/protocol/Rules.cpp @@ -38,12 +38,20 @@ 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. + + // The improved accuracy that will come from enabling large + // mantissas is a goal separate from SAV and LP. It was originally + // only tied to those two amendments to avoid needing a new + // amendment of it's own, and because they require that behavior. + // Because fixCleanup3_2_0 fixes a separate bug related to the large + // mantissas, that can take precedence and activate the large + // mantissas even in the absence of the other two amendments. bool const enableCuspRoundingFix = !r || r->enabled(fixCleanup3_2_0); - bool const enableLargeNumbers = enableCuspRoundingFix || + bool const enableVaultNumbers = enableCuspRoundingFix || (r->enabled(featureSingleAssetVault) || r->enabled(featureLendingProtocol)); Number::setMantissaScale( enableCuspRoundingFix ? MantissaRange::MantissaScale::Large - : enableLargeNumbers ? MantissaRange::MantissaScale::LargeLegacy + : enableVaultNumbers ? MantissaRange::MantissaScale::LargeLegacy : MantissaRange::MantissaScale::Small); *getCurrentTransactionRulesRef() = std::move(r); diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index 68208f5d36..9b80ff2414 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -65,7 +65,7 @@ namespace xrpl::test { /** * Tests of AMM that use offers too. */ -struct AMMExtended_test : public jtx::AMMTest +class AMMExtended_test : public jtx::AMMTest { // Use small Number mantissas for the life of this test. NumberMantissaScaleGuard const sg{xrpl::MantissaRange::MantissaScale::Small}; diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index c26816348d..351edcce51 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -31,7 +31,7 @@ class Number_test : public beast::unit_test::Suite int count = 0; for (auto it = s.rbegin(); it != s.rend(); ++it) { - if (count && count % 3 == 0) + if (count != 0 && count % 3 == 0) out.insert(out.begin(), '_'); out.insert(out.begin(), *it); ++count; @@ -222,7 +222,7 @@ public: {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}}, + {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) @@ -241,9 +241,13 @@ public: { test(cLarge); if (scale == MantissaRange::MantissaScale::LargeLegacy) + { test(cLargeLegacy); + } else + { test(cLargeCorrected); + } } { bool caught = false; @@ -1589,16 +1593,16 @@ public: 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; + constexpr std::int64_t kA_VALUE = 1'000'000'000'000'049'863LL; + constexpr std::int64_t kB_VALUE = 9'223'372'036'854'315'903LL; // Public conversion operator: STAmount::operator Number() const. - Number const a = aValue; - Number const b = bValue; + Number const a = kA_VALUE; + Number const b = kB_VALUE; Number const product = a * b; // Exact reference in BigInt. - BigInt const exactProduct = BigInt(aValue) * BigInt(bValue); + BigInt const exactProduct = BigInt(kA_VALUE) * BigInt(kB_VALUE); // What Number actually stored. BigInt storedValue = BigInt(product.mantissa()); @@ -1608,8 +1612,8 @@ public: BigInt const signedDifference = storedValue - exactProduct; log << "\n" - << " a = " << fmt(BigInt(aValue)) << "\n" - << " b = " << fmt(BigInt(bValue)) << "\n" + << " a = " << fmt(BigInt(kA_VALUE)) << "\n" + << " b = " << fmt(BigInt(kB_VALUE)) << "\n" << " exact a*b = " << fmt(exactProduct) << "\n" << " stored = " << fmt(storedValue) << "\n" << " stored - exact = " << fmt(signedDifference) << "\n" From 1b6047afe18e4c7625485c39c9dcfa6497fad518 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Thu, 7 May 2026 13:27:03 -0400 Subject: [PATCH 4/6] More clang-tidy changes: AMMExtended_test member and Number_test includes --- src/test/app/AMMExtended_test.cpp | 2 +- src/test/basics/Number_test.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index 9b80ff2414..83327f94cb 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -68,7 +68,7 @@ namespace xrpl::test { class AMMExtended_test : public jtx::AMMTest { // Use small Number mantissas for the life of this test. - NumberMantissaScaleGuard const sg{xrpl::MantissaRange::MantissaScale::Small}; + NumberMantissaScaleGuard const sg_{xrpl::MantissaRange::MantissaScale::Small}; // For now, just disable SAV entirely, which locks in the small Number // mantissas diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 351edcce51..893bff3154 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -6,7 +6,7 @@ #include #include -#include +#include #include #include From 22d2703ce875cc7376cf6adc059960f870eb1d6e Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Thu, 7 May 2026 16:50:43 -0400 Subject: [PATCH 5/6] What is it going to take to get clang-tidy to shut up? --- src/libxrpl/protocol/Rules.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libxrpl/protocol/Rules.cpp b/src/libxrpl/protocol/Rules.cpp index 8274947e83..3503dc8d67 100644 --- a/src/libxrpl/protocol/Rules.cpp +++ b/src/libxrpl/protocol/Rules.cpp @@ -50,9 +50,9 @@ setCurrentTransactionRules(std::optional r) bool const enableVaultNumbers = enableCuspRoundingFix || (r->enabled(featureSingleAssetVault) || r->enabled(featureLendingProtocol)); Number::setMantissaScale( - enableCuspRoundingFix ? MantissaRange::MantissaScale::Large - : enableVaultNumbers ? MantissaRange::MantissaScale::LargeLegacy - : MantissaRange::MantissaScale::Small); + enableCuspRoundingFix ? MantissaRange::MantissaScale::Large + : (enableVaultNumbers ? MantissaRange::MantissaScale::LargeLegacy + : MantissaRange::MantissaScale::Small)); *getCurrentTransactionRulesRef() = std::move(r); } From cd0f49a00362c129a7ea06b3988b8fd1e89a7a0d Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Thu, 7 May 2026 17:05:10 -0400 Subject: [PATCH 6/6] Address more nitpicky AI comments --- include/xrpl/basics/Number.h | 6 ++---- src/libxrpl/protocol/Rules.cpp | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index ebdb53aa83..5026dd112a 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -121,8 +121,7 @@ private: case MantissaScale::Large: return 1'000'000'000'000'000'000ULL; default: - // Since this can never be called outside a non-constexpr - // context, this throw assures that the build fails if an + // 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"); } @@ -139,8 +138,7 @@ private: 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 + // 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"); } diff --git a/src/libxrpl/protocol/Rules.cpp b/src/libxrpl/protocol/Rules.cpp index 3503dc8d67..bffe3602d0 100644 --- a/src/libxrpl/protocol/Rules.cpp +++ b/src/libxrpl/protocol/Rules.cpp @@ -42,7 +42,7 @@ setCurrentTransactionRules(std::optional r) // The improved accuracy that will come from enabling large // mantissas is a goal separate from SAV and LP. It was originally // only tied to those two amendments to avoid needing a new - // amendment of it's own, and because they require that behavior. + // amendment of its own, and because they require that behavior. // Because fixCleanup3_2_0 fixes a separate bug related to the large // mantissas, that can take precedence and activate the large // mantissas even in the absence of the other two amendments.