diff --git a/contracts/Managers/RoleManager.sol b/contracts/Managers/RoleManager.sol index 23a63ee..e961bb9 100644 --- a/contracts/Managers/RoleManager.sol +++ b/contracts/Managers/RoleManager.sol @@ -149,7 +149,8 @@ contract RoleManager is Governance2Step { roles: uint96( Roles.REPORTING_MANAGER | Roles.DEBT_MANAGER | - Roles.QUEUE_MANAGER + Roles.QUEUE_MANAGER | + Roles.DEBT_PURCHASER ) }); @@ -162,7 +163,7 @@ contract RoleManager is Governance2Step { // The keeper can process reports and update debt. _positions[KEEPER] = Position({ holder: _keeper, - roles: uint96(Roles.REPORTING_MANAGER | Roles.DEBT_MANAGER) + roles: uint96(Roles.REPORTING_MANAGER) }); // Set just the roles for a debt allocator. @@ -255,9 +256,14 @@ contract RoleManager is Governance2Step { // Create the name and symbol to be standardized based on rating. string memory ratingString = ratingToString[_rating]; - // Name is "{SYMBOL} yVault-{RATING}" + // Name is "{SYMBOL}-{RATING} yVault" string memory _name = string( - abi.encodePacked(ERC20(_asset).symbol(), " yVault-", ratingString) + abi.encodePacked( + ERC20(_asset).symbol(), + "-", + ratingString, + " yVault" + ) ); // Symbol is "yv{SYMBOL}-{RATING}". string memory _symbol = string( @@ -345,46 +351,44 @@ contract RoleManager is Governance2Step { address _vault, address _debtAllocator ) internal virtual { - // Cache positionInfo to be reused for each setter. - Position memory positionInfo = _positions[DADDY]; - // Set the roles for daddy. - IVault(_vault).set_role( - positionInfo.holder, - uint256(positionInfo.roles) - ); + _setRole(_vault, _positions[DADDY]); // Set the roles for Brain. - positionInfo = _positions[BRAIN]; - IVault(_vault).set_role( - positionInfo.holder, - uint256(positionInfo.roles) - ); + _setRole(_vault, _positions[BRAIN]); // Set the roles for Security. - positionInfo = _positions[SECURITY]; - IVault(_vault).set_role( - positionInfo.holder, - uint256(positionInfo.roles) - ); + _setRole(_vault, _positions[SECURITY]); // Set the roles for the Keeper. - positionInfo = _positions[KEEPER]; - IVault(_vault).set_role( - positionInfo.holder, - uint256(positionInfo.roles) - ); + _setRole(_vault, _positions[KEEPER]); // Set the roles for the Strategy Manager. - positionInfo = _positions[STRATEGY_MANAGER]; - IVault(_vault).set_role( - positionInfo.holder, - uint256(positionInfo.roles) - ); + _setRole(_vault, _positions[STRATEGY_MANAGER]); // Give the specific debt allocator its roles. - positionInfo = _positions[DEBT_ALLOCATOR]; - IVault(_vault).set_role(_debtAllocator, uint256(positionInfo.roles)); + _setRole( + _vault, + Position(_debtAllocator, _positions[DEBT_ALLOCATOR].roles) + ); + } + + /** + * @dev Used internally to set the roles on a vault for a given position. + * Will not set the roles if the position holder is address(0). + * This does not check that the roles are !=0 because it is expected that + * the holder will be set to 0 if the position is not being used. + * + * @param _vault Address of the vault. + * @param _position Holder address and roles to set. + */ + function _setRole( + address _vault, + Position memory _position + ) internal virtual { + if (_position.holder != address(0)) { + IVault(_vault).set_role(_position.holder, uint256(_position.roles)); + } } /** @@ -537,12 +541,12 @@ contract RoleManager is Governance2Step { require(vaultConfig[_vault].asset != address(0), "vault not added"); // Remove the roles from the old allocator. - IVault(_vault).set_role(vaultConfig[_vault].debtAllocator, 0); + _setRole(_vault, Position(vaultConfig[_vault].debtAllocator, 0)); // Give the new debt allocator the relevant roles. - IVault(_vault).set_role( - _debtAllocator, - getPositionRoles(DEBT_ALLOCATOR) + _setRole( + _vault, + Position(_debtAllocator, _positions[DEBT_ALLOCATOR].roles) ); // Update the vaults config. diff --git a/contracts/debtAllocators/DebtAllocator.sol b/contracts/debtAllocators/DebtAllocator.sol index 616f58d..2c6c4bf 100644 --- a/contracts/debtAllocators/DebtAllocator.sol +++ b/contracts/debtAllocators/DebtAllocator.sol @@ -37,6 +37,9 @@ contract DebtAllocator { uint256 newTotalDebtRatio ); + /// @notice An event emitted when a strategy is added or removed. + event StrategyChanged(address indexed strategy, Status status); + /// @notice An event emitted when the minimum time to wait is updated. event UpdateMinimumWait(uint256 newMinimumWait); @@ -49,8 +52,17 @@ contract DebtAllocator { /// @notice An event emitted when the max debt update loss is updated. event UpdateMaxDebtUpdateLoss(uint256 newMaxDebtUpdateLoss); + /// @notice Status when a strategy is added or removed from the allocator. + enum Status { + NULL, + ADDED, + REMOVED + } + /// @notice Struct for each strategies info. struct Config { + // Flag to set when a strategy is added. + bool added; // The ideal percent in Basis Points the strategy should have. uint16 targetRatio; // The max percent of assets the strategy should hold. @@ -58,11 +70,11 @@ contract DebtAllocator { // Timestamp of the last time debt was updated. // The debt updates must be done through this allocator // for this to be used. - uint128 lastUpdate; - // We have an extra 96 bits in the slot. + uint96 lastUpdate; + // We have an extra 120 bits in the slot. // So we declare the variable in the struct so it can be // used if this contract is inherited. - uint96 open; + uint120 open; } /// @notice Make sure the caller is governance. @@ -200,7 +212,7 @@ contract DebtAllocator { } // Update the last time the strategies debt was updated. - _configs[_strategy].lastUpdate = uint128(block.timestamp); + _configs[_strategy].lastUpdate = uint96(block.timestamp); } /** @@ -215,6 +227,12 @@ contract DebtAllocator { function shouldUpdateDebt( address _strategy ) public view virtual returns (bool, bytes memory) { + // Get the strategy specific debt config. + Config memory config = getConfig(_strategy); + + // Make sure the strategy has been added to the allocator. + if (!config.added) return (false, bytes("!added")); + // Check the base fee isn't too high. if (!DebtAllocatorFactory(factory).isCurrentBaseFeeAcceptable()) { return (false, bytes("Base Fee")); @@ -227,9 +245,6 @@ contract DebtAllocator { // Make sure its an active strategy. require(params.activation != 0, "!active"); - // Get the strategy specific debt config. - Config memory config = getConfig(_strategy); - if (block.timestamp - config.lastUpdate <= minimumWait) { return (false, bytes("min wait")); } @@ -395,6 +410,12 @@ contract DebtAllocator { // Get the current config. Config memory config = getConfig(_strategy); + // Set added flag if not set yet. + if (!config.added) { + config.added = true; + emit StrategyChanged(_strategy, Status.ADDED); + } + // Get what will be the new total debt ratio. uint256 newTotalDebtRatio = totalDebtRatio - config.targetRatio + @@ -419,6 +440,30 @@ contract DebtAllocator { ); } + /** + * @notice Remove a strategy from this debt allocator. + * @dev Will delete the full config for the strategy + * @param _strategy Address of the address ro remove. + */ + function removeStrategy(address _strategy) external virtual onlyManagers { + Config memory config = getConfig(_strategy); + require(config.added, "!added"); + + uint256 target = config.targetRatio; + + // Remove any debt ratio the strategy holds. + if (target != 0) { + totalDebtRatio -= target; + emit UpdateStrategyDebtRatio(_strategy, 0, 0, totalDebtRatio); + } + + // Remove the full config including the `added` flag. + delete _configs[_strategy]; + + // Emit Event. + emit StrategyChanged(_strategy, Status.REMOVED); + } + /** * @notice Set the minimum change variable for a strategy. * @dev This is the minimum amount of debt to be diff --git a/tests/debtAllocators/test_debt_allocator.py b/tests/debtAllocators/test_debt_allocator.py index 6c1ae16..e074e12 100644 --- a/tests/debtAllocators/test_debt_allocator.py +++ b/tests/debtAllocators/test_debt_allocator.py @@ -20,10 +20,11 @@ def test_setup(debt_allocator_factory, brain, user, strategy, vault): assert debt_allocator.managers(brain) == False assert debt_allocator.factory() == debt_allocator_factory.address assert debt_allocator.vault() == vault.address - assert debt_allocator.getConfig(strategy) == (0, 0, 0, 0) + assert debt_allocator.getConfig(strategy) == (False, 0, 0, 0, 0) assert debt_allocator.totalDebtRatio() == 0 - with ape.reverts("!active"): - debt_allocator.shouldUpdateDebt(strategy) + (bool, bytes) = debt_allocator.shouldUpdateDebt(strategy.address) + assert bool == False + assert bytes == ("!added").encode("utf-8") def test_set_keepers(debt_allocator_factory, debt_allocator, brain, user): @@ -75,7 +76,7 @@ def test_set_managers(debt_allocator, brain, user): def test_set_minimum_change(debt_allocator, brain, strategy, user): - assert debt_allocator.getConfig(strategy) == (0, 0, 0, 0) + assert debt_allocator.getConfig(strategy) == (False, 0, 0, 0, 0) assert debt_allocator.minimumChange() == 0 minimum = int(1e17) @@ -95,7 +96,7 @@ def test_set_minimum_change(debt_allocator, brain, strategy, user): def test_set_minimum_wait(debt_allocator, brain, strategy, user): - assert debt_allocator.getConfig(strategy) == (0, 0, 0, 0) + assert debt_allocator.getConfig(strategy) == (False, 0, 0, 0, 0) assert debt_allocator.minimumWait() == 0 minimum = int(1e17) @@ -112,7 +113,7 @@ def test_set_minimum_wait(debt_allocator, brain, strategy, user): def test_set_max_debt_update_loss(debt_allocator, brain, strategy, user): - assert debt_allocator.getConfig(strategy) == (0, 0, 0, 0) + assert debt_allocator.getConfig(strategy) == (False, 0, 0, 0, 0) assert debt_allocator.maxDebtUpdateLoss() == 1 max = int(69) @@ -134,7 +135,7 @@ def test_set_max_debt_update_loss(debt_allocator, brain, strategy, user): def test_set_ratios( debt_allocator, brain, daddy, vault, strategy, create_strategy, user ): - assert debt_allocator.getConfig(strategy) == (0, 0, 0, 0) + assert debt_allocator.getConfig(strategy) == (False, 0, 0, 0, 0) minimum = int(1e17) max = int(6_000) @@ -158,13 +159,18 @@ def test_set_ratios( tx = debt_allocator.setStrategyDebtRatio(strategy, target, max, sender=brain) + event = list(tx.decode_logs(debt_allocator.StrategyChanged))[0] + + assert event.strategy == strategy + assert event.status == 1 + event = list(tx.decode_logs(debt_allocator.UpdateStrategyDebtRatio))[0] assert event.newTargetRatio == target assert event.newMaxRatio == max assert event.newTotalDebtRatio == target assert debt_allocator.totalDebtRatio() == target - assert debt_allocator.getConfig(strategy) == (target, max, 0, 0) + assert debt_allocator.getConfig(strategy) == (True, target, max, 0, 0) new_strategy = create_strategy() vault.add_strategy(new_strategy, sender=daddy) @@ -182,13 +188,13 @@ def test_set_ratios( assert event.newMaxRatio == target * 1.2 assert event.newTotalDebtRatio == target assert debt_allocator.totalDebtRatio() == target - assert debt_allocator.getConfig(strategy) == (target, target * 1.2, 0, 0) + assert debt_allocator.getConfig(strategy) == (True, target, target * 1.2, 0, 0) def test_increase_debt_ratio( debt_allocator, brain, daddy, vault, strategy, create_strategy, user ): - assert debt_allocator.getConfig(strategy) == (0, 0, 0, 0) + assert debt_allocator.getConfig(strategy) == (False, 0, 0, 0, 0) minimum = int(1e17) target = int(5_000) @@ -213,7 +219,7 @@ def test_increase_debt_ratio( assert event.newMaxRatio == max assert event.newTotalDebtRatio == target assert debt_allocator.totalDebtRatio() == target - assert debt_allocator.getConfig(strategy) == (target, max, 0, 0) + assert debt_allocator.getConfig(strategy) == (True, target, max, 0, 0) new_strategy = create_strategy() vault.add_strategy(new_strategy, sender=daddy) @@ -232,7 +238,7 @@ def test_increase_debt_ratio( assert event.newMaxRatio == max assert event.newTotalDebtRatio == target assert debt_allocator.totalDebtRatio() == target - assert debt_allocator.getConfig(strategy) == (target, max, 0, 0) + assert debt_allocator.getConfig(strategy) == (True, target, max, 0, 0) target = int(10_000) max = int(10_000) @@ -245,13 +251,13 @@ def test_increase_debt_ratio( assert event.newMaxRatio == max assert event.newTotalDebtRatio == target assert debt_allocator.totalDebtRatio() == target - assert debt_allocator.getConfig(strategy) == (target, max, 0, 0) + assert debt_allocator.getConfig(strategy) == (True, target, max, 0, 0) def test_decrease_debt_ratio( debt_allocator, brain, vault, strategy, daddy, create_strategy, user ): - assert debt_allocator.getConfig(strategy) == (0, 0, 0, 0) + assert debt_allocator.getConfig(strategy) == (False, 0, 0, 0, 0) minimum = int(1e17) target = int(5_000) @@ -273,7 +279,7 @@ def test_decrease_debt_ratio( assert event.newMaxRatio == max assert event.newTotalDebtRatio == target assert debt_allocator.totalDebtRatio() == target - assert debt_allocator.getConfig(strategy) == (target, max, 0, 0) + assert debt_allocator.getConfig(strategy) == (True, target, max, 0, 0) target = int(2_000) max = target * 1.2 @@ -290,7 +296,7 @@ def test_decrease_debt_ratio( assert event.newMaxRatio == max assert event.newTotalDebtRatio == target assert debt_allocator.totalDebtRatio() == target - assert debt_allocator.getConfig(strategy) == (target, max, 0, 0) + assert debt_allocator.getConfig(strategy) == (True, target, max, 0, 0) target = int(0) max = int(0) @@ -303,17 +309,67 @@ def test_decrease_debt_ratio( assert event.newMaxRatio == max assert event.newTotalDebtRatio == target assert debt_allocator.totalDebtRatio() == target - assert debt_allocator.getConfig(strategy) == (0, 0, 0, 0) + assert debt_allocator.getConfig(strategy) == (True, 0, 0, 0, 0) + + +def test_remove_strategy( + debt_allocator, brain, vault, strategy, daddy, user, deposit_into_vault, amount +): + assert debt_allocator.getConfig(strategy) == (False, 0, 0, 0, 0) + + minimum = int(1) + max = int(6_000) + target = int(5_000) + + vault.add_strategy(strategy.address, sender=daddy) + + debt_allocator.setMinimumChange(minimum, sender=brain) + + tx = debt_allocator.setStrategyDebtRatio(strategy, target, max, sender=brain) + + event = list(tx.decode_logs(debt_allocator.StrategyChanged))[0] + + assert event.strategy == strategy + assert event.status == 1 + + event = list(tx.decode_logs(debt_allocator.UpdateStrategyDebtRatio))[0] + + assert event.newTargetRatio == target + assert event.newMaxRatio == max + assert event.newTotalDebtRatio == target + assert debt_allocator.totalDebtRatio() == target + assert debt_allocator.getConfig(strategy) == (True, target, max, 0, 0) + + deposit_into_vault(vault, amount) + vault.update_max_debt_for_strategy(strategy, MAX_INT, sender=daddy) + + print(debt_allocator.shouldUpdateDebt(strategy)) + assert debt_allocator.shouldUpdateDebt(strategy)[0] == True + + with ape.reverts("!manager"): + debt_allocator.removeStrategy(strategy, sender=user) + + tx = debt_allocator.removeStrategy(strategy, sender=brain) + + event = list(tx.decode_logs(debt_allocator.StrategyChanged)) + + assert len(event) == 1 + assert event[0].strategy == strategy + assert event[0].status == 2 + assert debt_allocator.totalDebtRatio() == 0 + assert debt_allocator.getConfig(strategy) == (False, 0, 0, 0, 0) + assert debt_allocator.shouldUpdateDebt(strategy)[0] == False def test_should_update_debt( debt_allocator, vault, strategy, brain, daddy, deposit_into_vault, amount ): - assert debt_allocator.getConfig(strategy.address) == (0, 0, 0, 0) + assert debt_allocator.getConfig(strategy.address) == (False, 0, 0, 0, 0) vault.add_role(debt_allocator, ROLES.DEBT_MANAGER, sender=daddy) - with ape.reverts("!active"): - debt_allocator.shouldUpdateDebt(strategy.address) + (bool, bytes) = debt_allocator.shouldUpdateDebt(strategy.address) + assert bool == False + assert bytes == ("!added").encode("utf-8") vault.add_strategy(strategy.address, sender=daddy) @@ -427,7 +483,7 @@ def test_update_debt( deposit_into_vault, amount, ): - assert debt_allocator.getConfig(strategy) == (0, 0, 0, 0) + assert debt_allocator.getConfig(strategy) == (False, 0, 0, 0, 0) deposit_into_vault(vault, amount) assert vault.totalIdle() == amount @@ -452,7 +508,7 @@ def test_update_debt( debt_allocator.update_debt(strategy, amount, sender=brain) - timestamp = debt_allocator.getConfig(strategy)[2] + timestamp = debt_allocator.getConfig(strategy)[3] assert timestamp != 0 assert vault.totalIdle() == 0 assert vault.totalDebt() == amount diff --git a/tests/manager/test_role_manager.py b/tests/manager/test_role_manager.py index 785b3cd..b5ea725 100644 --- a/tests/manager/test_role_manager.py +++ b/tests/manager/test_role_manager.py @@ -5,9 +5,14 @@ from utils.helpers import to_bytes32 daddy_roles = ROLES.ALL -brain_roles = ROLES.REPORTING_MANAGER | ROLES.DEBT_MANAGER | ROLES.QUEUE_MANAGER +brain_roles = ( + ROLES.REPORTING_MANAGER + | ROLES.DEBT_MANAGER + | ROLES.QUEUE_MANAGER + | ROLES.DEBT_PURCHASER +) security_roles = ROLES.MAX_DEBT_MANAGER -keeper_roles = ROLES.REPORTING_MANAGER | ROLES.DEBT_MANAGER +keeper_roles = ROLES.REPORTING_MANAGER debt_allocator_roles = ROLES.REPORTING_MANAGER | ROLES.DEBT_MANAGER strategy_manager_roles = ROLES.ADD_STRATEGY_MANAGER | ROLES.REVOKE_STRATEGY_MANAGER @@ -366,7 +371,7 @@ def test_deploy_new_vault( symbol = asset.symbol() assert vault.symbol() == f"yv{symbol}-A" - assert vault.name() == f"{symbol} yVault-A" + assert vault.name() == f"{symbol}-A yVault" # Check debt allocator assert debt_allocator.vault() == vault @@ -543,7 +548,7 @@ def test_deploy_new_vault__default_values( symbol = asset.symbol() assert vault.symbol() == f"yv{symbol}-B" - assert vault.name() == f"{symbol} yVault-B" + assert vault.name() == f"{symbol}-B yVault" # Check debt allocator assert debt_allocator.vault() == vault