From 4e61fe9c56cca82b79a229fa740014d478079c24 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Wed, 13 Apr 2022 16:01:52 -0400 Subject: [PATCH 01/13] AMM Add Number class and associated algorithms --- Builds/CMake/RippledCore.cmake | 3 + src/ripple/basics/Number.h | 322 +++++++++++++++++ src/ripple/basics/impl/Number.cpp | 582 ++++++++++++++++++++++++++++++ src/test/basics/Number_test.cpp | 143 ++++++++ 4 files changed, 1050 insertions(+) create mode 100644 src/ripple/basics/Number.h create mode 100644 src/ripple/basics/impl/Number.cpp create mode 100644 src/test/basics/Number_test.cpp diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 4bad3a87b4a..dca4720e381 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -50,6 +50,7 @@ target_sources (xrpl_core PRIVATE src/ripple/basics/impl/FileUtilities.cpp src/ripple/basics/impl/IOUAmount.cpp src/ripple/basics/impl/Log.cpp + src/ripple/basics/impl/Number.cpp src/ripple/basics/impl/StringUtilities.cpp #[===============================[ main sources: @@ -153,6 +154,7 @@ install ( src/ripple/basics/LocalValue.h src/ripple/basics/Log.h src/ripple/basics/MathUtilities.h + src/ripple/basics/Number.h src/ripple/basics/safe_cast.h src/ripple/basics/Slice.h src/ripple/basics/spinlock.h @@ -737,6 +739,7 @@ if (tests) src/test/basics/FileUtilities_test.cpp src/test/basics/IOUAmount_test.cpp src/test/basics/KeyCache_test.cpp + src/test/basics/Number_test.cpp src/test/basics/PerfLog_test.cpp src/test/basics/RangeSet_test.cpp src/test/basics/scope_test.cpp diff --git a/src/ripple/basics/Number.h b/src/ripple/basics/Number.h new file mode 100644 index 00000000000..607db3ff889 --- /dev/null +++ b/src/ripple/basics/Number.h @@ -0,0 +1,322 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2022 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_BASICS_NUMBER_H_INCLUDED +#define RIPPLE_BASICS_NUMBER_H_INCLUDED + +#include +#include +#include +#include + +namespace ripple { + +class Number; + +std::string +to_string(Number const& amount); + +class Number +{ + using rep = std::int64_t; + rep mantissa_{0}; + int exponent_{-2'147'483'648}; + +public: + struct unchecked + { + explicit unchecked() = default; + }; + + explicit Number() = default; + + Number(rep mantissa); + explicit Number(rep mantissa, int exponent); + explicit constexpr Number(rep mantissa, int exponent, unchecked) noexcept; + + Number(IOUAmount const& x); + + constexpr rep + mantissa() const noexcept; + constexpr int + exponent() const noexcept; + + constexpr Number + operator+() const noexcept; + constexpr Number + operator-() const noexcept; + Number& + operator++(); + Number + operator++(int); + Number& + operator--(); + Number + operator--(int); + + Number& + operator+=(Number const& x); + Number& + operator-=(Number const& x); + + Number& + operator*=(Number const& x); + Number& + operator/=(Number const& x); + + explicit operator IOUAmount() const; + + friend constexpr bool + operator==(Number const& x, Number const& y) noexcept + { + return x.mantissa_ == y.mantissa_ && x.exponent_ == y.exponent_; + } + + friend constexpr bool + operator!=(Number const& x, Number const& y) noexcept + { + return !(x == y); + } + + friend constexpr bool + operator<(Number const& x, Number const& y) noexcept + { + // If the two amounts have different signs (zero is treated as positive) + // then the comparison is true iff the left is negative. + bool const lneg = x.mantissa_ < 0; + bool const rneg = y.mantissa_ < 0; + + if (lneg != rneg) + return lneg; + + // Both have same sign and the left is zero: the right must be + // greater than 0. + if (x.mantissa_ == 0) + return y.mantissa_ > 0; + + // Both have same sign, the right is zero and the left is non-zero. + if (y.mantissa_ == 0) + return false; + + // Both have the same sign, compare by exponents: + if (x.exponent_ > y.exponent_) + return lneg; + if (x.exponent_ < y.exponent_) + return !lneg; + + // If equal exponents, compare mantissas + return x.mantissa_ < y.mantissa_; + } + + friend constexpr bool + operator>(Number const& x, Number const& y) noexcept + { + return y < x; + } + + friend constexpr bool + operator<=(Number const& x, Number const& y) noexcept + { + return !(y < x); + } + + friend constexpr bool + operator>=(Number const& x, Number const& y) noexcept + { + return !(x < y); + } + + friend std::ostream& + operator<<(std::ostream& os, Number const& x) + { + return os << to_string(x); + } + +private: + void + normalize(); + constexpr bool + isnormal() const noexcept; + + // The range for the mantissa when normalized + 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; + constexpr static int maxExponent = 32768; + + class guard; +}; + +inline constexpr Number::Number(rep mantissa, int exponent, unchecked) noexcept + : mantissa_{mantissa}, exponent_{exponent} +{ +} + +inline Number::Number(rep mantissa, int exponent) + : mantissa_{mantissa}, exponent_{exponent} +{ + normalize(); +} + +inline Number::Number(rep mantissa) : Number{mantissa, 0} +{ +} + +inline Number::Number(IOUAmount const& x) : Number{x.mantissa(), x.exponent()} +{ +} + +inline constexpr Number::rep +Number::mantissa() const noexcept +{ + return mantissa_; +} + +inline constexpr int +Number::exponent() const noexcept +{ + return exponent_; +} + +inline constexpr Number +Number::operator+() const noexcept +{ + return *this; +} + +inline constexpr Number +Number::operator-() const noexcept +{ + auto x = *this; + x.mantissa_ = -x.mantissa_; + return x; +} + +inline Number& +Number::operator++() +{ + *this += Number{1000000000000000, -15, unchecked{}}; + return *this; +} + +inline Number +Number::operator++(int) +{ + auto x = *this; + ++(*this); + return x; +} + +inline Number& +Number::operator--() +{ + *this -= Number{1000000000000000, -15, unchecked{}}; + return *this; +} + +inline Number +Number::operator--(int) +{ + auto x = *this; + --(*this); + return x; +} + +inline Number& +Number::operator-=(Number const& x) +{ + return *this += -x; +} + +inline Number +operator+(Number const& x, Number const& y) +{ + auto z = x; + z += y; + return z; +} + +inline Number +operator-(Number const& x, Number const& y) +{ + auto z = x; + z -= y; + return z; +} + +inline Number +operator*(Number const& x, Number const& y) +{ + auto z = x; + z *= y; + return z; +} + +inline Number +operator/(Number const& x, Number const& y) +{ + auto z = x; + z /= y; + return z; +} + +inline Number::operator IOUAmount() const +{ + return IOUAmount{mantissa(), exponent()}; +} + +inline constexpr bool +Number::isnormal() const noexcept +{ + auto const abs_m = mantissa_ < 0 ? -mantissa_ : mantissa_; + return minMantissa <= abs_m && abs_m <= maxMantissa && + minExponent <= exponent_ && exponent_ <= maxExponent; +} + +inline constexpr Number +abs(Number x) noexcept +{ + if (x < Number{}) + x = -x; + return x; +} + +// Returns f^n +// Uses a log_2(n) number of mulitiplications + +Number +power(Number f, unsigned n); + +// Returns f^(1/d) +// Uses Newton–Raphson iterations until the result stops changing +// to find the root of the polynomial g(x) = x^d - f + +Number +root(Number f, unsigned d); + +// Returns f^(n/d) + +Number +power(Number f, unsigned n, unsigned d); + +} // namespace ripple + +#endif // RIPPLE_BASICS_NUMBER_H_INCLUDED diff --git a/src/ripple/basics/impl/Number.cpp b/src/ripple/basics/impl/Number.cpp new file mode 100644 index 00000000000..1f056a2d93c --- /dev/null +++ b/src/ripple/basics/impl/Number.cpp @@ -0,0 +1,582 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2022 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include +#include +#include +#include +#include +#include + +#ifdef _MSVC_LANG +#include +using uint128_t = boost::multiprecision::uint128_t; +#else // !defined(_MSVC_LANG) +using uint128_t = __uint128_t; +#endif // !defined(_MSVC_LANG) + +namespace ripple { + +// guard + +class Number::guard +{ + std::uint64_t digits_; + std::uint8_t xbit_ : 1; + std::uint8_t sbit_ : 1; // TODO : get rid of + +public: + explicit guard() : digits_{0}, xbit_{0}, sbit_{0} + { + } + + void + set_positive() noexcept; + void + set_negative() noexcept; + bool + is_negative() const noexcept; + void + push(unsigned d) noexcept; + unsigned + pop() noexcept; + int + round() noexcept; +}; + +inline void +Number::guard::set_positive() noexcept +{ + sbit_ = 0; +} + +inline void +Number::guard::set_negative() noexcept +{ + sbit_ = 1; +} + +inline bool +Number::guard::is_negative() const noexcept +{ + return sbit_ == 1; +} + +inline void +Number::guard::push(unsigned d) noexcept +{ + xbit_ = xbit_ || (digits_ & 0x0000'0000'0000'000F) != 0; + digits_ >>= 4; + digits_ |= (d & 0x0000'0000'0000'000FULL) << 60; +} + +inline unsigned +Number::guard::pop() noexcept +{ + unsigned d = (digits_ & 0xF000'0000'0000'0000) >> 60; + digits_ <<= 4; + return d; +} + +int +Number::guard::round() noexcept +{ + if (digits_ > 0x5000'0000'0000'0000) + return 1; + if (digits_ < 0x5000'0000'0000'0000) + return -1; + if (xbit_) + return 1; + return 0; +} + +// Number + +constexpr Number one{1000000000000000, -15, Number::unchecked{}}; + +void +Number::normalize() +{ + if (mantissa_ == 0) + { + *this = Number{}; + return; + } + bool const negative = (mantissa_ < 0); + if (negative) + mantissa_ = -mantissa_; + auto m = static_cast>(mantissa_); + while ((m < minMantissa) && (exponent_ > minExponent)) + { + m *= 10; + --exponent_; + } + while (m > maxMantissa) + { + if (exponent_ >= maxExponent) + throw std::overflow_error("Number::normalize 1"); + m /= 10; + ++exponent_; + } + mantissa_ = m; + if ((exponent_ < minExponent) || (mantissa_ < minMantissa)) + { + *this = Number{}; + return; + } + + if (exponent_ > maxExponent) + throw std::overflow_error("Number::normalize 2"); + + if (negative) + mantissa_ = -mantissa_; +} + +Number& +Number::operator+=(Number const& y) +{ + if (y == Number{}) + return *this; + if (*this == Number{}) + { + *this = y; + return *this; + } + if (*this == -y) + { + *this = Number{}; + return *this; + } + assert(isnormal() && y.isnormal()); + auto xm = mantissa(); + auto xe = exponent(); + int xn = 1; + if (xm < 0) + { + xm = -xm; + xn = -1; + } + auto ym = y.mantissa(); + auto ye = y.exponent(); + int yn = 1; + if (ym < 0) + { + ym = -ym; + yn = -1; + } + guard g; + if (xe < ye) + { + if (xn == -1) + g.set_negative(); + do + { + g.push(xm % 10); + xm /= 10; + ++xe; + } while (xe < ye); + } + else if (xe > ye) + { + if (yn == -1) + g.set_negative(); + do + { + g.push(ym % 10); + ym /= 10; + ++ye; + } while (xe > ye); + } + if (xn == yn) + { + xm += ym; + if (xm > maxMantissa) + { + g.push(xm % 10); + xm /= 10; + ++xe; + } + auto r = g.round(); + if (r == 1 || (r == 0 && (xm & 1) == 1)) + { + ++xm; + if (xm > maxMantissa) + { + xm /= 10; + ++xe; + } + } + if (xe > maxExponent) + throw std::overflow_error("Number::addition overflow"); + } + else + { + if (xm > ym) + { + xm = xm - ym; + } + else + { + xm = ym - xm; + xe = ye; + xn = yn; + } + while (xm < minMantissa) + { + xm *= 10; + xm -= g.pop(); + --xe; + } + auto r = g.round(); + if (r == 1 || (r == 0 && (xm & 1) == 1)) + { + --xm; + if (xm < minMantissa) + { + xm *= 10; + --xe; + } + } + if (xe < minExponent) + { + xm = 0; + xe = Number{}.exponent_; + } + } + mantissa_ = xm * xn; + exponent_ = xe; + assert(isnormal()); + return *this; +} + +Number& +Number::operator*=(Number const& y) +{ + if (*this == Number{}) + return *this; + if (y == Number{}) + { + *this = y; + return *this; + } + assert(isnormal() && y.isnormal()); + auto xm = mantissa(); + auto xe = exponent(); + int xn = 1; + if (xm < 0) + { + xm = -xm; + xn = -1; + } + auto ym = y.mantissa(); + auto ye = y.exponent(); + int yn = 1; + if (ym < 0) + { + ym = -ym; + yn = -1; + } + auto zm = uint128_t(xm) * uint128_t(ym); + auto ze = xe + ye; + auto zn = xn * yn; + guard g; + while (zm > maxMantissa) + { + g.push(static_cast(zm % 10)); + zm /= 10; + ++ze; + } + xm = static_cast(zm); + xe = ze; + auto r = g.round(); + if (r == 1 || (r == 0 && (xm & 1) == 1)) + { + ++xm; + if (xm > maxMantissa) + { + xm /= 10; + ++xe; + } + } + if (xe < minExponent) + { + xm = 0; + xe = Number{}.exponent_; + } + if (xe > maxExponent) + throw std::overflow_error( + "Number::multiplication overflow : exponent is " + + std::to_string(xe)); + mantissa_ = xm * zn; + exponent_ = xe; + assert(isnormal()); + return *this; +} + +Number& +Number::operator/=(Number const& y) +{ + if (y == Number{}) + throw std::overflow_error("Number: divide by 0"); + int np = 1; + auto nm = mantissa(); + if (nm < 0) + { + nm = -nm; + np = -1; + } + int dp = 1; + auto dm = y.mantissa(); + if (dm < 0) + { + dm = -dm; + dp = -1; + } + // Divide numerator and denominator such that the + // denominator is in the range [1, 10). + const int offset = -15 - y.exponent(); + Number n{nm * (np * dp), exponent() + offset}; + Number d{dm, y.exponent() + offset}; + // Quadratic least squares fit to 1/x in the range [1, 10] + constexpr Number a0{9178756872006464, -16, unchecked{}}; + constexpr Number a1{-2149215784206187, -16, unchecked{}}; + constexpr Number a2{1405502114116773, -17, unchecked{}}; + static_assert(a0.isnormal()); + static_assert(a1.isnormal()); + static_assert(a2.isnormal()); + Number rm2{}; + Number rm1{}; + Number r = a2; + r = (a2 * d + a1) * d + a0; + do + { + rm2 = rm1; + rm1 = r; + r = r + r * (one - d * r); + } while (r != rm1 && r != rm2); + *this = n * r; + return *this; +} + +std::string +to_string(Number const& amount) +{ + // keep full internal accuracy, but make more human friendly if possible + if (amount == Number{}) + return "0"; + + auto const exponent = amount.exponent(); + auto mantissa = amount.mantissa(); + + // Use scientific notation for exponents that are too small or too large + if (((exponent != 0) && ((exponent < -25) || (exponent > -5)))) + { + std::string ret = std::to_string(mantissa); + ret.append(1, 'e'); + ret.append(std::to_string(exponent)); + return ret; + } + + bool negative = false; + + if (mantissa < 0) + { + mantissa = -mantissa; + negative = true; + } + + assert(exponent + 43 > 0); + + size_t const pad_prefix = 27; + size_t const pad_suffix = 23; + + std::string const raw_value(std::to_string(mantissa)); + std::string val; + + val.reserve(raw_value.length() + pad_prefix + pad_suffix); + val.append(pad_prefix, '0'); + val.append(raw_value); + val.append(pad_suffix, '0'); + + size_t const offset(exponent + 43); + + auto pre_from(val.begin()); + auto const pre_to(val.begin() + offset); + + auto const post_from(val.begin() + offset); + auto post_to(val.end()); + + // Crop leading zeroes. Take advantage of the fact that there's always a + // fixed amount of leading zeroes and skip them. + if (std::distance(pre_from, pre_to) > pad_prefix) + pre_from += pad_prefix; + + assert(post_to >= post_from); + + pre_from = std::find_if(pre_from, pre_to, [](char c) { return c != '0'; }); + + // Crop trailing zeroes. Take advantage of the fact that there's always a + // fixed amount of trailing zeroes and skip them. + if (std::distance(post_from, post_to) > pad_suffix) + post_to -= pad_suffix; + + assert(post_to >= post_from); + + post_to = std::find_if( + std::make_reverse_iterator(post_to), + std::make_reverse_iterator(post_from), + [](char c) { return c != '0'; }) + .base(); + + std::string ret; + + if (negative) + ret.append(1, '-'); + + // Assemble the output: + if (pre_from == pre_to) + ret.append(1, '0'); + else + ret.append(pre_from, pre_to); + + if (post_to != post_from) + { + ret.append(1, '.'); + ret.append(post_from, post_to); + } + + return ret; +} + +// Returns f^n +// Uses a log_2(n) number of mulitiplications + +Number +power(Number f, unsigned n) +{ + if (n == 0) + return one; + if (n == 1) + return f; + auto r = power(f, n / 2); + r *= r; + if (n % 2 != 0) + r *= f; + return r; +} + +// Returns f^(1/d) +// Uses Newton–Raphson iterations until the result stops changing +// to find the non-negative root of the polynomial g(x) = x^d - f + +Number +root(Number f, unsigned d) +{ + if (f == one || d == 1) + return f; + if (d == 0) + { + if (f == -one) + return one; + if (abs(f) < one) + return Number{}; + throw std::overflow_error("Number::root infinity"); + } + if (f < Number{} && d % 2 == 0) + throw std::overflow_error("Number::root nan"); + if (f == Number{}) + return f; + + // Scale f into the range (0, 1) such that f's exponent is a multiple of d + auto e = f.exponent() + 16; + auto const di = static_cast(d); + auto ex = [e = e, di = di]() // Euclidean remainder of e/d + { + int k = (e >= 0 ? e : e - (di - 1)) / di; + int k2 = e - k * di; + if (k2 == 0) + return 0; + return di - k2; + }(); + e += ex; + f = Number{f.mantissa(), f.exponent() - e}; // f /= 10^e; + bool neg = false; + if (f < Number{}) + { + neg = true; + f = -f; + } + + // Quadratic least squares curve fit of f^(1/d) in the range [0, 1] + auto const D = ((6 * di + 11) * di + 6) * di + 1; + auto const a0 = 3 * di * ((2 * di - 3) * di + 1); + auto const a1 = 24 * di * (2 * di - 1); + auto const a2 = -30 * (di - 1) * di; + Number r = ((Number{a2} * f + Number{a1}) * f + Number{a0}) / Number{D}; + if (neg) + { + f = -f; + r = -r; + } + + // Newton–Raphson iteration of f^(1/d) with initial guess r + // halt when r stops changing, checking for bouncing on the last iteration + Number rm1{}; + Number rm2{}; + do + { + rm2 = rm1; + rm1 = r; + r = (Number(d - 1) * r + f / power(r, d - 1)) / Number(d); + } while (r != rm1 && r != rm2); + + // return r * 10^(e/d) to reverse scaling + return Number{r.mantissa(), r.exponent() + e / di}; +} + +// Returns f^(n/d) + +Number +power(Number f, unsigned n, unsigned d) +{ + if (f == one) + return f; + auto g = std::gcd(n, d); + if (g == 0) + throw std::overflow_error("Number::power nan"); + if (d == 0) + { + if (f == -one) + return one; + if (abs(f) < one) + return Number{}; + if (abs(f) > one) + throw std::overflow_error("Number::power infinity"); + throw std::overflow_error("Number::power nan"); + } + if (n == 0) + return one; + n /= g; + d /= g; + if ((n % 2) == 1 && (d % 2) == 0 && f < Number{}) + throw std::overflow_error("Number::power nan"); + return root(power(f, n), d); +} + +} // namespace ripple diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp new file mode 100644 index 00000000000..570e162872e --- /dev/null +++ b/src/test/basics/Number_test.cpp @@ -0,0 +1,143 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2022 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include +#include +#include + +namespace ripple { + +class Number_test : public beast::unit_test::suite +{ +public: + void + testZero() + { + testcase("zero"); + + Number const z{0, 0}; + + BEAST_EXPECT(z.mantissa() == 0); + BEAST_EXPECT(z.exponent() == Number{}.exponent()); + + BEAST_EXPECT((z + z) == z); + BEAST_EXPECT((z - z) == z); + BEAST_EXPECT(z == -z); + } + + void + test_add() + { + testcase("test_add"); + Number x[]{ + Number{1'000'000'000'000'000, -15}, + Number{-1'000'000'000'000'000, -15}, + Number{-1'000'000'000'000'000, -15}, + Number{-6'555'555'555'555'555, -29}}; + Number y[]{ + Number{6'555'555'555'555'555, -29}, + Number{-6'555'555'555'555'555, -29}, + Number{6'555'555'555'555'555, -29}, + Number{1'000'000'000'000'000, -15}}; + Number z[]{ + Number{1'000'000'000'000'066, -15}, + Number{-1'000'000'000'000'066, -15}, + Number{-9'999'999'999'999'344, -16}, + Number{9'999'999'999'999'344, -16}}; + for (unsigned i = 0; i < std::size(x); ++i) + { + BEAST_EXPECT(x[i] + y[i] == z[i]); + } + } + + void + test_sub() + { + testcase("test_sub"); + Number x[]{ + Number{1'000'000'000'000'000, -15}, + Number{6'555'555'555'555'555, -29}}; + Number y[]{ + Number{6'555'555'555'555'555, -29}, + Number{1'000'000'000'000'000, -15}}; + Number z[]{ + Number{9'999'999'999'999'344, -16}, + Number{-9'999'999'999'999'344, -16}}; + for (unsigned i = 0; i < std::size(x); ++i) + { + BEAST_EXPECT(x[i] - y[i] == z[i]); + } + } + + void + test_div() + { + testcase("test_div"); + Number x[]{Number{1}, Number{1}, Number{0}}; + Number y[]{Number{2}, Number{10}, Number{100}}; + Number z[]{Number{5, -1}, Number{1, -1}, Number{0}}; + for (unsigned i = 0; i < std::size(x); ++i) + { + BEAST_EXPECT(x[i] / y[i] == z[i]); + } + } + + void + test_root() + { + testcase("test_root"); + Number x[]{Number{2}, Number{2'000'000}, Number{2, -30}}; + unsigned y[]{2, 2, 2}; + Number z[]{ + Number{1414213562373095, -15}, + Number{1414213562373095, -12}, + Number{1414213562373095, -30}}; + for (unsigned i = 0; i < std::size(x); ++i) + { + BEAST_EXPECT(root(x[i], y[i]) == z[i]); + } + } + + void + testConversions() + { + testcase("testConversions"); + + IOUAmount x{5, 6}; + Number y = x; + BEAST_EXPECT((y == Number{5, 6})); + IOUAmount z{y}; + BEAST_EXPECT(x == z); + } + + void + run() override + { + testZero(); + test_add(); + test_sub(); + test_div(); + test_root(); + testConversions(); + } +}; + +BEAST_DEFINE_TESTSUITE(Number, ripple_basics, ripple); + +} // namespace ripple From 407f04187a7cbab698c89b9eff9e2a49c7f128fb Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Fri, 15 Apr 2022 16:20:52 -0400 Subject: [PATCH 02/13] Add conversions between Number, XRPAmount and int64_t * Conversions to Number are implicit * Conversions away from Number are explicit and potentially lossy * If lossy, round to nearest, and to even on tie --- src/ripple/basics/Number.h | 8 ++++ src/ripple/basics/impl/Number.cpp | 39 +++++++++++++++++ src/test/basics/Number_test.cpp | 70 +++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+) diff --git a/src/ripple/basics/Number.h b/src/ripple/basics/Number.h index 607db3ff889..50975e7d052 100644 --- a/src/ripple/basics/Number.h +++ b/src/ripple/basics/Number.h @@ -21,6 +21,7 @@ #define RIPPLE_BASICS_NUMBER_H_INCLUDED #include +#include #include #include #include @@ -51,6 +52,7 @@ class Number explicit constexpr Number(rep mantissa, int exponent, unchecked) noexcept; Number(IOUAmount const& x); + Number(XRPAmount const& x); constexpr rep mantissa() const noexcept; @@ -81,6 +83,8 @@ class Number operator/=(Number const& x); explicit operator IOUAmount() const; + explicit operator XRPAmount() const; // round to nearest, even on tie + explicit operator rep() const; // round to nearest, even on tie friend constexpr bool operator==(Number const& x, Number const& y) noexcept @@ -184,6 +188,10 @@ inline Number::Number(IOUAmount const& x) : Number{x.mantissa(), x.exponent()} { } +inline Number::Number(XRPAmount const& x) : Number{x.drops()} +{ +} + inline constexpr Number::rep Number::mantissa() const noexcept { diff --git a/src/ripple/basics/impl/Number.cpp b/src/ripple/basics/impl/Number.cpp index 1f056a2d93c..c9e9c51ce9e 100644 --- a/src/ripple/basics/impl/Number.cpp +++ b/src/ripple/basics/impl/Number.cpp @@ -374,6 +374,45 @@ Number::operator/=(Number const& y) return *this; } +Number::operator rep() const +{ + std::int64_t drops = mantissa_; + int offset = exponent_; + guard g; + if (drops != 0) + { + if (drops < 0) + { + g.set_negative(); + drops = -drops; + } + for (; offset < 0; ++offset) + { + g.push(drops % 10); + drops /= 10; + } + for (; offset > 0; --offset) + { + if (drops > std::numeric_limits::max() / 10) + throw std::runtime_error("Number::operator rep() overflow"); + drops *= 10; + } + auto r = g.round(); + if (r == 1 || (r == 0 && (drops & 1) == 1)) + { + ++drops; + } + if (g.is_negative()) + drops = -drops; + } + return drops; +} + +Number::operator XRPAmount() const +{ + return XRPAmount{static_cast(*this)}; +} + std::string to_string(Number const& amount) { diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 570e162872e..444095bfb39 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -126,6 +126,75 @@ class Number_test : public beast::unit_test::suite BEAST_EXPECT(x == z); } + void + test_to_integer() + { + Number x[]{ + Number{0}, + Number{1}, + Number{2}, + Number{3}, + Number{-1}, + Number{-2}, + Number{-3}, + Number{10}, + Number{99}, + Number{1155}, + Number{9'999'999'999'999'999, 0}, + Number{9'999'999'999'999'999, 1}, + Number{9'999'999'999'999'999, 2}, + Number{-9'999'999'999'999'999, 2}, + Number{15, -1}, + Number{14, -1}, + Number{16, -1}, + Number{25, -1}, + Number{6, -1}, + Number{5, -1}, + Number{4, -1}, + Number{-15, -1}, + Number{-14, -1}, + Number{-16, -1}, + Number{-25, -1}, + Number{-6, -1}, + Number{-5, -1}, + Number{-4, -1}}; + std::int64_t y[]{ + 0, + 1, + 2, + 3, + -1, + -2, + -3, + 10, + 99, + 1155, + 9'999'999'999'999'999, + 99'999'999'999'999'990, + 999'999'999'999'999'900, + -999'999'999'999'999'900, + 2, + 1, + 2, + 2, + 1, + 0, + 0, + -2, + -1, + -2, + -2, + -1, + 0, + 0}; + static_assert(std::size(x) == std::size(y)); + for (unsigned u = 0; u < std::size(x); ++u) + { + auto j = static_cast(x[u]); + BEAST_EXPECT(j == y[u]); + } + } + void run() override { @@ -135,6 +204,7 @@ class Number_test : public beast::unit_test::suite test_div(); test_root(); testConversions(); + test_to_integer(); } }; From 5744b373936a166fe2d216bc53c94ae02835d86e Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Mon, 18 Apr 2022 14:21:57 -0400 Subject: [PATCH 03/13] Add clip * Return 0 if abs(x) < limit, else returns x --- src/ripple/basics/Number.h | 10 ++++++++++ src/test/basics/Number_test.cpp | 15 +++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/ripple/basics/Number.h b/src/ripple/basics/Number.h index 50975e7d052..3832a84959b 100644 --- a/src/ripple/basics/Number.h +++ b/src/ripple/basics/Number.h @@ -325,6 +325,16 @@ root(Number f, unsigned d); Number power(Number f, unsigned n, unsigned d); +// Return 0 if abs(x) < limit, else returns x + +inline constexpr Number +clip(Number const& x, Number const& limit) noexcept +{ + if (abs(x) < limit) + return Number{}; + return x; +} + } // namespace ripple #endif // RIPPLE_BASICS_NUMBER_H_INCLUDED diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 444095bfb39..43a8884c598 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -129,6 +129,7 @@ class Number_test : public beast::unit_test::suite void test_to_integer() { + testcase("test_to_integer"); Number x[]{ Number{0}, Number{1}, @@ -195,6 +196,19 @@ class Number_test : public beast::unit_test::suite } } + void + test_clip() + { + testcase("test_clip"); + Number limit{1, -6}; + BEAST_EXPECT((clip(Number{2, -6}, limit) == Number{2, -6})); + BEAST_EXPECT((clip(Number{1, -6}, limit) == Number{1, -6})); + BEAST_EXPECT((clip(Number{9, -7}, limit) == Number{0})); + BEAST_EXPECT((clip(Number{-2, -6}, limit) == Number{-2, -6})); + BEAST_EXPECT((clip(Number{-1, -6}, limit) == Number{-1, -6})); + BEAST_EXPECT((clip(Number{-9, -7}, limit) == Number{0})); + } + void run() override { @@ -205,6 +219,7 @@ class Number_test : public beast::unit_test::suite test_root(); testConversions(); test_to_integer(); + test_clip(); } }; From eb0774dc40edc03c172d337e0149d25ce38eb70d Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Wed, 20 Apr 2022 17:10:04 -0400 Subject: [PATCH 04/13] Add implicit conversion from STAmount to Number --- src/ripple/basics/impl/Number.cpp | 9 +++++---- src/ripple/protocol/STAmount.h | 9 +++++++++ src/ripple/rpc/impl/ShardArchiveHandler.cpp | 9 +++++---- src/test/basics/Number_test.cpp | 5 +++++ 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/ripple/basics/impl/Number.cpp b/src/ripple/basics/impl/Number.cpp index c9e9c51ce9e..e59ca5fb63f 100644 --- a/src/ripple/basics/impl/Number.cpp +++ b/src/ripple/basics/impl/Number.cpp @@ -325,7 +325,7 @@ Number::operator*=(Number const& y) std::to_string(xe)); mantissa_ = xm * zn; exponent_ = xe; - assert(isnormal()); + assert(isnormal() || *this == Number{}); return *this; } @@ -362,8 +362,9 @@ Number::operator/=(Number const& y) static_assert(a2.isnormal()); Number rm2{}; Number rm1{}; - Number r = a2; - r = (a2 * d + a1) * d + a0; + Number r = (a2 * d + a1) * d + a0; + // Newton–Raphson iteration of 1/x - d with initial guess r + // halt when r stops changing, checking for bouncing on the last iteration do { rm2 = rm1; @@ -376,7 +377,7 @@ Number::operator/=(Number const& y) Number::operator rep() const { - std::int64_t drops = mantissa_; + rep drops = mantissa_; int offset = exponent_; guard g; if (drops != 0) diff --git a/src/ripple/protocol/STAmount.h b/src/ripple/protocol/STAmount.h index d0add30dbba..b6e0e3046ff 100644 --- a/src/ripple/protocol/STAmount.h +++ b/src/ripple/protocol/STAmount.h @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -144,6 +145,7 @@ class STAmount final : public STBase, public CountedObject // Legacy support for new-style amounts STAmount(IOUAmount const& amount, Issue const& issue); STAmount(XRPAmount const& amount); + operator Number() const; //-------------------------------------------------------------------------- // @@ -370,6 +372,13 @@ inline STAmount::operator bool() const noexcept return *this != beast::zero; } +inline STAmount::operator Number() const +{ + if (mIsNative) + return xrp(); + return iou(); +} + inline STAmount& STAmount::operator=(beast::Zero) { clear(); diff --git a/src/ripple/rpc/impl/ShardArchiveHandler.cpp b/src/ripple/rpc/impl/ShardArchiveHandler.cpp index 2284780c2b6..d05744f483a 100644 --- a/src/ripple/rpc/impl/ShardArchiveHandler.cpp +++ b/src/ripple/rpc/impl/ShardArchiveHandler.cpp @@ -37,11 +37,12 @@ using namespace std::chrono_literals; boost::filesystem::path ShardArchiveHandler::getDownloadDirectory(Config const& config) { - return get(config.section(ConfigSection::shardDatabase()), - "download_path", + return boost::filesystem::path{ get(config.section(ConfigSection::shardDatabase()), - "path", - "")) / + "download_path", + get(config.section(ConfigSection::shardDatabase()), + "path", + ""))} / "download"; } diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 43a8884c598..b605ed434c6 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -20,6 +20,7 @@ #include #include #include +#include namespace ripple { @@ -124,6 +125,10 @@ class Number_test : public beast::unit_test::suite BEAST_EXPECT((y == Number{5, 6})); IOUAmount z{y}; BEAST_EXPECT(x == z); + XRPAmount xrp{500}; + STAmount st = xrp; + Number n = st; + BEAST_EXPECT(XRPAmount{n} == xrp); } void From 77d9a3d72416b978c9c805b227d1252eda936919 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Thu, 21 Apr 2022 16:47:07 -0400 Subject: [PATCH 05/13] Add tests --- src/ripple/basics/Number.h | 14 +- src/ripple/basics/impl/Number.cpp | 112 +++++-- src/test/basics/Number_test.cpp | 475 ++++++++++++++++++++++-------- 3 files changed, 453 insertions(+), 148 deletions(-) diff --git a/src/ripple/basics/Number.h b/src/ripple/basics/Number.h index 3832a84959b..92b99bf0145 100644 --- a/src/ripple/basics/Number.h +++ b/src/ripple/basics/Number.h @@ -37,7 +37,7 @@ class Number { using rep = std::int64_t; rep mantissa_{0}; - int exponent_{-2'147'483'648}; + int exponent_{std::numeric_limits::lowest()}; public: struct unchecked @@ -45,7 +45,7 @@ class Number explicit unchecked() = default; }; - explicit Number() = default; + explicit constexpr Number() = default; Number(rep mantissa); explicit Number(rep mantissa, int exponent); @@ -166,7 +166,7 @@ class Number constexpr static int minExponent = -32768; constexpr static int maxExponent = 32768; - class guard; + class Guard; }; inline constexpr Number::Number(rep mantissa, int exponent, unchecked) noexcept @@ -308,10 +308,10 @@ abs(Number x) noexcept } // Returns f^n -// Uses a log_2(n) number of mulitiplications +// Uses a log_2(n) number of multiplications Number -power(Number f, unsigned n); +power(Number const& f, unsigned n); // Returns f^(1/d) // Uses Newton–Raphson iterations until the result stops changing @@ -323,12 +323,12 @@ root(Number f, unsigned d); // Returns f^(n/d) Number -power(Number f, unsigned n, unsigned d); +power(Number const& f, unsigned n, unsigned d); // Return 0 if abs(x) < limit, else returns x inline constexpr Number -clip(Number const& x, Number const& limit) noexcept +squelch(Number const& x, Number const& limit) noexcept { if (abs(x) < limit) return Number{}; diff --git a/src/ripple/basics/impl/Number.cpp b/src/ripple/basics/impl/Number.cpp index e59ca5fb63f..10834f08e3b 100644 --- a/src/ripple/basics/impl/Number.cpp +++ b/src/ripple/basics/impl/Number.cpp @@ -33,53 +33,66 @@ using uint128_t = __uint128_t; namespace ripple { -// guard +// Guard -class Number::guard +// The Guard class is used to tempoarily add extra digits of +// preicision to an operation. This enables the final result +// to be correctly rounded to the internal precision of Number. + +class Number::Guard { - std::uint64_t digits_; - std::uint8_t xbit_ : 1; - std::uint8_t sbit_ : 1; // TODO : get rid of + std::uint64_t digits_; // 16 decimal guard digits + std::uint8_t xbit_ : 1; // has a non-zero digit been shifted off the end + std::uint8_t sbit_ : 1; // the sign of the guard digits public: - explicit guard() : digits_{0}, xbit_{0}, sbit_{0} + explicit Guard() : digits_{0}, xbit_{0}, sbit_{0} { } + // set & test the sign bit void set_positive() noexcept; void set_negative() noexcept; bool is_negative() const noexcept; + + // add a digit void push(unsigned d) noexcept; + + // recover a digit unsigned pop() 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. int round() noexcept; }; inline void -Number::guard::set_positive() noexcept +Number::Guard::set_positive() noexcept { sbit_ = 0; } inline void -Number::guard::set_negative() noexcept +Number::Guard::set_negative() noexcept { sbit_ = 1; } inline bool -Number::guard::is_negative() const noexcept +Number::Guard::is_negative() const noexcept { return sbit_ == 1; } inline void -Number::guard::push(unsigned d) noexcept +Number::Guard::push(unsigned d) noexcept { xbit_ = xbit_ || (digits_ & 0x0000'0000'0000'000F) != 0; digits_ >>= 4; @@ -87,7 +100,7 @@ Number::guard::push(unsigned d) noexcept } inline unsigned -Number::guard::pop() noexcept +Number::Guard::pop() noexcept { unsigned d = (digits_ & 0xF000'0000'0000'0000) >> 60; digits_ <<= 4; @@ -95,7 +108,7 @@ Number::guard::pop() noexcept } int -Number::guard::round() noexcept +Number::Guard::round() noexcept { if (digits_ > 0x5000'0000'0000'0000) return 1; @@ -127,10 +140,12 @@ Number::normalize() m *= 10; --exponent_; } + Guard g; while (m > maxMantissa) { if (exponent_ >= maxExponent) throw std::overflow_error("Number::normalize 1"); + g.push(m % 10); m /= 10; ++exponent_; } @@ -141,6 +156,16 @@ Number::normalize() return; } + auto r = g.round(); + if (r == 1 || (r == 0 && (mantissa_ & 1) == 1)) + { + ++mantissa_; + if (mantissa_ > maxMantissa) + { + mantissa_ /= 10; + ++exponent_; + } + } if (exponent_ > maxExponent) throw std::overflow_error("Number::normalize 2"); @@ -180,7 +205,7 @@ Number::operator+=(Number const& y) ym = -ym; yn = -1; } - guard g; + Guard g; if (xe < ye) { if (xn == -1) @@ -261,7 +286,6 @@ Number::operator+=(Number const& y) } mantissa_ = xm * xn; exponent_ = xe; - assert(isnormal()); return *this; } @@ -295,7 +319,7 @@ Number::operator*=(Number const& y) auto zm = uint128_t(xm) * uint128_t(ym); auto ze = xe + ye; auto zn = xn * yn; - guard g; + Guard g; while (zm > maxMantissa) { g.push(static_cast(zm % 10)); @@ -379,7 +403,7 @@ Number::operator rep() const { rep drops = mantissa_; int offset = exponent_; - guard g; + Guard g; if (drops != 0) { if (drops < 0) @@ -395,7 +419,7 @@ Number::operator rep() const for (; offset > 0; --offset) { if (drops > std::numeric_limits::max() / 10) - throw std::runtime_error("Number::operator rep() overflow"); + throw std::overflow_error("Number::operator rep() overflow"); drops *= 10; } auto r = g.round(); @@ -505,10 +529,10 @@ to_string(Number const& amount) } // Returns f^n -// Uses a log_2(n) number of mulitiplications +// Uses a log_2(n) number of multiplications Number -power(Number f, unsigned n) +power(Number const& f, unsigned n) { if (n == 0) return one; @@ -525,6 +549,11 @@ power(Number f, unsigned n) // Uses Newton–Raphson iterations until the result stops changing // to find the non-negative root of the polynomial g(x) = x^d - f +// This function, and power(Number f, unsigned n, unsigned d) +// treat corner cases such as 0 roots as advised by Annex F of +// the C standard, which itself is consistent with the IEEE +// floating point standards. + Number root(Number f, unsigned d) { @@ -590,10 +619,48 @@ root(Number f, unsigned d) return Number{r.mantissa(), r.exponent() + e / di}; } +Number +root2(Number f) +{ + if (f == one) + return f; + if (f < Number{}) + throw std::overflow_error("Number::root nan"); + if (f == Number{}) + return f; + + // Scale f into the range (0, 1) such that f's exponent is a multiple of d + auto e = f.exponent() + 16; + if (e % 2 != 0) + ++e; + f = Number{f.mantissa(), f.exponent() - e}; // f /= 10^e; + + // Quadratic least squares curve fit of f^(1/d) in the range [0, 1] + auto const D = 105; + auto const a0 = 18; + auto const a1 = 144; + auto const a2 = -60; + Number r = ((Number{a2} * f + Number{a1}) * f + Number{a0}) / Number{D}; + + // Newton–Raphson iteration of f^(1/2) with initial guess r + // halt when r stops changing, checking for bouncing on the last iteration + Number rm1{}; + Number rm2{}; + do + { + rm2 = rm1; + rm1 = r; + r = (r + f / r) / Number(2); + } while (r != rm1 && r != rm2); + + // return r * 10^(e/2) to reverse scaling + return Number{r.mantissa(), r.exponent() + e / 2}; +} + // Returns f^(n/d) Number -power(Number f, unsigned n, unsigned d) +power(Number const& f, unsigned n, unsigned d) { if (f == one) return f; @@ -606,9 +673,8 @@ power(Number f, unsigned n, unsigned d) return one; if (abs(f) < one) return Number{}; - if (abs(f) > one) - throw std::overflow_error("Number::power infinity"); - throw std::overflow_error("Number::power nan"); + // abs(f) > one + throw std::overflow_error("Number::power infinity"); } if (n == 0) return one; diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index b605ed434c6..b5425a7bc06 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -21,6 +21,8 @@ #include #include #include +#include +#include namespace ripple { @@ -42,77 +44,267 @@ class Number_test : public beast::unit_test::suite BEAST_EXPECT(z == -z); } + void + test_limits() + { + testcase("test_limits"); + bool caught = false; + try + { + Number x{10'000'000'000'000'000, 32768}; + } + catch (std::overflow_error const&) + { + caught = true; + } + BEAST_EXPECT(caught); + Number x{10'000'000'000'000'000, 32767}; + BEAST_EXPECT((x == Number{1'000'000'000'000'000, 32768})); + Number z{1'000'000'000'000'000, -32769}; + BEAST_EXPECT(z == Number{}); + Number y{1'000'000'000'000'001'500, 32000}; + BEAST_EXPECT((y == Number{1'000'000'000'000'002, 32003})); + Number m{std::numeric_limits::min()}; + BEAST_EXPECT((m == Number{-9'223'372'036'854'776, 3})); + Number M{std::numeric_limits::max()}; + BEAST_EXPECT((M == Number{9'223'372'036'854'776, 3})); + caught = false; + try + { + Number q{99'999'999'999'999'999, 32767}; + } + catch (std::overflow_error const&) + { + caught = true; + } + BEAST_EXPECT(caught); + } + void test_add() { testcase("test_add"); - Number x[]{ - Number{1'000'000'000'000'000, -15}, - Number{-1'000'000'000'000'000, -15}, - Number{-1'000'000'000'000'000, -15}, - Number{-6'555'555'555'555'555, -29}}; - Number y[]{ - Number{6'555'555'555'555'555, -29}, - Number{-6'555'555'555'555'555, -29}, - Number{6'555'555'555'555'555, -29}, - Number{1'000'000'000'000'000, -15}}; - Number z[]{ - Number{1'000'000'000'000'066, -15}, - Number{-1'000'000'000'000'066, -15}, - Number{-9'999'999'999'999'344, -16}, - Number{9'999'999'999'999'344, -16}}; - for (unsigned i = 0; i < std::size(x); ++i) + using Case = std::tuple; + Case c[]{ + {Number{1'000'000'000'000'000, -15}, + Number{6'555'555'555'555'555, -29}, + Number{1'000'000'000'000'066, -15}}, + {Number{-1'000'000'000'000'000, -15}, + Number{-6'555'555'555'555'555, -29}, + Number{-1'000'000'000'000'066, -15}}, + {Number{-1'000'000'000'000'000, -15}, + Number{6'555'555'555'555'555, -29}, + Number{-9'999'999'999'999'344, -16}}, + {Number{-6'555'555'555'555'555, -29}, + Number{1'000'000'000'000'000, -15}, + Number{9'999'999'999'999'344, -16}}, + {Number{}, Number{5}, Number{5}}, + {Number{5'555'555'555'555'555, -32768}, + Number{-5'555'555'555'555'554, -32768}, + Number{0}}, + {Number{-9'999'999'999'999'999, -31}, + Number{1'000'000'000'000'000, -15}, + Number{9'999'999'999'999'990, -16}}}; + for (auto const& [x, y, z] : c) + BEAST_EXPECT(x + y == z); + bool caught = false; + try { - BEAST_EXPECT(x[i] + y[i] == z[i]); + Number{9'999'999'999'999'999, 32768} + + Number{5'000'000'000'000'000, 32767}; } + catch (std::overflow_error const&) + { + caught = true; + } + BEAST_EXPECT(caught); } void test_sub() { testcase("test_sub"); - Number x[]{ - Number{1'000'000'000'000'000, -15}, - Number{6'555'555'555'555'555, -29}}; - Number y[]{ - Number{6'555'555'555'555'555, -29}, - Number{1'000'000'000'000'000, -15}}; - Number z[]{ - Number{9'999'999'999'999'344, -16}, - Number{-9'999'999'999'999'344, -16}}; - for (unsigned i = 0; i < std::size(x); ++i) + using Case = std::tuple; + Case c[]{ + {Number{1'000'000'000'000'000, -15}, + Number{6'555'555'555'555'555, -29}, + Number{9'999'999'999'999'344, -16}}, + {Number{6'555'555'555'555'555, -29}, + Number{1'000'000'000'000'000, -15}, + Number{-9'999'999'999'999'344, -16}}, + {Number{1'000'000'000'000'000, -15}, + Number{1'000'000'000'000'000, -15}, + Number{0}}, + {Number{1'000'000'000'000'000, -15}, + Number{1'000'000'000'000'001, -15}, + Number{-1'000'000'000'000'000, -30}}, + {Number{1'000'000'000'000'001, -15}, + Number{1'000'000'000'000'000, -15}, + Number{1'000'000'000'000'000, -30}}}; + for (auto const& [x, y, z] : c) + BEAST_EXPECT(x - y == z); + } + + void + test_mul() + { + testcase("test_mul"); + using Case = std::tuple; + Case c[]{ + {Number{7}, Number{8}, Number{56}}, + {Number{1414213562373095, -15}, + Number{1414213562373095, -15}, + Number{2000000000000000, -15}}, + {Number{-1414213562373095, -15}, + Number{1414213562373095, -15}, + Number{-2000000000000000, -15}}, + {Number{-1414213562373095, -15}, + Number{-1414213562373095, -15}, + Number{2000000000000000, -15}}, + {Number{3214285714285706, -15}, + Number{3111111111111119, -15}, + Number{1000000000000000, -14}}, + {Number{1000000000000000, -32768}, + Number{1000000000000000, -32768}, + Number{0}}}; + for (auto const& [x, y, z] : c) + BEAST_EXPECT(x * y == z); + bool caught = false; + try { - BEAST_EXPECT(x[i] - y[i] == z[i]); + Number{9'999'999'999'999'999, 32768} * + Number{5'000'000'000'000'000, 32767}; } + catch (std::overflow_error const&) + { + caught = true; + } + BEAST_EXPECT(caught); } void test_div() { testcase("test_div"); - Number x[]{Number{1}, Number{1}, Number{0}}; - Number y[]{Number{2}, Number{10}, Number{100}}; - Number z[]{Number{5, -1}, Number{1, -1}, Number{0}}; - for (unsigned i = 0; i < std::size(x); ++i) + using Case = std::tuple; + Case c[]{ + {Number{1}, Number{2}, Number{5, -1}}, + {Number{1}, Number{10}, Number{1, -1}}, + {Number{1}, Number{-10}, Number{-1, -1}}, + {Number{0}, Number{100}, Number{0}}}; + for (auto const& [x, y, z] : c) + BEAST_EXPECT(x / y == z); + bool caught = false; + try + { + Number{1000000000000000, -15} / Number{0}; + } + catch (std::overflow_error const&) { - BEAST_EXPECT(x[i] / y[i] == z[i]); + caught = true; } + BEAST_EXPECT(caught); } void test_root() { testcase("test_root"); - Number x[]{Number{2}, Number{2'000'000}, Number{2, -30}}; - unsigned y[]{2, 2, 2}; - Number z[]{ - Number{1414213562373095, -15}, - Number{1414213562373095, -12}, - Number{1414213562373095, -30}}; - for (unsigned i = 0; i < std::size(x); ++i) + using Case = std::tuple; + Case c[]{ + {Number{2}, 2, Number{1414213562373095, -15}}, + {Number{2'000'000}, 2, Number{1414213562373095, -12}}, + {Number{2, -30}, 2, Number{1414213562373095, -30}}, + {Number{-27}, 3, Number{-3}}, + {Number{1}, 5, Number{1}}, + {Number{-1}, 0, Number{1}}, + {Number{5, -1}, 0, Number{0}}, + {Number{0}, 5, Number{0}}, + {Number{5625, -4}, 2, Number{75, -2}}}; + for (auto const& [x, y, z] : c) + BEAST_EXPECT((root(x, y) == z)); + bool caught = false; + try + { + (void)root(Number{-2}, 0); + } + catch (std::overflow_error const&) + { + caught = true; + } + BEAST_EXPECT(caught); + caught = false; + try + { + (void)root(Number{-2}, 4); + } + catch (std::overflow_error const&) + { + caught = true; + } + BEAST_EXPECT(caught); + } + + void + test_power1() + { + testcase("test_power1"); + using Case = std::tuple; + Case c[]{ + {Number{64}, 0, Number{1}}, + {Number{64}, 1, Number{64}}, + {Number{64}, 2, Number{4096}}, + {Number{-64}, 2, Number{4096}}, + {Number{64}, 3, Number{262144}}, + {Number{-64}, 3, Number{-262144}}}; + for (auto const& [x, y, z] : c) + BEAST_EXPECT((power(x, y) == z)); + } + + void + test_power2() + { + testcase("test_power2"); + using Case = std::tuple; + Case c[]{ + {Number{1}, 3, 7, Number{1}}, + {Number{-1}, 1, 0, Number{1}}, + {Number{-1, -1}, 1, 0, Number{0}}, + {Number{16}, 0, 5, Number{1}}, + {Number{34}, 3, 3, Number{34}}, + {Number{4}, 3, 2, Number{8}}}; + for (auto const& [x, n, d, z] : c) + BEAST_EXPECT((power(x, n, d) == z)); + bool caught = false; + try + { + (void)power(Number{7}, 0, 0); + } + catch (std::overflow_error const&) + { + caught = true; + } + BEAST_EXPECT(caught); + caught = false; + try { - BEAST_EXPECT(root(x[i], y[i]) == z[i]); + (void)power(Number{7}, 1, 0); } + catch (std::overflow_error const&) + { + caught = true; + } + BEAST_EXPECT(caught); + caught = false; + try + { + (void)power(Number{-1, -1}, 3, 2); + } + catch (std::overflow_error const&) + { + caught = true; + } + BEAST_EXPECT(caught); } void @@ -129,102 +321,149 @@ class Number_test : public beast::unit_test::suite STAmount st = xrp; Number n = st; BEAST_EXPECT(XRPAmount{n} == xrp); + IOUAmount x0{0, 0}; + Number y0 = x0; + BEAST_EXPECT((y0 == Number{0})); + IOUAmount z0{y0}; + BEAST_EXPECT(x0 == z0); + XRPAmount xrp0{0}; + Number n0 = xrp0; + BEAST_EXPECT(n0 == Number{0}); + XRPAmount xrp1{n0}; + BEAST_EXPECT(xrp1 == xrp0); } void test_to_integer() { testcase("test_to_integer"); - Number x[]{ - Number{0}, - Number{1}, - Number{2}, - Number{3}, - Number{-1}, - Number{-2}, - Number{-3}, - Number{10}, - Number{99}, - Number{1155}, - Number{9'999'999'999'999'999, 0}, - Number{9'999'999'999'999'999, 1}, - Number{9'999'999'999'999'999, 2}, - Number{-9'999'999'999'999'999, 2}, - Number{15, -1}, - Number{14, -1}, - Number{16, -1}, - Number{25, -1}, - Number{6, -1}, - Number{5, -1}, - Number{4, -1}, - Number{-15, -1}, - Number{-14, -1}, - Number{-16, -1}, - Number{-25, -1}, - Number{-6, -1}, - Number{-5, -1}, - Number{-4, -1}}; - std::int64_t y[]{ - 0, - 1, - 2, - 3, - -1, - -2, - -3, - 10, - 99, - 1155, - 9'999'999'999'999'999, - 99'999'999'999'999'990, - 999'999'999'999'999'900, - -999'999'999'999'999'900, - 2, - 1, - 2, - 2, - 1, - 0, - 0, - -2, - -1, - -2, - -2, - -1, - 0, - 0}; - static_assert(std::size(x) == std::size(y)); - for (unsigned u = 0; u < std::size(x); ++u) - { - auto j = static_cast(x[u]); - BEAST_EXPECT(j == y[u]); - } - } - - void - test_clip() - { - testcase("test_clip"); + using Case = std::tuple; + Case c[]{ + {Number{0}, 0}, + {Number{1}, 1}, + {Number{2}, 2}, + {Number{3}, 3}, + {Number{-1}, -1}, + {Number{-2}, -2}, + {Number{-3}, -3}, + {Number{10}, 10}, + {Number{99}, 99}, + {Number{1155}, 1155}, + {Number{9'999'999'999'999'999, 0}, 9'999'999'999'999'999}, + {Number{9'999'999'999'999'999, 1}, 99'999'999'999'999'990}, + {Number{9'999'999'999'999'999, 2}, 999'999'999'999'999'900}, + {Number{-9'999'999'999'999'999, 2}, -999'999'999'999'999'900}, + {Number{15, -1}, 2}, + {Number{14, -1}, 1}, + {Number{16, -1}, 2}, + {Number{25, -1}, 2}, + {Number{6, -1}, 1}, + {Number{5, -1}, 0}, + {Number{4, -1}, 0}, + {Number{-15, -1}, -2}, + {Number{-14, -1}, -1}, + {Number{-16, -1}, -2}, + {Number{-25, -1}, -2}, + {Number{-6, -1}, -1}, + {Number{-5, -1}, 0}, + {Number{-4, -1}, 0}}; + for (auto const& [x, y] : c) + { + auto j = static_cast(x); + BEAST_EXPECT(j == y); + } + bool caught = false; + try + { + (void)static_cast(Number{9223372036854776, 3}); + } + catch (std::overflow_error const&) + { + caught = true; + } + BEAST_EXPECT(caught); + } + + void + test_squelch() + { + testcase("test_squelch"); Number limit{1, -6}; - BEAST_EXPECT((clip(Number{2, -6}, limit) == Number{2, -6})); - BEAST_EXPECT((clip(Number{1, -6}, limit) == Number{1, -6})); - BEAST_EXPECT((clip(Number{9, -7}, limit) == Number{0})); - BEAST_EXPECT((clip(Number{-2, -6}, limit) == Number{-2, -6})); - BEAST_EXPECT((clip(Number{-1, -6}, limit) == Number{-1, -6})); - BEAST_EXPECT((clip(Number{-9, -7}, limit) == Number{0})); + BEAST_EXPECT((squelch(Number{2, -6}, limit) == Number{2, -6})); + BEAST_EXPECT((squelch(Number{1, -6}, limit) == Number{1, -6})); + BEAST_EXPECT((squelch(Number{9, -7}, limit) == Number{0})); + BEAST_EXPECT((squelch(Number{-2, -6}, limit) == Number{-2, -6})); + BEAST_EXPECT((squelch(Number{-1, -6}, limit) == Number{-1, -6})); + BEAST_EXPECT((squelch(Number{-9, -7}, limit) == Number{0})); + } + + void + testToString() + { + testcase("testToString"); + BEAST_EXPECT(to_string(Number(-2, 0)) == "-2"); + BEAST_EXPECT(to_string(Number(0, 0)) == "0"); + BEAST_EXPECT(to_string(Number(2, 0)) == "2"); + BEAST_EXPECT(to_string(Number(25, -3)) == "0.025"); + BEAST_EXPECT(to_string(Number(-25, -3)) == "-0.025"); + BEAST_EXPECT(to_string(Number(25, 1)) == "250"); + BEAST_EXPECT(to_string(Number(-25, 1)) == "-250"); + BEAST_EXPECT(to_string(Number(2, 20)) == "2000000000000000e5"); + BEAST_EXPECT(to_string(Number(-2, -20)) == "-2000000000000000e-35"); + } + + void + test_relationals() + { + testcase("test_relationals"); + BEAST_EXPECT(!(Number{100} < Number{10})); + BEAST_EXPECT(Number{100} > Number{10}); + BEAST_EXPECT(Number{100} >= Number{10}); + BEAST_EXPECT(!(Number{100} <= Number{10})); + } + + void + test_stream() + { + testcase("test_stream"); + Number x{100}; + std::ostringstream os; + os << x; + BEAST_EXPECT(os.str() == to_string(x)); + } + + void + test_inc_dec() + { + testcase("test_inc_dec"); + Number x{100}; + Number y = +x; + BEAST_EXPECT(x == y); + BEAST_EXPECT(x++ == y); + BEAST_EXPECT(x == Number{101}); + BEAST_EXPECT(x-- == Number{101}); + BEAST_EXPECT(x == y); } void run() override { testZero(); + test_limits(); test_add(); test_sub(); + test_mul(); test_div(); test_root(); + test_power1(); + test_power2(); testConversions(); test_to_integer(); - test_clip(); + test_squelch(); + testToString(); + test_relationals(); + test_stream(); + test_inc_dec(); } }; From 1746e28dabffc5c8e44866d58f25d8109e8b32b2 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Wed, 1 Jun 2022 15:51:00 -0400 Subject: [PATCH 06/13] Use Number for IOUAmount and STAmount arithmetic * Guarded by amendment fixUniversalNumber * Produces slightly better accuracy in some computations. --- src/ripple/app/misc/impl/TxQ.cpp | 2 + src/ripple/app/paths/impl/AmountSpec.h | 4 +- src/ripple/app/tx/impl/Transactor.cpp | 1 + src/ripple/app/tx/impl/apply.cpp | 1 + src/ripple/basics/IOUAmount.h | 166 ++++++++++++++------ src/ripple/basics/Number.h | 12 -- src/ripple/basics/impl/IOUAmount.cpp | 206 +++++++------------------ src/ripple/protocol/Feature.h | 6 +- src/ripple/protocol/STAmount.h | 6 + src/ripple/protocol/impl/Feature.cpp | 1 + src/ripple/protocol/impl/STAmount.cpp | 29 ++++ src/test/app/Offer_test.cpp | 2 +- src/test/app/Taker_test.cpp | 5 +- src/test/app/TrustAndBalance_test.cpp | 4 +- 14 files changed, 228 insertions(+), 217 deletions(-) diff --git a/src/ripple/app/misc/impl/TxQ.cpp b/src/ripple/app/misc/impl/TxQ.cpp index 8424b1d29af..bf278970bb8 100644 --- a/src/ripple/app/misc/impl/TxQ.cpp +++ b/src/ripple/app/misc/impl/TxQ.cpp @@ -293,6 +293,7 @@ TxQ::MaybeTx::apply(Application& app, OpenView& view, beast::Journal j) // If the rules or flags change, preflight again assert(pfresult); STAmountSO stAmountSO{view.rules().enabled(fixSTAmountCanonicalize)}; + NumberSO stNumberSO{view.rules().enabled(fixUniversalNumber)}; if (pfresult->rules != view.rules() || pfresult->flags != flags) { @@ -717,6 +718,7 @@ TxQ::apply( beast::Journal j) { STAmountSO stAmountSO{view.rules().enabled(fixSTAmountCanonicalize)}; + NumberSO stNumberSO{view.rules().enabled(fixUniversalNumber)}; // See if the transaction paid a high enough fee that it can go straight // into the ledger. diff --git a/src/ripple/app/paths/impl/AmountSpec.h b/src/ripple/app/paths/impl/AmountSpec.h index 927c3d72fe5..ca814c7b3ac 100644 --- a/src/ripple/app/paths/impl/AmountSpec.h +++ b/src/ripple/app/paths/impl/AmountSpec.h @@ -36,7 +36,7 @@ struct AmountSpec union { XRPAmount xrp; - IOUAmount iou; + IOUAmount iou = {}; }; std::optional issuer; std::optional currency; @@ -64,7 +64,7 @@ struct EitherAmount union { - IOUAmount iou; + IOUAmount iou = {}; XRPAmount xrp; }; diff --git a/src/ripple/app/tx/impl/Transactor.cpp b/src/ripple/app/tx/impl/Transactor.cpp index 4c1a7e726cd..1eecccedb25 100644 --- a/src/ripple/app/tx/impl/Transactor.cpp +++ b/src/ripple/app/tx/impl/Transactor.cpp @@ -782,6 +782,7 @@ Transactor::operator()() JLOG(j_.trace()) << "apply: " << ctx_.tx.getTransactionID(); STAmountSO stAmountSO{view().rules().enabled(fixSTAmountCanonicalize)}; + NumberSO stNumberSO{view().rules().enabled(fixUniversalNumber)}; #ifdef DEBUG { diff --git a/src/ripple/app/tx/impl/apply.cpp b/src/ripple/app/tx/impl/apply.cpp index cc1e792c014..5144e05fea3 100644 --- a/src/ripple/app/tx/impl/apply.cpp +++ b/src/ripple/app/tx/impl/apply.cpp @@ -114,6 +114,7 @@ apply( beast::Journal j) { STAmountSO stAmountSO{view.rules().enabled(fixSTAmountCanonicalize)}; + NumberSO stNumberSO{view.rules().enabled(fixUniversalNumber)}; auto pfresult = preflight(app, view.rules(), tx, flags, j); auto pcresult = preclaim(pfresult, app, view); diff --git a/src/ripple/basics/IOUAmount.h b/src/ripple/basics/IOUAmount.h index 7e9e50d7ee9..c3ef1340a76 100644 --- a/src/ripple/basics/IOUAmount.h +++ b/src/ripple/basics/IOUAmount.h @@ -20,6 +20,8 @@ #ifndef RIPPLE_BASICS_IOUAMOUNT_H_INCLUDED #define RIPPLE_BASICS_IOUAMOUNT_H_INCLUDED +#include +#include #include #include #include @@ -56,84 +58,119 @@ class IOUAmount : private boost::totally_ordered, public: IOUAmount() = default; - IOUAmount(IOUAmount const& other) = default; - IOUAmount& - operator=(IOUAmount const& other) = default; - - IOUAmount(beast::Zero) - { - *this = beast::zero; - } + explicit IOUAmount(Number const& other); + IOUAmount(beast::Zero); + IOUAmount(std::int64_t mantissa, int exponent); - IOUAmount(std::int64_t mantissa, int exponent) - : mantissa_(mantissa), exponent_(exponent) - { - normalize(); - } + IOUAmount& operator=(beast::Zero); - IOUAmount& operator=(beast::Zero) - { - // The -100 is used to allow 0 to sort less than small positive values - // which will have a large negative exponent. - mantissa_ = 0; - exponent_ = -100; - return *this; - } + operator Number() const; IOUAmount& operator+=(IOUAmount const& other); IOUAmount& - operator-=(IOUAmount const& other) - { - *this += -other; - return *this; - } + operator-=(IOUAmount const& other); IOUAmount - operator-() const - { - return {-mantissa_, exponent_}; - } + operator-() const; bool - operator==(IOUAmount const& other) const - { - return exponent_ == other.exponent_ && mantissa_ == other.mantissa_; - } + operator==(IOUAmount const& other) const; bool operator<(IOUAmount const& other) const; /** Returns true if the amount is not zero */ - explicit operator bool() const noexcept - { - return mantissa_ != 0; - } + explicit operator bool() const noexcept; /** Return the sign of the amount */ int - signum() const noexcept - { - return (mantissa_ < 0) ? -1 : (mantissa_ ? 1 : 0); - } + signum() const noexcept; int - exponent() const noexcept - { - return exponent_; - } + exponent() const noexcept; std::int64_t - mantissa() const noexcept - { - return mantissa_; - } + mantissa() const noexcept; static IOUAmount minPositiveAmount(); }; +inline IOUAmount::IOUAmount(beast::Zero) +{ + *this = beast::zero; +} + +inline IOUAmount::IOUAmount(std::int64_t mantissa, int exponent) + : mantissa_(mantissa), exponent_(exponent) +{ + normalize(); +} + +inline IOUAmount& IOUAmount::operator=(beast::Zero) +{ + // The -100 is used to allow 0 to sort less than small positive values + // which will have a large negative exponent. + mantissa_ = 0; + exponent_ = -100; + return *this; +} + +inline IOUAmount::operator Number() const +{ + return Number{mantissa_, exponent_}; +} + +inline IOUAmount& +IOUAmount::operator-=(IOUAmount const& other) +{ + *this += -other; + return *this; +} + +inline IOUAmount +IOUAmount::operator-() const +{ + return {-mantissa_, exponent_}; +} + +inline bool +IOUAmount::operator==(IOUAmount const& other) const +{ + return exponent_ == other.exponent_ && mantissa_ == other.mantissa_; +} + +inline bool +IOUAmount::operator<(IOUAmount const& other) const +{ + return Number{*this} < Number{other}; +} + +inline IOUAmount::operator bool() const noexcept +{ + return mantissa_ != 0; +} + +inline int +IOUAmount::signum() const noexcept +{ + return (mantissa_ < 0) ? -1 : (mantissa_ ? 1 : 0); +} + +inline int +IOUAmount::exponent() const noexcept +{ + return exponent_; +} + +inline std::int64_t +IOUAmount::mantissa() const noexcept +{ + return mantissa_; +} + std::string to_string(IOUAmount const& amount); @@ -149,6 +186,35 @@ mulRatio( std::uint32_t den, bool roundUp); +// Since IOUAmount and STAmount do not have access to a ledger, this +// is needed to put low-level routines on an amendment switch. Only +// transactions need to use this switchover. Outside of a transaction +// it's safe to unconditionally use the new behavior. +extern LocalValue stNumberSwitchover; + +/** RAII class to set and restore the Number switchover. + */ + +class NumberSO +{ + bool saved_; + +public: + ~NumberSO() + { + *stNumberSwitchover = saved_; + } + + NumberSO(NumberSO const&) = delete; + NumberSO& + operator=(NumberSO const&) = delete; + + explicit NumberSO(bool v) : saved_(*stNumberSwitchover) + { + *stNumberSwitchover = v; + } +}; + } // namespace ripple #endif diff --git a/src/ripple/basics/Number.h b/src/ripple/basics/Number.h index 92b99bf0145..ead0e432186 100644 --- a/src/ripple/basics/Number.h +++ b/src/ripple/basics/Number.h @@ -20,7 +20,6 @@ #ifndef RIPPLE_BASICS_NUMBER_H_INCLUDED #define RIPPLE_BASICS_NUMBER_H_INCLUDED -#include #include #include #include @@ -51,7 +50,6 @@ class Number explicit Number(rep mantissa, int exponent); explicit constexpr Number(rep mantissa, int exponent, unchecked) noexcept; - Number(IOUAmount const& x); Number(XRPAmount const& x); constexpr rep @@ -82,7 +80,6 @@ class Number Number& operator/=(Number const& x); - explicit operator IOUAmount() const; explicit operator XRPAmount() const; // round to nearest, even on tie explicit operator rep() const; // round to nearest, even on tie @@ -184,10 +181,6 @@ inline Number::Number(rep mantissa) : Number{mantissa, 0} { } -inline Number::Number(IOUAmount const& x) : Number{x.mantissa(), x.exponent()} -{ -} - inline Number::Number(XRPAmount const& x) : Number{x.drops()} { } @@ -286,11 +279,6 @@ operator/(Number const& x, Number const& y) return z; } -inline Number::operator IOUAmount() const -{ - return IOUAmount{mantissa(), exponent()}; -} - inline constexpr bool Number::isnormal() const noexcept { diff --git a/src/ripple/basics/impl/IOUAmount.cpp b/src/ripple/basics/impl/IOUAmount.cpp index 27254662347..1fa5e4fd22d 100644 --- a/src/ripple/basics/impl/IOUAmount.cpp +++ b/src/ripple/basics/impl/IOUAmount.cpp @@ -27,12 +27,14 @@ namespace ripple { +LocalValue stNumberSwitchover(true); + /* The range for the mantissa when normalized */ -static std::int64_t const minMantissa = 1000000000000000ull; -static std::int64_t const maxMantissa = 9999999999999999ull; +static std::int64_t constexpr minMantissa = 1000000000000000ull; +static std::int64_t constexpr maxMantissa = 9999999999999999ull; /* The range for the exponent when normalized */ -static int const minExponent = -96; -static int const maxExponent = 80; +static int constexpr minExponent = -96; +static int constexpr maxExponent = 80; IOUAmount IOUAmount::minPositiveAmount() @@ -43,6 +45,17 @@ IOUAmount::minPositiveAmount() void IOUAmount::normalize() { + if (*stNumberSwitchover) + { + Number v{mantissa_, exponent_}; + mantissa_ = v.mantissa(); + exponent_ = v.exponent(); + if (exponent_ > maxExponent) + Throw("value overflow"); + if (exponent_ < minExponent) + *this = beast::zero; + return; + } if (mantissa_ == 0) { *this = beast::zero; @@ -82,166 +95,67 @@ IOUAmount::normalize() mantissa_ = -mantissa_; } +IOUAmount::IOUAmount(Number const& other) + : mantissa_(other.mantissa()), exponent_(other.exponent()) +{ + if (exponent_ > maxExponent) + Throw("value overflow"); + if (exponent_ < minExponent) + *this = beast::zero; +} + IOUAmount& IOUAmount::operator+=(IOUAmount const& other) { - if (other == beast::zero) - return *this; - - if (*this == beast::zero) + if (*stNumberSwitchover) { - *this = other; - return *this; + *this = IOUAmount{Number{*this} + Number{other}}; } - - auto m = other.mantissa_; - auto e = other.exponent_; - - while (exponent_ < e) - { - mantissa_ /= 10; - ++exponent_; - } - - while (e < exponent_) + else { - m /= 10; - ++e; - } - - // This addition cannot overflow an std::int64_t but we may throw from - // normalize if the result isn't representable. - mantissa_ += m; + if (other == beast::zero) + return *this; - if (mantissa_ >= -10 && mantissa_ <= 10) - { - *this = beast::zero; - return *this; - } - - normalize(); - - return *this; -} + if (*this == beast::zero) + { + *this = other; + return *this; + } -bool -IOUAmount::operator<(IOUAmount const& other) const -{ - // If the two amounts have different signs (zero is treated as positive) - // then the comparison is true iff the left is negative. - bool const lneg = mantissa_ < 0; - bool const rneg = other.mantissa_ < 0; + auto m = other.mantissa_; + auto e = other.exponent_; - if (lneg != rneg) - return lneg; + while (exponent_ < e) + { + mantissa_ /= 10; + ++exponent_; + } - // Both have same sign and the left is zero: the right must be - // greater than 0. - if (mantissa_ == 0) - return other.mantissa_ > 0; + while (e < exponent_) + { + m /= 10; + ++e; + } - // Both have same sign, the right is zero and the left is non-zero. - if (other.mantissa_ == 0) - return false; + // This addition cannot overflow an std::int64_t but we may throw from + // normalize if the result isn't representable. + mantissa_ += m; - // Both have the same sign, compare by exponents: - if (exponent_ > other.exponent_) - return lneg; - if (exponent_ < other.exponent_) - return !lneg; + if (mantissa_ >= -10 && mantissa_ <= 10) + { + *this = beast::zero; + return *this; + } - // If equal exponents, compare mantissas - return mantissa_ < other.mantissa_; + normalize(); + } + return *this; } std::string to_string(IOUAmount const& amount) { - // keep full internal accuracy, but make more human friendly if possible - if (amount == beast::zero) - return "0"; - - int const exponent = amount.exponent(); - auto mantissa = amount.mantissa(); - - // Use scientific notation for exponents that are too small or too large - if (((exponent != 0) && ((exponent < -25) || (exponent > -5)))) - { - std::string ret = std::to_string(mantissa); - ret.append(1, 'e'); - ret.append(std::to_string(exponent)); - return ret; - } - - bool negative = false; - - if (mantissa < 0) - { - mantissa = -mantissa; - negative = true; - } - - assert(exponent + 43 > 0); - - size_t const pad_prefix = 27; - size_t const pad_suffix = 23; - - std::string const raw_value(std::to_string(mantissa)); - std::string val; - - val.reserve(raw_value.length() + pad_prefix + pad_suffix); - val.append(pad_prefix, '0'); - val.append(raw_value); - val.append(pad_suffix, '0'); - - size_t const offset(exponent + 43); - - auto pre_from(val.begin()); - auto const pre_to(val.begin() + offset); - - auto const post_from(val.begin() + offset); - auto post_to(val.end()); - - // Crop leading zeroes. Take advantage of the fact that there's always a - // fixed amount of leading zeroes and skip them. - if (std::distance(pre_from, pre_to) > pad_prefix) - pre_from += pad_prefix; - - assert(post_to >= post_from); - - pre_from = std::find_if(pre_from, pre_to, [](char c) { return c != '0'; }); - - // Crop trailing zeroes. Take advantage of the fact that there's always a - // fixed amount of trailing zeroes and skip them. - if (std::distance(post_from, post_to) > pad_suffix) - post_to -= pad_suffix; - - assert(post_to >= post_from); - - post_to = std::find_if( - std::make_reverse_iterator(post_to), - std::make_reverse_iterator(post_from), - [](char c) { return c != '0'; }) - .base(); - - std::string ret; - - if (negative) - ret.append(1, '-'); - - // Assemble the output: - if (pre_from == pre_to) - ret.append(1, '0'); - else - ret.append(pre_from, pre_to); - - if (post_to != post_from) - { - ret.append(1, '.'); - ret.append(post_from, post_to); - } - - return ret; + return to_string(Number{amount}); } IOUAmount diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index b3e1dba78bd..6be2d4dfb68 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -74,7 +74,7 @@ namespace detail { // Feature.cpp. Because it's only used to reserve storage, and determine how // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than // the actual number of amendments. A LogicError on startup will verify this. -static constexpr std::size_t numFeatures = 55; +static constexpr std::size_t numFeatures = 56; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -340,8 +340,12 @@ extern uint256 const featureNonFungibleTokensV1_1; extern uint256 const fixTrustLinesToSelf; extern uint256 const fixRemoveNFTokenAutoTrustLine; extern uint256 const featureImmediateOfferKilled; +<<<<<<< HEAD extern uint256 const featureDisallowIncoming; extern uint256 const featureXRPFees; +======= +extern uint256 const fixUniversalNumber; +>>>>>>> Use Number for IOUAmount and STAmount arithmetic } // namespace ripple diff --git a/src/ripple/protocol/STAmount.h b/src/ripple/protocol/STAmount.h index b6e0e3046ff..0b9ca953f1a 100644 --- a/src/ripple/protocol/STAmount.h +++ b/src/ripple/protocol/STAmount.h @@ -277,7 +277,13 @@ class STAmount final : public STBase, public CountedObject STBase* move(std::size_t n, void* buf) override; + STAmount& + operator=(IOUAmount const& iou); + friend class detail::STVar; + + friend STAmount + operator+(STAmount const& v1, STAmount const& v2); }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 2e141c11fd1..d650950da4b 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -452,6 +452,7 @@ REGISTER_FIX (fixRemoveNFTokenAutoTrustLine, Supported::yes, DefaultVote::yes REGISTER_FEATURE(ImmediateOfferKilled, Supported::yes, DefaultVote::no); REGISTER_FEATURE(DisallowIncoming, Supported::yes, DefaultVote::no); REGISTER_FEATURE(XRPFees, Supported::yes, DefaultVote::no); +REGISTER_FIX (fixUniversalNumber, Supported::yes, DefaultVote::yes); // The following amendments have been active for at least two years. Their // pre-amendment code has been removed and the identifiers are deprecated. diff --git a/src/ripple/protocol/impl/STAmount.cpp b/src/ripple/protocol/impl/STAmount.cpp index d764eb00d33..51e8adb568d 100644 --- a/src/ripple/protocol/impl/STAmount.cpp +++ b/src/ripple/protocol/impl/STAmount.cpp @@ -339,6 +339,19 @@ STAmount::iou() const return {mantissa, exponent}; } +STAmount& +STAmount::operator=(IOUAmount const& iou) +{ + assert(mIsNative == false); + mOffset = iou.exponent(); + mIsNegative = iou < beast::zero; + if (mIsNegative) + mValue = static_cast(-iou.mantissa()); + else + mValue = static_cast(iou.mantissa()); + return *this; +} + //------------------------------------------------------------------------------ // // Operators @@ -382,6 +395,13 @@ operator+(STAmount const& v1, STAmount const& v2) if (v1.native()) return {v1.getFName(), getSNValue(v1) + getSNValue(v2)}; + if (*stNumberSwitchover) + { + auto x = v1; + x = v1.iou() + v2.iou(); + return x; + } + int ov1 = v1.exponent(), ov2 = v2.exponent(); std::int64_t vv1 = static_cast(v1.mantissa()); std::int64_t vv2 = static_cast(v2.mantissa()); @@ -733,6 +753,12 @@ STAmount::canonicalize() mIsNative = false; + if (*stNumberSwitchover) + { + *this = iou(); + return; + } + if (mValue == 0) { mOffset = -100; @@ -1170,6 +1196,9 @@ multiply(STAmount const& v1, STAmount const& v2, Issue const& issue) return STAmount(v1.getFName(), minV * maxV); } + if (*stNumberSwitchover) + return {IOUAmount{Number{v1} * Number{v2}}, issue}; + std::uint64_t value1 = v1.mantissa(); std::uint64_t value2 = v2.mantissa(); int offset1 = v1.exponent(); diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index 9f6e165bc16..fc9a38cd28e 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -2122,7 +2122,7 @@ class Offer_test : public beast::unit_test::suite jrr = ledgerEntryState(env, bob, gw, "USD"); BEAST_EXPECT( jrr[jss::node][sfBalance.fieldName][jss::value] == - "-0.966500000033334"); + "-0.9665000000333333"); } void diff --git a/src/test/app/Taker_test.cpp b/src/test/app/Taker_test.cpp index ff0152408ff..c7474b6798e 100644 --- a/src/test/app/Taker_test.cpp +++ b/src/test/app/Taker_test.cpp @@ -905,6 +905,7 @@ class Taker_test : public beast::unit_test::suite { testcase("IOU to IOU"); + NumberSO stNumberSO{true}; Quality q1 = get_quality("1", "1"); // Highly exaggerated 50% transfer rate for the input and output: @@ -937,7 +938,7 @@ class Taker_test : public beast::unit_test::suite q1, {"4", "4"}, "4", - {"2.666666666666666", "2.666666666666666"}, + {"2.666666666666667", "2.666666666666667"}, eur(), usd(), rate, @@ -993,7 +994,7 @@ class Taker_test : public beast::unit_test::suite q1, {"2", "2"}, "10", - {"1.666666666666666", "1.666666666666666"}, + {"1.666666666666667", "1.666666666666667"}, eur(), usd(), rate, diff --git a/src/test/app/TrustAndBalance_test.cpp b/src/test/app/TrustAndBalance_test.cpp index e67f3ba327f..0a94138ad71 100644 --- a/src/test/app/TrustAndBalance_test.cpp +++ b/src/test/app/TrustAndBalance_test.cpp @@ -411,13 +411,11 @@ class TrustAndBalance_test : public beast::unit_test::suite if (with_rate) { - // 65.00000000000001 is correct. - // This is result of limited precision. env.require(balance( alice, STAmount( carol["USD"].issue(), - 6500000000000001ull, + 6500000000000000ull, -14, false, true, From 868109da123671fb7a92527bde27c78e3180f71a Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Fri, 12 Aug 2022 10:33:43 -0400 Subject: [PATCH 07/13] Introduce rounding modes for Number: You can set a thread-local flag to direct Number how to round non-exact results with the syntax: Number::rounding_mode prev_mode = Number::setround(Number::towards_zero); This flag will stay in effect for this thread only until another call to setround. The previously set rounding mode is returned. You can also retrieve the current rounding mode with: Number::rounding_mode current_mode = Number::getround(); The available rounding modes are: * to_nearest : Rounds to nearest representable value. On tie, rounds to even. * towards_zero : Rounds towards zero. * downward : Rounds towards negative infinity. * upward : Rounds towards positive infinity. The default rounding mode is to_nearest. --- src/ripple/basics/Number.h | 13 ++ src/ripple/basics/impl/Number.cpp | 53 ++++++-- src/test/basics/Number_test.cpp | 199 +++++++++++++++++++++++++----- 3 files changed, 226 insertions(+), 39 deletions(-) diff --git a/src/ripple/basics/Number.h b/src/ripple/basics/Number.h index ead0e432186..cc009fa2ed4 100644 --- a/src/ripple/basics/Number.h +++ b/src/ripple/basics/Number.h @@ -149,7 +149,17 @@ class Number return os << to_string(x); } + // Thread local rounding control. Default is to_nearest + enum rounding_mode { to_nearest, towards_zero, downward, upward }; + static rounding_mode + getround(); + // Returns previously set mode + static rounding_mode + setround(rounding_mode mode); + private: + static thread_local rounding_mode mode_; + void normalize(); constexpr bool @@ -308,6 +318,9 @@ power(Number const& f, unsigned n); Number root(Number f, unsigned d); +Number +root2(Number f); + // Returns f^(n/d) Number diff --git a/src/ripple/basics/impl/Number.cpp b/src/ripple/basics/impl/Number.cpp index 10834f08e3b..0690be0773f 100644 --- a/src/ripple/basics/impl/Number.cpp +++ b/src/ripple/basics/impl/Number.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #ifdef _MSVC_LANG #include @@ -33,6 +34,20 @@ using uint128_t = __uint128_t; namespace ripple { +thread_local Number::rounding_mode Number::mode_ = Number::to_nearest; + +Number::rounding_mode +Number::getround() +{ + return mode_; +} + +Number::rounding_mode +Number::setround(rounding_mode mode) +{ + return std::exchange(mode_, mode); +} + // Guard // The Guard class is used to tempoarily add extra digits of @@ -107,16 +122,40 @@ Number::Guard::pop() noexcept return d; } +// Returns: +// -1 if Guard is less than half +// 0 if Guard is exactly half +// 1 if Guard is greater than half int Number::Guard::round() noexcept { - if (digits_ > 0x5000'0000'0000'0000) - return 1; - if (digits_ < 0x5000'0000'0000'0000) - return -1; - if (xbit_) - return 1; - return 0; + auto mode = Number::getround(); + switch (mode) + { + case to_nearest: + if (digits_ > 0x5000'0000'0000'0000) + return 1; + if (digits_ < 0x5000'0000'0000'0000) + return -1; + if (xbit_) + return 1; + return 0; + case towards_zero: + return -1; + case downward: + if (sbit_) + { + if (digits_ > 0 || xbit_) + return 1; + } + return -1; + case upward: + if (sbit_) + return -1; + if (digits_ > 0 || xbit_) + return 1; + return -1; + } } // Number diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index b5425a7bc06..d7bd8264878 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -26,6 +26,24 @@ namespace ripple { +class saveNumberRoundMode +{ + Number::rounding_mode mode_; + +public: + ~saveNumberRoundMode() + { + Number::setround(mode_); + } + explicit saveNumberRoundMode(Number::rounding_mode mode) noexcept + : mode_{mode} + { + } + saveNumberRoundMode(saveNumberRoundMode const&) = delete; + saveNumberRoundMode& + operator=(saveNumberRoundMode const&) = delete; +}; + class Number_test : public beast::unit_test::suite { public: @@ -338,39 +356,156 @@ class Number_test : public beast::unit_test::suite { testcase("test_to_integer"); using Case = std::tuple; - Case c[]{ - {Number{0}, 0}, - {Number{1}, 1}, - {Number{2}, 2}, - {Number{3}, 3}, - {Number{-1}, -1}, - {Number{-2}, -2}, - {Number{-3}, -3}, - {Number{10}, 10}, - {Number{99}, 99}, - {Number{1155}, 1155}, - {Number{9'999'999'999'999'999, 0}, 9'999'999'999'999'999}, - {Number{9'999'999'999'999'999, 1}, 99'999'999'999'999'990}, - {Number{9'999'999'999'999'999, 2}, 999'999'999'999'999'900}, - {Number{-9'999'999'999'999'999, 2}, -999'999'999'999'999'900}, - {Number{15, -1}, 2}, - {Number{14, -1}, 1}, - {Number{16, -1}, 2}, - {Number{25, -1}, 2}, - {Number{6, -1}, 1}, - {Number{5, -1}, 0}, - {Number{4, -1}, 0}, - {Number{-15, -1}, -2}, - {Number{-14, -1}, -1}, - {Number{-16, -1}, -2}, - {Number{-25, -1}, -2}, - {Number{-6, -1}, -1}, - {Number{-5, -1}, 0}, - {Number{-4, -1}, 0}}; - for (auto const& [x, y] : c) + saveNumberRoundMode save{Number::setround(Number::to_nearest)}; + { + Case c[]{ + {Number{0}, 0}, + {Number{1}, 1}, + {Number{2}, 2}, + {Number{3}, 3}, + {Number{-1}, -1}, + {Number{-2}, -2}, + {Number{-3}, -3}, + {Number{10}, 10}, + {Number{99}, 99}, + {Number{1155}, 1155}, + {Number{9'999'999'999'999'999, 0}, 9'999'999'999'999'999}, + {Number{9'999'999'999'999'999, 1}, 99'999'999'999'999'990}, + {Number{9'999'999'999'999'999, 2}, 999'999'999'999'999'900}, + {Number{-9'999'999'999'999'999, 2}, -999'999'999'999'999'900}, + {Number{15, -1}, 2}, + {Number{14, -1}, 1}, + {Number{16, -1}, 2}, + {Number{25, -1}, 2}, + {Number{6, -1}, 1}, + {Number{5, -1}, 0}, + {Number{4, -1}, 0}, + {Number{-15, -1}, -2}, + {Number{-14, -1}, -1}, + {Number{-16, -1}, -2}, + {Number{-25, -1}, -2}, + {Number{-6, -1}, -1}, + {Number{-5, -1}, 0}, + {Number{-4, -1}, 0}}; + for (auto const& [x, y] : c) + { + auto j = static_cast(x); + BEAST_EXPECT(j == y); + } + } + auto prev_mode = Number::setround(Number::towards_zero); + BEAST_EXPECT(prev_mode == Number::to_nearest); + { + Case c[]{ + {Number{0}, 0}, + {Number{1}, 1}, + {Number{2}, 2}, + {Number{3}, 3}, + {Number{-1}, -1}, + {Number{-2}, -2}, + {Number{-3}, -3}, + {Number{10}, 10}, + {Number{99}, 99}, + {Number{1155}, 1155}, + {Number{9'999'999'999'999'999, 0}, 9'999'999'999'999'999}, + {Number{9'999'999'999'999'999, 1}, 99'999'999'999'999'990}, + {Number{9'999'999'999'999'999, 2}, 999'999'999'999'999'900}, + {Number{-9'999'999'999'999'999, 2}, -999'999'999'999'999'900}, + {Number{15, -1}, 1}, + {Number{14, -1}, 1}, + {Number{16, -1}, 1}, + {Number{25, -1}, 2}, + {Number{6, -1}, 0}, + {Number{5, -1}, 0}, + {Number{4, -1}, 0}, + {Number{-15, -1}, -1}, + {Number{-14, -1}, -1}, + {Number{-16, -1}, -1}, + {Number{-25, -1}, -2}, + {Number{-6, -1}, 0}, + {Number{-5, -1}, 0}, + {Number{-4, -1}, 0}}; + for (auto const& [x, y] : c) + { + auto j = static_cast(x); + BEAST_EXPECT(j == y); + } + } + prev_mode = Number::setround(Number::downward); + BEAST_EXPECT(prev_mode == Number::towards_zero); + { + Case c[]{ + {Number{0}, 0}, + {Number{1}, 1}, + {Number{2}, 2}, + {Number{3}, 3}, + {Number{-1}, -1}, + {Number{-2}, -2}, + {Number{-3}, -3}, + {Number{10}, 10}, + {Number{99}, 99}, + {Number{1155}, 1155}, + {Number{9'999'999'999'999'999, 0}, 9'999'999'999'999'999}, + {Number{9'999'999'999'999'999, 1}, 99'999'999'999'999'990}, + {Number{9'999'999'999'999'999, 2}, 999'999'999'999'999'900}, + {Number{-9'999'999'999'999'999, 2}, -999'999'999'999'999'900}, + {Number{15, -1}, 1}, + {Number{14, -1}, 1}, + {Number{16, -1}, 1}, + {Number{25, -1}, 2}, + {Number{6, -1}, 0}, + {Number{5, -1}, 0}, + {Number{4, -1}, 0}, + {Number{-15, -1}, -2}, + {Number{-14, -1}, -2}, + {Number{-16, -1}, -2}, + {Number{-25, -1}, -3}, + {Number{-6, -1}, -1}, + {Number{-5, -1}, -1}, + {Number{-4, -1}, -1}}; + for (auto const& [x, y] : c) + { + auto j = static_cast(x); + BEAST_EXPECT(j == y); + } + } + prev_mode = Number::setround(Number::upward); + BEAST_EXPECT(prev_mode == Number::downward); { - auto j = static_cast(x); - BEAST_EXPECT(j == y); + Case c[]{ + {Number{0}, 0}, + {Number{1}, 1}, + {Number{2}, 2}, + {Number{3}, 3}, + {Number{-1}, -1}, + {Number{-2}, -2}, + {Number{-3}, -3}, + {Number{10}, 10}, + {Number{99}, 99}, + {Number{1155}, 1155}, + {Number{9'999'999'999'999'999, 0}, 9'999'999'999'999'999}, + {Number{9'999'999'999'999'999, 1}, 99'999'999'999'999'990}, + {Number{9'999'999'999'999'999, 2}, 999'999'999'999'999'900}, + {Number{-9'999'999'999'999'999, 2}, -999'999'999'999'999'900}, + {Number{15, -1}, 2}, + {Number{14, -1}, 2}, + {Number{16, -1}, 2}, + {Number{25, -1}, 3}, + {Number{6, -1}, 1}, + {Number{5, -1}, 1}, + {Number{4, -1}, 1}, + {Number{-15, -1}, -1}, + {Number{-14, -1}, -1}, + {Number{-16, -1}, -1}, + {Number{-25, -1}, -2}, + {Number{-6, -1}, 0}, + {Number{-5, -1}, 0}, + {Number{-4, -1}, 0}}; + for (auto const& [x, y] : c) + { + auto j = static_cast(x); + BEAST_EXPECT(j == y); + } } bool caught = false; try From 6272a34293e3f8a5e9a8be331251a9f5be905868 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Thu, 15 Sep 2022 08:17:48 -0400 Subject: [PATCH 08/13] Silence warnings --- src/ripple/basics/Number.h | 1 + src/ripple/basics/impl/Number.cpp | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ripple/basics/Number.h b/src/ripple/basics/Number.h index cc009fa2ed4..ef45b3f7795 100644 --- a/src/ripple/basics/Number.h +++ b/src/ripple/basics/Number.h @@ -22,6 +22,7 @@ #include #include +#include #include #include diff --git a/src/ripple/basics/impl/Number.cpp b/src/ripple/basics/impl/Number.cpp index 0690be0773f..c4b6daffafb 100644 --- a/src/ripple/basics/impl/Number.cpp +++ b/src/ripple/basics/impl/Number.cpp @@ -132,6 +132,8 @@ Number::Guard::round() noexcept auto mode = Number::getround(); switch (mode) { + // round to nearest if mode is not one of the predefined values + default: case to_nearest: if (digits_ > 0x5000'0000'0000'0000) return 1; @@ -506,8 +508,8 @@ to_string(Number const& amount) assert(exponent + 43 > 0); - size_t const pad_prefix = 27; - size_t const pad_suffix = 23; + ptrdiff_t const pad_prefix = 27; + ptrdiff_t const pad_suffix = 23; std::string const raw_value(std::to_string(mantissa)); std::string val; @@ -517,7 +519,7 @@ to_string(Number const& amount) val.append(raw_value); val.append(pad_suffix, '0'); - size_t const offset(exponent + 43); + ptrdiff_t const offset(exponent + 43); auto pre_from(val.begin()); auto const pre_to(val.begin() + offset); From 5733ff4a24bf6ad4e15bea27021b0c86def83259 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Thu, 15 Sep 2022 11:31:15 -0400 Subject: [PATCH 09/13] Remove undefined behavior * Taking the negative of a signed negative is UB, but taking the negative of an unsigned is not. --- src/ripple/basics/impl/IOUAmount.cpp | 75 +++++---- src/ripple/basics/impl/Number.cpp | 55 ++++--- src/test/app/Offer_test.cpp | 62 +++++--- src/test/app/Taker_test.cpp | 227 ++++++++++++++++----------- src/test/jtx/Env.h | 3 + src/test/jtx/impl/Env.cpp | 8 + 6 files changed, 251 insertions(+), 179 deletions(-) diff --git a/src/ripple/basics/impl/IOUAmount.cpp b/src/ripple/basics/impl/IOUAmount.cpp index 1fa5e4fd22d..76f4bbe9fcd 100644 --- a/src/ripple/basics/impl/IOUAmount.cpp +++ b/src/ripple/basics/impl/IOUAmount.cpp @@ -45,6 +45,12 @@ IOUAmount::minPositiveAmount() void IOUAmount::normalize() { + if (mantissa_ == 0) + { + *this = beast::zero; + return; + } + if (*stNumberSwitchover) { Number v{mantissa_, exponent_}; @@ -56,11 +62,6 @@ IOUAmount::normalize() *this = beast::zero; return; } - if (mantissa_ == 0) - { - *this = beast::zero; - return; - } bool const negative = (mantissa_ < 0); @@ -107,48 +108,46 @@ IOUAmount::IOUAmount(Number const& other) IOUAmount& IOUAmount::operator+=(IOUAmount const& other) { + if (other == beast::zero) + return *this; + + if (*this == beast::zero) + { + *this = other; + return *this; + } + if (*stNumberSwitchover) { *this = IOUAmount{Number{*this} + Number{other}}; + return *this; } - else - { - if (other == beast::zero) - return *this; - - if (*this == beast::zero) - { - *this = other; - return *this; - } + auto m = other.mantissa_; + auto e = other.exponent_; - auto m = other.mantissa_; - auto e = other.exponent_; - - while (exponent_ < e) - { - mantissa_ /= 10; - ++exponent_; - } - - while (e < exponent_) - { - m /= 10; - ++e; - } + while (exponent_ < e) + { + mantissa_ /= 10; + ++exponent_; + } - // This addition cannot overflow an std::int64_t but we may throw from - // normalize if the result isn't representable. - mantissa_ += m; + while (e < exponent_) + { + m /= 10; + ++e; + } - if (mantissa_ >= -10 && mantissa_ <= 10) - { - *this = beast::zero; - return *this; - } + // This addition cannot overflow an std::int64_t but we may throw from + // normalize if the result isn't representable. + mantissa_ += m; - normalize(); + if (mantissa_ >= -10 && mantissa_ <= 10) + { + *this = beast::zero; + return *this; } + + normalize(); return *this; } diff --git a/src/ripple/basics/impl/Number.cpp b/src/ripple/basics/impl/Number.cpp index c4b6daffafb..a7a8159fed3 100644 --- a/src/ripple/basics/impl/Number.cpp +++ b/src/ripple/basics/impl/Number.cpp @@ -25,7 +25,7 @@ #include #include -#ifdef _MSVC_LANG +#ifdef BOOST_COMP_MSVC #include using uint128_t = boost::multiprecision::uint128_t; #else // !defined(_MSVC_LANG) @@ -130,34 +130,37 @@ int Number::Guard::round() noexcept { auto mode = Number::getround(); - switch (mode) + + if (mode == towards_zero) + return -1; + + if (mode == downward) { - // round to nearest if mode is not one of the predefined values - default: - case to_nearest: - if (digits_ > 0x5000'0000'0000'0000) - return 1; - if (digits_ < 0x5000'0000'0000'0000) - return -1; - if (xbit_) - return 1; - return 0; - case towards_zero: - return -1; - case downward: - if (sbit_) - { - if (digits_ > 0 || xbit_) - return 1; - } - return -1; - case upward: - if (sbit_) - return -1; + if (sbit_) + { if (digits_ > 0 || xbit_) return 1; + } + return -1; + } + + if (mode == upward) + { + if (sbit_) return -1; + if (digits_ > 0 || xbit_) + return 1; + return -1; } + + // assume round to nearest if mode is not one of the predefined values + if (digits_ > 0x5000'0000'0000'0000) + return 1; + if (digits_ < 0x5000'0000'0000'0000) + return -1; + if (xbit_) + return 1; + return 0; } // Number @@ -173,9 +176,9 @@ Number::normalize() return; } bool const negative = (mantissa_ < 0); - if (negative) - mantissa_ = -mantissa_; auto m = static_cast>(mantissa_); + if (negative) + m = -m; while ((m < minMantissa) && (exponent_ > minExponent)) { m *= 10; diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index fc9a38cd28e..1162612b733 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -2092,37 +2092,53 @@ class Offer_test : public beast::unit_test::suite using namespace jtx; - Env env{*this, features}; + for (auto NumberSwitchOver : {false, true}) + { + Env env{*this, features}; + if (NumberSwitchOver) + env.enableFeature(fixUniversalNumber); + else + env.disableFeature(fixUniversalNumber); - auto const gw = Account{"gateway"}; - auto const alice = Account{"alice"}; - auto const bob = Account{"bob"}; - auto const USD = gw["USD"]; + auto const gw = Account{"gateway"}; + auto const alice = Account{"alice"}; + auto const bob = Account{"bob"}; + auto const USD = gw["USD"]; - env.fund(XRP(10000), gw, alice, bob); + env.fund(XRP(10000), gw, alice, bob); - env(rate(gw, 1.005)); + env(rate(gw, 1.005)); - env(trust(alice, USD(1000))); - env(trust(bob, USD(1000))); - env(trust(gw, alice["USD"](50))); + env(trust(alice, USD(1000))); + env(trust(bob, USD(1000))); + env(trust(gw, alice["USD"](50))); - env(pay(gw, bob, bob["USD"](1))); - env(pay(alice, gw, USD(50))); + env(pay(gw, bob, bob["USD"](1))); + env(pay(alice, gw, USD(50))); - env(trust(gw, alice["USD"](0))); + env(trust(gw, alice["USD"](0))); - env(offer(alice, USD(50), XRP(150000))); - env(offer(bob, XRP(100), USD(0.1))); + env(offer(alice, USD(50), XRP(150000))); + env(offer(bob, XRP(100), USD(0.1))); - auto jrr = ledgerEntryState(env, alice, gw, "USD"); - BEAST_EXPECT( - jrr[jss::node][sfBalance.fieldName][jss::value] == - "49.96666666666667"); - jrr = ledgerEntryState(env, bob, gw, "USD"); - BEAST_EXPECT( - jrr[jss::node][sfBalance.fieldName][jss::value] == - "-0.9665000000333333"); + auto jrr = ledgerEntryState(env, alice, gw, "USD"); + BEAST_EXPECT( + jrr[jss::node][sfBalance.fieldName][jss::value] == + "49.96666666666667"); + jrr = ledgerEntryState(env, bob, gw, "USD"); + if (NumberSwitchOver) + { + BEAST_EXPECT( + jrr[jss::node][sfBalance.fieldName][jss::value] == + "-0.9665000000333333"); + } + else + { + BEAST_EXPECT( + jrr[jss::node][sfBalance.fieldName][jss::value] == + "-0.966500000033334"); + } + } } void diff --git a/src/test/app/Taker_test.cpp b/src/test/app/Taker_test.cpp index c7474b6798e..38735d8cfce 100644 --- a/src/test/app/Taker_test.cpp +++ b/src/test/app/Taker_test.cpp @@ -905,100 +905,143 @@ class Taker_test : public beast::unit_test::suite { testcase("IOU to IOU"); - NumberSO stNumberSO{true}; - Quality q1 = get_quality("1", "1"); + for (auto NumberSwitchOver : {false, true}) + { + NumberSO stNumberSO{NumberSwitchOver}; + Quality q1 = get_quality("1", "1"); - // Highly exaggerated 50% transfer rate for the input and output: - Rate const rate{parityRate.value + (parityRate.value / 2)}; + // Highly exaggerated 50% transfer rate for the input and output: + Rate const rate{parityRate.value + (parityRate.value / 2)}; - // TAKER OWNER - // QUAL OFFER FUNDS QUAL OFFER FUNDS - // EXPECTED - // EUR USD - attempt( - Sell, - "N:N", - q1, - {"2", "2"}, - "10", - q1, - {"2", "2"}, - "10", - {"2", "2"}, - eur(), - usd(), - rate, - rate); - attempt( - Sell, - "N:B", - q1, - {"4", "4"}, - "10", - q1, - {"4", "4"}, - "4", - {"2.666666666666667", "2.666666666666667"}, - eur(), - usd(), - rate, - rate); - attempt( - Buy, - "N:T", - q1, - {"1", "1"}, - "10", - q1, - {"2", "2"}, - "10", - {"1", "1"}, - eur(), - usd(), - rate, - rate); - attempt( - Buy, - "N:BT", - q1, - {"2", "2"}, - "10", - q1, - {"6", "6"}, - "5", - {"2", "2"}, - eur(), - usd(), - rate, - rate); - attempt( - Buy, - "N:TB", - q1, - {"2", "2"}, - "2", - q1, - {"6", "6"}, - "1", - {"0.6666666666666667", "0.6666666666666667"}, - eur(), - usd(), - rate, - rate); - attempt( - Sell, - "A:N", - q1, - {"2", "2"}, - "2.5", - q1, - {"2", "2"}, - "10", - {"1.666666666666667", "1.666666666666667"}, - eur(), - usd(), - rate, - rate); + // TAKER OWNER + // QUAL OFFER FUNDS QUAL OFFER FUNDS + // EXPECTED + // EUR USD + attempt( + Sell, + "N:N", + q1, + {"2", "2"}, + "10", + q1, + {"2", "2"}, + "10", + {"2", "2"}, + eur(), + usd(), + rate, + rate); + if (NumberSwitchOver) + { + attempt( + Sell, + "N:B", + q1, + {"4", "4"}, + "10", + q1, + {"4", "4"}, + "4", + {"2.666666666666667", "2.666666666666667"}, + eur(), + usd(), + rate, + rate); + } + else + { + attempt( + Sell, + "N:B", + q1, + {"4", "4"}, + "10", + q1, + {"4", "4"}, + "4", + {"2.666666666666666", "2.666666666666666"}, + eur(), + usd(), + rate, + rate); + } + attempt( + Buy, + "N:T", + q1, + {"1", "1"}, + "10", + q1, + {"2", "2"}, + "10", + {"1", "1"}, + eur(), + usd(), + rate, + rate); + attempt( + Buy, + "N:BT", + q1, + {"2", "2"}, + "10", + q1, + {"6", "6"}, + "5", + {"2", "2"}, + eur(), + usd(), + rate, + rate); + attempt( + Buy, + "N:TB", + q1, + {"2", "2"}, + "2", + q1, + {"6", "6"}, + "1", + {"0.6666666666666667", "0.6666666666666667"}, + eur(), + usd(), + rate, + rate); + if (NumberSwitchOver) + { + attempt( + Sell, + "A:N", + q1, + {"2", "2"}, + "2.5", + q1, + {"2", "2"}, + "10", + {"1.666666666666667", "1.666666666666667"}, + eur(), + usd(), + rate, + rate); + } + else + { + attempt( + Sell, + "A:N", + q1, + {"2", "2"}, + "2.5", + q1, + {"2", "2"}, + "10", + {"1.666666666666666", "1.666666666666666"}, + eur(), + usd(), + rate, + rate); + } + } } void diff --git a/src/test/jtx/Env.h b/src/test/jtx/Env.h index 8841d2d2f58..2a85a57e1db 100644 --- a/src/test/jtx/Env.h +++ b/src/test/jtx/Env.h @@ -543,6 +543,9 @@ class Env void enableFeature(uint256 const feature); + void + disableFeature(uint256 const feature); + private: void fund(bool setDefaultRipple, STAmount const& amount, Account const& account); diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index 900b9812dae..41eac3204ed 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -466,6 +466,14 @@ Env::enableFeature(uint256 const feature) app().config().features.insert(feature); } +void +Env::disableFeature(uint256 const feature) +{ + // Env::close() must be called for feature + // enable to take place. + app().config().features.erase(feature); +} + } // namespace jtx } // namespace test From bd97fb52562137c249802d62d5796f8fee2bdbec Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Wed, 19 Oct 2022 16:00:41 -0400 Subject: [PATCH 10/13] Include rounding mode in XRPAmount to STAmount conversion. --- src/ripple/basics/IOUAmount.h | 4 - src/ripple/basics/Number.h | 18 + src/ripple/protocol/impl/STAmount.cpp | 38 +- src/test/app/NFToken_test.cpp | 22 +- src/test/app/Taker_test.cpp | 1557 +++++++++++++++---------- src/test/basics/Number_test.cpp | 42 +- 6 files changed, 1028 insertions(+), 653 deletions(-) diff --git a/src/ripple/basics/IOUAmount.h b/src/ripple/basics/IOUAmount.h index c3ef1340a76..764aa38aae3 100644 --- a/src/ripple/basics/IOUAmount.h +++ b/src/ripple/basics/IOUAmount.h @@ -186,10 +186,6 @@ mulRatio( std::uint32_t den, bool roundUp); -// Since IOUAmount and STAmount do not have access to a ledger, this -// is needed to put low-level routines on an amendment switch. Only -// transactions need to use this switchover. Outside of a transaction -// it's safe to unconditionally use the new behavior. extern LocalValue stNumberSwitchover; /** RAII class to set and restore the Number switchover. diff --git a/src/ripple/basics/Number.h b/src/ripple/basics/Number.h index ef45b3f7795..58d903579b5 100644 --- a/src/ripple/basics/Number.h +++ b/src/ripple/basics/Number.h @@ -337,6 +337,24 @@ squelch(Number const& x, Number const& limit) noexcept return x; } +class saveNumberRoundMode +{ + Number::rounding_mode mode_; + +public: + ~saveNumberRoundMode() + { + Number::setround(mode_); + } + explicit saveNumberRoundMode(Number::rounding_mode mode) noexcept + : mode_{mode} + { + } + saveNumberRoundMode(saveNumberRoundMode const&) = delete; + saveNumberRoundMode& + operator=(saveNumberRoundMode const&) = delete; +}; + } // namespace ripple #endif // RIPPLE_BASICS_NUMBER_H_INCLUDED diff --git a/src/ripple/protocol/impl/STAmount.cpp b/src/ripple/protocol/impl/STAmount.cpp index 51e8adb568d..d1a878c8b4f 100644 --- a/src/ripple/protocol/impl/STAmount.cpp +++ b/src/ripple/protocol/impl/STAmount.cpp @@ -725,24 +725,36 @@ STAmount::canonicalize() "Native currency amount out of range"); } - while (mOffset < 0) + if (*stNumberSwitchover && *stAmountCanonicalizeSwitchover) { - mValue /= 10; - ++mOffset; + Number num( + mIsNegative ? -mValue : mValue, mOffset, Number::unchecked{}); + XRPAmount xrp{num}; + mIsNegative = xrp.drops() < 0; + mValue = mIsNegative ? -xrp.drops() : xrp.drops(); + mOffset = 0; } - - while (mOffset > 0) + else { - if (*stAmountCanonicalizeSwitchover) + while (mOffset < 0) + { + mValue /= 10; + ++mOffset; + } + + while (mOffset > 0) { - // N.B. do not move the overflow check to after the - // multiplication - if (mValue > cMaxNativeN) - Throw( - "Native currency amount out of range"); + if (*stAmountCanonicalizeSwitchover) + { + // N.B. do not move the overflow check to after the + // multiplication + if (mValue > cMaxNativeN) + Throw( + "Native currency amount out of range"); + } + mValue *= 10; + --mOffset; } - mValue *= 10; - --mOffset; } if (mValue > cMaxNativeN) diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 42a6eb4d3ce..842f3f76cc8 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -2349,7 +2349,13 @@ class NFToken_test : public beast::unit_test::suite // See the impact of rounding when the nft is sold for small amounts // of drops. + for (auto NumberSwitchOver : {true}) { + if (NumberSwitchOver) + env.enableFeature(fixUniversalNumber); + else + env.disableFeature(fixUniversalNumber); + // An nft with a transfer fee of 1 basis point. uint256 const nftID = token::getNextID(env, alice, 0u, tfTransferable, 1); @@ -2374,16 +2380,16 @@ class NFToken_test : public beast::unit_test::suite // minter sells to carol. The payment is just small enough that // alice does not get any transfer fee. + auto pmt = NumberSwitchOver ? drops(50000) : drops(99999); STAmount carolBalance = env.balance(carol); uint256 const minterSellOfferIndex = keylet::nftoffer(minter, env.seq(minter)).key; - env(token::createOffer(minter, nftID, drops(99999)), - txflags(tfSellNFToken)); + env(token::createOffer(minter, nftID, pmt), txflags(tfSellNFToken)); env.close(); env(token::acceptSellOffer(carol, minterSellOfferIndex)); env.close(); - minterBalance += drops(99999) - fee; - carolBalance -= drops(99999) + fee; + minterBalance += pmt - fee; + carolBalance -= pmt + fee; BEAST_EXPECT(env.balance(alice) == aliceBalance); BEAST_EXPECT(env.balance(minter) == minterBalance); BEAST_EXPECT(env.balance(carol) == carolBalance); @@ -2393,13 +2399,13 @@ class NFToken_test : public beast::unit_test::suite STAmount beckyBalance = env.balance(becky); uint256 const beckyBuyOfferIndex = keylet::nftoffer(becky, env.seq(becky)).key; - env(token::createOffer(becky, nftID, drops(100000)), - token::owner(carol)); + pmt = NumberSwitchOver ? drops(50001) : drops(100000); + env(token::createOffer(becky, nftID, pmt), token::owner(carol)); env.close(); env(token::acceptBuyOffer(carol, beckyBuyOfferIndex)); env.close(); - carolBalance += drops(99999) - fee; - beckyBalance -= drops(100000) + fee; + carolBalance += pmt - drops(1) - fee; + beckyBalance -= pmt + fee; aliceBalance += drops(1); BEAST_EXPECT(env.balance(alice) == aliceBalance); diff --git a/src/test/app/Taker_test.cpp b/src/test/app/Taker_test.cpp index 38735d8cfce..0b69b25f24b 100644 --- a/src/test/app/Taker_test.cpp +++ b/src/test/app/Taker_test.cpp @@ -273,314 +273,521 @@ class Taker_test : public beast::unit_test::suite Quality q1 = get_quality("1", "1"); - // TAKER OWNER - // QUAL OFFER FUNDS QUAL OFFER FUNDS - // EXPECTED - // XRP USD - attempt( - Sell, - "N:N", - q1, - {"2", "2"}, - "2", - q1, - {"2", "2"}, - "2", - {"2", "2"}, - xrp(), - usd()); - attempt( - Sell, - "N:B", - q1, - {"2", "2"}, - "2", - q1, - {"2", "2"}, - "1.8", - {"1", "1.8"}, - xrp(), - usd()); - attempt( - Buy, - "N:T", - q1, - {"1", "1"}, - "2", - q1, - {"2", "2"}, - "2", - {"1", "1"}, - xrp(), - usd()); - attempt( - Buy, - "N:BT", - q1, - {"1", "1"}, - "2", - q1, - {"2", "2"}, - "1.8", - {"1", "1"}, - xrp(), - usd()); - attempt( - Buy, - "N:TB", - q1, - {"1", "1"}, - "2", - q1, - {"2", "2"}, - "0.8", - {"0", "0.8"}, - xrp(), - usd()); - - attempt( - Sell, - "T:N", - q1, - {"1", "1"}, - "2", - q1, - {"2", "2"}, - "2", - {"1", "1"}, - xrp(), - usd()); - attempt( - Sell, - "T:B", - q1, - {"1", "1"}, - "2", - q1, - {"2", "2"}, - "1.8", - {"1", "1.8"}, - xrp(), - usd()); - attempt( - Buy, - "T:T", - q1, - {"1", "1"}, - "2", - q1, - {"2", "2"}, - "2", - {"1", "1"}, - xrp(), - usd()); - attempt( - Buy, - "T:BT", - q1, - {"1", "1"}, - "2", - q1, - {"2", "2"}, - "1.8", - {"1", "1"}, - xrp(), - usd()); - attempt( - Buy, - "T:TB", - q1, - {"1", "1"}, - "2", - q1, - {"2", "2"}, - "0.8", - {"0", "0.8"}, - xrp(), - usd()); + for (auto NumberSwitchOver : {false, true}) + { + NumberSO stNumberSO{NumberSwitchOver}; + // TAKER OWNER + // QUAL OFFER FUNDS QUAL OFFER FUNDS + // EXPECTED + // XRP USD + attempt( + Sell, + "N:N", + q1, + {"2", "2"}, + "2", + q1, + {"2", "2"}, + "2", + {"2", "2"}, + xrp(), + usd()); + if (NumberSwitchOver) + { + attempt( + Sell, + "N:B", + q1, + {"2", "2"}, + "2", + q1, + {"2", "2"}, + "1.8", + {"2", "1.8"}, + xrp(), + usd()); + } + else + { + attempt( + Sell, + "N:B", + q1, + {"2", "2"}, + "2", + q1, + {"2", "2"}, + "1.8", + {"1", "1.8"}, + xrp(), + usd()); + } + attempt( + Buy, + "N:T", + q1, + {"1", "1"}, + "2", + q1, + {"2", "2"}, + "2", + {"1", "1"}, + xrp(), + usd()); + attempt( + Buy, + "N:BT", + q1, + {"1", "1"}, + "2", + q1, + {"2", "2"}, + "1.8", + {"1", "1"}, + xrp(), + usd()); + if (NumberSwitchOver) + { + attempt( + Buy, + "N:TB", + q1, + {"1", "1"}, + "2", + q1, + {"2", "2"}, + "0.8", + {"1", "0.8"}, + xrp(), + usd()); + } + else + { + attempt( + Buy, + "N:TB", + q1, + {"1", "1"}, + "2", + q1, + {"2", "2"}, + "0.8", + {"0", "0.8"}, + xrp(), + usd()); + } + attempt( + Sell, + "T:N", + q1, + {"1", "1"}, + "2", + q1, + {"2", "2"}, + "2", + {"1", "1"}, + xrp(), + usd()); + if (NumberSwitchOver) + { + attempt( + Sell, + "T:B", + q1, + {"1", "1"}, + "2", + q1, + {"2", "2"}, + "1.8", + {"1", "1"}, + xrp(), + usd()); + } + else + { + attempt( + Sell, + "T:B", + q1, + {"1", "1"}, + "2", + q1, + {"2", "2"}, + "1.8", + {"1", "1.8"}, + xrp(), + usd()); + } + attempt( + Buy, + "T:T", + q1, + {"1", "1"}, + "2", + q1, + {"2", "2"}, + "2", + {"1", "1"}, + xrp(), + usd()); + attempt( + Buy, + "T:BT", + q1, + {"1", "1"}, + "2", + q1, + {"2", "2"}, + "1.8", + {"1", "1"}, + xrp(), + usd()); + if (NumberSwitchOver) + { + attempt( + Buy, + "T:TB", + q1, + {"1", "1"}, + "2", + q1, + {"2", "2"}, + "0.8", + {"1", "0.8"}, + xrp(), + usd()); + } + else + { + attempt( + Buy, + "T:TB", + q1, + {"1", "1"}, + "2", + q1, + {"2", "2"}, + "0.8", + {"0", "0.8"}, + xrp(), + usd()); + } - attempt( - Sell, - "A:N", - q1, - {"2", "2"}, - "1", - q1, - {"2", "2"}, - "2", - {"1", "1"}, - xrp(), - usd()); - attempt( - Sell, - "A:B", - q1, - {"2", "2"}, - "1", - q1, - {"2", "2"}, - "1.8", - {"1", "1.8"}, - xrp(), - usd()); - attempt( - Buy, - "A:T", - q1, - {"2", "2"}, - "1", - q1, - {"3", "3"}, - "3", - {"1", "1"}, - xrp(), - usd()); - attempt( - Buy, - "A:BT", - q1, - {"2", "2"}, - "1", - q1, - {"3", "3"}, - "2.4", - {"1", "1"}, - xrp(), - usd()); - attempt( - Buy, - "A:TB", - q1, - {"2", "2"}, - "1", - q1, - {"3", "3"}, - "0.8", - {"0", "0.8"}, - xrp(), - usd()); + attempt( + Sell, + "A:N", + q1, + {"2", "2"}, + "1", + q1, + {"2", "2"}, + "2", + {"1", "1"}, + xrp(), + usd()); + if (NumberSwitchOver) + { + attempt( + Sell, + "A:B", + q1, + {"2", "2"}, + "1", + q1, + {"2", "2"}, + "1.8", + {"1", "1"}, + xrp(), + usd()); + } + else + { + attempt( + Sell, + "A:B", + q1, + {"2", "2"}, + "1", + q1, + {"2", "2"}, + "1.8", + {"1", "1.8"}, + xrp(), + usd()); + } + attempt( + Buy, + "A:T", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "3", + {"1", "1"}, + xrp(), + usd()); + attempt( + Buy, + "A:BT", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "2.4", + {"1", "1"}, + xrp(), + usd()); + if (NumberSwitchOver) + { + attempt( + Buy, + "A:TB", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "0.8", + {"1", "0.8"}, + xrp(), + usd()); + } + else + { + attempt( + Buy, + "A:TB", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "0.8", + {"0", "0.8"}, + xrp(), + usd()); + } - attempt( - Sell, - "TA:N", - q1, - {"2", "2"}, - "1", - q1, - {"2", "2"}, - "2", - {"1", "1"}, - xrp(), - usd()); - attempt( - Sell, - "TA:B", - q1, - {"2", "2"}, - "1", - q1, - {"3", "3"}, - "1.8", - {"1", "1.8"}, - xrp(), - usd()); - attempt( - Buy, - "TA:T", - q1, - {"2", "2"}, - "1", - q1, - {"3", "3"}, - "3", - {"1", "1"}, - xrp(), - usd()); - attempt( - Buy, - "TA:BT", - q1, - {"2", "2"}, - "1", - q1, - {"3", "3"}, - "1.8", - {"1", "1.8"}, - xrp(), - usd()); - attempt( - Buy, - "TA:TB", - q1, - {"2", "2"}, - "1", - q1, - {"3", "3"}, - "1.8", - {"1", "1.8"}, - xrp(), - usd()); + attempt( + Sell, + "TA:N", + q1, + {"2", "2"}, + "1", + q1, + {"2", "2"}, + "2", + {"1", "1"}, + xrp(), + usd()); + if (NumberSwitchOver) + { + attempt( + Sell, + "TA:B", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "1.8", + {"1", "1"}, + xrp(), + usd()); + } + else + { + attempt( + Sell, + "TA:B", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "1.8", + {"1", "1.8"}, + xrp(), + usd()); + } + attempt( + Buy, + "TA:T", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "3", + {"1", "1"}, + xrp(), + usd()); + if (NumberSwitchOver) + { + attempt( + Buy, + "TA:BT", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "1.8", + {"1", "1"}, + xrp(), + usd()); + attempt( + Buy, + "TA:TB", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "1.8", + {"1", "1"}, + xrp(), + usd()); + } + else + { + attempt( + Buy, + "TA:BT", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "1.8", + {"1", "1.8"}, + xrp(), + usd()); + attempt( + Buy, + "TA:TB", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "1.8", + {"1", "1.8"}, + xrp(), + usd()); + } - attempt( - Sell, - "AT:N", - q1, - {"2", "2"}, - "1", - q1, - {"3", "3"}, - "3", - {"1", "1"}, - xrp(), - usd()); - attempt( - Sell, - "AT:B", - q1, - {"2", "2"}, - "1", - q1, - {"3", "3"}, - "1.8", - {"1", "1.8"}, - xrp(), - usd()); - attempt( - Buy, - "AT:T", - q1, - {"2", "2"}, - "1", - q1, - {"3", "3"}, - "3", - {"1", "1"}, - xrp(), - usd()); - attempt( - Buy, - "AT:BT", - q1, - {"2", "2"}, - "1", - q1, - {"3", "3"}, - "1.8", - {"1", "1.8"}, - xrp(), - usd()); - attempt( - Buy, - "AT:TB", - q1, - {"2", "2"}, - "1", - q1, - {"3", "3"}, - "0.8", - {"0", "0.8"}, - xrp(), - usd()); + attempt( + Sell, + "AT:N", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "3", + {"1", "1"}, + xrp(), + usd()); + if (NumberSwitchOver) + { + attempt( + Sell, + "AT:B", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "1.8", + {"1", "1"}, + xrp(), + usd()); + } + else + { + attempt( + Sell, + "AT:B", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "1.8", + {"1", "1.8"}, + xrp(), + usd()); + } + attempt( + Buy, + "AT:T", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "3", + {"1", "1"}, + xrp(), + usd()); + if (NumberSwitchOver) + { + attempt( + Buy, + "AT:BT", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "1.8", + {"1", "1"}, + xrp(), + usd()); + attempt( + Buy, + "AT:TB", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "0.8", + {"1", "0.8"}, + xrp(), + usd()); + } + else + { + attempt( + Buy, + "AT:BT", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "1.8", + {"1", "1.8"}, + xrp(), + usd()); + attempt( + Buy, + "AT:TB", + q1, + {"2", "2"}, + "1", + q1, + {"3", "3"}, + "0.8", + {"0", "0.8"}, + xrp(), + usd()); + } + } } void @@ -588,316 +795,446 @@ class Taker_test : public beast::unit_test::suite { testcase("XRP Quantization: output"); - Quality q1 = get_quality("1", "1"); + for (auto NumberSwitchOver : {false, true}) + { + NumberSO stNumberSO{NumberSwitchOver}; + Quality q1 = get_quality("1", "1"); - // TAKER OWNER - // QUAL OFFER FUNDS QUAL OFFER FUNDS - // EXPECTED - // USD XRP - attempt( - Sell, - "N:N", - q1, - {"3", "3"}, - "3", - q1, - {"3", "3"}, - "3", - {"3", "3"}, - usd(), - xrp()); - attempt( - Sell, - "N:B", - q1, - {"3", "3"}, - "3", - q1, - {"3", "3"}, - "2", - {"2", "2"}, - usd(), - xrp()); - attempt( - Buy, - "N:T", - q1, - {"3", "3"}, - "2.5", - q1, - {"5", "5"}, - "5", - {"2.5", "2"}, - usd(), - xrp()); - attempt( - Buy, - "N:BT", - q1, - {"3", "3"}, - "1.5", - q1, - {"5", "5"}, - "4", - {"1.5", "1"}, - usd(), - xrp()); - attempt( - Buy, - "N:TB", - q1, - {"3", "3"}, - "2.2", - q1, - {"5", "5"}, - "1", - {"1", "1"}, - usd(), - xrp()); + // TAKER OWNER + // QUAL OFFER FUNDS QUAL OFFER FUNDS + // EXPECTED + // USD XRP + attempt( + Sell, + "N:N", + q1, + {"3", "3"}, + "3", + q1, + {"3", "3"}, + "3", + {"3", "3"}, + usd(), + xrp()); + attempt( + Sell, + "N:B", + q1, + {"3", "3"}, + "3", + q1, + {"3", "3"}, + "2", + {"2", "2"}, + usd(), + xrp()); + if (NumberSwitchOver) + { + attempt( + Buy, + "N:T", + q1, + {"3", "3"}, + "2.5", + q1, + {"5", "5"}, + "5", + {"2.5", "3"}, + usd(), + xrp()); + attempt( + Buy, + "N:BT", + q1, + {"3", "3"}, + "1.5", + q1, + {"5", "5"}, + "4", + {"1.5", "2"}, + usd(), + xrp()); + } + else + { + attempt( + Buy, + "N:T", + q1, + {"3", "3"}, + "2.5", + q1, + {"5", "5"}, + "5", + {"2.5", "2"}, + usd(), + xrp()); + attempt( + Buy, + "N:BT", + q1, + {"3", "3"}, + "1.5", + q1, + {"5", "5"}, + "4", + {"1.5", "1"}, + usd(), + xrp()); + } + attempt( + Buy, + "N:TB", + q1, + {"3", "3"}, + "2.2", + q1, + {"5", "5"}, + "1", + {"1", "1"}, + usd(), + xrp()); - attempt( - Sell, - "T:N", - q1, - {"1", "1"}, - "2", - q1, - {"2", "2"}, - "2", - {"1", "1"}, - usd(), - xrp()); - attempt( - Sell, - "T:B", - q1, - {"2", "2"}, - "2", - q1, - {"3", "3"}, - "1", - {"1", "1"}, - usd(), - xrp()); - attempt( - Buy, - "T:T", - q1, - {"1", "1"}, - "2", - q1, - {"2", "2"}, - "2", - {"1", "1"}, - usd(), - xrp()); - attempt( - Buy, - "T:BT", - q1, - {"1", "1"}, - "2", - q1, - {"3", "3"}, - "2", - {"1", "1"}, - usd(), - xrp()); - attempt( - Buy, - "T:TB", - q1, - {"2", "2"}, - "2", - q1, - {"3", "3"}, - "1", - {"1", "1"}, - usd(), - xrp()); + attempt( + Sell, + "T:N", + q1, + {"1", "1"}, + "2", + q1, + {"2", "2"}, + "2", + {"1", "1"}, + usd(), + xrp()); + attempt( + Sell, + "T:B", + q1, + {"2", "2"}, + "2", + q1, + {"3", "3"}, + "1", + {"1", "1"}, + usd(), + xrp()); + attempt( + Buy, + "T:T", + q1, + {"1", "1"}, + "2", + q1, + {"2", "2"}, + "2", + {"1", "1"}, + usd(), + xrp()); + attempt( + Buy, + "T:BT", + q1, + {"1", "1"}, + "2", + q1, + {"3", "3"}, + "2", + {"1", "1"}, + usd(), + xrp()); + attempt( + Buy, + "T:TB", + q1, + {"2", "2"}, + "2", + q1, + {"3", "3"}, + "1", + {"1", "1"}, + usd(), + xrp()); - attempt( - Sell, - "A:N", - q1, - {"2", "2"}, - "1.5", - q1, - {"2", "2"}, - "2", - {"1.5", "1"}, - usd(), - xrp()); - attempt( - Sell, - "A:B", - q1, - {"2", "2"}, - "1.8", - q1, - {"3", "3"}, - "2", - {"1.8", "1"}, - usd(), - xrp()); - attempt( - Buy, - "A:T", - q1, - {"2", "2"}, - "1.2", - q1, - {"3", "3"}, - "3", - {"1.2", "1"}, - usd(), - xrp()); - attempt( - Buy, - "A:BT", - q1, - {"2", "2"}, - "1.5", - q1, - {"4", "4"}, - "3", - {"1.5", "1"}, - usd(), - xrp()); - attempt( - Buy, - "A:TB", - q1, - {"2", "2"}, - "1.5", - q1, - {"4", "4"}, - "1", - {"1", "1"}, - usd(), - xrp()); + if (NumberSwitchOver) + { + attempt( + Sell, + "A:N", + q1, + {"2", "2"}, + "1.5", + q1, + {"2", "2"}, + "2", + {"1.5", "2"}, + usd(), + xrp()); + attempt( + Sell, + "A:B", + q1, + {"2", "2"}, + "1.8", + q1, + {"3", "3"}, + "2", + {"1.8", "2"}, + usd(), + xrp()); + } + else + { + attempt( + Sell, + "A:N", + q1, + {"2", "2"}, + "1.5", + q1, + {"2", "2"}, + "2", + {"1.5", "1"}, + usd(), + xrp()); + attempt( + Sell, + "A:B", + q1, + {"2", "2"}, + "1.8", + q1, + {"3", "3"}, + "2", + {"1.8", "1"}, + usd(), + xrp()); + } + attempt( + Buy, + "A:T", + q1, + {"2", "2"}, + "1.2", + q1, + {"3", "3"}, + "3", + {"1.2", "1"}, + usd(), + xrp()); + if (NumberSwitchOver) + { + attempt( + Buy, + "A:BT", + q1, + {"2", "2"}, + "1.5", + q1, + {"4", "4"}, + "3", + {"1.5", "2"}, + usd(), + xrp()); + } + else + { + attempt( + Buy, + "A:BT", + q1, + {"2", "2"}, + "1.5", + q1, + {"4", "4"}, + "3", + {"1.5", "1"}, + usd(), + xrp()); + } + attempt( + Buy, + "A:TB", + q1, + {"2", "2"}, + "1.5", + q1, + {"4", "4"}, + "1", + {"1", "1"}, + usd(), + xrp()); - attempt( - Sell, - "TA:N", - q1, - {"2", "2"}, - "1.5", - q1, - {"2", "2"}, - "2", - {"1.5", "1"}, - usd(), - xrp()); - attempt( - Sell, - "TA:B", - q1, - {"2", "2"}, - "1.5", - q1, - {"3", "3"}, - "1", - {"1", "1"}, - usd(), - xrp()); - attempt( - Buy, - "TA:T", - q1, - {"2", "2"}, - "1.5", - q1, - {"3", "3"}, - "3", - {"1.5", "1"}, - usd(), - xrp()); - attempt( - Buy, - "TA:BT", - q1, - {"2", "2"}, - "1.8", - q1, - {"4", "4"}, - "3", - {"1.8", "1"}, - usd(), - xrp()); - attempt( - Buy, - "TA:TB", - q1, - {"2", "2"}, - "1.2", - q1, - {"3", "3"}, - "1", - {"1", "1"}, - usd(), - xrp()); + if (NumberSwitchOver) + { + attempt( + Sell, + "TA:N", + q1, + {"2", "2"}, + "1.5", + q1, + {"2", "2"}, + "2", + {"1.5", "2"}, + usd(), + xrp()); + } + else + { + attempt( + Sell, + "TA:N", + q1, + {"2", "2"}, + "1.5", + q1, + {"2", "2"}, + "2", + {"1.5", "1"}, + usd(), + xrp()); + } + attempt( + Sell, + "TA:B", + q1, + {"2", "2"}, + "1.5", + q1, + {"3", "3"}, + "1", + {"1", "1"}, + usd(), + xrp()); + if (NumberSwitchOver) + { + attempt( + Buy, + "TA:T", + q1, + {"2", "2"}, + "1.5", + q1, + {"3", "3"}, + "3", + {"1.5", "2"}, + usd(), + xrp()); + attempt( + Buy, + "TA:BT", + q1, + {"2", "2"}, + "1.8", + q1, + {"4", "4"}, + "3", + {"1.8", "2"}, + usd(), + xrp()); + } + else + { + attempt( + Buy, + "TA:T", + q1, + {"2", "2"}, + "1.5", + q1, + {"3", "3"}, + "3", + {"1.5", "1"}, + usd(), + xrp()); + attempt( + Buy, + "TA:BT", + q1, + {"2", "2"}, + "1.8", + q1, + {"4", "4"}, + "3", + {"1.8", "1"}, + usd(), + xrp()); + } + attempt( + Buy, + "TA:TB", + q1, + {"2", "2"}, + "1.2", + q1, + {"3", "3"}, + "1", + {"1", "1"}, + usd(), + xrp()); - attempt( - Sell, - "AT:N", - q1, - {"2", "2"}, - "2.5", - q1, - {"4", "4"}, - "4", - {"2", "2"}, - usd(), - xrp()); - attempt( - Sell, - "AT:B", - q1, - {"2", "2"}, - "2.5", - q1, - {"3", "3"}, - "1", - {"1", "1"}, - usd(), - xrp()); - attempt( - Buy, - "AT:T", - q1, - {"2", "2"}, - "2.5", - q1, - {"3", "3"}, - "3", - {"2", "2"}, - usd(), - xrp()); - attempt( - Buy, - "AT:BT", - q1, - {"2", "2"}, - "2.5", - q1, - {"4", "4"}, - "3", - {"2", "2"}, - usd(), - xrp()); - attempt( - Buy, - "AT:TB", - q1, - {"2", "2"}, - "2.5", - q1, - {"3", "3"}, - "1", - {"1", "1"}, - usd(), - xrp()); + attempt( + Sell, + "AT:N", + q1, + {"2", "2"}, + "2.5", + q1, + {"4", "4"}, + "4", + {"2", "2"}, + usd(), + xrp()); + attempt( + Sell, + "AT:B", + q1, + {"2", "2"}, + "2.5", + q1, + {"3", "3"}, + "1", + {"1", "1"}, + usd(), + xrp()); + attempt( + Buy, + "AT:T", + q1, + {"2", "2"}, + "2.5", + q1, + {"3", "3"}, + "3", + {"2", "2"}, + usd(), + xrp()); + attempt( + Buy, + "AT:BT", + q1, + {"2", "2"}, + "2.5", + q1, + {"4", "4"}, + "3", + {"2", "2"}, + usd(), + xrp()); + attempt( + Buy, + "AT:TB", + q1, + {"2", "2"}, + "2.5", + q1, + {"3", "3"}, + "1", + {"1", "1"}, + usd(), + xrp()); + } } void diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index d7bd8264878..d3ece630e3e 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -26,24 +26,6 @@ namespace ripple { -class saveNumberRoundMode -{ - Number::rounding_mode mode_; - -public: - ~saveNumberRoundMode() - { - Number::setround(mode_); - } - explicit saveNumberRoundMode(Number::rounding_mode mode) noexcept - : mode_{mode} - { - } - saveNumberRoundMode(saveNumberRoundMode const&) = delete; - saveNumberRoundMode& - operator=(saveNumberRoundMode const&) = delete; -}; - class Number_test : public beast::unit_test::suite { public: @@ -580,6 +562,29 @@ class Number_test : public beast::unit_test::suite BEAST_EXPECT(x == y); } + void + test_toSTAmount() + { + NumberSO stNumberSO{true}; + Issue const issue; + Number const n{7'518'783'80596, -5}; + saveNumberRoundMode const save{Number::setround(Number::to_nearest)}; + auto res2 = STAmount{issue, n.mantissa(), n.exponent()}; + BEAST_EXPECT(res2 == STAmount{7518784}); + + Number::setround(Number::towards_zero); + res2 = STAmount{issue, n.mantissa(), n.exponent()}; + BEAST_EXPECT(res2 == STAmount{7518783}); + + Number::setround(Number::downward); + res2 = STAmount{issue, n.mantissa(), n.exponent()}; + BEAST_EXPECT(res2 == STAmount{7518783}); + + Number::setround(Number::upward); + res2 = STAmount{issue, n.mantissa(), n.exponent()}; + BEAST_EXPECT(res2 == STAmount{7518784}); + } + void run() override { @@ -599,6 +604,7 @@ class Number_test : public beast::unit_test::suite test_relationals(); test_stream(); test_inc_dec(); + test_toSTAmount(); } }; From 25c0589285e778c97d9e47008394874d30598ea7 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Thu, 22 Dec 2022 12:20:56 -0500 Subject: [PATCH 11/13] Replace Number division algorithm * Replace division with faster algorithm. * Correct some rounding bugs in multiplication. * Add tests for rounding bugs. --- src/ripple/basics/impl/Number.cpp | 39 +++---- src/test/basics/Number_test.cpp | 186 +++++++++++++++++++++++++----- 2 files changed, 175 insertions(+), 50 deletions(-) diff --git a/src/ripple/basics/impl/Number.cpp b/src/ripple/basics/impl/Number.cpp index a7a8159fed3..52d2b556d44 100644 --- a/src/ripple/basics/impl/Number.cpp +++ b/src/ripple/basics/impl/Number.cpp @@ -185,6 +185,8 @@ Number::normalize() --exponent_; } Guard g; + if (negative) + g.set_negative(); while (m > maxMantissa) { if (exponent_ >= maxExponent) @@ -364,6 +366,8 @@ Number::operator*=(Number const& y) auto ze = xe + ye; auto zn = xn * yn; Guard g; + if (zn == -1) + g.set_negative(); while (zm > maxMantissa) { g.push(static_cast(zm % 10)); @@ -402,8 +406,11 @@ Number::operator/=(Number const& y) { if (y == Number{}) throw std::overflow_error("Number: divide by 0"); + if (*this == Number{}) + return *this; int np = 1; auto nm = mantissa(); + auto ne = exponent(); if (nm < 0) { nm = -nm; @@ -411,35 +418,19 @@ Number::operator/=(Number const& y) } int dp = 1; auto dm = y.mantissa(); + auto de = y.exponent(); if (dm < 0) { dm = -dm; dp = -1; } - // Divide numerator and denominator such that the - // denominator is in the range [1, 10). - const int offset = -15 - y.exponent(); - Number n{nm * (np * dp), exponent() + offset}; - Number d{dm, y.exponent() + offset}; - // Quadratic least squares fit to 1/x in the range [1, 10] - constexpr Number a0{9178756872006464, -16, unchecked{}}; - constexpr Number a1{-2149215784206187, -16, unchecked{}}; - constexpr Number a2{1405502114116773, -17, unchecked{}}; - static_assert(a0.isnormal()); - static_assert(a1.isnormal()); - static_assert(a2.isnormal()); - Number rm2{}; - Number rm1{}; - Number r = (a2 * d + a1) * d + a0; - // Newton–Raphson iteration of 1/x - d with initial guess r - // halt when r stops changing, checking for bouncing on the last iteration - do - { - rm2 = rm1; - rm1 = r; - r = r + r * (one - d * r); - } while (r != rm1 && r != rm2); - *this = n * r; + // Shift by 10^17 gives greatest precision while not overflowing uint128_t + // or the cast back to int64_t + const uint128_t f = 100'000'000'000'000'000; + mantissa_ = static_cast(uint128_t(nm) * f / uint128_t(dm)); + exponent_ = ne - de - 17; + mantissa_ *= np * dp; + normalize(); return *this; } diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index d3ece630e3e..8c12ff7c5e4 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -150,25 +150,94 @@ class Number_test : public beast::unit_test::suite { testcase("test_mul"); using Case = std::tuple; - Case c[]{ - {Number{7}, Number{8}, Number{56}}, - {Number{1414213562373095, -15}, - Number{1414213562373095, -15}, - Number{2000000000000000, -15}}, - {Number{-1414213562373095, -15}, - Number{1414213562373095, -15}, - Number{-2000000000000000, -15}}, - {Number{-1414213562373095, -15}, - Number{-1414213562373095, -15}, - Number{2000000000000000, -15}}, - {Number{3214285714285706, -15}, - Number{3111111111111119, -15}, - Number{1000000000000000, -14}}, - {Number{1000000000000000, -32768}, - Number{1000000000000000, -32768}, - Number{0}}}; - for (auto const& [x, y, z] : c) - BEAST_EXPECT(x * y == z); + saveNumberRoundMode save{Number::setround(Number::to_nearest)}; + { + Case c[]{ + {Number{7}, Number{8}, Number{56}}, + {Number{1414213562373095, -15}, + Number{1414213562373095, -15}, + Number{2000000000000000, -15}}, + {Number{-1414213562373095, -15}, + Number{1414213562373095, -15}, + Number{-2000000000000000, -15}}, + {Number{-1414213562373095, -15}, + Number{-1414213562373095, -15}, + Number{2000000000000000, -15}}, + {Number{3214285714285706, -15}, + Number{3111111111111119, -15}, + Number{1000000000000000, -14}}, + {Number{1000000000000000, -32768}, + Number{1000000000000000, -32768}, + Number{0}}}; + for (auto const& [x, y, z] : c) + BEAST_EXPECT(x * y == z); + } + Number::setround(Number::towards_zero); + { + Case c[]{ + {Number{7}, Number{8}, Number{56}}, + {Number{1414213562373095, -15}, + Number{1414213562373095, -15}, + Number{1999999999999999, -15}}, + {Number{-1414213562373095, -15}, + Number{1414213562373095, -15}, + Number{-1999999999999999, -15}}, + {Number{-1414213562373095, -15}, + Number{-1414213562373095, -15}, + Number{1999999999999999, -15}}, + {Number{3214285714285706, -15}, + Number{3111111111111119, -15}, + Number{9999999999999999, -15}}, + {Number{1000000000000000, -32768}, + Number{1000000000000000, -32768}, + Number{0}}}; + for (auto const& [x, y, z] : c) + BEAST_EXPECT(x * y == z); + } + Number::setround(Number::downward); + { + Case c[]{ + {Number{7}, Number{8}, Number{56}}, + {Number{1414213562373095, -15}, + Number{1414213562373095, -15}, + Number{1999999999999999, -15}}, + {Number{-1414213562373095, -15}, + Number{1414213562373095, -15}, + Number{-2000000000000000, -15}}, + {Number{-1414213562373095, -15}, + Number{-1414213562373095, -15}, + Number{1999999999999999, -15}}, + {Number{3214285714285706, -15}, + Number{3111111111111119, -15}, + Number{9999999999999999, -15}}, + {Number{1000000000000000, -32768}, + Number{1000000000000000, -32768}, + Number{0}}}; + for (auto const& [x, y, z] : c) + BEAST_EXPECT(x * y == z); + } + Number::setround(Number::upward); + { + Case c[]{ + {Number{7}, Number{8}, Number{56}}, + {Number{1414213562373095, -15}, + Number{1414213562373095, -15}, + Number{2000000000000000, -15}}, + {Number{-1414213562373095, -15}, + Number{1414213562373095, -15}, + Number{-1999999999999999, -15}}, + {Number{-1414213562373095, -15}, + Number{-1414213562373095, -15}, + Number{2000000000000000, -15}}, + {Number{3214285714285706, -15}, + Number{3111111111111119, -15}, + Number{1000000000000000, -14}}, + {Number{1000000000000000, -32768}, + Number{1000000000000000, -32768}, + Number{0}}}; + for (auto const& [x, y, z] : c) + BEAST_EXPECT(x * y == z); + } bool caught = false; try { @@ -187,13 +256,78 @@ class Number_test : public beast::unit_test::suite { testcase("test_div"); using Case = std::tuple; - Case c[]{ - {Number{1}, Number{2}, Number{5, -1}}, - {Number{1}, Number{10}, Number{1, -1}}, - {Number{1}, Number{-10}, Number{-1, -1}}, - {Number{0}, Number{100}, Number{0}}}; - for (auto const& [x, y, z] : c) - BEAST_EXPECT(x / y == z); + saveNumberRoundMode save{Number::setround(Number::to_nearest)}; + { + Case c[]{ + {Number{1}, Number{2}, Number{5, -1}}, + {Number{1}, Number{10}, Number{1, -1}}, + {Number{1}, Number{-10}, Number{-1, -1}}, + {Number{0}, Number{100}, Number{0}}, + {Number{1414213562373095, -10}, + Number{1414213562373095, -10}, + Number{1}}, + {Number{9'999'999'999'999'999}, + Number{1'000'000'000'000'000}, + Number{9'999'999'999'999'999, -15}}, + {Number{2}, Number{3}, Number{6'666'666'666'666'667, -16}}, + {Number{-2}, Number{3}, Number{-6'666'666'666'666'667, -16}}}; + for (auto const& [x, y, z] : c) + BEAST_EXPECT(x / y == z); + } + Number::setround(Number::towards_zero); + { + Case c[]{ + {Number{1}, Number{2}, Number{5, -1}}, + {Number{1}, Number{10}, Number{1, -1}}, + {Number{1}, Number{-10}, Number{-1, -1}}, + {Number{0}, Number{100}, Number{0}}, + {Number{1414213562373095, -10}, + Number{1414213562373095, -10}, + Number{1}}, + {Number{9'999'999'999'999'999}, + Number{1'000'000'000'000'000}, + Number{9'999'999'999'999'999, -15}}, + {Number{2}, Number{3}, Number{6'666'666'666'666'666, -16}}, + {Number{-2}, Number{3}, Number{-6'666'666'666'666'666, -16}}}; + for (auto const& [x, y, z] : c) + BEAST_EXPECT(x / y == z); + } + Number::setround(Number::downward); + { + Case c[]{ + {Number{1}, Number{2}, Number{5, -1}}, + {Number{1}, Number{10}, Number{1, -1}}, + {Number{1}, Number{-10}, Number{-1, -1}}, + {Number{0}, Number{100}, Number{0}}, + {Number{1414213562373095, -10}, + Number{1414213562373095, -10}, + Number{1}}, + {Number{9'999'999'999'999'999}, + Number{1'000'000'000'000'000}, + Number{9'999'999'999'999'999, -15}}, + {Number{2}, Number{3}, Number{6'666'666'666'666'666, -16}}, + {Number{-2}, Number{3}, Number{-6'666'666'666'666'667, -16}}}; + for (auto const& [x, y, z] : c) + BEAST_EXPECT(x / y == z); + } + Number::setround(Number::upward); + { + Case c[]{ + {Number{1}, Number{2}, Number{5, -1}}, + {Number{1}, Number{10}, Number{1, -1}}, + {Number{1}, Number{-10}, Number{-1, -1}}, + {Number{0}, Number{100}, Number{0}}, + {Number{1414213562373095, -10}, + Number{1414213562373095, -10}, + Number{1}}, + {Number{9'999'999'999'999'999}, + Number{1'000'000'000'000'000}, + Number{9'999'999'999'999'999, -15}}, + {Number{2}, Number{3}, Number{6'666'666'666'666'667, -16}}, + {Number{-2}, Number{3}, Number{-6'666'666'666'666'666, -16}}}; + for (auto const& [x, y, z] : c) + BEAST_EXPECT(x / y == z); + } bool caught = false; try { From c954a854ed15f7783c3534f21c55e28466a3dbc6 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Sat, 28 Jan 2023 20:26:59 -0500 Subject: [PATCH 12/13] Optimize uint128_t division by 10 within Number.cpp * Optimization includes computing remainder from division. * Used only within Number::operator*=. --- src/ripple/basics/impl/IOUAmount.cpp | 2 +- src/ripple/basics/impl/Number.cpp | 35 ++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/ripple/basics/impl/IOUAmount.cpp b/src/ripple/basics/impl/IOUAmount.cpp index 76f4bbe9fcd..f011af19c01 100644 --- a/src/ripple/basics/impl/IOUAmount.cpp +++ b/src/ripple/basics/impl/IOUAmount.cpp @@ -53,7 +53,7 @@ IOUAmount::normalize() if (*stNumberSwitchover) { - Number v{mantissa_, exponent_}; + const Number v{mantissa_, exponent_}; mantissa_ = v.mantissa(); exponent_ = v.exponent(); if (exponent_ > maxExponent) diff --git a/src/ripple/basics/impl/Number.cpp b/src/ripple/basics/impl/Number.cpp index 52d2b556d44..9b3247536f9 100644 --- a/src/ripple/basics/impl/Number.cpp +++ b/src/ripple/basics/impl/Number.cpp @@ -18,6 +18,7 @@ //============================================================================== #include +#include #include #include #include @@ -335,6 +336,34 @@ Number::operator+=(Number const& y) 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) { @@ -370,8 +399,10 @@ Number::operator*=(Number const& y) g.set_negative(); while (zm > maxMantissa) { - g.push(static_cast(zm % 10)); - zm /= 10; + // The following is optimization for: + // g.push(static_cast(zm % 10)); + // zm /= 10; + g.push(divu10(zm)); ++ze; } xm = static_cast(zm); From ff1495ba9c8b520d75dfd1ed341958daf4b6f9a9 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Wed, 1 Feb 2023 11:20:13 -0500 Subject: [PATCH 13/13] Introduce min/max observers for Number Three static member functions are introduced with definitions consistent with std::numeric_limits: static constexpr Number min() noexcept; Returns: The minimum positive value. This is the value closest to zero. static constexpr Number max() noexcept; Returns: The maximum possible value. static constexpr Number lowest() noexcept; Returns: The negative value which is less than all other values. --- src/ripple/basics/Number.h | 25 +++++++++++++++++++++++++ src/ripple/basics/impl/IOUAmount.cpp | 2 +- src/ripple/protocol/Feature.h | 3 --- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/ripple/basics/Number.h b/src/ripple/basics/Number.h index 58d903579b5..c308abec712 100644 --- a/src/ripple/basics/Number.h +++ b/src/ripple/basics/Number.h @@ -81,6 +81,13 @@ class Number Number& operator/=(Number const& x); + static constexpr Number + min() noexcept; + static constexpr Number + max() noexcept; + static constexpr Number + lowest() noexcept; + explicit operator XRPAmount() const; // round to nearest, even on tie explicit operator rep() const; // round to nearest, even on tie @@ -290,6 +297,24 @@ operator/(Number const& x, Number const& y) return z; } +inline constexpr Number +Number::min() noexcept +{ + return Number{minMantissa, minExponent, unchecked{}}; +} + +inline constexpr Number +Number::max() noexcept +{ + return Number{maxMantissa, maxExponent, unchecked{}}; +} + +inline constexpr Number +Number::lowest() noexcept +{ + return -Number{maxMantissa, maxExponent, unchecked{}}; +} + inline constexpr bool Number::isnormal() const noexcept { diff --git a/src/ripple/basics/impl/IOUAmount.cpp b/src/ripple/basics/impl/IOUAmount.cpp index f011af19c01..c9b52874abd 100644 --- a/src/ripple/basics/impl/IOUAmount.cpp +++ b/src/ripple/basics/impl/IOUAmount.cpp @@ -53,7 +53,7 @@ IOUAmount::normalize() if (*stNumberSwitchover) { - const Number v{mantissa_, exponent_}; + Number const v{mantissa_, exponent_}; mantissa_ = v.mantissa(); exponent_ = v.exponent(); if (exponent_ > maxExponent) diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 6be2d4dfb68..78b5b152c87 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -340,12 +340,9 @@ extern uint256 const featureNonFungibleTokensV1_1; extern uint256 const fixTrustLinesToSelf; extern uint256 const fixRemoveNFTokenAutoTrustLine; extern uint256 const featureImmediateOfferKilled; -<<<<<<< HEAD extern uint256 const featureDisallowIncoming; extern uint256 const featureXRPFees; -======= extern uint256 const fixUniversalNumber; ->>>>>>> Use Number for IOUAmount and STAmount arithmetic } // namespace ripple