Incorrect reward epoch start date calculation
The Forta rewards system is based on epochs. A privileged address with the role `REWARDER_ROLE` calls the `reward()` function with a parameter for a specific `epochNumber` that consequently distributes the rewards for that epoch. Additionally, as users stake and delegate their stake, accounts in the Forta system accrue weight that is based on the active stake to distribute these rewards. Since accounts can modify their stake as well as delegate or un-delegate it, the rewards weight for each account can be modified, as seen, for example, in the `didAllocate()` function. In turn, this modifies the `DelegatedAccRewards` storage struct that stores the accumulated rewards for each share id. To keep track of changes done to the accumulated rewards, epochs with checkpoints are used to manage the accumulated rate of rewards, their value at the checkpoint, and the timestamp of the checkpoint.\\nFor example, in the `didAllocate()` function the `addRate()` function is being called to modify the accumulated rewards.\\n```\\nfunction didAllocate(\\n    uint8 subjectType,\\n    uint256 subject,\\n    uint256 stakeAmount,\\n    uint256 sharesAmount,\\n    address staker\\n) external onlyRole(ALLOCATOR\\_CONTRACT\\_ROLE) {\\n    bool delegated = getSubjectTypeAgency(subjectType) == SubjectStakeAgency.DELEGATED;\\n    if (delegated) {\\n        uint8 delegatorType = getDelegatorSubjectType(subjectType);\\n        uint256 shareId = FortaStakingUtils.subjectToActive(delegatorType, subject);\\n        DelegatedAccRewards storage s = \\_rewardsAccumulators[shareId];\\n        s.delegated.addRate(stakeAmount);\\n```\\n\\nThen the function flow goes into `setRate()` that checks the existing accumulated rewards storage and modifies it based on the current timestamp.\\n```\\nfunction addRate(Accumulator storage acc, uint256 rate) internal {\\n    setRate(acc, latest(acc).rate + rate);\\n}\\n```\\n\\n```\\nfunction setRate(Accumulator storage acc, uint256 rate) internal {\\n    EpochCheckpoint memory ckpt = EpochCheckpoint({ timestamp: SafeCast.toUint32(block.timestamp), rate: SafeCast.toUint224(rate), value: getValue(acc) });\\n    uint256 length = acc.checkpoints.length;\\n    if (length > 0 && isCurrentEpoch(acc.checkpoints[length - 1].timestamp)) {\\n        acc.checkpoints[length - 1] = ckpt;\\n    } else {\\n        acc.checkpoints.push(ckpt);\\n    }\\n}\\n```\\n\\nNamely, it pushes epoch checkpoints to the list of account checkpoints based on its timestamp. If the last checkpoint's timestamp is during the current epoch, then the last checkpoint is replaced with the new one altogether. If the last checkpoint's timestamp is different from the current epoch, a new checkpoint is added to the list. However, the `isCurrentEpoch()` function calls a function `getCurrentEpochTimestamp()` that incorrectly determines the start date of the current epoch. In particular, it doesn't take the offset into account when calculating how many epochs have already passed.\\n```\\nfunction getCurrentEpochTimestamp() internal view returns (uint256) {\\n    return ((block.timestamp / EPOCH\\_LENGTH) \\* EPOCH\\_LENGTH) + TIMESTAMP\\_OFFSET;\\n}\\n\\nfunction isCurrentEpoch(uint256 timestamp) internal view returns (bool) {\\n    uint256 currentEpochStart = getCurrentEpochTimestamp();\\n    return timestamp > currentEpochStart;\\n}\\n```\\n\\nInstead of `((block.timestamp / EPOCH_LENGTH) * EPOCH_LENGTH) + TIMESTAMP_OFFSET`, it should be `(((block.timestamp - TIMESTAMP_OFFSET) / EPOCH_LENGTH) * EPOCH_LENGTH) + TIMESTAMP_OFFSET`. In fact, it should simply call the `getEpochNumber()` function that correctly provides the epoch number for any timestamp.\\n```\\nfunction getEpochNumber(uint256 timestamp) internal pure returns (uint32) {\\n    return SafeCast.toUint32((timestamp - TIMESTAMP\\_OFFSET) / EPOCH\\_LENGTH);\\n}\\n```\\n\\nIn other words, the resulting function would look something like the following:\\n```\\n    function getCurrentEpochTimestamp() public view returns (uint256) {\\n        return (getEpochNumber(block.timestamp) * EPOCH_LENGTH) + TIMESTAMP_OFFSET;\\n    }\\n```\\n\\nOtherwise, if `block.timestamp` is such that `(block.timestamp - TIMESTAMP_OFFSET) / EPOCH_LENGTH = n` and `block.timestamp` / EPOCH_LENGTH = n+1, which would happen on roughly 4 out of 7 days of the week since `EPOCH_LENGTH = 1 weeks` and `TIMESTAMP_OFFSET = 4 days`, this would cause the `getCurrentEpochTimestamp()` function to return the end timestamp of the epoch (which is in the future) instead of the start. Therefore, if a checkpoint with such a timestamp is committed to the account's accumulated rewards checkpoints list, it will always fail the below check in the epoch it got submitted, and any checkpoint committed afterwards but during the same epoch with a similar type of `block.timestamp` (i.e. satisfying the condition at the beginning of this paragraph), would be pushed to the top of the list instead of replacing the previous checkpoint.\\n```\\nif (length > 0 && isCurrentEpoch(acc.checkpoints[length - 1].timestamp)) {\\n    acc.checkpoints[length - 1] = ckpt;\\n} else {\\n    acc.checkpoints.push(ckpt);\\n```\\n\\nThis causes several checkpoints to be stored for the same epoch, which would cause issues in functions such as `getAtEpoch()`, that feeds into `getValueAtEpoch()` function that provides data for the rewards' share calculation. In the end, this would cause issues in the accounting for the rewards calculation resulting in incorrect distributions.\\nDuring the discussion with the Forta Foundation team, it was additionally discovered that there are edge cases around the limits of epochs. Specifically, epoch's end time and the subsequent epoch's start time are exactly the same, although it should be that it is only the start of the next epoch. Similarly, that start time isn't recognized as part of the epoch due to `>` sign instead of `>=`. In particular, the following changes need to be made:\\n```\\n    function getEpochEndTimestamp(uint256 epochNumber) public pure returns (uint256) {\\n        return ((epochNumber + 1) * EPOCH_LENGTH) + TIMESTAMP_OFFSET - 1; <---- so it is 23:59:59 instead of next day 00:00:00\\n    }\\n\\n    function isCurrentEpoch(uint256 timestamp) public view returns (bool) {\\n        uint256 currentEpochStart = getCurrentEpochTimestamp();\\n        return timestamp >= currentEpochStart; <--- for the first second on Monday\\n    }\\n```\\n
A refactor of the epoch timestamp calculation functions is recommended to account for:\\nThe correct epoch number to calculate the start and end timestamps of epochs.\\nThe boundaries of epochs coinciding.\\nClarity in functions' intent. For example, adding a function just to calculate any epoch's start time and renaming `getCurrentEpochTimestamp()` to `getCurrentEpochStartTimestamp()`.
```\\nfunction didAllocate(\\n    uint8 subjectType,\\n    uint256 subject,\\n    uint256 stakeAmount,\\n    uint256 sharesAmount,\\n    address staker\\n) external onlyRole(ALLOCATOR\\_CONTRACT\\_ROLE) {\\n    bool delegated = getSubjectTypeAgency(subjectType) == SubjectStakeAgency.DELEGATED;\\n    if (delegated) {\\n        uint8 delegatorType = getDelegatorSubjectType(subjectType);\\n        uint256 shareId = FortaStakingUtils.subjectToActive(delegatorType, subject);\\n        DelegatedAccRewards storage s = \\_rewardsAccumulators[shareId];\\n        s.delegated.addRate(stakeAmount);\\n```\\n