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 25 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 34 additions & 30 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,27 @@ 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)");

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;
}

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 @@ -97,10 +101,13 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
interfaceId ==
(type(IGovernor).interfaceId ^
this.cancel.selector ^
this.clock.selector ^
this.CLOCK_MODE.selector ^
Amxx marked this conversation as resolved.
Show resolved Hide resolved
this.castVoteWithReasonAndParams.selector ^
this.castVoteWithReasonAndParamsBySig.selector ^
this.getVotesWithParams.selector) ||
interfaceId == (type(IGovernor).interfaceId ^ this.cancel.selector) ||
interfaceId ==
(type(IGovernor).interfaceId ^ this.cancel.selector ^ this.clock.selector ^ this.CLOCK_MODE.selector) ||
Amxx marked this conversation as resolved.
Show resolved Hide resolved
interfaceId == type(IGovernor).interfaceId ||
frangio marked this conversation as resolved.
Show resolved Hide resolved
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 timepoint = clock();
Amxx marked this conversation as resolved.
Show resolved Hide resolved

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

uint256 deadline = proposalDeadline(proposalId);

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

Expand All @@ -183,14 +192,14 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
* @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;
}

/**
Expand All @@ -211,13 +220,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 +257,10 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive
string memory description
) public virtual override returns (uint256) {
address proposer = _msgSender();
uint256 timepoint = clock();
Amxx marked this conversation as resolved.
Show resolved Hide resolved

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

Expand All @@ -263,16 +269,14 @@ 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 = timepoint + votingDelay();
uint256 deadline = snapshot + votingPeriod();

proposal.voteStart.setDeadline(snapshot);
proposal.voteEnd.setDeadline(deadline);
proposal.proposer = proposer;
_proposals[proposalId].proposer = proposer;
_proposals[proposalId].voteStart = snapshot;
_proposals[proposalId].voteEnd = deadline;

emit ProposalCreated(
proposalId,
Expand Down Expand Up @@ -416,19 +420,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 +550,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
32 changes: 23 additions & 9 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 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 EIP-6372.
Amxx marked this conversation as resolved.
Show resolved Hide resolved
*/
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 Down Expand Up @@ -158,27 +172,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
* Note: The `timepoint` parameter corresponds to the snapshot used for counting vote. This allows to scale the
Amxx marked this conversation as resolved.
Show resolved Hide resolved
* quorum depending on values such as the totalSupply of a token at this block (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 Down
2 changes: 1 addition & 1 deletion contracts/governance/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ In addition to modules and extensions, the core contract requires a few virtual

* <<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.
Amxx marked this conversation as resolved.
Show resolved Hide resolved
* <<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-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
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
ProposalDetails storage details = _proposalDetails[proposalId];

require(
_msgSender() == details.proposer || getVotes(details.proposer, block.number - 1) < proposalThreshold(),
_msgSender() == details.proposer || getVotes(details.proposer, clock() - 1) < proposalThreshold(),
ernestognw marked this conversation as resolved.
Show resolved Hide resolved
"GovernorBravo: proposer above threshold"
);

Expand Down Expand Up @@ -225,7 +225,7 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
* @dev See {IGovernorCompatibilityBravo-quorumVotes}.
*/
function quorumVotes() public view virtual override returns (uint256) {
return quorum(block.number - 1);
return quorum(clock() - 1);
}

// ==================================================== Voting ====================================================
Expand Down
19 changes: 9 additions & 10 deletions contracts/governance/extensions/GovernorPreventLateQuorum.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ import "../../utils/math/Math.sol";
*/
abstract contract GovernorPreventLateQuorum is Governor {
using SafeCast for uint256;
using Timers for Timers.BlockNumber;

uint64 private _voteExtension;
mapping(uint256 => Timers.BlockNumber) private _extendedDeadlines;

/// @custom:oz-retyped-from mapping(uint256 => Timers.BlockNumber)
mapping(uint256 => uint64) private _extendedDeadlines;

/// @dev Emitted when a proposal deadline is pushed back due to reaching quorum late in its voting period.
event ProposalExtended(uint256 indexed proposalId, uint64 extendedDeadline);
Expand All @@ -44,7 +45,7 @@ abstract contract GovernorPreventLateQuorum is Governor {
* proposal reached quorum late in the voting period. See {Governor-proposalDeadline}.
*/
function proposalDeadline(uint256 proposalId) public view virtual override returns (uint256) {
return Math.max(super.proposalDeadline(proposalId), _extendedDeadlines[proposalId].getDeadline());
return Math.max(super.proposalDeadline(proposalId), _extendedDeadlines[proposalId]);
}

/**
Expand All @@ -62,16 +63,14 @@ abstract contract GovernorPreventLateQuorum is Governor {
) internal virtual override returns (uint256) {
uint256 result = super._castVote(proposalId, account, support, reason, params);

Timers.BlockNumber storage extendedDeadline = _extendedDeadlines[proposalId];

if (extendedDeadline.isUnset() && _quorumReached(proposalId)) {
uint64 extendedDeadlineValue = block.number.toUint64() + lateQuorumVoteExtension();
if (_extendedDeadlines[proposalId] == 0 && _quorumReached(proposalId)) {
uint64 extendedDeadline = clock() + lateQuorumVoteExtension();

if (extendedDeadlineValue > proposalDeadline(proposalId)) {
emit ProposalExtended(proposalId, extendedDeadlineValue);
if (extendedDeadline > proposalDeadline(proposalId)) {
emit ProposalExtended(proposalId, extendedDeadline);
}

extendedDeadline.setDeadline(extendedDeadlineValue);
_extendedDeadlines[proposalId] = extendedDeadline;
}

return result;
Expand Down
15 changes: 6 additions & 9 deletions contracts/governance/extensions/GovernorTimelockCompound.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,11 @@ import "../../vendor/compound/ICompoundTimelock.sol";
*/
abstract contract GovernorTimelockCompound is IGovernorTimelock, Governor {
using SafeCast for uint256;
using Timers for Timers.Timestamp;

struct ProposalTimelock {
Timers.Timestamp timer;
}

ICompoundTimelock private _timelock;

mapping(uint256 => ProposalTimelock) private _proposalTimelocks;
/// @custom:oz-retyped-from mapping(uint256 => Timers.Timestamp)
Amxx marked this conversation as resolved.
Show resolved Hide resolved
mapping(uint256 => uint64) private _proposalTimelocks;

/**
* @dev Emitted when the timelock controller used for proposal execution is modified.
Expand Down Expand Up @@ -82,7 +78,7 @@ abstract contract GovernorTimelockCompound is IGovernorTimelock, Governor {
* @dev Public accessor to check the eta of a queued proposal
*/
function proposalEta(uint256 proposalId) public view virtual override returns (uint256) {
return _proposalTimelocks[proposalId].timer.getDeadline();
return _proposalTimelocks[proposalId];
}

/**
Expand All @@ -99,7 +95,8 @@ abstract contract GovernorTimelockCompound is IGovernorTimelock, Governor {
require(state(proposalId) == ProposalState.Succeeded, "Governor: proposal not successful");

uint256 eta = block.timestamp + _timelock.delay();
_proposalTimelocks[proposalId].timer.setDeadline(eta.toUint64());
_proposalTimelocks[proposalId] = eta;

for (uint256 i = 0; i < targets.length; ++i) {
require(
!_timelock.queuedTransactions(keccak256(abi.encode(targets[i], values[i], "", calldatas[i], eta))),
Expand Down Expand Up @@ -148,7 +145,7 @@ abstract contract GovernorTimelockCompound is IGovernorTimelock, Governor {
for (uint256 i = 0; i < targets.length; ++i) {
_timelock.cancelTransaction(targets[i], values[i], "", calldatas[i], eta);
}
_proposalTimelocks[proposalId].timer.reset();
delete _proposalTimelocks[proposalId];
}

return proposalId;
Expand Down
Loading