feat: implement ERC721Mintable and ERC721Burnable (#1276)

* feat: implement ERC721Mintable and ERC721Burnable

* fix: linting errors

* fix: remove unused mintable mock for ERC721BasicMock

* fix: add finishMinting tests

* fix: catch MintFinished typo

* inline ERC721Full behavior

* undo pretty formatting

* fix lint errors

* rename canMint to onlyBeforeMintingFinished for consistency with ERC20Mintable

contracts/mocks/ERC721BasicMock.sol

@@ -9,10 +9,10 @@ import "../token/ERC721/ERC721Basic.sol";
  */
 contract ERC721BasicMock is ERC721Basic {
   function mint(address _to, uint256 _tokenId) public {
-    super._mint(_to, _tokenId);
+    _mint(_to, _tokenId);
   }
 
   function burn(uint256 _tokenId) public {
-    super._burn(ownerOf(_tokenId), _tokenId);
+    _burn(ownerOf(_tokenId), _tokenId);
   }
 }

contracts/mocks/ERC721MintableBurnableImpl.sol

@@ -0,0 +1,18 @@
+pragma solidity ^0.4.24;
+
+import "../token/ERC721/ERC721.sol";
+import "../token/ERC721/ERC721Mintable.sol";
+import "../token/ERC721/ERC721Burnable.sol";
+
+
+/**
+ * @title ERC721MintableBurnableImpl
+ */
+contract ERC721MintableBurnableImpl is ERC721, ERC721Mintable, ERC721Burnable {
+  constructor()
+    ERC721Mintable()
+    ERC721("Test", "TEST")
+    public
+  {
+  }
+}

contracts/mocks/ERC721Mock.sol

@@ -1,25 +1,20 @@
 pragma solidity ^0.4.24;
 
 import "../token/ERC721/ERC721.sol";
+import "../token/ERC721/ERC721Mintable.sol";
+import "../token/ERC721/ERC721Burnable.sol";
 
 
 /**
  * @title ERC721Mock
  * This mock just provides a public mint and burn functions for testing purposes,
  * and a public setter for metadata URI
  */
-contract ERC721Mock is ERC721 {
-  constructor(string name, string symbol) public
-    ERC721(name, symbol)
-  { }
-
-  function mint(address _to, uint256 _tokenId) public {
-    _mint(_to, _tokenId);
-  }
-
-  function burn(uint256 _tokenId) public {
-    _burn(ownerOf(_tokenId), _tokenId);
-  }
+contract ERC721Mock is ERC721, ERC721Mintable, ERC721Burnable {
+  constructor(string _name, string _symbol) public
+    ERC721Mintable()
+    ERC721(_name, _symbol)
+  {}
 
   function exists(uint256 _tokenId) public view returns (bool) {
     return _exists(_tokenId);

contracts/token/ERC20/ERC20Mintable.sol

@@ -5,13 +5,12 @@ import "../../access/roles/MinterRole.sol";
 
 
 /**
- * @title Mintable token
- * @dev Simple ERC20 Token example, with mintable token creation
- * Based on code by TokenMarketNet: https://github.com/TokenMarketNet/ico/blob/master/contracts/MintableToken.sol
+ * @title ERC20Mintable
+ * @dev ERC20 minting logic
  */
 contract ERC20Mintable is ERC20, MinterRole {
-  event Mint(address indexed to, uint256 amount);
-  event MintFinished();
+  event Minted(address indexed to, uint256 amount);
+  event MintingFinished();
 
   bool private mintingFinished_ = false;
 
@@ -43,7 +42,7 @@ contract ERC20Mintable is ERC20, MinterRole {
     returns (bool)
   {
     _mint(_to, _amount);
-    emit Mint(_to, _amount);
+    emit Minted(_to, _amount);
     return true;
   }
 
@@ -58,7 +57,7 @@ contract ERC20Mintable is ERC20, MinterRole {
     returns (bool)
   {
     mintingFinished_ = true;
-    emit MintFinished();
+    emit MintingFinished();
     return true;
   }
 }

contracts/token/ERC721/ERC721Burnable.sol

@@ -0,0 +1,13 @@
+pragma solidity ^0.4.24;
+
+import "./ERC721.sol";
+
+
+contract ERC721Burnable is ERC721 {
+  function burn(uint256 _tokenId)
+    public
+  {
+    require(_isApprovedOrOwner(msg.sender, _tokenId));
+    _burn(ownerOf(_tokenId), _tokenId);
+  }
+}

contracts/token/ERC721/ERC721Mintable.sol

@@ -0,0 +1,71 @@
+pragma solidity ^0.4.24;
+
+import "./ERC721.sol";
+import "../../access/roles/MinterRole.sol";
+
+
+/**
+ * @title ERC721Mintable
+ * @dev ERC721 minting logic
+ */
+contract ERC721Mintable is ERC721, MinterRole {
+  event Minted(address indexed to, uint256 tokenId);
+  event MintingFinished();
+
+  bool public mintingFinished = false;
+
+  modifier onlyBeforeMintingFinished() {
+    require(!mintingFinished);
+    _;
+  }
+
+  /**
+   * @dev Function to mint tokens
+   * @param _to The address that will receive the minted tokens.
+   * @param _tokenId The token id to mint.
+   * @return A boolean that indicates if the operation was successful.
+   */
+  function mint(
+    address _to,
+    uint256 _tokenId
+  )
+    public
+    onlyMinter
+    onlyBeforeMintingFinished
+    returns (bool)
+  {
+    _mint(_to, _tokenId);
+    emit Minted(_to, _tokenId);
+    return true;
+  }
+
+  function mintWithTokenURI(
+    address _to,
+    uint256 _tokenId,
+    string _tokenURI
+  )
+    public
+    onlyMinter
+    onlyBeforeMintingFinished
+    returns (bool)
+  {
+    mint(_to, _tokenId);
+    _setTokenURI(_tokenId, _tokenURI);
+    return true;
+  }
+
+  /**
+   * @dev Function to stop minting new tokens.
+   * @return True if the operation was successful.
+   */
+  function finishMinting()
+    public
+    onlyMinter
+    onlyBeforeMintingFinished
+    returns (bool)
+  {
+    mintingFinished = true;
+    emit MintingFinished();
+    return true;
+  }
+}

test/token/ERC20/ERC20Capped.behavior.js

@@ -17,7 +17,7 @@ function shouldBehaveLikeERC20Capped (minter, [anyone], cap) {
 
     it('should mint when amount is less than cap', async function () {
       const { logs } = await this.token.mint(anyone, cap.sub(1), { from });
-      expectEvent.inLogs(logs, 'Mint');
+      expectEvent.inLogs(logs, 'Minted');
     });
 
     it('should fail to mint if the ammount exceeds the cap', async function () {

test/token/ERC20/ERC20Mintable.behavior.js

@@ -43,8 +43,7 @@ function shouldBehaveLikeERC20Mintable (minter, [anyone]) {
           it('emits a mint finished event', async function () {
             const { logs } = await this.token.finishMinting({ from });
 
-            logs.length.should.be.equal(1);
-            logs[0].event.should.equal('MintFinished');
+            await expectEvent.inLogs(logs, 'MintingFinished');
           });
         });
 
@@ -105,7 +104,7 @@ function shouldBehaveLikeERC20Mintable (minter, [anyone]) {
             });
 
             it('emits a mint and a transfer event', async function () {
-              const mintEvent = expectEvent.inLogs(this.logs, 'Mint', {
+              const mintEvent = expectEvent.inLogs(this.logs, 'Minted', {
                 to: anyone,
               });
               mintEvent.args.amount.should.be.bignumber.equal(amount);

test/token/ERC721/ERC721.test.js

@@ -11,63 +11,65 @@ require('chai')
   .use(require('chai-bignumber')(BigNumber))
   .should();
 
-contract('ERC721', function (accounts) {
+contract('ERC721', function ([
+  creator,
+  ...accounts
+]) {
   const name = 'Non Fungible Token';
   const symbol = 'NFT';
   const firstTokenId = 100;
   const secondTokenId = 200;
+  const thirdTokenId = 300;
   const nonExistentTokenId = 999;
-  const creator = accounts[0];
-  const anyone = accounts[9];
+
+  const minter = creator;
+
+  const [
+    owner,
+    newOwner,
+    another,
+    anyone,
+  ] = accounts;
 
   beforeEach(async function () {
     this.token = await ERC721.new(name, symbol, { from: creator });
   });
 
-  shouldBehaveLikeERC721Basic(accounts);
-  shouldBehaveLikeMintAndBurnERC721(accounts);
-
   describe('like a full ERC721', function () {
     beforeEach(async function () {
-      await this.token.mint(creator, firstTokenId, { from: creator });
-      await this.token.mint(creator, secondTokenId, { from: creator });
+      await this.token.mint(owner, firstTokenId, { from: minter });
+      await this.token.mint(owner, secondTokenId, { from: minter });
     });
 
     describe('mint', function () {
-      const to = accounts[1];
-      const tokenId = 3;
-
       beforeEach(async function () {
-        await this.token.mint(to, tokenId);
+        await this.token.mint(newOwner, thirdTokenId, { from: minter });
       });
 
       it('adjusts owner tokens by index', async function () {
-        (await this.token.tokenOfOwnerByIndex(to, 0)).toNumber().should.be.equal(tokenId);
+        (await this.token.tokenOfOwnerByIndex(newOwner, 0)).toNumber().should.be.equal(thirdTokenId);
       });
 
       it('adjusts all tokens list', async function () {
-        (await this.token.tokenByIndex(2)).toNumber().should.be.equal(tokenId);
+        (await this.token.tokenByIndex(2)).toNumber().should.be.equal(thirdTokenId);
       });
     });
 
     describe('burn', function () {
-      const tokenId = firstTokenId;
-      const sender = creator;
-
       beforeEach(async function () {
-        await this.token.burn(tokenId, { from: sender });
+        await this.token.burn(firstTokenId, { from: owner });
       });
 
       it('removes that token from the token list of the owner', async function () {
-        (await this.token.tokenOfOwnerByIndex(sender, 0)).toNumber().should.be.equal(secondTokenId);
+        (await this.token.tokenOfOwnerByIndex(owner, 0)).toNumber().should.be.equal(secondTokenId);
       });
 
       it('adjusts all tokens list', async function () {
         (await this.token.tokenByIndex(0)).toNumber().should.be.equal(secondTokenId);
       });
 
       it('burns all tokens', async function () {
-        await this.token.burn(secondTokenId, { from: sender });
+        await this.token.burn(secondTokenId, { from: owner });
         (await this.token.totalSupply()).toNumber().should.be.equal(0);
         await assertRevert(this.token.tokenByIndex(0));
       });
@@ -76,25 +78,25 @@ contract('ERC721', function (accounts) {
     describe('removeTokenFrom', function () {
       it('reverts if the correct owner is not passed', async function () {
         await assertRevert(
-          this.token.removeTokenFrom(anyone, firstTokenId, { from: creator })
+          this.token.removeTokenFrom(anyone, firstTokenId, { from: owner })
         );
       });
 
       context('once removed', function () {
         beforeEach(async function () {
-          await this.token.removeTokenFrom(creator, firstTokenId, { from: creator });
+          await this.token.removeTokenFrom(owner, firstTokenId, { from: owner });
         });
 
         it('has been removed', async function () {
-          await assertRevert(this.token.tokenOfOwnerByIndex(creator, 1));
+          await assertRevert(this.token.tokenOfOwnerByIndex(owner, 1));
         });
 
         it('adjusts token list', async function () {
-          (await this.token.tokenOfOwnerByIndex(creator, 0)).toNumber().should.be.equal(secondTokenId);
+          (await this.token.tokenOfOwnerByIndex(owner, 0)).toNumber().should.be.equal(secondTokenId);
         });
 
         it('adjusts owner count', async function () {
-          (await this.token.balanceOf(creator)).toNumber().should.be.equal(1);
+          (await this.token.balanceOf(owner)).toNumber().should.be.equal(1);
         });
 
         it('does not adjust supply', async function () {
@@ -125,7 +127,7 @@ contract('ERC721', function (accounts) {
 
       it('can burn token with metadata', async function () {
         await this.token.setTokenURI(firstTokenId, sampleUri);
-        await this.token.burn(firstTokenId);
+        await this.token.burn(firstTokenId, { from: owner });
         (await this.token.exists(firstTokenId)).should.equal(false);
       });
 
@@ -145,9 +147,6 @@ contract('ERC721', function (accounts) {
     });
 
     describe('tokenOfOwnerByIndex', function () {
-      const owner = creator;
-      const another = accounts[1];
-
       describe('when the given index is lower than the amount of tokens owned by the given address', function () {
         it('returns the token ID placed at the given index', async function () {
           (await this.token.tokenOfOwnerByIndex(owner, 0)).should.be.bignumber.equal(firstTokenId);
@@ -197,13 +196,12 @@ contract('ERC721', function (accounts) {
 
       [firstTokenId, secondTokenId].forEach(function (tokenId) {
         it(`should return all tokens after burning token ${tokenId} and minting new tokens`, async function () {
-          const owner = accounts[0];
           const newTokenId = 300;
           const anotherNewTokenId = 400;
 
           await this.token.burn(tokenId, { from: owner });
-          await this.token.mint(owner, newTokenId, { from: owner });
-          await this.token.mint(owner, anotherNewTokenId, { from: owner });
+          await this.token.mint(newOwner, newTokenId, { from: minter });
+          await this.token.mint(newOwner, anotherNewTokenId, { from: minter });
 
           (await this.token.totalSupply()).toNumber().should.be.equal(3);
 
@@ -218,6 +216,9 @@ contract('ERC721', function (accounts) {
     });
   });
 
+  shouldBehaveLikeERC721Basic(creator, minter, accounts);
+  shouldBehaveLikeMintAndBurnERC721(creator, minter, accounts);
+
   shouldSupportInterfaces([
     'ERC165',
     'ERC721',