diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 7327531f6b..6632a456ab 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -24,31 +24,16 @@ 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 - * default 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; + // isInteger_ is not serialized, transmitted, or used in + // calculations in any way. It is used only for internal validation + // of integer types. It is a one-way switch. Once it's on, it stays + // on. + bool isInteger_ = false; public: // The range for the mantissa when normalized @@ -71,8 +56,8 @@ public: explicit constexpr Number() = default; - Number(rep mantissa, EnforceInteger enforce = none); - explicit Number(rep mantissa, int exponent, EnforceInteger enforce = none); + Number(rep mantissa, bool isInteger = false); + explicit Number(rep mantissa, int exponent, bool isInteger = false); explicit constexpr Number(rep mantissa, int exponent, unchecked) noexcept; constexpr Number(Number const& other) = default; constexpr Number(Number&& other) = default; @@ -90,22 +75,22 @@ public: exponent() const noexcept; void - setIntegerEnforcement(EnforceInteger enforce); + setIsInteger(bool isInteger); - EnforceInteger - integerEnforcement() const noexcept; + bool + isInteger() const noexcept; bool valid() const noexcept; bool representable() const noexcept; - /// Combines setIntegerEnforcement(EnforceInteger) and valid() + /// Combines setIsInteger(bool) and valid() bool - valid(EnforceInteger enforce); + valid(bool isInteger); /// Because this function is const, it should only be used for one-off /// checks bool - valid(EnforceInteger enforce) const; + valid(bool isInteger) const; constexpr Number operator+() const noexcept; @@ -247,9 +232,6 @@ public: private: static thread_local rounding_mode mode_; - void - checkInteger(char const* what) const; - void normalize(); constexpr bool @@ -263,16 +245,14 @@ 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, bool isInteger) + : mantissa_{mantissa}, exponent_{exponent}, isInteger_(isInteger) { normalize(); - - checkInteger("Number::Number integer overflow"); } -inline Number::Number(rep mantissa, EnforceInteger enforce) - : Number{mantissa, 0, enforce} +inline Number::Number(rep mantissa, bool isInteger) + : Number{mantissa, 0, isInteger} { } @@ -283,9 +263,8 @@ Number::operator=(Number const& other) { mantissa_ = other.mantissa_; exponent_ = other.exponent_; - enforceInteger_ = std::max(enforceInteger_, other.enforceInteger_); - - checkInteger("Number::operator= integer overflow"); + if (!isInteger_) + isInteger_ = other.isInteger_; } return *this; @@ -300,10 +279,8 @@ Number::operator=(Number&& other) // 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"); + if (!isInteger_) + isInteger_ = std::move(other.isInteger_); } return *this; @@ -322,17 +299,17 @@ Number::exponent() const noexcept } inline void -Number::setIntegerEnforcement(EnforceInteger enforce) +Number::setIsInteger(bool isInteger) { - enforceInteger_ = enforce; - - checkInteger("Number::setIntegerEnforcement integer overflow"); + if (isInteger_) + return; + isInteger_ = isInteger; } -inline Number::EnforceInteger -Number::integerEnforcement() const noexcept +inline bool +Number::isInteger() const noexcept { - return enforceInteger_; + return isInteger_; } inline constexpr Number diff --git a/include/xrpl/protocol/Asset.h b/include/xrpl/protocol/Asset.h index 6765f7cf7d..0a7f41880a 100644 --- a/include/xrpl/protocol/Asset.h +++ b/include/xrpl/protocol/Asset.h @@ -87,7 +87,14 @@ public: bool integral() const { - return !holds() || get().native(); + return std::visit( + [&](TIss const& issue) { + if constexpr (std::is_same_v) + return issue.native(); + if constexpr (std::is_same_v) + return true; + }, + issue_); } friend constexpr bool diff --git a/include/xrpl/protocol/MPTAmount.h b/include/xrpl/protocol/MPTAmount.h index 5f552d839b..3ca1718cdc 100644 --- a/include/xrpl/protocol/MPTAmount.h +++ b/include/xrpl/protocol/MPTAmount.h @@ -64,13 +64,7 @@ public: operator Number() const { - return {value(), Number::strong}; - } - - Number - toNumber(Number::EnforceInteger enforce) const - { - return {value(), enforce}; + return {value(), true}; } /** Return the sign of the amount */ diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index d0092fdbec..85e1fd4a99 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; @@ -141,28 +135,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 @@ -170,17 +145,11 @@ 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; + bool + representableNumber() const noexcept; //-------------------------------------------------------------------------- // @@ -555,8 +524,6 @@ inline STAmount::operator bool() const noexcept inline STAmount::operator Number() const { - if (enforceConversion_) - return toNumber(*enforceConversion_); if (native()) return xrp(); if (mAsset.holds()) @@ -564,17 +531,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) { @@ -597,10 +553,6 @@ STAmount::operator=(Number const& number) mOffset = number.exponent(); canonicalize(); - // Convert it back to a Number to check that it's valid - [[maybe_unused]] - Number n = *this; - return *this; } diff --git a/include/xrpl/protocol/STNumber.h b/include/xrpl/protocol/STNumber.h index 43b96a2b46..2d9be9d91e 100644 --- a/include/xrpl/protocol/STNumber.h +++ b/include/xrpl/protocol/STNumber.h @@ -58,11 +58,11 @@ public: /// Sets the flag on the underlying number void - setIntegerEnforcement(Number::EnforceInteger enforce); + setIsInteger(bool isInteger); /// Gets the flag value on the underlying number - Number::EnforceInteger - integerEnforcement() const noexcept; + bool + isInteger() const noexcept; /// Checks the underlying number bool diff --git a/include/xrpl/protocol/XRPAmount.h b/include/xrpl/protocol/XRPAmount.h index 8f553a6083..448f87cb24 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(), true}; } /** Return the sign of the amount */ diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 489e27c79f..89d89ef601 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -155,15 +155,6 @@ Number::Guard::round() noexcept 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() { @@ -219,51 +210,48 @@ Number::normalize() bool Number::valid() const noexcept { - return valid(enforceInteger_); + return valid(isInteger_); } bool -Number::valid(EnforceInteger enforce) +Number::valid(bool isInteger) { - setIntegerEnforcement(enforce); + setIsInteger(isInteger); return valid(); } bool -Number::valid(EnforceInteger enforce) const +Number::valid(bool isInteger) 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; + if (!isInteger) + return true; + static Number const max = maxIntValue; + static Number const maxNeg = -max; + // Avoid making a copy + if (mantissa_ < 0) + return *this >= maxNeg; + return *this <= max; } 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; + if (!isInteger_) + return true; + static Number const max = maxMantissa; + static Number const maxNeg = -max; + // Avoid making a copy + if (mantissa_ < 0) + return *this >= maxNeg; + return *this <= max; } Number& Number::operator+=(Number const& y) { // The strictest setting prevails - enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_); + if (!isInteger_) + isInteger_ = y.isInteger_; if (y == Number{}) return *this; @@ -377,9 +365,6 @@ Number::operator+=(Number const& y) } mantissa_ = xm * xn; exponent_ = xe; - - checkInteger("Number::addition integer overflow"); - return *this; } @@ -415,7 +400,8 @@ Number& Number::operator*=(Number const& y) { // The strictest setting prevails - enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_); + if (!isInteger_) + isInteger_ = y.isInteger_; if (*this == Number{}) return *this; @@ -483,9 +469,6 @@ Number::operator*=(Number const& y) XRPL_ASSERT( isnormal() || *this == Number{}, "ripple::Number::operator*=(Number) : result is normal"); - - checkInteger("Number::multiplication integer overflow"); - return *this; } @@ -493,7 +476,8 @@ Number& Number::operator/=(Number const& y) { // The strictest setting prevails - enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_); + if (!isInteger_) + isInteger_ = y.isInteger_; if (y == Number{}) throw std::overflow_error("Number: divide by 0"); @@ -522,9 +506,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 103ff37ef0..069bd3a4d8 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/src/libxrpl/ledger/View.cpp @@ -2878,19 +2878,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; } @@ -2912,9 +2906,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(), @@ -2922,9 +2913,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; } @@ -2948,14 +2937,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(); @@ -2981,14 +2965,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 73b539df69..e3af247794 100644 --- a/src/libxrpl/protocol/STAmount.cpp +++ b/src/libxrpl/protocol/STAmount.cpp @@ -255,26 +255,20 @@ 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. - Number n = toNumber(Number::EnforceInteger::compatible); + Number n = *this; return n.valid(); } +bool +STAmount::representableNumber() const noexcept +{ + Number n = *this; + return n.representable(); +} + //------------------------------------------------------------------------------ // // Conversion diff --git a/src/libxrpl/protocol/STNumber.cpp b/src/libxrpl/protocol/STNumber.cpp index bab99c161d..c9aebaee58 100644 --- a/src/libxrpl/protocol/STNumber.cpp +++ b/src/libxrpl/protocol/STNumber.cpp @@ -95,15 +95,15 @@ STNumber::isDefault() const } void -STNumber::setIntegerEnforcement(Number::EnforceInteger enforce) +STNumber::setIsInteger(bool isInteger) { - value_.setIntegerEnforcement(enforce); + value_.setIsInteger(isInteger); } -Number::EnforceInteger -STNumber::integerEnforcement() const noexcept +bool +STNumber::isInteger() const noexcept { - return value_.integerEnforcement(); + return value_.isInteger(); } bool diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 1efafec65f..bc0bbeab60 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -735,230 +735,71 @@ public: { Number a{100}; - BEAST_EXPECT(a.integerEnforcement() == Number::none); + BEAST_EXPECT(!a.isInteger()); 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.isInteger()); + BEAST_EXPECT(a.valid()); + BEAST_EXPECT(a.representable()); + // If there's any interaction with an integer, the value + // becomes an integer. This is not always what the value is + // being used for, so it's up to the context to check or not + // check whether the number is a _valid_ integer. + a += Number{37, 2, true}; + BEAST_EXPECT(a.isInteger()); BEAST_EXPECT(a.valid()); BEAST_EXPECT(a.representable()); } { - Number a{100, Number::compatible}; - BEAST_EXPECT(a.integerEnforcement() == Number::compatible); + Number a{100, true}; + BEAST_EXPECT(a.isInteger()); 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}; + // The false in the assigned value does not override the + // flag in "a" + a = Number{1, 30, false}; + BEAST_EXPECT(a.isInteger()); BEAST_EXPECT(!a.valid()); BEAST_EXPECT(!a.representable()); a = -100; - BEAST_EXPECT(a.integerEnforcement() == Number::compatible); + BEAST_EXPECT(a.isInteger()); 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}; + a *= Number{1, 16}; + BEAST_EXPECT(a.isInteger()); 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); + // Intermittent value rounding can be lost, but the result + // will be rounded, so that's fine. + a /= Number{1, 5}; + BEAST_EXPECT(a.isInteger()); BEAST_EXPECT(a.valid()); BEAST_EXPECT(a.representable()); - a = Number{5, Number::strong}; - BEAST_EXPECT(a.integerEnforcement() == Number::strong); + a = Number{1, 13} - 3; + BEAST_EXPECT(a.isInteger()); BEAST_EXPECT(a.valid()); BEAST_EXPECT(a.representable()); - } - { - Number a{100, Number::strong}; - BEAST_EXPECT(a.integerEnforcement() == Number::strong); + a += 1; + BEAST_EXPECT(a.isInteger()); 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); + ++a; + BEAST_EXPECT(a.isInteger()); BEAST_EXPECT(a.valid()); BEAST_EXPECT(a.representable()); - } - { - Number a{INITIAL_XRP.drops(), Number::compatible}; + a++; + BEAST_EXPECT(a.isInteger()); 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.representable()); + a = Number{5, true}; + BEAST_EXPECT(a.isInteger()); BEAST_EXPECT(a.valid()); BEAST_EXPECT(a.representable()); } diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index 0dbbd4f24a..862064940a 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -2182,10 +2182,10 @@ ValidVault::Vault::make(SLE const& from) 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); + self.assetsTotal.setIsInteger(true); + self.assetsAvailable.setIsInteger(true); + self.assetsMaximum.setIsInteger(true); + self.lossUnrealized.setIsInteger(true); } return self; } diff --git a/src/xrpld/app/tx/detail/VaultClawback.cpp b/src/xrpld/app/tx/detail/VaultClawback.cpp index b464c1eb28..3dafa76629 100644 --- a/src/xrpld/app/tx/detail/VaultClawback.cpp +++ b/src/xrpld/app/tx/detail/VaultClawback.cpp @@ -163,11 +163,7 @@ VaultClawback::doApply() AccountID holder = tx[sfHolder]; 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); + STAmount assetsRecovered = {vaultAsset}; try { if (amount == beast::zero) @@ -180,9 +176,6 @@ VaultClawback::doApply() AuthHandling::ahIGNORE_AUTH, j_); - if (!sharesDestroyed.validNumber()) - return tecPRECISION_LOSS; - auto const maybeAssets = sharesToAssetsWithdraw(vault, sleIssuance, sharesDestroyed); if (!maybeAssets) @@ -198,8 +191,6 @@ VaultClawback::doApply() if (!maybeShares) return tecINTERNAL; // LCOV_EXCL_LINE sharesDestroyed = *maybeShares; - if (!sharesDestroyed.validNumber()) - return tecPRECISION_LOSS; } auto const maybeAssets = @@ -208,7 +199,8 @@ VaultClawback::doApply() return tecINTERNAL; // LCOV_EXCL_LINE assetsRecovered = *maybeAssets; } - if (!assetsRecovered.validNumber()) + if (!sharesDestroyed.representableNumber() || + !assetsRecovered.representableNumber()) return tecPRECISION_LOSS; // Clamp to maximum. diff --git a/src/xrpld/app/tx/detail/VaultCreate.cpp b/src/xrpld/app/tx/detail/VaultCreate.cpp index 400c36f1b0..c177eb9898 100644 --- a/src/xrpld/app/tx/detail/VaultCreate.cpp +++ b/src/xrpld/app/tx/detail/VaultCreate.cpp @@ -193,7 +193,29 @@ VaultCreate::doApply() vault->at(sfLossUnrealized) = Number(0); // Leave default values for AssetTotal and AssetAvailable, both zero. if (auto value = tx[~sfAssetsMaximum]) - vault->at(sfAssetsMaximum) = *value; + { + auto assetsMaximumProxy = vault->at(sfAssetsMaximum); + assetsMaximumProxy = *value; + if (asset.integral()) + { + // Only the Maximum can be a non-zero value, so only it needs to be + // checked. + assetsMaximumProxy.value().setIsInteger(true); + if (!assetsMaximumProxy.value().representable()) + return tecPRECISION_LOSS; + } + } + // TODO: Should integral types automatically set a limit to the + // Number::maxMantissa value? Or maxIntValue? + /* + else if (asset.integral()) + { + auto assetsMaximumProxy = vault->at(~sfAssetsMaximum); + assetsMaximumProxy = STNumber::maxIntValue + assetsMaximumProxy.value().setIsInteger(true); + } + */ + vault->at(sfShareMPTID) = mptIssuanceID; if (auto value = tx[~sfData]) vault->at(sfData) = *value; @@ -204,13 +226,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..e1d8e34dbd 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -175,6 +175,7 @@ VaultDeposit::doApply() } auto const& vaultAccount = vault->at(sfAccount); + auto const& vaultAsset = vault->at(sfAsset); // Note, vault owner is always authorized if (vault->isFlag(lsfVaultPrivate) && account_ != vault->at(sfOwner)) { @@ -219,10 +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); + STAmount sharesCreated = {vault->at(sfShareMPTID)}; + STAmount assetsDeposited = {vaultAsset}; try { // Compute exchange before transferring any amounts. @@ -233,14 +232,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."; @@ -248,6 +247,10 @@ VaultDeposit::doApply() // LCOV_EXCL_STOP } assetsDeposited = *maybeAssets; + + if (!sharesCreated.representableNumber() || + !assetsDeposited.representableNumber()) + return tecPRECISION_LOSS; } catch (std::overflow_error const&) { @@ -268,10 +271,10 @@ VaultDeposit::doApply() auto assetsTotalProxy = vault->at(sfAssetsTotal); auto assetsAvailableProxy = vault->at(sfAssetsAvailable); - if (vault->at(sfAsset).value().integral()) + if (vaultAsset.value().integral()) { - assetsTotalProxy.value().setIntegerEnforcement(Number::weak); - assetsAvailableProxy.value().setIntegerEnforcement(Number::weak); + assetsTotalProxy.value().setIsInteger(true); + assetsAvailableProxy.value().setIsInteger(true); } assetsTotalProxy += assetsDeposited; assetsAvailableProxy += assetsDeposited; diff --git a/src/xrpld/app/tx/detail/VaultSet.cpp b/src/xrpld/app/tx/detail/VaultSet.cpp index 75f1689d7f..6823d32a15 100644 --- a/src/xrpld/app/tx/detail/VaultSet.cpp +++ b/src/xrpld/app/tx/detail/VaultSet.cpp @@ -138,16 +138,17 @@ VaultSet::doApply() // Update mutable flags and fields if given. if (tx.isFieldPresent(sfData)) vault->at(sfData) = tx[sfData]; - if (tx.isFieldPresent(sfAssetsMaximum)) + if (auto const value = tx[~sfAssetsMaximum]) { - if (tx[sfAssetsMaximum] != 0 && - tx[sfAssetsMaximum] < *vault->at(sfAssetsTotal)) + if (*value != 0 && *value < *vault->at(sfAssetsTotal)) return tecLIMIT_EXCEEDED; - vault->at(sfAssetsMaximum) = tx[sfAssetsMaximum]; + auto assetsMaximumProxy = vault->at(sfAssetsMaximum); + assetsMaximumProxy = *value; if (vault->at(sfAsset).value().integral()) { - if (!vault->at(sfAssetsMaximum).value().valid(Number::compatible)) - return tecLIMIT_EXCEEDED; + assetsMaximumProxy.value().setIsInteger(true); + if (!assetsMaximumProxy.value().representable()) + return tecPRECISION_LOSS; } } diff --git a/src/xrpld/app/tx/detail/VaultWithdraw.cpp b/src/xrpld/app/tx/detail/VaultWithdraw.cpp index e28c54676c..2423d7128a 100644 --- a/src/xrpld/app/tx/detail/VaultWithdraw.cpp +++ b/src/xrpld/app/tx/detail/VaultWithdraw.cpp @@ -156,9 +156,7 @@ VaultWithdraw::doApply() Asset const vaultAsset = vault->at(sfAsset); MPTIssue const share{mptIssuanceID}; STAmount sharesRedeemed = {share}; - STAmount assetsWithdrawn; - assetsWithdrawn.setIntegerEnforcement(Number::weak); - sharesRedeemed.setIntegerEnforcement(Number::weak); + STAmount assetsWithdrawn = {vaultAsset}; try { if (amount.asset() == vaultAsset) @@ -172,15 +170,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) { @@ -191,11 +187,12 @@ VaultWithdraw::doApply() if (!maybeAssets) return tecINTERNAL; // LCOV_EXCL_LINE assetsWithdrawn = *maybeAssets; - if (!assetsWithdrawn.validNumber()) - return tecPRECISION_LOSS; } else return tefINTERNAL; // LCOV_EXCL_LINE + if (!sharesRedeemed.representableNumber() || + !assetsWithdrawn.representableNumber()) + return tecPRECISION_LOSS; } catch (std::overflow_error const&) {