Beedle - Oracle free perpetual lending

Summary

During the audit review of Lender.sol smart contract, I found six for loops which are not as gas efficient as they could be. This can be fixed by creating local variables which store the lengths of borrows.length, loanIds.length, loanIds.length, loanIds.length, loanIds.length and refinances.length arrays also using unchecked would reduce gas cost as well.

Vulnerability Details

In the Lender.sol smart contract on lines: 223, 294, 361, 441, 553 and 597. All these functions have a for loop which every time it loops it calls the length of a given array. This means will have multiple SLOADs instead of having one SLOAD for the whole array. By not using unchecked you are costing the contract more gas as without the code will check for underflow/overflow error cases every time the loop runs. You do not need to spend the extra gas checking for underflow/overflow error cases as the for loop conditional statements stop the variable i from overflow because of the conditions i < length where length is defined as uint256. The maximum value i can reach is max(uint)-1. Thus, incrementing i inside unchecked block is safe and consumes lesser gas.

Impact

The impact is that in the Lending.sol smart contract the following functions will cost more gas: borrow(), repay(), giveLoan(), startAuction(), seizeLoan() and refiance(). These functions will have to be called more than once so it is critical that they are as gas efficient as possible to save the users gas and improve the experience of using this lending protocol if the functions cost to much gas then users may be less likely to take part in the smart contract.

Tools Used

Foundry

Recommendations

Create a local variable for each for loop which stores borrows.length, loanIds.length, loanIds.length, loanIds.length, loanIds.length and refinances.length: uint length = borrows.length, uint length = loanIds.length and uint length = refiances.length . Then use these variables in the for loop conditional, for example replace i < borrow.length with i < length .This way the for loop can call this variable as one SLOAD and save gas instead of having to read the length of the array every loop which would cost more gas and require multiple SLOADs. Also put the code of all the for loops inside of an unchecked brackets: For example in borrow() inside the for loop would be written like this: unchecked{bytes32 poolId = borrows[i].poolId; uint256 debt = borrows[i].debt; uint256 collateral = borrows[i].collateral; // get the pool info Pool memory pool = pools[poolId]; // make sure the pool exists if (pool.lender == address(0)) revert PoolConfig(); // validate the loan if (debt < pool.minLoanSize) revert LoanTooSmall(); if (debt > pool.poolBalance) revert LoanTooLarge(); if (collateral == 0) revert ZeroCollateral(); // make sure the user isn't borrowing too much uint256 loanRatio = (debt * 10 ** 18) / collateral; if (loanRatio > pool.maxLoanRatio) revert RatioTooHigh(); // create the loan Loan memory loan = Loan({ lender: pool.lender, borrower: msg.sender, loanToken: pool.loanToken, collateralToken: pool.collateralToken, debt: debt, collateral: collateral, interestRate: pool.interestRate, startTimestamp: block.timestamp, auctionStartTimestamp: type(uint256).max, auctionLength: pool.auctionLength }); // update the pool balance _updatePoolBalance(poolId, pools[poolId].poolBalance - debt); pools[poolId].outstandingLoans += debt; // calculate the fees uint256 fees = (debt * borrowerFee) / 10000; // transfer fees IERC20(loan.loanToken).transfer(feeReceiver, fees); // transfer the loan tokens from the pool to the borrower IERC20(loan.loanToken).transfer(msg.sender, debt - fees); // transfer the collateral tokens from the borrower to the contract IERC20(loan.collateralToken).transferFrom( msg.sender, address(this), collateral ); loans.push(loan); emit Borrowed( msg.sender, pool.lender, loans.length - 1, debt, collateral, pool.interestRate, block.timestamp );}