Adding RBAC Mintable token (#923)

* added the RBACMintableToken
* added MintedCrowdsale with RBACMintableToken test
* added a mintable behaviour for tests
* moved minting tests in behaviour
* created a minted crowdsale behaviour to be tested with both mintable and rbacmintable token

contracts/token/ERC20/MintableToken.sol

@@ -22,13 +22,18 @@ contract MintableToken is StandardToken, Ownable {
     _;
   }
 
+  modifier hasMintPermission() {
+    require(msg.sender == owner);
+    _;
+  }
+
   /**
    * @dev Function to mint tokens
    * @param _to The address that will receive the minted tokens.
    * @param _amount The amount of tokens to mint.
    * @return A boolean that indicates if the operation was successful.
    */
-  function mint(address _to, uint256 _amount) onlyOwner canMint public returns (bool) {
+  function mint(address _to, uint256 _amount) hasMintPermission canMint public returns (bool) {
     totalSupply_ = totalSupply_.add(_amount);
     balances[_to] = balances[_to].add(_amount);
     emit Mint(_to, _amount);

contracts/token/ERC20/RBACMintableToken.sol

@@ -0,0 +1,41 @@
+pragma solidity ^0.4.23;
+
+import "./MintableToken.sol";
+import "../../ownership/rbac/RBAC.sol";
+
+
+/**
+ * @title RBACMintableToken
+ * @author Vittorio Minacori (@vittominacori)
+ * @dev Mintable Token, with RBAC minter permissions
+ */
+contract RBACMintableToken is MintableToken, RBAC {
+  /**
+   * A constant role name for indicating minters.
+   */
+  string public constant ROLE_MINTER = "minter";
+
+  /**
+   * @dev override the Mintable token modifier to add role based logic
+   */
+  modifier hasMintPermission() {
+    checkRole(msg.sender, ROLE_MINTER);
+    _;
+  }
+
+  /**
+   * @dev add a minter role to an address
+   * @param minter address
+   */
+  function addMinter(address minter) onlyOwner public {
+    addRole(minter, ROLE_MINTER);
+  }
+
+  /**
+   * @dev remove a minter role from an address
+   * @param minter address
+   */
+  function removeMinter(address minter) onlyOwner public {
+    removeRole(minter, ROLE_MINTER);
+  }
+}

test/crowdsale/MintedCrowdsale.behaviour.js

@@ -0,0 +1,44 @@
+const BigNumber = web3.BigNumber;
+
+const should = require('chai')
+  .use(require('chai-as-promised'))
+  .use(require('chai-bignumber')(BigNumber))
+  .should();
+
+export default function ([_, investor, wallet, purchaser], rate, value) {
+  const expectedTokenAmount = rate.mul(value);
+
+  describe('as a minted crowdsale', function () {
+    describe('accepting payments', function () {
+      it('should accept payments', async function () {
+        await this.crowdsale.send(value).should.be.fulfilled;
+        await this.crowdsale.buyTokens(investor, { value: value, from: purchaser }).should.be.fulfilled;
+      });
+    });
+
+    describe('high-level purchase', function () {
+      it('should log purchase', async function () {
+        const { logs } = await this.crowdsale.sendTransaction({ value: value, from: investor });
+        const event = logs.find(e => e.event === 'TokenPurchase');
+        should.exist(event);
+        event.args.purchaser.should.equal(investor);
+        event.args.beneficiary.should.equal(investor);
+        event.args.value.should.be.bignumber.equal(value);
+        event.args.amount.should.be.bignumber.equal(expectedTokenAmount);
+      });
+
+      it('should assign tokens to sender', async function () {
+        await this.crowdsale.sendTransaction({ value: value, from: investor });
+        let balance = await this.token.balanceOf(investor);
+        balance.should.be.bignumber.equal(expectedTokenAmount);
+      });
+
+      it('should forward funds to wallet', async function () {
+        const pre = web3.eth.getBalance(wallet);
+        await this.crowdsale.sendTransaction({ value, from: investor });
+        const post = web3.eth.getBalance(wallet);
+        post.minus(pre).should.be.bignumber.equal(value);
+      });
+    });
+  });
+}

test/crowdsale/MintedCrowdsale.test.js

@@ -1,61 +1,45 @@
+import shouldBehaveLikeMintedCrowdsale from './MintedCrowdsale.behaviour';
 import ether from '../helpers/ether';
 
 const BigNumber = web3.BigNumber;
 
-const should = require('chai')
-  .use(require('chai-as-promised'))
-  .use(require('chai-bignumber')(BigNumber))
-  .should();
-
 const MintedCrowdsale = artifacts.require('MintedCrowdsaleImpl');
 const MintableToken = artifacts.require('MintableToken');
+const RBACMintableToken = artifacts.require('RBACMintableToken');
 
 contract('MintedCrowdsale', function ([_, investor, wallet, purchaser]) {
   const rate = new BigNumber(1000);
-  const value = ether(42);
-
-  const expectedTokenAmount = rate.mul(value);
+  const value = ether(5);
 
-  beforeEach(async function () {
-    this.token = await MintableToken.new();
-    this.crowdsale = await MintedCrowdsale.new(rate, wallet, this.token.address);
-    await this.token.transferOwnership(this.crowdsale.address);
-  });
+  describe('using MintableToken', function () {
+    beforeEach(async function () {
+      this.token = await MintableToken.new();
+      this.crowdsale = await MintedCrowdsale.new(rate, wallet, this.token.address);
+      await this.token.transferOwnership(this.crowdsale.address);
+    });
 
-  describe('accepting payments', function () {
     it('should be token owner', async function () {
       const owner = await this.token.owner();
       owner.should.equal(this.crowdsale.address);
     });
 
-    it('should accept payments', async function () {
-      await this.crowdsale.send(value).should.be.fulfilled;
-      await this.crowdsale.buyTokens(investor, { value: value, from: purchaser }).should.be.fulfilled;
-    });
+    shouldBehaveLikeMintedCrowdsale([_, investor, wallet, purchaser], rate, value);
   });
 
-  describe('high-level purchase', function () {
-    it('should log purchase', async function () {
-      const { logs } = await this.crowdsale.sendTransaction({ value: value, from: investor });
-      const event = logs.find(e => e.event === 'TokenPurchase');
-      should.exist(event);
-      event.args.purchaser.should.equal(investor);
-      event.args.beneficiary.should.equal(investor);
-      event.args.value.should.be.bignumber.equal(value);
-      event.args.amount.should.be.bignumber.equal(expectedTokenAmount);
-    });
+  describe('using RBACMintableToken', function () {
+    const ROLE_MINTER = 'minter';
 
-    it('should assign tokens to sender', async function () {
-      await this.crowdsale.sendTransaction({ value: value, from: investor });
-      let balance = await this.token.balanceOf(investor);
-      balance.should.be.bignumber.equal(expectedTokenAmount);
+    beforeEach(async function () {
+      this.token = await RBACMintableToken.new();
+      this.crowdsale = await MintedCrowdsale.new(rate, wallet, this.token.address);
+      await this.token.addMinter(this.crowdsale.address);
     });
 
-    it('should forward funds to wallet', async function () {
-      const pre = web3.eth.getBalance(wallet);
-      await this.crowdsale.sendTransaction({ value, from: investor });
-      const post = web3.eth.getBalance(wallet);
-      post.minus(pre).should.be.bignumber.equal(value);
+    it('should have minter role on token', async function () {
+      const isMinter = await this.token.hasRole(this.crowdsale.address, ROLE_MINTER);
+      isMinter.should.equal(true);
     });
+
+    shouldBehaveLikeMintedCrowdsale([_, investor, wallet, purchaser], rate, value);
   });
 });

test/token/ERC20/MintableToken.behaviour.js

@@ -0,0 +1,148 @@
+import assertRevert from '../../helpers/assertRevert';
+
+const BigNumber = web3.BigNumber;
+
+require('chai')
+  .use(require('chai-as-promised'))
+  .use(require('chai-bignumber')(BigNumber))
+  .should();
+
+export default function ([owner, anotherAccount, minter]) {
+  describe('as a basic mintable token', function () {
+    describe('after token creation', function () {
+      it('sender should be token owner', async function () {
+        const tokenOwner = await this.token.owner({ from: owner });
+        tokenOwner.should.equal(owner);
+      });
+    });
+
+    describe('minting finished', function () {
+      describe('when the token minting is not finished', function () {
+        it('returns false', async function () {
+          const mintingFinished = await this.token.mintingFinished();
+          assert.equal(mintingFinished, false);
+        });
+      });
+
+      describe('when the token is minting finished', function () {
+        beforeEach(async function () {
+          await this.token.finishMinting({ from: owner });
+        });
+
+        it('returns true', async function () {
+          const mintingFinished = await this.token.mintingFinished();
+          assert.equal(mintingFinished, true);
+        });
+      });
+    });
+
+    describe('finish minting', function () {
+      describe('when the sender is the token owner', function () {
+        const from = owner;
+
+        describe('when the token minting was not finished', function () {
+          it('finishes token minting', async function () {
+            await this.token.finishMinting({ from });
+
+            const mintingFinished = await this.token.mintingFinished();
+            assert.equal(mintingFinished, true);
+          });
+
+          it('emits a mint finished event', async function () {
+            const { logs } = await this.token.finishMinting({ from });
+
+            assert.equal(logs.length, 1);
+            assert.equal(logs[0].event, 'MintFinished');
+          });
+        });
+
+        describe('when the token minting was already finished', function () {
+          beforeEach(async function () {
+            await this.token.finishMinting({ from });
+          });
+
+          it('reverts', async function () {
+            await assertRevert(this.token.finishMinting({ from }));
+          });
+        });
+      });
+
+      describe('when the sender is not the token owner', function () {
+        const from = anotherAccount;
+
+        describe('when the token minting was not finished', function () {
+          it('reverts', async function () {
+            await assertRevert(this.token.finishMinting({ from }));
+          });
+        });
+
+        describe('when the token minting was already finished', function () {
+          beforeEach(async function () {
+            await this.token.finishMinting({ from: owner });
+          });
+
+          it('reverts', async function () {
+            await assertRevert(this.token.finishMinting({ from }));
+          });
+        });
+      });
+    });
+
+    describe('mint', function () {
+      const amount = 100;
+
+      describe('when the sender has the minting permission', function () {
+        const from = minter;
+
+        describe('when the token minting is not finished', function () {
+          it('mints the requested amount', async function () {
+            await this.token.mint(owner, amount, { from });
+
+            const balance = await this.token.balanceOf(owner);
+            assert.equal(balance, amount);
+          });
+
+          it('emits a mint and a transfer event', async function () {
+            const { logs } = await this.token.mint(owner, amount, { from });
+
+            assert.equal(logs.length, 2);
+            assert.equal(logs[0].event, 'Mint');
+            assert.equal(logs[0].args.to, owner);
+            assert.equal(logs[0].args.amount, amount);
+            assert.equal(logs[1].event, 'Transfer');
+          });
+        });
+
+        describe('when the token minting is finished', function () {
+          beforeEach(async function () {
+            await this.token.finishMinting({ from: owner });
+          });
+
+          it('reverts', async function () {
+            await assertRevert(this.token.mint(owner, amount, { from }));
+          });
+        });
+      });
+
+      describe('when the sender has not the minting permission', function () {
+        const from = anotherAccount;
+
+        describe('when the token minting is not finished', function () {
+          it('reverts', async function () {
+            await assertRevert(this.token.mint(owner, amount, { from }));
+          });
+        });
+
+        describe('when the token minting is already finished', function () {
+          beforeEach(async function () {
+            await this.token.finishMinting({ from: owner });
+          });
+
+          it('reverts', async function () {
+            await assertRevert(this.token.mint(owner, amount, { from }));
+          });
+        });
+      });
+    });
+  });
+};

test/token/ERC20/MintableToken.test.js

@@ -1,137 +1,12 @@
-import assertRevert from '../../helpers/assertRevert';
+import shouldBehaveLikeMintableToken from './MintableToken.behaviour';
 const MintableToken = artifacts.require('MintableToken');
 
-contract('Mintable', function ([owner, anotherAccount]) {
+contract('MintableToken', function ([owner, anotherAccount]) {
+  const minter = owner;
+
   beforeEach(async function () {
     this.token = await MintableToken.new({ from: owner });
   });
 
-  describe('minting finished', function () {
-    describe('when the token is not finished', function () {
-      it('returns false', async function () {
-        const mintingFinished = await this.token.mintingFinished();
-        assert.equal(mintingFinished, false);
-      });
-    });
-
-    describe('when the token is finished', function () {
-      beforeEach(async function () {
-        await this.token.finishMinting({ from: owner });
-      });
-
-      it('returns true', async function () {
-        const mintingFinished = await this.token.mintingFinished.call();
-        assert.equal(mintingFinished, true);
-      });
-    });
-  });
-
-  describe('finish minting', function () {
-    describe('when the sender is the token owner', function () {
-      const from = owner;
-
-      describe('when the token was not finished', function () {
-        it('finishes token minting', async function () {
-          await this.token.finishMinting({ from });
-
-          const mintingFinished = await this.token.mintingFinished();
-          assert.equal(mintingFinished, true);
-        });
-
-        it('emits a mint finished event', async function () {
-          const { logs } = await this.token.finishMinting({ from });
-
-          assert.equal(logs.length, 1);
-          assert.equal(logs[0].event, 'MintFinished');
-        });
-      });
-
-      describe('when the token was already finished', function () {
-        beforeEach(async function () {
-          await this.token.finishMinting({ from });
-        });
-
-        it('reverts', async function () {
-          await assertRevert(this.token.finishMinting({ from }));
-        });
-      });
-    });
-
-    describe('when the sender is not the token owner', function () {
-      const from = anotherAccount;
-
-      describe('when the token was not finished', function () {
-        it('reverts', async function () {
-          await assertRevert(this.token.finishMinting({ from }));
-        });
-      });
-
-      describe('when the token was already finished', function () {
-        beforeEach(async function () {
-          await this.token.finishMinting({ from: owner });
-        });
-
-        it('reverts', async function () {
-          await assertRevert(this.token.finishMinting({ from }));
-        });
-      });
-    });
-  });
-
-  describe('mint', function () {
-    const amount = 100;
-
-    describe('when the sender is the token owner', function () {
-      const from = owner;
-
-      describe('when the token was not finished', function () {
-        it('mints the requested amount', async function () {
-          await this.token.mint(owner, amount, { from });
-
-          const balance = await this.token.balanceOf(owner);
-          assert.equal(balance, amount);
-        });
-
-        it('emits a mint finished event', async function () {
-          const { logs } = await this.token.mint(owner, amount, { from });
-
-          assert.equal(logs.length, 2);
-          assert.equal(logs[0].event, 'Mint');
-          assert.equal(logs[0].args.to, owner);
-          assert.equal(logs[0].args.amount, amount);
-          assert.equal(logs[1].event, 'Transfer');
-        });
-      });
-
-      describe('when the token minting is finished', function () {
-        beforeEach(async function () {
-          await this.token.finishMinting({ from });
-        });
-
-        it('reverts', async function () {
-          await assertRevert(this.token.mint(owner, amount, { from }));
-        });
-      });
-    });
-
-    describe('when the sender is not the token owner', function () {
-      const from = anotherAccount;
-
-      describe('when the token was not finished', function () {
-        it('reverts', async function () {
-          await assertRevert(this.token.mint(owner, amount, { from }));
-        });
-      });
-
-      describe('when the token was already finished', function () {
-        beforeEach(async function () {
-          await this.token.finishMinting({ from: owner });
-        });
-
-        it('reverts', async function () {
-          await assertRevert(this.token.mint(owner, amount, { from }));
-        });
-      });
-    });
-  });
+  shouldBehaveLikeMintableToken([owner, anotherAccount, minter]);
 });

test/token/ERC20/RBACMintableToken.test.js

@@ -0,0 +1,37 @@
+import expectThrow from '../../helpers/expectThrow';
+import shouldBehaveLikeMintableToken from './MintableToken.behaviour';
+const RBACMintableToken = artifacts.require('RBACMintableToken');
+
+const ROLE_MINTER = 'minter';
+
+contract('RBACMintableToken', function ([owner, anotherAccount, minter]) {
+  beforeEach(async function () {
+    this.token = await RBACMintableToken.new({ from: owner });
+    await this.token.addMinter(minter, { from: owner });
+  });
+
+  describe('handle roles', function () {
+    it('owner can add and remove a minter role', async function () {
+      await this.token.addMinter(anotherAccount, { from: owner });
+      let hasRole = await this.token.hasRole(anotherAccount, ROLE_MINTER);
+      assert.equal(hasRole, true);
+
+      await this.token.removeMinter(anotherAccount, { from: owner });
+      hasRole = await this.token.hasRole(anotherAccount, ROLE_MINTER);
+      assert.equal(hasRole, false);
+    });
+
+    it('another account can\'t add or remove a minter role', async function () {
+      await expectThrow(
+        this.token.addMinter(anotherAccount, { from: anotherAccount })
+      );
+
+      await this.token.addMinter(anotherAccount, { from: owner });
+      await expectThrow(
+        this.token.removeMinter(anotherAccount, { from: anotherAccount })
+      );
+    });
+  });
+
+  shouldBehaveLikeMintableToken([owner, anotherAccount, minter]);
+});