Debt DAO contest - IllIllI's results

Judge has assessed an item in Issue #454 as M risk. The relevant finding follows:

[L‑01] Don't use payable.transfer()/payable.send() The use of payable.transfer() is heavily frowned upon because it can lead to the locking of funds. The transfer() call requires that the recipient is either an EOA account, or is a contract that has a payable callback. For the contract case, the transfer() call only provides 2300 gas for the contract to complete its operations. This means the following cases can cause the transfer to fail:

The contract does not have a payable callback The contract's payable callback spends more than 2300 gas (which is only enough to emit something) The contract is called through a proxy which itself uses up the 2300 gas Use OpenZeppelin's Address.sendValue() instead There is 1 instance of this issue: File: contracts/utils/LineLib.sol 48: payable(receiver).transfer(amount); https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/LineLib.sol#L48

Summary

Low Risk Issues

Total: 9 instances over 4 issues

Non-critical Issues

Total: 140 instances over 18 issues

Low Risk Issues

[L‑01] Don't use payable.transfer()/payable.send()

The use of payable.transfer() is heavily frowned upon because it can lead to the locking of funds. The transfer() call requires that the recipient is either an EOA account, or is a contract that has a payable callback. For the contract case, the transfer() call only provides 2300 gas for the contract to complete its operations. This means the following cases can cause the transfer to fail:

• The contract does not have a payable callback
• The contract's payable callback spends more than 2300 gas (which is only enough to emit something)
• The contract is called through a proxy which itself uses up the 2300 gas Use OpenZeppelin's Address.sendValue() instead

There is 1 instance of this issue:

File: contracts/utils/LineLib.sol

48:               payable(receiver).transfer(amount);

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/LineLib.sol#L48

[L‑02] Unused/empty receive()/fallback() function

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth))). Having no access control on the function means that someone may send Ether to the contract, and have no way to get anything back out, which is a loss of funds

There is 1 instance of this issue:

File: contracts/modules/credit/SpigotedLine.sol

272:      receive() external payable {}

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L272

[L‑03] Missing checks for address(0x0) when assigning values to address state variables

There are 5 instances of this issue:

File: contracts/modules/credit/LineOfCredit.sol

56:           arbiter = arbiter_;

57:           borrower = borrower_;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L56

File: contracts/modules/credit/SpigotedLine.sol

66:           swapTarget = swapTarget_;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L66

File: contracts/modules/escrow/Escrow.sol

49:           oracle = _oracle;

50:           borrower = _borrower;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/escrow/Escrow.sol#L49

[L‑04] Open TODOs

Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment

There are 2 instances of this issue:

File: contracts/modules/factories/LineFactory.sol

140:          // TODO: test

145:          // TODO: test

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/factories/LineFactory.sol#L140

Non-critical Issues

[N‑01] Duplicate import statements

There is 1 instance of this issue:

File: contracts/utils/CreditLib.sol

6:    import { ILineOfCredit } from "../interfaces/ILineOfCredit.sol";

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditLib.sol#L6

[N‑02] The nonReentrant modifier should occur before all other modifiers

This is a best-practice to protect against reentrancy in other modifiers

There are 2 instances of this issue:

File: contracts/modules/credit/SpigotedLine.sol

96:           nonReentrant

157:          nonReentrant

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L96

[N‑03] Contract implements interface without extending the interface

Not extending the interface may lead to the wrong function signature being used, leading to unexpected behavior. If the interface is in fact being implemented, use the override keyword to indicate that fact

There is 1 instance of this issue:

File: contracts/modules/spigot/Spigot.sol

