diff --git a/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp b/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp index 620eb85928..deb3b7cdd2 100644 --- a/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp +++ b/src/ripple/app/paths/cursor/DeliverNodeReverse.cpp @@ -35,17 +35,15 @@ namespace path { // as the rate does not increase past the initial rate. // To deliver from an order book, when computing -TER PathCursor::deliverNodeReverse ( - AccountID const& uOutAccountID, // --> Output owner's account. - STAmount const& saOutReq, // --> Funds requested to be - // delivered for an increment. - STAmount& saOutAct) const // <-- Funds actually delivered for an - // increment. +TER PathCursor::deliverNodeReverseImpl ( + AccountID const& uOutAccountID, // --> Output owner's account. + STAmount const& saOutReq, // --> Funds requested to be + // delivered for an increment. + STAmount& saOutAct // <-- Funds actually delivered for an + // increment + ) const { TER resultCode = tesSUCCESS; - - node().directory.restart(multiQuality_); - // Accumulation of what the previous node must deliver. // Possible optimization: Note this gets zeroed on each increment, ideally // only on first increment, then it could be a limit on the forward pass. @@ -235,7 +233,7 @@ TER PathCursor::deliverNodeReverse ( // Compute in previous offer node how much could come in. // TODO(tom): Fix nasty recursion here! - resultCode = increment(-1).deliverNodeReverse( + resultCode = increment(-1).deliverNodeReverseImpl( node().offerOwnerAccount_, saInPassReq, saInPassAct); @@ -345,5 +343,19 @@ TER PathCursor::deliverNodeReverse ( return resultCode; } +TER PathCursor::deliverNodeReverse ( + AccountID const& uOutAccountID, // --> Output owner's account. + STAmount const& saOutReq, // --> Funds requested to be + // delivered for an increment. + STAmount& saOutAct // <-- Funds actually delivered for an + // increment + ) const +{ + for (int i = nodeIndex_; i >= 0 && !node (i).isAccount(); --i) + node (i).directory.restart (multiQuality_); + + return deliverNodeReverseImpl(uOutAccountID, saOutReq, saOutAct); +} + } // path } // ripple diff --git a/src/ripple/app/paths/cursor/PathCursor.h b/src/ripple/app/paths/cursor/PathCursor.h index 336045ac46..6b7f48acfc 100644 --- a/src/ripple/app/paths/cursor/PathCursor.h +++ b/src/ripple/app/paths/cursor/PathCursor.h @@ -85,6 +85,12 @@ private: STAmount const& saOutReq, STAmount& saOutAct) const; + // To deliver from an order book, when computing + TER deliverNodeReverseImpl ( + AccountID const& uOutAccountID, + STAmount const& saOutReq, + STAmount& saOutAct) const; + TER deliverNodeForward ( AccountID const& uInAccountID, STAmount const& saInReq, diff --git a/src/ripple/app/tx/tests/Offer.test.cpp b/src/ripple/app/tx/tests/Offer.test.cpp index 1bb3d00d9d..65778e1a6d 100644 --- a/src/ripple/app/tx/tests/Offer.test.cpp +++ b/src/ripple/app/tx/tests/Offer.test.cpp @@ -23,31 +23,69 @@ #include #include #include +#include namespace ripple { namespace test { -// An offer exists -bool isOffer (jtx::Env const& env, - jtx::Account const& account, - STAmount const& takerPays, - STAmount const& takerGets) -{ - bool exists = false; - forEachItem (*env.open(), account, - [&](std::shared_ptr const& sle) - { - if (sle->getType () == ltOFFER && - sle->getFieldAmount (sfTakerPays) == takerPays && - sle->getFieldAmount (sfTakerGets) == takerGets) - exists = true; - }); - return exists; -} - class Offer_test : public beast::unit_test::suite { public: + void testRmFundedOffer () + { + // We need at least two paths. One at good quality and one at bad quality. + // The bad quality path needs two offer books in a row. Each offer book + // should have two offers at the same quality, the offers should be + // completely consumed, and the payment should should require both offers to + // be satisified. The first offer must be "taker gets" XRP. Old, broken + // would remove the first "taker gets" xrp offer, even though the offer is + // still funded and not used for the payment. + + using namespace jtx; + Env env (*this); + auto const gw = Account ("gateway"); + auto const USD = gw["USD"]; + auto const BTC = gw["BTC"]; + Account const alice ("alice"); + Account const bob ("bob"); + Account const carol ("carol"); + + env.fund (XRP (10000), alice, bob, carol, gw); + env.trust (USD (1000), alice, bob, carol); + env.trust (BTC (1000), alice, bob, carol); + + env (pay (gw, alice, BTC (1000))); + + env (pay (gw, carol, USD (1000))); + env (pay (gw, carol, BTC (1000))); + + // Must be two offers at the same quality + // "taker gets" must be XRP + // (Different amounts so I can distinguish the offers) + env (offer (carol, BTC (49), XRP (49))); + env (offer (carol, BTC (51), XRP (51))); + + // Offers for the poor quality path + // Must be two offers at the same quality + env (offer (carol, XRP (50), USD (50))); + env (offer (carol, XRP (50), USD (50))); + + // Offers for the good quality path + env (offer (carol, BTC (1), USD (100))); + + PathSet paths ( + Path (XRP, USD), + Path (USD)); + + env (pay ("alice", "bob", USD (100)), + json (paths.json ()), + sendmax (BTC (1000)), + txflags (tfPartialPayment)); + + require (balance ("bob", USD (100))); + expect (!isOffer (env, "carol", BTC (1), USD (100)) && + isOffer (env, "carol", BTC (49), XRP (49))); + } void testCanceledOffer () { using namespace jtx; @@ -87,6 +125,7 @@ public: // Hack to silence logging deprecatedLogs ().severity (beast::Journal::Severity::kNone); testCanceledOffer (); + testRmFundedOffer (); } };