Minor cleanup

src/xrpld/app/tx/detail/VaultCreate.cpp

@@ -97,12 +97,12 @@ VaultCreate::calculateBaseFee(ReadView const& view, STTx const& tx)
 TER
 VaultCreate::preclaim(PreclaimContext const& ctx)
 {
-    auto asset = ctx.tx[sfAsset];
+    auto vaultAsset = ctx.tx[sfAsset];
     auto account = ctx.tx[sfAccount];
 
-    if (asset.holds<MPTIssue>())
+    if (vaultAsset.holds<MPTIssue>())
     {
-        auto mptID = asset.get<MPTIssue>().getMptID();
+        auto mptID = vaultAsset.get<MPTIssue>().getMptID();
         auto issuance = ctx.view.read(keylet::mptIssuance(mptID));
         if (!issuance)
             return tecOBJECT_NOT_FOUND;
@@ -113,14 +113,14 @@ VaultCreate::preclaim(PreclaimContext const& ctx)
     // Check for pseudo-account issuers - we do not want a vault to hold such
     // assets (e.g. MPT shares to other vaults or AMM LPTokens) as they would be
     // impossible to clawback (should the need arise)
-    if (!asset.native())
+    if (!vaultAsset.native())
     {
-        if (isPseudoAccount(ctx.view, asset.getIssuer()))
+        if (isPseudoAccount(ctx.view, vaultAsset.getIssuer()))
             return tecWRONG_ASSET;
     }
 
     // Cannot create Vault for an Asset frozen for the vault owner
-    if (isFrozen(ctx.view, account, asset))
+    if (isFrozen(ctx.view, account, vaultAsset))
         return tecFROZEN;
 
     if (auto const domain = ctx.tx[~sfDomainID])

src/xrpld/app/tx/detail/VaultDeposit.cpp

@@ -67,22 +67,22 @@ VaultDeposit::preclaim(PreclaimContext const& ctx)
         return tecWRONG_ASSET;
 
     auto const mptIssuanceID = vault->at(sfShareMPTID);
-    auto const share = MPTIssue(mptIssuanceID);
+    auto const vaultShare = MPTIssue(mptIssuanceID);
-    if (share == assets.asset())
+    if (vaultShare == assets.asset())
-        return tefINTERNAL;
+        return tefINTERNAL;  // LCOV_EXCL_LINE
 
     auto const sleIssuance = ctx.view.read(keylet::mptIssuance(mptIssuanceID));
     if (!sleIssuance)
-        return tefINTERNAL;
+        return tefINTERNAL;  // LCOV_EXCL_LINE
     if (sleIssuance->isFlag(lsfMPTLocked))
-        return tefINTERNAL;
+        return tefINTERNAL;  // LCOV_EXCL_LINE
 
     // Cannot deposit inside Vault an Asset frozen for the depositor
     if (isFrozen(ctx.view, account, vaultAsset))
         return vaultAsset.holds<Issue>() ? tecFROZEN : tecLOCKED;
 
     // Cannot deposit if the shares of the vault are frozen
-    if (isFrozen(ctx.view, account, share))
+    if (isFrozen(ctx.view, account, vaultShare))
         return tecLOCKED;
 
     if (vault->isFlag(tfVaultPrivate) && account != vault->at(sfOwner))
@@ -106,24 +106,11 @@ VaultDeposit::preclaim(PreclaimContext const& ctx)
             return tecNO_AUTH;
     }
 
-    if (auto const ter = std::visit(
+    // Source MPToken must exist (if asset is an MPT)
-            [&]<ValidIssueType TIss>(TIss const& issue) -> TER {
+    if (auto const ter = requireAuth(ctx.view, vaultAsset, account);
-                return requireAuth(ctx.view, issue, account);
-            },
-            vaultAsset.value());
         !isTesSuccess(ter))
         return ter;
 
-    if (assets.holds<MPTIssue>())
-    {
-        auto mptID = assets.get<MPTIssue>().getMptID();
-        auto issuance = ctx.view.read(keylet::mptIssuance(mptID));
-        if (!issuance)
-            return tecOBJECT_NOT_FOUND;
-        if ((issuance->getFlags() & lsfMPTCanTransfer) == 0)
-            return tecNO_AUTH;
-    }
-
     if (accountHolds(
             ctx.view,
             account,

src/xrpld/app/tx/detail/VaultWithdraw.cpp

@@ -63,9 +63,9 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
         return tecNO_ENTRY;
 
     auto const assets = ctx.tx[sfAmount];
-    auto const asset = vault->at(sfAsset);
+    auto const vaultAsset = vault->at(sfAsset);
-    auto const share = vault->at(sfShareMPTID);
+    auto const vaultShare = vault->at(sfShareMPTID);
-    if (assets.asset() != asset && assets.asset() != share)
+    if (assets.asset() != vaultAsset && assets.asset() != vaultShare)
         return tecWRONG_ASSET;
 
     // Enforce valid withdrawal policy
@@ -95,29 +95,18 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx)
             if (!ctx.view.exists(keylet::depositPreauth(dstAcct, account)))
                 return tecNO_PERMISSION;
         }
-
-        if (assets.holds<MPTIssue>())
-        {
-            if (auto const ter = canTransfer(
-                    ctx.view, assets.get<MPTIssue>(), account, dstAcct);
-                !isTesSuccess(ter))
-                return ter;
-        }
     }
 
-    if (auto const ter = std::visit(
+    // Destination MPToken must exist (if asset is an MPT)
-            [&]<ValidIssueType TIss>(TIss const& issue) -> TER {
+    if (auto const ter = requireAuth(ctx.view, vaultAsset, dstAcct);
-                return requireAuth(ctx.view, issue, dstAcct);
-            },
-            asset.value());
         !isTesSuccess(ter))
         return ter;
 
     // Cannot withdraw from a Vault an Asset frozen for the destination account
-    if (isFrozen(ctx.view, dstAcct, asset))
+    if (isFrozen(ctx.view, dstAcct, vaultAsset))
-        return asset.holds<Issue>() ? tecFROZEN : tecLOCKED;
+        return vaultAsset.holds<Issue>() ? tecFROZEN : tecLOCKED;
 
-    if (isFrozen(ctx.view, account, share))
+    if (isFrozen(ctx.view, account, vaultShare))
         return tecLOCKED;
 
     return tesSUCCESS;
@@ -128,12 +117,12 @@ VaultWithdraw::doApply()
 {
     auto const vault = view().peek(keylet::vault(ctx_.tx[sfVaultID]));
     if (!vault)
-        return tefINTERNAL;  // Enforced in preclaim
+        return tefINTERNAL;  // LCOV_EXCL_LINE
 
     auto const mptIssuanceID = (*vault)[sfShareMPTID];
     auto const sleIssuance = view().read(keylet::mptIssuance(mptIssuanceID));
     if (!sleIssuance)
-        return tefINTERNAL;
+        return tefINTERNAL;  // LCOV_EXCL_LINE
 
     // Note, we intentionally do not check lsfVaultPrivate flag on the Vault. If
     // you have a share in the vault, it means you were at some point authorized

src/xrpld/ledger/View.h

@@ -701,6 +701,12 @@ transferXRP(
     STAmount const& amount,
     beast::Journal j);
 
+/* Check if MPToken exists:
+ * - StrongAuth - before checking lsfMPTRequireAuth is set
+ * - WeakAuth - after checking if lsfMPTRequireAuth is set
+ */
+enum class MPTAuthType : bool { StrongAuth = true, WeakAuth = false };
+
 /** Check if the account lacks required authorization.
  *
  *   Return tecNO_AUTH or tecNO_LINE if it does
@@ -711,22 +717,46 @@ requireAuth(ReadView const& view, Issue const& issue, AccountID const& account);
 
 /** Check if the account lacks required authorization.
  *
- *   This will also check for expired credentials. If it is called directly
+ * This will also check for expired credentials. If it is called directly
- *   from preclaim, the user should convert result tecEXPIRED to tesSUCCESS and
+ * from preclaim, the user should convert result tecEXPIRED to tesSUCCESS and
- *   proceed to also check permissions with enforceMPTokenAuthorization inside
+ * proceed to also check permissions with enforceMPTokenAuthorization inside
- *   doApply. This will ensure that any expired credentials are deleted.
+ * doApply. This will ensure that any expired credentials are deleted.
  *
- *   requireAuth check is recursive for MPT shares in a vault, descending to
+ * requireAuth check is recursive for MPT shares in a vault, descending to
- *   assets in the vault, up to maxAssetCheckDepth recursion depth. This is
+ * assets in the vault, up to maxAssetCheckDepth recursion depth. This is
- *   purely defensive, as we currently do not allow such vaults to be created.
+ * purely defensive, as we currently do not allow such vaults to be created.
+ *
+ * If StrongAuth then return tecNO_AUTH if MPToken doesn't exist or
+ * lsfMPTRequireAuth is set and MPToken is not authorized. If WeakAuth then
+ * return tecNO_AUTH if lsfMPTRequireAuth is set and MPToken doesn't exist or is
+ * not authorized (explicitly or via credentials, if DomainID is set in
+ * MPTokenIssuance). Consequently, if WeakAuth and lsfMPTRequireAuth is *not*
+ * set, this function will return true even if MPToken does *not* exist.
  */
 [[nodiscard]] TER
 requireAuth(
     ReadView const& view,
     MPTIssue const& mptIssue,
     AccountID const& account,
+    MPTAuthType authType = MPTAuthType::StrongAuth,
     int depth = 0);
 
+[[nodiscard]] TER inline requireAuth(
+    ReadView const& view,
+    Asset const& asset,
+    AccountID const& account,
+    MPTAuthType authType = MPTAuthType::StrongAuth)
+{
+    return std::visit(
+        [&]<ValidIssueType TIss>(TIss const& issue_) {
+            if constexpr (std::is_same_v<TIss, Issue>)
+                return requireAuth(view, issue_, account);
+            else
+                return requireAuth(view, issue_, account, authType);
+        },
+        asset.value());
+}
+
 /** Enforce account has MPToken to match its authorization.
  *
  *   Called from doApply - it will check for expired (and delete if found any)
@@ -767,7 +797,8 @@ canTransfer(
     ReadView const& view,
     MPTIssue const& mptIssue,
     AccountID const& from,
-    AccountID const& to);
+    AccountID const& to,
+    int depth = 0);
 
 /** Deleter function prototype. Returns the status of the entry deletion
  * (if should not be skipped) and if the entry should be skipped. The status

src/xrpld/ledger/detail/View.cpp

@@ -501,7 +501,8 @@ accountHolds(
         if (zeroIfUnauthorized == ahZERO_IF_UNAUTHORIZED &&
             view.rules().enabled(featureSingleAssetVault))
         {
-            if (auto const err = requireAuth(view, mptIssue, account);
+            if (auto const err = requireAuth(
+                    view, mptIssue, account, MPTAuthType::StrongAuth);
                 !isTesSuccess(err))
                 amount.clear(mptIssue);
         }
@@ -2278,6 +2279,7 @@ requireAuth(
     ReadView const& view,
     MPTIssue const& mptIssue,
     AccountID const& account,
+    MPTAuthType authType,
     int depth)
 {
     auto const mptID = keylet::mptIssuance(mptIssue.getMptID());
@@ -2314,7 +2316,8 @@ requireAuth(
                         if constexpr (std::is_same_v<TIss, Issue>)
                             return requireAuth(view, issue, account);
                         else
-                            return requireAuth(view, issue, account, depth + 1);
+                            return requireAuth(
+                                view, issue, account, authType, depth + 1);
                     },
                     asset.value());
                 !isTesSuccess(err))
@@ -2324,6 +2327,11 @@ requireAuth(
 
     auto const mptokenID = keylet::mptoken(mptID.key, account);
     auto const sleToken = view.read(mptokenID);
+
+    // if account has no MPToken, fail
+    if (!sleToken && authType == MPTAuthType::StrongAuth)
+        return tecNO_AUTH;
+
     // Note, this check is not amendment-gated because DomainID will be always
     // empty **unless** writing to it has been enabled by an amendment
     auto const maybeDomainID = sleIssuance->at(~sfDomainID);
@@ -2331,27 +2339,24 @@ requireAuth(
     {
         XRPL_ASSERT(
             sleIssuance->getFieldU32(sfFlags) & lsfMPTRequireAuth,
-            "ripple::requireAuth issuance requires authorization");
+            "ripple::requireAuth : issuance requires authorization");
-        // We ignore error from validDomain if we found sleToken, as it could
+        // ter = tefINTERNAL | tecOBJECT_NOT_FOUND | tecNO_AUTH | tecEXPIRED
-        // belong to someone who is explicitly authorized e.g. a vault owner.
-        // err = tefINTERNAL | tecOBJECT_NOT_FOUND | tecNO_AUTH | tecEXPIRED
         if (auto const ter =
                 credentials::validDomain(view, *maybeDomainID, account);
-            isTesSuccess(ter) || sleToken == nullptr)
+            isTesSuccess(ter))
+            return ter;  // Note: sleToken might be null
+        else if (!sleToken)
             return ter;
+        // We ignore error from validDomain if we found sleToken, as it could
+        // belong to someone who is explicitly authorized e.g. a vault owner.
     }
 
-    // if account has no MPToken, fail
-    if (!sleToken)
-        return tecNO_AUTH;
-
     // mptoken must be authorized if issuance enabled requireAuth
     if (sleIssuance->isFlag(lsfMPTRequireAuth) &&
-        !(sleToken->isFlag(lsfMPTAuthorized)))
+        (!sleToken || (!(sleToken->getFlags() & lsfMPTAuthorized))))
         return tecNO_AUTH;
 
-    // We are authorized by permissioned domain.
+    return tesSUCCESS;  // Note: sleToken might be null
-    return tesSUCCESS;
 }
 
 [[nodiscard]] TER
@@ -2450,7 +2455,8 @@ canTransfer(
     ReadView const& view,
     MPTIssue const& mptIssue,
     AccountID const& from,
-    AccountID const& to)
+    AccountID const& to,
+    int depth)
 {
     auto const mptID = keylet::mptIssuance(mptIssue.getMptID());
     auto const sleIssuance = view.read(mptID);