/// @audit IPendleData.treasury()
16:   contract Spigot is ISpigot, ReentrancyGuard {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L16

[N‑04] Adding a return statement when the function defines a named return variable, is redundant

There are 5 instances of this issue:

File: contracts/modules/credit/LineOfCredit.sol

453:          return id;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L453

File: contracts/utils/CreditLib.sol

160:        return credit;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditLib.sol#L160

File: contracts/utils/SpigotLib.sol

57:           return claimed;

101:          return claimed;

121:          return claimed;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotLib.sol#L57

[N‑05] require()/revert() statements should have descriptive reason strings

There are 23 instances of this issue:

File: contracts/modules/credit/EscrowedLine.sol

64:       require(escrow_.liquidate(amount, targetToken, to));

90:       require(escrow.updateLine(newLine));

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/EscrowedLine.sol#L64

File: contracts/modules/credit/LineOfCredit.sol

112:          require(uint(status) >= uint( LineLib.STATUS.ACTIVE));

241:          require(interestRate.setRate(id, drate, frate));

259:          require(interestRate.setRate(id, drate, frate));

326:          require(amount <= credit.principal + credit.interestAccrued);

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L112

File: contracts/modules/credit/SpigotedLine.sol

62:           require(defaultRevenueSplit_ <= SpigotedLineLib.MAX_SPLIT);

143:        require(amount <= unusedTokens[credit.token]);

160:          require(msg.sender == borrower);

239:          require(msg.sender == arbiter);

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L62

File: contracts/utils/EscrowLib.sol

91:           require(amount > 0);

105:          require(msg.sender == ILineOfCredit(self.line).arbiter());

161:          require(amount > 0);

198:          require(amount > 0);

216:        require(msg.sender == self.line);

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L91

File: contracts/utils/SpigotedLineLib.sol

147:        require(ISpigot(spigot).updateOwner(newLine));

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotedLineLib.sol#L147

File: contracts/utils/SpigotLib.sol

96:               require(LineLib.sendOutTokenOrETH(token, self.treasury, claimed - escrowedAmount));

128:          require(revenueContract != address(this));

130:          require(self.settings[revenueContract].transferOwnerFunction == bytes4(0));

155:          require(success);

180:          require(newOwner != address(0));

189:          require(newOperator != address(0));

201:          require(newTreasury != address(0));

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotLib.sol#L96

[N‑06] constants should be defined rather than using magic numbers

Even assembly can benefit from using readable constants instead of hex/numeric literals

There are 7 instances of this issue:

File: contracts/utils/CreditLib.sol

/// @audit 18
140:            decimals = 18;

/// @audit 18
145:            decimals = !passed ? 18 : abi.decode(result, (uint8));

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditLib.sol#L140

File: contracts/utils/EscrowLib.sol

/// @audit 5
42:           uint256 _numerator = collateralValue * 10**5; // scale to 4 decimals

/// @audit 5
43:           return ((_numerator / debtValue) + 5) / 10;

/// @audit 18
113:                  deposit.assetDecimals = 18;

/// @audit 18
137:                      deposit.assetDecimals = 18;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L42

File: contracts/utils/SpigotLib.sol

/// @audit 100
90:           uint256 escrowedAmount = claimed * self.settings[revenueContract].ownerSplit / 100;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotLib.sol#L90

[N‑07] Numeric values having to do with time should use time units for readability

There are units for seconds, minutes, hours, days, and weeks, and since they're defined, they should be used

There is 1 instance of this issue:

File: contracts/modules/factories/LineFactory.sol

/// @audit 3000
14:       uint32 constant defaultMinCRatio = 3000; // 30.00% minimum collateral ratio

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/factories/LineFactory.sol#L14

[N‑08] Use a more recent version of solidity

Use a solidity version of at least 0.8.12 to get string.concat() to be used instead of abi.encodePacked(<str>,<str>)

There is 1 instance of this issue:

File: contracts/utils/MutualConsent.sol

3:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/MutualConsent.sol#L3

[N‑09] Use a more recent version of solidity

Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions

There are 6 instances of this issue:

File: contracts/modules/credit/LineOfCredit.sol

1:    pragma solidity ^0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L1

File: contracts/modules/credit/SpigotedLine.sol

1:    pragma solidity ^0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L1

File: contracts/modules/escrow/Escrow.sol

1:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/escrow/Escrow.sol#L1

File: contracts/modules/spigot/Spigot.sol

1:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L1

File: contracts/utils/EscrowLib.sol

1:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L1

File: contracts/utils/LineLib.sol

1:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/LineLib.sol#L1

[N‑10] Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)

While the compiler knows to optimize away the exponentiation, it's still better coding practice to use idioms that do not require compiler optimization, if they exist

There is 1 instance of this issue:

File: contracts/utils/EscrowLib.sol

42:           uint256 _numerator = collateralValue * 10**5; // scale to 4 decimals

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L42

[N‑11] Constant redefined elsewhere

Consider defining in only one contract so that values cannot become out of sync when only one location is updated. A cheap way to store constants in a single location is to create an internal constant in a library. If the variable is a local cache of another contract's value, consider making the cache variable internal or private, which will require external users to query the contract with the source of truth, so that callers don't get out of sync.

There are 5 instances of this issue:

File: contracts/modules/escrow/Escrow.sol

/// @audit seen in contracts/modules/credit/LineOfCredit.sol 
27:       address public immutable oracle;

/// @audit seen in contracts/modules/credit/LineOfCredit.sol 
29:       address public immutable borrower;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/escrow/Escrow.sol#L27

File: contracts/modules/factories/LineFactory.sol

/// @audit seen in contracts/modules/credit/LineOfCredit.sol 
16:       address public immutable arbiter;

/// @audit seen in contracts/modules/escrow/Escrow.sol 
17:       address public immutable oracle;

/// @audit seen in contracts/modules/credit/SpigotedLine.sol 
18:       address public immutable swapTarget;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/factories/LineFactory.sol#L16

[N‑12] Inconsistent spacing in comments

Some lines use // x and some use //x. The instances below point out the usages that don't follow the majority, within each file

There are 2 instances of this issue:

File: contracts/modules/credit/LineOfCredit.sol

58:           deadline = block.timestamp + ttl_;  //the deadline is the term/maturity/expiry date of the Line of Credit facility

526:                    credits[id].principal > 0 //`id` should be placed before `p` 

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L58

[N‑13] Non-library/interface files should use fixed compiler versions, not floating ones

There are 5 instances of this issue:

File: contracts/modules/credit/LineOfCredit.sol

1:    pragma solidity ^0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L1

File: contracts/modules/credit/SecuredLine.sol

1:    pragma solidity ^0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SecuredLine.sol#L1

File: contracts/modules/credit/SpigotedLine.sol

1:    pragma solidity ^0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L1

File: contracts/modules/interest-rate/InterestRateCredit.sol

1:    pragma solidity ^0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/interest-rate/InterestRateCredit.sol#L1

File: contracts/modules/oracle/Oracle.sol

2:    pragma solidity ^0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/oracle/Oracle.sol#L2

[N‑14] File does not contain an SPDX Identifier

There are 16 instances of this issue:

File: contracts/modules/credit/EscrowedLine.sol

0:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/EscrowedLine.sol#L0

File: contracts/modules/credit/LineOfCredit.sol

0:    pragma solidity ^0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L0

File: contracts/modules/credit/SecuredLine.sol

0:    pragma solidity ^0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SecuredLine.sol#L0

File: contracts/modules/credit/SpigotedLine.sol

0:    pragma solidity ^0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L0

File: contracts/modules/escrow/Escrow.sol

0:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/escrow/Escrow.sol#L0

File: contracts/modules/factories/LineFactory.sol

0:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/factories/LineFactory.sol#L0

File: contracts/modules/interest-rate/InterestRateCredit.sol

