diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 1f2c41809a..0917627073 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -260,6 +260,11 @@ public: unsigned pop() noexcept; + // if true, there are no recoverable digits in the guard, though there may be dropped digits + // (xbit_) + [[nodiscard]] bool + unrecoverable() const noexcept; + // if true, there are no digits in the guard, including dropped digits (xbit_) [[nodiscard]] bool empty() const noexcept; @@ -277,6 +282,17 @@ public: void doDropDigit(T& mantissa, int& exponent) noexcept; + /** + * Drop a digit from the mantissa, and increment the exponent, storing the dropped digit in + * this Guard. + * + * If a drop will not do anything meaningful (there are no recoverable digits in the guard, and + * the mantissa is 0), and if targetExponent > exponent, simply set exponent to targetExponent. + */ + template + void + doDropDigitWithTarget(T& mantissa, int& exponent, int const targetExponent) noexcept; + // Modify the result to the correctly rounded value template void @@ -374,10 +390,16 @@ Number::Guard::pop() noexcept return d; } +inline bool +Number::Guard::unrecoverable() const noexcept +{ + return digits_ == 0; +} + inline bool Number::Guard::empty() const noexcept { - return digits_ == 0 && !xbit_; + return unrecoverable() && !xbit_; } template @@ -401,6 +423,25 @@ Number::Guard::doDropDigit(uint128_t& mantissa, int& exponent) noexce ++exponent; } +template +void +Number::Guard::doDropDigitWithTarget(T& mantissa, int& exponent, int const targetExponent) noexcept +{ + XRPL_ASSERT( + exponent < targetExponent, "xrpl::Number::Guard::doDropDigitWithTarget : something to do"); + while (exponent < targetExponent) + { + if (mantissa == 0 && unrecoverable()) + { + // No number of dropped digits is going to change anything except the exponent at this + // point, so just jump to the result + exponent = targetExponent; + return; + } + doDropDigit(mantissa, exponent); + } +} + template void Number::Guard::pushOverflow(T mantissa) @@ -928,6 +969,7 @@ Number::operator+=(Number const& y) // to match, if necessary. auto const adjust = [&g, &upperLimit]( uint128_t& expandM, int& expandE, uint128_t& shrinkM, int& shrinkE) { + XRPL_ASSERT(shrinkE < expandE, "xrpl::Number::operator+= : exponents ordered correctly"); // Adjust up and down until the exponents match if (g.cuspRoundingFix == MantissaRange::CuspRoundingFix::Enabled330) { @@ -935,6 +977,8 @@ Number::operator+=(Number const& y) // 1. First, shrink the mantissa of shrinkM/shrinkE while shrinkM ends in 0. while (shrinkE < expandE && shrinkM % 10 == 0) { + // Don't use doDropDigitWithTarget here, because the loop will stop before the + // mantissa gets to 0. g.doDropDigit(shrinkM, shrinkE); } @@ -950,10 +994,11 @@ Number::operator+=(Number const& y) // 3. Finally, shrink the mantissa of shrinkM/shrinkE until the exponents match. Any removed // digits will be put into the Guard. This is the only step for non-Enabled330 modes. - while (shrinkE < expandE) + if (shrinkE < expandE) { - g.doDropDigit(shrinkM, shrinkE); + g.doDropDigitWithTarget(shrinkM, shrinkE, expandE); } + XRPL_ASSERT(shrinkE == expandE, "xrpl::Number::operator+= : exponents are equal"); }; // Shrink the mantissa and raise the exponent of the value with the lower exponent. Store any @@ -996,7 +1041,7 @@ Number::operator+=(Number const& y) // round. XRPL_ASSERT( xm > maxMantissa || g.empty(), - "xrpl::Number::operator+ : rounding state expected after add"); + "xrpl::Number::operator+= : rounding state expected after add"); } else { @@ -1038,7 +1083,7 @@ Number::operator+=(Number const& y) } XRPL_ASSERT( xm > maxMantissa || g.empty(), - "xrpl::Number::operator+ : rounding state expected after subtract"); + "xrpl::Number::operator+= : rounding state expected after subtract"); } else { @@ -1330,9 +1375,10 @@ operator rep() const g.setNegative(); drops = -drops; } - while (offset < 0) + if (offset < 0) { - g.doDropDigit(drops, offset); + g.doDropDigitWithTarget(drops, offset, 0); + XRPL_ASSERT(offset == 0, "xrpl::Number::operator rep() : exponents are equal"); } for (; offset > 0; --offset) { diff --git a/src/test/protocol/STNumber_test.cpp b/src/test/protocol/STNumber_test.cpp index 74792e0a70..1e5027df49 100644 --- a/src/test/protocol/STNumber_test.cpp +++ b/src/test/protocol/STNumber_test.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -176,61 +177,32 @@ struct STNumber_test : public beast::unit_test::Suite numberFromJson(sfNumber, std::to_string(kUMax)) == STNumber(sfNumber, Number(kUMax, 0))); + auto const expectJsonThrows = [this]( + json::Value const& num, std::string const& expected) { + try + { + numberFromJson(sfNumber, num); + fail(); + } + catch (std::exception const& e) + { + std::ostringstream out; + out << "Json: " << num.asString() << " got exception: " << e.what() + << ", expected: " << expected; + BEAST_EXPECTS(std::string(e.what()) == expected, out.str()); + } + }; + + // Obvious overflows tested here + expectJsonThrows("1e2000000", "Number::normalize 2"); + expectJsonThrows("1e2000000000", "Number::normalize 2"); + // Obvious non-numbers tested here - try - { - auto _ = numberFromJson(sfNumber, ""); - BEAST_EXPECT(false); - } - catch (std::runtime_error const& e) - { - std::string const expected = "'' is not a number"; - BEAST_EXPECT(e.what() == expected); - } - - try - { - auto _ = numberFromJson(sfNumber, "e"); - BEAST_EXPECT(false); - } - catch (std::runtime_error const& e) - { - std::string const expected = "'e' is not a number"; - BEAST_EXPECT(e.what() == expected); - } - - try - { - auto _ = numberFromJson(sfNumber, "1e"); - BEAST_EXPECT(false); - } - catch (std::runtime_error const& e) - { - std::string const expected = "'1e' is not a number"; - BEAST_EXPECT(e.what() == expected); - } - - try - { - auto _ = numberFromJson(sfNumber, "e2"); - BEAST_EXPECT(false); - } - catch (std::runtime_error const& e) - { - std::string const expected = "'e2' is not a number"; - BEAST_EXPECT(e.what() == expected); - } - - try - { - auto _ = numberFromJson(sfNumber, json::Value()); - BEAST_EXPECT(false); - } - catch (std::runtime_error const& e) - { - std::string const expected = "not a number"; - BEAST_EXPECT(e.what() == expected); - } + expectJsonThrows("", "'' is not a number"); + expectJsonThrows("e", "'e' is not a number"); + expectJsonThrows("1e", "'1e' is not a number"); + expectJsonThrows("e2", "'e2' is not a number"); + expectJsonThrows(json::Value(), "not a number"); try { diff --git a/src/tests/libxrpl/basics/Number.cpp b/src/tests/libxrpl/basics/Number.cpp index 32f93eb1f7..8e958b40d4 100644 --- a/src/tests/libxrpl/basics/Number.cpp +++ b/src/tests/libxrpl/basics/Number.cpp @@ -1,5 +1,6 @@ #include +#include #include #include #include @@ -16,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -183,6 +185,17 @@ TEST(NumberTest, limits) } EXPECT_TRUE(caught); + try + { + Number{1, 2000000, Number::Normalized{}}; + ADD_FAILURE(); + } + catch (std::overflow_error const& e) + { + std::string const expected = "Number::normalize 2"; + EXPECT_EQ(e.what(), expected) << e.what(); + } + if (scale == MantissaRange::MantissaScale::Large330) { // Normalization with the other scales, including the older large mantissa scales, will @@ -406,6 +419,158 @@ TEST(NumberTest, add) } } +TEST(NumberTest, add_sub_extreme_exponents) +{ + for (auto const mantissaScale : MantissaRange::getAllScales()) + { + NumberMantissaScaleGuard const sg(mantissaScale); + + auto const scale = Number::getMantissaScale(); + + EXPECT_EQ(Number::getround(), Number::RoundingMode::ToNearest) + << to_string(Number::getround()); + + // Special cases: Exponents at each end of the allowable range + for (auto const round : + {Number::RoundingMode::ToNearest, + Number::RoundingMode::TowardsZero, + Number::RoundingMode::Downward, + Number::RoundingMode::Upward}) + { + NumberRoundModeGuard const rg{round}; + + auto const bigMantissa = std::invoke([scale, round] { + auto m = Number::maxMantissa(); + if (scale != MantissaRange::MantissaScale::Small) + { + // At the large scales, the maxMantissa is not representable, so we need to + // shrink it down to a representable value. + m /= 10; + } + if (round == Number::RoundingMode::Upward) + { + // Rounding upward will overflow if the mantissa is at maxMantissa. Subtract an + // arbitrary small value to keep the mantissa near the limit, but with a + // little room to grow. 67 has no meaning, except that it's, you know, + // six seven. + m -= 67; + } + return m; + }); + auto const params = { + std::make_pair(Number::minMantissa(), 0), + // At the large scales, the maxMantissa is not representable, so we need to shrink + // it down to a representable value. Rounding upward will overflow if the mantissa + // is right at the all nines value. To keep things a little simpler, do those + // modifications unconditionally. + std::make_pair(bigMantissa, 1), + }; + for (auto const& [mantissa, exponentOffset] : params) + { + auto const x = Number{mantissa, Number::kMaxExponent, Number::Normalized{}}; + auto const y = + Number{mantissa, Number::kMinExponent + exponentOffset, Number::Normalized{}}; + + std::ostringstream detail; + detail << "Scale: " << to_string(scale) << ", round: " << to_string(round) + << ", x: " << x << ", y: " << y; + + EXPECT_EQ(x.mantissa(), mantissa); + EXPECT_EQ(x.exponent(), Number::kMaxExponent); + EXPECT_NE(x, beast::kZero); + EXPECT_EQ(y.mantissa(), mantissa); + EXPECT_EQ(y.exponent(), Number::kMinExponent + exponentOffset); + EXPECT_NE(y, beast::kZero); + + { + // x + y + auto const result = x + y; + + if (round == Number::RoundingMode::Upward) + { + // Rounding upward will take that little x-bit and round result up to the + // next representable value. + EXPECT_NE(result, x); + EXPECT_EQ(result, (Number{x.mantissa() + 1, x.exponent()})); + } + else + { + EXPECT_EQ(result, x); + } + } + { + // x - y + auto const result = x - y; + + switch (round) + { + case Number::RoundingMode::TowardsZero: + if (scale < MantissaRange::MantissaScale::Large330) + { + // Rounding TowardsZero was broken before Large330. + EXPECT_EQ(result, x) << detail.str(); + break; + } + [[fallthrough]]; + case Number::RoundingMode::Downward: + // Rounding downward (or toward zero in Large330) will take that little + // x-bit and round result down to the next representable value. + EXPECT_NE(result, x) << detail.str(); + EXPECT_EQ(result, (Number{x.mantissa() - 1, x.exponent()})) + << detail.str(); + break; + default: + // Rounding up and toNearest rounds back to the original value + EXPECT_EQ(result, x) << detail.str(); + } + } + { + // y + x + auto const result = y + x; + + if (round == Number::RoundingMode::Upward) + { + // Rounding upward will take that little x-bit and round result up to the + // next representable value. + EXPECT_NE(result, x); + EXPECT_EQ(result, (Number{x.mantissa() + 1, x.exponent()})); + } + else + { + EXPECT_EQ(result, x); + } + } + { + // y - x + auto const result = y - x; + + switch (round) + { + case Number::RoundingMode::TowardsZero: + if (scale < MantissaRange::MantissaScale::Large330) + { + // Rounding TowardsZero was broken before Large330. + EXPECT_EQ(result, -x) << detail.str(); + break; + } + [[fallthrough]]; + case Number::RoundingMode::Upward: + // Rounding upward (or toward zero in Large330) will take that little + // x-bit and round result up to the next representable negative value. + EXPECT_NE(result, -x) << detail.str(); + EXPECT_EQ(result, (Number{-x.mantissa() + 1, x.exponent()})) + << detail.str(); + break; + default: + // Rounding up and toNearest rounds back to the original value + EXPECT_EQ(result, -x) << detail.str(); + } + } + } + } + } +} + TEST(NumberTest, sub) { for (auto const mantissaScale : MantissaRange::getAllScales())