Inverse Finance contest - cryptostellar5's results

L-01 NO EVENT IS RAISED

All important functions wherever some change is happening need to raise events

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol

53: function setPendingOperator(address newOperator_) public onlyOperator {
62: function setReplenishmentPriceBps(uint newReplenishmentPriceBps_) public onlyOperator {

L-02 FLOATING PRAGMA VERSION SHOULD NOT BE USED

In the contracts, floating pragmas should not be used. Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

This is applicable in all the smart contracts

pragma solidity ^0.8.13;

N-01 INTERFACES SHOULD BE MOVED TO SEPARATE FILES

Most of the other interfaces in this project are in their own file in the interfaces directory. The interfaces below do not follow this pattern

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol

interface IMarket { 
    function recall(uint amount) external;
    function totalDebt() external view returns (uint);
    function borrowPaused() external view returns (bool);
}

interface IDola { 
    function mint(address to, uint amount) external;
    function burn(uint amount) external;
    function balanceOf(address user) external view returns (uint);
    function transfer(address to, uint amount) external returns (bool);
}

interface IDBR {
    function markets(address) external view returns (bool);
}

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol

interface IERC20 {
    function transfer(address,uint) external returns (bool);
    function transferFrom(address,address,uint) external returns (bool);
    function balanceOf(address) external view returns (uint);
}

interface IOracle {
    function getPrice(address,uint) external returns (uint);
    function viewPrice(address,uint) external view returns (uint);
}

interface IEscrow {
    function initialize(IERC20 _token, address beneficiary) external;
    function onDeposit() external;
    function pay(address recipient, uint amount) external;
    function balance() external view returns (uint);
}

interface IDolaBorrowingRights {
    function onBorrow(address user, uint additionalDebt) external;
    function onRepay(address user, uint repaidDebt) external;
    function onForceReplenish(address user, uint amount) external;
    function balanceOf(address user) external view returns (uint);
    function deficitOf(address user) external view returns (uint);
    function replenishmentPriceBps() external view returns (uint);
}

interface IBorrowController {
    function borrowAllowed(address msgSender, address borrower, uint amount) external returns (bool);
}

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol

interface IChainlinkFeed {
    function decimals() external view returns (uint8);
    function latestAnswer() external view returns (uint);
}

https://github.com/code-423n4/2022-10-inverse/blob/main/src/escrows/GovTokenEscrow.sol

interface IERC20 {
    function transfer(address,uint) external returns (bool);
    function transferFrom(address,address,uint) external returns (bool);
    function balanceOf(address) external view returns (uint);
    function delegate(address delegatee) external;
    function delegates(address delegator) external view returns (address delegatee);
}

https://github.com/code-423n4/2022-10-inverse/blob/main/src/escrows/INVEscrow.sol

interface IERC20 {
    function transfer(address,uint) external returns (bool);
    function transferFrom(address,address,uint) external returns (bool); 
    function balanceOf(address) external view returns (uint);
    function approve(address, uint) external view returns (uint);
    function delegate(address delegatee) external;
    function delegates(address delegator) external view returns (address delegatee);
}

interface IXINV {
    function balanceOf(address) external view returns (uint);
    function exchangeRateStored() external view returns (uint);
    function mint(uint mintAmount) external returns (uint);
    function redeemUnderlying(uint redeemAmount) external returns (uint);
    function syncDelegate(address user) external;
}

https://github.com/code-423n4/2022-10-inverse/blob/main/src/escrows/SimpleERC20Escrow.sol

interface IERC20 {
    function transfer(address,uint) external returns (bool);
    function transferFrom(address,address,uint) external returns (bool);
    function balanceOf(address) external view returns (uint);
}

N-02 EVENTS MISSING INDEXED FIELDS

Each event should use three indexed fields if there are three or more fields

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol

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

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol

140: event Expansion(IMarket indexed market, uint amount);
141: event Contraction(IMarket indexed market, uint amount);

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol

614: event Deposit(address indexed account, uint amount);
615: event Borrow(address indexed account, uint amount);
616: event Withdraw(address indexed account, address indexed to, uint amount);
617: event Repay(address indexed account, address indexed repayer, uint amount);
618: event ForceReplenish(address indexed account, address indexed replenisher, uint deficit, uint replenishmentCost, uint replenisherReward);
619: event Liquidate(address indexed account, address indexed liquidator, uint repaidDebt, uint liquidatorReward);
620: event CreateEscrow(address indexed user, address escrow);

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol

147: event RecordDailyLow(address indexed token, uint price);

N-03 REQUIRE() or REVERT() STATEMENTS SHOULD HAVE DESCRIPTIVE REASON STRINGS

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol

93: require(globalSupply <= supplyCeiling);

https://github.com/code-423n4/2022-10-inverse/blob/main/src/escrows/GovTokenEscrow.sol

67: require(msg.sender == beneficiary);

https://github.com/code-423n4/2022-10-inverse/blob/main/src/escrows/INVEscrow.sol

91: require(msg.sender == beneficiary);

N-04 OPEN TODO

https://github.com/code-423n4/2022-10-inverse/blob/main/src/escrows/INVEscrow.sol

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

N-05 NATSPEC IS INCOMPLETE

This is applicable throughout the contracts. 1 such instance is:

16: contract SimpleERC20Escrow { // missing @params

N-06 TYPOS

"th" is a typo instead of "the"

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol

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

G-01 SPLITTING REQUIRE() STATEMENTS THAT USE && SAVES GAS

Number of Instances Identified: 8

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol

249: require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER");

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol

75: require(_liquidationIncentiveBps > 0 && _liquidationIncentiveBps < 10000, "Invalid liquidation incentive");
162: require(_liquidationFactorBps > 0 && _liquidationFactorBps <= 10000, "Invalid liquidation factor");
173: require(_replenishmentIncentiveBps > 0 && _replenishmentIncentiveBps < 10000, "Invalid replenishment incentive");
184: require(_liquidationIncentiveBps > 0 && _liquidationIncentiveBps + liquidationFeeBps < 10000, "Invalid liquidation incentive");
195: require(_liquidationFeeBps > 0 && _liquidationFeeBps + liquidationIncentiveBps < 10000, "Invalid liquidation fee");
448: require(recoveredAddress != address(0) && recoveredAddress == from, "INVALID_SIGNER");
512: require(recoveredAddress != address(0) && recoveredAddress == from, "INVALID_SIGNER");

G-02 DONT COMPARE BOOLEAN EXPRESSIONS TO BOOLEAN LITERALS

Number of Instances Identified: 1

if (<x> == true) => if (<x>), if (<x> == false) => if (!<x>)

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol

350: require(minters[msg.sender] == true || msg.sender == operator, "ONLY MINTERS OR OPERATOR");

G-03 REQUIRE or REVERT STRINGS LONGER THAN 32 BYTES COST EXTRA GAS

Number of Instances Identified: 4

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol

63: require(newReplenishmentPriceBps_ > 0, "replenishment price must be over 0");
326: require(markets[msg.sender], "Only markets can call onForceReplenish");

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol

75: require(_liquidationIncentiveBps > 0 && _liquidationIncentiveBps < 10000, "Invalid liquidation incentive");
214: require(msg.sender == pauseGuardian || msg.sender == gov, "Only pause guardian or governance can pause");

G-04 DUPLICATED REQUIRE() or REVERT() CHECKS SHOULD BE REFACTORED TO A MODIFIER OR FUNCTION

Number of Instances Identified: 6

Saves deployment costs

The following require and revert checks have been used more than once within the same contract, hence they should be refactored to a modifier or a function

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol

195: require(balanceOf(from) >= amount, "Insufficient balance");
314: require(markets[msg.sender], "Only markets can call onRepay");

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol

48: require(msg.sender == gov, "ONLY GOV");
76: require(msg.sender == chair, "ONLY CHAIR");

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol

104: revert("Price not found");
117: require(price > 0, "Invalid feed price");

G-05 USAGE OF UINTS or INTS SMALLER THAN 32 BYTES - 256 BITS INCURS OVERHEAD

Number of Instances Identified: 6

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 Use a larger size then downcast where needed

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Oracle.sol

85: uint8 feedDecimals = feeds[token].feed.decimals();
86: uint8 tokenDecimals = feeds[token].tokenDecimals;
87: uint8 decimals = 36 - feedDecimals - tokenDecimals;
119: uint8 feedDecimals = feeds[token].feed.decimals();
120: uint8 tokenDecimals = feeds[token].tokenDecimals;
121: uint8 decimals = 36 - feedDecimals - tokenDecimals;

G-06 X += Y COSTS MORE GAS THAN X = X + Y FOR STATE VARIABLES

Number of Instances Identified: 26

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol

172: balances[msg.sender] -= amount;
174: balances[to] += amount;
196: balances[from] -= amount;
198: balances[to] += amount;
288: dueTokensAccrued[user] += accrued;
289: totalDueTokensAccrued += accrued;
304: debts[user] += additionalDebt;
316: debts[user] -= repaidDebt;
332: debts[user] += replenishmentCost;
360: _totalSupply += amount;
362: balances[to] += amount;
374: balances[from] -= amount;
376:  _totalSupply -= amount;

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol

91: supplies[market] += amount;
92: globalSupply += amount;
110: supplies[market] -= amount;
111: globalSupply -= amount;

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol

395: debts[borrower] += amount;
397: totalDebt += amount;
534: debts[user] -= amount;
535: totalDebt -= amount;
565: debts[user] += replenishmentCost;
568: totalDebt += replenishmentCost;
598: liquidatorReward += liquidatorReward * liquidationIncentiveBps / 10000;
599: debts[user] -= repaidDebt;
600: totalDebt -= repaidDebt;

G-07 USING BOOLS FOR STORAGE INCURS OVERHEAD

Number of Instances Identified: 6

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

https://github.com/code-423n4/2022-10-inverse/blob/main/src/BorrowController.sol

11: mapping(address => bool) public contractAllowlist;

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol

24: mapping (address => bool) public minters;
25: mapping (address => bool) public markets;

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol

52: bool immutable callOnDepositCallback;
53: bool public borrowPaused;
72: bool _callOnDepositCallback

G-08 USE CUSTOM ERRORS RATHER THAN REVERT() or REQUIRE() STRINGS TO SAVE GAS

Number of Instances Identified: 66

Some of them include : https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol

45: require(msg.sender == operator, "ONLY OPERATOR");
63: require(newReplenishmentPriceBps_ > 0, "replenishment price must be over 0");
224: require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED");

G-09 ++I COSTS LESS GAS COMPARED TO I++ OR I += 1 (SAME FOR --I VS I-- OR I -= 1)

Number of Instances Identified: 5

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol

239: nonces[owner]++
259: nonces[msg.sender]++;

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Market.sol

438: nonces[from]++
502: nonces[from]++,
521: nonces[msg.sender]++;

G-10 USING > 0 COSTS MORE GAS THAN != 0 WHEN USED ON A UINT 

Number of Instances Identified: 20

some of them include :

https://github.com/code-423n4/2022-10-inverse/blob/main/src/DBR.sol

63: require(newReplenishmentPriceBps_ > 0, "replenishment price must be over 0");
328: require(deficit > 0, "No deficit");