From ce8fd44234c02d5f6de1ad2a65616c14e52a6ea8 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 22 Oct 2019 06:28:15 -0700 Subject: [PATCH] Removed lingering references to stale terminology ("active pools") --- .../contracts/src/fees/MixinExchangeFees.sol | 6 +++--- .../contracts/src/interfaces/IStakingEvents.sol | 10 +++++----- .../contracts/src/interfaces/IStructs.sol | 2 +- .../contracts/src/libs/LibCobbDouglas.sol | 6 ++---- .../contracts/src/stake/MixinStakeBalances.sol | 2 +- .../staking/contracts/src/sys/MixinFinalizer.sol | 15 +++++++-------- .../contracts/test/TestDelegatorRewards.sol | 2 +- .../staking/test/unit_tests/finalizer_test.ts | 16 ++++++++-------- .../test/unit_tests/protocol_fees_test.ts | 4 ++-- 9 files changed, 30 insertions(+), 33 deletions(-) diff --git a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol index 3b90aa6362..e51a221090 100644 --- a/contracts/staking/contracts/src/fees/MixinExchangeFees.sol +++ b/contracts/staking/contracts/src/fees/MixinExchangeFees.sol @@ -86,7 +86,7 @@ contract MixinExchangeFees is IStructs.PoolStats memory poolStats = poolStatsByEpoch[poolId][currentEpoch_]; IStructs.AggregatedStats memory aggregatedStats = aggregatedStatsByEpoch[currentEpoch_]; - // If the pool was previously inactive in this epoch, initialize it. + // Perform some initialization if this is the first protocol fee collected in this epoch. if (poolStats.feesCollected == 0) { // Compute member and total weighted stake. (poolStats.membersStake, poolStats.weightedStake) = _computeMembersAndWeightedStake(poolId, poolStake); @@ -94,7 +94,7 @@ contract MixinExchangeFees is // Increase the total weighted stake. aggregatedStats.totalWeightedStake = aggregatedStats.totalWeightedStake.safeAdd(poolStats.weightedStake); - // Increase the number of active pools. + // Increase the number of pools to finalize. aggregatedStats.poolsToFinalize = aggregatedStats.poolsToFinalize.safeAdd(1); // Emit an event so keepers know what pools earned rewards this epoch. @@ -112,7 +112,7 @@ contract MixinExchangeFees is aggregatedStatsByEpoch[currentEpoch_] = aggregatedStats; } - /// @dev Get information on an active staking pool in this epoch. + /// @dev Get stats on a staking pool in this epoch. /// @param poolId Pool Id to query. /// @return PoolStats struct for pool id. function getStakingPoolStatsThisEpoch(bytes32 poolId) diff --git a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol index b0f0e7e6eb..483e6839e5 100644 --- a/contracts/staking/contracts/src/interfaces/IStakingEvents.sol +++ b/contracts/staking/contracts/src/interfaces/IStakingEvents.sol @@ -43,7 +43,7 @@ interface IStakingEvents { address exchangeAddress ); - /// @dev Emitted by MixinExchangeFees when a pool will earn rewards. + /// @dev Emitted by MixinExchangeFees when a pool starts earning rewards in an epoch. /// @param epoch The epoch in which the pool earned rewards. /// @param poolId The ID of the pool. event StakingPoolEarnedRewardsInEpoch( @@ -53,10 +53,10 @@ interface IStakingEvents { /// @dev Emitted by MixinFinalizer when an epoch has ended. /// @param epoch The closing epoch. - /// @param poolsToFinalize Number of pools to finalize in the closing epoch. - /// @param rewardsAvailable Rewards available to all active pools. - /// @param totalWeightedStake Total weighted stake across all active pools. - /// @param totalFeesCollected Total fees collected across all active pools. + /// @param poolsToFinalize Number of pools that earned rewards during `epoch` and must be finalized. + /// @param rewardsAvailable Rewards available to all pools that earned rewards during `epoch`. + /// @param totalWeightedStake Total weighted stake across all pools that earned rewards during `epoch`. + /// @param totalFeesCollected Total fees collected across all pools that earned rewards during `epoch`. event EpochEnded( uint256 indexed epoch, uint256 poolsToFinalize, diff --git a/contracts/staking/contracts/src/interfaces/IStructs.sol b/contracts/staking/contracts/src/interfaces/IStructs.sol index ad4b243d85..ac87e92565 100644 --- a/contracts/staking/contracts/src/interfaces/IStructs.sol +++ b/contracts/staking/contracts/src/interfaces/IStructs.sol @@ -29,7 +29,7 @@ interface IStructs { uint96 lastSetTimestamp; } - /// @dev Stats for a pool that actively traded. + /// @dev Stats for a pool that earned rewards. /// @param feesCollected Fees collected in ETH by this pool. /// @param weightedStake Amount of weighted stake in the pool. /// @param membersStake Amount of non-operator stake in the pool. diff --git a/contracts/staking/contracts/src/libs/LibCobbDouglas.sol b/contracts/staking/contracts/src/libs/LibCobbDouglas.sol index 347758bde2..b956cab0b0 100644 --- a/contracts/staking/contracts/src/libs/LibCobbDouglas.sol +++ b/contracts/staking/contracts/src/libs/LibCobbDouglas.sol @@ -33,11 +33,9 @@ library LibCobbDouglas { /// 0 <= alphaNumerator / alphaDenominator <= 1 /// @param totalRewards collected over an epoch. /// @param fees Fees attributed to the the staking pool. - /// @param totalFees Total fees collected across all active staking pools in - /// the epoch. + /// @param totalFees Total fees collected across all pools that earned rewards. /// @param stake Stake attributed to the staking pool. - /// @param totalStake Total stake across all active staking pools in the - /// epoch. + /// @param totalStake Total stake across all pools that earned rewards. /// @param alphaNumerator Numerator of `alpha` in the cobb-douglas function. /// @param alphaDenominator Denominator of `alpha` in the cobb-douglas /// function. diff --git a/contracts/staking/contracts/src/stake/MixinStakeBalances.sol b/contracts/staking/contracts/src/stake/MixinStakeBalances.sol index bc089d9651..70a6fe515f 100644 --- a/contracts/staking/contracts/src/stake/MixinStakeBalances.sol +++ b/contracts/staking/contracts/src/stake/MixinStakeBalances.sol @@ -72,7 +72,7 @@ contract MixinStakeBalances is /// @dev Returns the total stake for a given staker. /// @param staker of stake. - /// @return Total active stake for staker. + /// @return Total ZRX staked by `staker`. function getTotalStake(address staker) public view diff --git a/contracts/staking/contracts/src/sys/MixinFinalizer.sol b/contracts/staking/contracts/src/sys/MixinFinalizer.sol index ea07ded773..57ddb3c243 100644 --- a/contracts/staking/contracts/src/sys/MixinFinalizer.sol +++ b/contracts/staking/contracts/src/sys/MixinFinalizer.sol @@ -35,9 +35,9 @@ contract MixinFinalizer is /// @dev Begins a new epoch, preparing the prior one for finalization. /// 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 + /// If no pools earned rewards in the closing epoch, the epoch /// will be instantly finalized here. Otherwise, `finalizePool()` - /// should be called on each active pool afterwards. + /// should be called on these pools afterward calling this function. /// @return poolsToFinalize The number of unfinalized pools. function endEpoch() external @@ -80,7 +80,7 @@ contract MixinFinalizer is // Advance the epoch. This will revert if not enough time has passed. _goToNextEpoch(); - // If there were no active pools, the epoch is already finalized. + // If there are no pools to finalize then the epoch is finalized. if (aggregatedStats.poolsToFinalize == 0) { emit EpochFinalized(closingEpoch, 0, aggregatedStats.rewardsAvailable); } @@ -88,11 +88,11 @@ contract MixinFinalizer is return aggregatedStats.poolsToFinalize; } - /// @dev Instantly finalizes a single pool that was active in the previous + /// @dev Instantly finalizes a single pool that earned rewards in the previous /// epoch, crediting it rewards for members and withdrawing operator's /// rewards as WETH. This can be called by internal functions that need /// to finalize a pool immediately. Does nothing if the pool is already - /// finalized or was not active in the previous epoch. + /// finalized or did not earn rewards in the previous epoch. /// @param poolId The pool ID to finalize. function finalizePool(bytes32 poolId) external @@ -107,7 +107,7 @@ contract MixinFinalizer is return; } - // Noop if the pool was not active or already finalized (has no fees). + // Noop if the pool did not earn rewards or already finalized (has no fees). IStructs.PoolStats memory poolStats = poolStatsByEpoch[poolId][prevEpoch]; if (poolStats.feesCollected == 0) { return; @@ -235,8 +235,7 @@ contract MixinFinalizer is view returns (uint256 rewards) { - // There can't be any rewards if the pool was active or if it has - // no stake. + // There can't be any rewards if the pool did not collect any fees. if (poolStats.feesCollected == 0) { return rewards; } diff --git a/contracts/staking/contracts/test/TestDelegatorRewards.sol b/contracts/staking/contracts/test/TestDelegatorRewards.sol index 59ebfc4121..814b054808 100644 --- a/contracts/staking/contracts/test/TestDelegatorRewards.sol +++ b/contracts/staking/contracts/test/TestDelegatorRewards.sol @@ -94,7 +94,7 @@ contract TestDelegatorRewards is currentEpoch += 1; } - /// @dev Create and delegate stake that is active in the current epoch. + /// @dev Create and delegate stake to the current epoch. /// Only used to test purportedly unreachable states. /// Also withdraws pending rewards. function delegateStakeNow( diff --git a/contracts/staking/test/unit_tests/finalizer_test.ts b/contracts/staking/test/unit_tests/finalizer_test.ts index a8b5b39a47..e0b122b4e8 100644 --- a/contracts/staking/test/unit_tests/finalizer_test.ts +++ b/contracts/staking/test/unit_tests/finalizer_test.ts @@ -281,7 +281,7 @@ blockchainTests.resets('Finalizer unit tests', env => { }); }); - it('immediately finalizes if there are no active pools', async () => { + it('immediately finalizes if there are no pools to finalize', async () => { const receipt = await testContract.endEpoch.awaitTransactionSuccessAsync(); assertEpochFinalizedEvent(receipt.logs, { epoch: stakingConstants.INITIAL_EPOCH, @@ -290,7 +290,7 @@ blockchainTests.resets('Finalizer unit tests', env => { }); }); - it('does not immediately finalize if there is an active pool', async () => { + it('does not immediately finalize if there is a pool to finalize', async () => { await addActivePoolAsync(); const receipt = await testContract.endEpoch.awaitTransactionSuccessAsync(); const events = filterLogsToArguments( @@ -355,7 +355,7 @@ blockchainTests.resets('Finalizer unit tests', env => { }); describe('_finalizePool()', () => { - it('does nothing if there were no active pools', async () => { + it('does nothing if there were no pools to finalize', async () => { await testContract.endEpoch.awaitTransactionSuccessAsync(); const poolId = hexRandom(); const logs = await finalizePoolsAsync([poolId]); @@ -445,7 +445,7 @@ blockchainTests.resets('Finalizer unit tests', env => { return expect(getCurrentEpochAsync()).to.become(stakingConstants.INITIAL_EPOCH.plus(2)); }); - it('does not reward a pool that was only active 2 epochs ago', async () => { + it('does not reward a pool that only earned rewards 2 epochs ago', async () => { const pool1 = await addActivePoolAsync(); await testContract.endEpoch.awaitTransactionSuccessAsync(); await finalizePoolsAsync([pool1.poolId]); @@ -457,7 +457,7 @@ blockchainTests.resets('Finalizer unit tests', env => { expect(rewardsPaidEvents).to.deep.eq([]); }); - it('does not reward a pool that was only active 3 epochs ago', async () => { + it('does not reward a pool that only earned rewards 3 epochs ago', async () => { const pool1 = await addActivePoolAsync(); await testContract.endEpoch.awaitTransactionSuccessAsync(); await finalizePoolsAsync([pool1.poolId]); @@ -512,18 +512,18 @@ blockchainTests.resets('Finalizer unit tests', env => { return assertUnfinalizedPoolRewardsAsync(poolId, ZERO_REWARDS); }); - it('returns empty if pool was not active', async () => { + it('returns empty if pool did not earn rewards', async () => { await testContract.endEpoch.awaitTransactionSuccessAsync(); const poolId = hexRandom(); return assertUnfinalizedPoolRewardsAsync(poolId, ZERO_REWARDS); }); - it('returns empty if pool is active only in the current epoch', async () => { + it('returns empty if pool is earned rewards only in the current epoch', async () => { const pool = await addActivePoolAsync(); return assertUnfinalizedPoolRewardsAsync(pool.poolId, ZERO_REWARDS); }); - it('returns empty if pool was only active in the 2 epochs ago', async () => { + it('returns empty if pool only earned rewards in the 2 epochs ago', async () => { const pool = await addActivePoolAsync(); await testContract.endEpoch.awaitTransactionSuccessAsync(); await finalizePoolsAsync([pool.poolId]); diff --git a/contracts/staking/test/unit_tests/protocol_fees_test.ts b/contracts/staking/test/unit_tests/protocol_fees_test.ts index cabe971f2a..ef07e01aa5 100644 --- a/contracts/staking/test/unit_tests/protocol_fees_test.ts +++ b/contracts/staking/test/unit_tests/protocol_fees_test.ts @@ -413,7 +413,7 @@ blockchainTests('Protocol Fees unit tests', env => { .plus(operatorStake); } - it('no active pools to start', async () => { + it('no pools to finalize to start', async () => { const state = await getFinalizationStateAsync(); expect(state.poolsToFinalize).to.bignumber.eq(0); expect(state.totalFeesCollected).to.bignumber.eq(0); @@ -428,7 +428,7 @@ blockchainTests('Protocol Fees unit tests', env => { expect(pool.weightedStake).to.bignumber.eq(0); }); - it('activates a active pool the first time it earns a fee', async () => { + it('correctly emits event for pool the first time it earns a fee', async () => { const pool = await createTestPoolAsync(); const { poolId,