From 4e5fa5e137622ce3ebf7d1baad55e2a65c0ca0cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 26 Mar 2020 12:34:40 -0300 Subject: [PATCH 1/8] Add IERC721Metadata implementation into ERC721 --- contracts/token/ERC721/ERC721.sol | 106 +++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 3 deletions(-) diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index ea68f0aa893..430f652ac67 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -2,17 +2,18 @@ pragma solidity ^0.6.0; import "../../GSN/Context.sol"; import "./IERC721.sol"; +import "./IERC721Metadata.sol"; import "./IERC721Receiver.sol"; +import "../../introspection/ERC165.sol"; import "../../math/SafeMath.sol"; import "../../utils/Address.sol"; import "../../utils/Counters.sol"; -import "../../introspection/ERC165.sol"; /** * @title ERC721 Non-Fungible Token Standard basic implementation * @dev see https://eips.ethereum.org/EIPS/eip-721 */ -contract ERC721 is Context, ERC165, IERC721 { +contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { using SafeMath for uint256; using Address for address; using Counters for Counters.Counter; @@ -33,6 +34,27 @@ contract ERC721 is Context, ERC165, IERC721 { // 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; + + // Base URI + string private _baseURI; + + /* + * bytes4(keccak256('name()')) == 0x06fdde03 + * bytes4(keccak256('symbol()')) == 0x95d89b41 + * bytes4(keccak256('tokenURI(uint256)')) == 0xc87b56dd + * + * => 0x06fdde03 ^ 0x95d89b41 ^ 0xc87b56dd == 0x5b5e139f + */ + bytes4 private constant _INTERFACE_ID_ERC721_METADATA = 0x5b5e139f; + /* * bytes4(keccak256('balanceOf(address)')) == 0x70a08231 * bytes4(keccak256('ownerOf(uint256)')) == 0x6352211e @@ -49,9 +71,13 @@ contract ERC721 is Context, ERC165, IERC721 { */ bytes4 private constant _INTERFACE_ID_ERC721 = 0x80ac58cd; - constructor () public { + constructor (string memory name, string memory symbol) public { + _name = name; + _symbol = symbol; + // register the supported interfaces to conform to ERC721 via ERC165 _registerInterface(_INTERFACE_ID_ERC721); + _registerInterface(_INTERFACE_ID_ERC721_METADATA); } /** @@ -77,6 +103,53 @@ contract ERC721 is Context, ERC165, IERC721 { return owner; } + /** + * @dev Gets the token name. + * @return string representing the token name + */ + function name() external view override returns (string memory) { + return _name; + } + + /** + * @dev Gets the token symbol. + * @return string representing the token symbol + */ + function symbol() external view override returns (string memory) { + return _symbol; + } + + /** + * @dev Returns the URI for a given token ID. May return an empty string. + * + * If the token's URI is non-empty and a base URI was set (via + * {_setBaseURI}), it will be added to the token ID's URI as a prefix. + * + * Reverts if the token ID does not exist. + */ + function tokenURI(uint256 tokenId) external view override returns (string memory) { + require(_exists(tokenId), "ERC721Metadata: URI query for nonexistent token"); + + string memory _tokenURI = _tokenURIs[tokenId]; + + // Even if there is a base URI, it is only appended to non-empty token-specific URIs + if (bytes(_tokenURI).length == 0) { + return ""; + } else { + // abi.encodePacked is being used to concatenate strings + return string(abi.encodePacked(_baseURI, _tokenURI)); + } + } + + /** + * @dev Returns the base URI set via {_setBaseURI}. This will be + * automatically added as a preffix in {tokenURI} to each token's URI, when + * they are non-empty. + */ + function baseURI() external view returns (string memory) { + return _baseURI; + } + /** * @dev Approves another address to transfer the given token ID * The zero address indicates there is no approved address. @@ -276,6 +349,11 @@ contract ERC721 is Context, ERC165, IERC721 { _beforeTokenTransfer(owner, address(0), tokenId); + // Clear metadata (if any) + if (bytes(_tokenURIs[tokenId]).length != 0) { + delete _tokenURIs[tokenId]; + } + // Clear approvals _approve(address(0), tokenId); @@ -309,6 +387,28 @@ contract ERC721 is Context, ERC165, IERC721 { emit Transfer(from, to, tokenId); } + /** + * @dev Internal function to set the token URI for a given token. + * + * Reverts if the token ID does not exist. + * + * TIP: if all token IDs share a prefix (e.g. if your URIs look like + * `http://api.myproject.com/token/`), use {_setBaseURI} to store + * it and save gas. + */ + 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}. + */ + function _setBaseURI(string memory baseURI) internal virtual { + _baseURI = baseURI; + } + /** * @dev Internal function to invoke {IERC721Receiver-onERC721Received} on a target address. * The call is not executed if the target address is not a contract. From d6d0955f73064d2d558bcc60de61a12cc0d06f58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 26 Mar 2020 13:54:58 -0300 Subject: [PATCH 2/8] Add IERC721Enumerable into ERC721 --- contracts/token/ERC721/ERC721.sol | 171 ++++++++++++++++++++++++++++-- 1 file changed, 160 insertions(+), 11 deletions(-) diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index 430f652ac67..cabbb17927c 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -3,6 +3,7 @@ pragma solidity ^0.6.0; import "../../GSN/Context.sol"; import "./IERC721.sol"; import "./IERC721Metadata.sol"; +import "./IERC721Enumerable.sol"; import "./IERC721Receiver.sol"; import "../../introspection/ERC165.sol"; import "../../math/SafeMath.sol"; @@ -13,7 +14,7 @@ import "../../utils/Counters.sol"; * @title ERC721 Non-Fungible Token Standard basic implementation * @dev see https://eips.ethereum.org/EIPS/eip-721 */ -contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { +contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable { using SafeMath for uint256; using Address for address; using Counters for Counters.Counter; @@ -46,14 +47,17 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { // Base URI string private _baseURI; - /* - * bytes4(keccak256('name()')) == 0x06fdde03 - * bytes4(keccak256('symbol()')) == 0x95d89b41 - * bytes4(keccak256('tokenURI(uint256)')) == 0xc87b56dd - * - * => 0x06fdde03 ^ 0x95d89b41 ^ 0xc87b56dd == 0x5b5e139f - */ - bytes4 private constant _INTERFACE_ID_ERC721_METADATA = 0x5b5e139f; + // Mapping from owner to list of owned token IDs + mapping(address => 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; /* * bytes4(keccak256('balanceOf(address)')) == 0x70a08231 @@ -71,6 +75,24 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { */ bytes4 private constant _INTERFACE_ID_ERC721 = 0x80ac58cd; + /* + * bytes4(keccak256('name()')) == 0x06fdde03 + * bytes4(keccak256('symbol()')) == 0x95d89b41 + * bytes4(keccak256('tokenURI(uint256)')) == 0xc87b56dd + * + * => 0x06fdde03 ^ 0x95d89b41 ^ 0xc87b56dd == 0x5b5e139f + */ + bytes4 private constant _INTERFACE_ID_ERC721_METADATA = 0x5b5e139f; + + /* + * bytes4(keccak256('totalSupply()')) == 0x18160ddd + * bytes4(keccak256('tokenOfOwnerByIndex(address,uint256)')) == 0x2f745c59 + * bytes4(keccak256('tokenByIndex(uint256)')) == 0x4f6ccce7 + * + * => 0x18160ddd ^ 0x2f745c59 ^ 0x4f6ccce7 == 0x780e9d63 + */ + bytes4 private constant _INTERFACE_ID_ERC721_ENUMERABLE = 0x780e9d63; + constructor (string memory name, string memory symbol) public { _name = name; _symbol = symbol; @@ -78,6 +100,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { // register the supported interfaces to conform to ERC721 via ERC165 _registerInterface(_INTERFACE_ID_ERC721); _registerInterface(_INTERFACE_ID_ERC721_METADATA); + _registerInterface(_INTERFACE_ID_ERC721_ENUMERABLE); } /** @@ -150,6 +173,36 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { return _baseURI; } + /** + * @dev Gets the token ID at a given index of the tokens list of the requested owner. + * @param owner address owning the tokens list to be accessed + * @param index uint256 representing the index to be accessed of the requested tokens list + * @return uint256 token ID at the given index of the tokens list owned by the requested address + */ + function tokenOfOwnerByIndex(address owner, uint256 index) public view override returns (uint256) { + require(index < balanceOf(owner), "ERC721Enumerable: owner index out of bounds"); + return _ownedTokens[owner][index]; + } + + /** + * @dev Gets the total amount of tokens stored by the contract. + * @return uint256 representing the total amount of tokens + */ + function totalSupply() public view override returns (uint256) { + return _allTokens.length; + } + + /** + * @dev Gets the token ID at a given index of all the tokens in this contract + * Reverts if the index is greater or equal to the total number of tokens. + * @param index uint256 representing the index to be accessed of the tokens list + * @return uint256 token ID at the given index of the tokens list + */ + function tokenByIndex(uint256 index) public view override returns (uint256) { + require(index < totalSupply(), "ERC721Enumerable: global index out of bounds"); + return _allTokens[index]; + } + /** * @dev Approves another address to transfer the given token ID * The zero address indicates there is no approved address. @@ -333,6 +386,9 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { _beforeTokenTransfer(address(0), to, tokenId); + _addTokenToOwnerEnumeration(to, tokenId); + _addTokenToAllTokensEnumeration(tokenId); + _tokenOwner[tokenId] = to; _ownedTokensCount[to].increment(); @@ -354,6 +410,12 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { delete _tokenURIs[tokenId]; } + _removeTokenFromOwnerEnumeration(owner, tokenId); + // Since tokenId will be deleted, we can clear its slot in _ownedTokensIndex to trigger a gas refund + _ownedTokensIndex[tokenId] = 0; + + _removeTokenFromAllTokensEnumeration(tokenId); + // Clear approvals _approve(address(0), tokenId); @@ -376,6 +438,9 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { _beforeTokenTransfer(from, to, tokenId); + _removeTokenFromOwnerEnumeration(from, tokenId); + _addTokenToOwnerEnumeration(to, tokenId); + // Clear approvals _approve(address(0), tokenId); @@ -405,8 +470,17 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { * @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}. */ - function _setBaseURI(string memory baseURI) internal virtual { - _baseURI = baseURI; + function _setBaseURI(string memory baseURI_) internal virtual { + _baseURI = baseURI_; + } + + /** + * @dev Gets the list of token IDs of the requested owner. + * @param owner address owning the tokens + * @return uint256[] List of token IDs owned by the requested address + */ + function _tokensOfOwner(address owner) internal view returns (uint256[] storage) { + return _ownedTokens[owner]; } /** @@ -454,6 +528,81 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { emit Approval(ownerOf(tokenId), 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 { + _ownedTokensIndex[tokenId] = _ownedTokens[to].length; + _ownedTokens[to].push(tokenId); + } + + /** + * @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 = _ownedTokens[from].length.sub(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 + } + + // Deletes the contents at the last position of the array + _ownedTokens[from].pop(); + + // Note that _ownedTokensIndex[tokenId] hasn't been cleared: it still points to the old slot (now occupied by + // lastTokenId, or just over the end of the array if the token was the last one). + } + + /** + * @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.sub(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 + + // Delete the contents at the last position of the array + _allTokens.pop(); + + _allTokensIndex[tokenId] = 0; + } + /** * @dev Hook that is called before any token transfer. This includes minting * and burning. From c00c25156d26f3de0dddd6860d79f7edcd0f2415 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 26 Mar 2020 13:55:24 -0300 Subject: [PATCH 3/8] Update ERC721Pausable and ERC721Burnable --- contracts/token/ERC721/ERC721Burnable.sol | 2 ++ contracts/token/ERC721/ERC721Pausable.sol | 2 ++ 2 files changed, 4 insertions(+) diff --git a/contracts/token/ERC721/ERC721Burnable.sol b/contracts/token/ERC721/ERC721Burnable.sol index 5c6e3efab26..16de824490f 100644 --- a/contracts/token/ERC721/ERC721Burnable.sol +++ b/contracts/token/ERC721/ERC721Burnable.sol @@ -8,6 +8,8 @@ import "./ERC721.sol"; * @dev ERC721 Token that can be irreversibly burned (destroyed). */ contract ERC721Burnable is Context, ERC721 { + constructor (string memory name, string memory symbol) public ERC721(name, symbol) { } + /** * @dev Burns a specific ERC721 token. * @param tokenId uint256 id of the ERC721 token to be burned. diff --git a/contracts/token/ERC721/ERC721Pausable.sol b/contracts/token/ERC721/ERC721Pausable.sol index 3d422f89d53..a531cbddd18 100644 --- a/contracts/token/ERC721/ERC721Pausable.sol +++ b/contracts/token/ERC721/ERC721Pausable.sol @@ -8,6 +8,8 @@ import "../../utils/Pausable.sol"; * @dev ERC721 modified with pausable transfers. */ contract ERC721Pausable is ERC721, Pausable { + constructor (string memory name, string memory symbol) public ERC721(name, symbol) { } + function _beforeTokenTransfer(address from, address to, uint256 tokenId) internal virtual override { super._beforeTokenTransfer(from, to, tokenId); From 95465823d32c6a0e9ce411d302d9587fad3f23e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 26 Mar 2020 13:55:56 -0300 Subject: [PATCH 4/8] Delete ERC721Metadata, ERC721Enumerable and ERC721 (now ERC721) --- contracts/token/ERC721/ERC721Enumerable.sol | 175 -------------------- contracts/token/ERC721/ERC721Full.sol | 23 --- contracts/token/ERC721/ERC721Metadata.sol | 121 -------------- contracts/token/ERC721/IERC721Full.sol | 11 -- 4 files changed, 330 deletions(-) delete mode 100644 contracts/token/ERC721/ERC721Enumerable.sol delete mode 100644 contracts/token/ERC721/ERC721Full.sol delete mode 100644 contracts/token/ERC721/ERC721Metadata.sol delete mode 100644 contracts/token/ERC721/IERC721Full.sol diff --git a/contracts/token/ERC721/ERC721Enumerable.sol b/contracts/token/ERC721/ERC721Enumerable.sol deleted file mode 100644 index 2a250862223..00000000000 --- a/contracts/token/ERC721/ERC721Enumerable.sol +++ /dev/null @@ -1,175 +0,0 @@ -pragma solidity ^0.6.0; - -import "../../GSN/Context.sol"; -import "./IERC721Enumerable.sol"; -import "./ERC721.sol"; -import "../../introspection/ERC165.sol"; - -/** - * @title ERC-721 Non-Fungible Token with optional enumeration extension logic - * @dev See https://eips.ethereum.org/EIPS/eip-721 - */ -contract ERC721Enumerable is Context, ERC165, ERC721, IERC721Enumerable { - // Mapping from owner to list of owned token IDs - mapping(address => 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; - - /* - * bytes4(keccak256('totalSupply()')) == 0x18160ddd - * bytes4(keccak256('tokenOfOwnerByIndex(address,uint256)')) == 0x2f745c59 - * bytes4(keccak256('tokenByIndex(uint256)')) == 0x4f6ccce7 - * - * => 0x18160ddd ^ 0x2f745c59 ^ 0x4f6ccce7 == 0x780e9d63 - */ - bytes4 private constant _INTERFACE_ID_ERC721_ENUMERABLE = 0x780e9d63; - - /** - * @dev Constructor function. - */ - constructor () public { - // register the supported interface to conform to ERC721Enumerable via ERC165 - _registerInterface(_INTERFACE_ID_ERC721_ENUMERABLE); - } - - /** - * @dev Gets the token ID at a given index of the tokens list of the requested owner. - * @param owner address owning the tokens list to be accessed - * @param index uint256 representing the index to be accessed of the requested tokens list - * @return uint256 token ID at the given index of the tokens list owned by the requested address - */ - function tokenOfOwnerByIndex(address owner, uint256 index) public view override returns (uint256) { - require(index < balanceOf(owner), "ERC721Enumerable: owner index out of bounds"); - return _ownedTokens[owner][index]; - } - - /** - * @dev Gets the total amount of tokens stored by the contract. - * @return uint256 representing the total amount of tokens - */ - function totalSupply() public view override returns (uint256) { - return _allTokens.length; - } - - /** - * @dev Gets the token ID at a given index of all the tokens in this contract - * Reverts if the index is greater or equal to the total number of tokens. - * @param index uint256 representing the index to be accessed of the tokens list - * @return uint256 token ID at the given index of the tokens list - */ - function tokenByIndex(uint256 index) public view override returns (uint256) { - require(index < totalSupply(), "ERC721Enumerable: global index out of bounds"); - return _allTokens[index]; - } - - function _beforeTokenTransfer(address from, address to, uint256 tokenId) internal virtual override { - super._beforeTokenTransfer(from, to, tokenId); - - if (from == address(0)) { - // When minting - _addTokenToOwnerEnumeration(to, tokenId); - _addTokenToAllTokensEnumeration(tokenId); - } else if (to == address(0)) { - // When burning - _removeTokenFromOwnerEnumeration(from, tokenId); - // Since tokenId will be deleted, we can clear its slot in _ownedTokensIndex to trigger a gas refund - _ownedTokensIndex[tokenId] = 0; - - _removeTokenFromAllTokensEnumeration(tokenId); - } else { - _removeTokenFromOwnerEnumeration(from, tokenId); - _addTokenToOwnerEnumeration(to, tokenId); - } - } - - /** - * @dev Gets the list of token IDs of the requested owner. - * @param owner address owning the tokens - * @return uint256[] List of token IDs owned by the requested address - */ - function _tokensOfOwner(address owner) internal view returns (uint256[] storage) { - return _ownedTokens[owner]; - } - - /** - * @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 { - _ownedTokensIndex[tokenId] = _ownedTokens[to].length; - _ownedTokens[to].push(tokenId); - } - - /** - * @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 = _ownedTokens[from].length.sub(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 - } - - // Deletes the contents at the last position of the array - _ownedTokens[from].pop(); - - // Note that _ownedTokensIndex[tokenId] hasn't been cleared: it still points to the old slot (now occupied by - // lastTokenId, or just over the end of the array if the token was the last one). - } - - /** - * @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.sub(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 - - // Delete the contents at the last position of the array - _allTokens.pop(); - - _allTokensIndex[tokenId] = 0; - } -} diff --git a/contracts/token/ERC721/ERC721Full.sol b/contracts/token/ERC721/ERC721Full.sol deleted file mode 100644 index f22845b6eb6..00000000000 --- a/contracts/token/ERC721/ERC721Full.sol +++ /dev/null @@ -1,23 +0,0 @@ -pragma solidity ^0.6.0; - -import "./ERC721Enumerable.sol"; -import "./ERC721Metadata.sol"; - -/** - * @title Full ERC721 Token - * @dev This implementation includes all the required and some optional functionality of the ERC721 standard - * Moreover, it includes approve all functionality using operator terminology. - * - * See https://eips.ethereum.org/EIPS/eip-721 - */ -contract ERC721Full is ERC721Enumerable, ERC721Metadata { - constructor (string memory name, string memory symbol) public ERC721Metadata(name, symbol) { } - - function _beforeTokenTransfer(address from, address to, uint256 tokenId) - virtual - override(ERC721Enumerable, ERC721Metadata) - internal - { - super._beforeTokenTransfer(from, to, tokenId); - } -} diff --git a/contracts/token/ERC721/ERC721Metadata.sol b/contracts/token/ERC721/ERC721Metadata.sol deleted file mode 100644 index 391014df7cd..00000000000 --- a/contracts/token/ERC721/ERC721Metadata.sol +++ /dev/null @@ -1,121 +0,0 @@ -pragma solidity ^0.6.0; - -import "../../GSN/Context.sol"; -import "./ERC721.sol"; -import "./IERC721Metadata.sol"; -import "../../introspection/ERC165.sol"; - -contract ERC721Metadata is Context, ERC165, ERC721, IERC721Metadata { - // Token name - string private _name; - - // Token symbol - string private _symbol; - - // Optional mapping for token URIs - mapping(uint256 => string) private _tokenURIs; - - // Base URI - string private _baseURI; - - /* - * bytes4(keccak256('name()')) == 0x06fdde03 - * bytes4(keccak256('symbol()')) == 0x95d89b41 - * bytes4(keccak256('tokenURI(uint256)')) == 0xc87b56dd - * - * => 0x06fdde03 ^ 0x95d89b41 ^ 0xc87b56dd == 0x5b5e139f - */ - bytes4 private constant _INTERFACE_ID_ERC721_METADATA = 0x5b5e139f; - - /** - * @dev Constructor function - */ - constructor (string memory name, string memory symbol) public { - _name = name; - _symbol = symbol; - - // register the supported interfaces to conform to ERC721 via ERC165 - _registerInterface(_INTERFACE_ID_ERC721_METADATA); - } - - /** - * @dev Gets the token name. - * @return string representing the token name - */ - function name() external view override returns (string memory) { - return _name; - } - - /** - * @dev Gets the token symbol. - * @return string representing the token symbol - */ - function symbol() external view override returns (string memory) { - return _symbol; - } - - /** - * @dev Returns the URI for a given token ID. May return an empty string. - * - * If the token's URI is non-empty and a base URI was set (via - * {_setBaseURI}), it will be added to the token ID's URI as a prefix. - * - * Reverts if the token ID does not exist. - */ - function tokenURI(uint256 tokenId) external view override returns (string memory) { - require(_exists(tokenId), "ERC721Metadata: URI query for nonexistent token"); - - string memory _tokenURI = _tokenURIs[tokenId]; - - // Even if there is a base URI, it is only appended to non-empty token-specific URIs - if (bytes(_tokenURI).length == 0) { - return ""; - } else { - // abi.encodePacked is being used to concatenate strings - return string(abi.encodePacked(_baseURI, _tokenURI)); - } - } - - /** - * @dev Internal function to set the token URI for a given token. - * - * Reverts if the token ID does not exist. - * - * TIP: if all token IDs share a prefix (e.g. if your URIs look like - * `http://api.myproject.com/token/`), use {_setBaseURI} to store - * it and save gas. - */ - 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}. - */ - function _setBaseURI(string memory baseURI) internal virtual { - _baseURI = baseURI; - } - - /** - * @dev Returns the base URI set via {_setBaseURI}. This will be - * automatically added as a preffix in {tokenURI} to each token's URI, when - * they are non-empty. - */ - function baseURI() external view returns (string memory) { - return _baseURI; - } - - - function _beforeTokenTransfer(address from, address to, uint256 tokenId) internal virtual override { - super._beforeTokenTransfer(from, to, tokenId); - - if (to == address(0)) { // When burning tokens - // Clear metadata (if any) - if (bytes(_tokenURIs[tokenId]).length != 0) { - delete _tokenURIs[tokenId]; - } - } - } -} diff --git a/contracts/token/ERC721/IERC721Full.sol b/contracts/token/ERC721/IERC721Full.sol deleted file mode 100644 index af4ee87f6f6..00000000000 --- a/contracts/token/ERC721/IERC721Full.sol +++ /dev/null @@ -1,11 +0,0 @@ -pragma solidity ^0.6.0; - -import "./IERC721.sol"; -import "./IERC721Enumerable.sol"; -import "./IERC721Metadata.sol"; - -/** - * @title ERC-721 Non-Fungible Token Standard, full implementation interface - * @dev See https://eips.ethereum.org/EIPS/eip-721 - */ -abstract contract IERC721Full is IERC721, IERC721Enumerable, IERC721Metadata { } From 626788f2c7e20a18b9ec83d76ac92aea908ec6d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 26 Mar 2020 13:56:06 -0300 Subject: [PATCH 5/8] Update mocks --- contracts/mocks/ERC721BurnableMock.sol | 2 ++ contracts/mocks/ERC721FullMock.sol | 37 ---------------------- contracts/mocks/ERC721GSNRecipientMock.sol | 6 +++- contracts/mocks/ERC721Mock.sol | 26 ++++++++++++--- contracts/mocks/ERC721PausableMock.sol | 2 ++ 5 files changed, 31 insertions(+), 42 deletions(-) delete mode 100644 contracts/mocks/ERC721FullMock.sol diff --git a/contracts/mocks/ERC721BurnableMock.sol b/contracts/mocks/ERC721BurnableMock.sol index bb69ffb003f..67d5c3babac 100644 --- a/contracts/mocks/ERC721BurnableMock.sol +++ b/contracts/mocks/ERC721BurnableMock.sol @@ -3,6 +3,8 @@ pragma solidity ^0.6.0; import "../token/ERC721/ERC721Burnable.sol"; contract ERC721BurnableMock is ERC721Burnable { + constructor(string memory name, string memory symbol) public ERC721Burnable(name, symbol) { } + function mint(address to, uint256 tokenId) public { _mint(to, tokenId); } diff --git a/contracts/mocks/ERC721FullMock.sol b/contracts/mocks/ERC721FullMock.sol deleted file mode 100644 index 9aef7c1be09..00000000000 --- a/contracts/mocks/ERC721FullMock.sol +++ /dev/null @@ -1,37 +0,0 @@ -pragma solidity ^0.6.0; - -import "../token/ERC721/ERC721Full.sol"; -import "../token/ERC721/ERC721Burnable.sol"; - -/** - * @title ERC721FullMock - * This mock just provides public functions for setting metadata URI, getting all tokens of an owner, - * checking token existence, removal of a token from an address - */ -contract ERC721FullMock is ERC721Full, ERC721Burnable { - constructor (string memory name, string memory symbol) public ERC721Full(name, symbol) { } - - function exists(uint256 tokenId) public view returns (bool) { - return _exists(tokenId); - } - - function tokensOfOwner(address owner) public view returns (uint256[] memory) { - return _tokensOfOwner(owner); - } - - function setTokenURI(uint256 tokenId, string memory uri) public { - _setTokenURI(tokenId, uri); - } - - function setBaseURI(string memory baseURI) public { - _setBaseURI(baseURI); - } - - function _beforeTokenTransfer(address from, address to, uint256 tokenId) internal virtual override(ERC721, ERC721Full) { - super._beforeTokenTransfer(from, to, tokenId); - } - - function mint(address to, uint256 tokenId) public { - _mint(to, tokenId); - } -} diff --git a/contracts/mocks/ERC721GSNRecipientMock.sol b/contracts/mocks/ERC721GSNRecipientMock.sol index c5f7f3e850c..7c5403143e7 100644 --- a/contracts/mocks/ERC721GSNRecipientMock.sol +++ b/contracts/mocks/ERC721GSNRecipientMock.sol @@ -9,7 +9,11 @@ import "../GSN/GSNRecipientSignature.sol"; * A simple ERC721 mock that has GSN support enabled */ contract ERC721GSNRecipientMock is ERC721, GSNRecipient, GSNRecipientSignature { - constructor(address trustedSigner) public GSNRecipientSignature(trustedSigner) { } + constructor(string memory name, string memory symbol, address trustedSigner) + public + ERC721(name, symbol) + GSNRecipientSignature(trustedSigner) + { } function mint(uint256 tokenId) public { _mint(_msgSender(), tokenId); diff --git a/contracts/mocks/ERC721Mock.sol b/contracts/mocks/ERC721Mock.sol index 00b8291169a..7290aba4976 100644 --- a/contracts/mocks/ERC721Mock.sol +++ b/contracts/mocks/ERC721Mock.sol @@ -7,18 +7,36 @@ import "../token/ERC721/ERC721.sol"; * This mock just provides a public safeMint, mint, and burn functions for testing purposes */ contract ERC721Mock is ERC721 { - function safeMint(address to, uint256 tokenId) public { - _safeMint(to, tokenId); + constructor (string memory name, string memory symbol) public ERC721(name, symbol) { } + + function exists(uint256 tokenId) public view returns (bool) { + return _exists(tokenId); } - function safeMint(address to, uint256 tokenId, bytes memory _data) public { - _safeMint(to, tokenId, _data); + function tokensOfOwner(address owner) public view returns (uint256[] memory) { + return _tokensOfOwner(owner); + } + + function setTokenURI(uint256 tokenId, string memory uri) public { + _setTokenURI(tokenId, uri); + } + + function setBaseURI(string memory baseURI) public { + _setBaseURI(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); } diff --git a/contracts/mocks/ERC721PausableMock.sol b/contracts/mocks/ERC721PausableMock.sol index 5d845c95b18..5c4e0e8f766 100644 --- a/contracts/mocks/ERC721PausableMock.sol +++ b/contracts/mocks/ERC721PausableMock.sol @@ -7,6 +7,8 @@ import "../token/ERC721/ERC721Pausable.sol"; * This mock just provides a public mint, burn and exists functions for testing purposes */ contract ERC721PausableMock is ERC721Pausable { + constructor (string memory name, string memory symbol) public ERC721Pausable(name, symbol) { } + function mint(address to, uint256 tokenId) public { super._mint(to, tokenId); } From 5f15521458e7d93f2847da505249931f3f6d55fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 26 Mar 2020 13:56:14 -0300 Subject: [PATCH 6/8] Update tests --- test/GSN/ERC721GSNRecipientMock.test.js | 4 +- test/token/ERC721/ERC721.behavior.js | 628 ---------------- test/token/ERC721/ERC721.test.js | 884 ++++++++++++++++++++++- test/token/ERC721/ERC721Burnable.test.js | 5 +- test/token/ERC721/ERC721Full.test.js | 249 ------- test/token/ERC721/ERC721Holder.test.js | 5 +- test/token/ERC721/ERC721Pausable.test.js | 11 +- 7 files changed, 863 insertions(+), 923 deletions(-) delete mode 100644 test/token/ERC721/ERC721.behavior.js delete mode 100644 test/token/ERC721/ERC721Full.test.js diff --git a/test/GSN/ERC721GSNRecipientMock.test.js b/test/GSN/ERC721GSNRecipientMock.test.js index c89cf5c320a..90a0ef50eef 100644 --- a/test/GSN/ERC721GSNRecipientMock.test.js +++ b/test/GSN/ERC721GSNRecipientMock.test.js @@ -11,10 +11,12 @@ const ERC721GSNRecipientMock = contract.fromArtifact('ERC721GSNRecipientMock'); describe('ERC721GSNRecipient (integration)', function () { const [ signer, sender ] = accounts; + const name = 'Non Fungible Token'; + const symbol = 'NFT'; const tokenId = '42'; beforeEach(async function () { - this.token = await ERC721GSNRecipientMock.new(signer); + this.token = await ERC721GSNRecipientMock.new(name, symbol, signer); }); async function testMintToken (token, from, tokenId, options = {}) { diff --git a/test/token/ERC721/ERC721.behavior.js b/test/token/ERC721/ERC721.behavior.js deleted file mode 100644 index e6ef75f440d..00000000000 --- a/test/token/ERC721/ERC721.behavior.js +++ /dev/null @@ -1,628 +0,0 @@ -const { contract } = require('@openzeppelin/test-environment'); -const { BN, constants, expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); -const { expect } = require('chai'); -const { ZERO_ADDRESS } = constants; -const { shouldSupportInterfaces } = require('../../introspection/SupportsInterface.behavior'); - -const ERC721Mock = contract.fromArtifact('ERC721Mock'); -const ERC721ReceiverMock = contract.fromArtifact('ERC721ReceiverMock'); - -function shouldBehaveLikeERC721 ( - [owner, approved, anotherApproved, operator, other] -) { - const firstTokenId = new BN(1); - const secondTokenId = new BN(2); - const unknownTokenId = new BN(3); - const RECEIVER_MAGIC_VALUE = '0x150b7a02'; - - describe('like an ERC721', function () { - beforeEach(async function () { - await this.token.mint(owner, firstTokenId); - await this.token.mint(owner, secondTokenId); - this.toWhom = other; // default to anyone 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 = unknownTokenId; - - 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('clears the approval for the token ID', async function () { - expect(await this.token.getApproved(tokenId)).to.be.equal(ZERO_ADDRESS); - }); - - if (approved) { - it('emit only a transfer event', async function () { - expectEvent.inLogs(logs, 'Transfer', { - from: owner, - to: this.toWhom, - tokenId: tokenId, - }); - }); - } else { - it('emits only a transfer event', async function () { - expectEvent.inLogs(logs, 'Transfer', { - from: owner, - to: this.toWhom, - 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, unknownTokenId, { 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, false); - this.toWhom = this.receiver.address; - }); - - shouldTransferTokensByUsers(transferFun); - - it('should call 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('should call 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, - unknownTokenId, - { 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', false); - await expectRevert( - this.token.safeTransferFrom(owner, invalidReceiver.address, tokenId, { from: owner }), - 'ERC721: transfer to non ERC721Receiver implementer' - ); - }); - }); - - describe('to a receiver contract that throws', function () { - it('reverts', async function () { - const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, true); - await expectRevert( - this.token.safeTransferFrom(owner, revertingReceiver.address, tokenId, { from: owner }), - 'ERC721ReceiverMock: reverting' - ); - }); - }); - - 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'; - - beforeEach(async function () { - this.ERC721Mock = await ERC721Mock.new(); - }); - - describe('via safeMint', function () { // regular minting is tested in ERC721Mintable.test.js and others - it('should call onERC721Received — with data', async function () { - this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, false); - const receipt = await this.ERC721Mock.safeMint(this.receiver.address, tokenId, data); - - await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', { - from: ZERO_ADDRESS, - tokenId: tokenId, - data: data, - }); - }); - - it('should call onERC721Received — without data', async function () { - this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, false); - const receipt = await this.ERC721Mock.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', false); - await expectRevert( - this.ERC721Mock.safeMint(invalidReceiver.address, tokenId), - 'ERC721: transfer to non ERC721Receiver implementer' - ); - }); - }); - - context('to a receiver contract that throws', function () { - it('reverts', async function () { - const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, true); - await expectRevert( - this.ERC721Mock.safeMint(revertingReceiver.address, tokenId), - 'ERC721ReceiverMock: reverting' - ); - }); - }); - - context('to a contract that does not implement the required function', function () { - it('reverts', async function () { - const nonReceiver = this.token; - await expectRevert( - this.ERC721Mock.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, unknownTokenId, { 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(unknownTokenId), - '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('should return approved account', async function () { - expect(await this.token.getApproved(firstTokenId)).to.be.equal(approved); - }); - }); - }); - }); - - shouldSupportInterfaces([ - 'ERC165', - 'ERC721', - ]); - }); -} - -module.exports = { - shouldBehaveLikeERC721, -}; diff --git a/test/token/ERC721/ERC721.test.js b/test/token/ERC721/ERC721.test.js index 980f17322fb..eb156da3ff2 100644 --- a/test/token/ERC721/ERC721.test.js +++ b/test/token/ERC721/ERC721.test.js @@ -5,82 +5,894 @@ const { ZERO_ADDRESS } = constants; const { expect } = require('chai'); -const { shouldBehaveLikeERC721 } = require('./ERC721.behavior'); +const { shouldSupportInterfaces } = require('../../introspection/SupportsInterface.behavior'); + const ERC721Mock = contract.fromArtifact('ERC721Mock'); +const ERC721ReceiverMock = contract.fromArtifact('ERC721ReceiverMock'); describe('ERC721', function () { - const [ owner ] = 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(); + this.token = await ERC721Mock.new(name, symbol); }); - shouldBehaveLikeERC721(accounts); + 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'; - describe('internal functions', function () { - const tokenId = new BN('5042'); + it('it is empty by default', async function () { + expect(await this.token.tokenURI(firstTokenId)).to.be.equal(''); + }); - describe('_mint(address, uint256)', function () { - it('reverts with a null destination address', async function () { + it('reverts when queried for non existent token id', async function () { await expectRevert( - this.token.mint(ZERO_ADDRESS, tokenId), 'ERC721: mint to the zero address' + this.token.tokenURI(nonExistentTokenId), 'ERC721Metadata: URI query for nonexistent token' ); }); - context('with minted token', async function () { - beforeEach(async function () { - ({ logs: this.logs } = await this.token.mint(owner, tokenId)); + 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('token URI is empty for tokens with no URI but with base URI', async function () { + await this.token.setBaseURI(baseURI); + + expect(await this.token.tokenURI(firstTokenId)).to.be.equal(''); + }); + + 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'); }); + }); - it('emits a Transfer event', function () { - expectEvent.inLogs(this.logs, 'Transfer', { from: ZERO_ADDRESS, to: owner, tokenId }); + 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'); }); + }); - it('creates the token', async function () { - expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('1'); - expect(await this.token.ownerOf(tokenId)).to.equal(owner); + 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('reverts when adding a token id that already exists', async function () { - await expectRevert(this.token.mint(owner, tokenId), 'ERC721: token already minted'); + 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('_burn', function () { - it('reverts when burning a non-existent token id', async function () { - await expectRevert( - this.token.burn(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 }); }); - context('with minted token', function () { - beforeEach(async function () { - await this.token.mint(owner, tokenId); + 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('clears the approval for the token ID', async function () { + expect(await this.token.getApproved(tokenId)).to.be.equal(ZERO_ADDRESS); }); - context('with burnt token', function () { + if (approved) { + it('emit only a transfer event', async function () { + expectEvent.inLogs(logs, 'Transfer', { + from: owner, + to: this.toWhom, + tokenId: tokenId, + }); + }); + } else { + it('emits only a transfer event', async function () { + expectEvent.inLogs(logs, 'Transfer', { + from: owner, + to: this.toWhom, + 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: this.logs } = await this.token.burn(tokenId)); + ({ logs } = await transferFunction.call(this, owner, this.toWhom, tokenId, { from: owner })); }); + transferWasSuccessful({ owner, tokenId, approved }); + }); - it('emits a Transfer event', function () { - expectEvent.inLogs(this.logs, 'Transfer', { from: owner, to: ZERO_ADDRESS, tokenId }); + 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 }); + }); - it('deletes the token', async function () { - expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('0'); + 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, false); + this.toWhom = this.receiver.address; + }); + + shouldTransferTokensByUsers(transferFun); + + it('should call 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('should call 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', false); await expectRevert( - this.token.ownerOf(tokenId), 'ERC721: owner query for nonexistent token' + this.token.safeTransferFrom(owner, invalidReceiver.address, tokenId, { from: owner }), + 'ERC721: transfer to non ERC721Receiver implementer' ); }); + }); - it('reverts when burning a token id that has been deleted', async function () { + describe('to a receiver contract that throws', function () { + it('reverts', async function () { + const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, true); await expectRevert( - this.token.burn(tokenId), 'ERC721: owner query for nonexistent token' + this.token.safeTransferFrom(owner, revertingReceiver.address, tokenId, { from: owner }), + 'ERC721ReceiverMock: reverting' ); }); }); + + 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('should call onERC721Received — with data', async function () { + this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, false); + 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('should call onERC721Received — without data', async function () { + this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, false); + 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', false); + await expectRevert( + this.token.safeMint(invalidReceiver.address, tokenId), + 'ERC721: transfer to non ERC721Receiver implementer' + ); + }); + }); + + context('to a receiver contract that throws', function () { + it('reverts', async function () { + const revertingReceiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, true); + await expectRevert( + this.token.safeMint(revertingReceiver.address, tokenId), + 'ERC721ReceiverMock: reverting' + ); + }); + }); + + 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('should return approved account', async function () { + expect(await this.token.getApproved(firstTokenId)).to.be.equal(approved); + }); + }); + }); + }); + + describe('tokensOfOwner', function () { + it('returns total tokens of owner', async function () { + const tokenIds = await this.token.tokensOfOwner(owner); + expect(tokenIds.length).to.equal(2); + expect(tokenIds[0]).to.be.bignumber.equal(firstTokenId); + expect(tokenIds[1]).to.be.bignumber.equal(secondTokenId); + }); + }); + + 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('should return 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('should revert 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(`should return 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, { from: owner }); + 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('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), 'ERC721Enumerable: global 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' + ); + }); }); }); }); diff --git a/test/token/ERC721/ERC721Burnable.test.js b/test/token/ERC721/ERC721Burnable.test.js index 456d4171560..39bac288c26 100644 --- a/test/token/ERC721/ERC721Burnable.test.js +++ b/test/token/ERC721/ERC721Burnable.test.js @@ -14,8 +14,11 @@ describe('ERC721Burnable', function () { const secondTokenId = new BN(2); const unknownTokenId = new BN(3); + const name = 'Non Fungible Token'; + const symbol = 'NFT'; + beforeEach(async function () { - this.token = await ERC721BurnableMock.new(); + this.token = await ERC721BurnableMock.new(name, symbol); }); describe('like a burnable ERC721', function () { diff --git a/test/token/ERC721/ERC721Full.test.js b/test/token/ERC721/ERC721Full.test.js deleted file mode 100644 index e7473d02072..00000000000 --- a/test/token/ERC721/ERC721Full.test.js +++ /dev/null @@ -1,249 +0,0 @@ -const { accounts, contract } = require('@openzeppelin/test-environment'); - -const { BN, expectRevert } = require('@openzeppelin/test-helpers'); -const { expect } = require('chai'); - -const { shouldBehaveLikeERC721 } = require('./ERC721.behavior'); -const { shouldSupportInterfaces } = require('../../introspection/SupportsInterface.behavior'); - -const ERC721FullMock = contract.fromArtifact('ERC721FullMock'); - -describe('ERC721Full', function () { - const [owner, newOwner, other ] = accounts; - - const name = 'Non Fungible Token'; - const symbol = 'NFT'; - const firstTokenId = new BN(100); - const secondTokenId = new BN(200); - const thirdTokenId = new BN(300); - const nonExistentTokenId = new BN(999); - - beforeEach(async function () { - this.token = await ERC721FullMock.new(name, symbol); - }); - - describe('like a full ERC721', function () { - beforeEach(async function () { - await this.token.mint(owner, firstTokenId); - await this.token.mint(owner, secondTokenId); - }); - - describe('mint', function () { - beforeEach(async function () { - await this.token.mint(newOwner, thirdTokenId); - }); - - it('adjusts owner tokens by index', async function () { - expect(await this.token.tokenOfOwnerByIndex(newOwner, 0)).to.be.bignumber.equal(thirdTokenId); - }); - - it('adjusts all tokens list', async function () { - expect(await this.token.tokenByIndex(2)).to.be.bignumber.equal(thirdTokenId); - }); - }); - - describe('burn', function () { - beforeEach(async function () { - await this.token.burn(firstTokenId, { from: owner }); - }); - - 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' - ); - }); - }); - - 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 () { - 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('token URI is empty for tokens with no URI but with base URI', async function () { - await this.token.setBaseURI(baseURI); - - expect(await this.token.tokenURI(firstTokenId)).to.be.equal(''); - }); - - 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' - ); - }); - }); - }); - - describe('tokensOfOwner', function () { - it('returns total tokens of owner', async function () { - const tokenIds = await this.token.tokensOfOwner(owner); - expect(tokenIds.length).to.equal(2); - expect(tokenIds[0]).to.be.bignumber.equal(firstTokenId); - expect(tokenIds[1]).to.be.bignumber.equal(secondTokenId); - }); - }); - - 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('should return 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('should revert 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(`should return 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, { from: owner }); - 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())); - }); - }); - }); - }); - - shouldBehaveLikeERC721(accounts); - - shouldSupportInterfaces([ - 'ERC165', - 'ERC721', - 'ERC721Enumerable', - 'ERC721Metadata', - ]); -}); diff --git a/test/token/ERC721/ERC721Holder.test.js b/test/token/ERC721/ERC721Holder.test.js index b2d1017c530..174ef8f468b 100644 --- a/test/token/ERC721/ERC721Holder.test.js +++ b/test/token/ERC721/ERC721Holder.test.js @@ -10,8 +10,11 @@ const ERC721Mock = contract.fromArtifact('ERC721Mock'); describe('ERC721Holder', function () { const [ owner ] = accounts; + const name = 'Non Fungible Token'; + const symbol = 'NFT'; + it('receives an ERC721 token', async function () { - const token = await ERC721Mock.new(); + const token = await ERC721Mock.new(name, symbol); const tokenId = new BN(1); await token.mint(owner, tokenId); diff --git a/test/token/ERC721/ERC721Pausable.test.js b/test/token/ERC721/ERC721Pausable.test.js index 16f5b8421bf..a1586284740 100644 --- a/test/token/ERC721/ERC721Pausable.test.js +++ b/test/token/ERC721/ERC721Pausable.test.js @@ -5,19 +5,16 @@ const { ZERO_ADDRESS } = constants; const { expect } = require('chai'); -const { shouldBehaveLikeERC721 } = require('./ERC721.behavior'); - const ERC721PausableMock = contract.fromArtifact('ERC721PausableMock'); describe('ERC721Pausable', function () { const [ owner, receiver, operator ] = accounts; - beforeEach(async function () { - this.token = await ERC721PausableMock.new(); - }); + const name = 'Non Fungible Token'; + const symbol = 'NFT'; - context('when token is not paused yet', function () { - shouldBehaveLikeERC721(accounts); + beforeEach(async function () { + this.token = await ERC721PausableMock.new(name, symbol); }); context('when token is paused', function () { From ddf2785ce67a43eaf3b1d6235e9964d70f68aefc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Fri, 27 Mar 2020 13:20:53 -0300 Subject: [PATCH 7/8] Update contracts/token/ERC721/ERC721.sol Co-Authored-By: Francisco Giordano --- contracts/token/ERC721/ERC721.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index cabbb17927c..0a39c084b98 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -457,8 +457,8 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable * * Reverts if the token ID does not exist. * - * TIP: if all token IDs share a prefix (e.g. if your URIs look like - * `http://api.myproject.com/token/`), use {_setBaseURI} to store + * TIP: If all token IDs share a prefix (for example, if your URIs look like + * `https://api.myproject.com/token/`), use {_setBaseURI} to store * it and save gas. */ function _setTokenURI(uint256 tokenId, string memory _tokenURI) internal virtual { From 91e9b03f8f2fa07c76d92851811f3adf1c8fcc67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Fri, 27 Mar 2020 16:11:50 -0300 Subject: [PATCH 8/8] Make ERC721Pausable and ERC721Burnable abstract --- contracts/mocks/ERC721BurnableMock.sol | 2 +- contracts/mocks/ERC721PausableMock.sol | 2 +- contracts/token/ERC721/ERC721Burnable.sol | 4 +--- contracts/token/ERC721/ERC721Pausable.sol | 4 +--- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/contracts/mocks/ERC721BurnableMock.sol b/contracts/mocks/ERC721BurnableMock.sol index 67d5c3babac..e2a20df5584 100644 --- a/contracts/mocks/ERC721BurnableMock.sol +++ b/contracts/mocks/ERC721BurnableMock.sol @@ -3,7 +3,7 @@ pragma solidity ^0.6.0; import "../token/ERC721/ERC721Burnable.sol"; contract ERC721BurnableMock is ERC721Burnable { - constructor(string memory name, string memory symbol) public ERC721Burnable(name, symbol) { } + constructor(string memory name, string memory symbol) public ERC721(name, symbol) { } function mint(address to, uint256 tokenId) public { _mint(to, tokenId); diff --git a/contracts/mocks/ERC721PausableMock.sol b/contracts/mocks/ERC721PausableMock.sol index 5c4e0e8f766..6977cddb613 100644 --- a/contracts/mocks/ERC721PausableMock.sol +++ b/contracts/mocks/ERC721PausableMock.sol @@ -7,7 +7,7 @@ import "../token/ERC721/ERC721Pausable.sol"; * This mock just provides a public mint, burn and exists functions for testing purposes */ contract ERC721PausableMock is ERC721Pausable { - constructor (string memory name, string memory symbol) public ERC721Pausable(name, symbol) { } + constructor (string memory name, string memory symbol) public ERC721(name, symbol) { } function mint(address to, uint256 tokenId) public { super._mint(to, tokenId); diff --git a/contracts/token/ERC721/ERC721Burnable.sol b/contracts/token/ERC721/ERC721Burnable.sol index 16de824490f..6cee17d5817 100644 --- a/contracts/token/ERC721/ERC721Burnable.sol +++ b/contracts/token/ERC721/ERC721Burnable.sol @@ -7,9 +7,7 @@ import "./ERC721.sol"; * @title ERC721 Burnable Token * @dev ERC721 Token that can be irreversibly burned (destroyed). */ -contract ERC721Burnable is Context, ERC721 { - constructor (string memory name, string memory symbol) public ERC721(name, symbol) { } - +abstract contract ERC721Burnable is Context, ERC721 { /** * @dev Burns a specific ERC721 token. * @param tokenId uint256 id of the ERC721 token to be burned. diff --git a/contracts/token/ERC721/ERC721Pausable.sol b/contracts/token/ERC721/ERC721Pausable.sol index a531cbddd18..50da11c375b 100644 --- a/contracts/token/ERC721/ERC721Pausable.sol +++ b/contracts/token/ERC721/ERC721Pausable.sol @@ -7,9 +7,7 @@ import "../../utils/Pausable.sol"; * @title ERC721 Non-Fungible Pausable token * @dev ERC721 modified with pausable transfers. */ -contract ERC721Pausable is ERC721, Pausable { - constructor (string memory name, string memory symbol) public ERC721(name, symbol) { } - +abstract contract ERC721Pausable is ERC721, Pausable { function _beforeTokenTransfer(address from, address to, uint256 tokenId) internal virtual override { super._beforeTokenTransfer(from, to, tokenId);