diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index 4ab57b07f5..8d8b27b855 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -276,7 +276,17 @@ contract MixinStakingPoolRewards is // 2. `stake.currentEpoch < currentEpoch`. // Get the last epoch where a reward was credited to this pool. - uint256 lastRewardEpoch = cumulativeRewardsByPoolLastStored[poolId]; + uint256 lastRewardEpoch = lastPoolRewardEpoch[poolId]; + // Get the last reward epoch for which we collected rewards from. + uint256 lastCollectedRewardEpoch = + lastCollectedRewardsEpochToPoolByOwner[member][poolId]; + + // If either of these are true, the most recent reward has already been + // claimed. + if (lastCollectedRewardEpoch == lastRewardEpoch + || stake.currentEpoch >= lastRewardEpoch) { + return reward = 0; + } // If there are unfinalized rewards this epoch, compute the member's // share. @@ -291,43 +301,18 @@ contract MixinStakingPoolRewards is .safeMul(unfinalizedMembersReward) .safeDiv(unfinalizedDelegatedStake); } - // Add rewards up to the last reward epoch. - if (lastRewardEpoch != 0 && lastRewardEpoch > stake.currentEpoch) { - reward = reward.safeAdd( - _computeMemberRewardOverInterval( - poolId, - stake, - stake.currentEpoch, - stake.currentEpoch + 1 - ) - ); - if (lastRewardEpoch > stake.currentEpoch + 1) { - reward = reward.safeAdd( - _computeMemberRewardOverInterval( - poolId, - stake, - stake.currentEpoch + 1, - lastRewardEpoch - ) - ); - } - } - // Otherwise get the rewards earned up to the last reward epoch. - } else if (stake.currentEpoch < lastRewardEpoch) { - reward = _computeMemberRewardOverInterval( - poolId, - stake, - stake.currentEpoch, - stake.currentEpoch + 1 - ); - if (lastRewardEpoch > stake.currentEpoch + 1) { - reward = _computeMemberRewardOverInterval( + } + + // Add rewards up to the last reward epoch. + if (lastRewardEpoch != 0) { + reward = reward.safeAdd( + _computeMemberRewardOverInterval( poolId, stake, stake.currentEpoch + 1, lastRewardEpoch - ).safeSub(reward); - } + ) + ); } } diff --git a/contracts/staking/test/unit_tests/delegator_reward_balance_test.ts b/contracts/staking/test/unit_tests/delegator_reward_balance_test.ts index 64df0da462..6c91cd1949 100644 --- a/contracts/staking/test/unit_tests/delegator_reward_balance_test.ts +++ b/contracts/staking/test/unit_tests/delegator_reward_balance_test.ts @@ -18,7 +18,7 @@ import { import { assertRoughlyEquals, getRandomInteger, toBaseUnitAmount } from '../utils/number_utils'; -blockchainTests.resets('delegator unit rewards', env => { +blockchainTests.resets.only('delegator unit rewards', env => { let testContract: TestDelegatorRewardsContract; before(async () => { @@ -304,7 +304,7 @@ blockchainTests.resets('delegator unit rewards', env => { expect(delegatorReward).to.bignumber.eq(0); }); - it.only('has correct reward immediately after unstaking, restaking, and rewarding fees', async () => { + it('has correct reward immediately after unstaking, restaking, and rewarding fees', async () => { const poolId = hexRandom(); const { delegator, stake } = await delegateStakeAsync(poolId); await advanceEpochAsync(); // epoch 1 (stake now active) @@ -323,7 +323,7 @@ blockchainTests.resets('delegator unit rewards', env => { expect(delegatorReward).to.bignumber.eq(reward); }); - it('computes correct rewards for 2 staggered delegators', async () => { + it.only('computes correct rewards for 2 staggered delegators', async () => { const poolId = hexRandom(); const { delegator: delegatorA, stake: stakeA } = await delegateStakeAsync(poolId); await advanceEpochAsync(); // epoch 1 (stake A now active) @@ -347,7 +347,7 @@ blockchainTests.resets('delegator unit rewards', env => { assertRoughlyEquals(delegatorRewardA, expectedDelegatorRewardA); const delegatorRewardB = await getDelegatorRewardAsync(poolId, delegatorB); const expectedDelegatorRewardB = BigNumber.sum( - computeDelegatorRewards(reward2, stakeB, totalStake), + computeDelegatorRewards(BigNumber.sum(reward1, reward2), stakeB, totalStake), ); assertRoughlyEquals(delegatorRewardB, expectedDelegatorRewardB); });