Make FlexVotingClient clock agnostic (#77)

Contracts can track time in at least two different ways: block numbers or timestamps. [EIP6372](https://eips.ethereum.org/EIPS/eip-6372) introduced a standard specification for contract clocks, giving contracts a canonical way to check how other contracts are keeping time. Since time is fundamental to voting (viz voting periods), it was important to make sure that the `FlexVotingClient` would still work no matter what clock a governor was using. This PR makes `FlexVotingClient` clock agnostic, meaning that it is compatible with any clock system that conforms to EIP6372. Additionally, `FlexVotingClient` now exposes a `_checkpointTotalBalance` function to simplify total balance accounting in child contracts. The new function is analogous to the `_checkpointRawBalanceOf` function already in place for individual account balance checkpointing.

Author: davidlaprade

src/FlexVotingClient.sol

@@ -3,8 +3,8 @@ pragma solidity ^0.8.20;
 
 import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
 import {Checkpoints} from "@openzeppelin/contracts/utils/structs/Checkpoints.sol";
-import {IFractionalGovernor} from "./interfaces/IFractionalGovernor.sol";
-import {IVotingToken} from "./interfaces/IVotingToken.sol";
+import {IFractionalGovernor} from "src/interfaces/IFractionalGovernor.sol";
+import {IVotingToken} from "src/interfaces/IVotingToken.sol";
 
 /// @notice This is an abstract contract designed to make it easy to build clients
 /// for governance systems that inherit from GovernorCountingFractional, a.k.a.
@@ -51,8 +51,8 @@ abstract contract FlexVotingClient {
 
   // @dev Trace208 is used instead of Trace224 because the former allocates 48
   // bits to its _key. We need at least 48 bits because the _key is going to be
-  // a block number. And EIP-6372 specifies that when block numbers are used for
-  // internal clocks (as they are for ERC20Votes) they need to be uint48s.
+  // a timepoint. Timepoints in the context of ERC20Votes and ERC721Votes
+  // conform to the EIP-6372 standard, which specifies they be uint48s.
   using Checkpoints for Checkpoints.Trace208;
 
   /// @notice The voting options corresponding to those used in the Governor.
@@ -149,7 +149,7 @@ abstract contract FlexVotingClient {
       revert FlexVotingClient__NoVotesExpressed();
     }
 
-    uint256 _proposalSnapshotBlockNumber = GOVERNOR.proposalSnapshot(proposalId);
+    uint256 _proposalSnapshot = GOVERNOR.proposalSnapshot(proposalId);
 
     // We use the snapshot of total raw balances to determine the weight with
     // which to vote. We do this for two reasons:
@@ -167,12 +167,11 @@ abstract contract FlexVotingClient {
     // Using the total raw balance to proportion votes in this way means that in
     // many circumstances this function will not cast votes with all of its
     // weight.
-    uint256 _totalRawBalanceAtSnapshot = getPastTotalBalance(_proposalSnapshotBlockNumber);
+    uint256 _totalRawBalanceAtSnapshot = getPastTotalBalance(_proposalSnapshot);
 
     // We need 256 bits because of the multiplication we're about to do.
-    uint256 _votingWeightAtSnapshot = IVotingToken(address(GOVERNOR.token())).getPastVotes(
-      address(this), _proposalSnapshotBlockNumber
-    );
+    uint256 _votingWeightAtSnapshot =
+      IVotingToken(address(GOVERNOR.token())).getPastVotes(address(this), _proposalSnapshot);
 
     //      forVotesRaw          forVoteWeight
     // --------------------- = ------------------
@@ -205,21 +204,52 @@ abstract contract FlexVotingClient {
 
   /// @dev Checkpoints the _user's current raw balance.
   function _checkpointRawBalanceOf(address _user) internal {
-    balanceCheckpoints[_user].push(SafeCast.toUint48(block.number), _rawBalanceOf(_user));
+    balanceCheckpoints[_user].push(IVotingToken(GOVERNOR.token()).clock(), _rawBalanceOf(_user));
+  }
+
+  /// @dev Checkpoints the total balance after applying `_delta`.
+  function _checkpointTotalBalance(int256 _delta) internal {
+    // The casting in this function is safe since:
+    // - if oldTotal + delta > int256.max it will panic and revert.
+    // - if |delta| <= oldTotal
+    //   * there is no risk of wrapping
+    // - if |delta| > oldTotal
+    //   * uint256(oldTotal + delta) will wrap but the wrapped value will
+    //     necessarily be greater than uint208.max, so SafeCast will revert.
+    //   * the lowest that oldTotal + delta can be is int256.min (when
+    //     oldTotal is 0 and delta is int256.min). The wrapped value of a
+    //     negative signed integer is:
+    //       wrapped(integer) = uint256.max + integer
+    //     Substituting:
+    //       wrapped(int256.min) = uint256.max + int256.min
+    //     But:
+    //       uint256.max + int256.min > uint208.max
+    //     Substituting again:
+    //       wrapped(int256.min) > uint208.max, which will revert when safecast.
+    uint256 _oldTotal = uint256(totalBalanceCheckpoints.latest());
+    uint256 _newTotal = uint256(int256(_oldTotal) + _delta);
+
+    totalBalanceCheckpoints.push(
+      IVotingToken(GOVERNOR.token()).clock(), SafeCast.toUint208(_newTotal)
+    );
   }
 
-  /// @notice Returns the `_user`'s raw balance at `_blockNumber`.
+  /// @notice Returns the `_user`'s raw balance at `_timepoint`.
   /// @param _user The account that's historical raw balance will be looked up.
-  /// @param _blockNumber The block at which to lookup the _user's raw balance.
-  function getPastRawBalance(address _user, uint256 _blockNumber) public view returns (uint256) {
-    uint48 key = SafeCast.toUint48(_blockNumber);
+  /// @param _timepoint The timepoint at which to lookup the _user's raw
+  /// balance, either a block number or a timestamp as determined by
+  /// {GOVERNOR.token().clock()}.
+  function getPastRawBalance(address _user, uint256 _timepoint) public view returns (uint256) {
+    uint48 key = SafeCast.toUint48(_timepoint);
     return balanceCheckpoints[_user].upperLookup(key);
   }
 
-  /// @notice Returns the sum total of raw balances of all users at `_blockNumber`.
-  /// @param _blockNumber The block at which to lookup the total balance.
-  function getPastTotalBalance(uint256 _blockNumber) public view returns (uint256) {
-    uint48 key = SafeCast.toUint48(_blockNumber);
+  /// @notice Returns the sum total of raw balances of all users at `_timepoint`.
+  /// @param _timepoint The timepoint at which to lookup the total balance,
+  /// either a block number or a timestamp as determined by
+  /// {GOVERNOR.token().clock()}.
+  function getPastTotalBalance(uint256 _timepoint) public view returns (uint256) {
+    uint48 key = SafeCast.toUint48(_timepoint);
     return totalBalanceCheckpoints.upperLookup(key);
   }
 }

src/interfaces/IVotingToken.sol

@@ -3,6 +3,7 @@ pragma solidity ^0.8.20;
 
 /// @dev The interface that flexible voting-compatible voting tokens are expected to support.
 interface IVotingToken {
+  function clock() external view returns (uint48);
   function transfer(address to, uint256 amount) external returns (bool);
   function transferFrom(address from, address to, uint256 amount) external returns (bool);
   function delegate(address delegatee) external;