diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 39cb3b3a18..b5b6f8dade 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -8,8 +8,10 @@ #include #include #include +#include #include #include +#include #ifdef _MSC_VER #include @@ -69,11 +71,11 @@ isPowerOfTen(T value) * * The mantissa is in the range [min, max], where * - * The MantissaScale enum indicates whether the range is "small" or - * "large". This intentionally prevents the creation of any - * MantissaRanges representing other values. + * The MantissaScale enum indicates properties of the range: size, and + * some behavioral options. This intentionally prevents the creation of + * any MantissaRanges representing other values. * - * 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. * @@ -84,21 +86,41 @@ 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 - * by an STAmount - IOUs, XRP, and MPTs. It has a min value of 2^63/10+1 - * (truncated), and a max value of 2^63-1. + * The "Large" scales are intended to represent all values that can be represented + * by an STAmount - IOUs, XRP, and MPTs. + * + * They have a min value of 2^63/10+1 (truncated), and a max value of 2^63-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 { 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) - : max(getMax(scale)), internalMin(getInternalMin(scale, min)), scale(scale) + : max(getMax(scale)) + , internalMin(getInternalMin(scale, min)) + , cuspRoundingFixEnabled(isCuspFixEnabled(scale)) + , scale(scale) { // Keep the error messages terse. Since this is constexpr, if any of these throw, it won't // compile, so there's no real need to worry about runtime exceptions here. @@ -123,16 +145,23 @@ struct MantissaRange MantissaRange& operator=(MantissaRange&&) = delete; - rep max; - rep min{computeMin(max)}; + rep const max; + rep const min{computeMin(max)}; /* Used to determine if mantissas are in range, but have fewer digits than max. * * Unlike min, internalMin is always an exact power of 10, so a mantissa in the internal * representation will always have a consistent number of digits. */ - rep internalMin; - int log{computeLog(min)}; - MantissaScale scale; + rep const internalMin; + int const log{computeLog(min)}; + CuspRoundingFix const cuspRoundingFixEnabled; + MantissaScale const scale; + + static MantissaRange const& + getMantissaRange(MantissaScale scale); + + static std::set const& + getAllScales(); private: static constexpr rep @@ -142,11 +171,11 @@ private: { case MantissaScale::Small: return 9'999'999'999'999'999ULL; + case MantissaScale::LargeLegacy: case MantissaScale::Large: return std::numeric_limits::max(); 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"); } @@ -163,6 +192,7 @@ private: { switch (scale) { + case MantissaScale::LargeLegacy: case MantissaScale::Large: return 1'000'000'000'000'000'000ULL; default: @@ -178,6 +208,26 @@ private: auto const estimate = logTenEstimate(min); return estimate.first + (estimate.second == 1 ? 0 : 1); } + + static constexpr CuspRoundingFix + isCuspFixEnabled(MantissaScale scale) + { + switch (scale) + { + case MantissaScale::Small: + case MantissaScale::LargeLegacy: + return CuspRoundingFix::Disabled; + case MantissaScale::Large: + return CuspRoundingFix::Enabled; + default: + // If called in a constexpr context, this throw assures that the build fails if an + // invalid scale is used. + throw std::runtime_error("Unknown mantissa scale"); + } + } + + static std::unordered_map const& + getRanges(); }; // Like std::integral, but only 64-bit integral types. @@ -520,48 +570,34 @@ 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 < + auto MinMantissa, + auto MaxMantissa, + Integral64 T = std::decay_t, + Integral64 TMax = std::decay_t> + [[nodiscard]] + std::pair + normalizeToRange() const; + +private: + /** May use ranges that don't fit the restrictions of the "real" + * normalizeToRange(). + * + */ template [[nodiscard]] std::pair - normalizeToRange(T minMantissa, T maxMantissa) const; + normalizeToRangeImpl(T minMantissa, T maxMantissa, MantissaRange::CuspRoundingFix fix) const; + + // Number_test needs to use normalizeToRangeImpl + friend class Number_test; -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.internalMin == kSMALL_RANGE.min); - static_assert(kSMALL_RANGE.log == 15); - constexpr static MantissaRange kLARGE_RANGE{MantissaRange::MantissaScale::Large}; - static_assert(!isPowerOfTen(kLARGE_RANGE.min)); - static_assert(kLARGE_RANGE.min == 922'337'203'685'477'581ULL); - static_assert(kLARGE_RANGE.max == internalrep(9'223'372'036'854'775'807ULL)); - static_assert(kLARGE_RANGE.max == std::numeric_limits::max()); - static_assert(kLARGE_RANGE.internalMin == 1'000'000'000'000'000'000ULL); - static_assert(kLARGE_RANGE.log == 18); - // There are 2 values that will not fit in kLARGE_RANGE without some extra - // work - // * 9223372036854775808 - // * 9223372036854775809 - // They both end up < min, but with a leftover. If they round up, everything - // will be fine. If they don't, we'll need to bring them up into range. - // Guard::bringIntoRange handles this situation. - // The range for the mantissa when normalized. // Use reference_wrapper to avoid making copies, and prevent accidentally // changing the values inside the range. @@ -578,9 +614,6 @@ private: void normalize(MantissaRange const& range); - void - normalize(); - /** Normalize Number components to an arbitrary range. * * min/maxMantissa are parameters because this function is used by both @@ -594,7 +627,8 @@ private: T& mantissa, int& exponent, internalrep const& minMantissa, - internalrep const& maxMantissa); + internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled); template friend void @@ -603,7 +637,8 @@ private: T& mantissa, int& exponent, MantissaRange::rep const& minMantissa, - MantissaRange::rep const& maxMantissa); + MantissaRange::rep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled); [[nodiscard]] bool @@ -671,7 +706,8 @@ private: class Guard; public: - constexpr static internalrep kLARGEST_MANTISSA = kLARGE_RANGE.max; + constexpr static internalrep kLARGEST_MANTISSA = + MantissaRange{MantissaRange::MantissaScale::Large}.max; }; inline constexpr Number::Number( @@ -845,9 +881,34 @@ Number::isnormal() const noexcept return isnormal(kRANGE); } -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 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); + + // 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. + return normalizeToRangeImpl(kMIN, kMAX, MantissaRange::CuspRoundingFix::Disabled); +} + +/** Only intended to be used in tests + * + * May use ranges that don't fit the restrictions of the "real" + * normalizeToRange(). + * + */ +template +[[nodiscard]] +std::pair +Number::normalizeToRangeImpl(T minMantissa, T maxMantissa, MantissaRange::CuspRoundingFix fix) const { bool negative = mantissa_ < 0; internalrep mantissa = externalToInternal(mantissa_); @@ -866,7 +927,7 @@ Number::normalizeToRange(T minMantissa, T maxMantissa) const "Number::normalizeToRange: Number is negative for " "unsigned range."); } - Number::normalize(negative, mantissa, exponent, minMantissa, maxMantissa); + Number::normalize(negative, mantissa, exponent, minMantissa, maxMantissa, fix); // Cast mantissa to signed type first (if T is a signed type) to avoid // unsigned integer overflow when multiplying by negative sign @@ -920,6 +981,8 @@ to_string(MantissaRange::MantissaScale const& scale) { case MantissaRange::MantissaScale::Small: return "small"; + case MantissaRange::MantissaScale::LargeLegacy: + return "largeLegacy"; case MantissaRange::MantissaScale::Large: return "large"; default: diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index 2c7e6b5127..114e762e92 100644 --- a/include/xrpl/protocol/STAmount.h +++ b/include/xrpl/protocol/STAmount.h @@ -542,7 +542,7 @@ STAmount::fromNumber(A const& a, Number const& number) XRPL_ASSERT_PARTS( working.signum() >= 0, "xrpl::STAmount::fromNumber", "non-negative Number to normalize"); - 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 30e9ad3a2a..d3ce45b3bf 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 @@ -25,7 +27,82 @@ using int128_t = xrpl::detail::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 kSCALES = { + MantissaRange::MantissaScale::Small, + MantissaRange::MantissaScale::LargeLegacy, + MantissaRange::MantissaScale::Large, + }; + return kSCALES; +} + +std::unordered_map const& +MantissaRange::getRanges() +{ + static auto const kMAP = []() { + std::unordered_map map; + for (auto const scale : getAllScales()) + { + map.emplace(scale, scale); + } + + // Use these constexpr declarations to do static_asserts to verify the MantissaRanges are + // created correctly, but nothing else. + { + [[maybe_unused]] + constexpr static MantissaRange kRANGE{MantissaRange::MantissaScale::Small}; + static_assert(isPowerOfTen(kRANGE.min)); + static_assert(isPowerOfTen(kRANGE.internalMin)); + static_assert(kRANGE.min == 1'000'000'000'000'000LL); + static_assert(kRANGE.internalMin == kRANGE.min); + static_assert(kRANGE.max == 9'999'999'999'999'999LL); + static_assert(kRANGE.log == 15); + static_assert(kRANGE.min < Number::kLARGEST_MANTISSA); + static_assert(kRANGE.max < Number::kLARGEST_MANTISSA); + static_assert(kRANGE.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); + } + { + [[maybe_unused]] + constexpr static MantissaRange kRANGE{MantissaRange::MantissaScale::LargeLegacy}; + static_assert(!isPowerOfTen(kRANGE.min)); + static_assert(isPowerOfTen(kRANGE.internalMin)); + static_assert(kRANGE.min == 922'337'203'685'477'581ULL); + static_assert(kRANGE.internalMin == 1'000'000'000'000'000'000ULL); + static_assert(kRANGE.max == rep(9'223'372'036'854'775'807ULL)); + static_assert(kRANGE.log == 18); + static_assert(kRANGE.min < Number::kLARGEST_MANTISSA); + static_assert(kRANGE.max == Number::kLARGEST_MANTISSA); + static_assert(kRANGE.cuspRoundingFixEnabled == CuspRoundingFix::Disabled); + } + { + [[maybe_unused]] + constexpr static MantissaRange kRANGE{MantissaRange::MantissaScale::Large}; + static_assert(!isPowerOfTen(kRANGE.min)); + static_assert(isPowerOfTen(kRANGE.internalMin)); + static_assert(kRANGE.min == 922'337'203'685'477'581ULL); + static_assert(kRANGE.internalMin == 1'000'000'000'000'000'000ULL); + static_assert(kRANGE.max == rep(9'223'372'036'854'775'807ULL)); + static_assert(kRANGE.log == 18); + static_assert(kRANGE.min < Number::kLARGEST_MANTISSA); + static_assert(kRANGE.max == Number::kLARGEST_MANTISSA); + static_assert(kRANGE.cuspRoundingFixEnabled == CuspRoundingFix::Enabled); + } + return map; + }(); + + return kMAP; +} + +MantissaRange const& +MantissaRange::getMantissaRange(MantissaScale scale) +{ + return getRanges().at(scale); +} Number::RoundingMode Number::getround() @@ -48,10 +125,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 @@ -86,6 +190,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. @@ -101,6 +217,7 @@ public: int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, std::string_view location); // Modify the result to the correctly rounded value @@ -162,6 +279,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 @@ -234,18 +372,58 @@ Number::Guard::doRoundUp( int& exponent, internalrep const& minMantissa, internalrep const& maxMantissa, + MantissaRange::CuspRoundingFix cuspRoundingFixEnabled, std::string_view 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) + 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 < kLARGEST_MANTISSA) + { + // 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 kLARGEST_MANTISSA, so adding 1 will + // have no change of bringing it back over. + doDropDigit(mantissa, exponent); + XRPL_ASSERT_PARTS( + mantissa < maxMantissa, + "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 > kLARGEST_MANTISSA) + { + // Don't use doDropDigit here + mantissa /= 10; + ++exponent; + } } } bringIntoRange(negative, mantissa, exponent, minMantissa); @@ -286,9 +464,9 @@ Number::Guard::doRound(internalrep& drops, std::string_view location) const { static_assert(sizeof(internalrep) == sizeof(rep)); // This should be impossible, because it's impossible to represent - // "largestMantissa + 0.6" in Number, regardless of the scale. There aren't - // enough digits available. You'd either get a mantissa of "largestMantissa" - // or "largestMantissa / 10 + 1", neither of which will round up when + // "kLARGEST_MANTISSA + 0.6" in Number, regardless of the scale. There aren't + // enough digits available. You'd either get a mantissa of "kLARGEST_MANTISSA" + // or "kLARGEST_MANTISSA / 10 + 1", neither of which will round up when // converting to rep, though the latter might overflow _before_ // rounding. Throw(std::string(location)); // LCOV_EXCL_LINE @@ -434,31 +612,10 @@ Number::fromInternal(bool negative, Rep mantissa, int exponent) fromInternal(negative, mantissa, exponent, pRange); } -constexpr Number -Number::oneSmall() -{ - return Number{ - false, Number::kSMALL_RANGE.internalMin, -Number::kSMALL_RANGE.log, Number::Unchecked{}}; -}; - -constexpr Number kONE_SML = Number::oneSmall(); - -constexpr Number -Number::oneLarge() -{ - return Number{ - false, Number::kLARGE_RANGE.internalMin, -Number::kLARGE_RANGE.log, Number::Unchecked{}}; -}; - -constexpr Number kONE_LRG = Number::oneLarge(); - Number Number::one(MantissaRange const& range) { - if (&range == &kSMALL_RANGE) - return kONE_SML; - XRPL_ASSERT(&kRANGE.get() == &kLARGE_RANGE, "Number::one() : valid range"); - return kONE_LRG; + return Number{false, range.min, -range.log, Number::Unchecked{}}; } Number @@ -475,7 +632,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; @@ -503,9 +661,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 == 0)) { @@ -516,7 +672,14 @@ doNormalize( XRPL_ASSERT_PARTS(m <= maxMantissa, "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, @@ -535,9 +698,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 <> @@ -547,9 +711,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 <> @@ -559,9 +724,10 @@ 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 @@ -569,17 +735,11 @@ Number::normalize(MantissaRange const& range) { auto [negative, mantissa, exponent] = toInternal(range); - normalize(negative, mantissa, exponent, range.min, range.max); + normalize(negative, mantissa, exponent, range.min, range.max, range.cuspRoundingFixEnabled); fromInternal(negative, mantissa, exponent, &range); } -void -Number::normalize() -{ - normalize(kRANGE); -} - // Copy the number, but set a new exponent. Because the mantissa doesn't change, // the result will be "mostly" normalized, but the exponent could go out of // range. @@ -605,7 +765,7 @@ Number::shiftExponent(int exponentDelta) const Number::Number(bool negative, internalrep mantissa, int exponent, Normalized) { auto const& range = kRANGE.get(); - normalize(negative, mantissa, exponent, range.min, range.max); + normalize(negative, mantissa, exponent, range.min, range.max, range.cuspRoundingFixEnabled); fromInternal(negative, mantissa, exponent, &range); } @@ -648,9 +808,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) @@ -659,25 +817,29 @@ Number::operator+=(Number const& y) g.setNegative(); do { - g.push(ym % 10); - ym /= 10; - ++ye; + g.doDropDigit(ym, ye); } while (xe > ye); } auto const& minMantissa = range.min; auto const& maxMantissa = range.max; + auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; if (xn == yn) { xm += ym; if (xm > maxMantissa) { - 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 { @@ -700,39 +862,11 @@ Number::operator+=(Number const& y) g.doRoundDown(xn, xm, xe, minMantissa); } - normalize(xn, xm, xe, minMantissa, maxMantissa); + normalize(xn, xm, xe, minMantissa, maxMantissa, cuspRoundingFixEnabled); fromInternal(xn, xm, xe, &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) { @@ -767,15 +901,13 @@ Number::operator*=(Number const& y) auto const& minMantissa = range.min; auto const& maxMantissa = range.max; + auto const cuspRoundingFixEnabled = range.cuspRoundingFixEnabled; while (zm > maxMantissa) { - // 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( @@ -784,9 +916,10 @@ Number::operator*=(Number const& y) xe, minMantissa, maxMantissa, + cuspRoundingFixEnabled, "Number::multiplication overflow : exponent is " + std::to_string(xe)); - normalize(zn, xm, xe, minMantissa, maxMantissa); + normalize(zn, xm, xe, minMantissa, maxMantissa, cuspRoundingFixEnabled); fromInternal(zn, xm, xe, &range); return *this; } @@ -816,6 +949,7 @@ Number::operator/=(Number const& y) 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 @@ -823,8 +957,6 @@ Number::operator/=(Number const& y) // log(2^128,10) ~ 38.5 // kLARGE_RANGE.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 = range.scale == 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"); @@ -874,7 +1006,7 @@ Number::operator/=(Number const& y) ze -= 3; } } - normalize(zn, zm, ze, minMantissa, maxMantissa); + normalize(zn, zm, ze, minMantissa, maxMantissa, cuspRoundingFixEnabled); fromInternal(zn, zm, ze, &range); XRPL_ASSERT_PARTS(isnormal(range), "xrpl::Number::operator/=", "result is normalized"); @@ -885,6 +1017,7 @@ Number:: operator rep() const { auto const m = mantissa(); + // drops will always be non-negative internalrep drops = externalToInternal(m); if (drops == 0) @@ -897,14 +1030,13 @@ operator rep() const { g.setNegative(); } - for (; offset < 0; ++offset) + while (offset < 0) { - g.push(drops % 10); - drops /= 10; + g.doDropDigit(drops, offset); } for (; offset > 0; --offset) { - if (drops >= kLARGE_RANGE.min) + if (drops > kLARGEST_MANTISSA / 10) throw std::overflow_error("Number::operator rep() overflow"); drops *= 10; } @@ -930,7 +1062,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..bffe3602d0 100644 --- a/src/libxrpl/protocol/Rules.cpp +++ b/src/libxrpl/protocol/Rules.cpp @@ -38,11 +38,21 @@ 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)); + + // 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 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. + bool const enableCuspRoundingFix = !r || r->enabled(fixCleanup3_2_0); + bool const enableVaultNumbers = enableCuspRoundingFix || + (r->enabled(featureSingleAssetVault) || r->enabled(featureLendingProtocol)); Number::setMantissaScale( - enableLargeNumbers ? MantissaRange::MantissaScale::Large - : MantissaRange::MantissaScale::Small); + enableCuspRoundingFix ? MantissaRange::MantissaScale::Large + : (enableVaultNumbers ? 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..83327f94cb 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -65,10 +65,15 @@ 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}; + 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 @@ -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 356f0a9926..159f47360e 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 != 0 && count % 3 == 0) + out.insert(out.begin(), '_'); + out.insert(out.begin(), *it); + ++count; + } + return out; + } + public: void testZero() @@ -212,6 +232,16 @@ public: Number{false, 1'999'999'999'999'999'998ULL, 1, Number::Normalized{}}, }, }); + auto const cLargeLegacy = std::to_array({ + {Number{Number::kLARGEST_MANTISSA}, + Number{6, -1}, + Number{Number::kLARGEST_MANTISSA / 10, 1}}, + }); + auto const cLargeCorrected = std::to_array({ + {Number{Number::kLARGEST_MANTISSA}, + Number{6, -1}, + Number{(Number::kLARGEST_MANTISSA / 10) + 1, 1}}, + }); auto test = [this](auto const& c) { for (auto const& [x, y, z] : c) { @@ -228,6 +258,14 @@ public: else { test(cLarge); + if (scale == MantissaRange::MantissaScale::LargeLegacy) + { + test(cLargeLegacy); + } + else + { + test(cLargeCorrected); + } } { bool caught = false; @@ -1396,6 +1434,7 @@ public: "9223372036854775e3"); } break; + case MantissaRange::MantissaScale::LargeLegacy: case MantissaRange::MantissaScale::Large: // Test the edges // ((exponent < -(28)) || (exponent > -(8))))) @@ -1714,7 +1753,8 @@ public: auto const expectedMantissa, auto const expectedExponent, auto const line) { - auto const normalized = n.normalizeToRange(rangeMin, rangeMax); + auto const normalized = + n.normalizeToRangeImpl(rangeMin, rangeMax, MantissaRange::CuspRoundingFix::Enabled); BEAST_EXPECTS( normalized.first == expectedMantissa, "Number " + to_string(n) + " scaled to " + std::to_string(rangeMax) + @@ -1873,11 +1913,49 @@ public: } } + void + testUpwardRoundsDown() + { + testcase << "upward rounding produces a value below exact at kLARGEST_MANTISSA cusp"; + + NumberMantissaScaleGuard const mg{MantissaRange::MantissaScale::Large}; + NumberRoundModeGuard const rg{Number::RoundingMode::Upward}; + + 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 = kA_VALUE; + Number const b = kB_VALUE; + Number const product = a * b; + + // Exact reference in BigInt. + BigInt const exactProduct = BigInt(kA_VALUE) * BigInt(kB_VALUE); + + // 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(kA_VALUE)) << "\n" + << " b = " << fmt(BigInt(kB_VALUE)) << "\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(); @@ -1903,6 +1981,8 @@ public: testInt64(); testNormalizeToRange(); } + // 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,