ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-19 18:45:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove OwnerPaysFee as it's never fully supported (#5435)

Browse Source 
The OwnerPaysFee amendment was never fully supported, and this change removes the feature to the extent possible.
This commit is contained in:
Jingchen
2025-06-24 19:56:58 +01:00
committed by GitHub
parent 42fd74b77b
commit e9d46f0bfc
9 changed files with 27 additions and 439 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
include/xrpl/protocol/Feature.h
View File

@@ -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:

3
include/xrpl/protocol/detail/features.macro
View File

@@ -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.

4
src/libxrpl/protocol/Feature.cpp
View File

@@ -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;

273
src/test/app/AMMExtended_test.cpp
View File

@@ -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();

167
src/test/app/Flow_test.cpp
View File

@@ -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);

4
src/test/app/Offer_test.cpp
View File

@@ -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;

2
src/test/app/TheoreticalQuality_test.cpp
View File

@@ -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,

3
src/test/rpc/Feature_test.cpp
View File

@@ -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");

5
src/xrpld/app/paths/RippleCalc.cpp
View File

@@ -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,