From f8b4f692f1cb3b971138b71912bcd2c2eb9ee4d5 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde Date: Wed, 29 Oct 2025 18:17:50 +0000 Subject: [PATCH] refactor: Retire fixSTAmountCanonicalize code (#5956) Amendments activated for more than 2 years can be retired. This change retires the fixSTAmountCanonicalize amendment. --- include/xrpl/protocol/STAmount.h | 31 ----------- include/xrpl/protocol/detail/features.macro | 2 +- src/libxrpl/protocol/STAmount.cpp | 59 ++++++--------------- src/xrpld/app/misc/detail/TxQ.cpp | 2 - src/xrpld/app/tx/detail/Transactor.cpp | 5 +- src/xrpld/app/tx/detail/apply.cpp | 2 - 6 files changed, 18 insertions(+), 83 deletions(-) diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index f1e34463b6..ee68e33000 100644 --- a/include/xrpl/protocol/STAmount.h +++ b/include/xrpl/protocol/STAmount.h @@ -709,37 +709,6 @@ canAdd(STAmount const& amt1, STAmount const& amt2); bool canSubtract(STAmount const& amt1, STAmount const& amt2); -// Since `canonicalize` does not have access to a ledger, this is needed to put -// the low-level routine stAmountCanonicalize on an amendment switch. Only -// transactions need to use this switchover. Outside of a transaction it's safe -// to unconditionally use the new behavior. - -bool -getSTAmountCanonicalizeSwitchover(); - -void -setSTAmountCanonicalizeSwitchover(bool v); - -/** RAII class to set and restore the STAmount canonicalize switchover. - */ - -class STAmountSO -{ -public: - explicit STAmountSO(bool v) : saved_(getSTAmountCanonicalizeSwitchover()) - { - setSTAmountCanonicalizeSwitchover(v); - } - - ~STAmountSO() - { - setSTAmountCanonicalizeSwitchover(saved_); - } - -private: - bool saved_; -}; - } // namespace ripple //------------------------------------------------------------------------------ diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index b80b61fc8c..0c817829db 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -91,7 +91,6 @@ XRPL_FIX (TrustLinesToSelf, Supported::yes, VoteBehavior::DefaultNo XRPL_FEATURE(NonFungibleTokensV1_1, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(ExpandedSignerList, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(CheckCashMakesTrustLine, Supported::yes, VoteBehavior::DefaultNo) -XRPL_FIX (STAmountCanonicalize, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(TicketBatch, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(NegativeUNL, Supported::yes, VoteBehavior::DefaultYes) @@ -144,6 +143,7 @@ XRPL_RETIRE(fix1578) XRPL_RETIRE(fix1781) XRPL_RETIRE(fixCheckThreading) XRPL_RETIRE(fixRmSmallIncreasedQOffers) +XRPL_RETIRE(fixSTAmountCanonicalize) XRPL_RETIRE(CryptoConditions) XRPL_RETIRE(Escrow) XRPL_RETIRE(EnforceInvariants) diff --git a/src/libxrpl/protocol/STAmount.cpp b/src/libxrpl/protocol/STAmount.cpp index 0c72244885..dfd1b3dfbe 100644 --- a/src/libxrpl/protocol/STAmount.cpp +++ b/src/libxrpl/protocol/STAmount.cpp @@ -68,29 +68,6 @@ namespace ripple { -namespace { - -// Use a static inside a function to help prevent order-of-initialzation issues -LocalValue& -getStaticSTAmountCanonicalizeSwitchover() -{ - static LocalValue r{true}; - return r; -} -} // namespace - -bool -getSTAmountCanonicalizeSwitchover() -{ - return *getStaticSTAmountCanonicalizeSwitchover(); -} - -void -setSTAmountCanonicalizeSwitchover(bool v) -{ - *getStaticSTAmountCanonicalizeSwitchover() = v; -} - static std::uint64_t const tenTo14 = 100000000000000ull; static std::uint64_t const tenTo14m1 = tenTo14 - 1; static std::uint64_t const tenTo17 = tenTo14 * 1000; @@ -884,18 +861,14 @@ STAmount::canonicalize() return; } - if (getSTAmountCanonicalizeSwitchover()) - { - // log(cMaxNativeN, 10) == 17 - if (native() && mOffset > 17) - Throw( - "Native currency amount out of range"); - // log(maxMPTokenAmount, 10) ~ 18.96 - if (mAsset.holds() && mOffset > 18) - Throw("MPT amount out of range"); - } + // log(cMaxNativeN, 10) == 17 + if (native() && mOffset > 17) + Throw("Native currency amount out of range"); + // log(maxMPTokenAmount, 10) ~ 18.96 + if (mAsset.holds() && mOffset > 18) + Throw("MPT amount out of range"); - if (getSTNumberSwitchover() && getSTAmountCanonicalizeSwitchover()) + if (getSTNumberSwitchover()) { Number num( mIsNegative ? -mValue : mValue, mOffset, Number::unchecked{}); @@ -919,16 +892,14 @@ STAmount::canonicalize() while (mOffset > 0) { - if (getSTAmountCanonicalizeSwitchover()) - { - // N.B. do not move the overflow check to after the - // multiplication - if (native() && mValue > cMaxNativeN) - Throw( - "Native currency amount out of range"); - else if (!native() && mValue > maxMPTokenAmount) - Throw("MPT amount out of range"); - } + // N.B. do not move the overflow check to after the + // multiplication + if (native() && mValue > cMaxNativeN) + Throw( + "Native currency amount out of range"); + else if (!native() && mValue > maxMPTokenAmount) + Throw("MPT amount out of range"); + mValue *= 10; --mOffset; } diff --git a/src/xrpld/app/misc/detail/TxQ.cpp b/src/xrpld/app/misc/detail/TxQ.cpp index 7c0a6f07e2..23a083fbe8 100644 --- a/src/xrpld/app/misc/detail/TxQ.cpp +++ b/src/xrpld/app/misc/detail/TxQ.cpp @@ -300,7 +300,6 @@ TxQ::MaybeTx::apply(Application& app, OpenView& view, beast::Journal j) // If the rules or flags change, preflight again XRPL_ASSERT( pfresult, "ripple::TxQ::MaybeTx::apply : preflight result is set"); - STAmountSO stAmountSO{view.rules().enabled(fixSTAmountCanonicalize)}; NumberSO stNumberSO{view.rules().enabled(fixUniversalNumber)}; if (pfresult->rules != view.rules() || pfresult->flags != flags) @@ -734,7 +733,6 @@ TxQ::apply( ApplyFlags flags, beast::Journal j) { - STAmountSO stAmountSO{view.rules().enabled(fixSTAmountCanonicalize)}; NumberSO stNumberSO{view.rules().enabled(fixUniversalNumber)}; // See if the transaction is valid, properly formed, diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 2f62a142c0..6e3b56fe99 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -1163,9 +1163,8 @@ Transactor::operator()() { JLOG(j_.trace()) << "apply: " << ctx_.tx.getTransactionID(); - // raii classes for the current ledger rules. fixSTAmountCanonicalize and - // fixSTAmountCanonicalize predate the rulesGuard and should be replaced. - STAmountSO stAmountSO{view().rules().enabled(fixSTAmountCanonicalize)}; + // raii classes for the current ledger rules. + // fixUniversalNumber predate the rulesGuard and should be replaced. NumberSO stNumberSO{view().rules().enabled(fixUniversalNumber)}; CurrentTransactionRulesGuard currentTransctionRulesGuard(view().rules()); diff --git a/src/xrpld/app/tx/detail/apply.cpp b/src/xrpld/app/tx/detail/apply.cpp index e2e0adae45..0ddaf0578e 100644 --- a/src/xrpld/app/tx/detail/apply.cpp +++ b/src/xrpld/app/tx/detail/apply.cpp @@ -138,9 +138,7 @@ template ApplyResult apply(Application& app, OpenView& view, PreflightChecks&& preflightChecks) { - STAmountSO stAmountSO{view.rules().enabled(fixSTAmountCanonicalize)}; NumberSO stNumberSO{view.rules().enabled(fixUniversalNumber)}; - return doApply(preclaim(preflightChecks(), app, view), app, view); }