fix: Access Control #2 [WIP] (#25)

contracts/access/AccessControlled.sol

@@ -1,31 +1,28 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.18;
 
-import { ZNSRoles } from "./ZNSRoles.sol";
 import { IZNSAccessController } from "./IZNSAccessController.sol";
 
 
-abstract contract AccessControlled is ZNSRoles {
+abstract contract AccessControlled {
+
     event AccessControllerSet(address accessController);
 
     IZNSAccessController internal accessController;
 
-    modifier onlyRole(bytes32 role) {
-        accessController.checkRole(role, msg.sender);
+
+    modifier onlyAdmin() {
+        accessController.checkAdmin(msg.sender);
         _;
     }
 
     /**
-     * @dev This is here to make sure the external function is always implemented in children,
-     * otherwise we will not be able to reset the module (not ideal since it might
-     * not get to the final interface of a child).
-     * TODO AC: how do we make sure this gets to the final interface?
+     * @dev These 2 virtual functions are here to make sure they are always implemented in children,
+     * otherwise we will not be able to reset the module or read the AC address
      */
-    function setAccessController(address _accessController) external virtual;
+    function getAccessController() external view virtual returns (address);
 
-    function getAccessController() external view returns (address) {
-        return address(accessController);
-    }
+    function setAccessController(address _accessController) external virtual;
 
     function _setAccessController(address _accessController) internal {
         require(_accessController != address(0), "AC: _accessController is 0x0 address");

contracts/access/IZNSAccessController.sol

@@ -10,7 +10,17 @@ interface IZNSAccessController is IAccessControlUpgradeable {
         address[] calldata operatorAddresses
     ) external;
 
-    function checkRole(bytes32 role, address account) external view;
-
     function setRoleAdmin(bytes32 role, bytes32 adminRole) external;
+
+    function checkGovernor(address account) external view;
+
+    function checkAdmin(address account) external view;
+
+    function checkExecutor(address account) external view;
+
+    function checkRegistrar(address account) external view;
+
+    function isAdmin(address account) external view returns (bool);
+
+    function isRegistrar(address account) external view returns (bool);
 }

contracts/access/ZNSAccessController.sol

@@ -16,29 +16,46 @@ contract ZNSAccessController is AccessControlUpgradeable, ZNSRoles, IZNSAccessCo
         _grantRoleToMany(GOVERNOR_ROLE, governorAddresses);
         _grantRoleToMany(ADMIN_ROLE, adminAddresses);
 
-        // all of the governors control admins TODO AC: ???
+        // all of the governors control admins
         _setRoleAdmin(ADMIN_ROLE, GOVERNOR_ROLE);
-        // all of the governors control governors TODO AC: ???
+        // all of the governors control governors
         _setRoleAdmin(GOVERNOR_ROLE, GOVERNOR_ROLE);
-        // all of the admins control registrar TODO AC: ???
+        // all of the admins control registrar
         _setRoleAdmin(REGISTRAR_ROLE, ADMIN_ROLE);
     }
 
-    // TODO AC: should we keep this function here so that we can get standardized message?
-    //  test this function for gas usage with a standardized message vs a custom message
-    //  when using the recommended method of `hasRole`
-    function checkRole(bytes32 role, address account) external view override {
-        _checkRole(role, account);
+    // ** Access Validators **
+
+    function checkGovernor(address account) external view override {
+        _checkRole(GOVERNOR_ROLE, account);
+    }
+
+    function checkAdmin(address account) external view override {
+        _checkRole(ADMIN_ROLE, account);
+    }
+
+    function checkExecutor(address account) external view override {
+        _checkRole(EXECUTOR_ROLE, account);
+    }
+
+    function checkRegistrar(address account) external view override {
+        _checkRole(REGISTRAR_ROLE, account);
+    }
+
+    function isAdmin(address account) external view override returns (bool) {
+        return hasRole(ADMIN_ROLE, account);
+    }
+
+    function isRegistrar(address account) external view override returns (bool) {
+        return hasRole(REGISTRAR_ROLE, account);
     }
 
-    // TODO AC: is this function necessary? how often will it be used?
     function _grantRoleToMany(bytes32 role, address[] calldata addresses) internal {
         for (uint256 i = 0; i < addresses.length; i++) {
             _grantRole(role, addresses[i]);
         }
     }
 
-    // TODO AC: how safe is this?
     function setRoleAdmin(bytes32 role, bytes32 adminRole) external override onlyRole(GOVERNOR_ROLE) {
         _setRoleAdmin(role, adminRole);
     }

contracts/access/ZNSRoles.sol

@@ -2,18 +2,16 @@
 pragma solidity ^0.8.18;
 
 
+// TODO: can we declare these outside of contract, just in the ZNSAccessController file?
 abstract contract ZNSRoles {
-    // TODO AC: test getting this from AC contract vs inheriting these roles in every other contract
-    // the highest rank, only assigns Admins
+    // the highest rank, assigns Admins, new roles and Role Admins
     bytes32 public constant GOVERNOR_ROLE = keccak256("GOVERNOR_ROLE");
     // the main maintainer role, that gets access to all system functions
-    // TODO AC: should we split responsibilities in a better way?
     bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE");
     // operator can be here to future proof, if we need a new role
     // so we don't have to upgrade all contracts
-    // TODO AC: change name of this role
-    bytes32 public constant OPERATOR_ROLE = keccak256("OPERATOR_ROLE");
+    // TODO AC: possibly remove executor role?
+    bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE");
     // this role is here specifically for the ZNSEthRegistrar contract
     bytes32 public constant REGISTRAR_ROLE = keccak256("REGISTRAR_ROLE");
-    // TODO AC: what other roles do we need here?
 }

contracts/distribution/IZNSEthRegistrar.sol

@@ -18,14 +18,13 @@ interface IZNSEthRegistrar {
         address indexed registrant
     );
 
-    // TODO AC: remove ZNS from names here and in state vars
-    event ZnsRegistrySet(address znsRegistry);
+    event RegistrySet(address znsRegistry);
 
-    event ZnsTreasurySet(address znsTreasury);
+    event TreasurySet(address znsTreasury);
 
-    event ZnsDomainTokenSet(address znsDomainToken);
+    event DomainTokenSet(address znsDomainToken);
 
-    event ZnsAddressResolverSet(address znsAddressResolver);
+    event AddressResolverSet(address znsAddressResolver);
 
     function registerDomain(
         string calldata name,
@@ -36,13 +35,15 @@ interface IZNSEthRegistrar {
 
     function reclaimDomain(bytes32 domainHash) external;
 
-    function setZnsRegistry(address znsRegistry_) external;
+    function setRegistry(address znsRegistry_) external;
 
-    function setZnsTreasury(address znsTreasury_) external;
+    function setTreasury(address znsTreasury_) external;
 
-    function setZnsDomainToken(address znsDomainToken_) external;
+    function setDomainToken(address znsDomainToken_) external;
 
-    function setZnsAddressResolver(address znsAddressResolver_) external;
+    function setAddressResolver(address znsAddressResolver_) external;
 
     function setAccessController(address accessController_) external;
+
+    function getAccessController() external view returns (address);
 }

contracts/distribution/IZNSPriceOracle.sol

@@ -120,4 +120,8 @@ interface IZNSPriceOracle {
      * @param subdomainLength The length for subdomains
      */
     function setBaseLengths(uint256 rootLength, uint256 subdomainLength) external;
+
+    function setAccessController(address accessController) external;
+
+    function getAccessController() external view returns (address);
 }

contracts/distribution/IZNSTreasury.sol

@@ -2,12 +2,6 @@
 pragma solidity ^0.8.18;
 
 interface IZNSTreasury {
-    /**
-     * @notice Emitted when the znsRegistrar is updated
-     * @param znsRegistrar The address of the new registrar
-     */
-    event ZNSRegistrarSet(address znsRegistrar);
-
     /**
      * @notice Emitted when a new stake is deposited
      * @param domainHash The hash of the domain name
@@ -34,9 +28,9 @@ interface IZNSTreasury {
         uint256 indexed amount
     );
 
-    event ZnsPriceOracleSet(address znsPriceOracle);
+    event PriceOracleSet(address znsPriceOracle);
 
-    event ZnsStakingTokenSet(address znsStakingToken);
+    event StakingTokenSet(address znsStakingToken);
 
     event ZeroVaultAddressSet(address zeroVault);
 
@@ -54,4 +48,8 @@ interface IZNSTreasury {
     function setPriceOracle(address znsPriceOracle_) external;
 
     function setStakingToken(address stakingToken_) external;
+
+    function setAccessController(address accessController) external;
+
+    function getAccessController() external view returns (address);
 }