From f95789f57d44488c06440b7c1c4db24182395a0c Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Mon, 3 Jul 2023 15:35:08 -0600
Subject: [PATCH 1/7] Make Governor ERC-1271 compatible

---
 contracts/governance/Governor.sol             | 36 ++++++++-----------
 contracts/governance/IGovernor.sol            | 13 +++----
 .../utils/cryptography/SignatureChecker.sol   |  2 +-
 test/governance/Governor.test.js              | 32 ++++++-----------
 .../extensions/GovernorWithParams.test.js     | 29 +++++----------
 test/helpers/governance.js                    |  8 ++---
 .../SupportsInterface.behavior.js             |  6 ++--
 7 files changed, 47 insertions(+), 79 deletions(-)

diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol
index 1e1526cebbb..ef730d3f760 100644
--- a/contracts/governance/Governor.sol
+++ b/contracts/governance/Governor.sol
@@ -5,8 +5,8 @@ pragma solidity ^0.8.19;
 
 import {IERC721Receiver} from "../token/ERC721/IERC721Receiver.sol";
 import {IERC1155Receiver} from "../token/ERC1155/IERC1155Receiver.sol";
-import {ECDSA} from "../utils/cryptography/ECDSA.sol";
 import {EIP712} from "../utils/cryptography/EIP712.sol";
+import {SignatureChecker} from "../utils/cryptography/SignatureChecker.sol";
 import {IERC165, ERC165} from "../utils/introspection/ERC165.sol";
 import {SafeCast} from "../utils/math/SafeCast.sol";
 import {DoubleEndedQueue} from "../utils/structs/DoubleEndedQueue.sol";
@@ -519,22 +519,19 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         uint256 proposalId,
         uint8 support,
         address voter,
