Add ERC165 interface detection to AccessControl (#2562)

Co-authored-by: Hadrien Croubois <[email protected]>

Author: frangio

CHANGELOG.md

@@ -18,6 +18,7 @@
  * Rename `UpgradeableProxy` to `ERC1967Proxy`. ([#2547](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2547))
  * `ERC777`: Optimize the gas costs of the constructor. ([#2551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2551))
  * `ERC721URIStorage`: Add a new extension that implements the `_setTokenURI` behavior as it was available in 3.4.0. ([#2555](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2555))
+ * `AccessControl`: Added ERC165 interface detection. ([#2562](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2562))
 
 ### How to upgrade from 3.x
 

contracts/access/AccessControl.sol

@@ -3,6 +3,18 @@
 pragma solidity ^0.8.0;
 
 import "../utils/Context.sol";
+import "../utils/introspection/ERC165.sol";
+
+/**
+ * @dev External interface of AccessControl declared to support ERC165 detection.
+ */
+interface IAccessControl {
+    function hasRole(bytes32 role, address account) external view returns (bool);
+    function getRoleAdmin(bytes32 role) external view returns (bytes32);
+    function grantRole(bytes32 role, address account) external;
+    function revokeRole(bytes32 role, address account) external;
+    function renounceRole(bytes32 role, address account) external;
+}
 
 /**
  * @dev Contract module that allows children to implement role-based access
@@ -42,7 +54,7 @@ import "../utils/Context.sol";
  * grant and revoke this role. Extra precautions should be taken to secure
  * accounts that have been granted it.
  */
-abstract contract AccessControl is Context {
+abstract contract AccessControl is Context, IAccessControl, ERC165 {
     struct RoleData {
         mapping (address => bool) members;
         bytes32 adminRole;
@@ -79,10 +91,18 @@ abstract contract AccessControl is Context {
      */
     event RoleRevoked(bytes32 indexed role, address indexed account, address indexed sender);
 
+    /**
+     * @dev See {IERC165-supportsInterface}.
+     */
+    function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
+        return interfaceId == type(IAccessControl).interfaceId
+            || super.supportsInterface(interfaceId);
+    }
+
     /**
      * @dev Returns `true` if `account` has been granted `role`.
      */
-    function hasRole(bytes32 role, address account) public view returns (bool) {
+    function hasRole(bytes32 role, address account) public view override returns (bool) {
         return _roles[role].members[account];
     }
 
@@ -92,7 +112,7 @@ abstract contract AccessControl is Context {
      *
      * To change a role's admin, use {_setRoleAdmin}.
      */
-    function getRoleAdmin(bytes32 role) public view returns (bytes32) {
+    function getRoleAdmin(bytes32 role) public view override returns (bytes32) {
         return _roles[role].adminRole;
     }
 
@@ -106,7 +126,7 @@ abstract contract AccessControl is Context {
      *
      * - the caller must have ``role``'s admin role.
      */
-    function grantRole(bytes32 role, address account) public virtual {
+    function grantRole(bytes32 role, address account) public virtual override {
         require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to grant");
 
         _grantRole(role, account);
@@ -121,7 +141,7 @@ abstract contract AccessControl is Context {
      *
      * - the caller must have ``role``'s admin role.
      */
-    function revokeRole(bytes32 role, address account) public virtual {
+    function revokeRole(bytes32 role, address account) public virtual override {
         require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to revoke");
 
         _revokeRole(role, account);
@@ -141,7 +161,7 @@ abstract contract AccessControl is Context {
      *
      * - the caller must be `account`.
      */
-    function renounceRole(bytes32 role, address account) public virtual {
+    function renounceRole(bytes32 role, address account) public virtual override {
         require(account == _msgSender(), "AccessControl: can only renounce roles for self");
 
         _revokeRole(role, account);

contracts/access/AccessControlEnumerable.sol

@@ -5,14 +5,30 @@ pragma solidity ^0.8.0;
 import "./AccessControl.sol";
 import "../utils/structs/EnumerableSet.sol";
 
+/**
+ * @dev External interface of AccessControlEnumerable declared to support ERC165 detection.
+ */
+interface IAccessControlEnumerable {
+    function getRoleMember(bytes32 role, uint256 index) external view returns (address);
+    function getRoleMemberCount(bytes32 role) external view returns (uint256);
+}
+
 /**
  * @dev Extension of {AccessControl} that allows enumerating the members of each role.
  */
-abstract contract AccessControlEnumerable is AccessControl {
+abstract contract AccessControlEnumerable is IAccessControlEnumerable, AccessControl {
     using EnumerableSet for EnumerableSet.AddressSet;
 
     mapping (bytes32 => EnumerableSet.AddressSet) private _roleMembers;
 
+    /**
+     * @dev See {IERC165-supportsInterface}.
+     */
+    function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
+        return interfaceId == type(IAccessControlEnumerable).interfaceId
+            || super.supportsInterface(interfaceId);
+    }
+
     /**
      * @dev Returns one of the accounts that have `role`. `index` must be a
      * value between 0 and {getRoleMemberCount}, non-inclusive.
@@ -25,15 +41,15 @@ abstract contract AccessControlEnumerable is AccessControl {
      * https://forum.openzeppelin.com/t/iterating-over-elements-on-enumerableset-in-openzeppelin-contracts/2296[forum post]
      * for more information.
      */
-    function getRoleMember(bytes32 role, uint256 index) public view returns (address) {
+    function getRoleMember(bytes32 role, uint256 index) public view override returns (address) {
         return _roleMembers[role].at(index);
     }
 
     /**
      * @dev Returns the number of accounts that have `role`. Can be used
      * together with {getRoleMember} to enumerate all bearers of a role.
      */
-    function getRoleMemberCount(bytes32 role) public view returns (uint256) {
+    function getRoleMemberCount(bytes32 role) public view override returns (uint256) {
         return _roleMembers[role].length();
     }
 

contracts/token/ERC1155/presets/ERC1155PresetMinterPauser.sol

@@ -89,6 +89,13 @@ contract ERC1155PresetMinterPauser is Context, AccessControlEnumerable, ERC1155B
         _unpause();
     }
 
+    /**
+     * @dev See {IERC165-supportsInterface}.
+     */
+    function supportsInterface(bytes4 interfaceId) public view virtual override(AccessControlEnumerable, ERC1155) returns (bool) {
+        return super.supportsInterface(interfaceId);
+    }
+
     function _beforeTokenTransfer(
         address operator,
         address from,

contracts/token/ERC721/presets/ERC721PresetMinterPauserAutoId.sol

@@ -110,7 +110,7 @@ contract ERC721PresetMinterPauserAutoId is Context, AccessControlEnumerable, ERC
     /**
      * @dev See {IERC165-supportsInterface}.
      */
-    function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721, ERC721Enumerable) returns (bool) {
+    function supportsInterface(bytes4 interfaceId) public view virtual override(AccessControlEnumerable, ERC721, ERC721Enumerable) returns (bool) {
         return super.supportsInterface(interfaceId);
     }
 }

test/access/AccessControl.behavior.js

@@ -1,11 +1,15 @@
 const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
 const { expect } = require('chai');
 
+const { shouldSupportInterfaces } = require('../utils/introspection/SupportsInterface.behavior');
+
 const DEFAULT_ADMIN_ROLE = '0x0000000000000000000000000000000000000000000000000000000000000000';
 const ROLE = web3.utils.soliditySha3('ROLE');
 const OTHER_ROLE = web3.utils.soliditySha3('OTHER_ROLE');
 
 function shouldBehaveLikeAccessControl (errorPrefix, admin, authorized, other, otherAdmin, otherAuthorized) {
+  shouldSupportInterfaces(['AccessControl']);
+
   describe('default admin', function () {
     it('deployer has default admin role', async function () {
       expect(await this.accessControl.hasRole(DEFAULT_ADMIN_ROLE, admin)).to.equal(true);
@@ -156,6 +160,8 @@ function shouldBehaveLikeAccessControl (errorPrefix, admin, authorized, other, o
 }
 
 function shouldBehaveLikeAccessControlEnumerable (errorPrefix, admin, authorized, other, otherAdmin, otherAuthorized) {
+  shouldSupportInterfaces(['AccessControlEnumerable']);
+
   describe('enumerating', function () {
     it('role bearers can be enumerated', async function () {
       await this.accessControl.grantRole(ROLE, authorized, { from: admin });

test/token/ERC1155/presets/ERC1155PresetMinterPauser.test.js

@@ -1,5 +1,6 @@
 const { BN, constants, expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
 const { ZERO_ADDRESS } = constants;
+const { shouldSupportInterfaces } = require('../../../utils/introspection/SupportsInterface.behavior');
 
 const { expect } = require('chai');
 
@@ -24,6 +25,8 @@ contract('ERC1155PresetMinterPauser', function (accounts) {
     this.token = await ERC1155PresetMinterPauser.new(uri, { from: deployer });
   });
 
+  shouldSupportInterfaces(['ERC1155', 'AccessControl', 'AccessControlEnumerable']);
+
   it('deployer has the default admin role', async function () {
     expect(await this.token.getRoleMemberCount(DEFAULT_ADMIN_ROLE)).to.be.bignumber.equal('1');
     expect(await this.token.getRoleMember(DEFAULT_ADMIN_ROLE, 0)).to.equal(deployer);

test/token/ERC721/presets/ERC721PresetMinterPauserAutoId.test.js

@@ -1,5 +1,6 @@
 const { BN, constants, expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
 const { ZERO_ADDRESS } = constants;
+const { shouldSupportInterfaces } = require('../../../utils/introspection/SupportsInterface.behavior');
 
 const { expect } = require('chai');
 
@@ -19,6 +20,8 @@ contract('ERC721PresetMinterPauserAutoId', function (accounts) {
     this.token = await ERC721PresetMinterPauserAutoId.new(name, symbol, baseURI, { from: deployer });
   });
 
+  shouldSupportInterfaces(['ERC721', 'ERC721Enumerable', 'AccessControl', 'AccessControlEnumerable']);
+
   it('token has correct name', async function () {
     expect(await this.token.name()).to.equal(name);
   });

test/utils/introspection/SupportsInterface.behavior.js

@@ -39,6 +39,17 @@ const INTERFACES = {
     'onERC1155Received(address,address,uint256,uint256,bytes)',
     'onERC1155BatchReceived(address,address,uint256[],uint256[],bytes)',
   ],
+  AccessControl: [
+    'hasRole(bytes32,address)',
+    'getRoleAdmin(bytes32)',
+    'grantRole(bytes32,address)',
+    'revokeRole(bytes32,address)',
+    'renounceRole(bytes32,address)',
+  ],
+  AccessControlEnumerable: [
+    'getRoleMember(bytes32,uint256)',
+    'getRoleMemberCount(bytes32)',
+  ],
 };
 
 const INTERFACE_IDS = {};
@@ -54,7 +65,7 @@ for (const k of Object.getOwnPropertyNames(INTERFACES)) {
 function shouldSupportInterfaces (interfaces = []) {
   describe('Contract interface', function () {
     beforeEach(function () {
-      this.contractUnderTest = this.mock || this.token || this.holder;
+      this.contractUnderTest = this.mock || this.token || this.holder || this.accessControl;
     });
 
     for (const k of interfaces) {