Merge pull request #94 from Ion-Protocol/jun/PAG-M-05

M-05 Fix [PAG]

src/vault/Vault.sol

@@ -460,12 +460,16 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy
 
                 // For the IDLE pool, decrement the accumulator at the end of this
                 // loop, but no external interactions need to be made as the assets
-                // are already on this contract' balance.
+                // are already on this contract' balance. If the pool supply
+                // reverts, simply skip to the next iteration.
                 if (pool != IDLE) {
-                    pool.supply(address(this), toSupply, new bytes32[](0));
+                    try pool.supply(address(this), toSupply, new bytes32[](0)) {
+                        assets -= toSupply;
+                    } catch { }
+                } else {
+                    assets -= toSupply;
                 }
 
-                assets -= toSupply;
                 if (assets == 0) return;
             }
 
@@ -497,12 +501,17 @@ contract Vault is ERC4626, Multicall, AccessControlDefaultAdminRules, Reentrancy
                 uint256 toWithdraw = Math.min(withdrawable, assets);
 
                 // For the `IDLE` pool, they are already on this contract's
-                // balance. Update `assets` accumulator but don't actually transfer.
+                // balance. Update `assets` accumulator but don't actually
+                // transfer. If the pool withdraw reverts, simply skip to the
+                // next iteration.
                 if (pool != IDLE) {
-                    pool.withdraw(address(this), toWithdraw);
+                    try pool.withdraw(address(this), toWithdraw) {
+                        assets -= toWithdraw;
+                    } catch { }
+                } else {
+                    assets -= toWithdraw;
                 }
 
-                assets -= toWithdraw;
                 if (assets == 0) return;
             }
 

test/helpers/VaultSharedSetup.sol

@@ -289,4 +289,10 @@ contract VaultSharedSetup is IonPoolSharedSetup {
         IonPoolExposed(address(rsEthIonPool)).setSupplyFactor(7.1336673e27);
         IonPoolExposed(address(rswEthIonPool)).setSupplyFactor(10.1336673e27);
     }
+
+    function _zeroFloorSub(uint256 x, uint256 y) internal pure returns (uint256 z) {
+        assembly {
+            z := mul(gt(x, y), sub(x, y))
+        }
+    }
 }

test/unit/concrete/vault/Vault.t.sol

@@ -797,6 +797,46 @@ abstract contract VaultDeposit is VaultSharedSetup {
         assertEq(assetsDeposited, expectedDepositAmount, "mint return value");
     }
 
+    function test_Deposit_SkipPauseReverts() public {
+        // If the market is paused, then the deposit iteration should skip it.
+        updateAllocationCaps(vault, 10 ether, 20 ether, 30 ether);
+
+        uint256 depositAmt = 30 ether;
+        setERC20Balance(address(BASE_ASSET), address(this), depositAmt);
+
+        // The second market is paused.
+        rsEthIonPool.pause();
+
+        // 10 ether in weEthIonPool, 20 ether in rswEthIonPool
+        vault.deposit(depositAmt, address(this));
+
+        assertEq(
+            weEthIonPool.balanceOf(address(vault)),
+            claimAfterDeposit(0, 10 ether, weEthIonPool.supplyFactor()),
+            "weEthIonPool balance"
+        );
+        assertEq(rsEthIonPool.balanceOf(address(vault)), 0, "rsEthIonPool balance");
+        assertEq(
+            rswEthIonPool.balanceOf(address(vault)),
+            claimAfterDeposit(0, 20 ether, rswEthIonPool.supplyFactor()),
+            "rswEthIonPool balance"
+        );
+    }
+
+    function test_Deposit_AllMarketsPaused() public {
+        updateAllocationCaps(vault, 10 ether, 20 ether, 30 ether);
+
+        weEthIonPool.pause();
+        rsEthIonPool.pause();
+        rswEthIonPool.pause();
+
+        uint256 depositAmt = 30 ether;
+        setERC20Balance(address(BASE_ASSET), address(this), depositAmt);
+
+        vm.expectRevert(Vault.AllSupplyCapsReached.selector);
+        vault.deposit(depositAmt, address(this));
+    }
+
     function test_Mint_AllMarkets() public { }
 }
 
@@ -934,6 +974,42 @@ abstract contract VaultWithdraw is VaultSharedSetup {
         vault.withdraw(withdrawAmount, address(this), address(this));
     }
 
+    function test_Withdraw_SkipPauseReverts() public {
+        uint256 depositAmt = 6e18;
+        uint256 withdrawAmt = 3e18;
+
+        updateAllocationCaps(vault, 1e18, 2e18, 3e18);
+
+        // Deposit 1e18 to weETH, 2e18 to rsETH, 3e18 to rswETH
+        // rsETH is paused
+        // Withdraw 1e18 from weETH, 0 from rsETH, 2e18 from rswETH
+
+        setERC20Balance(address(BASE_ASSET), address(this), depositAmt);
+        vault.deposit(depositAmt, address(this));
+
+        uint256 initialWeEthIonPoolDeposit = weEthIonPool.balanceOf(address(vault));
+        uint256 initialRsEthIonPoolDeposit = rsEthIonPool.balanceOf(address(vault));
+        uint256 initialRswEthIonPoolDeposit = rswEthIonPool.balanceOf(address(vault));
+
+        rsEthIonPool.pause();
+
+        vault.withdraw(withdrawAmt, address(this), address(this));
+
+        uint256 rswEthIonPoolWithdrawAmt = withdrawAmt - initialWeEthIonPoolDeposit;
+        uint256 expectedRswEthIonPoolDeposit = initialRswEthIonPoolDeposit - rswEthIonPoolWithdrawAmt;
+        uint256 rswEthIonPoolDeposit = rswEthIonPool.balanceOf(address(vault));
+
+        assertEq(weEthIonPool.balanceOf(address(vault)), 0, "weEthIonPool balance");
+        assertEq(
+            rsEthIonPool.balanceOf(address(vault)), initialRsEthIonPoolDeposit, "rsEthIonPool deposit should not change"
+        );
+        assertLe(
+            expectedRswEthIonPoolDeposit - rswEthIonPoolDeposit,
+            rswEthIonPool.supplyFactor() / RAY,
+            "rswEthIonPool balance"
+        );
+    }
+
     // try to deposit and withdraw same amounts
     function test_Withdraw_FullWithdraw() public { }
 
@@ -1600,7 +1676,6 @@ contract VaultERC4626ExternalViews is VaultSharedSetup {
         vault.updateAllocationCaps(markets, allocationCaps);
 
         uint256 maxMintShares = vault.maxMint(NULL);
-        console2.log("maxMintShares: ", maxMintShares);
 
         setERC20Balance(address(BASE_ASSET), address(this), 60e18);
         vault.mint(maxMintShares, address(this));

test/unit/fuzz/vault/Vault.t.sol

@@ -11,6 +11,7 @@ import { RAY } from "./../../../../src/libraries/math/WadRayMath.sol";
 import { IERC20 } from "openzeppelin-contracts/contracts/interfaces/IERC20.sol";
 
 using Math for uint256;
+using WadRayMath for uint256;
 
 contract Vault_Fuzz is VaultSharedSetup {
     function setUp() public override {
@@ -56,6 +57,78 @@ contract Vault_Fuzz is VaultSharedSetup {
 
         assertEq(normalizedAmt, sharesToBurn);
     }
+
+    // NOTE Supplying the diff can revert if the normalized mint amount
+    // truncates to zero. Otherwise, it should be impossible to supply the
+    // 'diff' and end up violating the supply cap.
+
+    function testFuzz_DepositToFillSupplyCap(uint256 assets, uint256 supplyFactor) public {
+        supplyFactor = bound(supplyFactor, 1e27, 10e27);
+        IonPoolExposed(address(weEthIonPool)).setSupplyFactor(supplyFactor);
+
+        uint256 supplyCap = bound(assets, 100e18, type(uint128).max);
+        weEthIonPool.updateSupplyCap(supplyCap);
+
+        uint256 initialDeposit = bound(assets, 1e18, supplyCap - 10e18);
+        supply(address(this), weEthIonPool, initialDeposit);
+        uint256 initialTotalNormalized = weEthIonPool.totalSupplyUnaccrued();
+
+        uint256 supplyCapDiff = _zeroFloorSub(supplyCap, weEthIonPool.getTotalUnderlyingClaims());
+
+        // `IonPool.supply` math
+        uint256 amountScaled = supplyCapDiff.rayDivDown(supplyFactor);
+        uint256 resultingTotalNormalized = initialTotalNormalized + amountScaled;
+
+        uint256 resultingTotalClaim = resultingTotalNormalized.rayMulDown(supplyFactor);
+
+        supply(address(this), weEthIonPool, supplyCapDiff);
+
+        assertEq(
+            resultingTotalClaim, weEthIonPool.getTotalUnderlyingClaims(), "resulting should be the same as calculated"
+        );
+
+        // Is it possible that depositing this supplyCapDiff results in a revert?
+        // `IonPool` compares `getTotalUnderlyingClaims > _supplyCap`
+        assertLe(resultingTotalClaim, supplyCap, "supply cap reached");
+        assertLe(weEthIonPool.getTotalUnderlyingClaims(), supplyCap, "supply cap reached");
+    }
+
+    // Supplying the diff in the allocation cap should never end up violating
+    // the allocation cap.
+    // Is it possible that the `maxDeposit` returns more than the allocation cap?
+    function testFuzz_DepositToFillAllocationCap(uint256 assets, uint256 supplyFactor) public {
+        supplyFactor = bound(supplyFactor, 1e27, 10e27);
+        IonPoolExposed(address(weEthIonPool)).setSupplyFactor(supplyFactor);
+
+        uint256 allocationCap = bound(assets, 100e18, type(uint128).max);
+        updateAllocationCaps(vault, allocationCap, type(uint128).max, 0);
+
+        // Deposit, but leave some room below the allocation cap.
+        uint256 depositAmt = bound(assets, 1e18, allocationCap - 10e18);
+        setERC20Balance(address(BASE_ASSET), address(this), depositAmt);
+        vault.deposit(depositAmt, address(this));
+
+        uint256 initialTotalNormalized = weEthIonPool.totalSupplyUnaccrued();
+
+        uint256 allocationCapDiff = _zeroFloorSub(allocationCap, weEthIonPool.getUnderlyingClaimOf(address(vault)));
+
+        uint256 amountScaled = allocationCapDiff.rayDivDown(supplyFactor);
+        uint256 resultingTotalNormalized = initialTotalNormalized + amountScaled;
+        uint256 resultingTotalClaim = resultingTotalNormalized.rayMulDown(supplyFactor);
+
+        // Try to deposit a little more than the first allocation cap would
+        // allow, then check whether it's possible to violate the first
+        // allocation cap.
+
+        setERC20Balance(address(BASE_ASSET), address(this), allocationCapDiff + 123e18);
+        vault.deposit(allocationCapDiff + 123e18, address(this));
+
+        uint256 actualTotalClaim = weEthIonPool.getUnderlyingClaimOf(address(vault));
+        assertEq(resultingTotalClaim, actualTotalClaim, "expected and actual must be equal");
+
+        assertLe(resultingTotalClaim, allocationCap, "expected claim le to allocation cap");
+        assertLe(actualTotalClaim, allocationCap, "actual claim le to allocation cap");
+    }
 }
 
 contract VaultWithYieldAndFee_Fuzz is VaultSharedSetup {