From 2c187461cc892ff64b1693726ad73deb649987d3 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Mon, 17 Nov 2025 14:02:10 +0000 Subject: [PATCH] refactor: Retire NegativeUNL amendment (#6033) Amendments activated for more than 2 years can be retired. This change retires the NegativeUNL amendment. --- include/xrpl/protocol/detail/features.macro | 2 +- src/test/consensus/NegativeUNL_test.cpp | 45 +-------------------- src/test/rpc/Feature_test.cpp | 3 +- src/xrpld/app/consensus/RCLConsensus.cpp | 7 +--- src/xrpld/app/ledger/detail/BuildLedger.cpp | 2 +- src/xrpld/app/misc/NetworkOPs.cpp | 3 +- src/xrpld/app/tx/detail/Change.cpp | 7 ---- 7 files changed, 9 insertions(+), 60 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 75b8db515d..dd66d8a34c 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -66,7 +66,6 @@ XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo XRPL_FEATURE(DisallowIncoming, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes) -XRPL_FEATURE(NegativeUNL, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(RequireFullyCanonicalSig, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(DeletableAccounts, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(Checks, Supported::yes, VoteBehavior::DefaultYes) @@ -130,6 +129,7 @@ XRPL_RETIRE_FEATURE(HardenedValidations) XRPL_RETIRE_FEATURE(ImmediateOfferKilled) XRPL_RETIRE_FEATURE(MultiSign) XRPL_RETIRE_FEATURE(MultiSignReserve) +XRPL_RETIRE_FEATURE(NegativeUNL) XRPL_RETIRE_FEATURE(NonFungibleTokensV1_1) XRPL_RETIRE_FEATURE(PayChan) XRPL_RETIRE_FEATURE(SortedDirectories) diff --git a/src/test/consensus/NegativeUNL_test.cpp b/src/test/consensus/NegativeUNL_test.cpp index 8457d0e2be..5b7eebed2f 100644 --- a/src/test/consensus/NegativeUNL_test.cpp +++ b/src/test/consensus/NegativeUNL_test.cpp @@ -15,7 +15,6 @@ namespace test { /* * This file implements the following negative UNL related tests: * -- test filling and applying ttUNL_MODIFY Tx and ledger update - * -- test ttUNL_MODIFY Tx failure without featureNegativeUNL amendment * -- test the NegativeUNLVote class. The test cases are split to multiple * test classes to allow parallel execution. * -- test the negativeUNLFilter function @@ -208,7 +207,7 @@ class NegativeUNL_test : public beast::unit_test::suite testcase("Create UNLModify Tx and apply to ledgers"); - jtx::Env env(*this, jtx::testable_amendments() | featureNegativeUNL); + jtx::Env env(*this, jtx::testable_amendments()); std::vector publicKeys = createPublicKeys(3); // genesis ledger auto l = std::make_shared( @@ -216,7 +215,6 @@ class NegativeUNL_test : public beast::unit_test::suite env.app().config(), std::vector{}, env.app().getNodeFamily()); - BEAST_EXPECT(l->rules().enabled(featureNegativeUNL)); // Record the public keys and ledger sequences of expected negative UNL // validators when we build the ledger history @@ -500,44 +498,6 @@ class NegativeUNL_test : public beast::unit_test::suite } }; -class NegativeUNLNoAmendment_test : public beast::unit_test::suite -{ - void - testNegativeUNLNoAmendment() - { - testcase("No negative UNL amendment"); - - jtx::Env env(*this, jtx::testable_amendments() - featureNegativeUNL); - std::vector publicKeys = createPublicKeys(1); - // genesis ledger - auto l = std::make_shared( - create_genesis, - env.app().config(), - std::vector{}, - env.app().getNodeFamily()); - BEAST_EXPECT(!l->rules().enabled(featureNegativeUNL)); - - // generate more ledgers - for (auto i = 0; i < 256 - 1; ++i) - { - l = std::make_shared( - *l, env.app().timeKeeper().closeTime()); - } - BEAST_EXPECT(l->seq() == 256); - auto txDisable_0 = createTx(true, l->seq(), publicKeys[0]); - OpenView accum(&*l); - BEAST_EXPECT(applyAndTestResult(env, accum, txDisable_0, false)); - accum.apply(*l); - BEAST_EXPECT(negUnlSizeTest(l, 0, false, false)); - } - - void - run() override - { - testNegativeUNLNoAmendment(); - } -}; - /** * Utility class for creating validators and ledger history */ @@ -563,7 +523,7 @@ struct NetworkHistory }; NetworkHistory(beast::unit_test::suite& suite, Parameter const& p) - : env(suite, jtx::testable_amendments() | featureNegativeUNL) + : env(suite, jtx::testable_amendments()) , param(p) , validations(env.app().getValidations()) { @@ -1867,7 +1827,6 @@ class NegativeUNLVoteFilterValidations_test : public beast::unit_test::suite }; BEAST_DEFINE_TESTSUITE(NegativeUNL, consensus, ripple); -BEAST_DEFINE_TESTSUITE(NegativeUNLNoAmendment, consensus, ripple); BEAST_DEFINE_TESTSUITE(NegativeUNLVoteInternal, consensus, ripple); BEAST_DEFINE_TESTSUITE_MANUAL(NegativeUNLVoteScoreTable, consensus, ripple); diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index 250c634007..10f82ac88d 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -124,7 +124,8 @@ class Feature_test : public beast::unit_test::suite featureToName(fixRemoveNFTokenAutoTrustLine) == "fixRemoveNFTokenAutoTrustLine"); BEAST_EXPECT(featureToName(featureFlow) == "Flow"); - BEAST_EXPECT(featureToName(featureNegativeUNL) == "NegativeUNL"); + BEAST_EXPECT( + featureToName(featureDeletableAccounts) == "DeletableAccounts"); BEAST_EXPECT( featureToName(fixIncludeKeyletFields) == "fixIncludeKeyletFields"); BEAST_EXPECT(featureToName(featureTokenEscrow) == "TokenEscrow"); diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp index 7734ab790d..09584f2e77 100644 --- a/src/xrpld/app/consensus/RCLConsensus.cpp +++ b/src/xrpld/app/consensus/RCLConsensus.cpp @@ -346,9 +346,7 @@ RCLConsensus::Adaptor::onClose( prevLedger, validations, initialSet, j_); } } - else if ( - prevLedger->isVotingLedger() && - prevLedger->rules().enabled(featureNegativeUNL)) + else if (prevLedger->isVotingLedger()) { // previous ledger was a voting ledger, // so the current consensus session is for a flag ledger, @@ -1015,8 +1013,7 @@ RCLConsensus::Adaptor::preStartRound( inboundTransactions_.newRound(prevLgr.seq()); // Notify NegativeUNLVote that new validators are added - if (prevLgr.ledger_->rules().enabled(featureNegativeUNL) && - !nowTrusted.empty()) + if (!nowTrusted.empty()) nUnlVote_.newValidators(prevLgr.seq() + 1, nowTrusted); // propose only if we're in sync with the network (and validating) diff --git a/src/xrpld/app/ledger/detail/BuildLedger.cpp b/src/xrpld/app/ledger/detail/BuildLedger.cpp index 97824e3023..dd3593e1cf 100644 --- a/src/xrpld/app/ledger/detail/BuildLedger.cpp +++ b/src/xrpld/app/ledger/detail/BuildLedger.cpp @@ -28,7 +28,7 @@ buildLedgerImpl( { auto built = std::make_shared(*parent, closeTime); - if (built->isFlagLedger() && built->rules().enabled(featureNegativeUNL)) + if (built->isFlagLedger()) { built->updateNegativeUNL(); } diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index ebe539523b..963f3dc3ea 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -2063,8 +2063,7 @@ NetworkOPsImp::beginConsensus( "ripple::NetworkOPsImp::beginConsensus : closedLedger parent matches " "hash"); - if (prevLedger->rules().enabled(featureNegativeUNL)) - app_.validators().setNegativeUNL(prevLedger->negativeUNL()); + app_.validators().setNegativeUNL(prevLedger->negativeUNL()); TrustChanges const changes = app_.validators().updateTrusted( app_.getValidations().getCurrentNodeIDs(), closingInfo.parentCloseTime, diff --git a/src/xrpld/app/tx/detail/Change.cpp b/src/xrpld/app/tx/detail/Change.cpp index 4832287c2e..43db9ae13c 100644 --- a/src/xrpld/app/tx/detail/Change.cpp +++ b/src/xrpld/app/tx/detail/Change.cpp @@ -51,13 +51,6 @@ Transactor::invokePreflight(PreflightContext const& ctx) return temBAD_SEQUENCE; } - if (ctx.tx.getTxnType() == ttUNL_MODIFY && - !ctx.rules.enabled(featureNegativeUNL)) - { - JLOG(ctx.j.warn()) << "Change: NegativeUNL not enabled"; - return temDISABLED; - } - return tesSUCCESS; }