diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fb03e735fa..341320417a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index f1411ffa220..a6e5eddd02e 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -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. */ @@ -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"); diff --git a/contracts/governance/README.adoc b/contracts/governance/README.adoc index 4fd478153aa..d198c9f9315 100644 --- a/contracts/governance/README.adoc +++ b/contracts/governance/README.adoc @@ -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: @@ -72,10 +72,14 @@ NOTE: Functions of the `Governor` contract do not include access control. If you {{GovernorTimelockCompound}} -{{GovernorProposalThreshold}} +{{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}. diff --git a/contracts/governance/compatibility/GovernorCompatibilityBravo.sol b/contracts/governance/compatibility/GovernorCompatibilityBravo.sol index cbdd72791b7..87874c2f6e9 100644 --- a/contracts/governance/compatibility/GovernorCompatibilityBravo.sol +++ b/contracts/governance/compatibility/GovernorCompatibilityBravo.sol @@ -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"; @@ -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; @@ -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); } @@ -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}. */ diff --git a/contracts/governance/compatibility/IGovernorCompatibilityBravo.sol b/contracts/governance/compatibility/IGovernorCompatibilityBravo.sol index 4f4229d3de5..5d3632bdeff 100644 --- a/contracts/governance/compatibility/IGovernorCompatibilityBravo.sol +++ b/contracts/governance/compatibility/IGovernorCompatibilityBravo.sol @@ -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); } diff --git a/contracts/governance/extensions/GovernorProposalThreshold.sol b/contracts/governance/extensions/GovernorProposalThreshold.sol index 6bf4adfd128..60edc870356 100644 --- a/contracts/governance/extensions/GovernorProposalThreshold.sol +++ b/contracts/governance/extensions/GovernorProposalThreshold.sol @@ -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._ + * _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); } diff --git a/contracts/governance/extensions/GovernorSettings.sol b/contracts/governance/extensions/GovernorSettings.sol new file mode 100644 index 00000000000..2a3719ee458 --- /dev/null +++ b/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; + } +} diff --git a/contracts/mocks/GovernorCompMock.sol b/contracts/mocks/GovernorCompMock.sol index 299a90ac7cb..9dcbc536d0e 100644 --- a/contracts/mocks/GovernorCompMock.sol +++ b/contracts/mocks/GovernorCompMock.sol @@ -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( diff --git a/contracts/mocks/GovernorCompatibilityBravoMock.sol b/contracts/mocks/GovernorCompatibilityBravoMock.sol index 061e51e9145..60afbb918e9 100644 --- a/contracts/mocks/GovernorCompatibilityBravoMock.sol +++ b/contracts/mocks/GovernorCompatibilityBravoMock.sol @@ -3,14 +3,16 @@ 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_, @@ -18,11 +20,12 @@ contract GovernorCompatibilityBravoMock is GovernorCompatibilityBravo, GovernorT 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 @@ -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; } @@ -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, diff --git a/contracts/mocks/GovernorMock.sol b/contracts/mocks/GovernorMock.sol index 362ce7bc495..cc96dcd2775 100644 --- a/contracts/mocks/GovernorMock.sol +++ b/contracts/mocks/GovernorMock.sol @@ -2,32 +2,29 @@ pragma solidity ^0.8.0; -import "../governance/Governor.sol"; +import "../governance/extensions/GovernorProposalThreshold.sol"; +import "../governance/extensions/GovernorSettings.sol"; import "../governance/extensions/GovernorCountingSimple.sol"; import "../governance/extensions/GovernorVotesQuorumFraction.sol"; -contract GovernorMock is Governor, GovernorVotesQuorumFraction, GovernorCountingSimple { - uint256 immutable _votingDelay; - uint256 immutable _votingPeriod; - +contract GovernorMock is + GovernorProposalThreshold, + GovernorSettings, + GovernorVotesQuorumFraction, + GovernorCountingSimple +{ constructor( string memory name_, ERC20Votes token_, uint256 votingDelay_, uint256 votingPeriod_, uint256 quorumNumerator_ - ) Governor(name_) GovernorVotes(token_) GovernorVotesQuorumFraction(quorumNumerator_) { - _votingDelay = votingDelay_; - _votingPeriod = votingPeriod_; - } - - function votingDelay() public view override returns (uint256) { - return _votingDelay; - } - - function votingPeriod() public view override returns (uint256) { - return _votingPeriod; - } + ) + Governor(name_) + GovernorSettings(votingDelay_, votingPeriod_, 0) + GovernorVotes(token_) + GovernorVotesQuorumFraction(quorumNumerator_) + {} function cancel( address[] memory targets, @@ -47,4 +44,17 @@ contract GovernorMock is Governor, GovernorVotesQuorumFraction, GovernorCounting { return super.getVotes(account, blockNumber); } + + function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) { + return super.proposalThreshold(); + } + + function propose( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + string memory description + ) public virtual override(Governor, GovernorProposalThreshold) returns (uint256) { + return super.propose(targets, values, calldatas, description); + } } diff --git a/contracts/mocks/GovernorTimelockCompoundMock.sol b/contracts/mocks/GovernorTimelockCompoundMock.sol index b3af0782f66..848f4b409b3 100644 --- a/contracts/mocks/GovernorTimelockCompoundMock.sol +++ b/contracts/mocks/GovernorTimelockCompoundMock.sol @@ -3,13 +3,16 @@ pragma solidity ^0.8.0; import "../governance/extensions/GovernorTimelockCompound.sol"; +import "../governance/extensions/GovernorSettings.sol"; import "../governance/extensions/GovernorCountingSimple.sol"; import "../governance/extensions/GovernorVotesQuorumFraction.sol"; -contract GovernorTimelockCompoundMock is GovernorTimelockCompound, GovernorVotesQuorumFraction, GovernorCountingSimple { - uint256 immutable _votingDelay; - uint256 immutable _votingPeriod; - +contract GovernorTimelockCompoundMock is + GovernorSettings, + GovernorTimelockCompound, + GovernorVotesQuorumFraction, + GovernorCountingSimple +{ constructor( string memory name_, ERC20Votes token_, @@ -20,12 +23,10 @@ contract GovernorTimelockCompoundMock is GovernorTimelockCompound, GovernorVotes ) Governor(name_) GovernorTimelockCompound(timelock_) + GovernorSettings(votingDelay_, votingPeriod_, 0) GovernorVotes(token_) GovernorVotesQuorumFraction(quorumNumerator_) - { - _votingDelay = votingDelay_; - _votingPeriod = votingPeriod_; - } + {} function supportsInterface(bytes4 interfaceId) public @@ -37,14 +38,6 @@ contract GovernorTimelockCompoundMock is GovernorTimelockCompound, GovernorVotes return super.supportsInterface(interfaceId); } - function votingDelay() public view override returns (uint256) { - return _votingDelay; - } - - function votingPeriod() public view override returns (uint256) { - return _votingPeriod; - } - function quorum(uint256 blockNumber) public view @@ -76,6 +69,10 @@ contract GovernorTimelockCompoundMock is GovernorTimelockCompound, GovernorVotes return super.state(proposalId); } + function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) { + return super.proposalThreshold(); + } + function _execute( uint256 proposalId, address[] memory targets, diff --git a/contracts/mocks/GovernorTimelockControlMock.sol b/contracts/mocks/GovernorTimelockControlMock.sol index d9a19ee31b5..4d9e97fd522 100644 --- a/contracts/mocks/GovernorTimelockControlMock.sol +++ b/contracts/mocks/GovernorTimelockControlMock.sol @@ -3,13 +3,16 @@ pragma solidity ^0.8.0; import "../governance/extensions/GovernorTimelockControl.sol"; +import "../governance/extensions/GovernorSettings.sol"; import "../governance/extensions/GovernorCountingSimple.sol"; import "../governance/extensions/GovernorVotesQuorumFraction.sol"; -contract GovernorTimelockControlMock is GovernorTimelockControl, GovernorVotesQuorumFraction, GovernorCountingSimple { - uint256 immutable _votingDelay; - uint256 immutable _votingPeriod; - +contract GovernorTimelockControlMock is + GovernorSettings, + GovernorTimelockControl, + GovernorVotesQuorumFraction, + GovernorCountingSimple +{ constructor( string memory name_, ERC20Votes token_, @@ -20,12 +23,10 @@ contract GovernorTimelockControlMock is GovernorTimelockControl, GovernorVotesQu ) Governor(name_) GovernorTimelockControl(timelock_) + GovernorSettings(votingDelay_, votingPeriod_, 0) GovernorVotes(token_) GovernorVotesQuorumFraction(quorumNumerator_) - { - _votingDelay = votingDelay_; - _votingPeriod = votingPeriod_; - } + {} function supportsInterface(bytes4 interfaceId) public @@ -37,14 +38,6 @@ contract GovernorTimelockControlMock is GovernorTimelockControl, GovernorVotesQu return super.supportsInterface(interfaceId); } - function votingDelay() public view override returns (uint256) { - return _votingDelay; - } - - function votingPeriod() public view override returns (uint256) { - return _votingPeriod; - } - function quorum(uint256 blockNumber) public view @@ -76,6 +69,10 @@ contract GovernorTimelockControlMock is GovernorTimelockControl, GovernorVotesQu return super.state(proposalId); } + function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) { + return super.proposalThreshold(); + } + function _execute( uint256 proposalId, address[] memory targets, diff --git a/contracts/mocks/wizard/MyGovernor1.sol b/contracts/mocks/wizard/MyGovernor1.sol new file mode 100644 index 00000000000..72b486aa782 --- /dev/null +++ b/contracts/mocks/wizard/MyGovernor1.sol @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.2; + +import "../../governance/Governor.sol"; +import "../../governance/extensions/GovernorCountingSimple.sol"; +import "../../governance/extensions/GovernorVotes.sol"; +import "../../governance/extensions/GovernorVotesQuorumFraction.sol"; +import "../../governance/extensions/GovernorTimelockControl.sol"; + +contract MyGovernor1 is + Governor, + GovernorTimelockControl, + GovernorVotes, + GovernorVotesQuorumFraction, + GovernorCountingSimple +{ + constructor(ERC20Votes _token, TimelockController _timelock) + Governor("MyGovernor") + GovernorVotes(_token) + GovernorVotesQuorumFraction(4) + GovernorTimelockControl(_timelock) + {} + + function votingDelay() public pure override returns (uint256) { + return 1; // 1 block + } + + function votingPeriod() public pure override returns (uint256) { + return 45818; // 1 week + } + + // The following functions are overrides required by Solidity. + + function quorum(uint256 blockNumber) + public + view + override(IGovernor, GovernorVotesQuorumFraction) + returns (uint256) + { + return super.quorum(blockNumber); + } + + function getVotes(address account, uint256 blockNumber) + public + view + override(IGovernor, GovernorVotes) + returns (uint256) + { + return super.getVotes(account, blockNumber); + } + + function state(uint256 proposalId) public view override(Governor, GovernorTimelockControl) returns (ProposalState) { + return super.state(proposalId); + } + + function propose( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + string memory description + ) public override(Governor, IGovernor) returns (uint256) { + return super.propose(targets, values, calldatas, description); + } + + function _execute( + uint256 proposalId, + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) internal override(Governor, GovernorTimelockControl) { + super._execute(proposalId, targets, values, calldatas, descriptionHash); + } + + function _cancel( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) internal override(Governor, GovernorTimelockControl) returns (uint256) { + return super._cancel(targets, values, calldatas, descriptionHash); + } + + function _executor() internal view override(Governor, GovernorTimelockControl) returns (address) { + return super._executor(); + } + + function supportsInterface(bytes4 interfaceId) + public + view + override(Governor, GovernorTimelockControl) + returns (bool) + { + return super.supportsInterface(interfaceId); + } +} diff --git a/contracts/mocks/wizard/MyGovernor2.sol b/contracts/mocks/wizard/MyGovernor2.sol new file mode 100644 index 00000000000..3f25b91bfe8 --- /dev/null +++ b/contracts/mocks/wizard/MyGovernor2.sol @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.2; + +import "../../governance/Governor.sol"; +import "../../governance/extensions/GovernorProposalThreshold.sol"; +import "../../governance/extensions/GovernorCountingSimple.sol"; +import "../../governance/extensions/GovernorVotes.sol"; +import "../../governance/extensions/GovernorVotesQuorumFraction.sol"; +import "../../governance/extensions/GovernorTimelockControl.sol"; + +contract MyGovernor2 is + Governor, + GovernorTimelockControl, + GovernorProposalThreshold, + GovernorVotes, + GovernorVotesQuorumFraction, + GovernorCountingSimple +{ + constructor(ERC20Votes _token, TimelockController _timelock) + Governor("MyGovernor") + GovernorVotes(_token) + GovernorVotesQuorumFraction(4) + GovernorTimelockControl(_timelock) + {} + + function votingDelay() public pure override returns (uint256) { + return 1; // 1 block + } + + function votingPeriod() public pure override returns (uint256) { + return 45818; // 1 week + } + + function proposalThreshold() public pure override returns (uint256) { + return 1000e18; + } + + // The following functions are overrides required by Solidity. + + function quorum(uint256 blockNumber) + public + view + override(IGovernor, GovernorVotesQuorumFraction) + returns (uint256) + { + return super.quorum(blockNumber); + } + + function getVotes(address account, uint256 blockNumber) + public + view + override(IGovernor, GovernorVotes) + returns (uint256) + { + return super.getVotes(account, blockNumber); + } + + function state(uint256 proposalId) public view override(Governor, GovernorTimelockControl) returns (ProposalState) { + return super.state(proposalId); + } + + function propose( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + string memory description + ) public override(Governor, GovernorProposalThreshold, IGovernor) returns (uint256) { + return super.propose(targets, values, calldatas, description); + } + + function _execute( + uint256 proposalId, + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) internal override(Governor, GovernorTimelockControl) { + super._execute(proposalId, targets, values, calldatas, descriptionHash); + } + + function _cancel( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) internal override(Governor, GovernorTimelockControl) returns (uint256) { + return super._cancel(targets, values, calldatas, descriptionHash); + } + + function _executor() internal view override(Governor, GovernorTimelockControl) returns (address) { + return super._executor(); + } + + function supportsInterface(bytes4 interfaceId) + public + view + override(Governor, GovernorTimelockControl) + returns (bool) + { + return super.supportsInterface(interfaceId); + } +} diff --git a/contracts/mocks/wizard/MyGovernor3.sol b/contracts/mocks/wizard/MyGovernor3.sol new file mode 100644 index 00000000000..c2465751a79 --- /dev/null +++ b/contracts/mocks/wizard/MyGovernor3.sol @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.2; + +import "../../governance/Governor.sol"; +import "../../governance/compatibility/GovernorCompatibilityBravo.sol"; +import "../../governance/extensions/GovernorVotes.sol"; +import "../../governance/extensions/GovernorVotesQuorumFraction.sol"; +import "../../governance/extensions/GovernorTimelockControl.sol"; + +contract MyGovernor is + Governor, + GovernorTimelockControl, + GovernorCompatibilityBravo, + GovernorVotes, + GovernorVotesQuorumFraction +{ + constructor(ERC20Votes _token, TimelockController _timelock) + Governor("MyGovernor") + GovernorVotes(_token) + GovernorVotesQuorumFraction(4) + GovernorTimelockControl(_timelock) + {} + + function votingDelay() public pure override returns (uint256) { + return 1; // 1 block + } + + function votingPeriod() public pure override returns (uint256) { + return 45818; // 1 week + } + + function proposalThreshold() public pure override returns (uint256) { + return 1000e18; + } + + // The following functions are overrides required by Solidity. + + function quorum(uint256 blockNumber) + public + view + override(IGovernor, GovernorVotesQuorumFraction) + returns (uint256) + { + return super.quorum(blockNumber); + } + + function getVotes(address account, uint256 blockNumber) + public + view + override(IGovernor, GovernorVotes) + returns (uint256) + { + return super.getVotes(account, blockNumber); + } + + function state(uint256 proposalId) + public + view + override(Governor, IGovernor, GovernorTimelockControl) + returns (ProposalState) + { + return super.state(proposalId); + } + + function propose( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + string memory description + ) public override(Governor, GovernorCompatibilityBravo, IGovernor) returns (uint256) { + return super.propose(targets, values, calldatas, description); + } + + function _execute( + uint256 proposalId, + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) internal override(Governor, GovernorTimelockControl) { + super._execute(proposalId, targets, values, calldatas, descriptionHash); + } + + function _cancel( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) internal override(Governor, GovernorTimelockControl) returns (uint256) { + return super._cancel(targets, values, calldatas, descriptionHash); + } + + function _executor() internal view override(Governor, GovernorTimelockControl) returns (address) { + return super._executor(); + } + + function supportsInterface(bytes4 interfaceId) + public + view + override(Governor, IERC165, GovernorTimelockControl) + returns (bool) + { + return super.supportsInterface(interfaceId); + } +} diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index da3e0f1414f..6017db0507b 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -822,4 +822,125 @@ contract('Governor', function (accounts) { ); }); }); + + describe('Settings update', function () { + describe('setVotingDelay', function () { + beforeEach(async function () { + this.settings = { + proposal: [ + [ this.mock.address ], + [ web3.utils.toWei('0') ], + [ this.mock.contract.methods.setVotingDelay('0').encodeABI() ], + '', + ], + tokenHolder: owner, + voters: [ + { voter: voter1, weight: web3.utils.toWei('10'), support: Enums.VoteType.For }, + ], + }; + }); + afterEach(async function () { + expect(await this.mock.votingDelay()).to.be.bignumber.equal('0'); + + expectEvent( + this.receipts.execute, + 'VotingDelaySet', + { oldVotingDelay: '4', newVotingDelay: '0' }, + ); + }); + runGovernorWorkflow(); + }); + + describe('setVotingPeriod', function () { + beforeEach(async function () { + this.settings = { + proposal: [ + [ this.mock.address ], + [ web3.utils.toWei('0') ], + [ this.mock.contract.methods.setVotingPeriod('32').encodeABI() ], + '', + ], + tokenHolder: owner, + voters: [ + { voter: voter1, weight: web3.utils.toWei('10'), support: Enums.VoteType.For }, + ], + }; + }); + afterEach(async function () { + expect(await this.mock.votingPeriod()).to.be.bignumber.equal('32'); + + expectEvent( + this.receipts.execute, + 'VotingPeriodSet', + { oldVotingPeriod: '16', newVotingPeriod: '32' }, + ); + }); + runGovernorWorkflow(); + }); + + describe('setVotingPeriod to 0', function () { + beforeEach(async function () { + this.settings = { + proposal: [ + [ this.mock.address ], + [ web3.utils.toWei('0') ], + [ this.mock.contract.methods.setVotingPeriod('0').encodeABI() ], + '', + ], + tokenHolder: owner, + voters: [ + { voter: voter1, weight: web3.utils.toWei('10'), support: Enums.VoteType.For }, + ], + steps: { + execute: { error: 'GovernorSettings: voting period too low' }, + }, + }; + }); + afterEach(async function () { + expect(await this.mock.votingPeriod()).to.be.bignumber.equal('16'); + }); + runGovernorWorkflow(); + }); + + describe('setProposalThreshold', function () { + beforeEach(async function () { + this.settings = { + proposal: [ + [ this.mock.address ], + [ web3.utils.toWei('0') ], + [ this.mock.contract.methods.setProposalThreshold('1000000000000000000').encodeABI() ], + '', + ], + tokenHolder: owner, + voters: [ + { voter: voter1, weight: web3.utils.toWei('10'), support: Enums.VoteType.For }, + ], + }; + }); + afterEach(async function () { + expect(await this.mock.proposalThreshold()).to.be.bignumber.equal('1000000000000000000'); + + expectEvent( + this.receipts.execute, + 'ProposalThresholdSet', + { oldProposalThreshold: '0', newProposalThreshold: '1000000000000000000' }, + ); + }); + runGovernorWorkflow(); + }); + + describe('update protected', function () { + it('setVotingDelay', async function () { + await expectRevert(this.mock.setVotingDelay('0'), 'Governor: onlyGovernance'); + }); + + it('setVotingPeriod', async function () { + await expectRevert(this.mock.setVotingPeriod('32'), 'Governor: onlyGovernance'); + }); + + it('setProposalThreshold', async function () { + await expectRevert(this.mock.setProposalThreshold('1000000000000000000'), 'Governor: onlyGovernance'); + }); + }); + }); }); diff --git a/test/governance/extensions/GovernorComp.test.js b/test/governance/extensions/GovernorComp.test.js index 6761b16e409..78cf1e931c3 100644 --- a/test/governance/extensions/GovernorComp.test.js +++ b/test/governance/extensions/GovernorComp.test.js @@ -21,7 +21,7 @@ contract('GovernorComp', function (accounts) { beforeEach(async function () { this.owner = owner; this.token = await Token.new(tokenName, tokenSymbol); - this.mock = await Governor.new(name, this.token.address, 4, 16); + this.mock = await Governor.new(name, this.token.address); this.receiver = await CallReceiver.new(); await this.token.mint(owner, tokenSupply); await this.token.delegate(voter1, { from: voter1 }); diff --git a/test/governance/extensions/GovernorTimelockCompound.test.js b/test/governance/extensions/GovernorTimelockCompound.test.js index 6ac2dcaf571..303cc97e973 100644 --- a/test/governance/extensions/GovernorTimelockCompound.test.js +++ b/test/governance/extensions/GovernorTimelockCompound.test.js @@ -51,6 +51,10 @@ contract('GovernorTimelockCompound', function (accounts) { 'GovernorTimelock', ]); + it('doesn\'t accept ether transfers', async function () { + await expectRevert.unspecified(web3.eth.sendTransaction({ from: voter, to: this.mock.address, value: 1 })); + }); + it('post deployment check', async function () { expect(await this.mock.name()).to.be.equal(name); expect(await this.mock.token()).to.be.equal(this.token.address); diff --git a/test/governance/extensions/GovernorTimelockControl.test.js b/test/governance/extensions/GovernorTimelockControl.test.js index 303a1b6441d..fca7bd53558 100644 --- a/test/governance/extensions/GovernorTimelockControl.test.js +++ b/test/governance/extensions/GovernorTimelockControl.test.js @@ -45,6 +45,10 @@ contract('GovernorTimelockControl', function (accounts) { 'GovernorTimelock', ]); + it('doesn\'t accept ether transfers', async function () { + await expectRevert.unspecified(web3.eth.sendTransaction({ from: voter, to: this.mock.address, value: 1 })); + }); + it('post deployment check', async function () { expect(await this.mock.name()).to.be.equal(name); expect(await this.mock.token()).to.be.equal(this.token.address);