Platform: Code4rena
Start Date: 21/06/2022
Pot Size: $55,000 USDC
Total HM: 29
Participants: 88
Period: 5 days
Judge: gzeon
Total Solo HM: 7
Id: 134
League: ETH
Rank: 85/88
Findings: 1
Award: $62.46
๐ Selected for report: 0
๐ Solo Findings: 0
๐ Selected for report: BowTiedWardens
Also found by: 0v3rf10w, 0x1f8b, 0x29A, 0xKitsune, 0xNazgul, 0xf15ers, 0xkatana, 0xkowloon, Bnke0x0, ElKu, Fitraldys, Funen, GalloDaSballo, IllIllI, JC, Kaiziron, Lambda, MadWookie, Noah3o6, Nyamcil, RoiEvenHaim, TomJ, Tomio, UnusualTurtle, Waze, _Adam, ajtra, asutorufos, bardamu, c3phas, catchup, datapunk, defsec, delfin454000, fatherOfBlocks, grGred, hake, hansfriese, hyh, ignacio, joestakey, kebabsec, ladboy233, oyc_109, pashov, poirots, rfa, robee, sach1r0, samruna, sashik_eth, simon135, slywaters, z3s, zer0dot
62.4602 USDC - $62.46
Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.
The solidity compiler will always read the length of the array during each iteration. That is,
1.if it is a storage array, this is an extra sload operation (100 additional extra gas (EIP-2929 2) for each iteration except for the first), 2.if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first), 3.if it is a calldata array, this is an extra calldataload operation (3 additional gas for each iteration except for the first)
This extra costs can be avoided by caching the array length (in stack): When reading the length of an array, sload or mload or calldataload operation is only called once and subsequently replaced by a cheap dupN instruction. Even though mload , calldataload and dupN have the same gas cost, mload and calldataload needs an additional dupN to put the offset in the stack, i.e., an extra 3 gas. which brings this to 6 gas
Here, I suggest storing the arrayโs length in a variable before the for-loop, and use it instead:
File: Lender.sol line 265
for (uint256 i = 0; i < o.length; ) {
The above should be modified to
uint256 length = o.length; for (uint256 i = 0; i < length; ) {
Something similar to my proposal has been implemented on line 108
++i costs less gas compared to i++ or i += 1 for unsigned integer, as pre-increment is cheaper (about 5 gas per iteration). This statement is true even with the optimizer enabled.
i++ increments i and returns the initial value of i. Which means:
uint i = 1; i++; // == 1 but i == 2
But ++i returns the actual incremented value:
uint i = 1; ++i; // == 2 and i == 2 too, so no need for a temporary variable
In the first case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2
Instances include:
File: Lender.sol line 96
unchecked { i++; }
The above should be modified to
unchecked { ++i; }
File: Lender.sol line 120
unchecked { i++; }
File: Lender.sol line 289
unchecked { i++; }
Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met)
see Source
Custom errors have been used on most of the contracts and functions reviewed apart from the following places.
File: Lender.sol line 710
require (when != 0, 'no withdrawal scheduled');
File: Lender.sol line 712
require (block.timestamp >= when, 'withdrawal still on hold');