[SMR-1996] RBAC refactor

script/InitializeChildContracts.s.sol

@@ -58,7 +58,6 @@ contract InitializeChildContracts is Script {
             defaultAdmin: params.childAdminAddress,
             pauser: params.childPauserAddress,
             unpauser: params.childUnpauserAddress,
-            variableManager: params.childAdminAddress,
             adaptorManager: params.childAdminAddress
         });
         params.childERC20Bridge.initialize(

src/child/ChildERC20Bridge.sol

@@ -1,48 +1,32 @@
 // SPDX-License-Identifier: Apache 2.0
 pragma solidity 0.8.19;
 
-import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
-import {AccessControlUpgradeable} from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
 import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
 import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
 import {Address} from "@openzeppelin/contracts/utils/Address.sol";
 import {
     IChildERC20BridgeEvents,
     IChildERC20BridgeErrors,
-    IChildERC20Bridge,
-    IERC20Metadata
+    IChildERC20Bridge
 } from "../interfaces/child/IChildERC20Bridge.sol";
 import {IChildERC20BridgeAdaptor} from "../interfaces/child/IChildERC20BridgeAdaptor.sol";
 import {IChildERC20} from "../interfaces/child/IChildERC20.sol";
 import {IWIMX} from "../interfaces/child/IWIMX.sol";
+import {BridgeRoles} from "../common/BridgeRoles.sol";
 
 /**
- * @notice RootERC20Bridge is a bridge that allows ERC20 tokens to be transferred from the root chain to the child chain.
+ * @notice ChildERC20Bridge is a bridge that handles the depositing ERC20 and native tokens to the child chain from the rootchain
+ * and facilates the withdrawals of ERC20 and native tokens from the child chain to the rootchain.
  * @dev This contract is designed to be upgradeable.
- * @dev Follows a pattern of using a bridge adaptor to communicate with the child chain. This is because the underlying communication protocol may change,
+ * @dev Follows a pattern of using a bridge adaptor to communicate with the root chain. This is because the underlying communication protocol may change,
  *      and also allows us to decouple vendor-specific messaging logic from the bridge logic.
- * @dev Because of this pattern, any checks or logic that is agnostic to the messaging protocol should be done in RootERC20Bridge.
+ * @dev Because of this pattern, any checks or logic that is agnostic to the messaging protocol should be done in ChildERC20Bridge.
  * @dev Any checks or logic that is specific to the underlying messaging protocol should be done in the bridge adaptor.
  */
-contract ChildERC20Bridge is
-    AccessControlUpgradeable, // AccessControlUpgradeable inherits Initializable
-    IChildERC20BridgeErrors,
-    IChildERC20Bridge,
-    IChildERC20BridgeEvents
-{
-    using SafeERC20 for IERC20Metadata;
-
+contract ChildERC20Bridge is IChildERC20BridgeErrors, IChildERC20Bridge, IChildERC20BridgeEvents, BridgeRoles {
     /// @dev leave this as the first param for the integration tests
     mapping(address => address) public rootTokenToChildToken;
 
-    /**
-     * ROLES
-     */
-    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");
-    bytes32 public constant UNPAUSER_ROLE = keccak256("UNPAUSER_ROLE");
-    bytes32 public constant VARIABLE_MANAGER_ROLE = keccak256("VARIABLE_MANAGER_ROLE");
-    bytes32 public constant ADAPTOR_MANAGER_ROLE = keccak256("ADAPTOR_MANAGER_ROLE");
-
     bytes32 public constant MAP_TOKEN_SIG = keccak256("MAP_TOKEN");
     bytes32 public constant DEPOSIT_SIG = keccak256("DEPOSIT");
     bytes32 public constant WITHDRAW_SIG = keccak256("WITHDRAW");
@@ -97,8 +81,7 @@ contract ChildERC20Bridge is
         if (
             newBridgeAdaptor == address(0) || newChildTokenTemplate == address(0) || newRootIMXToken == address(0)
                 || newRoles.defaultAdmin == address(0) || newRoles.pauser == address(0) || newRoles.unpauser == address(0)
-                || newRoles.variableManager == address(0) || newRoles.adaptorManager == address(0)
-                || newWIMXToken == address(0)
+                || newRoles.adaptorManager == address(0) || newWIMXToken == address(0)
         ) {
             revert ZeroAddress();
         }
@@ -116,7 +99,6 @@ contract ChildERC20Bridge is
         _grantRole(DEFAULT_ADMIN_ROLE, newRoles.defaultAdmin);
         _grantRole(PAUSER_ROLE, newRoles.pauser);
         _grantRole(UNPAUSER_ROLE, newRoles.unpauser);
-        _grantRole(VARIABLE_MANAGER_ROLE, newRoles.variableManager);
         _grantRole(ADAPTOR_MANAGER_ROLE, newRoles.adaptorManager);
 
         rootERC20BridgeAdaptor = newRootERC20BridgeAdaptor;

src/common/BridgeRoles.sol

@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: Apache 2.0
+pragma solidity 0.8.19;
+
+import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
+import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
+
+/**
+ *
+ * @title BridgeRoles.sol
+ * @notice BridgeRoles.sol is an abstract contract that defines the roles and permissions and pausable functionality across the root and child chain bridge contracts.
+ * @dev This contract uses OpenZeppelin's AccessControl and Pausable contracts. This contract is abstract and is intended to be inherited by the root and child chain bridge contracts.
+ */
+abstract contract BridgeRoles is AccessControlUpgradeable, PausableUpgradeable {
+    // Roles
+    /// @notice Role identifier for those who can pause functionanlity.
+    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER");
+
+    /// @notice Role identifier for those who can unpause functionality.
+    bytes32 public constant UNPAUSER_ROLE = keccak256("UNPAUSER");
+
+    /// @notice Role identifier for those who can update the bridge adaptor.
+    bytes32 public constant ADAPTOR_MANAGER_ROLE = keccak256("ADAPTOR_MANAGER");
+
+    // Role granting functions
+    /**
+     * @notice Function to grant pauser role to an address
+     */
+    function grantPauserRole(address account) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        grantRole(PAUSER_ROLE, account);
+    }
+
+    /**
+     * @notice Function to grant unpauser role to an address
+     */
+    function grantUnpauserRole(address account) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        grantRole(UNPAUSER_ROLE, account);
+    }
+
+    /**
+     * @notice Function to grant adaptor manager role to an address
+     */
+    function grantAdaptorManagerRole(address account) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        grantRole(ADAPTOR_MANAGER_ROLE, account);
+    }
+
+    // Role revoking functions
+    /**
+     * @notice Function to revoke pauser role from an address
+     */
+    function revokePauserRole(address account) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        revokeRole(PAUSER_ROLE, account);
+    }
+
+    /**
+     * @notice Function to revoke unpauser role from an address
+     */
+    function revokeUnpauserRole(address account) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        revokeRole(UNPAUSER_ROLE, account);
+    }
+
+    /**
+     * @notice Function to revoke adaptor manager role from an address
+     */
+    function revokeAdaptorManagerRole(address account) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        revokeRole(ADAPTOR_MANAGER_ROLE, account);
+    }
+
+    // Pausable functions
+    /**
+     * @notice Function to pause the contract
+     * @dev Only PAUSER_ROLE can call this function
+     */
+    function pause() external onlyRole(PAUSER_ROLE) {
+        _pause();
+    }
+
+    /**
+     * @notice Function to pause the contract
+     * @dev Only UNPAUSER_ROLE can call this function
+     */
+    function unpause() external onlyRole(UNPAUSER_ROLE) {
+        _unpause();
+    }
+}

