Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas Optimizations #7

Open
code423n4 opened this issue Mar 29, 2022 · 1 comment
Open

Gas Optimizations #7

code423n4 opened this issue Mar 29, 2022 · 1 comment
Labels
bug Something isn't working G (Gas Optimization)

Comments

@code423n4
Copy link
Contributor

Gas Report

Table of Contents:

Foreword

  • @audit tags

The code is annotated at multiple places with //@audit comments to pinpoint the issues. Please, pay attention to them for more details.

Findings

Version

Upgrade pragma to at least 0.8.4

Using newer compiler versions and the optimizer give gas optimizations. Also, additional safety checks are available for free.

The advantages here are:

  • Low level inliner (>= 0.8.2): Cheaper runtime gas (especially relevant when the contract has small functions).
  • Optimizer improvements in packed structs (>= 0.8.3)
  • Custom errors (>= 0.8.4): cheaper deployment cost and runtime cost. Note: the runtime cost is only relevant when the revert condition is met. In short, replace revert strings by custom errors.

Consider upgrading pragma to at least 0.8.4:

PooledCreditLine/LenderPool.sol:2:pragma solidity 0.7.6;
PooledCreditLine/PooledCreditLine.sol:2:pragma solidity 0.7.6;
Verification/twitterVerifier.sol:2:pragma solidity 0.7.6;

Use Custom Errors instead of Revert Strings to save Gas after upgrading to 0.8.4+

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met)

Source: https://blog.soliditylang.org/2021/04/21/custom-errors/:

Starting from Solidity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors. Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");), but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them.

Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).

Instances include:

