Improve coverage

src/libxrpl/tx/transactors/lending/LoanAccept.cpp

@@ -80,8 +80,10 @@ LoanAccept::preclaim(PreclaimContext const& ctx)
     auto const brokerSle = ctx.view.read(keylet::loanBroker(loanSle->at(sfLoanBrokerID)));
     if (!brokerSle)
     {
+        // LCOV_EXCL_START
         JLOG(ctx.j.fatal()) << "LoanAccept: LoanBroker does not exist.";
-        return tefBAD_LEDGER;  // LCOV_EXCL_LINE
+        return tefBAD_LEDGER;
+        // LCOV_EXCL_STOP
     }
     auto const brokerOwner = brokerSle->at(sfOwner);
     auto const brokerPseudo = brokerSle->at(sfAccount);
@@ -89,8 +91,10 @@ LoanAccept::preclaim(PreclaimContext const& ctx)
     auto const vaultSle = ctx.view.read(keylet::vault(brokerSle->at(sfVaultID)));
     if (!vaultSle)
     {
+        // LCOV_EXCL_START
         JLOG(ctx.j.fatal()) << "LoanAccept: Vault does not exist.";
-        return tefBAD_LEDGER;  // LCOV_EXCL_LINE
+        return tefBAD_LEDGER;
+        // LCOV_EXCL_STOP
     }
     Asset const asset = vaultSle->at(sfAsset);
     auto const vaultPseudo = vaultSle->at(sfAccount);
@@ -210,7 +214,7 @@ LoanAccept::doApply()
 
     // 3.9.4.8 Make the borrower the owner of the loan.
     if (auto const ter = dirLink(view, borrower, loanSle, sfOwnerNode))
-        return ter;
+        return ter;  // LCOV_EXCL_LINE
     view.update(loanSle);
 
     associateAsset(*loanSle, vaultAsset);

src/libxrpl/tx/transactors/lending/LoanSet.cpp

