From ef3dc5bb58a43235adbc7b4e053ef0dea7617c12 Mon Sep 17 00:00:00 2001 From: seelabs Date: Thu, 24 Mar 2016 11:08:46 -0400 Subject: [PATCH] Return unfunded and expired offers when flow fails: Payments do not remove unfunded and expired offers when a payment fails. However, offer crossing is now using the payment engine and needs to know what offers were found in a removable state, even on failure. --- src/ripple/app/paths/Flow.cpp | 98 +++++++-------- src/ripple/app/paths/RippleCalc.cpp | 8 +- src/ripple/app/paths/RippleCalc.h | 10 +- src/ripple/app/paths/impl/BookStep.cpp | 28 ++--- src/ripple/app/paths/impl/DirectStep.cpp | 12 +- src/ripple/app/paths/impl/Steps.h | 8 +- src/ripple/app/paths/impl/StrandFlow.h | 58 ++++++--- src/ripple/app/paths/impl/XRPEndpointStep.cpp | 9 +- src/ripple/app/tests/Flow_test.cpp | 117 ++++++++++++++---- src/ripple/app/tx/impl/OfferStream.cpp | 2 +- src/ripple/app/tx/impl/OfferStream.h | 6 +- 11 files changed, 229 insertions(+), 127 deletions(-) diff --git a/src/ripple/app/paths/Flow.cpp b/src/ripple/app/paths/Flow.cpp index 10be83d99..15b285699 100644 --- a/src/ripple/app/paths/Flow.cpp +++ b/src/ripple/app/paths/Flow.cpp @@ -27,11 +27,32 @@ #include #include +#include + #include #include namespace ripple { +template +static +auto finishFlow (PaymentSandbox& sb, + Issue const& srcIssue, Issue const& dstIssue, + FlowResult&& f) +{ + path::RippleCalc::Output result; + if (f.ter == tesSUCCESS) + f.sandbox->apply (sb); + else + result.removableOffers = std::move (f.removableOffers); + + result.setResult (f.ter); + result.actualAmountIn = toSTAmount (f.in, srcIssue); + result.actualAmountOut = toSTAmount (f.out, dstIssue); + + return result; +}; + path::RippleCalc::Output flow ( PaymentSandbox& sb, @@ -45,7 +66,6 @@ flow ( boost::optional const& sendMax, beast::Journal j) { - path::RippleCalc::Output result; Issue const srcIssue = sendMax ? sendMax->issue () : Issue (deliver.issue ().currency, src); @@ -63,6 +83,7 @@ flow ( if (sr.first != tesSUCCESS) { + path::RippleCalc::Output result; result.setResult (sr.first); return result; } @@ -88,67 +109,40 @@ flow ( const bool dstIsXRP = isXRP (dstIssue.currency); auto const asDeliver = toAmountSpec (deliver); - boost::optional strandSB; // The src account may send either xrp or iou. The dst account may receive // either xrp or iou. Since XRP and IOU amounts are represented by different // types, use templates to tell `flow` about the amount types. if (srcIsXRP && dstIsXRP) { - auto f = flow ( - sb, strands, asDeliver.xrp, defaultPaths, partialPayment, limitQuality, sendMax, j); - - if (f.ter == tesSUCCESS) - strandSB.emplace (std::move (*f.sandbox)); - - result.setResult (f.ter); - result.actualAmountIn = toSTAmount (f.in); - result.actualAmountOut = toSTAmount (f.out); + return finishFlow (sb, srcIssue, dstIssue, + flow ( + sb, strands, asDeliver.xrp, defaultPaths, partialPayment, + limitQuality, sendMax, j)); } - else if (srcIsXRP && !dstIsXRP) + + if (srcIsXRP && !dstIsXRP) { - auto f = flow ( + return finishFlow (sb, srcIssue, dstIssue, + flow ( + sb, strands, asDeliver.iou, defaultPaths, partialPayment, + limitQuality, sendMax, j)); + } + + if (!srcIsXRP && dstIsXRP) + { + return finishFlow (sb, srcIssue, dstIssue, + flow ( + sb, strands, asDeliver.xrp, defaultPaths, partialPayment, + limitQuality, sendMax, j)); + } + + assert (!srcIsXRP && !dstIsXRP); + return finishFlow (sb, srcIssue, dstIssue, + flow ( sb, strands, asDeliver.iou, defaultPaths, partialPayment, - limitQuality, sendMax, j); + limitQuality, sendMax, j)); - if (f.ter == tesSUCCESS) - strandSB.emplace (std::move (*f.sandbox)); - - result.setResult (f.ter); - result.actualAmountIn = toSTAmount (f.in); - result.actualAmountOut = toSTAmount (f.out, dstIssue); - } - else if (!srcIsXRP && dstIsXRP) - { - auto f = flow ( - sb, strands, asDeliver.xrp, defaultPaths, partialPayment, - limitQuality, sendMax, j); - - if (f.ter == tesSUCCESS) - strandSB.emplace (std::move (*f.sandbox)); - - result.setResult (f.ter); - result.actualAmountIn = toSTAmount (f.in, srcIssue); - result.actualAmountOut = toSTAmount (f.out); - } - else if (!srcIsXRP && !dstIsXRP) - { - auto f = flow ( - sb, strands, asDeliver.iou, defaultPaths, partialPayment, - limitQuality, sendMax, j); - - if (f.ter == tesSUCCESS) - strandSB.emplace (std::move (*f.sandbox)); - - result.setResult (f.ter); - result.actualAmountIn = toSTAmount (f.in, srcIssue); - result.actualAmountOut = toSTAmount (f.out, dstIssue); - } - - // strandSB is only valid when flow was successful - if (strandSB) - strandSB->apply (sb); - return result; } } // ripple diff --git a/src/ripple/app/paths/RippleCalc.cpp b/src/ripple/app/paths/RippleCalc.cpp index 2a20b87c9..1a65573ef 100644 --- a/src/ripple/app/paths/RippleCalc.cpp +++ b/src/ripple/app/paths/RippleCalc.cpp @@ -33,7 +33,7 @@ namespace path { static TER deleteOffers (ApplyView& view, - std::set const& offers, beast::Journal j) + boost::container::flat_set const& offers, beast::Journal j) { for (auto& e: offers) if (TER r = offerDelete (view, @@ -100,6 +100,8 @@ RippleCalc::Output RippleCalc::rippleCalculate ( flowV1Out.setResult (result); flowV1Out.actualAmountIn = rc.actualAmountIn_; flowV1Out.actualAmountOut = rc.actualAmountOut_; + if (result != tesSUCCESS && !rc.permanentlyUnfundedOffers_.empty ()) + flowV1Out.removableOffers = std::move (rc.permanentlyUnfundedOffers_); } Output flowV2Out; @@ -139,7 +141,7 @@ RippleCalc::Output RippleCalc::rippleCalculate ( { JLOG (j.trace()) << "Exception from flow" << e.what (); if (!useFlowV1Output) - throw; + Throw(); } if (callFlowV2 && callFlowV1 && @@ -281,7 +283,7 @@ TER RippleCalc::rippleCalculate () getRate (saDstAmountReq_, saMaxAmountReq_) : 0; // Offers that became unfunded. - std::set unfundedOffersFromBestPaths; + boost::container::flat_set unfundedOffersFromBestPaths; int iPass = 0; diff --git a/src/ripple/app/paths/RippleCalc.h b/src/ripple/app/paths/RippleCalc.h index 380e84f36..ff235646e 100644 --- a/src/ripple/app/paths/RippleCalc.h +++ b/src/ripple/app/paths/RippleCalc.h @@ -26,6 +26,8 @@ #include #include +#include + namespace ripple { class Config; namespace path { @@ -53,6 +55,12 @@ public: // The computed output amount. STAmount actualAmountOut; + // Collection of offers found expired or unfunded. When a payment + // succeeds, unfunded and expired offers are removed. When a payment + // fails, they are not removed. This vector contains the offers that + // could have been removed but were not because the payment fails. It is + // useful for offer crossing, which does remove the offers. + boost::container::flat_set removableOffers; private: TER calculationResult_ = temUNKNOWN; @@ -105,7 +113,7 @@ public: // unfunded offers in a deterministic order (hence the ordered container). // // Offers that were found unfunded. - std::set permanentlyUnfundedOffers_; + boost::container::flat_set permanentlyUnfundedOffers_; // First time working in reverse a funding source was mentioned. Source may // only be used there. diff --git a/src/ripple/app/paths/impl/BookStep.cpp b/src/ripple/app/paths/impl/BookStep.cpp index fc4efad2c..d8968cb0c 100644 --- a/src/ripple/app/paths/impl/BookStep.cpp +++ b/src/ripple/app/paths/impl/BookStep.cpp @@ -31,6 +31,8 @@ #include #include +#include + #include #include @@ -104,14 +106,14 @@ public: revImp ( PaymentSandbox& sb, ApplyView& afView, - std::vector& ofrsToRm, + boost::container::flat_set& ofrsToRm, TOut const& out); std::pair fwdImp ( PaymentSandbox& sb, ApplyView& afView, - std::vector& ofrsToRm, + boost::container::flat_set& ofrsToRm, TIn const& in); std::pair @@ -210,7 +212,7 @@ void limitStepOut (Quality const& ofrQ, */ template static -std::pair, std::uint32_t> +std::pair, std::uint32_t> forEachOffer ( PaymentSandbox& sb, ApplyView& afView, @@ -300,7 +302,7 @@ std::pair BookStep::revImp ( PaymentSandbox& sb, ApplyView& afView, - std::vector& ofrsToRm, + boost::container::flat_set& ofrsToRm, TOut const& out) { cache_.reset (); @@ -358,11 +360,10 @@ BookStep::revImp ( auto const r = forEachOffer ( sb, afView, book_, strandSrc_, strandDst_, eachOffer, maxOffersToConsume_, j_); - std::vector toRm = std::move(std::get<0>(r)); + boost::container::flat_set toRm = std::move(std::get<0>(r)); std::uint32_t const offersConsumed = std::get<1>(r); - ofrsToRm.reserve (ofrsToRm.size () + toRm.size ()); - for (auto& o : toRm) - ofrsToRm.emplace_back (std::move (o)); + ofrsToRm.insert (boost::container::ordered_unique_range_t{}, + toRm.begin (), toRm.end ()); if (offersConsumed >= maxOffersToConsume_) { @@ -400,7 +401,7 @@ std::pair BookStep::fwdImp ( PaymentSandbox& sb, ApplyView& afView, - std::vector& ofrsToRm, + boost::container::flat_set& ofrsToRm, TIn const& in) { assert(cache_); @@ -456,11 +457,10 @@ BookStep::fwdImp ( auto const r = forEachOffer ( sb, afView, book_, strandSrc_, strandDst_, eachOffer, maxOffersToConsume_, j_); - std::vector toRm = std::move(std::get<0>(r)); + boost::container::flat_set toRm = std::move(std::get<0>(r)); std::uint32_t const offersConsumed = std::get<1>(r); - ofrsToRm.reserve (ofrsToRm.size () + toRm.size ()); - for (auto& o : toRm) - ofrsToRm.emplace_back (std::move (o)); + ofrsToRm.insert (boost::container::ordered_unique_range_t{}, + toRm.begin (), toRm.end ()); if (offersConsumed >= maxOffersToConsume_) { @@ -510,7 +510,7 @@ BookStep::validFwd ( try { - std::vector dummy; + boost::container::flat_set dummy; fwdImp (sb, afView, dummy, get (in)); // changes cache } catch (FlowException const&) diff --git a/src/ripple/app/paths/impl/DirectStep.cpp b/src/ripple/app/paths/impl/DirectStep.cpp index 36277841c..efb77e208 100644 --- a/src/ripple/app/paths/impl/DirectStep.cpp +++ b/src/ripple/app/paths/impl/DirectStep.cpp @@ -26,6 +26,8 @@ #include #include +#include + #include #include @@ -111,14 +113,14 @@ class DirectStepI : public StepImp revImp ( PaymentSandbox& sb, ApplyView& afView, - std::vector& ofrsToRm, + boost::container::flat_set& ofrsToRm, IOUAmount const& out); std::pair fwdImp ( PaymentSandbox& sb, ApplyView& afView, - std::vector& ofrsToRm, + boost::container::flat_set& ofrsToRm, IOUAmount const& in); std::pair @@ -195,7 +197,7 @@ std::pair DirectStepI::revImp ( PaymentSandbox& sb, ApplyView& /*afView*/, - std::vector& /*ofrsToRm*/, + boost::container::flat_set& /*ofrsToRm*/, IOUAmount const& out) { cache_.reset (); @@ -313,7 +315,7 @@ std::pair DirectStepI::fwdImp ( PaymentSandbox& sb, ApplyView& /*afView*/, - std::vector& /*ofrsToRm*/, + boost::container::flat_set& /*ofrsToRm*/, IOUAmount const& in) { assert (cache_); @@ -409,7 +411,7 @@ DirectStepI::validFwd ( try { - std::vector dummy; + boost::container::flat_set dummy; fwdImp (sb, afView, dummy, in.iou); // changes cache } catch (FlowException const&) diff --git a/src/ripple/app/paths/impl/Steps.h b/src/ripple/app/paths/impl/Steps.h index e18cc700d..7de6680d2 100644 --- a/src/ripple/app/paths/impl/Steps.h +++ b/src/ripple/app/paths/impl/Steps.h @@ -79,7 +79,7 @@ class Step rev ( PaymentSandbox& sb, ApplyView& afView, - std::vector& ofrsToRm, + boost::container::flat_set& ofrsToRm, EitherAmount const& out) = 0; /** @@ -98,7 +98,7 @@ class Step fwd ( PaymentSandbox&, ApplyView& afView, - std::vector& ofrsToRm, + boost::container::flat_set& ofrsToRm, EitherAmount const& in) = 0; virtual @@ -238,7 +238,7 @@ struct StepImp : public Step rev ( PaymentSandbox& sb, ApplyView& afView, - std::vector& ofrsToRm, + boost::container::flat_set& ofrsToRm, EitherAmount const& out) override { auto const r = @@ -252,7 +252,7 @@ struct StepImp : public Step fwd ( PaymentSandbox& sb, ApplyView& afView, - std::vector& ofrsToRm, + boost::container::flat_set& ofrsToRm, EitherAmount const& in) override { auto const r = diff --git a/src/ripple/app/paths/impl/StrandFlow.h b/src/ripple/app/paths/impl/StrandFlow.h index 41387b234..5d1b72d2c 100644 --- a/src/ripple/app/paths/impl/StrandFlow.h +++ b/src/ripple/app/paths/impl/StrandFlow.h @@ -31,6 +31,8 @@ #include +#include +#include #include #include @@ -43,14 +45,14 @@ struct StrandResult TInAmt in = beast::zero; TOutAmt out = beast::zero; boost::optional sandbox; - std::vector ofrsToRm; // offers to remove + boost::container::flat_set ofrsToRm; // offers to remove StrandResult () = default; StrandResult (TInAmt const& in_, TOutAmt const& out_, PaymentSandbox&& sandbox_, - std::vector ofrsToRm_) + boost::container::flat_set ofrsToRm_) : ter (tesSUCCESS) , in (in_) , out (out_) @@ -59,7 +61,7 @@ struct StrandResult { } - StrandResult (TER ter_, std::vector ofrsToRm_) + StrandResult (TER ter_, boost::container::flat_set ofrsToRm_) : ter (ter_), ofrsToRm (std::move (ofrsToRm_)) { } @@ -92,7 +94,7 @@ flow ( return {}; } - std::vector ofrsToRm; + boost::container::flat_set ofrsToRm; if (isDirectXrpToXrp (strand)) { @@ -222,28 +224,36 @@ struct FlowResult TInAmt in = beast::zero; TOutAmt out = beast::zero; boost::optional sandbox; + boost::container::flat_set removableOffers; TER ter = temUNKNOWN; FlowResult () = default; FlowResult (TInAmt const& in_, TOutAmt const& out_, - PaymentSandbox&& sandbox_) + PaymentSandbox&& sandbox_, + boost::container::flat_set ofrsToRm) : in (in_) , out (out_) , sandbox (std::move (sandbox_)) + , removableOffers(std::move (ofrsToRm)) , ter (tesSUCCESS) { } - FlowResult (TER ter_) - : ter (ter_) + FlowResult (TER ter_, boost::container::flat_set ofrsToRm) + : removableOffers(std::move (ofrsToRm)) + , ter (ter_) { } - FlowResult (TER ter_, TInAmt const& in_, TOutAmt const& out_) + FlowResult (TER ter_, + TInAmt const& in_, + TOutAmt const& out_, + boost::container::flat_set ofrsToRm) : in (in_) , out (out_) + , removableOffers (std::move (ofrsToRm)) , ter (ter_) { } @@ -395,18 +405,22 @@ flow (PaymentSandbox const& baseView, return std::accumulate (col.begin () + 1, col.end (), *col.begin ()); }; + // These offers only need to be removed if the payment is not + // successful + boost::container::flat_set ofrsToRmOnFail; + while (remainingOut > beast::zero && (!remainingIn || *remainingIn > beast::zero)) { ++curTry; if (curTry >= maxTries) { - return Result (telFAILED_PROCESSING); + return {telFAILED_PROCESSING, std::move(ofrsToRmOnFail)}; } activeStrands.activateNext(); - std::set ofrsToRm; + boost::container::flat_set ofrsToRm; boost::optional best; for (auto strand : activeStrands) { @@ -414,7 +428,8 @@ flow (PaymentSandbox const& baseView, sb, *strand, remainingIn, remainingOut, j); // rm bad offers even if the strand fails - ofrsToRm.insert (f.ofrsToRm.begin (), f.ofrsToRm.end ()); + ofrsToRm.insert (boost::container::ordered_unique_range_t{}, + f.ofrsToRm.begin (), f.ofrsToRm.end ()); if (f.ter != tesSUCCESS || f.out == beast::zero) continue; @@ -469,9 +484,16 @@ flow (PaymentSandbox const& baseView, best.reset (); // view in best must be destroyed before modifying base // view - for (auto const& o : ofrsToRm) - if (auto ok = sb.peek (keylet::offer (o))) - offerDelete (sb, ok, j); + if (!ofrsToRm.empty ()) + { + ofrsToRmOnFail.insert (boost::container::ordered_unique_range_t{}, + ofrsToRm.begin (), ofrsToRm.end ()); + for (auto const& o : ofrsToRm) + { + if (auto ok = sb.peek (keylet::offer (o))) + offerDelete (sb, ok, j); + } + } if (shouldBreak) break; @@ -489,19 +511,19 @@ flow (PaymentSandbox const& baseView, if (actualOut > outReq) { assert (0); - return {tefEXCEPTION}; + return {tefEXCEPTION, std::move(ofrsToRmOnFail)}; } if (!partialPayment) { - return {tecPATH_PARTIAL, actualIn, actualOut}; + return {tecPATH_PARTIAL, actualIn, actualOut, std::move(ofrsToRmOnFail)}; } else if (actualOut == beast::zero) { - return {tecPATH_DRY}; + return {tecPATH_DRY, std::move(ofrsToRmOnFail)}; } } - return Result (actualIn, actualOut, std::move (sb)); + return Result (actualIn, actualOut, std::move (sb), std::move(ofrsToRmOnFail)); } } // ripple diff --git a/src/ripple/app/paths/impl/XRPEndpointStep.cpp b/src/ripple/app/paths/impl/XRPEndpointStep.cpp index 94459bb2a..64b685284 100644 --- a/src/ripple/app/paths/impl/XRPEndpointStep.cpp +++ b/src/ripple/app/paths/impl/XRPEndpointStep.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #include @@ -83,14 +84,14 @@ class XRPEndpointStep : public StepImp revImp ( PaymentSandbox& sb, ApplyView& afView, - std::vector& ofrsToRm, + boost::container::flat_set& ofrsToRm, XRPAmount const& out); std::pair fwdImp ( PaymentSandbox& sb, ApplyView& afView, - std::vector& ofrsToRm, + boost::container::flat_set& ofrsToRm, XRPAmount const& in); std::pair @@ -154,7 +155,7 @@ std::pair XRPEndpointStep::revImp ( PaymentSandbox& sb, ApplyView& afView, - std::vector& ofrsToRm, + boost::container::flat_set& ofrsToRm, XRPAmount const& out) { auto const balance = xrpLiquid (sb, acc_); @@ -174,7 +175,7 @@ std::pair XRPEndpointStep::fwdImp ( PaymentSandbox& sb, ApplyView& afView, - std::vector& ofrsToRm, + boost::container::flat_set& ofrsToRm, XRPAmount const& in) { assert (cache_); diff --git a/src/ripple/app/tests/Flow_test.cpp b/src/ripple/app/tests/Flow_test.cpp index 86f9807b7..045358d15 100644 --- a/src/ripple/app/tests/Flow_test.cpp +++ b/src/ripple/app/tests/Flow_test.cpp @@ -17,9 +17,12 @@ #include #include +#include #include #include #include +#include +#include #include #include #include @@ -132,6 +135,28 @@ bool equal (Strand const& strand, Args&&... args) struct Flow_test : public beast::unit_test::suite { + // Account path element + static auto APE(AccountID const& a) + { + return STPathElement ( + STPathElement::typeAccount, a, xrpCurrency (), xrpAccount ()); + }; + + // Issue path element + static auto IPE(Issue const& iss) + { + return STPathElement ( + STPathElement::typeCurrency | STPathElement::typeIssuer, + xrpAccount (), iss.currency, iss.account); + }; + + // Currency path element + static auto CPE(Currency const& c) + { + return STPathElement ( + STPathElement::typeCurrency, xrpAccount (), c, xrpAccount ()); + }; + void testToStrand () { testcase ("To Strand"); @@ -152,28 +177,6 @@ struct Flow_test : public beast::unit_test::suite using B = ripple::Book; using XRPS = XRPEndpointStepInfo; - // Account path element - auto APE = [](AccountID const& a) - { - return STPathElement ( - STPathElement::typeAccount, a, xrpCurrency (), xrpAccount ()); - }; - - // Issue path element - auto IPE = [](Issue const& iss) - { - return STPathElement ( - STPathElement::typeCurrency | STPathElement::typeIssuer, - xrpAccount (), iss.currency, iss.account); - }; - - // Currency path element - auto CPE = [](Currency const& c) - { - return STPathElement ( - STPathElement::typeCurrency, xrpAccount (), c, xrpAccount ()); - }; - auto test = [&, this](jtx::Env& env, Issue const& deliver, boost::optional const& sendMaxIssue, STPath const& path, TER expTer, auto&&... expSteps) @@ -556,7 +559,7 @@ struct Flow_test : public beast::unit_test::suite expect (!isOffer (env, bob, USD (50), XRP (50))); } { - // test unfunded offers are removed + // test unfunded offers are removed when payment succeeds Env env (*this, features(featureFlowV2)); env.fund (XRP (10000), alice, bob, carol, gw); @@ -594,6 +597,74 @@ struct Flow_test : public beast::unit_test::suite // unfunded, but should not yet be found unfunded expect (isOffer (env, bob, EUR (50), USD (50))); } + { + // test unfunded offers are returned when the payment fails. + // bob makes two offers: a funded 50 USD for 50 BTC and an unfunded 50 + // EUR for 60 BTC. alice pays carol 61 USD with 61 BTC. alice only + // has 60 BTC, so the payment will fail. The payment uses two paths: + // one through bob's funded offer and one through his unfunded + // offer. When the payment fails `flow` should return the unfunded + // offer. This test is intentionally similar to the one that removes + // unfunded offers when the payment succeeds. + Env env (*this, features(featureFlowV2)); + + env.fund (XRP (10000), alice, bob, carol, gw); + env.trust (USD (1000), alice, bob, carol); + env.trust (BTC (1000), alice, bob, carol); + env.trust (EUR (1000), alice, bob, carol); + + env (pay (gw, alice, BTC (60))); + env (pay (gw, bob, USD (50))); + env (pay (gw, bob, EUR (50))); + + env (offer (bob, BTC (50), USD (50))); + env (offer (bob, BTC (60), EUR (50))); + env (offer (bob, EUR (50), USD (50))); + + // unfund offer + env (pay (bob, gw, EUR (50))); + expect (isOffer (env, bob, BTC (50), USD (50))); + expect (isOffer (env, bob, BTC (60), EUR (50))); + + auto flowJournal = env.app ().logs ().journal ("Flow"); + auto const flowResult = [&] + { + STAmount deliver (USD (51)); + STAmount smax (BTC (61)); + PaymentSandbox sb (env.current ().get (), tapNONE); + STPathSet paths; + { + // BTC -> USD + STPath p1 ({IPE (USD.issue ())}); + paths.push_back (p1); + // BTC -> EUR -> USD + STPath p2 ({IPE (EUR.issue ()), IPE (USD.issue ())}); + paths.push_back (p2); + } + + return flow (sb, deliver, alice, carol, paths, false, false, + boost::none, smax, flowJournal); + }(); + + expect (flowResult.removableOffers.size () == 1); + env.app ().openLedger ().modify ( + [&](OpenView& view, beast::Journal j) + { + if (flowResult.removableOffers.empty()) + return false; + Sandbox sb (&view, tapNONE); + for (auto const& o : flowResult.removableOffers) + if (auto ok = sb.peek (keylet::offer (o))) + offerDelete (sb, ok, flowJournal); + sb.apply (view); + return true; + }); + + // used in payment, but since payment failed should be untouched + expect (isOffer (env, bob, BTC (50), USD (50))); + // found unfunded + expect (!isOffer (env, bob, BTC (60), EUR (50))); + } } void testTransferRate () diff --git a/src/ripple/app/tx/impl/OfferStream.cpp b/src/ripple/app/tx/impl/OfferStream.cpp index 4db122a6c..4fd366e1d 100644 --- a/src/ripple/app/tx/impl/OfferStream.cpp +++ b/src/ripple/app/tx/impl/OfferStream.cpp @@ -216,7 +216,7 @@ OfferStream::permRmOffer (std::shared_ptr const& sle) template void FlowOfferStream::permRmOffer (std::shared_ptr const& sle) { - toRemove_.push_back (sle->key()); + toRemove_.insert (sle->key()); } template class FlowOfferStream; diff --git a/src/ripple/app/tx/impl/OfferStream.h b/src/ripple/app/tx/impl/OfferStream.h index e666c8483..5533383a7 100644 --- a/src/ripple/app/tx/impl/OfferStream.h +++ b/src/ripple/app/tx/impl/OfferStream.h @@ -28,6 +28,8 @@ #include #include +#include + namespace ripple { template @@ -163,7 +165,7 @@ template class FlowOfferStream : public TOfferStreamBase { private: - std::vector toRemove_; + boost::container::flat_set toRemove_; protected: void permRmOffer (std::shared_ptr const& sle) override; @@ -171,7 +173,7 @@ protected: public: using TOfferStreamBase::TOfferStreamBase; - std::vector const& toRemove () const + boost::container::flat_set const& toRemove () const { return toRemove_; };