From 562a9f567518528934579c94e3c17113cc886e9e Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 6 Oct 2021 14:24:37 +0200 Subject: [PATCH 01/22] GovernorSettings --- CHANGELOG.md | 1 + .../extensions/GovernorSettings.sol | 72 +++++++++++++++++++ contracts/mocks/GovernorCompMock.sol | 29 ++++---- .../mocks/GovernorCompatibilityBravoMock.sol | 54 +++++++------- contracts/mocks/GovernorMock.sol | 32 ++++----- .../mocks/GovernorTimelockCompoundMock.sol | 34 ++++----- .../mocks/GovernorTimelockControlMock.sol | 34 ++++----- 7 files changed, 167 insertions(+), 89 deletions(-) create mode 100644 contracts/governance/extensions/GovernorSettings.sol diff --git a/CHANGELOG.md b/CHANGELOG.md index 607d864b75e..301fb015813 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * `Ownable`: add an internal `_transferOwnership(address)`. ([#2568](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2568)) * `AccessControl`: add internal `_grantRole(bytes32,address)` and `_revokeRole(bytes32,address)`. ([#2568](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2568)) * `AccessControl`: mark `_setupRole(bytes32,address)` as deprecated in favor of `_grantRole(bytes32,address)`. ([#2568](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2568)) + * `GovernorSettings`: a new governor module that manages voting settings updatable through governance actions. ## 4.3.2 (2021-09-14) diff --git a/contracts/governance/extensions/GovernorSettings.sol b/contracts/governance/extensions/GovernorSettings.sol new file mode 100644 index 00000000000..ca02088359e --- /dev/null +++ b/contracts/governance/extensions/GovernorSettings.sol @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "./GovernorProposalThreshold.sol"; + +/** + * @dev Extension of {Governor} for settings updatable through governance. + * + * _Available since v4.5._ + */ +abstract contract GovernorSettings is GovernorProposalThreshold { + 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); + + constructor( + uint256 initialVotingDelay, + uint256 initialVotingPeriod, + uint256 initialProposalThreshold + ) { + _setVotingDelay(initialVotingDelay); + _setVotingPeriod(initialVotingPeriod); + _setProposalThreshold(initialProposalThreshold); + } + + function votingDelay() public view virtual override returns (uint256) { + return _votingDelay; + } + + function votingPeriod() public view virtual override returns (uint256) { + return _votingPeriod; + } + + function proposalThreshold() public view virtual override returns (uint256) { + return _proposalThreshold; + } + + function setVotingDelay(uint256 newVotingDelay) public onlyGovernance { + _setVotingDelay(newVotingDelay); + } + + function setVotingPeriod(uint256 newVotingPeriod) public onlyGovernance { + _setVotingPeriod(newVotingPeriod); + } + + function setProposalThreshold(uint256 newProposalThreshold) public onlyGovernance { + _setProposalThreshold(newProposalThreshold); + } + + function _setVotingDelay(uint256 newVotingDelay) internal virtual { + uint256 oldVotingDelay = _votingDelay; + _votingDelay = newVotingDelay; + emit VotingDelaySet(oldVotingDelay, newVotingDelay); + } + + function _setVotingPeriod(uint256 newVotingPeriod) internal virtual { + uint256 oldVotingPeriod = _votingPeriod; + _votingPeriod = newVotingPeriod; + emit VotingPeriodSet(oldVotingPeriod, newVotingPeriod); + } + + function _setProposalThreshold(uint256 newProposalThreshold) internal virtual { + uint256 oldProposalThreshold = _proposalThreshold; + _proposalThreshold = newProposalThreshold; + emit ProposalThresholdSet(oldProposalThreshold, newProposalThreshold); + } +} diff --git a/contracts/mocks/GovernorCompMock.sol b/contracts/mocks/GovernorCompMock.sol index 299a90ac7cb..63cf0bc994b 100644 --- a/contracts/mocks/GovernorCompMock.sol +++ b/contracts/mocks/GovernorCompMock.sol @@ -2,36 +2,31 @@ pragma solidity ^0.8.0; -import "../governance/Governor.sol"; +import "../governance/extensions/GovernorSettings.sol"; import "../governance/extensions/GovernorCountingSimple.sol"; import "../governance/extensions/GovernorVotesComp.sol"; -contract GovernorCompMock is Governor, GovernorVotesComp, GovernorCountingSimple { - uint256 immutable _votingDelay; - uint256 immutable _votingPeriod; - +contract GovernorCompMock is GovernorSettings, GovernorVotesComp, GovernorCountingSimple { constructor( string memory name_, ERC20VotesComp token_, uint256 votingDelay_, uint256 votingPeriod_ - ) Governor(name_) GovernorVotesComp(token_) { - _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) GovernorVotesComp(token_) {} function quorum(uint256) public pure override returns (uint256) { return 0; } + 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); + } + function cancel( address[] memory targets, uint256[] memory values, diff --git a/contracts/mocks/GovernorCompatibilityBravoMock.sol b/contracts/mocks/GovernorCompatibilityBravoMock.sol index 061e51e9145..1d3acf5fb4d 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, + GovernorTimelockCompound, + GovernorSettings, + 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; } @@ -75,7 +66,12 @@ contract GovernorCompatibilityBravoMock is GovernorCompatibilityBravo, GovernorT uint256[] memory values, bytes[] memory calldatas, string memory description - ) public virtual override(IGovernor, Governor, GovernorCompatibilityBravo) returns (uint256) { + ) + public + virtual + override(IGovernor, Governor, GovernorCompatibilityBravo, GovernorProposalThreshold) + returns (uint256) + { return super.propose(targets, values, calldatas, description); } @@ -139,6 +135,16 @@ contract GovernorCompatibilityBravoMock is GovernorCompatibilityBravo, GovernorT return super.getVotes(account, blockNumber); } + function proposalThreshold() + public + view + virtual + override(GovernorCompatibilityBravo, GovernorSettings) + returns (uint256) + { + return super.proposalThreshold(); + } + function _executor() internal view virtual override(Governor, GovernorTimelockCompound) returns (address) { return super._executor(); } diff --git a/contracts/mocks/GovernorMock.sol b/contracts/mocks/GovernorMock.sol index 362ce7bc495..9abd0684e70 100644 --- a/contracts/mocks/GovernorMock.sol +++ b/contracts/mocks/GovernorMock.sol @@ -2,31 +2,31 @@ pragma solidity ^0.8.0; -import "../governance/Governor.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 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 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); } function cancel( diff --git a/contracts/mocks/GovernorTimelockCompoundMock.sol b/contracts/mocks/GovernorTimelockCompoundMock.sol index b3af0782f66..6bef1d49450 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 + GovernorTimelockCompound, + GovernorSettings, + 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,15 @@ contract GovernorTimelockCompoundMock is GovernorTimelockCompound, GovernorVotes return super.state(proposalId); } + function propose( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + string memory description + ) public virtual override(IGovernor, Governor, GovernorProposalThreshold) returns (uint256) { + return super.propose(targets, values, calldatas, description); + } + function _execute( uint256 proposalId, address[] memory targets, diff --git a/contracts/mocks/GovernorTimelockControlMock.sol b/contracts/mocks/GovernorTimelockControlMock.sol index d9a19ee31b5..04e1d67c1ea 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 + GovernorTimelockControl, + GovernorSettings, + 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,15 @@ contract GovernorTimelockControlMock is GovernorTimelockControl, GovernorVotesQu return super.state(proposalId); } + function propose( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + string memory description + ) public virtual override(IGovernor, Governor, GovernorProposalThreshold) returns (uint256) { + return super.propose(targets, values, calldatas, description); + } + function _execute( uint256 proposalId, address[] memory targets, From 85f8ca5e172136fb46a623aab1a3cd07501d1af9 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 6 Oct 2021 14:25:56 +0200 Subject: [PATCH 02/22] add PR ref to changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 301fb015813..025ba282b89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ * `Ownable`: add an internal `_transferOwnership(address)`. ([#2568](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2568)) * `AccessControl`: add internal `_grantRole(bytes32,address)` and `_revokeRole(bytes32,address)`. ([#2568](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2568)) * `AccessControl`: mark `_setupRole(bytes32,address)` as deprecated in favor of `_grantRole(bytes32,address)`. ([#2568](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2568)) - * `GovernorSettings`: a new governor module that manages voting settings updatable through governance actions. + * `GovernorSettings`: a new governor module that manages voting settings updatable through governance actions. ([#2904](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2904)) ## 4.3.2 (2021-09-14) From 80be1711a681cb7c04ac896c1e0a5765562e3e4c Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 6 Oct 2021 14:47:26 +0200 Subject: [PATCH 03/22] testing --- .../extensions/GovernorSettings.sol | 2 + test/governance/Governor.test.js | 121 ++++++++++++++++++ 2 files changed, 123 insertions(+) diff --git a/contracts/governance/extensions/GovernorSettings.sol b/contracts/governance/extensions/GovernorSettings.sol index ca02088359e..f48b8b2b6c9 100644 --- a/contracts/governance/extensions/GovernorSettings.sol +++ b/contracts/governance/extensions/GovernorSettings.sol @@ -59,6 +59,8 @@ abstract contract GovernorSettings is GovernorProposalThreshold { } function _setVotingPeriod(uint256 newVotingPeriod) internal virtual { + // voting period must be at least one block long + require(newVotingPeriod > 0, "GovernorSettings: value too low"); uint256 oldVotingPeriod = _votingPeriod; _votingPeriod = newVotingPeriod; emit VotingPeriodSet(oldVotingPeriod, newVotingPeriod); diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index ed58eea571d..f1d2040dd3b 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -818,4 +818,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 low', 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: value 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'); + }); + }); + }); }); From 7f5f5979daaeb5575fbb63f3af8122fcfcf1c348 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 6 Oct 2021 15:30:16 +0200 Subject: [PATCH 04/22] fix inheritance order --- contracts/mocks/GovernorCompatibilityBravoMock.sol | 2 +- contracts/mocks/GovernorTimelockCompoundMock.sol | 2 +- contracts/mocks/GovernorTimelockControlMock.sol | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/mocks/GovernorCompatibilityBravoMock.sol b/contracts/mocks/GovernorCompatibilityBravoMock.sol index 1d3acf5fb4d..15bd0f703da 100644 --- a/contracts/mocks/GovernorCompatibilityBravoMock.sol +++ b/contracts/mocks/GovernorCompatibilityBravoMock.sol @@ -9,8 +9,8 @@ import "../governance/extensions/GovernorVotesComp.sol"; contract GovernorCompatibilityBravoMock is GovernorCompatibilityBravo, - GovernorTimelockCompound, GovernorSettings, + GovernorTimelockCompound, GovernorVotesComp { constructor( diff --git a/contracts/mocks/GovernorTimelockCompoundMock.sol b/contracts/mocks/GovernorTimelockCompoundMock.sol index 6bef1d49450..62a0d6fe0a5 100644 --- a/contracts/mocks/GovernorTimelockCompoundMock.sol +++ b/contracts/mocks/GovernorTimelockCompoundMock.sol @@ -8,8 +8,8 @@ import "../governance/extensions/GovernorCountingSimple.sol"; import "../governance/extensions/GovernorVotesQuorumFraction.sol"; contract GovernorTimelockCompoundMock is - GovernorTimelockCompound, GovernorSettings, + GovernorTimelockCompound, GovernorVotesQuorumFraction, GovernorCountingSimple { diff --git a/contracts/mocks/GovernorTimelockControlMock.sol b/contracts/mocks/GovernorTimelockControlMock.sol index 04e1d67c1ea..238c36d0d14 100644 --- a/contracts/mocks/GovernorTimelockControlMock.sol +++ b/contracts/mocks/GovernorTimelockControlMock.sol @@ -8,8 +8,8 @@ import "../governance/extensions/GovernorCountingSimple.sol"; import "../governance/extensions/GovernorVotesQuorumFraction.sol"; contract GovernorTimelockControlMock is - GovernorTimelockControl, GovernorSettings, + GovernorTimelockControl, GovernorVotesQuorumFraction, GovernorCountingSimple { From d85ecc6b435f184c17c859fd458d9e393b822067 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 6 Oct 2021 17:16:09 +0200 Subject: [PATCH 05/22] merge GovernorProposalThreshold into the core Governor contract --- contracts/governance/Governor.sol | 5 +++++ contracts/governance/IGovernor.sol | 6 ++++++ contracts/governance/README.adoc | 6 ++++-- .../GovernorCompatibilityBravo.sol | 20 ++----------------- .../IGovernorCompatibilityBravo.sol | 5 ----- .../extensions/GovernorProposalThreshold.sol | 20 ------------------- .../extensions/GovernorSettings.sol | 6 +++--- contracts/mocks/GovernorCompMock.sol | 9 --------- .../mocks/GovernorCompatibilityBravoMock.sol | 17 +--------------- contracts/mocks/GovernorMock.sol | 9 --------- .../mocks/GovernorTimelockCompoundMock.sol | 9 --------- .../mocks/GovernorTimelockControlMock.sol | 9 --------- .../SupportsInterface.behavior.js | 1 + 13 files changed, 22 insertions(+), 100 deletions(-) diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 741e02f7d98..bd5681b29e4 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -174,6 +174,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/IGovernor.sol b/contracts/governance/IGovernor.sol index ee5180dfc2d..0b264b32e5b 100644 --- a/contracts/governance/IGovernor.sol +++ b/contracts/governance/IGovernor.sol @@ -129,6 +129,12 @@ abstract contract IGovernor is IERC165 { */ function votingPeriod() public view virtual returns (uint256); + /** + * @notice module:user-config + * @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); + /** * @notice module:user-config * @dev Minimum number of cast voted required for a proposal to be successful. diff --git a/contracts/governance/README.adoc b/contracts/governance/README.adoc index 4fd478153aa..b7de9fe2722 100644 --- a/contracts/governance/README.adoc +++ b/contracts/governance/README.adoc @@ -40,7 +40,9 @@ 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. + +* {GovernorProposalThreshold}: Restricts proposals to delegates with a minimum voting power. This is now deprecated as this feature moved to the core Governor contract, but kept for backward compatibility. In addition to modules and extensions, the core contract requires a few virtual functions to be implemented to your particular specifications: @@ -72,7 +74,7 @@ NOTE: Functions of the `Governor` contract do not include access control. If you {{GovernorTimelockCompound}} -{{GovernorProposalThreshold}} +{{GovernorSettings}} {{GovernorCompatibilityBravo}} 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..b91a3df6b61 100644 --- a/contracts/governance/extensions/GovernorProposalThreshold.sol +++ b/contracts/governance/extensions/GovernorProposalThreshold.sol @@ -10,25 +10,5 @@ import "../Governor.sol"; * _Available since v4.3._ */ 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 index f48b8b2b6c9..d53c9f71b85 100644 --- a/contracts/governance/extensions/GovernorSettings.sol +++ b/contracts/governance/extensions/GovernorSettings.sol @@ -2,14 +2,14 @@ pragma solidity ^0.8.0; -import "./GovernorProposalThreshold.sol"; +import "../Governor.sol"; /** * @dev Extension of {Governor} for settings updatable through governance. * - * _Available since v4.5._ + * _Available since v4.4._ */ -abstract contract GovernorSettings is GovernorProposalThreshold { +abstract contract GovernorSettings is Governor { uint256 private _votingDelay; uint256 private _votingPeriod; uint256 private _proposalThreshold; diff --git a/contracts/mocks/GovernorCompMock.sol b/contracts/mocks/GovernorCompMock.sol index 63cf0bc994b..a000a6af836 100644 --- a/contracts/mocks/GovernorCompMock.sol +++ b/contracts/mocks/GovernorCompMock.sol @@ -18,15 +18,6 @@ contract GovernorCompMock is GovernorSettings, GovernorVotesComp, GovernorCounti return 0; } - 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); - } - function cancel( address[] memory targets, uint256[] memory values, diff --git a/contracts/mocks/GovernorCompatibilityBravoMock.sol b/contracts/mocks/GovernorCompatibilityBravoMock.sol index 15bd0f703da..895898cc7a8 100644 --- a/contracts/mocks/GovernorCompatibilityBravoMock.sol +++ b/contracts/mocks/GovernorCompatibilityBravoMock.sol @@ -66,12 +66,7 @@ contract GovernorCompatibilityBravoMock is uint256[] memory values, bytes[] memory calldatas, string memory description - ) - public - virtual - override(IGovernor, Governor, GovernorCompatibilityBravo, GovernorProposalThreshold) - returns (uint256) - { + ) public virtual override(IGovernor, Governor, GovernorCompatibilityBravo) returns (uint256) { return super.propose(targets, values, calldatas, description); } @@ -135,16 +130,6 @@ contract GovernorCompatibilityBravoMock is return super.getVotes(account, blockNumber); } - function proposalThreshold() - public - view - virtual - override(GovernorCompatibilityBravo, GovernorSettings) - returns (uint256) - { - return super.proposalThreshold(); - } - function _executor() internal view virtual override(Governor, GovernorTimelockCompound) returns (address) { return super._executor(); } diff --git a/contracts/mocks/GovernorMock.sol b/contracts/mocks/GovernorMock.sol index 9abd0684e70..a4967b9d5ea 100644 --- a/contracts/mocks/GovernorMock.sol +++ b/contracts/mocks/GovernorMock.sol @@ -20,15 +20,6 @@ contract GovernorMock is GovernorSettings, GovernorVotesQuorumFraction, Governor GovernorVotesQuorumFraction(quorumNumerator_) {} - 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); - } - function cancel( address[] memory targets, uint256[] memory values, diff --git a/contracts/mocks/GovernorTimelockCompoundMock.sol b/contracts/mocks/GovernorTimelockCompoundMock.sol index 62a0d6fe0a5..771ef454748 100644 --- a/contracts/mocks/GovernorTimelockCompoundMock.sol +++ b/contracts/mocks/GovernorTimelockCompoundMock.sol @@ -69,15 +69,6 @@ contract GovernorTimelockCompoundMock is return super.state(proposalId); } - function propose( - address[] memory targets, - uint256[] memory values, - bytes[] memory calldatas, - string memory description - ) public virtual override(IGovernor, Governor, GovernorProposalThreshold) returns (uint256) { - return super.propose(targets, values, calldatas, description); - } - function _execute( uint256 proposalId, address[] memory targets, diff --git a/contracts/mocks/GovernorTimelockControlMock.sol b/contracts/mocks/GovernorTimelockControlMock.sol index 238c36d0d14..6da503d8482 100644 --- a/contracts/mocks/GovernorTimelockControlMock.sol +++ b/contracts/mocks/GovernorTimelockControlMock.sol @@ -69,15 +69,6 @@ contract GovernorTimelockControlMock is return super.state(proposalId); } - function propose( - address[] memory targets, - uint256[] memory values, - bytes[] memory calldatas, - string memory description - ) public virtual override(IGovernor, Governor, GovernorProposalThreshold) returns (uint256) { - return super.propose(targets, values, calldatas, description); - } - function _execute( uint256 proposalId, address[] memory targets, diff --git a/test/utils/introspection/SupportsInterface.behavior.js b/test/utils/introspection/SupportsInterface.behavior.js index 2027b4057d0..50bff00bc8f 100644 --- a/test/utils/introspection/SupportsInterface.behavior.js +++ b/test/utils/introspection/SupportsInterface.behavior.js @@ -60,6 +60,7 @@ const INTERFACES = { 'proposalDeadline(uint256)', 'votingDelay()', 'votingPeriod()', + 'proposalThreshold()', 'quorum(uint256)', 'getVotes(address,uint256)', 'hasVoted(uint256,address)', From b00ec17aee147ab21f03d74a4f8cfa4a2cd2cd2c Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 13 Oct 2021 15:56:39 +0200 Subject: [PATCH 06/22] add support for previous interfaceId --- contracts/governance/Governor.sol | 6 +++++- test/governance/Governor.test.js | 1 + .../GovernorTimelockCompound.test.js | 1 + .../GovernorTimelockControl.test.js | 1 + .../SupportsInterface.behavior.js | 19 +++++++++++++++++++ 5 files changed, 27 insertions(+), 1 deletion(-) diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 5a545d43e50..097f0e4a72d 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -66,7 +66,11 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor { * @dev See {IERC165-supportsInterface}. */ function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC165) returns (bool) { - return interfaceId == type(IGovernor).interfaceId || super.supportsInterface(interfaceId); + // In addition to the current interfaceId, also support previous version of the interfaceId that did not + // include the proposalThreshold() function as standard + return interfaceId == type(IGovernor).interfaceId ^ this.proposalThreshold.selector + || interfaceId == type(IGovernor).interfaceId + || super.supportsInterface(interfaceId); } /** diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index 98aa1803de8..45ac0cdbf5a 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -41,6 +41,7 @@ contract('Governor', function (accounts) { shouldSupportInterfaces([ 'ERC165', 'Governor', + 'GovernorExtended' ]); it('deployment check', async function () { diff --git a/test/governance/extensions/GovernorTimelockCompound.test.js b/test/governance/extensions/GovernorTimelockCompound.test.js index 6ac2dcaf571..391a5e46b18 100644 --- a/test/governance/extensions/GovernorTimelockCompound.test.js +++ b/test/governance/extensions/GovernorTimelockCompound.test.js @@ -48,6 +48,7 @@ contract('GovernorTimelockCompound', function (accounts) { shouldSupportInterfaces([ 'ERC165', 'Governor', + 'GovernorExtended', 'GovernorTimelock', ]); diff --git a/test/governance/extensions/GovernorTimelockControl.test.js b/test/governance/extensions/GovernorTimelockControl.test.js index 303a1b6441d..10a1a20d4dc 100644 --- a/test/governance/extensions/GovernorTimelockControl.test.js +++ b/test/governance/extensions/GovernorTimelockControl.test.js @@ -42,6 +42,7 @@ contract('GovernorTimelockControl', function (accounts) { shouldSupportInterfaces([ 'ERC165', 'Governor', + 'GovernorExtended', 'GovernorTimelock', ]); diff --git a/test/utils/introspection/SupportsInterface.behavior.js b/test/utils/introspection/SupportsInterface.behavior.js index 50bff00bc8f..5e9435b72c3 100644 --- a/test/utils/introspection/SupportsInterface.behavior.js +++ b/test/utils/introspection/SupportsInterface.behavior.js @@ -51,6 +51,25 @@ const INTERFACES = { 'getRoleMemberCount(bytes32)', ], Governor: [ + 'name()', + 'version()', + 'COUNTING_MODE()', + 'hashProposal(address[],uint256[],bytes[],bytes32)', + 'state(uint256)', + 'proposalSnapshot(uint256)', + 'proposalDeadline(uint256)', + 'votingDelay()', + 'votingPeriod()', + 'quorum(uint256)', + 'getVotes(address,uint256)', + 'hasVoted(uint256,address)', + 'propose(address[],uint256[],bytes[],string)', + 'execute(address[],uint256[],bytes[],bytes32)', + 'castVote(uint256,uint8)', + 'castVoteWithReason(uint256,uint8,string)', + 'castVoteBySig(uint256,uint8,uint8,bytes32,bytes32)', + ], + GovernorExtended: [ 'name()', 'version()', 'COUNTING_MODE()', From 46f743db88b58371e6a60678b18aa4999e3395af Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 13 Oct 2021 15:57:47 +0200 Subject: [PATCH 07/22] fix lint --- contracts/governance/Governor.sol | 7 ++++--- test/governance/Governor.test.js | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 097f0e4a72d..7bd77ccf9b1 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -68,9 +68,10 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor { function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC165) returns (bool) { // In addition to the current interfaceId, also support previous version of the interfaceId that did not // include the proposalThreshold() function as standard - return interfaceId == type(IGovernor).interfaceId ^ this.proposalThreshold.selector - || interfaceId == type(IGovernor).interfaceId - || super.supportsInterface(interfaceId); + return + interfaceId == type(IGovernor).interfaceId ^ this.proposalThreshold.selector || + interfaceId == type(IGovernor).interfaceId || + super.supportsInterface(interfaceId); } /** diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index 45ac0cdbf5a..44b645b60b5 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -41,7 +41,7 @@ contract('Governor', function (accounts) { shouldSupportInterfaces([ 'ERC165', 'Governor', - 'GovernorExtended' + 'GovernorExtended', ]); it('deployment check', async function () { From 30de83a2b2c37e7c42b5f06fe98dba332aaf0400 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 13 Oct 2021 11:13:01 -0300 Subject: [PATCH 08/22] rename interfaceId in tests --- test/governance/Governor.test.js | 2 +- .../GovernorTimelockCompound.test.js | 2 +- .../GovernorTimelockControl.test.js | 2 +- .../SupportsInterface.behavior.js | 58 +++++++------------ 4 files changed, 25 insertions(+), 39 deletions(-) diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index 44b645b60b5..b43e05e8862 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -40,8 +40,8 @@ contract('Governor', function (accounts) { shouldSupportInterfaces([ 'ERC165', + 'Governor_v4_3', 'Governor', - 'GovernorExtended', ]); it('deployment check', async function () { diff --git a/test/governance/extensions/GovernorTimelockCompound.test.js b/test/governance/extensions/GovernorTimelockCompound.test.js index 391a5e46b18..e5b97dbfa39 100644 --- a/test/governance/extensions/GovernorTimelockCompound.test.js +++ b/test/governance/extensions/GovernorTimelockCompound.test.js @@ -47,8 +47,8 @@ contract('GovernorTimelockCompound', function (accounts) { shouldSupportInterfaces([ 'ERC165', + 'Governor_v4_3', 'Governor', - 'GovernorExtended', 'GovernorTimelock', ]); diff --git a/test/governance/extensions/GovernorTimelockControl.test.js b/test/governance/extensions/GovernorTimelockControl.test.js index 10a1a20d4dc..9394fded107 100644 --- a/test/governance/extensions/GovernorTimelockControl.test.js +++ b/test/governance/extensions/GovernorTimelockControl.test.js @@ -41,8 +41,8 @@ contract('GovernorTimelockControl', function (accounts) { shouldSupportInterfaces([ 'ERC165', + 'Governor_v4_3', 'Governor', - 'GovernorExtended', 'GovernorTimelock', ]); diff --git a/test/utils/introspection/SupportsInterface.behavior.js b/test/utils/introspection/SupportsInterface.behavior.js index 5e9435b72c3..dcf897de228 100644 --- a/test/utils/introspection/SupportsInterface.behavior.js +++ b/test/utils/introspection/SupportsInterface.behavior.js @@ -2,6 +2,26 @@ const { makeInterfaceId } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); +const Governor_v4_3 = [ + 'name()', + 'version()', + 'COUNTING_MODE()', + 'hashProposal(address[],uint256[],bytes[],bytes32)', + 'state(uint256)', + 'proposalSnapshot(uint256)', + 'proposalDeadline(uint256)', + 'votingDelay()', + 'votingPeriod()', + 'quorum(uint256)', + 'getVotes(address,uint256)', + 'hasVoted(uint256,address)', + 'propose(address[],uint256[],bytes[],string)', + 'execute(address[],uint256[],bytes[],bytes32)', + 'castVote(uint256,uint8)', + 'castVoteWithReason(uint256,uint8,string)', + 'castVoteBySig(uint256,uint8,uint8,bytes32,bytes32)', +]; + const INTERFACES = { ERC165: [ 'supportsInterface(bytes4)', @@ -50,44 +70,10 @@ const INTERFACES = { 'getRoleMember(bytes32,uint256)', 'getRoleMemberCount(bytes32)', ], + Governor_v4_3, Governor: [ - 'name()', - 'version()', - 'COUNTING_MODE()', - 'hashProposal(address[],uint256[],bytes[],bytes32)', - 'state(uint256)', - 'proposalSnapshot(uint256)', - 'proposalDeadline(uint256)', - 'votingDelay()', - 'votingPeriod()', - 'quorum(uint256)', - 'getVotes(address,uint256)', - 'hasVoted(uint256,address)', - 'propose(address[],uint256[],bytes[],string)', - 'execute(address[],uint256[],bytes[],bytes32)', - 'castVote(uint256,uint8)', - 'castVoteWithReason(uint256,uint8,string)', - 'castVoteBySig(uint256,uint8,uint8,bytes32,bytes32)', - ], - GovernorExtended: [ - 'name()', - 'version()', - 'COUNTING_MODE()', - 'hashProposal(address[],uint256[],bytes[],bytes32)', - 'state(uint256)', - 'proposalSnapshot(uint256)', - 'proposalDeadline(uint256)', - 'votingDelay()', - 'votingPeriod()', + ...Governor_v4_3, 'proposalThreshold()', - 'quorum(uint256)', - 'getVotes(address,uint256)', - 'hasVoted(uint256,address)', - 'propose(address[],uint256[],bytes[],string)', - 'execute(address[],uint256[],bytes[],bytes32)', - 'castVote(uint256,uint8)', - 'castVoteWithReason(uint256,uint8,string)', - 'castVoteBySig(uint256,uint8,uint8,bytes32,bytes32)', ], GovernorTimelock: [ 'timelock()', From 56600335e96e646943e86e354e8db0f0b81a0255 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 13 Oct 2021 16:30:17 +0200 Subject: [PATCH 09/22] enshure governor built using the wizard can are still supported --- contracts/governance/IGovernor.sol | 2 +- .../governance/extensions/GovernorProposalThreshold.sol | 9 ++++++++- contracts/mocks/GovernorCompMock.sol | 4 ++++ contracts/mocks/GovernorCompatibilityBravoMock.sol | 4 ++++ contracts/mocks/GovernorMock.sol | 4 ++++ contracts/mocks/GovernorTimelockCompoundMock.sol | 4 ++++ contracts/mocks/GovernorTimelockControlMock.sol | 4 ++++ 7 files changed, 29 insertions(+), 2 deletions(-) diff --git a/contracts/governance/IGovernor.sol b/contracts/governance/IGovernor.sol index 1443af3d57a..f3712a10c96 100644 --- a/contracts/governance/IGovernor.sol +++ b/contracts/governance/IGovernor.sol @@ -136,7 +136,7 @@ abstract contract IGovernor is IERC165 { * @notice module:user-config * @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); + function proposalThreshold() public view virtual returns (uint256) { return 0; } /** * @notice module:user-config diff --git a/contracts/governance/extensions/GovernorProposalThreshold.sol b/contracts/governance/extensions/GovernorProposalThreshold.sol index b91a3df6b61..fd5db45b9f7 100644 --- a/contracts/governance/extensions/GovernorProposalThreshold.sol +++ b/contracts/governance/extensions/GovernorProposalThreshold.sol @@ -10,5 +10,12 @@ import "../Governor.sol"; * _Available since v4.3._ */ abstract contract GovernorProposalThreshold is Governor { - + function propose(address[] memory targets, uint256[] memory values, bytes[] memory calldatas, string memory description) + public + virtual + override + returns (uint256) + { + return super.propose(targets, values, calldatas, description); + } } diff --git a/contracts/mocks/GovernorCompMock.sol b/contracts/mocks/GovernorCompMock.sol index a000a6af836..97c3100e3e7 100644 --- a/contracts/mocks/GovernorCompMock.sol +++ b/contracts/mocks/GovernorCompMock.sol @@ -36,4 +36,8 @@ contract GovernorCompMock is GovernorSettings, GovernorVotesComp, GovernorCounti { return super.getVotes(account, blockNumber); } + + function proposalThreshold() public view override(IGovernor, GovernorSettings) returns (uint256) { + return super.proposalThreshold(); + } } diff --git a/contracts/mocks/GovernorCompatibilityBravoMock.sol b/contracts/mocks/GovernorCompatibilityBravoMock.sol index 895898cc7a8..e48b3d41abf 100644 --- a/contracts/mocks/GovernorCompatibilityBravoMock.sol +++ b/contracts/mocks/GovernorCompatibilityBravoMock.sol @@ -61,6 +61,10 @@ contract GovernorCompatibilityBravoMock is return super.proposalEta(proposalId); } + function proposalThreshold() public view override(IGovernor, 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 a4967b9d5ea..dd64ba2aae4 100644 --- a/contracts/mocks/GovernorMock.sol +++ b/contracts/mocks/GovernorMock.sol @@ -38,4 +38,8 @@ contract GovernorMock is GovernorSettings, GovernorVotesQuorumFraction, Governor { return super.getVotes(account, blockNumber); } + + function proposalThreshold() public view override(IGovernor, GovernorSettings) returns (uint256) { + return super.proposalThreshold(); + } } diff --git a/contracts/mocks/GovernorTimelockCompoundMock.sol b/contracts/mocks/GovernorTimelockCompoundMock.sol index 771ef454748..c7534b26f36 100644 --- a/contracts/mocks/GovernorTimelockCompoundMock.sol +++ b/contracts/mocks/GovernorTimelockCompoundMock.sol @@ -69,6 +69,10 @@ contract GovernorTimelockCompoundMock is return super.state(proposalId); } + function proposalThreshold() public view override(IGovernor, 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 6da503d8482..80b53cdc6a8 100644 --- a/contracts/mocks/GovernorTimelockControlMock.sol +++ b/contracts/mocks/GovernorTimelockControlMock.sol @@ -69,6 +69,10 @@ contract GovernorTimelockControlMock is return super.state(proposalId); } + function proposalThreshold() public view override(IGovernor, GovernorSettings) returns (uint256) { + return super.proposalThreshold(); + } + function _execute( uint256 proposalId, address[] memory targets, From 2d85c553a032a2616c6dd8abd2a529033561466e Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 13 Oct 2021 16:30:41 +0200 Subject: [PATCH 10/22] add wizard produced mocks --- contracts/mocks/wizard/MyGovernor1.sol | 95 +++++++++++++++++++++++ contracts/mocks/wizard/MyGovernor2.sol | 100 +++++++++++++++++++++++++ contracts/mocks/wizard/MyGovernor3.sol | 99 ++++++++++++++++++++++++ 3 files changed, 294 insertions(+) create mode 100644 contracts/mocks/wizard/MyGovernor1.sol create mode 100644 contracts/mocks/wizard/MyGovernor2.sol create mode 100644 contracts/mocks/wizard/MyGovernor3.sol diff --git a/contracts/mocks/wizard/MyGovernor1.sol b/contracts/mocks/wizard/MyGovernor1.sol new file mode 100644 index 00000000000..430437233d6 --- /dev/null +++ b/contracts/mocks/wizard/MyGovernor1.sol @@ -0,0 +1,95 @@ +// 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, GovernorCountingSimple, GovernorVotes, GovernorVotesQuorumFraction, GovernorTimelockControl { + 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..fa392c5c3c4 --- /dev/null +++ b/contracts/mocks/wizard/MyGovernor2.sol @@ -0,0 +1,100 @@ +// 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, GovernorProposalThreshold, GovernorCountingSimple, GovernorVotes, GovernorVotesQuorumFraction, GovernorTimelockControl { + 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..ffe9560ffa1 --- /dev/null +++ b/contracts/mocks/wizard/MyGovernor3.sol @@ -0,0 +1,99 @@ +// 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, GovernorCompatibilityBravo, GovernorVotes, GovernorVotesQuorumFraction, GovernorTimelockControl { + 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); + } +} From aac4ecfadb0eb95d429a99e9dd11f0932af57405 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 13 Oct 2021 17:06:30 +0200 Subject: [PATCH 11/22] move proposalThreshold out of IGovernor and into Governor --- contracts/governance/Governor.sol | 14 +++-- contracts/governance/IGovernor.sol | 6 -- .../extensions/GovernorProposalThreshold.sol | 12 ++-- contracts/mocks/GovernorCompMock.sol | 2 +- .../mocks/GovernorCompatibilityBravoMock.sol | 2 +- contracts/mocks/GovernorMock.sol | 2 +- .../mocks/GovernorTimelockCompoundMock.sol | 2 +- .../mocks/GovernorTimelockControlMock.sol | 2 +- contracts/mocks/wizard/MyGovernor1.sol | 55 +++++++++--------- contracts/mocks/wizard/MyGovernor2.sol | 56 ++++++++++--------- contracts/mocks/wizard/MyGovernor3.sol | 48 +++++++++------- test/governance/Governor.test.js | 1 - .../GovernorTimelockCompound.test.js | 1 - .../GovernorTimelockControl.test.js | 1 - .../SupportsInterface.behavior.js | 40 ++++++------- 15 files changed, 120 insertions(+), 124 deletions(-) diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 7bd77ccf9b1..a6e5eddd02e 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -66,12 +66,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor { * @dev See {IERC165-supportsInterface}. */ function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC165) returns (bool) { - // In addition to the current interfaceId, also support previous version of the interfaceId that did not - // include the proposalThreshold() function as standard - return - interfaceId == type(IGovernor).interfaceId ^ this.proposalThreshold.selector || - interfaceId == type(IGovernor).interfaceId || - super.supportsInterface(interfaceId); + return interfaceId == type(IGovernor).interfaceId || super.supportsInterface(interfaceId); } /** @@ -148,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. */ diff --git a/contracts/governance/IGovernor.sol b/contracts/governance/IGovernor.sol index f3712a10c96..90f4549395b 100644 --- a/contracts/governance/IGovernor.sol +++ b/contracts/governance/IGovernor.sol @@ -132,12 +132,6 @@ abstract contract IGovernor is IERC165 { */ function votingPeriod() public view virtual returns (uint256); - /** - * @notice module:user-config - * @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; } - /** * @notice module:user-config * @dev Minimum number of cast voted required for a proposal to be successful. diff --git a/contracts/governance/extensions/GovernorProposalThreshold.sol b/contracts/governance/extensions/GovernorProposalThreshold.sol index fd5db45b9f7..2ad515ab2f8 100644 --- a/contracts/governance/extensions/GovernorProposalThreshold.sol +++ b/contracts/governance/extensions/GovernorProposalThreshold.sol @@ -10,12 +10,12 @@ import "../Governor.sol"; * _Available since v4.3._ */ abstract contract GovernorProposalThreshold is Governor { - function propose(address[] memory targets, uint256[] memory values, bytes[] memory calldatas, string memory description) - public - virtual - override - returns (uint256) - { + function propose( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + string memory description + ) public virtual override returns (uint256) { return super.propose(targets, values, calldatas, description); } } diff --git a/contracts/mocks/GovernorCompMock.sol b/contracts/mocks/GovernorCompMock.sol index 97c3100e3e7..3e8e40de17c 100644 --- a/contracts/mocks/GovernorCompMock.sol +++ b/contracts/mocks/GovernorCompMock.sol @@ -37,7 +37,7 @@ contract GovernorCompMock is GovernorSettings, GovernorVotesComp, GovernorCounti return super.getVotes(account, blockNumber); } - function proposalThreshold() public view override(IGovernor, GovernorSettings) returns (uint256) { + function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) { return super.proposalThreshold(); } } diff --git a/contracts/mocks/GovernorCompatibilityBravoMock.sol b/contracts/mocks/GovernorCompatibilityBravoMock.sol index e48b3d41abf..60afbb918e9 100644 --- a/contracts/mocks/GovernorCompatibilityBravoMock.sol +++ b/contracts/mocks/GovernorCompatibilityBravoMock.sol @@ -61,7 +61,7 @@ contract GovernorCompatibilityBravoMock is return super.proposalEta(proposalId); } - function proposalThreshold() public view override(IGovernor, GovernorSettings) returns (uint256) { + function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) { return super.proposalThreshold(); } diff --git a/contracts/mocks/GovernorMock.sol b/contracts/mocks/GovernorMock.sol index dd64ba2aae4..8a5c9b00e52 100644 --- a/contracts/mocks/GovernorMock.sol +++ b/contracts/mocks/GovernorMock.sol @@ -39,7 +39,7 @@ contract GovernorMock is GovernorSettings, GovernorVotesQuorumFraction, Governor return super.getVotes(account, blockNumber); } - function proposalThreshold() public view override(IGovernor, GovernorSettings) returns (uint256) { + function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) { return super.proposalThreshold(); } } diff --git a/contracts/mocks/GovernorTimelockCompoundMock.sol b/contracts/mocks/GovernorTimelockCompoundMock.sol index c7534b26f36..848f4b409b3 100644 --- a/contracts/mocks/GovernorTimelockCompoundMock.sol +++ b/contracts/mocks/GovernorTimelockCompoundMock.sol @@ -69,7 +69,7 @@ contract GovernorTimelockCompoundMock is return super.state(proposalId); } - function proposalThreshold() public view override(IGovernor, GovernorSettings) returns (uint256) { + function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) { return super.proposalThreshold(); } diff --git a/contracts/mocks/GovernorTimelockControlMock.sol b/contracts/mocks/GovernorTimelockControlMock.sol index 80b53cdc6a8..4d9e97fd522 100644 --- a/contracts/mocks/GovernorTimelockControlMock.sol +++ b/contracts/mocks/GovernorTimelockControlMock.sol @@ -69,7 +69,7 @@ contract GovernorTimelockControlMock is return super.state(proposalId); } - function proposalThreshold() public view override(IGovernor, GovernorSettings) returns (uint256) { + function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) { return super.proposalThreshold(); } diff --git a/contracts/mocks/wizard/MyGovernor1.sol b/contracts/mocks/wizard/MyGovernor1.sol index 430437233d6..3bbc8a89dac 100644 --- a/contracts/mocks/wizard/MyGovernor1.sol +++ b/contracts/mocks/wizard/MyGovernor1.sol @@ -7,7 +7,13 @@ import "../../governance/extensions/GovernorVotes.sol"; import "../../governance/extensions/GovernorVotesQuorumFraction.sol"; import "../../governance/extensions/GovernorTimelockControl.sol"; -contract MyGovernor1 is Governor, GovernorCountingSimple, GovernorVotes, GovernorVotesQuorumFraction, GovernorTimelockControl { +contract MyGovernor1 is + Governor, + GovernorCountingSimple, + GovernorVotes, + GovernorVotesQuorumFraction, + GovernorTimelockControl +{ constructor(ERC20Votes _token, TimelockController _timelock) Governor("MyGovernor") GovernorVotes(_token) @@ -43,44 +49,39 @@ contract MyGovernor1 is Governor, GovernorCountingSimple, GovernorVotes, Governo return super.getVotes(account, blockNumber); } - function state(uint256 proposalId) - public - view - override(Governor, GovernorTimelockControl) - returns (ProposalState) - { + 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) - { + 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) - { + 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) - { + 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) - { + function _executor() internal view override(Governor, GovernorTimelockControl) returns (address) { return super._executor(); } diff --git a/contracts/mocks/wizard/MyGovernor2.sol b/contracts/mocks/wizard/MyGovernor2.sol index fa392c5c3c4..4e747b8b437 100644 --- a/contracts/mocks/wizard/MyGovernor2.sol +++ b/contracts/mocks/wizard/MyGovernor2.sol @@ -8,7 +8,14 @@ import "../../governance/extensions/GovernorVotes.sol"; import "../../governance/extensions/GovernorVotesQuorumFraction.sol"; import "../../governance/extensions/GovernorTimelockControl.sol"; -contract MyGovernor2 is Governor, GovernorProposalThreshold, GovernorCountingSimple, GovernorVotes, GovernorVotesQuorumFraction, GovernorTimelockControl { +contract MyGovernor2 is + Governor, + GovernorProposalThreshold, + GovernorCountingSimple, + GovernorVotes, + GovernorVotesQuorumFraction, + GovernorTimelockControl +{ constructor(ERC20Votes _token, TimelockController _timelock) Governor("MyGovernor") GovernorVotes(_token) @@ -48,44 +55,39 @@ contract MyGovernor2 is Governor, GovernorProposalThreshold, GovernorCountingSim return super.getVotes(account, blockNumber); } - function state(uint256 proposalId) - public - view - override(Governor, GovernorTimelockControl) - returns (ProposalState) - { + 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) - { + 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) - { + 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) - { + 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) - { + function _executor() internal view override(Governor, GovernorTimelockControl) returns (address) { return super._executor(); } diff --git a/contracts/mocks/wizard/MyGovernor3.sol b/contracts/mocks/wizard/MyGovernor3.sol index ffe9560ffa1..bb51b5abe18 100644 --- a/contracts/mocks/wizard/MyGovernor3.sol +++ b/contracts/mocks/wizard/MyGovernor3.sol @@ -7,7 +7,13 @@ import "../../governance/extensions/GovernorVotes.sol"; import "../../governance/extensions/GovernorVotesQuorumFraction.sol"; import "../../governance/extensions/GovernorTimelockControl.sol"; -contract MyGovernor is Governor, GovernorCompatibilityBravo, GovernorVotes, GovernorVotesQuorumFraction, GovernorTimelockControl { +contract MyGovernor is + Governor, + GovernorCompatibilityBravo, + GovernorVotes, + GovernorVotesQuorumFraction, + GovernorTimelockControl +{ constructor(ERC20Votes _token, TimelockController _timelock) Governor("MyGovernor") GovernorVotes(_token) @@ -56,35 +62,35 @@ contract MyGovernor is Governor, GovernorCompatibilityBravo, GovernorVotes, Gove 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) - { + 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) - { + 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) - { + 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) - { + function _executor() internal view override(Governor, GovernorTimelockControl) returns (address) { return super._executor(); } diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index b43e05e8862..98aa1803de8 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -40,7 +40,6 @@ contract('Governor', function (accounts) { shouldSupportInterfaces([ 'ERC165', - 'Governor_v4_3', 'Governor', ]); diff --git a/test/governance/extensions/GovernorTimelockCompound.test.js b/test/governance/extensions/GovernorTimelockCompound.test.js index e5b97dbfa39..6ac2dcaf571 100644 --- a/test/governance/extensions/GovernorTimelockCompound.test.js +++ b/test/governance/extensions/GovernorTimelockCompound.test.js @@ -47,7 +47,6 @@ contract('GovernorTimelockCompound', function (accounts) { shouldSupportInterfaces([ 'ERC165', - 'Governor_v4_3', 'Governor', 'GovernorTimelock', ]); diff --git a/test/governance/extensions/GovernorTimelockControl.test.js b/test/governance/extensions/GovernorTimelockControl.test.js index 9394fded107..303a1b6441d 100644 --- a/test/governance/extensions/GovernorTimelockControl.test.js +++ b/test/governance/extensions/GovernorTimelockControl.test.js @@ -41,7 +41,6 @@ contract('GovernorTimelockControl', function (accounts) { shouldSupportInterfaces([ 'ERC165', - 'Governor_v4_3', 'Governor', 'GovernorTimelock', ]); diff --git a/test/utils/introspection/SupportsInterface.behavior.js b/test/utils/introspection/SupportsInterface.behavior.js index dcf897de228..2027b4057d0 100644 --- a/test/utils/introspection/SupportsInterface.behavior.js +++ b/test/utils/introspection/SupportsInterface.behavior.js @@ -2,26 +2,6 @@ const { makeInterfaceId } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); -const Governor_v4_3 = [ - 'name()', - 'version()', - 'COUNTING_MODE()', - 'hashProposal(address[],uint256[],bytes[],bytes32)', - 'state(uint256)', - 'proposalSnapshot(uint256)', - 'proposalDeadline(uint256)', - 'votingDelay()', - 'votingPeriod()', - 'quorum(uint256)', - 'getVotes(address,uint256)', - 'hasVoted(uint256,address)', - 'propose(address[],uint256[],bytes[],string)', - 'execute(address[],uint256[],bytes[],bytes32)', - 'castVote(uint256,uint8)', - 'castVoteWithReason(uint256,uint8,string)', - 'castVoteBySig(uint256,uint8,uint8,bytes32,bytes32)', -]; - const INTERFACES = { ERC165: [ 'supportsInterface(bytes4)', @@ -70,10 +50,24 @@ const INTERFACES = { 'getRoleMember(bytes32,uint256)', 'getRoleMemberCount(bytes32)', ], - Governor_v4_3, Governor: [ - ...Governor_v4_3, - 'proposalThreshold()', + 'name()', + 'version()', + 'COUNTING_MODE()', + 'hashProposal(address[],uint256[],bytes[],bytes32)', + 'state(uint256)', + 'proposalSnapshot(uint256)', + 'proposalDeadline(uint256)', + 'votingDelay()', + 'votingPeriod()', + 'quorum(uint256)', + 'getVotes(address,uint256)', + 'hasVoted(uint256,address)', + 'propose(address[],uint256[],bytes[],string)', + 'execute(address[],uint256[],bytes[],bytes32)', + 'castVote(uint256,uint8)', + 'castVoteWithReason(uint256,uint8,string)', + 'castVoteBySig(uint256,uint8,uint8,bytes32,bytes32)', ], GovernorTimelock: [ 'timelock()', From 8b8238b96db37c7d7dd3c77cb5f400679a49a340 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 13 Oct 2021 18:28:26 +0200 Subject: [PATCH 12/22] reorder mocks inheritance to pass inheritance ordering tests --- contracts/mocks/wizard/MyGovernor1.sol | 4 ++-- contracts/mocks/wizard/MyGovernor2.sol | 4 ++-- contracts/mocks/wizard/MyGovernor3.sol | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/contracts/mocks/wizard/MyGovernor1.sol b/contracts/mocks/wizard/MyGovernor1.sol index 3bbc8a89dac..72b486aa782 100644 --- a/contracts/mocks/wizard/MyGovernor1.sol +++ b/contracts/mocks/wizard/MyGovernor1.sol @@ -9,10 +9,10 @@ import "../../governance/extensions/GovernorTimelockControl.sol"; contract MyGovernor1 is Governor, - GovernorCountingSimple, + GovernorTimelockControl, GovernorVotes, GovernorVotesQuorumFraction, - GovernorTimelockControl + GovernorCountingSimple { constructor(ERC20Votes _token, TimelockController _timelock) Governor("MyGovernor") diff --git a/contracts/mocks/wizard/MyGovernor2.sol b/contracts/mocks/wizard/MyGovernor2.sol index 4e747b8b437..3f25b91bfe8 100644 --- a/contracts/mocks/wizard/MyGovernor2.sol +++ b/contracts/mocks/wizard/MyGovernor2.sol @@ -10,11 +10,11 @@ import "../../governance/extensions/GovernorTimelockControl.sol"; contract MyGovernor2 is Governor, + GovernorTimelockControl, GovernorProposalThreshold, - GovernorCountingSimple, GovernorVotes, GovernorVotesQuorumFraction, - GovernorTimelockControl + GovernorCountingSimple { constructor(ERC20Votes _token, TimelockController _timelock) Governor("MyGovernor") diff --git a/contracts/mocks/wizard/MyGovernor3.sol b/contracts/mocks/wizard/MyGovernor3.sol index bb51b5abe18..c2465751a79 100644 --- a/contracts/mocks/wizard/MyGovernor3.sol +++ b/contracts/mocks/wizard/MyGovernor3.sol @@ -9,10 +9,10 @@ import "../../governance/extensions/GovernorTimelockControl.sol"; contract MyGovernor is Governor, + GovernorTimelockControl, GovernorCompatibilityBravo, GovernorVotes, - GovernorVotesQuorumFraction, - GovernorTimelockControl + GovernorVotesQuorumFraction { constructor(ERC20Votes _token, TimelockController _timelock) Governor("MyGovernor") From 4b547769cb1c2cb3d357f42308f7c756fb92c156 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 13 Oct 2021 22:13:42 +0200 Subject: [PATCH 13/22] improve coverage --- test/governance/extensions/GovernorTimelockCompound.test.js | 4 ++++ test/governance/extensions/GovernorTimelockControl.test.js | 4 ++++ 2 files changed, 8 insertions(+) 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); From efafb348058dcef7d6aab76dd90edd4ab49a61b9 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 13 Oct 2021 22:30:34 +0200 Subject: [PATCH 14/22] improve coverage --- contracts/mocks/GovernorMock.sol | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/contracts/mocks/GovernorMock.sol b/contracts/mocks/GovernorMock.sol index 8a5c9b00e52..f7f19fba21f 100644 --- a/contracts/mocks/GovernorMock.sol +++ b/contracts/mocks/GovernorMock.sol @@ -2,11 +2,12 @@ pragma solidity ^0.8.0; +import "../governance/extensions/GovernorProposalThreshold.sol"; import "../governance/extensions/GovernorSettings.sol"; import "../governance/extensions/GovernorCountingSimple.sol"; import "../governance/extensions/GovernorVotesQuorumFraction.sol"; -contract GovernorMock is GovernorSettings, GovernorVotesQuorumFraction, GovernorCountingSimple { +contract GovernorMock is GovernorProposalThreshold, GovernorSettings, GovernorVotesQuorumFraction, GovernorCountingSimple { constructor( string memory name_, ERC20Votes token_, @@ -42,4 +43,13 @@ contract GovernorMock is GovernorSettings, GovernorVotesQuorumFraction, Governor 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); + } } From c5a9fd07acaa9b76c3fd8bd23c6a4f795224c561 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 13 Oct 2021 22:35:18 +0200 Subject: [PATCH 15/22] fix lint --- contracts/mocks/GovernorMock.sol | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/contracts/mocks/GovernorMock.sol b/contracts/mocks/GovernorMock.sol index f7f19fba21f..cc96dcd2775 100644 --- a/contracts/mocks/GovernorMock.sol +++ b/contracts/mocks/GovernorMock.sol @@ -7,7 +7,12 @@ import "../governance/extensions/GovernorSettings.sol"; import "../governance/extensions/GovernorCountingSimple.sol"; import "../governance/extensions/GovernorVotesQuorumFraction.sol"; -contract GovernorMock is GovernorProposalThreshold, GovernorSettings, GovernorVotesQuorumFraction, GovernorCountingSimple { +contract GovernorMock is + GovernorProposalThreshold, + GovernorSettings, + GovernorVotesQuorumFraction, + GovernorCountingSimple +{ constructor( string memory name_, ERC20Votes token_, From cc122ebc846d5c98265aaf27da9ee6c5ba9afede Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 13 Oct 2021 22:49:08 +0200 Subject: [PATCH 16/22] improve coverage --- contracts/mocks/GovernorCompMock.sol | 22 +++++++++---------- .../extensions/GovernorComp.test.js | 2 +- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/contracts/mocks/GovernorCompMock.sol b/contracts/mocks/GovernorCompMock.sol index 3e8e40de17c..9dcbc536d0e 100644 --- a/contracts/mocks/GovernorCompMock.sol +++ b/contracts/mocks/GovernorCompMock.sol @@ -2,22 +2,24 @@ pragma solidity ^0.8.0; -import "../governance/extensions/GovernorSettings.sol"; import "../governance/extensions/GovernorCountingSimple.sol"; import "../governance/extensions/GovernorVotesComp.sol"; -contract GovernorCompMock is GovernorSettings, GovernorVotesComp, GovernorCountingSimple { - constructor( - string memory name_, - ERC20VotesComp token_, - uint256 votingDelay_, - uint256 votingPeriod_ - ) Governor(name_) GovernorSettings(votingDelay_, votingPeriod_, 0) GovernorVotesComp(token_) {} +contract GovernorCompMock is GovernorVotesComp, GovernorCountingSimple { + constructor(string memory name_, ERC20VotesComp token_) Governor(name_) GovernorVotesComp(token_) {} function quorum(uint256) public pure override returns (uint256) { return 0; } + function votingDelay() public pure override returns (uint256) { + return 4; + } + + function votingPeriod() public pure override returns (uint256) { + return 16; + } + function cancel( address[] memory targets, uint256[] memory values, @@ -36,8 +38,4 @@ contract GovernorCompMock is GovernorSettings, GovernorVotesComp, GovernorCounti { return super.getVotes(account, blockNumber); } - - function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) { - return super.proposalThreshold(); - } } 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 }); From 07d972461d7feb3321a9fad3e08a6c7046f9ccf9 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 14 Oct 2021 11:55:52 +0200 Subject: [PATCH 17/22] Apply suggestions from code review Co-authored-by: Francisco Giordano --- contracts/governance/extensions/GovernorSettings.sol | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/contracts/governance/extensions/GovernorSettings.sol b/contracts/governance/extensions/GovernorSettings.sol index d53c9f71b85..25106bbfcbe 100644 --- a/contracts/governance/extensions/GovernorSettings.sol +++ b/contracts/governance/extensions/GovernorSettings.sol @@ -53,14 +53,13 @@ abstract contract GovernorSettings is Governor { } function _setVotingDelay(uint256 newVotingDelay) internal virtual { - uint256 oldVotingDelay = _votingDelay; + emit VotingDelaySet(_votingDelay, newVotingDelay); _votingDelay = newVotingDelay; - emit VotingDelaySet(oldVotingDelay, newVotingDelay); } function _setVotingPeriod(uint256 newVotingPeriod) internal virtual { // voting period must be at least one block long - require(newVotingPeriod > 0, "GovernorSettings: value too low"); + require(newVotingPeriod > 0, "GovernorSettings: voting period too low"); uint256 oldVotingPeriod = _votingPeriod; _votingPeriod = newVotingPeriod; emit VotingPeriodSet(oldVotingPeriod, newVotingPeriod); From 9a80138e125b829bdd3bf4b28b3c60e324b02eab Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 14 Oct 2021 11:56:26 +0200 Subject: [PATCH 18/22] improve doc --- contracts/governance/README.adoc | 6 +++++- .../governance/extensions/GovernorProposalThreshold.sol | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/contracts/governance/README.adoc b/contracts/governance/README.adoc index b7de9fe2722..436964d7790 100644 --- a/contracts/governance/README.adoc +++ b/contracts/governance/README.adoc @@ -42,7 +42,7 @@ Other extensions can customize the behavior or interface in multiple ways. * {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. -* {GovernorProposalThreshold}: Restricts proposals to delegates with a minimum voting power. This is now deprecated as this feature moved to the core Governor contract, but kept for backward compatibility. +* {GovernorProposalThreshold}: Restricts proposals to delegates with a minimum voting power. In addition to modules and extensions, the core contract requires a few virtual functions to be implemented to your particular specifications: @@ -78,6 +78,10 @@ NOTE: Functions of the `Governor` contract do not include access control. If you {{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/extensions/GovernorProposalThreshold.sol b/contracts/governance/extensions/GovernorProposalThreshold.sol index 2ad515ab2f8..60edc870356 100644 --- a/contracts/governance/extensions/GovernorProposalThreshold.sol +++ b/contracts/governance/extensions/GovernorProposalThreshold.sol @@ -8,6 +8,7 @@ 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 { function propose( From 734451b3541dfc9e5dcaad9b31a8c06871cc1a7a Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 14 Oct 2021 12:03:31 +0200 Subject: [PATCH 19/22] fix test --- test/governance/Governor.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index 98aa1803de8..6017db0507b 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -878,7 +878,7 @@ contract('Governor', function (accounts) { runGovernorWorkflow(); }); - describe('setVotingPeriod to low', function () { + describe('setVotingPeriod to 0', function () { beforeEach(async function () { this.settings = { proposal: [ @@ -892,7 +892,7 @@ contract('Governor', function (accounts) { { voter: voter1, weight: web3.utils.toWei('10'), support: Enums.VoteType.For }, ], steps: { - execute: { error: 'GovernorSettings: value too low' }, + execute: { error: 'GovernorSettings: voting period too low' }, }, }; }); From 7220c81c708bbe2cc0dbbce480540036772376cc Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 14 Oct 2021 12:03:29 -0300 Subject: [PATCH 20/22] remove deprecated contract from top level index --- contracts/governance/README.adoc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/contracts/governance/README.adoc b/contracts/governance/README.adoc index 436964d7790..d198c9f9315 100644 --- a/contracts/governance/README.adoc +++ b/contracts/governance/README.adoc @@ -40,9 +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. -* {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. - -* {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: From 095086d44603731255d1ef45e5b92e8de5e0f192 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 14 Oct 2021 22:12:01 -0300 Subject: [PATCH 21/22] simplify events --- contracts/governance/extensions/GovernorSettings.sol | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/contracts/governance/extensions/GovernorSettings.sol b/contracts/governance/extensions/GovernorSettings.sol index 25106bbfcbe..e56f1e60951 100644 --- a/contracts/governance/extensions/GovernorSettings.sol +++ b/contracts/governance/extensions/GovernorSettings.sol @@ -60,14 +60,12 @@ abstract contract GovernorSettings is Governor { function _setVotingPeriod(uint256 newVotingPeriod) internal virtual { // voting period must be at least one block long require(newVotingPeriod > 0, "GovernorSettings: voting period too low"); - uint256 oldVotingPeriod = _votingPeriod; + emit VotingPeriodSet(_votingPeriod, newVotingPeriod); _votingPeriod = newVotingPeriod; - emit VotingPeriodSet(oldVotingPeriod, newVotingPeriod); } function _setProposalThreshold(uint256 newProposalThreshold) internal virtual { - uint256 oldProposalThreshold = _proposalThreshold; + emit ProposalThresholdSet(_proposalThreshold, newProposalThreshold); _proposalThreshold = newProposalThreshold; - emit ProposalThresholdSet(oldProposalThreshold, newProposalThreshold); } } From a3ae78b6a776e62ff7ea5e8f0c28b4431e43823d Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 19 Oct 2021 13:45:20 +0200 Subject: [PATCH 22/22] add documentation to GovernorSettings --- .../extensions/GovernorSettings.sol | 49 +++++++++++++++++-- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/contracts/governance/extensions/GovernorSettings.sol b/contracts/governance/extensions/GovernorSettings.sol index 25106bbfcbe..fcf55c9ca7b 100644 --- a/contracts/governance/extensions/GovernorSettings.sol +++ b/contracts/governance/extensions/GovernorSettings.sol @@ -18,6 +18,9 @@ abstract contract GovernorSettings is Governor { event VotingPeriodSet(uint256 oldVotingPeriod, uint256 newVotingPeriod); event ProposalThresholdSet(uint256 oldProposalThreshold, uint256 newProposalThreshold); + /** + * @dev Initialize the governance parameters. + */ constructor( uint256 initialVotingDelay, uint256 initialVotingPeriod, @@ -28,46 +31,84 @@ abstract contract GovernorSettings is Governor { _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"); - uint256 oldVotingPeriod = _votingPeriod; + + emit VotingPeriodSet(_votingPeriod, newVotingPeriod); _votingPeriod = newVotingPeriod; - emit VotingPeriodSet(oldVotingPeriod, newVotingPeriod); } + /** + * @dev Internal setter for the the proposal threshold. + * + * Emits a {ProposalThresholdSet} event. + */ function _setProposalThreshold(uint256 newProposalThreshold) internal virtual { - uint256 oldProposalThreshold = _proposalThreshold; + emit ProposalThresholdSet(_proposalThreshold, newProposalThreshold); _proposalThreshold = newProposalThreshold; - emit ProposalThresholdSet(oldProposalThreshold, newProposalThreshold); } }