Split ERC20Votes and ERC20VotesComp (#2706)

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>

CHANGELOG.md

@@ -2,18 +2,19 @@
 
 ## Unreleased
 
- * `ERC20Votes`: add a new extension of the `ERC20` token with support for voting snapshots and delegation. This extension is compatible with Compound's `Comp` token interface. ([#2632](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2632))
+ * `ERC20Votes`: add a new extension of the `ERC20` token with support for voting snapshots and delegation. ([#2632](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2632))
+ * `ERC20VotesComp`: Variant of `ERC20Votes` that is compatible with Compound's `Comp` token interface but restricts supply to `uint96`. ([#2706](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2706))
  * Enumerables: Improve gas cost of removal in `EnumerableSet` and `EnumerableMap`.
  * Enumerables: Improve gas cost of lookup in `EnumerableSet` and `EnumerableMap`.
  * `Counter`: add a reset method. ([#2678](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2678))
- * Tokens: Wrap definitely safe subtractions in `unchecked` blocks. 
+ * Tokens: Wrap definitely safe subtractions in `unchecked` blocks.
  * `Math`: Add a `ceilDiv` method for performing ceiling division.
  * `ERC1155Supply`: add a new `ERC1155` extension that keeps track of the totalSupply of each tokenId. ([#2593](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2593))
 
  ### Breaking Changes
- 
+
  * `ERC20FlashMint` is no longer a Draft ERC. ([#2673](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2673)))
- 
+
 **How to update:** Change your import paths by removing the `draft-` prefix from `@openzeppelin/contracts/token/ERC20/extensions/draft-ERC20FlashMint.sol`.
 
 > See [Releases and Stability: Drafts](https://docs.openzeppelin.com/contracts/4.x/releases-stability#drafts).

contracts/mocks/ERC20VotesCompMock.sol

+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+
+import "../token/ERC20/extensions/ERC20VotesComp.sol";
+
+contract ERC20VotesCompMock is ERC20VotesComp {
+    constructor (string memory name, string memory symbol)
+    ERC20(name, symbol)
+    ERC20Permit(name)
+    {}
+
+    function mint(address account, uint256 amount) public {
+        _mint(account, amount);
+    }
+
+    function burn(address account, uint256 amount) public {
+        _burn(account, amount);
+    }
+
+    function getChainId() external view returns (uint256) {
+        return block.chainid;
+    }
+}

contracts/mocks/SafeCastMock.sol

@@ -12,14 +12,18 @@ contract SafeCastMock {
         return a.toUint256();
     }
 
-    function toInt256(uint a) public pure returns (int256) {
-        return a.toInt256();
+    function toUint224(uint a) public pure returns (uint224) {
+        return a.toUint224();
     }
 
     function toUint128(uint a) public pure returns (uint128) {
         return a.toUint128();
     }
 
+    function toUint96(uint a) public pure returns (uint96) {
+        return a.toUint96();
+    }
+
     function toUint64(uint a) public pure returns (uint64) {
         return a.toUint64();
     }
@@ -36,6 +40,10 @@ contract SafeCastMock {
         return a.toUint8();
     }
 
+    function toInt256(uint a) public pure returns (int256) {
+        return a.toInt256();
+    }
+
     function toInt128(int a) public pure returns (int128) {
         return a.toInt128();
     }

contracts/token/ERC20/README.adoc

@@ -21,7 +21,8 @@ Additionally there are multiple custom extensions, including:
 * {ERC20Snapshot}: efficient storage of past token balances to be later queried at any point in time.
 * {ERC20Permit}: gasless approval of tokens (standardized as ERC2612).
 * {ERC20FlashMint}: token level support for flash loans through the minting and burning of ephemeral tokens (standardized as ERC3156).
-* {ERC20Votes}: support for voting and vote delegation (compatible with Compound's token).
+* {ERC20Votes}: support for voting and vote delegation.
+* {ERC20VotesComp}: support for voting and vote delegation (compatible with Compound's tokenn, with uint96 restrictions).
 
 Finally, there are some utilities to interact with ERC20 contracts in various ways.
 
@@ -32,7 +33,6 @@ The following related EIPs are in draft status.
 
 - {ERC20Permit}
 - {ERC20FlashMint}
-- {ERC20Votes}
 
 NOTE: This core set of contracts is designed to be unopinionated, allowing developers to access the internal functions in ERC20 (such as <<ERC20-_mint-address-uint256-,`_mint`>>) and expose them as external functions in the way they prefer. On the other hand, xref:ROOT:erc20.adoc#Presets[ERC20 Presets] (such as {ERC20PresetMinterPauser}) are designed using opinionated patterns to provide developers with ready to use, deployable contracts.
 
@@ -54,6 +54,10 @@ NOTE: This core set of contracts is designed to be unopinionated, allowing devel
 
 {{ERC20Snapshot}}
 
+{{ERC20Votes}}
+
+{{ERC20VotesComp}}
+
 == Draft EIPs
 
 The following EIPs are still in Draft status. Due to their nature as drafts, the details of these contracts may change and we cannot guarantee their xref:ROOT:releases-stability.adoc[stability]. Minor releases of OpenZeppelin Contracts may contain breaking changes for the contracts in this directory, which will be duly announced in the https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/CHANGELOG.md[changelog]. The EIPs included here are used by projects in production and this may make them less likely to change significantly.
@@ -62,8 +66,6 @@ The following EIPs are still in Draft status. Due to their nature as drafts, the
 
 {{ERC20FlashMint}}
 
-{{ERC20Votes}}
-
 == Presets
 
 These contracts are preconfigured combinations of the above features. They can be used through inheritance or as models to copy and paste their source code.

contracts/token/ERC20/extensions/ERC20Votes.sol

@@ -3,17 +3,19 @@
 pragma solidity ^0.8.0;
 
 import "./draft-ERC20Permit.sol";
-import "./IERC20Votes.sol";
 import "../../../utils/math/Math.sol";
 import "../../../utils/math/SafeCast.sol";
 import "../../../utils/cryptography/ECDSA.sol";
 
 /**
- * @dev Extension of the ERC20 token contract to support Compound's voting and delegation.
+ * @dev Extension of ERC20 to support Compound-like voting and delegation. This version is more generic than Compound's,
+ * and supports token supply up to 2^224^ - 1, while COMP is limited to 2^96^ - 1.
  *
- * This extensions keeps a history (checkpoints) of each account's vote power. Vote power can be delegated either
+ * NOTE: If exact COMP compatibility is required, use the {ERC20VotesComp} variant of this module.
+ *
+ * This extension 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 {getCurrentVotes} and {getPriorVotes}.
+ * 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.
@@ -22,7 +24,12 @@ import "../../../utils/cryptography/ECDSA.sol";
  *
  * _Available since v4.2._
  */
-abstract contract ERC20Votes is IERC20Votes, ERC20Permit {
+abstract contract ERC20Votes is ERC20Permit {
+    struct Checkpoint {
+        uint32  fromBlock;
+        uint224 votes;
+    }
+
     bytes32 private constant _DELEGATION_TYPEHASH = keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)");
 
     mapping (address => address) private _delegates;
@@ -30,30 +37,40 @@ abstract contract ERC20Votes is IERC20Votes, ERC20Permit {
     Checkpoint[] private _totalSupplyCheckpoints;
 
     /**
+     * @dev 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.
+     */
+    event DelegateVotesChanged(address indexed delegate, uint256 previousBalance, uint256 newBalance);
+
+    /**
      * @dev Get the `pos`-th checkpoint for `account`.
      */
-    function checkpoints(address account, uint32 pos) external view virtual override returns (Checkpoint memory) {
+    function checkpoints(address account, uint32 pos) public view virtual returns (Checkpoint memory) {
         return _checkpoints[account][pos];
     }
 
     /**
      * @dev Get number of checkpoints for `account`.
      */
-    function numCheckpoints(address account) external view virtual override returns (uint32) {
+    function numCheckpoints(address account) public view virtual returns (uint32) {
         return SafeCast.toUint32(_checkpoints[account].length);
     }
 
     /**
      * @dev Get the address `account` is currently delegating to.
      */
-    function delegates(address account) public view virtual override returns (address) {
+    function delegates(address account) public view virtual returns (address) {
         return _delegates[account];
     }
 
     /**
      * @dev Gets the current votes balance for `account`
      */
-    function getCurrentVotes(address account) external view override returns (uint256) {
+    function getVotes(address account) public view returns (uint256) {
         uint256 pos = _checkpoints[account].length;
         return pos == 0 ? 0 : _checkpoints[account][pos - 1].votes;
     }
@@ -65,7 +82,7 @@ abstract contract ERC20Votes is IERC20Votes, ERC20Permit {
      *
      * - `blockNumber` must have been already mined
      */
-    function getPriorVotes(address account, uint256 blockNumber) external view override returns (uint256) {
+    function getPastVotes(address account, uint256 blockNumber) public view returns (uint256) {
         require(blockNumber < block.number, "ERC20Votes: block not yet mined");
         return _checkpointsLookup(_checkpoints[account], blockNumber);
     }
@@ -78,7 +95,7 @@ abstract contract ERC20Votes is IERC20Votes, ERC20Permit {
      *
      * - `blockNumber` must have been already mined
      */
-    function getPriorTotalSupply(uint256 blockNumber) external view override returns (uint256) {
+    function getPastTotalSupply(uint256 blockNumber) public view returns (uint256) {
         require(blockNumber < block.number, "ERC20Votes: block not yet mined");
         return _checkpointsLookup(_totalSupplyCheckpoints, blockNumber);
     }
@@ -115,7 +132,7 @@ abstract contract ERC20Votes is IERC20Votes, ERC20Permit {
     /**
      * @dev Delegate votes from the sender to `delegatee`.
      */
-    function delegate(address delegatee) public virtual override {
+    function delegate(address delegatee) public virtual {
         return _delegate(_msgSender(), delegatee);
     }
 
@@ -123,7 +140,7 @@ abstract contract ERC20Votes is IERC20Votes, ERC20Permit {
      * @dev Delegates votes from signer to `delegatee`
      */
     function delegateBySig(address delegatee, uint256 nonce, uint256 expiry, uint8 v, bytes32 r, bytes32 s)
-        public virtual override
+        public virtual
     {
         require(block.timestamp <= expiry, "ERC20Votes: signature expired");
         address signer = ECDSA.recover(
@@ -140,17 +157,24 @@ abstract contract ERC20Votes is IERC20Votes, ERC20Permit {
     }
 
     /**
-     * @dev snapshot the totalSupply after it has been increassed.
+     * @dev Maximum token supply. Defaults to `type(uint224).max` (2^224^ - 1).
+     */
+    function _maxSupply() internal view virtual returns (uint224) {
+        return type(uint224).max;
+    }
+
+    /**
+     * @dev Snapshots the totalSupply after it has been increased.
      */
     function _mint(address account, uint256 amount) internal virtual override {
         super._mint(account, amount);
-        require(totalSupply() <= type(uint224).max, "ERC20Votes: total supply exceeds 2**224");
+        require(totalSupply() <= _maxSupply(), "ERC20Votes: total supply risks overflowing votes");
 
         _writeCheckpoint(_totalSupplyCheckpoints, add, amount);
     }
 
     /**
-     * @dev snapshot the totalSupply after it has been decreased.
+     * @dev Snapshots the totalSupply after it has been decreased.
      */
     function _burn(address account, uint256 amount) internal virtual override {
         super._burn(account, amount);
@@ -159,7 +183,9 @@ abstract contract ERC20Votes is IERC20Votes, ERC20Permit {
     }
 
     /**
-     * @dev move voting power when tokens are transferred.
+     * @dev Move voting power when tokens are transferred.
+     *
+     * Emits a {DelegateVotesChanged} event.
      */
     function _beforeTokenTransfer(address from, address to, uint256 amount) internal virtual override {
         _moveVotingPower(delegates(from), delegates(to), amount);
@@ -167,6 +193,8 @@ abstract contract ERC20Votes is IERC20Votes, ERC20Permit {
 
     /**
      * @dev Change delegation for `delegator` to `delegatee`.
+     *
+     * Emits events {DelegateChanged} and {DelegateVotesChanged}.
      */
     function _delegate(address delegator, address delegatee) internal virtual {
         address currentDelegate = delegates(delegator);

contracts/token/ERC20/extensions/ERC20VotesComp.sol

+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "./ERC20Votes.sol";
+
+/**
+ * @dev Extension of ERC20 to support Compound's voting and delegation. This version exactly matches Compound's
+ * interface, with the drawback of only supporting supply up to (2^96^ - 1).
+ *
+ * NOTE: You should use this contract if you need exact compatibility with COMP (for example in order to use your token
+ * with Governor Alpha or Bravo) and if you are sure the supply cap of 2^96^ is enough for you. Otherwise, use the
+ * {ERC20Votes} variant of this module.
+ *
+ * This extensions 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 {getCurrentVotes} and {getPriorVotes}.
+ *
+ * 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.
+ *
+ * _Available since v4.2._
+ */
+abstract contract ERC20VotesComp is ERC20Votes {
+    /**
+     * @dev Comp version of the {getVotes} accessor, with `uint96` return type.
+     */
+    function getCurrentVotes(address account) external view returns (uint96) {
+        return SafeCast.toUint96(getVotes(account));
+    }
+    /**
+     * @dev Comp version of the {getPastVotes} accessor, with `uint96` return type.
+     */
+    function getPriorVotes(address account, uint256 blockNumber) external view returns (uint96) {
+        return SafeCast.toUint96(getPastVotes(account, blockNumber));
+    }
+
+    /**
+     * @dev Maximum token supply. Reduced to `type(uint96).max` (2^96^ - 1) to fit COMP interface.
+     */
+    function _maxSupply() internal view virtual override returns (uint224) {
+        return type(uint96).max;
+    }
+}

contracts/token/ERC20/extensions/IERC20Votes.sol

-// SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.0;
-
-import "../IERC20.sol";
-
-interface IERC20Votes is IERC20 {
-    struct Checkpoint {
-        uint32  fromBlock;
-        uint224 votes;
-    }
-
-    event DelegateChanged(address indexed delegator, address indexed fromDelegate, address indexed toDelegate);
-    event DelegateVotesChanged(address indexed delegate, uint256 previousBalance, uint256 newBalance);
-
-    function delegates(address owner) external view returns (address);
-    function checkpoints(address account, uint32 pos) external view returns (Checkpoint memory);
-    function numCheckpoints(address account) external view returns (uint32);
-    function getCurrentVotes(address account) external view returns (uint256);
-    function getPriorVotes(address account, uint256 blockNumber) external view returns (uint256);
-    function getPriorTotalSupply(uint256 blockNumber) external view returns(uint256);
-    function delegate(address delegatee) external;
-    function delegateBySig(address delegatee, uint nonce, uint expiry, uint8 v, bytes32 r, bytes32 s) external;
-}

contracts/utils/math/SafeCast.sol

@@ -29,7 +29,7 @@ library SafeCast {
      * - input must fit into 224 bits
      */
     function toUint224(uint256 value) internal pure returns (uint224) {
-        require(value < 2**224, "SafeCast: value doesn\'t fit in 224 bits");
+        require(value <= type(uint224).max, "SafeCast: value doesn\'t fit in 224 bits");
         return uint224(value);
     }
 
@@ -44,11 +44,26 @@ library SafeCast {
      * - input must fit into 128 bits
      */
     function toUint128(uint256 value) internal pure returns (uint128) {
-        require(value < 2**128, "SafeCast: value doesn\'t fit in 128 bits");
+        require(value <= type(uint128).max, "SafeCast: value doesn\'t fit in 128 bits");
         return uint128(value);
     }
 
     /**
+     * @dev Returns the downcasted uint96 from uint256, reverting on
+     * overflow (when the input is greater than largest uint96).
+     *
+     * Counterpart to Solidity's `uint96` operator.
+     *
+     * Requirements:
+     *
+     * - input must fit into 96 bits
+     */
+    function toUint96(uint256 value) internal pure returns (uint96) {
+        require(value <= type(uint96).max, "SafeCast: value doesn\'t fit in 96 bits");
+        return uint96(value);
+    }
+
+    /**
      * @dev Returns the downcasted uint64 from uint256, reverting on
      * overflow (when the input is greater than largest uint64).
      *
@@ -59,7 +74,7 @@ library SafeCast {
      * - input must fit into 64 bits
      */
     function toUint64(uint256 value) internal pure returns (uint64) {
-        require(value < 2**64, "SafeCast: value doesn\'t fit in 64 bits");
+        require(value <= type(uint64).max, "SafeCast: value doesn\'t fit in 64 bits");
         return uint64(value);
     }
 
@@ -74,7 +89,7 @@ library SafeCast {
      * - input must fit into 32 bits
      */
     function toUint32(uint256 value) internal pure returns (uint32) {
-        require(value < 2**32, "SafeCast: value doesn\'t fit in 32 bits");
+        require(value <= type(uint32).max, "SafeCast: value doesn\'t fit in 32 bits");
         return uint32(value);
     }
 
@@ -89,7 +104,7 @@ library SafeCast {
      * - input must fit into 16 bits
      */
     function toUint16(uint256 value) internal pure returns (uint16) {
-        require(value < 2**16, "SafeCast: value doesn\'t fit in 16 bits");
+        require(value <= type(uint16).max, "SafeCast: value doesn\'t fit in 16 bits");
         return uint16(value);
     }
 
@@ -104,7 +119,7 @@ library SafeCast {
      * - input must fit into 8 bits.
      */
     function toUint8(uint256 value) internal pure returns (uint8) {
-        require(value < 2**8, "SafeCast: value doesn\'t fit in 8 bits");
+        require(value <= type(uint8).max, "SafeCast: value doesn\'t fit in 8 bits");
         return uint8(value);
     }
 
@@ -134,7 +149,7 @@ library SafeCast {
      * _Available since v3.1._
      */
     function toInt128(int256 value) internal pure returns (int128) {
-        require(value >= -2**127 && value < 2**127, "SafeCast: value doesn\'t fit in 128 bits");
+        require(value >= type(int128).min && value <= type(int128).max, "SafeCast: value doesn\'t fit in 128 bits");
         return int128(value);
     }
 
@@ -152,7 +167,7 @@ library SafeCast {
      * _Available since v3.1._
      */
     function toInt64(int256 value) internal pure returns (int64) {
-        require(value >= -2**63 && value < 2**63, "SafeCast: value doesn\'t fit in 64 bits");
+        require(value >= type(int64).min && value <= type(int64).max, "SafeCast: value doesn\'t fit in 64 bits");
         return int64(value);
     }
 
@@ -170,7 +185,7 @@ library SafeCast {
      * _Available since v3.1._
      */
     function toInt32(int256 value) internal pure returns (int32) {
-        require(value >= -2**31 && value < 2**31, "SafeCast: value doesn\'t fit in 32 bits");
+        require(value >= type(int32).min && value <= type(int32).max, "SafeCast: value doesn\'t fit in 32 bits");
         return int32(value);
     }
 
@@ -188,7 +203,7 @@ library SafeCast {
      * _Available since v3.1._
      */
     function toInt16(int256 value) internal pure returns (int16) {
-        require(value >= -2**15 && value < 2**15, "SafeCast: value doesn\'t fit in 16 bits");
+        require(value >= type(int16).min && value <= type(int16).max, "SafeCast: value doesn\'t fit in 16 bits");
         return int16(value);
     }
 
@@ -206,7 +221,7 @@ library SafeCast {
      * _Available since v3.1._
      */
     function toInt8(int256 value) internal pure returns (int8) {
-        require(value >= -2**7 && value < 2**7, "SafeCast: value doesn\'t fit in 8 bits");
+        require(value >= type(int8).min && value <= type(int8).max, "SafeCast: value doesn\'t fit in 8 bits");
         return int8(value);
     }
 
@@ -218,7 +233,8 @@ library SafeCast {
      * - input must be less than or equal to maxInt256.
      */
     function toInt256(uint256 value) internal pure returns (int256) {
-        require(value < 2**255, "SafeCast: value doesn't fit in an int256");
+        // Note: Unsafe cast below is okay because `type(int256).max` is guaranteed to be positive
+        require(value <= uint256(type(int256).max), "SafeCast: value doesn't fit in an int256");
         return int256(value);
     }
 }

test/utils/math/SafeCast.test.js

@@ -41,7 +41,7 @@ contract('SafeCast', async (accounts) => {
     });
   }
 
-  [8, 16, 32, 64, 128].forEach(bits => testToUint(bits));
+  [8, 16, 32, 64, 96, 128, 224].forEach(bits => testToUint(bits));
 
   describe('toUint256', () => {
     const maxInt256 = new BN('2').pow(new BN(255)).subn(1);