Adhere to our code style conventions

cygnusv · cygnusv · commit 09299237c844 · 2021-10-07T12:29:29.000+02:00
diff --git a/contracts/token/T.sol b/contracts/token/T.sol
@@ -9,6 +9,10 @@ import "@thesis/solidity-contracts/contracts/token/MisfundRecovery.sol";
 
 /// @title T token
 /// @notice Threshold Network T token.
+/// @dev By default, token balance does not account for voting power.
+///      This makes transfers cheaper. The downside is that it requires users
+///      to delegate to themselves to activate checkpoints and have their
+///      voting power tracked.
 contract T is ERC20WithPermit, MisfundRecovery, Checkpoints {
     /// @notice The EIP-712 typehash for the delegation struct used by
     ///         `delegateBySig`.
@@ -89,18 +93,18 @@ contract T is ERC20WithPermit, MisfundRecovery, Checkpoints {
             // Does not allow to mint more than uint96 can fit. Otherwise, the
             // Checkpoint might not fit the balance.
             require(
-                totalSupply + amount <= _maxSupply(),
+                totalSupply + amount <= maxSupply(),
                 "Maximum total supply exceeded"
             );
-            _writeCheckpoint(_totalSupplyCheckpoints, _add, safeAmount);
+            writeCheckpoint(_totalSupplyCheckpoints, add, safeAmount);
         }
 
         // When burning:
         if (to == address(0)) {
-            _writeCheckpoint(_totalSupplyCheckpoints, _subtract, safeAmount);
+            writeCheckpoint(_totalSupplyCheckpoints, subtract, safeAmount);
         }
 
-        _moveVotingPower(delegates(from), delegates(to), safeAmount);
+        moveVotingPower(delegates(from), delegates(to), safeAmount);
     }
 
     function delegate(address delegator, address delegatee)
@@ -114,6 +118,6 @@ contract T is ERC20WithPermit, MisfundRecovery, Checkpoints {
 
         emit DelegateChanged(delegator, currentDelegate, delegatee);
 
-        _moveVotingPower(currentDelegate, delegatee, delegatorBalance);
+        moveVotingPower(currentDelegate, delegatee, delegatorBalance);
     }
 }
diff --git a/contracts/utils/Checkpoints.sol b/contracts/utils/Checkpoints.sol
@@ -6,21 +6,15 @@ import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 import "@openzeppelin/contracts/utils/math/Math.sol";
 import "@openzeppelin/contracts/utils/math/SafeCast.sol";
 
