Merge upstream openzeppelin-contracts into upstream-patched

CHANGELOG.md

@@ -28,6 +28,9 @@
  * `GSN`: Deprecate GSNv1 support in favor of upcomming support for GSNv2. ([#2521](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2521))
  * `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))
+ * Meta Transactions: add `ERC2771Context` and a `MinimalForwarder` for meta-transactions. ([#2508](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2508))
 
 ## 3.4.0 (2021-02-02)
 

contracts/access/AccessControl.sol

@@ -2,13 +2,14 @@
 
 pragma solidity ^0.8.0;
 
-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. 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
@@ -42,11 +43,8 @@ import "../utils/Context.sol";
  * accounts that have been granted it.
  */
 abstract contract AccessControl is Context {
-    using EnumerableSet for EnumerableSet.AddressSet;
-    using Address for address;
-
     struct RoleData {
-        EnumerableSet.AddressSet members;
+        mapping (address => bool) members;
         bytes32 adminRole;
     }
 
@@ -85,31 +83,7 @@ abstract contract AccessControl is Context {
      * @dev Returns `true` if `account` has been granted `role`.
      */
     function hasRole(bytes32 role, address account) public view 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);
+        return _roles[role].members[account];
     }
 
     /**
@@ -133,7 +107,7 @@ abstract contract AccessControl is Context {
      * - 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");
+        require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to grant");
 
         _grantRole(role, account);
     }
@@ -148,7 +122,7 @@ abstract contract AccessControl is Context {
      * - 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");
+        require(hasRole(getRoleAdmin(role), _msgSender()), "AccessControl: sender must be an admin to revoke");
 
         _revokeRole(role, account);
     }
@@ -199,18 +173,20 @@ abstract contract AccessControl is Context {
      * Emits a {RoleAdminChanged} event.
      */
     function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual {
-        emit RoleAdminChanged(role, _roles[role].adminRole, adminRole);
+        emit RoleAdminChanged(role, getRoleAdmin(role), adminRole);
         _roles[role].adminRole = adminRole;
     }
 
     function _grantRole(bytes32 role, address account) private {
-        if (_roles[role].members.add(account)) {
+        if (!hasRole(role, account)) {
+            _roles[role].members[account] = true;
             emit RoleGranted(role, account, _msgSender());
         }
     }
 
     function _revokeRole(bytes32 role, address account) private {
-        if (_roles[role].members.remove(account)) {
+        if (hasRole(role, account)) {
+            _roles[role].members[account] = false;
             emit RoleRevoked(role, account, _msgSender());
         }
     }

contracts/access/AccessControlEnumerable.sol

+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "./AccessControl.sol";
+import "../utils/EnumerableSet.sol";
+
+/**
+ * @dev Extension of {AccessControl} that allows enumerating the members of each role.
+ */
+abstract contract AccessControlEnumerable is AccessControl {
+    using EnumerableSet for EnumerableSet.AddressSet;
+
+    mapping (bytes32 => EnumerableSet.AddressSet) private _roleMembers;
+
+    /**
+     * @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 _roleMembers[role].at(index);
+    }
+
+    /**
+     * @dev Returns the number of accounts that have `role`. Can be used
+     * together with {getRoleMember} to enumerate all bearers of a role.
+     */
+    function getRoleMemberCount(bytes32 role) public view returns (uint256) {
+        return _roleMembers[role].length();
+    }
+
+    /**
+     * @dev Overload {grantRole} to track enumerable memberships
+     */
+    function grantRole(bytes32 role, address account) public virtual override {
+        super.grantRole(role, account);
+        _roleMembers[role].add(account);
+    }
+
+    /**
+     * @dev Overload {revokeRole} to track enumerable memberships
+     */
+    function revokeRole(bytes32 role, address account) public virtual override {
+        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);
+    }
+}

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}}

contracts/cryptography/ECDSA.sol

@@ -72,9 +72,9 @@ library ECDSA {
 
     /**
      * @dev Returns an Ethereum Signed Message, created from a `hash`. This
-     * replicates the behavior of the
-     * https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_sign[`eth_sign`]
-     * JSON-RPC method.
+     * produces hash corresponding to the one signed with the
+     * https://eth.wiki/json-rpc/API#eth_sign[`eth_sign`]
+     * JSON-RPC method as part of EIP-191.
      *
      * See {recover}.
      */
@@ -83,4 +83,17 @@ library ECDSA {
         // enforced by the type signature above
         return keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", hash));
     }
+
+    /**
+     * @dev Returns an Ethereum Signed Typed Data, created from a
+     * `domainSeparator` and a `structHash`. This produces hash corresponding
+     * to the one signed with the
+     * https://eips.ethereum.org/EIPS/eip-712[`eth_signTypedData`]
+     * JSON-RPC method as part of EIP-712.
+     *
+     * See {recover}.
+     */
+    function toTypedDataHash(bytes32 domainSeparator, bytes32 structHash) internal pure returns (bytes32) {
+        return keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash));
+    }
 }

contracts/drafts/EIP712.sol

@@ -2,6 +2,8 @@
 
 pragma solidity ^0.8.0;
 
+import "../cryptography/ECDSA.sol";
+
 /**
  * @dev https://eips.ethereum.org/EIPS/eip-712[EIP 712] is a standard for hashing and signing of typed structured data.
  *
@@ -82,7 +84,7 @@ abstract contract EIP712 {
      * ```
      */
     function _hashTypedDataV4(bytes32 structHash) internal view virtual returns (bytes32) {
-        return keccak256(abi.encodePacked("\x19\x01", _domainSeparatorV4(), structHash));
+        return ECDSA.toTypedDataHash(_domainSeparatorV4(), structHash);
     }
 
     /**

contracts/math/SafeMath.sol

@@ -2,6 +2,10 @@
 
 pragma solidity ^0.8.0;
 
+// CAUTION
+// This version of SafeMath should only be used with Solidity 0.8 or later,
+// because it relies on the compiler's built in overflow checks.
+
 /**
  * @dev Wrappers over Solidity's arithmetic operations.
  */

contracts/metatx/ERC2771Context.sol

+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "../utils/Context.sol";
+
+/*
+ * @dev Context variant with ERC2771 support.
+ */
+abstract contract ERC2771Context is Context {
+    address immutable _trustedForwarder;
+
+    constructor(address trustedForwarder) {
+        _trustedForwarder = trustedForwarder;
+    }
+
+    function isTrustedForwarder(address forwarder) public view virtual returns(bool) {
+        return forwarder == _trustedForwarder;
+    }
+
+    function _msgSender() internal view virtual override returns (address sender) {
+        if (isTrustedForwarder(msg.sender)) {
+            // The assembly code is more direct than the Solidity version using `abi.decode`.
+            assembly { sender := shr(96, calldataload(sub(calldatasize(), 20))) }
+        } else {
+            return super._msgSender();
+        }
+    }
+
+    function _msgData() internal view virtual override returns (bytes calldata) {
+        if (isTrustedForwarder(msg.sender)) {
+            return msg.data[:msg.data.length-20];
+        } else {
+            return super._msgData();
+        }
+    }
+}

contracts/metatx/MinimalForwarder.sol

+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "../cryptography/ECDSA.sol";
+import "../drafts/EIP712.sol";
+
+/*
+ * @dev Simple minimal forwarder to be used together with an ERC2771 compatible contract. See {ERC2771Context}.
+ */
+contract MinimalForwarder is EIP712 {
+    using ECDSA for bytes32;
+
+    struct ForwardRequest {
+        address from;
+        address to;
+        uint256 value;
+        uint256 gas;
+        uint256 nonce;
+        bytes data;
+    }
+
+    bytes32 private constant TYPEHASH = keccak256("ForwardRequest(address from,address to,uint256 value,uint256 gas,uint256 nonce,bytes data)");
+
+    mapping(address => uint256) private _nonces;
+
+    constructor() EIP712("MinimalForwarder", "0.0.1") {}
+
+    function getNonce(address from) public view returns (uint256) {
+        return _nonces[from];
+    }
+
+    function verify(ForwardRequest calldata req, bytes calldata signature) public view returns (bool) {
+        address signer = _hashTypedDataV4(keccak256(abi.encode(
+            TYPEHASH,
+            req.from,
+            req.to,
+            req.value,
+            req.gas,
+            req.nonce,
+            keccak256(req.data)
+        ))).recover(signature);
+        return _nonces[req.from] == req.nonce && signer == req.from;
+    }
+
+    function execute(ForwardRequest calldata req, bytes calldata signature) public payable returns (bool, bytes memory) {
+        require(verify(req, signature), "MinimalForwarder: signature does not match request");
+        _nonces[req.from] = req.nonce + 1;
+
+        // solhint-disable-next-line avoid-low-level-calls
+        (bool success, bytes memory returndata) = req.to.call{gas: req.gas, value: req.value}(abi.encodePacked(req.data, req.from));
+        // Validate that the relayer has sent enough gas for the call.
+        // See https://ronan.eth.link/blog/ethereum-gas-dangers/
+        assert(gasleft() > req.gas / 63);
+
+        return (success, returndata);
+    }
+}

contracts/metatx/README.adoc

+= Meta Transactions
+
+[.readme-notice]
+NOTE: This document is better viewed at https://docs.openzeppelin.com/contracts/api/math
+
+== Core
+
+{{ERC2771Context}}
+
+== Utils
+
+{{MinimalForwarder}}

contracts/mocks/AccessControlEnumerableMock.sol

+// 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);
+    }
+}