PooledCreditLine/LenderPool.sol:73:        require(msg.sender == address(POOLED_CREDIT_LINE), '!Pooled credit line');
PooledCreditLine/LenderPool.sol:78:        require(IPooledCreditLineVerifier(creditLines[creditLineID].verifier).isUser(msg.sender), '!Verified Lender');
PooledCreditLine/LenderPool.sol:115:        require(block.timestamp < creditLines[_id].startTime, '!Collecting');
PooledCreditLine/LenderPool.sol:119:        require(_maxLent > _totalLent, 'Lending Complete');
PooledCreditLine/LenderPool.sol:207:                require(_isLiquidationWithdrawn, "");
PooledCreditLine/LenderPool.sol:284:        require(_lendingShare != 0, "!lender");
PooledCreditLine/LenderPool.sol:304:        require(_lendingShare != 0, "!lender");
PooledCreditLine/LenderPool.sol:358:        require(from != to, 'self transfer');
PooledCreditLine/LenderPool.sol:361:            require(IPooledCreditLineVerifier(creditLines[id].verifier).isUser(to) || to == address(0), '!Verified && !burn');
PooledCreditLine/LenderPool.sol:369:                require(supply >= amount, 'amount > totalSupply');
PooledCreditLine/LenderPool.sol:372:                require(creditLines[id].areTokensTransferable, '!transferrable');
PooledCreditLine/PooledCreditLine.sol:162:        require(creditLineConstants[_id].borrower == msg.sender, '!Borrower');
PooledCreditLine/PooledCreditLine.sol:170:        require(lenderPool == msg.sender, '!LenderPool');
PooledCreditLine/PooledCreditLine.sol:322:        require(_min < _max, 'Invalid limits');
PooledCreditLine/PooledCreditLine.sol:333:        require(_min < _max, 'Invalid limits');
PooledCreditLine/PooledCreditLine.sol:344:        require(_min < _max, 'Invalid limits');
PooledCreditLine/PooledCreditLine.sol:355:        require(_min < _max, 'Invalid limits');
PooledCreditLine/PooledCreditLine.sol:366:        require(_min < _max, 'Invalid limits');
PooledCreditLine/PooledCreditLine.sol:377:        require(_min < _max, 'Invalid limits');
PooledCreditLine/PooledCreditLine.sol:388:        require(_min < _max, 'Invalid limits');
PooledCreditLine/PooledCreditLine.sol:439:        require(_priceOracle != address(0), "0 address");
PooledCreditLine/PooledCreditLine.sol:454:        require(_savingsAccount != address(0), "0 address");
PooledCreditLine/PooledCreditLine.sol:469:        require(_protocolFee <= SCALING_FACTOR, "< 100%");
PooledCreditLine/PooledCreditLine.sol:484:        require(_protocolFeeCollector != address(0), 'cant be 0 address');
PooledCreditLine/PooledCreditLine.sol:499:        require(_strategyRegistry != address(0), 'CL::I zero address');
PooledCreditLine/PooledCreditLine.sol:514:        require(_borrowerVerifier != address(0), 'CL::I zero address');
PooledCreditLine/PooledCreditLine.sol:529:        require(_verification != address(0), 'CL::I zero address');
PooledCreditLine/PooledCreditLine.sol:662:        require(IVerification(verification).isUser(msg.sender, borrowerVerifier), '!verified borrower');
PooledCreditLine/PooledCreditLine.sol:663:        require(_request.borrowAsset != _request.collateralAsset, 'borrow != collateral');
PooledCreditLine/PooledCreditLine.sol:664:        require(IPriceOracle(priceOracle).doesFeedExist(_request.borrowAsset, _request.collateralAsset), '!price');
PooledCreditLine/PooledCreditLine.sol:665:        require(_request.borrowAsset != address(0) && _request.collateralAsset != address(0), "ETH not allowed");
PooledCreditLine/PooledCreditLine.sol:666:        require(IStrategyRegistry(strategyRegistry).registry(_request.lenderStrategy) != 0, '!strategy');
PooledCreditLine/PooledCreditLine.sol:667:        require(IStrategyRegistry(strategyRegistry).registry(_request.collateralStrategy) != 0, '!strategy');
PooledCreditLine/PooledCreditLine.sol:668:        require(
PooledCreditLine/PooledCreditLine.sol:672:        require(
PooledCreditLine/PooledCreditLine.sol:676:        require(
PooledCreditLine/PooledCreditLine.sol:680:        require(
PooledCreditLine/PooledCreditLine.sol:684:        require(
PooledCreditLine/PooledCreditLine.sol:688:        require(
PooledCreditLine/PooledCreditLine.sol:692:        require(
PooledCreditLine/PooledCreditLine.sol:749:        require(creditLineVariables[_id].status == CreditLineStatus.REQUESTED, '!Requested');
PooledCreditLine/PooledCreditLine.sol:767:        require(getCreditLineStatus(_id) == CreditLineStatus.ACTIVE, '!Active');
PooledCreditLine/PooledCreditLine.sol:807:        require(_amount != 0, 'amount == 0');
PooledCreditLine/PooledCreditLine.sol:808:        require(block.timestamp >= creditLineConstants[_id].startsAt, "!started");
PooledCreditLine/PooledCreditLine.sol:809:        require(_amount <= calculateBorrowableAmount(_id), '>Borrowable');
PooledCreditLine/PooledCreditLine.sol:856:        require(currentStatus == CreditLineStatus.ACTIVE || currentStatus == CreditLineStatus.EXPIRED, '!(ACTIVE || EXPIRED)');
PooledCreditLine/PooledCreditLine.sol:902:        require(msg.sender == creditLineConstants[_id].borrower, '!Borrower');
PooledCreditLine/PooledCreditLine.sol:903:        require(creditLineVariables[_id].status == CreditLineStatus.ACTIVE, '!ACTIVE');
PooledCreditLine/PooledCreditLine.sol:904:        require(creditLineVariables[_id].principal == 0, 'Principal!=0');
PooledCreditLine/PooledCreditLine.sol:961:        require(_amount <= _withdrawableCollateral, '>Withdrawable');
PooledCreditLine/PooledCreditLine.sol:1044:        require(creditLineVariables[_id].principal != 0, 'CreditLine: cannot liquidate if principal is 0');
PooledCreditLine/PooledCreditLine.sol:1045:        require(currentStatus == CreditLineStatus.ACTIVE || currentStatus == CreditLineStatus.EXPIRED, '!(ACTIVE || EXPIRED)');
Verification/twitterVerifier.sol:92:        require(bytes(userData[msg.sender].twitterId).length == 0, 'User already exists');
Verification/twitterVerifier.sol:93:        require(twitterIdMap[_twitterId] == address(0), 'Signed message already used');
Verification/twitterVerifier.sol:94:        require(block.timestamp < _timestamp + 86400, 'Signed transaction expired');
Verification/twitterVerifier.sol:106:        require(hashAddressMap[digest] == address(0), 'Hash Already Used');
Verification/twitterVerifier.sol:110:        require(signer == signerAddress, 'Invalid signature');
Verification/twitterVerifier.sol:125:        require(bytes(_userdata).length != 0, 'User doesnt exists');
Verification/twitterVerifier.sol:137:        require(bytes(_userdata).length != 0, 'User does not exists');

Storage

Tighly pack storage variables

Here, struct LendingInfo can be tightly packed from:

File: LenderPool.sol
47:     struct LendingInfo { //@audit gas: can be tightly packed by moving "bool areTokensTransferable;" after an "address" type
48:         mapping(address => LenderInfo) lenders;
49:         uint256 startTime;
50:         address token;
51:         address collateralToken;
52:         uint256 borrowLimit;
53:         address verifier;
54:         address strategy; //@audit-info 20 bytes
55:         uint256 sharesHeld; //@audit-info 32 bytes
56:         uint256 borrowerInterestShares;
57:         uint256 borrowerInterestSharesWithdrawn;
58:         uint256 yieldInterestWithdrawnShares;
59:         uint256 collateralHeld; //@audit-info 32 bytes
60:         bool areTokensTransferable; //@audit-info 1 bytes (but taking a whole 32 bytes slot by itself)
61:     }

to

File: LenderPool.sol
     struct LendingInfo {
         mapping(address => LenderInfo) lenders;
         uint256 startTime;
         address token;
         address collateralToken;
         uint256 borrowLimit;
         address verifier;
         address strategy; //@audit-info 20 bytes
         bool areTokensTransferable; //@audit-info 1 bytes (taking the same slot as "address strategy")
         uint256 sharesHeld; //@audit-info 32 bytes
         uint256 borrowerInterestShares;
         uint256 borrowerInterestSharesWithdrawn;
         uint256 yieldInterestWithdrawnShares;
         uint256 collateralHeld; //@audit-info 32 bytes
     }

Which would save 1 storage slot.

Help the optimizer by saving a storage variable's reference instead of repeatedly fetching it

To help the optimizer, declare a storage type variable and use it instead of repeatedly fetching the reference in a map or an array.

The effect can be quite significant.

As an example, instead of repeatedly calling someMap[someIndex], save its reference like this: SomeStruct storage someStruct = someMap[someIndex]; and use it.

In LenderPool.sol, there is 1 practice that should change:

  • Instead of using creditLines[_id] (and creditLines[id]) directly, I suggest using a storage variable _creditLine after declaring it as such: LendingInfo storage _creditLine = creditLines[_id]; (and LendingInfo storage _creditLine = creditLines[id];, without the underscore). Instances include:
contracts/PooledCreditLine/LenderPool.sol:
  101:         creditLines[_id].startTime = block.timestamp.add(_collectionPeriod); 
  102:         creditLines[_id].token = _token; 
  103:         creditLines[_id].borrowLimit = _borrowLimit; 
  104:         creditLines[_id].verifier = _verifier; 
  105:         creditLines[_id].strategy = _strategy; 
  106:         creditLines[_id].areTokensTransferable = _areTokensTransferable; 
  115:         require(block.timestamp < creditLines[_id].startTime, '!Collecting'); 
  118:         uint256 _maxLent = creditLines[_id].borrowLimit; 
  125:         address _token = creditLines[_id].token; 
  135:         creditLines[_id].sharesHeld = SAVINGS_ACCOUNT.deposit(_token, creditLines[_id].strategy, address(this), _maxLent); 
  139:         delete creditLines[_id].startTime; 
  140:         delete creditLines[_id].borrowLimit; 
  144:         creditLines[_id].sharesHeld = creditLines[_id].sharesHeld.sub(_sharesBorrowed); 
  148:         creditLines[_id].sharesHeld = creditLines[_id].sharesHeld.add(_sharesRepaid); 
  149:         creditLines[_id].borrowerInterestShares = creditLines[_id].borrowerInterestShares.add(_interestShares); 
  159:         } else if(_creditLineStatus == uint256(CreditLineStatus.REQUESTED) && block.timestamp > creditLines[_id].startTime) {
  168:         uint256 _totalLiquidityWithdrawable = creditLines[_id].borrowLimit.sub(
  194:             block.timestamp > creditLines[_id].startTime 
  200:             address lentToken = creditLines[_id].token; 
  210:             address _strategy = creditLines[_id].strategy; 
  211:             address _lentToken = creditLines[_id].token; 
  214:             creditLines[_id].sharesHeld = creditLines[_id].sharesHeld.sub(_principalWithdrawable).sub(_interestWithdrawable); 
  232:         address _strategy = creditLines[_id].strategy; 
  233:         address _lentToken = creditLines[_id].token; 
  248:         uint256 _borrowerInterestShares = creditLines[_id].borrowerInterestShares; 
  254:                                             ).sub(creditLines[_id].lenders[_lender].borrowerInterestSharesWithdrawn); 
  257:             uint256 _notBorrowed = creditLines[_id].borrowLimit.sub(POOLED_CREDIT_LINE.getPrincipal(_id)); 
  259:             _totalInterestInShares = creditLines[_id].sharesHeld.sub(_notBorrowedInShares); 
  264:                                         .add(creditLines[_id].yieldInterestWithdrawnShares); 
  270:                                         ).sub(creditLines[_id].lenders[_lender].yieldInterestWithdrawnShares); 
  272:         creditLines[_id].lenders[_lender].yieldInterestWithdrawnShares += _yieldInterestForLender; 
  273:         creditLines[_id].lenders[_lender].borrowerInterestSharesWithdrawn += _borrowerInterestForLender; 
  274:         creditLines[_id].borrowerInterestSharesWithdrawn += _borrowerInterestForLender; 
  275:         creditLines[_id].yieldInterestWithdrawnShares += _yieldInterestForLender; 
  276:         creditLines[_id].sharesHeld -= (_yieldInterestForLender + _borrowerInterestForLender); 
  289:         uint256 _collateralLiquidated = creditLines[_id].collateralHeld; 
  295:             creditLines[_id].collateralHeld = creditLines[_id].collateralHeld.sub(_lenderCollateralShare); 
  297:             IERC20(creditLines[_id].collateralToken).safeTransfer(_lender, _lenderCollateralShare); 
  308:         creditLines[_id].collateralToken = _collateralToken; 
  309:         creditLines[_id].collateralHeld = _collateralLiquidated; 
  325:         uint256 yieldInterestOnTransferAmount = creditLines[id].lenders[from].yieldInterestWithdrawnShares.mul(amount).div(fromBalance);
  326:         uint256 borrowerInterestOnTransferAmount = creditLines[id].lenders[from].borrowerInterestSharesWithdrawn.mul(amount).div(fromBalance);
  329:             creditLines[id].lenders[from].borrowerInterestSharesWithdrawn
  330:                 =  creditLines[id].lenders[from].borrowerInterestSharesWithdrawn.sub(borrowerInterestOnTransferAmount);
  334:             creditLines[id].lenders[from].yieldInterestWithdrawnShares 
  335:                 =  creditLines[id].lenders[from].yieldInterestWithdrawnShares.sub(yieldInterestOnTransferAmount);
  340:                 creditLines[id].lenders[to].borrowerInterestSharesWithdrawn
  341:                         =  creditLines[id].lenders[to].borrowerInterestSharesWithdrawn.add(borrowerInterestOnTransferAmount);
  344:                 creditLines[id].lenders[to].yieldInterestWithdrawnShares 
  345:                         =  creditLines[id].lenders[to].yieldInterestWithdrawnShares.add(yieldInterestOnTransferAmount);
  361:             require(IPooledCreditLineVerifier(creditLines[id].verifier).isUser(to) || to == address(0), '!Verified && !burn');
  372:                 require(creditLines[id].areTokensTransferable, '!transferrable');

In PooledCreditLine.sol, there are 3 practices that should change:

  • Instead of using creditLineVariables[_id] directly, I suggest using a storage variable _creditLineVariable after declaring it as such: CreditLineVariables storage _creditLineVariable = creditLineVariables[_id];. Instances include:
contracts/PooledCreditLine/PooledCreditLine.sol:
   535:         return creditLineVariables[_id].principal;
   545:         CreditLineStatus currentStatus = creditLineVariables[_id].status;
   548:             creditLineVariables[_id].status = currentStatus;
   575:         uint256 _lastPrincipalUpdateTime = creditLineVariables[_id].lastPrincipalUpdateTime;
   587:         uint256 _interestAccrued = calculateInterest(creditLineVariables[_id].principal, creditLineConstants[_id].borrowRate, _timeElapsed);
   598:         uint256 _currentDebt = (creditLineVariables[_id].principal)
   599:             .add(creditLineVariables[_id].interestAccruedTillLastPrincipalUpdate)
   601:             .sub(creditLineVariables[_id].totalInterestRepaid);
   649:         uint256 _totalInterestAccrued = (creditLineVariables[_id].interestAccruedTillLastPrincipalUpdate).add(_newInterestAccured);
   650:         creditLineVariables[_id].interestAccruedTillLastPrincipalUpdate = _totalInterestAccrued;
   651:         creditLineVariables[_id].lastPrincipalUpdateTime = block.timestamp;
   652:         creditLineVariables[_id].principal = _updatedPrincipal;
   715:         creditLineVariables[_id].status = CreditLineStatus.REQUESTED;
   749:         require(creditLineVariables[_id].status == CreditLineStatus.REQUESTED, '!Requested');
   750:         creditLineVariables[_id].status = CreditLineStatus.ACTIVE;
   820:         updatePrincipal(_id, creditLineVariables[_id].principal.add(_borrowedAmount));
   859:         uint256 _totalInterestAccrued = (creditLineVariables[_id].interestAccruedTillLastPrincipalUpdate).add(
   862:         uint256 _interestToPay = _totalInterestAccrued.sub(creditLineVariables[_id].totalInterestRepaid);
   863:         uint256 _currentPrincipal = creditLineVariables[_id].principal;
   876:             creditLineVariables[_id].principal = _currentPrincipal.sub(_principalPaid);
   877:             creditLineVariables[_id].interestAccruedTillLastPrincipalUpdate = _totalInterestAccrued;
   878:             creditLineVariables[_id].lastPrincipalUpdateTime = block.timestamp;
   879:             creditLineVariables[_id].totalInterestRepaid = _totalInterestAccrued;
   881:             creditLineVariables[_id].totalInterestRepaid = creditLineVariables[_id].totalInterestRepaid.add(_amount);
   889:         if (creditLineVariables[_id].principal == 0) {
   891:                 creditLineVariables[_id].status = CreditLineStatus.CLOSED;
   903:         require(creditLineVariables[_id].status == CreditLineStatus.ACTIVE, '!ACTIVE');
   904:         require(creditLineVariables[_id].principal == 0, 'Principal!=0');
   905:         creditLineVariables[_id].status = CreditLineStatus.CLOSED;
  1044:         require(creditLineVariables[_id].principal != 0, 'CreditLine: cannot liquidate if principal is 0');
  1064:         creditLineVariables[_id].status = CreditLineStatus.LIQUIDATED;
  • Instead of using creditLineConstants[_id] directly, I suggest using a storage variable _creditLineConstant after declaring it as such: CreditLineConstants storage _creditLineConstant = creditLineConstants[_id];. Instances include:
contracts/PooledCreditLine/PooledCreditLine.sol:
   162:         require(creditLineConstants[_id].borrower == msg.sender, '!Borrower');
   546:         if (currentStatus == CreditLineStatus.ACTIVE && creditLineConstants[_id].endsAt <= block.timestamp) {
   578:         uint256 _endTime = creditLineConstants[_id].endsAt;
   579:         uint256 _penaltyRate = creditLineConstants[_id].gracePenaltyRate;
   587:         uint256 _interestAccrued = calculateInterest(creditLineVariables[_id].principal, creditLineConstants[_id].borrowRate, _timeElapsed);
   618:             creditLineConstants[_id].collateralAsset,
   619:             creditLineConstants[_id].borrowAsset
   626:         uint256 _collateralRatio = creditLineConstants[_id].idealCollateralRatio;
   636:         uint256 _borrowLimit = creditLineConstants[_id].borrowLimit;
   716:         creditLineConstants[_id].borrower = msg.sender;
   717:         creditLineConstants[_id].borrowLimit = _request.borrowLimit;
   718:         creditLineConstants[_id].idealCollateralRatio = _request.collateralRatio;
   719:         creditLineConstants[_id].borrowRate = _request.borrowRate;
   720:         creditLineConstants[_id].borrowAsset = _request.borrowAsset;
   721:         creditLineConstants[_id].collateralAsset = _request.collateralAsset;
   723:         creditLineConstants[_id].startsAt = block.timestamp + _request.collectionPeriod;
   724:         creditLineConstants[_id].endsAt = _endsAt;
   725:         creditLineConstants[_id].defaultsAt = _endsAt + _request.defaultGracePeriod;
   726:         creditLineConstants[_id].gracePenaltyRate = _request.gracePenaltyRate;
   727:         creditLineConstants[_id].lenderStrategy = _request.lenderStrategy;
   768:         address _collateralAsset = creditLineConstants[_id].collateralAsset;
   769:         address _strategy = creditLineConstants[_id].collateralStrategy;
   808:         require(block.timestamp >= creditLineConstants[_id].startsAt, "!started");
   810:         address _borrowAsset = creditLineConstants[_id].borrowAsset;
   815:         uint256 _sharesWithdrawn = _withdrawBorrowAmount(_borrowAsset, creditLineConstants[_id].lenderStrategy, _lender, _amount);
   835:         address _strategy = creditLineConstants[_id].lenderStrategy;
   836:         address _borrowAsset = creditLineConstants[_id].borrowAsset;
   885:         uint256 _repaidInterestShares = IYield(creditLineConstants[_id].lenderStrategy).getSharesForTokens(_interestPaid, creditLineConstants[_id].borrowAsset);
   902:         require(msg.sender == creditLineConstants[_id].borrower, '!Borrower');
   919:             creditLineConstants[_id].collateralAsset,
   920:             creditLineConstants[_id].borrowAsset
   938:         address _collateralAsset = creditLineConstants[_id].collateralAsset;
   939:         address _strategy = creditLineConstants[_id].collateralStrategy;
   962:         _transferCollateral(_id, creditLineConstants[_id].collateralAsset, _amount, _toSavingsAccount);
   980:         _transferCollateral(_id, creditLineConstants[_id].collateralAsset, _withdrawableCollateral, _toSavingsAccount);
   996:             creditLineConstants[_id].collateralAsset,
   997:             creditLineConstants[_id].borrowAsset
  1004:             .mul(creditLineConstants[_id].idealCollateralRatio)
  1021:         address _strategy = creditLineConstants[_id].collateralStrategy;
  1046:         address _collateralAsset = creditLineConstants[_id].collateralAsset;
  1051:         if (currentCollateralRatio < creditLineConstants[_id].idealCollateralRatio) {
  1053:         } else if (block.timestamp >= creditLineConstants[_id].defaultsAt) {
  1055:             address _borrowAsset = creditLineConstants[_id].borrowAsset;
  1080:         address _collateralAsset = creditLineConstants[_id].collateralAsset;
  1081:         address _borrowAsset = creditLineConstants[_id].borrowAsset;
  • Instead of using collateralShareInStrategy[_id] directly, I suggest using a storage variable _collateralShareInStrategy after declaring it as such: mapping(uint => uint256) storage _collateralShareInStrategy = collateralShareInStrategy[_id];. Instances include:
contracts/PooledCreditLine/PooledCreditLine.sol:
   781:         collateralShareInStrategy[_id][_strategy] = collateralShareInStrategy[_id][_strategy].add(_sharesDeposited);
   941:         uint256 _collateralShares = collateralShareInStrategy[_id][_strategy];
  1025:         collateralShareInStrategy[_id][_strategy] = collateralShareInStrategy[_id][_strategy].sub(

Arithmetics

Do not use SafeMath functions on arithmetics operations that can't underflow/overflow

When an overflow or an underflow isn't possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by not using the SafeMath library. This would avoid making an unnecessary check.

Instances include (see @audit tags):

contracts/PooledCreditLine/LenderPool.sol:
  122          if (_totalLent.add(_amount) > _maxLent) {
  123:             _amountToLend = _maxLent.sub(_totalLent); //@audit gas: safemath not needed here due to L122

contracts/PooledCreditLine/PooledCreditLine.sol:
   581          if (_lastPrincipalUpdateTime <= _endTime && block.timestamp > _endTime) {
   582:             penalityInterest = (block.timestamp.sub(_endTime)).mul(_penaltyRate).div(SCALING_FACTOR); //@audit gas: safemath not needed here due to L581

   822          uint256 _protocolFee = _borrowedAmount.mul(protocolFeeFraction).div(SCALING_FACTOR);
   823:         _borrowedAmount = _borrowedAmount.sub(_protocolFee); //@audit gas: safemath not needed here due to L822 (_protocolFee <= _borrowedAmount due to bounded protocolFeeFraction)

   874          if (_amount > _interestToPay) {
   875:             _principalPaid = _amount.sub(_interestToPay); //@audit gas: safemath not needed here due to L874

  1009          if (_collateralNeeded >= _totalCollateralTokens) {
  1010              return 0;
  1011          }
  1012:         return _totalCollateralTokens.sub(_collateralNeeded); //@audit gas: safemath not needed here due to L1009-L1010

For-Loops

An array's length should be cached to save gas in for-loops

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration.

Here, I suggest storing the array's length in a variable before the for-loop, and use it instead:

PooledCreditLine/LenderPool.sol:359:        for (uint256 i; i < ids.length; ++i) {
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Mar 29, 2022
code423n4 added a commit that referenced this issue Mar 29, 2022
@ritik99
Copy link
Collaborator

ritik99 commented Apr 12, 2022

All suggestions are valid/acknowledged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization)
Projects
None yet
Development

No branches or pull requests

2 participants