diff --git a/contracts/VaultV3.vy b/contracts/VaultV3.vy index d45ae5b7..239dbacd 100644 --- a/contracts/VaultV3.vy +++ b/contracts/VaultV3.vy @@ -112,6 +112,9 @@ event RoleSet: role: indexed(Roles) # STORAGE MANAGEMENT EVENTS +event UpdateFutureRoleManager: + future_role_manager: indexed(address) + event UpdateRoleManager: role_manager: indexed(address) @@ -861,7 +864,7 @@ def _redeem( # NOTE: strategy's debt decreases by the full amount but the total idle increases # by the actual amount only (as the difference is considered lost). - current_total_idle += (assets_to_withdraw - loss) + current_total_idle += (unsafe_sub(assets_to_withdraw, loss)) requested_assets -= loss current_total_debt -= assets_to_withdraw @@ -928,14 +931,11 @@ def _add_strategy(new_strategy: address, add_to_queue: bool): @internal def _revoke_strategy(strategy: address, force: bool=False): assert self.strategies[strategy].activation != 0, "strategy not active" - - # If force revoking a strategy, it will cause a loss. - loss: uint256 = 0 if self.strategies[strategy].current_debt != 0: assert force, "strategy has debt" # Vault realizes the full loss of outstanding debt. - loss = self.strategies[strategy].current_debt + loss: uint256 = self.strategies[strategy].current_debt # Adjust total vault debt. self.total_debt -= loss @@ -1031,7 +1031,7 @@ def _update_debt(strategy: address, target_debt: uint256, max_loss: uint256) -> # If we didn't get the amount we asked for and there is a max loss. if withdrawn < assets_to_withdraw and max_loss < MAX_BPS: # Make sure the loss is within the allowed range. - assert assets_to_withdraw - withdrawn <= assets_to_withdraw * max_loss / MAX_BPS, "too much loss" + assert unsafe_sub(assets_to_withdraw, withdrawn) <= assets_to_withdraw * max_loss / MAX_BPS, "too much loss" # If we got too much make sure not to increase PPS. elif withdrawn > assets_to_withdraw: @@ -1257,8 +1257,10 @@ def _process_report(strategy: address) -> (uint256, uint256): self.strategies[strategy].current_debt = current_debt self.total_debt += gain else: - self.total_idle = total_assets - + # Add in any refunds since it is now idle. + current_debt = unsafe_add(current_debt, total_refunds) + self.total_idle = current_debt + # Or record any reported loss elif loss > 0: current_debt = unsafe_sub(current_debt, loss) @@ -1266,7 +1268,9 @@ def _process_report(strategy: address) -> (uint256, uint256): self.strategies[strategy].current_debt = current_debt self.total_debt -= loss else: - self.total_idle = total_assets + # Add in any refunds since it is now idle. + current_debt = unsafe_add(current_debt, total_refunds) + self.total_idle = current_debt # Issue shares for fees that were calculated above if applicable. if total_fees_shares > 0: @@ -1537,30 +1541,32 @@ def set_role(account: address, role: Roles): @external def add_role(account: address, role: Roles): """ - @notice Add a new role to an address. - @dev This will add a new role to the account + @notice Add a new role/s to an address. + @dev This will add a new role/s to the account without effecting any of the previously held roles. @param account The account to add a role to. - @param role The new role to add to account. + @param role The new role/s to add to account. """ assert msg.sender == self.role_manager - self.roles[account] = self.roles[account] | role + new_roles: Roles = self.roles[account] | role + self.roles[account] = new_roles - log RoleSet(account, self.roles[account]) + log RoleSet(account, new_roles) @external def remove_role(account: address, role: Roles): """ - @notice Remove a single role from an account. + @notice Remove a role/s from an account. @dev This will leave all other roles for the account unchanged. - @param account The account to remove a Role from. - @param role The Role to remove. + @param account The account to remove a Role/s from. + @param role The Role/s to remove. """ assert msg.sender == self.role_manager - self.roles[account] = self.roles[account] & ~role + new_roles: Roles = self.roles[account] & ~role + self.roles[account] = new_roles - log RoleSet(account, self.roles[account]) + log RoleSet(account, new_roles) @external def transfer_role_manager(role_manager: address): @@ -1574,6 +1580,8 @@ def transfer_role_manager(role_manager: address): assert msg.sender == self.role_manager self.future_role_manager = role_manager + log UpdateFutureRoleManager(role_manager) + @external def accept_role_manager(): """ @@ -1671,15 +1679,16 @@ def buy_debt(strategy: address, amount: uint256): self._erc20_safe_transfer_from(self.asset, msg.sender, self, _amount) - # Lower strategy debt - self.strategies[strategy].current_debt -= _amount + # Lower strategy debt + new_debt: uint256 = unsafe_sub(current_debt, _amount) + self.strategies[strategy].current_debt = new_debt # lower total debt self.total_debt -= _amount # Increase total idle self.total_idle += _amount # log debt change - log DebtUpdated(strategy, current_debt, current_debt - _amount) + log DebtUpdated(strategy, current_debt, new_debt) # Transfer the strategies shares out. self._erc20_safe_transfer(strategy, msg.sender, shares) @@ -1746,7 +1755,7 @@ def update_debt( @param strategy The strategy to update the debt for. @param target_debt The target debt for the strategy. @param max_loss Optional to check realized losses on debt decreases. - @return The amount of debt added or removed. + @return The new current debt of the strategy. """ self._enforce_role(msg.sender, Roles.DEBT_MANAGER) return self._update_debt(strategy, target_debt, max_loss) @@ -1772,7 +1781,10 @@ def shutdown_vault(): self.deposit_limit = 0 log UpdateDepositLimit(0) - self.roles[msg.sender] = self.roles[msg.sender] | Roles.DEBT_MANAGER + new_roles: Roles = self.roles[msg.sender] | Roles.DEBT_MANAGER + self.roles[msg.sender] = new_roles + log RoleSet(msg.sender, new_roles) + log Shutdown() @@ -2099,7 +2111,7 @@ def FACTORY() -> address: """ return self.factory -@view +@pure @external def apiVersion() -> String[28]: """ diff --git a/foundry_tests/handlers/VaultHandler.sol b/foundry_tests/handlers/VaultHandler.sol index 19e67513..523fdebf 100644 --- a/foundry_tests/handlers/VaultHandler.sol +++ b/foundry_tests/handlers/VaultHandler.sol @@ -101,7 +101,7 @@ contract VaultHandler is ExtendedTest { deposit(_amount * 2); } } - _amount = bound(_amount, 0, vault.maxWithdraw(address(actor))); + _amount = bound(_amount, 0, vault.maxWithdraw(address(actor)) + 1); if (_amount == 0) ghost_zeroWithdrawals++; uint256 idle = vault.totalIdle(); @@ -122,7 +122,7 @@ contract VaultHandler is ExtendedTest { mint(_amount * 2); } } - _amount = bound(_amount, 0, vault.balanceOf(address(actor))); + _amount = bound(_amount, 0, vault.balanceOf(address(actor)) + 1); if (_amount == 0) ghost_zeroWithdrawals++; uint256 idle = vault.totalIdle(); @@ -243,6 +243,8 @@ contract VaultHandler is ExtendedTest { ) public countCall("unreportedLoss") { _amount = bound(_amount, 0, strategy.totalAssets() / 10); + if (_amount == 0) return; + // Simulate losing money vm.prank(address(strategy)); asset.transfer(address(69), _amount); diff --git a/package.json b/package.json index 0445d4ba..57824298 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "yearn-vaults-v3", "devDependencies": { - "@openzeppelin/contracts": "^4.9.0", + "@openzeppelin/contracts": "^4.9.5", "hardhat": "^2.12.2", "prettier": "^2.6.0", "prettier-plugin-solidity": "^1.0.0-beta.19", diff --git a/tests/unit/vault/test_emergency_shutdown.py b/tests/unit/vault/test_emergency_shutdown.py index 593cfe58..35b5ca06 100644 --- a/tests/unit/vault/test_emergency_shutdown.py +++ b/tests/unit/vault/test_emergency_shutdown.py @@ -25,7 +25,11 @@ def test_shutdown(gov, panda, vault): def test_shutdown_gives_debt_manager_role(gov, panda, vault): vault.set_role(panda.address, ROLES.EMERGENCY_MANAGER, sender=gov) assert ROLES.DEBT_MANAGER not in ROLES(vault.roles(panda)) - vault.shutdown_vault(sender=panda) + tx = vault.shutdown_vault(sender=panda) + event = list(tx.decode_logs(vault.RoleSet)) + assert len(event) == 1 + assert event[0].account == panda.address + assert event[0].role == ROLES.DEBT_MANAGER | ROLES.EMERGENCY_MANAGER assert ROLES.DEBT_MANAGER | ROLES.EMERGENCY_MANAGER == vault.roles(panda) diff --git a/tests/unit/vault/test_roles.py b/tests/unit/vault/test_roles.py index f77ee04c..75bca2d4 100644 --- a/tests/unit/vault/test_roles.py +++ b/tests/unit/vault/test_roles.py @@ -18,7 +18,10 @@ def test_transfers_role_manager(vault, gov, strategist): assert vault.role_manager() == gov assert vault.future_role_manager() == ZERO_ADDRESS - vault.transfer_role_manager(strategist, sender=gov) + tx = vault.transfer_role_manager(strategist, sender=gov) + event = list(tx.decode_logs(vault.UpdateFutureRoleManager)) + assert len(event) == 1 + assert event[0].future_role_manager == strategist assert vault.role_manager() == gov assert vault.future_role_manager() == strategist @@ -36,7 +39,10 @@ def test_gov_transfers_role_manager__gov_cant_accept(vault, gov, strategist): assert vault.role_manager() == gov assert vault.future_role_manager() == ZERO_ADDRESS - vault.transfer_role_manager(strategist, sender=gov) + tx = vault.transfer_role_manager(strategist, sender=gov) + event = list(tx.decode_logs(vault.UpdateFutureRoleManager)) + assert len(event) == 1 + assert event[0].future_role_manager == strategist assert vault.role_manager() == gov assert vault.future_role_manager() == strategist @@ -65,12 +71,18 @@ def test_gov_transfers_role_manager__can_change_future_manager( assert vault.role_manager() == gov assert vault.future_role_manager() == ZERO_ADDRESS - vault.transfer_role_manager(strategist, sender=gov) + tx = vault.transfer_role_manager(strategist, sender=gov) + event = list(tx.decode_logs(vault.UpdateFutureRoleManager)) + assert len(event) == 1 + assert event[0].future_role_manager == strategist assert vault.role_manager() == gov assert vault.future_role_manager() == strategist - vault.transfer_role_manager(bunny, sender=gov) + tx = vault.transfer_role_manager(bunny, sender=gov) + event = list(tx.decode_logs(vault.UpdateFutureRoleManager)) + assert len(event) == 1 + assert event[0].future_role_manager == bunny assert vault.role_manager() == gov assert vault.future_role_manager() == bunny diff --git a/tests/unit/vault/test_strategy_accounting.py b/tests/unit/vault/test_strategy_accounting.py index 0b067e78..ee090dcb 100644 --- a/tests/unit/vault/test_strategy_accounting.py +++ b/tests/unit/vault/test_strategy_accounting.py @@ -651,3 +651,118 @@ def test_set_accountant__with_accountant(gov, vault, deploy_accountant): assert event[0].accountant == accountant.address assert vault.accountant() == accountant.address + + +def test_process_report_on_self__gain_and_refunds( + chain, + gov, + asset, + vault, + set_fees_for_strategy, + airdrop_asset, + deploy_flexible_accountant, +): + vault_balance = asset.balanceOf(vault) + gain = vault_balance // 10 + loss = 0 + management_fee = 0 + performance_fee = 0 + refund_ratio = 5_000 + refund = gain * refund_ratio // MAX_BPS_ACCOUNTANT + + accountant = deploy_flexible_accountant(vault) + # set up accountant + asset.mint(accountant, gain, sender=gov) + + set_fees_for_strategy( + gov, vault, accountant, management_fee, performance_fee, refund_ratio + ) + + initial_idle = vault.totalIdle() + + airdrop_asset(gov, asset, vault, gain) + + # Not yet recorded + assert vault.totalIdle() == initial_idle + assert asset.balanceOf(vault) == initial_idle + gain + pps_before = vault.pricePerShare() + assets_before = vault.totalAssets() + supply_before = vault.totalSupply() + tx = vault.process_report(vault.address, sender=gov) + event = list(tx.decode_logs(vault.StrategyReported)) + + assert len(event) == 1 + assert event[0].strategy == vault.address + assert event[0].gain == gain + assert event[0].loss == 0 + assert event[0].current_debt == vault_balance + gain + refund + assert event[0].total_fees == 0 + assert event[0].total_refunds == refund + + # Due to refunds, pps should have increased + assert vault.pricePerShare() == pps_before + assert vault.totalAssets() == vault_balance + gain + refund + assert vault.totalSupply() > supply_before + assert vault.totalDebt() == 0 + assert vault.totalIdle() == vault_balance + gain + refund + assert asset.balanceOf(vault) == vault_balance + gain + refund + + chain.pending_timestamp += DAY + chain.mine(timestamp=chain.pending_timestamp) + + assert vault.pricePerShare() > pps_before + + +def test_process_report_on_self__loss_and_refunds( + chain, + gov, + asset, + vault, + set_fees_for_strategy, + airdrop_asset, + deploy_flexible_accountant, +): + vault_balance = asset.balanceOf(vault) + gain = 0 + loss = vault_balance // 10 + management_fee = 0 + performance_fee = 0 + refund_ratio = 5_000 + refund = loss * refund_ratio // MAX_BPS_ACCOUNTANT + + accountant = deploy_flexible_accountant(vault) + # set up accountant + asset.mint(accountant, loss, sender=gov) + + set_fees_for_strategy( + gov, vault, accountant, management_fee, performance_fee, refund_ratio + ) + + initial_idle = vault.totalIdle() + + asset.transfer(gov, loss, sender=vault.address) + + # Not yet recorded + assert vault.totalIdle() == initial_idle + assert asset.balanceOf(vault) == initial_idle - loss + pps_before = vault.pricePerShare() + assets_before = vault.totalAssets() + supply_before = vault.totalSupply() + tx = vault.process_report(vault.address, sender=gov) + event = list(tx.decode_logs(vault.StrategyReported)) + + assert len(event) == 1 + assert event[0].strategy == vault.address + assert event[0].gain == gain + assert event[0].loss == loss + assert event[0].current_debt == vault_balance + refund - loss + assert event[0].total_fees == 0 + assert event[0].total_refunds == refund + + # Due to refunds, pps should have increased + assert vault.pricePerShare() < pps_before + assert vault.totalAssets() == vault_balance + refund - loss + assert vault.totalSupply() == supply_before + assert vault.totalDebt() == 0 + assert vault.totalIdle() == vault_balance + refund - loss + assert asset.balanceOf(vault) == vault_balance + refund - loss diff --git a/yarn.lock b/yarn.lock index bd02e075..3a4c4a17 100644 --- a/yarn.lock +++ b/yarn.lock @@ -419,10 +419,10 @@ "@nomicfoundation/solidity-analyzer-win32-ia32-msvc" "0.1.0" "@nomicfoundation/solidity-analyzer-win32-x64-msvc" "0.1.0" -"@openzeppelin/contracts@^4.7.3": - version "4.8.2" - resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-4.8.2.tgz#d815ade0027b50beb9bcca67143c6bcc3e3923d6" - integrity sha512-kEUOgPQszC0fSYWpbh2kT94ltOJwj1qfT2DWo+zVttmGmf97JZ99LspePNaeeaLhCImaHVeBbjaQFZQn7+Zc5g== +"@openzeppelin/contracts@^4.9.5": + version "4.9.6" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-4.9.6.tgz#2a880a24eb19b4f8b25adc2a5095f2aa27f39677" + integrity sha512-xSmezSupL+y9VkHZJGDoCBpmnB2ogM13ccaYDWqJTfS3dbuHkgjuwDFUmaFauBCboQMGB/S5UqUl2y54X99BmA== "@scure/base@~1.1.0": version "1.1.1"