Gas Optimizations

[code423n4]

Overview

Risk Rating	Number of issues
Gas Issues	9

Table of Contents:

• 1. Unchecking arithmetics operations that can't underflow/overflow
• 2. Caching storage values in memory
• 3. Cheap Contract Deployment Through Clones
• 4. Use calldata instead of memory
• 5. <array>.length should not be looked up in every loop of a for-loop
• 6. ++i costs less gas compared to i++ or i += 1 (same for --i vs i-- or i -= 1)
• 7. It costs more gas to initialize variables with their default value than letting the default value be applied
• 8. Some variables should be immutable
• 9. Use Custom Errors instead of Revert Strings to save Gas

1. Unchecking arithmetics operations that can't underflow/overflow

Solidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. 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 using an unchecked block: https://docs.soliditylang.org/en/v0.8.10/control-structures.html#checked-or-unchecked-arithmetic

By keeping in mind the following function (calculateFee) which makes it so that a - calculateFee(a) > 0:

File: Lender.sol
661:     function calculateFee(uint256 a) internal view returns (uint256) {
662:         return feenominator > 0 ? a / feenominator : 0;
663:     }

Consider wrapping with an unchecked block here:

• File: Lender.sol

- 219:             uint256 returned = yield(u, y, a - calculateFee(a), address(this));
+ 219:             unchecked { uint256 returned = yield(u, y, a - calculateFee(a), address(this)); }

- 229:            uint256 returned = yield(u, y, a - calculateFee(a), msg.sender);
+ 229:            unchecked { uint256 returned = yield(u, y, a - calculateFee(a), msg.sender); }

- 400:        uint256 returned = IPendle(pendleAddr).swapExactTokensForTokens(a - fee, r, path, address(this), d)[0];
+ 400:       unchecked { uint256 returned = IPendle(pendleAddr).swapExactTokensForTokens(a - fee, r, path, address(this), d)[0]; }

- 502:        uint256 lent = a - fee;
+ 502:        unchecked { uint256 lent = a - fee; }

- 557:        uint256 lent = a - fee;
+ 557:        unchecked { uint256 lent = a - fee; }

- 605:        uint256 returned = token.deposit(a - fee, address(this));
+ 605:        unchecked { uint256 returned = token.deposit(a - fee, address(this)); }

2. Caching storage values in memory

The code can be optimized by minimizing the number of SLOADs.

SLOADs are expensive (100 gas after the 1st one) compared to MLOADs/MSTOREs (3 gas each). Storage values read multiple times should instead be cached in memory the first time (costing 1 SLOAD) and then read from this cache to avoid multiple SLOADs.

• feenominator: https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/lender/Lender.sol#L662

File: Lender.sol
661:     function calculateFee(uint256 a) internal view returns (uint256) {
662:         return feenominator > 0 ? a / feenominator : 0;
663:     }

• lender: https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/redeemer/Redeemer.sol#L134-L136

File: Redeemer.sol
134:             uint256 amount = IERC20(principal).balanceOf(lender);
135:             // Transfer the principal token from the lender contract to here
136:             Safe.transferFrom(IERC20(u), lender, address(this), amount);

• lender: https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/redeemer/Redeemer.sol#L177-L180

File: Redeemer.sol
177:         uint256 amount = IERC20(principal).balanceOf(lender);
178: 
179:         // Transfer the principal token from the lender contract to here
180:         Safe.transferFrom(IERC20(principal), lender, address(this), amount);

• marketPlace: https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/redeemer/Redeemer.sol#L164-L187

File: Redeemer.sol
164:         address principal = IMarketPlace(marketPlace).markets(u, m, p);
...
187:             IElementToken(principal).withdrawPrincipal(amount, marketPlace);

• lender: https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/redeemer/Redeemer.sol#L221-L224

File: Redeemer.sol
221:         uint256 amount = token.balanceOf(lender);
222: 
223:         // Transfer the user's tokens to the redeem contract
224:         Safe.transferFrom(token, lender, address(this), amount);

• lender: https://github.com/code-423n4/2022-06-illuminate/blob/92cbb0724e594ce025d6b6ed050d3548a38c264b/redeemer/Redeemer.sol#L256-L259

