MixinRefunds.sol#_getCancelAndRefundValue Cache and read storage variables from the stack can save gas

[code423n4]

Handle

WatchPug

Vulnerability details

For the storage variables that will be accessed multiple times, cache and read from the stack can save ~100 gas from each extra read (SLOAD after Berlin).

In MixinRefunds.sol#_getCancelAndRefundValue: freeTrialLength, expirationDuration and keyPrice are accessed multiple times.

https://github.com/code-423n4/2021-11-unlock/blob/ec41eada1dd116bcccc5603ce342257584bec783/smart-contracts/contracts/mixins/MixinRefunds.sol#L133-L159

  function _getCancelAndRefundValue(
    address _keyOwner
  )
    private view
    hasValidKey(_keyOwner)
    returns (uint refund)
  {
    Key storage key = keyByOwner[_keyOwner];
    // Math: safeSub is not required since `hasValidKey` confirms timeRemaining is positive
    uint timeRemaining = key.expirationTimestamp - block.timestamp;
    if(timeRemaining + freeTrialLength >= expirationDuration) {
      refund = keyPrice;
    } else {
      refund = keyPrice * timeRemaining / expirationDuration;
    }

    // Apply the penalty if this is not a free trial
    if(freeTrialLength == 0 || timeRemaining + freeTrialLength < expirationDuration)
    {
      uint penalty = keyPrice * refundPenaltyBasisPoints / BASIS_POINTS_DEN;
      if (refund > penalty) {
        refund -= penalty;
      } else {
        refund = 0;
      }
    }
  }

Recommendation

Change to:

  function _getCancelAndRefundValue(
    address _keyOwner
  )
    private view
    hasValidKey(_keyOwner)
    returns (uint refund)
  {
    Key storage key = keyByOwner[_keyOwner];
    // Math: safeSub is not required since `hasValidKey` confirms timeRemaining is positive
    uint timeRemaining = key.expirationTimestamp - block.timestamp;
    uint _expirationDuration = expirationDuration;
    uint _freeTrialLength = freeTrialLength;
    uint _keyPrice = keyPrice;
    if(timeRemaining + _freeTrialLength >= _expirationDuration) {
      refund = _keyPrice;
    } else {
      refund = _keyPrice * timeRemaining / _expirationDuration;
    }

    // Apply the penalty if this is not a free trial
    if(_freeTrialLength == 0 || timeRemaining + _freeTrialLength < _expirationDuration)
    {
      uint penalty = _keyPrice * refundPenaltyBasisPoints / BASIS_POINTS_DEN;
      if (refund > penalty) {
        refund -= penalty;
      } else {
        refund = 0;
      }
    }
  }