From 639026ea66ae2100e6e434b20bef44aa1a61dc63 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 22 Sep 2019 15:07:26 -0700 Subject: [PATCH 01/13] Delete vaults and fix contract build --- contracts/staking/contracts/src/Staking.sol | 7 - .../staking/contracts/src/StakingProxy.sol | 34 ----- .../contracts/src/fees/MixinExchangeFees.sol | 17 ++- .../src/immutable/MixinConstants.sol | 5 +- .../immutable/MixinDeploymentConstants.sol | 33 +++- .../contracts/src/immutable/MixinStorage.sol | 11 +- .../contracts/src/interfaces/IEthVault.sol | 70 --------- .../src/interfaces/IStakingEvents.sol | 4 - .../interfaces/IStakingPoolRewardVault.sol | 73 --------- .../src/interfaces/IStakingProxy.sol | 6 - .../contracts/src/interfaces/IStorage.sol | 12 -- .../contracts/src/interfaces/IStorageInit.sol | 4 - .../src/libs/LibStakingRichErrors.sol | 2 - .../contracts/src/stake/MixinStake.sol | 4 +- .../staking_pools/MixinStakingPoolRewards.sol | 42 +++--- .../contracts/src/sys/MixinFinalizer.sol | 8 +- .../staking/contracts/src/sys/MixinParams.sol | 57 ------- .../staking/contracts/src/vaults/EthVault.sol | 109 -------------- .../src/vaults/StakingPoolRewardVault.sol | 97 ------------ .../test/TestAssertStorageParams.sol | 8 +- .../test/TestCumulativeRewardTracking.sol | 2 +- .../contracts/test/TestDelegatorRewards.sol | 41 +---- .../staking/contracts/test/TestFinalizer.sol | 2 - .../staking/contracts/test/TestInitTarget.sol | 6 - .../contracts/test/TestMixinParams.sol | 54 ------- .../contracts/test/TestProtocolFees.sol | 6 +- .../staking/contracts/test/TestStaking.sol | 23 ++- .../contracts/test/TestStakingNoWETH.sol | 7 - .../contracts/test/TestStakingProxy.sol | 6 - .../contracts/test/TestStorageLayout.sol | 141 ------------------ contracts/staking/package.json | 2 +- contracts/staking/src/artifacts.ts | 12 -- contracts/staking/src/wrappers.ts | 6 - contracts/staking/test/storage_layout_test.ts | 19 --- contracts/staking/tsconfig.json | 6 - 35 files changed, 103 insertions(+), 833 deletions(-) delete mode 100644 contracts/staking/contracts/src/interfaces/IEthVault.sol delete mode 100644 contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol delete mode 100644 contracts/staking/contracts/src/vaults/EthVault.sol delete mode 100644 contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol delete mode 100644 contracts/staking/contracts/test/TestMixinParams.sol delete mode 100644 contracts/staking/contracts/test/TestStorageLayout.sol delete mode 100644 contracts/staking/test/storage_layout_test.ts diff --git a/contracts/staking/contracts/src/Staking.sol b/contracts/staking/contracts/src/Staking.sol index c0864d0961..6df9c8806e 100644 --- a/contracts/staking/contracts/src/Staking.sol +++ b/contracts/staking/contracts/src/Staking.sol @@ -32,7 +32,6 @@ contract Staking is IStakingEvents, MixinAbstract, MixinConstants, - MixinDeploymentConstants, Ownable, MixinStorage, MixinStakingPoolModifiers, @@ -60,13 +59,9 @@ contract Staking is /// This function should not be called directly. /// The StakingProxy contract will call it in `attachStakingContract()`. /// @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 ) public @@ -77,8 +72,6 @@ contract Staking is _initMixinScheduler(); _initMixinParams( _wethProxyAddress, - _ethVaultAddress, - _rewardVaultAddress, _zrxVaultAddress ); } diff --git a/contracts/staking/contracts/src/StakingProxy.sol b/contracts/staking/contracts/src/StakingProxy.sol index 173fd67d78..92a71ac510 100644 --- a/contracts/staking/contracts/src/StakingProxy.sol +++ b/contracts/staking/contracts/src/StakingProxy.sol @@ -37,15 +37,11 @@ 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. - /// @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 @@ -55,8 +51,6 @@ contract StakingProxy is _attachStakingContract( _stakingContract, _wethProxyAddress, - _ethVaultAddress, - _rewardVaultAddress, _zrxVaultAddress ); } @@ -78,17 +72,11 @@ contract StakingProxy is /// @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 @@ -97,8 +85,6 @@ contract StakingProxy is _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 ); } @@ -225,20 +211,6 @@ contract StakingProxy is )); } - if (address(ethVault) == NIL_ADDRESS) { - LibRichErrors.rrevert( - LibStakingRichErrors.InvalidParamValueError( - LibStakingRichErrors.InvalidParamValueErrorCode.InvalidEthVaultAddress - )); - } - - if (address(rewardVault) == NIL_ADDRESS) { - LibRichErrors.rrevert( - LibStakingRichErrors.InvalidParamValueError( - LibStakingRichErrors.InvalidParamValueErrorCode.InvalidRewardVaultAddress - )); - } - if (address(zrxVault) == NIL_ADDRESS) { LibRichErrors.rrevert( LibStakingRichErrors.InvalidParamValueError( @@ -250,14 +222,10 @@ contract StakingProxy is /// @dev Attach a staking contract; future calls will be delegated to the staking contract. /// @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 ) internal @@ -271,8 +239,6 @@ contract StakingProxy is abi.encodeWithSelector( IStorageInit(0).init.selector, _wethProxyAddress, - _ethVaultAddress, - _rewardVaultAddress, _zrxVaultAddress ) ); diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index 6fb78108da..333fde9011 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol @@ -25,7 +25,6 @@ import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; import "../libs/LibStakingRichErrors.sol"; import "../libs/LibCobbDouglas.sol"; -import "../immutable/MixinDeploymentConstants.sol"; import "../interfaces/IStructs.sol"; import "../stake/MixinStakeBalances.sol"; import "../sys/MixinFinalizer.sol"; @@ -37,7 +36,6 @@ contract MixinExchangeFees is IStakingEvents, MixinAbstract, MixinConstants, - MixinDeploymentConstants, Ownable, MixinStorage, MixinStakingPoolModifiers, @@ -74,7 +72,7 @@ contract MixinExchangeFees is // WETH. if (msg.value == 0) { wethAssetProxy.transferFrom( - WETH_ASSET_DATA, + _getWethAssetData(), payerAddress, address(this), protocolFeePaid @@ -127,6 +125,19 @@ contract MixinExchangeFees is activePoolsThisEpoch[poolId] = pool; } + /// @dev Returns the total balance of this contract, including WETH. + /// @return totalBalance Total balance. + function getTotalBalance() + external + view + returns (uint256 totalBalance) + { + totalBalance = address(this).balance.safeAdd( + _getWethContract().balanceOf(address(this)) + ); + return totalBalance; + } + /// @dev Get information on an active staking pool in this epoch. /// @param poolId Pool Id to query. /// @return pool ActivePool struct. diff --git a/contracts/staking/contracts/src/immutable/MixinConstants.sol b/contracts/staking/contracts/src/immutable/MixinConstants.sol index 50df13aa0f..837b2b1d68 100644 --- a/contracts/staking/contracts/src/immutable/MixinConstants.sol +++ b/contracts/staking/contracts/src/immutable/MixinConstants.sol @@ -18,8 +18,11 @@ pragma solidity ^0.5.9; +import "./MixinDeploymentConstants.sol"; -contract MixinConstants + +contract MixinConstants is + MixinDeploymentConstants { // 100% in parts-per-million. uint32 constant internal PPM_DENOMINATOR = 10**6; diff --git a/contracts/staking/contracts/src/immutable/MixinDeploymentConstants.sol b/contracts/staking/contracts/src/immutable/MixinDeploymentConstants.sol index 3a61137588..2b7a422987 100644 --- a/contracts/staking/contracts/src/immutable/MixinDeploymentConstants.sol +++ b/contracts/staking/contracts/src/immutable/MixinDeploymentConstants.sol @@ -18,6 +18,7 @@ pragma solidity ^0.5.9; +import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol"; import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetData.sol"; import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-utils/contracts/src/LibBytes.sol"; @@ -50,23 +51,41 @@ contract MixinDeploymentConstants { // bytes constant internal WETH_ASSET_DATA = hex"f47261b0000000000000000000000000c778417e063141139fce010982780140aa0cd5ab"; /// @dev Ensures that the WETH_ASSET_DATA is correct. - constructor() public { + constructor() + public + { // Ensure that the WETH_ASSET_DATA is correct. if (!WETH_ASSET_DATA.equals( - abi.encodeWithSelector(IAssetData(address(0)).ERC20Token.selector, WETH_ADDRESS) + abi.encodeWithSelector( + IAssetData(address(0)).ERC20Token.selector, + WETH_ADDRESS + ) )) { LibRichErrors.rrevert(LibStakingRichErrors.InvalidWethAssetDataError()); } } - /// @dev An overridable way to access the deployed WETH address. + /// @dev An overridable way to access the deployed WETH contract. /// Must be view to allow overrides to access state. - /// @return wethAddress The address of the configured WETH contract. - function _getWETHAddress() + /// @return wethContract The WETH contract instance. + function _getWethContract() internal view - returns (address wethAddress) + returns (IEtherToken wethContract) { - return WETH_ADDRESS; + wethContract = IEtherToken(WETH_ADDRESS); + return wethContract; + } + + /// @dev An overridable way to access the deployed WETH assetData. + /// Must be view to allow overrides to access state. + /// @return wethAssetData The assetData of the configured WETH contract. + function _getWethAssetData() + internal + view + returns (bytes memory wethAssetData) + { + wethAssetData = WETH_ASSET_DATA; + return wethAssetData; } } diff --git a/contracts/staking/contracts/src/immutable/MixinStorage.sol b/contracts/staking/contracts/src/immutable/MixinStorage.sol index 38445d7975..f7b62ec974 100644 --- a/contracts/staking/contracts/src/immutable/MixinStorage.sol +++ b/contracts/staking/contracts/src/immutable/MixinStorage.sol @@ -24,8 +24,6 @@ import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-utils/contracts/src/Authorizable.sol"; import "./MixinConstants.sol"; import "../interfaces/IZrxVault.sol"; -import "../interfaces/IEthVault.sol"; -import "../interfaces/IStakingPoolRewardVault.sol"; import "../interfaces/IStructs.sol"; import "../libs/LibStakingRichErrors.sol"; @@ -84,6 +82,9 @@ contract MixinStorage is // mapping from Pool Id to Pool mapping (bytes32 => IStructs.Pool) internal _poolById; + // mapping from PoolId to balance of members + mapping (bytes32 => uint256) public balanceByPoolId; + // current epoch uint256 public currentEpoch = INITIAL_EPOCH; @@ -105,12 +106,6 @@ contract MixinStorage is // ZRX vault (stores staked ZRX) IZrxVault public zrxVault; - // ETH Vault (stores eth balances of stakers and pool operators) - IEthVault public ethVault; - - // Rebate Vault (stores rewards for pools before they are moved to the eth vault on a per-user basis) - IStakingPoolRewardVault public rewardVault; - /* Tweakable parameters */ // Minimum seconds between epochs. diff --git a/contracts/staking/contracts/src/interfaces/IEthVault.sol b/contracts/staking/contracts/src/interfaces/IEthVault.sol deleted file mode 100644 index cf5e790402..0000000000 --- a/contracts/staking/contracts/src/interfaces/IEthVault.sol +++ /dev/null @@ -1,70 +0,0 @@ -/* - - Copyright 2019 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.5.9; - - -/// @dev This vault manages Ether. -interface IEthVault { - - /// @dev Emitted when Ether are deposited into the vault. - /// @param sender Address of sender (`msg.sender`). - /// @param owner of Ether. - /// @param amount of Ether deposited. - event EthDepositedIntoVault( - address indexed sender, - address indexed owner, - uint256 amount - ); - - /// @dev Emitted when Ether are withdrawn from the vault. - /// @param sender Address of sender (`msg.sender`). - /// @param owner of Ether. - /// @param amount of Ether withdrawn. - event EthWithdrawnFromVault( - address indexed sender, - address indexed owner, - uint256 amount - ); - - /// @dev Deposit an `amount` of WETH for `owner` into the vault. - /// The staking contract should have granted the vault an allowance - /// because it will pull the WETH via `transferFrom()`. - /// Note that this is only callable by the staking contract. - /// @param owner Owner of the WETH. - /// @param amount Amount of deposit. - function depositFor(address owner, uint256 amount) - external; - - /// @dev Withdraw an `amount` of WETH to `msg.sender` from the vault. - /// @param amount of WETH to withdraw. - function withdraw(uint256 amount) - external; - - /// @dev Withdraw ALL WETH to `msg.sender` from the vault. - function withdrawAll() - external - returns (uint256); - - /// @dev Returns the balance in WETH of the `owner` - /// @return Balance in WETH. - function balanceOf(address owner) - external - view - returns (uint256); -} diff --git a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol index 2084d222d5..2e64439bfd 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol @@ -96,8 +96,6 @@ interface IStakingEvents { /// @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. event ParamsSet( uint256 epochDurationInSeconds, @@ -107,8 +105,6 @@ interface IStakingEvents { uint256 cobbDouglasAlphaNumerator, uint256 cobbDouglasAlphaDenominator, address wethProxyAddress, - address ethVaultAddress, - address rewardVaultAddress, address zrxVaultAddress ); diff --git a/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol b/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol deleted file mode 100644 index 15b71f2b49..0000000000 --- a/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol +++ /dev/null @@ -1,73 +0,0 @@ -/* - - Copyright 2019 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.5.9; -pragma experimental ABIEncoderV2; - - -/// @dev This vault manages staking pool rewards. -interface IStakingPoolRewardVault { - - /// @dev Emitted when WETH is deposited into the vault. - /// @param sender Address of sender (`msg.sender`). - /// @param poolId that owns of WETH. - /// @param amount of WETH deposited. - event EthDepositedIntoVault( - address indexed sender, - bytes32 indexed poolId, - uint256 amount - ); - - /// @dev Emitted when rewards are transferred out of the vault. - /// @param poolId Unique Id of pool. - /// @param to Address to send funds to. - /// @param amount Amount of WETH to transfer. - event PoolRewardTransferred( - bytes32 indexed poolId, - address indexed to, - uint256 amount - ); - - /// @dev Deposit an amount of WETH for `poolId` into the vault. - /// The staking contract should have granted the vault an allowance - /// because it will pull the WETH via `transferFrom()`. - /// Note that this is only callable by the staking contract. - /// @param poolId Pool that holds the WETH. - /// @param amount Amount of deposit. - function depositFor(bytes32 poolId, uint256 amount) - external; - - /// @dev Withdraw some amount in WETH from a pool. - /// Note that this is only callable by the staking contract. - /// @param poolId Unique Id of pool. - /// @param to Address to send funds to. - /// @param amount Amount of WETH to transfer. - function transfer( - bytes32 poolId, - address payable to, - uint256 amount - ) - external; - - /// @dev Returns the balance in WETH of `poolId` - /// @return Balance in WETH. - function balanceOf(bytes32 poolId) - external - view - returns (uint256); -} diff --git a/contracts/staking/contracts/src/interfaces/IStakingProxy.sol b/contracts/staking/contracts/src/interfaces/IStakingProxy.sol index b5b43e20ce..c8838df778 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingProxy.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingProxy.sol @@ -47,17 +47,11 @@ interface IStakingProxy /* is IStaking */ /// @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; diff --git a/contracts/staking/contracts/src/interfaces/IStorage.sol b/contracts/staking/contracts/src/interfaces/IStorage.sol index cdb6b3552c..2c3a513c47 100644 --- a/contracts/staking/contracts/src/interfaces/IStorage.sol +++ b/contracts/staking/contracts/src/interfaces/IStorage.sol @@ -21,8 +21,6 @@ pragma experimental ABIEncoderV2; import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetProxy.sol"; import "../interfaces/IZrxVault.sol"; -import "../interfaces/IEthVault.sol"; -import "../interfaces/IStakingPoolRewardVault.sol"; import "../interfaces/IStructs.sol"; @@ -93,16 +91,6 @@ interface IStorage { view returns (IZrxVault); - function ethVault() - external - view - returns (IEthVault); - - function rewardVault() - external - view - returns (IStakingPoolRewardVault); - function epochDurationInSeconds() external view diff --git a/contracts/staking/contracts/src/interfaces/IStorageInit.sol b/contracts/staking/contracts/src/interfaces/IStorageInit.sol index 35e49c1a5d..1e172c25bc 100644 --- a/contracts/staking/contracts/src/interfaces/IStorageInit.sol +++ b/contracts/staking/contracts/src/interfaces/IStorageInit.sol @@ -23,13 +23,9 @@ interface IStorageInit { /// @dev Initialize storage owned by this 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( 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 7d3fe19e34..7864f88789 100644 --- a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol +++ b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol @@ -44,8 +44,6 @@ library LibStakingRichErrors { InvalidMaximumMakersInPool, InvalidMinimumPoolStake, InvalidWethProxyAddress, - InvalidEthVaultAddress, - InvalidRewardVaultAddress, InvalidZrxVaultAddress, InvalidEpochDuration } diff --git a/contracts/staking/contracts/src/stake/MixinStake.sol b/contracts/staking/contracts/src/stake/MixinStake.sol index 412f224f7a..2774b3c2ee 100644 --- a/contracts/staking/contracts/src/stake/MixinStake.sol +++ b/contracts/staking/contracts/src/stake/MixinStake.sol @@ -217,7 +217,7 @@ contract MixinStake is // to. IStructs.StoredBalance memory finalDelegatedStakeToPoolByOwner = _loadAndSyncBalance(_delegatedStakeToPoolByOwner[owner][poolId]); - _syncRewardsForDelegator( + _withdrawAndSyncDelegatorRewards( poolId, owner, initDelegatedStakeToPoolByOwner, @@ -256,7 +256,7 @@ contract MixinStake is // from IStructs.StoredBalance memory finalDelegatedStakeToPoolByOwner = _loadAndSyncBalance(_delegatedStakeToPoolByOwner[owner][poolId]); - _syncRewardsForDelegator( + _withdrawAndSyncDelegatorRewards( poolId, owner, initDelegatedStakeToPoolByOwner, diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index f52f89d325..9cbeb837b6 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -39,21 +39,22 @@ contract MixinStakingPoolRewards is { using LibSafeMath for uint256; - /// @dev Syncs rewards for a delegator. This includes transferring rewards - /// from the Reward Vault to the Eth Vault, and adding/removing + /// @dev Syncs rewards for a delegator. This includes transferring WETH + /// rewards to the delegator, and adding/removing /// dependencies on cumulative rewards. /// This is used by a delegator when they want to sync their rewards /// without delegating/undelegating. It's effectively the same as /// delegating zero stake. /// @param poolId Unique id of pool. - function syncDelegatorRewards(bytes32 poolId) + function withdrawDelegatorRewards(bytes32 poolId) external { address member = msg.sender; IStructs.StoredBalance memory finalDelegatedStakeToPoolByOwner = _loadAndSyncBalance(_delegatedStakeToPoolByOwner[member][poolId]); - _syncRewardsForDelegator( + + _withdrawAndSyncDelegatorRewards( poolId, member, // Initial balance @@ -81,7 +82,9 @@ contract MixinStakingPoolRewards is // rewards. IStructs.Pool memory pool = _poolById[poolId]; // Get any unfinalized rewards. - (uint256 unfinalizedTotalRewards, uint256 unfinalizedMembersStake) = _getUnfinalizedPoolRewards(poolId); + (uint256 unfinalizedTotalRewards, uint256 unfinalizedMembersStake) = + _getUnfinalizedPoolRewards(poolId); + // Get the operators' portion. (reward,) = _computeSplitStakingPoolRewards( pool.operatorShare, @@ -105,6 +108,7 @@ contract MixinStakingPoolRewards is // Get any unfinalized rewards. (uint256 unfinalizedTotalRewards, uint256 unfinalizedMembersStake) = _getUnfinalizedPoolRewards(poolId); + // Get the members' portion. (, uint256 unfinalizedMembersReward) = _computeSplitStakingPoolRewards( pool.operatorShare, @@ -129,7 +133,7 @@ contract MixinStakingPoolRewards is /// balance at the beginning of this transaction. /// @param finalDelegatedStakeToPoolByOwner The member's delegated balance /// at the end of this transaction. - function _syncRewardsForDelegator( + function _withdrawAndSyncDelegatorRewards( bytes32 poolId, address member, IStructs.StoredBalance memory initialDelegatedStakeToPoolByOwner, @@ -140,7 +144,7 @@ contract MixinStakingPoolRewards is // Transfer any rewards from the transient pool vault to the eth vault; // this must be done before we can modify the owner's portion of the // delegator pool. - _transferDelegatorRewardsToEthVault( + _finalizePoolAndWithdrawDelegatorRewards( poolId, member, initialDelegatedStakeToPoolByOwner, @@ -191,14 +195,14 @@ contract MixinStakingPoolRewards is reward, membersStake ); - // Deposit the operator's reward in the eth vault. - ethVault.depositFor(pool.operator, operatorReward); + // Transfer the operator's weth reward to the operator + _getWethContract().transfer(pool.operator, operatorReward); if (membersReward == 0) { return (0, 0); } - // Deposit the members' reward in the reward vault. - rewardVault.depositFor(poolId, membersReward); + // Increment the balance of the pool + balanceByPoolId[poolId] = balanceByPoolId[poolId].safeAdd(membersReward); // Fetch the last epoch at which we stored an entry for this pool; // this is the most up-to-date cumulative rewards for this pool. @@ -259,13 +263,12 @@ contract MixinStakingPoolRewards is return (operatorReward, membersReward); } - /// @dev Transfers a delegators accumulated rewards from the transient pool - /// Reward Pool vault to the Eth Vault. This is required before the - /// member's stake in the pool can be modified. + /// @dev Transfers a delegators accumulated rewards to the delegator. + /// This is required before the member's stake in the pool can be modified. /// @param poolId Unique id of pool. /// @param member The member of the pool. /// @param unsyncedStake Unsynced stake of the delegator to the pool. - function _transferDelegatorRewardsToEthVault( + function _finalizePoolAndWithdrawDelegatorRewards( bytes32 poolId, address member, IStructs.StoredBalance memory unsyncedStake, @@ -290,10 +293,11 @@ contract MixinStakingPoolRewards is return; } - // Transfer from RewardVault to this contract. - rewardVault.transfer(poolId, address(uint160(address(this))), balance); - // Transfer to EthVault. - ethVault.depositFor(member, balance); + // Decrement the balance of the pool + balanceByPoolId[poolId] = balanceByPoolId[poolId].safeSub(balance); + + // Withdraw the member's WETH balance + _getWethContract().transfer(member, balance); } /// @dev Computes the reward balance in ETH of a specific member of a pool. diff --git a/contracts/staking/contracts/src/sys/MixinFinalizer.sol b/contracts/staking/contracts/src/sys/MixinFinalizer.sol index 624e724d4f..7a8e860515 100644 --- a/contracts/staking/contracts/src/sys/MixinFinalizer.sol +++ b/contracts/staking/contracts/src/sys/MixinFinalizer.sol @@ -26,7 +26,6 @@ import "../libs/LibCobbDouglas.sol"; import "../libs/LibStakingRichErrors.sol"; import "../immutable/MixinStorage.sol"; import "../immutable/MixinConstants.sol"; -import "../immutable/MixinDeploymentConstants.sol"; import "../interfaces/IStakingEvents.sol"; import "../interfaces/IStructs.sol"; import "../stake/MixinStakeBalances.sol"; @@ -43,7 +42,6 @@ contract MixinFinalizer is IStakingEvents, MixinAbstract, MixinConstants, - MixinDeploymentConstants, Ownable, MixinStorage, MixinScheduler, @@ -245,12 +243,12 @@ contract MixinFinalizer is internal returns (uint256 balance) { - IEtherToken weth = IEtherToken(_getWETHAddress()); + IEtherToken wethContract = _getWethContract(); uint256 ethBalance = address(this).balance; if (ethBalance != 0) { - weth.deposit.value((address(this).balance))(); + wethContract.deposit.value(ethBalance)(); } - balance = weth.balanceOf(address(this)); + balance = wethContract.balanceOf(address(this)); return balance; } diff --git a/contracts/staking/contracts/src/sys/MixinParams.sol b/contracts/staking/contracts/src/sys/MixinParams.sol index 9c0856b4ab..8531538f9f 100644 --- a/contracts/staking/contracts/src/sys/MixinParams.sol +++ b/contracts/staking/contracts/src/sys/MixinParams.sol @@ -22,10 +22,7 @@ import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol"; import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetProxy.sol"; import "../immutable/MixinStorage.sol"; -import "../immutable/MixinDeploymentConstants.sol"; import "../interfaces/IStakingEvents.sol"; -import "../interfaces/IEthVault.sol"; -import "../interfaces/IStakingPoolRewardVault.sol"; import "../interfaces/IZrxVault.sol"; import "../libs/LibStakingRichErrors.sol"; @@ -33,7 +30,6 @@ import "../libs/LibStakingRichErrors.sol"; contract MixinParams is IStakingEvents, MixinConstants, - MixinDeploymentConstants, Ownable, MixinStorage { @@ -45,8 +41,6 @@ contract MixinParams is /// @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, @@ -56,8 +50,6 @@ contract MixinParams is uint32 _cobbDouglasAlphaNumerator, uint32 _cobbDouglasAlphaDenominator, address _wethProxyAddress, - address _ethVaultAddress, - address payable _rewardVaultAddress, address _zrxVaultAddress ) external @@ -71,8 +63,6 @@ contract MixinParams is _cobbDouglasAlphaNumerator, _cobbDouglasAlphaDenominator, _wethProxyAddress, - _ethVaultAddress, - _rewardVaultAddress, _zrxVaultAddress ); } @@ -85,8 +75,6 @@ contract MixinParams is /// @return _cobbDouglasAlphaNumerator Numerator 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 @@ -99,8 +87,6 @@ contract MixinParams is uint32 _cobbDouglasAlphaNumerator, uint32 _cobbDouglasAlphaDenominator, address _wethProxyAddress, - address _ethVaultAddress, - address _rewardVaultAddress, address _zrxVaultAddress ) { @@ -111,20 +97,14 @@ contract MixinParams is _cobbDouglasAlphaNumerator = cobbDouglasAlphaNumerator; _cobbDouglasAlphaDenominator = cobbDouglasAlphaDenominator; _wethProxyAddress = address(wethAssetProxy); - _ethVaultAddress = address(ethVault); - _rewardVaultAddress = address(rewardVault); _zrxVaultAddress = address(zrxVault); } /// @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. /// @param _zrxVaultAddress Address of the ZrxVault contract. function _initMixinParams( address _wethProxyAddress, - address _ethVaultAddress, - address payable _rewardVaultAddress, address _zrxVaultAddress ) internal @@ -142,8 +122,6 @@ contract MixinParams is 1, // cobbDouglasAlphaNumerator 2, // cobbDouglasAlphaDenominator _wethProxyAddress, - _ethVaultAddress, - _rewardVaultAddress, _zrxVaultAddress ); } @@ -160,8 +138,6 @@ contract MixinParams is cobbDouglasAlphaNumerator != 0 && cobbDouglasAlphaDenominator != 0 && address(wethAssetProxy) != NIL_ADDRESS && - address(ethVault) != NIL_ADDRESS && - address(rewardVault) != NIL_ADDRESS && address(zrxVault) != NIL_ADDRESS ) { LibRichErrors.rrevert( @@ -172,26 +148,6 @@ contract MixinParams is } } - /// @dev Rescind the WETH allowance for `oldSpenders` and grant `newSpenders` - /// an unlimited allowance. - /// @param oldSpenders Addresses to remove allowance from. - /// @param newSpenders Addresses to grant allowance to. - function _transferWETHAllownces( - address[2] memory oldSpenders, - address[2] memory newSpenders - ) - internal - { - IEtherToken weth = IEtherToken(_getWETHAddress()); - // Grant new allowances. - for (uint256 i = 0; i < oldSpenders.length; i++) { - // Rescind old allowance. - weth.approve(oldSpenders[i], 0); - // Grant new allowance. - weth.approve(newSpenders[i], uint256(-1)); - } - } - /// @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. @@ -200,8 +156,6 @@ contract MixinParams is /// @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, @@ -211,17 +165,10 @@ contract MixinParams is uint32 _cobbDouglasAlphaNumerator, uint32 _cobbDouglasAlphaDenominator, address _wethProxyAddress, - address _ethVaultAddress, - address payable _rewardVaultAddress, address _zrxVaultAddress ) private { - _transferWETHAllownces( - [address(ethVault), address(rewardVault)], - [_ethVaultAddress, _rewardVaultAddress] - ); - epochDurationInSeconds = _epochDurationInSeconds; rewardDelegatedStakeWeight = _rewardDelegatedStakeWeight; minimumPoolStake = _minimumPoolStake; @@ -229,8 +176,6 @@ contract MixinParams is cobbDouglasAlphaNumerator = _cobbDouglasAlphaNumerator; cobbDouglasAlphaDenominator = _cobbDouglasAlphaDenominator; wethAssetProxy = IAssetProxy(_wethProxyAddress); - ethVault = IEthVault(_ethVaultAddress); - rewardVault = IStakingPoolRewardVault(_rewardVaultAddress); zrxVault = IZrxVault(_zrxVaultAddress); emit ParamsSet( @@ -241,8 +186,6 @@ contract MixinParams is _cobbDouglasAlphaNumerator, _cobbDouglasAlphaDenominator, _wethProxyAddress, - _ethVaultAddress, - _rewardVaultAddress, _zrxVaultAddress ); } diff --git a/contracts/staking/contracts/src/vaults/EthVault.sol b/contracts/staking/contracts/src/vaults/EthVault.sol deleted file mode 100644 index d53f8b9614..0000000000 --- a/contracts/staking/contracts/src/vaults/EthVault.sol +++ /dev/null @@ -1,109 +0,0 @@ -/* - - Copyright 2019 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.5.9; - -import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol"; -import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; -import "../interfaces/IEthVault.sol"; -import "./MixinVaultCore.sol"; - - -/// @dev This vault manages WETH. -contract EthVault is - IEthVault, - IVaultCore, - MixinVaultCore -{ - using LibSafeMath for uint256; - - // Address of the WETH contract. - IEtherToken public weth; - // mapping from Owner to WETH balance - mapping (address => uint256) internal _balances; - - /// @param wethAddress Address of the WETH contract. - constructor(address wethAddress) public { - weth = IEtherToken(wethAddress); - } - - /// @dev Deposit an `amount` of WETH for `owner` into the vault. - /// The staking contract should have granted the vault an allowance - /// because it will pull the WETH via `transferFrom()`. - /// Note that this is only callable by the staking contract. - /// @param owner Owner of the WETH. - /// @param amount Amount of deposit. - function depositFor(address owner, uint256 amount) - external - onlyStakingProxy - { - // Transfer WETH from the staking contract into this contract. - weth.transferFrom(msg.sender, address(this), amount); - // Credit the owner. - _balances[owner] = _balances[owner].safeAdd(amount); - emit EthDepositedIntoVault(msg.sender, owner, amount); - } - - /// @dev Withdraw an `amount` of WETH to `msg.sender` from the vault. - /// @param amount of WETH to withdraw. - function withdraw(uint256 amount) - external - { - _withdrawFrom(msg.sender, amount); - } - - /// @dev Withdraw ALL WETH to `msg.sender` from the vault. - function withdrawAll() - external - returns (uint256 totalBalance) - { - // get total balance - address payable owner = msg.sender; - totalBalance = _balances[owner]; - - // withdraw WETH to owner - _withdrawFrom(owner, totalBalance); - return totalBalance; - } - - /// @dev Returns the balance in WETH of the `owner` - /// @return Balance in WETH. - function balanceOf(address owner) - external - view - returns (uint256) - { - return _balances[owner]; - } - - /// @dev Withdraw an `amount` of WETH to `owner` from the vault. - /// @param owner of WETH. - /// @param amount of WETH to withdraw. - function _withdrawFrom(address payable owner, uint256 amount) - internal - { - //Uupdate balance. - _balances[owner] = _balances[owner].safeSub(amount); - - // withdraw WETH to owner - weth.transfer(msg.sender, amount); - - // notify - emit EthWithdrawnFromVault(msg.sender, owner, amount); - } -} diff --git a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol deleted file mode 100644 index bc31598322..0000000000 --- a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol +++ /dev/null @@ -1,97 +0,0 @@ -/* - - Copyright 2019 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.5.9; -pragma experimental ABIEncoderV2; - -import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol"; -import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; -import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; -import "../libs/LibStakingRichErrors.sol"; -import "../libs/LibSafeDowncast.sol"; -import "../interfaces/IStakingPoolRewardVault.sol"; -import "./MixinVaultCore.sol"; - - -/// @dev This vault manages staking pool rewards. -contract StakingPoolRewardVault is - IStakingPoolRewardVault, - IVaultCore, - MixinVaultCore -{ - using LibSafeMath for uint256; - - // Address of the WETH contract. - IEtherToken public weth; - // mapping from poolId to Pool metadata - mapping (bytes32 => uint256) internal _balanceByPoolId; - - /// @param wethAddress Address of the WETH contract. - constructor(address wethAddress) public { - weth = IEtherToken(wethAddress); - } - - /// @dev Deposit an amount of WETH for `poolId` into the vault. - /// The staking contract should have granted the vault an allowance - /// because it will pull the WETH via `transferFrom()`. - /// Note that this is only callable by the staking contract. - /// @param poolId Pool that holds the WETH. - /// @param amount Amount of deposit. - function depositFor(bytes32 poolId, uint256 amount) - external - onlyStakingProxy - { - // Transfer WETH from the staking contract into this contract. - weth.transferFrom(msg.sender, address(this), amount); - // Credit the pool. - _balanceByPoolId[poolId] = _balanceByPoolId[poolId].safeAdd(amount); - emit EthDepositedIntoVault(msg.sender, poolId, amount); - } - - /// @dev Withdraw some amount in WETH from a pool. - /// Note that this is only callable by the staking contract. - /// @param poolId Unique Id of pool. - /// @param to Address to send funds to. - /// @param amount Amount of WETH to transfer. - function transfer( - bytes32 poolId, - address payable to, - uint256 amount - ) - external - onlyStakingProxy - { - _balanceByPoolId[poolId] = _balanceByPoolId[poolId].safeSub(amount); - weth.transfer(to, amount); - emit PoolRewardTransferred( - poolId, - to, - amount - ); - } - - /// @dev Returns the balance in WETH of `poolId` - /// @return Balance in WETH. - function balanceOf(bytes32 poolId) - external - view - returns (uint256 balance) - { - return _balanceByPoolId[poolId]; - } -} diff --git a/contracts/staking/contracts/test/TestAssertStorageParams.sol b/contracts/staking/contracts/test/TestAssertStorageParams.sol index 07a3213723..3c34585bab 100644 --- a/contracts/staking/contracts/test/TestAssertStorageParams.sol +++ b/contracts/staking/contracts/test/TestAssertStorageParams.sol @@ -33,16 +33,12 @@ contract TestAssertStorageParams is uint32 cobbDouglasAlphaNumerator; uint32 cobbDouglasAlphaDenominator; address wethProxyAddress; - address ethVaultAddress; - address rewardVaultAddress; address zrxVaultAddress; } constructor() public StakingProxy( - NIL_ADDRESS, - NIL_ADDRESS, NIL_ADDRESS, NIL_ADDRESS, NIL_ADDRESS, @@ -60,13 +56,11 @@ contract TestAssertStorageParams is cobbDouglasAlphaNumerator = params.cobbDouglasAlphaNumerator; cobbDouglasAlphaDenominator = params.cobbDouglasAlphaDenominator; wethAssetProxy = IAssetProxy(params.wethProxyAddress); - ethVault = IEthVault(params.ethVaultAddress); - rewardVault = IStakingPoolRewardVault(params.rewardVaultAddress); zrxVault = IZrxVault(params.zrxVaultAddress); _assertValidStorageParams(); } - function _attachStakingContract(address, address, address, address, address) + function _attachStakingContract(address, address, address) internal {} } diff --git a/contracts/staking/contracts/test/TestCumulativeRewardTracking.sol b/contracts/staking/contracts/test/TestCumulativeRewardTracking.sol index a185138ca9..4a1f3a1c15 100644 --- a/contracts/staking/contracts/test/TestCumulativeRewardTracking.sol +++ b/contracts/staking/contracts/test/TestCumulativeRewardTracking.sol @@ -42,7 +42,7 @@ contract TestCumulativeRewardTracking is constructor(address wethAddress) public TestStaking(wethAddress) {} - function init(address, address, address payable, address) public {} + function init(address, address) public {} function _forceSetCumulativeReward( bytes32 poolId, diff --git a/contracts/staking/contracts/test/TestDelegatorRewards.sol b/contracts/staking/contracts/test/TestDelegatorRewards.sol index 628cdc555e..3a0a83bc36 100644 --- a/contracts/staking/contracts/test/TestDelegatorRewards.sol +++ b/contracts/staking/contracts/test/TestDelegatorRewards.sol @@ -20,8 +20,6 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; import "../src/interfaces/IStructs.sol"; -import "../src/interfaces/IStakingPoolRewardVault.sol"; -import "../src/interfaces/IEthVault.sol"; import "./TestStakingNoWETH.sol"; @@ -53,15 +51,9 @@ contract TestDelegatorRewards is constructor() public { init( - address(1), - address(1), address(1), address(1) ); - // Set this contract up as the eth and reward vault to intercept - // deposits. - ethVault = IEthVault(address(this)); - rewardVault = IStakingPoolRewardVault(address(this)); } mapping (uint256 => mapping (bytes32 => UnfinalizedPoolReward)) private @@ -137,7 +129,7 @@ contract TestDelegatorRewards is _stake.currentEpochBalance += uint96(stake); _stake.nextEpochBalance += uint96(stake); _stake.currentEpoch = uint32(currentEpoch); - _syncRewardsForDelegator( + _withdrawAndSyncDelegatorRewards( poolId, delegator, initialStake, @@ -164,7 +156,7 @@ contract TestDelegatorRewards is _stake.isInitialized = true; _stake.nextEpochBalance += uint96(stake); _stake.currentEpoch = uint32(currentEpoch); - _syncRewardsForDelegator( + _withdrawAndSyncDelegatorRewards( poolId, delegator, initialStake, @@ -191,7 +183,7 @@ contract TestDelegatorRewards is _stake.isInitialized = true; _stake.nextEpochBalance -= uint96(stake); _stake.currentEpoch = uint32(currentEpoch); - _syncRewardsForDelegator( + _withdrawAndSyncDelegatorRewards( poolId, delegator, initialStake, @@ -199,33 +191,6 @@ contract TestDelegatorRewards is ); } - /// @dev `IEthVault.depositFor()`,` overridden to just emit events. - function depositFor( - address owner, - uint256 amount - ) - external - { - emit RecordDepositToEthVault( - owner, - amount - ); - } - - /// @dev `IStakingPoolRewardVault.depositFor()`,` - /// overridden to just emit events. - function depositFor( - bytes32 poolId, - uint256 membersReward - ) - external - { - emit RecordDepositToRewardVault( - poolId, - membersReward - ); - } - // solhint-disable no-simple-event-func-name /// @dev Overridden to realize `unfinalizedPoolRewardsByEpoch` in /// the current epoch and emit a event, diff --git a/contracts/staking/contracts/test/TestFinalizer.sol b/contracts/staking/contracts/test/TestFinalizer.sol index 4ff64b5d33..0d1214c09d 100644 --- a/contracts/staking/contracts/test/TestFinalizer.sol +++ b/contracts/staking/contracts/test/TestFinalizer.sol @@ -57,8 +57,6 @@ contract TestFinalizer is public { init( - address(1), - address(1), address(1), address(1) ); diff --git a/contracts/staking/contracts/test/TestInitTarget.sol b/contracts/staking/contracts/test/TestInitTarget.sol index e3ee74ed25..b06595e361 100644 --- a/contracts/staking/contracts/test/TestInitTarget.sol +++ b/contracts/staking/contracts/test/TestInitTarget.sol @@ -37,15 +37,11 @@ contract TestInitTarget is event InitAddresses( address wethProxyAddress, - address ethVaultAddress, - address rewardVaultAddress, address zrxVaultAddress ); function init( address wethProxyAddress, - address ethVaultAddress, - address rewardVaultAddress, address zrxVaultAddress ) external @@ -58,8 +54,6 @@ contract TestInitTarget is _initThisAddress = address(this); emit InitAddresses( wethProxyAddress, - ethVaultAddress, - rewardVaultAddress, zrxVaultAddress ); } diff --git a/contracts/staking/contracts/test/TestMixinParams.sol b/contracts/staking/contracts/test/TestMixinParams.sol deleted file mode 100644 index cf21e8ce63..0000000000 --- a/contracts/staking/contracts/test/TestMixinParams.sol +++ /dev/null @@ -1,54 +0,0 @@ -/* - - Copyright 2019 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.5.9; -pragma experimental ABIEncoderV2; - -import "../src/interfaces/IEthVault.sol"; -import "../src/interfaces/IStakingPoolRewardVault.sol"; -import "../src/sys/MixinParams.sol"; - - -// solhint-disable no-empty-blocks -contract TestMixinParams is - MixinParams -{ - - event WETHApprove(address spender, uint256 amount); - - /// @dev Sets the eth and reward vault addresses. - function setVaultAddresses( - address ethVaultAddress, - address rewardVaultAddress - ) - external - { - ethVault = IEthVault(ethVaultAddress); - rewardVault = IStakingPoolRewardVault(rewardVaultAddress); - } - - /// @dev WETH `approve()` function that just logs events. - function approve(address spender, uint256 amount) external returns (bool) { - emit WETHApprove(spender, amount); - } - - /// @dev Overridden return this contract's address. - function _getWETHAddress() internal view returns (address) { - return address(this); - } -} diff --git a/contracts/staking/contracts/test/TestProtocolFees.sol b/contracts/staking/contracts/test/TestProtocolFees.sol index 4fe8756290..c89b9724fd 100644 --- a/contracts/staking/contracts/test/TestProtocolFees.sol +++ b/contracts/staking/contracts/test/TestProtocolFees.sol @@ -48,8 +48,6 @@ contract TestProtocolFees is // Use this contract as the ERC20Proxy. address(this), // vault addresses must be non-zero - address(1), - address(1), address(1) ); validExchanges[exchangeAddress] = true; @@ -98,8 +96,8 @@ contract TestProtocolFees is emit ERC20ProxyTransferFrom(assetData, from, to, amount); } - function getWethAssetData() external pure returns (bytes memory) { - return WETH_ASSET_DATA; + function getWethAssetData() external view returns (bytes memory) { + return _getWethAssetData(); } /// @dev Overridden to use test pools. diff --git a/contracts/staking/contracts/test/TestStaking.sol b/contracts/staking/contracts/test/TestStaking.sol index e4a48ce2a5..4d8b06e812 100644 --- a/contracts/staking/contracts/test/TestStaking.sol +++ b/contracts/staking/contracts/test/TestStaking.sol @@ -20,7 +20,7 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; import "../src/Staking.sol"; - +import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol"; contract TestStaking is Staking @@ -32,10 +32,27 @@ contract TestStaking is } /// @dev Overridden to use testWethAddress; - function _getWETHAddress() internal view returns (address) { + function _getWethContract() + internal + view + returns (IEtherToken) + { // `testWethAddress` will not be set on the proxy this contract is // attached to, so we need to access the storage of the deployed // instance of this contract. - return TestStaking(address(uint160(stakingContract))).testWethAddress(); + address wethAddress = TestStaking(address(uint160(stakingContract))).testWethAddress(); + return IEtherToken(wethAddress); + } + + function _getWethAssetData() + internal + view + returns (bytes memory) + { + address wethAddress = TestStaking(address(uint160(stakingContract))).testWethAddress(); + return abi.encodeWithSelector( + IAssetData(address(0)).ERC20Token.selector, + wethAddress + ); } } diff --git a/contracts/staking/contracts/test/TestStakingNoWETH.sol b/contracts/staking/contracts/test/TestStakingNoWETH.sol index 94b7f380ac..afee8892a4 100644 --- a/contracts/staking/contracts/test/TestStakingNoWETH.sol +++ b/contracts/staking/contracts/test/TestStakingNoWETH.sol @@ -28,13 +28,6 @@ import "../src/Staking.sol"; contract TestStakingNoWETH is Staking { - function _transferWETHAllownces( - address[2] memory oldSpenders, - address[2] memory newSpenders - ) - internal - {} - function _wrapBalanceToWETHAndGetBalance() internal returns (uint256 balance) diff --git a/contracts/staking/contracts/test/TestStakingProxy.sol b/contracts/staking/contracts/test/TestStakingProxy.sol index b15e846489..9a2b90c91a 100644 --- a/contracts/staking/contracts/test/TestStakingProxy.sol +++ b/contracts/staking/contracts/test/TestStakingProxy.sol @@ -33,23 +33,17 @@ contract TestStakingProxy is _stakingContract, NIL_ADDRESS, NIL_ADDRESS, - NIL_ADDRESS, - NIL_ADDRESS, 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/contracts/test/TestStorageLayout.sol b/contracts/staking/contracts/test/TestStorageLayout.sol deleted file mode 100644 index 3743525d8e..0000000000 --- a/contracts/staking/contracts/test/TestStorageLayout.sol +++ /dev/null @@ -1,141 +0,0 @@ - - -/* - - Copyright 2019 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.5.5; - -import "../src/immutable/MixinStorage.sol"; -import "../src/interfaces/IStructs.sol"; - - -contract TestStorageLayout is - Ownable, - MixinStorage -{ - function assertExpectedStorageLayout() - public - pure - { - assembly { - function revertIncorrectStorageSlot() { - // Revert with `Error("INCORRECT_STORAGE_SLOT")` - mstore(0, 0x504fe8ef00000000000000000000000000000000000000000000000000000000) - mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) - mstore(64, 0x00000016494e434f52524543545f53544f524147455f534c4f54000000000000) - mstore(96, 0) - } - - // The staking contract writes to state that's stored in the staking proxy contract; hence, - // we require that slots do not change across upgrades to the staking contract. We expect - // storage slots to match the ordering in MixinStorage.sol. - - let slot := 0 - - if sub(owner_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(stakingContract_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(_activeStakeByOwner_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(_inactiveStakeByOwner_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(_delegatedStakeByOwner_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(_delegatedStakeToPoolByOwner_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(_delegatedStakeByPoolId_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(_withdrawableStakeByOwner_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(nextPoolId_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(poolJoinedByMakerAddress_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(_poolById_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(currentEpoch_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(currentEpochStartTimeInSeconds_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(_cumulativeRewardsByPool_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(_cumulativeRewardsByPoolReferenceCounter_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(_cumulativeRewardsByPoolLastStored_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(validExchanges_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(zrxVault_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(rewardVault_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(epochDurationInSeconds_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(rewardDelegatedStakeWeight_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(minimumPoolStake_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(maximumMakersInPool_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(cobbDouglasAlphaNumerator_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(cobbDouglasAlphaDenominator_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(totalFeesCollectedThisEpoch_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(totalWeightedStakeThisEpoch_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(_activePoolsByEpoch_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(numActivePoolsThisEpoch_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(unfinalizedState_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - } - } -} diff --git a/contracts/staking/package.json b/contracts/staking/package.json index 4964728dd0..f68c4ac585 100644 --- a/contracts/staking/package.json +++ b/contracts/staking/package.json @@ -37,7 +37,7 @@ }, "config": { "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually.", - "abis": "./generated-artifacts/@(EthVault|IEthVault|IStaking|IStakingEvents|IStakingPoolRewardVault|IStakingProxy|IStorage|IStorageInit|IStructs|IVaultCore|IZrxVault|LibCobbDouglas|LibFixedMath|LibFixedMathRichErrors|LibProxy|LibSafeDowncast|LibStakingRichErrors|MixinAbstract|MixinConstants|MixinCumulativeRewards|MixinDeploymentConstants|MixinExchangeFees|MixinExchangeManager|MixinFinalizer|MixinParams|MixinScheduler|MixinStake|MixinStakeBalances|MixinStakeStorage|MixinStakingPool|MixinStakingPoolMakers|MixinStakingPoolModifiers|MixinStakingPoolRewards|MixinStorage|MixinVaultCore|ReadOnlyProxy|Staking|StakingPoolRewardVault|StakingProxy|TestAssertStorageParams|TestCobbDouglas|TestCumulativeRewardTracking|TestDelegatorRewards|TestFinalizer|TestInitTarget|TestLibFixedMath|TestLibProxy|TestLibProxyReceiver|TestLibSafeDowncast|TestMixinParams|TestMixinVaultCore|TestProtocolFees|TestStaking|TestStakingNoWETH|TestStakingProxy|TestStorageLayout|ZrxVault).json" + "abis": "./generated-artifacts/@(IStaking|IStakingEvents|IStakingProxy|IStorage|IStorageInit|IStructs|IVaultCore|IZrxVault|LibCobbDouglas|LibFixedMath|LibFixedMathRichErrors|LibProxy|LibSafeDowncast|LibStakingRichErrors|MixinAbstract|MixinConstants|MixinCumulativeRewards|MixinDeploymentConstants|MixinExchangeFees|MixinExchangeManager|MixinFinalizer|MixinParams|MixinScheduler|MixinStake|MixinStakeBalances|MixinStakeStorage|MixinStakingPool|MixinStakingPoolMakers|MixinStakingPoolModifiers|MixinStakingPoolRewards|MixinStorage|MixinVaultCore|ReadOnlyProxy|Staking|StakingProxy|TestAssertStorageParams|TestCobbDouglas|TestCumulativeRewardTracking|TestDelegatorRewards|TestFinalizer|TestInitTarget|TestLibFixedMath|TestLibProxy|TestLibProxyReceiver|TestLibSafeDowncast|TestMixinVaultCore|TestProtocolFees|TestStaking|TestStakingNoWETH|TestStakingProxy|ZrxVault).json" }, "repository": { "type": "git", diff --git a/contracts/staking/src/artifacts.ts b/contracts/staking/src/artifacts.ts index 3845d4a837..011e0f0468 100644 --- a/contracts/staking/src/artifacts.ts +++ b/contracts/staking/src/artifacts.ts @@ -5,11 +5,8 @@ */ import { ContractArtifact } from 'ethereum-types'; -import * as EthVault from '../generated-artifacts/EthVault.json'; -import * as IEthVault from '../generated-artifacts/IEthVault.json'; import * as IStaking from '../generated-artifacts/IStaking.json'; import * as IStakingEvents from '../generated-artifacts/IStakingEvents.json'; -import * as IStakingPoolRewardVault from '../generated-artifacts/IStakingPoolRewardVault.json'; import * as IStakingProxy from '../generated-artifacts/IStakingProxy.json'; import * as IStorage from '../generated-artifacts/IStorage.json'; import * as IStorageInit from '../generated-artifacts/IStorageInit.json'; @@ -42,7 +39,6 @@ import * as MixinStorage from '../generated-artifacts/MixinStorage.json'; import * as MixinVaultCore from '../generated-artifacts/MixinVaultCore.json'; import * as ReadOnlyProxy from '../generated-artifacts/ReadOnlyProxy.json'; import * as Staking from '../generated-artifacts/Staking.json'; -import * as StakingPoolRewardVault from '../generated-artifacts/StakingPoolRewardVault.json'; import * as StakingProxy from '../generated-artifacts/StakingProxy.json'; import * as TestAssertStorageParams from '../generated-artifacts/TestAssertStorageParams.json'; import * as TestCobbDouglas from '../generated-artifacts/TestCobbDouglas.json'; @@ -54,13 +50,11 @@ import * as TestLibFixedMath from '../generated-artifacts/TestLibFixedMath.json' import * as TestLibProxy from '../generated-artifacts/TestLibProxy.json'; import * as TestLibProxyReceiver from '../generated-artifacts/TestLibProxyReceiver.json'; import * as TestLibSafeDowncast from '../generated-artifacts/TestLibSafeDowncast.json'; -import * as TestMixinParams from '../generated-artifacts/TestMixinParams.json'; import * as TestMixinVaultCore from '../generated-artifacts/TestMixinVaultCore.json'; import * as TestProtocolFees from '../generated-artifacts/TestProtocolFees.json'; import * as TestStaking from '../generated-artifacts/TestStaking.json'; import * as TestStakingNoWETH from '../generated-artifacts/TestStakingNoWETH.json'; import * as TestStakingProxy from '../generated-artifacts/TestStakingProxy.json'; -import * as TestStorageLayout from '../generated-artifacts/TestStorageLayout.json'; import * as ZrxVault from '../generated-artifacts/ZrxVault.json'; export const artifacts = { ReadOnlyProxy: ReadOnlyProxy as ContractArtifact, @@ -71,10 +65,8 @@ export const artifacts = { MixinConstants: MixinConstants as ContractArtifact, MixinDeploymentConstants: MixinDeploymentConstants as ContractArtifact, MixinStorage: MixinStorage as ContractArtifact, - IEthVault: IEthVault as ContractArtifact, IStaking: IStaking as ContractArtifact, IStakingEvents: IStakingEvents as ContractArtifact, - IStakingPoolRewardVault: IStakingPoolRewardVault as ContractArtifact, IStakingProxy: IStakingProxy as ContractArtifact, IStorage: IStorage as ContractArtifact, IStorageInit: IStorageInit as ContractArtifact, @@ -99,9 +91,7 @@ export const artifacts = { MixinFinalizer: MixinFinalizer as ContractArtifact, MixinParams: MixinParams as ContractArtifact, MixinScheduler: MixinScheduler as ContractArtifact, - EthVault: EthVault as ContractArtifact, MixinVaultCore: MixinVaultCore as ContractArtifact, - StakingPoolRewardVault: StakingPoolRewardVault as ContractArtifact, ZrxVault: ZrxVault as ContractArtifact, TestAssertStorageParams: TestAssertStorageParams as ContractArtifact, TestCobbDouglas: TestCobbDouglas as ContractArtifact, @@ -113,11 +103,9 @@ export const artifacts = { TestLibProxy: TestLibProxy as ContractArtifact, TestLibProxyReceiver: TestLibProxyReceiver as ContractArtifact, TestLibSafeDowncast: TestLibSafeDowncast as ContractArtifact, - TestMixinParams: TestMixinParams as ContractArtifact, TestMixinVaultCore: TestMixinVaultCore as ContractArtifact, TestProtocolFees: TestProtocolFees as ContractArtifact, TestStaking: TestStaking as ContractArtifact, TestStakingNoWETH: TestStakingNoWETH as ContractArtifact, TestStakingProxy: TestStakingProxy as ContractArtifact, - TestStorageLayout: TestStorageLayout as ContractArtifact, }; diff --git a/contracts/staking/src/wrappers.ts b/contracts/staking/src/wrappers.ts index 5af5125178..641e983bee 100644 --- a/contracts/staking/src/wrappers.ts +++ b/contracts/staking/src/wrappers.ts @@ -3,11 +3,8 @@ * Warning: This file is auto-generated by contracts-gen. Don't edit manually. * ----------------------------------------------------------------------------- */ -export * from '../generated-wrappers/eth_vault'; -export * from '../generated-wrappers/i_eth_vault'; export * from '../generated-wrappers/i_staking'; export * from '../generated-wrappers/i_staking_events'; -export * from '../generated-wrappers/i_staking_pool_reward_vault'; export * from '../generated-wrappers/i_staking_proxy'; export * from '../generated-wrappers/i_storage'; export * from '../generated-wrappers/i_storage_init'; @@ -40,7 +37,6 @@ export * from '../generated-wrappers/mixin_storage'; export * from '../generated-wrappers/mixin_vault_core'; export * from '../generated-wrappers/read_only_proxy'; export * from '../generated-wrappers/staking'; -export * from '../generated-wrappers/staking_pool_reward_vault'; export * from '../generated-wrappers/staking_proxy'; export * from '../generated-wrappers/test_assert_storage_params'; export * from '../generated-wrappers/test_cobb_douglas'; @@ -52,11 +48,9 @@ export * from '../generated-wrappers/test_lib_fixed_math'; export * from '../generated-wrappers/test_lib_proxy'; export * from '../generated-wrappers/test_lib_proxy_receiver'; export * from '../generated-wrappers/test_lib_safe_downcast'; -export * from '../generated-wrappers/test_mixin_params'; export * from '../generated-wrappers/test_mixin_vault_core'; export * from '../generated-wrappers/test_protocol_fees'; export * from '../generated-wrappers/test_staking'; export * from '../generated-wrappers/test_staking_no_w_e_t_h'; export * from '../generated-wrappers/test_staking_proxy'; -export * from '../generated-wrappers/test_storage_layout'; export * from '../generated-wrappers/zrx_vault'; diff --git a/contracts/staking/test/storage_layout_test.ts b/contracts/staking/test/storage_layout_test.ts deleted file mode 100644 index a5592247dd..0000000000 --- a/contracts/staking/test/storage_layout_test.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { blockchainTests, expect } from '@0x/contracts-test-utils'; - -import { artifacts, TestStorageLayoutContract } from '../src'; - -blockchainTests.resets('Storage layout tests', env => { - let testStorageLayoutContract: TestStorageLayoutContract; - before(async () => { - testStorageLayoutContract = await TestStorageLayoutContract.deployFrom0xArtifactAsync( - artifacts.TestStorageLayout, - env.provider, - env.txDefaults, - {}, - ); - }); - - it('should have the correct storage slots', async () => { - return expect(testStorageLayoutContract.assertExpectedStorageLayout.callAsync()).to.be.fulfilled(''); - }); -}); diff --git a/contracts/staking/tsconfig.json b/contracts/staking/tsconfig.json index ec1b3032eb..fab0abce47 100644 --- a/contracts/staking/tsconfig.json +++ b/contracts/staking/tsconfig.json @@ -3,11 +3,8 @@ "compilerOptions": { "outDir": "lib", "rootDir": ".", "resolveJsonModule": true }, "include": ["./src/**/*", "./test/**/*", "./generated-wrappers/**/*"], "files": [ - "generated-artifacts/EthVault.json", - "generated-artifacts/IEthVault.json", "generated-artifacts/IStaking.json", "generated-artifacts/IStakingEvents.json", - "generated-artifacts/IStakingPoolRewardVault.json", "generated-artifacts/IStakingProxy.json", "generated-artifacts/IStorage.json", "generated-artifacts/IStorageInit.json", @@ -40,7 +37,6 @@ "generated-artifacts/MixinVaultCore.json", "generated-artifacts/ReadOnlyProxy.json", "generated-artifacts/Staking.json", - "generated-artifacts/StakingPoolRewardVault.json", "generated-artifacts/StakingProxy.json", "generated-artifacts/TestAssertStorageParams.json", "generated-artifacts/TestCobbDouglas.json", @@ -52,13 +48,11 @@ "generated-artifacts/TestLibProxy.json", "generated-artifacts/TestLibProxyReceiver.json", "generated-artifacts/TestLibSafeDowncast.json", - "generated-artifacts/TestMixinParams.json", "generated-artifacts/TestMixinVaultCore.json", "generated-artifacts/TestProtocolFees.json", "generated-artifacts/TestStaking.json", "generated-artifacts/TestStakingNoWETH.json", "generated-artifacts/TestStakingProxy.json", - "generated-artifacts/TestStorageLayout.json", "generated-artifacts/ZrxVault.json" ], "exclude": ["./deploy/solc/solc_bin"] From 7de23c6af2809792737747a0b573f9858dcf73a1 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 22 Sep 2019 15:16:13 -0700 Subject: [PATCH 02/13] Rename variables and functions --- .../contracts/src/fees/MixinExchangeFees.sol | 1 + .../contracts/src/immutable/MixinStorage.sol | 10 +++++----- .../contracts/src/interfaces/IStructs.sol | 2 +- .../staking/contracts/src/stake/MixinStake.sol | 4 ++-- .../contracts/src/stake/MixinStakeBalances.sol | 16 ++++++++-------- .../contracts/src/stake/MixinStakeStorage.sol | 14 +++++++------- .../staking_pools/MixinStakingPoolRewards.sol | 18 +++++++++--------- .../contracts/src/sys/MixinFinalizer.sol | 12 ++++++------ .../contracts/test/TestDelegatorRewards.sol | 8 ++++---- .../staking/contracts/test/TestFinalizer.sol | 4 ++-- .../staking/contracts/test/TestStaking.sol | 1 + .../contracts/test/TestStakingNoWETH.sol | 2 +- 12 files changed, 47 insertions(+), 45 deletions(-) diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index 333fde9011..79abd09277 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol @@ -97,6 +97,7 @@ contract MixinExchangeFees is uint256 currentEpoch_ = currentEpoch; mapping (bytes32 => IStructs.ActivePool) storage activePoolsThisEpoch = _getActivePoolsFromEpoch(currentEpoch_); + IStructs.ActivePool memory pool = activePoolsThisEpoch[poolId]; // If the pool was previously inactive in this epoch, initialize it. diff --git a/contracts/staking/contracts/src/immutable/MixinStorage.sol b/contracts/staking/contracts/src/immutable/MixinStorage.sol index f7b62ec974..7778e35483 100644 --- a/contracts/staking/contracts/src/immutable/MixinStorage.sol +++ b/contracts/staking/contracts/src/immutable/MixinStorage.sol @@ -50,23 +50,23 @@ contract MixinStorage is mapping (uint8 => IStructs.StoredBalance) public globalStakeByStatus; // mapping from Owner to Amount of Active Stake - // (access using _loadAndSyncBalance or _loadUnsyncedBalance) + // (access using _loadSyncedBalance or _loadUnsyncedBalance) mapping (address => IStructs.StoredBalance) internal _activeStakeByOwner; // Mapping from Owner to Amount of Inactive Stake - // (access using _loadAndSyncBalance or _loadUnsyncedBalance) + // (access using _loadSyncedBalance or _loadUnsyncedBalance) mapping (address => IStructs.StoredBalance) internal _inactiveStakeByOwner; // Mapping from Owner to Amount Delegated - // (access using _loadAndSyncBalance or _loadUnsyncedBalance) + // (access using _loadSyncedBalance or _loadUnsyncedBalance) mapping (address => IStructs.StoredBalance) internal _delegatedStakeByOwner; // Mapping from Owner to Pool Id to Amount Delegated - // (access using _loadAndSyncBalance or _loadUnsyncedBalance) + // (access using _loadSyncedBalance or _loadUnsyncedBalance) mapping (address => mapping (bytes32 => IStructs.StoredBalance)) internal _delegatedStakeToPoolByOwner; // Mapping from Pool Id to Amount Delegated - // (access using _loadAndSyncBalance or _loadUnsyncedBalance) + // (access using _loadSyncedBalance or _loadUnsyncedBalance) mapping (bytes32 => IStructs.StoredBalance) internal _delegatedStakeByPoolId; // mapping from Owner to Amount of Withdrawable Stake diff --git a/contracts/staking/contracts/src/interfaces/IStructs.sol b/contracts/staking/contracts/src/interfaces/IStructs.sol index f53ac81efb..e91c68d4d3 100644 --- a/contracts/staking/contracts/src/interfaces/IStructs.sol +++ b/contracts/staking/contracts/src/interfaces/IStructs.sol @@ -52,7 +52,7 @@ interface IStructs { /// @dev Encapsulates a balance for the current and next epochs. /// Note that these balances may be stale if the current epoch /// is greater than `currentEpoch`. - /// Always load this struct using _loadAndSyncBalance or _loadUnsyncedBalance. + /// Always load this struct using _loadSyncedBalance or _loadUnsyncedBalance. /// @param isInitialized /// @param currentEpoch the current epoch /// @param currentEpochBalance balance in the current epoch. diff --git a/contracts/staking/contracts/src/stake/MixinStake.sol b/contracts/staking/contracts/src/stake/MixinStake.sol index 2774b3c2ee..38c7671f61 100644 --- a/contracts/staking/contracts/src/stake/MixinStake.sol +++ b/contracts/staking/contracts/src/stake/MixinStake.sol @@ -216,7 +216,7 @@ contract MixinStake is // Synchronizes reward state in the pool that the staker is delegating // to. IStructs.StoredBalance memory finalDelegatedStakeToPoolByOwner = - _loadAndSyncBalance(_delegatedStakeToPoolByOwner[owner][poolId]); + _loadSyncedBalance(_delegatedStakeToPoolByOwner[owner][poolId]); _withdrawAndSyncDelegatorRewards( poolId, owner, @@ -255,7 +255,7 @@ contract MixinStake is // synchronizes reward state in the pool that the staker is undelegating // from IStructs.StoredBalance memory finalDelegatedStakeToPoolByOwner = - _loadAndSyncBalance(_delegatedStakeToPoolByOwner[owner][poolId]); + _loadSyncedBalance(_delegatedStakeToPoolByOwner[owner][poolId]); _withdrawAndSyncDelegatorRewards( poolId, owner, diff --git a/contracts/staking/contracts/src/stake/MixinStakeBalances.sol b/contracts/staking/contracts/src/stake/MixinStakeBalances.sol index 21350423b1..1872d3f305 100644 --- a/contracts/staking/contracts/src/stake/MixinStakeBalances.sol +++ b/contracts/staking/contracts/src/stake/MixinStakeBalances.sol @@ -43,7 +43,7 @@ contract MixinStakeBalances is view returns (IStructs.StakeBalance memory balance) { - IStructs.StoredBalance memory storedBalance = _loadAndSyncBalance( + IStructs.StoredBalance memory storedBalance = _loadSyncedBalance( globalStakeByStatus[uint8(IStructs.StakeStatus.ACTIVE)] ); return IStructs.StakeBalance({ @@ -59,7 +59,7 @@ contract MixinStakeBalances is view returns (IStructs.StakeBalance memory balance) { - IStructs.StoredBalance memory storedBalance = _loadAndSyncBalance( + IStructs.StoredBalance memory storedBalance = _loadSyncedBalance( globalStakeByStatus[uint8(IStructs.StakeStatus.INACTIVE)] ); return IStructs.StakeBalance({ @@ -75,7 +75,7 @@ contract MixinStakeBalances is view returns (IStructs.StakeBalance memory balance) { - IStructs.StoredBalance memory storedBalance = _loadAndSyncBalance( + IStructs.StoredBalance memory storedBalance = _loadSyncedBalance( globalStakeByStatus[uint8(IStructs.StakeStatus.DELEGATED)] ); return IStructs.StakeBalance({ @@ -103,7 +103,7 @@ contract MixinStakeBalances is view returns (IStructs.StakeBalance memory balance) { - IStructs.StoredBalance memory storedBalance = _loadAndSyncBalance(_activeStakeByOwner[owner]); + IStructs.StoredBalance memory storedBalance = _loadSyncedBalance(_activeStakeByOwner[owner]); return IStructs.StakeBalance({ currentEpochBalance: storedBalance.currentEpochBalance, nextEpochBalance: storedBalance.nextEpochBalance @@ -118,7 +118,7 @@ contract MixinStakeBalances is view returns (IStructs.StakeBalance memory balance) { - IStructs.StoredBalance memory storedBalance = _loadAndSyncBalance(_inactiveStakeByOwner[owner]); + IStructs.StoredBalance memory storedBalance = _loadSyncedBalance(_inactiveStakeByOwner[owner]); return IStructs.StakeBalance({ currentEpochBalance: storedBalance.currentEpochBalance, nextEpochBalance: storedBalance.nextEpochBalance @@ -133,7 +133,7 @@ contract MixinStakeBalances is view returns (IStructs.StakeBalance memory balance) { - IStructs.StoredBalance memory storedBalance = _loadAndSyncBalance(_delegatedStakeByOwner[owner]); + IStructs.StoredBalance memory storedBalance = _loadSyncedBalance(_delegatedStakeByOwner[owner]); return IStructs.StakeBalance({ currentEpochBalance: storedBalance.currentEpochBalance, nextEpochBalance: storedBalance.nextEpochBalance @@ -160,7 +160,7 @@ contract MixinStakeBalances is view returns (IStructs.StakeBalance memory balance) { - IStructs.StoredBalance memory storedBalance = _loadAndSyncBalance(_delegatedStakeToPoolByOwner[owner][poolId]); + IStructs.StoredBalance memory storedBalance = _loadSyncedBalance(_delegatedStakeToPoolByOwner[owner][poolId]); return IStructs.StakeBalance({ currentEpochBalance: storedBalance.currentEpochBalance, nextEpochBalance: storedBalance.nextEpochBalance @@ -176,7 +176,7 @@ contract MixinStakeBalances is view returns (IStructs.StakeBalance memory balance) { - IStructs.StoredBalance memory storedBalance = _loadAndSyncBalance(_delegatedStakeByPoolId[poolId]); + IStructs.StoredBalance memory storedBalance = _loadSyncedBalance(_delegatedStakeByPoolId[poolId]); return IStructs.StakeBalance({ currentEpochBalance: storedBalance.currentEpochBalance, nextEpochBalance: storedBalance.nextEpochBalance diff --git a/contracts/staking/contracts/src/stake/MixinStakeStorage.sol b/contracts/staking/contracts/src/stake/MixinStakeStorage.sol index 87539f45a0..d66b643bda 100644 --- a/contracts/staking/contracts/src/stake/MixinStakeStorage.sol +++ b/contracts/staking/contracts/src/stake/MixinStakeStorage.sol @@ -53,8 +53,8 @@ contract MixinStakeStorage is } // load balance from storage and synchronize it - IStructs.StoredBalance memory from = _loadAndSyncBalance(fromPtr); - IStructs.StoredBalance memory to = _loadAndSyncBalance(toPtr); + IStructs.StoredBalance memory from = _loadSyncedBalance(fromPtr); + IStructs.StoredBalance memory to = _loadSyncedBalance(toPtr); // sanity check on balance if (amount > from.nextEpochBalance) { @@ -81,7 +81,7 @@ contract MixinStakeStorage is /// was stored. /// @param balancePtr to load and sync. /// @return synchronized balance. - function _loadAndSyncBalance(IStructs.StoredBalance storage balancePtr) + function _loadSyncedBalance(IStructs.StoredBalance storage balancePtr) internal view returns (IStructs.StoredBalance memory balance) @@ -119,7 +119,7 @@ contract MixinStakeStorage is internal { // Remove stake from balance - IStructs.StoredBalance memory balance = _loadAndSyncBalance(balancePtr); + IStructs.StoredBalance memory balance = _loadSyncedBalance(balancePtr); balance.nextEpochBalance = uint256(balance.nextEpochBalance).safeAdd(amount).downcastToUint96(); balance.currentEpochBalance = uint256(balance.currentEpochBalance).safeAdd(amount).downcastToUint96(); @@ -134,7 +134,7 @@ contract MixinStakeStorage is internal { // Remove stake from balance - IStructs.StoredBalance memory balance = _loadAndSyncBalance(balancePtr); + IStructs.StoredBalance memory balance = _loadSyncedBalance(balancePtr); balance.nextEpochBalance = uint256(balance.nextEpochBalance).safeSub(amount).downcastToUint96(); balance.currentEpochBalance = uint256(balance.currentEpochBalance).safeSub(amount).downcastToUint96(); @@ -149,7 +149,7 @@ contract MixinStakeStorage is internal { // Add stake to balance - IStructs.StoredBalance memory balance = _loadAndSyncBalance(balancePtr); + IStructs.StoredBalance memory balance = _loadSyncedBalance(balancePtr); balance.nextEpochBalance = uint256(balance.nextEpochBalance).safeAdd(amount).downcastToUint96(); // update state @@ -163,7 +163,7 @@ contract MixinStakeStorage is internal { // Remove stake from balance - IStructs.StoredBalance memory balance = _loadAndSyncBalance(balancePtr); + IStructs.StoredBalance memory balance = _loadSyncedBalance(balancePtr); balance.nextEpochBalance = uint256(balance.nextEpochBalance).safeSub(amount).downcastToUint96(); // update state diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index 9cbeb837b6..4a2bc501c1 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -52,7 +52,7 @@ contract MixinStakingPoolRewards is address member = msg.sender; IStructs.StoredBalance memory finalDelegatedStakeToPoolByOwner = - _loadAndSyncBalance(_delegatedStakeToPoolByOwner[member][poolId]); + _loadSyncedBalance(_delegatedStakeToPoolByOwner[member][poolId]); _withdrawAndSyncDelegatorRewards( poolId, @@ -86,7 +86,7 @@ contract MixinStakingPoolRewards is _getUnfinalizedPoolRewards(poolId); // Get the operators' portion. - (reward,) = _computeSplitStakingPoolRewards( + (reward,) = _computePoolRewardsSplit( pool.operatorShare, unfinalizedTotalRewards, unfinalizedMembersStake @@ -110,12 +110,12 @@ contract MixinStakingPoolRewards is _getUnfinalizedPoolRewards(poolId); // Get the members' portion. - (, uint256 unfinalizedMembersReward) = _computeSplitStakingPoolRewards( + (, uint256 unfinalizedMembersReward) = _computePoolRewardsSplit( pool.operatorShare, unfinalizedTotalRewards, unfinalizedMembersStake ); - return _computeRewardBalanceOfDelegator( + return _computeDelegatorReward( poolId, _loadUnsyncedBalance(_delegatedStakeToPoolByOwner[member][poolId]), currentEpoch, @@ -179,7 +179,7 @@ contract MixinStakingPoolRewards is /// will split the reward. /// @return operatorReward Portion of `reward` given to the pool operator. /// @return membersReward Portion of `reward` given to the pool members. - function _depositStakingPoolRewards( + function _syncPoolRewards( bytes32 poolId, uint256 reward, uint256 membersStake @@ -190,7 +190,7 @@ contract MixinStakingPoolRewards is IStructs.Pool memory pool = _poolById[poolId]; // Split the reward between operator and members - (operatorReward, membersReward) = _computeSplitStakingPoolRewards( + (operatorReward, membersReward) = _computePoolRewardsSplit( pool.operatorShare, reward, membersStake @@ -241,7 +241,7 @@ contract MixinStakingPoolRewards is /// to the pool in the epoch the rewards were earned. /// @return operatorReward Portion of `totalReward` given to the pool operator. /// @return membersReward Portion of `totalReward` given to the pool members. - function _computeSplitStakingPoolRewards( + function _computePoolRewardsSplit( uint32 operatorShare, uint256 totalReward, uint256 membersStake @@ -280,7 +280,7 @@ contract MixinStakingPoolRewards is finalizePool(poolId); // Compute balance owed to delegator - uint256 balance = _computeRewardBalanceOfDelegator( + uint256 balance = _computeDelegatorReward( poolId, unsyncedStake, currentEpoch, @@ -308,7 +308,7 @@ contract MixinStakingPoolRewards is /// (if any). /// @param unfinalizedMembersStake Unfinalized total members stake (if any). /// @return totalReward Balance in ETH. - function _computeRewardBalanceOfDelegator( + function _computeDelegatorReward( bytes32 poolId, IStructs.StoredBalance memory unsyncedStake, uint256 currentEpoch, diff --git a/contracts/staking/contracts/src/sys/MixinFinalizer.sol b/contracts/staking/contracts/src/sys/MixinFinalizer.sol index 7a8e860515..f095471778 100644 --- a/contracts/staking/contracts/src/sys/MixinFinalizer.sol +++ b/contracts/staking/contracts/src/sys/MixinFinalizer.sol @@ -77,7 +77,7 @@ contract MixinFinalizer is } // Set up unfinalized state. - state.rewardsAvailable = _wrapBalanceToWETHAndGetBalance(); + state.rewardsAvailable = _wrapEthAndGetWethBalance(); state.poolsRemaining = poolsRemaining = numActivePoolsThisEpoch; state.totalFeesCollected = totalFeesCollectedThisEpoch; state.totalWeightedStake = totalWeightedStakeThisEpoch; @@ -239,17 +239,17 @@ contract MixinFinalizer is /// @dev Converts the entire ETH balance of the contract into WETH and /// returns the total WETH balance of this contract. /// @return The WETH balance of this contract. - function _wrapBalanceToWETHAndGetBalance() + function _wrapEthAndGetWethBalance() internal - returns (uint256 balance) + returns (uint256 wethBalance) { IEtherToken wethContract = _getWethContract(); uint256 ethBalance = address(this).balance; if (ethBalance != 0) { wethContract.deposit.value(ethBalance)(); } - balance = wethContract.balanceOf(address(this)); - return balance; + wethBalance = wethContract.balanceOf(address(this)); + return wethBalance; } /// @dev Computes the reward owed to a pool during finalization. @@ -317,7 +317,7 @@ contract MixinFinalizer is // Pay the pool. // Note that we credit at the CURRENT epoch even though these rewards // were earned in the previous epoch. - (operatorReward, membersReward) = _depositStakingPoolRewards( + (operatorReward, membersReward) = _syncPoolRewards( poolId, rewards, pool.membersStake diff --git a/contracts/staking/contracts/test/TestDelegatorRewards.sol b/contracts/staking/contracts/test/TestDelegatorRewards.sol index 3a0a83bc36..a35a269ac6 100644 --- a/contracts/staking/contracts/test/TestDelegatorRewards.sol +++ b/contracts/staking/contracts/test/TestDelegatorRewards.sol @@ -85,8 +85,8 @@ contract TestDelegatorRewards is _initGenesisCumulativeRewards(poolId); } - /// @dev Expose/wrap `_depositStakingPoolRewards`. - function depositStakingPoolRewards( + /// @dev Expose/wrap `_syncPoolRewards`. + function syncPoolRewards( bytes32 poolId, address payable operatorAddress, uint256 operatorReward, @@ -100,7 +100,7 @@ contract TestDelegatorRewards is _setOperatorShare(poolId, operatorReward, membersReward); _initGenesisCumulativeRewards(poolId); - _depositStakingPoolRewards( + _syncPoolRewards( poolId, operatorReward + membersReward, membersStake @@ -210,7 +210,7 @@ contract TestDelegatorRewards is uint256 totalRewards = reward.operatorReward + reward.membersReward; membersStake = reward.membersStake; (operatorReward, membersReward) = - _depositStakingPoolRewards(poolId, totalRewards, membersStake); + _syncPoolRewards(poolId, totalRewards, membersStake); emit FinalizePool(poolId, operatorReward, membersReward, membersStake); } diff --git a/contracts/staking/contracts/test/TestFinalizer.sol b/contracts/staking/contracts/test/TestFinalizer.sol index 0d1214c09d..01c644cd04 100644 --- a/contracts/staking/contracts/test/TestFinalizer.sol +++ b/contracts/staking/contracts/test/TestFinalizer.sol @@ -142,7 +142,7 @@ contract TestFinalizer is } /// @dev Overridden to log and transfer to receivers. - function _depositStakingPoolRewards( + function _syncPoolRewards( bytes32 poolId, uint256 reward, uint256 membersStake @@ -151,7 +151,7 @@ contract TestFinalizer is returns (uint256 operatorReward, uint256 membersReward) { uint32 operatorShare = _operatorSharesByPool[poolId]; - (operatorReward, membersReward) = _computeSplitStakingPoolRewards( + (operatorReward, membersReward) = _computePoolRewardsSplit( operatorShare, reward, membersStake diff --git a/contracts/staking/contracts/test/TestStaking.sol b/contracts/staking/contracts/test/TestStaking.sol index 4d8b06e812..49bf62c45b 100644 --- a/contracts/staking/contracts/test/TestStaking.sol +++ b/contracts/staking/contracts/test/TestStaking.sol @@ -22,6 +22,7 @@ pragma experimental ABIEncoderV2; import "../src/Staking.sol"; import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol"; + contract TestStaking is Staking { diff --git a/contracts/staking/contracts/test/TestStakingNoWETH.sol b/contracts/staking/contracts/test/TestStakingNoWETH.sol index afee8892a4..e18bb1f22a 100644 --- a/contracts/staking/contracts/test/TestStakingNoWETH.sol +++ b/contracts/staking/contracts/test/TestStakingNoWETH.sol @@ -28,7 +28,7 @@ import "../src/Staking.sol"; contract TestStakingNoWETH is Staking { - function _wrapBalanceToWETHAndGetBalance() + function _wrapEthAndGetWethBalance() internal returns (uint256 balance) { From ef645e601c63fdd441c01cd49a4ea0374fb73cef Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 22 Sep 2019 16:30:56 -0700 Subject: [PATCH 03/13] Fix all tests except for reward tests (skipped) --- .../staking/test/actors/finalizer_actor.ts | 8 +- contracts/staking/test/actors/staker_actor.ts | 4 +- contracts/staking/test/migration.ts | 85 +------ contracts/staking/test/params.ts | 61 +---- contracts/staking/test/rewards_test.ts | 239 +++++++++--------- .../test/unit_tests/delegator_reward_test.ts | 4 +- contracts/staking/test/utils/api_wrapper.ts | 36 --- contracts/staking/test/utils/constants.ts | 2 - .../cumulative_reward_tracking_simulation.ts | 2 - contracts/staking/test/utils/types.ts | 2 - .../order-utils/src/staking_revert_errors.ts | 16 -- 11 files changed, 138 insertions(+), 321 deletions(-) diff --git a/contracts/staking/test/actors/finalizer_actor.ts b/contracts/staking/test/actors/finalizer_actor.ts index ad1ce3ca75..bbd20129e9 100644 --- a/contracts/staking/test/actors/finalizer_actor.ts +++ b/contracts/staking/test/actors/finalizer_actor.ts @@ -208,7 +208,7 @@ export class FinalizerActor extends BaseActor { ): Promise { const operatorBalanceByPoolId: OperatorBalanceByPoolId = {}; for (const poolId of Object.keys(operatorByPoolId)) { - operatorBalanceByPoolId[poolId] = await this._stakingApiWrapper.ethVaultContract.balanceOf.callAsync( + operatorBalanceByPoolId[poolId] = await this._stakingApiWrapper.wethContract.balanceOf.callAsync( operatorByPoolId[poolId], ); } @@ -228,9 +228,9 @@ export class FinalizerActor extends BaseActor { private async _getRewardVaultBalanceByPoolIdAsync(poolIds: string[]): Promise { const rewardVaultBalanceByPoolId: RewardVaultBalanceByPoolId = {}; for (const poolId of poolIds) { - rewardVaultBalanceByPoolId[poolId] = await this._stakingApiWrapper.rewardVaultContract.balanceOf.callAsync( - poolId, - ); + rewardVaultBalanceByPoolId[ + poolId + ] = await this._stakingApiWrapper.stakingContract.balanceByPoolId.callAsync(poolId); } return rewardVaultBalanceByPoolId; } diff --git a/contracts/staking/test/actors/staker_actor.ts b/contracts/staking/test/actors/staker_actor.ts index cb240e43eb..99b70c970e 100644 --- a/contracts/staking/test/actors/staker_actor.ts +++ b/contracts/staking/test/actors/staker_actor.ts @@ -155,8 +155,8 @@ export class StakerActor extends BaseActor { ); } - public async syncDelegatorRewardsAsync(poolId: string, revertError?: RevertError): Promise { - const txReceiptPromise = this._stakingApiWrapper.stakingContract.syncDelegatorRewards.awaitTransactionSuccessAsync( + public async withdrawDelegatorRewardsAsync(poolId: string, revertError?: RevertError): Promise { + const txReceiptPromise = this._stakingApiWrapper.stakingContract.withdrawDelegatorRewards.awaitTransactionSuccessAsync( poolId, { from: this._owner }, ); diff --git a/contracts/staking/test/migration.ts b/contracts/staking/test/migration.ts index 7ceec54cfa..61cf0a7262 100644 --- a/contracts/staking/test/migration.ts +++ b/contracts/staking/test/migration.ts @@ -101,8 +101,6 @@ blockchainTests('Migration tests', env => { it('should set the correct initial params', async () => { const wethProxyAddress = randomAddress(); - const ethVaultAddress = randomAddress(); - const rewardVaultAddress = randomAddress(); const zrxVaultAddress = randomAddress(); const stakingProxyContractAddress = (await StakingProxyContract.deployFrom0xArtifactAsync( @@ -113,8 +111,6 @@ blockchainTests('Migration tests', env => { stakingContract.address, stakingContract.address, wethProxyAddress, - ethVaultAddress, - rewardVaultAddress, zrxVaultAddress, )).address; @@ -131,9 +127,7 @@ blockchainTests('Migration tests', env => { expect(params[4]).to.bignumber.eq(stakingConstants.DEFAULT_PARAMS.cobbDouglasAlphaNumerator); expect(params[5]).to.bignumber.eq(stakingConstants.DEFAULT_PARAMS.cobbDouglasAlphaDenominator); expect(params[6]).to.eq(wethProxyAddress); - expect(params[7]).to.eq(ethVaultAddress); - expect(params[8]).to.eq(rewardVaultAddress); - expect(params[9]).to.eq(zrxVaultAddress); + expect(params[7]).to.eq(zrxVaultAddress); }); }); @@ -149,8 +143,6 @@ blockchainTests('Migration tests', env => { initTargetContract.address, constants.NULL_ADDRESS, constants.NULL_ADDRESS, - constants.NULL_ADDRESS, - constants.NULL_ADDRESS, { from: notAuthorizedAddress, }, @@ -164,8 +156,6 @@ blockchainTests('Migration tests', env => { initTargetContract.address, constants.NULL_ADDRESS, constants.NULL_ADDRESS, - constants.NULL_ADDRESS, - constants.NULL_ADDRESS, ); await assertInitStateAsync(proxyContract); }); @@ -175,8 +165,6 @@ blockchainTests('Migration tests', env => { initTargetContract.address, constants.NULL_ADDRESS, constants.NULL_ADDRESS, - constants.NULL_ADDRESS, - constants.NULL_ADDRESS, ); const logsArgs = filterLogsToArguments( receipt.logs, @@ -195,29 +183,18 @@ blockchainTests('Migration tests', env => { initTargetContract.address, constants.NULL_ADDRESS, constants.NULL_ADDRESS, - constants.NULL_ADDRESS, - constants.NULL_ADDRESS, ); return expect(tx).to.revertWith(INIT_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, - ); + await proxyContract.setAddressParams.awaitTransactionSuccessAsync(wethProxyAddress, 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, @@ -225,21 +202,15 @@ 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 () => { 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( @@ -248,8 +219,6 @@ 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); } }); @@ -259,8 +228,6 @@ blockchainTests('Migration tests', env => { revertAddress, constants.NULL_ADDRESS, constants.NULL_ADDRESS, - constants.NULL_ADDRESS, - constants.NULL_ADDRESS, ); return expect(tx).to.revertWith(STORAGE_PARAMS_REVERT_ERROR); }); @@ -273,8 +240,6 @@ blockchainTests('Migration tests', env => { 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); @@ -284,32 +249,16 @@ blockchainTests('Migration tests', env => { blockchainTests.resets('Staking.init()', async () => { it('throws if not called by an authorized address', async () => { - const tx = stakingContract.init.awaitTransactionSuccessAsync( - randomAddress(), - randomAddress(), - randomAddress(), - randomAddress(), - { - from: notAuthorizedAddress, - }, - ); + const tx = stakingContract.init.awaitTransactionSuccessAsync(randomAddress(), randomAddress(), { + from: notAuthorizedAddress, + }); const expectedError = new AuthorizableRevertErrors.SenderNotAuthorizedError(notAuthorizedAddress); return expect(tx).to.revertWith(expectedError); }); it('throws if already intitialized', async () => { - await stakingContract.init.awaitTransactionSuccessAsync( - randomAddress(), - randomAddress(), - randomAddress(), - randomAddress(), - ); - const tx = stakingContract.init.awaitTransactionSuccessAsync( - randomAddress(), - randomAddress(), - randomAddress(), - randomAddress(), - ); + await stakingContract.init.awaitTransactionSuccessAsync(randomAddress(), randomAddress()); + const tx = stakingContract.init.awaitTransactionSuccessAsync(randomAddress(), randomAddress()); const expectedError = new StakingRevertErrors.InitializationError(); return expect(tx).to.revertWith(expectedError); }); @@ -441,26 +390,6 @@ blockchainTests('Migration tests', env => { ); expect(tx).to.revertWith(expectedError); }); - it('reverts if ethVault is 0', async () => { - const tx = proxyContract.setAndAssertParams.awaitTransactionSuccessAsync({ - ...stakingConstants.DEFAULT_PARAMS, - ethVaultAddress: constants.NULL_ADDRESS, - }); - const expectedError = new StakingRevertErrors.InvalidParamValueError( - StakingRevertErrors.InvalidParamValueErrorCode.InvalidEthVaultAddress, - ); - expect(tx).to.revertWith(expectedError); - }); - it('reverts if rewardVault is 0', async () => { - const tx = proxyContract.setAndAssertParams.awaitTransactionSuccessAsync({ - ...stakingConstants.DEFAULT_PARAMS, - rewardVaultAddress: constants.NULL_ADDRESS, - }); - const expectedError = new StakingRevertErrors.InvalidParamValueError( - StakingRevertErrors.InvalidParamValueErrorCode.InvalidRewardVaultAddress, - ); - expect(tx).to.revertWith(expectedError); - }); it('reverts if zrxVault is 0', async () => { const tx = proxyContract.setAndAssertParams.awaitTransactionSuccessAsync({ ...stakingConstants.DEFAULT_PARAMS, diff --git a/contracts/staking/test/params.ts b/contracts/staking/test/params.ts index 3b969a25eb..45ba8994a6 100644 --- a/contracts/staking/test/params.ts +++ b/contracts/staking/test/params.ts @@ -1,28 +1,22 @@ -import { blockchainTests, constants, expect, filterLogsToArguments, randomAddress } from '@0x/contracts-test-utils'; +import { blockchainTests, expect, filterLogsToArguments } from '@0x/contracts-test-utils'; import { AuthorizableRevertErrors, BigNumber } from '@0x/utils'; import { TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import * as _ from 'lodash'; -import { - artifacts, - IStakingEventsParamsSetEventArgs, - TestMixinParamsContract, - TestMixinParamsEvents, - TestMixinParamsWETHApproveEventArgs, -} from '../src/'; +import { artifacts, IStakingEventsParamsSetEventArgs, MixinParamsContract } from '../src/'; import { constants as stakingConstants } from './utils/constants'; import { StakingParams } from './utils/types'; blockchainTests('Configurable Parameters unit tests', env => { - let testContract: TestMixinParamsContract; + let testContract: MixinParamsContract; let authorizedAddress: string; let notAuthorizedAddress: string; before(async () => { [authorizedAddress, notAuthorizedAddress] = await env.getAccountAddressesAsync(); - testContract = await TestMixinParamsContract.deployFrom0xArtifactAsync( - artifacts.TestMixinParams, + testContract = await MixinParamsContract.deployFrom0xArtifactAsync( + artifacts.MixinParams, env.provider, env.txDefaults, artifacts, @@ -46,8 +40,6 @@ blockchainTests('Configurable Parameters unit tests', env => { new BigNumber(_params.cobbDouglasAlphaNumerator), new BigNumber(_params.cobbDouglasAlphaDenominator), _params.wethProxyAddress, - _params.ethVaultAddress, - _params.rewardVaultAddress, _params.zrxVaultAddress, { from }, ); @@ -62,8 +54,6 @@ blockchainTests('Configurable Parameters unit tests', env => { expect(event.cobbDouglasAlphaNumerator).to.bignumber.eq(_params.cobbDouglasAlphaNumerator); 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); expect(event.zrxVaultAddress).to.eq(_params.zrxVaultAddress); // Assert `getParams()`. const actual = await testContract.getParams.callAsync(); @@ -74,9 +64,7 @@ blockchainTests('Configurable Parameters unit tests', env => { expect(actual[4]).to.bignumber.eq(_params.cobbDouglasAlphaNumerator); 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); - expect(actual[9]).to.eq(_params.zrxVaultAddress); + expect(actual[7]).to.eq(_params.zrxVaultAddress); return receipt; } @@ -89,43 +77,6 @@ blockchainTests('Configurable Parameters unit tests', env => { it('works if called by owner', async () => { return setParamsAndAssertAsync({}); }); - - describe('WETH allowance', () => { - it('rescinds allowance for old vaults and grants unlimited allowance to new ones', async () => { - const [oldEthVaultAddress, oldRewardVaultAddress, newEthVaultAddress, newRewardVaultAddress] = _.times( - 4, - () => randomAddress(), - ); - await testContract.setVaultAddresses.awaitTransactionSuccessAsync( - oldEthVaultAddress, - oldRewardVaultAddress, - ); - const { logs } = await setParamsAndAssertAsync({ - ethVaultAddress: newEthVaultAddress, - rewardVaultAddress: newRewardVaultAddress, - }); - const approveEvents = filterLogsToArguments( - logs, - TestMixinParamsEvents.WETHApprove, - ); - expect(approveEvents[0]).to.deep.eq({ - spender: oldEthVaultAddress, - amount: constants.ZERO_AMOUNT, - }); - expect(approveEvents[1]).to.deep.eq({ - spender: newEthVaultAddress, - amount: constants.MAX_UINT256, - }); - expect(approveEvents[2]).to.deep.eq({ - spender: oldRewardVaultAddress, - amount: constants.ZERO_AMOUNT, - }); - expect(approveEvents[3]).to.deep.eq({ - spender: newRewardVaultAddress, - amount: constants.MAX_UINT256, - }); - }); - }); }); }); // tslint:enable:no-unnecessary-type-assertion diff --git a/contracts/staking/test/rewards_test.ts b/contracts/staking/test/rewards_test.ts index 836a7e1d55..5179fddafa 100644 --- a/contracts/staking/test/rewards_test.ts +++ b/contracts/staking/test/rewards_test.ts @@ -1,5 +1,5 @@ import { ERC20Wrapper } from '@0x/contracts-asset-proxy'; -import { blockchainTests, describe, expect, shortZip } from '@0x/contracts-test-utils'; +import { blockchainTests, constants, describe, expect, shortZip } from '@0x/contracts-test-utils'; import { BigNumber } from '@0x/utils'; import * as _ from 'lodash'; @@ -12,7 +12,7 @@ import { DelegatorsByPoolId, OperatorByPoolId, StakeInfo, StakeStatus } from './ // tslint:disable:no-unnecessary-type-assertion // tslint:disable:max-file-line-count -blockchainTests.resets('Testing Rewards', env => { +blockchainTests.resets.skip('Testing Rewards', env => { // tokens & addresses let accounts: string[]; let owner: string; @@ -46,8 +46,6 @@ blockchainTests.resets('Testing Rewards', env => { minimumPoolStake: new BigNumber(2), cobbDouglasAlphaNumerator: new BigNumber(1), cobbDouglasAlphaDenominator: new BigNumber(6), - rewardVaultAddress: stakingApiWrapper.rewardVaultContract.address, - ethVaultAddress: stakingApiWrapper.ethVaultContract.address, zrxVaultAddress: stakingApiWrapper.zrxVaultContract.address, }); // setup stakers @@ -75,47 +73,47 @@ blockchainTests.resets('Testing Rewards', env => { describe('Reward Simulation', () => { interface EndBalances { // staker 1 - stakerRewardVaultBalance_1?: BigNumber; - stakerEthVaultBalance_1?: BigNumber; + stakerRewardBalance_1?: BigNumber; + stakerWethBalance_1?: BigNumber; // staker 2 - stakerRewardVaultBalance_2?: BigNumber; - stakerEthVaultBalance_2?: BigNumber; + stakerRewardBalance_2?: BigNumber; + stakerWethBalance_2?: BigNumber; // operator - operatorEthVaultBalance?: BigNumber; + operatorWethBalance?: BigNumber; // undivided balance in reward pool - poolRewardVaultBalance?: BigNumber; - membersRewardVaultBalance?: BigNumber; + poolRewardBalance?: BigNumber; + membersRewardBalance?: BigNumber; } const validateEndBalances = async (_expectedEndBalances: EndBalances): Promise => { const expectedEndBalances = { // staker 1 - stakerRewardVaultBalance_1: - _expectedEndBalances.stakerRewardVaultBalance_1 !== undefined - ? _expectedEndBalances.stakerRewardVaultBalance_1 - : ZERO, - stakerEthVaultBalance_1: - _expectedEndBalances.stakerEthVaultBalance_1 !== undefined - ? _expectedEndBalances.stakerEthVaultBalance_1 - : ZERO, + stakerRewardBalance_1: + _expectedEndBalances.stakerRewardBalance_1 !== undefined + ? _expectedEndBalances.stakerRewardBalance_1 + : constants.ZERO_AMOUNT, + stakerWethBalance_1: + _expectedEndBalances.stakerWethBalance_1 !== undefined + ? _expectedEndBalances.stakerWethBalance_1 + : constants.ZERO_AMOUNT, // staker 2 - stakerRewardVaultBalance_2: - _expectedEndBalances.stakerRewardVaultBalance_2 !== undefined - ? _expectedEndBalances.stakerRewardVaultBalance_2 - : ZERO, - stakerEthVaultBalance_2: - _expectedEndBalances.stakerEthVaultBalance_2 !== undefined - ? _expectedEndBalances.stakerEthVaultBalance_2 - : ZERO, + stakerRewardBalance_2: + _expectedEndBalances.stakerRewardBalance_2 !== undefined + ? _expectedEndBalances.stakerRewardBalance_2 + : constants.ZERO_AMOUNT, + stakerWethBalance_2: + _expectedEndBalances.stakerWethBalance_2 !== undefined + ? _expectedEndBalances.stakerWethBalance_2 + : constants.ZERO_AMOUNT, // operator - operatorEthVaultBalance: - _expectedEndBalances.operatorEthVaultBalance !== undefined - ? _expectedEndBalances.operatorEthVaultBalance - : ZERO, + operatorWethBalance: + _expectedEndBalances.operatorWethBalance !== undefined + ? _expectedEndBalances.operatorWethBalance + : constants.ZERO_AMOUNT, // undivided balance in reward pool - poolRewardVaultBalance: - _expectedEndBalances.poolRewardVaultBalance !== undefined - ? _expectedEndBalances.poolRewardVaultBalance - : ZERO, + poolRewardBalance: + _expectedEndBalances.poolRewardBalance !== undefined + ? _expectedEndBalances.poolRewardBalance + : constants.ZERO_AMOUNT, }; const finalEndBalancesAsArray = await Promise.all([ // staker 1 @@ -123,40 +121,40 @@ blockchainTests.resets('Testing Rewards', env => { poolId, stakers[0].getOwner(), ), - stakingApiWrapper.ethVaultContract.balanceOf.callAsync(stakers[0].getOwner()), + stakingApiWrapper.wethContract.balanceOf.callAsync(stakers[0].getOwner()), // staker 2 stakingApiWrapper.stakingContract.computeRewardBalanceOfDelegator.callAsync( poolId, stakers[1].getOwner(), ), - stakingApiWrapper.ethVaultContract.balanceOf.callAsync(stakers[1].getOwner()), + stakingApiWrapper.wethContract.balanceOf.callAsync(stakers[1].getOwner()), // operator - stakingApiWrapper.ethVaultContract.balanceOf.callAsync(poolOperator.getOwner()), + stakingApiWrapper.wethContract.balanceOf.callAsync(poolOperator.getOwner()), // undivided balance in reward pool - stakingApiWrapper.rewardVaultContract.balanceOf.callAsync(poolId), + stakingApiWrapper.stakingContract.balanceByPoolId.callAsync(poolId), ]); - expect(finalEndBalancesAsArray[0], 'stakerRewardVaultBalance_1').to.be.bignumber.equal( - expectedEndBalances.stakerRewardVaultBalance_1, + expect(finalEndBalancesAsArray[0], 'stakerRewardBalance_1').to.be.bignumber.equal( + expectedEndBalances.stakerRewardBalance_1, ); - expect(finalEndBalancesAsArray[1], 'stakerEthVaultBalance_1').to.be.bignumber.equal( - expectedEndBalances.stakerEthVaultBalance_1, + expect(finalEndBalancesAsArray[1], 'stakerWethBalance_1').to.be.bignumber.equal( + expectedEndBalances.stakerWethBalance_1, ); - expect(finalEndBalancesAsArray[2], 'stakerRewardVaultBalance_2').to.be.bignumber.equal( - expectedEndBalances.stakerRewardVaultBalance_2, + expect(finalEndBalancesAsArray[2], 'stakerRewardBalance_2').to.be.bignumber.equal( + expectedEndBalances.stakerRewardBalance_2, ); - expect(finalEndBalancesAsArray[3], 'stakerEthVaultBalance_2').to.be.bignumber.equal( - expectedEndBalances.stakerEthVaultBalance_2, + expect(finalEndBalancesAsArray[3], 'stakerWethBalance_2').to.be.bignumber.equal( + expectedEndBalances.stakerWethBalance_2, ); - expect(finalEndBalancesAsArray[4], 'operatorEthVaultBalance').to.be.bignumber.equal( - expectedEndBalances.operatorEthVaultBalance, + expect(finalEndBalancesAsArray[4], 'operatorWethBalance').to.be.bignumber.equal( + expectedEndBalances.operatorWethBalance, ); - expect(finalEndBalancesAsArray[5], 'poolRewardVaultBalance').to.be.bignumber.equal( - expectedEndBalances.poolRewardVaultBalance, + expect(finalEndBalancesAsArray[5], 'poolRewardBalance').to.be.bignumber.equal( + expectedEndBalances.poolRewardBalance, ); }; const payProtocolFeeAndFinalize = async (_fee?: BigNumber) => { - const fee = _fee !== undefined ? _fee : ZERO; - if (!fee.eq(ZERO)) { + const fee = _fee !== undefined ? _fee : constants.ZERO_AMOUNT; + if (!fee.eq(constants.ZERO_AMOUNT)) { await stakingApiWrapper.stakingContract.payProtocolFee.awaitTransactionSuccessAsync( poolOperator.getOwner(), takerAddress, @@ -166,7 +164,6 @@ blockchainTests.resets('Testing Rewards', env => { } await finalizer.finalizeAsync(); }; - const ZERO = new BigNumber(0); it('Reward balance should be zero if not delegated', async () => { // sanity balances - all zero await validateEndBalances({}); @@ -193,7 +190,7 @@ blockchainTests.resets('Testing Rewards', env => { await payProtocolFeeAndFinalize(reward); // sanity check final balances - all zero await validateEndBalances({ - operatorEthVaultBalance: reward, + operatorWethBalance: reward, }); }); it(`Operator should receive entire reward if no delegators in their pool @@ -206,7 +203,7 @@ blockchainTests.resets('Testing Rewards', env => { await payProtocolFeeAndFinalize(reward); // sanity check final balances await validateEndBalances({ - operatorEthVaultBalance: reward, + operatorWethBalance: reward, }); }); it('Should give pool reward to delegator', async () => { @@ -220,9 +217,9 @@ blockchainTests.resets('Testing Rewards', env => { await payProtocolFeeAndFinalize(reward); // sanity check final balances await validateEndBalances({ - stakerRewardVaultBalance_1: reward, - poolRewardVaultBalance: reward, - membersRewardVaultBalance: reward, + stakerRewardBalance_1: reward, + poolRewardBalance: reward, + membersRewardBalance: reward, }); }); it('Should split pool reward between delegators', async () => { @@ -239,10 +236,10 @@ blockchainTests.resets('Testing Rewards', env => { await payProtocolFeeAndFinalize(reward); // sanity check final balances await validateEndBalances({ - stakerRewardVaultBalance_1: reward.times(stakeAmounts[0]).dividedToIntegerBy(totalStakeAmount), - stakerRewardVaultBalance_2: reward.times(stakeAmounts[1]).dividedToIntegerBy(totalStakeAmount), - poolRewardVaultBalance: reward, - membersRewardVaultBalance: reward, + stakerRewardBalance_1: reward.times(stakeAmounts[0]).dividedToIntegerBy(totalStakeAmount), + stakerRewardBalance_2: reward.times(stakeAmounts[1]).dividedToIntegerBy(totalStakeAmount), + poolRewardBalance: reward, + membersRewardBalance: reward, }); }); it('Should split pool reward between delegators, when they join in different epochs', async () => { @@ -276,10 +273,10 @@ blockchainTests.resets('Testing Rewards', env => { await payProtocolFeeAndFinalize(reward); // sanity check final balances await validateEndBalances({ - stakerRewardVaultBalance_1: reward.times(stakeAmounts[0]).dividedToIntegerBy(totalStakeAmount), - stakerRewardVaultBalance_2: reward.times(stakeAmounts[1]).dividedToIntegerBy(totalStakeAmount), - poolRewardVaultBalance: reward, - membersRewardVaultBalance: reward, + stakerRewardBalance_1: reward.times(stakeAmounts[0]).dividedToIntegerBy(totalStakeAmount), + stakerRewardBalance_2: reward.times(stakeAmounts[1]).dividedToIntegerBy(totalStakeAmount), + poolRewardBalance: reward, + membersRewardBalance: reward, }); }); it('Should give pool reward to delegators only for the epoch during which they delegated', async () => { @@ -299,14 +296,14 @@ blockchainTests.resets('Testing Rewards', env => { await payProtocolFeeAndFinalize(rewardForBothDelegators); // sanity check final balances await validateEndBalances({ - stakerRewardVaultBalance_1: rewardForOnlyFirstDelegator.plus( + stakerRewardBalance_1: rewardForOnlyFirstDelegator.plus( rewardForBothDelegators.times(stakeAmounts[0]).dividedToIntegerBy(totalStakeAmount), ), - stakerRewardVaultBalance_2: rewardForBothDelegators + stakerRewardBalance_2: rewardForBothDelegators .times(stakeAmounts[1]) .dividedToIntegerBy(totalStakeAmount), - poolRewardVaultBalance: rewardForOnlyFirstDelegator.plus(rewardForBothDelegators), - membersRewardVaultBalance: rewardForOnlyFirstDelegator.plus(rewardForBothDelegators), + poolRewardBalance: rewardForOnlyFirstDelegator.plus(rewardForBothDelegators), + membersRewardBalance: rewardForOnlyFirstDelegator.plus(rewardForBothDelegators), }); }); it('Should split pool reward between delegators, over several consecutive epochs', async () => { @@ -339,14 +336,12 @@ blockchainTests.resets('Testing Rewards', env => { } // sanity check final balances await validateEndBalances({ - stakerRewardVaultBalance_1: rewardForOnlyFirstDelegator.plus( + stakerRewardBalance_1: rewardForOnlyFirstDelegator.plus( totalSharedRewards.times(stakeAmounts[0]).dividedToIntegerBy(totalStakeAmount), ), - stakerRewardVaultBalance_2: totalSharedRewards - .times(stakeAmounts[1]) - .dividedToIntegerBy(totalStakeAmount), - poolRewardVaultBalance: rewardForOnlyFirstDelegator.plus(totalSharedRewards), - membersRewardVaultBalance: rewardForOnlyFirstDelegator.plus(totalSharedRewards), + stakerRewardBalance_2: totalSharedRewards.times(stakeAmounts[1]).dividedToIntegerBy(totalStakeAmount), + poolRewardBalance: rewardForOnlyFirstDelegator.plus(totalSharedRewards), + membersRewardBalance: rewardForOnlyFirstDelegator.plus(totalSharedRewards), }); }); it('Should send existing rewards from reward vault to eth vault correctly when undelegating stake', async () => { @@ -366,8 +361,8 @@ blockchainTests.resets('Testing Rewards', env => { ); // sanity check final balances await validateEndBalances({ - stakerRewardVaultBalance_1: ZERO, - stakerEthVaultBalance_1: reward, + stakerRewardBalance_1: constants.ZERO_AMOUNT, + stakerWethBalance_1: reward, }); }); it('Should send existing rewards from reward vault to eth vault correctly when delegating more stake', async () => { @@ -383,8 +378,8 @@ blockchainTests.resets('Testing Rewards', env => { await stakers[0].stakeWithPoolAsync(poolId, stakeAmount); // sanity check final balances await validateEndBalances({ - stakerRewardVaultBalance_1: ZERO, - stakerEthVaultBalance_1: reward, + stakerRewardBalance_1: constants.ZERO_AMOUNT, + stakerWethBalance_1: reward, }); }); it('Should continue earning rewards after adding more stake and progressing several epochs', async () => { @@ -414,18 +409,18 @@ blockchainTests.resets('Testing Rewards', env => { } // sanity check final balances await validateEndBalances({ - stakerRewardVaultBalance_1: rewardBeforeAddingMoreStake.plus( + stakerRewardBalance_1: rewardBeforeAddingMoreStake.plus( totalRewardsAfterAddingMoreStake .times(stakeAmounts[0]) .dividedBy(totalStake) .integerValue(BigNumber.ROUND_DOWN), ), - stakerRewardVaultBalance_2: totalRewardsAfterAddingMoreStake + stakerRewardBalance_2: totalRewardsAfterAddingMoreStake .times(stakeAmounts[1]) .dividedBy(totalStake) .integerValue(BigNumber.ROUND_DOWN), - poolRewardVaultBalance: rewardBeforeAddingMoreStake.plus(totalRewardsAfterAddingMoreStake), - membersRewardVaultBalance: rewardBeforeAddingMoreStake.plus(totalRewardsAfterAddingMoreStake), + poolRewardBalance: rewardBeforeAddingMoreStake.plus(totalRewardsAfterAddingMoreStake), + membersRewardBalance: rewardBeforeAddingMoreStake.plus(totalRewardsAfterAddingMoreStake), }); }); it('Should stop collecting rewards after undelegating', async () => { @@ -453,8 +448,8 @@ blockchainTests.resets('Testing Rewards', env => { // sanity check final balances await validateEndBalances({ - stakerEthVaultBalance_1: rewardForDelegator, - operatorEthVaultBalance: rewardNotForDelegator, + stakerWethBalance_1: rewardForDelegator, + operatorWethBalance: rewardNotForDelegator, }); }); it('Should stop collecting rewards after undelegating, after several epochs', async () => { @@ -488,8 +483,8 @@ blockchainTests.resets('Testing Rewards', env => { } // sanity check final balances await validateEndBalances({ - stakerEthVaultBalance_1: rewardForDelegator, - operatorEthVaultBalance: totalRewardsNotForDelegator, + stakerWethBalance_1: rewardForDelegator, + operatorWethBalance: totalRewardsNotForDelegator, }); }); it('Should collect fees correctly when leaving and returning to a pool', async () => { @@ -522,10 +517,10 @@ blockchainTests.resets('Testing Rewards', env => { await payProtocolFeeAndFinalize(rewardsForDelegator[1]); // sanity check final balances await validateEndBalances({ - stakerRewardVaultBalance_1: rewardsForDelegator[1], - stakerEthVaultBalance_1: rewardsForDelegator[0], - operatorEthVaultBalance: rewardNotForDelegator, - poolRewardVaultBalance: rewardsForDelegator[1], + stakerRewardBalance_1: rewardsForDelegator[1], + stakerWethBalance_1: rewardsForDelegator[0], + operatorWethBalance: rewardNotForDelegator, + poolRewardBalance: rewardsForDelegator[1], }); }); it('Should collect fees correctly when re-delegating after un-delegating', async () => { @@ -559,10 +554,10 @@ blockchainTests.resets('Testing Rewards', env => { ); // sanity check final balances await validateEndBalances({ - stakerRewardVaultBalance_1: ZERO, - stakerEthVaultBalance_1: rewardForDelegator, - operatorEthVaultBalance: ZERO, - poolRewardVaultBalance: ZERO, + stakerRewardBalance_1: constants.ZERO_AMOUNT, + stakerWethBalance_1: rewardForDelegator, + operatorWethBalance: constants.ZERO_AMOUNT, + poolRewardBalance: constants.ZERO_AMOUNT, }); }); it('Should withdraw delegator rewards to eth vault when calling `syncDelegatorRewards`', async () => { @@ -579,15 +574,15 @@ blockchainTests.resets('Testing Rewards', env => { await payProtocolFeeAndFinalize(); // this should go to the delegator await payProtocolFeeAndFinalize(rewardForDelegator); - await stakingApiWrapper.stakingContract.syncDelegatorRewards.awaitTransactionSuccessAsync(poolId, { + await stakingApiWrapper.stakingContract.withdrawDelegatorRewards.awaitTransactionSuccessAsync(poolId, { from: stakers[0].getOwner(), }); // sanity check final balances await validateEndBalances({ - stakerRewardVaultBalance_1: ZERO, - stakerEthVaultBalance_1: rewardForDelegator, - operatorEthVaultBalance: ZERO, - poolRewardVaultBalance: ZERO, + stakerRewardBalance_1: constants.ZERO_AMOUNT, + stakerWethBalance_1: rewardForDelegator, + operatorWethBalance: constants.ZERO_AMOUNT, + poolRewardBalance: constants.ZERO_AMOUNT, }); }); it(`payout should be based on stake at the time of rewards`, async () => { @@ -608,10 +603,10 @@ blockchainTests.resets('Testing Rewards', env => { const reward = toBaseUnitAmount(10); await payProtocolFeeAndFinalize(reward); // Sync rewards to move the rewards into the EthVault. - await staker.syncDelegatorRewardsAsync(poolId); + await staker.withdrawDelegatorRewardsAsync(poolId); await validateEndBalances({ - stakerRewardVaultBalance_1: toBaseUnitAmount(0), - stakerEthVaultBalance_1: reward, + stakerRewardBalance_1: toBaseUnitAmount(0), + stakerWethBalance_1: reward, }); }); it(`should split payout between two delegators when syncing rewards`, async () => { @@ -629,16 +624,16 @@ blockchainTests.resets('Testing Rewards', env => { await payProtocolFeeAndFinalize(reward); // Sync rewards to move rewards from RewardVault into the EthVault. for (const [staker] of _.reverse(stakersAndStake)) { - await staker.syncDelegatorRewardsAsync(poolId); + await staker.withdrawDelegatorRewardsAsync(poolId); } const expectedStakerRewards = stakeAmounts.map(n => reward.times(n).dividedToIntegerBy(totalStakeAmount)); await validateEndBalances({ - stakerRewardVaultBalance_1: toBaseUnitAmount(0), - stakerRewardVaultBalance_2: toBaseUnitAmount(0), - stakerEthVaultBalance_1: expectedStakerRewards[0], - stakerEthVaultBalance_2: expectedStakerRewards[1], - poolRewardVaultBalance: new BigNumber(1), // Rounding error - membersRewardVaultBalance: new BigNumber(1), // Rounding error + stakerRewardBalance_1: toBaseUnitAmount(0), + stakerRewardBalance_2: toBaseUnitAmount(0), + stakerWethBalance_1: expectedStakerRewards[0], + stakerWethBalance_2: expectedStakerRewards[1], + poolRewardBalance: new BigNumber(1), // Rounding error + membersRewardBalance: new BigNumber(1), // Rounding error }); }); it(`delegator should not be credited payout twice by syncing rewards twice`, async () => { @@ -656,28 +651,28 @@ blockchainTests.resets('Testing Rewards', env => { await payProtocolFeeAndFinalize(reward); const expectedStakerRewards = stakeAmounts.map(n => reward.times(n).dividedToIntegerBy(totalStakeAmount)); await validateEndBalances({ - stakerRewardVaultBalance_1: expectedStakerRewards[0], - stakerRewardVaultBalance_2: expectedStakerRewards[1], - stakerEthVaultBalance_1: toBaseUnitAmount(0), - stakerEthVaultBalance_2: toBaseUnitAmount(0), - poolRewardVaultBalance: reward, - membersRewardVaultBalance: reward, + stakerRewardBalance_1: expectedStakerRewards[0], + stakerRewardBalance_2: expectedStakerRewards[1], + stakerWethBalance_1: toBaseUnitAmount(0), + stakerWethBalance_2: toBaseUnitAmount(0), + poolRewardBalance: reward, + membersRewardBalance: reward, }); // First staker will sync rewards to get rewards transferred to EthVault. const sneakyStaker = stakers[0]; const sneakyStakerExpectedEthVaultBalance = expectedStakerRewards[0]; - await sneakyStaker.syncDelegatorRewardsAsync(poolId); + await sneakyStaker.withdrawDelegatorRewardsAsync(poolId); // Should have been credited the correct amount of rewards. - let sneakyStakerEthVaultBalance = await stakingApiWrapper.ethVaultContract.balanceOf.callAsync( + let sneakyStakerEthVaultBalance = await stakingApiWrapper.wethContract.balanceOf.callAsync( sneakyStaker.getOwner(), ); expect(sneakyStakerEthVaultBalance, 'EthVault balance after first undelegate').to.bignumber.eq( sneakyStakerExpectedEthVaultBalance, ); // Now he'll try to do it again to see if he gets credited twice. - await sneakyStaker.syncDelegatorRewardsAsync(poolId); + await sneakyStaker.withdrawDelegatorRewardsAsync(poolId); /// The total amount credited should remain the same. - sneakyStakerEthVaultBalance = await stakingApiWrapper.ethVaultContract.balanceOf.callAsync( + sneakyStakerEthVaultBalance = await stakingApiWrapper.wethContract.balanceOf.callAsync( sneakyStaker.getOwner(), ); expect(sneakyStakerEthVaultBalance, 'EthVault balance after second undelegate').to.bignumber.eq( diff --git a/contracts/staking/test/unit_tests/delegator_reward_test.ts b/contracts/staking/test/unit_tests/delegator_reward_test.ts index f028db6b19..5b4a5b0815 100644 --- a/contracts/staking/test/unit_tests/delegator_reward_test.ts +++ b/contracts/staking/test/unit_tests/delegator_reward_test.ts @@ -26,7 +26,7 @@ import { toBaseUnitAmount, } from '../utils/number_utils'; -blockchainTests.resets('delegator unit rewards', env => { +blockchainTests.resets.skip('delegator unit rewards', env => { let testContract: TestDelegatorRewardsContract; before(async () => { @@ -57,7 +57,7 @@ blockchainTests.resets('delegator unit rewards', env => { }; // Generate a deterministic operator address based on the poolId. _opts.operator = poolIdToOperator(_opts.poolId); - await testContract.depositStakingPoolRewards.awaitTransactionSuccessAsync( + await testContract.syncPoolRewards.awaitTransactionSuccessAsync( _opts.poolId, _opts.operator, new BigNumber(_opts.operatorReward), diff --git a/contracts/staking/test/utils/api_wrapper.ts b/contracts/staking/test/utils/api_wrapper.ts index e3121fba35..94ac58c1e1 100644 --- a/contracts/staking/test/utils/api_wrapper.ts +++ b/contracts/staking/test/utils/api_wrapper.ts @@ -8,11 +8,9 @@ import * as _ from 'lodash'; import { artifacts, - EthVaultContract, IStakingEventsEpochEndedEventArgs, IStakingEventsStakingPoolActivatedEventArgs, ReadOnlyProxyContract, - StakingPoolRewardVaultContract, StakingProxyContract, TestCobbDouglasContract, TestStakingContract, @@ -31,8 +29,6 @@ export class StakingApiWrapper { // The StakingProxy.sol contract as a StakingProxyContract public stakingProxyContract: StakingProxyContract; public zrxVaultContract: ZrxVaultContract; - public ethVaultContract: EthVaultContract; - public rewardVaultContract: StakingPoolRewardVaultContract; public zrxTokenContract: DummyERC20TokenContract; public wethContract: WETH9Contract; public cobbDouglasContract: TestCobbDouglasContract; @@ -122,8 +118,6 @@ export class StakingApiWrapper { new BigNumber(_params.cobbDouglasAlphaNumerator), new BigNumber(_params.cobbDouglasAlphaDenominator), _params.wethProxyAddress, - _params.ethVaultAddress, - _params.rewardVaultAddress, _params.zrxVaultAddress, ); }, @@ -144,8 +138,6 @@ export class StakingApiWrapper { 'cobbDouglasAlphaNumerator', 'cobbDouglasAlphaDenominator', 'wethProxyAddress', - 'ethVaultAddress', - 'rewardVaultAddress', 'zrxVaultAddress', ], await this.stakingContract.getParams.callAsync(), @@ -180,16 +172,12 @@ export class StakingApiWrapper { stakingProxyContract: StakingProxyContract, stakingContract: TestStakingContract, zrxVaultContract: ZrxVaultContract, - ethVaultContract: EthVaultContract, - rewardVaultContract: StakingPoolRewardVaultContract, zrxTokenContract: DummyERC20TokenContract, wethContract: WETH9Contract, cobbDouglasContract: TestCobbDouglasContract, ) { this._web3Wrapper = env.web3Wrapper; this.zrxVaultContract = zrxVaultContract; - this.ethVaultContract = ethVaultContract; - this.rewardVaultContract = rewardVaultContract; this.zrxTokenContract = zrxTokenContract; this.wethContract = wethContract; this.cobbDouglasContract = cobbDouglasContract; @@ -253,22 +241,6 @@ export async function deployAndConfigureContractsAsync( env.txDefaults, artifacts, ); - // deploy eth vault - const ethVaultContract = await EthVaultContract.deployFrom0xArtifactAsync( - artifacts.EthVault, - env.provider, - env.txDefaults, - artifacts, - wethContract.address, - ); - // deploy reward vault - const rewardVaultContract = await StakingPoolRewardVaultContract.deployFrom0xArtifactAsync( - artifacts.StakingPoolRewardVault, - env.provider, - env.txDefaults, - artifacts, - wethContract.address, - ); // deploy zrx vault const zrxVaultContract = await ZrxVaultContract.deployFrom0xArtifactAsync( artifacts.ZrxVault, @@ -287,8 +259,6 @@ export async function deployAndConfigureContractsAsync( stakingContract.address, readOnlyProxyContract.address, erc20ProxyContract.address, - ethVaultContract.address, - rewardVaultContract.address, zrxVaultContract.address, ); // deploy cobb douglas contract @@ -303,18 +273,12 @@ export async function deployAndConfigureContractsAsync( 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); - // set staking proxy contract in eth vault - await ethVaultContract.setStakingProxy.awaitTransactionSuccessAsync(stakingProxyContract.address); return new StakingApiWrapper( env, ownerAddress, stakingProxyContract, stakingContract, zrxVaultContract, - ethVaultContract, - rewardVaultContract, zrxTokenContract, wethContract, cobbDouglasContract, diff --git a/contracts/staking/test/utils/constants.ts b/contracts/staking/test/utils/constants.ts index d9ce44ab29..c1be524905 100644 --- a/contracts/staking/test/utils/constants.ts +++ b/contracts/staking/test/utils/constants.ts @@ -18,8 +18,6 @@ export const constants = { cobbDouglasAlphaNumerator: new BigNumber(1), cobbDouglasAlphaDenominator: new BigNumber(2), wethProxyAddress: randomAddress(), - ethVaultAddress: randomAddress(), - rewardVaultAddress: randomAddress(), zrxVaultAddress: randomAddress(), }, PPM, diff --git a/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts b/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts index 7549e709b8..9af5885bd7 100644 --- a/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts +++ b/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts @@ -111,8 +111,6 @@ export class CumulativeRewardTrackingSimulation { 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); diff --git a/contracts/staking/test/utils/types.ts b/contracts/staking/test/utils/types.ts index 4f375956c7..86e1af75c0 100644 --- a/contracts/staking/test/utils/types.ts +++ b/contracts/staking/test/utils/types.ts @@ -12,8 +12,6 @@ export interface StakingParams { cobbDouglasAlphaNumerator: Numberish; cobbDouglasAlphaDenominator: Numberish; wethProxyAddress: string; - ethVaultAddress: string; - rewardVaultAddress: string; zrxVaultAddress: string; } diff --git a/packages/order-utils/src/staking_revert_errors.ts b/packages/order-utils/src/staking_revert_errors.ts index fccd286a05..698037a989 100644 --- a/packages/order-utils/src/staking_revert_errors.ts +++ b/packages/order-utils/src/staking_revert_errors.ts @@ -25,8 +25,6 @@ export enum InvalidParamValueErrorCode { InvalidMaximumMakersInPool, InvalidMinimumPoolStake, InvalidWethProxyAddress, - InvalidEthVaultAddress, - InvalidRewardVaultAddress, InvalidZrxVaultAddress, InvalidEpochDuration, } @@ -190,18 +188,6 @@ export class InvalidParamValueError extends RevertError { } } -export class EthVaultNotSetError extends RevertError { - constructor() { - super('EthVaultNotSetError', 'EthVaultNotSetError()'); - } -} - -export class RewardVaultNotSetError extends RevertError { - constructor() { - super('RewardVaultNotSetError', 'RewardVaultNotSetError()'); - } -} - export class InvalidStakeStatusError extends RevertError { constructor(status?: BigNumber) { super('InvalidStakeStatusError', 'InvalidStakeStatusError(uint256 status)', { status }); @@ -247,7 +233,6 @@ export class PreviousEpochNotFinalizedError extends RevertError { const types = [ AmountExceedsBalanceOfPoolError, BlockTimestampTooLowError, - EthVaultNotSetError, ExchangeAddressAlreadyRegisteredError, ExchangeAddressNotRegisteredError, InitializationError, @@ -267,7 +252,6 @@ const types = [ PoolExistenceError, PreviousEpochNotFinalizedError, ProxyDestinationCannotBeNilError, - RewardVaultNotSetError, WithdrawAmountExceedsMemberBalanceError, ]; From 1710f13242ca1d47d5908adae462fc5c90941837 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 22 Sep 2019 21:34:19 -0700 Subject: [PATCH 04/13] Fix incorrect return value in _syncPoolRewards --- .../staking_pools/MixinStakingPoolRewards.sol | 42 ++++++++++--------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index 4a2bc501c1..5837954811 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -195,39 +195,41 @@ contract MixinStakingPoolRewards is reward, membersStake ); - // Transfer the operator's weth reward to the operator - _getWethContract().transfer(pool.operator, operatorReward); - if (membersReward == 0) { - return (0, 0); + if (operatorReward > 0) { + // Transfer the operator's weth reward to the operator + _getWethContract().transfer(pool.operator, operatorReward); } - // Increment the balance of the pool - balanceByPoolId[poolId] = balanceByPoolId[poolId].safeAdd(membersReward); - // Fetch the last epoch at which we stored an entry for this pool; - // this is the most up-to-date cumulative rewards for this pool. - IStructs.Fraction memory mostRecentCumulativeReward = _getMostRecentCumulativeReward(poolId); + if (membersReward > 0) { + // Increment the balance of the pool + balanceByPoolId[poolId] = balanceByPoolId[poolId].safeAdd(membersReward); - // Compute new cumulative reward - IStructs.Fraction memory cumulativeReward; - (cumulativeReward.numerator, cumulativeReward.denominator) = LibFractions.add( + // Fetch the last epoch at which we stored an entry for this pool; + // this is the most up-to-date cumulative rewards for this pool. + IStructs.Fraction memory mostRecentCumulativeReward = _getMostRecentCumulativeReward(poolId); + + // Compute new cumulative reward + IStructs.Fraction memory cumulativeReward; + (cumulativeReward.numerator, cumulativeReward.denominator) = LibFractions.add( mostRecentCumulativeReward.numerator, mostRecentCumulativeReward.denominator, membersReward, membersStake ); - // Normalize to prevent overflows. - (cumulativeReward.numerator, cumulativeReward.denominator) = LibFractions.normalize( + // Normalize to prevent overflows. + (cumulativeReward.numerator, cumulativeReward.denominator) = LibFractions.normalize( cumulativeReward.numerator, cumulativeReward.denominator ); - // Store cumulative rewards for this epoch. - _forceSetCumulativeReward( - poolId, - currentEpoch, - cumulativeReward - ); + // Store cumulative rewards for this epoch. + _forceSetCumulativeReward( + poolId, + currentEpoch, + cumulativeReward + ); + } return (operatorReward, membersReward); } From ee687a7dc4a51f915338b87c27a29b8b708b110f Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 22 Sep 2019 22:33:08 -0700 Subject: [PATCH 05/13] Fix delegator reward unit tests --- .../contracts/test/TestDelegatorRewards.sol | 16 +-- .../contracts/test/TestStakingNoWETH.sol | 22 ++++ .../test/unit_tests/delegator_reward_test.ts | 107 ++++++++---------- 3 files changed, 72 insertions(+), 73 deletions(-) diff --git a/contracts/staking/contracts/test/TestDelegatorRewards.sol b/contracts/staking/contracts/test/TestDelegatorRewards.sol index a35a269ac6..eb19e54f21 100644 --- a/contracts/staking/contracts/test/TestDelegatorRewards.sol +++ b/contracts/staking/contracts/test/TestDelegatorRewards.sol @@ -26,16 +26,6 @@ import "./TestStakingNoWETH.sol"; contract TestDelegatorRewards is TestStakingNoWETH { - event RecordDepositToEthVault( - address owner, - uint256 amount - ); - - event RecordDepositToRewardVault( - bytes32 poolId, - uint256 membersReward - ); - event FinalizePool( bytes32 poolId, uint256 operatorReward, @@ -49,7 +39,9 @@ contract TestDelegatorRewards is uint256 membersStake; } - constructor() public { + constructor() + public + { init( address(1), address(1) @@ -99,7 +91,7 @@ contract TestDelegatorRewards is _poolById[poolId].operator = operatorAddress; _setOperatorShare(poolId, operatorReward, membersReward); _initGenesisCumulativeRewards(poolId); - + _syncPoolRewards( poolId, operatorReward + membersReward, diff --git a/contracts/staking/contracts/test/TestStakingNoWETH.sol b/contracts/staking/contracts/test/TestStakingNoWETH.sol index e18bb1f22a..135c68b00d 100644 --- a/contracts/staking/contracts/test/TestStakingNoWETH.sol +++ b/contracts/staking/contracts/test/TestStakingNoWETH.sol @@ -28,10 +28,32 @@ import "../src/Staking.sol"; contract TestStakingNoWETH is Staking { + event Transfer( + address indexed _from, + address indexed _to, + uint256 _value + ); + + function transfer(address to, uint256 amount) + external + returns (bool) + { + emit Transfer(address(this), to, amount); + return true; + } + function _wrapEthAndGetWethBalance() internal returns (uint256 balance) { return address(this).balance; } + + function _getWethContract() + internal + view + returns (IEtherToken) + { + return IEtherToken(address(this)); + } } diff --git a/contracts/staking/test/unit_tests/delegator_reward_test.ts b/contracts/staking/test/unit_tests/delegator_reward_test.ts index 5b4a5b0815..9c316a8ebb 100644 --- a/contracts/staking/test/unit_tests/delegator_reward_test.ts +++ b/contracts/staking/test/unit_tests/delegator_reward_test.ts @@ -16,8 +16,7 @@ import { artifacts, TestDelegatorRewardsContract, TestDelegatorRewardsEvents, - TestDelegatorRewardsRecordDepositToEthVaultEventArgs as EthVaultDepositEventArgs, - TestDelegatorRewardsRecordDepositToRewardVaultEventArgs as RewardVaultDepositEventArgs, + TestDelegatorRewardsTransferEventArgs, } from '../../src'; import { @@ -26,7 +25,7 @@ import { toBaseUnitAmount, } from '../utils/number_utils'; -blockchainTests.resets.skip('delegator unit rewards', env => { +blockchainTests.resets('delegator unit rewards', env => { let testContract: TestDelegatorRewardsContract; before(async () => { @@ -123,9 +122,8 @@ blockchainTests.resets.skip('delegator unit rewards', env => { return [_operatorReward, _membersReward]; } - type ResultWithDeposits = T & { - ethVaultDeposit: BigNumber; - rewardVaultDeposit: BigNumber; + type ResultWithTransfers = T & { + delegatorTransfers: BigNumber; }; interface DelegateStakeOpts { @@ -136,7 +134,7 @@ blockchainTests.resets.skip('delegator unit rewards', env => { async function delegateStakeNowAsync( poolId: string, opts?: Partial, - ): Promise> { + ): Promise> { return delegateStakeAsync(poolId, opts, true); } @@ -144,7 +142,7 @@ blockchainTests.resets.skip('delegator unit rewards', env => { poolId: string, opts?: Partial, now?: boolean, - ): Promise> { + ): Promise> { const _opts = { delegator: randomAddress(), stake: getRandomInteger(1, toBaseUnitAmount(10)), @@ -152,11 +150,10 @@ blockchainTests.resets.skip('delegator unit rewards', env => { }; const fn = now ? testContract.delegateStakeNow : testContract.delegateStake; const receipt = await fn.awaitTransactionSuccessAsync(_opts.delegator, poolId, new BigNumber(_opts.stake)); - const [ethVaultDeposit, rewardVaultDeposit] = getDepositsFromLogs(receipt.logs, poolId, _opts.delegator); + const delegatorTransfers = getTransfersFromLogs(receipt.logs, _opts.delegator); return { ..._opts, - ethVaultDeposit, - rewardVaultDeposit, + delegatorTransfers, }; } @@ -164,42 +161,31 @@ blockchainTests.resets.skip('delegator unit rewards', env => { poolId: string, delegator: string, stake?: Numberish, - ): Promise> { + ): Promise> { const _stake = new BigNumber( stake || (await testContract.getStakeDelegatedToPoolByOwner.callAsync(delegator, poolId)).currentEpochBalance, ); const receipt = await testContract.undelegateStake.awaitTransactionSuccessAsync(delegator, poolId, _stake); - const [ethVaultDeposit, rewardVaultDeposit] = getDepositsFromLogs(receipt.logs, poolId, delegator); + const delegatorTransfers = getTransfersFromLogs(receipt.logs, delegator); return { stake: _stake, - ethVaultDeposit, - rewardVaultDeposit, + delegatorTransfers, }; } - function getDepositsFromLogs(logs: LogEntry[], poolId: string, delegator?: string): [BigNumber, BigNumber] { - let ethVaultDeposit = constants.ZERO_AMOUNT; - let rewardVaultDeposit = constants.ZERO_AMOUNT; - const ethVaultDepositArgs = filterLogsToArguments( + function getTransfersFromLogs(logs: LogEntry[], delegator?: string): BigNumber { + let delegatorTransfers = constants.ZERO_AMOUNT; + const transferArgs = filterLogsToArguments( logs, - TestDelegatorRewardsEvents.RecordDepositToEthVault, + TestDelegatorRewardsEvents.Transfer, ); - for (const event of ethVaultDepositArgs) { - if (event.owner === delegator) { - ethVaultDeposit = ethVaultDeposit.plus(event.amount); + for (const event of transferArgs) { + if (event._to === delegator) { + delegatorTransfers = delegatorTransfers.plus(event._value); } } - const rewardVaultDepositArgs = filterLogsToArguments( - logs, - TestDelegatorRewardsEvents.RecordDepositToRewardVault, - ); - if (rewardVaultDepositArgs.length > 0) { - expect(rewardVaultDepositArgs.length).to.eq(1); - expect(rewardVaultDepositArgs[0].poolId).to.eq(poolId); - rewardVaultDeposit = rewardVaultDepositArgs[0].amount; - } - return [ethVaultDeposit, rewardVaultDeposit]; + return delegatorTransfers; } async function advanceEpochAsync(): Promise { @@ -216,16 +202,15 @@ blockchainTests.resets.skip('delegator unit rewards', env => { return testContract.computeRewardBalanceOfOperator.callAsync(poolId); } - async function touchStakeAsync(poolId: string, delegator: string): Promise> { + async function touchStakeAsync(poolId: string, delegator: string): Promise> { return undelegateStakeAsync(poolId, delegator, 0); } - async function finalizePoolAsync(poolId: string): Promise> { + async function finalizePoolAsync(poolId: string): Promise> { const receipt = await testContract.originalFinalizePool.awaitTransactionSuccessAsync(poolId); - const [ethVaultDeposit, rewardVaultDeposit] = getDepositsFromLogs(receipt.logs, poolId); + const delegatorTransfers = getTransfersFromLogs(receipt.logs, poolId); return { - ethVaultDeposit, - rewardVaultDeposit, + delegatorTransfers, }; } @@ -384,8 +369,8 @@ blockchainTests.resets.skip('delegator unit rewards', env => { await advanceEpochAsync(); // epoch 2 // rewards paid for stake in epoch 1. const { membersReward: reward } = await rewardPoolAsync({ poolId, membersStake: stake }); - const { ethVaultDeposit: deposit } = await undelegateStakeAsync(poolId, delegator); - assertRoughlyEquals(deposit, reward); + const { delegatorTransfers: withdrawal } = await undelegateStakeAsync(poolId, delegator); + assertRoughlyEquals(withdrawal, reward); const delegatorReward = await getDelegatorRewardBalanceAsync(poolId, delegator); expect(delegatorReward).to.bignumber.eq(0); }); @@ -397,8 +382,8 @@ blockchainTests.resets.skip('delegator unit rewards', env => { await advanceEpochAsync(); // epoch 2 // rewards paid for stake in epoch 1. const { membersReward: reward } = await rewardPoolAsync({ poolId, membersStake: stake }); - const { ethVaultDeposit: deposit } = await undelegateStakeAsync(poolId, delegator); - assertRoughlyEquals(deposit, reward); + const { delegatorTransfers: withdrawal } = await undelegateStakeAsync(poolId, delegator); + assertRoughlyEquals(withdrawal, reward); await delegateStakeAsync(poolId, { delegator, stake }); const delegatorReward = await getDelegatorRewardBalanceAsync(poolId, delegator); expect(delegatorReward).to.bignumber.eq(0); @@ -448,7 +433,7 @@ blockchainTests.resets.skip('delegator unit rewards', env => { // Pay rewards for epoch 1. const { membersReward: reward1 } = await rewardPoolAsync({ poolId, membersStake: rewardStake }); // add extra stake - const { ethVaultDeposit: deposit } = await delegateStakeAsync(poolId, { delegator, stake: stake2 }); + const { delegatorTransfers: withdrawal } = await delegateStakeAsync(poolId, { delegator, stake: stake2 }); await advanceEpochAsync(); // epoch 3 (stake2 now active) // Pay rewards for epoch 2. await advanceEpochAsync(); // epoch 4 @@ -459,7 +444,7 @@ blockchainTests.resets.skip('delegator unit rewards', env => { computeDelegatorRewards(reward1, stake1, rewardStake), computeDelegatorRewards(reward2, totalStake, rewardStake), ); - assertRoughlyEquals(BigNumber.sum(deposit, delegatorReward), expectedDelegatorReward); + assertRoughlyEquals(BigNumber.sum(withdrawal, delegatorReward), expectedDelegatorReward); }); it('uses old stake for rewards paid in the epoch right after extra stake is added', async () => { @@ -666,16 +651,16 @@ blockchainTests.resets.skip('delegator unit rewards', env => { }); describe('reward transfers', async () => { - it('transfers all rewards to eth vault when touching stake', async () => { + it('transfers all rewards to delegator when touching stake', async () => { const poolId = hexRandom(); const { delegator, stake } = await delegateStakeAsync(poolId); await advanceEpochAsync(); // epoch 1 (stake now active) await advanceEpochAsync(); // epoch 2 // rewards paid for stake in epoch 1 const { membersReward: reward } = await rewardPoolAsync({ poolId, membersStake: stake }); - const { ethVaultDeposit: deposit } = await touchStakeAsync(poolId, delegator); + const { delegatorTransfers: withdrawal } = await touchStakeAsync(poolId, delegator); const finalRewardBalance = await getDelegatorRewardBalanceAsync(poolId, delegator); - expect(deposit).to.bignumber.eq(reward); + expect(withdrawal).to.bignumber.eq(reward); expect(finalRewardBalance).to.bignumber.eq(0); }); @@ -697,7 +682,7 @@ blockchainTests.resets.skip('delegator unit rewards', env => { // touch the stake one last time stakeResults.push(await touchStakeAsync(poolId, delegator)); // Should only see deposits for epoch 2. - const allDeposits = stakeResults.map(r => r.ethVaultDeposit); + const allDeposits = stakeResults.map(r => r.delegatorTransfers); const expectedReward = computeDelegatorRewards(reward, stake, rewardStake); assertRoughlyEquals(BigNumber.sum(...allDeposits), expectedReward); }); @@ -725,7 +710,7 @@ blockchainTests.resets.skip('delegator unit rewards', env => { const { membersReward: reward2 } = await rewardPoolAsync({ poolId, membersStake: rewardStake }); // touch the stake to claim rewards stakeResults.push(await touchStakeAsync(poolId, delegator)); - const allDeposits = stakeResults.map(r => r.ethVaultDeposit); + const allDeposits = stakeResults.map(r => r.delegatorTransfers); const expectedReward = BigNumber.sum( computeDelegatorRewards(reward1, stake, rewardStake), computeDelegatorRewards(reward2, new BigNumber(stake).minus(unstake), rewardStake), @@ -743,11 +728,11 @@ blockchainTests.resets.skip('delegator unit rewards', env => { // rewards paid for stake in epoch 1 const { membersReward: reward } = await rewardPoolAsync({ poolId, membersStake: totalStake }); // delegator A will finalize and collect rewards by touching stake. - const { ethVaultDeposit: depositA } = await touchStakeAsync(poolId, delegatorA); - assertRoughlyEquals(depositA, computeDelegatorRewards(reward, stakeA, totalStake)); + const { delegatorTransfers: withdrawalA } = await touchStakeAsync(poolId, delegatorA); + assertRoughlyEquals(withdrawalA, computeDelegatorRewards(reward, stakeA, totalStake)); // delegator B will collect rewards by touching stake - const { ethVaultDeposit: depositB } = await touchStakeAsync(poolId, delegatorB); - assertRoughlyEquals(depositB, computeDelegatorRewards(reward, stakeB, totalStake)); + const { delegatorTransfers: withdrawalB } = await touchStakeAsync(poolId, delegatorB); + assertRoughlyEquals(withdrawalB, computeDelegatorRewards(reward, stakeB, totalStake)); }); it('delegator B collects correct rewards after delegator A finalizes', async () => { @@ -767,11 +752,11 @@ blockchainTests.resets.skip('delegator unit rewards', env => { }); const totalRewards = BigNumber.sum(prevReward, unfinalizedReward); // delegator A will finalize and collect rewards by touching stake. - const { ethVaultDeposit: depositA } = await touchStakeAsync(poolId, delegatorA); - assertRoughlyEquals(depositA, computeDelegatorRewards(totalRewards, stakeA, totalStake)); + const { delegatorTransfers: withdrawalA } = await touchStakeAsync(poolId, delegatorA); + assertRoughlyEquals(withdrawalA, computeDelegatorRewards(totalRewards, stakeA, totalStake)); // delegator B will collect rewards by touching stake - const { ethVaultDeposit: depositB } = await touchStakeAsync(poolId, delegatorB); - assertRoughlyEquals(depositB, computeDelegatorRewards(totalRewards, stakeB, totalStake)); + const { delegatorTransfers: withdrawalB } = await touchStakeAsync(poolId, delegatorB); + assertRoughlyEquals(withdrawalB, computeDelegatorRewards(totalRewards, stakeB, totalStake)); }); it('delegator A and B collect correct rewards after external finalization', async () => { @@ -793,11 +778,11 @@ blockchainTests.resets.skip('delegator unit rewards', env => { // finalize await finalizePoolAsync(poolId); // delegator A will collect rewards by touching stake. - const { ethVaultDeposit: depositA } = await touchStakeAsync(poolId, delegatorA); - assertRoughlyEquals(depositA, computeDelegatorRewards(totalRewards, stakeA, totalStake)); + const { delegatorTransfers: withdrawalA } = await touchStakeAsync(poolId, delegatorA); + assertRoughlyEquals(withdrawalA, computeDelegatorRewards(totalRewards, stakeA, totalStake)); // delegator B will collect rewards by touching stake - const { ethVaultDeposit: depositB } = await touchStakeAsync(poolId, delegatorB); - assertRoughlyEquals(depositB, computeDelegatorRewards(totalRewards, stakeB, totalStake)); + const { delegatorTransfers: withdrawalB } = await touchStakeAsync(poolId, delegatorB); + assertRoughlyEquals(withdrawalB, computeDelegatorRewards(totalRewards, stakeB, totalStake)); }); }); }); From 156560ae2236778b0884baf4dd48a1b53dc8f2fa Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 23 Sep 2019 10:39:38 -0700 Subject: [PATCH 06/13] Rename overloaded function --- contracts/staking/contracts/src/sys/MixinFinalizer.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/staking/contracts/src/sys/MixinFinalizer.sol b/contracts/staking/contracts/src/sys/MixinFinalizer.sol index f095471778..5ce7eb7bbc 100644 --- a/contracts/staking/contracts/src/sys/MixinFinalizer.sol +++ b/contracts/staking/contracts/src/sys/MixinFinalizer.sol @@ -200,7 +200,7 @@ contract MixinFinalizer is return (0, 0); } IStructs.ActivePool memory pool = _getActivePoolFromEpoch(epoch - 1, poolId); - reward = _getUnfinalizedPoolRewards(pool, unfinalizedState); + reward = _getUnfinalizedPoolRewardsFromState(pool, unfinalizedState); membersStake = pool.membersStake; } @@ -256,7 +256,7 @@ contract MixinFinalizer is /// @param pool The active pool. /// @param state The current state of finalization. /// @return rewards Unfinalized rewards for this pool. - function _getUnfinalizedPoolRewards( + function _getUnfinalizedPoolRewardsFromState( IStructs.ActivePool memory pool, IStructs.UnfinalizedState memory state ) @@ -312,7 +312,7 @@ contract MixinFinalizer is delete _getActivePoolsFromEpoch(epoch.safeSub(1))[poolId]; // Compute the rewards. - uint256 rewards = _getUnfinalizedPoolRewards(pool, state); + uint256 rewards = _getUnfinalizedPoolRewardsFromState(pool, state); // Pay the pool. // Note that we credit at the CURRENT epoch even though these rewards From fd35249de868f3136c5c5c49ead072621d0ce20c Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 23 Sep 2019 14:21:13 -0700 Subject: [PATCH 07/13] Track WETH reserved for rewards --- contracts/staking/contracts/src/immutable/MixinStorage.sol | 3 +++ contracts/staking/contracts/src/stake/MixinStake.sol | 2 ++ .../contracts/src/staking_pools/MixinStakingPoolRewards.sol | 2 ++ contracts/staking/contracts/src/sys/MixinFinalizer.sol | 4 +++- contracts/staking/test/rewards_test.ts | 2 +- 5 files changed, 11 insertions(+), 2 deletions(-) diff --git a/contracts/staking/contracts/src/immutable/MixinStorage.sol b/contracts/staking/contracts/src/immutable/MixinStorage.sol index 7778e35483..ac3a268762 100644 --- a/contracts/staking/contracts/src/immutable/MixinStorage.sol +++ b/contracts/staking/contracts/src/immutable/MixinStorage.sol @@ -146,6 +146,9 @@ contract MixinStorage is /// @dev State for unfinalized rewards. IStructs.UnfinalizedState public unfinalizedState; + /// @dev The WETH balance of this contract that is reserved for pool reward payouts. + uint256 _reservedWethBalance; + /// @dev Adds owner as an authorized address. constructor() public diff --git a/contracts/staking/contracts/src/stake/MixinStake.sol b/contracts/staking/contracts/src/stake/MixinStake.sol index 38c7671f61..7e9d727510 100644 --- a/contracts/staking/contracts/src/stake/MixinStake.sol +++ b/contracts/staking/contracts/src/stake/MixinStake.sol @@ -217,6 +217,7 @@ contract MixinStake is // to. IStructs.StoredBalance memory finalDelegatedStakeToPoolByOwner = _loadSyncedBalance(_delegatedStakeToPoolByOwner[owner][poolId]); + _withdrawAndSyncDelegatorRewards( poolId, owner, @@ -256,6 +257,7 @@ contract MixinStake is // from IStructs.StoredBalance memory finalDelegatedStakeToPoolByOwner = _loadSyncedBalance(_delegatedStakeToPoolByOwner[owner][poolId]); + _withdrawAndSyncDelegatorRewards( poolId, owner, diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index 5837954811..b2c75d5c77 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -204,6 +204,7 @@ contract MixinStakingPoolRewards is if (membersReward > 0) { // Increment the balance of the pool balanceByPoolId[poolId] = balanceByPoolId[poolId].safeAdd(membersReward); + _reservedWethBalance = _reservedWethBalance.safeAdd(membersReward); // Fetch the last epoch at which we stored an entry for this pool; // this is the most up-to-date cumulative rewards for this pool. @@ -297,6 +298,7 @@ contract MixinStakingPoolRewards is // Decrement the balance of the pool balanceByPoolId[poolId] = balanceByPoolId[poolId].safeSub(balance); + _reservedWethBalance = _reservedWethBalance.safeSub(balance); // Withdraw the member's WETH balance _getWethContract().transfer(member, balance); diff --git a/contracts/staking/contracts/src/sys/MixinFinalizer.sol b/contracts/staking/contracts/src/sys/MixinFinalizer.sol index 5ce7eb7bbc..630b84023c 100644 --- a/contracts/staking/contracts/src/sys/MixinFinalizer.sol +++ b/contracts/staking/contracts/src/sys/MixinFinalizer.sol @@ -248,7 +248,9 @@ contract MixinFinalizer is if (ethBalance != 0) { wethContract.deposit.value(ethBalance)(); } - wethBalance = wethContract.balanceOf(address(this)); + wethBalance = wethContract.balanceOf(address(this)) + .safeSub(_reservedWethBalance); + return wethBalance; } diff --git a/contracts/staking/test/rewards_test.ts b/contracts/staking/test/rewards_test.ts index 5179fddafa..19ada9994b 100644 --- a/contracts/staking/test/rewards_test.ts +++ b/contracts/staking/test/rewards_test.ts @@ -12,7 +12,7 @@ import { DelegatorsByPoolId, OperatorByPoolId, StakeInfo, StakeStatus } from './ // tslint:disable:no-unnecessary-type-assertion // tslint:disable:max-file-line-count -blockchainTests.resets.skip('Testing Rewards', env => { +blockchainTests.resets('Testing Rewards', env => { // tokens & addresses let accounts: string[]; let owner: string; From 3965d8f8c6072052ce1312f2609c4618b753951c Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 23 Sep 2019 15:14:21 -0700 Subject: [PATCH 08/13] Separate wrapping ETH and querying available WETH balance --- .../contracts/src/fees/MixinExchangeFees.sol | 2 +- .../contracts/src/sys/MixinFinalizer.sol | 26 ++++++++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index 79abd09277..99745db7a0 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol @@ -134,7 +134,7 @@ contract MixinExchangeFees is returns (uint256 totalBalance) { totalBalance = address(this).balance.safeAdd( - _getWethContract().balanceOf(address(this)) + _getAvailableWethBalance() ); return totalBalance; } diff --git a/contracts/staking/contracts/src/sys/MixinFinalizer.sol b/contracts/staking/contracts/src/sys/MixinFinalizer.sol index 630b84023c..a8a2646de8 100644 --- a/contracts/staking/contracts/src/sys/MixinFinalizer.sol +++ b/contracts/staking/contracts/src/sys/MixinFinalizer.sol @@ -76,8 +76,11 @@ contract MixinFinalizer is ); } + // Convert all ETH to WETH + _wrapEth(); + // Set up unfinalized state. - state.rewardsAvailable = _wrapEthAndGetWethBalance(); + state.rewardsAvailable = _getAvailableWethBalance(); state.poolsRemaining = poolsRemaining = numActivePoolsThisEpoch; state.totalFeesCollected = totalFeesCollectedThisEpoch; state.totalWeightedStake = totalWeightedStakeThisEpoch; @@ -236,19 +239,24 @@ contract MixinFinalizer is return activePools; } - /// @dev Converts the entire ETH balance of the contract into WETH and - /// returns the total WETH balance of this contract. - /// @return The WETH balance of this contract. - function _wrapEthAndGetWethBalance() + /// @dev Converts the entire ETH balance of this contract into WETH. + function _wrapEth() internal - returns (uint256 wethBalance) { - IEtherToken wethContract = _getWethContract(); uint256 ethBalance = address(this).balance; if (ethBalance != 0) { - wethContract.deposit.value(ethBalance)(); + _getWethContract().deposit.value(ethBalance)(); } - wethBalance = wethContract.balanceOf(address(this)) + } + + /// @dev Returns the WETH balance of this contract, minus + /// any WETH that has already been reserved for rewards. + function _getAvailableWethBalance() + internal + view + returns (uint256 wethBalance) + { + wethBalance = _getWethContract().balanceOf(address(this)) .safeSub(_reservedWethBalance); return wethBalance; From 62663ed6d2875b8deef6bdeda38569930dfb9e22 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 23 Sep 2019 15:26:03 -0700 Subject: [PATCH 09/13] Fix rewards tests --- contracts/staking/test/actors/finalizer_actor.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contracts/staking/test/actors/finalizer_actor.ts b/contracts/staking/test/actors/finalizer_actor.ts index bbd20129e9..0db7b504bf 100644 --- a/contracts/staking/test/actors/finalizer_actor.ts +++ b/contracts/staking/test/actors/finalizer_actor.ts @@ -49,7 +49,7 @@ export class FinalizerActor extends BaseActor { const [ expectedOperatorBalanceByPoolId, expectedRewardVaultBalanceByPoolId, - ] = await this._computeExpectedRewardVaultBalanceAsyncByPoolIdAsync( + ] = this._computeExpectedRewardVaultBalanceAsyncByPoolId( rewardByPoolId, operatorBalanceByPoolId, rewardVaultBalanceByPoolId, @@ -159,13 +159,13 @@ export class FinalizerActor extends BaseActor { return delegatorBalancesByPoolId; } - private async _computeExpectedRewardVaultBalanceAsyncByPoolIdAsync( + private _computeExpectedRewardVaultBalanceAsyncByPoolId( rewardByPoolId: RewardByPoolId, operatorBalanceByPoolId: OperatorBalanceByPoolId, rewardVaultBalanceByPoolId: RewardVaultBalanceByPoolId, delegatorStakesByPoolId: DelegatorBalancesByPoolId, operatorShareByPoolId: OperatorShareByPoolId, - ): Promise<[RewardVaultBalanceByPoolId, OperatorBalanceByPoolId]> { + ): [RewardVaultBalanceByPoolId, OperatorBalanceByPoolId] { const expectedOperatorBalanceByPoolId = _.cloneDeep(operatorBalanceByPoolId); const expectedRewardVaultBalanceByPoolId = _.cloneDeep(rewardVaultBalanceByPoolId); for (const poolId of Object.keys(rewardByPoolId)) { @@ -173,7 +173,7 @@ export class FinalizerActor extends BaseActor { [ expectedOperatorBalanceByPoolId[poolId], expectedRewardVaultBalanceByPoolId[poolId], - ] = await this._computeExpectedRewardVaultBalanceAsync( + ] = this._computeExpectedRewardVaultBalance( poolId, rewardByPoolId[poolId], expectedOperatorBalanceByPoolId[poolId], @@ -185,14 +185,14 @@ export class FinalizerActor extends BaseActor { return [expectedOperatorBalanceByPoolId, expectedRewardVaultBalanceByPoolId]; } - private async _computeExpectedRewardVaultBalanceAsync( + private _computeExpectedRewardVaultBalance( poolId: string, reward: BigNumber, operatorBalance: BigNumber, rewardVaultBalance: BigNumber, stakeBalances: BalanceByOwner, operatorShare: BigNumber, - ): Promise<[BigNumber, BigNumber]> { + ): [BigNumber, BigNumber] { const totalStakeDelegatedToPool = BigNumber.sum(...Object.values(stakeBalances)); const stakeDelegatedToPoolByOperator = stakeBalances[this._operatorByPoolId[poolId]]; const membersStakeDelegatedToPool = totalStakeDelegatedToPool.minus(stakeDelegatedToPoolByOperator); From 5ce988957f6fb35d76aed0dc91fd9c2e55fb8d94 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 23 Sep 2019 15:55:28 -0700 Subject: [PATCH 10/13] Fix tests --- .../staking/contracts/src/fees/MixinExchangeFees.sol | 5 +++-- contracts/staking/contracts/test/TestStakingNoWETH.sol | 9 +++++++-- contracts/staking/test/actors/finalizer_actor.ts | 4 +--- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index 99745db7a0..0df7cb9196 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol @@ -126,9 +126,10 @@ contract MixinExchangeFees is activePoolsThisEpoch[poolId] = pool; } - /// @dev Returns the total balance of this contract, including WETH. + /// @dev Returns the total balance of this contract, including WETH, + /// minus any WETH that has been reserved for rewards. /// @return totalBalance Total balance. - function getTotalBalance() + function getAvailableBalance() external view returns (uint256 totalBalance) diff --git a/contracts/staking/contracts/test/TestStakingNoWETH.sol b/contracts/staking/contracts/test/TestStakingNoWETH.sol index 135c68b00d..1fabdcc187 100644 --- a/contracts/staking/contracts/test/TestStakingNoWETH.sol +++ b/contracts/staking/contracts/test/TestStakingNoWETH.sol @@ -42,9 +42,14 @@ contract TestStakingNoWETH is return true; } - function _wrapEthAndGetWethBalance() + function _wrapEth() internal - returns (uint256 balance) + {} + + function _getAvailableWethBalance() + internal + view + returns (uint256) { return address(this).balance; } diff --git a/contracts/staking/test/actors/finalizer_actor.ts b/contracts/staking/test/actors/finalizer_actor.ts index 0db7b504bf..c3fea29841 100644 --- a/contracts/staking/test/actors/finalizer_actor.ts +++ b/contracts/staking/test/actors/finalizer_actor.ts @@ -241,9 +241,7 @@ export class FinalizerActor extends BaseActor { this._stakingApiWrapper.stakingContract.getActiveStakingPoolThisEpoch.callAsync(poolId), ), ); - const totalRewards = await this._stakingApiWrapper.utils.getEthAndWethBalanceOfAsync( - this._stakingApiWrapper.stakingContract.address, - ); + const totalRewards = await this._stakingApiWrapper.stakingContract.getAvailableBalance.callAsync(); const totalFeesCollected = BigNumber.sum(...activePools.map(p => p.feesCollected)); const totalWeightedStake = BigNumber.sum(...activePools.map(p => p.weightedStake)); if (totalRewards.eq(0) || totalFeesCollected.eq(0) || totalWeightedStake.eq(0)) { From e9f0f4af86f5c17411a51b851f954b42a8b0d3f9 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 23 Sep 2019 16:38:22 -0700 Subject: [PATCH 11/13] Rename functions and variables for clarity --- .../contracts/src/fees/MixinExchangeFees.sol | 2 +- .../contracts/src/immutable/MixinStorage.sol | 4 +-- .../staking_pools/MixinStakingPoolRewards.sol | 26 ++++++++++++++++--- .../contracts/src/sys/MixinFinalizer.sol | 2 +- .../staking/test/actors/finalizer_actor.ts | 4 +-- contracts/staking/test/rewards_test.ts | 2 +- 6 files changed, 29 insertions(+), 11 deletions(-) diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index 0df7cb9196..67f3306e37 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol @@ -129,7 +129,7 @@ contract MixinExchangeFees is /// @dev Returns the total balance of this contract, including WETH, /// minus any WETH that has been reserved for rewards. /// @return totalBalance Total balance. - function getAvailableBalance() + function getAvailableRewardsBalance() external view returns (uint256 totalBalance) diff --git a/contracts/staking/contracts/src/immutable/MixinStorage.sol b/contracts/staking/contracts/src/immutable/MixinStorage.sol index ac3a268762..05446258b2 100644 --- a/contracts/staking/contracts/src/immutable/MixinStorage.sol +++ b/contracts/staking/contracts/src/immutable/MixinStorage.sol @@ -83,7 +83,7 @@ contract MixinStorage is mapping (bytes32 => IStructs.Pool) internal _poolById; // mapping from PoolId to balance of members - mapping (bytes32 => uint256) public balanceByPoolId; + mapping (bytes32 => uint256) public rewardsByPoolId; // current epoch uint256 public currentEpoch = INITIAL_EPOCH; @@ -147,7 +147,7 @@ contract MixinStorage is IStructs.UnfinalizedState public unfinalizedState; /// @dev The WETH balance of this contract that is reserved for pool reward payouts. - uint256 _reservedWethBalance; + uint256 _wethReservedForPoolRewards; /// @dev Adds owner as an authorized address. constructor() diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index b2c75d5c77..8f5861ef31 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -203,8 +203,7 @@ contract MixinStakingPoolRewards is if (membersReward > 0) { // Increment the balance of the pool - balanceByPoolId[poolId] = balanceByPoolId[poolId].safeAdd(membersReward); - _reservedWethBalance = _reservedWethBalance.safeAdd(membersReward); + _incrementPoolRewards(poolId, membersReward); // Fetch the last epoch at which we stored an entry for this pool; // this is the most up-to-date cumulative rewards for this pool. @@ -297,8 +296,7 @@ contract MixinStakingPoolRewards is } // Decrement the balance of the pool - balanceByPoolId[poolId] = balanceByPoolId[poolId].safeSub(balance); - _reservedWethBalance = _reservedWethBalance.safeSub(balance); + _decrementPoolRewards(poolId, balance); // Withdraw the member's WETH balance _getWethContract().transfer(member, balance); @@ -432,4 +430,24 @@ contract MixinStakingPoolRewards is ); } } + + /// @dev Increments rewards for a pool. + /// @param poolId Unique id of pool. + /// @param amount Amount to increment rewards by. + function _incrementPoolRewards(bytes32 poolId, uint256 amount) + private + { + rewardsByPoolId[poolId] = rewardsByPoolId[poolId].safeAdd(amount); + _wethReservedForPoolRewards = _wethReservedForPoolRewards.safeAdd(amount); + } + + /// @dev Decrements rewards for a pool. + /// @param poolId Unique id of pool. + /// @param amount Amount to decrement rewards by. + function _decrementPoolRewards(bytes32 poolId, uint256 amount) + private + { + rewardsByPoolId[poolId] = rewardsByPoolId[poolId].safeSub(amount); + _wethReservedForPoolRewards = _wethReservedForPoolRewards.safeSub(amount); + } } diff --git a/contracts/staking/contracts/src/sys/MixinFinalizer.sol b/contracts/staking/contracts/src/sys/MixinFinalizer.sol index a8a2646de8..368423dfdf 100644 --- a/contracts/staking/contracts/src/sys/MixinFinalizer.sol +++ b/contracts/staking/contracts/src/sys/MixinFinalizer.sol @@ -257,7 +257,7 @@ contract MixinFinalizer is returns (uint256 wethBalance) { wethBalance = _getWethContract().balanceOf(address(this)) - .safeSub(_reservedWethBalance); + .safeSub(_wethReservedForPoolRewards); return wethBalance; } diff --git a/contracts/staking/test/actors/finalizer_actor.ts b/contracts/staking/test/actors/finalizer_actor.ts index c3fea29841..41f4400552 100644 --- a/contracts/staking/test/actors/finalizer_actor.ts +++ b/contracts/staking/test/actors/finalizer_actor.ts @@ -230,7 +230,7 @@ export class FinalizerActor extends BaseActor { for (const poolId of poolIds) { rewardVaultBalanceByPoolId[ poolId - ] = await this._stakingApiWrapper.stakingContract.balanceByPoolId.callAsync(poolId); + ] = await this._stakingApiWrapper.stakingContract.rewardsByPoolId.callAsync(poolId); } return rewardVaultBalanceByPoolId; } @@ -241,7 +241,7 @@ export class FinalizerActor extends BaseActor { this._stakingApiWrapper.stakingContract.getActiveStakingPoolThisEpoch.callAsync(poolId), ), ); - const totalRewards = await this._stakingApiWrapper.stakingContract.getAvailableBalance.callAsync(); + const totalRewards = await this._stakingApiWrapper.stakingContract.getAvailableRewardsBalance.callAsync(); const totalFeesCollected = BigNumber.sum(...activePools.map(p => p.feesCollected)); const totalWeightedStake = BigNumber.sum(...activePools.map(p => p.weightedStake)); if (totalRewards.eq(0) || totalFeesCollected.eq(0) || totalWeightedStake.eq(0)) { diff --git a/contracts/staking/test/rewards_test.ts b/contracts/staking/test/rewards_test.ts index 19ada9994b..76d7c39a4a 100644 --- a/contracts/staking/test/rewards_test.ts +++ b/contracts/staking/test/rewards_test.ts @@ -131,7 +131,7 @@ blockchainTests.resets('Testing Rewards', env => { // operator stakingApiWrapper.wethContract.balanceOf.callAsync(poolOperator.getOwner()), // undivided balance in reward pool - stakingApiWrapper.stakingContract.balanceByPoolId.callAsync(poolId), + stakingApiWrapper.stakingContract.rewardsByPoolId.callAsync(poolId), ]); expect(finalEndBalancesAsArray[0], 'stakerRewardBalance_1').to.be.bignumber.equal( expectedEndBalances.stakerRewardBalance_1, From 5bbd57d236094748ebc9ac64911a8b6f06cb66fb Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 23 Sep 2019 17:28:03 -0700 Subject: [PATCH 12/13] Delete unused rich reverts --- .../src/libs/LibStakingRichErrors.sol | 28 ------------------- 1 file changed, 28 deletions(-) diff --git a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol index 7864f88789..5b6954b92e 100644 --- a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol +++ b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol @@ -119,14 +119,6 @@ library LibStakingRichErrors { bytes4 internal constant POOL_EXISTENCE_ERROR_SELECTOR = 0x9ae94f01; - // bytes4(keccak256("EthVaultNotSetError()")) - bytes4 internal constant ETH_VAULT_NOT_SET_ERROR_SELECTOR = - 0xa067f596; - - // bytes4(keccak256("RewardVaultNotSetError()")) - bytes4 internal constant REWARD_VAULT_NOT_SET_ERROR_SELECTOR = - 0xe6976d70; - // bytes4(keccak256("InvalidStakeStatusError(uint256)")) bytes4 internal constant INVALID_STAKE_STATUS_ERROR_SELECTOR = 0xb7161acd; @@ -380,26 +372,6 @@ library LibStakingRichErrors { ); } - function EthVaultNotSetError() - internal - pure - returns (bytes memory) - { - return abi.encodeWithSelector( - ETH_VAULT_NOT_SET_ERROR_SELECTOR - ); - } - - function RewardVaultNotSetError() - internal - pure - returns (bytes memory) - { - return abi.encodeWithSelector( - REWARD_VAULT_NOT_SET_ERROR_SELECTOR - ); - } - function InvalidProtocolFeePaymentError( ProtocolFeePaymentErrorCodes errorCode, uint256 expectedProtocolFeePaid, From abb2b46ed33e1d7fa450ebb5d334db863d42f8bb Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 23 Sep 2019 17:37:04 -0700 Subject: [PATCH 13/13] Remove vaults from variable names and comments --- .../staking_pools/MixinStakingPoolRewards.sol | 9 +-- .../contracts/src/sys/MixinAbstract.sol | 4 +- .../contracts/src/sys/MixinFinalizer.sol | 4 +- .../contracts/test/TestDelegatorRewards.sol | 6 +- .../staking/test/actors/finalizer_actor.ts | 56 +++++++++---------- contracts/staking/test/rewards_test.ts | 30 +++++----- contracts/staking/test/utils/types.ts | 2 +- 7 files changed, 53 insertions(+), 58 deletions(-) diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index 8f5861ef31..ab66124779 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -69,7 +69,6 @@ contract MixinStakingPoolRewards is } /// @dev Computes the reward balance in ETH of the operator of a pool. - /// This does not include the balance in the ETH vault. /// @param poolId Unique id of pool. /// @return totalReward Balance in ETH. function computeRewardBalanceOfOperator(bytes32 poolId) @@ -77,7 +76,7 @@ contract MixinStakingPoolRewards is view returns (uint256 reward) { - // Because operator rewards are immediately sent to the ETH vault + // Because operator rewards are immediately withdrawn as WETH // on finalization, the only factor in this function are unfinalized // rewards. IStructs.Pool memory pool = _poolById[poolId]; @@ -95,7 +94,6 @@ contract MixinStakingPoolRewards is } /// @dev Computes the reward balance in ETH of a specific member of a pool. - /// This does not include the balance in the ETH vault. /// @param poolId Unique id of pool. /// @param member The member of the pool. /// @return totalReward Balance in ETH. @@ -125,8 +123,7 @@ contract MixinStakingPoolRewards is } /// @dev Syncs rewards for a delegator. This includes transferring rewards - /// from the Reward Vault to the Eth Vault, and adding/removing - /// dependencies on cumulative rewards. + /// withdrawing rewards and adding/removing dependencies on cumulative rewards. /// @param poolId Unique id of pool. /// @param member of the pool. /// @param initialDelegatedStakeToPoolByOwner The member's delegated @@ -141,7 +138,7 @@ contract MixinStakingPoolRewards is ) internal { - // Transfer any rewards from the transient pool vault to the eth vault; + // Withdraw any rewards from. // this must be done before we can modify the owner's portion of the // delegator pool. _finalizePoolAndWithdrawDelegatorRewards( diff --git a/contracts/staking/contracts/src/sys/MixinAbstract.sol b/contracts/staking/contracts/src/sys/MixinAbstract.sol index b8801917d6..3c42267344 100644 --- a/contracts/staking/contracts/src/sys/MixinAbstract.sol +++ b/contracts/staking/contracts/src/sys/MixinAbstract.sol @@ -27,8 +27,8 @@ import "../interfaces/IStructs.sol"; contract MixinAbstract { /// @dev Instantly finalizes a single pool that was active in the previous - /// epoch, crediting it rewards and sending those rewards to the reward - /// and eth vault. This can be called by internal functions that need + /// epoch, crediting it rewards for members and withdrawing operator's + /// rewards as WETH. This can be called by internal functions that need /// to finalize a pool immediately. Does nothing if the pool is already /// finalized or was not active in the previous epoch. /// @param poolId The pool ID to finalize. diff --git a/contracts/staking/contracts/src/sys/MixinFinalizer.sol b/contracts/staking/contracts/src/sys/MixinFinalizer.sol index 368423dfdf..8d511f14e3 100644 --- a/contracts/staking/contracts/src/sys/MixinFinalizer.sol +++ b/contracts/staking/contracts/src/sys/MixinFinalizer.sol @@ -111,8 +111,8 @@ contract MixinFinalizer is } /// @dev Instantly finalizes a single pool that was active in the previous - /// epoch, crediting it rewards and sending those rewards to the reward - /// and eth vault. This can be called by internal functions that need + /// epoch, crediting it rewards for members and withdrawing operator's + /// rewards as WETH. This can be called by internal functions that need /// to finalize a pool immediately. Does nothing if the pool is already /// finalized or was not active in the previous epoch. /// @param poolId The pool ID to finalize. diff --git a/contracts/staking/contracts/test/TestDelegatorRewards.sol b/contracts/staking/contracts/test/TestDelegatorRewards.sol index eb19e54f21..34bfa30f03 100644 --- a/contracts/staking/contracts/test/TestDelegatorRewards.sol +++ b/contracts/staking/contracts/test/TestDelegatorRewards.sol @@ -106,7 +106,7 @@ contract TestDelegatorRewards is /// @dev Create and delegate stake that is active in the current epoch. /// Only used to test purportedly unreachable states. - /// Also withdraws pending rewards to the eth vault. + /// Also withdraws pending rewards. function delegateStakeNow( address delegator, bytes32 poolId, @@ -131,7 +131,7 @@ contract TestDelegatorRewards is /// @dev Create and delegate stake that will occur in the next epoch /// (normal behavior). - /// Also withdraws pending rewards to the eth vault. + /// Also withdraws pending rewards. function delegateStake( address delegator, bytes32 poolId, @@ -158,7 +158,7 @@ contract TestDelegatorRewards is /// @dev Clear stake that will occur in the next epoch /// (normal behavior). - /// Also withdraws pending rewards to the eth vault. + /// Also withdraws pending rewards. function undelegateStake( address delegator, bytes32 poolId, diff --git a/contracts/staking/test/actors/finalizer_actor.ts b/contracts/staking/test/actors/finalizer_actor.ts index 41f4400552..744b4802b2 100644 --- a/contracts/staking/test/actors/finalizer_actor.ts +++ b/contracts/staking/test/actors/finalizer_actor.ts @@ -10,8 +10,8 @@ import { OperatorBalanceByPoolId, OperatorByPoolId, OperatorShareByPoolId, + RewardBalanceByPoolId, RewardByPoolId, - RewardVaultBalanceByPoolId, } from '../utils/types'; import { BaseActor } from './base_actor'; @@ -40,7 +40,7 @@ export class FinalizerActor extends BaseActor { public async finalizeAsync(): Promise { // cache initial info and balances const operatorShareByPoolId = await this._getOperatorShareByPoolIdAsync(this._poolIds); - const rewardVaultBalanceByPoolId = await this._getRewardVaultBalanceByPoolIdAsync(this._poolIds); + const rewardBalanceByPoolId = await this._getRewardBalanceByPoolIdAsync(this._poolIds); const delegatorBalancesByPoolId = await this._getDelegatorBalancesByPoolIdAsync(this._delegatorsByPoolId); const delegatorStakesByPoolId = await this._getDelegatorStakesByPoolIdAsync(this._delegatorsByPoolId); const operatorBalanceByPoolId = await this._getOperatorBalanceByPoolIdAsync(this._operatorByPoolId); @@ -48,11 +48,11 @@ export class FinalizerActor extends BaseActor { // compute expected changes const [ expectedOperatorBalanceByPoolId, - expectedRewardVaultBalanceByPoolId, - ] = this._computeExpectedRewardVaultBalanceAsyncByPoolId( + expectedRewardBalanceByPoolId, + ] = this._computeExpectedRewardBalanceByPoolId( rewardByPoolId, operatorBalanceByPoolId, - rewardVaultBalanceByPoolId, + rewardBalanceByPoolId, delegatorStakesByPoolId, operatorShareByPoolId, ); @@ -65,19 +65,19 @@ export class FinalizerActor extends BaseActor { ); // finalize await this._stakingApiWrapper.utils.skipToNextEpochAndFinalizeAsync(); - // assert reward vault changes - const finalRewardVaultBalanceByPoolId = await this._getRewardVaultBalanceByPoolIdAsync(this._poolIds); - expect(finalRewardVaultBalanceByPoolId, 'final pool balances in reward vault').to.be.deep.equal( - expectedRewardVaultBalanceByPoolId, + // assert reward changes + const finalRewardBalanceByPoolId = await this._getRewardBalanceByPoolIdAsync(this._poolIds); + expect(finalRewardBalanceByPoolId, 'final pool reward balances').to.be.deep.equal( + expectedRewardBalanceByPoolId, ); // assert delegator balances const finalDelegatorBalancesByPoolId = await this._getDelegatorBalancesByPoolIdAsync(this._delegatorsByPoolId); - expect(finalDelegatorBalancesByPoolId, 'final delegator balances in reward vault').to.be.deep.equal( + expect(finalDelegatorBalancesByPoolId, 'final delegator reward balances').to.be.deep.equal( expectedDelegatorBalancesByPoolId, ); // assert operator balances const finalOperatorBalanceByPoolId = await this._getOperatorBalanceByPoolIdAsync(this._operatorByPoolId); - expect(finalOperatorBalanceByPoolId, 'final operator balances in eth vault').to.be.deep.equal( + expect(finalOperatorBalanceByPoolId, 'final operator weth balance').to.be.deep.equal( expectedOperatorBalanceByPoolId, ); } @@ -159,37 +159,37 @@ export class FinalizerActor extends BaseActor { return delegatorBalancesByPoolId; } - private _computeExpectedRewardVaultBalanceAsyncByPoolId( + private _computeExpectedRewardBalanceByPoolId( rewardByPoolId: RewardByPoolId, operatorBalanceByPoolId: OperatorBalanceByPoolId, - rewardVaultBalanceByPoolId: RewardVaultBalanceByPoolId, + rewardBalanceByPoolId: RewardBalanceByPoolId, delegatorStakesByPoolId: DelegatorBalancesByPoolId, operatorShareByPoolId: OperatorShareByPoolId, - ): [RewardVaultBalanceByPoolId, OperatorBalanceByPoolId] { + ): [RewardBalanceByPoolId, OperatorBalanceByPoolId] { const expectedOperatorBalanceByPoolId = _.cloneDeep(operatorBalanceByPoolId); - const expectedRewardVaultBalanceByPoolId = _.cloneDeep(rewardVaultBalanceByPoolId); + const expectedRewardBalanceByPoolId = _.cloneDeep(rewardBalanceByPoolId); for (const poolId of Object.keys(rewardByPoolId)) { const operatorShare = operatorShareByPoolId[poolId]; [ expectedOperatorBalanceByPoolId[poolId], - expectedRewardVaultBalanceByPoolId[poolId], - ] = this._computeExpectedRewardVaultBalance( + expectedRewardBalanceByPoolId[poolId], + ] = this._computeExpectedRewardBalance( poolId, rewardByPoolId[poolId], expectedOperatorBalanceByPoolId[poolId], - expectedRewardVaultBalanceByPoolId[poolId], + expectedRewardBalanceByPoolId[poolId], delegatorStakesByPoolId[poolId], operatorShare, ); } - return [expectedOperatorBalanceByPoolId, expectedRewardVaultBalanceByPoolId]; + return [expectedOperatorBalanceByPoolId, expectedRewardBalanceByPoolId]; } - private _computeExpectedRewardVaultBalance( + private _computeExpectedRewardBalance( poolId: string, reward: BigNumber, operatorBalance: BigNumber, - rewardVaultBalance: BigNumber, + rewardBalance: BigNumber, stakeBalances: BalanceByOwner, operatorShare: BigNumber, ): [BigNumber, BigNumber] { @@ -200,7 +200,7 @@ export class FinalizerActor extends BaseActor { ? reward : reward.times(operatorShare).dividedToIntegerBy(PPM_100_PERCENT); const membersPortion = reward.minus(operatorPortion); - return [operatorBalance.plus(operatorPortion), rewardVaultBalance.plus(membersPortion)]; + return [operatorBalance.plus(operatorPortion), rewardBalance.plus(membersPortion)]; } private async _getOperatorBalanceByPoolIdAsync( @@ -225,14 +225,14 @@ export class FinalizerActor extends BaseActor { return operatorShareByPoolId; } - private async _getRewardVaultBalanceByPoolIdAsync(poolIds: string[]): Promise { - const rewardVaultBalanceByPoolId: RewardVaultBalanceByPoolId = {}; + private async _getRewardBalanceByPoolIdAsync(poolIds: string[]): Promise { + const rewardBalanceByPoolId: RewardBalanceByPoolId = {}; for (const poolId of poolIds) { - rewardVaultBalanceByPoolId[ - poolId - ] = await this._stakingApiWrapper.stakingContract.rewardsByPoolId.callAsync(poolId); + rewardBalanceByPoolId[poolId] = await this._stakingApiWrapper.stakingContract.rewardsByPoolId.callAsync( + poolId, + ); } - return rewardVaultBalanceByPoolId; + return rewardBalanceByPoolId; } private async _getRewardByPoolIdAsync(poolIds: string[]): Promise { diff --git a/contracts/staking/test/rewards_test.ts b/contracts/staking/test/rewards_test.ts index 76d7c39a4a..57ac5cc2c8 100644 --- a/contracts/staking/test/rewards_test.ts +++ b/contracts/staking/test/rewards_test.ts @@ -344,7 +344,7 @@ blockchainTests.resets('Testing Rewards', env => { membersRewardBalance: rewardForOnlyFirstDelegator.plus(totalSharedRewards), }); }); - it('Should send existing rewards from reward vault to eth vault correctly when undelegating stake', async () => { + it('Should withdraw existing rewards when undelegating stake', async () => { const stakeAmount = toBaseUnitAmount(4); // first staker delegates (epoch 0) await stakers[0].stakeWithPoolAsync(poolId, stakeAmount); @@ -353,7 +353,7 @@ blockchainTests.resets('Testing Rewards', env => { // earn reward const reward = toBaseUnitAmount(10); await payProtocolFeeAndFinalize(reward); - // undelegate (moves delegator's from the transient reward vault into the eth vault) + // undelegate (withdraws delegator's rewards) await stakers[0].moveStakeAsync( new StakeInfo(StakeStatus.Delegated, poolId), new StakeInfo(StakeStatus.Active), @@ -365,7 +365,7 @@ blockchainTests.resets('Testing Rewards', env => { stakerWethBalance_1: reward, }); }); - it('Should send existing rewards from reward vault to eth vault correctly when delegating more stake', async () => { + it('Should withdraw existing rewards correctly when delegating more stake', async () => { const stakeAmount = toBaseUnitAmount(4); // first staker delegates (epoch 0) await stakers[0].stakeWithPoolAsync(poolId, stakeAmount); @@ -560,7 +560,7 @@ blockchainTests.resets('Testing Rewards', env => { poolRewardBalance: constants.ZERO_AMOUNT, }); }); - it('Should withdraw delegator rewards to eth vault when calling `syncDelegatorRewards`', async () => { + it('Should withdraw delegator rewards when calling `withdrawDelegatorRewards`', async () => { // first staker delegates (epoch 0) const rewardForDelegator = toBaseUnitAmount(10); const stakeAmount = toBaseUnitAmount(4); @@ -602,7 +602,7 @@ blockchainTests.resets('Testing Rewards', env => { // finalize const reward = toBaseUnitAmount(10); await payProtocolFeeAndFinalize(reward); - // Sync rewards to move the rewards into the EthVault. + // withdraw rewards await staker.withdrawDelegatorRewardsAsync(poolId); await validateEndBalances({ stakerRewardBalance_1: toBaseUnitAmount(0), @@ -622,7 +622,7 @@ blockchainTests.resets('Testing Rewards', env => { // finalize const reward = toBaseUnitAmount(10); await payProtocolFeeAndFinalize(reward); - // Sync rewards to move rewards from RewardVault into the EthVault. + // withdraw rewards for (const [staker] of _.reverse(stakersAndStake)) { await staker.withdrawDelegatorRewardsAsync(poolId); } @@ -658,25 +658,23 @@ blockchainTests.resets('Testing Rewards', env => { poolRewardBalance: reward, membersRewardBalance: reward, }); - // First staker will sync rewards to get rewards transferred to EthVault. + // First staker will withdraw rewards. const sneakyStaker = stakers[0]; - const sneakyStakerExpectedEthVaultBalance = expectedStakerRewards[0]; + const sneakyStakerExpectedWethBalance = expectedStakerRewards[0]; await sneakyStaker.withdrawDelegatorRewardsAsync(poolId); // Should have been credited the correct amount of rewards. - let sneakyStakerEthVaultBalance = await stakingApiWrapper.wethContract.balanceOf.callAsync( + let sneakyStakerWethBalance = await stakingApiWrapper.wethContract.balanceOf.callAsync( sneakyStaker.getOwner(), ); - expect(sneakyStakerEthVaultBalance, 'EthVault balance after first undelegate').to.bignumber.eq( - sneakyStakerExpectedEthVaultBalance, + expect(sneakyStakerWethBalance, 'WETH balance after first undelegate').to.bignumber.eq( + sneakyStakerExpectedWethBalance, ); // Now he'll try to do it again to see if he gets credited twice. await sneakyStaker.withdrawDelegatorRewardsAsync(poolId); /// The total amount credited should remain the same. - sneakyStakerEthVaultBalance = await stakingApiWrapper.wethContract.balanceOf.callAsync( - sneakyStaker.getOwner(), - ); - expect(sneakyStakerEthVaultBalance, 'EthVault balance after second undelegate').to.bignumber.eq( - sneakyStakerExpectedEthVaultBalance, + sneakyStakerWethBalance = await stakingApiWrapper.wethContract.balanceOf.callAsync(sneakyStaker.getOwner()); + expect(sneakyStakerWethBalance, 'WETH balance after second undelegate').to.bignumber.eq( + sneakyStakerExpectedWethBalance, ); }); }); diff --git a/contracts/staking/test/utils/types.ts b/contracts/staking/test/utils/types.ts index 86e1af75c0..c430a13e96 100644 --- a/contracts/staking/test/utils/types.ts +++ b/contracts/staking/test/utils/types.ts @@ -101,7 +101,7 @@ export interface StakeBalances { totalDelegatedStakeByPool: StakeBalanceByPool; } -export interface RewardVaultBalanceByPoolId { +export interface RewardBalanceByPoolId { [key: string]: BigNumber; }