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

Add timestamp based governor with EIP-6372 and EIP-5805 #3934

Merged
merged 70 commits into from
Feb 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
4312336
Add EIP-5805 clock to the Votes module
Amxx Jan 6, 2023
3d11319
Add EIP-5805 clock to ERC20Votes
Amxx Jan 6, 2023
cee1f9c
make governor clock dependant
Amxx Jan 6, 2023
f5b77d8
wording blockNumber → timepoint
Amxx Jan 6, 2023
cb1fa78
test ERC20Votes with timestamp
Amxx Jan 9, 2023
b1c155d
check clock return value
Amxx Jan 9, 2023
c6d3ef9
fix lint
Amxx Jan 9, 2023
c73f08f
test Votes.sol in timestamp mode
Amxx Jan 9, 2023
8a70c35
test governor with timestamp
Amxx Jan 11, 2023
0981e95
all governance test against blockNumber & timestamp modes
Amxx Jan 11, 2023
bcedb7e
add testing of new governor against legacy voting tokens
Amxx Jan 12, 2023
fdeabb7
spelling
Amxx Jan 12, 2023
5d2cd45
Merge branch 'master' into feature/timestamp-governor
Amxx Jan 17, 2023
fb0619c
revert IVotes and add IERC5805 that extends IVotes with clock functions
Amxx Jan 20, 2023
82ff76b
rewrite time helper
Amxx Jan 20, 2023
21588f0
add retyped from directive
Amxx Jan 25, 2023
3ae7f12
more retyped-from directives
Amxx Jan 25, 2023
490a379
add IERC6372.sol
Amxx Jan 25, 2023
c3d83d6
fix retype annotations, add missing
frangio Jan 27, 2023
b2bd811
Merge branch 'master'
Amxx Jan 27, 2023
d16a02e
Update README.adoc
Amxx Jan 27, 2023
2546026
Update ERC20VotesLegacyMock.sol
Amxx Jan 27, 2023
8348311
compact the new ProposalCore struct
Amxx Jan 30, 2023
7c9d08f
fix
Amxx Jan 30, 2023
cfd7d79
refactor retypes
Amxx Jan 30, 2023
b7e46ed
cleanup
Amxx Jan 31, 2023
37d5a3d
Apply suggestions from code review
Amxx Jan 31, 2023
7550395
Merge branch 'master'
Amxx Jan 31, 2023
21483ad
Update contracts/governance/README.adoc
Amxx Feb 1, 2023
9359d68
add upperLookupRecent to Checkpoints
Amxx Feb 1, 2023
97a34fd
use upperLookupRecent in Votes.sol
Amxx Feb 1, 2023
5cae0e6
Merge branch 'master' into feature/timestamp-governor
Amxx Feb 1, 2023
0822651
fix lint
Amxx Feb 1, 2023
d49ab18
fix lint
Amxx Feb 1, 2023
ecf0bde
reorder for reentrancy
Amxx Feb 1, 2023
0c05bc7
fix inheritance ordering
Amxx Feb 1, 2023
662c96e
put storage error directly in the output
Amxx Feb 1, 2023
0fe6bb5
simplify output
Amxx Feb 1, 2023
3a2882d
use block solhint-disable/solhint-enable to improve readability
Amxx Feb 1, 2023
b5f0b63
add a proposalProposer accessor
Amxx Feb 1, 2023
250ba20
avoid duplicate sload in GovernorCompatibilityBravo.cancel
Amxx Feb 1, 2023
a851552
Merge branch 'master' into feature/timestamp-governor
Amxx Feb 1, 2023
a75ca18
Apply suggestions from code review
Amxx Feb 2, 2023
07b60a7
Apply suggestions from code review
Amxx Feb 2, 2023
dab340d
safecast
Amxx Feb 2, 2023
f27149b
Merge branch 'feature/timestamp-governor' of github.com:Amxx/openzepp…
Amxx Feb 2, 2023
54632b8
lint
Amxx Feb 2, 2023
32e0ebf
changeset
Amxx Feb 2, 2023
18174a9
relax gas compare requierement to still produce an output when tests are
Amxx Feb 2, 2023
e1be9a2
improve comments
Amxx Feb 2, 2023
6f7f346
use safecast in mocks
Amxx Feb 2, 2023
b173774
do recent lookup for _quorumNumeratorHistory
Amxx Feb 2, 2023
3d15872
Merge branch 'master' into feature/timestamp-governor
Amxx Feb 3, 2023
759bbd8
change casts
Amxx Feb 3, 2023
cf548e3
Merge branch 'master' into feature/timestamp-governor
Amxx Feb 3, 2023
155ee75
Update ninety-hornets-kick.md
frangio Feb 3, 2023
b192f8c
Update four-bats-sniff.md
frangio Feb 3, 2023
c772388
overriden -> overridden
frangio Feb 3, 2023
8cec0b6
Merge branch 'master' into feature/timestamp-governor
frangio Feb 4, 2023
e4fb558
Merge branch 'master' into feature/timestamp-governor
Amxx Feb 6, 2023
773c4c0
Update contracts/governance/extensions/GovernorVotesQuorumFraction.sol
Amxx Feb 6, 2023
df59b36
add EIP6372.behavior.js
Amxx Feb 6, 2023
3c6c942
improve coverage
Amxx Feb 6, 2023
9a42c99
rename event params
Amxx Feb 6, 2023
11ac9da
Update contracts/governance/extensions/GovernorTimelockCompound.sol
Amxx Feb 9, 2023
cc0f45f
Merge branch 'master'
Amxx Feb 9, 2023
e2eb048
Update governance.js
Amxx Feb 9, 2023
5e54e88
Update ERC721Votes.test.js
Amxx Feb 9, 2023
60992b6
improve coverage
Amxx Feb 9, 2023
8397ce7
remove full IGovernor from ERC165
Amxx Feb 9, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/four-bats-sniff.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`Governor`: Enable timestamp operation for blockchains without a stable block time. This is achieved by connecting a Governor's internal clock to match a voting token's EIP-6372 interface.
5 changes: 5 additions & 0 deletions .changeset/ninety-hornets-kick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`Votes`, `ERC20Votes`, `ERC721Votes`: support timestamp checkpointing using EIP-6372.
2 changes: 1 addition & 1 deletion .github/actions/storage-layout/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ runs:
- name: Compare layouts
if: steps.reference.outcome == 'success' && github.event_name == 'pull_request'
run: |
node scripts/checks/compare-layout.js --head ${{ inputs.layout }} --ref ${{ inputs.ref_layout }} >> $GITHUB_STEP_SUMMARY
node scripts/checks/compare-layout.js --head ${{ inputs.layout }} --ref ${{ inputs.ref_layout }}
frangio marked this conversation as resolved.
Show resolved Hide resolved
shell: bash
- name: Rename artifacts for upload
if: github.event_name != 'pull_request'
Expand Down
85 changes: 51 additions & 34 deletions contracts/governance/Governor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import "../utils/math/SafeCast.sol";
import "../utils/structs/DoubleEndedQueue.sol";
import "../utils/Address.sol";
import "../utils/Context.sol";
import "../utils/Timers.sol";
import "./IGovernor.sol";

