From ecf0725dd1bb37d0b14c54f558bcc6fa90348c96 Mon Sep 17 00:00:00 2001 From: GuiCz <115c7a5fac@pm.me> Date: Fri, 5 Jun 2020 19:29:06 +0200 Subject: [PATCH] Documentation/erc721 contracts (#2218) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Adds / Updates documentation of ERC721 contract * Improve ERC721Burnable documentation * Fix typo * Revert changes on ERC721 private functions * Add documentation to the ERC721 contract's constructor * Add IERC721Receiver & ERC721Holder documentations * Add references to IERC721 functions * Add references to IERC721Metadata/Receiver * PR fixes * Fixes to ERC721 documentation * Add missing fixes Co-authored-by: Nicolás Venturo --- contracts/token/ERC721/ERC721.sol | 237 ++++++++------------- contracts/token/ERC721/ERC721Burnable.sol | 7 +- contracts/token/ERC721/ERC721Holder.sol | 12 ++ contracts/token/ERC721/IERC721.sol | 9 +- contracts/token/ERC721/IERC721Receiver.sol | 19 +- 5 files changed, 119 insertions(+), 165 deletions(-) diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index 0e9653cef..4ad45f36f 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -87,6 +87,9 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable */ bytes4 private constant _INTERFACE_ID_ERC721_ENUMERABLE = 0x780e9d63; + /** + * @dev Initializes the contract by setting a `name` and a `symbol` to the token collection. + */ constructor (string memory name, string memory symbol) public { _name = name; _symbol = symbol; @@ -98,9 +101,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @dev Gets the balance of the specified address. - * @param owner address to query the balance of - * @return uint256 representing the amount owned by the passed address + * @dev See {IERC721-balanceOf}. */ function balanceOf(address owner) public view override returns (uint256) { require(owner != address(0), "ERC721: balance query for the zero address"); @@ -109,60 +110,28 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @dev Gets the owner of the specified token ID. - * @param tokenId uint256 ID of the token to query the owner of - * @return address currently marked as the owner of the given token ID + * @dev See {IERC721-ownerOf}. */ function ownerOf(uint256 tokenId) public view override returns (address) { return _tokenOwners.get(tokenId, "ERC721: owner query for nonexistent token"); } /** - * @dev Gets the token name. - * @return string representing the token name + * @dev See {IERC721Metadata-name}. */ function name() public view override returns (string memory) { return _name; } /** - * @dev Gets the token symbol. - * @return string representing the token symbol + * @dev See {IERC721Metadata-symbol}. */ function symbol() public view override returns (string memory) { return _symbol; } /** - * @dev Returns the URI for a given token ID. May return an empty string. - * - * If a base URI is set (via {_setBaseURI}), it is added as a prefix to the - * token's own URI (via {_setTokenURI}). - * - * If there is a base URI but no token URI, the token's ID will be used as - * its URI when appending it to the base URI. This pattern for autogenerated - * token URIs can lead to large gas savings. - * - * .Examples - * |=== - * |`_setBaseURI()` |`_setTokenURI()` |`tokenURI()` - * | "" - * | "" - * | "" - * | "" - * | "token.uri/123" - * | "token.uri/123" - * | "token.uri/" - * | "123" - * | "token.uri/123" - * | "token.uri/" - * | "" - * | "token.uri/" - * |=== - * - * Requirements: - * - * - `tokenId` must exist. + * @dev See {IERC721Metadata-tokenURI}. */ function tokenURI(uint256 tokenId) public view override returns (string memory) { require(_exists(tokenId), "ERC721Metadata: URI query for nonexistent token"); @@ -191,18 +160,14 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @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 + * @dev See {IERC721Enumerable-tokenOfOwnerByIndex}. */ function tokenOfOwnerByIndex(address owner, uint256 index) public view override returns (uint256) { return _holderTokens[owner].at(index); } /** - * @dev Gets the total amount of tokens stored by the contract. - * @return uint256 representing the total amount of tokens + * @dev See {IERC721Enumerable-totalSupply}. */ function totalSupply() public view override returns (uint256) { // _tokenOwners are indexed by tokenIds, so .length() returns the number of tokenIds @@ -210,10 +175,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @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 + * @dev See {IERC721Enumerable-tokenByIndex}. */ function tokenByIndex(uint256 index) public view override returns (uint256) { (uint256 tokenId, ) = _tokenOwners.at(index); @@ -221,12 +183,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @dev Approves another address to transfer the given token ID - * The zero address indicates there is no approved address. - * There can only be one approved address per token at a given time. - * Can only be called by the token owner or an approved operator. - * @param to address to be approved for the given token ID - * @param tokenId uint256 ID of the token to be approved + * @dev See {IERC721-approve}. */ function approve(address to, uint256 tokenId) public virtual override { address owner = ownerOf(tokenId); @@ -240,10 +197,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @dev Gets the approved address for a token ID, or zero if no address set - * Reverts if the token ID does not exist. - * @param tokenId uint256 ID of the token to query the approval of - * @return address currently approved for the given token ID + * @dev See {IERC721-getApproved}. */ function getApproved(uint256 tokenId) public view override returns (address) { require(_exists(tokenId), "ERC721: approved query for nonexistent token"); @@ -252,10 +206,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @dev Sets or unsets the approval of a given operator - * An operator is allowed to transfer all tokens of the sender on their behalf. - * @param operator operator address to set the approval - * @param approved representing the status of the approval to be set + * @dev See {IERC721-setApprovalForAll}. */ function setApprovalForAll(address operator, bool approved) public virtual override { require(operator != _msgSender(), "ERC721: approve to caller"); @@ -265,22 +216,14 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @dev Tells whether an operator is approved by a given owner. - * @param owner owner address which you want to query the approval of - * @param operator operator address which you want to query the approval of - * @return bool whether the given operator is approved by the given owner + * @dev See {IERC721-isApprovedForAll}. */ function isApprovedForAll(address owner, address operator) public view override returns (bool) { return _operatorApprovals[owner][operator]; } /** - * @dev Transfers the ownership of a given token ID to another address. - * Usage of this method is discouraged, use {safeTransferFrom} whenever possible. - * Requires the msg.sender to be the owner, approved, or operator. - * @param from current owner of the token - * @param to address to receive the ownership of the given token ID - * @param tokenId uint256 ID of the token to be transferred + * @dev See {IERC721-transferFrom}. */ function transferFrom(address from, address to, uint256 tokenId) public virtual override { //solhint-disable-next-line max-line-length @@ -290,31 +233,14 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @dev Safely transfers the ownership of a given token ID to another address - * If the target address is a contract, it must implement {IERC721Receiver-onERC721Received}, - * which is called upon a safe transfer, and return the magic value - * `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`; otherwise, - * the transfer is reverted. - * Requires the msg.sender to be the owner, approved, or operator - * @param from current owner of the token - * @param to address to receive the ownership of the given token ID - * @param tokenId uint256 ID of the token to be transferred + * @dev See {IERC721-safeTransferFrom}. */ function safeTransferFrom(address from, address to, uint256 tokenId) public virtual override { safeTransferFrom(from, to, tokenId, ""); } /** - * @dev Safely transfers the ownership of a given token ID to another address - * If the target address is a contract, it must implement {IERC721Receiver-onERC721Received}, - * which is called upon a safe transfer, and return the magic value - * `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`; otherwise, - * the transfer is reverted. - * Requires the _msgSender() to be the owner, approved, or operator - * @param from current owner of the token - * @param to address to receive the ownership of the given token ID - * @param tokenId uint256 ID of the token to be transferred - * @param _data bytes data to send along with a safe transfer check + * @dev See {IERC721-safeTransferFrom}. */ function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory _data) public virtual override { require(_isApprovedOrOwner(_msgSender(), tokenId), "ERC721: transfer caller is not owner nor approved"); @@ -322,16 +248,22 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @dev Safely transfers the ownership of a given token ID to another address - * If the target address is a contract, it must implement `onERC721Received`, - * which is called upon a safe transfer, and return the magic value - * `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`; otherwise, - * the transfer is reverted. - * Requires the msg.sender to be the owner, approved, or operator - * @param from current owner of the token - * @param to address to receive the ownership of the given token ID - * @param tokenId uint256 ID of the token to be transferred - * @param _data bytes data to send along with a safe transfer check + * @dev Safely transfers `tokenId` token from `from` to `to`, checking first that contract recipients + * are aware of the ERC721 protocol to prevent tokens from being forever locked. + * + * `_data` is additional data, it has no specified format and it is sent in call to `to`. + * + * This internal function is equivalent to {safeTransfertFrom}, and can be used to e.g. + * implement alternative mecanisms to perform token transfer, such as signature-based. + * + * Requirements: + * + * - `from` cannot be the zero address. + * - `to` cannot be the zero address. + * - `tokenId` token must exist and be owned by `from`. + * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon a safe transfer. + * + * Emits a {Transfer} event. */ function _safeTransfer(address from, address to, uint256 tokenId, bytes memory _data) internal virtual { _transfer(from, to, tokenId); @@ -339,20 +271,23 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @dev Returns whether the specified token exists. - * @param tokenId uint256 ID of the token to query the existence of - * @return bool whether the token exists + * @dev Returns whether `tokenId` exists. + * + * Tokens can be managed by their owner or approved accounts via {approve} or {setApprovalForAll}. + * + * Tokens start existing when they are minted (`_mint`), + * and stop existing when they are burned (`_burn`). */ function _exists(uint256 tokenId) internal view returns (bool) { return _tokenOwners.contains(tokenId); } /** - * @dev Returns whether the given spender can transfer a given token ID. - * @param spender address of the spender to query - * @param tokenId uint256 ID of the token to be transferred - * @return bool whether the msg.sender is approved for the given token ID, - * is an operator of the owner, or is the owner of the token + * @dev Returns whether `spender` is allowed to manage `tokenId`. + * + * Requirements: + * + * - `tokenId` must exist. */ function _isApprovedOrOwner(address spender, uint256 tokenId) internal view returns (bool) { require(_exists(tokenId), "ERC721: operator query for nonexistent token"); @@ -361,29 +296,21 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @dev Internal function to safely mint a new token. - * Reverts if the given token ID already exists. - * If the target address is a contract, it must implement `onERC721Received`, - * which is called upon a safe transfer, and return the magic value - * `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`; otherwise, - * the transfer is reverted. - * @param to The address that will own the minted token - * @param tokenId uint256 ID of the token to be minted + * @dev Safely mints `tokenId` and transfers it to `to`. + * + * Requirements: + d* + * - `tokenId` must not exist. + * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon a safe transfer. + * + * Emits a {Transfer} event. */ function _safeMint(address to, uint256 tokenId) internal virtual { _safeMint(to, tokenId, ""); } /** - * @dev Internal function to safely mint a new token. - * Reverts if the given token ID already exists. - * If the target address is a contract, it must implement `onERC721Received`, - * which is called upon a safe transfer, and return the magic value - * `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))`; otherwise, - * the transfer is reverted. - * @param to The address that will own the minted token - * @param tokenId uint256 ID of the token to be minted - * @param _data bytes data to send along with a safe transfer check + * @dev Same as {_safeMint-address-uint256-}, with an additional `data` parameter which is forwarded in {IERC721Receiver-onERC721Received} to contract recipients. */ function _safeMint(address to, uint256 tokenId, bytes memory _data) internal virtual { _mint(to, tokenId); @@ -391,10 +318,16 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @dev Internal function to mint a new token. - * Reverts if the given token ID already exists. - * @param to The address that will own the minted token - * @param tokenId uint256 ID of the token to be minted + * @dev Mints `tokenId` and transfers it to `to`. + * + * WARNING: Usage of this method is discouraged, use {_safeMint} whenever possible + * + * Requirements: + * + * - `tokenId` must not exist. + * - `to` cannot be the zero address. + * + * Emits a {Transfer} event. */ function _mint(address to, uint256 tokenId) internal virtual { require(to != address(0), "ERC721: mint to the zero address"); @@ -410,9 +343,14 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @dev Internal function to burn a specific token. - * Reverts if the token does not exist. - * @param tokenId uint256 ID of the token being burned + * @dev Destroys `tokenId`. + * The approval is cleared when the token is burned. + * + * Requirements: + * + * - `tokenId` must exist. + * + * Emits a {Transfer} event. */ function _burn(uint256 tokenId) internal virtual { address owner = ownerOf(tokenId); @@ -435,11 +373,15 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @dev Internal function to transfer ownership of a given token ID to another address. - * As opposed to {transferFrom}, this imposes no restrictions on msg.sender. - * @param from current owner of the token - * @param to address to receive the ownership of the given token ID - * @param tokenId uint256 ID of the token to be transferred + * @dev Transfers `tokenId` from `from` to `to`. + * As opposed to {transferFrom}, this imposes no restrictions on msg.sender. + * + * Requirements: + * + * - `to` cannot be the zero address. + * - `tokenId` token must be owned by `from`. + * + * Emits a {Transfer} event. */ function _transfer(address from, address to, uint256 tokenId) internal virtual { require(ownerOf(tokenId) == from, "ERC721: transfer of token that is not own"); @@ -459,13 +401,11 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable } /** - * @dev Internal function to set the token URI for a given token. + * @dev Sets `_tokenURI` as the tokenURI of `tokenId`. * - * Reverts if the token ID does not exist. + * Requirements: * - * 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. + * - `tokenId` must exist. */ function _setTokenURI(uint256 tokenId, string memory _tokenURI) internal virtual { require(_exists(tokenId), "ERC721Metadata: URI set of nonexistent token"); @@ -532,11 +472,12 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable * * Calling conditions: * - * - when `from` and `to` are both non-zero, ``from``'s `tokenId` will be + * - When `from` and `to` are both non-zero, ``from``'s `tokenId` will be * transferred to `to`. - * - when `from` is zero, `tokenId` will be minted for `to`. - * - when `to` is zero, ``from``'s `tokenId` will be burned. - * - `from` and `to` are never both zero. + * - When `from` is zero, `tokenId` will be minted for `to`. + * - When `to` is zero, ``from``'s `tokenId` will be burned. + * - `from` cannot be the zero address. + * - `to` cannot be the zero address. * * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks]. */ diff --git a/contracts/token/ERC721/ERC721Burnable.sol b/contracts/token/ERC721/ERC721Burnable.sol index 2a6732e58..6f767d23a 100644 --- a/contracts/token/ERC721/ERC721Burnable.sol +++ b/contracts/token/ERC721/ERC721Burnable.sol @@ -11,8 +11,11 @@ import "./ERC721.sol"; */ abstract contract ERC721Burnable is Context, ERC721 { /** - * @dev Burns a specific ERC721 token. - * @param tokenId uint256 id of the ERC721 token to be burned. + * @dev Burns `tokenId`. See {ERC721-_burn}. + * + * Requirements: + * + * - The caller must own `tokenId` or be an approved operator. */ function burn(uint256 tokenId) public virtual { //solhint-disable-next-line max-line-length diff --git a/contracts/token/ERC721/ERC721Holder.sol b/contracts/token/ERC721/ERC721Holder.sol index 2f6f91958..3684bd1cc 100644 --- a/contracts/token/ERC721/ERC721Holder.sol +++ b/contracts/token/ERC721/ERC721Holder.sol @@ -4,7 +4,19 @@ pragma solidity ^0.6.0; import "./IERC721Receiver.sol"; + /** + * @dev Implementation of the {IERC721Receiver} interface. + * + * Accepts all token transfers. + * Make sure the contract is able to use its token with {IERC721-safeTransferFrom}, {IERC721-approve} or {IERC721-setApprovalForAll}. + */ contract ERC721Holder is IERC721Receiver { + + /** + * @dev See {IERC721Receiver-onERC721Received}. + * + * Always returns `IERC721Receiver.onERC721Received.selector`. + */ function onERC721Received(address, address, uint256, bytes memory) public virtual override returns (bytes4) { return this.onERC721Received.selector; } diff --git a/contracts/token/ERC721/IERC721.sol b/contracts/token/ERC721/IERC721.sol index 6ad04f38a..16f7aca87 100644 --- a/contracts/token/ERC721/IERC721.sol +++ b/contracts/token/ERC721/IERC721.sol @@ -43,7 +43,8 @@ interface IERC721 is IERC165 { * * Requirements: * - * - `from`, `to` cannot be zero. + * - `from` cannot be the zero address. + * - `to` cannot be the zero address. * - `tokenId` token must exist and be owned by `from`. * - If the caller is not `from`, it must be have been allowed to move this token by either {approve} or {setApprovalForAll}. * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon a safe transfer. @@ -59,7 +60,8 @@ interface IERC721 is IERC165 { * * Requirements: * - * - `from`, `to` cannot be zero. + * - `from` cannot be the zero address. + * - `to` cannot be the zero address. * - `tokenId` token must be owned by `from`. * - If the caller is not `from`, it must be approved to move this token by either {approve} or {setApprovalForAll}. * @@ -115,7 +117,8 @@ interface IERC721 is IERC165 { * * Requirements: * - * - `from`, `to` cannot be zero. + * - `from` cannot be the zero address. + * - `to` cannot be the zero address. * - `tokenId` token must exist and be owned by `from`. * - If the caller is not `from`, it must be approved to move this token by either {approve} or {setApprovalForAll}. * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon a safe transfer. diff --git a/contracts/token/ERC721/IERC721Receiver.sol b/contracts/token/ERC721/IERC721Receiver.sol index f5b32e9e3..f98053e59 100644 --- a/contracts/token/ERC721/IERC721Receiver.sol +++ b/contracts/token/ERC721/IERC721Receiver.sol @@ -9,18 +9,13 @@ pragma solidity ^0.6.0; */ interface IERC721Receiver { /** - * @notice Handle the receipt of an NFT - * @dev The ERC721 smart contract calls this function on the recipient - * after a {IERC721-safeTransferFrom}. This function MUST return the function selector, - * otherwise the caller will revert the transaction. The selector to be - * returned can be obtained as `this.onERC721Received.selector`. This - * function MAY throw to revert and reject the transfer. - * Note: the ERC721 contract address is always the message sender. - * @param operator The address which called `safeTransferFrom` function - * @param from The address which previously owned the token - * @param tokenId The NFT identifier which is being transferred - * @param data Additional data with no specified format - * @return bytes4 `bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"))` + * @dev Whenever an {IERC721} `tokenId` token is transferred to this contract via {IERC721-safeTransferFrom} + * by `operator` from `from`, this function is called. + * + * It must return its Solidity selector to confirm the token transfer. + * If any other value is returned or the interface is not implemented by the recipient, the transfer will be reverted. + * + * The selector can be obtained in Solidity with `IERC721.onERC721Received.selector`. */ function onERC721Received(address operator, address from, uint256 tokenId, bytes calldata data) external returns (bytes4);