Jelajahi Sumber

Remove enumerable from ERC721 and add an ERC721Enumerable extension (#2511)

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
Hadrien Croubois 4 tahun lalu
induk
melakukan
09734e8028

+ 1 - 0
CHANGELOG.md

@@ -10,6 +10,7 @@
  * `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))
 
 ## 3.4.0 (2021-02-02)
 

+ 43 - 0
contracts/mocks/ERC721EnumerableMock.sol

@@ -0,0 +1,43 @@
+// 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);
+    }
+}

+ 12 - 6
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 {

+ 18 - 4
contracts/presets/ERC721PresetMinterPauserAutoId.sol

@@ -6,6 +6,7 @@ import "../access/AccessControl.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, AccessControl, 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);
+    }
 }

+ 41 - 107
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()));
+        string memory baseURI = _baseURI();
+        return bytes(baseURI).length > 0
+            ? string(abi.encodePacked(baseURI, 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}.
+     * @dev Base URI for computing {tokenURI}. Empty by default, can be overriden
+     * in child contracts.
      */
-    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();
-    }
-
-    /**
-     * @dev See {IERC721Enumerable-tokenByIndex}.
-     */
-    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.

+ 159 - 0
contracts/token/ERC721/ERC721Enumerable.sol

@@ -0,0 +1,159 @@
+// 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();
+    }
+}

+ 3 - 1
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

+ 0 - 4
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);

+ 945 - 0
test/token/ERC721/ERC721.behavior.js

@@ -0,0 +1,945 @@
+const { BN, constants, expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
+const { expect } = require('chai');
+const { ZERO_ADDRESS } = constants;
+
+const { shouldSupportInterfaces } = require('../../introspection/SupportsInterface.behavior');
+
+const ERC721ReceiverMock = artifacts.require('ERC721ReceiverMock');
+
+const Error = [ 'None', 'RevertWithMessage', 'RevertWithoutMessage', 'Panic' ]
+  .reduce((acc, entry, idx) => Object.assign({ [entry]: idx }, acc), {});
+
+const firstTokenId = new BN('5042');
+const secondTokenId = new BN('79217');
+const nonExistentTokenId = new BN('13');
+const baseURI = 'https://api.com/v1/';
+
+const RECEIVER_MAGIC_VALUE = '0x150b7a02';
+
+function shouldBehaveLikeERC721 (errorPrefix, owner, newOwner, approved, anotherApproved, operator, other) {
+  shouldSupportInterfaces([
+    'ERC165',
+    'ERC721',
+  ]);
+
+  context('with minted tokens', function () {
+    beforeEach(async function () {
+      await this.token.mint(owner, firstTokenId);
+      await this.token.mint(owner, secondTokenId);
+      this.toWhom = other; // default to other for toWhom in context-dependent tests
+    });
+
+    describe('balanceOf', function () {
+      context('when the given address owns some tokens', function () {
+        it('returns the amount of tokens owned by the given address', async function () {
+          expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('2');
+        });
+      });
+
+      context('when the given address does not own any tokens', function () {
+        it('returns 0', async function () {
+          expect(await this.token.balanceOf(other)).to.be.bignumber.equal('0');
+        });
+      });
+
+      context('when querying the zero address', function () {
+        it('throws', async function () {
+          await expectRevert(
+            this.token.balanceOf(ZERO_ADDRESS), 'ERC721: balance query for the zero address',
+          );
+        });
+      });
+    });
+
+    describe('ownerOf', function () {
+      context('when the given token ID was tracked by this token', function () {
+        const tokenId = firstTokenId;
+
+        it('returns the owner of the given token ID', async function () {
+          expect(await this.token.ownerOf(tokenId)).to.be.equal(owner);
+        });
+      });
+
+      context('when the given token ID was not tracked by this token', function () {
+        const tokenId = nonExistentTokenId;
+
+        it('reverts', async function () {
+          await expectRevert(
+            this.token.ownerOf(tokenId), 'ERC721: owner query for nonexistent token',
+          );
+        });
+      });
+    });
+
+    describe('transfers', function () {
+      const tokenId = firstTokenId;
+      const data = '0x42';
+
+      let logs = null;
+
+      beforeEach(async function () {
+        await this.token.approve(approved, tokenId, { from: owner });
+        await this.token.setApprovalForAll(operator, true, { from: owner });
+      });
+
+      const transferWasSuccessful = function ({ owner, tokenId, approved }) {
+        it('transfers the ownership of the given token ID to the given address', async function () {
+          expect(await this.token.ownerOf(tokenId)).to.be.equal(this.toWhom);
+        });
+
+        it('emits a Transfer event', async function () {
+          expectEvent.inLogs(logs, 'Transfer', { from: owner, to: this.toWhom, tokenId: tokenId });
+        });
+
+        it('clears the approval for the token ID', async function () {
+          expect(await this.token.getApproved(tokenId)).to.be.equal(ZERO_ADDRESS);
+        });
+
+        it('emits an Approval event', async function () {
+          expectEvent.inLogs(logs, 'Approval', { owner, approved: ZERO_ADDRESS, tokenId: tokenId });
+        });
+
+        it('adjusts owners balances', async function () {
+          expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('1');
+        });
+
+        it('adjusts owners tokens by index', async function () {
+          if (!this.token.tokenOfOwnerByIndex) return;
+
+          expect(await this.token.tokenOfOwnerByIndex(this.toWhom, 0)).to.be.bignumber.equal(tokenId);
+
+          expect(await this.token.tokenOfOwnerByIndex(owner, 0)).to.be.bignumber.not.equal(tokenId);
+        });
+      };
+
+      const shouldTransferTokensByUsers = function (transferFunction) {
+        context('when called by the owner', function () {
+          beforeEach(async function () {
+            ({ logs } = await transferFunction.call(this, owner, this.toWhom, tokenId, { from: owner }));
+          });
+          transferWasSuccessful({ owner, tokenId, approved });
+        });
+
+        context('when called by the approved individual', function () {
+          beforeEach(async function () {
+            ({ logs } = await transferFunction.call(this, owner, this.toWhom, tokenId, { from: approved }));
+          });
+          transferWasSuccessful({ owner, tokenId, approved });
+        });
+
+        context('when called by the operator', function () {
+          beforeEach(async function () {
+            ({ logs } = await transferFunction.call(this, owner, this.toWhom, tokenId, { from: operator }));
+          });
+          transferWasSuccessful({ owner, tokenId, approved });
+        });
+
+        context('when called by the owner without an approved user', function () {
+          beforeEach(async function () {
+            await this.token.approve(ZERO_ADDRESS, tokenId, { from: owner });
+            ({ logs } = await transferFunction.call(this, owner, this.toWhom, tokenId, { from: operator }));
+          });
+          transferWasSuccessful({ owner, tokenId, approved: null });
+        });
+
+        context('when sent to the owner', function () {
+          beforeEach(async function () {
+            ({ logs } = await transferFunction.call(this, owner, owner, tokenId, { from: owner }));
+          });
+
+          it('keeps ownership of the token', async function () {
+            expect(await this.token.ownerOf(tokenId)).to.be.equal(owner);
+          });
+
+          it('clears the approval for the token ID', async function () {
+            expect(await this.token.getApproved(tokenId)).to.be.equal(ZERO_ADDRESS);
+          });
+
+          it('emits only a transfer event', async function () {
+            expectEvent.inLogs(logs, 'Transfer', {
+              from: owner,
+              to: owner,
+              tokenId: tokenId,
+            });
+          });
+
+          it('keeps the owner balance', async function () {
+            expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('2');
+          });
+
+          it('keeps same tokens by index', async function () {
+            if (!this.token.tokenOfOwnerByIndex) return;
+            const tokensListed = await Promise.all(
+              [0, 1].map(i => this.token.tokenOfOwnerByIndex(owner, i)),
+            );
+            expect(tokensListed.map(t => t.toNumber())).to.have.members(
+              [firstTokenId.toNumber(), secondTokenId.toNumber()],
+            );
+          });
+        });
+
+        context('when the address of the previous owner is incorrect', function () {
+          it('reverts', async function () {
+            await expectRevert(
+              transferFunction.call(this, other, other, tokenId, { from: owner }),
+              'ERC721: transfer of token that is not own',
+            );
+          });
+        });
+
+        context('when the sender is not authorized for the token id', function () {
+          it('reverts', async function () {
+            await expectRevert(
+              transferFunction.call(this, owner, other, tokenId, { from: other }),
+              'ERC721: transfer caller is not owner nor approved',
+            );
+          });
+        });
+
+        context('when the given token ID does not exist', function () {
+          it('reverts', async function () {
+            await expectRevert(
+              transferFunction.call(this, owner, other, nonExistentTokenId, { from: owner }),
+              'ERC721: operator query for nonexistent token',
+            );
+          });
+        });
+
+        context('when the address to transfer the token to is the zero address', function () {
+          it('reverts', async function () {
+            await expectRevert(
+              transferFunction.call(this, owner, ZERO_ADDRESS, tokenId, { from: owner }),
+              'ERC721: transfer to the zero address',
+            );
+          });
+        });
+      };
+
+      describe('via transferFrom', function () {
+        shouldTransferTokensByUsers(function (from, to, tokenId, opts) {
+          return this.token.transferFrom(from, to, tokenId, opts);
+        });
+      });
+
+      describe('via safeTransferFrom', function () {
+        const safeTransferFromWithData = function (from, to, tokenId, opts) {
+          return this.token.methods['safeTransferFrom(address,address,uint256,bytes)'](from, to, tokenId, data, opts);
+        };
+
+        const safeTransferFromWithoutData = function (from, to, tokenId, opts) {
+          return this.token.methods['safeTransferFrom(address,address,uint256)'](from, to, tokenId, opts);
+        };
+
+        const shouldTransferSafely = function (transferFun, data) {
+          describe('to a user account', function () {
+            shouldTransferTokensByUsers(transferFun);
+          });
+
+          describe('to a valid receiver contract', function () {
+            beforeEach(async function () {
+              this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.None);
+              this.toWhom = this.receiver.address;
+            });
+
+            shouldTransferTokensByUsers(transferFun);
+
+            it('calls onERC721Received', async function () {
+              const receipt = await transferFun.call(this, owner, this.receiver.address, tokenId, { from: owner });
+
+              await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', {
+                operator: owner,
+                from: owner,
+                tokenId: tokenId,
+                data: data,
+              });
+            });
+
+            it('calls onERC721Received from approved', async function () {
+              const receipt = await transferFun.call(this, owner, this.receiver.address, tokenId, { from: approved });
+
+              await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', {
+                operator: approved,
+                from: owner,
+                tokenId: tokenId,
+                data: data,
+              });
+            });
+
+            describe('with an invalid token id', function () {
+              it('reverts', async function () {
+                await expectRevert(
+                  transferFun.call(
+                    this,
+                    owner,
+                    this.receiver.address,
+                    nonExistentTokenId,
+                    { from: owner },
+                  ),
+                  'ERC721: operator query for nonexistent token',
+                );
+              });
+            });
+          });
+        };
+
+        describe('with data', function () {
+          shouldTransferSafely(safeTransferFromWithData, data);
+        });
+
+        describe('without data', function () {
+          shouldTransferSafely(safeTransferFromWithoutData, null);
+        });
+
+        describe('to a receiver contract returning unexpected value', function () {
+          it('reverts', async function () {
+            const invalidReceiver = await ERC721ReceiverMock.new('0x42', Error.None);
+            await expectRevert(
+              this.token.safeTransferFrom(owner, invalidReceiver.address, tokenId, { from: owner }),
+              'ERC721: transfer to non ERC721Receiver implementer',
+            );
+          });
+        });
+
+        describe('to a receiver contract that reverts with message', function () {
+          it('reverts', async function () {
+            const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.RevertWithMessage);
+            await expectRevert(
+              this.token.safeTransferFrom(owner, revertingReceiver.address, tokenId, { from: owner }),
+              'ERC721ReceiverMock: reverting',
+            );
+          });
+        });
+
+        describe('to a receiver contract that reverts without message', function () {
+          it('reverts', async function () {
+            const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.RevertWithoutMessage);
+            await expectRevert(
+              this.token.safeTransferFrom(owner, revertingReceiver.address, tokenId, { from: owner }),
+              'ERC721: transfer to non ERC721Receiver implementer',
+            );
+          });
+        });
+
+        describe('to a receiver contract that panics', function () {
+          it('reverts', async function () {
+            const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.Panic);
+            await expectRevert.unspecified(
+              this.token.safeTransferFrom(owner, revertingReceiver.address, tokenId, { from: owner }),
+            );
+          });
+        });
+
+        describe('to a contract that does not implement the required function', function () {
+          it('reverts', async function () {
+            const nonReceiver = this.token;
+            await expectRevert(
+              this.token.safeTransferFrom(owner, nonReceiver.address, tokenId, { from: owner }),
+              'ERC721: transfer to non ERC721Receiver implementer',
+            );
+          });
+        });
+      });
+    });
+
+    describe('safe mint', function () {
+      const fourthTokenId = new BN(4);
+      const tokenId = fourthTokenId;
+      const data = '0x42';
+
+      describe('via safeMint', function () { // regular minting is tested in ERC721Mintable.test.js and others
+        it('calls onERC721Received — with data', async function () {
+          this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.None);
+          const receipt = await this.token.safeMint(this.receiver.address, tokenId, data);
+
+          await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', {
+            from: ZERO_ADDRESS,
+            tokenId: tokenId,
+            data: data,
+          });
+        });
+
+        it('calls onERC721Received — without data', async function () {
+          this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.None);
+          const receipt = await this.token.safeMint(this.receiver.address, tokenId);
+
+          await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', {
+            from: ZERO_ADDRESS,
+            tokenId: tokenId,
+          });
+        });
+
+        context('to a receiver contract returning unexpected value', function () {
+          it('reverts', async function () {
+            const invalidReceiver = await ERC721ReceiverMock.new('0x42', Error.None);
+            await expectRevert(
+              this.token.safeMint(invalidReceiver.address, tokenId),
+              'ERC721: transfer to non ERC721Receiver implementer',
+            );
+          });
+        });
+
+        context('to a receiver contract that reverts with message', function () {
+          it('reverts', async function () {
+            const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.RevertWithMessage);
+            await expectRevert(
+              this.token.safeMint(revertingReceiver.address, tokenId),
+              'ERC721ReceiverMock: reverting',
+            );
+          });
+        });
+
+        context('to a receiver contract that reverts without message', function () {
+          it('reverts', async function () {
+            const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.RevertWithoutMessage);
+            await expectRevert(
+              this.token.safeMint(revertingReceiver.address, tokenId),
+              'ERC721: transfer to non ERC721Receiver implementer',
+            );
+          });
+        });
+
+        context('to a receiver contract that panics', function () {
+          it('reverts', async function () {
+            const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.Panic);
+            await expectRevert.unspecified(
+              this.token.safeMint(revertingReceiver.address, tokenId),
+            );
+          });
+        });
+
+        context('to a contract that does not implement the required function', function () {
+          it('reverts', async function () {
+            const nonReceiver = this.token;
+            await expectRevert(
+              this.token.safeMint(nonReceiver.address, tokenId),
+              'ERC721: transfer to non ERC721Receiver implementer',
+            );
+          });
+        });
+      });
+    });
+
+    describe('approve', function () {
+      const tokenId = firstTokenId;
+
+      let logs = null;
+
+      const itClearsApproval = function () {
+        it('clears approval for the token', async function () {
+          expect(await this.token.getApproved(tokenId)).to.be.equal(ZERO_ADDRESS);
+        });
+      };
+
+      const itApproves = function (address) {
+        it('sets the approval for the target address', async function () {
+          expect(await this.token.getApproved(tokenId)).to.be.equal(address);
+        });
+      };
+
+      const itEmitsApprovalEvent = function (address) {
+        it('emits an approval event', async function () {
+          expectEvent.inLogs(logs, 'Approval', {
+            owner: owner,
+            approved: address,
+            tokenId: tokenId,
+          });
+        });
+      };
+
+      context('when clearing approval', function () {
+        context('when there was no prior approval', function () {
+          beforeEach(async function () {
+            ({ logs } = await this.token.approve(ZERO_ADDRESS, tokenId, { from: owner }));
+          });
+
+          itClearsApproval();
+          itEmitsApprovalEvent(ZERO_ADDRESS);
+        });
+
+        context('when there was a prior approval', function () {
+          beforeEach(async function () {
+            await this.token.approve(approved, tokenId, { from: owner });
+            ({ logs } = await this.token.approve(ZERO_ADDRESS, tokenId, { from: owner }));
+          });
+
+          itClearsApproval();
+          itEmitsApprovalEvent(ZERO_ADDRESS);
+        });
+      });
+
+      context('when approving a non-zero address', function () {
+        context('when there was no prior approval', function () {
+          beforeEach(async function () {
+            ({ logs } = await this.token.approve(approved, tokenId, { from: owner }));
+          });
+
+          itApproves(approved);
+          itEmitsApprovalEvent(approved);
+        });
+
+        context('when there was a prior approval to the same address', function () {
+          beforeEach(async function () {
+            await this.token.approve(approved, tokenId, { from: owner });
+            ({ logs } = await this.token.approve(approved, tokenId, { from: owner }));
+          });
+
+          itApproves(approved);
+          itEmitsApprovalEvent(approved);
+        });
+
+        context('when there was a prior approval to a different address', function () {
+          beforeEach(async function () {
+            await this.token.approve(anotherApproved, tokenId, { from: owner });
+            ({ logs } = await this.token.approve(anotherApproved, tokenId, { from: owner }));
+          });
+
+          itApproves(anotherApproved);
+          itEmitsApprovalEvent(anotherApproved);
+        });
+      });
+
+      context('when the address that receives the approval is the owner', function () {
+        it('reverts', async function () {
+          await expectRevert(
+            this.token.approve(owner, tokenId, { from: owner }), 'ERC721: approval to current owner',
+          );
+        });
+      });
+
+      context('when the sender does not own the given token ID', function () {
+        it('reverts', async function () {
+          await expectRevert(this.token.approve(approved, tokenId, { from: other }),
+            'ERC721: approve caller is not owner nor approved');
+        });
+      });
+
+      context('when the sender is approved for the given token ID', function () {
+        it('reverts', async function () {
+          await this.token.approve(approved, tokenId, { from: owner });
+          await expectRevert(this.token.approve(anotherApproved, tokenId, { from: approved }),
+            'ERC721: approve caller is not owner nor approved for all');
+        });
+      });
+
+      context('when the sender is an operator', function () {
+        beforeEach(async function () {
+          await this.token.setApprovalForAll(operator, true, { from: owner });
+          ({ logs } = await this.token.approve(approved, tokenId, { from: operator }));
+        });
+
+        itApproves(approved);
+        itEmitsApprovalEvent(approved);
+      });
+
+      context('when the given token ID does not exist', function () {
+        it('reverts', async function () {
+          await expectRevert(this.token.approve(approved, nonExistentTokenId, { from: operator }),
+            'ERC721: owner query for nonexistent token');
+        });
+      });
+    });
+
+    describe('setApprovalForAll', function () {
+      context('when the operator willing to approve is not the owner', function () {
+        context('when there is no operator approval set by the sender', function () {
+          it('approves the operator', async function () {
+            await this.token.setApprovalForAll(operator, true, { from: owner });
+
+            expect(await this.token.isApprovedForAll(owner, operator)).to.equal(true);
+          });
+
+          it('emits an approval event', async function () {
+            const { logs } = await this.token.setApprovalForAll(operator, true, { from: owner });
+
+            expectEvent.inLogs(logs, 'ApprovalForAll', {
+              owner: owner,
+              operator: operator,
+              approved: true,
+            });
+          });
+        });
+
+        context('when the operator was set as not approved', function () {
+          beforeEach(async function () {
+            await this.token.setApprovalForAll(operator, false, { from: owner });
+          });
+
+          it('approves the operator', async function () {
+            await this.token.setApprovalForAll(operator, true, { from: owner });
+
+            expect(await this.token.isApprovedForAll(owner, operator)).to.equal(true);
+          });
+
+          it('emits an approval event', async function () {
+            const { logs } = await this.token.setApprovalForAll(operator, true, { from: owner });
+
+            expectEvent.inLogs(logs, 'ApprovalForAll', {
+              owner: owner,
+              operator: operator,
+              approved: true,
+            });
+          });
+
+          it('can unset the operator approval', async function () {
+            await this.token.setApprovalForAll(operator, false, { from: owner });
+
+            expect(await this.token.isApprovedForAll(owner, operator)).to.equal(false);
+          });
+        });
+
+        context('when the operator was already approved', function () {
+          beforeEach(async function () {
+            await this.token.setApprovalForAll(operator, true, { from: owner });
+          });
+
+          it('keeps the approval to the given address', async function () {
+            await this.token.setApprovalForAll(operator, true, { from: owner });
+
+            expect(await this.token.isApprovedForAll(owner, operator)).to.equal(true);
+          });
+
+          it('emits an approval event', async function () {
+            const { logs } = await this.token.setApprovalForAll(operator, true, { from: owner });
+
+            expectEvent.inLogs(logs, 'ApprovalForAll', {
+              owner: owner,
+              operator: operator,
+              approved: true,
+            });
+          });
+        });
+      });
+
+      context('when the operator is the owner', function () {
+        it('reverts', async function () {
+          await expectRevert(this.token.setApprovalForAll(owner, true, { from: owner }),
+            'ERC721: approve to caller');
+        });
+      });
+    });
+
+    describe('getApproved', async function () {
+      context('when token is not minted', async function () {
+        it('reverts', async function () {
+          await expectRevert(
+            this.token.getApproved(nonExistentTokenId),
+            'ERC721: approved query for nonexistent token',
+          );
+        });
+      });
+
+      context('when token has been minted ', async function () {
+        it('should return the zero address', async function () {
+          expect(await this.token.getApproved(firstTokenId)).to.be.equal(
+            ZERO_ADDRESS,
+          );
+        });
+
+        context('when account has been approved', async function () {
+          beforeEach(async function () {
+            await this.token.approve(approved, firstTokenId, { from: owner });
+          });
+
+          it('returns approved account', async function () {
+            expect(await this.token.getApproved(firstTokenId)).to.be.equal(approved);
+          });
+        });
+      });
+    });
+  });
+
+  describe('_mint(address, uint256)', function () {
+    it('reverts with a null destination address', async function () {
+      await expectRevert(
+        this.token.mint(ZERO_ADDRESS, firstTokenId), 'ERC721: mint to the zero address',
+      );
+    });
+
+    context('with minted token', async function () {
+      beforeEach(async function () {
+        ({ logs: this.logs } = await this.token.mint(owner, firstTokenId));
+      });
+
+      it('emits a Transfer event', function () {
+        expectEvent.inLogs(this.logs, 'Transfer', { from: ZERO_ADDRESS, to: owner, tokenId: firstTokenId });
+      });
+
+      it('creates the token', async function () {
+        expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('1');
+        expect(await this.token.ownerOf(firstTokenId)).to.equal(owner);
+      });
+
+      it('reverts when adding a token id that already exists', async function () {
+        await expectRevert(this.token.mint(owner, firstTokenId), 'ERC721: token already minted');
+      });
+    });
+  });
+
+  describe('_burn', function () {
+    it('reverts when burning a non-existent token id', async function () {
+      await expectRevert(
+        this.token.burn(firstTokenId), 'ERC721: owner query for nonexistent token',
+      );
+    });
+
+    context('with minted tokens', function () {
+      beforeEach(async function () {
+        await this.token.mint(owner, firstTokenId);
+        await this.token.mint(owner, secondTokenId);
+      });
+
+      context('with burnt token', function () {
+        beforeEach(async function () {
+          ({ logs: this.logs } = await this.token.burn(firstTokenId));
+        });
+
+        it('emits a Transfer event', function () {
+          expectEvent.inLogs(this.logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, tokenId: firstTokenId });
+        });
+
+        it('emits an Approval event', function () {
+          expectEvent.inLogs(this.logs, 'Approval', { owner, approved: ZERO_ADDRESS, tokenId: firstTokenId });
+        });
+
+        it('deletes the token', async function () {
+          expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('1');
+          await expectRevert(
+            this.token.ownerOf(firstTokenId), 'ERC721: owner query for nonexistent token',
+          );
+        });
+
+        it('reverts when burning a token id that has been deleted', async function () {
+          await expectRevert(
+            this.token.burn(firstTokenId), 'ERC721: owner query for nonexistent token',
+          );
+        });
+      });
+    });
+  });
+}
+
+function shouldBehaveLikeERC721Enumerable (errorPrefix, owner, newOwner, approved, anotherApproved, operator, other) {
+  shouldSupportInterfaces([
+    'ERC721Enumerable',
+  ]);
+
+  context('with minted tokens', function () {
+    beforeEach(async function () {
+      await this.token.mint(owner, firstTokenId);
+      await this.token.mint(owner, secondTokenId);
+      this.toWhom = other; // default to other for toWhom in context-dependent tests
+    });
+
+    describe('totalSupply', function () {
+      it('returns total token supply', async function () {
+        expect(await this.token.totalSupply()).to.be.bignumber.equal('2');
+      });
+    });
+
+    describe('tokenOfOwnerByIndex', function () {
+      describe('when the given index is lower than the amount of tokens owned by the given address', function () {
+        it('returns the token ID placed at the given index', async function () {
+          expect(await this.token.tokenOfOwnerByIndex(owner, 0)).to.be.bignumber.equal(firstTokenId);
+        });
+      });
+
+      describe('when the index is greater than or equal to the total tokens owned by the given address', function () {
+        it('reverts', async function () {
+          await expectRevert(
+            this.token.tokenOfOwnerByIndex(owner, 2), 'ERC721Enumerable: owner index out of bounds',
+          );
+        });
+      });
+
+      describe('when the given address does not own any token', function () {
+        it('reverts', async function () {
+          await expectRevert(
+            this.token.tokenOfOwnerByIndex(other, 0), 'ERC721Enumerable: owner index out of bounds',
+          );
+        });
+      });
+
+      describe('after transferring all tokens to another user', function () {
+        beforeEach(async function () {
+          await this.token.transferFrom(owner, other, firstTokenId, { from: owner });
+          await this.token.transferFrom(owner, other, secondTokenId, { from: owner });
+        });
+
+        it('returns correct token IDs for target', async function () {
+          expect(await this.token.balanceOf(other)).to.be.bignumber.equal('2');
+          const tokensListed = await Promise.all(
+            [0, 1].map(i => this.token.tokenOfOwnerByIndex(other, i)),
+          );
+          expect(tokensListed.map(t => t.toNumber())).to.have.members([firstTokenId.toNumber(),
+            secondTokenId.toNumber()]);
+        });
+
+        it('returns empty collection for original owner', async function () {
+          expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('0');
+          await expectRevert(
+            this.token.tokenOfOwnerByIndex(owner, 0), 'ERC721Enumerable: owner index out of bounds',
+          );
+        });
+      });
+    });
+
+    describe('tokenByIndex', function () {
+      it('returns all tokens', async function () {
+        const tokensListed = await Promise.all(
+          [0, 1].map(i => this.token.tokenByIndex(i)),
+        );
+        expect(tokensListed.map(t => t.toNumber())).to.have.members([firstTokenId.toNumber(),
+          secondTokenId.toNumber()]);
+      });
+
+      it('reverts if index is greater than supply', async function () {
+        await expectRevert(
+          this.token.tokenByIndex(2), 'ERC721Enumerable: global index out of bounds',
+        );
+      });
+
+      [firstTokenId, secondTokenId].forEach(function (tokenId) {
+        it(`returns all tokens after burning token ${tokenId} and minting new tokens`, async function () {
+          const newTokenId = new BN(300);
+          const anotherNewTokenId = new BN(400);
+
+          await this.token.burn(tokenId);
+          await this.token.mint(newOwner, newTokenId);
+          await this.token.mint(newOwner, anotherNewTokenId);
+
+          expect(await this.token.totalSupply()).to.be.bignumber.equal('3');
+
+          const tokensListed = await Promise.all(
+            [0, 1, 2].map(i => this.token.tokenByIndex(i)),
+          );
+          const expectedTokens = [firstTokenId, secondTokenId, newTokenId, anotherNewTokenId].filter(
+            x => (x !== tokenId),
+          );
+          expect(tokensListed.map(t => t.toNumber())).to.have.members(expectedTokens.map(t => t.toNumber()));
+        });
+      });
+    });
+  });
+
+  describe('_mint(address, uint256)', function () {
+    it('reverts with a null destination address', async function () {
+      await expectRevert(
+        this.token.mint(ZERO_ADDRESS, firstTokenId), 'ERC721: mint to the zero address',
+      );
+    });
+
+    context('with minted token', async function () {
+      beforeEach(async function () {
+        ({ logs: this.logs } = await this.token.mint(owner, firstTokenId));
+      });
+
+      it('adjusts owner tokens by index', async function () {
+        expect(await this.token.tokenOfOwnerByIndex(owner, 0)).to.be.bignumber.equal(firstTokenId);
+      });
+
+      it('adjusts all tokens list', async function () {
+        expect(await this.token.tokenByIndex(0)).to.be.bignumber.equal(firstTokenId);
+      });
+    });
+  });
+
+  describe('_burn', function () {
+    it('reverts when burning a non-existent token id', async function () {
+      await expectRevert(
+        this.token.burn(firstTokenId), 'ERC721: owner query for nonexistent token',
+      );
+    });
+
+    context('with minted tokens', function () {
+      beforeEach(async function () {
+        await this.token.mint(owner, firstTokenId);
+        await this.token.mint(owner, secondTokenId);
+      });
+
+      context('with burnt token', function () {
+        beforeEach(async function () {
+          ({ logs: this.logs } = await this.token.burn(firstTokenId));
+        });
+
+        it('removes that token from the token list of the owner', async function () {
+          expect(await this.token.tokenOfOwnerByIndex(owner, 0)).to.be.bignumber.equal(secondTokenId);
+        });
+
+        it('adjusts all tokens list', async function () {
+          expect(await this.token.tokenByIndex(0)).to.be.bignumber.equal(secondTokenId);
+        });
+
+        it('burns all tokens', async function () {
+          await this.token.burn(secondTokenId, { from: owner });
+          expect(await this.token.totalSupply()).to.be.bignumber.equal('0');
+          await expectRevert(
+            this.token.tokenByIndex(0), 'ERC721Enumerable: global index out of bounds',
+          );
+        });
+      });
+    });
+  });
+}
+
+function shouldBehaveLikeERC721Metadata (errorPrefix, name, symbol, owner) {
+  shouldSupportInterfaces([
+    'ERC721Metadata',
+  ]);
+
+  describe('metadata', function () {
+    it('has a name', async function () {
+      expect(await this.token.name()).to.be.equal(name);
+    });
+
+    it('has a symbol', async function () {
+      expect(await this.token.symbol()).to.be.equal(symbol);
+    });
+
+    describe('token URI', function () {
+      beforeEach(async function () {
+        await this.token.mint(owner, firstTokenId);
+      });
+
+      it('return empty string by default', async function () {
+        expect(await this.token.tokenURI(firstTokenId)).to.be.equal('');
+      });
+
+      it('reverts when queried for non existent token id', async function () {
+        await expectRevert(
+          this.token.tokenURI(nonExistentTokenId), 'ERC721Metadata: URI query for nonexistent token',
+        );
+      });
+
+      describe('base URI', function () {
+        beforeEach(function () {
+          if (this.token.setBaseURI === undefined) {
+            this.skip();
+          }
+        });
+
+        it('base URI can be set', async function () {
+          await this.token.setBaseURI(baseURI);
+          expect(await this.token.baseURI()).to.equal(baseURI);
+        });
+
+        it('base URI is added as a prefix to the token URI', async function () {
+          await this.token.setBaseURI(baseURI);
+          expect(await this.token.tokenURI(firstTokenId)).to.be.equal(baseURI + firstTokenId.toString());
+        });
+
+        it('token URI can be changed by changing the base URI', async function () {
+          await this.token.setBaseURI(baseURI);
+          const newBaseURI = 'https://api.com/v2/';
+          await this.token.setBaseURI(newBaseURI);
+          expect(await this.token.tokenURI(firstTokenId)).to.be.equal(newBaseURI + firstTokenId.toString());
+        });
+      });
+    });
+  });
+}
+
+module.exports = {
+  shouldBehaveLikeERC721,
+  shouldBehaveLikeERC721Enumerable,
+  shouldBehaveLikeERC721Metadata,
+};

+ 6 - 911
test/token/ERC721/ERC721.test.js

@@ -1,923 +1,18 @@
-const { BN, constants, expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
-const { ZERO_ADDRESS } = constants;
-
-const { expect } = require('chai');
-
-const { shouldSupportInterfaces } = require('../../introspection/SupportsInterface.behavior');
+const {
+  shouldBehaveLikeERC721,
+  shouldBehaveLikeERC721Metadata,
+} = require('./ERC721.behavior');
 
 const ERC721Mock = artifacts.require('ERC721Mock');
-const ERC721ReceiverMock = artifacts.require('ERC721ReceiverMock');
-
-const Error = [ 'None', 'RevertWithMessage', 'RevertWithoutMessage', 'Panic' ]
-  .reduce((acc, entry, idx) => Object.assign({ [entry]: idx }, acc), {});
 
 contract('ERC721', function (accounts) {
-  const [owner, newOwner, approved, anotherApproved, operator, other] = accounts;
-
   const name = 'Non Fungible Token';
   const symbol = 'NFT';
 
-  const firstTokenId = new BN('5042');
-  const secondTokenId = new BN('79217');
-  const nonExistentTokenId = new BN('13');
-
-  const RECEIVER_MAGIC_VALUE = '0x150b7a02';
-
   beforeEach(async function () {
     this.token = await ERC721Mock.new(name, symbol);
   });
 
-  shouldSupportInterfaces([
-    'ERC165',
-    'ERC721',
-    'ERC721Enumerable',
-    'ERC721Metadata',
-  ]);
-
-  describe('metadata', function () {
-    it('has a name', async function () {
-      expect(await this.token.name()).to.be.equal(name);
-    });
-
-    it('has a symbol', async function () {
-      expect(await this.token.symbol()).to.be.equal(symbol);
-    });
-
-    describe('token URI', function () {
-      beforeEach(async function () {
-        await this.token.mint(owner, firstTokenId);
-      });
-
-      const baseURI = 'https://api.com/v1/';
-      const sampleUri = 'mock://mytoken';
-
-      it('it is empty by default', async function () {
-        expect(await this.token.tokenURI(firstTokenId)).to.be.equal('');
-      });
-
-      it('reverts when queried for non existent token id', async function () {
-        await expectRevert(
-          this.token.tokenURI(nonExistentTokenId), 'ERC721Metadata: URI query for nonexistent token',
-        );
-      });
-
-      it('can be set for a token id', async function () {
-        await this.token.setTokenURI(firstTokenId, sampleUri);
-        expect(await this.token.tokenURI(firstTokenId)).to.be.equal(sampleUri);
-      });
-
-      it('reverts when setting for non existent token id', async function () {
-        await expectRevert(
-          this.token.setTokenURI(nonExistentTokenId, sampleUri), 'ERC721Metadata: URI set of nonexistent token',
-        );
-      });
-
-      it('base URI can be set', async function () {
-        await this.token.setBaseURI(baseURI);
-        expect(await this.token.baseURI()).to.equal(baseURI);
-      });
-
-      it('base URI is added as a prefix to the token URI', async function () {
-        await this.token.setBaseURI(baseURI);
-        await this.token.setTokenURI(firstTokenId, sampleUri);
-
-        expect(await this.token.tokenURI(firstTokenId)).to.be.equal(baseURI + sampleUri);
-      });
-
-      it('token URI can be changed by changing the base URI', async function () {
-        await this.token.setBaseURI(baseURI);
-        await this.token.setTokenURI(firstTokenId, sampleUri);
-
-        const newBaseURI = 'https://api.com/v2/';
-        await this.token.setBaseURI(newBaseURI);
-        expect(await this.token.tokenURI(firstTokenId)).to.be.equal(newBaseURI + sampleUri);
-      });
-
-      it('tokenId is appended to base URI for tokens with no URI', async function () {
-        await this.token.setBaseURI(baseURI);
-
-        expect(await this.token.tokenURI(firstTokenId)).to.be.equal(baseURI + firstTokenId);
-      });
-
-      it('tokens with URI can be burnt ', async function () {
-        await this.token.setTokenURI(firstTokenId, sampleUri);
-
-        await this.token.burn(firstTokenId, { from: owner });
-
-        expect(await this.token.exists(firstTokenId)).to.equal(false);
-        await expectRevert(
-          this.token.tokenURI(firstTokenId), 'ERC721Metadata: URI query for nonexistent token',
-        );
-      });
-    });
-  });
-
-  context('with minted tokens', function () {
-    beforeEach(async function () {
-      await this.token.mint(owner, firstTokenId);
-      await this.token.mint(owner, secondTokenId);
-      this.toWhom = other; // default to other for toWhom in context-dependent tests
-    });
-
-    describe('balanceOf', function () {
-      context('when the given address owns some tokens', function () {
-        it('returns the amount of tokens owned by the given address', async function () {
-          expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('2');
-        });
-      });
-
-      context('when the given address does not own any tokens', function () {
-        it('returns 0', async function () {
-          expect(await this.token.balanceOf(other)).to.be.bignumber.equal('0');
-        });
-      });
-
-      context('when querying the zero address', function () {
-        it('throws', async function () {
-          await expectRevert(
-            this.token.balanceOf(ZERO_ADDRESS), 'ERC721: balance query for the zero address',
-          );
-        });
-      });
-    });
-
-    describe('ownerOf', function () {
-      context('when the given token ID was tracked by this token', function () {
-        const tokenId = firstTokenId;
-
-        it('returns the owner of the given token ID', async function () {
-          expect(await this.token.ownerOf(tokenId)).to.be.equal(owner);
-        });
-      });
-
-      context('when the given token ID was not tracked by this token', function () {
-        const tokenId = nonExistentTokenId;
-
-        it('reverts', async function () {
-          await expectRevert(
-            this.token.ownerOf(tokenId), 'ERC721: owner query for nonexistent token',
-          );
-        });
-      });
-    });
-
-    describe('transfers', function () {
-      const tokenId = firstTokenId;
-      const data = '0x42';
-
-      let logs = null;
-
-      beforeEach(async function () {
-        await this.token.approve(approved, tokenId, { from: owner });
-        await this.token.setApprovalForAll(operator, true, { from: owner });
-      });
-
-      const transferWasSuccessful = function ({ owner, tokenId, approved }) {
-        it('transfers the ownership of the given token ID to the given address', async function () {
-          expect(await this.token.ownerOf(tokenId)).to.be.equal(this.toWhom);
-        });
-
-        it('emits a Transfer event', async function () {
-          expectEvent.inLogs(logs, 'Transfer', { from: owner, to: this.toWhom, tokenId: tokenId });
-        });
-
-        it('clears the approval for the token ID', async function () {
-          expect(await this.token.getApproved(tokenId)).to.be.equal(ZERO_ADDRESS);
-        });
-
-        it('emits an Approval event', async function () {
-          expectEvent.inLogs(logs, 'Approval', { owner, approved: ZERO_ADDRESS, tokenId: tokenId });
-        });
-
-        it('adjusts owners balances', async function () {
-          expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('1');
-        });
-
-        it('adjusts owners tokens by index', async function () {
-          if (!this.token.tokenOfOwnerByIndex) return;
-
-          expect(await this.token.tokenOfOwnerByIndex(this.toWhom, 0)).to.be.bignumber.equal(tokenId);
-
-          expect(await this.token.tokenOfOwnerByIndex(owner, 0)).to.be.bignumber.not.equal(tokenId);
-        });
-      };
-
-      const shouldTransferTokensByUsers = function (transferFunction) {
-        context('when called by the owner', function () {
-          beforeEach(async function () {
-            ({ logs } = await transferFunction.call(this, owner, this.toWhom, tokenId, { from: owner }));
-          });
-          transferWasSuccessful({ owner, tokenId, approved });
-        });
-
-        context('when called by the approved individual', function () {
-          beforeEach(async function () {
-            ({ logs } = await transferFunction.call(this, owner, this.toWhom, tokenId, { from: approved }));
-          });
-          transferWasSuccessful({ owner, tokenId, approved });
-        });
-
-        context('when called by the operator', function () {
-          beforeEach(async function () {
-            ({ logs } = await transferFunction.call(this, owner, this.toWhom, tokenId, { from: operator }));
-          });
-          transferWasSuccessful({ owner, tokenId, approved });
-        });
-
-        context('when called by the owner without an approved user', function () {
-          beforeEach(async function () {
-            await this.token.approve(ZERO_ADDRESS, tokenId, { from: owner });
-            ({ logs } = await transferFunction.call(this, owner, this.toWhom, tokenId, { from: operator }));
-          });
-          transferWasSuccessful({ owner, tokenId, approved: null });
-        });
-
-        context('when sent to the owner', function () {
-          beforeEach(async function () {
-            ({ logs } = await transferFunction.call(this, owner, owner, tokenId, { from: owner }));
-          });
-
-          it('keeps ownership of the token', async function () {
-            expect(await this.token.ownerOf(tokenId)).to.be.equal(owner);
-          });
-
-          it('clears the approval for the token ID', async function () {
-            expect(await this.token.getApproved(tokenId)).to.be.equal(ZERO_ADDRESS);
-          });
-
-          it('emits only a transfer event', async function () {
-            expectEvent.inLogs(logs, 'Transfer', {
-              from: owner,
-              to: owner,
-              tokenId: tokenId,
-            });
-          });
-
-          it('keeps the owner balance', async function () {
-            expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('2');
-          });
-
-          it('keeps same tokens by index', async function () {
-            if (!this.token.tokenOfOwnerByIndex) return;
-            const tokensListed = await Promise.all(
-              [0, 1].map(i => this.token.tokenOfOwnerByIndex(owner, i)),
-            );
-            expect(tokensListed.map(t => t.toNumber())).to.have.members(
-              [firstTokenId.toNumber(), secondTokenId.toNumber()],
-            );
-          });
-        });
-
-        context('when the address of the previous owner is incorrect', function () {
-          it('reverts', async function () {
-            await expectRevert(
-              transferFunction.call(this, other, other, tokenId, { from: owner }),
-              'ERC721: transfer of token that is not own',
-            );
-          });
-        });
-
-        context('when the sender is not authorized for the token id', function () {
-          it('reverts', async function () {
-            await expectRevert(
-              transferFunction.call(this, owner, other, tokenId, { from: other }),
-              'ERC721: transfer caller is not owner nor approved',
-            );
-          });
-        });
-
-        context('when the given token ID does not exist', function () {
-          it('reverts', async function () {
-            await expectRevert(
-              transferFunction.call(this, owner, other, nonExistentTokenId, { from: owner }),
-              'ERC721: operator query for nonexistent token',
-            );
-          });
-        });
-
-        context('when the address to transfer the token to is the zero address', function () {
-          it('reverts', async function () {
-            await expectRevert(
-              transferFunction.call(this, owner, ZERO_ADDRESS, tokenId, { from: owner }),
-              'ERC721: transfer to the zero address',
-            );
-          });
-        });
-      };
-
-      describe('via transferFrom', function () {
-        shouldTransferTokensByUsers(function (from, to, tokenId, opts) {
-          return this.token.transferFrom(from, to, tokenId, opts);
-        });
-      });
-
-      describe('via safeTransferFrom', function () {
-        const safeTransferFromWithData = function (from, to, tokenId, opts) {
-          return this.token.methods['safeTransferFrom(address,address,uint256,bytes)'](from, to, tokenId, data, opts);
-        };
-
-        const safeTransferFromWithoutData = function (from, to, tokenId, opts) {
-          return this.token.methods['safeTransferFrom(address,address,uint256)'](from, to, tokenId, opts);
-        };
-
-        const shouldTransferSafely = function (transferFun, data) {
-          describe('to a user account', function () {
-            shouldTransferTokensByUsers(transferFun);
-          });
-
-          describe('to a valid receiver contract', function () {
-            beforeEach(async function () {
-              this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.None);
-              this.toWhom = this.receiver.address;
-            });
-
-            shouldTransferTokensByUsers(transferFun);
-
-            it('calls onERC721Received', async function () {
-              const receipt = await transferFun.call(this, owner, this.receiver.address, tokenId, { from: owner });
-
-              await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', {
-                operator: owner,
-                from: owner,
-                tokenId: tokenId,
-                data: data,
-              });
-            });
-
-            it('calls onERC721Received from approved', async function () {
-              const receipt = await transferFun.call(this, owner, this.receiver.address, tokenId, { from: approved });
-
-              await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', {
-                operator: approved,
-                from: owner,
-                tokenId: tokenId,
-                data: data,
-              });
-            });
-
-            describe('with an invalid token id', function () {
-              it('reverts', async function () {
-                await expectRevert(
-                  transferFun.call(
-                    this,
-                    owner,
-                    this.receiver.address,
-                    nonExistentTokenId,
-                    { from: owner },
-                  ),
-                  'ERC721: operator query for nonexistent token',
-                );
-              });
-            });
-          });
-        };
-
-        describe('with data', function () {
-          shouldTransferSafely(safeTransferFromWithData, data);
-        });
-
-        describe('without data', function () {
-          shouldTransferSafely(safeTransferFromWithoutData, null);
-        });
-
-        describe('to a receiver contract returning unexpected value', function () {
-          it('reverts', async function () {
-            const invalidReceiver = await ERC721ReceiverMock.new('0x42', Error.None);
-            await expectRevert(
-              this.token.safeTransferFrom(owner, invalidReceiver.address, tokenId, { from: owner }),
-              'ERC721: transfer to non ERC721Receiver implementer',
-            );
-          });
-        });
-
-        describe('to a receiver contract that reverts with message', function () {
-          it('reverts', async function () {
-            const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.RevertWithMessage);
-            await expectRevert(
-              this.token.safeTransferFrom(owner, revertingReceiver.address, tokenId, { from: owner }),
-              'ERC721ReceiverMock: reverting',
-            );
-          });
-        });
-
-        describe('to a receiver contract that reverts without message', function () {
-          it('reverts', async function () {
-            const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.RevertWithoutMessage);
-            await expectRevert(
-              this.token.safeTransferFrom(owner, revertingReceiver.address, tokenId, { from: owner }),
-              'ERC721: transfer to non ERC721Receiver implementer',
-            );
-          });
-        });
-
-        describe('to a receiver contract that panics', function () {
-          it('reverts', async function () {
-            const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.Panic);
-            await expectRevert.unspecified(
-              this.token.safeTransferFrom(owner, revertingReceiver.address, tokenId, { from: owner }),
-            );
-          });
-        });
-
-        describe('to a contract that does not implement the required function', function () {
-          it('reverts', async function () {
-            const nonReceiver = this.token;
-            await expectRevert(
-              this.token.safeTransferFrom(owner, nonReceiver.address, tokenId, { from: owner }),
-              'ERC721: transfer to non ERC721Receiver implementer',
-            );
-          });
-        });
-      });
-    });
-
-    describe('safe mint', function () {
-      const fourthTokenId = new BN(4);
-      const tokenId = fourthTokenId;
-      const data = '0x42';
-
-      describe('via safeMint', function () { // regular minting is tested in ERC721Mintable.test.js and others
-        it('calls onERC721Received — with data', async function () {
-          this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.None);
-          const receipt = await this.token.safeMint(this.receiver.address, tokenId, data);
-
-          await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', {
-            from: ZERO_ADDRESS,
-            tokenId: tokenId,
-            data: data,
-          });
-        });
-
-        it('calls onERC721Received — without data', async function () {
-          this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.None);
-          const receipt = await this.token.safeMint(this.receiver.address, tokenId);
-
-          await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', {
-            from: ZERO_ADDRESS,
-            tokenId: tokenId,
-          });
-        });
-
-        context('to a receiver contract returning unexpected value', function () {
-          it('reverts', async function () {
-            const invalidReceiver = await ERC721ReceiverMock.new('0x42', Error.None);
-            await expectRevert(
-              this.token.safeMint(invalidReceiver.address, tokenId),
-              'ERC721: transfer to non ERC721Receiver implementer',
-            );
-          });
-        });
-
-        context('to a receiver contract that reverts with message', function () {
-          it('reverts', async function () {
-            const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.RevertWithMessage);
-            await expectRevert(
-              this.token.safeMint(revertingReceiver.address, tokenId),
-              'ERC721ReceiverMock: reverting',
-            );
-          });
-        });
-
-        context('to a receiver contract that reverts without message', function () {
-          it('reverts', async function () {
-            const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.RevertWithoutMessage);
-            await expectRevert(
-              this.token.safeMint(revertingReceiver.address, tokenId),
-              'ERC721: transfer to non ERC721Receiver implementer',
-            );
-          });
-        });
-
-        context('to a receiver contract that panics', function () {
-          it('reverts', async function () {
-            const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, Error.Panic);
-            await expectRevert.unspecified(
-              this.token.safeMint(revertingReceiver.address, tokenId),
-            );
-          });
-        });
-
-        context('to a contract that does not implement the required function', function () {
-          it('reverts', async function () {
-            const nonReceiver = this.token;
-            await expectRevert(
-              this.token.safeMint(nonReceiver.address, tokenId),
-              'ERC721: transfer to non ERC721Receiver implementer',
-            );
-          });
-        });
-      });
-    });
-
-    describe('approve', function () {
-      const tokenId = firstTokenId;
-
-      let logs = null;
-
-      const itClearsApproval = function () {
-        it('clears approval for the token', async function () {
-          expect(await this.token.getApproved(tokenId)).to.be.equal(ZERO_ADDRESS);
-        });
-      };
-
-      const itApproves = function (address) {
-        it('sets the approval for the target address', async function () {
-          expect(await this.token.getApproved(tokenId)).to.be.equal(address);
-        });
-      };
-
-      const itEmitsApprovalEvent = function (address) {
-        it('emits an approval event', async function () {
-          expectEvent.inLogs(logs, 'Approval', {
-            owner: owner,
-            approved: address,
-            tokenId: tokenId,
-          });
-        });
-      };
-
-      context('when clearing approval', function () {
-        context('when there was no prior approval', function () {
-          beforeEach(async function () {
-            ({ logs } = await this.token.approve(ZERO_ADDRESS, tokenId, { from: owner }));
-          });
-
-          itClearsApproval();
-          itEmitsApprovalEvent(ZERO_ADDRESS);
-        });
-
-        context('when there was a prior approval', function () {
-          beforeEach(async function () {
-            await this.token.approve(approved, tokenId, { from: owner });
-            ({ logs } = await this.token.approve(ZERO_ADDRESS, tokenId, { from: owner }));
-          });
-
-          itClearsApproval();
-          itEmitsApprovalEvent(ZERO_ADDRESS);
-        });
-      });
-
-      context('when approving a non-zero address', function () {
-        context('when there was no prior approval', function () {
-          beforeEach(async function () {
-            ({ logs } = await this.token.approve(approved, tokenId, { from: owner }));
-          });
-
-          itApproves(approved);
-          itEmitsApprovalEvent(approved);
-        });
-
-        context('when there was a prior approval to the same address', function () {
-          beforeEach(async function () {
-            await this.token.approve(approved, tokenId, { from: owner });
-            ({ logs } = await this.token.approve(approved, tokenId, { from: owner }));
-          });
-
-          itApproves(approved);
-          itEmitsApprovalEvent(approved);
-        });
-
-        context('when there was a prior approval to a different address', function () {
-          beforeEach(async function () {
-            await this.token.approve(anotherApproved, tokenId, { from: owner });
-            ({ logs } = await this.token.approve(anotherApproved, tokenId, { from: owner }));
-          });
-
-          itApproves(anotherApproved);
-          itEmitsApprovalEvent(anotherApproved);
-        });
-      });
-
-      context('when the address that receives the approval is the owner', function () {
-        it('reverts', async function () {
-          await expectRevert(
-            this.token.approve(owner, tokenId, { from: owner }), 'ERC721: approval to current owner',
-          );
-        });
-      });
-
-      context('when the sender does not own the given token ID', function () {
-        it('reverts', async function () {
-          await expectRevert(this.token.approve(approved, tokenId, { from: other }),
-            'ERC721: approve caller is not owner nor approved');
-        });
-      });
-
-      context('when the sender is approved for the given token ID', function () {
-        it('reverts', async function () {
-          await this.token.approve(approved, tokenId, { from: owner });
-          await expectRevert(this.token.approve(anotherApproved, tokenId, { from: approved }),
-            'ERC721: approve caller is not owner nor approved for all');
-        });
-      });
-
-      context('when the sender is an operator', function () {
-        beforeEach(async function () {
-          await this.token.setApprovalForAll(operator, true, { from: owner });
-          ({ logs } = await this.token.approve(approved, tokenId, { from: operator }));
-        });
-
-        itApproves(approved);
-        itEmitsApprovalEvent(approved);
-      });
-
-      context('when the given token ID does not exist', function () {
-        it('reverts', async function () {
-          await expectRevert(this.token.approve(approved, nonExistentTokenId, { from: operator }),
-            'ERC721: owner query for nonexistent token');
-        });
-      });
-    });
-
-    describe('setApprovalForAll', function () {
-      context('when the operator willing to approve is not the owner', function () {
-        context('when there is no operator approval set by the sender', function () {
-          it('approves the operator', async function () {
-            await this.token.setApprovalForAll(operator, true, { from: owner });
-
-            expect(await this.token.isApprovedForAll(owner, operator)).to.equal(true);
-          });
-
-          it('emits an approval event', async function () {
-            const { logs } = await this.token.setApprovalForAll(operator, true, { from: owner });
-
-            expectEvent.inLogs(logs, 'ApprovalForAll', {
-              owner: owner,
-              operator: operator,
-              approved: true,
-            });
-          });
-        });
-
-        context('when the operator was set as not approved', function () {
-          beforeEach(async function () {
-            await this.token.setApprovalForAll(operator, false, { from: owner });
-          });
-
-          it('approves the operator', async function () {
-            await this.token.setApprovalForAll(operator, true, { from: owner });
-
-            expect(await this.token.isApprovedForAll(owner, operator)).to.equal(true);
-          });
-
-          it('emits an approval event', async function () {
-            const { logs } = await this.token.setApprovalForAll(operator, true, { from: owner });
-
-            expectEvent.inLogs(logs, 'ApprovalForAll', {
-              owner: owner,
-              operator: operator,
-              approved: true,
-            });
-          });
-
-          it('can unset the operator approval', async function () {
-            await this.token.setApprovalForAll(operator, false, { from: owner });
-
-            expect(await this.token.isApprovedForAll(owner, operator)).to.equal(false);
-          });
-        });
-
-        context('when the operator was already approved', function () {
-          beforeEach(async function () {
-            await this.token.setApprovalForAll(operator, true, { from: owner });
-          });
-
-          it('keeps the approval to the given address', async function () {
-            await this.token.setApprovalForAll(operator, true, { from: owner });
-
-            expect(await this.token.isApprovedForAll(owner, operator)).to.equal(true);
-          });
-
-          it('emits an approval event', async function () {
-            const { logs } = await this.token.setApprovalForAll(operator, true, { from: owner });
-
-            expectEvent.inLogs(logs, 'ApprovalForAll', {
-              owner: owner,
-              operator: operator,
-              approved: true,
-            });
-          });
-        });
-      });
-
-      context('when the operator is the owner', function () {
-        it('reverts', async function () {
-          await expectRevert(this.token.setApprovalForAll(owner, true, { from: owner }),
-            'ERC721: approve to caller');
-        });
-      });
-    });
-
-    describe('getApproved', async function () {
-      context('when token is not minted', async function () {
-        it('reverts', async function () {
-          await expectRevert(
-            this.token.getApproved(nonExistentTokenId),
-            'ERC721: approved query for nonexistent token',
-          );
-        });
-      });
-
-      context('when token has been minted ', async function () {
-        it('should return the zero address', async function () {
-          expect(await this.token.getApproved(firstTokenId)).to.be.equal(
-            ZERO_ADDRESS,
-          );
-        });
-
-        context('when account has been approved', async function () {
-          beforeEach(async function () {
-            await this.token.approve(approved, firstTokenId, { from: owner });
-          });
-
-          it('returns approved account', async function () {
-            expect(await this.token.getApproved(firstTokenId)).to.be.equal(approved);
-          });
-        });
-      });
-    });
-
-    describe('totalSupply', function () {
-      it('returns total token supply', async function () {
-        expect(await this.token.totalSupply()).to.be.bignumber.equal('2');
-      });
-    });
-
-    describe('tokenOfOwnerByIndex', function () {
-      describe('when the given index is lower than the amount of tokens owned by the given address', function () {
-        it('returns the token ID placed at the given index', async function () {
-          expect(await this.token.tokenOfOwnerByIndex(owner, 0)).to.be.bignumber.equal(firstTokenId);
-        });
-      });
-
-      describe('when the index is greater than or equal to the total tokens owned by the given address', function () {
-        it('reverts', async function () {
-          await expectRevert(
-            this.token.tokenOfOwnerByIndex(owner, 2), 'EnumerableSet: index out of bounds',
-          );
-        });
-      });
-
-      describe('when the given address does not own any token', function () {
-        it('reverts', async function () {
-          await expectRevert(
-            this.token.tokenOfOwnerByIndex(other, 0), 'EnumerableSet: index out of bounds',
-          );
-        });
-      });
-
-      describe('after transferring all tokens to another user', function () {
-        beforeEach(async function () {
-          await this.token.transferFrom(owner, other, firstTokenId, { from: owner });
-          await this.token.transferFrom(owner, other, secondTokenId, { from: owner });
-        });
-
-        it('returns correct token IDs for target', async function () {
-          expect(await this.token.balanceOf(other)).to.be.bignumber.equal('2');
-          const tokensListed = await Promise.all(
-            [0, 1].map(i => this.token.tokenOfOwnerByIndex(other, i)),
-          );
-          expect(tokensListed.map(t => t.toNumber())).to.have.members([firstTokenId.toNumber(),
-            secondTokenId.toNumber()]);
-        });
-
-        it('returns empty collection for original owner', async function () {
-          expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('0');
-          await expectRevert(
-            this.token.tokenOfOwnerByIndex(owner, 0), 'EnumerableSet: index out of bounds',
-          );
-        });
-      });
-    });
-
-    describe('tokenByIndex', function () {
-      it('returns all tokens', async function () {
-        const tokensListed = await Promise.all(
-          [0, 1].map(i => this.token.tokenByIndex(i)),
-        );
-        expect(tokensListed.map(t => t.toNumber())).to.have.members([firstTokenId.toNumber(),
-          secondTokenId.toNumber()]);
-      });
-
-      it('reverts if index is greater than supply', async function () {
-        await expectRevert(
-          this.token.tokenByIndex(2), 'EnumerableSet: index out of bounds',
-        );
-      });
-
-      [firstTokenId, secondTokenId].forEach(function (tokenId) {
-        it(`returns all tokens after burning token ${tokenId} and minting new tokens`, async function () {
-          const newTokenId = new BN(300);
-          const anotherNewTokenId = new BN(400);
-
-          await this.token.burn(tokenId);
-          await this.token.mint(newOwner, newTokenId);
-          await this.token.mint(newOwner, anotherNewTokenId);
-
-          expect(await this.token.totalSupply()).to.be.bignumber.equal('3');
-
-          const tokensListed = await Promise.all(
-            [0, 1, 2].map(i => this.token.tokenByIndex(i)),
-          );
-          const expectedTokens = [firstTokenId, secondTokenId, newTokenId, anotherNewTokenId].filter(
-            x => (x !== tokenId),
-          );
-          expect(tokensListed.map(t => t.toNumber())).to.have.members(expectedTokens.map(t => t.toNumber()));
-        });
-      });
-    });
-  });
-
-  describe('_mint(address, uint256)', function () {
-    it('reverts with a null destination address', async function () {
-      await expectRevert(
-        this.token.mint(ZERO_ADDRESS, firstTokenId), 'ERC721: mint to the zero address',
-      );
-    });
-
-    context('with minted token', async function () {
-      beforeEach(async function () {
-        ({ logs: this.logs } = await this.token.mint(owner, firstTokenId));
-      });
-
-      it('emits a Transfer event', function () {
-        expectEvent.inLogs(this.logs, 'Transfer', { from: ZERO_ADDRESS, to: owner, tokenId: firstTokenId });
-      });
-
-      it('creates the token', async function () {
-        expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('1');
-        expect(await this.token.ownerOf(firstTokenId)).to.equal(owner);
-      });
-
-      it('adjusts owner tokens by index', async function () {
-        expect(await this.token.tokenOfOwnerByIndex(owner, 0)).to.be.bignumber.equal(firstTokenId);
-      });
-
-      it('adjusts all tokens list', async function () {
-        expect(await this.token.tokenByIndex(0)).to.be.bignumber.equal(firstTokenId);
-      });
-
-      it('reverts when adding a token id that already exists', async function () {
-        await expectRevert(this.token.mint(owner, firstTokenId), 'ERC721: token already minted');
-      });
-    });
-  });
-
-  describe('_burn', function () {
-    it('reverts when burning a non-existent token id', async function () {
-      await expectRevert(
-        this.token.burn(firstTokenId), 'ERC721: owner query for nonexistent token',
-      );
-    });
-
-    context('with minted tokens', function () {
-      beforeEach(async function () {
-        await this.token.mint(owner, firstTokenId);
-        await this.token.mint(owner, secondTokenId);
-      });
-
-      context('with burnt token', function () {
-        beforeEach(async function () {
-          ({ logs: this.logs } = await this.token.burn(firstTokenId));
-        });
-
-        it('emits a Transfer event', function () {
-          expectEvent.inLogs(this.logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, tokenId: firstTokenId });
-        });
-
-        it('emits an Approval event', function () {
-          expectEvent.inLogs(this.logs, 'Approval', { owner, approved: ZERO_ADDRESS, tokenId: firstTokenId });
-        });
-
-        it('deletes the token', async function () {
-          expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('1');
-          await expectRevert(
-            this.token.ownerOf(firstTokenId), 'ERC721: owner query for nonexistent token',
-          );
-        });
-
-        it('removes that token from the token list of the owner', async function () {
-          expect(await this.token.tokenOfOwnerByIndex(owner, 0)).to.be.bignumber.equal(secondTokenId);
-        });
-
-        it('adjusts all tokens list', async function () {
-          expect(await this.token.tokenByIndex(0)).to.be.bignumber.equal(secondTokenId);
-        });
-
-        it('burns all tokens', async function () {
-          await this.token.burn(secondTokenId, { from: owner });
-          expect(await this.token.totalSupply()).to.be.bignumber.equal('0');
-          await expectRevert(
-            this.token.tokenByIndex(0), 'EnumerableSet: index out of bounds',
-          );
-        });
-
-        it('reverts when burning a token id that has been deleted', async function () {
-          await expectRevert(
-            this.token.burn(firstTokenId), 'ERC721: owner query for nonexistent token',
-          );
-        });
-      });
-    });
-  });
+  shouldBehaveLikeERC721('ERC721', ...accounts);
+  shouldBehaveLikeERC721Metadata('ERC721', name, symbol, ...accounts);
 });

+ 20 - 0
test/token/ERC721/ERC721Enumerable.test.js

@@ -0,0 +1,20 @@
+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);
+});