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

refactor: Retire fixReducedOffersV1 amendment (#5972)

Browse Source 
Amendments activated for more than 2 years can be retired. This change retires the fixReducedOffersV1 amendment.
This commit is contained in:
Jingchen
2025-10-31 20:25:05 +00:00
committed by GitHub
parent dfafb141cc
commit ab45a8a737
6 changed files with 29 additions and 123 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -77,7 +77,6 @@ XRPL_FIX (DisallowIncomingV1, Supported::yes, VoteBehavior::DefaultNo
XRPL_FEATURE(XChainBridge, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(AMM, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(Clawback, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (ReducedOffersV1, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (NFTokenRemint, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (NonFungibleTokensV1_2, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo)
@@ -141,6 +140,7 @@ XRPL_RETIRE(fix1781)
XRPL_RETIRE(fixAmendmentMajorityCalc)
XRPL_RETIRE(fixCheckThreading)
XRPL_RETIRE(fixQualityUpperBound)
XRPL_RETIRE(fixReducedOffersV1)
XRPL_RETIRE(fixRmSmallIncreasedQOffers)
XRPL_RETIRE(fixSTAmountCanonicalize)
XRPL_RETIRE(fixTakerDryOfferRemoval)

94
src/test/app/ReducedOffer_test.cpp
View File

@@ -80,12 +80,8 @@ public:
auto const bob = Account{"bob"};
auto const USD = gw["USD"];
// Make one test run without fixReducedOffersV1 and one with.
for (FeatureBitset features :
{testable_amendments() - fixReducedOffersV1,
testable_amendments() | fixReducedOffersV1})
{
Env env{*this, features};
Env env{*this, testable_amendments()};
// Make sure none of the offers we generate are under funded.
env.fund(XRP(10'000'000), gw, alice, bob);
@@ -208,19 +204,9 @@ public:
blockedCount += exerciseOfferPair(alicesOffer, bobsOffer);
}
// If fixReducedOffersV1 is enabled, then none of the test cases
// should produce a potentially blocking rate.
//
// Also verify that if fixReducedOffersV1 is not enabled then
// some of the test cases produced a potentially blocking rate.
if (features[fixReducedOffersV1])
{
BEAST_EXPECT(blockedCount == 0);
}
else
{
BEAST_EXPECT(blockedCount >= 170);
}
// None of the test cases should produce a potentially blocking
// rate.
BEAST_EXPECT(blockedCount == 0);
}
}
@@ -236,13 +222,9 @@ public:
auto const bob = Account{"bob"};
auto const USD = gw["USD"];
// Make one test run without fixReducedOffersV1 and one with.
for (FeatureBitset features :
{testable_amendments() - fixReducedOffersV1,
testable_amendments() | fixReducedOffersV1})
{
// Make sure none of the offers we generate are under funded.
Env env{*this, features};
Env env{*this, testable_amendments()};
env.fund(XRP(10'000'000), gw, alice, bob);
env.close();
@@ -367,19 +349,9 @@ public:
blockedCount += exerciseOfferPair(aliceOffer, bobsOffer);
}
// If fixReducedOffersV1 is enabled, then none of the test cases
// should produce a potentially blocking rate.
//
// Also verify that if fixReducedOffersV1 is not enabled then
// some of the test cases produced a potentially blocking rate.
if (features[fixReducedOffersV1])
{
BEAST_EXPECT(blockedCount == 0);
}
else
{
BEAST_EXPECT(blockedCount > 10);
}
// None of the test cases should produce a potentially blocking
// rate.
BEAST_EXPECT(blockedCount == 0);
}
}
@@ -389,9 +361,6 @@ public:
testcase("exercise underfunded XRP/IOU offer Q change");
// Bob places an offer that is not fully funded.
//
// This unit test compares the behavior of this situation before and
// after applying the fixReducedOffersV1 amendment.
using namespace jtx;
auto const alice = Account{"alice"};
@@ -399,12 +368,8 @@ public:
auto const gw = Account{"gw"};
auto const USD = gw["USD"];
// Make one test run without fixReducedOffersV1 and one with.
for (FeatureBitset features :
{testable_amendments() - fixReducedOffersV1,
testable_amendments() | fixReducedOffersV1})
{
Env env{*this, features};
Env env{*this, testable_amendments()};
env.fund(XRP(10000), alice, bob, gw);
env.close();
@@ -470,19 +435,9 @@ public:
}
}
// If fixReducedOffersV1 is enabled, then none of the test cases
// should produce a potentially blocking rate.
//
// Also verify that if fixReducedOffersV1 is not enabled then
// some of the test cases produced a potentially blocking rate.
if (features[fixReducedOffersV1])
{
BEAST_EXPECT(blockedOrderBookCount == 0);
}
else
{
BEAST_EXPECT(blockedOrderBookCount > 15);
}
// None of the test cases should produce a potentially blocking
// rate.
BEAST_EXPECT(blockedOrderBookCount == 0);
}
}
@@ -492,9 +447,6 @@ public:
testcase("exercise underfunded IOU/IOU offer Q change");
// Bob places an IOU/IOU offer that is not fully funded.
//
// This unit test compares the behavior of this situation before and
// after applying the fixReducedOffersV1 amendment.
using namespace jtx;
using namespace std::chrono_literals;
@@ -507,12 +459,8 @@ public:
STAmount const tinyUSD(USD.issue(), /*mantissa*/ 1, /*exponent*/ -81);
// Make one test run without fixReducedOffersV1 and one with.
for (FeatureBitset features :
{testable_amendments() - fixReducedOffersV1,
testable_amendments() | fixReducedOffersV1})
{
Env env{*this, features};
Env env{*this, testable_amendments()};
env.fund(XRP(10000), alice, bob, gw);
env.close();
@@ -594,19 +542,9 @@ public:
env.close();
}
// If fixReducedOffersV1 is enabled, then none of the test cases
// should produce a potentially blocking rate.
//
// Also verify that if fixReducedOffersV1 is not enabled then
// some of the test cases produced a potentially blocking rate.
if (features[fixReducedOffersV1])
{
BEAST_EXPECT(blockedOrderBookCount == 0);
}
else
{
BEAST_EXPECT(blockedOrderBookCount > 20);
}
// None of the test cases should produce a potentially blocking
// rate.
BEAST_EXPECT(blockedOrderBookCount == 0);
}
}

