Debt DAO contest - Saintcode_'s results

RECEIVE() FUNCTION WILL LOCK ETHER IN CONTRACT

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert

2 Instances: -SpigotedLine.sol line 272 -Spigot.sol line 227

CONSIDER TWO-PHASE OWNERSHIP TRANSFER

Owner can call .updateOwner function to transfer the ownership to the new address directly. As such, there is a risk that the ownership is transferred to an invalid address, thus causing the contract to be without a owner.

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

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

Instances: -LineOfCredit.sol lines 218, 483 -Spigot.sol line 125 -SpigotLib.sol line 125

<X> += <Y> COSTS MORE GAS THAN <X> = <X> + <Y> FOR STATE VARIABLES

Instances: -SpigotedLine.sol line 125, 172 -EscrowLib.sol line 96

ADD UNCHECKED {} FOR SUBTRACTIONS WHERE THE OPERANDS CANNOT UNDERFLOW BECAUSE OF A PREVIOUS REQUIRE() OR IF-STATEMENT

Instances: -SpigotedLine.sol line 125 -SpigotLib.sol line 96

>= COSTS LESS GAS THAN >

The compiler uses opcodes GT and ISZERO for solidity code that uses >, but only requires LT for >=, which saves 3 gas

change > 0 to >= 1

Instances: -LineOfCredit.sol lines 132, 398, 484, 487, 526 -EscrowLib.sol line 91, 119, 134, 161, 198

++I/I++ SHOULD BE UNCHECKED{++I}/UNCHECKED{I++} WHEN IT IS NOT POSSIBLE FOR THEM TO OVERFLOW, AS IS THE CASE WHEN USED IN FOR- AND WHILE-LOOPS

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop for (uint256 i = 0; i < orders.length; /** NOTE: Removed i++ **/ ) { // Do the thing // Unchecked pre-increment is cheapest unchecked { ++i; } }

Instances: -LineOfCredit.sol lines 179, 203, 520 -CreditListLib.sol line 23, 51 -EscrowLib.sol line 57

DUPLICATED REQUIRE()/REVERT() CHECKS SHOULD BE REFACTORED TO A MODIFIER OR FUNCTION

Saves deployment costs:

(msg.sender != borrower) should be refactored in a modifier: -SpigotedLine.sol lines 102, 140, 162