Improve encapsulation on ERCs (#1270)

* Improve encapsulation on ERC165

* Improve encapsulation on ERC20

* Improve encapsulation on ERC721

* Add tests, use standard getters

* fix tests

* Fix lint

* move interface ids to implementation contracts

* Do not prefix getters

Author: Leo Arias

contracts/drafts/TokenVesting.sol

@@ -21,44 +21,93 @@ contract TokenVesting is Ownable {
   event Revoked();
 
   // beneficiary of tokens after they are released
-  address public beneficiary;
+  address private beneficiary_;
 
-  uint256 public cliff;
-  uint256 public start;
-  uint256 public duration;
+  uint256 private cliff_;
+  uint256 private start_;
+  uint256 private duration_;
 
-  bool public revocable;
+  bool private revocable_;
 
-  mapping (address => uint256) public released;
-  mapping (address => bool) public revoked;
+  mapping (address => uint256) private released_;
+  mapping (address => bool) private revoked_;
 
   /**
    * @dev Creates a vesting contract that vests its balance of any ERC20 token to the
    * _beneficiary, gradually in a linear fashion until _start + _duration. By then all
    * of the balance will have vested.
    * @param _beneficiary address of the beneficiary to whom vested tokens are transferred
-   * @param _cliff duration in seconds of the cliff in which tokens will begin to vest
+   * @param _cliffDuration duration in seconds of the cliff in which tokens will begin to vest
    * @param _start the time (as Unix time) at which point vesting starts
    * @param _duration duration in seconds of the period in which the tokens will vest
    * @param _revocable whether the vesting is revocable or not
    */
   constructor(
     address _beneficiary,
     uint256 _start,
-    uint256 _cliff,
+    uint256 _cliffDuration,
     uint256 _duration,
     bool _revocable
   )
     public
   {
     require(_beneficiary != address(0));
-    require(_cliff <= _duration);
+    require(_cliffDuration <= _duration);
 
-    beneficiary = _beneficiary;
-    revocable = _revocable;
-    duration = _duration;
-    cliff = _start.add(_cliff);
-    start = _start;
+    beneficiary_ = _beneficiary;
+    revocable_ = _revocable;
+    duration_ = _duration;
+    cliff_ = _start.add(_cliffDuration);
+    start_ = _start;
+  }
+
+  /**
+   * @return the beneficiary of the tokens.
+   */
+  function beneficiary() public view returns(address) {
+    return beneficiary_;
+  }
+
+  /**
+   * @return the cliff time of the token vesting.
+   */
+  function cliff() public view returns(uint256) {
+    return cliff_;
+  }
+
+  /**
+   * @return the start time of the token vesting.
+   */
+  function start() public view returns(uint256) {
+    return start_;
+  }
+
+  /**
+   * @return the duration of the token vesting.
+   */
+  function duration() public view returns(uint256) {
+    return duration_;
+  }
+
+  /**
+   * @return true if the vesting is revocable.
+   */
+  function revocable() public view returns(bool) {
+    return revocable_;
+  }
+
+  /**
+   * @return the amount of the token released.
+   */
+  function released(address _token) public view returns(uint256) {
+    return released_[_token];
+  }
+
+  /**
+   * @return true if the token is revoked.
+   */
+  function revoked(address _token) public view returns(bool) {
+    return revoked_[_token];
   }
 
   /**
@@ -70,9 +119,9 @@ contract TokenVesting is Ownable {
 
     require(unreleased > 0);
 
-    released[_token] = released[_token].add(unreleased);
+    released_[_token] = released_[_token].add(unreleased);
 
-    _token.safeTransfer(beneficiary, unreleased);
+    _token.safeTransfer(beneficiary_, unreleased);
 
     emit Released(unreleased);
   }
@@ -83,15 +132,15 @@ contract TokenVesting is Ownable {
    * @param _token ERC20 token which is being vested
    */
   function revoke(IERC20 _token) public onlyOwner {
-    require(revocable);
-    require(!revoked[_token]);
+    require(revocable_);
+    require(!revoked_[_token]);
 
     uint256 balance = _token.balanceOf(address(this));
 
     uint256 unreleased = releasableAmount(_token);
     uint256 refund = balance.sub(unreleased);
 
-    revoked[_token] = true;
+    revoked_[_token] = true;
 
     _token.safeTransfer(owner(), refund);
 
@@ -103,7 +152,7 @@ contract TokenVesting is Ownable {
    * @param _token ERC20 token which is being vested
    */
   function releasableAmount(IERC20 _token) public view returns (uint256) {
-    return vestedAmount(_token).sub(released[_token]);
+    return vestedAmount(_token).sub(released_[_token]);
   }
 
   /**
@@ -112,14 +161,14 @@ contract TokenVesting is Ownable {
    */
   function vestedAmount(IERC20 _token) public view returns (uint256) {
     uint256 currentBalance = _token.balanceOf(this);
-    uint256 totalBalance = currentBalance.add(released[_token]);
+    uint256 totalBalance = currentBalance.add(released_[_token]);
 
-    if (block.timestamp < cliff) {
+    if (block.timestamp < cliff_) {
       return 0;
-    } else if (block.timestamp >= start.add(duration) || revoked[_token]) {
+    } else if (block.timestamp >= start_.add(duration_) || revoked_[_token]) {
       return totalBalance;
     } else {
-      return totalBalance.mul(block.timestamp.sub(start)).div(duration);
+      return totalBalance.mul(block.timestamp.sub(start_)).div(duration_);
     }
   }
 }

contracts/introspection/SupportsInterfaceWithLookup.sol

@@ -10,7 +10,7 @@ import "./IERC165.sol";
  */
 contract SupportsInterfaceWithLookup is IERC165 {
 
-  bytes4 public constant InterfaceId_ERC165 = 0x01ffc9a7;
+  bytes4 private constant InterfaceId_ERC165 = 0x01ffc9a7;
   /**
    * 0x01ffc9a7 ===
    *   bytes4(keccak256('supportsInterface(bytes4)'))

contracts/token/ERC20/ERC20Capped.sol

@@ -9,11 +9,18 @@ import "./ERC20Mintable.sol";
  */
 contract ERC20Capped is ERC20Mintable {
 
-  uint256 public cap;
+  uint256 private cap_;
 
   constructor(uint256 _cap) public {
     require(_cap > 0);
-    cap = _cap;
+    cap_ = _cap;
+  }
+
+  /**
+   * @return the cap for the token minting.
+   */
+  function cap() public view returns(uint256) {
+    return cap_;
   }
 
   /**
@@ -29,7 +36,7 @@ contract ERC20Capped is ERC20Mintable {
     public
     returns (bool)
   {
-    require(totalSupply().add(_amount) <= cap);
+    require(totalSupply().add(_amount) <= cap_);
 
     return super.mint(_to, _amount);
   }

contracts/token/ERC20/ERC20Detailed.sol

@@ -10,13 +10,34 @@ import "./IERC20.sol";
  * just as on Ethereum all the operations are done in wei.
  */
 contract ERC20Detailed is IERC20 {
-  string public name;
-  string public symbol;
-  uint8 public decimals;
+  string private name_;
+  string private symbol_;
+  uint8 private decimals_;
 
   constructor(string _name, string _symbol, uint8 _decimals) public {
-    name = _name;
-    symbol = _symbol;
-    decimals = _decimals;
+    name_ = _name;
+    symbol_ = _symbol;
+    decimals_ = _decimals;
+  }
+
+  /**
+   * @return the name of the token.
+   */
+  function name() public view returns(string) {
+    return name_;
+  }
+
+  /**
+   * @return the symbol of the token.
+   */
+  function symbol() public view returns(string) {
+    return symbol_;
+  }
+
+  /**
+   * @return the number of decimals of the token.
+   */
+  function decimals() public view returns(uint8) {
+    return decimals_;
   }
 }

contracts/token/ERC20/ERC20Mintable.sol

@@ -13,11 +13,11 @@ contract ERC20Mintable is ERC20, Ownable {
   event Mint(address indexed to, uint256 amount);
   event MintFinished();
 
-  bool public mintingFinished = false;
+  bool private mintingFinished_ = false;
 
 
   modifier canMint() {
-    require(!mintingFinished);
+    require(!mintingFinished_);
     _;
   }
 
@@ -26,6 +26,13 @@ contract ERC20Mintable is ERC20, Ownable {
     _;
   }
 
+  /**
+   * @return true if the minting is finished.
+   */
+  function mintingFinished() public view returns(bool) {
+    return mintingFinished_;
+  }
+
   /**
    * @dev Function to mint tokens
    * @param _to The address that will receive the minted tokens.
@@ -51,7 +58,7 @@ contract ERC20Mintable is ERC20, Ownable {
    * @return True if the operation was successful.
    */
   function finishMinting() public onlyOwner canMint returns (bool) {
-    mintingFinished = true;
+    mintingFinished_ = true;
     emit MintFinished();
     return true;
   }

contracts/token/ERC20/TokenTimelock.sol

@@ -12,13 +12,13 @@ contract TokenTimelock {
   using SafeERC20 for IERC20;
 
   // ERC20 basic token contract being held
-  IERC20 public token;
+  IERC20 private token_;
 
   // beneficiary of tokens after they are released
-  address public beneficiary;
+  address private beneficiary_;
 
   // timestamp when token release is enabled
-  uint256 public releaseTime;
+  uint256 private releaseTime_;
 
   constructor(
     IERC20 _token,
@@ -29,21 +29,42 @@ contract TokenTimelock {
   {
     // solium-disable-next-line security/no-block-members
     require(_releaseTime > block.timestamp);
-    token = _token;
-    beneficiary = _beneficiary;
-    releaseTime = _releaseTime;
+    token_ = _token;
+    beneficiary_ = _beneficiary;
+    releaseTime_ = _releaseTime;
+  }
+
+  /**
+   * @return the token being held.
+   */
+  function token() public view returns(IERC20) {
+    return token_;
+  }
+
+  /**
+   * @return the beneficiary of the tokens.
+   */
+  function beneficiary() public view returns(address) {
+    return beneficiary_;
+  }
+
+  /**
+   * @return the time when the tokens are released.
+   */
+  function releaseTime() public view returns(uint256) {
+    return releaseTime_;
   }
 
   /**
    * @notice Transfers tokens held by timelock to beneficiary.
    */
   function release() public {
     // solium-disable-next-line security/no-block-members
-    require(block.timestamp >= releaseTime);
+    require(block.timestamp >= releaseTime_);
 
-    uint256 amount = token.balanceOf(address(this));
+    uint256 amount = token_.balanceOf(address(this));
     require(amount > 0);
 
-    token.safeTransfer(beneficiary, amount);
+    token_.safeTransfer(beneficiary_, amount);
   }
 }