code-423n4 / 2022-03-sublime-findings

0 stars 0 forks source link

Gas Optimizations #22

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

G01: Shift if case up to return earlier

Line References

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L950

Description

The case if (_maxPossible <= _currentDebt) return 0; can be shifted up a couple of lines to avoid the additional storage reads of the borrow limit and principal.

Recommended Mitigation Steps

uint256 _maxPossible = type(uint256).max;
if (_collateralRatio != 0) {
  _maxPossible = _totalCollateralToken.mul(_ratioOfPrices).div(_collateralRatio).mul(SCALING_FACTOR).div(10**_decimals);
}
// shifted up
if (_maxPossible <= _currentDebt) return 0;

uint256 _borrowLimit = pooledCreditLineConstants[_id].borrowLimit;
uint256 _principal = pooledCreditLineVariables[_id].principal;

G02: Duplicate time check if cancelling request on low collection

Line References

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L1174

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/LenderPool.sol#L315

Description

CRLC2 and S2 are essentially the same checks because pooledCreditLineConstants[_id].startsAt and pooledCLConstants[_id].startTime have the same value.

Recommended Mitigation Steps

Either one of the checks can be removed.

G03: from != address(0) check is redundant in _rebalanceInterestWithdrawn()

Line References

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/LenderPool.sol#L700

Description

_rebalanceInterestWithdrawn() is called just once in a couple of lines above in _beforeTokenTransfer(). Note that the function won’t be invoked if from == address(0).

if (from == address(0)) {
  // note: replaced with continue;
  continue;
}

Hence, from will always be a non-zero address in _rebalanceInterestWithdrawn(), making the check redundant.

Recommended Mitigation Steps

if (to != address(0)) {
  _withdrawInterest(id, from);
  _withdrawInterest(id, to);
}
ritik99 commented 2 years ago

All suggestions are valid.