Description

There is an issue in the redelegation logic of the ValidatorStaking contract. In the _checkRedelegationCooldown function, data for a custom error is calculated even when the validation check passes, which indicates a potential underflow/overflow error.

See the original implementation using the require statement:

function _checkRedelegationCooldown(StorageV1 storage $, uint48 now_, address delegator, address valAddr)
  internal
  view
{
  uint256 lastRedelegationTime_ = $.lastRedelegationTime[delegator][valAddr];

  if (lastRedelegationTime_ > 0) {
    uint48 cooldown = $.redelegationCooldown;
    uint48 lasttime = lastRedelegationTime_.toUint48();
    require(
      now_ >= lasttime + cooldown, //
      IValidatorStaking__CooldownNotPassed(lasttime, now_, (lasttime + cooldown) - now_)
    );
  }
}

In this implementation, the third parameter for the IValidatorStaking__CooldownNotPassed error calculates (lasttime + cooldown) - now_ even when now_ >= lasttime + cooldown is true. This calculation would cause an underflow when the cooldown has passed.

The issue was fixed by refactoring the code to use an if statement with revert:

function _checkRedelegationCooldown(StorageV1 storage $, uint48 now_, address delegator, address valAddr)
  internal
  view
{
  uint256 lastRedelegationTime_ = $.lastRedelegationTime[delegator][valAddr];

  if (lastRedelegationTime_ > 0) {
    uint48 cooldown = $.redelegationCooldown;
    uint48 lasttime = lastRedelegationTime_.toUint48();
    if (now_ < lasttime + cooldown) {
      revert IValidatorStaking__CooldownNotPassed(lasttime, now_, (lasttime + cooldown) - now_);
    }
  }
}

Impact

This is a minor code-quality issue that could potentially lead to unexpected reverts due to underflow when calculating the error-message parameters.

Recommendations

Consider refactoring the code to use an if statement that explicitly checks the condition and triggers a revert.

Remediation

This issue has been acknowledged by Mitosis, and a fix was implemented in commit 7ef4c62e↗.