Upgrade to OZ v5 (#66)

* WIP

* Can compile

* Fix tests and remove signature override

* Format code

* Cleanup pr

* Remove additional nonce code

* Fix compilation

* Lower test coverage

* Convert GovernorCountingFractional to use error types

* Improve natspec

* Add SPDX license identifier to FlexVotingClient

* Update the compiler version to the latest, and match pragmas to OpenZeppelin

* Switch to Checkpoints.Trace208

* Make IFractionalGovernor.token a view function

* Create mock FlexVotingClient for testing

* Add boilerplate for FlexVotingClient tests

* Implement deposit and withdraw for the mock

* Fix balance checkpointing in MockFlexVotingClient

* Add a voting test for FlexVotingClient

* Implement borrowing in MockFlexVotingClient

* Add a bunch more voting tests

* Add more voting tests

* Add a basic borrow test

* hodler -> user

* Test that castVote can be called multiple times

* Add tests for `borrow` and `withdraw`

* Test that users can't expressVote with bogus support types

* holder --> user

* Test casting without any votes expressed

* Assume lender is not borrower

* forge fmt

* Fix test name for scopelint conventions

* Bump to latest solc

* Bump coverage threshold

* Use existing enums

* Use absolute paths for imports

* forge fmt

---------

Co-authored-by: David Laprade <[email protected]>
Co-authored-by: Ben DiFrancesco <[email protected]>

Author: 3 people

.github/workflows/ci.yml

@@ -106,7 +106,7 @@ jobs:
         uses: zgosalvez/github-actions-report-lcov@v2
         with:
           coverage-files: ./lcov.info
-          minimum-coverage: 70 # Set coverage threshold.
+          minimum-coverage: 92 # Set coverage threshold.
 
   slither-analyze:
     runs-on: ubuntu-latest

.gitmodules

@@ -6,4 +6,4 @@
 	url = https://github.com/foundry-rs/forge-std
 [submodule "lib/openzeppelin-contracts"]
 	path = lib/openzeppelin-contracts
-	url = https://github.com/openzeppelin/openzeppelin-contracts
+	url = https://github.com/OpenZeppelin/openzeppelin-contracts

foundry.toml

@@ -1,10 +1,8 @@
 [profile.default]
-  # We don't specify a solc version because Aave contracts are pinned to 0.8.10,
-  # but we use more recent solc versions for other contracts, so we let forge
-  # auto-detect solc versions.
   optimizer = true
   optimizer_runs = 10_000_000
   remappings = ["@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts"]
+  solc_version = '0.8.28'
   verbosity = 3
 
 [profile.ci]

lib/openzeppelin-contracts

@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: MIT
-
-pragma solidity ^0.8.10;
+pragma solidity ^0.8.20;
 
 import "forge-std/Script.sol";
 

script/Deploy.s.sol

@@ -1,8 +1,8 @@
-// SPDX-License-Identifier: UNLICENSED
-pragma solidity >=0.8.10;
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.20;
 
 import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
-import {Checkpoints} from "@openzeppelin/contracts/utils/Checkpoints.sol";
+import {Checkpoints} from "@openzeppelin/contracts/utils/structs/Checkpoints.sol";
 import {IFractionalGovernor} from "./interfaces/IFractionalGovernor.sol";
 import {IVotingToken} from "./interfaces/IVotingToken.sol";
 
@@ -48,7 +48,12 @@ import {IVotingToken} from "./interfaces/IVotingToken.sol";
 /// of the rest.
 abstract contract FlexVotingClient {
   using SafeCast for uint256;
-  using Checkpoints for Checkpoints.History;
+
+  // @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.
+  using Checkpoints for Checkpoints.Trace208;
 
   /// @notice The voting options corresponding to those used in the Governor.
   enum VoteType {
@@ -76,12 +81,12 @@ abstract contract FlexVotingClient {
 
   /// @dev Mapping from address to the checkpoint history of raw balances
   /// of that address.
-  mapping(address => Checkpoints.History) private balanceCheckpoints;
+  mapping(address => Checkpoints.Trace208) private balanceCheckpoints;
 
   /// @dev History of the sum total of raw balances in the system. May or may
   /// not be equivalent to this contract's balance of `GOVERNOR`s token at a
   /// given time.
-  Checkpoints.History internal totalBalanceCheckpoints;
+  Checkpoints.Trace208 internal totalBalanceCheckpoints;
 
   /// @param _governor The address of the flex-voting-compatible governance contract.
   constructor(address _governor) {
@@ -92,7 +97,7 @@ abstract contract FlexVotingClient {
   /// token that `_user` has claim to in this system. It may or may not be
   /// equivalent to the withdrawable balance of `GOVERNOR`s token for `user`,
   /// e.g. if the internal representation of balance has been scaled down.
-  function _rawBalanceOf(address _user) internal view virtual returns (uint256);
+  function _rawBalanceOf(address _user) internal view virtual returns (uint208);
 
   /// @dev Used as the `reason` param when submitting a vote to `GOVERNOR`.
   function _castVoteReasonString() internal virtual returns (string memory) {
@@ -194,19 +199,21 @@ abstract contract FlexVotingClient {
 
   /// @dev Checkpoints the _user's current raw balance.
   function _checkpointRawBalanceOf(address _user) internal {
-    balanceCheckpoints[_user].push(_rawBalanceOf(_user));
+    balanceCheckpoints[_user].push(SafeCast.toUint48(block.number), _rawBalanceOf(_user));
   }
 
   /// @notice Returns the `_user`'s raw balance at `_blockNumber`.
   /// @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) {
-    return balanceCheckpoints[_user].getAtProbablyRecentBlock(_blockNumber);
+    uint48 key = SafeCast.toUint48(_blockNumber);
+    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) {
-    return totalBalanceCheckpoints.getAtProbablyRecentBlock(_blockNumber);
+    uint48 key = SafeCast.toUint48(_blockNumber);
+    return totalBalanceCheckpoints.upperLookup(key);
   }
 }

src/FlexVotingClient.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.10;
+pragma solidity ^0.8.20;
 
 import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
 import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";