From fde6d3fe344e0881dc13a6f7b92a14a253cb92a7 Mon Sep 17 00:00:00 2001 From: Schlagonia Date: Tue, 26 Dec 2023 18:22:52 -0700 Subject: [PATCH 01/16] feat: update api and ape version --- .github/workflows/test.yaml | 4 ++-- ape-config.yaml | 9 +++------ contracts/VaultFactory.vy | 2 +- contracts/VaultV3.vy | 2 +- requirements.txt | 2 +- scripts/deploy.py | 2 +- tests/conftest.py | 4 ++-- tests/unit/vault/test_queue_management.py | 2 +- 8 files changed, 12 insertions(+), 15 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 6ef384f6..3b967b9b 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -23,8 +23,8 @@ jobs: - uses: ApeWorX/github-action@v2.0 with: python-version: '3.10' - ape-version-pin: "==0.6.3" - ape-plugins-list: 'solidity==0.6.0 vyper==0.6.1 hardhat==0.6.0' + ape-version-pin: "==0.7.0" + #ape-plugins-list: 'solidity==0.6.0 vyper==0.6.1 hardhat==0.6.0' - name: install vyper run: pip install git+https://github.com/vyperlang/vyper diff --git a/ape-config.yaml b/ape-config.yaml index 7f5685e4..c2f8bd0a 100644 --- a/ape-config.yaml +++ b/ape-config.yaml @@ -2,26 +2,23 @@ name: yearn-v3 plugins: - name: solidity - version: 0.6.0 - name: vyper - version: 0.6.1 - name: hardhat - version: 0.6.0 default_ecosystem: ethereum dependencies: - name: openzeppelin github: OpenZeppelin/openzeppelin-contracts - version: 4.7.3 + ref: 4.7.3 - name: tokenized-strategy github: yearn/tokenized-strategy - branch: master + ref: v3.0.2 contracts_folder: src solidity: import_remapping: - "@openzeppelin/contracts=openzeppelin/v4.7.3" - - "@tokenized-strategy=tokenized-strategy/master" + - "@tokenized-strategy=tokenized-strategy/v3.0.2" ethereum: local: diff --git a/contracts/VaultFactory.vy b/contracts/VaultFactory.vy index 0be88b58..8f42f805 100644 --- a/contracts/VaultFactory.vy +++ b/contracts/VaultFactory.vy @@ -68,7 +68,7 @@ struct PFConfig: fee_recipient: address # Identifier for this version of the vault. -API_VERSION: constant(String[28]) = "3.0.1" +API_VERSION: constant(String[28]) = "3.0.2" # The max amount the protocol fee can be set to. MAX_FEE_BPS: constant(uint16) = 5_000 # 50% diff --git a/contracts/VaultV3.vy b/contracts/VaultV3.vy index 67b7080b..6c52e7f0 100644 --- a/contracts/VaultV3.vy +++ b/contracts/VaultV3.vy @@ -174,7 +174,7 @@ MAX_BPS: constant(uint256) = 10_000 # Extended for profit locking calculations. MAX_BPS_EXTENDED: constant(uint256) = 1_000_000_000_000 # The version of this vault. -API_VERSION: constant(String[28]) = "3.0.1" +API_VERSION: constant(String[28]) = "3.0.2" # ENUMS # # Each permissioned function has its own Role. diff --git a/requirements.txt b/requirements.txt index c343a7a9..f37cb764 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1,2 @@ black==22.3.0 -eth-ape==0.6.3 \ No newline at end of file +eth-ape>=0.7.0 \ No newline at end of file diff --git a/scripts/deploy.py b/scripts/deploy.py index 6daf1c9f..9e783672 100644 --- a/scripts/deploy.py +++ b/scripts/deploy.py @@ -21,7 +21,7 @@ def deploy_blueprint_and_factory(): deployer_contract = project.IDeployer.at( "0x8D85e7c9A4e369E53Acc8d5426aE1568198b0112" ) - salt_string = "v3.0.1" + salt_string = "v3.0.2" # Create a SHA-256 hash object hash_object = hashlib.sha256() diff --git a/tests/conftest.py b/tests/conftest.py index b46227ae..96338497 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,7 +1,7 @@ import pytest from ape import chain from ape.types import ContractLog -from eth_account.messages import encode_structured_data +from eth_account.messages import encode_typed_data from utils.constants import MAX_INT, ROLES, WEEK import time import os @@ -459,7 +459,7 @@ def sign_vault_permit( "deadline": deadline, }, } - permit = encode_structured_data(data) + permit = encode_typed_data(full_message=data) return owner.sign_message(permit) return sign_vault_permit diff --git a/tests/unit/vault/test_queue_management.py b/tests/unit/vault/test_queue_management.py index c9244f54..a4ffa68d 100644 --- a/tests/unit/vault/test_queue_management.py +++ b/tests/unit/vault/test_queue_management.py @@ -372,7 +372,7 @@ def test__set_default_queue(create_vault, asset, gov, create_strategy): event_queue = list(event[0].new_default_queue) # Need to checksum each address to compare it correctly. for i in range(len(new_queue)): - assert Web3.toChecksumAddress(event_queue[i]) == new_queue[i] + assert Web3.to_checksum_address(event_queue[i]) == new_queue[i] def test__set_default_queue__inactive_strategy__reverts( From a6e361c524c18049db42766cd14f497c86e21dd0 Mon Sep 17 00:00:00 2001 From: Schlag <89420541+Schlagonia@users.noreply.github.com> Date: Sun, 7 Jan 2024 10:10:51 -0700 Subject: [PATCH 02/16] chore: removals (#189) * chore: remove increase and decrease allowances * chore: remove open roles --- contracts/VaultV3.vy | 76 +- tests/unit/vault/test_erc20.py | 37 - .../vault/test_role_permissioned_access.py | 909 ------------------ 3 files changed, 7 insertions(+), 1015 deletions(-) delete mode 100644 tests/unit/vault/test_role_permissioned_access.py diff --git a/contracts/VaultV3.vy b/contracts/VaultV3.vy index 6c52e7f0..791d21cc 100644 --- a/contracts/VaultV3.vy +++ b/contracts/VaultV3.vy @@ -111,10 +111,6 @@ event RoleSet: account: indexed(address) role: indexed(Roles) -event RoleStatusChanged: - role: indexed(Roles) - status: indexed(RoleStatusChange) - # STORAGE MANAGEMENT EVENTS event UpdateRoleManager: role_manager: indexed(address) @@ -204,10 +200,6 @@ enum Rounding: ROUND_DOWN ROUND_UP -enum RoleStatusChange: - OPENED - CLOSED - # IMMUTABLE # # Underlying token used by the vault. ASSET: immutable(ERC20) @@ -224,6 +216,7 @@ default_queue: public(DynArray[address, MAX_QUEUE]) # Should the vault use the default_queue regardless whats passed in. use_default_queue: public(bool) +### ACCOUNTING ### # ERC20 - amount of shares per account balance_of: HashMap[address, uint256] # ERC20 - owner -> (spender -> amount) @@ -231,7 +224,6 @@ allowance: public(HashMap[address, HashMap[address, uint256]]) # Total amount of shares that are currently minted including those locked. # NOTE: To get the ERC20 compliant version user totalSupply(). total_supply: public(uint256) - # Total amount of assets that has been deposited in strategies. total_debt: uint256 # Current assets held in the vault contract. Replacing balanceOf(this) to avoid price_per_share manipulation. @@ -240,16 +232,18 @@ total_idle: uint256 minimum_total_idle: public(uint256) # Maximum amount of tokens that the vault can accept. If totalAssets > deposit_limit, deposits will revert. deposit_limit: public(uint256) + +### PERIPHERY ### # Contract that charges fees and can give refunds. accountant: public(address) # Contract to control the deposit limit. deposit_limit_module: public(address) # Contract to control the withdraw limit. withdraw_limit_module: public(address) + +### ROLES ### # HashMap mapping addresses to their roles roles: public(HashMap[address, Roles]) -# HashMap mapping roles to their permissioned state. If false, the role is not open to the public. -open_roles: public(HashMap[Roles, bool]) # Address that can add and remove roles to addresses. role_manager: public(address) # Temporary variable to store the address of the next role_manager until the role is accepted. @@ -343,20 +337,6 @@ def _approve(owner: address, spender: address, amount: uint256) -> bool: log Approval(owner, spender, amount) return True -@internal -def _increase_allowance(owner: address, spender: address, amount: uint256) -> bool: - new_allowance: uint256 = self.allowance[owner][spender] + amount - self.allowance[owner][spender] = new_allowance - log Approval(owner, spender, new_allowance) - return True - -@internal -def _decrease_allowance(owner: address, spender: address, amount: uint256) -> bool: - new_allowance: uint256 = self.allowance[owner][spender] - amount - self.allowance[owner][spender] = new_allowance - log Approval(owner, spender, new_allowance) - return True - @internal def _permit( owner: address, @@ -1471,8 +1451,8 @@ def setProfitMaxUnlockTime(new_profit_max_unlock_time: uint256): # ROLE MANAGEMENT # @internal def _enforce_role(account: address, role: Roles): - # Make sure the sender either holds the role or it has been opened. - assert role in self.roles[account] or self.open_roles[role], "not allowed" + # Make sure the sender holds the role. + assert role in self.roles[account], "not allowed" @external def set_role(account: address, role: Roles): @@ -1515,28 +1495,6 @@ def remove_role(account: address, role: Roles): self.roles[account] = self.roles[account] & ~role log RoleSet(account, self.roles[account]) - -@external -def set_open_role(role: Roles): - """ - @notice Set a role to be open. - @param role The role to set. - """ - assert msg.sender == self.role_manager - self.open_roles[role] = True - - log RoleStatusChanged(role, RoleStatusChange.OPENED) - -@external -def close_open_role(role: Roles): - """ - @notice Close a opened role. - @param role The role to close. - """ - assert msg.sender == self.role_manager - self.open_roles[role] = False - - log RoleStatusChanged(role, RoleStatusChange.CLOSED) @external def transfer_role_manager(role_manager: address): @@ -1852,26 +1810,6 @@ def transferFrom(sender: address, receiver: address, amount: uint256) -> bool: return self._transfer_from(sender, receiver, amount) ## ERC20+4626 compatibility -@external -def increaseAllowance(spender: address, amount: uint256) -> bool: - """ - @notice Increase the allowance for a spender. - @param spender The address to increase the allowance for. - @param amount The amount to increase the allowance by. - @return True if the increase was successful. - """ - return self._increase_allowance(msg.sender, spender, amount) - -@external -def decreaseAllowance(spender: address, amount: uint256) -> bool: - """ - @notice Decrease the allowance for a spender. - @param spender The address to decrease the allowance for. - @param amount The amount to decrease the allowance by. - @return True if the decrease was successful. - """ - return self._decrease_allowance(msg.sender, spender, amount) - @external def permit( owner: address, diff --git a/tests/unit/vault/test_erc20.py b/tests/unit/vault/test_erc20.py index 676950c0..264e4bf8 100644 --- a/tests/unit/vault/test_erc20.py +++ b/tests/unit/vault/test_erc20.py @@ -55,43 +55,6 @@ def test_approve__with_amount__approve(fish, fish_amount, bunny, asset, create_v assert vault.allowance(fish, bunny) == fish_amount -def test_increase_allowance__with_amount__approve( - fish, fish_amount, bunny, asset, create_vault -): - vault = create_vault(asset) - - tx = vault.increaseAllowance(bunny.address, fish_amount, sender=fish) - event = list(tx.decode_logs(vault.Approval)) - - assert len(event) == 1 - assert event[0].owner == fish - assert event[0].spender == bunny - assert event[0].value == fish_amount - - assert vault.allowance(fish, bunny) == fish_amount - - -def test_decrease_allowance__with_amount__approve( - fish, fish_amount, bunny, asset, create_vault -): - vault = create_vault(asset) - decrease_amount = fish_amount // 2 - final_allowance = fish_amount - decrease_amount - - vault.approve(bunny.address, fish_amount, sender=fish) - assert vault.allowance(fish, bunny) == fish_amount - - tx = vault.decreaseAllowance(bunny.address, decrease_amount, sender=fish) - event = list(tx.decode_logs(vault.Approval)) - - assert len(event) == 1 - assert event[0].owner == fish - assert event[0].spender == bunny - assert event[0].value == final_allowance - - assert vault.allowance(fish, bunny) == final_allowance - - def test_transfer_from__with_approval__transfer( fish, fish_amount, bunny, doggie, asset, create_vault, user_deposit ): diff --git a/tests/unit/vault/test_role_permissioned_access.py b/tests/unit/vault/test_role_permissioned_access.py deleted file mode 100644 index 2d0c1971..00000000 --- a/tests/unit/vault/test_role_permissioned_access.py +++ /dev/null @@ -1,909 +0,0 @@ -import ape -from utils.constants import ( - ROLES, - WEEK, - StrategyChangeType, - RoleStatusChange, - ZERO_ADDRESS, -) -from utils.utils import from_units - - -def test_set_open_role__by_random_account__reverts(vault, bunny): - with ape.reverts(): - vault.set_open_role(ROLES.ADD_STRATEGY_MANAGER, sender=bunny) - - -def test_close_open_role__by_random_account__reverts(vault, gov, bunny): - tx = vault.set_open_role(ROLES.ADD_STRATEGY_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.ADD_STRATEGY_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - assert vault.open_roles(ROLES.ADD_STRATEGY_MANAGER) == True - with ape.reverts(): - vault.close_open_role(ROLES.ADD_STRATEGY_MANAGER, sender=bunny) - - -# STRATEGY MANAGEMENT - - -def test_add_strategy__add_strategy_role_closed__reverts(vault, create_strategy, bunny): - new_strategy = create_strategy(vault) - with ape.reverts("not allowed"): - vault.add_strategy(new_strategy, sender=bunny) - - -def test_revoke_strategy__revoke_strategy_role_closed__reverts( - vault, create_strategy, bunny, gov -): - new_strategy = create_strategy(vault) - vault.add_strategy(new_strategy, sender=gov) - with ape.reverts("not allowed"): - vault.revoke_strategy(new_strategy, sender=bunny) - - -def test_force_revoke_strategy__revoke_strategy_role_closed__reverts( - vault, create_strategy, bunny, gov -): - new_strategy = create_strategy(vault) - - vault.add_strategy(new_strategy, sender=gov) - with ape.reverts("not allowed"): - vault.force_revoke_strategy(new_strategy, sender=bunny) - - -def test_add_strategy__set_add_strategy_role_open(vault, create_strategy, bunny, gov): - new_strategy = create_strategy(vault) - tx = vault.set_open_role(ROLES.ADD_STRATEGY_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.ADD_STRATEGY_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.add_strategy(new_strategy, sender=bunny) - event = list(tx.decode_logs(vault.StrategyChanged)) - assert len(event) == 1 - assert event[0].strategy == new_strategy.address - assert event[0].change_type == StrategyChangeType.ADDED - - -def test_revoke_strategy__set_revoke_strategy_role_open( - vault, create_strategy, bunny, gov -): - new_strategy = create_strategy(vault) - vault.add_strategy(new_strategy, sender=gov) - tx = vault.set_open_role(ROLES.REVOKE_STRATEGY_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.REVOKE_STRATEGY_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.revoke_strategy(new_strategy, sender=bunny) - event = list(tx.decode_logs(vault.StrategyChanged)) - assert len(event) == 1 - assert event[0].strategy == new_strategy.address - assert event[0].change_type == StrategyChangeType.REVOKED - - -def test_force_revoke_strategy__set_revoke_strategy_role_open( - vault, create_strategy, bunny, gov -): - new_strategy = create_strategy(vault) - - vault.add_strategy(new_strategy, sender=gov) - tx = vault.set_open_role(ROLES.FORCE_REVOKE_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.FORCE_REVOKE_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.force_revoke_strategy(new_strategy, sender=bunny) - event = list(tx.decode_logs(vault.StrategyChanged)) - assert len(event) == 1 - assert event[0].strategy == new_strategy.address - assert event[0].change_type == StrategyChangeType.REVOKED - - -def test_add_strategy__set_add_strategy_role_open_then_close__reverts( - vault, create_strategy, bunny, gov -): - new_strategy = create_strategy(vault) - tx = vault.set_open_role(ROLES.ADD_STRATEGY_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.ADD_STRATEGY_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.add_strategy(new_strategy, sender=bunny) - event = list(tx.decode_logs(vault.StrategyChanged)) - assert len(event) == 1 - assert event[0].strategy == new_strategy.address - assert event[0].change_type == StrategyChangeType.ADDED - # close the role - tx = vault.close_open_role(ROLES.ADD_STRATEGY_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.ADD_STRATEGY_MANAGER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts("not allowed"): - vault.add_strategy(new_strategy, sender=bunny) - - -def test_revoke_strategy__set_revoke_strategy_role_open_then_close__reverts( - vault, create_strategy, bunny, gov -): - new_strategy = create_strategy(vault) - vault.add_strategy(new_strategy, sender=gov) - tx = vault.set_open_role(ROLES.REVOKE_STRATEGY_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.REVOKE_STRATEGY_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.revoke_strategy(new_strategy, sender=bunny) - event = list(tx.decode_logs(vault.StrategyChanged)) - assert len(event) == 1 - assert event[0].strategy == new_strategy.address - assert event[0].change_type == StrategyChangeType.REVOKED - - # close the role - tx = vault.close_open_role(ROLES.REVOKE_STRATEGY_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.REVOKE_STRATEGY_MANAGER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts("not allowed"): - vault.revoke_strategy(new_strategy, sender=bunny) - - -def test_force_revoke_strategy__set_revoke_strategy_role_open( - vault, create_strategy, bunny, gov -): - new_strategy = create_strategy(vault) - - vault.add_strategy(new_strategy, sender=gov) - tx = vault.set_open_role(ROLES.FORCE_REVOKE_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.FORCE_REVOKE_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.force_revoke_strategy(new_strategy, sender=bunny) - event = list(tx.decode_logs(vault.StrategyChanged)) - assert len(event) == 1 - assert event[0].strategy == new_strategy.address - assert event[0].change_type == StrategyChangeType.REVOKED - other_strategy = create_strategy(vault) - vault.add_strategy(other_strategy, sender=gov) - - tx = vault.close_open_role(ROLES.FORCE_REVOKE_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.FORCE_REVOKE_MANAGER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts("not allowed"): - vault.force_revoke_strategy(other_strategy, sender=bunny) - - -# REPORTING_MANAGER - - -def test_process_report__reporting_role_closed__reverts( - vault, create_strategy, bunny, gov -): - new_strategy = create_strategy(vault) - vault.add_strategy(new_strategy, sender=gov) - with ape.reverts("not allowed"): - vault.process_report(new_strategy, sender=bunny) - - -def test_process_report__set_reporting_role_open( - vault, - create_strategy, - asset, - fish_amount, - user_deposit, - add_strategy_to_vault, - add_debt_to_strategy, - bunny, - gov, -): - asset.mint(bunny, fish_amount, sender=gov) - user_deposit(bunny, vault, asset, fish_amount) - new_strategy = create_strategy(vault) - add_strategy_to_vault(gov, new_strategy, vault) - add_debt_to_strategy(gov, new_strategy, vault, fish_amount) - tx = vault.set_open_role(ROLES.REPORTING_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.REPORTING_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - asset.mint(new_strategy, fish_amount, sender=gov) - new_strategy.report(sender=gov) - - tx = vault.process_report(new_strategy, sender=bunny) - event = list(tx.decode_logs(vault.StrategyReported)) - assert len(event) == 1 - assert event[0].strategy == new_strategy.address and event[0].gain == fish_amount - - -def test_process_report__set_reporting_role_open_then_close__reverts( - vault, - create_strategy, - asset, - fish_amount, - user_deposit, - add_strategy_to_vault, - add_debt_to_strategy, - bunny, - gov, -): - asset.mint(bunny, fish_amount, sender=gov) - user_deposit(bunny, vault, asset, fish_amount) - new_strategy = create_strategy(vault) - add_strategy_to_vault(gov, new_strategy, vault) - add_debt_to_strategy(gov, new_strategy, vault, fish_amount) - - tx = vault.set_open_role(ROLES.REPORTING_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.REPORTING_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - asset.mint(new_strategy, fish_amount, sender=gov) - new_strategy.report(sender=gov) - - tx = vault.process_report(new_strategy, sender=bunny) - event = list(tx.decode_logs(vault.StrategyReported)) - assert len(event) == 1 - assert event[0].strategy == new_strategy.address and event[0].gain == fish_amount - - # close role - tx = vault.close_open_role(ROLES.REPORTING_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.REPORTING_MANAGER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts("not allowed"): - vault.process_report(new_strategy, sender=bunny) - - -# PROFIT UNLOCK MANGAGER - - -def test_update_profit_unlock__profit_unlock_role_closed__reverts(vault, bunny): - with ape.reverts(): - vault.setProfitMaxUnlockTime(WEEK * 2, sender=bunny) - - -def test_update_profit_unlock__set_profit_unlock_role_role_open(vault, bunny, gov): - tx = vault.set_open_role(ROLES.PROFIT_UNLOCK_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.PROFIT_UNLOCK_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.setProfitMaxUnlockTime(WEEK * 2, sender=bunny) - event = list(tx.decode_logs(vault.UpdateProfitMaxUnlockTime)) - assert len(event) == 1 - assert event[0].profit_max_unlock_time == WEEK * 2 - vault.profitMaxUnlockTime() == WEEK * 2 - - -def test_update_profit_unlock__set_profit_unlock_role_role_open_then_close__reverts( - vault, bunny, gov -): - tx = vault.set_open_role(ROLES.PROFIT_UNLOCK_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.PROFIT_UNLOCK_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.setProfitMaxUnlockTime(WEEK * 2, sender=bunny) - event = list(tx.decode_logs(vault.UpdateProfitMaxUnlockTime)) - assert len(event) == 1 - assert event[0].profit_max_unlock_time == WEEK * 2 - assert vault.profitMaxUnlockTime() == WEEK * 2 - tx = vault.close_open_role(ROLES.PROFIT_UNLOCK_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.PROFIT_UNLOCK_MANAGER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts(): - vault.setProfitMaxUnlockTime(WEEK, sender=bunny) - - -# ACCOUNTING MANAGEMENT - - -def test_set_minimum_total_idle__minimum_idle_role_closed__reverts(vault, bunny): - with ape.reverts("not allowed"): - vault.set_minimum_total_idle(0, sender=bunny) - - -def test_set_deposit_limit__deposit_limit_role_closed__reverts(vault, bunny): - with ape.reverts("not allowed"): - vault.set_deposit_limit(0, sender=bunny) - - -def test_set_deposit_limit_module__deposit_limit_role_closed__reverts(vault, bunny): - with ape.reverts("not allowed"): - vault.set_deposit_limit_module(bunny, sender=bunny) - - -def test_set_withdraw_limit_module__withdraw_limit_role_closed__reverts(vault, bunny): - with ape.reverts("not allowed"): - vault.set_withdraw_limit_module(bunny, sender=bunny) - - -def test_update_max_debt_for_strategy__max_debt_role_closed__reverts( - vault, create_strategy, bunny, gov -): - new_strategy = create_strategy(vault) - vault.add_strategy(new_strategy, sender=gov) - with ape.reverts("not allowed"): - vault.update_max_debt_for_strategy(new_strategy, 0, sender=bunny) - - -def test_set_minimum_total_idle__set_minimum_idle_role_open(vault, bunny, gov): - tx = vault.set_open_role(ROLES.MINIMUM_IDLE_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.MINIMUM_IDLE_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.set_minimum_total_idle(0, sender=bunny) - event = list(tx.decode_logs(vault.UpdateMinimumTotalIdle)) - assert len(event) == 1 - assert event[0].minimum_total_idle == 0 - - -def test_set_deposit_limit__set_deposit_limit_role_open(vault, bunny, gov): - tx = vault.set_open_role(ROLES.DEPOSIT_LIMIT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.DEPOSIT_LIMIT_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.set_deposit_limit(0, sender=bunny) - event = list(tx.decode_logs(vault.UpdateDepositLimit)) - assert len(event) == 1 - assert event[0].deposit_limit == 0 - - -def test_set_deposit_limit_module__set_deposit_limit_role_open(vault, bunny, gov): - tx = vault.set_open_role(ROLES.DEPOSIT_LIMIT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.DEPOSIT_LIMIT_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.set_deposit_limit_module(bunny, sender=bunny) - event = list(tx.decode_logs(vault.UpdateDepositLimitModule)) - assert len(event) == 1 - assert event[0].deposit_limit_module == bunny.address - assert vault.deposit_limit_module() == bunny.address - - -def test_set_withdraw_limit_module__set_withdraw_limit_role_open(vault, bunny, gov): - tx = vault.set_open_role(ROLES.WITHDRAW_LIMIT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.WITHDRAW_LIMIT_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.set_withdraw_limit_module(bunny, sender=bunny) - event = list(tx.decode_logs(vault.UpdateWithdrawLimitModule)) - assert len(event) == 1 - assert event[0].withdraw_limit_module == bunny.address - assert vault.withdraw_limit_module() == bunny.address - - -def test_update_max_debt_for_strategy__max_debt_limit_role_open( - vault, create_strategy, bunny, gov -): - tx = vault.set_open_role(ROLES.MAX_DEBT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.MAX_DEBT_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - new_strategy = create_strategy(vault) - vault.add_strategy(new_strategy, sender=gov) - tx = vault.update_max_debt_for_strategy(new_strategy, 420, sender=bunny) - event = list(tx.decode_logs(vault.UpdatedMaxDebtForStrategy)) - assert len(event) == 1 - assert ( - event[0].sender == bunny.address and event[0].strategy == new_strategy.address - ) - assert event[0].new_debt == 420 - - -def test_set_minimum_total_idle__set_minimum_idle_role_open_then_close__reverts( - vault, bunny, gov -): - tx = vault.set_open_role(ROLES.MINIMUM_IDLE_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.MINIMUM_IDLE_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.set_minimum_total_idle(0, sender=bunny) - event = list(tx.decode_logs(vault.UpdateMinimumTotalIdle)) - assert len(event) == 1 - assert event[0].minimum_total_idle == 0 - # close role - tx = vault.close_open_role(ROLES.MINIMUM_IDLE_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.MINIMUM_IDLE_MANAGER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts("not allowed"): - vault.set_minimum_total_idle(0, sender=bunny) - - -def test_set_deposit_limit__set_deposit_limit_role_open(vault, bunny, gov): - tx = vault.set_open_role(ROLES.DEPOSIT_LIMIT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.DEPOSIT_LIMIT_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.set_deposit_limit(0, sender=bunny) - event = list(tx.decode_logs(vault.UpdateDepositLimit)) - assert len(event) == 1 - assert event[0].deposit_limit == 0 - # close role - tx = vault.close_open_role(ROLES.DEPOSIT_LIMIT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.DEPOSIT_LIMIT_MANAGER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts("not allowed"): - vault.set_deposit_limit(0, sender=bunny) - - -def test_set_deposit_limit_module__set_deposit_limit_role_open(vault, bunny, gov): - tx = vault.set_open_role(ROLES.DEPOSIT_LIMIT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.DEPOSIT_LIMIT_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.set_deposit_limit_module(bunny, sender=bunny) - event = list(tx.decode_logs(vault.UpdateDepositLimitModule)) - assert len(event) == 1 - assert event[0].deposit_limit_module == bunny.address - assert vault.deposit_limit_module() == bunny.address - - # close role - tx = vault.close_open_role(ROLES.DEPOSIT_LIMIT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.DEPOSIT_LIMIT_MANAGER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts("not allowed"): - vault.set_deposit_limit_module(ZERO_ADDRESS, sender=bunny) - - -def test_set_withdraw_limit_module__set_withdraw_limit_role_open(vault, bunny, gov): - tx = vault.set_open_role(ROLES.WITHDRAW_LIMIT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.WITHDRAW_LIMIT_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.set_withdraw_limit_module(bunny, sender=bunny) - event = list(tx.decode_logs(vault.UpdateWithdrawLimitModule)) - assert len(event) == 1 - assert event[0].withdraw_limit_module == bunny.address - assert vault.withdraw_limit_module() == bunny.address - - # close role - tx = vault.close_open_role(ROLES.WITHDRAW_LIMIT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.WITHDRAW_LIMIT_MANAGER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts("not allowed"): - vault.set_withdraw_limit_module(ZERO_ADDRESS, sender=bunny) - - -def test_update_max_debt_for_strategy__set_max_debt_role_open_then_close__reverts( - vault, create_strategy, bunny, gov -): - tx = vault.set_open_role(ROLES.MAX_DEBT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.MAX_DEBT_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - new_strategy = create_strategy(vault) - vault.add_strategy(new_strategy, sender=gov) - tx = vault.update_max_debt_for_strategy(new_strategy, 420, sender=bunny) - event = list(tx.decode_logs(vault.UpdatedMaxDebtForStrategy)) - assert len(event) == 1 - assert ( - event[0].sender == bunny.address and event[0].strategy == new_strategy.address - ) - assert event[0].new_debt == 420 - # close role - tx = vault.close_open_role(ROLES.MAX_DEBT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.MAX_DEBT_MANAGER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts("not allowed"): - vault.update_max_debt_for_strategy(new_strategy, 420, sender=bunny) - - -# DEBT_PURCHASER - - -def test_buy_debt__debt_purchaser_role_closed__reverts(vault, strategy, bunny): - with ape.reverts("not allowed"): - vault.buy_debt(strategy.address, 0, sender=bunny) - - -def test_buy_debt__set_debt_purchaser_role_open( - vault, - strategy, - mint_and_deposit_into_vault, - add_debt_to_strategy, - fish_amount, - asset, - bunny, - gov, -): - amount = fish_amount - - mint_and_deposit_into_vault(vault, gov, amount) - add_debt_to_strategy(gov, strategy, vault, amount) - - # Approve vault to pull funds. - asset.mint(bunny.address, amount, sender=gov) - asset.approve(vault.address, amount, sender=bunny) - - tx = vault.set_open_role(ROLES.DEBT_PURCHASER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.DEBT_PURCHASER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.buy_debt(strategy.address, amount, sender=bunny) - event = list(tx.decode_logs(vault.DebtPurchased)) - - assert len(event) == 1 - assert event[0].strategy == strategy.address - assert event[0].amount == amount - - event = list(tx.decode_logs(vault.DebtUpdated)) - - assert len(event) == 1 - assert event[0].strategy == strategy.address - assert event[0].current_debt == amount - assert event[0].new_debt == 0 - - -def test_buy_debt__set_debt_purchaser_role_open_then_close__reverts( - vault, - strategy, - mint_and_deposit_into_vault, - add_debt_to_strategy, - fish_amount, - asset, - bunny, - gov, -): - amount = fish_amount - - mint_and_deposit_into_vault(vault, gov, amount) - add_debt_to_strategy(gov, strategy, vault, amount) - - # Approve vault to pull funds. - asset.mint(bunny.address, amount, sender=gov) - asset.approve(vault.address, amount, sender=bunny) - - vault.set_open_role(ROLES.DEBT_PURCHASER, sender=gov) - tx = vault.buy_debt(strategy.address, amount // 2, sender=bunny) - event = list(tx.decode_logs(vault.DebtPurchased)) - - assert len(event) == 1 - assert event[0].strategy == strategy.address - assert event[0].amount == amount // 2 - - event = list(tx.decode_logs(vault.DebtUpdated)) - - assert len(event) == 1 - assert event[0].strategy == strategy.address - assert event[0].current_debt == amount - assert event[0].new_debt == amount // 2 - # close role - - tx = vault.close_open_role(ROLES.DEBT_PURCHASER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.DEBT_PURCHASER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts("not allowed"): - vault.buy_debt(strategy.address, amount // 2, sender=bunny) - - -# DEBT_MANAGER - - -def test_update_debt__debt_role_closed__reverts(vault, create_strategy, bunny, gov): - new_strategy = create_strategy(vault) - vault.add_strategy(new_strategy, sender=gov) - with ape.reverts("not allowed"): - vault.update_debt(new_strategy, 0, sender=bunny) - - -def test_update_debt__set_debt_role_open( - vault, create_strategy, bunny, gov, mint_and_deposit_into_vault -): - new_strategy = create_strategy(vault) - vault.add_strategy(new_strategy, sender=gov) - vault.update_max_debt_for_strategy(new_strategy, 1338, sender=gov) - mint_and_deposit_into_vault(vault) - tx = vault.set_open_role(ROLES.DEBT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.DEBT_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.update_debt(new_strategy, 1337, sender=bunny) - event = list(tx.decode_logs(vault.DebtUpdated)) - assert len(event) == 1 - assert event[0].strategy == new_strategy.address and event[0].new_debt == 1337 - - -def test_update_debt__set_debt_role_open_then_close__reverts( - vault, create_strategy, bunny, gov, mint_and_deposit_into_vault -): - new_strategy = create_strategy(vault) - vault.add_strategy(new_strategy, sender=gov) - vault.update_max_debt_for_strategy(new_strategy, 1338, sender=gov) - mint_and_deposit_into_vault(vault) - tx = vault.set_open_role(ROLES.DEBT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.DEBT_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.update_debt(new_strategy, 1337, sender=bunny) - event = list(tx.decode_logs(vault.DebtUpdated)) - assert len(event) == 1 - assert event[0].strategy == new_strategy.address and event[0].new_debt == 1337 - # close role - tx = vault.close_open_role(ROLES.DEBT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.DEBT_MANAGER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts("not allowed"): - vault.update_debt(new_strategy, 1337, sender=bunny) - - -# ACCOUNTANT_MANAGER - - -def test_set_accountant__accountant_manager_closed__reverts(bunny, vault): - with ape.reverts("not allowed"): - vault.set_accountant(bunny, sender=bunny) - - -def test_set_accountant__accountant_manager_open(gov, vault, bunny): - # We temporarily give bunny the role of DEBT_MANAGER - tx = vault.set_open_role(ROLES.ACCOUNTANT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.ACCOUNTANT_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - assert vault.accountant() != bunny - vault.set_accountant(bunny, sender=bunny) - assert vault.accountant() == bunny - - -def test_set_accountant__accountant_manager_open_then_close__reverts( - gov, vault, bunny, fish -): - # We temporarily give bunny the role of DEBT_MANAGER - tx = vault.set_open_role(ROLES.ACCOUNTANT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.ACCOUNTANT_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - assert vault.accountant() != bunny - vault.set_accountant(bunny, sender=bunny) - assert vault.accountant() == bunny - - tx = vault.close_open_role(ROLES.ACCOUNTANT_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.ACCOUNTANT_MANAGER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts("not allowed"): - vault.set_accountant(fish, sender=fish) - - -# EMERGENCY_MANAGER - - -def test_shutdown_vault__emergency_role_closed__reverts(vault, bunny): - with ape.reverts("not allowed"): - vault.shutdown_vault(sender=bunny) - - -def test_shutdown_vault__set_emergency_role_open(vault, bunny, gov): - with ape.reverts(): - vault.shutdown_vault(sender=bunny) - - tx = vault.set_open_role(ROLES.EMERGENCY_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.EMERGENCY_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - tx = vault.shutdown_vault(sender=bunny) - event = list(tx.decode_logs(vault.Shutdown)) - assert len(event) == 1 - - -# QUEUE MANAGER - - -def test_set_default_queue__queue_manager_closed__reverts(bunny, vault): - with ape.reverts("not allowed"): - vault.set_default_queue([], sender=bunny) - - -def test_set_use_default_queue__queue_manager_closed__reverts(bunny, vault): - with ape.reverts("not allowed"): - vault.set_use_default_queue(True, sender=bunny) - - -def test_set_default_queue__queue_manager_open(gov, vault, strategy, bunny): - # We temporarily give bunny the role of DEBT_MANAGER - tx = vault.set_open_role(ROLES.QUEUE_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.QUEUE_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - assert vault.get_default_queue() != [] - vault.set_default_queue([], sender=bunny) - assert vault.get_default_queue() == [] - - -def test_set_use_default_queue__queue_manager_open(gov, vault, strategy, bunny): - # We temporarily give bunny the role of DEBT_MANAGER - tx = vault.set_open_role(ROLES.QUEUE_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.QUEUE_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - assert vault.use_default_queue() == False - tx = vault.set_use_default_queue(True, sender=bunny) - - event = list(tx.decode_logs(vault.UpdateUseDefaultQueue)) - assert len(event) == 1 - assert event[0].use_default_queue == True - assert vault.use_default_queue() == True - - -def test_set_default_queue__queue_manager_open_then_close__reverts( - gov, vault, strategy, bunny, fish -): - # We temporarily give bunny the role of DEBT_MANAGER - tx = vault.set_open_role(ROLES.QUEUE_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.QUEUE_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - assert vault.get_default_queue() != [] - vault.set_default_queue([], sender=bunny) - assert vault.get_default_queue() == [] - - tx = vault.close_open_role(ROLES.QUEUE_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.QUEUE_MANAGER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts("not allowed"): - vault.set_default_queue([], sender=fish) - - -def test_set_use_default_queue__queue_manager_open_then_close__reverts( - gov, vault, strategy, bunny, fish -): - # We temporarily give bunny the role of DEBT_MANAGER - tx = vault.set_open_role(ROLES.QUEUE_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.QUEUE_MANAGER - assert event[0].status == RoleStatusChange.OPENED - - assert vault.use_default_queue() == False - tx = vault.set_use_default_queue(True, sender=bunny) - - event = list(tx.decode_logs(vault.UpdateUseDefaultQueue)) - assert len(event) == 1 - assert event[0].use_default_queue == True - assert vault.use_default_queue() == True - - tx = vault.close_open_role(ROLES.QUEUE_MANAGER, sender=gov) - - event = list(tx.decode_logs(vault.RoleStatusChanged)) - assert len(event) == 1 - assert event[0].role == ROLES.QUEUE_MANAGER - assert event[0].status == RoleStatusChange.CLOSED - - with ape.reverts("not allowed"): - vault.set_use_default_queue(False, sender=fish) From f868c629be54d1f8768c3b2c092607b2ce1daf6d Mon Sep 17 00:00:00 2001 From: Schlag <89420541+Schlagonia@users.noreply.github.com> Date: Sun, 7 Jan 2024 10:12:10 -0700 Subject: [PATCH 03/16] feat: add max loss to debt updates (#190) --- TECH_SPEC.md | 2 +- contracts/VaultV3.vy | 18 +++- tests/unit/vault/test_debt_management.py | 118 +++++++++++++++++++---- 3 files changed, 113 insertions(+), 25 deletions(-) diff --git a/TECH_SPEC.md b/TECH_SPEC.md index 3c1fd131..cf6a8da5 100644 --- a/TECH_SPEC.md +++ b/TECH_SPEC.md @@ -163,7 +163,7 @@ This responsibility is taken by callers with DEBT_MANAGER role This role can increase or decrease strategies specific debt. -The vault sends and receives funds to/from strategies. The function updateDebt(strategy, target_debt) will set the current_debt of the strategy to target_debt (if possible) +The vault sends and receives funds to/from strategies. The function update_debt(strategy, target_debt, max_loss) will set the current_debt of the strategy to target_debt (if possible) If the strategy currently has less debt than the target_debt, the vault will send funds to it. diff --git a/contracts/VaultV3.vy b/contracts/VaultV3.vy index 791d21cc..288a8376 100644 --- a/contracts/VaultV3.vy +++ b/contracts/VaultV3.vy @@ -1016,7 +1016,7 @@ def _revoke_strategy(strategy: address, force: bool=False): # DEBT MANAGEMENT # @internal -def _update_debt(strategy: address, target_debt: uint256) -> uint256: +def _update_debt(strategy: address, target_debt: uint256, max_loss: uint256) -> uint256: """ The vault will re-balance the debt vs target debt. Target debt must be smaller or equal to strategy's max_debt. This function will compare the @@ -1076,8 +1076,13 @@ def _update_debt(strategy: address, target_debt: uint256) -> uint256: # We pull funds with {redeem} so there can be losses or rounding differences. withdrawn: uint256 = min(post_balance - pre_balance, current_debt) + # 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" + # If we got too much make sure not to increase PPS. - if withdrawn > assets_to_withdraw: + elif withdrawn > assets_to_withdraw: assets_to_withdraw = withdrawn # Update storage. @@ -1669,15 +1674,20 @@ def update_max_debt_for_strategy(strategy: address, new_max_debt: uint256): @external @nonreentrant("lock") -def update_debt(strategy: address, target_debt: uint256) -> uint256: +def update_debt( + strategy: address, + target_debt: uint256, + max_loss: uint256 = MAX_BPS +) -> uint256: """ @notice Update the debt for a strategy. @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. """ self._enforce_role(msg.sender, Roles.DEBT_MANAGER) - return self._update_debt(strategy, target_debt) + return self._update_debt(strategy, target_debt, max_loss) ## EMERGENCY MANAGEMENT ## @external diff --git a/tests/unit/vault/test_debt_management.py b/tests/unit/vault/test_debt_management.py index d10f44d0..37eccf0f 100644 --- a/tests/unit/vault/test_debt_management.py +++ b/tests/unit/vault/test_debt_management.py @@ -207,26 +207,6 @@ def test_update_debt__with_new_debt_greater_than_max_desired_debt( assert vault.totalDebt() == initial_debt + difference -# def test_update_debt__with_new_debt_less_than_min_desired_debt__reverts( -# gov, asset, vault, strategy, add_debt_to_strategy -# ): -# vault_balance = asset.balanceOf(vault) -# current_debt = vault_balance // 2 -# new_debt = vault_balance -# min_desired_debt = vault_balance * 2 -# -# # set existing debt -# add_debt_to_strategy(gov, strategy, vault, current_debt) -# -# # set new max debt lower than min debt -# vault.update_max_debt_for_strategy(strategy.address, new_debt, sender=gov) -# strategy.setMinDebt(min_desired_debt, sender=gov) -# -# with ape.reverts("new debt less than min debt"): -# vault.update_debt(strategy.address, sender=gov) -# - - @pytest.mark.parametrize("minimum_total_idle", [0, 10**21]) def test_set_minimum_total_idle__with_minimum_total_idle( gov, vault, minimum_total_idle @@ -495,6 +475,51 @@ def test_update_debt__with_lossy_strategy_that_withdraws_less_than_requested( assert vault.totalDebt() == new_debt +def test_update_debt__with_lossy_strategy_that_withdraws_less_than_requested__max_loss( + gov, asset, vault, lossy_strategy, add_debt_to_strategy +): + vault_balance = asset.balanceOf(vault) + + add_debt_to_strategy(gov, lossy_strategy, vault, vault_balance) + + initial_idle = vault.totalIdle() + initial_debt = vault.totalDebt() + current_debt = vault.strategies(lossy_strategy.address).current_debt + loss = current_debt // 10 + new_debt = 0 + difference = current_debt - loss + + lossy_strategy.setWithdrawingLoss(loss, sender=gov) + + initial_pps = vault.pricePerShare() + + # With 0 max loss should revert. + with ape.reverts("too much loss"): + vault.update_debt(lossy_strategy.address, 0, 0, sender=gov) + + # Up to the loss percent still reverts + with ape.reverts("too much loss"): + vault.update_debt(lossy_strategy.address, 0, 999, sender=gov) + + # Over the loss percent will succeed and account correctly. + tx = vault.update_debt(lossy_strategy.address, 0, 1_000, sender=gov) + event = list(tx.decode_logs(vault.DebtUpdated)) + + # Should have recorded the loss + assert len(event) == 1 + assert event[0].strategy == lossy_strategy.address + assert event[0].current_debt == current_debt + assert event[0].new_debt == new_debt + + # assert we got back 90% of requested and it recorded the loss. + assert vault.pricePerShare() < initial_pps + assert vault.strategies(lossy_strategy.address).current_debt == new_debt + assert asset.balanceOf(lossy_strategy) == new_debt + assert asset.balanceOf(vault) == (vault_balance - loss) + assert vault.totalIdle() == initial_idle + difference + assert vault.totalDebt() == new_debt + + def test_update_debt__with_faulty_strategy_that_withdraws_more_than_requested__only_half_withdrawn( gov, asset, vault, lossy_strategy, add_debt_to_strategy, airdrop_asset ): @@ -638,3 +663,56 @@ def test_update_debt__with_lossy_strategy_that_withdraws_less_than_requested_wit assert asset.balanceOf(vault) == (vault_balance - loss + fish_amount) assert vault.totalIdle() == initial_idle + difference assert vault.totalDebt() == new_debt + + +def test_update_debt__with_lossy_strategy_that_withdraws_less_than_requested_with_airdrop_and_max_loss( + gov, + asset, + vault, + lossy_strategy, + add_debt_to_strategy, + airdrop_asset, + fish_amount, +): + vault_balance = asset.balanceOf(vault) + + add_debt_to_strategy(gov, lossy_strategy, vault, vault_balance) + + initial_idle = vault.totalIdle() + initial_debt = vault.totalDebt() + current_debt = vault.strategies(lossy_strategy.address).current_debt + loss = current_debt // 10 + new_debt = 0 + difference = current_debt - loss + + lossy_strategy.setWithdrawingLoss(loss, sender=gov) + + initial_pps = vault.pricePerShare() + + # airdrop some asset to the vault + airdrop_asset(gov, asset, vault, fish_amount) + + # With 0 max loss should revert. + with ape.reverts("too much loss"): + vault.update_debt(lossy_strategy.address, 0, 0, sender=gov) + + # Up to the loss percent still reverts + with ape.reverts("too much loss"): + vault.update_debt(lossy_strategy.address, 0, 999, sender=gov) + + # At the amount doesn't revert + tx = vault.update_debt(lossy_strategy.address, 0, 1_000, sender=gov) + event = list(tx.decode_logs(vault.DebtUpdated)) + + assert len(event) == 1 + assert event[0].strategy == lossy_strategy.address + assert event[0].current_debt == current_debt + assert event[0].new_debt == new_debt + + # assert we only got back half of what was requested and the vault recorded it correctly + assert vault.pricePerShare() < initial_pps + assert vault.strategies(lossy_strategy.address).current_debt == new_debt + assert asset.balanceOf(lossy_strategy) == new_debt + assert asset.balanceOf(vault) == (vault_balance - loss + fish_amount) + assert vault.totalIdle() == initial_idle + difference + assert vault.totalDebt() == new_debt From d39e2ae75136985b33fca3c3f3c8f302343ce053 Mon Sep 17 00:00:00 2001 From: Schlag <89420541+Schlagonia@users.noreply.github.com> Date: Sat, 27 Jan 2024 13:31:18 -0700 Subject: [PATCH 04/16] feat: use cloning for vaults (#191) * feat: use cloning for vaults * fix: scripts * chore: fix interfaces * chore: lower case factory --- .github/workflows/test.yaml | 1 - contracts/VaultFactory.vy | 60 ++++---- contracts/VaultV3.vy | 128 ++++++++++-------- contracts/interfaces/IVault.sol | 31 ++--- contracts/interfaces/IVaultFactory.sol | 4 +- contracts/interfaces/Roles.sol | 21 +++ contracts/interfaces/VaultConstants.sol | 23 +--- .../mocks/ERC4626/MockTokenizedStrategy.sol | 2 +- scripts/deploy.py | 39 ++---- tests/conftest.py | 24 +--- tests/unit/factory/test_factory.py | 41 +++++- tests/unit/vault/test_shares.py | 13 +- 12 files changed, 210 insertions(+), 177 deletions(-) create mode 100644 contracts/interfaces/Roles.sol diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 3b967b9b..09c85dc7 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -24,7 +24,6 @@ jobs: with: python-version: '3.10' ape-version-pin: "==0.7.0" - #ape-plugins-list: 'solidity==0.6.0 vyper==0.6.1 hardhat==0.6.0' - name: install vyper run: pip install git+https://github.com/vyperlang/vyper diff --git a/contracts/VaultFactory.vy b/contracts/VaultFactory.vy index 8f42f805..7bc0076e 100644 --- a/contracts/VaultFactory.vy +++ b/contracts/VaultFactory.vy @@ -8,11 +8,9 @@ This vault Factory can be used by anyone wishing to deploy their own ERC4626 compliant Yearn V3 Vault of the same API version. - The factory uses the Blueprint (ERC-5202) standard to handle the - deployment of any new vaults off of the immutable address stored - at `VAULT_BLUEPRINT`. This allows the vaults to be deployed and - initialized fully on-chain with their init byte code, thus not - requiring any delegatecall patterns or post deployment initialization. + The factory clones new vaults from its specific `VAULT_ORIGINAL` + immutable address set on creation of the factory. + The deployments are done through create2 with a specific `salt` that is derived from a combination of the deployer's address, the underlying asset used, as well as the name and symbol specified. @@ -33,6 +31,15 @@ from vyper.interfaces import ERC20 +interface IVault: + def initialize( + asset: address, + name: String[64], + symbol: String[32], + role_manager: address, + profit_max_unlock_time: uint256 + ): nonpayable + event NewVault: vault_address: indexed(address) asset: indexed(address) @@ -74,7 +81,7 @@ API_VERSION: constant(String[28]) = "3.0.2" MAX_FEE_BPS: constant(uint16) = 5_000 # 50% # The address that all newly deployed vaults are based from. -VAULT_BLUEPRINT: immutable(address) +VAULT_ORIGINAL: immutable(address) # State of the Factory. If True no new vaults can be deployed. shutdown: public(bool) @@ -95,21 +102,21 @@ custom_protocol_fee: public(HashMap[address, uint16]) use_custom_protocol_fee: public(HashMap[address, bool]) @external -def __init__(name: String[64], vault_blueprint: address, governance: address): +def __init__(name: String[64], vault_original: address, governance: address): self.name = name - VAULT_BLUEPRINT = vault_blueprint + VAULT_ORIGINAL = vault_original self.governance = governance @external def deploy_new_vault( - asset: ERC20, + asset: address, name: String[64], symbol: String[32], role_manager: address, profit_max_unlock_time: uint256 ) -> address: """ - @notice Deploys a new vault base on the bLueprint. + @notice Deploys a new clone of the original vault. @param asset The asset to be used for the vault. @param name The name of the new vault. @param symbol The symbol of the new vault. @@ -120,29 +127,32 @@ def deploy_new_vault( # Make sure the factory is not shutdown. assert not self.shutdown, "shutdown" - # Deploy the new vault using the blueprint. - vault_address: address = create_from_blueprint( - VAULT_BLUEPRINT, - asset, - name, - symbol, - role_manager, - profit_max_unlock_time, - code_offset=3, - salt=keccak256(_abi_encode(msg.sender, asset.address, name, symbol)) + # Clone a new version of the vault using create2. + vault_address: address = create_minimal_proxy_to( + VAULT_ORIGINAL, + value=0, + salt=keccak256(_abi_encode(msg.sender, asset, name, symbol)) ) + + IVault(vault_address).initialize( + asset, + name, + symbol, + role_manager, + profit_max_unlock_time, + ) - log NewVault(vault_address, asset.address) + log NewVault(vault_address, asset) return vault_address @view @external -def vault_blueprint()-> address: +def vault_original()-> address: """ - @notice Get the address of the vault blueprint - @return The address of the vault blueprint + @notice Get the address of the vault to clone from + @return The address of the original vault. """ - return VAULT_BLUEPRINT + return VAULT_ORIGINAL @view @external diff --git a/contracts/VaultV3.vy b/contracts/VaultV3.vy index 288a8376..1118049f 100644 --- a/contracts/VaultV3.vy +++ b/contracts/VaultV3.vy @@ -200,15 +200,14 @@ enum Rounding: ROUND_DOWN ROUND_UP -# IMMUTABLE # +# STORAGE # # Underlying token used by the vault. -ASSET: immutable(ERC20) +asset: public(address) # Based off the `asset` decimals. -DECIMALS: immutable(uint256) +decimals: public(uint256) # Deployer contract used to retrieve the protocol fee config. -FACTORY: public(immutable(address)) +factory: address -# STORAGE # # HashMap that records all the strategies that are allowed to receive assets from the vault. strategies: public(HashMap[address, StrategyParams]) # The current default withdrawal queue. @@ -272,8 +271,13 @@ PERMIT_TYPE_HASH: constant(bytes32) = keccak256("Permit(address owner,address sp # Constructor @external -def __init__( - asset: ERC20, +def __init__(): + # Set `asset` so it cannot be re-initialized. + self.asset = self + +@external +def initialize( + asset: address, name: String[64], symbol: String[32], role_manager: address, @@ -281,7 +285,7 @@ def __init__( ): """ @notice - The constructor for the vault. Sets the asset, name, symbol, and role manager. + Initialize a new vault. Sets the asset, name, symbol, and role manager. @param asset The address of the asset that the vault will accept. @param name @@ -293,11 +297,16 @@ def __init__( @param profit_max_unlock_time The amount of time that the profit will be locked for """ - ASSET = asset - DECIMALS = convert(ERC20Detailed(asset.address).decimals(), uint256) - assert DECIMALS < 256 # dev: see VVE-2020-0001 + assert self.asset == empty(address), "initialized" + assert asset != empty(address), "ZERO ADDRESS" + + self.asset = asset + decimals: uint256 = convert(ERC20Detailed(asset).decimals(), uint256) + assert decimals < 256 # dev: see VVE-2020-0001 + self.decimals = decimals - FACTORY = msg.sender + # Set the factory as the deployer address. + self.factory = msg.sender # Must be less than one year for report cycles assert profit_max_unlock_time <= 31_556_952 # dev: profit unlock time too long @@ -678,7 +687,7 @@ def _deposit(sender: address, recipient: address, assets: uint256) -> uint256: assert assets <= self._max_deposit(recipient), "exceed deposit limit" # Transfer the tokens to the vault first. - self._erc20_safe_transfer_from(ASSET.address, msg.sender, self, assets) + self._erc20_safe_transfer_from(self.asset, msg.sender, self, assets) # Record the change in total assets. self.total_idle += assets @@ -705,7 +714,7 @@ def _mint(sender: address, recipient: address, shares: uint256) -> uint256: assert assets <= self._max_deposit(recipient), "exceed deposit limit" # Transfer the tokens to the vault first. - self._erc20_safe_transfer_from(ASSET.address, msg.sender, self, assets) + self._erc20_safe_transfer_from(self.asset, msg.sender, self, assets) # Record the change in total assets. self.total_idle += assets @@ -805,11 +814,12 @@ def _redeem( requested_assets: uint256 = assets # load to memory to save gas - curr_total_idle: uint256 = self.total_idle - + current_total_idle: uint256 = self.total_idle + _asset: address = self.asset + # If there are not enough assets in the Vault contract, we try to free # funds from strategies. - if requested_assets > curr_total_idle: + if requested_assets > current_total_idle: # Cache the default queue. _strategies: DynArray[address, MAX_QUEUE] = self.default_queue @@ -820,16 +830,16 @@ def _redeem( _strategies = strategies # load to memory to save gas - curr_total_debt: uint256 = self.total_debt + current_total_debt: uint256 = self.total_debt # Withdraw from strategies only what idle doesn't cover. # `assets_needed` is the total amount we need to fill the request. - assets_needed: uint256 = unsafe_sub(requested_assets, curr_total_idle) + assets_needed: uint256 = unsafe_sub(requested_assets, current_total_idle) # `assets_to_withdraw` is the amount to request from the current strategy. assets_to_withdraw: uint256 = 0 # To compare against real withdrawals from strategies - previous_balance: uint256 = ASSET.balanceOf(self) + previous_balance: uint256 = ERC20(_asset).balanceOf(self) for strategy in _strategies: # Make sure we have a valid strategy. @@ -871,7 +881,7 @@ def _redeem( # NOTE: done here instead of waiting for regular update of these values # because it's a rare case (so we can save minor amounts of gas) assets_needed -= unrealised_losses_share - curr_total_debt -= unrealised_losses_share + current_total_debt -= unrealised_losses_share # If max withdraw is 0 and unrealised loss is still > 0 then the strategy likely # realized a 100% loss and we will need to realize that loss before moving on. @@ -893,7 +903,7 @@ def _redeem( # WITHDRAW FROM STRATEGY self._withdraw_from_strategy(strategy, assets_to_withdraw) - post_balance: uint256 = ASSET.balanceOf(self) + post_balance: uint256 = ERC20(_asset).balanceOf(self) # Always check withdrawn against the real amounts. withdrawn: uint256 = post_balance - previous_balance @@ -914,9 +924,9 @@ 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). - curr_total_idle += (assets_to_withdraw - loss) + current_total_idle += (assets_to_withdraw - loss) requested_assets -= loss - curr_total_debt -= assets_to_withdraw + current_total_debt -= assets_to_withdraw # Vault will reduce debt because the unrealised loss has been taken by user new_debt: uint256 = current_debt - (assets_to_withdraw + unrealised_losses_share) @@ -927,7 +937,7 @@ def _redeem( log DebtUpdated(strategy, current_debt, new_debt) # Break if we have enough total idle to serve initial request. - if requested_assets <= curr_total_idle: + if requested_assets <= current_total_idle: break # We update the previous_balance variable here to save gas in next iteration. @@ -938,9 +948,9 @@ def _redeem( assets_needed -= assets_to_withdraw # If we exhaust the queue and still have insufficient total idle, revert. - assert curr_total_idle >= requested_assets, "insufficient assets in vault" + assert current_total_idle >= requested_assets, "insufficient assets in vault" # Commit memory to storage. - self.total_debt = curr_total_debt + self.total_debt = current_total_debt # Check if there is a loss and a non-default value was set. if assets > requested_assets and max_loss < MAX_BPS: @@ -950,9 +960,9 @@ def _redeem( # First burn the corresponding shares from the redeemer. self._burn_shares(shares, owner) # Commit memory to storage. - self.total_idle = curr_total_idle - requested_assets + self.total_idle = current_total_idle - requested_assets # Transfer the requested amount to the receiver. - self._erc20_safe_transfer(ASSET.address, receiver, requested_assets) + self._erc20_safe_transfer(_asset, receiver, requested_assets) log Withdraw(sender, receiver, owner, requested_assets, shares) return requested_assets @@ -961,7 +971,7 @@ def _redeem( @internal def _add_strategy(new_strategy: address): assert new_strategy not in [self, empty(address)], "strategy cannot be zero address" - assert IStrategy(new_strategy).asset() == ASSET.address, "invalid asset" + assert IStrategy(new_strategy).asset() == self.asset, "invalid asset" assert self.strategies[new_strategy].activation == 0, "strategy already active" # Add the new strategy to the mapping. @@ -1067,10 +1077,13 @@ def _update_debt(strategy: address, target_debt: uint256, max_loss: uint256) -> unrealised_losses_share: uint256 = self._assess_share_of_unrealised_losses(strategy, assets_to_withdraw) assert unrealised_losses_share == 0, "strategy has unrealised losses" + # Cache for repeated use. + _asset: address = self.asset + # Always check the actual amount withdrawn. - pre_balance: uint256 = ASSET.balanceOf(self) + pre_balance: uint256 = ERC20(_asset).balanceOf(self) self._withdraw_from_strategy(strategy, assets_to_withdraw) - post_balance: uint256 = ASSET.balanceOf(self) + post_balance: uint256 = ERC20(_asset).balanceOf(self) # making sure we are changing idle according to the real result no matter what. # We pull funds with {redeem} so there can be losses or rounding differences. @@ -1120,16 +1133,19 @@ def _update_debt(strategy: address, target_debt: uint256, max_loss: uint256) -> # Can't Deposit 0. if assets_to_deposit > 0: + # Cache for repeated use. + _asset: address = self.asset + # Approve the strategy to pull only what we are giving it. - self._erc20_safe_approve(ASSET.address, strategy, assets_to_deposit) + self._erc20_safe_approve(_asset, strategy, assets_to_deposit) # Always update based on actual amounts deposited. - pre_balance: uint256 = ASSET.balanceOf(self) + pre_balance: uint256 = ERC20(_asset).balanceOf(self) IStrategy(strategy).deposit(assets_to_deposit, self) - post_balance: uint256 = ASSET.balanceOf(self) + post_balance: uint256 = ERC20(_asset).balanceOf(self) # Make sure our approval is always back to 0. - self._erc20_safe_approve(ASSET.address, strategy, 0) + self._erc20_safe_approve(_asset, strategy, 0) # Making sure we are changing according to the real result no # matter what. This will spend more gas but makes it more robust. @@ -1208,7 +1224,7 @@ def _process_report(strategy: address) -> (uint256, uint256): if total_fees > 0: protocol_fee_bps: uint16 = 0 # Get the config for this vault. - protocol_fee_bps, protocol_fee_recipient = IFactory(FACTORY).protocol_fee_config() + protocol_fee_bps, protocol_fee_recipient = IFactory(self.factory).protocol_fee_config() if(protocol_fee_bps > 0): # Protocol fees are a percent of the fees the accountant is charging. @@ -1234,10 +1250,12 @@ def _process_report(strategy: address) -> (uint256, uint256): # Shares to lock is any amounts that would otherwise increase the vaults PPS. newly_locked_shares: uint256 = 0 if total_refunds > 0: + # Load `asset` to memory. + _asset: address = self.asset # Make sure we have enough approval and enough asset to pull. - total_refunds = min(total_refunds, min(ASSET.balanceOf(accountant), ASSET.allowance(accountant, self))) + total_refunds = min(total_refunds, min(ERC20(_asset).balanceOf(accountant), ERC20(_asset).allowance(accountant, self))) # Transfer the refunded amount of asset to the vault. - self._erc20_safe_transfer_from(ASSET.address, accountant, self, total_refunds) + self._erc20_safe_transfer_from(_asset, accountant, self, total_refunds) # Update storage to increase total assets. self.total_idle += total_refunds @@ -1552,7 +1570,7 @@ def pricePerShare() -> uint256: exact precision should use convertToAssets or convertToShares instead. @return The price per share. """ - return self._convert_to_assets(10 ** DECIMALS, Rounding.ROUND_DOWN) + return self._convert_to_assets(10 ** self.decimals, Rounding.ROUND_DOWN) @view @external @@ -1608,7 +1626,7 @@ def buy_debt(strategy: address, amount: uint256): assert shares > 0, "cannot buy zero" - self._erc20_safe_transfer_from(ASSET.address, msg.sender, self, _amount) + self._erc20_safe_transfer_from(self.asset, msg.sender, self, _amount) # Lower strategy debt self.strategies[strategy].current_debt -= _amount @@ -1866,24 +1884,6 @@ def totalSupply() -> uint256: """ return self._total_supply() -@view -@external -def asset() -> address: - """ - @notice Get the address of the asset. - @return The address of the asset. - """ - return ASSET.address - -@view -@external -def decimals() -> uint8: - """ - @notice Get the number of decimals of the asset/share. - @return The number of decimals of the asset/share. - """ - return convert(DECIMALS, uint8) - @view @external def totalAssets() -> uint256: @@ -2031,6 +2031,16 @@ def previewRedeem(shares: uint256) -> uint256: """ return self._convert_to_assets(shares, Rounding.ROUND_DOWN) +@view +@external +def FACTORY() -> address: + """ + @notice Address of the factory that deployed the vault. + @dev Is used to retrieve the protocol fees. + @return Address of the vault factory. + """ + return self.factory + @view @external def apiVersion() -> String[28]: diff --git a/contracts/interfaces/IVault.sol b/contracts/interfaces/IVault.sol index 847785b2..3577f05e 100644 --- a/contracts/interfaces/IVault.sol +++ b/contracts/interfaces/IVault.sol @@ -23,7 +23,6 @@ interface IVault is IERC4626 { ); // ROLE UPDATES event RoleSet(address indexed account, uint256 role); - event RoleStatusChanged(uint256 role, uint256 status); event UpdateRoleManager(address indexed role_manager); event UpdateAccountant(address indexed accountant); @@ -69,8 +68,6 @@ interface IVault is IERC4626 { function roles(address) external view returns (uint256); - function open_roles(uint256) external view returns (bool); - function role_manager() external view returns (address); function future_role_manager() external view returns (address); @@ -79,6 +76,14 @@ interface IVault is IERC4626 { function nonces(address) external view returns (uint256); + function initialize( + address, + string memory, + string memory, + address, + uint256 + ) external; + function set_accountant(address new_accountant) external; function set_default_queue(address[] memory new_default_queue) external; @@ -107,10 +112,6 @@ interface IVault is IERC4626 { function remove_role(address account, uint256 role) external; - function set_open_role(uint256 role) external; - - function close_open_role(uint256 role) external; - function transfer_role_manager(address role_manager) external; function accept_role_manager() external; @@ -143,6 +144,12 @@ interface IVault is IERC4626 { uint256 target_debt ) external returns (uint256); + function update_debt( + address strategy, + uint256 target_debt, + uint256 max_loss + ) external returns (uint256); + function shutdown_vault() external; function totalIdle() external view returns (uint256); @@ -220,16 +227,6 @@ interface IVault is IERC4626 { //// NON-STANDARD ERC-20 FUNCTIONS \\\\ - function increaseAllowance( - address spender, - uint256 amount - ) external returns (bool); - - function decreaseAllowance( - address spender, - uint256 amount - ) external returns (bool); - function DOMAIN_SEPARATOR() external view returns (bytes32); function permit( diff --git a/contracts/interfaces/IVaultFactory.sol b/contracts/interfaces/IVaultFactory.sol index 11e14ed6..88e196ad 100644 --- a/contracts/interfaces/IVaultFactory.sol +++ b/contracts/interfaces/IVaultFactory.sol @@ -34,14 +34,14 @@ interface IVaultFactory { function use_custom_protocol_fee(address) external view returns (bool); function deploy_new_vault( - ERC20 asset, + address asset, string memory name, string memory symbol, address role_manager, uint256 profit_max_unlock_time ) external returns (address); - function vault_blueprint() external view returns (address); + function vault_original() external view returns (address); function apiVersion() external view returns (string memory); diff --git a/contracts/interfaces/Roles.sol b/contracts/interfaces/Roles.sol new file mode 100644 index 00000000..1fac88bd --- /dev/null +++ b/contracts/interfaces/Roles.sol @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity 0.8.18; + +// prettier-ignore +library Roles { + uint256 internal constant ADD_STRATEGY_MANAGER = 1; + uint256 internal constant REVOKE_STRATEGY_MANAGER = 2; + uint256 internal constant FORCE_REVOKE_MANAGER = 4; + uint256 internal constant ACCOUNTANT_MANAGER = 8; + uint256 internal constant QUEUE_MANAGER = 16; + uint256 internal constant REPORTING_MANAGER = 32; + uint256 internal constant DEBT_MANAGER = 64; + uint256 internal constant MAX_DEBT_MANAGER = 128; + uint256 internal constant DEPOSIT_LIMIT_MANAGER = 256; + uint256 internal constant WITHDRAW_LIMIT_MANAGER = 512; + uint256 internal constant MINIMUM_IDLE_MANAGER = 1024; + uint256 internal constant PROFIT_UNLOCK_MANAGER = 2048; + uint256 internal constant DEBT_PURCHASER = 4096; + uint256 internal constant EMERGENCY_MANAGER = 8192; + uint256 internal constant ALL = 16383; +} diff --git a/contracts/interfaces/VaultConstants.sol b/contracts/interfaces/VaultConstants.sol index a2337b40..d1246f12 100644 --- a/contracts/interfaces/VaultConstants.sol +++ b/contracts/interfaces/VaultConstants.sol @@ -2,31 +2,10 @@ pragma solidity 0.8.18; // prettier-ignore -contract Roles { - uint256 public constant ADD_STRATEGY_MANAGER = 1; - uint256 public constant REVOKE_STRATEGY_MANAGER = 2; - uint256 public constant FORCE_REVOKE_MANAGER = 4; - uint256 public constant ACCOUNTANT_MANAGER = 8; - uint256 public constant QUEUE_MANAGER = 16; - uint256 public constant REPORTING_MANAGER = 32; - uint256 public constant DEBT_MANAGER = 64; - uint256 public constant MAX_DEBT_MANAGER = 128; - uint256 public constant DEPOSIT_LIMIT_MANAGER = 256; - uint256 public constant WITHDRAW_LIMIT_MANAGER = 512; - uint256 public constant MINIMUM_IDLE_MANAGER = 1024; - uint256 public constant PROFIT_UNLOCK_MANAGER = 2048; - uint256 public constant DEBT_PURCHASER = 4096; - uint256 public constant EMERGENCY_MANAGER = 8192; - uint256 public constant ALL = 16383; -} - -// prettier-ignore -contract VaultConstants is Roles { +contract VaultConstants { uint256 public constant MAX_QUEUE = 10; uint256 public constant MAX_BPS = 10_000; uint256 public constant MAX_BPS_EXTENDED = 1_000_000_000_000; uint256 public constant STRATEGY_ADDED = 1; uint256 public constant STRATEGY_REVOKED = 2; - uint256 public constant ROLE_OPENED = 1; - uint256 public constant ROLE_CLOSED = 2; } diff --git a/contracts/test/mocks/ERC4626/MockTokenizedStrategy.sol b/contracts/test/mocks/ERC4626/MockTokenizedStrategy.sol index 37774fa2..d7e385a7 100644 --- a/contracts/test/mocks/ERC4626/MockTokenizedStrategy.sol +++ b/contracts/test/mocks/ERC4626/MockTokenizedStrategy.sol @@ -58,7 +58,7 @@ contract MockTokenizedStrategy is TokenizedStrategy { function availableDepositLimit( address ) public view virtual returns (uint256) { - uint256 _totalAssets = strategyStorage().totalIdle; + uint256 _totalAssets = strategyStorage().totalAssets; uint256 _maxDebt = maxDebt; return _maxDebt > _totalAssets ? _maxDebt - _totalAssets : 0; } diff --git a/scripts/deploy.py b/scripts/deploy.py index 9e783672..268a0bd3 100644 --- a/scripts/deploy.py +++ b/scripts/deploy.py @@ -10,7 +10,7 @@ deployer = accounts.load("") -def deploy_blueprint_and_factory(): +def deploy_original_and_factory(): print("Deploying Vault Factory on ChainID", chain.chain_id) if input("Do you want to continue? ") == "n": @@ -35,43 +35,26 @@ def deploy_blueprint_and_factory(): print(f"Salt we are using {salt}") print("Init balance:", deployer.balance / 1e18) - # generate and deploy blueprint - vault_copy = deepcopy(vault) - blueprint_bytecode = b"\xFE\x71\x00" + HexBytes( - vault_copy.contract_type.deployment_bytecode.bytecode - ) - len_bytes = len(blueprint_bytecode).to_bytes(2, "big") - blueprint_constructor = vault_copy.constructor.encode_input( - ZERO_ADDRESS, "", "", ZERO_ADDRESS, 0 - ) - - # ERC5202 - blueprint_deploy_bytecode = HexBytes( - b"\x61" - + len_bytes - + b"\x3d\x81\x60\x0a\x3d\x39\xf3" - + blueprint_bytecode - + blueprint_constructor - ) + print(f"Deploying Original...") - print(f"Deploying BluePrint...") + original_deploy_bytecode = vault.contract_type.deployment_bytecode.bytecode - blueprint_tx = deployer_contract.deploy( - blueprint_deploy_bytecode, salt, sender=deployer + original_tx = deployer_contract.deploy( + original_deploy_bytecode, salt, sender=deployer ) - blueprint_event = list(blueprint_tx.decode_logs(deployer_contract.Deployed)) + original_event = list(original_tx.decode_logs(deployer_contract.Deployed)) - blueprint_address = blueprint_event[0].addr + original_address = original_event[0].addr - print(f"Deployed the vault Blueprint to {blueprint_address}") + print(f"Deployed the vault original to {original_address}") # deploy factory print(f"Deploying factory...") factory_constructor = vault_factory.constructor.encode_input( - "Yearn v3.0.1 Vault Factory", - blueprint_address, + "Yearn v3.0.2 Vault Factory", + original_address, "0x33333333D5eFb92f19a5F94a43456b3cec2797AE", ) @@ -92,4 +75,4 @@ def deploy_blueprint_and_factory(): def main(): - deploy_blueprint_and_factory() + deploy_original_and_factory() diff --git a/tests/conftest.py b/tests/conftest.py index 96338497..0725044a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -139,29 +139,17 @@ def create_token(name, decimals=18): @pytest.fixture(scope="session") -def vault_blueprint(project, gov): - blueprint_bytecode = b"\xFE\x71\x00" + HexBytes( - project.VaultV3.contract_type.deployment_bytecode.bytecode - ) # ERC5202 - len_bytes = len(blueprint_bytecode).to_bytes(2, "big") - deploy_bytecode = HexBytes( - b"\x61" + len_bytes + b"\x3d\x81\x60\x0a\x3d\x39\xf3" + blueprint_bytecode - ) - - c = w3.eth.contract(abi=[], bytecode=deploy_bytecode) - deploy_transaction = c.constructor() - tx_info = {"from": gov.address, "value": 0, "gasPrice": 0} - tx_hash = deploy_transaction.transact(tx_info) - - return w3.eth.get_transaction_receipt(tx_hash)["contractAddress"] +def vault_original(project, gov): + vault = gov.deploy(project.VaultV3) + return vault.address @pytest.fixture(scope="session") -def vault_factory(project, gov, vault_blueprint): +def vault_factory(project, gov, vault_original): return gov.deploy( project.VaultFactory, - "Vault V3 Factory 3.0.1-beta", - vault_blueprint, + "Vault V3 Factory test", + vault_original, gov.address, ) diff --git a/tests/unit/factory/test_factory.py b/tests/unit/factory/test_factory.py index e8638fd3..d53ced61 100644 --- a/tests/unit/factory/test_factory.py +++ b/tests/unit/factory/test_factory.py @@ -4,7 +4,7 @@ def test_new_vault_with_different_salt(gov, asset, bunny, fish, vault_factory): - assert vault_factory.name() == "Vault V3 Factory 3.0.1-beta" + assert vault_factory.name() == "Vault V3 Factory test" tx = vault_factory.deploy_new_vault( asset.address, @@ -117,3 +117,42 @@ def test__shutdown_factory__reverts(gov, asset, bunny, vault_factory): with ape.reverts("not governance"): vault_factory.shutdown_factory(sender=bunny) + + +def test_reinitialize_vault__reverst(gov, asset, bunny, vault_factory): + # Can't initialize the original + original = project.VaultV3.at(vault_factory.vault_original()) + + with ape.reverts("initialized"): + original.initialize( + asset.address, + "first_vault", + "fv", + bunny.address, + WEEK, + sender=gov, + ) + + tx = vault_factory.deploy_new_vault( + asset.address, + "first_vault", + "fv", + bunny.address, + WEEK, + sender=gov, + ) + event = list(tx.decode_logs(vault_factory.NewVault)) + new_vault = project.VaultV3.at(event[0].vault_address) + assert new_vault.name() == "first_vault" + assert new_vault.role_manager() == bunny.address + + # Can't reinitialze a new vault. + with ape.reverts("initialized"): + new_vault.initialize( + asset.address, + "first_vault", + "fv", + bunny.address, + WEEK, + sender=gov, + ) diff --git a/tests/unit/vault/test_shares.py b/tests/unit/vault/test_shares.py index ff26749f..9ddca2f6 100644 --- a/tests/unit/vault/test_shares.py +++ b/tests/unit/vault/test_shares.py @@ -433,9 +433,16 @@ def test_redeem__with_delegation_and_insufficient_allowance__reverts( @pytest.mark.parametrize("deposit_limit", [0, 10**18, MAX_INT]) -def test_set_deposit_limit__with_deposit_limit(project, gov, asset, deposit_limit): - # TODO unpermissioned set deposit limit test - vault = gov.deploy(project.VaultV3, asset, "VaultV3", "AV", gov, WEEK) +def test_set_deposit_limit__with_deposit_limit( + project, create_vault, gov, asset, deposit_limit +): + vault = create_vault( + asset=asset, + governance=gov, + max_profit_locking_time=WEEK, + vault_name="VaultV3", + vault_symbol="AV", + ) vault.set_role(gov, ROLES.DEPOSIT_LIMIT_MANAGER, sender=gov) tx = vault.set_deposit_limit(deposit_limit, sender=gov) event = list(tx.decode_logs(vault.UpdateDepositLimit)) From 4d27d310d2294f15fd0f3f0f92789dab1ff8d3d1 Mon Sep 17 00:00:00 2001 From: Schlagonia Date: Mon, 29 Jan 2024 14:51:58 -0700 Subject: [PATCH 05/16] fix: update to strategy changes --- .../test/mocks/ERC4626/LossyStrategy.sol | 14 +++++++------- .../mocks/ERC4626/MockTokenizedStrategy.sol | 19 +++---------------- 2 files changed, 10 insertions(+), 23 deletions(-) diff --git a/contracts/test/mocks/ERC4626/LossyStrategy.sol b/contracts/test/mocks/ERC4626/LossyStrategy.sol index 788a93e8..410efdfb 100644 --- a/contracts/test/mocks/ERC4626/LossyStrategy.sol +++ b/contracts/test/mocks/ERC4626/LossyStrategy.sol @@ -41,13 +41,13 @@ contract ERC4626LossyStrategy is MockTokenizedStrategy { yieldSource = address(new YieldSource(_asset)); ERC20(_asset).safeApprove(yieldSource, type(uint256).max); // So we can record losses when it happens. - strategyStorage().management = address(this); + _strategyStorage().management = address(this); vault = _vault; } // used to generate losses, accepts single arg to send losses to function setLoss(address _target, uint256 _loss) external { - strategyStorage().asset.safeTransferFrom(yieldSource, _target, _loss); + _strategyStorage().asset.safeTransferFrom(yieldSource, _target, _loss); // Record the loss MockTokenizedStrategy(address(this)).report(); } @@ -71,7 +71,7 @@ contract ERC4626LossyStrategy is MockTokenizedStrategy { if (withdrawingLoss < 0) { // Over withdraw to the vault - strategyStorage().asset.safeTransfer( + _strategyStorage().asset.safeTransfer( vault, uint256(-withdrawingLoss) ); @@ -80,16 +80,16 @@ contract ERC4626LossyStrategy is MockTokenizedStrategy { function harvestAndReport() external override returns (uint256) { return - strategyStorage().asset.balanceOf(address(this)) + - strategyStorage().asset.balanceOf(yieldSource); + _strategyStorage().asset.balanceOf(address(this)) + + _strategyStorage().asset.balanceOf(yieldSource); } function availableWithdrawLimit( address ) public view override returns (uint256) { return - strategyStorage().asset.balanceOf(address(this)) + - strategyStorage().asset.balanceOf(yieldSource) - + _strategyStorage().asset.balanceOf(address(this)) + + _strategyStorage().asset.balanceOf(yieldSource) - lockedFunds; } } diff --git a/contracts/test/mocks/ERC4626/MockTokenizedStrategy.sol b/contracts/test/mocks/ERC4626/MockTokenizedStrategy.sol index d7e385a7..752cd5e6 100644 --- a/contracts/test/mocks/ERC4626/MockTokenizedStrategy.sol +++ b/contracts/test/mocks/ERC4626/MockTokenizedStrategy.sol @@ -7,19 +7,6 @@ contract MockTokenizedStrategy is TokenizedStrategy { uint256 public minDebt; uint256 public maxDebt = type(uint256).max; - // Private variables and functions used in this mock. - bytes32 public constant BASE_STRATEGY_STORAGE = - bytes32(uint256(keccak256("yearn.base.strategy.storage")) - 1); - - function strategyStorage() internal pure returns (StrategyData storage S) { - // Since STORAGE_SLOT is a constant, we have to put a variable - // on the stack to access it from an inline assembly block. - bytes32 slot = BASE_STRATEGY_STORAGE; - assembly { - S.slot := slot - } - } - constructor( address _asset, string memory _name, @@ -27,7 +14,7 @@ contract MockTokenizedStrategy is TokenizedStrategy { address _keeper ) { // Cache storage pointer - StrategyData storage S = strategyStorage(); + StrategyData storage S = _strategyStorage(); // Set the strategy's underlying asset S.asset = ERC20(_asset); @@ -58,7 +45,7 @@ contract MockTokenizedStrategy is TokenizedStrategy { function availableDepositLimit( address ) public view virtual returns (uint256) { - uint256 _totalAssets = strategyStorage().totalAssets; + uint256 _totalAssets = _strategyStorage().totalAssets; uint256 _maxDebt = maxDebt; return _maxDebt > _totalAssets ? _maxDebt - _totalAssets : 0; } @@ -74,6 +61,6 @@ contract MockTokenizedStrategy is TokenizedStrategy { function freeFunds(uint256 _amount) external virtual {} function harvestAndReport() external virtual returns (uint256) { - return strategyStorage().asset.balanceOf(address(this)); + return _strategyStorage().asset.balanceOf(address(this)); } } From 9a90f380cc3a282f7ed5cb5e7d87d4028bcd8012 Mon Sep 17 00:00:00 2001 From: Schlag <89420541+Schlagonia@users.noreply.github.com> Date: Wed, 31 Jan 2024 16:56:20 -0700 Subject: [PATCH 06/16] feat: add to queue flag (#195) --- contracts/VaultV3.vy | 10 +++---- tests/unit/vault/test_queue_management.py | 35 ++++++++++++++++++++++- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/contracts/VaultV3.vy b/contracts/VaultV3.vy index 1118049f..7b727fec 100644 --- a/contracts/VaultV3.vy +++ b/contracts/VaultV3.vy @@ -969,7 +969,7 @@ def _redeem( ## STRATEGY MANAGEMENT ## @internal -def _add_strategy(new_strategy: address): +def _add_strategy(new_strategy: address, add_to_queue: bool): assert new_strategy not in [self, empty(address)], "strategy cannot be zero address" assert IStrategy(new_strategy).asset() == self.asset, "invalid asset" assert self.strategies[new_strategy].activation == 0, "strategy already active" @@ -982,8 +982,8 @@ def _add_strategy(new_strategy: address): max_debt: 0 }) - # If the default queue has space, add the strategy. - if len(self.default_queue) < MAX_QUEUE: + # If we are adding to the queue and the default queue has space, add the strategy. + if add_to_queue and len(self.default_queue) < MAX_QUEUE: self.default_queue.append(new_strategy) log StrategyChanged(new_strategy, StrategyChangeType.ADDED) @@ -1645,13 +1645,13 @@ def buy_debt(strategy: address, amount: uint256): ## STRATEGY MANAGEMENT ## @external -def add_strategy(new_strategy: address): +def add_strategy(new_strategy: address, add_to_queue: bool=True): """ @notice Add a new strategy. @param new_strategy The new strategy to add. """ self._enforce_role(msg.sender, Roles.ADD_STRATEGY_MANAGER) - self._add_strategy(new_strategy) + self._add_strategy(new_strategy, add_to_queue) @external def revoke_strategy(strategy: address): diff --git a/tests/unit/vault/test_queue_management.py b/tests/unit/vault/test_queue_management.py index a4ffa68d..5fda1c9c 100644 --- a/tests/unit/vault/test_queue_management.py +++ b/tests/unit/vault/test_queue_management.py @@ -230,7 +230,6 @@ def test_withdraw__queue__with_inactive_strategy__reverts( ) -# TODO: Add test to check removal and adding strategies works. def test__add_strategy__adds_to_queue(create_vault, asset, gov, create_strategy): vault = create_vault(asset) @@ -247,6 +246,23 @@ def test__add_strategy__adds_to_queue(create_vault, asset, gov, create_strategy) assert vault.get_default_queue() == [strategy_one.address, strategy_two.address] +def test__add_strategy__dont_add_to_queue(create_vault, asset, gov, create_strategy): + vault = create_vault(asset) + + assert vault.get_default_queue() == [] + + strategy_one = create_strategy(vault) + vault.add_strategy(strategy_one.address, False, sender=gov) + + assert vault.get_default_queue() == [] + assert vault.strategies(strategy_one)["activation"] != 0 + + strategy_two = create_strategy(vault) + vault.add_strategy(strategy_two.address, False, sender=gov) + + assert vault.get_default_queue() == [] + + def test__add_eleven_strategies__adds_ten_to_queue( create_vault, asset, gov, create_strategy ): @@ -295,6 +311,23 @@ def test__revoke_strategy__removes_strategy_from_queue( assert vault.get_default_queue() == [] +def test__revoke_strategy_not_in_queue(create_vault, asset, gov, create_strategy): + vault = create_vault(asset) + + assert vault.get_default_queue() == [] + + strategy_one = create_strategy(vault) + vault.add_strategy(strategy_one.address, False, sender=gov) + + assert vault.get_default_queue() == [] + assert vault.strategies(strategy_one)["activation"] != 0 + + vault.revoke_strategy(strategy_one.address, sender=gov) + + assert vault.strategies(strategy_one)["activation"] == 0 + assert vault.get_default_queue() == [] + + def test__revoke_strategy__mulitple_strategies__removes_strategy_from_queue( create_vault, asset, gov, create_strategy ): From 4c12ac1252dd6c5550cc38a82973f79b1af1072f Mon Sep 17 00:00:00 2001 From: Schlagonia Date: Thu, 1 Feb 2024 08:22:05 -0700 Subject: [PATCH 07/16] fix: updated strategy branch --- .gitignore | 2 ++ ape-config.yaml | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 4c7ec1fb..2d209c96 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,5 @@ venv/ .vscode yarn.lock env +cache/ +out/ \ No newline at end of file diff --git a/ape-config.yaml b/ape-config.yaml index c2f8bd0a..9affa89f 100644 --- a/ape-config.yaml +++ b/ape-config.yaml @@ -12,13 +12,13 @@ dependencies: ref: 4.7.3 - name: tokenized-strategy github: yearn/tokenized-strategy - ref: v3.0.2 + ref: dev_302 contracts_folder: src solidity: import_remapping: - "@openzeppelin/contracts=openzeppelin/v4.7.3" - - "@tokenized-strategy=tokenized-strategy/v3.0.2" + - "@tokenized-strategy=tokenized-strategy/dev_302" ethereum: local: From c23843da9beb01a402ba572a89c078893c42852b Mon Sep 17 00:00:00 2001 From: Schlag <89420541+Schlagonia@users.noreply.github.com> Date: Thu, 1 Feb 2024 13:03:36 -0700 Subject: [PATCH 08/16] feat: minor fixes (#194) * fix: redeem corrections * chore: dont burn zero shares * fix: use updated strategy storage * fix: rebase * chore: bump oz version * fix: oz 4626 fix * fix: lossy test * fix: round down in max redeem * fix: comment --- ape-config.yaml | 4 +- contracts/VaultV3.vy | 48 ++++++++++++---------- contracts/test/ERC4626BaseStrategy.sol | 10 +---- package.json | 2 +- tests/unit/vault/test_strategy_withdraw.py | 12 +++--- 5 files changed, 38 insertions(+), 38 deletions(-) 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 ( From c8fab704310315e558e30340bbba1bf831106f4f Mon Sep 17 00:00:00 2001 From: Schlag <89420541+Schlagonia@users.noreply.github.com> Date: Fri, 2 Feb 2024 17:02:22 -0700 Subject: [PATCH 09/16] test: add foundry tests (#196) * chore: setup foundry test * chore: add remappings * forge install: erc4626-tests * test: add foundry fuzzing tests * fix: max uint deposit limit * fix: test strategy * fix: foundry runner * fix: clamp overflow * fix: default tests * chore: clean up linting * fix: new strategy version --- .github/workflows/foundry.yaml | 65 +++++++++++ .github/workflows/lint.yaml | 2 +- .github/workflows/test.yaml | 1 - .gitignore | 3 +- .gitmodules | 6 + .solhint.json | 2 +- README.md | 13 ++- contracts/VaultV3.vy | 5 +- .../test/mocks/ERC4626/LossyStrategy.sol | 3 +- .../mocks/ERC4626/MockTokenizedStrategy.sol | 5 +- foundry.toml | 26 +++++ foundry_tests/tests/ERC4626Std.t.sol | 43 ++++++++ foundry_tests/utils/ExtendedTest.sol | 86 +++++++++++++++ foundry_tests/utils/Setup.sol | 75 +++++++++++++ foundry_tests/utils/VyperDeployer.sol | 103 ++++++++++++++++++ lib/erc4626-tests | 1 + lib/forge-std | 1 + package.json | 6 +- requirements.txt | 3 +- tests/conftest.py | 6 +- 20 files changed, 439 insertions(+), 16 deletions(-) create mode 100644 .github/workflows/foundry.yaml create mode 100644 .gitmodules create mode 100644 foundry.toml create mode 100644 foundry_tests/tests/ERC4626Std.t.sol create mode 100644 foundry_tests/utils/ExtendedTest.sol create mode 100644 foundry_tests/utils/Setup.sol create mode 100644 foundry_tests/utils/VyperDeployer.sol create mode 160000 lib/erc4626-tests create mode 160000 lib/forge-std diff --git a/.github/workflows/foundry.yaml b/.github/workflows/foundry.yaml new file mode 100644 index 00000000..607d7a75 --- /dev/null +++ b/.github/workflows/foundry.yaml @@ -0,0 +1,65 @@ +name: Foundry tests + +on: + push: + branches: + - master + pull_request: + +concurrency: + group: ${{github.workflow}}-${{github.ref}} + cancel-in-progress: true + +jobs: + unit: + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: + - ubuntu-latest + architecture: + - "x64" + python-version: + - "3.10" + node_version: + - 16 + + steps: + - name: Checkout + uses: actions/checkout@v3 + with: + submodules: recursive + + - name: Setup Python + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + architecture: ${{ matrix.architecture }} + + - name: Install Ape + uses: ApeWorX/github-action@v2.0 + with: + python-version: '3.10' + + - name: install vyper + run: pip install git+https://github.com/vyperlang/vyper + + - name: Compile contracts + # Compile Ape contracts to get dependencies + run: ape compile --force --size + + - name: Install Vyper + run: pip install vyper==0.3.7 + + - name: Use Node.js ${{ matrix.node_version }} + uses: actions/setup-node@v3 + with: + node-version: ${{ matrix.node_version }} + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + with: + version: nightly + + - name: Foundry tests + run: forge test -vvv \ No newline at end of file diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index ed6eb50a..d513b6ab 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -51,4 +51,4 @@ jobs: run: pip install -r requirements.txt - name: Run black - run: black --check --include "(tests|scripts)" . \ No newline at end of file + run: black --check . \ No newline at end of file diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 09c85dc7..6de42511 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -23,7 +23,6 @@ jobs: - uses: ApeWorX/github-action@v2.0 with: python-version: '3.10' - ape-version-pin: "==0.7.0" - name: install vyper run: pip install git+https://github.com/vyperlang/vyper diff --git a/.gitignore b/.gitignore index 2d209c96..57b504f7 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,8 @@ pyenv.cfg vyper_git_commithash.txt bin/ -lib/ +cache/ +out/ share/ build/ include/ diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 00000000..665e0dd7 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,6 @@ +[submodule "lib/forge-std"] + path = lib/forge-std + url = https://github.com/foundry-rs/forge-std +[submodule "lib/erc4626-tests"] + path = lib/erc4626-tests + url = https://github.com/a16z/erc4626-tests diff --git a/.solhint.json b/.solhint.json index c35b3e40..c1ae7de9 100644 --- a/.solhint.json +++ b/.solhint.json @@ -15,6 +15,6 @@ "not-rely-on-time": "off", "private-vars-leading-underscore": "warn", "reason-string": ["warn", { "maxLength": 64 }], - "yearn/underscore-function-args": "error" + "yearn/underscore-function-args": "off" } } diff --git a/README.md b/README.md index 3bfa0e22..ecaa90c3 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,8 @@ This repository runs on [ApeWorx](https://www.apeworx.io/). A python based devel You will need: - Python 3.8 or later - - Vyper 0.3.7 + - [Vyper 0.3.7](https://docs.vyperlang.org/en/stable/installing-vyper.html) + - [Foundry](https://book.getfoundry.sh/getting-started/installation) - Linux or macOS - Windows: Install Windows Subsystem Linux (WSL) with Python 3.8 or later - [Hardhat](https://hardhat.org/) installed globally @@ -24,7 +25,7 @@ You will need: Fork the repository and clone onto your local device ``` -git clone https://github.com/user/yearn-vaults-v3 +git clone --recursive https://github.com/user/yearn-vaults-v3 cd yearn-vaults-v3 ``` @@ -60,6 +61,14 @@ and test smart contracts with: ape test ``` +To run the Foundry tests + +NOTE: You will need to first compile with Ape before running foundry tests. +``` +forge test +``` + + ### To make a contribution please follow the [guidelines](https://github.com/yearn/yearn-vaults-v3/bloc/master/CONTRIBUTING.md) See the ApeWorx [documentation](https://docs.apeworx.io/ape/stable/) and [github](https://github.com/ApeWorX/ape) for more information. diff --git a/contracts/VaultV3.vy b/contracts/VaultV3.vy index dddf0cb5..f7f1c218 100644 --- a/contracts/VaultV3.vy +++ b/contracts/VaultV3.vy @@ -564,8 +564,11 @@ def _max_deposit(receiver: address) -> uint256: return IDepositLimitModule(deposit_limit_module).available_deposit_limit(receiver) # Else use the standard flow. - _total_assets: uint256 = self._total_assets() _deposit_limit: uint256 = self.deposit_limit + if (_deposit_limit == max_value(uint256)): + return _deposit_limit + + _total_assets: uint256 = self._total_assets() if (_total_assets >= _deposit_limit): return 0 diff --git a/contracts/test/mocks/ERC4626/LossyStrategy.sol b/contracts/test/mocks/ERC4626/LossyStrategy.sol index 410efdfb..32dd9476 100644 --- a/contracts/test/mocks/ERC4626/LossyStrategy.sol +++ b/contracts/test/mocks/ERC4626/LossyStrategy.sol @@ -32,12 +32,13 @@ contract ERC4626LossyStrategy is MockTokenizedStrategy { address public yieldSource; constructor( + address _factory, address _asset, string memory _name, address _management, address _keeper, address _vault - ) MockTokenizedStrategy(_asset, _name, _management, _keeper) { + ) MockTokenizedStrategy(_factory, _asset, _name, _management, _keeper) { yieldSource = address(new YieldSource(_asset)); ERC20(_asset).safeApprove(yieldSource, type(uint256).max); // So we can record losses when it happens. diff --git a/contracts/test/mocks/ERC4626/MockTokenizedStrategy.sol b/contracts/test/mocks/ERC4626/MockTokenizedStrategy.sol index 752cd5e6..ec78518b 100644 --- a/contracts/test/mocks/ERC4626/MockTokenizedStrategy.sol +++ b/contracts/test/mocks/ERC4626/MockTokenizedStrategy.sol @@ -8,11 +8,12 @@ contract MockTokenizedStrategy is TokenizedStrategy { uint256 public maxDebt = type(uint256).max; constructor( + address _factory, address _asset, string memory _name, address _management, address _keeper - ) { + ) TokenizedStrategy(_factory) { // Cache storage pointer StrategyData storage S = _strategyStorage(); @@ -24,7 +25,7 @@ contract MockTokenizedStrategy is TokenizedStrategy { S.decimals = ERC20(_asset).decimals(); // Set last report to this block. - S.lastReport = uint128(block.timestamp); + S.lastReport = uint96(block.timestamp); // Set the default management address. Can't be 0. require(_management != address(0), "ZERO ADDRESS"); diff --git a/foundry.toml b/foundry.toml new file mode 100644 index 00000000..2f89b3a4 --- /dev/null +++ b/foundry.toml @@ -0,0 +1,26 @@ +[profile.default] +src = 'contracts' +test = 'foundry_tests' +out = 'out' +libs = ['lib'] + +remappings = [ + 'forge-std/=lib/forge-std/src/', + 'erc4626-tests/=lib/erc4626-tests/', + "@tokenized-strategy=contracts/.cache/tokenized-strategy/dev_302", + '@openzeppelin/contracts=contracts/.cache/openzeppelin/v4.9.5/', +] +fs_permissions = [{ access = "read", path = "./"}] + +match_path = "foundry_tests/tests/*" +ffi = true + +[fuzz] +runs = 250 +max_test_rejects = 1_000_000 + +[invariant] +runs = 100 +depth = 100 + +# See more config options https://github.com/gakonst/foundry/tree/master/config \ No newline at end of file diff --git a/foundry_tests/tests/ERC4626Std.t.sol b/foundry_tests/tests/ERC4626Std.t.sol new file mode 100644 index 00000000..07190729 --- /dev/null +++ b/foundry_tests/tests/ERC4626Std.t.sol @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.18; + +import "erc4626-tests/ERC4626.test.sol"; + +import {Setup} from "../utils/Setup.sol"; + +// SEE https://github.com/a16z/erc4626-tests +contract VaultERC4626StdTest is ERC4626Test, Setup { + function setUp() public override(ERC4626Test, Setup) { + super.setUp(); + _underlying_ = address(asset); + _vault_ = address(vault); + _delta_ = 0; + _vaultMayBeEmpty = true; + _unlimitedAmount = true; + } + + // NOTE: The following tests are relaxed to consider only smaller values (of type uint120), + // since the maxWithdraw(), and maxRedeem() functions fail with large values (due to overflow). + + function test_maxWithdraw(Init memory init) public override { + init = clamp(init, type(uint120).max); + super.test_maxWithdraw(init); + } + + function test_maxRedeem(Init memory init) public override { + init = clamp(init, type(uint120).max); + super.test_maxRedeem(init); + } + + function clamp( + Init memory init, + uint max + ) internal pure returns (Init memory) { + for (uint i = 0; i < N; i++) { + init.share[i] = init.share[i] % max; + init.asset[i] = init.asset[i] % max; + } + init.yield = init.yield % int(max); + return init; + } +} diff --git a/foundry_tests/utils/ExtendedTest.sol b/foundry_tests/utils/ExtendedTest.sol new file mode 100644 index 00000000..e8fcc6ce --- /dev/null +++ b/foundry_tests/utils/ExtendedTest.sol @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: AGPL-3.0 +pragma solidity 0.8.18; + +import {Test} from "forge-std/Test.sol"; + +contract ExtendedTest is Test { + // solhint-disable-next-line + function assertNeq(address a, address b) internal { + if (a == b) { + emit log("Error: a != b not satisfied [address]"); + emit log_named_address(" Expected", b); + emit log_named_address(" Actual", a); + fail(); + } + } + + // @dev checks whether @a is within certain percentage of @b + // @a actual value + // @b expected value + // solhint-disable-next-line + function assertRelApproxEq( + uint256 a, + uint256 b, + uint256 maxPercentDelta + ) internal virtual { + uint256 delta = a > b ? a - b : b - a; + uint256 maxRelDelta = b / maxPercentDelta; + + if (delta > maxRelDelta) { + emit log("Error: a ~= b not satisfied [uint]"); + emit log_named_uint(" Expected", b); + emit log_named_uint(" Actual", a); + emit log_named_uint(" Max Delta", maxRelDelta); + emit log_named_uint(" Delta", delta); + fail(); + } + } + + // Can be removed once https://github.com/dapphub/ds-test/pull/25 is merged and we update submodules, but useful for now + // solhint-disable-next-line + function assertApproxEq( + uint256 a, + uint256 b, + uint256 margin_of_error + ) internal { + if (a > b) { + if (a - b > margin_of_error) { + emit log("Error a not equal to b"); + emit log_named_uint(" Expected", b); + emit log_named_uint(" Actual", a); + fail(); + } + } else { + if (b - a > margin_of_error) { + emit log("Error a not equal to b"); + emit log_named_uint(" Expected", b); + emit log_named_uint(" Actual", a); + fail(); + } + } + } + + // solhint-disable-next-line + function assertApproxEq( + uint256 a, + uint256 b, + uint256 margin_of_error, + string memory err + ) internal { + if (a > b) { + if (a - b > margin_of_error) { + emit log_named_string("Error", err); + emit log_named_uint(" Expected", b); + emit log_named_uint(" Actual", a); + fail(); + } + } else { + if (b - a > margin_of_error) { + emit log_named_string("Error", err); + emit log_named_uint(" Expected", b); + emit log_named_uint(" Actual", a); + fail(); + } + } + } +} diff --git a/foundry_tests/utils/Setup.sol b/foundry_tests/utils/Setup.sol new file mode 100644 index 00000000..0cbd28a9 --- /dev/null +++ b/foundry_tests/utils/Setup.sol @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: AGPL-3.0 +pragma solidity 0.8.18; + +import {ExtendedTest} from "./ExtendedTest.sol"; + +import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; +import {ERC20Mock} from "@openzeppelin/contracts/mocks/ERC20Mock.sol"; + +import {IVault} from "../../contracts/interfaces/IVault.sol"; +import {Roles} from "../../contracts/interfaces/Roles.sol"; +import {IVaultFactory} from "../../contracts/interfaces/IVaultFactory.sol"; + +import {VyperDeployer} from "./VyperDeployer.sol"; + +contract Setup is ExtendedTest { + IVault public vault; + ERC20Mock public asset; + IVaultFactory public vaultFactory; + VyperDeployer public vyperDeployer; + + address public daddy = address(69); + address public vaultManagement = address(2); + + uint256 public maxFuzzAmount = 1e30; + + function setUp() public virtual { + vyperDeployer = new VyperDeployer(); + + vaultFactory = setupFactory(); + + asset = new ERC20Mock(); + + vault = IVault(setUpVault()); + + vm.label(address(vault), "Vault"); + vm.label(address(asset), "Asset"); + vm.label(address(vaultFactory), "Vault Factory"); + vm.label(daddy, "Daddy"); + vm.label(vaultManagement, "Vault management"); + } + + function setupFactory() public returns (IVaultFactory _factory) { + address original = vyperDeployer.deployContract( + "contracts/", + "VaultV3" + ); + + bytes memory args = abi.encode("Test vault Factory", original, daddy); + + _factory = IVaultFactory( + vyperDeployer.deployContract("contracts/", "VaultFactory", args) + ); + } + + function setUpVault() public returns (IVault) { + IVault _vault = IVault( + vaultFactory.deploy_new_vault( + address(asset), + "Test vault", + "tsVault", + daddy, + 10 days + ) + ); + + vm.prank(daddy); + // Give the vault manager all the roles + _vault.set_role(vaultManagement, Roles.ALL); + + vm.prank(vaultManagement); + _vault.set_deposit_limit(type(uint256).max); + + return _vault; + } +} diff --git a/foundry_tests/utils/VyperDeployer.sol b/foundry_tests/utils/VyperDeployer.sol new file mode 100644 index 00000000..b739b6e1 --- /dev/null +++ b/foundry_tests/utils/VyperDeployer.sol @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.13; +import "forge-std/console.sol"; + +///@notice This cheat codes interface is named _CheatCodes so you can use the CheatCodes interface in other testing files without errors +interface _CheatCodes { + function ffi(string[] calldata) external returns (bytes memory); +} + +/** + * @title Vyper Contract Deployer + * @notice Forked and modified from here: + * https://github.com/pcaversaccio/snekmate/blob/main/lib/utils/VyperDeployer.sol + * @dev The Vyper deployer is a pre-built contract that takes a filename + * and deploys the corresponding Vyper contract, returning the address + * that the bytecode was deployed to. + */ + +contract VyperDeployer { + address constant HEVM_ADDRESS = + address(bytes20(uint160(uint256(keccak256("hevm cheat code"))))); + + /// @notice Initializes cheat codes in order to use ffi to compile Vyper contracts + _CheatCodes cheatCodes = _CheatCodes(HEVM_ADDRESS); + + /** + * @dev Compiles a Vyper contract and returns the address that the contract + * was deployed to. If the deployment fails, an error is thrown. + * @param path The directory path of the Vyper contract. + * For example, the path of "test" is "src/test/". + * @param fileName The file name of the Vyper contract. + * For example, the file name for "Token.vy" is "Token". + * @return deployedAddress The address that the contract was deployed to. + */ + function deployContract( + string memory path, + string memory fileName + ) public returns (address) { + ///@notice create a list of strings with the commands necessary to compile Vyper contracts + string[] memory cmds = new string[](2); + cmds[0] = "vyper"; + cmds[1] = string.concat(path, fileName, ".vy"); + + ///@notice compile the Vyper contract and return the bytecode + bytes memory bytecode = cheatCodes.ffi(cmds); + + ///@notice deploy the bytecode with the create instruction + address deployedAddress; + assembly { + deployedAddress := create(0, add(bytecode, 0x20), mload(bytecode)) + } + + ///@notice check that the deployment was successful + require( + deployedAddress != address(0), + "VyperDeployer could not deploy contract" + ); + + ///@notice return the address that the contract was deployed to + return deployedAddress; + } + + /** + * @dev Compiles a Vyper contract and returns the address that the contract + * was deployed to. If the deployment fails, an error is thrown. + * @param path The directory path of the Vyper contract. + * For example, the path of "test" is "src/test/". + * @param fileName The file name of the Vyper contract. + * For example, the file name for "Token.vy" is "Token". + * @return deployedAddress The address that the contract was deployed to. + */ + function deployContract( + string memory path, + string memory fileName, + bytes calldata args + ) public returns (address) { + ///@notice create a list of strings with the commands necessary to compile Vyper contracts + string[] memory cmds = new string[](2); + cmds[0] = "vyper"; + cmds[1] = string.concat(path, fileName, ".vy"); + + ///@notice compile the Vyper contract and return the bytecode + bytes memory _bytecode = cheatCodes.ffi(cmds); + + //add args to the deployment bytecode + bytes memory bytecode = abi.encodePacked(_bytecode, args); + + ///@notice deploy the bytecode with the create instruction + address deployedAddress; + assembly { + deployedAddress := create(0, add(bytecode, 0x20), mload(bytecode)) + } + + ///@notice check that the deployment was successful + require( + deployedAddress != address(0), + "VyperDeployer could not deploy contract" + ); + + ///@notice return the address that the contract was deployed to + return deployedAddress; + } +} diff --git a/lib/erc4626-tests b/lib/erc4626-tests new file mode 160000 index 00000000..8b1d7c2a --- /dev/null +++ b/lib/erc4626-tests @@ -0,0 +1 @@ +Subproject commit 8b1d7c2ac248c33c3506b1bff8321758943c5e11 diff --git a/lib/forge-std b/lib/forge-std new file mode 160000 index 00000000..2b58ecbc --- /dev/null +++ b/lib/forge-std @@ -0,0 +1 @@ +Subproject commit 2b58ecbcf3dfde7a75959dc7b4eb3d0670278de6 diff --git a/package.json b/package.json index 25ee730b..0445d4ba 100644 --- a/package.json +++ b/package.json @@ -12,8 +12,8 @@ "solhint-plugin-yearn": "pandadefi/solhint-plugin-yearn" }, "scripts": { - "format": "prettier --write 'contracts/**/*.(sol|json)' --verbose", - "format:check": "prettier --check 'contracts/**/*.*(sol|json)'", - "lint": "solhint 'contracts/**/*.sol'" + "format": "prettier --write 'contracts/**/*.(sol|json)' 'foundry_tests/**/*.(sol|json)'", + "format:check": "prettier --check 'contracts/**/*.*(sol|json)' 'foundry_tests/**/*.(sol|json)'", + "lint": "solhint 'contracts/**/*.sol' 'foundry_tests/**/*.sol'" } } diff --git a/requirements.txt b/requirements.txt index f37cb764..f0bc9548 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,2 +1,3 @@ black==22.3.0 -eth-ape>=0.7.0 \ No newline at end of file +eth-ape>=0.7.0 +vyper==0.3.7 \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py index 0725044a..4586d951 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -221,10 +221,11 @@ def create_vault( # create default liquid strategy with 0 fee @pytest.fixture(scope="session") -def create_strategy(project, strategist, gov): +def create_strategy(project, strategist, gov, vault_factory): def create_strategy(vault): return strategist.deploy( project.MockTokenizedStrategy, + vault_factory.address, vault.asset(), "Mock Tokenized Strategy", strategist, @@ -245,10 +246,11 @@ def create_locked_strategy(vault): # create lossy strategy with 0 fee @pytest.fixture(scope="session") -def create_lossy_strategy(project, strategist, gov): +def create_lossy_strategy(project, strategist, gov, vault_factory): def create_lossy_strategy(vault): return strategist.deploy( project.ERC4626LossyStrategy, + vault_factory.address, vault.asset(), "Mock Tokenized Strategy", strategist, From 41f96edfcc3c4b260986ba0d8c713dd47c0e6ea6 Mon Sep 17 00:00:00 2001 From: Schlag <89420541+Schlagonia@users.noreply.github.com> Date: Fri, 2 Feb 2024 17:21:28 -0700 Subject: [PATCH 10/16] fix: reporting costs (#192) * test: for no locking * chore: cheaper reports * chore: track current debt * chore: lower unlocked gas * fix: set to 0 * build: only burn or mint (#193) * build: only burn or mint * build: target end supply * chore: comments * fix: comments * fix: strategy changes * chore: rebase --- contracts/VaultV3.vy | 185 ++++++++++------------ tests/unit/vault/test_profit_unlocking.py | 8 +- 2 files changed, 95 insertions(+), 98 deletions(-) diff --git a/contracts/VaultV3.vy b/contracts/VaultV3.vy index f7f1c218..60b1cc24 100644 --- a/contracts/VaultV3.vy +++ b/contracts/VaultV3.vy @@ -221,8 +221,7 @@ balance_of: HashMap[address, uint256] # ERC20 - owner -> (spender -> amount) allowance: public(HashMap[address, HashMap[address, uint256]]) # Total amount of shares that are currently minted including those locked. -# NOTE: To get the ERC20 compliant version user totalSupply(). -total_supply: public(uint256) +total_supply: uint256 # Total amount of assets that has been deposited in strategies. total_debt: uint256 # Current assets held in the vault contract. Replacing balanceOf(this) to avoid price_per_share manipulation. @@ -419,26 +418,6 @@ def _total_supply() -> uint256: # Need to account for the shares issued to the vault that have unlocked. return self.total_supply - self._unlocked_shares() -@internal -def _burn_unlocked_shares(): - """ - Burns shares that have been unlocked since last update. - In case the full unlocking period has passed, it stops the unlocking. - """ - # Get the amount of shares that have unlocked - unlocked_shares: uint256 = self._unlocked_shares() - - # IF 0 there's nothing to do. - if unlocked_shares == 0: - return - - # Only do an SSTORE if necessary - if self.full_profit_unlock_date > block.timestamp: - self.last_profit_update = block.timestamp - - # Burn the shares unlocked. - self._burn_shares(unlocked_shares, self) - @view @internal def _total_assets() -> uint256: @@ -1190,9 +1169,6 @@ def _process_report(strategy: address) -> (uint256, uint256): # Make sure we have a valid strategy. assert self.strategies[strategy].activation != 0, "inactive strategy" - # Burn shares that have been unlocked since the last update - self._burn_unlocked_shares() - # Vault assesses profits using 4626 compliant interface. # NOTE: It is important that a strategies `convertToAssets` implementation # cannot be manipulated or else the vault could report incorrect gains/losses. @@ -1205,6 +1181,8 @@ def _process_report(strategy: address) -> (uint256, uint256): gain: uint256 = 0 loss: uint256 = 0 + ### Asses Gain or Loss ### + # Compare reported assets vs. the current debt. if total_assets > current_debt: # We have a gain. @@ -1212,53 +1190,84 @@ def _process_report(strategy: address) -> (uint256, uint256): else: # We have a loss. loss = unsafe_sub(current_debt, total_assets) + + _asset: address = self.asset + ### Asses Fees and Refunds ### # For Accountant fee assessment. total_fees: uint256 = 0 total_refunds: uint256 = 0 - # For Protocol fee assessment. - protocol_fees: uint256 = 0 - protocol_fee_recipient: address = empty(address) - - accountant: address = self.accountant # If accountant is not set, fees and refunds remain unchanged. + accountant: address = self.accountant if accountant != empty(address): total_fees, total_refunds = IAccountant(accountant).report(strategy, gain, loss) - # Protocol fees will be 0 if accountant fees are 0. - if total_fees > 0: - protocol_fee_bps: uint16 = 0 - # Get the config for this vault. - protocol_fee_bps, protocol_fee_recipient = IFactory(self.factory).protocol_fee_config() - - if(protocol_fee_bps > 0): - # Protocol fees are a percent of the fees the accountant is charging. - protocol_fees = total_fees * convert(protocol_fee_bps, uint256) / MAX_BPS + if total_refunds > 0: + # Make sure we have enough approval and enough asset to pull. + total_refunds = min(total_refunds, min(ERC20(_asset).balanceOf(accountant), ERC20(_asset).allowance(accountant, self))) + # Total fees to charge in shares. + total_fees_shares: uint256 = 0 + # For Protocol fee assessment. + protocol_fee_bps: uint16 = 0 + protocol_fees_shares: uint256 = 0 + protocol_fee_recipient: address = empty(address) # `shares_to_burn` is derived from amounts that would reduce the vaults PPS. # NOTE: this needs to be done before any pps changes shares_to_burn: uint256 = 0 - accountant_fees_shares: uint256 = 0 - protocol_fees_shares: uint256 = 0 # Only need to burn shares if there is a loss or fees. if loss + total_fees > 0: # The amount of shares we will want to burn to offset losses and fees. - shares_to_burn += self._convert_to_shares(loss + total_fees, Rounding.ROUND_UP) + shares_to_burn = self._convert_to_shares(loss + total_fees, Rounding.ROUND_UP) - # Vault calculates the amount of shares to mint as fees before changing totalAssets / totalSupply. + # If we have fees then get the proportional amount of shares to issue. if total_fees > 0: - # Accountant fees are total fees - protocol fees. - accountant_fees_shares = self._convert_to_shares(total_fees - protocol_fees, Rounding.ROUND_DOWN) - if protocol_fees > 0: - protocol_fees_shares = self._convert_to_shares(protocol_fees, Rounding.ROUND_DOWN) + # Get the total amount shares to issue for the fees. + total_fees_shares = shares_to_burn * total_fees / (loss + total_fees) + + # Get the protocol fee config for this vault. + protocol_fee_bps, protocol_fee_recipient = IFactory(self.factory).protocol_fee_config() + + # If there is a protocol fee. + if protocol_fee_bps > 0: + # Get the percent of fees to go to protocol fees. + protocol_fees_shares = total_fees_shares * convert(protocol_fee_bps, uint256) / MAX_BPS + + + # Shares to lock is any amount that would otherwise increase the vaults PPS. + shares_to_lock: uint256 = 0 + profit_max_unlock_time: uint256 = self.profit_max_unlock_time + # Get the amount we will lock to avoid a PPS increase. + if gain + total_refunds > 0 and profit_max_unlock_time != 0: + shares_to_lock = self._convert_to_shares(gain + total_refunds, Rounding.ROUND_DOWN) + + # The total current supply including locked shares. + total_supply: uint256 = self.total_supply + # The total shares the vault currently owns. Both locked and unlocked. + total_locked_shares: uint256 = self.balance_of[self] + # Get the desired end amount of shares after all accounting. + ending_supply: uint256 = total_supply + shares_to_lock - shares_to_burn - self._unlocked_shares() + + # If we will end with more shares than we have now. + if ending_supply > total_supply: + # Issue the difference. + self._issue_shares(unsafe_sub(ending_supply, total_supply), self) + + # Else we need to burn shares. + elif total_supply > ending_supply: + # Can't burn more than the vault owns. + to_burn: uint256 = min(unsafe_sub(total_supply, ending_supply), total_locked_shares) + self._burn_shares(to_burn, self) + + # Adjust the amount to lock for this period. + if shares_to_lock > shares_to_burn: + # Don't lock fees or losses. + shares_to_lock = unsafe_sub(shares_to_lock, shares_to_burn) + else: + shares_to_burn = 0 - # Shares to lock is any amounts that would otherwise increase the vaults PPS. - newly_locked_shares: uint256 = 0 + # Pull refunds if total_refunds > 0: - # Load `asset` to memory. - _asset: address = self.asset - # Make sure we have enough approval and enough asset to pull. - total_refunds = min(total_refunds, min(ERC20(_asset).balanceOf(accountant), ERC20(_asset).allowance(accountant, self))) # Transfer the refunded amount of asset to the vault. self._erc20_safe_transfer_from(_asset, accountant, self, total_refunds) # Update storage to increase total assets. @@ -1267,47 +1276,27 @@ def _process_report(strategy: address) -> (uint256, uint256): # Record any reported gains. if gain > 0: # NOTE: this will increase total_assets - self.strategies[strategy].current_debt += gain + current_debt = unsafe_add(current_debt, gain) + self.strategies[strategy].current_debt = current_debt self.total_debt += gain - profit_max_unlock_time: uint256 = self.profit_max_unlock_time - # Mint anything we are locking to the vault. - if gain + total_refunds > 0 and profit_max_unlock_time != 0: - newly_locked_shares = self._issue_shares_for_amount(gain + total_refunds, self) - - # Strategy is reporting a loss - if loss > 0: - self.strategies[strategy].current_debt -= loss + # Or record any reported loss + elif loss > 0: + current_debt = unsafe_sub(current_debt, loss) + self.strategies[strategy].current_debt = current_debt self.total_debt -= loss - # NOTE: should be precise (no new unlocked shares due to above's burn of shares) - # newly_locked_shares have already been minted / transferred to the vault, so they need to be subtracted - # no risk of underflow because they have just been minted. - previously_locked_shares: uint256 = self.balance_of[self] - newly_locked_shares - - # Now that pps has updated, we can burn the shares we intended to burn as a result of losses/fees. - # NOTE: If a value reduction (losses / fees) has occurred, prioritize burning locked profit to avoid - # negative impact on price per share. Price per share is reduced only if losses exceed locked value. - if shares_to_burn > 0: - # Cant burn more than the vault owns. - shares_to_burn = min(shares_to_burn, previously_locked_shares + newly_locked_shares) - self._burn_shares(shares_to_burn, self) - - # We burn first the newly locked shares, then the previously locked shares. - shares_not_to_lock: uint256 = min(shares_to_burn, newly_locked_shares) - # Reduce the amounts to lock by how much we burned - newly_locked_shares -= shares_not_to_lock - previously_locked_shares -= (shares_to_burn - shares_not_to_lock) - # Issue shares for fees that were calculated above if applicable. - if accountant_fees_shares > 0: - self._issue_shares(accountant_fees_shares, accountant) + if total_fees_shares > 0: + # Accountant fees are (total_fees - protocol_fees). + self._issue_shares(total_fees_shares - protocol_fees_shares, accountant) - if protocol_fees_shares > 0: - self._issue_shares(protocol_fees_shares, protocol_fee_recipient) + # If we also have protocol fees. + if protocol_fees_shares > 0: + self._issue_shares(protocol_fees_shares, protocol_fee_recipient) # Update unlocking rate and time to fully unlocked. - total_locked_shares: uint256 = previously_locked_shares + newly_locked_shares + total_locked_shares = self.balance_of[self] if total_locked_shares > 0: previously_locked_time: uint256 = 0 _full_profit_unlock_date: uint256 = self.full_profit_unlock_date @@ -1315,33 +1304,35 @@ def _process_report(strategy: address) -> (uint256, uint256): if _full_profit_unlock_date > block.timestamp: # There will only be previously locked shares if time remains. # We calculate this here since it will not occur every time we lock shares. - previously_locked_time = previously_locked_shares * (_full_profit_unlock_date - block.timestamp) + previously_locked_time = (total_locked_shares - shares_to_lock) * (_full_profit_unlock_date - block.timestamp) # new_profit_locking_period is a weighted average between the remaining time of the previously locked shares and the profit_max_unlock_time - new_profit_locking_period: uint256 = (previously_locked_time + newly_locked_shares * profit_max_unlock_time) / total_locked_shares + new_profit_locking_period: uint256 = (previously_locked_time + shares_to_lock * profit_max_unlock_time) / total_locked_shares # Calculate how many shares unlock per second. self.profit_unlocking_rate = total_locked_shares * MAX_BPS_EXTENDED / new_profit_locking_period # Calculate how long until the full amount of shares is unlocked. self.full_profit_unlock_date = block.timestamp + new_profit_locking_period # Update the last profitable report timestamp. self.last_profit_update = block.timestamp - else: - # NOTE: only setting this to 0 will turn in the desired effect, no need - # to update last_profit_update or full_profit_unlock_date - self.profit_unlocking_rate = 0 - + # NOTE: only setting this to the 0 will turn in the desired effect, + # no need to update profit_unlocking_rate + self.full_profit_unlock_date = 0 + # Record the report of profit timestamp. self.strategies[strategy].last_report = block.timestamp - # We have to recalculate the fees paid for cases with an overall loss. + # We have to recalculate the fees paid for cases with an overall loss or no profit locking + if loss + total_fees > gain + total_refunds or profit_max_unlock_time == 0: + total_fees = self._convert_to_assets(total_fees_shares, Rounding.ROUND_DOWN) + log StrategyReported( strategy, gain, loss, - self.strategies[strategy].current_debt, - self._convert_to_assets(protocol_fees_shares, Rounding.ROUND_DOWN), - self._convert_to_assets(protocol_fees_shares + accountant_fees_shares, Rounding.ROUND_DOWN), + current_debt, + total_fees * convert(protocol_fee_bps, uint256) / MAX_BPS, # Protocol Fees + total_fees, total_refunds ) diff --git a/tests/unit/vault/test_profit_unlocking.py b/tests/unit/vault/test_profit_unlocking.py index 18a05cfc..6d382a07 100644 --- a/tests/unit/vault/test_profit_unlocking.py +++ b/tests/unit/vault/test_profit_unlocking.py @@ -3002,9 +3002,15 @@ def test_set_profit_max_period_to_zero__with_fees_doesnt_lock( expected_fees_shares = first_profit * performance_fee / MAX_BPS_ACCOUNTANT first_price_per_share = vault.pricePerShare() + expected_fee_amount = ( + expected_fees_shares + * (amount + first_profit) + // (amount + expected_fees_shares) + ) + # Fees will immediately unlock as well when not locking. create_and_check_profit( - asset, strategy, gov, vault, first_profit, by_pass_fees=True + asset, strategy, gov, vault, first_profit, total_fees=expected_fee_amount ) # All profits should have been unlocked From c6cea6551502e2fd4f99028fdc1bf48117caf0a2 Mon Sep 17 00:00:00 2001 From: Schlagonia Date: Fri, 2 Feb 2024 12:20:14 -0700 Subject: [PATCH 11/16] fix: decimal type --- contracts/VaultV3.vy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/VaultV3.vy b/contracts/VaultV3.vy index 60b1cc24..7db9463e 100644 --- a/contracts/VaultV3.vy +++ b/contracts/VaultV3.vy @@ -204,7 +204,7 @@ enum Rounding: # Underlying token used by the vault. asset: public(address) # Based off the `asset` decimals. -decimals: public(uint256) +decimals: public(uint8) # Deployer contract used to retrieve the protocol fee config. factory: address @@ -302,7 +302,7 @@ def initialize( self.asset = asset decimals: uint256 = convert(ERC20Detailed(asset).decimals(), uint256) assert decimals < 256 # dev: see VVE-2020-0001 - self.decimals = decimals + self.decimals = convert(decimals, uint8) # Set the factory as the deployer address. self.factory = msg.sender @@ -1570,7 +1570,7 @@ def pricePerShare() -> uint256: exact precision should use convertToAssets or convertToShares instead. @return The price per share. """ - return self._convert_to_assets(10 ** self.decimals, Rounding.ROUND_DOWN) + return self._convert_to_assets(10 ** convert(self.decimals, uint256), Rounding.ROUND_DOWN) @view @external From 5dac60ee13c2740c32895ddc1611d3334f10f0fb Mon Sep 17 00:00:00 2001 From: Schlagonia Date: Fri, 2 Feb 2024 13:10:24 -0700 Subject: [PATCH 12/16] chore: ignore snapshot --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 57b504f7..1f52a787 100644 --- a/.gitignore +++ b/.gitignore @@ -19,4 +19,5 @@ venv/ yarn.lock env cache/ -out/ \ No newline at end of file +out/ +.gas-snapshot \ No newline at end of file From e92525130a37a4cbd6b59c74724977c65e060931 Mon Sep 17 00:00:00 2001 From: Schlagonia Date: Sat, 3 Feb 2024 11:47:01 -0700 Subject: [PATCH 13/16] chore: deployment and interfaces --- README.md | 10 +++++++++ TECH_SPEC.md | 6 ++--- contracts/VaultV3.vy | 21 +++++++++-------- contracts/interfaces/IDeployer.sol | 7 ++++-- contracts/interfaces/IVault.sol | 2 -- scripts/deploy.py | 36 +++++++++++++++--------------- 6 files changed, 48 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index ecaa90c3..4960ebce 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,16 @@ NOTE: You will need to first compile with Ape before running foundry tests. forge test ``` +## Deployment + +Deployments of the Vault Factory are done using create2 to be at a deterministic address on any EVM chain. + +Check the [docs](https://docs.yearn.fi/developers/v3/overview) for the most updated deployment address. + +Deployments on new chains can be done permissionlessly by anyone using the included script. +``` +ape run scripts/deploy.py --network YOUR_RPC_URL +``` ### To make a contribution please follow the [guidelines](https://github.com/yearn/yearn-vaults-v3/bloc/master/CONTRIBUTING.md) diff --git a/TECH_SPEC.md b/TECH_SPEC.md index cf6a8da5..8d2051d0 100644 --- a/TECH_SPEC.md +++ b/TECH_SPEC.md @@ -163,7 +163,7 @@ This responsibility is taken by callers with DEBT_MANAGER role This role can increase or decrease strategies specific debt. -The vault sends and receives funds to/from strategies. The function update_debt(strategy, target_debt, max_loss) will set the current_debt of the strategy to target_debt (if possible) +The vault sends and receives funds to/from strategies. The function update_debt(strategy, target_debt, max_loss) (max_loss defaults to 100%) will set the current_debt of the strategy to target_debt (if possible) If the strategy currently has less debt than the target_debt, the vault will send funds to it. @@ -224,15 +224,15 @@ Strategies are completely independent smart contracts that can be implemented fo In any case, to be compatible with the vault, they need to implement the following functions, which are a subset of ERC4626 vaults: - asset(): view returning underlying asset -- totalAssets(): view returning current amount of assets. It can include rewards valued in `asset` ¡ - maxDeposit(address): view returning the amount max that the strategy can take safely - deposit(assets, receiver): deposits `assets` amount of tokens into the strategy. it can be restricted to vault only or be open +- maxRedeem(owner): return the max amount of shares that `owner` can redeem. - redeem(shares, receiver, owner): redeems `shares` of the strategy for the underlying asset. - balanceOf(address): return the number of shares of the strategy that the address has - convertToAssets(shares): Converts `shares` into the corresponding amount of asset. - convertToShares(assets): Converts `assets` into the corresponding amount of shares. - previewWithdraw(assets): Converts `assets` into the corresponding amount of shares rounding up. -- maxRedeem(owner): return the max amount of shares that `owner` can redeem. + This means that the vault can deposit into any ERC4626 vault but also that a non-compliant strategy can be implemented provided that these functions have been implemented (even in a non ERC4626 compliant way). diff --git a/contracts/VaultV3.vy b/contracts/VaultV3.vy index 7db9463e..88cec4d5 100644 --- a/contracts/VaultV3.vy +++ b/contracts/VaultV3.vy @@ -5,7 +5,7 @@ @license GNU AGPLv3 @author yearn.finance @notice - The Yearn VaultV3 is designed as an non-opinionated system to distribute funds of + The Yearn VaultV3 is designed as a non-opinionated system to distribute funds of depositors for a specific `asset` into different opportunities (aka Strategies) and manage accounting in a robust way. @@ -28,26 +28,26 @@ initial deposit. The vault is built to be customized by the management to be able to fit their - specific desired needs Including the customization of strategies, accountants, + specific desired needs. Including the customization of strategies, accountants, ownership etc. """ +# INTERFACES # + from vyper.interfaces import ERC20 from vyper.interfaces import ERC20Detailed -# INTERFACES # interface IStrategy: def asset() -> address: view def balanceOf(owner: address) -> uint256: view - def maxDeposit(receiver: address) -> uint256: view - def redeem(shares: uint256, receiver: address, owner: address) -> uint256: nonpayable - def deposit(assets: uint256, receiver: address) -> uint256: nonpayable - def totalAssets() -> (uint256): view def convertToAssets(shares: uint256) -> uint256: view def convertToShares(assets: uint256) -> uint256: view def previewWithdraw(assets: uint256) -> uint256: view + def maxDeposit(receiver: address) -> uint256: view + def deposit(assets: uint256, receiver: address) -> uint256: nonpayable def maxRedeem(owner: address) -> uint256: view - + def redeem(shares: uint256, receiver: address, owner: address) -> uint256: nonpayable + interface IAccountant: def report(strategy: address, gain: uint256, loss: uint256) -> (uint256, uint256): nonpayable @@ -300,6 +300,7 @@ def initialize( assert asset != empty(address), "ZERO ADDRESS" self.asset = asset + # Get the decimals for the vault to use. decimals: uint256 = convert(ERC20Detailed(asset).decimals(), uint256) assert decimals < 256 # dev: see VVE-2020-0001 self.decimals = convert(decimals, uint8) @@ -394,7 +395,7 @@ def _burn_shares(shares: uint256, owner: address): def _unlocked_shares() -> uint256: """ Returns the amount of shares that have been unlocked. - To avoid sudden price_per_share spikes, profits must be processed + To avoid sudden price_per_share spikes, profits can be processed through an unlocking period. The mechanism involves shares to be minted to the vault which are unlocked gradually over time. Shares that have been locked are gradually unlocked over profit_max_unlock_time. @@ -1191,7 +1192,9 @@ def _process_report(strategy: address) -> (uint256, uint256): # We have a loss. loss = unsafe_sub(current_debt, total_assets) + # Cache `asset` for repeated use. _asset: address = self.asset + ### Asses Fees and Refunds ### # For Accountant fee assessment. diff --git a/contracts/interfaces/IDeployer.sol b/contracts/interfaces/IDeployer.sol index ed925479..c36316fe 100644 --- a/contracts/interfaces/IDeployer.sol +++ b/contracts/interfaces/IDeployer.sol @@ -2,7 +2,10 @@ pragma solidity 0.8.18; interface IDeployer { - event Deployed(address addr, uint256 salt); + event ContractCreation(address indexed newContract, bytes32 indexed salt); - function deploy(bytes memory code, uint256 salt) external; + function deployCreate2( + bytes32 salt, + bytes memory initCode + ) external payable returns (address newContract); } diff --git a/contracts/interfaces/IVault.sol b/contracts/interfaces/IVault.sol index 3577f05e..5e5c49aa 100644 --- a/contracts/interfaces/IVault.sol +++ b/contracts/interfaces/IVault.sol @@ -54,8 +54,6 @@ interface IVault is IERC4626 { function use_default_queue() external view returns (bool); - function total_supply() external view returns (uint256); - function minimum_total_idle() external view returns (uint256); function deposit_limit() external view returns (uint256); diff --git a/scripts/deploy.py b/scripts/deploy.py index 268a0bd3..4b371a03 100644 --- a/scripts/deploy.py +++ b/scripts/deploy.py @@ -1,14 +1,6 @@ from ape import project, accounts, Contract, chain, networks -from ape.utils import ZERO_ADDRESS -from web3 import Web3, HTTPProvider from hexbytes import HexBytes -import os import hashlib -from copy import deepcopy - -# Add the wallet to use here. -deployer = accounts.load("") - def deploy_original_and_factory(): print("Deploying Vault Factory on ChainID", chain.chain_id) @@ -16,11 +8,16 @@ def deploy_original_and_factory(): if input("Do you want to continue? ") == "n": return + deployer = input("Name of account to use? ") + deployer = accounts.load(deployer) + vault_factory = project.VaultFactory vault = project.VaultV3 + deployer_contract = project.IDeployer.at( - "0x8D85e7c9A4e369E53Acc8d5426aE1568198b0112" + "0xba5Ed099633D3B313e4D5F7bdc1305d3c28ba5Ed" ) + salt_string = "v3.0.2" # Create a SHA-256 hash object @@ -34,20 +31,21 @@ def deploy_original_and_factory(): print(f"Salt we are using {salt}") print("Init balance:", deployer.balance / 1e18) - + print("------------------") print(f"Deploying Original...") original_deploy_bytecode = vault.contract_type.deployment_bytecode.bytecode - original_tx = deployer_contract.deploy( - original_deploy_bytecode, salt, sender=deployer + original_tx = deployer_contract.deployCreate2( + salt, original_deploy_bytecode, sender=deployer ) - original_event = list(original_tx.decode_logs(deployer_contract.Deployed)) + original_event = list(original_tx.decode_logs(deployer_contract.ContractCreation)) - original_address = original_event[0].addr + original_address = original_event[0].newContract print(f"Deployed the vault original to {original_address}") + print("------------------") # deploy factory print(f"Deploying factory...") @@ -63,15 +61,17 @@ def deploy_original_and_factory(): + factory_constructor ) - factory_tx = deployer_contract.deploy( - factory_deploy_bytecode, salt, sender=deployer + factory_tx = deployer_contract.deployCreate2( + salt, factory_deploy_bytecode, sender=deployer ) - factory_event = list(factory_tx.decode_logs(deployer_contract.Deployed)) + factory_event = list(factory_tx.decode_logs(deployer_contract.ContractCreation)) - factory_address = factory_event[0].addr + factory_address = factory_event[0].newContract print(f"Deployed Vault Factory to {factory_address}") + print("------------------") + print(f"Encoded Constructor to use for verifaction {factory_constructor.hex()[2:]}") def main(): From e8f6f9c9b768c81742f910d81193ae7324454489 Mon Sep 17 00:00:00 2001 From: Schlagonia Date: Sat, 3 Feb 2024 11:48:47 -0700 Subject: [PATCH 14/16] fix: black --- scripts/deploy.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/deploy.py b/scripts/deploy.py index 4b371a03..fd3d65d9 100644 --- a/scripts/deploy.py +++ b/scripts/deploy.py @@ -2,6 +2,7 @@ from hexbytes import HexBytes import hashlib + def deploy_original_and_factory(): print("Deploying Vault Factory on ChainID", chain.chain_id) From b608306ef5df6d32910d2f535132b7c6d45401ce Mon Sep 17 00:00:00 2001 From: Schlagonia Date: Sun, 4 Feb 2024 16:01:03 -0700 Subject: [PATCH 15/16] chore: updated strategy branch --- ape-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ape-config.yaml b/ape-config.yaml index a787aade..f7230301 100644 --- a/ape-config.yaml +++ b/ape-config.yaml @@ -12,13 +12,13 @@ dependencies: ref: 4.9.5 - name: tokenized-strategy github: yearn/tokenized-strategy - ref: dev_302 + ref: master contracts_folder: src solidity: import_remapping: - "@openzeppelin/contracts=openzeppelin/v4.9.5" - - "@tokenized-strategy=tokenized-strategy/dev_302" + - "@tokenized-strategy=tokenized-strategy/master" ethereum: local: From 15a7f644b9e457d41e2df8ec6c802422a53bd8c3 Mon Sep 17 00:00:00 2001 From: Schlagonia Date: Sun, 4 Feb 2024 16:12:30 -0700 Subject: [PATCH 16/16] fix: foundry remapping --- foundry.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/foundry.toml b/foundry.toml index 2f89b3a4..99ebea0b 100644 --- a/foundry.toml +++ b/foundry.toml @@ -7,7 +7,7 @@ libs = ['lib'] remappings = [ 'forge-std/=lib/forge-std/src/', 'erc4626-tests/=lib/erc4626-tests/', - "@tokenized-strategy=contracts/.cache/tokenized-strategy/dev_302", + "@tokenized-strategy=contracts/.cache/tokenized-strategy/master", '@openzeppelin/contracts=contracts/.cache/openzeppelin/v4.9.5/', ] fs_permissions = [{ access = "read", path = "./"}]