Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to OZ v5 #66

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ jobs:
uses: zgosalvez/github-actions-report-lcov@v2
with:
coverage-files: ./lcov.info
minimum-coverage: 70 # Set coverage threshold.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add more tests

minimum-coverage: 67 # Set coverage threshold.

slither-analyze:
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 1 addition & 3 deletions foundry.toml
Original file line number Diff line number Diff line change
@@ -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.25'
verbosity = 3

[profile.ci]
Expand Down
2 changes: 1 addition & 1 deletion lib/openzeppelin-contracts
3 changes: 1 addition & 2 deletions script/Deploy.s.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.10;
pragma solidity ^0.8.20;

import "forge-std/Script.sol";

Expand Down
22 changes: 12 additions & 10 deletions src/FlexVotingClient.sol
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -48,7 +48,7 @@ import {IVotingToken} from "./interfaces/IVotingToken.sol";
/// of the rest.
abstract contract FlexVotingClient {
using SafeCast for uint256;
using Checkpoints for Checkpoints.History;
using Checkpoints for Checkpoints.Trace224;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be a 208?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to allign with ERC-5805, then yes

/// @notice The voting options corresponding to those used in the Governor.
enum VoteType {
Expand Down Expand Up @@ -76,12 +76,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.Trace224) 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.Trace224 internal totalBalanceCheckpoints;

/// @param _governor The address of the flex-voting-compatible governance contract.
constructor(address _governor) {
Expand All @@ -92,7 +92,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 (uint224);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed because there is no Trace256. Should we write one for OZ?

Copy link

@Amxx Amxx May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Traces are a list of checkpoint. Each checkpoint contains:

  • a key
  • a value

such that sizeof(key) + sizeof(value) = 256

  • Trace224 is uint224 for value, and uint32 for key
  • Trace208 is uint208 for value, and uint48 for key

Trace256 would not have any room for the key ... unless you start have checkpoints that are 2 slots.


/// @dev Used as the `reason` param when submitting a vote to `GOVERNOR`.
function _castVoteReasonString() internal virtual returns (string memory) {
Expand Down Expand Up @@ -194,19 +194,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.toUint32(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);
uint32 key = SafeCast.toUint32(_blockNumber);
return balanceCheckpoints[_user].upperLookup(key);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


/// @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);
uint32 key = SafeCast.toUint32(_blockNumber);
return totalBalanceCheckpoints.upperLookup(key);
}
}
2 changes: 1 addition & 1 deletion src/FractionalPool.sol
Original file line number Diff line number Diff line change
@@ -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";
Expand Down
Loading
Loading