diff --git a/contracts/EmergencyProtectedTimelock.sol b/contracts/EmergencyProtectedTimelock.sol index e97a79e8..55e4b69b 100644 --- a/contracts/EmergencyProtectedTimelock.sol +++ b/contracts/EmergencyProtectedTimelock.sol @@ -21,12 +21,6 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { using ExecutableProposals for ExecutableProposals.Context; using EmergencyProtection for EmergencyProtection.Context; - // --- - // Errors - // --- - - error CallerIsNotAdminExecutor(address value); - // --- // Sanity Check Parameters & Immutables // --- @@ -150,7 +144,7 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { /// @notice Updates the address of the governance contract. /// @param newGovernance The address of the new governance contract to be set. function setGovernance(address newGovernance) external { - _checkCallerIsAdminExecutor(); + _timelockState.checkCallerIsAdminExecutor(); _timelockState.setGovernance(newGovernance); _proposals.cancelAll(); } @@ -159,7 +153,7 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { /// Ensures that the new delay value complies with the defined sanity check bounds. /// @param newAfterSubmitDelay The delay required before a submitted proposal can be scheduled. function setAfterSubmitDelay(Duration newAfterSubmitDelay) external { - _checkCallerIsAdminExecutor(); + _timelockState.checkCallerIsAdminExecutor(); _timelockState.setAfterSubmitDelay(newAfterSubmitDelay, MAX_AFTER_SUBMIT_DELAY); _timelockState.checkExecutionDelay(MIN_EXECUTION_DELAY); } @@ -168,7 +162,7 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { /// Ensures that the new delay value complies with the defined sanity check bounds. /// @param newAfterScheduleDelay The delay required before a scheduled proposal can be executed. function setAfterScheduleDelay(Duration newAfterScheduleDelay) external { - _checkCallerIsAdminExecutor(); + _timelockState.checkCallerIsAdminExecutor(); _timelockState.setAfterScheduleDelay(newAfterScheduleDelay, MAX_AFTER_SCHEDULE_DELAY); _timelockState.checkExecutionDelay(MIN_EXECUTION_DELAY); } @@ -177,7 +171,7 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { /// @param executor The address of the executor contract. /// @param owner The address of the new owner. function transferExecutorOwnership(address executor, address owner) external { - _checkCallerIsAdminExecutor(); + _timelockState.checkCallerIsAdminExecutor(); IOwnable(executor).transferOwnership(owner); } @@ -188,21 +182,21 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { /// @notice Sets the emergency activation committee address. /// @param emergencyActivationCommittee The address of the emergency activation committee. function setEmergencyProtectionActivationCommittee(address emergencyActivationCommittee) external { - _checkCallerIsAdminExecutor(); + _timelockState.checkCallerIsAdminExecutor(); _emergencyProtection.setEmergencyActivationCommittee(emergencyActivationCommittee); } /// @notice Sets the emergency execution committee address. /// @param emergencyExecutionCommittee The address of the emergency execution committee. function setEmergencyProtectionExecutionCommittee(address emergencyExecutionCommittee) external { - _checkCallerIsAdminExecutor(); + _timelockState.checkCallerIsAdminExecutor(); _emergencyProtection.setEmergencyExecutionCommittee(emergencyExecutionCommittee); } /// @notice Sets the emergency protection end date. /// @param emergencyProtectionEndDate The timestamp of the emergency protection end date. function setEmergencyProtectionEndDate(Timestamp emergencyProtectionEndDate) external { - _checkCallerIsAdminExecutor(); + _timelockState.checkCallerIsAdminExecutor(); _emergencyProtection.setEmergencyProtectionEndDate( emergencyProtectionEndDate, MAX_EMERGENCY_PROTECTION_DURATION ); @@ -211,14 +205,14 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { /// @notice Sets the emergency mode duration. /// @param emergencyModeDuration The duration of the emergency mode. function setEmergencyModeDuration(Duration emergencyModeDuration) external { - _checkCallerIsAdminExecutor(); + _timelockState.checkCallerIsAdminExecutor(); _emergencyProtection.setEmergencyModeDuration(emergencyModeDuration, MAX_EMERGENCY_MODE_DURATION); } /// @notice Sets the emergency governance address. /// @param emergencyGovernance The address of the emergency governance. function setEmergencyGovernance(address emergencyGovernance) external { - _checkCallerIsAdminExecutor(); + _timelockState.checkCallerIsAdminExecutor(); _emergencyProtection.setEmergencyGovernance(emergencyGovernance); } @@ -241,7 +235,7 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { function deactivateEmergencyMode() external { _emergencyProtection.checkEmergencyMode({isActive: true}); if (!_emergencyProtection.isEmergencyModeDurationPassed()) { - _checkCallerIsAdminExecutor(); + _timelockState.checkCallerIsAdminExecutor(); } _emergencyProtection.deactivateEmergencyMode(); _proposals.cancelAll(); @@ -388,17 +382,7 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { /// @notice Sets the address of the admin executor. /// @param newAdminExecutor The address of the new admin executor. function setAdminExecutor(address newAdminExecutor) external { - _checkCallerIsAdminExecutor(); + _timelockState.checkCallerIsAdminExecutor(); _timelockState.setAdminExecutor(newAdminExecutor); } - - // --- - // Internal Methods - // --- - - function _checkCallerIsAdminExecutor() internal view { - if (msg.sender != _timelockState.adminExecutor) { - revert CallerIsNotAdminExecutor(msg.sender); - } - } } diff --git a/contracts/libraries/TimelockState.sol b/contracts/libraries/TimelockState.sol index 19258615..098edc69 100644 --- a/contracts/libraries/TimelockState.sol +++ b/contracts/libraries/TimelockState.sol @@ -12,6 +12,7 @@ library TimelockState { error CallerIsNotGovernance(address caller); error InvalidGovernance(address governance); + error CallerIsNotAdminExecutor(address caller); error InvalidAdminExecutor(address adminExecutor); error InvalidExecutionDelay(Duration executionDelay); error InvalidAfterSubmitDelay(Duration afterSubmitDelay); @@ -42,7 +43,7 @@ library TimelockState { } // --- - // Main Functionality + // State Management // --- /// @notice Sets the governance address. @@ -102,6 +103,10 @@ library TimelockState { emit AdminExecutorSet(newAdminExecutor); } + // --- + // Getters + // --- + /// @notice Retrieves the delay period required after a proposal is submitted before it can be scheduled. /// @param self The context of the Timelock State library. /// @return Duration The current after submit delay. @@ -116,6 +121,10 @@ library TimelockState { return self.afterScheduleDelay; } + // --- + // Checks + // --- + /// @notice Checks if the caller is the governance address, reverting if not. /// @param self The context of the Timelock State library. function checkCallerIsGovernance(Context storage self) internal view { @@ -133,4 +142,14 @@ library TimelockState { revert InvalidExecutionDelay(executionDelay); } } + + /// @notice Ensures that the caller is the designated admin executor. + /// @param self The context of the calling contract. + function checkCallerIsAdminExecutor(Context storage self) internal view { + if (self.adminExecutor != msg.sender) { + revert CallerIsNotAdminExecutor(msg.sender); + } + } } + + diff --git a/test/unit/EmergencyProtectedTimelock.t.sol b/test/unit/EmergencyProtectedTimelock.t.sol index fd41500f..3f7823ef 100644 --- a/test/unit/EmergencyProtectedTimelock.t.sol +++ b/test/unit/EmergencyProtectedTimelock.t.sol @@ -359,7 +359,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { Duration newAfterSubmitDelay = _defaultSanityCheckParams.maxAfterSubmitDelay + Durations.from(1 seconds); vm.expectRevert( - abi.encodeWithSelector(EmergencyProtectedTimelock.CallerIsNotAdminExecutor.selector, address(this)) + abi.encodeWithSelector(TimelockState.CallerIsNotAdminExecutor.selector, address(this)) ); _timelock.setAfterSubmitDelay(newAfterSubmitDelay); } @@ -431,7 +431,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { Duration newAfterScheduleDelay = _defaultSanityCheckParams.maxAfterScheduleDelay + Durations.from(1 seconds); vm.expectRevert( - abi.encodeWithSelector(EmergencyProtectedTimelock.CallerIsNotAdminExecutor.selector, address(this)) + abi.encodeWithSelector(TimelockState.CallerIsNotAdminExecutor.selector, address(this)) ); _timelock.setAfterScheduleDelay(newAfterScheduleDelay); } @@ -487,7 +487,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { vm.assume(stranger != _adminExecutor); vm.prank(stranger); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtectedTimelock.CallerIsNotAdminExecutor.selector, stranger)); + vm.expectRevert(abi.encodeWithSelector(TimelockState.CallerIsNotAdminExecutor.selector, stranger)); _timelock.transferExecutorOwnership(_adminExecutor, makeAddr("newOwner")); } @@ -533,7 +533,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { vm.assume(stranger != _adminExecutor); vm.prank(stranger); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtectedTimelock.CallerIsNotAdminExecutor.selector, stranger)); + vm.expectRevert(abi.encodeWithSelector(TimelockState.CallerIsNotAdminExecutor.selector, stranger)); _timelock.setGovernance(makeAddr("newGovernance")); } @@ -702,7 +702,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { assertEq(_isEmergencyStateActivated(), true); vm.prank(stranger); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtectedTimelock.CallerIsNotAdminExecutor.selector, stranger)); + vm.expectRevert(abi.encodeWithSelector(TimelockState.CallerIsNotAdminExecutor.selector, stranger)); _timelock.deactivateEmergencyMode(); } @@ -800,7 +800,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { vm.assume(stranger != _adminExecutor); EmergencyProtectedTimelock _localTimelock = _deployEmergencyProtectedTimelock(); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtectedTimelock.CallerIsNotAdminExecutor.selector, stranger)); + vm.expectRevert(abi.encodeWithSelector(TimelockState.CallerIsNotAdminExecutor.selector, stranger)); vm.prank(stranger); _localTimelock.setEmergencyProtectionActivationCommittee(_emergencyActivator); @@ -825,7 +825,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { vm.assume(stranger != _adminExecutor && stranger != _emergencyEnactor); EmergencyProtectedTimelock _localTimelock = _deployEmergencyProtectedTimelock(); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtectedTimelock.CallerIsNotAdminExecutor.selector, stranger)); + vm.expectRevert(abi.encodeWithSelector(TimelockState.CallerIsNotAdminExecutor.selector, stranger)); vm.prank(stranger); _localTimelock.setEmergencyProtectionExecutionCommittee(_emergencyEnactor); @@ -853,7 +853,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { vm.assume(stranger != _adminExecutor); EmergencyProtectedTimelock _localTimelock = _deployEmergencyProtectedTimelock(); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtectedTimelock.CallerIsNotAdminExecutor.selector, stranger)); + vm.expectRevert(abi.encodeWithSelector(TimelockState.CallerIsNotAdminExecutor.selector, stranger)); vm.prank(stranger); _localTimelock.setEmergencyProtectionEndDate(_emergencyProtectionDuration.addTo(Timestamps.now())); @@ -884,7 +884,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { vm.assume(stranger != _adminExecutor); EmergencyProtectedTimelock _localTimelock = _deployEmergencyProtectedTimelock(); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtectedTimelock.CallerIsNotAdminExecutor.selector, stranger)); + vm.expectRevert(abi.encodeWithSelector(TimelockState.CallerIsNotAdminExecutor.selector, stranger)); vm.prank(stranger); _localTimelock.setEmergencyModeDuration(_emergencyModeDuration); @@ -1250,7 +1250,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { vm.assume(stranger != _adminExecutor); vm.prank(stranger); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtectedTimelock.CallerIsNotAdminExecutor.selector, stranger)); + vm.expectRevert(abi.encodeWithSelector(TimelockState.CallerIsNotAdminExecutor.selector, stranger)); _timelock.setAdminExecutor(address(0x123)); }