feat(accesscontrol): add external setRoleAdmin and grantRoleBatch/rev…

.gitignore

@@ -12,3 +12,4 @@ docs/
 
 # Dotenv file
 .env
+.gas-snapshot

src/access/AccessControl/AccessControlFacet.sol

@@ -79,6 +79,24 @@ contract AccessControlFacet {
         return s.adminRole[_role];
     }
 
+    /// @notice Sets the admin role for a role.
+    /// @param _role The role to set the admin for.
+    /// @param _adminRole The new admin role to set.
+    /// @dev Emits a {RoleAdminChanged} event.
+    /// @custom:error AccessControlUnauthorizedAccount If the caller is not the current admin of the role.
+    function setRoleAdmin(bytes32 _role, bytes32 _adminRole) external {
+        AccessControlStorage storage s = getStorage();
+        bytes32 previousAdminRole = s.adminRole[_role];
+
+        // Check if the caller is the current admin of the role.
+        if (!s.hasRole[msg.sender][previousAdminRole]) {
+            revert AccessControlUnauthorizedAccount(msg.sender, previousAdminRole);
+        }
+
+        s.adminRole[_role] = _adminRole;
+        emit RoleAdminChanged(_role, previousAdminRole, _adminRole);
+    }
+
     /// @notice Grants a role to an account.
     /// @param _role The role to grant.
     /// @param _account The account to grant the role to.
@@ -121,6 +139,56 @@ contract AccessControlFacet {
         }
     }
 
+    /// @notice Grants a role to multiple accounts in a single transaction.
+    /// @param _role The role to grant.
+    /// @param _accounts The accounts to grant the role to.
+    /// @dev Emits a {RoleGranted} event for each newly granted account.
+    /// @custom:error AccessControlUnauthorizedAccount If the caller is not the admin of the role.
+    function grantRoleBatch(bytes32 _role, address[] calldata _accounts) external {
+        AccessControlStorage storage s = getStorage();
+        bytes32 adminRole = s.adminRole[_role];
+
+        // Check if the caller is the admin of the role.
+        if (!s.hasRole[msg.sender][adminRole]) {
+            revert AccessControlUnauthorizedAccount(msg.sender, adminRole);
+        }
+
+        uint256 length = _accounts.length;
+        for (uint256 i = 0; i < length; i++) {
+            address account = _accounts[i];
+            bool _hasRole = s.hasRole[account][_role];
+            if (!_hasRole) {
+                s.hasRole[account][_role] = true;
+                emit RoleGranted(_role, account, msg.sender);
+            }
+        }
+    }
+
+    /// @notice Revokes a role from multiple accounts in a single transaction.
+    /// @param _role The role to revoke.
+    /// @param _accounts The accounts to revoke the role from.
+    /// @dev Emits a {RoleRevoked} event for each account the role is revoked from.
+    /// @custom:error AccessControlUnauthorizedAccount If the caller is not the admin of the role.
+    function revokeRoleBatch(bytes32 _role, address[] calldata _accounts) external {
+        AccessControlStorage storage s = getStorage();
+        bytes32 adminRole = s.adminRole[_role];
+
+        // Check if the caller is the admin of the role.
+        if (!s.hasRole[msg.sender][adminRole]) {
+            revert AccessControlUnauthorizedAccount(msg.sender, adminRole);
+        }
+
+        uint256 length = _accounts.length;
+        for (uint256 i = 0; i < length; i++) {
+            address account = _accounts[i];
+            bool _hasRole = s.hasRole[account][_role];
+            if (_hasRole) {
+                s.hasRole[account][_role] = false;
+                emit RoleRevoked(_role, account, msg.sender);
+            }
+        }
+    }
+
     /// @notice Renounces a role from the caller.
     /// @param _role The role to renounce.
     /// @param _account The account to renounce the role from.

test/access/AccessControl/AccessControlFacet.t.sol

@@ -120,6 +120,203 @@ contract AccessControlFacetTest is Test {
         assertEq(accessControl.getRoleAdmin(MINTER_ROLE), PAUSER_ROLE);
     }
 
