diff --git a/.changeset/fair-humans-peel.md b/.changeset/fair-humans-peel.md new file mode 100644 index 000000000..3c0dc3c06 --- /dev/null +++ b/.changeset/fair-humans-peel.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': major +--- + +`ERC721URIStorage`, `ERC721Royalty`: Stop resetting token-specific URI and royalties when burning. diff --git a/contracts/token/ERC721/extensions/ERC721Royalty.sol b/contracts/token/ERC721/extensions/ERC721Royalty.sol index 5b889b90d..6f31368e2 100644 --- a/contracts/token/ERC721/extensions/ERC721Royalty.sol +++ b/contracts/token/ERC721/extensions/ERC721Royalty.sol @@ -24,17 +24,4 @@ abstract contract ERC721Royalty is ERC2981, ERC721 { function supportsInterface(bytes4 interfaceId) public view virtual override(ERC721, ERC2981) returns (bool) { return super.supportsInterface(interfaceId); } - - /** - * @dev See {ERC721-_update}. When burning, this override will additionally clear the royalty information for the token. - */ - function _update(address to, uint256 tokenId, address auth) internal virtual override returns (address) { - address previousOwner = super._update(to, tokenId, auth); - - if (to == address(0)) { - _resetTokenRoyalty(tokenId); - } - - return previousOwner; - } } diff --git a/contracts/token/ERC721/extensions/ERC721URIStorage.sol b/contracts/token/ERC721/extensions/ERC721URIStorage.sol index 0a4f57322..b2adbbea3 100644 --- a/contracts/token/ERC721/extensions/ERC721URIStorage.sol +++ b/contracts/token/ERC721/extensions/ERC721URIStorage.sol @@ -58,19 +58,4 @@ abstract contract ERC721URIStorage is IERC4906, ERC721 { _tokenURIs[tokenId] = _tokenURI; emit MetadataUpdate(tokenId); } - - /** - * @dev See {ERC721-_update}. When burning, this override will additionally check if a - * token-specific URI was set for the token, and if so, it deletes the token URI from - * the storage mapping. - */ - function _update(address to, uint256 tokenId, address auth) internal virtual override returns (address) { - address previousOwner = super._update(to, tokenId, auth); - - if (to == address(0) && bytes(_tokenURIs[tokenId]).length != 0) { - delete _tokenURIs[tokenId]; - } - - return previousOwner; - } } diff --git a/test/token/ERC721/extensions/ERC721Royalty.test.js b/test/token/ERC721/extensions/ERC721Royalty.test.js index 1c0536bf5..78cba9858 100644 --- a/test/token/ERC721/extensions/ERC721Royalty.test.js +++ b/test/token/ERC721/extensions/ERC721Royalty.test.js @@ -1,15 +1,15 @@ -const { BN, constants } = require('@openzeppelin/test-helpers'); +require('@openzeppelin/test-helpers'); const { shouldBehaveLikeERC2981 } = require('../../common/ERC2981.behavior'); const ERC721Royalty = artifacts.require('$ERC721Royalty'); contract('ERC721Royalty', function (accounts) { - const [account1, account2] = accounts; - const tokenId1 = new BN('1'); - const tokenId2 = new BN('2'); - const royalty = new BN('200'); - const salePrice = new BN('1000'); + const [account1, account2, recipient] = accounts; + const tokenId1 = web3.utils.toBN('1'); + const tokenId2 = web3.utils.toBN('2'); + const royalty = web3.utils.toBN('200'); + const salePrice = web3.utils.toBN('1000'); beforeEach(async function () { this.token = await ERC721Royalty.new('My Token', 'TKN'); @@ -25,15 +25,21 @@ contract('ERC721Royalty', function (accounts) { describe('token specific functions', function () { beforeEach(async function () { - await this.token.$_setTokenRoyalty(tokenId1, account1, royalty); + await this.token.$_setTokenRoyalty(tokenId1, recipient, royalty); }); - it('removes royalty information after burn', async function () { + it('royalty information are kept during burn and re-mint', async function () { await this.token.$_burn(tokenId1); - const tokenInfo = await this.token.royaltyInfo(tokenId1, salePrice); - expect(tokenInfo[0]).to.be.equal(constants.ZERO_ADDRESS); - expect(tokenInfo[1]).to.be.bignumber.equal(new BN('0')); + const tokenInfoA = await this.token.royaltyInfo(tokenId1, salePrice); + expect(tokenInfoA[0]).to.be.equal(recipient); + expect(tokenInfoA[1]).to.be.bignumber.equal(salePrice.mul(royalty).divn(1e4)); + + await this.token.$_mint(account2, tokenId1); + + const tokenInfoB = await this.token.royaltyInfo(tokenId1, salePrice); + expect(tokenInfoB[0]).to.be.equal(recipient); + expect(tokenInfoB[1]).to.be.bignumber.equal(salePrice.mul(royalty).divn(1e4)); }); }); diff --git a/test/token/ERC721/extensions/ERC721URIStorage.test.js b/test/token/ERC721/extensions/ERC721URIStorage.test.js index dc208d1ea..8c882fab0 100644 --- a/test/token/ERC721/extensions/ERC721URIStorage.test.js +++ b/test/token/ERC721/extensions/ERC721URIStorage.test.js @@ -88,7 +88,7 @@ contract('ERC721URIStorage', function (accounts) { }); it('tokens without URI can be burnt ', async function () { - await this.token.$_burn(firstTokenId, { from: owner }); + await this.token.$_burn(firstTokenId); await expectRevertCustomError(this.token.tokenURI(firstTokenId), 'ERC721NonexistentToken', [firstTokenId]); }); @@ -96,9 +96,19 @@ contract('ERC721URIStorage', function (accounts) { it('tokens with URI can be burnt ', async function () { await this.token.$_setTokenURI(firstTokenId, sampleUri); - await this.token.$_burn(firstTokenId, { from: owner }); + await this.token.$_burn(firstTokenId); await expectRevertCustomError(this.token.tokenURI(firstTokenId), 'ERC721NonexistentToken', [firstTokenId]); }); + + it('tokens URI is kept if token is burnt and reminted ', async function () { + await this.token.$_setTokenURI(firstTokenId, sampleUri); + + await this.token.$_burn(firstTokenId); + await expectRevertCustomError(this.token.tokenURI(firstTokenId), 'ERC721NonexistentToken', [firstTokenId]); + + await this.token.$_mint(owner, firstTokenId); + expect(await this.token.tokenURI(firstTokenId)).to.be.equal(sampleUri); + }); }); });