Update modifiers to call public view functions (#1277)

* Update modifiers to call public view functions.

Fixes #1179.

* remove isMinter

* fix is owner call

* fix isOpen

* Remove unnecessary solium disable

contracts/crowdsale/validation/TimedCrowdsale.sol

@@ -18,8 +18,7 @@ contract TimedCrowdsale is Crowdsale {
    * @dev Reverts if not in crowdsale time range.
    */
   modifier onlyWhileOpen {
-    // solium-disable-next-line security/no-block-members
+    require(isOpen());
-    require(block.timestamp >= openingTime && block.timestamp <= closingTime);
     _;
   }
 
@@ -37,6 +36,14 @@ contract TimedCrowdsale is Crowdsale {
     closingTime = _closingTime;
   }
 
+  /**
+   * @return true if the crowdsale is open, false otherwise.
+   */
+  function isOpen() public view returns (bool) {
+    // solium-disable-next-line security/no-block-members
+    return block.timestamp >= openingTime && block.timestamp <= closingTime;
+  }
+
   /**
    * @dev Checks whether the period in which the crowdsale is open has already elapsed.
    * @return Whether crowdsale period has elapsed

contracts/ownership/Ownable.sol

@@ -36,10 +36,17 @@ contract Ownable {
    * @dev Throws if called by any account other than the owner.
    */
   modifier onlyOwner() {
-    require(msg.sender == owner_);
+    require(isOwner());
     _;
   }
 
+  /**
+   * @return true if `msg.sender` is the owner of the contract.
+   */
+  function isOwner() public view returns(bool) {
+    return msg.sender == owner_;
+  }
+
   /**
    * @dev Allows the current owner to relinquish control of the contract.
    * @notice Renouncing to ownership will leave the contract without an owner.

contracts/token/ERC20/ERC20Mintable.sol

@@ -16,13 +16,13 @@ contract ERC20Mintable is ERC20, Ownable {
   bool private mintingFinished_ = false;
 
 
-  modifier canMint() {
+  modifier onlyBeforeMintingFinished() {
     require(!mintingFinished_);
     _;
   }
 
-  modifier hasMintPermission() {
+  modifier onlyMinter() {
-    require(msg.sender == owner());
+    require(isOwner());
     _;
   }
 
@@ -44,8 +44,8 @@ contract ERC20Mintable is ERC20, Ownable {
     uint256 _amount
   )
     public
-    hasMintPermission
+    onlyMinter
-    canMint
+    onlyBeforeMintingFinished
     returns (bool)
   {
     _mint(_to, _amount);
@@ -57,7 +57,12 @@ contract ERC20Mintable is ERC20, 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
+    onlyOwner
+    onlyBeforeMintingFinished
+    returns (bool)
+  {
     mintingFinished_ = true;
     emit MintFinished();
     return true;

contracts/token/ERC20/RBACMintableToken.sol

@@ -18,7 +18,7 @@ contract RBACMintableToken is ERC20Mintable, RBAC {
   /**
    * @dev override the Mintable token modifier to add role based logic
    */
-  modifier hasMintPermission() {
+  modifier onlyMinter() {
     checkRole(msg.sender, ROLE_MINTER);
     _;
   }

test/crowdsale/TimedCrowdsale.test.js

@@ -52,17 +52,20 @@ contract('TimedCrowdsale', function ([_, investor, wallet, purchaser]) {
     it('should be ended only after end', async function () {
       (await this.crowdsale.hasClosed()).should.equal(false);
       await increaseTimeTo(this.afterClosingTime);
+      (await this.crowdsale.isOpen()).should.equal(false);
       (await this.crowdsale.hasClosed()).should.equal(true);
     });
 
     describe('accepting payments', function () {
       it('should reject payments before start', async function () {
+        (await this.crowdsale.isOpen()).should.equal(false);
         await expectThrow(this.crowdsale.send(value), EVMRevert);
         await expectThrow(this.crowdsale.buyTokens(investor, { from: purchaser, value: value }), EVMRevert);
       });
 
       it('should accept payments after start', async function () {
         await increaseTimeTo(this.openingTime);
+        (await this.crowdsale.isOpen()).should.equal(true);
         await this.crowdsale.send(value);
         await this.crowdsale.buyTokens(investor, { value: value, from: purchaser });
       });

test/ownership/Ownable.behavior.js

@@ -13,8 +13,11 @@ function shouldBehaveLikeOwnable (owner, [anyone]) {
     });
 
     it('changes owner after transfer', async function () {
+      (await this.ownable.isOwner({ from: anyone })).should.be.equal(false);
       await this.ownable.transferOwnership(anyone, { from: owner });
+
       (await this.ownable.owner()).should.equal(anyone);
+      (await this.ownable.isOwner({ from: anyone })).should.be.equal(true);
     });
 
     it('should prevent non-owners from transfering', async function () {