Improve encapsulation on lifecycle, ownership and payments

contracts/bounties/BreakInvariantBounty.sol

@@ -50,7 +50,7 @@ contract BreakInvariantBounty is PullPayment, Ownable {
    * @dev Transfers the current balance to the owner and terminates the contract.
    */
   function destroy() public onlyOwner {
-    selfdestruct(owner);
+    selfdestruct(getOwner());
   }
 
   /**

contracts/lifecycle/Pausable.sol

@@ -12,38 +12,45 @@ contract Pausable is Ownable {
   event Paused();
   event Unpaused();
 
-  bool public paused = false;
+  bool private paused_ = false;
 
 
+  /**
+   * @return true if the contract is paused, false otherwise.
+   */
+  function isPaused() public view returns(bool) {
+    return paused_;
+  }
+
   /**
    * @dev Modifier to make a function callable only when the contract is not paused.
    */
   modifier whenNotPaused() {
-    require(!paused);
+    require(!paused_);
     _;
   }
 
   /**
    * @dev Modifier to make a function callable only when the contract is paused.
    */
   modifier whenPaused() {
-    require(paused);
+    require(paused_);
     _;
   }
 
   /**
    * @dev called by the owner to pause, triggers stopped state
    */
   function pause() public onlyOwner whenNotPaused {
-    paused = true;
+    paused_ = true;
     emit Paused();
   }
 
   /**
    * @dev called by the owner to unpause, returns to normal state
    */
   function unpause() public onlyOwner whenPaused {
-    paused = false;
+    paused_ = false;
     emit Unpaused();
   }
 }

contracts/ownership/CanReclaimToken.sol

@@ -20,7 +20,7 @@ contract CanReclaimToken is Ownable {
    */
   function reclaimToken(IERC20 _token) external onlyOwner {
     uint256 balance = _token.balanceOf(this);
-    _token.safeTransfer(owner, balance);
+    _token.safeTransfer(getOwner(), balance);
   }
 
 }

contracts/ownership/Claimable.sol

@@ -10,30 +10,35 @@ 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);
+    _transferOwnership(pendingOwner_);
   }
 }

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,8 +47,8 @@ contract Ownable {
    * modifier anymore.
    */
   function renounceOwnership() public onlyOwner {
-    emit OwnershipRenounced(owner);
-    owner = address(0);
+    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/payment/RefundEscrow.sol

@@ -16,25 +16,39 @@ contract RefundEscrow is Ownable, ConditionalEscrow {
   event Closed();
   event RefundsEnabled();
 
-  State public state;
-  address public beneficiary;
+  State private state_;
+  address private beneficiary_;
 
   /**
    * @dev Constructor.
    * @param _beneficiary The beneficiary of the deposits.
    */
   constructor(address _beneficiary) public {
     require(_beneficiary != address(0));
-    beneficiary = _beneficiary;
-    state = State.Active;
+    beneficiary_ = _beneficiary;
+    state_ = State.Active;
+  }
+
+  /**
+   * @return the current state of the escrow.
+   */
+  function getState() public view returns(State) {
+    return state_;
+  }
+
+  /**
+   * @return the beneficiary of the escrow.
+   */
+  function getBeneficiary() public view returns(address) {
+    return beneficiary_;
   }
 
   /**
    * @dev Stores funds that may later be refunded.
    * @param _refundee The address funds will be sent to if a refund occurs.
    */
   function deposit(address _refundee) public payable {
-    require(state == State.Active);
+    require(state_ == State.Active);
     super.deposit(_refundee);
   }
 
@@ -43,32 +57,32 @@ contract RefundEscrow is Ownable, ConditionalEscrow {
    * further deposits.
    */
   function close() public onlyOwner {
-    require(state == State.Active);
-    state = State.Closed;
+    require(state_ == State.Active);
+    state_ = State.Closed;
     emit Closed();
   }
 
   /**
    * @dev Allows for refunds to take place, rejecting further deposits.
    */
   function enableRefunds() public onlyOwner {
-    require(state == State.Active);
-    state = State.Refunding;
+    require(state_ == State.Active);
+    state_ = State.Refunding;
     emit RefundsEnabled();
   }
 
   /**
    * @dev Withdraws the beneficiary's funds.
    */
   function beneficiaryWithdraw() public {
-    require(state == State.Closed);
-    beneficiary.transfer(address(this).balance);
+    require(state_ == State.Closed);
+    beneficiary_.transfer(address(this).balance);
   }
 
   /**
    * @dev Returns whether refundees can withdraw their deposits (be refunded).
    */
   function withdrawalAllowed(address _payee) public view returns (bool) {
-    return state == State.Refunding;
+    return state_ == State.Refunding;
   }
 }