Gas Optimizations

[code423n4]

Summary

Gas Optimizations

	Issue	Instances
1	Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate	2
2	State variables only set in the constructor should be declared immutable	1
3	Using calldata instead of memory for read-only arguments in external functions saves gas	1
4	Use EIP-1167 minimal proxies for 10x cheaper contract instantiation	1
5	State variables should be cached in stack variables rather than re-reading them from storage	9
6	<array>.length should not be looked up in every loop of a for-loop	1
7	Using bools for storage incurs overhead	1
8	It costs more gas to initialize non-constant/non-immutable variables to zero than to let the default of zero be applied	1
9	++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)	3
10	Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead	31
11	Using private rather than public for constants, saves gas	1
12	Use custom errors rather than revert()/require() strings to save gas	2

Total: 54 instances over 12 issues

Gas Optimizations

1. Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.

There are 2 instances of this issue:

File: lender/Lender.sol   #1

43        mapping(address => uint256) public fees;
44        /// @notice maps a token address to a point in time, a hold, after which a withdrawal can be made
45:       mapping (address => uint256) public withdrawals;

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L43-L45

File: marketplace/MarketPlace.sol   #2

35        mapping(address => mapping(uint256 => address[9])) public markets;
36    
37        /// @notice pools map markets to their respective YieldSpace pools for the MetaPrincipal token
38:       mapping(address => mapping(uint256 => address)) public pools;

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/marketplace/MarketPlace.sol#L35-L38

2. State variables only set in the constructor should be declared immutable

Avoids a Gsset (20000 gas) in the constructor, and replaces each Gwarmacces (100 gas) with a PUSH32 (3 gas).

There is 1 instance of this issue:

File: redeemer/Redeemer.sol   #1

33:       address public apwineAddr;

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/redeemer/Redeemer.sol#L33

3. Using calldata instead of memory for read-only arguments in external functions saves gas

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 memory index. 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.

If the array is passed to an internal function which passes the array to another internal function where the array is modified and therefore memory is used in the external call, it's still more gass-efficient to use calldata when the external function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one

There is 1 instance of this issue:

File: marketplace/MarketPlace.sol   #1

70:           address[8] memory t,

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/marketplace/MarketPlace.sol#L70

4. Use EIP-1167 minimal proxies for 10x cheaper contract instantiation

When new contracts have to be instantiated frequently, it's much cheaper for it to be done via minimal proxies. The only downside is that they rely on delegatecall() calls for every function, which adds an overhead of ~800 gas, but this is multiple orders of manitude less than the amount saved during deployment

There is 1 instance of this issue:

File: marketplace/MarketPlace.sol   #1

80:          address iToken = address(new ERC5095(u, m, redeemer, lender, n, s, d));

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/marketplace/MarketPlace.sol#L80

5. State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second+ access of a state variable within a function. Caching of a state variable replace each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.

There are 9 instances of this issue:

File: lender/Lender.sol

/// @audit marketPlace on line 146
147:              revert Exists(marketPlace);

/// @audit feenominator on line 681
681:          return feenominator > 0 ? a / feenominator : 0;

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L147

File: redeemer/Redeemer.sol

/// @audit marketPlace on line 164
187:              IElementToken(principal).withdrawPrincipal(amount, marketPlace);

