Browse Source

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
Leo Arias 7 years ago
parent
commit
7bb87237f3

+ 74 - 25
contracts/drafts/TokenVesting.sol

@@ -21,23 +21,23 @@ 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
@@ -45,20 +45,69 @@ contract TokenVesting is Ownable {
   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_);
     }
   }
 }

+ 1 - 1
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)'))

+ 10 - 3
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);
   }

+ 27 - 6
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_;
   }
 }

+ 10 - 3
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;
   }

+ 30 - 9
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,9 +29,30 @@ 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_;
   }
 
   /**
@@ -39,11 +60,11 @@ contract TokenTimelock {
    */
   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);
   }
 }

+ 48 - 32
contracts/token/ERC721/ERC721.sol

@@ -20,19 +20,35 @@ contract ERC721 is SupportsInterfaceWithLookup, ERC721Basic, IERC721 {
   string internal symbol_;
 
   // Mapping from owner to list of owned token IDs
-  mapping(address => uint256[]) internal ownedTokens;
+  mapping(address => uint256[]) private ownedTokens_;
 
   // Mapping from token ID to index of the owner tokens list
-  mapping(uint256 => uint256) internal ownedTokensIndex;
+  mapping(uint256 => uint256) private ownedTokensIndex_;
 
   // Array with all token ids, used for enumeration
-  uint256[] internal allTokens;
+  uint256[] private allTokens_;
 
   // Mapping from token id to position in the allTokens array
-  mapping(uint256 => uint256) internal allTokensIndex;
+  mapping(uint256 => uint256) private allTokensIndex_;
 
   // Optional mapping for token URIs
-  mapping(uint256 => string) internal tokenURIs;
+  mapping(uint256 => string) private tokenURIs_;
+
+  bytes4 private constant InterfaceId_ERC721Enumerable = 0x780e9d63;
+  /**
+   * 0x780e9d63 ===
+   *   bytes4(keccak256('totalSupply()')) ^
+   *   bytes4(keccak256('tokenOfOwnerByIndex(address,uint256)')) ^
+   *   bytes4(keccak256('tokenByIndex(uint256)'))
+   */
+
+  bytes4 private constant InterfaceId_ERC721Metadata = 0x5b5e139f;
+  /**
+   * 0x5b5e139f ===
+   *   bytes4(keccak256('name()')) ^
+   *   bytes4(keccak256('symbol()')) ^
+   *   bytes4(keccak256('tokenURI(uint256)'))
+   */
 
   /**
    * @dev Constructor function
@@ -69,7 +85,7 @@ contract ERC721 is SupportsInterfaceWithLookup, ERC721Basic, IERC721 {
    */
   function tokenURI(uint256 _tokenId) public view returns (string) {
     require(_exists(_tokenId));
-    return tokenURIs[_tokenId];
+    return tokenURIs_[_tokenId];
   }
 
   /**
@@ -87,7 +103,7 @@ contract ERC721 is SupportsInterfaceWithLookup, ERC721Basic, IERC721 {
     returns (uint256)
   {
     require(_index < balanceOf(_owner));
-    return ownedTokens[_owner][_index];
+    return ownedTokens_[_owner][_index];
   }
 
   /**
@@ -95,7 +111,7 @@ contract ERC721 is SupportsInterfaceWithLookup, ERC721Basic, IERC721 {
    * @return uint256 representing the total amount of tokens
    */
   function totalSupply() public view returns (uint256) {
-    return allTokens.length;
+    return allTokens_.length;
   }
 
   /**
@@ -106,7 +122,7 @@ contract ERC721 is SupportsInterfaceWithLookup, ERC721Basic, IERC721 {
    */
   function tokenByIndex(uint256 _index) public view returns (uint256) {
     require(_index < totalSupply());
-    return allTokens[_index];
+    return allTokens_[_index];
   }
 
   /**
@@ -117,7 +133,7 @@ contract ERC721 is SupportsInterfaceWithLookup, ERC721Basic, IERC721 {
    */
   function _setTokenURI(uint256 _tokenId, string _uri) internal {
     require(_exists(_tokenId));
-    tokenURIs[_tokenId] = _uri;
+    tokenURIs_[_tokenId] = _uri;
   }
 
   /**
@@ -127,9 +143,9 @@ contract ERC721 is SupportsInterfaceWithLookup, ERC721Basic, IERC721 {
    */
   function _addTokenTo(address _to, uint256 _tokenId) internal {
     super._addTokenTo(_to, _tokenId);
-    uint256 length = ownedTokens[_to].length;
-    ownedTokens[_to].push(_tokenId);
-    ownedTokensIndex[_tokenId] = length;
+    uint256 length = ownedTokens_[_to].length;
+    ownedTokens_[_to].push(_tokenId);
+    ownedTokensIndex_[_tokenId] = length;
   }
 
   /**
@@ -142,20 +158,20 @@ contract ERC721 is SupportsInterfaceWithLookup, ERC721Basic, IERC721 {
 
     // To prevent a gap in the array, we store the last token in the index of the token to delete, and
     // then delete the last slot.
-    uint256 tokenIndex = ownedTokensIndex[_tokenId];
-    uint256 lastTokenIndex = ownedTokens[_from].length.sub(1);
-    uint256 lastToken = ownedTokens[_from][lastTokenIndex];
+    uint256 tokenIndex = ownedTokensIndex_[_tokenId];
+    uint256 lastTokenIndex = ownedTokens_[_from].length.sub(1);
+    uint256 lastToken = ownedTokens_[_from][lastTokenIndex];
 
-    ownedTokens[_from][tokenIndex] = lastToken;
+    ownedTokens_[_from][tokenIndex] = lastToken;
     // This also deletes the contents at the last position of the array
-    ownedTokens[_from].length--;
+    ownedTokens_[_from].length--;
 
     // Note that this will handle single-element arrays. In that case, both tokenIndex and lastTokenIndex are going to
     // be zero. Then we can make sure that we will remove _tokenId from the ownedTokens list since we are first swapping
     // the lastToken to the first position, and then dropping the element placed in the last position of the list
 
-    ownedTokensIndex[_tokenId] = 0;
-    ownedTokensIndex[lastToken] = tokenIndex;
+    ownedTokensIndex_[_tokenId] = 0;
+    ownedTokensIndex_[lastToken] = tokenIndex;
   }
 
   /**
@@ -167,8 +183,8 @@ contract ERC721 is SupportsInterfaceWithLookup, ERC721Basic, IERC721 {
   function _mint(address _to, uint256 _tokenId) internal {
     super._mint(_to, _tokenId);
 
-    allTokensIndex[_tokenId] = allTokens.length;
-    allTokens.push(_tokenId);
+    allTokensIndex_[_tokenId] = allTokens_.length;
+    allTokens_.push(_tokenId);
   }
 
   /**
@@ -181,21 +197,21 @@ contract ERC721 is SupportsInterfaceWithLookup, ERC721Basic, IERC721 {
     super._burn(_owner, _tokenId);
 
     // Clear metadata (if any)
-    if (bytes(tokenURIs[_tokenId]).length != 0) {
-      delete tokenURIs[_tokenId];
+    if (bytes(tokenURIs_[_tokenId]).length != 0) {
+      delete tokenURIs_[_tokenId];
     }
 
     // Reorg all tokens array
-    uint256 tokenIndex = allTokensIndex[_tokenId];
-    uint256 lastTokenIndex = allTokens.length.sub(1);
-    uint256 lastToken = allTokens[lastTokenIndex];
+    uint256 tokenIndex = allTokensIndex_[_tokenId];
+    uint256 lastTokenIndex = allTokens_.length.sub(1);
+    uint256 lastToken = allTokens_[lastTokenIndex];
 
-    allTokens[tokenIndex] = lastToken;
-    allTokens[lastTokenIndex] = 0;
+    allTokens_[tokenIndex] = lastToken;
+    allTokens_[lastTokenIndex] = 0;
 
-    allTokens.length--;
-    allTokensIndex[_tokenId] = 0;
-    allTokensIndex[lastToken] = tokenIndex;
+    allTokens_.length--;
+    allTokensIndex_[_tokenId] = 0;
+    allTokensIndex_[lastToken] = tokenIndex;
   }
 
 }

+ 32 - 18
contracts/token/ERC721/ERC721Basic.sol

@@ -21,16 +21,30 @@ contract ERC721Basic is SupportsInterfaceWithLookup, IERC721Basic {
   bytes4 private constant ERC721_RECEIVED = 0x150b7a02;
 
   // Mapping from token ID to owner
-  mapping (uint256 => address) internal tokenOwner;
+  mapping (uint256 => address) private tokenOwner_;
 
   // Mapping from token ID to approved address
-  mapping (uint256 => address) internal tokenApprovals;
+  mapping (uint256 => address) private tokenApprovals_;
 
   // Mapping from owner to number of owned token
-  mapping (address => uint256) internal ownedTokensCount;
+  mapping (address => uint256) private ownedTokensCount_;
 
   // Mapping from owner to operator approvals
-  mapping (address => mapping (address => bool)) internal operatorApprovals;
+  mapping (address => mapping (address => bool)) private operatorApprovals_;
+
+  bytes4 private constant InterfaceId_ERC721 = 0x80ac58cd;
+  /*
+   * 0x80ac58cd ===
+   *   bytes4(keccak256('balanceOf(address)')) ^
+   *   bytes4(keccak256('ownerOf(uint256)')) ^
+   *   bytes4(keccak256('approve(address,uint256)')) ^
+   *   bytes4(keccak256('getApproved(uint256)')) ^
+   *   bytes4(keccak256('setApprovalForAll(address,bool)')) ^
+   *   bytes4(keccak256('isApprovedForAll(address,address)')) ^
+   *   bytes4(keccak256('transferFrom(address,address,uint256)')) ^
+   *   bytes4(keccak256('safeTransferFrom(address,address,uint256)')) ^
+   *   bytes4(keccak256('safeTransferFrom(address,address,uint256,bytes)'))
+   */
 
   constructor()
     public
@@ -46,7 +60,7 @@ contract ERC721Basic is SupportsInterfaceWithLookup, IERC721Basic {
    */
   function balanceOf(address _owner) public view returns (uint256) {
     require(_owner != address(0));
-    return ownedTokensCount[_owner];
+    return ownedTokensCount_[_owner];
   }
 
   /**
@@ -55,7 +69,7 @@ contract ERC721Basic is SupportsInterfaceWithLookup, IERC721Basic {
    * @return owner address currently marked as the owner of the given token ID
    */
   function ownerOf(uint256 _tokenId) public view returns (address) {
-    address owner = tokenOwner[_tokenId];
+    address owner = tokenOwner_[_tokenId];
     require(owner != address(0));
     return owner;
   }
@@ -73,7 +87,7 @@ contract ERC721Basic is SupportsInterfaceWithLookup, IERC721Basic {
     require(_to != owner);
     require(msg.sender == owner || isApprovedForAll(owner, msg.sender));
 
-    tokenApprovals[_tokenId] = _to;
+    tokenApprovals_[_tokenId] = _to;
     emit Approval(owner, _to, _tokenId);
   }
 
@@ -83,7 +97,7 @@ contract ERC721Basic is SupportsInterfaceWithLookup, IERC721Basic {
    * @return address currently approved for the given token ID
    */
   function getApproved(uint256 _tokenId) public view returns (address) {
-    return tokenApprovals[_tokenId];
+    return tokenApprovals_[_tokenId];
   }
 
   /**
@@ -94,7 +108,7 @@ contract ERC721Basic is SupportsInterfaceWithLookup, IERC721Basic {
    */
   function setApprovalForAll(address _to, bool _approved) public {
     require(_to != msg.sender);
-    operatorApprovals[msg.sender][_to] = _approved;
+    operatorApprovals_[msg.sender][_to] = _approved;
     emit ApprovalForAll(msg.sender, _to, _approved);
   }
 
@@ -112,7 +126,7 @@ contract ERC721Basic is SupportsInterfaceWithLookup, IERC721Basic {
     view
     returns (bool)
   {
-    return operatorApprovals[_owner][_operator];
+    return operatorApprovals_[_owner][_operator];
   }
 
   /**
@@ -194,7 +208,7 @@ contract ERC721Basic is SupportsInterfaceWithLookup, IERC721Basic {
    * @return whether the token exists
    */
   function _exists(uint256 _tokenId) internal view returns (bool) {
-    address owner = tokenOwner[_tokenId];
+    address owner = tokenOwner_[_tokenId];
     return owner != address(0);
   }
 
@@ -255,8 +269,8 @@ contract ERC721Basic is SupportsInterfaceWithLookup, IERC721Basic {
    */
   function _clearApproval(address _owner, uint256 _tokenId) internal {
     require(ownerOf(_tokenId) == _owner);
-    if (tokenApprovals[_tokenId] != address(0)) {
-      tokenApprovals[_tokenId] = address(0);
+    if (tokenApprovals_[_tokenId] != address(0)) {
+      tokenApprovals_[_tokenId] = address(0);
     }
   }
 
@@ -266,9 +280,9 @@ contract ERC721Basic is SupportsInterfaceWithLookup, IERC721Basic {
    * @param _tokenId uint256 ID of the token to be added to the tokens list of the given address
    */
   function _addTokenTo(address _to, uint256 _tokenId) internal {
-    require(tokenOwner[_tokenId] == address(0));
-    tokenOwner[_tokenId] = _to;
-    ownedTokensCount[_to] = ownedTokensCount[_to].add(1);
+    require(tokenOwner_[_tokenId] == address(0));
+    tokenOwner_[_tokenId] = _to;
+    ownedTokensCount_[_to] = ownedTokensCount_[_to].add(1);
   }
 
   /**
@@ -278,8 +292,8 @@ contract ERC721Basic is SupportsInterfaceWithLookup, IERC721Basic {
    */
   function _removeTokenFrom(address _from, uint256 _tokenId) internal {
     require(ownerOf(_tokenId) == _from);
-    ownedTokensCount[_from] = ownedTokensCount[_from].sub(1);
-    tokenOwner[_tokenId] = address(0);
+    ownedTokensCount_[_from] = ownedTokensCount_[_from].sub(1);
+    tokenOwner_[_tokenId] = address(0);
   }
 
   /**

+ 0 - 30
contracts/token/ERC721/IERC721Basic.sol

@@ -9,36 +9,6 @@ import "../../introspection/IERC165.sol";
  */
 contract IERC721Basic is IERC165 {
 
-  bytes4 internal constant InterfaceId_ERC721 = 0x80ac58cd;
-  /*
-   * 0x80ac58cd ===
-   *   bytes4(keccak256('balanceOf(address)')) ^
-   *   bytes4(keccak256('ownerOf(uint256)')) ^
-   *   bytes4(keccak256('approve(address,uint256)')) ^
-   *   bytes4(keccak256('getApproved(uint256)')) ^
-   *   bytes4(keccak256('setApprovalForAll(address,bool)')) ^
-   *   bytes4(keccak256('isApprovedForAll(address,address)')) ^
-   *   bytes4(keccak256('transferFrom(address,address,uint256)')) ^
-   *   bytes4(keccak256('safeTransferFrom(address,address,uint256)')) ^
-   *   bytes4(keccak256('safeTransferFrom(address,address,uint256,bytes)'))
-   */
-
-  bytes4 internal constant InterfaceId_ERC721Enumerable = 0x780e9d63;
-  /**
-   * 0x780e9d63 ===
-   *   bytes4(keccak256('totalSupply()')) ^
-   *   bytes4(keccak256('tokenOfOwnerByIndex(address,uint256)')) ^
-   *   bytes4(keccak256('tokenByIndex(uint256)'))
-   */
-
-  bytes4 internal constant InterfaceId_ERC721Metadata = 0x5b5e139f;
-  /**
-   * 0x5b5e139f ===
-   *   bytes4(keccak256('name()')) ^
-   *   bytes4(keccak256('symbol()')) ^
-   *   bytes4(keccak256('tokenURI(uint256)'))
-   */
-
   event Transfer(
     address indexed from,
     address indexed to,

+ 0 - 0
test/token/ERC20/DetailedERC20.test.js → test/token/ERC20/ERC20Detailed.test.js


+ 6 - 0
test/token/ERC20/TokenTimelock.test.js

@@ -33,6 +33,12 @@ contract('TokenTimelock', function ([_, owner, beneficiary]) {
         await this.token.mint(this.timelock.address, amount, { from: owner });
       });
 
+      it('can get state', async function () {
+        (await this.timelock.token()).should.be.equal(this.token.address);
+        (await this.timelock.beneficiary()).should.be.equal(beneficiary);
+        (await this.timelock.releaseTime()).should.be.bignumber.equal(this.releaseTime);
+      });
+
       it('cannot be released before time limit', async function () {
         await expectThrow(this.timelock.release());
       });

+ 30 - 18
test/token/ERC20/TokenVesting.test.js

@@ -19,35 +19,44 @@ contract('TokenVesting', function ([_, owner, beneficiary]) {
 
   beforeEach(async function () {
     this.start = (await latestTime()) + duration.minutes(1); // +1 minute so it starts after contract instantiation
-    this.cliff = duration.years(1);
+    this.cliffDuration = duration.years(1);
     this.duration = duration.years(2);
   });
 
   it('rejects a duration shorter than the cliff', async function () {
-    const cliff = this.duration;
-    const duration = this.cliff;
+    const cliffDuration = this.duration;
+    const duration = this.cliffDuration;
 
-    cliff.should.be.gt(duration);
+    cliffDuration.should.be.gt(duration);
 
     await expectThrow(
-      TokenVesting.new(beneficiary, this.start, cliff, duration, true, { from: owner })
+      TokenVesting.new(beneficiary, this.start, cliffDuration, duration, true, { from: owner })
     );
   });
 
   it('requires a valid beneficiary', async function () {
     await expectThrow(
-      TokenVesting.new(ZERO_ADDRESS, this.start, this.cliff, this.duration, true, { from: owner })
+      TokenVesting.new(ZERO_ADDRESS, this.start, this.cliffDuration, this.duration, true, { from: owner })
     );
   });
 
   context('once deployed', function () {
     beforeEach(async function () {
-      this.vesting = await TokenVesting.new(beneficiary, this.start, this.cliff, this.duration, true, { from: owner });
+      this.vesting = await TokenVesting.new(
+        beneficiary, this.start, this.cliffDuration, this.duration, true, { from: owner });
 
       this.token = await ERC20Mintable.new({ from: owner });
       await this.token.mint(this.vesting.address, amount, { from: owner });
     });
 
+    it('can get state', async function () {
+      (await this.vesting.beneficiary()).should.be.equal(beneficiary);
+      (await this.vesting.cliff()).should.be.bignumber.equal(this.start + this.cliffDuration);
+      (await this.vesting.start()).should.be.bignumber.equal(this.start);
+      (await this.vesting.duration()).should.be.bignumber.equal(this.duration);
+      (await this.vesting.revocable()).should.be.equal(true);
+    });
+
     it('cannot be released before cliff', async function () {
       await expectThrow(
         this.vesting.release(this.token.address),
@@ -56,33 +65,34 @@ contract('TokenVesting', function ([_, owner, beneficiary]) {
     });
 
     it('can be released after cliff', async function () {
-      await increaseTimeTo(this.start + this.cliff + duration.weeks(1));
+      await increaseTimeTo(this.start + this.cliffDuration + duration.weeks(1));
       await this.vesting.release(this.token.address);
     });
 
     it('should release proper amount after cliff', async function () {
-      await increaseTimeTo(this.start + this.cliff);
+      await increaseTimeTo(this.start + this.cliffDuration);
 
       const { receipt } = await this.vesting.release(this.token.address);
       const block = await ethGetBlock(receipt.blockNumber);
       const releaseTime = block.timestamp;
 
-      (await this.token.balanceOf(beneficiary)).should.bignumber.equal(
-        amount.mul(releaseTime - this.start).div(this.duration).floor()
-      );
+      const releasedAmount = amount.mul(releaseTime - this.start).div(this.duration).floor();
+      (await this.token.balanceOf(beneficiary)).should.bignumber.equal(releasedAmount);
+      (await this.vesting.released(this.token.address)).should.bignumber.equal(releasedAmount);
     });
 
     it('should linearly release tokens during vesting period', async function () {
-      const vestingPeriod = this.duration - this.cliff;
+      const vestingPeriod = this.duration - this.cliffDuration;
       const checkpoints = 4;
 
       for (let i = 1; i <= checkpoints; i++) {
-        const now = this.start + this.cliff + i * (vestingPeriod / checkpoints);
+        const now = this.start + this.cliffDuration + i * (vestingPeriod / checkpoints);
         await increaseTimeTo(now);
 
         await this.vesting.release(this.token.address);
         const expectedVesting = amount.mul(now - this.start).div(this.duration).floor();
         (await this.token.balanceOf(beneficiary)).should.bignumber.equal(expectedVesting);
+        (await this.vesting.released(this.token.address)).should.bignumber.equal(expectedVesting);
       }
     });
 
@@ -90,15 +100,17 @@ contract('TokenVesting', function ([_, owner, beneficiary]) {
       await increaseTimeTo(this.start + this.duration);
       await this.vesting.release(this.token.address);
       (await this.token.balanceOf(beneficiary)).should.bignumber.equal(amount);
+      (await this.vesting.released(this.token.address)).should.bignumber.equal(amount);
     });
 
     it('should be revoked by owner if revocable is set', async function () {
       await this.vesting.revoke(this.token.address, { from: owner });
+      (await this.vesting.revoked(this.token.address)).should.equal(true);
     });
 
     it('should fail to be revoked by owner if revocable not set', async function () {
       const vesting = await TokenVesting.new(
-        beneficiary, this.start, this.cliff, this.duration, false, { from: owner }
+        beneficiary, this.start, this.cliffDuration, this.duration, false, { from: owner }
       );
 
       await expectThrow(
@@ -108,7 +120,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) {
     });
 
     it('should return the non-vested tokens when revoked by owner', async function () {
-      await increaseTimeTo(this.start + this.cliff + duration.weeks(12));
+      await increaseTimeTo(this.start + this.cliffDuration + duration.weeks(12));
 
       const vested = await this.vesting.vestedAmount(this.token.address);
 
@@ -118,7 +130,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) {
     });
 
     it('should keep the vested tokens when revoked by owner', async function () {
-      await increaseTimeTo(this.start + this.cliff + duration.weeks(12));
+      await increaseTimeTo(this.start + this.cliffDuration + duration.weeks(12));
 
       const vestedPre = await this.vesting.vestedAmount(this.token.address);
 
@@ -130,7 +142,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) {
     });
 
     it('should fail to be revoked a second time', async function () {
-      await increaseTimeTo(this.start + this.cliff + duration.weeks(12));
+      await increaseTimeTo(this.start + this.cliffDuration + duration.weeks(12));
 
       await this.vesting.vestedAmount(this.token.address);