diff --git a/ape-config.yaml b/ape-config.yaml index 9affa89f..a787aade 100644 --- a/ape-config.yaml +++ b/ape-config.yaml @@ -9,7 +9,7 @@ default_ecosystem: ethereum dependencies: - name: openzeppelin github: OpenZeppelin/openzeppelin-contracts - ref: 4.7.3 + ref: 4.9.5 - name: tokenized-strategy github: yearn/tokenized-strategy ref: dev_302 @@ -17,7 +17,7 @@ dependencies: solidity: import_remapping: - - "@openzeppelin/contracts=openzeppelin/v4.7.3" + - "@openzeppelin/contracts=openzeppelin/v4.9.5" - "@tokenized-strategy=tokenized-strategy/dev_302" ethereum: diff --git a/contracts/VaultV3.vy b/contracts/VaultV3.vy index 7b727fec..dddf0cb5 100644 --- a/contracts/VaultV3.vy +++ b/contracts/VaultV3.vy @@ -729,8 +729,11 @@ def _mint(sender: address, recipient: address, shares: uint256) -> uint256: def _assess_share_of_unrealised_losses(strategy: address, assets_needed: uint256) -> uint256: """ Returns the share of losses that a user would take if withdrawing from this strategy + This accounts for losses that have been realized at the strategy level but not yet + realized at the vault level. + e.g. if the strategy has unrealised losses for 10% of its current debt and the user - wants to withdraw 1000 tokens, the losses that he will take are 100 token + wants to withdraw 1_000 tokens, the losses that they will take is 100 token """ # Minimum of how much debt the debt should be worth. strategy_current_debt: uint256 = self.strategies[strategy].current_debt @@ -746,12 +749,12 @@ def _assess_share_of_unrealised_losses(strategy: address, assets_needed: uint256 # but will only receive assets_to_withdraw. # NOTE: If there are unrealised losses, the user will take his share. numerator: uint256 = assets_needed * strategy_assets - losses_user_share: uint256 = assets_needed - numerator / strategy_current_debt + users_share_of_loss: uint256 = assets_needed - numerator / strategy_current_debt # Always round up. if numerator % strategy_current_debt != 0: - losses_user_share += 1 + users_share_of_loss += 1 - return losses_user_share + return users_share_of_loss @internal def _withdraw_from_strategy(strategy: address, assets_to_withdraw: uint256): @@ -777,38 +780,37 @@ def _redeem( receiver: address, owner: address, assets: uint256, - shares_to_burn: uint256, + shares: uint256, max_loss: uint256, strategies: DynArray[address, MAX_QUEUE] ) -> uint256: """ This will attempt to free up the full amount of assets equivalent to - `shares_to_burn` and transfer them to the `receiver`. If the vault does + `shares` and transfer them to the `receiver`. If the vault does not have enough idle funds it will go through any strategies provided by - either the withdrawer or the queue_manager to free up enough funds to + either the withdrawer or the default_queue to free up enough funds to service the request. The vault will attempt to account for any unrealized losses taken on from strategies since their respective last reports. Any losses realized during the withdraw from a strategy will be passed on - to the user that is redeeming their vault shares. + to the user that is redeeming their vault shares unless it exceeds the given + `max_loss`. """ assert receiver != empty(address), "ZERO ADDRESS" + assert shares > 0, "no shares to redeem" + assert assets > 0, "no assets to withdraw" assert max_loss <= MAX_BPS, "max loss" - + # If there is a withdraw limit module, check the max. if self.withdraw_limit_module != empty(address): assert assets <= self._max_withdraw(owner, max_loss, strategies), "exceed withdraw limit" - shares: uint256 = shares_to_burn - shares_balance: uint256 = self.balance_of[owner] - - assert shares > 0, "no shares to redeem" - assert shares_balance >= shares, "insufficient shares to redeem" + assert self.balance_of[owner] >= shares, "insufficient shares to redeem" if sender != owner: - self._spend_allowance(owner, sender, shares_to_burn) + self._spend_allowance(owner, sender, shares) # The amount of the underlying token to withdraw. requested_assets: uint256 = assets @@ -852,7 +854,7 @@ def _redeem( assets_to_withdraw = min(assets_needed, current_debt) # Cache max_withdraw now for use if unrealized loss > 0 - # Use maxRedeem and convert since we use redeem. + # Use maxRedeem and convert it since we use redeem. max_withdraw: uint256 = IStrategy(strategy).convertToAssets( IStrategy(strategy).maxRedeem(self) ) @@ -905,7 +907,7 @@ def _redeem( self._withdraw_from_strategy(strategy, assets_to_withdraw) post_balance: uint256 = ERC20(_asset).balanceOf(self) - # Always check withdrawn against the real amounts. + # Always check against the real amounts. withdrawn: uint256 = post_balance - previous_balance loss: uint256 = 0 # Check if we redeemed too much. @@ -1461,8 +1463,12 @@ def setProfitMaxUnlockTime(new_profit_max_unlock_time: uint256): # If setting to 0 we need to reset any locked values. if (new_profit_max_unlock_time == 0): - # Burn any shares the vault still has. - self._burn_shares(self.balance_of[self], self) + + share_balance: uint256 = self.balance_of[self] + if share_balance > 0: + # Burn any shares the vault still has. + self._burn_shares(share_balance, self) + # Reset unlocking variables to 0. self.profit_unlocking_rate = 0 self.full_profit_unlock_date = 0 @@ -2006,8 +2012,8 @@ def maxRedeem( @return The maximum amount of shares that can be redeemed. """ return min( - # Convert to shares is rounding up so we check against the full balance. - self._convert_to_shares(self._max_withdraw(owner, max_loss, strategies), Rounding.ROUND_UP), + # Min of the shares equivalent of max_withdraw or the full balance + self._convert_to_shares(self._max_withdraw(owner, max_loss, strategies), Rounding.ROUND_DOWN), self.balance_of[owner] ) diff --git a/contracts/test/ERC4626BaseStrategy.sol b/contracts/test/ERC4626BaseStrategy.sol index c4a75e35..e37f7995 100644 --- a/contracts/test/ERC4626BaseStrategy.sol +++ b/contracts/test/ERC4626BaseStrategy.sol @@ -20,7 +20,7 @@ abstract contract ERC4626BaseStrategy is ERC4626 { constructor( address _vault, address _asset - ) ERC4626(IERC20Metadata(address(_asset))) { + ) ERC4626(IERC20(address(_asset))) { _initialize(_vault, _asset); } @@ -30,13 +30,7 @@ abstract contract ERC4626BaseStrategy is ERC4626 { vault = _vault; } - function decimals() - public - view - virtual - override(ERC20, IERC20Metadata) - returns (uint8) - { + function decimals() public view virtual override returns (uint8) { return _decimals; } diff --git a/package.json b/package.json index 8ce40a8a..25ee730b 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "yearn-vaults-v3", "devDependencies": { - "@openzeppelin/contracts": "^4.7.3", + "@openzeppelin/contracts": "^4.9.0", "hardhat": "^2.12.2", "prettier": "^2.6.0", "prettier-plugin-solidity": "^1.0.0-beta.19", diff --git a/tests/unit/vault/test_strategy_withdraw.py b/tests/unit/vault/test_strategy_withdraw.py index c3d3b3ef..cf5afb75 100644 --- a/tests/unit/vault/test_strategy_withdraw.py +++ b/tests/unit/vault/test_strategy_withdraw.py @@ -1807,7 +1807,7 @@ def test_redeem__half_of_strategy_assets_from_locked_lossy_strategy_with_unreali asset, create_vault, create_strategy, - create_locked_strategy, + create_lossy_strategy, user_deposit, add_strategy_to_vault, add_debt_to_strategy, @@ -1822,7 +1822,7 @@ def test_redeem__half_of_strategy_assets_from_locked_lossy_strategy_with_unreali ) # withdraw a quarter deposit (half of strategy debt) shares = amount liquid_strategy = create_strategy(vault) - lossy_strategy = create_locked_strategy(vault) + lossy_strategy = create_lossy_strategy(vault) strategies = [lossy_strategy, liquid_strategy] max_loss = 10_000 @@ -1840,9 +1840,9 @@ def test_redeem__half_of_strategy_assets_from_locked_lossy_strategy_with_unreali add_debt_to_strategy(gov, strategy, vault, amount_per_strategy) # lose half of assets in lossy strategy - asset.transfer(gov, amount_to_lose, sender=lossy_strategy) + lossy_strategy.setLoss(gov, amount_to_lose, sender=lossy_strategy) # Lock half the remaining funds. - lossy_strategy.setLockedFunds(amount_to_lock, DAY, sender=gov) + lossy_strategy.setLockedFunds(amount_to_lock, sender=gov) tx = vault.redeem( amount_to_withdraw, @@ -1861,7 +1861,7 @@ def test_redeem__half_of_strategy_assets_from_locked_lossy_strategy_with_unreali event = list(tx.decode_logs(vault.Withdraw)) - assert len(event) >= 1 + assert len(event) > 1 n = len(event) - 1 assert event[n].sender == fish assert event[n].receiver == fish @@ -1889,7 +1889,7 @@ def test_redeem__half_of_strategy_assets_from_locked_lossy_strategy_with_unreali assert asset.balanceOf(vault) == 0 assert asset.balanceOf(liquid_strategy) == amount_per_strategy - expected_liquid_out assert ( - asset.balanceOf(lossy_strategy) + asset.balanceOf(lossy_strategy.yieldSource()) == amount_per_strategy - amount_to_lose - expected_locked_out ) # withdrawn from strategy assert (