diff --git a/.gitignore b/.gitignore index 0495e2f5d6..b4b64c2260 100644 --- a/.gitignore +++ b/.gitignore @@ -75,8 +75,9 @@ generated_docs/ TODO.md -# VSCode file +# IDE file .vscode +.idea # generated contract artifacts/ contracts/broker/generated-artifacts/ diff --git a/contracts/treasury/CHANGELOG.json b/contracts/treasury/CHANGELOG.json index 060e64ae59..76d3ecb0b0 100644 --- a/contracts/treasury/CHANGELOG.json +++ b/contracts/treasury/CHANGELOG.json @@ -1,4 +1,12 @@ [ + { + "version": "1.4.0", + "changes": [ + { + "note": "Support cast vote by signature in Treasury" + } + ] + }, { "timestamp": 1631120757, "version": "1.3.5", diff --git a/contracts/treasury/contracts/src/IZrxTreasury.sol b/contracts/treasury/contracts/src/IZrxTreasury.sol index 8baea3bba1..f0590a16a2 100644 --- a/contracts/treasury/contracts/src/IZrxTreasury.sol +++ b/contracts/treasury/contracts/src/IZrxTreasury.sol @@ -136,8 +136,9 @@ interface IZrxTreasury { returns (uint256 proposalId); /// @dev Casts a vote for the given proposal. Only callable - /// during the voting period for that proposal. See - /// `getVotingPower` for how voting power is computed. + /// during the voting period for that proposal. + /// One address can only vote once. + /// See `getVotingPower` for how voting power is computed. /// @param proposalId The ID of the proposal to vote on. /// @param support Whether to support the proposal or not. /// @param operatedPoolIds The pools operated by `msg.sender`. The @@ -150,6 +151,28 @@ interface IZrxTreasury { ) external; + /// @dev Casts a vote for the given proposal, by signature. + /// Only callable during the voting period for that proposal. + /// One address/voter can only vote once. + /// See `getVotingPower` for how voting power is computed. + /// @param proposalId The ID of the proposal to vote on. + /// @param support Whether to support the proposal or not. + /// @param operatedPoolIds The pools operated by the signer. The + /// ZRX currently delegated to those pools will be accounted + /// for in the voting power. + /// @param v the v field of the signature + /// @param r the r field of the signature + /// @param s the s field of the signature + function castVoteBySignature( + uint256 proposalId, + bool support, + bytes32[] memory operatedPoolIds, + uint8 v, + bytes32 r, + bytes32 s + ) + external; + /// @dev Executes a proposal that has passed and is /// currently executable. /// @param proposalId The ID of the proposal to execute. diff --git a/contracts/treasury/contracts/src/ZrxTreasury.sol b/contracts/treasury/contracts/src/ZrxTreasury.sol index 5097f4897c..b969639127 100644 --- a/contracts/treasury/contracts/src/ZrxTreasury.sol +++ b/contracts/treasury/contracts/src/ZrxTreasury.sol @@ -34,11 +34,25 @@ contract ZrxTreasury is using LibRichErrorsV06 for bytes; using LibBytesV06 for bytes; + /// Contract name + string private constant CONTRACT_NAME = "Zrx Treasury"; + + /// Contract version + string private constant CONTRACT_VERSION = "1.0.0"; + + /// The EIP-712 typehash for the contract's domain + bytes32 private constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); + + /// The EIP-712 typehash for the vote struct + bytes32 private constant VOTE_TYPEHASH = keccak256("TreasuryVote(uint256 proposalId,bool support,bytes32[] operatedPoolIds)"); + // Immutables IStaking public immutable override stakingProxy; DefaultPoolOperator public immutable override defaultPoolOperator; bytes32 public immutable override defaultPoolId; uint256 public immutable override votingPeriod; + bytes32 immutable domainSeparator; + uint256 public override proposalThreshold; uint256 public override quorumThreshold; @@ -67,6 +81,15 @@ contract ZrxTreasury is defaultPoolId = params.defaultPoolId; IStaking.Pool memory defaultPool = stakingProxy_.getStakingPool(params.defaultPoolId); defaultPoolOperator = DefaultPoolOperator(defaultPool.operator); + domainSeparator = keccak256( + abi.encode( + DOMAIN_TYPEHASH, + keccak256(bytes(CONTRACT_NAME)), + _getChainId(), + keccak256(bytes(CONTRACT_VERSION)), + address(this) + ) + ); } // solhint-disable @@ -105,7 +128,7 @@ contract ZrxTreasury is /// be executed if it passes. Must be at least two epochs /// from the current epoch. /// @param description A text description for the proposal. - /// @param operatedPoolIds The pools operated by `msg.sender`. The + /// @param operatedPoolIds The pools operated by the signer. The /// ZRX currently delegated to those pools will be accounted /// for in the voting power. /// @return proposalId The ID of the newly created proposal. @@ -150,8 +173,9 @@ contract ZrxTreasury is } /// @dev Casts a vote for the given proposal. Only callable - /// during the voting period for that proposal. See - /// `getVotingPower` for how voting power is computed. + /// during the voting period for that proposal. + /// One address can only vote once. + /// See `getVotingPower` for how voting power is computed. /// @param proposalId The ID of the proposal to vote on. /// @param support Whether to support the proposal or not. /// @param operatedPoolIds The pools operated by `msg.sender`. The @@ -165,43 +189,39 @@ contract ZrxTreasury is public override { - if (proposalId >= proposalCount()) { - revert("castVote/INVALID_PROPOSAL_ID"); - } - if (hasVoted[proposalId][msg.sender]) { - revert("castVote/ALREADY_VOTED"); - } + return _castVote(msg.sender, proposalId, support, operatedPoolIds); + } - Proposal memory proposal = proposals[proposalId]; - if ( - proposal.voteEpoch != stakingProxy.currentEpoch() || - _hasVoteEnded(proposal.voteEpoch) - ) { - revert("castVote/VOTING_IS_CLOSED"); - } - - uint256 votingPower = getVotingPower(msg.sender, operatedPoolIds); - if (votingPower == 0) { - revert("castVote/NO_VOTING_POWER"); - } - - if (support) { - proposals[proposalId].votesFor = proposals[proposalId].votesFor - .safeAdd(votingPower); - hasVoted[proposalId][msg.sender] = true; - } else { - proposals[proposalId].votesAgainst = proposals[proposalId].votesAgainst - .safeAdd(votingPower); - hasVoted[proposalId][msg.sender] = true; - } - - emit VoteCast( - msg.sender, - operatedPoolIds, - proposalId, - support, - votingPower + /// @dev Casts a vote for the given proposal, by signature. + /// Only callable during the voting period for that proposal. + /// One address/voter can only vote once. + /// See `getVotingPower` for how voting power is computed. + /// @param proposalId The ID of the proposal to vote on. + /// @param support Whether to support the proposal or not. + /// @param operatedPoolIds The pools operated by voter. The + /// ZRX currently delegated to those pools will be accounted + /// for in the voting power. + /// @param v the v field of the signature + /// @param r the r field of the signature + /// @param s the s field of the signature + function castVoteBySignature( + uint256 proposalId, + bool support, + bytes32[] memory operatedPoolIds, + uint8 v, + bytes32 r, + bytes32 s + ) + public + override + { + bytes32 structHash = keccak256( + abi.encode(VOTE_TYPEHASH, proposalId, support, keccak256(abi.encodePacked(operatedPoolIds))) ); + bytes32 digest = keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash)); + address signatory = ecrecover(digest, v, r, s); + + return _castVote(signatory, proposalId, support, operatedPoolIds); } /// @dev Executes a proposal that has passed and is @@ -373,4 +393,60 @@ contract ZrxTreasury is .safeAdd(votingPeriod); return block.timestamp > voteEndTime; } + + /// @dev Casts a vote for the given proposal. Only callable + /// during the voting period for that proposal. See + /// `getVotingPower` for how voting power is computed. + function _castVote( + address voter, + uint256 proposalId, + bool support, + bytes32[] memory operatedPoolIds + ) + private + { + if (proposalId >= proposalCount()) { + revert("_castVote/INVALID_PROPOSAL_ID"); + } + if (hasVoted[proposalId][voter]) { + revert("_castVote/ALREADY_VOTED"); + } + + Proposal memory proposal = proposals[proposalId]; + if ( + proposal.voteEpoch != stakingProxy.currentEpoch() || + _hasVoteEnded(proposal.voteEpoch) + ) { + revert("_castVote/VOTING_IS_CLOSED"); + } + + uint256 votingPower = getVotingPower(voter, operatedPoolIds); + if (votingPower == 0) { + revert("_castVote/NO_VOTING_POWER"); + } + + if (support) { + proposals[proposalId].votesFor = proposals[proposalId].votesFor + .safeAdd(votingPower); + } else { + proposals[proposalId].votesAgainst = proposals[proposalId].votesAgainst + .safeAdd(votingPower); + } + hasVoted[proposalId][voter] = true; + + emit VoteCast( + voter, + operatedPoolIds, + proposalId, + support, + votingPower + ); + } + + /// @dev Gets the Ethereum chain id + function _getChainId() private pure returns (uint256) { + uint256 chainId; + assembly { chainId := chainid() } + return chainId; + } } diff --git a/contracts/treasury/test/treasury_test.ts b/contracts/treasury/test/treasury_test.ts index dcf843e132..b2224686d2 100644 --- a/contracts/treasury/test/treasury_test.ts +++ b/contracts/treasury/test/treasury_test.ts @@ -17,8 +17,9 @@ import { randomAddress, verifyEventsFromLogs, } from '@0x/contracts-test-utils'; -import { BigNumber } from '@0x/utils'; -import * as _ from 'lodash'; +import { TreasuryVote } from '@0x/protocol-utils'; +import { BigNumber, hexUtils } from '@0x/utils'; +import * as ethUtil from 'ethereumjs-util'; import { artifacts } from './artifacts'; import { DefaultPoolOperatorContract, ZrxTreasuryContract, ZrxTreasuryEvents } from './wrappers'; @@ -55,6 +56,8 @@ blockchainTests.resets('Treasury governance', env => { let nonDefaultPoolId: string; let poolOperator: string; let delegator: string; + let relayer: string; + let delegatorPrivateKey: string; let actions: ProposedAction[]; async function deployStakingAsync(): Promise { @@ -105,7 +108,10 @@ blockchainTests.resets('Treasury governance', env => { } before(async () => { - [admin, poolOperator, delegator] = await env.getAccountAddressesAsync(); + const accounts = await env.getAccountAddressesAsync(); + [admin, poolOperator, delegator, relayer] = accounts; + delegatorPrivateKey = hexUtils.toHex(constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(delegator)]); + zrx = await DummyERC20TokenContract.deployFrom0xArtifactAsync( erc20Artifacts.DummyERC20Token, env.provider, @@ -399,7 +405,7 @@ blockchainTests.resets('Treasury governance', env => { expect(await treasury.proposalCount().callAsync()).to.bignumber.equal(1); }); }); - describe('castVote()', () => { + describe('castVote() and castVoteBySignature()', () => { const VOTE_PROPOSAL_ID = new BigNumber(0); const DELEGATOR_VOTING_POWER = new BigNumber(420); @@ -418,17 +424,18 @@ blockchainTests.resets('Treasury governance', env => { .propose(actions, currentEpoch.plus(2), PROPOSAL_DESCRIPTION, []) .awaitTransactionSuccessAsync({ from: delegator }); }); + // castVote() it('Cannot vote on invalid proposalId', async () => { await fastForwardToNextEpochAsync(); await fastForwardToNextEpochAsync(); const tx = treasury .castVote(INVALID_PROPOSAL_ID, true, []) .awaitTransactionSuccessAsync({ from: delegator }); - return expect(tx).to.revertWith('castVote/INVALID_PROPOSAL_ID'); + return expect(tx).to.revertWith('_castVote/INVALID_PROPOSAL_ID'); }); it('Cannot vote before voting period starts', async () => { const tx = treasury.castVote(VOTE_PROPOSAL_ID, true, []).awaitTransactionSuccessAsync({ from: delegator }); - return expect(tx).to.revertWith('castVote/VOTING_IS_CLOSED'); + return expect(tx).to.revertWith('_castVote/VOTING_IS_CLOSED'); }); it('Cannot vote after voting period ends', async () => { await fastForwardToNextEpochAsync(); @@ -436,14 +443,14 @@ blockchainTests.resets('Treasury governance', env => { await env.web3Wrapper.increaseTimeAsync(TREASURY_PARAMS.votingPeriod.plus(1).toNumber()); await env.web3Wrapper.mineBlockAsync(); const tx = treasury.castVote(VOTE_PROPOSAL_ID, true, []).awaitTransactionSuccessAsync({ from: delegator }); - return expect(tx).to.revertWith('castVote/VOTING_IS_CLOSED'); + return expect(tx).to.revertWith('_castVote/VOTING_IS_CLOSED'); }); it('Cannot vote twice on same proposal', async () => { await fastForwardToNextEpochAsync(); await fastForwardToNextEpochAsync(); await treasury.castVote(VOTE_PROPOSAL_ID, true, []).awaitTransactionSuccessAsync({ from: delegator }); const tx = treasury.castVote(VOTE_PROPOSAL_ID, false, []).awaitTransactionSuccessAsync({ from: delegator }); - return expect(tx).to.revertWith('castVote/ALREADY_VOTED'); + return expect(tx).to.revertWith('_castVote/ALREADY_VOTED'); }); it('Can cast a valid vote', async () => { await fastForwardToNextEpochAsync(); @@ -465,6 +472,110 @@ blockchainTests.resets('Treasury governance', env => { ZrxTreasuryEvents.VoteCast, ); }); + // castVoteBySignature() + it('Cannot vote by signature on invalid proposalId', async () => { + await fastForwardToNextEpochAsync(); + await fastForwardToNextEpochAsync(); + const vote = new TreasuryVote({ + proposalId: INVALID_PROPOSAL_ID, + verifyingContract: admin, + }); + const signature = vote.getSignatureWithKey(delegatorPrivateKey); + const tx = treasury + .castVoteBySignature(INVALID_PROPOSAL_ID, true, [], signature.v, signature.r, signature.s) + .awaitTransactionSuccessAsync({ from: relayer }); + return expect(tx).to.revertWith('_castVote/INVALID_PROPOSAL_ID'); + }); + it('Cannot vote by signature before voting period starts', async () => { + const vote = new TreasuryVote({ + proposalId: VOTE_PROPOSAL_ID, + verifyingContract: admin, + }); + const signature = vote.getSignatureWithKey(delegatorPrivateKey); + const tx = treasury + .castVoteBySignature(VOTE_PROPOSAL_ID, true, [], signature.v, signature.r, signature.s) + .awaitTransactionSuccessAsync({ from: relayer }); + return expect(tx).to.revertWith('_castVote/VOTING_IS_CLOSED'); + }); + it('Cannot vote by signature after voting period ends', async () => { + await fastForwardToNextEpochAsync(); + await fastForwardToNextEpochAsync(); + await env.web3Wrapper.increaseTimeAsync(TREASURY_PARAMS.votingPeriod.plus(1).toNumber()); + await env.web3Wrapper.mineBlockAsync(); + + const vote = new TreasuryVote({ + proposalId: VOTE_PROPOSAL_ID, + verifyingContract: admin, + }); + const signature = vote.getSignatureWithKey(delegatorPrivateKey); + const tx = treasury + .castVoteBySignature(VOTE_PROPOSAL_ID, true, [], signature.v, signature.r, signature.s) + .awaitTransactionSuccessAsync({ from: relayer }); + return expect(tx).to.revertWith('_castVote/VOTING_IS_CLOSED'); + }); + it('Can recover the address from signature correctly', async () => { + const vote = new TreasuryVote({ + proposalId: VOTE_PROPOSAL_ID, + verifyingContract: admin, + }); + const signature = vote.getSignatureWithKey(delegatorPrivateKey); + const publicKey = ethUtil.ecrecover( + ethUtil.toBuffer(vote.getEIP712Hash()), + signature.v, + ethUtil.toBuffer(signature.r), + ethUtil.toBuffer(signature.s), + ); + const address = ethUtil.publicToAddress(publicKey); + + expect(ethUtil.bufferToHex(address)).to.be.equal(delegator); + }); + it('Can cast a valid vote by signature', async () => { + await fastForwardToNextEpochAsync(); + await fastForwardToNextEpochAsync(); + + const vote = new TreasuryVote({ + proposalId: VOTE_PROPOSAL_ID, + verifyingContract: treasury.address, + chainId: 1337, + support: false, + }); + const signature = vote.getSignatureWithKey(delegatorPrivateKey); + const tx = await treasury + .castVoteBySignature(VOTE_PROPOSAL_ID, false, [], signature.v, signature.r, signature.s) + .awaitTransactionSuccessAsync({ from: relayer }); + + verifyEventsFromLogs( + tx.logs, + [ + { + voter: delegator, + operatedPoolIds: [], + proposalId: VOTE_PROPOSAL_ID, + support: vote.support, + votingPower: DELEGATOR_VOTING_POWER, + }, + ], + ZrxTreasuryEvents.VoteCast, + ); + }); + it('Cannot vote by signature twice on same proposal', async () => { + await fastForwardToNextEpochAsync(); + await fastForwardToNextEpochAsync(); + await treasury.castVote(VOTE_PROPOSAL_ID, true, []) + .awaitTransactionSuccessAsync({ from: delegator }); + + const secondVote = new TreasuryVote({ + proposalId: VOTE_PROPOSAL_ID, + verifyingContract: treasury.address, + chainId: 1337, + support: false, + }); + const signature = secondVote.getSignatureWithKey(delegatorPrivateKey); + const secondVoteTx = treasury + .castVoteBySignature(VOTE_PROPOSAL_ID, false, [], signature.v, signature.r, signature.s) + .awaitTransactionSuccessAsync({ from: relayer }); + return expect(secondVoteTx).to.revertWith('_castVote/ALREADY_VOTED'); + }); }); describe('execute()', () => { let passedProposalId: BigNumber; @@ -473,7 +584,7 @@ blockchainTests.resets('Treasury governance', env => { let ongoingVoteProposalId: BigNumber; before(async () => { - // OPerator has enough ZRX to create and pass a proposal + // Operator has enough ZRX to create and pass a proposal await staking.stake(TREASURY_PARAMS.quorumThreshold).awaitTransactionSuccessAsync({ from: poolOperator }); await staking .moveStake( @@ -549,7 +660,7 @@ blockchainTests.resets('Treasury governance', env => { }); it('Cannot execute before or after the execution epoch', async () => { const tooEarly = treasury.execute(passedProposalId, actions).awaitTransactionSuccessAsync(); - expect(tooEarly).to.revertWith('_assertProposalExecutable/CANNOT_EXECUTE_THIS_EPOCH'); + await expect(tooEarly).to.revertWith('_assertProposalExecutable/CANNOT_EXECUTE_THIS_EPOCH'); await fastForwardToNextEpochAsync(); // Proposal 0 is executable here await fastForwardToNextEpochAsync(); diff --git a/packages/protocol-utils/CHANGELOG.json b/packages/protocol-utils/CHANGELOG.json index c35732effb..e89c0dc42e 100644 --- a/packages/protocol-utils/CHANGELOG.json +++ b/packages/protocol-utils/CHANGELOG.json @@ -1,4 +1,12 @@ [ + { + "version": "1.9.0", + "changes": [ + { + "note": "Add 'TreasuryVote' class" + } + ] + }, { "timestamp": 1630459879, "version": "1.8.4", diff --git a/packages/protocol-utils/package.json b/packages/protocol-utils/package.json index 8a14fda3a7..e725dacebd 100644 --- a/packages/protocol-utils/package.json +++ b/packages/protocol-utils/package.json @@ -67,7 +67,7 @@ "@0x/contract-wrappers": "^13.17.6", "@0x/json-schemas": "^6.1.3", "@0x/subproviders": "^6.5.3", - "@0x/utils": "^6.4.3", + "@0x/utils": "^6.4.4", "@0x/web3-wrapper": "^7.5.3", "chai": "^4.0.1", "ethereumjs-util": "^7.0.10", diff --git a/packages/protocol-utils/src/index.ts b/packages/protocol-utils/src/index.ts index a368e8fa72..dabf725aae 100644 --- a/packages/protocol-utils/src/index.ts +++ b/packages/protocol-utils/src/index.ts @@ -9,3 +9,4 @@ export * from './signature_utils'; export * from './transformer_utils'; export * from './constants'; export * from './vip_utils'; +export * from './treasury_votes'; diff --git a/packages/protocol-utils/src/treasury_votes.ts b/packages/protocol-utils/src/treasury_votes.ts new file mode 100644 index 0000000000..956864c732 --- /dev/null +++ b/packages/protocol-utils/src/treasury_votes.ts @@ -0,0 +1,97 @@ +import { BigNumber, hexUtils, NULL_ADDRESS } from '@0x/utils'; +import * as ethUtil from 'ethereumjs-util'; + +import { ZERO } from './constants'; +import { EIP712_DOMAIN_PARAMETERS, getTypeHash } from './eip712_utils'; +import { eip712SignHashWithKey, Signature } from './signature_utils'; + +const VOTE_DEFAULT_VALUES = { + proposalId: ZERO, + support: false, + operatedPoolIds: [] as string[], + chainId: 1, + version: '1.0.0', + verifyingContract: NULL_ADDRESS, +}; + +export type TreasuryVoteFields = typeof VOTE_DEFAULT_VALUES; + +export class TreasuryVote { + public static readonly CONTRACT_NAME = 'Zrx Treasury'; + + public static readonly MESSAGE_STRUCT_NAME = 'TreasuryVote'; + public static readonly MESSAGE_STRUCT_ABI = [ + { type: 'uint256', name: 'proposalId' }, + { type: 'bool', name: 'support' }, + { type: 'bytes32[]', name: 'operatedPoolIds' }, + ]; + public static readonly MESSAGE_TYPE_HASH = getTypeHash( + TreasuryVote.MESSAGE_STRUCT_NAME, TreasuryVote.MESSAGE_STRUCT_ABI, + ); + + public static readonly DOMAIN_STRUCT_NAME = 'EIP712Domain'; + public static readonly DOMAIN_TYPE_HASH = getTypeHash( + TreasuryVote.DOMAIN_STRUCT_NAME, EIP712_DOMAIN_PARAMETERS, + ); + + public proposalId: BigNumber; + public support: boolean; + public operatedPoolIds: string[]; + public chainId: number; + public version: string; + public verifyingContract: string; + + constructor(fields: Partial = {}) { + const _fields = { ...VOTE_DEFAULT_VALUES, ...fields }; + this.proposalId = _fields.proposalId; + this.support = _fields.support; + this.operatedPoolIds = _fields.operatedPoolIds; + this.chainId = _fields.chainId; + this.version = _fields.version; + this.verifyingContract = _fields.verifyingContract; + } + + public getDomainHash(): string { + return hexUtils.hash( + hexUtils.concat( + hexUtils.leftPad(TreasuryVote.DOMAIN_TYPE_HASH), + hexUtils.hash( + hexUtils.toHex(Buffer.from(TreasuryVote.CONTRACT_NAME)), + ), + hexUtils.leftPad(this.chainId), + hexUtils.hash( + hexUtils.toHex(Buffer.from(this.version)), + ), + hexUtils.leftPad(this.verifyingContract), + ), + ); + } + + public getStructHash(): string { + return hexUtils.hash( + hexUtils.concat( + hexUtils.leftPad(TreasuryVote.MESSAGE_TYPE_HASH), + hexUtils.leftPad(this.proposalId), + hexUtils.leftPad(this.support ? 1 : 0), + hexUtils.hash( + ethUtil.toBuffer(hexUtils.concat(...this.operatedPoolIds.map(id => hexUtils.leftPad(id)))), + ), + ), + ); + } + + public getEIP712Hash(): string { + return hexUtils.hash( + hexUtils.toHex( + hexUtils.concat( + '0x1901', + this.getDomainHash(), + this.getStructHash()), + ), + ); + } + + public getSignatureWithKey(privateKey: string): Signature { + return eip712SignHashWithKey(this.getEIP712Hash(), privateKey); + } +}