Fix handling of AssetsAvailable in Clawback

Author: Bronek

src/xrpld/app/tx/detail/VaultClawback.cpp

@@ -195,20 +195,27 @@ VaultClawback::doApply()
     }
 
     // Clamp to maximum.
-    Number maxAssets = *vault->at(sfAssetsAvailable);
-    if (assets > maxAssets)
+    Number const assetsAvailable = *vault->at(sfAssetsAvailable);
+    if (assets > assetsAvailable)
     {
-        assets = maxAssets;
-        shares = assetsToSharesWithdraw(vault, sleIssuance, assets);
-        assets = std::min<Number>(
-            sharesToAssetsWithdraw(vault, sleIssuance, shares),
-            maxAssets);
+        assets = assetsAvailable;
+        // Note, it is important to truncate the number of shares, since
+        // otherwise the corresponding assets might breach the AssetsAvailable
+        shares = assetsToSharesWithdraw(
+            vault, sleIssuance, assets, TruncateShares::yes);
+        assets = sharesToAssetsWithdraw(vault, sleIssuance, shares);
+        if (assets > assetsAvailable)
+        {
+            // LCOV_EXCL_START
+            JLOG(j_.error()) << "VaultClawback: invalid rounding of shares.";
+            return tefINTERNAL;
+            // LCOV_EXCL_STOP
+        }
     }
 
     if (shares == beast::zero)
         return tecINSUFFICIENT_FUNDS;
 
-    // As per calculation in assetsToSharesWithdraw and sharesToAssetsWithdraw
     auto const sharesOutstanding = sleIssuance->at(sfOutstandingAmount);
     if (vaultAsset.holds<Issue>() && !vaultAsset.get<Issue>().native() &&
         Number(sharesOutstanding) == shares)

src/xrpld/app/tx/detail/VaultWithdraw.cpp

@@ -226,7 +226,6 @@ VaultWithdraw::doApply()
         return tecINSUFFICIENT_FUNDS;
     }
 
-    // As per calculation in assetsToSharesWithdraw and sharesToAssetsWithdraw
     auto const sharesOutstanding = sleIssuance->at(sfOutstandingAmount);
     if (vaultAsset.holds<Issue>() && !vaultAsset.get<Issue>().native() &&
         Number(sharesOutstanding) == shares)

src/xrpld/ledger/View.h

@@ -930,14 +930,17 @@ sharesToAssetsDeposit(
     std::shared_ptr<SLE const> const& issuance,
     STAmount const& shares);
 
+enum class TruncateShares : bool { no = false, yes = true };
+
 // From the perspective of a vault,
 // return the number of shares to demand from the depositor
 // when they ask to withdraw a fixed amount of assets.
 [[nodiscard]] STAmount
 assetsToSharesWithdraw(
     std::shared_ptr<SLE const> const& vault,
     std::shared_ptr<SLE const> const& issuance,
-    STAmount const& assets);
+    STAmount const& assets,
+    TruncateShares truncate = TruncateShares::no);
 
 // From the perspective of a vault,
 // return the number of assets to give the depositor

src/xrpld/ledger/detail/View.cpp

@@ -2849,7 +2849,8 @@ sharesToAssetsDeposit(
 assetsToSharesWithdraw(
     std::shared_ptr<SLE const> const& vault,
     std::shared_ptr<SLE const> const& issuance,
-    STAmount const& assets)
+    STAmount const& assets,
+    TruncateShares truncate)
 {
     XRPL_ASSERT(
         assets.asset() == vault->at(sfAsset),
@@ -2860,7 +2861,10 @@ assetsToSharesWithdraw(
     if (assetTotal == 0)
         return shares;
     Number shareTotal = issuance->at(sfOutstandingAmount);
-    shares = shareTotal * (assets / assetTotal);
+    Number result = shareTotal * (assets / assetTotal);
+    if (truncate == TruncateShares::yes)
+        result = result.truncate();
+    shares = result;
     return shares;
 }