Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove enumerable from ERC721 and add an ERC721Enumerable extension #2511

Merged
merged 30 commits into from
Feb 19, 2021
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
c330fcd
Add a light variant of ERC721 (non enumerable)
Amxx Feb 3, 2021
da49687
separate metadata from ERC721 and fix behavior test
Amxx Feb 3, 2021
fb9877e
make ERC721 non enumerable and add a ERC721Enumerable
Amxx Feb 3, 2021
0fe0c01
remove old comment
Amxx Feb 3, 2021
53da911
fix lint
Amxx Feb 5, 2021
0383c05
fix test
Amxx Feb 5, 2021
b1366d6
Merge branch 'master' into feature/ERC721Light
Amxx Feb 10, 2021
a8e3db7
erc721 enumerability on using hook and erc721 data
Amxx Feb 12, 2021
44b7863
use mapping instead of array in ERC721enumerable to avoid double sstore
Amxx Feb 12, 2021
b89a7de
Merge branch 'master' into feature/ERC721Light
Amxx Feb 17, 2021
2e27c98
merge ERC721Metadata back into ERC721 (basic version without storage)
Amxx Feb 17, 2021
0f8301d
Merge branch 'feature/ERC721Light' of github.com:Amxx/openzeppelin-co…
Amxx Feb 17, 2021
2619bb1
documentation of _baseUri
Amxx Feb 17, 2021
1c76dd3
fix gsn test
Amxx Feb 17, 2021
261e750
fix preset test
Amxx Feb 17, 2021
ce3017f
add changelog entry
Amxx Feb 18, 2021
bee22d0
Merge remote-tracking branch 'upstream/master' into feature/ERC721Light
frangio Feb 18, 2021
7e36934
fix merge errors
frangio Feb 18, 2021
08183d1
Update CHANGELOG.md
Amxx Feb 19, 2021
c136072
Update contracts/token/ERC721/ERC721.sol
Amxx Feb 19, 2021
1e69ce6
Update contracts/token/ERC721/ERC721.sol
Amxx Feb 19, 2021
e1b0490
address issues raised in PR
Amxx Feb 19, 2021
5cab2bc
Merge remote-tracking branch 'Amxx/feature/ERC721Light' into feature/…
Amxx Feb 19, 2021
0313e5f
test baseURI/tokenURI mechanisms in shouldBehaveLikeERC721Metadata
Amxx Feb 19, 2021
d56cdcb
setBaseURI tests aware of target contract
Amxx Feb 19, 2021
3a0323e
Merge branch 'master' into feature/ERC721Light
Amxx Feb 19, 2021
c39ef2d
remove unsupported ?? operator
frangio Feb 19, 2021
7d74a7a
fix optimization to get string once
frangio Feb 19, 2021
ced1689
add ERC721Enumerable to docs site
frangio Feb 19, 2021
325e471
add docs for ERC721Enumerable
frangio Feb 19, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* `EnumerableMap`: change implementation to optimize for `key → value` lookups instead of enumeration. ([#2518](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2518))
* `GSN`: Deprecate GSNv1 support in favor of upcomming support for GSNv2. ([#2521](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2521))
* `ERC165`: Remove uses of storage in the base ERC165 implementation. ERC165 based contracts now use storage-less virtual functions. Old behaviour remains available in the `ERC165Storage` extension. ([#2505](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2505))
* `ERC721`: remove enumerability of tokens from de base implementation. This feature is now provided through the `ERC721Enumerable` extension. ([#2511](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2511))
Amxx marked this conversation as resolved.
Show resolved Hide resolved

## 3.4.0 (2021-02-02)

Expand Down
33 changes: 33 additions & 0 deletions contracts/mocks/ERC721EnumerableMock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../token/ERC721/ERC721Enumerable.sol";

/**
* @title ERC721Mock
* This mock just provides a public safeMint, mint, and burn functions for testing purposes
*/
contract ERC721EnumerableMock is ERC721Enumerable {
constructor(string memory name, string memory symbol) ERC721(name, symbol) { }

function exists(uint256 tokenId) public view returns (bool) {
return _exists(tokenId);
}

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);
}
}
8 changes: 0 additions & 8 deletions contracts/mocks/ERC721Mock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,6 @@ contract ERC721Mock is ERC721 {
return _exists(tokenId);
}

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);
}
Expand Down
22 changes: 18 additions & 4 deletions contracts/presets/ERC721PresetMinterPauserAutoId.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import "../access/AccessControl.sol";
import "../utils/Context.sol";
import "../utils/Counters.sol";
import "../token/ERC721/ERC721.sol";
import "../token/ERC721/ERC721Enumerable.sol";
import "../token/ERC721/ERC721Burnable.sol";
import "../token/ERC721/ERC721Pausable.sol";

Expand All @@ -24,28 +25,34 @@ import "../token/ERC721/ERC721Pausable.sol";
* roles, as well as the default admin role, which will let it grant both minter
* and pauser roles to other accounts.
*/
contract ERC721PresetMinterPauserAutoId is Context, AccessControl, ERC721Burnable, ERC721Pausable {
contract ERC721PresetMinterPauserAutoId is Context, AccessControl, ERC721Enumerable, ERC721Burnable, ERC721Pausable {
using Counters for Counters.Counter;

bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");

Counters.Counter private _tokenIdTracker;

string private _baseTokenURI;

/**
* @dev Grants `DEFAULT_ADMIN_ROLE`, `MINTER_ROLE` and `PAUSER_ROLE` to the
* account that deploys the contract.
*
* Token URIs will be autogenerated based on `baseURI` and their token IDs.
* See {ERC721-tokenURI}.
*/
constructor(string memory name, string memory symbol, string memory baseURI) ERC721(name, symbol) {
constructor(string memory name, string memory symbol, string memory baseTokenURI) ERC721(name, symbol) {
_baseTokenURI = baseTokenURI;

_setupRole(DEFAULT_ADMIN_ROLE, _msgSender());

_setupRole(MINTER_ROLE, _msgSender());
_setupRole(PAUSER_ROLE, _msgSender());
}

_setBaseURI(baseURI);
function _baseURI() internal view virtual override returns (string memory) {
return _baseTokenURI;
}

/**
Expand Down Expand Up @@ -96,7 +103,14 @@ contract ERC721PresetMinterPauserAutoId is Context, AccessControl, ERC721Burnabl
_unpause();
}

function _beforeTokenTransfer(address from, address to, uint256 tokenId) internal virtual override(ERC721, ERC721Pausable) {
function _beforeTokenTransfer(address from, address to, uint256 tokenId) internal virtual override(ERC721, ERC721Enumerable, ERC721Pausable) {
super._beforeTokenTransfer(from, to, tokenId);
}

/**
* @dev See {IERC165-supportsInterface}.
*/
function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721, ERC721Enumerable) returns (bool) {
return super.supportsInterface(interfaceId);
}
}
143 changes: 36 additions & 107 deletions contracts/token/ERC721/ERC721.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,47 +5,36 @@ pragma solidity ^0.8.0;
import "../../utils/Context.sol";
import "./IERC721.sol";
import "./IERC721Metadata.sol";
import "./IERC721Enumerable.sol";
import "./IERC721Receiver.sol";
import "../../introspection/ERC165.sol";
import "../../utils/Address.sol";
import "../../utils/EnumerableSet.sol";
import "../../utils/EnumerableMap.sol";
import "../../utils/Strings.sol";

/**
* @title ERC721 Non-Fungible Token Standard basic implementation
* @title ERC721 Non-Fungible Token Standard enumerable implementation
* @dev see https://eips.ethereum.org/EIPS/eip-721
Amxx marked this conversation as resolved.
Show resolved Hide resolved
*/
contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable {
contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {
using Address for address;
using EnumerableSet for EnumerableSet.UintSet;
using EnumerableMap for EnumerableMap.UintToAddressMap;
using Strings for uint256;

// Mapping from holder address to their (enumerable) set of owned tokens
mapping (address => EnumerableSet.UintSet) private _holderTokens;

// Enumerable mapping from token ids to their owners
EnumerableMap.UintToAddressMap private _tokenOwners;

// Mapping from token ID to approved address
mapping (uint256 => address) private _tokenApprovals;

// Mapping from owner to operator approvals
mapping (address => mapping (address => bool)) private _operatorApprovals;

// Token name
string private _name;

// Token symbol
string private _symbol;

// Optional mapping for token URIs
mapping (uint256 => string) private _tokenURIs;
// Mapping from token ID to owner address
mapping (uint256 => address) private _owners;

// Mapping owner address to token count
mapping (address => uint256) private _balances;

// Base URI
string private _baseURI;
// Mapping from token ID to approved address
mapping (uint256 => address) private _tokenApprovals;

// Mapping from owner to operator approvals
mapping (address => mapping (address => bool)) private _operatorApprovals;

/**
* @dev Initializes the contract by setting a `name` and a `symbol` to the token collection.
Expand All @@ -61,7 +50,6 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) {
return interfaceId == type(IERC721).interfaceId
|| interfaceId == type(IERC721Metadata).interfaceId
|| interfaceId == type(IERC721Enumerable).interfaceId
|| super.supportsInterface(interfaceId);
}

Expand All @@ -70,14 +58,16 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
*/
function balanceOf(address owner) public view virtual override returns (uint256) {
require(owner != address(0), "ERC721: balance query for the zero address");
return _holderTokens[owner].length();
return _balances[owner];
}

/**
* @dev See {IERC721-ownerOf}.
*/
function ownerOf(uint256 tokenId) public view virtual override returns (address) {
return _tokenOwners.get(tokenId, "ERC721: owner query for nonexistent token");
address owner = _owners[tokenId];
require(owner != address(0), "ERC721: owner query for nonexistent token");
return owner;
}

/**
Expand All @@ -99,52 +89,15 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
*/
function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
require(_exists(tokenId), "ERC721Metadata: URI query for nonexistent token");

string memory _tokenURI = _tokenURIs[tokenId];
string memory base = baseURI();

// If there is no base URI, return the token URI.
if (bytes(base).length == 0) {
return _tokenURI;
}
// If both are set, concatenate the baseURI and tokenURI (via abi.encodePacked).
if (bytes(_tokenURI).length > 0) {
return string(abi.encodePacked(base, _tokenURI));
}
// If there is a baseURI but no tokenURI, concatenate the tokenID to the baseURI.
return string(abi.encodePacked(base, tokenId.toString()));
}

/**
* @dev Returns the base URI set via {_setBaseURI}. This will be
* automatically added as a prefix in {tokenURI} to each token's URI, or
* to the token ID if no specific URI is set for that token ID.
*/
function baseURI() public view virtual returns (string memory) {
return _baseURI;
}

/**
* @dev See {IERC721Enumerable-tokenOfOwnerByIndex}.
*/
function tokenOfOwnerByIndex(address owner, uint256 index) public view virtual override returns (uint256) {
return _holderTokens[owner].at(index);
}

/**
* @dev See {IERC721Enumerable-totalSupply}.
*/
function totalSupply() public view virtual override returns (uint256) {
// _tokenOwners are indexed by tokenIds, so .length() returns the number of tokenIds
return _tokenOwners.length();
return string(abi.encodePacked(_baseURI(), tokenId.toString()));
Amxx marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* @dev See {IERC721Enumerable-tokenByIndex}.
* @dev Base URI for computing tokenURI. Empty by default, can be overriden
Amxx marked this conversation as resolved.
Show resolved Hide resolved
* in child contracts.
*/
function tokenByIndex(uint256 index) public view virtual override returns (uint256) {
(uint256 tokenId, ) = _tokenOwners.at(index);
return tokenId;
function _baseURI() internal view virtual returns (string memory) {
return "";
}

/**
Expand Down Expand Up @@ -244,7 +197,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
* and stop existing when they are burned (`_burn`).
*/
function _exists(uint256 tokenId) internal view virtual returns (bool) {
return _tokenOwners.contains(tokenId);
return _owners[tokenId] != address(0);
}

/**
Expand Down Expand Up @@ -301,9 +254,8 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable

_beforeTokenTransfer(address(0), to, tokenId);

_holderTokens[to].add(tokenId);

_tokenOwners.set(tokenId, to);
_balances[to] += 1;
_owners[tokenId] = to;

emit Transfer(address(0), to, tokenId);
}
Expand All @@ -319,21 +271,15 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
* Emits a {Transfer} event.
*/
function _burn(uint256 tokenId) internal virtual {
address owner = ERC721.ownerOf(tokenId); // internal owner
address owner = ERC721.ownerOf(tokenId);

_beforeTokenTransfer(owner, address(0), tokenId);

// Clear approvals
_approve(address(0), tokenId);

// Clear metadata (if any)
if (bytes(_tokenURIs[tokenId]).length != 0) {
delete _tokenURIs[tokenId];
}

_holderTokens[owner].remove(tokenId);

_tokenOwners.remove(tokenId);
_balances[owner] -= 1;
delete _owners[tokenId];

emit Transfer(owner, address(0), tokenId);
}
Expand All @@ -350,41 +296,29 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
* Emits a {Transfer} event.
*/
function _transfer(address from, address to, uint256 tokenId) internal virtual {
require(ERC721.ownerOf(tokenId) == from, "ERC721: transfer of token that is not own"); // internal owner
require(ERC721.ownerOf(tokenId) == from, "ERC721: transfer of token that is not own");
require(to != address(0), "ERC721: transfer to the zero address");

_beforeTokenTransfer(from, to, tokenId);

// Clear approvals from the previous owner
_approve(address(0), tokenId);

_holderTokens[from].remove(tokenId);
_holderTokens[to].add(tokenId);

_tokenOwners.set(tokenId, to);
_balances[from] -= 1;
_balances[to] += 1;
_owners[tokenId] = to;

emit Transfer(from, to, tokenId);
}

/**
* @dev Sets `_tokenURI` as the tokenURI of `tokenId`.
* @dev Approve `to` to operate on `tokenId`
*
* Requirements:
*
* - `tokenId` must exist.
* Emits a {Approval} event.
*/
function _setTokenURI(uint256 tokenId, string memory _tokenURI) internal virtual {
require(_exists(tokenId), "ERC721Metadata: URI set of nonexistent token");
_tokenURIs[tokenId] = _tokenURI;
}

/**
* @dev Internal function to set the base URI for all token IDs. It is
* automatically added as a prefix to the value returned in {tokenURI},
* or to the token ID if {tokenURI} is empty.
*/
function _setBaseURI(string memory baseURI_) internal virtual {
_baseURI = baseURI_;
function _approve(address to, uint256 tokenId) internal virtual {
_tokenApprovals[tokenId] = to;
emit Approval(ERC721.ownerOf(tokenId), to, tokenId);
}

/**
Expand Down Expand Up @@ -418,11 +352,6 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
}
}

function _approve(address to, uint256 tokenId) private {
_tokenApprovals[tokenId] = to;
emit Approval(ERC721.ownerOf(tokenId), to, tokenId); // internal owner
}

/**
* @dev Hook that is called before any token transfer. This includes minting
* and burning.
Expand Down
Loading