Add unit test

src/test/app/Loan_test.cpp

@@ -5556,11 +5556,11 @@ protected:
         });
     }
 
-#if LOANTODO
     void
-    testCoverDepositAllowsNonTransferableMPT()
+    testCoverDepositWithdrawNonTransferableMPT()
     {
-        testcase("CoverDeposit accepts MPT without CanTransfer");
+        testcase(
+            "CoverDeposit and CoverWithdraw reject MPT without CanTransfer");
         using namespace jtx;
         using namespace loanBroker;
 
@@ -5580,7 +5580,7 @@ protected:
 
         env.close();
 
-        PrettyAsset const asset = mpt["BUG"];
+        PrettyAsset const asset = mpt["MPT"];
         mpt.authorize({.account = alice});
         env.close();
 
@@ -5614,21 +5614,58 @@ protected:
         env(pay(alice, pseudoAccount, asset(1)), ter(tecNO_AUTH));
         env.close();
 
+        // Cover cannot be transferred to broker account
         auto const depositAmount = asset(1);
-        env(coverDeposit(alice, brokerKeylet.key, depositAmount));
-        BEAST_EXPECT(env.ter() == tesSUCCESS);
+        env(coverDeposit(alice, brokerKeylet.key, depositAmount),
+            ter{tecNO_AUTH});
         env.close();
 
         if (auto const refreshed = env.le(brokerKeylet);
             BEAST_EXPECT(refreshed))
         {
-            // with an MPT that cannot be transferred the covrAvailable should
-            // remain zero
             BEAST_EXPECT(refreshed->at(sfCoverAvailable) == 0);
+            env.require(balance(pseudoAccount, asset(0)));
+        }
+
+        // Set CanTransfer again and transfer some deposit
+        mpt.set({.mutableFlags = tmfMPTSetCanTransfer});
+        env.close();
+
+        env(coverDeposit(alice, brokerKeylet.key, depositAmount));
+        env.close();
+
+        if (auto const refreshed = env.le(brokerKeylet);
+            BEAST_EXPECT(refreshed))
+        {
+            BEAST_EXPECT(refreshed->at(sfCoverAvailable) == 1);
             env.require(balance(pseudoAccount, depositAmount));
         }
+
+        // Remove CanTransfer after the deposit
+        mpt.set({.mutableFlags = tmfMPTClearCanTransfer});
+        env.close();
+
+        // Cover cannot be transferred from broker account
+        env(coverWithdraw(alice, brokerKeylet.key, depositAmount),
+            ter{tecNO_AUTH});
+        env.close();
+
+        // Set CanTransfer again and withdraw
+        mpt.set({.mutableFlags = tmfMPTSetCanTransfer});
+        env.close();
+
+        env(coverWithdraw(alice, brokerKeylet.key, depositAmount));
+        env.close();
+
+        if (auto const refreshed = env.le(brokerKeylet);
+            BEAST_EXPECT(refreshed))
+        {
+            BEAST_EXPECT(refreshed->at(sfCoverAvailable) == 0);
+            env.require(balance(pseudoAccount, asset(0)));
+        }
     }
 
+#if LOANTODO
     void
     testLoanPayLateFullPaymentBypassesPenalties()
     {
@@ -6504,11 +6541,11 @@ public:
     run() override
     {
 #if LOANTODO
-        testCoverDepositAllowsNonTransferableMPT();
         testLoanPayLateFullPaymentBypassesPenalties();
         testPoC_UnsignedUnderflowOnFullPayAfterEarlyPeriodic();
         testLoanCoverMinimumRoundingExploit();
 #endif
+        testCoverDepositWithdrawNonTransferableMPT();
         testDustManipulation();
 
         testIssuerLoan();

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

@@ -58,13 +58,10 @@ LoanBrokerCoverDeposit::preclaim(PreclaimContext const& ctx)
         return tecWRONG_ASSET;
 
     auto const pseudoAccountID = sleBroker->at(sfAccount);
-    if (auto ter = canTransfer(ctx.view, vaultAsset, account, pseudoAccountID);
-        !isTesSuccess(ter))
-    {
-        JLOG(ctx.j.warn()) << "Assets are non-transferable.";
-        return ter;
-    }
-
+    // Cannot transfer a non-transferable Asset
+    if (auto const ret =
+            canTransfer(ctx.view, vaultAsset, account, pseudoAccountID))
+        return ret;
     // Cannot transfer a frozen Asset
     if (auto const ret = checkFrozen(ctx.view, account, vaultAsset))
         return ret;

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

@@ -67,12 +67,10 @@ LoanBrokerCoverWithdraw::preclaim(PreclaimContext const& ctx)
 
     // The broker's pseudo-account is the source of funds.
     auto const pseudoAccountID = sleBroker->at(sfAccount);
-    if (auto ter = canTransfer(ctx.view, vaultAsset, pseudoAccountID, dstAcct);
-        !isTesSuccess(ter))
-    {
-        JLOG(ctx.j.warn()) << "Assets are non-transferable.";
-        return ter;
-    }
+    // Cannot transfer a non-transferable Asset
+    if (auto const ret =
+            canTransfer(ctx.view, vaultAsset, pseudoAccountID, dstAcct))
+        return ret;
 
     // Withdrawal to a 3rd party destination account is essentially a transfer.
     // Enforce all the usual asset transfer checks.