-        uint8 v,
-        bytes32 r,
-        bytes32 s
+        bytes memory signature
     ) public virtual override returns (uint256) {
-        address signer = ECDSA.recover(
+        bool valid = SignatureChecker.isValidSignatureNow(
+            voter,
             _hashTypedDataV4(keccak256(abi.encode(BALLOT_TYPEHASH, proposalId, support, voter, _useNonce(voter)))),
-            v,
-            r,
-            s
+            signature
         );
 
-        if (voter != signer) {
-            revert GovernorInvalidSigner(signer, voter);
+        if (!valid) {
+            revert GovernorInvalidSignature(voter);
         }
 
-        return _castVote(proposalId, signer, support, "");
+        return _castVote(proposalId, voter, support, "");
     }
 
     /**
@@ -546,11 +543,10 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         address voter,
         string calldata reason,
         bytes memory params,
-        uint8 v,
-        bytes32 r,
-        bytes32 s
+        bytes memory signature
     ) public virtual override returns (uint256) {
-        address signer = ECDSA.recover(
+        bool valid = SignatureChecker.isValidSignatureNow(
+            voter,
             _hashTypedDataV4(
                 keccak256(
                     abi.encode(
@@ -564,16 +560,14 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
                     )
                 )
             ),
-            v,
-            r,
-            s
+            signature
         );
 
-        if (voter != signer) {
-            revert GovernorInvalidSigner(signer, voter);
+        if (!valid) {
+            revert GovernorInvalidSignature(voter);
         }
 
-        return _castVote(proposalId, signer, support, reason, params);
+        return _castVote(proposalId, voter, support, reason, params);
     }
 
     /**
diff --git a/contracts/governance/IGovernor.sol b/contracts/governance/IGovernor.sol
index 20636e3ba59..190564021e5 100644
--- a/contracts/governance/IGovernor.sol
+++ b/contracts/governance/IGovernor.sol
@@ -81,9 +81,10 @@ abstract contract IGovernor is IERC165, IERC6372 {
     error GovernorInvalidVoteType();
 
     /**
-     * @dev The `voter` doesn't match with the recovered `signer`.
+     * @dev The provided signature is not valid for the expected `voter`.
+     * If the `voter` is a contract, the signature is not valid using {IERC1271-isValidSignature}.
      */
-    error GovernorInvalidSigner(address signer, address voter);
+    error GovernorInvalidSignature(address voter);
 
     /**
      * @dev Emitted when a proposal is created.
@@ -361,9 +362,7 @@ abstract contract IGovernor is IERC165, IERC6372 {
         uint256 proposalId,
         uint8 support,
         address voter,
-        uint8 v,
-        bytes32 r,
-        bytes32 s
+        bytes memory signature
     ) public virtual returns (uint256 balance);
 
     /**
@@ -377,8 +376,6 @@ abstract contract IGovernor is IERC165, IERC6372 {
         address voter,
         string calldata reason,
         bytes memory params,
-        uint8 v,
-        bytes32 r,
-        bytes32 s
+        bytes memory signature
     ) public virtual returns (uint256 balance);
 }
diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol
index dfc512b397b..5caf7bef98e 100644
--- a/contracts/utils/cryptography/SignatureChecker.sol
+++ b/contracts/utils/cryptography/SignatureChecker.sol
@@ -9,7 +9,7 @@ import {IERC1271} from "../../interfaces/IERC1271.sol";
 /**
  * @dev Signature verification helper that can be used instead of `ECDSA.recover` to seamlessly support both ECDSA
  * signatures from externally owned accounts (EOAs) as well as ERC1271 signatures from smart contract wallets like
- * Argent and Gnosis Safe.
+ * Argent and Safe Wallet (previously Gnosis Safe).
  *
  * _Available since v4.1._
  */
diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js
index f4a352c6d8c..bb35b105e17 100644
--- a/test/governance/Governor.test.js
+++ b/test/governance/Governor.test.js
@@ -2,7 +2,6 @@ const { constants, expectEvent, expectRevert } = require('@openzeppelin/test-hel
 const { expect } = require('chai');
 const ethSigUtil = require('eth-sig-util');
 const Wallet = require('ethereumjs-wallet').default;
-const { fromRpcSig, toRpcSig } = require('ethereumjs-util');
 
 const Enums = require('../helpers/enums');
 const { getDomain, domainType } = require('../helpers/eip712');
@@ -186,8 +185,7 @@ contract('Governor', function (accounts) {
               domain,
               message,
             }))
-            .then(data => ethSigUtil.signTypedMessage(voterBySig.getPrivateKey(), { data }))
-            .then(fromRpcSig);
+            .then(data => ethSigUtil.signTypedMessage(voterBySig.getPrivateKey(), { data }));
 
         await this.token.delegate(voterBySigAddress, { from: voter1 });
 
@@ -328,9 +326,9 @@ contract('Governor', function (accounts) {
               }));
 
             this.signature = (contract, message) =>
-              this.data(contract, message)
-                .then(data => ethSigUtil.signTypedMessage(this.voterBySig.getPrivateKey(), { data }))
-                .then(fromRpcSig);
+              this.data(contract, message).then(data =>
+                ethSigUtil.signTypedMessage(this.voterBySig.getPrivateKey(), { data }),
+              );
 
             await this.token.delegate(this.voterBySig.address, { from: voter1 });
 
@@ -348,19 +346,13 @@ contract('Governor', function (accounts) {
               nonce,
               signature: async (...params) => {
                 const sig = await this.signature(...params);
-                sig.s[12] ^= 0xff;
-                return sig;
+                const tamperedSig = web3.utils.hexToBytes(sig);
+                tamperedSig[42] ^= 0xff;
+                return web3.utils.bytesToHex(tamperedSig);
               },
             };
 
-            const { r, s, v } = await this.helper.sign(voteParams);
-            const message = this.helper.forgeMessage(voteParams);
-            const data = await this.data(this.mock, message);
-
-            await expectRevertCustomError(this.helper.vote(voteParams), 'GovernorInvalidSigner', [
-              ethSigUtil.recoverTypedSignature({ sig: toRpcSig(v, r, s), data }),
-              voteParams.voter,
-            ]);
+            await expectRevertCustomError(this.helper.vote(voteParams), 'GovernorInvalidSignature', [voteParams.voter]);
           });
 
           it('if vote nonce is incorrect', async function () {
@@ -373,15 +365,11 @@ contract('Governor', function (accounts) {
               signature: this.signature,
             };
 
-            const { r, s, v } = await this.helper.sign(voteParams);
-            const message = this.helper.forgeMessage(voteParams);
-            const data = await this.data(this.mock, { ...message, nonce });
-
             await expectRevertCustomError(
               this.helper.vote(voteParams),
               // The signature check implies the nonce can't be tampered without changing the signer
-              'GovernorInvalidSigner',
-              [ethSigUtil.recoverTypedSignature({ sig: toRpcSig(v, r, s), data }), voteParams.voter],
+              'GovernorInvalidSignature',
+              [voteParams.voter],
             );
           });
         });
diff --git a/test/governance/extensions/GovernorWithParams.test.js b/test/governance/extensions/GovernorWithParams.test.js
index fb58edaaf57..a21e88bec06 100644
--- a/test/governance/extensions/GovernorWithParams.test.js
+++ b/test/governance/extensions/GovernorWithParams.test.js
@@ -2,7 +2,6 @@ const { expectEvent } = require('@openzeppelin/test-helpers');
 const { expect } = require('chai');
 const ethSigUtil = require('eth-sig-util');
 const Wallet = require('ethereumjs-wallet').default;
-const { fromRpcSig, toRpcSig } = require('ethereumjs-util');
 
 const Enums = require('../../helpers/enums');
 const { getDomain, domainType } = require('../../helpers/eip712');
