diff --git a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol index 0625018666..4ab57b07f5 100644 --- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol +++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol @@ -255,36 +255,80 @@ contract MixinStakingPoolRewards is view returns (uint256 totalReward) { - // reward balance is always zero in these two scenarios: - // 1. The owner's delegated stake is current as of this epoch: their rewards have been moved to the ETH vault. - // 2. The current epoch is zero: delegation begins at epoch 1 - if (unsyncedDelegatedStakeToPoolByOwner.currentEpoch == currentEpoch || currentEpoch == 0) return 0; + uint256 currentEpoch = getCurrentEpoch(); + // There can be no rewards in epoch 0 because there is no delegated + // stake. + if (currentEpoch == 0) { + return reward = 0; + } - // compute reward accumulated during `delegatedStake.currentEpoch`; - uint256 rewardsAccumulatedDuringLastStoredEpoch = (unsyncedDelegatedStakeToPoolByOwner.currentEpochBalance != 0) - ? _computeMemberRewardOverInterval( + IStructs.StoredBalance memory stake = + _loadUnsyncedBalance(delegatedStakeToPoolByOwner[member][poolId]); + // 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 (stake.currentEpoch == currentEpoch) { + return reward = 0; + } + + // From here we know: + // 1. `currentEpoch > 0` + // 2. `stake.currentEpoch < currentEpoch`. + + // Get the last epoch where a reward was credited to this pool. + uint256 lastRewardEpoch = cumulativeRewardsByPoolLastStored[poolId]; + + // If there are unfinalized rewards this epoch, compute the member's + // share. + if (unfinalizedMembersReward != 0 && unfinalizedDelegatedStake != 0) { + // Unfinalized rewards are always earned from stake in + // the prior epoch so we want the stake at `currentEpoch-1`. + uint256 _stake = stake.currentEpoch == currentEpoch - 1 ? + stake.currentEpochBalance : + stake.nextEpochBalance; + if (_stake != 0) { + reward = _stake + .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, - unsyncedDelegatedStakeToPoolByOwner.currentEpochBalance, - uint256(unsyncedDelegatedStakeToPoolByOwner.currentEpoch).safeSub(1), - unsyncedDelegatedStakeToPoolByOwner.currentEpoch - ) - : 0; - - // compute the reward accumulated by the `next` balance; - // this starts at `delegatedStake.currentEpoch + 1` and goes up until the last epoch, during which - // rewards were accumulated. This is at most the most recently finalized epoch (current epoch - 1). - uint256 rewardsAccumulatedAfterLastStoredEpoch = (_cumulativeRewardsByPoolLastStored[poolId] > unsyncedDelegatedStakeToPoolByOwner.currentEpoch) - ? _computeMemberRewardOverInterval( - poolId, - unsyncedDelegatedStakeToPoolByOwner.nextEpochBalance, - unsyncedDelegatedStakeToPoolByOwner.currentEpoch, - _cumulativeRewardsByPoolLastStored[poolId] - ) - : 0; - - // compute the total reward - totalReward = rewardsAccumulatedDuringLastStoredEpoch.safeAdd(rewardsAccumulatedAfterLastStoredEpoch); - return totalReward; + stake, + stake.currentEpoch, + stake.currentEpoch + 1 + ); + if (lastRewardEpoch > stake.currentEpoch + 1) { + reward = _computeMemberRewardOverInterval( + poolId, + stake, + stake.currentEpoch + 1, + lastRewardEpoch + ).safeSub(reward); + } + } } /// @dev Adds or removes cumulative reward dependencies for a delegator. 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 43956f8dde..64df0da462 100644 --- a/contracts/staking/test/unit_tests/delegator_reward_balance_test.ts +++ b/contracts/staking/test/unit_tests/delegator_reward_balance_test.ts @@ -273,7 +273,22 @@ blockchainTests.resets('delegator unit rewards', env => { assertRoughlyEquals(delegatorReward, expectedDelegatorRewards); }); - it.only('has correct reward immediately after unstaking', async () => { + it('has correct reward immediately after unstaking', async () => { + const poolId = hexRandom(); + const { delegator, stake } = await delegateStakeAsync(poolId); + await advanceEpochAsync(); // epoch 1 (stake now active) + await advanceEpochAsync(); // epoch 2 + // rewards paid for stake in epoch 1. + const { reward } = await rewardPoolMembersAsync( + { poolId, stake }, + ); + const { deposit } = await undelegateStakeAsync(poolId, delegator); + expect(deposit).to.bignumber.eq(reward); + const delegatorReward = await getDelegatorRewardAsync(poolId, delegator); + expect(delegatorReward).to.bignumber.eq(0); + }); + + it('has correct reward immediately after unstaking and restaking', async () => { const poolId = hexRandom(); const { delegator, stake } = await delegateStakeAsync(poolId); await advanceEpochAsync(); // epoch 1 (stake now active) @@ -282,8 +297,28 @@ blockchainTests.resets('delegator unit rewards', env => { const { reward } = await rewardPoolMembersAsync( { poolId, stake }, ); + const { deposit } = await undelegateStakeAsync(poolId, delegator); + expect(deposit).to.bignumber.eq(reward); + await delegateStakeAsync(poolId, { delegator, stake }); + const delegatorReward = await getDelegatorRewardAsync(poolId, delegator); + expect(delegatorReward).to.bignumber.eq(0); + }); + + it.only('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) + await advanceEpochAsync(); // epoch 2 + // rewards paid for stake in epoch 1. + await rewardPoolMembersAsync({ poolId, stake }); await undelegateStakeAsync(poolId, delegator); + await delegateStakeAsync(poolId, { delegator, stake }); await advanceEpochAsync(); // epoch 3 + await advanceEpochAsync(); // epoch 4 + // rewards paid for stake in epoch 3. + const { reward } = await rewardPoolMembersAsync( + { poolId, stake }, + ); const delegatorReward = await getDelegatorRewardAsync(poolId, delegator); expect(delegatorReward).to.bignumber.eq(reward); }); diff --git a/contracts/utils/contracts/src/LibFractions.sol b/contracts/utils/contracts/src/LibFractions.sol index 9f3f7750c2..01a44aabf8 100644 --- a/contracts/utils/contracts/src/LibFractions.sol +++ b/contracts/utils/contracts/src/LibFractions.sol @@ -57,7 +57,7 @@ library LibFractions { pure returns (uint256 result) { - if (s == 0) { + if (s == 0 || d1 == 0) { return 0; } if (n2 == 0) {