From 6340c986c90b85ec9c2bf1c00de297299978f185 Mon Sep 17 00:00:00 2001 From: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Date: Wed, 13 May 2026 11:42:34 +0200 Subject: [PATCH] feat: Enable and rename fixSecurity3_1_3 to fixCleanup3_1_3 (#7128) Co-authored-by: Claude Opus 4.6 (1M context) --- include/xrpl/protocol/detail/features.macro | 2 +- .../tx/invariants/PermissionedDEXInvariant.h | 4 +-- .../ledger/helpers/CredentialHelpers.cpp | 2 +- src/libxrpl/ledger/helpers/LendingHelpers.cpp | 4 +-- src/libxrpl/ledger/helpers/MPTokenHelpers.cpp | 4 +-- .../ledger/helpers/PermissionedDEXHelpers.cpp | 6 ++-- src/libxrpl/ledger/helpers/TokenHelpers.cpp | 6 ++-- src/libxrpl/tx/invariants/InvariantCheck.cpp | 6 ++-- .../tx/invariants/LoanBrokerInvariant.cpp | 2 +- .../invariants/PermissionedDEXInvariant.cpp | 6 ++-- .../tx/transactors/lending/LoanManage.cpp | 2 +- .../tx/transactors/lending/LoanPay.cpp | 6 ++-- .../tx/transactors/nft/NFTokenAcceptOffer.cpp | 8 ++--- .../PermissionedDomainSet.cpp | 2 +- .../tx/transactors/vault/VaultClawback.cpp | 4 +-- .../tx/transactors/vault/VaultWithdraw.cpp | 4 +-- src/test/app/Credentials_test.cpp | 6 ++-- src/test/app/Invariants_test.cpp | 15 ++++----- src/test/app/Loan_test.cpp | 12 +++---- src/test/app/MPToken_test.cpp | 8 ++--- src/test/app/NFToken_test.cpp | 32 +++++++++---------- src/test/app/PermissionedDEX_test.cpp | 15 ++++----- src/test/app/TxQ_test.cpp | 23 +++++++------ src/test/app/Vault_test.cpp | 20 ++++++------ 24 files changed, 101 insertions(+), 98 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 2c2cd14e97..dc26a5a43b 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -17,7 +17,7 @@ XRPL_FIX (Cleanup3_2_0, Supported::No, VoteBehavior::DefaultNo) XRPL_FEATURE(MPTokensV2, Supported::No, VoteBehavior::DefaultNo) -XRPL_FIX (Security3_1_3, 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_FEATURE(LendingProtocol, Supported::Yes, VoteBehavior::DefaultNo) diff --git a/include/xrpl/tx/invariants/PermissionedDEXInvariant.h b/include/xrpl/tx/invariants/PermissionedDEXInvariant.h index da187779b2..3784a32840 100644 --- a/include/xrpl/tx/invariants/PermissionedDEXInvariant.h +++ b/include/xrpl/tx/invariants/PermissionedDEXInvariant.h @@ -11,8 +11,8 @@ namespace xrpl { class ValidPermissionedDEX { bool regularOffers_ = false; - bool badHybridsOld_ = false; // pre-fixSecurity3_1_3: missing field/domain or size > 1 - bool badHybrids_ = false; // post-fixSecurity3_1_3: also catches size == 0 (size != 1) + bool badHybridsOld_ = false; // pre-fixCleanup3_1_3: missing field/domain or size > 1 + bool badHybrids_ = false; // post-fixCleanup3_1_3: also catches size == 0 (size != 1) hash_set domains_; public: diff --git a/src/libxrpl/ledger/helpers/CredentialHelpers.cpp b/src/libxrpl/ledger/helpers/CredentialHelpers.cpp index 05e45a404c..cb0cbf6322 100644 --- a/src/libxrpl/ledger/helpers/CredentialHelpers.cpp +++ b/src/libxrpl/ledger/helpers/CredentialHelpers.cpp @@ -61,7 +61,7 @@ removeExpired(ApplyView& view, STVector256 const& arr, beast::Journal const j) JLOG(j.trace()) << "Credentials are expired. Cred: " << sleCred->getText(); // delete expired credentials even if the transaction failed auto const err = deleteSLE(view, sleCred, j); - if (view.rules().enabled(fixSecurity3_1_3) && !isTesSuccess(err)) + if (view.rules().enabled(fixCleanup3_1_3) && !isTesSuccess(err)) return Unexpected(err); foundExpired = true; } diff --git a/src/libxrpl/ledger/helpers/LendingHelpers.cpp b/src/libxrpl/ledger/helpers/LendingHelpers.cpp index a8bee7aecb..3bfa18e3e8 100644 --- a/src/libxrpl/ledger/helpers/LendingHelpers.cpp +++ b/src/libxrpl/ledger/helpers/LendingHelpers.cpp @@ -1964,13 +1964,13 @@ loanMakePayment( // ------------------------------------------------------------- // overpayment handling // - // If the "fixSecurity3_1_3" amendment is enabled, truncate "amount", + // If the "fixCleanup3_1_3" amendment is enabled, truncate "amount", // at the loan scale. If the raw value is used, the overpayment // amount could be meaningless dust. Trying to process such a small // amount will, at best, waste time when all the result values round // to zero. At worst, it can cause logical errors with tiny amounts // of interest that don't add up correctly. - auto const roundedAmount = view.rules().enabled(fixSecurity3_1_3) + auto const roundedAmount = view.rules().enabled(fixCleanup3_1_3) ? roundToAsset(asset, amount, loanScale, Number::RoundingMode::TowardsZero) : amount; if (paymentType == LoanPaymentType::Overpayment && loan->isFlag(lsfLoanOverpayment) && diff --git a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp index 75fe61b34b..252921c499 100644 --- a/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/MPTokenHelpers.cpp @@ -169,7 +169,7 @@ authorizeMPToken( auto const mptokenKey = keylet::mptoken(mptIssuanceID, account); auto const sleMpt = view.peek(mptokenKey); if (!sleMpt || (*sleMpt)[sfMPTAmount] != 0 || - (view.rules().enabled(fixSecurity3_1_3) && + (view.rules().enabled(fixCleanup3_1_3) && (*sleMpt)[~sfLockedAmount].valueOr(0) != 0)) return tecINTERNAL; // LCOV_EXCL_LINE @@ -284,7 +284,7 @@ removeEmptyHolding( // accounting out of balance, so fail. Since this should be impossible // anyway, I'm not going to put any effort into it. if (mptoken->at(sfMPTAmount) != 0 || - (view.rules().enabled(fixSecurity3_1_3) && (*mptoken)[~sfLockedAmount].valueOr(0) != 0)) + (view.rules().enabled(fixCleanup3_1_3) && (*mptoken)[~sfLockedAmount].valueOr(0) != 0)) return tecHAS_OBLIGATIONS; return authorizeMPToken( diff --git a/src/libxrpl/ledger/helpers/PermissionedDEXHelpers.cpp b/src/libxrpl/ledger/helpers/PermissionedDEXHelpers.cpp index 7b4194cccf..0151c5b2df 100644 --- a/src/libxrpl/ledger/helpers/PermissionedDEXHelpers.cpp +++ b/src/libxrpl/ledger/helpers/PermissionedDEXHelpers.cpp @@ -60,9 +60,9 @@ offerInDomain( if (sleOffer->getFieldH256(sfDomainID) != domainID) return false; // LCOV_EXCL_LINE - if (view.rules().enabled(fixSecurity3_1_3)) + if (view.rules().enabled(fixCleanup3_1_3)) { - // post-fixSecurity3_1_3: a valid hybrid offer must have + // post-fixCleanup3_1_3: a valid hybrid offer must have // sfAdditionalBooks present with exactly 1 entry if (sleOffer->isFlag(lsfHybrid) && (!sleOffer->isFieldPresent(sfAdditionalBooks) || @@ -75,7 +75,7 @@ offerInDomain( } else { - // pre-fixSecurity3_1_3: a valid hybrid offer must have + // pre-fixCleanup3_1_3: a valid hybrid offer must have // sfAdditionalBooks present (size is not checked) if (sleOffer->isFlag(lsfHybrid) && !sleOffer->isFieldPresent(sfAdditionalBooks)) { diff --git a/src/libxrpl/ledger/helpers/TokenHelpers.cpp b/src/libxrpl/ledger/helpers/TokenHelpers.cpp index 000f459ef6..0894fdba38 100644 --- a/src/libxrpl/ledger/helpers/TokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/TokenHelpers.cpp @@ -1222,9 +1222,9 @@ directSendNoLimitMultiMPT( std::uint64_t const sendAmount = amount.mpt().value(); - if (view.rules().enabled(fixSecurity3_1_3)) + if (view.rules().enabled(fixCleanup3_1_3)) { - // Post-fixSecurity3_1_3: aggregate MaximumAmount + // Post-fixCleanup3_1_3: aggregate MaximumAmount // check. WARNING: the order of conditions is // critical — each guards the subtraction in the // next against unsigned underflow. Do not reorder. @@ -1242,7 +1242,7 @@ directSendNoLimitMultiMPT( } else { - // Pre-fixSecurity3_1_3: per-iteration MaximumAmount + // Pre-fixCleanup3_1_3: per-iteration MaximumAmount // check. Reads sfOutstandingAmount from a stale // view.read() snapshot — incorrect for multi-destination // sends but retained for ledger replay compatibility. diff --git a/src/libxrpl/tx/invariants/InvariantCheck.cpp b/src/libxrpl/tx/invariants/InvariantCheck.cpp index e9c32bf14c..59887ad18c 100644 --- a/src/libxrpl/tx/invariants/InvariantCheck.cpp +++ b/src/libxrpl/tx/invariants/InvariantCheck.cpp @@ -341,7 +341,7 @@ NoZeroEscrow::visitEntry( bad_ |= true; }; - bool const overwriteFixEnabled = isFeatureEnabled(fixSecurity3_1_3, true); + bool const overwriteFixEnabled = isFeatureEnabled(fixCleanup3_1_3, true); if (after && after->getType() == ltMPTOKEN_ISSUANCE) { @@ -628,7 +628,7 @@ NoXRPTrustLines::visitEntry( std::shared_ptr const&, std::shared_ptr const& after) { - bool const overwriteFixEnabled = isFeatureEnabled(fixSecurity3_1_3, true); + bool const overwriteFixEnabled = isFeatureEnabled(fixCleanup3_1_3, true); if (after && after->getType() == ltRIPPLE_STATE) { @@ -673,7 +673,7 @@ NoDeepFreezeTrustLinesWithoutFreeze::visitEntry( { if (after && after->getType() == ltRIPPLE_STATE) { - bool const overwriteFixEnabled = isFeatureEnabled(fixSecurity3_1_3, true); + bool const overwriteFixEnabled = isFeatureEnabled(fixCleanup3_1_3, true); std::uint32_t const uFlags = after->getFieldU32(sfFlags); bool const lowFreeze = (uFlags & lsfLowFreeze) != 0u; diff --git a/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp b/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp index b98d769393..56995ef94c 100644 --- a/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp +++ b/src/libxrpl/tx/invariants/LoanBrokerInvariant.cpp @@ -197,7 +197,7 @@ ValidLoanBroker::finalize( return false; } - if (view.rules().enabled(fixSecurity3_1_3)) + if (view.rules().enabled(fixCleanup3_1_3)) { // Don't check the balance when LoanBroker is deleted, // sfCoverAvailable is not zeroed diff --git a/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp b/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp index 6466812743..b480b90a31 100644 --- a/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp +++ b/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp @@ -41,14 +41,14 @@ ValidPermissionedDEX::visitEntry( regularOffers_ = true; } - // pre-fixSecurity3_1_3: hybrid offer missing domain, missing + // pre-fixCleanup3_1_3: hybrid offer missing domain, missing // sfAdditionalBooks, or sfAdditionalBooks has more than one entry if (after->isFlag(lsfHybrid) && (!after->isFieldPresent(sfDomainID) || !after->isFieldPresent(sfAdditionalBooks) || after->getFieldArray(sfAdditionalBooks).size() > 1)) badHybridsOld_ = true; - // post-fixSecurity3_1_3: same as above but also catches size == 0 + // post-fixCleanup3_1_3: same as above but also catches size == 0 if (after->isFlag(lsfHybrid) && (!after->isFieldPresent(sfDomainID) || !after->isFieldPresent(sfAdditionalBooks) || after->getFieldArray(sfAdditionalBooks).size() != 1)) @@ -70,7 +70,7 @@ ValidPermissionedDEX::finalize( // For each offercreate transaction, check if // permissioned offers are valid - bool const isMalformed = view.rules().enabled(fixSecurity3_1_3) ? badHybrids_ : badHybridsOld_; + bool const isMalformed = view.rules().enabled(fixCleanup3_1_3) ? badHybrids_ : badHybridsOld_; if (txType == ttOFFER_CREATE && isMalformed) { JLOG(j.fatal()) << "Invariant failed: hybrid offer is malformed"; diff --git a/src/libxrpl/tx/transactors/lending/LoanManage.cpp b/src/libxrpl/tx/transactors/lending/LoanManage.cpp index 648f33b980..46c85c9f96 100644 --- a/src/libxrpl/tx/transactors/lending/LoanManage.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanManage.cpp @@ -424,7 +424,7 @@ LoanManage::doApply() // Pre-amendment, associateAsset was only called on the noop (no flags) // path. Post-amendment, we call associateAsset on all successful paths. - if (view.rules().enabled(fixSecurity3_1_3) && isTesSuccess(result)) + if (view.rules().enabled(fixCleanup3_1_3) && isTesSuccess(result)) { associateAsset(*loanSle, vaultAsset); associateAsset(*brokerSle, vaultAsset); diff --git a/src/libxrpl/tx/transactors/lending/LoanPay.cpp b/src/libxrpl/tx/transactors/lending/LoanPay.cpp index d7147128cc..a9668d9f4c 100644 --- a/src/libxrpl/tx/transactors/lending/LoanPay.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanPay.cpp @@ -147,7 +147,7 @@ LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx) std::int64_t constexpr kMAX_FEE_INCREMENTS = kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION / kLOAN_PAYMENTS_PER_FEE_INCREMENT; - if (view.rules().enabled(fixSecurity3_1_3) && + if (view.rules().enabled(fixCleanup3_1_3) && amount >= regularPayment * kLOAN_MAXIMUM_PAYMENTS_PER_TRANSACTION) { // The payment handler will never process more than @@ -169,7 +169,7 @@ LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx) std::int64_t(1), static_cast(numPaymentEstimate / kLOAN_PAYMENTS_PER_FEE_INCREMENT)); XRPL_ASSERT( - !view.rules().enabled(fixSecurity3_1_3) || feeIncrements <= kMAX_FEE_INCREMENTS, + !view.rules().enabled(fixCleanup3_1_3) || feeIncrements <= kMAX_FEE_INCREMENTS, "xrpl::LoanPay::calculateBaseFee : number of fee increments is in " "range"); @@ -201,7 +201,7 @@ LoanPay::preclaim(PreclaimContext const& ctx) if (tx.isFlag(tfLoanOverpayment) && !loanSle->isFlag(lsfLoanOverpayment)) { JLOG(ctx.j.warn()) << "Requested overpayment on a loan that doesn't allow it"; - return ctx.view.rules().enabled(fixSecurity3_1_3) ? TER{tecNO_PERMISSION} : temINVALID_FLAG; + return ctx.view.rules().enabled(fixCleanup3_1_3) ? TER{tecNO_PERMISSION} : temINVALID_FLAG; } auto const principalOutstanding = loanSle->at(sfPrincipalOutstanding); diff --git a/src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp b/src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp index 2aa2720e46..c46930ba08 100644 --- a/src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp +++ b/src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp @@ -68,10 +68,10 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) if (hasExpired(ctx.view, (*offerSLE)[~sfExpiration])) { - // Before fixSecurity3_1_3 amendment, expired offers caused tecEXPIRED in preclaim, + // Before fixCleanup3_1_3 amendment, expired offers caused tecEXPIRED in preclaim, // leaving them on ledger forever. After the amendment, we allow expired offers to // reach doApply() where they get deleted and tecEXPIRED is returned. - if (!ctx.view.rules().enabled(fixSecurity3_1_3)) + if (!ctx.view.rules().enabled(fixCleanup3_1_3)) return {nullptr, tecEXPIRED}; // Amendment enabled: return the expired offer to be handled in doApply. } @@ -450,9 +450,9 @@ NFTokenAcceptOffer::doApply() auto bo = loadToken(ctx_.tx[~sfNFTokenBuyOffer]); auto so = loadToken(ctx_.tx[~sfNFTokenSellOffer]); - // With fixSecurity3_1_3 amendment, check for expired offers and delete them, returning + // With fixCleanup3_1_3 amendment, check for expired offers and delete them, returning // tecEXPIRED. This ensures expired offers are properly cleaned up from the ledger. - if (view().rules().enabled(fixSecurity3_1_3)) + if (view().rules().enabled(fixCleanup3_1_3)) { bool foundExpired = false; diff --git a/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp b/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp index 39b65d0947..3258dda409 100644 --- a/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp +++ b/src/libxrpl/tx/transactors/permissioned_domain/PermissionedDomainSet.cpp @@ -110,7 +110,7 @@ PermissionedDomainSet::doApply() if (balance < reserve) return tecINSUFFICIENT_RESERVE; - bool const fix313 = view().rules().enabled(fixSecurity3_1_3); + bool const fix313 = view().rules().enabled(fixCleanup3_1_3); auto const seq = fix313 ? ctx_.tx.getSeqValue() : ctx_.tx.getFieldU32(sfSequence); Keylet const pdKeylet = keylet::permissionedDomain(account_, seq); auto slePd = std::make_shared(pdKeylet); diff --git a/src/libxrpl/tx/transactors/vault/VaultClawback.cpp b/src/libxrpl/tx/transactors/vault/VaultClawback.cpp index b6d92a6083..33f4dbf4fa 100644 --- a/src/libxrpl/tx/transactors/vault/VaultClawback.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultClawback.cpp @@ -243,11 +243,11 @@ VaultClawback::assetsToClawback( auto const mptIssuanceID = *vault->at(sfShareMPTID); MPTIssue const share{mptIssuanceID}; - // Pre-fixSecurity3_1_3: zero-amount clawback returned early without + // Pre-fixCleanup3_1_3: zero-amount clawback returned early without // clamping to assetsAvailable, allowing more assets to be recovered // than available when there was an outstanding loan. Retained for // ledger replay compatibility. - if (!ctx_.view().rules().enabled(fixSecurity3_1_3) && clawbackAmount == beast::kZERO) + if (!ctx_.view().rules().enabled(fixCleanup3_1_3) && clawbackAmount == beast::kZERO) { auto const sharesDestroyed = accountHolds( view(), holder, share, FreezeHandling::IgnoreFreeze, AuthHandling::IgnoreAuth, j_); diff --git a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp index 94c6f0f6d2..5b4949ccf6 100644 --- a/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp +++ b/src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp @@ -80,9 +80,9 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) // LCOV_EXCL_STOP } - if (ctx.view.rules().enabled(fixSecurity3_1_3) && amount.asset() == vaultShare) + if (ctx.view.rules().enabled(fixCleanup3_1_3) && amount.asset() == vaultShare) { - // Post-fixSecurity3_1_3: if the user specified shares, convert + // Post-fixCleanup3_1_3: if the user specified shares, convert // to the equivalent asset amount before checking withdrawal // limits. Pre-amendment the limit check was skipped for // share-denominated withdrawals. diff --git a/src/test/app/Credentials_test.cpp b/src/test/app/Credentials_test.cpp index fced18c387..1d3f3b3ea8 100644 --- a/src/test/app/Credentials_test.cpp +++ b/src/test/app/Credentials_test.cpp @@ -1036,7 +1036,7 @@ struct Credentials_test : public beast::unit_test::Suite void testRemoveExpiredCorruption(FeatureBitset features) { - bool const fixEnabled = features[fixSecurity3_1_3]; + bool const fixEnabled = features[fixCleanup3_1_3]; testcase( "removeExpired ignores deleteSLE failure " + (fixEnabled ? std::string(" after fix") : std::string(" before fix"))); @@ -1164,8 +1164,8 @@ struct Credentials_test : public beast::unit_test::Suite testFlags(all); testRPC(); - testRemoveExpiredCorruption(all - fixSecurity3_1_3); - testRemoveExpiredCorruption(all | fixSecurity3_1_3); + testRemoveExpiredCorruption(all - fixCleanup3_1_3); + testRemoveExpiredCorruption(all | fixCleanup3_1_3); } }; diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index c9f0a04a18..b0f1aee03c 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -94,7 +94,7 @@ class Invariants_test : public beast::unit_test::Suite static FeatureBitset defaultAmendments() { - return xrpl::test::jtx::testableAmendments() | fixSecurity3_1_3 | fixCleanup3_2_0; + return xrpl::test::jtx::testableAmendments() | fixCleanup3_1_3 | fixCleanup3_2_0; } /** Run a specific test case to put the ledger into a state that will be @@ -1801,7 +1801,7 @@ class Invariants_test : public beast::unit_test::Suite using namespace test::jtx; bool const fixPDEnabled = features[fixPermissionedDomainInvariant]; - bool const fixS313Enabled = features[fixSecurity3_1_3]; + bool const fixS313Enabled = features[fixCleanup3_1_3]; testcase << "PermissionedDEX" + std::string(fixPDEnabled ? " fixPD" : "") + std::string(fixS313Enabled ? " fixS313" : ""); @@ -2387,7 +2387,7 @@ class Invariants_test : public beast::unit_test::Suite } // Test: cover available greater than pseudo-account asset balance - // (requires fixSecurity3_1_3) + // (requires fixCleanup3_1_3) doInvariantCheck( {{"Loan Broker cover available is greater than pseudo-account asset balance"}}, [&](Account const&, Account const&, ApplyContext& ac) { @@ -4116,7 +4116,7 @@ class Invariants_test : public beast::unit_test::Suite testInvariantOverwrite(FeatureBitset features) { using namespace test::jtx; - bool const fixEnabled = features[fixSecurity3_1_3]; + bool const fixEnabled = features[fixCleanup3_1_3]; std::initializer_list const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}; std::initializer_list const passTers = {tesSUCCESS, tesSUCCESS}; @@ -4385,16 +4385,15 @@ public: testPermissionedDEX(defaultAmendments() | fixPermissionedDomainInvariant); testPermissionedDEX(defaultAmendments() - fixPermissionedDomainInvariant); testPermissionedDEX( - (defaultAmendments() | fixPermissionedDomainInvariant) - fixSecurity3_1_3); - testPermissionedDEX( - defaultAmendments() - fixPermissionedDomainInvariant - fixSecurity3_1_3); + (defaultAmendments() | fixPermissionedDomainInvariant) - fixCleanup3_1_3); + testPermissionedDEX(defaultAmendments() - fixPermissionedDomainInvariant - fixCleanup3_1_3); testNoModifiedUnmodifiableFields(); testValidPseudoAccounts(); testValidLoanBroker(); testVault(); testMPT(); testInvariantOverwrite(defaultAmendments()); - testInvariantOverwrite(defaultAmendments() - fixSecurity3_1_3); + testInvariantOverwrite(defaultAmendments() - fixCleanup3_1_3); testVaultComputeCoarsestScale(); } }; diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index 70997b1dcd..ede616bc7c 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -2146,7 +2146,7 @@ protected: Ter(tecNO_PERMISSION)); { - env.disableFeature(fixSecurity3_1_3); + env.disableFeature(fixCleanup3_1_3); env(pay(borrower, loanKeylet.key, STAmount{broker.asset, state.periodicPayment * Number{15, -1}}, @@ -2154,7 +2154,7 @@ protected: Fee(XRPAmount{ baseFee * (Number{15, -1} / kLOAN_PAYMENTS_PER_FEE_INCREMENT + 1)}), Ter(temINVALID_FLAG)); - env.enableFeature(fixSecurity3_1_3); + env.enableFeature(fixCleanup3_1_3); } } // Try to send a payment marked as multiple mutually exclusive @@ -4763,7 +4763,7 @@ protected: void testDosLoanPay(FeatureBitset features) { - bool const feeCapped = features[fixSecurity3_1_3]; + bool const feeCapped = features[fixCleanup3_1_3]; // From FIND-005 testcase << "DoS LoanPay: fee calculation " << (feeCapped ? "capped" : "uncapped"); @@ -4780,7 +4780,7 @@ protected: env.fund(XRP(1'000'000), issuer, lender, borrower); env.close(); - BEAST_EXPECT(feeCapped == env.current()->rules().enabled(fixSecurity3_1_3)); + BEAST_EXPECT(feeCapped == env.current()->rules().enabled(fixCleanup3_1_3)); PrettyAsset const iouAsset = issuer[iouCurrency_]; env(trust(lender, iouAsset(100'000'000))); @@ -7664,8 +7664,8 @@ public: testLoanPayDebtDecreaseInvariant(); testWrongMaxDebtBehavior(); testLoanPayComputePeriodicPaymentValidTotalInterestInvariant(); - testDosLoanPay(all | fixSecurity3_1_3); - testDosLoanPay(all - fixSecurity3_1_3); + testDosLoanPay(all | fixCleanup3_1_3); + testDosLoanPay(all - fixCleanup3_1_3); testLoanPayComputePeriodicPaymentValidTotalPrincipalPaidInvariant(); testLoanPayComputePeriodicPaymentValidTotalInterestPaidInvariant(); testLoanNextPaymentDueDateOverflow(); diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 126778c6a0..36895dd1cf 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -3310,7 +3310,7 @@ class MPToken_test : public beast::unit_test::Suite testMultiSendMaximumAmount(FeatureBitset features) { // Verify that directSendNoLimitMultiMPT correctly enforces MaximumAmount - // when the issuer sends to multiple receivers. Pre-fixSecurity3_1_3, + // when the issuer sends to multiple receivers. Pre-fixCleanup3_1_3, // a stale view.read() snapshot caused per-iteration checks to miss // aggregate overflows. Post-fix, a running total is used instead. testcase("Multi-send MaximumAmount enforcement"); @@ -3412,14 +3412,14 @@ class MPToken_test : public beast::unit_test::Suite // individual send (100 <= 150) even though the aggregate (200) // exceeds MaximumAmount. Preserved for ledger replay. { - // KNOWN BUG (pre-fixSecurity3_1_3): preserved for ledger replay only - env.disableFeature(fixSecurity3_1_3); + // KNOWN BUG (pre-fixCleanup3_1_3): preserved for ledger replay only + env.disableFeature(fixCleanup3_1_3); runTest( R{{alice.id(), 100}, {bob.id(), 100}}, tesSUCCESS, 250, "pre-amendment allows over-send"); - env.enableFeature(fixSecurity3_1_3); + env.enableFeature(fixCleanup3_1_3); } } diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index b81976b83a..5be33fc4e5 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -1095,10 +1095,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite BEAST_EXPECT(ownerCount(env, buyer) == buyerCount); // The buy offer must not have expired. - // NOTE: this is only a preclaim check with the fixSecurity3_1_3 amendment disabled. + // NOTE: this is only a preclaim check with the fixCleanup3_1_3 amendment disabled. env(token::acceptBuyOffer(alice, buyerExpOfferIndex), Ter(tecEXPIRED)); env.close(); - if (features[fixSecurity3_1_3]) + if (features[fixCleanup3_1_3]) { buyerCount--; } @@ -1115,13 +1115,13 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite BEAST_EXPECT(ownerCount(env, buyer) == buyerCount); // The sell offer must not have expired. - // NOTE: this is only a preclaim check with the fixSecurity3_1_3 amendment disabled. + // NOTE: this is only a preclaim check with the fixCleanup3_1_3 amendment disabled. env(token::acceptSellOffer(buyer, aliceExpOfferIndex), Ter(tecEXPIRED)); env.close(); // Alice's count is decremented by one when the expired offer is // removed. - if (features[fixSecurity3_1_3]) + if (features[fixCleanup3_1_3]) { aliceCount--; } @@ -3101,10 +3101,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite // No one can accept an expired sell offer. env(token::acceptSellOffer(buyer, offer1), Ter(tecEXPIRED)); - // With fixSecurity3_1_3 amendment, the first accept + // With fixCleanup3_1_3 amendment, the first accept // attempt deletes the expired offer. Without the amendment, // the offer remains and we can try to accept it again. - if (features[fixSecurity3_1_3]) + if (features[fixCleanup3_1_3]) { // After amendment: offer was deleted by first accept attempt minterCount--; @@ -3123,7 +3123,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite BEAST_EXPECT(ownerCount(env, minter) == minterCount); BEAST_EXPECT(ownerCount(env, buyer) == buyerCount); - if (!features[fixSecurity3_1_3]) + if (!features[fixCleanup3_1_3]) { // Before amendment: expired offer still exists and needs to be // cancelled @@ -3189,10 +3189,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite // An expired buy offer cannot be accepted. env(token::acceptBuyOffer(minter, offer1), Ter(tecEXPIRED)); - // With fixSecurity3_1_3 amendment, the first accept + // With fixCleanup3_1_3 amendment, the first accept // attempt deletes the expired offer. Without the amendment, // the offer remains and we can try to accept it again. - if (features[fixSecurity3_1_3]) + if (features[fixCleanup3_1_3]) { // After amendment: offer was deleted by first accept attempt buyerCount--; @@ -3211,7 +3211,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite BEAST_EXPECT(ownerCount(env, minter) == minterCount); BEAST_EXPECT(ownerCount(env, buyer) == buyerCount); - if (!features[fixSecurity3_1_3]) + if (!features[fixCleanup3_1_3]) { // Before amendment: expired offer still exists and can be // cancelled @@ -3288,7 +3288,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite env(token::brokerOffers(issuer, buyOffer1, sellOffer1), Ter(tecEXPIRED)); env.close(); - if (features[fixSecurity3_1_3]) + if (features[fixCleanup3_1_3]) { // With amendment: expired offers are deleted minterCount--; @@ -3298,7 +3298,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite BEAST_EXPECT(ownerCount(env, minter) == minterCount); BEAST_EXPECT(ownerCount(env, buyer) == buyerCount); - if (features[fixSecurity3_1_3]) + if (features[fixCleanup3_1_3]) { // The buy offer was deleted, so no need to cancel it // The sell offer still exists, so we can cancel it @@ -3377,7 +3377,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite env.close(); BEAST_EXPECT(ownerCount(env, issuer) == 0); - if (features[fixSecurity3_1_3]) + if (features[fixCleanup3_1_3]) { // After amendment: expired offers were deleted during broker // attempt @@ -3463,7 +3463,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite // The expired offers are still in the ledger. BEAST_EXPECT(ownerCount(env, issuer) == 0); - if (!features[fixSecurity3_1_3]) + if (!features[fixCleanup3_1_3]) { // Before amendment: expired offers still exist in ledger BEAST_EXPECT(ownerCount(env, minter) == 2); @@ -7193,7 +7193,7 @@ public: { testWithFeats( allFeatures_ - fixNFTokenReserve - featureNFTokenMintOffer - featureDynamicNFT - - fixSecurity3_1_3); + fixCleanup3_1_3); } }; @@ -7230,7 +7230,7 @@ class NfTokenWoExpiredOfferRemovalTest : public NFTokenBaseUtil_test void run() override { - testWithFeats(allFeatures_ - fixSecurity3_1_3); + testWithFeats(allFeatures_ - fixCleanup3_1_3); } }; diff --git a/src/test/app/PermissionedDEX_test.cpp b/src/test/app/PermissionedDEX_test.cpp index a0b16aa557..21e6bd8baa 100644 --- a/src/test/app/PermissionedDEX_test.cpp +++ b/src/test/app/PermissionedDEX_test.cpp @@ -1392,13 +1392,12 @@ class PermissionedDEX_test : public beast::unit_test::Suite void testHybridMalformedOffer(FeatureBitset features) { - bool const fixS313Enabled = features[fixSecurity3_1_3]; + bool const fixS313Enabled = features[fixCleanup3_1_3]; testcase << "Hybrid offer with empty AdditionalBooks" - << (fixS313Enabled ? " (fixSecurity3_1_3 enabled)" - : " (fixSecurity3_1_3 disabled)"); + << (fixS313Enabled ? " (fixCleanup3_1_3 enabled)" : " (fixCleanup3_1_3 disabled)"); - // offerInDomain has two code paths gated by fixSecurity3_1_3: + // offerInDomain has two code paths gated by fixCleanup3_1_3: // // pre-fix: only rejects a hybrid offer when sfAdditionalBooks is // entirely absent — an empty array (size 0) passes through. @@ -1425,7 +1424,7 @@ class PermissionedDEX_test : public beast::unit_test::Suite // Directly manipulate the offer SLE in the open ledger so that // sfAdditionalBooks is present but empty (size 0). This is the - // malformed state that fixSecurity3_1_3 is designed to catch. + // malformed state that fixCleanup3_1_3 is designed to catch. auto const offerKey = keylet::offer(bob.id(), bobOfferSeq); env.app().getOpenLedger().modify([&offerKey](OpenView& view, beast::Journal) { auto const sle = view.read(offerKey); @@ -1439,7 +1438,7 @@ class PermissionedDEX_test : public beast::unit_test::Suite if (fixS313Enabled) { - // post-fixSecurity3_1_3: offerInDomain rejects the malformed + // post-fixCleanup3_1_3: offerInDomain rejects the malformed // offer (size == 0), so no valid domain offer is found. env(pay(alice, carol, USD(10)), Path(~USD), @@ -1449,7 +1448,7 @@ class PermissionedDEX_test : public beast::unit_test::Suite } else { - // pre-fixSecurity3_1_3: offerInDomain only checks for a missing + // pre-fixCleanup3_1_3: offerInDomain only checks for a missing // sfAdditionalBooks field; size == 0 passes through, so the // malformed offer is crossed and the payment succeeds. env(pay(alice, carol, USD(10)), Path(~USD), Sendmax(XRP(10)), Domain(domainID)); @@ -1478,7 +1477,7 @@ public: testHybridInvalidOffer(all); testHybridOfferDirectories(all); testHybridMalformedOffer(all); - testHybridMalformedOffer(all - fixSecurity3_1_3); + testHybridMalformedOffer(all - fixCleanup3_1_3); } }; diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index cf79fb1d18..c950739078 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -1023,16 +1023,21 @@ public: checkMetrics(*this, env, 0, std::nullopt, 0, 3); // ledgers in queue is 2 because of makeConfig - auto const initQueueMax = initFee(env, 3, 2, 10, 200, 50); + initFee(env, 3, 2, 10, 200, 50); + // Close an empty ledger to shrink queue from the flag-ledger + // size to 2*3=6, independent of amendment count. + env.close(); + constexpr std::size_t kINIT_QUEUE_MAX = 6; + checkMetrics(*this, env, 0, kINIT_QUEUE_MAX, 0, 3); // Create several accounts while the fee is cheap so they all apply. env.fund(drops(2000), noripple(alice)); env.fund(XRP(500000), noripple(bob, charlie, daria)); - checkMetrics(*this, env, 0, initQueueMax, 4, 3); + checkMetrics(*this, env, 0, kINIT_QUEUE_MAX, 4, 3); // Alice - price starts exploding: held env(noop(alice), Fee(11), queued); - checkMetrics(*this, env, 1, initQueueMax, 4, 3); + checkMetrics(*this, env, 1, kINIT_QUEUE_MAX, 4, 3); auto aliceSeq = env.seq(alice); auto bobSeq = env.seq(bob); @@ -1040,28 +1045,28 @@ public: // Alice - try to queue a second transaction, but leave a gap env(noop(alice), Seq(aliceSeq + 2), Fee(100), Ter(telCAN_NOT_QUEUE)); - checkMetrics(*this, env, 1, initQueueMax, 4, 3); + checkMetrics(*this, env, 1, kINIT_QUEUE_MAX, 4, 3); // Alice - queue a second transaction. Yay! env(noop(alice), Seq(aliceSeq + 1), Fee(13), queued); - checkMetrics(*this, env, 2, initQueueMax, 4, 3); + checkMetrics(*this, env, 2, kINIT_QUEUE_MAX, 4, 3); // Alice - queue a third transaction. Yay. env(noop(alice), Seq(aliceSeq + 2), Fee(17), queued); - checkMetrics(*this, env, 3, initQueueMax, 4, 3); + checkMetrics(*this, env, 3, kINIT_QUEUE_MAX, 4, 3); // Bob - queue a transaction env(noop(bob), queued); - checkMetrics(*this, env, 4, initQueueMax, 4, 3); + checkMetrics(*this, env, 4, kINIT_QUEUE_MAX, 4, 3); // Bob - queue a second transaction env(noop(bob), Seq(bobSeq + 1), Fee(50), queued); - checkMetrics(*this, env, 5, initQueueMax, 4, 3); + checkMetrics(*this, env, 5, kINIT_QUEUE_MAX, 4, 3); // Charlie - queue a transaction, with a higher fee // than default env(noop(charlie), Fee(15), queued); - checkMetrics(*this, env, 6, initQueueMax, 4, 3, 257); + checkMetrics(*this, env, 6, kINIT_QUEUE_MAX, 4, 3, 257); BEAST_EXPECT(env.seq(alice) == aliceSeq); BEAST_EXPECT(env.seq(bob) == bobSeq); diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index f2c42b7bae..2f32cf605a 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -4916,7 +4916,7 @@ class Vault_test : public beast::unit_test::Suite using namespace loanBroker; using namespace loan; Env env(*this); - env.enableFeature(fixSecurity3_1_3); + env.enableFeature(fixCleanup3_1_3); auto const setupVault = [&](PrettyAsset const& asset, Account const& owner, @@ -5403,14 +5403,14 @@ class Vault_test : public beast::unit_test::Suite env.close(); testCase(mpt, "MPT", owner, depositor, issuer); - // Test pre-fixSecurity3_1_3 legacy path: zero-amount clawback + // Test pre-fixCleanup3_1_3 legacy path: zero-amount clawback // returns early without clamping to assetsAvailable. { testcase( - "VaultClawback (asset) - IOU pre-fixSecurity3_1_3" + "VaultClawback (asset) - IOU pre-fixCleanup3_1_3" " zero-amount clawback unclamped with outstanding loan"); - env.disableFeature(fixSecurity3_1_3); + env.disableFeature(fixCleanup3_1_3); auto [vault, vaultKeylet] = setupVault(iou, owner, depositor, issuer); @@ -5468,7 +5468,7 @@ class Vault_test : public beast::unit_test::Suite BEAST_EXPECT(sharesAfter == sharesBefore); } - env.enableFeature(fixSecurity3_1_3); + env.enableFeature(fixCleanup3_1_3); } } @@ -5867,7 +5867,7 @@ class Vault_test : public beast::unit_test::Suite { testcase("Vault clawback only recovers unlocked shares"); - Env env{*this, testableAmendments() | fixSecurity3_1_3}; + Env env{*this, testableAmendments() | fixCleanup3_1_3}; auto const baseFee = env.current()->fees().base; Account const owner{"owner"}; Account const depositor{"depositor"}; @@ -5957,9 +5957,9 @@ class Vault_test : public beast::unit_test::Suite auto const allAmendments = testableAmendments() | featureSingleAssetVault; - for (auto const& features : {allAmendments, allAmendments - fixSecurity3_1_3}) + for (auto const& features : {allAmendments, allAmendments - fixCleanup3_1_3}) { - bool const withFix = features[fixSecurity3_1_3]; + bool const withFix = features[fixCleanup3_1_3]; Env env{*this, features}; Account const owner{"owner"}; @@ -6117,7 +6117,7 @@ class Vault_test : public beast::unit_test::Suite env.close(); auto const sleMptAfter = env.le(keylet::mptoken(shareMptID, depositor)); - if (!f[fixSecurity3_1_3]) + if (!f[fixCleanup3_1_3]) { // Without the fix, removeEmptyHolding deletes the MPToken // even though sfLockedAmount > 0, leaving the escrow's locked @@ -6137,7 +6137,7 @@ class Vault_test : public beast::unit_test::Suite } }; - runTest(amendments - fixSecurity3_1_3); + runTest(amendments - fixCleanup3_1_3); runTest(amendments); }