Merge pull request #222 from aragon/refactor

[VestedToken] Add revokability and burnablity options and general refactor and optimizations

contracts/token/VestedToken.sol

@@ -1,6 +1,5 @@
 pragma solidity ^0.4.8;
 
-
 import "./StandardToken.sol";
 import "./LimitedTransferToken.sol";
 
@@ -10,15 +9,19 @@ import "./LimitedTransferToken.sol";
  */
 contract VestedToken is StandardToken, LimitedTransferToken {
   struct TokenGrant {
-    address granter;
-    uint256 value;
+    address granter;     // 20 bytes
+    uint256 value;       // 32 bytes
     uint64 cliff;
     uint64 vesting;
-    uint64 start;
-  }
+    uint64 start;        // 3 * 8 = 24 bytes
+    bool revokable;
+    bool burnsOnRevoke;  // 2 * 1 = 2 bits? or 2 bytes?
+  } // total 78 bytes = 3 sstore per operation (32 per sstore)
 
   mapping (address => TokenGrant[]) public grants;
 
+  event NewTokenGrant(address indexed from, address indexed to, uint256 value, uint256 grantId);
+
   /**
    * @dev Grant tokens to a specified address
    * @param _to address The address which the tokens will be granted to.
@@ -32,47 +35,88 @@ contract VestedToken is StandardToken, LimitedTransferToken {
     uint256 _value,
     uint64 _start,
     uint64 _cliff,
-    uint64 _vesting) {
+    uint64 _vesting,
+    bool _revokable,
+    bool _burnsOnRevoke
+  ) public {
 
-    if (_cliff < _start) {
+    // Check for date inconsistencies that may cause unexpected behavior
+    if (_cliff < _start || _vesting < _cliff) {
       throw;
     }
-    if (_vesting < _start) {
-      throw;
-    }
-    if (_vesting < _cliff) {
-      throw;
-    }
-
 
-    TokenGrant memory grant = TokenGrant(msg.sender, _value, _cliff, _vesting, _start);
-    grants[_to].push(grant);
+    uint count = grants[_to].push(
+                TokenGrant(
+                  _revokable ? msg.sender : 0, // avoid storing an extra 20 bytes when it is non-revokable
+                  _value,
+                  _cliff,
+                  _vesting,
+                  _start,
+                  _revokable,
+                  _burnsOnRevoke
+                )
+              );
 
     transfer(_to, _value);
-  }
 
+    NewTokenGrant(msg.sender, _to, _value, count - 1);
+  }
 
   /**
    * @dev Revoke the grant of tokens of a specifed address.
    * @param _holder The address which will have its tokens revoked.
    * @param _grantId The id of the token grant.
    */
