From 4dd27098ecf98de6addf552d85eacb7b3e8bf500 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Wed, 3 Feb 2021 12:27:02 +0100
Subject: [PATCH 01/16] Add a light variation of the AccessControl contract

---
 contracts/access/AccessControl.sol         | 117 +------------
 contracts/access/AccessControlLight.sol    | 189 +++++++++++++++++++++
 contracts/mocks/AccessControlLightMock.sol |  15 ++
 test/access/AccessControlLight.test.js     | 165 ++++++++++++++++++
 4 files changed, 377 insertions(+), 109 deletions(-)
 create mode 100644 contracts/access/AccessControlLight.sol
 create mode 100644 contracts/mocks/AccessControlLightMock.sol
 create mode 100644 test/access/AccessControlLight.test.js

diff --git a/contracts/access/AccessControl.sol b/contracts/access/AccessControl.sol
index 1871e33a70b..f8cdf7e0313 100644
--- a/contracts/access/AccessControl.sol
+++ b/contracts/access/AccessControl.sol
@@ -2,13 +2,12 @@
 
 pragma solidity ^0.8.0;
 
+import "./AccessControlLight.sol";
 import "../utils/EnumerableSet.sol";
-import "../utils/Address.sol";
-import "../utils/Context.sol";
 
 /**
  * @dev Contract module that allows children to implement role-based access
- * control mechanisms.
+ * control mechanisms. Full, enumerable, version.
  *
  * Roles are referred to by their `bytes32` identifier. These should be exposed
  * in the external API and be unique. The best way to achieve this is by
@@ -41,7 +40,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 AccessControlLight {
     using EnumerableSet for EnumerableSet.AddressSet;
     using Address for address;
 
@@ -52,39 +51,10 @@ abstract contract AccessControl is Context {
 
     mapping (bytes32 => RoleData) private _roles;
 
-    bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00;
-
-    /**
-     * @dev Emitted when `newAdminRole` is set as ``role``'s admin role, replacing `previousAdminRole`
-     *
-     * `DEFAULT_ADMIN_ROLE` is the starting admin for all roles, despite
-     * {RoleAdminChanged} not being emitted signaling this.
-     *
-     * _Available since v3.1._
-     */
-    event RoleAdminChanged(bytes32 indexed role, bytes32 indexed previousAdminRole, bytes32 indexed newAdminRole);
-
-    /**
-     * @dev Emitted when `account` is granted `role`.
-     *
-     * `sender` is the account that originated the contract call, an admin role
-     * bearer except when using {_setupRole}.
-     */
-    event RoleGranted(bytes32 indexed role, address indexed account, address indexed sender);
-
-    /**
-     * @dev Emitted when `account` is revoked `role`.
-     *
-     * `sender` is the account that originated the contract call:
-     *   - if using `revokeRole`, it is the admin role bearer
-     *   - if using `renounceRole`, it is the role bearer (i.e. `account`)
-     */
-    event RoleRevoked(bytes32 indexed role, address indexed account, address indexed sender);
-
     /**
      * @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 virtual override returns (bool) {
         return _roles[role].members.contains(account);
     }
 
@@ -118,98 +88,27 @@ 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 virtual override returns (bytes32) {
         return _roles[role].adminRole;
     }
 
-    /**
-     * @dev Grants `role` to `account`.
-     *
-     * If `account` had not been already granted `role`, emits a {RoleGranted}
-     * event.
-     *
-     * Requirements:
-     *
-     * - the caller must have ``role``'s admin role.
-     */
-    function grantRole(bytes32 role, address account) public virtual {
-        require(hasRole(_roles[role].adminRole, _msgSender()), "AccessControl: sender must be an admin to grant");
-
-        _grantRole(role, account);
-    }
-
-    /**
-     * @dev Revokes `role` from `account`.
-     *
-     * If `account` had been granted `role`, emits a {RoleRevoked} event.
-     *
-     * Requirements:
-     *
-     * - the caller must have ``role``'s admin role.
-     */
-    function revokeRole(bytes32 role, address account) public virtual {
-        require(hasRole(_roles[role].adminRole, _msgSender()), "AccessControl: sender must be an admin to revoke");
-
-        _revokeRole(role, account);
-    }
-
-    /**
-     * @dev Revokes `role` from the calling account.
-     *
-     * Roles are often managed via {grantRole} and {revokeRole}: this function's
-     * purpose is to provide a mechanism for accounts to lose their privileges
-     * if they are compromised (such as when a trusted device is misplaced).
-     *
-     * If the calling account had been granted `role`, emits a {RoleRevoked}
-     * event.
-     *
-     * Requirements:
-     *
-     * - the caller must be `account`.
-     */
-    function renounceRole(bytes32 role, address account) public virtual {
-        require(account == _msgSender(), "AccessControl: can only renounce roles for self");
-
-        _revokeRole(role, account);
-    }
-
-    /**
-     * @dev Grants `role` to `account`.
-     *
-     * If `account` had not been already granted `role`, emits a {RoleGranted}
-     * event. Note that unlike {grantRole}, this function doesn't perform any
-     * checks on the calling account.
-     *
-     * [WARNING]
-     * ====
-     * This function should only be called from the constructor when setting
-     * up the initial roles for the system.
-     *
-     * Using this function in any other way is effectively circumventing the admin
-     * system imposed by {AccessControl}.
-     * ====
-     */
-    function _setupRole(bytes32 role, address account) internal virtual {
-        _grantRole(role, account);
-    }
-
     /**
      * @dev Sets `adminRole` as ``role``'s admin role.
      *
      * Emits a {RoleAdminChanged} event.
      */
-    function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual {
+    function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual override {
         emit RoleAdminChanged(role, _roles[role].adminRole, adminRole);
         _roles[role].adminRole = adminRole;
     }
 
-    function _grantRole(bytes32 role, address account) private {
+    function _grantRole(bytes32 role, address account) internal virtual override {
         if (_roles[role].members.add(account)) {
             emit RoleGranted(role, account, _msgSender());
         }
     }
 
-    function _revokeRole(bytes32 role, address account) private {
+    function _revokeRole(bytes32 role, address account) internal virtual override {
         if (_roles[role].members.remove(account)) {
             emit RoleRevoked(role, account, _msgSender());
         }
diff --git a/contracts/access/AccessControlLight.sol b/contracts/access/AccessControlLight.sol
new file mode 100644
index 00000000000..ee36650420d
--- /dev/null
+++ b/contracts/access/AccessControlLight.sol
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "../utils/Address.sol";
+import "../utils/Context.sol";
+
+/**
+ * @dev Contract module that allows children to implement role-based access
+ * control mechanisms. Light, not enumerable, version.
+ *
+ * Roles are referred to by their `bytes32` identifier. These should be exposed
+ * in the external API and be unique. The best way to achieve this is by
+ * using `public constant` hash digests:
+ *
+ * ```
+ * bytes32 public constant MY_ROLE = keccak256("MY_ROLE");
+ * ```
+ *
+ * Roles can be used to represent a set of permissions. To restrict access to a
+ * function call, use {hasRole}:
+ *
+ * ```
+ * function foo() public {
+ *     require(hasRole(MY_ROLE, msg.sender));
+ *     ...
+ * }
+ * ```
+ *
+ * Roles can be granted and revoked dynamically via the {grantRole} and
+ * {revokeRole} functions. Each role has an associated admin role, and only
+ * accounts that have a role's admin role can call {grantRole} and {revokeRole}.
+ *
+ * By default, the admin role for all roles is `DEFAULT_ADMIN_ROLE`, which means
+ * that only accounts with this role will be able to grant or revoke other
+ * roles. More complex role relationships can be created by using
+ * {_setRoleAdmin}.
+ *
+ * WARNING: The `DEFAULT_ADMIN_ROLE` is also its own admin: it has permission to
+ * grant and revoke this role. Extra precautions should be taken to secure
+ * accounts that have been granted it.
+ */
+abstract contract AccessControlLight is Context {
+    using Address for address;
+
+    mapping (bytes32 => bytes32) private _admins;
+    mapping (bytes32 => mapping (address => bool)) private _members;
+
+    bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00;
+
+    /**
+     * @dev Emitted when `newAdminRole` is set as ``role``'s admin role, replacing `previousAdminRole`
+     *
+     * `DEFAULT_ADMIN_ROLE` is the starting admin for all roles, despite
+     * {RoleAdminChanged} not being emitted signaling this.
+     *
+     * _Available since v3.1._
+     */
+    event RoleAdminChanged(bytes32 indexed role, bytes32 indexed previousAdminRole, bytes32 indexed newAdminRole);
+
+    /**
+     * @dev Emitted when `account` is granted `role`.
+     *
+     * `sender` is the account that originated the contract call, an admin role
+     * bearer except when using {_setupRole}.
+     */
+    event RoleGranted(bytes32 indexed role, address indexed account, address indexed sender);
+
+    /**
+     * @dev Emitted when `account` is revoked `role`.
+     *
+     * `sender` is the account that originated the contract call:
+     *   - if using `revokeRole`, it is the admin role bearer
+     *   - if using `renounceRole`, it is the role bearer (i.e. `account`)
+     */
+    event RoleRevoked(bytes32 indexed role, address indexed account, address indexed sender);
+
+    /**
+     * @dev Returns `true` if `account` has been granted `role`.
+     */
+    function hasRole(bytes32 role, address account) public view virtual returns (bool) {
+        return _members[role][account];
+    }
+
+    /**
+     * @dev Returns the admin role that controls `role`. See {grantRole} and
+     * {revokeRole}.
+     *
+     * To change a role's admin, use {_setRoleAdmin}.
+     */
+    function getRoleAdmin(bytes32 role) public view virtual returns (bytes32) {
+        return _admins[role];
+    }
+
+    /**
+     * @dev Grants `role` to `account`.
+     *
+     * If `account` had not been already granted `role`, emits a {RoleGranted}
+     * event.
+     *
+     * Requirements:
+     *
+     * - the caller must have ``role``'s admin role.
+     */
+    function grantRole(bytes32 role, address account) public virtual {
+        require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to grant");
+
+        _grantRole(role, account);
+    }
+
+    /**
+     * @dev Revokes `role` from `account`.
+     *
+     * If `account` had been granted `role`, emits a {RoleRevoked} event.
+     *
+     * Requirements:
+     *
+     * - the caller must have ``role``'s admin role.
+     */
+    function revokeRole(bytes32 role, address account) public virtual {
+        require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to revoke");
+
+        _revokeRole(role, account);
+    }
+
+    /**
+     * @dev Revokes `role` from the calling account.
+     *
+     * Roles are often managed via {grantRole} and {revokeRole}: this function's
+     * purpose is to provide a mechanism for accounts to lose their privileges
+     * if they are compromised (such as when a trusted device is misplaced).
+     *
+     * If the calling account had been granted `role`, emits a {RoleRevoked}
+     * event.
+     *
+     * Requirements:
+     *
+     * - the caller must be `account`.
+     */
+    function renounceRole(bytes32 role, address account) public virtual {
+        require(account == _msgSender(), "AccessControl: can only renounce roles for self");
+
+        _revokeRole(role, account);
+    }
+
+    /**
+     * @dev Grants `role` to `account`.
+     *
+     * If `account` had not been already granted `role`, emits a {RoleGranted}
+     * event. Note that unlike {grantRole}, this function doesn't perform any
+     * checks on the calling account.
+     *
+     * [WARNING]
+     * ====
+     * This function should only be called from the constructor when setting
+     * up the initial roles for the system.
+     *
+     * Using this function in any other way is effectively circumventing the admin
+     * system imposed by {AccessControl}.
+     * ====
+     */
+    function _setupRole(bytes32 role, address account) internal virtual {
+        _grantRole(role, account);
+    }
+
+    /**
+     * @dev Sets `adminRole` as ``role``'s admin role.
+     *
+     * Emits a {RoleAdminChanged} event.
+     */
+    function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual {
+        emit RoleAdminChanged(role, getRoleAdmin(role), adminRole);
+        _admins[role] = adminRole;
+    }
+
+    function _grantRole(bytes32 role, address account) internal virtual {
+        if (!_members[role][account]) {
+            _members[role][account] = true;
+            emit RoleGranted(role, account, _msgSender());
+        }
+    }
+
+    function _revokeRole(bytes32 role, address account) internal virtual {
+        if (_members[role][account]) {
+            _members[role][account] = false;
+            emit RoleRevoked(role, account, _msgSender());
+        }
+    }
+}
diff --git a/contracts/mocks/AccessControlLightMock.sol b/contracts/mocks/AccessControlLightMock.sol
new file mode 100644
index 00000000000..08eca0b52ff
--- /dev/null
+++ b/contracts/mocks/AccessControlLightMock.sol
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "../access/AccessControlLight.sol";
+
+contract AccessControlLightMock is AccessControlLight {
+    constructor() {
+        _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
+    }
+
+    function setRoleAdmin(bytes32 roleId, bytes32 adminRoleId) public {
+        _setRoleAdmin(roleId, adminRoleId);
+    }
+}
diff --git a/test/access/AccessControlLight.test.js b/test/access/AccessControlLight.test.js
new file mode 100644
index 00000000000..8baefe124d4
--- /dev/null
+++ b/test/access/AccessControlLight.test.js
@@ -0,0 +1,165 @@
+const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
+
+const { expect } = require('chai');
+
+const AccessControlMock = artifacts.require('AccessControlLightMock');
+
+contract('AccessControl', function (accounts) {
+  const [ admin, authorized, otherAuthorized, other, otherAdmin ] = accounts;
+
+  const DEFAULT_ADMIN_ROLE = '0x0000000000000000000000000000000000000000000000000000000000000000';
+  const ROLE = web3.utils.soliditySha3('ROLE');
+  const OTHER_ROLE = web3.utils.soliditySha3('OTHER_ROLE');
+
+  beforeEach(async function () {
+    this.accessControl = await AccessControlMock.new({ from: admin });
+  });
+
+  describe('default admin', function () {
+    it('deployer has default admin role', async function () {
+      expect(await this.accessControl.hasRole(DEFAULT_ADMIN_ROLE, admin)).to.equal(true);
+    });
+
+    it('other roles\'s admin is the default admin role', async function () {
+      expect(await this.accessControl.getRoleAdmin(ROLE)).to.equal(DEFAULT_ADMIN_ROLE);
+    });
+
+    it('default admin role\'s admin is itself', async function () {
+      expect(await this.accessControl.getRoleAdmin(DEFAULT_ADMIN_ROLE)).to.equal(DEFAULT_ADMIN_ROLE);
+    });
+  });
+
+  describe('granting', function () {
+    it('admin can grant role to other accounts', async function () {
+      const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: admin });
+      expectEvent(receipt, 'RoleGranted', { account: authorized, role: ROLE, sender: admin });
+
+      expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(true);
+    });
+
+    it('non-admin cannot grant role to other accounts', async function () {
+      await expectRevert(
+        this.accessControl.grantRole(ROLE, authorized, { from: other }),
+        'AccessControl: sender must be an admin to grant',
+      );
+    });
+
+    it('accounts can be granted a role multiple times', async function () {
+      await this.accessControl.grantRole(ROLE, authorized, { from: admin });
+      const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: admin });
+      expectEvent.notEmitted(receipt, 'RoleGranted');
+    });
+  });
+
+  describe('revoking', function () {
+    it('roles that are not had can be revoked', async function () {
+      expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false);
+
+      const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
+      expectEvent.notEmitted(receipt, 'RoleRevoked');
+    });
+
+    context('with granted role', function () {
+      beforeEach(async function () {
+        await this.accessControl.grantRole(ROLE, authorized, { from: admin });
+      });
+
+      it('admin can revoke role', async function () {
+        const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
+        expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: admin });
+
+        expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false);
+      });
+
+      it('non-admin cannot revoke role', async function () {
+        await expectRevert(
+          this.accessControl.revokeRole(ROLE, authorized, { from: other }),
+          'AccessControl: sender must be an admin to revoke',
+        );
+      });
+
+      it('a role can be revoked multiple times', async function () {
+        await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
+
+        const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
+        expectEvent.notEmitted(receipt, 'RoleRevoked');
+      });
+    });
+  });
+
+  describe('renouncing', function () {
+    it('roles that are not had can be renounced', async function () {
+      const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
+      expectEvent.notEmitted(receipt, 'RoleRevoked');
+    });
+
+    context('with granted role', function () {
+      beforeEach(async function () {
+        await this.accessControl.grantRole(ROLE, authorized, { from: admin });
+      });
+
+      it('bearer can renounce role', async function () {
+        const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
+        expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: authorized });
+
+        expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false);
+      });
+
+      it('only the sender can renounce their roles', async function () {
+        await expectRevert(
+          this.accessControl.renounceRole(ROLE, authorized, { from: admin }),
+          'AccessControl: can only renounce roles for self',
+        );
+      });
+
+      it('a role can be renounced multiple times', async function () {
+        await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
+
+        const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
+        expectEvent.notEmitted(receipt, 'RoleRevoked');
+      });
+    });
+  });
+
+  describe('setting role admin', function () {
+    beforeEach(async function () {
+      const receipt = await this.accessControl.setRoleAdmin(ROLE, OTHER_ROLE);
+      expectEvent(receipt, 'RoleAdminChanged', {
+        role: ROLE,
+        previousAdminRole: DEFAULT_ADMIN_ROLE,
+        newAdminRole: OTHER_ROLE,
+      });
+
+      await this.accessControl.grantRole(OTHER_ROLE, otherAdmin, { from: admin });
+    });
+
+    it('a role\'s admin role can be changed', async function () {
+      expect(await this.accessControl.getRoleAdmin(ROLE)).to.equal(OTHER_ROLE);
+    });
+
+    it('the new admin can grant roles', async function () {
+      const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: otherAdmin });
+      expectEvent(receipt, 'RoleGranted', { account: authorized, role: ROLE, sender: otherAdmin });
+    });
+
+    it('the new admin can revoke roles', async function () {
+      await this.accessControl.grantRole(ROLE, authorized, { from: otherAdmin });
+      const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: otherAdmin });
+      expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: otherAdmin });
+    });
+
+    it('a role\'s previous admins no longer grant roles', async function () {
+      await expectRevert(
+        this.accessControl.grantRole(ROLE, authorized, { from: admin }),
+        'AccessControl: sender must be an admin to grant',
+      );
+    });
+
+    it('a role\'s previous admins no longer revoke roles', async function () {
+      await expectRevert(
+        this.accessControl.revokeRole(ROLE, authorized, { from: admin }),
+        'AccessControl: sender must be an admin to revoke',
+      );
+    });
+  });
+});