@@ -144,9 +143,9 @@ contract('GovernorWithParams', function (accounts) {
             }));
 
           this.signature = (contract, message) =>
-            this.data(contract, message)
-              .then(data => ethSigUtil.signTypedMessage(this.voterBySig.getPrivateKey(), { data }))
-              .then(fromRpcSig);
+            this.data(contract, message).then(data =>
+              ethSigUtil.signTypedMessage(this.voterBySig.getPrivateKey(), { data }),
+            );
 
           await this.token.delegate(this.voterBySig.address, { from: voter2 });
 
@@ -193,21 +192,15 @@ contract('GovernorWithParams', function (accounts) {
             nonce,
             signature: async (...params) => {
               const sig = await this.signature(...params);
-              sig.s[12] ^= 0xff;
-              return sig;
+              const tamperedSig = web3.utils.hexToBytes(sig);
+              tamperedSig[42] ^= 0xff;
+              return web3.utils.bytesToHex(tamperedSig);
             },
             reason: 'no particular reason',
             params: encodedParams,
           };
 
-          const { r, s, v } = await this.helper.sign(voteParams);
-          const message = this.helper.forgeMessage(voteParams);
-          const data = await this.data(this.mock, message);
-
-          await expectRevertCustomError(this.helper.vote(voteParams), 'GovernorInvalidSigner', [
-            ethSigUtil.recoverTypedSignature({ sig: toRpcSig(v, r, s), data }),
-            voteParams.voter,
-          ]);
+          await expectRevertCustomError(this.helper.vote(voteParams), 'GovernorInvalidSignature', [voteParams.voter]);
         });
 
         it('reverts if vote nonce is incorrect', async function () {
@@ -222,15 +215,11 @@ contract('GovernorWithParams', function (accounts) {
             params: encodedParams,
           };
 
-          const { r, s, v } = await this.helper.sign(voteParams);
-          const message = this.helper.forgeMessage(voteParams);
-          const data = await this.data(this.mock, { ...message, nonce });
-
           await expectRevertCustomError(
             this.helper.vote(voteParams),
             // The signature check implies the nonce can't be tampered without changing the signer
-            'GovernorInvalidSigner',
-            [ethSigUtil.recoverTypedSignature({ sig: toRpcSig(v, r, s), data }), voteParams.voter],
+            'GovernorInvalidSignature',
+            [voteParams.voter],
           );
         });
       });
