Skip to content

Commit 6210a41

Browse files
davidlapradedavidlaprade
committed
Restructure inheritance for FlexVotingClient
1 parent 557ff2b commit 6210a41

File tree

5 files changed

+153
-124
lines changed

5 files changed

+153
-124
lines changed

‎src/FlexVotingBase.sol‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
// SPDX-License-Identifier: MIT
2+
pragma solidity ^0.8.20;
3+
4+
import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
5+
import {Checkpoints} from "@openzeppelin/contracts/utils/structs/Checkpoints.sol";
6+
import {IFractionalGovernor} from "src/interfaces/IFractionalGovernor.sol";
7+
import {IVotingToken} from "src/interfaces/IVotingToken.sol";
8+
9+
/// @notice This is an abstract contract designed to make it easy to build clients
10+
/// for governance systems that inherit from GovernorCountingFractional, a.k.a.
11+
/// Flexible Voting governors.
12+
///
13+
/// A "client" in this sense is a contract that:
14+
15+
/// - (a) receives deposits of governance tokens from its users,
16+
/// - (b) gives said depositors the ability to express their voting preferences
17+
/// on governance proposals, and
18+
/// - (c) casts votes on said proposals to flexible voting governors according
19+
/// to the expressed preferences of its depositors.
20+
///
21+
/// This contract assumes that a child contract will implement a mechanism for
22+
/// receiving and storing deposit balances, part (a). With that in place, this
23+
/// contract supplies features (b) and (c).
24+
///
25+
/// A key concept here is that of a user's "raw balance". The raw balance is the
26+
/// system's internal representation of a user's claim on the governance tokens
27+
/// that it custodies. Since different systems might represent such claims in
28+
/// different ways, this contract leaves the implementation of the `_rawBalance`
29+
/// function to the child contract.
30+
///
31+
/// The simplest such representation would be to directly store the cumulative
32+
/// balance of the governance token that the user has deposited. In such a
33+
/// system, the amount that the user deposits is the amount that the user has
34+
/// claim to. If the user has claim to 1e18 governance tokens, the internal
35+
/// representation is just 1e18.
36+
///
37+
/// In many systems, however, the raw balance will not be equivalent to the
38+
/// amount of governance tokens the user has claim to. In Aave, for example,
39+
/// deposit amounts are scaled down by an ever-increasing index that represents
40+
/// the cumulative amount of interest earned over the lifetime of deposits. The
41+
/// "raw balance" of a user in Aave's case is this scaled down amount, since it
42+
/// is the value that represents the user's claim on deposits. Thus for Aave, a
43+
/// users's raw balance will always be less than the actual amount they have
44+
/// claim to.
45+
///
46+
/// If the raw balance can be identified and defined for a system, and
47+
/// `_rawBalance` can be implemented for it, then this contract will take care
48+
/// of the rest.
49+
abstract contract FlexVotingBase {
50+
using SafeCast for uint256;
51+
52+
// @dev Trace208 is used instead of Trace224 because the former allocates 48
53+
// bits to its _key. We need at least 48 bits because the _key is going to be
54+
// a timepoint. Timepoints in the context of ERC20Votes and ERC721Votes
55+
// conform to the EIP-6372 standard, which specifies they be uint48s.
56+
using Checkpoints for Checkpoints.Trace208;
57+
58+
/// @notice The governor contract associated with this governance token. It
59+
/// must be one that supports fractional voting, e.g. GovernorCountingFractional.
60+
IFractionalGovernor public immutable GOVERNOR;
61+
62+
/// @dev Mapping from address to the checkpoint history of internal voting
63+
/// weight for that address, i.e. how much weight they can call `expressVote`
64+
/// with at a given time.
65+
mapping(address => Checkpoints.Trace208) internal voteWeightCheckpoints;
66+
67+
/// @dev History of the sum total of voting weight in the system. May or may
68+
/// not be equivalent to this contract's balance of `GOVERNOR`s token at a
69+
/// given time.
70+
Checkpoints.Trace208 internal totalVoteWeightCheckpoints;
71+
72+
/// @param _governor The address of the flex-voting-compatible governance contract.
73+
constructor(address _governor) {
74+
GOVERNOR = IFractionalGovernor(_governor);
75+
}
76+
77+
/// @dev Returns a representation of the current amount of `GOVERNOR`s
78+
/// token that `_user` has claim to in this system. It may or may not be
79+
/// equivalent to the withdrawable balance of `GOVERNOR`s token for `user`,
80+
/// e.g. if the internal representation of balance has been scaled down.
81+
function _rawBalanceOf(address _user) internal view virtual returns (uint208);
82+
83+
// TODO rename to avoid collision with FlexVotingDelegatable.
84+
/// @dev Delegates the present contract's voting rights with `GOVERNOR` to itself.
85+
function _selfDelegate() internal {
86+
IVotingToken(GOVERNOR.token()).delegate(address(this));
87+
}
88+
89+
function _applyDeltaToCheckpoint(
90+
Checkpoints.Trace208 storage _checkpoint,
91+
int256 _delta
92+
) internal returns (uint208 _prevTotal, uint208 _newTotal) {
93+
// The casting in this function is safe since:
94+
// - if oldTotal + delta > int256.max it will panic and revert.
95+
// - if |delta| <= oldTotal there is no risk of wrapping
96+
// - if |delta| > oldTotal
97+
// * uint256(oldTotal + delta) will wrap but the wrapped value will
98+
// necessarily be greater than uint208.max, so SafeCast will revert.
99+
// * the lowest that oldTotal + delta can be is int256.min (when
100+
// oldTotal is 0 and delta is int256.min). The wrapped value of a
101+
// negative signed integer is:
102+
// wrapped(integer) = uint256.max + integer
103+
// Substituting:
104+
// wrapped(int256.min) = uint256.max + int256.min
105+
// But:
106+
// uint256.max + int256.min > uint208.max
107+
// Substituting again:
108+
// wrapped(int256.min) > uint208.max, which will revert when safecast.
109+
_prevTotal = _checkpoint.latest();
110+
int256 _castTotal = int256(uint256(_prevTotal));
111+
_newTotal = SafeCast.toUint208(uint256(_castTotal + _delta));
112+
113+
uint48 _timepoint = IVotingToken(GOVERNOR.token()).clock();
114+
_checkpoint.push(_timepoint, _newTotal);
115+
}
116+
117+
/// @dev Checkpoints internal voting weight of `user` after applying `_delta`.
118+
function _checkpointVoteWeightOf(
119+
address _user,
120+
int256 _delta
121+
) internal virtual {
122+
_applyDeltaToCheckpoint(voteWeightCheckpoints[_user], _delta);
123+
}
124+
125+
/// @dev Checkpoints the total vote weight after applying `_delta`.
126+
function _checkpointTotalVoteWeight(int256 _delta) internal virtual {
127+
_applyDeltaToCheckpoint(totalVoteWeightCheckpoints, _delta);
128+
}
129+
}

