Merge remote-tracking branch 'XRPLF/develop' into ximinez/lending-XLS-66

* XRPLF/develop:
  refactor: Retire fixSTAmountCanonicalize code (5956)

include/xrpl/protocol/STAmount.h

@@ -765,37 +765,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
 
 //------------------------------------------------------------------------------

include/xrpl/protocol/detail/features.macro

@@ -92,7 +92,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)
@@ -145,6 +144,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)

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<bool>&
-getStaticSTAmountCanonicalizeSwitchover()
-{
-    static LocalValue<bool> 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)
-            // log(cMaxNativeN, 10) == 17
+            Throw<std::runtime_error>("Native currency amount out of range");
-            if (native() && mOffset > 17)
+        // log(maxMPTokenAmount, 10) ~ 18.96
-                Throw<std::runtime_error>(
+        if (mAsset.holds<MPTIssue>() && mOffset > 18)
-                    "Native currency amount out of range");
+            Throw<std::runtime_error>("MPT amount out of range");
-            // log(maxMPTokenAmount, 10) ~ 18.96
-            if (mAsset.holds<MPTIssue>() && mOffset > 18)
-                Throw<std::runtime_error>("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
-                    // N.B. do not move the overflow check to after the
+                if (native() && mValue > cMaxNativeN)
-                    // multiplication
+                    Throw<std::runtime_error>(
-                    if (native() && mValue > cMaxNativeN)
+                        "Native currency amount out of range");
-                        Throw<std::runtime_error>(
+                else if (!native() && mValue > maxMPTokenAmount)
-                            "Native currency amount out of range");
+                    Throw<std::runtime_error>("MPT amount out of range");
-                    else if (!native() && mValue > maxMPTokenAmount)
+
-                        Throw<std::runtime_error>("MPT amount out of range");
-                }
                 mValue *= 10;
                 --mOffset;
             }

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,

src/xrpld/app/tx/detail/Transactor.cpp

@@ -1181,9 +1181,8 @@ Transactor::operator()()
 {
     JLOG(j_.trace()) << "apply: " << ctx_.tx.getTransactionID();
 
-    // raii classes for the current ledger rules. fixSTAmountCanonicalize and
+    // raii classes for the current ledger rules.
-    // fixSTAmountCanonicalize predate the rulesGuard and should be replaced.
+    // fixUniversalNumber predate the rulesGuard and should be replaced.
-    STAmountSO stAmountSO{view().rules().enabled(fixSTAmountCanonicalize)};
     NumberSO stNumberSO{view().rules().enabled(fixUniversalNumber)};
     CurrentTransactionRulesGuard currentTransctionRulesGuard(view().rules());
 

src/xrpld/app/tx/detail/apply.cpp

@@ -128,9 +128,7 @@ template <typename PreflightChecks>
 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);
 }