diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index fd04c8b788..1ea43a1e93 100644 --- a/include/xrpl/basics/Number.h +++ b/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; - static_assert(maxIntValue == 999'999'999'999'999LL); + constexpr static rep maxIntValue = minMantissa / 10; // The range for the exponent when normalized constexpr static int minExponent = -32768; diff --git a/include/xrpl/protocol/Asset.h b/include/xrpl/protocol/Asset.h index 6765f7cf7d..d0efe814a9 100644 --- a/include/xrpl/protocol/Asset.h +++ b/include/xrpl/protocol/Asset.h @@ -84,12 +84,6 @@ public: return holds() && get().native(); } - bool - integral() const - { - return !holds() || get().native(); - } - friend constexpr bool operator==(Asset const& lhs, Asset const& rhs); diff --git a/include/xrpl/protocol/MPTAmount.h b/include/xrpl/protocol/MPTAmount.h index 5f552d839b..1c7c7a25e7 100644 --- a/include/xrpl/protocol/MPTAmount.h +++ b/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; diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index 11df94518f..d12e218250 100644 --- a/include/xrpl/protocol/STAmount.h +++ b/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 enforceConversion_; - public: using value_type = STAmount; @@ -143,28 +137,9 @@ public: STAmount(A const& asset, int mantissa, int exponent = 0); template - STAmount( - A const& asset, - Number const& number, - std::optional enforce = std::nullopt) + STAmount(A const& asset, Number const& number) : 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( - "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 enforce); - - std::optional - 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()) @@ -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()) - 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; } diff --git a/include/xrpl/protocol/STNumber.h b/include/xrpl/protocol/STNumber.h index 43b96a2b46..2ec3d66fd1 100644 --- a/include/xrpl/protocol/STNumber.h +++ b/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_; diff --git a/include/xrpl/protocol/SystemParameters.h b/include/xrpl/protocol/SystemParameters.h index 7a2c5a7f63..de78b65265 100644 --- a/include/xrpl/protocol/SystemParameters.h +++ b/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 diff --git a/include/xrpl/protocol/XRPAmount.h b/include/xrpl/protocol/XRPAmount.h index 159174accc..f26bb7366f 100644 --- a/include/xrpl/protocol/XRPAmount.h +++ b/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 diff --git a/src/libxrpl/ledger/View.cpp b/src/libxrpl/ledger/View.cpp index 7c158d4448..9b6a8723c5 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/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(), - Number::weak}; + .truncate()}; - Number const shareTotal{ - unsafe_cast(issuance->at(sfOutstandingAmount)), - Number::strong}; + Number const shareTotal = issuance->at(sfOutstandingAmount); 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{ - unsafe_cast(issuance->at(sfOutstandingAmount)), - Number::strong}; + Number const shareTotal = issuance->at(sfOutstandingAmount); 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{ - unsafe_cast(issuance->at(sfOutstandingAmount)), - Number::strong}; + Number const shareTotal = issuance->at(sfOutstandingAmount); 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{ - unsafe_cast(issuance->at(sfOutstandingAmount)), - Number::strong}; + Number const shareTotal = issuance->at(sfOutstandingAmount); assets = (assetTotal * shares) / shareTotal; return assets; } diff --git a/src/libxrpl/protocol/STAmount.cpp b/src/libxrpl/protocol/STAmount.cpp index 545a09a625..566957509c 100644 --- a/src/libxrpl/protocol/STAmount.cpp +++ b/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 enforce) -{ - enforceConversion_ = enforce; -} - -std::optional -STAmount::integerEnforcement() const noexcept -{ - return enforceConversion_; -} - -bool -STAmount::validNumber() const noexcept -{ - Number n = toNumber(Number::EnforceInteger::weak); - return n.valid(); -} - //------------------------------------------------------------------------------ // // Conversion diff --git a/src/libxrpl/protocol/STNumber.cpp b/src/libxrpl/protocol/STNumber.cpp index bab99c161d..c353f6b795 100644 --- a/src/libxrpl/protocol/STNumber.cpp +++ b/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) { diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 893a50f605..fbc4a5ff9b 100644 --- a/src/test/app/Vault_test.cpp +++ b/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"); - // 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"); + testcase("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("No MPT scale deposit overflow on total shares"); + testCase(18, [&, this](Env& env, Data d) { + 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, diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index a505ff522d..968e621657 100644 --- a/src/test/basics/Number_test.cpp +++ b/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); diff --git a/src/xrpld/app/tx/detail/VaultClawback.cpp b/src/xrpld/app/tx/detail/VaultClawback.cpp index 1c73d36bb0..7c56ca1d60 100644 --- a/src/xrpld/app/tx/detail/VaultClawback.cpp +++ b/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 (vaultAsset != amount->asset()) - return tecWRONG_ASSET; - else if (!amount->validNumber()) - return tecPRECISION_LOSS; - } + if (auto const amount = ctx.tx[~sfAmount]; + amount && vaultAsset != amount->asset()) + return tecWRONG_ASSET; 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) diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index 8e42fd188d..3e5ae741e3 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/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()) @@ -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); - auto assetsAvailableProxy = vault->at(sfAssetsAvailable); - 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; + vault->at(sfAssetsTotal) += assetsDeposited; + vault->at(sfAssetsAvailable) += assetsDeposited; 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. diff --git a/src/xrpld/app/tx/detail/VaultWithdraw.cpp b/src/xrpld/app/tx/detail/VaultWithdraw.cpp index 04857d400e..dc8cbde7c0 100644 --- a/src/xrpld/app/tx/detail/VaultWithdraw.cpp +++ b/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()) @@ -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