MintableToken using Roles

contracts/access/rbac/MinterRole.sol

@@ -0,0 +1,39 @@
+pragma solidity ^0.4.24;
+
+import "./Roles.sol";
+
+
+contract MinterRole {
+  using Roles for Roles.Role;
+
+  Roles.Role private minters;
+
+  constructor(address[] _minters) public {
+    minters.addMany(_minters);
+  }
+
+  function transferMinter(address _account) public {
+    minters.transfer(_account);
+  }
+
+  function renounceMinter() public {
+    minters.renounce();
+  }
+
+  function isMinter(address _account) public view returns (bool) {
+    return minters.has(_account);
+  }
+
+  modifier onlyMinter() {
+    require(isMinter(msg.sender));
+    _;
+  }
+
+  function _addMinter(address _account) internal {
+    minters.add(_account);
+  }
+
+  function _removeMinter(address _account) internal {
+    minters.remove(_account);
+  }
+}

contracts/access/rbac/RBAC.sol

@@ -30,7 +30,7 @@ contract RBAC {
     public
     view
   {
-    roles[_role].check(_operator);
+    require(roles[_role].has(_operator));
   }
 
   /**

contracts/access/rbac/Roles.sol

@@ -48,14 +48,6 @@ library Roles {
     remove(_role, msg.sender);
   }
 
-  /**
-   * @dev check if an account has this role
-   * // reverts
-   */
-  function check(Role storage _role, address _account) internal view {
-    require(has(_role, _account));
-  }
-
   /**
    * @dev check if an account has this role
    * @return bool

contracts/examples/SampleCrowdsale.sol

@@ -17,6 +17,11 @@ contract SampleCrowdsaleToken is MintableToken {
   string public constant symbol = "SCT";
   uint8 public constant decimals = 18;
 
+  constructor(address[] _minters)
+    MintableToken(_minters)
+    public
+  {
+  }
 }
 
 

contracts/mocks/FinalizableCrowdsaleImpl.sol

@@ -1,6 +1,6 @@
 pragma solidity ^0.4.24;
 
-import "../token/ERC20/MintableToken.sol";
+import "../token/ERC20/ERC20.sol";
 import "../crowdsale/distribution/FinalizableCrowdsale.sol";
 
 
@@ -11,7 +11,7 @@ contract FinalizableCrowdsaleImpl is FinalizableCrowdsale {
     uint256 _closingTime,
     uint256 _rate,
     address _wallet,
-    MintableToken _token
+    ERC20 _token
   )
     public
     Crowdsale(_rate, _wallet, _token)

contracts/mocks/MintableTokenMock.sol

@@ -0,0 +1,21 @@
+pragma solidity ^0.4.24;
+
+import "../token/ERC20/MintableToken.sol";
+
+
+// Mock contract exposing internal methods
+contract MintableTokenMock is MintableToken {
+  constructor(address[] minters) MintableToken(minters) public {
+  }
+
+  function addMinter(address _account) public {
+    _addMinter(_account);
+  }
+
+  function removeMinter(address _account) public {
+    _removeMinter(_account);
+  }
+
+  function onlyMinterMock() public view onlyMinter {
+  }
+}

contracts/mocks/RolesMock.sol

@@ -28,10 +28,6 @@ contract RolesMock {
     dummyRole.transfer(_account);
   }
 
-  function check(address _account) public view {
-    dummyRole.check(_account);
-  }
-
   function has(address _account) public view returns (bool) {
     return dummyRole.has(_account);
   }

contracts/token/ERC20/CappedToken.sol

@@ -11,7 +11,10 @@ contract CappedToken is MintableToken {
 
   uint256 public cap;
 
-  constructor(uint256 _cap) public {
+  constructor(uint256 _cap, address[] _minters)
+    public
+    MintableToken(_minters)
+  {
     require(_cap > 0);
     cap = _cap;
   }

contracts/token/ERC20/MintableToken.sol

@@ -1,31 +1,31 @@
 pragma solidity ^0.4.24;
 
 import "./StandardToken.sol";
-import "../../ownership/Ownable.sol";
+import "../../access/rbac/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
  */
-contract MintableToken is StandardToken, Ownable {
+contract MintableToken is StandardToken, MinterRole {
   event Mint(address indexed to, uint256 amount);
   event MintFinished();
 
   bool public mintingFinished = false;
 
+  constructor(address[] _minters)
+    MinterRole(_minters)
+    public
+  {
+  }
 
   modifier canMint() {
     require(!mintingFinished);
     _;
   }
 
-  modifier hasMintPermission() {
-    require(msg.sender == owner);
-    _;
-  }
-
   /**
    * @dev Function to mint tokens
    * @param _to The address that will receive the minted tokens.
@@ -37,7 +37,7 @@ contract MintableToken is StandardToken, Ownable {
     uint256 _amount
   )
     public
-    hasMintPermission
+    onlyMinter
     canMint
     returns (bool)
   {
@@ -50,7 +50,7 @@ contract MintableToken is StandardToken, Ownable {
    * @dev Function to stop minting new tokens.
    * @return True if the operation was successful.
    */
-  function finishMinting() public onlyOwner canMint returns (bool) {
+  function finishMinting() public onlyMinter canMint returns (bool) {
     mintingFinished = true;
     emit MintFinished();
     return true;

test/access/rbac/PublicRole.behavior.js

@@ -0,0 +1,98 @@
+const { assertRevert } = require('../../helpers/assertRevert');
+
+require('chai')
+  .should();
+
+function capitalize (str) {
+  return str.replace(/\b\w/g, l => l.toUpperCase());
+}
+
+function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], rolename) {
+  rolename = capitalize(rolename);
+  const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000';
+
+  describe(`role ${rolename}`, function () {
+    beforeEach('check preconditions', async function () {
+      (await this.contract[`is${rolename}`](authorized)).should.equal(true);
+      (await this.contract[`is${rolename}`](otherAuthorized)).should.equal(true);
+      (await this.contract[`is${rolename}`](anyone)).should.equal(false);
+    });
+
+    describe('add', function () {
+      it('adds role to a new account', async function () {
+        await this.contract[`add${rolename}`](authorized);
+        (await this.contract[`is${rolename}`](authorized)).should.equal(true);
+        (await this.contract[`is${rolename}`](anyone)).should.equal(false);
+      });
+
+      it('adds role to an already-assigned account', async function () {
+        await this.contract[`add${rolename}`](authorized);
+        await this.contract[`add${rolename}`](authorized);
+        (await this.contract[`is${rolename}`](authorized)).should.equal(true);
+      });
+
+      it('doesn\'t revert when adding role to the null account', async function () {
+        await this.contract[`add${rolename}`](ZERO_ADDRESS);
+      });
+    });
+
+    describe('remove', function () {
+      it('removes role from an already assgined account', async function () {
+        await this.contract[`remove${rolename}`](authorized);
+        (await this.contract[`is${rolename}`](authorized)).should.equal(false);
+        (await this.contract[`is${rolename}`](otherAuthorized)).should.equal(true);
+      });
+
+      it('doesn\'t revert when removing from an unassigned account', async function () {
+        await this.contract[`remove${rolename}`](anyone);
+      });
+
+      it('doesn\'t revert when removing role from the null account', async function () {
+        await this.contract[`remove${rolename}`](ZERO_ADDRESS);
+      });
+    });
+
+    describe('transfering', function () {
+      context('from account with role', function () {
+        const from = authorized;
+
+        it('transfers to other account without the role', async function () {
+          await this.contract[`transfer${rolename}`](anyone, { from });
+          (await this.contract[`is${rolename}`](anyone)).should.equal(true);
+          (await this.contract[`is${rolename}`](authorized)).should.equal(false);
+        });
+
+        it('reverts when transfering to an account with role', async function () {
+          await assertRevert(this.contract[`transfer${rolename}`](otherAuthorized, { from }));
+        });
+
+        it('reverts when transfering to the null account', async function () {
+          await assertRevert(this.contract[`transfer${rolename}`](ZERO_ADDRESS, { from }));
+        });
+      });
+
+      context('from account without role', function () {
+        const from = anyone;
+
+        it('reverts', async function () {
+          await assertRevert(this.contract[`transfer${rolename}`](anyone, { from }));
+        });
+      });
+    });
+
+    describe('renouncing roles', function () {
+      it('renounces an assigned role', async function () {
+        await this.contract[`renounce${rolename}`]({ from: authorized });
+        (await this.contract[`is${rolename}`](authorized)).should.equal(false);
+      });
+
+      it('doesn\'t revert when renouncing unassigned role', async function () {
+        await this.contract[`renounce${rolename}`]({ from: anyone });
+      });
+    });
+  });
+}
+
+module.exports = {
+  shouldBehaveLikePublicRole,
+};

test/access/rbac/Roles.test.js

@@ -10,46 +10,37 @@ contract('Roles', function ([_, authorized, otherAuthorized, anyone]) {
 
   beforeEach(async function () {
     this.roles = await RolesMock.new();
-    this.testRole = async (account, expected) => {
-      if (expected) {
-        (await this.roles.has(account)).should.equal(true);
-        await this.roles.check(account); // this call shouldn't revert, but is otherwise a no-op
-      } else {
-        (await this.roles.has(account)).should.equal(false);
-        await assertRevert(this.roles.check(account));
-      }
-    };
   });
 
   context('initially', function () {
     it('doesn\'t pre-assign roles', async function () {
-      await this.testRole(authorized, false);
-      await this.testRole(otherAuthorized, false);
-      await this.testRole(anyone, false);
+      (await this.roles.has(authorized)).should.equal(false);
+      (await this.roles.has(otherAuthorized)).should.equal(false);
+      (await this.roles.has(anyone)).should.equal(false);
     });
 
     describe('adding roles', function () {
       it('adds roles to a single account', async function () {
         await this.roles.add(authorized);
-        await this.testRole(authorized, true);
-        await this.testRole(anyone, false);
+        (await this.roles.has(authorized)).should.equal(true);
+        (await this.roles.has(anyone)).should.equal(false);
       });
 
       it('adds roles to an already-assigned account', async function () {
         await this.roles.add(authorized);
         await this.roles.add(authorized);
-        await this.testRole(authorized, true);
+        (await this.roles.has(authorized)).should.equal(true);
       });
 
       it('adds roles to multiple accounts', async function () {
         await this.roles.addMany([authorized, otherAuthorized]);
-        await this.testRole(authorized, true);
-        await this.testRole(otherAuthorized, true);
+        (await this.roles.has(authorized)).should.equal(true);
+        (await this.roles.has(otherAuthorized)).should.equal(true);
       });
 
       it('adds roles to multiple identical accounts', async function () {
         await this.roles.addMany([authorized, authorized]);
-        await this.testRole(authorized, true);
+        (await this.roles.has(authorized)).should.equal(true);
       });
 
       it('doesn\'t revert when adding roles to the null account', async function () {
@@ -66,8 +57,8 @@ contract('Roles', function ([_, authorized, otherAuthorized, anyone]) {
     describe('removing roles', function () {
       it('removes a single role', async function () {
         await this.roles.remove(authorized);
-        await this.testRole(authorized, false);
-        await this.testRole(otherAuthorized, true);
+        (await this.roles.has(authorized)).should.equal(false);
+        (await this.roles.has(otherAuthorized)).should.equal(true);
       });
 
       it('doesn\'t revert when removing unassigned roles', async function () {
@@ -85,8 +76,8 @@ contract('Roles', function ([_, authorized, otherAuthorized, anyone]) {
 
         it('transfers to other account with no role', async function () {
           await this.roles.transfer(anyone, { from });
-          await this.testRole(anyone, true);
-          await this.testRole(authorized, false);
+          (await this.roles.has(anyone)).should.equal(true);
+          (await this.roles.has(authorized)).should.equal(false);
         });
 
         it('reverts when transfering to an account with role', async function () {
@@ -110,7 +101,7 @@ contract('Roles', function ([_, authorized, otherAuthorized, anyone]) {
     describe('renouncing roles', function () {
       it('renounces an assigned role', async function () {
         await this.roles.renounce({ from: authorized });
-        await this.testRole(authorized, false);
+        (await this.roles.has(authorized)).should.equal(false);
       });
 
       it('doesn\'t revert when renouncing unassigned role', async function () {