Skip to content

Commit

Permalink
fix(Vault): cast balance updates to uint96
Browse files Browse the repository at this point in the history
  • Loading branch information
PierrickGT committed Oct 3, 2023
1 parent dcaf0d7 commit cde015a
Show file tree
Hide file tree
Showing 15 changed files with 49 additions and 52 deletions.
2 changes: 1 addition & 1 deletion lib/brokentoken
2 changes: 1 addition & 1 deletion lib/pt-v5-prize-pool
21 changes: 11 additions & 10 deletions src/Vault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ contract Vault is IERC4626, ERC20Permit, ILiquidationSource, IClaimable, Ownable
/* ============ Variables ============ */

/// The maximum amount of shares that can be minted.
uint256 private constant UINT112_MAX = type(uint112).max;
uint256 private constant UINT96_MAX = type(uint96).max;

/// @notice Address of the underlying asset used by the Vault.
IERC20 private immutable _asset;
Expand Down Expand Up @@ -751,7 +751,10 @@ contract Vault is IERC4626, ERC20Permit, ILiquidationSource, IClaimable, Ownable
}

/// @inheritdoc ILiquidationSource
function isLiquidationPair(address _tokenOut, address liquidationPair_) external view returns (bool) {
function isLiquidationPair(
address _tokenOut,
address liquidationPair_
) external view returns (bool) {
return _tokenOut == address(this) && liquidationPair_ == _liquidationPair;
}

Expand Down Expand Up @@ -860,9 +863,7 @@ contract Vault is IERC4626, ERC20Permit, ILiquidationSource, IClaimable, Ownable
* @param liquidationPair_ New liquidationPair address
* @return address New liquidationPair address
*/
function setLiquidationPair(
address liquidationPair_
) external onlyOwner returns (address) {
function setLiquidationPair(address liquidationPair_) external onlyOwner returns (address) {
if (address(liquidationPair_) == address(0)) revert LPZeroAddress();

_liquidationPair = liquidationPair_;
Expand Down Expand Up @@ -1108,12 +1109,12 @@ contract Vault is IERC4626, ERC20Permit, ILiquidationSource, IClaimable, Ownable

/**
* @notice Returns the maximum amount of underlying assets that can be deposited into the Vault.
* @dev We use type(uint112).max cause this is the type used to store balances in TwabController.
* @dev We use type(uint96).max cause this is the type used to store balances in TwabController.
* @param _depositedAssets Assets deposited into the YieldVault
* @return uint256 Amount of underlying assets that can be deposited
*/
function _maxDeposit(uint256 _depositedAssets) internal view returns (uint256) {
uint256 _vaultMaxDeposit = UINT112_MAX - _depositedAssets;
uint256 _vaultMaxDeposit = UINT96_MAX - _depositedAssets;
uint256 _yieldVaultMaxDeposit = _yieldVault.maxDeposit(address(this));

// Vault shares are minted 1:1 when the vault is collateralized,
Expand Down Expand Up @@ -1384,7 +1385,7 @@ contract Vault is IERC4626, ERC20Permit, ILiquidationSource, IClaimable, Ownable
* @param _shares Shares to mint
*/
function _mint(address _receiver, uint256 _shares) internal virtual override {
_twabController.mint(_receiver, SafeCast.toUint112(_shares));
_twabController.mint(_receiver, SafeCast.toUint96(_shares));
emit Transfer(address(0), _receiver, _shares);
}

Expand All @@ -1397,7 +1398,7 @@ contract Vault is IERC4626, ERC20Permit, ILiquidationSource, IClaimable, Ownable
* @param _shares The shares to burn
*/
function _burn(address _owner, uint256 _shares) internal virtual override {
_twabController.burn(_owner, SafeCast.toUint112(_shares));
_twabController.burn(_owner, SafeCast.toUint96(_shares));
emit Transfer(_owner, address(0), _shares);
}

Expand All @@ -1411,7 +1412,7 @@ contract Vault is IERC4626, ERC20Permit, ILiquidationSource, IClaimable, Ownable
* @param _shares Shares to transfer
*/
function _transfer(address _from, address _to, uint256 _shares) internal virtual override {
_twabController.transfer(_from, _to, SafeCast.toUint112(_shares));
_twabController.transfer(_from, _to, SafeCast.toUint96(_shares));
emit Transfer(_from, _to, _shares);
}

Expand Down
6 changes: 3 additions & 3 deletions test/contracts/external/LiquidatorLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,10 @@ library LiquidatorLib {
uint256 reserve1_1 = FixedMathLib.div(_amountIn1, _liquidityFraction);

// Ensure we can fit K into a uint256
// Ensure new virtual reserves fit into uint112
// Ensure new virtual reserves fit into uint96
if (
reserve0_1 <= type(uint112).max &&
reserve1_1 <= type(uint112).max &&
reserve0_1 <= type(uint96).max &&
reserve1_1 <= type(uint96).max &&
uint256(reserve1_1) * reserve0_1 > _minK
) {
reserve0 = uint128(reserve0_1);
Expand Down
26 changes: 13 additions & 13 deletions test/fuzz/Vault.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,8 @@ contract VaultFuzzTest is ERC4626Test, Helpers {
}

function test_liquidate(Init memory init, uint256 shares) public virtual {
// We set the higher bound to uint104 to avoid overflowing above uint112
init.yield = int256(bound(shares, 10e18, type(uint104).max));
// We set the higher bound to uint88 to avoid overflowing above uint96
init.yield = int256(bound(shares, 10e18, type(uint88).max));

setUpVault(init);
vault.setLiquidationPair(address(liquidationPair));
Expand Down Expand Up @@ -368,15 +368,15 @@ contract VaultFuzzTest is ERC4626Test, Helpers {
}
}

// We set the higher bound to uint104 to avoid overflowing above uint112
// We set the higher bound to uint88 to avoid overflowing above uint96
function setUpVault(Init memory init) public virtual override {
for (uint256 i = 0; i < N; i++) {
init.user[i] = makeAddr(Strings.toString(i));
address user = init.user[i];

vm.assume(_isEOA(user));

uint256 shares = bound(init.share[i], 0, type(uint104).max);
uint256 shares = bound(init.share[i], 0, type(uint88).max);
try IMockERC20(_underlying_).mint(user, shares) {} catch {
vm.assume(false);
}
Expand All @@ -388,7 +388,7 @@ contract VaultFuzzTest is ERC4626Test, Helpers {
vm.assume(false);
}

uint256 assets = bound(init.asset[i], 0, type(uint104).max);
uint256 assets = bound(init.asset[i], 0, type(uint88).max);
try IMockERC20(_underlying_).mint(user, assets) {} catch {
vm.assume(false);
}
Expand All @@ -398,22 +398,22 @@ contract VaultFuzzTest is ERC4626Test, Helpers {
}

function _max_deposit(address from) internal virtual override returns (uint256) {
if (_unlimitedAmount) return type(uint104).max;
return uint104(IERC20(_underlying_).balanceOf(from));
if (_unlimitedAmount) return type(uint88).max;
return uint88(IERC20(_underlying_).balanceOf(from));
}

function _max_mint(address from) internal virtual override returns (uint256) {
if (_unlimitedAmount) return type(uint112).max;
return uint104(vault_convertToShares(IERC20(_underlying_).balanceOf(from)));
if (_unlimitedAmount) return type(uint96).max;
return uint88(vault_convertToShares(IERC20(_underlying_).balanceOf(from)));
}

function _max_withdraw(address from) internal virtual override returns (uint256) {
if (_unlimitedAmount) return type(uint104).max;
return uint104(vault_convertToAssets(IERC20(_vault_).balanceOf(from)));
if (_unlimitedAmount) return type(uint88).max;
return uint88(vault_convertToAssets(IERC20(_vault_).balanceOf(from)));
}

function _max_redeem(address from) internal virtual override returns (uint256) {
if (_unlimitedAmount) return type(uint104).max;
return uint104(IERC20(_vault_).balanceOf(from));
if (_unlimitedAmount) return type(uint88).max;
return uint88(IERC20(_vault_).balanceOf(from));
}
}
4 changes: 2 additions & 2 deletions test/unit/Vault/Deposit.feature
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ Feature: Deposit
# Deposit - Errors
Scenario: Alice deposits into the Vault
Given Alice owns 0 Vault shares
When Alice deposits type(uint112).max + 1 underlying assets
When Alice deposits type(uint96).max + 1 underlying assets
Then the transaction reverts with the custom error `DepositMoreThanMax`

Scenario: Alice deposits into the Vault
Expand Down Expand Up @@ -110,7 +110,7 @@ Feature: Deposit
# Mint - Errors
Scenario: Alice mints shares from the Vault
Given Alice owns 0 Vault shares
When Alice mints type(uint112).max + 1 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
Expand Down
8 changes: 4 additions & 4 deletions test/unit/Vault/Deposit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -185,13 +185,13 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken {
function testDepositMoreThanMax() external {
vm.startPrank(alice);

uint256 _amount = uint256(type(uint112).max) + 1;
uint256 _amount = uint256(type(uint96).max) + 1;

underlyingAsset.mint(alice, _amount);
underlyingAsset.approve(address(vault), type(uint256).max);

vm.expectRevert(
abi.encodeWithSelector(Vault.DepositMoreThanMax.selector, alice, _amount, type(uint112).max)
abi.encodeWithSelector(Vault.DepositMoreThanMax.selector, alice, _amount, type(uint96).max)
);

vault.deposit(_amount, alice);
Expand Down Expand Up @@ -384,13 +384,13 @@ contract VaultDepositTest is UnitBaseSetup, BrokenToken {
function testMintMoreThanMax() external {
vm.startPrank(alice);

uint256 _amount = uint256(type(uint112).max) + 1;
uint256 _amount = uint256(type(uint96).max) + 1;

underlyingAsset.mint(alice, _amount);
underlyingAsset.approve(address(vault), type(uint256).max);

vm.expectRevert(
abi.encodeWithSelector(Vault.MintMoreThanMax.selector, alice, _amount, type(uint112).max)
abi.encodeWithSelector(Vault.MintMoreThanMax.selector, alice, _amount, type(uint96).max)
);

vault.mint(_amount, alice);
Expand Down
4 changes: 2 additions & 2 deletions test/unit/Vault/DepositBrokenToken.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ contract VaultDepositBrokenTokenTest is UnitBrokenTokenBaseSetup {
vm.startPrank(alice);
underlyingAsset.approve(address(vault), type(uint256).max);

// Token with 50 decimals, amount is greater than type(uint112).max
// Token with 50 decimals, amount is greater than type(uint96).max
if (brokenERC20Name == keccak256(bytes("HighDecimalToken"))) {
vm.expectRevert(
abi.encodeWithSelector(Vault.DepositMoreThanMax.selector, alice, _amount, type(uint112).max)
abi.encodeWithSelector(Vault.DepositMoreThanMax.selector, alice, _amount, type(uint96).max)
);

vault.deposit(_amount, alice);
Expand Down
2 changes: 1 addition & 1 deletion test/unit/Vault/Liquidate.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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(uint112).max Vault shares and 10e18 of yield fee shares have accrued
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 error SafeCast: value doesn't fit in 112 bits
10 changes: 5 additions & 5 deletions test/unit/Vault/Liquidate.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ contract VaultLiquidateTest is UnitBaseSetup {
vault.setYieldFeeRecipient(bob);

uint256 _yield = 10e18;
uint256 _amount = type(uint112).max - _yield;
uint256 _amount = type(uint96).max - _yield;

underlyingAsset.mint(address(this), _amount);
_sponsor(underlyingAsset, vault, _amount);
Expand Down Expand Up @@ -361,7 +361,7 @@ contract VaultLiquidateTest is UnitBaseSetup {

assertEq(vault.balanceOf(bob), 0);

// _yieldFeeShares has not been minted yet, so totatSupply is type(uint112).max - _yield
// _yieldFeeShares has not been minted yet, so totatSupply is type(uint96).max - _yield
assertEq(vault.totalSupply(), _amount);
assertEq(vault.yieldFeeShares(), _yieldFeeShares);

Expand Down Expand Up @@ -509,15 +509,15 @@ contract VaultLiquidateTest is UnitBaseSetup {
vm.stopPrank();
}

function testTransferTokensOut_AmountOutGTUint112() public {
function testTransferTokensOut_AmountOutGTuint96() public {
_setLiquidationPair();

uint256 _amount = 1000e18;

underlyingAsset.mint(address(this), _amount);
_sponsor(underlyingAsset, vault, _amount);

uint256 _amountOut = type(uint120).max;
uint256 _amountOut = type(uint104).max;
_accrueYield(underlyingAsset, yieldVault, _amountOut);

vm.startPrank(address(alice));
Expand All @@ -533,7 +533,7 @@ contract VaultLiquidateTest is UnitBaseSetup {

vm.startPrank(address(liquidationPair));

vm.expectRevert(bytes("SafeCast: value doesn't fit in 112 bits"));
vm.expectRevert(bytes("SafeCast: value doesn't fit in 96 bits"));

vault.transferTokensOut(address(this), alice, address(vault), _amountOut);

Expand Down
8 changes: 2 additions & 6 deletions test/unit/Vault/Vault.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -381,9 +381,7 @@ contract VaultTest is UnitBaseSetup {
}

function testSetLiquidationPairOnlyOwner() public {
address _newLiquidationPair = address(
0xff3c527f9F5873bd735878F23Ff7eC5AB2E3b820
);
address _newLiquidationPair = address(0xff3c527f9F5873bd735878F23Ff7eC5AB2E3b820);

vm.startPrank(alice);

Expand All @@ -395,9 +393,7 @@ contract VaultTest is UnitBaseSetup {

/* ============ isLiquidationPair ============ */
function testIsLiquidationPair() public {
address _newLiquidationPair = address(
0xff3c527f9F5873bd735878F23Ff7eC5AB2E3b820
);
address _newLiquidationPair = address(0xff3c527f9F5873bd735878F23Ff7eC5AB2E3b820);

vault.setLiquidationPair(address(liquidationPair));
assertEq(vault.isLiquidationPair(address(vault), address(_newLiquidationPair)), false);
Expand Down

0 comments on commit cde015a

Please sign in to comment.