Refactor Number, add tests, update LoanManage, update LoanPay

- Loan tests are not expected to pass.
- Refactor Number to put rounding logic into reusable functions.
- Add Number tests to explicitly test rounding - may be redundant, but
  easy to reason about.  Verifies that the default rounding matches
  banker's rounding.
- Enable LoanManage. Apparently any remaining issues were fixed by the
  previous commit's cleanups.
- Partially enabled LoanPay.

src/libxrpl/basics/Number.cpp

@@ -93,6 +93,18 @@ public:
     // tie, round towards even.
     int
     round() noexcept;
+
+    // Modify the result to the correctly rounded value
+    void
+    doRoundUp(rep& mantissa, int& exponent, std::string location);
+
+    // Modify the result to the correctly rounded value
+    void
+    doRoundDown(rep& mantissa, int& exponent);
+
+    // Modify the result to the correctly rounded value
+    void
+    doRound(rep& drops);
 };
 
 inline void
@@ -170,6 +182,61 @@ Number::Guard::round() noexcept
     return 0;
 }
 
+void
+Number::Guard::doRoundUp(rep& mantissa, int& exponent, std::string location)
+{
+    auto r = round();
+    if (r == 1 || (r == 0 && (mantissa & 1) == 1))
+    {
+        ++mantissa;
+        if (mantissa > maxMantissa)
+        {
+            mantissa /= 10;
+            ++exponent;
+        }
+    }
+    if (exponent < minExponent)
+    {
+        mantissa = 0;
+        exponent = Number{}.exponent_;
+    }
+    if (exponent > maxExponent)
+        throw std::overflow_error(location);
+}
+
+void
+Number::Guard::doRoundDown(rep& mantissa, int& exponent)
+{
+    auto r = round();
+    if (r == 1 || (r == 0 && (mantissa & 1) == 1))
+    {
+        --mantissa;
+        if (mantissa < minMantissa)
+        {
+            mantissa *= 10;
+            --exponent;
+        }
+    }
+    if (exponent < minExponent)
+    {
+        mantissa = 0;
+        exponent = Number{}.exponent_;
+    }
+}
+
+// Modify the result to the correctly rounded value
+void
+Number::Guard::doRound(rep& drops)
+{
+    auto r = round();
+    if (r == 1 || (r == 0 && (drops & 1) == 1))
+    {
+        ++drops;
+    }
+    if (is_negative())
+        drops = -drops;
+}
+
 // Number
 
 constexpr Number one{1000000000000000, -15, Number::unchecked{}};
@@ -209,18 +276,7 @@ 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");
+    g.doRoundUp(mantissa_, exponent_, "Number::normalize 2");
 
     if (negative)
         mantissa_ = -mantissa_;
@@ -292,18 +348,7 @@ Number::operator+=(Number const& y)
             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");
+        g.doRoundUp(xm, xe, "Number::addition overflow");
     }
     else
     {
@@ -323,21 +368,7 @@ Number::operator+=(Number const& y)
             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_;
-        }
+        g.doRoundDown(xm, xe);
     }
     mantissa_ = xm * xn;
     exponent_ = xe;
@@ -417,25 +448,10 @@ Number::operator*=(Number const& y)
     }
     xm = static_cast<rep>(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));
+    g.doRoundUp(
+        xm,
+        xe,
+        "Number::multiplication overflow : exponent is " + std::to_string(xe));
     mantissa_ = xm * zn;
     exponent_ = xe;
     XRPL_ASSERT(
@@ -500,13 +516,7 @@ Number::operator rep() const
                 throw std::overflow_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;
+        g.doRound(drops);
     }
     return drops;
 }

src/test/basics/Number_test.cpp

@@ -744,6 +744,115 @@ public:
         BEAST_EXPECT(Number(-100, -30000).truncate() == Number(0, 0));
     }
 
