Skip to content

Latest commit

 

History

History
112 lines (88 loc) · 4.85 KB

057.md

File metadata and controls

112 lines (88 loc) · 4.85 KB
Raw

Rough Brick Meerkat

High

alterDelegatee affecting the rewards calculation of all stakers

Summary

When the staker changes the governance delegate using function alterDelegatee, it affects the rewards of all stakers.

Vulnerability Detail

The function alterDelegatee in GovernanceStaker contract is called to change the address to which governance voting power is assigned. In this function, call is made to function getEarningPower in the file BinaryEligibilityOracleEarningPowerCalculator.sol to get the earning power of _newDelegatee. As the delegateeScores[_newDelegatee] has not been updated, this value will be ‘0’. This value will make the _newDelegatee ineligible (because delegateeScores[_newDelegatee] < delegateeEligibilityThresholdScore).

This affects the deposit.earningPower and the totalEarningPower thereby not only affecting the reward for the concerned deposit but also for all the other stakes.

  function alterDelegatee(DepositIdentifier _depositId, address _newDelegatee) external virtual {
    Deposit storage deposit = deposits[_depositId];
    _revertIfNotDepositOwner(deposit, msg.sender);
    _alterDelegatee(deposit, _depositId, _newDelegatee);
  }
  function _alterDelegatee(
    Deposit storage deposit,
    DepositIdentifier _depositId,
    address _newDelegatee
  ) internal virtual {
    _revertIfAddressZero(_newDelegatee);
    _checkpointGlobalReward();
    _checkpointReward(deposit);


    DelegationSurrogate _oldSurrogate = surrogates(deposit.delegatee);
    uint256 _newEarningPower =
      earningPowerCalculator.getEarningPower(deposit.balance, deposit.owner, _newDelegatee);


    totalEarningPower =
      _calculateTotalEarningPower(deposit.earningPower, _newEarningPower, totalEarningPower);
    depositorTotalEarningPower[deposit.owner] = _calculateTotalEarningPower(
      deposit.earningPower, _newEarningPower, depositorTotalEarningPower[deposit.owner]
    );
  function getEarningPower(uint256 _amountStaked, address, /* _staker */ address _delegatee)
    external
    view
    returns (uint256)
  {
    if (_isOracleStale() || isOraclePaused) return _amountStaked;
    return _isDelegateeEligible(_delegatee) ? _amountStaked : 0;
  }
  function _isDelegateeEligible(address _delegatee) internal view returns (bool) {
    return delegateeScores[_delegatee] >= delegateeEligibilityThresholdScore;
  }

Impact

Affects negatively for the staker who is changing the governance delegate and positively for all other stakers.

Code Snippet

https://github.com/sherlock-audit/2024-11-tally/blob/b125d1f2b52170a3789b1060a52fc6609e6e2262/staker/src/GovernanceStaker.sol#L374-L378

https://github.com/sherlock-audit/2024-11-tally/blob/b125d1f2b52170a3789b1060a52fc6609e6e2262/staker/src/GovernanceStaker.sol#L624-L641

https://github.com/sherlock-audit/2024-11-tally/blob/b125d1f2b52170a3789b1060a52fc6609e6e2262/staker/src/BinaryEligibilityOracleEarningPowerCalculator.sol#L130-L137

https://github.com/sherlock-audit/2024-11-tally/blob/b125d1f2b52170a3789b1060a52fc6609e6e2262/staker/src/BinaryEligibilityOracleEarningPowerCalculator.sol#L282-L284

Tool used

Manual Review

Recommendation

In the GovernanceStaker contract, a) The function alterDelegatee should be called with one additional parameter _oldDelegatee. b) The internal function _alterDelegatee should be called with one additional parameter _oldDelegatee. c) _newScore = delegateeScores[_oldDelegatee]; d) call the internal function _updateDelegateeScore(address _newDelegatee, uint256 _newScore) prior to _newEarningPower calculation at #L634 in GovernanceStaker contract.

The modified code may look like

  function alterDelegatee(DepositIdentifier _depositId, address _oldDelegatee, address _newDelegatee) external virtual {   //modified
    Deposit storage deposit = deposits[_depositId];
    _revertIfNotDepositOwner(deposit, msg.sender);
    _alterDelegatee(deposit, _depositId, _newDelegatee);
  }
  function _alterDelegatee(
    Deposit storage deposit,
    DepositIdentifier _depositId,
    address _oldDelegatee,          //added
    address _newDelegatee
  ) internal virtual {
    _revertIfAddressZero(_newDelegatee);
    _checkpointGlobalReward();
    _checkpointReward(deposit);


    DelegationSurrogate _oldSurrogate = surrogates(deposit.delegatee);
     _newScore = delegateeScores[_oldDelegatee];	         //added
     _updateDelegateeScore(address _newDelegatee, uint256 _newScore);	         //added
    uint256 _newEarningPower =
      earningPowerCalculator.getEarningPower(deposit.balance, deposit.owner, _newDelegatee);


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