diff --git a/contracts/staking/CHANGELOG.json b/contracts/staking/CHANGELOG.json index 5272f915f6..c09b42b1a5 100644 --- a/contracts/staking/CHANGELOG.json +++ b/contracts/staking/CHANGELOG.json @@ -45,6 +45,14 @@ { "note": "Reference counting for cumulative rewards.", "pr": 2154 + }, + { + "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/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/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index 152e8485e1..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. @@ -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; @@ -288,9 +278,6 @@ contract MixinExchangeFees is initialContractBalance )); } - if (totalRewardsPaid > 0) { - _depositIntoStakingPoolRewardVault(totalRewardsPaid); - } finalContractBalance = address(this).balance; diff --git a/contracts/staking/contracts/src/immutable/MixinStorage.sol b/contracts/staking/contracts/src/immutable/MixinStorage.sol index 98ac6c237a..62756e5e34 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) 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 e538bf7c16..a017b66a32 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 ); @@ -136,4 +136,14 @@ interface IStakingEvents { bytes32 poolId, address makerAddress ); + + /// @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 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 95c4e7d6d2..2653ce4154 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol @@ -21,160 +21,50 @@ 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 the eth vault is changed - /// @param newEthVault address of new rth 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, + /// @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 withdrawn for an operator. + /// @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 RewardWithdrawnForOperator( - bytes32 poolId, + event PoolRewardTransferredToEthVault( + bytes32 indexed poolId, + address indexed member, 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 () + /// @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 payable; - 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 - ) - 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; - /// @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 member of pool to transfer funds to. /// @param amount Amount in ETH to transfer. - function transferMemberBalanceToEthVault( + /// @param ethVaultAddress address of Eth Vault to send rewards to. + function transferToEthVault( bytes32 poolId, address member, - uint256 amount + uint256 amount, + address ethVaultAddress ) 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..df4deb2b49 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 metadata for a staking pool. + /// @param initialized True iff the balance struct is initialized. + /// @param operator of the pool. + /// @param operatorShare Fraction of the total balance owned by the operator, in ppm. + /// @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/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/MixinCumulativeRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol index 12b38f0f26..53dfb4b88e 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; @@ -132,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 e96fd8551c..4f3ce21f8e 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPool.sol @@ -24,32 +24,12 @@ 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 + MixinStorage, + MixinStakingPoolMakers, MixinStakingPoolRewards { using LibSafeMath for uint256; @@ -64,191 +44,73 @@ 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 + uint32 currentOperatorShare = poolById[poolId].operatorShare; + _assertNewOperatorShare( poolId, - makerAddress + currentOperatorShare, + newOperatorShare + ); + + // decrease operator share + poolById[poolId].operatorShare = newOperatorShare; + emit OperatorShareDecreased( + poolId, + currentOperatorShare, + 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. + /// @dev Returns a staking pool /// @param poolId Unique id of pool. - /// @param makerAddress Address of maker. - function addMakerToStakingPool( - bytes32 poolId, - address makerAddress - ) - external - onlyStakingPoolOperator(poolId) - { - // 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 - ); - } - - /// @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) + function getStakingPool(bytes32 poolId) public view - returns (bytes32) + returns (IStructs.Pool memory) { - 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 poolById[poolId]; } /// @dev Computes the unique id that comes after the input pool id. @@ -261,4 +123,52 @@ 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 + 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 + ) + ); + } + } + + /// @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, + uint32 newOperatorShare + ) + private + pure + { + // 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..bf16479d21 --- /dev/null +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolMakers.sol @@ -0,0 +1,202 @@ +/* + + 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/LibSafeMath.sol"; +import "../libs/LibStakingRichErrors.sol"; +import "../libs/LibSafeDowncast.sol"; +import "../interfaces/IStructs.sol"; +import "../interfaces/IStakingEvents.sol"; +import "../immutable/MixinStorage.sol"; +import "./MixinStakingPoolModifiers.sol"; + + +/// @dev This mixin contains logic for staking pools. +contract MixinStakingPoolMakers is + IStakingEvents, + MixinStorage, + 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 + ) + 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 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 + { + // 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( + 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 != pool.operator) { + LibRichErrors.rrevert(LibStakingRichErrors.MakerPoolAssignmentError( + LibStakingRichErrors.MakerPoolAssignmentErrorCodes.MakerAddressNotPendingAdd, + makerAddress, + makerPendingPoolId + )); + } + + // Is the pool already full? + if (pool.numberOfMakers == 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(pool.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..7892f0c9af --- /dev/null +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolModifiers.sol @@ -0,0 +1,63 @@ +/* + + 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 "../immutable/MixinStorage.sol"; + + +contract MixinStakingPoolModifiers is + 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 operator = poolById[poolId].operator; + if (msg.sender != operator) { + LibRichErrors.rrevert(LibStakingRichErrors.OnlyCallableByPoolOperatorError( + msg.sender, + operator + )); + } + + _; + } + + /// @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 operator = poolById[poolId].operator; + if ( + msg.sender != operator && + msg.sender != makerAddress + ) { + LibRichErrors.rrevert( + LibStakingRichErrors.OnlyCallableByPoolOperatorOrMakerError( + msg.sender, + operator, + makerAddress + ) + ); + } + + _; + } +} 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 b3c75f858c..0000000000 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewardVault.sol +++ /dev/null @@ -1,121 +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 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) - external - onlyStakingPoolOperator(poolId) - { - rewardVault.decreaseOperatorShare(poolId, newOperatorShare); - } - - /// @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 and sanity check - address payable rewardVaultAddress = address(uint160(address(rewardVault))); - if (rewardVaultAddress == NIL_ADDRESS) { - LibRichErrors.rrevert( - LibStakingRichErrors.RewardVaultNotSetError() - ); - } - - // perform transfer - 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 - { - // sanity check - IStakingPoolRewardVault _rewardVault = rewardVault; - if (address(_rewardVault) == NIL_ADDRESS) { - LibRichErrors.rrevert( - LibStakingRichErrors.RewardVaultNotSetError() - ); - } - - // perform transfer - _rewardVault.transferMemberBalanceToEthVault(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..93b723c929 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -19,6 +19,7 @@ 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"; @@ -29,6 +30,28 @@ 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. + /// 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 + { + 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. @@ -83,12 +106,14 @@ 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 _recordRewardForDelegators( + function _handleStakingPoolReward( bytes32 poolId, uint256 reward, uint256 amountOfDelegatedStake, @@ -96,6 +121,27 @@ 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 + ); + + 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); + 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 ); @@ -153,7 +199,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. diff --git a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol index 5d841419a4..3fdcaa1e92 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,275 +30,59 @@ 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 () + /// @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 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. - function setEthVault(address ethVaultAddress) - external - onlyOwner - { - _ethVault = IEthVault(ethVaultAddress); - 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 - ) - external - onlyStakingProxy - returns ( - uint256 operatorPortion, - uint256 membersPortion - ) - { - // 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 member of pool to transfer funds to. /// @param amount Amount in ETH to transfer. - function transferMemberBalanceToEthVault( + /// @param ethVaultAddress address of Eth Vault to send rewards to. + function transferToEthVault( bytes32 poolId, address member, - uint256 amount + uint256 amount, + address ethVaultAddress ) external onlyStakingProxy { - if (amount == 0) { - return; - } - - // 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); + _balanceByPoolId[poolId] = _balanceByPoolId[poolId].safeSub(amount); + IEthVault(ethVaultAddress).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/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", diff --git a/contracts/staking/src/artifacts.ts b/contracts/staking/src/artifacts.ts index 809ded5ee2..6c284c9264 100644 --- a/contracts/staking/src/artifacts.ts +++ b/contracts/staking/src/artifacts.ts @@ -32,8 +32,9 @@ 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'; import * as MixinVaultCore from '../generated-artifacts/MixinVaultCore.json'; import * as ReadOnlyProxy from '../generated-artifacts/ReadOnlyProxy.json'; @@ -82,7 +83,8 @@ export const artifacts = { MixinStakeStorage: MixinStakeStorage as ContractArtifact, MixinCumulativeRewards: MixinCumulativeRewards as ContractArtifact, MixinStakingPool: MixinStakingPool as ContractArtifact, - MixinStakingPoolRewardVault: MixinStakingPoolRewardVault as ContractArtifact, + MixinStakingPoolMakers: MixinStakingPoolMakers as ContractArtifact, + MixinStakingPoolModifiers: MixinStakingPoolModifiers 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 7da08f1979..2d700c3830 100644 --- a/contracts/staking/src/wrappers.ts +++ b/contracts/staking/src/wrappers.ts @@ -30,7 +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_reward_vault'; +export * from '../generated-wrappers/mixin_staking_pool_makers'; +export * from '../generated-wrappers/mixin_staking_pool_modifiers'; 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/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..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,10 +35,8 @@ 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( - poolId, - ); - expect(numMakersAfterRemoving, 'number of makers in pool').to.be.bignumber.equal(1); + const pool = await this._stakingApiWrapper.stakingContract.getStakingPool.callAsync(poolId); + expect(pool.numberOfMakers, 'number of makers in pool').to.be.bignumber.equal(1); } return poolId; } @@ -103,8 +101,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/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 a0d8194a0b..86ce5e9d0a 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,15 +112,7 @@ 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]; - const membersBalance = pool[2]; - const poolBalances = { poolBalance: operatorBalance.plus(membersBalance), operatorBalance, membersBalance }; const finalEndBalancesAsArray = await Promise.all([ // staker 1 stakingApiWrapper.stakingContract.computeRewardBalanceOfDelegator.callAsync( @@ -141,6 +128,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 +143,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 +189,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 +206,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 +513,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 +557,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 +597,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 +634,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..941dac3a73 100644 --- a/contracts/staking/test/utils/api_wrapper.ts +++ b/contracts/staking/test/utils/api_wrapper.ts @@ -215,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, 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..9d6f2b05f1 100644 --- a/contracts/staking/tsconfig.json +++ b/contracts/staking/tsconfig.json @@ -30,7 +30,8 @@ "generated-artifacts/MixinStakeBalances.json", "generated-artifacts/MixinStakeStorage.json", "generated-artifacts/MixinStakingPool.json", - "generated-artifacts/MixinStakingPoolRewardVault.json", + "generated-artifacts/MixinStakingPoolMakers.json", + "generated-artifacts/MixinStakingPoolModifiers.json", "generated-artifacts/MixinStakingPoolRewards.json", "generated-artifacts/MixinStorage.json", "generated-artifacts/MixinVaultCore.json",