Revert "Catch up the consequences of Number changes"

This reverts commit 0175dd70db.

include/xrpl/basics/Number.h

@@ -53,8 +53,7 @@ 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 = minMantissa / 10;
-    static_assert(maxIntValue == 999'999'999'999'999LL);
 
     // The range for the exponent when normalized
     constexpr static int minExponent = -32768;

include/xrpl/protocol/Asset.h

@@ -84,12 +84,6 @@ public:
         return holds<Issue>() && get<Issue>().native();
     }
 
-    bool
-    integral() const
-    {
-        return !holds<Issue>() || get<Issue>().native();
-    }
-
     friend constexpr bool
     operator==(Asset const& lhs, Asset const& rhs);
 

include/xrpl/protocol/MPTAmount.h

@@ -62,17 +62,11 @@ public:
     explicit constexpr
     operator bool() const noexcept;
 
-    operator Number() const
+    operator Number() const noexcept
     {
         return {value(), Number::strong};
     }
 
-    Number
-    toNumber(Number::EnforceInteger enforce) const
-    {
-        return {value(), enforce};
-    }
-
     /** Return the sign of the amount */
     constexpr int
     signum() const noexcept;

include/xrpl/protocol/STAmount.h

@@ -40,12 +40,6 @@ private:
     exponent_type mOffset;
     bool mIsNegative;
 
-    // The Enforce integer setting is not stored or serialized. If set, it is
-    // used during automatic conversions to Number. If not set, the default
-    // behavior is used. It can also be overridden when coverting by using
-    // toNumber().
-    std::optional<Number::EnforceInteger> enforceConversion_;
-
 public:
     using value_type = STAmount;
 
@@ -143,28 +137,9 @@ public:
     STAmount(A const& asset, int mantissa, int exponent = 0);
 
     template <AssetType A>
-    STAmount(
+    STAmount(A const& asset, Number const& number)
-        A const& asset,
-        Number const& number,
-        std::optional<Number::EnforceInteger> enforce = std::nullopt)
         : STAmount(asset, number.mantissa(), number.exponent())
     {
-        enforceConversion_ = enforce;
-        if (!enforce)
-        {
-            // Use the default conversion behavior
-            [[maybe_unused]]
-            Number const n = *this;
-        }
-        else if (enforce == Number::strong)
-        {
-            // Throw if it's not valid
-            if (!validNumber())
-            {
-                Throw<std::overflow_error>(
-                    "STAmount::STAmount integer Number lost precision");
-            }
-        }
     }
 
     // Legacy support for new-style amounts
@@ -172,17 +147,6 @@ public:
     STAmount(XRPAmount const& amount);
     STAmount(MPTAmount const& amount, MPTIssue const& mptIssue);
     operator Number() const;
-    Number
-    toNumber(Number::EnforceInteger enforce) const;
-
-    void
-    setIntegerEnforcement(std::optional<Number::EnforceInteger> enforce);
-
-    std::optional<Number::EnforceInteger>
-    integerEnforcement() const noexcept;
-
-    bool
-    validNumber() const noexcept;
 
     //--------------------------------------------------------------------------
     //
@@ -193,9 +157,6 @@ public:
     int
     exponent() const noexcept;
 
-    bool
-    integral() const noexcept;
-
     bool
     native() const noexcept;
 
@@ -476,12 +437,6 @@ STAmount::exponent() const noexcept
     return mOffset;
 }
 
