Add a bool return to _grantRole and _revokeRole

OpenZeppelin · May 17, 2023 · c41f431 · c41f431
1 parent 8de6eba
commit c41f431
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 12 deletions.
diff --git a/contracts/access/AccessControl.sol b/contracts/access/AccessControl.sol
@@ -225,10 +225,13 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
      *
      * May emit a {RoleGranted} event.
      */
-    function _grantRole(bytes32 role, address account) internal virtual {
+    function _grantRole(bytes32 role, address account) internal virtual returns (bool) {
         if (!hasRole(role, account)) {
             _roles[role].members[account] = true;
             emit RoleGranted(role, account, _msgSender());
+            return true;
+        } else {
+            return false;
         }
     }
 
@@ -239,10 +242,13 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
      *
      * May emit a {RoleRevoked} event.
      */
-    function _revokeRole(bytes32 role, address account) internal virtual {
+    function _revokeRole(bytes32 role, address account) internal virtual returns (bool) {
         if (hasRole(role, account)) {
             _roles[role].members[account] = false;
             emit RoleRevoked(role, account, _msgSender());
+            return true;
+        } else {
+            return false;
         }
     }
 }
diff --git a/contracts/access/AccessControlDefaultAdminRules.sol b/contracts/access/AccessControlDefaultAdminRules.sol
@@ -126,22 +126,22 @@ abstract contract AccessControlDefaultAdminRules is IAccessControlDefaultAdminRu
      * NOTE: Exposing this function through another mechanism may make the `DEFAULT_ADMIN_ROLE`
      * assignable again. Make sure to guarantee this is the expected behavior in your implementation.
      */
-    function _grantRole(bytes32 role, address account) internal virtual override {
+    function _grantRole(bytes32 role, address account) internal virtual override returns (bool) {
         if (role == DEFAULT_ADMIN_ROLE) {
             require(defaultAdmin() == address(0), "AccessControl: default admin already granted");
             _currentDefaultAdmin = account;
         }
-        super._grantRole(role, account);
+        return super._grantRole(role, account);
     }
 
     /**
      * @dev See {AccessControl-_revokeRole}.
      */
-    function _revokeRole(bytes32 role, address account) internal virtual override {
+    function _revokeRole(bytes32 role, address account) internal virtual override returns (bool) {
         if (role == DEFAULT_ADMIN_ROLE && account == defaultAdmin()) {
             delete _currentDefaultAdmin;
         }
-        super._revokeRole(role, account);
+        return super._revokeRole(role, account);
     }
 
     /**

diff --git a/contracts/access/AccessControlEnumerable.sol b/contracts/access/AccessControlEnumerable.sol
@@ -49,16 +49,22 @@ abstract contract AccessControlEnumerable is IAccessControlEnumerable, AccessCon
     /**
      * @dev Overload {_grantRole} to track enumerable memberships
      */
-    function _grantRole(bytes32 role, address account) internal virtual override {
-        super._grantRole(role, account);
-        _roleMembers[role].add(account);
+    function _grantRole(bytes32 role, address account) internal virtual override returns (bool) {
+        bool granted = super._grantRole(role, account);
+        if (granted) {
+            _roleMembers[role].add(account);
+        }
+        return granted;
     }
 
     /**
      * @dev Overload {_revokeRole} to track enumerable memberships
      */
-    function _revokeRole(bytes32 role, address account) internal virtual override {
-        super._revokeRole(role, account);
-        _roleMembers[role].remove(account);
+    function _revokeRole(bytes32 role, address account) internal virtual override returns (bool) {
+        bool revoked = super._revokeRole(role, account);
+        if (revoked) {
+            _roleMembers[role].remove(account);
+        }
+        return revoked;
     }
 }