refactor: Retire fix1578 amendment (#5927)

Amendments activated for more than 2 years can be retired. This change retires the fix1578 amendment.
This commit is contained in:
Pratik Mankawde
2025-10-29 16:08:17 +00:00
committed by GitHub
parent bd3bc917f8
commit efa917d9f3
7 changed files with 158 additions and 188 deletions

View File

@@ -106,7 +106,6 @@ XRPL_FIX (CheckThreading, Supported::yes, VoteBehavior::DefaultYe
XRPL_FIX (MasterKeyAsRegularKey, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FIX (TakerDryOfferRemoval, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(MultiSignReserve, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FIX (1578, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(DepositPreauth, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FIX (1623, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(Checks, Supported::yes, VoteBehavior::DefaultYes)
@@ -154,3 +153,4 @@ XRPL_RETIRE(fix1515)
XRPL_RETIRE(fix1543)
XRPL_RETIRE(fix1781)
XRPL_RETIRE(fix1571)
XRPL_RETIRE(fix1578)

View File

@@ -198,109 +198,100 @@ private:
// Fill or Kill - unless we fully cross, just charge a fee and don't
// place the offer on the books. But also clean up expired offers
// that are discovered along the way.
//
// fix1578 changes the return code. Verify expected behavior
// without and with fix1578.
for (auto const& tweakedFeatures :
{features - fix1578, features | fix1578})
{
testAMM(
[&](AMM& ammAlice, Env& env) {
// Order that can't be filled
TER const killedCode{
tweakedFeatures[fix1578] ? TER{tecKILLED}
: TER{tesSUCCESS}};
env(offer(carol, USD(100), XRP(100)),
txflags(tfFillOrKill),
ter(killedCode));
env.close();
BEAST_EXPECT(ammAlice.expectBalances(
XRP(10'100), USD(10'000), ammAlice.tokens()));
// fee = AMM
BEAST_EXPECT(expectLedgerEntryRoot(
env, carol, XRP(30'000) - (txfee(env, 1))));
BEAST_EXPECT(expectOffers(env, carol, 0));
BEAST_EXPECT(expectHolding(env, carol, USD(30'000)));
testAMM(
[&](AMM& ammAlice, Env& env) {
// Order that can't be filled
TER const killedCode{TER{tecKILLED}};
env(offer(carol, USD(100), XRP(100)),
txflags(tfFillOrKill),
ter(killedCode));
env.close();
BEAST_EXPECT(ammAlice.expectBalances(
XRP(10'100), USD(10'000), ammAlice.tokens()));
// fee = AMM
BEAST_EXPECT(expectLedgerEntryRoot(
env, carol, XRP(30'000) - (txfee(env, 1))));
BEAST_EXPECT(expectOffers(env, carol, 0));
BEAST_EXPECT(expectHolding(env, carol, USD(30'000)));
// Order that can be filled
env(offer(carol, XRP(100), USD(100)),
txflags(tfFillOrKill),
ter(tesSUCCESS));
BEAST_EXPECT(ammAlice.expectBalances(
XRP(10'000), USD(10'100), ammAlice.tokens()));
BEAST_EXPECT(expectLedgerEntryRoot(
env, carol, XRP(30'000) + XRP(100) - txfee(env, 2)));
BEAST_EXPECT(expectHolding(env, carol, USD(29'900)));
BEAST_EXPECT(expectOffers(env, carol, 0));
},
{{XRP(10'100), USD(10'000)}},
0,
std::nullopt,
{tweakedFeatures});
// Order that can be filled
env(offer(carol, XRP(100), USD(100)),
txflags(tfFillOrKill),
ter(tesSUCCESS));
BEAST_EXPECT(ammAlice.expectBalances(
XRP(10'000), USD(10'100), ammAlice.tokens()));
BEAST_EXPECT(expectLedgerEntryRoot(
env, carol, XRP(30'000) + XRP(100) - txfee(env, 2)));
BEAST_EXPECT(expectHolding(env, carol, USD(29'900)));
BEAST_EXPECT(expectOffers(env, carol, 0));
},
{{XRP(10'100), USD(10'000)}},
0,
std::nullopt,
{features});
// Immediate or Cancel - cross as much as possible
// and add nothing on the books.
testAMM(
[&](AMM& ammAlice, Env& env) {
env(offer(carol, XRP(200), USD(200)),
txflags(tfImmediateOrCancel),
ter(tesSUCCESS));
// Immediate or Cancel - cross as much as possible
// and add nothing on the books.
testAMM(
[&](AMM& ammAlice, Env& env) {
env(offer(carol, XRP(200), USD(200)),
txflags(tfImmediateOrCancel),
ter(tesSUCCESS));
// AMM generates a synthetic offer of 100USD/100XRP
// to match the CLOB offer quality.
BEAST_EXPECT(ammAlice.expectBalances(
XRP(10'000), USD(10'100), ammAlice.tokens()));
// +AMM - offer * fee
BEAST_EXPECT(expectLedgerEntryRoot(
env, carol, XRP(30'000) + XRP(100) - txfee(env, 1)));
// AMM
BEAST_EXPECT(expectHolding(env, carol, USD(29'900)));
BEAST_EXPECT(expectOffers(env, carol, 0));
},
{{XRP(10'100), USD(10'000)}},
0,
std::nullopt,
{tweakedFeatures});
// AMM generates a synthetic offer of 100USD/100XRP
// to match the CLOB offer quality.
BEAST_EXPECT(ammAlice.expectBalances(
XRP(10'000), USD(10'100), ammAlice.tokens()));
// +AMM - offer * fee
BEAST_EXPECT(expectLedgerEntryRoot(
env, carol, XRP(30'000) + XRP(100) - txfee(env, 1)));
// AMM
BEAST_EXPECT(expectHolding(env, carol, USD(29'900)));
BEAST_EXPECT(expectOffers(env, carol, 0));
},
{{XRP(10'100), USD(10'000)}},
0,
std::nullopt,
{features});
// tfPassive -- place the offer without crossing it.
testAMM(
[&](AMM& ammAlice, Env& env) {
// Carol creates a passive offer that could cross AMM.
// Carol's offer should stay in the ledger.
env(offer(carol, XRP(100), USD(100), tfPassive));
env.close();
BEAST_EXPECT(ammAlice.expectBalances(
XRP(10'100), STAmount{USD, 10'000}, ammAlice.tokens()));
BEAST_EXPECT(expectOffers(
env, carol, 1, {{{XRP(100), STAmount{USD, 100}}}}));
},
{{XRP(10'100), USD(10'000)}},
0,
std::nullopt,
{tweakedFeatures});
// tfPassive -- place the offer without crossing it.
testAMM(
[&](AMM& ammAlice, Env& env) {
// Carol creates a passive offer that could cross AMM.
// Carol's offer should stay in the ledger.
env(offer(carol, XRP(100), USD(100), tfPassive));
env.close();
BEAST_EXPECT(ammAlice.expectBalances(
XRP(10'100), STAmount{USD, 10'000}, ammAlice.tokens()));
BEAST_EXPECT(expectOffers(
env, carol, 1, {{{XRP(100), STAmount{USD, 100}}}}));
},
{{XRP(10'100), USD(10'000)}},
0,
std::nullopt,
{features});
// tfPassive -- cross only offers of better quality.
testAMM(
[&](AMM& ammAlice, Env& env) {
env(offer(alice, USD(110), XRP(100)));
env.close();
// tfPassive -- cross only offers of better quality.
testAMM(
[&](AMM& ammAlice, Env& env) {
env(offer(alice, USD(110), XRP(100)));
env.close();
// Carol creates a passive offer. That offer should cross
// AMM and leave Alice's offer untouched.
env(offer(carol, XRP(100), USD(100), tfPassive));
env.close();
BEAST_EXPECT(ammAlice.expectBalances(
XRP(10'900),
STAmount{USD, UINT64_C(9'082'56880733945), -11},
ammAlice.tokens()));
BEAST_EXPECT(expectOffers(env, carol, 0));
BEAST_EXPECT(expectOffers(env, alice, 1));
},
{{XRP(11'000), USD(9'000)}},
0,
std::nullopt,
{tweakedFeatures});
}
// Carol creates a passive offer. That offer should cross
// AMM and leave Alice's offer untouched.
env(offer(carol, XRP(100), USD(100), tfPassive));
env.close();
BEAST_EXPECT(ammAlice.expectBalances(
XRP(10'900),
STAmount{USD, UINT64_C(9'082'56880733945), -11},
ammAlice.tokens()));
BEAST_EXPECT(expectOffers(env, carol, 0));
BEAST_EXPECT(expectOffers(env, alice, 1));
},
{{XRP(11'000), USD(9'000)}},
0,
std::nullopt,
{features});
}
void
@@ -867,8 +858,7 @@ private:
using namespace jtx;
// Code returned if an offer is killed.
TER const killedCode{
features[fix1578] ? TER{tecKILLED} : TER{tesSUCCESS}};
TER const killedCode{TER{tecKILLED}};
{
Env env{*this, features};

View File

@@ -846,13 +846,8 @@ public:
// Fill or Kill - unless we fully cross, just charge a fee and don't
// place the offer on the books. But also clean up expired offers
// that are discovered along the way.
//
// fix1578 changes the return code. Verify expected behavior
// without and with fix1578.
for (auto const& tweakedFeatures :
{features - fix1578, features | fix1578})
{
Env env{*this, tweakedFeatures};
Env env{*this, features};
auto const f = env.current()->fees().base;
@@ -878,9 +873,7 @@ public:
// Order that can't be filled but will remove bob's expired offer:
{
TER const killedCode{
tweakedFeatures[fix1578] ? TER{tecKILLED}
: TER{tesSUCCESS}};
TER const killedCode{TER{tecKILLED}};
env(offer(alice, XRP(1000), USD(1000)),
txflags(tfFillOrKill),
ter(killedCode));
@@ -3008,8 +3001,7 @@ public:
env.close();
// Code returned if an offer is killed.
TER const killedCode{
features[fix1578] ? TER{tecKILLED} : TER{tesSUCCESS}};
TER const killedCode{TER{tecKILLED}};
// bob offers XRP for USD.
env(trust(bob, USD(200)));

View File

@@ -143,7 +143,6 @@ class Feature_test : public beast::unit_test::suite
featureToName(fixTrustLinesToSelf) == "fixTrustLinesToSelf");
BEAST_EXPECT(featureToName(featureFlow) == "Flow");
BEAST_EXPECT(featureToName(featureNegativeUNL) == "NegativeUNL");
BEAST_EXPECT(featureToName(fix1578) == "fix1578");
BEAST_EXPECT(
featureToName(fixTakerDryOfferRemoval) ==
"fixTakerDryOfferRemoval");

View File

@@ -85,83 +85,74 @@ public:
auto const bob = Account("bob");
auto const carol = Account("carol");
// fix1578 changes the return code. Verify expected behavior
// without and with fix1578.
for (auto const& tweakedFeatures :
{features - fix1578, features | fix1578})
Env env(*this, features);
env.fund(XRP(10000), gw, alice, bob, carol);
env.close();
env.trust(alice["USD"](100), bob);
env.trust(bob["USD"](100), carol);
env.close();
// After this payment alice has a -50 USD balance with bob, and
// bob has a -50 USD balance with carol. So neither alice nor
// bob should be able to clear the noRipple flag.
env(pay(alice, carol, carol["USD"](50)), path(bob));
env.close();
TER const terNeg{TER{tecNO_PERMISSION}};
env(trust(alice, bob["USD"](100), bob, tfSetNoRipple), ter(terNeg));
env(trust(bob, carol["USD"](100), carol, tfSetNoRipple), ter(terNeg));
env.close();
Json::Value params;
params[jss::source_account] = alice.human();
params[jss::destination_account] = carol.human();
params[jss::destination_amount] = [] {
Json::Value dest_amt;
dest_amt[jss::currency] = "USD";
dest_amt[jss::value] = "1";
dest_amt[jss::issuer] = Account("carol").human();
return dest_amt;
}();
auto const resp =
env.rpc("json", "ripple_path_find", to_string(params));
BEAST_EXPECT(resp[jss::result][jss::alternatives].size() == 1);
auto getAccountLines = [&env](Account const& acct) {
auto const r = jtx::getAccountLines(env, acct);
return r[jss::lines];
};
{
Env env(*this, tweakedFeatures);
auto const aliceLines = getAccountLines(alice);
BEAST_EXPECT(aliceLines.size() == 1);
BEAST_EXPECT(aliceLines[0u][jss::no_ripple].asBool() == false);
env.fund(XRP(10000), gw, alice, bob, carol);
env.close();
auto const bobLines = getAccountLines(bob);
BEAST_EXPECT(bobLines.size() == 2);
BEAST_EXPECT(bobLines[0u][jss::no_ripple].asBool() == false);
BEAST_EXPECT(bobLines[1u][jss::no_ripple].asBool() == false);
}
env.trust(alice["USD"](100), bob);
env.trust(bob["USD"](100), carol);
env.close();
// Now carol sends the 50 USD back to alice. Then alice and
// bob can set the noRipple flag.
env(pay(carol, alice, alice["USD"](50)), path(bob));
env.close();
// After this payment alice has a -50 USD balance with bob, and
// bob has a -50 USD balance with carol. So neither alice nor
// bob should be able to clear the noRipple flag.
env(pay(alice, carol, carol["USD"](50)), path(bob));
env.close();
env(trust(alice, bob["USD"](100), bob, tfSetNoRipple));
env(trust(bob, carol["USD"](100), carol, tfSetNoRipple));
env.close();
{
auto const aliceLines = getAccountLines(alice);
BEAST_EXPECT(aliceLines.size() == 1);
BEAST_EXPECT(aliceLines[0u].isMember(jss::no_ripple));
TER const terNeg{
tweakedFeatures[fix1578] ? TER{tecNO_PERMISSION}
: TER{tesSUCCESS}};
env(trust(alice, bob["USD"](100), bob, tfSetNoRipple), ter(terNeg));
env(trust(bob, carol["USD"](100), carol, tfSetNoRipple),
ter(terNeg));
env.close();
Json::Value params;
params[jss::source_account] = alice.human();
params[jss::destination_account] = carol.human();
params[jss::destination_amount] = [] {
Json::Value dest_amt;
dest_amt[jss::currency] = "USD";
dest_amt[jss::value] = "1";
dest_amt[jss::issuer] = Account("carol").human();
return dest_amt;
}();
auto const resp =
env.rpc("json", "ripple_path_find", to_string(params));
BEAST_EXPECT(resp[jss::result][jss::alternatives].size() == 1);
auto getAccountLines = [&env](Account const& acct) {
auto const r = jtx::getAccountLines(env, acct);
return r[jss::lines];
};
{
auto const aliceLines = getAccountLines(alice);
BEAST_EXPECT(aliceLines.size() == 1);
BEAST_EXPECT(aliceLines[0u][jss::no_ripple].asBool() == false);
auto const bobLines = getAccountLines(bob);
BEAST_EXPECT(bobLines.size() == 2);
BEAST_EXPECT(bobLines[0u][jss::no_ripple].asBool() == false);
BEAST_EXPECT(bobLines[1u][jss::no_ripple].asBool() == false);
}
// Now carol sends the 50 USD back to alice. Then alice and
// bob can set the noRipple flag.
env(pay(carol, alice, alice["USD"](50)), path(bob));
env.close();
env(trust(alice, bob["USD"](100), bob, tfSetNoRipple));
env(trust(bob, carol["USD"](100), carol, tfSetNoRipple));
env.close();
{
auto const aliceLines = getAccountLines(alice);
BEAST_EXPECT(aliceLines.size() == 1);
BEAST_EXPECT(aliceLines[0u].isMember(jss::no_ripple));
auto const bobLines = getAccountLines(bob);
BEAST_EXPECT(bobLines.size() == 2);
BEAST_EXPECT(bobLines[0u].isMember(jss::no_ripple_peer));
BEAST_EXPECT(bobLines[1u].isMember(jss::no_ripple));
}
auto const bobLines = getAccountLines(bob);
BEAST_EXPECT(bobLines.size() == 2);
BEAST_EXPECT(bobLines[0u].isMember(jss::no_ripple_peer));
BEAST_EXPECT(bobLines[1u].isMember(jss::no_ripple));
}
}

View File

@@ -795,9 +795,7 @@ CreateOffer::applyGuts(Sandbox& sb, Sandbox& sbCancel)
if (bFillOrKill)
{
JLOG(j_.trace()) << "Fill or Kill: offer killed";
if (sb.rules().enabled(fix1578))
return {tecKILLED, false};
return {tesSUCCESS, false};
return {tecKILLED, false};
}
// For 'immediate or cancel' offers, the amount remaining doesn't get

View File

@@ -576,7 +576,7 @@ SetTrust::doApply()
if ((bHigh ? saHighBalance : saLowBalance) >= beast::zero)
uFlagsOut |= (bHigh ? lsfHighNoRipple : lsfLowNoRipple);
else if (view().rules().enabled(fix1578))
else
// Cannot set noRipple on a negative balance.
return tecNO_PERMISSION;
}