11
src/xrpld/app/paths/detail/AMMOffer.cpp
View File

@@ -92,14 +92,9 @@ AMMOffer<TIn, TOut>::limitOut(
// poolPays * poolGets < (poolPays - assetOut) * (poolGets + assetIn)
if (ammLiquidity_.multiPath())
{
if (auto const& rules = getCurrentTransactionRules();
rules && rules->enabled(fixReducedOffersV1))
// It turns out that the ceil_out implementation has some slop in
// it. ceil_out_strict removes that slop. But removing that slop
// affects transaction outcomes, so the change must be made using
// an amendment.
return quality().ceil_out_strict(offrAmt, limit, roundUp);
return quality().ceil_out(offrAmt, limit);
// It turns out that the ceil_out implementation has some slop in
// it, which ceil_out_strict removes.
return quality().ceil_out_strict(offrAmt, limit, roundUp);
}
// Change the offer size according to the conservation function. The offer
// quality is increased in this case, but it doesn't matter since there is

18
src/xrpld/app/tx/detail/CreateOffer.cpp
View File

@@ -477,22 +477,8 @@ CreateOffer::flowCross(
// what is a good threshold to check?
afterCross.in.clear();
afterCross.out = [&]() {
// Careful analysis showed that rounding up this
// divRound result could lead to placing a reduced
// offer in the ledger that blocks order books. So
// the fixReducedOffersV1 amendment changes the
// behavior to round down instead.
if (psb.rules().enabled(fixReducedOffersV1))
return divRoundStrict(
afterCross.in,
rate,
takerAmount.out.issue(),
false);
return divRound(
afterCross.in, rate, takerAmount.out.issue(), true);
}();
afterCross.out = divRoundStrict(
afterCross.in, rate, takerAmount.out.issue(), false);
}
else
{

11
src/xrpld/app/tx/detail/Offer.h
View File

@@ -241,14 +241,9 @@ TOffer<TIn, TOut>::limitOut(
TOut const& limit,
bool roundUp) const
{
if (auto const& rules = getCurrentTransactionRules();
rules && rules->enabled(fixReducedOffersV1))
// It turns out that the ceil_out implementation has some slop in
// it. ceil_out_strict removes that slop. But removing that slop
// affects transaction outcomes, so the change must be made using
// an amendment.
return quality().ceil_out_strict(offrAmt, limit, roundUp);
return m_quality.ceil_out(offrAmt, limit);
// It turns out that the ceil_out implementation has some slop in
// it, which ceil_out_strict removes.
return quality().ceil_out_strict(offrAmt, limit, roundUp);
}
template <class TIn, class TOut>

16
src/xrpld/app/tx/detail/OfferStream.cpp
View File

@@ -184,7 +184,6 @@ TOfferStreamBase<TIn, TOut>::shouldRmSmallIncreasedQOffer() const
}
TTakerGets const ownerFunds = toAmount<TTakerGets>(*ownerFunds_);
bool const fixReduced = view_.rules().enabled(fixReducedOffersV1);
auto const effectiveAmounts = [&] {
if (offer_.owner() != offer_.issueOut().account &&
@@ -193,22 +192,15 @@ TOfferStreamBase<TIn, TOut>::shouldRmSmallIncreasedQOffer() const
// adjust the amounts by owner funds.
//
// It turns out we can prevent order book blocking by rounding down
// the ceil_out() result. This adjustment changes transaction
// results, so it must be made under an amendment.
if (fixReduced)
return offer_.quality().ceil_out_strict(
ofrAmts, ownerFunds, /* roundUp */ false);
return offer_.quality().ceil_out(ofrAmts, ownerFunds);
// the ceil_out() result.
return offer_.quality().ceil_out_strict(
ofrAmts, ownerFunds, /* roundUp */ false);
}
return ofrAmts;
}();
// If either the effective in or out are zero then remove the offer.
// This can happen with fixReducedOffersV1 since it rounds down.
if (fixReduced &&
(effectiveAmounts.in.signum() <= 0 ||
effectiveAmounts.out.signum() <= 0))
if (effectiveAmounts.in.signum() <= 0 || effectiveAmounts.out.signum() <= 0)
return true;
if (effectiveAmounts.in > TTakerPays::minPositiveAmount())