Inverse Finance contest - delfin454000's results

QA Report - low risk

Missing checks for address(0x0) when assigning values to address state variables

---

Check for address(0x0) is missing for operator:

BorrowController.sol: L14

        operator = _operator;

Also DBR.sol: L39 and Oracle.sol: L32

---

Check for address(0x0) is missing for gov:

Fed.sol: L39

        gov = _gov;

Also Fed.sol: L50 and Market.sol: L77

---

Check for address(0x0) is missing for chair:

Fed.sol: L40

        chair = _chair;

Also Fed.sol: L68

---

Check for address(0x0) is missing for beneficiary:

GovTokenEscrow.sol: L34

        beneficiary = _beneficiary;

Also INVEscrow.sol: L48

---

Checks for address(0x0) are also missing for lender, pauseGuardian, and escrowImplementation:

Market.sol: L78-80

        lender = _lender;
        pauseGuardian = _pauseGuardian;
        escrowImplementation = _escrowImplementation;

---

---

QA Report - non-critical

Typos

---

DBR.sol: L60

    @param newReplenishmentPriceBps_ The new replen

Change replen to replenishment price or whatever was intended

---

Market.sol: L157

     At 5000, 50% of of a borrower's underwater debt can be liquidated. Only callable by governance.

Remove repeated word of

---

Market.sol: L201

    @param amount The amount od DOLA to recall to the the lender.

Remove repeated word the

---

Market.sol: L589

    @param repaidDebt Th amount of user user debt to liquidate.

Change Th to The and remove repeated word user

---

BorrowController.sol: L36

    @param deniedContract The addres of the denied contract

Change addres to address

---

DBR.sol: L50

    @notice Sets pending operator of the contract. Operator role must be claimed by the new oprator. Only callable by Operator.

Change oprator to operator

---

DBR.sol: L87

    @notice Removes a minter from the set of addresses allowe to mint DBR tokens. Only callable by Operator.

Change allowe to allowed

---

DBR.sol: L128

    @notice Get the DBR deficit of an address. Will return 0 if th user has zero DBR or more.

Change th to the

---

Market.sol: L146

    @dev Collateral factor mus be set below 100%

Change mus to must

---

Market.sol: L181

    @param _liquidationIncentiveBps The new liqudation incentive set in basis points. 1 = 0.01% 

Change liqudation to liquidation

---

Market.sol: L575

    @notice View function for getting the amount of liquidateable debt a user holds.

Change liquidateable to liquidatable

---

---

Long single-line comments

In theory, comments over 80 characters should wrap using multi-line comment syntax. Even if somewhat longer comments (up to 120 characters) are acceptable and a scroll bar is provided, there are cases where very long comments interfere with readability. Below are five instances of extra-long comments (or groups of comments) whose readability could be improved, as shown:

---

Fed.sol: L99

    @dev Markets can have more DOLA in them than they've been supplied, due to force replenishes. This call will revert if trying to contract more than have been supplied.

Suggestion:

    @dev Markets can have more DOLA in them than they've been supplied, due to force replenishes. 
     This call will revert if trying to contract more than have been supplied.

---

Oracle.sol: L9-15

/**
@title Oracle
@notice Oracle used by markets. Can use both fixed price feeds and Chainlink-style feeds for prices.
The Pessimistic Oracle introduces collateral factor into the pricing formula. It ensures that any given oracle price is dampened to prevent borrowers from borrowing more than the lowest recorded value of their collateral over the past 2 days.
This has the advantage of making price manipulation attacks more difficult, as an attacker needs to log artificially high lows.
It has the disadvantage of reducing borrow power of borrowers to a 2-day minimum value of their collateral, where the value must have been seen by the oracle.
*/

Suggestion:

/**
@title Oracle
@notice Oracle used by markets. Can use both fixed price feeds and Chainlink-style feeds for prices.
 The Pessimistic Oracle introduces collateral factor into the pricing formula. 
 It ensures that any given oracle price is dampened to prevent borrowers from
 borrowing more than the lowest recorded value of their collateral over the past 2 days. 
 This has the advantage of making price manipulation attacks more difficult,
 as an attacker needs to log artificially high lows. 
 It has the disadvantage of reducing borrow power of borrowers to a 2-day minimum
 value of their collateral, where the value must have been seen by the oracle.
*/

---

