Implicitly authorize Vault and LoanBroker pseudo-accounts if MPTRequireAuth is set.

include/xrpl/ledger/View.h

@@ -673,14 +673,17 @@ createPseudoAccount(
     uint256 const& pseudoOwnerKey,
     SField const& ownerField);
 
-// Returns true iff sleAcct is a pseudo-account.
+// Returns true iff sleAcct is a pseudo-account or specific
+// pseudo-accounts in pseudoAccountFields.
 //
 // Returns false if sleAcct is
 // * NOT a pseudo-account OR
 // * NOT a ltACCOUNT_ROOT OR
 // * null pointer
 [[nodiscard]] bool
-isPseudoAccount(std::shared_ptr<SLE const> sleAcct);
+isPseudoAccount(
+    std::shared_ptr<SLE const> sleAcct,
+    std::set<SField const*> const& pseudoAccountFields = {});
 
 // Returns the list of fields that define an ACCOUNT_ROOT as a pseudo-account if
 // set
@@ -694,9 +697,13 @@ isPseudoAccount(std::shared_ptr<SLE const> sleAcct);
 getPseudoAccountFields();
 
 [[nodiscard]] inline bool
-isPseudoAccount(ReadView const& view, AccountID const& accountId)
+isPseudoAccount(
+    ReadView const& view,
+    AccountID const& accountId,
+    std::set<SField const*> const& pseudoAccountFields = {})
 {
-    return isPseudoAccount(view.read(keylet::account(accountId)));
+    return isPseudoAccount(
+        view.read(keylet::account(accountId)), pseudoAccountFields);
 }
 
 [[nodiscard]] TER
@@ -1020,7 +1027,8 @@ requireAuth(
  * 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.
+ * lsfMPTRequireAuth is set and MPToken is not authorized. Vault and LoanBroker
+ * pseudo-accounts are implicitly 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

src/libxrpl/ledger/View.cpp

@@ -1223,7 +1223,8 @@ getPseudoAccountFields()
         {
             // LCOV_EXCL_START
             LogicError(
-                "ripple::isPseudoAccount : unable to find account root ledger "
+                "ripple::getPseudoAccountFields : unable to find account root "
+                "ledger "
                 "format");
             // LCOV_EXCL_STOP
         }
@@ -1241,7 +1242,9 @@ getPseudoAccountFields()
 }
 
 [[nodiscard]] bool
