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

feat: add baseTokenURI to ERC721Metadata #1970

Merged
merged 15 commits into from
Nov 15, 2019
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### New features:
* `SafeCast.toUintXX`: new library for integer downcasting, which allows for safe operation on smaller types (e.g. `uint32`) when combined with `SafeMath`. ([#1926](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1926))
* `ERC721Metadata`: added `baseURI`, which can be used for dramatic gas savings when all token URIs share a prefix (e.g. `http://api.myapp.com/tokens/<id>`). ([#1970](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1970))

### Breaking changes:
* `ERC165Checker` now requires a minimum Solidity compiler version of 0.5.10. ([#1829](https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1829))
Expand Down
4 changes: 4 additions & 0 deletions contracts/mocks/ERC721FullMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,8 @@ contract ERC721FullMock is ERC721Full, ERC721Mintable, ERC721MetadataMintable, E
function setTokenURI(uint256 tokenId, string memory uri) public {
_setTokenURI(tokenId, uri);
}

function setBaseURI(string memory baseURI) public {
_setBaseURI(baseURI);
}
}
53 changes: 45 additions & 8 deletions contracts/token/ERC721/ERC721Metadata.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ contract ERC721Metadata is Context, ERC165, ERC721, IERC721Metadata {
// Token symbol
string private _symbol;

// Base URI
string private _baseURI;

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

Expand Down Expand Up @@ -52,24 +55,58 @@ contract ERC721Metadata is Context, ERC165, ERC721, IERC721Metadata {
}

/**
* @dev Returns an URI for a given token ID.
* Throws if the token ID does not exist. May return an empty string.
* @param tokenId uint256 ID of the token to query
* @dev Returns the URI for a given token ID. May return an empty string.
*
* If the token's URI is non-empty and a base URI was set (via
* {_setBaseURI}), it will be added to the token ID's URI as a prefix.
*
* Reverts if the token ID does not exist.
*/
function tokenURI(uint256 tokenId) external view returns (string memory) {
require(_exists(tokenId), "ERC721Metadata: URI query for nonexistent token");
return _tokenURIs[tokenId];

string memory _tokenURI = _tokenURIs[tokenId];

// Even if there is a base URI, it is only appended to non-empty token-specific URIs
if (bytes(_tokenURI).length == 0) {
return "";
} else {
// abi.encodePacked is being used to concatenate strings
return string(abi.encodePacked(_baseURI, _tokenURI));
}
}

/**
* @dev Returns the base URI set via {_setBaseURI}. This will be
* automatically added as a preffix in {tokenURI} to each token's URI, when
* they are non-empty.
*/
function baseURI() external view returns (string memory) {
return _baseURI;
}

/**
* @dev Internal function to set the token URI for a given token.
*
* Reverts if the token ID does not exist.
* @param tokenId uint256 ID of the token to set its URI
* @param uri string URI to assign
*
* TIP: if all token IDs share a prefix (e.g. if your URIs look like
* `http://api.myproject.com/token/<id>`), use {_setBaseURI} to store
* it and save gas.
*/
function _setTokenURI(uint256 tokenId, string memory uri) internal {
function _setTokenURI(uint256 tokenId, string memory _tokenURI) internal {
require(_exists(tokenId), "ERC721Metadata: URI set of nonexistent token");
_tokenURIs[tokenId] = uri;
_tokenURIs[tokenId] = _tokenURI;
}

/**
* @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}.
*
* _Available since v2.5.0._
*/
function _setBaseURI(string memory baseURI) internal {
_baseURI = baseURI;
}

/**
Expand Down
81 changes: 58 additions & 23 deletions test/token/ERC721/ERC721Full.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ contract('ERC721Full', function ([
});

describe('metadata', function () {
const sampleUri = 'mock://mytoken';

it('has a name', async function () {
expect(await this.token.name()).to.be.equal(name);
});
Expand All @@ -82,31 +80,68 @@ contract('ERC721Full', function ([
expect(await this.token.symbol()).to.be.equal(symbol);
});

it('sets and returns metadata for a token id', async function () {
await this.token.setTokenURI(firstTokenId, sampleUri);
expect(await this.token.tokenURI(firstTokenId)).to.be.equal(sampleUri);
});
describe('token URI', function () {
const baseURI = 'https://api.com/v1/';
const sampleUri = 'mock://mytoken';

it('reverts when setting metadata for non existent token id', async function () {
await expectRevert(
this.token.setTokenURI(nonExistentTokenId, sampleUri), 'ERC721Metadata: URI set of nonexistent token'
);
});
it('it is empty by default', async function () {
expect(await this.token.tokenURI(firstTokenId)).to.be.equal('');
});

it('can burn token with metadata', async function () {
await this.token.setTokenURI(firstTokenId, sampleUri);
await this.token.burn(firstTokenId, { from: owner });
expect(await this.token.exists(firstTokenId)).to.equal(false);
});
it('reverts when queried for non existent token id', async function () {
await expectRevert(
this.token.tokenURI(nonExistentTokenId), 'ERC721Metadata: URI query for nonexistent token'
);
});

it('returns empty metadata for token', async function () {
expect(await this.token.tokenURI(firstTokenId)).to.be.equal('');
});
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 querying metadata for non existent token id', async function () {
await expectRevert(
this.token.tokenURI(nonExistentTokenId), 'ERC721Metadata: URI query for nonexistent token'
);
it('reverts when setting for non existent token id', async function () {
await expectRevert(
this.token.setTokenURI(nonExistentTokenId, sampleUri), 'ERC721Metadata: 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('token URI is empty for tokens with no URI but with base URI', async function () {
await this.token.setBaseURI(baseURI);

expect(await this.token.tokenURI(firstTokenId)).to.be.equal('');
});

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), 'ERC721Metadata: URI query for nonexistent token'
);
});
});
});

Expand Down