From d9c63cb9bc15c9dcbe5926e2f06c5be55a7746ca Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 9 Jun 2026 16:43:24 -0400 Subject: [PATCH] Update to use a new amendment, since this PR will not be part of 3.2.0 - This requires creating yet another MantissaScale, and CuspRoundingFix option. --- include/xrpl/basics/Number.h | 30 ++++++++++------ include/xrpl/protocol/detail/features.macro | 2 ++ src/libxrpl/basics/Number.cpp | 38 ++++++++++++++------- src/libxrpl/protocol/Rules.cpp | 35 +++++++++++-------- src/libxrpl/protocol/STNumber.cpp | 5 +-- src/test/app/Invariants_test.cpp | 7 ++-- src/test/basics/Number_test.cpp | 21 ++++++------ 7 files changed, 83 insertions(+), 55 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 61b63fe234..7e2c8b06c9 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -130,13 +130,16 @@ struct MantissaRange final Small, // LargeLegacy can be removed when fixCleanup3_2_0 is retired LargeLegacy, - Large, + // Large3_2_0 can be removed when fixCleanup3_3_0 is retired + Large3_2_0, + LargeNew, // TODO: Convert this back to Large after conversion is done }; // This entire enum can be removed when fixCleanup3_2_0 is retired - enum class CuspRoundingFix : bool { - Disabled = false, - Enabled = true, + enum class CuspRoundingFix : std::uint8_t { + Disabled = 0, + Enabled3_2_0 = 1, + EnabledNew = 2, // TODO: Convert this back to Enabled after conversion is done }; explicit constexpr MantissaRange(MantissaScale sc) : scale(sc) @@ -164,7 +167,8 @@ private: case MantissaScale::Small: return 15; case MantissaScale::LargeLegacy: - case MantissaScale::Large: + case MantissaScale::Large3_2_0: + case MantissaScale::LargeNew: return 18; // LCOV_EXCL_START default: @@ -193,8 +197,10 @@ private: case MantissaScale::Small: case MantissaScale::LargeLegacy: return CuspRoundingFix::Disabled; - case MantissaScale::Large: - return CuspRoundingFix::Enabled; + case MantissaScale::Large3_2_0: + return CuspRoundingFix::Enabled3_2_0; + case MantissaScale::LargeNew: + return CuspRoundingFix::EnabledNew; default: // If called in a constexpr context, this throw assures that the build fails if an // invalid scale is used. @@ -867,11 +873,13 @@ 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 "LargeLegacy"; + case MantissaRange::MantissaScale::Large3_2_0: + return "Large320"; + case MantissaRange::MantissaScale::LargeNew: + return "Large"; default: throw std::runtime_error("Bad scale"); } diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index c99c1e5ce8..f83442926d 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -15,6 +15,8 @@ // Add new amendments to the top of this list. // Keep it sorted in reverse chronological order. +// The name "NumberStuff" is a placeholder +XRPL_FIX (NumberStuff, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FIX (Cleanup3_2_0, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(MPTokensV2, Supported::No, VoteBehavior::DefaultNo) XRPL_FIX (Cleanup3_1_3, Supported::Yes, VoteBehavior::DefaultYes) diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 9386b411d0..8253e6c6ee 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -31,7 +31,7 @@ namespace xrpl { thread_local Number::RoundingMode Number::mode = Number::RoundingMode::ToNearest; thread_local std::reference_wrapper Number::kRange = - MantissaRange::getMantissaRange(MantissaRange::MantissaScale::Large); + MantissaRange::getMantissaRange(MantissaRange::MantissaScale::LargeNew); std::set const& MantissaRange::getAllScales() @@ -39,7 +39,8 @@ MantissaRange::getAllScales() static std::set const kScales = { MantissaRange::MantissaScale::Small, MantissaRange::MantissaScale::LargeLegacy, - MantissaRange::MantissaScale::Large, + MantissaRange::MantissaScale::Large3_2_0, + MantissaRange::MantissaScale::LargeNew, }; return kScales; } @@ -80,14 +81,25 @@ MantissaRange::getRanges() } { [[maybe_unused]] - constexpr static MantissaRange kRange{MantissaRange::MantissaScale::Large}; + constexpr static MantissaRange kRange{MantissaRange::MantissaScale::Large3_2_0}; static_assert(isPowerOfTen(kRange.min)); static_assert(kRange.min == 1'000'000'000'000'000'000ULL); static_assert(kRange.max == rep(9'999'999'999'999'999'999ULL)); static_assert(kRange.log == 18); static_assert(kRange.min < Number::kMaxRep); static_assert(kRange.max > Number::kMaxRep); - static_assert(kRange.cuspRoundingFix == CuspRoundingFix::Enabled); + static_assert(kRange.cuspRoundingFix == CuspRoundingFix::Enabled3_2_0); + } + { + [[maybe_unused]] + constexpr static MantissaRange kRange{MantissaRange::MantissaScale::LargeNew}; + static_assert(isPowerOfTen(kRange.min)); + static_assert(kRange.min == 1'000'000'000'000'000'000ULL); + static_assert(kRange.max == rep(9'999'999'999'999'999'999ULL)); + static_assert(kRange.log == 18); + static_assert(kRange.min < Number::kMaxRep); + static_assert(kRange.max > Number::kMaxRep); + static_assert(kRange.cuspRoundingFix == CuspRoundingFix::EnabledNew); } return map; }(); @@ -225,7 +237,7 @@ public: doDropDigit(T& mantissa, int& exponent) noexcept; enum class Round { - // The result is exact. No rounding is needed. Only used if cuspRoundingFix is enabled. + // The result is exact. No rounding is needed. Only used if cuspRoundingFix is EnabledNew. Exact = -2, // Round down. Since we use integer math, that usually means no change is needed. // Exceptions are for when the result is between kMaxRap and kMaxRepUp (round to kMaxRep), @@ -234,7 +246,8 @@ public: Down = -1, // The result was exactly half-way between two integers. This will round to even. Even = 0, - // Round up. Always adds 1 (or subtracts 1 in some cases if cuspRoundingFix is not enabled) + // Round up. Always adds 1 (or subtracts 1 in some cases if cuspRoundingFix is not + // EnabledNew) Up = 1, }; @@ -350,7 +363,7 @@ Number::Guard::round() const noexcept { auto mode = Number::getround(); - if (cuspRoundingFix != MantissaRange::CuspRoundingFix::Disabled && empty()) + if (cuspRoundingFix >= MantissaRange::CuspRoundingFix::EnabledNew && empty()) { // No remainder return Round::Exact; @@ -419,7 +432,7 @@ Number::Guard::doRoundUp(bool& negative, T& mantissa, int& exponent, std::string auto const safeToIncrement = [this](auto const& mantissa) { return mantissa < maxMantissa && mantissa < kMaxRep; }; - if (cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled) + if (cuspRoundingFix != MantissaRange::CuspRoundingFix::Disabled) { // Ensure mantissa after incrementing fits within both the // min/maxMantissa range and is a valid "rep". @@ -469,7 +482,7 @@ void Number::Guard::doRoundDown(bool& negative, T& mantissa, int& exponent) { auto r = round(); - if (cuspRoundingFix != MantissaRange::CuspRoundingFix::Disabled) + if (cuspRoundingFix >= MantissaRange::CuspRoundingFix::EnabledNew) { // If there was any remainder, subtract 1 from the result. This is sufficient to get the // best rounding. @@ -801,7 +814,7 @@ Number::operator+=(Number const& y) xe = ye; xn = yn; } - if (cuspRoundingFix != MantissaRange::CuspRoundingFix::Disabled) + if (cuspRoundingFix >= MantissaRange::CuspRoundingFix::EnabledNew) { // Grow xm/xe and pull digits out of the Guard until it's a little bit larger than // maxMantissa, so that normalize will have enough information to make an accurate @@ -836,6 +849,7 @@ Number::operator+=(Number const& y) negative_ = xn; mantissa_ = static_cast(xm); exponent_ = xe; + XRPL_ASSERT(isnormal(), "xrpl::Number::operator+= : result is normal"); return *this; } @@ -971,7 +985,7 @@ Number::operator/=(Number const& y) // This is equivalent to if we had used an initial factor of 10^22, // a couple digits more than we actually need. // - // Stage 3: If there is still a remainder, and the CuspRoundingFix + // Stage 3: If there is still a remainder, and the cuspRoundingFix // is enabled, pass a flag indicating such to doNormalize. The Guard // in doNormalize will treat that flag as if non-zero digits had // been dropped from the mantissa when shrinking it into range. @@ -1055,7 +1069,7 @@ Number::operator/=(Number const& y) // rounding fix is enabled, flag if there is still // a remainder from stage 2. bool const useTrailingRemainder = - cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled; + cuspRoundingFix != MantissaRange::CuspRoundingFix::Disabled; if (useTrailingRemainder) { dropped = partialNumerator % dm != 0; diff --git a/src/libxrpl/protocol/Rules.cpp b/src/libxrpl/protocol/Rules.cpp index 08a95145eb..06f50423b8 100644 --- a/src/libxrpl/protocol/Rules.cpp +++ b/src/libxrpl/protocol/Rules.cpp @@ -40,22 +40,29 @@ setCurrentTransactionRules(std::optional r) // Push the appropriate setting, instead of having the class pull every time // the value is needed. That could get expensive fast. - // If any new conditions with new amendments are added, those amendments must also be added to - // useRulesGuards. - bool const enableVaultNumbers = - !r || (r->enabled(featureSingleAssetVault) || r->enabled(featureLendingProtocol)); - bool const enableCuspRoundingFix = !r || r->enabled(fixCleanup3_2_0); - XRPL_ASSERT( - !r || useRulesGuards(*r) == (enableCuspRoundingFix || enableVaultNumbers), - "setCurrentTransactionRules : rule decisions match"); - // Declare the range this way to keep clang-tidy from complaining - auto const range = [enableCuspRoundingFix, enableVaultNumbers]() { + auto const range = [&r]() { + // If any new conditions with new amendments are added, those amendments must also be added + // to useRulesGuards. + bool const enableVaultNumbers = + !r || (r->enabled(featureSingleAssetVault) || r->enabled(featureLendingProtocol)); + bool const enableCuspRounding3_2_0 = !r || r->enabled(fixCleanup3_2_0); + bool const enableCuspRounding3_3_0 = !r || r->enabled(fixNumberStuff); + XRPL_ASSERT( + !r || + useRulesGuards(*r) == + (enableVaultNumbers || enableCuspRounding3_2_0 || enableCuspRounding3_3_0), + "setCurrentTransactionRules : rule decisions match"); + if (enableVaultNumbers) { - if (enableCuspRoundingFix) + if (enableCuspRounding3_3_0) { - return MantissaRange::MantissaScale::Large; + return MantissaRange::MantissaScale::LargeNew; + } + if (enableCuspRounding3_2_0) + { + return MantissaRange::MantissaScale::Large3_2_0; } return MantissaRange::MantissaScale::LargeLegacy; } @@ -76,8 +83,8 @@ useRulesGuards(Rules const& rules) // As soon as any one of these amendments is retired, this whole function can be removed, along // with createGuards, and any other callers, and the first set of guards can be created directly // at the call site, without using optional. - return rules.enabled(fixCleanup3_2_0) || rules.enabled(featureSingleAssetVault) || - rules.enabled(featureLendingProtocol); + return rules.enabled(featureSingleAssetVault) || rules.enabled(featureLendingProtocol) || + rules.enabled(fixCleanup3_2_0) || rules.enabled(fixNumberStuff); } void diff --git a/src/libxrpl/protocol/STNumber.cpp b/src/libxrpl/protocol/STNumber.cpp index 8ef7b9760f..11cde05da2 100644 --- a/src/libxrpl/protocol/STNumber.cpp +++ b/src/libxrpl/protocol/STNumber.cpp @@ -88,7 +88,6 @@ STNumber::add(Serializer& s) const } else { -#if !NDEBUG // There are circumstances where an already-rounded Number is // serialized without being touched by a transactor, and thus // without an asset. We can't know if it's rounded, because it could @@ -96,11 +95,9 @@ STNumber::add(Serializer& s) const // Json. Regardless, the only time we should be serializing an // STNumber is when the scale is large. XRPL_ASSERT_PARTS( - Number::getMantissaScale() == MantissaRange::MantissaScale::LargeLegacy || - Number::getMantissaScale() == MantissaRange::MantissaScale::Large, + Number::getMantissaScale() != MantissaRange::MantissaScale::Small, "xrpl::STNumber::add", "STNumber only used with large mantissa scale"); -#endif } } diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 3654036869..ebc499229d 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -4767,11 +4767,10 @@ class Invariants_test : public beast::unit_test::Suite std::vector values; }; - for (auto const mantissaScale : { - MantissaRange::MantissaScale::LargeLegacy, - MantissaRange::MantissaScale::Large, - }) + for (auto const mantissaScale : MantissaRange::getAllScales()) { + if (mantissaScale == MantissaRange::MantissaScale::Small) + continue; NumberMantissaScaleGuard const g{mantissaScale}; auto makeDelta = [&vaultAsset](Number const& n) -> ValidVault::DeltaInfo { diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index d2e9ac2638..ad55fd4b94 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -1397,8 +1397,7 @@ public: "9223372036854775e3"); } break; - case MantissaRange::MantissaScale::LargeLegacy: - case MantissaRange::MantissaScale::Large: + default: // Test the edges // ((exponent < -(28)) || (exponent > -(8))))) test(Number::min(), "1e-32750"); @@ -1436,9 +1435,6 @@ public: test( -(Number{std::numeric_limits::max(), 0} + 1), "-9223372036854775810"); - break; - default: - BEAST_EXPECT(false); } } @@ -1721,7 +1717,8 @@ public: switch (scale) { - case MantissaRange::MantissaScale::Large: + case MantissaRange::MantissaScale::Large3_2_0: + case MantissaRange::MantissaScale::LargeNew: BEAST_EXPECT(signedDifference >= 0); BEAST_EXPECT(signedDifference < pow10(product.exponent())); BEAST_EXPECT( @@ -1803,7 +1800,8 @@ public: // Upward invariant: stored >= exact. Bug: stored < exact. switch (scale) { - case MantissaRange::MantissaScale::Large: + case MantissaRange::MantissaScale::Large3_2_0: + case MantissaRange::MantissaScale::LargeNew: BEAST_EXPECT(stored >= exact); BEAST_EXPECT(diff < pow10(quotient.exponent())); break; @@ -1853,7 +1851,8 @@ public: // invariant: stored <= exact. Bug: stored > exact. switch (scale) { - case MantissaRange::MantissaScale::Large: + case MantissaRange::MantissaScale::Large3_2_0: + case MantissaRange::MantissaScale::LargeNew: BEAST_EXPECT(stored <= exact); BEAST_EXPECT(diff > -pow10(quotient.exponent())); break; @@ -1910,7 +1909,8 @@ public: // invariant: stored >= exact. Bug: stored < exact. switch (scale) { - case MantissaRange::MantissaScale::Large: + case MantissaRange::MantissaScale::Large3_2_0: + case MantissaRange::MantissaScale::LargeNew: BEAST_EXPECT(stored >= exact); BEAST_EXPECT(diff < pow10(quotient.exponent())); break; @@ -2012,7 +2012,8 @@ public: switch (scale) { case MantissaRange::MantissaScale::Small: - case MantissaRange::MantissaScale::LargeLegacy: { + case MantissaRange::MantissaScale::LargeLegacy: + case MantissaRange::MantissaScale::Large3_2_0: { // Without the fix, all the results but one round up if (r == Number::RoundingMode::Downward) {