diff --git a/contracts/Managers/RoleManager.sol b/contracts/Managers/RoleManager.sol index 3b84b31..e961bb9 100644 --- a/contracts/Managers/RoleManager.sol +++ b/contracts/Managers/RoleManager.sol @@ -351,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)); + } } /** @@ -543,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 5075a5f..2c6c4bf 100644 --- a/contracts/debtAllocators/DebtAllocator.sol +++ b/contracts/debtAllocators/DebtAllocator.sol @@ -54,6 +54,7 @@ contract DebtAllocator { /// @notice Status when a strategy is added or removed from the allocator. enum Status { + NULL, ADDED, REMOVED } diff --git a/tests/debtAllocators/test_debt_allocator.py b/tests/debtAllocators/test_debt_allocator.py index 2db2e3d..e074e12 100644 --- a/tests/debtAllocators/test_debt_allocator.py +++ b/tests/debtAllocators/test_debt_allocator.py @@ -162,7 +162,7 @@ def test_set_ratios( event = list(tx.decode_logs(debt_allocator.StrategyChanged))[0] assert event.strategy == strategy - assert event.status == 0 + assert event.status == 1 event = list(tx.decode_logs(debt_allocator.UpdateStrategyDebtRatio))[0] @@ -330,7 +330,7 @@ def test_remove_strategy( event = list(tx.decode_logs(debt_allocator.StrategyChanged))[0] assert event.strategy == strategy - assert event.status == 0 + assert event.status == 1 event = list(tx.decode_logs(debt_allocator.UpdateStrategyDebtRatio))[0] @@ -355,7 +355,7 @@ def test_remove_strategy( assert len(event) == 1 assert event[0].strategy == strategy - assert event[0].status == 1 + 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