-inline bool
-STAmount::integral() const noexcept
-{
-    return mAsset.integral();
-}
-
 inline bool
 STAmount::native() const noexcept
 {
@@ -557,8 +512,6 @@ inline STAmount::operator bool() const noexcept
 
 inline STAmount::operator Number() const
 {
-    if (enforceConversion_)
-        return toNumber(*enforceConversion_);
     if (native())
         return xrp();
     if (mAsset.holds<MPTIssue>())
@@ -566,17 +519,6 @@ inline STAmount::operator Number() const
     return iou();
 }
 
-inline Number
-STAmount::toNumber(Number::EnforceInteger enforce) const
-{
-    if (native())
-        return xrp().toNumber(enforce);
-    if (mAsset.holds<MPTIssue>())
-        return mpt().toNumber(enforce);
-    // It doesn't make sense to enforce limits on IOUs
-    return iou();
-}
-
 inline STAmount&
 STAmount::operator=(beast::Zero)
 {
@@ -598,11 +540,6 @@ STAmount::operator=(Number const& number)
     mValue = mIsNegative ? -number.mantissa() : number.mantissa();
     mOffset = number.exponent();
     canonicalize();
-
-    // Convert it back to a Number to check that it's valid
-    [[maybe_unused]]
-    Number n = *this;
-
     return *this;
 }
 
@@ -618,7 +555,7 @@ STAmount::clear()
 {
     // The -100 is used to allow 0 to sort less than a small positive values
     // which have a negative exponent.
-    mOffset = integral() ? 0 : -100;
+    mOffset = native() ? 0 : -100;
     mValue = 0;
     mIsNegative = false;
 }

include/xrpl/protocol/STNumber.h

@@ -56,18 +56,6 @@ public:
     bool
     isDefault() const override;
 
-    /// Sets the flag on the underlying number
-    void
-    setIntegerEnforcement(Number::EnforceInteger enforce);
-
-    /// Gets the flag value on the underlying number
-    Number::EnforceInteger
-    integerEnforcement() const noexcept;
-
-    /// Checks the underlying number
-    bool
-    valid() const noexcept;
-
     operator Number() const
     {
         return value_;

include/xrpl/protocol/SystemParameters.h

@@ -23,7 +23,6 @@ systemName()
 
 /** Number of drops in the genesis account. */
 constexpr XRPAmount INITIAL_XRP{100'000'000'000 * DROPS_PER_XRP};
-static_assert(INITIAL_XRP.drops() == 100'000'000'000'000'000);
 
 /** Returns true if the amount does not exceed the initial XRP in existence. */
 inline bool

include/xrpl/protocol/XRPAmount.h

@@ -146,12 +146,6 @@ public:
         return {drops(), Number::weak};
     }
 
-    Number
-    toNumber(Number::EnforceInteger enforce) const
-    {
-        return {value(), enforce};
-    }
-
     /** Return the sign of the amount */
     constexpr int
     signum() const noexcept

src/libxrpl/ledger/View.cpp

@@ -3454,17 +3454,13 @@ assetsToSharesDeposit(
 
     Number const assetTotal = vault->at(sfAssetsTotal);
     STAmount shares{vault->at(sfShareMPTID)};
-    shares.setIntegerEnforcement(Number::weak);
     if (assetTotal == 0)
         return STAmount{
             shares.asset(),
             Number(assets.mantissa(), assets.exponent() + vault->at(sfScale))
-                .truncate(),
+                .truncate()};
-            Number::weak};
 
-    Number const shareTotal{
+    Number const shareTotal = issuance->at(sfOutstandingAmount);
-        unsafe_cast<std::int64_t>(issuance->at(sfOutstandingAmount)),
-        Number::strong};
     shares = ((shareTotal * assets) / assetTotal).truncate();
     return shares;
 }
@@ -3486,7 +3482,6 @@ sharesToAssetsDeposit(
 
     Number const assetTotal = vault->at(sfAssetsTotal);
     STAmount assets{vault->at(sfAsset)};
-    assets.setIntegerEnforcement(Number::weak);
     if (assetTotal == 0)
         return STAmount{
             assets.asset(),
@@ -3494,9 +3489,7 @@ sharesToAssetsDeposit(
             shares.exponent() - vault->at(sfScale),
             false};
 
-    Number const shareTotal{
+    Number const shareTotal = issuance->at(sfOutstandingAmount);
-        unsafe_cast<std::int64_t>(issuance->at(sfOutstandingAmount)),
-        Number::strong};
     assets = (assetTotal * shares) / shareTotal;
     return assets;
 }
@@ -3520,12 +3513,9 @@ assetsToSharesWithdraw(
     Number assetTotal = vault->at(sfAssetsTotal);
     assetTotal -= vault->at(sfLossUnrealized);
     STAmount shares{vault->at(sfShareMPTID)};
-    shares.setIntegerEnforcement(Number::weak);
     if (assetTotal == 0)
         return shares;
-    Number const shareTotal{
+    Number const shareTotal = issuance->at(sfOutstandingAmount);
-        unsafe_cast<std::int64_t>(issuance->at(sfOutstandingAmount)),
-        Number::strong};
     Number result = (shareTotal * assets) / assetTotal;
     if (truncate == TruncateShares::yes)
         result = result.truncate();
@@ -3551,12 +3541,9 @@ sharesToAssetsWithdraw(
     Number assetTotal = vault->at(sfAssetsTotal);
     assetTotal -= vault->at(sfLossUnrealized);
     STAmount assets{vault->at(sfAsset)};
-    assets.setIntegerEnforcement(Number::weak);
     if (assetTotal == 0)
         return assets;
-    Number const shareTotal{
+    Number const shareTotal = issuance->at(sfOutstandingAmount);
-        unsafe_cast<std::int64_t>(issuance->at(sfOutstandingAmount)),
-        Number::strong};
     assets = (assetTotal * shares) / shareTotal;
     return assets;
 }

src/libxrpl/protocol/STAmount.cpp

@@ -255,25 +255,6 @@ STAmount::move(std::size_t n, void* buf)
     return emplace(n, buf, std::move(*this));
 }
 
-void
-STAmount::setIntegerEnforcement(std::optional<Number::EnforceInteger> enforce)
-{
-    enforceConversion_ = enforce;
-}
-
-std::optional<Number::EnforceInteger>
-STAmount::integerEnforcement() const noexcept
-{
-    return enforceConversion_;
-}
-
-bool
-STAmount::validNumber() const noexcept
-{
-    Number n = toNumber(Number::EnforceInteger::weak);
-    return n.valid();
-}
-
 //------------------------------------------------------------------------------
 //
 // Conversion

src/libxrpl/protocol/STNumber.cpp

@@ -94,24 +94,6 @@ STNumber::isDefault() const
     return value_ == Number();
 }
 
-void
-STNumber::setIntegerEnforcement(Number::EnforceInteger enforce)
-{
-    value_.setIntegerEnforcement(enforce);
-}
-
-Number::EnforceInteger
-STNumber::integerEnforcement() const noexcept
-{
-    return value_.integerEnforcement();
-}
-
-bool
-STNumber::valid() const noexcept
-{
-    return value_.valid();
-}
-
 std::ostream&
 operator<<(std::ostream& out, STNumber const& rhs)
 {

src/test/app/Vault_test.cpp

@@ -3713,32 +3713,7 @@ class Vault_test : public beast::unit_test::suite
         });
 
         testCase(18, [&, this](Env& env, Data d) {
-            testcase("MPT scale deposit overflow");
+            testcase("Scale deposit overflow on second deposit");
-            // The computed number of shares can not be represented as an MPT
-            // without truncation
-
-            {
-                auto tx = d.vault.deposit(
-                    {.depositor = d.depositor,
-                     .id = d.keylet.key,
-                     .amount = d.asset(5)});
-                env(tx, ter{tecPRECISION_LOSS});
-                env.close();
-            }
-        });
-
-        testCase(14, [&, this](Env& env, Data d) {
-            testcase("MPT scale deposit overflow on first deposit");
-            auto tx = d.vault.deposit(
-                {.depositor = d.depositor,
-                 .id = d.keylet.key,
-                 .amount = d.asset(10)});
-            env(tx, ter{tecPRECISION_LOSS});
-            env.close();
-        });
-
-        testCase(14, [&, this](Env& env, Data d) {
-            testcase("MPT scale deposit overflow on second deposit");
 
             {
                 auto tx = d.vault.deposit(
@@ -3759,8 +3734,8 @@ class Vault_test : public beast::unit_test::suite
             }
         });
 
-        testCase(14, [&, this](Env& env, Data d) {
+        testCase(18, [&, this](Env& env, Data d) {
-            testcase("No MPT scale deposit overflow on total shares");
+            testcase("Scale deposit overflow on total shares");
 
             {
                 auto tx = d.vault.deposit(
@@ -3776,7 +3751,7 @@ class Vault_test : public beast::unit_test::suite
                     {.depositor = d.depositor,
                      .id = d.keylet.key,
                      .amount = d.asset(5)});
-                env(tx);
+                env(tx, ter{tecPATH_DRY});
                 env.close();
             }
         });
@@ -4060,28 +4035,6 @@ class Vault_test : public beast::unit_test::suite
         testCase(18, [&, this](Env& env, Data d) {
             testcase("Scale withdraw overflow");
 
-            {
-                auto tx = d.vault.deposit(
-                    {.depositor = d.depositor,
-                     .id = d.keylet.key,
-                     .amount = d.asset(5)});
-                env(tx, ter{tecPRECISION_LOSS});
-                env.close();
-            }
-
-            {
-                auto tx = d.vault.withdraw(
-                    {.depositor = d.depositor,
-                     .id = d.keylet.key,
-                     .amount = STAmount(d.asset, Number(10, 0))});
-                env(tx, ter{tecPRECISION_LOSS});
-                env.close();
-            }
-        });
-
-        testCase(14, [&, this](Env& env, Data d) {
-            testcase("MPT scale withdraw overflow");
-
             {
                 auto tx = d.vault.deposit(
                     {.depositor = d.depositor,
@@ -4300,29 +4253,6 @@ class Vault_test : public beast::unit_test::suite
         testCase(18, [&, this](Env& env, Data d) {
             testcase("Scale clawback overflow");
 
-            {
-                auto tx = d.vault.deposit(
-                    {.depositor = d.depositor,
-                     .id = d.keylet.key,
-                     .amount = d.asset(5)});
-                env(tx, ter(tecPRECISION_LOSS));
-                env.close();
-            }
-
-            {
-                auto tx = d.vault.clawback(
-                    {.issuer = d.issuer,
-                     .id = d.keylet.key,
-                     .holder = d.depositor,
-                     .amount = STAmount(d.asset, Number(10, 0))});
-                env(tx, ter{tecPRECISION_LOSS});
-                env.close();
-            }
-        });
-
-        testCase(14, [&, this](Env& env, Data d) {
-            testcase("MPT Scale clawback overflow");
-
             {
                 auto tx = d.vault.deposit(
                     {.depositor = d.depositor,

src/test/basics/Number_test.cpp

@@ -967,7 +967,7 @@ public:
             {
                 BEAST_EXPECT(e.what() == "Number::operator= integer overflow"s);
                 // The throw is done _after_ the number is updated.
-                BEAST_EXPECT((a == Number::maxIntValue * 2));
+                BEAST_EXPECT((a == Number{2, 14}));
                 BEAST_EXPECT(!a.valid());
             }
             try
@@ -979,7 +979,7 @@ public:
             {
                 BEAST_EXPECT(e.what() == "Number::Number integer overflow"s);
                 // The Number doesn't get updated because the ctor throws
-                BEAST_EXPECT((a == Number::maxIntValue * 2));
+                BEAST_EXPECT((a == Number{2, 14}));
                 BEAST_EXPECT(!a.valid());
             }
             a = Number(1, 10);

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

@@ -71,13 +71,9 @@ VaultClawback::preclaim(PreclaimContext const& ctx)
     }
 
     Asset const vaultAsset = vault->at(sfAsset);
-    if (auto const amount = ctx.tx[~sfAmount])
+    if (auto const amount = ctx.tx[~sfAmount];
-    {
+        amount && vaultAsset != amount->asset())
-        if (vaultAsset != amount->asset())
+        return tecWRONG_ASSET;
-            return tecWRONG_ASSET;
-        else if (!amount->validNumber())
-            return tecPRECISION_LOSS;
-    }
 
     if (vaultAsset.native())
     {
@@ -161,8 +157,6 @@ VaultClawback::doApply()
     MPTIssue const share{mptIssuanceID};
     STAmount sharesDestroyed = {share};
     STAmount assetsRecovered;
-    assetsRecovered.setIntegerEnforcement(Number::weak);
-    sharesDestroyed.setIntegerEnforcement(Number::weak);
     try
     {
         if (amount == beast::zero)
@@ -175,9 +169,6 @@ VaultClawback::doApply()
                 AuthHandling::ahIGNORE_AUTH,
                 j_);
 
-            if (!sharesDestroyed.validNumber())
-                return tecPRECISION_LOSS;
-
             auto const maybeAssets =
                 sharesToAssetsWithdraw(vault, sleIssuance, sharesDestroyed);
             if (!maybeAssets)
@@ -193,8 +184,6 @@ VaultClawback::doApply()
                 if (!maybeShares)
                     return tecINTERNAL;  // LCOV_EXCL_LINE
                 sharesDestroyed = *maybeShares;
-                if (!sharesDestroyed.validNumber())
-                    return tecPRECISION_LOSS;
             }
 
             auto const maybeAssets =
@@ -203,8 +192,6 @@ VaultClawback::doApply()
                 return tecINTERNAL;  // LCOV_EXCL_LINE
             assetsRecovered = *maybeAssets;
         }
-        if (!assetsRecovered.validNumber())
-            return tecPRECISION_LOSS;
 
         // Clamp to maximum.
         if (assetsRecovered > *assetsAvailable)

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

@@ -42,9 +42,6 @@ VaultDeposit::preclaim(PreclaimContext const& ctx)
     if (assets.asset() != vaultAsset)
         return tecWRONG_ASSET;
 
-    if (!assets.validNumber())
-        return tecPRECISION_LOSS;
-
     if (vaultAsset.native())
         ;  // No special checks for XRP
     else if (vaultAsset.holds<MPTIssue>())
@@ -220,7 +217,6 @@ VaultDeposit::doApply()
     }
 
     STAmount sharesCreated = {vault->at(sfShareMPTID)}, assetsDeposited;
-    sharesCreated.setIntegerEnforcement(Number::weak);
     try
     {
         // Compute exchange before transferring any amounts.
@@ -231,14 +227,14 @@ VaultDeposit::doApply()
                 return tecINTERNAL;  // LCOV_EXCL_LINE
             sharesCreated = *maybeShares;
         }
-        if (sharesCreated == beast::zero || !sharesCreated.validNumber())
+        if (sharesCreated == beast::zero)
             return tecPRECISION_LOSS;
 
         auto const maybeAssets =
             sharesToAssetsDeposit(vault, sleIssuance, sharesCreated);
         if (!maybeAssets)
             return tecINTERNAL;  // LCOV_EXCL_LINE
-        else if (*maybeAssets > amount || !maybeAssets->validNumber())
+        else if (*maybeAssets > amount)
         {
             // LCOV_EXCL_START
             JLOG(j_.error()) << "VaultDeposit: would take more than offered.";
@@ -264,23 +260,13 @@ VaultDeposit::doApply()
         sharesCreated.asset() != assetsDeposited.asset(),
         "ripple::VaultDeposit::doApply : assets are not shares");
 
-    auto assetsTotalProxy = vault->at(sfAssetsTotal);
+    vault->at(sfAssetsTotal) += assetsDeposited;
-    auto assetsAvailableProxy = vault->at(sfAssetsAvailable);
+    vault->at(sfAssetsAvailable) += assetsDeposited;
-    if (vault->at(sfAsset).value().integral())
-    {
-        assetsTotalProxy.value().setIntegerEnforcement(Number::weak);
-        assetsAvailableProxy.value().setIntegerEnforcement(Number::weak);
-    }
-    assetsTotalProxy += assetsDeposited;
-    assetsAvailableProxy += assetsDeposited;
-    if (!assetsTotalProxy.value().valid() ||
-        !assetsAvailableProxy.value().valid())
-        return tecLIMIT_EXCEEDED;
     view().update(vault);
 
     // A deposit must not push the vault over its limit.
     auto const maximum = *vault->at(sfAssetsMaximum);
-    if (maximum != 0 && *assetsTotalProxy > maximum)
+    if (maximum != 0 && *vault->at(sfAssetsTotal) > maximum)
         return tecLIMIT_EXCEEDED;
 
     // Transfer assets from depositor to vault.

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

@@ -47,9 +47,6 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
     if (assets.asset() != vaultAsset && assets.asset() != vaultShare)
         return tecWRONG_ASSET;
 
-    if (!assets.validNumber())
-        return tecPRECISION_LOSS;
-
     if (vaultAsset.native())
         ;  // No special checks for XRP
     else if (vaultAsset.holds<MPTIssue>())
@@ -142,8 +139,6 @@ VaultWithdraw::doApply()
     MPTIssue const share{mptIssuanceID};
     STAmount sharesRedeemed = {share};
     STAmount assetsWithdrawn;
-    assetsWithdrawn.setIntegerEnforcement(Number::weak);
-    sharesRedeemed.setIntegerEnforcement(Number::weak);
     try
     {
         if (amount.asset() == vaultAsset)
@@ -157,15 +152,13 @@ VaultWithdraw::doApply()
                 sharesRedeemed = *maybeShares;
             }
 
-            if (sharesRedeemed == beast::zero || !sharesRedeemed.validNumber())
+            if (sharesRedeemed == beast::zero)
                 return tecPRECISION_LOSS;
             auto const maybeAssets =
                 sharesToAssetsWithdraw(vault, sleIssuance, sharesRedeemed);
             if (!maybeAssets)
                 return tecINTERNAL;  // LCOV_EXCL_LINE
             assetsWithdrawn = *maybeAssets;
-            if (!assetsWithdrawn.validNumber())
-                return tecPRECISION_LOSS;
         }
         else if (amount.asset() == share)
         {
@@ -176,8 +169,6 @@ VaultWithdraw::doApply()
             if (!maybeAssets)
                 return tecINTERNAL;  // LCOV_EXCL_LINE
             assetsWithdrawn = *maybeAssets;
-            if (!assetsWithdrawn.validNumber())
-                return tecPRECISION_LOSS;
         }
         else
             return tefINTERNAL;  // LCOV_EXCL_LINE