-  function revokeTokenGrant(address _holder, uint _grantId) {
+  function revokeTokenGrant(address _holder, uint _grantId) public {
     TokenGrant grant = grants[_holder][_grantId];
 
-    if (grant.granter != msg.sender) {
+    if (!grant.revokable) { // Check if grant was revokable
       throw;
     }
+
+    if (grant.granter != msg.sender) { // Only granter can revoke it
+      throw;
+    }
+
+    address receiver = grant.burnsOnRevoke ? 0xdead : msg.sender;
+
     uint256 nonVested = nonVestedTokens(grant, uint64(now));
 
     // remove grant from array
     delete grants[_holder][_grantId];
-    grants[_holder][_grantId] = grants[_holder][grants[_holder].length - 1];
+    grants[_holder][_grantId] = grants[_holder][grants[_holder].length.sub(1)];
     grants[_holder].length -= 1;
 
-    balances[msg.sender] = balances[msg.sender].add(nonVested);
+    balances[receiver] = balances[receiver].add(nonVested);
     balances[_holder] = balances[_holder].sub(nonVested);
-    Transfer(_holder, msg.sender, nonVested);
+
+    Transfer(_holder, receiver, nonVested);
+  }
+
+
+  /**
+   * @dev Calculate the total amount of transferable tokens of a holder at a given time
+   * @param holder address The address of the holder
+   * @param time uint64 The specific time.
+   * @return An uint representing a holder's total amount of transferable tokens.
+   */
+  function transferableTokens(address holder, uint64 time) constant public returns (uint256) {
+    uint256 grantIndex = tokenGrantsCount(holder);
+
+    if (grantIndex == 0) return balanceOf(holder); // shortcut for holder without grants
+
+    // Iterate through all the grants the holder has, and add all non-vested tokens
+    uint256 nonVested = 0;
+    for (uint256 i = 0; i < grantIndex; i++) {
+      nonVested = SafeMath.add(nonVested, nonVestedTokens(grants[holder][i], time));
+    }
+
+    // Balance - totalNonVested is the amount of tokens a holder can transfer at any given time
+    uint256 vestedTransferable = SafeMath.sub(balanceOf(holder), nonVested);
+
+    // Return the minimum of how many vested can transfer and other value
+    // in case there are other limiting transferability factors (default is balanceOf)
+    return SafeMath.min256(vestedTransferable, super.transferableTokens(holder, time));
   }
 
   /**
@@ -84,14 +128,65 @@ contract VestedToken is StandardToken, LimitedTransferToken {
     return grants[_holder].length;
   }
 
+  /**
+   * @dev Calculate amount of vested tokens at a specifc time.
+   * @param tokens uint256 The amount of tokens grantted.
+   * @param time uint64 The time to be checked
+   * @param start uint64 A time representing the begining of the grant
+   * @param cliff uint64 The cliff period.
+   * @param vesting uint64 The vesting period.
+   * @return An uint representing the amount of vested tokensof a specif grant.
+   *  transferableTokens
+   *   |                         _/--------   vestedTokens rect
+   *   |                       _/
+   *   |                     _/
+   *   |                   _/
+   *   |                 _/
+   *   |                /
+   *   |              .|
+   *   |            .  |
+   *   |          .    |
+   *   |        .      |
+   *   |      .        |
+   *   |    .          |
+   *   +===+===========+---------+----------> time
+   *      Start       Clift    Vesting
+   */
+  function calculateVestedTokens(
+    uint256 tokens,
+    uint256 time,
+    uint256 start,
+    uint256 cliff,
+    uint256 vesting) constant returns (uint256)
+    {
+      // Shortcuts for before cliff and after vesting cases.
+      if (time < cliff) return 0;
+      if (time >= vesting) return tokens;
+
+      // Interpolate all vested tokens.
+      // As before cliff the shortcut returns 0, we can use just calculate a value
+      // in the vesting rect (as shown in above's figure)
+
+      // vestedTokens = tokens * (time - start) / (vesting - start)
+      uint256 vestedTokens = SafeMath.div(
+                                    SafeMath.mul(
+                                      tokens,
+                                      SafeMath.sub(time, start)
+                                      ),
+                                    SafeMath.sub(vesting, start)
+                                    );
+
+      return vestedTokens;
+  }
+
   /**
    * @dev Get all information about a specifc grant.
    * @param _holder The address which will have its tokens revoked.
    * @param _grantId The id of the token grant.
-   * @return Returns all the values that represent a TokenGrant(address, value, start, cliff 
-   * and vesting) plus the vested value at the current time.
+   * @return Returns all the values that represent a TokenGrant(address, value, start, cliff,
+   * revokability, burnsOnRevoke, and vesting) plus the vested value at the current time.
    */
-  function tokenGrant(address _holder, uint _grantId) constant returns (address granter, uint256 value, uint256 vested, uint64 start, uint64 cliff, uint64 vesting) {
+  function tokenGrant(address _holder, uint _grantId) constant returns (address granter, uint256 value, uint256 vested, uint64 start, uint64 cliff, uint64 vesting, bool revokable, bool burnsOnRevoke) {
     TokenGrant grant = grants[_holder][_grantId];
 
     granter = grant.granter;
@@ -99,6 +194,8 @@ contract VestedToken is StandardToken, LimitedTransferToken {
     start = grant.start;
     cliff = grant.cliff;
     vesting = grant.vesting;
+    revokable = grant.revokable;
+    burnsOnRevoke = grant.burnsOnRevoke;
 
     vested = vestedTokens(grant, uint64(now));
   }
@@ -119,38 +216,6 @@ contract VestedToken is StandardToken, LimitedTransferToken {
     );
   }
 
-  /**
-   * @dev Calculate amount of vested tokens at a specifc time.
-   * @param tokens uint256 The amount of tokens grantted.
-   * @param time uint64 The time to be checked
-   * @param start uint64 A time representing the begining of the grant
-   * @param cliff uint64 The cliff period.
-   * @param vesting uint64 The vesting period.
-   * @return An uint representing the amount of vested tokensof a specif grant.
-   */
-  function calculateVestedTokens(
-    uint256 tokens,
-    uint256 time,
-    uint256 start,
-    uint256 cliff,
-    uint256 vesting) constant returns (uint256 vestedTokens)
-    {
-
-    if (time < cliff) {
-      return 0;
-    }
-    if (time >= vesting) {
-      return tokens;
-    }
-
-    uint256 cliffTokens = tokens.mul(cliff.sub(start)).div(vesting.sub(start));
-    vestedTokens = cliffTokens;
-
-    uint256 vestingTokens = tokens.sub(cliffTokens);
-
-    vestedTokens = vestedTokens.add(vestingTokens.mul(time.sub(cliff)).div(vesting.sub(cliff)));
-  }
-
   /**
    * @dev Calculate the amount of non vested tokens at a specific time.
    * @param grant TokenGrant The grant to be checked.
@@ -174,20 +239,4 @@ contract VestedToken is StandardToken, LimitedTransferToken {
       date = SafeMath.max64(grants[holder][i].vesting, date);
     }
   }
-
-  /**
-   * @dev Calculate the total amount of transferable tokens of a holder at a given time
-   * @param holder address The address of the holder
-   * @param time uint64 The specific time.
-   * @return An uint representing a holder's total amount of transferable tokens.
-   */
-  function transferableTokens(address holder, uint64 time) constant public returns (uint256 nonVested) {
-    uint256 grantIndex = grants[holder].length;
-    for (uint256 i = 0; i < grantIndex; i++) {
-      uint256 current = nonVestedTokens(grants[holder][i], time);
-      nonVested = nonVested.add(current);
-    }
-
-    return SafeMath.min256(balances[holder].sub(nonVested), super.transferableTokens(holder, time));
-  }
 }

test/VestedToken.js

@@ -23,12 +23,12 @@ contract('VestedToken', function(accounts) {
     assert.equal(await token.transferableTokens(receiver, now), tokenAmount);
   })
 
-  describe('getting a token grant', async () => {
+  describe('getting a revokable/non-burnable token grant', async () => {
     const cliff = 10000
     const vesting = 20000 // seconds
 
     beforeEach(async () => {
-      await token.grantVestedTokens(receiver, tokenAmount, now, now + cliff, now + vesting, { from: granter })
+      await token.grantVestedTokens(receiver, tokenAmount, now, now + cliff, now + vesting, true, false, { from: granter })
     })
 
     it('tokens are received', async () => {
@@ -103,7 +103,8 @@ contract('VestedToken', function(accounts) {
 
       let newNow = web3.eth.getBlock(web3.eth.blockNumber).timestamp
 
-      await token.grantVestedTokens(receiver, tokenAmount, newNow, newNow + cliff, newNow + vesting, { from: granter })
+      await token.grantVestedTokens(receiver, tokenAmount, newNow, newNow + cliff, newNow + vesting, false, false, { from: granter })
+
       await token.transfer(accounts[7], 13, { from: receiver })
       assert.equal(await token.balanceOf(accounts[7]), tokenAmount / 2);
 
@@ -114,4 +115,61 @@ contract('VestedToken', function(accounts) {
       assert.equal(await token.balanceOf(accounts[7]), tokenAmount * 2)
     })
   })
+
+  describe('getting a non-revokable token grant', async () => {
+    const cliff = 10000
+    const vesting = 20000 // seconds
+
+    beforeEach(async () => {
+      await token.grantVestedTokens(receiver, tokenAmount, now, now + cliff, now + vesting, false, false, { from: granter })
+    })
+
+    it('tokens are received', async () => {
+      assert.equal(await token.balanceOf(receiver), tokenAmount);
+    })
+
+    it('throws when granter attempts to revoke', async () => {
+      try {
+        await token.revokeTokenGrant(receiver, 0, { from: granter });
+      } catch(error) {
+        return assertJump(error);
+      }
+      assert.fail('should have thrown before');
+    })
+  })
+
+  describe('getting a revokable/burnable token grant', async () => {
+    const cliff = 100000
+    const vesting = 200000 // seconds
+    const burnAddress = '0x000000000000000000000000000000000000dead'
+
+    beforeEach(async () => {
+      await token.grantVestedTokens(receiver, tokenAmount, now, now + cliff, now + vesting, true, true, { from: granter })
+    })
+
+    it('tokens are received', async () => {
+      assert.equal(await token.balanceOf(receiver), tokenAmount);
+    })
+
+    it('can be revoked by granter and tokens are burned', async () => {
+      await token.revokeTokenGrant(receiver, 0, { from: granter });
+      assert.equal(await token.balanceOf(receiver), 0);
+      assert.equal(await token.balanceOf(burnAddress), tokenAmount);
+    })
+
+    it('cannot be revoked by non granter', async () => {
+      try {
+        await token.revokeTokenGrant(receiver, 0, { from: accounts[3] });
+      } catch(error) {
+        return assertJump(error);
+      }
+      assert.fail('should have thrown before');
+    })
+
+    it('can be revoked by granter and non vested tokens are returned', async () => {
+      await timer(cliff);
+      await token.revokeTokenGrant(receiver, 0, { from: granter });
+      assert.equal(await token.balanceOf(burnAddress), tokenAmount * cliff / vesting);
+    })
+  })
 });