more explicit sanity checks for computing balance in interval (previously all failed with div-by-zero)

typos

contracts/staking/contracts/src/libs/LibStakingRichErrors.sol

@@ -51,6 +51,12 @@ library LibStakingRichErrors {
         PoolIsFull
     }
 
+    enum CumulativeRewardIntervalErrorCode {
+        BeginEpochMustBeLessThanEndEpoch,
+        BeginEpochDoesNotHaveReward,
+        EndEpochDoesNotHaveReward
+    }
+
     // bytes4(keccak256("MiscalculatedRewardsError(uint256,uint256)"))
     bytes4 internal constant MISCALCULATED_REWARDS_ERROR_SELECTOR =
         0xf7806c4e;
@@ -147,6 +153,10 @@ library LibStakingRichErrors {
     bytes internal constant INVALID_WETH_ASSET_DATA_ERROR =
         hex"24bf322c";
 
+    // bytes4(keccak256("CumulativeRewardIntervalError(uint8,bytes32,uint256,uint256)"))
+    bytes4 internal constant CUMULATIVE_REWARD_INTERVAL_ERROR_SELECTOR =
+        0x1f806d55;
+
     // solhint-disable func-name-mixedcase
     function MiscalculatedRewardsError(
         uint256 totalRewardsPaid,
@@ -455,4 +465,23 @@ library LibStakingRichErrors {
     {
         return INVALID_WETH_ASSET_DATA_ERROR;
     }
+
+    function CumulativeRewardIntervalError(
+        CumulativeRewardIntervalErrorCode errorCode,
+        bytes32 poolId,
+        uint256 beginEpoch,
+        uint256 endEpoch
+    )
+        internal
+        pure
+        returns (bytes memory)
+    {
+        return abi.encodeWithSelector(
+            CUMULATIVE_REWARD_INTERVAL_ERROR_SELECTOR,
+            errorCode,
+            poolId,
+            beginEpoch,
+            endEpoch
+        );
+    }
 }

contracts/staking/contracts/src/stake/MixinStake.sol

@@ -190,7 +190,7 @@ contract MixinStake is
         // increment how much stake has been delegated to pool
         _incrementNextBalance(delegatedStakeByPoolId[poolId], amount);
 
-        // synchronizes reward state in the pool that the staker is undelegating from
+        // synchronizes reward state in the pool that the staker is delegating to
         IStructs.StoredBalance memory finalDelegatedStakeToPoolByOwner = _loadAndSyncBalance(delegatedStakeToPoolByOwner[owner][poolId]);
         _syncRewardsForDelegator(poolId, owner, initDelegatedStakeToPoolByOwner, finalDelegatedStakeToPoolByOwner);
     }

contracts/staking/contracts/src/staking_pools/MixinCumulativeRewards.sol

@@ -263,7 +263,7 @@ contract MixinCumulativeRewards is
 
     /// @dev Computes a member's reward over a given epoch interval.
     /// @param poolId Uniqud Id of pool.
-    /// @param memberStakeOverInterval Stake delegated to pool by meber over the interval.
+    /// @param memberStakeOverInterval Stake delegated to pool by member over the interval.
     /// @param beginEpoch beginning of interval.
     /// @param endEpoch end of interval.
     /// @return rewards accumulated over interval [beginEpoch, endEpoch]
@@ -277,19 +277,55 @@ contract MixinCumulativeRewards is
         view
         returns (uint256)
     {
-        // sanity check
+        // sanity check inputs
         if (memberStakeOverInterval == 0) {
             return 0;
         }
 
+        // sanity check interval
+        if (beginEpoch >= endEpoch) {
+            LibRichErrors.rrevert(
+                LibStakingRichErrors.CumulativeRewardIntervalError(
+                    LibStakingRichErrors.CumulativeRewardIntervalErrorCode.BeginEpochMustBeLessThanEndEpoch,
+                    poolId,
+                    beginEpoch,
+                    endEpoch
+                )
+            );
+        }
+
+        // sanity check begin reward
+        IStructs.Fraction memory beginReward = cumulativeRewardsByPool[poolId][beginEpoch];
+        if (!_isCumulativeRewardSet(beginReward)) {
+            LibRichErrors.rrevert(
+                LibStakingRichErrors.CumulativeRewardIntervalError(
+                    LibStakingRichErrors.CumulativeRewardIntervalErrorCode.BeginEpochDoesNotHaveReward,
+                    poolId,
+                    beginEpoch,
+                    endEpoch
+                )
+            );
+        }
+
+        // sanity check end reward
+        IStructs.Fraction memory endReward = cumulativeRewardsByPool[poolId][endEpoch];
+        if (!_isCumulativeRewardSet(endReward)) {
+            LibRichErrors.rrevert(
+                LibStakingRichErrors.CumulativeRewardIntervalError(
+                    LibStakingRichErrors.CumulativeRewardIntervalErrorCode.EndEpochDoesNotHaveReward,
+                    poolId,
+                    beginEpoch,
+                    endEpoch
+                )
+            );
+        }
+
         // compute reward
-        IStructs.Fraction memory beginRatio = cumulativeRewardsByPool[poolId][beginEpoch];
-        IStructs.Fraction memory endRatio = cumulativeRewardsByPool[poolId][endEpoch];
         uint256 reward = LibFractions.scaleFractionalDifference(
-            endRatio.numerator,
-            endRatio.denominator,
-            beginRatio.numerator,
-            beginRatio.denominator,
+            endReward.numerator,
+            endReward.denominator,
+            beginReward.numerator,
+            beginReward.denominator,
             memberStakeOverInterval
         );
         return reward;

contracts/staking/test/cumulative_reward_tracking_test.ts

@@ -34,7 +34,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 () => {
+        it('should set CR when a pool is created at epoch 0', async () => {
             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 () => {
@@ -166,7 +166,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => {
                 ],
             );
         });
-        it('should set CR and update Most Recent CR when redelegating, plus remove the CR that is no longer depends on.', async () => {
+        it('should set CR and update Most Recent CR when redelegating, plus remove the CR that it no longer depends on.', async () => {
             await simulation.runTestAsync(
                 [
                     TestAction.CreatePool, // creates CR in epoch 0

contracts/staking/test/rewards_test.ts

@@ -620,7 +620,9 @@ blockchainTests.resets('Testing Rewards', env => {
                 membersRewardVaultBalance: rewardsForDelegator[1],
             });
         });
-        it('Should collect fees correctly when there is a payout for `currentEpochBalance` but not `nextEpochBalance`', async () => {
+        it('Should collect fees correctly when re-delegating after un-delegating', async () => {
+            // Note - there are two ranges over which payouts are computed (see _computeRewardBalanceOfDelegator).
+            // This triggers the first range (rewards for `delegatedStake.currentEpoch`), but not the second.
             // first staker delegates (epoch 0)
             const rewardForDelegator = toBaseUnitAmount(10);
             const stakeAmount = toBaseUnitAmount(4);
@@ -640,7 +642,6 @@ blockchainTests.resets('Testing Rewards', env => {
             );
             // this should go to the delegator
             await payProtocolFeeAndFinalize(rewardForDelegator);
-
             // delegate stake ~ this will result in a payout where rewards are computed on
             // the balance's `currentEpochBalance` field but not the `nextEpochBalance` field.
             await stakers[0].moveStakeAsync(

packages/order-utils/CHANGELOG.json

@@ -93,6 +93,10 @@
             {
                 "note": "Add `InitializationError`, `InvalidParamValue` to `StakingRevertErrors`.",
                 "pr": 2131
+            },
+            {
+                "note": "Add `CumulativeRewardIntervalError`.",
+                "pr": 2154
             }
         ]
     },

packages/order-utils/src/staking_revert_errors.ts

@@ -30,6 +30,12 @@ export enum InitializationErrorCode {
     MixinParamsAlreadyInitialized,
 }
 
+export enum CumulativeRewardIntervalErrorCode {
+    BeginEpochMustBeLessThanEndEpoch,
+    BeginEpochDoesNotHaveReward,
+    EndEpochDoesNotHaveReward,
+}
+
 export class MiscalculatedRewardsError extends RevertError {
     constructor(totalRewardsPaid?: BigNumber | number | string, initialContractBalance?: BigNumber | number | string) {
         super(
@@ -225,6 +231,21 @@ export class ProxyDestinationCannotBeNilError extends RevertError {
     }
 }
 
+export class CumulativeRewardIntervalError extends RevertError {
+    constructor(
+        errorCode?: CumulativeRewardIntervalErrorCode,
+        poolId?: string,
+        beginEpoch?: BigNumber | number | string,
+        endEpoch?: BigNumber | number | string,
+    ) {
+        super(
+            'CumulativeRewardIntervalError',
+            'CumulativeRewardIntervalError(uint8 errorCode, bytes32 poolId, uint256 beginEpoch, uint256 endEpoch)',
+            { errorCode, poolId, beginEpoch, endEpoch },
+        );
+    }
+}
+
 const types = [
     AmountExceedsBalanceOfPoolError,
     BlockTimestampTooLowError,
@@ -249,6 +270,7 @@ const types = [
     RewardVaultNotSetError,
     WithdrawAmountExceedsMemberBalanceError,
     ProxyDestinationCannotBeNilError,
+    CumulativeRewardIntervalError,
 ];
 
 // Register the types we've defined.

packages/utils/src/safe_math_revert_errors.ts

@@ -11,6 +11,7 @@ export enum BinOpErrorCodes {
 }
 
 export enum DowncastErrorCodes {
+    ValueTooLargeToDowncastToUint32,
     ValueTooLargeToDowncastToUint64,
     ValueTooLargeToDowncastToUint96,
 }