diff --git a/test/helpers/governance.js b/test/helpers/governance.js
index 588aeb258c0..3ae0695ac0d 100644
--- a/test/helpers/governance.js
+++ b/test/helpers/governance.js
@@ -91,16 +91,16 @@ class GovernorHelper {
     return vote.signature
       ? // if signature, and either params or reason →
         vote.params || vote.reason
-        ? this.sign(vote).then(({ v, r, s }) =>
+        ? this.sign(vote).then(signature =>
             this.governor.castVoteWithReasonAndParamsBySig(
               ...concatOpts(
-                [proposal.id, vote.support, vote.voter, vote.reason || '', vote.params || '', v, r, s],
+                [proposal.id, vote.support, vote.voter, vote.reason || '', vote.params || '', signature],
                 opts,
               ),
             ),
           )
-        : this.sign(vote).then(({ v, r, s }) =>
-            this.governor.castVoteBySig(...concatOpts([proposal.id, vote.support, vote.voter, v, r, s], opts)),
+        : this.sign(vote).then(signature =>
+            this.governor.castVoteBySig(...concatOpts([proposal.id, vote.support, vote.voter, signature], opts)),
           )
       : vote.params
       ? // otherwise if params
diff --git a/test/utils/introspection/SupportsInterface.behavior.js b/test/utils/introspection/SupportsInterface.behavior.js
index 71be4313f03..49dcd91f164 100644
--- a/test/utils/introspection/SupportsInterface.behavior.js
+++ b/test/utils/introspection/SupportsInterface.behavior.js
@@ -66,7 +66,7 @@ const INTERFACES = {
     'execute(address[],uint256[],bytes[],bytes32)',
     'castVote(uint256,uint8)',
     'castVoteWithReason(uint256,uint8,string)',
-    'castVoteBySig(uint256,uint8,address,uint8,bytes32,bytes32)',
+    'castVoteBySig(uint256,uint8,address,bytes)',
   ],
   GovernorWithParams: [
     'name()',
@@ -87,8 +87,8 @@ const INTERFACES = {
     'castVote(uint256,uint8)',
     'castVoteWithReason(uint256,uint8,string)',
     'castVoteWithReasonAndParams(uint256,uint8,string,bytes)',
-    'castVoteBySig(uint256,uint8,address,uint8,bytes32,bytes32)',
-    'castVoteWithReasonAndParamsBySig(uint256,uint8,address,string,bytes,uint8,bytes32,bytes32)',
+    'castVoteBySig(uint256,uint8,address,bytes)',
+    'castVoteWithReasonAndParamsBySig(uint256,uint8,address,string,bytes,bytes)',
   ],
   GovernorCancel: ['proposalProposer(uint256)', 'cancel(address[],uint256[],bytes[],bytes32)'],
   GovernorTimelock: ['timelock()', 'proposalEta(uint256)', 'queue(address[],uint256[],bytes[],bytes32)'],

From 3c74d60113886ad6e3fdf5a770c73e0c543bfabb Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Mon, 3 Jul 2023 16:40:31 -0600
Subject: [PATCH 2/7] Improved test and docs

---
 contracts/governance/IGovernor.sol            |   5 +-
 test/governance/Governor.test.js              | 143 ++++++++++++------
 .../extensions/GovernorWithParams.test.js     |  75 +++++++--
 3 files changed, 160 insertions(+), 63 deletions(-)

diff --git a/contracts/governance/IGovernor.sol b/contracts/governance/IGovernor.sol
index 190564021e5..af469822697 100644
--- a/contracts/governance/IGovernor.sol
+++ b/contracts/governance/IGovernor.sol
@@ -354,7 +354,7 @@ abstract contract IGovernor is IERC165, IERC6372 {
     ) public virtual returns (uint256 balance);
 
     /**
-     * @dev Cast a vote using the user's cryptographic signature.
+     * @dev Cast a vote using the voter's signature, including ERC-1271 signature support.
      *
      * Emits a {VoteCast} event.
      */
@@ -366,7 +366,8 @@ abstract contract IGovernor is IERC165, IERC6372 {
     ) public virtual returns (uint256 balance);
 
     /**
-     * @dev Cast a vote with a reason and additional encoded parameters using the user's cryptographic signature.
+     * @dev Cast a vote with a reason and additional encoded parameters using the voter's signature,
+     * including ERC-1271 signature support.
      *
      * Emits a {VoteCast} or {VoteCastWithParams} event depending on the length of params.
      */
diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js
index bb35b105e17..26856ed202d 100644
--- a/test/governance/Governor.test.js
+++ b/test/governance/Governor.test.js
@@ -17,6 +17,7 @@ const Governor = artifacts.require('$GovernorMock');
 const CallReceiver = artifacts.require('CallReceiverMock');
 const ERC721 = artifacts.require('$ERC721');
 const ERC1155 = artifacts.require('$ERC1155');
+const ERC1271WalletMock = artifacts.require('ERC1271WalletMock');
 
 const TOKENS = [
   { Token: artifacts.require('$ERC20Votes'), mode: 'blocknumber' },
@@ -165,54 +166,6 @@ contract('Governor', function (accounts) {
         expect(await web3.eth.getBalance(this.receiver.address)).to.be.bignumber.equal(value);
       });
 
-      it('votes with signature', async function () {
-        const voterBySig = Wallet.generate();
-        const voterBySigAddress = web3.utils.toChecksumAddress(voterBySig.getAddressString());
-
-        const signature = (contract, message) =>
-          getDomain(contract)
-            .then(domain => ({
-              primaryType: 'Ballot',
-              types: {
-                EIP712Domain: domainType(domain),
-                Ballot: [
-                  { name: 'proposalId', type: 'uint256' },
-                  { name: 'support', type: 'uint8' },
-                  { name: 'voter', type: 'address' },
-                  { name: 'nonce', type: 'uint256' },
-                ],
-              },
-              domain,
-              message,
-            }))
-            .then(data => ethSigUtil.signTypedMessage(voterBySig.getPrivateKey(), { data }));
-
-        await this.token.delegate(voterBySigAddress, { from: voter1 });
-
-        const nonce = await this.mock.nonces(voterBySigAddress);
-
-        // Run proposal
-        await this.helper.propose();
-        await this.helper.waitForSnapshot();
-        expectEvent(
-          await this.helper.vote({ support: Enums.VoteType.For, voter: voterBySigAddress, nonce, signature }),
-          'VoteCast',
-          {
-            voter: voterBySigAddress,
-            support: Enums.VoteType.For,
-          },
-        );
-        await this.helper.waitForDeadline();
-        await this.helper.execute();
-
-        // After
-        expect(await this.mock.hasVoted(this.proposal.id, owner)).to.be.equal(false);
-        expect(await this.mock.hasVoted(this.proposal.id, voter1)).to.be.equal(false);
-        expect(await this.mock.hasVoted(this.proposal.id, voter2)).to.be.equal(false);
-        expect(await this.mock.hasVoted(this.proposal.id, voterBySigAddress)).to.be.equal(true);
-        expect(await this.mock.nonces(voterBySigAddress)).to.be.bignumber.equal(nonce.addn(1));
-      });
-
       it('send ethers', async function () {
         const empty = web3.utils.toChecksumAddress(web3.utils.randomHex(20));
 
@@ -242,6 +195,100 @@ contract('Governor', function (accounts) {
         expect(await web3.eth.getBalance(empty)).to.be.bignumber.equal(value);
       });
 
+      describe('vote with signature', function () {
+        beforeEach(async function () {
+          this.sign = privateKey => (contract, message) =>
+            getDomain(contract)
+              .then(domain => ({
+                primaryType: 'Ballot',
+                types: {
+                  EIP712Domain: domainType(domain),
+                  Ballot: [
+                    { name: 'proposalId', type: 'uint256' },
+                    { name: 'support', type: 'uint8' },
+                    { name: 'voter', type: 'address' },
+                    { name: 'nonce', type: 'uint256' },
+                  ],
+                },
+                domain,
+                message,
+              }))
+              .then(data => ethSigUtil.signTypedMessage(privateKey, { data }));
+        });
+
+        it('votes with an EOA signature', async function () {
+          const voterBySig = Wallet.generate();
+          const voterBySigAddress = web3.utils.toChecksumAddress(voterBySig.getAddressString());
+
+          await this.token.delegate(voterBySigAddress, { from: voter1 });
+
+          const nonce = await this.mock.nonces(voterBySigAddress);
+
+          // Run proposal
+          await this.helper.propose();
+          await this.helper.waitForSnapshot();
+          expectEvent(
+            await this.helper.vote({
+              support: Enums.VoteType.For,
+              voter: voterBySigAddress,
+              nonce,
+              signature: this.sign(voterBySig.getPrivateKey()),
+            }),
+            'VoteCast',
+            {
+              voter: voterBySigAddress,
+              support: Enums.VoteType.For,
+            },
+          );
+          await this.helper.waitForDeadline();
+          await this.helper.execute();
+
+          // After
+          expect(await this.mock.hasVoted(this.proposal.id, voterBySigAddress)).to.be.equal(true);
+          expect(await this.mock.nonces(voterBySigAddress)).to.be.bignumber.equal(nonce.addn(1));
+        });
+
+        it('votes with a valid EIP-1271 signature', async function () {
+          const ERC1271WalletOwner = Wallet.generate();
+          ERC1271WalletOwner.address = web3.utils.toChecksumAddress(ERC1271WalletOwner.getAddressString());
+
+          const wallet = await ERC1271WalletMock.new(ERC1271WalletOwner.address);
+
+          await this.token.delegate(wallet.address, { from: voter1 });
+
+          const nonce = await this.mock.nonces(wallet.address);
+
+          // Run proposal
+          await this.helper.propose();
+          await this.helper.waitForSnapshot();
+          expectEvent(
+            await this.helper.vote({
+              support: Enums.VoteType.For,
+              voter: wallet.address,
+              nonce,
+              signature: this.sign(ERC1271WalletOwner.getPrivateKey()),
+            }),
+            'VoteCast',
+            {
+              voter: wallet.address,
+              support: Enums.VoteType.For,
+            },
+          );
+          await this.helper.waitForDeadline();
+          await this.helper.execute();
+
+          // After
+          expect(await this.mock.hasVoted(this.proposal.id, wallet.address)).to.be.equal(true);
+          expect(await this.mock.nonces(wallet.address)).to.be.bignumber.equal(nonce.addn(1));
+        });
+
+        afterEach(async function () {
+          expect(await this.mock.hasVoted(this.proposal.id, owner)).to.be.equal(false);
+          expect(await this.mock.hasVoted(this.proposal.id, voter1)).to.be.equal(false);
+          expect(await this.mock.hasVoted(this.proposal.id, voter2)).to.be.equal(false);
+        });
+      });
+
       describe('should revert', function () {
         describe('on propose', function () {
           it('if proposal already exists', async function () {
diff --git a/test/governance/extensions/GovernorWithParams.test.js b/test/governance/extensions/GovernorWithParams.test.js
index a21e88bec06..5a02fcfabf6 100644
--- a/test/governance/extensions/GovernorWithParams.test.js
+++ b/test/governance/extensions/GovernorWithParams.test.js
@@ -10,6 +10,7 @@ const { expectRevertCustomError } = require('../../helpers/customError');
 
 const Governor = artifacts.require('$GovernorWithParamsMock');
 const CallReceiver = artifacts.require('CallReceiverMock');
+const ERC1271WalletMock = artifacts.require('ERC1271WalletMock');
 
 const rawParams = {
   uintParam: web3.utils.toBN('42'),
@@ -119,7 +120,7 @@ contract('GovernorWithParams', function (accounts) {
         expect(votes.forVotes).to.be.bignumber.equal(weight);
       });
 
-      describe('voting by signature', function () {
+      describe.only('voting by signature', function () {
         beforeEach(async function () {
           this.voterBySig = Wallet.generate();
           this.voterBySig.address = web3.utils.toChecksumAddress(this.voterBySig.getAddressString());
@@ -142,35 +143,71 @@ contract('GovernorWithParams', function (accounts) {
               message,
             }));
 
-          this.signature = (contract, message) =>
-            this.data(contract, message).then(data =>
-              ethSigUtil.signTypedMessage(this.voterBySig.getPrivateKey(), { data }),
-            );
+          this.sign = privateKey => (contract, message) =>
+            this.data(contract, message).then(data => ethSigUtil.signTypedMessage(privateKey, { data }));
+        });
 
+        it('suports EOA signatures', async function () {
           await this.token.delegate(this.voterBySig.address, { from: voter2 });
 
+          const weight = web3.utils.toBN(web3.utils.toWei('7')).sub(rawParams.uintParam);
+
+          const nonce = await this.mock.nonces(this.voterBySig.address);
+
           // Run proposal
           await this.helper.propose();
           await this.helper.waitForSnapshot();
+          const tx = await this.helper.vote({
+            support: Enums.VoteType.For,
+            voter: this.voterBySig.address,
+            nonce,
+            reason: 'no particular reason',
+            params: encodedParams,
+            signature: this.sign(this.voterBySig.getPrivateKey()),
+          });
+
+          expectEvent(tx, 'CountParams', { ...rawParams });
+          expectEvent(tx, 'VoteCastWithParams', {
+            voter: this.voterBySig.address,
+            proposalId: this.proposal.id,
+            support: Enums.VoteType.For,
+            weight,
+            reason: 'no particular reason',
+            params: encodedParams,
+          });
+
+          const votes = await this.mock.proposalVotes(this.proposal.id);
+          expect(votes.forVotes).to.be.bignumber.equal(weight);
+          expect(await this.mock.nonces(this.voterBySig.address)).to.be.bignumber.equal(nonce.addn(1));
         });
 
-        it('is properly supported', async function () {
+        it('supports EIP-1271 signature signatures', async function () {
+          const ERC1271WalletOwner = Wallet.generate();
+          ERC1271WalletOwner.address = web3.utils.toChecksumAddress(ERC1271WalletOwner.getAddressString());
+
+          const wallet = await ERC1271WalletMock.new(ERC1271WalletOwner.address);
+
+          await this.token.delegate(wallet.address, { from: voter2 });
+
           const weight = web3.utils.toBN(web3.utils.toWei('7')).sub(rawParams.uintParam);
 
-          const nonce = await this.mock.nonces(this.voterBySig.address);
+          const nonce = await this.mock.nonces(wallet.address);
 
+          // Run proposal
+          await this.helper.propose();
+          await this.helper.waitForSnapshot();
           const tx = await this.helper.vote({
             support: Enums.VoteType.For,
-            voter: this.voterBySig.address,
+            voter: wallet.address,
             nonce,
             reason: 'no particular reason',
             params: encodedParams,
-            signature: this.signature,
+            signature: this.sign(ERC1271WalletOwner.getPrivateKey()),
           });
 
           expectEvent(tx, 'CountParams', { ...rawParams });
           expectEvent(tx, 'VoteCastWithParams', {
-            voter: this.voterBySig.address,
+            voter: wallet.address,
             proposalId: this.proposal.id,
             support: Enums.VoteType.For,
             weight,
@@ -180,18 +217,25 @@ contract('GovernorWithParams', function (accounts) {
 
           const votes = await this.mock.proposalVotes(this.proposal.id);
           expect(votes.forVotes).to.be.bignumber.equal(weight);
-          expect(await this.mock.nonces(this.voterBySig.address)).to.be.bignumber.equal(nonce.addn(1));
+          expect(await this.mock.nonces(wallet.address)).to.be.bignumber.equal(nonce.addn(1));
         });
 
         it('reverts if signature does not match signer', async function () {
+          await this.token.delegate(this.voterBySig.address, { from: voter2 });
+
           const nonce = await this.mock.nonces(this.voterBySig.address);
 
+          const signature = this.sign(this.voterBySig.getPrivateKey());
+
+          // Run proposal
+          await this.helper.propose();
+          await this.helper.waitForSnapshot();
           const voteParams = {
             support: Enums.VoteType.For,
             voter: this.voterBySig.address,
             nonce,
             signature: async (...params) => {
-              const sig = await this.signature(...params);
+              const sig = await signature(...params);
               const tamperedSig = web3.utils.hexToBytes(sig);
               tamperedSig[42] ^= 0xff;
               return web3.utils.bytesToHex(tamperedSig);
@@ -204,13 +248,18 @@ contract('GovernorWithParams', function (accounts) {
         });
 
         it('reverts if vote nonce is incorrect', async function () {
+          await this.token.delegate(this.voterBySig.address, { from: voter2 });
+
           const nonce = await this.mock.nonces(this.voterBySig.address);
 
+          // Run proposal
+          await this.helper.propose();
+          await this.helper.waitForSnapshot();
           const voteParams = {
             support: Enums.VoteType.For,
             voter: this.voterBySig.address,
             nonce: nonce.addn(1),
-            signature: this.signature,
+            signature: this.sign(this.voterBySig.getPrivateKey()),
             reason: 'no particular reason',
             params: encodedParams,
           };

From 351450b87242335723e85236a3da31fba412cc2d Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Mon, 3 Jul 2023 16:42:24 -0600
Subject: [PATCH 3/7] Add changeset

---
 .changeset/dry-crews-sell.md | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 .changeset/dry-crews-sell.md

diff --git a/.changeset/dry-crews-sell.md b/.changeset/dry-crews-sell.md
new file mode 100644
index 00000000000..18c995bc972
--- /dev/null
+++ b/.changeset/dry-crews-sell.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`Governor`: Add support for casting votes with ERC-1271 signatures

From 01cbd6c5a06c99c0e70e19d93cf54ab7e2ed9b6e Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Mon, 3 Jul 2023 16:46:37 -0600
Subject: [PATCH 4/7] Add another changeset

---
 .changeset/popular-deers-raise.md | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 .changeset/popular-deers-raise.md

diff --git a/.changeset/popular-deers-raise.md b/.changeset/popular-deers-raise.md
new file mode 100644
index 00000000000..4753043a3ce
--- /dev/null
+++ b/.changeset/popular-deers-raise.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`Governor`: Use `bytes memory` signature instead of `r`, `s` and `v` parameters in the `castVoteBySig` and `castVoteWithReasonAndParamsBySig` functions

From c0fab294626337fe3f54080d0a8c794a528d89fe Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Tue, 4 Jul 2023 10:26:24 -0600
Subject: [PATCH 5/7] Apply review suggestions

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Co-authored-by: Francisco <fg@frang.io>
---
 test/governance/Governor.test.js              | 48 +++++++++++--------
 .../extensions/GovernorWithParams.test.js     |  6 +--
 2 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js
index 26856ed202d..4e47ab9f210 100644
--- a/test/governance/Governor.test.js
+++ b/test/governance/Governor.test.js
@@ -196,24 +196,30 @@ contract('Governor', function (accounts) {
       });
 
       describe('vote with signature', function () {
-        beforeEach(async function () {
-          this.sign = privateKey => (contract, message) =>
-            getDomain(contract)
-              .then(domain => ({
-                primaryType: 'Ballot',
-                types: {
-                  EIP712Domain: domainType(domain),
-                  Ballot: [
-                    { name: 'proposalId', type: 'uint256' },
-                    { name: 'support', type: 'uint8' },
-                    { name: 'voter', type: 'address' },
-                    { name: 'nonce', type: 'uint256' },
-                  ],
-                },
-                domain,
-                message,
-              }))
-              .then(data => ethSigUtil.signTypedMessage(privateKey, { data }));
+        const sign = privateKey => async (contract, message) => {
+          const domain = await getDomain(contract);
+          return ethSigUtil.signTypedMessage(privateKey, {
+            data: {
+              primaryType: 'Ballot',
+              types: {
+                EIP712Domain: domainType(domain),
+                Ballot: [
+                  { name: 'proposalId', type: 'uint256' },
+                  { name: 'support', type: 'uint8' },
+                  { name: 'voter', type: 'address' },
+                  { name: 'nonce', type: 'uint256' },
+                ],
+              },
+              domain,
+              message,
+            },
+          });
+        };
+
+        afterEach('no other votes are cast for proposalId', async function () {
+          expect(await this.mock.hasVoted(this.proposal.id, owner)).to.be.equal(false);
+          expect(await this.mock.hasVoted(this.proposal.id, voter1)).to.be.equal(false);
+          expect(await this.mock.hasVoted(this.proposal.id, voter2)).to.be.equal(false);
         });
 
         it('votes with an EOA signature', async function () {
@@ -232,7 +238,7 @@ contract('Governor', function (accounts) {
               support: Enums.VoteType.For,
               voter: voterBySigAddress,
               nonce,
-              signature: this.sign(voterBySig.getPrivateKey()),
+              signature: sign(voterBySig.getPrivateKey()),
             }),
             'VoteCast',
             {
@@ -266,7 +272,7 @@ contract('Governor', function (accounts) {
               support: Enums.VoteType.For,
               voter: wallet.address,
               nonce,
-              signature: this.sign(ERC1271WalletOwner.getPrivateKey()),
+              signature: sign(ERC1271WalletOwner.getPrivateKey()),
             }),
             'VoteCast',
             {
@@ -282,7 +288,7 @@ contract('Governor', function (accounts) {
           expect(await this.mock.nonces(wallet.address)).to.be.bignumber.equal(nonce.addn(1));
         });
 
-        afterEach(async function () {
+        afterEach('no other votes are cast', async function () {
           expect(await this.mock.hasVoted(this.proposal.id, owner)).to.be.equal(false);
           expect(await this.mock.hasVoted(this.proposal.id, voter1)).to.be.equal(false);
           expect(await this.mock.hasVoted(this.proposal.id, voter2)).to.be.equal(false);
diff --git a/test/governance/extensions/GovernorWithParams.test.js b/test/governance/extensions/GovernorWithParams.test.js
index 5a02fcfabf6..896c2e094b8 100644
--- a/test/governance/extensions/GovernorWithParams.test.js
+++ b/test/governance/extensions/GovernorWithParams.test.js
@@ -120,7 +120,7 @@ contract('GovernorWithParams', function (accounts) {
         expect(votes.forVotes).to.be.bignumber.equal(weight);
       });
 
-      describe.only('voting by signature', function () {
+      describe('voting by signature', function () {
         beforeEach(async function () {
           this.voterBySig = Wallet.generate();
           this.voterBySig.address = web3.utils.toChecksumAddress(this.voterBySig.getAddressString());
@@ -143,8 +143,8 @@ contract('GovernorWithParams', function (accounts) {
               message,
             }));
 
-          this.sign = privateKey => (contract, message) =>
-            this.data(contract, message).then(data => ethSigUtil.signTypedMessage(privateKey, { data }));
+          this.sign = privateKey => async (contract, message) =>
+            ethSigUtil.signTypedMessage(privateKey, { data: await this.data(contract, message) });
         });
 
         it('suports EOA signatures', async function () {

From 89ba1a44ac0466cb401bbfd987899502eb73adf0 Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Tue, 4 Jul 2023 10:28:38 -0600
Subject: [PATCH 6/7] Changesets periods

Co-authored-by: Francisco <fg@frang.io>
---
 .changeset/dry-crews-sell.md      | 2 +-
 .changeset/popular-deers-raise.md | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/.changeset/dry-crews-sell.md b/.changeset/dry-crews-sell.md
index 18c995bc972..35c85ad6ebb 100644
--- a/.changeset/dry-crews-sell.md
+++ b/.changeset/dry-crews-sell.md
@@ -2,4 +2,4 @@
 'openzeppelin-solidity': minor
 ---
 
-`Governor`: Add support for casting votes with ERC-1271 signatures
+`Governor`: Add support for casting votes with ERC-1271 signatures.
diff --git a/.changeset/popular-deers-raise.md b/.changeset/popular-deers-raise.md
index 4753043a3ce..134fc7dc5cb 100644
--- a/.changeset/popular-deers-raise.md
+++ b/.changeset/popular-deers-raise.md
@@ -2,4 +2,4 @@
 'openzeppelin-solidity': major
 ---
 
-`Governor`: Use `bytes memory` signature instead of `r`, `s` and `v` parameters in the `castVoteBySig` and `castVoteWithReasonAndParamsBySig` functions
+`Governor`: Use `bytes memory` signature instead of `r`, `s` and `v` parameters in the `castVoteBySig` and `castVoteWithReasonAndParamsBySig` functions.

From 3b9dcd26c46ff8df4f75d0e56605a8da099b2e1c Mon Sep 17 00:00:00 2001
From: ernestognw <ernestognw@gmail.com>
Date: Tue, 4 Jul 2023 13:01:17 -0600
Subject: [PATCH 7/7] Merge changesets

---
 .changeset/dry-crews-sell.md      | 5 -----
 .changeset/popular-deers-raise.md | 2 +-
 2 files changed, 1 insertion(+), 6 deletions(-)
 delete mode 100644 .changeset/dry-crews-sell.md

diff --git a/.changeset/dry-crews-sell.md b/.changeset/dry-crews-sell.md
deleted file mode 100644
index 35c85ad6ebb..00000000000
--- a/.changeset/dry-crews-sell.md
+++ /dev/null
@@ -1,5 +0,0 @@
----
-'openzeppelin-solidity': minor
----
-
-`Governor`: Add support for casting votes with ERC-1271 signatures.
diff --git a/.changeset/popular-deers-raise.md b/.changeset/popular-deers-raise.md
index 134fc7dc5cb..ec1fb746625 100644
--- a/.changeset/popular-deers-raise.md
+++ b/.changeset/popular-deers-raise.md
@@ -2,4 +2,4 @@
 'openzeppelin-solidity': major
 ---
 
-`Governor`: Use `bytes memory` signature instead of `r`, `s` and `v` parameters in the `castVoteBySig` and `castVoteWithReasonAndParamsBySig` functions.
+`Governor`: Add support for casting votes with ERC-1271 signatures by using a `bytes memory signature` instead of `r`, `s` and `v` arguments in the `castVoteBySig` and `castVoteWithReasonAndParamsBySig` functions.