refactor to simplify into 1 contract and interface

README.md

@@ -10,14 +10,11 @@ Executors serve as admins of the bridged domain instances of the protocol. <br>T
 ![Architecture Diagram](/diagram.png)
 ## 🤝 Contribution Guidelines
 In order to add governance relay infrastructure for a new domain, perform the following steps:
-1. Go to [XChain Helpers](https://github.com/marsfoundation/xchain-helpers) repository and add:
-    1. A domain helper abstracting away the process of passing messages between host domain and your bridged domain.
-    2. A helper function to the XChainForwarders library abstracting away host domain interactions with the bridge.
-2. Add `BridgeExecutor` to the `/src/executors` directory. Follow currently used naming convention.
-3. Add a new test file for your domain to the `/test` directory. Inherit `CrosschainTestBase` and add tests specific to your domain to the test suite. All of the tests have to pass. Follow linting and naming convention used in other test files.
-4. Use proper labeling for your open PR (always set adequate priority and status)
-5. Get an approving review from at least one of three designated reviewers - **@hexonaut**, **@lucas-manuel** or **@barrutko**
-6. Enjoy governance messages being passed through the bridge to your domain! 🎉
+1. Go to [XChain Helpers](https://github.com/marsfoundation/xchain-helpers) repository and complete the process for adding a new domain.
+2. Add a new test file for your domain to the `/test` directory. Inherit `CrosschainTestBase` and add tests specific to your domain to the test suite. All of the tests have to pass. Follow linting and naming convention used in other test files.
+3. Use proper labeling for your open PR (always set adequate priority and status)
+4. Get an approving review from at least one of three designated reviewers - **@hexonaut**, **@lucas-manuel** or **@barrutko**
+5. Enjoy governance messages being passed through the bridge to your domain! 🎉
 
 ***
 *The IP in this repository was assigned to Mars SPC Limited in respect of the MarsOne SP*

src/executors/BridgeExecutorBase.sol → src/Executor.sol

@@ -1,18 +1,17 @@
 // SPDX-License-Identifier: AGPL-3.0
 pragma solidity ^0.8.10;
 
-import { Address } from "lib/openzeppelin-contracts/contracts/utils/Address.sol";
+import { AccessControl } from "openzeppelin-contracts/contracts/access/AccessControl.sol";
+import { Address }       from "openzeppelin-contracts/contracts/utils/Address.sol";
 
-import { IExecutorBase } from 'src/interfaces/IExecutorBase.sol';
+import { IExecutor } from './interfaces/IExecutor.sol';
 
 /**
- * @title BridgeExecutorBase
+ * @title Executor
  * @author Aave
- * @notice Abstract contract that implements basic executor functionality
- * @dev It does not implement an external `queue` function. This should instead be done in the inheriting
- * contract with proper access control
+ * @notice Executor which queues up message calls and executes them after an optional delay
  */
-abstract contract BridgeExecutorBase is IExecutorBase {
+contract Executor is IExecutor, AccessControl {
 
     using Address for address;
 
@@ -68,9 +67,71 @@ abstract contract BridgeExecutorBase is IExecutorBase {
         _updateDelay(delay);
         _updateGracePeriod(gracePeriod);
         _updateGuardian(guardian);
+
+        _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
     }
 
-    /// @inheritdoc IExecutorBase
+    /// @inheritdoc IExecutor
+    function queue(
+        address[] memory targets,
+        uint256[] memory values,
+        string[] memory signatures,
+        bytes[] memory calldatas,
+        bool[] memory withDelegatecalls
+    ) external override onlyRole(DEFAULT_ADMIN_ROLE) {
+        if (targets.length == 0) revert EmptyTargets();
+        uint256 targetsLength = targets.length;
+        if (
+            targetsLength != values.length ||
+            targetsLength != signatures.length ||
+            targetsLength != calldatas.length ||
+            targetsLength != withDelegatecalls.length
+        ) revert InconsistentParamsLength();
+
+        uint256 actionsSetId = _actionsSetCounter;
+        uint256 executionTime = block.timestamp + _delay;
+        unchecked {
+            ++_actionsSetCounter;
+        }
+
+        for (uint256 i = 0; i < targetsLength; ) {
+            bytes32 actionHash = keccak256(
+                abi.encode(
+                    targets[i],
+                    values[i],
+                    signatures[i],
+                    calldatas[i],
+                    executionTime,
+                    withDelegatecalls[i]
+                )
+            );
+            if (isActionQueued(actionHash)) revert DuplicateAction();
+            _queuedActions[actionHash] = true;
+            unchecked {
+                ++i;
+            }
+        }
+
+        ActionsSet storage actionsSet = _actionsSets[actionsSetId];
+        actionsSet.targets = targets;
+        actionsSet.values = values;
+        actionsSet.signatures = signatures;
+        actionsSet.calldatas = calldatas;
+        actionsSet.withDelegatecalls = withDelegatecalls;
+        actionsSet.executionTime = executionTime;
+
+        emit ActionsSetQueued(
+            actionsSetId,
+            targets,
+            values,
+            signatures,
+            calldatas,
+            withDelegatecalls,
+            executionTime
+        );
+    }
+
+    /// @inheritdoc IExecutor
     function execute(uint256 actionsSetId) external payable override {
         if (getCurrentState(actionsSetId) != ActionsSetState.Queued) revert OnlyQueuedActions();
 
@@ -98,7 +159,7 @@ abstract contract BridgeExecutorBase is IExecutorBase {
         emit ActionsSetExecuted(actionsSetId, msg.sender, returnedData);
     }
 
-    /// @inheritdoc IExecutorBase
+    /// @inheritdoc IExecutor
     function cancel(uint256 actionsSetId) external override onlyGuardian {
         if (getCurrentState(actionsSetId) != ActionsSetState.Queued) revert OnlyQueuedActions();
 
@@ -123,23 +184,23 @@ abstract contract BridgeExecutorBase is IExecutorBase {
         emit ActionsSetCanceled(actionsSetId);
     }
 
-    /// @inheritdoc IExecutorBase
+    /// @inheritdoc IExecutor
     function updateGuardian(address guardian) external override onlyThis {
         _updateGuardian(guardian);
     }
 
-    /// @inheritdoc IExecutorBase
+    /// @inheritdoc IExecutor
     function updateDelay(uint256 delay) external override onlyThis {
         _updateDelay(delay);
     }
 
-    /// @inheritdoc IExecutorBase
+    /// @inheritdoc IExecutor
     function updateGracePeriod(uint256 gracePeriod) external override onlyThis {
         if (gracePeriod < MINIMUM_GRACE_PERIOD) revert GracePeriodTooShort();
         _updateGracePeriod(gracePeriod);
     }
 
-    /// @inheritdoc IExecutorBase
+    /// @inheritdoc IExecutor
     function executeDelegateCall(address target, bytes calldata data)
         external
         payable
@@ -150,30 +211,30 @@ abstract contract BridgeExecutorBase is IExecutorBase {
         return target.functionDelegateCall(data);
     }
 
-    /// @inheritdoc IExecutorBase
+    /// @inheritdoc IExecutor
     function receiveFunds() external payable override {}
 
-    /// @inheritdoc IExecutorBase
+    /// @inheritdoc IExecutor
     function getDelay() external view override returns (uint256) {
         return _delay;
     }
 
-    /// @inheritdoc IExecutorBase
+    /// @inheritdoc IExecutor
     function getGracePeriod() external view override returns (uint256) {
         return _gracePeriod;
     }
 
-    /// @inheritdoc IExecutorBase
+    /// @inheritdoc IExecutor
     function getGuardian() external view override returns (address) {
         return _guardian;
     }
 
-    /// @inheritdoc IExecutorBase
+    /// @inheritdoc IExecutor
     function getActionsSetCount() external view override returns (uint256) {
         return _actionsSetCounter;
     }
 
-    /// @inheritdoc IExecutorBase
+    /// @inheritdoc IExecutor
     function getActionsSetById(uint256 actionsSetId)
         external
         view
@@ -183,7 +244,7 @@ abstract contract BridgeExecutorBase is IExecutorBase {
         return _actionsSets[actionsSetId];
     }
 
-    /// @inheritdoc IExecutorBase
+    /// @inheritdoc IExecutor
     function getCurrentState(uint256 actionsSetId) public view override returns (ActionsSetState) {
         if (_actionsSetCounter <= actionsSetId) revert InvalidActionsSetId();
         ActionsSet storage actionsSet = _actionsSets[actionsSetId];
@@ -198,7 +259,7 @@ abstract contract BridgeExecutorBase is IExecutorBase {
         }
     }
 
-    /// @inheritdoc IExecutorBase
+    /// @inheritdoc IExecutor
     function isActionQueued(bytes32 actionHash) public view override returns (bool) {
         return _queuedActions[actionHash];
     }
@@ -218,74 +279,6 @@ abstract contract BridgeExecutorBase is IExecutorBase {
         _gracePeriod = gracePeriod;
     }
 
-    /**
-    * @notice Queue an ActionsSet
-    * @dev If a signature is empty, calldata is used for the execution, calldata is appended to signature otherwise
-    * @param targets Array of targets to be called by the actions set
-    * @param values Array of values to pass in each call by the actions set
-    * @param signatures Array of function signatures to encode in each call (can be empty)
-    * @param calldatas Array of calldata to pass in each call (can be empty)
-    * @param withDelegatecalls Array of whether to delegatecall for each call
-    **/
-    function _queue(
-        address[] memory targets,
-        uint256[] memory values,
-        string[] memory signatures,
-        bytes[] memory calldatas,
-        bool[] memory withDelegatecalls
-    ) internal {
-        if (targets.length == 0) revert EmptyTargets();
-        uint256 targetsLength = targets.length;
-        if (
-            targetsLength != values.length ||
-            targetsLength != signatures.length ||
-            targetsLength != calldatas.length ||
-            targetsLength != withDelegatecalls.length
-        ) revert InconsistentParamsLength();
-
-        uint256 actionsSetId = _actionsSetCounter;
-        uint256 executionTime = block.timestamp + _delay;
-        unchecked {
-            ++_actionsSetCounter;
-        }
-
-        for (uint256 i = 0; i < targetsLength; ) {
-            bytes32 actionHash = keccak256(
-                abi.encode(
-                    targets[i],
-                    values[i],
-                    signatures[i],
-                    calldatas[i],
-                    executionTime,
-                    withDelegatecalls[i]
-                )
-            );
-            if (isActionQueued(actionHash)) revert DuplicateAction();
-            _queuedActions[actionHash] = true;
-            unchecked {
-                ++i;
-            }
-        }
-
-        ActionsSet storage actionsSet = _actionsSets[actionsSetId];
-        actionsSet.targets = targets;
-        actionsSet.values = values;
-        actionsSet.signatures = signatures;
-        actionsSet.calldatas = calldatas;
-        actionsSet.withDelegatecalls = withDelegatecalls;
-        actionsSet.executionTime = executionTime;
-
-        emit ActionsSetQueued(
-            actionsSetId,
-            targets,
-            values,
-            signatures,
-            calldatas,
-            withDelegatecalls,
-            executionTime
-        );
-    }
-
     function _executeTransaction(
         address target,
         uint256 value,