/// @audit marketPlace on line 283
281:      ) public authorized(IMarketPlace(marketPlace).markets(u, m, 0)) returns (bool) {

/// @audit lender on line 128
134:              uint256 amount = IERC20(principal).balanceOf(lender);

/// @audit lender on line 134
136:              Safe.transferFrom(IERC20(u), lender, address(this), amount);

/// @audit lender on line 177
180:          Safe.transferFrom(IERC20(principal), lender, address(this), amount);

/// @audit lender on line 221
224:          Safe.transferFrom(token, lender, address(this), amount);

/// @audit lender on line 256
259:          Safe.transferFrom(token, lender, address(this), amount);

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/redeemer/Redeemer.sol#L187

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

The overheads outlined below are PER LOOP, excluding the first loop

• storage arrays incur a Gwarmaccess (100 gas)
• memory arrays use MLOAD (3 gas)
• calldata arrays use CALLDATALOAD (3 gas)

Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset

There is 1 instance of this issue:

File: lender/Lender.sol   #1

265:              for (uint256 i = 0; i < o.length; ) {

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L265

7. Using bools for storage incurs overhead

    // Booleans are more expensive than uint256 or any type that takes up a full
    // word because each write operation emits an extra SLOAD to first read the
    // slot's contents, replace the bits taken up by the boolean, and then write
    // back. This is the compiler's defense against contract upgrades and
    // pointer aliasing, and it cannot be disabled.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27
Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from 'false' to 'true', after having been 'true' in the past

There is 1 instance of this issue:

File: lender/Lender.sol   #1

30:       mapping(uint8 => bool) public paused;

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L30

8. It costs more gas to initialize non-constant/non-immutable variables to zero than to let the default of zero be applied

Not overwriting the default for stack variables saves 8 gas. Storage and memory variables have larger savings

There is 1 instance of this issue:

File: lender/Lender.sol   #1

265:              for (uint256 i = 0; i < o.length; ) {

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L265

9. ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

Saves 5 gas per loop

There are 3 instances of this issue:

File: lender/Lender.sol   #1

96:                   i++;

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L96

File: lender/Lender.sol   #2

120:                  i++;

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L120

File: lender/Lender.sol   #3

289:                      i++;

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L289

10. Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.

https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html
Use a larger size then downcast where needed

There are 31 instances of this issue:

File: lender/Lender.sol

48:       event Lend(uint8 principal, address indexed underlying, uint256 indexed maturity, uint256 returned);

50:       event Mint(uint8 principal, address indexed underlying, uint256 indexed maturity, uint256 amount);

87:           for (uint8 i; i < 9; ) {

168:          uint8 p,

193:          uint8 p,

248:          uint8 p,

318:          uint8 p,

378:          uint8 p,

434:          uint8 p,

487:          uint8 p,

490:          uint128 a,

546:          uint8 p,

598:          uint8 p,

648:          uint128 returned = IYield(y).sellBasePreview(Cast.u128(a));

734:      function pause(uint8 p, bool b) external authorized(admin) returns (bool) {

750:      modifier unpaused(uint8 p) {

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L48

File: marketplace/MarketPlace.sol

73:           uint8 d

98:       function setPrincipal(uint8 p, address u, uint256 m, address a) external authorized(admin) returns (bool) {

139:          uint128 a

140:      ) external returns (uint128) {

154:          uint128 a

155:      ) external returns (uint128) {

169:          uint128 a

170:      ) external returns (uint128) {

184:          uint128 a

185:      ) external returns (uint128) {

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/marketplace/MarketPlace.sol#L73

File: redeemer/Redeemer.sol

36:       event Redeem(uint8 principal, address indexed underlying, uint256 indexed maturity, uint256 amount);

108:          uint8 p,

159:          uint8 p,

207:          uint8 p,

241:          uint8 p,

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/redeemer/Redeemer.sol#L36

11. Using private rather than public for constants, saves gas

If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table

There is 1 instance of this issue:

File: lender/Lender.sol   #1

23:       uint256 constant public HOLD = 3 days;

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L23

12. Use custom errors rather than revert()/require() strings to save gas

Custom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they're hitby avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas

There are 2 instances of this issue:

File: lender/Lender.sol   #1

710:          require (when != 0, 'no withdrawal scheduled');

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L710

File: lender/Lender.sol   #2

712:          require (block.timestamp >= when, 'withdrawal still on hold');

https://github.com/code-423n4/2022-06-illuminate/blob/912be2a90ded4a557f121fe565d12ec48d0c4684/lender/Lender.sol#L712