Extend Governor with parameterized votes (#3043)

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
OpenZeppelin · Mar 1, 2022 · fd07cc6 · fd07cc6
1 parent f9d9279
commit fd07cc6
Show file tree

Hide file tree

Showing 24 changed files with 535 additions and 107 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,13 @@
  * `ERC1155`: Add a `_afterTokenTransfer` hook for improved extensibility. ([#3166](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3166))
  * `DoubleEndedQueue`: a new data structure that supports efficient push and pop to both front and back, useful for FIFO and LIFO queues. ([#3153](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3153))
  * `Governor`: improved security of `onlyGovernance` modifier when using an external executor contract (e.g. a timelock) that can operate without necessarily going through the governance protocol. ([#3147](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3147))
+ * `Governor`: Add a way to parameterize votes. This can be used to implement voting systems such as fractionalized voting, ERC721 based voting, or any number of other systems. The `params` argument added to `_countVote` method, and included in the newly added `_getVotes` method, can be used by counting and voting modules respectively for such purposes.
+
+### Breaking changes
+
+* `Governor`: Adds internal virtual `_getVotes` method that must be implemented; this is a breaking change for existing concrete extensions to `Governor`. To fix this on an existing voting module extension, rename `getVotes` to `_getVotes` and add a `bytes memory` argument. ([#3043](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3043))
+* `Governor`: Adds `params` parameter to internal virtual `_countVote ` method; this is a breaking change for existing concrete extensions to `Governor`. To fix this on an existing counting module extension, add a `bytes memory` argument to `_countVote`. ([#3043](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3043))
+* `Governor`: Does not emit `VoteCast` event when params data is non-empty; instead emits `VoteCastWithParams` event. To fix this on an integration that consumes the `VoteCast` event, also fetch/monitor `VoteCastWithParams` events. ([#3043](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3043))
 
 ## 4.5.0 (2022-02-09)
 

diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol
@@ -19,7 +19,7 @@ import "./IGovernor.sol";
  * This contract is abstract and requires several function to be implemented in various modules:
  *
  * - A counting module must implement {quorum}, {_quorumReached}, {_voteSucceeded} and {_countVote}
- * - A voting module must implement {getVotes}
+ * - A voting module must implement {_getVotes}
  * - Additionanly, the {votingPeriod} must also be implemented
  *
  * _Available since v4.3._
@@ -30,6 +30,8 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
     using Timers for Timers.BlockNumber;
 
     bytes32 public constant BALLOT_TYPEHASH = keccak256("Ballot(uint256 proposalId,uint8 support)");
+    bytes32 public constant EXTENDED_BALLOT_TYPEHASH =
+        keccak256("ExtendedBallot(uint256 proposalId,uint8 support,string reason,bytes params)");
 
     struct ProposalCore {
         Timers.BlockNumber voteStart;
@@ -86,7 +88,16 @@ 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 castVoteWithReasonAndParams() function as standard
+        return
+            interfaceId ==
+            (type(IGovernor).interfaceId ^
+                this.castVoteWithReasonAndParams.selector ^
+                this.castVoteWithReasonAndParamsBySig.selector ^
+                this.getVotesWithParams.selector) ||
+            interfaceId == type(IGovernor).interfaceId ||
+            super.supportsInterface(interfaceId);
     }
 
     /**
@@ -193,6 +204,15 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
      */
     function _voteSucceeded(uint256 proposalId) internal view virtual returns (bool);
 
+    /**
+     * @dev Get the voting weight of `account` at a specific `blockNumber`, for a vote as described by `params`.
+     */
+    function _getVotes(
+        address account,
+        uint256 blockNumber,
+        bytes memory params
+    ) internal view virtual returns (uint256);
+
     /**
      * @dev Register a vote with a given support and voting weight.
      *
@@ -202,9 +222,20 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
         uint256 proposalId,
         address account,
         uint8 support,
-        uint256 weight
+        uint256 weight,
+        bytes memory params
     ) internal virtual;
 
+    /**
+     * @dev Default additional encoded parameters used by castVote methods that don't include them
+     *
+     * Note: Should be overriden by specific implementations to use an appropriate value, the
+     * meaning of the additional params, in the context of that implementation
+     */
+    function _defaultParams() internal view virtual returns (bytes memory) {
+        return "";
+    }
+
     /**
      * @dev See {IGovernor-propose}.
      */
@@ -355,6 +386,24 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
         return proposalId;
     }
 
+    /**
+     * @dev See {IGovernor-getVotes}.
+     */
+    function getVotes(address account, uint256 blockNumber) public view virtual override returns (uint256) {
+        return _getVotes(account, blockNumber, _defaultParams());
+    }
+
+    /**
+     * @dev See {IGovernor-getVotesWithParams}.
+     */
+    function getVotesWithParams(
+        address account,
+        uint256 blockNumber,
+        bytes memory params
+    ) public view virtual override returns (uint256) {
+        return _getVotes(account, blockNumber, params);
+    }
+
     /**
      * @dev See {IGovernor-castVote}.
      */
@@ -375,6 +424,19 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
         return _castVote(proposalId, voter, support, reason);
     }
 
+    /**
+     * @dev See {IGovernor-castVoteWithReasonAndParams}.
+     */
+    function castVoteWithReasonAndParams(
+        uint256 proposalId,
+        uint8 support,
+        string calldata reason,
+        bytes memory params
+    ) public virtual override returns (uint256) {
+        address voter = _msgSender();
+        return _castVote(proposalId, voter, support, reason, params);
+    }
+
     /**
      * @dev See {IGovernor-castVoteBySig}.
      */
@@ -394,9 +456,41 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
         return _castVote(proposalId, voter, support, "");
     }
 
+    /**
+     * @dev See {IGovernor-castVoteWithReasonAndParamsBySig}.
+     */
+    function castVoteWithReasonAndParamsBySig(
+        uint256 proposalId,
+        uint8 support,
+        string calldata reason,
+        bytes memory params,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) public virtual override returns (uint256) {
+        address voter = ECDSA.recover(
+            _hashTypedDataV4(
+                keccak256(
+                    abi.encode(
+                        EXTENDED_BALLOT_TYPEHASH,
+                        proposalId,
+                        support,
+                        keccak256(bytes(reason)),
+                        keccak256(params)
+                    )
+                )
+            ),
+            v,
+            r,
+            s
+        );
+
+        return _castVote(proposalId, voter, support, reason, params);
+    }
+
     /**
      * @dev Internal vote casting mechanism: Check that the vote is pending, that it has not been cast yet, retrieve
-     * voting weight using {IGovernor-getVotes} and call the {_countVote} internal function.
+     * voting weight using {IGovernor-getVotes} and call the {_countVote} internal function. Uses the _defaultParams().
      *
      * Emits a {IGovernor-VoteCast} event.
      */
@@ -405,14 +499,34 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
         address account,
         uint8 support,
         string memory reason
+    ) internal virtual returns (uint256) {
+        return _castVote(proposalId, account, support, reason, _defaultParams());
+    }
+
+    /**
+     * @dev Internal vote casting mechanism: Check that the vote is pending, that it has not been cast yet, retrieve
+     * voting weight using {IGovernor-getVotes} and call the {_countVote} internal function.
+     *
+     * Emits a {IGovernor-VoteCast} event.
+     */
+    function _castVote(
+        uint256 proposalId,
+        address account,
+        uint8 support,
+        string memory reason,
+        bytes memory params
     ) internal virtual returns (uint256) {
         ProposalCore storage proposal = _proposals[proposalId];
         require(state(proposalId) == ProposalState.Active, "Governor: vote not currently active");
 
-        uint256 weight = getVotes(account, proposal.voteStart.getDeadline());
-        _countVote(proposalId, account, support, weight);
+        uint256 weight = _getVotes(account, proposal.voteStart.getDeadline(), params);
+        _countVote(proposalId, account, support, weight, params);
 
-        emit VoteCast(account, proposalId, support, weight, reason);
+        if (params.length == 0) {
+            emit VoteCast(account, proposalId, support, weight, reason);
+        } else {
+            emit VoteCastWithParams(account, proposalId, support, weight, reason, params);
+        }
 
         return weight;
     }

diff --git a/contracts/governance/IGovernor.sol b/contracts/governance/IGovernor.sol
@@ -48,12 +48,27 @@ abstract contract IGovernor is IERC165 {
     event ProposalExecuted(uint256 proposalId);
 
     /**
-     * @dev Emitted when a vote is cast.
+     * @dev Emitted when a vote is cast without params.
      *
-     * Note: `support` values should be seen as buckets. There interpretation depends on the voting module used.
+     * Note: `support` values should be seen as buckets. Their interpretation depends on the voting module used.
      */
     event VoteCast(address indexed voter, uint256 proposalId, uint8 support, uint256 weight, string reason);
 
+    /**
+     * @dev Emitted when a vote is cast with params.
+     *
+     * Note: `support` values should be seen as buckets. Their interpretation depends on the voting module used.
+     * `params` are additional encoded parameters. Their intepepretation also depends on the voting module used.
+     */
+    event VoteCastWithParams(
+        address indexed voter,
+        uint256 proposalId,
+        uint8 support,
+        uint256 weight,
+        string reason,
+        bytes params
+    );
+
     /**
      * @notice module:core
      * @dev Name of the governor instance (used in building the ERC712 domain separator).
@@ -78,6 +93,12 @@ abstract contract IGovernor is IERC165 {
      * - `quorum=bravo` means that only For votes are counted towards quorum.
      * - `quorum=for,abstain` means that both For and Abstain votes are counted towards quorum.
      *
+     * If a counting module makes use of encoded `params`, it should  include this under a `params` key with a unique
+     * name that describes the behavior. For example:
+     *
+     * - `params=fractional` might refer to a scheme where votes are divided fractionally between for/against/abstain.
+     * - `params=erc721` might refer to a scheme where specific NFTs are delegated to vote.
+     *
      * NOTE: The string can be decoded by the standard
      * https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams[`URLSearchParams`]
      * JavaScript class.
@@ -151,6 +172,16 @@ abstract contract IGovernor is IERC165 {
      */
     function getVotes(address account, uint256 blockNumber) public view virtual returns (uint256);
 
+    /**
+     * @notice module:reputation
+     * @dev Voting power of an `account` at a specific `blockNumber` given additional encoded parameters.
+     */
+    function getVotesWithParams(
+        address account,
+        uint256 blockNumber,
+        bytes memory params
+    ) public view virtual returns (uint256);
+
     /**
      * @notice module:voting
      * @dev Returns weither `account` has cast a vote on `proposalId`.
@@ -204,7 +235,19 @@ abstract contract IGovernor is IERC165 {
     ) public virtual returns (uint256 balance);
 
     /**
-     * @dev Cast a vote using the user cryptographic signature.
+     * @dev Cast a vote with a reason and additional encoded parameters
+     *
+     * Emits a {VoteCast} event.
+     */
+    function castVoteWithReasonAndParams(
+        uint256 proposalId,
+        uint8 support,
+        string calldata reason,
+        bytes memory params
+    ) public virtual returns (uint256 balance);
+
+    /**
+     * @dev Cast a vote using the user's cryptographic signature.
      *
      * Emits a {VoteCast} event.
      */
@@ -215,4 +258,19 @@ abstract contract IGovernor is IERC165 {
         bytes32 r,
         bytes32 s
     ) public virtual returns (uint256 balance);
+
+    /**
+     * @dev Cast a vote with a reason and additional encoded parameters using the user's cryptographic signature.
+     *
+     * Emits a {VoteCast} event.
+     */
+    function castVoteWithReasonAndParamsBySig(
+        uint256 proposalId,
+        uint8 support,
+        string calldata reason,
+        bytes memory params,
+        uint8 v,
+        bytes32 r,
+        bytes32 s
+    ) public virtual returns (uint256 balance);
 }
diff --git a/contracts/governance/compatibility/GovernorCompatibilityBravo.sol b/contracts/governance/compatibility/GovernorCompatibilityBravo.sol
@@ -265,7 +265,8 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
         uint256 proposalId,
         address account,
         uint8 support,
-        uint256 weight
+        uint256 weight,
+        bytes memory // params
     ) internal virtual override {
         ProposalDetails storage details = _proposalDetails[proposalId];
         Receipt storage receipt = details.receipts[account];

diff --git a/contracts/governance/extensions/GovernorCountingSimple.sol b/contracts/governance/extensions/GovernorCountingSimple.sol
@@ -86,7 +86,8 @@ abstract contract GovernorCountingSimple is Governor {
         uint256 proposalId,
         address account,
         uint8 support,
-        uint256 weight
+        uint256 weight,
+        bytes memory // params
     ) internal virtual override {
         ProposalVote storage proposalvote = _proposalVotes[proposalId];
 

diff --git a/contracts/governance/extensions/GovernorPreventLateQuorum.sol b/contracts/governance/extensions/GovernorPreventLateQuorum.sol
@@ -57,9 +57,10 @@ abstract contract GovernorPreventLateQuorum is Governor {
         uint256 proposalId,
         address account,
         uint8 support,
-        string memory reason
+        string memory reason,
+        bytes memory params
     ) internal virtual override returns (uint256) {
-        uint256 result = super._castVote(proposalId, account, support, reason);
+        uint256 result = super._castVote(proposalId, account, support, reason, params);
 
         Timers.BlockNumber storage extendedDeadline = _extendedDeadlines[proposalId];
 

diff --git a/contracts/governance/extensions/GovernorVotes.sol b/contracts/governance/extensions/GovernorVotes.sol
@@ -19,9 +19,13 @@ abstract contract GovernorVotes is Governor {
     }
 
     /**
-     * Read the voting weight from the token's built in snapshot mechanism (see {IGovernor-getVotes}).
+     * Read the voting weight from the token's built in snapshot mechanism (see {Governor-_getVotes}).
      */
-    function getVotes(address account, uint256 blockNumber) public view virtual override returns (uint256) {
+    function _getVotes(
+        address account,
+        uint256 blockNumber,
+        bytes memory /*params*/
+    ) internal view virtual override returns (uint256) {
         return token.getPastVotes(account, blockNumber);
     }
 }
diff --git a/contracts/governance/extensions/GovernorVotesComp.sol b/contracts/governance/extensions/GovernorVotesComp.sol
@@ -19,9 +19,13 @@ abstract contract GovernorVotesComp is Governor {
     }
 
     /**
-     * Read the voting weight from the token's built in snapshot mechanism (see {IGovernor-getVotes}).
+     * Read the voting weight from the token's built in snapshot mechanism (see {Governor-_getVotes}).
      */
-    function getVotes(address account, uint256 blockNumber) public view virtual override returns (uint256) {
+    function _getVotes(
+        address account,
+        uint256 blockNumber,
+        bytes memory /*params*/
+    ) internal view virtual override returns (uint256) {
         return token.getPriorVotes(account, blockNumber);
     }
 }
diff --git a/contracts/mocks/GovernorCompMock.sol b/contracts/mocks/GovernorCompMock.sol
@@ -28,14 +28,4 @@ contract GovernorCompMock is GovernorVotesComp, GovernorCountingSimple {
     ) public returns (uint256 proposalId) {
         return _cancel(targets, values, calldatas, salt);
     }
-
-    function getVotes(address account, uint256 blockNumber)
-        public
-        view
-        virtual
-        override(IGovernor, GovernorVotesComp)
-        returns (uint256)
-    {
-        return super.getVotes(account, blockNumber);
-    }
 }