fix(maxDeposit/Mint): fix ERC4626 compliance

src/Vault.sol

@@ -374,16 +374,26 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable {
    * @inheritdoc ERC4626
    * @dev We use type(uint96).max cause this is the type used to store balances in TwabController.
    */
-  function maxDeposit(address) public view virtual override returns (uint256) {
-    return _isVaultCollateralized() ? type(uint96).max : 0;
+  function maxDeposit(address recipient) public view virtual override returns (uint256) {
+    if (!_isVaultCollateralized()) return 0;
+
+    uint256 _vaultMaxDeposit = type(uint96).max - _convertToAssets(balanceOf(recipient), Math.Rounding.Down);
+    uint256 _yieldVaultMaxDeposit = _yieldVault.maxDeposit(address(this));
+
+    return _yieldVaultMaxDeposit < _vaultMaxDeposit ? _yieldVaultMaxDeposit : _vaultMaxDeposit;
   }
 
   /**
    * @inheritdoc ERC4626
    * @dev We use type(uint96).max cause this is the type used to store balances in TwabController.
    */
-  function maxMint(address) public view virtual override returns (uint256) {
-    return _isVaultCollateralized() ? type(uint96).max : 0;
+  function maxMint(address recipient) public view virtual override returns (uint256) {
+    if (!_isVaultCollateralized()) return 0;
+
+    uint256 _vaultMaxMint = type(uint96).max - balanceOf(recipient);
+    uint256 _yieldVaultMaxMint = _yieldVault.maxMint(address(this));
+
+    return _yieldVaultMaxMint < _vaultMaxMint ? _yieldVaultMaxMint : _vaultMaxMint;
   }
 
   /**
@@ -439,7 +449,7 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable {
 
   /// @inheritdoc ERC4626
   function mint(uint256 _shares, address _receiver) public virtual override returns (uint256) {
-    uint256 _assets = _beforeMint(_shares, _receiver);
+    uint256 _assets = _convertToAssets(_shares, Math.Rounding.Up);
 
     _deposit(msg.sender, _receiver, _assets, _shares);
 
@@ -464,7 +474,7 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable {
     bytes32 _r,
     bytes32 _s
   ) external returns (uint256) {
-    uint256 _assets = _beforeMint(_shares, _receiver);
+    uint256 _assets = _convertToAssets(_shares, Math.Rounding.Up);
 
     _permit(IERC20Permit(asset()), msg.sender, address(this), _assets, _deadline, _v, _r, _s);
     _deposit(msg.sender, _receiver, _assets, _shares);
@@ -968,17 +978,6 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable {
     emit Deposit(_caller, _receiver, _assets, _shares);
   }
 
-  /**
-   * @notice Compute the amount of assets to deposit before minting `_shares`.
-   * @param _shares Amount of shares to mint
-   * @param _receiver Address of the receiver of the vault shares
-   * @return uint256 Amount of assets to deposit.
-   */
-  function _beforeMint(uint256 _shares, address _receiver) internal view returns (uint256) {
-    if (_shares > maxMint(_receiver)) revert MintMoreThanMax(_receiver, _shares, maxMint(_receiver));
-    return _convertToAssets(_shares, Math.Rounding.Up);
-  }
-
   /**
    * @notice Deposit assets into the Vault and delegate to the sponsorship address.
    * @param _assets Amount of assets to deposit
@@ -1127,6 +1126,8 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable {
    * @dev Updates the exchange rate.
    */
   function _mint(address _receiver, uint256 _shares) internal virtual override {
+    if (_shares > maxMint(_receiver)) revert MintMoreThanMax(_receiver, _shares, maxMint(_receiver));
+
     _twabController.mint(_receiver, SafeCast.toUint96(_shares));
     _updateExchangeRate();
 

test/unit/Vault/Deposit.feature

@@ -57,6 +57,17 @@ Feature: Deposit
     Then the YieldVault must mint to the Vault an amount of shares equivalent to the amount of underlying assets deposited
     Then the Vault `totalSupply` must be equal to 1,000
 
+  # Deposit - Errors
+  Scenario: Alice deposits into the Vault
+    Given Alice owns 0 Vault shares
+    When Alice deposits type(uint96).max + 1 underlying assets
+    Then the transaction reverts with the custom error DepositMoreThanMax
+
+  Scenario: Alice deposits into the Vault
+    Given Alice owns 0 Vault shares and YieldVault's maxDeposit function returns type(uint88).max
+    When Alice deposits type(uint88).max + 1 underlying assets
+    Then the transaction reverts with the custom error DepositMoreThanMax
+
   # Deposit - Attacks
   # Inflation attack
   Scenario: Bob front runs Alice deposits into the Vault
@@ -118,6 +129,17 @@ Feature: Deposit
     Then the YieldVault must mint to the Vault an amount of shares equivalent to the amount of underlying assets deposited
     Then the Vault `totalSupply` must be equal to 1,000
 
+  # 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
+
+  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
+
   # Sponsor
   Scenario: Alice sponsors the Vault
     Given Alice owns 0 Vault shares and has not sponsored the Vault

test/unit/Vault/Deposit.t.sol

@@ -1,12 +1,11 @@
 // SPDX-License-Identifier: GPL-3.0
 pragma solidity ^0.8.19;
 
-import { MintMoreThanMax, DepositMoreThanMax } from "../../../src/Vault.sol";
-
 import { BrokenToken } from "brokentoken/BrokenToken.sol";
+import { IERC4626 } from "openzeppelin/token/ERC20/extensions/ERC4626.sol";
 
 import { IERC20, UnitBaseSetup } from "../../utils/UnitBaseSetup.t.sol";
-import { console2 } from "forge-std/Test.sol";
+import { MintMoreThanMax, DepositMoreThanMax } from "../../../src/Vault.sol";
 
 contract VaultDepositTest is UnitBaseSetup, BrokenToken {
   /* ============ Events ============ */
@@ -51,20 +50,6 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken {
     vm.stopPrank();
   }
 
-  function testDepositMoreThanMax() external {
-    vm.startPrank(alice);
-
-    uint256 _moreThanMax = uint256(type(uint96).max) + 1;
-    uint256 _amount = _moreThanMax;
-    underlyingAsset.mint(alice, _amount);
-    underlyingAsset.approve(address(vault), type(uint256).max);
-
-    vm.expectRevert(abi.encodeWithSelector(DepositMoreThanMax.selector, alice, _amount, type(uint96).max));
-    vault.deposit(_amount, alice);
-
-    vm.stopPrank();
-  }
-
   function testDepositAssetsLivingInVault() external {
     uint256 _vaultAmount = 500e18;
     underlyingAsset.mint(address(vault), _vaultAmount);
@@ -187,6 +172,42 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken {
     vm.stopPrank();
   }
 
+  /* ============ Deposit - Errors ============ */
+  function testDepositMoreThanMax() external {
+    vm.startPrank(alice);
+
+    uint256 _amount = uint256(type(uint96).max) + 1;
+
+    underlyingAsset.mint(alice, _amount);
+    underlyingAsset.approve(address(vault), type(uint256).max);
+
+    vm.expectRevert(abi.encodeWithSelector(DepositMoreThanMax.selector, alice, _amount, type(uint96).max));
+    vault.deposit(_amount, alice);
+
+    vm.stopPrank();
+  }
+
+  function testDepositMoreThanYieldVaultMax() external {
+    vm.startPrank(alice);
+
+
+    uint256 _amount = uint256(type(uint88).max) + 1;
+
+    underlyingAsset.mint(alice, _amount);
+    underlyingAsset.approve(address(vault), type(uint256).max);
+
+    vm.mockCall(
+      address(yieldVault),
+      abi.encodeWithSelector(IERC4626.maxDeposit.selector, address(vault)),
+      abi.encode(type(uint88).max)
+    );
+
+    vm.expectRevert(abi.encodeWithSelector(DepositMoreThanMax.selector, alice, _amount, type(uint88).max));
+    vault.deposit(_amount, alice);
+
+    vm.stopPrank();
+  }
+
   /* ============ Deposit - Attacks ============ */
   function testFailDepositInflationAttack() external {
     vm.startPrank(bob);
@@ -385,10 +406,12 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken {
     vm.stopPrank();
   }
 
+  /* ============ Mint - Errors ============ */
   function testMintMoreThanMax() external {
     vm.startPrank(alice);
 
     uint256 _amount = uint256(type(uint96).max) + 1;
+
     underlyingAsset.mint(alice, _amount);
     underlyingAsset.approve(address(vault), type(uint256).max);
 
@@ -399,6 +422,27 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken {
     vm.stopPrank();
   }
 
+  function testMintMoreThanYieldVaultMax() external {
+    vm.startPrank(alice);
+
+    uint256 _amount = uint256(type(uint88).max) + 1;
+
+    underlyingAsset.mint(alice, _amount);
+    underlyingAsset.approve(address(vault), type(uint256).max);
+
+    vm.mockCall(
+      address(yieldVault),
+      abi.encodeWithSelector(IERC4626.maxMint.selector, address(vault)),
+      abi.encode(type(uint88).max)
+    );
+
+    vm.expectRevert(abi.encodeWithSelector(MintMoreThanMax.selector, alice, _amount, type(uint88).max));
+
+    vault.mint(_amount, alice);
+
+    vm.stopPrank();
+  }
+
   /* ============ Sponsor ============ */
   function testSponsor() external {
     vm.startPrank(alice);

test/unit/Vault/Liquidate.feature

@@ -54,7 +54,7 @@ Feature: Liquidate
     Then the Vault total supply must increase by 1
 
 
-  # Failure
+  # Liquidate - Errors
   Scenario: Bob swaps prize tokens in exchange of Vault shares
     Given the YieldVault is now undercollateralized
     When `liquidate` is called
@@ -85,7 +85,19 @@ Feature: Liquidate
     When Bob swaps 0 prize tokens for uint256.max Vault shares through the LiquidationRouter
     Then the transaction reverts with the custom error `LiquidationAmountOutGTYield`
 
-  Scenario: Bob mints an arbitrary amount of yield fee
+  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`
+
+
+  # MintYieldFee - Errors
+  Scenario: Bob mints an arbitrary amount of yield fee shares
     Given no yield fee has accrued
     When Bob mints 10 yield fee shares
     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
+    When Bob mints 1e18 yield fee shares
+    Then the transaction reverts with the custom error `MintMoreThanMax`

test/unit/Vault/Liquidate.t.sol

@@ -317,7 +317,7 @@ contract VaultLiquidateTest is UnitBaseSetup {
     assertEq(vault.yieldFeeTotalSupply(), 0);
   }
 
-  /* ============ Errors ============ */
+  /* ============ Liquidate - Errors ============ */
   function testLiquidateYieldVaultUndercollateralized() public {
     _setLiquidationPair();
 
@@ -412,8 +412,92 @@ contract VaultLiquidateTest is UnitBaseSetup {
     vm.stopPrank();
   }
 
+  function testLiquidateAmountOutGTMaxMint() public {
+    _setLiquidationPair();
+
+    uint256 _amount = 1000e18;
+
+    underlyingAsset.mint(address(this), _amount);
+    _sponsor(underlyingAsset, vault, _amount, address(this));
+
+    uint256 _amountOut = type(uint104).max;
+    _accrueYield(underlyingAsset, yieldVault, _amountOut);
+
+    vm.startPrank(address(alice));
+
+    prizeToken.mint(alice, type(uint256).max);
+    prizeToken.approve(address(this), type(uint256).max);
+
+    vm.stopPrank();
+
+    uint256 _amountIn = liquidationPair.computeExactAmountIn(_amountOut);
+
+    IERC20(address(prizeToken)).transferFrom(
+      alice,
+      address(prizePool),
+      _amountIn
+    );
+
+    vm.startPrank(address(liquidationPair));
+
+    vm.expectRevert(
+      abi.encodeWithSelector(MintMoreThanMax.selector, alice, _amountOut, type(uint96).max)
+    );
+
+    vault.liquidate(alice, address(prizeToken), _amountIn, address(vault), _amountOut);
+
+    vm.stopPrank();
+  }
+
+  /* ============ MintYieldFee - Errors ============ */
   function testMintYieldFeeGTYieldFeeSupply() public {
     vm.expectRevert(abi.encodeWithSelector(YieldFeeGTAvailable.selector, 10e18, 0));
     vault.mintYieldFee(10e18);
   }
+
+  function testMintYieldFeeMoreThanMax() public {
+    _setLiquidationPair();
+
+    vault.setYieldFeePercentage(YIELD_FEE_PERCENTAGE);
+    vault.setYieldFeeRecipient(bob);
+
+    uint256 _amount = 1000e18;
+
+    underlyingAsset.mint(address(this), _amount);
+    _sponsor(underlyingAsset, vault, _amount, address(this));
+
+    uint256 _yield = 10e18;
+    _accrueYield(underlyingAsset, yieldVault, _yield);
+
+    vm.startPrank(alice);
+
+    prizeToken.mint(alice, 1000e18);
+
+    uint256 _liquidatableYield = vault.liquidatableBalanceOf(address(vault));
+
+    _liquidate(
+      liquidationRouter,
+      liquidationPair,
+      prizeToken,
+      _liquidatableYield,
+      alice
+    );
+
+    vm.stopPrank();
+
+    vm.startPrank(bob);
+
+    underlyingAsset.mint(bob, vault.maxDeposit(bob));
+    underlyingAsset.approve(address(vault), vault.maxDeposit(bob));
+
+    vault.deposit(vault.maxDeposit(bob), bob);
+
+    vm.expectRevert(
+      abi.encodeWithSelector(MintMoreThanMax.selector, bob, 1e18, 0)
+    );
+
+    vault.mintYieldFee(1e18);
+
+    vm.stopPrank();
+  }
 }

test/unit/Vault/Withdraw.feature

@@ -43,12 +43,6 @@ Feature: Withdraw
     Then the YieldVault balance of underlying assets must be equal to 0
     Then the Vault `totalSupply` must be equal to 0
 
-  # Withdraw - Attacks
-  Scenario: Bob tries to withdraw more than uint96 to exploit a rounding down error
-    Given Bob owns double the max amount that fits in uint96 Vault shares
-    When Bob `withdraw` his full deposit
-    Then it reverts with the error "SafeCast: value doesn't fit in 96 bits"
-
   # Redeem
   Scenario: Alice redeems her full deposit
     Given Alice owns 1,000 Vault shares