From a9187f9f653acccac1f5e5d8f94f4f3ad6586c8a Mon Sep 17 00:00:00 2001 From: Ben DiFrancesco Date: Mon, 17 Jan 2022 12:36:49 -0500 Subject: [PATCH] Implement and test Govenor castVoteWithReasonAndParamsBySig method --- contracts/governance/Governor.sol | 39 ++++++- contracts/governance/IGovernor.sol | 17 ++- .../extensions/GovernorWithParams.test.js | 101 ++++++++++++++++-- .../SupportsInterface.behavior.js | 1 + 4 files changed, 149 insertions(+), 9 deletions(-) diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index dd6af79fa04..c0291a099e6 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -28,6 +28,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; @@ -70,7 +72,10 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor { // 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) || + interfaceId == + (type(IGovernor).interfaceId ^ + this.castVoteWithReasonAndParams.selector ^ + this.castVoteWithReasonAndParamsBySig.selector) || interfaceId == type(IGovernor).interfaceId || super.supportsInterface(interfaceId); } @@ -381,6 +386,38 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor { return _castVote(proposalId, voter, support, "", _defaultParams()); } + /** + * @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. diff --git a/contracts/governance/IGovernor.sol b/contracts/governance/IGovernor.sol index 3bc1e44837a..74461082da7 100644 --- a/contracts/governance/IGovernor.sol +++ b/contracts/governance/IGovernor.sol @@ -216,7 +216,7 @@ abstract contract IGovernor is IERC165 { ) public virtual returns (uint256 balance); /** - * @dev Cast a vote using the user cryptographic signature. + * @dev Cast a vote using the user's cryptographic signature. * * Emits a {VoteCast} event. */ @@ -227,4 +227,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/test/governance/extensions/GovernorWithParams.test.js b/test/governance/extensions/GovernorWithParams.test.js index 14dfbaf35de..5ef63ba9e17 100644 --- a/test/governance/extensions/GovernorWithParams.test.js +++ b/test/governance/extensions/GovernorWithParams.test.js @@ -1,6 +1,10 @@ -const { BN, expectEvent, expectRevert, time } = require('@openzeppelin/test-helpers'); +const { BN, constants, expectEvent } = require('@openzeppelin/test-helpers'); const { web3 } = require('@openzeppelin/test-helpers/src/setup'); const Enums = require('../../helpers/enums'); +const ethSigUtil = require('eth-sig-util'); +const Wallet = require('ethereumjs-wallet').default; +const { EIP712Domain } = require('../../helpers/eip712'); +const { fromRpcSig } = require('ethereumjs-util'); const { runGovernorWorkflow } = require('../GovernorWorkflow.behavior'); @@ -12,6 +16,7 @@ contract('GovernorWithParams', function (accounts) { const [owner, proposer, voter1, voter2, voter3, voter4] = accounts; const name = 'OZ-Governor'; + const version = '1'; const tokenName = 'MockToken'; const tokenSymbol = 'MTKN'; const tokenSupply = web3.utils.toWei('100'); @@ -104,7 +109,7 @@ contract('GovernorWithParams', function (accounts) { }); describe('Voting with params is properly supported', function () { - const voter2Weight = web3.utils.toWei('1.0'); + const voter2Weight = web3.utils.toWei('1.0'); beforeEach(async function () { this.settings = { proposal: [ @@ -117,8 +122,7 @@ contract('GovernorWithParams', function (accounts) { tokenHolder: owner, voters: [ { voter: voter1, weight: web3.utils.toWei('0.2'), support: Enums.VoteType.Against }, - { voter: voter2, weight: voter2Weight }, // do not actually vote, only getting tokens - { voter: voter3, weight: web3.utils.toWei('0.9') }, // do not actually vote, only getting tokens + { voter: voter2, weight: voter2Weight }, // do not actually vote, only getting tokenss ], steps: { wait: { enable: false }, @@ -132,14 +136,97 @@ contract('GovernorWithParams', function (accounts) { const uintParam = new BN(1); const strParam = 'These are my params'; - const reducedWeight = (new BN(voter2Weight)).sub(uintParam); + const reducedWeight = new BN(voter2Weight).sub(uintParam); const params = web3.eth.abi.encodeParameters(['uint256', 'string'], [uintParam, strParam]); const tx = await this.mock.castVoteWithReasonAndParams(this.id, Enums.VoteType.For, '', params, { from: voter2 }); expectEvent(tx, 'CountParams', { uintParam, strParam }); - expectEvent(tx, 'VoteCast', {voter: voter2, weight: reducedWeight}); + expectEvent(tx, 'VoteCast', { voter: voter2, weight: reducedWeight }); + }); + runGovernorWorkflow(); + }); + + describe('Voting with params by signature is propoerly supported', function () { + const voterBySig = Wallet.generate(); // generate voter by signature wallet + const sigVoterWeight = web3.utils.toWei('1.0'); + + beforeEach(async function () { + this.chainId = await web3.eth.getChainId(); + this.voter = web3.utils.toChecksumAddress(voterBySig.getAddressString()); + + // use delegateBySig to enable vote delegation sig voting wallet + const { v, r, s } = fromRpcSig( + ethSigUtil.signTypedMessage(voterBySig.getPrivateKey(), { + data: { + types: { + EIP712Domain, + Delegation: [ + { name: 'delegatee', type: 'address' }, + { name: 'nonce', type: 'uint256' }, + { name: 'expiry', type: 'uint256' }, + ], + }, + domain: { name: tokenName, version: '1', chainId: this.chainId, verifyingContract: this.token.address }, + primaryType: 'Delegation', + message: { delegatee: this.voter, nonce: 0, expiry: constants.MAX_UINT256 }, + }, + }), + ); + await this.token.delegateBySig(this.voter, 0, constants.MAX_UINT256, v, r, s); + + this.settings = { + proposal: [ + [this.receiver.address], + [0], + [this.receiver.contract.methods.mockFunction().encodeABI()], + '', + ], + proposer, + tokenHolder: owner, + voters: [ + { voter: voter1, weight: web3.utils.toWei('0.2'), support: Enums.VoteType.Against }, + { voter: this.voter, weight: sigVoterWeight }, // do not actually vote, only getting tokens + ], + steps: { + wait: { enable: false }, + execute: { enable: false }, + }, + }; + }); - // TODO: Cast vote with voter3 using params & signature; confirm events exist in tx receipt + afterEach(async function () { + expect(await this.mock.state(this.id)).to.be.bignumber.equal(Enums.ProposalState.Active); + + const reason = 'This is my reason'; + const uintParam = new BN(1); + const strParam = 'These are my params'; + const reducedWeight = new BN(sigVoterWeight).sub(uintParam); + const params = web3.eth.abi.encodeParameters(['uint256', 'string'], [uintParam, strParam]); + + // prepare signature for vote by signature + const { v, r, s } = fromRpcSig( + ethSigUtil.signTypedMessage(voterBySig.getPrivateKey(), { + data: { + types: { + EIP712Domain, + ExtendedBallot: [ + { name: 'proposalId', type: 'uint256' }, + { name: 'support', type: 'uint8' }, + { name: 'reason', type: 'string' }, + { name: 'params', type: 'bytes' }, + ], + }, + domain: { name, version, chainId: this.chainId, verifyingContract: this.mock.address }, + primaryType: 'ExtendedBallot', + message: { proposalId: this.id, support: Enums.VoteType.For, reason, params }, + }, + }), + ); + + const tx = await this.mock.castVoteWithReasonAndParamsBySig(this.id, Enums.VoteType.For, reason, params, v, r, s); + + expectEvent(tx, 'CountParams', { uintParam, strParam }); + expectEvent(tx, 'VoteCast', {voter: this.voter, weight: reducedWeight}); }); runGovernorWorkflow(); }); diff --git a/test/utils/introspection/SupportsInterface.behavior.js b/test/utils/introspection/SupportsInterface.behavior.js index d80148d5d51..2210c9dc46b 100644 --- a/test/utils/introspection/SupportsInterface.behavior.js +++ b/test/utils/introspection/SupportsInterface.behavior.js @@ -88,6 +88,7 @@ const INTERFACES = { 'castVoteWithReason(uint256,uint8,string)', 'castVoteWithReasonAndParams(uint256,uint8,string,bytes)', 'castVoteBySig(uint256,uint8,uint8,bytes32,bytes32)', + 'castVoteWithReasonAndParamsBySig(uint256,uint8,string,bytes,uint8,bytes32,bytes32)', ], GovernorTimelock: [ 'timelock()',