diff --git a/contracts/DualGovernance.sol b/contracts/DualGovernance.sol index d25c21d8..d32354b0 100644 --- a/contracts/DualGovernance.sol +++ b/contracts/DualGovernance.sol @@ -226,7 +226,7 @@ contract DualGovernance is IDualGovernance { /// or `VetoSignallingDeactivation` state. /// @dev If the Dual Governance state is not `VetoSignalling` or `VetoSignallingDeactivation`, the function will /// exit early, emitting the `CancelAllPendingProposalsSkipped` event without canceling any proposals. - /// @return isProposalsCancelled A boolean indicating whether the proposals were successfully canceled (`true`) + /// @return isProposalsCancelled A boolean indicating whether the proposals were successfully cancelled (`true`) /// or the cancellation was skipped due to an inappropriate state (`false`). function cancelAllPendingProposals() external returns (bool) { _stateMachine.activateNextState(); @@ -240,7 +240,7 @@ contract DualGovernance is IDualGovernance { /// reached consensus. This could lead to a situation where a proposer’s cancelAllPendingProposals() call /// becomes unexecutable if the Dual Governance state changes. However, it might become executable again if /// the system state shifts back to VetoSignalling or VetoSignallingDeactivation. - /// To avoid such a scenario, an early return is used instead of a revert when proposals cannot be canceled + /// To avoid such a scenario, an early return is used instead of a revert when proposals cannot be cancelled /// due to an unsuitable Dual Governance state. emit CancelAllPendingProposalsSkipped(); return false; @@ -265,7 +265,7 @@ contract DualGovernance is IDualGovernance { /// - The Dual Governance system is in the `Normal` or `VetoCooldown` state. /// - If the system is in the `VetoCooldown` state, the proposal must have been submitted before the system /// last entered the `VetoSignalling` state. - /// - The proposal has not already been scheduled, canceled, or executed. + /// - The proposal has not already been scheduled, cancelled, or executed. /// - The required delay period, as defined by `ITimelock.getAfterSubmitDelay()`, has elapsed since the proposal /// was submitted. /// @param proposalId The unique identifier of the proposal to check. @@ -282,9 +282,9 @@ contract DualGovernance is IDualGovernance { /// `DualGovernance.cancelAllPendingProposals()` method. /// @dev Proposal cancellation is only allowed when the Dual Governance system is in the `VetoSignalling` or /// `VetoSignallingDeactivation` states. In any other state, the cancellation will be skipped and no proposals - /// will be canceled. + /// will be cancelled. /// @return canCancelAllPendingProposals A boolean value indicating whether the pending proposals can be - /// canceled (`true`) or not (`false`) based on the current `effective` state of the Dual Governance system. + /// cancelled (`true`) or not (`false`) based on the current `effective` state of the Dual Governance system. function canCancelAllPendingProposals() external view returns (bool) { return _stateMachine.canCancelAllPendingProposals({useEffectiveState: true}); } @@ -397,7 +397,7 @@ contract DualGovernance is IDualGovernance { _proposers.setProposerExecutor(proposerAccount, newExecutor); /// @dev after update of the proposer, check that admin executor still belongs to some proposer - _proposers.checkRegisteredExecutor(TIMELOCK.getAdminExecutor()); + _proposers.checkRegisteredExecutor(msg.sender); } /// @notice Unregisters a proposer from the system. diff --git a/contracts/EmergencyProtectedTimelock.sol b/contracts/EmergencyProtectedTimelock.sol index 5e634f0d..6ac4097f 100644 --- a/contracts/EmergencyProtectedTimelock.sol +++ b/contracts/EmergencyProtectedTimelock.sol @@ -137,7 +137,7 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { /// @param proposalId The id of the proposal to be executed. function execute(uint256 proposalId) external { _emergencyProtection.checkEmergencyMode({isActive: false}); - _proposals.execute(proposalId, _timelockState.getAfterScheduleDelay()); + _proposals.execute(proposalId, _timelockState.getAfterScheduleDelay(), MIN_EXECUTION_DELAY); } /// @notice Cancels all non-executed proposals, preventing them from being executed in the future. @@ -237,7 +237,11 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock { function emergencyExecute(uint256 proposalId) external { _emergencyProtection.checkEmergencyMode({isActive: true}); _emergencyProtection.checkCallerIsEmergencyExecutionCommittee(); - _proposals.execute({proposalId: proposalId, afterScheduleDelay: Duration.wrap(0)}); + _proposals.execute({ + proposalId: proposalId, + afterScheduleDelay: Durations.ZERO, + minExecutionDelay: Durations.ZERO + }); } /// @notice Deactivates the emergency mode. diff --git a/contracts/Escrow.sol b/contracts/Escrow.sol index 495474b0..421fa4c6 100644 --- a/contracts/Escrow.sol +++ b/contracts/Escrow.sol @@ -140,7 +140,7 @@ contract Escrow is ISignallingEscrow, IRageQuitEscrow { } _checkCallerIsDualGovernance(); - _escrowState.initialize(minAssetsLockDuration); + _escrowState.initialize(minAssetsLockDuration, MAX_MIN_ASSETS_LOCK_DURATION); ST_ETH.approve(address(WST_ETH), type(uint256).max); ST_ETH.approve(address(WITHDRAWAL_QUEUE), type(uint256).max); @@ -275,6 +275,9 @@ contract Escrow is ISignallingEscrow, IRageQuitEscrow { /// @param hints An array of hints required by the WithdrawalQueue to efficiently retrieve /// the claimable amounts for the unstETH NFTs. function markUnstETHFinalized(uint256[] memory unstETHIds, uint256[] calldata hints) external { + if (unstETHIds.length == 0) { + revert EmptyUnstETHIds(); + } DUAL_GOVERNANCE.activateNextState(); _escrowState.checkSignallingEscrow(); @@ -310,6 +313,7 @@ contract Escrow is ISignallingEscrow, IRageQuitEscrow { /// @param newMinAssetsLockDuration The new minimum lock duration to be set. function setMinAssetsLockDuration(Duration newMinAssetsLockDuration) external { _checkCallerIsDualGovernance(); + _escrowState.checkSignallingEscrow(); _escrowState.setMinAssetsLockDuration(newMinAssetsLockDuration, MAX_MIN_ASSETS_LOCK_DURATION); } diff --git a/contracts/TimelockedGovernance.sol b/contracts/TimelockedGovernance.sol index 64744417..b6804de3 100644 --- a/contracts/TimelockedGovernance.sol +++ b/contracts/TimelockedGovernance.sol @@ -70,6 +70,7 @@ contract TimelockedGovernance is IGovernance { } /// @notice Cancels all pending proposals that have not been executed. + /// @return A boolean indicating whether the operation was successful. function cancelAllPendingProposals() external returns (bool) { _checkCallerIsGovernance(); TIMELOCK.cancelAllNonExecutedProposals(); diff --git a/contracts/committees/TiebreakerCoreCommittee.sol b/contracts/committees/TiebreakerCoreCommittee.sol index 6013f481..8ebea8a6 100644 --- a/contracts/committees/TiebreakerCoreCommittee.sol +++ b/contracts/committees/TiebreakerCoreCommittee.sol @@ -11,6 +11,7 @@ import {ITimelock} from "../interfaces/ITimelock.sol"; import {ITiebreaker} from "../interfaces/ITiebreaker.sol"; import {IDualGovernance} from "../interfaces/IDualGovernance.sol"; import {ITiebreakerCoreCommittee} from "../interfaces/ITiebreakerCoreCommittee.sol"; +import {ISealable} from "../interfaces/ISealable.sol"; import {HashConsensus} from "./HashConsensus.sol"; import {ProposalsList} from "./ProposalsList.sol"; @@ -26,6 +27,7 @@ enum ProposalType { contract TiebreakerCoreCommittee is ITiebreakerCoreCommittee, HashConsensus, ProposalsList { error ResumeSealableNonceMismatch(); error ProposalDoesNotExist(uint256 proposalId); + error SealableIsNotPaused(address sealable); error InvalidSealable(address sealable); address public immutable DUAL_GOVERNANCE; @@ -112,15 +114,12 @@ contract TiebreakerCoreCommittee is ITiebreakerCoreCommittee, HashConsensus, Pro /// @notice Votes on a proposal to resume a sealable address /// @dev Allows committee members to vote on resuming a sealable address - /// reverts if the sealable address is the zero address + /// reverts if the sealable address is the zero address or if the sealable address is not paused /// @param sealable The address to resume /// @param nonce The nonce for the resume proposal function sealableResume(address sealable, uint256 nonce) external { _checkCallerIsMember(); - - if (sealable == address(0)) { - revert InvalidSealable(sealable); - } + checkSealableIsPaused(sealable); if (nonce != _sealableResumeNonces[sealable]) { revert ResumeSealableNonceMismatch(); @@ -147,6 +146,18 @@ contract TiebreakerCoreCommittee is ITiebreakerCoreCommittee, HashConsensus, Pro return _getHashState(key); } + /// @notice Checks if a sealable address is paused + /// @dev Checks if the sealable address is paused by calling the getResumeSinceTimestamp function on the ISealable contract + /// @param sealable The address to check + function checkSealableIsPaused(address sealable) public view { + if (sealable == address(0)) { + revert InvalidSealable(sealable); + } + if (ISealable(sealable).getResumeSinceTimestamp() <= block.timestamp) { + revert SealableIsNotPaused(sealable); + } + } + /// @notice Executes an approved resume sealable proposal /// @dev Executes the resume sealable proposal by calling the tiebreakerResumeSealable function on the Dual Governance contract /// @param sealable The address to resume diff --git a/contracts/committees/TiebreakerSubCommittee.sol b/contracts/committees/TiebreakerSubCommittee.sol index 9cde60a1..c85bb917 100644 --- a/contracts/committees/TiebreakerSubCommittee.sol +++ b/contracts/committees/TiebreakerSubCommittee.sol @@ -21,8 +21,6 @@ enum ProposalType { /// @notice This contract allows a subcommittee to vote on and execute proposals for scheduling and resuming sealable addresses /// @dev Inherits from HashConsensus for voting mechanisms and ProposalsList for proposal management contract TiebreakerSubCommittee is HashConsensus, ProposalsList { - error InvalidSealable(address sealable); - address public immutable TIEBREAKER_CORE_COMMITTEE; constructor( @@ -95,14 +93,12 @@ contract TiebreakerSubCommittee is HashConsensus, ProposalsList { /// @notice Votes on a proposal to resume a sealable address /// @dev Allows committee members to vote on resuming a sealable address - /// reverts if the sealable address is the zero address + /// reverts if the sealable address is the zero address or if the sealable address is not paused /// @param sealable The address to resume function sealableResume(address sealable) external { _checkCallerIsMember(); + ITiebreakerCoreCommittee(TIEBREAKER_CORE_COMMITTEE).checkSealableIsPaused(sealable); - if (sealable == address(0)) { - revert InvalidSealable(sealable); - } (bytes memory proposalData, bytes32 key,) = _encodeSealableResume(sealable); _vote(key, true); _pushProposal(key, uint256(ProposalType.ResumeSealable), proposalData); diff --git a/contracts/interfaces/IDualGovernance.sol b/contracts/interfaces/IDualGovernance.sol index d1a6cc09..44801495 100644 --- a/contracts/interfaces/IDualGovernance.sol +++ b/contracts/interfaces/IDualGovernance.sol @@ -38,17 +38,20 @@ interface IDualGovernance is IGovernance, ITiebreaker { function getEffectiveState() external view returns (State effectiveState); function getStateDetails() external view returns (StateDetails memory stateDetails); - function registerProposer(address proposer, address executor) external; + function registerProposer(address proposerAccount, address executor) external; function setProposerExecutor(address proposerAccount, address newExecutor) external; - function unregisterProposer(address proposer) external; - function isProposer(address account) external view returns (bool); - function getProposer(address account) external view returns (Proposers.Proposer memory proposer); + function unregisterProposer(address proposerAccount) external; + function isProposer(address proposerAccount) external view returns (bool); + function getProposer(address proposerAccount) external view returns (Proposers.Proposer memory proposer); function getProposers() external view returns (Proposers.Proposer[] memory proposers); - function isExecutor(address account) external view returns (bool); + function isExecutor(address executor) external view returns (bool); function resealSealable(address sealable) external; function setResealCommittee(address newResealCommittee) external; function setResealManager(IResealManager newResealManager) external; function getResealManager() external view returns (IResealManager); function getResealCommittee() external view returns (address); + + function setProposalsCanceller(address newProposalsCanceller) external; + function getProposalsCanceller() external view returns (address); } diff --git a/contracts/interfaces/ITiebreakerCoreCommittee.sol b/contracts/interfaces/ITiebreakerCoreCommittee.sol index 079e9f0f..80dc5986 100644 --- a/contracts/interfaces/ITiebreakerCoreCommittee.sol +++ b/contracts/interfaces/ITiebreakerCoreCommittee.sol @@ -7,4 +7,5 @@ interface ITiebreakerCoreCommittee { function scheduleProposal(uint256 proposalId) external; function sealableResume(address sealable, uint256 nonce) external; function checkProposalExists(uint256 proposalId) external view; + function checkSealableIsPaused(address sealable) external view; } diff --git a/contracts/libraries/DualGovernanceConfig.sol b/contracts/libraries/DualGovernanceConfig.sol index ad0cd7b8..dba4d41e 100644 --- a/contracts/libraries/DualGovernanceConfig.sol +++ b/contracts/libraries/DualGovernanceConfig.sol @@ -2,7 +2,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.26; -import {PercentD16} from "../types/PercentD16.sol"; +import {PercentD16, PercentsD16, HUNDRED_PERCENT_D16} from "../types/PercentD16.sol"; import {Duration, Durations} from "../types/Duration.sol"; import {Timestamp, Timestamps} from "../types/Timestamp.sol"; @@ -13,6 +13,7 @@ library DualGovernanceConfig { // Errors // --- + error InvalidSecondSealRageQuitSupport(PercentD16 secondSealRageQuitSupport); error InvalidRageQuitSupportRange(PercentD16 firstSealRageQuitSupport, PercentD16 secondSealRageQuitSupport); error InvalidRageQuitEthWithdrawalsDelayRange( Duration rageQuitEthWithdrawalsMinDelay, Duration rageQuitEthWithdrawalsMaxDelay @@ -26,7 +27,7 @@ library DualGovernanceConfig { /// @notice Configuration values for Dual Governance. /// @param firstSealRageQuitSupport The percentage of the total stETH supply that must be reached in the Signalling - /// Escrow to transition Dual Governance from the Normal state to the VetoSignalling state. + /// Escrow to transition Dual Governance from Normal, VetoCooldown and RageQuit states to the VetoSignalling state. /// @param secondSealRageQuitSupport The percentage of the total stETH supply that must be reached in the /// Signalling Escrow to transition Dual Governance into the RageQuit state. /// @@ -64,6 +65,12 @@ library DualGovernanceConfig { Duration rageQuitEthWithdrawalsDelayGrowth; } + // --- + // Constants + // --- + + uint256 internal constant MAX_SECOND_SEAL_RAGE_QUIT_SUPPORT = HUNDRED_PERCENT_D16; + // --- // Main Functionality // --- @@ -72,6 +79,10 @@ library DualGovernanceConfig { /// of the Dual Governance system. /// @param self The configuration context. function validate(Context memory self) internal pure { + if (self.secondSealRageQuitSupport > PercentsD16.from(MAX_SECOND_SEAL_RAGE_QUIT_SUPPORT)) { + revert InvalidSecondSealRageQuitSupport(self.secondSealRageQuitSupport); + } + if (self.firstSealRageQuitSupport >= self.secondSealRageQuitSupport) { revert InvalidRageQuitSupportRange(self.firstSealRageQuitSupport, self.secondSealRageQuitSupport); } @@ -203,11 +214,11 @@ library DualGovernanceConfig { Context memory self, uint256 rageQuitRound ) internal pure returns (Duration) { - return Durations.min( - self.rageQuitEthWithdrawalsMinDelay.plusSeconds( - rageQuitRound * self.rageQuitEthWithdrawalsDelayGrowth.toSeconds() - ), - self.rageQuitEthWithdrawalsMaxDelay - ); + uint256 rageQuitWithdrawalsDelayInSeconds = self.rageQuitEthWithdrawalsMinDelay.toSeconds() + + rageQuitRound * self.rageQuitEthWithdrawalsDelayGrowth.toSeconds(); + + return rageQuitWithdrawalsDelayInSeconds > self.rageQuitEthWithdrawalsMaxDelay.toSeconds() + ? self.rageQuitEthWithdrawalsMaxDelay + : Durations.from(rageQuitWithdrawalsDelayInSeconds); } } diff --git a/contracts/libraries/EscrowState.sol b/contracts/libraries/EscrowState.sol index c3b4707e..482d0a78 100644 --- a/contracts/libraries/EscrowState.sol +++ b/contracts/libraries/EscrowState.sol @@ -71,11 +71,16 @@ library EscrowState { /// @notice Initializes the Escrow state to SignallingEscrow. /// @param self The context of the Escrow State library. - /// @param minAssetsLockDuration The minimum assets lock duration. - function initialize(Context storage self, Duration minAssetsLockDuration) internal { + /// @param minAssetsLockDuration The initial minimum assets lock duration. + /// @param maxMinAssetsLockDuration Sanity check upper bound for min assets lock duration. + function initialize( + Context storage self, + Duration minAssetsLockDuration, + Duration maxMinAssetsLockDuration + ) internal { _checkState(self, State.NotInitialized); _setState(self, State.SignallingEscrow); - _setMinAssetsLockDuration(self, minAssetsLockDuration); + setMinAssetsLockDuration(self, minAssetsLockDuration, maxMinAssetsLockDuration); } /// @notice Starts the rage quit process. @@ -116,7 +121,8 @@ library EscrowState { ) { revert InvalidMinAssetsLockDuration(newMinAssetsLockDuration); } - _setMinAssetsLockDuration(self, newMinAssetsLockDuration); + self.minAssetsLockDuration = newMinAssetsLockDuration; + emit MinAssetsLockDurationSet(newMinAssetsLockDuration); } // --- @@ -205,12 +211,4 @@ library EscrowState { self.state = newState; emit EscrowStateChanged(prevState, newState); } - - /// @notice Sets the minimum assets lock duration. - /// @param self The context of the Escrow State library. - /// @param newMinAssetsLockDuration The new minimum assets lock duration. - function _setMinAssetsLockDuration(Context storage self, Duration newMinAssetsLockDuration) private { - self.minAssetsLockDuration = newMinAssetsLockDuration; - emit MinAssetsLockDurationSet(newMinAssetsLockDuration); - } } diff --git a/contracts/libraries/ExecutableProposals.sol b/contracts/libraries/ExecutableProposals.sol index f34ba52c..61d6d10f 100644 --- a/contracts/libraries/ExecutableProposals.sol +++ b/contracts/libraries/ExecutableProposals.sol @@ -67,7 +67,7 @@ library ExecutableProposals { /// @notice The context for the library, storing relevant proposals data. /// @param proposalsCount The total number of proposals submitted so far. - /// @param lastCancelledProposalId The id of the most recently canceled proposal. + /// @param lastCancelledProposalId The id of the most recently cancelled proposal. /// @param proposals A mapping of proposal ids to their corresponding `Proposal` data. struct Context { uint64 proposalsCount; @@ -83,6 +83,7 @@ library ExecutableProposals { error UnexpectedProposalStatus(uint256 proposalId, Status status); error AfterSubmitDelayNotPassed(uint256 proposalId); error AfterScheduleDelayNotPassed(uint256 proposalId); + error MinExecutionDelayNotPassed(uint256 proposalId); // --- // Events @@ -128,7 +129,7 @@ library ExecutableProposals { } /// @notice Marks a previously submitted proposal as scheduled for execution if the required delay period - /// has passed since submission and the proposal was not canceled. + /// has passed since submission and the proposal was not cancelled. /// @param self The context of the Executable Proposal library. /// @param proposalId The id of the proposal to schedule. /// @param afterSubmitDelay The required delay duration after submission before the proposal can be scheduled. @@ -154,11 +155,17 @@ library ExecutableProposals { } /// @notice Marks a previously scheduled proposal as executed and runs the associated external calls if the - /// required delay period has passed since scheduling and the proposal has not been canceled. + /// required delay period has passed since scheduling and the proposal has not been cancelled. /// @param self The context of the Executable Proposal library. /// @param proposalId The id of the proposal to execute. /// @param afterScheduleDelay The minimum delay required after scheduling before execution is allowed. - function execute(Context storage self, uint256 proposalId, Duration afterScheduleDelay) internal { + /// @param minExecutionDelay The minimum time that must elapse after submission before execution is allowed. + function execute( + Context storage self, + uint256 proposalId, + Duration afterScheduleDelay, + Duration minExecutionDelay + ) internal { Proposal memory proposal = self.proposals[proposalId]; _checkProposalNotCancelled(self, proposalId, proposal.data); @@ -171,13 +178,17 @@ library ExecutableProposals { revert AfterScheduleDelayNotPassed(proposalId); } + if (minExecutionDelay.addTo(proposal.data.submittedAt) > Timestamps.now()) { + revert MinExecutionDelayNotPassed(proposalId); + } + self.proposals[proposalId].data.status = Status.Executed; ExternalCalls.execute(IExternalExecutor(proposal.data.executor), proposal.calls); emit ProposalExecuted(proposalId); } - /// @notice Marks all non-executed proposals up to the most recently submitted as canceled, preventing their execution. + /// @notice Marks all non-executed proposals up to the most recently submitted as cancelled, preventing their execution. /// @param self The context of the Executable Proposal library. function cancelAll(Context storage self) internal { uint64 lastCancelledProposalId = self.proposalsCount; diff --git a/contracts/libraries/Proposers.sol b/contracts/libraries/Proposers.sol index 07ae5278..27ad5cac 100644 --- a/contracts/libraries/Proposers.sol +++ b/contracts/libraries/Proposers.sol @@ -163,7 +163,7 @@ library Proposers { } } - /// @notice Checks if an `proposerAccount` is a registered proposer. + /// @notice Checks if a `proposerAccount` is a registered proposer. /// @param self The context of the Proposers library. /// @param proposerAccount The address to check. /// @return bool `true` if the `proposerAccount` is a registered proposer, otherwise `false`. diff --git a/contracts/libraries/TimelockState.sol b/contracts/libraries/TimelockState.sol index 9de8fa2e..afb59929 100644 --- a/contracts/libraries/TimelockState.sol +++ b/contracts/libraries/TimelockState.sol @@ -5,7 +5,7 @@ pragma solidity 0.8.26; import {Duration} from "../types/Duration.sol"; /// @title Timelock State Library -/// @dev Library for managing the configuration related the state of timelock contract. +/// @dev Library for managing the configuration related to the state of timelock contract. library TimelockState { // --- // Errors diff --git a/contracts/types/Duration.sol b/contracts/types/Duration.sol index 1048f93d..516973ae 100644 --- a/contracts/types/Duration.sol +++ b/contracts/types/Duration.sol @@ -153,8 +153,4 @@ library Durations { /// to `MAX_DURATION_VALUE`, which fits within the `uint32`. res = Duration.wrap(uint32(durationInSeconds)); } - - function min(Duration d1, Duration d2) internal pure returns (Duration res) { - res = d1 < d2 ? d1 : d2; - } } diff --git a/docs/mechanism.md b/docs/mechanism.md index acdefc7c..a5ea2ecd 100644 --- a/docs/mechanism.md +++ b/docs/mechanism.md @@ -214,7 +214,7 @@ SecondSealRageQuitSupport = 0.1 #### Deactivation sub-state -The sub-state's purpose is to allow all stakers to observe the Veto Signalling being deactivated and react accordingly before non-canceled proposals can be executed. In this sub-state, the DAO cannot submit proposals to the DG or execute pending proposals. +The sub-state's purpose is to allow all stakers to observe the Veto Signalling being deactivated and react accordingly before non-cancelled proposals can be executed. In this sub-state, the DAO cannot submit proposals to the DG or execute pending proposals. **Transition to the parent state**. If, while the sub-state is active, the following condition becomes true: @@ -245,7 +245,7 @@ VetoSignallingDeactivationMaxDuration = 3 days ### Veto Cooldown state -In the Veto Cooldown state, the DAO cannot submit proposals to the DG but can execute pending non-canceled proposals, provided that the proposal being executed was submitted more than `ProposalExecutionMinTimelock` days ago and before the Veto Signalling state was entered the last time. This state exists to guarantee that no staker possessing enough stETH to generate `FirstSealRageQuitSupport` can lock the governance indefinitely without rage quitting the protocol. +In the Veto Cooldown state, the DAO cannot submit proposals to the DG but can execute pending non-cancelled proposals, provided that the proposal being executed was submitted more than `ProposalExecutionMinTimelock` days ago and before the Veto Signalling state was entered the last time. This state exists to guarantee that no staker possessing enough stETH to generate `FirstSealRageQuitSupport` can lock the governance indefinitely without rage quitting the protocol. **Transition to Veto Signalling**. If, while the state is active, the following condition becomes true: diff --git a/docs/plan-b.md b/docs/plan-b.md index e475c5d9..a5c97348 100644 --- a/docs/plan-b.md +++ b/docs/plan-b.md @@ -133,7 +133,7 @@ function cancelAllPendingProposals() returns (bool) Cancels all currently submitted and non-executed proposals. If a proposal was submitted but not scheduled, it becomes unschedulable. If a proposal was scheduled, it becomes unexecutable. -The function will return `true` if all proposals are successfully canceled. If the subsequent call to the [`EmergencyProtectedTimelock.cancelAllNonExecutedProposals`](#Function-EmergencyProtectedTimelockcancelAllNonExecutedProposals) method fails, the function will revert with an error. +The function will return `true` if all proposals are successfully cancelled. If the subsequent call to the [`EmergencyProtectedTimelock.cancelAllNonExecutedProposals`](#Function-EmergencyProtectedTimelockcancelAllNonExecutedProposals) method fails, the function will revert with an error. #### Preconditions @@ -160,6 +160,56 @@ While active, the Emergency Activation Committee can enable Emergency Mode. This --- +### Function: `EmergencyProtectedTimelock.MIN_EXECUTION_DELAY` + +```solidity +Duration public immutable MIN_EXECUTION_DELAY; +``` + +The minimum duration that must pass between a proposal's submission and its execution. + +--- + +### Function: `EmergencyProtectedTimelock.MAX_AFTER_SUBMIT_DELAY` + +```solidity +Duration public immutable MAX_AFTER_SUBMIT_DELAY; +``` + +The upper bound for the delay required before a submitted proposal can be scheduled for execution. + +--- + +### Function: `EmergencyProtectedTimelock.MAX_AFTER_SCHEDULE_DELAY` + +```solidity +Duration public immutable MAX_AFTER_SCHEDULE_DELAY; +``` + +The upper bound for the delay required before a scheduled proposal can be executed. + +--- + +### Function: `EmergencyProtectedTimelock.MAX_EMERGENCY_MODE_DURATION` + +```solidity +Duration public immutable MAX_EMERGENCY_MODE_DURATION; +``` + +The upper bound for the time the timelock can remain in emergency mode. + +--- + +### Function: `EmergencyProtectedTimelock.MAX_EMERGENCY_PROTECTION_DURATION` + +```solidity +Duration public immutable MAX_EMERGENCY_PROTECTION_DURATION; +``` + +The upper bound for the time the emergency protection mechanism can be activated. + +--- + ### Function: `EmergencyProtectedTimelock.submit` ```solidity @@ -192,6 +242,7 @@ Schedules a previously submitted and non-cancelled proposal for execution after - MUST be called by the `governance` address. - The proposal MUST already be submitted. +- `EmergencyProtectedTimelock.MIN_EXECUTION_DELAY` MUST have elapsed since the proposal’s submission. - The post-submit timelock MUST have elapsed since the proposal submission. --- diff --git a/docs/specification.md b/docs/specification.md index 610ecc38..2c4a2cd1 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -66,8 +66,8 @@ The general proposal flow is the following: 1. A proposer submits a proposal, i.e. a set of EVM calls (represented by an array of [`ExternalCall`](#Struct-ExternalCall) structs) to be issued by the proposer's associated [executor contract](#Contract-Executor), by calling the [`DualGovernance.submitProposal`](#Function-DualGovernancesubmitProposal) function. 2. This starts a [dynamic timelock period](#Dynamic-timelock) that allows stakers to oppose the DAO, potentially leaving the protocol before the timelock elapses. -3. By the end of the dynamic timelock period, the proposal is either canceled by the DAO or executable. - * If it's canceled, it cannot be scheduled for execution. However, any proposer is free to submit a new proposal with the same set of calls. +3. By the end of the dynamic timelock period, the proposal is either cancelled by the DAO or executable. + * If it's cancelled, it cannot be scheduled for execution. However, any proposer is free to submit a new proposal with the same set of calls. * Otherwise, anyone can schedule the proposal for execution by calling the [`DualGovernance.scheduleProposal`](#Function-DualGovernancescheduleProposal) function, with the execution flow that follows being dependent on the [deployment mode](#Proposal-execution-and-deployment-modes). 4. The proposal's execution results in the proposal's EVM calls being issued by the executor contract associated with the proposer. @@ -85,8 +85,8 @@ While the Dual Governance is in the `VetoSignalling` or `VetoSignallingDeactivat By the time the dynamic timelock described above elapses, one of the following outcomes is possible: - The DAO was not opposed by stakers (the **happy path** scenario). -- The DAO was opposed by stakers and canceled all pending proposals (the **two-sided de-escalation** scenario). -- The DAO was opposed by stakers and didn't cancel pending proposals, forcing the stakers to leave via the rage quit process, or canceled the proposals but some stakers still left (the **rage quit** scenario). +- The DAO was opposed by stakers and cancelled all pending proposals (the **two-sided de-escalation** scenario). +- The DAO was opposed by stakers and didn't cancel pending proposals, forcing the stakers to leave via the rage quit process, or cancelled the proposals but some stakers still left (the **rage quit** scenario). - The DAO was opposed by stakers and didn't cancel pending proposals but the total stake opposing the DAO was too small to trigger the rage quit (the **failed escalation** scenario). @@ -295,7 +295,7 @@ function cancelAllPendingProposals() returns (bool) Cancels all currently submitted and non-executed proposals. If a proposal was submitted but not scheduled, it becomes unschedulable. If a proposal was scheduled, it becomes unexecutable. If the current governance state is neither `VetoSignalling` nor `VetoSignallingDeactivation`, the function will exit early without canceling any proposals, emitting the `CancelAllPendingProposalsSkipped` event and returning `false`. -If proposals are successfully canceled, the `CancelAllPendingProposalsExecuted` event will be emitted, and the function will return `true`. +If proposals are successfully cancelled, the `CancelAllPendingProposalsExecuted` event will be emitted, and the function will return `true`. #### Preconditions @@ -329,6 +329,7 @@ Sets the configuration provider for the Dual Governance system. - The `newConfigProvider` address MUST NOT be the same as the current configuration provider. - The values returned by the config MUST be valid: - `firstSealRageQuitSupport` MUST be less than `secondSealRageQuitSupport`. + - `secondSealRageQuitSupport` MUST be less than or equal to 100%, represented as a percentage with 16 decimal places of precision. - `vetoSignallingMinDuration` MUST be less than `vetoSignallingMaxDuration`. - `rageQuitEthWithdrawalsMinDelay` MUST be less than or equal to `rageQuitEthWithdrawalsMaxDelay`. - `minAssetsLockDuration` MUST NOT be zero. @@ -849,6 +850,8 @@ The `Escrow` instance is intended to be used behind [minimal proxy contracts](ht - MUST be called using the proxy contract. - MUST be called by the `DualGovernance` contract. - MUST NOT have been initialized previously. +- `minAssetsLockDuration` MUST NOT exceed `Escrow.MAX_MIN_ASSETS_LOCK_DURATION`. +- `minAssetsLockDuration` MUST NOT be equal to previous value (by default `0`). --- @@ -1479,6 +1482,56 @@ The governance reset entails the following steps: --- +### Function: `EmergencyProtectedTimelock.MIN_EXECUTION_DELAY` + +```solidity +Duration public immutable MIN_EXECUTION_DELAY; +``` + +The minimum duration that must pass between a proposal's submission and its execution. + +--- + +### Function: `EmergencyProtectedTimelock.MAX_AFTER_SUBMIT_DELAY` + +```solidity +Duration public immutable MAX_AFTER_SUBMIT_DELAY; +``` + +The upper bound for the delay required before a submitted proposal can be scheduled for execution. + +--- + +### Function: `EmergencyProtectedTimelock.MAX_AFTER_SCHEDULE_DELAY` + +```solidity +Duration public immutable MAX_AFTER_SCHEDULE_DELAY; +``` + +The upper bound for the delay required before a scheduled proposal can be executed. + +--- + +### Function: `EmergencyProtectedTimelock.MAX_EMERGENCY_MODE_DURATION` + +```solidity +Duration public immutable MAX_EMERGENCY_MODE_DURATION; +``` + +The upper bound for the time the timelock can remain in emergency mode. + +--- + +### Function: `EmergencyProtectedTimelock.MAX_EMERGENCY_PROTECTION_DURATION` + +```solidity +Duration public immutable MAX_EMERGENCY_PROTECTION_DURATION; +``` + +The upper bound for the time the emergency protection mechanism can be activated. + +--- + ### Function: `EmergencyProtectedTimelock.submit` ```solidity @@ -1527,6 +1580,7 @@ Instructs the executor contract associated with the proposal to issue the propos - Emergency mode MUST NOT be active. - The proposal MUST be already submitted & scheduled for execution. +- `EmergencyProtectedTimelock.MIN_EXECUTION_DELAY` MUST have elapsed since the proposal’s submission. - The emergency protection delay MUST already elapse since the moment the proposal was scheduled. --- @@ -2176,6 +2230,16 @@ Executes a scheduled proposal by calling the `tiebreakerScheduleProposal` functi --- +### Function: `TiebreakerCoreCommittee.checkProposalExists` + +```solidity +function checkProposalExists(uint256 proposalId) public view +``` + +Checks if the specified proposal exists, otherwise reverts. + +--- + ### Function: `TiebreakerCoreCommittee.getSealableResumeNonce` ```solidity @@ -2197,6 +2261,7 @@ Submits a request to resume operations of a sealable contract by voting on it an #### Preconditions - MUST be called by a member. +- The `sealable` address MUST be in `Paused` state. - The provided nonce MUST match the current nonce of the sealable contract. --- @@ -2209,7 +2274,7 @@ function getSealableResumeState(address sealable, uint256 nonce) returns (uint256 support, uint256 executionQuorum, bool isExecuted) ``` -Returns the state of a sealable resume request including support count, quorum, and execution status. +Returns the state of sealable resume request including support count, quorum, and execution status. --- @@ -2227,6 +2292,15 @@ Executes a sealable resume request by calling the `tiebreakerResumeSealable` fun --- +### Function: `TiebreakerCoreCommittee.checkSealableIsPaused` + +```solidity +function checkSealableIsPaused(address sealable) public view +``` + +Checks if the specified sealable address is not a zero address and that it is paused, otherwise reverts. +--- + ## Contract: `TiebreakerSubCommittee` `TiebreakerSubCommittee` is a smart contract that extends the functionalities of `HashConsensus` and `ProposalsList` to manage the scheduling of proposals and the resumption of sealable contracts through a consensus mechanism. It interacts with the `TiebreakerCoreCommittee` contract to execute decisions once consensus is reached. @@ -2284,6 +2358,7 @@ Submits a request to resume operations of a sealable contract by voting on it an #### Preconditions - MUST be called by a member. +- The `sealable` address MUST be in `Paused` state. ### Function: `TiebreakerSubCommittee.getSealableResumeState` @@ -2293,7 +2368,7 @@ function getSealableResumeState(address sealable) returns (uint256 support, uint256 executionQuorum, bool isExecuted) ``` -Returns the state of a sealable resume request including support count, quorum, and execution status. +Returns the state of current sealable resume request including support count, quorum, and execution status. --- diff --git a/test/unit/Escrow.t.sol b/test/unit/Escrow.t.sol index 4a16ea36..93567c57 100644 --- a/test/unit/Escrow.t.sol +++ b/test/unit/Escrow.t.sol @@ -135,20 +135,23 @@ contract EscrowUnitTests is UnitTest { // initialize() // --- - function test_initialize_HappyPath() external { + function testFuzz_initialize_HappyPath(Duration minAssetLockDuration) external { + vm.assume(minAssetLockDuration > Durations.ZERO); + vm.assume(minAssetLockDuration <= _maxMinAssetsLockDuration); + vm.expectEmit(); emit EscrowStateLib.EscrowStateChanged(EscrowState.NotInitialized, EscrowState.SignallingEscrow); vm.expectEmit(); - emit EscrowStateLib.MinAssetsLockDurationSet(Durations.ZERO); + emit EscrowStateLib.MinAssetsLockDurationSet(minAssetLockDuration); vm.expectCall(address(_stETH), abi.encodeCall(IERC20.approve, (address(_wstETH), type(uint256).max))); vm.expectCall(address(_stETH), abi.encodeCall(IERC20.approve, (address(_withdrawalQueue), type(uint256).max))); Escrow escrowInstance = - _createInitializedEscrowProxy({minWithdrawalsBatchSize: 100, minAssetsLockDuration: Durations.ZERO}); + _createInitializedEscrowProxy({minWithdrawalsBatchSize: 100, minAssetsLockDuration: minAssetLockDuration}); assertEq(escrowInstance.MIN_WITHDRAWALS_BATCH_SIZE(), 100); - assertEq(escrowInstance.getMinAssetsLockDuration(), Durations.ZERO); + assertEq(escrowInstance.getMinAssetsLockDuration(), minAssetLockDuration); assertTrue(escrowInstance.getEscrowState() == EscrowState.SignallingEscrow); IEscrow.SignallingEscrowDetails memory signallingEscrowDetails = escrowInstance.getSignallingEscrowDetails(); @@ -739,8 +742,11 @@ contract EscrowUnitTests is UnitTest { function test_markUnstETHFinalized_RevertOn_UnexpectedEscrowState() external { _transitToRageQuit(); - uint256[] memory unstethIds = new uint256[](0); - uint256[] memory hints = new uint256[](0); + uint256[] memory unstethIds = new uint256[](1); + uint256[] memory hints = new uint256[](1); + + unstethIds[0] = 1; + hints[0] = 1; vm.expectRevert( abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.RageQuitEscrow) @@ -748,6 +754,14 @@ contract EscrowUnitTests is UnitTest { _escrow.markUnstETHFinalized(unstethIds, hints); } + function test_markUnstETHFinalized_RevertOn_EmptyUnstETHIds() external { + uint256[] memory unstethIds = new uint256[](0); + uint256[] memory hints = new uint256[](0); + + vm.expectRevert(abi.encodeWithSelector(Escrow.EmptyUnstETHIds.selector)); + _escrow.markUnstETHFinalized(unstethIds, hints); + } + // --- // startRageQuit() // --- @@ -1190,6 +1204,18 @@ contract EscrowUnitTests is UnitTest { _escrow.setMinAssetsLockDuration(newMinAssetsLockDuration); } + function test_setMinAssetsLockDuration_RevertOn_RageQuitState() external { + Duration newMinAssetsLockDuration = Durations.from(1); + + _transitToRageQuit(); + + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.RageQuitEscrow) + ); + vm.prank(_dualGovernance); + _escrow.setMinAssetsLockDuration(newMinAssetsLockDuration); + } + // --- // withdrawETH() // --- diff --git a/test/unit/committees/TiebreakerCore.t.sol b/test/unit/committees/TiebreakerCore.t.sol index 27de2186..561e1e78 100644 --- a/test/unit/committees/TiebreakerCore.t.sol +++ b/test/unit/committees/TiebreakerCore.t.sol @@ -8,6 +8,7 @@ import {Timestamp} from "contracts/types/Timestamp.sol"; import {ITimelock} from "contracts/interfaces/ITimelock.sol"; import {ITiebreaker} from "contracts/interfaces/ITiebreaker.sol"; +import {ISealable} from "contracts/interfaces/ISealable.sol"; import {TargetMock} from "test/utils/target-mock.sol"; import {UnitTest} from "test/utils/unit-test.sol"; @@ -125,6 +126,8 @@ contract TiebreakerCoreUnitTest is UnitTest { function test_sealableResume_HappyPath() external { uint256 nonce = tiebreakerCore.getSealableResumeNonce(sealable); + _mockSealableResumeSinceTimestamp(sealable, block.timestamp + 1000); + vm.prank(committeeMembers[0]); tiebreakerCore.sealableResume(sealable, nonce); @@ -142,9 +145,28 @@ contract TiebreakerCoreUnitTest is UnitTest { assertFalse(isExecuted); } + function test_sealableResume_RevertOn_Sealable_Is_Resumed() external { + uint256 nonce = tiebreakerCore.getSealableResumeNonce(sealable); + _mockSealableResumeSinceTimestamp(sealable, block.timestamp); + + vm.prank(committeeMembers[0]); + vm.expectRevert(abi.encodeWithSelector(TiebreakerCoreCommittee.SealableIsNotPaused.selector, sealable)); + tiebreakerCore.sealableResume(sealable, nonce); + } + + function test_sealableResume_RevertOn_Sealable_Is_Resumed_For_1_Second() external { + uint256 nonce = tiebreakerCore.getSealableResumeNonce(sealable); + _mockSealableResumeSinceTimestamp(sealable, block.timestamp - 1); + + vm.prank(committeeMembers[0]); + vm.expectRevert(abi.encodeWithSelector(TiebreakerCoreCommittee.SealableIsNotPaused.selector, sealable)); + tiebreakerCore.sealableResume(sealable, nonce); + } + function test_sealableResume_RevertOn_NonceMismatch() external { uint256 wrongNonce = 999; + _mockSealableResumeSinceTimestamp(sealable, block.timestamp + 1000); vm.prank(committeeMembers[0]); vm.expectRevert(abi.encodeWithSelector(TiebreakerCoreCommittee.ResumeSealableNonceMismatch.selector)); tiebreakerCore.sealableResume(sealable, wrongNonce); @@ -159,6 +181,8 @@ contract TiebreakerCoreUnitTest is UnitTest { function test_executeSealableResume_HappyPath() external { uint256 nonce = tiebreakerCore.getSealableResumeNonce(sealable); + _mockSealableResumeSinceTimestamp(sealable, block.timestamp + 1000); + vm.prank(committeeMembers[0]); tiebreakerCore.sealableResume(sealable, nonce); vm.prank(committeeMembers[1]); @@ -215,4 +239,12 @@ contract TiebreakerCoreUnitTest is UnitTest { assertEq(quorumAt, quorumAtExpected); assertTrue(isExecuted); } + + function _mockSealableResumeSinceTimestamp(address sealableAddress, uint256 resumeSinceTimestamp) internal { + vm.mockCall( + sealableAddress, + abi.encodeWithSelector(ISealable.getResumeSinceTimestamp.selector), + abi.encode(resumeSinceTimestamp) + ); + } } diff --git a/test/unit/committees/TiebreakerSubCommittee.t.sol b/test/unit/committees/TiebreakerSubCommittee.t.sol index 9412fe14..f1e2c8ad 100644 --- a/test/unit/committees/TiebreakerSubCommittee.t.sol +++ b/test/unit/committees/TiebreakerSubCommittee.t.sol @@ -2,17 +2,20 @@ pragma solidity 0.8.26; import {ITiebreakerCoreCommittee} from "contracts/interfaces/ITiebreakerCoreCommittee.sol"; +import {ISealable} from "contracts/interfaces/ISealable.sol"; import {TiebreakerCoreCommittee} from "contracts/committees/TiebreakerCoreCommittee.sol"; import {TiebreakerSubCommittee, ProposalType} from "contracts/committees/TiebreakerSubCommittee.sol"; import {HashConsensus} from "contracts/committees/HashConsensus.sol"; import {Timestamp} from "contracts/types/Timestamp.sol"; -import {UnitTest} from "test/utils/unit-test.sol"; +import {UnitTest} from "test/utils/unit-test.sol"; import {TargetMock} from "test/utils/target-mock.sol"; contract TiebreakerCoreMock is TargetMock { error ProposalDoesNotExist(uint256 proposalId); + error SealableIsNotPaused(address sealable); + error InvalidSealable(address sealable); uint256 public proposalsCount; @@ -25,6 +28,15 @@ contract TiebreakerCoreMock is TargetMock { function setProposalsCount(uint256 _proposalsCount) external { proposalsCount = _proposalsCount; } + + function checkSealableIsPaused(address sealable) public view { + if (sealable == address(0)) { + revert InvalidSealable(sealable); + } + if (ISealable(sealable).getResumeSinceTimestamp() <= block.timestamp) { + revert SealableIsNotPaused(sealable); + } + } } contract TiebreakerSubCommitteeUnitTest is UnitTest { @@ -120,6 +132,7 @@ contract TiebreakerSubCommitteeUnitTest is UnitTest { abi.encodeWithSelector(ITiebreakerCoreCommittee.getSealableResumeNonce.selector, sealable), abi.encode(0) ); + _mockSealableResumeSinceTimestamp(sealable, block.timestamp + 1000); vm.prank(committeeMembers[0]); tiebreakerSubCommittee.sealableResume(sealable); @@ -138,6 +151,32 @@ contract TiebreakerSubCommitteeUnitTest is UnitTest { assertFalse(isExecuted); } + function test_sealableResume_RevertOn_Sealable_Is_Resumed() external { + vm.mockCall( + tiebreakerCore, + abi.encodeWithSelector(ITiebreakerCoreCommittee.getSealableResumeNonce.selector, sealable), + abi.encode(0) + ); + _mockSealableResumeSinceTimestamp(sealable, block.timestamp); + + vm.prank(committeeMembers[0]); + vm.expectRevert(abi.encodeWithSelector(TiebreakerCoreCommittee.SealableIsNotPaused.selector, sealable)); + tiebreakerSubCommittee.sealableResume(sealable); + } + + function test_sealableResume_RevertOn_Sealable_Is_Resumed_For_1_Second() external { + vm.mockCall( + tiebreakerCore, + abi.encodeWithSelector(ITiebreakerCoreCommittee.getSealableResumeNonce.selector, sealable), + abi.encode(0) + ); + _mockSealableResumeSinceTimestamp(sealable, block.timestamp - 1); + + vm.prank(committeeMembers[0]); + vm.expectRevert(abi.encodeWithSelector(TiebreakerCoreCommittee.SealableIsNotPaused.selector, sealable)); + tiebreakerSubCommittee.sealableResume(sealable); + } + function testFuzz_sealableResume_RevertOn_NotMember(address caller) external { vm.assume(caller != committeeMembers[0] && caller != committeeMembers[1] && caller != committeeMembers[2]); @@ -148,7 +187,7 @@ contract TiebreakerSubCommitteeUnitTest is UnitTest { function test_sealableResume_RevertOn_SealableZeroAddress() external { vm.prank(committeeMembers[0]); - vm.expectRevert(abi.encodeWithSelector(TiebreakerSubCommittee.InvalidSealable.selector, address(0))); + vm.expectRevert(abi.encodeWithSelector(TiebreakerCoreCommittee.InvalidSealable.selector, address(0))); tiebreakerSubCommittee.sealableResume(address(0)); } @@ -158,6 +197,7 @@ contract TiebreakerSubCommitteeUnitTest is UnitTest { abi.encodeWithSelector(ITiebreakerCoreCommittee.getSealableResumeNonce.selector, sealable), abi.encode(0) ); + _mockSealableResumeSinceTimestamp(sealable, block.timestamp + 1000); vm.prank(committeeMembers[0]); tiebreakerSubCommittee.sealableResume(sealable); @@ -180,6 +220,7 @@ contract TiebreakerSubCommitteeUnitTest is UnitTest { abi.encodeWithSelector(ITiebreakerCoreCommittee.getSealableResumeNonce.selector, sealable), abi.encode(0) ); + _mockSealableResumeSinceTimestamp(sealable, block.timestamp + 1000); vm.prank(committeeMembers[0]); tiebreakerSubCommittee.sealableResume(sealable); @@ -236,6 +277,7 @@ contract TiebreakerSubCommitteeUnitTest is UnitTest { abi.encodeWithSelector(ITiebreakerCoreCommittee.getSealableResumeNonce.selector, sealable), abi.encode(0) ); + _mockSealableResumeSinceTimestamp(sealable, block.timestamp + 1000); (uint256 support, uint256 executionQuorum, Timestamp quorumAt, bool isExecuted) = tiebreakerSubCommittee.getSealableResumeState(sealable); @@ -271,4 +313,12 @@ contract TiebreakerSubCommitteeUnitTest is UnitTest { assertEq(quorumAt, Timestamp.wrap(uint40(block.timestamp))); assertTrue(isExecuted); } + + function _mockSealableResumeSinceTimestamp(address sealableAddress, uint256 resumeSinceTimestamp) internal { + vm.mockCall( + sealableAddress, + abi.encodeWithSelector(ISealable.getResumeSinceTimestamp.selector), + abi.encode(resumeSinceTimestamp) + ); + } } diff --git a/test/unit/libraries/DualGovernanceConfig.t.sol b/test/unit/libraries/DualGovernanceConfig.t.sol index ec9a6c68..623a0889 100644 --- a/test/unit/libraries/DualGovernanceConfig.t.sol +++ b/test/unit/libraries/DualGovernanceConfig.t.sol @@ -11,9 +11,8 @@ import {UnitTest} from "test/utils/unit-test.sol"; contract DualGovernanceConfigTest is UnitTest { using DualGovernanceConfig for DualGovernanceConfig.Context; - // Unrealistically huge value for the percents value, to limit max fuzz value to avoid overflow - // due to very high values - PercentD16 internal immutable _SECOND_SEAL_RAGE_QUIT_SUPPORT_LIMIT = PercentsD16.fromBasisPoints(1_000_000_00); + PercentD16 internal immutable _MAX_SECOND_SEAL_RAGE_QUIT_SUPPORT = + PercentsD16.from(DualGovernanceConfig.MAX_SECOND_SEAL_RAGE_QUIT_SUPPORT); Timestamp internal immutable _MAX_TIMESTAMP = Timestamps.from(block.timestamp + 100 * 365 days); // The actual max value will not exceed 255, but for testing is used higher upper bound @@ -40,12 +39,30 @@ contract DualGovernanceConfigTest is UnitTest { function testFuzz_validate_HappyPath(DualGovernanceConfig.Context memory config) external { _assumeConfigParams(config); - config.validate(); + this.external__validate(config); + } + + function testFuzz_validate_RevertOn_InvalidSecondSealRageQuitSupport(DualGovernanceConfig.Context memory config) + external + { + vm.assume(config.secondSealRageQuitSupport > _MAX_SECOND_SEAL_RAGE_QUIT_SUPPORT); + vm.assume(config.firstSealRageQuitSupport < config.secondSealRageQuitSupport); + vm.assume(config.vetoSignallingMinDuration < config.vetoSignallingMaxDuration); + vm.assume(config.rageQuitEthWithdrawalsMinDelay <= config.rageQuitEthWithdrawalsMaxDelay); + vm.assume(config.minAssetsLockDuration > Durations.ZERO); + + vm.expectRevert( + abi.encodeWithSelector( + DualGovernanceConfig.InvalidSecondSealRageQuitSupport.selector, config.secondSealRageQuitSupport + ) + ); + this.external__validate(config); } function testFuzz_validate_RevertOn_InvalidSealRageQuitSupportRange(DualGovernanceConfig.Context memory config) external { + vm.assume(config.secondSealRageQuitSupport <= _MAX_SECOND_SEAL_RAGE_QUIT_SUPPORT); vm.assume(config.firstSealRageQuitSupport >= config.secondSealRageQuitSupport); vm.assume(config.vetoSignallingMinDuration < config.vetoSignallingMaxDuration); vm.assume(config.rageQuitEthWithdrawalsMinDelay <= config.rageQuitEthWithdrawalsMaxDelay); @@ -57,14 +74,14 @@ contract DualGovernanceConfigTest is UnitTest { config.secondSealRageQuitSupport ) ); - config.validate(); + this.external__validate(config); } function testFuzz_validate_RevertOn_InvalidVetoSignallingDurationRange(DualGovernanceConfig.Context memory config) external { vm.assume(config.firstSealRageQuitSupport < config.secondSealRageQuitSupport); - vm.assume(config.secondSealRageQuitSupport < _SECOND_SEAL_RAGE_QUIT_SUPPORT_LIMIT); + vm.assume(config.secondSealRageQuitSupport < _MAX_SECOND_SEAL_RAGE_QUIT_SUPPORT); vm.assume(config.vetoSignallingMinDuration >= config.vetoSignallingMaxDuration); vm.assume(config.rageQuitEthWithdrawalsMinDelay <= config.rageQuitEthWithdrawalsMaxDelay); @@ -75,14 +92,14 @@ contract DualGovernanceConfigTest is UnitTest { config.vetoSignallingMaxDuration ) ); - config.validate(); + this.external__validate(config); } function testFuzz_validate_RevertOn_InvalidRageQuitEthWithdrawalsDelayRange( DualGovernanceConfig.Context memory config ) external { vm.assume(config.firstSealRageQuitSupport < config.secondSealRageQuitSupport); - vm.assume(config.secondSealRageQuitSupport < _SECOND_SEAL_RAGE_QUIT_SUPPORT_LIMIT); + vm.assume(config.secondSealRageQuitSupport < _MAX_SECOND_SEAL_RAGE_QUIT_SUPPORT); vm.assume(config.vetoSignallingMinDuration < config.vetoSignallingMaxDuration); vm.assume(config.rageQuitEthWithdrawalsMinDelay > config.rageQuitEthWithdrawalsMaxDelay); @@ -93,7 +110,7 @@ contract DualGovernanceConfigTest is UnitTest { config.rageQuitEthWithdrawalsMaxDelay ) ); - config.validate(); + this.external__validate(config); } function test_validate_RevertOn_InvalidMinAssetsLockDuration() external { @@ -102,7 +119,7 @@ contract DualGovernanceConfigTest is UnitTest { vm.expectRevert( abi.encodeWithSelector(DualGovernanceConfig.InvalidMinAssetsLockDuration.selector, Durations.ZERO) ); - _dualGovernanceConfig.validate(); + this.external__validate(_dualGovernanceConfig); } // --- @@ -318,7 +335,7 @@ contract DualGovernanceConfigTest is UnitTest { assertEq(config.calcRageQuitWithdrawalsDelay({rageQuitRound: 0}), config.rageQuitEthWithdrawalsMinDelay); } - function test_calcRageQuitWithdrawalsDelay_HappyPath_MaxDelayWhenRageQuitRoundIsZero() external { + function test_calcRageQuitWithdrawalsDelay_HappyPath_MaxDelayWhenRageQuitRoundIsMaxRageQuitRound() external { DualGovernanceConfig.Context memory config; config.rageQuitEthWithdrawalsMinDelay = Durations.from(15 days); @@ -335,22 +352,19 @@ contract DualGovernanceConfigTest is UnitTest { DualGovernanceConfig.Context memory config, uint16 rageQuitRound ) external { - vm.assume(rageQuitRound > 0); + _assumeConfigParams(config); vm.assume(rageQuitRound <= _MAX_RAGE_QUIT_ROUND); - vm.assume( - config.rageQuitEthWithdrawalsMinDelay.toSeconds() - + config.rageQuitEthWithdrawalsDelayGrowth.toSeconds() * (rageQuitRound + 1) <= MAX_DURATION_VALUE - ); - vm.assume(config.rageQuitEthWithdrawalsMinDelay <= config.rageQuitEthWithdrawalsMaxDelay); + uint256 computedRageQuitEthWithdrawalsDelayInSeconds = config.rageQuitEthWithdrawalsMinDelay.toSeconds() + + rageQuitRound * config.rageQuitEthWithdrawalsDelayGrowth.toSeconds(); - Duration computedRageQuitEthWithdrawalsMinDelay = config.rageQuitEthWithdrawalsMinDelay.plusSeconds( - rageQuitRound * config.rageQuitEthWithdrawalsDelayGrowth.toSeconds() - ); - if (computedRageQuitEthWithdrawalsMinDelay > config.rageQuitEthWithdrawalsMaxDelay) { - computedRageQuitEthWithdrawalsMinDelay = config.rageQuitEthWithdrawalsMaxDelay; + if (computedRageQuitEthWithdrawalsDelayInSeconds > config.rageQuitEthWithdrawalsMaxDelay.toSeconds()) { + computedRageQuitEthWithdrawalsDelayInSeconds = config.rageQuitEthWithdrawalsMaxDelay.toSeconds(); } - assertEq(config.calcRageQuitWithdrawalsDelay(rageQuitRound), computedRageQuitEthWithdrawalsMinDelay); + assertEq( + config.calcRageQuitWithdrawalsDelay(rageQuitRound), + Durations.from(computedRageQuitEthWithdrawalsDelayInSeconds) + ); } // --- @@ -360,8 +374,12 @@ contract DualGovernanceConfigTest is UnitTest { function _assumeConfigParams(DualGovernanceConfig.Context memory config) internal view { vm.assume(config.firstSealRageQuitSupport < config.secondSealRageQuitSupport); vm.assume(config.vetoSignallingMinDuration < config.vetoSignallingMaxDuration); - vm.assume(config.secondSealRageQuitSupport < _SECOND_SEAL_RAGE_QUIT_SUPPORT_LIMIT); + vm.assume(config.secondSealRageQuitSupport <= _MAX_SECOND_SEAL_RAGE_QUIT_SUPPORT); vm.assume(config.rageQuitEthWithdrawalsMinDelay <= config.rageQuitEthWithdrawalsMaxDelay); vm.assume(config.minAssetsLockDuration > Durations.ZERO); } + + function external__validate(DualGovernanceConfig.Context memory config) external { + config.validate(); + } } diff --git a/test/unit/libraries/EscrowState.t.sol b/test/unit/libraries/EscrowState.t.sol index be0bb1f4..09b1d6dd 100644 --- a/test/unit/libraries/EscrowState.t.sol +++ b/test/unit/libraries/EscrowState.t.sol @@ -17,14 +17,20 @@ contract EscrowStateUnitTests is UnitTest { // initialize() // --- - function testFuzz_initialize_happyPath(Duration minAssetsLockDuration) external { + function testFuzz_initialize_happyPath( + Duration minAssetsLockDuration, + Duration maxMinAssetsLockDuration + ) external { + vm.assume(minAssetsLockDuration > Durations.ZERO); + vm.assume(minAssetsLockDuration <= maxMinAssetsLockDuration); + _context.state = State.NotInitialized; vm.expectEmit(); emit EscrowState.EscrowStateChanged(State.NotInitialized, State.SignallingEscrow); emit EscrowState.MinAssetsLockDurationSet(minAssetsLockDuration); - EscrowState.initialize(_context, minAssetsLockDuration); + EscrowState.initialize(_context, minAssetsLockDuration, maxMinAssetsLockDuration); checkContext({ state: State.SignallingEscrow, @@ -35,12 +41,34 @@ contract EscrowStateUnitTests is UnitTest { }); } - function testFuzz_initialize_RevertOn_InvalidState(Duration minAssetsLockDuration) external { + function testFuzz_initialize_RevertOn_InvalidState( + Duration minAssetsLockDuration, + Duration maxMinAssetsLockDuration + ) external { + vm.assume(minAssetsLockDuration <= maxMinAssetsLockDuration); _context.state = State.SignallingEscrow; vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.SignallingEscrow)); - EscrowState.initialize(_context, minAssetsLockDuration); + EscrowState.initialize(_context, minAssetsLockDuration, maxMinAssetsLockDuration); + } + + function testFuzz_initalize_RevertOn_InvalidMinAssetLockDuration_ZeroDuration(Duration maxMinAssetsLockDuration) + external + { + vm.expectRevert(abi.encodeWithSelector(EscrowState.InvalidMinAssetsLockDuration.selector, 0)); + EscrowState.initialize(_context, Durations.ZERO, maxMinAssetsLockDuration); + } + + function testFuzz_initalize_RevertOn_InvalidMinAssetLockDuration_ExceedMaxDuration( + Duration minAssetsLockDuration, + Duration maxMinAssetsLockDuration + ) external { + vm.assume(maxMinAssetsLockDuration < minAssetsLockDuration); + vm.expectRevert( + abi.encodeWithSelector(EscrowState.InvalidMinAssetsLockDuration.selector, minAssetsLockDuration) + ); + EscrowState.initialize(_context, minAssetsLockDuration, maxMinAssetsLockDuration); } // --- diff --git a/test/unit/libraries/ExecutableProposals.t.sol b/test/unit/libraries/ExecutableProposals.t.sol index 4613d4ac..55768d13 100644 --- a/test/unit/libraries/ExecutableProposals.t.sol +++ b/test/unit/libraries/ExecutableProposals.t.sol @@ -148,8 +148,8 @@ contract ExecutableProposalsUnitTests is UnitTest { _proposals.schedule(proposalId, Durations.ZERO); } - function testFuzz_execute_proposal(Duration delay) external { - vm.assume(delay > Durations.ZERO && delay.toSeconds() <= MAX_DURATION_VALUE); + function testFuzz_execute_proposal(Duration afterScheduleDelay, Duration minExecutionDelay) external { + vm.assume(afterScheduleDelay > Durations.ZERO); _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock))); uint256 proposalId = _proposals.getProposalsCount(); @@ -163,12 +163,11 @@ contract ExecutableProposalsUnitTests is UnitTest { assertEq(proposal.data.submittedAt, submittedAndScheduledAt); assertEq(proposal.data.scheduledAt, submittedAndScheduledAt); - _wait(delay); + _wait(afterScheduleDelay > minExecutionDelay ? afterScheduleDelay : minExecutionDelay); - // TODO: figure out why event is not emitted - // vm.expectEmit(); - // emit ExecutableProposals.ProposalExecuted(); - _proposals.execute(proposalId, delay); + vm.expectEmit(); + emit ExecutableProposals.ProposalExecuted(proposalId); + this.external__execute(proposalId, afterScheduleDelay, minExecutionDelay); Vm.Log[] memory entries = vm.getRecordedLogs(); assertEq(entries.length, 0); @@ -187,7 +186,7 @@ contract ExecutableProposalsUnitTests is UnitTest { ExecutableProposals.UnexpectedProposalStatus.selector, proposalId, ProposalStatus.NotExist ) ); - _proposals.execute(proposalId, Durations.ZERO); + this.external__execute(proposalId, Durations.ZERO, Durations.ZERO); } function test_cannot_execute_unscheduled_proposal() external { @@ -199,21 +198,21 @@ contract ExecutableProposalsUnitTests is UnitTest { ExecutableProposals.UnexpectedProposalStatus.selector, proposalId, ProposalStatus.Submitted ) ); - _proposals.execute(proposalId, Durations.ZERO); + this.external__execute(proposalId, Durations.ZERO, Durations.ZERO); } function test_cannot_execute_twice() external { _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock))); uint256 proposalId = _proposals.getProposalsCount(); _proposals.schedule(proposalId, Durations.ZERO); - _proposals.execute(proposalId, Durations.ZERO); + this.external__execute(proposalId, Durations.ZERO, Durations.ZERO); vm.expectRevert( abi.encodeWithSelector( ExecutableProposals.UnexpectedProposalStatus.selector, proposalId, ProposalStatus.Executed ) ); - _proposals.execute(proposalId, Durations.ZERO); + this.external__execute(proposalId, Durations.ZERO, Durations.ZERO); } function test_cannot_execute_cancelled_proposal() external { @@ -227,19 +226,31 @@ contract ExecutableProposalsUnitTests is UnitTest { ExecutableProposals.UnexpectedProposalStatus.selector, proposalId, ProposalStatus.Cancelled ) ); - _proposals.execute(proposalId, Durations.ZERO); + this.external__execute(proposalId, Durations.ZERO, Durations.ZERO); } - function testFuzz_cannot_execute_before_delay_passed(Duration delay) external { - vm.assume(delay > Durations.ZERO && delay.toSeconds() <= MAX_DURATION_VALUE); + function testFuzz_cannot_execute_before_after_schedule_delay_passed(Duration afterScheduleDelay) external { + vm.assume(afterScheduleDelay > Durations.ZERO); _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock))); uint256 proposalId = _proposals.getProposalsCount(); _proposals.schedule(proposalId, Durations.ZERO); - _wait(delay.minusSeconds(1 seconds)); + _wait(afterScheduleDelay.minusSeconds(1 seconds)); vm.expectRevert(abi.encodeWithSelector(ExecutableProposals.AfterScheduleDelayNotPassed.selector, proposalId)); - _proposals.execute(proposalId, delay); + this.external__execute(proposalId, afterScheduleDelay, Durations.ZERO); + } + + function testFuzz_cannot_execute_before_min_execution_delay_passed(Duration minExecutionDelay) external { + vm.assume(minExecutionDelay > Durations.ZERO); + _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock))); + uint256 proposalId = _proposals.getProposalsCount(); + _proposals.schedule(proposalId, Durations.ZERO); + + _wait(minExecutionDelay.minusSeconds(1 seconds)); + + vm.expectRevert(abi.encodeWithSelector(ExecutableProposals.MinExecutionDelayNotPassed.selector, proposalId)); + this.external__execute(proposalId, Durations.ZERO, minExecutionDelay); } function test_cancel_all_proposals() external { @@ -301,7 +312,7 @@ contract ExecutableProposalsUnitTests is UnitTest { assertEq(calls[i].payload, expectedCalls[i].payload); } - _proposals.execute(proposalId, Durations.ZERO); + _proposals.execute(proposalId, Durations.ZERO, Durations.ZERO); proposalDetails = _proposals.getProposalDetails(proposalId); @@ -396,7 +407,7 @@ contract ExecutableProposalsUnitTests is UnitTest { _proposals.schedule(2, Durations.ZERO); assertEq(_proposals.getProposalsCount(), 3); - _proposals.execute(1, Durations.ZERO); + _proposals.execute(1, Durations.ZERO, Durations.ZERO); assertEq(_proposals.getProposalsCount(), 3); _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock))); @@ -421,7 +432,7 @@ contract ExecutableProposalsUnitTests is UnitTest { assert(_proposals.canExecute(proposalId, delay)); - _proposals.execute(proposalId, Durations.ZERO); + _proposals.execute(proposalId, Durations.ZERO, Durations.ZERO); assert(!_proposals.canExecute(proposalId, delay)); } @@ -442,7 +453,7 @@ contract ExecutableProposalsUnitTests is UnitTest { assertEq(_proposals.getProposalsCount(), 1); uint256 executedProposalId = 1; _proposals.schedule(executedProposalId, Durations.ZERO); - _proposals.execute(executedProposalId, Durations.ZERO); + _proposals.execute(executedProposalId, Durations.ZERO, Durations.ZERO); _proposals.submit(address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock))); assertEq(_proposals.getProposalsCount(), 2); @@ -480,7 +491,7 @@ contract ExecutableProposalsUnitTests is UnitTest { assert(_proposals.canSchedule(proposalId, delay)); _proposals.schedule(proposalId, delay); - _proposals.execute(proposalId, Durations.ZERO); + _proposals.execute(proposalId, Durations.ZERO, Durations.ZERO); assert(!_proposals.canSchedule(proposalId, delay)); } @@ -494,4 +505,8 @@ contract ExecutableProposalsUnitTests is UnitTest { assert(!_proposals.canSchedule(proposalId, Durations.ZERO)); } + + function external__execute(uint256 proposalId, Duration afterScheduleDelay, Duration minExecutionDelay) external { + _proposals.execute(proposalId, afterScheduleDelay, minExecutionDelay); + } } diff --git a/test/unit/types/Duration.t.sol b/test/unit/types/Duration.t.sol index 0b7a3921..95945198 100644 --- a/test/unit/types/Duration.t.sol +++ b/test/unit/types/Duration.t.sol @@ -296,10 +296,6 @@ contract DurationTests is UnitTest { this.external__from(durationInSeconds); } - function testFuzz_min_HappyPath(Duration d1, Duration d2) external { - assertEq(Durations.min(d1, d2), Durations.from(Math.min(d1.toSeconds(), d2.toSeconds()))); - } - // --- // Helper test methods // --- diff --git a/test/utils/SetupDeployment.sol b/test/utils/SetupDeployment.sol index 9e1d1201..eb6bfb54 100644 --- a/test/utils/SetupDeployment.sol +++ b/test/utils/SetupDeployment.sol @@ -178,6 +178,7 @@ abstract contract SetupDeployment is Test { dgDeployConfig.FIRST_SEAL_RAGE_QUIT_SUPPORT = PercentsD16.fromBasisPoints(3_00); // 3% dgDeployConfig.SECOND_SEAL_RAGE_QUIT_SUPPORT = PercentsD16.fromBasisPoints(15_00); // 15% dgDeployConfig.MIN_ASSETS_LOCK_DURATION = Durations.from(5 hours); + dgDeployConfig.MAX_MIN_ASSETS_LOCK_DURATION = Durations.from(365 days); dgDeployConfig.VETO_SIGNALLING_MIN_DURATION = Durations.from(3 days); dgDeployConfig.VETO_SIGNALLING_MAX_DURATION = Durations.from(30 days); dgDeployConfig.VETO_SIGNALLING_MIN_ACTIVE_DURATION = Durations.from(5 hours);