‎src/FlexVotingClient.sol‎

Lines changed: 15 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -5,57 +5,22 @@ import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
55
import {Checkpoints} from "@openzeppelin/contracts/utils/structs/Checkpoints.sol";
66
import {IFractionalGovernor} from "src/interfaces/IFractionalGovernor.sol";
77
import {IVotingToken} from "src/interfaces/IVotingToken.sol";
8+
import {FlexVotingBase} from "src/FlexVotingBase.sol";
89

910
/// @notice This is an abstract contract designed to make it easy to build clients
1011
/// for governance systems that inherit from GovernorCountingFractional, a.k.a.
1112
/// Flexible Voting governors.
1213
///
13-
/// A "client" in this sense is a contract that:
14-
15-
/// - (a) receives deposits of governance tokens from its users,
16-
/// - (b) gives said depositors the ability to express their voting preferences
17-
/// on governance proposals, and
18-
/// - (c) casts votes on said proposals to flexible voting governors according
19-
/// to the expressed preferences of its depositors.
20-
///
21-
/// This contract assumes that a child contract will implement a mechanism for
22-
/// receiving and storing deposit balances, part (a). With that in place, this
23-
/// contract supplies features (b) and (c).
24-
///
25-
/// A key concept here is that of a user's "raw balance". The raw balance is the
26-
/// system's internal representation of a user's claim on the governance tokens
27-
/// that it custodies. Since different systems might represent such claims in
28-
/// different ways, this contract leaves the implementation of the `_rawBalance`
29-
/// function to the child contract.
30-
///
31-
/// The simplest such representation would be to directly store the cumulative
32-
/// balance of the governance token that the user has deposited. In such a
33-
/// system, the amount that the user deposits is the amount that the user has
34-
/// claim to. If the user has claim to 1e18 governance tokens, the internal
35-
/// representation is just 1e18.
36-
///
37-
/// In many systems, however, the raw balance will not be equivalent to the
38-
/// amount of governance tokens the user has claim to. In Aave, for example,
39-
/// deposit amounts are scaled down by an ever-increasing index that represents
40-
/// the cumulative amount of interest earned over the lifetime of deposits. The
41-
/// "raw balance" of a user in Aave's case is this scaled down amount, since it
42-
/// is the value that represents the user's claim on deposits. Thus for Aave, a
43-
/// users's raw balance will always be less than the actual amount they have
44-
/// claim to.
45-
///
46-
/// If the raw balance can be identified and defined for a system, and
47-
/// `_rawBalance` can be implemented for it, then this contract will take care
48-
/// of the rest.
49-
abstract contract FlexVotingClient {
50-
using SafeCast for uint256;
51-
52-
// @dev Trace208 is used instead of Trace224 because the former allocates 48
53-
// bits to its _key. We need at least 48 bits because the _key is going to be
54-
// a timepoint. Timepoints in the context of ERC20Votes and ERC721Votes
55-
// conform to the EIP-6372 standard, which specifies they be uint48s.
14+
/// This contract extends FlexVotingBase, adding two features:
15+
/// (a) the ability for depositors to express voting preferences on
16+
/// {Governor}'s proprosals, and
17+
/// (b) the ability to cast fractional, rolled up votes on behalf of depositors.
18+
abstract contract FlexVotingClient is FlexVotingBase {
5619
using Checkpoints for Checkpoints.Trace208;
20+
using SafeCast for uint256;
5721

58-
/// @notice The voting options corresponding to those used in the Governor.
22+
/// @notice The voting options. The order of options should match that of the
23+
/// voting options in the corresponding {Governor} contract.
5924
enum VoteType {
6025
Against,
6126
For,
@@ -70,54 +35,26 @@ abstract contract FlexVotingClient {
7035
}
7136

7237
/// @dev Map proposalId to an address to whether they have voted on this proposal.
73-
mapping(uint256 => mapping(address => bool)) private proposalVotersHasVoted;
38+
mapping(uint256 => mapping(address => bool)) private proposalVoterHasVoted;
7439

7540
/// @notice Map proposalId to vote totals expressed on this proposal.
7641
mapping(uint256 => ProposalVote) public proposalVotes;
7742

78-
/// @notice The governor contract associated with this governance token. It
79-
/// must be one that supports fractional voting, e.g. GovernorCountingFractional.
80-
IFractionalGovernor public immutable GOVERNOR;
81-
82-
/// @dev Mapping from address to the checkpoint history of internal voting
83-
/// weight for that address, i.e. how much weight they can call `expressVote`
84-
/// with at a given time.
85-
mapping(address => Checkpoints.Trace208) internal voteWeightCheckpoints;
86-
87-
/// @dev History of the sum total of voting weight in the system. May or may
88-
/// not be equivalent to this contract's balance of `GOVERNOR`s token at a
89-
/// given time.
90-
Checkpoints.Trace208 internal totalVoteWeightCheckpoints;
91-
92-
// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/7b74442c5e87ea51dde41c7f18a209fa5154f1a4/contracts/governance/extensions/GovernorCountingFractional.sol#L37
43+
/// Constant used by OZ's implementation of {GovernorCountingFractional} to
44+
/// signal fractional voting.
45+
/// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/7b74442c5e87ea51dde41c7f18a209fa5154f1a4/contracts/governance/extensions/GovernorCountingFractional.sol#L37
9346
uint8 internal constant VOTE_TYPE_FRACTIONAL = 255;
9447

9548
error FlexVotingClient__NoVotingWeight();
9649
error FlexVotingClient__AlreadyVoted();
9750
error FlexVotingClient__InvalidSupportValue();
9851
error FlexVotingClient__NoVotesExpressed();
9952

100-
/// @param _governor The address of the flex-voting-compatible governance contract.
101-
constructor(address _governor) {
102-
GOVERNOR = IFractionalGovernor(_governor);
103-
}
104-
105-
/// @dev Returns a representation of the current amount of `GOVERNOR`s
106-
/// token that `_user` has claim to in this system. It may or may not be
107-
/// equivalent to the withdrawable balance of `GOVERNOR`s token for `user`,
108-
/// e.g. if the internal representation of balance has been scaled down.
109-
function _rawBalanceOf(address _user) internal view virtual returns (uint208);
110-
11153
/// @dev Used as the `reason` param when submitting a vote to `GOVERNOR`.
11254
function _castVoteReasonString() internal virtual returns (string memory) {
11355
return "rolled-up vote from governance token holders";
11456
}
11557

116-
/// @dev Delegates the present contract's voting rights with `GOVERNOR` to itself.
117-
function _selfDelegate() internal {
118-
IVotingToken(GOVERNOR.token()).delegate(address(this));
119-
}
120-
12158
/// @notice Allow the caller to express their voting preference for a given
12259
/// proposal. Their preference is recorded internally but not moved to the
12360
/// Governor until `castVote` is called.
@@ -128,8 +65,8 @@ abstract contract FlexVotingClient {
12865
uint256 weight = getPastVoteWeight(voter, GOVERNOR.proposalSnapshot(proposalId));
12966
if (weight == 0) revert FlexVotingClient__NoVotingWeight();
13067

131-
if (proposalVotersHasVoted[proposalId][voter]) revert FlexVotingClient__AlreadyVoted();
132-
proposalVotersHasVoted[proposalId][voter] = true;
68+
if (proposalVoterHasVoted[proposalId][voter]) revert FlexVotingClient__AlreadyVoted();
69+
proposalVoterHasVoted[proposalId][voter] = true;
13370

13471
if (support == uint8(VoteType.Against)) {
13572
proposalVotes[proposalId].againstVotes += SafeCast.toUint128(weight);
@@ -202,44 +139,6 @@ abstract contract FlexVotingClient {
202139
);
203140
}
204141

205-
function _applyDeltaToCheckpoint(Checkpoints.Trace208 storage _checkpoint, int256 _delta)
206-
internal
207-
returns (uint208 _prevTotal, uint208 _newTotal)
208-
{
209-
// The casting in this function is safe since:
210-
// - if oldTotal + delta > int256.max it will panic and revert.
211-
// - if |delta| <= oldTotal there is no risk of wrapping
212-
// - if |delta| > oldTotal
213-
// * uint256(oldTotal + delta) will wrap but the wrapped value will
214-
// necessarily be greater than uint208.max, so SafeCast will revert.
215-
// * the lowest that oldTotal + delta can be is int256.min (when
216-
// oldTotal is 0 and delta is int256.min). The wrapped value of a
217-
// negative signed integer is:
218-
// wrapped(integer) = uint256.max + integer
219-
// Substituting:
220-
// wrapped(int256.min) = uint256.max + int256.min
221-
// But:
222-
// uint256.max + int256.min > uint208.max
223-
// Substituting again:
224-
// wrapped(int256.min) > uint208.max, which will revert when safecast.
225-
_prevTotal = _checkpoint.latest();
226-
int256 _castTotal = int256(uint256(_prevTotal));
227-
_newTotal = SafeCast.toUint208(uint256(_castTotal + _delta));
228-
229-
uint48 _timepoint = IVotingToken(GOVERNOR.token()).clock();
230-
_checkpoint.push(_timepoint, _newTotal);
231-
}
232-
233-
/// @dev Checkpoints internal voting weight of `user` after applying `_delta`.
234-
function _checkpointVoteWeightOf(address _user, int256 _delta) internal virtual {
235-
_applyDeltaToCheckpoint(voteWeightCheckpoints[_user], _delta);
236-
}
237-
238-
/// @dev Checkpoints the total vote weight after applying `_delta`.
239-
function _checkpointTotalVoteWeight(int256 _delta) internal virtual {
240-
_applyDeltaToCheckpoint(totalVoteWeightCheckpoints, _delta);
241-
}
242-
243142
/// @notice Returns the `_user`'s internal voting weight at `_timepoint`.
244143
/// @param _user The account that's historical vote weight will be looked up.
245144
/// @param _timepoint The timepoint at which to lookup the _user's internal

‎src/FlexVotingDelegatable.sol‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@ import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
55
import {Context} from "@openzeppelin/contracts/utils/Context.sol";
66
import {Checkpoints} from "@openzeppelin/contracts/utils/structs/Checkpoints.sol";
77

8-
import {IVotingToken} from "src/interfaces/IVotingToken.sol";
9-
import {FlexVotingClient} from "src/FlexVotingClient.sol";
8+
import {FlexVotingBase} from "src/FlexVotingBase.sol";
109

11-
abstract contract FlexVotingDelegatable is Context, FlexVotingClient {
10+
abstract contract FlexVotingDelegatable is Context, FlexVotingBase {
1211
using Checkpoints for Checkpoints.Trace208;
1312

1413
// @dev Emitted when an account changes its delegate.

‎test/MockFlexVotingClient.sol‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
66
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
77
import {ERC20Votes} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Votes.sol";
88
import {IVotingToken} from "src/interfaces/IVotingToken.sol";
9+
import {FlexVotingBase} from "src/FlexVotingBase.sol";
910
import {FlexVotingClient} from "src/FlexVotingClient.sol";
1011

1112
contract MockFlexVotingClient is FlexVotingClient {
@@ -20,7 +21,7 @@ contract MockFlexVotingClient is FlexVotingClient {
2021
/// @notice Map borrower to total amount borrowed.
2122
mapping(address => uint256) public borrowTotal;
2223

23-
constructor(address _governor) FlexVotingClient(_governor) {
24+
constructor(address _governor) FlexVotingBase(_governor) {
2425
TOKEN = ERC20Votes(GOVERNOR.token());
2526
_selfDelegate();
2627
}

‎test/MockFlexVotingDelegatableClient.sol‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,15 @@ pragma solidity >=0.8.10;
44
import {FlexVotingClient} from "src/FlexVotingClient.sol";
55
import {FlexVotingDelegatable} from "src/FlexVotingDelegatable.sol";
66
import {MockFlexVotingClient} from "test/MockFlexVotingClient.sol";
7+
import {FlexVotingBase} from "src/FlexVotingBase.sol";
78

89
contract MockFlexVotingDelegatableClient is MockFlexVotingClient, FlexVotingDelegatable {
910
constructor(address _governor) MockFlexVotingClient(_governor) {}
1011

11-
function _checkpointVoteWeightOf(address _user, int256 _delta)
12-
internal
13-
override(FlexVotingClient, FlexVotingDelegatable)
14-
{
12+
function _checkpointVoteWeightOf(
13+
address _user,
14+
int256 _delta
15+
) internal override(FlexVotingBase, FlexVotingDelegatable) {
1516
return FlexVotingDelegatable._checkpointVoteWeightOf(_user, _delta);
1617
}
1718
}

0 commit comments

Comments
 (0)