-/**
- * @dev Abstract contract to support checkpoints for Compound-like voting and delegation.
- * This implementation supports token supply up to 2^96^ - 1.
- *
- * This contract keeps a history (checkpoints) of each account's vote power. Vote power can be delegated either
- * by calling the {delegate} function directly, or by providing a signature to be used with {delegateBySig}. Voting
- * power can be queried through the public accessors {getVotes} and {getPastVotes}.
- *
- * By default, token balance does not account for voting power. This makes transfers cheaper. The downside is that it
- * requires users to delegate to themselves in order to activate checkpoints and have their voting power tracked.
- * Enabling self-delegation can easily be done by overriding the {delegates} function. Keep in mind however that this
- * will significantly increase the base gas cost of transfers.
- *
- * NOTE: Extracted from OpenZeppelin ERCVotes.sol. Maybe worth trying to push it upstream there.
- */
+/// @title Checkpoints
+/// @dev Abstract contract to support checkpoints for Compound-like voting and
+///      delegation. This implementation supports token supply up to 2^96 - 1.
+///      This contract keeps a history (checkpoints) of each account's vote
+///      power. Vote power can be delegated either by calling the {delegate}
+///      function directly, or by providing a signature to be used with
+///      {delegateBySig}. Voting power can be publicly queried through
+///      {getVotes} and {getPastVotes}.
+///      NOTE: Extracted from OpenZeppelin ERCVotes.sol.
 abstract contract Checkpoints {
     struct Checkpoint {
         uint32 fromBlock;
@@ -31,18 +25,15 @@ abstract contract Checkpoints {
     mapping(address => uint128[]) internal _checkpoints;
     uint128[] internal _totalSupplyCheckpoints;
 
-    /**
-     * @dev Emitted when an account changes their delegate.
-     */
+    /// @notice Emitted when an account changes their delegate.
     event DelegateChanged(
         address indexed delegator,
         address indexed fromDelegate,
         address indexed toDelegate
     );
 
-    /**
-     * @dev Emitted when a token transfer or delegate change results in changes to an account's voting power.
-     */
+    /// @notice Emitted when a balance or delegate change results in changes
+    ///         to an account's voting power.
     event DelegateVotesChanged(
         address indexed delegate,
         uint256 previousBalance,
@@ -51,9 +42,6 @@ abstract contract Checkpoints {
 
     function delegate(address delegator, address delegatee) internal virtual;
 
-    /**
-     * @dev Get the `pos`-th checkpoint for `account`.
-     */
     function checkpoints(address account, uint32 pos)
         public
         view
@@ -66,9 +54,7 @@ abstract contract Checkpoints {
         checkpoint = Checkpoint(fromBlock, votes);
     }
 
-    /**
-     * @dev Get number of checkpoints for `account`.
-     */
+    /// @notice Get number of checkpoints for `account`.
     function numCheckpoints(address account)
         public
         view
@@ -78,9 +64,7 @@ abstract contract Checkpoints {
         return SafeCast.toUint32(_checkpoints[account].length);
     }
 
-    /**
-     * @dev Get the address `account` is currently delegating to.
-     */
+    /// @notice Get the address `account` is currently delegating to.
     function delegates(address account) public view virtual returns (address) {
         return _delegates[account];
     }
@@ -105,25 +89,26 @@ abstract contract Checkpoints {
         view
         returns (uint96)
     {
-        return _checkpointsLookup(_checkpoints[account], blockNumber);
+        return lookupCheckpoint(_checkpoints[account], blockNumber);
     }
 
-    /**
-     * @dev Retrieve the `totalSupply` at the end of `blockNumber`. Note, this value is the sum of all balances.
-     * It is but NOT the sum of all the delegated votes!
-     *
-     * Requirements:
-     *
-     * - `blockNumber` must have been already mined
-     */
+    /// @notice Retrieve the `totalSupply` at the end of `blockNumber`.
+    ///         Note, this value is the sum of all balances, but it is NOT the
+    ///         sum of all the delegated votes!
+    /// @param blockNumber The block number to get the total supply at
+    /// @dev `blockNumber` must have been already mined
     function getPastTotalSupply(uint256 blockNumber)
         public
         view
         returns (uint96)
     {
-        return _checkpointsLookup(_totalSupplyCheckpoints, blockNumber);
+        return lookupCheckpoint(_totalSupplyCheckpoints, blockNumber);
     }
 
+    /// @notice Encodes a `blockNumber` and `value` into a single `uint128`
+    ///         checkpoint.
+    /// @dev `blockNumber` is stored in the first 32 bits, while `value` in the
+    ///      remaining 96 bits.
     function encodeCheckpoint(uint32 blockNumber, uint96 value)
         internal
         pure
@@ -132,46 +117,55 @@ abstract contract Checkpoints {
         return (uint128(blockNumber) << 96) | uint128(value);
     }
 
-    function decodeBlockNumber(uint128 _checkpoint)
+    /// @notice Decodes a block number from a `uint128` `checkpoint`.
+    function decodeBlockNumber(uint128 checkpoint)
         internal
         pure
         returns (uint32)
     {
-        return uint32(bytes4(bytes16(_checkpoint)));
+        return uint32(bytes4(bytes16(checkpoint)));
     }
 
-    function decodeValue(uint128 _checkpoint) internal pure returns (uint96) {
-        return uint96(_checkpoint);
+    /// @notice Decodes a voting value from a `uint128` `checkpoint`.
+    function decodeValue(uint128 checkpoint) internal pure returns (uint96) {
+        return uint96(checkpoint);
     }
 
-    function decodeCheckpoint(uint128 _checkpoint)
+    /// @notice Decodes a block number and voting value from a `uint128`
+    ///         `checkpoint`.
+    function decodeCheckpoint(uint128 checkpoint)
         internal
         pure
         returns (uint32 blockNumber, uint96 value)
     {
-        blockNumber = decodeBlockNumber(_checkpoint);
-        value = decodeValue(_checkpoint);
+        blockNumber = decodeBlockNumber(checkpoint);
+        value = decodeValue(checkpoint);
     }
 
-    /**
-     * @dev Lookup a value in a list of (sorted) checkpoints.
-     */
-    function _checkpointsLookup(uint128[] storage ckpts, uint256 blockNumber)
+    /// @notice Lookup a value in a list of (sorted) checkpoints.
+    /// @param ckpts The checkpoints array to use
+    /// @param blockNumber Block number when we want to get the checkpoint at
+    function lookupCheckpoint(uint128[] storage ckpts, uint256 blockNumber)
         internal
         view
         returns (uint96)
     {
-        // We run a binary search to look for the earliest checkpoint taken after `blockNumber`.
-        //
-        // During the loop, the index of the wanted checkpoint remains in the range [low-1, high).
-        // With each iteration, either `low` or `high` is moved towards the middle of the range to maintain the invariant.
-        // - If the middle checkpoint is after `blockNumber`, we look in [low, mid)
-        // - If the middle checkpoint is before or equal to `blockNumber`, we look in [mid+1, high)
-        // Once we reach a single value (when low == high), we've found the right checkpoint at the index high-1, if not
-        // out of bounds (in which case we're looking too far in the past and the result is 0).
-        // Note that if the latest checkpoint available is exactly for `blockNumber`, we end up with an index that is
-        // past the end of the array, so we technically don't find a checkpoint after `blockNumber`, but it works out
-        // the same.
+        // We run a binary search to look for the earliest checkpoint taken
+        // after `blockNumber`. During the loop, the index of the wanted
+        // checkpoint remains in the range [low-1, high). With each iteration,
+        // either `low` or `high` is moved towards the middle of the range to
+        // maintain the invariant.
+        // - If the middle checkpoint is after `blockNumber`,
+        //   we look in [low, mid)
+        // - If the middle checkpoint is before or equal to `blockNumber`,
+        //   we look in [mid+1, high)
+        // Once we reach a single value (when low == high), we've found the
+        // right checkpoint at the index high-1, if not out of bounds (in that
+        // case we're looking too far in the past and the result is 0).
+        // Note that if the latest checkpoint available is exactly for
+        // `blockNumber`, we end up with an index that is past the end of the
+        // array, so we technically don't find a checkpoint after
+        // `blockNumber`, but it works out the same.
         require(blockNumber < block.number, "Block not yet determined");
 
         uint256 high = ckpts.length;
@@ -189,40 +183,50 @@ abstract contract Checkpoints {
         return high == 0 ? 0 : decodeValue(ckpts[high - 1]);
     }
 
-    /**
-     * @dev Maximum token supply. Defaults to `type(uint96).max` (2^96^ - 1).
-     */
-    function _maxSupply() internal view virtual returns (uint96) {
+    /// @notice Maximum token supply. Defaults to `type(uint96).max` (2^96 - 1)
+    function maxSupply() internal view virtual returns (uint96) {
         return type(uint96).max;
     }
 
-    function _moveVotingPower(
+    /// @notice Moves voting power from one delegate to another
+    /// @param src Address of old delegate
+    /// @param dst Address of new delegate
+    /// @param amount Voting power amount to transfer between delegates
+    function moveVotingPower(
         address src,
         address dst,
         uint256 amount
     ) internal {
         if (src != dst && amount > 0) {
             if (src != address(0)) {
-                (uint256 oldWeight, uint256 newWeight) = _writeCheckpoint(
+                (uint256 oldWeight, uint256 newWeight) = writeCheckpoint(
                     _checkpoints[src],
-                    _subtract,
+                    subtract,
                     amount
                 );
                 emit DelegateVotesChanged(src, oldWeight, newWeight);
             }
 
             if (dst != address(0)) {
-                (uint256 oldWeight, uint256 newWeight) = _writeCheckpoint(
+                (uint256 oldWeight, uint256 newWeight) = writeCheckpoint(
                     _checkpoints[dst],
-                    _add,
+                    add,
                     amount
                 );
                 emit DelegateVotesChanged(dst, oldWeight, newWeight);
             }
         }
     }
 
-    function _writeCheckpoint(
+    /// @notice Writes a new checkpoint based on operating last stored value
+    ///         with a `delta`. Usually, said operation is the `add` or
+    ///         `subtract` functions from this contract, but more complex
+    ///         functions can be passed as parameters.
+    /// @param ckpts The checkpoints array to use
+    /// @param op The function to apply over the last value and the `delta`
+    /// @param delta Variation with respect to last stored value to be used
+    ///              for new checkpoint
+    function writeCheckpoint(
         uint128[] storage ckpts,
         function(uint256, uint256) view returns (uint256) op,
         uint256 delta
@@ -251,12 +255,12 @@ abstract contract Checkpoints {
     }
 
     // slither-disable-next-line dead-code
-    function _add(uint256 a, uint256 b) internal pure returns (uint256) {
+    function add(uint256 a, uint256 b) internal pure returns (uint256) {
         return a + b;
     }
 
     // slither-disable-next-line dead-code
-    function _subtract(uint256 a, uint256 b) internal pure returns (uint256) {
+    function subtract(uint256 a, uint256 b) internal pure returns (uint256) {
         return a - b;
     }
 }
