From fe129e8e4f6db98000cf4fde580a0324b060351a Mon Sep 17 00:00:00 2001 From: seelabs Date: Thu, 25 Jun 2020 08:13:49 -0700 Subject: [PATCH] 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 | 20 +++ src/ripple/app/paths/impl/Steps.h | 25 ++++ src/ripple/app/paths/impl/StrandFlow.h | 169 +++++++++++++++++++------ src/ripple/protocol/Feature.h | 4 +- src/ripple/protocol/impl/Feature.cpp | 4 +- src/test/app/CrossingLimits_test.cpp | 117 +++++++++++------ src/test/app/Flow_test.cpp | 12 +- src/test/jtx/PathSet.h | 60 +++++++-- 8 files changed, 318 insertions(+), 93 deletions(-) diff --git a/src/ripple/app/paths/impl/BookStep.cpp b/src/ripple/app/paths/impl/BookStep.cpp index ee8e8edcd..dfb404116 100644 --- a/src/ripple/app/paths/impl/BookStep.cpp +++ b/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 revImp( PaymentSandbox& sb, @@ -480,6 +491,13 @@ BookStep::qualityUpperBound( return {q, dir}; } +template +std::uint32_t +BookStep::offersUsed() const +{ + return offersUsed_; +} + // Adjust the offer amount and step amount subject to the given input limit template static void @@ -779,6 +797,7 @@ BookStep::revImp( auto const r = forEachOffer(sb, afView, prevStepDebtDir, eachOffer); boost::container::flat_set 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::fwdImp( auto const r = forEachOffer(sb, afView, prevStepDebtDir, eachOffer); boost::container::flat_set toRm = std::move(std::get<0>(r)); std::uint32_t const offersConsumed = std::get<1>(r); + offersUsed_ = offersConsumed; SetUnion(ofrsToRm, toRm); if (offersConsumed >= maxOffersToConsume_) diff --git a/src/ripple/app/paths/impl/Steps.h b/src/ripple/app/paths/impl/Steps.h index 8860ddbc9..552161dc1 100644 --- a/src/ripple/app/paths/impl/Steps.h +++ b/src/ripple/app/paths/impl/Steps.h @@ -188,6 +188,19 @@ public: virtual std::pair, 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>; + +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 diff --git a/src/ripple/app/paths/impl/StrandFlow.h b/src/ripple/app/paths/impl/StrandFlow.h index 539803c63..8e2663231 100644 --- a/src/ripple/app/paths/impl/StrandFlow.h +++ b/src/ripple/app/paths/impl/StrandFlow.h @@ -29,6 +29,7 @@ #include #include #include +#include #include @@ -48,6 +49,9 @@ struct StrandResult TOutAmt out = beast::zero; ///< Currency amount out boost::optional sandbox; ///< Resulting Sandbox state boost::container::flat_set 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 ofrsToRm_) - : success(false), ofrsToRm(std::move(ofrsToRm_)) + StrandResult( + Strand const& strand, + boost::container::flat_set ofrsToRm_) + : success(false) + , ofrsToRm(std::move(ofrsToRm_)) + , ofrsUsed(offersUsed(strand)) { } }; @@ -108,7 +118,7 @@ flow( if (isDirectXrpToXrp(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(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(r.first) != *maxIn) { @@ -160,7 +170,7 @@ flow( << to_string(get(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 const& step) { return step->inactive(); }); return Result( + strand, get(strandIn), get(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 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> 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(lhs) > std::get(rhs); + }); + next_.clear(); + next_.reserve(strandQuals.size()); + for (auto const& sq : strandQuals) + { + next_.push_back(std::get(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 - begin() + // Push the strands from index i to the end of cur_ to next_ + void + pushRemainingCurToNext(size_t i) { - return cur_.begin(); - } - - auto - end() - { - return cur_.end(); - } - - auto - begin() const - { - return cur_.begin(); - } - - auto - end() const - { - return cur_.end(); + if (i >= cur_.size()) + return; + next_.insert(next_.end(), std::next(cur_.begin(), i), 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 ofrsToRm; boost::optional best; @@ -537,8 +594,16 @@ flow( // offers Constructed as `false,0` to workaround a gcc warning about // uninitialized variables boost::optional 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 - // the activeStrands. If it doesn't end up having the best - // quality, keep it active. + // offers) and ends up having the best quality, remove it + // from the activeStrands. If it doesn't end up having the + // 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) { diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 5f9970423..cb7a1c06a 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -113,7 +113,8 @@ class FeatureCollections "HardenedValidations", "fixAmendmentMajorityCalc", // Fix Amendment majority calculation "NegativeUNL", - "TicketBatch"}; + "TicketBatch", + "FlowSortStrands"}; std::vector features; boost::container::flat_map 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 diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index c850ed07f..17386f7a5 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/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. diff --git a/src/test/app/CrossingLimits_test.cpp b/src/test/app/CrossingLimits_test.cpp index c4515e916..a6a460652 100644 --- a/src/test/app/CrossingLimits_test.cpp +++ b/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 - // received 600 USD in this step. (200 funded offers consumed - // 800 unfunded offers) + // XRP(1) to USD(3) offers available. + // Bob has spent 400 EUR and received 600 USD in this step. + // 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 - // left. Bob spent 500 EUR and receives 500 USD. - // In total: Bob spent EUR(600 + 400 + 500) = EUR(1500). He started - // with 2000 so has 500 remaining - // Bob received USD(1200 + 600 + 500) = USD(2300). - // Alice spent 300*4 + 199*3 + 500 = 2297 USD. She started - // with 4000 so has 1703 USD remaining. Alice received - // 600 + 400 + 500 = 1500 EUR + // Bob started with 2000 EUR + // Bob spent 500 EUR (100+400) + // Bob has 1500 EUR left + // In this step: + // Bob spents 500 EUR and receives 500 USD. + // In total: + // Bob spent 1100 EUR (200 + 400 + 500) + // 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, EUR(500))); + env.require(balance(bob, USD(1500))); + 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, EUR(1500))); + env.require(balance(alice, USD(2503))); + 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 - // 999 left. Bob spent 499 EUR and receives 499 USD. - // In total: Bob spent EUR(1 + 600 + 400 + 499) = EUR(1500). He - // started with 2000 so has 500 remaining - // Bob received USD(10 + 1200 + 600 + 499) = USD(2309). - // Alice spent 10 + 300*4 + 199*3 + 499 = 2306 USD. She - // started with 4000 so has 1704 USD remaining. Alice - // received 600 + 400 + 500 = 1500 EUR + // Bob has 2000 EUR, and has spent 1+200+400=601 EUR. He has + // 1399 left. Bob spent 499 EUR and receives 499 USD. + // In total: Bob spent EUR(1 + 200 + 400 + 499) = EUR(1100). He + // started with 2000 so has 900 remaining + // Bob received USD(10 + 400 + 600 + 499) = USD(1509). + // Alice spent 10 + 100*4 + 199*3 + 499 = 1506 USD. She + // started with 4000 so has 2494 USD remaining. Alice + // 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, EUR(500))); + env.require(balance(bob, USD(1509))); + 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, EUR(1500))); + env.require(balance(alice, USD(2494))); + 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)), - ter(withFlowCross ? TER{tecOVERSIZE} : tesSUCCESS)); + bool const withSortStrands = features[featureFlowSortStrands]; + + 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); } }; diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp index 273f76690..5023a1cab 100644 --- a/src/test/app/Flow_test.cpp +++ b/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))); } diff --git a/src/test/jtx/PathSet.h b/src/test/jtx/PathSet.h index df3c46893..f578ceb19 100644 --- a/src/test/jtx/PathSet.h +++ b/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 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 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; - forEachItem( - *env.current(), account, [&](std::shared_ptr const& sle) { - if (sle->getType() == ltOFFER && - sle->getFieldAmount(sfTakerPays) == takerPays && - sle->getFieldAmount(sfTakerGets) == takerGets) - exists = true; - }); - return exists; + return countOffers(env, account, takerPays, takerGets) > 0; +} + +/** An offer exists + */ +inline bool +isOffer( + jtx::Env& env, + jtx::Account const& account, + Issue const& takerPays, + Issue const& takerGets) +{ + return countOffers(env, account, takerPays, takerGets) > 0; } class Path