From b91ab4b0c5aef04ca471f30a9052d190c427d1ea Mon Sep 17 00:00:00 2001 From: Jem <0x0xjem@gmail.com> Date: Tue, 4 Feb 2025 16:42:46 +0400 Subject: [PATCH 1/8] First pass at function for enable/disable functionality --- src/libraries/Enableable.sol | 99 ++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 src/libraries/Enableable.sol diff --git a/src/libraries/Enableable.sol b/src/libraries/Enableable.sol new file mode 100644 index 000000000..f477d734a --- /dev/null +++ b/src/libraries/Enableable.sol @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +import {RolesConsumer} from "src/modules/ROLES/OlympusRoles.sol"; + +abstract contract Enableable is RolesConsumer { + // ===== STATE VARIABLES ===== // + + bool public isEnabled; + bytes32 public constant ROLE = "emergency_shutdown"; + + // ===== ERRORS ===== // + + error NotDisabled(); + error NotEnabled(); + + // ===== EVENTS ===== // + + event Disabled(); + event Enabled(); + + // ===== MODIFIERS ===== // + + modifier whenEnabled() { + if (!isEnabled) revert NotEnabled(); + _; + } + + modifier whenDisabled() { + if (isEnabled) revert NotDisabled(); + _; + } + + // ===== ENABLEABLE FUNCTIONS ===== // + + /// @notice Enable the contract + /// @dev This function performs the following steps: + /// 1. Validates that the caller has `ROLE` ("emergency_shutdown") + /// 2. Validates that the contract is disabled + /// 3. Calls the implementation-specific `_enable()` function + /// 4. Changes the state of the contract to enabled + /// 5. Emits the `Enabled` event + /// + /// @param enableData_ The data to pass to the implementation-specific `_enable()` function + function enable(bytes calldata enableData_) public onlyRole(ROLE) whenDisabled { + // Call the implementation-specific enable function + _enable(enableData_); + + // Change the state + isEnabled = true; + + // Emit the enabled event + emit Enabled(); + } + + /// @notice Implementation-specific enable function + /// @dev This function is called by the `enable()` function + /// + /// The implementing contract should override this function and perform the following: + /// 1. Validate any parameters (if needed) or revert + /// 2. Validate state (if needed) or revert + /// 3. Perform any necessary actions, apart from modifying the `isEnabled` state variable + /// + /// @param enableData_ Custom data that can be used by the implementation. The format of this data is + /// left to the discretion of the implementation. + function _enable(bytes calldata enableData_) internal virtual; + + /// @notice Disable the contract + /// @dev This function performs the following steps: + /// 1. Validates that the caller has `ROLE` ("emergency_shutdown") + /// 2. Validates that the contract is enabled + /// 3. Calls the implementation-specific `_disable()` function + /// 4. Changes the state of the contract to disabled + /// 5. Emits the `Disabled` event + /// + /// @param disableData_ The data to pass to the implementation-specific `_disable()` function + function disable(bytes calldata disableData_) public onlyRole(ROLE) whenEnabled { + // Call the implementation-specific disable function + _disable(disableData_); + + // Change the state + isEnabled = false; + + // Emit the disabled event + emit Disabled(); + } + + /// @notice Implementation-specific disable function + /// @dev This function is called by the `disable()` function. + /// + /// The implementing contract should override this function and perform the following: + /// 1. Validate any parameters (if needed) or revert + /// 2. Validate state (if needed) or revert + /// 3. Perform any necessary actions, apart from modifying the `isEnabled` state variable + /// + /// @param disableData_ Custom data that can be used by the implementation. The format of this data is + /// left to the discretion of the implementation. + function _disable(bytes calldata disableData_) internal virtual; +} From 61ac6f7b7e15364bad899e98c11a56bb77483e9d Mon Sep 17 00:00:00 2001 From: Jem <0x0xjem@gmail.com> Date: Tue, 4 Feb 2025 18:17:21 +0400 Subject: [PATCH 2/8] Add documentation --- src/libraries/Enableable.sol | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libraries/Enableable.sol b/src/libraries/Enableable.sol index f477d734a..97c20a392 100644 --- a/src/libraries/Enableable.sol +++ b/src/libraries/Enableable.sol @@ -3,6 +3,11 @@ pragma solidity 0.8.15; import {RolesConsumer} from "src/modules/ROLES/OlympusRoles.sol"; +/// @title Enableable +/// @notice This contract is designed to be inherited by contracts that need to be enabled or disabled. It replaces the inconsistent usage of `active` and `locallyActive` state variables across the codebase. +/// @dev A contract that inherits from this contract should use the `whenEnabled` and `whenDisabled` modifiers to gate access to certain functions. +/// +/// If custom logic and/or parameters are needed for the enable/disable functions, the inheriting contract can override the `_enable()` and `_disable()` functions. For example, `enable()` could be called with initialisation data that is decoded, validated and assigned in `_enable()`. abstract contract Enableable is RolesConsumer { // ===== STATE VARIABLES ===== // From 6d57c2108e496244c5e2ad288e9412eedeb0b147 Mon Sep 17 00:00:00 2001 From: Jem <0x0xjem@gmail.com> Date: Tue, 4 Feb 2025 18:17:38 +0400 Subject: [PATCH 3/8] Make implementation-specific functions optional --- src/libraries/Enableable.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/Enableable.sol b/src/libraries/Enableable.sol index 97c20a392..ac39dc2e3 100644 --- a/src/libraries/Enableable.sol +++ b/src/libraries/Enableable.sol @@ -61,14 +61,14 @@ abstract contract Enableable is RolesConsumer { /// @notice Implementation-specific enable function /// @dev This function is called by the `enable()` function /// - /// The implementing contract should override this function and perform the following: + /// The implementing contract can override this function and perform the following: /// 1. Validate any parameters (if needed) or revert /// 2. Validate state (if needed) or revert /// 3. Perform any necessary actions, apart from modifying the `isEnabled` state variable /// /// @param enableData_ Custom data that can be used by the implementation. The format of this data is /// left to the discretion of the implementation. - function _enable(bytes calldata enableData_) internal virtual; + function _enable(bytes calldata enableData_) internal virtual {} /// @notice Disable the contract /// @dev This function performs the following steps: @@ -93,12 +93,12 @@ abstract contract Enableable is RolesConsumer { /// @notice Implementation-specific disable function /// @dev This function is called by the `disable()` function. /// - /// The implementing contract should override this function and perform the following: + /// The implementing contract can override this function and perform the following: /// 1. Validate any parameters (if needed) or revert /// 2. Validate state (if needed) or revert /// 3. Perform any necessary actions, apart from modifying the `isEnabled` state variable /// /// @param disableData_ Custom data that can be used by the implementation. The format of this data is /// left to the discretion of the implementation. - function _disable(bytes calldata disableData_) internal virtual; + function _disable(bytes calldata disableData_) internal virtual {} } From 46f3a45dcfe642a1addf398dc8dc1111396ed593 Mon Sep 17 00:00:00 2001 From: Jem <0x0xjem@gmail.com> Date: Wed, 5 Feb 2025 12:22:48 +0400 Subject: [PATCH 4/8] Rename Enableable -> PolicyEnabler and change directories --- .../Enableable.sol => policies/utils/PolicyEnabler.sol} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename src/{libraries/Enableable.sol => policies/utils/PolicyEnabler.sol} (98%) diff --git a/src/libraries/Enableable.sol b/src/policies/utils/PolicyEnabler.sol similarity index 98% rename from src/libraries/Enableable.sol rename to src/policies/utils/PolicyEnabler.sol index ac39dc2e3..24f3b414c 100644 --- a/src/libraries/Enableable.sol +++ b/src/policies/utils/PolicyEnabler.sol @@ -3,12 +3,12 @@ pragma solidity 0.8.15; import {RolesConsumer} from "src/modules/ROLES/OlympusRoles.sol"; -/// @title Enableable +/// @title PolicyEnabler /// @notice This contract is designed to be inherited by contracts that need to be enabled or disabled. It replaces the inconsistent usage of `active` and `locallyActive` state variables across the codebase. /// @dev A contract that inherits from this contract should use the `whenEnabled` and `whenDisabled` modifiers to gate access to certain functions. /// /// If custom logic and/or parameters are needed for the enable/disable functions, the inheriting contract can override the `_enable()` and `_disable()` functions. For example, `enable()` could be called with initialisation data that is decoded, validated and assigned in `_enable()`. -abstract contract Enableable is RolesConsumer { +abstract contract PolicyEnabler is RolesConsumer { // ===== STATE VARIABLES ===== // bool public isEnabled; From e76051c6cd9f75754ed0e8c0a330c6596c0c58d7 Mon Sep 17 00:00:00 2001 From: Jem <0x0xjem@gmail.com> Date: Wed, 5 Feb 2025 12:33:55 +0400 Subject: [PATCH 5/8] Add role constants --- src/policies/utils/PolicyEnabler.sol | 7 ++++--- src/policies/utils/RoleDefinitions.sol | 9 +++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 src/policies/utils/RoleDefinitions.sol diff --git a/src/policies/utils/PolicyEnabler.sol b/src/policies/utils/PolicyEnabler.sol index 24f3b414c..dba175345 100644 --- a/src/policies/utils/PolicyEnabler.sol +++ b/src/policies/utils/PolicyEnabler.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.15; import {RolesConsumer} from "src/modules/ROLES/OlympusRoles.sol"; +import {EMERGENCY_ROLE} from "./RoleDefinitions.sol"; /// @title PolicyEnabler /// @notice This contract is designed to be inherited by contracts that need to be enabled or disabled. It replaces the inconsistent usage of `active` and `locallyActive` state variables across the codebase. @@ -11,8 +12,8 @@ import {RolesConsumer} from "src/modules/ROLES/OlympusRoles.sol"; abstract contract PolicyEnabler is RolesConsumer { // ===== STATE VARIABLES ===== // + /// @notice Whether the policy functionality is enabled bool public isEnabled; - bytes32 public constant ROLE = "emergency_shutdown"; // ===== ERRORS ===== // @@ -47,7 +48,7 @@ abstract contract PolicyEnabler is RolesConsumer { /// 5. Emits the `Enabled` event /// /// @param enableData_ The data to pass to the implementation-specific `_enable()` function - function enable(bytes calldata enableData_) public onlyRole(ROLE) whenDisabled { + function enable(bytes calldata enableData_) public onlyRole(EMERGENCY_ROLE) whenDisabled { // Call the implementation-specific enable function _enable(enableData_); @@ -79,7 +80,7 @@ abstract contract PolicyEnabler is RolesConsumer { /// 5. Emits the `Disabled` event /// /// @param disableData_ The data to pass to the implementation-specific `_disable()` function - function disable(bytes calldata disableData_) public onlyRole(ROLE) whenEnabled { + function disable(bytes calldata disableData_) public onlyRole(EMERGENCY_ROLE) whenEnabled { // Call the implementation-specific disable function _disable(disableData_); diff --git a/src/policies/utils/RoleDefinitions.sol b/src/policies/utils/RoleDefinitions.sol new file mode 100644 index 000000000..0ab071660 --- /dev/null +++ b/src/policies/utils/RoleDefinitions.sol @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +/// @dev Allows enabling/disabling the protocol/policies in an emergency +bytes32 constant EMERGENCY_ROLE = "emergency"; +/// @dev Administrative access, e.g. configuration parameters. Typically assigned to on-chain governance. +bytes32 constant ADMIN_ROLE = "admin"; +/// @dev Managerial access, e.g. managing specific protocol parameters. Typically assigned to a multisig/council. +bytes32 constant MANAGER_ROLE = "manager"; From e173fb157daffd5efebf7ff938ea914177ad5934 Mon Sep 17 00:00:00 2001 From: Jem <0x0xjem@gmail.com> Date: Wed, 5 Feb 2025 12:42:29 +0400 Subject: [PATCH 6/8] Allow emergency or admin access --- src/policies/utils/PolicyEnabler.sol | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/policies/utils/PolicyEnabler.sol b/src/policies/utils/PolicyEnabler.sol index dba175345..bbfe4af9d 100644 --- a/src/policies/utils/PolicyEnabler.sol +++ b/src/policies/utils/PolicyEnabler.sol @@ -2,13 +2,18 @@ pragma solidity 0.8.15; import {RolesConsumer} from "src/modules/ROLES/OlympusRoles.sol"; -import {EMERGENCY_ROLE} from "./RoleDefinitions.sol"; +import {ADMIN_ROLE, EMERGENCY_ROLE} from "./RoleDefinitions.sol"; /// @title PolicyEnabler /// @notice This contract is designed to be inherited by contracts that need to be enabled or disabled. It replaces the inconsistent usage of `active` and `locallyActive` state variables across the codebase. /// @dev A contract that inherits from this contract should use the `whenEnabled` and `whenDisabled` modifiers to gate access to certain functions. /// -/// If custom logic and/or parameters are needed for the enable/disable functions, the inheriting contract can override the `_enable()` and `_disable()` functions. For example, `enable()` could be called with initialisation data that is decoded, validated and assigned in `_enable()`. +/// Inheriting contracts must do the following: +/// - In `configureDependencies()`, assign the module address to the `ROLES` state variable, e.g. `ROLES = ROLESv1(getModuleAddress(toKeycode("ROLES")));` +/// +/// The following are optional: +/// - Override the `_enable()` and `_disable()` functions if custom logic and/or parameters are needed for the enable/disable functions. +/// - For example, `enable()` could be called with initialisation data that is decoded, validated and assigned in `_enable()`. abstract contract PolicyEnabler is RolesConsumer { // ===== STATE VARIABLES ===== // @@ -17,6 +22,7 @@ abstract contract PolicyEnabler is RolesConsumer { // ===== ERRORS ===== // + error NotAuthorised(); error NotDisabled(); error NotEnabled(); @@ -27,6 +33,12 @@ abstract contract PolicyEnabler is RolesConsumer { // ===== MODIFIERS ===== // + modifier onlyEmergencyOrAdminRole() { + if (!ROLES.hasRole(msg.sender, EMERGENCY_ROLE) && !ROLES.hasRole(msg.sender, ADMIN_ROLE)) + revert NotAuthorised(); + _; + } + modifier whenEnabled() { if (!isEnabled) revert NotEnabled(); _; @@ -48,7 +60,7 @@ abstract contract PolicyEnabler is RolesConsumer { /// 5. Emits the `Enabled` event /// /// @param enableData_ The data to pass to the implementation-specific `_enable()` function - function enable(bytes calldata enableData_) public onlyRole(EMERGENCY_ROLE) whenDisabled { + function enable(bytes calldata enableData_) public onlyEmergencyOrAdminRole whenDisabled { // Call the implementation-specific enable function _enable(enableData_); @@ -80,7 +92,7 @@ abstract contract PolicyEnabler is RolesConsumer { /// 5. Emits the `Disabled` event /// /// @param disableData_ The data to pass to the implementation-specific `_disable()` function - function disable(bytes calldata disableData_) public onlyRole(EMERGENCY_ROLE) whenEnabled { + function disable(bytes calldata disableData_) public onlyEmergencyOrAdminRole whenEnabled { // Call the implementation-specific disable function _disable(disableData_); From 65824155724ea1ced3b2cc45885d1aca87010873 Mon Sep 17 00:00:00 2001 From: Jem <0x0xjem@gmail.com> Date: Thu, 6 Feb 2025 12:10:27 +0400 Subject: [PATCH 7/8] Rename modifiers --- src/policies/utils/PolicyEnabler.sol | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/policies/utils/PolicyEnabler.sol b/src/policies/utils/PolicyEnabler.sol index bbfe4af9d..739cacf83 100644 --- a/src/policies/utils/PolicyEnabler.sol +++ b/src/policies/utils/PolicyEnabler.sol @@ -6,7 +6,7 @@ import {ADMIN_ROLE, EMERGENCY_ROLE} from "./RoleDefinitions.sol"; /// @title PolicyEnabler /// @notice This contract is designed to be inherited by contracts that need to be enabled or disabled. It replaces the inconsistent usage of `active` and `locallyActive` state variables across the codebase. -/// @dev A contract that inherits from this contract should use the `whenEnabled` and `whenDisabled` modifiers to gate access to certain functions. +/// @dev A contract that inherits from this contract should use the `onlyEnabled` and `onlyDisabled` modifiers to gate access to certain functions. /// /// Inheriting contracts must do the following: /// - In `configureDependencies()`, assign the module address to the `ROLES` state variable, e.g. `ROLES = ROLESv1(getModuleAddress(toKeycode("ROLES")));` @@ -33,18 +33,21 @@ abstract contract PolicyEnabler is RolesConsumer { // ===== MODIFIERS ===== // + /// @notice Modifier that reverts if the caller does not have the emergency or admin role modifier onlyEmergencyOrAdminRole() { if (!ROLES.hasRole(msg.sender, EMERGENCY_ROLE) && !ROLES.hasRole(msg.sender, ADMIN_ROLE)) revert NotAuthorised(); _; } - modifier whenEnabled() { + /// @notice Modifier that reverts if the policy is not enabled + modifier onlyEnabled() { if (!isEnabled) revert NotEnabled(); _; } - modifier whenDisabled() { + /// @notice Modifier that reverts if the policy is enabled + modifier onlyDisabled() { if (isEnabled) revert NotDisabled(); _; } @@ -60,7 +63,7 @@ abstract contract PolicyEnabler is RolesConsumer { /// 5. Emits the `Enabled` event /// /// @param enableData_ The data to pass to the implementation-specific `_enable()` function - function enable(bytes calldata enableData_) public onlyEmergencyOrAdminRole whenDisabled { + function enable(bytes calldata enableData_) public onlyEmergencyOrAdminRole onlyDisabled { // Call the implementation-specific enable function _enable(enableData_); @@ -92,7 +95,7 @@ abstract contract PolicyEnabler is RolesConsumer { /// 5. Emits the `Disabled` event /// /// @param disableData_ The data to pass to the implementation-specific `_disable()` function - function disable(bytes calldata disableData_) public onlyEmergencyOrAdminRole whenEnabled { + function disable(bytes calldata disableData_) public onlyEmergencyOrAdminRole onlyEnabled { // Call the implementation-specific disable function _disable(disableData_); From 3ec98aedb6f37c386ee2056f6fbdafdcb38f14fd Mon Sep 17 00:00:00 2001 From: Jem <0x0xjem@gmail.com> Date: Thu, 6 Feb 2025 12:25:42 +0400 Subject: [PATCH 8/8] Add tests --- .../utils/PolicyEnabler/MockPolicyEnabler.sol | 90 +++++++++++++ .../utils/PolicyEnabler/PolicyEnablerTest.sol | 118 ++++++++++++++++++ .../utils/PolicyEnabler/disable.t.sol | 110 ++++++++++++++++ .../policies/utils/PolicyEnabler/enable.t.sol | 101 +++++++++++++++ .../utils/PolicyEnabler/onlyDisabled.t.sol | 11 ++ .../utils/PolicyEnabler/onlyEnabled.t.sol | 11 ++ 6 files changed, 441 insertions(+) create mode 100644 src/test/policies/utils/PolicyEnabler/MockPolicyEnabler.sol create mode 100644 src/test/policies/utils/PolicyEnabler/PolicyEnablerTest.sol create mode 100644 src/test/policies/utils/PolicyEnabler/disable.t.sol create mode 100644 src/test/policies/utils/PolicyEnabler/enable.t.sol create mode 100644 src/test/policies/utils/PolicyEnabler/onlyDisabled.t.sol create mode 100644 src/test/policies/utils/PolicyEnabler/onlyEnabled.t.sol diff --git a/src/test/policies/utils/PolicyEnabler/MockPolicyEnabler.sol b/src/test/policies/utils/PolicyEnabler/MockPolicyEnabler.sol new file mode 100644 index 000000000..859412fbb --- /dev/null +++ b/src/test/policies/utils/PolicyEnabler/MockPolicyEnabler.sol @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: Unlicense +// solhint-disable one-contract-per-file +pragma solidity 0.8.15; + +import {console2} from "forge-std/console2.sol"; + +import {Kernel, Keycode, Policy, toKeycode} from "src/Kernel.sol"; +import {ROLESv1} from "src/modules/ROLES/ROLES.v1.sol"; +import {PolicyEnabler} from "src/policies/utils/PolicyEnabler.sol"; + +/// @notice Mock Policy that can be enabled and disabled. +/// @dev This contract does not implement any custom enable/disable logic. +contract MockPolicyEnabler is Policy, PolicyEnabler { + uint256 public enableValue; + uint256 public disableValue; + uint256 public disableAnotherValue; + + constructor(Kernel kernel_) Policy(kernel_) {} + + function configureDependencies() external override returns (Keycode[] memory dependencies) { + dependencies = new Keycode[](1); + dependencies[0] = toKeycode("ROLES"); + + ROLES = ROLESv1(getModuleAddress(dependencies[0])); + + return dependencies; + } + + function requiresEnabled() external view onlyEnabled returns (bool) { + return true; + } + + function requiresDisabled() external view onlyDisabled returns (bool) { + return true; + } +} + +/// @notice Mock Policy that can be enabled and disabled. +/// @dev This contract implements custom enable/disable logic. +contract MockPolicyEnablerWithCustomLogic is MockPolicyEnabler { + // Define a structure for the enable data + struct EnableData { + uint256 value; + } + + struct DisableData { + uint256 value; + uint256 anotherValue; + } + + bool public enableShouldRevert; + bool public disableShouldRevert; + + constructor(Kernel kernel_) MockPolicyEnabler(kernel_) {} + + function setEnableShouldRevert(bool shouldRevert_) external { + enableShouldRevert = shouldRevert_; + } + + function setDisableShouldRevert(bool shouldRevert_) external { + disableShouldRevert = shouldRevert_; + } + + function _enable(bytes calldata data_) internal override { + // Decode the enable data + EnableData memory enableData = abi.decode(data_, (EnableData)); + + // Log the enable data + console2.log("Enable data:", enableData.value); + + // solhint-disable-next-line custom-errors + if (enableShouldRevert) revert("Enable should revert"); + + enableValue = enableData.value; + } + + function _disable(bytes calldata data_) internal override { + // Decode the disable data + DisableData memory disableData = abi.decode(data_, (DisableData)); + + // Log the disable data + console2.log("Disable data:", disableData.value, disableData.anotherValue); + + // solhint-disable-next-line custom-errors + if (disableShouldRevert) revert("Disable should revert"); + + disableValue = disableData.value; + disableAnotherValue = disableData.anotherValue; + } +} diff --git a/src/test/policies/utils/PolicyEnabler/PolicyEnablerTest.sol b/src/test/policies/utils/PolicyEnabler/PolicyEnablerTest.sol new file mode 100644 index 000000000..d47365cc7 --- /dev/null +++ b/src/test/policies/utils/PolicyEnabler/PolicyEnablerTest.sol @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity 0.8.15; + +import {Test} from "forge-std/Test.sol"; +import {Kernel, Actions} from "src/Kernel.sol"; + +import {OlympusRoles} from "src/modules/ROLES/OlympusRoles.sol"; +import {RolesAdmin} from "src/policies/RolesAdmin.sol"; + +import {ADMIN_ROLE, EMERGENCY_ROLE} from "src/policies/utils/RoleDefinitions.sol"; + +import {MockPolicyEnabler, MockPolicyEnablerWithCustomLogic} from "./MockPolicyEnabler.sol"; + +contract PolicyEnablerTest is Test { + address public constant EMERGENCY = address(0xAAAA); + address public constant ADMIN = address(0xBBBB); + + Kernel public kernel; + OlympusRoles public roles; + RolesAdmin public rolesAdmin; + MockPolicyEnabler public policyEnabler; + MockPolicyEnablerWithCustomLogic public policyEnablerWithCustomLogic; + + uint256 public enableValue; + uint256 public disableValue; + uint256 public disableAnotherValue; + + bytes public enableData; + bytes public disableData; + + function setUp() public { + kernel = new Kernel(); + roles = new OlympusRoles(kernel); + rolesAdmin = new RolesAdmin(kernel); + + policyEnabler = new MockPolicyEnabler(kernel); + + // Install + kernel.executeAction(Actions.InstallModule, address(roles)); + kernel.executeAction(Actions.ActivatePolicy, address(rolesAdmin)); + kernel.executeAction(Actions.ActivatePolicy, address(policyEnabler)); + + // Grant roles + rolesAdmin.grantRole(ADMIN_ROLE, ADMIN); + rolesAdmin.grantRole(EMERGENCY_ROLE, EMERGENCY); + } + + modifier givenPolicyHasCustomLogic() { + policyEnabler = new MockPolicyEnablerWithCustomLogic(kernel); + + kernel.executeAction(Actions.ActivatePolicy, address(policyEnabler)); + + _; + } + + modifier givenPolicyEnableCustomLogicReverts() { + (MockPolicyEnablerWithCustomLogic(address(policyEnabler))).setEnableShouldRevert(true); + _; + } + + modifier givenPolicyDisableCustomLogicReverts() { + (MockPolicyEnablerWithCustomLogic(address(policyEnabler))).setDisableShouldRevert(true); + _; + } + + modifier givenEnabled() { + vm.prank(EMERGENCY); + policyEnabler.enable(enableData); + _; + } + + modifier givenDisabled() { + vm.prank(EMERGENCY); + policyEnabler.disable(disableData); + _; + } + + modifier givenEnableData(uint256 value_) { + enableValue = value_; + + enableData = abi.encode(MockPolicyEnablerWithCustomLogic.EnableData({value: value_})); + _; + } + + modifier givenDisableData(uint256 value_, uint256 anotherValue_) { + disableValue = value_; + disableAnotherValue = anotherValue_; + + disableData = abi.encode( + MockPolicyEnablerWithCustomLogic.DisableData({ + value: value_, + anotherValue: anotherValue_ + }) + ); + _; + } + + function _assertStateVariables( + bool isEnabled_, + uint256 expectedEnableValue_, + uint256 expectedDisableValue_, + uint256 expectedDisableAnotherValue_ + ) internal { + // Assert enabled + assertEq(policyEnabler.isEnabled(), isEnabled_, "isEnabled"); + + // Enable + assertEq(policyEnabler.enableValue(), expectedEnableValue_, "enableValue"); + + // Disable + assertEq(policyEnabler.disableValue(), expectedDisableValue_, "disableValue"); + assertEq( + policyEnabler.disableAnotherValue(), + expectedDisableAnotherValue_, + "disableAnotherValue" + ); + } +} diff --git a/src/test/policies/utils/PolicyEnabler/disable.t.sol b/src/test/policies/utils/PolicyEnabler/disable.t.sol new file mode 100644 index 000000000..9256c9288 --- /dev/null +++ b/src/test/policies/utils/PolicyEnabler/disable.t.sol @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity 0.8.15; + +import {PolicyEnabler} from "src/policies/utils/PolicyEnabler.sol"; +import {PolicyEnablerTest} from "./PolicyEnablerTest.sol"; + +contract PolicyEnablerDisableTest is PolicyEnablerTest { + event Disabled(); + + // given the caller does not have the emergency or admin roles + // [X] it reverts + // given the policy is disabled + // [X] it reverts + // given the caller has the admin role + // [X] it sets the enabled flag to false + // [X] it emits the Disabled event + // given the caller has the emergency role + // [X] it sets the enabled flag to false + // [X] it emits the Disabled event + // given the policy has custom disable logic + // given the custom disable logic reverts + // [ X] it reverts + // [X] it calls the implementation-specific disable function + // [X] it sets the enabled flag to false + // [X] it emits the Disabled event + + function test_callerNotEmergencyOrAdminRole_reverts(address caller_) public { + vm.assume(caller_ != EMERGENCY && caller_ != ADMIN); + + // Expect revert + vm.expectRevert(PolicyEnabler.NotAuthorised.selector); + + // Call function + vm.prank(caller_); + policyEnabler.disable(disableData); + } + + function test_policyDisabled_reverts() public { + // Expect revert + vm.expectRevert(PolicyEnabler.NotEnabled.selector); + + // Call function + vm.prank(EMERGENCY); + policyEnabler.disable(disableData); + } + + function test_callerHasAdminRole() public givenEnabled { + // Expect event + vm.expectEmit(); + emit Disabled(); + + // Call function + vm.prank(ADMIN); + policyEnabler.disable(disableData); + + // Assert state + _assertStateVariables(false, 0, 0, 0); + } + + function test_callerHasEmergencyRole() public givenEnabled { + // Expect event + vm.expectEmit(); + emit Disabled(); + + // Call function + vm.prank(EMERGENCY); + policyEnabler.disable(disableData); + + // Assert state + _assertStateVariables(false, 0, 0, 0); + } + + function test_customLogic_reverts() + public + givenPolicyHasCustomLogic + givenPolicyDisableCustomLogicReverts + givenEnableData(5) + givenEnabled + givenDisableData(1, 2) + { + // Expect revert + vm.expectRevert("Disable should revert"); + + // Call function + vm.prank(EMERGENCY); + policyEnabler.disable(disableData); + } + + function test_customLogic( + uint256 value_, + uint256 anotherValue_ + ) + public + givenPolicyHasCustomLogic + givenEnableData(5) + givenEnabled + givenDisableData(value_, anotherValue_) + { + // Expect event + vm.expectEmit(); + emit Disabled(); + + // Call function + vm.prank(EMERGENCY); + policyEnabler.disable(disableData); + + // Assert state + _assertStateVariables(false, 5, value_, anotherValue_); + } +} diff --git a/src/test/policies/utils/PolicyEnabler/enable.t.sol b/src/test/policies/utils/PolicyEnabler/enable.t.sol new file mode 100644 index 000000000..cd3f1fe8f --- /dev/null +++ b/src/test/policies/utils/PolicyEnabler/enable.t.sol @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity 0.8.15; + +import {PolicyEnabler} from "src/policies/utils/PolicyEnabler.sol"; +import {PolicyEnablerTest} from "./PolicyEnablerTest.sol"; + +contract PolicyEnablerEnableTest is PolicyEnablerTest { + event Enabled(); + + // given the caller does not have the emergency or admin roles + // [X] it reverts + // given the policy is enabled + // [X] it reverts + // given the caller has the admin role + // [X] it sets the enabled flag to true + // [X] it emits the Enabled event + // given the caller has the emergency role + // [X] it sets the enabled flag to true + // [X] it emits the Enabled event + // given the policy has custom enable logic + // given the custom enable logic reverts + // [X] it reverts + // [X] it calls the implementation-specific enable function + // [X] it sets the enabled flag to true + // [X] it emits the Enabled event + + function test_callerNotEmergencyOrAdminRole_reverts(address caller_) public { + vm.assume(caller_ != EMERGENCY && caller_ != ADMIN); + + // Expect revert + vm.expectRevert(PolicyEnabler.NotAuthorised.selector); + + // Call function + vm.prank(caller_); + policyEnabler.enable(enableData); + } + + function test_policyEnabled_reverts() public givenEnabled { + // Expect revert + vm.expectRevert(PolicyEnabler.NotDisabled.selector); + + // Call function + vm.prank(EMERGENCY); + policyEnabler.enable(enableData); + } + + function test_callerHasAdminRole() public { + // Expect event + vm.expectEmit(); + emit Enabled(); + + // Call function + vm.prank(ADMIN); + policyEnabler.enable(enableData); + + // Assert state + _assertStateVariables(true, 0, 0, 0); + } + + function test_callerHasEmergencyRole() public { + // Expect event + vm.expectEmit(); + emit Enabled(); + + // Call function + vm.prank(EMERGENCY); + policyEnabler.enable(enableData); + + // Assert state + _assertStateVariables(true, 0, 0, 0); + } + + function test_customLogic_reverts() + public + givenPolicyHasCustomLogic + givenPolicyEnableCustomLogicReverts + givenEnableData(1) + { + // Expect revert + vm.expectRevert("Enable should revert"); + + // Call function + vm.prank(EMERGENCY); + policyEnabler.enable(enableData); + } + + function test_customLogic( + uint256 value_ + ) public givenPolicyHasCustomLogic givenEnableData(value_) { + // Expect event + vm.expectEmit(); + emit Enabled(); + + // Call function + vm.prank(EMERGENCY); + policyEnabler.enable(enableData); + + // Assert state + _assertStateVariables(true, value_, 0, 0); + } +} diff --git a/src/test/policies/utils/PolicyEnabler/onlyDisabled.t.sol b/src/test/policies/utils/PolicyEnabler/onlyDisabled.t.sol new file mode 100644 index 000000000..c413c4e84 --- /dev/null +++ b/src/test/policies/utils/PolicyEnabler/onlyDisabled.t.sol @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity 0.8.15; + +import {PolicyEnabler} from "src/policies/utils/PolicyEnabler.sol"; +import {PolicyEnablerTest} from "./PolicyEnablerTest.sol"; + +contract PolicyEnablerOnlyDisabledTest is PolicyEnablerTest { + // given the policy is enabled + // [ ] it reverts + // [ ] it does not revert +} diff --git a/src/test/policies/utils/PolicyEnabler/onlyEnabled.t.sol b/src/test/policies/utils/PolicyEnabler/onlyEnabled.t.sol new file mode 100644 index 000000000..daed4d0fe --- /dev/null +++ b/src/test/policies/utils/PolicyEnabler/onlyEnabled.t.sol @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity 0.8.15; + +import {PolicyEnabler} from "src/policies/utils/PolicyEnabler.sol"; +import {PolicyEnablerTest} from "./PolicyEnablerTest.sol"; + +contract PolicyEnablerOnlyEnabledTest is PolicyEnablerTest { + // given the policy is disabled + // [ ] it reverts + // [ ] it does not revert +}