Summary

getPeriodReward function from the Linear Distribution Interval Decrease library sometimes provides different rewards for the same time period when decrease amount is 0 due truncation when duration (endTime - startTime) is equal to the interval, but startTime != payoutStart + x * interval. The probability of getting a wrong reward is based on the following:

• initialAmount doesn't divide exactly by interval

• startTime and endTime values placement in intervals

Note: Please note that this finding is marked as a medium risk in the context of the reward function corectness and output consistency. Consequently, unit names for parameters are specifically avoided not to cause bias w.r.t. impact. Functionality is packed as a library, which should be agnostic of the system's configuration and parameters (currency, token value, unit precision, parameter tuning, etc.). At the whole system level, impact might be limited/minimized by the use case.

Vulnerability Details

For certain pool configurations, some users will not get the correct reward because of truncation. Looking at it another way, certain users can time their transactions to obtain better rewards.

=== Context ===
Let's look at the getPeriodReward function from the LinearDistributionIntervalDecrease library. This function will provide a reward amount based on some variables:

• the initial reward amount per interval prior to decreasing - in the constant reward (0 decrease) scenario, which is explicitly supported by looking at line 6 comment in the lib, this is the constant reward amount

• payoutStart - timestamp since which the payout process starts

• startTime, endTime - delimits time delta in which reward is calculated

• interval - time increment per which the reward is paid, and after which reward is decreased - for the latter it's not the case in 0 decrease scenario

In a scenario where the reward decreases, the variations of startTime and endTime with respect to payoutStart are relevant. For example:

                   r = 100          r = 90

payoutStart>[ . . . . . . . . . ][ . . . . . . . . . .]
^ ^ ^ ^
s1 s2 e1 e2
In this case, s1 = payoutStart, s2 = payoutStart + interval, s2 = payoutStart + interval/2 , e2 = payoutStart + interval + interval/2

For time interval s1-e1, expected return value is 100, but for time interval s2-e2, expected value is 95
However, in a scenario where the reward is never decreased (decreaseAmount = 0), if (e2 - s2) == (e1 - s2) rewards should be identical.

Summarizing, for the constant reward scenario, the reward amounted for the same period of time should be the same, regardless of time passed / intervals passed since payoutStart.
However, there are certain configurations where the above statement is not respected.

=== Problem ===
Let's look at the following calls of the getPeriodReward
Function parameters:

initialAmount = 100
decreaseAmount = 0;
payoutStart = 25;
interval = 11;
startTime = 25;
endTime = 36 (exactly 1 interval).

For the provided parameters, function call returns exactly 100, which is the expected amount for 1 interval with 0 decrease.
However, if startTime and endTime are increased with 1 time unit (still exactly 1 interval), provided return value will be 99.
If startTime != payoutStart + x * interval and the reward amount is small enough, the value will be truncated

This is a mild case, where the difference is ~1%.
However, for the same parameters, if initialReward is changed to 2, first case reward will be 2, while second case reward will be 1. It's a 2x difference for the same amount of time. For a reward as small as 2, truncation will happen for virtually any interval >= 3 time units.

=== Why is this happening? ===
Note: for simplicity, in all future examples, decreaseAmount is 0 and payoutStart is 0

This is happening because the reward is calculated as a sum of three parts: reward(part interval of startTime) + reward(full intervals) + reward(part interval of endTime)
This way of calculating is necessary when decreaseAmount != 0, as time spent in intervals closer to payoutStart is more valuable. However, this doesn't bring any value in the 0-decrease scenario.

By calculating the reward as such, the algorithm computes the rewards from 3 unsigned integers. Since the implementation in the two functions that compute these integers boils down to reward * timeDelta / interval. This way, the chance of getting a truncated results depends on the variation of these parameters. Consequently, the chance of it happening increases

• for each divisor of interval that is not a divisor of reward

• when startTime/endTime are very close (but not exactly on) interval end/start
  -e.g.: for parameters amount=3, interval = 3, startTime = 1, endTime = 4 -> reward is 3
  for parameters amount=2, interval = 3, startTime = 1, endTime = 4 -> reward is 2
  for parameters amount = 75, interval = 2, startTime = 1, endTime = 3 -> reward is 74

Since startTime and endTime are derived from block.timestamp, this gives the user the power to time his transactions in such a way to maximize revenue, whilst incorrect rewards are provided to some of the honest users.

Impact

For certain configurations, an honest user can earn anything down to 50% of the expected reward, while some users can abuse this and time their transaction to always earn the maximal reward, which would lead to spikes in the usage pattern.

Tools Used

Remix for function testing
Manual inspection

Recommendations

Ideally, for the constant reward case case, a separate implementation shall be used, which simply computes the reward as (endTime - startTime) * initalAmount / interval. The exact implementation is provided in a separate finding that addresses the gas cost of the function in the 0-decrease scenario.

If by any reason a separate implementation is not desired, a second-best solution would be to validate pool configurations by ensuring initialAmount divides exactly by interval.