From 7e7dc991e36b4fb291b86b8731b1dccc1f953f96 Mon Sep 17 00:00:00 2001 From: Peter Chen <34582813+PeterChen13579@users.noreply.github.com> Date: Thu, 25 Jun 2026 16:14:39 -0700 Subject: [PATCH] fix review comments (#7636) --- .../xrpl/ledger/helpers/CredentialHelpers.h | 118 ++++-------------- include/xrpl/protocol/LedgerFormats.h | 4 +- 2 files changed, 24 insertions(+), 98 deletions(-) diff --git a/include/xrpl/ledger/helpers/CredentialHelpers.h b/include/xrpl/ledger/helpers/CredentialHelpers.h index bd6d538837..d6b797ce34 100644 --- a/include/xrpl/ledger/helpers/CredentialHelpers.h +++ b/include/xrpl/ledger/helpers/CredentialHelpers.h @@ -13,116 +13,53 @@ namespace xrpl { namespace credentials { -/** - * @brief Check whether a credential has expired. - * - * @param sleCredential Credential ledger entry to inspect. - * @param closed Parent close time to compare against sfExpiration. - * @return true if sfExpiration is before closed; otherwise false. - */ +// These function will be used by the code that use DepositPreauth / Credentials +// (and any future pre-authorization modes) as part of authorization (all the +// transfer funds transactions) + +// Check if credential sfExpiration field has passed ledger's parentCloseTime bool checkExpired(SLE const& sleCredential, NetClock::time_point const& closed); -/** - * @brief Remove a Credential ledger object from the view. - * - * @param view Mutable ledger view. - * @param sleCredential Credential ledger entry to delete. - * @param j Journal for diagnostics. - * @return tesSUCCESS on success, or an error code if ledger cleanup fails. - */ +// Actually remove a credentials object from the ledger [[nodiscard]] TER deleteSLE(ApplyView& view, SLE::ref sleCredential, beast::Journal j); -/** - * @brief Check amendment and parameter rules for sfCredentialIDs. - * - * @param tx Transaction to validate. - * @param j Journal for diagnostics. - * @return tesSUCCESS if sfCredentialIDs is absent or well-formed; otherwise a - * malformed transaction code. - */ +// Amendment and parameters checks for sfCredentialIDs field NotTEC checkFields(STTx const& tx, beast::Journal j); -/** - * @brief Check whether the transaction's provided credentials are valid. - * - * Use this only from preclaim because it does not remove expired credentials. - * If this is called from preclaim, verifyDepositPreauth() must also be called - * from doApply(). - * - * @param tx Transaction containing optional sfCredentialIDs. - * @param view Read-only ledger view. - * @param src Source account that must own the credentials. - * @param j Journal for diagnostics. - * @return tesSUCCESS if credentials are valid, otherwise a credential error. - */ +// Accessing the ledger to check if provided credentials are valid. Do not use +// in doApply (only in preclaim) since it does not remove expired credentials. +// If you call it in preclaim, you also must call verifyDepositPreauth in +// doApply TER valid(STTx const& tx, ReadView const& view, AccountID const& src, beast::Journal j); -/** - * @brief Check whether an account has an accepted credential for a domain. - * - * If this is called from preclaim and returns tecEXPIRED, verifyValidDomain() - * should be called from doApply() so expired credentials are deleted. - * - * @param view Read-only ledger view. - * @param domainID PermissionedDomain ledger entry ID. - * @param subject Account whose credentials are checked. - * @return tesSUCCESS if a matching accepted credential exists, tecEXPIRED if - * only expired credentials match, or an authorization/object error. - */ +// Check if subject has any credential matching the given domain. If you call it +// in preclaim and it returns tecEXPIRED, you should call verifyValidDomain in +// doApply. This will ensure that expired credentials are deleted. TER validDomain(ReadView const& view, uint256 domainID, AccountID const& subject); -/** - * @brief Check DepositPreauth authorization using credential IDs. - * - * @param view Read-only ledger view. - * @param ctx Credential IDs supplied by the transaction. - * @param dst Destination account requiring deposit authorization. - * @return tesSUCCESS if credentials match a DepositPreauth object, otherwise - * an authorization or internal error. - */ +// This function is only called when we are about to return tecNO_PERMISSION +// because all the checks for the DepositPreauth authorization failed. TER authorizedDepositPreauth(ReadView const& view, STVector256 const& ctx, AccountID const& dst); -/** - * @brief Sort credential issuer/type pairs. - * - * @param credentials Credential descriptors from a transaction. - * @return Sorted issuer/type pairs, or an empty set if duplicates exist. - */ +// Sort credentials array, return empty set if there are duplicates std::set> makeSorted(STArray const& credentials); -/** - * @brief Check a credentials array for DepositPreauth or PermissionedDomainSet. - * - * @param credentials Credential descriptors to validate. - * @param maxSize Maximum allowed array size. - * @param j Journal for diagnostics. - * @return tesSUCCESS if the array is well-formed, otherwise a malformed - * transaction code. - */ +// Check credentials array passed to DepositPreauth/PermissionedDomainSet +// transactions NotTEC checkArray(STArray const& credentials, unsigned maxSize, beast::Journal j); } // namespace credentials -/** - * @brief Verify an account has a credential for a domain and delete expired - * credentials encountered while checking. - * - * @param view Mutable ledger view. - * @param account Account whose credentials are checked. - * @param domainID PermissionedDomain ledger entry ID. - * @param j Journal for diagnostics. - * @return tesSUCCESS if a matching accepted credential exists, tecEXPIRED if - * expired credentials were removed but no valid credential remains, or - * an authorization/object error. - */ +// Check expired credentials and for credentials matching DomainID of the ledger +// object TER verifyValidDomain(ApplyView& view, AccountID const& account, uint256 domainID, beast::Journal j); @@ -159,18 +96,7 @@ checkDepositPreauth( TER cleanupExpiredCredentials(STTx const& tx, ApplyView& view, beast::Journal j); -/** - * @brief Remove expired credentials, then check DepositPreauth authorization. - * - * @param tx Transaction containing optional credential IDs. - * @param view Mutable ledger view. - * @param src Source account. - * @param dst Destination account. - * @param sleDst Destination AccountRoot, if it exists. - * @param j Journal for diagnostics. - * @return tesSUCCESS if the deposit is authorized, otherwise an expiration, - * authorization, or deletion error. - */ +// Check expired credentials and for existing DepositPreauth ledger object TER verifyDepositPreauth( STTx const& tx, diff --git a/include/xrpl/protocol/LedgerFormats.h b/include/xrpl/protocol/LedgerFormats.h index e4f244e1c5..70afd12f34 100644 --- a/include/xrpl/protocol/LedgerFormats.h +++ b/include/xrpl/protocol/LedgerFormats.h @@ -187,9 +187,9 @@ enum LedgerEntryType : std::uint16_t { LSF_FLAG(lsmfMPTCanEnableCanTrade, 0x00000010) \ LSF_FLAG(lsmfMPTCanEnableCanTransfer, 0x00000020) \ LSF_FLAG(lsmfMPTCanEnableCanClawback, 0x00000040) \ + LSF_FLAG(lsmfMPTCannotEnableCanHoldConfidentialBalance, 0x00000080) \ LSF_FLAG(lsmfMPTCanMutateMetadata, 0x00010000) \ - LSF_FLAG(lsmfMPTCanMutateTransferFee, 0x00020000) \ - LSF_FLAG(lsmfMPTCannotEnableCanHoldConfidentialBalance, 0x00040000)) \ + LSF_FLAG(lsmfMPTCanMutateTransferFee, 0x00020000)) \ \ LEDGER_OBJECT(MPToken, \ LSF_FLAG2(lsfMPTLocked, 0x00000001) \