Add a distinction between a "valid" and a "representable" Number

- "valid" means the value is <= Number::maxIntValue, which has been
  changed to maxMantissa / 100. A valid number could get bigger and be
  ok - such as when paying late interest on a loan.
- "representable" means the value is <= Number::maxMantissa. An
  unrepresentable number WILL be rounded or truncated.
- Adds a fourth level of enforcement: "compatible". It is used for
  converting XRP to Number (for AMM), and when doing explicit checks.
- "weak" will now throw if the number is unrepresentable.

include/xrpl/basics/Number.h

@@ -28,12 +28,17 @@ public:
     /** Describes whether and how to enforce this number as an integer.
      *
      * - none: No enforcement. The value may vary freely. This is the default.
-     * - weak: If the absolute value is greater than maxIntValue, valid() will
+     * - compatible: If the absolute value is greater than maxIntValue, valid()
-     *   return false.
+     *   will return false. Needed for backward compatibility with XRP used in
-     * - strong: Assignment operations will throw if the absolute value is above
+     *   AMMs, and available for functions that will do their own checking. This
-     *   maxIntValue.
+     *   is the default for automatic conversions from XRPAmount to Number.
+     * - weak: Like compatible, plus, if the value is unrepresentable (larger
+     *   than maxMantissa), assignment and other operations will throw.
+     * - strong: Like weak, plus, if the absolute value is invalid (larger than
+     *   maxIntValue), assignment and other operations will throw. This is the
+     *   defalut for automatic conversions from MPTAmount to Number.
      */
-    enum EnforceInteger { none, weak, strong };
+    enum EnforceInteger { none, compatible, weak, strong };
 
 private:
     using rep = std::int64_t;
@@ -42,8 +47,7 @@ private:
 
     // The enforcement setting is not serialized, and does not affect the
     // ledger. If not "none", the value is checked to be within the valid
-    // integer range. With "strong", the checks will be made as automatic as
+    // integer range. See the enum description for more detail.
-    // possible.
     EnforceInteger enforceInteger_ = none;
 
 public:
