Both isFrozen and requireAuth are now recursive for vault shares

include/xrpl/protocol/Protocol.h

@@ -122,6 +122,8 @@ std::size_t constexpr maxDataPayloadLength = 256;
 /** Vault withdrawal policies */
 std::uint8_t constexpr vaultStrategyFirstComeFirstServe = 1;
 
+std::uint8_t constexpr maxFreezeCheckDepth = 5;
+
 /** A ledger index. */
 using LedgerIndex = std::uint32_t;
 

src/test/app/Vault_test.cpp

@@ -78,6 +78,12 @@ class Vault_test : public beast::unit_test::suite
             env.close();
             BEAST_EXPECT(env.le(keylet));
 
+            auto const share = [&env, keylet = keylet, this]() -> PrettyAsset {
+                auto const vault = env.le(keylet);
+                BEAST_EXPECT(vault != nullptr);
+                return MPTIssue(vault->at(sfShareMPTID));
+            }();
+
             // Several 3rd party accounts which cannot receive funds
             Account alice{"alice"};
             Account dave{"dave"};
@@ -352,6 +358,36 @@ class Vault_test : public beast::unit_test::suite
                     ter{asset.raw().holds<Issue>() ? tecNO_LINE : tecNO_AUTH});
             }
 
+            if (!asset.raw().native() && asset.raw().holds<Issue>())
+            {
+                testcase(prefix + " temporary authorization for 3rd party");
+                env(trust(erin, asset(1000)));
+                env(trust(issuer, asset(0), erin, tfSetfAuth));
+                env(pay(issuer, erin, asset(10)));
+
+                // Erin deposits all in vault, then sends shares to depositor
+                auto tx = vault.deposit(
+                    {.depositor = erin, .id = keylet.key, .amount = asset(10)});
+                env(tx);
+                env(pay(erin, depositor, share(10)));
+
+                testcase(prefix + " withdraw to authorized 3rd party");
+                // Depositor withdraws shares, destined to Erin
+                tx = vault.withdraw(
+                    {.depositor = depositor,
+                     .id = keylet.key,
+                     .amount = asset(10)});
+                tx[sfDestination] = erin.human();
+                env(tx);
+                // Erin returns assets to issuer
+                env(pay(erin, issuer, asset(10)));
+
+                testcase(prefix + " fail to pay to unauthorized 3rd party");
+                env(trust(erin, asset(0)));
+                // Erin has MPToken but is no longer authorized to hold assets
+                env(pay(depositor, erin, share(1)), ter{tecNO_LINE});
+            }
+
             {
                 testcase(
                     prefix +
@@ -1252,6 +1288,12 @@ class Vault_test : public beast::unit_test::suite
             return vault->at(sfAccount);
         }();
 
+        auto const share = [&env, keylet = keylet, this]() -> Asset {
+            auto const vault = env.le(keylet);
+            BEAST_EXPECT(vault != nullptr);
+            return MPTIssue(vault->at(sfShareMPTID));
+        }();
+
         auto const vaultBalance =  //
             [&, account = vaultAccount, this]() -> PrettyAmount {
             auto const sle = env.le(keylet::line(account, issue));
@@ -1298,7 +1340,7 @@ class Vault_test : public beast::unit_test::suite
         }
 
         {
-            testcase("IOU deleted trust line, withdraw to 3rd party");
+            testcase("IOU froze trust line, cannot withdraw to 3rd party");
             env(trust(issuer, asset(0), owner, tfSetFreeze));
 
             auto tx = vault.withdraw(
@@ -1306,10 +1348,16 @@ class Vault_test : public beast::unit_test::suite
             env(tx, ter{tecFROZEN});
 
             tx[sfDestination] = charlie.human();
-            env(tx);
+            env(tx, ter{tecLOCKED});  // owner transitively locked via MPToken
             env.close();
 
-            BEAST_EXPECT(env.balance(charlie, issue) == asset(30));
+            auto tx1 = test::jtx::pay(owner, charlie, STAmount{share, 10});
+            env(tx1, ter{tecLOCKED});
+
+            auto tx2 = test::jtx::pay(charlie, owner, STAmount{share, 10});
+            env(tx2, ter{tecLOCKED});
+
+            BEAST_EXPECT(env.balance(charlie, issue) == asset(20));
         }
 
         {

src/xrpld/ledger/View.h

@@ -88,8 +88,13 @@ isGlobalFrozen(ReadView const& view, MPTIssue const& mptIssue);
 [[nodiscard]] bool
 isGlobalFrozen(ReadView const& view, Asset const& asset);
 
+// Note, depth parameter is used to limit the recursion depth
 [[nodiscard]] bool
-isVaultPseudoAccountFrozen(ReadView const& view, MPTIssue const& mptShare);
+isVaultPseudoAccountFrozen(
+    ReadView const& view,
+    AccountID const& account,
+    MPTIssue const& mptShare,
+    int depth);
 
 [[nodiscard]] bool
 isIndividualFrozen(
@@ -134,7 +139,11 @@ isFrozen(
     AccountID const& issuer);
 
 [[nodiscard]] inline bool
-isFrozen(ReadView const& view, AccountID const& account, Issue const& issue)
+isFrozen(
+    ReadView const& view,
+    AccountID const& account,
+    Issue const& issue,
+    int = 0 /*ignored*/)
 {
     return isFrozen(view, account, issue.currency, issue.account);
 }
@@ -143,13 +152,20 @@ isFrozen(ReadView const& view, AccountID const& account, Issue const& issue)
 isFrozen(
     ReadView const& view,
     AccountID const& account,
-    MPTIssue const& mptIssue);
+    MPTIssue const& mptIssue,
+    int depth = 0);
 
 [[nodiscard]] inline bool
-isFrozen(ReadView const& view, AccountID const& account, Asset const& asset)
+isFrozen(
+    ReadView const& view,
+    AccountID const& account,
+    Asset const& asset,
+    int depth = 0)
 {
     return std::visit(
-        [&](auto const& issue) { return isFrozen(view, account, issue); },
+        [&](auto const& issue) {
+            return isFrozen(view, account, issue, depth);
+        },
         asset.value());
 }
 
@@ -158,7 +174,8 @@ isAnyFrozen(
     ReadView const& view,
     AccountID const& account1,
     AccountID const& account2,
-    MPTIssue const& mptIssue);
+    MPTIssue const& mptIssue,
+    int depth = 0);
 
 /*
 We do not have a use case for these (yet ?)
@@ -687,9 +704,6 @@ struct TokenDescriptor
     std::shared_ptr<SLE const> issuance;
 };
 
-[[nodiscard]] TER
-requireAuth(ReadView const& view, Issue const& issue, AccountID const& account);
-
 /** Check if the account lacks required authorization.
  *
  *   Return tecNO_AUTH or tecNO_LINE if it does
@@ -709,7 +723,8 @@ requireAuth(ReadView const& view, Issue const& issue, AccountID const& account);
 requireAuth(
     ReadView const& view,
     MPTIssue const& mptIssue,
-    AccountID const& account);
+    AccountID const& account,
+    int depth = 0);
 
 /** Enforce account has MPToken to match its authorization.
  *

src/xrpld/ledger/detail/View.cpp

@@ -274,11 +274,12 @@ bool
 isFrozen(
     ReadView const& view,
     AccountID const& account,
-    MPTIssue const& mptIssue)
+    MPTIssue const& mptIssue,
+    int depth)
 {
     return isGlobalFrozen(view, mptIssue) ||
         isIndividualFrozen(view, account, mptIssue) ||
-        isVaultPseudoAccountFrozen(view, mptIssue);
+        isVaultPseudoAccountFrozen(view, account, mptIssue, depth);
 }
 
 [[nodiscard]] bool
@@ -286,16 +287,22 @@ isAnyFrozen(
     ReadView const& view,
     AccountID const& account1,
     AccountID const& account2,
-    MPTIssue const& mptIssue)
+    MPTIssue const& mptIssue,
+    int depth)
 {
     return isGlobalFrozen(view, mptIssue) ||
         isIndividualFrozen(view, account1, mptIssue) ||
         isIndividualFrozen(view, account2, mptIssue) ||
-        isVaultPseudoAccountFrozen(view, mptIssue);
+        isVaultPseudoAccountFrozen(view, account1, mptIssue, depth) ||
+        isVaultPseudoAccountFrozen(view, account2, mptIssue, depth);
 }
 
 bool
-isVaultPseudoAccountFrozen(ReadView const& view, MPTIssue const& mptShare)
+isVaultPseudoAccountFrozen(
+    ReadView const& view,
+    AccountID const& account,
+    MPTIssue const& mptShare,
+    int depth)
 {
     if (!view.rules().enabled(featureSingleAssetVault))
         return false;
@@ -321,6 +328,9 @@ isVaultPseudoAccountFrozen(ReadView const& view, MPTIssue const& mptShare)
     if (!mptIssuer->isFieldPresent(sfVaultID))
         return false;  // not a Vault pseudo-account, common case
 
+    if (depth > maxFreezeCheckDepth)
+        return true;  // fail at maximum 2^maxFreezeCheckDepth checks
+
     auto const vault =
         view.read(keylet::vault(mptIssuer->getFieldH256(sfVaultID)));
     if (vault == nullptr)
@@ -331,7 +341,8 @@ isVaultPseudoAccountFrozen(ReadView const& view, MPTIssue const& mptShare)
     }
 
     Asset const asset{vault->at(sfAsset)};
-    return isFrozen(view, issuer, asset);
+    return isFrozen(view, issuer, asset, depth + 1) ||
+        isFrozen(view, account, asset, depth + 1);
 }
 
 bool
@@ -2234,7 +2245,8 @@ TER
 requireAuth(
     ReadView const& view,
     MPTIssue const& mptIssue,
-    AccountID const& account)
+    AccountID const& account,
+    int depth)
 {
     auto const mptID = keylet::mptIssuance(mptIssue.getMptID());
     auto const sleIssuance = view.read(mptID);
@@ -2247,6 +2259,35 @@ requireAuth(
     if (mptIssuer == account)  // Issuer won't have MPToken
         return tesSUCCESS;
 
+    if (view.rules().enabled(featureSingleAssetVault))
+    {
+        // requireAuth is recursive if the issuer is a vault pseudo-account
+        auto const sleIssuer = view.read(keylet::account(mptIssuer));
+        if (!sleIssuer)
+            return tefINTERNAL;  // LCOV_EXCL_LINE
+
+        if (sleIssuer->isFieldPresent(sfVaultID))
+        {
+            auto const sleVault =
+                view.read(keylet::vault(sleIssuer->getFieldH256(sfVaultID)));
+            if (!sleVault)
+                return tefINTERNAL;  // LCOV_EXCL_LINE
+
+            if (depth > maxFreezeCheckDepth)
+                return tecKILLED;  // TODO: consider different code
+
+            auto const asset = sleVault->at(sfAsset);
+            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, depth + 1);
+                },
+                asset.value());
+        }
+    }
+
     auto const mptokenID = keylet::mptoken(mptID.key, account);
     auto const sleToken = view.read(mptokenID);
     // Note, this is not amendment-gated because  we do not want to maintain in