0:    pragma solidity ^0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/interest-rate/InterestRateCredit.sol#L0

File: contracts/modules/spigot/Spigot.sol

0:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L0

File: contracts/utils/CreditLib.sol

0:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditLib.sol#L0

File: contracts/utils/CreditListLib.sol

0:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditListLib.sol#L0

File: contracts/utils/EscrowLib.sol

0:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L0

File: contracts/utils/LineFactoryLib.sol

0:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/LineFactoryLib.sol#L0

File: contracts/utils/LineLib.sol

0:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/LineLib.sol#L0

File: contracts/utils/MutualConsent.sol

0:    // forked from https://github.com/IndexCoop/index-coop-smart-contracts/blob/master/contracts/lib/MutualConsent.sol

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/MutualConsent.sol#L0

File: contracts/utils/SpigotedLineLib.sol

0:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotedLineLib.sol#L0

File: contracts/utils/SpigotLib.sol

0:    pragma solidity 0.8.9;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotLib.sol#L0

[N‑15] NatSpec is incomplete

There are 56 instances of this issue:

File: contracts/modules/credit/EscrowedLine.sol

/// @audit Missing: '@param newLine'
82      /**
83       * see SecuredlLine.rollover
84       * @notice helper function to allow borrower to easily swithc collateral to a new Line after repyment
85       *(@dev priviliegad internal function.
86       * @dev MUST only be callable if line is REPAID
87       * @return - if function successfully executed
88      */
89:     function _rollover(address newLine) internal virtual returns(bool) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/EscrowedLine.sol#L82-L89

File: contracts/modules/credit/LineOfCredit.sol

