From db50afa34f5faac900d89e8cf54ce8cfaf43bd71 Mon Sep 17 00:00:00 2001
From: Vito <5780819+Tapanito@users.noreply.github.com>
Date: Mon, 10 Aug 2026 16:34:36 +0200
Subject: [PATCH] test: Cover Vault helper early-return branches

Adds unit-test coverage for three previously-untested branches in
VaultHelpers:

- addVaultAssets propagates a non-tes return when the underlying
  accountSend fails (exercised via a sender with no trust line for the
  vault asset).
- clawbackVaultAssets propagates a non-tes return when its accountSend
  fails (exercised via a synthetic third-party recipient with no trust
  line; the production caller always uses the asset issuer, which
  cannot hit this branch).
- moveVaultAssets short-circuits and returns tesSUCCESS without
  touching accountSendMulti when every recipient's amount is zero
  (exercised via two zero-amount recipients, which still satisfies the
  recipients.size() > 1 precondition).
---
 src/test/app/VaultHelpers_test.cpp | 61 ++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/src/test/app/VaultHelpers_test.cpp b/src/test/app/VaultHelpers_test.cpp
index 96c6d9aaed..19c41d9f87 100644
--- a/src/test/app/VaultHelpers_test.cpp
+++ b/src/test/app/VaultHelpers_test.cpp
@@ -142,6 +142,19 @@ class VaultHelpers_test : public beast::unit_test::Suite
             BEAST_EXPECT(totalAfter == totalBeforeNegative - 10);
             BEAST_EXPECT(availableAfter == availableBeforeNegative);
         }
+
+        // If the underlying accountSend fails (here: sender has no trust
+        // line for the vault asset and is not its issuer, so cannot source
+        // the IOU), the helper propagates the non-tes error rather than
+        // silently swallowing it.
+        {
+            Account const stranger{"stranger"};
+            env.fund(XRP(10'000), stranger);
+            env.close();
+            STAmount const ten{vaultAsset, 10};
+            auto const ter = addVaultAssets(view, vault, stranger, ten, ten, env.journal);
+            BEAST_EXPECT(!isTesSuccess(ter));
+        }
     }
 
     void
@@ -221,6 +234,21 @@ class VaultHelpers_test : public beast::unit_test::Suite
             BEAST_EXPECT(Number(vault->at(sfAssetsTotal)) == totalBeforeFail);
             BEAST_EXPECT(Number(vault->at(sfAssetsAvailable)) == availableBeforeFail);
         }
+
+        // If the underlying accountSend fails (here: recipient has no trust
+        // line for the vault asset and cannot receive it), the helper
+        // propagates the non-tes error rather than silently swallowing it.
+        // In production the recipient is always the asset issuer, which
+        // implicitly holds its own asset; this synthetic third-party
+        // recipient stands in only to exercise the failure branch.
+        {
+            Account const stranger{"stranger"};
+            env.fund(XRP(10'000), stranger);
+            env.close();
+            STAmount const ten{vaultAsset, 10};
+            auto const failTer = clawbackVaultAssets(view, vault, stranger, ten, env.journal);
+            BEAST_EXPECT(!isTesSuccess(failTer));
+        }
     }
 
     void
@@ -417,6 +445,39 @@ class VaultHelpers_test : public beast::unit_test::Suite
         BEAST_EXPECT(availableAfter == availableBefore - 100);
         BEAST_EXPECT(borrowerBalanceAfter == borrowerBalanceBefore + eighty);
         BEAST_EXPECT(feeRecipientBalanceAfter == feeRecipientBalanceBefore + twenty);
+
+        // Zero-amount recipients still count toward the recipients.size() > 1
+        // precondition and drive the sum-of-amounts to zero, which short-
+        // circuits the accountSendMulti call. The Vault's fields are still
+        // updated (both to their pre-call values, since the deltas are all
+        // zero), and no funds move.
+        {
+            Number const totalBeforeZero = vault->at(sfAssetsTotal);
+            Number const availableBeforeZero = vault->at(sfAssetsAvailable);
+            auto const borrowerBalanceBeforeZero = accountHolds(
+                view,
+                borrower,
+                vaultAsset,
+                FreezeHandling::IgnoreFreeze,
+                AuthHandling::IgnoreAuth,
+                env.journal);
+            MultiplePaymentDestinations const zeroRecipients{
+                {borrower, Number{0}},
+                {feeRecipient, Number{0}},
+            };
+            auto const zeroTer = moveVaultAssets(view, vault, zeroRecipients, zero, env.journal);
+            auto const borrowerBalanceAfterZero = accountHolds(
+                view,
+                borrower,
+                vaultAsset,
+                FreezeHandling::IgnoreFreeze,
+                AuthHandling::IgnoreAuth,
+                env.journal);
+            BEAST_EXPECT(isTesSuccess(zeroTer));
+            BEAST_EXPECT(Number(vault->at(sfAssetsTotal)) == totalBeforeZero);
+            BEAST_EXPECT(Number(vault->at(sfAssetsAvailable)) == availableBeforeZero);
+            BEAST_EXPECT(borrowerBalanceAfterZero == borrowerBalanceBeforeZero);
+        }
     }
 
 public: