Merge pull request #43 from GenerationSoftware/gen-579-c4-mr-67-vault…

…-will-stop-participating

fix(liquidate): liquidate even if maxMint reached

src/Vault.sol

@@ -58,14 +58,6 @@ error WithdrawMoreThanMax(address owner, uint256 amount, uint256 max);
  */
 error RedeemMoreThanMax(address owner, uint256 amount, uint256 max);
 
-/**
- * @notice Emitted when the amount of shares being minted to the receiver is greater than the max amount allowed.
- * @param receiver The receiver address
- * @param shares The shares being minted
- * @param max The max amount of shares that can be minted to the receiver
- */
-error MintMoreThanMax(address receiver, uint256 shares, uint256 max);
-
 /// @notice Emitted when `_deposit` is called but no shares are minted back to the receiver.
 error MintZeroShares();
 
@@ -149,10 +141,10 @@ error YieldFeeGTAvailableShares(uint256 shares, uint256 yieldFeeShares);
 
 /**
  * @notice Emitted when the minted yield exceeds the amount of available yield in the YieldVault.
- * @param assets The amount of yield assets requested
+ * @param shares The amount of yield shares to mint
  * @param availableYield The amount of yield available
  */
-error YieldFeeGTAvailableYield(uint256 assets, uint256 availableYield);
+error YieldFeeGTAvailableYield(uint256 shares, uint256 availableYield);
 
 /// @notice Emitted when the Liquidation Pair being set is the zero address.
 error LPZeroAddress();
@@ -485,7 +477,7 @@ contract Vault is IERC4626, ERC20Permit, ILiquidationSource, Ownable {
 
   /**
    * @inheritdoc IERC4626
-   * @dev We use type(uint96).max cause this is the type used to store balances in TwabController.
+   * @dev We use type(uint112).max cause this is the type used to store balances in TwabController.
    */
   function maxDeposit(address) public view virtual override returns (uint256) {
     uint256 _depositedAssets = _totalSupply();
@@ -503,17 +495,17 @@ contract Vault is IERC4626, ERC20Permit, ILiquidationSource, Ownable {
 
   /**
    * @inheritdoc IERC4626
-   * @dev We use type(uint96).max cause this is the type used to store balances in TwabController.
+   * @dev We use type(uint112).max cause this is the type used to store balances in TwabController.
    */
   function maxMint(address) public view virtual override returns (uint256) {
     uint256 _depositedAssets = _totalSupply();
 
     if (!_isVaultCollateralized(_depositedAssets, _totalAssets())) return 0;
 
     uint256 _vaultMaxMint = UINT112_MAX - _depositedAssets;
-    uint256 _yieldVaultMaxMint = _yieldVault.maxMint(address(this));
+    uint256 _yieldVaultMaxDeposit = _yieldVault.maxDeposit(address(this));
 
-    return _yieldVaultMaxMint < _vaultMaxMint ? _yieldVaultMaxMint : _vaultMaxMint;
+    return _yieldVaultMaxDeposit < _vaultMaxMint ? _yieldVaultMaxDeposit : _vaultMaxMint;
   }
 
   /// @inheritdoc IERC4626
@@ -1361,27 +1353,23 @@ contract Vault is IERC4626, ERC20Permit, ILiquidationSource, Ownable {
 
   /**
    * @notice Creates `_shares` tokens and assigns them to `_receiver`, increasing the total supply.
-   * @param _receiver Address that will receive the minted shares
-   * @param _shares Shares to mint
    * @dev Emits a {Transfer} event with `from` set to the zero address.
    * @dev `_receiver` cannot be the zero address.
+   * @param _receiver Address that will receive the minted shares
+   * @param _shares Shares to mint
    */
   function _mint(address _receiver, uint256 _shares) internal virtual override {
-    if (_shares > maxMint(_receiver))
-      revert MintMoreThanMax(_receiver, _shares, maxMint(_receiver));
-
     _twabController.mint(_receiver, SafeCast.toUint112(_shares));
-
     emit Transfer(address(0), _receiver, _shares);
   }
 
   /**
    * @notice Destroys `_shares` tokens from `_owner`, reducing the total supply.
-   * @param _owner The owner of the shares
-   * @param _shares The shares to burn
    * @dev Emits a {Transfer} event with `to` set to the zero address.
    * @dev `_owner` cannot be the zero address.
    * @dev `_owner` must have at least `_shares` tokens.
+   * @param _owner The owner of the shares
+   * @param _shares The shares to burn
    */
   function _burn(address _owner, uint256 _shares) internal virtual override {
     _twabController.burn(_owner, SafeCast.toUint112(_shares));
@@ -1391,12 +1379,12 @@ contract Vault is IERC4626, ERC20Permit, ILiquidationSource, Ownable {
 
   /**
    * @notice Updates `_from` and `_to` TWAB balance for a transfer.
-   * @param _from Address to transfer from
-   * @param _to Address to transfer to
-   * @param _shares Shares to transfer
    * @dev `_from` cannot be the zero address.
    * @dev `_to` cannot be the zero address.
    * @dev `_from` must have a balance of at least `_shares`.
+   * @param _from Address to transfer from
+   * @param _to Address to transfer to
+   * @param _shares Shares to transfer
    */
   function _transfer(address _from, address _to, uint256 _shares) internal virtual override {
     _twabController.transfer(_from, _to, SafeCast.toUint112(_shares));
@@ -1445,6 +1433,8 @@ contract Vault is IERC4626, ERC20Permit, ILiquidationSource, Ownable {
     return _withdrawableAssets >= _depositedAssets;
   }
 
+  /* ============ Modifiers ============ */
+
   /// @notice Require reverting if the vault is under-collateralized.
   modifier onlyVaultCollateralized() {
     if (!_isVaultCollateralized(_totalSupply(), _totalAssets())) revert VaultUnderCollateralized();

test/fuzz/Vault.t.sol

@@ -335,8 +335,8 @@ contract VaultFuzzTest is ERC4626Test, Helpers {
   }
 
   function test_liquidate(Init memory init, uint256 shares) public virtual {
-    // We set the higher bound to uint88 to avoid overflowing above uint96
-    init.yield = int(bound(shares, 10e18, type(uint88).max));
+    // We set the higher bound to uint104 to avoid overflowing above uint112
+    init.yield = int(bound(shares, 10e18, type(uint104).max));
 
     setUpVault(init);
     vault.setLiquidationPair(ILiquidationPair(address(liquidationPair)));
@@ -367,15 +367,15 @@ contract VaultFuzzTest is ERC4626Test, Helpers {
     }
   }
 
-  // We set the higher bound to uint88 to avoid overflowing above uint96
+  // We set the higher bound to uint104 to avoid overflowing above uint112
   function setUpVault(Init memory init) public virtual override {
     for (uint i = 0; i < N; i++) {
       init.user[i] = makeAddr(Strings.toString(i));
       address user = init.user[i];
 
       vm.assume(_isEOA(user));
 
-      uint shares = bound(init.share[i], 0, type(uint88).max);
+      uint shares = bound(init.share[i], 0, type(uint104).max);
       try IMockERC20(_underlying_).mint(user, shares) {} catch {
         vm.assume(false);
       }
@@ -387,7 +387,7 @@ contract VaultFuzzTest is ERC4626Test, Helpers {
         vm.assume(false);
       }
 
-      uint assets = bound(init.asset[i], 0, type(uint88).max);
+      uint assets = bound(init.asset[i], 0, type(uint104).max);
       try IMockERC20(_underlying_).mint(user, assets) {} catch {
         vm.assume(false);
       }
@@ -397,22 +397,22 @@ contract VaultFuzzTest is ERC4626Test, Helpers {
   }
 
   function _max_deposit(address from) internal virtual override returns (uint) {
-    if (_unlimitedAmount) return type(uint88).max;
-    return uint88(IERC20(_underlying_).balanceOf(from));
+    if (_unlimitedAmount) return type(uint104).max;
+    return uint104(IERC20(_underlying_).balanceOf(from));
   }
 
   function _max_mint(address from) internal virtual override returns (uint) {
-    if (_unlimitedAmount) return type(uint96).max;
-    return uint88(vault_convertToShares(IERC20(_underlying_).balanceOf(from)));
+    if (_unlimitedAmount) return type(uint112).max;
+    return uint104(vault_convertToShares(IERC20(_underlying_).balanceOf(from)));
   }
 
   function _max_withdraw(address from) internal virtual override returns (uint) {
-    if (_unlimitedAmount) return type(uint88).max;
-    return uint88(vault_convertToAssets(IERC20(_vault_).balanceOf(from)));
+    if (_unlimitedAmount) return type(uint104).max;
+    return uint104(vault_convertToAssets(IERC20(_vault_).balanceOf(from)));
   }
 
   function _max_redeem(address from) internal virtual override returns (uint) {
-    if (_unlimitedAmount) return type(uint88).max;
-    return uint88(IERC20(_vault_).balanceOf(from));
+    if (_unlimitedAmount) return type(uint104).max;
+    return uint104(IERC20(_vault_).balanceOf(from));
   }
 }

test/unit/Vault/Deposit.feature

@@ -57,7 +57,7 @@ Feature: Deposit
   # Deposit - Errors
   Scenario: Alice deposits into the Vault
     Given Alice owns 0 Vault shares
-    When Alice deposits type(uint96).max + 1 underlying assets
+    When Alice deposits type(uint112).max + 1 underlying assets
     Then the transaction reverts with the custom error `DepositMoreThanMax`
 
   Scenario: Alice deposits into the Vault
@@ -115,13 +115,13 @@ Feature: Deposit
   # Mint - Errors
   Scenario: Alice mints shares from the Vault
     Given Alice owns 0 Vault shares
-    When Alice mints type(uint96).max + 1 shares
-    Then the transaction reverts with the custom error MintMoreThanMax
+    When Alice mints type(uint112).max + 1 shares
+    Then the transaction reverts with the error SafeCast: value doesn't fit in 112 bits
 
   Scenario: Alice mints shares from the Vault
     Given Alice owns 0 Vault shares and YieldVault's maxMint function returns type(uint88).max
     When Alice mints type(uint88).max + 1 shares
-    Then the transaction reverts with the custom error MintMoreThanMax
+    Then the transaction reverts with the error ERC4626: deposit more than max
 
   Scenario: Alice mints 0 shares from the Vault
     Given Alice owns 0 Vault shares

test/unit/Vault/Deposit.t.sol

@@ -193,6 +193,7 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken {
     vm.expectRevert(
       abi.encodeWithSelector(DepositMoreThanMax.selector, alice, _amount, type(uint112).max)
     );
+
     vault.deposit(_amount, alice);
 
     vm.stopPrank();
@@ -422,9 +423,7 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken {
     underlyingAsset.mint(alice, _amount);
     underlyingAsset.approve(address(vault), type(uint256).max);
 
-    vm.expectRevert(
-      abi.encodeWithSelector(MintMoreThanMax.selector, alice, _amount, type(uint112).max)
-    );
+    vm.expectRevert(bytes("SafeCast: value doesn't fit in 112 bits"));
 
     vault.mint(_amount, alice);
 
@@ -439,15 +438,15 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken {
     underlyingAsset.mint(alice, _amount);
     underlyingAsset.approve(address(vault), type(uint256).max);
 
-    vm.mockCall(
+    bytes memory _errorMessage = bytes("ERC4626: deposit more than max");
+
+    vm.mockCallRevert(
       address(yieldVault),
-      abi.encodeWithSelector(IERC4626.maxMint.selector, address(vault)),
-      abi.encode(type(uint88).max)
+      abi.encodeWithSelector(IERC4626.deposit.selector, _amount, address(vault)),
+      _errorMessage
     );
 
-    vm.expectRevert(
-      abi.encodeWithSelector(MintMoreThanMax.selector, alice, _amount, type(uint88).max)
-    );
+    vm.expectRevert(_errorMessage);
 
     vault.mint(_amount, alice);
 

test/unit/Vault/DepositBrokenToken.t.sol

@@ -45,6 +45,7 @@ contract VaultDepositBrokenTokenTest is UnitBrokenTokenBaseSetup {
       vm.expectRevert(
         abi.encodeWithSelector(DepositMoreThanMax.selector, alice, _amount, type(uint112).max)
       );
+
       vault.deposit(_amount, alice);
 
       vm.stopPrank();

test/unit/Vault/Liquidate.feature

@@ -88,7 +88,7 @@ Feature: Liquidate
   Scenario: Alice swaps prize tokens in exchange of Vault shares
     Given type(uint104).max underlying assets have accrued in the YieldVault
     When Alice swaps type(uint104).max prize tokens for type(uint104).max Vault shares
-    Then the transaction reverts with the custom error `MintMoreThanMax`
+    Then the transaction reverts with the error SafeCast: value doesn't fit in 112 bits
 
 
   # MintYieldFee - Errors
@@ -98,6 +98,6 @@ Feature: Liquidate
     Then the transaction reverts with the custom error `YieldFeeGTAvailable`
 
   Scenario: Bob mints 1e18 yield fee shares
-    Given Bob owns type(uint96).max Vault shares and 10e18 of yield fee shares have accrued
+    Given Bob owns type(uint112).max Vault shares and 10e18 of yield fee shares have accrued
     When Bob mints 1e18 yield fee shares
-    Then the transaction reverts with the custom error `MintMoreThanMax`
+    Then the transaction reverts with the error SafeCast: value doesn't fit in 112 bits

test/unit/Vault/Liquidate.t.sol

@@ -317,6 +317,76 @@ contract VaultLiquidateTest is UnitBaseSetup {
     assertEq(vault.yieldFeeShares(), 0);
   }
 
+  function testTransferTokensOut_AndMintFees_YieldVaultMaxMintReached() external {
+    _setLiquidationPair();
+
+    vault.setYieldFeePercentage(YIELD_FEE_PERCENTAGE);
+    vault.setYieldFeeRecipient(bob);
+
+    uint256 _yield = 10e18;
+    uint256 _amount = type(uint112).max - _yield;
+
+    underlyingAsset.mint(address(this), _amount);
+    _sponsor(underlyingAsset, vault, _amount);
+
+    _accrueYield(underlyingAsset, yieldVault, _yield);
+
+    vm.startPrank(alice);
+
+    prizeToken.mint(alice, 1000e18);
+
+    uint256 _liquidatedYield = vault.liquidatableBalanceOf(address(vault));
+
+    // Yield Vault has reached its max mint limit
+    vm.mockCall(
+      address(yieldVault),
+      abi.encodeWithSelector(IERC4626.maxMint.selector, address(vault)),
+      abi.encode(type(uint104).max)
+    );
+
+    // Yield has accrued, so despite the max mint limit reached,
+    // we should still be able to liquidate prize tokens in exchange of Vault shares
+    _liquidate(liquidationRouter, liquidationPair, prizeToken, _liquidatedYield, alice);
+
+    assertEq(vault.balanceOf(alice), _liquidatedYield);
+
+    vault.withdraw(_liquidatedYield, alice, alice);
+
+    assertEq(vault.balanceOf(alice), 0);
+    assertEq(underlyingAsset.balanceOf(alice), _liquidatedYield);
+
+    vm.stopPrank();
+
+    uint256 _yieldFeeShares = _getYieldFeeShares(_liquidatedYield, YIELD_FEE_PERCENTAGE);
+
+    assertEq(vault.balanceOf(bob), 0);
+
+    // _yieldFeeShares has not been minted yet, so totatSupply is type(uint112).max - _yield
+    assertEq(vault.totalSupply(), _amount);
+    assertEq(vault.yieldFeeShares(), _yieldFeeShares);
+
+    vm.expectEmit();
+    emit MintYieldFee(address(this), bob, _yieldFeeShares);
+
+    vault.mintYieldFee(_yieldFeeShares);
+
+    assertEq(vault.balanceOf(bob), _yieldFeeShares);
+
+    assertEq(vault.totalSupply(), _amount + _yieldFeeShares);
+    assertEq(vault.yieldFeeShares(), 0);
+
+    vm.startPrank(bob);
+
+    vault.withdraw(_yieldFeeShares, bob, bob);
+
+    assertEq(vault.totalSupply(), _amount);
+
+    assertEq(vault.balanceOf(bob), 0);
+    assertEq(underlyingAsset.balanceOf(bob), _yieldFeeShares);
+
+    vm.stopPrank();
+  }
+
   /* ============ Liquidate - Errors ============ */
   function testTransferTokensOut_YieldVaultUndercollateralized() public {
     _setLiquidationPair();
@@ -433,7 +503,7 @@ contract VaultLiquidateTest is UnitBaseSetup {
     vm.stopPrank();
   }
 
-  function testTransferTokensOut_AmountOutGTMaxMint() public {
+  function testTransferTokensOut_AmountOutGTUint112() public {
     _setLiquidationPair();
 
     uint256 _amount = 1000e18;
@@ -457,14 +527,7 @@ contract VaultLiquidateTest is UnitBaseSetup {
 
     vm.startPrank(address(liquidationPair));
 
-    vm.expectRevert(
-      abi.encodeWithSelector(
-        MintMoreThanMax.selector,
-        alice,
-        _amountOut,
-        type(uint112).max - _amount
-      )
-    );
+    vm.expectRevert(bytes("SafeCast: value doesn't fit in 112 bits"));
 
     vault.transferTokensOut(address(this), alice, address(vault), _amountOut);
 
@@ -515,7 +578,7 @@ contract VaultLiquidateTest is UnitBaseSetup {
 
     vault.deposit(vault.maxDeposit(bob), bob);
 
-    vm.expectRevert(abi.encodeWithSelector(MintMoreThanMax.selector, bob, 1e18, 0));
+    vm.expectRevert();
 
     vault.mintYieldFee(1e18);