@0x:contracts-staking Addressed some review comments

contracts/staking/contracts/src/StakingProxy.sol

@@ -33,13 +33,16 @@ contract StakingProxy is
 
     /// @dev Constructor.
     /// @param _stakingContract Staking contract to delegate calls to.
-    constructor(address _stakingContract, address _readOnlyProxy)
+    /// @param _readOnlyProxy The address of the read only proxy.
+    /// @param _wethProxyAddress The address that can transfer WETH for fees.
+    constructor(address _stakingContract, address _readOnlyProxy, address _wethProxyAddress)
         public
         MixinStorage()
     {
         stakingContract = _stakingContract;
         readOnlyProxyCallee = _stakingContract;
         readOnlyProxy = _readOnlyProxy;
+        wethAssetProxy = IAssetProxy(_wethProxyAddress);
     }
 
     /// @dev Delegates calls to the staking contract, if it is set.

contracts/staking/contracts/src/fees/MixinExchangeFees.sol

@@ -19,8 +19,6 @@
 pragma solidity ^0.5.9;
 pragma experimental ABIEncoderV2;
 
-import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetData.sol";
-import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetProxy.sol";
 import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol";
 import "@0x/contracts-utils/contracts/src/LibRichErrors.sol";
 import "@0x/contracts-utils/contracts/src/LibSafeMath.sol";
