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.

Sign up for GitHub

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

Refactor ERC721 _requireMinted and ownerOf #4566

Merged
merged 2 commits into from
Sep 4, 2023
Merged

Refactor ERC721 _requireMinted and ownerOf #4566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
725f120
Refactor ERC721 `_requireMinted` and `ownerOf`
frangio Sep 2, 2023
247fe25
remove virtual
frangio Sep 4, 2023
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
5 changes: 5 additions & 0 deletions .changeset/proud-spiders-attend.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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`.
23 changes: 12 additions & 11 deletions contracts/token/ERC721/ERC721.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand All @@ -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()) : "";
Expand All @@ -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);
}
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion contracts/token/ERC721/extensions/ERC721URIStorage.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down