From 961208382660991e7679eb03865a2785e2c8cabe Mon Sep 17 00:00:00 2001 From: Francisco Date: Mon, 4 Sep 2023 10:54:21 -0300 Subject: [PATCH] Refactor ERC721 `_requireMinted` and `ownerOf` (#4566) --- .changeset/proud-spiders-attend.md | 5 ++++ contracts/token/ERC721/ERC721.sol | 23 ++++++++++--------- .../ERC721/extensions/ERC721URIStorage.sol | 2 +- 3 files changed, 18 insertions(+), 12 deletions(-) create mode 100644 .changeset/proud-spiders-attend.md diff --git a/.changeset/proud-spiders-attend.md b/.changeset/proud-spiders-attend.md new file mode 100644 index 00000000000..a8f7694c709 --- /dev/null +++ b/.changeset/proud-spiders-attend.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': major +--- + +`ERC721`: Renamed `_requireMinted` to `_requireOwned` and added a return value with the current owner. Implemented `ownerOf` in terms of `_requireOwned`. diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index d56e3377577..efca4b4342a 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -65,11 +65,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er * @dev See {IERC721-ownerOf}. */ function ownerOf(uint256 tokenId) public view virtual returns (address) { - address owner = _ownerOf(tokenId); - if (owner == address(0)) { - revert ERC721NonexistentToken(tokenId); - } - return owner; + return _requireOwned(tokenId); } /** @@ -90,7 +86,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er * @dev See {IERC721Metadata-tokenURI}. */ function tokenURI(uint256 tokenId) public view virtual returns (string memory) { - _requireMinted(tokenId); + _requireOwned(tokenId); string memory baseURI = _baseURI(); return bytes(baseURI).length > 0 ? string.concat(baseURI, tokenId.toString()) : ""; @@ -116,7 +112,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er * @dev See {IERC721-getApproved}. */ function getApproved(uint256 tokenId) public view virtual returns (address) { - _requireMinted(tokenId); + _requireOwned(tokenId); return _getApproved(tokenId); } @@ -397,7 +393,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er * Emits an {Approval} event. */ function _approve(address to, uint256 tokenId, address auth) internal virtual returns (address) { - address owner = ownerOf(tokenId); + address owner = _requireOwned(tokenId); // We do not use _isAuthorized because single-token approvals should not be able to call approve if (auth != address(0) && owner != auth && !isApprovedForAll(owner, auth)) { @@ -427,12 +423,17 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er } /** - * @dev Reverts if the `tokenId` has not been minted yet. + * @dev Reverts if the `tokenId` doesn't have a current owner (it hasn't been minted, or it has been burned). + * Returns the owner. + * + * Overrides to ownership logic should be done to {_ownerOf}. */ - function _requireMinted(uint256 tokenId) internal view virtual { - if (_ownerOf(tokenId) == address(0)) { + function _requireOwned(uint256 tokenId) internal view returns (address) { + address owner = _ownerOf(tokenId); + if (owner == address(0)) { revert ERC721NonexistentToken(tokenId); } + return owner; } /** diff --git a/contracts/token/ERC721/extensions/ERC721URIStorage.sol b/contracts/token/ERC721/extensions/ERC721URIStorage.sol index b2adbbea3c2..e515f236966 100644 --- a/contracts/token/ERC721/extensions/ERC721URIStorage.sol +++ b/contracts/token/ERC721/extensions/ERC721URIStorage.sol @@ -32,7 +32,7 @@ abstract contract ERC721URIStorage is IERC4906, ERC721 { * @dev See {IERC721Metadata-tokenURI}. */ function tokenURI(uint256 tokenId) public view virtual override returns (string memory) { - _requireMinted(tokenId); + _requireOwned(tokenId); string memory _tokenURI = _tokenURIs[tokenId]; string memory base = _baseURI();