Skip to content

Commit

Permalink
Remove hardcoded function resolution (#4299)
Browse files Browse the repository at this point in the history
  • Loading branch information
frangio authored Jun 2, 2023
1 parent eecd5e1 commit ffceb3c
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 20 deletions.
5 changes: 5 additions & 0 deletions .changeset/red-dots-fold.md
Original file line number Diff line number Diff line change
@@ -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`.
2 changes: 1 addition & 1 deletion contracts/token/ERC1155/extensions/ERC1155Supply.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion contracts/token/ERC20/extensions/ERC20FlashMint.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
32 changes: 16 additions & 16 deletions contracts/token/ERC721/ERC721.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions contracts/token/ERC721/extensions/ERC721Enumerable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}

Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit ffceb3c

Please sign in to comment.