Skip to content

Latest commit

 

History

History
79 lines (57 loc) · 3.4 KB

039.md

File metadata and controls

79 lines (57 loc) · 3.4 KB

Gentle Chocolate Eel

High

scaledUnclaimedRewardCheckpoint is not properly reset to zero after the reward has been claimed in the GovernorStaker._claimReward()

Summary

scaledUnclaimedRewardCheckpoint is not properly reset to zero after the reward has been claimed

Vulnerability Detail

In the GovernorStaker contract, a comment is written by the protocol indicating that the value of scaledUnclaimedRewardCheckpoint is set to 0 when deposit rewards are claimed.

  /// @param scaledUnclaimedRewardCheckpoint Checkpoint of the unclaimed rewards earned by a given
  /// deposit with the scale factor included. This value is stored any time an action is taken that
  /// specifically impacts the rate at which rewards are earned by a given deposit. Total unclaimed
  /// rewards for a deposit are thus this value plus all rewards earned after this checkpoint was
  /// taken. This value is reset to zero when the deposit's rewards are claimed.

However in the _claimReward when deposits are claimed, the code currently only subtracts the reward from it. After this subtraction, theres not part in the snippet which set scaledUnclaimedRewardCheckpoint to 0, and this does not align with what above the comment says.

  function _claimReward(DepositIdentifier _depositId, Deposit storage deposit, address _claimer)
    internal
    virtual
    returns (uint256)
  {
    _checkpointGlobalReward();
    _checkpointReward(deposit);


    uint256 _reward = deposit.scaledUnclaimedRewardCheckpoint / SCALE_FACTOR;
    // Intentionally reverts due to overflow if unclaimed rewards are less than fee.
    uint256 _payout = _reward - claimFeeParameters.feeAmount;
    if (_payout == 0) return 0;


    // retain sub-wei dust that would be left due to the precision loss
    deposit.scaledUnclaimedRewardCheckpoint =
      deposit.scaledUnclaimedRewardCheckpoint - (_reward * SCALE_FACTOR);
    emit RewardClaimed(_depositId, _claimer, _payout);


    uint256 _newEarningPower =
      earningPowerCalculator.getEarningPower(deposit.balance, deposit.owner, deposit.delegatee);


    totalEarningPower =
      _calculateTotalEarningPower(deposit.earningPower, _newEarningPower, totalEarningPower);
    depositorTotalEarningPower[deposit.owner] = _calculateTotalEarningPower(
      deposit.earningPower, _newEarningPower, depositorTotalEarningPower[deposit.owner]
    );
    deposit.earningPower = _newEarningPower.toUint96();


    SafeERC20.safeTransfer(REWARD_TOKEN, _claimer, _payout);
    if (claimFeeParameters.feeAmount > 0) {
      SafeERC20.safeTransfer(
        REWARD_TOKEN, claimFeeParameters.feeCollector, claimFeeParameters.feeAmount
      );
    }
    return _payout;              // @audit>> scaledUnclaimedRewardCheckpoint is not set to 0 as comment says in ln:131-134
  }

Impact

This may lead to incorrect reward calculations since the protocol has reasons for commenting that they would always reset the scaled Unclaimed Reward Checkpoint to 0 after deposit rewards are claimed.

Code Snippet

https://github.com/sherlock-audit/2024-11-tally/blob/main/staker/src/GovernanceStaker.sol#L131-L135

https://github.com/sherlock-audit/2024-11-tally/blob/main/staker/src/GovernanceStaker.sol#L710-L751

Tool used

Manual Review

Recommendation

scaledUnclaimedRewardCheckpoint should be reset to 0 as the comment says