Inverse Finance contest - 0x1f8b's results

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L82 https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Oracle.sol#L116

Vulnerability details

Impact

According to Chainlink's documentation, the latestAnswer function is deprecated.

Proof of Concept

This function does not error if no answer has been reached but returns 0. Besides, the latestAnswer is reported with 18 decimals for crypto quotes but 8 decimals for FX quotes (See Chainlink FAQ for more details). And also, is not possible to check if the price is fresh.

Affected source code:

• Oracle.sol:82
• Oracle.sol:116

Recommended Mitigation Steps

• Use V3 interface functions: https://docs.chain.link/docs/price-feeds-api-reference/
• Use the latestRoundData function to get the price instead. Add checks on the return data with proper revert messages if the price is stale or the round is uncomplete, for example:

(uint80 roundID, int256 price, , uint256 timeStamp, uint80 answeredInRound) = feeds[token].feed.latestRoundData();
require(answeredInRound >= roundID, "...");
require(timeStamp != 0, "...");

• Low
  • 1. Allows malleable SECP256K1 signatures
  • 2. Lack of checks address0
  • 3. Avoid using tx.origin
  • 4. Mixing and Outdated compiler
  • 5. Lack of ACK during owner change
  • 6. Market pause is not checked during contraction
  • 7. Lack of no reentrant modifier
  • 8. Lack of checks the integer ranges
  • 9. Lack of checks supportsInterface
  • 10. Lack of event emit
  • 11. Oracle not compatible with tokens of 19 or more decimals
  • 12. Wrong visibility
• Non critical
  • 13. Bad nomenclature
  • 14. Open TODO
  • 15. Avoid duplicate code
  • 16. Avoid hardcoded values

Low

1. Allows malleable SECP256K1 signatures

Here, the ecrecover() method doesn't check the s range.

Homestead (EIP-2) added this limitation, however the precompile remained unaltered. The majority of libraries, including OpenZeppelin, do this check.

Since an order can only be confirmed once and its hash is saved, there doesn't seem to be a serious danger in existing use cases.

Reference:

• https://github.com/OpenZeppelin/openzeppelin-contracts/blob/7201e6707f6631d9499a569f492870ebdd4133cf/contracts/utils/cryptography/ECDSA.sol#L138-L149

Affected source code:

• DBR.sol:226-248
• Market.sol:425-447
• Market.sol:489-511

2. Lack of checks address(0)

The following methods have a lack of checks if the received argument is an address, it's good practice in order to reduce human error to check that the address specified in the constructor or initialize is different than address(0).

Affected source code:

• BorrowController.sol:14
• BorrowController.sol:26
• SimpleERC20Escrow.sol:28
• GovTokenEscrow.sol:33-34
• INVEscrow.sol:35
• INVEscrow.sol:47-48
• Fed.sol:37-40
• Fed.sol:50
• Fed.sol:68
• Oracle.sol:32
• Oracle.sol:44
• DBR.sol:39
• DBR.sol:54
• Market.sol:77-83
• Market.sol:130
• Market.sol:136
• Market.sol:142

3. Avoid using tx.origin

tx.origin is a global variable in Solidity that returns the address of the account that sent the transaction.

Using the variable could make a contract vulnerable if an authorized account calls a malicious contract. You can impersonate a user using a third party contract.

This can make it easier to create a vault on behalf of another user with an external administrator (by receiving it as an argument).

Affected source code:

• BorrowController.sol:47

4. Mixing and Outdated compiler

The pragma version used are:

pragma solidity ^0.8.13;

Note that mixing pragma is not recommended. Because different compiler versions have different meanings and behaviors, it also significantly raises maintenance costs. As a result, depending on the compiler version selected for any given file, deployed contracts may have security issues.

The minimum required version must be 0.8.17; otherwise, contracts will be affected by the following important bug fixes:

0.8.14:

• ABI Encoder: When ABI-encoding values from calldata that contain nested arrays, correctly validate the nested array length against calldatasize() in all cases.
• Override Checker: Allow changing data location for parameters only when overriding external functions.

0.8.15

• Code Generation: Avoid writing dirty bytes to storage when copying bytes arrays.
• Yul Optimizer: Keep all memory side-effects of inline assembly blocks.

0.8.16

• Code Generation: Fix data corruption that affected ABI-encoding of calldata values represented by tuples: structs at any nesting level; argument lists of external functions, events and errors; return value lists of external functions. The 32 leading bytes of the first dynamically-encoded value in the tuple would get zeroed when the last component contained a statically-encoded array.

