From e1d51bae73a6545e0ca0a15516d49cdd785642f3 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 16 Sep 2019 16:02:50 -0700 Subject: [PATCH] Tests are passing and ran linter --- .../staking_pools/MixinCumulativeRewards.sol | 37 +++++++++++-------- .../staking_pools/MixinStakingPoolRewards.sol | 2 +- .../test/TestCumulativeRewardTracking.sol | 24 +++++++++--- .../test/cumulative_reward_tracking_test.ts | 6 +-- .../cumulative_reward_tracking_simulation.ts | 13 ++++--- 5 files changed, 48 insertions(+), 34 deletions(-) diff --git a/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol index be519b9c0b..c5a8494f1c 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol @@ -162,29 +162,37 @@ contract MixinCumulativeRewards is function _trySetMostRecentCumulativeRewardEpoch(bytes32 poolId, uint256 epoch) internal { - // load the current value, sanity check that we're not trying to go back in time - uint256 currentMostRecentEpoch = cumulativeRewardsByPoolLastStored[poolId]; - assert(epoch >= currentMostRecentEpoch); - // check if we should do any work + uint256 currentMostRecentEpoch = cumulativeRewardsByPoolLastStored[poolId]; if (epoch == currentMostRecentEpoch) { return; } // update state to reflect the most recent cumulative reward - _forceSetMostRecentCumulativeRewardEpoch(poolId, epoch); - - // unset the previous most recent reward, if it is no longer needed - _tryUnsetCumulativeReward(poolId, currentMostRecentEpoch); + _forceSetMostRecentCumulativeRewardEpoch( + poolId, + currentMostRecentEpoch, + epoch + ); } /// @dev Forcefully sets the epoch of the most recent cumulative reward. /// @param poolId Unique Id of pool. - /// @param epoch of the most recent cumulative reward. - function _forceSetMostRecentCumulativeRewardEpoch(bytes32 poolId, uint256 epoch) + /// @param currentMostRecentEpoch of the most recent cumulative reward. + /// @param newMostRecentEpoch of the new most recent cumulative reward. + function _forceSetMostRecentCumulativeRewardEpoch( + bytes32 poolId, + uint256 currentMostRecentEpoch, + uint256 newMostRecentEpoch + ) internal { - cumulativeRewardsByPoolLastStored[poolId] = epoch; + // sanity check that we're not trying to go back in time + assert(newMostRecentEpoch >= currentMostRecentEpoch); + cumulativeRewardsByPoolLastStored[poolId] = newMostRecentEpoch; + + // unset the previous most recent reward, if it is no longer needed + _tryUnsetCumulativeReward(poolId, currentMostRecentEpoch); } /// @dev Adds a dependency on a cumulative reward for a given epoch. @@ -209,8 +217,7 @@ contract MixinCumulativeRewards is } else { _removeDependencyOnCumulativeReward( poolId, - epoch, - mostRecentCumulativeRewardInfo.cumulativeRewardEpoch + epoch ); } } @@ -240,11 +247,9 @@ contract MixinCumulativeRewards is /// @dev Removes a dependency on a cumulative reward for a given epoch. /// @param poolId Unique Id of pool. /// @param epoch to remove dependency from. - /// @param mostRecentCumulativeRewardEpoch Epoch of the most recent cumulative reward. function _removeDependencyOnCumulativeReward( bytes32 poolId, - uint256 epoch, - uint256 mostRecentCumulativeRewardEpoch + uint256 epoch ) internal { diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index 158578f027..6fae9bf65c 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -135,7 +135,7 @@ contract MixinStakingPoolRewards is ); // store cumulative rewards and set most recent - _trySetCumulativeReward( + _forceSetCumulativeReward( poolId, epoch, IStructs.Fraction({ diff --git a/contracts/staking/contracts/test/TestCumulativeRewardTracking.sol b/contracts/staking/contracts/test/TestCumulativeRewardTracking.sol index 2747a0dcba..90583db8cb 100644 --- a/contracts/staking/contracts/test/TestCumulativeRewardTracking.sol +++ b/contracts/staking/contracts/test/TestCumulativeRewardTracking.sol @@ -48,7 +48,11 @@ contract TestCumulativeRewardTracking is internal { emit SetCumulativeReward(poolId, epoch); - MixinCumulativeRewards._forceSetCumulativeReward(poolId, epoch, value); + MixinCumulativeRewards._forceSetCumulativeReward( + poolId, + epoch, + value + ); } function _forceUnsetCumulativeReward(bytes32 poolId, uint256 epoch) @@ -58,18 +62,26 @@ contract TestCumulativeRewardTracking is MixinCumulativeRewards._forceUnsetCumulativeReward(poolId, epoch); } - function _forceSetMostRecentCumulativeRewardEpoch(bytes32 poolId, uint256 epoch) + function _forceSetMostRecentCumulativeRewardEpoch( + bytes32 poolId, + uint256 currentMostRecentEpoch, + uint256 newMostRecentEpoch + ) internal { - emit SetMostRecentCumulativeReward(poolId, epoch); - MixinCumulativeRewards._forceSetMostRecentCumulativeRewardEpoch(poolId, epoch); + emit SetMostRecentCumulativeReward(poolId, newMostRecentEpoch); + MixinCumulativeRewards._forceSetMostRecentCumulativeRewardEpoch( + poolId, + currentMostRecentEpoch, + newMostRecentEpoch + ); } function _assertMixinParamsBeforeInit() internal - {} + {} // solhint-disable-line no-empty-blocks function _assertMixinSchedulerBeforeInit() internal - {} + {} // solhint-disable-line no-empty-blocks } diff --git a/contracts/staking/test/cumulative_reward_tracking_test.ts b/contracts/staking/test/cumulative_reward_tracking_test.ts index 83429ceed4..59bc6d6ed0 100644 --- a/contracts/staking/test/cumulative_reward_tracking_test.ts +++ b/contracts/staking/test/cumulative_reward_tracking_test.ts @@ -35,11 +35,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => { describe('Tracking Cumulative Rewards (CR)', () => { it('should set CR hen a pool is created is epoch 0', async () => { - await simulation.runTestAsync( - [], - [TestAction.CreatePool], - [{ event: 'SetCumulativeReward', epoch: 0 }], - ); + await simulation.runTestAsync([], [TestAction.CreatePool], [{ event: 'SetCumulativeReward', epoch: 0 }]); }); it('should set CR and Most Recent CR when a pool is created in epoch >0', async () => { await simulation.runTestAsync( diff --git a/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts b/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts index 70d30aa6c3..1df1d47c10 100644 --- a/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts +++ b/contracts/staking/test/utils/cumulative_reward_tracking_simulation.ts @@ -37,20 +37,20 @@ export class CumulativeRewardTrackingSimulation { private static _extractTestLogs(txReceiptLogs: DecodedLogArgs[]): TestLog[] { const logs = []; for (const log of txReceiptLogs) { - if ((log as DecodedLogArgs).event === 'SetMostRecentCumulativeReward') { + if (log.event === 'SetMostRecentCumulativeReward') { logs.push({ event: 'SetMostRecentCumulativeReward', - epoch: (log as DecodedLogArgs).args.epoch.toNumber(), + epoch: log.args.epoch.toNumber(), }); - } else if ((log as DecodedLogArgs).event === 'SetCumulativeReward') { + } else if (log.event === 'SetCumulativeReward') { logs.push({ event: 'SetCumulativeReward', - epoch: (log as DecodedLogArgs).args.epoch.toNumber(), + epoch: log.args.epoch.toNumber(), }); - } else if ((log as DecodedLogArgs).event === 'UnsetCumulativeReward') { + } else if (log.event === 'UnsetCumulativeReward') { logs.push({ event: 'UnsetCumulativeReward', - epoch: (log as DecodedLogArgs).args.epoch.toNumber(), + epoch: log.args.epoch.toNumber(), }); } } @@ -159,6 +159,7 @@ export class CumulativeRewardTrackingSimulation { { from: this._poolOperator }, ); const createStakingPoolLog = txReceipt.logs[0]; + // tslint:disable-next-line no-unnecessary-type-assertion this._poolId = (createStakingPoolLog as DecodedLogArgs).args.poolId; break;