Fix treasury voting power calculation (#214)

* Fix treasury voting power calculation

* Update changelog

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

contracts/treasury/contracts/src/IZrxTreasury.sol

@@ -30,6 +30,7 @@ interface IZrxTreasury {
         uint256 votingPeriod;
         uint256 proposalThreshold;
         uint256 quorumThreshold;
+        bytes32 defaultPoolId;
     }
 
     struct ProposedAction {

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,

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))