0.8.17

• Yul Optimizer: Prevent the incorrect removal of storage writes before calls to Yul functions that conditionally terminate the external EVM call.

Apart from these, there are several minor bug fixes and improvements.

5. Lack of ACK during owner change

It's possible to lose the ownership under specific circumstances.

Because of human error it's possible to set a new invalid owner. When you want to change the owner's address it's better to propose a new owner, and then accept this ownership with the new wallet.

Affected source code:

• Fed.sol:50
• Market.sol:130

6. Market pause is not checked during contraction

In the Fed contract, during the expansion method is checked that the market is not paused, this requirement is not done during the contraction.

    function contraction(IMarket market, uint amount) public {
        require(msg.sender == chair, "ONLY CHAIR");
        require(dbr.markets(address(market)), "UNSUPPORTED MARKET");
+       require(!market.borrowPaused(), "CANNOT EXPAND PAUSED MARKETS");
        uint supply = supplies[market];
        require(amount <= supply, "AMOUNT TOO BIG"); // can't burn profits
        market.recall(amount);
        dola.burn(amount);
        supplies[market] -= amount;
        globalSupply -= amount;
        emit Contraction(market, amount);
    }

Affected source code:

• Fed.sol:105

7. Lack of no reentrant modifier

The Market.getEscrow, Fed.expansion and Fed.contraction methods do not have the noReentrant modifier and make calls to an external contract that can take advantage of and call these methods again, but it seems to fail due to the lack of tokens.

However, if any of the other addresses used their receive event to provide liquidity to the contract, the attacking account could benefit from it.

