Merge pull request lidofinance#14 from lidofinance/feature/timelock-l…

…ibs-changes

EmergencyProtectedTimelock scenario tests & fixes

contracts/DualGovernance.sol

@@ -123,10 +123,6 @@ contract DualGovernance {
         proposal = _proposals.adopt(proposalId, CONFIG.minProposalExecutionTimelock());
     }
 
-    function _getTime() internal view virtual returns (uint256) {
-        return block.timestamp;
-    }
-
     modifier onlyAdminExecutor() {
         if (msg.sender != TIMELOCK.ADMIN_EXECUTOR()) {
             revert Unauthorized();

contracts/EmergencyProtectedTimelock.sol

@@ -1,13 +1,12 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.23;
 
-import {Address} from "@openzeppelin/contracts/utils/Address.sol";
 import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
 
 import {IOwnable} from "./interfaces/IOwnable.sol";
 import {ITimelock} from "./interfaces/ITimelock.sol";
 
-import {EmergencyProtection} from "./libraries/EmergencyProtection.sol";
+import {EmergencyProtection, EmergencyState} from "./libraries/EmergencyProtection.sol";
 import {ScheduledCallsBatches, ScheduledCallsBatch, ExecutorCall} from "./libraries/ScheduledCalls.sol";
 
 contract EmergencyProtectedTimelock is ITimelock {
@@ -18,7 +17,6 @@ contract EmergencyProtectedTimelock is ITimelock {
     error NotGovernance(address sender);
     error NotAdminExecutor(address sender);
 
-    event DelaySet(uint256 delay);
     event GovernanceSet(address indexed governance);
 
     address public immutable ADMIN_EXECUTOR;
@@ -34,19 +32,21 @@ contract EmergencyProtectedTimelock is ITimelock {
         EMERGENCY_GOVERNANCE = emergencyGovernance;
     }
 
-    // executes call immediately when the delay for scheduled calls is set to 0
+    // executes call immediately when the delay is set to 0
     function relay(address executor, ExecutorCall[] calldata calls) external onlyGovernance {
         _scheduledCalls.relay(executor, calls);
     }
 
     // schedules call to be executed after some delay
     function schedule(uint256 batchId, address executor, ExecutorCall[] calldata calls) external onlyGovernance {
-        _scheduledCalls.add(batchId, executor, calls);
+        _scheduledCalls.schedule(batchId, executor, calls);
     }
 
     // executes scheduled call
     function execute(uint256 batchId) external {
-        if (_emergencyProtection.isActive()) {
+        // Until the emergency mode is deactivated manually, the execution of the calls is allowed
+        // only for the emergency committee
+        if (_emergencyProtection.isEmergencyModeActivated()) {
             _emergencyProtection.validateIsCommittee(msg.sender);
         }
         _scheduledCalls.execute(batchId);
@@ -56,34 +56,45 @@ contract EmergencyProtectedTimelock is ITimelock {
         _scheduledCalls.removeCanceled(batchId);
     }
 
-    function setGovernance(address governance, uint256 delay) external onlyAdminExecutor {
-        _setGovernance(governance, delay);
+    function setGovernanceAndDelay(address governance, uint256 delay) external onlyAdminExecutor {
+        _setGovernance(governance);
+        _scheduledCalls.setDelay(delay);
+    }
+
+    function setDelay(uint256 delay) external onlyAdminExecutor {
+        _scheduledCalls.setDelay(delay);
     }
 
     function transferExecutorOwnership(address executor, address owner) external onlyAdminExecutor {
         IOwnable(executor).transferOwnership(owner);
     }
 
-    function setEmergencyProtection(address committee, uint256 lifetime, uint256 duration) external onlyAdminExecutor {
-        _emergencyProtection.setup(committee, lifetime, duration);
+    function setEmergencyProtection(
+        address committee,
+        uint256 protectionDuration,
+        uint256 emergencyModeDuration
+    ) external onlyAdminExecutor {
+        _emergencyProtection.setup(committee, protectionDuration, emergencyModeDuration);
     }
 
     function emergencyModeActivate() external {
         _emergencyProtection.activate();
     }
 
     function emergencyModeDeactivate() external {
-        if (_emergencyProtection.isActive()) {
+        if (!_emergencyProtection.isEmergencyModePassed()) {
             _assertAdminExecutor();
         }
-        _scheduledCalls.cancelAll();
         _emergencyProtection.deactivate();
+        _scheduledCalls.cancelAll();
     }
 
     function emergencyResetGovernance() external {
-        _scheduledCalls.cancelAll();
+        _emergencyProtection.validateIsCommittee(msg.sender);
         _emergencyProtection.reset();
-        _setGovernance(EMERGENCY_GOVERNANCE, 0);
+        _scheduledCalls.cancelAll();
+        _scheduledCalls.setDelay(0);
+        _setGovernance(EMERGENCY_GOVERNANCE);
     }
 
     function getDelay() external view returns (uint256 delay) {
@@ -107,41 +118,23 @@ contract EmergencyProtectedTimelock is ITimelock {
     }
 
     function getIsExecutable(uint256 batchId) external view returns (bool isExecutable) {
-        isExecutable = _scheduledCalls.isExecutable(batchId);
+        isExecutable = !_emergencyProtection.isEmergencyModeActivated() && _scheduledCalls.isExecutable(batchId);
     }
 
     function getIsCanceled(uint256 batchId) external view returns (bool isExecutable) {
         isExecutable = _scheduledCalls.isCanceled(batchId);
     }
 
-    struct EmergencyState {
-        bool isActive;
-        address committee;
-        uint256 protectedTill;
-        uint256 emergencyModeEndsAfter;
-        uint256 emergencyModeDuration;
-    }
-
     function getEmergencyState() external view returns (EmergencyState memory res) {
-        EmergencyProtection.State memory state = _emergencyProtection;
-        res.isActive = _emergencyProtection.isActive();
-        res.committee = state.committee;
-        res.protectedTill = state.protectedTill;
-        res.emergencyModeEndsAfter = state.emergencyModeEndsAfter;
-        res.emergencyModeDuration = state.emergencyModeDuration;
+        res = _emergencyProtection.getEmergencyState();
     }
 
-    function _setGovernance(address governance, uint256 delay) internal {
+    function _setGovernance(address governance) internal {
         address prevGovernance = _governance;
-        uint256 prevDelay = _scheduledCalls.delay;
         if (prevGovernance != governance) {
             _governance = governance;
             emit GovernanceSet(governance);
         }
-        if (prevDelay != delay) {
-            _scheduledCalls.delay = delay.toUint32();
-            emit DelaySet(delay);
-        }
     }
 
     function _assertAdminExecutor() private view {

contracts/libraries/EmergencyProtection.sol

@@ -3,21 +3,27 @@ pragma solidity 0.8.23;
 
 import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
 
+struct EmergencyState {
+    address committee;
+    uint256 protectedTill;
+    bool isEmergencyModeActivated;
+    uint256 emergencyModeDuration;
+    uint256 emergencyModeEndsAfter;
+}
+
 library EmergencyProtection {
     error NotEmergencyCommittee(address sender);
-    error EmergencyCommitteeExpired();
-    error EmergencyModeNotEntered();
+    error EmergencyModeNotActivated();
     error EmergencyPeriodFinished();
-    error EmergencyPeriodNotFinished();
-    error EmergencyModeIsActive();
-    error EmergencyModeWasActivatedPreviously();
+    error EmergencyCommitteeExpired();
+    error EmergencyModeAlreadyActive();
 
     event EmergencyModeActivated();
     event EmergencyModeDeactivated();
     event EmergencyGovernanceReset();
     event EmergencyCommitteeSet(address indexed guardian);
-    event EmergencyDurationSet(uint256 emergencyModeDuration);
-    event EmergencyCommitteeActiveTillSet(uint256 guardedTill);
+    event EmergencyModeDurationSet(uint256 emergencyModeDuration);
+    event EmergencyCommitteeProtectedTillSet(uint256 protectedTill);
 
     struct State {
         // has rights to activate emergency mode
@@ -28,25 +34,30 @@ library EmergencyProtection {
         uint32 emergencyModeDuration;
     }
 
-    function setup(State storage self, address committee, uint256 lifetime, uint256 duration) internal {
+    function setup(
+        State storage self,
+        address committee,
+        uint256 protectionDuration,
+        uint256 emergencyModeDuration
+    ) internal {
         address prevCommittee = self.committee;
         if (prevCommittee != committee) {
             self.committee = committee;
             emit EmergencyCommitteeSet(committee);
         }
 
         uint256 prevProtectedTill = self.protectedTill;
-        uint256 protectedTill = block.timestamp + lifetime;
+        uint256 protectedTill = block.timestamp + protectionDuration;
 
         if (prevProtectedTill != protectedTill) {
             self.protectedTill = SafeCast.toUint40(protectedTill);
-            emit EmergencyCommitteeActiveTillSet(protectedTill);
+            emit EmergencyCommitteeProtectedTillSet(protectedTill);
         }
 
         uint256 prevDuration = self.emergencyModeDuration;
-        if (prevDuration != duration) {
-            self.emergencyModeDuration = SafeCast.toUint32(duration);
-            emit EmergencyDurationSet(duration);
+        if (prevDuration != emergencyModeDuration) {
+            self.emergencyModeDuration = SafeCast.toUint32(emergencyModeDuration);
+            emit EmergencyModeDurationSet(emergencyModeDuration);
         }
     }
 
@@ -58,7 +69,7 @@ library EmergencyProtection {
             revert EmergencyCommitteeExpired();
         }
         if (self.emergencyModeEndsAfter != 0) {
-            revert EmergencyModeIsActive();
+            revert EmergencyModeAlreadyActive();
         }
         self.emergencyModeEndsAfter = SafeCast.toUint40(block.timestamp + self.emergencyModeDuration);
         emit EmergencyModeActivated();
@@ -67,7 +78,7 @@ library EmergencyProtection {
     function deactivate(State storage self) internal {
         uint256 endsAfter = self.emergencyModeEndsAfter;
         if (endsAfter == 0) {
-            revert EmergencyModeNotEntered();
+            revert EmergencyModeNotActivated();
         }
         _reset(self);
         emit EmergencyModeDeactivated();
@@ -76,10 +87,7 @@ library EmergencyProtection {
     function reset(State storage self) internal {
         uint256 endsAfter = self.emergencyModeEndsAfter;
         if (endsAfter == 0) {
-            revert EmergencyModeNotEntered();
-        }
-        if (msg.sender != self.committee) {
-            revert NotEmergencyCommittee(msg.sender);
+            revert EmergencyModeNotActivated();
         }
         if (block.timestamp > endsAfter) {
             revert EmergencyPeriodFinished();
@@ -88,25 +96,31 @@ library EmergencyProtection {
         emit EmergencyGovernanceReset();
     }
 
-    function isActive(State storage self) internal view returns (bool) {
-        uint256 endsAfter = self.emergencyModeEndsAfter;
-        if (endsAfter == 0) return false;
-        return endsAfter >= block.timestamp;
-    }
-
-    function isCommittee(State storage self) internal view returns (bool) {
-        return msg.sender == self.committee;
-    }
-
     function validateIsCommittee(State storage self, address account) internal view {
         if (self.committee != account) {
             revert NotEmergencyCommittee(account);
         }
     }
 
+    function getEmergencyState(State storage self) internal view returns (EmergencyState memory res) {
+        res.committee = self.committee;
+        res.protectedTill = self.protectedTill;
+        res.emergencyModeDuration = self.emergencyModeDuration;
+        res.emergencyModeEndsAfter = self.emergencyModeEndsAfter;
+        res.isEmergencyModeActivated = isEmergencyModeActivated(self);
+    }
+
+    function isEmergencyModeActivated(State storage self) internal view returns (bool) {
+        return self.emergencyModeEndsAfter != 0;
+    }
+
+    function isEmergencyModePassed(State storage self) internal view returns (bool) {
+        uint256 endsAfter = self.emergencyModeEndsAfter;
+        return endsAfter != 0 && block.timestamp > endsAfter;
+    }
+
     function _reset(State storage self) private {
         self.committee = address(0);
-        self.protectedTill = 0;
         self.emergencyModeDuration = 0;
         self.emergencyModeEndsAfter = 0;
     }

contracts/libraries/Proposals.sol

@@ -17,7 +17,7 @@ struct Proposal {
 struct ProposalPacked {
     address proposer;
     uint40 proposedAt;
-    // time passed, starting from proposedAt to adopting the proposal
+    // Time passed, starting from the proposedAt till the adoption of the proposal
     uint32 adoptionTime;
     address executor;
     ExecutorCall[] calls;
@@ -30,7 +30,7 @@ library Proposals {
     uint256 private constant FIRST_PROPOSAL_ID = 1;
 
     struct State {
-        // all proposals with ids less or equal than given one cannot be executed
+        // any proposals with ids less or equal to the given one cannot be executed
         uint256 lastCanceledProposalId;
         ProposalPacked[] proposals;
     }
@@ -50,27 +50,27 @@ library Proposals {
         address proposer,
         address executor,
         ExecutorCall[] calldata calls
-    ) internal returns (uint256) {
+    ) internal returns (uint256 newProposalId) {
         if (calls.length == 0) {
             revert EmptyCalls();
         }
 
+        newProposalId = self.proposals.length;
         self.proposals.push();
-        uint256 newProposalId = self.proposals.length - FIRST_PROPOSAL_ID;
+
         ProposalPacked storage newProposal = self.proposals[newProposalId];
         newProposal.proposer = proposer;
         newProposal.executor = executor;
         newProposal.adoptionTime = 0;
         newProposal.proposedAt = block.timestamp.toUint40();
 
-        // copying of arrays of custom types from calldata to storage has not supported
-        // by the Solidity compiler yet, so copy item by item
+        // copying of arrays of custom types from calldata to storage has not been supported by the
+        // Solidity compiler yet, so insert item by item
         for (uint256 i = 0; i < calls.length; ++i) {
             newProposal.calls.push(calls[i]);
         }
 
         emit Proposed(newProposalId, proposer, executor, calls);
-        return newProposalId;
     }
 
     function cancelAll(State storage self) internal {
@@ -86,19 +86,18 @@ library Proposals {
             revert ProposalCanceled(proposalId);
         }
         uint256 proposedAt = packed.proposedAt;
-        uint256 adoptionTime = packed.adoptionTime;
+        if (packed.adoptionTime != 0) {
+            revert ProposalAlreadyAdopted(proposalId, proposedAt + packed.adoptionTime);
+        }
         if (block.timestamp < proposedAt + delay) {
             revert ProposalNotExecutable(proposalId);
         }
-        if (adoptionTime != 0) {
-            revert ProposalAlreadyAdopted(proposalId, proposedAt + adoptionTime);
-        }
-        uint256 adoptionDelay = block.timestamp - proposedAt;
+        uint256 adoptionTime = block.timestamp - proposedAt;
         // the proposal can't be proposed and adopted at the same transaction
-        if (adoptionDelay == 0) {
+        if (adoptionTime == 0) {
             revert InvalidAdoptionDelay(0);
         }
-        packed.adoptionTime = adoptionDelay.toUint32();
+        packed.adoptionTime = adoptionTime.toUint32();
         proposal = _unpack(proposalId, packed);
     }