Skip to content

Commit

Permalink
Merge pull request #151 from lidofinance/refactor/check-admin-executor
Browse files Browse the repository at this point in the history 
Move `_checkCallerIsAdminExecutor()` into `TimelockState` lib
  • Loading branch information
@bulbozaur
bulbozaur authored Nov 29, 2024
2 parents f41b870 + 10ec606 commit ed8f264
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 38 deletions.
38 changes: 11 additions & 27 deletions contracts/EmergencyProtectedTimelock.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
// ---
Expand Down Expand Up @@ -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();
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}

Expand All @@ -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
);
Expand All @@ -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);
}

Expand All @@ -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();
Expand Down Expand Up @@ -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);
}
}
}
21 changes: 20 additions & 1 deletion contracts/libraries/TimelockState.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -42,7 +43,7 @@ library TimelockState {
}

// ---
// Main Functionality
// State Management
// ---

/// @notice Sets the governance address.
Expand Down Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -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);
}
}
}


20 changes: 10 additions & 10 deletions test/unit/EmergencyProtectedTimelock.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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"));
}

Expand Down Expand Up @@ -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"));
}

Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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);

Expand All @@ -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);

Expand Down Expand Up @@ -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()));

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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));
}

Expand Down

0 comments on commit ed8f264

Please sign in to comment.