@@ -477,7 +477,7 @@ applyPendingLoan(
     // Link the loan into the broker's directory. The borrower directory link is
     // deferred to LoanAccept for the two-step (pending) flow.
     if (auto const ter = dirLink(view, brokerPseudo, loan, sfLoanBrokerNode))
-        return ter;
+        return ter;  // LCOV_EXCL_LINE
 
     associateAsset(*vaultSle, vaultAsset);
     associateAsset(*brokerSle, vaultAsset);
@@ -583,10 +583,10 @@ applyImmediateLoan(
     // Link the loan into the broker's directory, then make the borrower the
     // owner of the loan by linking it into the borrower's directory.
     if (auto const ter = dirLink(view, brokerPseudo, loan, sfLoanBrokerNode))
-        return ter;
+        return ter;  // LCOV_EXCL_LINE
 
     if (auto const ter = dirLink(view, plan.borrower, loan, sfOwnerNode))
-        return ter;
+        return ter;  // LCOV_EXCL_LINE
 
     associateAsset(*vaultSle, vaultAsset);
     associateAsset(*brokerSle, vaultAsset);

src/test/app/Invariants_test.cpp

@@ -2927,6 +2927,7 @@ class Invariants_test : public beast::unit_test::Suite
             std::optional<int> assetsAvailable = std::nullopt;
             std::optional<int> lossUnrealized = std::nullopt;
             std::optional<int> assetsMaximum = std::nullopt;
+            std::optional<int> assetsReserved = std::nullopt;
             std::optional<int> sharesTotal = std::nullopt;
             std::optional<int> vaultAssets = std::nullopt;
             std::optional<AccountAmount> accountAssets = std::nullopt;
@@ -2948,6 +2949,8 @@ class Invariants_test : public beast::unit_test::Suite
                 (*sleVault)[sfLossUnrealized] = *args.lossUnrealized;
             if (args.assetsMaximum)
                 (*sleVault)[sfAssetsMaximum] = *args.assetsMaximum;
+            if (args.assetsReserved)
+                (*sleVault)[sfAssetsReserved] = *args.assetsReserved;
 
             // Remaining fields are adjusted in terms of difference
             if (args.assetsTotal)
@@ -3585,6 +3588,20 @@ class Invariants_test : public beast::unit_test::Suite
             precloseXrp,
             TxAccount::A2);
 
+        doInvariantCheck(
+            {"assets reserved must be positive or zero"},
+            [&](Account const& a1, Account const& a2, ApplyContext& ac) {
+                auto const keylet = keylet::vault(a1.id(), SeqProxy::rawSequence(ac.view().seq()));
+                return kAdjust(ac.view(), keylet, kArgs(a2.id(), 0, [&](Adjustments& sample) {
+                                   sample.assetsReserved = -1;
+                               }));
+            },
+            XRPAmount{},
+            STTx{ttVAULT_SET, [](STObject& tx) {}},
+            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            precloseXrp,
+            TxAccount::A2);
+
         doInvariantCheck(
             {"set must not change shares outstanding",
              "updated zero sized vault must have no assets outstanding",

src/test/app/lending/LoanRounding_test.cpp

@@ -15,8 +15,12 @@
 
 #include <xrpl/basics/Number.h>
 #include <xrpl/beast/unit_test/suite.h>
+#include <xrpl/beast/utility/Journal.h>
 #include <xrpl/beast/utility/Zero.h>
 #include <xrpl/json/json_value.h>
+#include <xrpl/ledger/ApplyView.h>
+#include <xrpl/ledger/OpenView.h>
+#include <xrpl/ledger/Sandbox.h>
 #include <xrpl/ledger/helpers/LendingHelpers.h>
 #include <xrpl/protocol/Asset.h>
 #include <xrpl/protocol/Feature.h>
@@ -959,6 +963,129 @@ private:
         env.close();
     }
 
+    // LoanDelete::deleteActiveLoan clears any sub-scale residual left on
+    // LoanBroker.DebtTotal when the last active loan is removed. In production
+    // the residual comes from cross-loan rounding when multiple loans on the
+    // same broker operate at significantly different scales (see the comment
+    // above the adjustImpreciseNumber call in LoanPay.cpp's doApply). Building
+    // that accumulation deterministically from real txs is fragile, so this
+    // test installs a sub-drop residual directly on the broker SLE via
+    // OpenLedger::modify — the same lower-layer edit LoanTwoStep_test's
+    // makeVaultAccrual uses to force VaultVersion::Legacy — and then submits
+    // the LoanDelete against the mutated open view. LoanBrokerInvariant only
+    // forbids negative DebtTotal, so a positive sub-scale value is
+    // invariant-safe; the residual (5e-8 drops) rounds toward zero to 0 drops
+    // so the XRPL_ASSERT_PARTS guarding the branch also holds.
+    void
+    testDeleteLastLoanClearsDebtDust()
+    {
+        testcase("coverage: LoanDelete clears sub-scale DebtTotal dust on last loan");
+
+        using namespace jtx;
+        using namespace loan;
+
+        Account const issuer{"issuer"};
+        Account const lender{"lender"};
+        Account const borrower{"borrower"};
+
+        Env env(*this, all_);
+        env.fund(XRP(1'000'000), issuer, lender, borrower);
+        env.close();
+
+        // scale = 1 keeps xrpAsset(N) at N drops so the tiny residual
+        // installed below is unambiguously sub-drop.
+        PrettyAsset const xrpAsset{xrpIssue(), 1};
+
+        // 0% interest so origination and payoff cancel to exactly zero on
+        // DebtTotal; the residual we test is installed by hand below.
+        BrokerParameters const brokerParams{
+            .vaultDeposit = 100'000,
+            .debtMax = 10'000,
+            .coverRateMin = TenthBips32{0},
+            .coverDeposit = 0,
+            .managementFeeRate = TenthBips16{0},
+            .coverRateLiquidation = TenthBips32{0}};
+        BrokerInfo const broker{createVaultAndBroker(env, xrpAsset, lender, brokerParams)};
+
+        auto const sleBroker0 = env.le(broker.brokerKeylet());
+        if (!BEAST_EXPECT(sleBroker0))
+            return;
+        auto const loanKeylet =
+            keylet::loan(broker.brokerID, SeqProxy::rawSequence(sleBroker0->at(sfLoanSequence)));
+
+        // Active loan (immediate flow), single payment, 0% interest.
+        env(set(borrower, broker.brokerID, xrpAsset(100).value()),
+            Sig(sfCounterpartySignature, lender),
+            kInterestRate(TenthBips32{0}),
+            kPaymentTotal(1),
+            kPaymentInterval(3600),
+            Fee(env.current()->fees().base * 2));
+        env.close();
+
+        // Fully pay off; 0% interest means the actual debit is exactly the
+        // principal, so DebtTotal returns cleanly to zero.
+        env(pay(borrower, loanKeylet.key, xrpAsset(200).value(), tfLoanFullPayment));
+        env.close();
+
+        // Baseline: DebtTotal is exactly zero, the broker still owns the
+        // (now fully-paid) loan, and PaymentRemaining is zero so LoanDelete
+        // will not trip tecHAS_OBLIGATIONS.
+        if (auto const b = env.le(broker.brokerKeylet()); BEAST_EXPECT(b))
+        {
+            BEAST_EXPECT(b->at(sfDebtTotal) == beast::kZero);
+            BEAST_EXPECT(b->at(sfOwnerCount) == 1);
+        }
+        if (auto const l = env.le(loanKeylet); BEAST_EXPECT(l))
+            BEAST_EXPECT(l->at(sfPaymentRemaining) == 0);
+
+        // Install a sub-drop residual on the broker's DebtTotal directly on
+        // the open ledger. Not closing after: OpenLedger::accept rebuilds
+        // the open view from the last-closed ledger and re-applies pending
+        // txs, discarding raw mutations, so every post-condition below is
+        // read from the open view.
+        Number const kResidual{5, -8};
+        auto const mutated =
+            env.app().getOpenLedger().modify([&](OpenView& view, beast::Journal) -> bool {
+                Sandbox sb(&view, TapNone);
+                auto b = sb.peek(broker.brokerKeylet());
+                if (!b)
+                    return false;
+                b->at(sfDebtTotal) = kResidual;
+                sb.update(b);
+                sb.apply(view);
+                return true;
+            });
+        if (!BEAST_EXPECT(mutated))
+            return;
+
+        // Sanity: the residual is visible on the open view and rounds to
+        // zero at the vault's asset scale (which is what the branch's
+        // XRPL_ASSERT_PARTS requires).
+        if (auto const b = env.le(broker.brokerKeylet()); BEAST_EXPECT(b))
+            BEAST_EXPECT(b->at(sfDebtTotal) == kResidual);
+        if (auto const v = env.le(broker.vaultKeylet()); BEAST_EXPECT(v))
+        {
+            BEAST_EXPECT(
+                roundToAsset(
+                    v->at(sfAsset),
+                    Number{kResidual},
+                    getAssetsTotalScale(v),
+                    Number::RoundingMode::TowardsZero) == beast::kZero);
+        }
+
+        // Delete against the mutated open view. The last-loan branch of
+        // deleteActiveLoan fires: DebtTotal is zeroed, OwnerCount goes to
+        // zero, and the loan SLE is erased.
+        env(del(lender, loanKeylet.key));
+
+        if (auto const b = env.le(broker.brokerKeylet()); BEAST_EXPECT(b))
+        {
+            BEAST_EXPECT(b->at(sfDebtTotal) == beast::kZero);
+            BEAST_EXPECT(b->at(sfOwnerCount) == 0);
+        }
+        BEAST_EXPECT(!env.le(loanKeylet));
+    }
+
     void
     runAmendmentIndependent()
     {
@@ -967,6 +1094,7 @@ private:
         testBugOverpaymentPrincipalChange();
         testBugOverpayUnroundedAmount();
         testBugInterestDueDeltaCrash();
+        testDeleteLastLoanClearsDebtDust();
     }
 
     // Tests run under each entry in amendmentCombinations().

src/test/app/lending/LoanTwoStep_test.cpp

@@ -1857,6 +1857,89 @@ private:
             expectStillPending(env, loanKeylet);
         }
 
+        // The preceding test advances the ledger clock past both StartDate
+        // and RedemptionDate, so LoanAccept::preclaim's StartDate expiry
+        // check fires first and the vault-phase branch itself is never
+        // exercised. The next two cases isolate the phase gate by rewriting
+        // the vault's SubscriptionDate / RedemptionDate on the open ledger
+        // (bypassing the normally-immutable-field invariant the same way
+        // makeVaultAccrual does for sfLEVersion) while leaving the loan's
+        // StartDate comfortably in the future.
+        for (auto const scenario : {VaultPhase::Subscription, VaultPhase::Redemption})
+        {
+            char const* const phaseName =
+                scenario == VaultPhase::Subscription ? "Subscription" : "Redemption";
+            TER const expected =
+                scenario == VaultPhase::Subscription ? TER{tecTOO_SOON} : TER{tecEXPIRED};
+            testcase << "Two-step: LoanAccept rejected during " << phaseName
+                     << " (StartDate not yet expired)";
+
+            Env env(*this, features);
+            env.fund(XRP(100'000'000), noripple(lender));
+            env.fund(XRP(1'000'000), borrower);
+            env.close();
+
+            BrokerParameters params{};
+            params.vaultKind = VaultKind::ClosedEnded;
+            params.subscriptionOffset = 60;
+            // Generous so LoanSet's finalPayment < RedemptionDate guard passes.
+            params.redemptionOffset = 10u * 365u * 24u * 60u * 60u;
+            auto const asset = createAsset(env, AssetType::XRP, params, issuer, lender, borrower);
+            auto const broker = createVaultAndBroker(env, asset, lender, params);
+
+            // Propose while the vault is in Investment. StartDate is 1h out
+            // so the StartDate expiry check does not fire before the phase
+            // check, no matter which phase the mutation forces below.
+            auto const loanKeylet = nextLoanKeylet(env, broker);
+            std::uint32_t const startDate = (env.now() + 1h).time_since_epoch().count();
+            propose(env, broker, lender, borrower, startDate);
+            env.close();
+            expectStillPending(env, loanKeylet);
+
+            // Force the vault into the target phase by rewriting the
+            // relevant date on the open ledger. Not closing between the
+            // mutation and the LoanAccept: OpenLedger::accept rebuilds the
+            // open view from the last-closed ledger and re-applies pending
+            // txs, discarding raw mutations.
+            std::uint32_t const parentClose =
+                env.current()->parentCloseTime().time_since_epoch().count();
+            auto const changed =
+                env.app().getOpenLedger().modify([&](OpenView& view, beast::Journal) -> bool {
+                    Sandbox sb(&view, TapNone);
+                    auto v = sb.peek(broker.vaultKeylet());
+                    if (!v)
+                        return false;
+                    if (scenario == VaultPhase::Subscription)
+                    {
+                        // parentClose < SubscriptionDate → Subscription.
+                        // Sit strictly below StartDate so nothing else shifts.
+                        v->setFieldU32(sfSubscriptionDate, parentClose + 600);
+                    }
+                    else
+                    {
+                        // RedemptionDate < parentClose → Redemption.
+                        // SubscriptionDate is already <= parentClose from
+                        // createVaultAndBroker's phase advance.
+                        v->setFieldU32(sfRedemptionDate, parentClose - 1);
+                    }
+                    sb.update(v);
+                    sb.apply(view);
+                    return true;
+                });
+            if (!BEAST_EXPECT(changed))
+                continue;
+
+            // Sanity: the open-ledger view now reports the intended phase,
+            // and StartDate is still in the future so the phase gate — not
+            // the StartDate expiry check — is what will trip.
+            if (auto const v = env.le(broker.vaultKeylet()); BEAST_EXPECT(v))
+                BEAST_EXPECT(getVaultPhase(*env.current(), v) == scenario);
+            BEAST_EXPECT(parentClose < startDate);
+
+            env(accept(borrower, loanKeylet.key), Ter(expected));
+            expectStillPending(env, loanKeylet);
+        }
+
         {
             testcase("Two-step: pending loan bounds cover clawback, LoanAccept still succeeds");
 

src/test/app/vault/VaultValidation_test.cpp

@@ -17,9 +17,13 @@
 #include <xrpl/basics/base_uint.h>
 #include <xrpl/basics/strHex.h>
 #include <xrpl/beast/unit_test/suite.h>
+#include <xrpl/beast/utility/Journal.h>
 #include <xrpl/beast/utility/Zero.h>
 #include <xrpl/json/json_forwards.h>
 #include <xrpl/json/json_value.h>
+#include <xrpl/ledger/ApplyView.h>
+#include <xrpl/ledger/OpenView.h>
+#include <xrpl/ledger/Sandbox.h>
 #include <xrpl/protocol/Asset.h>
 #include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/Indexes.h>
@@ -1068,6 +1072,86 @@ private:
         }
     }
 
