refactor: vault share pricing safety and Expected API

include/xrpl/ledger/VaultHelpers.h

@@ -1,8 +1,11 @@
 #pragma once
 
+#include <xrpl/basics/Expected.h>
+#include <xrpl/beast/utility/Journal.h>
 #include <xrpl/protocol/Rules.h>
 #include <xrpl/protocol/STAmount.h>
 #include <xrpl/protocol/STLedgerEntry.h>
+#include <xrpl/protocol/TER.h>
 
 #include <optional>
 
@@ -10,35 +13,66 @@ namespace xrpl::vault {
 
 enum class TruncateShares : bool { no = false, yes = true };
 
-// SLE-based public API — dispatches to v1 or v2 based on amendment rules.
+// Low-level v2 math — exposed for unit testing.
+namespace detail {
 
-[[nodiscard]] std::optional<STAmount>
-assetsToSharesDeposit(
-    Rules const& rules,
-    SLE::const_ref vault,
-    SLE::const_ref issuance,
-    STAmount const& assets);
+[[nodiscard]] STAmount
+assetsToSharesDeposit(SLE::const_ref vault, SLE::const_ref issuance, STAmount const& assets);
 
-[[nodiscard]] std::optional<STAmount>
-sharesToAssetsDeposit(
-    Rules const& rules,
-    SLE::const_ref vault,
-    SLE::const_ref issuance,
-    STAmount const& shares);
+[[nodiscard]] STAmount
+sharesToAssetsDeposit(SLE::const_ref vault, SLE::const_ref issuance, STAmount const& shares);
 
-[[nodiscard]] std::optional<STAmount>
+[[nodiscard]] STAmount
 assetsToSharesWithdraw(
-    Rules const& rules,
     SLE::const_ref vault,
     SLE::const_ref issuance,
     STAmount const& assets,
     TruncateShares truncate = TruncateShares::no);
 
-[[nodiscard]] std::optional<STAmount>
-sharesToAssetsWithdraw(
+[[nodiscard]] STAmount
+sharesToAssetsWithdraw(SLE::const_ref vault, SLE::const_ref issuance, STAmount const& shares);
+
+}  // namespace detail
+
+// High-level API — orchestrates forward+reverse conversions, handles overflow.
+
+struct ExchangeResult
+{
+    STAmount assets;
+    STAmount shares;
+};
+
+[[nodiscard]] Expected<ExchangeResult, TER>
+computeDeposit(
     Rules const& rules,
     SLE::const_ref vault,
     SLE::const_ref issuance,
-    STAmount const& shares);
+    STAmount const& assets,
+    beast::Journal j);
+
+[[nodiscard]] Expected<ExchangeResult, TER>
+computeWithdrawByAssets(
+    Rules const& rules,
+    SLE::const_ref vault,
+    SLE::const_ref issuance,
+    STAmount const& assets,
+    beast::Journal j);
+
+[[nodiscard]] Expected<ExchangeResult, TER>
+computeWithdrawByShares(
+    Rules const& rules,
+    SLE::const_ref vault,
+    SLE::const_ref issuance,
+    STAmount const& shares,
+    beast::Journal j);
+
+[[nodiscard]] Expected<ExchangeResult, TER>
+computeClawback(
+    Rules const& rules,
+    SLE::const_ref vault,
+    SLE::const_ref issuance,
+    STAmount const& clawbackAmount,
+    Number const& assetsAvailable,
+    beast::Journal j);
 
 }  // namespace xrpl::vault

src/libxrpl/ledger/VaultHelpers.cpp

@@ -1,5 +1,6 @@
-#include <xrpl/beast/utility/instrumentation.h>
 #include <xrpl/ledger/VaultHelpers.h>
+//
+#include <xrpl/basics/Log.h>
 #include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/LedgerFormats.h>
 #include <xrpl/protocol/SField.h>
@@ -7,123 +8,107 @@
 
 namespace xrpl::vault {
 
-namespace {
+namespace detail {
 
-namespace v2 {
-
-std::optional<STAmount>
+STAmount
 assetsToSharesDeposit(SLE::const_ref vault, SLE::const_ref issuance, STAmount const& assets)
 {
-    XRPL_ASSERT(!assets.negative(), "xrpl::vault::v2::assetsToSharesDeposit : non-negative assets");
-    XRPL_ASSERT(
-        assets.asset() == vault->at(sfAsset),
-        "xrpl::vault::v2::assetsToSharesDeposit : assets and vault match");
-    if (assets.negative() || assets.asset() != vault->at(sfAsset))
-        return std::nullopt;  // LCOV_EXCL_LINE
-
     Number const assetTotal = vault->at(sfAssetsTotal);
+    auto const scale = vault->at(sfScale);
     STAmount shares{vault->at(sfShareMPTID)};
     if (assetTotal == 0)
         return STAmount{
             shares.asset(),
-            Number(assets.mantissa(), assets.exponent() + vault->at(sfScale)).truncate()};
+            Number(assets.mantissa(), assets.exponent() + scale).truncate(),
+        };
 
-    auto const netAssetValue = assetTotal - Number(vault->at(sfInterestUnrealized));
+    Number const interestUnrealized = vault->at(sfInterestUnrealized);
     Number const shareTotal = issuance->at(sfOutstandingAmount);
+    auto const netAssetValue = assetTotal - interestUnrealized;
+    XRPL_ASSERT(netAssetValue > 0, "xrpl::vault::detail::assetsToSharesDeposit : positive NAV");
+
     shares = ((shareTotal * assets) / netAssetValue).truncate();
     return shares;
 }
 
-std::optional<STAmount>
+STAmount
 sharesToAssetsDeposit(SLE::const_ref vault, SLE::const_ref issuance, STAmount const& shares)
 {
-    XRPL_ASSERT(!shares.negative(), "xrpl::vault::v2::sharesToAssetsDeposit : non-negative shares");
-    XRPL_ASSERT(
-        shares.asset() == vault->at(sfShareMPTID),
-        "xrpl::vault::v2::sharesToAssetsDeposit : shares and vault match");
-    if (shares.negative() || shares.asset() != vault->at(sfShareMPTID))
-        return std::nullopt;  // LCOV_EXCL_LINE
-
     Number const assetTotal = vault->at(sfAssetsTotal);
+    auto const scale = vault->at(sfScale);
     STAmount assets{vault->at(sfAsset)};
     if (assetTotal == 0)
         return STAmount{
-            assets.asset(), shares.mantissa(), shares.exponent() - vault->at(sfScale), false};
+            assets.asset(),
+            shares.mantissa(),
+            shares.exponent() - scale,
+            false,
+        };
 
-    auto const netAssetValue = assetTotal - Number(vault->at(sfInterestUnrealized));
+    Number const interestUnrealized = vault->at(sfInterestUnrealized);
     Number const shareTotal = issuance->at(sfOutstandingAmount);
+    auto const netAssetValue = assetTotal - interestUnrealized;
+    XRPL_ASSERT(netAssetValue > 0, "xrpl::vault::detail::sharesToAssetsDeposit : positive NAV");
+
     assets = (netAssetValue * shares) / shareTotal;
     return assets;
 }
 
-std::optional<STAmount>
+STAmount
 assetsToSharesWithdraw(
     SLE::const_ref vault,
     SLE::const_ref issuance,
     STAmount const& assets,
     TruncateShares truncate)
 {
-    XRPL_ASSERT(
-        !assets.negative(), "xrpl::vault::v2::assetsToSharesWithdraw : non-negative assets");
-    XRPL_ASSERT(
-        assets.asset() == vault->at(sfAsset),
-        "xrpl::vault::v2::assetsToSharesWithdraw : assets and vault match");
-    if (assets.negative() || assets.asset() != vault->at(sfAsset))
-        return std::nullopt;  // LCOV_EXCL_LINE
+    Number const assetTotal = vault->at(sfAssetsTotal);
+    Number const interestUnrealized = vault->at(sfInterestUnrealized);
+    Number const lossUnrealized = vault->at(sfLossUnrealized);
+    Number const netAssetValue = assetTotal - interestUnrealized - lossUnrealized;
 
-    Number netAssetValue = vault->at(sfAssetsTotal);
-    netAssetValue -= vault->at(sfInterestUnrealized);
-    netAssetValue -= vault->at(sfLossUnrealized);
     STAmount shares{vault->at(sfShareMPTID)};
     if (netAssetValue == 0)
         return shares;
+    XRPL_ASSERT(netAssetValue > 0, "xrpl::vault::detail::assetsToSharesWithdraw : positive NAV");
+
     Number const shareTotal = issuance->at(sfOutstandingAmount);
     Number result = (shareTotal * assets) / netAssetValue;
     if (truncate == TruncateShares::yes)
         result = result.truncate();
+
     shares = result;
     return shares;
 }
 
-std::optional<STAmount>
+STAmount
 sharesToAssetsWithdraw(SLE::const_ref vault, SLE::const_ref issuance, STAmount const& shares)
 {
-    XRPL_ASSERT(
-        !shares.negative(), "xrpl::vault::v2::sharesToAssetsWithdraw : non-negative shares");
-    XRPL_ASSERT(
-        shares.asset() == vault->at(sfShareMPTID),
-        "xrpl::vault::v2::sharesToAssetsWithdraw : shares and vault match");
-    if (shares.negative() || shares.asset() != vault->at(sfShareMPTID))
-        return std::nullopt;  // LCOV_EXCL_LINE
+    Number const assetTotal = vault->at(sfAssetsTotal);
+    Number const interestUnrealized = vault->at(sfInterestUnrealized);
+    Number const lossUnrealized = vault->at(sfLossUnrealized);
+    Number const netAssetValue = assetTotal - interestUnrealized - lossUnrealized;
 
-    Number netAssetValue = vault->at(sfAssetsTotal);
-    netAssetValue -= vault->at(sfInterestUnrealized);
-    netAssetValue -= vault->at(sfLossUnrealized);
     STAmount assets{vault->at(sfAsset)};
     if (netAssetValue == 0)
         return assets;
+    XRPL_ASSERT(netAssetValue > 0, "xrpl::vault::detail::sharesToAssetsWithdraw : positive NAV");
+
     Number const shareTotal = issuance->at(sfOutstandingAmount);
     assets = (netAssetValue * shares) / shareTotal;
     return assets;
 }
 
-}  // namespace v2
+}  // namespace detail
 
 // v1 math is intentionally not factored out like v2. Since Single Asset Vault
 // is already released, refactoring v1 risks introducing behavioral changes in
 // production code that we cannot gate behind an amendment.
