diff --git a/include/xrpl/protocol/Feature.h b/include/xrpl/protocol/Feature.h index a2eb7d8e50..304cac5bc6 100644 --- a/include/xrpl/protocol/Feature.h +++ b/include/xrpl/protocol/Feature.h @@ -55,6 +55,11 @@ * `VoteBehavior::DefaultYes`. The communication process is beyond * the scope of these instructions. * + * 5) A feature marked as Obsolete can mean either: + * 1) It is in the ledger (marked as Supported::yes) and it is on its way to + * become Retired + * 2) The feature is not in the ledger (has always been marked as + * Supported::no) and the code to support it has been removed * * When a feature has been enabled for several years, the conditional code * may be removed, and the feature "retired". To retire a feature: diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 1be0af5d01..2442abef7f 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -114,7 +114,6 @@ XRPL_FEATURE(DepositAuth, Supported::yes, VoteBehavior::DefaultYe XRPL_FIX (1513, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(FlowCross, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYes) -XRPL_FEATURE(OwnerPaysFee, Supported::no, VoteBehavior::DefaultNo) // The following amendments are obsolete, but must remain supported // because they could potentially get enabled. @@ -131,6 +130,8 @@ XRPL_FIX (NFTokenNegOffer, Supported::yes, VoteBehavior::Obsolete) XRPL_FIX (NFTokenDirV1, Supported::yes, VoteBehavior::Obsolete) XRPL_FEATURE(NonFungibleTokensV1, Supported::yes, VoteBehavior::Obsolete) XRPL_FEATURE(CryptoConditionsSuite, Supported::yes, VoteBehavior::Obsolete) +// This sits here temporarily and will be moved to another section soon +XRPL_FEATURE(OwnerPaysFee, Supported::no, VoteBehavior::Obsolete) // The following amendments have been active for at least two years. Their // pre-amendment code has been removed and the identifiers are deprecated. diff --git a/src/libxrpl/protocol/Feature.cpp b/src/libxrpl/protocol/Feature.cpp index eeeee1c185..e6442d2663 100644 --- a/src/libxrpl/protocol/Feature.cpp +++ b/src/libxrpl/protocol/Feature.cpp @@ -254,7 +254,7 @@ FeatureCollections::registerFeature( { check(!readOnly, "Attempting to register a feature after startup."); check( - support == Supported::yes || vote == VoteBehavior::DefaultNo, + support == Supported::yes || vote != VoteBehavior::DefaultYes, "Invalid feature parameters. Must be supported to be up-voted."); Feature const* i = getByName(name); if (!i) @@ -268,7 +268,7 @@ FeatureCollections::registerFeature( features.emplace_back(name, f); auto const getAmendmentSupport = [=]() { - if (vote == VoteBehavior::Obsolete) + if (vote == VoteBehavior::Obsolete && support == Supported::yes) return AmendmentSupport::Retired; return support == Supported::yes ? AmendmentSupport::Supported : AmendmentSupport::Unsupported; diff --git a/src/test/app/AMMExtended_test.cpp b/src/test/app/AMMExtended_test.cpp index 3d959a6a09..70b2f30e1d 100644 --- a/src/test/app/AMMExtended_test.cpp +++ b/src/test/app/AMMExtended_test.cpp @@ -1183,9 +1183,7 @@ private: using namespace jtx; - // The problem was identified when featureOwnerPaysFee was enabled, - // so make sure that gets included. - Env env{*this, features | featureOwnerPaysFee}; + Env env{*this, features}; // The fee that's charged for transactions. auto const fee = env.current()->fees().base; @@ -2217,271 +2215,6 @@ private: } } - void - testTransferRate(FeatureBitset features) - { - testcase("Transfer Rate"); - - using namespace jtx; - - { - // transfer fee on AMM - Env env(*this, features); - - fund(env, gw, {alice, bob, carol}, XRP(10'000), {USD(1'000)}); - env(rate(gw, 1.25)); - env.close(); - - AMM ammBob(env, bob, XRP(100), USD(150)); - // no transfer fee on create - BEAST_EXPECT(expectLine(env, bob, USD(1000 - 150))); - - env(pay(alice, carol, USD(50)), path(~USD), sendmax(XRP(50))); - env.close(); - - BEAST_EXPECT(expectLine(env, bob, USD(1'000 - 150))); - BEAST_EXPECT( - ammBob.expectBalances(XRP(150), USD(100), ammBob.tokens())); - BEAST_EXPECT(expectLedgerEntryRoot( - env, alice, xrpMinusFee(env, 10'000 - 50))); - BEAST_EXPECT(expectLine(env, carol, USD(1'050))); - } - - { - // Transfer fee AMM and offer - Env env(*this, features); - - fund( - env, - gw, - {alice, bob, carol}, - XRP(10'000), - {USD(1'000), EUR(1'000)}); - env(rate(gw, 1.25)); - env.close(); - - AMM ammBob(env, bob, XRP(100), USD(140)); - BEAST_EXPECT(expectLine(env, bob, USD(1'000 - 140))); - - env(offer(bob, USD(50), EUR(50))); - - // alice buys 40EUR with 40XRP - env(pay(alice, carol, EUR(40)), path(~USD, ~EUR), sendmax(XRP(40))); - - // 40XRP is swapped in for 40USD - BEAST_EXPECT( - ammBob.expectBalances(XRP(140), USD(100), ammBob.tokens())); - // 40USD buys 40EUR via bob's offer. 40EUR delivered to carol - // and bob pays 25% on 40EUR, 40EUR*0.25=10EUR - BEAST_EXPECT(expectLine(env, bob, EUR(1'000 - 40 - 40 * 0.25))); - // bob gets 40USD back from the offer - BEAST_EXPECT(expectLine(env, bob, USD(1'000 - 140 + 40))); - BEAST_EXPECT(expectLedgerEntryRoot( - env, alice, xrpMinusFee(env, 10'000 - 40))); - BEAST_EXPECT(expectLine(env, carol, EUR(1'040))); - BEAST_EXPECT(expectOffers(env, bob, 1, {{USD(10), EUR(10)}})); - } - - { - // Transfer fee two consecutive AMM - Env env(*this, features); - - fund( - env, - gw, - {alice, bob, carol}, - XRP(10'000), - {USD(1'000), EUR(1'000)}); - env(rate(gw, 1.25)); - env.close(); - - AMM ammBobXRP_USD(env, bob, XRP(100), USD(140)); - BEAST_EXPECT(expectLine(env, bob, USD(1'000 - 140))); - - AMM ammBobUSD_EUR(env, bob, USD(100), EUR(140)); - BEAST_EXPECT(expectLine(env, bob, EUR(1'000 - 140))); - BEAST_EXPECT(expectLine(env, bob, USD(1'000 - 140 - 100))); - - // alice buys 40EUR with 40XRP - env(pay(alice, carol, EUR(40)), path(~USD, ~EUR), sendmax(XRP(40))); - - // 40XRP is swapped in for 40USD - BEAST_EXPECT(ammBobXRP_USD.expectBalances( - XRP(140), USD(100), ammBobXRP_USD.tokens())); - // 40USD is swapped in for 40EUR - BEAST_EXPECT(ammBobUSD_EUR.expectBalances( - USD(140), EUR(100), ammBobUSD_EUR.tokens())); - // no other charges on bob - BEAST_EXPECT(expectLine(env, bob, USD(1'000 - 140 - 100))); - BEAST_EXPECT(expectLine(env, bob, EUR(1'000 - 140))); - BEAST_EXPECT(expectLedgerEntryRoot( - env, alice, xrpMinusFee(env, 10'000 - 40))); - BEAST_EXPECT(expectLine(env, carol, EUR(1'040))); - } - - { - // Payment via AMM with limit quality, deliver less - // than requested - Env env(*this, features); - - fund( - env, - gw, - {alice, bob, carol}, - XRP(1'000), - {USD(1'200), GBP(1'200)}); - env(rate(gw, 1.25)); - env.close(); - - AMM amm(env, bob, GBP(1'000), USD(1'100)); - - // requested quality limit is 90USD/110GBP = 0.8181 - // trade quality is 77.2727USD/94.4444GBP = 0.8181 - env(pay(alice, carol, USD(90)), - path(~USD), - sendmax(GBP(110)), - txflags(tfNoRippleDirect | tfPartialPayment | tfLimitQuality)); - env.close(); - - if (!features[fixAMMv1_1]) - { - // alice buys 77.2727USD with 75.5555GBP and pays 25% tr fee - // on 75.5555GBP - // 1,200 - 75.55555*1.25 = 1200 - 94.4444 = 1105.55555GBP - BEAST_EXPECT(expectLine( - env, - alice, - STAmount{GBP, UINT64_C(1'105'555555555555), -12})); - // 75.5555GBP is swapped in for 77.7272USD - BEAST_EXPECT(amm.expectBalances( - STAmount{GBP, UINT64_C(1'075'555555555556), -12}, - STAmount{USD, UINT64_C(1'022'727272727272), -12}, - amm.tokens())); - } - else - { - // alice buys 77.2727USD with 75.5555GBP and pays 25% tr fee - // on 75.5555GBP - // 1,200 - 75.55555*1.25 = 1200 - 94.4444 = 1105.55555GBP - BEAST_EXPECT(expectLine( - env, - alice, - STAmount{GBP, UINT64_C(1'105'555555555554), -12})); - // 75.5555GBP is swapped in for 77.7272USD - BEAST_EXPECT(amm.expectBalances( - STAmount{GBP, UINT64_C(1'075'555555555557), -12}, - STAmount{USD, UINT64_C(1'022'727272727272), -12}, - amm.tokens())); - } - BEAST_EXPECT(expectLine( - env, carol, STAmount{USD, UINT64_C(1'277'272727272728), -12})); - } - - { - // AMM offer crossing - Env env(*this, features); - - fund(env, gw, {alice, bob}, XRP(1'000), {USD(1'200), EUR(1'200)}); - env(rate(gw, 1.25)); - env.close(); - - AMM amm(env, bob, USD(1'000), EUR(1'150)); - - env(offer(alice, EUR(100), USD(100))); - env.close(); - - if (!features[fixAMMv1_1]) - { - // 95.2380USD is swapped in for 100EUR - BEAST_EXPECT(amm.expectBalances( - STAmount{USD, UINT64_C(1'095'238095238095), -12}, - EUR(1'050), - amm.tokens())); - // alice pays 25% tr fee on 95.2380USD - // 1200-95.2380*1.25 = 1200 - 119.0477 = 1080.9523USD - BEAST_EXPECT(expectLine( - env, - alice, - STAmount{USD, UINT64_C(1'080'952380952381), -12}, - EUR(1'300))); - } - else - { - // 95.2380USD is swapped in for 100EUR - BEAST_EXPECT(amm.expectBalances( - STAmount{USD, UINT64_C(1'095'238095238096), -12}, - EUR(1'050), - amm.tokens())); - // alice pays 25% tr fee on 95.2380USD - // 1200-95.2380*1.25 = 1200 - 119.0477 = 1080.9523USD - BEAST_EXPECT(expectLine( - env, - alice, - STAmount{USD, UINT64_C(1'080'95238095238), -11}, - EUR(1'300))); - } - BEAST_EXPECT(expectOffers(env, alice, 0)); - } - - { - // First pass through a strand redeems, second pass issues, - // through an offer limiting step is not an endpoint - Env env(*this, features); - auto const USDA = alice["USD"]; - auto const USDB = bob["USD"]; - Account const dan("dan"); - - env.fund(XRP(10'000), bob, carol, dan, gw); - fund(env, {alice}, XRP(10'000)); - env(rate(gw, 1.25)); - env.trust(USD(2'000), alice, bob, carol, dan); - env.trust(EUR(2'000), carol, dan); - env.trust(USDA(1'000), bob); - env.trust(USDB(1'000), gw); - env(pay(gw, bob, USD(50))); - env(pay(gw, dan, EUR(1'050))); - env(pay(gw, dan, USD(1'000))); - AMM ammDan(env, dan, USD(1'000), EUR(1'050)); - - if (!features[fixAMMv1_1]) - { - // alice -> bob -> gw -> carol. $50 should have transfer fee; - // $10, no fee - env(pay(alice, carol, EUR(50)), - path(bob, gw, ~EUR), - sendmax(USDA(60)), - txflags(tfNoRippleDirect)); - BEAST_EXPECT(ammDan.expectBalances( - USD(1'050), EUR(1'000), ammDan.tokens())); - BEAST_EXPECT(expectLine(env, dan, USD(0))); - BEAST_EXPECT(expectLine(env, dan, EUR(0))); - BEAST_EXPECT(expectLine(env, bob, USD(-10))); - BEAST_EXPECT(expectLine(env, bob, USDA(60))); - BEAST_EXPECT(expectLine(env, carol, EUR(50))); - } - else - { - // alice -> bob -> gw -> carol. $50 should have transfer fee; - // $10, no fee - env(pay(alice, carol, EUR(50)), - path(bob, gw, ~EUR), - sendmax(USDA(60.1)), - txflags(tfNoRippleDirect)); - BEAST_EXPECT(ammDan.expectBalances( - STAmount{USD, UINT64_C(1'050'000000000001), -12}, - EUR(1'000), - ammDan.tokens())); - BEAST_EXPECT(expectLine(env, dan, USD(0))); - BEAST_EXPECT(expectLine(env, dan, EUR(0))); - BEAST_EXPECT(expectLine( - env, bob, STAmount{USD, INT64_C(-10'000000000001), -12})); - BEAST_EXPECT(expectLine( - env, bob, STAmount{USDA, UINT64_C(60'000000000001), -12})); - BEAST_EXPECT(expectLine(env, carol, EUR(50))); - } - } - } - void testTransferRateNoOwnerFee(FeatureBitset features) { @@ -4057,13 +3790,9 @@ private: { using namespace jtx; FeatureBitset const all{supported_amendments()}; - FeatureBitset const ownerPaysFee{featureOwnerPaysFee}; testFalseDry(all); testBookStep(all); - testBookStep(all | ownerPaysFee); - testTransferRate(all | ownerPaysFee); - testTransferRate((all - fixAMMv1_1 - fixAMMv1_3) | ownerPaysFee); testTransferRateNoOwnerFee(all); testTransferRateNoOwnerFee(all - fixAMMv1_1 - fixAMMv1_3); testLimitQuality(); diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp index d0b8686db6..68485f4eee 100644 --- a/src/test/app/Flow_test.cpp +++ b/src/test/app/Flow_test.cpp @@ -599,158 +599,18 @@ struct Flow_test : public beast::unit_test::suite Account const bob("bob"); Account const carol("carol"); - { - // Simple payment through a gateway with a - // transfer rate - Env env(*this, features); + // Offer where the owner is also the issuer, sender pays fee + Env env(*this, features); - env.fund(XRP(10000), alice, bob, carol, gw); - env.close(); - env(rate(gw, 1.25)); - env.trust(USD(1000), alice, bob, carol); - env(pay(gw, alice, USD(50))); - env.require(balance(alice, USD(50))); - env(pay(alice, bob, USD(40)), sendmax(USD(50))); - env.require(balance(bob, USD(40)), balance(alice, USD(0))); - } - { - // transfer rate is not charged when issuer is src or dst - Env env(*this, features); - - env.fund(XRP(10000), alice, bob, carol, gw); - env.close(); - env(rate(gw, 1.25)); - env.trust(USD(1000), alice, bob, carol); - env(pay(gw, alice, USD(50))); - env.require(balance(alice, USD(50))); - env(pay(alice, gw, USD(40)), sendmax(USD(40))); - env.require(balance(alice, USD(10))); - } - { - // transfer fee on an offer - Env env(*this, features); - - env.fund(XRP(10000), alice, bob, carol, gw); - env.close(); - env(rate(gw, 1.25)); - env.trust(USD(1000), alice, bob, carol); - env(pay(gw, bob, USD(65))); - - env(offer(bob, XRP(50), USD(50))); - - env(pay(alice, carol, USD(50)), path(~USD), sendmax(XRP(50))); - env.require( - balance(alice, xrpMinusFee(env, 10000 - 50)), - balance(bob, USD(2.5)), // owner pays transfer fee - balance(carol, USD(50))); - } - - { - // Transfer fee two consecutive offers - Env env(*this, features); - - env.fund(XRP(10000), alice, bob, carol, gw); - env.close(); - env(rate(gw, 1.25)); - env.trust(USD(1000), alice, bob, carol); - env.trust(EUR(1000), alice, bob, carol); - env(pay(gw, bob, USD(50))); - env(pay(gw, bob, EUR(50))); - - env(offer(bob, XRP(50), USD(50))); - env(offer(bob, USD(50), EUR(50))); - - env(pay(alice, carol, EUR(40)), path(~USD, ~EUR), sendmax(XRP(40))); - env.require( - balance(alice, xrpMinusFee(env, 10000 - 40)), - balance(bob, USD(40)), - balance(bob, EUR(0)), - balance(carol, EUR(40))); - } - - { - // First pass through a strand redeems, second pass issues, no - // offers limiting step is not an endpoint - Env env(*this, features); - auto const USDA = alice["USD"]; - auto const USDB = bob["USD"]; - - env.fund(XRP(10000), alice, bob, carol, gw); - env.close(); - env(rate(gw, 1.25)); - env.trust(USD(1000), alice, bob, carol); - env.trust(USDA(1000), bob); - env.trust(USDB(1000), gw); - env(pay(gw, bob, USD(50))); - // alice -> bob -> gw -> carol. $50 should have transfer fee; $10, - // no fee - env(pay(alice, carol, USD(50)), path(bob), sendmax(USDA(60))); - env.require( - balance(bob, USD(-10)), - balance(bob, USDA(60)), - balance(carol, USD(50))); - } - { - // First pass through a strand redeems, second pass issues, through - // an offer limiting step is not an endpoint - Env env(*this, features); - auto const USDA = alice["USD"]; - auto const USDB = bob["USD"]; - Account const dan("dan"); - - env.fund(XRP(10000), alice, bob, carol, dan, gw); - env.close(); - env(rate(gw, 1.25)); - env.trust(USD(1000), alice, bob, carol, dan); - env.trust(EUR(1000), carol, dan); - env.trust(USDA(1000), bob); - env.trust(USDB(1000), gw); - env(pay(gw, bob, USD(50))); - env(pay(gw, dan, EUR(100))); - env(offer(dan, USD(100), EUR(100))); - // alice -> bob -> gw -> carol. $50 should have transfer fee; $10, - // no fee - env(pay(alice, carol, EUR(50)), - path(bob, gw, ~EUR), - sendmax(USDA(60)), - txflags(tfNoRippleDirect)); - env.require( - balance(bob, USD(-10)), - balance(bob, USDA(60)), - balance(dan, USD(50)), - balance(dan, EUR(37.5)), - balance(carol, EUR(50))); - } - - { - // Offer where the owner is also the issuer, owner pays fee - Env env(*this, features); - - env.fund(XRP(10000), alice, bob, gw); - env.close(); - env(rate(gw, 1.25)); - env.trust(USD(1000), alice, bob); - env(offer(gw, XRP(100), USD(100))); - env(pay(alice, bob, USD(100)), sendmax(XRP(100))); - env.require( - balance(alice, xrpMinusFee(env, 10000 - 100)), - balance(bob, USD(100))); - } - if (!features[featureOwnerPaysFee]) - { - // Offer where the owner is also the issuer, sender pays fee - Env env(*this, features); - - env.fund(XRP(10000), alice, bob, gw); - env.close(); - env(rate(gw, 1.25)); - env.trust(USD(1000), alice, bob); - env(offer(gw, XRP(125), USD(125))); - env(pay(alice, bob, USD(100)), sendmax(XRP(200))); - env.require( - balance(alice, xrpMinusFee(env, 10000 - 125)), - balance(bob, USD(100))); - } + env.fund(XRP(10000), alice, bob, gw); + env.close(); + env(rate(gw, 1.25)); + env.trust(USD(1000), alice, bob); + env(offer(gw, XRP(125), USD(125))); + env(pay(alice, bob, USD(100)), sendmax(XRP(200))); + env.require( + balance(alice, xrpMinusFee(env, 10000 - 125)), + balance(bob, USD(100))); } void @@ -1445,7 +1305,6 @@ struct Flow_test : public beast::unit_test::suite testWithFeats(FeatureBitset features) { using namespace jtx; - FeatureBitset const ownerPaysFee{featureOwnerPaysFee}; FeatureBitset const reducedOffersV2(fixReducedOffersV2); testLineQuality(features); @@ -1453,9 +1312,7 @@ struct Flow_test : public beast::unit_test::suite testBookStep(features - reducedOffersV2); testDirectStep(features); testBookStep(features); - testDirectStep(features | ownerPaysFee); - testBookStep(features | ownerPaysFee); - testTransferRate(features | ownerPaysFee); + testTransferRate(features); testSelfPayment1(features); testSelfPayment2(features); testSelfFundedXRPEndpoint(false, features); diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index 0891b27df8..d3481881c4 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -3643,9 +3643,7 @@ public: using namespace jtx; - // The problem was identified when featureOwnerPaysFee was enabled, - // so make sure that gets included. - Env env{*this, features | featureOwnerPaysFee}; + Env env{*this, features}; // The fee that's charged for transactions. auto const fee = env.current()->fees().base; diff --git a/src/test/app/TheoreticalQuality_test.cpp b/src/test/app/TheoreticalQuality_test.cpp index 1b3e6d9a82..dcffd810ed 100644 --- a/src/test/app/TheoreticalQuality_test.cpp +++ b/src/test/app/TheoreticalQuality_test.cpp @@ -264,7 +264,7 @@ class TheoreticalQuality_test : public beast::unit_test::suite sendMaxIssue, rcp.paths, /*defaultPaths*/ rcp.paths.empty(), - sb.rules().enabled(featureOwnerPaysFee), + false, OfferCrossing::no, ammContext, std::nullopt, diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index 40de395a71..06697f80c1 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -139,7 +139,8 @@ class Feature_test : public beast::unit_test::suite // Test a random sampling of the variables. If any of these get retired // or removed, swap out for any other feature. - BEAST_EXPECT(featureToName(featureOwnerPaysFee) == "OwnerPaysFee"); + BEAST_EXPECT( + featureToName(fixTrustLinesToSelf) == "fixTrustLinesToSelf"); BEAST_EXPECT(featureToName(featureFlow) == "Flow"); BEAST_EXPECT(featureToName(featureNegativeUNL) == "NegativeUNL"); BEAST_EXPECT(featureToName(fix1578) == "fix1578"); diff --git a/src/xrpld/app/paths/RippleCalc.cpp b/src/xrpld/app/paths/RippleCalc.cpp index 4e472e07c8..9c438bdfa9 100644 --- a/src/xrpld/app/paths/RippleCalc.cpp +++ b/src/xrpld/app/paths/RippleCalc.cpp @@ -95,9 +95,6 @@ RippleCalc::rippleCalculate( return std::nullopt; }(); - bool const ownerPaysTransferFee = - view.rules().enabled(featureOwnerPaysFee); - try { flowOut = flow( @@ -108,7 +105,7 @@ RippleCalc::rippleCalculate( spsPaths, defaultPaths, partialPayment, - ownerPaysTransferFee, + false, OfferCrossing::no, limitQuality, sendMax,