-
Notifications
You must be signed in to change notification settings - Fork 12.1k
Add GovernorCountingFractional #5045
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
Changes from 17 commits
729ef4c
b56632a
66f2b3b
459ce71
7cff638
12123b0
bc9f970
93f460f
e792599
5ae1d9e
2bf4d2d
f9db044
7ffc6d2
e4d4fad
19c8142
0420d9a
8e95e96
95f3fb2
68033f5
37e9a0e
dce6fba
e0f5b71
f1b71bc
37391ce
72a420b
149fe65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'openzeppelin-solidity': minor | ||
--- | ||
|
||
`GovernorCountingFractional`: Add a governor counting module that allows distributing voting power amongst 3 options (For, Against, Abstain). |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,228 @@ | ||
// SPDX-License-Identifier: MIT | ||
|
||
pragma solidity ^0.8.20; | ||
|
||
import {Governor} from "../Governor.sol"; | ||
import {GovernorCountingSimple} from "./GovernorCountingSimple.sol"; | ||
import {Math} from "../../utils/math/Math.sol"; | ||
|
||
/** | ||
* @dev Extension of {Governor} for fractional voting. | ||
* | ||
* Similar to {GovernorCountingSimple}, this contract is a votes counting module for {Governor} that supports 3 options: | ||
* Against, For, Abstain. Additionally, it includes a fourth option: Fractional, which allows voters to split their voting | ||
* power amongst the other 3 options. | ||
* | ||
* Votes cast with the Fractional support must be accompanied by a `params` argument that is tree packed `uint128` values | ||
ernestognw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* representing the weight the delegate assigns to Against, For, and Abstain respectively. For those votes cast for the other | ||
* 3 options, the `params` argument must be empty. | ||
* | ||
* This is mostly useful when the delegate is a contract that implements its own rules for voting. These delegate-contracts | ||
* can cast fractional votes according to the preferences of multiple entities delegating their voting power. | ||
* | ||
* Some example use cases include: | ||
* | ||
* * Voting from tokens that are held by a DeFi pool | ||
* * Voting from an L2 with tokens held by a bridge | ||
* * Voting privately from a shielded pool using zero knowledge proofs. | ||
* | ||
* Based on ScopeLift's GovernorCountingFractional[https://github.com/ScopeLift/flexible-voting/blob/e5de2efd1368387b840931f19f3c184c85842761/src/GovernorCountingFractional.sol] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there any important differences that people should consider if they migrate from ScopeLift's implementation to this one, or if an interface has to support both? I think it may be worth listing them here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So far:
The voting format may vary after the reviews. I think it's currently fine since it reads it's "based on", so no compatibility should be assumed right away |
||
*/ | ||
abstract contract GovernorCountingFractional is Governor { | ||
using Math for *; | ||
|
||
ernestognw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
struct ProposalVote { | ||
uint256 againstVotes; | ||
uint256 forVotes; | ||
uint256 abstainVotes; | ||
mapping(address voter => uint256) usedVotes; | ||
Amxx marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/** | ||
* @dev Mapping from proposal ID to vote tallies for that proposal. | ||
*/ | ||
mapping(uint256 => ProposalVote) private _proposalVotes; | ||
|
||
/** | ||
* @dev A fractional vote params uses more votes than are available for that user. | ||
*/ | ||
error GovernorExceedRemainingWeight(address voter, uint256 usedVotes, uint256 remainingWeight); | ||
|
||
/** | ||
* @dev See {IGovernor-COUNTING_MODE}. | ||
*/ | ||
// solhint-disable-next-line func-name-mixedcase | ||
function COUNTING_MODE() public pure virtual override returns (string memory) { | ||
return "support=bravo&quorum=for,abstain¶ms=fractional"; | ||
} | ||
|
||
/** | ||
* @dev See {IGovernor-hasVoted}. | ||
*/ | ||
function hasVoted(uint256 proposalId, address account) public view virtual override returns (bool) { | ||
return usedVotes(proposalId, account) > 0; | ||
} | ||
|
||
/** | ||
* @dev Get the number of votes already cast by `account` for a proposal with `proposalId`. Useful for | ||
* integrations that allow delegates to cast rolling, partial votes. | ||
*/ | ||
function usedVotes(uint256 proposalId, address account) public view virtual returns (uint256) { | ||
return _proposalVotes[proposalId].usedVotes[account]; | ||
} | ||
|
||
/** | ||
* @dev Get current distribution of votes for a given proposal. | ||
*/ | ||
function proposalVotes( | ||
uint256 proposalId | ||
) public view virtual returns (uint256 againstVotes, uint256 forVotes, uint256 abstainVotes) { | ||
ProposalVote storage proposalVote = _proposalVotes[proposalId]; | ||
return (proposalVote.againstVotes, proposalVote.forVotes, proposalVote.abstainVotes); | ||
} | ||
|
||
/** | ||
* @dev See {Governor-_quorumReached}. | ||
*/ | ||
function _quorumReached(uint256 proposalId) internal view virtual override returns (bool) { | ||
ProposalVote storage proposalVote = _proposalVotes[proposalId]; | ||
return quorum(proposalSnapshot(proposalId)) <= proposalVote.forVotes + proposalVote.abstainVotes; | ||
} | ||
|
||
/** | ||
* @dev See {Governor-_voteSucceeded}. In this module, forVotes must be > againstVotes. | ||
*/ | ||
function _voteSucceeded(uint256 proposalId) internal view virtual override returns (bool) { | ||
ProposalVote storage proposalVote = _proposalVotes[proposalId]; | ||
return proposalVote.forVotes > proposalVote.againstVotes; | ||
} | ||
|
||
/** | ||
* @dev See {Governor-_countVote}. Function that records the delegate's votes. | ||
* | ||
* Executing this function consume's the delegate's weight on the proposal. This weight can be distributed amongst | ||
* the 3 options (Against, For, Abstain) by specifying a Fractional `support`. | ||
* | ||
* When support is anything other than Fractiona`, the `params` argument must be empty and the delegate's full remaining | ||
* weight is cast for the specified `support` option, as in {GovernorCountingSimple} and following the `VoteType` | ||
* enum from Governor Bravo. | ||
* | ||
* Given a Fractional `support`, the `params` argument must be tree packed `uint128` values representing the weight | ||
* the delegate assigns to Against, For, and Abstain respectively. This format can be produced using: | ||
* | ||
* `abi.encodePacked(uint128(againstVotes), uint128(forVotes), uint128(abstainVotes))` | ||
* | ||
* The sum total of the three decoded vote weights _must_ be less than or equal to the delegate's remaining weight | ||
* on the proposal, i.e. their checkpointed total weight minus votes already cast on the proposal. | ||
* | ||
* NOTE: Consider the number of votes is restricted to 128 bits. Depending on how many decimals the underlying token | ||
* has, a single voter may require to split their vote into multiple transactions. For precision higher than | ||
* ~30 decimals, large token holders may require an exponentially large number of transactions to cast their vote. | ||
* | ||
* See `_countVoteNominal` and `_countVoteFractional` for more details. | ||
*/ | ||
function _countVote( | ||
uint256 proposalId, | ||
address account, | ||
uint8 support, | ||
uint256 totalWeight, | ||
bytes memory params | ||
) internal virtual override returns (uint256) { | ||
// Compute number of remaining votes. Returns 0 on overflow. | ||
(, uint256 remainingWeight) = totalWeight.trySub(usedVotes(proposalId, account)); | ||
if (remainingWeight == 0) { | ||
revert GovernorAlreadyCastVote(account); | ||
} | ||
|
||
// For clarity of event indexing, fractional voting must be clearly advertised in the "support" field. | ||
if (support <= uint8(GovernorCountingSimple.VoteType.Abstain)) { | ||
if (params.length != 0) revert GovernorInvalidVoteParams(); | ||
return _countVoteNominal(proposalId, account, support, remainingWeight); | ||
} else if (support == uint8(GovernorCountingSimple.VoteType.Fractional)) { | ||
if (params.length != 0x30) revert GovernorInvalidVoteParams(); | ||
return _countVoteFractional(proposalId, account, params, remainingWeight); | ||
} else { | ||
revert GovernorInvalidVoteType(); | ||
} | ||
} | ||
|
||
/** | ||
* @dev Record votes with full weight cast for `support`. | ||
* | ||
* Because this function votes with the delegate's remaining weight, it can only be called once per proposal and | ||
* thus does not require any replay protection. | ||
*/ | ||
function _countVoteNominal( | ||
uint256 proposalId, | ||
address account, | ||
uint8 support, | ||
uint256 weight | ||
) private returns (uint256) { | ||
ProposalVote storage details = _proposalVotes[proposalId]; | ||
details.usedVotes[account] += weight; | ||
|
||
if (support == uint8(GovernorCountingSimple.VoteType.Against)) { | ||
details.againstVotes += weight; | ||
} else if (support == uint8(GovernorCountingSimple.VoteType.For)) { | ||
details.forVotes += weight; | ||
} else if (support == uint8(GovernorCountingSimple.VoteType.Abstain)) { | ||
details.abstainVotes += weight; | ||
} | ||
|
||
return weight; | ||
} | ||
|
||
/** | ||
* @dev Count votes with fractional weight. | ||
* | ||
* The `params` argument is expected to be three packed `uint128`: | ||
* `abi.encodePacked(uint128(againstVotes), uint128(forVotes), uint128(abstainVotes))` | ||
* | ||
* This function can be called multiple times for the same account and proposal (i.e. partial/rolling votes are | ||
* allowed). For example, an account with total weight of 10 could call this function three times with the | ||
* following vote data: | ||
* | ||
* * Against: 1, For: 0, Abstain: 2 | ||
* * Against: 3, For: 1, Abstain: 0 | ||
* * Against: 1, For: 1, Abstain: 1 | ||
* | ||
* Casting votes like this will make the calling account to cast a total of 5 `Against` votes, 2 `For` votes | ||
* and 3 `Abstain` votes. Though partial, votes are still final once cast and cannot be changed or overridden. | ||
* Subsequent partial votes simply increment existing totals. | ||
*/ | ||
function _countVoteFractional( | ||
uint256 proposalId, | ||
address account, | ||
bytes memory params, | ||
uint256 weight | ||
) private returns (uint256) { | ||
uint128 againstVotes = _extractUint128(params, 0); | ||
uint128 forVotes = _extractUint128(params, 1); | ||
uint128 abstainVotes = _extractUint128(params, 2); | ||
|
||
uint256 usedWeight = againstVotes + forVotes + abstainVotes; | ||
if (usedWeight > weight) { | ||
revert GovernorExceedRemainingWeight(account, usedWeight, weight); | ||
} | ||
|
||
ProposalVote storage details = _proposalVotes[proposalId]; | ||
if (againstVotes > 0) { | ||
details.againstVotes += againstVotes; | ||
} | ||
if (forVotes > 0) { | ||
details.forVotes += forVotes; | ||
} | ||
if (abstainVotes > 0) { | ||
details.abstainVotes += abstainVotes; | ||
} | ||
details.usedVotes[account] += usedWeight; | ||
|
||
return usedWeight; | ||
} | ||
|
||
function _extractUint128(bytes memory data, uint256 pos) private pure returns (uint128 result) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function concerns me because the length of Is it possible to just check the validity of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think having the helper to unpack all three parameters is the easiest to ensure memory safetyness. I'll update with this alternative There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is private, and everywhere we call it we first validate the length. Is that not good enough? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GIven how frequent it is for people to copy-paste from our code I think we should try to make functions self-contained and well documented even if they're private. In this case, not only that I think this is the cleanest way of making the assembly block |
||
assembly { | ||
result := shr(128, mload(add(data, add(0x20, mul(0x10, pos))))) | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
// SPDX-License-Identifier: MIT | ||
|
||
pragma solidity ^0.8.20; | ||
|
||
import {Governor} from "../../governance/Governor.sol"; | ||
import {GovernorSettings} from "../../governance/extensions/GovernorSettings.sol"; | ||
import {GovernorCountingFractional} from "../../governance/extensions/GovernorCountingFractional.sol"; | ||
import {GovernorVotesQuorumFraction} from "../../governance/extensions/GovernorVotesQuorumFraction.sol"; | ||
|
||
abstract contract GovernorFractionalMock is GovernorSettings, GovernorVotesQuorumFraction, GovernorCountingFractional { | ||
function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) { | ||
return super.proposalThreshold(); | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.