diff --git a/src/ripple/app/paths/impl/DirectStep.cpp b/src/ripple/app/paths/impl/DirectStep.cpp index a46d5a553..914405009 100644 --- a/src/ripple/app/paths/impl/DirectStep.cpp +++ b/src/ripple/app/paths/impl/DirectStep.cpp @@ -39,6 +39,7 @@ class DirectStepI : public StepImp AccountID src_; AccountID dst_; Currency currency_; + bool isLast_ = false; // Charge transfer fees when the prev step redeems Step const* const prevStep_ = nullptr; @@ -72,18 +73,22 @@ class DirectStepI : public StepImp PaymentSandbox& sb, bool srcRedeems, bool fwd) const; + public: DirectStepI ( AccountID const& src, AccountID const& dst, Currency const& c, Step const* prevStep, + bool isLast, beast::Journal j) :src_(src) , dst_(dst) , currency_ (c) + , isLast_ (isLast) , prevStep_ (prevStep) - , j_ (j) {} + , j_ (j) + {} AccountID const& src () const { @@ -122,6 +127,9 @@ class DirectStepI : public StepImp bool redeems (ReadView const& sb, bool fwd) const override; + std::uint32_t + lineQualityIn (ReadView const& v) const override; + std::pair revImp ( PaymentSandbox& sb, @@ -484,11 +492,11 @@ DirectStepI::validFwd ( static std::uint32_t quality ( - PaymentSandbox const& sb, + ReadView const& sb, AccountID const& src, AccountID const& dst, Currency const& currency, - // set true for quality in, false for quality out + // set true for dst quality in, false for src quality out bool qin) { if (src == dst) @@ -502,17 +510,19 @@ quality ( auto const& field = [&]() -> SF_U32 const& { - if (dst < src) + if (qin) { - if (qin) + // compute dst quality in + if (dst < src) return sfLowQualityIn; else - return sfLowQualityOut; + return sfHighQualityIn; } else { - if (qin) - return sfHighQualityIn; + // compute src quality out + if (src < dst) + return sfLowQualityOut; else return sfHighQualityOut; } @@ -536,27 +546,35 @@ DirectStepI::qualities ( { if (srcRedeems) { - return std::make_pair( - quality ( - sb, src_, dst_, currency_, - false), - QUALITY_ONE); + if (!prevStep_) + return {QUALITY_ONE, QUALITY_ONE}; + + auto const prevStepQIn = prevStep_->lineQualityIn (sb); + auto srcQOut = quality (sb, src_, dst_, currency_, false); + if (prevStepQIn > srcQOut) + srcQOut = prevStepQIn; + return {srcQOut, QUALITY_ONE}; } else { // Charge a transfer rate when issuing and previous step redeems auto const prevStepRedeems = prevStep_ && prevStep_->redeems (sb, fwd); - std::uint32_t const srcQOut = prevStepRedeems - ? transferRate (sb, src_).value - : QUALITY_ONE; - return std::make_pair( - srcQOut, - quality ( // dst quality in - sb, src_, dst_, currency_, - true)); + std::uint32_t const srcQOut = + prevStepRedeems ? transferRate (sb, src_).value : QUALITY_ONE; + auto dstQIn = quality (sb, src_, dst_, currency_, true); + if (isLast_ && dstQIn > QUALITY_ONE) + dstQIn = QUALITY_ONE; + return {srcQOut, dstQIn}; } } +std::uint32_t +DirectStepI::lineQualityIn (ReadView const& v) const +{ + // dst quality in + return quality (v, src_, dst_, currency_, true); +} + TER DirectStepI::check (StrandContext const& ctx) const { if (!src_ || !dst_) @@ -565,6 +583,12 @@ TER DirectStepI::check (StrandContext const& ctx) const return temBAD_PATH; } + if (src_ == dst_) + { + JLOG (j_.debug()) << "DirectStepI: same src and dst."; + return temBAD_PATH; + } + { auto sleSrc = ctx.view.read (keylet::account (src_)); if (!sleSrc) @@ -692,7 +716,7 @@ make_DirectStepI ( { // Only charge a transfer fee if the previous step redeems auto r = std::make_unique ( - src, dst, c, ctx.prevStep, ctx.j); + src, dst, c, ctx.prevStep, ctx.isLast, ctx.j); auto ter = r->check (ctx); if (ter != tesSUCCESS) return {ter, nullptr}; diff --git a/src/ripple/app/paths/impl/Steps.h b/src/ripple/app/paths/impl/Steps.h index 7bff5a946..6a212d5d8 100644 --- a/src/ripple/app/paths/impl/Steps.h +++ b/src/ripple/app/paths/impl/Steps.h @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -132,6 +133,14 @@ public: return false; } + /** If this step is a DirectStepI, return the quality in of the dst account. + */ + virtual std::uint32_t + lineQualityIn (ReadView const&) const + { + return QUALITY_ONE; + } + /** If this step is a BookStep, return the book. */ diff --git a/src/ripple/app/tests/Flow_test.cpp b/src/ripple/app/tests/Flow_test.cpp index f19c24bfd..a57dbf89c 100644 --- a/src/ripple/app/tests/Flow_test.cpp +++ b/src/ripple/app/tests/Flow_test.cpp @@ -512,6 +512,82 @@ struct Flow_test : public beast::unit_test::suite } } + void testLineQuality () + { + testcase ("Line Quality"); + + using namespace jtx; + auto const alice = Account ("alice"); + auto const bob = Account ("bob"); + auto const carol = Account ("carol"); + auto const dan = Account ("dan"); + auto const USDA = alice["USD"]; + auto const USDB = bob["USD"]; + auto const USDC = carol["USD"]; + auto const USDD = dan["USD"]; + + // Dan -> Bob -> Alice -> Carol; vary bobDanQIn and bobAliceQOut + for (auto bobDanQIn : {80, 100, 120}) + for (auto bobAliceQOut : {80, 100, 120}) + for (auto const& f : {feature ("nullFeature"), featureFlowV2}) + { + if (f != featureFlowV2 && bobDanQIn < 100 && bobAliceQOut < 100) + continue; // Bug in flow v1 + Env env (*this, features (f)); + env.fund (XRP (10000), alice, bob, carol, dan); + env (trust (bob, USDD (100)), qualityInPercent (bobDanQIn)); + env (trust (bob, USDA (100)), + qualityOutPercent (bobAliceQOut)); + env (trust (carol, USDA (100))); + + env (pay (alice, bob, USDA (100))); + env.require (balance (bob, USDA (100))); + env (pay (dan, carol, USDA (10)), path (bob), + sendmax (USDD (100)), txflags (tfNoRippleDirect)); + env.require (balance (bob, USDA (90))); + if (bobAliceQOut > bobDanQIn) + env.require ( + balance (bob, USDD (10.0 * double(bobAliceQOut) / + double(bobDanQIn)))); + else + env.require (balance (bob, USDD (10))); + env.require (balance (carol, USDA (10))); + } + + // bob -> alice -> carol; vary carolAliceQIn + for (auto carolAliceQIn : {80, 100, 120}) + for (auto const& f : {feature ("nullFeature"), featureFlowV2}) + { + Env env (*this, features (f)); + env.fund (XRP (10000), alice, bob, carol); + env (trust (bob, USDA (10))); + env (trust (carol, USDA (10)), qualityInPercent (carolAliceQIn)); + + env (pay (alice, bob, USDA (10))); + env.require (balance (bob, USDA (10))); + env (pay (bob, carol, USDA (5)), sendmax (USDA (10))); + auto const effectiveQ = + carolAliceQIn > 100 ? 1.0 : carolAliceQIn / 100.0; + env.require (balance (bob, USDA (10.0 - 5.0 / effectiveQ))); + } + + // bob -> alice -> carol; bobAliceQOut varies. + for (auto bobAliceQOut : {80, 100, 120}) + for (auto const& f : {feature ("nullFeature"), featureFlowV2}) + { + Env env (*this, features (f)); + env.fund (XRP (10000), alice, bob, carol); + env (trust (bob, USDA (10)), qualityOutPercent (bobAliceQOut)); + env (trust (carol, USDA (10))); + + env (pay (alice, bob, USDA (10))); + env.require (balance (bob, USDA (10))); + env (pay (bob, carol, USDA (5)), sendmax (USDA (5))); + env.require (balance (carol, USDA (5))); + env.require (balance (bob, USDA (10-5))); + } + } + void testBookStep () { testcase ("Book Step"); @@ -1185,6 +1261,7 @@ struct Flow_test : public beast::unit_test::suite void run() override { testDirectStep (); + testLineQuality(); testBookStep (); testTransferRate (); testToStrand ();