/**
Expand All @@ -29,22 +28,29 @@ import "./IGovernor.sol";
abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receiver, IERC1155Receiver {
frangio marked this conversation as resolved.
Show resolved Hide resolved
using DoubleEndedQueue for DoubleEndedQueue.Bytes32Deque;
using SafeCast for uint256;
using Timers for Timers.BlockNumber;

bytes32 public constant BALLOT_TYPEHASH = keccak256("Ballot(uint256 proposalId,uint8 support)");
bytes32 public constant EXTENDED_BALLOT_TYPEHASH =
keccak256("ExtendedBallot(uint256 proposalId,uint8 support,string reason,bytes params)");

// solhint-disable var-name-mixedcase
struct ProposalCore {
Timers.BlockNumber voteStart;
Timers.BlockNumber voteEnd;
// --- start retyped from Timers.BlockNumber at offset 0x00 ---
frangio marked this conversation as resolved.
Show resolved Hide resolved
uint64 voteStart;
address proposer;
Amxx marked this conversation as resolved.
Show resolved Hide resolved
bytes4 __gap_unused0;
// --- start retyped from Timers.BlockNumber at offset 0x20 ---
uint64 voteEnd;
bytes24 __gap_unused1;
// --- Remaining fields starting at offset 0x40 ---------------
bool executed;
bool canceled;
address proposer;
}
// solhint-enable var-name-mixedcase

string private _name;

/// @custom:oz-retyped-from mapping(uint256 => Governor.ProposalCore)
mapping(uint256 => ProposalCore) private _proposals;

// This queue keeps track of the governor operating on itself. Calls to functions protected by the
Expand Down Expand Up @@ -96,12 +102,13 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
return
interfaceId ==
(type(IGovernor).interfaceId ^
type(IERC6372).interfaceId ^
this.cancel.selector ^
this.castVoteWithReasonAndParams.selector ^
this.castVoteWithReasonAndParamsBySig.selector ^
this.getVotesWithParams.selector) ||
interfaceId == (type(IGovernor).interfaceId ^ this.cancel.selector) ||
interfaceId == type(IGovernor).interfaceId ||
frangio marked this conversation as resolved.
Show resolved Hide resolved
// Previous interface for backwards compatibility
interfaceId == (type(IGovernor).interfaceId ^ type(IERC6372).interfaceId ^ this.cancel.selector) ||
interfaceId == type(IERC1155Receiver).interfaceId ||
super.supportsInterface(interfaceId);
}
Expand Down Expand Up @@ -162,13 +169,15 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
revert("Governor: unknown proposal id");
}

if (snapshot >= block.number) {
uint256 currentTimepoint = clock();

if (snapshot >= currentTimepoint) {
return ProposalState.Pending;
}

uint256 deadline = proposalDeadline(proposalId);

if (deadline >= block.number) {
if (deadline >= currentTimepoint) {
return ProposalState.Active;
}

Expand All @@ -179,25 +188,32 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
}
}

/**
* @dev Part of the Governor Bravo's interface: _"The number of votes required in order for a voter to become a proposer"_.
*/
function proposalThreshold() public view virtual returns (uint256) {
return 0;
}

