Optimize payment path exploration in flow:

* Use theoretical quality to order the strands
* Do not use strands below the user specified quality limit
* Stop exploring strands (at the current quality iteration) once any strand is non-dry

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

@@ -52,6 +52,14 @@ protected:
     bool const ownerPaysTransferFee_;
     // Mark as inactive (dry) if too many offers are consumed
     bool inactive_ = false;
+    /** Number of offers consumed or partially consumed the last time
+        the step ran, including expired and unfunded offers.
+
+        N.B. This this not the total number offers consumed by this step for the
+        entire payment, it is only the number the last time it ran. Offers may
+        be partially consumed multiple times during a payment.
+    */
+    std::uint32_t offersUsed_ = 0;
     beast::Journal const j_;
 
     struct Cache
@@ -125,6 +133,9 @@ public:
     qualityUpperBound(ReadView const& v, DebtDirection prevStepDir)
         const override;
 
+    std::uint32_t
+    offersUsed() const override;
+
     std::pair<TIn, TOut>
     revImp(
         PaymentSandbox& sb,
@@ -480,6 +491,13 @@ BookStep<TIn, TOut, TDerived>::qualityUpperBound(
     return {q, dir};
 }
 
+template <class TIn, class TOut, class TDerived>
+std::uint32_t
+BookStep<TIn, TOut, TDerived>::offersUsed() const
+{
+    return offersUsed_;
+}
+
 // Adjust the offer amount and step amount subject to the given input limit
 template <class TIn, class TOut>
 static void
@@ -779,6 +797,7 @@ BookStep<TIn, TOut, TDerived>::revImp(
         auto const r = forEachOffer(sb, afView, prevStepDebtDir, eachOffer);
         boost::container::flat_set<uint256> toRm = std::move(std::get<0>(r));
         std::uint32_t const offersConsumed = std::get<1>(r);
+        offersUsed_ = offersConsumed;
         SetUnion(ofrsToRm, toRm);
 
         if (offersConsumed >= maxOffersToConsume_)
@@ -948,6 +967,7 @@ BookStep<TIn, TOut, TDerived>::fwdImp(
         auto const r = forEachOffer(sb, afView, prevStepDebtDir, eachOffer);
         boost::container::flat_set<uint256> toRm = std::move(std::get<0>(r));
         std::uint32_t const offersConsumed = std::get<1>(r);
+        offersUsed_ = offersConsumed;
         SetUnion(ofrsToRm, toRm);
 
         if (offersConsumed >= maxOffersToConsume_)

src/ripple/app/paths/impl/Steps.h

@@ -188,6 +188,19 @@ public:
     virtual std::pair<boost::optional<Quality>, DebtDirection>
     qualityUpperBound(ReadView const& v, DebtDirection prevStepDir) const = 0;
 
+    /** Return the number of offers consumed or partially consumed the last time
+        the step ran, including expired and unfunded offers.
+
+        N.B. This this not the total number offers consumed by this step for the
+        entire payment, it is only the number the last time it ran. Offers may
+        be partially consumed multiple times during a payment.
+     */
+    virtual std::uint32_t
+    offersUsed() const
+    {
+        return 0;
+    }
+
     /**
        If this step is a BookStep, return the book.
     */
@@ -281,6 +294,18 @@ private:
 
 /// @cond INTERNAL
 using Strand = std::vector<std::unique_ptr<Step>>;
+
+inline std::uint32_t
+offersUsed(Strand const& strand)
+{
+    std::uint32_t r = 0;
+    for (auto const& step : strand)
+    {
+        if (step)
+            r += step->offersUsed();
+    }
+    return r;
+}
 /// @endcond
 
 /// @cond INTERNAL

src/ripple/app/paths/impl/StrandFlow.h

@@ -29,6 +29,7 @@
 #include <ripple/basics/IOUAmount.h>
 #include <ripple/basics/Log.h>
 #include <ripple/basics/XRPAmount.h>
+#include <ripple/protocol/Feature.h>
 
 #include <boost/container/flat_set.hpp>
 
@@ -48,6 +49,9 @@ struct StrandResult
     TOutAmt out = beast::zero;                     ///< Currency amount out
     boost::optional<PaymentSandbox> sandbox;       ///< Resulting Sandbox state
     boost::container::flat_set<uint256> ofrsToRm;  ///< Offers to remove
+    // Num offers consumed or partially consumed (includes expired and unfunded
+    // offers)
+    std::uint32_t ofrsUsed = 0;
     // strand can be inactive if there is no more liquidity or too many offers
     // have been consumed
     bool inactive = false;  ///< Strand should not considered as a further
@@ -57,6 +61,7 @@ struct StrandResult
     StrandResult() = default;
 
     StrandResult(
+        Strand const& strand,
         TInAmt const& in_,
         TOutAmt const& out_,
         PaymentSandbox&& sandbox_,
@@ -67,12 +72,17 @@ struct StrandResult
         , out(out_)
         , sandbox(std::move(sandbox_))
         , ofrsToRm(std::move(ofrsToRm_))
+        , ofrsUsed(offersUsed(strand))
         , inactive(inactive_)
     {
     }
 
-    explicit StrandResult(boost::container::flat_set<uint256> ofrsToRm_)
+    StrandResult(
-        : success(false), ofrsToRm(std::move(ofrsToRm_))
+        Strand const& strand,
+        boost::container::flat_set<uint256> ofrsToRm_)
+        : success(false)
+        , ofrsToRm(std::move(ofrsToRm_))
+        , ofrsUsed(offersUsed(strand))
     {
     }
 };
@@ -108,7 +118,7 @@ flow(
 
     if (isDirectXrpToXrp<TInAmt, TOutAmt>(strand))
     {
-        return Result{std::move(ofrsToRm)};
+        return Result{strand, std::move(ofrsToRm)};
     }
 
     try
@@ -130,7 +140,7 @@ flow(
                 if (strand[i]->isZero(r.second))
                 {
                     JLOG(j.trace()) << "Strand found dry in rev";
-                    return Result{std::move(ofrsToRm)};
+                    return Result{strand, std::move(ofrsToRm)};
                 }
 
                 if (i == 0 && maxIn && *maxIn < get<TInAmt>(r.first))
@@ -148,7 +158,7 @@ flow(
                     if (strand[i]->isZero(r.second))
                     {
                         JLOG(j.trace()) << "First step found dry";
-                        return Result{std::move(ofrsToRm)};
+                        return Result{strand, std::move(ofrsToRm)};
                     }
                     if (get<TInAmt>(r.first) != *maxIn)
                     {
@@ -160,7 +170,7 @@ flow(
                             << to_string(get<TInAmt>(r.first))
                             << " maxIn: " << to_string(*maxIn);
                         assert(0);
-                        return Result{std::move(ofrsToRm)};
+                        return Result{strand, std::move(ofrsToRm)};
                     }
                 }
                 else if (!strand[i]->equalOut(r.second, stepOut))
@@ -181,7 +191,7 @@ flow(
                         // A tiny input amount can cause this step to output
                         // zero. I.e. 10^-80 IOU into an IOU -> XRP offer.
                         JLOG(j.trace()) << "Limiting step found dry";
-                        return Result{std::move(ofrsToRm)};
+                        return Result{strand, std::move(ofrsToRm)};
                     }
                     if (!strand[i]->equalOut(r.second, stepOut))
                     {
@@ -196,7 +206,7 @@ flow(
                         JLOG(j.fatal()) << "Re-executed limiting step failed";
 #endif
                         assert(0);
-                        return Result{std::move(ofrsToRm)};
+                        return Result{strand, std::move(ofrsToRm)};
                     }
                 }
 
@@ -215,7 +225,7 @@ flow(
                     // A tiny input amount can cause this step to output zero.
                     // I.e. 10^-80 IOU into an IOU -> XRP offer.
                     JLOG(j.trace()) << "Non-limiting step found dry";
-                    return Result{std::move(ofrsToRm)};
+                    return Result{strand, std::move(ofrsToRm)};
                 }
                 if (!strand[i]->equalIn(r.first, stepIn))
                 {
@@ -230,7 +240,7 @@ flow(
                     JLOG(j.fatal()) << "Re-executed forward pass failed";
 #endif
                     assert(0);
-                    return Result{std::move(ofrsToRm)};
+                    return Result{strand, std::move(ofrsToRm)};
                 }
                 stepIn = r.second;
             }
@@ -267,6 +277,7 @@ flow(
             [](std::unique_ptr<Step> const& step) { return step->inactive(); });
 
         return Result(
+            strand,
             get<TInAmt>(strandIn),
             get<TOutAmt>(strandOut),
             std::move(*sb),
@@ -275,7 +286,7 @@ flow(
     }
     catch (FlowException const&)
     {
-        return Result{std::move(ofrsToRm)};
+        return Result{strand, std::move(ofrsToRm)};
     }
 }
 
@@ -366,41 +377,85 @@ public:
     // Start a new iteration in the search for liquidity
     // Set the current strands to the strands in `next_`
     void
-    activateNext()
+    activateNext(
+        ReadView const& v,
+        boost::optional<Quality> const& limitQuality)
     {
-        // Swap, don't move, so we keep the reserve in next_
+        // add the strands in `next_` to `cur_`, sorted by theoretical quality.
+        // Best quality first.
         cur_.clear();
+        if (v.rules().enabled(featureFlowSortStrands) && !next_.empty())
+        {
+            std::vector<std::pair<Quality, Strand const*>> strandQuals;
+            strandQuals.reserve(next_.size());
+            if (next_.size() > 1)  // no need to sort one strand
+            {
+                for (Strand const* strand : next_)
+                {
+                    if (!strand)
+                    {
+                        // should not happen
+                        continue;
+                    }
+                    if (auto const qual = qualityUpperBound(v, *strand))
+                    {
+                        if (limitQuality && *qual < *limitQuality)
+                        {
+                            // If a strand's quality is ever over limitQuality
+                            // it is no longer part of the candidate set. Note
+                            // that when transfer fees are charged, and an
+                            // account goes from redeeming to issuing then
+                            // strand quality _can_ increase; However, this is
+                            // an unusual corner case.
+                            continue;
+                        }
+                        strandQuals.push_back({*qual, strand});
+                    }
+                }
+                // must stable sort for deterministic order across different c++
+                // standard library implementations
+                std::stable_sort(
+                    strandQuals.begin(),
+                    strandQuals.end(),
+                    [](auto const& lhs, auto const& rhs) {
+                        // higher qualities first
+                        return std::get<Quality>(lhs) > std::get<Quality>(rhs);
+                    });
+                next_.clear();
+                next_.reserve(strandQuals.size());
+                for (auto const& sq : strandQuals)
+                {
+                    next_.push_back(std::get<Strand const*>(sq));
+                }
+            }
+        }
         std::swap(cur_, next_);
     }
 
+    Strand const*
+    get(size_t i) const
+    {
+        if (i >= cur_.size())
+        {
+            assert(0);
+            return nullptr;
+        }
+        return cur_[i];
+    }
+
     void
     push(Strand const* s)
     {
         next_.push_back(s);
     }
 
-    auto
+    // Push the strands from index i to the end of cur_ to next_
-    begin()
+    void
+    pushRemainingCurToNext(size_t i)
     {
-        return cur_.begin();
+        if (i >= cur_.size())
-    }
+            return;
-
+        next_.insert(next_.end(), std::next(cur_.begin(), i), cur_.end());
-    auto
-    end()
-    {
-        return cur_.end();
-    }
-
-    auto
-    begin() const
-    {
-        return cur_.begin();
-    }
-
-    auto
-    end() const
-    {
-        return cur_.end();
     }
 
     auto
@@ -422,7 +477,7 @@ public:
 /**
    Request `out` amount from a collection of strands
 
-   Attempt to fullfill the payment by using liquidity from the strands in order
+   Attempt to fulfill the payment by using liquidity from the strands in order
    from least expensive to most expensive
 
    @param baseView Trust lines and balances
@@ -478,6 +533,8 @@ flow(
 
     std::size_t const maxTries = 1000;
     std::size_t curTry = 0;
+    std::uint32_t maxOffersToConsider = 1500;
+    std::uint32_t offersConsidered = 0;
 
     // There is a bug in gcc that incorrectly warns about using uninitialized
     // values if `remainingIn` is initialized through a copy constructor. We can
@@ -526,7 +583,7 @@ flow(
             return {telFAILED_PROCESSING, std::move(ofrsToRmOnFail)};
         }
 
-        activeStrands.activateNext();
+        activeStrands.activateNext(sb, limitQuality);
 
         boost::container::flat_set<uint256> ofrsToRm;
         boost::optional<BestStrand> best;
@@ -537,8 +594,16 @@ flow(
         // offers Constructed as `false,0` to workaround a gcc warning about
         // uninitialized variables
         boost::optional<std::size_t> markInactiveOnUse{false, 0};
-        for (auto strand : activeStrands)
+        for (size_t strandIndex = 0, sie = activeStrands.size();
+             strandIndex != sie;
+             ++strandIndex)
         {
+            Strand const* strand = activeStrands.get(strandIndex);
+            if (!strand)
+            {
+                // should not happen
+                continue;
+            }
             if (offerCrossing && limitQuality)
             {
                 auto const strandQ = qualityUpperBound(sb, *strand);
@@ -551,6 +616,8 @@ flow(
             // rm bad offers even if the strand fails
             SetUnion(ofrsToRm, f.ofrsToRm);
 
+            offersConsidered += f.ofrsUsed;
+
             if (!f.success || f.out == beast::zero)
                 continue;
 
@@ -576,26 +643,46 @@ flow(
                 continue;
             }
 
+            if (baseView.rules().enabled(featureFlowSortStrands))
+            {
+                assert(!best);
+                if (!f.inactive)
+                    activeStrands.push(strand);
+                best.emplace(f.in, f.out, std::move(*f.sandbox), *strand, q);
+                activeStrands.pushRemainingCurToNext(strandIndex + 1);
+                break;
+            }
+
             activeStrands.push(strand);
 
             if (!best || best->quality < q ||
                 (best->quality == q && best->out < f.out))
             {
                 // If this strand is inactive (because it consumed too many
-                // offers) and ends up having the best quality, remove it from
+                // offers) and ends up having the best quality, remove it
-                // the activeStrands. If it doesn't end up having the best
+                // from the activeStrands. If it doesn't end up having the
-                // quality, keep it active.
+                // best quality, keep it active.
 
                 if (f.inactive)
+                {
+                    // This should be `nextSize`, not `size`. This issue is
+                    // fixed in featureFlowSortStrands.
                     markInactiveOnUse = activeStrands.size() - 1;
+                }
                 else
+                {
                     markInactiveOnUse.reset();
+                }
 
                 best.emplace(f.in, f.out, std::move(*f.sandbox), *strand, q);
             }
         }
 
-        bool const shouldBreak = !best;
+        bool const shouldBreak = [&] {
+            if (baseView.rules().enabled(featureFlowSortStrands))
+                return !best || offersConsidered >= maxOffersToConsider;
+            return !best;
+        }();
 
         if (best)
         {

src/ripple/protocol/Feature.h

@@ -113,7 +113,8 @@ class FeatureCollections
         "HardenedValidations",
         "fixAmendmentMajorityCalc",  // Fix Amendment majority calculation
         "NegativeUNL",
-        "TicketBatch"};
+        "TicketBatch",
+        "FlowSortStrands"};
 
     std::vector<uint256> features;
     boost::container::flat_map<uint256, std::size_t> featureToIndex;
@@ -371,6 +372,7 @@ extern uint256 const featureHardenedValidations;
 extern uint256 const fixAmendmentMajorityCalc;
 extern uint256 const featureNegativeUNL;
 extern uint256 const featureTicketBatch;
+extern uint256 const featureFlowSortStrands;
 
 }  // namespace ripple
 

src/ripple/protocol/impl/Feature.cpp

@@ -133,6 +133,7 @@ detail::supportedAmendments()
         "fixAmendmentMajorityCalc",
         //"NegativeUNL",      // Commented out to prevent automatic enablement
         "TicketBatch",
+        "FlowSortStrands",
     };
     return supported;
 }
@@ -186,7 +187,8 @@ uint256 const
     featureHardenedValidations      = *getRegisteredFeature("HardenedValidations"),
     fixAmendmentMajorityCalc        = *getRegisteredFeature("fixAmendmentMajorityCalc"),
     featureNegativeUNL              = *getRegisteredFeature("NegativeUNL"),
-    featureTicketBatch              = *getRegisteredFeature("TicketBatch");
+    featureTicketBatch              = *getRegisteredFeature("TicketBatch"),
+    featureFlowSortStrands          = *getRegisteredFeature("FlowSortStrands");
 
 // The following amendments have been active for at least two years. Their
 // pre-amendment code has been removed and the identifiers are deprecated.

src/test/app/CrossingLimits_test.cpp

@@ -311,6 +311,11 @@ public:
         // second test the strand does not have the best quality (the
         // implementation has to handle this case correct and not mark the
         // strand dry until the liquidity is actually used)
+
+        // The implementation allows any single step to consume at most 1000
+        // offers. With the `FlowSortStrands` feature enabled, if the total
+        // number of offers consumed by all the steps combined exceeds 1500, the
+        // payment stops.
         {
             Env env(*this, features);
 
@@ -324,7 +329,7 @@ public:
             // Notice the strand with the 800 unfunded offers has the initial
             // best quality
             n_offers(env, 2000, alice, EUR(2), XRP(1));
-            n_offers(env, 300, alice, XRP(1), USD(4));
+            n_offers(env, 100, alice, XRP(1), USD(4));
             n_offers(
                 env, 801, carol, XRP(1), USD(3));  // only one offer is funded
             n_offers(env, 1000, alice, XRP(1), USD(3));
@@ -334,7 +339,10 @@ public:
             // Bob offers to buy 2000 USD for 2000 EUR; He starts with 2000 EUR
             //  1. The best quality is the autobridged offers that take 2 EUR
             //  and give 4 USD.
-            //     Bob spends 600 EUR and receives 1200 USD.
+            //     Bob spends 200 EUR and receives 400 USD.
+            //     100 EUR->XRP offers consumed.
+            //     100 XRP->USD offers consumed.
+            //     200 total offers consumed.
             //
             //  2. The best quality is the autobridged offers that take 2 EUR
             //  and give 3 USD.
@@ -345,19 +353,27 @@ public:
             //        A book step is allowed to consume a maxium of 1000 offers
             //        at a given quality, and that limit is now reached.
             //     d. Now the strand is dry, even though there are still funded
-            //     XRP(1) to USD(3) offers available. Bob has spent 400 EUR and
+            //     XRP(1) to USD(3) offers available.
-            //     received 600 USD in this step. (200 funded offers consumed
+            //        Bob has spent 400 EUR and received 600 USD in this step.
-            //     800 unfunded offers)
+            //        200 EUR->XRP offers consumed
+            //        800 unfunded XRP->USD offers consumed
+            //        200 funded XRP->USD offers consumed (1 carol, 199 alice)
+            //        1400 total offers consumed so far (100 left before the
+            //        limit)
             //  3. The best is the non-autobridged offers that takes 500 EUR and
             //  gives 500 USD.
-            //     Bob has 2000 EUR, and has spent 600+400=1000 EUR. He has 1000
+            //     Bob started with 2000 EUR
-            //     left. Bob spent 500 EUR and receives 500 USD.
+            //     Bob spent 500 EUR (100+400)
-            // In total: Bob spent EUR(600 + 400 + 500) = EUR(1500). He started
+            //     Bob has 1500 EUR left
-            // with 2000 so has 500 remaining
+            //     In this step:
-            //           Bob received USD(1200 + 600 + 500) = USD(2300).
+            //     Bob spents 500 EUR and receives 500 USD.
-            //           Alice spent 300*4 + 199*3 + 500 = 2297 USD. She started
+            // In total:
-            //           with 4000 so has 1703 USD remaining. Alice received
+            //           Bob spent 1100 EUR (200 + 400 + 500)
-            //           600 + 400 + 500 = 1500 EUR
+            //           Bob has 900 EUR remaining (2000 - 1100)
+            //           Bob received 1500 USD (400 + 600 + 500)
+            //           Alice spent 1497 USD (100*4 + 199*3 + 500)
+            //           Alice has 2503 remaining (4000 - 1497)
+            //           Alice received 1100 EUR (200 + 400 + 500)
             env.trust(EUR(10000), bob);
             env.close();
             env(pay(gw, bob, EUR(2000)));
@@ -365,15 +381,15 @@ public:
             env(offer(bob, USD(4000), EUR(4000)));
             env.close();
 
-            env.require(balance(bob, USD(2300)));
+            env.require(balance(bob, USD(1500)));
-            env.require(balance(bob, EUR(500)));
+            env.require(balance(bob, EUR(900)));
             env.require(offers(bob, 1));
             env.require(owners(bob, 3));
 
-            env.require(balance(alice, USD(1703)));
+            env.require(balance(alice, USD(2503)));
-            env.require(balance(alice, EUR(1500)));
+            env.require(balance(alice, EUR(1100)));
             auto const numAOffers =
-                2000 + 300 + 1000 + 1 - (2 * 300 + 2 * 199 + 1 + 1);
+                2000 + 100 + 1000 + 1 - (2 * 100 + 2 * 199 + 1 + 1);
             env.require(offers(alice, numAOffers));
             env.require(owners(alice, numAOffers + 2));
 
@@ -393,7 +409,7 @@ public:
             // initial best quality
             n_offers(env, 1, alice, EUR(1), USD(10));
             n_offers(env, 2000, alice, EUR(2), XRP(1));
-            n_offers(env, 300, alice, XRP(1), USD(4));
+            n_offers(env, 100, alice, XRP(1), USD(4));
             n_offers(
                 env, 801, carol, XRP(1), USD(3));  // only one offer is funded
             n_offers(env, 1000, alice, XRP(1), USD(3));
@@ -407,7 +423,7 @@ public:
             //
             //  2. The best quality is the autobridged offers that takes 2 EUR
             //  and gives 4 USD.
-            //     Bob spends 600 EUR and receives 1200 USD.
+            //     Bob spends 200 EUR and receives 400 USD.
             //
             //  3. The best quality is the autobridged offers that takes 2 EUR
             //  and gives 3 USD.
@@ -423,14 +439,14 @@ public:
             //     800 unfunded offers)
             //  4. The best is the non-autobridged offers that takes 499 EUR and
             //  gives 499 USD.
-            //     Bob has 2000 EUR, and has spent 1+600+400=1001 EUR. He has
+            //     Bob has 2000 EUR, and has spent 1+200+400=601 EUR. He has
-            //     999 left. Bob spent 499 EUR and receives 499 USD.
+            //     1399 left. Bob spent 499 EUR and receives 499 USD.
-            // In total: Bob spent EUR(1 + 600 + 400 + 499) = EUR(1500). He
+            // In total: Bob spent EUR(1 + 200 + 400 + 499) = EUR(1100). He
-            // started with 2000 so has 500 remaining
+            // started with 2000 so has 900 remaining
-            //           Bob received USD(10 + 1200 + 600 + 499) = USD(2309).
+            //           Bob received USD(10 + 400 + 600 + 499) = USD(1509).
-            //           Alice spent 10 + 300*4 + 199*3 + 499 = 2306 USD. She
+            //           Alice spent 10 + 100*4 + 199*3 + 499 = 1506 USD. She
-            //           started with 4000 so has 1704 USD remaining. Alice
+            //           started with 4000 so has 2494 USD remaining. Alice
-            //           received 600 + 400 + 500 = 1500 EUR
+            //           received 200 + 400 + 500 = 1100 EUR
             env.trust(EUR(10000), bob);
             env.close();
             env(pay(gw, bob, EUR(2000)));
@@ -438,15 +454,15 @@ public:
             env(offer(bob, USD(4000), EUR(4000)));
             env.close();
 
-            env.require(balance(bob, USD(2309)));
+            env.require(balance(bob, USD(1509)));
-            env.require(balance(bob, EUR(500)));
+            env.require(balance(bob, EUR(900)));
             env.require(offers(bob, 1));
             env.require(owners(bob, 3));
 
-            env.require(balance(alice, USD(1694)));
+            env.require(balance(alice, USD(2494)));
-            env.require(balance(alice, EUR(1500)));
+            env.require(balance(alice, EUR(1100)));
             auto const numAOffers =
-                1 + 2000 + 300 + 1000 + 1 - (1 + 2 * 300 + 2 * 199 + 1 + 1);
+                1 + 2000 + 100 + 1000 + 1 - (1 + 2 * 100 + 2 * 199 + 1 + 1);
             env.require(offers(alice, numAOffers));
             env.require(owners(alice, numAOffers + 2));
 
@@ -506,6 +522,17 @@ public:
         // up a book with many offers. At each quality keep the number of offers
         // below the limit. However, if all the offers are consumed it would
         // create a tecOVERSIZE error.
+
+        // The featureFlowSortStrands introduces a way of tracking the total
+        // number of consumed offers; with this feature the transaction no
+        // longer fails with a tecOVERSIZE error.
+        // The implementation allows any single step to consume at most 1000
+        // offers. With the `FlowSortStrands` feature enabled, if the total
+        // number of offers consumed by all the steps combined exceeds 1500, the
+        // payment stops. Since the first set of offers consumes 998 offers, the
+        // second set will consume 998, which is not over the limit and the
+        // payment stops. So 2*998, or 1996 is the expected value when
+        // `FlowSortStrands` is enabled.
         n_offers(env, 998, alice, XRP(1.00), USD(1));
         n_offers(env, 998, alice, XRP(0.99), USD(1));
         n_offers(env, 998, alice, XRP(0.98), USD(1));
@@ -514,11 +541,26 @@ public:
         n_offers(env, 998, alice, XRP(0.95), USD(1));
 
         bool const withFlowCross = features[featureFlowCross];
-        env(offer(bob, USD(8000), XRP(8000)),
+        bool const withSortStrands = features[featureFlowSortStrands];
-            ter(withFlowCross ? TER{tecOVERSIZE} : tesSUCCESS));
+
+        auto const expectedTER = [&]() -> TER {
+            if (withFlowCross && !withSortStrands)
+                return TER{tecOVERSIZE};
+            return tesSUCCESS;
+        }();
+
+        env(offer(bob, USD(8000), XRP(8000)), ter(expectedTER));
         env.close();
 
-        env.require(balance(bob, USD(withFlowCross ? 0 : 850)));
+        auto const expectedUSD = [&] {
+            if (!withFlowCross)
+                return USD(850);
+            if (!withSortStrands)
+                return USD(0);
+            return USD(1996);
+        }();
+
+        env.require(balance(bob, expectedUSD));
     }
 
     void
@@ -533,8 +575,9 @@ public:
         };
         using namespace jtx;
         auto const sa = supported_amendments();
-        testAll(sa - featureFlowCross);
         testAll(sa);
+        testAll(sa - featureFlowSortStrands);
+        testAll(sa - featureFlowCross - featureFlowSortStrands);
     }
 };
 

src/test/app/Flow_test.cpp

@@ -392,13 +392,13 @@ struct Flow_test : public beast::unit_test::suite
             env(pay(gw, bob, EUR(50)));
 
             env(offer(bob, BTC(50), USD(50)));
-            env(offer(bob, BTC(60), EUR(50)));
+            env(offer(bob, BTC(40), EUR(50)));
             env(offer(bob, EUR(50), USD(50)));
 
             // unfund offer
             env(pay(bob, gw, EUR(50)));
             BEAST_EXPECT(isOffer(env, bob, BTC(50), USD(50)));
-            BEAST_EXPECT(isOffer(env, bob, BTC(60), EUR(50)));
+            BEAST_EXPECT(isOffer(env, bob, BTC(40), EUR(50)));
             BEAST_EXPECT(isOffer(env, bob, EUR(50), USD(50)));
 
             env(pay(alice, carol, USD(50)),
@@ -414,7 +414,7 @@ struct Flow_test : public beast::unit_test::suite
             // used in the payment
             BEAST_EXPECT(!isOffer(env, bob, BTC(50), USD(50)));
             // found unfunded
-            BEAST_EXPECT(!isOffer(env, bob, BTC(60), EUR(50)));
+            BEAST_EXPECT(!isOffer(env, bob, BTC(40), EUR(50)));
             // unfunded, but should not yet be found unfunded
             BEAST_EXPECT(isOffer(env, bob, EUR(50), USD(50)));
         }
@@ -435,17 +435,20 @@ struct Flow_test : public beast::unit_test::suite
             env.trust(EUR(1000), alice, bob, carol);
 
             env(pay(gw, alice, BTC(60)));
-            env(pay(gw, bob, USD(50)));
+            env(pay(gw, bob, USD(60)));
             env(pay(gw, bob, EUR(50)));
+            env(pay(gw, carol, EUR(1)));
 
             env(offer(bob, BTC(50), USD(50)));
             env(offer(bob, BTC(60), EUR(50)));
+            env(offer(carol, BTC(1000), EUR(1)));
             env(offer(bob, EUR(50), USD(50)));
 
             // unfund offer
             env(pay(bob, gw, EUR(50)));
             BEAST_EXPECT(isOffer(env, bob, BTC(50), USD(50)));
             BEAST_EXPECT(isOffer(env, bob, BTC(60), EUR(50)));
+            BEAST_EXPECT(isOffer(env, carol, BTC(1000), EUR(1)));
 
             auto flowJournal = env.app().logs().journal("Flow");
             auto const flowResult = [&] {
@@ -499,6 +502,7 @@ struct Flow_test : public beast::unit_test::suite
 
             // used in payment, but since payment failed should be untouched
             BEAST_EXPECT(isOffer(env, bob, BTC(50), USD(50)));
+            BEAST_EXPECT(isOffer(env, carol, BTC(1000), EUR(1)));
             // found unfunded
             BEAST_EXPECT(!isOffer(env, bob, BTC(60), EUR(50)));
         }

src/test/jtx/PathSet.h

@@ -27,6 +27,44 @@
 namespace ripple {
 namespace test {
 
+/** Count offer
+ */
+inline std::size_t
+countOffers(
+    jtx::Env& env,
+    jtx::Account const& account,
+    Issue const& takerPays,
+    Issue const& takerGets)
+{
+    size_t count = 0;
+    forEachItem(
+        *env.current(), account, [&](std::shared_ptr<SLE const> const& sle) {
+            if (sle->getType() == ltOFFER &&
+                sle->getFieldAmount(sfTakerPays).issue() == takerPays &&
+                sle->getFieldAmount(sfTakerGets).issue() == takerGets)
+                ++count;
+        });
+    return count;
+}
+
+inline std::size_t
+countOffers(
+    jtx::Env& env,
+    jtx::Account const& account,
+    STAmount const& takerPays,
+    STAmount const& takerGets)
+{
+    size_t count = 0;
+    forEachItem(
+        *env.current(), account, [&](std::shared_ptr<SLE const> const& sle) {
+            if (sle->getType() == ltOFFER &&
+                sle->getFieldAmount(sfTakerPays) == takerPays &&
+                sle->getFieldAmount(sfTakerGets) == takerGets)
+                ++count;
+        });
+    return count;
+}
+
 /** An offer exists
  */
 inline bool
@@ -36,15 +74,19 @@ isOffer(
     STAmount const& takerPays,
     STAmount const& takerGets)
 {
-    bool exists = false;
+    return countOffers(env, account, takerPays, takerGets) > 0;
-    forEachItem(
+}
-        *env.current(), account, [&](std::shared_ptr<SLE const> const& sle) {
+
-            if (sle->getType() == ltOFFER &&
+/** An offer exists
-                sle->getFieldAmount(sfTakerPays) == takerPays &&
+ */
-                sle->getFieldAmount(sfTakerGets) == takerGets)
+inline bool
-                exists = true;
+isOffer(
-        });
+    jtx::Env& env,
-    return exists;
+    jtx::Account const& account,
+    Issue const& takerPays,
+    Issue const& takerGets)
+{
+    return countOffers(env, account, takerPays, takerGets) > 0;
 }
 
 class Path