contracts/mocks/ERC2771ContextMock.sol

+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "./ContextMock.sol";
+import "../metatx/ERC2771Context.sol";
+
+// By inheriting from ERC2771Context, Context's internal functions are overridden automatically
+contract ERC2771ContextMock is ContextMock, ERC2771Context {
+    constructor(address trustedForwarder) ERC2771Context(trustedForwarder) {}
+
+    function _msgSender() internal override(Context, ERC2771Context) view virtual returns (address) {
+        return ERC2771Context._msgSender();
+    }
+
+    function _msgData() internal override(Context, ERC2771Context) view virtual returns (bytes calldata) {
+        return ERC2771Context._msgData();
+    }
+}

contracts/mocks/ERC721EnumerableMock.sol

+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "../token/ERC721/ERC721Enumerable.sol";
+
+/**
+ * @title ERC721Mock
+ * This mock just provides a public safeMint, mint, and burn functions for testing purposes
+ */
+contract ERC721EnumerableMock is ERC721Enumerable {
+    string private _baseTokenURI;
+
+    constructor (string memory name, string memory symbol) ERC721(name, symbol) { }
+
+    function _baseURI() internal view virtual override returns (string memory) {
+        return _baseTokenURI;
+    }
+
+    function setBaseURI(string calldata newBaseTokenURI) public {
+        _baseTokenURI = newBaseTokenURI;
+    }
+
+    function baseURI() public view returns (string memory) {
+        return _baseURI();
+    }
+
+    function mint(address to, uint256 tokenId) public {
+        _mint(to, tokenId);
+    }
+
+    function safeMint(address to, uint256 tokenId) public {
+        _safeMint(to, tokenId);
+    }
+
+    function safeMint(address to, uint256 tokenId, bytes memory _data) public {
+        _safeMint(to, tokenId, _data);
+    }
+
+    function burn(uint256 tokenId) public {
+        _burn(tokenId);
+    }
+}

