From 010767184613e804b66ec0247ccb22f4fcd8e27e Mon Sep 17 00:00:00 2001 From: Timur Ialymov Date: Tue, 18 Aug 2026 18:51:03 +0100 Subject: [PATCH] fix: Clarify the cleanup bound and narrow the invariant relaxation The bound passed to the pseudo-account credential cleanup limits how many owner-directory entries the walk visits, not how many credentials it deletes: entries the walk leaves alone are charged against the same budget. Both comments claimed the latter, so say what the code actually does. Narrow the tecINCOMPLETE relaxation in the MPT issuance invariant to VaultDelete. That is the only transaction carrying the DestroyMptIssuance privilege that can return tecINCOMPLETE, and the comment above the branch already said as much, so the code now matches it and any future transaction with the same privilege stays under the strict rule. Rename removeExpiredCredentials to removeDeletedCredentials. It now also deletes credentials removed by a bounded cleanup rather than by expiry, which is the same reason removeDeletedTrustLines carries that name. --- include/xrpl/ledger/helpers/CredentialHelpers.h | 9 ++++++--- include/xrpl/protocol/Protocol.h | 8 ++++++-- src/libxrpl/tx/Transactor.cpp | 6 +++--- src/libxrpl/tx/invariants/MPTInvariant.cpp | 3 ++- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/include/xrpl/ledger/helpers/CredentialHelpers.h b/include/xrpl/ledger/helpers/CredentialHelpers.h index 76c2786cde..6d235b4316 100644 --- a/include/xrpl/ledger/helpers/CredentialHelpers.h +++ b/include/xrpl/ledger/helpers/CredentialHelpers.h @@ -39,9 +39,12 @@ deleteSLE(ApplyView& view, SLE::ref sleCredential, beast::Journal j); * * Cleans up credentials that were linked to a pseudo-account (Vault, LoanBroker, * AMM), which such an account can neither accept nor delete. Only credentials - * are removed, at most @p maxNodesToDelete of them; on reaching that bound the - * result is `tecINCOMPLETE` and the caller must propagate it so a later - * transaction resumes. + * are removed; every other object is left in place. The walk visits at most + * @p maxNodesToDelete directory entries and charges the ones it leaves alone + * against that budget too, so a directory holding other objects yields fewer + * than @p maxNodesToDelete deletions. On reaching the bound the result is + * `tecINCOMPLETE` and the caller must propagate it so a later transaction + * resumes. * * @param view Mutable ledger view. * @param pseudoAcct The pseudo-account whose directory is cleaned. diff --git a/include/xrpl/protocol/Protocol.h b/include/xrpl/protocol/Protocol.h index 46f74b905a..e6768efd76 100644 --- a/include/xrpl/protocol/Protocol.h +++ b/include/xrpl/protocol/Protocol.h @@ -397,8 +397,12 @@ using TxID = uint256; constexpr std::uint16_t kMaxDeletableAmmTrustLines = 512; /** - * The maximum number of credentials to delete from a pseudo-account's owner - * directory in a single transaction. + * The maximum number of owner-directory entries to walk when clearing + * credentials pinned to a pseudo-account, in a single transaction. + * + * The walk stops after this many entries whether or not each one turns out to + * be a credential, so a directory that also holds other objects yields fewer + * deletions per transaction. */ constexpr std::uint16_t kMaxDeletablePseudoAccountCredentials = 512; diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index b52288fa74..304f4d0f43 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -1246,7 +1246,7 @@ removeExpiredNFTokenOffers( } static void -removeExpiredCredentials(ApplyView& view, std::vector const& creds, beast::Journal viewJ) +removeDeletedCredentials(ApplyView& view, std::vector const& creds, beast::Journal viewJ) { for (auto const& index : creds) { @@ -1255,7 +1255,7 @@ removeExpiredCredentials(ApplyView& view, std::vector const& creds, bea if (auto const ter = credentials::deleteSLE(view, sle, viewJ); !isTesSuccess(ter)) { JLOG(viewJ.error()) - << "removeExpiredCredentials: failed to delete expired credential. Err: " + << "removeDeletedCredentials: failed to delete credential. Err: " << transToken(ter); } } @@ -1529,7 +1529,7 @@ Transactor::processPersistentChanges(TER result, XRPAmount fee) removeDeletedTrustLines(view(), ids, viewJ); break; case ltCREDENTIAL: - removeExpiredCredentials(view(), ids, viewJ); + removeDeletedCredentials(view(), ids, viewJ); break; // LCOV_EXCL_START default: diff --git a/src/libxrpl/tx/invariants/MPTInvariant.cpp b/src/libxrpl/tx/invariants/MPTInvariant.cpp index 2892b80aa2..3be71428e4 100644 --- a/src/libxrpl/tx/invariants/MPTInvariant.cpp +++ b/src/libxrpl/tx/invariants/MPTInvariant.cpp @@ -235,7 +235,8 @@ ValidMPTIssuance::finalize( // pseudo-account returns tecINCOMPLETE and has not yet reached the // share issuance. Don't require the issuance to be removed until // the deletion completes (a later transaction). - if (rules.enabled(fixCleanup3_4_0) && result == tecINCOMPLETE) + if (rules.enabled(fixCleanup3_4_0) && txnType == ttVAULT_DELETE && + result == tecINCOMPLETE) return mptIssuancesDeleted_ == 0 && mptIssuancesCreated_ == 0; if (mptIssuancesDeleted_ == 0)