From b7ed35cb0420d95fe7609ceabe6a79e2fbb90044 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 12 Nov 2025 16:34:41 -0500 Subject: [PATCH 1/8] Fix merge issue: references to retired features --- src/xrpld/app/tx/detail/Payment.cpp | 3 --- 1 file changed, 3 deletions(-) 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`, From 2e34506835a3487313194ba01b3f43b2c00b3548 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 12 Nov 2025 19:14:54 -0500 Subject: [PATCH 2/8] Revert "Add a distinction between a "valid" and a "representable" Number" This reverts commit 8e56af20ee9e9e35b50c66d9ab10e63ef841955d. --- include/xrpl/basics/Number.h | 31 +++------ include/xrpl/protocol/XRPAmount.h | 2 +- src/libxrpl/basics/Number.cpp | 32 +-------- src/libxrpl/ledger/View.cpp | 8 --- src/libxrpl/protocol/STAmount.cpp | 4 +- src/test/app/Vault_test.cpp | 10 +-- src/test/basics/Number_test.cpp | 76 +--------------------- src/xrpld/app/tx/detail/InvariantCheck.cpp | 20 ------ src/xrpld/app/tx/detail/VaultClawback.cpp | 5 -- src/xrpld/app/tx/detail/VaultCreate.cpp | 7 -- src/xrpld/app/tx/detail/VaultDeposit.cpp | 2 - src/xrpld/app/tx/detail/VaultSet.cpp | 5 -- src/xrpld/app/tx/detail/VaultWithdraw.cpp | 2 - 13 files changed, 19 insertions(+), 185 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 342184df8d..fd04c8b788 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -28,17 +28,12 @@ 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. + * - weak: If the absolute value is greater than maxIntValue, valid() will + * return false. + * - strong: Assignment operations will throw if the absolute value is above + * maxIntValue. */ - enum EnforceInteger { none, compatible, weak, strong }; + enum EnforceInteger { none, weak, strong }; private: using rep = std::int64_t; @@ -47,7 +42,8 @@ private: // The enforcement setting is not serialized, and does not affect the // ledger. If not "none", the value is checked to be within the valid - // integer range. See the enum description for more detail. + // integer range. With "strong", the checks will be made as automatic as + // possible. EnforceInteger enforceInteger_ = none; public: @@ -57,8 +53,8 @@ public: constexpr static rep 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); + constexpr static rep maxIntValue = maxMantissa / 10; + static_assert(maxIntValue == 999'999'999'999'999LL); // The range for the exponent when normalized constexpr static int minExponent = -32768; @@ -97,15 +93,6 @@ public: bool valid() const noexcept; - bool - representable() const noexcept; - /// Combines setIntegerEnforcement(EnforceInteger) and valid() - bool - valid(EnforceInteger enforce); - /// Because this function is const, it should only be used for one-off - /// checks - bool - valid(EnforceInteger enforce) const; constexpr Number operator+() const noexcept; diff --git a/include/xrpl/protocol/XRPAmount.h b/include/xrpl/protocol/XRPAmount.h index 8f553a6083..159174accc 100644 --- a/include/xrpl/protocol/XRPAmount.h +++ b/include/xrpl/protocol/XRPAmount.h @@ -143,7 +143,7 @@ public: operator Number() const noexcept { - return {drops(), Number::compatible}; + return {drops(), Number::weak}; } Number diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index d0837860be..2587af2e70 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -227,8 +227,6 @@ 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 @@ -275,20 +273,7 @@ Number::normalize() 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) + if (enforceInteger_ != none) { static Number const max = maxIntValue; static Number const maxNeg = -maxIntValue; @@ -300,21 +285,6 @@ Number::valid(EnforceInteger enforce) const 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) { diff --git a/src/libxrpl/ledger/View.cpp b/src/libxrpl/ledger/View.cpp index fa9e71f113..7c158d4448 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/src/libxrpl/ledger/View.cpp @@ -3454,8 +3454,6 @@ 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{ @@ -3488,8 +3486,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{ @@ -3524,8 +3520,6 @@ 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; @@ -3557,8 +3551,6 @@ 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; diff --git a/src/libxrpl/protocol/STAmount.cpp b/src/libxrpl/protocol/STAmount.cpp index 26335c12bf..545a09a625 100644 --- a/src/libxrpl/protocol/STAmount.cpp +++ b/src/libxrpl/protocol/STAmount.cpp @@ -270,9 +270,7 @@ STAmount::integerEnforcement() const noexcept 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); + Number n = toNumber(Number::EnforceInteger::weak); return n.valid(); } diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index f823c56a72..893a50f605 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -3727,7 +3727,7 @@ class Vault_test : public beast::unit_test::suite } }); - testCase(13, [&, this](Env& env, Data d) { + testCase(14, [&, this](Env& env, Data d) { testcase("MPT scale deposit overflow on first deposit"); auto tx = d.vault.deposit( {.depositor = d.depositor, @@ -3737,7 +3737,7 @@ class Vault_test : public beast::unit_test::suite env.close(); }); - testCase(13, [&, this](Env& env, Data d) { + testCase(14, [&, this](Env& env, Data d) { testcase("MPT scale deposit overflow on second deposit"); { @@ -3759,7 +3759,7 @@ class Vault_test : public beast::unit_test::suite } }); - testCase(13, [&, this](Env& env, Data d) { + testCase(14, [&, this](Env& env, Data d) { testcase("No MPT scale deposit overflow on total shares"); { @@ -4079,7 +4079,7 @@ class Vault_test : public beast::unit_test::suite } }); - testCase(13, [&, this](Env& env, Data d) { + testCase(14, [&, this](Env& env, Data d) { testcase("MPT scale withdraw overflow"); { @@ -4320,7 +4320,7 @@ class Vault_test : public beast::unit_test::suite } }); - testCase(13, [&, this](Env& env, Data d) { + testCase(14, [&, this](Env& env, Data d) { testcase("MPT Scale clawback overflow"); { diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index c814f5f7a3..a505ff522d 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -846,87 +846,28 @@ public: Number a{100}; BEAST_EXPECT(a.integerEnforcement() == Number::none); BEAST_EXPECT(a.valid()); - BEAST_EXPECT(a.representable()); a = Number{1, 30}; BEAST_EXPECT(a.valid()); - BEAST_EXPECT(a.representable()); a = -100; BEAST_EXPECT(a.valid()); - BEAST_EXPECT(a.representable()); - } - { - Number a{100, Number::compatible}; - BEAST_EXPECT(a.integerEnforcement() == Number::compatible); - BEAST_EXPECT(a.valid()); - BEAST_EXPECT(a.representable()); - a = Number{1, 15}; - BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(a.representable()); - a = Number{1, 30, Number::none}; - BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(!a.representable()); - a = -100; - BEAST_EXPECT(a.integerEnforcement() == Number::compatible); - BEAST_EXPECT(a.valid()); - BEAST_EXPECT(a.representable()); - a = Number{5, Number::weak}; - BEAST_EXPECT(a.integerEnforcement() == Number::weak); - BEAST_EXPECT(a.valid()); - BEAST_EXPECT(a.representable()); - a = Number{5, Number::strong}; - BEAST_EXPECT(a.integerEnforcement() == Number::strong); - BEAST_EXPECT(a.valid()); - BEAST_EXPECT(a.representable()); } { Number a{100, Number::weak}; BEAST_EXPECT(a.integerEnforcement() == Number::weak); BEAST_EXPECT(a.valid()); - BEAST_EXPECT(a.representable()); - a = Number{1, 15}; + a = Number{1, 30, Number::none}; 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}; @@ -939,19 +880,15 @@ public: BEAST_EXPECT((a == Number{1, 30})); } BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(!a.representable()); a = -100; BEAST_EXPECT(a.integerEnforcement() == Number::strong); BEAST_EXPECT(a.valid()); - BEAST_EXPECT(a.representable()); } { - Number a{INITIAL_XRP.drops(), Number::compatible}; + Number a{INITIAL_XRP.drops(), Number::weak}; BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(!a.representable()); a = -a; BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(!a.representable()); try { @@ -967,7 +904,6 @@ public: // assigned to the Number BEAST_EXPECT(a == -INITIAL_XRP); BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(!a.representable()); } try { @@ -981,7 +917,6 @@ public: // assigned to the Number BEAST_EXPECT(a == -INITIAL_XRP); BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(!a.representable()); } a = Number::maxIntValue; try @@ -995,7 +930,6 @@ public: // This time, the throw is done _after_ the number is updated. BEAST_EXPECT(a == Number::maxIntValue + 1); BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(a.representable()); } a = -Number::maxIntValue; try @@ -1009,7 +943,6 @@ public: // This time, the throw is done _after_ the number is updated. BEAST_EXPECT(a == -Number::maxIntValue - 1); BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(a.representable()); } a = Number(1, 10); try @@ -1024,7 +957,6 @@ public: // The throw is done _after_ the number is updated. BEAST_EXPECT((a == Number{1, 20})); BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(!a.representable()); } try { @@ -1037,7 +969,6 @@ public: // The throw is done _after_ the number is updated. BEAST_EXPECT((a == Number::maxIntValue * 2)); BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(a.representable()); } try { @@ -1050,7 +981,6 @@ public: // The Number doesn't get updated because the ctor throws BEAST_EXPECT((a == Number::maxIntValue * 2)); BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(a.representable()); } a = Number(1, 10); try @@ -1064,12 +994,10 @@ public: // The throw is done _after_ the number is updated. BEAST_EXPECT((a == Number{1, 20})); BEAST_EXPECT(!a.valid()); - BEAST_EXPECT(!a.representable()); } a /= Number(1, 15); BEAST_EXPECT((a == Number{1, 5})); BEAST_EXPECT(a.valid()); - BEAST_EXPECT(a.representable()); } } 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/VaultClawback.cpp b/src/xrpld/app/tx/detail/VaultClawback.cpp index b464c1eb28..1c73d36bb0 100644 --- a/src/xrpld/app/tx/detail/VaultClawback.cpp +++ b/src/xrpld/app/tx/detail/VaultClawback.cpp @@ -150,11 +150,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,8 +161,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 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..8e42fd188d 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -220,8 +220,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 { 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..04857d400e 100644 --- a/src/xrpld/app/tx/detail/VaultWithdraw.cpp +++ b/src/xrpld/app/tx/detail/VaultWithdraw.cpp @@ -207,8 +207,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); From 694abd1c7917a89b826f196d8560bf3c0002dce0 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 12 Nov 2025 19:15:00 -0500 Subject: [PATCH 3/8] Revert "Catch up the consequences of Number changes" This reverts commit 0175dd70dbc6a962e93c1fd4ec4cbadc1bcfd903. --- include/xrpl/basics/Number.h | 3 +- include/xrpl/protocol/Asset.h | 6 -- include/xrpl/protocol/MPTAmount.h | 8 +-- include/xrpl/protocol/STAmount.h | 67 +------------------ include/xrpl/protocol/STNumber.h | 12 ---- include/xrpl/protocol/SystemParameters.h | 1 - include/xrpl/protocol/XRPAmount.h | 6 -- src/libxrpl/ledger/View.cpp | 23 ++----- src/libxrpl/protocol/STAmount.cpp | 19 ------ src/libxrpl/protocol/STNumber.cpp | 18 ------ src/test/app/Vault_test.cpp | 78 ++--------------------- src/test/basics/Number_test.cpp | 4 +- src/xrpld/app/tx/detail/VaultClawback.cpp | 19 +----- src/xrpld/app/tx/detail/VaultDeposit.cpp | 24 ++----- src/xrpld/app/tx/detail/VaultWithdraw.cpp | 11 +--- 15 files changed, 24 insertions(+), 275 deletions(-) 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 From 814577758fc2f62ac5093b331836bf1de7855f50 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 12 Nov 2025 19:15:03 -0500 Subject: [PATCH 4/8] Revert "Fix build error - avoid copy" This reverts commit cb6df196dc00dad4ce4e48d6623b88b765e9a27c. --- src/test/app/AMM_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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}}) { From 1b4e18a1a7c9e58dfe87c2d7d5d3895f37a17511 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 12 Nov 2025 19:15:06 -0500 Subject: [PATCH 5/8] Revert "Add integer enforcement when converting to XRP/MPTAmount to Number" This reverts commit b605a2cdcc2ac8ca27e3b44036b2ea375069ac89. --- include/xrpl/protocol/MPTAmount.h | 2 +- include/xrpl/protocol/XRPAmount.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/xrpl/protocol/MPTAmount.h b/include/xrpl/protocol/MPTAmount.h index 1c7c7a25e7..af14786501 100644 --- a/include/xrpl/protocol/MPTAmount.h +++ b/include/xrpl/protocol/MPTAmount.h @@ -64,7 +64,7 @@ public: operator Number() const noexcept { - return {value(), Number::strong}; + return value(); } /** Return the sign of the amount */ diff --git a/include/xrpl/protocol/XRPAmount.h b/include/xrpl/protocol/XRPAmount.h index f26bb7366f..e102a3707f 100644 --- a/include/xrpl/protocol/XRPAmount.h +++ b/include/xrpl/protocol/XRPAmount.h @@ -143,7 +143,7 @@ public: operator Number() const noexcept { - return {drops(), Number::weak}; + return drops(); } /** Return the sign of the amount */ From 9b0b7b5a91276903842c42d30c12abebd5bd87e5 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 12 Nov 2025 19:15:07 -0500 Subject: [PATCH 6/8] Revert "Make all STNumber fields "soeDEFAULT"" This reverts commit 24f37d73f614dafbd522068781532e06584ce68e. --- include/xrpl/protocol/detail/ledger_entries.macro | 6 +++--- src/test/app/Vault_test.cpp | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) 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/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index fbc4a5ff9b..dec47f8835 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -4554,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)); From 398170ef3d97787a9ee8e67afdfee522548c404b Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 12 Nov 2025 19:15:14 -0500 Subject: [PATCH 7/8] Revert "Add optional enforcement of valid integer range to Number" This reverts commit 3cb447a4fe7866a533ed9a577ca392d3898cebe5. --- include/xrpl/basics/Number.h | 116 ++-------------------- src/libxrpl/basics/Number.cpp | 40 -------- src/test/basics/Number_test.cpp | 169 -------------------------------- 3 files changed, 7 insertions(+), 318 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 1ea43a1e93..080a807909 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -13,47 +13,16 @@ class Number; std::string to_string(Number const& amount); -template -constexpr bool -isPowerOfTen(T value) -{ - while (value >= 10 && value % 10 == 0) - value /= 10; - return value == 1; -} - 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. - * - weak: If the absolute value is greater than maxIntValue, valid() will - * return false. - * - strong: Assignment operations will throw if the absolute value is above - * maxIntValue. - */ - enum EnforceInteger { none, 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. With "strong", the checks will be made as automatic as - // possible. - EnforceInteger enforceInteger_ = none; - public: // The range for the mantissa when normalized - constexpr static rep minMantissa = 1'000'000'000'000'000LL; - static_assert(isPowerOfTen(minMantissa)); - constexpr static rep maxMantissa = minMantissa * 10 - 1; - static_assert(maxMantissa == 9'999'999'999'999'999LL); - - constexpr static rep maxIntValue = minMantissa / 10; + constexpr static std::int64_t minMantissa = 1'000'000'000'000'000LL; + constexpr static std::int64_t maxMantissa = 9'999'999'999'999'999LL; // The range for the exponent when normalized constexpr static int minExponent = -32768; @@ -66,33 +35,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; - constexpr Number operator+() const noexcept; constexpr Number @@ -218,9 +169,6 @@ public: private: static thread_local rounding_mode mode_; - void - checkInteger(char const* what) const; - void normalize(); constexpr bool @@ -236,52 +184,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 { @@ -294,20 +206,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/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 2587af2e70..797ca83b67 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -222,13 +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); -} - void Number::normalize() { @@ -270,27 +263,9 @@ Number::normalize() mantissa_ = -mantissa_; } -bool -Number::valid() const noexcept -{ - if (enforceInteger_ != 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; -} - Number& Number::operator+=(Number const& y) { - // The strictest setting prevails - enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_); - if (y == Number{}) return *this; if (*this == Number{}) @@ -378,9 +353,6 @@ Number::operator+=(Number const& y) } mantissa_ = xm * xn; exponent_ = xe; - - checkInteger("Number::addition integer overflow"); - return *this; } @@ -415,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{}) @@ -469,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{}) @@ -508,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/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 968e621657..cec69fe05f 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,173 +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()); - a = Number{1, 30}; - BEAST_EXPECT(a.valid()); - a = -100; - BEAST_EXPECT(a.valid()); - } - { - Number a{100, Number::weak}; - BEAST_EXPECT(a.integerEnforcement() == Number::weak); - BEAST_EXPECT(a.valid()); - a = Number{1, 30, Number::none}; - BEAST_EXPECT(!a.valid()); - a = -100; - BEAST_EXPECT(a.integerEnforcement() == Number::weak); - BEAST_EXPECT(a.valid()); - a = Number{5, Number::strong}; - BEAST_EXPECT(a.integerEnforcement() == Number::strong); - BEAST_EXPECT(a.valid()); - } - { - Number a{100, Number::strong}; - BEAST_EXPECT(a.integerEnforcement() == Number::strong); - BEAST_EXPECT(a.valid()); - 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()); - a = -100; - BEAST_EXPECT(a.integerEnforcement() == Number::strong); - BEAST_EXPECT(a.valid()); - } - { - Number a{INITIAL_XRP.drops(), Number::weak}; - BEAST_EXPECT(!a.valid()); - a = -a; - BEAST_EXPECT(!a.valid()); - - 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()); - } - 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()); - } - 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()); - } - 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()); - } - 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()); - } - 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{2, 14})); - BEAST_EXPECT(!a.valid()); - } - 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{2, 14})); - BEAST_EXPECT(!a.valid()); - } - 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()); - } - a /= Number(1, 15); - BEAST_EXPECT((a == Number{1, 5})); - BEAST_EXPECT(a.valid()); - } - } - - void run() override { testZero(); @@ -1023,7 +855,6 @@ public: test_toSTAmount(); test_truncate(); testRounding(); - testInteger(); } }; From a4aa72eada60c4d0aaf90ead3d2d62b3c0c9136e Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 12 Nov 2025 19:19:06 -0500 Subject: [PATCH 8/8] Fix revert issues --- include/xrpl/basics/Number.h | 13 ++++++++++++- include/xrpl/protocol/Asset.h | 6 ++++++ include/xrpl/protocol/STAmount.h | 9 +++++++++ src/test/basics/Number_test.cpp | 1 + 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 080a807909..4a4ee1cacb 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -13,6 +13,15 @@ class Number; std::string to_string(Number const& amount); +template +constexpr bool +isPowerOfTen(T value) +{ + while (value >= 10 && value % 10 == 0) + value /= 10; + return value == 1; +} + class Number { using rep = std::int64_t; @@ -22,7 +31,9 @@ class Number public: // The range for the mantissa when normalized constexpr static std::int64_t minMantissa = 1'000'000'000'000'000LL; - constexpr static std::int64_t maxMantissa = 9'999'999'999'999'999LL; + static_assert(isPowerOfTen(minMantissa)); + constexpr static std::int64_t maxMantissa = minMantissa * 10 - 1; + static_assert(maxMantissa == 9'999'999'999'999'999LL); // 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 d0efe814a9..6765f7cf7d 100644 --- a/include/xrpl/protocol/Asset.h +++ b/include/xrpl/protocol/Asset.h @@ -84,6 +84,12 @@ 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/STAmount.h b/include/xrpl/protocol/STAmount.h index d12e218250..f3f285249c 100644 --- a/include/xrpl/protocol/STAmount.h +++ b/include/xrpl/protocol/STAmount.h @@ -157,6 +157,9 @@ public: int exponent() const noexcept; + bool + integral() const noexcept; + bool native() const noexcept; @@ -437,6 +440,12 @@ STAmount::exponent() const noexcept return mOffset; } +inline bool +STAmount::integral() const noexcept +{ + return mAsset.integral(); +} + inline bool STAmount::native() const noexcept { diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index cec69fe05f..bb5262d028 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -834,6 +834,7 @@ public: } } + void run() override { testZero();