From d258fdc7b03a3bc7ff7b97e1ae17efa4b31847a5 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Tue, 5 May 2026 15:30:53 +0100 Subject: [PATCH] refactor: Retire fixUniversalNumber amendment Remove all conditional code paths guarded by fixUniversalNumber, keeping only the "enabled" path since the amendment is now permanently active. Co-Authored-By: Claude Opus 4.6 (1M context) --- include/xrpl/protocol/AMMCore.h | 2 +- include/xrpl/protocol/IOUAmount.h | 33 ------ src/libxrpl/protocol/AMMCore.cpp | 2 +- src/libxrpl/protocol/IOUAmount.cpp | 102 ++----------------- src/libxrpl/protocol/STAmount.cpp | 158 ++--------------------------- src/libxrpl/tx/Transactor.cpp | 3 - src/libxrpl/tx/applySteps.cpp | 4 - src/test/app/AMM_test.cpp | 5 +- src/test/app/NFToken_test.cpp | 14 +-- src/test/app/OfferMPT_test.cpp | 1 - src/test/app/Offer_test.cpp | 18 +--- src/xrpld/app/misc/detail/TxQ.cpp | 5 - 12 files changed, 22 insertions(+), 325 deletions(-) diff --git a/include/xrpl/protocol/AMMCore.h b/include/xrpl/protocol/AMMCore.h index 9d8f8c62b0..eb6bae9b37 100644 --- a/include/xrpl/protocol/AMMCore.h +++ b/include/xrpl/protocol/AMMCore.h @@ -65,7 +65,7 @@ invalidAMMAssetPair( std::optional ammAuctionTimeSlot(std::uint64_t current, STObject const& auctionSlot); -/** Return true if required AMM amendments are enabled +/** Return true if required AMM amendment is enabled */ bool ammEnabled(Rules const&); diff --git a/include/xrpl/protocol/IOUAmount.h b/include/xrpl/protocol/IOUAmount.h index 6ce773fabd..9a5800d675 100644 --- a/include/xrpl/protocol/IOUAmount.h +++ b/include/xrpl/protocol/IOUAmount.h @@ -1,6 +1,5 @@ #pragma once -#include #include #include @@ -179,36 +178,4 @@ to_string(IOUAmount const& amount); IOUAmount mulRatio(IOUAmount const& amt, std::uint32_t num, std::uint32_t den, bool roundUp); -// Since many uses of the number class do not have access to a ledger, -// getSTNumberSwitchover needs to be globally accessible. - -bool -getSTNumberSwitchover(); - -void -setSTNumberSwitchover(bool v); - -/** RAII class to set and restore the Number switchover. - */ - -class NumberSO -{ - bool saved_; - -public: - ~NumberSO() - { - setSTNumberSwitchover(saved_); - } - - NumberSO(NumberSO const&) = delete; - NumberSO& - operator=(NumberSO const&) = delete; - - explicit NumberSO(bool v) : saved_(getSTNumberSwitchover()) - { - setSTNumberSwitchover(v); - } -}; - } // namespace xrpl diff --git a/src/libxrpl/protocol/AMMCore.cpp b/src/libxrpl/protocol/AMMCore.cpp index b4c44f0f36..71d4b83857 100644 --- a/src/libxrpl/protocol/AMMCore.cpp +++ b/src/libxrpl/protocol/AMMCore.cpp @@ -128,7 +128,7 @@ ammAuctionTimeSlot(std::uint64_t current, STObject const& auctionSlot) bool ammEnabled(Rules const& rules) { - return rules.enabled(featureAMM) && rules.enabled(fixUniversalNumber); + return rules.enabled(featureAMM); } } // namespace xrpl diff --git a/src/libxrpl/protocol/IOUAmount.cpp b/src/libxrpl/protocol/IOUAmount.cpp index e4326d611e..2969b91ebc 100644 --- a/src/libxrpl/protocol/IOUAmount.cpp +++ b/src/libxrpl/protocol/IOUAmount.cpp @@ -1,6 +1,5 @@ #include -#include #include #include #include @@ -17,29 +16,6 @@ namespace xrpl { -namespace { - -// Use a static inside a function to help prevent order-of-initialization issues -LocalValue& -getStaticSTNumberSwitchover() -{ - static LocalValue kR{true}; - return kR; -} -} // namespace - -bool -getSTNumberSwitchover() -{ - return *getStaticSTNumberSwitchover(); -} - -void -setSTNumberSwitchover(bool v) -{ - *getStaticSTNumberSwitchover() = v; -} - /* The range for the mantissa when normalized */ // log(2^63,10) ~ 18.96 // @@ -75,56 +51,28 @@ IOUAmount::normalize() return; } - if (getSTNumberSwitchover()) + Number const v{mantissa_, exponent_}; + *this = fromNumber(v); + if (exponent_ > kMAX_EXPONENT) { - Number const v{mantissa_, exponent_}; - *this = fromNumber(v); - if (exponent_ > kMAX_EXPONENT) - Throw("value overflow"); - if (exponent_ < kMIN_EXPONENT) - *this = beast::kZERO; - return; + Throw("value overflow"); } - - bool const negative = (mantissa_ < 0); - - if (negative) - mantissa_ = -mantissa_; - - while ((mantissa_ < kMIN_MANTISSA) && (exponent_ > kMIN_EXPONENT)) - { - mantissa_ *= 10; - --exponent_; - } - - while (mantissa_ > kMAX_MANTISSA) - { - if (exponent_ >= kMAX_EXPONENT) - Throw("IOUAmount::normalize"); - - mantissa_ /= 10; - ++exponent_; - } - - if ((exponent_ < kMIN_EXPONENT) || (mantissa_ < kMIN_MANTISSA)) + if (exponent_ < kMIN_EXPONENT) { *this = beast::kZERO; - return; } - - if (exponent_ > kMAX_EXPONENT) - Throw("value overflow"); - - if (negative) - mantissa_ = -mantissa_; } IOUAmount::IOUAmount(Number const& other) : IOUAmount(fromNumber(other)) { if (exponent_ > kMAX_EXPONENT) + { Throw("value overflow"); + } if (exponent_ < kMIN_EXPONENT) + { *this = beast::kZERO; + } } IOUAmount& @@ -139,37 +87,7 @@ IOUAmount::operator+=(IOUAmount const& other) return *this; } - if (getSTNumberSwitchover()) - { - *this = IOUAmount{Number{*this} + Number{other}}; - return *this; - } - auto m = other.mantissa_; - auto e = other.exponent_; - - while (exponent_ < e) - { - mantissa_ /= 10; - ++exponent_; - } - - while (e < exponent_) - { - m /= 10; - ++e; - } - - // This addition cannot overflow an std::int64_t but we may throw from - // normalize if the result isn't representable. - mantissa_ += m; - - if (mantissa_ >= -10 && mantissa_ <= 10) - { - *this = beast::kZERO; - return *this; - } - - normalize(); + *this = IOUAmount{Number{*this} + Number{other}}; return *this; } diff --git a/src/libxrpl/protocol/STAmount.cpp b/src/libxrpl/protocol/STAmount.cpp index 25857d387e..c77a1454f8 100644 --- a/src/libxrpl/protocol/STAmount.cpp +++ b/src/libxrpl/protocol/STAmount.cpp @@ -387,47 +387,9 @@ operator+(STAmount const& v1, STAmount const& v2) if (v1.holds()) return {v1.asset_, v1.mpt().value() + v2.mpt().value()}; - if (getSTNumberSwitchover()) - { - auto x = v1; - x = v1.iou() + v2.iou(); - return x; - } - - int ov1 = v1.exponent(), ov2 = v2.exponent(); - std::int64_t vv1 = static_cast(v1.mantissa()); - std::int64_t vv2 = static_cast(v2.mantissa()); - - if (v1.negative()) - vv1 = -vv1; - - if (v2.negative()) - vv2 = -vv2; - - while (ov1 < ov2) - { - vv1 /= 10; - ++ov1; - } - - while (ov2 < ov1) - { - vv2 /= 10; - ++ov2; - } - - // This addition cannot overflow an std::int64_t. It can overflow an - // STAmount and the constructor will throw. - - std::int64_t const fv = vv1 + vv2; - - if ((fv >= -10) && (fv <= 10)) - return {v1.getFName(), v1.asset()}; - - if (fv >= 0) - return STAmount{v1.getFName(), v1.asset(), static_cast(fv), ov1, false}; - - return STAmount{v1.getFName(), v1.asset(), static_cast(-fv), ov1, true}; + auto x = v1; + x = v1.iou() + v2.iou(); + return x; } STAmount @@ -876,7 +838,6 @@ STAmount::canonicalize() if (asset_.holds() && offset_ > 18) Throw("MPT amount out of range"); - if (getSTNumberSwitchover()) { Number const num(isNegative_, value_, offset_, Number::Unchecked{}); auto set = [&](auto const& val) { @@ -898,31 +859,6 @@ STAmount::canonicalize() } offset_ = 0; } - else - { - while (offset_ < 0) - { - value_ /= 10; - ++offset_; - } - - while (offset_ > 0) - { - // N.B. do not move the overflow check to after the - // multiplication - if (native() && value_ > kMAX_NATIVE_N) - { - Throw("Native currency amount out of range"); - } - else if (!native() && value_ > kMAX_MP_TOKEN_AMOUNT) - { - Throw("MPT amount out of range"); - } - - value_ *= 10; - --offset_; - } - } if (native() && value_ > kMAX_NATIVE_N) { @@ -936,53 +872,7 @@ STAmount::canonicalize() return; } - if (getSTNumberSwitchover()) - { - *this = iou(); - return; - } - - if (value_ == 0) - { - offset_ = -100; - isNegative_ = false; - return; - } - - while ((value_ < kMIN_VALUE) && (offset_ > kMIN_OFFSET)) - { - value_ *= 10; - --offset_; - } - - while (value_ > kMAX_VALUE) - { - if (offset_ >= kMAX_OFFSET) - Throw("value overflow"); - - value_ /= 10; - ++offset_; - } - - if ((offset_ < kMIN_OFFSET) || (value_ < kMIN_VALUE)) - { - value_ = 0; - isNegative_ = false; - offset_ = -100; - return; - } - - if (offset_ > kMAX_OFFSET) - Throw("value overflow"); - - XRPL_ASSERT( - (value_ == 0) || ((value_ >= kMIN_VALUE) && (value_ <= kMAX_VALUE)), - "xrpl::STAmount::canonicalize : value inside range"); - XRPL_ASSERT( - (value_ == 0) || ((offset_ >= kMIN_OFFSET) && (offset_ <= kMAX_OFFSET)), - "xrpl::STAmount::canonicalize : offset inside range"); - XRPL_ASSERT( - (value_ != 0) || (offset_ != -100), "xrpl::STAmount::canonicalize : value or offset set"); + *this = iou(); } void @@ -1350,44 +1240,8 @@ multiply(STAmount const& v1, STAmount const& v2, Asset const& asset) return STAmount(asset, minV * maxV); } - if (getSTNumberSwitchover()) - { - auto const r = Number{v1} * Number{v2}; - return STAmount{asset, r}; - } - - std::uint64_t value1 = v1.mantissa(); - std::uint64_t value2 = v2.mantissa(); - int offset1 = v1.exponent(); - int offset2 = v2.exponent(); - - if (v1.integral()) - { - while (value1 < STAmount::kMIN_VALUE) - { - value1 *= 10; - --offset1; - } - } - - if (v2.integral()) - { - while (value2 < STAmount::kMIN_VALUE) - { - value2 *= 10; - --offset2; - } - } - - // We multiply the two mantissas (each is between 10^15 - // and 10^16), so their product is in the 10^30 to 10^32 - // range. Dividing their product by 10^14 maintains the - // precision, by scaling the result to 10^16 to 10^18. - return STAmount( - asset, - muldiv(value1, value2, kTEN_TO14) + 7, - offset1 + offset2 + 14, - v1.negative() != v2.negative()); + auto const r = Number{v1} * Number{v2}; + return STAmount{asset, r}; } // This is the legacy version of canonicalizeRound. It's been in use diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index 79dec33c66..6f3dfd6f66 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include @@ -1183,8 +1182,6 @@ Transactor::operator()() // with_txn_type(). // // raii classes for the current ledger rules. - // fixUniversalNumber predate the rulesGuard and should be replaced. - NumberSO const stNumberSO{view().rules().enabled(fixUniversalNumber)}; CurrentTransactionRulesGuard const currentTransactionRulesGuard(view().rules()); #ifdef DEBUG diff --git a/src/libxrpl/tx/applySteps.cpp b/src/libxrpl/tx/applySteps.cpp index 7833341808..77ffadee05 100644 --- a/src/libxrpl/tx/applySteps.cpp +++ b/src/libxrpl/tx/applySteps.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include #include #include @@ -71,14 +70,11 @@ withTxnType(Rules const& rules, TxType txnType, F&& f) // // See also Transactor::operator(). // - std::optional stNumberSO; std::optional rulesGuard; std::optional mantissaScaleGuard; if (rules.enabled(featureSingleAssetVault) || rules.enabled(featureLendingProtocol)) { // raii classes for the current ledger rules. - // fixUniversalNumber predates the rulesGuard and should be replaced. - stNumberSO.emplace(rules.enabled(fixUniversalNumber)); rulesGuard.emplace(rules); } else diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 8e2c0255ef..abb20e6009 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -4344,13 +4344,10 @@ private: testcase("Amendment"); FeatureBitset const all{testableAmendments()}; FeatureBitset const noAMM{all - featureAMM}; - FeatureBitset const noNumber{all - fixUniversalNumber}; - FeatureBitset const noAMMAndNumber{all - featureAMM - fixUniversalNumber}; using namespace jtx; - for (auto const& feature : {noAMM, noNumber, noAMMAndNumber}) { - Env env{*this, feature}; + Env env{*this, noAMM}; fund(env, gw_, {alice_}, {USD(1'000)}, Fund::All); AMM amm(env, alice_, XRP(1'000), USD(1'000), Ter(temDISABLED)); diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 3f4260e1b1..6f204886cf 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -2218,17 +2218,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite // See the impact of rounding when the nft is sold for small amounts // of drops. - for (auto numberSwitchOver : {true}) { - if (numberSwitchOver) - { - env.enableFeature(fixUniversalNumber); - } - else - { - env.disableFeature(fixUniversalNumber); - } - // An nft with a transfer fee of 1 basis point. uint256 const nftID = token::getNextID(env, alice, 0u, tfTransferable, 1); env(token::mint(alice), Txflags(tfTransferable), token::XferFee(1)); @@ -2250,7 +2240,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite // minter sells to carol. The payment is just small enough that // alice does not get any transfer fee. - auto pmt = numberSwitchOver ? drops(50000) : drops(99999); + auto pmt = drops(50000); STAmount carolBalance = env.balance(carol); uint256 const minterSellOfferIndex = keylet::nftoffer(minter, env.seq(minter)).key; env(token::createOffer(minter, nftID, pmt), Txflags(tfSellNFToken)); @@ -2267,7 +2257,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite // transfer that enables a transfer fee of 1 basis point. STAmount beckyBalance = env.balance(becky); uint256 const beckyBuyOfferIndex = keylet::nftoffer(becky, env.seq(becky)).key; - pmt = numberSwitchOver ? drops(50001) : drops(100000); + pmt = drops(50001); env(token::createOffer(becky, nftID, pmt), token::Owner(carol)); env.close(); env(token::acceptBuyOffer(carol, beckyBuyOfferIndex)); diff --git a/src/test/app/OfferMPT_test.cpp b/src/test/app/OfferMPT_test.cpp index 21ee05289c..7fbf042fca 100644 --- a/src/test/app/OfferMPT_test.cpp +++ b/src/test/app/OfferMPT_test.cpp @@ -1823,7 +1823,6 @@ public: using namespace jtx; Env env{*this, features}; - env.enableFeature(fixUniversalNumber); auto const gw = Account{"gateway"}; auto const alice = Account{"alice"}; diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index 5d5f16981c..dbedfc61a3 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -1969,17 +1969,8 @@ public: using namespace jtx; - for (auto numberSwitchOver : {false, true}) { Env env{*this, features}; - if (numberSwitchOver) - { - env.enableFeature(fixUniversalNumber); - } - else - { - env.disableFeature(fixUniversalNumber); - } auto const gw = Account{"gateway"}; auto const alice = Account{"alice"}; @@ -2008,14 +1999,7 @@ public: jrr = ledgerEntryState(env, bob, gw, "USD"); json::Value const bobUSD = jrr[jss::node][sfBalance.fieldName][jss::value]; - if (!numberSwitchOver) - { - BEAST_EXPECT(bobUSD == "-0.966500000033334"); - } - else - { - BEAST_EXPECT(bobUSD == "-0.9665000000333333"); - } + BEAST_EXPECT(bobUSD == "-0.9665000000333333"); } } diff --git a/src/xrpld/app/misc/detail/TxQ.cpp b/src/xrpld/app/misc/detail/TxQ.cpp index 8b478f964e..a963c68456 100644 --- a/src/xrpld/app/misc/detail/TxQ.cpp +++ b/src/xrpld/app/misc/detail/TxQ.cpp @@ -15,8 +15,6 @@ #include #include #include -#include -#include #include #include #include @@ -305,7 +303,6 @@ TxQ::MaybeTx::apply(Application& app, OpenView& view, beast::Journal j) { // If the rules or flags change, preflight again XRPL_ASSERT(pfResult, "xrpl::TxQ::MaybeTx::apply : preflight result is set"); - NumberSO const stNumberSO{view.rules().enabled(fixUniversalNumber)}; // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above if (pfResult->rules != view.rules() || pfResult->flags != flags) @@ -730,8 +727,6 @@ TxQ::apply( ApplyFlags flags, beast::Journal j) { - NumberSO const stNumberSO{view.rules().enabled(fixUniversalNumber)}; - // See if the transaction is valid, properly formed, // etc. before doing potentially expensive queue // replace and multi-transaction operations.