File: Redeemer.sol
256:         uint256 amount = token.balanceOf(lender);
257: 
258:         // Transfer the user's tokens to the redeem contract
259:         Safe.transferFrom(token, lender, address(this), amount);

3. Cheap Contract Deployment Through Clones

marketplace/MarketPlace.sol:80:        address iToken = address(new ERC5095(u, m, redeemer, lender, n, s, d));

There's a way to save a significant amount of gas on deployment using Clones: https://www.youtube.com/watch?v=3Mw-pMmJ7TA .

This is a solution that was adopted, as an example, by Porter Finance. They realized that deploying using clones was 10x cheaper:

• Research clone factory to deploy erc20 bond contracts porter-finance/v1-core#15 (comment)
• Bond factory porter-finance/v1-core#34

Consider applying a similar pattern.

4. Use calldata instead of memory

When a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memoryindex. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution. Structs have the same overhead as an array of length one

When arguments are read-only on external functions, the data location should be calldata:

marketplace/MarketPlace.sol:70:        address[8] memory t,

5. <array>.length should not be looked up in every loop of a for-loop

Reading array length at each iteration of the loop consumes more gas than necessary.

In the best case scenario (length read on a memory variable), caching the array length in the stack saves around 3 gas per iteration.
In the worst case scenario (external calls at each iteration), the amount of gas wasted can be massive.

Here, Consider storing the array's length in a variable before the for-loop, and use this new variable instead:

lender/Lender.sol:265:            for (uint256 i = 0; i < o.length; ) {

6. ++i costs less gas compared to i++ or i += 1 (same for --i vs i-- or i -= 1)

Pre-increments and pre-decrements are cheaper.

For a uint256 i variable, the following is true with the Optimizer enabled at 10k:

Increment:

• i += 1 is the most expensive form
• i++ costs 6 gas less than i += 1
• ++i costs 5 gas less than i++ (11 gas less than i += 1)

Decrement:

• i -= 1 is the most expensive form
• i-- costs 11 gas less than i -= 1
• --i costs 5 gas less than i-- (16 gas less than i -= 1)

Note that post-increments (or post-decrements) return the old value before incrementing or decrementing, hence the name post-increment:

uint i = 1;  
uint j = 2;
require(j == i++, "This will be false as i is incremented after the comparison");

However, pre-increments (or pre-decrements) return the new value:

uint i = 1;  
uint j = 2;
require(j == ++i, "This will be true as i is incremented before the comparison");

In the pre-increment case, the compiler has to create a temporary variable (when used) for returning 1 instead of 2.

Affected code:

lender/Lender.sol:96:                i++;
lender/Lender.sol:120:                i++;
lender/Lender.sol:283:                    i++;

Consider using pre-increments and pre-decrements where they are relevant (meaning: not where post-increments/decrements logic are relevant).

7. It costs more gas to initialize variables with their default value than letting the default value be applied

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address...). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

As an example: for (uint256 i = 0; i < numIterations; ++i) { should be replaced with for (uint256 i; i < numIterations; ++i) {

Affected code:

lender/Lender.sol:265:            for (uint256 i = 0; i < o.length; ) {

Consider removing explicit initializations for default values.

8. Some variables should be immutable

These variables are only set in the constructor and are never edited after that:

lender/Lender.sol:26:    address public admin;
lender/Lender.sol:28:    address public marketPlace;
lender/Lender.sol:33:    address public swivelAddr;
marketplace/MarketPlace.sol:41:    address public admin;
redeemer/Redeemer.sol:19:    address public admin;
redeemer/Redeemer.sol:21:    address public marketPlace;
redeemer/Redeemer.sol:23:    address public lender;
redeemer/Redeemer.sol:27:    address public swivelAddr;
redeemer/Redeemer.sol:33:    address public apwineAddr;

Consider marking them as immutable, as it would avoid the expensive storage-writing operation (around 20 000 gas)

9. Use Custom Errors instead of Revert Strings to save Gas

As this is the case in almost the whole solution, consider also using custom errors here:

lender/Lender.sol:691:        require (when != 0, 'no withdrawal scheduled');
lender/Lender.sol:693:        require (block.timestamp >= when, 'withdrawal still on hold');