/**
* @dev See {IGovernor-proposalSnapshot}.
*/
function proposalSnapshot(uint256 proposalId) public view virtual override returns (uint256) {
return _proposals[proposalId].voteStart.getDeadline();
return _proposals[proposalId].voteStart;
}

/**
* @dev See {IGovernor-proposalDeadline}.
*/
function proposalDeadline(uint256 proposalId) public view virtual override returns (uint256) {
return _proposals[proposalId].voteEnd.getDeadline();
return _proposals[proposalId].voteEnd;
}

/**
* @dev Part of the Governor Bravo's interface: _"The number of votes required in order for a voter to become a proposer"_.
* @dev Address of the proposer
*/
function proposalThreshold() public view virtual returns (uint256) {
return 0;
function _proposalProposer(uint256 proposalId) internal view virtual returns (address) {
return _proposals[proposalId].proposer;
}

/**
Expand All @@ -211,13 +227,9 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
function _voteSucceeded(uint256 proposalId) internal view virtual returns (bool);

/**
* @dev Get the voting weight of `account` at a specific `blockNumber`, for a vote as described by `params`.
* @dev Get the voting weight of `account` at a specific `timepoint`, for a vote as described by `params`.
*/
function _getVotes(
address account,
uint256 blockNumber,
bytes memory params
) internal view virtual returns (uint256);
function _getVotes(address account, uint256 timepoint, bytes memory params) internal view virtual returns (uint256);

/**
* @dev Register a vote for `proposalId` by `account` with a given `support`, voting `weight` and voting `params`.
Expand Down Expand Up @@ -252,9 +264,10 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
string memory description
) public virtual override returns (uint256) {
address proposer = _msgSender();
uint256 currentTimepoint = clock();

require(
getVotes(proposer, block.number - 1) >= proposalThreshold(),
getVotes(proposer, currentTimepoint - 1) >= proposalThreshold(),
"Governor: proposer votes below proposal threshold"
);

Expand All @@ -263,16 +276,20 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
require(targets.length == values.length, "Governor: invalid proposal length");
require(targets.length == calldatas.length, "Governor: invalid proposal length");
require(targets.length > 0, "Governor: empty proposal");
require(_proposals[proposalId].proposer == address(0), "Governor: proposal already exists");

ProposalCore storage proposal = _proposals[proposalId];
require(proposal.voteStart.isUnset(), "Governor: proposal already exists");

uint64 snapshot = block.number.toUint64() + votingDelay().toUint64();
uint64 deadline = snapshot + votingPeriod().toUint64();
uint256 snapshot = currentTimepoint + votingDelay();
uint256 deadline = snapshot + votingPeriod();

proposal.voteStart.setDeadline(snapshot);
proposal.voteEnd.setDeadline(deadline);
proposal.proposer = proposer;
_proposals[proposalId] = ProposalCore({
proposer: proposer,
voteStart: snapshot.toUint64(),
voteEnd: deadline.toUint64(),
executed: false,
canceled: false,
__gap_unused0: 0,
__gap_unused1: 0
});

emit ProposalCreated(
proposalId,
Expand Down Expand Up @@ -416,19 +433,19 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
/**
* @dev See {IGovernor-getVotes}.
*/
function getVotes(address account, uint256 blockNumber) public view virtual override returns (uint256) {
return _getVotes(account, blockNumber, _defaultParams());
function getVotes(address account, uint256 timepoint) public view virtual override returns (uint256) {
return _getVotes(account, timepoint, _defaultParams());
}

/**
* @dev See {IGovernor-getVotesWithParams}.
*/
function getVotesWithParams(
address account,
uint256 blockNumber,
uint256 timepoint,
bytes memory params
) public view virtual override returns (uint256) {
return _getVotes(account, blockNumber, params);
return _getVotes(account, timepoint, params);
}

/**
Expand Down Expand Up @@ -546,7 +563,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
ProposalCore storage proposal = _proposals[proposalId];
require(state(proposalId) == ProposalState.Active, "Governor: vote not currently active");

uint256 weight = _getVotes(account, proposal.voteStart.getDeadline(), params);
uint256 weight = _getVotes(account, proposal.voteStart, params);
_countVote(proposalId, account, support, weight, params);

if (params.length == 0) {
Expand Down
62 changes: 40 additions & 22 deletions contracts/governance/IGovernor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@

pragma solidity ^0.8.0;
Amxx marked this conversation as resolved.
Show resolved Hide resolved

import "../utils/introspection/ERC165.sol";
import "../interfaces/IERC165.sol";
import "../interfaces/IERC6372.sol";

/**
* @dev Interface of the {Governor} core.
*
* _Available since v4.3._
*/
abstract contract IGovernor is IERC165 {
abstract contract IGovernor is IERC165, IERC6372 {
enum ProposalState {
Pending,
Active,
Expand All @@ -32,8 +33,8 @@ abstract contract IGovernor is IERC165 {
uint256[] values,
string[] signatures,
bytes[] calldatas,
uint256 startBlock,
uint256 endBlock,
uint256 voteStart,
uint256 voteEnd,
string description
);

Expand Down Expand Up @@ -81,6 +82,19 @@ abstract contract IGovernor is IERC165 {
*/
function version() public view virtual returns (string memory);

/**
* @notice module:core
* @dev See {IERC6372}
*/
function clock() public view virtual override returns (uint48);

/**
* @notice module:core
* @dev See EIP-6372.
*/
// solhint-disable-next-line func-name-mixedcase
function CLOCK_MODE() public view virtual override returns (string memory);

/**
* @notice module:voting
* @dev A description of the possible `support` values for {castVote} and the way these votes are counted, meant to
Expand All @@ -104,7 +118,7 @@ abstract contract IGovernor is IERC165 {
* JavaScript class.
*/
// solhint-disable-next-line func-name-mixedcase
function COUNTING_MODE() public pure virtual returns (string memory);
function COUNTING_MODE() public view virtual returns (string memory);

/**
* @notice module:core
Expand All @@ -125,29 +139,33 @@ abstract contract IGovernor is IERC165 {

/**
* @notice module:core
* @dev Block number used to retrieve user's votes and quorum. As per Compound's Comp and OpenZeppelin's
* ERC20Votes, the snapshot is performed at the end of this block. Hence, voting for this proposal starts at the
* beginning of the following block.
* @dev Timepoint used to retrieve user's votes and quorum. If using block number (as per Compound's Comp), the
* snapshot is performed at the end of this block. Hence, voting for this proposal starts at the beginning of the
* following block.
*/
function proposalSnapshot(uint256 proposalId) public view virtual returns (uint256);

/**
* @notice module:core
* @dev Block number at which votes close. Votes close at the end of this block, so it is possible to cast a vote
* during this block.
* @dev Timepoint at which votes close. If using block number, votes close at the end of this block, so it is
* possible to cast a vote during this block.
*/
function proposalDeadline(uint256 proposalId) public view virtual returns (uint256);

/**
* @notice module:user-config
* @dev Delay, in number of block, between the proposal is created and the vote starts. This can be increased to
* leave time for users to buy voting power, or delegate it, before the voting of a proposal starts.
* @dev Delay, between the proposal is created and the vote starts. The unit this duration is expressed in depends
* on the clock (see EIP-6372) this contract uses.
*
* This can be increased to leave time for users to buy voting power, or delegate it, before the voting of a
* proposal starts.
*/
function votingDelay() public view virtual returns (uint256);

/**
* @notice module:user-config
* @dev Delay, in number of blocks, between the vote start and vote ends.
* @dev Delay, between the vote start and vote ends. The unit this duration is expressed in depends on the clock
* (see EIP-6372) this contract uses.
*
* NOTE: The {votingDelay} can delay the start of the vote. This must be considered when setting the voting
* duration compared to the voting delay.
Expand All @@ -158,27 +176,27 @@ abstract contract IGovernor is IERC165 {
* @notice module:user-config
* @dev Minimum number of cast voted required for a proposal to be successful.
*
* Note: The `blockNumber` parameter corresponds to the snapshot used for counting vote. This allows to scale the
* quorum depending on values such as the totalSupply of a token at this block (see {ERC20Votes}).
* NOTE: The `timepoint` parameter corresponds to the snapshot used for counting vote. This allows to scale the
* quorum depending on values such as the totalSupply of a token at this timepoint (see {ERC20Votes}).
*/
function quorum(uint256 blockNumber) public view virtual returns (uint256);
function quorum(uint256 timepoint) public view virtual returns (uint256);

/**
* @notice module:reputation
* @dev Voting power of an `account` at a specific `blockNumber`.
* @dev Voting power of an `account` at a specific `timepoint`.
*
* Note: this can be implemented in a number of ways, for example by reading the delegated balance from one (or
* multiple), {ERC20Votes} tokens.
*/
function getVotes(address account, uint256 blockNumber) public view virtual returns (uint256);
function getVotes(address account, uint256 timepoint) public view virtual returns (uint256);

/**
* @notice module:reputation
* @dev Voting power of an `account` at a specific `blockNumber` given additional encoded parameters.
* @dev Voting power of an `account` at a specific `timepoint` given additional encoded parameters.
*/
function getVotesWithParams(
address account,
uint256 blockNumber,
uint256 timepoint,
bytes memory params
) public view virtual returns (uint256);

Expand All @@ -189,8 +207,8 @@ abstract contract IGovernor is IERC165 {
function hasVoted(uint256 proposalId, address account) public view virtual returns (bool);

/**
* @dev Create a new proposal. Vote start {IGovernor-votingDelay} blocks after the proposal is created and ends
* {IGovernor-votingPeriod} blocks after the voting starts.
* @dev Create a new proposal. Vote start after a delay specified by {IGovernor-votingDelay} and lasts for a
* duration specified by {IGovernor-votingPeriod}.
*
* Emits a {ProposalCreated} event.
*/
Expand Down
6 changes: 3 additions & 3 deletions contracts/governance/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ Other extensions can customize the behavior or interface in multiple ways.

In addition to modules and extensions, the core contract requires a few virtual functions to be implemented to your particular specifications:

* <<Governor-votingDelay-,`votingDelay()`>>: Delay (in number of blocks) since the proposal is submitted until voting power is fixed and voting starts. This can be used to enforce a delay after a proposal is published for users to buy tokens, or delegate their votes.
* <<Governor-votingPeriod-,`votingPeriod()`>>: Delay (in number of blocks) since the proposal starts until voting ends.
* <<Governor-quorum-uint256-,`quorum(uint256 blockNumber)`>>: Quorum required for a proposal to be successful. This function includes a `blockNumber` argument so the quorum can adapt through time, for example, to follow a token's `totalSupply`.
* <<Governor-votingDelay-,`votingDelay()`>>: Delay (in EIP-6372 clock) since the proposal is submitted until voting power is fixed and voting starts. This can be used to enforce a delay after a proposal is published for users to buy tokens, or delegate their votes.
* <<Governor-votingPeriod-,`votingPeriod()`>>: Delay (in EIP-6372 clock) since the proposal starts until voting ends.
* <<Governor-quorum-uint256-,`quorum(uint256 timepoint)`>>: Quorum required for a proposal to be successful. This function includes a `timepoint` argument (see EIP-6372) so the quorum can adapt through time, for example, to follow a token's `totalSupply`.

NOTE: Functions of the `Governor` contract do not include access control. If you want to restrict access, you should add these checks by overloading the particular functions. Among these, {Governor-_cancel} is internal by default, and you will have to expose it (with the right access control mechanism) yourself if this function is needed.

Expand Down