contracts/mocks/ERC721Mock.sol

@@ -9,18 +9,24 @@ import "../token/ERC721/ERC721.sol";
  * This mock just provides a public safeMint, mint, and burn functions for testing purposes
  */
 contract ERC721Mock is ERC721 {
+    string private _baseTokenURI;
+
     constructor (string memory name, string memory symbol) ERC721(name, symbol) { }
 
-    function exists(uint256 tokenId) public view returns (bool) {
-        return _exists(tokenId);
+    function _baseURI() internal view virtual override returns (string memory) {
+        return _baseTokenURI;
     }
 
-    function setTokenURI(uint256 tokenId, string memory uri) public {
-        _setTokenURI(tokenId, uri);
+    function setBaseURI(string calldata newBaseTokenURI) public {
+        _baseTokenURI = newBaseTokenURI;
     }
 
-    function setBaseURI(string memory baseURI) public {
-        _setBaseURI(baseURI);
+    function baseURI() public view returns (string memory) {
+        return _baseURI();
+    }
+
+    function exists(uint256 tokenId) public view returns (bool) {
+        return _exists(tokenId);
     }
 
     function mint(address to, uint256 tokenId) public {

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");
 

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");
 

contracts/presets/ERC721PresetMinterPauserAutoId.sol

@@ -2,10 +2,11 @@
 
 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";
+import "../token/ERC721/ERC721Enumerable.sol";
 import "../token/ERC721/ERC721Burnable.sol";
 import "../token/ERC721/ERC721Pausable.sol";
 
@@ -24,7 +25,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, ERC721Enumerable, ERC721Burnable, ERC721Pausable {
     using Counters for Counters.Counter;
 
     bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
@@ -32,6 +33,8 @@ contract ERC721PresetMinterPauserAutoId is Context, AccessControl, ERC721Burnabl
 
     Counters.Counter private _tokenIdTracker;
 
+    string private _baseTokenURI;
+
     /**
      * @dev Grants `DEFAULT_ADMIN_ROLE`, `MINTER_ROLE` and `PAUSER_ROLE` to the
      * account that deploys the contract.
@@ -39,13 +42,17 @@ contract ERC721PresetMinterPauserAutoId is Context, AccessControl, ERC721Burnabl
      * Token URIs will be autogenerated based on `baseURI` and their token IDs.
      * See {ERC721-tokenURI}.
      */
-    constructor(string memory name, string memory symbol, string memory baseURI) ERC721(name, symbol) {
+    constructor(string memory name, string memory symbol, string memory baseTokenURI) ERC721(name, symbol) {
+        _baseTokenURI = baseTokenURI;
+
         _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
 
         _setupRole(MINTER_ROLE, _msgSender());
         _setupRole(PAUSER_ROLE, _msgSender());
+    }
 
-        _setBaseURI(baseURI);
+    function _baseURI() internal view virtual override returns (string memory) {
+        return _baseTokenURI;
     }
 
     /**
@@ -96,7 +103,14 @@ contract ERC721PresetMinterPauserAutoId is Context, AccessControl, ERC721Burnabl
         _unpause();
     }
 
-    function _beforeTokenTransfer(address from, address to, uint256 tokenId) internal virtual override(ERC721, ERC721Pausable) {
+    function _beforeTokenTransfer(address from, address to, uint256 tokenId) internal virtual override(ERC721, ERC721Enumerable, ERC721Pausable) {
         super._beforeTokenTransfer(from, to, tokenId);
     }
+
+    /**
+     * @dev See {IERC165-supportsInterface}.
+     */
+    function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721, ERC721Enumerable) returns (bool) {
+        return super.supportsInterface(interfaceId);
+    }
 }

contracts/token/ERC721/ERC721.sol

@@ -5,47 +5,37 @@ pragma solidity ^0.8.0;
 import "../../utils/Context.sol";
 import "./IERC721.sol";
 import "./IERC721Metadata.sol";
-import "./IERC721Enumerable.sol";
 import "./IERC721Receiver.sol";
 import "../../introspection/ERC165.sol";
 import "../../utils/Address.sol";
-import "../../utils/EnumerableSet.sol";
-import "../../utils/EnumerableMap.sol";
 import "../../utils/Strings.sol";
 
 /**
- * @title ERC721 Non-Fungible Token Standard basic implementation
- * @dev see https://eips.ethereum.org/EIPS/eip-721
+ * @dev Implementation of https://eips.ethereum.org/EIPS/eip-721[ERC721] Non-Fungible Token Standard, including
+ * the Metadata extension, but not including the Enumerable extension, which is available separately as
+ * {ERC721Enumerable}.
  */
-contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable {
+contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {
     using Address for address;
-    using EnumerableSet for EnumerableSet.UintSet;
-    using EnumerableMap for EnumerableMap.UintToAddressMap;
     using Strings for uint256;
 
-    // Mapping from holder address to their (enumerable) set of owned tokens
-    mapping (address => EnumerableSet.UintSet) private _holderTokens;
-
-    // Enumerable mapping from token ids to their owners
-    EnumerableMap.UintToAddressMap private _tokenOwners;
-
-    // Mapping from token ID to approved address
-    mapping (uint256 => address) private _tokenApprovals;
-
-    // Mapping from owner to operator approvals
-    mapping (address => mapping (address => bool)) private _operatorApprovals;
-
     // Token name
     string private _name;
 
     // Token symbol
     string private _symbol;
 
-    // Optional mapping for token URIs
-    mapping (uint256 => string) private _tokenURIs;
+    // Mapping from token ID to owner address
+    mapping (uint256 => address) private _owners;
+
+    // Mapping owner address to token count
+    mapping (address => uint256) private _balances;
 
-    // Base URI
-    string private _baseURI;
+    // Mapping from token ID to approved address
+    mapping (uint256 => address) private _tokenApprovals;
+
+    // Mapping from owner to operator approvals
+    mapping (address => mapping (address => bool)) private _operatorApprovals;
 
     /**
      * @dev Initializes the contract by setting a `name` and a `symbol` to the token collection.
@@ -61,7 +51,6 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
     function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) {
         return interfaceId == type(IERC721).interfaceId
             || interfaceId == type(IERC721Metadata).interfaceId
-            || interfaceId == type(IERC721Enumerable).interfaceId
             || super.supportsInterface(interfaceId);
     }
 
@@ -70,14 +59,16 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
      */
     function balanceOf(address owner) public view virtual override returns (uint256) {
         require(owner != address(0), "ERC721: balance query for the zero address");
-        return _holderTokens[owner].length();
+        return _balances[owner];
     }
 
     /**
      * @dev See {IERC721-ownerOf}.
      */
     function ownerOf(uint256 tokenId) public view virtual override returns (address) {
-        return _tokenOwners.get(tokenId, "ERC721: owner query for nonexistent token");
+        address owner = _owners[tokenId];
+        require(owner != address(0), "ERC721: owner query for nonexistent token");
+        return owner;
     }
 
     /**
@@ -100,51 +91,18 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
     function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
         require(_exists(tokenId), "ERC721Metadata: URI query for nonexistent token");
 
-        string memory _tokenURI = _tokenURIs[tokenId];
-        string memory base = baseURI();
-
-        // If there is no base URI, return the token URI.
-        if (bytes(base).length == 0) {
-            return _tokenURI;
-        }
-        // If both are set, concatenate the baseURI and tokenURI (via abi.encodePacked).
-        if (bytes(_tokenURI).length > 0) {
-            return string(abi.encodePacked(base, _tokenURI));
-        }
-        // If there is a baseURI but no tokenURI, concatenate the tokenID to the baseURI.
-        return string(abi.encodePacked(base, tokenId.toString()));
-    }
-
-    /**
-    * @dev Returns the base URI set via {_setBaseURI}. This will be
-    * automatically added as a prefix in {tokenURI} to each token's URI, or
-    * to the token ID if no specific URI is set for that token ID.
-    */
-    function baseURI() public view virtual returns (string memory) {
-        return _baseURI;
-    }
-
-    /**
-     * @dev See {IERC721Enumerable-tokenOfOwnerByIndex}.
-     */
-    function tokenOfOwnerByIndex(address owner, uint256 index) public view virtual override returns (uint256) {
-        return _holderTokens[owner].at(index);
-    }
-
-    /**
-     * @dev See {IERC721Enumerable-totalSupply}.
-     */
-    function totalSupply() public view virtual override returns (uint256) {
-        // _tokenOwners are indexed by tokenIds, so .length() returns the number of tokenIds
-        return _tokenOwners.length();
+        string memory baseURI = _baseURI();
+        return bytes(baseURI).length > 0
+            ? string(abi.encodePacked(baseURI, tokenId.toString()))
+            : '';
     }
 
     /**
-     * @dev See {IERC721Enumerable-tokenByIndex}.
+     * @dev Base URI for computing {tokenURI}. Empty by default, can be overriden
+     * in child contracts.
      */
-    function tokenByIndex(uint256 index) public view virtual override returns (uint256) {
-        (uint256 tokenId, ) = _tokenOwners.at(index);
-        return tokenId;
+    function _baseURI() internal view virtual returns (string memory) {
+        return "";
     }
 
     /**
@@ -244,7 +202,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
      * and stop existing when they are burned (`_burn`).
      */
     function _exists(uint256 tokenId) internal view virtual returns (bool) {
-        return _tokenOwners.contains(tokenId);
+        return _owners[tokenId] != address(0);
     }
 
     /**
@@ -301,9 +259,8 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
 
         _beforeTokenTransfer(address(0), to, tokenId);
 
-        _holderTokens[to].add(tokenId);
-
-        _tokenOwners.set(tokenId, to);
+        _balances[to] += 1;
+        _owners[tokenId] = to;
 
         emit Transfer(address(0), to, tokenId);
     }
@@ -319,21 +276,15 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
      * Emits a {Transfer} event.
      */
     function _burn(uint256 tokenId) internal virtual {
-        address owner = ERC721.ownerOf(tokenId); // internal owner
+        address owner = ERC721.ownerOf(tokenId);
 
         _beforeTokenTransfer(owner, address(0), tokenId);
 
         // Clear approvals
         _approve(address(0), tokenId);
 
-        // Clear metadata (if any)
-        if (bytes(_tokenURIs[tokenId]).length != 0) {
-            delete _tokenURIs[tokenId];
-        }
-
-        _holderTokens[owner].remove(tokenId);
-
-        _tokenOwners.remove(tokenId);
+        _balances[owner] -= 1;
+        delete _owners[tokenId];
 
         emit Transfer(owner, address(0), tokenId);
     }
@@ -350,7 +301,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
      * Emits a {Transfer} event.
      */
     function _transfer(address from, address to, uint256 tokenId) internal virtual {
-        require(ERC721.ownerOf(tokenId) == from, "ERC721: transfer of token that is not own"); // internal owner
+        require(ERC721.ownerOf(tokenId) == from, "ERC721: transfer of token that is not own");
         require(to != address(0), "ERC721: transfer to the zero address");
 
         _beforeTokenTransfer(from, to, tokenId);
@@ -358,33 +309,21 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
         // Clear approvals from the previous owner
         _approve(address(0), tokenId);
 
-        _holderTokens[from].remove(tokenId);
-        _holderTokens[to].add(tokenId);
-
-        _tokenOwners.set(tokenId, to);
+        _balances[from] -= 1;
+        _balances[to] += 1;
+        _owners[tokenId] = to;
 
         emit Transfer(from, to, tokenId);
     }
 
     /**
-     * @dev Sets `_tokenURI` as the tokenURI of `tokenId`.
+     * @dev Approve `to` to operate on `tokenId`
      *
-     * Requirements:
-     *
-     * - `tokenId` must exist.
+     * Emits a {Approval} event.
      */
-    function _setTokenURI(uint256 tokenId, string memory _tokenURI) internal virtual {
-        require(_exists(tokenId), "ERC721Metadata: URI set of nonexistent token");
-        _tokenURIs[tokenId] = _tokenURI;
-    }
-
-    /**
-     * @dev Internal function to set the base URI for all token IDs. It is
-     * automatically added as a prefix to the value returned in {tokenURI},
-     * or to the token ID if {tokenURI} is empty.
-     */
-    function _setBaseURI(string memory baseURI_) internal virtual {
-        _baseURI = baseURI_;
+    function _approve(address to, uint256 tokenId) internal virtual {
+        _tokenApprovals[tokenId] = to;
+        emit Approval(ERC721.ownerOf(tokenId), to, tokenId);
     }
 
     /**
@@ -418,11 +357,6 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
         }
     }
 
-    function _approve(address to, uint256 tokenId) private {
-        _tokenApprovals[tokenId] = to;
-        emit Approval(ERC721.ownerOf(tokenId), to, tokenId); // internal owner
-    }
-
     /**
      * @dev Hook that is called before any token transfer. This includes minting
      * and burning.

contracts/token/ERC721/ERC721Enumerable.sol

+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "./ERC721.sol";
+import "./IERC721Enumerable.sol";
+
+/**
+ * @dev This implements an optional extension of {ERC721} defined in the EIP that adds
+ * enumerability of all the token ids in the contract as well as all token ids owned by each
+ * account.
+ */
+abstract contract ERC721Enumerable is ERC721, IERC721Enumerable {
+    // Mapping from owner to list of owned token IDs
+    mapping(address => mapping(uint256 => uint256)) private _ownedTokens;
+
+    // Mapping from token ID to index of the owner tokens list
+    mapping(uint256 => uint256) private _ownedTokensIndex;
+
+    // Array with all token ids, used for enumeration
+    uint256[] private _allTokens;
+
+    // Mapping from token id to position in the allTokens array
+    mapping(uint256 => uint256) private _allTokensIndex;
+
+    /**
+     * @dev See {IERC165-supportsInterface}.
+     */
+    function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC721) returns (bool) {
+        return interfaceId == type(IERC721Enumerable).interfaceId
+            || super.supportsInterface(interfaceId);
+    }
+
+    /**
+     * @dev See {IERC721Enumerable-tokenOfOwnerByIndex}.
+     */
+    function tokenOfOwnerByIndex(address owner, uint256 index) public view virtual override returns (uint256) {
+        require(index < ERC721.balanceOf(owner), "ERC721Enumerable: owner index out of bounds");
+        return _ownedTokens[owner][index];
+    }
+
+    /**
+     * @dev See {IERC721Enumerable-totalSupply}.
+     */
+    function totalSupply() public view virtual override returns (uint256) {
+        return _allTokens.length;
+    }
+
+    /**
+     * @dev See {IERC721Enumerable-tokenByIndex}.
+     */
+    function tokenByIndex(uint256 index) public view virtual override returns (uint256) {
+        require(index < ERC721Enumerable.totalSupply(), "ERC721Enumerable: global index out of bounds");
+        return _allTokens[index];
+    }
+
+    /**
+     * @dev Hook that is called before any token transfer. This includes minting
+     * and burning.
+     *
+     * Calling conditions:
+     *
+     * - When `from` and `to` are both non-zero, ``from``'s `tokenId` will be
+     * transferred to `to`.
+     * - When `from` is zero, `tokenId` will be minted for `to`.
+     * - When `to` is zero, ``from``'s `tokenId` will be burned.
+     * - `from` cannot be the zero address.
+     * - `to` cannot be the zero address.
+     *
+     * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks].
+     */
+    function _beforeTokenTransfer(address from, address to, uint256 tokenId) internal virtual override {
+        super._beforeTokenTransfer(from, to, tokenId);
+
+        if (from == address(0)) {
+            _addTokenToAllTokensEnumeration(tokenId);
+        } else if (from != to) {
+            _removeTokenFromOwnerEnumeration(from, tokenId);
+        }
+        if (to == address(0)) {
+            _removeTokenFromAllTokensEnumeration(tokenId);
+        } else if (to != from) {
+            _addTokenToOwnerEnumeration(to, tokenId);
+        }
+    }
+
+    /**
+     * @dev Private function to add a token to this extension's ownership-tracking data structures.
+     * @param to address representing the new owner of the given token ID
+     * @param tokenId uint256 ID of the token to be added to the tokens list of the given address
+     */
+    function _addTokenToOwnerEnumeration(address to, uint256 tokenId) private {
+        uint256 length = ERC721.balanceOf(to);
+        _ownedTokens[to][length] = tokenId;
+        _ownedTokensIndex[tokenId] = length;
+    }
+
+    /**
+     * @dev Private function to add a token to this extension's token tracking data structures.
+     * @param tokenId uint256 ID of the token to be added to the tokens list
+     */
+    function _addTokenToAllTokensEnumeration(uint256 tokenId) private {
+        _allTokensIndex[tokenId] = _allTokens.length;
+        _allTokens.push(tokenId);
+    }
+
+    /**
+     * @dev Private function to remove a token from this extension's ownership-tracking data structures. Note that
+     * while the token is not assigned a new owner, the `_ownedTokensIndex` mapping is _not_ updated: this allows for
+     * gas optimizations e.g. when performing a transfer operation (avoiding double writes).
+     * This has O(1) time complexity, but alters the order of the _ownedTokens array.
+     * @param from address representing the previous owner of the given token ID
+     * @param tokenId uint256 ID of the token to be removed from the tokens list of the given address
+     */
+    function _removeTokenFromOwnerEnumeration(address from, uint256 tokenId) private {
+        // To prevent a gap in from's tokens array, we store the last token in the index of the token to delete, and
+        // then delete the last slot (swap and pop).
+
+        uint256 lastTokenIndex = ERC721.balanceOf(from) - 1;
+        uint256 tokenIndex = _ownedTokensIndex[tokenId];
+
+        // When the token to delete is the last token, the swap operation is unnecessary
+        if (tokenIndex != lastTokenIndex) {
+            uint256 lastTokenId = _ownedTokens[from][lastTokenIndex];
+
+            _ownedTokens[from][tokenIndex] = lastTokenId; // Move the last token to the slot of the to-delete token
+            _ownedTokensIndex[lastTokenId] = tokenIndex; // Update the moved token's index
+        }
+
+        // This also deletes the contents at the last position of the array
+        delete _ownedTokensIndex[tokenId];
+        delete _ownedTokens[from][lastTokenIndex];
+    }
+
+    /**
+     * @dev Private function to remove a token from this extension's token tracking data structures.
+     * This has O(1) time complexity, but alters the order of the _allTokens array.
+     * @param tokenId uint256 ID of the token to be removed from the tokens list
+     */
+    function _removeTokenFromAllTokensEnumeration(uint256 tokenId) private {
+        // To prevent a gap in the tokens array, we store the last token in the index of the token to delete, and
+        // then delete the last slot (swap and pop).
+
+        uint256 lastTokenIndex = _allTokens.length - 1;
+        uint256 tokenIndex = _allTokensIndex[tokenId];
+
+        // When the token to delete is the last token, the swap operation is unnecessary. However, since this occurs so
+        // rarely (when the last minted token is burnt) that we still do the swap here to avoid the gas cost of adding
+        // an 'if' statement (like in _removeTokenFromOwnerEnumeration)
+        uint256 lastTokenId = _allTokens[lastTokenIndex];
+
+        _allTokens[tokenIndex] = lastTokenId; // Move the last token to the slot of the to-delete token
+        _allTokensIndex[lastTokenId] = tokenIndex; // Update the moved token's index
+
+        // This also deletes the contents at the last position of the array
+        delete _allTokensIndex[tokenId];
+        _allTokens.pop();
+    }
+}

contracts/token/ERC721/README.adoc

@@ -7,7 +7,7 @@ This set of interfaces, contracts, and utilities are all related to the https://
 
 TIP: For a walk through on how to create an ERC721 token read our xref:ROOT:erc721.adoc[ERC721 guide].
 
-The EIP consists of three interfaces, found here as {IERC721}, {IERC721Metadata}, and {IERC721Enumerable}. Only the first one is required in a contract to be ERC721 compliant. However, all three are implemented in {ERC721}.
+The EIP consists of three interfaces, found here as {IERC721}, {IERC721Metadata}, and {IERC721Enumerable}. Only the first one is required in a contract to be ERC721 compliant. The core interface and the metadata extension are both implemented in {ERC721}. The enumerable extension is provided separately in {ERC721Enumerable}.
 
 Additionally, {IERC721Receiver} can be used to prevent tokens from becoming forever locked in contracts. Imagine sending an in-game item to an exchange address that can't send it back!. When using <<IERC721-safeTransferFrom,`safeTransferFrom`>>, the token contract checks to see that the receiver is an {IERC721Receiver}, which implies that it knows how to handle {ERC721} tokens. If you're writing a contract that needs to receive {ERC721} tokens, you'll want to include this interface.
 
@@ -29,6 +29,8 @@ NOTE: This core set of contracts is designed to be unopinionated, allowing devel
 
 {{ERC721}}
 
+{{ERC721Enumerable}}
+
 {{IERC721Receiver}}
 
 == Extensions

test/access/AccessControl.behavior.js

+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]);
+    });
+  });
+}
+
+module.exports = {
+  shouldBehaveLikeAccessControl,
+  shouldBehaveLikeAccessControlEnumerable,
+};

test/access/AccessControl.test.js

-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, 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',
-        );
+    this.accessControl = await AccessControlMock.new({ from: accounts[0] });
   });
 
