From be1a70c461cf0f2fcfa8f75d65d854e5f607eeea Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 13 Sep 2019 21:04:59 -0700 Subject: [PATCH 1/6] refactored the staking vault --- .../contracts/src/fees/MixinExchangeFees.sol | 20 +- .../contracts/src/immutable/MixinStorage.sol | 4 +- .../src/interfaces/IStakingEvents.sol | 26 ++ .../interfaces/IStakingPoolRewardVault.sol | 163 +++--------- .../contracts/src/interfaces/IStructs.sol | 12 + .../src/staking_pools/MixinEthVault.sol | 81 ++++++ .../src/staking_pools/MixinStakingPool.sol | 250 +++++------------ .../staking_pools/MixinStakingPoolMakers.sol | 217 +++++++++++++++ .../MixinStakingPoolModifiers.sol | 81 ++++++ .../MixinStakingPoolRewardVault.sol | 60 ++--- .../staking_pools/MixinStakingPoolRewards.sol | 50 +++- .../src/vaults/StakingPoolRewardVault.sol | 251 ++---------------- .../contracts/test/TestStorageLayout.sol | 2 +- contracts/staking/src/artifacts.ts | 4 + contracts/staking/src/wrappers.ts | 2 + .../staking/test/actors/finalizer_actor.ts | 71 +++-- .../test/actors/pool_operator_actor.ts | 5 +- contracts/staking/test/rewards_test.ts | 65 ++--- contracts/staking/test/utils/api_wrapper.ts | 10 + contracts/staking/test/utils/types.ts | 12 +- contracts/staking/test/vaults_test.ts | 57 ---- contracts/staking/tsconfig.json | 2 + 22 files changed, 723 insertions(+), 722 deletions(-) create mode 100644 contracts/staking/contracts/src/staking_pools/MixinEthVault.sol create mode 100644 contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol create mode 100644 contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol delete mode 100644 contracts/staking/test/vaults_test.ts diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index 152e8485e1..8ea7e4812a 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol @@ -210,7 +210,7 @@ contract MixinExchangeFees is // compute weighted stake uint256 totalStakeDelegatedToPool = getTotalStakeDelegatedToPool(poolId).currentEpochBalance; - uint256 stakeHeldByPoolOperator = getStakeDelegatedToPoolByOwner(rewardVault.operatorOf(poolId), poolId).currentEpochBalance; + uint256 stakeHeldByPoolOperator = getStakeDelegatedToPoolByOwner(poolById[poolId].operator, poolId).currentEpochBalance; uint256 weightedStake = stakeHeldByPoolOperator.safeAdd( totalStakeDelegatedToPool .safeSub(stakeHeldByPoolOperator) @@ -255,23 +255,13 @@ contract MixinExchangeFees is cobbDouglasAlphaDenominator ); - // record reward in vault - (, uint256 membersPortion) = rewardVault.recordDepositFor( + // pay reward to pool + _handleStakingPoolReward( activePools[i].poolId, reward, - activePools[i].delegatedStake == 0 // true -> reward is for operator only + activePools[i].delegatedStake, + currentEpoch ); - totalRewardsPaid = totalRewardsPaid.safeAdd(reward); - - // sync cumulative rewards, if necessary. - if (membersPortion > 0) { - _recordRewardForDelegators( - activePools[i].poolId, - membersPortion, - activePools[i].delegatedStake, - currentEpoch - ); - } // clear state for gas refunds protocolFeesThisEpochByPool[activePools[i].poolId] = 0; diff --git a/contracts/staking/contracts/src/immutable/MixinStorage.sol b/contracts/staking/contracts/src/immutable/MixinStorage.sol index 98ac6c237a..244541f606 100644 --- a/contracts/staking/contracts/src/immutable/MixinStorage.sol +++ b/contracts/staking/contracts/src/immutable/MixinStorage.sol @@ -77,8 +77,8 @@ contract MixinStorage is // whether the operator of that pool has subsequently added the maker. mapping (address => IStructs.MakerPoolJoinStatus) public poolJoinedByMakerAddress; - // mapping from Pool Id to number of makers assigned to that pool - mapping (bytes32 => uint256) public numMakersByPoolId; + // mapping from Pool Id to Pool + mapping (bytes32 => IStructs.Pool) internal poolById; // current epoch uint256 public currentEpoch = INITIAL_EPOCH; diff --git a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol index e538bf7c16..c7133e71f4 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol @@ -136,4 +136,30 @@ interface IStakingEvents { bytes32 poolId, address makerAddress ); + + /// @dev Emitted by MixinStakingPoolRewardVault when the vault's address is changed. + /// @param rewardVaultAddress Address of new reward vault. + event StakingPoolRewardVaultChanged( + address rewardVaultAddress + ); + + /// @dev Emitted when a staking pool's operator share is decreased. + /// @param poolId Unique Id of pool. + /// @param oldOperatorShare Previous share of rewards owned by operator. + /// @param newOperatorShare Newly decreased share of rewards owned by operator. + event OperatorShareDecreased( + bytes32 poolId, + uint32 oldOperatorShare, + uint32 newOperatorShare + ); + + /// @dev Emitted when an operator reward is transferred to the Eth vault. + /// @param amount The amount in ETH withdrawn. + /// @param operator of the pool. + /// @param poolId The pool the reward was deposited for. + event OperatorRewardTransferredToEthVault( + bytes32 poolId, + address operator, + uint256 amount + ); } diff --git a/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol b/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol index 95c4e7d6d2..cbecee1f36 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol @@ -21,160 +21,59 @@ pragma experimental ABIEncoderV2; /// @dev This vault manages staking pool rewards. -/// Rewards can be deposited and withdraw by the staking contract. -/// There is a "Catastrophic Failure Mode" that, when invoked, only -/// allows withdrawals to be made. Once this vault is in catastrophic -/// failure mode, it cannot be returned to normal mode; this prevents -/// corruption of related state in the staking contract. interface IStakingPoolRewardVault { - /// @dev Holds the balances and other data for a staking pool. - /// @param operatorShare Fraction of the total balance owned by the operator, in ppm. - /// @param operatorBalance Balance in ETH of the operator. - /// @param membersBalance Balance in ETH co-owned by the pool members. - struct Pool { - uint32 operatorShare; - uint96 operatorBalance; - uint96 membersBalance; - address payable operatorAddress; - } + /// @dev Emitted when Eth is deposited into the vault. + /// @param sender Address of sender (`msg.sender`). + /// @param poolId that owns of Eth. + /// @param amount of Eth deposited. + event EthDepositedIntoVault( + address indexed sender, + bytes32 indexed poolId, + uint256 amount + ); + + /// @dev Emitted when a reward is transferred to the ETH vault. + /// @param amount The amount in ETH withdrawn. + /// @param member of the pool. + /// @param poolId The pool the reward was deposited for. + event PoolRewardTransferredToEthVault( + bytes32 poolId, + address member, + uint256 amount + ); /// @dev Emitted when the eth vault is changed - /// @param newEthVault address of new rth vault. + /// @param newEthVault address of new Eth vault. event EthVaultChanged( address newEthVault ); - /// @dev Emitted when reward is deposited. - /// @param poolId The pool the reward was deposited for. - /// Note that a poolId of "0" means "unknown" at time of deposit. - /// In this case, the reward would be deposited later in the transaction. - /// This is an optimization for the staking contract, which may make many deposits - /// in the same transaction. - /// @param amount The amount in ETH deposited. - event RewardDeposited( - bytes32 poolId, - uint256 amount - ); - - /// @dev Emitted when a reward is withdrawn for an operator. - /// @param amount The amount in ETH withdrawn. - /// @param poolId The pool the reward was deposited for. - event RewardWithdrawnForOperator( - bytes32 poolId, - uint256 amount - ); - - /// @dev Emitted when a reward is withdrawn for a pool member. - /// @param amount The amount in ETH withdrawn. - /// @param poolId The pool the reward was deposited for. - event RewardWithdrawnForMember( - bytes32 poolId, - uint256 amount - ); - - /// @dev Emitted when a staking pool is registered. - /// @param poolId Unique Id of pool that was registered. - /// @param operatorShare Share of rewards owned by operator. in ppm. - event StakingPoolRegistered( - bytes32 poolId, - uint32 operatorShare - ); - - /// @dev Emitted when a staking pool's operator share is decreased. - /// @param poolId Unique Id of pool that was registered. - /// @param oldOperatorShare Previous share of rewards owned by operator. - /// @param newOperatorShare Newly decreased share of rewards owned by operator. - event OperatorShareDecreased( - bytes32 poolId, - uint32 oldOperatorShare, - uint32 newOperatorShare - ); - - /// @dev Fallback function. - /// Note that this is only callable by the staking contract, and when - /// not in catastrophic failure mode. - function () - external - payable; - + /// @dev Sets the Eth Vault. + /// Note that only the contract owner can call this. + /// @param ethVaultAddress Address of the Eth Vault. function setEthVault(address ethVaultAddress) external; - /// @dev Record a deposit for a pool. This deposit should be in the same transaction, - /// which is enforced by the staking contract. We do not enforce it here to save (a lot of) gas. - /// Note that this is only callable by the staking contract, and when - /// not in catastrophic failure mode. - /// @param poolId Unique Id of pool. - /// @param amount Amount in ETH to record. - /// @param operatorOnly Only attribute amount to operator. - /// @return operatorPortion Portion of amount attributed to the operator. - /// @return operatorPortion Portion of amount attributed to the delegators. - function recordDepositFor( - bytes32 poolId, - uint256 amount, - bool operatorOnly - ) + /// @dev Deposit an amount of ETH (`msg.value`) for `poolId` into the vault. + /// Note that this is only callable by the staking contract. + /// @param poolId that owns the ETH. + function depositFor(bytes32 poolId) external - returns ( - uint256 operatorPortion, - uint256 delegatorsPortion - ); - - /// @dev Withdraw some amount in ETH of an operator's reward. - /// Note that this is only callable by the staking contract, and when - /// not in catastrophic failure mode. - /// @param poolId Unique Id of pool. - function transferOperatorBalanceToEthVault( - bytes32 poolId, - address operator, - uint256 amount - ) - external; + payable; /// @dev Withdraw some amount in ETH of a pool member. - /// Note that this is only callable by the staking contract, and when - /// not in catastrophic failure mode. + /// Note that this is only callable by the staking contract. /// @param poolId Unique Id of pool. /// @param amount Amount in ETH to transfer. - function transferMemberBalanceToEthVault( + function transferToEthVault( bytes32 poolId, address member, uint256 amount ) external; - /// @dev Register a new staking pool. - /// Note that this is only callable by the staking contract, and when - /// not in catastrophic failure mode. - /// @param poolId Unique Id of pool. - /// @param operatorAddress Address of the pool operator. - /// @param operatorShare Share of rewards given to the pool operator, in ppm. - function registerStakingPool( - bytes32 poolId, - address payable operatorAddress, - uint32 operatorShare - ) - external; - - /// @dev Decreases the operator share for the given pool (i.e. increases pool rewards for members). - /// Note that this is only callable by the staking contract, and will revert if the new operator - /// share value is greater than the old value. - /// @param poolId Unique Id of pool. - /// @param newOperatorShare The newly decresaed percentage of any rewards owned by the operator. - function decreaseOperatorShare(bytes32 poolId, uint32 newOperatorShare) - external; - - /// @dev Returns the address of the operator of a given pool - /// @param poolId Unique id of pool - /// @return operatorAddress Operator of the pool - function operatorOf(bytes32 poolId) - external - view - returns (address payable); - - /// @dev Returns the total balance of a pool. - /// @param poolId Unique Id of pool. + /// @dev Returns the balance in ETH of `poolId` /// @return Balance in ETH. function balanceOf(bytes32 poolId) external diff --git a/contracts/staking/contracts/src/interfaces/IStructs.sol b/contracts/staking/contracts/src/interfaces/IStructs.sol index 8b6a731a1c..46880b473f 100644 --- a/contracts/staking/contracts/src/interfaces/IStructs.sol +++ b/contracts/staking/contracts/src/interfaces/IStructs.sol @@ -95,4 +95,16 @@ interface IStructs { uint256 cumulativeRewardEpoch; IStructs.Fraction cumulativeReward; } + + /// @dev Holds the balances and other data for a staking pool. + /// @param initialzed 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. + /// @param numberOfMakers Number of makers in the pool. + struct Pool { + bool initialized; + address payable operator; + uint32 operatorShare; + uint32 numberOfMakers; + } } diff --git a/contracts/staking/contracts/src/staking_pools/MixinEthVault.sol b/contracts/staking/contracts/src/staking_pools/MixinEthVault.sol new file mode 100644 index 0000000000..1a022a96b9 --- /dev/null +++ b/contracts/staking/contracts/src/staking_pools/MixinEthVault.sol @@ -0,0 +1,81 @@ +/* + + 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 "../interfaces/IStakingEvents.sol"; +import "../interfaces/IEthVault.sol"; +import "../immutable/MixinStorage.sol"; + + +/// @dev This mixin contains logic for managing and interfacing with the Eth Vault. +/// (see vaults/EthVault.sol). +contract MixinEthVault is + IStakingEvents, + MixinConstants, + Ownable, + MixinStorage +{ + + /// @dev Set the Eth Vault. + /// @param ethVaultAddress Address of the Eth Vault. + function setEthVault(address ethVaultAddress) + external + onlyOwner + { + ethVault = IEthVault(ethVaultAddress); + } + + /// @dev Return the current Eth Vault + /// @return Eth Vault + function getEthVault() + public + view + returns (address) + { + return address(ethVault); + } + + /// @dev Transfers operator reward to the ETH vault. + /// @param poolId Unique Id of pool to transfer reward for, + /// @param operator of the pool. + /// @param amount of ETH to transfer. + function _transferOperatorRewardToEthVault( + bytes32 poolId, + address operator, + uint256 amount + ) + internal + { + // sanity check on eth vault + IEthVault _ethVault = ethVault; + if (address(_ethVault) == address(0)) { + LibRichErrors.rrevert( + LibStakingRichErrors.EthVaultNotSetError() + ); + } + + // perform transfer + _ethVault.depositFor.value(amount)(operator); + emit OperatorRewardTransferredToEthVault( + poolId, + operator, + amount + ); + } +} diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol index e96fd8551c..62719886ca 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol @@ -24,31 +24,9 @@ import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; import "../libs/LibStakingRichErrors.sol"; import "../interfaces/IStructs.sol"; import "./MixinStakingPoolRewards.sol"; +import "./MixinStakingPoolMakers.sol"; -/// @dev This mixin contains logic for staking pools. -/// A pool has a single operator and any number of delegators (members). -/// Any staker can create a pool, although at present it is only beneficial -/// for market makers to create staking pools. A market maker *must* create a -/// pool in order to receive fee-based rewards at the end of each epoch (see MixinExchangeFees). -/// Moreover, creating a staking pool leverages the delegated stake within the pool, -/// which is counted towards a maker's total stake when computing rewards. A market maker -/// can register any number of makerAddresses with their pool, and can incentivize delegators -/// to join their pool by specifying a fixed percentage of their fee-based rewards to be split amonst -/// the members of their pool. Any rewards set aside for members of the pool is divided based on -/// how much stake each member delegated. -/// -/// Terminology: -/// "Pool Id" - A unique id generated by this contract and assigned to each pool when it is created. -/// "Pool Operator" - The creator and operator of the pool. -/// "Pool Members" - Members of the pool who opted-in by delegating to the pool. -/// "Market Makers" - Market makers on the 0x protocol. -/// -/// How-To for Market Makers: -/// 1. Create a pool, specifying what percentage of rewards kept for yourself. -/// The remaining is divided among members of your pool. -/// 2. Add the addresses that you use to market make on 0x. -/// 3. Leverage the staking power of others by convincing them to delegate to your pool. contract MixinStakingPool is MixinStakingPoolRewards { @@ -64,191 +42,84 @@ contract MixinStakingPool is returns (bytes32 poolId) { // note that an operator must be payable - address payable operatorAddress = msg.sender; + address payable operator = msg.sender; // assign pool id and generate next id poolId = nextPoolId; nextPoolId = _computeNextStakingPoolId(poolId); + // sanity check on operator share + _assertNewOperatorShare( + poolId, + PPM_DENOMINATOR, // max operator share + operatorShare + ); + + // create and store pool + IStructs.Pool memory pool = IStructs.Pool({ + initialized: true, + operator: operator, + operatorShare: operatorShare, + numberOfMakers: 0 + }); + poolById[poolId] = pool; + // initialize cumulative rewards for this pool; // this is used to track rewards earned by delegators. _initializeCumulativeRewards(poolId); - // register pool in reward vault - rewardVault.registerStakingPool(poolId, operatorAddress, operatorShare); - // Staking pool has been created - emit StakingPoolCreated(poolId, operatorAddress, operatorShare); + emit StakingPoolCreated(poolId, operator, operatorShare); if (addOperatorAsMaker) { - // Is the maker already in a pool? - if (isMakerAssignedToStakingPool(operatorAddress)) { - LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( - LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressAlreadyRegistered, - operatorAddress, - getStakingPoolIdOfMaker(operatorAddress) - )); - } - - IStructs.MakerPoolJoinStatus memory poolJoinStatus = IStructs.MakerPoolJoinStatus({ - poolId: poolId, - confirmed: true - }); - poolJoinedByMakerAddress[operatorAddress] = poolJoinStatus; - numMakersByPoolId[poolId] += 1; - - // Operator has been added as a maker to tbe pool - emit MakerAddedToStakingPool( - poolId, - operatorAddress - ); + _addMakerToStakingPool(poolId, operator); } return poolId; } - /// @dev Allows caller to join a staking pool if already assigned. - /// @param poolId Unique id of pool. - function joinStakingPoolAsMaker(bytes32 poolId) + /// @dev Decreases the operator share for the given pool (i.e. increases pool rewards for members). + /// @param poolId Unique Id of pool. + /// @param newOperatorShare The newly decreased percentage of any rewards owned by the operator. + function decreaseStakingPoolOperatorShare(bytes32 poolId, uint32 newOperatorShare) external { - // Is the maker already in a pool? - address makerAddress = msg.sender; - if (isMakerAssignedToStakingPool(makerAddress)) { - LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( - LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressAlreadyRegistered, - makerAddress, - getStakingPoolIdOfMaker(makerAddress) - )); - } - - IStructs.MakerPoolJoinStatus memory poolJoinStatus = IStructs.MakerPoolJoinStatus({ - poolId: poolId, - confirmed: false - }); - poolJoinedByMakerAddress[makerAddress] = poolJoinStatus; - - // Maker has joined to the pool, awaiting operator confirmation - emit PendingAddMakerToPool( + // load pool and assert that we can decrease + IStructs.Pool memory pool = poolById[poolId]; + _assertNewOperatorShare( poolId, - makerAddress + pool.operatorShare, + newOperatorShare + ); + + // decrease operator share + poolById[poolId].operatorShare = newOperatorShare; + emit OperatorShareDecreased( + poolId, + pool.operatorShare, + newOperatorShare ); } - /// @dev Adds a maker to a staking pool. Note that this is only callable by the pool operator. - /// Note also that the maker must have previously called joinStakingPoolAsMaker. - /// @param poolId Unique id of pool. - /// @param makerAddress Address of maker. - function addMakerToStakingPool( - bytes32 poolId, - address makerAddress - ) + /// @dev Returns the staking pool with `poolId` + /// @param poolId Unique id of pool + /// @return operator Operator of the pool + function getStakingPool(bytes32 poolId) external - onlyStakingPoolOperator(poolId) + view + returns (IStructs.Pool memory) { - // Is the maker already in a pool? - if (isMakerAssignedToStakingPool(makerAddress)) { - LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( - LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressAlreadyRegistered, - makerAddress, - getStakingPoolIdOfMaker(makerAddress) - )); - } - - // Is the maker trying to join this pool? - bytes32 makerPendingPoolId = poolJoinedByMakerAddress[makerAddress].poolId; - if (makerPendingPoolId != poolId) { - LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( - LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressNotPendingAdd, - makerAddress, - makerPendingPoolId - )); - } - - // Is the pool already full? - if (numMakersByPoolId[poolId] == maximumMakersInPool) { - LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( - LibStakingRichErrors.MakerPoolAssignmentErrorCodes.PoolIsFull, - makerAddress, - poolId - )); - } - - // Add maker to pool - IStructs.MakerPoolJoinStatus memory poolJoinStatus = IStructs.MakerPoolJoinStatus({ - poolId: poolId, - confirmed: true - }); - poolJoinedByMakerAddress[makerAddress] = poolJoinStatus; - numMakersByPoolId[poolId] += 1; - - // Maker has been added to the pool - emit MakerAddedToStakingPool( - poolId, - makerAddress - ); + return poolById[poolId]; } - /// @dev Removes a maker from a staking pool. Note that this is only callable by the pool operator or maker. - /// Note also that the maker does not have to *agree* to leave the pool; this action is - /// at the sole discretion of the pool operator. - /// @param poolId Unique id of pool. - /// @param makerAddress Address of maker. - function removeMakerFromStakingPool( - bytes32 poolId, - address makerAddress - ) - external - onlyStakingPoolOperatorOrMaker(poolId, makerAddress) - { - bytes32 makerPoolId = getStakingPoolIdOfMaker(makerAddress); - if (makerPoolId != poolId) { - LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( - LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressNotRegistered, - makerAddress, - makerPoolId - )); - } - - // remove the pool and confirmation from the maker status - IStructs.MakerPoolJoinStatus memory poolJoinStatus = IStructs.MakerPoolJoinStatus({ - poolId: NIL_POOL_ID, - confirmed: false - }); - poolJoinedByMakerAddress[makerAddress] = poolJoinStatus; - numMakersByPoolId[poolId] -= 1; - - // Maker has been removed from the pool` - emit MakerRemovedFromStakingPool( - poolId, - makerAddress - ); - } - - /// @dev Returns the pool id of the input maker. - /// @param makerAddress Address of maker - /// @return Pool id, nil if maker is not yet assigned to a pool. - function getStakingPoolIdOfMaker(address makerAddress) + /// @dev Returns the unique id that will be assigned to the next pool that is created. + /// @return Pool id. + function getNextStakingPoolId() public view returns (bytes32) { - if (isMakerAssignedToStakingPool(makerAddress)) { - return poolJoinedByMakerAddress[makerAddress].poolId; - } else { - return NIL_POOL_ID; - } - } - - /// @dev Returns true iff the maker is assigned to a staking pool. - /// @param makerAddress Address of maker - /// @return True iff assigned. - function isMakerAssignedToStakingPool(address makerAddress) - public - view - returns (bool) - { - return poolJoinedByMakerAddress[makerAddress].confirmed; + return nextPoolId; } /// @dev Computes the unique id that comes after the input pool id. @@ -261,4 +132,29 @@ contract MixinStakingPool is { return bytes32(uint256(poolId).safeAdd(POOL_ID_INCREMENT_AMOUNT)); } + + function _assertNewOperatorShare( + bytes32 poolId, + uint32 currentOperatorShare, + uint32 newOperatorShare + ) + private + { + // sanity checks + if (newOperatorShare > PPM_DENOMINATOR) { + // operator share must be a valid fraction + LibRichErrors.rrevert(LibStakingRichErrors.OperatorShareError( + LibStakingRichErrors.OperatorShareErrorCodes.OperatorShareTooLarge, + poolId, + newOperatorShare + )); + } else if (newOperatorShare >= currentOperatorShare) { + // new share must be less than the current share + LibRichErrors.rrevert(LibStakingRichErrors.OperatorShareError( + LibStakingRichErrors.OperatorShareErrorCodes.CanOnlyDecreaseOperatorShare, + poolId, + newOperatorShare + )); + } + } } diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol new file mode 100644 index 0000000000..f9c5de4031 --- /dev/null +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol @@ -0,0 +1,217 @@ +/* + + 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-utils/contracts/src/LibRichErrors.sol"; +import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; +import "../libs/LibStakingRichErrors.sol"; +import "../interfaces/IStructs.sol"; +import "../interfaces/IStakingEvents.sol"; +import "../immutable/MixinConstants.sol"; +import "../immutable/MixinStorage.sol"; +import "./MixinStakingPoolRewards.sol"; +import "./MixinStakingPoolModifiers.sol"; + + +/// @dev This mixin contains logic for staking pools. +contract MixinStakingPoolMakers is + IStakingEvents, + MixinConstants, + Ownable, + MixinStorage, + MixinZrxVault, + MixinStakingPoolRewardVault, + MixinScheduler, + MixinStakeStorage, + MixinStakeBalances, + MixinStakingPoolRewards, + MixinStakingPoolModifiers +{ + + using LibSafeMath for uint256; + + function joinStakingPoolAsMaker( + bytes32 poolId + ) + external + { + // Is the maker already in a pool? + address makerAddress = msg.sender; + if (isMakerAssignedToStakingPool(makerAddress)) { + LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( + LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressAlreadyRegistered, + makerAddress, + getStakingPoolIdOfMaker(makerAddress) + )); + } + + IStructs.MakerPoolJoinStatus memory poolJoinStatus = IStructs.MakerPoolJoinStatus({ + poolId: poolId, + confirmed: false + }); + poolJoinedByMakerAddress[makerAddress] = poolJoinStatus; + + // Maker has joined to the pool, awaiting operator confirmation + emit PendingAddMakerToPool( + poolId, + makerAddress + ); + } + + /// @dev Adds a maker to a staking pool. Note that this is only callable by the pool operator. + /// Note also that the maker must have previously called joinStakingPoolAsMaker. + /// @param poolId Unique id of pool. + /// @param makerAddress Address of maker. + function addMakerToStakingPool( + bytes32 poolId, + address makerAddress + ) + external + onlyStakingPoolOperator(poolId) + { + _addMakerToStakingPool(poolId, makerAddress); + } + + /// @dev Removes a maker from a staking pool. Note that this is only callable by the pool operator or maker. + /// Note also that the maker does not have to *agree* to leave the pool; this action is + /// at the sole discretion of the pool operator. + /// @param poolId Unique id of pool. + /// @param makerAddress Address of maker. + function removeMakerFromStakingPool( + bytes32 poolId, + address makerAddress + ) + external + onlyStakingPoolOperatorOrMaker(poolId, makerAddress) + { + bytes32 makerPoolId = getStakingPoolIdOfMaker(makerAddress); + if (makerPoolId != poolId) { + LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( + LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressNotRegistered, + makerAddress, + makerPoolId + )); + } + + // remove the pool and confirmation from the maker status + IStructs.MakerPoolJoinStatus memory poolJoinStatus = IStructs.MakerPoolJoinStatus({ + poolId: NIL_POOL_ID, + confirmed: false + }); + poolJoinedByMakerAddress[makerAddress] = poolJoinStatus; + poolById[poolId].numberOfMakers = uint256(poolById[poolId].numberOfMakers).safeSub(1).downcastToUint32(); + + // Maker has been removed from the pool` + emit MakerRemovedFromStakingPool( + poolId, + makerAddress + ); + } + + /// @dev Returns the pool id of the input maker. + /// @param makerAddress Address of maker + /// @return Pool id, nil if maker is not yet assigned to a pool. + function getStakingPoolIdOfMaker(address makerAddress) + public + view + returns (bytes32) + { + if (isMakerAssignedToStakingPool(makerAddress)) { + return poolJoinedByMakerAddress[makerAddress].poolId; + } else { + return NIL_POOL_ID; + } + } + + /// @dev Returns true iff the maker is assigned to a staking pool. + /// @param makerAddress Address of maker + /// @return True iff assigned. + function isMakerAssignedToStakingPool(address makerAddress) + public + view + returns (bool) + { + return poolJoinedByMakerAddress[makerAddress].confirmed; + } + + /// @dev Returns the current number of makers in a given pool. + /// @param poolId Unique id of pool. + /// @return Size of pool. + function getNumberOfMakersInStakingPool(bytes32 poolId) + public + view + returns (uint256) + { + return poolById[poolId].numberOfMakers; + } + + /// @dev Adds a maker to a staking pool. Note that this is only callable by the pool operator. + /// Note also that the maker must have previously called joinStakingPoolAsMaker. + /// @param poolId Unique id of pool. + /// @param makerAddress Address of maker. + function _addMakerToStakingPool( + bytes32 poolId, + address makerAddress + ) + internal + { + // Is the maker already in a pool? + if (isMakerAssignedToStakingPool(makerAddress)) { + LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( + LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressAlreadyRegistered, + makerAddress, + getStakingPoolIdOfMaker(makerAddress) + )); + } + + // Is the maker trying to join this pool; or are they the operator? + bytes32 makerPendingPoolId = poolJoinedByMakerAddress[makerAddress].poolId; + if (makerPendingPoolId != poolId && makerAddress != poolById[poolId].operator) { + LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( + LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressNotPendingAdd, + makerAddress, + makerPendingPoolId + )); + } + + // Is the pool already full? + if (getNumberOfMakersInStakingPool(poolId) == maximumMakersInPool) { + LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( + LibStakingRichErrors.MakerPoolAssignmentErrorCodes.PoolIsFull, + makerAddress, + poolId + )); + } + + // Add maker to pool + IStructs.MakerPoolJoinStatus memory poolJoinStatus = IStructs.MakerPoolJoinStatus({ + poolId: poolId, + confirmed: true + }); + poolJoinedByMakerAddress[makerAddress] = poolJoinStatus; + poolById[poolId].numberOfMakers = uint256(poolById[poolId].numberOfMakers).safeAdd(1).downcastToUint32(); + + // Maker has been added to the pool + emit MakerAddedToStakingPool( + poolId, + makerAddress + ); + } +} diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol new file mode 100644 index 0000000000..5f730e3d4b --- /dev/null +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol @@ -0,0 +1,81 @@ +/* + + 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-utils/contracts/src/LibRichErrors.sol"; +import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; +import "../libs/LibStakingRichErrors.sol"; +import "../interfaces/IStructs.sol"; +import "../interfaces/IStakingEvents.sol"; +import "../immutable/MixinConstants.sol"; +import "../immutable/MixinStorage.sol"; +import "./MixinStakingPoolRewards.sol"; + + +contract MixinStakingPoolModifiers is + IStakingEvents, + MixinConstants, + Ownable, + MixinStorage, + MixinZrxVault, + MixinStakingPoolRewardVault, + MixinScheduler, + MixinStakeStorage, + MixinStakeBalances, + MixinStakingPoolRewards +{ + + using LibSafeMath for uint256; + + /// @dev Asserts that the sender is the operator of the input pool. + /// @param poolId Pool sender must be operator of. + modifier onlyStakingPoolOperator(bytes32 poolId) { + address poolOperator = poolById[poolId].operator; + if (msg.sender != poolOperator) { + LibRichErrors.rrevert(LibStakingRichErrors.OnlyCallableByPoolOperatorError( + msg.sender, + poolOperator + )); + } + + _; + } + + /// @dev Asserts that the sender is the operator of the input pool or the input maker. + /// @param poolId Pool sender must be operator of. + /// @param makerAddress Address of a maker in the pool. + modifier onlyStakingPoolOperatorOrMaker(bytes32 poolId, address makerAddress) { + address poolOperator; + if ( + msg.sender != makerAddress && + msg.sender != (poolOperator = poolById[poolId].operator) + ) { + LibRichErrors.rrevert( + LibStakingRichErrors.OnlyCallableByPoolOperatorOrMakerError( + msg.sender, + poolOperator, + makerAddress + ) + ); + } + + _; + } +} diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol index b3c75f858c..90f2e38c1d 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol @@ -32,51 +32,25 @@ contract MixinStakingPoolRewardVault is IStakingEvents, MixinStorage { - /// @dev Asserts that the sender is the operator of the input pool. - /// @param poolId Pool sender must be operator of. - modifier onlyStakingPoolOperator(bytes32 poolId) { - address poolOperator = rewardVault.operatorOf(poolId); - if (msg.sender != poolOperator) { - LibRichErrors.rrevert(LibStakingRichErrors.OnlyCallableByPoolOperatorError( - msg.sender, - poolOperator - )); - } - _; - } - - /// @dev Asserts that the sender is the operator of the input pool or the input maker. - /// @param poolId Pool sender must be operator of. - /// @param makerAddress Address of a maker in the pool. - modifier onlyStakingPoolOperatorOrMaker(bytes32 poolId, address makerAddress) { - address poolOperator; - if ( - msg.sender != makerAddress && - msg.sender != (poolOperator = rewardVault.operatorOf(poolId)) - ) { - LibRichErrors.rrevert( - LibStakingRichErrors.OnlyCallableByPoolOperatorOrMakerError( - msg.sender, - poolOperator, - makerAddress - ) - ); - } - - _; - } - - /// @dev Decreases the operator share for the given pool (i.e. increases pool rewards for members). - /// Note that this is only callable by the pool operator, and will revert if the new operator - /// share value is greater than the old value. - /// @param poolId Unique Id of pool. - /// @param newOperatorShare The newly decreased percentage of any rewards owned by the operator. - function decreaseStakingPoolOperatorShare(bytes32 poolId, uint32 newOperatorShare) + /// @dev Sets the address of the reward vault. + /// This can only be called by the owner of this contract. + function setStakingPoolRewardVault(address payable rewardVaultAddress) external - onlyStakingPoolOperator(poolId) + onlyOwner { - rewardVault.decreaseOperatorShare(poolId, newOperatorShare); + rewardVault = IStakingPoolRewardVault(rewardVaultAddress); + emit StakingPoolRewardVaultChanged(rewardVaultAddress); + } + + /// @dev Returns the staking pool reward vault + /// @return Address of reward vault. + function getStakingPoolRewardVault() + public + view + returns (address) + { + return address(rewardVault); } /// @dev Deposits an amount in ETH into the reward vault. @@ -116,6 +90,6 @@ contract MixinStakingPoolRewardVault is } // perform transfer - _rewardVault.transferMemberBalanceToEthVault(poolId, member, amount); + _rewardVault.transferToEthVault(poolId, member, amount); } } diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index 081b1309ac..72fe0836e2 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -22,6 +22,7 @@ pragma experimental ABIEncoderV2; import "@0x/contracts-utils/contracts/src/LibFractions.sol"; import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; import "./MixinCumulativeRewards.sol"; +import "./MixinEthVault.sol"; contract MixinStakingPoolRewards is @@ -29,6 +30,26 @@ 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 dependencies on cumulative rewards. + /// @param poolId Unique id of pool. + function syncDelegatorRewards(bytes32 poolId) + external + { + address member = msg.sender; + + IStructs.StoredBalance memory finalDelegatedStakeToPoolByOwner = _loadAndSyncBalance(delegatedStakeToPoolByOwner[member][poolId]); + _syncRewardsForDelegator( + poolId, + member, + _loadUnsyncedBalance(delegatedStakeToPoolByOwner[member][poolId]), // initial balance + finalDelegatedStakeToPoolByOwner + ); + + // update stored balance with synchronized version; this prevents redundant withdrawals. + delegatedStakeToPoolByOwner[member][poolId] = finalDelegatedStakeToPoolByOwner; + } + /// @dev Computes the reward balance in ETH of a specific member of a pool. /// @param poolId Unique id of pool. /// @param member The member of the pool. @@ -88,7 +109,7 @@ contract MixinStakingPoolRewards is /// @param reward to record for delegators. /// @param amountOfDelegatedStake the amount of delegated stake that will split this reward. /// @param epoch at which this was earned. - function _recordRewardForDelegators( + function _handleStakingPoolReward( bytes32 poolId, uint256 reward, uint256 amountOfDelegatedStake, @@ -96,6 +117,31 @@ contract MixinStakingPoolRewards is ) internal { + IStructs.Pool memory pool = poolById[poolId]; + + // compute the operator's portion of the reward and transfer it to the ETH vault (we round in favor of the operator). + uint256 operatorPortion = amountOfDelegatedStake == 0 + ? reward + : LibMath.getPartialAmountCeil( + uint256(pool.operatorShare), + PPM_DENOMINATOR, + reward + ); + + _transferOperatorRewardToEthVault( + poolId, + pool.operator, + operatorPortion + ); + + // compute the reward portion for the pool members and transfer it to the Reward Vault. + uint256 membersPortion = reward.safeSub(operatorPortion); + if (membersPortion == 0) { + return; + } + + rewardVault.depositFor.value(membersPortion)(poolId); + // cache a storage pointer to the cumulative rewards for `poolId` indexed by epoch. mapping (uint256 => IStructs.Fraction) storage _cumulativeRewardsByPoolPtr = _cumulativeRewardsByPool[poolId]; @@ -108,7 +154,7 @@ contract MixinStakingPoolRewards is (uint256 numerator, uint256 denominator) = LibFractions.addFractions( mostRecentCumulativeRewards.numerator, mostRecentCumulativeRewards.denominator, - reward, + membersPortion, amountOfDelegatedStake ); diff --git a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol index 5d841419a4..5013f7a729 100644 --- a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol @@ -19,7 +19,6 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; -import "@0x/contracts-exchange-libs/contracts/src/LibMath.sol"; import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; import "../libs/LibStakingRichErrors.sol"; @@ -31,38 +30,19 @@ import "../immutable/MixinConstants.sol"; /// @dev This vault manages staking pool rewards. -/// Rewards can be deposited and withdrawn by the staking contract. -/// There is a "Catastrophic Failure Mode" that, when invoked, only -/// allows withdrawals to be made. Once this vault is in catastrophic -/// failure mode, it cannot be returned to normal mode; this prevents -/// corruption of related state in the staking contract. -/// -/// When in Catastrophic Failure Mode, the Staking contract can still -/// perform withdrawals on behalf of its users. contract StakingPoolRewardVault is IStakingPoolRewardVault, MixinConstants, MixinVaultCore { using LibSafeMath for uint256; - using LibSafeDowncast for uint256; // mapping from poolId to Pool metadata - mapping (bytes32 => Pool) public poolById; + mapping (bytes32 => uint256) internal balanceByPoolId; // address of ether vault IEthVault internal _ethVault; - /// @dev Fallback function. This contract is payable, but only by the staking contract. - function () - external - payable - onlyStakingProxy - onlyNotInCatastrophicFailure - { - emit RewardDeposited(UNKNOWN_STAKING_POOL_ID, msg.value); - } - /// @dev Sets the Eth Vault. /// Note that only the contract owner can call this. /// @param ethVaultAddress Address of the Eth Vault. @@ -74,71 +54,23 @@ contract StakingPoolRewardVault is emit EthVaultChanged(ethVaultAddress); } - /// @dev Record a deposit for a pool. This deposit should be in the same transaction, - /// which is enforced by the staking contract. We do not enforce it here to save (a lot of) gas. - /// Note that this is only callable by the staking contract, and when - /// not in catastrophic failure mode. - /// @param poolId Unique Id of pool. - /// @param amount Amount in ETH to record. - /// @param operatorOnly Only attribute amount to operator. - /// @return operatorPortion Portion of amount attributed to the operator. - /// @return membersPortion Portion of amount attributed to the pool. - function recordDepositFor( - bytes32 poolId, - uint256 amount, - bool operatorOnly - ) + /// @dev Deposit an amount of ETH (`msg.value`) for `poolId` into the vault. + /// Note that this is only callable by the staking contract. + /// @param poolId that holds the ETH. + function depositFor(bytes32 poolId) external - onlyStakingProxy - returns ( - uint256 operatorPortion, - uint256 membersPortion - ) + payable + onlyStakingContract { - // update balance of pool - (operatorPortion, membersPortion) = _incrementPoolBalances(poolById[poolId], amount, operatorOnly); - return (operatorPortion, membersPortion); - } - - /// @dev Withdraw some amount in ETH of an operator's reward. - /// Note that this is only callable by the staking contract, and when - /// not in catastrophic failure mode. - /// @param poolId Unique Id of pool. - function transferOperatorBalanceToEthVault( - bytes32 poolId, - address operator, - uint256 amount - ) - external - onlyStakingProxy - { - if (amount == 0) { - return; - } - - // sanity check - sufficient balance? - uint256 operatorBalance = uint256(poolById[poolId].operatorBalance); - if (amount > operatorBalance) { - LibRichErrors.rrevert(LibStakingRichErrors.AmountExceedsBalanceOfPoolError( - amount, - poolById[poolId].operatorBalance - )); - } - - // update balance and transfer `amount` in ETH to staking contract - poolById[poolId].operatorBalance = operatorBalance.safeSub(amount).downcastToUint96(); - _transferToEthVault(operator, amount); - - // notify - emit RewardWithdrawnForOperator(poolId, amount); + balanceByPoolId[poolId] = balanceByPoolId[poolId].safeAdd(msg.value); + emit EthDepositedIntoVault(msg.sender, poolId, msg.value); } /// @dev Withdraw some amount in ETH of a pool member. - /// Note that this is only callable by the staking contract, and when - /// not in catastrophic failure mode. + /// Note that this is only callable by the staking contract. /// @param poolId Unique Id of pool. /// @param amount Amount in ETH to transfer. - function transferMemberBalanceToEthVault( + function transferToEthVault( bytes32 poolId, address member, uint256 amount @@ -146,160 +78,31 @@ contract StakingPoolRewardVault is external onlyStakingProxy { - if (amount == 0) { - return; + // sanity check on eth vault + IEthVault _ethVault = ethVault; + if (address(_ethVault) == address(0)) { + LibRichErrors.rrevert( + LibStakingRichErrors.EthVaultNotSetError() + ); } - // sanity check - sufficient balance? - uint256 membersBalance = uint256(poolById[poolId].membersBalance); - if (amount > membersBalance) { - LibRichErrors.rrevert(LibStakingRichErrors.AmountExceedsBalanceOfPoolError( - amount, - poolById[poolId].membersBalance - )); - } - - // update balance and transfer `amount` in ETH to staking contract - poolById[poolId].membersBalance = membersBalance.safeSub(amount).downcastToUint96(); - _transferToEthVault(member, amount); - - // notify - emit RewardWithdrawnForMember(poolId, amount); + // perform transfer + balanceByPoolId[poolId] = balanceByPoolId[poolId].safeSub(amount); + _ethVault.depositFor.value(amount)(member); + emit PoolRewardTransferredToEthVault( + poolId, + member, + amount + ); } - /// @dev Register a new staking pool. - /// Note that this is only callable by the staking contract, and when - /// not in catastrophic failure mode. - /// @param poolId Unique Id of pool. - /// @param operatorShare Fraction of rewards given to the pool operator, in ppm. - function registerStakingPool( - bytes32 poolId, - address payable operatorAddress, - uint32 operatorShare - ) - external - onlyStakingProxy - onlyNotInCatastrophicFailure - { - // operator share must be a valid fraction - if (operatorShare > PPM_DENOMINATOR) { - LibRichErrors.rrevert(LibStakingRichErrors.OperatorShareError( - LibStakingRichErrors.OperatorShareErrorCodes.OperatorShareTooLarge, - poolId, - operatorShare - )); - } - - // pool must not already exist - Pool storage pool = poolById[poolId]; - if (pool.operatorAddress != NIL_ADDRESS) { - LibRichErrors.rrevert(LibStakingRichErrors.PoolExistenceError( - poolId, - true - )); - } - - // initialize pool - pool.operatorAddress = operatorAddress; - pool.operatorShare = operatorShare; - - // notify - emit StakingPoolRegistered(poolId, operatorShare); - } - - /// @dev Decreases the operator share for the given pool (i.e. increases pool rewards for members). - /// Note that this is only callable by the staking contract, and will revert if the new operator - /// share value is greater than the old value. - /// @param poolId Unique Id of pool. - /// @param newOperatorShare The newly decreased percentage of any rewards owned by the operator. - function decreaseOperatorShare(bytes32 poolId, uint32 newOperatorShare) - external - onlyStakingProxy - onlyNotInCatastrophicFailure - { - uint32 oldOperatorShare = poolById[poolId].operatorShare; - - if (newOperatorShare >= oldOperatorShare) { - LibRichErrors.rrevert(LibStakingRichErrors.OperatorShareError( - LibStakingRichErrors.OperatorShareErrorCodes.CanOnlyDecreaseOperatorShare, - poolId, - newOperatorShare - )); - } else { - poolById[poolId].operatorShare = newOperatorShare; - emit OperatorShareDecreased(poolId, oldOperatorShare, newOperatorShare); - } - } - - /// @dev Returns the address of the operator of a given pool - /// @param poolId Unique id of pool - /// @return operatorAddress Operator of the pool - function operatorOf(bytes32 poolId) - external - view - returns (address payable) - { - return poolById[poolId].operatorAddress; - } - - /// @dev Returns the total balance of a pool. - /// @param poolId Unique Id of pool. + /// @dev Returns the balance in ETH of `poolId` /// @return Balance in ETH. function balanceOf(bytes32 poolId) external view returns (uint256) { - return poolById[poolId].operatorBalance + poolById[poolId].membersBalance; - } - - /// @dev Increments a balances in a Pool struct, splitting the input amount between the - /// pool operator and members of the pool based on the pool operator's share. - /// @param pool Pool struct with the balances to increment. - /// @param amount Amount to add to balance. - /// @param operatorOnly Only give this balance to the operator. - /// @return portion of amount given to operator and delegators, respectively. - function _incrementPoolBalances(Pool storage pool, uint256 amount, bool operatorOnly) - private - returns (uint256 operatorPortion, uint256 membersPortion) - { - // compute portions. One of the two must round down: the operator always receives the leftover from rounding. - operatorPortion = operatorOnly - ? amount - : LibMath.getPartialAmountCeil( - uint256(pool.operatorShare), - PPM_DENOMINATOR, - amount - ); - - membersPortion = amount.safeSub(operatorPortion); - - // compute new balances - uint256 newOperatorBalance = uint256(pool.operatorBalance).safeAdd(operatorPortion); - uint256 newMembersBalance = uint256(pool.membersBalance).safeAdd(membersPortion); - - // save new balances - pool.operatorBalance = newOperatorBalance.downcastToUint96(); - pool.membersBalance = newMembersBalance.downcastToUint96(); - - return ( - operatorPortion, - membersPortion - ); - } - - function _transferToEthVault(address from, uint256 amount) - private - { - // sanity check on eth vault - IEthVault ethVault_ = _ethVault; - if (address(ethVault_) == address(0)) { - LibRichErrors.rrevert( - LibStakingRichErrors.EthVaultNotSetError() - ); - } - - // perform xfer - ethVault_.depositFor.value(amount)(from); + return balanceByPoolId[poolId]; } } diff --git a/contracts/staking/contracts/test/TestStorageLayout.sol b/contracts/staking/contracts/test/TestStorageLayout.sol index 0a4ae57f1a..9469f275d1 100644 --- a/contracts/staking/contracts/test/TestStorageLayout.sol +++ b/contracts/staking/contracts/test/TestStorageLayout.sol @@ -79,7 +79,7 @@ contract TestStorageLayout is if sub(poolJoinedByMakerAddress_slot, slot) { revertIncorrectStorageSlot() } slot := add(slot, 1) - if sub(numMakersByPoolId_slot, slot) { revertIncorrectStorageSlot() } + if sub(poolById_slot, slot) { revertIncorrectStorageSlot() } slot := add(slot, 1) if sub(currentEpoch_slot, slot) { revertIncorrectStorageSlot() } diff --git a/contracts/staking/src/artifacts.ts b/contracts/staking/src/artifacts.ts index 809ded5ee2..165b9ea0f1 100644 --- a/contracts/staking/src/artifacts.ts +++ b/contracts/staking/src/artifacts.ts @@ -32,6 +32,8 @@ import * as MixinStake from '../generated-artifacts/MixinStake.json'; import * as MixinStakeBalances from '../generated-artifacts/MixinStakeBalances.json'; import * as MixinStakeStorage from '../generated-artifacts/MixinStakeStorage.json'; import * as MixinStakingPool from '../generated-artifacts/MixinStakingPool.json'; +import * as MixinStakingPoolMakers from '../generated-artifacts/MixinStakingPoolMakers.json'; +import * as MixinStakingPoolModifiers from '../generated-artifacts/MixinStakingPoolModifiers.json'; import * as MixinStakingPoolRewards from '../generated-artifacts/MixinStakingPoolRewards.json'; import * as MixinStakingPoolRewardVault from '../generated-artifacts/MixinStakingPoolRewardVault.json'; import * as MixinStorage from '../generated-artifacts/MixinStorage.json'; @@ -82,6 +84,8 @@ export const artifacts = { MixinStakeStorage: MixinStakeStorage as ContractArtifact, MixinCumulativeRewards: MixinCumulativeRewards as ContractArtifact, MixinStakingPool: MixinStakingPool as ContractArtifact, + MixinStakingPoolMakers: MixinStakingPoolMakers as ContractArtifact, + MixinStakingPoolModifiers: MixinStakingPoolModifiers as ContractArtifact, MixinStakingPoolRewardVault: MixinStakingPoolRewardVault as ContractArtifact, MixinStakingPoolRewards: MixinStakingPoolRewards as ContractArtifact, MixinParams: MixinParams as ContractArtifact, diff --git a/contracts/staking/src/wrappers.ts b/contracts/staking/src/wrappers.ts index 7da08f1979..bb9aee4280 100644 --- a/contracts/staking/src/wrappers.ts +++ b/contracts/staking/src/wrappers.ts @@ -30,6 +30,8 @@ export * from '../generated-wrappers/mixin_stake'; export * from '../generated-wrappers/mixin_stake_balances'; export * from '../generated-wrappers/mixin_stake_storage'; export * from '../generated-wrappers/mixin_staking_pool'; +export * from '../generated-wrappers/mixin_staking_pool_makers'; +export * from '../generated-wrappers/mixin_staking_pool_modifiers'; export * from '../generated-wrappers/mixin_staking_pool_reward_vault'; export * from '../generated-wrappers/mixin_staking_pool_rewards'; export * from '../generated-wrappers/mixin_storage'; diff --git a/contracts/staking/test/actors/finalizer_actor.ts b/contracts/staking/test/actors/finalizer_actor.ts index cd06f0961b..11c370d7d1 100644 --- a/contracts/staking/test/actors/finalizer_actor.ts +++ b/contracts/staking/test/actors/finalizer_actor.ts @@ -6,9 +6,9 @@ import { StakingApiWrapper } from '../utils/api_wrapper'; import { MemberBalancesByPoolId, MembersByPoolId, + OperatorBalanceByPoolId, OperatorByPoolId, OperatorShareByPoolId, - RewardVaultBalance, RewardVaultBalanceByPoolId, } from '../utils/types'; @@ -21,8 +21,6 @@ interface Reward { export class FinalizerActor extends BaseActor { private readonly _poolIds: string[]; - // @TODO (hysz): this will be used later to liquidate the reward vault. - // tslint:disable-next-line no-unused-variable private readonly _operatorByPoolId: OperatorByPoolId; private readonly _membersByPoolId: MembersByPoolId; @@ -44,9 +42,14 @@ export class FinalizerActor extends BaseActor { const operatorShareByPoolId = await this._getOperatorShareByPoolIdAsync(this._poolIds); const rewardVaultBalanceByPoolId = await this._getRewardVaultBalanceByPoolIdAsync(this._poolIds); const memberBalancesByPoolId = await this._getMemberBalancesByPoolIdAsync(this._membersByPoolId); + const operatorBalanceByPoolId = await this._getOperatorBalanceByPoolIdAsync(this._operatorByPoolId); // compute expected changes - const expectedRewardVaultBalanceByPoolId = await this._computeExpectedRewardVaultBalanceAsyncByPoolIdAsync( + const [ + expectedOperatorBalanceByPoolId, + expectedRewardVaultBalanceByPoolId, + ] = await this._computeExpectedRewardVaultBalanceAsyncByPoolIdAsync( rewards, + operatorBalanceByPoolId, rewardVaultBalanceByPoolId, operatorShareByPoolId, ); @@ -70,6 +73,11 @@ export class FinalizerActor extends BaseActor { expect(finalMemberBalancesByPoolId, 'final delegator balances in reward vault').to.be.deep.equal( expectedMemberBalancesByPoolId, ); + // assert operator balances + const finalOperatorBalanceByPoolId = await this._getOperatorBalanceByPoolIdAsync(this._operatorByPoolId); + expect(finalOperatorBalanceByPoolId, 'final operator balances in eth vault').to.be.deep.equal( + expectedOperatorBalanceByPoolId, + ); } private async _computeExpectedMemberBalancesByPoolIdAsync( @@ -125,28 +133,35 @@ export class FinalizerActor extends BaseActor { private async _computeExpectedRewardVaultBalanceAsyncByPoolIdAsync( rewards: Reward[], + operatorBalanceByPoolId: OperatorBalanceByPoolId, rewardVaultBalanceByPoolId: RewardVaultBalanceByPoolId, operatorShareByPoolId: OperatorShareByPoolId, - ): Promise { + ): Promise<[RewardVaultBalanceByPoolId, OperatorBalanceByPoolId]> { + const expectedOperatorBalanceByPoolId = _.cloneDeep(operatorBalanceByPoolId); const expectedRewardVaultBalanceByPoolId = _.cloneDeep(rewardVaultBalanceByPoolId); for (const reward of rewards) { const operatorShare = operatorShareByPoolId[reward.poolId]; - expectedRewardVaultBalanceByPoolId[reward.poolId] = await this._computeExpectedRewardVaultBalanceAsync( + [ + expectedOperatorBalanceByPoolId[reward.poolId], + expectedRewardVaultBalanceByPoolId[reward.poolId], + ] = await this._computeExpectedRewardVaultBalanceAsync( reward.poolId, reward.reward, + expectedOperatorBalanceByPoolId[reward.poolId], expectedRewardVaultBalanceByPoolId[reward.poolId], operatorShare, ); } - return expectedRewardVaultBalanceByPoolId; + return [expectedOperatorBalanceByPoolId, expectedRewardVaultBalanceByPoolId]; } private async _computeExpectedRewardVaultBalanceAsync( poolId: string, reward: BigNumber, - rewardVaultBalance: RewardVaultBalance, + operatorBalance: BigNumber, + rewardVaultBalance: BigNumber, operatorShare: BigNumber, - ): Promise { + ): Promise<[BigNumber, BigNumber]> { const totalStakeDelegatedToPool = (await this._stakingApiWrapper.stakingContract.getTotalStakeDelegatedToPool.callAsync( poolId, )).currentEpochBalance; @@ -154,19 +169,26 @@ export class FinalizerActor extends BaseActor { ? reward : reward.times(operatorShare).dividedToIntegerBy(100); const membersPortion = reward.minus(operatorPortion); - return { - poolBalance: rewardVaultBalance.poolBalance.plus(reward), - operatorBalance: rewardVaultBalance.operatorBalance.plus(operatorPortion), - membersBalance: rewardVaultBalance.membersBalance.plus(membersPortion), - }; + return [operatorBalance.plus(operatorPortion), rewardVaultBalance.plus(membersPortion)]; + } + + private async _getOperatorBalanceByPoolIdAsync( + operatorByPoolId: OperatorByPoolId, + ): Promise { + const operatorBalanceByPoolId: OperatorBalanceByPoolId = {}; + for (const poolId of Object.keys(operatorByPoolId)) { + operatorBalanceByPoolId[poolId] = await this._stakingApiWrapper.ethVaultContract.balanceOf.callAsync( + operatorByPoolId[poolId], + ); + } + return operatorBalanceByPoolId; } private async _getOperatorShareByPoolIdAsync(poolIds: string[]): Promise { const operatorShareByPoolId: OperatorShareByPoolId = {}; for (const poolId of poolIds) { - const pool = await this._stakingApiWrapper.rewardVaultContract.poolById.callAsync(poolId); - const operatorShare = new BigNumber(pool[0]); - operatorShareByPoolId[poolId] = operatorShare; + const pool = await this._stakingApiWrapper.stakingContract.getStakingPool.callAsync(poolId); + operatorShareByPoolId[poolId] = new BigNumber(pool.operatorShare); } return operatorShareByPoolId; } @@ -174,19 +196,10 @@ export class FinalizerActor extends BaseActor { private async _getRewardVaultBalanceByPoolIdAsync(poolIds: string[]): Promise { const rewardVaultBalanceByPoolId: RewardVaultBalanceByPoolId = {}; for (const poolId of poolIds) { - rewardVaultBalanceByPoolId[poolId] = await this._getRewardVaultBalanceAsync(poolId); + rewardVaultBalanceByPoolId[poolId] = await this._stakingApiWrapper.rewardVaultContract.balanceOf.callAsync( + poolId, + ); } return rewardVaultBalanceByPoolId; } - - private async _getRewardVaultBalanceAsync(poolId: string): Promise { - const pool = await this._stakingApiWrapper.rewardVaultContract.poolById.callAsync(poolId); - const operatorBalance = pool[1]; - const membersBalance = pool[2]; - return { - poolBalance: operatorBalance.plus(membersBalance), - operatorBalance, - membersBalance, - }; - } } diff --git a/contracts/staking/test/actors/pool_operator_actor.ts b/contracts/staking/test/actors/pool_operator_actor.ts index 211444f323..a95adf8117 100644 --- a/contracts/staking/test/actors/pool_operator_actor.ts +++ b/contracts/staking/test/actors/pool_operator_actor.ts @@ -103,8 +103,7 @@ export class PoolOperatorActor extends BaseActor { } await txReceiptPromise; // Check operator share - const pool = await this._stakingApiWrapper.rewardVaultContract.poolById.callAsync(poolId); - const decreasedOperatorShare = new BigNumber(pool[0]); - expect(decreasedOperatorShare, 'updated operator share').to.be.bignumber.equal(newOperatorShare); + const pool = await this._stakingApiWrapper.stakingContract.getStakingPool.callAsync(poolId); + expect(pool.operatorShare, 'updated operator share').to.be.bignumber.equal(newOperatorShare); } } diff --git a/contracts/staking/test/rewards_test.ts b/contracts/staking/test/rewards_test.ts index a0d8194a0b..cf536689e0 100644 --- a/contracts/staking/test/rewards_test.ts +++ b/contracts/staking/test/rewards_test.ts @@ -77,7 +77,6 @@ blockchainTests.resets('Testing Rewards', env => { stakerRewardVaultBalance_2?: BigNumber; stakerEthVaultBalance_2?: BigNumber; // operator - operatorRewardVaultBalance?: BigNumber; operatorEthVaultBalance?: BigNumber; // undivided balance in reward pool poolRewardVaultBalance?: BigNumber; @@ -104,10 +103,6 @@ blockchainTests.resets('Testing Rewards', env => { ? _expectedEndBalances.stakerEthVaultBalance_2 : ZERO, // operator - operatorRewardVaultBalance: - _expectedEndBalances.operatorRewardVaultBalance !== undefined - ? _expectedEndBalances.operatorRewardVaultBalance - : ZERO, operatorEthVaultBalance: _expectedEndBalances.operatorEthVaultBalance !== undefined ? _expectedEndBalances.operatorEthVaultBalance @@ -117,10 +112,6 @@ blockchainTests.resets('Testing Rewards', env => { _expectedEndBalances.poolRewardVaultBalance !== undefined ? _expectedEndBalances.poolRewardVaultBalance : ZERO, - membersRewardVaultBalance: - _expectedEndBalances.membersRewardVaultBalance !== undefined - ? _expectedEndBalances.membersRewardVaultBalance - : ZERO, }; const pool = await stakingApiWrapper.rewardVaultContract.poolById.callAsync(poolId); const operatorBalance = pool[1]; @@ -141,6 +132,8 @@ blockchainTests.resets('Testing Rewards', env => { stakingApiWrapper.ethVaultContract.balanceOf.callAsync(stakers[1].getOwner()), // operator stakingApiWrapper.ethVaultContract.balanceOf.callAsync(poolOperator), + // undivided balance in reward pool + stakingApiWrapper.rewardVaultContract.balanceOf.callAsync(poolId), ]); expect(finalEndBalancesAsArray[0], 'stakerRewardVaultBalance_1').to.be.bignumber.equal( expectedEndBalances.stakerRewardVaultBalance_1, @@ -154,19 +147,12 @@ blockchainTests.resets('Testing Rewards', env => { expect(finalEndBalancesAsArray[3], 'stakerEthVaultBalance_2').to.be.bignumber.equal( expectedEndBalances.stakerEthVaultBalance_2, ); - expect(finalEndBalancesAsArray[4], 'operatorEthVaultBalance').to.be.bignumber.equal( expectedEndBalances.operatorEthVaultBalance, ); - expect(poolBalances.operatorBalance, 'operatorRewardVaultBalance').to.be.bignumber.equal( - expectedEndBalances.operatorRewardVaultBalance, - ); - expect(poolBalances.poolBalance, 'poolRewardVaultBalance').to.be.bignumber.equal( + expect(finalEndBalancesAsArray[5], 'poolRewardVaultBalance').to.be.bignumber.equal( expectedEndBalances.poolRewardVaultBalance, ); - expect(poolBalances.membersBalance, 'membersRewardVaultBalance').to.be.bignumber.equal( - expectedEndBalances.membersRewardVaultBalance, - ); }; const payProtocolFeeAndFinalize = async (_fee?: BigNumber) => { const fee = _fee !== undefined ? _fee : ZERO; @@ -207,8 +193,7 @@ blockchainTests.resets('Testing Rewards', env => { await payProtocolFeeAndFinalize(reward); // sanity check final balances - all zero await validateEndBalances({ - operatorRewardVaultBalance: reward, - poolRewardVaultBalance: reward, + operatorEthVaultBalance: reward, }); }); it('Operator should receive entire reward if no delegators in their pool (staker joins this epoch but is active next epoch)', async () => { @@ -225,8 +210,7 @@ blockchainTests.resets('Testing Rewards', env => { await payProtocolFeeAndFinalize(reward); // sanity check final balances await validateEndBalances({ - operatorRewardVaultBalance: reward, - poolRewardVaultBalance: reward, + operatorEthVaultBalance: reward, }); }); it('Should give pool reward to delegator', async () => { @@ -533,8 +517,7 @@ blockchainTests.resets('Testing Rewards', env => { // sanity check final balances await validateEndBalances({ stakerEthVaultBalance_1: rewardForDelegator, - poolRewardVaultBalance: rewardNotForDelegator, - operatorRewardVaultBalance: rewardNotForDelegator, + operatorEthVaultBalance: rewardNotForDelegator, }); }); it('Should stop collecting rewards after undelegating, after several epochs', async () => { @@ -578,8 +561,7 @@ blockchainTests.resets('Testing Rewards', env => { // sanity check final balances await validateEndBalances({ stakerEthVaultBalance_1: rewardForDelegator, - poolRewardVaultBalance: totalRewardsNotForDelegator, - operatorRewardVaultBalance: totalRewardsNotForDelegator, + operatorEthVaultBalance: totalRewardsNotForDelegator, }); }); it('Should collect fees correctly when leaving and returning to a pool', async () => { @@ -619,9 +601,8 @@ blockchainTests.resets('Testing Rewards', env => { await validateEndBalances({ stakerRewardVaultBalance_1: rewardsForDelegator[1], stakerEthVaultBalance_1: rewardsForDelegator[0], - operatorRewardVaultBalance: rewardNotForDelegator, - poolRewardVaultBalance: rewardNotForDelegator.plus(rewardsForDelegator[1]), - membersRewardVaultBalance: rewardsForDelegator[1], + operatorEthVaultBalance: rewardNotForDelegator, + poolRewardVaultBalance: rewardsForDelegator[1], }); }); it('Should collect fees correctly when re-delegating after un-delegating', async () => { @@ -657,9 +638,33 @@ blockchainTests.resets('Testing Rewards', env => { await validateEndBalances({ stakerRewardVaultBalance_1: ZERO, stakerEthVaultBalance_1: rewardForDelegator, - operatorRewardVaultBalance: ZERO, + operatorEthVaultBalance: ZERO, + poolRewardVaultBalance: ZERO, + }); + }); + it('Should withdraw delegator rewards to eth vault when calling `syncDelegatorRewards`', async () => { + // first staker delegates (epoch 0) + const rewardForDelegator = toBaseUnitAmount(10); + const stakeAmount = toBaseUnitAmount(4); + await stakers[0].stakeAsync(stakeAmount); + await stakers[0].moveStakeAsync( + new StakeInfo(StakeStatus.Active), + new StakeInfo(StakeStatus.Delegated, poolId), + stakeAmount, + ); + // skip epoch, so staker can start earning rewards + await payProtocolFeeAndFinalize(); + // this should go to the delegator + await payProtocolFeeAndFinalize(rewardForDelegator); + await stakingApiWrapper.stakingContract.syncDelegatorRewards.awaitTransactionSuccessAsync(poolId, { + from: stakers[0].getOwner(), + }); + // sanity check final balances + await validateEndBalances({ + stakerRewardVaultBalance_1: ZERO, + stakerEthVaultBalance_1: rewardForDelegator, + operatorEthVaultBalance: ZERO, poolRewardVaultBalance: ZERO, - membersRewardVaultBalance: ZERO, }); }); }); diff --git a/contracts/staking/test/utils/api_wrapper.ts b/contracts/staking/test/utils/api_wrapper.ts index bbc4dca8db..253d25c47e 100644 --- a/contracts/staking/test/utils/api_wrapper.ts +++ b/contracts/staking/test/utils/api_wrapper.ts @@ -208,6 +208,16 @@ export async function deployAndConfigureContractsAsync( rewardVaultContract.address, zrxVaultContract.address, ); + // set eth vault in staking contract + const setEthVaultCalldata = stakingContract.setEthVault.getABIEncodedTransactionData(ethVaultContract.address); + const setEthVaultCalldataTxData = { + from: ownerAddress, + to: stakingProxyContract.address, + data: setEthVaultCalldata, + }; + await env.web3Wrapper.awaitTransactionSuccessAsync( + await env.web3Wrapper.sendTransactionAsync(setEthVaultCalldataTxData), + ); // configure erc20 proxy to accept calls from zrx vault await erc20ProxyContract.addAuthorizedAddress.awaitTransactionSuccessAsync(zrxVaultContract.address); diff --git a/contracts/staking/test/utils/types.ts b/contracts/staking/test/utils/types.ts index e5dac3b91c..70e58a89d3 100644 --- a/contracts/staking/test/utils/types.ts +++ b/contracts/staking/test/utils/types.ts @@ -90,20 +90,18 @@ export interface StakeBalances { totalDelegatedStakeByPool: StakeBalanceByPool; } -export interface RewardVaultBalance { - poolBalance: BigNumber; - operatorBalance: BigNumber; - membersBalance: BigNumber; -} - export interface RewardVaultBalanceByPoolId { - [key: string]: RewardVaultBalance; + [key: string]: BigNumber; } export interface OperatorShareByPoolId { [key: string]: BigNumber; } +export interface OperatorBalanceByPoolId { + [key: string]: BigNumber; +} + export interface BalanceByOwner { [key: string]: BigNumber; } diff --git a/contracts/staking/test/vaults_test.ts b/contracts/staking/test/vaults_test.ts deleted file mode 100644 index 6efd17bad8..0000000000 --- a/contracts/staking/test/vaults_test.ts +++ /dev/null @@ -1,57 +0,0 @@ -import { ERC20Wrapper } from '@0x/contracts-asset-proxy'; -import { blockchainTests, expect } from '@0x/contracts-test-utils'; -import { StakingRevertErrors } from '@0x/order-utils'; -import * as _ from 'lodash'; - -import { deployAndConfigureContractsAsync, StakingApiWrapper } from './utils/api_wrapper'; - -// tslint:disable:no-unnecessary-type-assertion -blockchainTests('Staking Vaults', env => { - // tokens & addresses - let accounts: string[]; - let owner: string; - let users: string[]; - // wrappers - let stakingApiWrapper: StakingApiWrapper; - let erc20Wrapper: ERC20Wrapper; - // tests - before(async () => { - // create accounts - accounts = await env.getAccountAddressesAsync(); - owner = accounts[0]; - users = accounts.slice(1); - // set up ERC20Wrapper - erc20Wrapper = new ERC20Wrapper(env.provider, accounts, owner); - // deploy staking contracts - stakingApiWrapper = await deployAndConfigureContractsAsync(env, owner, erc20Wrapper); - }); - blockchainTests.resets('Reward Vault', () => { - // @TODO (hysz): Resolve non-EOA transaction issue so that this test can be unskipped - it.skip('basic management', async () => { - // 1 setup test parameters - const poolOperator = users[0]; - const operatorShare = 39; - const poolId = await stakingApiWrapper.utils.createStakingPoolAsync(poolOperator, operatorShare, true); - const notStakingContractAddress = poolOperator; - // should fail to create pool if it already exists - let revertError = new StakingRevertErrors.PoolExistenceError(poolId, true); - let tx = stakingApiWrapper.rewardVaultContract.registerStakingPool.awaitTransactionSuccessAsync( - poolId, - poolOperator, - operatorShare, - { from: stakingApiWrapper.stakingContractAddress }, - ); - await expect(tx).to.revertWith(revertError); - // should fail to create a pool from an address other than the staking contract - revertError = new StakingRevertErrors.OnlyCallableByStakingContractError(notStakingContractAddress); - tx = stakingApiWrapper.rewardVaultContract.registerStakingPool.awaitTransactionSuccessAsync( - poolId, - poolOperator, - operatorShare, - { from: notStakingContractAddress }, - ); - await expect(tx).to.revertWith(revertError); - }); - }); -}); -// tslint:enable:no-unnecessary-type-assertion diff --git a/contracts/staking/tsconfig.json b/contracts/staking/tsconfig.json index 16769c81fb..6e5e2dfa1d 100644 --- a/contracts/staking/tsconfig.json +++ b/contracts/staking/tsconfig.json @@ -30,6 +30,8 @@ "generated-artifacts/MixinStakeBalances.json", "generated-artifacts/MixinStakeStorage.json", "generated-artifacts/MixinStakingPool.json", + "generated-artifacts/MixinStakingPoolMakers.json", + "generated-artifacts/MixinStakingPoolModifiers.json", "generated-artifacts/MixinStakingPoolRewardVault.json", "generated-artifacts/MixinStakingPoolRewards.json", "generated-artifacts/MixinStorage.json", From db97fe81647e1700b1664a9a60bdf9a76f8e0b5f Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 16 Sep 2019 22:02:06 -0700 Subject: [PATCH 2/6] updated changelog --- contracts/staking/CHANGELOG.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contracts/staking/CHANGELOG.json b/contracts/staking/CHANGELOG.json index 5272f915f6..2a953327fb 100644 --- a/contracts/staking/CHANGELOG.json +++ b/contracts/staking/CHANGELOG.json @@ -45,6 +45,10 @@ { "note": "Reference counting for cumulative rewards.", "pr": 2154 + }, + { + "note": "Refactored Staking Reward Vault. Moved pool management logic into staking contract.", + "pr": 2156 } ] } From 768387fea9d0da40ce13d13b055c0121faad398d Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 17 Sep 2019 14:31:38 -0700 Subject: [PATCH 3/6] Rebased and addressed PR comments --- contracts/staking/contracts/src/Staking.sol | 2 +- .../contracts/src/immutable/MixinStorage.sol | 2 +- .../src/interfaces/IStakingEvents.sol | 6 +- .../interfaces/IStakingPoolRewardVault.sol | 21 ++--- .../contracts/src/interfaces/IStructs.sol | 2 +- .../src/libs/LibStakingRichErrors.sol | 8 +- .../contracts/src/stake/MixinStake.sol | 29 +++---- .../src/staking_pools/MixinEthVault.sol | 81 ------------------- .../src/staking_pools/MixinStakingPool.sol | 43 ++++++---- .../staking_pools/MixinStakingPoolMakers.sol | 15 +--- .../MixinStakingPoolModifiers.sol | 32 ++------ .../MixinStakingPoolRewardVault.sol | 25 ++---- .../staking_pools/MixinStakingPoolRewards.sol | 36 +++++++-- .../src/vaults/StakingPoolRewardVault.sol | 40 +++------ .../test/actors/pool_operator_actor.ts | 4 +- contracts/staking/test/pools_test.ts | 12 +-- contracts/staking/test/rewards_test.ts | 8 +- contracts/staking/test/utils/api_wrapper.ts | 12 --- 18 files changed, 125 insertions(+), 253 deletions(-) delete mode 100644 contracts/staking/contracts/src/staking_pools/MixinEthVault.sol diff --git a/contracts/staking/contracts/src/Staking.sol b/contracts/staking/contracts/src/Staking.sol index 919262325a..21b7f7bb97 100644 --- a/contracts/staking/contracts/src/Staking.sol +++ b/contracts/staking/contracts/src/Staking.sol @@ -29,8 +29,8 @@ import "./fees/MixinExchangeFees.sol"; contract Staking is IStaking, MixinParams, - MixinStake, MixinStakingPool, + MixinStake, MixinExchangeFees { // this contract can receive ETH diff --git a/contracts/staking/contracts/src/immutable/MixinStorage.sol b/contracts/staking/contracts/src/immutable/MixinStorage.sol index 244541f606..62756e5e34 100644 --- a/contracts/staking/contracts/src/immutable/MixinStorage.sol +++ b/contracts/staking/contracts/src/immutable/MixinStorage.sol @@ -78,7 +78,7 @@ contract MixinStorage is mapping (address => IStructs.MakerPoolJoinStatus) public poolJoinedByMakerAddress; // mapping from Pool Id to Pool - mapping (bytes32 => IStructs.Pool) internal poolById; + mapping (bytes32 => IStructs.Pool) public poolById; // current epoch uint256 public currentEpoch = INITIAL_EPOCH; diff --git a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol index c7133e71f4..eeb4f5f969 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol @@ -105,11 +105,11 @@ interface IStakingEvents { /// @dev Emitted by MixinStakingPool when a new pool is created. /// @param poolId Unique id generated for pool. - /// @param operatorAddress Address of creator/operator of pool. + /// @param operator The operator (creator) of pool. /// @param operatorShare The share of rewards given to the operator, in ppm. event StakingPoolCreated( bytes32 poolId, - address operatorAddress, + address operator, uint32 operatorShare ); @@ -148,7 +148,7 @@ interface IStakingEvents { /// @param oldOperatorShare Previous share of rewards owned by operator. /// @param newOperatorShare Newly decreased share of rewards owned by operator. event OperatorShareDecreased( - bytes32 poolId, + bytes32 indexed poolId, uint32 oldOperatorShare, uint32 newOperatorShare ); diff --git a/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol b/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol index cbecee1f36..2653ce4154 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol @@ -38,23 +38,11 @@ interface IStakingPoolRewardVault { /// @param member of the pool. /// @param poolId The pool the reward was deposited for. event PoolRewardTransferredToEthVault( - bytes32 poolId, - address member, + bytes32 indexed poolId, + address indexed member, uint256 amount ); - /// @dev Emitted when the eth vault is changed - /// @param newEthVault address of new Eth vault. - event EthVaultChanged( - address newEthVault - ); - - /// @dev Sets the Eth Vault. - /// Note that only the contract owner can call this. - /// @param ethVaultAddress Address of the Eth Vault. - function setEthVault(address ethVaultAddress) - external; - /// @dev Deposit an amount of ETH (`msg.value`) for `poolId` into the vault. /// Note that this is only callable by the staking contract. /// @param poolId that owns the ETH. @@ -65,11 +53,14 @@ interface IStakingPoolRewardVault { /// @dev Withdraw some amount in ETH of a pool member. /// Note that this is only callable by the staking contract. /// @param poolId Unique Id of pool. + /// @param member of pool to transfer funds to. /// @param amount Amount in ETH to transfer. + /// @param ethVaultAddress address of Eth Vault to send rewards to. function transferToEthVault( bytes32 poolId, address member, - uint256 amount + uint256 amount, + address ethVaultAddress ) external; diff --git a/contracts/staking/contracts/src/interfaces/IStructs.sol b/contracts/staking/contracts/src/interfaces/IStructs.sol index 46880b473f..cdd4aec185 100644 --- a/contracts/staking/contracts/src/interfaces/IStructs.sol +++ b/contracts/staking/contracts/src/interfaces/IStructs.sol @@ -96,7 +96,7 @@ interface IStructs { IStructs.Fraction cumulativeReward; } - /// @dev Holds the balances and other data for a staking pool. + /// @dev Holds the metadata for a staking pool. /// @param initialzed 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. diff --git a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol index 885e5d58a8..b0026eed36 100644 --- a/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol +++ b/contracts/staking/contracts/src/libs/LibStakingRichErrors.sol @@ -233,7 +233,7 @@ library LibStakingRichErrors { function OnlyCallableByPoolOperatorError( address senderAddress, - address poolOperatorAddress + address operator ) internal pure @@ -242,13 +242,13 @@ library LibStakingRichErrors { return abi.encodeWithSelector( ONLY_CALLABLE_BY_POOL_OPERATOR_ERROR_SELECTOR, senderAddress, - poolOperatorAddress + operator ); } function OnlyCallableByPoolOperatorOrMakerError( address senderAddress, - address poolOperatorAddress, + address operator, address makerAddress ) internal @@ -258,7 +258,7 @@ library LibStakingRichErrors { return abi.encodeWithSelector( ONLY_CALLABLE_BY_POOL_OPERATOR_OR_MAKER_ERROR_SELECTOR, senderAddress, - poolOperatorAddress, + operator, makerAddress ); } diff --git a/contracts/staking/contracts/src/stake/MixinStake.sol b/contracts/staking/contracts/src/stake/MixinStake.sol index 9969652ac2..908cf9ba32 100644 --- a/contracts/staking/contracts/src/stake/MixinStake.sol +++ b/contracts/staking/contracts/src/stake/MixinStake.sol @@ -21,12 +21,17 @@ pragma experimental ABIEncoderV2; import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; import "../staking_pools/MixinStakingPoolRewards.sol"; +import "../staking_pools/MixinStakingPool.sol"; import "../libs/LibStakingRichErrors.sol"; /// @dev This mixin contains logic for managing ZRX tokens and Stake. contract MixinStake is - MixinStakingPoolRewards + MixinStorage, + MixinStakingPoolMakers, + MixinStakingPoolRewards, + MixinStakingPool + { using LibSafeMath for uint256; @@ -162,15 +167,8 @@ contract MixinStake is ) private { - // revert if pool with given poolId doesn't exist - if (rewardVault.operatorOf(poolId) == NIL_ADDRESS) { - LibRichErrors.rrevert( - LibStakingRichErrors.PoolExistenceError( - poolId, - false - ) - ); - } + // sanity check the pool we're delegating to exists + _assertStakingPoolExists(poolId); // cache amount delegated to pool by owner IStructs.StoredBalance memory initDelegatedStakeToPoolByOwner = _loadUnsyncedBalance(_delegatedStakeToPoolByOwner[owner][poolId]); @@ -197,15 +195,8 @@ contract MixinStake is ) private { - // revert if pool with given poolId doesn't exist - if (rewardVault.operatorOf(poolId) == NIL_ADDRESS) { - LibRichErrors.rrevert( - LibStakingRichErrors.PoolExistenceError( - poolId, - false - ) - ); - } + // sanity check the pool we're undelegating from exists + _assertStakingPoolExists(poolId); // cache amount delegated to pool by owner IStructs.StoredBalance memory initDelegatedStakeToPoolByOwner = _loadUnsyncedBalance(_delegatedStakeToPoolByOwner[owner][poolId]); diff --git a/contracts/staking/contracts/src/staking_pools/MixinEthVault.sol b/contracts/staking/contracts/src/staking_pools/MixinEthVault.sol deleted file mode 100644 index 1a022a96b9..0000000000 --- a/contracts/staking/contracts/src/staking_pools/MixinEthVault.sol +++ /dev/null @@ -1,81 +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 "../interfaces/IStakingEvents.sol"; -import "../interfaces/IEthVault.sol"; -import "../immutable/MixinStorage.sol"; - - -/// @dev This mixin contains logic for managing and interfacing with the Eth Vault. -/// (see vaults/EthVault.sol). -contract MixinEthVault is - IStakingEvents, - MixinConstants, - Ownable, - MixinStorage -{ - - /// @dev Set the Eth Vault. - /// @param ethVaultAddress Address of the Eth Vault. - function setEthVault(address ethVaultAddress) - external - onlyOwner - { - ethVault = IEthVault(ethVaultAddress); - } - - /// @dev Return the current Eth Vault - /// @return Eth Vault - function getEthVault() - public - view - returns (address) - { - return address(ethVault); - } - - /// @dev Transfers operator reward to the ETH vault. - /// @param poolId Unique Id of pool to transfer reward for, - /// @param operator of the pool. - /// @param amount of ETH to transfer. - function _transferOperatorRewardToEthVault( - bytes32 poolId, - address operator, - uint256 amount - ) - internal - { - // sanity check on eth vault - IEthVault _ethVault = ethVault; - if (address(_ethVault) == address(0)) { - LibRichErrors.rrevert( - LibStakingRichErrors.EthVaultNotSetError() - ); - } - - // perform transfer - _ethVault.depositFor.value(amount)(operator); - emit OperatorRewardTransferredToEthVault( - poolId, - operator, - amount - ); - } -} diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol index 62719886ca..2b3a134b0b 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol @@ -28,6 +28,8 @@ import "./MixinStakingPoolMakers.sol"; contract MixinStakingPool is + MixinStorage, + MixinStakingPoolMakers, MixinStakingPoolRewards { using LibSafeMath for uint256; @@ -85,10 +87,10 @@ contract MixinStakingPool is external { // load pool and assert that we can decrease - IStructs.Pool memory pool = poolById[poolId]; + uint32 currentOperatorShare = poolById[poolId].operatorShare; _assertNewOperatorShare( poolId, - pool.operatorShare, + currentOperatorShare, newOperatorShare ); @@ -96,22 +98,11 @@ contract MixinStakingPool is poolById[poolId].operatorShare = newOperatorShare; emit OperatorShareDecreased( poolId, - pool.operatorShare, + currentOperatorShare, newOperatorShare ); } - /// @dev Returns the staking pool with `poolId` - /// @param poolId Unique id of pool - /// @return operator Operator of the pool - function getStakingPool(bytes32 poolId) - external - view - returns (IStructs.Pool memory) - { - return poolById[poolId]; - } - /// @dev Returns the unique id that will be assigned to the next pool that is created. /// @return Pool id. function getNextStakingPoolId() @@ -122,6 +113,14 @@ contract MixinStakingPool is return nextPoolId; } + function getStakingPool(bytes32 poolId) + public + view + returns (IStructs.Pool memory) + { + return poolById[poolId]; + } + /// @dev Computes the unique id that comes after the input pool id. /// @param poolId Unique id of pool. /// @return Next pool id after input pool. @@ -133,6 +132,22 @@ contract MixinStakingPool is return bytes32(uint256(poolId).safeAdd(POOL_ID_INCREMENT_AMOUNT)); } + function _assertStakingPoolExists(bytes32 poolId) + internal + view + returns (bool) + { + if (poolById[poolId].operator == NIL_ADDRESS) { + // we use the pool's operator as a proxy for its existence + LibRichErrors.rrevert( + LibStakingRichErrors.PoolExistenceError( + poolId, + false + ) + ); + } + } + function _assertNewOperatorShare( bytes32 poolId, uint32 currentOperatorShare, diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol index f9c5de4031..71e01399e7 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol @@ -19,34 +19,27 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; -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/IStructs.sol"; import "../interfaces/IStakingEvents.sol"; -import "../immutable/MixinConstants.sol"; import "../immutable/MixinStorage.sol"; -import "./MixinStakingPoolRewards.sol"; import "./MixinStakingPoolModifiers.sol"; /// @dev This mixin contains logic for staking pools. contract MixinStakingPoolMakers is IStakingEvents, - MixinConstants, - Ownable, MixinStorage, - MixinZrxVault, - MixinStakingPoolRewardVault, - MixinScheduler, - MixinStakeStorage, - MixinStakeBalances, - MixinStakingPoolRewards, MixinStakingPoolModifiers { using LibSafeMath for uint256; + using LibSafeDowncast for uint256; + /// @dev Allows caller to join a staking pool if already assigned. + /// @param poolId Unique id of pool. function joinStakingPoolAsMaker( bytes32 poolId ) diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol index 5f730e3d4b..71be9b68dc 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol @@ -19,39 +19,21 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; -import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; -import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; -import "../libs/LibStakingRichErrors.sol"; -import "../interfaces/IStructs.sol"; -import "../interfaces/IStakingEvents.sol"; -import "../immutable/MixinConstants.sol"; import "../immutable/MixinStorage.sol"; -import "./MixinStakingPoolRewards.sol"; contract MixinStakingPoolModifiers is - IStakingEvents, - MixinConstants, - Ownable, - MixinStorage, - MixinZrxVault, - MixinStakingPoolRewardVault, - MixinScheduler, - MixinStakeStorage, - MixinStakeBalances, - MixinStakingPoolRewards + MixinStorage { - using LibSafeMath for uint256; - /// @dev Asserts that the sender is the operator of the input pool. /// @param poolId Pool sender must be operator of. modifier onlyStakingPoolOperator(bytes32 poolId) { - address poolOperator = poolById[poolId].operator; - if (msg.sender != poolOperator) { + address operator = poolById[poolId].operator; + if (msg.sender != operator) { LibRichErrors.rrevert(LibStakingRichErrors.OnlyCallableByPoolOperatorError( msg.sender, - poolOperator + operator )); } @@ -62,15 +44,15 @@ contract MixinStakingPoolModifiers is /// @param poolId Pool sender must be operator of. /// @param makerAddress Address of a maker in the pool. modifier onlyStakingPoolOperatorOrMaker(bytes32 poolId, address makerAddress) { - address poolOperator; + address operator; if ( msg.sender != makerAddress && - msg.sender != (poolOperator = poolById[poolId].operator) + msg.sender != (operator = poolById[poolId].operator) ) { LibRichErrors.rrevert( LibStakingRichErrors.OnlyCallableByPoolOperatorOrMakerError( msg.sender, - poolOperator, + operator, makerAddress ) ); diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol index 90f2e38c1d..703e115aff 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol @@ -58,15 +58,8 @@ contract MixinStakingPoolRewardVault is function _depositIntoStakingPoolRewardVault(uint256 amount) internal { - // cast to payable and sanity check + // cast to payable then transfer address payable rewardVaultAddress = address(uint160(address(rewardVault))); - if (rewardVaultAddress == NIL_ADDRESS) { - LibRichErrors.rrevert( - LibStakingRichErrors.RewardVaultNotSetError() - ); - } - - // perform transfer rewardVaultAddress.transfer(amount); } @@ -81,15 +74,11 @@ contract MixinStakingPoolRewardVault is ) internal { - // sanity check - IStakingPoolRewardVault _rewardVault = rewardVault; - if (address(_rewardVault) == NIL_ADDRESS) { - LibRichErrors.rrevert( - LibStakingRichErrors.RewardVaultNotSetError() - ); - } - - // perform transfer - _rewardVault.transferToEthVault(poolId, member, amount); + rewardVault.transferToEthVault( + poolId, + member, + amount, + address(ethVault) + ); } } diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index 72fe0836e2..7c2a3dd976 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -19,10 +19,10 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; +import "@0x/contracts-exchange-libs/contracts/src/LibMath.sol"; import "@0x/contracts-utils/contracts/src/LibFractions.sol"; import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; import "./MixinCumulativeRewards.sol"; -import "./MixinEthVault.sol"; contract MixinStakingPoolRewards is @@ -38,16 +38,16 @@ contract MixinStakingPoolRewards is { address member = msg.sender; - IStructs.StoredBalance memory finalDelegatedStakeToPoolByOwner = _loadAndSyncBalance(delegatedStakeToPoolByOwner[member][poolId]); + IStructs.StoredBalance memory finalDelegatedStakeToPoolByOwner = _loadAndSyncBalance(_delegatedStakeToPoolByOwner[member][poolId]); _syncRewardsForDelegator( poolId, member, - _loadUnsyncedBalance(delegatedStakeToPoolByOwner[member][poolId]), // initial balance + _loadUnsyncedBalance(_delegatedStakeToPoolByOwner[member][poolId]), // initial balance finalDelegatedStakeToPoolByOwner ); // update stored balance with synchronized version; this prevents redundant withdrawals. - delegatedStakeToPoolByOwner[member][poolId] = finalDelegatedStakeToPoolByOwner; + _delegatedStakeToPoolByOwner[member][poolId] = finalDelegatedStakeToPoolByOwner; } /// @dev Computes the reward balance in ETH of a specific member of a pool. @@ -104,10 +104,12 @@ contract MixinStakingPoolRewards is ); } - /// @dev Records a reward for delegators. This adds to the `_cumulativeRewardsByPool`. + /// @dev Handles a pool's reward. This will deposit the operator's reward into the Eth Vault and + /// the members' reward into the Staking Pool Vault. It also records the cumulative reward, which + /// is used to compute each delegator's portion of the members' reward. /// @param poolId Unique Id of pool. - /// @param reward to record for delegators. - /// @param amountOfDelegatedStake the amount of delegated stake that will split this reward. + /// @param reward received by the pool. + /// @param amountOfDelegatedStake the amount of delegated stake that will split the reward. /// @param epoch at which this was earned. function _handleStakingPoolReward( bytes32 poolId, @@ -289,4 +291,24 @@ contract MixinStakingPoolRewards is ); } } + + /// @dev Transfers operator reward to the ETH vault. + /// @param poolId Unique Id of pool to transfer reward for, + /// @param operator of the pool. + /// @param amount of ETH to transfer. + function _transferOperatorRewardToEthVault( + bytes32 poolId, + address operator, + uint256 amount + ) + private + { + // perform transfer and notify + ethVault.depositFor.value(amount)(operator); + emit OperatorRewardTransferredToEthVault( + poolId, + operator, + amount + ); + } } diff --git a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol index 5013f7a729..3fdcaa1e92 100644 --- a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol @@ -38,21 +38,7 @@ contract StakingPoolRewardVault is using LibSafeMath for uint256; // mapping from poolId to Pool metadata - mapping (bytes32 => uint256) internal balanceByPoolId; - - // address of ether vault - IEthVault internal _ethVault; - - /// @dev Sets the Eth Vault. - /// Note that only the contract owner can call this. - /// @param ethVaultAddress Address of the Eth Vault. - function setEthVault(address ethVaultAddress) - external - onlyOwner - { - _ethVault = IEthVault(ethVaultAddress); - emit EthVaultChanged(ethVaultAddress); - } + mapping (bytes32 => uint256) internal _balanceByPoolId; /// @dev Deposit an amount of ETH (`msg.value`) for `poolId` into the vault. /// Note that this is only callable by the staking contract. @@ -60,35 +46,29 @@ contract StakingPoolRewardVault is function depositFor(bytes32 poolId) external payable - onlyStakingContract + onlyStakingProxy { - balanceByPoolId[poolId] = balanceByPoolId[poolId].safeAdd(msg.value); + _balanceByPoolId[poolId] = _balanceByPoolId[poolId].safeAdd(msg.value); emit EthDepositedIntoVault(msg.sender, poolId, msg.value); } /// @dev Withdraw some amount in ETH of a pool member. /// Note that this is only callable by the staking contract. /// @param poolId Unique Id of pool. + /// @param member of pool to transfer funds to. /// @param amount Amount in ETH to transfer. + /// @param ethVaultAddress address of Eth Vault to send rewards to. function transferToEthVault( bytes32 poolId, address member, - uint256 amount + uint256 amount, + address ethVaultAddress ) external onlyStakingProxy { - // sanity check on eth vault - IEthVault _ethVault = ethVault; - if (address(_ethVault) == address(0)) { - LibRichErrors.rrevert( - LibStakingRichErrors.EthVaultNotSetError() - ); - } - - // perform transfer - balanceByPoolId[poolId] = balanceByPoolId[poolId].safeSub(amount); - _ethVault.depositFor.value(amount)(member); + _balanceByPoolId[poolId] = _balanceByPoolId[poolId].safeSub(amount); + IEthVault(ethVaultAddress).depositFor.value(amount)(member); emit PoolRewardTransferredToEthVault( poolId, member, @@ -103,6 +83,6 @@ contract StakingPoolRewardVault is view returns (uint256) { - return balanceByPoolId[poolId]; + return _balanceByPoolId[poolId]; } } diff --git a/contracts/staking/test/actors/pool_operator_actor.ts b/contracts/staking/test/actors/pool_operator_actor.ts index a95adf8117..86d6078c88 100644 --- a/contracts/staking/test/actors/pool_operator_actor.ts +++ b/contracts/staking/test/actors/pool_operator_actor.ts @@ -35,10 +35,10 @@ export class PoolOperatorActor extends BaseActor { ); expect(poolIdOfMaker, 'pool id of maker').to.be.equal(poolId); // check the number of makers in the pool - const numMakersAfterRemoving = await this._stakingApiWrapper.stakingContract.numMakersByPoolId.callAsync( + const pool = await this._stakingApiWrapper.stakingContract.getStakingPool.callAsync( poolId, ); - expect(numMakersAfterRemoving, 'number of makers in pool').to.be.bignumber.equal(1); + expect(pool.numberOfMakers, 'number of makers in pool').to.be.bignumber.equal(1); } return poolId; } diff --git a/contracts/staking/test/pools_test.ts b/contracts/staking/test/pools_test.ts index 79995fb1d2..71f6e98163 100644 --- a/contracts/staking/test/pools_test.ts +++ b/contracts/staking/test/pools_test.ts @@ -138,8 +138,8 @@ blockchainTests('Staking Pool Management', env => { ); // check the number of makers in the pool - let numMakers = await stakingApiWrapper.stakingContract.numMakersByPoolId.callAsync(poolId); - expect(numMakers, 'number of makers in pool after adding').to.be.bignumber.equal(3); + let pool = await stakingApiWrapper.stakingContract.getStakingPool.callAsync(poolId); + expect(pool.numberOfMakers, 'number of makers in pool after adding').to.be.bignumber.equal(3); // remove maker from pool await Promise.all( @@ -149,8 +149,8 @@ blockchainTests('Staking Pool Management', env => { ); // check the number of makers in the pool - numMakers = await stakingApiWrapper.stakingContract.numMakersByPoolId.callAsync(poolId); - expect(numMakers, 'number of makers in pool after removing').to.be.bignumber.equal(0); + pool = await stakingApiWrapper.stakingContract.getStakingPool.callAsync(poolId); + expect(pool.numberOfMakers, 'number of makers in pool after removing').to.be.bignumber.equal(0); }); it('Should fail if maker already assigned to another pool tries to join', async () => { // test parameters @@ -337,8 +337,8 @@ blockchainTests('Staking Pool Management', env => { ); // check the number of makers in the pool - const numMakers = await stakingApiWrapper.stakingContract.numMakersByPoolId.callAsync(poolId); - expect(numMakers, 'number of makers in pool').to.be.bignumber.equal( + const pool = await stakingApiWrapper.stakingContract.getStakingPool.callAsync(poolId); + expect(pool.numberOfMakers, 'number of makers in pool').to.be.bignumber.equal( stakingConstants.DEFAULT_PARAMS.maximumMakersInPool, ); diff --git a/contracts/staking/test/rewards_test.ts b/contracts/staking/test/rewards_test.ts index cf536689e0..cb71829ca4 100644 --- a/contracts/staking/test/rewards_test.ts +++ b/contracts/staking/test/rewards_test.ts @@ -113,10 +113,12 @@ blockchainTests.resets('Testing Rewards', env => { ? _expectedEndBalances.poolRewardVaultBalance : ZERO, }; - const pool = await stakingApiWrapper.rewardVaultContract.poolById.callAsync(poolId); - const operatorBalance = pool[1]; - const membersBalance = pool[2]; + /* + const pool = await stakingApiWrapper.stakingContract.getStakingPool.callAsync(poolId); + const operatorBalance = pool[2]; + const membersBalance = pool[3]; const poolBalances = { poolBalance: operatorBalance.plus(membersBalance), operatorBalance, membersBalance }; + */ const finalEndBalancesAsArray = await Promise.all([ // staker 1 stakingApiWrapper.stakingContract.computeRewardBalanceOfDelegator.callAsync( diff --git a/contracts/staking/test/utils/api_wrapper.ts b/contracts/staking/test/utils/api_wrapper.ts index 253d25c47e..941dac3a73 100644 --- a/contracts/staking/test/utils/api_wrapper.ts +++ b/contracts/staking/test/utils/api_wrapper.ts @@ -208,16 +208,6 @@ export async function deployAndConfigureContractsAsync( rewardVaultContract.address, zrxVaultContract.address, ); - // set eth vault in staking contract - const setEthVaultCalldata = stakingContract.setEthVault.getABIEncodedTransactionData(ethVaultContract.address); - const setEthVaultCalldataTxData = { - from: ownerAddress, - to: stakingProxyContract.address, - data: setEthVaultCalldata, - }; - await env.web3Wrapper.awaitTransactionSuccessAsync( - await env.web3Wrapper.sendTransactionAsync(setEthVaultCalldataTxData), - ); // configure erc20 proxy to accept calls from zrx vault await erc20ProxyContract.addAuthorizedAddress.awaitTransactionSuccessAsync(zrxVaultContract.address); @@ -225,8 +215,6 @@ export async function deployAndConfigureContractsAsync( await zrxVaultContract.setStakingProxy.awaitTransactionSuccessAsync(stakingProxyContract.address); // set staking proxy contract in reward vault await rewardVaultContract.setStakingProxy.awaitTransactionSuccessAsync(stakingProxyContract.address); - // set the eth vault in the reward vault - await rewardVaultContract.setEthVault.awaitTransactionSuccessAsync(ethVaultContract.address); return new StakingApiWrapper( env, ownerAddress, From 877abeda6307301e4c5ca1a625ca72876cb0d8dd Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 17 Sep 2019 17:22:03 -0700 Subject: [PATCH 4/6] Removed MixinStakingPoolRewards --- .../contracts/src/fees/MixinExchangeFees.sol | 5 +- .../src/interfaces/IStakingEvents.sol | 6 -- .../staking_pools/MixinCumulativeRewards.sol | 2 - .../MixinStakingPoolRewardVault.sol | 84 ------------------- .../staking_pools/MixinStakingPoolRewards.sol | 33 ++------ contracts/staking/src/artifacts.ts | 2 - contracts/staking/src/wrappers.ts | 1 - contracts/staking/tsconfig.json | 1 - 8 files changed, 8 insertions(+), 126 deletions(-) delete mode 100644 contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index 8ea7e4812a..69ea51d920 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol @@ -156,7 +156,7 @@ contract MixinExchangeFees is /// Each pool receives a portion of the fees generated this epoch (see _cobbDouglas) that is /// proportional to (i) the fee volume attributed to their pool over the epoch, and /// (ii) the amount of stake provided by the maker and their delegators. Rebates are paid - /// into the Reward Vault (see MixinStakingPoolRewardVault) where they can be withdraw by makers and + /// into the Reward Vault where they can be withdraw by makers and /// the members of their pool. There will be a small amount of ETH leftover in this contract /// after paying out the rebates; at present, this rolls over into the next epoch. Eventually, /// we plan to deposit this leftover into a DAO managed by the 0x community. @@ -278,9 +278,6 @@ contract MixinExchangeFees is initialContractBalance )); } - if (totalRewardsPaid > 0) { - _depositIntoStakingPoolRewardVault(totalRewardsPaid); - } finalContractBalance = address(this).balance; diff --git a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol index eeb4f5f969..9203436d9c 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol @@ -137,12 +137,6 @@ interface IStakingEvents { address makerAddress ); - /// @dev Emitted by MixinStakingPoolRewardVault when the vault's address is changed. - /// @param rewardVaultAddress Address of new reward vault. - event StakingPoolRewardVaultChanged( - address rewardVaultAddress - ); - /// @dev Emitted when a staking pool's operator share is decreased. /// @param poolId Unique Id of pool. /// @param oldOperatorShare Previous share of rewards owned by operator. diff --git a/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol index 12b38f0f26..aed8acc133 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol @@ -22,11 +22,9 @@ pragma experimental ABIEncoderV2; import "@0x/contracts-utils/contracts/src/LibFractions.sol"; import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; import "../stake/MixinStakeBalances.sol"; -import "./MixinStakingPoolRewardVault.sol"; contract MixinCumulativeRewards is - MixinStakingPoolRewardVault, MixinStakeBalances { using LibSafeMath for uint256; diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol deleted file mode 100644 index 703e115aff..0000000000 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol +++ /dev/null @@ -1,84 +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-utils/contracts/src/LibRichErrors.sol"; -import "../libs/LibStakingRichErrors.sol"; -import "../interfaces/IStakingEvents.sol"; -import "../interfaces/IStakingPoolRewardVault.sol"; -import "../immutable/MixinStorage.sol"; - - -/// @dev This mixin contains logic for interfacing with the Staking Pool Reward Vault (vaults/StakingPoolRewardVault.sol) -/// Note that setters are callable only by the owner of this contract, and withdraw functionality is accessible only -/// from within this contract. -contract MixinStakingPoolRewardVault is - IStakingEvents, - MixinStorage -{ - - /// @dev Sets the address of the reward vault. - /// This can only be called by the owner of this contract. - function setStakingPoolRewardVault(address payable rewardVaultAddress) - external - onlyOwner - { - rewardVault = IStakingPoolRewardVault(rewardVaultAddress); - emit StakingPoolRewardVaultChanged(rewardVaultAddress); - } - - /// @dev Returns the staking pool reward vault - /// @return Address of reward vault. - function getStakingPoolRewardVault() - public - view - returns (address) - { - return address(rewardVault); - } - - /// @dev Deposits an amount in ETH into the reward vault. - /// @param amount The amount in ETH to deposit. - function _depositIntoStakingPoolRewardVault(uint256 amount) - internal - { - // cast to payable then transfer - address payable rewardVaultAddress = address(uint160(address(rewardVault))); - rewardVaultAddress.transfer(amount); - } - - /// @dev Transfer from transient Reward Pool vault to ETH Vault. - /// @param poolId Unique Id of pool. - /// @param member of pool to transfer ETH to. - /// @param amount The amount in ETH to transfer. - function _transferMemberBalanceToEthVault( - bytes32 poolId, - address member, - uint256 amount - ) - internal - { - rewardVault.transferToEthVault( - poolId, - member, - amount, - address(ethVault) - ); - } -} diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index 7c2a3dd976..45a5886a67 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -130,11 +130,7 @@ contract MixinStakingPoolRewards is reward ); - _transferOperatorRewardToEthVault( - poolId, - pool.operator, - operatorPortion - ); + ethVault.depositFor.value(operatorPortion)(pool.operator); // compute the reward portion for the pool members and transfer it to the Reward Vault. uint256 membersPortion = reward.safeSub(operatorPortion); @@ -201,7 +197,12 @@ contract MixinStakingPoolRewards is } // transfer from transient Reward Pool vault to ETH Vault - _transferMemberBalanceToEthVault(poolId, member, balance); + rewardVault.transferToEthVault( + poolId, + member, + balance, + address(ethVault) + ); } /// @dev Computes the reward balance in ETH of a specific member of a pool. @@ -291,24 +292,4 @@ contract MixinStakingPoolRewards is ); } } - - /// @dev Transfers operator reward to the ETH vault. - /// @param poolId Unique Id of pool to transfer reward for, - /// @param operator of the pool. - /// @param amount of ETH to transfer. - function _transferOperatorRewardToEthVault( - bytes32 poolId, - address operator, - uint256 amount - ) - private - { - // perform transfer and notify - ethVault.depositFor.value(amount)(operator); - emit OperatorRewardTransferredToEthVault( - poolId, - operator, - amount - ); - } } diff --git a/contracts/staking/src/artifacts.ts b/contracts/staking/src/artifacts.ts index 165b9ea0f1..6c284c9264 100644 --- a/contracts/staking/src/artifacts.ts +++ b/contracts/staking/src/artifacts.ts @@ -35,7 +35,6 @@ import * as MixinStakingPool from '../generated-artifacts/MixinStakingPool.json' import * as MixinStakingPoolMakers from '../generated-artifacts/MixinStakingPoolMakers.json'; import * as MixinStakingPoolModifiers from '../generated-artifacts/MixinStakingPoolModifiers.json'; import * as MixinStakingPoolRewards from '../generated-artifacts/MixinStakingPoolRewards.json'; -import * as MixinStakingPoolRewardVault from '../generated-artifacts/MixinStakingPoolRewardVault.json'; import * as MixinStorage from '../generated-artifacts/MixinStorage.json'; import * as MixinVaultCore from '../generated-artifacts/MixinVaultCore.json'; import * as ReadOnlyProxy from '../generated-artifacts/ReadOnlyProxy.json'; @@ -86,7 +85,6 @@ export const artifacts = { MixinStakingPool: MixinStakingPool as ContractArtifact, MixinStakingPoolMakers: MixinStakingPoolMakers as ContractArtifact, MixinStakingPoolModifiers: MixinStakingPoolModifiers as ContractArtifact, - MixinStakingPoolRewardVault: MixinStakingPoolRewardVault as ContractArtifact, MixinStakingPoolRewards: MixinStakingPoolRewards as ContractArtifact, MixinParams: MixinParams as ContractArtifact, MixinScheduler: MixinScheduler as ContractArtifact, diff --git a/contracts/staking/src/wrappers.ts b/contracts/staking/src/wrappers.ts index bb9aee4280..2d700c3830 100644 --- a/contracts/staking/src/wrappers.ts +++ b/contracts/staking/src/wrappers.ts @@ -32,7 +32,6 @@ export * from '../generated-wrappers/mixin_stake_storage'; export * from '../generated-wrappers/mixin_staking_pool'; export * from '../generated-wrappers/mixin_staking_pool_makers'; export * from '../generated-wrappers/mixin_staking_pool_modifiers'; -export * from '../generated-wrappers/mixin_staking_pool_reward_vault'; export * from '../generated-wrappers/mixin_staking_pool_rewards'; export * from '../generated-wrappers/mixin_storage'; export * from '../generated-wrappers/mixin_vault_core'; diff --git a/contracts/staking/tsconfig.json b/contracts/staking/tsconfig.json index 6e5e2dfa1d..9d6f2b05f1 100644 --- a/contracts/staking/tsconfig.json +++ b/contracts/staking/tsconfig.json @@ -32,7 +32,6 @@ "generated-artifacts/MixinStakingPool.json", "generated-artifacts/MixinStakingPoolMakers.json", "generated-artifacts/MixinStakingPoolModifiers.json", - "generated-artifacts/MixinStakingPoolRewardVault.json", "generated-artifacts/MixinStakingPoolRewards.json", "generated-artifacts/MixinStorage.json", "generated-artifacts/MixinVaultCore.json", From 5a225795e10d83e1cce4d09795c134789e8b961d Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 17 Sep 2019 17:35:35 -0700 Subject: [PATCH 5/6] Updated changelog and ran linter --- contracts/staking/CHANGELOG.json | 4 ++++ .../contracts/src/interfaces/IStakingEvents.sol | 10 ---------- .../staking/contracts/src/interfaces/IStructs.sol | 2 +- .../contracts/src/staking_pools/MixinStakingPool.sol | 8 ++++++++ .../src/staking_pools/MixinStakingPoolRewards.sol | 2 ++ contracts/staking/test/actors/pool_operator_actor.ts | 6 ++---- contracts/staking/test/rewards_test.ts | 6 ------ 7 files changed, 17 insertions(+), 21 deletions(-) diff --git a/contracts/staking/CHANGELOG.json b/contracts/staking/CHANGELOG.json index 2a953327fb..c09b42b1a5 100644 --- a/contracts/staking/CHANGELOG.json +++ b/contracts/staking/CHANGELOG.json @@ -49,6 +49,10 @@ { "note": "Refactored Staking Reward Vault. Moved pool management logic into staking contract.", "pr": 2156 + }, + { + "note": "Removed MixinStakingPoolRewardVault.sol", + "pr": 2156 } ] } diff --git a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol index 9203436d9c..a017b66a32 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol @@ -146,14 +146,4 @@ interface IStakingEvents { uint32 oldOperatorShare, uint32 newOperatorShare ); - - /// @dev Emitted when an operator reward is transferred to the Eth vault. - /// @param amount The amount in ETH withdrawn. - /// @param operator of the pool. - /// @param poolId The pool the reward was deposited for. - event OperatorRewardTransferredToEthVault( - bytes32 poolId, - address operator, - uint256 amount - ); } diff --git a/contracts/staking/contracts/src/interfaces/IStructs.sol b/contracts/staking/contracts/src/interfaces/IStructs.sol index cdd4aec185..df4deb2b49 100644 --- a/contracts/staking/contracts/src/interfaces/IStructs.sol +++ b/contracts/staking/contracts/src/interfaces/IStructs.sol @@ -97,7 +97,7 @@ interface IStructs { } /// @dev Holds the metadata for a staking pool. - /// @param initialzed True iff the balance struct is initialized. + /// @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. /// @param numberOfMakers Number of makers in the pool. diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol index 2b3a134b0b..f4ff80b55a 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol @@ -113,6 +113,8 @@ contract MixinStakingPool is return nextPoolId; } + /// @dev Returns a staking pool + /// @param poolId Unique id of pool. function getStakingPool(bytes32 poolId) public view @@ -132,6 +134,8 @@ contract MixinStakingPool is return bytes32(uint256(poolId).safeAdd(POOL_ID_INCREMENT_AMOUNT)); } + /// @dev Reverts iff a staking pool does not exist. + /// @param poolId Unique id of pool. function _assertStakingPoolExists(bytes32 poolId) internal view @@ -148,6 +152,10 @@ contract MixinStakingPool is } } + /// @dev Reverts iff the new operator share is invalid. + /// @param poolId Unique id of pool. + /// @param currentOperatorShare Current operator share. + /// @param newOperatorShare New operator share. function _assertNewOperatorShare( bytes32 poolId, uint32 currentOperatorShare, diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index 45a5886a67..93b723c929 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -32,6 +32,8 @@ 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. + /// 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) external diff --git a/contracts/staking/test/actors/pool_operator_actor.ts b/contracts/staking/test/actors/pool_operator_actor.ts index 86d6078c88..bd74e5bb17 100644 --- a/contracts/staking/test/actors/pool_operator_actor.ts +++ b/contracts/staking/test/actors/pool_operator_actor.ts @@ -1,5 +1,5 @@ import { expect } from '@0x/contracts-test-utils'; -import { BigNumber, RevertError } from '@0x/utils'; +import { RevertError } from '@0x/utils'; import * as _ from 'lodash'; import { constants as stakingConstants } from '../utils/constants'; @@ -35,9 +35,7 @@ export class PoolOperatorActor extends BaseActor { ); expect(poolIdOfMaker, 'pool id of maker').to.be.equal(poolId); // check the number of makers in the pool - const pool = await this._stakingApiWrapper.stakingContract.getStakingPool.callAsync( - poolId, - ); + const pool = await this._stakingApiWrapper.stakingContract.getStakingPool.callAsync(poolId); expect(pool.numberOfMakers, 'number of makers in pool').to.be.bignumber.equal(1); } return poolId; diff --git a/contracts/staking/test/rewards_test.ts b/contracts/staking/test/rewards_test.ts index cb71829ca4..86ce5e9d0a 100644 --- a/contracts/staking/test/rewards_test.ts +++ b/contracts/staking/test/rewards_test.ts @@ -113,12 +113,6 @@ blockchainTests.resets('Testing Rewards', env => { ? _expectedEndBalances.poolRewardVaultBalance : ZERO, }; - /* - const pool = await stakingApiWrapper.stakingContract.getStakingPool.callAsync(poolId); - const operatorBalance = pool[2]; - const membersBalance = pool[3]; - const poolBalances = { poolBalance: operatorBalance.plus(membersBalance), operatorBalance, membersBalance }; - */ const finalEndBalancesAsArray = await Promise.all([ // staker 1 stakingApiWrapper.stakingContract.computeRewardBalanceOfDelegator.callAsync( From 2869dd3bac1283b68b572568904d0bd97b99d1ca Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 17 Sep 2019 20:32:49 -0700 Subject: [PATCH 6/6] Removed unnecessary sloads in MixinStakingPoolMakers --- .../staking_pools/MixinCumulativeRewards.sol | 1 + .../src/staking_pools/MixinStakingPool.sol | 11 +--------- .../staking_pools/MixinStakingPoolMakers.sol | 20 ++++++------------- .../MixinStakingPoolModifiers.sol | 6 +++--- contracts/staking/package.json | 2 +- 5 files changed, 12 insertions(+), 28 deletions(-) diff --git a/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol index aed8acc133..53dfb4b88e 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol @@ -130,6 +130,7 @@ contract MixinCumulativeRewards is /// @dev Returns info on most recent cumulative reward. function _getMostRecentCumulativeRewardInfo(bytes32 poolId) internal + view returns (IStructs.CumulativeRewardInfo memory) { // fetch the last epoch at which we stored a cumulative reward for this pool diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol index f4ff80b55a..4f3ce21f8e 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol @@ -103,16 +103,6 @@ contract MixinStakingPool is ); } - /// @dev Returns the unique id that will be assigned to the next pool that is created. - /// @return Pool id. - function getNextStakingPoolId() - public - view - returns (bytes32) - { - return nextPoolId; - } - /// @dev Returns a staking pool /// @param poolId Unique id of pool. function getStakingPool(bytes32 poolId) @@ -162,6 +152,7 @@ contract MixinStakingPool is uint32 newOperatorShare ) private + pure { // sanity checks if (newOperatorShare > PPM_DENOMINATOR) { diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol index 71e01399e7..bf16479d21 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol @@ -144,17 +144,6 @@ contract MixinStakingPoolMakers is return poolJoinedByMakerAddress[makerAddress].confirmed; } - /// @dev Returns the current number of makers in a given pool. - /// @param poolId Unique id of pool. - /// @return Size of pool. - function getNumberOfMakersInStakingPool(bytes32 poolId) - public - view - returns (uint256) - { - return poolById[poolId].numberOfMakers; - } - /// @dev Adds a maker to a staking pool. Note that this is only callable by the pool operator. /// Note also that the maker must have previously called joinStakingPoolAsMaker. /// @param poolId Unique id of pool. @@ -165,6 +154,9 @@ contract MixinStakingPoolMakers is ) internal { + // cache pool for use throughout this function + IStructs.Pool memory pool = poolById[poolId]; + // Is the maker already in a pool? if (isMakerAssignedToStakingPool(makerAddress)) { LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( @@ -176,7 +168,7 @@ contract MixinStakingPoolMakers is // Is the maker trying to join this pool; or are they the operator? bytes32 makerPendingPoolId = poolJoinedByMakerAddress[makerAddress].poolId; - if (makerPendingPoolId != poolId && makerAddress != poolById[poolId].operator) { + if (makerPendingPoolId != poolId && makerAddress != pool.operator) { LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressNotPendingAdd, makerAddress, @@ -185,7 +177,7 @@ contract MixinStakingPoolMakers is } // Is the pool already full? - if (getNumberOfMakersInStakingPool(poolId) == maximumMakersInPool) { + if (pool.numberOfMakers == maximumMakersInPool) { LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( LibStakingRichErrors.MakerPoolAssignmentErrorCodes.PoolIsFull, makerAddress, @@ -199,7 +191,7 @@ contract MixinStakingPoolMakers is confirmed: true }); poolJoinedByMakerAddress[makerAddress] = poolJoinStatus; - poolById[poolId].numberOfMakers = uint256(poolById[poolId].numberOfMakers).safeAdd(1).downcastToUint32(); + poolById[poolId].numberOfMakers = uint256(pool.numberOfMakers).safeAdd(1).downcastToUint32(); // Maker has been added to the pool emit MakerAddedToStakingPool( diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol index 71be9b68dc..7892f0c9af 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol @@ -44,10 +44,10 @@ contract MixinStakingPoolModifiers is /// @param poolId Pool sender must be operator of. /// @param makerAddress Address of a maker in the pool. modifier onlyStakingPoolOperatorOrMaker(bytes32 poolId, address makerAddress) { - address operator; + address operator = poolById[poolId].operator; if ( - msg.sender != makerAddress && - msg.sender != (operator = poolById[poolId].operator) + msg.sender != operator && + msg.sender != makerAddress ) { LibRichErrors.rrevert( LibStakingRichErrors.OnlyCallableByPoolOperatorOrMakerError( diff --git a/contracts/staking/package.json b/contracts/staking/package.json index 7664ef0613..092fdb2502 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|LibFixedMath|LibFixedMathRichErrors|LibProxy|LibSafeDowncast|LibStakingRichErrors|MixinConstants|MixinCumulativeRewards|MixinDeploymentConstants|MixinExchangeFees|MixinExchangeManager|MixinParams|MixinScheduler|MixinStake|MixinStakeBalances|MixinStakeStorage|MixinStakingPool|MixinStakingPoolRewardVault|MixinStakingPoolRewards|MixinStorage|MixinVaultCore|ReadOnlyProxy|Staking|StakingPoolRewardVault|StakingProxy|TestCobbDouglas|TestCumulativeRewardTracking|TestInitTarget|TestLibFixedMath|TestLibProxy|TestLibProxyReceiver|TestLibSafeDowncast|TestProtocolFees|TestProtocolFeesERC20Proxy|TestStaking|TestStakingProxy|TestStorageLayout|ZrxVault).json" + "abis": "./generated-artifacts/@(EthVault|IEthVault|IStaking|IStakingEvents|IStakingPoolRewardVault|IStakingProxy|IStorage|IStorageInit|IStructs|IVaultCore|IZrxVault|LibFixedMath|LibFixedMathRichErrors|LibProxy|LibSafeDowncast|LibStakingRichErrors|MixinConstants|MixinCumulativeRewards|MixinDeploymentConstants|MixinExchangeFees|MixinExchangeManager|MixinParams|MixinScheduler|MixinStake|MixinStakeBalances|MixinStakeStorage|MixinStakingPool|MixinStakingPoolMakers|MixinStakingPoolModifiers|MixinStakingPoolRewards|MixinStorage|MixinVaultCore|ReadOnlyProxy|Staking|StakingPoolRewardVault|StakingProxy|TestCobbDouglas|TestCumulativeRewardTracking|TestInitTarget|TestLibFixedMath|TestLibProxy|TestLibProxyReceiver|TestLibSafeDowncast|TestProtocolFees|TestProtocolFeesERC20Proxy|TestStaking|TestStakingProxy|TestStorageLayout|ZrxVault).json" }, "repository": { "type": "git",