Improve encapsulation on ownership

contracts/drafts/TokenVesting.sol

@@ -142,7 +142,7 @@ contract TokenVesting is Ownable {
 
     revoked_[_token] = true;
 
-    _token.safeTransfer(owner, refund);
+    _token.safeTransfer(getOwner(), refund);
 
     emit Revoked();
   }

contracts/ownership/Claimable.sol

@@ -10,30 +10,37 @@ import "./Ownable.sol";
  * This allows the new owner to accept the transfer.
  */
 contract Claimable is Ownable {
-  address public pendingOwner;
+  address private pendingOwner_;
 
   /**
    * @dev Modifier throws if called by any account other than the pendingOwner.
    */
   modifier onlyPendingOwner() {
-    require(msg.sender == pendingOwner);
+    require(msg.sender == pendingOwner_);
     _;
   }
 
+  /**
+   * @return the address of the pending owner.
+   */
+  function getPendingOwner() public view returns(address) {
+    return pendingOwner_;
+  }
+
   /**
    * @dev Allows the current owner to set the pendingOwner address.
    * @param newOwner The address to transfer ownership to.
    */
   function transferOwnership(address newOwner) public onlyOwner {
-    pendingOwner = newOwner;
+    pendingOwner_ = newOwner;
   }
 
   /**
    * @dev Allows the pendingOwner address to finalize the transfer.
    */
   function claimOwnership() public onlyPendingOwner {
-    emit OwnershipTransferred(owner, pendingOwner);
-    owner = pendingOwner;
-    pendingOwner = address(0);
+    emit OwnershipTransferred(getOwner(), pendingOwner_);
+    owner = pendingOwner_;
+    pendingOwner_ = address(0);
   }
 }

contracts/ownership/DelayedClaimable.sol

@@ -10,8 +10,22 @@ import "./Claimable.sol";
  */
 contract DelayedClaimable is Claimable {
 
-  uint256 public end;
-  uint256 public start;
+  uint256 private start_;
+  uint256 private end_;
+
+  /**
+   * @return the start of the claimable period.
+   */
+  function getStart() public view returns(uint256) {
+    return start_;
+  }
+
+  /**
+   * @return the end of the claimable period.
+   */
+  function getEnd() public view returns(uint256) {
+    return end_;
+  }
 
   /**
    * @dev Used to specify the time period during which a pending
@@ -21,20 +35,18 @@ contract DelayedClaimable is Claimable {
    */
   function setLimits(uint256 _start, uint256 _end) public onlyOwner {
     require(_start <= _end);
-    end = _end;
-    start = _start;
+    end_ = _end;
+    start_ = _start;
   }
 
   /**
-   * @dev Allows the pendingOwner address to finalize the transfer, as long as it is called within
+   * @dev Allows the pending owner address to finalize the transfer, as long as it is called within
    * the specified start and end time.
    */
   function claimOwnership() public onlyPendingOwner {
-    require((block.number <= end) && (block.number >= start));
-    emit OwnershipTransferred(owner, pendingOwner);
-    owner = pendingOwner;
-    pendingOwner = address(0);
-    end = 0;
+    require((block.number <= end_) && (block.number >= start_));
+    super.claimOwnership();
+    end_ = 0;
   }
 
 }

contracts/ownership/Heritable.sol

@@ -51,9 +51,9 @@ contract Heritable is Ownable {
   }
 
   function setHeir(address _newHeir) public onlyOwner {
-    require(_newHeir != owner);
+    require(_newHeir != getOwner());
     heartbeat();
-    emit HeirChanged(owner, _newHeir);
+    emit HeirChanged(getOwner(), _newHeir);
     heir_ = _newHeir;
   }
 
