From fce9ff6bc562d9911c006b5bd1a4444d12de3092 Mon Sep 17 00:00:00 2001
From: Timur Ialymov <yalymov.tm@gmail.com>
Date: Thu, 30 Jul 2026 15:46:46 +0100
Subject: [PATCH] fix: Add amendment-gated VaultInvariant check that
 lossUnrealized is non-negative

The VaultInvariant validated the upper bound on the vault's sfLossUnrealized
field but never the lower bound. A negative lossUnrealized would pass the
invariant and cause the withdraw math (assetTotal -= lossUnrealized) to use an
effective asset total greater than assetsTotal, letting withdrawers extract more
assets than the vault tracks.

Add a defense-in-depth check that lossUnrealized must not be negative. Because
featureSingleAssetVault is already released, the check is gated behind
fixCleanup3_4_0 to avoid introducing a new transaction failure path that could
fork the network.

The test uses ttLOAN_MANAGE (permitted to change lossUnrealized, isolating the
check) plus a paired case with the amendment disabled asserting the transaction
still succeeds.

Ref: FN-32.
---
 src/libxrpl/tx/invariants/VaultInvariant.cpp |  6 ++++
 src/test/app/Invariants_test.cpp             | 35 ++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/src/libxrpl/tx/invariants/VaultInvariant.cpp b/src/libxrpl/tx/invariants/VaultInvariant.cpp
index a9ba0ec8746..eca50eb8093 100644
--- a/src/libxrpl/tx/invariants/VaultInvariant.cpp
+++ b/src/libxrpl/tx/invariants/VaultInvariant.cpp
@@ -483,6 +483,12 @@ ValidVault::finalize(
         result = false;
     }
 
+    if (view.rules().enabled(fixCleanup3_4_0) && afterVault.lossUnrealized < kZero)
+    {
+        JLOG(j.fatal()) << "Invariant failed: loss unrealized must not be negative";
+        result = false;
+    }
+
     if (afterVault.assetsTotal < kZero)
     {
         JLOG(j.fatal()) << "Invariant failed: assets outstanding must be positive";
diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp
index eaf1f2704c8..ac6d8e068fa 100644
--- a/src/test/app/Invariants_test.cpp
+++ b/src/test/app/Invariants_test.cpp
@@ -3374,6 +3374,41 @@ class Invariants_test : public beast::unit_test::Suite
             precloseXrp,
             TxAccount::A2);
 
+        // A negative loss unrealized must trip the invariant. ttLOAN_MANAGE is
+        // allowed to change loss unrealized, so it isolates this check from the
+        // "must not change loss unrealized" invariant. Gated behind
+        // fixCleanup3_4_0 (see below).
+        doInvariantCheck(
+            {"loss unrealized must not be negative"},
+            [&](Account const& a1, Account const& a2, ApplyContext& ac) {
+                auto const keylet = keylet::vault(a1.id(), ac.view().seq());
+                return kAdjust(ac.view(), keylet, kArgs(a2.id(), 0, [&](Adjustments& sample) {
+                                   sample.lossUnrealized = -1;
+                               }));
+            },
+            XRPAmount{},
+            STTx{ttLOAN_MANAGE, [](STObject& tx) {}},
+            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            precloseXrp,
+            TxAccount::A2);
+
+        // Without fixCleanup3_4_0 the same state must NOT trip the invariant,
+        // preserving pre-amendment behavior (no fork risk).
+        doInvariantCheck(
+            makeEnv(defaultAmendments() - fixCleanup3_4_0),
+            {},
+            [&](Account const& a1, Account const& a2, ApplyContext& ac) {
+                auto const keylet = keylet::vault(a1.id(), ac.view().seq());
+                return kAdjust(ac.view(), keylet, kArgs(a2.id(), 0, [&](Adjustments& sample) {
+                                   sample.lossUnrealized = -1;
+                               }));
+            },
+            XRPAmount{},
+            STTx{ttLOAN_MANAGE, [](STObject& tx) {}},
+            {tesSUCCESS, tesSUCCESS},
+            precloseXrp,
+            TxAccount::A2);
+
         doInvariantCheck(
             {"set assets outstanding must not exceed assets maximum"},
             [&](Account const& a1, Account const& a2, ApplyContext& ac) {