From 4b9f6bfafe0af93813245ba644b3b128ef13878c Mon Sep 17 00:00:00 2001
From: Bronek Kozicki <brok@incorrekt.com>
Date: Mon, 10 Nov 2025 15:09:08 +0000
Subject: [PATCH] Add unit test

---
 src/test/app/Loan_test.cpp                    | 55 ++++++++++++++++---
 .../app/tx/detail/LoanBrokerCoverDeposit.cpp  | 11 ++--
 .../app/tx/detail/LoanBrokerCoverWithdraw.cpp | 10 ++--
 3 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp
index 57da3e28e7..50d65fc196 100644
--- a/src/test/app/Loan_test.cpp
+++ b/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();
diff --git a/src/xrpld/app/tx/detail/LoanBrokerCoverDeposit.cpp b/src/xrpld/app/tx/detail/LoanBrokerCoverDeposit.cpp
index d4db596ad1..974bb36a93 100644
--- a/src/xrpld/app/tx/detail/LoanBrokerCoverDeposit.cpp
+++ b/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;
diff --git a/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp b/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp
index 2ad7be5cda..023299dfd1 100644
--- a/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp
+++ b/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.