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 Governor module for governance-settable parameters #2904

Merged
merged 26 commits into from Oct 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
562a9f5
GovernorSettings
Amxx Oct 6, 2021
85f8ca5
add PR ref to changelog
Amxx Oct 6, 2021
80be171
testing
Amxx Oct 6, 2021
7f5f597
fix inheritance order
Amxx Oct 6, 2021
62089aa
Merge branch 'master' into feature/governance-settings
Amxx Oct 6, 2021
d85ecc6
merge GovernorProposalThreshold into the core Governor contract
Amxx Oct 6, 2021
7ac8508
Merge branch 'master' into feature/governance-settings
Amxx Oct 7, 2021
4616494
Merge branch 'master' into feature/governance-settings
Amxx Oct 11, 2021
b00ec17
add support for previous interfaceId
Amxx Oct 13, 2021
46f743d
fix lint
Amxx Oct 13, 2021
30de83a
rename interfaceId in tests
frangio Oct 13, 2021
5660033
enshure governor built using the wizard can are still supported
Amxx Oct 13, 2021
2d85c55
add wizard produced mocks
Amxx Oct 13, 2021
aac4ecf
move proposalThreshold out of IGovernor and into Governor
Amxx Oct 13, 2021
8b8238b
reorder mocks inheritance to pass inheritance ordering tests
Amxx Oct 13, 2021
4b54776
improve coverage
Amxx Oct 13, 2021
efafb34
improve coverage
Amxx Oct 13, 2021
c5a9fd0
fix lint
Amxx Oct 13, 2021
cc122eb
improve coverage
Amxx Oct 13, 2021
07d9724
Apply suggestions from code review
Amxx Oct 14, 2021
9a80138
improve doc
Amxx Oct 14, 2021
734451b
fix test
Amxx Oct 14, 2021
7220c81
remove deprecated contract from top level index
frangio Oct 14, 2021
095086d
simplify events
frangio Oct 15, 2021
a3ae78b
add documentation to GovernorSettings
Amxx Oct 19, 2021
e10edf1
Merge remote-tracking branch 'Amxx/feature/governance-settings' into …
Amxx Oct 19, 2021
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -8,6 +8,7 @@
* `EIP712`: cache `address(this)` to immutable storage to avoid potential issues if a vanilla contract is used in a delegatecall context. ([#2852](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2852))
* Add internal `_setApprovalForAll` to `ERC721` and `ERC1155`. ([#2834](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2834))
* `Governor`: shift vote start and end by one block to better match Compound's GovernorBravo and prevent voting at the Governor level if the voting snapshot is not ready. ([#2892](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2892))
* `GovernorSettings`: a new governor module that manages voting settings updatable through governance actions. ([#2904](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2904))
* `PaymentSplitter`: now supports ERC20 assets in addition to Ether. ([#2858](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2858))

## 4.3.2 (2021-09-14)
Expand Down
12 changes: 12 additions & 0 deletions contracts/governance/Governor.sol
Expand Up @@ -143,6 +143,13 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
return _proposals[proposalId].voteEnd.getDeadline();
}

/**
* @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 Amount of votes already cast passes the threshold limit.
*/
Expand Down Expand Up @@ -174,6 +181,11 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
bytes[] memory calldatas,
string memory description
) public virtual override returns (uint256) {
require(
getVotes(msg.sender, block.number - 1) >= proposalThreshold(),
"GovernorCompatibilityBravo: proposer votes below proposal threshold"
);

uint256 proposalId = hashProposal(targets, values, calldatas, keccak256(bytes(description)));

require(targets.length == values.length, "Governor: invalid proposal length");
Expand Down
8 changes: 6 additions & 2 deletions contracts/governance/README.adoc
Expand Up @@ -40,7 +40,7 @@ Other extensions can customize the behavior or interface in multiple ways.

* {GovernorCompatibilityBravo}: Extends the interface to be fully `GovernorBravo`-compatible. Note that events are compatible regardless of whether this extension is included or not.

* {GovernorProposalThreshold}: Restricts proposals to delegates with a minimum voting power.
* {GovernorSettings}: Manages some of the settings (voting delay, voting period duration, and proposal threshold) in a way that can be updated through a governance proposal, without requiering an upgrade.

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

Expand Down Expand Up @@ -72,10 +72,14 @@ NOTE: Functions of the `Governor` contract do not include access control. If you

{{GovernorTimelockCompound}}

{{GovernorProposalThreshold}}
Amxx marked this conversation as resolved.
Show resolved Hide resolved
{{GovernorSettings}}

{{GovernorCompatibilityBravo}}

=== Deprecated

{{GovernorProposalThreshold}}

== Timelock

In a governance system, the {TimelockController} contract is in carge of introducing a delay between a proposal and its execution. It can be used with or without a {Governor}.
Expand Down
20 changes: 2 additions & 18 deletions contracts/governance/compatibility/GovernorCompatibilityBravo.sol
Expand Up @@ -5,7 +5,6 @@ pragma solidity ^0.8.0;
import "../../utils/Counters.sol";
import "../../utils/math/SafeCast.sol";
import "../extensions/IGovernorTimelock.sol";
import "../extensions/GovernorProposalThreshold.sol";
import "../Governor.sol";
import "./IGovernorCompatibilityBravo.sol";

Expand All @@ -19,12 +18,7 @@ import "./IGovernorCompatibilityBravo.sol";
*
* _Available since v4.3._
*/
abstract contract GovernorCompatibilityBravo is
IGovernorTimelock,
IGovernorCompatibilityBravo,
Governor,
GovernorProposalThreshold
{
abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorCompatibilityBravo, Governor {
using Counters for Counters.Counter;
using Timers for Timers.BlockNumber;

Expand Down Expand Up @@ -63,7 +57,7 @@ abstract contract GovernorCompatibilityBravo is
uint256[] memory values,
bytes[] memory calldatas,
string memory description
) public virtual override(IGovernor, Governor, GovernorProposalThreshold) returns (uint256) {
) public virtual override(IGovernor, Governor) returns (uint256) {
_storeProposal(_msgSender(), targets, values, new string[](calldatas.length), calldatas, description);
return super.propose(targets, values, calldatas, description);
}
Expand Down Expand Up @@ -169,16 +163,6 @@ abstract contract GovernorCompatibilityBravo is
}

// ==================================================== Views =====================================================
/**
* @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
override(IGovernorCompatibilityBravo, GovernorProposalThreshold)
returns (uint256);

/**
* @dev See {IGovernorCompatibilityBravo-proposals}.
*/
Expand Down
Expand Up @@ -110,9 +110,4 @@ abstract contract IGovernorCompatibilityBravo is IGovernor {
* @dev Part of the Governor Bravo's interface: _"Gets the receipt for a voter on a given proposal"_.
*/
function getReceipt(uint256 proposalId, address voter) public view virtual returns (Receipt memory);

/**
* @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);
}
14 changes: 1 addition & 13 deletions contracts/governance/extensions/GovernorProposalThreshold.sol
Expand Up @@ -8,27 +8,15 @@ import "../Governor.sol";
* @dev Extension of {Governor} for proposal restriction to token holders with a minimum balance.
*
* _Available since v4.3._
Amxx marked this conversation as resolved.
Show resolved Hide resolved
* _Deprecated since v4.4._
*/
abstract contract GovernorProposalThreshold is Governor {
/**
* @dev See {IGovernor-propose}.
*/
function propose(
address[] memory targets,
uint256[] memory values,
bytes[] memory calldatas,
string memory description
) public virtual override returns (uint256) {
require(
getVotes(msg.sender, block.number - 1) >= proposalThreshold(),
"GovernorCompatibilityBravo: proposer votes below proposal threshold"
);

return super.propose(targets, values, calldatas, description);
}

/**
* @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);
}
113 changes: 113 additions & 0 deletions contracts/governance/extensions/GovernorSettings.sol
@@ -0,0 +1,113 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../Governor.sol";

/**
* @dev Extension of {Governor} for settings updatable through governance.
*
* _Available since v4.4._
*/
abstract contract GovernorSettings is Governor {
uint256 private _votingDelay;
uint256 private _votingPeriod;
uint256 private _proposalThreshold;

event VotingDelaySet(uint256 oldVotingDelay, uint256 newVotingDelay);
event VotingPeriodSet(uint256 oldVotingPeriod, uint256 newVotingPeriod);
event ProposalThresholdSet(uint256 oldProposalThreshold, uint256 newProposalThreshold);

/**
* @dev Initialize the governance parameters.
*/
constructor(
uint256 initialVotingDelay,
uint256 initialVotingPeriod,
uint256 initialProposalThreshold
) {
_setVotingDelay(initialVotingDelay);
_setVotingPeriod(initialVotingPeriod);
_setProposalThreshold(initialProposalThreshold);
}

/**
* @dev See {IGovernor-votingDelay}.
*/
function votingDelay() public view virtual override returns (uint256) {
return _votingDelay;
}

/**
* @dev See {IGovernor-votingPeriod}.
*/
function votingPeriod() public view virtual override returns (uint256) {
return _votingPeriod;
}

/**
* @dev See {Governor-proposalThreshold}.
*/
function proposalThreshold() public view virtual override returns (uint256) {
return _proposalThreshold;
}

/**
* @dev Update the voting delay. This operation can only be performed through a governance proposal.
*
* Emits a {VotingDelaySet} event.
*/
function setVotingDelay(uint256 newVotingDelay) public onlyGovernance {
_setVotingDelay(newVotingDelay);
}

/**
* @dev Update the voting period. This operation can only be performed through a governance proposal.
*
* Emits a {VotingPeriodSet} event.
*/
function setVotingPeriod(uint256 newVotingPeriod) public onlyGovernance {
_setVotingPeriod(newVotingPeriod);
}

/**
* @dev Update the proposal threshold. This operation can only be performed through a governance proposal.
*
* Emits a {ProposalThresholdSet} event.
*/
function setProposalThreshold(uint256 newProposalThreshold) public onlyGovernance {
_setProposalThreshold(newProposalThreshold);
}

/**
* @dev Internal setter for the the voting delay.
*
* Emits a {VotingDelaySet} event.
*/
function _setVotingDelay(uint256 newVotingDelay) internal virtual {
emit VotingDelaySet(_votingDelay, newVotingDelay);
_votingDelay = newVotingDelay;
}

/**
* @dev Internal setter for the the voting period.
*
* Emits a {VotingPeriodSet} event.
*/
function _setVotingPeriod(uint256 newVotingPeriod) internal virtual {
// voting period must be at least one block long
require(newVotingPeriod > 0, "GovernorSettings: voting period too low");
emit VotingPeriodSet(_votingPeriod, newVotingPeriod);
_votingPeriod = newVotingPeriod;
}

/**
* @dev Internal setter for the the proposal threshold.
*
* Emits a {ProposalThresholdSet} event.
*/
function _setProposalThreshold(uint256 newProposalThreshold) internal virtual {
emit ProposalThresholdSet(_proposalThreshold, newProposalThreshold);
_proposalThreshold = newProposalThreshold;
}
}
28 changes: 8 additions & 20 deletions contracts/mocks/GovernorCompMock.sol
Expand Up @@ -2,34 +2,22 @@

pragma solidity ^0.8.0;

import "../governance/Governor.sol";
import "../governance/extensions/GovernorCountingSimple.sol";
import "../governance/extensions/GovernorVotesComp.sol";

contract GovernorCompMock is Governor, GovernorVotesComp, GovernorCountingSimple {
uint256 immutable _votingDelay;
uint256 immutable _votingPeriod;

constructor(
string memory name_,
ERC20VotesComp token_,
uint256 votingDelay_,
uint256 votingPeriod_
) Governor(name_) GovernorVotesComp(token_) {
_votingDelay = votingDelay_;
_votingPeriod = votingPeriod_;
}
contract GovernorCompMock is GovernorVotesComp, GovernorCountingSimple {
constructor(string memory name_, ERC20VotesComp token_) Governor(name_) GovernorVotesComp(token_) {}

function votingDelay() public view override returns (uint256) {
return _votingDelay;
function quorum(uint256) public pure override returns (uint256) {
return 0;
}

function votingPeriod() public view override returns (uint256) {
return _votingPeriod;
function votingDelay() public pure override returns (uint256) {
return 4;
}

function quorum(uint256) public pure override returns (uint256) {
return 0;
function votingPeriod() public pure override returns (uint256) {
return 16;
}

function cancel(
Expand Down
41 changes: 18 additions & 23 deletions contracts/mocks/GovernorCompatibilityBravoMock.sol
Expand Up @@ -3,26 +3,29 @@
pragma solidity ^0.8.0;

import "../governance/compatibility/GovernorCompatibilityBravo.sol";
import "../governance/extensions/GovernorVotesComp.sol";
import "../governance/extensions/GovernorTimelockCompound.sol";
import "../governance/extensions/GovernorSettings.sol";
import "../governance/extensions/GovernorVotesComp.sol";

contract GovernorCompatibilityBravoMock is GovernorCompatibilityBravo, GovernorTimelockCompound, GovernorVotesComp {
uint256 immutable _votingDelay;
uint256 immutable _votingPeriod;
uint256 immutable _proposalThreshold;

contract GovernorCompatibilityBravoMock is
GovernorCompatibilityBravo,
GovernorSettings,
GovernorTimelockCompound,
GovernorVotesComp
{
constructor(
string memory name_,
ERC20VotesComp token_,
uint256 votingDelay_,
uint256 votingPeriod_,
uint256 proposalThreshold_,
ICompoundTimelock timelock_
) Governor(name_) GovernorVotesComp(token_) GovernorTimelockCompound(timelock_) {
_votingDelay = votingDelay_;
_votingPeriod = votingPeriod_;
_proposalThreshold = proposalThreshold_;
}
)
Governor(name_)
GovernorTimelockCompound(timelock_)
GovernorSettings(votingDelay_, votingPeriod_, proposalThreshold_)
GovernorVotesComp(token_)
{}

function supportsInterface(bytes4 interfaceId)
public
Expand All @@ -34,18 +37,6 @@ contract GovernorCompatibilityBravoMock is GovernorCompatibilityBravo, GovernorT
return super.supportsInterface(interfaceId);
}

function votingDelay() public view override returns (uint256) {
return _votingDelay;
}

function votingPeriod() public view override returns (uint256) {
return _votingPeriod;
}

function proposalThreshold() public view virtual override returns (uint256) {
return _proposalThreshold;
}

function quorum(uint256) public pure override returns (uint256) {
return 0;
}
Expand All @@ -70,6 +61,10 @@ contract GovernorCompatibilityBravoMock is GovernorCompatibilityBravo, GovernorT
return super.proposalEta(proposalId);
}

function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) {
return super.proposalThreshold();
}

function propose(
address[] memory targets,
uint256[] memory values,
Expand Down