From 7a2562dc2bc5d8082255d5a81ecc1dfac9a6f7aa Mon Sep 17 00:00:00 2001 From: Bart Date: Wed, 13 May 2026 16:46:30 -0400 Subject: [PATCH] chore: Consolidate fix amendments (#7134) Co-authored-by: Bart <11445373+bthomee@users.noreply.github.com> --- include/xrpl/protocol/detail/features.macro | 9 ++- .../PermissionedDomainInvariant.cpp | 2 +- .../PermissionedDomainSet.cpp | 4 +- src/test/app/Invariants_test.cpp | 72 +++++++++---------- src/test/app/PermissionedDEX_test.cpp | 6 +- src/test/app/PermissionedDomains_test.cpp | 7 +- 6 files changed, 45 insertions(+), 55 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index dc26a5a43b..fd62b74d59 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -15,11 +15,10 @@ // Add new amendments to the top of this list. // Keep it sorted in reverse chronological order. -XRPL_FIX (Cleanup3_2_0, Supported::No, VoteBehavior::DefaultNo) -XRPL_FEATURE(MPTokensV2, Supported::No, VoteBehavior::DefaultNo) +XRPL_FIX (Cleanup3_2_0, Supported::No, VoteBehavior::DefaultNo) +XRPL_FEATURE(MPTokensV2, Supported::No, VoteBehavior::DefaultNo) XRPL_FIX (Cleanup3_1_3, Supported::Yes, VoteBehavior::DefaultYes) -XRPL_FIX (PermissionedDomainInvariant, Supported::Yes, VoteBehavior::DefaultNo) -XRPL_FIX (BatchInnerSigs, Supported::No, VoteBehavior::DefaultNo) +XRPL_FIX (BatchInnerSigs, Supported::No, VoteBehavior::DefaultNo) XRPL_FEATURE(LendingProtocol, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(PermissionDelegationV1_1, Supported::No, VoteBehavior::DefaultNo) XRPL_FIX (DirectoryLimit, Supported::Yes, VoteBehavior::DefaultNo) @@ -34,7 +33,7 @@ XRPL_FIX (EnforceNFTokenTrustlineV2, Supported::Yes, VoteBehavior::DefaultN XRPL_FIX (AMMv1_3, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(PermissionedDEX, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FEATURE(Batch, Supported::No, VoteBehavior::DefaultNo) -XRPL_FEATURE(SingleAssetVault, Supported::Yes, VoteBehavior::DefaultNo) +XRPL_FEATURE(SingleAssetVault, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FIX (PayChanCancelAfter, Supported::Yes, VoteBehavior::DefaultNo) // Check flags in Credential transactions XRPL_FIX (InvalidTxFlags, Supported::Yes, VoteBehavior::DefaultNo) diff --git a/src/libxrpl/tx/invariants/PermissionedDomainInvariant.cpp b/src/libxrpl/tx/invariants/PermissionedDomainInvariant.cpp index 784a2503ca..60cc897f99 100644 --- a/src/libxrpl/tx/invariants/PermissionedDomainInvariant.cpp +++ b/src/libxrpl/tx/invariants/PermissionedDomainInvariant.cpp @@ -102,7 +102,7 @@ ValidPermissionedDomain::finalize( return true; }; - if (view.rules().enabled(fixPermissionedDomainInvariant)) + if (view.rules().enabled(fixCleanup3_1_3)) { // No permissioned domains should be affected if the transaction failed if (!isTesSuccess(result)) diff --git a/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp b/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp index 3258dda409..83921e8526 100644 --- a/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp +++ b/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp @@ -110,8 +110,8 @@ PermissionedDomainSet::doApply() if (balance < reserve) return tecINSUFFICIENT_RESERVE; - bool const fix313 = view().rules().enabled(fixCleanup3_1_3); - auto const seq = fix313 ? ctx_.tx.getSeqValue() : ctx_.tx.getFieldU32(sfSequence); + bool const fixEnabled = view().rules().enabled(fixCleanup3_1_3); + auto const seq = fixEnabled ? ctx_.tx.getSeqValue() : ctx_.tx.getFieldU32(sfSequence); Keylet const pdKeylet = keylet::permissionedDomain(account_, seq); auto slePd = std::make_shared(pdKeylet); diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 99de5be302..d9670ed6a3 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -1314,11 +1314,11 @@ class Invariants_test : public beast::unit_test::Suite { using namespace test::jtx; - bool const fixPDEnabled = features[fixPermissionedDomainInvariant]; + bool const fixEnabled = features[fixCleanup3_1_3]; std::initializer_list const badTers = {tecINVARIANT_FAILED, tecINVARIANT_FAILED}; std::initializer_list const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}; - testcase << "PermissionedDomain" + std::string(fixPDEnabled ? " fix" : ""); + testcase << "PermissionedDomain" + std::string(fixEnabled ? " fix" : ""); doInvariantCheck( Env(*this, features), @@ -1328,7 +1328,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); testcase << "PermissionedDomain 2"; @@ -1341,7 +1341,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); testcase << "PermissionedDomain 3"; doInvariantCheck( @@ -1365,7 +1365,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); testcase << "PermissionedDomain 4"; doInvariantCheck( @@ -1388,7 +1388,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); testcase << "PermissionedDomain Set 1"; doInvariantCheck( @@ -1409,7 +1409,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); testcase << "PermissionedDomain Set 2"; doInvariantCheck( @@ -1440,7 +1440,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); testcase << "PermissionedDomain Set 3"; doInvariantCheck( @@ -1470,7 +1470,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); testcase << "PermissionedDomain Set 4"; doInvariantCheck( @@ -1498,7 +1498,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : badTers); + fixEnabled ? failTers : badTers); std::initializer_list const goodTers = {tesSUCCESS, tesSUCCESS}; @@ -1516,7 +1516,7 @@ class Invariants_test : public beast::unit_test::Suite testcase << "PermissionedDomain set 2 domains "; doInvariantCheck( Env(*this, features), - fixPDEnabled ? badMoreThan1 : emptyV, + fixEnabled ? badMoreThan1 : emptyV, [](Account const& a1, Account const& a2, ApplyContext& ac) { createPermissionedDomain(ac, a1, a2); createPermissionedDomain(ac, a1, a2, 2, 11); @@ -1524,7 +1524,7 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : goodTers); + fixEnabled ? failTers : goodTers); } { @@ -1545,7 +1545,7 @@ class Invariants_test : public beast::unit_test::Suite std::move(env1), a1, a2, - fixPDEnabled ? badMoreThan1 : emptyV, + fixEnabled ? badMoreThan1 : emptyV, [&pd1, &pd2](Account const&, Account const&, ApplyContext& ac) { auto sle1 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd1}); auto sle2 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd2}); @@ -1555,18 +1555,18 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_DELETE, [](STObject&) {}}, - fixPDEnabled ? failTers : goodTers); + fixEnabled ? failTers : goodTers); } { testcase << "PermissionedDomain set 0 domains "; doInvariantCheck( Env(*this, features), - fixPDEnabled ? badNoDomains : emptyV, + fixEnabled ? badNoDomains : emptyV, [](Account const&, Account const&, ApplyContext&) { return true; }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? badTers : goodTers); + fixEnabled ? badTers : goodTers); } { @@ -1587,11 +1587,11 @@ class Invariants_test : public beast::unit_test::Suite Env(*this, features), a1, a2, - fixPDEnabled ? badNoDomains : emptyV, + fixEnabled ? badNoDomains : emptyV, [](Account const&, Account const&, ApplyContext&) { return true; }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_DELETE, [](STObject&) {}}, - fixPDEnabled ? badTers : goodTers); + fixEnabled ? badTers : goodTers); } { @@ -1611,7 +1611,7 @@ class Invariants_test : public beast::unit_test::Suite std::move(env1), a1, a2, - fixPDEnabled ? badDeleted : emptyV, + fixEnabled ? badDeleted : emptyV, [&pd1](Account const&, Account const&, ApplyContext& ac) { auto sle1 = ac.view().peek({ltPERMISSIONED_DOMAIN, pd1}); ac.view().erase(sle1); @@ -1619,28 +1619,28 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_SET, [](STObject&) {}}, - fixPDEnabled ? failTers : goodTers); + fixEnabled ? failTers : goodTers); } { testcase << "PermissionedDomain del, create domain "; doInvariantCheck( Env(*this, features), - fixPDEnabled ? badNotDeleted : emptyV, + fixEnabled ? badNotDeleted : emptyV, [](Account const& a1, Account const& a2, ApplyContext& ac) { createPermissionedDomain(ac, a1, a2); return true; }, XRPAmount{}, STTx{ttPERMISSIONED_DOMAIN_DELETE, [](STObject&) {}}, - fixPDEnabled ? failTers : goodTers); + fixEnabled ? failTers : goodTers); } { testcase << "PermissionedDomain invalid tx"; doInvariantCheck( - fixPDEnabled ? badTx : emptyV, + fixEnabled ? badTx : emptyV, [&](Account const& a1, Account const& a2, ApplyContext& ac) { createPermissionedDomain(ac, a1, a2); return true; @@ -1800,11 +1800,9 @@ class Invariants_test : public beast::unit_test::Suite { using namespace test::jtx; - bool const fixPDEnabled = features[fixPermissionedDomainInvariant]; - bool const fixS313Enabled = features[fixCleanup3_1_3]; + bool const fixEnabled = features[fixCleanup3_1_3]; - testcase << "PermissionedDEX" + std::string(fixPDEnabled ? " fixPD" : "") + - std::string(fixS313Enabled ? " fixS313" : ""); + testcase << "PermissionedDEX" + std::string(fixEnabled ? " fix" : ""); doInvariantCheck( Env(*this, features), @@ -1908,8 +1906,8 @@ class Invariants_test : public beast::unit_test::Suite std::move(env1), a1, a2, - fixS313Enabled ? std::vector{{"hybrid offer is malformed"}} - : std::vector{}, + fixEnabled ? std::vector{{"hybrid offer is malformed"}} + : std::vector{}, [&pd1](Account const& a1, Account const& a2, ApplyContext& ac) { Keylet const offerKey = keylet::offer(a2.id(), 10); auto sleOffer = std::make_shared(offerKey); @@ -1926,9 +1924,8 @@ class Invariants_test : public beast::unit_test::Suite }, XRPAmount{}, STTx{ttOFFER_CREATE, [&](STObject&) {}}, - fixS313Enabled - ? std::initializer_list{tecINVARIANT_FAILED, tecINVARIANT_FAILED} - : std::initializer_list{tesSUCCESS, tesSUCCESS}); + fixEnabled ? std::initializer_list{tecINVARIANT_FAILED, tecINVARIANT_FAILED} + : std::initializer_list{tesSUCCESS, tesSUCCESS}); } // hybrid offer missing sfAdditionalBooks @@ -4380,13 +4377,10 @@ public: testNoZeroEscrow(); testValidNewAccountRoot(); testNFTokenPageInvariants(); - testPermissionedDomainInvariants(defaultAmendments() | fixPermissionedDomainInvariant); - testPermissionedDomainInvariants(defaultAmendments() - fixPermissionedDomainInvariant); - testPermissionedDEX(defaultAmendments() | fixPermissionedDomainInvariant); - testPermissionedDEX(defaultAmendments() - fixPermissionedDomainInvariant); - testPermissionedDEX( - (defaultAmendments() | fixPermissionedDomainInvariant) - fixCleanup3_1_3); - testPermissionedDEX(defaultAmendments() - fixPermissionedDomainInvariant - fixCleanup3_1_3); + testPermissionedDomainInvariants(defaultAmendments() | fixCleanup3_1_3); + testPermissionedDomainInvariants(defaultAmendments() - fixCleanup3_1_3); + testPermissionedDEX(defaultAmendments() | fixCleanup3_1_3); + testPermissionedDEX(defaultAmendments() - fixCleanup3_1_3); testNoModifiedUnmodifiableFields(); testValidPseudoAccounts(); testValidLoanBroker(); diff --git a/src/test/app/PermissionedDEX_test.cpp b/src/test/app/PermissionedDEX_test.cpp index 21e6bd8baa..45b4a58901 100644 --- a/src/test/app/PermissionedDEX_test.cpp +++ b/src/test/app/PermissionedDEX_test.cpp @@ -1392,10 +1392,10 @@ class PermissionedDEX_test : public beast::unit_test::Suite void testHybridMalformedOffer(FeatureBitset features) { - bool const fixS313Enabled = features[fixCleanup3_1_3]; + bool const fixEnabled = features[fixCleanup3_1_3]; testcase << "Hybrid offer with empty AdditionalBooks" - << (fixS313Enabled ? " (fixCleanup3_1_3 enabled)" : " (fixCleanup3_1_3 disabled)"); + << (fixEnabled ? " (fixCleanup3_1_3 enabled)" : " (fixCleanup3_1_3 disabled)"); // offerInDomain has two code paths gated by fixCleanup3_1_3: // @@ -1436,7 +1436,7 @@ class PermissionedDEX_test : public beast::unit_test::Suite return true; }); - if (fixS313Enabled) + if (fixEnabled) { // post-fixCleanup3_1_3: offerInDomain rejects the malformed // offer (size == 0), so no valid domain offer is found. diff --git a/src/test/app/PermissionedDomains_test.cpp b/src/test/app/PermissionedDomains_test.cpp index 008f54ddd8..0593b06eaa 100644 --- a/src/test/app/PermissionedDomains_test.cpp +++ b/src/test/app/PermissionedDomains_test.cpp @@ -50,12 +50,9 @@ exceptionExpected(Env& env, json::Value const& jv) class PermissionedDomains_test : public beast::unit_test::Suite { FeatureBitset withFeature_{ - (testableAmendments() // - | featurePermissionedDomains | featureCredentials) - - fixPermissionedDomainInvariant}; + (testableAmendments() | featurePermissionedDomains | featureCredentials) - fixCleanup3_1_3}; FeatureBitset withFix_{ - testableAmendments() // - | featurePermissionedDomains | featureCredentials | fixPermissionedDomainInvariant}; + testableAmendments() | featurePermissionedDomains | featureCredentials | fixCleanup3_1_3}; // Verify that each tx type can execute if the feature is enabled. void