+    // Covers the third obligation gate in VaultDelete::preclaim
+    // (sfAssetsReserved != 0). The first two guards (sfAssetsAvailable and
+    // sfAssetsTotal) short-circuit on any real-world path that inflates
+    // sfAssetsReserved — the only production writer is the two-step LoanSet
+    // pending-loan bookkeeping, which simultaneously moves the same amount
+    // out of sfAssetsAvailable, so the first check always fires first.
+    // Reproducing the (Available == 0, Total == 0, Reserved != 0)
+    // combination from real txs is not possible, so this test installs the
+    // residual directly on the vault SLE via OpenLedger::modify (the same
+    // lower-layer edit VaultShares_test uses to tamper with token fields)
+    // and confirms preclaim rejects the delete with tecHAS_OBLIGATIONS.
+    void
+    testVaultDeleteAssetsReservedBlocks()
+    {
+        testcase("VaultDelete rejected when only AssetsReserved is non-zero");
+
+        using namespace test::jtx;
+
+        Env env{*this};
+        Account const owner{"owner"};
+        env.fund(XRP(1'000'000), owner);
+        env.close();
+
+        Vault const vault{env};
+        PrettyAsset const xrpAsset = xrpIssue();
+        auto const [tx, keylet] = vault.create({.owner = owner, .asset = xrpAsset});
+        env(tx, Ter(tesSUCCESS));
+        env.close();
+
+        // Baseline: a freshly-created empty vault has all three buckets at
+        // zero, so without the mutation below VaultDelete would succeed.
+        if (auto const v = env.le(keylet); BEAST_EXPECT(v))
+        {
+            BEAST_EXPECT(v->at(sfAssetsAvailable) == beast::kZero);
+            BEAST_EXPECT(v->at(sfAssetsTotal) == beast::kZero);
+            BEAST_EXPECT(v->at(sfAssetsReserved) == beast::kZero);
+        }
+
+        // Install a non-zero sfAssetsReserved directly on the vault SLE.
+        // ValidVault only inspects vault accounting when a tx mutates the
+        // vault; the raw edit happens outside the tx machinery so no
+        // invariant fires. VaultDelete below rejects at preclaim, so it
+        // never modifies the vault and invariants stay silent for the tx
+        // too.
+        Number const kReserved{1'000};
+        auto const mutated =
+            env.app().getOpenLedger().modify([&](OpenView& view, beast::Journal) -> bool {
+                Sandbox sb(&view, TapNone);
+                auto v = sb.peek(keylet);
+                if (!v)
+                    return false;
+                v->at(sfAssetsReserved) = kReserved;
+                sb.update(v);
+                sb.apply(view);
+                return true;
+            });
+        if (!BEAST_EXPECT(mutated))
+            return;
+
+        // Sanity: the residual is visible and the two preceding guards
+        // (Available, Total) still resolve to zero, so preclaim's third
+        // check is the one that fires.
+        if (auto const v = env.le(keylet); BEAST_EXPECT(v))
+        {
+            BEAST_EXPECT(v->at(sfAssetsAvailable) == beast::kZero);
+            BEAST_EXPECT(v->at(sfAssetsTotal) == beast::kZero);
+            BEAST_EXPECT(v->at(sfAssetsReserved) == kReserved);
+        }
+
+        // Delete against the mutated open view. Not closing after: on
+        // close, OpenLedger::accept rebuilds the open view from the
+        // last-closed ledger and re-applies pending txs, discarding raw
+        // mutations, so the post-condition is read from the open view.
+        env(vault.del({.owner = owner, .id = keylet.key}), Ter(tecHAS_OBLIGATIONS));
+
+        // Preclaim rejected the delete, so the fee was charged but the
+        // vault SLE is untouched.
+        BEAST_EXPECT(env.le(keylet) != nullptr);
+    }
+
 public:
     void
     run() override
@@ -1077,6 +1161,7 @@ public:
         testCreateFailIOU();
         testCreateFailMPT();
         testVaultDeleteMemoData();
+        testVaultDeleteAssetsReservedBlocks();
         testVaultCreateLEVersion();
     }
 };