Rm some offers where the quality is reduced

src/ripple/app/tx/impl/OfferStream.cpp

@@ -19,6 +19,7 @@
 
 #include <ripple/app/tx/impl/OfferStream.h>
 #include <ripple/basics/Log.h>
+#include <ripple/protocol/Feature.h>
 
 namespace ripple {
 
@@ -132,6 +133,73 @@ accountFundsHelper(
         view, id, issue.currency, issue.account, freezeHandling, j));
 }
 
+template <class TIn, class TOut>
+template <class TTakerPays, class TTakerGets>
+bool
+TOfferStreamBase<TIn, TOut>::shouldRmSmallIncreasedQOffer() const
+{
+    static_assert(
+        std::is_same_v<TTakerPays, IOUAmount> ||
+            std::is_same_v<TTakerPays, XRPAmount>,
+        "STAmount is not supported");
+
+    static_assert(
+        std::is_same_v<TTakerGets, IOUAmount> ||
+            std::is_same_v<TTakerGets, XRPAmount>,
+        "STAmount is not supported");
+
+    static_assert(
+        !std::is_same_v<TTakerPays, XRPAmount> ||
+            !std::is_same_v<TTakerGets, XRPAmount>,
+        "Cannot have XRP/XRP offers");
+
+    if (!view_.rules().enabled(fixRmSmallIncreasedQOffers))
+        return false;
+
+    // Consider removing the offer if:
+    //  o `TakerPays` is XRP (because of XRP drops granularity) or
+    //  o `TakerPays` and `TakerGets` are both IOU and `TakerPays`<`TakerGets`
+    constexpr bool const inIsXRP = std::is_same_v<TTakerPays, XRPAmount>;
+    constexpr bool const outIsXRP = std::is_same_v<TTakerGets, XRPAmount>;
+
+    if constexpr (outIsXRP)
+    {
+        // If `TakerGets` is XRP, the worst this offer's quality can change is
+        // to about 10^-81 `TakerPays` and 1 drop `TakerGets`. This will be
+        // remarkably good quality for any realistic asset, so these offers
+        // don't need this extra check.
+        return false;
+    }
+
+    TAmounts<TTakerPays, TTakerGets> const ofrAmts{
+        toAmount<TTakerPays>(offer_.amount().in),
+        toAmount<TTakerGets>(offer_.amount().out)};
+
+    if constexpr (!inIsXRP && !outIsXRP)
+    {
+        if (ofrAmts.in >= ofrAmts.out)
+            return false;
+    }
+
+    TTakerGets const ownerFunds = toAmount<TTakerGets>(*ownerFunds_);
+
+    auto const effectiveAmounts = [&] {
+        if (offer_.owner() != offer_.issueOut().account &&
+            ownerFunds < ofrAmts.out)
+        {
+            // adjust the amounts by owner funds
+            return offer_.quality().ceil_out(ofrAmts, ownerFunds);
+        }
+        return ofrAmts;
+    }();
+
+    if (effectiveAmounts.in > TTakerPays::minPositiveAmount())
+        return false;
+
+    Quality const effectiveQuality{effectiveAmounts};
+    return effectiveQuality < offer_.quality();
+}
+
 template <class TIn, class TOut>
 bool
 TOfferStreamBase<TIn, TOut>::step()
@@ -222,6 +290,68 @@ TOfferStreamBase<TIn, TOut>::step()
                     << "Removing became unfunded offer " << entry->key();
             }
             offer_ = TOffer<TIn, TOut>{};
+            // See comment at top of loop for how the offer is removed
+            continue;
+        }
+
+        bool const rmSmallIncreasedQOffer = [&] {
+            bool const inIsXRP = isXRP(offer_.issueIn());
+            bool const outIsXRP = isXRP(offer_.issueOut());
+            if (inIsXRP && !outIsXRP)
+            {
+                // Without the `if constexpr`, the
+                // `shouldRmSmallIncreasedQOffer` template will be instantiated
+                // even if it is never used. This can cause compiler errors in
+                // some cases, hence the `if constexpr` guard.
+                // Note that TIn can be XRPAmount or STAmount, and TOut can be
+                // IOUAmount or STAmount.
+                if constexpr (!(std::is_same_v<TIn, IOUAmount> ||
+                                std::is_same_v<TOut, XRPAmount>))
+                    return shouldRmSmallIncreasedQOffer<XRPAmount, IOUAmount>();
+            }
+            if (!inIsXRP && outIsXRP)
+            {
+                // See comment above for `if constexpr` rationale
+                if constexpr (!(std::is_same_v<TIn, XRPAmount> ||
+                                std::is_same_v<TOut, IOUAmount>))
+                    return shouldRmSmallIncreasedQOffer<IOUAmount, XRPAmount>();
+            }
+            if (!inIsXRP && !outIsXRP)
+            {
+                // See comment above for `if constexpr` rationale
+                if constexpr (!(std::is_same_v<TIn, XRPAmount> ||
+                                std::is_same_v<TOut, XRPAmount>))
+                    return shouldRmSmallIncreasedQOffer<IOUAmount, IOUAmount>();
+            }
+            assert(0);  // xrp/xrp offer!?! should never happen
+            return false;
+        }();
+
+        if (rmSmallIncreasedQOffer)
+        {
+            auto const original_funds = accountFundsHelper(
+                cancelView_,
+                offer_.owner(),
+                amount.out,
+                offer_.issueOut(),
+                fhZERO_IF_FROZEN,
+                j_);
+
+            if (original_funds == *ownerFunds_)
+            {
+                permRmOffer(entry->key());
+                JLOG(j_.trace())
+                    << "Removing tiny offer due to reduced quality "
+                    << entry->key();
+            }
+            else
+            {
+                JLOG(j_.trace()) << "Removing tiny offer that became tiny due "
+                                    "to reduced quality "
+                                 << entry->key();
+            }
+            offer_ = TOffer<TIn, TOut>{};
+            // See comment at top of loop for how the offer is removed
             continue;
         }
 

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

