@0x:contracts-staking Addressed some review comments

contracts/staking/contracts/src/StakingProxy.sol

@@ -31,11 +31,13 @@ contract StakingProxy is
 
     /// @dev Constructor.
     /// @param _stakingContract Staking contract to delegate calls to.
-    constructor(address _stakingContract)
+    /// @param _wethProxyAddress The address that can transfer WETH for fees.
+    constructor(address _stakingContract, address _wethProxyAddress)
         public
         MixinStorage()
     {
         stakingContract = _stakingContract;
+        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";
@@ -112,7 +110,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

@@ -45,10 +45,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

@@ -31,4 +31,22 @@ contract MixinDeploymentConstants {
     uint32 constant internal REWARD_DELEGATED_STAKE_WEIGHT = 900000; // 90%
 
     uint256 constant internal CHAIN_ID = 1;
+
+    // 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,14 +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
-
-    // 0x ERC20 Proxy
-    IAssetProxy internal erc20Proxy;
+    {
+        // 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());
+        }
+    }
+
+    // WETH Asset Proxy
+    IAssetProxy internal wethAssetProxy;
 
     // address of staking contract
     address internal stakingContract;

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

@@ -118,10 +118,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/IStructs.sol

@@ -102,12 +102,4 @@ interface IStructs {
         uint256 numerator;
         uint256 denominator;
     }
-
-    /// @dev Tracks the amount of protocol fees paid in ETH and WETH in a staking pool.
-    /// @param ethBalance The balance of the pool in ETH.
-    /// @param wethBalance The balance of the pool in WETH.
-    struct ProtocolFeeBalance {
-        uint128 ethBalance;
-        uint128 wethBalance;
-    }
 }

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

@@ -48,17 +48,17 @@ interface IZrxVault {
         uint256 amount
     );
 
-    /// @dev Emitted when the ERC20 Proxy is changed.
-    /// @param erc20ProxyAddress Address of the new ERC20 proxy.
-    event Erc20ProxyChanged(
-        address erc20ProxyAddress
+    /// @dev Emitted when the Zrx Proxy is changed.
+    /// @param zrxProxyAddress Address of the new ERC20 proxy.
+    event ZrxProxyChanged(
+        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.
-    function setErc20Proxy(address erc20ProxyAddress)
+    /// @param zrxProxyAddress Address of the 0x Zrx Asset Proxy.
+    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,7 @@ import "@0x/contracts-utils/contracts/src/LibRichErrors.sol";
 
 
 library LibStakingRichErrors {
+
     // bytes4(keccak256("MiscalculatedRewardsError(uint256,uint256)"))
     bytes4 internal constant MISCALCULATED_REWARDS_ERROR_SELECTOR =
         0xf7806c4e;
@@ -138,6 +139,10 @@ library LibStakingRichErrors {
     bytes4 internal constant INVALID_PROTOCOL_FEE_PAYMENT_ERROR_SELECTOR =
         0x31d7a505;
 
+    // bytes4(keccak256("InvalidWethAssetDataError()"))
+    bytes internal constant INVALID_WETH_ASSET_DATA_ERROR =
+        hex"24bf322c";
+
     // solhint-disable func-name-mixedcase
     function MiscalculatedRewardsError(
         uint256 totalRewardsPaid,
@@ -524,4 +529,12 @@ library LibStakingRichErrors {
             status
         );
     }
+
+    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
-    IAssetProxy internal erc20Proxy;
+    // Zrx Asset Proxy
+    IAssetProxy internal zrxAssetProxy;
 
     // Zrx Token
     IERC20Token internal zrxToken;
@@ -52,33 +52,33 @@ 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,
             zrxTokenAddress
         );
     }
 
-    /// @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.
-    function setErc20Proxy(address erc20ProxyAddress)
+    /// @param zrxProxyAddress Address of the 0x Zrx Proxy.
+    function setZrxProxy(address zrxProxyAddress)
         external
         onlyOwner
         onlyNotInCatastrophicFailure
     {
-        erc20Proxy = IAssetProxy(erc20ProxyAddress);
-        emit Erc20ProxyChanged(erc20ProxyAddress);
+        zrxAssetProxy = IAssetProxy(zrxProxyAddress);
+        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,12 +19,19 @@
 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 setWethProxy(address wethProxyAddress)
+        external
+    {
+        wethAssetProxy = IAssetProxy(wethProxyAddress);
+    }
+
     function setPoolIdOfMaker(bytes32 poolId, address makerAddress)
         external
     {

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.