diff --git a/.changeset/red-dots-fold.md b/.changeset/red-dots-fold.md new file mode 100644 index 00000000000..6df4c8f0cbc --- /dev/null +++ b/.changeset/red-dots-fold.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': major +--- + +Overrides are now used internally for a number of functions that were previously hardcoded to their default implementation in certain locations: `ERC1155Supply.totalSupply`, `ERC721.ownerOf`, `ERC721.balanceOf` in `ERC721Enumerable`, and `ERC20.totalSupply` in `ERC20FlashMint`. diff --git a/contracts/token/ERC1155/extensions/ERC1155Supply.sol b/contracts/token/ERC1155/extensions/ERC1155Supply.sol index 599df00f31c..4ad83ea0289 100644 --- a/contracts/token/ERC1155/extensions/ERC1155Supply.sol +++ b/contracts/token/ERC1155/extensions/ERC1155Supply.sol @@ -38,7 +38,7 @@ abstract contract ERC1155Supply is ERC1155 { * @dev Indicates whether any token exist with a given id, or not. */ function exists(uint256 id) public view virtual returns (bool) { - return ERC1155Supply.totalSupply(id) > 0; + return totalSupply(id) > 0; } /** diff --git a/contracts/token/ERC20/extensions/ERC20FlashMint.sol b/contracts/token/ERC20/extensions/ERC20FlashMint.sol index ce68793b581..18b0a81c6d1 100644 --- a/contracts/token/ERC20/extensions/ERC20FlashMint.sol +++ b/contracts/token/ERC20/extensions/ERC20FlashMint.sol @@ -25,7 +25,7 @@ abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender { * @return The amount of token that can be loaned. */ function maxFlashLoan(address token) public view virtual override returns (uint256) { - return token == address(this) ? type(uint256).max - ERC20.totalSupply() : 0; + return token == address(this) ? type(uint256).max - totalSupply() : 0; } /** diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index 58a626cdad0..af00ecaf4ee 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -108,7 +108,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { * @dev See {IERC721-approve}. */ function approve(address to, uint256 tokenId) public virtual override { - address owner = ERC721.ownerOf(tokenId); + address owner = ownerOf(tokenId); require(to != owner, "ERC721: approval to current owner"); require( @@ -217,7 +217,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { * - `tokenId` must exist. */ function _isApprovedOrOwner(address spender, uint256 tokenId) internal view virtual returns (bool) { - address owner = ERC721.ownerOf(tokenId); + address owner = ownerOf(tokenId); return (spender == owner || isApprovedForAll(owner, spender) || getApproved(tokenId) == spender); } @@ -295,21 +295,20 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { * Emits a {Transfer} event. */ function _burn(uint256 tokenId) internal virtual { - address owner = ERC721.ownerOf(tokenId); + address owner = ownerOf(tokenId); _beforeTokenTransfer(owner, address(0), tokenId, 1); // Update ownership in case tokenId was transferred by `_beforeTokenTransfer` hook - owner = ERC721.ownerOf(tokenId); + owner = ownerOf(tokenId); // Clear approvals delete _tokenApprovals[tokenId]; - unchecked { - // Cannot overflow, as that would require more tokens to be burned/transferred - // out than the owner initially received through minting and transferring in. - _balances[owner] -= 1; - } + // Decrease balance with checked arithmetic, because an `ownerOf` override may + // invalidate the assumption that `_balances[from] >= 1`. + _balances[owner] -= 1; + delete _owners[tokenId]; emit Transfer(owner, address(0), tokenId); @@ -329,26 +328,27 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { * Emits a {Transfer} event. */ function _transfer(address from, address to, uint256 tokenId) internal virtual { - require(ERC721.ownerOf(tokenId) == from, "ERC721: transfer from incorrect owner"); + require(ownerOf(tokenId) == from, "ERC721: transfer from incorrect owner"); require(to != address(0), "ERC721: transfer to the zero address"); _beforeTokenTransfer(from, to, tokenId, 1); // Check that tokenId was not transferred by `_beforeTokenTransfer` hook - require(ERC721.ownerOf(tokenId) == from, "ERC721: transfer from incorrect owner"); + require(ownerOf(tokenId) == from, "ERC721: transfer from incorrect owner"); // Clear approvals from the previous owner delete _tokenApprovals[tokenId]; + // Decrease balance with checked arithmetic, because an `ownerOf` override may + // invalidate the assumption that `_balances[from] >= 1`. + _balances[from] -= 1; + unchecked { - // `_balances[from]` cannot overflow for the same reason as described in `_burn`: - // `from`'s balance is the number of token held, which is at least one before the current - // transfer. // `_balances[to]` could overflow in the conditions described in `_mint`. That would require // all 2**256 token ids to be minted, which in practice is impossible. - _balances[from] -= 1; _balances[to] += 1; } + _owners[tokenId] = to; emit Transfer(from, to, tokenId); @@ -363,7 +363,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { */ function _approve(address to, uint256 tokenId) internal virtual { _tokenApprovals[tokenId] = to; - emit Approval(ERC721.ownerOf(tokenId), to, tokenId); + emit Approval(ownerOf(tokenId), to, tokenId); } /** diff --git a/contracts/token/ERC721/extensions/ERC721Enumerable.sol b/contracts/token/ERC721/extensions/ERC721Enumerable.sol index 6b702d53bec..a9513b21ddb 100644 --- a/contracts/token/ERC721/extensions/ERC721Enumerable.sol +++ b/contracts/token/ERC721/extensions/ERC721Enumerable.sol @@ -35,7 +35,7 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable { * @dev See {IERC721Enumerable-tokenOfOwnerByIndex}. */ function tokenOfOwnerByIndex(address owner, uint256 index) public view virtual override returns (uint256) { - require(index < ERC721.balanceOf(owner), "ERC721Enumerable: owner index out of bounds"); + require(index < balanceOf(owner), "ERC721Enumerable: owner index out of bounds"); return _ownedTokens[owner][index]; } @@ -90,7 +90,7 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable { * @param tokenId uint256 ID of the token to be added to the tokens list of the given address */ function _addTokenToOwnerEnumeration(address to, uint256 tokenId) private { - uint256 length = ERC721.balanceOf(to); + uint256 length = balanceOf(to); _ownedTokens[to][length] = tokenId; _ownedTokensIndex[tokenId] = length; }