Statemind audit results

contracts/DualGovernance.sol

@@ -44,6 +44,7 @@ contract DualGovernance is IDualGovernance {
     error ProposalSchedulingBlocked(uint256 proposalId);
     error ResealIsNotAllowedInNormalState();
     error InvalidResealCommittee(address resealCommittee);
+    error InvalidTiebreakerActivationTimeoutBounds();
 
     // ---
     // Events
@@ -144,6 +145,10 @@ contract DualGovernance is IDualGovernance {
     // ---
 
     constructor(ExternalDependencies memory dependencies, SanityCheckParams memory sanityCheckParams) {
+        if (sanityCheckParams.minTiebreakerActivationTimeout > sanityCheckParams.maxTiebreakerActivationTimeout) {
+            revert InvalidTiebreakerActivationTimeoutBounds();
+        }
+
         TIMELOCK = dependencies.timelock;
         RESEAL_MANAGER = dependencies.resealManager;
 
@@ -188,7 +193,7 @@ contract DualGovernance is IDualGovernance {
             revert ProposalSubmissionBlocked();
         }
         Proposers.Proposer memory proposer = _proposers.getProposer(msg.sender);
-        proposalId = TIMELOCK.submit(proposer.executor, calls, metadata);
+        proposalId = TIMELOCK.submit(proposer.account, proposer.executor, calls, metadata);
     }
 
     /// @notice Schedules a previously submitted proposal for execution in the Dual Governance system.
@@ -363,7 +368,7 @@ contract DualGovernance is IDualGovernance {
         _proposers.unregister(proposer);
 
         /// @dev after the removal of the proposer, check that admin executor still belongs to some proposer
-        if (!_proposers.isExecutor(TIMELOCK.getAdminExecutor())) {
+        if (!_proposers.isExecutor(msg.sender)) {
             revert UnownedAdminExecutor();
         }
     }

contracts/EmergencyProtectedTimelock.sol

@@ -1,8 +1,8 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.26;
 
-import {Duration} from "./types/Duration.sol";
 import {Timestamp} from "./types/Timestamp.sol";
+import {Duration, Durations} from "./types/Duration.sol";
 
 import {IOwnable} from "./interfaces/IOwnable.sol";
 import {IEmergencyProtectedTimelock} from "./interfaces/IEmergencyProtectedTimelock.sol";
@@ -32,17 +32,23 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock {
     // ---
 
     /// @notice The parameters for the sanity checks.
+    /// @param minExecutionDelay The minimum allowable duration for the combined after-submit and after-schedule delays.
     /// @param maxAfterSubmitDelay The maximum allowable delay before a submitted proposal can be scheduled for execution.
     /// @param maxAfterScheduleDelay The maximum allowable delay before a scheduled proposal can be executed.
     /// @param maxEmergencyModeDuration The maximum time the timelock can remain in emergency mode.
     /// @param maxEmergencyProtectionDuration The maximum time the emergency protection mechanism can be activated.
     struct SanityCheckParams {
+        Duration minExecutionDelay;
         Duration maxAfterSubmitDelay;
         Duration maxAfterScheduleDelay;
         Duration maxEmergencyModeDuration;
         Duration maxEmergencyProtectionDuration;
     }
 
+    /// @notice Represents the minimum allowed time that must pass between the submission of a proposal and its execution.
+    /// @dev The minimum permissible value for the sum of `afterScheduleDelay` and `afterSubmitDelay`.
+    Duration public immutable MIN_EXECUTION_DELAY;
+
     /// @notice The upper bound for the delay required before a submitted proposal can be scheduled for execution.
     Duration public immutable MAX_AFTER_SUBMIT_DELAY;
 
@@ -79,31 +85,49 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock {
     // Constructor
     // ---
 
-    constructor(SanityCheckParams memory sanityCheckParams, address adminExecutor) {
+    constructor(
+        SanityCheckParams memory sanityCheckParams,
+        address adminExecutor,
+        Duration afterSubmitDelay,
+        Duration afterScheduleDelay
+    ) {
         _ADMIN_EXECUTOR = adminExecutor;
 
+        MIN_EXECUTION_DELAY = sanityCheckParams.minExecutionDelay;
         MAX_AFTER_SUBMIT_DELAY = sanityCheckParams.maxAfterSubmitDelay;
         MAX_AFTER_SCHEDULE_DELAY = sanityCheckParams.maxAfterScheduleDelay;
         MAX_EMERGENCY_MODE_DURATION = sanityCheckParams.maxEmergencyModeDuration;
         MAX_EMERGENCY_PROTECTION_DURATION = sanityCheckParams.maxEmergencyProtectionDuration;
+
+        if (afterSubmitDelay > Durations.ZERO) {
+            _timelockState.setAfterSubmitDelay(afterSubmitDelay, MAX_AFTER_SUBMIT_DELAY);
+        }
+
+        if (afterScheduleDelay > Durations.ZERO) {
+            _timelockState.setAfterScheduleDelay(afterScheduleDelay, MAX_AFTER_SCHEDULE_DELAY);
+        }
+
+        _timelockState.checkExecutionDelay(MIN_EXECUTION_DELAY);
     }
 
     // ---
     // Main Timelock Functionality
     // ---
 
     /// @notice Submits a new proposal to execute a series of calls through an executor.
+    /// @param proposer The address of the proposer submitting the proposal.
     /// @param executor The address of the executor contract that will execute the calls.
     /// @param calls An array of `ExternalCall` structs representing the calls to be executed.
     /// @param metadata A string containing additional information about the proposal.
     /// @return newProposalId The id of the newly created proposal.
     function submit(
+        address proposer,
         address executor,
         ExternalCall[] calldata calls,
         string calldata metadata
     ) external returns (uint256 newProposalId) {
         _timelockState.checkCallerIsGovernance();
-        newProposalId = _proposals.submit(executor, calls, metadata);
+        newProposalId = _proposals.submit(proposer, executor, calls, metadata);
     }
 
     /// @notice Schedules a proposal for execution after a specified delay.
@@ -137,13 +161,22 @@ contract EmergencyProtectedTimelock is IEmergencyProtectedTimelock {
         _timelockState.setGovernance(newGovernance);
     }
 
-    /// @notice Configures the delays for submitting and scheduling proposals, within defined upper bounds.
-    /// @param afterSubmitDelay The delay required before a submitted proposal can be scheduled.
-    /// @param afterScheduleDelay The delay required before a scheduled proposal can be executed.
-    function setupDelays(Duration afterSubmitDelay, Duration afterScheduleDelay) external {
+    /// @notice Sets the delay required to pass from the submission of a proposal before it can be scheduled for execution.
+    ///     Ensures that the new delay value complies with the defined sanity check bounds.
+    /// @param newAfterSubmitDelay The delay required before a submitted proposal can be scheduled.
+    function setAfterSubmitDelay(Duration newAfterSubmitDelay) external {
+        _checkCallerIsAdminExecutor();
+        _timelockState.setAfterSubmitDelay(newAfterSubmitDelay, MAX_AFTER_SUBMIT_DELAY);
+        _timelockState.checkExecutionDelay(MIN_EXECUTION_DELAY);
+    }
+
+    /// @notice Sets the delay required to pass from the scheduling of a proposal before it can be executed.
+    ///     Ensures that the new delay value complies with the defined sanity check bounds.
+    /// @param newAfterScheduleDelay The delay required before a scheduled proposal can be executed.
+    function setAfterScheduleDelay(Duration newAfterScheduleDelay) external {
         _checkCallerIsAdminExecutor();
-        _timelockState.setAfterSubmitDelay(afterSubmitDelay, MAX_AFTER_SUBMIT_DELAY);
-        _timelockState.setAfterScheduleDelay(afterScheduleDelay, MAX_AFTER_SCHEDULE_DELAY);
+        _timelockState.setAfterScheduleDelay(newAfterScheduleDelay, MAX_AFTER_SCHEDULE_DELAY);
+        _timelockState.checkExecutionDelay(MIN_EXECUTION_DELAY);
     }
 
     /// @notice Transfers ownership of the executor contract to a new owner.

contracts/Escrow.sol

@@ -251,6 +251,9 @@ contract Escrow is IEscrow {
     ///     that were previously locked by the vetoer.
     /// @param unstETHIds An array of ids representing the unstETH NFTs to be unlocked.
     function unlockUnstETH(uint256[] memory unstETHIds) external {
+        if (unstETHIds.length == 0) {
+            revert EmptyUnstETHIds();
+        }
         DUAL_GOVERNANCE.activateNextState();
         _escrowState.checkSignallingEscrow();
         _accounting.checkMinAssetsLockDurationPassed(msg.sender, _escrowState.minAssetsLockDuration);
@@ -283,33 +286,6 @@ contract Escrow is IEscrow {
         DUAL_GOVERNANCE.activateNextState();
     }
 
-    // ---
-    // Convert To NFT
-    // ---
-
-    /// @notice Allows vetoers to convert their locked stETH or wstETH tokens into unstETH NFTs on behalf of the
-    ///     Veto Signalling Escrow contract.
-    /// @param stETHAmounts An array representing the amounts of stETH to be converted into unstETH NFTs.
-    /// @return unstETHIds An array of ids representing the newly created unstETH NFTs corresponding to
-    ///     the converted stETH amounts.
-    function requestWithdrawals(uint256[] calldata stETHAmounts) external returns (uint256[] memory unstETHIds) {
-        DUAL_GOVERNANCE.activateNextState();
-        _escrowState.checkSignallingEscrow();
-
-        unstETHIds = WITHDRAWAL_QUEUE.requestWithdrawals(stETHAmounts, address(this));
-        WithdrawalRequestStatus[] memory statuses = WITHDRAWAL_QUEUE.getWithdrawalStatus(unstETHIds);
-
-        uint256 sharesTotal = 0;
-        for (uint256 i = 0; i < statuses.length; ++i) {
-            sharesTotal += statuses[i].amountOfShares;
-        }
-        _accounting.accountStETHSharesUnlock(msg.sender, SharesValues.from(sharesTotal));
-        _accounting.accountUnstETHLock(msg.sender, unstETHIds, statuses);
-
-        /// @dev Skip calling activateNextState here to save gas, as converting stETH to unstETH NFTs
-        ///     does not affect the RageQuit support.
-    }
-
     // ---
     // Start Rage Quit
     // ---
@@ -540,23 +516,33 @@ contract Escrow is IEscrow {
         state.lastAssetsLockTimestamp = assets.lastAssetsLockTimestamp.toSeconds();
     }
 
+    // @notice Retrieves the unstETH NFT ids of the specified vetoer.
+    /// @param vetoer The address of the vetoer whose unstETH NFTs are being queried.
+    /// @return unstETHIds An array of unstETH NFT ids locked by the vetoer.
+    function getVetoerUnstETHIds(address vetoer) external view returns (uint256[] memory unstETHIds) {
+        unstETHIds = _accounting.assets[vetoer].unstETHIds;
+    }
+
     /// @notice Returns the total count of unstETH NFTs that have not been claimed yet.
     /// @return unclaimedUnstETHIdsCount The total number of unclaimed unstETH NFTs.
     function getUnclaimedUnstETHIdsCount() external view returns (uint256) {
+        _escrowState.checkRageQuitEscrow();
         return _batchesQueue.getTotalUnclaimedUnstETHIdsCount();
     }
 
     /// @notice Retrieves the unstETH NFT ids of the next batch available for claiming.
     /// @param limit The maximum number of unstETH NFTs to return in the batch.
-    /// @return unstETHIds An array of unstETH NFT IDs available for the next withdrawal batch.
+    /// @return unstETHIds An array of unstETH NFT ids available for the next withdrawal batch.
     function getNextWithdrawalBatch(uint256 limit) external view returns (uint256[] memory unstETHIds) {
-        return _batchesQueue.getNextWithdrawalsBatches(limit);
+        _escrowState.checkRageQuitEscrow();
+        unstETHIds = _batchesQueue.getNextWithdrawalsBatches(limit);
     }
 
-    /// @notice Returns whether all withdrawal batches have been finalized.
-    /// @return isWithdrawalsBatchesFinalized A boolean value indicating whether all withdrawal batches have been
-    ///     finalized (`true`) or not (`false`).
-    function isWithdrawalsBatchesFinalized() external view returns (bool) {
+    /// @notice Returns whether all withdrawal batches have been closed.
+    /// @return isWithdrawalsBatchesClosed A boolean value indicating whether all withdrawal batches have been
+    ///     closed (`true`) or not (`false`).
+    function isWithdrawalsBatchesClosed() external view returns (bool) {
+        _escrowState.checkRageQuitEscrow();
         return _batchesQueue.isClosed();
     }
 

contracts/ResealManager.sol

@@ -49,7 +49,7 @@ contract ResealManager is IResealManager {
         _checkCallerIsGovernance();
 
         uint256 sealableResumeSinceTimestamp = ISealable(sealable).getResumeSinceTimestamp();
-        if (sealableResumeSinceTimestamp < block.timestamp || sealableResumeSinceTimestamp == PAUSE_INFINITELY) {
+        if (block.timestamp >= sealableResumeSinceTimestamp || sealableResumeSinceTimestamp == PAUSE_INFINITELY) {
             revert SealableWrongPauseState();
         }
         Address.functionCall(sealable, abi.encodeWithSelector(ISealable.resume.selector));
@@ -66,7 +66,7 @@ contract ResealManager is IResealManager {
         _checkCallerIsGovernance();
 
         uint256 sealableResumeSinceTimestamp = ISealable(sealable).getResumeSinceTimestamp();
-        if (sealableResumeSinceTimestamp < block.timestamp) {
+        if (block.timestamp >= sealableResumeSinceTimestamp) {
             revert SealableWrongPauseState();
         }
         Address.functionCall(sealable, abi.encodeWithSelector(ISealable.resume.selector));

contracts/TimelockedGovernance.sol

@@ -14,6 +14,8 @@ contract TimelockedGovernance is IGovernance {
     // ---
 
     error CallerIsNotGovernance(address caller);
+    error InvalidGovernance(address governance);
+    error InvalidTimelock(ITimelock timelock);
 
     // ---
     // Immutable Variables
@@ -30,6 +32,12 @@ contract TimelockedGovernance is IGovernance {
     /// @param governance The address of the governance contract.
     /// @param timelock The address of the timelock contract.
     constructor(address governance, ITimelock timelock) {
+        if (governance == address(0)) {
+            revert InvalidGovernance(governance);
+        }
+        if (address(timelock) == address(0)) {
+            revert InvalidTimelock(timelock);
+        }
         GOVERNANCE = governance;
         TIMELOCK = timelock;
     }
@@ -43,7 +51,7 @@ contract TimelockedGovernance is IGovernance {
         string calldata metadata
     ) external returns (uint256 proposalId) {
         _checkCallerIsGovernance();
-        return TIMELOCK.submit(TIMELOCK.getAdminExecutor(), calls, metadata);
+        return TIMELOCK.submit(msg.sender, TIMELOCK.getAdminExecutor(), calls, metadata);
     }
 
     /// @notice Schedules a submitted proposal.
@@ -52,12 +60,6 @@ contract TimelockedGovernance is IGovernance {
         TIMELOCK.schedule(proposalId);
     }
 
-    /// @notice Executes a scheduled proposal.
-    /// @param proposalId The id of the proposal to be executed.
-    function executeProposal(uint256 proposalId) external {
-        TIMELOCK.execute(proposalId);
-    }
-
     /// @notice Checks if a proposal can be scheduled.
     /// @param proposalId The id of the proposal to check.
     /// @return A boolean indicating whether the proposal can be scheduled.