+namespace {
 namespace v1 {
 
-std::optional<STAmount>
+STAmount
 assetsToSharesDeposit(SLE::const_ref vault, SLE::const_ref issuance, STAmount const& assets)
 {
-    XRPL_ASSERT(!assets.negative(), "xrpl::vault::v1::assetsToSharesDeposit : non-negative assets");
-    XRPL_ASSERT(
-        assets.asset() == vault->at(sfAsset),
-        "xrpl::vault::v1::assetsToSharesDeposit : assets and vault match");
-    if (assets.negative() || assets.asset() != vault->at(sfAsset))
-        return std::nullopt;  // LCOV_EXCL_LINE
-
     Number const assetTotal = vault->at(sfAssetsTotal);
     STAmount shares{vault->at(sfShareMPTID)};
     if (assetTotal == 0)
@@ -136,16 +121,9 @@ assetsToSharesDeposit(SLE::const_ref vault, SLE::const_ref issuance, STAmount co
     return shares;
 }
 
-std::optional<STAmount>
+STAmount
 sharesToAssetsDeposit(SLE::const_ref vault, SLE::const_ref issuance, STAmount const& shares)
 {
-    XRPL_ASSERT(!shares.negative(), "xrpl::vault::v1::sharesToAssetsDeposit : non-negative shares");
-    XRPL_ASSERT(
-        shares.asset() == vault->at(sfShareMPTID),
-        "xrpl::vault::v1::sharesToAssetsDeposit : shares and vault match");
-    if (shares.negative() || shares.asset() != vault->at(sfShareMPTID))
-        return std::nullopt;  // LCOV_EXCL_LINE
-
     Number const assetTotal = vault->at(sfAssetsTotal);
     STAmount assets{vault->at(sfAsset)};
     if (assetTotal == 0)
@@ -157,21 +135,13 @@ sharesToAssetsDeposit(SLE::const_ref vault, SLE::const_ref issuance, STAmount co
     return assets;
 }
 
-std::optional<STAmount>
+STAmount
 assetsToSharesWithdraw(
     SLE::const_ref vault,
     SLE::const_ref issuance,
     STAmount const& assets,
     TruncateShares truncate)
 {
-    XRPL_ASSERT(
-        !assets.negative(), "xrpl::vault::v1::assetsToSharesWithdraw : non-negative assets");
-    XRPL_ASSERT(
-        assets.asset() == vault->at(sfAsset),
-        "xrpl::vault::v1::assetsToSharesWithdraw : assets and vault match");
-    if (assets.negative() || assets.asset() != vault->at(sfAsset))
-        return std::nullopt;  // LCOV_EXCL_LINE
-
     Number assetTotal = vault->at(sfAssetsTotal);
     assetTotal -= vault->at(sfLossUnrealized);
     STAmount shares{vault->at(sfShareMPTID)};
@@ -185,17 +155,9 @@ assetsToSharesWithdraw(
     return shares;
 }
 
-std::optional<STAmount>
+STAmount
 sharesToAssetsWithdraw(SLE::const_ref vault, SLE::const_ref issuance, STAmount const& shares)
 {
-    XRPL_ASSERT(
-        !shares.negative(), "xrpl::vault::v1::sharesToAssetsWithdraw : non-negative shares");
-    XRPL_ASSERT(
-        shares.asset() == vault->at(sfShareMPTID),
-        "xrpl::vault::v1::sharesToAssetsWithdraw : shares and vault match");
-    if (shares.negative() || shares.asset() != vault->at(sfShareMPTID))
-        return std::nullopt;  // LCOV_EXCL_LINE
-
     Number assetTotal = vault->at(sfAssetsTotal);
     assetTotal -= vault->at(sfLossUnrealized);
     STAmount assets{vault->at(sfAsset)};
@@ -208,55 +170,299 @@ sharesToAssetsWithdraw(SLE::const_ref vault, SLE::const_ref issuance, STAmount c
 
 }  // namespace v1
 
-}  // anonymous namespace
+// v2 vault state validation — checks ledger invariants before math.
+// Returns tecINTERNAL and logs on invalid state. Returns tesSUCCESS if valid.
+TER
+validateVaultState(SLE::const_ref vault, SLE::const_ref issuance, beast::Journal j)
+{
+    Number const assetTotal = vault->at(sfAssetsTotal);
+    if (assetTotal == 0)
+        return tesSUCCESS;
 
-[[nodiscard]] std::optional<STAmount>
+    Number const interestUnrealized = vault->at(sfInterestUnrealized);
+    Number const lossUnrealized = vault->at(sfLossUnrealized);
+    Number const nav = assetTotal - interestUnrealized - lossUnrealized;
+    if (nav < 0)
+    {
+        JLOG(j.error()) << "vault state: NAV < 0"
+                        << " (assetsTotal=" << assetTotal
+                        << ", interestUnrealized=" << interestUnrealized
+                        << ", lossUnrealized=" << lossUnrealized << ")";
+        return tecINTERNAL;
+    }
+
+    Number const shareTotal = issuance->at(sfOutstandingAmount);
+    if (nav > 0 && shareTotal <= 0)
+    {
+        JLOG(j.error()) << "vault state: no outstanding shares with positive NAV"
+                        << " (NAV=" << nav << ", shareTotal=" << shareTotal << ")";
+        return tecINTERNAL;
+    }
+
+    return tesSUCCESS;
+}
+
+// Dispatch to v1 or v2 math based on amendment.
+// v1 is frozen — no refactoring to preserve existing behaviour.
+
+STAmount
 assetsToSharesDeposit(
     Rules const& rules,
     SLE::const_ref vault,
     SLE::const_ref issuance,
     STAmount const& assets)
 {
-    if (rules.enabled(fixLendingProtocolV1_1))
-        return v2::assetsToSharesDeposit(vault, issuance, assets);
+    if (rules.enabled(featureLendingProtocolV1_1))
+        return detail::assetsToSharesDeposit(vault, issuance, assets);
     return v1::assetsToSharesDeposit(vault, issuance, assets);
 }
 
-[[nodiscard]] std::optional<STAmount>
+STAmount
 sharesToAssetsDeposit(
     Rules const& rules,
     SLE::const_ref vault,
     SLE::const_ref issuance,
     STAmount const& shares)
 {
-    if (rules.enabled(fixLendingProtocolV1_1))
-        return v2::sharesToAssetsDeposit(vault, issuance, shares);
+    if (rules.enabled(featureLendingProtocolV1_1))
+        return detail::sharesToAssetsDeposit(vault, issuance, shares);
     return v1::sharesToAssetsDeposit(vault, issuance, shares);
 }
 
-[[nodiscard]] std::optional<STAmount>
+STAmount
 assetsToSharesWithdraw(
     Rules const& rules,
     SLE::const_ref vault,
     SLE::const_ref issuance,
     STAmount const& assets,
-    TruncateShares truncate)
+    TruncateShares truncate = TruncateShares::no)
 {
-    if (rules.enabled(fixLendingProtocolV1_1))
-        return v2::assetsToSharesWithdraw(vault, issuance, assets, truncate);
+    if (rules.enabled(featureLendingProtocolV1_1))
+        return detail::assetsToSharesWithdraw(vault, issuance, assets, truncate);
     return v1::assetsToSharesWithdraw(vault, issuance, assets, truncate);
 }
 
-[[nodiscard]] std::optional<STAmount>
+STAmount
 sharesToAssetsWithdraw(
     Rules const& rules,
     SLE::const_ref vault,
     SLE::const_ref issuance,
     STAmount const& shares)
 {
-    if (rules.enabled(fixLendingProtocolV1_1))
-        return v2::sharesToAssetsWithdraw(vault, issuance, shares);
+    if (rules.enabled(featureLendingProtocolV1_1))
+        return detail::sharesToAssetsWithdraw(vault, issuance, shares);
     return v1::sharesToAssetsWithdraw(vault, issuance, shares);
 }
 
+}  // anonymous namespace
+
+[[nodiscard]] Expected<ExchangeResult, TER>
+computeDeposit(
+    Rules const& rules,
+    SLE::const_ref vault,
+    SLE::const_ref issuance,
+    STAmount const& assets,
+    beast::Journal j)
+{
+    XRPL_ASSERT(vault->getType() == ltVAULT, "xrpl::vault::computeDeposit : vault SLE");
+    XRPL_ASSERT(
+        issuance->getType() == ltMPTOKEN_ISSUANCE, "xrpl::vault::computeDeposit : issuance SLE");
+    if (assets.negative())
+    {
+        JLOG(j.error()) << "computeDeposit: negative assets";
+        return Unexpected(tecINTERNAL);
+    }
+    if (assets.asset() != vault->at(sfAsset))
+    {
+        JLOG(j.error()) << "computeDeposit: asset mismatch";
+        return Unexpected(tecINTERNAL);
+    }
+    if (rules.enabled(featureLendingProtocolV1_1))
+    {
+        if (auto const ter = validateVaultState(vault, issuance, j); ter != tesSUCCESS)
+            return Unexpected(ter);
+    }
+    try
+    {
+        auto const shares = assetsToSharesDeposit(rules, vault, issuance, assets);
+        if (shares == beast::zero)
+            return Unexpected(tecPRECISION_LOSS);
+
+        auto const assetsOut = sharesToAssetsDeposit(rules, vault, issuance, shares);
+        if (assetsOut > assets)
+        {
+            // LCOV_EXCL_START
+            JLOG(j.error()) << "computeDeposit: would take more than offered.";
+            return Unexpected(tecINTERNAL);
+            // LCOV_EXCL_STOP
+        }
+
+        return ExchangeResult{assetsOut, shares};
+    }
+    catch (std::overflow_error const&)
+    {
+        JLOG(j.debug()) << "computeDeposit: overflow error with"
+                        << " scale=" << vault->at(sfScale)
+                        << ", assetsTotal=" << vault->at(sfAssetsTotal)
+                        << ", sharesTotal=" << issuance->at(sfOutstandingAmount)
+                        << ", amount=" << assets;
+        return Unexpected(tecPATH_DRY);
+    }
+}
+
+[[nodiscard]] Expected<ExchangeResult, TER>
+computeWithdrawByAssets(
+    Rules const& rules,
+    SLE::const_ref vault,
+    SLE::const_ref issuance,
+    STAmount const& assets,
+    beast::Journal j)
+{
+    XRPL_ASSERT(vault->getType() == ltVAULT, "xrpl::vault::computeWithdrawByAssets : vault SLE");
+    XRPL_ASSERT(
+        issuance->getType() == ltMPTOKEN_ISSUANCE,
+        "xrpl::vault::computeWithdrawByAssets : issuance SLE");
+    if (assets.negative())
+    {
+        JLOG(j.error()) << "computeWithdrawByAssets: negative assets";
+        return Unexpected(tecINTERNAL);
+    }
+    if (assets.asset() != vault->at(sfAsset))
+    {
+        JLOG(j.error()) << "computeWithdrawByAssets: asset mismatch";
+        return Unexpected(tecINTERNAL);
+    }
+    if (rules.enabled(featureLendingProtocolV1_1))
+    {
+        if (auto const ter = validateVaultState(vault, issuance, j); ter != tesSUCCESS)
+            return Unexpected(ter);
+    }
+    try
+    {
+        auto const shares = assetsToSharesWithdraw(rules, vault, issuance, assets);
+        if (shares == beast::zero)
+            return Unexpected(tecPRECISION_LOSS);
+
+        auto const assetsOut = sharesToAssetsWithdraw(rules, vault, issuance, shares);
+        return ExchangeResult{assetsOut, shares};
+    }
+    catch (std::overflow_error const&)
+    {
+        JLOG(j.debug()) << "computeWithdrawByAssets: overflow error with"
+                        << " scale=" << vault->at(sfScale)
+                        << ", assetsTotal=" << vault->at(sfAssetsTotal)
+                        << ", sharesTotal=" << issuance->at(sfOutstandingAmount)
+                        << ", amount=" << assets;
+        return Unexpected(tecPATH_DRY);
+    }
+}
+
+[[nodiscard]] Expected<ExchangeResult, TER>
+computeWithdrawByShares(
+    Rules const& rules,
+    SLE::const_ref vault,
+    SLE::const_ref issuance,
+    STAmount const& shares,
+    beast::Journal j)
+{
+    XRPL_ASSERT(vault->getType() == ltVAULT, "xrpl::vault::computeWithdrawByShares : vault SLE");
+    XRPL_ASSERT(
+        issuance->getType() == ltMPTOKEN_ISSUANCE,
+        "xrpl::vault::computeWithdrawByShares : issuance SLE");
+    if (shares.negative())
+    {
+        JLOG(j.error()) << "computeWithdrawByShares: negative shares";
+        return Unexpected(tecINTERNAL);
+    }
+    if (shares.asset() != vault->at(sfShareMPTID))
+    {
+        JLOG(j.error()) << "computeWithdrawByShares: share asset mismatch";
+        return Unexpected(tecINTERNAL);
+    }
+    if (rules.enabled(featureLendingProtocolV1_1))
+    {
+        if (auto const ter = validateVaultState(vault, issuance, j); ter != tesSUCCESS)
+            return Unexpected(ter);
+    }
+    try
+    {
+        auto const assets = sharesToAssetsWithdraw(rules, vault, issuance, shares);
+        return ExchangeResult{assets, shares};
+    }
+    catch (std::overflow_error const&)
+    {
+        JLOG(j.debug()) << "computeWithdrawByShares: overflow error with"
+                        << " scale=" << vault->at(sfScale)
+                        << ", assetsTotal=" << vault->at(sfAssetsTotal)
+                        << ", sharesTotal=" << issuance->at(sfOutstandingAmount)
+                        << ", shares=" << shares;
+        return Unexpected(tecPATH_DRY);
+    }
+}
+
+[[nodiscard]] Expected<ExchangeResult, TER>
+computeClawback(
+    Rules const& rules,
+    SLE::const_ref vault,
+    SLE::const_ref issuance,
+    STAmount const& clawbackAmount,
+    Number const& assetsAvailable,
+    beast::Journal j)
+{
+    XRPL_ASSERT(vault->getType() == ltVAULT, "xrpl::vault::computeClawback : vault SLE");
+    XRPL_ASSERT(
+        issuance->getType() == ltMPTOKEN_ISSUANCE, "xrpl::vault::computeClawback : issuance SLE");
+    if (clawbackAmount.negative())
+    {
+        JLOG(j.error()) << "computeClawback: negative clawbackAmount";
+        return Unexpected(tecINTERNAL);
+    }
+    if (clawbackAmount.asset() != vault->at(sfAsset))
+    {
+        JLOG(j.error()) << "computeClawback: asset mismatch";
+        return Unexpected(tecINTERNAL);
+    }
+    if (rules.enabled(featureLendingProtocolV1_1))
+    {
+        if (auto const ter = validateVaultState(vault, issuance, j); ter != tesSUCCESS)
+            return Unexpected(ter);
+    }
+    try
+    {
+        auto sharesDestroyed = assetsToSharesWithdraw(rules, vault, issuance, clawbackAmount);
+        auto assetsRecovered = sharesToAssetsWithdraw(rules, vault, issuance, sharesDestroyed);
+
+        // Clamp to maximum.
+        if (assetsRecovered > assetsAvailable)
+        {
+            assetsRecovered = assetsAvailable;
+            // Note, it is important to truncate the number of shares,
+            // otherwise the corresponding assets might breach the
+            // AssetsAvailable
+            sharesDestroyed = assetsToSharesWithdraw(
+                rules, vault, issuance, assetsRecovered, TruncateShares::yes);
+            assetsRecovered = sharesToAssetsWithdraw(rules, vault, issuance, sharesDestroyed);
+
+            if (assetsRecovered > assetsAvailable)
+            {
+                // LCOV_EXCL_START
+                JLOG(j.error()) << "computeClawback: invalid rounding of shares.";
+                return Unexpected(tecINTERNAL);
+                // LCOV_EXCL_STOP
+            }
+        }
+
+        return ExchangeResult{assetsRecovered, sharesDestroyed};
+    }
+    catch (std::overflow_error const&)
+    {
+        JLOG(j.debug()) << "computeClawback: overflow error with"
+                        << " scale=" << vault->at(sfScale)
+                        << ", assetsTotal=" << vault->at(sfAssetsTotal)
+                        << ", sharesTotal=" << issuance->at(sfOutstandingAmount)
+                        << ", amount=" << clawbackAmount;
+        return Unexpected(tecPATH_DRY);
+    }
+}
+
 }  // namespace xrpl::vault

src/libxrpl/tx/transactors/vault/VaultClawback.cpp

@@ -235,75 +235,19 @@ VaultClawback::assetsToClawback(
             FreezeHandling::fhIGNORE_FREEZE,
             AuthHandling::ahIGNORE_AUTH,
             j_);
-        auto const maybeAssets =
-            vault::sharesToAssetsWithdraw(rules, vault, sleShareIssuance, sharesDestroyed);
-        if (!maybeAssets)
-            return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
+        auto const result =
+            vault::computeWithdrawByShares(rules, vault, sleShareIssuance, sharesDestroyed, j_);
+        if (!result)
+            return Unexpected(result.error());
 
-        return std::make_pair(*maybeAssets, sharesDestroyed);
+        return std::make_pair(result->assets, result->shares);
     }
 
-    STAmount sharesDestroyed;
-    STAmount assetsRecovered = clawbackAmount;
-    try
-    {
-        {
-            auto const maybeShares =
-                vault::assetsToSharesWithdraw(rules, vault, sleShareIssuance, assetsRecovered);
-            if (!maybeShares)
-                return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
-            sharesDestroyed = *maybeShares;
-        }
-
-        auto const maybeAssets =
-            vault::sharesToAssetsWithdraw(rules, vault, sleShareIssuance, sharesDestroyed);
-        if (!maybeAssets)
-            return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
-        assetsRecovered = *maybeAssets;
-
-        // Clamp to maximum.
-        if (assetsRecovered > *assetsAvailable)
-        {
-            assetsRecovered = *assetsAvailable;
-            // Note, it is important to truncate the number of shares,
-            // otherwise the corresponding assets might breach the
-            // AssetsAvailable
-            {
-                auto const maybeShares = vault::assetsToSharesWithdraw(
-                    rules, vault, sleShareIssuance, assetsRecovered, vault::TruncateShares::yes);
-                if (!maybeShares)
-                    return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
-                sharesDestroyed = *maybeShares;
-            }
-
-            auto const maybeAssets =
-                vault::sharesToAssetsWithdraw(rules, vault, sleShareIssuance, sharesDestroyed);
-            if (!maybeAssets)
-                return Unexpected(tecINTERNAL);  // LCOV_EXCL_LINE
-            assetsRecovered = *maybeAssets;
-            if (assetsRecovered > *assetsAvailable)
-            {
-                // LCOV_EXCL_START
-                JLOG(j_.error()) << "VaultClawback: invalid rounding of shares.";
-                return Unexpected(tecINTERNAL);
-                // LCOV_EXCL_STOP
-            }
-        }
-    }
-    catch (std::overflow_error const&)
-    {
-        // It's easy to hit this exception from Number with large enough
-        // Scale so we avoid spamming the log and only use debug here.
-        JLOG(j_.debug())  //
-            << "VaultClawback: overflow error with"
-            << " scale=" << (int)vault->at(sfScale).value()  //
-            << ", assetsTotal=" << vault->at(sfAssetsTotal).value()
-            << ", sharesTotal=" << sleShareIssuance->at(sfOutstandingAmount)
-            << ", amount=" << clawbackAmount.value();
-        return Unexpected(tecPATH_DRY);
-    }
-
-    return std::make_pair(assetsRecovered, sharesDestroyed);
+    auto const result = vault::computeClawback(
+        rules, vault, sleShareIssuance, clawbackAmount, Number{assetsAvailable}, j_);
+    if (!result)
+        return Unexpected(result.error());
+    return std::make_pair(result->assets, result->shares);
 }
 
 TER

