Skip to content

Commit

Permalink
fix(deposit): revert when minting 0 shares
Browse files Browse the repository at this point in the history
  • Loading branch information
PierrickGT committed Aug 10, 2023
1 parent ae789f4 commit 8eef4cf
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 11 deletions.
6 changes: 6 additions & 0 deletions src/Vault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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));

Expand Down
5 changes: 5 additions & 0 deletions test/unit/Vault/Deposit.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 11 additions & 1 deletion test/unit/Vault/Deposit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 ============ */
Expand Down Expand Up @@ -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);
Expand Down
12 changes: 2 additions & 10 deletions test/unit/Vault/Undercollateralization.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit 8eef4cf

Please sign in to comment.