diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 342184df8d..4a4ee1cacb 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -24,42 +24,17 @@ isPowerOfTen(T value) class Number { -public: - /** Describes whether and how to enforce this number as an integer. - * - * - none: No enforcement. The value may vary freely. This is the default. - * - compatible: If the absolute value is greater than maxIntValue, valid() - * will return false. Needed for backward compatibility with XRP used in - * AMMs, and available for functions that will do their own checking. This - * 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, compatible, weak, strong }; - -private: using rep = std::int64_t; rep mantissa_{0}; int exponent_{std::numeric_limits::lowest()}; - // 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. See the enum description for more detail. - EnforceInteger enforceInteger_ = none; - public: // The range for the mantissa when normalized - constexpr static rep minMantissa = 1'000'000'000'000'000LL; + constexpr static std::int64_t minMantissa = 1'000'000'000'000'000LL; static_assert(isPowerOfTen(minMantissa)); - constexpr static rep maxMantissa = minMantissa * 10 - 1; + constexpr static std::int64_t maxMantissa = minMantissa * 10 - 1; static_assert(maxMantissa == 9'999'999'999'999'999LL); - constexpr static rep maxIntValue = maxMantissa / 100; - static_assert(maxIntValue == 99'999'999'999'999LL); - // The range for the exponent when normalized constexpr static int minExponent = -32768; constexpr static int maxExponent = 32768; @@ -71,42 +46,15 @@ public: explicit constexpr Number() = default; - Number(rep mantissa, EnforceInteger enforce = none); - explicit Number(rep mantissa, int exponent, EnforceInteger enforce = none); + Number(rep mantissa); + explicit Number(rep mantissa, int exponent); explicit constexpr Number(rep mantissa, int exponent, unchecked) noexcept; - constexpr Number(Number const& other) = default; - constexpr Number(Number&& other) = default; - - ~Number() = default; - - constexpr Number& - operator=(Number const& other); - constexpr Number& - operator=(Number&& other); constexpr rep mantissa() const noexcept; constexpr int exponent() const noexcept; - void - setIntegerEnforcement(EnforceInteger enforce); - - EnforceInteger - integerEnforcement() const noexcept; - - 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; constexpr Number @@ -232,9 +180,6 @@ public: private: static thread_local rounding_mode mode_; - void - checkInteger(char const* what) const; - void normalize(); constexpr bool @@ -250,52 +195,16 @@ inline constexpr Number::Number(rep mantissa, int exponent, unchecked) noexcept { } -inline Number::Number(rep mantissa, int exponent, EnforceInteger enforce) - : mantissa_{mantissa}, exponent_{exponent}, enforceInteger_(enforce) +inline Number::Number(rep mantissa, int exponent) + : mantissa_{mantissa}, exponent_{exponent} { normalize(); - - checkInteger("Number::Number integer overflow"); } -inline Number::Number(rep mantissa, EnforceInteger enforce) - : Number{mantissa, 0, enforce} +inline Number::Number(rep mantissa) : Number{mantissa, 0} { } -constexpr Number& -Number::operator=(Number const& other) -{ - if (this != &other) - { - mantissa_ = other.mantissa_; - exponent_ = other.exponent_; - enforceInteger_ = std::max(enforceInteger_, other.enforceInteger_); - - checkInteger("Number::operator= integer overflow"); - } - - return *this; -} - -constexpr Number& -Number::operator=(Number&& other) -{ - if (this != &other) - { - // std::move doesn't really do anything for these types, but - // this is future-proof in case the types ever change - mantissa_ = std::move(other.mantissa_); - exponent_ = std::move(other.exponent_); - if (other.enforceInteger_ > enforceInteger_) - enforceInteger_ = std::move(other.enforceInteger_); - - checkInteger("Number::operator= integer overflow"); - } - - return *this; -} - inline constexpr Number::rep Number::mantissa() const noexcept { @@ -308,20 +217,6 @@ Number::exponent() const noexcept return exponent_; } -inline void -Number::setIntegerEnforcement(EnforceInteger enforce) -{ - enforceInteger_ = enforce; - - checkInteger("Number::setIntegerEnforcement integer overflow"); -} - -inline Number::EnforceInteger -Number::integerEnforcement() const noexcept -{ - return enforceInteger_; -} - inline constexpr Number Number::operator+() const noexcept { diff --git a/include/xrpl/protocol/MPTAmount.h b/include/xrpl/protocol/MPTAmount.h index 5f552d839b..af14786501 100644 --- a/include/xrpl/protocol/MPTAmount.h +++ b/include/xrpl/protocol/MPTAmount.h @@ -62,15 +62,9 @@ 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 value(); } /** Return the sign of the amount */ diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index 11df94518f..f3f285249c 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; //-------------------------------------------------------------------------- // @@ -557,8 +521,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 +528,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 +549,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 +564,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 8f553a6083..e102a3707f 100644 --- a/include/xrpl/protocol/XRPAmount.h +++ b/include/xrpl/protocol/XRPAmount.h @@ -143,13 +143,7 @@ public: operator Number() const noexcept { - return {drops(), Number::compatible}; - } - - Number - toNumber(Number::EnforceInteger enforce) const - { - return {value(), enforce}; + return drops(); } /** Return the sign of the amount */ diff --git a/include/xrpl/protocol/detail/ledger_entries.macro b/include/xrpl/protocol/detail/ledger_entries.macro index 5f6d1b844e..011b3f5ea6 100644 --- a/include/xrpl/protocol/detail/ledger_entries.macro +++ b/include/xrpl/protocol/detail/ledger_entries.macro @@ -480,10 +480,10 @@ LEDGER_ENTRY(ltVAULT, 0x0084, Vault, vault, ({ {sfAccount, soeREQUIRED}, {sfData, soeOPTIONAL}, {sfAsset, soeREQUIRED}, - {sfAssetsTotal, soeDEFAULT}, - {sfAssetsAvailable, soeDEFAULT}, + {sfAssetsTotal, soeREQUIRED}, + {sfAssetsAvailable, soeREQUIRED}, {sfAssetsMaximum, soeDEFAULT}, - {sfLossUnrealized, soeDEFAULT}, + {sfLossUnrealized, soeREQUIRED}, {sfShareMPTID, soeREQUIRED}, {sfWithdrawalPolicy, soeREQUIRED}, {sfScale, soeDEFAULT}, diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index d0837860be..797ca83b67 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -222,15 +222,6 @@ Number::Guard::doRound(rep& drops) constexpr Number one{1000000000000000, -15, Number::unchecked{}}; -void -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 Number::normalize() { @@ -272,55 +263,9 @@ Number::normalize() mantissa_ = -mantissa_; } -bool -Number::valid() const noexcept -{ - 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; - // Avoid making a copy - if (mantissa_ < 0) - return *this >= maxNeg; - return *this <= max; - } - 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) { - // The strictest setting prevails - enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_); - if (y == Number{}) return *this; if (*this == Number{}) @@ -408,9 +353,6 @@ Number::operator+=(Number const& y) } mantissa_ = xm * xn; exponent_ = xe; - - checkInteger("Number::addition integer overflow"); - return *this; } @@ -445,9 +387,6 @@ divu10(uint128_t& u) Number& Number::operator*=(Number const& y) { - // The strictest setting prevails - enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_); - if (*this == Number{}) return *this; if (y == Number{}) @@ -499,18 +438,12 @@ Number::operator*=(Number const& y) XRPL_ASSERT( isnormal() || *this == Number{}, "ripple::Number::operator*=(Number) : result is normal"); - - checkInteger("Number::multiplication integer overflow"); - return *this; } Number& Number::operator/=(Number const& y) { - // The strictest setting prevails - enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_); - if (y == Number{}) throw std::overflow_error("Number: divide by 0"); if (*this == Number{}) @@ -538,9 +471,6 @@ Number::operator/=(Number const& y) exponent_ = ne - de - 17; mantissa_ *= np * dp; normalize(); - - checkInteger("Number::division integer overflow"); - return *this; } diff --git a/src/libxrpl/ledger/View.cpp b/src/libxrpl/ledger/View.cpp index fa9e71f113..9b6a8723c5 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/src/libxrpl/ledger/View.cpp @@ -3454,19 +3454,13 @@ 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{ 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; } @@ -3488,9 +3482,6 @@ 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{ assets.asset(), @@ -3498,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; } @@ -3524,14 +3513,9 @@ 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; - 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(); @@ -3557,14 +3541,9 @@ 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; - 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 26335c12bf..566957509c 100644 --- a/src/libxrpl/protocol/STAmount.cpp +++ b/src/libxrpl/protocol/STAmount.cpp @@ -255,27 +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 -{ - // compatible will not throw. IOUs will ignore the flag, and will - // always be valid. - Number n = toNumber(Number::EnforceInteger::compatible); - 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/AMM_test.cpp b/src/test/app/AMM_test.cpp index 66bceec327..5a1816ebae 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -1384,7 +1384,7 @@ private: // equal asset deposit: unit test to exercise the rounding-down of // LPTokens in the AMMHelpers.cpp: adjustLPTokens calculations // The LPTokens need to have 16 significant digits and a fractional part - for (Number const& deltaLPTokens : + for (Number const deltaLPTokens : {Number{UINT64_C(100000'0000000009), -10}, Number{UINT64_C(100000'0000000001), -10}}) { diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index f823c56a72..dec47f8835 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(13, [&, 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(13, [&, 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(13, [&, 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(13, [&, 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(13, [&, this](Env& env, Data d) { - testcase("MPT Scale clawback overflow"); - { auto tx = d.vault.deposit( {.depositor = d.depositor, @@ -4624,8 +4554,7 @@ class Vault_test : public beast::unit_test::suite BEAST_EXPECT(checkString(vault, sfAssetsAvailable, "50")); BEAST_EXPECT(checkString(vault, sfAssetsMaximum, "1000")); BEAST_EXPECT(checkString(vault, sfAssetsTotal, "50")); - // Since this field is default, it is not returned. - BEAST_EXPECT(!vault.isMember(sfLossUnrealized.getJsonName())); + BEAST_EXPECT(checkString(vault, sfLossUnrealized, "0")); auto const strShareID = strHex(sle->at(sfShareMPTID)); BEAST_EXPECT(checkString(vault, sfShareMPTID, strShareID)); diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index c814f5f7a3..bb5262d028 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -2,7 +2,6 @@ #include #include #include -#include #include #include @@ -835,244 +834,6 @@ public: } } - void - testInteger() - { - testcase("Integer enforcement"); - - using namespace std::string_literals; - - { - 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()); - 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}; - 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.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::compatible}; - BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(!a.representable()); - a = -a; - BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(!a.representable()); - - try - { - a.setIntegerEnforcement(Number::strong); - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT( - e.what() == - "Number::setIntegerEnforcement integer overflow"s); - // The throw is internal to the operator before the result is - // assigned to the Number - BEAST_EXPECT(a == -INITIAL_XRP); - BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(!a.representable()); - } - try - { - ++a; - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT(e.what() == "Number::addition integer overflow"s); - // The throw is internal to the operator before the result is - // assigned to the Number - BEAST_EXPECT(a == -INITIAL_XRP); - BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(!a.representable()); - } - a = Number::maxIntValue; - try - { - ++a; - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT(e.what() == "Number::addition integer overflow"s); - // 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 - { - --a; - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT(e.what() == "Number::addition integer overflow"s); - // 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 - { - a *= Number(1, 10); - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT( - e.what() == "Number::multiplication integer overflow"s); - // The throw is done _after_ the number is updated. - BEAST_EXPECT((a == Number{1, 20})); - BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(!a.representable()); - } - try - { - a = Number::maxIntValue * 2; - 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::maxIntValue * 2)); - BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(a.representable()); - } - try - { - a = Number(3, 15, Number::strong); - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - 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.valid()); - BEAST_EXPECT(a.representable()); - } - a = Number(1, 10); - try - { - a /= Number(1, -10); - BEAST_EXPECT(false); - } - catch (std::overflow_error const& e) - { - BEAST_EXPECT(e.what() == "Number::division integer overflow"s); - // 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()); - } - } - void run() override { @@ -1095,7 +856,6 @@ public: test_toSTAmount(); test_truncate(); testRounding(); - testInteger(); } }; diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index b6ddfde24f..a23b4bcb47 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -2532,13 +2532,6 @@ 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; } @@ -2773,19 +2766,6 @@ 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 { // 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 diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index 931048d1a9..9d6a5e5783 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -567,9 +567,6 @@ Payment::makeRipplePayment(Payment::RipplePaymentParams const& p) beast::Journal j_ = p.j; auto view = [&p]() -> ApplyView& { return p.ctx.view(); }; - bool const depositPreauth = - p.ctx.view().rules().enabled(featureDepositPreauth); - bool const depositAuth = p.ctx.view().rules().enabled(featureDepositAuth); // Below this line, copied straight from Payment::doApply // except `ctx_.tx.getFieldPathSet(sfPaths)` replaced with `paths`, diff --git a/src/xrpld/app/tx/detail/VaultClawback.cpp b/src/xrpld/app/tx/detail/VaultClawback.cpp index b464c1eb28..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()) { @@ -150,11 +146,8 @@ 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), @@ -164,10 +157,6 @@ 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 { if (amount == beast::zero) @@ -180,9 +169,6 @@ VaultClawback::doApply() AuthHandling::ahIGNORE_AUTH, j_); - if (!sharesDestroyed.validNumber()) - return tecPRECISION_LOSS; - auto const maybeAssets = sharesToAssetsWithdraw(vault, sleIssuance, sharesDestroyed); if (!maybeAssets) @@ -198,8 +184,6 @@ VaultClawback::doApply() if (!maybeShares) return tecINTERNAL; // LCOV_EXCL_LINE sharesDestroyed = *maybeShares; - if (!sharesDestroyed.validNumber()) - return tecPRECISION_LOSS; } auto const maybeAssets = @@ -208,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/VaultCreate.cpp b/src/xrpld/app/tx/detail/VaultCreate.cpp index ea5b1d3e76..d84205b13f 100644 --- a/src/xrpld/app/tx/detail/VaultCreate.cpp +++ b/src/xrpld/app/tx/detail/VaultCreate.cpp @@ -213,13 +213,6 @@ 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 diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index 53b7fd67c0..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,9 +217,6 @@ 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 { // Compute exchange before transferring any amounts. @@ -233,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."; @@ -266,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/VaultSet.cpp b/src/xrpld/app/tx/detail/VaultSet.cpp index 439bc598e0..97522dad12 100644 --- a/src/xrpld/app/tx/detail/VaultSet.cpp +++ b/src/xrpld/app/tx/detail/VaultSet.cpp @@ -147,11 +147,6 @@ 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) diff --git a/src/xrpld/app/tx/detail/VaultWithdraw.cpp b/src/xrpld/app/tx/detail/VaultWithdraw.cpp index fc2ee114e3..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 @@ -207,8 +198,6 @@ 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);