-      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);
 });

test/access/AccessControlEnumerable.test.js

+const {
+  shouldBehaveLikeAccessControl,
+  shouldBehaveLikeAccessControlEnumerable,
+} = require('./AccessControl.behavior.js');
+
+const AccessControlMock = artifacts.require('AccessControlEnumerableMock');
+
+contract('AccessControl', function (accounts) {
+  beforeEach(async function () {
+    this.accessControl = await AccessControlMock.new({ from: accounts[0] });
+  });
+
+  shouldBehaveLikeAccessControl('AccessControl', ...accounts);
+  shouldBehaveLikeAccessControlEnumerable('AccessControl', ...accounts);
+});

test/metatx/ERC2771Context.test.js

+const ethSigUtil = require('eth-sig-util');
+const Wallet = require('ethereumjs-wallet').default;
+const { EIP712Domain } = require('../helpers/eip712');
+
+const { expectEvent } = require('@openzeppelin/test-helpers');
+const { expect } = require('chai');
+
+const ERC2771ContextMock = artifacts.require('ERC2771ContextMock');
+const MinimalForwarder = artifacts.require('MinimalForwarder');
+const ContextMockCaller = artifacts.require('ContextMockCaller');
+
+const { shouldBehaveLikeRegularContext } = require('../utils/Context.behavior');
+
+const name = 'MinimalForwarder';
+const version = '0.0.1';
+
+contract('ERC2771Context', function (accounts) {
+  beforeEach(async function () {
+    this.forwarder = await MinimalForwarder.new();
+    this.recipient = await ERC2771ContextMock.new(this.forwarder.address);
+
+    this.domain = {
+      name,
+      version,
+      chainId: await web3.eth.getChainId(),
+      verifyingContract: this.forwarder.address,
+    };
+    this.types = {
+      EIP712Domain,
+      ForwardRequest: [
+        { name: 'from', type: 'address' },
+        { name: 'to', type: 'address' },
+        { name: 'value', type: 'uint256' },
+        { name: 'gas', type: 'uint256' },
+        { name: 'nonce', type: 'uint256' },
+        { name: 'data', type: 'bytes' },
+      ],
+    };
+  });
+
+  it('recognize trusted forwarder', async function () {
+    expect(await this.recipient.isTrustedForwarder(this.forwarder.address));
+  });
+
+  context('when called directly', function () {
+    beforeEach(async function () {
+      this.context = this.recipient; // The Context behavior expects the contract in this.context
+      this.caller = await ContextMockCaller.new();
+    });
+
+    shouldBehaveLikeRegularContext(...accounts);
+  });
+
+  context('when receiving a relayed call', function () {
+    beforeEach(async function () {
+      this.wallet = Wallet.generate();
+      this.sender = web3.utils.toChecksumAddress(this.wallet.getAddressString());
+      this.data = {
+        types: this.types,
+        domain: this.domain,
+        primaryType: 'ForwardRequest',
+      };
+    });
+
+    describe('msgSender', function () {
+      it('returns the relayed transaction original sender', async function () {
+        const data = this.recipient.contract.methods.msgSender().encodeABI();
+
+        const req = {
+          from: this.sender,
+          to: this.recipient.address,
+          value: '0',
+          gas: '100000',
+          nonce: (await this.forwarder.getNonce(this.sender)).toString(),
+          data,
+        };
+
+        const sign = ethSigUtil.signTypedMessage(this.wallet.getPrivateKey(), { data: { ...this.data, message: req } });
+
+        // rejected by lint :/
+        // expect(await this.forwarder.verify(req, sign)).to.be.true;
+
+        const { tx } = await this.forwarder.execute(req, sign);
+        await expectEvent.inTransaction(tx, ERC2771ContextMock, 'Sender', { sender: this.sender });
+      });
+    });
+
+    describe('msgData', function () {
+      it('returns the relayed transaction original data', async function () {
+        const integerValue = '42';
+        const stringValue = 'OpenZeppelin';
+        const data = this.recipient.contract.methods.msgData(integerValue, stringValue).encodeABI();
+
+        const req = {
+          from: this.sender,
+          to: this.recipient.address,
+          value: '0',
+          gas: '100000',
+          nonce: (await this.forwarder.getNonce(this.sender)).toString(),
+          data,
+        };
+
+        const sign = ethSigUtil.signTypedMessage(this.wallet.getPrivateKey(), { data: { ...this.data, message: req } });
+
+        // rejected by lint :/
+        // expect(await this.forwarder.verify(req, sign)).to.be.true;
+
+        const { tx } = await this.forwarder.execute(req, sign);
+        await expectEvent.inTransaction(tx, ERC2771ContextMock, 'Data', { data, integerValue, stringValue });
+      });
+    });
+  });
+});

