diff --git a/contracts/integrations/package.json b/contracts/integrations/package.json index ec3050c0e7..f96a82547b 100644 --- a/contracts/integrations/package.json +++ b/contracts/integrations/package.json @@ -80,7 +80,6 @@ "@0x/contracts-exchange": "^2.2.0-beta.0", "@0x/contracts-exchange-forwarder": "^3.1.0-beta.0", "@0x/contracts-exchange-libs": "^3.1.0-beta.0", - "@0x/contracts-extensions": "^4.1.0-beta.0", "@0x/contracts-multisig": "^3.2.0-beta.0", "@0x/contracts-staking": "^1.1.0-beta.0", "@0x/contracts-test-utils": "^3.2.0-beta.0", diff --git a/contracts/staking/contracts/src/ZrxVault.sol b/contracts/staking/contracts/src/ZrxVault.sol index 529a936f64..12b5462f6d 100644 --- a/contracts/staking/contracts/src/ZrxVault.sol +++ b/contracts/staking/contracts/src/ZrxVault.sol @@ -35,7 +35,7 @@ contract ZrxVault is using LibSafeMath for uint256; // Address of staking proxy contract - address payable public stakingProxyAddress; + address public stakingProxyAddress; // True iff vault has been set to Catastrophic Failure Mode bool public isInCatastrophicFailure; @@ -91,7 +91,7 @@ contract ZrxVault is /// @dev Sets the address of the StakingProxy contract. /// Note that only the contract owner can call this function. /// @param _stakingProxyAddress Address of Staking proxy contract. - function setStakingProxy(address payable _stakingProxyAddress) + function setStakingProxy(address _stakingProxyAddress) external onlyAuthorized { diff --git a/contracts/staking/contracts/src/interfaces/IStructs.sol b/contracts/staking/contracts/src/interfaces/IStructs.sol index 3c5ca8ecc4..3b889f2f7b 100644 --- a/contracts/staking/contracts/src/interfaces/IStructs.sol +++ b/contracts/staking/contracts/src/interfaces/IStructs.sol @@ -94,12 +94,10 @@ interface IStructs { } /// @dev Holds the metadata for a staking pool. - /// @param initialized True iff the balance struct is initialized. /// @param operator of the pool. /// @param operatorShare Fraction of the total balance owned by the operator, in ppm. struct Pool { - bool initialized; - address payable operator; + address operator; uint32 operatorShare; } } diff --git a/contracts/staking/contracts/src/interfaces/IZrxVault.sol b/contracts/staking/contracts/src/interfaces/IZrxVault.sol index f1528f45ec..0b17be0d11 100644 --- a/contracts/staking/contracts/src/interfaces/IZrxVault.sol +++ b/contracts/staking/contracts/src/interfaces/IZrxVault.sol @@ -50,7 +50,7 @@ interface IZrxVault { /// @dev Sets the address of the StakingProxy contract. /// Note that only the contract staker can call this function. /// @param _stakingProxyAddress Address of Staking proxy contract. - function setStakingProxy(address payable _stakingProxyAddress) + function setStakingProxy(address _stakingProxyAddress) external; /// @dev Vault enters into Catastrophic Failure Mode. diff --git a/contracts/staking/contracts/src/stake/MixinStake.sol b/contracts/staking/contracts/src/stake/MixinStake.sol index 08ead69be2..415792238e 100644 --- a/contracts/staking/contracts/src/stake/MixinStake.sol +++ b/contracts/staking/contracts/src/stake/MixinStake.sol @@ -35,7 +35,7 @@ contract MixinStake is function stake(uint256 amount) external { - address payable staker = msg.sender; + address staker = msg.sender; // deposit equivalent amount of ZRX into vault getZrxVault().depositFrom(staker, amount); @@ -109,7 +109,7 @@ contract MixinStake is ) external { - address payable staker = msg.sender; + address staker = msg.sender; // handle delegation if (from.status == IStructs.StakeStatus.DELEGATED) { @@ -154,7 +154,7 @@ contract MixinStake is /// @param amount Amount of stake to delegate. function _delegateStake( bytes32 poolId, - address payable staker, + address staker, uint256 amount ) private @@ -192,7 +192,7 @@ contract MixinStake is /// @param amount Amount of stake to un-delegate. function _undelegateStake( bytes32 poolId, - address payable staker, + address staker, uint256 amount ) private diff --git a/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol index c873be205d..e100dafa82 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol @@ -70,10 +70,7 @@ contract MixinCumulativeRewards is { uint256 currentEpoch_ = currentEpoch; _cumulativeRewardsByPool[poolId][currentEpoch_] = value; - // Update state to reflect the most recent cumulative reward - uint256 currentMostRecentEpoch = _cumulativeRewardsByPoolLastStored[poolId]; - assert(currentEpoch_ >= currentMostRecentEpoch); _cumulativeRewardsByPoolLastStored[poolId] = currentEpoch_; } diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol index 64710750ef..8869c00c64 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol @@ -51,7 +51,7 @@ contract MixinStakingPool is returns (bytes32 poolId) { // note that an operator must be payable - address payable operator = msg.sender; + address operator = msg.sender; // compute unique id for this pool poolId = lastPoolId = bytes32(uint256(lastPoolId).safeAdd(1)); @@ -65,7 +65,6 @@ contract MixinStakingPool is // create and store pool IStructs.Pool memory pool = IStructs.Pool({ - initialized: true, operator: operator, operatorShare: operatorShare }); diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index 35de11be6f..0fc899fd4d 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -139,12 +139,10 @@ contract MixinStakingPoolRewards is } // Add a cumulative reward entry for this epoch. - if (!_isCumulativeRewardSet(_cumulativeRewardsByPool[poolId][currentEpoch])) { - _forceSetCumulativeReward( - poolId, - _getMostRecentCumulativeReward(poolId) - ); - } + _forceSetCumulativeReward( + poolId, + _getMostRecentCumulativeReward(poolId) + ); } /// @dev Handles a pool's reward at the current epoch. diff --git a/contracts/staking/contracts/src/sys/MixinAbstract.sol b/contracts/staking/contracts/src/sys/MixinAbstract.sol index a5ee43efbc..2ca468b689 100644 --- a/contracts/staking/contracts/src/sys/MixinAbstract.sol +++ b/contracts/staking/contracts/src/sys/MixinAbstract.sol @@ -35,12 +35,7 @@ contract MixinAbstract { /// @return membersStake The total stake for all non-operator members in /// this pool. function finalizePool(bytes32 poolId) - public - returns ( - uint256 operatorReward, - uint256 membersReward, - uint256 membersStake - ); + public; /// @dev Computes the reward owed to a pool during finalization. /// Does nothing if the pool is already finalized. diff --git a/contracts/staking/contracts/src/sys/MixinFinalizer.sol b/contracts/staking/contracts/src/sys/MixinFinalizer.sol index 0db85a3e4c..72c9907d9c 100644 --- a/contracts/staking/contracts/src/sys/MixinFinalizer.sol +++ b/contracts/staking/contracts/src/sys/MixinFinalizer.sol @@ -96,54 +96,61 @@ contract MixinFinalizer is /// 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. - /// @return operatorReward The reward credited to the pool operator. - /// @return membersReward The reward credited to the pool members. - /// @return membersStake The total stake for all non-operator members in - /// this pool. function finalizePool(bytes32 poolId) public - returns ( - uint256 operatorReward, - uint256 membersReward, - uint256 membersStake - ) { - uint256 epoch = currentEpoch; - // There are no pools to finalize at epoch 0. - if (epoch == 0) { - return (0, 0, 0); + // Noop on epoch 0 + uint256 currentEpoch_ = currentEpoch; + if (currentEpoch_ == 0) { + return; } - uint256 prevEpoch = epoch - 1; - // Load the finalization state into memory. + // Load the finalization and pool state into memory. IStructs.UnfinalizedState memory state = unfinalizedState; - // If there are no more unfinalized pools remaining, there's nothing - // to do. + // Noop if all active pools have been finalized. if (state.poolsRemaining == 0) { - return (0, 0, 0); + return; } + uint256 prevEpoch = currentEpoch_.safeSub(1); IStructs.ActivePool memory pool = _getActivePoolFromEpoch(prevEpoch, poolId); - // Do nothing if the pool was not active or already finalized (has no fees). + + // Noop if the pool was not active or already finalized (has no fees). if (pool.feesCollected == 0) { - return (operatorReward, membersReward, membersStake); + return; } - (operatorReward, membersReward) = _creditRewardsToPool( - epoch, + // Clear the pool state so we don't finalize it again, and to recoup + // some gas. + delete _getActivePoolsFromEpoch(prevEpoch)[poolId]; + + // Compute the rewards. + uint256 rewards = _getUnfinalizedPoolRewardsFromState(pool, state); + + // Pay the operator and update rewards for the pool. + // Note that we credit at the CURRENT epoch even though these rewards + // were earned in the previous epoch. + (uint256 operatorReward, uint256 membersReward) = _syncPoolRewards( + poolId, + rewards, + pool.membersStake + ); + + // Emit an event. + emit RewardsPaid( + currentEpoch_, poolId, - pool, - state + operatorReward, + membersReward ); + uint256 totalReward = operatorReward.safeAdd(membersReward); - if (totalReward > 0) { - // Increase `totalRewardsFinalized`. - unfinalizedState.totalRewardsFinalized = - state.totalRewardsFinalized = - state.totalRewardsFinalized.safeAdd(totalReward); - } + // Increase `totalRewardsFinalized`. + unfinalizedState.totalRewardsFinalized = + state.totalRewardsFinalized = + state.totalRewardsFinalized.safeAdd(totalReward); // Decrease the number of unfinalized pools left. unfinalizedState.poolsRemaining = @@ -159,8 +166,6 @@ contract MixinFinalizer is state.rewardsAvailable.safeSub(state.totalRewardsFinalized) ); } - membersStake = pool.membersStake; - return (operatorReward, membersReward, membersStake); } /// @dev Computes the reward owed to a pool during finalization. @@ -279,46 +284,4 @@ contract MixinFinalizer is rewards = rewardsRemaining; } } - - /// @dev Credits finalization rewards to a pool that was active in the - /// last epoch. - /// @param epoch The current epoch. - /// @param poolId The pool ID to finalize. - /// @param pool The active pool to finalize. - /// @param state The current state of finalization. - /// @return operatorReward The reward credited to the pool operator. - /// @return membersReward The reward credited to the pool members. - function _creditRewardsToPool( - uint256 epoch, - bytes32 poolId, - IStructs.ActivePool memory pool, - IStructs.UnfinalizedState memory state - ) - private - returns (uint256 operatorReward, uint256 membersReward) - { - // Clear the pool state so we don't finalize it again, and to recoup - // some gas. - delete _getActivePoolsFromEpoch(epoch.safeSub(1))[poolId]; - - // Compute the rewards. - uint256 rewards = _getUnfinalizedPoolRewardsFromState(pool, state); - - // Pay the pool. - // Note that we credit at the CURRENT epoch even though these rewards - // were earned in the previous epoch. - (operatorReward, membersReward) = _syncPoolRewards( - poolId, - rewards, - pool.membersStake - ); - - // Emit an event. - emit RewardsPaid( - epoch, - poolId, - operatorReward, - membersReward - ); - } } diff --git a/contracts/staking/contracts/test/TestDelegatorRewards.sol b/contracts/staking/contracts/test/TestDelegatorRewards.sol index 0766579a9d..e91ce5954c 100644 --- a/contracts/staking/contracts/test/TestDelegatorRewards.sol +++ b/contracts/staking/contracts/test/TestDelegatorRewards.sol @@ -177,11 +177,6 @@ contract TestDelegatorRewards is /// the current epoch and emit a event, function finalizePool(bytes32 poolId) public - returns ( - uint256 operatorReward, - uint256 membersReward, - uint256 membersStake - ) { UnfinalizedPoolReward memory reward = unfinalizedPoolRewardsByEpoch[currentEpoch][poolId]; delete unfinalizedPoolRewardsByEpoch[currentEpoch][poolId]; @@ -189,8 +184,8 @@ contract TestDelegatorRewards is _setOperatorShare(poolId, reward.operatorReward, reward.membersReward); uint256 totalRewards = reward.operatorReward + reward.membersReward; - membersStake = reward.membersStake; - (operatorReward, membersReward) = + uint256 membersStake = reward.membersStake; + (uint256 operatorReward, uint256 membersReward) = _syncPoolRewards(poolId, totalRewards, membersStake); emit FinalizePool(poolId, operatorReward, membersReward, membersStake); } diff --git a/contracts/staking/test/cumulative_reward_tracking_test.ts b/contracts/staking/test/cumulative_reward_tracking_test.ts index b2b4e360fc..1abd31c169 100644 --- a/contracts/staking/test/cumulative_reward_tracking_test.ts +++ b/contracts/staking/test/cumulative_reward_tracking_test.ts @@ -53,7 +53,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => { // Creates CR for epoch 1 TestAction.Delegate, ], - [], + [{ event: 'SetCumulativeReward', epoch: 0 }], ); }); it('re-delegating in the same epoch', async () => { @@ -68,7 +68,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => { // Updates CR for epoch 0 TestAction.Delegate, ], - [], + [{ event: 'SetCumulativeReward', epoch: 0 }, { event: 'SetCumulativeReward', epoch: 0 }], ); }); it('delegating in new epoch', async () => { @@ -268,7 +268,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => { // Clears CR for epoch 2 TestAction.Delegate, ], - [], + [{ event: 'SetCumulativeReward', epoch: 3 }], ); }); it('delegate in epoch 0 and 1, earn reward in epoch 3, then undelegate half', async () => { @@ -303,7 +303,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => { // Clears CR for epoch 2 TestAction.Undelegate, ], - [], + [{ event: 'SetCumulativeReward', epoch: 3 }], ); }); it('delegate in epoch 1, 2, earn rewards in epoch 3, skip to epoch 4, then delegate', async () => {