+    // ============================================
+    // setRoleAdmin Tests (external)
+    // ============================================
+
+    function test_SetRoleAdmin_SucceedsWhenCallerIsCurrentAdmin() public {
+        // DEFAULT_ADMIN_ROLE is current admin for new roles
+        vm.expectEmit(true, true, true, true);
+        emit RoleAdminChanged(MINTER_ROLE, DEFAULT_ADMIN_ROLE, PAUSER_ROLE);
+
+        vm.prank(ADMIN);
+        accessControl.setRoleAdmin(MINTER_ROLE, PAUSER_ROLE);
+
+        assertEq(accessControl.getRoleAdmin(MINTER_ROLE), PAUSER_ROLE);
+    }
+
+    function test_RevertWhen_SetRoleAdmin_CallerIsNotCurrentAdmin() public {
+        // Set current admin to PAUSER_ROLE
+        accessControl.forceSetRoleAdmin(MINTER_ROLE, PAUSER_ROLE);
+
+        // ADMIN has DEFAULT_ADMIN_ROLE but not PAUSER_ROLE
+        vm.expectRevert(
+            abi.encodeWithSelector(AccessControlFacet.AccessControlUnauthorizedAccount.selector, ADMIN, PAUSER_ROLE)
+        );
+        vm.prank(ADMIN);
+        accessControl.setRoleAdmin(MINTER_ROLE, UPGRADER_ROLE);
+    }
+
+    // ============================================
+    // Batch Grant/Revoke Tests
+    // ============================================
+
+    function test_GrantRoleBatch_SucceedsAndEmitsPerNewGrant() public {
+        address[] memory accounts = new address[](3);
+        accounts[0] = ALICE;
+        accounts[1] = BOB;
+        accounts[2] = CHARLIE;
+
+        vm.startPrank(ADMIN);
+        // Expect three RoleGranted events
+        vm.expectEmit(true, true, true, true);
+        emit RoleGranted(MINTER_ROLE, ALICE, ADMIN);
+        vm.expectEmit(true, true, true, true);
+        emit RoleGranted(MINTER_ROLE, BOB, ADMIN);
+        vm.expectEmit(true, true, true, true);
+        emit RoleGranted(MINTER_ROLE, CHARLIE, ADMIN);
+        accessControl.grantRoleBatch(MINTER_ROLE, accounts);
+        vm.stopPrank();
+
+        assertTrue(accessControl.hasRole(MINTER_ROLE, ALICE));
+        assertTrue(accessControl.hasRole(MINTER_ROLE, BOB));
+        assertTrue(accessControl.hasRole(MINTER_ROLE, CHARLIE));
+    }
+
+    function test_GrantRoleBatch_SkipsAlreadyGrantedWithoutExtraEvents() public {
+        // Pre-grant ALICE
+        vm.prank(ADMIN);
+        accessControl.grantRole(MINTER_ROLE, ALICE);
+
+        address[] memory accounts = new address[](3);
+        accounts[0] = ALICE; // already granted
+        accounts[1] = BOB;
+        accounts[2] = CHARLIE;
+
+        vm.startPrank(ADMIN);
+        // Expect only two events for new grants
+        vm.expectEmit(true, true, true, true);
+        emit RoleGranted(MINTER_ROLE, BOB, ADMIN);
+        vm.expectEmit(true, true, true, true);
+        emit RoleGranted(MINTER_ROLE, CHARLIE, ADMIN);
+        accessControl.grantRoleBatch(MINTER_ROLE, accounts);
+        vm.stopPrank();
+
+        assertTrue(accessControl.hasRole(MINTER_ROLE, ALICE));
+        assertTrue(accessControl.hasRole(MINTER_ROLE, BOB));
+        assertTrue(accessControl.hasRole(MINTER_ROLE, CHARLIE));
+    }
+
+    function test_RevertWhen_GrantRoleBatch_CallerIsNotAdmin() public {
+        address[] memory accounts = new address[](2);
+        accounts[0] = ALICE;
+        accounts[1] = BOB;
+
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                AccessControlFacet.AccessControlUnauthorizedAccount.selector, ALICE, DEFAULT_ADMIN_ROLE
+            )
+        );
+        vm.prank(ALICE);
+        accessControl.grantRoleBatch(MINTER_ROLE, accounts);
+    }
+
+    function test_RevokeRoleBatch_SucceedsAndEmitsPerRevocation() public {
+        // Setup grants
+        vm.startPrank(ADMIN);
+        accessControl.grantRole(MINTER_ROLE, ALICE);
+        accessControl.grantRole(MINTER_ROLE, BOB);
+        accessControl.grantRole(MINTER_ROLE, CHARLIE);
+        vm.stopPrank();
+
+        address[] memory accounts = new address[](3);
+        accounts[0] = ALICE;
+        accounts[1] = BOB;
+        accounts[2] = CHARLIE;
+
+        vm.startPrank(ADMIN);
+        vm.expectEmit(true, true, true, true);
+        emit RoleRevoked(MINTER_ROLE, ALICE, ADMIN);
+        vm.expectEmit(true, true, true, true);
+        emit RoleRevoked(MINTER_ROLE, BOB, ADMIN);
+        vm.expectEmit(true, true, true, true);
+        emit RoleRevoked(MINTER_ROLE, CHARLIE, ADMIN);
+        accessControl.revokeRoleBatch(MINTER_ROLE, accounts);
+        vm.stopPrank();
+
+        assertFalse(accessControl.hasRole(MINTER_ROLE, ALICE));
+        assertFalse(accessControl.hasRole(MINTER_ROLE, BOB));
+        assertFalse(accessControl.hasRole(MINTER_ROLE, CHARLIE));
+    }
+
+    function test_RevokeRoleBatch_SkipsNotGrantedWithoutExtraEvents() public {
+        // Only ALICE has the role
+        vm.prank(ADMIN);
+        accessControl.grantRole(MINTER_ROLE, ALICE);
+
+        address[] memory accounts = new address[](3);
+        accounts[0] = ALICE; // granted
+        accounts[1] = BOB; // not granted
+        accounts[2] = CHARLIE; // not granted
+
+        vm.startPrank(ADMIN);
+        vm.expectEmit(true, true, true, true);
+        emit RoleRevoked(MINTER_ROLE, ALICE, ADMIN);
+        accessControl.revokeRoleBatch(MINTER_ROLE, accounts);
+        vm.stopPrank();
+
+        assertFalse(accessControl.hasRole(MINTER_ROLE, ALICE));
+        assertFalse(accessControl.hasRole(MINTER_ROLE, BOB));
+        assertFalse(accessControl.hasRole(MINTER_ROLE, CHARLIE));
+    }
+
+    function test_RevertWhen_RevokeRoleBatch_CallerIsNotAdmin() public {
+        // Setup grants
+        vm.prank(ADMIN);
+        accessControl.grantRole(MINTER_ROLE, ALICE);
+
+        address[] memory accounts = new address[](1);
+        accounts[0] = ALICE;
+
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                AccessControlFacet.AccessControlUnauthorizedAccount.selector, BOB, DEFAULT_ADMIN_ROLE
+            )
+        );
+        vm.prank(BOB);
+        accessControl.revokeRoleBatch(MINTER_ROLE, accounts);
+    }
+
+    function test_GrantRoleBatch_SucceedsWithEmptyArray() public {
+        address[] memory accounts = new address[](0);
+
+        // Should just succeed with no reverts and no events
+        vm.prank(ADMIN);
+        accessControl.grantRoleBatch(MINTER_ROLE, accounts);
+    }
+
+    function test_RevokeRoleBatch_SucceedsWithEmptyArray() public {
+        address[] memory accounts = new address[](0);
+
+        // Should just succeed with no reverts and no events
+        vm.prank(ADMIN);
+        accessControl.revokeRoleBatch(MINTER_ROLE, accounts);
+    }
+
+    function test_DelegatedAdminCanExerciseAdminPowers() public {
+        // === Arrange ===
+        vm.startPrank(ADMIN);
+        accessControl.grantRole(MINTER_ROLE, BOB);
+        accessControl.grantRole(PAUSER_ROLE, ALICE);
+        accessControl.setRoleAdmin(MINTER_ROLE, PAUSER_ROLE);
+        vm.stopPrank();
+
+        // Assert the setup is correct before acting
+        assertTrue(accessControl.hasRole(MINTER_ROLE, BOB));
+        assertTrue(accessControl.hasRole(PAUSER_ROLE, ALICE));
+
+        // === Act ===
+        // Expect the event to be emitted
+        vm.expectEmit(true, true, true, true);
+        emit RoleRevoked(MINTER_ROLE, BOB, ALICE);
+
+        vm.prank(ALICE);
+        accessControl.revokeRole(MINTER_ROLE, BOB);
+
+        // === Assert ===
+        assertFalse(accessControl.hasRole(MINTER_ROLE, BOB));
+    }
+
     function test_GetRoleAdmin_DefaultAdminRoleAdminIsItself() public view {
         assertEq(accessControl.getRoleAdmin(DEFAULT_ADMIN_ROLE), DEFAULT_ADMIN_ROLE);
     }