Swap limit bug fix (#221)

* skipping liquidity check when swapping from perps to underlying

* code review fixes

Author: aalavandhan

spot-contracts/contracts/RolloverVault.sol

@@ -440,8 +440,9 @@ contract RolloverVault is
     function swapUnderlyingForPerps(uint256 underlyingAmtIn) external nonReentrant whenNotPaused returns (uint256) {
         // Calculates the fee adjusted perp amount to transfer to the user.
         // NOTE: This operation should precede any token transfers.
-        IERC20Upgradeable underlying_ = underlying;
         IPerpetualTranche perp_ = perp;
+        IERC20Upgradeable underlying_ = underlying;
+        uint256 underlyingBalPre = underlying_.balanceOf(address(this));
         (uint256 perpAmtOut, uint256 perpFeeAmtToBurn, SubscriptionParams memory s) = computeUnderlyingToPerpSwapAmt(
             underlyingAmtIn
         );
@@ -468,8 +469,16 @@ contract RolloverVault is
         // NOTE: In case this operation mints slightly more perps than that are required for the swap,
         // The vault continues to hold the perp dust until the subsequent `swapPerpsForUnderlying` or manual `recover(perp)`.
 
-        // Revert if vault liquidity is too low.
-        _enforceUnderlyingBalAfterSwap(underlying_, s.vaultTVL);
+        // If vault liquidity has reduced, revert if it reduced too much.
+        //  - Absolute balance is strictly greater than `minUnderlyingBal`.
+        //  - Ratio of the balance to the vault's TVL is strictly greater than `minUnderlyingPerc`.
+        uint256 underlyingBalPost = underlying_.balanceOf(address(this));
+        if (
+            underlyingBalPost < underlyingBalPre &&
+            (underlyingBalPost <= minUnderlyingBal || underlyingBalPost.mulDiv(ONE, s.vaultTVL) <= minUnderlyingPerc)
+        ) {
+            revert InsufficientLiquidity();
+        }
 
         // sync underlying
         _syncAsset(underlying_);
@@ -482,11 +491,8 @@ contract RolloverVault is
         // Calculates the fee adjusted underlying amount to transfer to the user.
         IPerpetualTranche perp_ = perp;
         IERC20Upgradeable underlying_ = underlying;
-        (
-            uint256 underlyingAmtOut,
-            uint256 perpFeeAmtToBurn,
-            SubscriptionParams memory s
-        ) = computePerpToUnderlyingSwapAmt(perpAmtIn);
+        uint256 underlyingBalPre = underlying_.balanceOf(address(this));
+        (uint256 underlyingAmtOut, uint256 perpFeeAmtToBurn, ) = computePerpToUnderlyingSwapAmt(perpAmtIn);
 
         // Revert if insufficient tokens are swapped in or out
         if (underlyingAmtOut <= 0 || perpAmtIn <= 0) {
@@ -507,8 +513,11 @@ contract RolloverVault is
         // transfer underlying out
         underlying_.safeTransfer(msg.sender, underlyingAmtOut);
 
-        // Revert if vault liquidity is too low.
-        _enforceUnderlyingBalAfterSwap(underlying_, s.vaultTVL);
+        // Revert if swap reduces vault's available liquidity.
+        uint256 underlyingBalPost = underlying_.balanceOf(address(this));
+        if (underlyingBalPost < underlyingBalPre) {
+            revert InsufficientLiquidity();
+        }
 
         // sync underlying
         _syncAsset(underlying_);
@@ -964,15 +973,4 @@ contract RolloverVault is
         (uint256 trancheClaim, uint256 trancheSupply) = tranche.getTrancheCollateralization(collateralToken);
         return trancheClaim.mulDiv(trancheAmt, trancheSupply, MathUpgradeable.Rounding.Up);
     }
-
-    /// @dev Checks if the vault's underlying balance is above admin defined constraints.
-    ///      - Absolute balance is strictly greater than `minUnderlyingBal`.
-    ///      - Ratio of the balance to the vault's TVL is strictly greater than `minUnderlyingPerc`.
-    ///      NOTE: We assume the vault TVL and the underlying to have the same base denomination.
-    function _enforceUnderlyingBalAfterSwap(IERC20Upgradeable underlying_, uint256 vaultTVL) private view {
-        uint256 underlyingBal = underlying_.balanceOf(address(this));
-        if (underlyingBal <= minUnderlyingBal || underlyingBal.mulDiv(ONE, vaultTVL) <= minUnderlyingPerc) {
-            revert InsufficientLiquidity();
-        }
-    }
 }

spot-contracts/test/rollover-vault/RolloverVault_swap.ts

@@ -114,8 +114,8 @@ describe("RolloverVault", function () {
     );
     expect(await vault.assetCount()).to.eq(2);
 
-    await collateralToken.approve(vault.address, toFixedPtAmt("1000"));
-    await perp.approve(vault.address, toFixedPtAmt("1000"));
+    await collateralToken.approve(vault.address, toFixedPtAmt("10000"));
+    await perp.approve(vault.address, toFixedPtAmt("10000"));
   });
 
   afterEach(async function () {
@@ -1260,5 +1260,23 @@ describe("RolloverVault", function () {
         expect(await vault.callStatic.getTVL()).to.eq(toFixedPtAmt("4434.6153846153846152"));
       });
     });
+
+    describe("when vault reduces underlying liquidity", function () {
+      it("should be reverted", async function () {
+        await feePolicy.computePerpBurnFeePerc.returns(toPercFixedPtAmt("0.1"));
+        await feePolicy.computePerpToUnderlyingVaultSwapFeePerc.returns(toPercFixedPtAmt("0.15"));
+        await vault.swapPerpsForUnderlying(toFixedPtAmt("800"));
+
+        const bond = await getDepositBond(perp);
+        const tranches = await getTranches(bond);
+        await depositIntoBond(bond, toFixedPtAmt("1000"), deployer);
+        await tranches[0].approve(perp.address, toFixedPtAmt("200"));
+        await perp.deposit(tranches[0].address, toFixedPtAmt("200"));
+        await expect(vault.swapPerpsForUnderlying(toFixedPtAmt("1"))).to.be.revertedWithCustomError(
+          vault,
+          "InsufficientLiquidity",
+        );
+      });
+    });
   });
 });