Debt DAO contest - aphak5010's results

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L70-L75 https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotLib.sol#L34 https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotLib.sol#L90

Vulnerability details

Impact

Spigot.claimRevenue function can be called by anyone.
If called with non-existing revenueContract address, the pushed revenues will be processed with ownerSplit set to 0, effectively sending all pushed revenue to the treasury.

This behavior makes the functionality to push revenue completely unusable.

Also if the Line of Credit is in distress and all revenue should be used to repay debt, this can be used to send all revenue to the treasury.

Proof of Concept

1. Set up a Spigot and add a revenue contract.
2. An attacker calls Spigot.claimRevenue() with revenueContract = address(0) parameter: https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L70-L75
3. In SpigotLib._claimRevenue, the if block will be executed because self.settings[revenueContract].claimFunction == bytes4(0). Also the ownerSplit in the SpigotLib.claimRevenue function will be 0.
4. All pushed revenue will be sent to treasury.

Tools Used

VSCode

Recommended Mitigation Steps

Add a mapping address => bool called allowedRevenueContracts.
When a revenueContract is added, allowedRevenueContacts[revenueContract] is set to true.
When claiming revenue, check if allowedRevenueContacts[revenueContract] == true.

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L137 https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditLib.sol#L186

Vulnerability details

Impact