+    void
+    testRounding()
+    {
+        // Test that rounding works as expected.
+        testcase("Rounding");
+
+        using NumberRoundings = std::map<Number::rounding_mode, std::int64_t>;
+
+        std::map<Number, NumberRoundings> const expected{
+            // Positive numbers
+            {Number{13, -1},
+             {{Number::to_nearest, 1},
+              {Number::towards_zero, 1},
+              {Number::downward, 1},
+              {Number::upward, 2}}},
+            {Number{23, -1},
+             {{Number::to_nearest, 2},
+              {Number::towards_zero, 2},
+              {Number::downward, 2},
+              {Number::upward, 3}}},
+            {Number{15, -1},
+             {{Number::to_nearest, 2},
+              {Number::towards_zero, 1},
+              {Number::downward, 1},
+              {Number::upward, 2}}},
+            {Number{25, -1},
+             {{Number::to_nearest, 2},
+              {Number::towards_zero, 2},
+              {Number::downward, 2},
+              {Number::upward, 3}}},
+            {Number{152, -2},
+             {{Number::to_nearest, 2},
+              {Number::towards_zero, 1},
+              {Number::downward, 1},
+              {Number::upward, 2}}},
+            {Number{252, -2},
+             {{Number::to_nearest, 3},
+              {Number::towards_zero, 2},
+              {Number::downward, 2},
+              {Number::upward, 3}}},
+            {Number{17, -1},
+             {{Number::to_nearest, 2},
+              {Number::towards_zero, 1},
+              {Number::downward, 1},
+              {Number::upward, 2}}},
+            {Number{27, -1},
+             {{Number::to_nearest, 3},
+              {Number::towards_zero, 2},
+              {Number::downward, 2},
+              {Number::upward, 3}}},
+
+            // Negative numbers
+            {Number{-13, -1},
+             {{Number::to_nearest, -1},
+              {Number::towards_zero, -1},
+              {Number::downward, -2},
+              {Number::upward, -1}}},
+            {Number{-23, -1},
+             {{Number::to_nearest, -2},
+              {Number::towards_zero, -2},
+              {Number::downward, -3},
+              {Number::upward, -2}}},
+            {Number{-15, -1},
+             {{Number::to_nearest, -2},
+              {Number::towards_zero, -1},
+              {Number::downward, -2},
+              {Number::upward, -1}}},
+            {Number{-25, -1},
+             {{Number::to_nearest, -2},
+              {Number::towards_zero, -2},
+              {Number::downward, -3},
+              {Number::upward, -2}}},
+            {Number{-152, -2},
+             {{Number::to_nearest, -2},
+              {Number::towards_zero, -1},
+              {Number::downward, -2},
+              {Number::upward, -1}}},
+            {Number{-252, -2},
+             {{Number::to_nearest, -3},
+              {Number::towards_zero, -2},
+              {Number::downward, -3},
+              {Number::upward, -2}}},
+            {Number{-17, -1},
+             {{Number::to_nearest, -2},
+              {Number::towards_zero, -1},
+              {Number::downward, -2},
+              {Number::upward, -1}}},
+            {Number{-27, -1},
+             {{Number::to_nearest, -3},
+              {Number::towards_zero, -2},
+              {Number::downward, -3},
+              {Number::upward, -2}}},
+        };
+
+        for (auto const& [num, roundings] : expected)
+        {
+            for (auto const& [mode, val] : roundings)
+            {
+                NumberRoundModeGuard g{mode};
+                auto const res = static_cast<std::int64_t>(num);
+                BEAST_EXPECTS(
+                    res == val,
+                    to_string(num) + " with mode " + std::to_string(mode) +
+                        " expected " + std::to_string(val) + " got " +
+                        std::to_string(res));
+            }
+        }
+    }
+
     void
     run() override
     {
@@ -765,6 +874,7 @@ public:
         test_inc_dec();
         test_toSTAmount();
         test_truncate();
+        testRounding();
     }
 };
 

src/xrpld/app/tx/detail/LoanManage.cpp

@@ -367,8 +367,6 @@ LoanManage::unimpairLoan(
 TER
 LoanManage::doApply()
 {
-    return temDISABLED;
-#if LOANCOMPLETE
     auto const& tx = ctx_.tx;
     auto& view = ctx_.view();
 
@@ -407,7 +405,6 @@ LoanManage::doApply()
     }
 
     return tesSUCCESS;
-#endif
 }
 
 //------------------------------------------------------------------------------

src/xrpld/app/tx/detail/LoanPay.cpp

@@ -45,8 +45,6 @@ LoanPay::preflight(PreflightContext const& ctx)
 TER
 LoanPay::preclaim(PreclaimContext const& ctx)
 {
-    return temDISABLED;
-#if LOANCOMPLETE
     auto const& tx = ctx.tx;
 
     auto const account = tx[sfAccount];
@@ -119,7 +117,6 @@ LoanPay::preclaim(PreclaimContext const& ctx)
     }
 
     return tesSUCCESS;
-#endif
 }
 
 TER