From 3ad7728a0e92e41b917b86eab407716759877bca Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Sat, 21 Sep 2019 03:42:38 -0400 Subject: [PATCH] `@0x/contracts-staking`: Remove `IStructs.CumulativeRewardInfo`, etc. `@0x/contracts-staking`: Convert all rewards to WETH. `@0x/contracts-staking`: Style changes. `@0x/contracts-staking`: Address misc. review comments. `@0x/contracts-staking`: Make `LibFractions` scaling a separate step. --- .../staking/contracts/src/ReadOnlyProxy.sol | 2 - contracts/staking/contracts/src/Staking.sol | 6 +- .../staking/contracts/src/StakingProxy.sol | 10 +- .../contracts/src/fees/MixinExchangeFees.sol | 88 +++---- .../immutable/MixinDeploymentConstants.sol | 11 + .../contracts/src/immutable/MixinStorage.sol | 23 +- .../contracts/src/interfaces/IEthVault.sol | 22 +- .../interfaces/IStakingPoolRewardVault.sol | 30 +-- .../contracts/src/interfaces/IStructs.sol | 25 +- .../contracts/src/libs/LibCobbDouglas.sol | 8 +- .../contracts/src/stake/MixinStake.sol | 6 +- .../staking_pools/MixinCumulativeRewards.sol | 53 ++-- .../staking_pools/MixinStakingPoolRewards.sol | 153 ++++++----- .../contracts/src/sys/MixinAbstract.sol | 35 +-- .../contracts/src/sys/MixinFinalizer.sol | 237 +++++++----------- .../staking/contracts/src/sys/MixinParams.sol | 28 +++ .../staking/contracts/src/vaults/EthVault.sol | 54 ++-- .../src/vaults/StakingPoolRewardVault.sol | 32 +-- .../contracts/test/TestDelegatorRewards.sol | 16 +- .../staking/contracts/test/TestFinalizer.sol | 87 +------ .../staking/contracts/test/TestStaking.sol | 15 +- .../contracts/test/TestStorageLayout.sol | 14 +- .../staking/test/actors/finalizer_actor.ts | 10 +- .../test/cumulative_reward_tracking_test.ts | 2 +- contracts/staking/test/epoch_test.ts | 2 +- contracts/staking/test/rewards_test.ts | 14 +- contracts/staking/test/stake_test.ts | 2 +- .../test/unit_tests/delegator_reward_test.ts | 2 +- contracts/staking/test/utils/api_wrapper.ts | 39 ++- .../cumulative_reward_tracking_simulation.ts | 28 +-- .../utils/contracts/src/LibFractions.sol | 73 ++++-- 31 files changed, 504 insertions(+), 623 deletions(-) diff --git a/contracts/staking/contracts/src/ReadOnlyProxy.sol b/contracts/staking/contracts/src/ReadOnlyProxy.sol index 7aa9eba73d..4e3277c908 100644 --- a/contracts/staking/contracts/src/ReadOnlyProxy.sol +++ b/contracts/staking/contracts/src/ReadOnlyProxy.sol @@ -23,8 +23,6 @@ import "./libs/LibProxy.sol"; contract ReadOnlyProxy is - MixinConstants, - Ownable, MixinStorage { using LibProxy for address; diff --git a/contracts/staking/contracts/src/Staking.sol b/contracts/staking/contracts/src/Staking.sol index d0d4dd2945..f8810a1028 100644 --- a/contracts/staking/contracts/src/Staking.sol +++ b/contracts/staking/contracts/src/Staking.sol @@ -37,17 +37,17 @@ contract Staking is MixinStorage, MixinStakingPoolModifiers, MixinExchangeManager, - MixinParams, MixinScheduler, + MixinParams, MixinStakeStorage, MixinStakingPoolMakers, MixinStakeBalances, MixinCumulativeRewards, MixinStakingPoolRewards, + MixinFinalizer, MixinStakingPool, MixinStake, - MixinExchangeFees, - MixinFinalizer + MixinExchangeFees { // this contract can receive ETH // solhint-disable no-empty-blocks diff --git a/contracts/staking/contracts/src/StakingProxy.sol b/contracts/staking/contracts/src/StakingProxy.sol index 09ad728a3a..85045ed24d 100644 --- a/contracts/staking/contracts/src/StakingProxy.sol +++ b/contracts/staking/contracts/src/StakingProxy.sol @@ -75,7 +75,7 @@ contract StakingProxy is /// @dev Attach a staking contract; future calls will be delegated to the staking contract. /// Note that this is callable only by this contract's owner. - /// @param _stakingContract Address of staking contract. + /// @param _stakingContract Address of staking contract. /// @param _wethProxyAddress The address that can transfer WETH for fees. /// Use address in storage if NIL_ADDRESS is passed in. /// @param _ethVaultAddress Address of the EthVault contract. @@ -209,6 +209,14 @@ contract StakingProxy is )); } + // Minimum stake must be > 1 + if (minimumStake < 2) { + LibRichErrors.rrevert( + LibStakingRichErrors.InvalidParamValueError( + LibStakingRichErrors.InvalidParamValueErrorCode.InvalidMinimumPoolStake + )); + } + // ERC20Proxy and Vault contract addresses must always be initialized if (address(wethAssetProxy) == NIL_ADDRESS) { LibRichErrors.rrevert( diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index ffbf767363..9c64afc37f 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol @@ -20,6 +20,7 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol"; +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"; @@ -27,22 +28,11 @@ import "../libs/LibCobbDouglas.sol"; import "../immutable/MixinDeploymentConstants.sol"; import "../interfaces/IStructs.sol"; import "../stake/MixinStakeBalances.sol"; -import "../sys/MixinAbstract.sol"; +import "../sys/MixinFinalizer.sol"; import "../staking_pools/MixinStakingPool.sol"; import "./MixinExchangeManager.sol"; -/// @dev This mixin contains the logic for 0x protocol fees. -/// Protocol fees are sent by 0x exchanges every time there is a trade. -/// If the maker has associated their address with a pool (see -/// MixinStakingPool.sol), then the fee will be attributed to their pool. -/// At the end of an epoch the maker and their pool will receive a rebate -/// 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. Note that delegated stake (see MixinStake) is -/// weighted less than stake provided by directly by the maker; this is a -/// disincentive for market makers to monopolize a single pool that they -/// all delegate to. contract MixinExchangeFees is IStakingEvents, MixinAbstract, @@ -58,6 +48,7 @@ contract MixinExchangeFees is MixinStakeBalances, MixinCumulativeRewards, MixinStakingPoolRewards, + MixinFinalizer, MixinStakingPool { using LibSafeMath for uint256; @@ -70,9 +61,7 @@ contract MixinExchangeFees is /// @param protocolFeePaid The protocol fee that should be paid. function payProtocolFee( address makerAddress, - // solhint-disable-next-line address payerAddress, - // solhint-disable-next-line uint256 protocolFeePaid ) external @@ -108,9 +97,9 @@ contract MixinExchangeFees is } // Look up the pool for this epoch. - uint256 currentEpoch = currentEpoch; + uint256 currentEpoch_ = currentEpoch; mapping (bytes32 => IStructs.ActivePool) storage activePoolsThisEpoch = - _getActivePoolsFromEpoch(currentEpoch); + _getActivePoolsFromEpoch(currentEpoch_); IStructs.ActivePool memory pool = activePoolsThisEpoch[poolId]; // If the pool was previously inactive in this epoch, initialize it. @@ -128,32 +117,19 @@ contract MixinExchangeFees is // Emit an event so keepers know what pools to pass into // `finalize()`. - emit StakingPoolActivated(currentEpoch, poolId); + emit StakingPoolActivated(currentEpoch_, poolId); } // Credit the fees to the pool. pool.feesCollected = pool.feesCollected.safeAdd(protocolFeePaid); // Increase the total fees collected this epoch. - totalFeesCollectedThisEpoch = totalFeesCollectedThisEpoch.safeAdd( - protocolFeePaid - ); + totalFeesCollectedThisEpoch = totalFeesCollectedThisEpoch.safeAdd(protocolFeePaid); // Store the pool. activePoolsThisEpoch[poolId] = pool; } - /// @dev Returns the total amount of fees collected thus far, in the current - /// epoch. - /// @return _totalFeesCollectedThisEpoch Total fees collected this epoch. - function getTotalProtocolFeesThisEpoch() - external - view - returns (uint256 _totalFeesCollectedThisEpoch) - { - _totalFeesCollectedThisEpoch = totalFeesCollectedThisEpoch; - } - /// @dev Returns the total balance of this contract, including WETH. /// @return totalBalance Total balance. function getTotalBalance() @@ -162,8 +138,9 @@ contract MixinExchangeFees is returns (uint256 totalBalance) { totalBalance = address(this).balance.safeAdd( - IEtherToken(WETH_ADDRESS).balanceOf(address(this)) + IEtherToken(_getWETHAddress()).balanceOf(address(this)) ); + return totalBalance; } /// @dev Get information on an active staking pool in this epoch. @@ -175,6 +152,7 @@ contract MixinExchangeFees is returns (IStructs.ActivePool memory pool) { pool = _getActivePoolFromEpoch(currentEpoch, poolId); + return pool; } /// @dev Computes the members and weighted stake for a pool at the current @@ -184,8 +162,8 @@ contract MixinExchangeFees is /// @return membersStake Non-operator stake in the pool. /// @return weightedStake Weighted stake of the pool. function _computeMembersAndWeightedStake( - bytes32 poolId, - uint256 totalStake + bytes32 poolId, + uint256 totalStake ) private view @@ -197,10 +175,13 @@ contract MixinExchangeFees is ).currentEpochBalance; membersStake = totalStake.safeSub(operatorStake); weightedStake = operatorStake.safeAdd( - membersStake - .safeMul(rewardDelegatedStakeWeight) - .safeDiv(PPM_DENOMINATOR) + LibMath.getPartialAmountFloor( + rewardDelegatedStakeWeight, + PPM_DENOMINATOR, + membersStake + ) ); + return (membersStake, weightedStake); } /// @dev Checks that the protocol fee passed into `payProtocolFee()` is @@ -211,21 +192,24 @@ contract MixinExchangeFees is private view { - if (protocolFeePaid == 0 || - (msg.value != protocolFeePaid && msg.value != 0)) { - LibRichErrors.rrevert( - LibStakingRichErrors.InvalidProtocolFeePaymentError( - protocolFeePaid == 0 ? - LibStakingRichErrors - .ProtocolFeePaymentErrorCodes - .ZeroProtocolFeePaid : - LibStakingRichErrors - .ProtocolFeePaymentErrorCodes - .MismatchedFeeAndPayment, - protocolFeePaid, - msg.value - ) - ); + if (protocolFeePaid != 0) { + return; } + if (msg.value == protocolFeePaid || msg.value == 0) { + return; + } + LibRichErrors.rrevert( + LibStakingRichErrors.InvalidProtocolFeePaymentError( + protocolFeePaid == 0 ? + LibStakingRichErrors + .ProtocolFeePaymentErrorCodes + .ZeroProtocolFeePaid : + LibStakingRichErrors + .ProtocolFeePaymentErrorCodes + .MismatchedFeeAndPayment, + protocolFeePaid, + msg.value + ) + ); } } diff --git a/contracts/staking/contracts/src/immutable/MixinDeploymentConstants.sol b/contracts/staking/contracts/src/immutable/MixinDeploymentConstants.sol index 53d716e318..3a61137588 100644 --- a/contracts/staking/contracts/src/immutable/MixinDeploymentConstants.sol +++ b/contracts/staking/contracts/src/immutable/MixinDeploymentConstants.sol @@ -58,4 +58,15 @@ contract MixinDeploymentConstants { LibRichErrors.rrevert(LibStakingRichErrors.InvalidWethAssetDataError()); } } + + /// @dev An overridable way to access the deployed WETH address. + /// Must be view to allow overrides to access state. + /// @return wethAddress The address of the configured WETH contract. + function _getWETHAddress() + internal + view + returns (address wethAddress) + { + return WETH_ADDRESS; + } } diff --git a/contracts/staking/contracts/src/immutable/MixinStorage.sol b/contracts/staking/contracts/src/immutable/MixinStorage.sol index 2e247d1a1b..38445d7975 100644 --- a/contracts/staking/contracts/src/immutable/MixinStorage.sol +++ b/contracts/staking/contracts/src/immutable/MixinStorage.sol @@ -143,30 +143,13 @@ contract MixinStorage is /// @dev State information for each active pool in an epoch. /// In practice, we only store state for `currentEpoch % 2`. - mapping(uint256 => mapping(bytes32 => IStructs.ActivePool)) - internal - _activePoolsByEpoch; + mapping (uint256 => mapping (bytes32 => IStructs.ActivePool)) internal _activePoolsByEpoch; /// @dev Number of pools activated in the current epoch. uint256 public numActivePoolsThisEpoch; - /// @dev Rewards (ETH) available to the epoch being finalized (the previous - /// epoch). This is simply the balance of the contract at the end of - /// the epoch. - uint256 public unfinalizedRewardsAvailable; - - /// @dev The number of active pools in the last epoch that have yet to be - /// finalized through `finalizePools()`. - uint256 public unfinalizedPoolsRemaining; - - /// @dev The total fees collected for the epoch being finalized. - uint256 public unfinalizedTotalFeesCollected; - - /// @dev The total fees collected for the epoch being finalized. - uint256 public unfinalizedTotalWeightedStake; - - /// @dev How many rewards were paid at the end of finalization. - uint256 totalRewardsPaidLastEpoch; + /// @dev State for unfinalized rewards. + IStructs.UnfinalizedState public unfinalizedState; /// @dev Adds owner as an authorized address. constructor() diff --git a/contracts/staking/contracts/src/interfaces/IEthVault.sol b/contracts/staking/contracts/src/interfaces/IEthVault.sol index 225658768f..cf5e790402 100644 --- a/contracts/staking/contracts/src/interfaces/IEthVault.sol +++ b/contracts/staking/contracts/src/interfaces/IEthVault.sol @@ -42,29 +42,27 @@ interface IEthVault { uint256 amount ); - /// @dev Record a deposit of an amount of ETH for `owner` into the vault. - /// The staking contract should pay this contract the ETH owed in the - /// same transaction. + /// @dev Deposit an `amount` of WETH for `owner` into the vault. + /// The staking contract should have granted the vault an allowance + /// because it will pull the WETH via `transferFrom()`. /// Note that this is only callable by the staking contract. - /// @param owner Owner of the ETH. + /// @param owner Owner of the WETH. /// @param amount Amount of deposit. - function recordDepositFor(address owner, uint256 amount) + function depositFor(address owner, uint256 amount) external; - /// @dev Withdraw an `amount` of ETH to `msg.sender` from the vault. - /// Note that only the Staking contract can call this. - /// Note that this can only be called when *not* in Catostrophic Failure mode. - /// @param amount of ETH to withdraw. + /// @dev Withdraw an `amount` of WETH to `msg.sender` from the vault. + /// @param amount of WETH to withdraw. function withdraw(uint256 amount) external; - /// @dev Withdraw ALL ETH to `msg.sender` from the vault. + /// @dev Withdraw ALL WETH to `msg.sender` from the vault. function withdrawAll() external returns (uint256); - /// @dev Returns the balance in ETH of the `owner` - /// @return Balance in ETH. + /// @dev Returns the balance in WETH of the `owner` + /// @return Balance in WETH. function balanceOf(address owner) external view diff --git a/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol b/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol index ed5745e5d6..15b71f2b49 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingPoolRewardVault.sol @@ -23,40 +23,40 @@ pragma experimental ABIEncoderV2; /// @dev This vault manages staking pool rewards. interface IStakingPoolRewardVault { - /// @dev Emitted when Eth is deposited into the vault. + /// @dev Emitted when WETH is deposited into the vault. /// @param sender Address of sender (`msg.sender`). - /// @param poolId that owns of Eth. - /// @param amount of Eth deposited. + /// @param poolId that owns of WETH. + /// @param amount of WETH deposited. event EthDepositedIntoVault( address indexed sender, bytes32 indexed poolId, uint256 amount ); - /// @dev Emitted when rewards are transferred out fo the vault. + /// @dev Emitted when rewards are transferred out of the vault. /// @param poolId Unique Id of pool. /// @param to Address to send funds to. - /// @param amount Amount of ETH to transfer. + /// @param amount Amount of WETH to transfer. event PoolRewardTransferred( bytes32 indexed poolId, - address to, + address indexed to, uint256 amount ); - /// @dev Record a deposit of an amount of ETH for `poolId` into the vault. - /// The staking contract should pay this contract the ETH owed in the - /// same transaction. + /// @dev Deposit an amount of WETH for `poolId` into the vault. + /// The staking contract should have granted the vault an allowance + /// because it will pull the WETH via `transferFrom()`. /// Note that this is only callable by the staking contract. - /// @param poolId Pool that holds the ETH. + /// @param poolId Pool that holds the WETH. /// @param amount Amount of deposit. - function recordDepositFor(bytes32 poolId, uint256 amount) + function depositFor(bytes32 poolId, uint256 amount) external; - /// @dev Withdraw some amount in ETH from a pool. + /// @dev Withdraw some amount in WETH from a pool. /// Note that this is only callable by the staking contract. /// @param poolId Unique Id of pool. /// @param to Address to send funds to. - /// @param amount Amount of ETH to transfer. + /// @param amount Amount of WETH to transfer. function transfer( bytes32 poolId, address payable to, @@ -64,8 +64,8 @@ interface IStakingPoolRewardVault { ) external; - /// @dev Returns the balance in ETH of `poolId` - /// @return Balance in ETH. + /// @dev Returns the balance in WETH of `poolId` + /// @return Balance in WETH. function balanceOf(bytes32 poolId) external view diff --git a/contracts/staking/contracts/src/interfaces/IStructs.sol b/contracts/staking/contracts/src/interfaces/IStructs.sol index 4d738ea3dd..f53ac81efb 100644 --- a/contracts/staking/contracts/src/interfaces/IStructs.sol +++ b/contracts/staking/contracts/src/interfaces/IStructs.sol @@ -32,6 +32,23 @@ interface IStructs { uint256 membersStake; } + /// @dev Holds state for unfinalized epoch rewards. + /// @param rewardsAvailable Rewards (ETH) available to the epoch + /// being finalized (the previous epoch). This is simply the balance + /// of the contract at the end of the epoch. + /// @param poolsRemaining The number of active pools in the last + /// epoch that have yet to be finalized through `finalizePools()`. + /// @param totalFeesCollected The total fees collected for the epoch being finalized. + /// @param totalWeightedStake The total fees collected for the epoch being finalized. + /// @param totalRewardsFinalized Amount of rewards that have been paid during finalization. + struct UnfinalizedState { + uint256 rewardsAvailable; + uint256 poolsRemaining; + uint256 totalFeesCollected; + uint256 totalWeightedStake; + uint256 totalRewardsFinalized; + } + /// @dev Encapsulates a balance for the current and next epochs. /// Note that these balances may be stale if the current epoch /// is greater than `currentEpoch`. @@ -87,14 +104,6 @@ interface IStructs { bool confirmed; } - /// @dev Encapsulates the epoch and value of a cumulative reward. - /// @param cumulativeRewardEpoch Epoch of the reward. - /// @param cumulativeReward Value of the reward. - struct CumulativeRewardInfo { - 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. diff --git a/contracts/staking/contracts/src/libs/LibCobbDouglas.sol b/contracts/staking/contracts/src/libs/LibCobbDouglas.sol index fa5c0e62d3..aec288fe1f 100644 --- a/contracts/staking/contracts/src/libs/LibCobbDouglas.sol +++ b/contracts/staking/contracts/src/libs/LibCobbDouglas.sol @@ -41,7 +41,7 @@ library LibCobbDouglas { /// @param alphaNumerator Numerator of `alpha` in the cobb-douglas function. /// @param alphaDenominator Denominator of `alpha` in the cobb-douglas /// function. - /// @return ownerRewards Rewards owned to the staking pool. + /// @return rewards Rewards owed to the staking pool. function cobbDouglas( uint256 totalRewards, uint256 fees, @@ -53,12 +53,12 @@ library LibCobbDouglas { ) internal pure - returns (uint256 ownerRewards) + returns (uint256 rewards) { int256 feeRatio = LibFixedMath.toFixed(fees, totalFees); int256 stakeRatio = LibFixedMath.toFixed(stake, totalStake); if (feeRatio == 0 || stakeRatio == 0) { - return ownerRewards = 0; + return rewards = 0; } // The cobb-doublas function has the form: // `totalRewards * feeRatio ^ alpha * stakeRatio ^ (1-alpha)` @@ -93,6 +93,6 @@ library LibCobbDouglas { LibFixedMath.mul(stakeRatio, n) : LibFixedMath.div(stakeRatio, n); // Multiply the above with totalRewards. - ownerRewards = LibFixedMath.uintMul(n, totalRewards); + rewards = LibFixedMath.uintMul(n, totalRewards); } } diff --git a/contracts/staking/contracts/src/stake/MixinStake.sol b/contracts/staking/contracts/src/stake/MixinStake.sol index b68d080be5..412f224f7a 100644 --- a/contracts/staking/contracts/src/stake/MixinStake.sol +++ b/contracts/staking/contracts/src/stake/MixinStake.sol @@ -158,10 +158,8 @@ contract MixinStake is : 0; // execute move - IStructs.StoredBalance storage fromPtr = - _getBalancePtrFromStatus(owner, from.status); - IStructs.StoredBalance storage toPtr = - _getBalancePtrFromStatus(owner, to.status); + IStructs.StoredBalance storage fromPtr = _getBalancePtrFromStatus(owner, from.status); + IStructs.StoredBalance storage toPtr = _getBalancePtrFromStatus(owner, to.status); _moveStake(fromPtr, toPtr, amount); // update global total of stake in the statuses being moved between diff --git a/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol index ef0b4be8b7..06fd6c5c8f 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol @@ -80,7 +80,7 @@ contract MixinCumulativeRewards is if (_cumulativeRewardsByPoolReferenceCounter[poolId][epoch] != 0) { return false; } - // Must not be the most recent CR. + // Must not be the most recently *stored* CR. if (_cumulativeRewardsByPoolLastStored[poolId] == epoch) { return false; } @@ -150,25 +150,6 @@ contract MixinCumulativeRewards is delete _cumulativeRewardsByPool[poolId][epoch]; } - /// @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 - uint256 cumulativeRewardsLastStored = - _cumulativeRewardsByPoolLastStored[poolId]; - - // Query and return cumulative reward info for this pool - return IStructs.CumulativeRewardInfo({ - cumulativeReward: - _cumulativeRewardsByPool[poolId][cumulativeRewardsLastStored], - cumulativeRewardEpoch: cumulativeRewardsLastStored - }); - } - /// @dev Tries to set the epoch of the most recent cumulative reward. /// The value will only be set if the input epoch is greater than the /// current most recent value. @@ -219,14 +200,13 @@ contract MixinCumulativeRewards is /// @dev Adds a dependency on a cumulative reward for a given epoch. /// @param poolId Unique Id of pool. /// @param epoch Epoch to remove dependency from. - /// @param mostRecentCumulativeRewardInfo Info for the most recent - /// cumulative reward (value and epoch) + /// @param mostRecentCumulativeReward The most recent cumulative reward. /// @param isDependent True iff there is a dependency on the cumulative /// reward for `poolId` at `epoch` function _addOrRemoveDependencyOnCumulativeReward( bytes32 poolId, uint256 epoch, - IStructs.CumulativeRewardInfo memory mostRecentCumulativeRewardInfo, + IStructs.Fraction memory mostRecentCumulativeReward, bool isDependent ) internal @@ -235,7 +215,7 @@ contract MixinCumulativeRewards is _addDependencyOnCumulativeReward( poolId, epoch, - mostRecentCumulativeRewardInfo + mostRecentCumulativeReward ); } else { _removeDependencyOnCumulativeReward( @@ -313,7 +293,7 @@ contract MixinCumulativeRewards is } // Compute reward - reward = LibFractions.scaleFractionalDifference( + reward = LibFractions.scaleDifference( endReward.numerator, endReward.denominator, beginReward.numerator, @@ -322,15 +302,26 @@ contract MixinCumulativeRewards is ); } + /// @dev Fetch the most recent cumulative reward entry for a pool. + /// @param poolId Unique ID of pool. + /// @return cumulativeReward The most recent cumulative reward `poolId`. + function _getMostRecentCumulativeReward(bytes32 poolId) + internal + view + returns (IStructs.Fraction memory cumulativeReward) + { + uint256 lastStoredEpoch = _cumulativeRewardsByPoolLastStored[poolId]; + return _cumulativeRewardsByPool[poolId][lastStoredEpoch]; + } + /// @dev Adds a dependency on a cumulative reward for a given epoch. /// @param poolId Unique Id of pool. /// @param epoch Epoch to remove dependency from. - /// @param mostRecentCumulativeRewardInfo Info on the most recent cumulative - /// reward. + /// @param mostRecentCumulativeReward The most recent cumulative reward. function _addDependencyOnCumulativeReward( bytes32 poolId, uint256 epoch, - IStructs.CumulativeRewardInfo memory mostRecentCumulativeRewardInfo + IStructs.Fraction memory mostRecentCumulativeReward ) private { @@ -342,7 +333,7 @@ contract MixinCumulativeRewards is _trySetCumulativeReward( poolId, epoch, - mostRecentCumulativeRewardInfo.cumulativeReward + mostRecentCumulativeReward ); } @@ -356,10 +347,8 @@ contract MixinCumulativeRewards is private { // Remove dependency by decreasing reference counter - uint256 newReferenceCounter = - _cumulativeRewardsByPoolReferenceCounter[poolId][epoch].safeSub(1); _cumulativeRewardsByPoolReferenceCounter[poolId][epoch] = - newReferenceCounter; + _cumulativeRewardsByPoolReferenceCounter[poolId][epoch].safeSub(1); // Clear cumulative reward from state, if it is no longer needed _tryUnsetCumulativeReward(poolId, epoch); diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index b44a10a52b..d3cb07f36b 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -84,11 +84,12 @@ contract MixinStakingPoolRewards is (uint256 unfinalizedTotalRewards, uint256 unfinalizedMembersStake) = _getUnfinalizedPoolRewards(poolId); // Get the operators' portion. - (reward,) = _splitStakingPoolRewards( + (reward,) = _computeSplitStakingPoolRewards( pool.operatorShare, unfinalizedTotalRewards, unfinalizedMembersStake ); + return reward; } /// @dev Computes the reward balance in ETH of a specific member of a pool. @@ -106,12 +107,12 @@ contract MixinStakingPoolRewards is (uint256 unfinalizedTotalRewards, uint256 unfinalizedMembersStake) = _getUnfinalizedPoolRewards(poolId); // Get the members' portion. - (, uint256 unfinalizedMembersReward) = _splitStakingPoolRewards( + (, uint256 unfinalizedMembersReward) = _computeSplitStakingPoolRewards( pool.operatorShare, unfinalizedTotalRewards, unfinalizedMembersStake ); - reward = _computeRewardBalanceOfDelegator( + return _computeRewardBalanceOfDelegator( poolId, _loadUnsyncedBalance(_delegatedStakeToPoolByOwner[member][poolId]), currentEpoch, @@ -137,7 +138,7 @@ contract MixinStakingPoolRewards is ) internal { - // Rransfer any rewards from the transient pool vault to the eth vault; + // Transfer any rewards from the transient pool vault to the eth vault; // this must be done before we can modify the owner's portion of the // delegator pool. _transferDelegatorRewardsToEthVault( @@ -165,17 +166,17 @@ contract MixinStakingPoolRewards is } /// @dev Handles a pool's reward at the current epoch. - /// This will compute the reward split and record the cumulative - /// reward, which is used to compute each delegator's portion of the - /// members' reward. It will NOT make any transfers to the eth or - /// reward vaults. That should be done with a separate call to - /// `_depositStakingPoolRewards()``. + /// This will split the reward between the operator and members, + /// depositing them into their respective vaults, and update the + /// accounting needed to allow members to withdraw their individual + /// rewards. /// @param poolId Unique Id of pool. /// @param reward received by the pool. /// @param membersStake the amount of non-operator delegated stake that /// will split the reward. - /// @return operatorReward - function _recordStakingPoolRewards( + /// @return operatorReward Portion of `reward` given to the pool operator. + /// @return membersReward Portion of `reward` given to the pool members. + function _depositStakingPoolRewards( bytes32 poolId, uint256 reward, uint256 membersStake @@ -186,71 +187,61 @@ contract MixinStakingPoolRewards is IStructs.Pool memory pool = _poolById[poolId]; // Split the reward between operator and members - (operatorReward, membersReward) = - _splitStakingPoolRewards(pool.operatorShare, reward, membersStake); - - // Record the operator's reward in the eth vault. - ethVault.recordDepositFor(pool.operator, operatorReward); + (operatorReward, membersReward) = _computeSplitStakingPoolRewards( + pool.operatorShare, + reward, + membersStake + ); + // Deposit the operator's reward in the eth vault. + ethVault.depositFor(pool.operator, operatorReward); if (membersReward == 0) { - return (operatorReward, membersReward); + return (0, 0); } - // Record the members reward in the reward vault. - rewardVault.recordDepositFor(poolId, membersReward); - - // Cache a storage pointer to the cumulative rewards for `poolId` - // indexed by epoch. - mapping (uint256 => IStructs.Fraction) - storage - _cumulativeRewardsByPoolPtr = _cumulativeRewardsByPool[poolId]; + // Deposit the members' reward in the reward vault. + rewardVault.depositFor(poolId, membersReward); // Fetch the last epoch at which we stored an entry for this pool; // this is the most up-to-date cumulative rewards for this pool. - uint256 cumulativeRewardsLastStored = - _cumulativeRewardsByPoolLastStored[poolId]; - IStructs.Fraction memory mostRecentCumulativeRewards = - _cumulativeRewardsByPoolPtr[cumulativeRewardsLastStored]; + IStructs.Fraction memory mostRecentCumulativeReward = + _getMostRecentCumulativeReward(poolId); // Compute new cumulative reward - (uint256 numerator, uint256 denominator) = LibFractions.addFractions( - mostRecentCumulativeRewards.numerator, - mostRecentCumulativeRewards.denominator, - membersReward, - membersStake - ); + IStructs.Fraction memory cumulativeReward; + (cumulativeReward.numerator, cumulativeReward.denominator) = + LibFractions.add( + mostRecentCumulativeReward.numerator, + mostRecentCumulativeReward.denominator, + membersReward, + membersStake + ); + // Normalize to prevent overflows. + (cumulativeReward.numerator, cumulativeReward.denominator) = + LibFractions.normalize( + cumulativeReward.numerator, + cumulativeReward.denominator + ); - // store cumulative rewards and set most recent + // Store cumulative rewards for this epoch. _forceSetCumulativeReward( poolId, currentEpoch, - IStructs.Fraction(numerator, denominator) + cumulativeReward ); + + return (operatorReward, membersReward); } - /// @dev Deposit rewards into the eth vault and reward vault for pool - /// operators and members rewards, respectively. This should be called - /// in tandem with `_recordStakingPoolRewards()`. We separate them - /// so we can bath deposits, because ETH transfers are expensive. - /// @param operatorReward Operator rewards. - /// @param membersReward Operator rewards. - function _depositStakingPoolRewards( - uint256 operatorReward, - uint256 membersReward - ) - internal - { - address(uint160(address(ethVault))).transfer(operatorReward); - address(uint160(address(rewardVault))).transfer(membersReward); - } - - /// @dev Split a pool reward between the operator and members based on - /// the `operatorShare` and `membersStake`. + /// @dev Compute the split of a pool reward between the operator and members + /// based on the `operatorShare` and `membersStake`. /// @param operatorShare The fraction of rewards owed to the operator, /// in PPM. /// @param totalReward The pool reward. /// @param membersStake The amount of member (non-operator) stake delegated /// to the pool in the epoch the rewards were earned. - function _splitStakingPoolRewards( + /// @return operatorReward Portion of `totalReward` given to the pool operator. + /// @return membersReward Portion of `totalReward` given to the pool members. + function _computeSplitStakingPoolRewards( uint32 operatorShare, uint256 totalReward, uint256 membersStake @@ -269,6 +260,7 @@ contract MixinStakingPoolRewards is ); membersReward = totalReward - operatorReward; } + return (operatorReward, membersReward); } /// @dev Transfers a delegators accumulated rewards from the transient pool @@ -286,7 +278,7 @@ contract MixinStakingPoolRewards is private { // Ensure the pool is finalized. - _finalizePool(poolId); + finalizePool(poolId); // Compute balance owed to delegator uint256 balance = _computeRewardBalanceOfDelegator( @@ -302,13 +294,10 @@ contract MixinStakingPoolRewards is return; } - // Transfer from transient Reward Pool vault to ETH Vault - ethVault.recordDepositFor(member, balance); - rewardVault.transfer( - poolId, - address(uint160(address(ethVault))), - balance - ); + // Transfer from RewardVault to this contract. + rewardVault.transfer(poolId, address(uint160(address(this))), balance); + // Transfer to EthVault. + ethVault.depositFor(member, balance); } /// @dev Computes the reward balance in ETH of a specific member of a pool. @@ -333,14 +322,14 @@ contract MixinStakingPoolRewards is // There can be no rewards in epoch 0 because there is no delegated // stake. if (currentEpoch == 0) { - return reward = 0; + return 0; } // There can be no rewards if the last epoch when stake was synced is // equal to the current epoch, because all prior rewards, including // rewards finalized this epoch have been claimed. if (unsyncedStake.currentEpoch == currentEpoch) { - return reward = 0; + return 0; } // If there are unfinalized rewards this epoch, compute the member's @@ -348,13 +337,15 @@ contract MixinStakingPoolRewards is if (unfinalizedMembersReward != 0 && unfinalizedMembersStake != 0) { // Unfinalized rewards are always earned from stake in // the prior epoch so we want the stake at `currentEpoch-1`. - uint256 _stake = unsyncedStake.currentEpoch >= currentEpoch - 1 ? + uint256 _stake = unsyncedStake.currentEpoch >= currentEpoch.safeSub(1) ? unsyncedStake.currentEpochBalance : unsyncedStake.nextEpochBalance; if (_stake != 0) { - reward = _stake - .safeMul(unfinalizedMembersReward) - .safeDiv(unfinalizedMembersStake); + reward = LibMath.getPartialAmountFloor( + unfinalizedMembersReward, + unfinalizedMembersStake, + _stake + ); } } @@ -365,32 +356,30 @@ contract MixinStakingPoolRewards is // If the stake has been touched since the last reward epoch, // it has already been claimed. if (unsyncedStake.currentEpoch >= lastRewardEpoch) { - return reward; + return 0; } // From here we know: `unsyncedStake.currentEpoch < currentEpoch > 0`. - if (unsyncedStake.currentEpoch >= lastRewardEpoch) { - return reward; - } - + uint256 nextStakeEpoch = uint256(unsyncedStake.currentEpoch).safeAdd(1); reward = reward.safeAdd( _computeMemberRewardOverInterval( poolId, unsyncedStake.currentEpochBalance, unsyncedStake.currentEpoch, - unsyncedStake.currentEpoch + 1 + nextStakeEpoch ) ); - if (unsyncedStake.currentEpoch + 1 < lastRewardEpoch) { + if (nextStakeEpoch < lastRewardEpoch) { reward = reward.safeAdd( _computeMemberRewardOverInterval( poolId, unsyncedStake.nextEpochBalance, - unsyncedStake.currentEpoch + 1, + nextStakeEpoch, lastRewardEpoch ) ); } + return reward; } /// @dev Adds or removes cumulative reward dependencies for a delegator. @@ -415,8 +404,8 @@ contract MixinStakingPoolRewards is // Get the most recent cumulative reward, which will serve as a // reference point when updating dependencies - IStructs.CumulativeRewardInfo memory mostRecentCumulativeRewardInfo = - _getMostRecentCumulativeRewardInfo(poolId); + IStructs.Fraction memory mostRecentCumulativeReward = + _getMostRecentCumulativeReward(poolId); // Record dependency on current epoch. if (_delegatedStakeToPoolByOwner.currentEpochBalance != 0 @@ -425,7 +414,7 @@ contract MixinStakingPoolRewards is _addOrRemoveDependencyOnCumulativeReward( poolId, _delegatedStakeToPoolByOwner.currentEpoch, - mostRecentCumulativeRewardInfo, + mostRecentCumulativeReward, isDependent ); } @@ -435,7 +424,7 @@ contract MixinStakingPoolRewards is _addOrRemoveDependencyOnCumulativeReward( poolId, uint256(_delegatedStakeToPoolByOwner.currentEpoch).safeAdd(1), - mostRecentCumulativeRewardInfo, + mostRecentCumulativeReward, isDependent ); } diff --git a/contracts/staking/contracts/src/sys/MixinAbstract.sol b/contracts/staking/contracts/src/sys/MixinAbstract.sol index 987b695b74..5be73219cf 100644 --- a/contracts/staking/contracts/src/sys/MixinAbstract.sol +++ b/contracts/staking/contracts/src/sys/MixinAbstract.sol @@ -29,54 +29,29 @@ contract MixinAbstract { /// @dev Computes the reward owed to a pool during finalization. /// Does nothing if the pool is already finalized. /// @param poolId The pool's ID. - /// @return operatorReward The reward owed to the pool operator. + /// @return totalReward The total reward owed to a pool. /// @return membersStake The total stake for all non-operator members in /// this pool. function _getUnfinalizedPoolRewards(bytes32 poolId) internal view returns ( - uint256 reward, + uint256 totalReward, uint256 membersStake ); - /// @dev Get an active pool from an epoch by its ID. - /// @param epoch The epoch the pool was/will be active in. - /// @param poolId The ID of the pool. - /// @return pool The pool with ID `poolId` that was active in `epoch`. - function _getActivePoolFromEpoch( - uint256 epoch, - bytes32 poolId - ) - internal - view - returns (IStructs.ActivePool memory pool); - - /// @dev Get a mapping of active pools from an epoch. - /// This uses the formula `epoch % 2` as the epoch index in order - /// to reuse state, because we only need to remember, at most, two - /// epochs at once. - /// @return activePools The pools that were active in `epoch`. - function _getActivePoolsFromEpoch( - uint256 epoch - ) - internal - view - returns (mapping (bytes32 => IStructs.ActivePool) storage activePools); - /// @dev Instantly finalizes a single pool that was active in the previous /// epoch, crediting it rewards and sending those rewards to the reward /// and eth vault. This can be called by internal functions that need /// to finalize a pool immediately. Does nothing if the pool is already - /// finalized. Does nothing if the pool was not active or was already - /// finalized. + /// finalized or was not active in the previous epoch. /// @param poolId The pool ID to finalize. /// @return operatorReward The reward credited to the pool operator. /// @return membersReward The reward credited to the pool members. /// @return membersStake The total stake for all non-operator members in /// this pool. - function _finalizePool(bytes32 poolId) - internal + function finalizePool(bytes32 poolId) + public returns ( uint256 operatorReward, uint256 membersReward, diff --git a/contracts/staking/contracts/src/sys/MixinFinalizer.sol b/contracts/staking/contracts/src/sys/MixinFinalizer.sol index b58fccf14a..58557eb7bc 100644 --- a/contracts/staking/contracts/src/sys/MixinFinalizer.sol +++ b/contracts/staking/contracts/src/sys/MixinFinalizer.sol @@ -58,43 +58,41 @@ contract MixinFinalizer is /// Throws if not enough time has passed between epochs or if the /// previous epoch was not fully finalized. /// If there were no active pools in the closing epoch, the epoch - /// will be instantly finalized here. Otherwise, `finalizePools()` + /// will be instantly finalized here. Otherwise, `finalizePool()` /// should be called on each active pool afterwards. - /// @return _unfinalizedPoolsRemaining The number of unfinalized pools. + /// @return poolsRemaining The number of unfinalized pools. function endEpoch() external - returns (uint256 _unfinalizedPoolsRemaining) + returns (uint256 poolsRemaining) { uint256 closingEpoch = currentEpoch; // Make sure the previous epoch has been fully finalized. - if (unfinalizedPoolsRemaining != 0) { + if (poolsRemaining != 0) { LibRichErrors.rrevert( LibStakingRichErrors.PreviousEpochNotFinalizedError( closingEpoch.safeSub(1), - unfinalizedPoolsRemaining + poolsRemaining ) ); } - // Unrwap any WETH protocol fees. - _unwrapWETH(); - - // Populate finalization state. - unfinalizedPoolsRemaining = - _unfinalizedPoolsRemaining = numActivePoolsThisEpoch; - unfinalizedRewardsAvailable = address(this).balance; - unfinalizedTotalFeesCollected = totalFeesCollectedThisEpoch; - unfinalizedTotalWeightedStake = totalWeightedStakeThisEpoch; - totalRewardsPaidLastEpoch = 0; + // Set up unfinalized state. + IStructs.UnfinalizedState memory state; + state.rewardsAvailable = _wrapBalanceToWETHAndGetBalance(); + state.poolsRemaining = poolsRemaining = numActivePoolsThisEpoch; + state.totalFeesCollected = totalFeesCollectedThisEpoch; + state.totalWeightedStake = totalWeightedStakeThisEpoch; + state.totalRewardsFinalized = 0; + unfinalizedState = state; // Emit an event. emit EpochEnded( closingEpoch, - unfinalizedPoolsRemaining, - unfinalizedRewardsAvailable, - unfinalizedTotalFeesCollected, - unfinalizedTotalWeightedStake + state.poolsRemaining, + state.rewardsAvailable, + state.totalFeesCollected, + state.totalWeightedStake ); // Reset current epoch state. @@ -106,91 +104,8 @@ contract MixinFinalizer is _goToNextEpoch(); // If there were no active pools, the epoch is already finalized. - if (unfinalizedPoolsRemaining == 0) { - emit EpochFinalized(closingEpoch, 0, unfinalizedRewardsAvailable); - } - } - - /// @dev Finalizes pools that were active in the previous epoch, paying out - /// rewards to the reward and eth vault. Keepers should call this - /// function repeatedly until all active pools that were emitted in in - /// a `StakingPoolActivated` in the prior epoch have been finalized. - /// Pools that have already been finalized will be silently ignored. - /// We deliberately try not to revert here in case multiple parties - /// are finalizing pools. - /// @param poolIds List of active pool IDs to finalize. - /// @return _unfinalizedPoolsRemaining The number of unfinalized pools left. - function finalizePools(bytes32[] calldata poolIds) - external - returns (uint256 _unfinalizedPoolsRemaining) - { - uint256 epoch = currentEpoch; - // There are no pools to finalize at epoch 0. - if (epoch == 0) { - return _unfinalizedPoolsRemaining = 0; - } - - uint256 poolsRemaining = unfinalizedPoolsRemaining; - // If there are no more unfinalized pools remaining, there's nothing - // to do. if (poolsRemaining == 0) { - return _unfinalizedPoolsRemaining = 0; - } - - // Pointer to the active pools in the last epoch. - mapping(bytes32 => IStructs.ActivePool) storage activePools = - _getActivePoolsFromEpoch(epoch - 1); - uint256 numPoolIds = poolIds.length; - uint256 rewardsPaid = 0; - uint256 totalOperatorRewardsPaid = 0; - uint256 totalMembersRewardsPaid = 0; - - for (uint256 i = 0; i != numPoolIds && poolsRemaining != 0; ++i) - { - bytes32 poolId = poolIds[i]; - IStructs.ActivePool memory pool = activePools[poolId]; - - // Ignore pools that aren't active. - if (pool.feesCollected == 0) { - continue; - } - - (uint256 operatorReward, uint256 membersReward) = - _creditRewardsToPool(epoch, poolId, pool, rewardsPaid); - - totalOperatorRewardsPaid = - totalOperatorRewardsPaid.safeAdd(operatorReward); - totalMembersRewardsPaid = - totalMembersRewardsPaid.safeAdd(membersReward); - - rewardsPaid = rewardsPaid - .safeAdd(operatorReward) - .safeAdd(membersReward); - - // Decrease the number of unfinalized pools left. - poolsRemaining = poolsRemaining.safeSub(1); - } - - // Update finalization states. - if (rewardsPaid != 0) { - totalRewardsPaidLastEpoch = - totalRewardsPaidLastEpoch.safeAdd(rewardsPaid); - } - unfinalizedPoolsRemaining = _unfinalizedPoolsRemaining = poolsRemaining; - - // If there are no more unfinalized pools remaining, the epoch is - // finalized. - if (poolsRemaining == 0) { - emit EpochFinalized( - epoch - 1, - totalRewardsPaidLastEpoch, - unfinalizedRewardsAvailable.safeSub(totalRewardsPaidLastEpoch) - ); - } - - // Deposit all the rewards at once. - if (rewardsPaid != 0) { - _depositStakingPoolRewards(totalOperatorRewardsPaid, totalMembersRewardsPaid); + emit EpochFinalized(closingEpoch, 0, state.rewardsAvailable); } } @@ -198,15 +113,14 @@ contract MixinFinalizer is /// epoch, crediting it rewards and sending those rewards to the reward /// and eth vault. This can be called by internal functions that need /// to finalize a pool immediately. Does nothing if the pool is already - /// finalized. Does nothing if the pool was not active or was already - /// finalized. + /// finalized or was not active in the previous epoch. /// @param poolId The pool ID to finalize. /// @return operatorReward The reward credited to the pool operator. /// @return membersReward The reward credited to the pool members. /// @return membersStake The total stake for all non-operator members in /// this pool. - function _finalizePool(bytes32 poolId) - internal + function finalizePool(bytes32 poolId) + public returns ( uint256 operatorReward, uint256 membersReward, @@ -216,46 +130,62 @@ contract MixinFinalizer is uint256 epoch = currentEpoch; // There are no pools to finalize at epoch 0. if (epoch == 0) { - return (operatorReward, membersReward, membersStake); + return (0, 0, 0); + } + uint256 prevEpoch = epoch - 1; + + // Load the finalization state into memory. + IStructs.UnfinalizedState memory state = unfinalizedState; + + // If there are no more unfinalized pools remaining, there's nothing + // to do. + if (state.poolsRemaining == 0) { + return (0, 0, 0); } - IStructs.ActivePool memory pool = - _getActivePoolFromEpoch(epoch - 1, poolId); - // Do nothing if the pool was not active (has no fees). + IStructs.ActivePool memory pool = _getActivePoolFromEpoch(prevEpoch, poolId); + // Do nothing if the pool was not active or already finalized (has no fees). if (pool.feesCollected == 0) { return (operatorReward, membersReward, membersStake); } - (operatorReward, membersReward) = - _creditRewardsToPool(epoch, poolId, pool, 0); + (operatorReward, membersReward) = _creditRewardsToPool( + epoch, + poolId, + pool, + state + ); uint256 totalReward = operatorReward.safeAdd(membersReward); if (totalReward > 0) { - totalRewardsPaidLastEpoch = - totalRewardsPaidLastEpoch.safeAdd(totalReward); - _depositStakingPoolRewards(operatorReward, membersReward); + // Increase `totalRewardsFinalized`. + unfinalizedState.totalRewardsFinalized = + state.totalRewardsFinalized = + state.totalRewardsFinalized.safeAdd(totalReward); } // Decrease the number of unfinalized pools left. - uint256 poolsRemaining = unfinalizedPoolsRemaining; - unfinalizedPoolsRemaining = poolsRemaining = poolsRemaining.safeSub(1); + unfinalizedState.poolsRemaining = + state.poolsRemaining = + state.poolsRemaining.safeSub(1); // If there are no more unfinalized pools remaining, the epoch is // finalized. - if (poolsRemaining == 0) { + if (state.poolsRemaining == 0) { emit EpochFinalized( - epoch - 1, - totalRewardsPaidLastEpoch, - unfinalizedRewardsAvailable.safeSub(totalRewardsPaidLastEpoch) + prevEpoch, + state.totalRewardsFinalized, + state.rewardsAvailable.safeSub(state.totalRewardsFinalized) ); } membersStake = pool.membersStake; + return (operatorReward, membersReward, membersStake); } /// @dev Computes the reward owed to a pool during finalization. /// Does nothing if the pool is already finalized. /// @param poolId The pool's ID. - /// @return operatorReward The reward owed to the pool operator. + /// @return totalReward The total reward owed to a pool. /// @return membersStake The total stake for all non-operator members in /// this pool. function _getUnfinalizedPoolRewards(bytes32 poolId) @@ -269,11 +199,10 @@ contract MixinFinalizer is uint256 epoch = currentEpoch; // There are no pools to finalize at epoch 0. if (epoch == 0) { - return (reward, membersStake); + return (0, 0); } - IStructs.ActivePool memory pool = - _getActivePoolFromEpoch(epoch - 1, poolId); - reward = _getUnfinalizedPoolRewards(pool, 0); + IStructs.ActivePool memory pool = _getActivePoolFromEpoch(epoch - 1, poolId); + reward = _getUnfinalizedPoolRewards(pool, unfinalizedState); membersStake = pool.membersStake; } @@ -290,6 +219,7 @@ contract MixinFinalizer is returns (IStructs.ActivePool memory pool) { pool = _getActivePoolsFromEpoch(epoch)[poolId]; + return pool; } /// @dev Get a mapping of active pools from an epoch. @@ -305,26 +235,32 @@ contract MixinFinalizer is returns (mapping (bytes32 => IStructs.ActivePool) storage activePools) { activePools = _activePoolsByEpoch[epoch % 2]; + return activePools; } - /// @dev Converts the entire WETH balance of the contract into ETH. - function _unwrapWETH() + /// @dev Converts the entire ETH balance of the contract into WETH and + /// returns the total WETH balance of this contract. + /// @return The WETH balance of this contract. + function _wrapBalanceToWETHAndGetBalance() internal + returns (uint256 balance) { - uint256 wethBalance = IEtherToken(WETH_ADDRESS) - .balanceOf(address(this)); - if (wethBalance != 0) { - IEtherToken(WETH_ADDRESS).withdraw(wethBalance); + IEtherToken weth = IEtherToken(_getWETHAddress()); + uint256 ethBalance = address(this).balance; + if (ethBalance != 0) { + weth.deposit.value((address(this).balance)); } + balance = weth.balanceOf(address(this)); + return balance; } /// @dev Computes the reward owed to a pool during finalization. /// @param pool The active pool. - /// @param unpaidRewards Rewards that have been credited but not finalized. + /// @param state The current state of finalization. /// @return rewards Unfinalized rewards for this pool. function _getUnfinalizedPoolRewards( IStructs.ActivePool memory pool, - uint256 unpaidRewards + IStructs.UnfinalizedState memory state ) private view @@ -333,27 +269,25 @@ contract MixinFinalizer is // There can't be any rewards if the pool was active or if it has // no stake. if (pool.feesCollected == 0) { - return rewards = 0; + return rewards; } - uint256 unfinalizedRewardsAvailable_ = unfinalizedRewardsAvailable; // Use the cobb-douglas function to compute the total reward. - rewards = LibCobbDouglas._cobbDouglas( - unfinalizedRewardsAvailable_, + rewards = LibCobbDouglas.cobbDouglas( + state.rewardsAvailable, pool.feesCollected, - unfinalizedTotalFeesCollected, + state.totalFeesCollected, pool.weightedStake, - unfinalizedTotalWeightedStake, + state.totalWeightedStake, cobbDouglasAlphaNumerator, cobbDouglasAlphaDenominator ); // Clip the reward to always be under - // `unfinalizedRewardsAvailable - totalRewardsPaid - unpaidRewards`, + // `rewardsAvailable - totalRewardsPaid`, // in case cobb-douglas overflows, which should be unlikely. - uint256 rewardsRemaining = unfinalizedRewardsAvailable_ - .safeSub(totalRewardsPaidLastEpoch) - .safeSub(unpaidRewards); + uint256 rewardsRemaining = + state.rewardsAvailable.safeSub(state.totalRewardsFinalized); if (rewardsRemaining < rewards) { rewards = rewardsRemaining; } @@ -364,30 +298,29 @@ contract MixinFinalizer is /// @param epoch The current epoch. /// @param poolId The pool ID to finalize. /// @param pool The active pool to finalize. - /// @param unpaidRewards Rewards that have been credited but not finalized. - /// @return rewards Rewards. + /// @param state The current state of finalization. /// @return operatorReward The reward credited to the pool operator. /// @return membersReward The reward credited to the pool members. function _creditRewardsToPool( uint256 epoch, bytes32 poolId, IStructs.ActivePool memory pool, - uint256 unpaidRewards + IStructs.UnfinalizedState memory state ) private returns (uint256 operatorReward, uint256 membersReward) { // Clear the pool state so we don't finalize it again, and to recoup // some gas. - delete _getActivePoolsFromEpoch(epoch - 1)[poolId]; + delete _getActivePoolsFromEpoch(epoch.safeSub(1))[poolId]; // Compute the rewards. - uint256 rewards = _getUnfinalizedPoolRewards(pool, unpaidRewards); + uint256 rewards = _getUnfinalizedPoolRewards(pool, state); - // Credit the pool. + // Pay the pool. // Note that we credit at the CURRENT epoch even though these rewards // were earned in the previous epoch. - (operatorReward, membersReward) = _recordStakingPoolRewards( + (operatorReward, membersReward) = _depositStakingPoolRewards( poolId, rewards, pool.membersStake diff --git a/contracts/staking/contracts/src/sys/MixinParams.sol b/contracts/staking/contracts/src/sys/MixinParams.sol index cf8c14ab51..2c0b7b29e6 100644 --- a/contracts/staking/contracts/src/sys/MixinParams.sol +++ b/contracts/staking/contracts/src/sys/MixinParams.sol @@ -18,9 +18,11 @@ pragma solidity ^0.5.9; +import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol"; import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-asset-proxy/contracts/src/interfaces/IAssetProxy.sol"; import "../immutable/MixinStorage.sol"; +import "../immutable/MixinDeploymentConstants.sol"; import "../interfaces/IStakingEvents.sol"; import "../interfaces/IEthVault.sol"; import "../interfaces/IStakingPoolRewardVault.sol"; @@ -31,6 +33,7 @@ import "../libs/LibStakingRichErrors.sol"; contract MixinParams is IStakingEvents, MixinConstants, + MixinDeploymentConstants, Ownable, MixinStorage { @@ -194,6 +197,11 @@ contract MixinParams is ) private { + _transferWETHAllownces( + [address(ethVault), address(rewardVault)], + [_ethVaultAddress, _rewardVaultAddress] + ); + epochDurationInSeconds = _epochDurationInSeconds; rewardDelegatedStakeWeight = _rewardDelegatedStakeWeight; minimumPoolStake = _minimumPoolStake; @@ -218,4 +226,24 @@ contract MixinParams is _zrxVaultAddress ); } + + /// @dev Rescind the WETH allowance for `oldSpenders` and grant `newSpenders` + /// an unlimited allowance. + /// @param oldSpenders Addresses to remove allowance from. + /// @param newSpenders Addresses to grant allowance to. + function _transferWETHAllownces( + address[2] memory oldSpenders, + address[2] memory newSpenders + ) + private + { + IEtherToken weth = IEtherToken(_getWETHAddress()); + // Grant new allowances. + for (uint256 i = 0; i < oldSpenders.length; i++) { + // Rescind old allowance. + weth.approve(oldSpenders[i], 0); + // Grant new allowance. + weth.approve(newSpenders[i], uint256(-1)); + } + } } diff --git a/contracts/staking/contracts/src/vaults/EthVault.sol b/contracts/staking/contracts/src/vaults/EthVault.sol index fb31f44ed7..56c05ae11b 100644 --- a/contracts/staking/contracts/src/vaults/EthVault.sol +++ b/contracts/staking/contracts/src/vaults/EthVault.sol @@ -18,52 +18,52 @@ pragma solidity ^0.5.9; +import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol"; import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; import "../interfaces/IEthVault.sol"; +import "../immutable/MixinDeploymentConstants.sol"; import "./MixinVaultCore.sol"; -/// @dev This vault manages ETH. +/// @dev This vault manages WETH. contract EthVault is IEthVault, IVaultCore, + MixinDeploymentConstants, Ownable, MixinVaultCore { using LibSafeMath for uint256; - // mapping from Owner to ETH balance + // mapping from Owner to WETH balance mapping (address => uint256) internal _balances; - // solhint-disable no-empty-blocks - /// @dev Payable fallback for bulk-deposits. - function () external payable {} - - /// @dev Record a deposit of an amount of ETH for `owner` into the vault. - /// The staking contract should pay this contract the ETH owed in the - /// same transaction. + /// @dev Deposit an `amount` of WETH for `owner` into the vault. + /// The staking contract should have granted the vault an allowance + /// because it will pull the WETH via `transferFrom()`. /// Note that this is only callable by the staking contract. - /// @param owner Owner of the ETH. + /// @param owner Owner of the WETH. /// @param amount Amount of deposit. - function recordDepositFor(address owner, uint256 amount) + function depositFor(address owner, uint256 amount) external onlyStakingProxy { + // Transfer WETH from the staking contract into this contract. + IEtherToken(_getWETHAddress()).transferFrom(msg.sender, address(this), amount); + // Credit the owner. _balances[owner] = _balances[owner].safeAdd(amount); emit EthDepositedIntoVault(msg.sender, owner, amount); } - /// @dev Withdraw an `amount` of ETH to `msg.sender` from the vault. - /// Note that only the Staking contract can call this. - /// Note that this can only be called when *not* in Catostrophic Failure mode. - /// @param amount of ETH to withdraw. + /// @dev Withdraw an `amount` of WETH to `msg.sender` from the vault. + /// @param amount of WETH to withdraw. function withdraw(uint256 amount) external { _withdrawFrom(msg.sender, amount); } - /// @dev Withdraw ALL ETH to `msg.sender` from the vault. + /// @dev Withdraw ALL WETH to `msg.sender` from the vault. function withdrawAll() external returns (uint256 totalBalance) @@ -72,13 +72,13 @@ contract EthVault is address payable owner = msg.sender; totalBalance = _balances[owner]; - // withdraw ETH to owner + // withdraw WETH to owner _withdrawFrom(owner, totalBalance); return totalBalance; } - /// @dev Returns the balance in ETH of the `owner` - /// @return Balance in ETH. + /// @dev Returns the balance in WETH of the `owner` + /// @return Balance in WETH. function balanceOf(address owner) external view @@ -87,21 +87,19 @@ contract EthVault is return _balances[owner]; } - /// @dev Withdraw an `amount` of ETH to `owner` from the vault. - /// @param owner of ETH. - /// @param amount of ETH to withdraw. + /// @dev Withdraw an `amount` of WETH to `owner` from the vault. + /// @param owner of WETH. + /// @param amount of WETH to withdraw. function _withdrawFrom(address payable owner, uint256 amount) internal { - // update balance - // note that this call will revert if trying to withdraw more - // than the current balance + //Uupdate balance. _balances[owner] = _balances[owner].safeSub(amount); + // withdraw WETH to owner + IEtherToken(_getWETHAddress()).transfer(msg.sender, amount); + // notify emit EthWithdrawnFromVault(msg.sender, owner, amount); - - // withdraw ETH to owner - owner.transfer(amount); } } diff --git a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol index 3e25a5dc91..4a6ac1a770 100644 --- a/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol +++ b/contracts/staking/contracts/src/vaults/StakingPoolRewardVault.sol @@ -19,20 +19,21 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; +import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol"; import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; import "../libs/LibStakingRichErrors.sol"; import "../libs/LibSafeDowncast.sol"; import "./MixinVaultCore.sol"; import "../interfaces/IStakingPoolRewardVault.sol"; -import "../immutable/MixinConstants.sol"; +import "../immutable/MixinDeploymentConstants.sol"; /// @dev This vault manages staking pool rewards. contract StakingPoolRewardVault is IStakingPoolRewardVault, IVaultCore, - MixinConstants, + MixinDeploymentConstants, Ownable, MixinVaultCore { @@ -41,29 +42,28 @@ contract StakingPoolRewardVault is // mapping from poolId to Pool metadata mapping (bytes32 => uint256) internal _balanceByPoolId; - // solhint-disable no-empty-blocks - /// @dev Payable fallback for bulk-deposits. - function () external payable {} - - /// @dev Record a deposit of an amount of ETH for `poolId` into the vault. - /// The staking contract should pay this contract the ETH owed in the - /// same transaction. + /// @dev Deposit an amount of WETH for `poolId` into the vault. + /// The staking contract should have granted the vault an allowance + /// because it will pull the WETH via `transferFrom()`. /// Note that this is only callable by the staking contract. - /// @param poolId Pool that holds the ETH. + /// @param poolId Pool that holds the WETH. /// @param amount Amount of deposit. - function recordDepositFor(bytes32 poolId, uint256 amount) + function depositFor(bytes32 poolId, uint256 amount) external onlyStakingProxy { + // Transfer WETH from the staking contract into this contract. + IEtherToken(_getWETHAddress()).transferFrom(msg.sender, address(this), amount); + // Credit the pool. _balanceByPoolId[poolId] = _balanceByPoolId[poolId].safeAdd(amount); emit EthDepositedIntoVault(msg.sender, poolId, amount); } - /// @dev Withdraw some amount in ETH from a pool. + /// @dev Withdraw some amount in WETH from a pool. /// Note that this is only callable by the staking contract. /// @param poolId Unique Id of pool. /// @param to Address to send funds to. - /// @param amount Amount of ETH to transfer. + /// @param amount Amount of WETH to transfer. function transfer( bytes32 poolId, address payable to, @@ -73,7 +73,7 @@ contract StakingPoolRewardVault is onlyStakingProxy { _balanceByPoolId[poolId] = _balanceByPoolId[poolId].safeSub(amount); - to.transfer(amount); + IEtherToken(_getWETHAddress()).transfer(to, amount); emit PoolRewardTransferred( poolId, to, @@ -81,8 +81,8 @@ contract StakingPoolRewardVault is ); } - /// @dev Returns the balance in ETH of `poolId` - /// @return Balance in ETH. + /// @dev Returns the balance in WETH of `poolId` + /// @return Balance in WETH. function balanceOf(bytes32 poolId) external view diff --git a/contracts/staking/contracts/test/TestDelegatorRewards.sol b/contracts/staking/contracts/test/TestDelegatorRewards.sol index 922ff0f5d7..6bf5dcaad0 100644 --- a/contracts/staking/contracts/test/TestDelegatorRewards.sol +++ b/contracts/staking/contracts/test/TestDelegatorRewards.sol @@ -94,8 +94,8 @@ contract TestDelegatorRewards is _initGenesisCumulativeRewards(poolId); } - /// @dev Expose/wrap `_recordStakingPoolRewards`. - function recordStakingPoolRewards( + /// @dev Expose/wrap `_depositStakingPoolRewards`. + function depositStakingPoolRewards( bytes32 poolId, address payable operatorAddress, uint256 operatorReward, @@ -109,7 +109,7 @@ contract TestDelegatorRewards is _setOperatorShare(poolId, operatorReward, membersReward); _initGenesisCumulativeRewards(poolId); - _recordStakingPoolRewards( + _depositStakingPoolRewards( poolId, operatorReward + membersReward, membersStake @@ -206,8 +206,8 @@ contract TestDelegatorRewards is ); } - /// @dev `IEthVault.recordDepositFor()`,` overridden to just emit events. - function recordDepositFor( + /// @dev `IEthVault.depositFor()`,` overridden to just emit events. + function depositFor( address owner, uint256 amount ) @@ -219,9 +219,9 @@ contract TestDelegatorRewards is ); } - /// @dev `IStakingPoolRewardVault.recordDepositFor()`,` + /// @dev `IStakingPoolRewardVault.depositFor()`,` /// overridden to just emit events. - function recordDepositFor( + function depositFor( bytes32 poolId, uint256 membersReward ) @@ -252,7 +252,7 @@ contract TestDelegatorRewards is uint256 totalRewards = reward.operatorReward + reward.membersReward; membersStake = reward.membersStake; (operatorReward, membersReward) = - _recordStakingPoolRewards(poolId, totalRewards, membersStake); + _depositStakingPoolRewards(poolId, totalRewards, membersStake); emit FinalizePool(poolId, operatorReward, membersReward, membersStake); } diff --git a/contracts/staking/contracts/test/TestFinalizer.sol b/contracts/staking/contracts/test/TestFinalizer.sol index 5d8d0eea26..7b0acac671 100644 --- a/contracts/staking/contracts/test/TestFinalizer.sol +++ b/contracts/staking/contracts/test/TestFinalizer.sol @@ -97,16 +97,6 @@ contract TestFinalizer is numActivePoolsThisEpoch += 1; } - /// @dev Expose `_finalizePool()` - function finalizePool(bytes32 poolId) - external - returns (FinalizedPoolRewards memory reward) - { - (reward.operatorReward, - reward.membersReward, - reward.membersStake) = _finalizePool(poolId); - } - /// @dev Drain the balance of this contract. function drainBalance() external @@ -114,35 +104,6 @@ contract TestFinalizer is address(0).transfer(address(this).balance); } - /// @dev Get finalization-related state variables. - function getFinalizationState() - external - view - returns ( - uint256 _balance, - uint256 _currentEpoch, - uint256 _closingEpoch, - uint256 _numActivePoolsThisEpoch, - uint256 _totalFeesCollectedThisEpoch, - uint256 _totalWeightedStakeThisEpoch, - uint256 _unfinalizedPoolsRemaining, - uint256 _unfinalizedRewardsAvailable, - uint256 _unfinalizedTotalFeesCollected, - uint256 _unfinalizedTotalWeightedStake - ) - { - _balance = address(this).balance; - _currentEpoch = currentEpoch; - _closingEpoch = currentEpoch - 1; - _numActivePoolsThisEpoch = numActivePoolsThisEpoch; - _totalFeesCollectedThisEpoch = totalFeesCollectedThisEpoch; - _totalWeightedStakeThisEpoch = totalWeightedStakeThisEpoch; - _unfinalizedPoolsRemaining = unfinalizedPoolsRemaining; - _unfinalizedRewardsAvailable = unfinalizedRewardsAvailable; - _unfinalizedTotalFeesCollected = unfinalizedTotalFeesCollected; - _unfinalizedTotalWeightedStake = unfinalizedTotalWeightedStake; - } - /// @dev Compute Cobb-Douglas. function cobbDouglas( uint256 totalRewards, @@ -155,7 +116,7 @@ contract TestFinalizer is view returns (uint256 ownerRewards) { - ownerRewards = LibCobbDouglas._cobbDouglas( + ownerRewards = LibCobbDouglas.cobbDouglas( totalRewards, ownerFees, totalFees, @@ -185,8 +146,8 @@ contract TestFinalizer is pool = _getActivePoolFromEpoch(epoch, poolId); } - /// @dev Overridden to log and do some basic math. - function _recordStakingPoolRewards( + /// @dev Overridden to log and transfer to receivers. + function _depositStakingPoolRewards( bytes32 poolId, uint256 reward, uint256 membersStake @@ -194,52 +155,16 @@ contract TestFinalizer is internal returns (uint256 operatorReward, uint256 membersReward) { + uint32 operatorShare = _operatorSharesByPool[poolId]; (operatorReward, membersReward) = - _splitReward(poolId, reward, membersStake); - emit RecordStakingPoolRewards( - poolId, - reward, - membersStake - ); - } - - /// @dev Overridden to log and transfer to receivers. - function _depositStakingPoolRewards( - uint256 operatorReward, - uint256 membersReward - ) - internal - { - emit DepositStakingPoolRewards(operatorReward, membersReward); + _computeSplitStakingPoolRewards(operatorShare, reward, membersStake); address(_operatorRewardsReceiver).transfer(operatorReward); address(_membersRewardsReceiver).transfer(membersReward); + emit DepositStakingPoolRewards(operatorReward, membersReward); } /// @dev Overriden to just increase the epoch counter. function _goToNextEpoch() internal { currentEpoch += 1; } - - // solhint-disable no-empty-blocks - /// @dev Overridden to do nothing. - function _unwrapWETH() internal {} - - /// @dev Split a pool's total reward between the operator and members. - function _splitReward( - bytes32 poolId, - uint256 amount, - uint256 membersStake - ) - private - view - returns (uint256 operatorReward, uint256 membersReward) - { - uint32 operatorShare = _operatorSharesByPool[poolId]; - (operatorReward, membersReward) = _splitStakingPoolRewards( - operatorShare, - amount, - membersStake - ); - } - } diff --git a/contracts/staking/contracts/test/TestStaking.sol b/contracts/staking/contracts/test/TestStaking.sol index 3241f43e8d..b38874e927 100644 --- a/contracts/staking/contracts/test/TestStaking.sol +++ b/contracts/staking/contracts/test/TestStaking.sol @@ -25,6 +25,12 @@ import "../src/Staking.sol"; contract TestStaking is Staking { + address internal _wethAddress; + + constructor(address wethAddress) public { + _wethAddress = wethAddress; + } + /// @dev Overridden to avoid hard-coded WETH. function getTotalBalance() external @@ -34,9 +40,8 @@ contract TestStaking is totalBalance = address(this).balance; } - // Stub out `_unwrapWETH` to prevent the calls to `finalizeFees` from failing in tests - // that do not relate to protocol fee payments in WETH. - function _unwrapWETH() - internal - {} // solhint-disable-line no-empty-blocks + /// @dev Overridden to use _wethAddress; + function _getWETHAddress() internal view returns (address) { + return _wethAddress; + } } diff --git a/contracts/staking/contracts/test/TestStorageLayout.sol b/contracts/staking/contracts/test/TestStorageLayout.sol index 1cb748fe18..fdb00b505a 100644 --- a/contracts/staking/contracts/test/TestStorageLayout.sol +++ b/contracts/staking/contracts/test/TestStorageLayout.sol @@ -135,19 +135,7 @@ contract TestStorageLayout is if sub(numActivePoolsThisEpoch_slot, slot) { revertIncorrectStorageSlot() } slot := add(slot, 1) - if sub(unfinalizedRewardsAvailable_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(unfinalizedPoolsRemaining_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(unfinalizedTotalFeesCollected_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(unfinalizedTotalWeightedStake_slot, slot) { revertIncorrectStorageSlot() } - slot := add(slot, 1) - - if sub(totalRewardsPaidLastEpoch_slot, slot) { revertIncorrectStorageSlot() } + if sub(unfinalizedState_slot, slot) { revertIncorrectStorageSlot() } slot := add(slot, 1) } } diff --git a/contracts/staking/test/actors/finalizer_actor.ts b/contracts/staking/test/actors/finalizer_actor.ts index 74298fd708..17c1e57d44 100644 --- a/contracts/staking/test/actors/finalizer_actor.ts +++ b/contracts/staking/test/actors/finalizer_actor.ts @@ -118,8 +118,10 @@ export class FinalizerActor extends BaseActor { private async _getDelegatorBalancesByPoolIdAsync( delegatorsByPoolId: DelegatorsByPoolId, ): Promise { - const computeRewardBalanceOfDelegator = this._stakingApiWrapper.stakingContract.computeRewardBalanceOfDelegator; - const computeRewardBalanceOfOperator = this._stakingApiWrapper.stakingContract.computeRewardBalanceOfOperator; + const { + computeRewardBalanceOfDelegator, + computeRewardBalanceOfOperator, + } = this._stakingApiWrapper.stakingContract; const delegatorBalancesByPoolId: DelegatorBalancesByPoolId = {}; for (const poolId of Object.keys(delegatorsByPoolId)) { @@ -142,7 +144,7 @@ export class FinalizerActor extends BaseActor { private async _getDelegatorStakesByPoolIdAsync( delegatorsByPoolId: DelegatorsByPoolId, ): Promise { - const getStakeDelegatedToPoolByOwner = this._stakingApiWrapper.stakingContract.getStakeDelegatedToPoolByOwner; + const { getStakeDelegatedToPoolByOwner } = this._stakingApiWrapper.stakingContract; const delegatorBalancesByPoolId: DelegatorBalancesByPoolId = {}; for (const poolId of Object.keys(delegatorsByPoolId)) { const delegators = delegatorsByPoolId[poolId]; @@ -247,7 +249,7 @@ export class FinalizerActor extends BaseActor { } const rewards = await Promise.all( activePools.map(async pool => - this._stakingApiWrapper.utils.cobbDouglas( + this._stakingApiWrapper.utils.cobbDouglasAsync( totalRewards, pool.feesCollected, totalFeesCollected, diff --git a/contracts/staking/test/cumulative_reward_tracking_test.ts b/contracts/staking/test/cumulative_reward_tracking_test.ts index 35e2258f97..2f84aeacb1 100644 --- a/contracts/staking/test/cumulative_reward_tracking_test.ts +++ b/contracts/staking/test/cumulative_reward_tracking_test.ts @@ -28,7 +28,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => { // set up ERC20Wrapper erc20Wrapper = new ERC20Wrapper(env.provider, accounts, owner); // deploy staking contracts - stakingApiWrapper = await deployAndConfigureContractsAsync(env, owner, erc20Wrapper, artifacts.TestStaking); + stakingApiWrapper = await deployAndConfigureContractsAsync(env, owner, erc20Wrapper); simulation = new CumulativeRewardTrackingSimulation(stakingApiWrapper, actors); await simulation.deployAndConfigureTestContractsAsync(env); }); diff --git a/contracts/staking/test/epoch_test.ts b/contracts/staking/test/epoch_test.ts index 3d7e9c5b0b..13ca7e6635 100644 --- a/contracts/staking/test/epoch_test.ts +++ b/contracts/staking/test/epoch_test.ts @@ -23,7 +23,7 @@ blockchainTests('Epochs', env => { // set up ERC20Wrapper erc20Wrapper = new ERC20Wrapper(env.provider, accounts, owner); // deploy staking contracts - stakingApiWrapper = await deployAndConfigureContractsAsync(env, owner, erc20Wrapper, artifacts.TestStaking); + stakingApiWrapper = await deployAndConfigureContractsAsync(env, owner, erc20Wrapper); }); describe('Epochs & TimeLocks', () => { it('basic epochs & timeLock periods', async () => { diff --git a/contracts/staking/test/rewards_test.ts b/contracts/staking/test/rewards_test.ts index bc93f968a3..92902aca29 100644 --- a/contracts/staking/test/rewards_test.ts +++ b/contracts/staking/test/rewards_test.ts @@ -1,5 +1,5 @@ import { ERC20Wrapper } from '@0x/contracts-asset-proxy'; -import { blockchainTests, describe, expect } from '@0x/contracts-test-utils'; +import { blockchainTests, describe, expect, shortZip } from '@0x/contracts-test-utils'; import { BigNumber } from '@0x/utils'; import * as _ from 'lodash'; @@ -42,7 +42,7 @@ blockchainTests.resets('Testing Rewards', env => { // set up ERC20Wrapper erc20Wrapper = new ERC20Wrapper(env.provider, accounts, owner); // deploy staking contracts - stakingApiWrapper = await deployAndConfigureContractsAsync(env, owner, erc20Wrapper, artifacts.TestStaking); + stakingApiWrapper = await deployAndConfigureContractsAsync(env, owner, erc20Wrapper); // set up staking parameters await stakingApiWrapper.utils.setParamsAsync({ minimumPoolStake: new BigNumber(1), @@ -600,16 +600,16 @@ blockchainTests.resets('Testing Rewards', env => { // skip epoch, so staker can start earning rewards await payProtocolFeeAndFinalize(); // undelegate some stake - const unstakeAmount = toBaseUnitAmount(2.5); + const undelegateAmount = toBaseUnitAmount(2.5); await staker.moveStakeAsync( new StakeInfo(StakeStatus.Delegated, poolId), new StakeInfo(StakeStatus.Active), - unstakeAmount, + undelegateAmount, ); // finalize const reward = toBaseUnitAmount(10); await payProtocolFeeAndFinalize(reward); - // Unstake nothing to move the rewards into the EthVault. + // Undelegate 0 stake to move the rewards into the EthVault. await staker.moveStakeAsync( new StakeInfo(StakeStatus.Delegated, poolId), new StakeInfo(StakeStatus.Active), @@ -624,7 +624,7 @@ blockchainTests.resets('Testing Rewards', env => { const stakeAmounts = [toBaseUnitAmount(5), toBaseUnitAmount(10)]; const totalStakeAmount = BigNumber.sum(...stakeAmounts); // stake and delegate both - const stakersAndStake = _.zip(stakers.slice(0, 2), stakeAmounts) as Array<[StakerActor, BigNumber]>; + const stakersAndStake = shortZip(stakers, stakeAmounts); for (const [staker, stakeAmount] of stakersAndStake) { await staker.stakeWithPoolAsync(poolId, stakeAmount); } @@ -655,7 +655,7 @@ blockchainTests.resets('Testing Rewards', env => { const stakeAmounts = [toBaseUnitAmount(5), toBaseUnitAmount(10)]; const totalStakeAmount = BigNumber.sum(...stakeAmounts); // stake and delegate both - const stakersAndStake = _.zip(stakers.slice(0, 2), stakeAmounts) as Array<[StakerActor, BigNumber]>; + const stakersAndStake = shortZip(stakers, stakeAmounts); for (const [staker, stakeAmount] of stakersAndStake) { await staker.stakeWithPoolAsync(poolId, stakeAmount); } diff --git a/contracts/staking/test/stake_test.ts b/contracts/staking/test/stake_test.ts index 171ab949a7..bdeba00631 100644 --- a/contracts/staking/test/stake_test.ts +++ b/contracts/staking/test/stake_test.ts @@ -36,7 +36,7 @@ blockchainTests.resets('Stake Statuses', env => { // set up ERC20Wrapper erc20Wrapper = new ERC20Wrapper(env.provider, accounts, owner); // deploy staking contracts - stakingApiWrapper = await deployAndConfigureContractsAsync(env, owner, erc20Wrapper, artifacts.TestStaking); + stakingApiWrapper = await deployAndConfigureContractsAsync(env, owner, erc20Wrapper); // setup new staker staker = new StakerActor(actors[0], stakingApiWrapper); diff --git a/contracts/staking/test/unit_tests/delegator_reward_test.ts b/contracts/staking/test/unit_tests/delegator_reward_test.ts index 712121fe54..a36a5ab97a 100644 --- a/contracts/staking/test/unit_tests/delegator_reward_test.ts +++ b/contracts/staking/test/unit_tests/delegator_reward_test.ts @@ -57,7 +57,7 @@ blockchainTests.resets('delegator unit rewards', env => { }; // Generate a deterministic operator address based on the poolId. _opts.operator = poolIdToOperator(_opts.poolId); - await testContract.recordStakingPoolRewards.awaitTransactionSuccessAsync( + await testContract.depositStakingPoolRewards.awaitTransactionSuccessAsync( _opts.poolId, _opts.operator, new BigNumber(_opts.operatorReward), diff --git a/contracts/staking/test/utils/api_wrapper.ts b/contracts/staking/test/utils/api_wrapper.ts index 0a45cc5616..8891fb7e38 100644 --- a/contracts/staking/test/utils/api_wrapper.ts +++ b/contracts/staking/test/utils/api_wrapper.ts @@ -1,9 +1,9 @@ import { ERC20Wrapper } from '@0x/contracts-asset-proxy'; -import { artifacts as erc20Artifacts, DummyERC20TokenContract } from '@0x/contracts-erc20'; +import { artifacts as erc20Artifacts, DummyERC20TokenContract, WETH9Contract } from '@0x/contracts-erc20'; import { BlockchainTestsEnvironment, constants, filterLogsToArguments, txDefaults } from '@0x/contracts-test-utils'; import { BigNumber, logUtils } from '@0x/utils'; import { Web3Wrapper } from '@0x/web3-wrapper'; -import { BlockParamLiteral, ContractArtifact, TransactionReceiptWithDecodedLogs } from 'ethereum-types'; +import { BlockParamLiteral, ContractArtifact, DecodedLogArgs, TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import * as _ from 'lodash'; import { @@ -17,6 +17,7 @@ import { StakingPoolRewardVaultContract, StakingProxyContract, TestCobbDouglasContract, + TestStakingContract, ZrxVaultContract, } from '../../src'; @@ -48,14 +49,20 @@ export class StakingApiWrapper { await this._web3Wrapper.mineBlockAsync(); }, - skipToNextEpochAndFinalizeAsync: async (): Promise => { + skipToNextEpochAndFinalizeAsync: async (): Promise => { await this.utils.fastForwardToNextEpochAsync(); const endOfEpochInfo = await this.utils.endEpochAsync(); - const receipt = await this.stakingContract.finalizePools.awaitTransactionSuccessAsync( - endOfEpochInfo.activePoolIds, - ); - logUtils.log(`Finalization cost ${receipt.gasUsed} gas`); - return receipt; + let totalGasUsed = 0; + const allLogs = [] as LogEntry[]; + for (const poolId of endOfEpochInfo.activePoolIds) { + const receipt = await this.stakingContract.finalizePool.awaitTransactionSuccessAsync( + poolId, + ); + totalGasUsed += receipt.gasUsed; + allLogs.splice(allLogs.length, 0, receipt.logs); + } + logUtils.log(`Finalization cost ${totalGasUsed} gas`); + return allLogs; }, endEpochAsync: async (): Promise => { @@ -144,7 +151,7 @@ export class StakingApiWrapper { ) as any) as StakingParams; }, - cobbDouglas: async ( + cobbDouglasAsync: async ( totalRewards: BigNumber, ownerFees: BigNumber, totalFees: BigNumber, @@ -219,13 +226,23 @@ export async function deployAndConfigureContractsAsync( const [zrxTokenContract] = await erc20Wrapper.deployDummyTokensAsync(1, constants.DUMMY_TOKEN_DECIMALS); await erc20Wrapper.setBalancesAndAllowancesAsync(); + // deploy WETH + const wethContract = await WETH9Contract.deployFrom0xArtifactAsync( + erc20Artifacts.WETH9, + env.provider, + txDefaults, + artifacts, + ); + // deploy staking contract - const stakingContract = await StakingContract.deployFrom0xArtifactAsync( - customStakingArtifact !== undefined ? customStakingArtifact : artifacts.Staking, + const stakingContract = await TestStakingContract.deployFrom0xArtifactAsync( + customStakingArtifact !== undefined ? customStakingArtifact : artifacts.TestStaking, env.provider, env.txDefaults, artifacts, + wethContract.address, ); + // deploy read-only proxy const readOnlyProxyContract = await ReadOnlyProxyContract.deployFrom0xArtifactAsync( artifacts.ReadOnlyProxy, diff --git a/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts b/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts index 56f7ef9d6e..50686db577 100644 --- a/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts +++ b/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts @@ -1,10 +1,9 @@ import { BlockchainTestsEnvironment, constants, expect, txDefaults } from '@0x/contracts-test-utils'; import { BigNumber } from '@0x/utils'; -import { DecodedLogArgs, TransactionReceiptWithDecodedLogs } from 'ethereum-types'; +import { DecodedLogEntry, TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import * as _ from 'lodash'; -import { TestCumulativeRewardTrackingContract } from '../../generated-wrappers/test_cumulative_reward_tracking'; -import { artifacts } from '../../src'; +import { artifacts, TestCumulativeRewardTrackingContract, IStakingEvents } from '../../src'; import { StakingApiWrapper } from './api_wrapper'; import { toBaseUnitAmount } from './number_utils'; @@ -118,20 +117,21 @@ export class CumulativeRewardTrackingSimulation { CumulativeRewardTrackingSimulation._assertTestLogs(expectedTestLogs, testLogs); } - private async _executeActionsAsync(actions: TestAction[]): Promise { - let logs: DecodedLogArgs[] = []; + private async _executeActionsAsync(actions: TestAction[]): Promise>> { + const combinedLogs = [] as Array>; for (const action of actions) { - let txReceipt: TransactionReceiptWithDecodedLogs; + let receipt: TransactionReceiptWithDecodedLogs; + let logs = [] as DecodedLogEntry; switch (action) { case TestAction.Finalize: - txReceipt = await this._stakingApiWrapper.utils.skipToNextEpochAndFinalizeAsync(); + logs = await this._stakingApiWrapper.utils.skipToNextEpochAndFinalizeAsync(); break; case TestAction.Delegate: await this._stakingApiWrapper.stakingContract.stake.sendTransactionAsync(this._amountToStake, { from: this._staker, }); - txReceipt = await this._stakingApiWrapper.stakingContract.moveStake.awaitTransactionSuccessAsync( + receipt = await this._stakingApiWrapper.stakingContract.moveStake.awaitTransactionSuccessAsync( new StakeInfo(StakeStatus.Active), new StakeInfo(StakeStatus.Delegated, this._poolId), this._amountToStake, @@ -140,7 +140,7 @@ export class CumulativeRewardTrackingSimulation { break; case TestAction.Undelegate: - txReceipt = await this._stakingApiWrapper.stakingContract.moveStake.awaitTransactionSuccessAsync( + receipt = await this._stakingApiWrapper.stakingContract.moveStake.awaitTransactionSuccessAsync( new StakeInfo(StakeStatus.Delegated, this._poolId), new StakeInfo(StakeStatus.Active), this._amountToStake, @@ -149,7 +149,7 @@ export class CumulativeRewardTrackingSimulation { break; case TestAction.PayProtocolFee: - txReceipt = await this._stakingApiWrapper.stakingContract.payProtocolFee.awaitTransactionSuccessAsync( + receipt = await this._stakingApiWrapper.stakingContract.payProtocolFee.awaitTransactionSuccessAsync( this._poolOperator, this._takerAddress, this._protocolFeeAmount, @@ -158,12 +158,12 @@ export class CumulativeRewardTrackingSimulation { break; case TestAction.CreatePool: - txReceipt = await this._stakingApiWrapper.stakingContract.createStakingPool.awaitTransactionSuccessAsync( + receipt = await this._stakingApiWrapper.stakingContract.createStakingPool.awaitTransactionSuccessAsync( 0, true, { from: this._poolOperator }, ); - const createStakingPoolLog = txReceipt.logs[0]; + const createStakingPoolLog = logs[0]; // tslint:disable-next-line no-unnecessary-type-assertion this._poolId = (createStakingPoolLog as DecodedLogArgs).args.poolId; break; @@ -171,8 +171,8 @@ export class CumulativeRewardTrackingSimulation { default: throw new Error('Unrecognized test action'); } - logs = logs.concat(txReceipt.logs); + combinedLogs.splice(combinedLogs.length - 1, 0, logs); } - return logs; + return combinedLogs; } } diff --git a/contracts/utils/contracts/src/LibFractions.sol b/contracts/utils/contracts/src/LibFractions.sol index 39dadf1aca..bc6f441f02 100644 --- a/contracts/utils/contracts/src/LibFractions.sol +++ b/contracts/utils/contracts/src/LibFractions.sol @@ -7,17 +7,14 @@ library LibFractions { using LibSafeMath for uint256; - /// @dev Maximum value for addition result components. - uint256 constant internal RESCALE_THRESHOLD = 10 ** 27; - /// @dev Safely adds two fractions `n1/d1 + n2/d2` /// @param n1 numerator of `1` /// @param d1 denominator of `1` /// @param n2 numerator of `2` /// @param d2 denominator of `2` - /// @return numerator of sum - /// @return denominator of sum - function addFractions( + /// @return numerator Numerator of sum + /// @return denominator Denominator of sum + function add( uint256 n1, uint256 d1, uint256 n2, @@ -40,15 +37,61 @@ library LibFractions { .safeMul(d2) .safeAdd(n2.safeMul(d1)); denominator = d1.safeMul(d2); + return (numerator, denominator); + } - // If either the numerator or the denominator are > RESCALE_THRESHOLD, - // re-scale them to prevent overflows in future operations. - if (numerator > RESCALE_THRESHOLD || denominator > RESCALE_THRESHOLD) { + /// @dev Rescales a fraction to prevent overflows during addition if either + /// the numerator or the denominator are > `maxValue`. + /// @param numerator The numerator. + /// @param denominator The denominator. + /// @param maxValue The maximum value allowed for both the numerator and + /// denominator. + /// @return scaledNumerator The rescaled numerator. + /// @return scaledDenominator The rescaled denominator. + function normalize( + uint256 numerator, + uint256 denominator, + uint256 maxValue + ) + internal + pure + returns ( + uint256 scaledNumerator, + uint256 scaledDenominator + ) + { + // If either the numerator or the denominator are > `maxValue`, + // re-scale them by `maxValue` to prevent overflows in future operations. + if (numerator > maxValue || denominator > maxValue) { uint256 rescaleBase = numerator >= denominator ? numerator : denominator; - rescaleBase /= RESCALE_THRESHOLD; - numerator = numerator.safeDiv(rescaleBase); - denominator = denominator.safeDiv(rescaleBase); + rescaleBase /= maxValue; + scaledNumerator = numerator.safeDiv(rescaleBase); + scaledDenominator = denominator.safeDiv(rescaleBase); + } else { + scaledNumerator = numerator; + scaledDenominator = denominator; } + return (scaledNumerator, scaledDenominator); + } + + /// @dev Rescales a fraction to prevent overflows during addition if either + /// the numerator or the denominator are > 2 ** 127. + /// @param numerator The numerator. + /// @param denominator The denominator. + /// @return scaledNumerator The rescaled numerator. + /// @return scaledDenominator The rescaled denominator. + function normalize( + uint256 numerator, + uint256 denominator + ) + internal + pure + returns ( + uint256 scaledNumerator, + uint256 scaledDenominator + ) + { + return normalize(numerator, denominator, 2 ** 127); } /// @dev Safely scales the difference between two fractions. @@ -57,8 +100,8 @@ library LibFractions { /// @param n2 numerator of `2` /// @param d2 denominator of `2` /// @param s scalar to multiply by difference. - /// @return result = `s * (n1/d1 - n2/d2)`. - function scaleFractionalDifference( + /// @return result `s * (n1/d1 - n2/d2)`. + function scaleDifference( uint256 n1, uint256 d1, uint256 n2, @@ -81,7 +124,7 @@ library LibFractions { .safeMul(d2) .safeSub(n2.safeMul(d1)); uint256 tmp = numerator.safeDiv(d2); - result = s + return s .safeMul(tmp) .safeDiv(d1); }