fix review comments (#7636)

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<std::pair<AccountID, Slice>>
 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,

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)                                                                                        \