Push progress

packages/ap-contracts/contracts/token/ICT/Checkpoint/Checkpoint.sol

@@ -7,64 +7,74 @@ import "./CheckpointStorage.sol";
 contract Checkpoint is CheckpointStorage {
 
     // Emit when new checkpoint created
-    event CheckpointCreated(uint256 indexed _checkpointId);
+    event CheckpointCreated(uint256 indexed checkpointId);
 
     /**
      * @notice Queries a value at a defined checkpoint
-     * @param _checkpoints is array of Checkpoint objects
-     * @param _checkpointId is the Checkpoint ID to query
-     * @param _currentValue is the Current value of checkpoint
+     * @param checkpoints array of Checkpoint objects
+     * @param timestamp timestamp to retrieve the value at
      * @return uint256
      */
     function getValueAt(
-        Checkpoint[] memory _checkpoints,
-        uint256 _checkpointId,
-        uint256 _currentValue
+        Checkpoint[] storage checkpoints,
+        uint256 timestamp
     ) 
-        public
-        pure 
-        returns(uint256)
+        internal
+        view 
+        returns (uint256)
     {
-        //Checkpoint id 0 is when the token is first created - everyone has a zero balance
-        if (_checkpointId == 0) {
-            return 0;
-        }
-        if (_checkpoints.length == 0) {
-            return _currentValue;
-        }
-        if (_checkpoints[0].checkpointId >= _checkpointId) {
-            return _checkpoints[0].value;
-        }
-        if (_checkpoints[_checkpoints.length - 1].checkpointId < _checkpointId) {
-            return _currentValue;
-        }
-        if (_checkpoints[_checkpoints.length - 1].checkpointId == _checkpointId) {
-            return _checkpoints[_checkpoints.length - 1].value;
-        }
+        // initially return 0
+        if (checkpoints.length == 0) return 0;
+
+        // Shortcut for the actual value
+        if (timestamp >= checkpoints[checkpoints.length - 1].timestamp)
+            return checkpoints[checkpoints.length - 1].value;
+        if (timestamp < checkpoints[0].timestamp) return 0;
+
+        // Binary search of the value in the array
         uint256 min = 0;
-        uint256 max = _checkpoints.length - 1;
+        uint256 max = checkpoints.length - 1;
         while (max > min) {
-            uint256 mid = (max + min) / 2;
-            if (_checkpoints[mid].checkpointId == _checkpointId) {
-                max = mid;
-                break;
-            }
-            if (_checkpoints[mid].checkpointId < _checkpointId) {
-                min = mid + 1;
+            uint256 mid = (max + min + 1) / 2;
+            if (checkpoints[mid].timestamp <= timestamp) {
+                min = mid;
             } else {
-                max = mid;
+                max = mid - 1;
             }
         }
-        return _checkpoints[max].value;
+        return checkpoints[min].value;
     }
 
-    function createCheckpoint() public returns(uint256) {
-        // currentCheckpointId can only be incremented by 1 and hence it can not be overflowed
-        currentCheckpointId = currentCheckpointId + 1;
-        /*solium-disable-next-line security/no-block-members*/
-        checkpointTimes.push(now);
+    /**
+     * @notice Create a new checkpoint for a value if
+     * there does not exist a checkpoint for the current block timestamp,
+     * otherwise updates the value of the current checkpoint.
+     * @param checkpoints Checkpointed values
+     * @param value Value to be updated
+     */ 
+    function updateValueAtNow(
+        Checkpoint[] storage checkpoints,
+        uint value
+    )
+        internal
+    {
+        // create a new checkpoint if:
+        // - there are no checkpoints
+        // - the current block has a greater timestamp than the last checkpoint
+        // otherwise update value at current checkpoint
+        if (
+            checkpoints.length == 0
+            || (block.timestamp > checkpoints[checkpoints.length - 1].timestamp)
+        ) {
+            // create checkpoint with value
+            checkpoints.push(Checkpoint({ timestamp: uint128(block.timestamp), value: value }));
 
-        emit CheckpointCreated(currentCheckpointId);
-        return currentCheckpointId;
-    }  
+            emit CheckpointCreated(checkpoints.length - 1);
+
+        } else {
+            // update value at current checkpoint
+            Checkpoint storage oldCheckPoint = checkpoints[checkpoints.length - 1];
+            oldCheckPoint.value = value;
+        }
+    }
 }

packages/ap-contracts/contracts/token/ICT/Checkpoint/CheckpointStorage.sol

@@ -3,14 +3,14 @@ pragma solidity 0.6.4;
 
 contract CheckpointStorage {
 
+    /** 
+     * @dev `Checkpoint` is the structure that attaches a timestamp to a 
+     * given value, the timestamp attached is the one that last changed the value
+     */
     struct Checkpoint {
-        uint256 checkpointId;
+        // `timestamp` is the timestamp that the value was generated from
+        uint128 timestamp;
+        // `value` is the amount of tokens at a specific timestamp
         uint256 value;
     }
-
-    // Value of current checkpoint
-    uint256 public currentCheckpointId;
-
-    // Times at which each checkpoint was created
-    uint256[] checkpointTimes;
 }

packages/ap-contracts/contracts/token/ICT/CheckpointToken/CheckpointToken.sol → packages/ap-contracts/contracts/token/ICT/CheckpointedToken/CheckpointedToken.sol

@@ -1,13 +1,13 @@
 pragma solidity 0.6.4;
 pragma experimental ABIEncoderV2;
 
-import "@openzeppelin/contracts/token/ERC20/ERC20Mintable.sol";
-import "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
+import "openzeppelin-solidity/contracts/token/ERC20/ERC20.sol";
+import "openzeppelin-solidity/contracts/utils/ReentrancyGuard.sol";
 
 import "./CheckpointedTokenStorage.sol";
 
 
-contract CheckpointedToken is CheckpointedTokenStorage, ERC20Mintable, ReentrancyGuard {
+contract CheckpointedToken is CheckpointedTokenStorage, ERC20, ReentrancyGuard {
 
     /**
      * @notice returns an array of holders with non zero balance at a given checkpoint
@@ -80,31 +80,21 @@ contract CheckpointedToken is CheckpointedTokenStorage, ERC20Mintable, Reentranc
     }
 
     /**
-     * @notice Queries balances as of a defined checkpoint
+     * @notice Queries the balances of a holder at a specific timestamp
      * @param holder Holder to query balance for
-     * @param checkpointId Checkpoint ID to query as of
+     * @param timestamp Timestamp of the balance checkpoint
      */
-    function balanceOfAt(address holder, uint256 checkpointId) public view returns(uint256) {
-        require(checkpointId <= currentCheckpointId, "Invalid checkpoint");
-        return getValueAt(checkpointBalances[holder], checkpointId, balanceOf(holder));
+    function balanceOfAt(address holder, uint256 timestamp) public view returns(uint256) {
+        return getValueAt(checkpointBalances[holder], timestamp);
     }
 
     /**
-     * @notice Queries totalSupply as of a defined checkpoint
-     * @param checkpointId Checkpoint ID to query
+     * @notice Queries totalSupply at a specific timestamp
+     * @param timestamp Timestamp of the totalSupply checkpoint 
      * @return uint256
      */
-    function totalSupplyAt(uint256 checkpointId) public view returns(uint256) {
-        require(checkpointId <= currentCheckpointId, "Invalid checkpoint");
-        return checkpointTotalSupply[checkpointId];
-    }
-
-    function createTokenCheckpoint() public returns(uint256) {
-        createCheckpoint();
-
-        checkpointTotalSupply[currentCheckpointId] = totalSupply();
-
-        return currentCheckpointId;
+    function totalSupplyAt(uint256 timestamp) public view returns(uint256) {
+        return getValueAt(checkpointTotalSupply, timestamp);
     }
 
     function _isExistingHolder(address holder) internal view returns(bool) {
@@ -129,42 +119,30 @@ contract CheckpointedToken is CheckpointedTokenStorage, ERC20Mintable, Reentranc
         }
     }
 
+    /**
+     * @notice Internal - adjusts totalSupply at checkpoint before a token transfer
+     */
+    function _adjustTotalSupplyCheckpoints() internal {
+        updateValueAtNow(checkpointTotalSupply, totalSupply());
+    }
+
     /**
      * @notice Internal - adjusts token holder balance at checkpoint before a token transfer
      * @param holder address of the token holder affected
      */
     function _adjustBalanceCheckpoints(address holder) internal {
-        //No checkpoints set yet
-        if (currentCheckpointId == 0) {
-            return;
-        }
-        //No new checkpoints since last update
-        if (
-            (checkpointBalances[holder].length > 0) 
-            && (checkpointBalances[holder][checkpointBalances[holder].length - 1].checkpointId == currentCheckpointId)
-        ) {
-            return;
-        }
-        //New checkpoint, so record balance
-        checkpointBalances[holder].push(Checkpoint({checkpointId: currentCheckpointId, value: balanceOf(holder)}));
+        updateValueAtNow(checkpointBalances[holder], balanceOf(holder));
     }
 
     /**
      * @notice Updates internal variables when performing a transfer
      * @param from sender of transfer
      * @param to receiver of transfer
      * @param value value of transfer
-     * @return bool success
      */
-    function _updateTransfer(address from, address to, uint256 value) internal nonReentrant returns(bool verified) {
-        // NB - the ordering in this function implies the following:
-        //  - holder counts are updated before transfer managers are called - i.e. transfer managers will see
-        //holder counts including the current transfer.
-        //  - checkpoints are updated after the transfer managers are called. This allows TMs to create
-        //checkpoints as though they have been created before the current transactions,
-        //  - to avoid the situation where a transfer manager transfers tokens, and this function is called recursively,
-        //the function is marked as nonReentrant. This means that no TM can transfer (or mint / burn) tokens in the execute transfer function.
+    function _updateTransfer(address from, address to, uint256 value) internal {
         _adjustHolderCount(from, to, value);
+        _adjustTotalSupplyCheckpoints();
         _adjustBalanceCheckpoints(from);
         _adjustBalanceCheckpoints(to);
     }
@@ -174,6 +152,7 @@ contract CheckpointedToken is CheckpointedTokenStorage, ERC20Mintable, Reentranc
         uint256 value
     )
         internal
+        override
     {
         _updateTransfer(address(0), tokenHolder, value);
         super._mint(tokenHolder, value);
@@ -199,5 +178,4 @@ contract CheckpointedToken is CheckpointedTokenStorage, ERC20Mintable, Reentranc
         _updateTransfer(from, to, value);
         super._transfer(from, to, value);
     }
-
 }

packages/ap-contracts/contracts/token/ICT/CheckpointToken/CheckpointTokenStorage.sol → packages/ap-contracts/contracts/token/ICT/CheckpointedToken/CheckpointedTokenStorage.sol

@@ -6,8 +6,7 @@ import "../Checkpoint/Checkpoint.sol";
 
 contract CheckpointedTokenStorage is Checkpoint {
 
-    // Mapping of checkpoints that relate to total supply
-    mapping(uint256 => uint256) checkpointTotalSupply;
+    Checkpoint[] checkpointTotalSupply;
 
     // Map each holder to a series of checkpoints
     mapping(address => Checkpoint[]) checkpointBalances;