From 244113fd9cb85ccd56776cefc215d08c3868385d Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 3 Mar 2021 11:34:12 +0100 Subject: [PATCH 1/3] add ERC721TokenUri extension --- contracts/mocks/ERC721BurnableMock.sol | 12 +++ contracts/mocks/ERC721EnumerableMock.sol | 4 + contracts/mocks/ERC721PausableMock.sol | 26 +++-- contracts/mocks/ERC721TokenUriMock.sol | 41 ++++++++ .../ERC721/extensions/ERC721TokenUri.sol | 95 +++++++++++++++++++ .../ERC721/extensions/ERC721TokenUri.test.js | 87 +++++++++++++++++ 6 files changed, 256 insertions(+), 9 deletions(-) create mode 100644 contracts/mocks/ERC721TokenUriMock.sol create mode 100644 contracts/token/ERC721/extensions/ERC721TokenUri.sol create mode 100644 test/token/ERC721/extensions/ERC721TokenUri.test.js diff --git a/contracts/mocks/ERC721BurnableMock.sol b/contracts/mocks/ERC721BurnableMock.sol index a1a5893c512..de15340ea74 100644 --- a/contracts/mocks/ERC721BurnableMock.sol +++ b/contracts/mocks/ERC721BurnableMock.sol @@ -7,7 +7,19 @@ import "../token/ERC721/extensions/ERC721Burnable.sol"; contract ERC721BurnableMock is ERC721Burnable { constructor(string memory name, string memory symbol) ERC721(name, symbol) { } + function exists(uint256 tokenId) public view returns (bool) { + return _exists(tokenId); + } + function mint(address to, uint256 tokenId) public { _mint(to, tokenId); } + + function safeMint(address to, uint256 tokenId) public { + _safeMint(to, tokenId); + } + + function safeMint(address to, uint256 tokenId, bytes memory _data) public { + _safeMint(to, tokenId, _data); + } } diff --git a/contracts/mocks/ERC721EnumerableMock.sol b/contracts/mocks/ERC721EnumerableMock.sol index 8ec0cddb2ec..f64f37eb648 100644 --- a/contracts/mocks/ERC721EnumerableMock.sol +++ b/contracts/mocks/ERC721EnumerableMock.sol @@ -25,6 +25,10 @@ contract ERC721EnumerableMock is ERC721Enumerable { return _baseURI(); } + function exists(uint256 tokenId) public view returns (bool) { + return _exists(tokenId); + } + function mint(address to, uint256 tokenId) public { _mint(to, tokenId); } diff --git a/contracts/mocks/ERC721PausableMock.sol b/contracts/mocks/ERC721PausableMock.sol index 2133341aaf6..8541c8996e0 100644 --- a/contracts/mocks/ERC721PausableMock.sol +++ b/contracts/mocks/ERC721PausableMock.sol @@ -11,23 +11,31 @@ import "../token/ERC721/extensions/ERC721Pausable.sol"; contract ERC721PausableMock is ERC721Pausable { constructor (string memory name, string memory symbol) ERC721(name, symbol) { } - function mint(address to, uint256 tokenId) public { - super._mint(to, tokenId); + function pause() external { + _pause(); } - function burn(uint256 tokenId) public { - super._burn(tokenId); + function unpause() external { + _unpause(); } function exists(uint256 tokenId) public view returns (bool) { - return super._exists(tokenId); + return _exists(tokenId); } - function pause() external { - _pause(); + function mint(address to, uint256 tokenId) public { + _mint(to, tokenId); } - function unpause() external { - _unpause(); + function safeMint(address to, uint256 tokenId) public { + _safeMint(to, tokenId); + } + + function safeMint(address to, uint256 tokenId, bytes memory _data) public { + _safeMint(to, tokenId, _data); + } + + function burn(uint256 tokenId) public { + _burn(tokenId); } } diff --git a/contracts/mocks/ERC721TokenUriMock.sol b/contracts/mocks/ERC721TokenUriMock.sol new file mode 100644 index 00000000000..4390809b843 --- /dev/null +++ b/contracts/mocks/ERC721TokenUriMock.sol @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../token/ERC721/extensions/ERC721TokenUri.sol"; + +/** + * @title ERC721Mock + * This mock just provides a public safeMint, mint, and burn functions for testing purposes + */ +contract ERC721TokenUriMock is ERC721TokenUri { + constructor (string memory name, string memory symbol) ERC721(name, symbol) { } + + function setBaseURI(string calldata newBaseTokenURI) public { + _setBaseURI(newBaseTokenURI); + } + + function setTokenURI(uint256 tokenId, string memory _tokenURI) public { + _setTokenURI(tokenId, _tokenURI); + } + + function exists(uint256 tokenId) public view returns (bool) { + return _exists(tokenId); + } + + function mint(address to, uint256 tokenId) public { + _mint(to, tokenId); + } + + function safeMint(address to, uint256 tokenId) public { + _safeMint(to, tokenId); + } + + function safeMint(address to, uint256 tokenId, bytes memory _data) public { + _safeMint(to, tokenId, _data); + } + + function burn(uint256 tokenId) public { + _burn(tokenId); + } +} diff --git a/contracts/token/ERC721/extensions/ERC721TokenUri.sol b/contracts/token/ERC721/extensions/ERC721TokenUri.sol new file mode 100644 index 00000000000..720c1a93e63 --- /dev/null +++ b/contracts/token/ERC721/extensions/ERC721TokenUri.sol @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../ERC721.sol"; + +/** + * @dev ERC721 token with storage based token uri management. + */ +abstract contract ERC721TokenUri is ERC721 { + using Strings for uint256; + + // Optional mapping for token URIs + mapping (uint256 => string) private _tokenURIs; + + // Base URI + string private _storedBaseURI; + + /** + * @dev See {IERC721Metadata-tokenURI}. + */ + function tokenURI(uint256 tokenId) public view virtual override returns (string memory) { + require(_exists(tokenId), "ERC721TokenUri: URI query for nonexistent token"); + + string memory _tokenURI = _tokenURIs[tokenId]; + string memory base = baseURI(); + + // If there is no base URI, return the token URI. + if (bytes(base).length == 0) { + return _tokenURI; + } + // If both are set, concatenate the baseURI and tokenURI (via abi.encodePacked). + if (bytes(_tokenURI).length > 0) { + return string(abi.encodePacked(base, _tokenURI)); + } + // If there is a baseURI but no tokenURI, concatenate the tokenID to the baseURI. + return string(abi.encodePacked(base, tokenId.toString())); + } + + /** + * @dev Returns the base URI set via {_setBaseURI}. This will be + * automatically added as a prefix in {tokenURI} to each token's URI, or + * to the token ID if no specific URI is set for that token ID. + */ + function baseURI() public view virtual returns (string memory) { + return _baseURI(); + } + + /** + * @dev Base URI for computing {tokenURI}. Empty by default, can be overriden + * in child contracts. + */ + function _baseURI() internal view virtual override returns (string memory) { + return _storedBaseURI; + } + + /** + * @dev Internal function to set the base URI for all token IDs. It is + * automatically added as a prefix to the value returned in {tokenURI}, + * or to the token ID if {tokenURI} is empty. + */ + function _setBaseURI(string memory baseURI_) internal virtual { + _storedBaseURI = baseURI_; + } + + /** + * @dev Sets `_tokenURI` as the tokenURI of `tokenId`. + * + * Requirements: + * + * - `tokenId` must exist. + */ + function _setTokenURI(uint256 tokenId, string memory _tokenURI) internal virtual { + require(_exists(tokenId), "ERC721TokenUri: URI set of nonexistent token"); + _tokenURIs[tokenId] = _tokenURI; + } + + /** + * @dev Destroys `tokenId`. + * The approval is cleared when the token is burned. + * + * Requirements: + * + * - `tokenId` must exist. + * + * Emits a {Transfer} event. + */ + function _burn(uint256 tokenId) internal virtual override { + super._burn(tokenId); + + if (bytes(_tokenURIs[tokenId]).length != 0) { + delete _tokenURIs[tokenId]; + } + } +} diff --git a/test/token/ERC721/extensions/ERC721TokenUri.test.js b/test/token/ERC721/extensions/ERC721TokenUri.test.js new file mode 100644 index 00000000000..032d87d4a07 --- /dev/null +++ b/test/token/ERC721/extensions/ERC721TokenUri.test.js @@ -0,0 +1,87 @@ +const { BN, expectRevert } = require('@openzeppelin/test-helpers'); + +const { expect } = require('chai'); + +const ERC721TokenUriMock = artifacts.require('ERC721TokenUriMock'); + +contract('ERC721TokenUri', function (accounts) { + const [ owner ] = accounts; + + const name = 'Non Fungible Token'; + const symbol = 'NFT'; + + const firstTokenId = new BN('5042'); + const nonExistentTokenId = new BN('13'); + + beforeEach(async function () { + this.token = await ERC721TokenUriMock.new(name, symbol); + }); + + describe('token URI', function () { + beforeEach(async function () { + await this.token.mint(owner, firstTokenId); + }); + + const baseURI = 'https://api.com/v1/'; + const sampleUri = 'mock://mytoken'; + + it('it is empty by default', async function () { + expect(await this.token.tokenURI(firstTokenId)).to.be.equal(''); + }); + + it('reverts when queried for non existent token id', async function () { + await expectRevert( + this.token.tokenURI(nonExistentTokenId), 'ERC721TokenUri: URI query for nonexistent token', + ); + }); + + it('can be set for a token id', async function () { + await this.token.setTokenURI(firstTokenId, sampleUri); + expect(await this.token.tokenURI(firstTokenId)).to.be.equal(sampleUri); + }); + + it('reverts when setting for non existent token id', async function () { + await expectRevert( + this.token.setTokenURI(nonExistentTokenId, sampleUri), 'ERC721TokenUri: URI set of nonexistent token', + ); + }); + + it('base URI can be set', async function () { + await this.token.setBaseURI(baseURI); + expect(await this.token.baseURI()).to.equal(baseURI); + }); + + it('base URI is added as a prefix to the token URI', async function () { + await this.token.setBaseURI(baseURI); + await this.token.setTokenURI(firstTokenId, sampleUri); + + expect(await this.token.tokenURI(firstTokenId)).to.be.equal(baseURI + sampleUri); + }); + + it('token URI can be changed by changing the base URI', async function () { + await this.token.setBaseURI(baseURI); + await this.token.setTokenURI(firstTokenId, sampleUri); + + const newBaseURI = 'https://api.com/v2/'; + await this.token.setBaseURI(newBaseURI); + expect(await this.token.tokenURI(firstTokenId)).to.be.equal(newBaseURI + sampleUri); + }); + + it('tokenId is appended to base URI for tokens with no URI', async function () { + await this.token.setBaseURI(baseURI); + + expect(await this.token.tokenURI(firstTokenId)).to.be.equal(baseURI + firstTokenId); + }); + + it('tokens with URI can be burnt ', async function () { + await this.token.setTokenURI(firstTokenId, sampleUri); + + await this.token.burn(firstTokenId, { from: owner }); + + expect(await this.token.exists(firstTokenId)).to.equal(false); + await expectRevert( + this.token.tokenURI(firstTokenId), 'ERC721TokenUri: URI query for nonexistent token', + ); + }); + }); +}); From 22ed7d73eaf155d9ba06303f09b17883dba7516f Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 3 Mar 2021 11:38:36 +0100 Subject: [PATCH 2/3] add Changelog entry & update documentation --- CHANGELOG.md | 1 + contracts/token/ERC721/README.adoc | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c716c328df6..837594ec7d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ * `ERC20Capped`: optimize gas usage of by enforcing te check directly in `_mint`. ([#2524](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2524)) * Rename `UpgradeableProxy` to `ERC1967Proxy`. ([#2547](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2547)) * `ERC777`: Optimize the gas costs of the constructor. ([#2551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2551)) + * `ERC721TokenUri`: Add a new extension ERC721TokenUri that implements the tokenURI behavior as it was available in 3.4.0. ([#2555](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2555)) ### How to upgrade from 3.x diff --git a/contracts/token/ERC721/README.adoc b/contracts/token/ERC721/README.adoc index 07b73d50cbc..5394ded422b 100644 --- a/contracts/token/ERC721/README.adoc +++ b/contracts/token/ERC721/README.adoc @@ -39,6 +39,8 @@ NOTE: This core set of contracts is designed to be unopinionated, allowing devel {{ERC721Burnable}} +{{ERC721TokenUri}} + == Presets These contracts are preconfigured combinations of the above features. They can be used through inheritance or as models to copy and paste their source code. From f0b092db7a8757be9499c26b09950a666fc59c86 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 3 Mar 2021 16:04:46 +0100 Subject: [PATCH 3/3] rename to ERC721URIStorage --- ...enUriMock.sol => ERC721URIStorageMock.sol} | 16 ++++++-- ...RC721TokenUri.sol => ERC721URIStorage.sol} | 39 +++---------------- ...enUri.test.js => ERC721URIStorage.test.js} | 12 +++--- 3 files changed, 24 insertions(+), 43 deletions(-) rename contracts/mocks/{ERC721TokenUriMock.sol => ERC721URIStorageMock.sol} (70%) rename contracts/token/ERC721/extensions/{ERC721TokenUri.sol => ERC721URIStorage.sol} (52%) rename test/token/ERC721/extensions/{ERC721TokenUri.test.js => ERC721URIStorage.test.js} (85%) diff --git a/contracts/mocks/ERC721TokenUriMock.sol b/contracts/mocks/ERC721URIStorageMock.sol similarity index 70% rename from contracts/mocks/ERC721TokenUriMock.sol rename to contracts/mocks/ERC721URIStorageMock.sol index 4390809b843..4c355d19af6 100644 --- a/contracts/mocks/ERC721TokenUriMock.sol +++ b/contracts/mocks/ERC721URIStorageMock.sol @@ -2,17 +2,27 @@ pragma solidity ^0.8.0; -import "../token/ERC721/extensions/ERC721TokenUri.sol"; +import "../token/ERC721/extensions/ERC721URIStorage.sol"; /** * @title ERC721Mock * This mock just provides a public safeMint, mint, and burn functions for testing purposes */ -contract ERC721TokenUriMock is ERC721TokenUri { +contract ERC721URIStorageMock is ERC721URIStorage { + string private _baseTokenURI; + constructor (string memory name, string memory symbol) ERC721(name, symbol) { } + function _baseURI() internal view virtual override returns (string memory) { + return _baseTokenURI; + } + function setBaseURI(string calldata newBaseTokenURI) public { - _setBaseURI(newBaseTokenURI); + _baseTokenURI = newBaseTokenURI; + } + + function baseURI() public view returns (string memory) { + return _baseURI(); } function setTokenURI(uint256 tokenId, string memory _tokenURI) public { diff --git a/contracts/token/ERC721/extensions/ERC721TokenUri.sol b/contracts/token/ERC721/extensions/ERC721URIStorage.sol similarity index 52% rename from contracts/token/ERC721/extensions/ERC721TokenUri.sol rename to contracts/token/ERC721/extensions/ERC721URIStorage.sol index 720c1a93e63..ab43d6d4243 100644 --- a/contracts/token/ERC721/extensions/ERC721TokenUri.sol +++ b/contracts/token/ERC721/extensions/ERC721URIStorage.sol @@ -7,23 +7,20 @@ import "../ERC721.sol"; /** * @dev ERC721 token with storage based token uri management. */ -abstract contract ERC721TokenUri is ERC721 { +abstract contract ERC721URIStorage is ERC721 { using Strings for uint256; // Optional mapping for token URIs mapping (uint256 => string) private _tokenURIs; - // Base URI - string private _storedBaseURI; - /** * @dev See {IERC721Metadata-tokenURI}. */ function tokenURI(uint256 tokenId) public view virtual override returns (string memory) { - require(_exists(tokenId), "ERC721TokenUri: URI query for nonexistent token"); + require(_exists(tokenId), "ERC721URIStorage: URI query for nonexistent token"); string memory _tokenURI = _tokenURIs[tokenId]; - string memory base = baseURI(); + string memory base = _baseURI(); // If there is no base URI, return the token URI. if (bytes(base).length == 0) { @@ -33,34 +30,8 @@ abstract contract ERC721TokenUri is ERC721 { if (bytes(_tokenURI).length > 0) { return string(abi.encodePacked(base, _tokenURI)); } - // If there is a baseURI but no tokenURI, concatenate the tokenID to the baseURI. - return string(abi.encodePacked(base, tokenId.toString())); - } - - /** - * @dev Returns the base URI set via {_setBaseURI}. This will be - * automatically added as a prefix in {tokenURI} to each token's URI, or - * to the token ID if no specific URI is set for that token ID. - */ - function baseURI() public view virtual returns (string memory) { - return _baseURI(); - } - /** - * @dev Base URI for computing {tokenURI}. Empty by default, can be overriden - * in child contracts. - */ - function _baseURI() internal view virtual override returns (string memory) { - return _storedBaseURI; - } - - /** - * @dev Internal function to set the base URI for all token IDs. It is - * automatically added as a prefix to the value returned in {tokenURI}, - * or to the token ID if {tokenURI} is empty. - */ - function _setBaseURI(string memory baseURI_) internal virtual { - _storedBaseURI = baseURI_; + return super.tokenURI(tokenId); } /** @@ -71,7 +42,7 @@ abstract contract ERC721TokenUri is ERC721 { * - `tokenId` must exist. */ function _setTokenURI(uint256 tokenId, string memory _tokenURI) internal virtual { - require(_exists(tokenId), "ERC721TokenUri: URI set of nonexistent token"); + require(_exists(tokenId), "ERC721URIStorage: URI set of nonexistent token"); _tokenURIs[tokenId] = _tokenURI; } diff --git a/test/token/ERC721/extensions/ERC721TokenUri.test.js b/test/token/ERC721/extensions/ERC721URIStorage.test.js similarity index 85% rename from test/token/ERC721/extensions/ERC721TokenUri.test.js rename to test/token/ERC721/extensions/ERC721URIStorage.test.js index 032d87d4a07..4a005f94bc1 100644 --- a/test/token/ERC721/extensions/ERC721TokenUri.test.js +++ b/test/token/ERC721/extensions/ERC721URIStorage.test.js @@ -2,9 +2,9 @@ const { BN, expectRevert } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); -const ERC721TokenUriMock = artifacts.require('ERC721TokenUriMock'); +const ERC721URIStorageMock = artifacts.require('ERC721URIStorageMock'); -contract('ERC721TokenUri', function (accounts) { +contract('ERC721URIStorage', function (accounts) { const [ owner ] = accounts; const name = 'Non Fungible Token'; @@ -14,7 +14,7 @@ contract('ERC721TokenUri', function (accounts) { const nonExistentTokenId = new BN('13'); beforeEach(async function () { - this.token = await ERC721TokenUriMock.new(name, symbol); + this.token = await ERC721URIStorageMock.new(name, symbol); }); describe('token URI', function () { @@ -31,7 +31,7 @@ contract('ERC721TokenUri', function (accounts) { it('reverts when queried for non existent token id', async function () { await expectRevert( - this.token.tokenURI(nonExistentTokenId), 'ERC721TokenUri: URI query for nonexistent token', + this.token.tokenURI(nonExistentTokenId), 'ERC721URIStorage: URI query for nonexistent token', ); }); @@ -42,7 +42,7 @@ contract('ERC721TokenUri', function (accounts) { it('reverts when setting for non existent token id', async function () { await expectRevert( - this.token.setTokenURI(nonExistentTokenId, sampleUri), 'ERC721TokenUri: URI set of nonexistent token', + this.token.setTokenURI(nonExistentTokenId, sampleUri), 'ERC721URIStorage: URI set of nonexistent token', ); }); @@ -80,7 +80,7 @@ contract('ERC721TokenUri', function (accounts) { expect(await this.token.exists(firstTokenId)).to.equal(false); await expectRevert( - this.token.tokenURI(firstTokenId), 'ERC721TokenUri: URI query for nonexistent token', + this.token.tokenURI(firstTokenId), 'ERC721URIStorage: URI query for nonexistent token', ); }); });