Clean ups

Author: ximinez

src/test/app/LoanBroker_test.cpp

@@ -96,6 +96,7 @@ class LoanBroker_test : public beast::unit_test::suite
 
     void
     lifecycle(
+        const char* label,
         jtx::Env& env,
         jtx::Account const& alice,
         jtx::Account const& evan,
@@ -106,7 +107,15 @@ class LoanBroker_test : public beast::unit_test::suite
         std::function<void(SLE::const_ref)> checkChangedBroker)
     {
         auto const keylet = keylet::loanbroker(alice.id(), env.seq(alice));
-        testcase("Lifecycle: " + to_string(vault.asset));
+        {
+            auto const& asset = vault.asset.raw();
+            testcase << "Lifecycle: "
+                     << (asset.native()                ? "XRP "
+                             : asset.holds<Issue>()    ? "IOU "
+                             : asset.holds<MPTIssue>() ? "MPT "
+                                                       : "Unknown ")
+                     << label;
+        }
 
         using namespace jtx;
         using namespace loanBroker;
@@ -124,7 +133,8 @@ class LoanBroker_test : public beast::unit_test::suite
         env.close();
         if (auto broker = env.le(keylet); BEAST_EXPECT(broker))
         {
-            log << to_string(broker->getJson()) << std::endl;
+            // log << "Broker after create: " << to_string(broker->getJson())
+            //     << std::endl;
             BEAST_EXPECT(broker->at(sfVaultID) == vault.vaultID);
             BEAST_EXPECT(broker->at(sfAccount) != alice.id());
             BEAST_EXPECT(broker->at(sfOwner) == alice.id());
@@ -136,10 +146,18 @@ class LoanBroker_test : public beast::unit_test::suite
             if (checkBroker)
                 checkBroker(broker);
 
-            // Load the pseudo-account
+            // if (auto const vaultSLE = env.le(keylet::vault(vault.vaultID)))
+            //{
+            //     log << "Vault: " << to_string(vaultSLE->getJson()) <<
+            //     std::endl;
+            // }
+            //  Load the pseudo-account
             auto const pseudoKeylet = keylet::account(broker->at(sfAccount));
             if (auto const pseudo = env.le(pseudoKeylet); BEAST_EXPECT(pseudo))
             {
+                // log << "Pseudo-account after create: "
+                //     << to_string(pseudo->getJson()) << std::endl
+                //     << std::endl;
                 BEAST_EXPECT(
                     pseudo->at(sfFlags) ==
                     (lsfDisableMaster | lsfDefaultRipple | lsfDepositAuth));
@@ -201,6 +219,15 @@ class LoanBroker_test : public beast::unit_test::suite
             env(del(evan, keylet.key), ter(tecNO_PERMISSION));
             // TODO: test deletion with an active loan
             // delete the broker
+            // log << "Broker before delete: " << to_string(broker->getJson())
+            //    << std::endl;
+            // if (auto const pseudo = env.le(pseudoKeylet);
+            // BEAST_EXPECT(pseudo))
+            //{
+            //    log << "Pseudo-account before delete: "
+            //        << to_string(pseudo->getJson()) << std::endl
+            //        << std::endl;
+            //}
             env(del(alice, keylet.key));
             env.close();
             {
@@ -215,7 +242,7 @@ class LoanBroker_test : public beast::unit_test::suite
     void
     testLifecycle()
     {
-        testcase("Create and update");
+        testcase("Lifecycle");
         using namespace jtx;
 
         // Create 3 loan brokers: one for XRP, one for an IOU, and one for an
@@ -346,6 +373,7 @@ class LoanBroker_test : public beast::unit_test::suite
 
             std::string testData;
             lifecycle(
+                "default fields",
                 env,
                 alice,
                 evan,
@@ -426,6 +454,7 @@ class LoanBroker_test : public beast::unit_test::suite
                 });
 
             lifecycle(
+                "non-default fields",
                 env,
                 alice,
                 evan,

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

@@ -91,28 +91,27 @@ TER
 LoanBrokerDelete::doApply()
 {
     auto const& tx = ctx_.tx;
-    auto& view = ctx_.view();
 
     auto const brokerID = tx[sfLoanBrokerID];
 
     // Delete the loan broker
-    auto broker = view.peek(keylet::loanbroker(brokerID));
+    auto broker = view().peek(keylet::loanbroker(brokerID));
     auto const vaultID = broker->at(sfVaultID);
-    auto const sleVault = view.read(keylet::vault(vaultID));
+    auto const sleVault = view().read(keylet::vault(vaultID));
     auto const vaultPseudoID = sleVault->at(sfAccount);
     auto const vaultAsset = sleVault->at(sfAsset);
 
     auto const brokerPseudoID = broker->at(sfAccount);
 
-    if (!view.dirRemove(
+    if (!view().dirRemove(
             keylet::ownerDir(account_),
             broker->at(sfOwnerNode),
             broker->key(),
             false))
     {
         return tefBAD_LEDGER;
     }
-    if (!view.dirRemove(
+    if (!view().dirRemove(
             keylet::ownerDir(vaultPseudoID),
             broker->at(sfVaultNode),
             broker->key(),
@@ -125,7 +124,7 @@ LoanBrokerDelete::doApply()
         auto const coverAvailable =
             STAmount{vaultAsset, broker->at(sfCoverAvailable)};
         if (auto const ter = accountSend(
-                view,
+                view(),
                 brokerPseudoID,
                 account_,
                 coverAvailable,
@@ -134,12 +133,15 @@ LoanBrokerDelete::doApply()
             return ter;
     }
 
-    auto brokerPseudoSLE = view.peek(keylet::account(brokerPseudoID));
+    if (auto ter = removeEmptyHolding(view(), brokerPseudoID, vaultAsset, j_))
+        return ter;
+
+    auto brokerPseudoSLE = view().peek(keylet::account(brokerPseudoID));
     if (!brokerPseudoSLE)
         return tefBAD_LEDGER;
 
-    // Making the payment should have deleted any obligations
-    // associated with the broker or broker pseudo-account.
+    // Making the payment and removing the empty holding should have deleted any
+    // obligations associated with the broker or broker pseudo-account.
     if (*brokerPseudoSLE->at(sfBalance))
     {
         JLOG(j_.warn()) << "LoanBrokerDelete: Pseudo-account has a balance";
@@ -152,15 +154,15 @@ LoanBrokerDelete::doApply()
         return tecHAS_OBLIGATIONS;
     }
     if (auto const directory = keylet::ownerDir(brokerPseudoID);
-        view.read(directory))
+        view().read(directory))
     {
         JLOG(j_.warn()) << "LoanBrokerDelete: Pseudo-account has a directory";
         return tecHAS_OBLIGATIONS;
     }
 
-    view.erase(brokerPseudoSLE);
+    view().erase(brokerPseudoSLE);
 
-    view.erase(broker);
+    view().erase(broker);
 
     return tesSUCCESS;
 }