Skip to content

Commit

Permalink
Change execution order to avoid reentry through the _beforeTokenTrans…
Browse files Browse the repository at this point in the history
…fer hook (OpenZeppelin#3611)

Co-authored-by: Francisco <frangio.1@gmail.com>
  • Loading branch information
ronhuafeng and frangio committed Sep 9, 2022
1 parent 935a70e commit c725078
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 1 deletion.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@
* `ERC165Checker`: add `supportsERC165InterfaceUnchecked` for consulting individual interfaces without the full ERC165 protocol. ([#3339](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3339))
* `Address`: optimize `functionCall` by calling `functionCallWithValue` directly. ([#3468](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3468))
* `Address`: optimize `functionCall` functions by checking contract size only if there is no returned data. ([#3469](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3469))
* `GovernorCompatibilityBravo`: remove unused `using` statements ([#3506](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3506))
* `GovernorCompatibilityBravo`: remove unused `using` statements. ([#3506](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3506))
* `ERC20`: optimize `_transfer`, `_mint` and `_burn` by using `unchecked` arithmetic when possible. ([#3513](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3513))
* `ERC20FlashMint`: add an internal `_flashFee` function for overriding. ([#3551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3551))
* `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))
* `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))
Expand Down
9 changes: 9 additions & 0 deletions contracts/token/ERC721/ERC721.sol
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,9 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {

_beforeTokenTransfer(address(0), to, tokenId);

// Check that tokenId was not minted by `_beforeTokenTransfer` hook
require(!_exists(tokenId), "ERC721: token already minted");

_balances[to] += 1;
_owners[tokenId] = to;

Expand All @@ -306,6 +309,9 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {

_beforeTokenTransfer(owner, address(0), tokenId);

// Update ownership in case tokenId was transfered by `_beforeTokenTransfer` hook
owner = ERC721.ownerOf(tokenId);

// Clear approvals
delete _tokenApprovals[tokenId];

Expand Down Expand Up @@ -338,6 +344,9 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {

_beforeTokenTransfer(from, to, tokenId);

// Check that tokenId was not transfered by `_beforeTokenTransfer` hook
require(ERC721.ownerOf(tokenId) == from, "ERC721: transfer from incorrect owner");

// Clear approvals from the previous owner
delete _tokenApprovals[tokenId];

Expand Down

0 comments on commit c725078

Please sign in to comment.