Illuminate contest - slywaters's results

Vulnerability Report - Low and Non-Critical Risk Findings

Developer notes should be removed from production code

Comments in production code should not contain developer discussion or notes about known bugs or problems. These issues should be tracked elsewhere and resolved before being deployed.

Comments of this kind can also indicate potential avenues of attack for an adversary.

The following lines are affected:

2022-06-illuminate/lender/Cast.sol:9:        require(n <= type(uint128).max, ''); // TODO err msgs
2022-06-illuminate/lender/Element.sol:15:    // TODO audit structure / names / order-of-members etc...
2022-06-illuminate/lender/Element.sol:8:    // TODO are these established element names? kind? not type? etc...
2022-06-illuminate/lender/Element.sol:9:    // TODO In, Out vs GIVEN_IN, GIVEN_OUT. If those names are needed they should be GivenIn etc...
2022-06-illuminate/lender/Safe.sol:4:// TODO audit for which methods are needed
2022-06-illuminate/redeemer/Safe.sol:4:// TODO audit for which methods are needed

Gas Report

Variables do not need to be initialized with 'empty' values such as 0, false, or address(0)

Uninitialized variables by default contain a value equivalent to 0: uints are initialized to 0; bools to false; addresses to address(0).

Explicitly assigning these values to variables when they are declared increases gas costs while providing no funciton.

e.g. change this code:

uint256 var = 0;

to

uint256 var;

For more information, please consult the following resources:

Tips and Tricks to Save Gas and Reduce Bytecode Size

The following lines of code are affected:

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

Replace postfix increment (var++) with prefix increment (++var)

Using ++i costs less gas than using i++. In the context of a for-loop, gas is saved on each iteration.

The following lines of code are affected:

2022-06-illuminate/lender/Lender.sol:120:                i++;
2022-06-illuminate/lender/Lender.sol:283:                    i++;
2022-06-illuminate/lender/Lender.sol:96:                i++;

Array length can be cached

In the context of a for-loop that iterates over an array, it costs less gas to cache the array's length in a variable and read from this variable rather than use the arrays .length property. Reading the .length property for on the array will cause a recalculation of the array's length on each iteration of the loop which is a more expensive operation than reading from a stack variable.

For example, the following code:

for (uint i; i < arr.length; ++i) {
	// ...
}

should be changed to:

uint length = arr.length;
for (uint i; i < length; ++i) {
	// ...
}

Note that in the second case, the length of the array must not change during the loop's execution.

For more information, see the following resource:

Solidity gas optimizations

The following lines of code are affected:

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

Use strict less-than/greather-than comparisons rather than less-than-or-equal/greater-than-or-equal

When compiled, Solidity code using the >= or <= comparison operators in fact executes two separate checks: one for 'is-equal-to' and a second for 'is-greater-than/is-less-than'. By contrast, using > or < performs only one check. Therefore code that is written to use strict comparison operators is more gas-efficient.

If this change is applied, be sure to update the relevant variables being evaluated. For clarity, it is also advised to rename the variables to make this change explicit, e.g. renaming a variable from MINIMUM to MINIMUM_PLUS_ONE.

The following lines are affected:

2022-06-illuminate/lender/Cast.sol:9:        require(n <= type(uint128).max, ''); / TODO err msgs
2022-06-illuminate/lender/Lender.sol:693:        require (block.timestamp >= when, 'withdrawal still on hold');
2022-06-illuminate/marketplace/ERC20.sol:116:        require(_balanceOf[src] >= wad, "ERC20: Insufficient balance");
2022-06-illuminate/marketplace/ERC20.sol:152:                require(allowed >= wad, "ERC20: Insufficient approval");
2022-06-illuminate/marketplace/ERC20.sol:189:            require(_balanceOf[src] >= wad, "ERC20: Insufficient balance");
2022-06-illuminate/marketplace/ERC20Permit.sol:56:        require(deadline >= block.timestamp, "ERC20Permit: expired deadline");
2022-06-illuminate/marketplace/ERC5095.sol:100:            require(_allowance[holder][msg.sender] >= underlyingAmount, 'not enough approvals');
2022-06-illuminate/marketplace/ERC5095.sol:116:            require(_allowance[holder][msg.sender] >= underlyingAmount, 'not enough approvals');

Pragma statement should be at least 0.8.4

The pragma declaration allows for Solidity versions less than version 0.8.4. Several gas optimization features have been introduced in versions of Solidity between 0.8.0 and 0.8.4, including:

• The "low-level inliner" (version 0.8.2)
• Improvements to the optimizer (version 0.8.3)
• Custom errors (version 0.8.4)

Please note that Solidity version 0.8.9 contains bugfixes in addition to these gas improvements and that if possible it is advised to use versions greater than or equal to 0.8.9 for additional benefit.

For more information consult the following resources:

• Saving Gas with Simple Inlining
• Solidity 0.8.3 release announcement: Optimizer improvements
• Solidity docs: errors and the revert statement

The following pragma statements should be updated:

2022-06-illuminate/marketplace/ERC20.sol:3:pragma solidity ^0.8.0;
2022-06-illuminate/marketplace/ERC20Permit.sol:3:pragma solidity ^0.8.0;