From db1ac1ea7e02ff03e31b23190f25f14cb8c14e7c Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Wed, 3 Feb 2021 12:28:35 +0100
Subject: [PATCH 02/16] remove unused library

---
 contracts/access/AccessControl.sol      | 1 -
 contracts/access/AccessControlLight.sol | 5 +----
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/contracts/access/AccessControl.sol b/contracts/access/AccessControl.sol
index f8cdf7e0313..96aabe00446 100644
--- a/contracts/access/AccessControl.sol
+++ b/contracts/access/AccessControl.sol
@@ -42,7 +42,6 @@ import "../utils/EnumerableSet.sol";
  */
 abstract contract AccessControl is AccessControlLight {
     using EnumerableSet for EnumerableSet.AddressSet;
-    using Address for address;
 
     struct RoleData {
         EnumerableSet.AddressSet members;
diff --git a/contracts/access/AccessControlLight.sol b/contracts/access/AccessControlLight.sol
index ee36650420d..b0086993452 100644
--- a/contracts/access/AccessControlLight.sol
+++ b/contracts/access/AccessControlLight.sol
@@ -2,7 +2,6 @@
 
 pragma solidity ^0.8.0;
 
-import "../utils/Address.sol";
 import "../utils/Context.sol";
 
 /**
@@ -41,13 +40,11 @@ import "../utils/Context.sol";
  * accounts that have been granted it.
  */
 abstract contract AccessControlLight is Context {
-    using Address for address;
+    bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00;
 
     mapping (bytes32 => bytes32) private _admins;
     mapping (bytes32 => mapping (address => bool)) private _members;
 
-    bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00;
-
     /**
      * @dev Emitted when `newAdminRole` is set as ``role``'s admin role, replacing `previousAdminRole`
      *

From 293b6047847461c4c79a93a83f4f9bae15f99662 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Fri, 5 Feb 2021 14:48:28 +0100
Subject: [PATCH 03/16] rename AccessControl & AccessControlLight into
 AccessControlEnumerable & AccessControl

---
 contracts/access/AccessControl.sol            | 147 ++++++++++----
 contracts/access/AccessControlEnumerable.sol  | 115 +++++++++++
 contracts/access/AccessControlLight.sol       | 186 ------------------
 contracts/mocks/AccessControlLightMock.sol    |   4 +-
 .../presets/ERC1155PresetMinterPauser.sol     |   4 +-
 contracts/presets/ERC20PresetMinterPauser.sol |   4 +-
 .../ERC721PresetMinterPauserAutoId.sol        |   4 +-
 test/access/AccessControl.test.js             |  17 --
 ...est.js => AccessControlEnumerable.test.js} |  21 +-
 9 files changed, 251 insertions(+), 251 deletions(-)
 create mode 100644 contracts/access/AccessControlEnumerable.sol
 delete mode 100644 contracts/access/AccessControlLight.sol
 rename test/access/{AccessControlLight.test.js => AccessControlEnumerable.test.js} (89%)

diff --git a/contracts/access/AccessControl.sol b/contracts/access/AccessControl.sol
index 96aabe00446..ce90d014fab 100644
--- a/contracts/access/AccessControl.sol
+++ b/contracts/access/AccessControl.sol
@@ -2,12 +2,11 @@
 
 pragma solidity ^0.8.0;
 
-import "./AccessControlLight.sol";
-import "../utils/EnumerableSet.sol";
+import "../utils/Context.sol";
 
 /**
  * @dev Contract module that allows children to implement role-based access
- * control mechanisms. Full, enumerable, version.
+ * control mechanisms. Light, not enumerable, version.
  *
  * Roles are referred to by their `bytes32` identifier. These should be exposed
  * in the external API and be unique. The best way to achieve this is by
@@ -40,55 +39,125 @@ import "../utils/EnumerableSet.sol";
  * grant and revoke this role. Extra precautions should be taken to secure
  * accounts that have been granted it.
  */
-abstract contract AccessControl is AccessControlLight {
-    using EnumerableSet for EnumerableSet.AddressSet;
+abstract contract AccessControl is Context {
+    bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00;
 
-    struct RoleData {
-        EnumerableSet.AddressSet members;
-        bytes32 adminRole;
-    }
+    mapping (bytes32 => bytes32) private _admins;
+    mapping (bytes32 => mapping (address => bool)) private _members;
 
-    mapping (bytes32 => RoleData) private _roles;
+    /**
+     * @dev Emitted when `newAdminRole` is set as ``role``'s admin role, replacing `previousAdminRole`
+     *
+     * `DEFAULT_ADMIN_ROLE` is the starting admin for all roles, despite
+     * {RoleAdminChanged} not being emitted signaling this.
+     *
+     * _Available since v3.1._
+     */
+    event RoleAdminChanged(bytes32 indexed role, bytes32 indexed previousAdminRole, bytes32 indexed newAdminRole);
+
+    /**
+     * @dev Emitted when `account` is granted `role`.
+     *
+     * `sender` is the account that originated the contract call, an admin role
+     * bearer except when using {_setupRole}.
+     */
+    event RoleGranted(bytes32 indexed role, address indexed account, address indexed sender);
+
+    /**
+     * @dev Emitted when `account` is revoked `role`.
+     *
+     * `sender` is the account that originated the contract call:
+     *   - if using `revokeRole`, it is the admin role bearer
+     *   - if using `renounceRole`, it is the role bearer (i.e. `account`)
+     */
+    event RoleRevoked(bytes32 indexed role, address indexed account, address indexed sender);
 
     /**
      * @dev Returns `true` if `account` has been granted `role`.
      */
-    function hasRole(bytes32 role, address account) public view virtual override returns (bool) {
-        return _roles[role].members.contains(account);
+    function hasRole(bytes32 role, address account) public view virtual returns (bool) {
+        return _members[role][account];
     }
 
     /**
-     * @dev Returns the number of accounts that have `role`. Can be used
-     * together with {getRoleMember} to enumerate all bearers of a role.
+     * @dev Returns the admin role that controls `role`. See {grantRole} and
+     * {revokeRole}.
+     *
+     * To change a role's admin, use {_setRoleAdmin}.
      */
-    function getRoleMemberCount(bytes32 role) public view returns (uint256) {
-        return _roles[role].members.length();
+    function getRoleAdmin(bytes32 role) public view virtual returns (bytes32) {
+        return _admins[role];
     }
 
     /**
-     * @dev Returns one of the accounts that have `role`. `index` must be a
-     * value between 0 and {getRoleMemberCount}, non-inclusive.
+     * @dev Grants `role` to `account`.
+     *
+     * If `account` had not been already granted `role`, emits a {RoleGranted}
+     * event.
      *
-     * Role bearers are not sorted in any particular way, and their ordering may
-     * change at any point.
+     * Requirements:
      *
-     * WARNING: When using {getRoleMember} and {getRoleMemberCount}, make sure
-     * you perform all queries on the same block. See the following
-     * https://forum.openzeppelin.com/t/iterating-over-elements-on-enumerableset-in-openzeppelin-contracts/2296[forum post]
-     * for more information.
+     * - the caller must have ``role``'s admin role.
      */
-    function getRoleMember(bytes32 role, uint256 index) public view returns (address) {
-        return _roles[role].members.at(index);
+    function grantRole(bytes32 role, address account) public virtual {
+        require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to grant");
+
+        _grantRole(role, account);
     }
 
     /**
-     * @dev Returns the admin role that controls `role`. See {grantRole} and
-     * {revokeRole}.
+     * @dev Revokes `role` from `account`.
      *
-     * To change a role's admin, use {_setRoleAdmin}.
+     * If `account` had been granted `role`, emits a {RoleRevoked} event.
+     *
+     * Requirements:
+     *
+     * - the caller must have ``role``'s admin role.
+     */
+    function revokeRole(bytes32 role, address account) public virtual {
+        require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to revoke");
+
+        _revokeRole(role, account);
+    }
+
+    /**
+     * @dev Revokes `role` from the calling account.
+     *
+     * Roles are often managed via {grantRole} and {revokeRole}: this function's
+     * purpose is to provide a mechanism for accounts to lose their privileges
+     * if they are compromised (such as when a trusted device is misplaced).
+     *
+     * If the calling account had been granted `role`, emits a {RoleRevoked}
+     * event.
+     *
+     * Requirements:
+     *
+     * - the caller must be `account`.
+     */
+    function renounceRole(bytes32 role, address account) public virtual {
+        require(account == _msgSender(), "AccessControl: can only renounce roles for self");
+
+        _revokeRole(role, account);
+    }
+
+    /**
+     * @dev Grants `role` to `account`.
+     *
+     * If `account` had not been already granted `role`, emits a {RoleGranted}
+     * event. Note that unlike {grantRole}, this function doesn't perform any
+     * checks on the calling account.
+     *
+     * [WARNING]
+     * ====
+     * This function should only be called from the constructor when setting
+     * up the initial roles for the system.
+     *
+     * Using this function in any other way is effectively circumventing the admin
+     * system imposed by {AccessControl}.
+     * ====
      */
-    function getRoleAdmin(bytes32 role) public view virtual override returns (bytes32) {
-        return _roles[role].adminRole;
+    function _setupRole(bytes32 role, address account) internal virtual {
+        _grantRole(role, account);
     }
 
     /**
@@ -96,19 +165,21 @@ abstract contract AccessControl is AccessControlLight {
      *
      * Emits a {RoleAdminChanged} event.
      */
-    function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual override {
-        emit RoleAdminChanged(role, _roles[role].adminRole, adminRole);
-        _roles[role].adminRole = adminRole;
+    function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual {
+        emit RoleAdminChanged(role, getRoleAdmin(role), adminRole);
+        _admins[role] = adminRole;
     }
 
-    function _grantRole(bytes32 role, address account) internal virtual override {
-        if (_roles[role].members.add(account)) {
+    function _grantRole(bytes32 role, address account) internal virtual {
+        if (!_members[role][account]) {
+            _members[role][account] = true;
             emit RoleGranted(role, account, _msgSender());
         }
     }
 
-    function _revokeRole(bytes32 role, address account) internal virtual override {
-        if (_roles[role].members.remove(account)) {
+    function _revokeRole(bytes32 role, address account) internal virtual {
+        if (_members[role][account]) {
+            _members[role][account] = false;
             emit RoleRevoked(role, account, _msgSender());
         }
     }
diff --git a/contracts/access/AccessControlEnumerable.sol b/contracts/access/AccessControlEnumerable.sol
new file mode 100644
index 00000000000..0dd65b8bc47
--- /dev/null
+++ b/contracts/access/AccessControlEnumerable.sol
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "./AccessControl.sol";
+import "../utils/EnumerableSet.sol";
+
+/**
+ * @dev Contract module that allows children to implement role-based access
+ * control mechanisms. Full, enumerable, version.
+ *
+ * Roles are referred to by their `bytes32` identifier. These should be exposed
+ * in the external API and be unique. The best way to achieve this is by
+ * using `public constant` hash digests:
+ *
+ * ```
+ * bytes32 public constant MY_ROLE = keccak256("MY_ROLE");
+ * ```
+ *
+ * Roles can be used to represent a set of permissions. To restrict access to a
+ * function call, use {hasRole}:
+ *
+ * ```
+ * function foo() public {
+ *     require(hasRole(MY_ROLE, msg.sender));
+ *     ...
+ * }
+ * ```
+ *
+ * Roles can be granted and revoked dynamically via the {grantRole} and
+ * {revokeRole} functions. Each role has an associated admin role, and only
+ * accounts that have a role's admin role can call {grantRole} and {revokeRole}.
+ *
+ * By default, the admin role for all roles is `DEFAULT_ADMIN_ROLE`, which means
+ * that only accounts with this role will be able to grant or revoke other
+ * roles. More complex role relationships can be created by using
+ * {_setRoleAdmin}.
+ *
+ * WARNING: The `DEFAULT_ADMIN_ROLE` is also its own admin: it has permission to
+ * grant and revoke this role. Extra precautions should be taken to secure
+ * accounts that have been granted it.
+ */
+abstract contract AccessControlEnumerable is AccessControl {
+    using EnumerableSet for EnumerableSet.AddressSet;
+
+    struct RoleData {
+        EnumerableSet.AddressSet members;
+        bytes32 adminRole;
+    }
+
+    mapping (bytes32 => RoleData) private _roles;
+
+    /**
+     * @dev Returns `true` if `account` has been granted `role`.
+     */
+    function hasRole(bytes32 role, address account) public view virtual override returns (bool) {
+        return _roles[role].members.contains(account);
+    }
+
+    /**
+     * @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) {
+        return _roles[role].members.length();
+    }
+
+    /**
+     * @dev Returns one of the accounts that have `role`. `index` must be a
+     * value between 0 and {getRoleMemberCount}, non-inclusive.
+     *
+     * Role bearers are not sorted in any particular way, and their ordering may
+     * change at any point.
+     *
+     * WARNING: When using {getRoleMember} and {getRoleMemberCount}, make sure
+     * you perform all queries on the same block. See the following
+     * 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) {
+        return _roles[role].members.at(index);
+    }
+
+    /**
+     * @dev Returns the admin role that controls `role`. See {grantRole} and
+     * {revokeRole}.
+     *
+     * To change a role's admin, use {_setRoleAdmin}.
+     */
+    function getRoleAdmin(bytes32 role) public view virtual override returns (bytes32) {
+        return _roles[role].adminRole;
+    }
+
+    /**
+     * @dev Sets `adminRole` as ``role``'s admin role.
+     *
+     * Emits a {RoleAdminChanged} event.
+     */
+    function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual override {
+        emit RoleAdminChanged(role, _roles[role].adminRole, adminRole);
+        _roles[role].adminRole = adminRole;
+    }
+
+    function _grantRole(bytes32 role, address account) internal virtual override {
+        if (_roles[role].members.add(account)) {
+            emit RoleGranted(role, account, _msgSender());
+        }
+    }
+
+    function _revokeRole(bytes32 role, address account) internal virtual override {
+        if (_roles[role].members.remove(account)) {
+            emit RoleRevoked(role, account, _msgSender());
+        }
+    }
+}
diff --git a/contracts/access/AccessControlLight.sol b/contracts/access/AccessControlLight.sol
deleted file mode 100644
index b0086993452..00000000000
--- a/contracts/access/AccessControlLight.sol
+++ /dev/null
@@ -1,186 +0,0 @@
-// SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.0;
-
-import "../utils/Context.sol";
-
-/**
- * @dev Contract module that allows children to implement role-based access
- * control mechanisms. Light, not enumerable, version.
- *
- * Roles are referred to by their `bytes32` identifier. These should be exposed
- * in the external API and be unique. The best way to achieve this is by
- * using `public constant` hash digests:
- *
- * ```
- * bytes32 public constant MY_ROLE = keccak256("MY_ROLE");
- * ```
- *
- * Roles can be used to represent a set of permissions. To restrict access to a
- * function call, use {hasRole}:
- *
- * ```
- * function foo() public {
- *     require(hasRole(MY_ROLE, msg.sender));
- *     ...
- * }
- * ```
- *
- * Roles can be granted and revoked dynamically via the {grantRole} and
- * {revokeRole} functions. Each role has an associated admin role, and only
- * accounts that have a role's admin role can call {grantRole} and {revokeRole}.
- *
- * By default, the admin role for all roles is `DEFAULT_ADMIN_ROLE`, which means
- * that only accounts with this role will be able to grant or revoke other
- * roles. More complex role relationships can be created by using
- * {_setRoleAdmin}.
- *
- * WARNING: The `DEFAULT_ADMIN_ROLE` is also its own admin: it has permission to
- * grant and revoke this role. Extra precautions should be taken to secure
- * accounts that have been granted it.
- */
-abstract contract AccessControlLight is Context {
-    bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00;
-
-    mapping (bytes32 => bytes32) private _admins;
-    mapping (bytes32 => mapping (address => bool)) private _members;
-
-    /**
-     * @dev Emitted when `newAdminRole` is set as ``role``'s admin role, replacing `previousAdminRole`
-     *
-     * `DEFAULT_ADMIN_ROLE` is the starting admin for all roles, despite
-     * {RoleAdminChanged} not being emitted signaling this.
-     *
-     * _Available since v3.1._
-     */
-    event RoleAdminChanged(bytes32 indexed role, bytes32 indexed previousAdminRole, bytes32 indexed newAdminRole);
-
-    /**
-     * @dev Emitted when `account` is granted `role`.
-     *
-     * `sender` is the account that originated the contract call, an admin role
-     * bearer except when using {_setupRole}.
-     */
-    event RoleGranted(bytes32 indexed role, address indexed account, address indexed sender);
-
-    /**
-     * @dev Emitted when `account` is revoked `role`.
-     *
-     * `sender` is the account that originated the contract call:
-     *   - if using `revokeRole`, it is the admin role bearer
-     *   - if using `renounceRole`, it is the role bearer (i.e. `account`)
-     */
-    event RoleRevoked(bytes32 indexed role, address indexed account, address indexed sender);
-
-    /**
-     * @dev Returns `true` if `account` has been granted `role`.
-     */
-    function hasRole(bytes32 role, address account) public view virtual returns (bool) {
-        return _members[role][account];
-    }
-
-    /**
-     * @dev Returns the admin role that controls `role`. See {grantRole} and
-     * {revokeRole}.
-     *
-     * To change a role's admin, use {_setRoleAdmin}.
-     */
-    function getRoleAdmin(bytes32 role) public view virtual returns (bytes32) {
-        return _admins[role];
-    }
-
-    /**
-     * @dev Grants `role` to `account`.
-     *
-     * If `account` had not been already granted `role`, emits a {RoleGranted}
-     * event.
-     *
-     * Requirements:
-     *
-     * - the caller must have ``role``'s admin role.
-     */
-    function grantRole(bytes32 role, address account) public virtual {
-        require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to grant");
-
-        _grantRole(role, account);
-    }
-
-    /**
-     * @dev Revokes `role` from `account`.
-     *
-     * If `account` had been granted `role`, emits a {RoleRevoked} event.
-     *
-     * Requirements:
-     *
-     * - the caller must have ``role``'s admin role.
-     */
-    function revokeRole(bytes32 role, address account) public virtual {
-        require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to revoke");
-
-        _revokeRole(role, account);
-    }
-
-    /**
-     * @dev Revokes `role` from the calling account.
-     *
-     * Roles are often managed via {grantRole} and {revokeRole}: this function's
-     * purpose is to provide a mechanism for accounts to lose their privileges
-     * if they are compromised (such as when a trusted device is misplaced).
-     *
-     * If the calling account had been granted `role`, emits a {RoleRevoked}
-     * event.
-     *
-     * Requirements:
-     *
-     * - the caller must be `account`.
-     */
-    function renounceRole(bytes32 role, address account) public virtual {
-        require(account == _msgSender(), "AccessControl: can only renounce roles for self");
-
-        _revokeRole(role, account);
-    }
-
-    /**
-     * @dev Grants `role` to `account`.
-     *
-     * If `account` had not been already granted `role`, emits a {RoleGranted}
-     * event. Note that unlike {grantRole}, this function doesn't perform any
-     * checks on the calling account.
-     *
-     * [WARNING]
-     * ====
-     * This function should only be called from the constructor when setting
-     * up the initial roles for the system.
-     *
-     * Using this function in any other way is effectively circumventing the admin
-     * system imposed by {AccessControl}.
-     * ====
-     */
-    function _setupRole(bytes32 role, address account) internal virtual {
-        _grantRole(role, account);
-    }
-
-    /**
-     * @dev Sets `adminRole` as ``role``'s admin role.
-     *
-     * Emits a {RoleAdminChanged} event.
-     */
-    function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual {
-        emit RoleAdminChanged(role, getRoleAdmin(role), adminRole);
-        _admins[role] = adminRole;
-    }
-
-    function _grantRole(bytes32 role, address account) internal virtual {
-        if (!_members[role][account]) {
-            _members[role][account] = true;
-            emit RoleGranted(role, account, _msgSender());
-        }
-    }
-
-    function _revokeRole(bytes32 role, address account) internal virtual {
-        if (_members[role][account]) {
-            _members[role][account] = false;
-            emit RoleRevoked(role, account, _msgSender());
-        }
-    }
-}
diff --git a/contracts/mocks/AccessControlLightMock.sol b/contracts/mocks/AccessControlLightMock.sol
index 08eca0b52ff..ec2cbbb103f 100644
--- a/contracts/mocks/AccessControlLightMock.sol
+++ b/contracts/mocks/AccessControlLightMock.sol
@@ -2,9 +2,9 @@
 
 pragma solidity ^0.8.0;
 
-import "../access/AccessControlLight.sol";
+import "../access/AccessControlEnumerable.sol";
 
-contract AccessControlLightMock is AccessControlLight {
+contract AccessControlEnumerableMock is AccessControlEnumerable {
     constructor() {
         _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
     }
diff --git a/contracts/presets/ERC1155PresetMinterPauser.sol b/contracts/presets/ERC1155PresetMinterPauser.sol
index 73fabafdd19..e8423f8b4ca 100644
--- a/contracts/presets/ERC1155PresetMinterPauser.sol
+++ b/contracts/presets/ERC1155PresetMinterPauser.sol
@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.0;
 
-import "../access/AccessControl.sol";
+import "../access/AccessControlEnumerable.sol";
 import "../utils/Context.sol";
 import "../token/ERC1155/ERC1155.sol";
 import "../token/ERC1155/ERC1155Burnable.sol";
@@ -22,7 +22,7 @@ import "../token/ERC1155/ERC1155Pausable.sol";
  * roles, as well as the default admin role, which will let it grant both minter
  * and pauser roles to other accounts.
  */
-contract ERC1155PresetMinterPauser is Context, AccessControl, ERC1155Burnable, ERC1155Pausable {
+contract ERC1155PresetMinterPauser is Context, AccessControlEnumerable, ERC1155Burnable, ERC1155Pausable {
     bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
     bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");
 
diff --git a/contracts/presets/ERC20PresetMinterPauser.sol b/contracts/presets/ERC20PresetMinterPauser.sol
index a8891c4426c..a9047d996fd 100644
--- a/contracts/presets/ERC20PresetMinterPauser.sol
+++ b/contracts/presets/ERC20PresetMinterPauser.sol
@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.0;
 
-import "../access/AccessControl.sol";
+import "../access/AccessControlEnumerable.sol";
 import "../utils/Context.sol";
 import "../token/ERC20/ERC20.sol";
 import "../token/ERC20/ERC20Burnable.sol";
@@ -22,7 +22,7 @@ import "../token/ERC20/ERC20Pausable.sol";
  * roles, as well as the default admin role, which will let it grant both minter
  * and pauser roles to other accounts.
  */
-contract ERC20PresetMinterPauser is Context, AccessControl, ERC20Burnable, ERC20Pausable {
+contract ERC20PresetMinterPauser is Context, AccessControlEnumerable, ERC20Burnable, ERC20Pausable {
     bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
     bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");
 
diff --git a/contracts/presets/ERC721PresetMinterPauserAutoId.sol b/contracts/presets/ERC721PresetMinterPauserAutoId.sol
index 2cd9bdaa403..9da99bac582 100644
--- a/contracts/presets/ERC721PresetMinterPauserAutoId.sol
+++ b/contracts/presets/ERC721PresetMinterPauserAutoId.sol
@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.0;
 
-import "../access/AccessControl.sol";
+import "../access/AccessControlEnumerable.sol";
 import "../utils/Context.sol";
 import "../utils/Counters.sol";
 import "../token/ERC721/ERC721.sol";
@@ -24,7 +24,7 @@ import "../token/ERC721/ERC721Pausable.sol";
  * roles, as well as the default admin role, which will let it grant both minter
  * and pauser roles to other accounts.
  */
-contract ERC721PresetMinterPauserAutoId is Context, AccessControl, ERC721Burnable, ERC721Pausable {
+contract ERC721PresetMinterPauserAutoId is Context, AccessControlEnumerable, ERC721Burnable, ERC721Pausable {
     using Counters for Counters.Counter;
 
     bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
diff --git a/test/access/AccessControl.test.js b/test/access/AccessControl.test.js
index f86bce573d5..833e8d598bd 100644
--- a/test/access/AccessControl.test.js
+++ b/test/access/AccessControl.test.js
@@ -121,23 +121,6 @@ contract('AccessControl', function (accounts) {
     });
   });
 
-  describe('enumerating', function () {
-    it('role bearers can be enumerated', async function () {
-      await this.accessControl.grantRole(ROLE, authorized, { from: admin });
-      await this.accessControl.grantRole(ROLE, otherAuthorized, { from: admin });
-
-      const memberCount = await this.accessControl.getRoleMemberCount(ROLE);
-      expect(memberCount).to.bignumber.equal('2');
-
-      const bearers = [];
-      for (let i = 0; i < memberCount; ++i) {
-        bearers.push(await this.accessControl.getRoleMember(ROLE, i));
-      }
-
-      expect(bearers).to.have.members([authorized, otherAuthorized]);
-    });
-  });
-
   describe('setting role admin', function () {
     beforeEach(async function () {
       const receipt = await this.accessControl.setRoleAdmin(ROLE, OTHER_ROLE);
diff --git a/test/access/AccessControlLight.test.js b/test/access/AccessControlEnumerable.test.js
similarity index 89%
rename from test/access/AccessControlLight.test.js
rename to test/access/AccessControlEnumerable.test.js
index 8baefe124d4..4e4cbff9265 100644
--- a/test/access/AccessControlLight.test.js
+++ b/test/access/AccessControlEnumerable.test.js
@@ -2,9 +2,9 @@ const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
 
 const { expect } = require('chai');
 
-const AccessControlMock = artifacts.require('AccessControlLightMock');
+const AccessControlMock = artifacts.require('AccessControlEnumerableMock');
 
-contract('AccessControl', function (accounts) {
+contract('AccessControlEnumerable', function (accounts) {
   const [ admin, authorized, otherAuthorized, other, otherAdmin ] = accounts;
 
   const DEFAULT_ADMIN_ROLE = '0x0000000000000000000000000000000000000000000000000000000000000000';
@@ -121,6 +121,23 @@ contract('AccessControl', function (accounts) {
     });
   });
 
+  describe('enumerating', function () {
+    it('role bearers can be enumerated', async function () {
+      await this.accessControl.grantRole(ROLE, authorized, { from: admin });
+      await this.accessControl.grantRole(ROLE, otherAuthorized, { from: admin });
+
+      const memberCount = await this.accessControl.getRoleMemberCount(ROLE);
+      expect(memberCount).to.bignumber.equal('2');
+
+      const bearers = [];
+      for (let i = 0; i < memberCount; ++i) {
+        bearers.push(await this.accessControl.getRoleMember(ROLE, i));
+      }
+
+      expect(bearers).to.have.members([authorized, otherAuthorized]);
+    });
+  });
+
   describe('setting role admin', function () {
     beforeEach(async function () {
       const receipt = await this.accessControl.setRoleAdmin(ROLE, OTHER_ROLE);

From 20adbbe4fc0734c38286572d08b007486927b2ad Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Fri, 5 Feb 2021 14:55:32 +0100
Subject: [PATCH 04/16] fix lint

---
 test/access/AccessControl.test.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/access/AccessControl.test.js b/test/access/AccessControl.test.js
index 833e8d598bd..c0eba7a6209 100644
--- a/test/access/AccessControl.test.js
+++ b/test/access/AccessControl.test.js
@@ -5,7 +5,7 @@ const { expect } = require('chai');
 const AccessControlMock = artifacts.require('AccessControlMock');
 
 contract('AccessControl', function (accounts) {
-  const [ admin, authorized, otherAuthorized, other, otherAdmin ] = accounts;
+  const [ admin, authorized, other, otherAdmin ] = accounts;
 
   const DEFAULT_ADMIN_ROLE = '0x0000000000000000000000000000000000000000000000000000000000000000';
   const ROLE = web3.utils.soliditySha3('ROLE');

From 3868b999bec26dd0e449bedee62bf5f8347d2aab Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Mon, 15 Feb 2021 14:33:59 +0100
Subject: [PATCH 05/16] improve enumerability tracking of AccessControl (still
 some dusplicated sstore)

---
 contracts/access/AccessControl.sol           | 10 ++--
 contracts/access/AccessControlEnumerable.sol | 56 +++++---------------
 2 files changed, 19 insertions(+), 47 deletions(-)

diff --git a/contracts/access/AccessControl.sol b/contracts/access/AccessControl.sol
index ce90d014fab..7aa4dc4d11a 100644
--- a/contracts/access/AccessControl.sol
+++ b/contracts/access/AccessControl.sol
@@ -100,7 +100,7 @@ abstract contract AccessControl is Context {
      * - the caller must have ``role``'s admin role.
      */
     function grantRole(bytes32 role, address account) public virtual {
-        require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to grant");
+        require(AccessControl.hasRole(AccessControl.getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to grant");
 
         _grantRole(role, account);
     }
@@ -115,7 +115,7 @@ abstract contract AccessControl is Context {
      * - the caller must have ``role``'s admin role.
      */
     function revokeRole(bytes32 role, address account) public virtual {
-        require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to revoke");
+        require(AccessControl.hasRole(AccessControl.getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to revoke");
 
         _revokeRole(role, account);
     }
@@ -166,19 +166,19 @@ abstract contract AccessControl is Context {
      * Emits a {RoleAdminChanged} event.
      */
     function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual {
-        emit RoleAdminChanged(role, getRoleAdmin(role), adminRole);
+        emit RoleAdminChanged(role, AccessControl.getRoleAdmin(role), adminRole);
         _admins[role] = adminRole;
     }
 
     function _grantRole(bytes32 role, address account) internal virtual {
-        if (!_members[role][account]) {
+        if (!AccessControl.hasRole(role, account)) {
             _members[role][account] = true;
             emit RoleGranted(role, account, _msgSender());
         }
     }
 
     function _revokeRole(bytes32 role, address account) internal virtual {
-        if (_members[role][account]) {
+        if (AccessControl.hasRole(role, account)) {
             _members[role][account] = false;
             emit RoleRevoked(role, account, _msgSender());
         }
diff --git a/contracts/access/AccessControlEnumerable.sol b/contracts/access/AccessControlEnumerable.sol
index 0dd65b8bc47..030f162ba67 100644
--- a/contracts/access/AccessControlEnumerable.sol
+++ b/contracts/access/AccessControlEnumerable.sol
@@ -43,27 +43,7 @@ import "../utils/EnumerableSet.sol";
 abstract contract AccessControlEnumerable is AccessControl {
     using EnumerableSet for EnumerableSet.AddressSet;
 
-    struct RoleData {
-        EnumerableSet.AddressSet members;
-        bytes32 adminRole;
-    }
-
-    mapping (bytes32 => RoleData) private _roles;
-
-    /**
-     * @dev Returns `true` if `account` has been granted `role`.
-     */
-    function hasRole(bytes32 role, address account) public view virtual override returns (bool) {
-        return _roles[role].members.contains(account);
-    }
-
-    /**
-     * @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) {
-        return _roles[role].members.length();
-    }
+    mapping (bytes32 => EnumerableSet.AddressSet) private _roleMembers;
 
     /**
      * @dev Returns one of the accounts that have `role`. `index` must be a
@@ -78,38 +58,30 @@ abstract contract AccessControlEnumerable is AccessControl {
      * for more information.
      */
     function getRoleMember(bytes32 role, uint256 index) public view returns (address) {
-        return _roles[role].members.at(index);
+        return _roleMembers[role].at(index);
     }
 
     /**
-     * @dev Returns the admin role that controls `role`. See {grantRole} and
-     * {revokeRole}.
-     *
-     * To change a role's admin, use {_setRoleAdmin}.
+     * @dev Returns the number of accounts that have `role`. Can be used
+     * together with {getRoleMember} to enumerate all bearers of a role.
      */
-    function getRoleAdmin(bytes32 role) public view virtual override returns (bytes32) {
-        return _roles[role].adminRole;
+    function getRoleMemberCount(bytes32 role) public view returns (uint256) {
+        return _roleMembers[role].length();
     }
 
     /**
-     * @dev Sets `adminRole` as ``role``'s admin role.
-     *
-     * Emits a {RoleAdminChanged} event.
+     * @dev Overload {_grantRole} to track enumerable memberships
      */
-    function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual override {
-        emit RoleAdminChanged(role, _roles[role].adminRole, adminRole);
-        _roles[role].adminRole = adminRole;
-    }
-
     function _grantRole(bytes32 role, address account) internal virtual override {
-        if (_roles[role].members.add(account)) {
-            emit RoleGranted(role, account, _msgSender());
-        }
+        super._grantRole(role, account);
+        _roleMembers[role].add(account);
     }
 
+    /**
+     * @dev Overload {_revokeRole} to track enumerable memberships
+     */
     function _revokeRole(bytes32 role, address account) internal virtual override {
-        if (_roles[role].members.remove(account)) {
-            emit RoleRevoked(role, account, _msgSender());
-        }
+        super._revokeRole(role, account);
+        _roleMembers[role].remove(account);
     }
 }

From 9380f3d0580a915950dce7dc2878ba76926215c1 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Mon, 15 Feb 2021 15:00:03 +0100
Subject: [PATCH 06/16] fixe #2139

---
 contracts/access/AccessControlEnumerable.sol | 28 ++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/contracts/access/AccessControlEnumerable.sol b/contracts/access/AccessControlEnumerable.sol
index 030f162ba67..7e2d9e8db91 100644
--- a/contracts/access/AccessControlEnumerable.sol
+++ b/contracts/access/AccessControlEnumerable.sol
@@ -42,8 +42,10 @@ import "../utils/EnumerableSet.sol";
  */
 abstract contract AccessControlEnumerable is AccessControl {
     using EnumerableSet for EnumerableSet.AddressSet;
+    using EnumerableSet for EnumerableSet.Bytes32Set;
 
     mapping (bytes32 => EnumerableSet.AddressSet) private _roleMembers;
+    mapping (address => EnumerableSet.Bytes32Set) private _addressRoles;
 
     /**
      * @dev Returns one of the accounts that have `role`. `index` must be a
@@ -69,12 +71,37 @@ abstract contract AccessControlEnumerable is AccessControl {
         return _roleMembers[role].length();
     }
 
+    /**
+     * @dev Returns one of the roles that `account` has. `index` must be a
+     * value between 0 and {getAddressRoleCount}, non-inclusive.
+     *
+     * Role not sorted in any particular way, and their ordering may change at
+     * any point.
+     *
+     * WARNING: When using {getAddressRole} and {getAddressRoleCount}, make sure
+     * you perform all queries on the same block. See the following
+     * https://forum.openzeppelin.com/t/iterating-over-elements-on-enumerableset-in-openzeppelin-contracts/2296[forum post]
+     * for more information.
+     */
+    function getAddressRole(address account, uint256 index) public view returns (bytes32) {
+        return _addressRoles[account].at(index);
+    }
+
+    /**
+     * @dev Returns the number of role that `account` has. Can be used
+     * together with {getAddressRole} to enumerate all role of an account.
+     */
+    function getAddressRoleCount(address account) public view returns (uint256) {
+        return _addressRoles[account].length();
+    }
+
     /**
      * @dev Overload {_grantRole} to track enumerable memberships
      */
     function _grantRole(bytes32 role, address account) internal virtual override {
         super._grantRole(role, account);
         _roleMembers[role].add(account);
+        _addressRoles[account].add(role);
     }
 
     /**
@@ -83,5 +110,6 @@ abstract contract AccessControlEnumerable is AccessControl {
     function _revokeRole(bytes32 role, address account) internal virtual override {
         super._revokeRole(role, account);
         _roleMembers[role].remove(account);
+        _addressRoles[account].remove(role);
     }
 }

From df5263d3d428e21788a7848ef465ff728dd772af Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Thu, 18 Feb 2021 16:04:50 +0100
Subject: [PATCH 07/16] split accesscontrol enumerability + enumerability tests

---
 contracts/access/AccessControlEnumerable.sol  |  30 +--
 contracts/access/AccessControlEnumerable2.sol |  87 ++++++++
 .../mocks/AccessControlEnumerableMock.sol     |  28 +++
 contracts/mocks/AccessControlLightMock.sol    |  15 --
 test/access/AccessControl.behavior.js         | 204 ++++++++++++++++++
 test/access/AccessControl.test.js             | 158 +-------------
 test/access/AccessControlEnumerable.test.js   | 183 +---------------
 7 files changed, 335 insertions(+), 370 deletions(-)
 create mode 100644 contracts/access/AccessControlEnumerable2.sol
 create mode 100644 contracts/mocks/AccessControlEnumerableMock.sol
 delete mode 100644 contracts/mocks/AccessControlLightMock.sol
 create mode 100644 test/access/AccessControl.behavior.js

diff --git a/contracts/access/AccessControlEnumerable.sol b/contracts/access/AccessControlEnumerable.sol
index 7e2d9e8db91..11d063e5a39 100644
--- a/contracts/access/AccessControlEnumerable.sol
+++ b/contracts/access/AccessControlEnumerable.sol
@@ -7,7 +7,7 @@ import "../utils/EnumerableSet.sol";
 
 /**
  * @dev Contract module that allows children to implement role-based access
- * control mechanisms. Full, enumerable, version.
+ * control mechanisms. Role to account enumerable version.
  *
  * Roles are referred to by their `bytes32` identifier. These should be exposed
  * in the external API and be unique. The best way to achieve this is by
@@ -42,10 +42,8 @@ import "../utils/EnumerableSet.sol";
  */
 abstract contract AccessControlEnumerable is AccessControl {
     using EnumerableSet for EnumerableSet.AddressSet;
-    using EnumerableSet for EnumerableSet.Bytes32Set;
 
     mapping (bytes32 => EnumerableSet.AddressSet) private _roleMembers;
-    mapping (address => EnumerableSet.Bytes32Set) private _addressRoles;
 
     /**
      * @dev Returns one of the accounts that have `role`. `index` must be a
@@ -71,37 +69,12 @@ abstract contract AccessControlEnumerable is AccessControl {
         return _roleMembers[role].length();
     }
 
-    /**
-     * @dev Returns one of the roles that `account` has. `index` must be a
-     * value between 0 and {getAddressRoleCount}, non-inclusive.
-     *
-     * Role not sorted in any particular way, and their ordering may change at
-     * any point.
-     *
-     * WARNING: When using {getAddressRole} and {getAddressRoleCount}, make sure
-     * you perform all queries on the same block. See the following
-     * https://forum.openzeppelin.com/t/iterating-over-elements-on-enumerableset-in-openzeppelin-contracts/2296[forum post]
-     * for more information.
-     */
-    function getAddressRole(address account, uint256 index) public view returns (bytes32) {
-        return _addressRoles[account].at(index);
-    }
-
-    /**
-     * @dev Returns the number of role that `account` has. Can be used
-     * together with {getAddressRole} to enumerate all role of an account.
-     */
-    function getAddressRoleCount(address account) public view returns (uint256) {
-        return _addressRoles[account].length();
-    }
-
     /**
      * @dev Overload {_grantRole} to track enumerable memberships
      */
     function _grantRole(bytes32 role, address account) internal virtual override {
         super._grantRole(role, account);
         _roleMembers[role].add(account);
-        _addressRoles[account].add(role);
     }
 
     /**
@@ -110,6 +83,5 @@ abstract contract AccessControlEnumerable is AccessControl {
     function _revokeRole(bytes32 role, address account) internal virtual override {
         super._revokeRole(role, account);
         _roleMembers[role].remove(account);
-        _addressRoles[account].remove(role);
     }
 }
diff --git a/contracts/access/AccessControlEnumerable2.sol b/contracts/access/AccessControlEnumerable2.sol
new file mode 100644
index 00000000000..c3c6507115a
--- /dev/null
+++ b/contracts/access/AccessControlEnumerable2.sol
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "./AccessControl.sol";
+import "../utils/EnumerableSet.sol";
+
+/**
+ * @dev Contract module that allows children to implement role-based access
+ * control mechanisms. Account to role enumerable version.
+ *
+ * Roles are referred to by their `bytes32` identifier. These should be exposed
+ * in the external API and be unique. The best way to achieve this is by
+ * using `public constant` hash digests:
+ *
+ * ```
+ * bytes32 public constant MY_ROLE = keccak256("MY_ROLE");
+ * ```
+ *
+ * Roles can be used to represent a set of permissions. To restrict access to a
+ * function call, use {hasRole}:
+ *
+ * ```
+ * function foo() public {
+ *     require(hasRole(MY_ROLE, msg.sender));
+ *     ...
+ * }
+ * ```
+ *
+ * Roles can be granted and revoked dynamically via the {grantRole} and
+ * {revokeRole} functions. Each role has an associated admin role, and only
+ * accounts that have a role's admin role can call {grantRole} and {revokeRole}.
+ *
+ * By default, the admin role for all roles is `DEFAULT_ADMIN_ROLE`, which means
+ * that only accounts with this role will be able to grant or revoke other
+ * roles. More complex role relationships can be created by using
+ * {_setRoleAdmin}.
+ *
+ * WARNING: The `DEFAULT_ADMIN_ROLE` is also its own admin: it has permission to
+ * grant and revoke this role. Extra precautions should be taken to secure
+ * accounts that have been granted it.
+ */
+abstract contract AccessControlEnumerable2 is AccessControl {
+    using EnumerableSet for EnumerableSet.Bytes32Set;
+
+    mapping (address => EnumerableSet.Bytes32Set) private _addressRoles;
+
+    /**
+     * @dev Returns one of the roles that `account` has. `index` must be a
+     * value between 0 and {getAddressRoleCount}, non-inclusive.
+     *
+     * Role not sorted in any particular way, and their ordering may change at
+     * any point.
+     *
+     * WARNING: When using {getAddressRole} and {getAddressRoleCount}, make sure
+     * you perform all queries on the same block. See the following
+     * https://forum.openzeppelin.com/t/iterating-over-elements-on-enumerableset-in-openzeppelin-contracts/2296[forum post]
+     * for more information.
+     */
+    function getAddressRole(address account, uint256 index) public view returns (bytes32) {
+        return _addressRoles[account].at(index);
+    }
+
+    /**
+     * @dev Returns the number of role that `account` has. Can be used
+     * together with {getAddressRole} to enumerate all role of an account.
+     */
+    function getAddressRoleCount(address account) public view returns (uint256) {
+        return _addressRoles[account].length();
+    }
+
+    /**
+     * @dev Overload {_grantRole} to track enumerable memberships
+     */
+    function _grantRole(bytes32 role, address account) internal virtual override {
+        super._grantRole(role, account);
+        _addressRoles[account].add(role);
+    }
+
+    /**
+     * @dev Overload {_revokeRole} to track enumerable memberships
+     */
+    function _revokeRole(bytes32 role, address account) internal virtual override {
+        super._revokeRole(role, account);
+        _addressRoles[account].remove(role);
+    }
+}
diff --git a/contracts/mocks/AccessControlEnumerableMock.sol b/contracts/mocks/AccessControlEnumerableMock.sol
new file mode 100644
index 00000000000..68c78dbbc8b
--- /dev/null
+++ b/contracts/mocks/AccessControlEnumerableMock.sol
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "../access/AccessControlEnumerable.sol";
+import "../access/AccessControlEnumerable2.sol";
+
+contract AccessControlEnumerableMock is AccessControlEnumerable, AccessControlEnumerable2 {
+    constructor() {
+        _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
+    }
+
+    function setRoleAdmin(bytes32 roleId, bytes32 adminRoleId) public {
+        _setRoleAdmin(roleId, adminRoleId);
+    }
+
+    function _grantRole(bytes32 role, address account)
+    internal virtual override(AccessControlEnumerable, AccessControlEnumerable2)
+    {
+        super._grantRole(role, account);
+    }
+
+    function _revokeRole(bytes32 role, address account)
+    internal virtual override(AccessControlEnumerable, AccessControlEnumerable2)
+    {
+        super._revokeRole(role, account);
+    }
+}
diff --git a/contracts/mocks/AccessControlLightMock.sol b/contracts/mocks/AccessControlLightMock.sol
deleted file mode 100644
index ec2cbbb103f..00000000000
--- a/contracts/mocks/AccessControlLightMock.sol
+++ /dev/null
@@ -1,15 +0,0 @@
-// SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.0;
-
-import "../access/AccessControlEnumerable.sol";
-
-contract AccessControlEnumerableMock is AccessControlEnumerable {
-    constructor() {
-        _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
-    }
-
-    function setRoleAdmin(bytes32 roleId, bytes32 adminRoleId) public {
-        _setRoleAdmin(roleId, adminRoleId);
-    }
-}
diff --git a/test/access/AccessControl.behavior.js b/test/access/AccessControl.behavior.js
new file mode 100644
index 00000000000..738d653b206
--- /dev/null
+++ b/test/access/AccessControl.behavior.js
@@ -0,0 +1,204 @@
+const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
+const { expect } = require('chai');
+
+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) {
+  describe('default admin', function () {
+    it('deployer has default admin role', async function () {
+      expect(await this.accessControl.hasRole(DEFAULT_ADMIN_ROLE, admin)).to.equal(true);
+    });
+
+    it('other roles\'s admin is the default admin role', async function () {
+      expect(await this.accessControl.getRoleAdmin(ROLE)).to.equal(DEFAULT_ADMIN_ROLE);
+    });
+
+    it('default admin role\'s admin is itself', async function () {
+      expect(await this.accessControl.getRoleAdmin(DEFAULT_ADMIN_ROLE)).to.equal(DEFAULT_ADMIN_ROLE);
+    });
+  });
+
+  describe('granting', function () {
+    it('admin can grant role to other accounts', async function () {
+      const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: admin });
+      expectEvent(receipt, 'RoleGranted', { account: authorized, role: ROLE, sender: admin });
+
+      expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(true);
+    });
+
+    it('non-admin cannot grant role to other accounts', async function () {
+      await expectRevert(
+        this.accessControl.grantRole(ROLE, authorized, { from: other }),
+        `${errorPrefix}: sender must be an admin to grant`,
+      );
+    });
+
+    it('accounts can be granted a role multiple times', async function () {
+      await this.accessControl.grantRole(ROLE, authorized, { from: admin });
+      const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: admin });
+      expectEvent.notEmitted(receipt, 'RoleGranted');
+    });
+  });
+
+  describe('revoking', function () {
+    it('roles that are not had can be revoked', async function () {
+      expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false);
+
+      const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
+      expectEvent.notEmitted(receipt, 'RoleRevoked');
+    });
+
+    context('with granted role', function () {
+      beforeEach(async function () {
+        await this.accessControl.grantRole(ROLE, authorized, { from: admin });
+      });
+
+      it('admin can revoke role', async function () {
+        const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
+        expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: admin });
+
+        expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false);
+      });
+
+      it('non-admin cannot revoke role', async function () {
+        await expectRevert(
+          this.accessControl.revokeRole(ROLE, authorized, { from: other }),
+          `${errorPrefix}: sender must be an admin to revoke`,
+        );
+      });
+
+      it('a role can be revoked multiple times', async function () {
+        await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
+
+        const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
+        expectEvent.notEmitted(receipt, 'RoleRevoked');
+      });
+    });
+  });
+
+  describe('renouncing', function () {
+    it('roles that are not had can be renounced', async function () {
+      const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
+      expectEvent.notEmitted(receipt, 'RoleRevoked');
+    });
+
+    context('with granted role', function () {
+      beforeEach(async function () {
+        await this.accessControl.grantRole(ROLE, authorized, { from: admin });
+      });
+
+      it('bearer can renounce role', async function () {
+        const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
+        expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: authorized });
+
+        expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false);
+      });
+
+      it('only the sender can renounce their roles', async function () {
+        await expectRevert(
+          this.accessControl.renounceRole(ROLE, authorized, { from: admin }),
+          `${errorPrefix}: can only renounce roles for self`,
+        );
+      });
+
+      it('a role can be renounced multiple times', async function () {
+        await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
+
+        const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
+        expectEvent.notEmitted(receipt, 'RoleRevoked');
+      });
+    });
+  });
+
+  describe('setting role admin', function () {
+    beforeEach(async function () {
+      const receipt = await this.accessControl.setRoleAdmin(ROLE, OTHER_ROLE);
+      expectEvent(receipt, 'RoleAdminChanged', {
+        role: ROLE,
+        previousAdminRole: DEFAULT_ADMIN_ROLE,
+        newAdminRole: OTHER_ROLE,
+      });
+
+      await this.accessControl.grantRole(OTHER_ROLE, otherAdmin, { from: admin });
+    });
+
+    it('a role\'s admin role can be changed', async function () {
+      expect(await this.accessControl.getRoleAdmin(ROLE)).to.equal(OTHER_ROLE);
+    });
+
+    it('the new admin can grant roles', async function () {
+      const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: otherAdmin });
+      expectEvent(receipt, 'RoleGranted', { account: authorized, role: ROLE, sender: otherAdmin });
+    });
+
+    it('the new admin can revoke roles', async function () {
+      await this.accessControl.grantRole(ROLE, authorized, { from: otherAdmin });
+      const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: otherAdmin });
+      expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: otherAdmin });
+    });
+
+    it('a role\'s previous admins no longer grant roles', async function () {
+      await expectRevert(
+        this.accessControl.grantRole(ROLE, authorized, { from: admin }),
+        'AccessControl: sender must be an admin to grant',
+      );
+    });
+
+    it('a role\'s previous admins no longer revoke roles', async function () {
+      await expectRevert(
+        this.accessControl.revokeRole(ROLE, authorized, { from: admin }),
+        'AccessControl: sender must be an admin to revoke',
+      );
+    });
+  });
+}
+
+function shouldBehaveLikeAccessControlEnumerable (errorPrefix, admin, authorized, other, otherAdmin, otherAuthorized) {
+  describe('enumerating', function () {
+    it('role bearers can be enumerated', async function () {
+      await this.accessControl.grantRole(ROLE, authorized, { from: admin });
+      await this.accessControl.grantRole(ROLE, other, { from: admin });
+      await this.accessControl.grantRole(ROLE, otherAuthorized, { from: admin });
+      await this.accessControl.revokeRole(ROLE, other, { from: admin });
+
+      const memberCount = await this.accessControl.getRoleMemberCount(ROLE);
+      expect(memberCount).to.bignumber.equal('2');
+
+      const bearers = [];
+      for (let i = 0; i < memberCount; ++i) {
+        bearers.push(await this.accessControl.getRoleMember(ROLE, i));
+      }
+
+      expect(bearers).to.have.members([authorized, otherAuthorized]);
+    });
+  });
+}
+
+function shouldBehaveLikeAccessControlEnumerable2 (errorPrefix, admin, authorized, other, otherAdmin, otherAuthorized) {
+  describe('enumerating', function () {
+    it('role beared can be enumerated', async function () {
+      await this.accessControl.grantRole(ROLE, authorized, { from: admin });
+      await this.accessControl.grantRole(DEFAULT_ADMIN_ROLE, authorized, { from: admin });
+      await this.accessControl.grantRole(OTHER_ROLE, authorized, { from: admin });
+      await this.accessControl.revokeRole(DEFAULT_ADMIN_ROLE, authorized, { from: admin });
+
+      const roleCount = await this.accessControl.getAddressRoleCount(authorized);
+      expect(roleCount).to.bignumber.equal('2');
+
+      const beared = [];
+      for (let i = 0; i < roleCount; ++i) {
+        beared.push(await this.accessControl.getAddressRole(authorized, i));
+      }
+
+      expect(beared).to.have.members([ROLE, OTHER_ROLE]);
+    });
+  });
+}
+
+module.exports = {
+  shouldBehaveLikeAccessControl,
+  shouldBehaveLikeAccessControlEnumerable,
+  shouldBehaveLikeAccessControlEnumerable2,
+};
diff --git a/test/access/AccessControl.test.js b/test/access/AccessControl.test.js
index c0eba7a6209..f829141f7bd 100644
--- a/test/access/AccessControl.test.js
+++ b/test/access/AccessControl.test.js
@@ -1,165 +1,15 @@
 const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
-
 const { expect } = require('chai');
 
+const { shouldBehaveLikeAccessControl } = require('./AccessControl.behavior.js');
+
 const AccessControlMock = artifacts.require('AccessControlMock');
 
 contract('AccessControl', function (accounts) {
-  const [ admin, authorized, other, otherAdmin ] = accounts;
-
-  const DEFAULT_ADMIN_ROLE = '0x0000000000000000000000000000000000000000000000000000000000000000';
-  const ROLE = web3.utils.soliditySha3('ROLE');
-  const OTHER_ROLE = web3.utils.soliditySha3('OTHER_ROLE');
 
   beforeEach(async function () {
-    this.accessControl = await AccessControlMock.new({ from: admin });
-  });
-
-  describe('default admin', function () {
-    it('deployer has default admin role', async function () {
-      expect(await this.accessControl.hasRole(DEFAULT_ADMIN_ROLE, admin)).to.equal(true);
-    });
-
-    it('other roles\'s admin is the default admin role', async function () {
-      expect(await this.accessControl.getRoleAdmin(ROLE)).to.equal(DEFAULT_ADMIN_ROLE);
-    });
-
-    it('default admin role\'s admin is itself', async function () {
-      expect(await this.accessControl.getRoleAdmin(DEFAULT_ADMIN_ROLE)).to.equal(DEFAULT_ADMIN_ROLE);
-    });
-  });
-
-  describe('granting', function () {
-    it('admin can grant role to other accounts', async function () {
-      const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: admin });
-      expectEvent(receipt, 'RoleGranted', { account: authorized, role: ROLE, sender: admin });
-
-      expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(true);
-    });
-
-    it('non-admin cannot grant role to other accounts', async function () {
-      await expectRevert(
-        this.accessControl.grantRole(ROLE, authorized, { from: other }),
-        'AccessControl: sender must be an admin to grant',
-      );
-    });
-
-    it('accounts can be granted a role multiple times', async function () {
-      await this.accessControl.grantRole(ROLE, authorized, { from: admin });
-      const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: admin });
-      expectEvent.notEmitted(receipt, 'RoleGranted');
-    });
-  });
-
-  describe('revoking', function () {
-    it('roles that are not had can be revoked', async function () {
-      expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false);
-
-      const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
-      expectEvent.notEmitted(receipt, 'RoleRevoked');
-    });
-
-    context('with granted role', function () {
-      beforeEach(async function () {
-        await this.accessControl.grantRole(ROLE, authorized, { from: admin });
-      });
-
-      it('admin can revoke role', async function () {
-        const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
-        expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: admin });
-
-        expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false);
-      });
-
-      it('non-admin cannot revoke role', async function () {
-        await expectRevert(
-          this.accessControl.revokeRole(ROLE, authorized, { from: other }),
-          'AccessControl: sender must be an admin to revoke',
-        );
-      });
-
-      it('a role can be revoked multiple times', async function () {
-        await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
-
-        const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
-        expectEvent.notEmitted(receipt, 'RoleRevoked');
-      });
-    });
-  });
-
-  describe('renouncing', function () {
-    it('roles that are not had can be renounced', async function () {
-      const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
-      expectEvent.notEmitted(receipt, 'RoleRevoked');
-    });
-
-    context('with granted role', function () {
-      beforeEach(async function () {
-        await this.accessControl.grantRole(ROLE, authorized, { from: admin });
-      });
-
-      it('bearer can renounce role', async function () {
-        const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
-        expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: authorized });
-
-        expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false);
-      });
-
-      it('only the sender can renounce their roles', async function () {
-        await expectRevert(
-          this.accessControl.renounceRole(ROLE, authorized, { from: admin }),
-          'AccessControl: can only renounce roles for self',
-        );
-      });
-
-      it('a role can be renounced multiple times', async function () {
-        await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
-
-        const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
-        expectEvent.notEmitted(receipt, 'RoleRevoked');
-      });
-    });
+    this.accessControl = await AccessControlMock.new({ from: accounts[0] });
   });
 
-  describe('setting role admin', function () {
-    beforeEach(async function () {
-      const receipt = await this.accessControl.setRoleAdmin(ROLE, OTHER_ROLE);
-      expectEvent(receipt, 'RoleAdminChanged', {
-        role: ROLE,
-        previousAdminRole: DEFAULT_ADMIN_ROLE,
-        newAdminRole: OTHER_ROLE,
-      });
-
-      await this.accessControl.grantRole(OTHER_ROLE, otherAdmin, { from: admin });
-    });
-
-    it('a role\'s admin role can be changed', async function () {
-      expect(await this.accessControl.getRoleAdmin(ROLE)).to.equal(OTHER_ROLE);
-    });
-
-    it('the new admin can grant roles', async function () {
-      const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: otherAdmin });
-      expectEvent(receipt, 'RoleGranted', { account: authorized, role: ROLE, sender: otherAdmin });
-    });
-
-    it('the new admin can revoke roles', async function () {
-      await this.accessControl.grantRole(ROLE, authorized, { from: otherAdmin });
-      const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: otherAdmin });
-      expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: otherAdmin });
-    });
-
-    it('a role\'s previous admins no longer grant roles', async function () {
-      await expectRevert(
-        this.accessControl.grantRole(ROLE, authorized, { from: admin }),
-        'AccessControl: sender must be an admin to grant',
-      );
-    });
-
-    it('a role\'s previous admins no longer revoke roles', async function () {
-      await expectRevert(
-        this.accessControl.revokeRole(ROLE, authorized, { from: admin }),
-        'AccessControl: sender must be an admin to revoke',
-      );
-    });
-  });
+  shouldBehaveLikeAccessControl('AccessControl', ...accounts);
 });
diff --git a/test/access/AccessControlEnumerable.test.js b/test/access/AccessControlEnumerable.test.js
index 4e4cbff9265..73076d6c2a9 100644
--- a/test/access/AccessControlEnumerable.test.js
+++ b/test/access/AccessControlEnumerable.test.js
@@ -1,182 +1,21 @@
 const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
-
 const { expect } = require('chai');
 
-const AccessControlMock = artifacts.require('AccessControlEnumerableMock');
+const {
+  shouldBehaveLikeAccessControl,
+  shouldBehaveLikeAccessControlEnumerable,
+  shouldBehaveLikeAccessControlEnumerable2,
+} = require('./AccessControl.behavior.js');
 
-contract('AccessControlEnumerable', function (accounts) {
-  const [ admin, authorized, otherAuthorized, other, otherAdmin ] = accounts;
+const AccessControlMock = artifacts.require('AccessControlEnumerableMock');
 
-  const DEFAULT_ADMIN_ROLE = '0x0000000000000000000000000000000000000000000000000000000000000000';
-  const ROLE = web3.utils.soliditySha3('ROLE');
-  const OTHER_ROLE = web3.utils.soliditySha3('OTHER_ROLE');
+contract('AccessControl', function (accounts) {
 
   beforeEach(async function () {
-    this.accessControl = await AccessControlMock.new({ from: admin });
-  });
-
-  describe('default admin', function () {
-    it('deployer has default admin role', async function () {
-      expect(await this.accessControl.hasRole(DEFAULT_ADMIN_ROLE, admin)).to.equal(true);
-    });
-
-    it('other roles\'s admin is the default admin role', async function () {
-      expect(await this.accessControl.getRoleAdmin(ROLE)).to.equal(DEFAULT_ADMIN_ROLE);
-    });
-
-    it('default admin role\'s admin is itself', async function () {
-      expect(await this.accessControl.getRoleAdmin(DEFAULT_ADMIN_ROLE)).to.equal(DEFAULT_ADMIN_ROLE);
-    });
-  });
-
-  describe('granting', function () {
-    it('admin can grant role to other accounts', async function () {
-      const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: admin });
-      expectEvent(receipt, 'RoleGranted', { account: authorized, role: ROLE, sender: admin });
-
-      expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(true);
-    });
-
-    it('non-admin cannot grant role to other accounts', async function () {
-      await expectRevert(
-        this.accessControl.grantRole(ROLE, authorized, { from: other }),
-        'AccessControl: sender must be an admin to grant',
-      );
-    });
-
-    it('accounts can be granted a role multiple times', async function () {
-      await this.accessControl.grantRole(ROLE, authorized, { from: admin });
-      const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: admin });
-      expectEvent.notEmitted(receipt, 'RoleGranted');
-    });
-  });
-
-  describe('revoking', function () {
-    it('roles that are not had can be revoked', async function () {
-      expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false);
-
-      const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
-      expectEvent.notEmitted(receipt, 'RoleRevoked');
-    });
-
-    context('with granted role', function () {
-      beforeEach(async function () {
-        await this.accessControl.grantRole(ROLE, authorized, { from: admin });
-      });
-
-      it('admin can revoke role', async function () {
-        const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
-        expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: admin });
-
-        expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false);
-      });
-
-      it('non-admin cannot revoke role', async function () {
-        await expectRevert(
-          this.accessControl.revokeRole(ROLE, authorized, { from: other }),
-          'AccessControl: sender must be an admin to revoke',
-        );
-      });
-
-      it('a role can be revoked multiple times', async function () {
-        await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
-
-        const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin });
-        expectEvent.notEmitted(receipt, 'RoleRevoked');
-      });
-    });
+    this.accessControl = await AccessControlMock.new({ from: accounts[0] });
   });
 
-  describe('renouncing', function () {
-    it('roles that are not had can be renounced', async function () {
-      const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
-      expectEvent.notEmitted(receipt, 'RoleRevoked');
-    });
-
-    context('with granted role', function () {
-      beforeEach(async function () {
-        await this.accessControl.grantRole(ROLE, authorized, { from: admin });
-      });
-
-      it('bearer can renounce role', async function () {
-        const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
-        expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: authorized });
-
-        expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false);
-      });
-
-      it('only the sender can renounce their roles', async function () {
-        await expectRevert(
-          this.accessControl.renounceRole(ROLE, authorized, { from: admin }),
-          'AccessControl: can only renounce roles for self',
-        );
-      });
-
-      it('a role can be renounced multiple times', async function () {
-        await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
-
-        const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized });
-        expectEvent.notEmitted(receipt, 'RoleRevoked');
-      });
-    });
-  });
-
-  describe('enumerating', function () {
-    it('role bearers can be enumerated', async function () {
-      await this.accessControl.grantRole(ROLE, authorized, { from: admin });
-      await this.accessControl.grantRole(ROLE, otherAuthorized, { from: admin });
-
-      const memberCount = await this.accessControl.getRoleMemberCount(ROLE);
-      expect(memberCount).to.bignumber.equal('2');
-
-      const bearers = [];
-      for (let i = 0; i < memberCount; ++i) {
-        bearers.push(await this.accessControl.getRoleMember(ROLE, i));
-      }
-
-      expect(bearers).to.have.members([authorized, otherAuthorized]);
-    });
-  });
-
-  describe('setting role admin', function () {
-    beforeEach(async function () {
-      const receipt = await this.accessControl.setRoleAdmin(ROLE, OTHER_ROLE);
-      expectEvent(receipt, 'RoleAdminChanged', {
-        role: ROLE,
-        previousAdminRole: DEFAULT_ADMIN_ROLE,
-        newAdminRole: OTHER_ROLE,
-      });
-
-      await this.accessControl.grantRole(OTHER_ROLE, otherAdmin, { from: admin });
-    });
-
-    it('a role\'s admin role can be changed', async function () {
-      expect(await this.accessControl.getRoleAdmin(ROLE)).to.equal(OTHER_ROLE);
-    });
-
-    it('the new admin can grant roles', async function () {
-      const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: otherAdmin });
-      expectEvent(receipt, 'RoleGranted', { account: authorized, role: ROLE, sender: otherAdmin });
-    });
-
-    it('the new admin can revoke roles', async function () {
-      await this.accessControl.grantRole(ROLE, authorized, { from: otherAdmin });
-      const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: otherAdmin });
-      expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: otherAdmin });
-    });
-
-    it('a role\'s previous admins no longer grant roles', async function () {
-      await expectRevert(
-        this.accessControl.grantRole(ROLE, authorized, { from: admin }),
-        'AccessControl: sender must be an admin to grant',
-      );
-    });
-
-    it('a role\'s previous admins no longer revoke roles', async function () {
-      await expectRevert(
-        this.accessControl.revokeRole(ROLE, authorized, { from: admin }),
-        'AccessControl: sender must be an admin to revoke',
-      );
-    });
-  });
+  shouldBehaveLikeAccessControl('AccessControl', ...accounts);
+  shouldBehaveLikeAccessControlEnumerable('AccessControl', ...accounts);
+  shouldBehaveLikeAccessControlEnumerable2('AccessControl', ...accounts);
 });

