feat: add baseTokenURI to ERC721Metadata (#1970)

* feat: add baseTokenURI * fix: tests * chore: dev notation * chore: changelog * chore: typo * Remove extra getters, return empty URI by default * Update docs * Rename baseTokenURI to baseURI * Roll back visibility change of tokenURI * Update changelog entry * Version setBaseURI docs * Improve internal names and comments * Fix compilation errors * Add an external getter for baseURI
OpenZeppelin · Nov 15, 2019 · 49042f2 · 49042f2
1 parent 714f13d
commit 49042f2
Show file tree

Hide file tree

Showing 4 changed files with 108 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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))
 
 ### Improvements
  * `ERC777`: `_burn` is now internal, providing more flexibility and making it easier to create tokens that deflate. ([#1908](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/1908))

diff --git a/contracts/mocks/ERC721FullMock.sol b/contracts/mocks/ERC721FullMock.sol
@@ -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);
+    }
 }
diff --git a/contracts/token/ERC721/ERC721Metadata.sol b/contracts/token/ERC721/ERC721Metadata.sol
@@ -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;
 
@@ -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;
     }
 
     /**

diff --git a/test/token/ERC721/ERC721Full.test.js b/test/token/ERC721/ERC721Full.test.js
@@ -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);
       });
@@ -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'
+          );
+        });
       });
     });