-isPseudoAccount(std::shared_ptr<SLE const> sleAcct)
+isPseudoAccount(
+    std::shared_ptr<SLE const> sleAcct,
+    std::set<SField const*> const& pseudoAccountFields)
 {
     auto const& fields = getPseudoAccountFields();
 
@@ -1249,8 +1252,12 @@ isPseudoAccount(std::shared_ptr<SLE const> sleAcct)
     // semantics of true return value clean.
     return sleAcct && sleAcct->getType() == ltACCOUNT_ROOT &&
         std::count_if(
-            fields.begin(), fields.end(), [&sleAcct](SField const* sf) -> bool {
-                return sleAcct->isFieldPresent(*sf);
+            fields.begin(),
+            fields.end(),
+            [&sleAcct, &pseudoAccountFields](SField const* sf) -> bool {
+                return sleAcct->isFieldPresent(*sf) &&
+                    (pseudoAccountFields.empty() ||
+                     pseudoAccountFields.contains(sf));
             }) > 0;
 }
 
@@ -3118,7 +3125,10 @@ requireAuth(
     if (mptIssuer == account)  // Issuer won't have MPToken
         return tesSUCCESS;
 
-    if (view.rules().enabled(featureSingleAssetVault))
+    bool const featureSAVEnabled =
+        view.rules().enabled(featureSingleAssetVault);
+
+    if (featureSAVEnabled)
     {
         if (depth >= maxAssetCheckDepth)
             return tecINTERNAL;  // LCOV_EXCL_LINE
@@ -3177,6 +3187,13 @@ requireAuth(
         // belong to someone who is explicitly authorized e.g. a vault owner.
     }
 
+    if (featureSAVEnabled)
+    {
+        // Implicitly authorize Vault and LoanBroker pseudo-accounts
+        if (isPseudoAccount(view, account, {&sfVaultID, &sfLoanBrokerID}))
+            return tesSUCCESS;
+    }
+
     // mptoken must be authorized if issuance enabled requireAuth
     if (sleIssuance->isFlag(lsfMPTRequireAuth) &&
         (!sleToken || !sleToken->isFlag(lsfMPTAuthorized)))

src/test/app/LoanBroker_test.cpp

@@ -1229,6 +1229,112 @@ class LoanBroker_test : public beast::unit_test::suite
         (void)LoanBrokerCoverDeposit::preclaim(pctx);
     }
 
+    void
+    testRequireAuth()
+    {
+        testcase("Require Auth - Implicit Pseudo-account authorization");
+        using namespace jtx;
+        using namespace loanBroker;
+
+        Account const issuer{"issuer"};
+        Account const alice{"alice"};
+        Env env(*this);
+        Vault vault{env};
+
+        env.fund(XRP(100'000), issuer, alice);
+        env.close();
+
+        auto asset = MPTTester({
+            .env = env,
+            .issuer = issuer,
+            .holders = {alice},
+            .flags = MPTDEXFlags | tfMPTRequireAuth | tfMPTCanClawback |
+                tfMPTCanLock,
+            .authHolder = true,
+        });
+
+        env(pay(issuer, alice, asset(100'000)));
+        env.close();
+
+        // Alice is not authorized, can still create the vault
+        asset.authorize(
+            {.account = issuer, .holder = alice, .flags = tfMPTUnauthorize});
+        auto [tx, vaultKeylet] = vault.create({.owner = alice, .asset = asset});
+        env(tx);
+        env.close();
+
+        auto const le = env.le(vaultKeylet);
+        VaultInfo vaultInfo = [&]() {
+            if (BEAST_EXPECT(le))
+                return VaultInfo{asset, vaultKeylet.key, le->at(sfAccount)};
+            return VaultInfo{asset, {}, {}};
+        }();
+        if (vaultInfo.vaultID == uint256{})
+            return;
+
+        auto forUnauthAuth = [&](auto&& doTx) {
+            for (auto const flag : {tfMPTUnauthorize, 0u})
+            {
+                asset.authorize(
+                    {.account = issuer, .holder = alice, .flags = flag});
+                env.close();
+                doTx(flag == 0);
+                env.close();
+            }
+        };
+
+        // Can't deposit into Vault if the vault owner is not authorized
+        forUnauthAuth([&](bool authorized) {
+            auto const err = !authorized ? ter(tecNO_AUTH) : ter(tesSUCCESS);
+            env(vault.deposit(
+                    {.depositor = alice,
+                     .id = vaultKeylet.key,
+                     .amount = asset(51)}),
+                err);
+        });
+
+        // Can't withdraw from Vault if the vault owner is not authorized
+        forUnauthAuth([&](bool authorized) {
+            auto const err = !authorized ? ter(tecNO_AUTH) : ter(tesSUCCESS);
+            env(vault.withdraw(
+                    {.depositor = alice,
+                     .id = vaultKeylet.key,
+                     .amount = asset(1)}),
+                err);
+        });
+
+        auto const brokerKeylet =
+            keylet::loanbroker(alice.id(), env.seq(alice));
+        // Can create LoanBroker if the vault owner is not authorized
+        forUnauthAuth([&](auto) { env(set(alice, vaultInfo.vaultID)); });
+
+        auto broker = env.le(brokerKeylet);
+        if (!BEAST_EXPECT(broker))
+            return;
+
+        // Can't cover deposit into Vault if the vault owner is not authorized
+        forUnauthAuth([&](bool authorized) {
+            auto const err =
+                !authorized ? ter(tecINSUFFICIENT_FUNDS) : ter(tesSUCCESS);
+            env(coverDeposit(alice, brokerKeylet.key, vaultInfo.asset(10)),
+                err);
+        });
+
+        // Can't cover withdraw from Vault if the vault owner is not authorized
+        forUnauthAuth([&](bool authorized) {
+            auto const err = !authorized ? ter(tecNO_AUTH) : ter(tesSUCCESS);
+            env(coverWithdraw(alice, brokerKeylet.key, vaultInfo.asset(5)),
+                err);
+        });
+
+        // Issuer can always cover clawback. The holder authorization is n/a.
+        forUnauthAuth([&](bool) {
+            env(coverClawback(issuer),
+                loanBrokerID(brokerKeylet.key),
+                amount(vaultInfo.asset(1)));
+        });
+    }
+
 public:
     void
     run() override
@@ -1242,6 +1348,7 @@ public:
         testInvalidLoanBrokerCoverWithdraw();
         testInvalidLoanBrokerDelete();
         testInvalidLoanBrokerSet();
+        testRequireAuth();
 
         // TODO: Write clawback failure tests with an issuer / MPT that doesn't
         // have the right flags set.

src/test/app/Loan_test.cpp

@@ -4640,6 +4640,66 @@ class Loan_test : public beast::unit_test::suite
         }
     }
 
+    void
+    testRequireAuth()
+    {
+        testcase("Require Auth - Implicit Pseudo-account authorization");
+        using namespace jtx;
+        using namespace loan;
+        Account const lender{"lender"};
+        Account const issuer{"issuer"};
+        Account const borrower{"borrower"};
+        Env env(*this);
+
+        env.fund(XRP(100'000), issuer, lender, borrower);
+        env.close();
+
+        auto asset = MPTTester({
+            .env = env,
+            .issuer = issuer,
+            .holders = {lender, borrower},
+            .flags = MPTDEXFlags | tfMPTRequireAuth | tfMPTCanClawback |
+                tfMPTCanLock,
+            .authHolder = true,
+        });
+
+        env(pay(issuer, lender, asset(5'000'000)));
+        BrokerInfo brokerInfo{createVaultAndBroker(env, asset, lender)};
+
+        auto const loanSetFee = fee(env.current()->fees().base * 2);
+        STAmount const debtMaximumRequest = brokerInfo.asset(1'000).value();
+
+        auto forUnauthAuth = [&](auto&& doTx) {
+            for (auto const flag : {tfMPTUnauthorize, 0u})
+            {
+                asset.authorize(
+                    {.account = issuer, .holder = borrower, .flags = flag});
+                env.close();
+                doTx(flag == 0);
+                env.close();
+            }
+        };
+
+        // Can't create a loan if the borrower is not authorized
+        forUnauthAuth([&](bool authorized) {
+            auto const err = !authorized ? ter(tecNO_AUTH) : ter(tesSUCCESS);
+            env(set(borrower, brokerInfo.brokerID, debtMaximumRequest),
+                sig(sfCounterpartySignature, lender),
+                loanSetFee,
+                err);
+        });
+
+        std::uint32_t constexpr loanSequence = 1;
+        auto const loanKeylet = keylet::loan(brokerInfo.brokerID, loanSequence);
+
+        // Can't loan pay if the borrower is not authorize
+        forUnauthAuth([&](bool authorized) {
+            auto const err =
+                !authorized ? ter(tecINSUFFICIENT_FUNDS) : ter(tesSUCCESS);
+            env(pay(borrower, loanKeylet.key, debtMaximumRequest), err);
+        });
+    }
+
 public:
     void
     run() override
@@ -4669,6 +4729,8 @@ public:
         testLoanPayComputePeriodicPaymentValidTotalPrincipalPaidInvariant();
         testLoanPayComputePeriodicPaymentValidTotalInterestPaidInvariant();
         testLoanNextPaymentDueDateOverflow();
+
+        testRequireAuth();
     }
 };