From 57f4b4eb7ff279d848262a0c5fcee56bd8820494 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Fri, 21 Nov 2025 14:19:43 +0000 Subject: [PATCH 01/11] refactor: Retire Checks amendment (#6055) Amendments activated for more than 2 years can be retired. This change retires the Checks amendment. --- include/xrpl/protocol/detail/features.macro | 2 +- .../xrpl/protocol/detail/transactions.macro | 6 +++--- src/test/app/Check_test.cpp | 19 ------------------- src/test/app/Delegate_test.cpp | 3 --- 4 files changed, 4 insertions(+), 26 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index dd66d8a34c..116da10ca3 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -68,7 +68,6 @@ XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYe XRPL_FEATURE(FlowSortStrands, 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) XRPL_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYes) // The following amendments are obsolete, but must remain supported @@ -116,6 +115,7 @@ XRPL_RETIRE_FIX(STAmountCanonicalize) XRPL_RETIRE_FIX(TakerDryOfferRemoval) XRPL_RETIRE_FIX(TrustLinesToSelf) +XRPL_RETIRE_FEATURE(Checks) XRPL_RETIRE_FEATURE(CheckCashMakesTrustLine) XRPL_RETIRE_FEATURE(CryptoConditions) XRPL_RETIRE_FEATURE(DepositAuth) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index 23049d2543..c3cc2cef76 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -226,7 +226,7 @@ TRANSACTION(ttPAYCHAN_CLAIM, 15, PaymentChannelClaim, #endif TRANSACTION(ttCHECK_CREATE, 16, CheckCreate, Delegation::delegatable, - featureChecks, + uint256{}, noPriv, ({ {sfDestination, soeREQUIRED}, @@ -242,7 +242,7 @@ TRANSACTION(ttCHECK_CREATE, 16, CheckCreate, #endif TRANSACTION(ttCHECK_CASH, 17, CheckCash, Delegation::delegatable, - featureChecks, + uint256{}, noPriv, ({ {sfCheckID, soeREQUIRED}, @@ -256,7 +256,7 @@ TRANSACTION(ttCHECK_CASH, 17, CheckCash, #endif TRANSACTION(ttCHECK_CANCEL, 18, CheckCancel, Delegation::delegatable, - featureChecks, + uint256{}, noPriv, ({ {sfCheckID, soeREQUIRED}, diff --git a/src/test/app/Check_test.cpp b/src/test/app/Check_test.cpp index 61e20a0098..eb234cea56 100644 --- a/src/test/app/Check_test.cpp +++ b/src/test/app/Check_test.cpp @@ -125,25 +125,6 @@ class Check_test : public beast::unit_test::suite using namespace test::jtx; Account const alice{"alice"}; - { - // If the Checks amendment is not enabled, you should not be able - // to create, cash, or cancel checks. - Env env{*this, features - featureChecks}; - - env.fund(XRP(1000), alice); - env.close(); - - uint256 const checkId{ - getCheckIndex(env.master, env.seq(env.master))}; - env(check::create(env.master, alice, XRP(100)), ter(temDISABLED)); - env.close(); - - env(check::cash(alice, checkId, XRP(100)), ter(temDISABLED)); - env.close(); - - env(check::cancel(alice, checkId), ter(temDISABLED)); - env.close(); - } { // If the Checks amendment is enabled all check-related // facilities should be available. diff --git a/src/test/app/Delegate_test.cpp b/src/test/app/Delegate_test.cpp index 9e1c777a91..e36ad9a550 100644 --- a/src/test/app/Delegate_test.cpp +++ b/src/test/app/Delegate_test.cpp @@ -1703,9 +1703,6 @@ class Delegate_test : public beast::unit_test::suite // NFTokenMint, NFTokenBurn, NFTokenCreateOffer, NFTokenCancelOffer, // NFTokenAcceptOffer are not included, they are tested separately. std::unordered_map txRequiredFeatures{ - {"CheckCreate", featureChecks}, - {"CheckCash", featureChecks}, - {"CheckCancel", featureChecks}, {"Clawback", featureClawback}, {"AMMClawback", featureAMMClawback}, {"AMMCreate", featureAMM}, From e4dccfd49b2a6415aecf968690c3676bd5f555f0 Mon Sep 17 00:00:00 2001 From: Pratik Mankawde Date: Fri, 21 Nov 2025 15:18:00 +0000 Subject: [PATCH 02/11] refactor: Retire DisallowIncoming amendment (#6045) Amendments activated for more than 2 years can be retired. This change retires the DisallowIncoming amendment. --- include/xrpl/protocol/detail/features.macro | 2 +- src/test/app/Check_test.cpp | 17 +------- src/test/app/NFToken_test.cpp | 20 ++------- src/test/app/PayChan_test.cpp | 17 +------- src/test/app/SetTrust_test.cpp | 22 ++-------- src/test/rpc/AccountInfo_test.cpp | 48 ++++++++------------- src/xrpld/app/tx/detail/CreateCheck.cpp | 3 +- src/xrpld/app/tx/detail/NFTokenUtils.cpp | 28 ++++-------- src/xrpld/app/tx/detail/PayChan.cpp | 3 +- src/xrpld/app/tx/detail/SetAccount.cpp | 36 +++++++--------- src/xrpld/app/tx/detail/SetTrust.cpp | 28 ++++++------ src/xrpld/rpc/handlers/AccountInfo.cpp | 7 +-- 12 files changed, 67 insertions(+), 164 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 116da10ca3..4180f1e7ff 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -63,7 +63,6 @@ XRPL_FEATURE(AMM, Supported::yes, VoteBehavior::DefaultNo XRPL_FEATURE(Clawback, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo) 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(RequireFullyCanonicalSig, Supported::yes, VoteBehavior::DefaultYes) @@ -120,6 +119,7 @@ XRPL_RETIRE_FEATURE(CheckCashMakesTrustLine) XRPL_RETIRE_FEATURE(CryptoConditions) XRPL_RETIRE_FEATURE(DepositAuth) XRPL_RETIRE_FEATURE(DepositPreauth) +XRPL_RETIRE_FEATURE(DisallowIncoming) XRPL_RETIRE_FEATURE(Escrow) XRPL_RETIRE_FEATURE(EnforceInvariants) XRPL_RETIRE_FEATURE(ExpandedSignerList) diff --git a/src/test/app/Check_test.cpp b/src/test/app/Check_test.cpp index eb234cea56..b7d158041b 100644 --- a/src/test/app/Check_test.cpp +++ b/src/test/app/Check_test.cpp @@ -67,8 +67,6 @@ public: class Check_test : public beast::unit_test::suite { - FeatureBitset const disallowIncoming{featureDisallowIncoming}; - static uint256 getCheckIndex(AccountID const& account, std::uint32_t uSequence) { @@ -258,24 +256,12 @@ class Check_test : public beast::unit_test::suite using namespace test::jtx; - // test flag doesn't set unless amendment enabled - { - Env env{*this, features - disallowIncoming}; - Account const alice{"alice"}; - env.fund(XRP(10000), alice); - env(fset(alice, asfDisallowIncomingCheck)); - env.close(); - auto const sle = env.le(alice); - uint32_t flags = sle->getFlags(); - BEAST_EXPECT(!(flags & lsfDisallowIncomingCheck)); - } - Account const gw{"gateway"}; Account const alice{"alice"}; Account const bob{"bob"}; IOU const USD{gw["USD"]}; - Env env{*this, features | disallowIncoming}; + Env env{*this, features}; STAmount const startBalance{XRP(1000).value()}; env.fund(startBalance, gw, alice, bob); @@ -2594,7 +2580,6 @@ public: { using namespace test::jtx; auto const sa = testable_amendments(); - testWithFeats(sa - disallowIncoming); testWithFeats(sa); testTrustLineCreation(sa); } diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index a11446df71..12ac309acb 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -12,8 +12,6 @@ namespace ripple { class NFTokenBaseUtil_test : public beast::unit_test::suite { - FeatureBitset const disallowIncoming{featureDisallowIncoming}; - // Helper function that returns the number of NFTs minted by an issuer. static std::uint32_t mintedCount(test::jtx::Env const& env, test::jtx::Account const& issuer) @@ -2960,19 +2958,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite using namespace test::jtx; - // test flag doesn't set unless amendment enabled - { - Env env{*this, features - disallowIncoming}; - Account const alice{"alice"}; - env.fund(XRP(10000), alice); - env(fset(alice, asfDisallowIncomingNFTokenOffer)); - env.close(); - auto const sle = env.le(alice); - uint32_t flags = sle->getFlags(); - BEAST_EXPECT(!(flags & lsfDisallowIncomingNFTokenOffer)); - } - - Env env{*this, features | disallowIncoming}; + Env env{*this, features}; Account const issuer{"issuer"}; Account const minter{"minter"}; @@ -7624,8 +7610,8 @@ class NFTokenDisallowIncoming_test : public NFTokenBaseUtil_test run() override { testWithFeats( - allFeatures - featureDisallowIncoming - fixNFTokenReserve - - featureNFTokenMintOffer - featureDynamicNFT); + allFeatures - fixNFTokenReserve - featureNFTokenMintOffer - + featureDynamicNFT); } }; diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index bee3167b29..59c404ae28 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -14,8 +14,6 @@ using namespace jtx::paychan; struct PayChan_test : public beast::unit_test::suite { - FeatureBitset const disallowIncoming{featureDisallowIncoming}; - static std::pair> channelKeyAndSle( ReadView const& view, @@ -242,20 +240,8 @@ struct PayChan_test : public beast::unit_test::suite testcase("Disallow Incoming Flag"); using namespace jtx; - // test flag doesn't set unless amendment enabled - { - Env env{*this, features - disallowIncoming}; - Account const alice{"alice"}; - env.fund(XRP(10000), alice); - env(fset(alice, asfDisallowIncomingPayChan)); - env.close(); - auto const sle = env.le(alice); - uint32_t flags = sle->getFlags(); - BEAST_EXPECT(!(flags & lsfDisallowIncomingPayChan)); - } - using namespace std::literals::chrono_literals; - Env env{*this, features | disallowIncoming}; + Env env{*this, features}; auto const alice = Account("alice"); auto const bob = Account("bob"); auto const cho = Account("cho"); @@ -2127,7 +2113,6 @@ public: { using namespace test::jtx; FeatureBitset const all{testable_amendments()}; - testWithFeats(all - disallowIncoming); testWithFeats(all); testDepositAuthCreds(); testMetaAndOwnership(all - fixIncludeKeyletFields); diff --git a/src/test/app/SetTrust_test.cpp b/src/test/app/SetTrust_test.cpp index 88e359ab23..07573cf13f 100644 --- a/src/test/app/SetTrust_test.cpp +++ b/src/test/app/SetTrust_test.cpp @@ -9,8 +9,6 @@ namespace test { class SetTrust_test : public beast::unit_test::suite { - FeatureBitset const disallowIncoming{featureDisallowIncoming}; - public: void testTrustLineDelete() @@ -478,25 +476,12 @@ public: using namespace test::jtx; - // test flag doesn't set unless amendment enabled - { - Env env{*this, features - disallowIncoming}; - Account const alice{"alice"}; - env.fund(XRP(10000), alice); - env(fset(alice, asfDisallowIncomingTrustline)); - env.close(); - auto const sle = env.le(alice); - uint32_t flags = sle->getFlags(); - BEAST_EXPECT(!(flags & lsfDisallowIncomingTrustline)); - } - // fixDisallowIncomingV1 { for (bool const withFix : {true, false}) { - auto const amend = withFix - ? features | disallowIncoming - : (features | disallowIncoming) - fixDisallowIncomingV1; + auto const amend = + withFix ? features : features - fixDisallowIncomingV1; Env env{*this, amend}; auto const dist = Account("dist"); @@ -532,7 +517,7 @@ public: } } - Env env{*this, features | disallowIncoming}; + Env env{*this, features}; auto const gw = Account{"gateway"}; auto const alice = Account{"alice"}; @@ -630,7 +615,6 @@ public: { using namespace test::jtx; auto const sa = testable_amendments(); - testWithFeats(sa - disallowIncoming); testWithFeats(sa); } }; diff --git a/src/test/rpc/AccountInfo_test.cpp b/src/test/rpc/AccountInfo_test.cpp index 86d0a4b4f6..5e2fc608b3 100644 --- a/src/test/rpc/AccountInfo_test.cpp +++ b/src/test/rpc/AccountInfo_test.cpp @@ -615,33 +615,23 @@ public: {"disallowIncomingTrustline", asfDisallowIncomingTrustline}}}; - if (features[featureDisallowIncoming]) + for (auto& asf : disallowIncomingFlags) { - for (auto& asf : disallowIncomingFlags) - { - // Clear a flag and check that account_info returns results - // as expected - env(fclear(alice, asf.second)); - env.close(); - auto const f1 = getAccountFlag(asf.first, alice); - BEAST_EXPECT(f1.has_value()); - BEAST_EXPECT(!f1.value()); + // Clear a flag and check that account_info returns results + // as expected + env(fclear(alice, asf.second)); + env.close(); + auto const f1 = getAccountFlag(asf.first, alice); + BEAST_EXPECT(f1.has_value()); + BEAST_EXPECT(!f1.value()); - // Set a flag and check that account_info returns results - // as expected - env(fset(alice, asf.second)); - env.close(); - auto const f2 = getAccountFlag(asf.first, alice); - BEAST_EXPECT(f2.has_value()); - BEAST_EXPECT(f2.value()); - } - } - else - { - for (auto& asf : disallowIncomingFlags) - { - BEAST_EXPECT(!getAccountFlag(asf.first, alice)); - } + // Set a flag and check that account_info returns results + // as expected + env(fset(alice, asf.second)); + env.close(); + auto const f2 = getAccountFlag(asf.first, alice); + BEAST_EXPECT(f2.has_value()); + BEAST_EXPECT(f2.value()); } static constexpr std::pair @@ -706,12 +696,8 @@ public: FeatureBitset const allFeatures{ ripple::test::jtx::testable_amendments()}; testAccountFlags(allFeatures); - testAccountFlags(allFeatures - featureDisallowIncoming); - testAccountFlags( - allFeatures - featureDisallowIncoming - featureClawback); - testAccountFlags( - allFeatures - featureDisallowIncoming - featureClawback - - featureTokenEscrow); + testAccountFlags(allFeatures - featureClawback); + testAccountFlags(allFeatures - featureClawback - featureTokenEscrow); } }; diff --git a/src/xrpld/app/tx/detail/CreateCheck.cpp b/src/xrpld/app/tx/detail/CreateCheck.cpp index f5a36e6ac5..df859c4364 100644 --- a/src/xrpld/app/tx/detail/CreateCheck.cpp +++ b/src/xrpld/app/tx/detail/CreateCheck.cpp @@ -61,8 +61,7 @@ CreateCheck::preclaim(PreclaimContext const& ctx) auto const flags = sleDst->getFlags(); // Check if the destination has disallowed incoming checks - if (ctx.view.rules().enabled(featureDisallowIncoming) && - (flags & lsfDisallowIncomingCheck)) + if (flags & lsfDisallowIncomingCheck) return tecNO_PERMISSION; // Pseudo-accounts cannot cash checks. Note, this is not amendment-gated diff --git a/src/xrpld/app/tx/detail/NFTokenUtils.cpp b/src/xrpld/app/tx/detail/NFTokenUtils.cpp index 67f3a0de5f..db2b00cb2c 100644 --- a/src/xrpld/app/tx/detail/NFTokenUtils.cpp +++ b/src/xrpld/app/tx/detail/NFTokenUtils.cpp @@ -919,31 +919,21 @@ tokenOfferCreatePreclaim( return tecNO_DST; // check if the destination has disallowed incoming offers - if (view.rules().enabled(featureDisallowIncoming)) - { - // flag cannot be set unless amendment is enabled but - // out of an abundance of caution check anyway - - if (sleDst->getFlags() & lsfDisallowIncomingNFTokenOffer) - return tecNO_PERMISSION; - } + if (sleDst->getFlags() & lsfDisallowIncomingNFTokenOffer) + return tecNO_PERMISSION; } if (owner) { - // Check if the owner (buy offer) has disallowed incoming offers - if (view.rules().enabled(featureDisallowIncoming)) - { - auto const sleOwner = view.read(keylet::account(*owner)); + auto const sleOwner = view.read(keylet::account(*owner)); - // defensively check - // it should not be possible to specify owner that doesn't exist - if (!sleOwner) - return tecNO_TARGET; + // defensively check + // it should not be possible to specify owner that doesn't exist + if (!sleOwner) + return tecNO_TARGET; - if (sleOwner->getFlags() & lsfDisallowIncomingNFTokenOffer) - return tecNO_PERMISSION; - } + if (sleOwner->getFlags() & lsfDisallowIncomingNFTokenOffer) + return tecNO_PERMISSION; } if (view.rules().enabled(fixEnforceNFTokenTrustlineV2) && !amount.native()) diff --git a/src/xrpld/app/tx/detail/PayChan.cpp b/src/xrpld/app/tx/detail/PayChan.cpp index c3dc99e7fb..b52ae78523 100644 --- a/src/xrpld/app/tx/detail/PayChan.cpp +++ b/src/xrpld/app/tx/detail/PayChan.cpp @@ -202,8 +202,7 @@ PayChanCreate::preclaim(PreclaimContext const& ctx) auto const flags = sled->getFlags(); // Check if they have disallowed incoming payment channels - if (ctx.view.rules().enabled(featureDisallowIncoming) && - (flags & lsfDisallowIncomingPayChan)) + if (flags & lsfDisallowIncomingPayChan) return tecNO_PERMISSION; if ((flags & lsfRequireDestTag) && !ctx.tx[~sfDestinationTag]) diff --git a/src/xrpld/app/tx/detail/SetAccount.cpp b/src/xrpld/app/tx/detail/SetAccount.cpp index db513d9f05..e860eca22d 100644 --- a/src/xrpld/app/tx/detail/SetAccount.cpp +++ b/src/xrpld/app/tx/detail/SetAccount.cpp @@ -595,29 +595,25 @@ SetAccount::doApply() sle->isFieldPresent(sfNFTokenMinter)) sle->makeFieldAbsent(sfNFTokenMinter); - // Set or clear flags for disallowing various incoming instruments - if (ctx_.view().rules().enabled(featureDisallowIncoming)) - { - if (uSetFlag == asfDisallowIncomingNFTokenOffer) - uFlagsOut |= lsfDisallowIncomingNFTokenOffer; - else if (uClearFlag == asfDisallowIncomingNFTokenOffer) - uFlagsOut &= ~lsfDisallowIncomingNFTokenOffer; + if (uSetFlag == asfDisallowIncomingNFTokenOffer) + uFlagsOut |= lsfDisallowIncomingNFTokenOffer; + else if (uClearFlag == asfDisallowIncomingNFTokenOffer) + uFlagsOut &= ~lsfDisallowIncomingNFTokenOffer; - if (uSetFlag == asfDisallowIncomingCheck) - uFlagsOut |= lsfDisallowIncomingCheck; - else if (uClearFlag == asfDisallowIncomingCheck) - uFlagsOut &= ~lsfDisallowIncomingCheck; + if (uSetFlag == asfDisallowIncomingCheck) + uFlagsOut |= lsfDisallowIncomingCheck; + else if (uClearFlag == asfDisallowIncomingCheck) + uFlagsOut &= ~lsfDisallowIncomingCheck; - if (uSetFlag == asfDisallowIncomingPayChan) - uFlagsOut |= lsfDisallowIncomingPayChan; - else if (uClearFlag == asfDisallowIncomingPayChan) - uFlagsOut &= ~lsfDisallowIncomingPayChan; + if (uSetFlag == asfDisallowIncomingPayChan) + uFlagsOut |= lsfDisallowIncomingPayChan; + else if (uClearFlag == asfDisallowIncomingPayChan) + uFlagsOut &= ~lsfDisallowIncomingPayChan; - if (uSetFlag == asfDisallowIncomingTrustline) - uFlagsOut |= lsfDisallowIncomingTrustline; - else if (uClearFlag == asfDisallowIncomingTrustline) - uFlagsOut &= ~lsfDisallowIncomingTrustline; - } + if (uSetFlag == asfDisallowIncomingTrustline) + uFlagsOut |= lsfDisallowIncomingTrustline; + else if (uClearFlag == asfDisallowIncomingTrustline) + uFlagsOut &= ~lsfDisallowIncomingTrustline; // Set or clear flags for disallowing escrow if (ctx_.view().rules().enabled(featureTokenEscrow)) diff --git a/src/xrpld/app/tx/detail/SetTrust.cpp b/src/xrpld/app/tx/detail/SetTrust.cpp index dcbdbc013f..37ecdf2386 100644 --- a/src/xrpld/app/tx/detail/SetTrust.cpp +++ b/src/xrpld/app/tx/detail/SetTrust.cpp @@ -200,31 +200,27 @@ SetTrust::preclaim(PreclaimContext const& ctx) // This might be nullptr auto const sleDst = ctx.view.read(keylet::account(uDstAccountID)); - if ((ctx.view.rules().enabled(featureDisallowIncoming) || - ammEnabled(ctx.view.rules()) || + if ((ammEnabled(ctx.view.rules()) || ctx.view.rules().enabled(featureSingleAssetVault)) && sleDst == nullptr) return tecNO_DST; // If the destination has opted to disallow incoming trustlines // then honour that flag - if (ctx.view.rules().enabled(featureDisallowIncoming)) + if (sleDst->getFlags() & lsfDisallowIncomingTrustline) { - if (sleDst->getFlags() & lsfDisallowIncomingTrustline) + // The original implementation of featureDisallowIncoming was + // too restrictive. If + // o fixDisallowIncomingV1 is enabled and + // o The trust line already exists + // Then allow the TrustSet. + if (ctx.view.rules().enabled(fixDisallowIncomingV1) && + ctx.view.exists(keylet::line(id, uDstAccountID, currency))) { - // The original implementation of featureDisallowIncoming was - // too restrictive. If - // o fixDisallowIncomingV1 is enabled and - // o The trust line already exists - // Then allow the TrustSet. - if (ctx.view.rules().enabled(fixDisallowIncomingV1) && - ctx.view.exists(keylet::line(id, uDstAccountID, currency))) - { - // pass - } - else - return tecNO_PERMISSION; + // pass } + else + return tecNO_PERMISSION; } // In general, trust lines to pseudo accounts are not permitted, unless diff --git a/src/xrpld/rpc/handlers/AccountInfo.cpp b/src/xrpld/rpc/handlers/AccountInfo.cpp index 3caafc411b..7365ccc6ef 100644 --- a/src/xrpld/rpc/handlers/AccountInfo.cpp +++ b/src/xrpld/rpc/handlers/AccountInfo.cpp @@ -116,11 +116,8 @@ doAccountInfo(RPC::JsonContext& context) for (auto const& lsf : lsFlags) acctFlags[lsf.first.data()] = sleAccepted->isFlag(lsf.second); - if (ledger->rules().enabled(featureDisallowIncoming)) - { - for (auto const& lsf : disallowIncomingFlags) - acctFlags[lsf.first.data()] = sleAccepted->isFlag(lsf.second); - } + for (auto const& lsf : disallowIncomingFlags) + acctFlags[lsf.first.data()] = sleAccepted->isFlag(lsf.second); if (ledger->rules().enabled(featureClawback)) acctFlags[allowTrustLineClawbackFlag.first.data()] = From fb74dc28e1b7cb9f121d024dd896c82f95f538a3 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Fri, 21 Nov 2025 17:11:00 +0000 Subject: [PATCH 03/11] chore: Clean up comment in NetworkOps_test.cpp (#6066) This change removes a copyright notice that was accidentally copied over from another file. --- .github/scripts/rename/copyright.sh | 3 --- src/test/app/NetworkOPs_test.cpp | 2 -- 2 files changed, 5 deletions(-) diff --git a/.github/scripts/rename/copyright.sh b/.github/scripts/rename/copyright.sh index c0f64de496..41dbdc1f94 100755 --- a/.github/scripts/rename/copyright.sh +++ b/.github/scripts/rename/copyright.sh @@ -70,9 +70,6 @@ fi if ! grep -q 'Dev Null' src/test/app/tx/apply_test.cpp; then echo -e "// Copyright (c) 2020 Dev Null Productions\n\n$(cat src/test/app/tx/apply_test.cpp)" > src/test/app/tx/apply_test.cpp fi -if ! grep -q 'Dev Null' src/test/app/NetworkOPs_test.cpp; then - echo -e "// Copyright (c) 2020 Dev Null Productions\n\n$(cat src/test/app/NetworkOPs_test.cpp)" > src/test/app/NetworkOPs_test.cpp -fi if ! grep -q 'Dev Null' src/test/rpc/ManifestRPC_test.cpp; then echo -e "// Copyright (c) 2020 Dev Null Productions\n\n$(cat src/test/rpc/ManifestRPC_test.cpp)" > src/test/rpc/ManifestRPC_test.cpp fi diff --git a/src/test/app/NetworkOPs_test.cpp b/src/test/app/NetworkOPs_test.cpp index d9afe6d628..3965778221 100644 --- a/src/test/app/NetworkOPs_test.cpp +++ b/src/test/app/NetworkOPs_test.cpp @@ -1,5 +1,3 @@ -// Copyright (c) 2020 Dev Null Productions - #include #include #include From 58e03190ac360c53d120e69ab22e96899c824278 Mon Sep 17 00:00:00 2001 From: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Date: Fri, 21 Nov 2025 22:59:12 +0100 Subject: [PATCH 04/11] docs: Improve `VaultWithdraw` documentation (#6068) --- src/xrpld/app/tx/detail/VaultWithdraw.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/xrpld/app/tx/detail/VaultWithdraw.cpp b/src/xrpld/app/tx/detail/VaultWithdraw.cpp index 8cd3f5cd97..7777f2257f 100644 --- a/src/xrpld/app/tx/detail/VaultWithdraw.cpp +++ b/src/xrpld/app/tx/detail/VaultWithdraw.cpp @@ -121,6 +121,8 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) if (auto const ret = checkFrozen(ctx.view, dstAcct, vaultAsset)) return ret; + // Cannot return shares to the vault, if the underlying asset was frozen for + // the submitter if (auto const ret = checkFrozen(ctx.view, account, vaultShare)) return ret; From 8449c6c3655c29c8ee9e3407aa6420f445069388 Mon Sep 17 00:00:00 2001 From: Olek <115580134+oleks-rip@users.noreply.github.com> Date: Fri, 21 Nov 2025 17:20:45 -0500 Subject: [PATCH 05/11] Fix: nullptr resolving without db config (#6029) If the config disables SQL db usage, such as a validator: ``` [ledger_tx_tables] use_tx_tables = 0 ``` then the pointer to DB engine is null, but it was still resolved during startup. Although it didn't crash in Release mode, possibly due to the compiler optimizing it away, it did crash in Debug mode. This change explicitly checks for the validity of the pointer and generates a runtime error if not set. --- src/xrpld/app/rdb/backend/detail/Node.cpp | 12 ++++++++++-- src/xrpld/app/rdb/backend/detail/Node.h | 2 +- src/xrpld/app/rdb/backend/detail/SQLiteDatabase.cpp | 3 +-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/xrpld/app/rdb/backend/detail/Node.cpp b/src/xrpld/app/rdb/backend/detail/Node.cpp index ed8c5c06aa..04f328390d 100644 --- a/src/xrpld/app/rdb/backend/detail/Node.cpp +++ b/src/xrpld/app/rdb/backend/detail/Node.cpp @@ -171,7 +171,7 @@ getRowsMinMax(soci::session& session, TableType type) bool saveValidatedLedger( DatabaseCon& ldgDB, - DatabaseCon& txnDB, + std::unique_ptr const& txnDB, Application& app, std::shared_ptr const& ledger, bool current) @@ -254,7 +254,15 @@ saveValidatedLedger( if (app.config().useTxTables()) { - auto db = txnDB.checkoutDb(); + if (!txnDB) + { + // LCOV_EXCL_START + JLOG(j.fatal()) << "TxTables db isn't available"; + Throw("TxTables db isn't available"); + // LCOV_EXCL_STOP + } + + auto db = txnDB->checkoutDb(); soci::transaction tr(*db); diff --git a/src/xrpld/app/rdb/backend/detail/Node.h b/src/xrpld/app/rdb/backend/detail/Node.h index 412631571e..2c0fd69445 100644 --- a/src/xrpld/app/rdb/backend/detail/Node.h +++ b/src/xrpld/app/rdb/backend/detail/Node.h @@ -111,7 +111,7 @@ getRowsMinMax(soci::session& session, TableType type); bool saveValidatedLedger( DatabaseCon& ldgDB, - DatabaseCon& txnDB, + std::unique_ptr const& txnDB, Application& app, std::shared_ptr const& ledger, bool current); diff --git a/src/xrpld/app/rdb/backend/detail/SQLiteDatabase.cpp b/src/xrpld/app/rdb/backend/detail/SQLiteDatabase.cpp index 4934205bfd..944ed814f5 100644 --- a/src/xrpld/app/rdb/backend/detail/SQLiteDatabase.cpp +++ b/src/xrpld/app/rdb/backend/detail/SQLiteDatabase.cpp @@ -392,8 +392,7 @@ SQLiteDatabaseImp::saveValidatedLedger( { if (existsLedger()) { - if (!detail::saveValidatedLedger( - *lgrdb_, *txdb_, app_, ledger, current)) + if (!detail::saveValidatedLedger(*lgrdb_, txdb_, app_, ledger, current)) return false; } From 800a315383631e3460a51ee164c87d4032687e28 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Mon, 24 Nov 2025 11:23:16 +0000 Subject: [PATCH 06/11] refactor: Retire CryptoConditionsSuite amendment (#6036) Amendments activated for more than 2 years can be retired. This change retires the CryptoConditionsSuite amendment. --- include/xrpl/protocol/detail/features.macro | 7 ++++--- src/test/rpc/Feature_test.cpp | 17 ++++++++++++++++- src/xrpld/app/tx/detail/Escrow.cpp | 6 ------ 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 4180f1e7ff..d7a182faf0 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -79,9 +79,9 @@ XRPL_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYe // enabled (added to the ledger). // // If a feature remains obsolete for long enough that no clients are able -// to vote for it, the feature can be removed (entirely?) from the code. -XRPL_FEATURE(CryptoConditionsSuite, Supported::yes, VoteBehavior::Obsolete) - +// to vote for it, the feature can be removed entirely from the code. Until +// then the feature needs to be marked explicitly as obsolete, e.g. +// XRPL_FEATURE(Example, Supported::yes, 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. // All known amendments and amendments that may appear in a validated ledger @@ -117,6 +117,7 @@ XRPL_RETIRE_FIX(TrustLinesToSelf) XRPL_RETIRE_FEATURE(Checks) XRPL_RETIRE_FEATURE(CheckCashMakesTrustLine) XRPL_RETIRE_FEATURE(CryptoConditions) +XRPL_RETIRE_FEATURE(CryptoConditionsSuite) XRPL_RETIRE_FEATURE(DepositAuth) XRPL_RETIRE_FEATURE(DepositPreauth) XRPL_RETIRE_FEATURE(DisallowIncoming) diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index 10f82ac88d..2158524e3e 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -529,7 +529,22 @@ class Feature_test : public beast::unit_test::suite using namespace test::jtx; Env env{*this}; - constexpr char const* featureName = "CryptoConditionsSuite"; + + auto const& supportedAmendments = detail::supportedAmendments(); + auto obsoleteFeature = std::find_if( + std::begin(supportedAmendments), + std::end(supportedAmendments), + [](auto const& pair) { + return pair.second == VoteBehavior::Obsolete; + }); + + if (obsoleteFeature == std::end(supportedAmendments)) + { + pass(); + return; + } + + auto const featureName = obsoleteFeature->first; auto jrr = env.rpc("feature", featureName)[jss::result]; if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) diff --git a/src/xrpld/app/tx/detail/Escrow.cpp b/src/xrpld/app/tx/detail/Escrow.cpp index 5cf90809a2..c22b8145c6 100644 --- a/src/xrpld/app/tx/detail/Escrow.cpp +++ b/src/xrpld/app/tx/detail/Escrow.cpp @@ -153,12 +153,6 @@ EscrowCreate::preflight(PreflightContext const& ctx) << ec.message(); return temMALFORMED; } - - // Conditions other than PrefixSha256 require the - // "CryptoConditionsSuite" amendment: - if (condition->type != Type::preimageSha256 && - !ctx.rules.enabled(featureCryptoConditionsSuite)) - return temDISABLED; } return tesSUCCESS; From a791c03dc16085a7beb0f7683b1d7d3c6d1b8a6f Mon Sep 17 00:00:00 2001 From: Jingchen Date: Mon, 24 Nov 2025 11:52:08 +0000 Subject: [PATCH 07/11] refactor: Retire DeletableAccounts amendment (#6056) Amendments activated for more than 2 years can be retired. This change retires the DeletableAccounts amendment. --- include/xrpl/protocol/detail/features.macro | 2 +- .../xrpl/protocol/detail/transactions.macro | 2 +- src/libxrpl/ledger/View.cpp | 2 +- src/test/app/AccountDelete_test.cpp | 55 ------------------- src/test/app/NFToken_test.cpp | 7 --- src/test/app/TxQ_test.cpp | 2 +- src/test/rpc/Feature_test.cpp | 3 +- src/test/rpc/LedgerClosed_test.cpp | 2 +- src/test/rpc/LedgerRPC_test.cpp | 4 +- src/test/rpc/LedgerRequestRPC_test.cpp | 18 +++--- src/xrpld/app/tx/detail/DeleteAccount.cpp | 3 - src/xrpld/app/tx/detail/InvariantCheck.cpp | 7 +-- src/xrpld/app/tx/detail/Payment.cpp | 6 +- src/xrpld/app/tx/detail/XChainBridge.cpp | 5 +- 14 files changed, 20 insertions(+), 98 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index d7a182faf0..d06d3030f6 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_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(RequireFullyCanonicalSig, Supported::yes, VoteBehavior::DefaultYes) -XRPL_FEATURE(DeletableAccounts, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYes) // The following amendments are obsolete, but must remain supported @@ -118,6 +117,7 @@ XRPL_RETIRE_FEATURE(Checks) XRPL_RETIRE_FEATURE(CheckCashMakesTrustLine) XRPL_RETIRE_FEATURE(CryptoConditions) XRPL_RETIRE_FEATURE(CryptoConditionsSuite) +XRPL_RETIRE_FEATURE(DeletableAccounts) XRPL_RETIRE_FEATURE(DepositAuth) XRPL_RETIRE_FEATURE(DepositPreauth) XRPL_RETIRE_FEATURE(DisallowIncoming) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index c3cc2cef76..e551c89e55 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -297,7 +297,7 @@ TRANSACTION(ttTRUST_SET, 20, TrustSet, #endif TRANSACTION(ttACCOUNT_DELETE, 21, AccountDelete, Delegation::notDelegatable, - featureDeletableAccounts, + uint256{}, mustDeleteAcct, ({ {sfDestination, soeREQUIRED}, diff --git a/src/libxrpl/ledger/View.cpp b/src/libxrpl/ledger/View.cpp index 069bd3a4d8..6356dd1131 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/src/libxrpl/ledger/View.cpp @@ -1863,7 +1863,7 @@ rippleSendIOU( // Direct send: redeeming IOUs and/or sending own IOUs. auto const ter = rippleCreditIOU(view, uSenderID, uReceiverID, saAmount, false, j); - if (view.rules().enabled(featureDeletableAccounts) && ter != tesSUCCESS) + if (ter != tesSUCCESS) return ter; saActual = saAmount; return tesSUCCESS; diff --git a/src/test/app/AccountDelete_test.cpp b/src/test/app/AccountDelete_test.cpp index 9d5d1cd877..f22f9c4165 100644 --- a/src/test/app/AccountDelete_test.cpp +++ b/src/test/app/AccountDelete_test.cpp @@ -418,60 +418,6 @@ public: env.close(); } - void - testAmendmentEnable() - { - // Start with the featureDeletableAccounts amendment disabled. - // Then enable the amendment and delete an account. - using namespace jtx; - - testcase("Amendment enable"); - - Env env{*this, testable_amendments() - featureDeletableAccounts}; - Account const alice("alice"); - Account const becky("becky"); - - env.fund(XRP(10000), alice, becky); - env.close(); - - // Close enough ledgers to be able to delete alice's account. - incLgrSeqForAccDel(env, alice); - - // Verify that alice's account root is present. - Keylet const aliceAcctKey{keylet::account(alice.id())}; - BEAST_EXPECT(env.closed()->exists(aliceAcctKey)); - - auto const alicePreDelBal{env.balance(alice)}; - auto const beckyPreDelBal{env.balance(becky)}; - - auto const acctDelFee{drops(env.current()->fees().increment)}; - env(acctdelete(alice, becky), fee(acctDelFee), ter(temDISABLED)); - env.close(); - - // Verify that alice's account root is still present and alice and - // becky both have their XRP. - BEAST_EXPECT(env.current()->exists(aliceAcctKey)); - BEAST_EXPECT(env.balance(alice) == alicePreDelBal); - BEAST_EXPECT(env.balance(becky) == beckyPreDelBal); - - // When the amendment is enabled the previous transaction is - // retried into the new open ledger and succeeds. - env.enableFeature(featureDeletableAccounts); - env.close(); - - // alice's account is still in the most recently closed ledger. - BEAST_EXPECT(env.closed()->exists(aliceAcctKey)); - - // Verify that alice's account root is gone from the current ledger - // and becky has alice's XRP. - BEAST_EXPECT(!env.current()->exists(aliceAcctKey)); - BEAST_EXPECT( - env.balance(becky) == alicePreDelBal + beckyPreDelBal - acctDelFee); - - env.close(); - BEAST_EXPECT(!env.closed()->exists(aliceAcctKey)); - } - void testTooManyOffers() { @@ -1148,7 +1094,6 @@ public: testBasics(); testDirectories(); testOwnedTypes(); - testAmendmentEnable(); testTooManyOffers(); testImplicitlyCreatedTrustline(); testBalanceTooSmallForFee(); diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 12ac309acb..5f57322be1 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -5720,7 +5720,6 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite // Close the ledger until the ledger sequence is large enough to delete // the account (no longer within ) - // This is enforced by the featureDeletableAccounts amendment auto incLgrSeqForAcctDel = [&](Env& env, Account const& acct) { int const delta = [&]() -> int { if (env.seq(acct) + 255 > openLedgerSeq(env)) @@ -5839,8 +5838,6 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite } env.close(); - // Increment ledger sequence to the number that is - // enforced by the featureDeletableAccounts amendment incLgrSeqForAcctDel(env, alice); // Verify that alice's account root is present. @@ -5943,8 +5940,6 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite BEAST_EXPECT(ticketCount(env, alice) == 0); - // Increment ledger sequence to the number that is - // enforced by the featureDeletableAccounts amendment incLgrSeqForAcctDel(env, alice); // Verify that alice's account root is present. @@ -6053,8 +6048,6 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite BEAST_EXPECT(ticketCount(env, minter) == 0); - // Increment ledger sequence to the number that is - // enforced by the featureDeletableAccounts amendment incLgrSeqForAcctDel(env, alice); // Verify that alice's account root is present. diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index 4b7f156738..8e78969397 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -4330,7 +4330,7 @@ public: Account const ellie("ellie"); Account const fiona("fiona"); - constexpr int ledgersInQueue = 10; + constexpr int ledgersInQueue = 20; auto cfg = makeConfig( {{"minimum_txn_in_ledger_standalone", "1"}, {"ledgers_in_queue", std::to_string(ledgersInQueue)}, diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index 2158524e3e..374e4735c8 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -124,8 +124,7 @@ class Feature_test : public beast::unit_test::suite featureToName(fixRemoveNFTokenAutoTrustLine) == "fixRemoveNFTokenAutoTrustLine"); BEAST_EXPECT(featureToName(featureFlow) == "Flow"); - BEAST_EXPECT( - featureToName(featureDeletableAccounts) == "DeletableAccounts"); + BEAST_EXPECT(featureToName(featureDID) == "DID"); BEAST_EXPECT( featureToName(fixIncludeKeyletFields) == "fixIncludeKeyletFields"); BEAST_EXPECT(featureToName(featureTokenEscrow) == "TokenEscrow"); diff --git a/src/test/rpc/LedgerClosed_test.cpp b/src/test/rpc/LedgerClosed_test.cpp index b3a5e60afd..ba25a115c3 100644 --- a/src/test/rpc/LedgerClosed_test.cpp +++ b/src/test/rpc/LedgerClosed_test.cpp @@ -38,7 +38,7 @@ public: lc_result = env.rpc("ledger_closed")[jss::result]; BEAST_EXPECT( lc_result[jss::ledger_hash] == - "E86DE7F3D7A4D9CE17EF7C8BA08A8F4D8F643B9552F0D895A31CDA78F541DE4E"); + "0F1A9E0C109ADEF6DA2BDE19217C12BBEC57174CBDBD212B0EBDC1CEDB853185"); BEAST_EXPECT(lc_result[jss::ledger_index] == 3); } diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index 2d9daa5d9d..57a20527d0 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -307,8 +307,8 @@ class LedgerRPC_test : public beast::unit_test::suite { std::string const hash3{ - "E86DE7F3D7A4D9CE17EF7C8BA08A8F4D" - "8F643B9552F0D895A31CDA78F541DE4E"}; + "0F1A9E0C109ADEF6DA2BDE19217C12BBEC57174CBDBD212B0EBDC1CEDB8531" + "85"}; // access via the ledger_hash field Json::Value jvParams; jvParams[jss::ledger_hash] = hash3; diff --git a/src/test/rpc/LedgerRequestRPC_test.cpp b/src/test/rpc/LedgerRequestRPC_test.cpp index 5917dcedf2..0751b120aa 100644 --- a/src/test/rpc/LedgerRequestRPC_test.cpp +++ b/src/test/rpc/LedgerRequestRPC_test.cpp @@ -200,7 +200,7 @@ public: result = env.rpc("ledger_request", "3")[jss::result]; constexpr char const* hash3 = - "8D631B20BC989AF568FBA97375290544B0703A5ADC1CF9E9053580461690C9EE"; + "9FFD8AE09190D5002FE4252A1B29EABCF40DABBCE3B42619C6BD0BE381D51103"; BEAST_EXPECT(result[jss::ledger][jss::ledger_index] == "3"); BEAST_EXPECT( result[jss::ledger][jss::total_coins] == "99999999999999980"); @@ -209,14 +209,14 @@ public: BEAST_EXPECT(result[jss::ledger][jss::parent_hash] == hash2); BEAST_EXPECT( result[jss::ledger][jss::account_hash] == - "BC9EF2A16BFF80BCFABA6FA84688D858D33BD0FA0435CAA9DF6DA4105A39A29E"); + "35738B8517F37D08983AF6BC7DA483CCA9CF6B41B1FECB31A20028D78FE0BB22"); BEAST_EXPECT( result[jss::ledger][jss::transaction_hash] == - "0213EC486C058B3942FBE3DAC6839949A5C5B02B8B4244C8998EFDF04DBD8222"); + "CBD7F0948EBFA2241DE4EA627939A0FFEE6B80A90FE09C42C825DA546E9B73FF"); result = env.rpc("ledger_request", "4")[jss::result]; constexpr char const* hash4 = - "1A8E7098B23597E73094DADA58C9D62F3AB93A12C6F7666D56CA85A6CFDE530F"; + "7C9B614445517B8C6477E0AB10A35FFC1A23A34FEA41A91ECBDE884CC097C6E1"; BEAST_EXPECT(result[jss::ledger][jss::ledger_index] == "4"); BEAST_EXPECT( result[jss::ledger][jss::total_coins] == "99999999999999960"); @@ -225,14 +225,14 @@ public: BEAST_EXPECT(result[jss::ledger][jss::parent_hash] == hash3); BEAST_EXPECT( result[jss::ledger][jss::account_hash] == - "C690188F123C91355ADA8BDF4AC5B5C927076D3590C215096868A5255264C6DD"); + "1EE701DD2A150205173E1EDE8D474DF6803EC95253DAAEE965B9D896CFC32A04"); BEAST_EXPECT( result[jss::ledger][jss::transaction_hash] == - "3CBDB8F42E04333E1642166BFB93AC9A7E1C6C067092CD5D881D6F3AB3D67E76"); + "9BBDDBF926100DFFF364E16268F544B19F5B9BC6ECCBBC104F98D13FA9F3BC35"); result = env.rpc("ledger_request", "5")[jss::result]; constexpr char const* hash5 = - "C6A222D71AE65D7B4F240009EAD5DEB20D7EEDE5A4064F28BBDBFEEB6FBE48E5"; + "98885D02145CCE4AD2605F1809F17188DB2053B14ED399CAC985DD8E03DCA8C0"; BEAST_EXPECT(result[jss::ledger][jss::ledger_index] == "5"); BEAST_EXPECT( result[jss::ledger][jss::total_coins] == "99999999999999940"); @@ -241,10 +241,10 @@ public: BEAST_EXPECT(result[jss::ledger][jss::parent_hash] == hash4); BEAST_EXPECT( result[jss::ledger][jss::account_hash] == - "EA81CD9D36740736F00CB747E0D0E32D3C10B695823D961F0FB9A1CE7133DD4D"); + "41D64D64796468DEA7AE2A7282C0BB525D6FD7ABC29453C5E5BC6406E947CBCE"); BEAST_EXPECT( result[jss::ledger][jss::transaction_hash] == - "C3D086CD6BDB9E97AD1D513B2C049EF2840BD21D0B3E22D84EBBB89B6D2EF59D"); + "8FE8592EF22FBC2E8C774C7A1ED76AA3FCE64BED17D748CBA9AFDF7072FE36C7"); result = env.rpc("ledger_request", "6")[jss::result]; BEAST_EXPECT(result[jss::error] == "invalidParams"); diff --git a/src/xrpld/app/tx/detail/DeleteAccount.cpp b/src/xrpld/app/tx/detail/DeleteAccount.cpp index 0654c8dbce..c9fd0cc75e 100644 --- a/src/xrpld/app/tx/detail/DeleteAccount.cpp +++ b/src/xrpld/app/tx/detail/DeleteAccount.cpp @@ -22,9 +22,6 @@ namespace ripple { bool DeleteAccount::checkExtraFeatures(PreflightContext const& ctx) { - if (!ctx.rules.enabled(featureDeletableAccounts)) - return false; - if (ctx.tx.isFieldPresent(sfCredentialIDs) && !ctx.rules.enabled(featureCredentials)) return false; diff --git a/src/xrpld/app/tx/detail/InvariantCheck.cpp b/src/xrpld/app/tx/detail/InvariantCheck.cpp index c15f2b64a5..80c41f4d2b 100644 --- a/src/xrpld/app/tx/detail/InvariantCheck.cpp +++ b/src/xrpld/app/tx/detail/InvariantCheck.cpp @@ -1028,12 +1028,7 @@ ValidNewAccountRoot::finalize( return false; } - std::uint32_t const startingSeq = // - pseudoAccount // - ? 0 // - : view.rules().enabled(featureDeletableAccounts) // - ? view.seq() // - : 1; + std::uint32_t const startingSeq = pseudoAccount ? 0 : view.seq(); if (accountSeq_ != startingSeq) { diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index 3aff4db5e1..56aeb1b0aa 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -398,14 +398,10 @@ Payment::doApply() if (!sleDst) { - std::uint32_t const seqno{ - view().rules().enabled(featureDeletableAccounts) ? view().seq() - : 1}; - // Create the account. sleDst = std::make_shared(k); sleDst->setAccountID(sfAccount, dstAccountID); - sleDst->setFieldU32(sfSequence, seqno); + sleDst->setFieldU32(sfSequence, view().seq()); view().insert(sleDst); } diff --git a/src/xrpld/app/tx/detail/XChainBridge.cpp b/src/xrpld/app/tx/detail/XChainBridge.cpp index 554d4a8436..e555dca0a1 100644 --- a/src/xrpld/app/tx/detail/XChainBridge.cpp +++ b/src/xrpld/app/tx/detail/XChainBridge.cpp @@ -464,12 +464,9 @@ transferHelper( } // Create the account. - std::uint32_t const seqno{ - psb.rules().enabled(featureDeletableAccounts) ? psb.seq() : 1}; - sleDst = std::make_shared(dstK); sleDst->setAccountID(sfAccount, dst); - sleDst->setFieldU32(sfSequence, seqno); + sleDst->setFieldU32(sfSequence, psb.seq()); psb.insert(sleDst); } From 21c02232a54e0069a7a0e7c40e0d74259d46ecec Mon Sep 17 00:00:00 2001 From: Jingchen Date: Mon, 24 Nov 2025 13:58:37 +0000 Subject: [PATCH 08/11] refactor: Retire RequireFullyCanonicalSig amendment (#6035) Amendments activated for more than 2 years can be retired. This change retires the RequireFullyCanonicalSig amendment. --- include/xrpl/protocol/PublicKey.h | 6 +- include/xrpl/protocol/STTx.h | 36 ++-------- include/xrpl/protocol/detail/features.macro | 2 +- src/libxrpl/protocol/PublicKey.cpp | 9 +-- src/libxrpl/protocol/STTx.cpp | 76 ++++++--------------- src/test/app/tx/apply_test.cpp | 17 ----- src/test/ledger/View_test.cpp | 6 +- src/test/protocol/STTx_test.cpp | 3 +- src/test/protocol/SecretKey_test.cpp | 8 +-- src/test/rpc/Feature_test.cpp | 18 ++--- src/xrpld/app/tx/detail/Batch.cpp | 3 +- src/xrpld/app/tx/detail/PayChan.cpp | 2 +- src/xrpld/app/tx/detail/apply.cpp | 8 +-- src/xrpld/rpc/handlers/PayChanClaim.cpp | 3 +- 14 files changed, 51 insertions(+), 146 deletions(-) diff --git a/include/xrpl/protocol/PublicKey.h b/include/xrpl/protocol/PublicKey.h index 445e0e3b97..8d8062408b 100644 --- a/include/xrpl/protocol/PublicKey.h +++ b/include/xrpl/protocol/PublicKey.h @@ -231,11 +231,7 @@ verifyDigest( SHA512-Half, and the resulting digest is signed. */ [[nodiscard]] bool -verify( - PublicKey const& publicKey, - Slice const& m, - Slice const& sig, - bool mustBeFullyCanonical = true) noexcept; +verify(PublicKey const& publicKey, Slice const& m, Slice const& sig) noexcept; /** Calculate the 160-bit node ID from a node public key. */ NodeID diff --git a/include/xrpl/protocol/STTx.h b/include/xrpl/protocol/STTx.h index 716881d910..02f865e349 100644 --- a/include/xrpl/protocol/STTx.h +++ b/include/xrpl/protocol/STTx.h @@ -103,22 +103,15 @@ public: std::optional> signatureTarget = {}); - enum class RequireFullyCanonicalSig : bool { no, yes }; - /** Check the signature. - @param requireCanonicalSig If `true`, check that the signature is fully - canonical. If `false`, only check that the signature is valid. @param rules The current ledger rules. @return `true` if valid signature. If invalid, the error message string. */ Expected - checkSign(RequireFullyCanonicalSig requireCanonicalSig, Rules const& rules) - const; + checkSign(Rules const& rules) const; Expected - checkBatchSign( - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules) const; + checkBatchSign(Rules const& rules) const; // SQL Functions with metadata. static std::string const& @@ -140,40 +133,25 @@ public: private: /** Check the signature. - @param requireCanonicalSig If `true`, check that the signature is fully - canonical. If `false`, only check that the signature is valid. @param rules The current ledger rules. @param sigObject Reference to object that contains the signature fields. Will be *this more often than not. @return `true` if valid signature. If invalid, the error message string. */ Expected - checkSign( - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules, - STObject const& sigObject) const; + checkSign(Rules const& rules, STObject const& sigObject) const; Expected - checkSingleSign( - RequireFullyCanonicalSig requireCanonicalSig, - STObject const& sigObject) const; + checkSingleSign(STObject const& sigObject) const; Expected - checkMultiSign( - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules, - STObject const& sigObject) const; + checkMultiSign(Rules const& rules, STObject const& sigObject) const; Expected - checkBatchSingleSign( - STObject const& batchSigner, - RequireFullyCanonicalSig requireCanonicalSig) const; + checkBatchSingleSign(STObject const& batchSigner) const; Expected - checkBatchMultiSign( - STObject const& batchSigner, - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules) const; + checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const; STBase* copy(std::size_t n, void* buf) const override; diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index d06d3030f6..054b9d0a03 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -65,7 +65,6 @@ XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes) -XRPL_FEATURE(RequireFullyCanonicalSig, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYes) // The following amendments are obsolete, but must remain supported @@ -133,6 +132,7 @@ XRPL_RETIRE_FEATURE(MultiSignReserve) XRPL_RETIRE_FEATURE(NegativeUNL) XRPL_RETIRE_FEATURE(NonFungibleTokensV1_1) XRPL_RETIRE_FEATURE(PayChan) +XRPL_RETIRE_FEATURE(RequireFullyCanonicalSig) XRPL_RETIRE_FEATURE(SortedDirectories) XRPL_RETIRE_FEATURE(TicketBatch) XRPL_RETIRE_FEATURE(TickSize) diff --git a/src/libxrpl/protocol/PublicKey.cpp b/src/libxrpl/protocol/PublicKey.cpp index 0a29af7d32..f1794331ad 100644 --- a/src/libxrpl/protocol/PublicKey.cpp +++ b/src/libxrpl/protocol/PublicKey.cpp @@ -267,18 +267,13 @@ verifyDigest( } bool -verify( - PublicKey const& publicKey, - Slice const& m, - Slice const& sig, - bool mustBeFullyCanonical) noexcept +verify(PublicKey const& publicKey, Slice const& m, Slice const& sig) noexcept { if (auto const type = publicKeyType(publicKey)) { if (*type == KeyType::secp256k1) { - return verifyDigest( - publicKey, sha512Half(m), sig, mustBeFullyCanonical); + return verifyDigest(publicKey, sha512Half(m), sig); } else if (*type == KeyType::ed25519) { diff --git a/src/libxrpl/protocol/STTx.cpp b/src/libxrpl/protocol/STTx.cpp index dbcf1f304e..b89f337288 100644 --- a/src/libxrpl/protocol/STTx.cpp +++ b/src/libxrpl/protocol/STTx.cpp @@ -237,10 +237,7 @@ STTx::sign( } Expected -STTx::checkSign( - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules, - STObject const& sigObject) const +STTx::checkSign(Rules const& rules, STObject const& sigObject) const { try { @@ -249,9 +246,8 @@ STTx::checkSign( // multi-signing. Otherwise we're single-signing. Blob const& signingPubKey = sigObject.getFieldVL(sfSigningPubKey); - return signingPubKey.empty() - ? checkMultiSign(requireCanonicalSig, rules, sigObject) - : checkSingleSign(requireCanonicalSig, sigObject); + return signingPubKey.empty() ? checkMultiSign(rules, sigObject) + : checkSingleSign(sigObject); } catch (std::exception const&) { @@ -260,18 +256,16 @@ STTx::checkSign( } Expected -STTx::checkSign( - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules) const +STTx::checkSign(Rules const& rules) const { - if (auto const ret = checkSign(requireCanonicalSig, rules, *this); !ret) + if (auto const ret = checkSign(rules, *this); !ret) return ret; /* Placeholder for field that will be added by Lending Protocol if (isFieldPresent(sfCounterpartySignature)) { auto const counterSig = getFieldObject(sfCounterpartySignature); - if (auto const ret = checkSign(requireCanonicalSig, rules, counterSig); + if (auto const ret = checkSign(rules, counterSig); !ret) return Unexpected("Counterparty: " + ret.error()); } @@ -280,9 +274,7 @@ STTx::checkSign( } Expected -STTx::checkBatchSign( - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules) const +STTx::checkBatchSign(Rules const& rules) const { try { @@ -299,8 +291,8 @@ STTx::checkBatchSign( { Blob const& signingPubKey = signer.getFieldVL(sfSigningPubKey); auto const result = signingPubKey.empty() - ? checkBatchMultiSign(signer, requireCanonicalSig, rules) - : checkBatchSingleSign(signer, requireCanonicalSig); + ? checkBatchMultiSign(signer, rules) + : checkBatchSingleSign(signer); if (!result) return result; @@ -395,10 +387,7 @@ STTx::getMetaSQL( } static Expected -singleSignHelper( - STObject const& sigObject, - Slice const& data, - bool const fullyCanonical) +singleSignHelper(STObject const& sigObject, Slice const& data) { // We don't allow both a non-empty sfSigningPubKey and an sfSigners. // That would allow the transaction to be signed two ways. So if both @@ -413,11 +402,8 @@ singleSignHelper( if (publicKeyType(makeSlice(spk))) { Blob const signature = sigObject.getFieldVL(sfTxnSignature); - validSig = verify( - PublicKey(makeSlice(spk)), - data, - makeSlice(signature), - fullyCanonical); + validSig = + verify(PublicKey(makeSlice(spk)), data, makeSlice(signature)); } } catch (std::exception const&) @@ -432,33 +418,24 @@ singleSignHelper( } Expected -STTx::checkSingleSign( - RequireFullyCanonicalSig requireCanonicalSig, - STObject const& sigObject) const +STTx::checkSingleSign(STObject const& sigObject) const { auto const data = getSigningData(*this); - bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) || - (requireCanonicalSig == STTx::RequireFullyCanonicalSig::yes); - return singleSignHelper(sigObject, makeSlice(data), fullyCanonical); + return singleSignHelper(sigObject, makeSlice(data)); } Expected -STTx::checkBatchSingleSign( - STObject const& batchSigner, - RequireFullyCanonicalSig requireCanonicalSig) const +STTx::checkBatchSingleSign(STObject const& batchSigner) const { Serializer msg; serializeBatch(msg, getFlags(), getBatchTransactionIDs()); - bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) || - (requireCanonicalSig == STTx::RequireFullyCanonicalSig::yes); - return singleSignHelper(batchSigner, msg.slice(), fullyCanonical); + return singleSignHelper(batchSigner, msg.slice()); } Expected multiSignHelper( STObject const& sigObject, std::optional txnAccountID, - bool const fullyCanonical, std::function makeMsg, Rules const& rules) { @@ -515,8 +492,7 @@ multiSignHelper( validSig = verify( PublicKey(makeSlice(spk)), makeMsg(accountID).slice(), - makeSlice(signature), - fullyCanonical); + makeSlice(signature)); } } catch (std::exception const& e) @@ -535,14 +511,8 @@ multiSignHelper( } Expected -STTx::checkBatchMultiSign( - STObject const& batchSigner, - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules) const +STTx::checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const { - bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) || - (requireCanonicalSig == RequireFullyCanonicalSig::yes); - // We can ease the computational load inside the loop a bit by // pre-constructing part of the data that we hash. Fill a Serializer // with the stuff that stays constant from signature to signature. @@ -551,7 +521,6 @@ STTx::checkBatchMultiSign( return multiSignHelper( batchSigner, std::nullopt, - fullyCanonical, [&dataStart](AccountID const& accountID) -> Serializer { Serializer s = dataStart; finishMultiSigningData(accountID, s); @@ -561,14 +530,8 @@ STTx::checkBatchMultiSign( } Expected -STTx::checkMultiSign( - RequireFullyCanonicalSig requireCanonicalSig, - Rules const& rules, - STObject const& sigObject) const +STTx::checkMultiSign(Rules const& rules, STObject const& sigObject) const { - bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) || - (requireCanonicalSig == RequireFullyCanonicalSig::yes); - // Used inside the loop in multiSignHelper to enforce that // the account owner may not multisign for themselves. auto const txnAccountID = &sigObject != this @@ -582,7 +545,6 @@ STTx::checkMultiSign( return multiSignHelper( sigObject, txnAccountID, - fullyCanonical, [&dataStart](AccountID const& accountID) -> Serializer { Serializer s = dataStart; finishMultiSigningData(accountID, s); diff --git a/src/test/app/tx/apply_test.cpp b/src/test/app/tx/apply_test.cpp index 236905cefd..308ad3d3b8 100644 --- a/src/test/app/tx/apply_test.cpp +++ b/src/test/app/tx/apply_test.cpp @@ -35,23 +35,6 @@ public: SerialIter sitTrans(makeSlice(*ret)); STTx const tx = *std::make_shared(std::ref(sitTrans)); - { - test::jtx::Env no_fully_canonical( - *this, - test::jtx::testable_amendments() - - featureRequireFullyCanonicalSig); - - Validity valid = checkValidity( - no_fully_canonical.app().getHashRouter(), - tx, - no_fully_canonical.current()->rules(), - no_fully_canonical.app().config()) - .first; - - if (valid != Validity::Valid) - fail("Non-Fully canonical signature was not permitted"); - } - { test::jtx::Env fully_canonical( *this, test::jtx::testable_amendments()); diff --git a/src/test/ledger/View_test.cpp b/src/test/ledger/View_test.cpp index b7dbf4ec9e..ab978fda59 100644 --- a/src/test/ledger/View_test.cpp +++ b/src/test/ledger/View_test.cpp @@ -1114,10 +1114,10 @@ class GetAmendments_test : public beast::unit_test::suite break; } - // There should be at least 5 amendments. Don't do exact comparison + // There should be at least 3 amendments. Don't do exact comparison // to avoid maintenance as more amendments are added in the future. BEAST_EXPECT(i == 254); - BEAST_EXPECT(majorities.size() >= 5); + BEAST_EXPECT(majorities.size() >= 3); // None of the amendments should be enabled yet. auto enableds = getEnabledAmendments(*env.closed()); @@ -1135,7 +1135,7 @@ class GetAmendments_test : public beast::unit_test::suite break; } BEAST_EXPECT(i == 255); - BEAST_EXPECT(enableds.size() >= 5); + BEAST_EXPECT(enableds.size() >= 3); } void diff --git a/src/test/protocol/STTx_test.cpp b/src/test/protocol/STTx_test.cpp index 8a434486ea..50cd8c511f 100644 --- a/src/test/protocol/STTx_test.cpp +++ b/src/test/protocol/STTx_test.cpp @@ -1615,8 +1615,7 @@ public: BEAST_EXPECT(!defaultRules.enabled(featureAMM)); unexpected( - !j.checkSign(STTx::RequireFullyCanonicalSig::yes, defaultRules), - "Transaction fails signature test"); + !j.checkSign(defaultRules), "Transaction fails signature test"); Serializer rawTxn; j.add(rawTxn); diff --git a/src/test/protocol/SecretKey_test.cpp b/src/test/protocol/SecretKey_test.cpp index bd7e2ccfe4..803988e9d5 100644 --- a/src/test/protocol/SecretKey_test.cpp +++ b/src/test/protocol/SecretKey_test.cpp @@ -145,7 +145,7 @@ public: auto sig = sign(pk, sk, makeSlice(data)); BEAST_EXPECT(sig.size() != 0); - BEAST_EXPECT(verify(pk, makeSlice(data), sig, true)); + BEAST_EXPECT(verify(pk, makeSlice(data), sig)); // Construct wrong data: auto badData = data; @@ -156,17 +156,17 @@ public: std::max_element(badData.begin(), badData.end())); // Wrong data: should fail - BEAST_EXPECT(!verify(pk, makeSlice(badData), sig, true)); + BEAST_EXPECT(!verify(pk, makeSlice(badData), sig)); // Slightly change the signature: if (auto ptr = sig.data()) ptr[j % sig.size()]++; // Wrong signature: should fail - BEAST_EXPECT(!verify(pk, makeSlice(data), sig, true)); + BEAST_EXPECT(!verify(pk, makeSlice(data), sig)); // Wrong data and signature: should fail - BEAST_EXPECT(!verify(pk, makeSlice(badData), sig, true)); + BEAST_EXPECT(!verify(pk, makeSlice(badData), sig)); } } } diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index 374e4735c8..e01f7566ac 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -183,16 +183,16 @@ class Feature_test : public beast::unit_test::suite using namespace test::jtx; Env env{*this}; - auto jrr = env.rpc("feature", "RequireFullyCanonicalSig")[jss::result]; + auto jrr = env.rpc("feature", "Flow")[jss::result]; BEAST_EXPECTS(jrr[jss::status] == jss::success, "status"); jrr.removeMember(jss::status); BEAST_EXPECT(jrr.size() == 1); BEAST_EXPECT( - jrr.isMember("00C1FC4A53E60AB02C864641002B3172F38677E29C26C54066851" - "79B37E1EDAC")); + jrr.isMember("740352F2412A9909880C23A559FCECEDA3BE2126FED62FC7660D6" + "28A06927F11")); auto feature = *(jrr.begin()); - BEAST_EXPECTS(feature[jss::name] == "RequireFullyCanonicalSig", "name"); + BEAST_EXPECTS(feature[jss::name] == "Flow", "name"); BEAST_EXPECTS(!feature[jss::enabled].asBool(), "enabled"); BEAST_EXPECTS( feature[jss::vetoed].isBool() && !feature[jss::vetoed].asBool(), @@ -200,7 +200,7 @@ class Feature_test : public beast::unit_test::suite BEAST_EXPECTS(feature[jss::supported].asBool(), "supported"); // feature names are case-sensitive - expect error here - jrr = env.rpc("feature", "requireFullyCanonicalSig")[jss::result]; + jrr = env.rpc("feature", "flow")[jss::result]; BEAST_EXPECT(jrr[jss::error] == "badFeature"); BEAST_EXPECT(jrr[jss::error_message] == "Feature unknown or invalid."); } @@ -419,9 +419,9 @@ class Feature_test : public beast::unit_test::suite break; } - // There should be at least 5 amendments. Don't do exact comparison + // There should be at least 3 amendments. Don't do exact comparison // to avoid maintenance as more amendments are added in the future. - BEAST_EXPECT(majorities.size() >= 5); + BEAST_EXPECT(majorities.size() >= 3); std::map const& votes = ripple::detail::supportedAmendments(); @@ -476,8 +476,8 @@ class Feature_test : public beast::unit_test::suite testcase("Veto"); using namespace test::jtx; - Env env{*this, FeatureBitset{featureRequireFullyCanonicalSig}}; - constexpr char const* featureName = "RequireFullyCanonicalSig"; + Env env{*this, FeatureBitset{featureFlow}}; + constexpr char const* featureName = "Flow"; auto jrr = env.rpc("feature", featureName)[jss::result]; if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) diff --git a/src/xrpld/app/tx/detail/Batch.cpp b/src/xrpld/app/tx/detail/Batch.cpp index a2c51d6c82..b233249170 100644 --- a/src/xrpld/app/tx/detail/Batch.cpp +++ b/src/xrpld/app/tx/detail/Batch.cpp @@ -451,8 +451,7 @@ Batch::preflightSigValidated(PreflightContext const& ctx) } // Check the batch signers signatures. - auto const sigResult = ctx.tx.checkBatchSign( - STTx::RequireFullyCanonicalSig::yes, ctx.rules); + auto const sigResult = ctx.tx.checkBatchSign(ctx.rules); if (!sigResult) { diff --git a/src/xrpld/app/tx/detail/PayChan.cpp b/src/xrpld/app/tx/detail/PayChan.cpp index b52ae78523..ae47036ab6 100644 --- a/src/xrpld/app/tx/detail/PayChan.cpp +++ b/src/xrpld/app/tx/detail/PayChan.cpp @@ -440,7 +440,7 @@ PayChanClaim::preflight(PreflightContext const& ctx) PublicKey const pk(ctx.tx[sfPublicKey]); Serializer msg; serializePayChanAuthorization(msg, k.key, authAmt); - if (!verify(pk, msg.slice(), *sig, /*canonical*/ true)) + if (!verify(pk, msg.slice(), *sig)) return temBAD_SIGNATURE; } diff --git a/src/xrpld/app/tx/detail/apply.cpp b/src/xrpld/app/tx/detail/apply.cpp index e9709c20f5..06be378c2b 100644 --- a/src/xrpld/app/tx/detail/apply.cpp +++ b/src/xrpld/app/tx/detail/apply.cpp @@ -58,13 +58,7 @@ checkValidity( if (!any(flags & SF_SIGGOOD)) { - // Don't know signature state. Check it. - auto const requireCanonicalSig = - rules.enabled(featureRequireFullyCanonicalSig) - ? STTx::RequireFullyCanonicalSig::yes - : STTx::RequireFullyCanonicalSig::no; - - auto const sigVerify = tx.checkSign(requireCanonicalSig, rules); + auto const sigVerify = tx.checkSign(rules); if (!sigVerify) { router.setFlags(id, SF_SIGBAD); diff --git a/src/xrpld/rpc/handlers/PayChanClaim.cpp b/src/xrpld/rpc/handlers/PayChanClaim.cpp index dd36a873f0..95c4ba5f3d 100644 --- a/src/xrpld/rpc/handlers/PayChanClaim.cpp +++ b/src/xrpld/rpc/handlers/PayChanClaim.cpp @@ -137,8 +137,7 @@ doChannelVerify(RPC::JsonContext& context) serializePayChanAuthorization(msg, channelId, XRPAmount(drops)); Json::Value result; - result[jss::signature_verified] = - verify(*pk, msg.slice(), makeSlice(*sig), /*canonical*/ true); + result[jss::signature_verified] = verify(*pk, msg.slice(), makeSlice(*sig)); return result; } From b550dc00edc62f2974f2eaee50d58cd1aecdca80 Mon Sep 17 00:00:00 2001 From: Bart Date: Mon, 24 Nov 2025 21:43:39 -0500 Subject: [PATCH 09/11] ci: Remove missing commits check (#6077) This change removes the CI check for missing commits, as well as a stray path to the publish-docs workflow that isn't used in the on-trigger workflow. --- .github/workflows/on-trigger.yml | 6 -- .../reusable-check-missing-commits.yml | 62 ------------------- 2 files changed, 68 deletions(-) delete mode 100644 .github/workflows/reusable-check-missing-commits.yml diff --git a/.github/workflows/on-trigger.yml b/.github/workflows/on-trigger.yml index 9df6417c07..b5a56fb671 100644 --- a/.github/workflows/on-trigger.yml +++ b/.github/workflows/on-trigger.yml @@ -14,9 +14,7 @@ on: - "master" paths: # These paths are unique to `on-trigger.yml`. - - ".github/workflows/reusable-check-missing-commits.yml" - ".github/workflows/on-trigger.yml" - - ".github/workflows/publish-docs.yml" # Keep the paths below in sync with those in `on-pr.yml`. - ".github/actions/build-deps/**" @@ -63,10 +61,6 @@ defaults: shell: bash jobs: - check-missing-commits: - if: ${{ github.event_name == 'push' && github.ref_type == 'branch' && contains(fromJSON('["develop", "release"]'), github.ref_name) }} - uses: ./.github/workflows/reusable-check-missing-commits.yml - build-test: uses: ./.github/workflows/reusable-build-test.yml strategy: diff --git a/.github/workflows/reusable-check-missing-commits.yml b/.github/workflows/reusable-check-missing-commits.yml deleted file mode 100644 index 07d29174d8..0000000000 --- a/.github/workflows/reusable-check-missing-commits.yml +++ /dev/null @@ -1,62 +0,0 @@ -# This workflow checks that all commits in the "master" branch are also in the -# "release" and "develop" branches, and that all commits in the "release" branch -# are also in the "develop" branch. -name: Check for missing commits - -# This workflow can only be triggered by other workflows. -on: workflow_call - -concurrency: - group: ${{ github.workflow }}-${{ github.ref }}-missing-commits - cancel-in-progress: true - -defaults: - run: - shell: bash - -jobs: - check: - runs-on: ubuntu-latest - steps: - - name: Checkout repository - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 - with: - fetch-depth: 0 - - name: Check for missing commits - env: - MESSAGE: | - - If you are reading this, then the commits indicated above are missing - from the "develop" and/or "release" branch. Do a reverse-merge as soon - as possible. See CONTRIBUTING.md for instructions. - run: | - set -o pipefail - # Branches are ordered by how "canonical" they are. Every commit in one - # branch should be in all the branches behind it. - order=(master release develop) - branches=() - for branch in "${order[@]}"; do - # Check that the branches exist so that this job will work on forked - # repos, which don't necessarily have master and release branches. - echo "Checking if ${branch} exists." - if git ls-remote --exit-code --heads origin \ - refs/heads/${branch} > /dev/null; then - branches+=(origin/${branch}) - fi - done - - prior=() - for branch in "${branches[@]}"; do - if [[ ${#prior[@]} -ne 0 ]]; then - echo "Checking ${prior[@]} for commits missing from ${branch}." - git log --oneline --no-merges "${prior[@]}" \ - ^$branch | tee -a "missing-commits.txt" - echo - fi - prior+=("${branch}") - done - - if [[ $(cat missing-commits.txt | wc -l) -ne 0 ]]; then - echo "${MESSAGE}" - exit 1 - fi From b124c9f7e3383d6a64a4d7623f5ea089dbef61d2 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Tue, 25 Nov 2025 14:21:17 +0000 Subject: [PATCH 10/11] refactor: Retire Flow and FlowSortStrands amendments (#6054) Amendments activated for more than 2 years can be retired. This change retires the Flow and FlowSortStrands amendments. --- include/xrpl/protocol/detail/features.macro | 4 +- src/test/app/CrossingLimits_test.cpp | 39 ++++------------- src/test/app/TxQ_test.cpp | 4 +- src/test/jtx/Env_test.cpp | 20 ++++++--- src/test/ledger/View_test.cpp | 4 +- src/test/rpc/Feature_test.cpp | 20 ++++----- src/xrpld/app/paths/RippleCalc.cpp | 9 ---- src/xrpld/app/paths/detail/StrandFlow.h | 48 ++++----------------- 8 files changed, 47 insertions(+), 101 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 054b9d0a03..5f1eca7afe 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -64,8 +64,6 @@ XRPL_FEATURE(Clawback, Supported::yes, VoteBehavior::DefaultNo XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes) -XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes) -XRPL_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYes) // The following amendments are obsolete, but must remain supported // because they could potentially get enabled. @@ -124,7 +122,9 @@ XRPL_RETIRE_FEATURE(Escrow) XRPL_RETIRE_FEATURE(EnforceInvariants) XRPL_RETIRE_FEATURE(ExpandedSignerList) XRPL_RETIRE_FEATURE(FeeEscalation) +XRPL_RETIRE_FEATURE(Flow) XRPL_RETIRE_FEATURE(FlowCross) +XRPL_RETIRE_FEATURE(FlowSortStrands) XRPL_RETIRE_FEATURE(HardenedValidations) XRPL_RETIRE_FEATURE(ImmediateOfferKilled) XRPL_RETIRE_FEATURE(MultiSign) diff --git a/src/test/app/CrossingLimits_test.cpp b/src/test/app/CrossingLimits_test.cpp index 8f0483b590..13d7aef505 100644 --- a/src/test/app/CrossingLimits_test.cpp +++ b/src/test/app/CrossingLimits_test.cpp @@ -267,9 +267,8 @@ public: // strand dry until the liquidity is actually used) // The implementation allows any single step to consume at most 1000 - // offers. With the `FlowSortStrands` feature enabled, if the total - // number of offers consumed by all the steps combined exceeds 1500, the - // payment stops. + // offers.If the total number of offers consumed by all the steps + // combined exceeds 1500, the payment stops. { Env env(*this, features); @@ -457,16 +456,12 @@ public: // below the limit. However, if all the offers are consumed it would // create a tecOVERSIZE error. - // The featureFlowSortStrands introduces a way of tracking the total - // number of consumed offers; with this feature the transaction no - // longer fails with a tecOVERSIZE error. // The implementation allows any single step to consume at most 1000 - // offers. With the `FlowSortStrands` feature enabled, if the total - // number of offers consumed by all the steps combined exceeds 1500, the - // payment stops. Since the first set of offers consumes 998 offers, the - // second set will consume 998, which is not over the limit and the - // payment stops. So 2*998, or 1996 is the expected value when - // `FlowSortStrands` is enabled. + // offers. If the total number of offers consumed by all the steps + // combined exceeds 1500, the payment stops. Since the first set of + // offers consumes 998 offers, the second set will consume 998, which is + // not over the limit and the payment stops. So 2*998, or 1996 is the + // expected value. n_offers(env, 998, alice, XRP(1.00), USD(1)); n_offers(env, 998, alice, XRP(0.99), USD(1)); n_offers(env, 998, alice, XRP(0.98), USD(1)); @@ -474,24 +469,10 @@ public: n_offers(env, 998, alice, XRP(0.96), USD(1)); n_offers(env, 998, alice, XRP(0.95), USD(1)); - bool const withSortStrands = features[featureFlowSortStrands]; - - auto const expectedTER = [&]() -> TER { - if (!withSortStrands) - return TER{tecOVERSIZE}; - return tesSUCCESS; - }(); - - env(offer(bob, USD(8000), XRP(8000)), ter(expectedTER)); + env(offer(bob, USD(8000), XRP(8000)), ter(tesSUCCESS)); env.close(); - auto const expectedUSD = [&] { - if (!withSortStrands) - return USD(0); - return USD(1996); - }(); - - env.require(balance(bob, expectedUSD)); + env.require(balance(bob, USD(1996))); } void @@ -507,9 +488,7 @@ public: using namespace jtx; auto const sa = testable_amendments(); testAll(sa); - testAll(sa - featureFlowSortStrands); testAll(sa - featurePermissionedDEX); - testAll(sa - featureFlowSortStrands - featurePermissionedDEX); } }; diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index 8e78969397..e59cf7deff 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -1054,7 +1054,7 @@ public: // Charlie - queue a transaction, with a higher fee // than default env(noop(charlie), fee(15), queued); - checkMetrics(*this, env, 6, initQueueMax, 4, 3); + checkMetrics(*this, env, 6, initQueueMax, 4, 3, 257); BEAST_EXPECT(env.seq(alice) == aliceSeq); BEAST_EXPECT(env.seq(bob) == bobSeq); @@ -4330,7 +4330,7 @@ public: Account const ellie("ellie"); Account const fiona("fiona"); - constexpr int ledgersInQueue = 20; + constexpr int ledgersInQueue = 30; auto cfg = makeConfig( {{"minimum_txn_in_ledger_standalone", "1"}, {"ledgers_in_queue", std::to_string(ledgersInQueue)}, diff --git a/src/test/jtx/Env_test.cpp b/src/test/jtx/Env_test.cpp index 5e4f534b6a..fbc4bd37a2 100644 --- a/src/test/jtx/Env_test.cpp +++ b/src/test/jtx/Env_test.cpp @@ -798,16 +798,18 @@ public: { // a Env FeatureBitset has *only* those features - Env env{*this, FeatureBitset{featureDynamicMPT | featureFlow}}; + Env env{ + *this, FeatureBitset{featureDynamicMPT | featureTokenEscrow}}; BEAST_EXPECT(env.app().config().features.size() == 2); foreachFeature(supported, [&](uint256 const& f) { - bool const has = (f == featureDynamicMPT || f == featureFlow); + bool const has = + (f == featureDynamicMPT || f == featureTokenEscrow); this->BEAST_EXPECT(has == hasFeature(env, f)); }); } auto const missingSomeFeatures = - testable_amendments() - featureDynamicMPT - featureFlow; + testable_amendments() - featureDynamicMPT - featureTokenEscrow; BEAST_EXPECT(missingSomeFeatures.count() == (supported.count() - 2)); { // a Env supported_features_except is missing *only* those features @@ -815,7 +817,8 @@ public: BEAST_EXPECT( env.app().config().features.size() == (supported.count() - 2)); foreachFeature(supported, [&](uint256 const& f) { - bool hasnot = (f == featureDynamicMPT || f == featureFlow); + bool hasnot = + (f == featureDynamicMPT || f == featureTokenEscrow); this->BEAST_EXPECT(hasnot != hasFeature(env, f)); }); } @@ -828,7 +831,9 @@ public: Env env{ *this, FeatureBitset{ - featureDynamicMPT, featureFlow, *neverSupportedFeat}}; + featureDynamicMPT, + featureTokenEscrow, + *neverSupportedFeat}}; // this app will have just 2 supported amendments and // one additional never supported feature flag @@ -836,7 +841,7 @@ public: BEAST_EXPECT(hasFeature(env, *neverSupportedFeat)); foreachFeature(supported, [&](uint256 const& f) { - bool has = (f == featureDynamicMPT || f == featureFlow); + bool has = (f == featureDynamicMPT || f == featureTokenEscrow); this->BEAST_EXPECT(has == hasFeature(env, f)); }); } @@ -856,7 +861,8 @@ public: (supported.count() - 2 + 1)); BEAST_EXPECT(hasFeature(env, *neverSupportedFeat)); foreachFeature(supported, [&](uint256 const& f) { - bool hasnot = (f == featureDynamicMPT || f == featureFlow); + bool hasnot = + (f == featureDynamicMPT || f == featureTokenEscrow); this->BEAST_EXPECT(hasnot != hasFeature(env, f)); }); } diff --git a/src/test/ledger/View_test.cpp b/src/test/ledger/View_test.cpp index ab978fda59..9c7a5a286e 100644 --- a/src/test/ledger/View_test.cpp +++ b/src/test/ledger/View_test.cpp @@ -1117,7 +1117,7 @@ class GetAmendments_test : public beast::unit_test::suite // There should be at least 3 amendments. Don't do exact comparison // to avoid maintenance as more amendments are added in the future. BEAST_EXPECT(i == 254); - BEAST_EXPECT(majorities.size() >= 3); + BEAST_EXPECT(majorities.size() >= 2); // None of the amendments should be enabled yet. auto enableds = getEnabledAmendments(*env.closed()); @@ -1135,7 +1135,7 @@ class GetAmendments_test : public beast::unit_test::suite break; } BEAST_EXPECT(i == 255); - BEAST_EXPECT(enableds.size() >= 3); + BEAST_EXPECT(enableds.size() >= 2); } void diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index e01f7566ac..058b4a9572 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -123,7 +123,7 @@ class Feature_test : public beast::unit_test::suite BEAST_EXPECT( featureToName(fixRemoveNFTokenAutoTrustLine) == "fixRemoveNFTokenAutoTrustLine"); - BEAST_EXPECT(featureToName(featureFlow) == "Flow"); + BEAST_EXPECT(featureToName(featureBatch) == "Batch"); BEAST_EXPECT(featureToName(featureDID) == "DID"); BEAST_EXPECT( featureToName(fixIncludeKeyletFields) == "fixIncludeKeyletFields"); @@ -183,16 +183,16 @@ class Feature_test : public beast::unit_test::suite using namespace test::jtx; Env env{*this}; - auto jrr = env.rpc("feature", "Flow")[jss::result]; + auto jrr = env.rpc("feature", "fixAMMOverflowOffer")[jss::result]; BEAST_EXPECTS(jrr[jss::status] == jss::success, "status"); jrr.removeMember(jss::status); BEAST_EXPECT(jrr.size() == 1); BEAST_EXPECT( - jrr.isMember("740352F2412A9909880C23A559FCECEDA3BE2126FED62FC7660D6" - "28A06927F11")); + jrr.isMember("12523DF04B553A0B1AD74F42DDB741DE8DC06A03FC089A0EF197E" + "2A87F1D8107")); auto feature = *(jrr.begin()); - BEAST_EXPECTS(feature[jss::name] == "Flow", "name"); + BEAST_EXPECTS(feature[jss::name] == "fixAMMOverflowOffer", "name"); BEAST_EXPECTS(!feature[jss::enabled].asBool(), "enabled"); BEAST_EXPECTS( feature[jss::vetoed].isBool() && !feature[jss::vetoed].asBool(), @@ -200,7 +200,7 @@ class Feature_test : public beast::unit_test::suite BEAST_EXPECTS(feature[jss::supported].asBool(), "supported"); // feature names are case-sensitive - expect error here - jrr = env.rpc("feature", "flow")[jss::result]; + jrr = env.rpc("feature", "fMM")[jss::result]; BEAST_EXPECT(jrr[jss::error] == "badFeature"); BEAST_EXPECT(jrr[jss::error_message] == "Feature unknown or invalid."); } @@ -419,9 +419,9 @@ class Feature_test : public beast::unit_test::suite break; } - // There should be at least 3 amendments. Don't do exact comparison + // There should be at least 2 amendments. Don't do exact comparison // to avoid maintenance as more amendments are added in the future. - BEAST_EXPECT(majorities.size() >= 3); + BEAST_EXPECT(majorities.size() >= 2); std::map const& votes = ripple::detail::supportedAmendments(); @@ -476,8 +476,8 @@ class Feature_test : public beast::unit_test::suite testcase("Veto"); using namespace test::jtx; - Env env{*this, FeatureBitset{featureFlow}}; - constexpr char const* featureName = "Flow"; + Env env{*this, FeatureBitset{featurePriceOracle}}; + constexpr char const* featureName = "fixAMMOverflowOffer"; auto jrr = env.rpc("feature", featureName)[jss::result]; if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) diff --git a/src/xrpld/app/paths/RippleCalc.cpp b/src/xrpld/app/paths/RippleCalc.cpp index ac30932f11..a6751169d7 100644 --- a/src/xrpld/app/paths/RippleCalc.cpp +++ b/src/xrpld/app/paths/RippleCalc.cpp @@ -43,15 +43,6 @@ RippleCalc::rippleCalculate( PaymentSandbox flowSB(&view); auto j = l.journal("Flow"); - if (!view.rules().enabled(featureFlow)) - { - // The new payment engine was enabled several years ago. New transaction - // should never use the old rules. Assume this is a replay - j.fatal() - << "Old payment rules are required for this transaction. Assuming " - "this is a replay and running with the new rules."; - } - { bool const defaultPaths = !pInputs ? true : pInputs->defaultPathsAllowed; diff --git a/src/xrpld/app/paths/detail/StrandFlow.h b/src/xrpld/app/paths/detail/StrandFlow.h index c9a4619717..fb93f3fdb7 100644 --- a/src/xrpld/app/paths/detail/StrandFlow.h +++ b/src/xrpld/app/paths/detail/StrandFlow.h @@ -433,7 +433,7 @@ public: // add the strands in `next_` to `cur_`, sorted by theoretical quality. // Best quality first. cur_.clear(); - if (v.rules().enabled(featureFlowSortStrands) && !next_.empty()) + if (!next_.empty()) { std::vector> strandQuals; strandQuals.reserve(next_.size()); @@ -719,46 +719,16 @@ flow( continue; } - if (baseView.rules().enabled(featureFlowSortStrands)) - { - XRPL_ASSERT(!best, "ripple::flow : best is unset"); - if (!f.inactive) - activeStrands.push(strand); - best.emplace(f.in, f.out, std::move(*f.sandbox), *strand, q); - activeStrands.pushRemainingCurToNext(strandIndex + 1); - break; - } - - activeStrands.push(strand); - - if (!best || best->quality < q || - (best->quality == q && best->out < f.out)) - { - // If this strand is inactive (because it consumed too many - // offers) and ends up having the best quality, remove it - // from the activeStrands. If it doesn't end up having the - // best quality, keep it active. - - if (f.inactive) - { - // This should be `nextSize`, not `size`. This issue is - // fixed in featureFlowSortStrands. - markInactiveOnUse = activeStrands.size() - 1; - } - else - { - markInactiveOnUse.reset(); - } - - best.emplace(f.in, f.out, std::move(*f.sandbox), *strand, q); - } + XRPL_ASSERT(!best, "ripple::flow : best is unset"); + if (!f.inactive) + activeStrands.push(strand); + best.emplace(f.in, f.out, std::move(*f.sandbox), *strand, q); + activeStrands.pushRemainingCurToNext(strandIndex + 1); + break; } - bool const shouldBreak = [&] { - if (baseView.rules().enabled(featureFlowSortStrands)) - return !best || offersConsidered >= maxOffersToConsider; - return !best; - }(); + bool const shouldBreak = + !best || offersConsidered >= maxOffersToConsider; if (best) { From 856470203a15d03a9960bdd6e8cc4379db17738e Mon Sep 17 00:00:00 2001 From: Bart Date: Tue, 25 Nov 2025 16:03:17 -0500 Subject: [PATCH 11/11] ci: Trigger clio pipeline on PRs targeting release branches (#6080) This change triggers the Clio pipeline on PRs that target any of the `release*` branches (in addition to the `master` branch), as opposed to only the `release` branch. --- .github/workflows/on-pr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/on-pr.yml b/.github/workflows/on-pr.yml index 3f009f63bb..ff3d25812a 100644 --- a/.github/workflows/on-pr.yml +++ b/.github/workflows/on-pr.yml @@ -122,7 +122,7 @@ jobs: needs: - should-run - build-test - if: ${{ needs.should-run.outputs.go == 'true' && contains(fromJSON('["release", "master"]'), github.ref_name) }} + if: ${{ needs.should-run.outputs.go == 'true' && (startsWith(github.base_ref, 'release') || github.base_ref == 'master') }} uses: ./.github/workflows/reusable-notify-clio.yml secrets: clio_notify_token: ${{ secrets.CLIO_NOTIFY_TOKEN }}