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"