src/interfaces/child/IChildERC20Bridge.sol

@@ -1,15 +1,13 @@
 // SPDX-License-Identifier: Apache 2.0
 pragma solidity 0.8.19;
 
-import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
 import {IChildERC20} from "./IChildERC20.sol";
 
 interface IChildERC20Bridge {
     struct InitializationRoles {
         address defaultAdmin; // The address which will inherit `DEFAULT_ADMIN_ROLE`.
         address pauser; // The address which will inherit `PAUSER_ROLE`.
         address unpauser; // The address which will inherit `UNPAUSER_ROLE`.
-        address variableManager; // The address which will inherit `VARIABLE_MANAGER_ROLE`.
         address adaptorManager; // The address which will inherit `ADAPTOR_MANAGER_ROLE`.
     }
 

src/interfaces/root/IRootERC20Bridge.sol

@@ -12,6 +12,16 @@ interface IRootERC20Bridge {
         address adaptorManager; // The address which will inherit `ADAPTOR_MANAGER_ROLE`.
     }
 
+    /**
+     * @notice Function to revoke variable manager role from an address
+     */
+    function revokeVariableManagerRole(address account) external;
+
+    /**
+     * @notice Function to grant variable manager role to an address
+     */
+    function grantVariableManagerRole(address account) external;
+
     function childBridgeAdaptor() external view returns (string memory);
     /**
      * @notice Receives a bridge message from child chain, parsing the message type then executing.
@@ -33,6 +43,19 @@ interface IRootERC20Bridge {
      */
     function mapToken(IERC20Metadata rootToken) external payable returns (address);
 
+    /**
+     * @notice Deposits `amount` of ETH to `msg.sender` on the child chain.
+     * @param amount The amount of ETH to deposit.
+     */
+    function depositETH(uint256 amount) external payable;
+
+    /**
+     * @notice Deposits `amount` of ETH to `receiver` on the child chain.
+     * @param receiver The address to deposit the ETH to.
+     * @param amount The amount of ETH to deposit.
+     */
+    function depositToETH(address receiver, uint256 amount) external payable;
+
     /**
      * @notice Initiate sending a deposit message to the child chain.
      * @custom:requires `rootToken` to already be mapped with `mapToken`.

src/root/RootERC20Bridge.sol

@@ -2,47 +2,36 @@
 pragma solidity 0.8.19;
 
 import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
-import {AccessControlUpgradeable} from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
 import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
 import {Address} from "@openzeppelin/contracts/utils/Address.sol";
 import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
-import {IAxelarGateway} from "@axelar-cgp-solidity/contracts/interfaces/IAxelarGateway.sol";
 import {
     IRootERC20Bridge,
     IERC20Metadata,
     IRootERC20BridgeEvents,
     IRootERC20BridgeErrors
 } from "../interfaces/root/IRootERC20Bridge.sol";
 import {IRootERC20BridgeAdaptor} from "../interfaces/root/IRootERC20BridgeAdaptor.sol";
-import {IChildERC20} from "../interfaces/child/IChildERC20.sol";
 import {IWETH} from "../interfaces/root/IWETH.sol";
+import {BridgeRoles} from "../common/BridgeRoles.sol";
 
 /**
- * @notice RootERC20Bridge is a bridge that allows ERC20 tokens to be transferred from the root chain to the child chain.
+ * @notice RootERC20Bridge is a bridge that allows ERC20 and native tokens to be bridged from the root chain to the child chain
+ * and facilitates the withdrawals of ERC20 and native tokens from the child chain to the root chain.
  * @dev This contract is designed to be upgradeable.
  * @dev Follows a pattern of using a bridge adaptor to communicate with the child chain. This is because the underlying communication protocol may change,
  *      and also allows us to decouple vendor-specific messaging logic from the bridge logic.
  * @dev Because of this pattern, any checks or logic that is agnostic to the messaging protocol should be done in RootERC20Bridge.
  * @dev Any checks or logic that is specific to the underlying messaging protocol should be done in the bridge adaptor.
  */
-contract RootERC20Bridge is
-    AccessControlUpgradeable, // AccessControlUpgradeable inherits Initializable
-    IRootERC20Bridge,
-    IRootERC20BridgeEvents,
-    IRootERC20BridgeErrors
-{
+contract RootERC20Bridge is IRootERC20Bridge, IRootERC20BridgeEvents, IRootERC20BridgeErrors, BridgeRoles {
     using SafeERC20 for IERC20Metadata;
 
     /// @dev leave this as the first param for the integration tests
     mapping(address => address) public rootTokenToChildToken;
 
-    /**
-     * ROLES
-     */
-    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");
-    bytes32 public constant UNPAUSER_ROLE = keccak256("UNPAUSER_ROLE");
-    bytes32 public constant VARIABLE_MANAGER_ROLE = keccak256("VARIABLE_MANAGER_ROLE");
-    bytes32 public constant ADAPTOR_MANAGER_ROLE = keccak256("ADAPTOR_MANAGER_ROLE");
+    /// @notice Role identifier those who can update the cumulative IMX deposit limit.
+    bytes32 public constant VARIABLE_MANAGER_ROLE = keccak256("VARIABLE_MANAGER");
 
     uint256 public constant UNLIMITED_DEPOSIT = 0;
     bytes32 public constant MAP_TOKEN_SIG = keccak256("MAP_TOKEN");
@@ -130,6 +119,20 @@ contract RootERC20Bridge is
         imxCumulativeDepositLimit = newImxCumulativeDepositLimit;
     }
 
+    /**
+     * @inheritdoc IRootERC20Bridge
+     */
+    function revokeVariableManagerRole(address account) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        revokeRole(VARIABLE_MANAGER_ROLE, account);
+    }
+
+    /**
+     * @inheritdoc IRootERC20Bridge
+     */
+    function grantVariableManagerRole(address account) external onlyRole(DEFAULT_ADMIN_ROLE) {
+        grantRole(VARIABLE_MANAGER_ROLE, account);
+    }
+
     /**
      * @notice Updates the root bridge adaptor.
      * @param newRootBridgeAdaptor Address of new root bridge adaptor.
@@ -217,10 +220,16 @@ contract RootERC20Bridge is
         return _mapToken(rootToken);
     }
 
+    /**
+     * @inheritdoc IRootERC20Bridge
+     */
     function depositETH(uint256 amount) external payable {
         _depositETH(msg.sender, amount);
     }
 
+    /**
+     * @inheritdoc IRootERC20Bridge
+     */
     function depositToETH(address receiver, uint256 amount) external payable {
         _depositETH(receiver, amount);
     }

test/integration/child/ChildAxelarBridge.t.sol

@@ -42,7 +42,6 @@ contract ChildERC20BridgeIntegrationTest is Test, IChildERC20BridgeEvents, IChil
             defaultAdmin: address(this),
             pauser: address(this),
             unpauser: address(this),
-            variableManager: address(this),
             adaptorManager: address(this)
         });
         childERC20Bridge.initialize(
@@ -309,7 +308,7 @@ contract ChildERC20BridgeIntegrationTest is Test, IChildERC20BridgeEvents, IChil
         address rootAddress = address(0x123);
         {
             // Found by running `forge inspect src/child/ChildERC20Bridge.sol:ChildERC20Bridge storageLayout | grep -B3 -A5 -i "rootTokenToChildToken"`
-            uint256 rootTokenToChildTokenMappingSlot = 151;
+            uint256 rootTokenToChildTokenMappingSlot = 201;
             address childAddress = address(444444);
             bytes32 slot = getMappingStorageSlotFor(rootAddress, rootTokenToChildTokenMappingSlot);
             bytes32 data = bytes32(uint256(uint160(childAddress)));