Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ERC721URIStorage extension #2555

Merged
merged 3 commits into from
Mar 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 12 additions & 0 deletions contracts/mocks/ERC721BurnableMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these changes related to the PR? I don't understand why they're here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some ERC721 mocks make all internal functions available, other don't. I wanted to improve consistency by making all the mocks show all the functions.

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);
}
}
4 changes: 4 additions & 0 deletions contracts/mocks/ERC721EnumerableMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
26 changes: 17 additions & 9 deletions contracts/mocks/ERC721PausableMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
51 changes: 51 additions & 0 deletions contracts/mocks/ERC721URIStorageMock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../token/ERC721/extensions/ERC721URIStorage.sol";

/**
* @title ERC721Mock
* This mock just provides a public safeMint, mint, and burn functions for testing purposes
*/
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 {
_baseTokenURI = newBaseTokenURI;
}

function baseURI() public view returns (string memory) {
return _baseURI();
}

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);
}
}
2 changes: 2 additions & 0 deletions contracts/token/ERC721/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
66 changes: 66 additions & 0 deletions contracts/token/ERC721/extensions/ERC721URIStorage.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../ERC721.sol";

/**
* @dev ERC721 token with storage based token uri management.
*/
abstract contract ERC721URIStorage is ERC721 {
using Strings for uint256;

// Optional mapping for token URIs
mapping (uint256 => string) private _tokenURIs;

/**
* @dev See {IERC721Metadata-tokenURI}.
*/
function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
require(_exists(tokenId), "ERC721URIStorage: 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));
}

return super.tokenURI(tokenId);
}

/**
* @dev Sets `_tokenURI` as the tokenURI of `tokenId`.
*
* Requirements:
*
* - `tokenId` must exist.
*/
function _setTokenURI(uint256 tokenId, string memory _tokenURI) internal virtual {
require(_exists(tokenId), "ERC721URIStorage: 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];
}
}
}
87 changes: 87 additions & 0 deletions test/token/ERC721/extensions/ERC721URIStorage.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
const { BN, expectRevert } = require('@openzeppelin/test-helpers');

const { expect } = require('chai');

const ERC721URIStorageMock = artifacts.require('ERC721URIStorageMock');

contract('ERC721URIStorage', 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 ERC721URIStorageMock.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), 'ERC721URIStorage: 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), 'ERC721URIStorage: 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), 'ERC721URIStorage: URI query for nonexistent token',
);
});
});
});