NoBey/openzeppelin-contracts
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Apply suggestions from code review

Browse Source 
Co-authored-by: Francisco <fg@frang.io>
Co-authored-by: Ernesto García <ernestognw@gmail.com>
This commit is contained in:
Hadrien Croubois
2023-07-13 10:00:39 +02:00
committed by GitHub
parent 10815081f7
commit fb4d9510de
1 changed files with 19 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
contracts/token/ERC721/ERC721.sol
View File

@ -179,8 +179,10 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
/** /**
* @dev Returns the owner of the `tokenId`. Does NOT revert if token doesn't exist * @dev Returns the owner of the `tokenId`. Does NOT revert if token doesn't exist
* *
* WARNING: Any override of this function that allocates token to accounts following a custom logic should go in * IMPORTANT: Any overrides to this function that add ownership of tokens not tracked by the
* pair with a call to {_increaseBalance} so that balances and ownerships remain consistent with one another. * core ERC721 logic MUST be matched with the use of {_increaseBalance} to keep balances
* consistent with ownership. The invariant to preserve is that for any address `a` the value returned by
* `balanceOf(a)` must be equal to the number of tokens such that `_ownerOf(tokenId)` is `a`.
*/ */
function _ownerOf(uint256 tokenId) internal view virtual returns (address) { function _ownerOf(uint256 tokenId) internal view virtual returns (address) {
return _owners[tokenId]; return _owners[tokenId];
@ -209,20 +211,17 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
* @dev Returns whether `spender` is allowed to manage `owner`'s tokens, or `tokenId` in * @dev Returns whether `spender` is allowed to manage `owner`'s tokens, or `tokenId` in
* particular (ignoring whether it is owned by `owner`). * particular (ignoring whether it is owned by `owner`).
* *
* WARNING: * WARNING: This function doesn't check that `owner` is the actual owner of the specified `tokenId`. Moreover, it returns true if `owner == spender` in order to skip the check where needed. Consider checking for cases where `spender == address(0)` since they could lead to unexpected behavior.
* - ownership is not checked.
* - spender = address(0) will lead to this function returning true in unexpected situations.
*/ */
function _isApproved(address owner, address spender, uint256 tokenId) internal view virtual returns (bool) { function _isApproved(address owner, address spender, uint256 tokenId) internal view virtual returns (bool) {
return owner == spender || isApprovedForAll(owner, spender) || getApproved(tokenId) == spender; return owner == spender || isApprovedForAll(owner, spender) || getApproved(tokenId) == spender;
} }
/** /**
* @dev Checks whether `spender` was approved by `owner` to operate on `tokenId` or is the owner of `tokenId`. * @dev Checks if `spender` can operate on `tokenId`, assuming the provided `owner` is the actual owner.
* Reverts if `spender` is not approved nor owner of `tokenId`. * Reverts if `spender` has not approval for all assets of the provided `owner` nor the actual owner approved the `spender` for the specific `tokenId`.
* *
* WARNING: * WARNING: This function relies on {_isApproved}, so it doesn't check whether `owner` is the actual owner of `tokenId` and will skip the check if `spender == owner`
* - ownership is not checked.
*/ */
function _checkApproved(address owner, address spender, uint256 tokenId) internal view virtual { function _checkApproved(address owner, address spender, uint256 tokenId) internal view virtual {
if (!_isApproved(owner, spender, tokenId)) { if (!_isApproved(owner, spender, tokenId)) {
@ -291,7 +290,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
} }
/** /**
* @dev Safely mints `tokenId` and transfers it to `to`. * @dev Mints `tokenId`, transfers it to `to` and checks for `to` acceptance.
* *
* Requirements: * Requirements:
* *
@ -355,8 +354,8 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
} }
/** /**
* @dev Safely transfers `tokenId` token from `from` to `to`, checking first that contract recipients * @dev Safely transfers `tokenId` token from `from` to `to`, checking that contract recipients
* are aware of the ERC721 protocol to prevent tokens from being forever locked. * are aware of the ERC721 standard to prevent tokens from being forever locked.
* *
* `data` is additional data, it has no specified format and it is sent in call to `to`. * `data` is additional data, it has no specified format and it is sent in call to `to`.
* *
@ -373,8 +372,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
* Emits a {Transfer} event. * Emits a {Transfer} event.
*/ */
function _safeTransfer(address from, address to, uint256 tokenId) internal { function _safeTransfer(address from, address to, uint256 tokenId) internal {
_transfer(from, to, tokenId); _safeTransfer(from, to, tokenId, "");
_checkOnERC721Received(from, to, tokenId, "");
} }
/** /**
@ -403,6 +401,10 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
/** /**
* @dev Approve `operator` to operate on all of `owner` tokens * @dev Approve `operator` to operate on all of `owner` tokens
* *
* Requirements:
* - operator can't be the address zero.
* - owner can't approve himself.
*
* Emits an {ApprovalForAll} event. * Emits an {ApprovalForAll} event.
*/ */
function _setApprovalForAll(address owner, address operator, bool approved) internal virtual { function _setApprovalForAll(address owner, address operator, bool approved) internal virtual {
@ -461,6 +463,8 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er
* remain consistent with one another. * remain consistent with one another.
*/ */
function _increaseBalance(address account, uint128 value) internal virtual { function _increaseBalance(address account, uint128 value) internal virtual {
_balances[account] += value; unchecked {
_balances[account] += value;
}
} }
} }