From 71de1a0d93cd6377fae5c39be44e5a7a7e39761f Mon Sep 17 00:00:00 2001
From: Ed Hennis <ed@ripple.com>
Date: Thu, 1 May 2025 18:23:11 -0400
Subject: [PATCH] Refactor issuance validity check in VaultCreate

- Utility function: canAddHolding
- Call canAddHolding from any transactor that call addEmptyHolding
  (LoanBrokerSet, LoanSet)
---
 src/xrpld/app/tx/detail/LoanBrokerSet.cpp |  2 ++
 src/xrpld/app/tx/detail/LoanSet.cpp       |  5 +++
 src/xrpld/app/tx/detail/VaultCreate.cpp   | 22 ++-----------
 src/xrpld/ledger/View.h                   |  5 +++
 src/xrpld/ledger/detail/View.cpp          | 38 +++++++++++++++++++++++
 5 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/src/xrpld/app/tx/detail/LoanBrokerSet.cpp b/src/xrpld/app/tx/detail/LoanBrokerSet.cpp
index 074855eb0e..c552336ea6 100644
--- a/src/xrpld/app/tx/detail/LoanBrokerSet.cpp
+++ b/src/xrpld/app/tx/detail/LoanBrokerSet.cpp
@@ -141,6 +141,8 @@ LoanBrokerSet::preclaim(PreclaimContext const& ctx)
             JLOG(ctx.j.warn()) << "Account is not the owner of the Vault.";
             return tecNO_PERMISSION;
         }
+        if (auto const ter = canAddHolding(ctx.view, sleVault->at(sfAsset)))
+            return ter;
     }
     return tesSUCCESS;
 }
diff --git a/src/xrpld/app/tx/detail/LoanSet.cpp b/src/xrpld/app/tx/detail/LoanSet.cpp
index 20bd18b391..518e819a60 100644
--- a/src/xrpld/app/tx/detail/LoanSet.cpp
+++ b/src/xrpld/app/tx/detail/LoanSet.cpp
@@ -200,6 +200,10 @@ LoanSet::preclaim(PreclaimContext const& ctx)
         // Should be impossible
         return tefBAD_LEDGER;  // LCOV_EXCL_LINE
     auto const asset = vault->at(sfAsset);
+
+    if (auto const ter = canAddHolding(ctx.view, asset))
+        return ter;
+
     if (isFrozen(ctx.view, brokerOwner, asset) ||
         isFrozen(ctx.view, brokerPseudo, asset))
     {
@@ -212,6 +216,7 @@ LoanSet::preclaim(PreclaimContext const& ctx)
         if (isDeepFrozen(ctx.view, borrower, issue.currency, issue.account))
             return tecFROZEN;
     }
+
     auto const principalRequested = tx[sfPrincipalRequested];
     if (auto const assetsAvailable = vault->at(sfAssetsAvailable);
         assetsAvailable < principalRequested)
diff --git a/src/xrpld/app/tx/detail/VaultCreate.cpp b/src/xrpld/app/tx/detail/VaultCreate.cpp
index 4e99505761..46fcf0d499 100644
--- a/src/xrpld/app/tx/detail/VaultCreate.cpp
+++ b/src/xrpld/app/tx/detail/VaultCreate.cpp
@@ -100,26 +100,8 @@ VaultCreate::preclaim(PreclaimContext const& ctx)
     auto vaultAsset = ctx.tx[sfAsset];
     auto account = ctx.tx[sfAccount];
 
-    if (vaultAsset.native())
-        ;  // No special checks for XRP
-    else if (vaultAsset.holds<MPTIssue>())
-    {
-        auto mptID = vaultAsset.get<MPTIssue>().getMptID();
-        auto issuance = ctx.view.read(keylet::mptIssuance(mptID));
-        if (!issuance)
-            return tecOBJECT_NOT_FOUND;
-        if (!issuance->isFlag(lsfMPTCanTransfer))
-            return tecNO_AUTH;
-    }
-    else if (vaultAsset.holds<Issue>())
-    {
-        auto const issuer =
-            ctx.view.read(keylet::account(vaultAsset.getIssuer()));
-        if (!issuer)
-            return terNO_ACCOUNT;
-        else if (!issuer->isFlag(lsfDefaultRipple))
-            return terNO_RIPPLE;
-    }
+    if (auto const ter = canAddHolding(ctx.view, vaultAsset))
+        return ter;
 
     // 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
diff --git a/src/xrpld/ledger/View.h b/src/xrpld/ledger/View.h
index 0510d1ff63..430d11b0f6 100644
--- a/src/xrpld/ledger/View.h
+++ b/src/xrpld/ledger/View.h
@@ -551,6 +551,11 @@ isPseudoAccount(ReadView const& view, AccountID accountId)
     return isPseudoAccount(view.read(keylet::account(accountId)));
 }
 
+[[nodiscard]] TER
+canAddHolding(ReadView const& view, Asset const& asset);
+
+/// Any transactors that call addEmptyHolding() in doApply must call
+/// canAddHolding() in preflight with the same View and Asset
 [[nodiscard]] TER
 addEmptyHolding(
     ApplyView& view,
diff --git a/src/xrpld/ledger/detail/View.cpp b/src/xrpld/ledger/detail/View.cpp
index e1ae98a94b..a9886d6f49 100644
--- a/src/xrpld/ledger/detail/View.cpp
+++ b/src/xrpld/ledger/detail/View.cpp
@@ -1150,6 +1150,44 @@ createPseudoAccount(
     return account;
 }
 
+[[nodiscard]] TER
+canAddHolding(ReadView const& view, Issue const& issue)
+{
+    if (issue.native())
+        return tesSUCCESS;  // No special checks for XRP
+
+    auto const issuer = view.read(keylet::account(issue.getIssuer()));
+    if (!issuer)
+        return terNO_ACCOUNT;
+    else if (!issuer->isFlag(lsfDefaultRipple))
+        return terNO_RIPPLE;
+
+    return tesSUCCESS;
+}
+
+[[nodiscard]] TER
+canAddHolding(ReadView const& view, MPTIssue const& mptIssue)
+{
+    auto mptID = mptIssue.getMptID();
+    auto issuance = view.read(keylet::mptIssuance(mptID));
+    if (!issuance)
+        return tecOBJECT_NOT_FOUND;
+    if (!issuance->isFlag(lsfMPTCanTransfer))
+        return tecNO_AUTH;
+
+    return tesSUCCESS;
+}
+
+[[nodiscard]] TER
+canAddHolding(ReadView const& view, Asset const& asset)
+{
+    return std::visit(
+        [&]<ValidIssueType TIss>(TIss const& issue) -> TER {
+            return canAddHolding(view, issue);
+        },
+        asset.value());
+}
+
 [[nodiscard]] TER
 addEmptyHolding(
     ApplyView& view,