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

Use unchecked for ERC721 balance updates (#3524)

Browse Source 
Co-authored-by: Francisco <frangio.1@gmail.com>
This commit is contained in:
Hadrien Croubois
2022-08-23 04:48:09 +02:00
committed by GitHub
parent 98c3a79b57
commit f491e98d37
2 changed files with 24 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -12,6 +12,7 @@
* `ERC721`: optimize transfers by making approval clearing implicit instead of emitting an event. ([#3481](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3481))
* `ERC721`: optimize burn by making approval clearing implicit instead of emitting an event. ([#3538](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3538))
* `ERC721`: Fix balance accounting when a custom `_beforeTokenTransfer` hook results in a transfer of the token under consideration. ([#3611](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3611))
* `ERC721`: use unchecked arithmetic for balance updates. ([#3524](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3524))
* `ReentrancyGuard`: Reduce code size impact of the modifier by using internal functions. ([#3515](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3515))
* `SafeCast`: optimize downcasting of signed integers. ([#3565](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3565))
* `VestingWallet`: remove unused library `Math.sol`. ([#3605](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3605))

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

@ -285,7 +285,14 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {
// Check that tokenId was not minted by `_beforeTokenTransfer` hook
require(!_exists(tokenId), "ERC721: token already minted");
_balances[to] += 1;
unchecked {
// Will not overflow unless all 2**256 token ids are minted to the same owner.
// Given that tokens are minted one by one, it is impossible in practice that
// this ever happens. Might change if we allow batch minting.
// The ERC fails to describe this case.
_balances[to] += 1;
}
_owners[tokenId] = to;
emit Transfer(address(0), to, tokenId);
@ -309,13 +316,17 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {
_beforeTokenTransfer(owner, address(0), tokenId);
// Update ownership in case tokenId was transfered by `_beforeTokenTransfer` hook
// Update ownership in case tokenId was transferred by `_beforeTokenTransfer` hook
owner = ERC721.ownerOf(tokenId);
// Clear approvals
delete _tokenApprovals[tokenId];
_balances[owner] -= 1;
unchecked {
// Cannot overflow, as that would require more tokens to be burned/transferred
// out than the owner initialy received through minting and transferring in.
_balances[owner] -= 1;
}
delete _owners[tokenId];
emit Transfer(owner, address(0), tokenId);
@ -350,8 +361,15 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {
// Clear approvals from the previous owner
delete _tokenApprovals[tokenId];
_balances[from] -= 1;
_balances[to] += 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);