diff --git a/src/ripple/app/paths/impl/XRPEndpointStep.cpp b/src/ripple/app/paths/impl/XRPEndpointStep.cpp index 2c6b91680fe..d30bd1887f7 100644 --- a/src/ripple/app/paths/impl/XRPEndpointStep.cpp +++ b/src/ripple/app/paths/impl/XRPEndpointStep.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -359,6 +360,18 @@ XRPEndpointStep::check (StrandContext const& ctx) const if (ter != tesSUCCESS) return ter; + if (ctx.view.rules().enabled(fix1781)) + { + auto const issuesIndex = isLast_ ? 0 : 1; + if (!ctx.seenDirectIssues[issuesIndex].insert(xrpIssue()).second) + { + JLOG(j_.debug()) + << "XRPEndpointStep: loop detected: Index: " << ctx.strandSize + << ' ' << *this; + return temBAD_PATH_LOOP; + } + } + return tesSUCCESS; } diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index d11a7e3e649..3a7ee97862d 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -111,6 +111,7 @@ class FeatureCollections // fixQualityUpperBound should be activated before FlowCross "fixQualityUpperBound", "RequireFullyCanonicalSig", + "fix1781", // XRPEndpointSteps should be included in the circular payment check }; std::vector features; @@ -399,6 +400,7 @@ extern uint256 const fixPayChanRecipientOwnerDir; extern uint256 const featureDeletableAccounts; extern uint256 const fixQualityUpperBound; extern uint256 const featureRequireFullyCanonicalSig; +extern uint256 const fix1781; } // ripple diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index db96583a058..bac6e2e8f33 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -130,6 +130,7 @@ detail::supportedAmendments () "DeletableAccounts", "fixQualityUpperBound", "RequireFullyCanonicalSig", + "fix1781", }; return supported; } @@ -189,5 +190,6 @@ uint256 const fixPayChanRecipientOwnerDir = *getRegisteredFeature("fixPayChanRec uint256 const featureDeletableAccounts = *getRegisteredFeature("DeletableAccounts"); uint256 const fixQualityUpperBound = *getRegisteredFeature("fixQualityUpperBound"); uint256 const featureRequireFullyCanonicalSig = *getRegisteredFeature("RequireFullyCanonicalSig"); +uint256 const fix1781 = *getRegisteredFeature("fix1781"); } // ripple diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp index 9a7637fa471..bfd1460b4d4 100644 --- a/src/test/app/Flow_test.cpp +++ b/src/test/app/Flow_test.cpp @@ -1180,6 +1180,100 @@ struct Flow_test : public beast::unit_test::suite ter(temBAD_PATH)); } + void + testXRPPathLoop() + { + testcase("Circular XRP"); + + using namespace jtx; + auto const alice = Account("alice"); + auto const bob = Account("bob"); + auto const gw = Account("gw"); + auto const USD = gw["USD"]; + auto const EUR = gw["EUR"]; + + for (auto const withFix : {true, false}) + { + auto const feats = [&withFix]() -> FeatureBitset { + if (withFix) + return supported_amendments(); + return supported_amendments() - FeatureBitset{fix1781}; + }(); + { + // Payment path starting with XRP + Env env(*this, feats); + env.fund(XRP(10000), alice, bob, gw); + env.trust(USD(1000), alice, bob); + env.trust(EUR(1000), alice, bob); + env(pay(gw, alice, USD(100))); + env(pay(gw, alice, EUR(100))); + env.close(); + + env(offer(alice, XRP(100), USD(100)), txflags(tfPassive)); + env(offer(alice, USD(100), XRP(100)), txflags(tfPassive)); + env(offer(alice, XRP(100), EUR(100)), txflags(tfPassive)); + env.close(); + + TER const expectedTer = + withFix ? TER{temBAD_PATH_LOOP} : TER{tesSUCCESS}; + env(pay(alice, bob, EUR(1)), + path(~USD, ~XRP, ~EUR), + sendmax(XRP(1)), + txflags(tfNoRippleDirect), + ter(expectedTer)); + } + pass(); + } + { + // Payment path ending with XRP + Env env(*this); + env.fund(XRP(10000), alice, bob, gw); + env.trust(USD(1000), alice, bob); + env.trust(EUR(1000), alice, bob); + env(pay(gw, alice, USD(100))); + env(pay(gw, alice, EUR(100))); + env.close(); + + env(offer(alice, XRP(100), USD(100)), txflags(tfPassive)); + env(offer(alice, EUR(100), XRP(100)), txflags(tfPassive)); + env.close(); + // EUR -> //XRP -> //USD ->XRP + env(pay(alice, bob, XRP(1)), + path(~XRP, ~USD, ~XRP), + sendmax(EUR(1)), + txflags(tfNoRippleDirect), + ter(temBAD_PATH_LOOP)); + } + { + // Payment where loop is formed in the middle of the path, not on an + // endpoint + auto const JPY = gw["JPY"]; + Env env(*this); + env.fund(XRP(10000), alice, bob, gw); + env.close(); + env.trust(USD(1000), alice, bob); + env.trust(EUR(1000), alice, bob); + env.trust(JPY(1000), alice, bob); + env.close(); + env(pay(gw, alice, USD(100))); + env(pay(gw, alice, EUR(100))); + env(pay(gw, alice, JPY(100))); + env.close(); + + env(offer(alice, USD(100), XRP(100)), txflags(tfPassive)); + env(offer(alice, XRP(100), EUR(100)), txflags(tfPassive)); + env(offer(alice, EUR(100), XRP(100)), txflags(tfPassive)); + env(offer(alice, XRP(100), JPY(100)), txflags(tfPassive)); + env.close(); + + env(pay(alice, bob, JPY(1)), + path(~XRP, ~EUR, ~XRP, ~JPY), + sendmax(USD(1)), + txflags(tfNoRippleDirect), + ter(temBAD_PATH_LOOP)); + } + } + void testWithFeats(FeatureBitset features) { using namespace jtx; @@ -1204,6 +1298,7 @@ struct Flow_test : public beast::unit_test::suite void run() override { testLimitQuality(); + testXRPPathLoop(); testRIPD1443(); testRIPD1449(); @@ -1231,7 +1326,7 @@ struct Flow_manual_test : public Flow_test testWithFeats(all - flowCross - f1513); testWithFeats(all - flowCross ); testWithFeats(all - f1513); - testWithFeats(all ); + testWithFeats(all ); testEmptyStrand(all - f1513); testEmptyStrand(all ); diff --git a/src/test/app/PayStrand_test.cpp b/src/test/app/PayStrand_test.cpp index 3b7c4c6f9a4..59f8967e136 100644 --- a/src/test/app/PayStrand_test.cpp +++ b/src/test/app/PayStrand_test.cpp @@ -703,7 +703,7 @@ struct PayStrand_test : public beast::unit_test::suite alice, /*deliver*/ xrpIssue(), /*limitQuality*/ boost::none, - /*sendMaxIssue*/ xrpIssue(), + /*sendMaxIssue*/ EUR.issue(), path, true, false,