GovTokenEscrow.sol: L16

 This specific escrow is meant as an example of how an escrow can be implemented that allows depositors to delegate votes with their collateral, unlike pooled deposit protocols.

Suggestion:

 This specific escrow is meant as an example of how an escrow can be implemented that
 allows depositors to delegate votes with their collateral, unlike pooled deposit protocols.

---

INVEscrow.sol: L25

 This escrow allows user to deposit INV collateral directly into the xINV contract, earning APY and allowing them to delegate votes on behalf of the xINV collateral

Suggestion:

 This escrow allows user to deposit INV collateral directly into the xINV contract,
 earning APY and allowing them to delegate votes on behalf of the xINV collateral.

---

INVEscrow.sol: L62

        if(invBalance < amount) xINV.redeemUnderlying(amount - invBalance); // we do not check return value because next call will fail if this fails anyway

Suggestion:

        // We do not check return value below because next call will fail if this fails anyway
        if(invBalance < amount) xINV.redeemUnderlying(amount - invBalance);

---

---

Open TODOs and other open items

Comments that appear to refer to open items should be addressed.

---

GovTokenEscrow.sol: L56

    /* Uncomment if Escrow contract should handle on deposit callbacks. This function should remain callable by anyone to handle direct inbound transfers.

Similarly for SimpleERC20Escrow.sol: L49

---

INVEscrow.sol: L35

        xINV = _xINV; // TODO: Test whether an immutable variable will persist across proxies

---

---

Missing NatSpec

NatSpec statements are entirely missing for all constructors and the four functions referenced below:

DBR.sol: L262-264

    function DOMAIN_SEPARATOR() public view virtual returns (bytes32) {
        return block.chainid == INITIAL_CHAIN_ID ? INITIAL_DOMAIN_SEPARATOR : computeDomainSeparator();
    }

Similary, Market.sol: L97-99

---

DBR.sol: L266-277

    function computeDomainSeparator() internal view virtual returns (bytes32) {
        return
            keccak256(
                abi.encode(
                    keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
                    keccak256(bytes(name)),
                    keccak256("1"),
                    block.chainid,
                    address(this)
                )
            );
    }

Similary, Market.sol: L101-112

---

---

Missing individual NatSpec statements

The following functions are missing @returnstatements

---

Market.sol: L221-226

    /**
    @notice Internal function for creating an escrow for users to deposit collateral in.
    @dev Uses create2 and minimal proxies to create the escrow at a deterministic address
    @param user The address of the user to create an escrow for.
    */
    function createEscrow(address user) internal returns (IEscrow instance) {

Missing: @return

Similarly for the following functions:

Market.sol: L240-245

Market.sol: L287-292

Market.sol: L308-312

Market.sol: L318-323

Market.sol: L329-334

Market.sol: L339-344

Market.sol: L348-353

Market.sol: L365-370

Market.sol: L574-578

The following functions are missing @paramstatements

Oracle.sol: L73-78

    /**
    @notice Gets the price of a specific token in DOLA
    @param token The address of the token to get price of
    @return The price of the token in DOLA, adjusted for token and feed decimals
    */
    function viewPrice(address token, uint collateralFactorBps) external view returns (uint) {

Missing: @param collateralFactorBps

Similarly for Oracle.sol: L107-112

---

---

Missing require revert string

A require message should be included to enable users to understand the reason for failure.

Recommendation: Add a brief (<= 32 char) explanatory message to each of the requires referenced below. Consider using Custom Error codes (which would save both deployment and runtime cost and therefore be cheaper than revert strings).

---

Fed.sol: L93

        require(globalSupply <= supplyCeiling);

---

GovTokenEscrow.sol: L67

        require(msg.sender == beneficiary);

Similarly for INVEscrow.sol: L91

---

---

Event is missing indexed fields

Each event referenced below should use three indexed fields if there are at least three fields

---

DBR.sol: L381-382

    event Transfer(address indexed from, address indexed to, uint256 amount);
    event Approval(address indexed owner, address indexed spender, uint256 amount);

---

Market.sol: L616-619

    event Withdraw(address indexed account, address indexed to, uint amount);
    event Repay(address indexed account, address indexed repayer, uint amount);
    event ForceReplenish(address indexed account, address indexed replenisher, uint deficit, uint replenishmentCost, uint replenisherReward);
    event Liquidate(address indexed account, address indexed liquidator, uint repaidDebt, uint liquidatorReward);

---

---