From 398170ef3d97787a9ee8e67afdfee522548c404b Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 12 Nov 2025 19:15:14 -0500 Subject: [PATCH] Revert "Add optional enforcement of valid integer range to Number" This reverts commit 3cb447a4fe7866a533ed9a577ca392d3898cebe5. --- include/xrpl/basics/Number.h | 116 ++-------------------- src/libxrpl/basics/Number.cpp | 40 -------- src/test/basics/Number_test.cpp | 169 -------------------------------- 3 files changed, 7 insertions(+), 318 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 1ea43a1e93..080a807909 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -13,47 +13,16 @@ class Number; std::string to_string(Number const& amount); -template -constexpr bool -isPowerOfTen(T value) -{ - while (value >= 10 && value % 10 == 0) - value /= 10; - return value == 1; -} - class Number { -public: - /** Describes whether and how to enforce this number as an integer. - * - * - none: No enforcement. The value may vary freely. This is the default. - * - weak: If the absolute value is greater than maxIntValue, valid() will - * return false. - * - strong: Assignment operations will throw if the absolute value is above - * maxIntValue. - */ - enum EnforceInteger { none, weak, strong }; - -private: using rep = std::int64_t; rep mantissa_{0}; int exponent_{std::numeric_limits::lowest()}; - // The enforcement setting is not serialized, and does not affect the - // ledger. If not "none", the value is checked to be within the valid - // integer range. With "strong", the checks will be made as automatic as - // possible. - EnforceInteger enforceInteger_ = none; - public: // The range for the mantissa when normalized - constexpr static rep minMantissa = 1'000'000'000'000'000LL; - static_assert(isPowerOfTen(minMantissa)); - constexpr static rep maxMantissa = minMantissa * 10 - 1; - static_assert(maxMantissa == 9'999'999'999'999'999LL); - - constexpr static rep maxIntValue = minMantissa / 10; + constexpr static std::int64_t minMantissa = 1'000'000'000'000'000LL; + constexpr static std::int64_t maxMantissa = 9'999'999'999'999'999LL; // The range for the exponent when normalized constexpr static int minExponent = -32768; @@ -66,33 +35,15 @@ public: explicit constexpr Number() = default; - Number(rep mantissa, EnforceInteger enforce = none); - explicit Number(rep mantissa, int exponent, EnforceInteger enforce = none); + Number(rep mantissa); + explicit Number(rep mantissa, int exponent); explicit constexpr Number(rep mantissa, int exponent, unchecked) noexcept; - constexpr Number(Number const& other) = default; - constexpr Number(Number&& other) = default; - - ~Number() = default; - - constexpr Number& - operator=(Number const& other); - constexpr Number& - operator=(Number&& other); constexpr rep mantissa() const noexcept; constexpr int exponent() const noexcept; - void - setIntegerEnforcement(EnforceInteger enforce); - - EnforceInteger - integerEnforcement() const noexcept; - - bool - valid() const noexcept; - constexpr Number operator+() const noexcept; constexpr Number @@ -218,9 +169,6 @@ public: private: static thread_local rounding_mode mode_; - void - checkInteger(char const* what) const; - void normalize(); constexpr bool @@ -236,52 +184,16 @@ inline constexpr Number::Number(rep mantissa, int exponent, unchecked) noexcept { } -inline Number::Number(rep mantissa, int exponent, EnforceInteger enforce) - : mantissa_{mantissa}, exponent_{exponent}, enforceInteger_(enforce) +inline Number::Number(rep mantissa, int exponent) + : mantissa_{mantissa}, exponent_{exponent} { normalize(); - - checkInteger("Number::Number integer overflow"); } -inline Number::Number(rep mantissa, EnforceInteger enforce) - : Number{mantissa, 0, enforce} +inline Number::Number(rep mantissa) : Number{mantissa, 0} { } -constexpr Number& -Number::operator=(Number const& other) -{ - if (this != &other) - { - mantissa_ = other.mantissa_; - exponent_ = other.exponent_; - enforceInteger_ = std::max(enforceInteger_, other.enforceInteger_); - - checkInteger("Number::operator= integer overflow"); - } - - return *this; -} - -constexpr Number& -Number::operator=(Number&& other) -{ - if (this != &other) - { - // std::move doesn't really do anything for these types, but - // this is future-proof in case the types ever change - mantissa_ = std::move(other.mantissa_); - exponent_ = std::move(other.exponent_); - if (other.enforceInteger_ > enforceInteger_) - enforceInteger_ = std::move(other.enforceInteger_); - - checkInteger("Number::operator= integer overflow"); - } - - return *this; -} - inline constexpr Number::rep Number::mantissa() const noexcept { @@ -294,20 +206,6 @@ Number::exponent() const noexcept return exponent_; } -inline void -Number::setIntegerEnforcement(EnforceInteger enforce) -{ - enforceInteger_ = enforce; - - checkInteger("Number::setIntegerEnforcement integer overflow"); -} - -inline Number::EnforceInteger -Number::integerEnforcement() const noexcept -{ - return enforceInteger_; -} - inline constexpr Number Number::operator+() const noexcept { diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 2587af2e70..797ca83b67 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -222,13 +222,6 @@ Number::Guard::doRound(rep& drops) constexpr Number one{1000000000000000, -15, Number::unchecked{}}; -void -Number::checkInteger(char const* what) const -{ - if (enforceInteger_ == strong && !valid()) - throw std::overflow_error(what); -} - void Number::normalize() { @@ -270,27 +263,9 @@ Number::normalize() mantissa_ = -mantissa_; } -bool -Number::valid() const noexcept -{ - if (enforceInteger_ != none) - { - static Number const max = maxIntValue; - static Number const maxNeg = -maxIntValue; - // Avoid making a copy - if (mantissa_ < 0) - return *this >= maxNeg; - return *this <= max; - } - return true; -} - Number& Number::operator+=(Number const& y) { - // The strictest setting prevails - enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_); - if (y == Number{}) return *this; if (*this == Number{}) @@ -378,9 +353,6 @@ Number::operator+=(Number const& y) } mantissa_ = xm * xn; exponent_ = xe; - - checkInteger("Number::addition integer overflow"); - return *this; } @@ -415,9 +387,6 @@ divu10(uint128_t& u) Number& Number::operator*=(Number const& y) { - // The strictest setting prevails - enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_); - if (*this == Number{}) return *this; if (y == Number{}) @@ -469,18 +438,12 @@ Number::operator*=(Number const& y) XRPL_ASSERT( isnormal() || *this == Number{}, "ripple::Number::operator*=(Number) : result is normal"); - - checkInteger("Number::multiplication integer overflow"); - return *this; } Number& Number::operator/=(Number const& y) { - // The strictest setting prevails - enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_); - if (y == Number{}) throw std::overflow_error("Number: divide by 0"); if (*this == Number{}) @@ -508,9 +471,6 @@ Number::operator/=(Number const& y) exponent_ = ne - de - 17; mantissa_ *= np * dp; normalize(); - - checkInteger("Number::division integer overflow"); - return *this; } diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 968e621657..cec69fe05f 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -2,7 +2,6 @@ #include #include #include -#include #include #include @@ -835,173 +834,6 @@ public: } } - void - testInteger() - { - testcase("Integer enforcement"); - - using namespace std::string_literals; - - { - Number a{100}; - BEAST_EXPECT(a.integerEnforcement() == Number::none); - BEAST_EXPECT(a.valid()); - a = Number{1, 30}; - BEAST_EXPECT(a.valid()); - a = -100; - BEAST_EXPECT(a.valid()); - } - { - Number a{100, Number::weak}; - BEAST_EXPECT(a.integerEnforcement() == Number::weak); - BEAST_EXPECT(a.valid()); - a = Number{1, 30, Number::none}; - BEAST_EXPECT(!a.valid()); - a = -100; - BEAST_EXPECT(a.integerEnforcement() == Number::weak); - BEAST_EXPECT(a.valid()); - a = Number{5, Number::strong}; - BEAST_EXPECT(a.integerEnforcement() == Number::strong); - BEAST_EXPECT(a.valid()); - } - { - Number a{100, Number::strong}; - BEAST_EXPECT(a.integerEnforcement() == Number::strong); - BEAST_EXPECT(a.valid()); - try - { - a = Number{1, 30}; - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT(e.what() == "Number::operator= integer overflow"s); - // The throw is done _after_ the number is updated. - BEAST_EXPECT((a == Number{1, 30})); - } - BEAST_EXPECT(!a.valid()); - a = -100; - BEAST_EXPECT(a.integerEnforcement() == Number::strong); - BEAST_EXPECT(a.valid()); - } - { - Number a{INITIAL_XRP.drops(), Number::weak}; - BEAST_EXPECT(!a.valid()); - a = -a; - BEAST_EXPECT(!a.valid()); - - try - { - a.setIntegerEnforcement(Number::strong); - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT( - e.what() == - "Number::setIntegerEnforcement integer overflow"s); - // The throw is internal to the operator before the result is - // assigned to the Number - BEAST_EXPECT(a == -INITIAL_XRP); - BEAST_EXPECT(!a.valid()); - } - try - { - ++a; - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT(e.what() == "Number::addition integer overflow"s); - // The throw is internal to the operator before the result is - // assigned to the Number - BEAST_EXPECT(a == -INITIAL_XRP); - BEAST_EXPECT(!a.valid()); - } - a = Number::maxIntValue; - try - { - ++a; - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT(e.what() == "Number::addition integer overflow"s); - // This time, the throw is done _after_ the number is updated. - BEAST_EXPECT(a == Number::maxIntValue + 1); - BEAST_EXPECT(!a.valid()); - } - a = -Number::maxIntValue; - try - { - --a; - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT(e.what() == "Number::addition integer overflow"s); - // This time, the throw is done _after_ the number is updated. - BEAST_EXPECT(a == -Number::maxIntValue - 1); - BEAST_EXPECT(!a.valid()); - } - a = Number(1, 10); - try - { - a *= Number(1, 10); - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT( - e.what() == "Number::multiplication integer overflow"s); - // The throw is done _after_ the number is updated. - BEAST_EXPECT((a == Number{1, 20})); - BEAST_EXPECT(!a.valid()); - } - try - { - a = Number::maxIntValue * 2; - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT(e.what() == "Number::operator= integer overflow"s); - // The throw is done _after_ the number is updated. - BEAST_EXPECT((a == Number{2, 14})); - BEAST_EXPECT(!a.valid()); - } - try - { - a = Number(3, 15, Number::strong); - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT(e.what() == "Number::Number integer overflow"s); - // The Number doesn't get updated because the ctor throws - BEAST_EXPECT((a == Number{2, 14})); - BEAST_EXPECT(!a.valid()); - } - a = Number(1, 10); - try - { - a /= Number(1, -10); - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT(e.what() == "Number::division integer overflow"s); - // The throw is done _after_ the number is updated. - BEAST_EXPECT((a == Number{1, 20})); - BEAST_EXPECT(!a.valid()); - } - a /= Number(1, 15); - BEAST_EXPECT((a == Number{1, 5})); - BEAST_EXPECT(a.valid()); - } - } - - void run() override { testZero(); @@ -1023,7 +855,6 @@ public: test_toSTAmount(); test_truncate(); testRounding(); - testInteger(); } };