From fe0c26387c53b48fe64eab6936e62575f96bb5b8 Mon Sep 17 00:00:00 2001 From: mzhu25 Date: Wed, 28 Apr 2021 10:22:52 -0700 Subject: [PATCH] Fix treasury voting power calculation (#214) * Fix treasury voting power calculation * Update changelog --- contracts/treasury/CHANGELOG.json | 9 ++++++ .../treasury/contracts/src/IZrxTreasury.sol | 1 + .../treasury/contracts/src/ZrxTreasury.sol | 20 ++++++------- contracts/treasury/test/treasury_test.ts | 30 +++++++++++++++---- 4 files changed, 44 insertions(+), 16 deletions(-) diff --git a/contracts/treasury/CHANGELOG.json b/contracts/treasury/CHANGELOG.json index 740c468db9..f1a68da07f 100644 --- a/contracts/treasury/CHANGELOG.json +++ b/contracts/treasury/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "1.1.5", + "changes": [ + { + "note": "Patched votingPower logic", + "pr": 214 + } + ] + }, { "timestamp": 1619596077, "version": "1.1.4", diff --git a/contracts/treasury/contracts/src/IZrxTreasury.sol b/contracts/treasury/contracts/src/IZrxTreasury.sol index c16160f35f..8baea3bba1 100644 --- a/contracts/treasury/contracts/src/IZrxTreasury.sol +++ b/contracts/treasury/contracts/src/IZrxTreasury.sol @@ -30,6 +30,7 @@ interface IZrxTreasury { uint256 votingPeriod; uint256 proposalThreshold; uint256 quorumThreshold; + bytes32 defaultPoolId; } struct ProposedAction { diff --git a/contracts/treasury/contracts/src/ZrxTreasury.sol b/contracts/treasury/contracts/src/ZrxTreasury.sol index d450bfd2e6..5097f4897c 100644 --- a/contracts/treasury/contracts/src/ZrxTreasury.sol +++ b/contracts/treasury/contracts/src/ZrxTreasury.sol @@ -20,8 +20,6 @@ pragma solidity ^0.6.12; pragma experimental ABIEncoderV2; -import "@0x/contracts-erc20/contracts/src/v06/IERC20TokenV06.sol"; -import "@0x/contracts-erc20/contracts/src/v06/LibERC20TokenV06.sol"; import "@0x/contracts-utils/contracts/src/v06/LibBytesV06.sol"; import "@0x/contracts-utils/contracts/src/v06/LibSafeMathV06.sol"; import "@0x/contracts-utils/contracts/src/v06/errors/LibRichErrorsV06.sol"; @@ -32,7 +30,6 @@ import "./IZrxTreasury.sol"; contract ZrxTreasury is IZrxTreasury { - using LibERC20TokenV06 for IERC20TokenV06; using LibSafeMathV06 for uint256; using LibRichErrorsV06 for bytes; using LibBytesV06 for bytes; @@ -52,11 +49,9 @@ contract ZrxTreasury is /// @dev Initializes the ZRX treasury and creates the default /// staking pool. /// @param stakingProxy_ The 0x staking proxy contract. - /// @param weth_ The WETH token contract. /// @param params Immutable treasury parameters. constructor( IStaking stakingProxy_, - IERC20TokenV06 weth_, TreasuryParameters memory params ) public @@ -66,15 +61,12 @@ contract ZrxTreasury is "VOTING_PERIOD_TOO_LONG" ); stakingProxy = stakingProxy_; - DefaultPoolOperator defaultPoolOperator_ = new DefaultPoolOperator( - stakingProxy_, - weth_ - ); - defaultPoolOperator = defaultPoolOperator_; - defaultPoolId = defaultPoolOperator_.poolId(); votingPeriod = params.votingPeriod; proposalThreshold = params.proposalThreshold; quorumThreshold = params.quorumThreshold; + defaultPoolId = params.defaultPoolId; + IStaking.Pool memory defaultPool = stakingProxy_.getStakingPool(params.defaultPoolId); + defaultPoolOperator = DefaultPoolOperator(defaultPool.operator); } // solhint-disable @@ -286,6 +278,12 @@ contract ZrxTreasury is // Add voting power for operated staking pools. for (uint256 i = 0; i != operatedPoolIds.length; i++) { + for (uint256 j = 0; j != i; j++) { + require( + operatedPoolIds[i] != operatedPoolIds[j], + "getVotingPower/DUPLICATE_POOL_ID" + ); + } IStaking.Pool memory pool = stakingProxy.getStakingPool(operatedPoolIds[i]); require( pool.operator == account, diff --git a/contracts/treasury/test/treasury_test.ts b/contracts/treasury/test/treasury_test.ts index 33d2667490..dcf843e132 100644 --- a/contracts/treasury/test/treasury_test.ts +++ b/contracts/treasury/test/treasury_test.ts @@ -28,6 +28,7 @@ blockchainTests.resets('Treasury governance', env => { votingPeriod: new BigNumber(3).times(stakingConstants.ONE_DAY_IN_SECONDS), proposalThreshold: new BigNumber(100), quorumThreshold: new BigNumber(1000), + defaultPoolId: stakingConstants.INITIAL_POOL_ID, }; const PROPOSAL_DESCRIPTION = 'A very compelling proposal!'; const TREASURY_BALANCE = constants.INITIAL_ERC20_BALANCE; @@ -135,6 +136,16 @@ blockchainTests.resets('Treasury governance', env => { .approve(erc20ProxyContract.address, constants.INITIAL_ERC20_ALLOWANCE) .awaitTransactionSuccessAsync({ from: delegator }); + defaultPoolOperator = await DefaultPoolOperatorContract.deployFrom0xArtifactAsync( + artifacts.DefaultPoolOperator, + env.provider, + env.txDefaults, + { ...artifacts, ...erc20Artifacts }, + staking.address, + weth.address, + ); + defaultPoolId = stakingConstants.INITIAL_POOL_ID; + const createStakingPoolTx = staking.createStakingPool(stakingConstants.PPM, false); nonDefaultPoolId = await createStakingPoolTx.callAsync({ from: poolOperator }); await createStakingPoolTx.awaitTransactionSuccessAsync({ from: poolOperator }); @@ -145,9 +156,9 @@ blockchainTests.resets('Treasury governance', env => { env.txDefaults, { ...artifacts, ...erc20Artifacts }, staking.address, - weth.address, TREASURY_PARAMS, ); + await zrx.mint(TREASURY_BALANCE).awaitTransactionSuccessAsync(); await zrx.transfer(treasury.address, TREASURY_BALANCE).awaitTransactionSuccessAsync(); actions = [ @@ -166,10 +177,6 @@ blockchainTests.resets('Treasury governance', env => { value: constants.ZERO_AMOUNT, }, ]; - - defaultPoolId = await treasury.defaultPoolId().callAsync(); - const defaultPoolOperatorAddress = await treasury.defaultPoolOperator().callAsync(); - defaultPoolOperator = new DefaultPoolOperatorContract(defaultPoolOperatorAddress, env.provider, env.txDefaults); }); describe('getVotingPower()', () => { it('Unstaked ZRX has no voting power', async () => { @@ -222,6 +229,19 @@ blockchainTests.resets('Treasury governance', env => { const operatorVotingPower = await treasury.getVotingPower(poolOperator, [nonDefaultPoolId]).callAsync(); expect(operatorVotingPower).to.bignumber.equal(TREASURY_PARAMS.proposalThreshold.dividedBy(2)); }); + it('Reverts if given duplicate pool IDs', async () => { + await staking.stake(TREASURY_PARAMS.proposalThreshold).awaitTransactionSuccessAsync({ from: delegator }); + await staking + .moveStake( + new StakeInfo(StakeStatus.Undelegated), + new StakeInfo(StakeStatus.Delegated, nonDefaultPoolId), + TREASURY_PARAMS.proposalThreshold, + ) + .awaitTransactionSuccessAsync({ from: delegator }); + await fastForwardToNextEpochAsync(); + const tx = treasury.getVotingPower(poolOperator, [nonDefaultPoolId, nonDefaultPoolId]).callAsync(); + return expect(tx).to.revertWith('getVotingPower/DUPLICATE_POOL_ID'); + }); it('Correctly sums voting power delegated to multiple pools', async () => { await staking .stake(TREASURY_PARAMS.proposalThreshold.times(2))