diff --git a/src/ripple/app/paths/impl/BookStep.cpp b/src/ripple/app/paths/impl/BookStep.cpp index 3d7374866..fd85f91f0 100644 --- a/src/ripple/app/paths/impl/BookStep.cpp +++ b/src/ripple/app/paths/impl/BookStep.cpp @@ -999,8 +999,7 @@ BookStep::check(StrandContext const& ctx) const return temBAD_PATH_LOOP; } - if (ctx.view.rules().enabled(fix1373) && - ctx.seenDirectIssues[1].count(book_.out)) + if (ctx.seenDirectIssues[1].count(book_.out)) { JLOG(j_.debug()) << "BookStep: loop detected: " << *this; return temBAD_PATH_LOOP; diff --git a/src/ripple/app/paths/impl/PaySteps.cpp b/src/ripple/app/paths/impl/PaySteps.cpp index a7eb6da42..6d87c33a2 100644 --- a/src/ripple/app/paths/impl/PaySteps.cpp +++ b/src/ripple/app/paths/impl/PaySteps.cpp @@ -101,9 +101,7 @@ toStep ( JLOG (j.error()) << "Found offer/account payment step. Aborting payment strand."; assert (0); - if (ctx.view.rules().enabled(fix1373)) - return {temBAD_PATH, std::unique_ptr{}}; - Throw (tefEXCEPTION, "Found offer/account payment step."); + return {temBAD_PATH, std::unique_ptr{}}; } assert ((e2->getNodeType () & STPathElement::typeCurrency) || @@ -133,255 +131,7 @@ toStep ( } std::pair -toStrandV1 ( - ReadView const& view, - AccountID const& src, - AccountID const& dst, - Issue const& deliver, - boost::optional const& limitQuality, - boost::optional const& sendMaxIssue, - STPath const& path, - bool ownerPaysTransferFee, - bool offerCrossing, - beast::Journal j) -{ - if (isXRP (src)) - { - JLOG (j.debug()) << "toStrand with xrpAccount as src"; - return {temBAD_PATH, Strand{}}; - } - if (isXRP (dst)) - { - JLOG (j.debug()) << "toStrand with xrpAccount as dst"; - return {temBAD_PATH, Strand{}}; - } - if (!isConsistent (deliver)) - { - JLOG (j.debug()) << "toStrand inconsistent deliver issue"; - return {temBAD_PATH, Strand{}}; - } - if (sendMaxIssue && !isConsistent (*sendMaxIssue)) - { - JLOG (j.debug()) << "toStrand inconsistent sendMax issue"; - return {temBAD_PATH, Strand{}}; - } - - Issue curIssue = [&] - { - auto& currency = - sendMaxIssue ? sendMaxIssue->currency : deliver.currency; - if (isXRP (currency)) - return xrpIssue (); - return Issue{currency, src}; - }(); - - STPathElement const firstNode ( - STPathElement::typeAll, src, curIssue.currency, curIssue.account); - - boost::optional sendMaxPE; - if (sendMaxIssue && sendMaxIssue->account != src && - (path.empty () || !path[0].isAccount () || - path[0].getAccountID () != sendMaxIssue->account)) - sendMaxPE.emplace (sendMaxIssue->account, boost::none, boost::none); - - STPathElement const lastNode (dst, boost::none, boost::none); - - auto hasCurrency = [](STPathElement const* pe) - { - return pe->getNodeType () & STPathElement::typeCurrency; - }; - - boost::optional deliverOfferNode; - boost::optional deliverAccountNode; - - std::vector pes; - // reserve enough for the path, the implied source, destination, - // sendmax and deliver. - pes.reserve (4 + path.size ()); - pes.push_back (&firstNode); - if (sendMaxPE) - pes.push_back (&*sendMaxPE); - for (auto& i : path) - pes.push_back (&i); - - // Note that for offer crossing (only) we do use an offer book even if - // all that is changing is the Issue.account. - STPathElement const* const lastCurrency = - *std::find_if (pes.rbegin(), pes.rend(), hasCurrency); - if ((lastCurrency->getCurrency() != deliver.currency) || - (offerCrossing && lastCurrency->getIssuerID() != deliver.account)) - { - deliverOfferNode.emplace (boost::none, deliver.currency, deliver.account); - pes.push_back (&*deliverOfferNode); - } - if (!((pes.back ()->isAccount() && pes.back ()->getAccountID () == deliver.account) || - (lastNode.isAccount() && lastNode.getAccountID () == deliver.account))) - { - deliverAccountNode.emplace (deliver.account, boost::none, boost::none); - pes.push_back (&*deliverAccountNode); - } - if (*pes.back() != lastNode) - pes.push_back (&lastNode); - - auto const strandSrc = firstNode.getAccountID (); - auto const strandDst = lastNode.getAccountID (); - bool const isDefaultPath = path.empty(); - - Strand result; - result.reserve (2 * pes.size ()); - - /* A strand may not include the same account node more than once - in the same currency. In a direct step, an account will show up - at most twice: once as a src and once as a dst (hence the two element array). - The strandSrc and strandDst will only show up once each. - */ - std::array, 2> seenDirectIssues; - // A strand may not include the same offer book more than once - boost::container::flat_set seenBookOuts; - seenDirectIssues[0].reserve (pes.size()); - seenDirectIssues[1].reserve (pes.size()); - seenBookOuts.reserve (pes.size()); - auto ctx = [&](bool isLast = false) - { - return StrandContext{view, result, strandSrc, strandDst, deliver, - limitQuality, isLast, ownerPaysTransferFee, offerCrossing, - isDefaultPath, seenDirectIssues, seenBookOuts, j}; - }; - - for (int i = 0; i < pes.size () - 1; ++i) - { - /* Iterate through the path elements considering them in pairs. - The first element of the pair is `cur` and the second element is - `next`. When an offer is one of the pairs, the step created will be for - `next`. This means when `cur` is an offer and `next` is an - account then no step is created, as a step has already been created for - that offer. - */ - boost::optional impliedPE; - auto cur = pes[i]; - auto next = pes[i + 1]; - - if (cur->isNone() || next->isNone()) - return {temBAD_PATH, Strand{}}; - - /* If both account and issuer are set, use the account - (and don't insert an implied node for the issuer). - This matches the behavior of the previous generation payment code - */ - if (cur->isAccount()) - curIssue.account = cur->getAccountID (); - else if (cur->hasIssuer()) - curIssue.account = cur->getIssuerID (); - - if (cur->hasCurrency()) - curIssue.currency = cur->getCurrency (); - - if (cur->isAccount() && next->isAccount()) - { - if (!isXRP (curIssue.currency) && - curIssue.account != cur->getAccountID () && - curIssue.account != next->getAccountID ()) - { - JLOG (j.trace()) << "Inserting implied account"; - auto msr = make_DirectStepI (ctx(), cur->getAccountID (), - curIssue.account, curIssue.currency); - if (msr.first != tesSUCCESS) - return {msr.first, Strand{}}; - result.push_back (std::move (msr.second)); - Currency dummy; - impliedPE.emplace (STPathElement::typeAccount, - curIssue.account, dummy, curIssue.account); - cur = &*impliedPE; - } - } - else if (cur->isAccount() && next->isOffer()) - { - if (curIssue.account != cur->getAccountID ()) - { - JLOG (j.trace()) << "Inserting implied account before offer"; - auto msr = make_DirectStepI (ctx(), cur->getAccountID (), - curIssue.account, curIssue.currency); - if (msr.first != tesSUCCESS) - return {msr.first, Strand{}}; - result.push_back (std::move (msr.second)); - Currency dummy; - impliedPE.emplace (STPathElement::typeAccount, - curIssue.account, dummy, curIssue.account); - cur = &*impliedPE; - } - } - else if (cur->isOffer() && next->isAccount()) - { - if (curIssue.account != next->getAccountID () && - !isXRP (next->getAccountID ())) - { - JLOG (j.trace()) << "Inserting implied account after offer"; - auto msr = make_DirectStepI (ctx(), curIssue.account, - next->getAccountID (), curIssue.currency); - if (msr.first != tesSUCCESS) - return {msr.first, Strand{}}; - result.push_back (std::move (msr.second)); - } - continue; - } - - if (!next->isOffer() && - next->hasCurrency() && next->getCurrency () != curIssue.currency) - { - auto const& nextCurrency = next->getCurrency (); - auto const& nextIssuer = - next->hasIssuer () ? next->getIssuerID () : curIssue.account; - - if (isXRP (curIssue.currency)) - { - JLOG (j.trace()) << "Inserting implied XI offer"; - auto msr = make_BookStepXI ( - ctx(), {nextCurrency, nextIssuer}); - if (msr.first != tesSUCCESS) - return {msr.first, Strand{}}; - result.push_back (std::move (msr.second)); - } - else if (isXRP (nextCurrency)) - { - JLOG (j.trace()) << "Inserting implied IX offer"; - auto msr = make_BookStepIX (ctx(), curIssue); - if (msr.first != tesSUCCESS) - return {msr.first, Strand{}}; - result.push_back (std::move (msr.second)); - } - else - { - JLOG (j.trace()) << "Inserting implied II offer"; - auto msr = make_BookStepII ( - ctx(), curIssue, {nextCurrency, nextIssuer}); - if (msr.first != tesSUCCESS) - return {msr.first, Strand{}}; - result.push_back (std::move (msr.second)); - } - impliedPE.emplace ( - boost::none, nextCurrency, nextIssuer); - cur = &*impliedPE; - curIssue.currency = nextCurrency; - curIssue.account = nextIssuer; - } - - auto s = - toStep (ctx (/*isLast*/ i == pes.size () - 2), cur, next, curIssue); - if (s.first == tesSUCCESS) - result.emplace_back (std::move (s.second)); - else - { - JLOG (j.debug()) << "toStep failed: " << s.first; - return {s.first, Strand{}}; - } - } - - return {tesSUCCESS, std::move (result)}; -} - - -std::pair -toStrandV2 ( +toStrand ( ReadView const& view, AccountID const& src, AccountID const& dst, @@ -689,27 +439,6 @@ toStrandV2 ( return {tesSUCCESS, std::move (result)}; } -std::pair -toStrand ( - ReadView const& view, - AccountID const& src, - AccountID const& dst, - Issue const& deliver, - boost::optional const& limitQuality, - boost::optional const& sendMaxIssue, - STPath const& path, - bool ownerPaysTransferFee, - bool offerCrossing, - beast::Journal j) -{ - if (view.rules().enabled(fix1373)) - return toStrandV2(view, src, dst, deliver, limitQuality, - sendMaxIssue, path, ownerPaysTransferFee, offerCrossing, j); - else - return toStrandV1(view, src, dst, deliver, limitQuality, - sendMaxIssue, path, ownerPaysTransferFee, offerCrossing, j); -} - std::pair> toStrands ( ReadView const& view, diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 25f333421..23b0bec3a 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -374,7 +374,7 @@ extern uint256 const retiredTickSize; extern uint256 const retiredFix1368; extern uint256 const retiredEscrow; extern uint256 const featureCryptoConditionsSuite; -extern uint256 const fix1373; +extern uint256 const retiredFix1373; extern uint256 const featureEnforceInvariants; extern uint256 const featureSortedDirectories; extern uint256 const fix1201; diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index e58bd5832..95b19384c 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -165,7 +165,7 @@ uint256 const retiredTickSize = *getRegisteredFeature("TickSize"); uint256 const retiredFix1368 = *getRegisteredFeature("fix1368"); uint256 const retiredEscrow = *getRegisteredFeature("Escrow"); uint256 const featureCryptoConditionsSuite = *getRegisteredFeature("CryptoConditionsSuite"); -uint256 const fix1373 = *getRegisteredFeature("fix1373"); +uint256 const retiredFix1373 = *getRegisteredFeature("fix1373"); uint256 const featureEnforceInvariants = *getRegisteredFeature("EnforceInvariants"); uint256 const featureSortedDirectories = *getRegisteredFeature("SortedDirectories"); uint256 const fix1201 = *getRegisteredFeature("fix1201"); diff --git a/src/test/app/CrossingLimits_test.cpp b/src/test/app/CrossingLimits_test.cpp index 6f8a304fd..f25cb02dc 100644 --- a/src/test/app/CrossingLimits_test.cpp +++ b/src/test/app/CrossingLimits_test.cpp @@ -528,9 +528,8 @@ public: }; using namespace jtx; auto const sa = supported_amendments(); - testAll(sa - fix1373 - featureFlowCross); - testAll(sa - featureFlowCross); - testAll(sa ); + testAll(sa - featureFlowCross); + testAll(sa ); } }; diff --git a/src/test/app/DeliverMin_test.cpp b/src/test/app/DeliverMin_test.cpp index af05d7f79..6309061c8 100644 --- a/src/test/app/DeliverMin_test.cpp +++ b/src/test/app/DeliverMin_test.cpp @@ -112,8 +112,7 @@ public: { using namespace jtx; auto const sa = supported_amendments(); - test_convert_all_of_an_asset(sa - fix1373 - featureFlowCross); - test_convert_all_of_an_asset(sa - featureFlowCross); + test_convert_all_of_an_asset(sa - featureFlowCross); test_convert_all_of_an_asset(sa); } }; diff --git a/src/test/app/Discrepancy_test.cpp b/src/test/app/Discrepancy_test.cpp index 999b00e43..2e6223d7a 100644 --- a/src/test/app/Discrepancy_test.cpp +++ b/src/test/app/Discrepancy_test.cpp @@ -146,8 +146,7 @@ public: { using namespace test::jtx; auto const sa = supported_amendments(); - testXRPDiscrepancy (sa - fix1373 - featureFlowCross); - testXRPDiscrepancy (sa - featureFlowCross); + testXRPDiscrepancy (sa - featureFlowCross); testXRPDiscrepancy (sa); } }; diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp index 31e73ac7a..094ab8d91 100644 --- a/src/test/app/Flow_test.cpp +++ b/src/test/app/Flow_test.cpp @@ -1304,8 +1304,7 @@ struct Flow_test : public beast::unit_test::suite using namespace jtx; auto const sa = supported_amendments(); - testWithFeats(sa - fix1373 - featureFlowCross); - testWithFeats(sa - featureFlowCross); + testWithFeats(sa - featureFlowCross); testWithFeats(sa); testEmptyStrand(sa); } @@ -1317,16 +1316,13 @@ struct Flow_manual_test : public Flow_test { using namespace jtx; auto const all = supported_amendments(); - FeatureBitset const f1373{fix1373}; FeatureBitset const flowCross{featureFlowCross}; FeatureBitset const f1513{fix1513}; - testWithFeats(all - f1373 - flowCross - f1513); - testWithFeats(all - f1373 - flowCross ); - testWithFeats(all - flowCross - f1513); - testWithFeats(all - flowCross ); - testWithFeats(all - f1513); - testWithFeats(all ); + testWithFeats(all - flowCross - f1513); + testWithFeats(all - flowCross ); + testWithFeats(all - f1513); + testWithFeats(all ); testEmptyStrand(all - f1513); testEmptyStrand(all ); diff --git a/src/test/app/Freeze_test.cpp b/src/test/app/Freeze_test.cpp index 9b6cf5f82..d87d97967 100644 --- a/src/test/app/Freeze_test.cpp +++ b/src/test/app/Freeze_test.cpp @@ -545,8 +545,7 @@ public: }; using namespace test::jtx; auto const sa = supported_amendments(); - testAll(sa - fix1373 - featureFlowCross); - testAll(sa - featureFlowCross); + testAll(sa - featureFlowCross); testAll(sa); } }; diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index 97596f1d9..9e69beede 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -4602,14 +4602,13 @@ public: { using namespace jtx; FeatureBitset const all{supported_amendments()}; - FeatureBitset const f1373{fix1373}; FeatureBitset const flowCross{featureFlowCross}; FeatureBitset const takerDryOffer{fixTakerDryOfferRemoval}; - testAll(all - f1373 - takerDryOffer); - testAll(all - flowCross - takerDryOffer); - testAll(all - flowCross ); - testAll(all ); + testAll(all - takerDryOffer); + testAll(all - flowCross - takerDryOffer); + testAll(all - flowCross ); + testAll(all ); testFalseAssert(); } }; @@ -4620,21 +4619,16 @@ class Offer_manual_test : public Offer_test { using namespace jtx; FeatureBitset const all{supported_amendments()}; - FeatureBitset const f1373{fix1373}; FeatureBitset const flowCross{featureFlowCross}; FeatureBitset const f1513{fix1513}; FeatureBitset const takerDryOffer{fixTakerDryOfferRemoval}; - testAll(all - f1373 - flowCross - f1513); - testAll(all - f1373 - flowCross ); - testAll(all - f1373 - f1513); - testAll(all - f1373 ); - testAll(all - flowCross - f1513); - testAll(all - flowCross ); - testAll(all - f1513); - testAll(all ); + testAll(all - flowCross - f1513); + testAll(all - flowCross ); + testAll(all - f1513); + testAll(all ); - testAll(all - f1373 - flowCross - takerDryOffer); + testAll(all - flowCross - takerDryOffer); } }; diff --git a/src/test/app/PayStrand_test.cpp b/src/test/app/PayStrand_test.cpp index 59f8967e1..9ab03de2f 100644 --- a/src/test/app/PayStrand_test.cpp +++ b/src/test/app/PayStrand_test.cpp @@ -1050,7 +1050,6 @@ struct PayStrand_test : public beast::unit_test::suite auto const USD = gw["USD"]; auto const EUR = gw["EUR"]; - if (features[fix1373]) { Env env(*this, features); env.fund(XRP(10000), alice, bob, gw); @@ -1147,17 +1146,12 @@ struct PayStrand_test : public beast::unit_test::suite env(offer(bob, XRP(100), USD(100)), txflags(tfPassive)); env(offer(bob, USD(100), XRP(100)), txflags(tfPassive)); - auto const expectedResult = [&] () -> TER { - if (!features[fix1373]) - return tesSUCCESS; - return temBAD_PATH_LOOP; - }(); // payment path: USD -> USD/XRP -> XRP/USD env(pay(alice, carol, USD(100)), sendmax(USD(100)), path(~XRP, ~USD), txflags(tfNoRippleDirect), - ter(expectedResult)); + ter(temBAD_PATH_LOOP)); } { Env env(*this, features); @@ -1245,16 +1239,13 @@ struct PayStrand_test : public beast::unit_test::suite { using namespace jtx; auto const sa = supported_amendments(); - testToStrand(sa - fix1373 - featureFlowCross); - testToStrand(sa - featureFlowCross); + testToStrand(sa - featureFlowCross); testToStrand(sa); - testRIPD1373(sa - fix1373 - featureFlowCross); - testRIPD1373(sa - featureFlowCross); + testRIPD1373(sa - featureFlowCross); testRIPD1373(sa); - testLoop(sa - fix1373 - featureFlowCross); - testLoop(sa - featureFlowCross); + testLoop(sa - featureFlowCross); testLoop(sa); testNoAccount(sa); diff --git a/src/test/app/SetAuth_test.cpp b/src/test/app/SetAuth_test.cpp index f9a213486..6b4a2120b 100644 --- a/src/test/app/SetAuth_test.cpp +++ b/src/test/app/SetAuth_test.cpp @@ -70,8 +70,7 @@ struct SetAuth_test : public beast::unit_test::suite { using namespace jtx; auto const sa = supported_amendments(); - testAuth(sa - fix1373 - featureFlowCross); - testAuth(sa - featureFlowCross); + testAuth(sa - featureFlowCross); testAuth(sa); } }; diff --git a/src/test/app/TrustAndBalance_test.cpp b/src/test/app/TrustAndBalance_test.cpp index bc65dec2a..06215eafe 100644 --- a/src/test/app/TrustAndBalance_test.cpp +++ b/src/test/app/TrustAndBalance_test.cpp @@ -510,8 +510,7 @@ public: using namespace test::jtx; auto const sa = supported_amendments(); - testWithFeatures(sa - fix1373 - featureFlowCross); - testWithFeatures(sa -featureFlowCross); + testWithFeatures(sa -featureFlowCross); testWithFeatures(sa); } }; diff --git a/src/test/ledger/BookDirs_test.cpp b/src/test/ledger/BookDirs_test.cpp index 2df04bf97..15f556e53 100644 --- a/src/test/ledger/BookDirs_test.cpp +++ b/src/test/ledger/BookDirs_test.cpp @@ -95,8 +95,7 @@ struct BookDirs_test : public beast::unit_test::suite { using namespace jtx; auto const sa = supported_amendments(); - test_bookdir(sa - fix1373 - featureFlowCross); - test_bookdir(sa - featureFlowCross); + test_bookdir(sa - featureFlowCross); test_bookdir(sa); } }; diff --git a/src/test/ledger/PaymentSandbox_test.cpp b/src/test/ledger/PaymentSandbox_test.cpp index 2f0ed7ce4..56bd189d9 100644 --- a/src/test/ledger/PaymentSandbox_test.cpp +++ b/src/test/ledger/PaymentSandbox_test.cpp @@ -387,8 +387,7 @@ public: }; using namespace jtx; auto const sa = supported_amendments(); - testAll(sa - fix1373 - featureFlowCross); - testAll(sa - featureFlowCross); + testAll(sa - featureFlowCross); testAll(sa); } }; diff --git a/src/test/rpc/GatewayBalances_test.cpp b/src/test/rpc/GatewayBalances_test.cpp index 0f09021bf..96b786fb7 100644 --- a/src/test/rpc/GatewayBalances_test.cpp +++ b/src/test/rpc/GatewayBalances_test.cpp @@ -154,8 +154,7 @@ public: { using namespace jtx; auto const sa = supported_amendments(); - testGWB(sa - fix1373 - featureFlowCross); - testGWB(sa - featureFlowCross); + testGWB(sa - featureFlowCross); testGWB(sa); } }; diff --git a/src/test/rpc/NoRipple_test.cpp b/src/test/rpc/NoRipple_test.cpp index 71c62b687..cec32b2cb 100644 --- a/src/test/rpc/NoRipple_test.cpp +++ b/src/test/rpc/NoRipple_test.cpp @@ -269,8 +269,7 @@ public: }; using namespace jtx; auto const sa = supported_amendments(); - withFeatsTests(sa - fix1373 - featureFlowCross); - withFeatsTests(sa - featureFlowCross); + withFeatsTests(sa - featureFlowCross); withFeatsTests(sa); } };