From 3843c64c408a4508d2936e9fae1d23e24ce4b342 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 15 Sep 2019 15:19:31 -0500 Subject: [PATCH 01/11] Update visibility, events, and naming in vaults --- contracts/staking/contracts/src/Staking.sol | 1 - .../contracts/src/interfaces/IVaultCore.sol | 17 ++++------ .../contracts/src/interfaces/IZrxVault.sol | 9 ++--- .../staking/contracts/src/vaults/EthVault.sol | 3 +- .../contracts/src/vaults/MixinVaultCore.sol | 33 ++++++++++++------- .../src/vaults/StakingPoolRewardVault.sol | 12 +++---- .../staking/contracts/src/vaults/ZrxVault.sol | 21 +++++++----- 7 files changed, 50 insertions(+), 46 deletions(-) diff --git a/contracts/staking/contracts/src/Staking.sol b/contracts/staking/contracts/src/Staking.sol index dc9ad3357b..ab4fb9c529 100644 --- a/contracts/staking/contracts/src/Staking.sol +++ b/contracts/staking/contracts/src/Staking.sol @@ -33,7 +33,6 @@ contract Staking is MixinStakingPool, MixinExchangeFees { - // this contract can receive ETH // solhint-disable no-empty-blocks function () diff --git a/contracts/staking/contracts/src/interfaces/IVaultCore.sol b/contracts/staking/contracts/src/interfaces/IVaultCore.sol index 91ab394dd4..78d2916ae4 100644 --- a/contracts/staking/contracts/src/interfaces/IVaultCore.sol +++ b/contracts/staking/contracts/src/interfaces/IVaultCore.sol @@ -32,22 +32,17 @@ pragma solidity ^0.5.9; /// state in the staking contract. interface IVaultCore { - /// @dev Emitted when the Staking contract is changed. - /// @param stakingContractAddress Address of the new Staking contract. - event StakingContractChanged( - address stakingContractAddress - ); + /// @dev Emmitted whenever a StakingProxy is set in a vault. + event StakingProxySet(address stakingProxyAddress); /// @dev Emitted when the Staking contract is put into Catastrophic Failure Mode /// @param sender Address of sender (`msg.sender`) - event InCatastrophicFailureMode( - address sender - ); + event InCatastrophicFailureMode(address sender); - /// @dev Sets the address of the Staking Contract. + /// @dev Sets the address of the StakingProxy contract. /// Note that only the contract owner can call this function. - /// @param _stakingContractAddress Address of Staking contract. - function setStakingContract(address payable _stakingContractAddress) + /// @param _stakingContractAddress Address of Staking proxy contract. + function setStakingProxy(address payable _stakingProxyAddress) external; /// @dev Vault enters into Catastrophic Failure Mode. diff --git a/contracts/staking/contracts/src/interfaces/IZrxVault.sol b/contracts/staking/contracts/src/interfaces/IZrxVault.sol index 62aeaf34dd..7d151a5966 100644 --- a/contracts/staking/contracts/src/interfaces/IZrxVault.sol +++ b/contracts/staking/contracts/src/interfaces/IZrxVault.sol @@ -48,16 +48,13 @@ interface IZrxVault { uint256 amount ); - /// @dev Emitted when the Zrx Proxy is changed. - /// @param zrxProxyAddress Address of the new ERC20 proxy. - event ZrxProxyChanged( - address zrxProxyAddress - ); + /// @dev Emitted whenever the ZRX AssetProxy is set. + event ZrxProxySet(address zrxProxyAddress); /// @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 zrxProxyAddress Address of the 0x Zrx Asset Proxy. + /// @param zrxProxyAddress Address of the 0x Zrx Proxy. function setZrxProxy(address zrxProxyAddress) external; diff --git a/contracts/staking/contracts/src/vaults/EthVault.sol b/contracts/staking/contracts/src/vaults/EthVault.sol index 7c4a755c2b..dc83149dac 100644 --- a/contracts/staking/contracts/src/vaults/EthVault.sol +++ b/contracts/staking/contracts/src/vaults/EthVault.sol @@ -34,8 +34,9 @@ contract EthVault is mapping (address => uint256) internal _balances; /// @dev Constructor. - constructor() + constructor(address _stakingProxyAddress) public + MixinVaultCore(_stakingProxyAddress) {} // solhint-disable-line no-empty-blocks /// @dev Deposit an `amount` of ETH from `owner` into the vault. diff --git a/contracts/staking/contracts/src/vaults/MixinVaultCore.sol b/contracts/staking/contracts/src/vaults/MixinVaultCore.sol index b916cc90a1..ef7033be6d 100644 --- a/contracts/staking/contracts/src/vaults/MixinVaultCore.sol +++ b/contracts/staking/contracts/src/vaults/MixinVaultCore.sol @@ -40,20 +40,21 @@ contract MixinVaultCore is IVaultCore { // Address of staking contract - address payable internal stakingContractAddress; + address payable public stakingProxyAddress; // True iff vault has been set to Catastrophic Failure Mode - bool internal isInCatastrophicFailure; + bool public isInCatastrophicFailure; /// @dev Constructor. - constructor() public { - stakingContractAddress = 0x0000000000000000000000000000000000000000; - isInCatastrophicFailure = false; + constructor(address _stakingProxyAddress) + public + { + _setStakingProxy(_stakingProxyAddress); } /// @dev Asserts that the sender (`msg.sender`) is the staking contract. - modifier onlyStakingContract { - if (msg.sender != stakingContractAddress) { + modifier onlyStakingProxy { + if (msg.sender != stakingProxyAddress) { LibRichErrors.rrevert(LibStakingRichErrors.OnlyCallableByStakingContractError( msg.sender )); @@ -77,15 +78,14 @@ contract MixinVaultCore is _; } - /// @dev Sets the address of the Staking Contract. + /// @dev Sets the address of the StakingProxy contract. /// Note that only the contract owner can call this function. - /// @param _stakingContractAddress Address of Staking contract. - function setStakingContract(address payable _stakingContractAddress) + /// @param _stakingContractAddress Address of Staking proxy contract. + function setStakingProxy(address payable _stakingProxyAddress) external onlyOwner { - stakingContractAddress = _stakingContractAddress; - emit StakingContractChanged(stakingContractAddress); + _setStakingProxy(_stakingProxyContract); } /// @dev Vault enters into Catastrophic Failure Mode. @@ -98,4 +98,13 @@ contract MixinVaultCore is isInCatastrophicFailure = true; emit InCatastrophicFailureMode(msg.sender); } + + /// @dev Sets the address of the StakingProxy contract. + /// @param _stakingContractAddress Address of Staking proxy contract. + function _setStakingProxy(address payable _stakingProxyAddress) + internal + { + stakingProxyAddress = _stakingProxyAddress; + emit StakingProxySet(_stakingProxyAddress); + } } diff --git a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol index a035fdcf45..fb8f72da5f 100644 --- a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol @@ -57,7 +57,7 @@ contract StakingPoolRewardVault is function () external payable - onlyStakingContract + onlyStakingProxy onlyNotInCatastrophicFailure { emit RewardDeposited(UNKNOWN_STAKING_POOL_ID, msg.value); @@ -89,7 +89,7 @@ contract StakingPoolRewardVault is bool operatorOnly ) external - onlyStakingContract + onlyStakingProxy returns ( uint256 operatorPortion, uint256 membersPortion @@ -110,7 +110,7 @@ contract StakingPoolRewardVault is uint256 amount ) external - onlyStakingContract + onlyStakingProxy { if (amount == 0) { return; @@ -144,7 +144,7 @@ contract StakingPoolRewardVault is uint256 amount ) external - onlyStakingContract + onlyStakingProxy { if (amount == 0) { return; @@ -178,7 +178,7 @@ contract StakingPoolRewardVault is uint32 operatorShare ) external - onlyStakingContract + onlyStakingProxy onlyNotInCatastrophicFailure { // operator share must be a valid fraction @@ -214,7 +214,7 @@ contract StakingPoolRewardVault is /// @param newOperatorShare The newly decreased percentage of any rewards owned by the operator. function decreaseOperatorShare(bytes32 poolId, uint32 newOperatorShare) external - onlyStakingContract + onlyStakingProxy onlyNotInCatastrophicFailure { uint32 oldOperatorShare = poolById[poolId].operatorShare; diff --git a/contracts/staking/contracts/src/vaults/ZrxVault.sol b/contracts/staking/contracts/src/vaults/ZrxVault.sol index 09a7994031..6993d8d20d 100644 --- a/contracts/staking/contracts/src/vaults/ZrxVault.sol +++ b/contracts/staking/contracts/src/vaults/ZrxVault.sol @@ -52,15 +52,18 @@ contract ZrxVault is bytes internal _zrxAssetData; /// @dev Constructor. - /// @param zrxProxyAddress Address of the 0x Zrx Proxy. + /// @param _stakingProxyAddress Address of StakingProxy contract. + /// @param _zrxProxyAddress Address of the 0x Zrx Proxy. /// @param _zrxTokenAddress Address of the Zrx Token. constructor( - address zrxProxyAddress, + address _stakingProxyAddress, + address _zrxProxyAddress, address _zrxTokenAddress ) public + MixinVaultCore(_stakingProxyAddress) { - zrxAssetProxy = IAssetProxy(zrxProxyAddress); + zrxAssetProxy = IAssetProxy(_zrxProxyAddress); _zrxToken = IERC20Token(_zrxTokenAddress); _zrxAssetData = abi.encodeWithSelector( IAssetData(address(0)).ERC20Token.selector, @@ -71,14 +74,14 @@ contract ZrxVault is /// @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 zrxProxyAddress Address of the 0x Zrx Proxy. - function setZrxProxy(address zrxProxyAddress) + /// @param _zrxProxyAddress Address of the 0x Zrx Proxy. + function setZrxProxy(address _zrxProxyAddress) external onlyOwner onlyNotInCatastrophicFailure { - zrxAssetProxy = IAssetProxy(zrxProxyAddress); - emit ZrxProxyChanged(zrxProxyAddress); + zrxAssetProxy = IAssetProxy(_zrxProxyAddress); + emit ZrxProxySet(_zrxProxyAddress); } /// @dev Deposit an `amount` of Zrx Tokens from `owner` into the vault. @@ -88,7 +91,7 @@ contract ZrxVault is /// @param amount of Zrx Tokens to deposit. function depositFrom(address owner, uint256 amount) external - onlyStakingContract + onlyStakingProxy onlyNotInCatastrophicFailure { // update balance @@ -113,7 +116,7 @@ contract ZrxVault is /// @param amount of Zrx Tokens to withdraw. function withdrawFrom(address owner, uint256 amount) external - onlyStakingContract + onlyStakingProxy onlyNotInCatastrophicFailure { _withdrawFrom(owner, amount); From b70db37b4f94e900f04b8712e345867d7793fa00 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 15 Sep 2019 16:26:42 -0500 Subject: [PATCH 02/11] Set wethAssetProxy, ethVault, rewardVault, and zrxVault using init pattern --- .../staking/contracts/src/StakingProxy.sol | 64 +++++- .../src/interfaces/IStakingEvents.sol | 10 +- .../contracts/src/interfaces/IStorageInit.sol | 12 +- .../src/libs/LibStakingRichErrors.sol | 6 +- .../staking/contracts/src/sys/MixinParams.sol | 213 +++++++++++++++--- 5 files changed, 259 insertions(+), 46 deletions(-) diff --git a/contracts/staking/contracts/src/StakingProxy.sol b/contracts/staking/contracts/src/StakingProxy.sol index 43c2049ff4..9cfc7a8da1 100644 --- a/contracts/staking/contracts/src/StakingProxy.sol +++ b/contracts/staking/contracts/src/StakingProxy.sol @@ -35,13 +35,28 @@ contract StakingProxy is /// @param _stakingContract Staking contract to delegate calls to. /// @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) + /// @param _ethVaultAddress Address of the EthVault contract. + /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. + /// @param _zrxVaultAddress Address of the ZrxVault contract. + constructor( + address _stakingContract, + address _readOnlyProxy, + address _wethProxyAddress, + address _ethVaultAddress, + address _rewardVaultAddress, + address _zrxVaultAddress + ) public MixinStorage() { readOnlyProxy = _readOnlyProxy; - wethAssetProxy = IAssetProxy(_wethProxyAddress); - _attachStakingContract(_stakingContract); + _attachStakingContract( + _stakingContract, + _wethProxyAddress, + _ethVaultAddress, + _rewardVaultAddress, + _zrxVaultAddress + ); } /// @dev Delegates calls to the staking contract, if it is set. @@ -59,11 +74,27 @@ contract StakingProxy is /// @dev Attach a staking contract; future calls will be delegated to the staking contract. /// Note that this is callable only by this contract's owner. /// @param _stakingContract Address of staking contract. - function attachStakingContract(address _stakingContract) + /// @param _wethProxyAddress The address that can transfer WETH for fees. + /// @param _ethVaultAddress Address of the EthVault contract. + /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. + /// @param _zrxVaultAddress Address of the ZrxVault contract. + function attachStakingContract( + address _stakingContract, + address _wethProxyAddress, + address _ethVaultAddress, + address _rewardVaultAddress, + address _zrxVaultAddress + ) external onlyOwner { - _attachStakingContract(_stakingContract); + _attachStakingContract( + _stakingContract, + _wethProxyAddress, + _ethVaultAddress, + _rewardVaultAddress, + _zrxVaultAddress + ); } /// @dev Detach the current staking contract. @@ -132,13 +163,30 @@ contract StakingProxy is /// @dev Attach a staking contract; future calls will be delegated to the staking contract. /// @param _stakingContract Address of staking contract. - function _attachStakingContract(address _stakingContract) + /// @param _wethProxyAddress The address that can transfer WETH for fees. + /// @param _ethVaultAddress Address of the EthVault contract. + /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. + /// @param _zrxVaultAddress Address of the ZrxVault contract. + function _attachStakingContract( + address _stakingContract, + address _wethProxyAddress, + address _ethVaultAddress, + address _rewardVaultAddress, + address _zrxVaultAddress + ) private { stakingContract = readOnlyProxyCallee = _stakingContract; // Call `init()` on the staking contract to initialize storage. - (bool didInitSucceed, bytes memory initReturnData) = - _stakingContract.delegatecall(abi.encode(IStorageInit(0).init.selector)); + (bool didInitSucceed, bytes memory initReturnData) = _stakingContract.delegatecall( + abi.encodeWithSelector( + IStorageInit(0).init.selector, + _wethProxyAddress, + _ethVaultAddress, + _rewardVaultAddress, + _zrxVaultAddress + ) + ); if (!didInitSucceed) { assembly { revert(add(initReturnData, 0x20), mload(initReturnData)) } } diff --git a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol index 194656e4b6..3bcafcdf07 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol @@ -60,13 +60,19 @@ interface IStakingEvents { /// @param maximumMakersInPool Maximum number of maker addresses allowed to be registered to a pool. /// @param cobbDouglasAlphaNumerator Numerator for cobb douglas alpha factor. /// @param cobbDouglasAlphaDenomintor Denominator for cobb douglas alpha factor. - event ParamsChanged( + /// @param ethVaultAddress Address of the EthVault contract. + /// @param rewardVaultAddress Address of the StakingPoolRewardVault contract. + /// @param zrxVaultAddress Address of the ZrxVault contract. + event ParamsSet( uint256 epochDurationInSeconds, uint32 rewardDelegatedStakeWeight, uint256 minimumPoolStake, uint256 maximumMakersInPool, uint256 cobbDouglasAlphaNumerator, - uint256 cobbDouglasAlphaDenomintor + uint256 cobbDouglasAlphaDenomintor, + address ethVaultAddress, + address rewardVaultAddress, + address zrxVaultAddress ); /// @dev Emitted by MixinScheduler when the timeLock period is changed. diff --git a/contracts/staking/contracts/src/interfaces/IStorageInit.sol b/contracts/staking/contracts/src/interfaces/IStorageInit.sol index e06fce9897..35e49c1a5d 100644 --- a/contracts/staking/contracts/src/interfaces/IStorageInit.sol +++ b/contracts/staking/contracts/src/interfaces/IStorageInit.sol @@ -22,5 +22,15 @@ pragma solidity ^0.5.9; interface IStorageInit { /// @dev Initialize storage owned by this contract. - function init() external; + /// @param _wethProxyAddress The address that can transfer WETH for fees. + /// @param _ethVaultAddress Address of the EthVault contract. + /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. + /// @param _zrxVaultAddress Address of the ZrxVault contract. + function init( + address _wethProxyAddress, + address _ethVaultAddress, + address _rewardVaultAddress, + address _zrxVaultAddress + ) + external; } diff --git a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol index 3daf14c8cb..8c3721d813 100644 --- a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol +++ b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol @@ -41,7 +41,11 @@ library LibStakingRichErrors { enum InvalidParamValueErrorCode { InvalidCobbDouglasAlpha, InvalidRewardDelegatedStakeWeight, - InvalidMaximumMakersInPool + InvalidMaximumMakersInPool, + InvalidWethProxyAddress, + InvalidEthVaultAddress, + InvalidRewardVaultAddress, + InvalidZrxVaultAddress } enum MakerPoolAssignmentErrorCodes { diff --git a/contracts/staking/contracts/src/sys/MixinParams.sol b/contracts/staking/contracts/src/sys/MixinParams.sol index cd6fba21de..0c470feed7 100644 --- a/contracts/staking/contracts/src/sys/MixinParams.sol +++ b/contracts/staking/contracts/src/sys/MixinParams.sol @@ -21,6 +21,9 @@ pragma solidity ^0.5.9; import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "../immutable/MixinStorage.sol"; import "../interfaces/IStakingEvents.sol"; +import "../interfaces/IEthVault.sol"; +import "../interfaces/IStakingPoolRewardVault.sol"; +import "../interfaces/IZrxVault.sol"; import "../libs/LibStakingRichErrors.sol"; @@ -34,39 +37,37 @@ contract MixinParams is /// @param _minimumPoolStake Minimum amount of stake required in a pool to collect rewards. /// @param _maximumMakersInPool Maximum number of maker addresses allowed to be registered to a pool. /// @param _cobbDouglasAlphaNumerator Numerator for cobb douglas alpha factor. - /// @param _cobbDouglasAlphaDenomintor Denominator for cobb douglas alpha factor. + /// @param _cobbDouglasAlphaDenominator Denominator for cobb douglas alpha factor. + /// @param _wethProxyAddress The address that can transfer WETH for fees. + /// @param _ethVaultAddress Address of the EthVault contract. + /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. + /// @param _zrxVaultAddress Address of the ZrxVault contract. function setParams( uint256 _epochDurationInSeconds, uint32 _rewardDelegatedStakeWeight, uint256 _minimumPoolStake, uint256 _maximumMakersInPool, uint32 _cobbDouglasAlphaNumerator, - uint32 _cobbDouglasAlphaDenomintor + uint32 _cobbDouglasAlphaDenominator, + address _wethProxyAddress, + address _ethVaultAddress, + address _rewardVaultAddress, + address _zrxVaultAddress ) external onlyOwner { - _assertValidRewardDelegatedStakeWeight(_rewardDelegatedStakeWeight); - _assertValidMaximumMakersInPool(_maximumMakersInPool); - _assertValidCobbDouglasAlpha( + _setParams( + _epochDurationInSeconds, + _rewardDelegatedStakeWeight, + _minimumPoolStake, + _maximumMakersInPool, _cobbDouglasAlphaNumerator, - _cobbDouglasAlphaDenomintor - ); - - epochDurationInSeconds = _epochDurationInSeconds; - rewardDelegatedStakeWeight = _rewardDelegatedStakeWeight; - minimumPoolStake = _minimumPoolStake; - maximumMakersInPool = _maximumMakersInPool; - cobbDouglasAlphaNumerator = _cobbDouglasAlphaNumerator; - cobbDouglasAlphaDenomintor = _cobbDouglasAlphaDenomintor; - - emit ParamsChanged( - epochDurationInSeconds, - rewardDelegatedStakeWeight, - minimumPoolStake, - maximumMakersInPool, - cobbDouglasAlphaNumerator, - cobbDouglasAlphaDenomintor + _cobbDouglasAlphaDenominator, + _wethProxyAddress, + _ethVaultAddress, + _rewardVaultAddress, + _zrxVaultAddress ); } @@ -76,7 +77,11 @@ contract MixinParams is /// @return _minimumPoolStake Minimum amount of stake required in a pool to collect rewards. /// @return _maximumMakersInPool Maximum number of maker addresses allowed to be registered to a pool. /// @return _cobbDouglasAlphaNumerator Numerator for cobb douglas alpha factor. - /// @return _cobbDouglasAlphaDenomintor Denominator for cobb douglas alpha factor. + /// @return _cobbDouglasAlphaDenominator Denominator for cobb douglas alpha factor. + /// @return _wethProxyAddress The address that can transfer WETH for fees. + /// @return _ethVaultAddress Address of the EthVault contract. + /// @return _rewardVaultAddress Address of the StakingPoolRewardVault contract. + /// @return _zrxVaultAddress Address of the ZrxVault contract. function getParams() external view @@ -86,7 +91,11 @@ contract MixinParams is uint256 _minimumPoolStake, uint256 _maximumMakersInPool, uint32 _cobbDouglasAlphaNumerator, - uint32 _cobbDouglasAlphaDenomintor + uint32 _cobbDouglasAlphaDenominator, + address _wethProxyAddress, + address _ethVaultAddress, + address _rewardVaultAddress, + address _zrxVaultAddress ) { _epochDurationInSeconds = epochDurationInSeconds; @@ -94,7 +103,11 @@ contract MixinParams is _minimumPoolStake = minimumPoolStake; _maximumMakersInPool = maximumMakersInPool; _cobbDouglasAlphaNumerator = cobbDouglasAlphaNumerator; - _cobbDouglasAlphaDenomintor = cobbDouglasAlphaDenomintor; + _cobbDouglasAlphaDenominator = cobbDouglasAlphaDenomintor; + _wethProxyAddress = address(wethAssetProxy); + _ethVaultAddress = address(ethVault); + _rewardVaultAddress = address(rewardVault); + _zrxVaultAddress = address(_zrxVaultAddress); } /// @dev Assert param values before initializing them. @@ -108,7 +121,11 @@ contract MixinParams is minimumPoolStake != 0 && maximumMakersInPool != 0 && cobbDouglasAlphaNumerator != 0 && - cobbDouglasAlphaDenomintor != 0 + cobbDouglasAlphaDenomintor != 0 && + address(wethAssetProxy) != NIL_ADDRESS && + address(ethVault) != NIL_ADDRESS && + address(rewardVault) != NIL_ADDRESS && + address(zrxVault) != NIL_ADDRESS ) { LibRichErrors.rrevert( LibStakingRichErrors.InitializationError( @@ -118,20 +135,105 @@ contract MixinParams is } } - /// @dev Initialize storage belonging to this mixin. - function _initMixinParams() + /// @dev Initialzize storage belonging to this mixin. + /// @param _wethProxyAddress The address that can transfer WETH for fees. + /// @param _ethVaultAddress Address of the EthVault contract. + /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. + /// @param _zrxVaultAddress Address of the ZrxVault contract. + function _initMixinParams( + address _wethProxyAddress, + address _ethVaultAddress, + address _rewardVaultAddress, + address _zrxVaultAddress + ) internal { // assert the current values before overwriting them. _assertMixinParamsBeforeInit(); // Set up defaults. - epochDurationInSeconds = 2 weeks; - rewardDelegatedStakeWeight = (90 * PPM_DENOMINATOR) / 100; // 90% - minimumPoolStake = 100 * MIN_TOKEN_VALUE; // 100 ZRX - maximumMakersInPool = 10; - cobbDouglasAlphaNumerator = 1; - cobbDouglasAlphaDenomintor = 2; + _epochDurationInSeconds = 2 weeks; + _rewardDelegatedStakeWeight = (90 * PPM_DENOMINATOR) / 100; // 90% + _minimumPoolStake = 100 * MIN_TOKEN_VALUE; // 100 ZRX + _maximumMakersInPool = 10; + _cobbDouglasAlphaNumerator = 1; + _cobbDouglasAlphaDenomintor = 2; + _setParams( + _epochDurationInSeconds, + _rewardDelegatedStakeWeight, + _minimumPoolStake, + _maximumMakersInPool, + _cobbDouglasAlphaNumerator, + _cobbDouglasAlphaDenomintor, + _wethProxyAddress, + _ethVaultAddress, + _rewardVaultAddress, + _zrxVaultAddress + ); + } + + /// @dev Set all configurable parameters at once. + /// @param _epochDurationInSeconds Minimum seconds between epochs. + /// @param _rewardDelegatedStakeWeight How much delegated stake is weighted vs operator stake, in ppm. + /// @param _minimumPoolStake Minimum amount of stake required in a pool to collect rewards. + /// @param _maximumMakersInPool Maximum number of maker addresses allowed to be registered to a pool. + /// @param _cobbDouglasAlphaNumerator Numerator for cobb douglas alpha factor. + /// @param _cobbDouglasAlphaDenominator Denominator for cobb douglas alpha factor. + /// @param _wethProxyAddress The address that can transfer WETH for fees. + /// @param _ethVaultAddress Address of the EthVault contract. + /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. + /// @param _zrxVaultAddress Address of the ZrxVault contract. + function _setParams( + uint256 _epochDurationInSeconds, + uint32 _rewardDelegatedStakeWeight, + uint256 _minimumPoolStake, + uint256 _maximumMakersInPool, + uint32 _cobbDouglasAlphaNumerator, + uint32 _cobbDouglasAlphaDenominator, + address _wethProxyAddress, + address _ethVaultAddress, + address _rewardVaultAddress, + address _zrxVaultAddress + ) + internal + { + _assertValidRewardDelegatedStakeWeight(_rewardDelegatedStakeWeight); + _assertValidMaximumMakersInPool(_maximumMakersInPool); + _assertValidCobbDouglasAlpha( + _cobbDouglasAlphaNumerator, + _cobbDouglasAlphaDenominator + ); + _assertValidAddresses( + _wethProxyAddress, + _ethVaultAddress, + _rewardVaultAddress, + _zrxVaultAddress + ); + // TODO: set boundaries on some of these params + + epochDurationInSeconds = _epochDurationInSeconds; + rewardDelegatedStakeWeight = _rewardDelegatedStakeWeight; + minimumPoolStake = _minimumPoolStake; + maximumMakersInPool = _maximumMakersInPool; + cobbDouglasAlphaNumerator = _cobbDouglasAlphaNumerator; + cobbDouglasAlphaDenomintor = _cobbDouglasAlphaDenominator; + wethAssetProxy = IAssetProxy(_wethProxyAddress); + ethVault = IEthVault(_ethVaultAddress); + rewardVault = IStakingPoolRewardVault(_rewardVaultAddress); + zrxVault = IZrxVault(_zrxVaultAddress); + + emit ParamsSet( + _epochDurationInSeconds, + _rewardDelegatedStakeWeight, + _minimumPoolStake, + _maximumMakersInPool, + _cobbDouglasAlphaNumerator, + _cobbDouglasAlphaDenominator, + _wethProxyAddress, + _ethVaultAddress, + _rewardVaultAddress, + _zrxVaultAddress + ); } /// @dev Asserts that cobb douglas alpha values are valid. @@ -184,4 +286,47 @@ contract MixinParams is )); } } + + /// @dev Asserts that passed in addresses are non-zero. + /// @param _wethProxyAddress The address that can transfer WETH for fees. + /// @param _ethVaultAddress Address of the EthVault contract. + /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. + /// @param _zrxVaultAddress Address of the ZrxVault contract. + function _assertValidVaultAddresses( + address _wethProxyAddress, + address _ethVaultAddress, + address _rewardVaultAddress, + address _zrxVaultAddress + ) + private + pure + { + if (_wethProxyAddress == NIL_ADDRESS) { + LibRichErrors.rrevert( + LibStakingRichErrors.InvalidParamValueError( + LibStakingRichErrors.InvalidParamValueErrorCode.InvalidWethProxyAddress + )); + } + + if (_ethVaultAddress == NIL_ADDRESS) { + LibRichErrors.rrevert( + LibStakingRichErrors.InvalidParamValueError( + LibStakingRichErrors.InvalidParamValueErrorCode.InvalidEthVaultAddress + )); + } + + if (_rewardVaultAddress == NIL_ADDRESS) { + LibRichErrors.rrevert( + LibStakingRichErrors.InvalidParamValueError( + LibStakingRichErrors.InvalidParamValueErrorCode.InvalidRewardVaultAddress + )); + } + + if (_zrxVaultAddress == NIL_ADDRESS) { + LibRichErrors.rrevert( + LibStakingRichErrors.InvalidParamValueError( + LibStakingRichErrors.InvalidParamValueErrorCode.InvalidZrxVaultAddress + )); + } + } } From 16ebdfad9adbcbe3cf69d1b223467c53e4876ccd Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 15 Sep 2019 16:45:20 -0500 Subject: [PATCH 03/11] Remove redundant setters and require statements --- .../src/interfaces/IStakingEvents.sol | 6 ------ .../contracts/src/stake/MixinZrxVault.sol | 21 ------------------- .../MixinStakingPoolRewardVault.sol | 10 --------- .../staking/contracts/src/sys/MixinParams.sol | 1 + 4 files changed, 1 insertion(+), 37 deletions(-) diff --git a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol index 3bcafcdf07..cd5e77801b 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol @@ -134,10 +134,4 @@ interface IStakingEvents { bytes32 poolId, address makerAddress ); - - /// @dev Emitted by MixinStakingPoolRewardVault when the vault's address is changed. - /// @param rewardVaultAddress Address of new reward vault. - event StakingPoolRewardVaultChanged( - address rewardVaultAddress - ); } diff --git a/contracts/staking/contracts/src/stake/MixinZrxVault.sol b/contracts/staking/contracts/src/stake/MixinZrxVault.sol index 3b4f61e0be..1248f0b153 100644 --- a/contracts/staking/contracts/src/stake/MixinZrxVault.sol +++ b/contracts/staking/contracts/src/stake/MixinZrxVault.sol @@ -27,15 +27,6 @@ import "../immutable/MixinStorage.sol"; contract MixinZrxVault is MixinStorage { - /// @dev Set the Zrx Vault. - /// @param zrxVaultAddress Address of the Zrx Vault. - function setZrxVault(address zrxVaultAddress) - external - onlyOwner - { - zrxVault = IZrxVault(zrxVaultAddress); - } - /// @dev Deposits Zrx Tokens from the `owner` into the vault. /// @param owner of Zrx Tokens /// @param amount of tokens to deposit. @@ -43,10 +34,6 @@ contract MixinZrxVault is internal { IZrxVault _zrxVault = zrxVault; - require( - address(_zrxVault) != address(0), - "INVALID_ZRX_VAULT" - ); _zrxVault.depositFrom(owner, amount); } @@ -57,10 +44,6 @@ contract MixinZrxVault is internal { IZrxVault _zrxVault = zrxVault; - require( - address(_zrxVault) != address(0), - "INVALID_ZRX_VAULT" - ); _zrxVault.withdrawFrom(owner, amount); } @@ -72,10 +55,6 @@ contract MixinZrxVault is returns (uint256) { IZrxVault _zrxVault = zrxVault; - require( - address(_zrxVault) != address(0), - "INVALID_ZRX_VAULT" - ); return _zrxVault.balanceOf(owner); } } diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol index d13c99d17c..b3c75f858c 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol @@ -67,16 +67,6 @@ contract MixinStakingPoolRewardVault is _; } - /// @dev Sets the address of the reward vault. - /// This can only be called by the owner of this contract. - function setStakingPoolRewardVault(address payable rewardVaultAddress) - external - onlyOwner - { - rewardVault = IStakingPoolRewardVault(rewardVaultAddress); - emit StakingPoolRewardVaultChanged(rewardVaultAddress); - } - /// @dev Decreases the operator share for the given pool (i.e. increases pool rewards for members). /// Note that this is only callable by the pool operator, and will revert if the new operator /// share value is greater than the old value. diff --git a/contracts/staking/contracts/src/sys/MixinParams.sol b/contracts/staking/contracts/src/sys/MixinParams.sol index 0c470feed7..a750ea4e81 100644 --- a/contracts/staking/contracts/src/sys/MixinParams.sol +++ b/contracts/staking/contracts/src/sys/MixinParams.sol @@ -19,6 +19,7 @@ pragma solidity ^0.5.9; import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; +import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetProxy.sol"; import "../immutable/MixinStorage.sol"; import "../interfaces/IStakingEvents.sol"; import "../interfaces/IEthVault.sol"; From de9527ce2f2ba2879dcb8249370d0b7685c69459 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 16 Sep 2019 09:21:08 -0700 Subject: [PATCH 04/11] Do not initialize stakingProxy in vault constructors --- contracts/staking/contracts/src/Staking.sol | 18 ++++- .../staking/contracts/src/StakingProxy.sol | 45 +++++------ .../src/interfaces/IStakingEvents.sol | 2 + .../contracts/src/interfaces/IVaultCore.sol | 2 +- .../staking/contracts/src/sys/MixinParams.sol | 81 +++++++++---------- .../staking/contracts/src/vaults/EthVault.sol | 6 -- .../contracts/src/vaults/MixinVaultCore.sol | 21 +---- .../staking/contracts/src/vaults/ZrxVault.sol | 3 - .../contracts/test/TestProtocolFees.sol | 8 +- .../contracts/test/TestStakingProxy.sol | 13 +-- 10 files changed, 93 insertions(+), 106 deletions(-) diff --git a/contracts/staking/contracts/src/Staking.sol b/contracts/staking/contracts/src/Staking.sol index ab4fb9c529..919262325a 100644 --- a/contracts/staking/contracts/src/Staking.sol +++ b/contracts/staking/contracts/src/Staking.sol @@ -43,13 +43,27 @@ contract Staking is /// @dev Initialize storage owned by this contract. /// This function should not be called directly. /// The StakingProxy contract will call it in `attachStakingContract()`. - function init() + /// @param _wethProxyAddress The address that can transfer WETH for fees. + /// @param _ethVaultAddress Address of the EthVault contract. + /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. + /// @param _zrxVaultAddress Address of the ZrxVault contract. + function init( + address _wethProxyAddress, + address _ethVaultAddress, + address payable _rewardVaultAddress, + address _zrxVaultAddress + ) external onlyOwner { // DANGER! When performing upgrades, take care to modify this logic // to prevent accidentally clearing prior state. _initMixinScheduler(); - _initMixinParams(); + _initMixinParams( + _wethProxyAddress, + _ethVaultAddress, + _rewardVaultAddress, + _zrxVaultAddress + ); } } diff --git a/contracts/staking/contracts/src/StakingProxy.sol b/contracts/staking/contracts/src/StakingProxy.sol index 9cfc7a8da1..c2a69727af 100644 --- a/contracts/staking/contracts/src/StakingProxy.sol +++ b/contracts/staking/contracts/src/StakingProxy.sol @@ -50,7 +50,7 @@ contract StakingProxy is MixinStorage() { readOnlyProxy = _readOnlyProxy; - _attachStakingContract( + _init( _stakingContract, _wethProxyAddress, _ethVaultAddress, @@ -74,27 +74,11 @@ contract StakingProxy is /// @dev Attach a staking contract; future calls will be delegated to the staking contract. /// Note that this is callable only by this contract's owner. /// @param _stakingContract Address of staking contract. - /// @param _wethProxyAddress The address that can transfer WETH for fees. - /// @param _ethVaultAddress Address of the EthVault contract. - /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. - /// @param _zrxVaultAddress Address of the ZrxVault contract. - function attachStakingContract( - address _stakingContract, - address _wethProxyAddress, - address _ethVaultAddress, - address _rewardVaultAddress, - address _zrxVaultAddress - ) + function attachStakingContract(address _stakingContract) external onlyOwner { - _attachStakingContract( - _stakingContract, - _wethProxyAddress, - _ethVaultAddress, - _rewardVaultAddress, - _zrxVaultAddress - ); + _attachStakingContract(_stakingContract); } /// @dev Detach the current staking contract. @@ -117,7 +101,6 @@ contract StakingProxy is } else { stakingContract = readOnlyProxyCallee; } - emit ReadOnlyModeSet(readOnlyMode); } @@ -163,11 +146,20 @@ contract StakingProxy is /// @dev Attach a staking contract; future calls will be delegated to the staking contract. /// @param _stakingContract Address of staking contract. + function _attachStakingContract(address _stakingContract) + private + { + stakingContract = readOnlyProxyCallee = _stakingContract; + emit StakingContractAttachedToProxy(_stakingContract); + } + + /// @dev Initializes Staking contract specific state. + /// @param _stakingContract Address of staking contract. /// @param _wethProxyAddress The address that can transfer WETH for fees. /// @param _ethVaultAddress Address of the EthVault contract. /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. /// @param _zrxVaultAddress Address of the ZrxVault contract. - function _attachStakingContract( + function _init( address _stakingContract, address _wethProxyAddress, address _ethVaultAddress, @@ -176,9 +168,11 @@ contract StakingProxy is ) private { - stakingContract = readOnlyProxyCallee = _stakingContract; + // Attach the Staking contract + _attachStakingContract(_stakingContract); + // Call `init()` on the staking contract to initialize storage. - (bool didInitSucceed, bytes memory initReturnData) = _stakingContract.delegatecall( + (bool didInitSucceed, bytes memory initReturnData) = stakingContract.delegatecall( abi.encodeWithSelector( IStorageInit(0).init.selector, _wethProxyAddress, @@ -188,8 +182,9 @@ contract StakingProxy is ) ); if (!didInitSucceed) { - assembly { revert(add(initReturnData, 0x20), mload(initReturnData)) } + assembly { + revert(add(initReturnData, 0x20), mload(initReturnData)) + } } - emit StakingContractAttachedToProxy(_stakingContract); } } diff --git a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol index cd5e77801b..ba50eb09af 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol @@ -60,6 +60,7 @@ interface IStakingEvents { /// @param maximumMakersInPool Maximum number of maker addresses allowed to be registered to a pool. /// @param cobbDouglasAlphaNumerator Numerator for cobb douglas alpha factor. /// @param cobbDouglasAlphaDenomintor Denominator for cobb douglas alpha factor. + /// @param wethProxyAddress The address that can transfer WETH for fees. /// @param ethVaultAddress Address of the EthVault contract. /// @param rewardVaultAddress Address of the StakingPoolRewardVault contract. /// @param zrxVaultAddress Address of the ZrxVault contract. @@ -70,6 +71,7 @@ interface IStakingEvents { uint256 maximumMakersInPool, uint256 cobbDouglasAlphaNumerator, uint256 cobbDouglasAlphaDenomintor, + address wethProxyAddress, address ethVaultAddress, address rewardVaultAddress, address zrxVaultAddress diff --git a/contracts/staking/contracts/src/interfaces/IVaultCore.sol b/contracts/staking/contracts/src/interfaces/IVaultCore.sol index 78d2916ae4..d6eb5e39f2 100644 --- a/contracts/staking/contracts/src/interfaces/IVaultCore.sol +++ b/contracts/staking/contracts/src/interfaces/IVaultCore.sol @@ -41,7 +41,7 @@ interface IVaultCore { /// @dev Sets the address of the StakingProxy contract. /// Note that only the contract owner can call this function. - /// @param _stakingContractAddress Address of Staking proxy contract. + /// @param _stakingProxyAddress Address of Staking proxy contract. function setStakingProxy(address payable _stakingProxyAddress) external; diff --git a/contracts/staking/contracts/src/sys/MixinParams.sol b/contracts/staking/contracts/src/sys/MixinParams.sol index a750ea4e81..76922a188c 100644 --- a/contracts/staking/contracts/src/sys/MixinParams.sol +++ b/contracts/staking/contracts/src/sys/MixinParams.sol @@ -52,7 +52,7 @@ contract MixinParams is uint32 _cobbDouglasAlphaDenominator, address _wethProxyAddress, address _ethVaultAddress, - address _rewardVaultAddress, + address payable _rewardVaultAddress, address _zrxVaultAddress ) external @@ -111,31 +111,6 @@ contract MixinParams is _zrxVaultAddress = address(_zrxVaultAddress); } - /// @dev Assert param values before initializing them. - /// This must be updated for each migration. - function _assertMixinParamsBeforeInit() - internal - { - // Ensure state is uninitialized. - if (epochDurationInSeconds != 0 && - rewardDelegatedStakeWeight != 0 && - minimumPoolStake != 0 && - maximumMakersInPool != 0 && - cobbDouglasAlphaNumerator != 0 && - cobbDouglasAlphaDenomintor != 0 && - address(wethAssetProxy) != NIL_ADDRESS && - address(ethVault) != NIL_ADDRESS && - address(rewardVault) != NIL_ADDRESS && - address(zrxVault) != NIL_ADDRESS - ) { - LibRichErrors.rrevert( - LibStakingRichErrors.InitializationError( - LibStakingRichErrors.InitializationErrorCode.MixinParamsAlreadyInitialized - ) - ); - } - } - /// @dev Initialzize storage belonging to this mixin. /// @param _wethProxyAddress The address that can transfer WETH for fees. /// @param _ethVaultAddress Address of the EthVault contract. @@ -144,28 +119,23 @@ contract MixinParams is function _initMixinParams( address _wethProxyAddress, address _ethVaultAddress, - address _rewardVaultAddress, + address payable _rewardVaultAddress, address _zrxVaultAddress ) internal { - // assert the current values before overwriting them. - _assertMixinParamsBeforeInit(); + // Ensure state is uninitialized. + _assertStorageNotInitialized(); // Set up defaults. - _epochDurationInSeconds = 2 weeks; - _rewardDelegatedStakeWeight = (90 * PPM_DENOMINATOR) / 100; // 90% - _minimumPoolStake = 100 * MIN_TOKEN_VALUE; // 100 ZRX - _maximumMakersInPool = 10; - _cobbDouglasAlphaNumerator = 1; - _cobbDouglasAlphaDenomintor = 2; + // These cannot be set to variables, or we go over the stack variable limit. _setParams( - _epochDurationInSeconds, - _rewardDelegatedStakeWeight, - _minimumPoolStake, - _maximumMakersInPool, - _cobbDouglasAlphaNumerator, - _cobbDouglasAlphaDenomintor, + 2 weeks, // epochDurationInSeconds + (90 * PPM_DENOMINATOR) / 100, // rewardDelegatedStakeWeight + 100 * MIN_TOKEN_VALUE, // minimumPoolStake + 10, // maximumMakersInPool + 1, // cobbDouglasAlphaNumerator + 2, // cobbDouglasAlphaDenomintor _wethProxyAddress, _ethVaultAddress, _rewardVaultAddress, @@ -193,10 +163,10 @@ contract MixinParams is uint32 _cobbDouglasAlphaDenominator, address _wethProxyAddress, address _ethVaultAddress, - address _rewardVaultAddress, + address payable _rewardVaultAddress, address _zrxVaultAddress ) - internal + private { _assertValidRewardDelegatedStakeWeight(_rewardDelegatedStakeWeight); _assertValidMaximumMakersInPool(_maximumMakersInPool); @@ -237,6 +207,29 @@ contract MixinParams is ); } + function _assertStorageNotInitialized() + private + view + { + if (epochDurationInSeconds != 0 && + rewardDelegatedStakeWeight != 0 && + minimumPoolStake != 0 && + maximumMakersInPool != 0 && + cobbDouglasAlphaNumerator != 0 && + cobbDouglasAlphaDenomintor != 0 && + address(wethAssetProxy) != NIL_ADDRESS && + address(ethVault) != NIL_ADDRESS && + address(rewardVault) != NIL_ADDRESS && + address(zrxVault) != NIL_ADDRESS + ) { + LibRichErrors.rrevert( + LibStakingRichErrors.InitializationError( + LibStakingRichErrors.InitializationErrorCode.MixinParamsAlreadyInitialized + ) + ); + } + } + /// @dev Asserts that cobb douglas alpha values are valid. /// @param numerator Numerator for cobb douglas alpha factor. /// @param denominator Denominator for cobb douglas alpha factor. @@ -293,7 +286,7 @@ contract MixinParams is /// @param _ethVaultAddress Address of the EthVault contract. /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. /// @param _zrxVaultAddress Address of the ZrxVault contract. - function _assertValidVaultAddresses( + function _assertValidAddresses( address _wethProxyAddress, address _ethVaultAddress, address _rewardVaultAddress, diff --git a/contracts/staking/contracts/src/vaults/EthVault.sol b/contracts/staking/contracts/src/vaults/EthVault.sol index dc83149dac..8627feb626 100644 --- a/contracts/staking/contracts/src/vaults/EthVault.sol +++ b/contracts/staking/contracts/src/vaults/EthVault.sol @@ -33,12 +33,6 @@ contract EthVault is // mapping from Owner to ETH balance mapping (address => uint256) internal _balances; - /// @dev Constructor. - constructor(address _stakingProxyAddress) - public - MixinVaultCore(_stakingProxyAddress) - {} // solhint-disable-line no-empty-blocks - /// @dev Deposit an `amount` of ETH from `owner` into the vault. /// Note that only the Staking contract can call this. /// Note that this can only be called when *not* in Catostrophic Failure mode. diff --git a/contracts/staking/contracts/src/vaults/MixinVaultCore.sol b/contracts/staking/contracts/src/vaults/MixinVaultCore.sol index ef7033be6d..ed06bc2d13 100644 --- a/contracts/staking/contracts/src/vaults/MixinVaultCore.sol +++ b/contracts/staking/contracts/src/vaults/MixinVaultCore.sol @@ -45,13 +45,6 @@ contract MixinVaultCore is // True iff vault has been set to Catastrophic Failure Mode bool public isInCatastrophicFailure; - /// @dev Constructor. - constructor(address _stakingProxyAddress) - public - { - _setStakingProxy(_stakingProxyAddress); - } - /// @dev Asserts that the sender (`msg.sender`) is the staking contract. modifier onlyStakingProxy { if (msg.sender != stakingProxyAddress) { @@ -80,12 +73,13 @@ contract MixinVaultCore is /// @dev Sets the address of the StakingProxy contract. /// Note that only the contract owner can call this function. - /// @param _stakingContractAddress Address of Staking proxy contract. + /// @param _stakingProxyAddress Address of Staking proxy contract. function setStakingProxy(address payable _stakingProxyAddress) external onlyOwner { - _setStakingProxy(_stakingProxyContract); + stakingProxyAddress = _stakingProxyAddress; + emit StakingProxySet(_stakingProxyAddress); } /// @dev Vault enters into Catastrophic Failure Mode. @@ -98,13 +92,4 @@ contract MixinVaultCore is isInCatastrophicFailure = true; emit InCatastrophicFailureMode(msg.sender); } - - /// @dev Sets the address of the StakingProxy contract. - /// @param _stakingContractAddress Address of Staking proxy contract. - function _setStakingProxy(address payable _stakingProxyAddress) - internal - { - stakingProxyAddress = _stakingProxyAddress; - emit StakingProxySet(_stakingProxyAddress); - } } diff --git a/contracts/staking/contracts/src/vaults/ZrxVault.sol b/contracts/staking/contracts/src/vaults/ZrxVault.sol index 6993d8d20d..486a92d048 100644 --- a/contracts/staking/contracts/src/vaults/ZrxVault.sol +++ b/contracts/staking/contracts/src/vaults/ZrxVault.sol @@ -52,16 +52,13 @@ contract ZrxVault is bytes internal _zrxAssetData; /// @dev Constructor. - /// @param _stakingProxyAddress Address of StakingProxy contract. /// @param _zrxProxyAddress Address of the 0x Zrx Proxy. /// @param _zrxTokenAddress Address of the Zrx Token. constructor( - address _stakingProxyAddress, address _zrxProxyAddress, address _zrxTokenAddress ) public - MixinVaultCore(_stakingProxyAddress) { zrxAssetProxy = IAssetProxy(_zrxProxyAddress); _zrxToken = IERC20Token(_zrxTokenAddress); diff --git a/contracts/staking/contracts/test/TestProtocolFees.sol b/contracts/staking/contracts/test/TestProtocolFees.sol index 4a543405c6..e887159dfe 100644 --- a/contracts/staking/contracts/test/TestProtocolFees.sol +++ b/contracts/staking/contracts/test/TestProtocolFees.sol @@ -36,8 +36,12 @@ contract TestProtocolFees is constructor(address exchangeAddress, address wethProxyAddress) public { validExchanges[exchangeAddress] = true; - wethAssetProxy = IAssetProxy(wethProxyAddress); - _initMixinParams(); + _initMixinParams( + wethProxyAddress, + NIL_ADDRESS, + address(0), + NIL_ADDRESS + ); } function addMakerToPool(bytes32 poolId, address makerAddress) diff --git a/contracts/staking/contracts/test/TestStakingProxy.sol b/contracts/staking/contracts/test/TestStakingProxy.sol index 5e50267811..7373deb380 100644 --- a/contracts/staking/contracts/test/TestStakingProxy.sol +++ b/contracts/staking/contracts/test/TestStakingProxy.sol @@ -28,10 +28,13 @@ contract TestStakingProxy is // solhint-disable no-empty-blocks constructor(address _stakingContract) public - StakingProxy(_stakingContract, address(0), address(0)) + StakingProxy( + _stakingContract, + NIL_ADDRESS, + NIL_ADDRESS, + NIL_ADDRESS, + NIL_ADDRESS, + NIL_ADDRESS + ) {} - - function getAttachedContract() external view returns (address) { - return stakingContract; - } } From 6641af2a58755e4ea9abb55915076910bc400cb6 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 16 Sep 2019 09:21:30 -0700 Subject: [PATCH 05/11] Fix build --- contracts/staking/test/migration.ts | 32 ++++++--- contracts/staking/test/params.ts | 47 ++++++------- contracts/staking/test/protocol_fees.ts | 10 ++- contracts/staking/test/utils/api_wrapper.ts | 73 ++++++++------------- contracts/staking/test/utils/constants.ts | 11 +++- contracts/staking/test/utils/types.ts | 12 ++-- contracts/test-utils/src/address_utils.ts | 15 ++--- contracts/test-utils/src/index.ts | 2 +- 8 files changed, 103 insertions(+), 99 deletions(-) diff --git a/contracts/staking/test/migration.ts b/contracts/staking/test/migration.ts index 5cb300a93c..5ef89d0aa5 100644 --- a/contracts/staking/test/migration.ts +++ b/contracts/staking/test/migration.ts @@ -1,4 +1,4 @@ -import { blockchainTests, constants, expect, filterLogsToArguments, hexRandom } from '@0x/contracts-test-utils'; +import { blockchainTests, constants, expect, filterLogsToArguments, randomAddress } from '@0x/contracts-test-utils'; import { StakingRevertErrors } from '@0x/order-utils'; import { BigNumber, OwnableRevertErrors, StringRevertError } from '@0x/utils'; @@ -42,10 +42,6 @@ blockchainTests('Migration tests', env => { ); }); - function randomAddress(): string { - return hexRandom(constants.ADDRESS_LENGTH); - } - async function enableInitRevertsAsync(): Promise { const revertAddress = await initTargetContract.SHOULD_REVERT_ADDRESS.callAsync(); // Deposit some ether into `revertAddress` to signal `initTargetContract` @@ -67,7 +63,7 @@ blockchainTests('Migration tests', env => { }); expect(senderAddress).to.eq(ownerAddress); expect(thisAddress).to.eq(proxyContract.address); - const attachedAddress = await proxyContract.getAttachedContract.callAsync(); + const attachedAddress = await proxyContract.stakingContract.callAsync(); expect(attachedAddress).to.eq(initTargetContract.address); } @@ -150,14 +146,32 @@ blockchainTests('Migration tests', env => { }); it('throws if not called by owner', async () => { - const tx = stakingContract.init.awaitTransactionSuccessAsync({ from: notOwnerAddress }); + const tx = stakingContract.init.awaitTransactionSuccessAsync( + randomAddress(), + randomAddress(), + randomAddress(), + randomAddress(), + { + from: notOwnerAddress, + }, + ); const expectedError = new OwnableRevertErrors.OnlyOwnerError(notOwnerAddress, ownerAddress); return expect(tx).to.revertWith(expectedError); }); it('throws if already intitialized', async () => { - await stakingContract.init.awaitTransactionSuccessAsync(); - const tx = stakingContract.init.awaitTransactionSuccessAsync(); + await stakingContract.init.awaitTransactionSuccessAsync( + randomAddress(), + randomAddress(), + randomAddress(), + randomAddress(), + ); + const tx = stakingContract.init.awaitTransactionSuccessAsync( + randomAddress(), + randomAddress(), + randomAddress(), + randomAddress(), + ); const expectedError = new StakingRevertErrors.InitializationError(); return expect(tx).to.revertWith(expectedError); }); diff --git a/contracts/staking/test/params.ts b/contracts/staking/test/params.ts index 2924fd3098..2cf50d195c 100644 --- a/contracts/staking/test/params.ts +++ b/contracts/staking/test/params.ts @@ -1,8 +1,11 @@ -import { blockchainTests, constants, expect, filterLogsToArguments, Numberish } from '@0x/contracts-test-utils'; +import { blockchainTests, constants, expect, filterLogsToArguments } from '@0x/contracts-test-utils'; import { StakingRevertErrors } from '@0x/order-utils'; import { BigNumber, OwnableRevertErrors } from '@0x/utils'; -import { artifacts, IStakingEventsParamsChangedEventArgs, MixinParamsContract } from '../src/'; +import { artifacts, IStakingEventsParamsSetEventArgs, MixinParamsContract } from '../src/'; + +import { constants as stakingConstants } from './utils/constants'; +import { StakingParams } from './utils/types'; blockchainTests('Configurable Parameters', env => { let testContract: MixinParamsContract; @@ -20,29 +23,9 @@ blockchainTests('Configurable Parameters', env => { }); blockchainTests.resets('setParams()', () => { - interface Params { - epochDurationInSeconds: Numberish; - rewardDelegatedStakeWeight: Numberish; - minimumPoolStake: Numberish; - maximumMakersInPool: Numberish; - cobbDouglasAlphaNumerator: Numberish; - cobbDouglasAlphaDenomintor: Numberish; - } - - const TWO_WEEKS = 14 * 24 * 60 * 60; - const PPM_90_PERCENT = 10 ** 6 * 0.9; - const DEFAULT_PARAMS = { - epochDurationInSeconds: TWO_WEEKS, - rewardDelegatedStakeWeight: PPM_90_PERCENT, - minimumPoolStake: constants.DUMMY_TOKEN_DECIMALS.times(100), - maximumMakersInPool: 10, - cobbDouglasAlphaNumerator: 1, - cobbDouglasAlphaDenomintor: 2, - }; - - async function setParamsAndAssertAsync(params: Partial, from?: string): Promise { + async function setParamsAndAssertAsync(params: Partial, from?: string): Promise { const _params = { - ...DEFAULT_PARAMS, + ...stakingConstants.DEFAULT_PARAMS, ...params, }; const receipt = await testContract.setParams.awaitTransactionSuccessAsync( @@ -52,17 +35,25 @@ blockchainTests('Configurable Parameters', env => { new BigNumber(_params.maximumMakersInPool), new BigNumber(_params.cobbDouglasAlphaNumerator), new BigNumber(_params.cobbDouglasAlphaDenomintor), + _params.wethProxyAddress, + _params.ethVaultAddress, + _params.rewardVaultAddress, + _params.zrxVaultAddress, { from }, ); // Assert event. expect(receipt.logs.length).to.eq(1); - const event = filterLogsToArguments(receipt.logs, 'ParamsChanged')[0]; + const event = filterLogsToArguments(receipt.logs, 'ParamsChanged')[0]; expect(event.epochDurationInSeconds).to.bignumber.eq(_params.epochDurationInSeconds); expect(event.rewardDelegatedStakeWeight).to.bignumber.eq(_params.rewardDelegatedStakeWeight); expect(event.minimumPoolStake).to.bignumber.eq(_params.minimumPoolStake); expect(event.maximumMakersInPool).to.bignumber.eq(_params.maximumMakersInPool); expect(event.cobbDouglasAlphaNumerator).to.bignumber.eq(_params.cobbDouglasAlphaNumerator); expect(event.cobbDouglasAlphaDenomintor).to.bignumber.eq(_params.cobbDouglasAlphaDenomintor); + expect(event.wethProxyAddress).to.eq(_params.wethProxyAddress); + expect(event.ethVaultAddress).to.eq(_params.ethVaultAddress); + expect(event.rewardVaultAddress).to.eq(_params.rewardVaultAddress); + expect(event.zrxVaultAddress).to.eq(_params.zrxVaultAddress); // Assert `getParams()`. const actual = await testContract.getParams.callAsync(); expect(actual[0]).to.bignumber.eq(_params.epochDurationInSeconds); @@ -71,6 +62,10 @@ blockchainTests('Configurable Parameters', env => { expect(actual[3]).to.bignumber.eq(_params.maximumMakersInPool); expect(actual[4]).to.bignumber.eq(_params.cobbDouglasAlphaNumerator); expect(actual[5]).to.bignumber.eq(_params.cobbDouglasAlphaDenomintor); + expect(actual[6]).to.eq(_params.wethProxyAddress); + expect(actual[7]).to.eq(_params.ethVaultAddress); + expect(actual[8]).to.eq(_params.rewardVaultAddress); + expect(actual[9]).to.eq(_params.zrxVaultAddress); } it('throws if not called by owner', async () => { @@ -99,7 +94,7 @@ blockchainTests('Configurable Parameters', env => { describe('maximumMakersInPool', () => { it('throws when == 0', async () => { const params = { - maximumMakersInPool: 0, + maximumMakersInPool: constants.ZERO_AMOUNT, }; const tx = setParamsAndAssertAsync(params); const expectedError = new StakingRevertErrors.InvalidParamValueError( diff --git a/contracts/staking/test/protocol_fees.ts b/contracts/staking/test/protocol_fees.ts index dd9250ff09..34ea87a11f 100644 --- a/contracts/staking/test/protocol_fees.ts +++ b/contracts/staking/test/protocol_fees.ts @@ -1,4 +1,11 @@ -import { blockchainTests, constants, expect, filterLogsToArguments, hexRandom } from '@0x/contracts-test-utils'; +import { + blockchainTests, + constants, + expect, + filterLogsToArguments, + hexRandom, + randomAddress, +} from '@0x/contracts-test-utils'; import { StakingRevertErrors } from '@0x/order-utils'; import { BigNumber } from '@0x/utils'; import { LogEntry } from 'ethereum-types'; @@ -54,7 +61,6 @@ blockchainTests('Protocol Fee Unit Tests', env => { } blockchainTests.resets('payProtocolFee()', () => { - const randomAddress = () => hexRandom(constants.ADDRESS_LENGTH); const DEFAULT_PROTOCOL_FEE_PAID = new BigNumber(150e3).times(1e9); const { ZERO_AMOUNT } = constants; const makerAddress = randomAddress(); diff --git a/contracts/staking/test/utils/api_wrapper.ts b/contracts/staking/test/utils/api_wrapper.ts index dd704c849f..f98bc5a160 100644 --- a/contracts/staking/test/utils/api_wrapper.ts +++ b/contracts/staking/test/utils/api_wrapper.ts @@ -79,6 +79,10 @@ export class StakingApiWrapper { _params.maximumMakersInPool, _params.cobbDouglasAlphaNumerator, _params.cobbDouglasAlphaDenomintor, + _params.wethProxyAddress, + _params.ethVaultAddress, + _params.rewardVaultAddress, + _params.zrxVaultAddress, ); }, @@ -164,17 +168,20 @@ export async function deployAndConfigureContractsAsync( txDefaults, artifacts, ); - // deploy staking proxy - const stakingProxyContract = await StakingProxyContract.deployFrom0xArtifactAsync( - artifacts.StakingProxy, + // deploy eth vault + const ethVaultContract = await EthVaultContract.deployFrom0xArtifactAsync( + artifacts.EthVault, + env.provider, + txDefaults, + artifacts, + ); + // deploy reward vault + const rewardVaultContract = await StakingPoolRewardVaultContract.deployFrom0xArtifactAsync( + artifacts.StakingPoolRewardVault, env.provider, txDefaults, artifacts, - stakingContract.address, - readOnlyProxyContract.address, - erc20ProxyContract.address, ); - // deploy zrx vault const zrxVaultContract = await ZrxVaultContract.deployFrom0xArtifactAsync( artifacts.ZrxVault, @@ -184,48 +191,26 @@ export async function deployAndConfigureContractsAsync( erc20ProxyContract.address, zrxTokenContract.address, ); - // configure erc20 proxy to accept calls from zrx vault - await erc20ProxyContract.addAuthorizedAddress.awaitTransactionSuccessAsync(zrxVaultContract.address); - // set staking proxy contract in zrx vault - await zrxVaultContract.setStakingContract.awaitTransactionSuccessAsync(stakingProxyContract.address); - // set zrx vault in staking contract - const setZrxVaultCalldata = stakingContract.setZrxVault.getABIEncodedTransactionData(zrxVaultContract.address); - const setZrxVaultTxData = { - from: ownerAddress, - to: stakingProxyContract.address, - data: setZrxVaultCalldata, - }; - await env.web3Wrapper.awaitTransactionSuccessAsync(await env.web3Wrapper.sendTransactionAsync(setZrxVaultTxData)); - // deploy eth vault - const ethVaultContract = await EthVaultContract.deployFrom0xArtifactAsync( - artifacts.EthVault, - env.provider, - txDefaults, - artifacts, - ); - // deploy reward vault - const rewardVaultContract = await StakingPoolRewardVaultContract.deployFrom0xArtifactAsync( - artifacts.StakingPoolRewardVault, + // deploy staking proxy + const stakingProxyContract = await StakingProxyContract.deployFrom0xArtifactAsync( + artifacts.StakingProxy, env.provider, txDefaults, artifacts, - ); - // set eth vault in reward vault - await rewardVaultContract.setEthVault.sendTransactionAsync(ethVaultContract.address); - // set staking proxy contract in reward vault - await rewardVaultContract.setStakingContract.awaitTransactionSuccessAsync(stakingProxyContract.address); - // set reward vault in staking contract - const setStakingPoolRewardVaultCalldata = stakingContract.setStakingPoolRewardVault.getABIEncodedTransactionData( + stakingContract.address, + readOnlyProxyContract.address, + erc20ProxyContract.address, + ethVaultContract.address, rewardVaultContract.address, + zrxVaultContract.address, ); - const setStakingPoolRewardVaultTxData = { - from: ownerAddress, - to: stakingProxyContract.address, - data: setStakingPoolRewardVaultCalldata, - }; - await env.web3Wrapper.awaitTransactionSuccessAsync( - await env.web3Wrapper.sendTransactionAsync(setStakingPoolRewardVaultTxData), - ); + + // configure erc20 proxy to accept calls from zrx vault + await erc20ProxyContract.addAuthorizedAddress.awaitTransactionSuccessAsync(zrxVaultContract.address); + // set staking proxy contract in zrx vault + await zrxVaultContract.setStakingProxy.awaitTransactionSuccessAsync(stakingProxyContract.address); + // set staking proxy contract in reward vault + await rewardVaultContract.setStakingProxy.awaitTransactionSuccessAsync(stakingProxyContract.address); return new StakingApiWrapper( env, diff --git a/contracts/staking/test/utils/constants.ts b/contracts/staking/test/utils/constants.ts index 3093a86623..acb627f2b9 100644 --- a/contracts/staking/test/utils/constants.ts +++ b/contracts/staking/test/utils/constants.ts @@ -1,7 +1,8 @@ -import { constants as testConstants } from '@0x/contracts-test-utils'; +import { constants as testConstants, randomAddress } from '@0x/contracts-test-utils'; import { BigNumber } from '@0x/utils'; const TWO_WEEKS = 14 * 24 * 60 * 60; +const PPM_90_PERCENT = 10 ** 6 * 0.9; export const constants = { TOKEN_MULTIPLIER: testConstants.DUMMY_TOKEN_DECIMALS, INITIAL_POOL_ID: '0x0000000000000000000000000000000100000000000000000000000000000000', @@ -11,10 +12,14 @@ export const constants = { INITIAL_EPOCH: new BigNumber(0), DEFAULT_PARAMS: { epochDurationInSeconds: new BigNumber(TWO_WEEKS), - rewardDelegatedStakeWeight: new BigNumber(0.9 * 1e6), // 90% - minimumPoolStake: testConstants.DUMMY_TOKEN_DECIMALS.times(100), // 100 ZRX + rewardDelegatedStakeWeight: new BigNumber(PPM_90_PERCENT), + minimumPoolStake: testConstants.DUMMY_TOKEN_DECIMALS.times(100), maximumMakersInPool: new BigNumber(10), cobbDouglasAlphaNumerator: new BigNumber(1), cobbDouglasAlphaDenomintor: new BigNumber(2), + wethProxyAddress: randomAddress(), + ethVaultAddress: randomAddress(), + rewardVaultAddress: randomAddress(), + zrxVaultAddress: randomAddress(), }, }; diff --git a/contracts/staking/test/utils/types.ts b/contracts/staking/test/utils/types.ts index 87efe0ecf8..e5569c16da 100644 --- a/contracts/staking/test/utils/types.ts +++ b/contracts/staking/test/utils/types.ts @@ -4,11 +4,15 @@ import { constants } from './constants'; export interface StakingParams { epochDurationInSeconds: BigNumber; - rewardDelegatedStakeWeight: BigNumber; + rewardDelegatedStakeWeight: number | BigNumber; minimumPoolStake: BigNumber; - maxMakersInPool: BigNumber; - cobbDouglasAlphaNumerator: BigNumber; - cobbDouglasAlphaDenomintor: BigNumber; + maximumMakersInPool: BigNumber; + cobbDouglasAlphaNumerator: number | BigNumber; + cobbDouglasAlphaDenomintor: number | BigNumber; + wethProxyAddress: string; + ethVaultAddress: string; + rewardVaultAddress: string; + zrxVaultAddress: string; } export interface StakerBalances { diff --git a/contracts/test-utils/src/address_utils.ts b/contracts/test-utils/src/address_utils.ts index 634da0c164..37858ef568 100644 --- a/contracts/test-utils/src/address_utils.ts +++ b/contracts/test-utils/src/address_utils.ts @@ -1,11 +1,6 @@ -import { generatePseudoRandomSalt } from '@0x/order-utils'; -import { crypto } from '@0x/order-utils/lib/src/crypto'; +import { constants } from './constants'; +import { hexRandom } from './hex_utils'; -export const addressUtils = { - generatePseudoRandomAddress(): string { - const randomBigNum = generatePseudoRandomSalt(); - const randomBuff = crypto.solSHA3([randomBigNum]); - const randomAddress = `0x${randomBuff.slice(0, 20).toString('hex')}`; - return randomAddress; - }, -}; +export function randomAddress(): string { + return hexRandom(constants.ADDRESS_LENGTH); +} diff --git a/contracts/test-utils/src/index.ts b/contracts/test-utils/src/index.ts index 5ae2b898c5..4156b23991 100644 --- a/contracts/test-utils/src/index.ts +++ b/contracts/test-utils/src/index.ts @@ -22,7 +22,7 @@ export { typeEncodingUtils } from './type_encoding_utils'; export { profiler } from './profiler'; export { coverage } from './coverage'; export { Web3ProviderEngine } from '@0x/subproviders'; -export { addressUtils } from './address_utils'; +export { randomAddress } from './address_utils'; export { OrderFactory } from './order_factory'; export { bytes32Values, testCombinatoriallyWithReferenceFunc, uint256Values } from './combinatorial_utils'; export { TransactionFactory } from './transaction_factory'; From de567da846d7dc3102033e7fe98bc03cc4df9b94 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 16 Sep 2019 19:40:24 -0700 Subject: [PATCH 06/11] Fix typo across files --- .../contracts/src/fees/MixinExchangeFees.sol | 2 +- .../contracts/src/immutable/MixinStorage.sol | 2 +- .../contracts/src/interfaces/IStakingEvents.sol | 4 ++-- .../contracts/src/interfaces/IStorage.sol | 2 +- .../staking/contracts/src/sys/MixinParams.sol | 8 ++++---- .../staking/contracts/test/TestStorageLayout.sol | 2 +- contracts/staking/test/params.ts | 16 ++++++++-------- contracts/staking/test/rewards_test.ts | 5 ++++- contracts/staking/test/utils/api_wrapper.ts | 11 ++++++++--- contracts/staking/test/utils/constants.ts | 2 +- contracts/staking/test/utils/types.ts | 2 +- 11 files changed, 32 insertions(+), 24 deletions(-) diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index 5b8befac37..152e8485e1 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol @@ -252,7 +252,7 @@ contract MixinExchangeFees is totalWeightedStake != 0 ? activePools[i].weightedStake : 1, // only rewards are accounted for if no one has staked totalWeightedStake != 0 ? totalWeightedStake : 1, // this is to avoid divide-by-zero in cobb douglas cobbDouglasAlphaNumerator, - cobbDouglasAlphaDenomintor + cobbDouglasAlphaDenominator ); // record reward in vault diff --git a/contracts/staking/contracts/src/immutable/MixinStorage.sol b/contracts/staking/contracts/src/immutable/MixinStorage.sol index 85d5f64182..98ac6c237a 100644 --- a/contracts/staking/contracts/src/immutable/MixinStorage.sol +++ b/contracts/staking/contracts/src/immutable/MixinStorage.sol @@ -129,5 +129,5 @@ contract MixinStorage is uint32 public cobbDouglasAlphaNumerator; // Denominator for cobb douglas alpha factor. - uint32 public cobbDouglasAlphaDenomintor; + uint32 public cobbDouglasAlphaDenominator; } diff --git a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol index ba50eb09af..e538bf7c16 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol @@ -59,7 +59,7 @@ interface IStakingEvents { /// @param minimumPoolStake Minimum amount of stake required in a pool to collect rewards. /// @param maximumMakersInPool Maximum number of maker addresses allowed to be registered to a pool. /// @param cobbDouglasAlphaNumerator Numerator for cobb douglas alpha factor. - /// @param cobbDouglasAlphaDenomintor Denominator for cobb douglas alpha factor. + /// @param cobbDouglasAlphaDenominator Denominator for cobb douglas alpha factor. /// @param wethProxyAddress The address that can transfer WETH for fees. /// @param ethVaultAddress Address of the EthVault contract. /// @param rewardVaultAddress Address of the StakingPoolRewardVault contract. @@ -70,7 +70,7 @@ interface IStakingEvents { uint256 minimumPoolStake, uint256 maximumMakersInPool, uint256 cobbDouglasAlphaNumerator, - uint256 cobbDouglasAlphaDenomintor, + uint256 cobbDouglasAlphaDenominator, address wethProxyAddress, address ethVaultAddress, address rewardVaultAddress, diff --git a/contracts/staking/contracts/src/interfaces/IStorage.sol b/contracts/staking/contracts/src/interfaces/IStorage.sol index 9d386382d9..cdb6b3552c 100644 --- a/contracts/staking/contracts/src/interfaces/IStorage.sol +++ b/contracts/staking/contracts/src/interfaces/IStorage.sol @@ -128,7 +128,7 @@ interface IStorage { view returns (uint32); - function cobbDouglasAlphaDenomintor() + function cobbDouglasAlphaDenominator() external view returns (uint32); diff --git a/contracts/staking/contracts/src/sys/MixinParams.sol b/contracts/staking/contracts/src/sys/MixinParams.sol index 76922a188c..19e4399226 100644 --- a/contracts/staking/contracts/src/sys/MixinParams.sol +++ b/contracts/staking/contracts/src/sys/MixinParams.sol @@ -104,7 +104,7 @@ contract MixinParams is _minimumPoolStake = minimumPoolStake; _maximumMakersInPool = maximumMakersInPool; _cobbDouglasAlphaNumerator = cobbDouglasAlphaNumerator; - _cobbDouglasAlphaDenominator = cobbDouglasAlphaDenomintor; + _cobbDouglasAlphaDenominator = cobbDouglasAlphaDenominator; _wethProxyAddress = address(wethAssetProxy); _ethVaultAddress = address(ethVault); _rewardVaultAddress = address(rewardVault); @@ -135,7 +135,7 @@ contract MixinParams is 100 * MIN_TOKEN_VALUE, // minimumPoolStake 10, // maximumMakersInPool 1, // cobbDouglasAlphaNumerator - 2, // cobbDouglasAlphaDenomintor + 2, // cobbDouglasAlphaDenominator _wethProxyAddress, _ethVaultAddress, _rewardVaultAddress, @@ -187,7 +187,7 @@ contract MixinParams is minimumPoolStake = _minimumPoolStake; maximumMakersInPool = _maximumMakersInPool; cobbDouglasAlphaNumerator = _cobbDouglasAlphaNumerator; - cobbDouglasAlphaDenomintor = _cobbDouglasAlphaDenominator; + cobbDouglasAlphaDenominator = _cobbDouglasAlphaDenominator; wethAssetProxy = IAssetProxy(_wethProxyAddress); ethVault = IEthVault(_ethVaultAddress); rewardVault = IStakingPoolRewardVault(_rewardVaultAddress); @@ -216,7 +216,7 @@ contract MixinParams is minimumPoolStake != 0 && maximumMakersInPool != 0 && cobbDouglasAlphaNumerator != 0 && - cobbDouglasAlphaDenomintor != 0 && + cobbDouglasAlphaDenominator != 0 && address(wethAssetProxy) != NIL_ADDRESS && address(ethVault) != NIL_ADDRESS && address(rewardVault) != NIL_ADDRESS && diff --git a/contracts/staking/contracts/test/TestStorageLayout.sol b/contracts/staking/contracts/test/TestStorageLayout.sol index 5603773c02..0a4ae57f1a 100644 --- a/contracts/staking/contracts/test/TestStorageLayout.sol +++ b/contracts/staking/contracts/test/TestStorageLayout.sol @@ -127,7 +127,7 @@ contract TestStorageLayout is if sub(cobbDouglasAlphaNumerator_slot, slot) { revertIncorrectStorageSlot() } slot := add(slot, 1) - if sub(cobbDouglasAlphaDenomintor_slot, slot) { revertIncorrectStorageSlot() } + if sub(cobbDouglasAlphaDenominator_slot, slot) { revertIncorrectStorageSlot() } slot := add(slot, 1) } } diff --git a/contracts/staking/test/params.ts b/contracts/staking/test/params.ts index 2cf50d195c..29e164c3f5 100644 --- a/contracts/staking/test/params.ts +++ b/contracts/staking/test/params.ts @@ -34,7 +34,7 @@ blockchainTests('Configurable Parameters', env => { new BigNumber(_params.minimumPoolStake), new BigNumber(_params.maximumMakersInPool), new BigNumber(_params.cobbDouglasAlphaNumerator), - new BigNumber(_params.cobbDouglasAlphaDenomintor), + new BigNumber(_params.cobbDouglasAlphaDenominator), _params.wethProxyAddress, _params.ethVaultAddress, _params.rewardVaultAddress, @@ -49,7 +49,7 @@ blockchainTests('Configurable Parameters', env => { expect(event.minimumPoolStake).to.bignumber.eq(_params.minimumPoolStake); expect(event.maximumMakersInPool).to.bignumber.eq(_params.maximumMakersInPool); expect(event.cobbDouglasAlphaNumerator).to.bignumber.eq(_params.cobbDouglasAlphaNumerator); - expect(event.cobbDouglasAlphaDenomintor).to.bignumber.eq(_params.cobbDouglasAlphaDenomintor); + expect(event.cobbDouglasAlphaDenominator).to.bignumber.eq(_params.cobbDouglasAlphaDenominator); expect(event.wethProxyAddress).to.eq(_params.wethProxyAddress); expect(event.ethVaultAddress).to.eq(_params.ethVaultAddress); expect(event.rewardVaultAddress).to.eq(_params.rewardVaultAddress); @@ -61,7 +61,7 @@ blockchainTests('Configurable Parameters', env => { expect(actual[2]).to.bignumber.eq(_params.minimumPoolStake); expect(actual[3]).to.bignumber.eq(_params.maximumMakersInPool); expect(actual[4]).to.bignumber.eq(_params.cobbDouglasAlphaNumerator); - expect(actual[5]).to.bignumber.eq(_params.cobbDouglasAlphaDenomintor); + expect(actual[5]).to.bignumber.eq(_params.cobbDouglasAlphaDenominator); expect(actual[6]).to.eq(_params.wethProxyAddress); expect(actual[7]).to.eq(_params.ethVaultAddress); expect(actual[8]).to.eq(_params.rewardVaultAddress); @@ -108,7 +108,7 @@ blockchainTests('Configurable Parameters', env => { it('throws with denominator == 0', async () => { const params = { cobbDouglasAlphaNumerator: 0, - cobbDouglasAlphaDenomintor: 0, + cobbDouglasAlphaDenominator: 0, }; const tx = setParamsAndAssertAsync(params); const expectedError = new StakingRevertErrors.InvalidParamValueError( @@ -120,7 +120,7 @@ blockchainTests('Configurable Parameters', env => { it('throws with numerator > denominator', async () => { const params = { cobbDouglasAlphaNumerator: 2, - cobbDouglasAlphaDenomintor: 1, + cobbDouglasAlphaDenominator: 1, }; const tx = setParamsAndAssertAsync(params); const expectedError = new StakingRevertErrors.InvalidParamValueError( @@ -132,7 +132,7 @@ blockchainTests('Configurable Parameters', env => { it('accepts numerator == denominator', async () => { const params = { cobbDouglasAlphaNumerator: 1, - cobbDouglasAlphaDenomintor: 1, + cobbDouglasAlphaDenominator: 1, }; return setParamsAndAssertAsync(params); }); @@ -140,7 +140,7 @@ blockchainTests('Configurable Parameters', env => { it('accepts numerator < denominator', async () => { const params = { cobbDouglasAlphaNumerator: 1, - cobbDouglasAlphaDenomintor: 2, + cobbDouglasAlphaDenominator: 2, }; return setParamsAndAssertAsync(params); }); @@ -148,7 +148,7 @@ blockchainTests('Configurable Parameters', env => { it('accepts numerator == 0', async () => { const params = { cobbDouglasAlphaNumerator: 0, - cobbDouglasAlphaDenomintor: 1, + cobbDouglasAlphaDenominator: 1, }; return setParamsAndAssertAsync(params); }); diff --git a/contracts/staking/test/rewards_test.ts b/contracts/staking/test/rewards_test.ts index f7c63c5ce7..02bf3ece51 100644 --- a/contracts/staking/test/rewards_test.ts +++ b/contracts/staking/test/rewards_test.ts @@ -45,7 +45,10 @@ blockchainTests.resets('Testing Rewards', env => { await stakingApiWrapper.utils.setParamsAsync({ minimumPoolStake: new BigNumber(0), cobbDouglasAlphaNumerator: new BigNumber(1), - cobbDouglasAlphaDenomintor: new BigNumber(6), + cobbDouglasAlphaDenominator: new BigNumber(6), + rewardVaultAddress: stakingApiWrapper.rewardVaultContract.address, + ethVaultAddress: stakingApiWrapper.ethVaultContract.address, + zrxVaultAddress: stakingApiWrapper.zrxVaultContract.address, }); // setup stakers stakers = [new StakerActor(actors[0], stakingApiWrapper), new StakerActor(actors[1], stakingApiWrapper)]; diff --git a/contracts/staking/test/utils/api_wrapper.ts b/contracts/staking/test/utils/api_wrapper.ts index f98bc5a160..bbc4dca8db 100644 --- a/contracts/staking/test/utils/api_wrapper.ts +++ b/contracts/staking/test/utils/api_wrapper.ts @@ -78,7 +78,7 @@ export class StakingApiWrapper { _params.minimumPoolStake, _params.maximumMakersInPool, _params.cobbDouglasAlphaNumerator, - _params.cobbDouglasAlphaDenomintor, + _params.cobbDouglasAlphaDenominator, _params.wethProxyAddress, _params.ethVaultAddress, _params.rewardVaultAddress, @@ -94,7 +94,11 @@ export class StakingApiWrapper { 'minimumPoolStake', 'maximumMakersInPool', 'cobbDouglasAlphaNumerator', - 'cobbDouglasAlphaDenomintor', + 'cobbDouglasAlphaDenominator', + 'wethProxyAddress', + 'ethVaultAddress', + 'rewardVaultAddress', + 'zrxVaultAddress', ], await this.stakingContract.getParams.callAsync(), ) as any) as StakingParams; @@ -211,7 +215,8 @@ export async function deployAndConfigureContractsAsync( await zrxVaultContract.setStakingProxy.awaitTransactionSuccessAsync(stakingProxyContract.address); // set staking proxy contract in reward vault await rewardVaultContract.setStakingProxy.awaitTransactionSuccessAsync(stakingProxyContract.address); - + // set the eth vault in the reward vault + await rewardVaultContract.setEthVault.awaitTransactionSuccessAsync(ethVaultContract.address); return new StakingApiWrapper( env, ownerAddress, diff --git a/contracts/staking/test/utils/constants.ts b/contracts/staking/test/utils/constants.ts index acb627f2b9..3e2e9d4450 100644 --- a/contracts/staking/test/utils/constants.ts +++ b/contracts/staking/test/utils/constants.ts @@ -16,7 +16,7 @@ export const constants = { minimumPoolStake: testConstants.DUMMY_TOKEN_DECIMALS.times(100), maximumMakersInPool: new BigNumber(10), cobbDouglasAlphaNumerator: new BigNumber(1), - cobbDouglasAlphaDenomintor: new BigNumber(2), + cobbDouglasAlphaDenominator: new BigNumber(2), wethProxyAddress: randomAddress(), ethVaultAddress: randomAddress(), rewardVaultAddress: randomAddress(), diff --git a/contracts/staking/test/utils/types.ts b/contracts/staking/test/utils/types.ts index e5569c16da..e5dac3b91c 100644 --- a/contracts/staking/test/utils/types.ts +++ b/contracts/staking/test/utils/types.ts @@ -8,7 +8,7 @@ export interface StakingParams { minimumPoolStake: BigNumber; maximumMakersInPool: BigNumber; cobbDouglasAlphaNumerator: number | BigNumber; - cobbDouglasAlphaDenomintor: number | BigNumber; + cobbDouglasAlphaDenominator: number | BigNumber; wethProxyAddress: string; ethVaultAddress: string; rewardVaultAddress: string; From 4705b151880ec1479b66bda4dbd07c97e0c9a617 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 16 Sep 2019 21:55:52 -0700 Subject: [PATCH 07/11] Add addresses back as optional params to attachStakingContract --- .../staking/contracts/src/StakingProxy.sol | 44 +++++---- .../src/interfaces/IStakingProxy.sol | 16 +++- .../staking/contracts/src/sys/MixinParams.sol | 3 +- .../staking/contracts/test/TestInitTarget.sol | 25 +++++- .../contracts/test/TestProtocolFees.sol | 14 +-- .../contracts/test/TestStakingProxy.sol | 15 +++- contracts/staking/test/migration.ts | 89 +++++++++++++++++-- contracts/staking/test/params.ts | 2 +- 8 files changed, 167 insertions(+), 41 deletions(-) diff --git a/contracts/staking/contracts/src/StakingProxy.sol b/contracts/staking/contracts/src/StakingProxy.sol index c2a69727af..809c902c1a 100644 --- a/contracts/staking/contracts/src/StakingProxy.sol +++ b/contracts/staking/contracts/src/StakingProxy.sol @@ -50,7 +50,7 @@ contract StakingProxy is MixinStorage() { readOnlyProxy = _readOnlyProxy; - _init( + _attachStakingContract( _stakingContract, _wethProxyAddress, _ethVaultAddress, @@ -73,12 +73,32 @@ contract StakingProxy is /// @dev Attach a staking contract; future calls will be delegated to the staking contract. /// Note that this is callable only by this contract's owner. - /// @param _stakingContract Address of staking contract. - function attachStakingContract(address _stakingContract) + /// @param _stakingContract Address of staking contract. + /// @param _wethProxyAddress The address that can transfer WETH for fees. + /// Use address in storage if NIL_ADDRESS is passed in. + /// @param _ethVaultAddress Address of the EthVault contract. + /// Use address in storage if NIL_ADDRESS is passed in. + /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. + /// Use address in storage if NIL_ADDRESS is passed in. + /// @param _zrxVaultAddress Address of the ZrxVault contract. + /// Use address in storage if NIL_ADDRESS is passed in. + function attachStakingContract( + address _stakingContract, + address _wethProxyAddress, + address _ethVaultAddress, + address _rewardVaultAddress, + address _zrxVaultAddress + ) external onlyOwner { - _attachStakingContract(_stakingContract); + _attachStakingContract( + _stakingContract, + _wethProxyAddress == NIL_ADDRESS ? address(wethAssetProxy) : _wethProxyAddress, + _ethVaultAddress == NIL_ADDRESS ? address(ethVault) : _ethVaultAddress, + _rewardVaultAddress == NIL_ADDRESS ? address(rewardVault) : _rewardVaultAddress, + _zrxVaultAddress == NIL_ADDRESS ? address(zrxVault) : _zrxVaultAddress + ); } /// @dev Detach the current staking contract. @@ -146,20 +166,11 @@ contract StakingProxy is /// @dev Attach a staking contract; future calls will be delegated to the staking contract. /// @param _stakingContract Address of staking contract. - function _attachStakingContract(address _stakingContract) - private - { - stakingContract = readOnlyProxyCallee = _stakingContract; - emit StakingContractAttachedToProxy(_stakingContract); - } - - /// @dev Initializes Staking contract specific state. - /// @param _stakingContract Address of staking contract. /// @param _wethProxyAddress The address that can transfer WETH for fees. /// @param _ethVaultAddress Address of the EthVault contract. /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. /// @param _zrxVaultAddress Address of the ZrxVault contract. - function _init( + function _attachStakingContract( address _stakingContract, address _wethProxyAddress, address _ethVaultAddress, @@ -168,8 +179,9 @@ contract StakingProxy is ) private { - // Attach the Staking contract - _attachStakingContract(_stakingContract); + // Attach the staking contract + stakingContract = readOnlyProxyCallee = _stakingContract; + emit StakingContractAttachedToProxy(_stakingContract); // Call `init()` on the staking contract to initialize storage. (bool didInitSucceed, bytes memory initReturnData) = stakingContract.delegatecall( diff --git a/contracts/staking/contracts/src/interfaces/IStakingProxy.sol b/contracts/staking/contracts/src/interfaces/IStakingProxy.sol index 495a9035e5..b5b43e20ce 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingProxy.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingProxy.sol @@ -45,7 +45,21 @@ interface IStakingProxy /* is IStaking */ /// @dev Attach a staking contract; future calls will be delegated to the staking contract. /// Note that this is callable only by this contract's owner. /// @param _stakingContract Address of staking contract. - function attachStakingContract(address _stakingContract) + /// @param _wethProxyAddress The address that can transfer WETH for fees. + /// Use address in storage if NIL_ADDRESS is passed in. + /// @param _ethVaultAddress Address of the EthVault contract. + /// Use address in storage if NIL_ADDRESS is passed in. + /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. + /// Use address in storage if NIL_ADDRESS is passed in. + /// @param _zrxVaultAddress Address of the ZrxVault contract. + /// Use address in storage if NIL_ADDRESS is passed in. + function attachStakingContract( + address _stakingContract, + address _wethProxyAddress, + address _ethVaultAddress, + address _rewardVaultAddress, + address _zrxVaultAddress + ) external; /// @dev Detach the current staking contract. diff --git a/contracts/staking/contracts/src/sys/MixinParams.sol b/contracts/staking/contracts/src/sys/MixinParams.sol index 19e4399226..9ca23257b5 100644 --- a/contracts/staking/contracts/src/sys/MixinParams.sol +++ b/contracts/staking/contracts/src/sys/MixinParams.sol @@ -108,7 +108,7 @@ contract MixinParams is _wethProxyAddress = address(wethAssetProxy); _ethVaultAddress = address(ethVault); _rewardVaultAddress = address(rewardVault); - _zrxVaultAddress = address(_zrxVaultAddress); + _zrxVaultAddress = address(zrxVault); } /// @dev Initialzize storage belonging to this mixin. @@ -207,6 +207,7 @@ contract MixinParams is ); } + /// @dev Asserts that upgradable storage has not yet been initialized. function _assertStorageNotInitialized() private view diff --git a/contracts/staking/contracts/test/TestInitTarget.sol b/contracts/staking/contracts/test/TestInitTarget.sol index 576869ceb0..b64a2f076e 100644 --- a/contracts/staking/contracts/test/TestInitTarget.sol +++ b/contracts/staking/contracts/test/TestInitTarget.sol @@ -18,15 +18,12 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; -import "@0x/contracts-utils/contracts/src/Ownable.sol"; import "../src/immutable/MixinStorage.sol"; contract TestInitTarget is - Ownable, MixinStorage { - // We can't store state in this contract before it is attached, so // we will grant this predefined address a balance to indicate that // `init()` should revert. @@ -38,13 +35,33 @@ contract TestInitTarget is // `address(this)` of the last `init()` call. address private _initThisAddress = address(0); - function init() external { + event InitAddresses( + address wethProxyAddress, + address ethVaultAddress, + address rewardVaultAddress, + address zrxVaultAddress + ); + + function init( + address wethProxyAddress, + address ethVaultAddress, + address rewardVaultAddress, + address zrxVaultAddress + ) + external + { if (SHOULD_REVERT_ADDRESS.balance != 0) { revert("FORCED_REVERT"); } _initCounter += 1; _initSender = msg.sender; _initThisAddress = address(this); + emit InitAddresses( + wethProxyAddress, + ethVaultAddress, + rewardVaultAddress, + zrxVaultAddress + ); } function getInitState() diff --git a/contracts/staking/contracts/test/TestProtocolFees.sol b/contracts/staking/contracts/test/TestProtocolFees.sol index e887159dfe..37f989d9e4 100644 --- a/contracts/staking/contracts/test/TestProtocolFees.sol +++ b/contracts/staking/contracts/test/TestProtocolFees.sol @@ -38,9 +38,9 @@ contract TestProtocolFees is validExchanges[exchangeAddress] = true; _initMixinParams( wethProxyAddress, - NIL_ADDRESS, - address(0), - NIL_ADDRESS + address(1), // vault addresses must be non-zero + address(1), + address(1) ); } @@ -55,14 +55,6 @@ contract TestProtocolFees is return WETH_ASSET_DATA; } - function getActivePoolsByEpoch() - external - view - returns (bytes32[] memory) - { - return activePoolsThisEpoch; - } - /// @dev Create a test pool. function createTestPool( bytes32 poolId, diff --git a/contracts/staking/contracts/test/TestStakingProxy.sol b/contracts/staking/contracts/test/TestStakingProxy.sol index 7373deb380..9e4c2d9bca 100644 --- a/contracts/staking/contracts/test/TestStakingProxy.sol +++ b/contracts/staking/contracts/test/TestStakingProxy.sol @@ -24,7 +24,6 @@ import "../src/StakingProxy.sol"; contract TestStakingProxy is StakingProxy { - // solhint-disable no-empty-blocks constructor(address _stakingContract) public @@ -37,4 +36,18 @@ contract TestStakingProxy is NIL_ADDRESS ) {} + + function setAddressParams( + address _wethProxyAddress, + address _ethVaultAddress, + address payable _rewardVaultAddress, + address _zrxVaultAddress + ) + external + { + wethAssetProxy = IAssetProxy(_wethProxyAddress); + ethVault = IEthVault(_ethVaultAddress); + rewardVault = IStakingPoolRewardVault(_rewardVaultAddress); + zrxVault = IZrxVault(_zrxVaultAddress); + } } diff --git a/contracts/staking/test/migration.ts b/contracts/staking/test/migration.ts index 5ef89d0aa5..95a0cbe291 100644 --- a/contracts/staking/test/migration.ts +++ b/contracts/staking/test/migration.ts @@ -6,6 +6,7 @@ import { artifacts, StakingContract, TestInitTargetContract, + TestInitTargetInitAddressesEventArgs, TestStakingProxyContract, TestStakingProxyStakingContractAttachedToProxyEventArgs, } from '../src/'; @@ -89,21 +90,38 @@ blockchainTests('Migration tests', env => { it('throws if not called by owner', async () => { const attachedAddress = randomAddress(); - const tx = proxyContract.attachStakingContract.awaitTransactionSuccessAsync(attachedAddress, { - from: notOwnerAddress, - }); + const tx = proxyContract.attachStakingContract.awaitTransactionSuccessAsync( + attachedAddress, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + { + from: notOwnerAddress, + }, + ); const expectedError = new OwnableRevertErrors.OnlyOwnerError(notOwnerAddress, ownerAddress); return expect(tx).to.revertWith(expectedError); }); it('calls init() and attaches the contract', async () => { - await proxyContract.attachStakingContract.awaitTransactionSuccessAsync(initTargetContract.address); + await proxyContract.attachStakingContract.awaitTransactionSuccessAsync( + initTargetContract.address, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + ); await assertInitStateAsync(proxyContract); }); it('emits a `StakingContractAttachedToProxy` event', async () => { const receipt = await proxyContract.attachStakingContract.awaitTransactionSuccessAsync( initTargetContract.address, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, ); const logsArgs = filterLogsToArguments( receipt.logs, @@ -118,15 +136,74 @@ blockchainTests('Migration tests', env => { it('reverts if init() reverts', async () => { await enableInitRevertsAsync(); - const tx = proxyContract.attachStakingContract.awaitTransactionSuccessAsync(initTargetContract.address); + const tx = proxyContract.attachStakingContract.awaitTransactionSuccessAsync( + initTargetContract.address, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + ); return expect(tx).to.revertWith(REVERT_ERROR); }); + + it('calls init with initialized addresses if passed in args are null', async () => { + const wethProxyAddress = randomAddress(); + const ethVaultAddress = randomAddress(); + const rewardVaultAddress = randomAddress(); + const zrxVaultAddress = randomAddress(); + await proxyContract.setAddressParams.awaitTransactionSuccessAsync( + wethProxyAddress, + ethVaultAddress, + rewardVaultAddress, + zrxVaultAddress, + ); + const receipt = await proxyContract.attachStakingContract.awaitTransactionSuccessAsync( + initTargetContract.address, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + ); + const logsArgs = filterLogsToArguments( + receipt.logs, + 'InitAddresses', + ); + for (const args of logsArgs) { + expect(args.wethProxyAddress).to.eq(wethProxyAddress); + } + }); + it('calls init with passed in addresses if they are not null', async () => { + const wethProxyAddress = randomAddress(); + const ethVaultAddress = randomAddress(); + const rewardVaultAddress = randomAddress(); + const zrxVaultAddress = randomAddress(); + const receipt = await proxyContract.attachStakingContract.awaitTransactionSuccessAsync( + initTargetContract.address, + wethProxyAddress, + ethVaultAddress, + rewardVaultAddress, + zrxVaultAddress, + ); + const logsArgs = filterLogsToArguments( + receipt.logs, + 'InitAddresses', + ); + for (const args of logsArgs) { + expect(args.wethProxyAddress).to.eq(wethProxyAddress); + } + }); }); blockchainTests.resets('upgrades', async () => { it('modifies prior state', async () => { const proxyContract = await deployStakingProxyAsync(initTargetContract.address); - await proxyContract.attachStakingContract.awaitTransactionSuccessAsync(initTargetContract.address); + await proxyContract.attachStakingContract.awaitTransactionSuccessAsync( + initTargetContract.address, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + ); const initCounter = await initTargetContract.getInitCounter.callAsync({ to: proxyContract.address }); expect(initCounter).to.bignumber.eq(2); }); diff --git a/contracts/staking/test/params.ts b/contracts/staking/test/params.ts index 29e164c3f5..4e7ac700f9 100644 --- a/contracts/staking/test/params.ts +++ b/contracts/staking/test/params.ts @@ -43,7 +43,7 @@ blockchainTests('Configurable Parameters', env => { ); // Assert event. expect(receipt.logs.length).to.eq(1); - const event = filterLogsToArguments(receipt.logs, 'ParamsChanged')[0]; + const event = filterLogsToArguments(receipt.logs, 'ParamsSet')[0]; expect(event.epochDurationInSeconds).to.bignumber.eq(_params.epochDurationInSeconds); expect(event.rewardDelegatedStakeWeight).to.bignumber.eq(_params.rewardDelegatedStakeWeight); expect(event.minimumPoolStake).to.bignumber.eq(_params.minimumPoolStake); From 94738444de4b3b5a3b3308a38f9fba84d5a58f88 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 17 Sep 2019 09:41:27 -0700 Subject: [PATCH 08/11] Fix build an tests --- .../staking/contracts/src/sys/MixinParams.sol | 50 +++++++++---------- .../contracts/src/sys/MixinScheduler.sol | 31 ++++++------ .../test/TestCumulativeRewardTracking.sol | 6 ++- .../cumulative_reward_tracking_simulation.ts | 6 ++- 4 files changed, 50 insertions(+), 43 deletions(-) diff --git a/contracts/staking/contracts/src/sys/MixinParams.sol b/contracts/staking/contracts/src/sys/MixinParams.sol index 9ca23257b5..7bb89cfdfe 100644 --- a/contracts/staking/contracts/src/sys/MixinParams.sol +++ b/contracts/staking/contracts/src/sys/MixinParams.sol @@ -125,7 +125,7 @@ contract MixinParams is internal { // Ensure state is uninitialized. - _assertStorageNotInitialized(); + _assertParamsNotInitialized(); // Set up defaults. // These cannot be set to variables, or we go over the stack variable limit. @@ -143,6 +143,30 @@ contract MixinParams is ); } + /// @dev Asserts that upgradable storage has not yet been initialized. + function _assertParamsNotInitialized() + internal + view + { + if (epochDurationInSeconds != 0 && + rewardDelegatedStakeWeight != 0 && + minimumPoolStake != 0 && + maximumMakersInPool != 0 && + cobbDouglasAlphaNumerator != 0 && + cobbDouglasAlphaDenominator != 0 && + address(wethAssetProxy) != NIL_ADDRESS && + address(ethVault) != NIL_ADDRESS && + address(rewardVault) != NIL_ADDRESS && + address(zrxVault) != NIL_ADDRESS + ) { + LibRichErrors.rrevert( + LibStakingRichErrors.InitializationError( + LibStakingRichErrors.InitializationErrorCode.MixinParamsAlreadyInitialized + ) + ); + } + } + /// @dev Set all configurable parameters at once. /// @param _epochDurationInSeconds Minimum seconds between epochs. /// @param _rewardDelegatedStakeWeight How much delegated stake is weighted vs operator stake, in ppm. @@ -207,30 +231,6 @@ contract MixinParams is ); } - /// @dev Asserts that upgradable storage has not yet been initialized. - function _assertStorageNotInitialized() - private - view - { - if (epochDurationInSeconds != 0 && - rewardDelegatedStakeWeight != 0 && - minimumPoolStake != 0 && - maximumMakersInPool != 0 && - cobbDouglasAlphaNumerator != 0 && - cobbDouglasAlphaDenominator != 0 && - address(wethAssetProxy) != NIL_ADDRESS && - address(ethVault) != NIL_ADDRESS && - address(rewardVault) != NIL_ADDRESS && - address(zrxVault) != NIL_ADDRESS - ) { - LibRichErrors.rrevert( - LibStakingRichErrors.InitializationError( - LibStakingRichErrors.InitializationErrorCode.MixinParamsAlreadyInitialized - ) - ); - } - } - /// @dev Asserts that cobb douglas alpha values are valid. /// @param numerator Numerator for cobb douglas alpha factor. /// @param denominator Denominator for cobb douglas alpha factor. diff --git a/contracts/staking/contracts/src/sys/MixinScheduler.sol b/contracts/staking/contracts/src/sys/MixinScheduler.sol index 4145d43cb0..0414ee4139 100644 --- a/contracts/staking/contracts/src/sys/MixinScheduler.sol +++ b/contracts/staking/contracts/src/sys/MixinScheduler.sol @@ -49,27 +49,13 @@ contract MixinScheduler is return currentEpochStartTimeInSeconds.safeAdd(epochDurationInSeconds); } - /// @dev Assert scheduler state before initializing it. - /// This must be updated for each migration. - function _assertMixinSchedulerBeforeInit() - internal - { - if (currentEpochStartTimeInSeconds != 0) { - LibRichErrors.rrevert( - LibStakingRichErrors.InitializationError( - LibStakingRichErrors.InitializationErrorCode.MixinSchedulerAlreadyInitialized - ) - ); - } - } - /// @dev Initializes state owned by this mixin. /// Fails if state was already initialized. function _initMixinScheduler() internal { // assert the current values before overwriting them. - _assertMixinSchedulerBeforeInit(); + _assertSchedulerNotInitialized(); // solhint-disable-next-line currentEpochStartTimeInSeconds = block.timestamp; @@ -109,4 +95,19 @@ contract MixinScheduler is earliestEndTimeInSeconds ); } + + /// @dev Assert scheduler state before initializing it. + /// This must be updated for each migration. + function _assertSchedulerNotInitialized() + internal + view + { + if (currentEpochStartTimeInSeconds != 0) { + LibRichErrors.rrevert( + LibStakingRichErrors.InitializationError( + LibStakingRichErrors.InitializationErrorCode.MixinSchedulerAlreadyInitialized + ) + ); + } + } } diff --git a/contracts/staking/contracts/test/TestCumulativeRewardTracking.sol b/contracts/staking/contracts/test/TestCumulativeRewardTracking.sol index 90583db8cb..7d9bf080d7 100644 --- a/contracts/staking/contracts/test/TestCumulativeRewardTracking.sol +++ b/contracts/staking/contracts/test/TestCumulativeRewardTracking.sol @@ -77,11 +77,13 @@ contract TestCumulativeRewardTracking is ); } - function _assertMixinParamsBeforeInit() + function _assertParamsNotInitialized() internal + view {} // solhint-disable-line no-empty-blocks - function _assertMixinSchedulerBeforeInit() + function _assertSchedulerNotInitialized() internal + view {} // solhint-disable-line no-empty-blocks } diff --git a/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts b/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts index 1df1d47c10..d90f895a70 100644 --- a/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts +++ b/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts @@ -1,4 +1,4 @@ -import { BlockchainTestsEnvironment, expect, txDefaults } from '@0x/contracts-test-utils'; +import { BlockchainTestsEnvironment, constants, expect, txDefaults } from '@0x/contracts-test-utils'; import { BigNumber } from '@0x/utils'; import { DecodedLogArgs, TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import * as _ from 'lodash'; @@ -108,6 +108,10 @@ export class CumulativeRewardTrackingSimulation { await this._executeActionsAsync(initActions); await this._stakingApiWrapper.stakingProxyContract.attachStakingContract.awaitTransactionSuccessAsync( this.getTestCumulativeRewardTrackingContract().address, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, + constants.NULL_ADDRESS, ); const testLogs = await this._executeActionsAsync(testActions); CumulativeRewardTrackingSimulation._assertTestLogs(expectedTestLogs, testLogs); From bb46f184edf04d75fd64c040783b75f2468a7dc1 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 17 Sep 2019 10:46:58 -0700 Subject: [PATCH 09/11] Remove use of generatePseudoRandomAddress in favor of randomAddress --- contracts/coordinator/test/libs.ts | 4 +- contracts/coordinator/test/mixins.ts | 4 +- .../test/lib_eip712_exchange_domain.ts | 4 +- .../test/lib_exchange_rich_error_decoder.ts | 14 +++---- .../exchange/test/signature_validator.ts | 12 +++--- contracts/test-utils/src/address_utils.ts | 3 ++ contracts/utils/test/lib_address_array.ts | 40 +++++++++---------- 7 files changed, 42 insertions(+), 39 deletions(-) diff --git a/contracts/coordinator/test/libs.ts b/contracts/coordinator/test/libs.ts index 6e584246d7..66cf0100aa 100644 --- a/contracts/coordinator/test/libs.ts +++ b/contracts/coordinator/test/libs.ts @@ -1,4 +1,4 @@ -import { addressUtils, chaiSetup, constants, provider, txDefaults, web3Wrapper } from '@0x/contracts-test-utils'; +import { chaiSetup, constants, provider, randomAddress, txDefaults, web3Wrapper } from '@0x/contracts-test-utils'; import { BlockchainLifecycle } from '@0x/dev-utils'; import { transactionHashUtils } from '@0x/order-utils'; import { BigNumber, providerUtils } from '@0x/utils'; @@ -13,7 +13,7 @@ const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); describe('Libs tests', () => { let coordinatorContract: CoordinatorContract; let chainId: number; - const exchangeAddress = addressUtils.generatePseudoRandomAddress(); + const exchangeAddress = randomAddress(); before(async () => { await blockchainLifecycle.startAsync(); diff --git a/contracts/coordinator/test/mixins.ts b/contracts/coordinator/test/mixins.ts index 98b039ee88..06f1f9a65b 100644 --- a/contracts/coordinator/test/mixins.ts +++ b/contracts/coordinator/test/mixins.ts @@ -1,11 +1,11 @@ import { constants as exchangeConstants, exchangeDataEncoder, ExchangeFunctionName } from '@0x/contracts-exchange'; import { - addressUtils, chaiSetup, constants, expectContractCallFailedAsync, getLatestBlockTimestampAsync, provider, + randomAddress, TransactionFactory, txDefaults, web3Wrapper, @@ -33,7 +33,7 @@ describe('Mixins tests', () => { let approvalFactory1: ApprovalFactory; let approvalFactory2: ApprovalFactory; let defaultOrder: SignedOrder; - const exchangeAddress = addressUtils.generatePseudoRandomAddress(); + const exchangeAddress = randomAddress(); let exchangeDomain: EIP712DomainWithDefaultSchema; before(async () => { diff --git a/contracts/exchange-libs/test/lib_eip712_exchange_domain.ts b/contracts/exchange-libs/test/lib_eip712_exchange_domain.ts index 56363d9771..b2bcdc2e00 100644 --- a/contracts/exchange-libs/test/lib_eip712_exchange_domain.ts +++ b/contracts/exchange-libs/test/lib_eip712_exchange_domain.ts @@ -1,4 +1,4 @@ -import { addressUtils, blockchainTests, constants, expect } from '@0x/contracts-test-utils'; +import { blockchainTests, constants, expect, randomAddress } from '@0x/contracts-test-utils'; import { BigNumber, signTypedDataUtils } from '@0x/utils'; import * as ethUtil from 'ethereumjs-util'; @@ -28,7 +28,7 @@ blockchainTests('LibEIP712ExchangeDomain', env => { }); it('should calculate the correct domain hash when verifyingContractAddressIfExists is set to a non-null address', async () => { const chainId = 1; - const verifyingContractAddress = addressUtils.generatePseudoRandomAddress(); + const verifyingContractAddress = randomAddress(); const libEIP712ExchangeDomainContract = await TestLibEIP712ExchangeDomainContract.deployFrom0xArtifactAsync( artifacts.TestLibEIP712ExchangeDomain, env.provider, diff --git a/contracts/exchange/test/lib_exchange_rich_error_decoder.ts b/contracts/exchange/test/lib_exchange_rich_error_decoder.ts index 274c69eeb5..f0c85b5394 100644 --- a/contracts/exchange/test/lib_exchange_rich_error_decoder.ts +++ b/contracts/exchange/test/lib_exchange_rich_error_decoder.ts @@ -1,11 +1,11 @@ import { - addressUtils, blockchainTests, constants, expect, hexRandom, OrderStatus, orderUtils, + randomAddress, } from '@0x/contracts-test-utils'; import { ExchangeRevertErrors, generatePseudoRandomSalt } from '@0x/order-utils'; import { BigNumber, RevertError } from '@0x/utils'; @@ -59,8 +59,8 @@ blockchainTests.resets('LibExchangeRichErrorDecoder', ({ provider, txDefaults }) (() => { const errorCode = ExchangeRevertErrors.SignatureErrorCode.Illegal; const orderHash = orderUtils.generatePseudoRandomOrderHash(); - const signer = addressUtils.generatePseudoRandomAddress(); - const validator = addressUtils.generatePseudoRandomAddress(); + const signer = randomAddress(); + const validator = randomAddress(); const data = hexRandom(ERROR_DATA_LENGTH); const signature = hexRandom(SIGNATURE_LENGTH); const errorData = hexRandom(ERROR_DATA_LENGTH); @@ -78,7 +78,7 @@ blockchainTests.resets('LibExchangeRichErrorDecoder', ({ provider, txDefaults }) (() => { const orderHash = orderUtils.generatePseudoRandomOrderHash(); - const address = addressUtils.generatePseudoRandomAddress(); + const address = randomAddress(); createDecodeTest(ExchangeRevertErrors.ExchangeInvalidContextError, [ ExchangeRevertErrors.ExchangeContextErrorCodes.InvalidMaker, orderHash, @@ -103,14 +103,14 @@ blockchainTests.resets('LibExchangeRichErrorDecoder', ({ provider, txDefaults }) })(); (() => { - const maker = addressUtils.generatePseudoRandomAddress(); - const sender = addressUtils.generatePseudoRandomAddress(); + const maker = randomAddress(); + const sender = randomAddress(); const currentEpoch = generatePseudoRandomSalt(); createDecodeTest(ExchangeRevertErrors.OrderEpochError, [maker, sender, currentEpoch]); })(); (() => { - const assetProxyAddress = addressUtils.generatePseudoRandomAddress(); + const assetProxyAddress = randomAddress(); createDecodeTest(ExchangeRevertErrors.AssetProxyExistsError, [ hexRandom(ASSET_PROXY_ID_LENGTH), assetProxyAddress, diff --git a/contracts/exchange/test/signature_validator.ts b/contracts/exchange/test/signature_validator.ts index 0c9f41b7d9..b9f8f3b10d 100644 --- a/contracts/exchange/test/signature_validator.ts +++ b/contracts/exchange/test/signature_validator.ts @@ -1,5 +1,4 @@ import { - addressUtils, blockchainTests, constants, expect, @@ -8,6 +7,7 @@ import { LogDecoder, OrderFactory, orderUtils, + randomAddress, TransactionFactory, } from '@0x/contracts-test-utils'; import { @@ -428,11 +428,11 @@ blockchainTests.resets('MixinSignatureValidator', env => { const defaultOrderParams = { ...constants.STATIC_ORDER_PARAMS, makerAddress, - feeRecipientAddress: addressUtils.generatePseudoRandomAddress(), - makerAssetData: assetDataUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), - takerAssetData: assetDataUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), - makerFeeAssetData: assetDataUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), - takerFeeAssetData: assetDataUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), + feeRecipientAddress: randomAddress(), + makerAssetData: assetDataUtils.encodeERC20AssetData(randomAddress()), + takerAssetData: assetDataUtils.encodeERC20AssetData(randomAddress()), + makerFeeAssetData: assetDataUtils.encodeERC20AssetData(randomAddress()), + takerFeeAssetData: assetDataUtils.encodeERC20AssetData(randomAddress()), makerFee: constants.ZERO_AMOUNT, takerFee: constants.ZERO_AMOUNT, domain: { diff --git a/contracts/test-utils/src/address_utils.ts b/contracts/test-utils/src/address_utils.ts index 37858ef568..0825d860b9 100644 --- a/contracts/test-utils/src/address_utils.ts +++ b/contracts/test-utils/src/address_utils.ts @@ -1,6 +1,9 @@ import { constants } from './constants'; import { hexRandom } from './hex_utils'; +/** + * Generates a random address. + */ export function randomAddress(): string { return hexRandom(constants.ADDRESS_LENGTH); } diff --git a/contracts/utils/test/lib_address_array.ts b/contracts/utils/test/lib_address_array.ts index d69f8bf306..5f74cc3659 100644 --- a/contracts/utils/test/lib_address_array.ts +++ b/contracts/utils/test/lib_address_array.ts @@ -1,4 +1,4 @@ -import { addressUtils, chaiSetup, provider, txDefaults, web3Wrapper } from '@0x/contracts-test-utils'; +import { chaiSetup, provider, randomAddress, txDefaults, web3Wrapper } from '@0x/contracts-test-utils'; import { BlockchainLifecycle } from '@0x/dev-utils'; import { BigNumber, LibAddressArrayRevertErrors } from '@0x/utils'; import * as chai from 'chai'; @@ -29,23 +29,23 @@ describe('LibAddressArray', () => { describe('append', () => { it('should append to empty array', async () => { - const addr = addressUtils.generatePseudoRandomAddress(); + const addr = randomAddress(); const result = await lib.publicAppend.callAsync([], addr); const expected = [addr]; expect(result).to.deep.equal(expected); }); it('should append to non-empty array', async () => { - const arr = _.times(3, () => addressUtils.generatePseudoRandomAddress()); - const addr = addressUtils.generatePseudoRandomAddress(); + const arr = _.times(3, () => randomAddress()); + const addr = randomAddress(); const expected = [...arr, addr]; const result = await lib.publicAppend.callAsync(arr, addr); expect(result).to.deep.equal(expected); }); it('should revert if the free memory pointer was moved to before the end of the array', async () => { - const arr = _.times(3, () => addressUtils.generatePseudoRandomAddress()); - const addr = addressUtils.generatePseudoRandomAddress(); + const arr = _.times(3, () => randomAddress()); + const addr = randomAddress(); const freeMemOffset = new BigNumber(-1); const addressArrayEndPtr = new BigNumber(256); const expectedError = new LibAddressArrayRevertErrors.MismanagedMemoryError( @@ -56,8 +56,8 @@ describe('LibAddressArray', () => { }); it('should keep the same memory address if free memory pointer does not move', async () => { - const arr = _.times(3, () => addressUtils.generatePseudoRandomAddress()); - const addr = addressUtils.generatePseudoRandomAddress(); + const arr = _.times(3, () => randomAddress()); + const addr = randomAddress(); const freeMemOffset = new BigNumber(0); const expected = [...arr, addr]; const [result, oldArrayMemStart, newArrayMemStart] = await lib.testAppendRealloc.callAsync( @@ -70,8 +70,8 @@ describe('LibAddressArray', () => { }); it('should change memory address if free memory pointer advances', async () => { - const arr = _.times(3, () => addressUtils.generatePseudoRandomAddress()); - const addr = addressUtils.generatePseudoRandomAddress(); + const arr = _.times(3, () => randomAddress()); + const addr = randomAddress(); const freeMemOffset = new BigNumber(1); const expectedArray = [...arr, addr]; const [result, oldArrayMemStart, newArrayMemStart] = await lib.testAppendRealloc.callAsync( @@ -88,27 +88,27 @@ describe('LibAddressArray', () => { describe('contains', () => { it('should return false on an empty array', async () => { - const addr = addressUtils.generatePseudoRandomAddress(); + const addr = randomAddress(); const isFound = await lib.publicContains.callAsync([], addr); expect(isFound).to.equal(false); }); it('should return false on a missing item', async () => { - const arr = _.times(3, () => addressUtils.generatePseudoRandomAddress()); - const addr = addressUtils.generatePseudoRandomAddress(); + const arr = _.times(3, () => randomAddress()); + const addr = randomAddress(); const isFound = await lib.publicContains.callAsync(arr, addr); expect(isFound).to.equal(false); }); it('should return true on an included item', async () => { - const arr = _.times(4, () => addressUtils.generatePseudoRandomAddress()); + const arr = _.times(4, () => randomAddress()); const addr = _.sample(arr) as string; const isFound = await lib.publicContains.callAsync(arr, addr); expect(isFound).to.equal(true); }); it('should return true on the only item in the array', async () => { - const arr = _.times(1, () => addressUtils.generatePseudoRandomAddress()); + const arr = _.times(1, () => randomAddress()); const isFound = await lib.publicContains.callAsync(arr, arr[0]); expect(isFound).to.equal(true); }); @@ -116,20 +116,20 @@ describe('LibAddressArray', () => { describe('indexOf', () => { it('should fail on an empty array', async () => { - const addr = addressUtils.generatePseudoRandomAddress(); + const addr = randomAddress(); const [isSuccess] = await lib.publicIndexOf.callAsync([], addr); expect(isSuccess).to.equal(false); }); it('should fail on a missing item', async () => { - const arr = _.times(3, () => addressUtils.generatePseudoRandomAddress()); - const addr = addressUtils.generatePseudoRandomAddress(); + const arr = _.times(3, () => randomAddress()); + const addr = randomAddress(); const [isSuccess] = await lib.publicIndexOf.callAsync(arr, addr); expect(isSuccess).to.equal(false); }); it('should succeed on an included item', async () => { - const arr = _.times(4, () => addressUtils.generatePseudoRandomAddress()); + const arr = _.times(4, () => randomAddress()); const expectedIndexOf = _.random(0, arr.length - 1); const addr = arr[expectedIndexOf]; const [isSuccess, index] = await lib.publicIndexOf.callAsync(arr, addr); @@ -138,7 +138,7 @@ describe('LibAddressArray', () => { }); it('should succeed on the only item in the array', async () => { - const arr = _.times(1, () => addressUtils.generatePseudoRandomAddress()); + const arr = _.times(1, () => randomAddress()); const [isSuccess, index] = await lib.publicIndexOf.callAsync(arr, arr[0]); expect(isSuccess).to.equal(true); expect(index).bignumber.to.equal(0); From 0d441a829f2b044ab3a71f931f2c598bdddfd2e7 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 17 Sep 2019 13:01:56 -0700 Subject: [PATCH 10/11] Add missing checks to attachStakingContract tests --- contracts/staking/test/migration.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/contracts/staking/test/migration.ts b/contracts/staking/test/migration.ts index 95a0cbe291..a0c192631a 100644 --- a/contracts/staking/test/migration.ts +++ b/contracts/staking/test/migration.ts @@ -170,6 +170,9 @@ blockchainTests('Migration tests', env => { ); for (const args of logsArgs) { expect(args.wethProxyAddress).to.eq(wethProxyAddress); + expect(args.ethVaultAddress).to.eq(ethVaultAddress); + expect(args.rewardVaultAddress).to.eq(rewardVaultAddress); + expect(args.zrxVaultAddress).to.eq(zrxVaultAddress); } }); it('calls init with passed in addresses if they are not null', async () => { @@ -190,6 +193,9 @@ blockchainTests('Migration tests', env => { ); for (const args of logsArgs) { expect(args.wethProxyAddress).to.eq(wethProxyAddress); + expect(args.ethVaultAddress).to.eq(ethVaultAddress); + expect(args.rewardVaultAddress).to.eq(rewardVaultAddress); + expect(args.zrxVaultAddress).to.eq(zrxVaultAddress); } }); }); From 2d125cdc205fc1a7577128bfc657f3a037d84267 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 17 Sep 2019 13:50:18 -0700 Subject: [PATCH 11/11] Fix typos and remove redundant cached variables --- contracts/staking/contracts/src/stake/MixinZrxVault.sol | 9 +++------ contracts/staking/contracts/src/sys/MixinParams.sol | 2 +- contracts/staking/test/migration.ts | 3 +-- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/contracts/staking/contracts/src/stake/MixinZrxVault.sol b/contracts/staking/contracts/src/stake/MixinZrxVault.sol index 1248f0b153..1e8c628d79 100644 --- a/contracts/staking/contracts/src/stake/MixinZrxVault.sol +++ b/contracts/staking/contracts/src/stake/MixinZrxVault.sol @@ -33,8 +33,7 @@ contract MixinZrxVault is function _depositFromOwnerIntoZrxVault(address owner, uint256 amount) internal { - IZrxVault _zrxVault = zrxVault; - _zrxVault.depositFrom(owner, amount); + zrxVault.depositFrom(owner, amount); } /// @dev Withdraws Zrx Tokens from to `owner` from the vault. @@ -43,8 +42,7 @@ contract MixinZrxVault is function _withdrawToOwnerFromZrxVault(address owner, uint256 amount) internal { - IZrxVault _zrxVault = zrxVault; - _zrxVault.withdrawFrom(owner, amount); + zrxVault.withdrawFrom(owner, amount); } /// @dev Returns balance of `owner` in the ZRX ault. @@ -54,7 +52,6 @@ contract MixinZrxVault is view returns (uint256) { - IZrxVault _zrxVault = zrxVault; - return _zrxVault.balanceOf(owner); + return zrxVault.balanceOf(owner); } } diff --git a/contracts/staking/contracts/src/sys/MixinParams.sol b/contracts/staking/contracts/src/sys/MixinParams.sol index 7bb89cfdfe..59cdaa575a 100644 --- a/contracts/staking/contracts/src/sys/MixinParams.sol +++ b/contracts/staking/contracts/src/sys/MixinParams.sol @@ -111,7 +111,7 @@ contract MixinParams is _zrxVaultAddress = address(zrxVault); } - /// @dev Initialzize storage belonging to this mixin. + /// @dev Initialize storage belonging to this mixin. /// @param _wethProxyAddress The address that can transfer WETH for fees. /// @param _ethVaultAddress Address of the EthVault contract. /// @param _rewardVaultAddress Address of the StakingPoolRewardVault contract. diff --git a/contracts/staking/test/migration.ts b/contracts/staking/test/migration.ts index a0c192631a..e8cba2a780 100644 --- a/contracts/staking/test/migration.ts +++ b/contracts/staking/test/migration.ts @@ -89,9 +89,8 @@ blockchainTests('Migration tests', env => { }); it('throws if not called by owner', async () => { - const attachedAddress = randomAddress(); const tx = proxyContract.attachStakingContract.awaitTransactionSuccessAsync( - attachedAddress, + initTargetContract.address, constants.NULL_ADDRESS, constants.NULL_ADDRESS, constants.NULL_ADDRESS,