From 7d6155cc99411af6b728a5a049ff92d65068656e Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Mon, 14 Apr 2025 15:46:55 -0400 Subject: [PATCH] Clean ups --- src/test/app/LoanBroker_test.cpp | 37 +++++++++++++++++--- src/xrpld/app/tx/detail/LoanBrokerDelete.cpp | 26 +++++++------- 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 45eca50514..8f6987a81f 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/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 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() ? "IOU " + : asset.holds() ? "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, diff --git a/src/xrpld/app/tx/detail/LoanBrokerDelete.cpp b/src/xrpld/app/tx/detail/LoanBrokerDelete.cpp index ed0871aaf5..e1dcebf3e9 100644 --- a/src/xrpld/app/tx/detail/LoanBrokerDelete.cpp +++ b/src/xrpld/app/tx/detail/LoanBrokerDelete.cpp @@ -91,20 +91,19 @@ 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(), @@ -112,7 +111,7 @@ LoanBrokerDelete::doApply() { 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; }