ERC721 deploy ready fixes (#2192)

* Add baseURI to ERC721MinterPauser constructor

* Add tokenURI to mint

* Remove tokenId and tokenURI from ERC721 deploy ready

* Rename ERC721MinterPauser to ERC721MinterAutoID, remove Pausable mechanisms

* Restore pausing to ERC721

* Rename deploy ready to presets

* Rename ERC721 preset

contracts/deploy-ready/ERC20MinterPauser.sol → contracts/presets/ERC20PresetMinterPauser.sol

@@ -16,11 +16,11 @@ import "../token/ERC20/ERC20Pausable.sol";
  * This contract uses {AccessControl} to lock permissioned functions using the
  * different roles - head to its documentation for details.
  *
- * The account that deploys the contract will be granted the minter role, the
- * pauser role, and the default admin role, meaning it will be able to grant
- * both the minter and pauser roles.
+ * The account that deploys the contract will be granted the minter and pauser
+ * roles, as well as the default admin role, which will let it grant both minter
+ * and pauser roles to aother accounts
  */
-contract ERC20MinterPauser is Context, AccessControl, ERC20Burnable, ERC20Pausable {
+contract ERC20PresetMinterPauser is Context, AccessControl, ERC20Burnable, ERC20Pausable {
     bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
     bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");
 
@@ -47,7 +47,7 @@ contract ERC20MinterPauser is Context, AccessControl, ERC20Burnable, ERC20Pausab
      * - the caller must have the `MINTER_ROLE`.
      */
     function mint(address to, uint256 amount) public {
-        require(hasRole(MINTER_ROLE, _msgSender()), "ERC20MinterPauser: must have minter role to mint");
+        require(hasRole(MINTER_ROLE, _msgSender()), "ERC20PresetMinterPauser: must have minter role to mint");
         _mint(to, amount);
     }
 
@@ -61,7 +61,7 @@ contract ERC20MinterPauser is Context, AccessControl, ERC20Burnable, ERC20Pausab
      * - the caller must have the `PAUSER_ROLE`.
      */
     function pause() public {
-        require(hasRole(PAUSER_ROLE, _msgSender()), "ERC20MinterPauser: must have pauser role to pause");
+        require(hasRole(PAUSER_ROLE, _msgSender()), "ERC20PresetMinterPauser: must have pauser role to pause");
         _pause();
     }
 
@@ -75,7 +75,7 @@ contract ERC20MinterPauser is Context, AccessControl, ERC20Burnable, ERC20Pausab
      * - the caller must have the `PAUSER_ROLE`.
      */
     function unpause() public {
-        require(hasRole(PAUSER_ROLE, _msgSender()), "ERC20MinterPauser: must have pauser role to unpause");
+        require(hasRole(PAUSER_ROLE, _msgSender()), "ERC20PresetMinterPauser: must have pauser role to unpause");
         _unpause();
     }
 

contracts/deploy-ready/ERC721MinterPauser.sol → contracts/presets/ERC721PresetMinterPauserAutoId.sol

@@ -2,6 +2,7 @@ pragma solidity ^0.6.0;
 
 import "../access/AccessControl.sol";
 import "../GSN/Context.sol";
+import "../utils/Counters.sol";
 import "../token/ERC721/ERC721.sol";
 import "../token/ERC721/ERC721Burnable.sol";
 import "../token/ERC721/ERC721Pausable.sol";
@@ -12,33 +13,43 @@ import "../token/ERC721/ERC721Pausable.sol";
  *  - ability for holders to burn (destroy) their tokens
  *  - a minter role that allows for token minting (creation)
  *  - a pauser role that allows to stop all token transfers
+ *  - token ID and URI autogeneration
  *
  * This contract uses {AccessControl} to lock permissioned functions using the
  * different roles - head to its documentation for details.
  *
- * The account that deploys the contract will be granted the minter role, the
- * pauser role, and the default admin role, meaning it will be able to grant
- * both the minter and pauser roles.
+ * The account that deploys the contract will be granted the minter and pauser
+ * roles, as well as the default admin role, which will let it grant both minter
+ * and pauser roles to aother accounts
  */
-contract ERC721MinterPauser is Context, AccessControl, ERC721Burnable, ERC721Pausable {
+contract ERC721PresetMinterPauserAutoId is Context, AccessControl, ERC721Burnable, ERC721Pausable {
+    using Counters for Counters.Counter;
+
     bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
     bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");
 
+    Counters.Counter private _tokenIdTracker;
+
     /**
-     * @dev Grants `DEFAULT_ADMIN_ROLE`, `MINTER_ROLE` and `PAUSER_ROLE` to the
-     * account that deploys the contract.
+     * @dev Grants `DEFAULT_ADMIN_ROLE` and `MINTER_ROLE`to the account that
+     * deploys the contract.
      *
-     * See {ERC721-constructor}.
+     * Token URIs will be autogenerated based on `baseURI` and their token IDs.
+     * See {ERC721-tokenURI}.
      */
-    constructor(string memory name, string memory symbol) public ERC721(name, symbol) {
+    constructor(string memory name, string memory symbol, string memory baseURI) public ERC721(name, symbol) {
         _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
 
         _setupRole(MINTER_ROLE, _msgSender());
         _setupRole(PAUSER_ROLE, _msgSender());
+
+        _setBaseURI(baseURI);
     }
 
     /**
-     * @dev Creates the `tokenId` tokens for `to`.
+     * @dev Creates a new token for `to`. Its token ID will be automatically
+     * assigned (and available on the emitted {Transfer} event), and the token
+     * URI autogenerated based on the base URI passed at construction.
      *
      * See {ERC721-_mint}.
      *
@@ -46,9 +57,13 @@ contract ERC721MinterPauser is Context, AccessControl, ERC721Burnable, ERC721Pau
      *
      * - the caller must have the `MINTER_ROLE`.
      */
-    function mint(address to, uint256 tokenId) public {
-        require(hasRole(MINTER_ROLE, _msgSender()), "ERC721MinterPauser: must have minter role to mint");
-        _mint(to, tokenId);
+    function mint(address to) public {
+        require(hasRole(MINTER_ROLE, _msgSender()), "ERC721PresetMinterPauserAutoId: must have minter role to mint");
+
+        // We can just use balanceOf to create the new tokenId because tokens
+        // can be burned (destroyed), so we need a separate counter.
+        _mint(to, _tokenIdTracker.current());
+        _tokenIdTracker.increment();
     }
 
     /**
@@ -61,21 +76,21 @@ contract ERC721MinterPauser is Context, AccessControl, ERC721Burnable, ERC721Pau
      * - the caller must have the `PAUSER_ROLE`.
      */
     function pause() public {
-        require(hasRole(PAUSER_ROLE, _msgSender()), "ERC721MinterPauser: must have pauser role to pause");
+        require(hasRole(PAUSER_ROLE, _msgSender()), "ERC721PresetMinterPauserAutoId: must have pauser role to pause");
         _pause();
     }
 
     /**
      * @dev Unpauses all token transfers.
      *
-     * See {ERC20Pausable} and {Pausable-_unpause}.
+     * See {ERC721Pausable} and {Pausable-_unpause}.
      *
      * Requirements:
      *
      * - the caller must have the `PAUSER_ROLE`.
      */
     function unpause() public {
-        require(hasRole(PAUSER_ROLE, _msgSender()), "ERC721MinterPauser: must have pauser role to unpause");
+        require(hasRole(PAUSER_ROLE, _msgSender()), "ERC721PresetMinterPauserAutoId: must have pauser role to unpause");
         _unpause();
     }
 

contracts/deploy-ready/README.adoc → contracts/presets/README.adoc

@@ -1,4 +1,4 @@
-= Deploy Ready
+= Presets
 
 These contracts integrate different Ethereum standards (ERCs) with custom extensions and modules, showcasing common configurations that are ready to deploy **without having to write any Solidity code**.
 
@@ -8,6 +8,6 @@ TIP: Intermediate and advanced users can use these as starting points when writi
 
 == Tokens
 
-{{ERC20MinterPauser}}
+{{ERC20PresetMinterPauser}}
 
-{{ERC721MinterPauser}}
+{{ERC721PresetMinterPauserAutoId}}

contracts/token/ERC721/ERC721.sol

@@ -134,11 +134,33 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Enumerable
     /**
      * @dev Returns the URI for a given token ID. May return an empty string.
      *
-     * If no base URI was set (via {_setBaseURI}), return the token ID's URI.
-     * If a base URI was set, it will be added as a prefix to the token ID's URI,
-     * or to the token ID itself, if no URI is set for that token ID.
+     * If a base URI is set (via {_setBaseURI}), it is added as a prefix to the
+     * token's own URI (via {_setTokenURI}).
      *
-     * Reverts if the token ID does not exist.
+     * If there is a base URI but no token URI, the token's ID will be used as
+     * its URI when appending it to the base URI. This pattern for autogenerated
+     * token URIs can lead to large gas savings.
+     *
+     * .Examples
+     * |===
+     * |`_setBaseURI()` |`_setTokenURI()` |`tokenURI()`
+     * | ""
+     * | ""
+     * | ""
+     * | ""
+     * | "token.uri/123"
+     * | "token.uri/123"
+     * | "token.uri/"
+     * | "123"
+     * | "token.uri/123"
+     * | "token.uri/"
+     * | ""
+     * | "token.uri/<tokenId>"
+     * |===
+     *
+     * Requirements:
+     *
+     * - `tokenId` must exist.
      */
     function tokenURI(uint256 tokenId) public view override returns (string memory) {
         require(_exists(tokenId), "ERC721Metadata: URI query for nonexistent token");

test/deploy-ready/ERC20MinterPauser.test.js → test/presets/ERC20PresetMinterPauser.test.js

@@ -5,9 +5,9 @@ const { ZERO_ADDRESS } = constants;
 
 const { expect } = require('chai');
 
-const ERC20MinterPauser = contract.fromArtifact('ERC20MinterPauser');
+const ERC20PresetMinterPauser = contract.fromArtifact('ERC20PresetMinterPauser');
 
-describe('ERC20MinterPauser', function () {
+describe('ERC20PresetMinterPauser', function () {
   const [ deployer, other ] = accounts;
 
   const name = 'MinterPauserToken';
@@ -20,7 +20,7 @@ describe('ERC20MinterPauser', function () {
   const PAUSER_ROLE = web3.utils.soliditySha3('PAUSER_ROLE');
 
   beforeEach(async function () {
-    this.token = await ERC20MinterPauser.new(name, symbol, { from: deployer });
+    this.token = await ERC20PresetMinterPauser.new(name, symbol, { from: deployer });
   });
 
   it('deployer has the default admin role', async function () {
@@ -54,7 +54,7 @@ describe('ERC20MinterPauser', function () {
     it('other accounts cannot mint tokens', async function () {
       await expectRevert(
         this.token.mint(other, amount, { from: other }),
-        'ERC20MinterPauser: must have minter role to mint'
+        'ERC20PresetMinterPauser: must have minter role to mint'
       );
     });
   });
@@ -86,7 +86,7 @@ describe('ERC20MinterPauser', function () {
     });
 
     it('other accounts cannot pause', async function () {
-      await expectRevert(this.token.pause({ from: other }), 'ERC20MinterPauser: must have pauser role to pause');
+      await expectRevert(this.token.pause({ from: other }), 'ERC20PresetMinterPauser: must have pauser role to pause');
     });
   });
 

test/deploy-ready/ERC721MinterPauser.test.js → test/presets/ERC721PresetMinterPauserAutoId.js

@@ -5,22 +5,32 @@ const { ZERO_ADDRESS } = constants;
 
 const { expect } = require('chai');
 
-const ERC721MinterPauser = contract.fromArtifact('ERC721MinterPauser');
+const ERC721PresetMinterPauserAutoId = contract.fromArtifact('ERC721PresetMinterPauserAutoId');
 
-describe('ERC721MinterPauser', function () {
+describe('ERC721PresetMinterPauserAutoId', function () {
   const [ deployer, other ] = accounts;
 
-  const name = 'MinterPauserToken';
-  const symbol = 'DRT';
-
-  const tokenId = new BN('1337');
+  const name = 'MinterAutoIDToken';
+  const symbol = 'MAIT';
+  const baseURI = 'my.app/';
 
   const DEFAULT_ADMIN_ROLE = '0x0000000000000000000000000000000000000000000000000000000000000000';
   const MINTER_ROLE = web3.utils.soliditySha3('MINTER_ROLE');
-  const PAUSER_ROLE = web3.utils.soliditySha3('PAUSER_ROLE');
 
   beforeEach(async function () {
-    this.token = await ERC721MinterPauser.new(name, symbol, { from: deployer });
+    this.token = await ERC721PresetMinterPauserAutoId.new(name, symbol, baseURI, { from: deployer });
+  });
+
+  it('token has correct name', async function () {
+    expect(await this.token.name()).to.equal(name);
+  });
+
+  it('token has correct symbol', async function () {
+    expect(await this.token.symbol()).to.equal(symbol);
+  });
+
+  it('token has correct base URI', async function () {
+    expect(await this.token.baseURI()).to.equal(baseURI);
   });
 
   it('deployer has the default admin role', async function () {
@@ -33,29 +43,27 @@ describe('ERC721MinterPauser', function () {
     expect(await this.token.getRoleMember(MINTER_ROLE, 0)).to.equal(deployer);
   });
 
-  it('deployer has the pauser role', async function () {
-    expect(await this.token.getRoleMemberCount(PAUSER_ROLE)).to.be.bignumber.equal('1');
-    expect(await this.token.getRoleMember(PAUSER_ROLE, 0)).to.equal(deployer);
-  });
-
-  it('minter and pauser role admin is the default admin', async function () {
+  it('minter role admin is the default admin', async function () {
     expect(await this.token.getRoleAdmin(MINTER_ROLE)).to.equal(DEFAULT_ADMIN_ROLE);
-    expect(await this.token.getRoleAdmin(PAUSER_ROLE)).to.equal(DEFAULT_ADMIN_ROLE);
   });
 
   describe('minting', function () {
     it('deployer can mint tokens', async function () {
-      const receipt = await this.token.mint(other, tokenId, { from: deployer });
+      const tokenId = new BN('0');
+
+      const receipt = await this.token.mint(other, { from: deployer });
       expectEvent(receipt, 'Transfer', { from: ZERO_ADDRESS, to: other, tokenId });
 
       expect(await this.token.balanceOf(other)).to.be.bignumber.equal('1');
       expect(await this.token.ownerOf(tokenId)).to.equal(other);
+
+      expect(await this.token.tokenURI(tokenId)).to.equal(baseURI + tokenId);
     });
 
     it('other accounts cannot mint tokens', async function () {
       await expectRevert(
-        this.token.mint(other, tokenId, { from: other }),
-        'ERC721MinterPauser: must have minter role to mint'
+        this.token.mint(other, { from: other }),
+        'ERC721PresetMinterPauserAutoId: must have minter role to mint'
       );
     });
   });
@@ -81,19 +89,24 @@ describe('ERC721MinterPauser', function () {
       await this.token.pause({ from: deployer });
 
       await expectRevert(
-        this.token.mint(other, tokenId, { from: deployer }),
+        this.token.mint(other, { from: deployer }),
         'ERC721Pausable: token transfer while paused'
       );
     });
 
     it('other accounts cannot pause', async function () {
-      await expectRevert(this.token.pause({ from: other }), 'ERC721MinterPauser: must have pauser role to pause');
+      await expectRevert(
+        this.token.pause({ from: other }),
+        'ERC721PresetMinterPauserAutoId: must have pauser role to pause'
+      );
     });
   });
 
   describe('burning', function () {
     it('holders can burn their tokens', async function () {
-      await this.token.mint(other, tokenId, { from: deployer });
+      const tokenId = new BN('0');
+
+      await this.token.mint(other, { from: deployer });
 
       const receipt = await this.token.burn(tokenId, { from: other });