-   function expansion(IMarket market, uint amount) public {
+   function expansion(IMarket market, uint amount) public noReentrant {
        ...
    }

-   function contraction(IMarket market, uint amount) public {
+   function contraction(IMarket market, uint amount) public noReentrant {
        ...
    }

For example, in getEscrow if the escrow allows a callback, it could create two scrows, loosing funds if in this callback it will call again getEscrow, using for example deposit

    function getEscrow(address user) internal returns (IEscrow) {
        if(escrows[user] != IEscrow(address(0))) return escrows[user];
        IEscrow escrow = createEscrow(user);
        escrow.initialize(collateral, user);
        escrows[user] = escrow;
        return escrow;
    }

• Bob call deposit.
• During the escrow initialization it happend a reentrancy and call again deposit.
• The first deposit will be loss in the first escrow.

Please note that current escrows do not allow re-entry, so I decided to use Low. It's always good to change the storage flags before the externals calls.

Affected source code:

• Fed.sol:86
• Fed.sol:103
• Market.sol:245

8. Lack of checks the integer ranges

The following methods lack checks on the following integer arguments, you can see the recommendations above.

Affected source code:

_replenishmentPriceBps is not checked to be != 0 during the constructor, nevertheless it's checked in setReplenishmentPriceBps

• DBR.sol:36

replenishmentIncentiveBps is not checked to be > 0 during the constructor, nevertheless it's checked in setReplenismentIncentiveBps

• Market.sol:76

9. Lack of checks supportsInterface

The EIP-165 standard helps detect that a smart contract implements the expected logic, prevents human error when configuring smart contract bindings, so it is recommended to check that the received argument is a contract and supports the expected interface.

Reference:

• https://eips.ethereum.org/EIPS/eip-165

Affected source code:

• DBR.sol:99
• Market.sol:81-83
• Market.sol:118
• Market.sol:124

10. Lack of event emit

The Market.pauseBorrows, Market.setLiquidationFeeBps, Market.setLiquidationIncentiveBps, Market.setReplenismentIncentiveBps, Market.setLiquidationFactorBps, Market.setCollateralFactorBps, Market.setBorrowController, Market.setOracle methods do not emit an event when the state changes, something that it's very important for dApps and users.

Affected source code:

• Market.sol:118
• Market.sol:124
• Market.sol:149
• Market.sol:161
• Market.sol:172
• Market.sol:183
• Market.sol:194
• Market.sol:218

11. Oracle not compatible with tokens of 19 or more decimals

Keep in mind that the version of solidity used, despite being greater than 0.8, does not prevent integer overflows during casting, it only does so in mathematical operations.

In the case that feed.decimals() returns 18, and the token is more than 18 decimals, the following subtraction will cause an underflow, denying the oracle service.

    uint8 feedDecimals = feeds[token].feed.decimals();  // 18 => [ETH/DAI] https://rinkeby.etherscan.io/address/0x74825dbc8bf76cc4e9494d0ecb210f676efa001d#readContract
    uint8 tokenDecimals = feeds[token].tokenDecimals;   // > 18
    uint8 decimals = 36 - feedDecimals - tokenDecimals; // overflow

All pairs have 8 decimals except the ETH pairs, so a token with 19 decimals in ETH, will fault.

Affected source code:

• Oracle.sol:87-98

12. Wrong visibility

The method accrueDueTokens doesn't check that the call is made by a market, and it's public, it should be changed to internal or private to be more resilient.

require(markets[msg.sender], "Only markets can call onBorrow");

Affected source code:

• DBR.sol:284

---

Non critical

13. Bad nomenclature

The interface IERC20 contains two methdos that are not pressent in the official ERC20, delegate and delegates, it's recommended to change the name of the contract because not any ERC20 it's valid.

Affected source code:

• GovTokenEscrow.sol:9-10
• INVEscrow.sol:10-11

14. Open TODO

The code that contains "open todos" reflects that the development is not finished and that the code can change a posteriori, prior release, with or without audit.

Affected source code:

// TODO: Test whether an immutable variable will persist across proxies

• INVEscrow.sol:35

15. Avoid duplicate code

The viewPrice and getPrice methods of the Oracle contract are very similar, the only difference being the following peace of code:

            if(todaysLow == 0 || normalizedPrice < todaysLow) {
                dailyLows[token][day] = normalizedPrice;
                todaysLow = normalizedPrice;
                emit RecordDailyLow(token, normalizedPrice);
            }

It's recommended to reuse the code in order to be more readable and light.

Affected source code:

• Oracle.sol:126-130
• Oracle.sol:79-103

16. Avoid hardcoded values

It is not good practice to hardcode values, but if you are dealing with addresses much less, these can change between implementations, networks or projects, so it is convenient to remove these values from the source code.

Affected source code:

• Market.sol:44

It's recommended to create a factor variable for 10000:

• Market.sol:74-76
• Market.sol:150
• Market.sol:162
• Market.sol:173
• Market.sol:184
• Market.sol:195
• Market.sol:336
• Market.sol:346
• Market.sol:360
• Market.sol:377
• Market.sol:563-564
• Market.sol:583
• Market.sol:595-606

• Gas
  • 1. Reduce boolean comparison
  • 2. Avoid compound assignment operator in state variables
    • Total gas saved: 13 * 25 = 325
  • 3. Reduce the size of error messages Long revert Strings
    • Total gas saved: 18 * 4 = 72
  • 4. Avoid unused returns
  • 5. delete optimization
    • Total gas saved: 5 * 2 = 10
  • 6. Use the unchecked keyword
  • 7. Remove unnecessary variables
  • 8. Avoid duplicate code
  • 9. Gas saving using variable cache
  • 10. Avoid the call Market.predictEscrow
  • 11. Optimize DBR.accrueDueTokens

Gas

1. Reduce boolean comparison

Compare a boolean value using == true or == false instead of using the boolean value is more expensive.

NOT opcode it's cheaper than using EQUAL or NOTEQUAL when the value it's false, or just the value without == true when it's true, because it will use less opcodes inside the VM.

Proof of concept (without optimizations):

pragma solidity 0.8.16;

contract TesterA {
function testEqual(bool a) public view returns (bool) { return a == true; }
}

contract TesterB {
function testNot(bool a) public view returns (bool) { return a; }
}

Gas saving executing: 18 per entry for == true

TesterA.testEqual:   21814
TesterB.testNot:     21796   

pragma solidity 0.8.16;

contract TesterA {
function testEqual(bool a) public view returns (bool) { return a == false; }
}

contract TesterB {
function testNot(bool a) public view returns (bool) { return !a; }
}

Gas saving executing: 15 per entry for == false

TesterA.testEqual:   21814
TesterB.testNot:     21799

Affected source code:

Use the value instead of != true:

• DBR.sol:350
• Fed.sol:89

2. Avoid compound assignment operator in state variables

Using compound assignment operators for state variables (like State += X or State -= X ...) it's more expensive than using operator assignment (like State = State + X or State = State - X ...).

Proof of concept (without optimizations):

pragma solidity 0.8.15;

contract TesterA {
uint private _a;
function testShort() public {
_a += 1;
}
}

contract TesterB {
uint private _a;
function testLong() public {
_a = _a + 1;
}
}

Gas saving executing: 13 per entry

TesterA.testShort: 43507
TesterB.testLong:  43494

Affected source code:

• DBR.sol:172
• DBR.sol:174
• DBR.sol:196
• DBR.sol:198
• DBR.sol:288
• DBR.sol:289
• DBR.sol:304
• DBR.sol:316
• DBR.sol:332
• DBR.sol:360
• DBR.sol:362
• DBR.sol:374
• DBR.sol:376
• Fed.sol:91
• Fed.sol:92
• Fed.sol:110
• Fed.sol:111
• Market.sol:395
• Market.sol:397
• Market.sol:534
• Market.sol:535
• Market.sol:565
• Market.sol:568
• Market.sol:599
• Market.sol:600

Total gas saved: 13 * 25 = 325

3. Reduce the size of error messages (Long revert Strings)

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition is met.

Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.

Proof of concept (without optimizations):

pragma solidity 0.8.15;

contract TesterA {
function testShortRevert(bool path) public view {
require(path, "test error");
}
}

contract TesterB {
function testLongRevert(bool path) public view {
require(path, "test big error message, more than 32 bytes");
}
}

Gas saving executing: 18 per entry

TesterA.testShortRevert: 21886
TesterB.testLongRevert:  21904

Affected source code:

• DBR.sol:63
• DBR.sol:326
• Market.sol:76
• Market.sol:214

Total gas saved: 18 * 4 = 72

4. Avoid unused returns

Having a method that always returns the same value is not correct in terms of consumption, if you want to modify a value, and the method will perform a revert in case of failure, it is not necessary to return a true, since it will never be false. It is less expensive not to return anything, and it also eliminates the need to double-check the returned value by the caller.

Affected source code:

• DBR.sol:158-162
• DBR.sol:170-178
• DBR.sol:188-202

5. delete optimization

Use delete instead of set to default value (false or 0).

5 gas could be saved per entry in the following affected lines:

Affected source code:

• BorrowController.sol:38
• DBR.sol:91

Total gas saved: 5 * 2 = 10

6. Use the unchecked keyword

When an underflow or overflow cannot occur, one might conserve gas by using the unchecked keyword to prevent unnecessary arithmetic underflow/overflow tests.

• The subtraction has already been checked to be safe before.

    function contraction(IMarket market, uint amount) public {
        require(msg.sender == chair, "ONLY CHAIR");
        require(dbr.markets(address(market)), "UNSUPPORTED MARKET");
        uint supply = supplies[market];
        require(amount <= supply, "AMOUNT TOO BIG"); // can't burn profits
        market.recall(amount);
        dola.burn(amount);
+       unckeched {
        supplies[market] -= amount;
        globalSupply -= amount;
+       }
        emit Contraction(market, amount);
    }

• The subtraction has already been checked to be safe before.

    function getProfit(IMarket market) public view returns (uint) {
        uint marketValue = dola.balanceOf(address(market)) + market.totalDebt();
        uint supply = supplies[market];
        if(supply >= marketValue) return 0;
+       unckeched {
        return marketValue - supply;
+       }
    }

• If globalSupply don't overflow, supplies also won't overflow.

    function expansion(IMarket market, uint amount) public {
        require(msg.sender == chair, "ONLY CHAIR");
        require(dbr.markets(address(market)), "UNSUPPORTED MARKET");
        require(market.borrowPaused() != true, "CANNOT EXPAND PAUSED MARKETS");
        dola.mint(address(market), amount);
+       globalSupply += amount;
+       unckeched {
        supplies[market] += amount;
+       }
-       globalSupply += amount;
        require(globalSupply <= supplyCeiling);
        emit Expansion(market, amount);
    }

• The subtraction has already been checked to be safe before.

    function totalSupply() public view returns (uint) {
-       if(totalDueTokensAccrued > _totalSupply) return 0;
+       if(totalDueTokensAccrued >= _totalSupply) return 0;
+       unckeched {
        return _totalSupply - totalDueTokensAccrued;
+       }
    }

• The subtraction has already been checked to be safe before, also it could be saved some math operations caching with a variable.

    function balanceOf(address user) public view returns (uint) {
        uint debt = debts[user];
        uint accrued = (block.timestamp - lastUpdated[user]) * debt / 365 days;
-       if(dueTokensAccrued[user] + accrued > balances[user]) return 0;
-       return balances[user] - dueTokensAccrued[user] - accrued;
+       accrued += dueTokensAccrued[user];
+       if(accrued >= balances[user]) return 0;
+       unckeched {
+           return balances[user] - accrued;
+       }
    }

• The subtraction has already been checked to be safe before, also it could be saved some math operations caching with a variable.

    function deficitOf(address user) public view returns (uint) {
        uint debt = debts[user];
        uint accrued = (block.timestamp - lastUpdated[user]) * debt / 365 days;
-       if(dueTokensAccrued[user] + accrued < balances[user]) return 0;
-       return dueTokensAccrued[user] + accrued - balances[user];
+       accrued += dueTokensAccrued[user];
+       if(accrued <= balances[user]) return 0;
+       unckeched {
+           return accrued - balances[user];
+       }
    }

• If totalDueTokensAccrued don't overflow, dueTokensAccrued also won't overflow.

    function accrueDueTokens(address user) public {
        uint debt = debts[user];
        if(lastUpdated[user] == block.timestamp) return;
        uint accrued = (block.timestamp - lastUpdated[user]) * debt / 365 days;
+       totalDueTokensAccrued += accrued;
+       unckeched {
        dueTokensAccrued[user] += accrued;
+       }
-       totalDueTokensAccrued += accrued;
        lastUpdated[user] = block.timestamp;
        emit Transfer(user, address(0), accrued);
    }

• The subtraction has already been checked to be safe before, also it could be saved some math operations caching with a variable.

    function getWithdrawalLimitInternal(address user) internal returns (uint) {
        IEscrow escrow = predictEscrow(user);
        uint collateralBalance = escrow.balance();
        if(collateralBalance == 0) return 0;
        uint debt = debts[user];
        if(debt == 0) return collateralBalance;
        if(collateralFactorBps == 0) return 0;
        uint minimumCollateral = debt * 1 ether / oracle.getPrice(address(collateral), collateralFactorBps) * 10000 / collateralFactorBps;
        if(collateralBalance <= minimumCollateral) return 0;
+       unckeched {
        return collateralBalance - minimumCollateral;
+       }
    }

• The subtraction has already been checked to be safe before, also it could be saved some math operations caching with a variable.

    function getWithdrawalLimit(address user) public view returns (uint) {
        IEscrow escrow = predictEscrow(user);
        uint collateralBalance = escrow.balance();
        if(collateralBalance == 0) return 0;
        uint debt = debts[user];
        if(debt == 0) return collateralBalance;
        if(collateralFactorBps == 0) return 0;
        uint minimumCollateral = debt * 1 ether / oracle.viewPrice(address(collateral), collateralFactorBps) * 10000 / collateralFactorBps;
        if(collateralBalance <= minimumCollateral) return 0;
+       unckeched {
        return collateralBalance - minimumCollateral;
+       }
    }

• If debts[borrower] don't overflow, totalDebt also won't overflow.

    function borrowInternal(address borrower, address to, uint amount) internal {
        require(!borrowPaused, "Borrowing is paused");
        if(borrowController != IBorrowController(address(0))) {
            require(borrowController.borrowAllowed(msg.sender, borrower, amount), "Denied by borrow controller");
        }
        uint credit = getCreditLimitInternal(borrower);
        debts[borrower] += amount;
        require(credit >= debts[borrower], "Exceeded credit limit");
+       unckeched {
        totalDebt += amount;
+       }
        dbr.onBorrow(borrower, amount);
        dola.transfer(to, amount);
        emit Borrow(borrower, amount);
    }

• The subtraction has already been checked to be safe before, also it could be saved some math operations caching with a variable.

    function repay(address user, uint amount) public {
-       uint debt = debts[user];
-       require(debt >= amount, "Insufficient debt");
+       require(debts[user] >= amount, "Insufficient debt");
+       unckeched {
        debts[user] -= amount;
        totalDebt -= amount;
+       }
        dbr.onRepay(user, amount);
        dola.transferFrom(msg.sender, address(this), amount);
        emit Repay(user, msg.sender, amount);
    }

• The subtraction has already been checked to be safe before, also it could be saved some math operations caching with a variable.

    function liquidate(address user, uint repaidDebt) public {
        require(repaidDebt > 0, "Must repay positive debt");
        uint debt = debts[user];
        require(getCreditLimitInternal(user) < debt, "User debt is healthy");
        require(repaidDebt <= debt * liquidationFactorBps / 10000, "Exceeded liquidation factor");
        uint price = oracle.getPrice(address(collateral), collateralFactorBps);
        uint liquidatorReward = repaidDebt * 1 ether / price;
        liquidatorReward += liquidatorReward * liquidationIncentiveBps / 10000;
+       unckeched {
        debts[user] -= repaidDebt;
        totalDebt -= repaidDebt;
+       }
        dbr.onRepay(user, repaidDebt);
        dola.transferFrom(msg.sender, address(this), repaidDebt);
        IEscrow escrow = predictEscrow(user);
        escrow.pay(msg.sender, liquidatorReward);
        if(liquidationFeeBps > 0) {
            uint liquidationFee = repaidDebt * 1 ether / price * liquidationFeeBps / 10000;
            if(escrow.balance() >= liquidationFee) {
                escrow.pay(gov, liquidationFee);
            }
        }
        emit Liquidate(user, msg.sender, repaidDebt, liquidatorReward);
    }

Affected source code:

• Fed.sol:103-113
• Fed.sol:124
• Fed.sol:91-92
• DBR.sol:109-112
• DBR.sol:120-125
• DBR.sol:133-138
• DBR.sol:284-292
• Market.sol:362
• Market.sol:379
• Market.sol:397
• Market.sol:531-539
• Market.sol:599-600

7. Remove unnecessary variables

The following state variables can be removed without affecting the logic of the contract since they are not used and/or are redundant because they could be used inline.

    function contraction(IMarket market, uint amount) public {
        require(msg.sender == chair, "ONLY CHAIR");
        require(dbr.markets(address(market)), "UNSUPPORTED MARKET");
-       uint supply = supplies[market];
-       require(amount <= supply, "AMOUNT TOO BIG"); // can't burn profits
+       require(amount <= supplies[market], "AMOUNT TOO BIG"); // can't burn profits
        market.recall(amount);
        dola.burn(amount);
        supplies[market] -= amount;
        globalSupply -= amount;
        emit Contraction(market, amount);
    }

Affected source code:

• Fed.sol:106-107

8. Avoid duplicate code

The viewPrice and getPrice methods of the Oracle contract are very similar, the only difference being the following peace of code:

            if(todaysLow == 0 || normalizedPrice < todaysLow) {
                dailyLows[token][day] = normalizedPrice;
                todaysLow = normalizedPrice;
                emit RecordDailyLow(token, normalizedPrice);
            }

It's recommended to reuse the code in order to be more readable and smaller for deployment.

Affected source code:

• Oracle.sol:126-130
• Oracle.sol:79-103

9. Gas saving using variable cache

    function getEscrow(address user) internal returns (IEscrow) {
+       IEscrow escrow = escrows[user];
+       if(escrow != IEscrow(address(0))) return escrow;
-       if(escrows[user] != IEscrow(address(0))) return escrows[user];
-       IEscrow escrow = createEscrow(user);
+       escrow = createEscrow(user);
+       escrows[user] = escrow;
        escrow.initialize(collateral, user);
-       escrows[user] = escrow;
        return escrow;
    }

Affected source code:

• Market.sol:245-251

10. Avoid the call Market.predictEscrow

The call to predictEscrow supposes an unnecessary expense in the following methods, this is because in the case of not existing in the mapping escrows[user] will not have been created yet, and the balance or pay methods cannot be called if was not created.

It's recommended to use the stored value escrows[user].

Affected source code:

• Market.sol:313
• Market.sol:324
• Market.sol:354
• Market.sol:371
• Market.sol:603

11. Optimize DBR.accrueDueTokens

The method DBR.accrueDueTokens doesn't check that the call is made by a market, and it's public, it should be changed to internal or private to save gas, and move the debt variable to avoid waste gas when it's fresh.

    function accrueDueTokens(address user) internal {
-       uint debt = debts[user];
        if(lastUpdated[user] == block.timestamp) return;
+       uint debt = debts[user];
        uint accrued = (block.timestamp - lastUpdated[user]) * debt / 365 days;
        dueTokensAccrued[user] += accrued;
        totalDueTokensAccrued += accrued;
        lastUpdated[user] = block.timestamp;
        emit Transfer(user, address(0), accrued);
    }

Affected source code:

• DBR.sol:284