diff --git a/ape-config.yaml b/ape-config.yaml index 59a7dcc..1485aab 100644 --- a/ape-config.yaml +++ b/ape-config.yaml @@ -18,23 +18,26 @@ dependencies: - name: openzeppelin github: OpenZeppelin/openzeppelin-contracts ref: 4.8.2 + - name: yearn-vaults github: yearn/yearn-vaults-v3 ref: v3.0.1 exclude: - - test/ + - test/**/* + - name: tokenized-strategy github: yearn/tokenized-strategy ref: v3.0.1 contracts_folder: src exclude: - - test/ + - test/**/* + - name: periphery github: yearn/tokenized-strategy-periphery ref: master contracts_folder: src exclude: - - test/ + - test/**/* solidity: import_remapping: diff --git a/contracts/Managers/RoleManager.sol b/contracts/Managers/RoleManager.sol index 4ec4529..23a63ee 100644 --- a/contracts/Managers/RoleManager.sol +++ b/contracts/Managers/RoleManager.sol @@ -120,7 +120,7 @@ contract RoleManager is Governance2Step { mapping(address => VaultConfig) public vaultConfig; /// @notice Mapping of underlying asset, api version and rating to vault. mapping(address => mapping(string => mapping(uint256 => address))) - public _assetToVault; + internal _assetToVault; constructor( address _governance, @@ -153,7 +153,7 @@ contract RoleManager is Governance2Step { ) }); - // Security cna set the max debt for strategies to have. + // Security can set the max debt for strategies to have. _positions[SECURITY] = Position({ holder: _security, roles: uint96(Roles.MAX_DEBT_MANAGER) @@ -393,20 +393,23 @@ contract RoleManager is Governance2Step { * @param _vault Address of the vault to set up the accountant for. */ function _setAccountant(address _vault) internal virtual { - // Temporarily give this contract the ability to set the accountant. - IVault(_vault).add_role(address(this), Roles.ACCOUNTANT_MANAGER); - // Get the current accountant. address accountant = getPositionHolder(ACCOUNTANT); - // Set the account on the vault. - IVault(_vault).set_accountant(accountant); + // If there is an accountant set. + if (accountant != address(0)) { + // Temporarily give this contract the ability to set the accountant. + IVault(_vault).add_role(address(this), Roles.ACCOUNTANT_MANAGER); - // Take away the role. - IVault(_vault).remove_role(address(this), Roles.ACCOUNTANT_MANAGER); + // Set the account on the vault. + IVault(_vault).set_accountant(accountant); - // Whitelist the vault in the accountant. - HealthCheckAccountant(accountant).addVault(_vault); + // Take away the role. + IVault(_vault).remove_role(address(this), Roles.ACCOUNTANT_MANAGER); + + // Whitelist the vault in the accountant. + HealthCheckAccountant(accountant).addVault(_vault); + } } /** @@ -586,6 +589,29 @@ contract RoleManager is Governance2Step { emit RemovedVault(_vault); } + /** + * @notice Removes a specific role(s) for a `_holder` from the `_vaults`. + * @dev Can be used to remove one specific role or multiple. + * @param _vaults Array of vaults to adjust. + * @param _holder Address who's having a role removed. + * @param _role The role or roles to remove from the `_holder`. + */ + function removeRoles( + address[] calldata _vaults, + address _holder, + uint256 _role + ) external virtual onlyGovernance { + address _vault; + for (uint256 i = 0; i < _vaults.length; ++i) { + _vault = _vaults[i]; + // Make sure the vault is added to this Role Manager. + require(vaultConfig[_vault].asset != address(0), "vault not added"); + + // Remove the role. + IVault(_vault).remove_role(_holder, _role); + } + } + /*////////////////////////////////////////////////////////////// SETTERS //////////////////////////////////////////////////////////////*/ @@ -799,7 +825,15 @@ contract RoleManager is Governance2Step { } /** - * @notice Get the address assigned to the allocator. + * @notice Get the address assigned to be the debt allocator if any. + * @return The address assigned to be the debt allocator if any. + */ + function getDebtAllocator() external view virtual returns (address) { + return getPositionHolder(DEBT_ALLOCATOR); + } + + /** + * @notice Get the address assigned to the allocator factory. * @return The address assigned to the allocator factory. */ function getAllocatorFactory() external view virtual returns (address) { diff --git a/contracts/Mocks/MockTokenizedStrategy.sol b/contracts/Mocks/MockTokenizedStrategy.sol index 8709463..8d4dbfb 100644 --- a/contracts/Mocks/MockTokenizedStrategy.sol +++ b/contracts/Mocks/MockTokenizedStrategy.sol @@ -1,7 +1,82 @@ // SPDX-License-Identifier: AGPL-3.0 pragma solidity 0.8.18; -import {MockTokenizedStrategy} from "@yearn-vaults/test/mocks/ERC4626/MockTokenizedStrategy.sol"; +import {TokenizedStrategy, ERC20} from "@tokenized-strategy/TokenizedStrategy.sol"; + +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, + address _management, + address _keeper + ) { + // Cache storage pointer + StrategyData storage S = strategyStorage(); + + // Set the strategy's underlying asset + S.asset = ERC20(_asset); + // Set the Strategy Tokens name. + S.name = _name; + // Set decimals based off the `asset`. + S.decimals = ERC20(_asset).decimals(); + + // Set last report to this block. + S.lastReport = uint128(block.timestamp); + + // Set the default management address. Can't be 0. + require(_management != address(0), "ZERO ADDRESS"); + S.management = _management; + S.performanceFeeRecipient = _management; + // Set the keeper address + S.keeper = _keeper; + } + + function setMinDebt(uint256 _minDebt) external { + minDebt = _minDebt; + } + + function setMaxDebt(uint256 _maxDebt) external { + maxDebt = _maxDebt; + } + + function availableDepositLimit( + address + ) public view virtual returns (uint256) { + uint256 _totalAssets = strategyStorage().totalIdle; + uint256 _maxDebt = maxDebt; + return _maxDebt > _totalAssets ? _maxDebt - _totalAssets : 0; + } + + function availableWithdrawLimit( + address /*_owner*/ + ) public view virtual returns (uint256) { + return type(uint256).max; + } + + function deployFunds(uint256 _amount) external virtual {} + + function freeFunds(uint256 _amount) external virtual {} + + function harvestAndReport() external virtual returns (uint256) { + return strategyStorage().asset.balanceOf(address(this)); + } +} contract MockTokenized is MockTokenizedStrategy { uint256 public apr; diff --git a/contracts/accountants/HealthCheckAccountant.sol b/contracts/accountants/HealthCheckAccountant.sol index 4ce37f9..b6afab9 100644 --- a/contracts/accountants/HealthCheckAccountant.sol +++ b/contracts/accountants/HealthCheckAccountant.sol @@ -75,7 +75,7 @@ contract HealthCheckAccountant { _; } - modifier onlyVaultOrFeeManagers() { + modifier onlyVaultOrFeeManager() { _checkVaultOrFeeManager(); _; } @@ -151,29 +151,18 @@ contract HealthCheckAccountant { ) { require(_feeManager != address(0), "ZERO ADDRESS"); require(_feeRecipient != address(0), "ZERO ADDRESS"); - require( - defaultManagement <= MANAGEMENT_FEE_THRESHOLD, - "exceeds management fee threshold" - ); - require( - defaultPerformance <= PERFORMANCE_FEE_THRESHOLD, - "exceeds performance fee threshold" - ); - require(defaultMaxLoss <= MAX_BPS, "too high"); feeManager = _feeManager; feeRecipient = _feeRecipient; - defaultConfig = Fee({ - managementFee: defaultManagement, - performanceFee: defaultPerformance, - refundRatio: defaultRefund, - maxFee: defaultMaxFee, - maxGain: defaultMaxGain, - maxLoss: defaultMaxLoss - }); - - emit UpdateDefaultFeeConfig(defaultConfig); + _updateDefaultConfig( + defaultManagement, + defaultPerformance, + defaultRefund, + defaultMaxFee, + defaultMaxGain, + defaultMaxLoss + ); } /** @@ -275,7 +264,7 @@ contract HealthCheckAccountant { * @dev This is not used to set any of the fees for the specific vault or strategy. Each fee will be set separately. * @param vault The address of a vault to allow to use this accountant. */ - function addVault(address vault) external virtual onlyVaultOrFeeManagers { + function addVault(address vault) external virtual onlyVaultOrFeeManager { // Ensure the vault has not already been added. require(!vaults[vault], "already added"); @@ -288,9 +277,7 @@ contract HealthCheckAccountant { * @notice Function to remove a vault from this accountant's fee charging list. * @param vault The address of the vault to be removed from this accountant. */ - function removeVault( - address vault - ) external virtual onlyVaultOrFeeManagers { + function removeVault(address vault) external virtual onlyVaultOrFeeManager { // Ensure the vault has been previously added. require(vaults[vault], "not added"); @@ -323,14 +310,36 @@ contract HealthCheckAccountant { uint16 defaultMaxGain, uint16 defaultMaxLoss ) external virtual onlyFeeManager { + _updateDefaultConfig( + defaultManagement, + defaultPerformance, + defaultRefund, + defaultMaxFee, + defaultMaxGain, + defaultMaxLoss + ); + } + + /** + * @dev Updates the Accountant's default fee config. + * Is used during deployment and during any future updates. + */ + function _updateDefaultConfig( + uint16 defaultManagement, + uint16 defaultPerformance, + uint16 defaultRefund, + uint16 defaultMaxFee, + uint16 defaultMaxGain, + uint16 defaultMaxLoss + ) internal virtual { // Check for threshold and limit conditions. require( defaultManagement <= MANAGEMENT_FEE_THRESHOLD, - "exceeds management fee threshold" + "management fee threshold" ); require( defaultPerformance <= PERFORMANCE_FEE_THRESHOLD, - "exceeds performance fee threshold" + "performance fee threshold" ); require(defaultMaxLoss <= MAX_BPS, "too high"); @@ -373,11 +382,11 @@ contract HealthCheckAccountant { // Check for threshold and limit conditions. require( customManagement <= MANAGEMENT_FEE_THRESHOLD, - "exceeds management fee threshold" + "management fee threshold" ); require( customPerformance <= PERFORMANCE_FEE_THRESHOLD, - "exceeds performance fee threshold" + "performance fee threshold" ); require(customMaxLoss <= MAX_BPS, "too high"); @@ -583,8 +592,8 @@ contract HealthCheckAccountant { uint256 _amount ) internal { if (ERC20(_token).allowance(address(this), _contract) < _amount) { - ERC20(_token).approve(_contract, 0); - ERC20(_token).approve(_contract, _amount); + ERC20(_token).safeApprove(_contract, 0); + ERC20(_token).safeApprove(_contract, _amount); } } diff --git a/contracts/accountants/RefundAccountant.sol b/contracts/accountants/RefundAccountant.sol index 8eb5738..399241e 100644 --- a/contracts/accountants/RefundAccountant.sol +++ b/contracts/accountants/RefundAccountant.sol @@ -14,20 +14,11 @@ contract RefundAccountant is HealthCheckAccountant { event UpdateRefund( address indexed vault, address indexed strategy, - bool refund, uint256 amount ); - /// @notice Struct to hold refund info for a strategy. - struct Refund { - // If the accountant should refund on the report. - bool refund; - // The amount if any to refund. - uint248 amount; - } - - /// @notice Mapping of vault => strategy => struct if there is a reward refund to give. - mapping(address => mapping(address => Refund)) public refund; + /// @notice Mapping of vault => strategy => amount if there is a reward refund to give. + mapping(address => mapping(address => uint256)) public refund; constructor( address _feeManager, @@ -72,11 +63,11 @@ contract RefundAccountant is HealthCheckAccountant { { (totalFees, totalRefunds) = super.report(strategy, gain, loss); - Refund memory refundConfig = refund[msg.sender][strategy]; + uint256 _refund = refund[msg.sender][strategy]; // Check if the strategy is being given a refund. - if (refundConfig.refund) { + if (_refund != 0) { // Add it to the existing refunds. - totalRefunds += uint256(refundConfig.amount); + totalRefunds += _refund; // Make sure the vault is approved correctly. _checkAllowance( @@ -92,17 +83,15 @@ contract RefundAccountant is HealthCheckAccountant { /** * @notice Set a strategy to use to refund a reward amount for - * aut compounding reward tokens. + * auto compounding reward tokens. * * @param _vault Address of the vault to refund. * @param _strategy Address of the strategy to refund during the report. - * @param _refund Bool to turn it on or off. - * @param _amount Amount to refund per report. + * @param _amount Amount to refund on the next report. */ function setRefund( address _vault, address _strategy, - bool _refund, uint256 _amount ) external virtual onlyFeeManager { require(vaults[_vault], "not added"); @@ -110,13 +99,9 @@ contract RefundAccountant is HealthCheckAccountant { IVault(_vault).strategies(_strategy).activation != 0, "!active" ); - require(_refund || _amount == 0, "no refund and non zero amount"); - refund[_vault][_strategy] = Refund({ - refund: _refund, - amount: uint248(_amount) - }); + refund[_vault][_strategy] = _amount; - emit UpdateRefund(_vault, _strategy, _refund, uint256(_amount)); + emit UpdateRefund(_vault, _strategy, _amount); } } diff --git a/contracts/debtAllocators/DebtAllocator.sol b/contracts/debtAllocators/DebtAllocator.sol index aad03d5..616f58d 100644 --- a/contracts/debtAllocators/DebtAllocator.sol +++ b/contracts/debtAllocators/DebtAllocator.sol @@ -107,12 +107,12 @@ contract DebtAllocator { uint256 internal constant MAX_BPS = 10_000; + /// @notice Address to get permissioned roles from. + address public immutable factory; + /// @notice Address of the vault this serves as allocator for. address public vault; - /// @notice Address to get permissioned roles from. - address public factory; - /// @notice Time to wait between debt updates in seconds. uint256 public minimumWait; @@ -127,13 +127,17 @@ contract DebtAllocator { /// @notice Max loss to accept on debt updates in basis points. uint256 public maxDebtUpdateLoss; - /// @notice Mapping of addresses that are allowed to update debt. + /// @notice Mapping of addresses that are allowed to update debt ratios. mapping(address => bool) public managers; /// @notice Mapping of strategy => its config. mapping(address => Config) internal _configs; constructor(address _vault, uint256 _minimumChange) { + // Set the factory to retrieve roles from. Will be the same for all clones so can use immutable. + factory = msg.sender; + + // Initialize original version. initialize(_vault, _minimumChange); } @@ -145,8 +149,7 @@ contract DebtAllocator { */ function initialize(address _vault, uint256 _minimumChange) public virtual { require(address(vault) == address(0), "!initialized"); - // Set the factory to retrieve roles from. - factory = msg.sender; + // Set initial variables. vault = _vault; minimumChange = _minimumChange; @@ -158,10 +161,9 @@ contract DebtAllocator { /** * @notice Debt update wrapper for the vault. * @dev This can be used if a minimum time between debt updates - * is desired to be enforced and to enforce a max loss. + * is desired to be used for the trigger and to enforce a max loss. * - * This contract and the msg.sender must have the DEBT_MANAGER - * role assigned to them. + * This contract must have the DEBT_MANAGER role assigned to them. * * The function signature matches the vault so no update to the * call data is required. @@ -281,9 +283,18 @@ contract DebtAllocator { } // If current debt is greater than our max. } else if (maxDebt < params.current_debt) { + uint256 toPull = params.current_debt - targetDebt; + + uint256 currentIdle = _vault.totalIdle(); + uint256 minIdle = _vault.minimum_total_idle(); + if (minIdle > currentIdle) { + // Pull at least the amount needed for minIdle. + toPull = Math.max(toPull, minIdle - currentIdle); + } + // Find out by how much. Aim for the target. - uint256 toPull = Math.min( - params.current_debt - targetDebt, + toPull = Math.min( + toPull, // Account for the current liquidity constraints. // Use max redeem to match vault logic. IVault(_strategy).convertToAssets( @@ -293,7 +304,7 @@ contract DebtAllocator { // Check if it's over the threshold. if (toPull > minimumChange) { - // Can't lower debt if there is unrealised losses. + // Can't lower debt if there are unrealised losses. if ( _vault.assess_share_of_unrealised_losses( _strategy, @@ -410,7 +421,7 @@ contract DebtAllocator { /** * @notice Set the minimum change variable for a strategy. - * @dev This is the amount of debt that will needed to be + * @dev This is the minimum amount of debt to be * added or pulled for it to trigger an update. * * @param _minimumChange The new minimum to set for the strategy. diff --git a/contracts/debtAllocators/YieldManager/YieldManager.sol b/contracts/debtAllocators/YieldManager/YieldManager.sol index 954e9fb..2671918 100644 --- a/contracts/debtAllocators/YieldManager/YieldManager.sol +++ b/contracts/debtAllocators/YieldManager/YieldManager.sol @@ -152,7 +152,7 @@ contract YieldManager is Governance { _accountedFor -= _loss; } - // Make sure we the vault can withdraw that amount. + // Make sure the vault can withdraw that amount. require( _maxWithdraw(_vault, _strategy) >= _currentDebt - _newDebt, "max withdraw" @@ -219,7 +219,7 @@ contract YieldManager is Governance { */ function validateAllocation( address _vault, - Allocation[] memory _newAllocations + Allocation[] calldata _newAllocations ) external view virtual returns (bool) { // Get the total assets the vault has. uint256 _totalAssets = IVault(_vault).totalAssets(); @@ -253,7 +253,7 @@ contract YieldManager is Governance { */ function getCurrentAndExpectedRate( address _vault, - Allocation[] memory _newAllocations + Allocation[] calldata _newAllocations ) external view @@ -319,7 +319,7 @@ contract YieldManager is Governance { */ function updateAllocationPermissioned( address _vault, - Allocation[] memory _newAllocations + Allocation[] calldata _newAllocations ) external virtual onlyGovernance { address allocator = vaultAllocator[_vault]; require(allocator != address(0), "vault not added"); @@ -358,7 +358,7 @@ contract YieldManager is Governance { _totalAssets -= _loss; } - // Make sure we the vault can withdraw that amount. + // Make sure the vault can withdraw that amount. require( _maxWithdraw(_vault, _strategy) >= _currentDebt - _newDebt, "max withdraw" diff --git a/tests/accountants/test_refund_accountant.py b/tests/accountants/test_refund_accountant.py index 31571dd..49fbf17 100644 --- a/tests/accountants/test_refund_accountant.py +++ b/tests/accountants/test_refund_accountant.py @@ -22,70 +22,64 @@ def test_setup(daddy, vault, strategy, refund_accountant, fee_recipient): assert accountant.customConfig(vault.address, strategy.address).maxFee == 0 assert accountant.customConfig(vault.address, strategy.address).maxGain == 0 assert accountant.customConfig(vault.address, strategy.address).maxLoss == 0 - assert accountant.refund(vault.address, strategy.address) == (False, 0) + assert accountant.refund(vault.address, strategy.address) == 0 def test_add_reward_refund(daddy, vault, strategy, refund_accountant): accountant = refund_accountant - assert accountant.refund(vault.address, strategy.address) == (False, 0) + assert accountant.refund(vault.address, strategy.address) == 0 amount = int(100) with ape.reverts("not added"): - accountant.setRefund(vault, strategy, True, amount, sender=daddy) + accountant.setRefund(vault, strategy, amount, sender=daddy) accountant.addVault(vault, sender=daddy) with ape.reverts("!active"): - accountant.setRefund(vault, strategy, True, amount, sender=daddy) + accountant.setRefund(vault, strategy, amount, sender=daddy) vault.add_strategy(strategy, sender=daddy) - tx = accountant.setRefund(vault, strategy, True, amount, sender=daddy) + tx = accountant.setRefund(vault, strategy, amount, sender=daddy) event = list(tx.decode_logs(accountant.UpdateRefund)) assert len(event) == 1 assert event[0].vault == vault.address assert event[0].strategy == strategy.address - assert event[0].refund == True assert event[0].amount == amount - assert accountant.refund(vault.address, strategy.address) == (True, 100) + assert accountant.refund(vault.address, strategy.address) == 100 - with ape.reverts("no refund and non zero amount"): - accountant.setRefund(vault, strategy, False, amount, sender=daddy) - - tx = accountant.setRefund(vault, strategy, False, 0, sender=daddy) + tx = accountant.setRefund(vault, strategy, 0, sender=daddy) event = list(tx.decode_logs(accountant.UpdateRefund)) assert len(event) == 1 assert event[0].vault == vault.address assert event[0].strategy == strategy.address - assert event[0].refund == False assert event[0].amount == 0 - assert accountant.refund(vault.address, strategy.address) == (False, 0) + assert accountant.refund(vault.address, strategy.address) == 0 def test_reward_refund( daddy, vault, strategy, refund_accountant, user, asset, deposit_into_vault ): accountant = refund_accountant - assert accountant.refund(vault.address, strategy.address) == (False, 0) + assert accountant.refund(vault.address, strategy.address) == 0 amount = int(100) accountant.addVault(vault, sender=daddy) vault.add_strategy(strategy, sender=daddy) - tx = accountant.setRefund(vault, strategy, True, amount, sender=daddy) + tx = accountant.setRefund(vault, strategy, amount, sender=daddy) event = list(tx.decode_logs(accountant.UpdateRefund)) assert len(event) == 1 assert event[0].vault == vault.address assert event[0].strategy == strategy.address - assert event[0].refund == True assert event[0].amount == amount - assert accountant.refund(vault.address, strategy.address) == (True, 100) + assert accountant.refund(vault.address, strategy.address) == 100 vault.set_accountant(accountant, sender=daddy) @@ -120,7 +114,7 @@ def test_reward_refund( assert vault.fullProfitUnlockDate() > 0 # Make sure the amounts got reset. - assert accountant.refund(vault.address, strategy.address) == (False, 0) + assert accountant.refund(vault.address, strategy.address) == 0 tx = accountant.report(strategy, 0, 0, sender=vault) assert tx.return_value == (0, 0) @@ -138,7 +132,7 @@ def test_reward_refund__with_gain( accountant = refund_accountant # Set performance fee to 10% and 0 management fee accountant.updateDefaultConfig(0, 1_000, 0, 10_000, 10_000, 0, sender=daddy) - assert accountant.refund(vault.address, strategy.address) == (False, 0) + assert accountant.refund(vault.address, strategy.address) == 0 accountant.addVault(vault, sender=daddy) vault.add_strategy(strategy, sender=daddy) @@ -150,15 +144,14 @@ def test_reward_refund__with_gain( gain = to_deposit // 10 loss = 0 - tx = accountant.setRefund(vault, strategy, True, refund, sender=daddy) + tx = accountant.setRefund(vault, strategy, refund, sender=daddy) event = list(tx.decode_logs(accountant.UpdateRefund)) assert len(event) == 1 assert event[0].vault == vault.address assert event[0].strategy == strategy.address - assert event[0].refund == True assert event[0].amount == refund - assert accountant.refund(vault.address, strategy.address) == (True, refund) + assert accountant.refund(vault.address, strategy.address) == refund vault.set_accountant(accountant, sender=daddy) @@ -195,7 +188,7 @@ def test_reward_refund__with_gain( assert vault.fullProfitUnlockDate() > 0 # Make sure the amounts got reset. - assert accountant.refund(vault.address, strategy.address) == (False, 0) + assert accountant.refund(vault.address, strategy.address) == 0 tx = accountant.report(strategy, 0, 0, sender=vault) assert tx.return_value == (0, 0) @@ -215,7 +208,7 @@ def test_reward_refund__with_loss__and_refund( accountant.updateDefaultConfig( 0, 1_000, 10_000, 10_000, 10_000, 10_000, sender=daddy ) - assert accountant.refund(vault.address, strategy.address) == (False, 0) + assert accountant.refund(vault.address, strategy.address) == 0 accountant.addVault(vault, sender=daddy) vault.add_strategy(strategy, sender=daddy) @@ -227,15 +220,14 @@ def test_reward_refund__with_loss__and_refund( gain = 0 loss = to_deposit // 10 - tx = accountant.setRefund(vault, strategy, True, refund, sender=daddy) + tx = accountant.setRefund(vault, strategy, refund, sender=daddy) event = list(tx.decode_logs(accountant.UpdateRefund)) assert len(event) == 1 assert event[0].vault == vault.address assert event[0].strategy == strategy.address - assert event[0].refund == True assert event[0].amount == refund - assert accountant.refund(vault.address, strategy.address) == (True, refund) + assert accountant.refund(vault.address, strategy.address) == refund vault.set_accountant(accountant, sender=daddy) @@ -272,7 +264,7 @@ def test_reward_refund__with_loss__and_refund( assert vault.fullProfitUnlockDate() > 0 # Make sure the amounts got reset. - assert accountant.refund(vault.address, strategy.address) == (False, 0) + assert accountant.refund(vault.address, strategy.address) == 0 tx = accountant.report(strategy, 0, 0, sender=vault) assert tx.return_value == (0, 0) diff --git a/tests/manager/test_role_manager.py b/tests/manager/test_role_manager.py index 1ac246c..785b3cd 100644 --- a/tests/manager/test_role_manager.py +++ b/tests/manager/test_role_manager.py @@ -1441,3 +1441,111 @@ def test_remove_vault( vault.accept_role_manager(sender=daddy) assert vault.role_manager() == daddy + + +def test_remove_role( + role_manager, + daddy, + brain, + security, + keeper, + strategy_manager, + asset, + user, + strategy, + healthcheck_accountant, + registry, + release_registry, + vault_factory, + debt_allocator_factory, +): + setup_role_manager( + role_manager=role_manager, + release_registry=release_registry, + registry=registry, + vault_factory=vault_factory, + accountant=healthcheck_accountant, + daddy=daddy, + ) + + rating = int(2) + + assert role_manager.getAllVaults() == [] + assert ( + role_manager.getVault(asset, vault_factory.apiVersion(), rating) == ZERO_ADDRESS + ) + assert registry.numAssets() == 0 + assert registry.numEndorsedVaults(asset) == 0 + + # Deploy a vault + tx = role_manager.newVault(asset, rating, sender=daddy) + + event = list(tx.decode_logs(registry.NewEndorsedVault))[0] + vault = project.dependencies["yearn-vaults"]["v3.0.1"].VaultV3.at(event.vault) + + event = list(tx.decode_logs(debt_allocator_factory.NewDebtAllocator))[0] + debt_allocator = project.DebtAllocator.at(event.allocator) + + (vault_asset, vault_rating, vault_debt_allocator, index) = role_manager.vaultConfig( + vault + ) + + assert vault_asset == asset + assert vault_rating == rating + assert vault_debt_allocator == debt_allocator + assert index == 0 + assert role_manager.getAllVaults() == [vault] + assert role_manager.getVault(asset, vault_factory.apiVersion(), rating) == vault + assert role_manager.vaults(index) == vault + assert role_manager.isVaultsRoleManager(vault) == True + + # Check roles + assert vault.roles(role_manager) == 0 + assert vault.roles(daddy) == daddy_roles + + # Remove 1 role and see if the rest remain the same. + new_roles = daddy_roles & ~ROLES.ADD_STRATEGY_MANAGER + + with ape.reverts("vault not added"): + role_manager.removeRoles( + [strategy], daddy, ROLES.ADD_STRATEGY_MANAGER, sender=daddy + ) + + with ape.reverts("!governance"): + role_manager.removeRoles( + [vault], daddy, ROLES.ADD_STRATEGY_MANAGER, sender=user + ) + + tx = role_manager.removeRoles( + [vault], daddy, ROLES.ADD_STRATEGY_MANAGER, sender=daddy + ) + + event = list(tx.decode_logs(vault.RoleSet)) + + assert len(event) == 1 + assert event[0].account == daddy + assert event[0].role == new_roles + assert vault.roles(daddy) == new_roles + + with ape.reverts("not allowed"): + vault.add_strategy(strategy, sender=daddy) + + # Remove two roles at once + to_remove = ROLES.REVOKE_STRATEGY_MANAGER | ROLES.FORCE_REVOKE_MANAGER + + new_roles = new_roles & ~to_remove + + tx = role_manager.removeRoles([vault], daddy, to_remove, sender=daddy) + + event = list(tx.decode_logs(vault.RoleSet)) + + assert len(event) == 1 + assert event[0].account == daddy + assert event[0].role == new_roles + assert vault.roles(daddy) == new_roles + + with ape.reverts("not allowed"): + vault.revoke_strategy(strategy, sender=daddy) + + with ape.reverts("not allowed"): + vault.force_revoke_strategy(strategy, sender=daddy)