Add Governor extension GovernorNoncesKeyed to use NoncesKeyed for vote by sig (#5574)

Co-authored-by: Hadrien Croubois <[email protected]>
Co-authored-by: ernestognw <[email protected]>

Author: 3 people

.changeset/popular-geese-tan.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`GovernorNoncesKeyed`: Extension of `Governor` that adds support for keyed nonces when voting by sig.

contracts/governance/Governor.sol

@@ -538,16 +538,9 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         address voter,
         bytes memory signature
     ) public virtual returns (uint256) {
-        bool valid = SignatureChecker.isValidSignatureNow(
-            voter,
-            _hashTypedDataV4(keccak256(abi.encode(BALLOT_TYPEHASH, proposalId, support, voter, _useNonce(voter)))),
-            signature
-        );
-
-        if (!valid) {
+        if (!_validateVoteSig(proposalId, support, voter, signature)) {
             revert GovernorInvalidSignature(voter);
         }
-
         return _castVote(proposalId, voter, support, "");
     }
 
@@ -560,31 +553,56 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         bytes memory params,
         bytes memory signature
     ) public virtual returns (uint256) {
-        bool valid = SignatureChecker.isValidSignatureNow(
-            voter,
-            _hashTypedDataV4(
-                keccak256(
-                    abi.encode(
-                        EXTENDED_BALLOT_TYPEHASH,
-                        proposalId,
-                        support,
-                        voter,
-                        _useNonce(voter),
-                        keccak256(bytes(reason)),
-                        keccak256(params)
-                    )
-                )
-            ),
-            signature
-        );
-
-        if (!valid) {
+        if (!_validateExtendedVoteSig(proposalId, support, voter, reason, params, signature)) {
             revert GovernorInvalidSignature(voter);
         }
-
         return _castVote(proposalId, voter, support, reason, params);
     }
 
+    /// @dev Validate the `signature` used in {castVoteBySig} function.
+    function _validateVoteSig(
+        uint256 proposalId,
+        uint8 support,
+        address voter,
+        bytes memory signature
+    ) internal virtual returns (bool) {
+        return
+            SignatureChecker.isValidSignatureNow(
+                voter,
+                _hashTypedDataV4(keccak256(abi.encode(BALLOT_TYPEHASH, proposalId, support, voter, _useNonce(voter)))),
+                signature
+            );
+    }
+
+    /// @dev Validate the `signature` used in {castVoteWithReasonAndParamsBySig} function.
+    function _validateExtendedVoteSig(
+        uint256 proposalId,
+        uint8 support,
+        address voter,
+        string memory reason,
+        bytes memory params,
+        bytes memory signature
+    ) internal virtual returns (bool) {
+        return
+            SignatureChecker.isValidSignatureNow(
+                voter,
+                _hashTypedDataV4(
+                    keccak256(
+                        abi.encode(
+                            EXTENDED_BALLOT_TYPEHASH,
+                            proposalId,
+                            support,
+                            voter,
+                            _useNonce(voter),
+                            keccak256(bytes(reason)),
+                            keccak256(params)
+                        )
+                    )
+                ),
+                signature
+            );
+    }
+
     /**
      * @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. Uses the _defaultParams().

contracts/governance/README.adoc

@@ -54,6 +54,8 @@ Other extensions can customize the behavior or interface in multiple ways.
 
 * {GovernorSuperQuorum}: Extension of {Governor} with a super quorum. Proposals that meet the super quorum (and have a majority of for votes) advance to the `Succeeded` state before the proposal deadline.
 
+* {GovernorNoncesKeyed}: An extension of {Governor} with support for keyed nonces in addition to traditional nonces when voting by signature.
+
 In addition to modules and extensions, the core contract requires a few virtual functions to be implemented to your particular specifications:
 
 * <<Governor-votingDelay-,`votingDelay()`>>: Delay (in ERC-6372 clock) since the proposal is submitted until voting power is fixed and voting starts. This can be used to enforce a delay after a proposal is published for users to buy tokens, or delegate their votes.
@@ -100,6 +102,8 @@ NOTE: Functions of the `Governor` contract do not include access control. If you
 
 {{GovernorSuperQuorum}}
 
+{{GovernorNoncesKeyed}}
+
 == Utils
 
 {{Votes}}

contracts/governance/extensions/GovernorNoncesKeyed.sol

@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.24;
+
+import {Governor} from "../Governor.sol";
+import {Nonces} from "../../utils/Nonces.sol";
+import {NoncesKeyed} from "../../utils/NoncesKeyed.sol";
+import {SignatureChecker} from "../../utils/cryptography/SignatureChecker.sol";
+
+/**
+ * @dev An extension of {Governor} that extends existing nonce management to use {NoncesKeyed}, where the key is the first 192 bits of the `proposalId`.
+ * This is useful for voting by signature while maintaining separate sequences of nonces for each proposal.
+ *
+ * NOTE: Traditional (un-keyed) nonces are still supported and can continue to be used as if this extension was not present.
+ */
+abstract contract GovernorNoncesKeyed is Governor, NoncesKeyed {
+    function _useCheckedNonce(address owner, uint256 nonce) internal virtual override(Nonces, NoncesKeyed) {
+        super._useCheckedNonce(owner, nonce);
+    }
+
+    /**
+     * @dev Check the signature against keyed nonce and falls back to the traditional nonce.
+     *
+     * NOTE: This function won't call `super._validateVoteSig` if the keyed nonce is valid.
+     * Side effects may be skipped depending on the linearization of the function.
+     */
+    function _validateVoteSig(
+        uint256 proposalId,
+        uint8 support,
+        address voter,
+        bytes memory signature
+    ) internal virtual override returns (bool) {
+        if (
+            SignatureChecker.isValidSignatureNow(
+                voter,
+                _hashTypedDataV4(
+                    keccak256(
+                        abi.encode(BALLOT_TYPEHASH, proposalId, support, voter, nonces(voter, uint192(proposalId)))
+                    )
+                ),
+                signature
+            )
+        ) {
+            _useNonce(voter, uint192(proposalId));
+            return true;
+        } else {
+            return super._validateVoteSig(proposalId, support, voter, signature);
+        }
+    }
+
+    /**
+     * @dev Check the signature against keyed nonce and falls back to the traditional nonce.
+     *
+     * NOTE: This function won't call `super._validateExtendedVoteSig` if the keyed nonce is valid.
+     * Side effects may be skipped depending on the linearization of the function.
+     */
+    function _validateExtendedVoteSig(
+        uint256 proposalId,
+        uint8 support,
+        address voter,
+        string memory reason,
+        bytes memory params,
+        bytes memory signature
+    ) internal virtual override returns (bool) {
+        if (
+            SignatureChecker.isValidSignatureNow(
+                voter,
+                _hashTypedDataV4(
+                    keccak256(
+                        abi.encode(
+                            EXTENDED_BALLOT_TYPEHASH,
+                            proposalId,
+                            support,
+                            voter,
+                            nonces(voter, uint192(proposalId)),
+                            keccak256(bytes(reason)),
+                            keccak256(params)
+                        )
+                    )
+                ),
+                signature
+            )
+        ) {
+            _useNonce(voter, uint192(proposalId));
+            return true;
+        } else {
+            return super._validateExtendedVoteSig(proposalId, support, voter, reason, params, signature);
+        }
+    }
+}

contracts/mocks/governance/GovernorNoncesKeyedMock.sol

@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.24;
+
+import {Governor, Nonces} from "../../governance/Governor.sol";
+import {GovernorSettings} from "../../governance/extensions/GovernorSettings.sol";
+import {GovernorCountingSimple} from "../../governance/extensions/GovernorCountingSimple.sol";
+import {GovernorVotesQuorumFraction} from "../../governance/extensions/GovernorVotesQuorumFraction.sol";
+import {GovernorProposalGuardian} from "../../governance/extensions/GovernorProposalGuardian.sol";
+import {GovernorNoncesKeyed} from "../../governance/extensions/GovernorNoncesKeyed.sol";
+
+abstract contract GovernorNoncesKeyedMock is
+    GovernorSettings,
+    GovernorVotesQuorumFraction,
+    GovernorCountingSimple,
+    GovernorNoncesKeyed
+{
+    function proposalThreshold() public view override(Governor, GovernorSettings) returns (uint256) {
+        return super.proposalThreshold();
+    }
+
+    function _validateVoteSig(
+        uint256 proposalId,
+        uint8 support,
+        address voter,
+        bytes memory signature
+    ) internal virtual override(Governor, GovernorNoncesKeyed) returns (bool) {
+        return super._validateVoteSig(proposalId, support, voter, signature);
+    }
+
+    function _validateExtendedVoteSig(
+        uint256 proposalId,
+        uint8 support,
+        address voter,
+        string memory reason,
+        bytes memory params,
+        bytes memory signature
+    ) internal virtual override(Governor, GovernorNoncesKeyed) returns (bool) {
+        return super._validateExtendedVoteSig(proposalId, support, voter, reason, params, signature);
+    }
+
+    function _useCheckedNonce(address owner, uint256 nonce) internal virtual override(Nonces, GovernorNoncesKeyed) {
+        super._useCheckedNonce(owner, nonce);
+    }
+}

test/governance/Governor.test.js

@@ -198,31 +198,35 @@ describe('Governor', function () {
       });
 
       describe('vote with signature', function () {
-        it('votes with an EOA signature', async function () {
+        it('votes with an EOA signature on two proposals', async function () {
           await this.token.connect(this.voter1).delegate(this.userEOA);
 
-          const nonce = await this.mock.nonces(this.userEOA);
-
-          // Run proposal
-          await this.helper.propose();
-          await this.helper.waitForSnapshot();
-          await expect(
-            this.helper.vote({
-              support: VoteType.For,
-              voter: this.userEOA.address,
-              nonce,
-              signature: signBallot(this.userEOA),
-            }),
-          )
-            .to.emit(this.mock, 'VoteCast')
-            .withArgs(this.userEOA, this.proposal.id, VoteType.For, ethers.parseEther('10'), '');
-
-          await this.helper.waitForDeadline();
-          await this.helper.execute();
+          for (let i = 0; i < 2; i++) {
+            const nonce = await this.mock.nonces(this.userEOA);
 
-          // After
-          expect(await this.mock.hasVoted(this.proposal.id, this.userEOA)).to.be.true;
-          expect(await this.mock.nonces(this.userEOA)).to.equal(nonce + 1n);
+            // Run proposal
+            await this.helper.propose();
+            await this.helper.waitForSnapshot();
+            await expect(
+              this.helper.vote({
+                support: VoteType.For,
+                voter: this.userEOA.address,
+                nonce,
+                signature: signBallot(this.userEOA),
+              }),
+            )
+              .to.emit(this.mock, 'VoteCast')
+              .withArgs(this.userEOA, this.proposal.id, VoteType.For, ethers.parseEther('10'), '');
+
+            // After
+            expect(await this.mock.hasVoted(this.proposal.id, this.userEOA)).to.be.true;
+            expect(await this.mock.nonces(this.userEOA)).to.equal(nonce + 1n);
+
+            // Update proposal to allow for re-propose
+            this.helper.description += ' - updated';
+          }
+
+          await expect(this.mock.nonces(this.userEOA)).to.eventually.equal(2n);
         });
 
         it('votes with a valid EIP-1271 signature', async function () {