From 79a0cb096b1ec86fe40367e6ccb61f4a42fa15db Mon Sep 17 00:00:00 2001 From: seelabs Date: Thu, 9 May 2019 10:16:06 -0400 Subject: [PATCH] Payment paths with a zero output step are dry (RIPD-1749): A tiny input amount to a payment step can cause this step to output zero. For example, if a previous steps outputs a dust amount of 10^-80, and this step is a IOU -> XRP offer, the offer may output zero drops. In this case, call the strand dry. Before this patch, an error would be logged, the strand would be called dry; in debug mode an assert triggered. Note, this patch is not transaction breaking, as the caller did not user the ter code. The caller only checked for success or failuer. This patch addresses github issue issue reported here: https://github.com/ripple/rippled/issues/2929 --- src/ripple/app/paths/impl/AmountSpec.h | 12 +++++++ src/ripple/app/paths/impl/StrandFlow.h | 49 +++++++++++++++++++++----- src/test/app/Flow_test.cpp | 35 ++++++++++++++++++ 3 files changed, 87 insertions(+), 9 deletions(-) diff --git a/src/ripple/app/paths/impl/AmountSpec.h b/src/ripple/app/paths/impl/AmountSpec.h index 52cfbfc81..4d32fad47 100644 --- a/src/ripple/app/paths/impl/AmountSpec.h +++ b/src/ripple/app/paths/impl/AmountSpec.h @@ -105,6 +105,18 @@ struct EitherAmount else iou = a.iou; } + +#ifndef NDEBUG + friend std::ostream& + operator<<(std::ostream& stream, EitherAmount const& amt) + { + if (amt.native) + stream << to_string(amt.xrp); + else + stream << to_string(amt.iou); + return stream; + } +#endif }; template diff --git a/src/ripple/app/paths/impl/StrandFlow.h b/src/ripple/app/paths/impl/StrandFlow.h index c6551012a..247697e47 100644 --- a/src/ripple/app/paths/impl/StrandFlow.h +++ b/src/ripple/app/paths/impl/StrandFlow.h @@ -144,14 +144,21 @@ flow ( *sb, *afView, ofrsToRm, EitherAmount (*maxIn)); limitStepOut = r.second; - if (strand[i]->isZero (r.second) || - get (r.first) != *maxIn) + if (strand[i]->isZero(r.second)) + { + JLOG(j.trace()) << "First step found dry"; + return {tecPATH_DRY, std::move(ofrsToRm)}; + } + if (get (r.first) != *maxIn) { // Something is very wrong // throwing out the sandbox can only increase liquidity // yet the limiting is still limiting - JLOG (j.fatal()) << "Re-executed limiting step failed"; - assert (0); + JLOG(j.fatal()) + << "Re-executed limiting step failed. r.first: " + << to_string(get(r.first)) + << " maxIn: " << to_string(*maxIn); + assert(0); return {telFAILED_PROCESSING, std::move (ofrsToRm)}; } } @@ -168,13 +175,25 @@ flow ( r = strand[i]->rev (*sb, *afView, ofrsToRm, stepOut); limitStepOut = r.second; - if (strand[i]->isZero (r.second) || - !strand[i]->equalOut (r.second, stepOut)) + if (strand[i]->isZero(r.second)) + { + // 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 {tecPATH_DRY, std::move(ofrsToRm)}; + } + if (!strand[i]->equalOut (r.second, stepOut)) { // Something is very wrong // throwing out the sandbox can only increase liquidity // yet the limiting is still limiting +#ifndef NDEBUG + JLOG(j.fatal()) + << "Re-executed limiting step failed. r.second: " + << r.second << " stepOut: " << stepOut; +#else JLOG (j.fatal()) << "Re-executed limiting step failed"; +#endif assert (0); return {telFAILED_PROCESSING, std::move (ofrsToRm)}; } @@ -189,13 +208,25 @@ flow ( EitherAmount stepIn (limitStepOut); for (auto i = limitingStep + 1; i < s; ++i) { - auto const r = strand[i]->fwd (*sb, *afView, ofrsToRm, stepIn); - if (strand[i]->isZero (r.second) || - !strand[i]->equalIn (r.first, stepIn)) + auto const r = strand[i]->fwd(*sb, *afView, ofrsToRm, stepIn); + if (strand[i]->isZero(r.second)) + { + // 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 {tecPATH_DRY, std::move(ofrsToRm)}; + } + if (!strand[i]->equalIn (r.first, stepIn)) { // The limits should already have been found, so executing a strand forward // from the limiting step should not find a new limit +#ifndef NDEBUG + JLOG(j.fatal()) + << "Re-executed forward pass failed. r.first: " + << r.first << " stepIn: " << stepIn; +#else JLOG (j.fatal()) << "Re-executed forward pass failed"; +#endif assert (0); return {telFAILED_PROCESSING, std::move (ofrsToRm)}; } diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp index 4a211eaae..bcd533947 100644 --- a/src/test/app/Flow_test.cpp +++ b/src/test/app/Flow_test.cpp @@ -1248,6 +1248,40 @@ struct Flow_test : public beast::unit_test::suite ter(temBAD_PATH)); } + void + testZeroOutputStep() + { + testcase("Zero Output Step"); + + using namespace jtx; + auto const alice = Account("alice"); + auto const bob = Account("bob"); + auto const carol = Account("carol"); + auto const gw = Account("gw"); + auto const USD = gw["USD"]; + auto const EUR = gw["EUR"]; + + auto const features = supported_amendments(); + Env env(*this, features); + env.fund(XRP(10000), alice, bob, carol, gw); + env.trust(USD(1000), alice, bob, carol); + env.trust(EUR(1000), alice, bob, carol); + env(pay(gw, alice, USD(100))); + env(pay(gw, bob, USD(100))); + env(pay(gw, bob, EUR(100))); + env.close(); + + env(offer(bob, USD(100), EUR(100))); + env(offer(bob, EUR(100), XRP(0.000001))); + env.close(); + + env(pay(alice, carol, XRP(1)), + path(~EUR, ~XRP), + sendmax(USD(1)), + txflags(tfPartialPayment), + ter(tecPATH_DRY)); + } + void testWithFeats(FeatureBitset features) { using namespace jtx; @@ -1276,6 +1310,7 @@ struct Flow_test : public beast::unit_test::suite void run() override { testLimitQuality(); + testZeroOutputStep(); testRIPD1443(true); testRIPD1443(false); testRIPD1449(true);