test/metatx/MinimalForwarder.test.js

+const ethSigUtil = require('eth-sig-util');
+const Wallet = require('ethereumjs-wallet').default;
+const { EIP712Domain } = require('../helpers/eip712');
+
+const { expectRevert, constants } = require('@openzeppelin/test-helpers');
+const { expect } = require('chai');
+
+const MinimalForwarder = artifacts.require('MinimalForwarder');
+
+const name = 'MinimalForwarder';
+const version = '0.0.1';
+
+contract('MinimalForwarder', function (accounts) {
+  beforeEach(async function () {
+    this.forwarder = await MinimalForwarder.new();
+    this.domain = {
+      name,
+      version,
+      chainId: await web3.eth.getChainId(),
+      verifyingContract: this.forwarder.address,
+    };
+    this.types = {
+      EIP712Domain,
+      ForwardRequest: [
+        { name: 'from', type: 'address' },
+        { name: 'to', type: 'address' },
+        { name: 'value', type: 'uint256' },
+        { name: 'gas', type: 'uint256' },
+        { name: 'nonce', type: 'uint256' },
+        { name: 'data', type: 'bytes' },
+      ],
+    };
+  });
+
+  context('with message', function () {
+    beforeEach(async function () {
+      this.wallet = Wallet.generate();
+      this.sender = web3.utils.toChecksumAddress(this.wallet.getAddressString());
+      this.req = {
+        from: this.sender,
+        to: constants.ZERO_ADDRESS,
+        value: '0',
+        gas: '100000',
+        nonce: Number(await this.forwarder.getNonce(this.sender)),
+        data: '0x',
+      };
+      this.sign = ethSigUtil.signTypedMessage(
+        this.wallet.getPrivateKey(),
+        {
+          data: {
+            types: this.types,
+            domain: this.domain,
+            primaryType: 'ForwardRequest',
+            message: this.req,
+          },
+        },
+      );
+    });
+
+    context('verify', function () {
+      context('valid signature', function () {
+        beforeEach(async function () {
+          expect(await this.forwarder.getNonce(this.req.from))
+            .to.be.bignumber.equal(web3.utils.toBN(this.req.nonce));
+        });
+
+        it('success', async function () {
+          expect(await this.forwarder.verify(this.req, this.sign)).to.be.equal(true);
+        });
+
+        afterEach(async function () {
+          expect(await this.forwarder.getNonce(this.req.from))
+            .to.be.bignumber.equal(web3.utils.toBN(this.req.nonce));
+        });
+      });
+
+      context('invalid signature', function () {
+        it('tampered from', async function () {
+          expect(await this.forwarder.verify({ ...this.req, from: accounts[0] }, this.sign))
+            .to.be.equal(false);
+        });
+        it('tampered to', async function () {
+          expect(await this.forwarder.verify({ ...this.req, to: accounts[0] }, this.sign))
+            .to.be.equal(false);
+        });
+        it('tampered value', async function () {
+          expect(await this.forwarder.verify({ ...this.req, value: web3.utils.toWei('1') }, this.sign))
+            .to.be.equal(false);
+        });
+        it('tampered nonce', async function () {
+          expect(await this.forwarder.verify({ ...this.req, nonce: this.req.nonce + 1 }, this.sign))
+            .to.be.equal(false);
+        });
+        it('tampered data', async function () {
+          expect(await this.forwarder.verify({ ...this.req, data: '0x1742' }, this.sign))
+            .to.be.equal(false);
+        });
+        it('tampered signature', async function () {
+          const tamperedsign = web3.utils.hexToBytes(this.sign);
+          tamperedsign[42] ^= 0xff;
+          expect(await this.forwarder.verify(this.req, web3.utils.bytesToHex(tamperedsign)))
+            .to.be.equal(false);
+        });
+      });
+    });
+
+    context('execute', function () {
+      context('valid signature', function () {
+        beforeEach(async function () {
+          expect(await this.forwarder.getNonce(this.req.from))
+            .to.be.bignumber.equal(web3.utils.toBN(this.req.nonce));
+        });
+
+        it('success', async function () {
+          await this.forwarder.execute(this.req, this.sign); // expect to not revert
+        });
+
+        afterEach(async function () {
+          expect(await this.forwarder.getNonce(this.req.from))
+            .to.be.bignumber.equal(web3.utils.toBN(this.req.nonce + 1));
+        });
+      });
+
+      context('invalid signature', function () {
+        it('tampered from', async function () {
+          await expectRevert(
+            this.forwarder.execute({ ...this.req, from: accounts[0] }, this.sign),
+            'MinimalForwarder: signature does not match request',
+          );
+        });
+        it('tampered to', async function () {
+          await expectRevert(
+            this.forwarder.execute({ ...this.req, to: accounts[0] }, this.sign),
+            'MinimalForwarder: signature does not match request',
+          );
+        });
+        it('tampered value', async function () {
+          await expectRevert(
+            this.forwarder.execute({ ...this.req, value: web3.utils.toWei('1') }, this.sign),
+            'MinimalForwarder: signature does not match request',
+          );
+        });
+        it('tampered nonce', async function () {
+          await expectRevert(
+            this.forwarder.execute({ ...this.req, nonce: this.req.nonce + 1 }, this.sign),
+            'MinimalForwarder: signature does not match request',
+          );
+        });
+        it('tampered data', async function () {
+          await expectRevert(
+            this.forwarder.execute({ ...this.req, data: '0x1742' }, this.sign),
+            'MinimalForwarder: signature does not match request',
+          );
+        });
+        it('tampered signature', async function () {
+          const tamperedsign = web3.utils.hexToBytes(this.sign);
+          tamperedsign[42] ^= 0xff;
+          await expectRevert(
+            this.forwarder.execute(this.req, web3.utils.bytesToHex(tamperedsign)),
+            'MinimalForwarder: signature does not match request',
+          );
+        });
+      });
+    });
+  });
+});

