feat: executor role (#20)

src/MultiSigEnterpriseVault.sol

@@ -24,9 +24,8 @@ contract MultiSigEnterpriseVault is User, IMultiSigEnterpriseVault {
     uint256 initialThreshold,
     uint256 initialOwnerOverrideLimit
   ) User(owner, initialOwnerOverrideLimit) {
-    if (AddressUtils.isValidUserAddress(owner)) {
-      signatoryThreshold = initialThreshold;
-    }
+    AddressUtils.requireValidUserAddress(owner);
+    signatoryThreshold = initialThreshold;
   }
 
   /**
@@ -37,7 +36,7 @@ contract MultiSigEnterpriseVault is User, IMultiSigEnterpriseVault {
   function ownerUpdateSignatoryThreshold(
     uint256 newThreshold
   ) public onlyOwner {
-    if (totalSigners() >= signatoryThreshold) {
+    if (newThreshold > signatoryThreshold && totalSigners() >= signatoryThreshold) {
       revert SignersApprovalRequired();
     }
 

src/components/User.sol

@@ -96,11 +96,12 @@ abstract contract User is OwnerRole, ExecutorRole, SignerRole, IUser {
     address newExecutor
   ) public onlyOwner {
     address oldExecutor = executor();
-    if (oldExecutor.isValidUserAddress() && newExecutor.isValidUserAddress()) {
-      _updateExecutor(newExecutor);
-      _removeUser(oldExecutor);
-      _addUser(newExecutor, RoleType.EXECUTOR);
-    }
+    oldExecutor.requireValidUserAddress();
+    newExecutor.requireValidUserAddress();
+
+    _updateExecutor(newExecutor);
+    _removeUser(oldExecutor);
+    _addUser(newExecutor, RoleType.EXECUTOR);
   }
 
   /**
@@ -112,10 +113,10 @@ abstract contract User is OwnerRole, ExecutorRole, SignerRole, IUser {
    */
   function removeExecutor() public onlyOwner {
     address oldExecutor = executor();
-    if (oldExecutor.isValidUserAddress()) {
-      _removeExecutor();
-      _removeUser(oldExecutor);
-    }
+    oldExecutor.requireValidUserAddress();
+
+    _removeExecutor();
+    _removeUser(oldExecutor);
   }
 
   /**
@@ -138,10 +139,25 @@ abstract contract User is OwnerRole, ExecutorRole, SignerRole, IUser {
   function removeSigner(
     address signer
   ) public onlyOwner {
-    if (signer.isValidUserAddress()) {
-      _removeSigner(signer);
-      _removeUser(signer);
-    }
+    signer.requireValidUserAddress();
+    _removeSigner(signer);
+    _removeUser(signer);
+  }
+
+  /**
+   * @notice Allows an executor to approve the owner override after the timelock has elapsed.
+   */
+  function approveOwnerOverride() public onlyExecutor {
+    address currentOwner = owner();
+    address currentExecutor = _msgSender();
+    uint256 currentTimestamp = block.timestamp;
+
+    _approveOwnerOverride(currentOwner, currentTimestamp, ownerOverrideTimelock);
+    _changeOwner();
+
+    _removeUser(currentOwner);
+    _removeUser(currentExecutor);
+    _addUser(currentExecutor, RoleType.OWNER);
   }
 
   /**
@@ -151,7 +167,7 @@ abstract contract User is OwnerRole, ExecutorRole, SignerRole, IUser {
    */
   function _isUser(
     address user
-  ) internal view returns (bool status) {
+  ) private view returns (bool status) {
     status = _users[user].user != address(0);
   }
 
@@ -163,6 +179,7 @@ abstract contract User is OwnerRole, ExecutorRole, SignerRole, IUser {
    * @dev This is a private function to store user details.
    */
   function _addUser(address user, RoleType role) private {
+    user.requireValidUserAddress();
     _users[user] = UserProfile(user, role, block.timestamp);
     _userCount.increment();
   }
@@ -175,9 +192,9 @@ abstract contract User is OwnerRole, ExecutorRole, SignerRole, IUser {
     address user
   ) private {
     UserProfile storage profile = _users[user];
-    if (profile.user.isValidUserAddress()) {
-      delete _users[user];
-      _userCount.decrement();
-    }
+    profile.user.requireValidUserAddress();
+
+    delete _users[user];
+    _userCount.decrement();
   }
 }

src/components/roles/ExecutorRole.sol

@@ -2,9 +2,10 @@
 pragma solidity ^0.8.27;
 
 import {AccessControl} from '@openzeppelin/contracts/access/AccessControl.sol';
-import {EXECUTOR_ROLE, OWNER_ROLE} from '../../utilities/VaultConstants.sol';
 import {IExecutorRole} from '../../interfaces/roles/IExecutorRole.sol';
 import {AddressUtils} from '../../libraries/AddressUtils.sol';
+import {SafeMath} from '../../libraries/SafeMath.sol';
+import '../../utilities/VaultConstants.sol';
 
 /**
  * @title Executor Role Contract
@@ -13,10 +14,17 @@ import {AddressUtils} from '../../libraries/AddressUtils.sol';
  */
 abstract contract ExecutorRole is AccessControl, IExecutorRole {
   using AddressUtils for address;
+  using SafeMath for uint256;
 
   /// @dev Stores the address of the current executor.
   address private _executor;
 
+  /// @dev Timestamp of when the owner override was initiated.
+  uint256 private overrideInitiatedAt;
+
+  /// @dev Status to indicate if an override is active.
+  bool public isOverrideActive;
+
   /**
    * @dev Modifier to restrict access to functions to only the executor.
    * Reverts with `AccessControlUnauthorizedExecutor` if the caller is not the executor.
@@ -36,6 +44,20 @@ abstract contract ExecutorRole is AccessControl, IExecutorRole {
     return _executor;
   }
 
+  /**
+   * @notice Initiates the owner override process with a timelock.
+   * @dev Only callable by the executor. The override process will start and only be executable after the timelock period has passed.
+   */
+  function initiateOwnerOverride() public onlyRole(EXECUTOR_ROLE) {
+    if (isOverrideActive) {
+      revert OwnerOverrideAlreadyActive();
+    }
+
+    overrideInitiatedAt = block.timestamp;
+    isOverrideActive = true;
+    emit OwnerOverrideInitiated(_msgSender(), overrideInitiatedAt);
+  }
+
   /**
    * @notice Adds a new executor.
    * @param newExecutor The address of the new executor.
@@ -44,12 +66,15 @@ abstract contract ExecutorRole is AccessControl, IExecutorRole {
   function _addExecutor(
     address newExecutor
   ) internal onlyRole(OWNER_ROLE) {
-    require(_executor == address(0), 'ExecutorRole: Executor already exists');
-    if (newExecutor.isValidUserAddress()) {
-      grantRole(EXECUTOR_ROLE, newExecutor);
-      _executor = newExecutor;
-      emit ExecutorAdded(newExecutor);
+    if (_executor != address(0)) {
+      revert ExecutorAlreadyExists();
     }
+
+    _validateExecutorAddress(newExecutor);
+    grantRole(EXECUTOR_ROLE, newExecutor);
+    _executor = newExecutor;
+
+    emit ExecutorAdded(newExecutor);
   }
 
   /**
@@ -58,11 +83,10 @@ abstract contract ExecutorRole is AccessControl, IExecutorRole {
    */
   function _removeExecutor() internal onlyRole(OWNER_ROLE) {
     address oldExecutor = _executor;
-    if (oldExecutor.isValidUserAddress()) {
-      revokeRole(EXECUTOR_ROLE, oldExecutor);
-      _executor = address(0);
-      emit ExecutorRemoved(oldExecutor);
-    }
+    oldExecutor.requireValidUserAddress();
+    revokeRole(EXECUTOR_ROLE, oldExecutor);
+    _executor = address(0);
+    emit ExecutorRemoved(oldExecutor);
   }
 
   /**
@@ -74,15 +98,61 @@ abstract contract ExecutorRole is AccessControl, IExecutorRole {
     address newExecutor
   ) internal onlyRole(OWNER_ROLE) {
     address oldExecutor = _executor;
-    if (oldExecutor.isValidUserAddress() && newExecutor.isValidUserAddress()) {
-      // Revoke old executor role and assign to the new executor
-      revokeRole(EXECUTOR_ROLE, oldExecutor);
-      grantRole(EXECUTOR_ROLE, newExecutor);
+    oldExecutor.requireValidUserAddress();
+
+    _validateExecutorAddress(newExecutor);
+    revokeRole(EXECUTOR_ROLE, oldExecutor);
+    grantRole(EXECUTOR_ROLE, newExecutor);
 
-      // Update the executor address
-      _executor = newExecutor;
+    _executor = newExecutor;
 
-      emit ExecutorUpdated(oldExecutor, newExecutor);
+    emit ExecutorUpdated(oldExecutor, newExecutor);
+  }
+
+  /**
+   * @notice Internal function to approve the owner override after the timelock has elapsed.
+   *
+   * @param ownerAddress The address of the current owner.
+   * @param executionTimestamp The time at which the owner override is executed.
+   * @param ownerOverrideTimelock The duration after which the override can be approved.
+   *
+   * @dev This is called from the `User` contract to execute the owner override.
+   * Requirements
+   * - The owner override is active
+   * - The override timelock has passed
+   */
+  function _approveOwnerOverride(
+    address ownerAddress,
+    uint256 executionTimestamp,
+    uint256 ownerOverrideTimelock
+  ) internal onlyRole(EXECUTOR_ROLE) {
+    if (!isOverrideActive) {
+      revert OwnerOverrideNotActive();
+    }
+
+    uint256 requiredTime = overrideInitiatedAt.add(ownerOverrideTimelock);
+    uint256 elapsedTime = executionTimestamp.subtract(overrideInitiatedAt);
+    if (elapsedTime < ownerOverrideTimelock) {
+      revert OwnerOverrideTimelockNotElapsed(executionTimestamp, requiredTime);
+    }
+
+    address newOwner = _msgSender();
+    _revokeRole(OWNER_ROLE, ownerAddress);
+    _revokeRole(EXECUTOR_ROLE, newOwner);
+    _grantRole(OWNER_ROLE, newOwner);
+
+    isOverrideActive = false;
+    _executor = address(0);
+
+    emit OwnerOverrideApproved(newOwner, executionTimestamp);
+  }
+
+  function _validateExecutorAddress(
+    address newExecutor
+  ) private view {
+    newExecutor.requireValidUserAddress();
+    if (hasRole(OWNER_ROLE, newExecutor) || hasRole(SIGNER_ROLE, newExecutor)) {
+      revert InvalidExecutorUser(newExecutor);
     }
   }
 }

src/components/roles/OwnerRole.sol

@@ -69,7 +69,7 @@ abstract contract OwnerRole is AccessControl, IOwnerRole {
    */
   function increaseOwnerOverrideTimelockLimit(
     uint256 newLimit
-  ) public onlyOwner {
+  ) public onlyRole(OWNER_ROLE) {
     require(newLimit > ownerOverrideTimelock, 'OwnerRole: New limit must be higher');
     ownerOverrideTimelock = newLimit;
     emit OwnerOverrideTimelockIncreased(newLimit);
@@ -85,9 +85,23 @@ abstract contract OwnerRole is AccessControl, IOwnerRole {
    */
   function decreaseOwnerOverrideTimelockLimit(
     uint256 newLimit
-  ) public onlyOwner {
+  ) public onlyRole(OWNER_ROLE) {
     require(newLimit < ownerOverrideTimelock, 'OwnerRole: New limit must be lower');
     ownerOverrideTimelock = newLimit;
     emit OwnerOverrideTimelockDecreased(newLimit);
   }
+
+  /**
+   * @notice Internal function to change the owner address.
+   */
+  function _changeOwner() internal onlyRole(OWNER_ROLE) {
+    address newOwner = _msgSender();
+    address oldOwner = _owner;
+    if (newOwner == oldOwner) {
+      revert AccessControlUnauthorizedOwner(newOwner);
+    }
+
+    _owner = newOwner;
+    emit OwnerChanged(oldOwner, newOwner);
+  }
 }

src/components/roles/SignerRole.sol

@@ -69,12 +69,12 @@ abstract contract SignerRole is AccessControl, ISignerRole {
     address newSigner
   ) internal onlyRole(OWNER_ROLE) {
     require(!isSigner(newSigner), 'SignerRole: Signer already exists');
-    if (newSigner.isValidUserAddress()) {
-      grantRole(SIGNER_ROLE, newSigner);
-      _signers.push(newSigner);
-      _signerCount.increment();
-      emit SignerAdded(newSigner);
-    }
+    newSigner.requireValidUserAddress();
+
+    grantRole(SIGNER_ROLE, newSigner);
+    _signers.push(newSigner);
+    _signerCount.increment();
+    emit SignerAdded(newSigner);
   }
 
   /**

src/interfaces/roles/IExecutorRole.sol

@@ -13,6 +13,31 @@ interface IExecutorRole {
    */
   error AccessControlUnauthorizedExecutor(address account);
 
+  /**
+   * @dev Error thrown when attempting to add a new executor with an existing user role.
+   */
+  error InvalidExecutorUser(address account);
+
+  /**
+   * @dev Error thrown when attempting to add a new executor when one exists.
+   */
+  error ExecutorAlreadyExists();
+
+  /**
+   * @dev Error thrown when attempting to initiate an already active owner override.
+   */
+  error OwnerOverrideAlreadyActive();
+
+  /**
+   * @dev Error thrown when attempting to approve a non active owner override.
+   */
+  error OwnerOverrideNotActive();
+
+  /**
+   * @dev Error thrown when trying to approve an owner override before the timelock has passed.
+   */
+  error OwnerOverrideTimelockNotElapsed(uint256 currentTime, uint256 requiredTime);
+
   /**
    * @dev Event emitted when a new executor is added.
    * @param executor The new executor address.
@@ -30,5 +55,19 @@ interface IExecutorRole {
    * @param oldExecutor The old executor address.
    * @param newExecutor The new executor address.
    */
-  event ExecutorUpdated(address oldExecutor, address indexed newExecutor);
+  event ExecutorUpdated(address indexed oldExecutor, address indexed newExecutor);
+
+  /**
+   * @dev Event emitted when the owner override process is initiated by the executor.
+   * @param executor The address of the executor.
+   * @param timestamp The time when the override was initiated.
+   */
+  event OwnerOverrideInitiated(address indexed executor, uint256 timestamp);
+
+  /**
+   * @dev Event emitted when the owner override process is approved after the timelock.
+   * @param executor The address of the executor.
+   * @param timestamp The time when the override was approved.
+   */
+  event OwnerOverrideApproved(address indexed executor, uint256 timestamp);
 }