Fix set role admin (#880)

* feat: add setRoleAdmin to AccessControlFacet The setRoleAdmin can be only accessed by the admin. * feat: add setRoleAdmin to AccessControl interface * test: add testSetRoleAdmin_ShouldSetAdminRoleForGivenRole test * feat: update access control for setRoleAdmin * test: fix ShouldSetAdminRoleForGivenRole and add test for revert
ubiquity · Jan 19, 2024 · ba8e447 · ba8e447
1 parent 43f8d96
commit ba8e447
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 0 deletions.
diff --git a/packages/contracts/src/dollar/access/AccessControlInternal.sol b/packages/contracts/src/dollar/access/AccessControlInternal.sol
@@ -83,6 +83,18 @@ abstract contract AccessControlInternal {
         return LibAccessControl.accessControlStorage().roles[role].adminRole;
     }
 
+    /**
+     * @notice Set admin role for a given role
+     * @param role Role to set
+     * @param adminRole role for the provided role
+     */
+    function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual {
+        LibAccessControl
+            .accessControlStorage()
+            .roles[role]
+            .adminRole = adminRole;
+    }
+
     /**
      * @notice Assigns role to a given account
      * @param role Role to assign

diff --git a/packages/contracts/src/dollar/facets/AccessControlFacet.sol b/packages/contracts/src/dollar/facets/AccessControlFacet.sol
@@ -24,6 +24,11 @@ contract AccessControlFacet is
         return _grantRole(role, account);
     }
 
+    /// @inheritdoc IAccessControl
+    function setRoleAdmin(bytes32 role, bytes32 adminRole) external onlyAdmin {
+        _setRoleAdmin(role, adminRole);
+    }
+
     /// @inheritdoc IAccessControl
     function hasRole(
         bytes32 role,

diff --git a/packages/contracts/src/dollar/interfaces/IAccessControl.sol b/packages/contracts/src/dollar/interfaces/IAccessControl.sol
@@ -22,6 +22,13 @@ interface IAccessControl {
      */
     function getRoleAdmin(bytes32 role) external view returns (bytes32);
 
+    /**
+     * @notice Sets admin role for a given role
+     * @param role Role to set
+     * @param adminRole Admin role to set for a provided role
+     */
+    function setRoleAdmin(bytes32 role, bytes32 adminRole) external;
+
     /**
      * @notice Assigns role to a given account
      * @param role Role to assign

diff --git a/packages/contracts/test/diamond/facets/AccessControlFacet.t.sol b/packages/contracts/test/diamond/facets/AccessControlFacet.t.sol
@@ -135,4 +135,25 @@ contract AccessControlFacetTest is DiamondTestSetup {
         );
         assertEq(adminRole, DEFAULT_ADMIN_ROLE);
     }
+
+    function testSetRoleAdmin_ShouldRevertWhenNotAdmin() public {
+        vm.prank(mock_sender);
+
+        vm.expectRevert("Manager: Caller is not admin");
+        accessControlFacet.setRoleAdmin(
+            DOLLAR_TOKEN_BURNER_ROLE,
+            DEFAULT_ADMIN_ROLE
+        );
+    }
+
+    function testSetRoleAdmin_ShouldSetAdminRoleForGivenRole() public {
+        bytes32 adminRole = accessControlFacet.getRoleAdmin(
+            DOLLAR_TOKEN_MINTER_ROLE
+        );
+        assertEq(adminRole, DEFAULT_ADMIN_ROLE);
+        vm.prank(admin);
+        accessControlFacet.setRoleAdmin(DOLLAR_TOKEN_MINTER_ROLE, PAUSER_ROLE);
+        adminRole = accessControlFacet.getRoleAdmin(DOLLAR_TOKEN_MINTER_ROLE);
+        assertEq(adminRole, PAUSER_ROLE);
+    }
 }