diff --git a/contracts/crowdsale/validation/TimedCrowdsale.sol b/contracts/crowdsale/validation/TimedCrowdsale.sol index 52b7f4ef3..23f0e6e87 100644 --- a/contracts/crowdsale/validation/TimedCrowdsale.sol +++ b/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(block.timestamp >= openingTime && block.timestamp <= closingTime); + require(isOpen()); _; } @@ -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 diff --git a/contracts/ownership/Ownable.sol b/contracts/ownership/Ownable.sol index f89013a1a..c6ea3e16f 100644 --- a/contracts/ownership/Ownable.sol +++ b/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. diff --git a/contracts/token/ERC20/ERC20Mintable.sol b/contracts/token/ERC20/ERC20Mintable.sol index 518936699..8b80dd80a 100644 --- a/contracts/token/ERC20/ERC20Mintable.sol +++ b/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() { - require(msg.sender == owner()); + modifier onlyMinter() { + require(isOwner()); _; } @@ -44,8 +44,8 @@ contract ERC20Mintable is ERC20, Ownable { uint256 _amount ) public - hasMintPermission - canMint + onlyMinter + 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; diff --git a/contracts/token/ERC20/RBACMintableToken.sol b/contracts/token/ERC20/RBACMintableToken.sol index ad6ab19ca..65e16f564 100644 --- a/contracts/token/ERC20/RBACMintableToken.sol +++ b/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); _; } diff --git a/test/crowdsale/TimedCrowdsale.test.js b/test/crowdsale/TimedCrowdsale.test.js index ad9ca29f5..4c7eff303 100644 --- a/test/crowdsale/TimedCrowdsale.test.js +++ b/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 }); }); diff --git a/test/ownership/Ownable.behavior.js b/test/ownership/Ownable.behavior.js index 6257c01df..eac3c10ce 100644 --- a/test/ownership/Ownable.behavior.js +++ b/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 () {