Amendment fixReducedOffersV2 fixes issue #4937

src/ripple/app/paths/AMMOffer.h

@@ -103,15 +103,15 @@ class AMMOffer
     limitOut(
         TAmounts<TIn, TOut> const& offrAmt,
         TOut const& limit,
-        bool fixReducedOffers,
         bool roundUp) const;
 
     /** Limit in of the provided offer. If one-path then swapIn
      * using current balances. If multi-path then ceil_in using
      * current quality.
      */
     TAmounts<TIn, TOut>
-    limitIn(TAmounts<TIn, TOut> const& offrAmt, TIn const& limit) const;
+    limitIn(TAmounts<TIn, TOut> const& offrAmt, TIn const& limit, bool roundUp)
+        const;
 
     QualityFunction
     getQualityFunc() const;

src/ripple/app/paths/impl/AMMOffer.cpp

@@ -81,7 +81,6 @@ TAmounts<TIn, TOut>
 AMMOffer<TIn, TOut>::limitOut(
     TAmounts<TIn, TOut> const& offrAmt,
     TOut const& limit,
-    bool fixReducedOffers,
     bool roundUp) const
 {
     // Change the offer size proportionally to the original offer quality
@@ -92,7 +91,8 @@ AMMOffer<TIn, TOut>::limitOut(
     // poolPays * poolGets < (poolPays - assetOut) * (poolGets + assetIn)
     if (ammLiquidity_.multiPath())
     {
-        if (fixReducedOffers)
+        if (auto const& rules = getCurrentTransactionRules();
+            rules && rules->enabled(fixReducedOffersV1))
             // It turns out that the ceil_out implementation has some slop in
             // it.  ceil_out_strict removes that slop.  But removing that slop
             // affects transaction outcomes, so the change must be made using
@@ -110,11 +110,18 @@ template <typename TIn, typename TOut>
 TAmounts<TIn, TOut>
 AMMOffer<TIn, TOut>::limitIn(
     TAmounts<TIn, TOut> const& offrAmt,
-    TIn const& limit) const
+    TIn const& limit,
+    bool roundUp) const
 {
     // See the comments above in limitOut().
     if (ammLiquidity_.multiPath())
+    {
+        if (auto const& rules = getCurrentTransactionRules();
+            rules && rules->enabled(fixReducedOffersV2))
+            return quality().ceil_in_strict(offrAmt, limit, roundUp);
+
         return quality().ceil_in(offrAmt, limit);
+    }
     return {limit, swapAssetIn(balances_, limit, ammLiquidity_.tradingFee())};
 }
 

src/ripple/app/paths/impl/BookStep.cpp

@@ -668,7 +668,14 @@ limitStepIn(
         stpAmt.in = limit;
         auto const inLmt =
             mulRatio(stpAmt.in, QUALITY_ONE, transferRateIn, /*roundUp*/ false);
-        ofrAmt = offer.limitIn(ofrAmt, inLmt);
+        // It turns out we can prevent order book blocking by (strictly)
+        // rounding down the ceil_in() result.  By rounding down we guarantee
+        // that the quality of an offer left in the ledger is as good or
+        // better than the quality of the containing order book page.
+        //
+        // This adjustment changes transaction outcomes, so it must be made
+        // under an amendment.
+        ofrAmt = offer.limitIn(ofrAmt, inLmt, /* roundUp */ false);
         stpAmt.out = ofrAmt.out;
         ownerGives = mulRatio(
             ofrAmt.out, transferRateOut, QUALITY_ONE, /*roundUp*/ false);
@@ -685,8 +692,7 @@ limitStepOut(
     TOut& ownerGives,
     std::uint32_t transferRateIn,
     std::uint32_t transferRateOut,
-    TOut const& limit,
-    Rules const& rules)
+    TOut const& limit)
 {
     if (limit < stpAmt.out)
     {
@@ -696,7 +702,6 @@ limitStepOut(
         ofrAmt = offer.limitOut(
             ofrAmt,
             stpAmt.out,
-            rules.enabled(fixReducedOffersV1),
             /*roundUp*/ true);
         stpAmt.in =
             mulRatio(ofrAmt.in, transferRateIn, QUALITY_ONE, /*roundUp*/ true);
@@ -736,7 +741,6 @@ BookStep<TIn, TOut, TDerived>::forEachOffer(
         sb, afView, book_, sb.parentCloseTime(), counter, j_);
 
     bool const flowCross = afView.rules().enabled(featureFlowCross);
-    bool const fixReduced = afView.rules().enabled(fixReducedOffersV1);
     bool offerAttempted = false;
     std::optional<Quality> ofrQ;
     auto execOffer = [&](auto& offer) {
@@ -817,8 +821,7 @@ BookStep<TIn, TOut, TDerived>::forEachOffer(
             // It turns out we can prevent order book blocking by (strictly)
             // rounding down the ceil_out() result.  This adjustment changes
             // transaction outcomes, so it must be made under an amendment.
-            ofrAmt = offer.limitOut(
-                ofrAmt, stpAmt.out, fixReduced, /*roundUp*/ false);
+            ofrAmt = offer.limitOut(ofrAmt, stpAmt.out, /*roundUp*/ false);
 
             stpAmt.in =
                 mulRatio(ofrAmt.in, ofrInRate, QUALITY_ONE, /*roundUp*/ true);
@@ -1055,8 +1058,7 @@ BookStep<TIn, TOut, TDerived>::revImp(
                 ownerGivesAdj,
                 transferRateIn,
                 transferRateOut,
-                remainingOut,
-                afView.rules());
+                remainingOut);
             remainingOut = beast::zero;
             savedIns.insert(stpAdjAmt.in);
             savedOuts.insert(remainingOut);
@@ -1208,8 +1210,7 @@ BookStep<TIn, TOut, TDerived>::fwdImp(
                 ownerGivesAdjRev,
                 transferRateIn,
                 transferRateOut,
-                remainingOut,
-                afView.rules());
+                remainingOut);
 
             if (stpAdjAmtRev.in == remainingIn)
             {
@@ -1228,7 +1229,7 @@ BookStep<TIn, TOut, TDerived>::fwdImp(
             }
             else
             {
-                // This is (likely) a problem case, and wil be caught
+                // This is (likely) a problem case, and will be caught
                 // with later checks
                 savedOuts.insert(lastOutAmt);
             }

src/ripple/app/tx/impl/Offer.h

@@ -23,6 +23,7 @@
 #include <ripple/basics/contract.h>
 #include <ripple/ledger/View.h>
 #include <ripple/protocol/Quality.h>
+#include <ripple/protocol/Rules.h>
 #include <ripple/protocol/SField.h>
 #include <ripple/protocol/STLedgerEntry.h>
 #include <ostream>
@@ -140,11 +141,11 @@ class TOffer : private TOfferBase<TIn, TOut>
     limitOut(
         TAmounts<TIn, TOut> const& offrAmt,
         TOut const& limit,
-        bool fixReducedOffers,
         bool roundUp) const;
 
     TAmounts<TIn, TOut>
-    limitIn(TAmounts<TIn, TOut> const& offrAmt, TIn const& limit) const;
+    limitIn(TAmounts<TIn, TOut> const& offrAmt, TIn const& limit, bool roundUp)
+        const;
 
     template <typename... Args>
     static TER
@@ -219,10 +220,10 @@ TAmounts<TIn, TOut>
 TOffer<TIn, TOut>::limitOut(
     TAmounts<TIn, TOut> const& offrAmt,
     TOut const& limit,
-    bool fixReducedOffers,
     bool roundUp) const
 {
-    if (fixReducedOffers)
+    if (auto const& rules = getCurrentTransactionRules();
+        rules && rules->enabled(fixReducedOffersV1))
         // It turns out that the ceil_out implementation has some slop in
         // it.  ceil_out_strict removes that slop.  But removing that slop
         // affects transaction outcomes, so the change must be made using
@@ -233,9 +234,18 @@ TOffer<TIn, TOut>::limitOut(
 
 template <class TIn, class TOut>
 TAmounts<TIn, TOut>
-TOffer<TIn, TOut>::limitIn(TAmounts<TIn, TOut> const& offrAmt, TIn const& limit)
-    const
+TOffer<TIn, TOut>::limitIn(
+    TAmounts<TIn, TOut> const& offrAmt,
+    TIn const& limit,
+    bool roundUp) const
 {
+    if (auto const& rules = getCurrentTransactionRules();
+        rules && rules->enabled(fixReducedOffersV2))
+        // It turns out that the ceil_in implementation has some slop in
+        // it.  ceil_in_strict removes that slop.  But removing that slop
+        // affects transaction outcomes, so the change must be made using
+        // an amendment.
+        return quality().ceil_in_strict(offrAmt, limit, roundUp);
     return m_quality.ceil_in(offrAmt, limit);
 }
 

src/ripple/protocol/Feature.h

@@ -74,7 +74,7 @@ namespace detail {
 // Feature.cpp. Because it's only used to reserve storage, and determine how
 // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
 // the actual number of amendments. A LogicError on startup will verify this.
-static constexpr std::size_t numFeatures = 73;
+static constexpr std::size_t numFeatures = 74;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated
@@ -360,6 +360,7 @@ extern uint256 const fixEmptyDID;
 extern uint256 const fixXChainRewardRounding;
 extern uint256 const fixPreviousTxnID;
 extern uint256 const fixAMMv1_1;
+extern uint256 const fixReducedOffersV2;
 
 }  // namespace ripple