20,000 USDC
View results
Submission Details
Severity: gas
Valid

The `for loops` inside the borrow(), repay(), giveLoan() & startAuction() functions in Lender contract are probably gas-guzzlers

  1. The for loops inside both the borrow() and repay() functions in the Lender.sol contract are probably gas-guzzlers:

https://github.com/Cyfrin/2023-07-beedle/blob/d9718f2ca6521756624c017c90f3d4d4e80da90c/src/Lender.sol#L233-L286
https://github.com/Cyfrin/2023-07-beedle/blob/d9718f2ca6521756624c017c90f3d4d4e80da90c/src/Lender.sol#L292-L293

The functions allows the borrower to open multiple debt positions (borrows) or do multiple repayments at once. However, there are several potential gas-related issues in these two functions, which I'll explain below:

  • Looping over multiple borrows/repayments: The functions iterate over the borrows/repayments array and processes each borrow/repayment request one by one. If the borrows/repayments array is large, it could lead to high gas consumption, potentially causing transactions to exceed the block gas limit.

  • ERC-20 Token Transfers: The functions perform multiple ERC-20 token transfers, which can be gas-intensive operations, especially if the number of borrows/repayments is large. Performing token transfers within a loop can increase the overall gas cost.

  • Storage Updates: Inside the loop, the functions update storage values for each borrow/repayment. Writing to storage is more expensive than reading from storage. When this update is combined with looping, it can have a significant impact on gas consumption.

  • Multiple Emit Calls: The function emits an event for each borrow/repayment. Emitting events can consume additional gas, especially when done repeatedly inside a loop.

POTENTIAL RISKS:

  • nightmare:
    the borrower fails to repay their loans in time, before liquidation is triggered, due to the repay() function running out of gas due to too many repayment loops, and effectively DoS borrower's ability to prevent liquidations in certain specific cases/scenarios. (as an interesting side note, could this be another possible attack vector by rogue lenders/liquidators, where the borrower(s) is the target?...)

  • bad:
    high likelihood of quick DoS of borrow()/repay() function when array of borrows/repayments is too large, and too large is probably not that large and can probably happen very quickly due to the high number of EVM operations inside the for loop, including at least 3 token transfers. Borrowers wont be able to borrow/repay from/to more than a few pools at a time before the current call runs out of gas and reverts.

  • low:
    borrowers might pay exceedingly high transaction/gas fees for every borrow()/repay() function call.

RECOMMENDATIONS:

At the very least, strongly advisable to implement the following approach:

Instead of doing:

function borrow(Borrow[] calldata borrows) public {
    for (uint256 i = 0; i < borrows.length; i++) {  

Rather implement it this way:

function borrow(Borrow[] calldata borrows) public {
		uint256 borrowsLength = borrows.length;
    for (uint256 i; i < borrowsLength; ) {
    		//...
    		// for loop logic
    		//...
    		unchecked {
    				i++;
    		}   
    }
}

Same applies to repay() function:

function repay(uint256[] calldata loanIds) public {
    for (uint256 i = 0; i < loanIds.length; i++) {

Rather implement it this way:

function repay(uint256[] calldata loanIds) public {
		uint256 loanIdsLength = loanIds.length;
    for (uint256 i; i < loanIdsLength; ) {
    		//...
    		// for loop logic
    		//...
    		unchecked {
    				i++;
    		} 
    }
}

Same applies to giveLoan() & startAuction() functions, as per above:
https://github.com/Cyfrin/2023-07-beedle/blob/d9718f2ca6521756624c017c90f3d4d4e80da90c/src/Lender.sol#L359
https://github.com/Cyfrin/2023-07-beedle/blob/d9718f2ca6521756624c017c90f3d4d4e80da90c/src/Lender.sol#L437-L438

function giveLoan(
    uint256[] calldata loanIds,
    bytes32[] calldata poolIds
) external {
	uint256 loanIdsLength = loanIds.length;
    for (uint256 i; i < loanIdsLength; ) {
        //...
    		// for loop logic
    		//...
    		unchecked {
    				i++;
    		} 
    }
}

function startAuction(uint256[] calldata loanIds) public {
	uint256 loanIdsLength = loanIds.length;
    for (uint256 i; i < loanIdsLength; ) {
        //...
    		// for loop logic
    		//...
    		unchecked {
    				i++;
    		} 
    }
}

Also, for borrow():

        if (debt < pool.minLoanSize) revert LoanTooSmall();
        if (debt > pool.poolBalance) revert LoanTooLarge();
        if (collateral == 0) revert ZeroCollateral();

Can be gas optimized as follows:

					if (collateral == 0) revert ZeroCollateral();
        if (debt < pool.minLoanSize) revert LoanTooSmall();
        if (debt > pool.poolBalance) revert LoanTooLarge();

Additionally, to mitigate these gas-related issues, consider the following optimizations:

  • Batching: implement a max limit to the total for loop iterations, i.e. limit the number of borrows/repayments that can be processed in a single call to avoid exceeding the block gas limit. Probably not viable to limit the size of the borrows/repayments array, so the above is probably the better approach.

  • Gasless Transactions: Consider implementing a gasless transaction mechanism, such as using meta-transactions or gas station networks, to allow users to interact with the contract without paying gas fees directly. This can shift the gas burden from the end-users to relayers or sponsors.

  • Gas Token: Explore the usage of gas tokens to refund gas costs. Gas tokens can be minted during low gas price periods and burned during high gas price periods to offset some gas costs.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.