From 547840f44d414099048ea394a1c2c4cd1de409a4 Mon Sep 17 00:00:00 2001 From: aalavandhann <6264334+aalavandhan@users.noreply.github.com> Date: Thu, 29 Aug 2024 13:15:09 -0400 Subject: [PATCH 1/2] skipping liquidity check when swapping from perps to underlying --- spot-contracts/contracts/RolloverVault.sol | 32 ++++++++----------- .../test/rollover-vault/RolloverVault_swap.ts | 22 +++++++++++-- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/spot-contracts/contracts/RolloverVault.sol b/spot-contracts/contracts/RolloverVault.sol index 75970182..38016754 100644 --- a/spot-contracts/contracts/RolloverVault.sol +++ b/spot-contracts/contracts/RolloverVault.sol @@ -469,7 +469,12 @@ contract RolloverVault is // 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); + // - Absolute balance is strictly greater than `minUnderlyingBal`. + // - Ratio of the balance to the vault's TVL is strictly greater than `minUnderlyingPerc`. + uint256 underlyingBal = underlying_.balanceOf(address(this)); + if (underlyingBal <= minUnderlyingBal || underlyingBal.mulDiv(ONE, s.vaultTVL) <= minUnderlyingPerc) { + revert InsufficientLiquidity(); + } // sync underlying _syncAsset(underlying_); @@ -482,11 +487,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 +509,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 +969,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(); - } - } } diff --git a/spot-contracts/test/rollover-vault/RolloverVault_swap.ts b/spot-contracts/test/rollover-vault/RolloverVault_swap.ts index e2eb17b7..08c5ff06 100644 --- a/spot-contracts/test/rollover-vault/RolloverVault_swap.ts +++ b/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", + ); + }); + }); }); }); From 2e2204c6c2a86961277204c8aa623798d5cd8e0b Mon Sep 17 00:00:00 2001 From: aalavandhann <6264334+aalavandhan@users.noreply.github.com> Date: Thu, 29 Aug 2024 22:12:56 -0400 Subject: [PATCH 2/2] code review fixes --- spot-contracts/contracts/RolloverVault.sol | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/spot-contracts/contracts/RolloverVault.sol b/spot-contracts/contracts/RolloverVault.sol index 38016754..101b76b7 100644 --- a/spot-contracts/contracts/RolloverVault.sol +++ b/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,11 +469,14 @@ 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. + // 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 underlyingBal = underlying_.balanceOf(address(this)); - if (underlyingBal <= minUnderlyingBal || underlyingBal.mulDiv(ONE, s.vaultTVL) <= minUnderlyingPerc) { + uint256 underlyingBalPost = underlying_.balanceOf(address(this)); + if ( + underlyingBalPost < underlyingBalPre && + (underlyingBalPost <= minUnderlyingBal || underlyingBalPost.mulDiv(ONE, s.vaultTVL) <= minUnderlyingPerc) + ) { revert InsufficientLiquidity(); } @@ -511,7 +515,7 @@ contract RolloverVault is // Revert if swap reduces vault's available liquidity. uint256 underlyingBalPost = underlying_.balanceOf(address(this)); - if (underlyingBalPost <= underlyingBalPre) { + if (underlyingBalPost < underlyingBalPre) { revert InsufficientLiquidity(); }