@@ -53,8 +57,8 @@ public:
     constexpr static rep maxMantissa = minMantissa * 10 - 1;
     static_assert(maxMantissa == 9'999'999'999'999'999LL);
 
-    constexpr static rep maxIntValue = maxMantissa / 10;
+    constexpr static rep maxIntValue = maxMantissa / 100;
-    static_assert(maxIntValue == 999'999'999'999'999LL);
+    static_assert(maxIntValue == 99'999'999'999'999LL);
 
     // The range for the exponent when normalized
     constexpr static int minExponent = -32768;
@@ -93,6 +97,15 @@ public:
 
     bool
     valid() const noexcept;
+    bool
+    representable() const noexcept;
+    /// Combines setIntegerEnforcement(EnforceInteger) and valid()
+    bool
+    valid(EnforceInteger enforce);
+    /// Because this function is const, it should only be used for one-off
+    /// checks
+    bool
+    valid(EnforceInteger enforce) const;
 
     constexpr Number
     operator+() const noexcept;

include/xrpl/protocol/XRPAmount.h

@@ -143,7 +143,7 @@ public:
 
     operator Number() const noexcept
     {
-        return {drops(), Number::weak};
+        return {drops(), Number::compatible};
     }
 
     Number

src/libxrpl/basics/Number.cpp

@@ -160,6 +160,8 @@ Number::checkInteger(char const* what) const
 {
     if (enforceInteger_ == strong && !valid())
         throw std::overflow_error(what);
+    if (enforceInteger_ == weak && !representable())
+        throw std::overflow_error(what);
 }
 
 void
@@ -217,7 +219,20 @@ Number::normalize()
 bool
 Number::valid() const noexcept
 {
-    if (enforceInteger_ != none)
+    return valid(enforceInteger_);
+}
+
+bool
+Number::valid(EnforceInteger enforce)
+{
+    setIntegerEnforcement(enforce);
+    return valid();
+}
+
+bool
+Number::valid(EnforceInteger enforce) const
+{
+    if (enforce != none)
     {
         static Number const max = maxIntValue;
         static Number const maxNeg = -maxIntValue;
@@ -229,6 +244,21 @@ Number::valid() const noexcept
     return true;
 }
 
+bool
+Number::representable() const noexcept
+{
+    if (enforceInteger_ != none)
+    {
+        static Number const max = maxMantissa;
+        static Number const maxNeg = -maxMantissa;
+        // Avoid making a copy
+        if (mantissa_ < 0)
+            return *this >= maxNeg;
+        return *this <= max;
+    }
+    return true;
+}
+
 Number&
 Number::operator+=(Number const& y)
 {

src/libxrpl/ledger/View.cpp

@@ -2878,6 +2878,8 @@ assetsToSharesDeposit(
 
     Number const assetTotal = vault->at(sfAssetsTotal);
     STAmount shares{vault->at(sfShareMPTID)};
+    // STAmount will ignore enforcement for IOUs, so we can set it regardless of
+    // type.
     shares.setIntegerEnforcement(Number::weak);
     if (assetTotal == 0)
         return STAmount{
@@ -2910,6 +2912,8 @@ sharesToAssetsDeposit(
 
     Number const assetTotal = vault->at(sfAssetsTotal);
     STAmount assets{vault->at(sfAsset)};
+    // STAmount will ignore enforcement for IOUs, so we can set it regardless of
+    // type.
     assets.setIntegerEnforcement(Number::weak);
     if (assetTotal == 0)
         return STAmount{
@@ -2944,6 +2948,8 @@ assetsToSharesWithdraw(
     Number assetTotal = vault->at(sfAssetsTotal);
     assetTotal -= vault->at(sfLossUnrealized);
     STAmount shares{vault->at(sfShareMPTID)};
+    // STAmount will ignore enforcement for IOUs, so we can set it regardless of
+    // type.
     shares.setIntegerEnforcement(Number::weak);
     if (assetTotal == 0)
         return shares;
@@ -2975,6 +2981,8 @@ sharesToAssetsWithdraw(
     Number assetTotal = vault->at(sfAssetsTotal);
     assetTotal -= vault->at(sfLossUnrealized);
     STAmount assets{vault->at(sfAsset)};
+    // STAmount will ignore enforcement for IOUs, so we can set it regardless of
+    // type.
     assets.setIntegerEnforcement(Number::weak);
     if (assetTotal == 0)
         return assets;

src/libxrpl/protocol/STAmount.cpp

@@ -270,7 +270,8 @@ STAmount::integerEnforcement() const noexcept
 bool
 STAmount::validNumber() const noexcept
 {
-    Number n = toNumber(Number::EnforceInteger::weak);
+    // compatible will not throw.
+    Number n = toNumber(Number::EnforceInteger::compatible);
     return n.valid();
 }
 

src/test/app/Vault_test.cpp

@@ -3611,7 +3611,7 @@ class Vault_test : public beast::unit_test::suite
             }
         });
 
-        testCase(14, [&, this](Env& env, Data d) {
+        testCase(13, [&, this](Env& env, Data d) {
             testcase("MPT scale deposit overflow on first deposit");
             auto tx = d.vault.deposit(
                 {.depositor = d.depositor,
@@ -3621,7 +3621,7 @@ class Vault_test : public beast::unit_test::suite
             env.close();
         });
 
-        testCase(14, [&, this](Env& env, Data d) {
+        testCase(13, [&, this](Env& env, Data d) {
             testcase("MPT scale deposit overflow on second deposit");
 
             {
@@ -3643,7 +3643,7 @@ class Vault_test : public beast::unit_test::suite
             }
         });
 
-        testCase(14, [&, this](Env& env, Data d) {
+        testCase(13, [&, this](Env& env, Data d) {
             testcase("No MPT scale deposit overflow on total shares");
 
             {
@@ -3963,7 +3963,7 @@ class Vault_test : public beast::unit_test::suite
             }
         });
 
-        testCase(14, [&, this](Env& env, Data d) {
+        testCase(13, [&, this](Env& env, Data d) {
             testcase("MPT scale withdraw overflow");
 
             {
@@ -4204,7 +4204,7 @@ class Vault_test : public beast::unit_test::suite
             }
         });
 
-        testCase(14, [&, this](Env& env, Data d) {
+        testCase(13, [&, this](Env& env, Data d) {
             testcase("MPT Scale clawback overflow");
 
             {

src/test/basics/Number_test.cpp

@@ -737,28 +737,87 @@ public:
             Number a{100};
             BEAST_EXPECT(a.integerEnforcement() == Number::none);
             BEAST_EXPECT(a.valid());
+            BEAST_EXPECT(a.representable());
             a = Number{1, 30};
             BEAST_EXPECT(a.valid());
+            BEAST_EXPECT(a.representable());
             a = -100;
             BEAST_EXPECT(a.valid());
+            BEAST_EXPECT(a.representable());
+        }
+        {
+            Number a{100, Number::compatible};
+            BEAST_EXPECT(a.integerEnforcement() == Number::compatible);
+            BEAST_EXPECT(a.valid());
+            BEAST_EXPECT(a.representable());
+            a = Number{1, 15};
+            BEAST_EXPECT(!a.valid());
+            BEAST_EXPECT(a.representable());
+            a = Number{1, 30, Number::none};
+            BEAST_EXPECT(!a.valid());
+            BEAST_EXPECT(!a.representable());
+            a = -100;
+            BEAST_EXPECT(a.integerEnforcement() == Number::compatible);
+            BEAST_EXPECT(a.valid());
+            BEAST_EXPECT(a.representable());
+            a = Number{5, Number::weak};
+            BEAST_EXPECT(a.integerEnforcement() == Number::weak);
+            BEAST_EXPECT(a.valid());
+            BEAST_EXPECT(a.representable());
+            a = Number{5, Number::strong};
+            BEAST_EXPECT(a.integerEnforcement() == Number::strong);
+            BEAST_EXPECT(a.valid());
+            BEAST_EXPECT(a.representable());
         }
         {
             Number a{100, Number::weak};
             BEAST_EXPECT(a.integerEnforcement() == Number::weak);
             BEAST_EXPECT(a.valid());
-            a = Number{1, 30, Number::none};
+            BEAST_EXPECT(a.representable());
+            a = Number{1, 15};
             BEAST_EXPECT(!a.valid());
+            BEAST_EXPECT(a.representable());
+            try
+            {
+                a = Number{1, 30, Number::compatible};
+                BEAST_EXPECT(false);
+            }
+            catch (std::overflow_error const& e)
+            {
+                BEAST_EXPECT(e.what() == "Number::operator= integer overflow"s);
+                // The throw is done _after_ the number is updated.
+                BEAST_EXPECT((a == Number{1, 30}));
+            }
+            BEAST_EXPECT(a.integerEnforcement() == Number::weak);
+            BEAST_EXPECT(!a.valid());
+            BEAST_EXPECT(!a.representable());
             a = -100;
             BEAST_EXPECT(a.integerEnforcement() == Number::weak);
             BEAST_EXPECT(a.valid());
+            BEAST_EXPECT(a.representable());
             a = Number{5, Number::strong};
             BEAST_EXPECT(a.integerEnforcement() == Number::strong);
             BEAST_EXPECT(a.valid());
+            BEAST_EXPECT(a.representable());
         }
         {
             Number a{100, Number::strong};
             BEAST_EXPECT(a.integerEnforcement() == Number::strong);
             BEAST_EXPECT(a.valid());
+            BEAST_EXPECT(a.representable());
+            try
+            {
+                a = Number{1, 15, Number::compatible};
+                BEAST_EXPECT(false);
+            }
+            catch (std::overflow_error const& e)
+            {
+                BEAST_EXPECT(e.what() == "Number::operator= integer overflow"s);
+                // The throw is done _after_ the number is updated.
+                BEAST_EXPECT((a == Number{1, 15}));
+            }
+            BEAST_EXPECT(!a.valid());
+            BEAST_EXPECT(a.representable());
             try
             {
                 a = Number{1, 30};
@@ -771,15 +830,19 @@ public:
                 BEAST_EXPECT((a == Number{1, 30}));
             }
             BEAST_EXPECT(!a.valid());
+            BEAST_EXPECT(!a.representable());
             a = -100;
             BEAST_EXPECT(a.integerEnforcement() == Number::strong);
             BEAST_EXPECT(a.valid());
+            BEAST_EXPECT(a.representable());
         }
         {
-            Number a{INITIAL_XRP.drops(), Number::weak};
+            Number a{INITIAL_XRP.drops(), Number::compatible};
             BEAST_EXPECT(!a.valid());
+            BEAST_EXPECT(!a.representable());
             a = -a;
             BEAST_EXPECT(!a.valid());
+            BEAST_EXPECT(!a.representable());
 
             try
             {
@@ -795,6 +858,7 @@ public:
                 // assigned to the Number
                 BEAST_EXPECT(a == -INITIAL_XRP);
                 BEAST_EXPECT(!a.valid());
+                BEAST_EXPECT(!a.representable());
             }
             try
             {
@@ -808,6 +872,7 @@ public:
                 // assigned to the Number
                 BEAST_EXPECT(a == -INITIAL_XRP);
                 BEAST_EXPECT(!a.valid());
+                BEAST_EXPECT(!a.representable());
             }
             a = Number::maxIntValue;
             try
@@ -821,6 +886,7 @@ public:
                 // This time, the throw is done _after_ the number is updated.
                 BEAST_EXPECT(a == Number::maxIntValue + 1);
                 BEAST_EXPECT(!a.valid());
+                BEAST_EXPECT(a.representable());
             }
             a = -Number::maxIntValue;
             try
@@ -834,6 +900,7 @@ public:
                 // This time, the throw is done _after_ the number is updated.
                 BEAST_EXPECT(a == -Number::maxIntValue - 1);
                 BEAST_EXPECT(!a.valid());
+                BEAST_EXPECT(a.representable());
             }
             a = Number(1, 10);
             try
@@ -848,6 +915,7 @@ public:
                 // The throw is done _after_ the number is updated.
                 BEAST_EXPECT((a == Number{1, 20}));
                 BEAST_EXPECT(!a.valid());
+                BEAST_EXPECT(!a.representable());
             }
             try
             {
@@ -860,6 +928,7 @@ public:
                 // The throw is done _after_ the number is updated.
                 BEAST_EXPECT((a == Number::maxIntValue * 2));
                 BEAST_EXPECT(!a.valid());
+                BEAST_EXPECT(a.representable());
             }
             try
             {
@@ -872,6 +941,7 @@ public:
                 // The Number doesn't get updated because the ctor throws
                 BEAST_EXPECT((a == Number::maxIntValue * 2));
                 BEAST_EXPECT(!a.valid());
+                BEAST_EXPECT(a.representable());
             }
             a = Number(1, 10);
             try
@@ -885,10 +955,12 @@ public:
                 // The throw is done _after_ the number is updated.
                 BEAST_EXPECT((a == Number{1, 20}));
                 BEAST_EXPECT(!a.valid());
+                BEAST_EXPECT(!a.representable());
             }
             a /= Number(1, 15);
             BEAST_EXPECT((a == Number{1, 5}));
             BEAST_EXPECT(a.valid());
+            BEAST_EXPECT(a.representable());
         }
     }
 

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

@@ -2180,6 +2180,13 @@ ValidVault::Vault::make(SLE const& from)
     self.assetsAvailable = from.at(sfAssetsAvailable);
     self.assetsMaximum = from.at(sfAssetsMaximum);
     self.lossUnrealized = from.at(sfLossUnrealized);
+    if (self.asset.integral())
+    {
+        self.assetsTotal.setIntegerEnforcement(Number::compatible);
+        self.assetsAvailable.setIntegerEnforcement(Number::compatible);
+        self.assetsMaximum.setIntegerEnforcement(Number::compatible);
+        self.lossUnrealized.setIntegerEnforcement(Number::compatible);
+    }
     return self;
 }
 
@@ -2413,6 +2420,19 @@ ValidVault::finalize(
         beforeVault_.empty() || beforeVault_[0].key == afterVault.key,
         "ripple::ValidVault::finalize : single vault operation");
 
+    if (!afterVault.assetsTotal.representable() ||
+        !afterVault.assetsAvailable.representable() ||
+        !afterVault.assetsMaximum.representable() ||
+        !afterVault.lossUnrealized.representable())
+    {
+        JLOG(j.fatal()) << "Invariant failed: vault overflowed maximum current "
+                           "representable integer value";
+        XRPL_ASSERT(
+            enforce,
+            "ripple::ValidVault::finalize : vault integer limit invariant");
+        return !enforce;  // That's all we can do here
+    }
+
     auto const updatedShares = [&]() -> std::optional<Shares> {
         // At this moment we only know that a vault is being updated and there
         // might be some MPTokenIssuance objects which are also updated in the

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

@@ -150,8 +150,11 @@ VaultClawback::doApply()
         amount.asset() == vaultAsset,
         "ripple::VaultClawback::doApply : matching asset");
 
+    // Both of these values are going to be decreased in this transaction,
+    // so the limit doesn't really matter.
     auto assetsAvailable = vault->at(sfAssetsAvailable);
     auto assetsTotal = vault->at(sfAssetsTotal);
+
     [[maybe_unused]] auto const lossUnrealized = vault->at(sfLossUnrealized);
     XRPL_ASSERT(
         lossUnrealized <= (assetsTotal - assetsAvailable),
@@ -161,6 +164,8 @@ VaultClawback::doApply()
     MPTIssue const share{mptIssuanceID};
     STAmount sharesDestroyed = {share};
     STAmount assetsRecovered;
+    // STAmount will ignore enforcement for IOUs, so we can set it regardless of
+    // type.
     assetsRecovered.setIntegerEnforcement(Number::weak);
     sharesDestroyed.setIntegerEnforcement(Number::weak);
     try

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

@@ -210,6 +210,13 @@ VaultCreate::doApply()
         vault->at(sfWithdrawalPolicy) = vaultStrategyFirstComeFirstServe;
     if (scale)
         vault->at(sfScale) = scale;
+    if (asset.integral())
+    {
+        // Only the Maximum can be a non-zero value, so only it needs to be
+        // checked.
+        if (!vault->at(sfAssetsMaximum).value().valid(Number::compatible))
+            return tecLIMIT_EXCEEDED;
+    }
     view().insert(vault);
 
     // Explicitly create MPToken for the vault owner

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

@@ -220,6 +220,8 @@ VaultDeposit::doApply()
     }
 
     STAmount sharesCreated = {vault->at(sfShareMPTID)}, assetsDeposited;
+    // STAmount will ignore enforcement for IOUs, so we can set it regardless of
+    // type.
     sharesCreated.setIntegerEnforcement(Number::weak);
     try
     {

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

@@ -144,6 +144,11 @@ VaultSet::doApply()
             tx[sfAssetsMaximum] < *vault->at(sfAssetsTotal))
             return tecLIMIT_EXCEEDED;
         vault->at(sfAssetsMaximum) = tx[sfAssetsMaximum];
+        if (vault->at(sfAsset).value().integral())
+        {
+            if (!vault->at(sfAssetsMaximum).value().valid(Number::compatible))
+                return tecLIMIT_EXCEEDED;
+        }
     }
 
     if (auto const domainId = tx[~sfDomainID]; domainId)

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

@@ -222,6 +222,8 @@ VaultWithdraw::doApply()
         return tecINSUFFICIENT_FUNDS;
     }
 
+    // These values are only going to decrease, and can't be less than 0, so
+    // there's no need for integer range enforcement.
     auto assetsAvailable = vault->at(sfAssetsAvailable);
     auto assetsTotal = vault->at(sfAssetsTotal);
     [[maybe_unused]] auto const lossUnrealized = vault->at(sfLossUnrealized);