code-423n4 / 2021-12-sublime-findings

0 stars 0 forks source link

Inline unnecessary function can make the code simpler and save some gas #124

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-12-sublime/blob/9df1b7c4247f8631647c7627a8da9bdc16db8b11/contracts/CreditLine/CreditLine.sol#L466-L472

  function updateinterestAccruedTillLastPrincipalUpdate(uint256 _id) internal {
      require(creditLineVariables[_id].status == CreditLineStatus.ACTIVE, 'CreditLine: The credit line is not yet active.');

      uint256 _interestAccrued = calculateInterestAccrued(_id);
      uint256 _newInterestAccrued = (creditLineVariables[_id].interestAccruedTillLastPrincipalUpdate).add(_interestAccrued);
      creditLineVariables[_id].interestAccruedTillLastPrincipalUpdate = _newInterestAccrued;
  }

https://github.com/code-423n4/2021-12-sublime/blob/9df1b7c4247f8631647c7627a8da9bdc16db8b11/contracts/CreditLine/CreditLine.sol#L691-L727

    function borrow(uint256 _id, uint256 _amount) external payable nonReentrant onlyCreditLineBorrower(_id) {
        require(creditLineVariables[_id].status == CreditLineStatus.ACTIVE, 'CreditLine: The credit line is not yet active.');
        uint256 _borrowableAmount = calculateBorrowableAmount(_id);
        require(_amount <= _borrowableAmount, "CreditLine::borrow - The current collateral ratio doesn't allow to withdraw the amount");
        address _borrowAsset = creditLineConstants[_id].borrowAsset;
        address _lender = creditLineConstants[_id].lender;

        updateinterestAccruedTillLastPrincipalUpdate(_id);
        creditLineVariables[_id].principal = creditLineVariables[_id].principal.add(_amount);
        creditLineVariables[_id].lastPrincipalUpdateTime = block.timestamp;

        uint256 _tokenDiffBalance;
        if (_borrowAsset != address(0)) {
            uint256 _balanceBefore = IERC20(_borrowAsset).balanceOf(address(this));
            _withdrawBorrowAmount(_borrowAsset, _amount, _lender);
            uint256 _balanceAfter = IERC20(_borrowAsset).balanceOf(address(this));
            _tokenDiffBalance = _balanceAfter.sub(_balanceBefore);
        } else {
            uint256 _balanceBefore = address(this).balance;
            _withdrawBorrowAmount(_borrowAsset, _amount, _lender);
            uint256 _balanceAfter = address(this).balance;
            _tokenDiffBalance = _balanceAfter.sub(_balanceBefore);
        }
        uint256 _protocolFee = _tokenDiffBalance.mul(protocolFeeFraction).div(10**30);
        _tokenDiffBalance = _tokenDiffBalance.sub(_protocolFee);

        if (_borrowAsset == address(0)) {
            (bool feeSuccess, ) = protocolFeeCollector.call{value: _protocolFee}('');
            require(feeSuccess, 'Transfer fail');
            (bool success, ) = msg.sender.call{value: _tokenDiffBalance}('');
            require(success, 'Transfer fail');
        } else {
            IERC20(_borrowAsset).safeTransfer(protocolFeeCollector, _protocolFee);
            IERC20(_borrowAsset).safeTransfer(msg.sender, _tokenDiffBalance);
        }
        emit BorrowedFromCreditLine(_id, _tokenDiffBalance);
    }

updateinterestAccruedTillLastPrincipalUpdate() is unnecessary as it's being used only once. Therefore it can be inlined in borrow() to make the code simpler and save gas.

Recommendation

Change to:

  function borrow(uint256 _id, uint256 _amount) external payable nonReentrant onlyCreditLineBorrower(_id) {
      require(creditLineVariables[_id].status == CreditLineStatus.ACTIVE, 'CreditLine: The credit line is not yet active.');
      uint256 _borrowableAmount = calculateBorrowableAmount(_id);
      require(_amount <= _borrowableAmount, "CreditLine::borrow - The current collateral ratio doesn't allow to withdraw the amount");
      address _borrowAsset = creditLineConstants[_id].borrowAsset;
      address _lender = creditLineConstants[_id].lender;

      creditLineVariables[_id].interestAccruedTillLastPrincipalUpdate = (creditLineVariables[_id].interestAccruedTillLastPrincipalUpdate).add(calculateInterestAccrued(_id));
      creditLineVariables[_id].principal = creditLineVariables[_id].principal.add(_amount);
      creditLineVariables[_id].lastPrincipalUpdateTime = block.timestamp;

      uint256 _tokenDiffBalance;
      if (_borrowAsset != address(0)) {
          uint256 _balanceBefore = IERC20(_borrowAsset).balanceOf(address(this));
          _withdrawBorrowAmount(_borrowAsset, _amount, _lender);
          uint256 _balanceAfter = IERC20(_borrowAsset).balanceOf(address(this));
          _tokenDiffBalance = _balanceAfter.sub(_balanceBefore);
      } else {
          uint256 _balanceBefore = address(this).balance;
          _withdrawBorrowAmount(_borrowAsset, _amount, _lender);
          uint256 _balanceAfter = address(this).balance;
          _tokenDiffBalance = _balanceAfter.sub(_balanceBefore);
      }
      uint256 _protocolFee = _tokenDiffBalance.mul(protocolFeeFraction).div(10**30);
      _tokenDiffBalance = _tokenDiffBalance.sub(_protocolFee);

      if (_borrowAsset == address(0)) {
          (bool feeSuccess, ) = protocolFeeCollector.call{value: _protocolFee}('');
          require(feeSuccess, 'Transfer fail');
          (bool success, ) = msg.sender.call{value: _tokenDiffBalance}('');
          require(success, 'Transfer fail');
      } else {
          IERC20(_borrowAsset).safeTransfer(protocolFeeCollector, _protocolFee);
          IERC20(_borrowAsset).safeTransfer(msg.sender, _tokenDiffBalance);
      }
      emit BorrowedFromCreditLine(_id, _tokenDiffBalance);
  }
ritik99 commented 2 years ago

We believe the code separation helps with readability