Add Governor module for governance-settable parameters

CHANGELOG.md

@@ -8,6 +8,7 @@
  * `EIP712`: cache `address(this)` to immutable storage to avoid potential issues if a vanilla contract is used in a delegatecall context. ([#2852](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2852))
  * Add internal `_setApprovalForAll` to `ERC721` and `ERC1155`. ([#2834](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2834))
  * `Governor`: shift vote start and end by one block to better match Compound's GovernorBravo and prevent voting at the Governor level if the voting snapshot is not ready. ([#2892](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2892))
+ * `GovernorSettings`: a new governor module that manages voting settings updatable through governance actions. ([#2904](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2904))
  * `PaymentSplitter`: now supports ERC20 assets in addition to Ether. ([#2858](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2858))
 
 ## 4.3.2 (2021-09-14)

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);
     }
 
     /**
@@ -174,6 +178,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");

contracts/governance/IGovernor.sol

@@ -132,6 +132,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.

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

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}.
      */

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

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

contracts/governance/extensions/GovernorSettings.sol

@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import "../Governor.sol";
+
+/**
+ * @dev Extension of {Governor} for settings updatable through governance.
+ *
+ * _Available since v4.4._
+ */
+abstract contract GovernorSettings is Governor {
+    uint256 private _votingDelay;
+    uint256 private _votingPeriod;
+    uint256 private _proposalThreshold;
+
+    event VotingDelaySet(uint256 oldVotingDelay, uint256 newVotingDelay);
+    event VotingPeriodSet(uint256 oldVotingPeriod, uint256 newVotingPeriod);
+    event ProposalThresholdSet(uint256 oldProposalThreshold, uint256 newProposalThreshold);
+
+    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 {
+        // 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);
+    }
+
+    function _setProposalThreshold(uint256 newProposalThreshold) internal virtual {
+        uint256 oldProposalThreshold = _proposalThreshold;
+        _proposalThreshold = newProposalThreshold;
+        emit ProposalThresholdSet(oldProposalThreshold, newProposalThreshold);
+    }
+}

contracts/mocks/GovernorCompMock.sol

@@ -2,31 +2,17 @@
 
 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;

contracts/mocks/GovernorCompatibilityBravoMock.sol

@@ -3,26 +3,29 @@
 pragma solidity ^0.8.0;
 
 import "../governance/compatibility/GovernorCompatibilityBravo.sol";
-import "../governance/extensions/GovernorVotesComp.sol";
 import "../governance/extensions/GovernorTimelockCompound.sol";
+import "../governance/extensions/GovernorSettings.sol";
+import "../governance/extensions/GovernorVotesComp.sol";
 
-contract GovernorCompatibilityBravoMock is GovernorCompatibilityBravo, GovernorTimelockCompound, GovernorVotesComp {
-    uint256 immutable _votingDelay;
-    uint256 immutable _votingPeriod;
-    uint256 immutable _proposalThreshold;
-
+contract GovernorCompatibilityBravoMock is
+    GovernorCompatibilityBravo,
+    GovernorSettings,
+    GovernorTimelockCompound,
+    GovernorVotesComp
+{
     constructor(
         string memory name_,
         ERC20VotesComp token_,
         uint256 votingDelay_,
         uint256 votingPeriod_,
         uint256 proposalThreshold_,
         ICompoundTimelock timelock_
-    ) Governor(name_) GovernorVotesComp(token_) GovernorTimelockCompound(timelock_) {
-        _votingDelay = votingDelay_;
-        _votingPeriod = votingPeriod_;
-        _proposalThreshold = proposalThreshold_;
-    }
+    )
+        Governor(name_)
+        GovernorTimelockCompound(timelock_)
+        GovernorSettings(votingDelay_, votingPeriod_, proposalThreshold_)
+        GovernorVotesComp(token_)
+    {}
 
     function supportsInterface(bytes4 interfaceId)
         public
@@ -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;
     }

contracts/mocks/GovernorMock.sol

@@ -2,32 +2,23 @@
 
 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 cancel(
         address[] memory targets,