From b4eb5c75b90551ba6fdc4dbe71d01f89ce015f05 Mon Sep 17 00:00:00 2001 From: Andrew Parker Date: Thu, 16 Jun 2022 15:47:02 +1000 Subject: [PATCH 1/3] Implicitly clear approval As per the ERC721 standard, the transfer function shouldn't emit an Approve event, as clearing the approval is implicit in the Transfer event. Docs regarding Approve event: ``` /// @dev This emits when the approved address for an NFT is changed or /// reaffirmed. The zero address indicates there is no approved address. /// When a Transfer event emits, this also indicates that the approved /// address for that NFT (if any) is reset to none. ``` [EIP-721](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md) --- contracts/token/ERC721/ERC721.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index 62cb43be61d..3faa4fb7c93 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -338,7 +338,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { _beforeTokenTransfer(from, to, tokenId); // Clear approvals from the previous owner - _approve(address(0), tokenId); + delete _tokenApprovals[tokenId]; _balances[from] -= 1; _balances[to] += 1; From 4c19fa9f8811a300e2926bfac4e63cd6ca0a956f Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 16 Jun 2022 21:18:12 +0200 Subject: [PATCH 2/3] update tests --- test/token/ERC721/ERC721.behavior.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/token/ERC721/ERC721.behavior.js b/test/token/ERC721/ERC721.behavior.js index d6aa4e6b919..b0893bc5330 100644 --- a/test/token/ERC721/ERC721.behavior.js +++ b/test/token/ERC721/ERC721.behavior.js @@ -96,10 +96,6 @@ function shouldBehaveLikeERC721 (errorPrefix, owner, newOwner, approved, another expect(await this.token.getApproved(tokenId)).to.be.equal(ZERO_ADDRESS); }); - it('emits an Approval event', async function () { - expectEvent(receipt, 'Approval', { owner, approved: ZERO_ADDRESS, tokenId: tokenId }); - }); - it('adjusts owners balances', async function () { expect(await this.token.balanceOf(owner)).to.be.bignumber.equal('1'); }); From b7090f7fdf59fce60db06bb0682d2e9af8cca497 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Fri, 1 Jul 2022 20:16:24 -0300 Subject: [PATCH 3/3] update changelog and add compatibility warning --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c2edb59785..06409f694c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ * `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)) * `ERC20`: optimize `_transfer`, `_mint` and `_burn` by using `unchecked` arithmetic when possible. ([#3513](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3513)) + * `ERC721`: optimize transfers by making approval clearing implicit instead of emitting an event. ([#3481](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3481)) + +### Compatibility Note + +ERC-721 integrators that interpret contract state from events should make sure that they implement the clearing of approval that is implicit in every transfer according to the EIP. Previous versions of OpenZeppellin Contracts emitted an explicit `Approval` event even though it was not required by the specification, and this is no longer the case. ## 4.7.0 (2022-06-29)