@@ -87,7 +87,7 @@ contract Heritable is Ownable {
    */
   function proclaimDeath() public onlyHeir {
     require(_ownerLives());
-    emit OwnerProclaimedDead(owner, heir_, timeOfDeath_);
+    emit OwnerProclaimedDead(getOwner(), heir_, timeOfDeath_);
     // solium-disable-next-line security/no-block-members
     timeOfDeath_ = block.timestamp;
   }
@@ -96,7 +96,7 @@ contract Heritable is Ownable {
    * @dev Owner can send a heartbeat if they were mistakenly pronounced dead.
    */
   function heartbeat() public onlyOwner {
-    emit OwnerHeartbeated(owner);
+    emit OwnerHeartbeated(getOwner());
     timeOfDeath_ = 0;
   }
 
@@ -107,9 +107,8 @@ contract Heritable is Ownable {
     require(!_ownerLives());
     // solium-disable-next-line security/no-block-members
     require(block.timestamp >= timeOfDeath_ + heartbeatTimeout_);
-    emit OwnershipTransferred(owner, heir_);
-    emit HeirOwnershipClaimed(owner, heir_);
-    owner = heir_;
+    emit HeirOwnershipClaimed(getOwner(), heir_);
+    transferOwnership(heir_);
     timeOfDeath_ = 0;
   }
 

contracts/ownership/Ownable.sol

@@ -7,7 +7,7 @@ pragma solidity ^0.4.24;
  * functions, this simplifies the implementation of "user permissions".
  */
 contract Ownable {
-  address public owner;
+  address private owner_;
 
 
   event OwnershipRenounced(address indexed previousOwner);
@@ -22,14 +22,21 @@ contract Ownable {
    * account.
    */
   constructor() public {
-    owner = msg.sender;
+    owner_ = msg.sender;
+  }
+
+  /**
+   * @return the address of the owner.
+   */
+  function getOwner() public view returns(address) {
+    return owner_;
   }
 
   /**
    * @dev Throws if called by any account other than the owner.
    */
   modifier onlyOwner() {
-    require(msg.sender == owner);
+    require(msg.sender == owner_);
     _;
   }
 
@@ -40,7 +47,7 @@ contract Ownable {
    * modifier anymore.
    */
   function renounceOwnership() public onlyOwner {
-    emit OwnershipRenounced(owner);
+    emit OwnershipRenounced(owner_);
     owner = address(0);
   }
 
@@ -58,7 +65,7 @@ contract Ownable {
    */
   function _transferOwnership(address _newOwner) internal {
     require(_newOwner != address(0));
-    emit OwnershipTransferred(owner, _newOwner);
-    owner = _newOwner;
+    emit OwnershipTransferred(owner_, _newOwner);
+    owner_ = _newOwner;
   }
 }

contracts/ownership/Superuser.sol

@@ -27,7 +27,7 @@ contract Superuser is Ownable, RBAC {
   }
 
   modifier onlyOwnerOrSuperuser() {
-    require(msg.sender == owner || isSuperuser(msg.sender));
+    require(msg.sender == getOwner() || isSuperuser(msg.sender));
     _;
   }
 

contracts/token/ERC20/ERC20Mintable.sol

@@ -22,7 +22,7 @@ contract ERC20Mintable is ERC20, Ownable {
   }
 
   modifier hasMintPermission() {
-    require(msg.sender == owner);
+    require(msg.sender == getOwner());
     _;
   }
 

test/access/SignatureBouncer.test.js

@@ -20,7 +20,7 @@ contract('Bouncer', function ([_, owner, anyone, bouncerAddress, authorizedUser]
 
   context('management', function () {
     it('has a default owner of self', async function () {
-      (await this.bouncer.owner()).should.equal(owner);
+      (await this.bouncer.getOwner()).should.equal(owner);
     });
 
     it('allows the owner to add a bouncer', async function () {

test/crowdsale/MintedCrowdsale.test.js

@@ -21,7 +21,7 @@ contract('MintedCrowdsale', function ([_, investor, wallet, purchaser]) {
     });
 
     it('should be token owner', async function () {
-      (await this.token.owner()).should.equal(this.crowdsale.address);
+      (await this.token.getOwner()).should.equal(this.crowdsale.address);
     });
 
     shouldBehaveLikeMintedCrowdsale([_, investor, wallet, purchaser], rate, value);

test/ownership/Claimable.test.js

@@ -16,12 +16,12 @@ contract('Claimable', function ([_, owner, newOwner, anyone]) {
   });
 
   it('should have an owner', async function () {
-    (await claimable.owner()).should.not.equal(0);
+    (await claimable.getOwner()).should.not.equal(0);
   });
 
   it('changes pendingOwner after transfer', async function () {
     await claimable.transferOwnership(newOwner, { from: owner });
-    (await claimable.pendingOwner()).should.equal(newOwner);
+    (await claimable.getPendingOwner()).should.equal(newOwner);
   });
 
   it('should prevent to claimOwnership from anyone', async function () {
@@ -40,7 +40,7 @@ contract('Claimable', function ([_, owner, newOwner, anyone]) {
     it('changes allow pending owner to claim ownership', async function () {
       await claimable.claimOwnership({ from: newOwner });
 
-      (await claimable.owner()).should.equal(newOwner);
+      (await claimable.getOwner()).should.equal(newOwner);
     });
   });
 });

test/ownership/DelayedClaimable.test.js

@@ -17,35 +17,35 @@ contract('DelayedClaimable', function ([_, owner, newOwner]) {
     await this.delayedClaimable.transferOwnership(newOwner, { from: owner });
     await this.delayedClaimable.setLimits(0, 1000, { from: owner });
 
-    (await this.delayedClaimable.end()).should.be.bignumber.equal(1000);
+    (await this.delayedClaimable.getEnd()).should.be.bignumber.equal(1000);
 
-    (await this.delayedClaimable.start()).should.be.bignumber.equal(0);
+    (await this.delayedClaimable.getStart()).should.be.bignumber.equal(0);
   });
 
   it('changes pendingOwner after transfer successful', async function () {
     await this.delayedClaimable.transferOwnership(newOwner, { from: owner });
     await this.delayedClaimable.setLimits(0, 1000, { from: owner });
 
-    (await this.delayedClaimable.end()).should.be.bignumber.equal(1000);
+    (await this.delayedClaimable.getEnd()).should.be.bignumber.equal(1000);
 
-    (await this.delayedClaimable.start()).should.be.bignumber.equal(0);
+    (await this.delayedClaimable.getStart()).should.be.bignumber.equal(0);
 
-    (await this.delayedClaimable.pendingOwner()).should.equal(newOwner);
+    (await this.delayedClaimable.getPendingOwner()).should.equal(newOwner);
     await this.delayedClaimable.claimOwnership({ from: newOwner });
-    (await this.delayedClaimable.owner()).should.equal(newOwner);
+    (await this.delayedClaimable.getOwner()).should.equal(newOwner);
   });
 
   it('changes pendingOwner after transfer fails', async function () {
     await this.delayedClaimable.transferOwnership(newOwner, { from: owner });
     await this.delayedClaimable.setLimits(100, 110, { from: owner });
 
-    (await this.delayedClaimable.end()).should.be.bignumber.equal(110);
+    (await this.delayedClaimable.getEnd()).should.be.bignumber.equal(110);
 
-    (await this.delayedClaimable.start()).should.be.bignumber.equal(100);
+    (await this.delayedClaimable.getStart()).should.be.bignumber.equal(100);
 
-    (await this.delayedClaimable.pendingOwner()).should.equal(newOwner);
+    (await this.delayedClaimable.getPendingOwner()).should.equal(newOwner);
     await assertRevert(this.delayedClaimable.claimOwnership({ from: newOwner }));
-    (await this.delayedClaimable.owner()).should.not.equal(newOwner);
+    (await this.delayedClaimable.getOwner()).should.not.equal(newOwner);
   });
 
   it('set end and start invalid values fail', async function () {

test/ownership/Ownable.behavior.js

@@ -9,12 +9,12 @@ require('chai')
 function shouldBehaveLikeOwnable (owner, [anyone]) {
   describe('as an ownable', function () {
     it('should have an owner', async function () {
-      (await this.ownable.owner()).should.equal(owner);
+      (await this.ownable.getOwner()).should.equal(owner);
     });
 
     it('changes owner after transfer', async function () {
       await this.ownable.transferOwnership(anyone, { from: owner });
-      (await this.ownable.owner()).should.equal(anyone);
+      (await this.ownable.getOwner()).should.equal(anyone);
     });
 
     it('should prevent non-owners from transfering', async function () {
@@ -27,7 +27,7 @@ function shouldBehaveLikeOwnable (owner, [anyone]) {
 
     it('loses owner after renouncement', async function () {
       await this.ownable.renounceOwnership({ from: owner });
-      (await this.ownable.owner()).should.equal(ZERO_ADDRESS);
+      (await this.ownable.getOwner()).should.equal(ZERO_ADDRESS);
     });
 
     it('should prevent non-owners from renouncement', async function () {

test/ownership/Superuser.test.js

@@ -40,7 +40,7 @@ contract('Superuser', function ([_, firstOwner, newSuperuser, newOwner, anyone])
         'OwnershipTransferred'
       );
 
-      (await this.superuser.owner()).should.equal(newOwner);
+      (await this.superuser.getOwner()).should.equal(newOwner);
     });
 
     it('should change owner after the owner transfers the ownership', async function () {
@@ -49,7 +49,7 @@ contract('Superuser', function ([_, firstOwner, newSuperuser, newOwner, anyone])
         'OwnershipTransferred'
       );
 
-      (await this.superuser.owner()).should.equal(newOwner);
+      (await this.superuser.getOwner()).should.equal(newOwner);
     });
   });
 

test/token/ERC20/ERC20Mintable.behavior.js

@@ -13,7 +13,7 @@ function shouldBehaveLikeERC20Mintable (owner, minter, [anyone]) {
   describe('as a basic mintable token', function () {
     describe('after token creation', function () {
       it('sender should be token owner', async function () {
-        (await this.token.owner({ from: owner })).should.equal(owner);
+        (await this.token.getOwner({ from: owner })).should.equal(owner);
       });
     });