Merge pull request #98 from lidofinance/feature/failure-models-of-set…

…ters

Change failure models of setters

contracts/EmergencyProtectedTimelock.sol

@@ -109,7 +109,7 @@ contract EmergencyProtectedTimelock is ITimelock {
         _timelockState.setGovernance(newGovernance);
     }
 
-    function setDelays(Duration afterSubmitDelay, Duration afterScheduleDelay) external {
+    function setupDelays(Duration afterSubmitDelay, Duration afterScheduleDelay) external {
         _checkCallerIsAdminExecutor();
         _timelockState.setAfterSubmitDelay(afterSubmitDelay, MAX_AFTER_SUBMIT_DELAY);
         _timelockState.setAfterScheduleDelay(afterScheduleDelay, MAX_AFTER_SCHEDULE_DELAY);
@@ -128,22 +128,41 @@ contract EmergencyProtectedTimelock is ITimelock {
     // Emergency Protection Functionality
     // ---
 
-    function setupEmergencyProtection(
-        address emergencyGovernance,
-        address emergencyActivationCommittee,
-        address emergencyExecutionCommittee,
-        Timestamp emergencyProtectionEndDate,
-        Duration emergencyModeDuration
-    ) external {
+    /// @dev Sets the emergency activation committee address.
+    /// @param emergencyActivationCommittee The address of the emergency activation committee.
+    function setEmergencyProtectionActivationCommittee(address emergencyActivationCommittee) external {
         _checkCallerIsAdminExecutor();
-
-        _emergencyProtection.setEmergencyGovernance(emergencyGovernance);
         _emergencyProtection.setEmergencyActivationCommittee(emergencyActivationCommittee);
+    }
+
+    /// @dev Sets the emergency execution committee address.
+    /// @param emergencyExecutionCommittee The address of the emergency execution committee.
+    function setEmergencyProtectionExecutionCommittee(address emergencyExecutionCommittee) external {
+        _checkCallerIsAdminExecutor();
+        _emergencyProtection.setEmergencyExecutionCommittee(emergencyExecutionCommittee);
+    }
+
+    /// @dev Sets the emergency protection end date.
+    /// @param emergencyProtectionEndDate The timestamp of the emergency protection end date.
+    function setEmergencyProtectionEndDate(Timestamp emergencyProtectionEndDate) external {
+        _checkCallerIsAdminExecutor();
         _emergencyProtection.setEmergencyProtectionEndDate(
             emergencyProtectionEndDate, MAX_EMERGENCY_PROTECTION_DURATION
         );
+    }
+
+    /// @dev Sets the emergency mode duration.
+    /// @param emergencyModeDuration The duration of the emergency mode.
+    function setEmergencyModeDuration(Duration emergencyModeDuration) external {
+        _checkCallerIsAdminExecutor();
         _emergencyProtection.setEmergencyModeDuration(emergencyModeDuration, MAX_EMERGENCY_MODE_DURATION);
-        _emergencyProtection.setEmergencyExecutionCommittee(emergencyExecutionCommittee);
+    }
+
+    /// @dev Sets the emergency governance address.
+    /// @param emergencyGovernance The address of the emergency governance.
+    function setEmergencyGovernance(address emergencyGovernance) external {
+        _checkCallerIsAdminExecutor();
+        _emergencyProtection.setEmergencyGovernance(emergencyGovernance);
     }
 
     /// @dev Activates the emergency mode.

contracts/committees/HashConsensus.sol

@@ -23,6 +23,7 @@ abstract contract HashConsensus is Ownable {
     error HashAlreadyUsed(bytes32 hash);
     error QuorumIsNotReached();
     error InvalidQuorum();
+    error InvalidTimelockDuration(uint256 timelock);
     error TimelockNotPassed();
 
     struct HashState {
@@ -155,6 +156,9 @@ abstract contract HashConsensus is Ownable {
     /// @param timelock The new timelock duration in seconds
     function setTimelockDuration(uint256 timelock) public {
         _checkOwner();
+        if (timelock == timelockDuration) {
+            revert InvalidTimelockDuration(timelock);
+        }
         timelockDuration = timelock;
         emit TimelockDurationSet(timelock);
     }
@@ -171,7 +175,7 @@ abstract contract HashConsensus is Ownable {
     /// @dev The quorum value must be greater than zero and not exceed the current number of members.
     /// @param executionQuorum The new quorum value to be set.
     function _setQuorum(uint256 executionQuorum) internal {
-        if (executionQuorum == 0 || executionQuorum > _members.length()) {
+        if (executionQuorum == 0 || executionQuorum > _members.length() || executionQuorum == quorum) {
             revert InvalidQuorum();
         }
         quorum = executionQuorum;

contracts/libraries/EmergencyProtection.sol

@@ -11,6 +11,9 @@ library EmergencyProtection {
     error CallerIsNotEmergencyActivationCommittee(address caller);
     error CallerIsNotEmergencyExecutionCommittee(address caller);
     error EmergencyProtectionExpired(Timestamp protectedTill);
+    error InvalidEmergencyGovernance(address governance);
+    error InvalidEmergencyActivationCommittee(address committee);
+    error InvalidEmergencyExecutionCommittee(address committee);
     error InvalidEmergencyModeDuration(Duration value);
     error InvalidEmergencyProtectionEndDate(Timestamp value);
     error UnexpectedEmergencyModeState(bool value);
@@ -52,7 +55,6 @@ library EmergencyProtection {
         }
 
         self.emergencyModeEndsAfter = self.emergencyModeDuration.addTo(now_);
-
         emit EmergencyModeActivated();
     }
 
@@ -71,57 +73,73 @@ library EmergencyProtection {
     // Setup functionality
     // ---
 
+    /// @dev Sets the emergency governance address.
+    /// @param self The storage reference to the Context struct.
+    /// @param newEmergencyGovernance The new emergency governance address.
     function setEmergencyGovernance(Context storage self, address newEmergencyGovernance) internal {
         if (newEmergencyGovernance == self.emergencyGovernance) {
-            return;
+            revert InvalidEmergencyGovernance(newEmergencyGovernance);
         }
         self.emergencyGovernance = newEmergencyGovernance;
         emit EmergencyGovernanceSet(newEmergencyGovernance);
     }
 
+    /// @dev Sets the emergency protection end date.
+    /// @param self The storage reference to the Context struct.
+    /// @param newEmergencyProtectionEndDate The new emergency protection end date.
+    /// @param maxEmergencyProtectionDuration The maximum duration for the emergency protection.
     function setEmergencyProtectionEndDate(
         Context storage self,
         Timestamp newEmergencyProtectionEndDate,
         Duration maxEmergencyProtectionDuration
     ) internal {
-        if (newEmergencyProtectionEndDate > maxEmergencyProtectionDuration.addTo(Timestamps.now())) {
+        if (
+            newEmergencyProtectionEndDate > maxEmergencyProtectionDuration.addTo(Timestamps.now())
+                || newEmergencyProtectionEndDate == self.emergencyProtectionEndsAfter
+        ) {
             revert InvalidEmergencyProtectionEndDate(newEmergencyProtectionEndDate);
         }
-
-        if (newEmergencyProtectionEndDate == self.emergencyProtectionEndsAfter) {
-            return;
-        }
         self.emergencyProtectionEndsAfter = newEmergencyProtectionEndDate;
         emit EmergencyProtectionEndDateSet(newEmergencyProtectionEndDate);
     }
 
+    /// @dev Sets the emergency mode duration.
+    /// @param self The storage reference to the Context struct.
+    /// @param newEmergencyModeDuration The new emergency mode duration.
+    /// @param maxEmergencyModeDuration The maximum duration for the emergency mode.
     function setEmergencyModeDuration(
         Context storage self,
         Duration newEmergencyModeDuration,
         Duration maxEmergencyModeDuration
     ) internal {
-        if (newEmergencyModeDuration > maxEmergencyModeDuration) {
+        if (
+            newEmergencyModeDuration > maxEmergencyModeDuration
+                || newEmergencyModeDuration == self.emergencyModeDuration
+        ) {
             revert InvalidEmergencyModeDuration(newEmergencyModeDuration);
         }
-        if (newEmergencyModeDuration == self.emergencyModeDuration) {
-            return;
-        }
 
         self.emergencyModeDuration = newEmergencyModeDuration;
         emit EmergencyModeDurationSet(newEmergencyModeDuration);
     }
 
+    /// @dev Sets the emergency activation committee address.
+    /// @param self The storage reference to the Context struct.
+    /// @param newActivationCommittee The new emergency activation committee address.
     function setEmergencyActivationCommittee(Context storage self, address newActivationCommittee) internal {
         if (newActivationCommittee == self.emergencyActivationCommittee) {
-            return;
+            revert InvalidEmergencyActivationCommittee(newActivationCommittee);
         }
         self.emergencyActivationCommittee = newActivationCommittee;
         emit EmergencyActivationCommitteeSet(newActivationCommittee);
     }
 
+    /// @dev Sets the emergency execution committee address.
+    /// @param self The storage reference to the Context struct.
+    /// @param newExecutionCommittee The new emergency execution committee address.
     function setEmergencyExecutionCommittee(Context storage self, address newExecutionCommittee) internal {
         if (newExecutionCommittee == self.emergencyExecutionCommittee) {
-            return;
+            revert InvalidEmergencyExecutionCommittee(newExecutionCommittee);
         }
         self.emergencyExecutionCommittee = newExecutionCommittee;
         emit EmergencyExecutionCommitteeSet(newExecutionCommittee);

contracts/libraries/EscrowState.sol

@@ -18,6 +18,7 @@ enum State {
     RageQuitEscrow
 }
 
+/// @title EscrowState
 /// @notice Represents the logic to manipulate the state of the Escrow
 library EscrowState {
     // ---
@@ -28,13 +29,13 @@ library EscrowState {
     error UnexpectedState(State value);
     error RageQuitExtraTimelockNotStarted();
     error WithdrawalsTimelockNotPassed();
-    error BatchesCreationNotInProgress();
+    error InvalidMinAssetsLockDuration(Duration newMinAssetsLockDuration);
 
     // ---
     // Events
     // ---
 
-    event RageQuitTimelockStarted();
+    event RageQuitTimelockStarted(Timestamp startedAt);
     event EscrowStateChanged(State from, State to);
     event RageQuitStarted(Duration rageQuitExtensionDelay, Duration rageQuitWithdrawalsTimelock);
     event MinAssetsLockDurationSet(Duration newAssetsLockDuration);
@@ -59,12 +60,19 @@ library EscrowState {
         Duration rageQuitWithdrawalsTimelock;
     }
 
+    /// @notice Initializes the Escrow state to SignallingEscrow
+    /// @param self The context of the Escrow instance
+    /// @param minAssetsLockDuration The minimum assets lock duration
     function initialize(Context storage self, Duration minAssetsLockDuration) internal {
         _checkState(self, State.NotInitialized);
         _setState(self, State.SignallingEscrow);
         _setMinAssetsLockDuration(self, minAssetsLockDuration);
     }
 
+    /// @notice Starts the rage quit process
+    /// @param self The context of the Escrow instance
+    /// @param rageQuitExtensionDelay The delay period for the rage quit extension
+    /// @param rageQuitWithdrawalsTimelock The timelock period for rage quit withdrawals
     function startRageQuit(
         Context storage self,
         Duration rageQuitExtensionDelay,
@@ -77,14 +85,19 @@ library EscrowState {
         emit RageQuitStarted(rageQuitExtensionDelay, rageQuitWithdrawalsTimelock);
     }
 
+    /// @notice Starts the rage quit extension delay
+    /// @param self The context of the Escrow instance
     function startRageQuitExtensionDelay(Context storage self) internal {
         self.rageQuitExtensionDelayStartedAt = Timestamps.now();
-        emit RageQuitTimelockStarted();
+        emit RageQuitTimelockStarted(self.rageQuitExtensionDelayStartedAt);
     }
 
+    /// @notice Sets the minimum assets lock duration
+    /// @param self The context of the Escrow instance
+    /// @param newMinAssetsLockDuration The new minimum assets lock duration
     function setMinAssetsLockDuration(Context storage self, Duration newMinAssetsLockDuration) internal {
         if (self.minAssetsLockDuration == newMinAssetsLockDuration) {
-            return;
+            revert InvalidMinAssetsLockDuration(newMinAssetsLockDuration);
         }
         _setMinAssetsLockDuration(self, newMinAssetsLockDuration);
     }
@@ -93,20 +106,28 @@ library EscrowState {
     // Checks
     // ---
 
+    /// @notice Checks if the Escrow is in the SignallingEscrow state
+    /// @param self The context of the Escrow instance
     function checkSignallingEscrow(Context storage self) internal view {
         _checkState(self, State.SignallingEscrow);
     }
 
+    /// @notice Checks if the Escrow is in the RageQuitEscrow state
+    /// @param self The context of the Escrow instance
     function checkRageQuitEscrow(Context storage self) internal view {
         _checkState(self, State.RageQuitEscrow);
     }
 
+    /// @notice Checks if batch claiming is in progress
+    /// @param self The context of the Escrow instance
     function checkBatchesClaimingInProgress(Context storage self) internal view {
         if (!self.rageQuitExtensionDelayStartedAt.isZero()) {
             revert ClaimingIsFinished();
         }
     }
 
+    /// @notice Checks if the withdrawals timelock has passed
+    /// @param self The context of the Escrow instance
     function checkWithdrawalsTimelockPassed(Context storage self) internal view {
         if (self.rageQuitExtensionDelayStartedAt.isZero()) {
             revert RageQuitExtraTimelockNotStarted();
@@ -120,16 +141,26 @@ library EscrowState {
     // ---
     // Getters
     // ---
+
+    /// @notice Checks if the rage quit extension delay has started
+    /// @param self The context of the Escrow instance
+    /// @return True if the rage quit extension delay has started, false otherwise
     function isRageQuitExtensionDelayStarted(Context storage self) internal view returns (bool) {
         return self.rageQuitExtensionDelayStartedAt.isNotZero();
     }
 
+    /// @notice Checks if the rage quit extension delay has passed
+    /// @param self The context of the Escrow instance
+    /// @return True if the rage quit extension delay has passed, false otherwise
     function isRageQuitExtensionDelayPassed(Context storage self) internal view returns (bool) {
         Timestamp rageQuitExtensionDelayStartedAt = self.rageQuitExtensionDelayStartedAt;
         return rageQuitExtensionDelayStartedAt.isNotZero()
             && Timestamps.now() > self.rageQuitExtensionDelay.addTo(rageQuitExtensionDelayStartedAt);
     }
 
+    /// @notice Checks if the Escrow is in the RageQuitEscrow state
+    /// @param self The context of the Escrow instance
+    /// @return True if the Escrow is in the RageQuitEscrow state, false otherwise
     function isRageQuitEscrow(Context storage self) internal view returns (bool) {
         return self.state == State.RageQuitEscrow;
     }
@@ -138,18 +169,27 @@ library EscrowState {
     // Private Methods
     // ---
 
+    /// @notice Checks if the Escrow is in the expected state
+    /// @param self The context of the Escrow instance
+    /// @param state The expected state
     function _checkState(Context storage self, State state) private view {
         if (self.state != state) {
             revert UnexpectedState(state);
         }
     }
 
+    /// @notice Sets the state of the Escrow
+    /// @param self The context of the Escrow instance
+    /// @param newState The new state
     function _setState(Context storage self, State newState) private {
         State prevState = self.state;
         self.state = newState;
         emit EscrowStateChanged(prevState, newState);
     }
 
+    /// @notice Sets the minimum assets lock duration
+    /// @param self The context of the Escrow instance
+    /// @param newMinAssetsLockDuration The new minimum assets lock duration
     function _setMinAssetsLockDuration(Context storage self, Duration newMinAssetsLockDuration) private {
         self.minAssetsLockDuration = newMinAssetsLockDuration;
         emit MinAssetsLockDurationSet(newMinAssetsLockDuration);

contracts/libraries/Tiebreaker.sol

@@ -68,12 +68,9 @@ library Tiebreaker {
     }
 
     function setTiebreakerCommittee(Context storage self, address newTiebreakerCommittee) internal {
-        if (newTiebreakerCommittee == address(0)) {
+        if (newTiebreakerCommittee == address(0) || newTiebreakerCommittee == self.tiebreakerCommittee) {
             revert InvalidTiebreakerCommittee(newTiebreakerCommittee);
         }
-        if (self.tiebreakerCommittee == newTiebreakerCommittee) {
-            return;
-        }
         self.tiebreakerCommittee = newTiebreakerCommittee;
         emit TiebreakerCommitteeSet(newTiebreakerCommittee);
     }
@@ -87,13 +84,10 @@ library Tiebreaker {
         if (
             newTiebreakerActivationTimeout < minTiebreakerActivationTimeout
                 || newTiebreakerActivationTimeout > maxTiebreakerActivationTimeout
+                || newTiebreakerActivationTimeout == self.tiebreakerActivationTimeout
         ) {
             revert InvalidTiebreakerActivationTimeout(newTiebreakerActivationTimeout);
         }
-
-        if (self.tiebreakerActivationTimeout == newTiebreakerActivationTimeout) {
-            return;
-        }
         self.tiebreakerActivationTimeout = newTiebreakerActivationTimeout;
         emit TiebreakerActivationTimeoutSet(newTiebreakerActivationTimeout);
     }