@@ -105,6 +103,9 @@ contract MixinExchangeFees is
             (msg.value != protocolFeePaid && msg.value != 0)
         ) {
             LibRichErrors.rrevert(LibStakingRichErrors.InvalidProtocolFeePaymentError(
+                protocolFeePaid == 0 ?
+                    LibStakingRichErrors.ProtocolFeePaymentErrorCodes.ZeroProtocolFeePaid :
+                    LibStakingRichErrors.ProtocolFeePaymentErrorCodes.MismatchedFeeAndPayment,
                 protocolFeePaid,
                 msg.value
             ));
@@ -112,7 +113,7 @@ contract MixinExchangeFees is
 
         // Transfer the protocol fee to this address if it should be paid in WETH.
         if (msg.value == 0) {
-            erc20Proxy.transferFrom(
+            wethAssetProxy.transferFrom(
                 WETH_ASSET_DATA,
                 payerAddress,
                 address(this),

contracts/staking/contracts/src/fees/MixinExchangeManager.sol

@@ -45,17 +45,6 @@ contract MixinExchangeManager is
         _;
     }
 
-    /// @dev Adds a new erc20 proxy.
-    /// @param erc20AssetProxy The asset proxy that will transfer erc20 tokens.
-    function addERC20AssetProxy(address erc20AssetProxy)
-        external
-        onlyOwner
-    {
-        // Update the erc20 asset proxy.
-        erc20Proxy = IAssetProxy(erc20AssetProxy);
-        emit ERC20AssetProxy(erc20AssetProxy);
-    }
-
     /// @dev Adds a new exchange address
     /// @param addr Address of exchange contract to add
     function addExchangeAddress(address addr)

contracts/staking/contracts/src/immutable/MixinConstants.sol

@@ -34,8 +34,6 @@ contract MixinConstants is
 
     bytes32 constant internal NIL_POOL_ID = 0x0000000000000000000000000000000000000000000000000000000000000000;
 
-    bytes32 constant internal NIL_POOL_ID = 0x0000000000000000000000000000000000000000000000000000000000000000;
-
     address constant internal NIL_ADDRESS = 0x0000000000000000000000000000000000000000;
 
     bytes32 constant internal UNKNOWN_STAKING_POOL_ID = 0x0000000000000000000000000000000000000000000000000000000000000000;
@@ -45,10 +43,4 @@ contract MixinConstants is
     uint64 constant internal INITIAL_TIMELOCK_PERIOD = INITIAL_EPOCH;
 
     uint256 constant internal MIN_TOKEN_VALUE = 10**18;
-
-    // The address of the canonical WETH contract -- this will be used as an alternative to ether for paying protocol fees.
-    address constant internal WETH_ADDRESS = address(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);
-
-    // abi.encodeWithSelector(IAssetData(address(0)).ERC20Token.selector, WETH_ADDRESS)
-    bytes constant internal WETH_ASSET_DATA = hex"f47261b0000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2";
 }

contracts/staking/contracts/src/immutable/MixinDeploymentConstants.sol

@@ -33,4 +33,22 @@ contract MixinDeploymentConstants {
     uint256 constant internal CHAIN_ID = 1;
 
     uint256 constant internal MAX_MAKERS_IN_POOL = 10;
+
+    // Mainnet WETH9 Address
+    address constant internal WETH_ADDRESS = address(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);
+
+    // Kovan WETH9 Address
+    // address constant internal WETH_ADDRESS = address(0xd0a1e359811322d97991e03f863a0c30c2cf029c);
+
+    // Ropsten & Rinkeby WETH9 Address
+    // address constant internal WETH_ADDRESS = address(0xc778417e063141139fce010982780140aa0cd5ab);
+
+    // Mainnet Weth Asset Data
+    bytes constant internal WETH_ASSET_DATA = hex"f47261b0000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2";
+
+    // Kovan Weth Asset Data
+    // bytes constant internal WETH_ASSET_DATA = hex"f47261b0000000000000000000000000d0a1e359811322d97991e03f863a0c30c2cf029c";
+
+    // Ropsten & Rinkeby Weth Asset Data
+    // bytes constant internal WETH_ASSET_DATA = hex"f47261b0000000000000000000000000c778417e063141139fce010982780140aa0cd5ab";
 }

contracts/staking/contracts/src/immutable/MixinStorage.sol

@@ -20,12 +20,15 @@ pragma solidity ^0.5.9;
 
 import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetData.sol";
 import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetProxy.sol";
+import "@0x/contracts-utils/contracts/src/LibBytes.sol";
+import "@0x/contracts-utils/contracts/src/LibRichErrors.sol";
 import "@0x/contracts-utils/contracts/src/Ownable.sol";
 import "./MixinConstants.sol";
 import "../interfaces/IZrxVault.sol";
 import "../interfaces/IEthVault.sol";
 import "../interfaces/IStakingPoolRewardVault.sol";
 import "../interfaces/IStructs.sol";
+import "../libs/LibStakingRichErrors.sol";
 
 
 // solhint-disable max-states-count, no-empty-blocks
@@ -34,13 +37,23 @@ contract MixinStorage is
     Ownable,
     MixinConstants
 {
+    using LibBytes for bytes;
+
+    /// @dev Ensures that the WETH_ASSET_DATA is correct.
     constructor()
         public
         Ownable()
-    {} // solhint-disable-line no-empty-blocks
+    {
+        // Ensure that the WETH_ASSET_DATA from MixinDeploymentConstants is correct.
+        if (!WETH_ASSET_DATA.equals(
+            abi.encodeWithSelector(IAssetData(address(0)).ERC20Token.selector, WETH_ADDRESS)
+        )) {
+            LibRichErrors.rrevert(LibStakingRichErrors.InvalidWethAssetDataError());
+        }
+    }
 
-    // 0x ERC20 Proxy
+    // WETH Asset Proxy
-    IAssetProxy internal erc20Proxy;
+    IAssetProxy internal wethAssetProxy;
 
     // address of staking contract
     address internal stakingContract;

contracts/staking/contracts/src/interfaces/IStakingEvents.sol

@@ -126,10 +126,4 @@ interface IStakingEvents {
     event StakingPoolRewardVaultChanged(
         address rewardVaultAddress
     );
-
-    /// @dev Emitted by MixinExchangeManager when the erc20AssetProxy address changes.
-    /// @param erc20AddressProxy The new erc20 asset proxy address.
-    event ERC20AssetProxy(
-        address erc20AddressProxy
-    );
 }

contracts/staking/contracts/src/interfaces/IZrxVault.sol

@@ -48,17 +48,17 @@ interface IZrxVault {
         uint256 amount
     );
 
-    /// @dev Emitted when the ERC20 Proxy is changed.
+    /// @dev Emitted when the Zrx Proxy is changed.
-    /// @param erc20ProxyAddress Address of the new ERC20 proxy.
+    /// @param zrxProxyAddress Address of the new ERC20 proxy.
-    event Erc20ProxyChanged(
+    event ZrxProxyChanged(
-        address erc20ProxyAddress
+        address zrxProxyAddress
     );
 
-    /// @dev Sets the ERC20 proxy.
+    /// @dev Sets the Zrx proxy.
     /// Note that only the contract owner can call this.
     /// Note that this can only be called when *not* in Catastrophic Failure mode.
-    /// @param erc20ProxyAddress Address of the 0x ERC20 Proxy.
+    /// @param zrxProxyAddress Address of the 0x Zrx Asset Proxy.
-    function setErc20Proxy(address erc20ProxyAddress)
+    function setZrxProxy(address zrxProxyAddress)
         external;
 
     /// @dev Deposit an `amount` of Zrx Tokens from `owner` into the vault.

contracts/staking/contracts/src/libs/LibStakingRichErrors.sol

@@ -22,6 +22,12 @@ import "@0x/contracts-utils/contracts/src/LibRichErrors.sol";
 
 
 library LibStakingRichErrors {
+
+    enum ProtocolFeePaymentErrorCodes {
+        ZeroProtocolFeePaid,
+        MismatchedFeeAndPayment
+    }
+
     // bytes4(keccak256("MiscalculatedRewardsError(uint256,uint256)"))
     bytes4 internal constant MISCALCULATED_REWARDS_ERROR_SELECTOR =
         0xf7806c4e;
@@ -113,9 +119,13 @@ library LibStakingRichErrors {
         POOL_IS_FULL
     }
 
-    // bytes4(keccak256("InvalidProtocolFeePaymentError(uint256,uint256)"))
+    // bytes4(keccak256("InvalidProtocolFeePaymentError(uint8,uint256,uint256)"))
     bytes4 internal constant INVALID_PROTOCOL_FEE_PAYMENT_ERROR_SELECTOR =
-        0x31d7a505;
+        0xefd6cb33;
+
+    // bytes4(keccak256("InvalidWethAssetDataError()"))
+    bytes internal constant INVALID_WETH_ASSET_DATA_ERROR =
+        hex"24bf322c";
 
     // solhint-disable func-name-mixedcase
     function MiscalculatedRewardsError(
@@ -364,6 +374,7 @@ library LibStakingRichErrors {
     }
 
     function InvalidProtocolFeePaymentError(
+        ProtocolFeePaymentErrorCodes errorCode,
         uint256 expectedProtocolFeePaid,
         uint256 actualProtocolFeePaid
     )
@@ -373,6 +384,7 @@ library LibStakingRichErrors {
     {
         return abi.encodeWithSelector(
             INVALID_PROTOCOL_FEE_PAYMENT_ERROR_SELECTOR,
+            errorCode,
             expectedProtocolFeePaid,
             actualProtocolFeePaid
         );
@@ -407,4 +419,11 @@ library LibStakingRichErrors {
         return PROXY_DESTINATION_CANNOT_BE_NIL;
     }
 
+    function InvalidWethAssetDataError()
+        internal
+        pure
+        returns (bytes memory)
+    {
+        return INVALID_WETH_ASSET_DATA_ERROR;
+    }
 }

contracts/staking/contracts/src/vaults/ZrxVault.sol

@@ -42,8 +42,8 @@ contract ZrxVault is
     // mapping from Owner to ZRX balance
     mapping (address => uint256) internal balances;
 
-    // 0x ERC20 Proxy
+    // Zrx Asset Proxy
-    IAssetProxy internal erc20Proxy;
+    IAssetProxy internal zrxAssetProxy;
 
     // Zrx Token
     IERC20Token internal zrxToken;
@@ -52,15 +52,15 @@ contract ZrxVault is
     bytes internal zrxAssetData;
 
     /// @dev Constructor.
-    /// @param erc20ProxyAddress Address of the 0x ERC20 Proxy.
+    /// @param zrxProxyAddress Address of the 0x Zrx Proxy.
     /// @param zrxTokenAddress Address of the Zrx Token.
     constructor(
-        address erc20ProxyAddress,
+        address zrxProxyAddress,
         address zrxTokenAddress
     )
         public
     {
-        erc20Proxy = IAssetProxy(erc20ProxyAddress);
+        zrxAssetProxy = IAssetProxy(zrxProxyAddress);
         zrxToken = IERC20Token(zrxTokenAddress);
         zrxAssetData = abi.encodeWithSelector(
             IAssetData(address(0)).ERC20Token.selector,
@@ -68,17 +68,17 @@ contract ZrxVault is
         );
     }
 
-    /// @dev Sets the ERC20 proxy.
+    /// @dev Sets the Zrx proxy.
     /// Note that only the contract owner can call this.
     /// Note that this can only be called when *not* in Catastrophic Failure mode.
-    /// @param erc20ProxyAddress Address of the 0x ERC20 Proxy.
+    /// @param zrxProxyAddress Address of the 0x Zrx Proxy.
-    function setErc20Proxy(address erc20ProxyAddress)
+    function setZrxProxy(address zrxProxyAddress)
         external
         onlyOwner
         onlyNotInCatastrophicFailure
     {
-        erc20Proxy = IAssetProxy(erc20ProxyAddress);
+        zrxAssetProxy = IAssetProxy(zrxProxyAddress);
-        emit Erc20ProxyChanged(erc20ProxyAddress);
+        emit ZrxProxyChanged(zrxProxyAddress);
     }
 
     /// @dev Deposit an `amount` of Zrx Tokens from `owner` into the vault.
@@ -98,7 +98,7 @@ contract ZrxVault is
         emit ZrxDepositedIntoVault(msg.sender, owner, amount);
 
         // deposit ZRX from owner
-        erc20Proxy.transferFrom(
+        zrxAssetProxy.transferFrom(
             zrxAssetData,
             owner,
             address(this),

contracts/staking/contracts/test/TestProtocolFees.sol

@@ -19,16 +19,24 @@
 pragma solidity ^0.5.9;
 pragma experimental ABIEncoderV2;
 
+import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetProxy.sol";
 import "../src/Staking.sol";
 
 
 contract TestProtocolFees is
     Staking
 {
-    function setPoolIdOfMaker(bytes32 poolId, address makerAddress)
+    function setWethProxy(address wethProxyAddress)
         external
     {
-        poolIdByMakerAddress[makerAddress] = poolId;
+        wethAssetProxy = IAssetProxy(wethProxyAddress);
+    }
+
+    function addMakerToPool(bytes32 poolId, address makerAddress)
+        external
+    {
+        poolJoinedByMakerAddress[makerAddress].poolId = poolId;
+        poolJoinedByMakerAddress[makerAddress].confirmed = true;
     }
 
     function getActivePoolsByEpoch()

contracts/staking/contracts/test/TestStaking.sol

@@ -25,8 +25,10 @@ import "../src/Staking.sol";
 contract TestStaking is
     Staking
 {
+    /*
     // Stub out `payProtocolFee` to be the naive payProtocolFee function so that tests will
-    // not fail for WETH protocol fees.
+    // not fail for WETH protocol fees. These tests will fail otherwise because many of them
+    // will transfer
     function payProtocolFee(
         address makerAddress,
         address,
@@ -44,6 +46,7 @@ contract TestStaking is
             activePoolsThisEpoch.push(poolId);
         }
     }
+    */
 
     // Stub out `_unwrapWETH` to prevent the calls to `finalizeFees` from failing in tests
     // that do not relate to protocol fee payments in WETH.

contracts/staking/package.json

@@ -37,7 +37,7 @@
     },
     "config": {
         "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually.",
-        "abis": "./generated-artifacts/@(EthVault|IEthVault|IStaking|IStakingEvents|IStakingPoolRewardVault|IStakingProxy|IStructs|IVaultCore|IZrxVault|LibFixedMath|LibFixedMathRichErrors|LibProxy|LibSafeDowncast|LibStakingRichErrors|MixinConstants|MixinDeploymentConstants|MixinEthVault|MixinExchangeFees|MixinExchangeManager|MixinScheduler|MixinStake|MixinStakeBalances|MixinStakeStorage|MixinStakingPool|MixinStakingPoolRewardVault|MixinStakingPoolRewards|MixinStorage|MixinVaultCore|MixinZrxVault|ReadOnlyProxy|Staking|StakingPoolRewardVault|StakingProxy|TestCobbDouglas|TestLibFixedMath|TestStorageLayout|ZrxVault).json"
+        "abis": "./generated-artifacts/@(EthVault|IEthVault|IStaking|IStakingEvents|IStakingPoolRewardVault|IStakingProxy|IStructs|IVaultCore|IZrxVault|LibFixedMath|LibFixedMathRichErrors|LibProxy|LibSafeDowncast|LibStakingRichErrors|MixinConstants|MixinDeploymentConstants|MixinEthVault|MixinExchangeFees|MixinExchangeManager|MixinScheduler|MixinStake|MixinStakeBalances|MixinStakeStorage|MixinStakingPool|MixinStakingPoolRewardVault|MixinStakingPoolRewards|MixinStorage|MixinVaultCore|MixinZrxVault|ReadOnlyProxy|Staking|StakingPoolRewardVault|StakingProxy|TestCobbDouglas|TestLibFixedMath|TestProtocolFees|TestProtocolFeesERC20Proxy|TestStaking|TestStorageLayout|ZrxVault).json"
     },
     "repository": {
         "type": "git",

contracts/staking/test/actors/protocol_fee_actor.ts

@@ -0,0 +1,129 @@
+import { constants, expect } from '@0x/contracts-test-utils';
+import { StakingRevertErrors } from '@0x/order-utils';
+import { BigNumber } from '@0x/utils';
+import { LogWithDecodedArgs } from 'ethereum-types';
+
+import { TestProtocolFeesContract, TestProtocolFeesERC20ProxyTransferFromCalledEventArgs } from '../../src';
+
+export interface PayProtocolFeeArgs {
+    poolId: string;
+    makerAddress: string;
+    payerAddress: string;
+    protocolFeePaid: BigNumber;
+    from: string;
+    value: BigNumber;
+}
+
+// tslint:disable:no-unnecessary-type-assertion
+export class ProtocolFeeActor {
+    private readonly _exchanges: string[];
+    private readonly _registered_makers: string[];
+    private readonly _protocolFees: TestProtocolFeesContract;
+    private readonly _wethAssetData = '0xf47261b0000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2';
+
+    constructor(exchanges: string[], makers: string[], protocolFees: TestProtocolFeesContract) {
+        this._exchanges = exchanges;
+        this._protocolFees = protocolFees;
+        this._registered_makers = makers;
+    }
+
+    /**
+     * This function will test the `payProtocolFee()` function, and will revert if the behavior deviates
+     * whatsoever from the expected behavior.
+     * @param makerAddress The address of the order's maker.
+     * @param payerAddress The address that is responsible for paying the protocol fee.
+     * @param protocolFeePaid The fee that should be paid to the staking contract.
+     * @param from The address that should send the transaction.
+     * @param value The amount of value that should be sent in the transaction.
+     */
+    public async payProtocolFeeAsync(args: PayProtocolFeeArgs): Promise<void> {
+        // Get the original state to compare with afterwards
+        const originalActivePools = await this._protocolFees.getActivePoolsByEpoch.callAsync();
+        const originalProtocolFeesCollected = await this._protocolFees.getProtocolFeesThisEpochByPool.callAsync(
+            args.poolId,
+        );
+
+        // If the poolId is already registered, it should not be added to the active pools list. Otherwise, it should be added.
+        const shouldBeAdded = !originalActivePools.includes(args.poolId);
+
+        // Handle all of the failure cases.
+        const tx = this._protocolFees.payProtocolFee.awaitTransactionSuccessAsync(
+            args.makerAddress,
+            args.payerAddress,
+            args.protocolFeePaid,
+            { from: args.from, value: args.value },
+        );
+        if (!this._exchanges.includes(args.from)) {
+            const expectedError = new StakingRevertErrors.OnlyCallableByExchangeError(args.from);
+            return expect(tx, 'should revert when the `from` address is not a registered exchange').to.revertWith(
+                expectedError,
+            );
+        } else if (args.protocolFeePaid.eq(0)) {
+            const expectedError = new StakingRevertErrors.InvalidProtocolFeePaymentError(
+                StakingRevertErrors.ProtocolFeePaymentErrorCodes.ZeroProtocolFeePaid,
+                constants.ZERO_AMOUNT,
+                new BigNumber(args.value),
+            );
+            return expect(tx, 'should revert when the `protocolFeePaid` is zero').to.revertWith(expectedError);
+        } else if (!args.protocolFeePaid.eq(args.value) && !args.value.eq(0)) {
+            const expectedError = new StakingRevertErrors.InvalidProtocolFeePaymentError(
+                StakingRevertErrors.ProtocolFeePaymentErrorCodes.MismatchedFeeAndPayment,
+                args.protocolFeePaid,
+                new BigNumber(args.value),
+            );
+            return expect(tx, 'should revert when the `protocolFeePaid` and the value are mismatched').to.revertWith(
+                expectedError,
+            );
+        }
+
+        // Call the transaction and collect the logs.
+        const receipt = await tx;
+
+        // If WETH should have been paid, an event should be logged. Otherwise, no event should have been logged.
+        if (args.value.eq(0)) {
+            // Ensure that one log was recorded.
+            expect(receipt.logs.length, 'log length should be one').to.be.eq(1);
+
+            // Ensure that the correct log was recorded.
+            const log = receipt.logs[0] as LogWithDecodedArgs<TestProtocolFeesERC20ProxyTransferFromCalledEventArgs>;
+            expect(log.event, 'log event should be `TransferFromCalled`').to.be.eq('TransferFromCalled');
+            expect(log.args.assetData, 'log `assetData` should be `wethAssetData`').to.be.eq(this._wethAssetData);
+            expect(log.args.amount, 'log `amount` should be `protocolFeePaid`').bignumber.to.be.eq(
+                args.protocolFeePaid,
+            );
+            expect(log.args.from, 'log `from` should be `payerAddress`').to.be.eq(args.payerAddress);
+            expect(log.args.to).to.be.eq(this._protocolFees.address);
+        } else {
+            expect(receipt.logs.length, 'log length should be zero').to.be.eq(0);
+        }
+
+        // Get the final state.
+        const finalActivePools = await this._protocolFees.getActivePoolsByEpoch.callAsync();
+        const finalProtocolFeesCollected = await this._protocolFees.getProtocolFeesThisEpochByPool.callAsync(
+            args.poolId,
+        );
+
+        // Check that the active pools list was updated appropriately.
+        if (shouldBeAdded && this._registered_makers.includes(args.makerAddress)) {
+            // Check that the pool id was added to the list of active pools for this epoch.
+            expect(finalActivePools.length, 'final active pools should have been updated').to.be.eq(
+                originalActivePools.length + 1,
+            );
+            expect(finalActivePools.includes(args.poolId), 'final active pools should contain pool id').to.be.true();
+        } else {
+            // Check that active pools list was not altered.
+            expect(finalActivePools, 'final active pools should be identical to original active pools').to.be.deep.eq(
+                originalActivePools,
+            );
+        }
+
+        // Check that the pool has the correct amount of fees attributed to it for this epoch.
+        if (this._registered_makers.includes(args.makerAddress)) {
+            expect(
+                finalProtocolFeesCollected,
+                'final protocol fees should be the original protocol fees plus the fee paid',
+            ).bignumber.to.be.eq(originalProtocolFeesCollected.plus(args.protocolFeePaid));
+        }
+    }
+}
+// tslint:enable:no-unnecessary-type-assertion

contracts/staking/test/catastrophe_test.ts

@@ -36,7 +36,13 @@ blockchainTests.resets('Catastrophe Tests', env => {
         [zrxTokenContract] = await erc20Wrapper.deployDummyTokensAsync(1, ZRX_TOKEN_DECIMALS);
         await erc20Wrapper.setBalancesAndAllowancesAsync();
         // deploy staking contracts
-        stakingWrapper = new StakingWrapper(env.provider, owner, erc20ProxyContract, zrxTokenContract);
+        stakingWrapper = new StakingWrapper(
+            env.provider,
+            owner,
+            erc20ProxyContract,
+            erc20ProxyContract,
+            zrxTokenContract,
+        );
         await stakingWrapper.deployAndConfigureContractsAsync();
     });
 

contracts/staking/test/epoch_test.ts

@@ -44,8 +44,8 @@ describe('Epochs', () => {
         [zrxTokenContract] = await erc20Wrapper.deployDummyTokensAsync(1, ZRX_TOKEN_DECIMALS);
         await erc20Wrapper.setBalancesAndAllowancesAsync();
         // deploy staking contracts
-        stakingWrapper = new StakingWrapper(provider, owner, erc20ProxyContract, zrxTokenContract);
+        stakingWrapper = new StakingWrapper(provider, owner, erc20ProxyContract, erc20ProxyContract, zrxTokenContract);
-        await stakingWrapper.deployAndConfigureContractsAsync();
+        await stakingWrapper.deployAndConfigureContractsAsync(artifacts.TestStaking);
     });
     beforeEach(async () => {
         await blockchainLifecycle.startAsync();

contracts/staking/test/exchange_test.ts

@@ -33,7 +33,13 @@ blockchainTests('Exchange Integrations', env => {
         [zrxTokenContract] = await erc20Wrapper.deployDummyTokensAsync(1, ZRX_TOKEN_DECIMALS);
         await erc20Wrapper.setBalancesAndAllowancesAsync();
         // deploy staking contracts
-        stakingWrapper = new StakingWrapper(env.provider, owner, erc20ProxyContract, zrxTokenContract);
+        stakingWrapper = new StakingWrapper(
+            env.provider,
+            owner,
+            erc20ProxyContract,
+            erc20ProxyContract,
+            zrxTokenContract,
+        );
         await stakingWrapper.deployAndConfigureContractsAsync();
     });
     blockchainTests.resets('Exchange Tracking in Staking Contract', () => {

contracts/staking/test/pools_test.ts

@@ -35,7 +35,13 @@ blockchainTests('Staking Pool Management', env => {
         [zrxTokenContract] = await erc20Wrapper.deployDummyTokensAsync(1, DUMMY_TOKEN_DECIMALS);
         await erc20Wrapper.setBalancesAndAllowancesAsync();
         // deploy staking contracts
-        stakingWrapper = new StakingWrapper(env.provider, owner, erc20ProxyContract, zrxTokenContract);
+        stakingWrapper = new StakingWrapper(
+            env.provider,
+            owner,
+            erc20ProxyContract,
+            erc20ProxyContract,
+            zrxTokenContract,
+        );
         await stakingWrapper.deployAndConfigureContractsAsync();
     });
     blockchainTests.resets('Staking Pool Management', () => {

contracts/staking/test/protocol_fees.ts

@@ -1,14 +1,9 @@
-import { blockchainTests, constants, expect, LogDecoder } from '@0x/contracts-test-utils';
+import { blockchainTests, constants } from '@0x/contracts-test-utils';
-import { StakingRevertErrors } from '@0x/order-utils';
 import { BigNumber } from '@0x/utils';
-import { LogWithDecodedArgs } from 'ethereum-types';
 
-import {
+import { artifacts, TestProtocolFeesContract, TestProtocolFeesERC20ProxyContract } from '../src';
-    artifacts,
+
-    TestProtocolFeesContract,
+import { ProtocolFeeActor } from './actors/protocol_fee_actor';
-    TestProtocolFeesERC20ProxyContract,
-    TestProtocolFeesERC20ProxyTransferFromCalledEventArgs,
-} from '../src';
 
 // tslint:disable:no-unnecessary-type-assertion
 blockchainTests('Protocol Fee Unit Tests', env => {
@@ -19,12 +14,8 @@ blockchainTests('Protocol Fee Unit Tests', env => {
     let makerAddress: string;
     let payerAddress: string;
 
-    // The contract that will be used for testng `payProtocolFee` and `_unwrapETH`.
+    // The actor that will be used for testng `payProtocolFee` and `_unwrapETH`.
-    let protocolFees: TestProtocolFeesContract;
+    let protocolFeeActor: ProtocolFeeActor;
-    let proxy: TestProtocolFeesERC20ProxyContract;
-
-    // The log decoder that will be used to decode logs from TestProtocolFeesERC20Proxy.
-    let logDecoder: LogDecoder;
 
     // The default protocol fee that will be paid -- a somewhat realistic value.
     const DEFAULT_PROTOCOL_FEE_PAID = new BigNumber(150000).times(10000000);
@@ -32,9 +23,6 @@ blockchainTests('Protocol Fee Unit Tests', env => {
     // The default pool Id that will be used.
     const DEFAULT_POOL_ID = '0x0000000000000000000000000000000000000000000000000000000000000001';
 
-    // The WETH asset data that should be set in the contract.
-    const WETH_ASSET_DATA = '0xf47261b0000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2';
-
     before(async () => {
         // Get accounts to represent the exchange and an address that is not a registered exchange.
         [
@@ -46,18 +34,18 @@ blockchainTests('Protocol Fee Unit Tests', env => {
         ] = (await env.web3Wrapper.getAvailableAddressesAsync()).slice(0, 6);
 
         // Deploy the protocol fees contract.
-        protocolFees = await TestProtocolFeesContract.deployFrom0xArtifactAsync(
+        const protocolFees = await TestProtocolFeesContract.deployFrom0xArtifactAsync(
             artifacts.TestProtocolFees,
             env.provider,
             {
                 ...env.txDefaults,
                 from: owner,
             },
-            {},
+            artifacts,
         );
 
         // Deploy the erc20Proxy for testing.
-        proxy = await TestProtocolFeesERC20ProxyContract.deployFrom0xArtifactAsync(
+        const proxy = await TestProtocolFeesERC20ProxyContract.deployFrom0xArtifactAsync(
             artifacts.TestProtocolFeesERC20Proxy,
             env.provider,
             env.txDefaults,
@@ -65,297 +53,186 @@ blockchainTests('Protocol Fee Unit Tests', env => {
         );
 
         // Register the test ERC20Proxy in the exchange.
-        await protocolFees.addERC20AssetProxy.awaitTransactionSuccessAsync(proxy.address);
+        await protocolFees.setWethProxy.awaitTransactionSuccessAsync(proxy.address);
 
         // Register an exchange in the protocol fee contract.
         await protocolFees.addExchangeAddress.awaitTransactionSuccessAsync(exchange, { from: owner });
 
         // "Register" the makerAddress in the default pool.
-        await protocolFees.setPoolIdOfMaker.awaitTransactionSuccessAsync(DEFAULT_POOL_ID, makerAddress);
+        await protocolFees.addMakerToPool.awaitTransactionSuccessAsync(DEFAULT_POOL_ID, makerAddress);
 
-        // Create the log decoder that will be used to decode TransferFromCalledEvent logs.
+        // Initialize the protocol fee actor.
-        logDecoder = new LogDecoder(env.web3Wrapper, artifacts);
+        protocolFeeActor = new ProtocolFeeActor([exchange], [makerAddress], protocolFees);
     });
 
     blockchainTests.resets('payProtocolFee', () => {
-        // Verify that the DEFAULT_POOL_ID was pushed to the active pool list and that the correct amount
-        // is registered in the pool, or that the NIL_POOL's state was unaffected depending on which pool id
-        // was provided.
-        async function verifyEndStateAsync(poolId: string, amount: BigNumber): Promise<void> {
-            if (poolId === DEFAULT_POOL_ID) {
-                // Ensure that the `DEFAULT_POOL_ID` was pushed into this epoch's active pool.
-                const activePools = await protocolFees.getActivePoolsByEpoch.callAsync();
-                expect(activePools.length).to.be.eq(1);
-                expect(activePools[0]).to.be.eq(DEFAULT_POOL_ID);
-
-                // Ensure that the `DEFAULT_PROTOCOL_FEE_PAID` was attributed to the maker's pool.
-                const feesInMakerPool = await protocolFees.getProtocolFeesThisEpochByPool.callAsync(DEFAULT_POOL_ID);
-                expect(feesInMakerPool).bignumber.to.be.eq(amount);
-            } else {
-                // Ensure that the only active pool this epoch is the "zero" pool.
-                const activePools = await protocolFees.getActivePoolsByEpoch.callAsync();
-                expect(activePools.length).to.be.eq(0);
-
-                // Ensure that the `NIL_POOL` was not attributed a payment.
-                const feesInMakerPool = await protocolFees.getProtocolFeesThisEpochByPool.callAsync(
-                    constants.NULL_BYTES32,
-                );
-                expect(feesInMakerPool).bignumber.to.be.eq(constants.ZERO_AMOUNT);
-            }
-        }
-
         it('should revert if called by a non-exchange', async () => {
-            const expectedError = new StakingRevertErrors.OnlyCallableByExchangeError(nonExchange);
+            await protocolFeeActor.payProtocolFeeAsync({
-            const tx = protocolFees.payProtocolFee.sendTransactionAsync(
+                poolId: DEFAULT_POOL_ID,
                 makerAddress,
                 payerAddress,
-                DEFAULT_PROTOCOL_FEE_PAID,
+                protocolFeePaid: DEFAULT_PROTOCOL_FEE_PAID,
-                { from: nonExchange },
+                from: nonExchange,
-            );
+                value: constants.ZERO_AMOUNT,
-            return expect(tx).to.revertWith(expectedError);
+            });
         });
 
         it('should revert if `protocolFeePaid` is zero with zero value sent', async () => {
-            const expectedError = new StakingRevertErrors.InvalidProtocolFeePaymentError(
+            await protocolFeeActor.payProtocolFeeAsync({
-                constants.ZERO_AMOUNT,
+                poolId: DEFAULT_POOL_ID,
-                constants.ZERO_AMOUNT,
-            );
-            const tx = protocolFees.payProtocolFee.sendTransactionAsync(
                 makerAddress,
                 payerAddress,
-                constants.ZERO_AMOUNT,
+                protocolFeePaid: constants.ZERO_AMOUNT,
-                { from: exchange, value: constants.ZERO_AMOUNT },
+                from: exchange,
-            );
+                value: constants.ZERO_AMOUNT,
-            return expect(tx).to.revertWith(expectedError);
+            });
         });
 
         it('should revert if `protocolFeePaid` is zero with non-zero value sent', async () => {
-            const expectedError = new StakingRevertErrors.InvalidProtocolFeePaymentError(
+            await protocolFeeActor.payProtocolFeeAsync({
-                constants.ZERO_AMOUNT,
+                poolId: DEFAULT_POOL_ID,
-                DEFAULT_PROTOCOL_FEE_PAID,
-            );
-            const tx = protocolFees.payProtocolFee.sendTransactionAsync(
                 makerAddress,
                 payerAddress,
-                constants.ZERO_AMOUNT,
+                protocolFeePaid: constants.ZERO_AMOUNT,
-                { from: exchange, value: DEFAULT_PROTOCOL_FEE_PAID },
+                from: exchange,
-            );
+                value: DEFAULT_PROTOCOL_FEE_PAID,
-            return expect(tx).to.revertWith(expectedError);
+            });
         });
 
         it('should revert if `protocolFeePaid` is different than the provided message value', async () => {
-            const differentProtocolFeePaid = DEFAULT_PROTOCOL_FEE_PAID.minus(50);
+            await protocolFeeActor.payProtocolFeeAsync({
-            const expectedError = new StakingRevertErrors.InvalidProtocolFeePaymentError(
+                poolId: DEFAULT_POOL_ID,
-                differentProtocolFeePaid,
-                DEFAULT_PROTOCOL_FEE_PAID,
-            );
-            const tx = protocolFees.payProtocolFee.sendTransactionAsync(
                 makerAddress,
                 payerAddress,
-                differentProtocolFeePaid,
+                protocolFeePaid: DEFAULT_PROTOCOL_FEE_PAID.minus(50),
-                { from: exchange, value: DEFAULT_PROTOCOL_FEE_PAID },
+                from: exchange,
-            );
+                value: DEFAULT_PROTOCOL_FEE_PAID,
-            return expect(tx).to.revertWith(expectedError);
+            });
         });
 
         it('should call `transferFrom` in the proxy if no value is sent and the maker is not in a pool', async () => {
-            const receipt = await protocolFees.payProtocolFee.awaitTransactionSuccessAsync(
+            await protocolFeeActor.payProtocolFeeAsync({
-                payerAddress, // This is an unregistered maker address
+                poolId: DEFAULT_POOL_ID,
+                makerAddress: payerAddress, // This is an unregistered maker address
                 payerAddress,
-                DEFAULT_PROTOCOL_FEE_PAID,
+                protocolFeePaid: DEFAULT_PROTOCOL_FEE_PAID,
-                { from: exchange, value: 0 },
+                from: exchange,
-            );
+                value: constants.ZERO_AMOUNT,
-
+            });
-            // Ensure that the correct number of logs were recorded.
-            expect(receipt.logs.length).to.be.eq(1);
-
-            // Ensure that the correct log was recorded.
-            const log = logDecoder.decodeLogOrThrow(receipt.logs[0]) as LogWithDecodedArgs<
-                TestProtocolFeesERC20ProxyTransferFromCalledEventArgs
-            >;
-            expect(log.event).to.be.eq('TransferFromCalled');
-            expect(log.args.assetData).to.be.eq(WETH_ASSET_DATA);
-            expect(log.args.amount).bignumber.to.be.eq(DEFAULT_PROTOCOL_FEE_PAID);
-            expect(log.args.from).to.be.eq(payerAddress);
-            expect(log.args.to).to.be.eq(protocolFees.address);
-
-            // Verify that the end state is correct.
-            await verifyEndStateAsync(constants.NULL_BYTES32, constants.ZERO_AMOUNT);
         });
 
         it('should call `transferFrom` in the proxy and update `protocolFeesThisEpochByPool` if no value is sent and the maker is in a pool', async () => {
-            const receipt = await protocolFees.payProtocolFee.awaitTransactionSuccessAsync(
+            await protocolFeeActor.payProtocolFeeAsync({
+                poolId: DEFAULT_POOL_ID,
                 makerAddress,
                 payerAddress,
-                DEFAULT_PROTOCOL_FEE_PAID,
+                protocolFeePaid: DEFAULT_PROTOCOL_FEE_PAID,
-                { from: exchange, value: 0 },
+                from: exchange,
-            );
+                value: constants.ZERO_AMOUNT,
-
+            });
-            // Ensure that the correct number of logs were recorded.
-            expect(receipt.logs.length).to.be.eq(1);
-
-            // Ensure that the correct log was recorded.
-            const log = logDecoder.decodeLogOrThrow(receipt.logs[0]) as LogWithDecodedArgs<
-                TestProtocolFeesERC20ProxyTransferFromCalledEventArgs
-            >;
-            expect(log.event).to.be.eq('TransferFromCalled');
-            expect(log.args.assetData).to.be.eq(WETH_ASSET_DATA);
-            expect(log.args.amount).bignumber.to.be.eq(DEFAULT_PROTOCOL_FEE_PAID);
-            expect(log.args.from).to.be.eq(payerAddress);
-            expect(log.args.to).to.be.eq(protocolFees.address);
-
-            // Verify that the end state is correct.
-            await verifyEndStateAsync(DEFAULT_POOL_ID, DEFAULT_PROTOCOL_FEE_PAID);
         });
 
         it('should not call `transferFrom` in the proxy and should not update `protocolFeesThisEpochByPool` if value is sent and the maker is not in a pool', async () => {
-            const receipt = await protocolFees.payProtocolFee.awaitTransactionSuccessAsync(
+            await protocolFeeActor.payProtocolFeeAsync({
-                payerAddress, // This is an unregistered maker address
+                poolId: DEFAULT_POOL_ID,
+                makerAddress: payerAddress, // This is an unregistered maker address
                 payerAddress,
-                DEFAULT_PROTOCOL_FEE_PAID,
+                protocolFeePaid: DEFAULT_PROTOCOL_FEE_PAID,
-                { from: exchange, value: DEFAULT_PROTOCOL_FEE_PAID },
+                from: exchange,
-            );
+                value: DEFAULT_PROTOCOL_FEE_PAID,
-
+            });
-            // Ensure that the correct number of logs were recorded.
-            expect(receipt.logs.length).to.be.eq(0);
-
-            // Verify that the end state is correct.
-            await verifyEndStateAsync(constants.NULL_BYTES32, constants.ZERO_AMOUNT);
         });
 
         it('should not call `transferFrom` in the proxy and should update `protocolFeesThisEpochByPool` if value is sent and the maker is in a pool', async () => {
-            const receipt = await protocolFees.payProtocolFee.awaitTransactionSuccessAsync(
+            await protocolFeeActor.payProtocolFeeAsync({
+                poolId: DEFAULT_POOL_ID,
                 makerAddress,
                 payerAddress,
-                DEFAULT_PROTOCOL_FEE_PAID,
+                protocolFeePaid: DEFAULT_PROTOCOL_FEE_PAID,
-                { from: exchange, value: DEFAULT_PROTOCOL_FEE_PAID },
+                from: exchange,
-            );
+                value: DEFAULT_PROTOCOL_FEE_PAID,
-
+            });
-            // Ensure that the correct number of logs were recorded.
-            expect(receipt.logs.length).to.be.eq(0);
-
-            // Verify that the end state is correct.
-            await verifyEndStateAsync(DEFAULT_POOL_ID, DEFAULT_PROTOCOL_FEE_PAID);
         });
 
         it('should only have one active pool if a fee is paid on behalf of one maker ETH twice', async () => {
-            // Pay the first fee
+            await protocolFeeActor.payProtocolFeeAsync({
-            await protocolFees.payProtocolFee.awaitTransactionSuccessAsync(
+                poolId: DEFAULT_POOL_ID,
                 makerAddress,
                 payerAddress,
-                DEFAULT_PROTOCOL_FEE_PAID,
+                protocolFeePaid: DEFAULT_PROTOCOL_FEE_PAID,
-                { from: exchange, value: DEFAULT_PROTOCOL_FEE_PAID },
+                from: exchange,
-            );
+                value: DEFAULT_PROTOCOL_FEE_PAID,
+            });
 
-            // Pay the second fee
+            await protocolFeeActor.payProtocolFeeAsync({
-            const receipt = await protocolFees.payProtocolFee.awaitTransactionSuccessAsync(
+                poolId: DEFAULT_POOL_ID,
                 makerAddress,
                 payerAddress,
-                DEFAULT_PROTOCOL_FEE_PAID,
+                protocolFeePaid: DEFAULT_PROTOCOL_FEE_PAID,
-                { from: exchange, value: DEFAULT_PROTOCOL_FEE_PAID },
+                from: exchange,
-            );
+                value: DEFAULT_PROTOCOL_FEE_PAID,
-
+            });
-            // Ensure that the correct number of logs were recorded.
-            expect(receipt.logs.length).to.be.eq(0);
-
-            // Verify that the end state is correct -- namely that the active pools list was only updated once,
-            // and that the correct amount is recorded on behalf of the maker.
-            await verifyEndStateAsync(DEFAULT_POOL_ID, DEFAULT_PROTOCOL_FEE_PAID.times(2));
         });
 
         it('should only have one active pool if a fee is paid on behalf of one maker in WETH and then ETH', async () => {
-            // Pay the first fee
+            await protocolFeeActor.payProtocolFeeAsync({
-            await protocolFees.payProtocolFee.awaitTransactionSuccessAsync(
+                poolId: DEFAULT_POOL_ID,
                 makerAddress,
                 payerAddress,
-                DEFAULT_PROTOCOL_FEE_PAID,
+                protocolFeePaid: DEFAULT_PROTOCOL_FEE_PAID,
-                { from: exchange, value: 0 },
+                from: exchange,
-            );
+                value: constants.ZERO_AMOUNT,
+            });
 
-            // Pay the second fee
+            await protocolFeeActor.payProtocolFeeAsync({
-            const receipt = await protocolFees.payProtocolFee.awaitTransactionSuccessAsync(
+                poolId: DEFAULT_POOL_ID,
                 makerAddress,
                 payerAddress,
-                DEFAULT_PROTOCOL_FEE_PAID,
+                protocolFeePaid: DEFAULT_PROTOCOL_FEE_PAID,
-                { from: exchange, value: DEFAULT_PROTOCOL_FEE_PAID },
+                from: exchange,
-            );
+                value: DEFAULT_PROTOCOL_FEE_PAID,
-
+            });
-            // Ensure that the correct number of logs were recorded.
-            expect(receipt.logs.length).to.be.eq(0);
-
-            // Verify that the end state is correct -- namely that the active pools list was only updated once,
-            // and that the correct amount is recorded on behalf of the maker.
-            await verifyEndStateAsync(DEFAULT_POOL_ID, DEFAULT_PROTOCOL_FEE_PAID.times(2));
         });
 
         it('should only have one active pool if a fee is paid on behalf of one maker in ETH and then WETH', async () => {
-            // Pay the first fee
+            await protocolFeeActor.payProtocolFeeAsync({
-            await protocolFees.payProtocolFee.awaitTransactionSuccessAsync(
+                poolId: DEFAULT_POOL_ID,
                 makerAddress,
                 payerAddress,
-                DEFAULT_PROTOCOL_FEE_PAID,
+                protocolFeePaid: DEFAULT_PROTOCOL_FEE_PAID,
-                { from: exchange, value: DEFAULT_PROTOCOL_FEE_PAID },
+                from: exchange,
-            );
+                value: DEFAULT_PROTOCOL_FEE_PAID,
+            });
 
-            // Pay the second fee
+            await protocolFeeActor.payProtocolFeeAsync({
-            const receipt = await protocolFees.payProtocolFee.awaitTransactionSuccessAsync(
+                poolId: DEFAULT_POOL_ID,
                 makerAddress,
                 payerAddress,
-                DEFAULT_PROTOCOL_FEE_PAID,
+                protocolFeePaid: DEFAULT_PROTOCOL_FEE_PAID,
-                { from: exchange, value: 0 },
+                from: exchange,
-            );
+                value: constants.ZERO_AMOUNT,
-
+            });
-            // Ensure that the correct number of logs were recorded.
-            expect(receipt.logs.length).to.be.eq(1);
-
-            // Ensure that the correct log was recorded
-            const log = logDecoder.decodeLogOrThrow(receipt.logs[0]) as LogWithDecodedArgs<
-                TestProtocolFeesERC20ProxyTransferFromCalledEventArgs
-            >;
-            expect(log.event).to.be.eq('TransferFromCalled');
-            expect(log.args.assetData).to.be.eq(WETH_ASSET_DATA);
-            expect(log.args.amount).bignumber.to.be.eq(DEFAULT_PROTOCOL_FEE_PAID);
-            expect(log.args.from).to.be.eq(payerAddress);
-            expect(log.args.to).to.be.eq(protocolFees.address);
-
-            // Verify that the end state is correct -- namely that the active pools list was only updated once,
-            // and that the correct amount is recorded on behalf of the maker.
-            await verifyEndStateAsync(DEFAULT_POOL_ID, DEFAULT_PROTOCOL_FEE_PAID.times(2));
         });
 
         it('should only have one active pool if a fee is paid on behalf of one maker in WETH twice', async () => {
-            // Pay the first fee
+            await protocolFeeActor.payProtocolFeeAsync({
-            await protocolFees.payProtocolFee.awaitTransactionSuccessAsync(
+                poolId: DEFAULT_POOL_ID,
                 makerAddress,
                 payerAddress,
-                DEFAULT_PROTOCOL_FEE_PAID,
+                protocolFeePaid: DEFAULT_PROTOCOL_FEE_PAID,
-                { from: exchange, value: 0 },
+                from: exchange,
-            );
+                value: constants.ZERO_AMOUNT,
+            });
 
-            // Pay the second fee
+            await protocolFeeActor.payProtocolFeeAsync({
-            const receipt = await protocolFees.payProtocolFee.awaitTransactionSuccessAsync(
+                poolId: DEFAULT_POOL_ID,
                 makerAddress,
                 payerAddress,
-                DEFAULT_PROTOCOL_FEE_PAID,
+                protocolFeePaid: DEFAULT_PROTOCOL_FEE_PAID,
-                { from: exchange, value: 0 },
+                from: exchange,
-            );
+                value: constants.ZERO_AMOUNT,
-
+            });
-            // Ensure that the correct number of logs were recorded.
-            expect(receipt.logs.length).to.be.eq(1);
-
-            // Ensure that the correct log was recorded.
-            const log = logDecoder.decodeLogOrThrow(receipt.logs[0]) as LogWithDecodedArgs<
-                TestProtocolFeesERC20ProxyTransferFromCalledEventArgs
-            >;
-            expect(log.event).to.be.eq('TransferFromCalled');
-            expect(log.args.assetData).to.be.eq(WETH_ASSET_DATA);
-            expect(log.args.amount).bignumber.to.be.eq(DEFAULT_PROTOCOL_FEE_PAID);
-            expect(log.args.from).to.be.eq(payerAddress);
-            expect(log.args.to).to.be.eq(protocolFees.address);
-
-            // Verify that the end state is correct -- namely that the active pools list was only updated once,
-            // and that the correct amount is recorded on behalf of the maker.
-            await verifyEndStateAsync(DEFAULT_POOL_ID, DEFAULT_PROTOCOL_FEE_PAID.times(2));
         });
     });
 });
+// tslint:enable:no-unnecessary-type-assertion

contracts/staking/test/rewards_test.ts

@@ -41,15 +41,15 @@ blockchainTests.resets('Testing Rewards', () => {
         exchangeAddress = accounts[1];
         takerAddress = accounts[2];
         actors = accounts.slice(3);
-        // deploy erƒsc20 proxy
+        // deploy erc20 proxy
         erc20Wrapper = new ERC20Wrapper(provider, accounts, owner);
         erc20ProxyContract = await erc20Wrapper.deployProxyAsync();
         // deploy zrx token
         [zrxTokenContract] = await erc20Wrapper.deployDummyTokensAsync(1, ZRX_TOKEN_DECIMALS);
         await erc20Wrapper.setBalancesAndAllowancesAsync();
         // deploy staking contracts
-        stakingWrapper = new StakingWrapper(provider, owner, erc20ProxyContract, zrxTokenContract);
+        stakingWrapper = new StakingWrapper(provider, owner, erc20ProxyContract, erc20ProxyContract, zrxTokenContract);
-        await stakingWrapper.deployAndConfigureContractsAsync();
+        await stakingWrapper.deployAndConfigureContractsAsync(artifacts.TestStaking);
         // setup stakers
         stakers = [new StakerActor(actors[0], stakingWrapper), new StakerActor(actors[1], stakingWrapper)];
         // setup pools

contracts/staking/test/stake_test.ts

@@ -42,8 +42,14 @@ blockchainTests.resets('Stake Statuses', env => {
         [zrxTokenContract] = await erc20Wrapper.deployDummyTokensAsync(1, ZRX_TOKEN_DECIMALS);
         await erc20Wrapper.setBalancesAndAllowancesAsync();
         // deploy staking contracts
-        stakingWrapper = new StakingWrapper(env.provider, owner, erc20ProxyContract, zrxTokenContract);
+        stakingWrapper = new StakingWrapper(
-        await stakingWrapper.deployAndConfigureContractsAsync();
+            env.provider,
+            owner,
+            erc20ProxyContract,
+            erc20ProxyContract,
+            zrxTokenContract,
+        );
+        await stakingWrapper.deployAndConfigureContractsAsync(artifacts.TestStaking);
         // setup new staker
         staker = new StakerActor(actors[0], stakingWrapper);
         // setup pools

contracts/staking/test/utils/staking_wrapper.ts

@@ -24,6 +24,7 @@ export class StakingWrapper {
     private readonly _provider: Provider;
     private readonly _logDecoder: LogDecoder;
     private readonly _ownerAddress: string;
+    private readonly _wethProxyContract: ERC20ProxyContract;
     private readonly _erc20ProxyContract: ERC20ProxyContract;
     private readonly _zrxTokenContract: DummyERC20TokenContract;
     private _stakingContractIfExists?: StakingContract;
@@ -63,6 +64,7 @@ export class StakingWrapper {
         provider: Provider,
         ownerAddres: string,
         erc20ProxyContract: any, // This needs to be the `any` type so that other types of proxies can be used
+        wethProxyContract: any, // This needs to be the `any` type so that other types of proxies can be used
         zrxTokenContract: DummyERC20TokenContract,
     ) {
         this._web3Wrapper = new Web3Wrapper(provider);
@@ -71,6 +73,7 @@ export class StakingWrapper {
         this._logDecoder = new LogDecoder(this._web3Wrapper, decoderArtifacts);
         this._ownerAddress = ownerAddres;
         this._erc20ProxyContract = erc20ProxyContract;
+        this._wethProxyContract = wethProxyContract;
         this._zrxTokenContract = zrxTokenContract;
     }
     public getStakingContract(): StakingContract {
@@ -93,7 +96,7 @@ export class StakingWrapper {
         this._validateDeployedOrThrow();
         return this._rewardVaultContractIfExists as StakingPoolRewardVaultContract;
     }
-    public async deployAndConfigureContractsAsync(): Promise<void> {
+    public async deployAndConfigureContractsAsync(customStakingArtifact?: any): Promise<void> {
         // deploy read-only proxy
         this._readOnlyProxyContractIfExists = await ReadOnlyProxyContract.deployFrom0xArtifactAsync(
             artifacts.ReadOnlyProxy,
@@ -134,7 +137,7 @@ export class StakingWrapper {
         );
         // deploy staking contract
         this._stakingContractIfExists = await StakingContract.deployFrom0xArtifactAsync(
-            artifacts.Staking,
+            customStakingArtifact === undefined ? artifacts.Staking : customStakingArtifact,
             this._provider,
             txDefaults,
             artifacts,
@@ -147,6 +150,11 @@ export class StakingWrapper {
             artifacts,
             this._stakingContractIfExists.address,
             this._readOnlyProxyContractIfExists.address,
+            this._wethProxyContract.address,
+        );
+        // configure weth proxy to accept calls from staking.
+        await this._wethProxyContract.addAuthorizedAddress.awaitTransactionSuccessAsync(
+            (this._stakingProxyContractIfExists as StakingProxyContract).address, // tslint:disable-line:no-unnecessary-type-assertion
         );
         // set staking proxy contract in zrx vault
         await this._zrxVaultContractIfExists.setStakingContract.awaitTransactionSuccessAsync(

contracts/staking/test/vaults_test.ts

@@ -33,7 +33,13 @@ blockchainTests('Staking Vaults', env => {
         [zrxTokenContract] = await erc20Wrapper.deployDummyTokensAsync(1, ZRX_TOKEN_DECIMALS);
         await erc20Wrapper.setBalancesAndAllowancesAsync();
         // deploy staking contracts
-        stakingWrapper = new StakingWrapper(env.provider, owner, erc20ProxyContract, zrxTokenContract);
+        stakingWrapper = new StakingWrapper(
+            env.provider,
+            owner,
+            erc20ProxyContract,
+            erc20ProxyContract,
+            zrxTokenContract,
+        );
         await stakingWrapper.deployAndConfigureContractsAsync();
     });
     blockchainTests.resets('Reward Vault', () => {

contracts/utils/contracts/src/LibBytes.sol

@@ -617,7 +617,7 @@ library LibBytes {
         );
     }
 
-    /// @dev Writes a new length to a byte array. 
+    /// @dev Writes a new length to a byte array.
     ///      Decreasing length will lead to removing the corresponding lower order bytes from the byte array.
     ///      Increasing length may lead to appending adjacent in-memory bytes to the end of the byte array.
     /// @param b Bytes array to write new length to.

packages/order-utils/src/staking_revert_errors.ts

@@ -9,6 +9,11 @@ export enum MakerPoolAssignmentErrorCodes {
     PoolIsFull,
 }
 
+export enum ProtocolFeePaymentErrorCodes {
+    ZeroProtocolFeePaid,
+    MismatchedFeeAndPayment,
+}
+
 export class MiscalculatedRewardsError extends RevertError {
     constructor(totalRewardsPaid?: BigNumber | number | string, initialContractBalance?: BigNumber | number | string) {
         super(
@@ -181,13 +186,14 @@ export class InvalidStakeStatusError extends RevertError {
 
 export class InvalidProtocolFeePaymentError extends RevertError {
     constructor(
+        errorCode?: ProtocolFeePaymentErrorCodes,
         expectedProtocolFeePaid?: BigNumber | number | string,
         actualProtocolFeePaid?: BigNumber | number | string,
     ) {
         super(
             'InvalidProtocolFeePaymentError',
-            'InvalidProtocolFeePaymentError(uint256 expectedProtocolFeePaid, uint256 actualProtocolFeePaid)',
+            'InvalidProtocolFeePaymentError(uint8 errorCode, uint256 expectedProtocolFeePaid, uint256 actualProtocolFeePaid)',
-            { expectedProtocolFeePaid, actualProtocolFeePaid },
+            { errorCode, expectedProtocolFeePaid, actualProtocolFeePaid },
         );
     }
 }