diff --git a/src/ripple/app/paths/Flow.cpp b/src/ripple/app/paths/Flow.cpp index 3d060fdc6bd..83379d34e79 100644 --- a/src/ripple/app/paths/Flow.cpp +++ b/src/ripple/app/paths/Flow.cpp @@ -65,7 +65,7 @@ flow( bool defaultPaths, bool partialPayment, bool ownerPaysTransferFee, - bool offerCrossing, + OfferCrossing offerCrossing, std::optional const& limitQuality, std::optional const& sendMax, beast::Journal j, diff --git a/src/ripple/app/paths/Flow.h b/src/ripple/app/paths/Flow.h index b692c3bdf07..deafd1c7716 100644 --- a/src/ripple/app/paths/Flow.h +++ b/src/ripple/app/paths/Flow.h @@ -44,7 +44,7 @@ struct FlowDebugInfo; @param partialPayment If the payment cannot deliver the entire requested amount, deliver as much as possible, given the constraints @param ownerPaysTransferFee If true then owner, not sender, pays fee - @param offerCrossing If true then flow is executing offer crossing, not + @param offerCrossing If Yes or Sell then flow is executing offer crossing, not payments @param limitQuality Do not use liquidity below this quality threshold @param sendMax Do not spend more than this amount @@ -62,7 +62,7 @@ flow( bool defaultPaths, bool partialPayment, bool ownerPaysTransferFee, - bool offerCrossing, + OfferCrossing offerCrossing, std::optional const& limitQuality, std::optional const& sendMax, beast::Journal j, diff --git a/src/ripple/app/paths/RippleCalc.cpp b/src/ripple/app/paths/RippleCalc.cpp index 6feb276c625..99c2dd1114f 100644 --- a/src/ripple/app/paths/RippleCalc.cpp +++ b/src/ripple/app/paths/RippleCalc.cpp @@ -106,7 +106,7 @@ RippleCalc::rippleCalculate( defaultPaths, partialPayment, ownerPaysTransferFee, - /* offerCrossing */ false, + OfferCrossing::No, limitQuality, sendMax, j, diff --git a/src/ripple/app/paths/impl/PaySteps.cpp b/src/ripple/app/paths/impl/PaySteps.cpp index 81c358a2bbc..b96d6ee57b2 100644 --- a/src/ripple/app/paths/impl/PaySteps.cpp +++ b/src/ripple/app/paths/impl/PaySteps.cpp @@ -141,7 +141,7 @@ toStrand( std::optional const& sendMaxIssue, STPath const& path, bool ownerPaysTransferFee, - bool offerCrossing, + OfferCrossing offerCrossing, AMMContext& ammContext, beast::Journal j) { @@ -475,7 +475,7 @@ toStrands( STPathSet const& paths, bool addDefaultPath, bool ownerPaysTransferFee, - bool offerCrossing, + OfferCrossing offerCrossing, AMMContext& ammContext, beast::Journal j) { @@ -588,7 +588,7 @@ StrandContext::StrandContext( std::optional const& limitQuality_, bool isLast_, bool ownerPaysTransferFee_, - bool offerCrossing_, + OfferCrossing offerCrossing_, bool isDefaultPath_, std::array, 2>& seenDirectIssues_, boost::container::flat_set& seenBookOuts_, diff --git a/src/ripple/app/paths/impl/Steps.h b/src/ripple/app/paths/impl/Steps.h index 35c465f18be..6a18bcda63e 100644 --- a/src/ripple/app/paths/impl/Steps.h +++ b/src/ripple/app/paths/impl/Steps.h @@ -39,6 +39,7 @@ class AMMContext; enum class DebtDirection { issues, redeems }; enum class QualityDirection { in, out }; enum class StrandDirection { forward, reverse }; +enum OfferCrossing { No = 0, Yes = 1, Sell = 2 }; inline bool redeems(DebtDirection dir) @@ -398,7 +399,7 @@ toStrand( std::optional const& sendMaxIssue, STPath const& path, bool ownerPaysTransferFee, - bool offerCrossing, + OfferCrossing offerCrossing, AMMContext& ammContext, beast::Journal j); @@ -438,7 +439,7 @@ toStrands( STPathSet const& paths, bool addDefaultPath, bool ownerPaysTransferFee, - bool offerCrossing, + OfferCrossing offerCrossing, AMMContext& ammContext, beast::Journal j); @@ -531,9 +532,10 @@ struct StrandContext bool const isFirst; ///< true if Step is first in Strand bool const isLast = false; ///< true if Step is last in Strand bool const ownerPaysTransferFee; ///< true if owner, not sender, pays fee - bool const offerCrossing; ///< true if offer crossing, not payment - bool const isDefaultPath; ///< true if Strand is default path - size_t const strandSize; ///< Length of Strand + OfferCrossing const + offerCrossing; ///< Yes/Sell if offer crossing, not payment + bool const isDefaultPath; ///< true if Strand is default path + size_t const strandSize; ///< Length of Strand /** The previous step in the strand. Needed to check the no ripple constraint */ @@ -563,7 +565,7 @@ struct StrandContext std::optional const& limitQuality_, bool isLast_, bool ownerPaysTransferFee_, - bool offerCrossing_, + OfferCrossing offerCrossing_, bool isDefaultPath_, std::array, 2>& seenDirectIssues_, ///< For detecting currency loops diff --git a/src/ripple/app/paths/impl/StrandFlow.h b/src/ripple/app/paths/impl/StrandFlow.h index 225f8ef857c..4a0dc00478f 100644 --- a/src/ripple/app/paths/impl/StrandFlow.h +++ b/src/ripple/app/paths/impl/StrandFlow.h @@ -557,7 +557,7 @@ flow( std::vector const& strands, TOutAmt const& outReq, bool partialPayment, - bool offerCrossing, + OfferCrossing offerCrossing, std::optional const& limitQuality, std::optional const& sendMaxST, beast::Journal j, @@ -828,23 +828,7 @@ flow( * even if it means obtaining more than TakerPays. Since the pays and gets * are reversed, the owner must send the entire TakerGets. */ - bool const fixFillOrKill = - baseView.rules().enabled(fixFillOrKillOnFlowCross); - // tfSell is handled by setting the deliver amount to max - auto const sell = [&]() -> bool { - if constexpr (std::is_same_v) - { - static auto max = XRPAmount{STAmount::cMaxNative}; - return outReq == max; - } - else if constexpr (std::is_same_v) - { - static auto max = - IOUAmount{STAmount::cMaxValue / 2, STAmount::cMaxOffset}; - return outReq == max; - } - return false; - }(); + bool const fillOrKillEnabled = baseView.rules().enabled(fixFillOrKill); if (actualOut != outReq) { @@ -862,7 +846,8 @@ flow( // fixFillOrKill amendment: // That case is handled here if tfSell is also not set; i.e, // case 1. - if (!offerCrossing || (fixFillOrKill && !sell)) + if (!offerCrossing || + (fillOrKillEnabled && offerCrossing != OfferCrossing::Sell)) return { tecPATH_PARTIAL, actualIn, @@ -874,7 +859,9 @@ flow( return {tecPATH_DRY, std::move(ofrsToRmOnFail)}; } } - if (offerCrossing && (!partialPayment && (!fixFillOrKill || sell))) + if (offerCrossing && + (!partialPayment && + (!fillOrKillEnabled || offerCrossing == OfferCrossing::Sell))) { // If we're offer crossing and partialPayment is *not* true, then // we're handling a FillOrKill offer. In this case remainingIn must diff --git a/src/ripple/app/tx/impl/CashCheck.cpp b/src/ripple/app/tx/impl/CashCheck.cpp index b258ae7d9d8..64cb2735713 100644 --- a/src/ripple/app/tx/impl/CashCheck.cpp +++ b/src/ripple/app/tx/impl/CashCheck.cpp @@ -447,7 +447,7 @@ CashCheck::doApply() true, // default path static_cast(optDeliverMin), // partial payment true, // owner pays transfer fee - false, // offer crossing + OfferCrossing::No, std::nullopt, sleCheck->getFieldAmount(sfSendMax), viewJ); diff --git a/src/ripple/app/tx/impl/CreateOffer.cpp b/src/ripple/app/tx/impl/CreateOffer.cpp index dd01a64b5f2..a6f090a3f51 100644 --- a/src/ripple/app/tx/impl/CreateOffer.cpp +++ b/src/ripple/app/tx/impl/CreateOffer.cpp @@ -736,8 +736,10 @@ CreateOffer::flowCross( } // Special handling for the tfSell flag. STAmount deliver = takerAmount.out; + OfferCrossing offerCrossing = OfferCrossing::Yes; if (txFlags & tfSell) { + offerCrossing = OfferCrossing::Sell; // We are selling, so we will accept *more* than the offer // specified. Since we don't know how much they might offer, // we allow delivery of the largest possible amount. @@ -764,7 +766,7 @@ CreateOffer::flowCross( true, // default path !(txFlags & tfFillOrKill), // partial payment true, // owner pays transfer fee - true, // offer crossing + offerCrossing, threshold, sendMax, j_); diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index e8ef8e3dbe8..da70030f0ba 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -348,7 +348,7 @@ extern uint256 const fixNonFungibleTokensV1_2; extern uint256 const fixNFTokenRemint; extern uint256 const fixReducedOffersV1; extern uint256 const featureClawback; -extern uint256 const fixFillOrKillOnFlowCross; +extern uint256 const fixFillOrKill; } // namespace ripple diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 6ffb2c716dc..299d46bcff5 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -455,7 +455,7 @@ REGISTER_FIX (fixNFTokenRemint, Supported::yes, VoteBehavior::De REGISTER_FIX (fixReducedOffersV1, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FEATURE(Clawback, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FEATURE(AMM, Supported::yes, VoteBehavior::DefaultNo); -REGISTER_FIX(fixFillOrKillOnFlowCross, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FIX(fixFillOrKill, Supported::yes, VoteBehavior::DefaultNo); // The following amendments are obsolete, but must remain supported // because they could potentially get enabled. diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index ad2c1f67805..5dda2e55779 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -2103,7 +2103,7 @@ struct AMMExtended_test : public jtx::AMMTest false, false, true, - false, + OfferCrossing::No, std::nullopt, smax, flowJournal); diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp index 131cad6f042..82b8a89702d 100644 --- a/src/test/app/Flow_test.cpp +++ b/src/test/app/Flow_test.cpp @@ -473,7 +473,7 @@ struct Flow_test : public beast::unit_test::suite false, false, true, - false, + OfferCrossing::No, std::nullopt, smax, flowJournal); diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index b847b9f09d6..d261b834fd2 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -5084,7 +5084,7 @@ class Offer_test : public beast::unit_test::suite void testFillOrKill(FeatureBitset features) { - testcase("fixFillOrKillOnFlowCross"); + testcase("fixFillOrKill"); using namespace jtx; Env env(*this, features); Account const issuer("issuer"); @@ -5115,8 +5115,8 @@ class Offer_test : public beast::unit_test::suite // tfFillOrKill, TakerPays must be filled { - TER const err = features[fixFillOrKillOnFlowCross] || - !features[featureFlowCross] + TER const err = + features[fixFillOrKill] || !features[featureFlowCross] ? TER(tesSUCCESS) : tecKILLED; @@ -5222,7 +5222,7 @@ class Offer_test : public beast::unit_test::suite BEAST_EXPECT(expectOffers(env, taker, 0)); } - // Fail regardless of fixFillOrKillOnFlowCross amendment + // Fail regardless of fixFillOrKill amendment for (auto const flags : {tfFillOrKill, tfFillOrKill + tfSell}) { env(offer(maker, XRP(100), USD(100))); @@ -5344,7 +5344,7 @@ class Offer_test : public beast::unit_test::suite FeatureBitset const takerDryOffer{fixTakerDryOfferRemoval}; FeatureBitset const rmSmallIncreasedQOffers{fixRmSmallIncreasedQOffers}; FeatureBitset const immediateOfferKilled{featureImmediateOfferKilled}; - FeatureBitset const fixFillOrKill{fixFillOrKillOnFlowCross}; + FeatureBitset const fixFillOrKill{fixFillOrKill}; testAll(all - takerDryOffer - immediateOfferKilled); testAll(all - flowCross - takerDryOffer - immediateOfferKilled); diff --git a/src/test/app/PayStrand_test.cpp b/src/test/app/PayStrand_test.cpp index a70ab099745..e934f2b92b3 100644 --- a/src/test/app/PayStrand_test.cpp +++ b/src/test/app/PayStrand_test.cpp @@ -656,7 +656,7 @@ struct PayStrand_test : public beast::unit_test::suite sendMaxIssue, path, true, - false, + OfferCrossing::No, ammContext, env.app().logs().journal("Flow")); BEAST_EXPECT(ter == expTer); @@ -684,7 +684,7 @@ struct PayStrand_test : public beast::unit_test::suite /*sendMaxIssue*/ EUR.issue(), path, true, - false, + OfferCrossing::No, ammContext, env.app().logs().journal("Flow")); (void)_; @@ -701,7 +701,7 @@ struct PayStrand_test : public beast::unit_test::suite /*sendMaxIssue*/ EUR.issue(), path, true, - false, + OfferCrossing::No, ammContext, env.app().logs().journal("Flow")); (void)_; @@ -821,7 +821,7 @@ struct PayStrand_test : public beast::unit_test::suite USD.issue(), STPath(), true, - false, + OfferCrossing::No, ammContext, flowJournal); BEAST_EXPECT(r.first == temBAD_PATH); @@ -837,7 +837,7 @@ struct PayStrand_test : public beast::unit_test::suite std::nullopt, STPath(), true, - false, + OfferCrossing::No, ammContext, flowJournal); BEAST_EXPECT(r.first == temBAD_PATH); @@ -853,7 +853,7 @@ struct PayStrand_test : public beast::unit_test::suite std::nullopt, STPath(), true, - false, + OfferCrossing::No, ammContext, flowJournal); BEAST_EXPECT(r.first == temBAD_PATH); @@ -990,7 +990,7 @@ struct PayStrand_test : public beast::unit_test::suite std::nullopt, STPath(), true, - false, + OfferCrossing::No, ammContext, env.app().logs().journal("Flow")); BEAST_EXPECT(ter == tesSUCCESS); @@ -1017,7 +1017,7 @@ struct PayStrand_test : public beast::unit_test::suite USD.issue(), path, false, - false, + OfferCrossing::No, ammContext, env.app().logs().journal("Flow")); BEAST_EXPECT(ter == tesSUCCESS); diff --git a/src/test/app/TheoreticalQuality_test.cpp b/src/test/app/TheoreticalQuality_test.cpp index 5da26512deb..5b7f7c6b9e7 100644 --- a/src/test/app/TheoreticalQuality_test.cpp +++ b/src/test/app/TheoreticalQuality_test.cpp @@ -266,7 +266,7 @@ class TheoreticalQuality_test : public beast::unit_test::suite rcp.paths, /*defaultPaths*/ rcp.paths.empty(), sb.rules().enabled(featureOwnerPaysFee), - /*offerCrossing*/ false, + OfferCrossing::No, ammContext, dummyJ);