The lender and borrower can both call the function SpigotedLine.useAndRepay (https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L137) and specify an amount that is used to repay the debt.
The lender can call this function with an amount > credit.interestAccrued + credit.principal. This will cause an Underflow in the CreditLib.repay function (https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditLib.sol#L186).

Practically, this means that if a SpigotedLine is used, the lender can cause the debt for the borrower to be so big that it could never be repaid (principal is a uint256).

So the borrower will get liquidated and lose access to the revenue contracts and all the assets will be used to pay the lenders.

Proof of Concept

I used the following test and added it to SpigotedLine.t.sol.
It will cause an underflow in the credit.principal.

function test_audit_lender_can_use_and_repay_underflow() public {
    deal(address(lender), lentAmount + 1 ether);
    deal(address(revenueToken), MAX_REVENUE);
    address revenueC = address(0xbeef); // need new spigot for testing
    bytes32 id = _createCredit(address(revenueToken), Denominations.ETH, revenueC);
    _borrow(id, lentAmount - 1);

    bytes memory tradeData = abi.encodeWithSignature(
    'trade(address,address,uint256,uint256)',
    address(revenueToken),
    Denominations.ETH,
    1 gwei,
    lentAmount
    );

    hoax(borrower);
    line.claimAndTrade(address(revenueToken), tradeData);
    (, uint256 principal,,,,,) = line.credits(line.ids(0));
    console.log(principal);

    vm.prank(lender); // prank lender
    // This amount is 1 bigger than the principal
    line.useAndRepay(lentAmount);

    (, principal,,,,,) = line.credits(line.ids(0));
    console.log(principal);
}

After the exploitation, credit.principal will be 115792089237316195423570985008687907853269984665640564039457584007913129639935.

Tools Used

VSCode

Recommended Mitigation Steps

Limit the debt that is repaid to credit.interestAccrued + credit.principal as you do in the SpigotedLine.claimAndRepay function (https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L115-L118).

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/MutualConsent.sol#L11-L68 https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L247-L262

Vulnerability details

Impact

Contracts that inherit from the MutualConsent contract, have access to a mutualConsent modifier.
Functions that use this modifier need consent from two parties to be called successfully.

Once one party has given consent for a function call, it cannot revoke consent.
This means that the other party can call this function at any time now.

This opens the door for several exploitation paths.
Most notably though the functions LineOfCredit.setRates(), LineOfCredit.addCredit() and LineOfCredit.increaseCredit() can cause problems.

One party can use Social Engineering to make the other party consent to multiple function calls and exploit the multiple consents.

Proof of Concept

1. A borrower and lender want to change the rates for a credit.
   The borrower wants to create the possibility for himself to change the rates in the future without the lender's consent.
2. The borrower and lender agree to set dRate and fRate to 5%.
3. The lender calls the LineOfCredit.setRates() function to give his consent.
4. The borrower might now say to the lender "Let's put the rate to 5.1% instead, I will give an extra 0.1%"
5. The borrower and lender now both call the LineOfCredit.setRates() function to set the rates to 5.1%.
6. The borrower can now set the rates to 5% at any time. E.g. they might increase the rates further in the future (the borrower playing by the rules)
   and at some point the borrower can decide to set the rates to 5%.

Links:
MutualConsent contract: https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/MutualConsent.sol

LineOfCredit.setRates() function: https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L247-L262

Tools Used

VSCode

Recommended Mitigation Steps

There are several options to fix this issue:

1. Add a function to the MutualConsent contract to revoke consent for a function call.
2. Make consent valid only for a certain amount of time.
3. Invalidate existing consents for a function when function is called with different arguments.

Option 3 requires a lot of additional bookkeeping but is probably the cleanest solution.

Lines of code

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

Vulnerability details

Impact

When transferring ETH into the Escrow, an amount must be specified.
However if msg.value > amount, then msg.value - amount Wei will be lost since ETH withdrawals (with Escrow.releaseCollateral or Escrow.liquidate function) are checked against deposited[token].amount and not against address(this).balance.

Proof of Concept

You can add the following test to the Escrow.t.sol file.
The call to the escrow.releaseCollateral function will fail, although the balance of the Escrow is sufficient.

function test_audit_can_remove_collateral_ETH_greater_amount() public {
    uint borrowerBalance = borrower.balance;
    uint escrowBalance = address(escrow).balance;

    escrow.addCollateral{value: mintAmount}(mintAmount - 1, Denominations.ETH);

    // This will cause a revert because the escrow thinks the balance of ETH is mintAmount -1
    escrow.releaseCollateral(mintAmount, Denominations.ETH, borrower);
}

Tools Used

VSCode

Recommended Mitigation Steps

If ETH is sent to the escrow, calculate self.deposited[token].amount += msg.value instead of self.deposited[token].amount += amount.

function addCollateral(EscrowState storage self, address oracle, uint256 amount, address token)
    external
    returns (uint256)
{
    require(amount > 0);
    if(!self.enabled[token])  { revert InvalidCollateral(); }

    LineLib.receiveTokenOrETH(token, msg.sender, amount);

    if (token == Denominations.ETH) {
        self.deposited[token].amount += msg.value;
    } else {
        self.deposited[token].amount += amount;
    }

    emit AddCollateral(token, amount);

    return _getLatestCollateralRatio(self, oracle);
}

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L93 https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotedLineLib.sol#L129-L137 https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotedLineLib.sol#L85

Vulnerability details

Impact

The SpigotedLine.claimAndRepay function (https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L93) can be called by the lender with an arbitrary zeroExTradeData argument that the SwapTarget contract is then called with (https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotedLineLib.sol#L129-L137).

Essentially the only restriction is that the amount of tokens bought cannot be zero (https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotedLineLib.sol#L85).

Therefore a lender can execute trades that are not in favor of the borrower.

Proof of Concept

E.g. a lender might have most of the credit repaid so there is no risk of loss of funds for the lender.

The lender can then execute a number of trades that incur high fees for the borrower thereby causing loss of funds for the borrower.
The exact structure of trades to maximize fees depends of course on the SwapTarget.

E.g. the SwapTarget might have a fixed fee.
So the lender can make trades very often. Always with a small amount of revenue.
The borrower would never do this. He would make one big trade and pay the fee once.

Tools Used

VSCode

Recommended Mitigation Steps

Allow only the borrower to make trades.

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/escrow/Escrow.sol#L86-L91 https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L87-L101

Vulnerability details

Impact

When transferring token to the Escrow contract by calling the Escrow.addCollateral (https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/escrow/Escrow.sol#L86-L91) function, the borrower can accidentally transfer ETH as well (msg.value > 0).
This ETH will not be registered by the Escrow (https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L87-L101) and cannot be withdrawn again.
It is lost.

Proof of Concept

1. Setup an Escrow and enable token1 as collateral.
2. The borrower calls Escrow.addCollateral to add token1 as collateral with msg.value = amount
3. The borrower cannot withdraw amount Wei from the Escrow and it is lost.

Tools Used

VSCode

Recommended Mitigation Steps

If the token argument does not equal Denominations.ETH, then add require(msg.value == 0, "Cannot send ETH and Token").

Lines of code

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

Vulnerability details

Impact

All accounting is done with the token amount that is sent, not the token amount that is received.
This means that fee-on-transfer tokens are not supported by the application.
This leads to accounting errors in the application if fee-on-transfer tokens are used anyway.

Proof of Concept

To prove this, I modified the ERC20.sol file in the openzeppelin library that is used in the tests and made it a fee-on-transfer token by modifying the _transfer function.

function _transfer(
    address from,
    address to,
    uint256 amount
) internal virtual {
    require(from != address(0), "ERC20: transfer from the zero address");
    require(to != address(0), "ERC20: transfer to the zero address");

    _beforeTokenTransfer(from, to, amount);

    uint256 fromBalance = _balances[from];
    require(fromBalance >= amount, "ERC20: transfer amount exceeds balance");
    unchecked {
        _balances[from] = fromBalance - amount;
        if (amount > 0) {
            _balances[to] = _balances[to] + amount - 1;
        } else {
            _balances[to] = _balances[to] + amount;
        }
    }

    emit Transfer(from, to, amount);

    _afterTokenTransfer(from, to, amount);
}

Now 82 out of 222 Tests will fail.

Tools Used

VSCode

Recommended Mitigation Steps

In case you don't want to support fee-on-transfer tokens, make it very clear which tokens are supported by your contracts and which are not.
In case you want to support fee-on-transfer tokens always work with the amount that is left after the transfer, meaning amountSent - fee.

Debt DAO Low Risk and Non-Critical Issues

Summary

[L-00] Missing zero address checks

There are 9 instances of missing zero address checks.
I will list all of them with a quick description of the impact.

_operator variable in Spigot constructor

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L37
If the operator is accidentally set to the zero address and the removeSpigot function is called, access to the RevenueContract will be lost.

_treasury variable in Spigot constructor

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L38
If the treasury is accidentally set to the zero address, revenue will be transferred to the zero address.
This means the revenue will be lost.

_owner variable in Spigot constructor

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L36
If the owner is accidentally set to the zero address, access to the Spigot will be lost.

operator variable in SpigotLib.removeSpigot function

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotLib.sol#L143
This function transfers ownership of the revenueContract to the operator.
If the operator is the zero address, access to the revenueContract is lost.

oracle_ variable in LineOfCredit constructor

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L50
If the oracle is set to the zero address, no token values can be calculated.

arbiter_ variable in LineOfCredit constructor

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L51
If the arbiter is set to the zero address, the LineOfCredit cannot be declared insolvent.

borrower_ variable in LineOfCredit constructor

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/LineOfCredit.sol#L52
The borrower should obviously not be the zero address.

_borrower variable in Escrow constructor

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/escrow/Escrow.sol#L46
If the borrower is set to the zero address collateral can be added but not released.

_oracle variable in Escrow constructor

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/escrow/Escrow.sol#L44
If the oracle is set to the zero address, no token values can be calculated.

[L-01] Comment / Docs not according to logic

Transfer ownership function

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L187-L189
The comment states that the "transfer ownership" function can be whitelisted.
But the SpigotLib.operate function does not allow to call the transferOwnerFunction: https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotLib.sol#L73

Update treasury function

In the documentation it is stated that the treasury is simply an address that receives funds and that it has "no ability to interact with the Spigot" (https://docs.debtdao.finance/products/the-spigot/spigot-roles/treasury).
However the treasury can call the Spigot.updateTreasury function (https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/spigot/Spigot.sol#L181) and set the treasury to a new address. So it can obviously interact with the Spigot.

[L-02] Implement 2-Step-Process for assigning roles

A 2-Step-Process should be used for assigning roles that are critical to the operation of the contract.
E.g. if a wrong owner is set in the SpigotLib contract, access to the contract is lost.

Instances:

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

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

For each instance of this issue, implement a updateRole function that sets a pendingRole address variable.
The pendingRole address must then call a claimRole function to actually claim the role.

[N-00] Unlocked pragma

Some of the files in scope use 0.8.9 as the Solidity compiler version. Others use ^0.8.9, meaning that the file will compile with versions >=0.8.9 and <0.9.0.
It is considered best practice to use a locked Solidity version, thereby only allowing compilation with a specific version.
So the instances of ^0.8.9 should be changed to 0.8.9.

There are 5 instances of this.

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

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

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

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

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

[N-01] Missing SPDX License Identifiers

It is best practice to include a SPDX License Identifier in each file.

The Solidity documentation (https://docs.soliditylang.org/en/v0.6.8/layout-of-source-files.html#spdx-license-identifier) states:

Trust in smart contract can be better established if their source code is available.
Since making source code available always touches on legal problems
with regards to copyright, the Solidity compiler encouranges the use of
machine-readable SPDX license identifiers.
Every source file should start with a comment indicating its license

Of all files in scope, only Oracle.sol includes one.
So a SPDX License Identifier should be added to all the remaining 17 files.

[N-02] Variable registry can be immutable

The registry variable in Oracle.sol is only assigned once in the constrcutor.
So it can be immutable.

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

[N-03] Function getLatestAnswer can be declared view

The function getLatestAnswer in Oracle.sol does not modify any state.
So it can be declared view.

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

[N-04] Event is missing indexed fields

Each event should use three indexed fields if it has three or more fields.

There are 8 instances of events that do not have 3 indexed fields.

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/interfaces/ISpigot.sol#L13

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

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

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

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

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

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

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

[N-05] Broken link in comment

The file MutualConsent.sol contains a comment with a link to the original file that this file is forked from.
However the link is broken and returns a 404 Not Found error code.

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

[N-06] Use scientific notation for powers of ten

1e4 instead of 10000
https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/interest-rate/InterestRateCredit.sol#L9

1e5 instead of 10**5
https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/EscrowLib.sol#L42

[N-07] Unreachable code

The return false; statement can never be reached since the line above is a revert.
https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/SpigotedLineLib.sol#L234

Debt DAO Gas Optimization Findings

Summary

The Gas savings are calculated using the forge test --gas-report command.
The same tests were used that are provided in the contest repository.

[G-00] Use functions instead of modifiers

Modifiers make code more elegant and readable but cost more Gas than functions.
Arguably, the additional Gas cost outweighs the readability benefit.

There are 6 instances where modifiers are defined.

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

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

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

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

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

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

[G-01] Save storage variable in memory

self.owner in SpigotLib.claimEscrow function

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

Function can be rewritten like this:

function claimEscrow(SpigotState storage self, address token)
    external
    returns (uint256 claimed) 
{
    address owner = self.owner;
    if(msg.sender != owner) { revert CallerAccessDenied(); }
    
    claimed = self.escrowed[token];

    if(claimed == 0) { revert ClaimFailed(); }

    LineLib.sendOutTokenOrETH(token, owner, claimed);

    self.escrowed[token] = 0; // keep 1 in escrow for recurring call gas optimizations?

    emit ClaimEscrow(token, claimed, owner);

    return claimed;
}

Method call Gas saved: 160

[G-02] Simplify formula for calculating interest

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

The formula for calculating interest in InterestRateCredit._accrueInterest currently is:

((rate.dRate * drawnBalance * timespan / INTEREST_DENOMINATOR) +
 (rate.fRate * (facilityBalance - drawnBalance) * timespan / INTEREST_DENOMINATOR))

This can be simplified to:

(rate.dRate * drawnBalance +
 rate.fRate * (facilityBalance - drawnBalance)) * timespan / INTEREST_DENOMINATOR

This means one multiplication with timespan and division by INTEREST_DENOMINATOR can be saved.

Deployment Gas saved: 2400
Method call Gas saved: 344

[G-03] Simplify formula for calculateValue function

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

The formula for calculating interest in CreditLib.calculateValue currently is:

price <= 0 ? 0 : (amount * uint(price)) / (1 * 10 ** decimals)

This can be simplified to:

price < 1 ? 0 : (amount * uint(price)) / (10 ** decimals)

Deployment Gas saved: 2203
Method call Gas saved: 95

[G-04] Use > instead of >= in healthcheck function

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

Change require(uint(status) >= uint( LineLib.STATUS.ACTIVE)); t0 require(uint(status) > uint( LineLib.STATUS.UNINITIALIZED));.

Deployment Gas saved: 400
Method call Gas saved: 3