Merge pull request #17 from GenerationSoftware/gen-253-c4-issue-272

fix(deposit): revert when minting 0 shares

src/Vault.sol

@@ -65,6 +65,9 @@ error RedeemMoreThanMax(address owner, uint256 amount, uint256 max);
  */
 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();
+
 /// @notice Emitted when `sweep` is called but no underlying assets are currently held by the Vault.
 error SweepZeroAssets();
 
@@ -970,13 +973,16 @@ contract Vault is ERC4626, ERC20Permit, ILiquidationSource, Ownable {
    *      - if `_vaultAssets` balance is greater than or equal to `_assets`,
    *        we know the vault has enough underlying assets to fulfill the deposit
    *        so we don't transfer any assets from the user wallet into the vault
+   * @dev Will revert if 0 shares are minted back to the receiver.
    */
   function _deposit(
     address _caller,
     address _receiver,
     uint256 _assets,
     uint256 _shares
   ) internal virtual override {
+    if (_shares == 0) revert MintZeroShares();
+
     IERC20 _asset = IERC20(asset());
     uint256 _vaultAssets = _asset.balanceOf(address(this));
 

test/unit/Vault/Deposit.feature

@@ -116,6 +116,11 @@ Feature: Deposit
     When Alice mints type(uint88).max + 1 shares
     Then the transaction reverts with the custom error MintMoreThanMax
 
+  Scenario: Alice mints 0 shares from the Vault
+    Given Alice owns 0 Vault shares
+    When Alice mints 0 shares
+    Then the transaction reverts with the custom error MintZeroShares
+
   # Sponsor
   Scenario: Alice sponsors the Vault
     Given Alice owns 0 Vault shares and has not sponsored the Vault

test/unit/Vault/Deposit.t.sol

@@ -5,7 +5,7 @@ import { BrokenToken } from "brokentoken/BrokenToken.sol";
 import { IERC4626 } from "openzeppelin/token/ERC20/extensions/ERC4626.sol";
 
 import { IERC20, UnitBaseSetup } from "../../utils/UnitBaseSetup.t.sol";
-import { MintMoreThanMax, DepositMoreThanMax, SweepZeroAssets } from "../../../src/Vault.sol";
+import { MintMoreThanMax, MintZeroShares, DepositMoreThanMax, SweepZeroAssets } from "../../../src/Vault.sol";
 
 contract VaultDepositTest is UnitBaseSetup, BrokenToken {
   /* ============ Events ============ */
@@ -396,6 +396,16 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken {
     vm.stopPrank();
   }
 
+  function testMintZeroShares() external {
+    vm.startPrank(alice);
+
+    vm.expectRevert(abi.encodeWithSelector(MintZeroShares.selector));
+
+    vault.mint(0, alice);
+
+    vm.stopPrank();
+  }
+
   /* ============ Sponsor ============ */
   function testSponsor() external {
     vm.startPrank(alice);

test/unit/Vault/Undercollateralization.t.sol

@@ -81,30 +81,22 @@ contract VaultUndercollateralizationTest is UnitBaseSetup {
     _deposit(underlyingAsset, vault, _aliceAmount, alice);
     assertEq(vault.balanceOf(alice), _aliceAmount);
 
-    vm.stopPrank();
-
     assertEq(vault.isVaultCollateralized(), true);
 
     // We burn underlying assets from the YieldVault to trigger the undercollateralization
     underlyingAsset.burn(address(yieldVault), 10_000_000e18);
 
+    assertEq(vault.exchangeRate(), 5e17); // 50%
     assertEq(vault.isVaultCollateralized(), false);
 
     assertEq(vault.maxWithdraw(alice), _aliceAmountUndercollateralized);
 
-    vm.startPrank(alice);
-
-    vm.expectEmit();
-    emit RecordedExchangeRate(5e17); // 50%
-
-    // Trigger recorded exchange rate by depositing 0
-    vault.deposit(0, alice);
-
     // After the next withdraw, there will be no assets left in the vault, so the exchange rate should be reset after the shares are burned
     vm.expectEmit();
     emit RecordedExchangeRate(1e18); // 100%
 
     vault.withdraw(vault.maxWithdraw(alice), alice, alice);
+
     assertEq(underlyingAsset.balanceOf(alice), _aliceAmountUndercollateralized);
     assertEq(vault.totalSupply(), 0);