Support cast vote by signature in ZrxTreasury (#297)

* Support cast vote by signature in ZrxTreasury

* Address comments and fix existing tests

* test that doesnt work

* test file format

* updates

* address some of the comments

* Remove unused const

* get rid of vote_factory

* unit test for castVoteBySignature p1

* unit test for castVoteBySignature p2

* Add version to domain, and one more test

* unit test for castVoteBySignature p3

* unit test for castVoteBySignature p4

* bump utils version

* remove debug code

* address some comments

* address more pr comments

* move Vote class to protocol-utils

* Address pr comments and update changelogs
This commit is contained in:
Cece Z 2021-09-14 16:12:51 -04:00 committed by GitHub
parent 15fb00e958
commit d7dbc0576d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 377 additions and 52 deletions

3
.gitignore vendored
View File

@ -75,8 +75,9 @@ generated_docs/
TODO.md
# VSCode file
# IDE file
.vscode
.idea
# generated contract artifacts/
contracts/broker/generated-artifacts/

View File

@ -1,4 +1,12 @@
[
{
"version": "1.4.0",
"changes": [
{
"note": "Support cast vote by signature in Treasury"
}
]
},
{
"timestamp": 1631120757,
"version": "1.3.5",

View File

@ -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.

View File

@ -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;
}
}

View File

@ -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<void> {
@ -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();

View File

@ -1,4 +1,12 @@
[
{
"version": "1.9.0",
"changes": [
{
"note": "Add 'TreasuryVote' class"
}
]
},
{
"timestamp": 1630459879,
"version": "1.8.4",

View File

@ -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",

View File

@ -9,3 +9,4 @@ export * from './signature_utils';
export * from './transformer_utils';
export * from './constants';
export * from './vip_utils';
export * from './treasury_votes';

View File

@ -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<TreasuryVoteFields> = {}) {
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);
}
}