@@ -85,6 +85,10 @@ class TOfferStreamBase
     virtual void
     permRmOffer(uint256 const& offerIndex) = 0;
 
+    template <class TTakerPays, class TTakerGets>
+    bool
+    shouldRmSmallIncreasedQOffer() const;
+
 public:
     TOfferStreamBase(
         ApplyView& view,

src/ripple/basics/IOUAmount.h

@@ -129,6 +129,9 @@ class IOUAmount : private boost::totally_ordered<IOUAmount>,
     {
         return mantissa_;
     }
+
+    static IOUAmount
+    minPositiveAmount();
 };
 
 std::string

src/ripple/basics/XRPAmount.h

@@ -238,6 +238,12 @@ class XRPAmount : private boost::totally_ordered<XRPAmount>,
         s >> val.drops_;
         return s;
     }
+
+    static XRPAmount
+    minPositiveAmount()
+    {
+        return XRPAmount{1};
+    }
 };
 
 /** Number of drops per 1 XRP */

src/ripple/basics/impl/IOUAmount.cpp

@@ -34,6 +34,12 @@ static std::int64_t const maxMantissa = 9999999999999999ull;
 static int const minExponent = -96;
 static int const maxExponent = 80;
 
+IOUAmount
+IOUAmount::minPositiveAmount()
+{
+    return IOUAmount(minMantissa, minExponent);
+}
+
 void
 IOUAmount::normalize()
 {
@@ -353,7 +359,7 @@ mulRatio(
         {
             if (!result)
             {
-                return IOUAmount(minMantissa, minExponent);
+                return IOUAmount::minPositiveAmount();
             }
             // This addition cannot overflow because the mantissa is already
             // normalized

src/ripple/protocol/AmountConversions.h

@@ -63,11 +63,7 @@ toSTAmount(XRPAmount const& xrp, Issue const& iss)
 
 template <class T>
 T
-toAmount(STAmount const& amt)
-{
-    static_assert(sizeof(T) == -1, "Must use specialized function");
-    return T(0);
-}
+toAmount(STAmount const& amt) = delete;
 
 template <>
 inline STAmount
@@ -102,6 +98,28 @@ toAmount<XRPAmount>(STAmount const& amt)
     return XRPAmount(sMant);
 }
 
+template <class T>
+T
+toAmount(IOUAmount const& amt) = delete;
+
+template <>
+inline IOUAmount
+toAmount<IOUAmount>(IOUAmount const& amt)
+{
+    return amt;
+}
+
+template <class T>
+T
+toAmount(XRPAmount const& amt) = delete;
+
+template <>
+inline XRPAmount
+toAmount<XRPAmount>(XRPAmount const& amt)
+{
+    return amt;
+}
+
 }  // namespace ripple
 
 #endif

src/ripple/protocol/Feature.h

@@ -116,6 +116,7 @@ class FeatureCollections
         "TicketBatch",
         "FlowSortStrands",
         "fixSTAmountCanonicalize",
+        "fixRmSmallIncreasedQOffers",
     };
 
     std::vector<uint256> features;
@@ -376,6 +377,7 @@ extern uint256 const featureNegativeUNL;
 extern uint256 const featureTicketBatch;
 extern uint256 const featureFlowSortStrands;
 extern uint256 const fixSTAmountCanonicalize;
+extern uint256 const fixRmSmallIncreasedQOffers;
 
 }  // namespace ripple
 

src/ripple/protocol/Quality.h

@@ -132,6 +132,13 @@ class Quality
     /** Create a quality from the ratio of two amounts. */
     explicit Quality(Amounts const& amount);
 
+    /** Create a quality from the ratio of two amounts. */
+    template <class In, class Out>
+    Quality(TAmounts<In, Out> const& amount)
+        : Quality(Amounts(toSTAmount(amount.in), toSTAmount(amount.out)))
+    {
+    }
+
     /** Create a quality from the ratio of two amounts. */
     template <class In, class Out>
     Quality(Out const& out, In const& in)

src/ripple/protocol/impl/Feature.cpp

@@ -135,6 +135,7 @@ detail::supportedAmendments()
         "TicketBatch",
         "FlowSortStrands",
         "fixSTAmountCanonicalize",
+        "fixRmSmallIncreasedQOffers",
     };
     return supported;
 }
@@ -190,7 +191,8 @@ uint256 const
     featureNegativeUNL              = *getRegisteredFeature("NegativeUNL"),
     featureTicketBatch              = *getRegisteredFeature("TicketBatch"),
     featureFlowSortStrands          = *getRegisteredFeature("FlowSortStrands"),
-    fixSTAmountCanonicalize         = *getRegisteredFeature("fixSTAmountCanonicalize");
+    fixSTAmountCanonicalize         = *getRegisteredFeature("fixSTAmountCanonicalize"),
+    fixRmSmallIncreasedQOffers      = *getRegisteredFeature("fixRmSmallIncreasedQOffers");
 
 // The following amendments have been active for at least two years. Their
 // pre-amendment code has been removed and the identifiers are deprecated.