govstaking/src/BinaryEligibilityOracleEarningPowerCalculator.sol #19
Conversation
| STALE_ORACLE_WINDOW = _staleOracleWindow; | ||
| _setOraclePauseGuardian(_oraclePauseGuardian); | ||
| _setDelegateeScoreEligibilityThreshold(_delegateeScoreEligibilityThreshold); | ||
| _setUpdateEligibilityDelay(_updateEligibilityDelay); |
There was a problem hiding this comment.
it seems that lastOracleUpdateTime is not set here, so it means that the oracle is created in _isOracleStale() == true state. Some users may profit off of that by checkpointing their earningPower at this moment, since it would be max, independently of quality of delegatee
There was a problem hiding this comment.
Good edge case. But I think this also depends on the time it takes to update the delegatee score after creation. Even if they initially receive a higher earning power, once the first request to updateDelegateeScore is made, it will be bumped down because the all delegatees would initially be considered ineligible
There was a problem hiding this comment.
Presumably, it would be updated relatively soon after deployment.
| if (msg.sender != scoreOracle) { | ||
| revert BinaryEligibilityOracleEarningPowerCalculator__Unauthorized("not oracle", msg.sender); | ||
| } | ||
| if (delegateeScoreLockStatus[_delegatee]) { |
There was a problem hiding this comment.
INFO: If all delegatees are locked (to zero for example), the oracle is forced into stale mode, potentially unlocking max earningPower again
| emit DelegateeEligibilityThresholdScoreSet( | ||
| delegateeEligibilityThresholdScore, _newDelegateeScoreEligibilityThreshold | ||
| ); | ||
| delegateeEligibilityThresholdScore = _newDelegateeScoreEligibilityThreshold; |
There was a problem hiding this comment.
If delegatees get ineligible due to an increase of eligibilityThreshold, they also get instantly bumpable, because the logic checks if previous score is greater than current eligibilityThreshold.
There was a problem hiding this comment.
Additionally, after the delegatee becomes ineligible, the updateEligibilityDelay wouldn't work as expected. Because timeOfIneligibility[_delegatee] is never updated in this case. Due to this all the deposits with this delegatee could be instantly bumped down.
| address _delegatee, | ||
| uint256 /* _oldEarningPower */ | ||
| ) external view returns (uint256, bool) { | ||
| if (_isOracleStale() || isOraclePaused) return (_amountStaked, true); |
There was a problem hiding this comment.
As _amountStaked is returned as earning power when the oracle is stale or paused. A malicious user can bump all the deposits which are delegated to ineligible delegatees to claim tips if the oracle ever becomes stale or paused.
I think its better to return _oldEarningPower when the oracle is stale or paused.
What are your thoughts on this?
There was a problem hiding this comment.
This is a good catch and a valid concern.
Using the oldEarningPower does make sense in most scenarios. There are some tradeoffs, for example if the oracle is paused/stale for an extended period of time then a delegatee that was previously ineligible before the oracle was paused/stale, then became eligible after the oracle is paused/stale, they will still retain the previous ineligible status.
There was a problem hiding this comment.
I think we want to return the amount staked mainly because in those situations the oracle could be malfunctioning or malicious. As @0xSpearmint mentions there are situations where some are worse off because we are using oldEarningPower , rather than rely on old or malicious earning power we revert to a state where only stake is considered to earn rewards.
govstaking/src/BinaryEligibilityOracleEarningPowerCalculator.sol