From 122d37f4e58f920e90947320b1e516163ac208e5 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Thu, 18 Feb 2021 16:06:29 +0100
Subject: [PATCH 08/16] fix lint

---
 test/access/AccessControl.test.js           | 8 +++-----
 test/access/AccessControlEnumerable.test.js | 4 ----
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/test/access/AccessControl.test.js b/test/access/AccessControl.test.js
index f829141f7bd..cd9912adb4f 100644
--- a/test/access/AccessControl.test.js
+++ b/test/access/AccessControl.test.js
@@ -1,12 +1,10 @@
-const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
-const { expect } = require('chai');
-
-const { shouldBehaveLikeAccessControl } = require('./AccessControl.behavior.js');
+const {
+  shouldBehaveLikeAccessControl,
+} = require('./AccessControl.behavior.js');
 
 const AccessControlMock = artifacts.require('AccessControlMock');
 
 contract('AccessControl', function (accounts) {
-
   beforeEach(async function () {
     this.accessControl = await AccessControlMock.new({ from: accounts[0] });
   });
diff --git a/test/access/AccessControlEnumerable.test.js b/test/access/AccessControlEnumerable.test.js
index 73076d6c2a9..193a46d894c 100644
--- a/test/access/AccessControlEnumerable.test.js
+++ b/test/access/AccessControlEnumerable.test.js
@@ -1,6 +1,3 @@
-const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
-const { expect } = require('chai');
-
 const {
   shouldBehaveLikeAccessControl,
   shouldBehaveLikeAccessControlEnumerable,
@@ -10,7 +7,6 @@ const {
 const AccessControlMock = artifacts.require('AccessControlEnumerableMock');
 
 contract('AccessControl', function (accounts) {
-
   beforeEach(async function () {
     this.accessControl = await AccessControlMock.new({ from: accounts[0] });
   });

From 681a44f48dd84b0725d68d60362ab47df2b07da6 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hc@iex.ec>
Date: Fri, 19 Feb 2021 13:42:33 +0100
Subject: [PATCH 09/16] Update contracts/access/AccessControlEnumerable.sol

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
---
 contracts/access/AccessControlEnumerable.sol | 34 +-------------------
 1 file changed, 1 insertion(+), 33 deletions(-)

diff --git a/contracts/access/AccessControlEnumerable.sol b/contracts/access/AccessControlEnumerable.sol
index 11d063e5a39..df7e583c775 100644
--- a/contracts/access/AccessControlEnumerable.sol
+++ b/contracts/access/AccessControlEnumerable.sol
@@ -6,39 +6,7 @@ import "./AccessControl.sol";
 import "../utils/EnumerableSet.sol";
 
 /**
- * @dev Contract module that allows children to implement role-based access
- * control mechanisms. Role to account enumerable version.
- *
- * Roles are referred to by their `bytes32` identifier. These should be exposed
- * in the external API and be unique. The best way to achieve this is by
- * using `public constant` hash digests:
- *
- * ```
- * bytes32 public constant MY_ROLE = keccak256("MY_ROLE");
- * ```
- *
- * Roles can be used to represent a set of permissions. To restrict access to a
- * function call, use {hasRole}:
- *
- * ```
- * function foo() public {
- *     require(hasRole(MY_ROLE, msg.sender));
- *     ...
- * }
- * ```
- *
- * Roles can be granted and revoked dynamically via the {grantRole} and
- * {revokeRole} functions. Each role has an associated admin role, and only
- * accounts that have a role's admin role can call {grantRole} and {revokeRole}.
- *
- * By default, the admin role for all roles is `DEFAULT_ADMIN_ROLE`, which means
- * that only accounts with this role will be able to grant or revoke other
- * roles. More complex role relationships can be created by using
- * {_setRoleAdmin}.
- *
- * WARNING: The `DEFAULT_ADMIN_ROLE` is also its own admin: it has permission to
- * grant and revoke this role. Extra precautions should be taken to secure
- * accounts that have been granted it.
+ * @dev Extension of {AccessControl} that allows enumerating the members of each role.
  */
 abstract contract AccessControlEnumerable is AccessControl {
     using EnumerableSet for EnumerableSet.AddressSet;

From 85084829f353ce4a4a1042b2130377a6f0abb249 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Fri, 19 Feb 2021 14:16:55 +0100
Subject: [PATCH 10/16] fallback to previous syntax (using a RoleData struct)

---
 contracts/access/AccessControl.sol | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/contracts/access/AccessControl.sol b/contracts/access/AccessControl.sol
index 7aa4dc4d11a..11aa4125eed 100644
--- a/contracts/access/AccessControl.sol
+++ b/contracts/access/AccessControl.sol
@@ -40,10 +40,13 @@ import "../utils/Context.sol";
  * accounts that have been granted it.
  */
 abstract contract AccessControl is Context {
-    bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00;
+    struct RoleData {
+        mapping (address => bool) members;
+        bytes32 adminRole;
+    }
+    mapping (bytes32 => RoleData) private _roles;
 
-    mapping (bytes32 => bytes32) private _admins;
-    mapping (bytes32 => mapping (address => bool)) private _members;
+    bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00;
 
     /**
      * @dev Emitted when `newAdminRole` is set as ``role``'s admin role, replacing `previousAdminRole`
@@ -76,7 +79,7 @@ abstract contract AccessControl is Context {
      * @dev Returns `true` if `account` has been granted `role`.
      */
     function hasRole(bytes32 role, address account) public view virtual returns (bool) {
-        return _members[role][account];
+        return _roles[role].members[account];
     }
 
     /**
@@ -86,7 +89,7 @@ abstract contract AccessControl is Context {
      * To change a role's admin, use {_setRoleAdmin}.
      */
     function getRoleAdmin(bytes32 role) public view virtual returns (bytes32) {
-        return _admins[role];
+        return _roles[role].adminRole;
     }
 
     /**
@@ -167,19 +170,19 @@ abstract contract AccessControl is Context {
      */
     function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual {
         emit RoleAdminChanged(role, AccessControl.getRoleAdmin(role), adminRole);
-        _admins[role] = adminRole;
+        _roles[role].adminRole = adminRole;
     }
 
     function _grantRole(bytes32 role, address account) internal virtual {
         if (!AccessControl.hasRole(role, account)) {
-            _members[role][account] = true;
+            _roles[role].members[account] = true;
             emit RoleGranted(role, account, _msgSender());
         }
     }
 
     function _revokeRole(bytes32 role, address account) internal virtual {
         if (AccessControl.hasRole(role, account)) {
-            _members[role][account] = false;
+            _roles[role].members[account] = false;
             emit RoleRevoked(role, account, _msgSender());
         }
     }

From fa704a2d55be52aaf08ee61784991c8da5f7c525 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Fri, 19 Feb 2021 15:56:07 +0100
Subject: [PATCH 11/16] remove AccessControlEnumerable2 for now

---
 contracts/access/AccessControlEnumerable2.sol | 87 -------------------
 .../mocks/AccessControlEnumerableMock.sol     | 15 +---
 test/access/AccessControl.behavior.js         | 22 -----
 test/access/AccessControlEnumerable.test.js   |  2 -
 4 files changed, 1 insertion(+), 125 deletions(-)
 delete mode 100644 contracts/access/AccessControlEnumerable2.sol

diff --git a/contracts/access/AccessControlEnumerable2.sol b/contracts/access/AccessControlEnumerable2.sol
deleted file mode 100644
index c3c6507115a..00000000000
--- a/contracts/access/AccessControlEnumerable2.sol
+++ /dev/null
@@ -1,87 +0,0 @@
-// SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.0;
-
-import "./AccessControl.sol";
-import "../utils/EnumerableSet.sol";
-
-/**
- * @dev Contract module that allows children to implement role-based access
- * control mechanisms. Account to role enumerable version.
- *
- * Roles are referred to by their `bytes32` identifier. These should be exposed
- * in the external API and be unique. The best way to achieve this is by
- * using `public constant` hash digests:
- *
- * ```
- * bytes32 public constant MY_ROLE = keccak256("MY_ROLE");
- * ```
- *
- * Roles can be used to represent a set of permissions. To restrict access to a
- * function call, use {hasRole}:
- *
- * ```
- * function foo() public {
- *     require(hasRole(MY_ROLE, msg.sender));
- *     ...
- * }
- * ```
- *
- * Roles can be granted and revoked dynamically via the {grantRole} and
- * {revokeRole} functions. Each role has an associated admin role, and only
- * accounts that have a role's admin role can call {grantRole} and {revokeRole}.
- *
- * By default, the admin role for all roles is `DEFAULT_ADMIN_ROLE`, which means
- * that only accounts with this role will be able to grant or revoke other
- * roles. More complex role relationships can be created by using
- * {_setRoleAdmin}.
- *
- * WARNING: The `DEFAULT_ADMIN_ROLE` is also its own admin: it has permission to
- * grant and revoke this role. Extra precautions should be taken to secure
- * accounts that have been granted it.
- */
-abstract contract AccessControlEnumerable2 is AccessControl {
-    using EnumerableSet for EnumerableSet.Bytes32Set;
-
-    mapping (address => EnumerableSet.Bytes32Set) private _addressRoles;
-
-    /**
-     * @dev Returns one of the roles that `account` has. `index` must be a
-     * value between 0 and {getAddressRoleCount}, non-inclusive.
-     *
-     * Role not sorted in any particular way, and their ordering may change at
-     * any point.
-     *
-     * WARNING: When using {getAddressRole} and {getAddressRoleCount}, make sure
-     * you perform all queries on the same block. See the following
-     * https://forum.openzeppelin.com/t/iterating-over-elements-on-enumerableset-in-openzeppelin-contracts/2296[forum post]
-     * for more information.
-     */
-    function getAddressRole(address account, uint256 index) public view returns (bytes32) {
-        return _addressRoles[account].at(index);
-    }
-
-    /**
-     * @dev Returns the number of role that `account` has. Can be used
-     * together with {getAddressRole} to enumerate all role of an account.
-     */
-    function getAddressRoleCount(address account) public view returns (uint256) {
-        return _addressRoles[account].length();
-    }
-
-    /**
-     * @dev Overload {_grantRole} to track enumerable memberships
-     */
-    function _grantRole(bytes32 role, address account) internal virtual override {
-        super._grantRole(role, account);
-        _addressRoles[account].add(role);
-    }
-
-    /**
-     * @dev Overload {_revokeRole} to track enumerable memberships
-     */
-    function _revokeRole(bytes32 role, address account) internal virtual override {
-        super._revokeRole(role, account);
-        _addressRoles[account].remove(role);
-    }
-}
diff --git a/contracts/mocks/AccessControlEnumerableMock.sol b/contracts/mocks/AccessControlEnumerableMock.sol
index 68c78dbbc8b..ec2cbbb103f 100644
--- a/contracts/mocks/AccessControlEnumerableMock.sol
+++ b/contracts/mocks/AccessControlEnumerableMock.sol
@@ -3,9 +3,8 @@
 pragma solidity ^0.8.0;
 
 import "../access/AccessControlEnumerable.sol";
-import "../access/AccessControlEnumerable2.sol";
 
-contract AccessControlEnumerableMock is AccessControlEnumerable, AccessControlEnumerable2 {
+contract AccessControlEnumerableMock is AccessControlEnumerable {
     constructor() {
         _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
     }
@@ -13,16 +12,4 @@ contract AccessControlEnumerableMock is AccessControlEnumerable, AccessControlEn
     function setRoleAdmin(bytes32 roleId, bytes32 adminRoleId) public {
         _setRoleAdmin(roleId, adminRoleId);
     }
-
-    function _grantRole(bytes32 role, address account)
-    internal virtual override(AccessControlEnumerable, AccessControlEnumerable2)
-    {
-        super._grantRole(role, account);
-    }
-
-    function _revokeRole(bytes32 role, address account)
-    internal virtual override(AccessControlEnumerable, AccessControlEnumerable2)
-    {
-        super._revokeRole(role, account);
-    }
 }
diff --git a/test/access/AccessControl.behavior.js b/test/access/AccessControl.behavior.js
index 738d653b206..5b7c32fa3c2 100644
--- a/test/access/AccessControl.behavior.js
+++ b/test/access/AccessControl.behavior.js
@@ -176,29 +176,7 @@ function shouldBehaveLikeAccessControlEnumerable (errorPrefix, admin, authorized
   });
 }
 
-function shouldBehaveLikeAccessControlEnumerable2 (errorPrefix, admin, authorized, other, otherAdmin, otherAuthorized) {
-  describe('enumerating', function () {
-    it('role beared can be enumerated', async function () {
-      await this.accessControl.grantRole(ROLE, authorized, { from: admin });
-      await this.accessControl.grantRole(DEFAULT_ADMIN_ROLE, authorized, { from: admin });
-      await this.accessControl.grantRole(OTHER_ROLE, authorized, { from: admin });
-      await this.accessControl.revokeRole(DEFAULT_ADMIN_ROLE, authorized, { from: admin });
-
-      const roleCount = await this.accessControl.getAddressRoleCount(authorized);
-      expect(roleCount).to.bignumber.equal('2');
-
-      const beared = [];
-      for (let i = 0; i < roleCount; ++i) {
-        beared.push(await this.accessControl.getAddressRole(authorized, i));
-      }
-
-      expect(beared).to.have.members([ROLE, OTHER_ROLE]);
-    });
-  });
-}
-
 module.exports = {
   shouldBehaveLikeAccessControl,
   shouldBehaveLikeAccessControlEnumerable,
-  shouldBehaveLikeAccessControlEnumerable2,
 };
diff --git a/test/access/AccessControlEnumerable.test.js b/test/access/AccessControlEnumerable.test.js
index 193a46d894c..fa5b54691da 100644
--- a/test/access/AccessControlEnumerable.test.js
+++ b/test/access/AccessControlEnumerable.test.js
@@ -1,7 +1,6 @@
 const {
   shouldBehaveLikeAccessControl,
   shouldBehaveLikeAccessControlEnumerable,
-  shouldBehaveLikeAccessControlEnumerable2,
 } = require('./AccessControl.behavior.js');
 
 const AccessControlMock = artifacts.require('AccessControlEnumerableMock');
@@ -13,5 +12,4 @@ contract('AccessControl', function (accounts) {
 
   shouldBehaveLikeAccessControl('AccessControl', ...accounts);
   shouldBehaveLikeAccessControlEnumerable('AccessControl', ...accounts);
-  shouldBehaveLikeAccessControlEnumerable2('AccessControl', ...accounts);
 });

From db0b96bc3c3ce3305906afa2c047ef80474202d6 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Fri, 19 Feb 2021 18:06:28 +0100
Subject: [PATCH 12/16] view function should not be virtual in AccessControl

---
 contracts/access/AccessControl.sol           | 19 ++++++++++---------
 contracts/access/AccessControlEnumerable.sol | 12 ++++++------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/contracts/access/AccessControl.sol b/contracts/access/AccessControl.sol
index 11aa4125eed..a9adc278071 100644
--- a/contracts/access/AccessControl.sol
+++ b/contracts/access/AccessControl.sol
@@ -44,6 +44,7 @@ abstract contract AccessControl is Context {
         mapping (address => bool) members;
         bytes32 adminRole;
     }
+
     mapping (bytes32 => RoleData) private _roles;
 
     bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00;
@@ -78,7 +79,7 @@ abstract contract AccessControl is Context {
     /**
      * @dev Returns `true` if `account` has been granted `role`.
      */
-    function hasRole(bytes32 role, address account) public view virtual returns (bool) {
+    function hasRole(bytes32 role, address account) public view returns (bool) {
         return _roles[role].members[account];
     }
 
@@ -88,7 +89,7 @@ abstract contract AccessControl is Context {
      *
      * To change a role's admin, use {_setRoleAdmin}.
      */
-    function getRoleAdmin(bytes32 role) public view virtual returns (bytes32) {
+    function getRoleAdmin(bytes32 role) public view returns (bytes32) {
         return _roles[role].adminRole;
     }
 
@@ -103,7 +104,7 @@ abstract contract AccessControl is Context {
      * - the caller must have ``role``'s admin role.
      */
     function grantRole(bytes32 role, address account) public virtual {
-        require(AccessControl.hasRole(AccessControl.getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to grant");
+        require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to grant");
 
         _grantRole(role, account);
     }
@@ -118,7 +119,7 @@ abstract contract AccessControl is Context {
      * - the caller must have ``role``'s admin role.
      */
     function revokeRole(bytes32 role, address account) public virtual {
-        require(AccessControl.hasRole(AccessControl.getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to revoke");
+        require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to revoke");
 
         _revokeRole(role, account);
     }
@@ -169,19 +170,19 @@ abstract contract AccessControl is Context {
      * Emits a {RoleAdminChanged} event.
      */
     function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual {
-        emit RoleAdminChanged(role, AccessControl.getRoleAdmin(role), adminRole);
+        emit RoleAdminChanged(role, getRoleAdmin(role), adminRole);
         _roles[role].adminRole = adminRole;
     }
 
-    function _grantRole(bytes32 role, address account) internal virtual {
-        if (!AccessControl.hasRole(role, account)) {
+    function _grantRole(bytes32 role, address account) private {
+        if (!hasRole(role, account)) {
             _roles[role].members[account] = true;
             emit RoleGranted(role, account, _msgSender());
         }
     }
 
-    function _revokeRole(bytes32 role, address account) internal virtual {
-        if (AccessControl.hasRole(role, account)) {
+    function _revokeRole(bytes32 role, address account) private {
+        if (hasRole(role, account)) {
             _roles[role].members[account] = false;
             emit RoleRevoked(role, account, _msgSender());
         }
diff --git a/contracts/access/AccessControlEnumerable.sol b/contracts/access/AccessControlEnumerable.sol
index df7e583c775..3dfc9cd0e3e 100644
--- a/contracts/access/AccessControlEnumerable.sol
+++ b/contracts/access/AccessControlEnumerable.sol
@@ -38,18 +38,18 @@ abstract contract AccessControlEnumerable is AccessControl {
     }
 
     /**
-     * @dev Overload {_grantRole} to track enumerable memberships
+     * @dev Overload {grantRole} to track enumerable memberships
      */
-    function _grantRole(bytes32 role, address account) internal virtual override {
-        super._grantRole(role, account);
+    function grantRole(bytes32 role, address account) public virtual override {
+        super.grantRole(role, account);
         _roleMembers[role].add(account);
     }
 
     /**
-     * @dev Overload {_revokeRole} to track enumerable memberships
+     * @dev Overload {revokeRole} to track enumerable memberships
      */
-    function _revokeRole(bytes32 role, address account) internal virtual override {
-        super._revokeRole(role, account);
+    function revokeRole(bytes32 role, address account) public virtual override {
+        super.revokeRole(role, account);
         _roleMembers[role].remove(account);
     }
 }

From f2dcdb2e68c9d47f4b9e1feabe434618527b9465 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Fri, 19 Feb 2021 18:20:54 +0100
Subject: [PATCH 13/16] fix enumerability of role given using _setupRole

---
 contracts/access/AccessControlEnumerable.sol | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/contracts/access/AccessControlEnumerable.sol b/contracts/access/AccessControlEnumerable.sol
index 3dfc9cd0e3e..be58a7b8de8 100644
--- a/contracts/access/AccessControlEnumerable.sol
+++ b/contracts/access/AccessControlEnumerable.sol
@@ -52,4 +52,12 @@ abstract contract AccessControlEnumerable is AccessControl {
         super.revokeRole(role, account);
         _roleMembers[role].remove(account);
     }
+
+    /**
+     * @dev Overload {_setupRole} to track enumerable memberships
+     */
+    function _setupRole(bytes32 role, address account) internal virtual override {
+        super._setupRole(role, account);
+        _roleMembers[role].add(account);
+    }
 }

From 9c11117890c2cab12391bf1cd8a2228dccd30210 Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hc@iex.ec>
Date: Fri, 19 Feb 2021 18:25:34 +0100
Subject: [PATCH 14/16] Update contracts/access/AccessControl.sol

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
---
 contracts/access/AccessControl.sol | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/contracts/access/AccessControl.sol b/contracts/access/AccessControl.sol
index a9adc278071..708ec9adac2 100644
--- a/contracts/access/AccessControl.sol
+++ b/contracts/access/AccessControl.sol
@@ -6,7 +6,10 @@ import "../utils/Context.sol";
 
 /**
  * @dev Contract module that allows children to implement role-based access
- * control mechanisms. Light, not enumerable, version.
+ * control mechanisms. This is a lightweight version that doesn't allow enumerating role
+ * members except through off-chain means by accessing the contract event logs. Some
+ * applications may benefit from on-chain enumerability, for those cases see
+ * {AccessControlEnumerable}.
  *
  * Roles are referred to by their `bytes32` identifier. These should be exposed
  * in the external API and be unique. The best way to achieve this is by

From c300783e570bd3b41c43a342734647e00485bcdc Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Fri, 19 Feb 2021 18:26:14 +0100
Subject: [PATCH 15/16] add {{AccessControlEnumerable}} entry in
 access/README.adoc

---
 contracts/access/README.adoc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/contracts/access/README.adoc b/contracts/access/README.adoc
index 67496c57e2a..c1431c1200e 100644
--- a/contracts/access/README.adoc
+++ b/contracts/access/README.adoc
@@ -15,6 +15,8 @@ This directory provides ways to restrict who can access the functions of a contr
 
 {{AccessControl}}
 
+{{AccessControlEnumerable}}
+
 == Timelock
 
 {{TimelockController}}

From c8d3344373cfab2972dfc40ddda0cb3eb0233341 Mon Sep 17 00:00:00 2001
From: Francisco Giordano <frangio.1@gmail.com>
Date: Fri, 19 Feb 2021 14:31:22 -0300
Subject: [PATCH 16/16] add changelog entry

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3bf5f9ab6c3..f902f6c0c08 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@
  * `ERC165`: Remove uses of storage in the base ERC165 implementation. ERC165 based contracts now use storage-less virtual functions. Old behaviour remains available in the `ERC165Storage` extension. ([#2505](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2505))
  * `Initializable`: Make initializer check stricter during construction. ([#2531](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2531))
  * `ERC721`: remove enumerability of tokens from the base implementation. This feature is now provided separately through the `ERC721Enumerable` extension. ([#2511](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2511))
+ * `AccessControl`: removed enumerability by default for a more lightweight contract. It is now opt-in through `AccessControlEnumerable`. ([#2512](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2512))
 
 ## 3.4.0 (2021-02-02)