src/libxrpl/tx/transactors/vault/VaultDeposit.cpp

@@ -185,47 +185,12 @@ VaultDeposit::doApply()
         }
     }
 
-    STAmount sharesCreated = {vault->at(sfShareMPTID)}, assetsDeposited;
+    // Compute exchange before transferring any amounts.
     auto const& rules = ctx_.view().rules();
-    try
-    {
-        // Compute exchange before transferring any amounts.
-        {
-            auto const maybeShares =
-                vault::assetsToSharesDeposit(rules, vault, sleIssuance, amount);
-            if (!maybeShares)
-                return tecINTERNAL;  // LCOV_EXCL_LINE
-            sharesCreated = *maybeShares;
-        }
-        if (sharesCreated == beast::zero)
-            return tecPRECISION_LOSS;
-
-        auto const maybeAssets =
-            vault::sharesToAssetsDeposit(rules, vault, sleIssuance, sharesCreated);
-        if (!maybeAssets)
-        {
-            return tecINTERNAL;  // LCOV_EXCL_LINE
-        }
-        if (*maybeAssets > amount)
-        {
-            // LCOV_EXCL_START
-            JLOG(j_.error()) << "VaultDeposit: would take more than offered.";
-            return tecINTERNAL;
-            // LCOV_EXCL_STOP
-        }
-        assetsDeposited = *maybeAssets;
-    }
-    catch (std::overflow_error const&)
-    {
-        // It's easy to hit this exception from Number with large enough Scale
-        // so we avoid spamming the log and only use debug here.
-        JLOG(j_.debug())  //
-            << "VaultDeposit: overflow error with"
-            << " scale=" << (int)vault->at(sfScale).value()  //
-            << ", assetsTotal=" << vault->at(sfAssetsTotal).value()
-            << ", sharesTotal=" << sleIssuance->at(sfOutstandingAmount) << ", amount=" << amount;
-        return tecPATH_DRY;
-    }
+    auto const result = vault::computeDeposit(rules, vault, sleIssuance, amount, j_);
+    if (!result)
+        return result.error();
+    auto const& [assetsDeposited, sharesCreated] = *result;
 
     XRPL_ASSERT(
         sharesCreated.asset() != assetsDeposited.asset(),

src/libxrpl/tx/transactors/vault/VaultWithdraw.cpp

@@ -114,57 +114,18 @@ VaultWithdraw::doApply()
     Asset const vaultAsset = vault->at(sfAsset);
 
     MPTIssue const share{mptIssuanceID};
-    STAmount sharesRedeemed = {share};
-    STAmount assetsWithdrawn;
     auto const& rules = ctx_.view().rules();
-    try
-    {
-        if (amount.asset() == vaultAsset)
-        {
-            // Fixed assets, variable shares.
-            {
-                auto const maybeShares =
-                    vault::assetsToSharesWithdraw(rules, vault, sleIssuance, amount);
-                if (!maybeShares)
-                    return tecINTERNAL;  // LCOV_EXCL_LINE
-                sharesRedeemed = *maybeShares;
-            }
 
-            if (sharesRedeemed == beast::zero)
-                return tecPRECISION_LOSS;
-            auto const maybeAssets =
-                vault::sharesToAssetsWithdraw(rules, vault, sleIssuance, sharesRedeemed);
-            if (!maybeAssets)
-                return tecINTERNAL;  // LCOV_EXCL_LINE
-            assetsWithdrawn = *maybeAssets;
-        }
-        else if (amount.asset() == share)
-        {
-            // Fixed shares, variable assets.
-            sharesRedeemed = amount;
-            auto const maybeAssets =
-                vault::sharesToAssetsWithdraw(rules, vault, sleIssuance, sharesRedeemed);
-            if (!maybeAssets)
-                return tecINTERNAL;  // LCOV_EXCL_LINE
-            assetsWithdrawn = *maybeAssets;
-        }
-        else
-        {
-            return tefINTERNAL;  // LCOV_EXCL_LINE
-        }
-    }
-    catch (std::overflow_error const&)
-    {
-        // It's easy to hit this exception from Number with large enough Scale
-        // so we avoid spamming the log and only use debug here.
-        JLOG(j_.debug())  //
-            << "VaultWithdraw: overflow error with"
-            << " scale=" << (int)vault->at(sfScale).value()  //
-            << ", assetsTotal=" << vault->at(sfAssetsTotal).value()
-            << ", sharesTotal=" << sleIssuance->at(sfOutstandingAmount)
-            << ", amount=" << amount.value();
-        return tecPATH_DRY;
-    }
+    auto const result = [&]() -> Expected<vault::ExchangeResult, TER> {
+        if (amount.asset() == vaultAsset)
+            return vault::computeWithdrawByAssets(rules, vault, sleIssuance, amount, j_);
+        if (amount.asset() == share)
+            return vault::computeWithdrawByShares(rules, vault, sleIssuance, amount, j_);
+        return Unexpected(tefINTERNAL);  // LCOV_EXCL_LINE
+    }();
+    if (!result)
+        return result.error();
+    auto const& [assetsWithdrawn, sharesRedeemed] = *result;
 
     if (accountHolds(
             view(),

src/test/app/VaultHelpers_test.cpp

@@ -0,0 +1,734 @@
+#include <test/jtx.h>
+
+#include <xrpl/beast/unit_test/suite.h>
+#include <xrpl/ledger/VaultHelpers.h>
+#include <xrpl/protocol/Feature.h>
+#include <xrpl/protocol/Indexes.h>
+#include <xrpl/protocol/Issue.h>
+#include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/STAmount.h>
+#include <xrpl/protocol/STIssue.h>
+#include <xrpl/protocol/STLedgerEntry.h>
+#include <xrpl/protocol/STNumber.h>
+
+#include <string>
+#include <vector>
+
+namespace xrpl {
+namespace test {
+
+class VaultHelpers_test : public beast::unit_test::suite
+{
+    jtx::Env* env_ = nullptr;
+    AccountID const issuerID{0x2};
+    std::uint32_t const seq{1};
+    MPTID const shareMPTID = makeMptID(seq, issuerID);
+    Issue const assetIssue{Currency(0x1), AccountID(0x2)};
+    MPTIssue const shareIssue{shareMPTID};
+
+    SLE::pointer
+    makeVault(
+        Number const& assetsTotal,
+        Number const& lossUnrealized,
+        Number const& interestUnrealized,
+        std::int32_t scale)
+    {
+        auto sle = std::make_shared<SLE>(keylet::vault(AccountID(0x1), 1));
+        (*sle)[sfAsset] = STIssue{sfAsset, assetIssue};
+        (*sle)[sfShareMPTID] = shareMPTID;
+        (*sle)[sfAssetsTotal] = STNumber{sfAssetsTotal, assetsTotal};
+        (*sle)[sfLossUnrealized] = STNumber{sfLossUnrealized, lossUnrealized};
+        (*sle)[sfInterestUnrealized] = STNumber{sfInterestUnrealized, interestUnrealized};
+        (*sle)[sfScale] = scale;
+        return sle;
+    }
+
+    SLE::pointer
+    makeIssuance(std::uint64_t outstandingAmount)
+    {
+        auto sle = std::make_shared<SLE>(keylet::mptIssuance(shareMPTID));
+        (*sle)[sfOutstandingAmount] = outstandingAmount;
+        return sle;
+    }
+
+    Rules
+    rules() const
+    {
+        return env_->current()->rules();
+    }
+
+    STAmount
+    asset(Number const& value) const
+    {
+        return STAmount{assetIssue, value};
+    }
+
+    STAmount
+    shares(Number const& value) const
+    {
+        return STAmount{shareIssue, value};
+    }
+
+    void
+    testAssetsToSharesDeposit()
+    {
+        using namespace vault;
+
+        struct TestCase
+        {
+            std::string name;
+            Number assetsTotal;
+            Number interestUnrealized;
+            std::uint64_t shareTotal;
+            std::int32_t scale;
+            Number depositAssets;
+            STAmount expectedShares;
+        };
+
+        auto const testCases = std::vector<TestCase>{
+            // Initial: shares = Number(assets.mantissa, assets.exponent + scale).truncate()
+            {
+                .name = "Initial deposit IOU scale=6",
+                .assetsTotal = 0,
+                .interestUnrealized = 0,
+                .shareTotal = 0,
+                .scale = 6,
+                .depositAssets = 100,
+                .expectedShares =
+                    shares(Number(Number{100}.mantissa(), Number{100}.exponent() + 6).truncate()),
+            },
+            {
+                .name = "Initial deposit scale=0",
+                .assetsTotal = 0,
+                .interestUnrealized = 0,
+                .shareTotal = 0,
+                .scale = 0,
+                .depositAssets = 10'000'000,
+                .expectedShares =
+                    shares(Number(Number{10'000'000}.mantissa(), Number{10'000'000}.exponent() + 0)
+                               .truncate()),
+            },
+            {
+                .name = "Initial deposit scale=2",
+                .assetsTotal = 0,
+                .interestUnrealized = 0,
+                .shareTotal = 0,
+                .scale = 2,
+                .depositAssets = 50,
+                .expectedShares =
+                    shares(Number(Number{50}.mantissa(), Number{50}.exponent() + 2).truncate()),
+            },
+            {
+                .name = "Initial deposit scale=18",
+                .assetsTotal = 0,
+                .interestUnrealized = 0,
+                .shareTotal = 0,
+                .scale = 18,
+                .depositAssets = 1,
+                .expectedShares =
+                    shares(Number(Number{1}.mantissa(), Number{1}.exponent() + 18).truncate()),
+            },
+            // Subsequent: shares = floor(shareTotal * assets / depositNAV)
+            {
+                .name = "Subsequent proportional",
+                .assetsTotal = 100,
+                .interestUnrealized = 0,
+                .shareTotal = 100'000'000,
+                .scale = 6,
+                .depositAssets = 50,
+                .expectedShares = shares(Number{(100'000'000LL * 50) / 100}.truncate()),
+            },
+            {
+                .name = "With unrealized interest",
+                .assetsTotal = 1000,
+                .interestUnrealized = 50,
+                .shareTotal = 950,
+                .scale = 0,
+                .depositAssets = 95,
+                // depositNAV = 1000 - 50 = 950
+                .expectedShares = shares(Number{(950 * 95) / 950}.truncate()),
+            },
+            {
+                .name = "Deposit with interest+loss (deposit ignores loss)",
+                .assetsTotal = 1000,
+                .interestUnrealized = 50,
+                .shareTotal = 950,
+                .scale = 0,
+                .depositAssets = 1,
+                // depositNAV = 1000 - 50 = 950
+                .expectedShares = shares(Number{(950 * 1) / 950}.truncate()),
+            },
+            {
+                .name = "Floor rounds down (inflated vault)",
+                .assetsTotal = 7,
+                .interestUnrealized = 0,
+                .shareTotal = 3,
+                .scale = 0,
+                .depositAssets = 3,
+                // shares = floor(3 * 3 / 7) = floor(9/7) = 1
+                .expectedShares = shares(Number{(3 * 3) / 7}.truncate()),
+            },
+            {
+                .name = "Tiny into large vault",
+                .assetsTotal = Number{1, 9},
+                .interestUnrealized = 0,
+                .shareTotal = UINT64_C(1'000'000'000'000'000),
+                .scale = 6,
+                .depositAssets = Number{1, -6},
+                .expectedShares =
+                    shares(((Number{1, 15} * Number{1, -6}) / Number{1, 9}).truncate()),
+            },
+            {
+                .name = "Extreme 1e15:1 ratio",
+                .assetsTotal = Number{1, 15},
+                .interestUnrealized = 0,
+                .shareTotal = UINT64_C(1'000'000'000'000'000),
+                .scale = 0,
+                .depositAssets = 1,
+                .expectedShares = shares(((Number{1, 15} * 1) / Number{1, 15}).truncate()),
+            },
+            {
+                .name = "Inflated vault — fewer shares per asset",
+                .assetsTotal = 110,
+                .interestUnrealized = 0,
+                .shareTotal = 100,
+                .scale = 0,
+                .depositAssets = 100,
+                .expectedShares = shares(Number{(100 * 100) / 110}.truncate()),
+            },
+        };
+
+        for (auto const& tc : testCases)
+        {
+            testcase("assetsToSharesDeposit v2: " + tc.name);
+
+            auto const vaultSle = makeVault(tc.assetsTotal, 0, tc.interestUnrealized, tc.scale);
+            auto const issuanceSle = makeIssuance(tc.shareTotal);
+
+            auto const result = vault::detail::assetsToSharesDeposit(
+                vaultSle, issuanceSle, asset(tc.depositAssets));
+
+            BEAST_EXPECTS(
+                result == tc.expectedShares,
+                "expected " + to_string(tc.expectedShares) + ", got " + to_string(result));
+        }
+    }
+
+    void
+    testSharesToAssetsDeposit()
+    {
+        using namespace vault;
+
+        struct TestCase
+        {
+            std::string name;
+            Number assetsTotal;
+            Number interestUnrealized;
+            std::uint64_t shareTotal;
+            std::int32_t scale;
+            Number depositShares;
+            STAmount expectedAssets;
+        };
+
+        auto const testCases = std::vector<TestCase>{
+            // Initial: assets = Number(shares.mantissa, shares.exponent - scale)
+            {
+                .name = "Initial (assetTotal=0) scale=6",
+                .assetsTotal = 0,
+                .interestUnrealized = 0,
+                .shareTotal = 0,
+                .scale = 6,
+                .depositShares = Number{1, 8},
+                .expectedAssets =
+                    asset(Number(Number{1, 8}.mantissa(), Number{1, 8}.exponent() - 6)),
+            },
+            {
+                .name = "Initial (assetTotal=0) scale=0",
+                .assetsTotal = 0,
+                .interestUnrealized = 0,
+                .shareTotal = 0,
+                .scale = 0,
+                .depositShares = 500,
+                .expectedAssets = asset(Number(Number{500}.mantissa(), Number{500}.exponent() - 0)),
+            },
+            // Subsequent: assets = depositNAV * shares / shareTotal
+            {
+                .name = "Proportional back-calc",
+                .assetsTotal = 100,
+                .interestUnrealized = 0,
+                .shareTotal = 100'000'000,
+                .scale = 6,
+                .depositShares = Number{5, 7},
+                .expectedAssets = asset((Number{100} * Number{5, 7}) / 100'000'000),
+            },
+            {
+                .name = "With interest",
+                .assetsTotal = 1000,
+                .interestUnrealized = 50,
+                .shareTotal = 950,
+                .scale = 0,
+                .depositShares = 95,
+                // depositNAV = 950
+                .expectedAssets = asset(Number{950 * 95} / 950),
+            },
+            {
+                .name = "Non-integer back-calc (7/3)",
+                .assetsTotal = 7,
+                .interestUnrealized = 0,
+                .shareTotal = 3,
+                .scale = 0,
+                .depositShares = 1,
+                // assets = 7 * 1 / 3
+                .expectedAssets = asset(Number{7} / 3),
+            },
+            {
+                .name = "Floor invariant: actualAssets <= requested",
+                .assetsTotal = 10,
+                .interestUnrealized = 0,
+                .shareTotal = 7,
+                .scale = 0,
+                .depositShares = 2,
+                // assets = 10 * 2 / 7
+                .expectedAssets = asset(Number{10 * 2} / 7),
+            },
+        };
+
+        for (auto const& tc : testCases)
+        {
+            testcase("sharesToAssetsDeposit v2: " + tc.name);
+
+            auto const vaultSle = makeVault(tc.assetsTotal, 0, tc.interestUnrealized, tc.scale);
+            auto const issuanceSle = makeIssuance(tc.shareTotal);
+
+            auto const result = vault::detail::sharesToAssetsDeposit(
+                vaultSle, issuanceSle, shares(tc.depositShares));
+
+            BEAST_EXPECTS(
+                result == tc.expectedAssets,
+                "expected " + to_string(tc.expectedAssets) + ", got " + to_string(result));
+        }
+    }
+
+    void
+    testAssetsToSharesWithdraw()
+    {
+        using namespace vault;
+
+        struct TestCase
+        {
+            std::string name;
+            Number assetsTotal;
+            Number interestUnrealized;
+            Number lossUnrealized;
+            std::uint64_t shareTotal;
+            Number withdrawAssets;
+            TruncateShares truncate;
+            STAmount expectedShares;
+        };
+
+        // shares = shareTotal * assets / withdrawalNAV
+        // withdrawalNAV = assetsTotal - interestUnrealized - lossUnrealized
+
+        auto const testCases = std::vector<TestCase>{
+            {
+                .name = "Basic no loss",
+                .assetsTotal = 100,
+                .interestUnrealized = 0,
+                .lossUnrealized = 0,
+                .shareTotal = 100'000'000,
+                .withdrawAssets = 50,
+                .truncate = TruncateShares::no,
+                // NAV = 100
+                .expectedShares = shares(Number{100'000'000LL * 50} / 100),
+            },
+            {
+                .name = "With paper loss",
+                .assetsTotal = 10,
+                .interestUnrealized = 0,
+                .lossUnrealized = 3,
+                .shareTotal = 10,
+                .withdrawAssets = 1,
+                .truncate = TruncateShares::yes,
+                // NAV = 7; shares = floor(10 * 1 / 7)
+                .expectedShares = shares(Number{(10 * 1) / 7}.truncate()),
+            },
+            {
+                .name = "Fractional still floors same",
+                .assetsTotal = 10,
+                .interestUnrealized = 0,
+                .lossUnrealized = 3,
+                .shareTotal = 10,
+                .withdrawAssets = Number{11, -1},
+                .truncate = TruncateShares::yes,
+                // NAV = 7; shares = floor(10 * 1.1 / 7)
+                .expectedShares = shares(((Number{10} * Number{11, -1}) / 7).truncate()),
+            },
+            {
+                .name = "Floor at 0.5 boundary (confirms floor, not round)",
+                .assetsTotal = 10,
+                .interestUnrealized = 0,
+                .lossUnrealized = 0,
+                .shareTotal = 10,
+                .withdrawAssets = Number{15, -1},
+                .truncate = TruncateShares::yes,
+                // NAV = 10; shares = floor(10 * 1.5 / 10)
+                .expectedShares = shares(((Number{10} * Number{15, -1}) / 10).truncate()),
+            },
+            {
+                .name = "Zero NAV returns zero shares",
+                .assetsTotal = 100,
+                .interestUnrealized = 50,
+                .lossUnrealized = 50,
+                .shareTotal = 500,
+                .withdrawAssets = 10,
+                .truncate = TruncateShares::no,
+                .expectedShares = shares(0),
+            },
+            {
+                .name = "With loss — assetsOut <= requested",
+                .assetsTotal = 10,
+                .interestUnrealized = 0,
+                .lossUnrealized = 2,
+                .shareTotal = 10,
+                .withdrawAssets = 3,
+                .truncate = TruncateShares::yes,
+                // NAV = 8; shares = floor(10 * 3 / 8)
+                .expectedShares = shares(Number{(10 * 3) / 8}.truncate()),
+            },
+            {
+                .name = "With losses and interest (no truncation)",
+                .assetsTotal = 1000,
+                .interestUnrealized = 100,
+                .lossUnrealized = 100,
+                .shareTotal = 500,
+                .withdrawAssets = 100,
+                .truncate = TruncateShares::no,
+                // NAV = 800; raw = 62.5, assigned to MPT STAmount
+                .expectedShares = shares(Number{500 * 100} / 800),
+            },
+            {
+                .name = "With losses and interest (truncated)",
+                .assetsTotal = 1000,
+                .interestUnrealized = 100,
+                .lossUnrealized = 100,
+                .shareTotal = 500,
+                .withdrawAssets = 100,
+                .truncate = TruncateShares::yes,
+                // NAV = 800; shares = floor(500 * 100 / 800)
+                .expectedShares = shares(Number{(500 * 100) / 800}.truncate()),
+            },
+        };
+
+        for (auto const& tc : testCases)
+        {
+            testcase("assetsToSharesWithdraw v2: " + tc.name);
+
+            auto const vaultSle =
+                makeVault(tc.assetsTotal, tc.lossUnrealized, tc.interestUnrealized, 0);
+            auto const issuanceSle = makeIssuance(tc.shareTotal);
+
+            auto const result = vault::detail::assetsToSharesWithdraw(
+                vaultSle, issuanceSle, asset(tc.withdrawAssets), tc.truncate);
+
+            BEAST_EXPECTS(
+                result == tc.expectedShares,
+                "expected " + to_string(tc.expectedShares) + ", got " + to_string(result));
+        }
+    }
+
+    void
+    testSharesToAssetsWithdraw()
+    {
+        using namespace vault;
+
+        struct TestCase
+        {
+            std::string name;
+            Number assetsTotal;
+            Number interestUnrealized;
+            Number lossUnrealized;
+            std::uint64_t shareTotal;
+            Number withdrawShares;
+            STAmount expectedAssets;
+        };
+
+        // assets = withdrawalNAV * shares / shareTotal
+        // withdrawalNAV = assetsTotal - interestUnrealized - lossUnrealized
+
+        auto const testCases = std::vector<TestCase>{
+            {
+                .name = "Basic redeem half",
+                .assetsTotal = 100,
+                .interestUnrealized = 0,
+                .lossUnrealized = 0,
+                .shareTotal = 100'000'000,
+                .withdrawShares = Number{5, 7},
+                .expectedAssets = asset((Number{100} * Number{5, 7}) / 100'000'000),
+            },
+            {
+                .name = "Redeem all",
+                .assetsTotal = 100,
+                .interestUnrealized = 0,
+                .lossUnrealized = 0,
+                .shareTotal = 100'000'000,
+                .withdrawShares = Number{1, 8},
+                .expectedAssets = asset((Number{100} * Number{1, 8}) / 100'000'000),
+            },
+            {
+                .name = "With loss",
+                .assetsTotal = 1050,
+                .interestUnrealized = 50,
+                .lossUnrealized = 100,
+                .shareTotal = 1000,
+                .withdrawShares = 100,
+                // NAV = 900
+                .expectedAssets = asset(Number{900 * 100} / 1000),
+            },
+            {
+                .name = "Both interest+loss (spec example)",
+                .assetsTotal = 1001,
+                .interestUnrealized = 50,
+                .lossUnrealized = 100,
+                .shareTotal = 951,
+                .withdrawShares = 1,
+                // NAV = 851
+                .expectedAssets = asset(Number{851} / 951),
+            },
+            {
+                .name = "Loss only (no interest)",
+                .assetsTotal = 1100,
+                .interestUnrealized = 0,
+                .lossUnrealized = 200,
+                .shareTotal = 1100,
+                .withdrawShares = 100,
+                // NAV = 900
+                .expectedAssets = asset(Number{900 * 100} / 1100),
+            },
+            {
+                .name = "After hard default",
+                .assetsTotal = 600,
+                .interestUnrealized = 0,
+                .lossUnrealized = 0,
+                .shareTotal = 1000,
+                .withdrawShares = 500,
+                .expectedAssets = asset(Number{600 * 500} / 1000),
+            },
+            {
+                .name = "Non-terminating fraction (1/3)",
+                .assetsTotal = 1,
+                .interestUnrealized = 0,
+                .lossUnrealized = 0,
+                .shareTotal = 3,
+                .withdrawShares = 1,
+                .expectedAssets = asset(Number{1} / 3),
+            },
+            {
+                .name = "Zero NAV returns zero assets",
+                .assetsTotal = 100,
+                .interestUnrealized = 50,
+                .lossUnrealized = 50,
+                .shareTotal = 500,
+                .withdrawShares = 10,
+                .expectedAssets = asset(0),
+            },
+            {
+                .name = "High precision with loss",
+                .assetsTotal = Number{1, 12},
+                .interestUnrealized = 0,
+                .lossUnrealized = Number{2, 11},
+                .shareTotal = UINT64_C(1'000'000'000'000),
+                .withdrawShares = 1,
+                // NAV = 8e11
+                .expectedAssets = asset(Number{8, 11} / Number{1, 12}),
+            },
+            {
+                .name = "Inflated vault redeem — captures yield",
+                .assetsTotal = 209,
+                .interestUnrealized = 0,
+                .lossUnrealized = 0,
+                .shareTotal = 190,
+                .withdrawShares = 100,
+                .expectedAssets = asset(Number{209 * 100} / 190),
+            },
+        };
+
+        for (auto const& tc : testCases)
+        {
+            testcase("sharesToAssetsWithdraw v2: " + tc.name);
+
+            auto const vaultSle =
+                makeVault(tc.assetsTotal, tc.lossUnrealized, tc.interestUnrealized, 0);
+            auto const issuanceSle = makeIssuance(tc.shareTotal);
+
+            auto const result = vault::detail::sharesToAssetsWithdraw(
+                vaultSle, issuanceSle, shares(tc.withdrawShares));
+
+            BEAST_EXPECTS(
+                result == tc.expectedAssets,
+                "expected " + to_string(tc.expectedAssets) + ", got " + to_string(result));
+        }
+    }
+
+    void
+    testComputeDeposit()
+    {
+        using namespace vault;
+
+        testcase("computeDeposit: normal");
+        {
+            auto const vaultSle = makeVault(100, 0, 0, 6);
+            auto const issuanceSle = makeIssuance(100'000'000);
+
+            auto const result =
+                computeDeposit(rules(), vaultSle, issuanceSle, asset(50), env_->journal);
+            BEAST_EXPECT(result.has_value());
+            if (result)
+            {
+                BEAST_EXPECT(result->shares != beast::zero);
+                BEAST_EXPECT(result->assets <= asset(50));
+            }
+        }
+
+        testcase("computeDeposit: initial deposit");
+        {
+            auto const vaultSle = makeVault(0, 0, 0, 6);
+            auto const issuanceSle = makeIssuance(0);
+
+            auto const result =
+                computeDeposit(rules(), vaultSle, issuanceSle, asset(100), env_->journal);
+            BEAST_EXPECT(result.has_value());
+            if (result)
+            {
+                BEAST_EXPECT(result->shares != beast::zero);
+                BEAST_EXPECT(result->assets == asset(100));
+            }
+        }
+
+        testcase("computeDeposit: precision loss returns tecPRECISION_LOSS");
+        {
+            // Huge vault, tiny deposit — shares truncate to zero.
+            auto const vaultSle = makeVault(Number{1, 15}, 0, 0, 0);
+            auto const issuanceSle = makeIssuance(1);
+
+            auto const result = computeDeposit(
+                rules(), vaultSle, issuanceSle, asset(Number{1, -15}), env_->journal);
+            BEAST_EXPECT(!result.has_value());
+            if (!result)
+                BEAST_EXPECT(result.error() == tecPRECISION_LOSS);
+        }
+    }
+
+    void
+    testComputeWithdrawByAssets()
+    {
+        using namespace vault;
+
+        testcase("computeWithdrawByAssets: normal");
+        {
+            auto const vaultSle = makeVault(100, 0, 0, 0);
+            auto const issuanceSle = makeIssuance(100);
+
+            auto const result =
+                computeWithdrawByAssets(rules(), vaultSle, issuanceSle, asset(50), env_->journal);
+            BEAST_EXPECT(result.has_value());
+            if (result)
+            {
+                BEAST_EXPECT(result->shares != beast::zero);
+                BEAST_EXPECT(result->assets != beast::zero);
+            }
+        }
+
+        testcase("computeWithdrawByAssets: precision loss");
+        {
+            auto const vaultSle = makeVault(Number{1, 15}, 0, 0, 0);
+            auto const issuanceSle = makeIssuance(1);
+
+            auto const result = computeWithdrawByAssets(
+                rules(), vaultSle, issuanceSle, asset(Number{1, -15}), env_->journal);
+            BEAST_EXPECT(!result.has_value());
+            if (!result)
+                BEAST_EXPECT(result.error() == tecPRECISION_LOSS);
+        }
+    }
+
+    void
+    testComputeWithdrawByShares()
+    {
+        using namespace vault;
+
+        testcase("computeWithdrawByShares: normal");
+        {
+            auto const vaultSle = makeVault(100, 0, 0, 0);
+            auto const issuanceSle = makeIssuance(100);
+
+            auto const result =
+                computeWithdrawByShares(rules(), vaultSle, issuanceSle, shares(50), env_->journal);
+            BEAST_EXPECT(result.has_value());
+            if (result)
+            {
+                BEAST_EXPECT(result->assets != beast::zero);
+                BEAST_EXPECT(result->shares == shares(50));
+            }
+        }
+    }
+
+    void
+    testComputeClawback()
+    {
+        using namespace vault;
+
+        testcase("computeClawback: normal");
+        {
+            auto const vaultSle = makeVault(1000, 0, 0, 0);
+            auto const issuanceSle = makeIssuance(1000);
+
+            auto const result = computeClawback(
+                rules(), vaultSle, issuanceSle, asset(100), Number{1000}, env_->journal);
+            BEAST_EXPECT(result.has_value());
+            if (result)
+            {
+                BEAST_EXPECT(result->assets != beast::zero);
+                BEAST_EXPECT(result->shares != beast::zero);
+            }
+        }
+
+        testcase("computeClawback: clamped to assetsAvailable");
+        {
+            // assetsAvailable is small — clawback should be clamped.
+            auto const vaultSle = makeVault(1000, 0, 0, 0);
+            auto const issuanceSle = makeIssuance(1000);
+
+            auto const result = computeClawback(
+                rules(), vaultSle, issuanceSle, asset(500), Number{10}, env_->journal);
+            BEAST_EXPECT(result.has_value());
+            if (result)
+                BEAST_EXPECT(result->assets <= asset(10));
+        }
+    }
+
+public:
+    void
+    run() override
+    {
+        using namespace jtx;
+        Env env{*this};
+        env_ = &env;
+
+        // v2 tests (amendment enabled by default)
+        testAssetsToSharesDeposit();
+        testSharesToAssetsDeposit();
+        testAssetsToSharesWithdraw();
+        testSharesToAssetsWithdraw();
+
+        // High-level API tests
+        testComputeDeposit();
+        testComputeWithdrawByAssets();
+        testComputeWithdrawByShares();
+        testComputeClawback();
+    }
+};
+
+BEAST_DEFINE_TESTSUITE(VaultHelpers, app, xrpl);
+
+}  // namespace test
+}  // namespace xrpl

src/test/app/Vault_test.cpp

@@ -5253,7 +5253,7 @@ class Vault_test : public beast::unit_test::suite
             testcase("VaultDelete data featureLendingProtocolV1_1 disabled");
             env.disableFeature(featureLendingProtocolV1_1);
             delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength, 'A'));
-            env(delTx, ter(temDISABLED), THISLINE);
+            env(delTx, ter(temDISABLED));
             env.close();
             env.enableFeature(featureLendingProtocolV1_1);
         }
@@ -5262,7 +5262,7 @@ class Vault_test : public beast::unit_test::suite
         {
             testcase("VaultDelete data featureLendingProtocolV1_1 enabled data too large");
             delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength + 1, 'A'));
-            env(delTx, ter(temMALFORMED), THISLINE);
+            env(delTx, ter(temMALFORMED));
             env.close();
         }
 
@@ -5270,7 +5270,7 @@ class Vault_test : public beast::unit_test::suite
         {
             testcase("VaultDelete data featureLendingProtocolV1_1 enabled data empty");
             delTx[sfMemoData] = strHex(std::string(0, 'A'));
-            env(delTx, ter(temMALFORMED), THISLINE);
+            env(delTx, ter(temMALFORMED));
             env.close();
         }
 
@@ -5278,12 +5278,12 @@ class Vault_test : public beast::unit_test::suite
             testcase("VaultDelete data featureLendingProtocolV1_1 enabled data valid");
             PrettyAsset const xrpAsset = xrpIssue();
             auto [tx, keylet] = vault.create({.owner = owner, .asset = xrpAsset});
-            env(tx, ter(tesSUCCESS), THISLINE);
+            env(tx, ter(tesSUCCESS));
             env.close();
             // Recreate the transaction as the vault keylet changed
             auto delTx = vault.del({.owner = owner, .id = keylet.key});
             delTx[sfMemoData] = strHex(std::string(maxDataPayloadLength, 'A'));
-            env(delTx, ter(tesSUCCESS), THISLINE);
+            env(delTx, ter(tesSUCCESS));
             env.close();
         }
     }