/// @audit Missing: '@return'
216         @param id - the position id for credit position
217       */
218:      function _accrue(Credit memory credit, bytes32 id) internal returns(Credit memory) {

/// @audit Missing: '@param status_'
415       /**
416         * @notice - updates `status` variable in storage if current status is diferent from existing status.
417         * @dev - privileged internal function. MUST check params and logic flow before calling
418         * @dev - does not save new status if it is the same as current status
419         * @return status - the current status of the line after updating
420        */
421:      function _updateStatus(LineLib.STATUS status_) internal returns(LineLib.STATUS) {

/// @audit Missing: '@return'
433        * @param amount - amount of tokens lender will initially deposit
434       */
435       function _createCredit(
436           address lender,
437           address token,
438           uint256 amount
439       )
440           internal
441:          returns (bytes32 id)

/// @audit Missing: '@param credit'
456     /**
457      * @dev - Reduces `principal` and/or `interestAccrued` on a credit line.
458               Expects checks for conditions of repaying and param sanitizing before calling
459               e.g. early repayment of principal, tokens have actually been paid by borrower, etc.
460      * @dev - privileged internal function. MUST check params and logic flow before calling
461      * @param id - position id with all data pertaining to line
462      * @param amount - amount of Credit Token being repaid on credit line
463      * @return credit - position struct in memory with updated values
464     */
465       function _repay(Credit memory credit, bytes32 id, uint256 amount)
466           internal
467:          returns (Credit memory)

/// @audit Missing: '@param credit'
/// @audit Missing: '@param id'
477       /**
478        * @notice - checks that a credit line is fully repaid and removes it
479        * @dev deletes credit storage. Store any data u might need later in call before _close()
480        * @dev - privileged internal function. MUST check params and logic flow before calling
481        * @return credit - position struct in memory with updated values
482        */
483:      function _close(Credit memory credit, bytes32 id) internal virtual returns (bool) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L216-L218

File: contracts/modules/credit/SecuredLine.sol

/// @audit Missing: '@return'
77       * @param targetToken - token in escrow that will be sold of to repay position
78       */
79    
80      function liquidate(
81        uint256 amount,
82        address targetToken
83      )
84        external
85        whileBorrowing
86:       returns(uint256)

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SecuredLine.sol#L77-L86

File: contracts/modules/escrow/Escrow.sol

/// @audit Missing: '@param _line'
69        /**
70        * @notice - Allows current owner to transfer ownership to another address
71        * @dev    - Used if we setup Escrow before Line exists. Line has no way to interface with this function so once transfered `line` is set forever
72        * @return didUpdate - if function successfully executed or not
73        */
74:       function updateLine(address _line) external returns(bool) {

/// @audit Missing: '@return'
98         * @param token - the token to all borrow to deposit as collateral
99         */
100:      function enableCollateral(address token) external returns (bool) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/escrow/Escrow.sol#L69-L74

File: contracts/modules/oracle/Oracle.sol

/// @audit Missing: '@param token'
19        /**
20         * @return price - the latest price in USD to 8 decimals
21         */
22:       function getLatestAnswer(address token) external returns (int) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/oracle/Oracle.sol#L19-L22

File: contracts/modules/spigot/Spigot.sol

/// @audit Missing: '@param token'
57        /**
58    
59         * @notice - Claims revenue tokens from the Spigot (push and pull payments) and escrows them for the Owner withdraw later.
60                   - Calls predefined function in contract settings to claim revenue.
61                   - Automatically sends portion to Treasury and then escrows Owner's share
62                   - There is no conversion or trade of revenue tokens. 
63         * @dev    - Assumes the only side effect of calling claimFunc on revenueContract is we receive new tokens.
64                   - Any other side effects could be dangerous to the Spigot or upstream contracts.
65         * @dev    - callable by anyone
66         * @param revenueContract - Contract with registered settings to claim revenue from
67         * @param data - Transaction data, including function signature, to properly claim revenue on revenueContract
68         * @return claimed -  The amount of revenue tokens claimed from revenueContract and split between `owner` and `treasury`
69        */
70        function claimRevenue(address revenueContract, address token, bytes calldata data)
71            external nonReentrant
72:           returns (uint256 claimed)

/// @audit Missing: '@return'
106        * @param data - tx data, including function signature, to call contract with
107        */
108:      function operate(address revenueContract, bytes calldata data) external returns (bool) {

/// @audit Missing: '@return'
123        * @param setting - Spigot settings for smart contract   
124        */
125:      function addSpigot(address revenueContract, Setting memory setting) external returns (bool) {

/// @audit Missing: '@return'
135        * @param revenueContract - smart contract to transfer ownership of
136        */
137       function removeSpigot(address revenueContract)
138           external
139:          returns (bool)

/// @audit Missing: '@return'
157        * @param newOwner - Address to give control to
158        */
159:      function updateOwner(address newOwner) external returns (bool) {

/// @audit Missing: '@return'
168        * @param newOperator - Address to give control to
169        */
170:      function updateOperator(address newOperator) external returns (bool) {

/// @audit Missing: '@return'
179        * @param newTreasury - Address to divert funds to
180        */
181:      function updateTreasury(address newTreasury) external returns (bool) {

/// @audit Missing: '@return'
192        * @param allowed - true/false whether to allow this function to be called by Operator
193        */
194:       function updateWhitelistedFunction(bytes4 func, bool allowed) external returns (bool) {

/// @audit Missing: '@return'
204        * @param token Revenue token that is being garnished from spigots
205       */
206:      function getEscrowed(address token) external view returns (uint256) {

/// @audit Missing: '@return'
213        * @param func Function to check on whitelist 
214       */
215   
216:      function isWhitelisted(bytes4 func) external view returns(bool) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L57-L72

File: contracts/utils/CreditLib.sol

/// @audit Missing: '@param id'
/// @audit Missing: '@param amount'
/// @audit Missing: '@param lender'
/// @audit Missing: '@param token'
/// @audit Missing: '@return'
120     /**
121       * see ILineOfCredit._createCredit
122       * @notice called by LineOfCredit._createCredit during every repayment function
123       * @param oracle - interset rate contract used by line that will calculate interest owed
124      */
125     function create(
126         bytes32 id,
127         uint256 amount,
128         address lender,
129         address token,
130         address oracle
131     )
132         external 
133:        returns(ILineOfCredit.Credit memory credit)

/// @audit Missing: '@param id'
/// @audit Missing: '@param amount'
/// @audit Missing: '@return'
163     /**
164       * see ILineOfCredit._repay
165       * @notice called by LineOfCredit._repay during every repayment function
166       * @param credit - The lender position being repaid
167      */
168     function repay(
169       ILineOfCredit.Credit memory credit,
170       bytes32 id,
171       uint256 amount
172     )
173       external
174:      returns (ILineOfCredit.Credit memory)

/// @audit Missing: '@param id'
/// @audit Missing: '@param amount'
/// @audit Missing: '@return'
197     /**
198       * see ILineOfCredit.withdraw
199       * @notice called by LineOfCredit.withdraw during every repayment function
200       * @param credit - The lender position that is being bwithdrawn from
201      */
202     function withdraw(
203       ILineOfCredit.Credit memory credit,
204       bytes32 id,
205       uint256 amount
206     )
207       external
208:      returns (ILineOfCredit.Credit memory)

/// @audit Missing: '@param credit'
/// @audit Missing: '@param id'
/// @audit Missing: '@return'
234     /**
235       * see ILineOfCredit._accrue
236       * @notice called by LineOfCredit._accrue during every repayment function
237       * @param interest - interset rate contract used by line that will calculate interest owed
238      */
239     function accrue(
240       ILineOfCredit.Credit memory credit,
241       bytes32 id,
242       address interest
243     )
244       public
245:      returns (ILineOfCredit.Credit memory)

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditLib.sol#L120-L133

File: contracts/utils/EscrowLib.sol

/// @audit Missing: '@param self'
28        /**
29         * @notice updates the cratio according to the collateral value vs line value
30         * @dev calls accrue interest on the line contract to update the latest interest payable
31         * @param oracle - address to call for collateral token prices
32         * @return cratio - the updated collateral ratio in 4 decimals
33        */
34:       function _getLatestCollateralRatio(EscrowState storage self, address oracle) public returns (uint256) {

/// @audit Missing: '@param self'
46        /**
47        * @notice - Iterates over all enabled tokens and calculates the USD value of all deposited collateral
48        * @param oracle - address to call for collateral token prices
49        * @return totalCollateralValue - the collateral's USD value in 8 decimals
50        */
51:       function _getCollateralValue(EscrowState storage self, address oracle) public returns (uint256) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L28-L34

File: contracts/utils/LineFactoryLib.sol

/// @audit Missing: '@param oracle'
/// @audit Missing: '@param arbiter'
33        /**
34          @notice sets up new line based of config of old line. Old line does not need to have REPAID status for this call to succeed.
35          @dev borrower must call rollover() on `oldLine` with newly created line address
36          @param oldLine  - line to copy config from for new line.
37          @param borrower - borrower address on new line
38          @param ttl      - set total term length of line
39          @return newLine - address of newly deployed line with oldLine config
40         */
41        function rolloverSecuredLine(
42            address payable oldLine,
43            address borrower, 
44            address oracle,
45            address arbiter,
46            uint ttl
47:       ) external returns(address) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/LineFactoryLib.sol#L33-L47

File: contracts/utils/LineLib.sol

/// @audit Missing: '@return'
32         * @param amount - amount of tokens to send
33         */
34        function sendOutTokenOrETH(
35          address token,
36          address receiver,
37          uint256 amount
38        )
39          external
40:         returns (bool)

/// @audit Missing: '@return'
57         * @param amount - amount of tokens to send
58         */
59        function receiveTokenOrETH(
60          address token,
61          address sender,
62          uint256 amount
63        )
64          external
65:         returns (bool)

/// @audit Missing: '@return'
78         * @param token - address of token to check. Denominations.ETH for raw ETH
79        */
80:       function getBalance(address token) external view returns (uint256) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/LineLib.sol#L32-L40

File: contracts/utils/SpigotedLineLib.sol

/// @audit Missing: '@param spigot'
/// @audit Missing: '@param status'
/// @audit Missing: '@param defaultSplit'
163       /**
164        * @notice Changes the revenue split between a Borrower's treasury and the LineOfCredit based on line health, runs with updateOwnerSplit()
165        * @dev    - callable `arbiter` + `borrower`
166        * @param revenueContract - spigot to update
167        * @return whether or not split was updated
168        */
169:      function updateSplit(address spigot, address revenueContract, LineLib.STATUS status, uint8 defaultSplit) external returns (bool) {

/// @audit Missing: '@param spigot'
/// @audit Missing: '@param status'
/// @audit Missing: '@param borrower'
/// @audit Missing: '@param arbiter'
/// @audit Missing: '@param to'
186       /**
187   
188      * @notice -  Transfers ownership of the entire Spigot and its revenuw streams from its then Owner to either 
189                   the Borrower (if a Line of Credit has been been fully repaid) or 
190                   to the Arbiter (if the Line of Credit is liquidatable).
191      * @dev    - callable by anyone 
192      * @return - whether or not Spigot was released
193     */
194:      function releaseSpigot(address spigot, LineLib.STATUS status, address borrower, address arbiter, address to) external returns (bool) {

/// @audit Missing: '@param to'
/// @audit Missing: '@param token'
/// @audit Missing: '@param amount'
/// @audit Missing: '@param status'
/// @audit Missing: '@param borrower'
/// @audit Missing: '@param arbiter'
211     /**
212      * @notice -  Sends any remaining tokens (revenue or credit tokens) in the Spigot to the Borrower after the loan has been repaid.
213                -  In case of a Borrower default (loan status = liquidatable), this is a fallback mechanism to withdraw all the tokens and send them to the Arbiter
214                -  Does not transfer anything if line is healthy
215      * @return - whether or not spigot was released
216     */
217:      function sweep(address to, address token, uint256 amount, LineLib.STATUS status, address borrower, address arbiter) external returns (bool) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotedLineLib.sol#L163-L169

[N‑16] Event is missing indexed fields

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

There are 4 instances of this issue:

File: contracts/utils/MutualConsent.sol

21        event MutualConsentRegistered(
22            bytes32 _consentHash
23:       );

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/MutualConsent.sol#L21-L23

File: contracts/utils/SpigotLib.sol

241       event AddSpigot(
242           address indexed revenueContract,
243           uint256 ownerSplit
244:      );

255       event ClaimRevenue(
256           address indexed token,
257           uint256 indexed amount,
258           uint256 escrowed,
259           address revenueContract
260:      );

262       event ClaimEscrow(
263           address indexed token,
264           uint256 indexed amount,
265           address owner
266:      );

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotLib.sol#L241-L244

[N‑17] Not using the named return variables anywhere in the function is confusing

Consider changing the variable to be an unnamed one

There are 2 instances of this issue:

File: contracts/modules/spigot/Spigot.sol

/// @audit claimed
70        function claimRevenue(address revenueContract, address token, bytes calldata data)
71            external nonReentrant
72:           returns (uint256 claimed)

/// @audit claimed
85        function claimEscrow(address token)
86            external
87            nonReentrant
88:           returns (uint256 claimed) 

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L70-L72

[N‑18] Duplicated require()/revert() checks should be refactored to a modifier or function

The compiler will inline the function, which will avoid JUMP instructions usually associated with functions

There are 2 instances of this issue:

File: contracts/modules/credit/LineOfCredit.sol

259:          require(interestRate.setRate(id, drate, frate));

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L259

File: contracts/utils/EscrowLib.sol

161:          require(amount > 0);

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L161

---

Excluded findings

These findings are excluded from awards calculations because there are publicly-available automated tools that find them. The valid ones appear here for completeness

Summary

Non-critical Issues

Total: 1 instances over 1 issues

Non-critical Issues

[N‑01] Return values of approve() not checked

Not all IERC20 implementations revert() when there's a failure in approve(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything

There is 1 instance of this issue:

File: contracts/utils/SpigotedLineLib.sol

/// @audit (valid but excluded finding)
134:              IERC20(sellToken).approve(swapTarget, amount);

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotedLineLib.sol#L134

Summary

Gas Optimizations

Total: 84 instances over 16 issues with 21688 gas saved

Gas totals use lower bounds of ranges and count two iterations of each for-loop. All values above are runtime, not deployment, values; deployment values are listed in the individual issue descriptions. The table above as well as its gas numbers do not include any of the excluded findings.

Gas Optimizations

[G‑01] State variables only set in the constructor should be declared immutable

Avoids a Gsset (20000 gas) in the constructor, and replaces the first access in each transaction (Gcoldsload - 2100 gas) and each access thereafter (Gwarmacces - 100 gas) with a PUSH32 (3 gas).

While strings are not value types, and therefore cannot be immutable/constant if not hard-coded outside of the constructor, the same behavior can be achieved by making the current contract abstract with virtual functions for the string accessors, and having a child contract override the functions with the hard-coded implementation-specific values.

There are 2 instances of this issue:

File: contracts/modules/oracle/Oracle.sol

/// @audit registry (constructor)
16:           registry = FeedRegistryInterface(_registry);

/// @audit registry (access)
29:           ) = registry.latestRoundData(token, Denominations.USD);

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/oracle/Oracle.sol#L16

[G‑02] 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. Note that even if an interface defines a function as having memory arguments, it's still valid for implementation contracs to use calldata arguments instead.

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

Note that I've also flagged instances where the function is public but can be marked as external since it's not called by the contract, and cases where a constructor is involved

There are 5 instances of this issue:

File: contracts/modules/spigot/Spigot.sol

/// @audit setting
125:      function addSpigot(address revenueContract, Setting memory setting) external returns (bool) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L125

File: contracts/utils/CreditLib.sol

/// @audit credit
73      function getOutstandingDebt(
74        ILineOfCredit.Credit memory credit,
75        bytes32 id,
76        address oracle,
77        address interestRate
78      )
79        external
80:       returns (ILineOfCredit.Credit memory c, uint256 principal, uint256 interest)

/// @audit credit
168     function repay(
169       ILineOfCredit.Credit memory credit,
170       bytes32 id,
171       uint256 amount
172     )
173       external
174:      returns (ILineOfCredit.Credit memory)

/// @audit credit
202     function withdraw(
203       ILineOfCredit.Credit memory credit,
204       bytes32 id,
205       uint256 amount
206     )
207       external
208:      returns (ILineOfCredit.Credit memory)

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditLib.sol#L73-L80

File: contracts/utils/SpigotLib.sol

/// @audit setting
125:      function addSpigot(SpigotState storage self, address revenueContract, ISpigot.Setting memory setting) external returns (bool) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotLib.sol#L125

[G‑03] Using storage instead of memory for structs/arrays saves gas

When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read. Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct

There are 3 instances of this issue:

File: contracts/modules/credit/LineOfCredit.sol

205:            Credit memory credit = credits[id];

323:          Credit memory credit = credits[id];

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L205

File: contracts/modules/credit/SpigotedLine.sol

139:        Credit memory credit = credits[id];

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L139

[G‑04] Avoid contract existence checks by using low level calls

Prior to 0.8.10 the compiler inserted extra code, including EXTCODESIZE (100 gas), to check for contract existence for external function calls. In more recent solidity versions, the compiler will not insert these checks if the external call has a return value. Similar behavior can be achieved in earlier versions by using low-level calls, since low level calls never check for contract existence

There are 27 instances of this issue:

File: contracts/modules/credit/SecuredLine.sol

/// @audit status()
57:       if(ILineOfCredit(newLine).status() != LineLib.STATUS.UNINITIALIZED) { revert BadNewLine(); }

/// @audit init()
63:       if(ILineOfCredit(newLine).init() != LineLib.STATUS.ACTIVE) { revert BadRollover(); }

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SecuredLine.sol#L57

File: contracts/utils/CreditLib.sol

/// @audit getLatestAnswer()
84:       int256 price = IOracle(oracle).getLatestAnswer(c.token);

/// @audit getLatestAnswer()
135:        int price = IOracle(oracle).getLatestAnswer(token);

/// @audit accrueInterest()
251:        uint256 accruedToken = IInterestRateCredit(interest).accrueInterest(

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditLib.sol#L84

File: contracts/utils/EscrowLib.sol

/// @audit updateOutstandingDebt()
35:           (uint256 principal, uint256 interest) = ILineOfCredit(self.line).updateOutstandingDebt();

/// @audit arbiter()
105:          require(msg.sender == ILineOfCredit(self.line).arbiter());

/// @audit getLatestAnswer()
126:                  int256 price = IOracle(oracle).getLatestAnswer(deposit.asset);

/// @audit status()
173:            ILineOfCredit(self.line).status() != LineLib.STATUS.REPAID // if repaid, skip;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L35

File: contracts/utils/LineFactoryLib.sol

/// @audit spigot()
48:           address s = address(SecuredLine(oldLine).spigot());

/// @audit escrow()
49:           address e = address(SecuredLine(oldLine).escrow());

/// @audit swapTarget()
50:           address payable st = SecuredLine(oldLine).swapTarget();

/// @audit defaultRevenueSplit()
51:           uint8 split = SecuredLine(oldLine).defaultRevenueSplit();

/// @audit init()
72:           if(SecuredLine(payable(line)).init() != LineLib.STATUS.ACTIVE) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/LineFactoryLib.sol#L48

File: contracts/utils/LineLib.sol

/// @audit safeTransfer()
46:               IERC20(token).safeTransfer(receiver, amount);

/// @audit transfer()
48:               payable(receiver).transfer(amount);

/// @audit safeTransferFrom()
69:               IERC20(token).safeTransferFrom(sender, address(this), amount);

/// @audit balanceOf()
83:               IERC20(token).balanceOf(address(this)) :

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/LineLib.sol#L46

File: contracts/utils/SpigotedLineLib.sol

/// @audit claimEscrow()
73:           uint256 claimed = ISpigot(spigot).claimEscrow(claimToken);

/// @audit approve()
134:              IERC20(sellToken).approve(swapTarget, amount);

/// @audit updateOwner()
147:        require(ISpigot(spigot).updateOwner(newLine));

/// @audit owner()
153:        address owner_ = ISpigot(spigot).owner();

/// @audit getSetting()
170:          (uint8 split,, bytes4 transferFunc) = ISpigot(spigot).getSetting(revenueContract);

/// @audit updateOwnerSplit()
176:              return ISpigot(spigot).updateOwnerSplit(revenueContract, defaultSplit);

/// @audit updateOwnerSplit()
179:              return ISpigot(spigot).updateOwnerSplit(revenueContract, MAX_SPLIT);

/// @audit updateOwner()
196:            if(!ISpigot(spigot).updateOwner(to)) { revert ReleaseSpigotFailed(); }

/// @audit updateOwner()
201:            if(!ISpigot(spigot).updateOwner(to)) { revert ReleaseSpigotFailed(); }

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotedLineLib.sol#L73

[G‑05] 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 replaces 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 5 instances of this issue:

File: contracts/modules/credit/LineOfCredit.sol

/// @audit count on line 499
502:          if (count == 0) { _updateStatus(LineLib.STATUS.REPAID); }

/// @audit ids on line 172
180:              id = ids[i];

/// @audit ids on line 201
204:            id = ids[i];

/// @audit ids on line 517
521:              id = ids[i];

/// @audit ids on line 532
532:                  ids[i] = ids[nextQSpot];    // id put into old `p` position

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L502

[G‑06] internal functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

There are 4 instances of this issue:

File: contracts/modules/credit/LineOfCredit.sol

167       function _updateOutstandingDebt()
168           internal
169:          returns (uint256 principal, uint256 interest)

435       function _createCredit(
436           address lender,
437           address token,
438           uint256 amount
439       )
440           internal
441:          returns (bytes32 id)

516:      function _sortIntoQ(bytes32 p) internal returns (bool) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L167-L169

File: contracts/modules/interest-rate/InterestRateCredit.sol

42        function _accrueInterest(
43            bytes32 id,
44            uint256 drawnBalance,
45            uint256 facilityBalance
46:       ) internal returns (uint256) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/interest-rate/InterestRateCredit.sol#L42-L46

[G‑07] Add unchecked {} for subtractions where the operands cannot underflow because of a previous require() or if-statement

require(a <= b); x = b - a => require(a <= b); unchecked { x = b - a }

There are 3 instances of this issue:

File: contracts/modules/credit/SpigotedLine.sol

/// @audit if-condition on line 120
122:              unusedTokens[credit.token] -= repaid - newTokens;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L122

File: contracts/utils/SpigotedLineLib.sol

/// @audit if-condition on line 100
101:            uint256 diff = oldClaimTokens - newClaimTokens;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotedLineLib.sol#L101

File: contracts/utils/SpigotLib.sol

/// @audit if-condition on line 95
96:               require(LineLib.sendOutTokenOrETH(token, self.treasury, claimed - escrowedAmount));

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotLib.sol#L96

[G‑08] ++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

There are 6 instances of this issue:

File: contracts/modules/credit/LineOfCredit.sol

179:          for (uint256 i; i < len; ++i) {

203:          for (uint256 i; i < len; ++i) {

520:          for (uint256 i; i <= lastSpot; ++i) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L179

File: contracts/utils/CreditListLib.sol

23:         for(uint256 i; i < len; ++i) {

51:           for(uint i = 1; i < len; ++i) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditListLib.sol#L23

File: contracts/utils/EscrowLib.sol

57:           for (uint256 i; i < length; ++i) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L57

[G‑09] require()/revert() strings longer than 32 bytes cost extra gas

Each extra memory word of bytes past the original 32 incurs an MSTORE which costs 3 gas

There is 1 instance of this issue:

File: contracts/modules/interest-rate/InterestRateCredit.sol

26            require(
27                msg.sender == lineContract,
28                "InterestRateCred: only line contract."
29:           );

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/interest-rate/InterestRateCredit.sol#L26-L29

[G‑10] Optimize names to save gas

public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted

There are 15 instances of this issue:

File: contracts/modules/credit/LineOfCredit.sol

/// @audit init(), healthcheck(), counts(), declareInsolvent()
16:   contract LineOfCredit is ILineOfCredit, MutualConsent {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L16

File: contracts/modules/credit/SecuredLine.sol

/// @audit liquidate()
11:   contract SecuredLine is SpigotedLine, EscrowedLine, ISecuredLine {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SecuredLine.sol#L11

File: contracts/modules/credit/SpigotedLine.sol

/// @audit unused(), claimAndRepay(), useAndRepay(), claimAndTrade(), updateOwnerSplit(), addSpigot(), updateWhitelist(), releaseSpigot(), sweep()
22:   contract SpigotedLine is ISpigotedLine, LineOfCredit, ReentrancyGuard {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L22

File: contracts/modules/escrow/Escrow.sol

/// @audit isLiquidatable(), updateLine(), addCollateral(), enableCollateral(), releaseCollateral(), getCollateralRatio(), getCollateralValue(), liquidate()
19:   contract Escrow is IEscrow {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/escrow/Escrow.sol#L19

File: contracts/modules/factories/LineFactory.sol

/// @audit deployEscrow(), deploySpigot(), deploySecuredLine(), deploySecuredLineWithConfig(), deploySecuredLineWithModules(), rolloverSecuredLine()
9:    contract LineFactory is ILineFactory {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/factories/LineFactory.sol#L9

File: contracts/modules/interest-rate/InterestRateCredit.sol

/// @audit setRate()
5:    contract InterestRateCredit is IInterestRateCredit {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/interest-rate/InterestRateCredit.sol#L5

File: contracts/modules/oracle/Oracle.sol

/// @audit getLatestAnswer()
13:   contract Oracle is IOracle {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/oracle/Oracle.sol#L13

File: contracts/modules/spigot/Spigot.sol

/// @audit operator(), claimRevenue(), claimEscrow(), operate(), addSpigot(), removeSpigot(), updateOwnerSplit(), updateOwner(), updateOperator(), updateTreasury(), updateWhitelistedFunction(), getEscrowed(), isWhitelisted(), getSetting()
16:   contract Spigot is ISpigot, ReentrancyGuard {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L16

File: contracts/utils/CreditLib.sol

/// @audit computeId(), getOutstandingDebt(), calculateValue(), create(), repay(), withdraw(), accrue()
14:   library CreditLib {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditLib.sol#L14

File: contracts/utils/CreditListLib.sol

/// @audit removePosition(), stepQ()
10:   library CreditListLib {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditListLib.sol#L10

File: contracts/utils/EscrowLib.sol

/// @audit _getLatestCollateralRatio(), _getCollateralValue(), addCollateral(), enableCollateral(), releaseCollateral(), getCollateralRatio(), getCollateralValue(), liquidate(), isLiquidatable(), updateLine()
21:   library EscrowLib {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L21

File: contracts/utils/LineFactoryLib.sol

/// @audit rolloverSecuredLine(), transferModulesToLine(), deploySecuredLine()
7:    library LineFactoryLib {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/LineFactoryLib.sol#L7

File: contracts/utils/LineLib.sol

/// @audit sendOutTokenOrETH(), receiveTokenOrETH(), getBalance()
14:   library LineLib {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/LineLib.sol#L14

File: contracts/utils/SpigotedLineLib.sol

/// @audit claimAndTrade(), trade(), rollover(), canDeclareInsolvent(), updateSplit(), releaseSpigot(), sweep()
10:   library SpigotedLineLib {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotedLineLib.sol#L10

File: contracts/utils/SpigotLib.sol

/// @audit _claimRevenue(), operate(), claimRevenue(), claimEscrow(), addSpigot(), removeSpigot(), updateOwnerSplit(), updateOwner(), updateOperator(), updateTreasury(), updateWhitelistedFunction(), getEscrowed(), isWhitelisted(), getSetting()
23:   library SpigotLib {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotLib.sol#L23

[G‑11] 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 Each operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed

There are 2 instances of this issue:

File: contracts/utils/CreditLib.sol

/// @audit uint8 decimals
140:            decimals = 18;

/// @audit uint8 decimals
145:            decimals = !passed ? 18 : abi.decode(result, (uint8));

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditLib.sol#L140

[G‑12] 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 are 3 instances of this issue:

File: contracts/modules/credit/LineOfCredit.sol

21:       uint256 public immutable deadline;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L21

File: contracts/modules/credit/SpigotedLine.sol

32:       uint8 public immutable defaultRevenueSplit;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L32

File: contracts/modules/escrow/Escrow.sol

24:       uint32 public immutable minimumCollateralRatio;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/escrow/Escrow.sol#L24

[G‑13] Inverting the condition of an if-else-statement wastes gas

Flipping the true and false blocks instead saves 3 gas

There are 2 instances of this issue:

File: contracts/utils/CreditLib.sol

145:            decimals = !passed ? 18 : abi.decode(result, (uint8));

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditLib.sol#L145

File: contracts/utils/EscrowLib.sol

122                   deposit.asset = !is4626
123                       ? token
124:                      : abi.decode(tokenAddrBytes, (address));

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L122-L124

[G‑14] require() or revert() statements that check input arguments should be at the top of the function

Checks that involve constants should come before checks that involve state variables, function calls, and calculations. By doing these checks first, the function is able to revert before wasting a Gcoldsload (2100 gas*) in a function that may ultimately revert in the unhappy case.

There is 1 instance of this issue:

File: contracts/modules/credit/LineOfCredit.sol

/// @audit expensive op on line 324
326:          require(amount <= credit.principal + credit.interestAccrued);

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L326

[G‑15] 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 hit by avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas

There is 1 instance of this issue:

File: contracts/modules/interest-rate/InterestRateCredit.sol

26            require(
27                msg.sender == lineContract,
28                "InterestRateCred: only line contract."
29:           );

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/interest-rate/InterestRateCredit.sol#L26-L29

[G‑16] Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost

There are 4 instances of this issue:

File: contracts/modules/credit/LineOfCredit.sol

340       function borrow(bytes32 id, uint256 amount)
341           external
342           override
343           whileActive
344           onlyBorrower
345:          returns (bool)

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L340-L345

File: contracts/modules/credit/SecuredLine.sol

48      function rollover(address newLine)
49        external
50        onlyBorrower
51        override
52:       returns(bool)

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SecuredLine.sol#L48-L52

File: contracts/modules/interest-rate/InterestRateCredit.sol

34        function accrueInterest(
35            bytes32 id,
36            uint256 drawnBalance,
37            uint256 facilityBalance
38:       ) external override onlyLineContract returns (uint256) {

74        function setRate(
75            bytes32 id,
76            uint128 dRate,
77            uint128 fRate
78:       ) external onlyLineContract returns (bool) {

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/interest-rate/InterestRateCredit.sol#L34-L38

---

Excluded findings

These findings are excluded from awards calculations because there are publicly-available automated tools that find them. The valid ones appear here for completeness

Summary

Gas Optimizations

Total: 4 instances over 2 issues with 17118 gas saved

Gas totals use lower bounds of ranges and count two iterations of each for-loop. All values above are runtime, not deployment, values; deployment values are listed in the individual issue descriptions. The table above as well as its gas numbers do not include any of the excluded findings.

Gas Optimizations

[G‑01] 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: contracts/utils/MutualConsent.sol

/// @audit (valid but excluded finding)
15:       mapping(bytes32 => bool) public mutualConsents;

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/MutualConsent.sol#L15

[G‑02] Using > 0 costs more gas than != 0 when used on a uint in a require() statement

This change saves 6 gas per instance. The optimization works until solidity version 0.8.13 where there is a regression in gas costs.

There are 3 instances of this issue:

File: contracts/utils/EscrowLib.sol

/// @audit (valid but excluded finding)
91:           require(amount > 0);

/// @audit (valid but excluded finding)
161:          require(amount > 0);

/// @audit (valid but excluded finding)
198:          require(amount > 0);

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L91