Move addEmptyHolding to VaultCreate and removeEmptyHolding to VaultDelete

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

@@ -17,10 +17,12 @@
 */
 //==============================================================================
 
+#include <xrpld/app/tx/detail/MPTokenAuthorize.h>
 #include <xrpld/app/tx/detail/MPTokenIssuanceCreate.h>
 #include <xrpld/app/tx/detail/VaultCreate.h>
 #include <xrpld/ledger/View.h>
 
+#include <xrpl/protocol/Asset.h>
 #include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/MPTIssue.h>
@@ -128,6 +130,70 @@ VaultCreate::preclaim(PreclaimContext const& ctx)
     return tesSUCCESS;
 }
 
+[[nodiscard]] static TER
+addEmptyHolding(
+    ApplyView& view,
+    AccountID const& accountID,
+    XRPAmount priorBalance,
+    Issue const& issue,
+    beast::Journal journal)
+{
+    // Every account can hold XRP.
+    if (issue.native())
+        return tesSUCCESS;
+
+    auto const& issuerId = issue.getIssuer();
+    auto const& currency = issue.currency;
+    if (isGlobalFrozen(view, issuerId))
+        return tecFROZEN;
+
+    auto const& srcId = issuerId;
+    auto const& dstId = accountID;
+    auto const high = srcId > dstId;
+    auto const index = keylet::line(srcId, dstId, currency);
+    auto const sle = view.peek(keylet::account(accountID));
+    if (!sle)
+        return tefINTERNAL;
+    return trustCreate(
+        view,
+        high,
+        srcId,
+        dstId,
+        index.key,
+        sle,
+        /*auth=*/false,
+        /*noRipple=*/true,
+        /*freeze=*/false,
+        /*deepFreeze*/ false,
+        /*balance=*/STAmount{Issue{currency, noAccount()}},
+        /*limit=*/STAmount{Issue{currency, dstId}},
+        /*qualityIn=*/0,
+        /*qualityOut=*/0,
+        journal);
+}
+
+[[nodiscard]] static TER
+addEmptyHolding(
+    ApplyView& view,
+    AccountID const& accountID,
+    XRPAmount priorBalance,
+    MPTIssue const& mptIssue,
+    beast::Journal journal)
+{
+    auto const& mptID = mptIssue.getMptID();
+    auto const mpt = view.peek(keylet::mptIssuance(mptID));
+    if (!mpt)
+        return tefINTERNAL;
+    if (mpt->getFlags() & lsfMPTLocked)
+        return tecLOCKED;
+    return MPTokenAuthorize::authorize(
+        view,
+        journal,
+        {.priorBalance = priorBalance,
+         .mptIssuanceID = mptID,
+         .accountID = accountID});
+}
+
 TER
 VaultCreate::doApply()
 {
@@ -156,9 +222,15 @@ VaultCreate::doApply()
         return maybePseudo.error();
     auto& pseudo = *maybePseudo;
     auto pseudoId = pseudo->at(sfAccount);
+    auto asset = tx[sfAsset];
 
-    if (auto ter =
-            addEmptyHolding(view(), pseudoId, mPriorBalance, tx[sfAsset], j_))
+    if (auto ter = std::visit(
+            [&]<ValidIssueType TIss>(TIss const& issue) -> TER {
+                return addEmptyHolding(
+                    view(), pseudoId, mPriorBalance, issue, j_);
+            },
+            asset.value());
+        !isTesSuccess(ter))
         return ter;
 
     auto txFlags = tx.getFlags();
@@ -168,10 +240,10 @@ VaultCreate::doApply()
     if (txFlags & tfVaultPrivate)
         mptFlags |= lsfMPTRequireAuth;
 
-    // Note, here we are **not** creating an MPToken for the assets held in the
-    // vault. That MPToken or TrustLine/RippleState is created above, in
-    // addEmptyHolding. Here we are creating MPTokenIssuance for the shares in
-    // the vault
+    // Note, here we are **not** creating an MPToken for the assets held in
+    // the vault. That MPToken or TrustLine/RippleState is created above, in
+    // addEmptyHolding. Here we are creating MPTokenIssuance for the shares
+    // in the vault
     auto maybeShare = MPTokenIssuanceCreate::create(
         view(),
         j_,
@@ -192,7 +264,7 @@ VaultCreate::doApply()
     vault->at(sfSequence) = sequence;
     vault->at(sfOwner) = ownerId;
     vault->at(sfAccount) = pseudoId;
-    vault->at(sfAsset) = tx[sfAsset];
+    vault->at(sfAsset) = asset;
     vault->at(sfAssetsTotal) = Number(0);
     vault->at(sfAssetsAvailable) = Number(0);
     vault->at(sfLossUnrealized) = Number(0);

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

@@ -17,6 +17,7 @@
 */
 //==============================================================================
 
+#include <xrpld/app/tx/detail/MPTokenAuthorize.h>
 #include <xrpld/app/tx/detail/MPTokenIssuanceDestroy.h>
 #include <xrpld/app/tx/detail/VaultDelete.h>
 #include <xrpld/ledger/View.h>
@@ -65,6 +66,56 @@ VaultDelete::preclaim(PreclaimContext const& ctx)
     return tesSUCCESS;
 }
 
+[[nodiscard]] static TER
+removeEmptyHolding(
+    ApplyView& view,
+    AccountID const& accountID,
+    Issue const& issue,
+    beast::Journal journal)
+{
+    if (issue.native())
+    {
+        auto const sle = view.read(keylet::account(accountID));
+        if (!sle)
+            return tecINTERNAL;
+        auto const balance = sle->getFieldAmount(sfBalance);
+        if (balance.xrp() != 0)
+            return tecHAS_OBLIGATIONS;
+        return tesSUCCESS;
+    }
+
+    // `asset` is an IOU.
+    auto const line = view.peek(keylet::line(accountID, issue));
+    if (!line)
+        return tecOBJECT_NOT_FOUND;
+    if (line->at(sfBalance)->iou() != beast::zero)
+        return tecHAS_OBLIGATIONS;
+    return trustDelete(
+        view,
+        line,
+        line->at(sfLowLimit)->getIssuer(),
+        line->at(sfHighLimit)->getIssuer(),
+        journal);
+}
+
+[[nodiscard]] static TER
+removeEmptyHolding(
+    ApplyView& view,
+    AccountID const& accountID,
+    MPTIssue const& mptIssue,
+    beast::Journal journal)
+{
+    auto const& mptID = mptIssue.getMptID();
+    // `MPTokenAuthorize::authorize` asserts that the balance is 0.
+    return MPTokenAuthorize::authorize(
+        view,
+        journal,
+        {.priorBalance = {},
+         .mptIssuanceID = mptID,
+         .accountID = accountID,
+         .flags = tfMPTUnauthorize});
+}
+
 TER
 VaultDelete::doApply()
 {
@@ -73,8 +124,14 @@ VaultDelete::doApply()
         return tefINTERNAL;  // Enforced in preclaim
 
     // Destroy the asset holding.
-    if (auto ter = removeEmptyHolding(
-            view(), vault->at(sfAccount), vault->at(sfAsset), j_))
+    auto asset = vault->at(sfAsset);
+    if (auto ter = std::visit(
+            [&]<ValidIssueType TIss>(TIss const& issue) -> TER {
+                return removeEmptyHolding(
+                    view(), vault->at(sfAccount), issue, j_);
+            },
+            (*asset).value());
+        !isTesSuccess(ter))
         return ter;
 
     // Destroy the share issuance.

src/xrpld/ledger/View.h

@@ -535,30 +535,6 @@ trustDelete(
     AccountID const& uHighAccountID,
     beast::Journal j);
 
-/** Create the structures necessary for an account to hold an asset.
- *
- * If the asset is:
- * - XRP: Do nothing.
- * - IOU: Check that the asset is not globally frozen,
- *   and create a trust line (with limit 0).
- * - MPT: Check that the asset is not globally locked,
- *   and create an MPToken.
- */
-[[nodiscard]] TER
-addEmptyHolding(
-    ApplyView& view,
-    AccountID const& account,
-    XRPAmount priorBalance,
-    Asset const& asset,
-    beast::Journal journal);
-
-[[nodiscard]] TER
-removeEmptyHolding(
-    ApplyView& view,
-    AccountID const& account,
-    Asset const& asset,
-    beast::Journal journal);
-
 /** Delete an offer.
 
     Requirements:

src/xrpld/ledger/detail/View.cpp

@@ -1263,127 +1263,6 @@ trustDelete(
     return tesSUCCESS;
 }
 
-[[nodiscard]] TER
-addEmptyHolding(
-    ApplyView& view,
-    AccountID const& accountID,
-    XRPAmount priorBalance,
-    Asset const& asset,
-    beast::Journal journal)
-{
-    if (asset.holds<Issue>())
-    {
-        auto const& issue = asset.get<Issue>();
-        // Every account can hold XRP.
-        if (issue.native())
-            return tesSUCCESS;
-
-        auto const& issuerId = issue.getIssuer();
-        auto const& currency = issue.currency;
-        if (isGlobalFrozen(view, issuerId))
-            return tecFROZEN;
-
-        auto const& srcId = issuerId;
-        auto const& dstId = accountID;
-        auto const high = srcId > dstId;
-        auto const index = keylet::line(srcId, dstId, currency);
-        auto const sle = view.peek(keylet::account(accountID));
-        if (!sle)
-            return tefINTERNAL;
-        return trustCreate(
-            view,
-            high,
-            srcId,
-            dstId,
-            index.key,
-            sle,
-            /*auth=*/false,
-            /*noRipple=*/true,
-            /*freeze=*/false,
-            /*deepFreeze*/ false,
-            /*balance=*/STAmount{Issue{currency, noAccount()}},
-            /*limit=*/STAmount{Issue{currency, dstId}},
-            /*qualityIn=*/0,
-            /*qualityOut=*/0,
-            journal);
-    }
-
-    if (asset.holds<MPTIssue>())
-    {
-        auto const& mptIssue = asset.get<MPTIssue>();
-        auto const& mptID = mptIssue.getMptID();
-        auto const mpt = view.peek(keylet::mptIssuance(mptID));
-        if (!mpt)
-            return tefINTERNAL;
-        if (mpt->getFlags() & lsfMPTLocked)
-            return tecLOCKED;
-        return MPTokenAuthorize::authorize(
-            view,
-            journal,
-            {.priorBalance = priorBalance,
-             .mptIssuanceID = mptID,
-             .accountID = accountID});
-    }
-
-    UNREACHABLE(
-        "ripple::addEmptyHolding : neither Issue nor MPTIssue");  // LCOV_EXCL_LINE
-    return tecINTERNAL;  // LCOV_EXCL_LINE
-}
-
-[[nodiscard]] TER
-removeEmptyHolding(
-    ApplyView& view,
-    AccountID const& accountID,
-    Asset const& asset,
-    beast::Journal journal)
-{
-    if (asset.holds<Issue>())
-    {
-        auto const& issue = asset.get<Issue>();
-        if (issue.native())
-        {
-            auto const sle = view.read(keylet::account(accountID));
-            if (!sle)
-                return tecINTERNAL;
-            auto const balance = sle->getFieldAmount(sfBalance);
-            if (balance.xrp() != 0)
-                return tecHAS_OBLIGATIONS;
-            return tesSUCCESS;
-        }
-
-        // `asset` is an IOU.
-        auto const line = view.peek(keylet::line(accountID, issue));
-        if (!line)
-            return tecOBJECT_NOT_FOUND;
-        if (line->at(sfBalance)->iou() != beast::zero)
-            return tecHAS_OBLIGATIONS;
-        return trustDelete(
-            view,
-            line,
-            line->at(sfLowLimit)->getIssuer(),
-            line->at(sfHighLimit)->getIssuer(),
-            journal);
-    }
-
-    if (asset.holds<MPTIssue>())
-    {
-        auto const& mptIssue = asset.get<MPTIssue>();
-        auto const& mptID = mptIssue.getMptID();
-        // `MPTokenAuthorize::authorize` asserts that the balance is 0.
-        return MPTokenAuthorize::authorize(
-            view,
-            journal,
-            {.priorBalance = {},
-             .mptIssuanceID = mptID,
-             .accountID = accountID,
-             .flags = tfMPTUnauthorize});
-    }
-
-    UNREACHABLE(
-        "ripple::removeEmptyHolding : neither Issue nor MPTIssue");  // LCOV_EXCL_LINE
-    return tecINTERNAL;  // LCOV_EXCL_LINE
-}
-
 TER
 offerDelete(ApplyView& view, std::shared_ptr<SLE> const& sle, beast::Journal j)
 {