test/presets/ERC721PresetMinterPauserAutoId.test.js

@@ -27,10 +27,6 @@ contract('ERC721PresetMinterPauserAutoId', function (accounts) {
     expect(await this.token.symbol()).to.equal(symbol);
   });
 
-  it('token has correct base URI', async function () {
-    expect(await this.token.baseURI()).to.equal(baseURI);
-  });
-
   it('deployer has the default admin role', async function () {
     expect(await this.token.getRoleMemberCount(DEFAULT_ADMIN_ROLE)).to.be.bignumber.equal('1');
     expect(await this.token.getRoleMember(DEFAULT_ADMIN_ROLE, 0)).to.equal(deployer);

test/token/ERC721/ERC721Enumerable.test.js

+const {
+  shouldBehaveLikeERC721,
+  shouldBehaveLikeERC721Metadata,
+  shouldBehaveLikeERC721Enumerable,
+} = require('./ERC721.behavior');
+
+const ERC721Mock = artifacts.require('ERC721EnumerableMock');
+
+contract('ERC721Enumerable', function (accounts) {
+  const name = 'Non Fungible Token';
+  const symbol = 'NFT';
+
+  beforeEach(async function () {
+    this.token = await ERC721Mock.new(name, symbol);
+  });
+
+  shouldBehaveLikeERC721('ERC721', ...accounts);
+  shouldBehaveLikeERC721Metadata('ERC721', name, symbol, ...accounts);
+  shouldBehaveLikeERC721Enumerable('ERC721', ...accounts);
+});