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

@@ -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.

src/ripple/app/paths/impl/AmountSpec.h

@@ -36,7 +36,7 @@ struct AmountSpec
     union
     {
         XRPAmount xrp;
-        IOUAmount iou;
+        IOUAmount iou = {};
     };
     std::optional<AccountID> issuer;
     std::optional<Currency> currency;
@@ -64,7 +64,7 @@ struct EitherAmount
 
     union
     {
-        IOUAmount iou;
+        IOUAmount iou = {};
         XRPAmount xrp;
     };
 

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
     {

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);

src/ripple/basics/IOUAmount.h

@@ -20,6 +20,8 @@
 #ifndef RIPPLE_BASICS_IOUAMOUNT_H_INCLUDED
 #define RIPPLE_BASICS_IOUAMOUNT_H_INCLUDED
 
+#include <ripple/basics/LocalValue.h>
+#include <ripple/basics/Number.h>
 #include <ripple/beast/utility/Zero.h>
 #include <boost/operators.hpp>
 #include <cstdint>
@@ -56,84 +58,119 @@ private:
 
 public:
     IOUAmount() = default;
-    IOUAmount(IOUAmount const& other) = default;
-    IOUAmount&
-    operator=(IOUAmount const& other) = default;
+    explicit IOUAmount(Number const& other);
+    IOUAmount(beast::Zero);
+    IOUAmount(std::int64_t mantissa, int exponent);
 
-    IOUAmount(beast::Zero)
-    {
-        *this = beast::zero;
-    }
+    IOUAmount& operator=(beast::Zero);
 
-    IOUAmount(std::int64_t mantissa, int exponent)
-        : mantissa_(mantissa), exponent_(exponent)
-    {
-        normalize();
-    }
-
-    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<bool> 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

src/ripple/basics/Number.h

@@ -20,7 +20,6 @@
 #ifndef RIPPLE_BASICS_NUMBER_H_INCLUDED
 #define RIPPLE_BASICS_NUMBER_H_INCLUDED
 
-#include <ripple/basics/IOUAmount.h>
 #include <ripple/basics/XRPAmount.h>
 #include <cstdint>
 #include <ostream>
@@ -51,7 +50,6 @@ public:
     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 @@ public:
     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
 {

src/ripple/basics/impl/IOUAmount.cpp

@@ -27,12 +27,14 @@
 
 namespace ripple {
 
+LocalValue<bool> 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<std::overflow_error>("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<std::overflow_error>("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)
+    else
     {
-        mantissa_ /= 10;
-        ++exponent_;
+        if (other == beast::zero)
+            return *this;
+
+        if (*this == beast::zero)
+        {
+            *this = other;
+            return *this;
+        }
+
+        auto m = other.mantissa_;
+        auto e = other.exponent_;
+
+        while (exponent_ < e)
+        {
+            mantissa_ /= 10;
+            ++exponent_;
+        }
+
+        while (e < exponent_)
+        {
+            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 (mantissa_ >= -10 && mantissa_ <= 10)
+        {
+            *this = beast::zero;
+            return *this;
+        }
+
+        normalize();
     }
-
-    while (e < exponent_)
-    {
-        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 (mantissa_ >= -10 && mantissa_ <= 10)
-    {
-        *this = beast::zero;
-        return *this;
-    }
-
-    normalize();
-
     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;
-
-    if (lneg != rneg)
-        return lneg;
-
-    // Both have same sign and the left is zero: the right must be
-    // greater than 0.
-    if (mantissa_ == 0)
-        return other.mantissa_ > 0;
-
-    // Both have same sign, the right is zero and the left is non-zero.
-    if (other.mantissa_ == 0)
-        return false;
-
-    // Both have the same sign, compare by exponents:
-    if (exponent_ > other.exponent_)
-        return lneg;
-    if (exponent_ < other.exponent_)
-        return !lneg;
-
-    // If equal exponents, compare mantissas
-    return mantissa_ < other.mantissa_;
-}
-
 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

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
 

src/ripple/protocol/STAmount.h

@@ -277,7 +277,13 @@ private:
     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);
 };
 
 //------------------------------------------------------------------------------

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.

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<std::uint64_t>(-iou.mantissa());
+    else
+        mValue = static_cast<std::uint64_t>(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<std::int64_t>(v1.mantissa());
     std::int64_t vv2 = static_cast<std::int64_t>(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();

src/test/app/Offer_test.cpp

@@ -2122,7 +2122,7 @@ public:
         jrr = ledgerEntryState(env, bob, gw, "USD");
         BEAST_EXPECT(
             jrr[jss::node][sfBalance.fieldName][jss::value] ==
-            "-0.966500000033334");
+            "-0.9665000000333333");
     }
 
     void

src/test/app/Taker_test.cpp

@@ -905,6 +905,7 @@ public:
     {
         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 @@ public:
             q1,
             {"4", "4"},
             "4",
-            {"2.666666666666666", "2.666666666666666"},
+            {"2.666666666666667", "2.666666666666667"},
             eur(),
             usd(),
             rate,
@@ -993,7 +994,7 @@ public:
             q1,
             {"2", "2"},
             "10",
-            {"1.666666666666666", "1.666666666666666"},
+            {"1.666666666666667", "1.666666666666667"},
             eur(),
             usd(),
             rate,

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,