Inverse Finance contest - __141345__'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

There is no check for stale price and round completeness. Price can be stale and can lead to wrong answer return value.

Proof of Concept

// src/Oracle.sol
82         uint price = feeds[token].feed.latestAnswer();
116        uint price = feeds[token].feed.latestAnswer();

Reference:

Chainlink - latestanswer

Recommended Mitigation Steps

Validate data feed with the latestRoundData() function. Add checks on the return data with proper revert messages if the price is stale or the round is incomplete, for example:

    (uint80 roundID, int256 price, , uint256 timeStamp, uint80 answeredInRound) = oracle.latestRoundData();
    require(answeredInRound >= roundID, "...");
    require(timeStamp != 0, "...");

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.

Instances number of this issue: 6

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

// src/Market.sol
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);

Events not emitted for important state changes

When changing critical state variables events are not emitted. Emitting events allows monitoring activities with off-chain monitoring tools.

Instances number of this issue: 20

// src/BorrowController.sol
    function setOperator(address _operator) public onlyOperator { operator = _operator; }

// src/DBR.sol
    function setPendingOperator(address newOperator_) public onlyOperator {
    function setReplenishmentPriceBps(uint newReplenishmentPriceBps_) public onlyOperator {

// src/Market.sol
    function setOracle(IOracle _oracle) public onlyGov { oracle = _oracle; }
    function setBorrowController(IBorrowController _borrowController) public onlyGov { borrowController = _borrowController; }
    function setGov(address _gov) public onlyGov { gov = _gov; }
    function setLender(address _lender) public onlyGov { lender = _lender; }
    function setPauseGuardian(address _pauseGuardian) public onlyGov { pauseGuardian = _pauseGuardian; }
    function setCollateralFactorBps(uint _collateralFactorBps) public onlyGov {
    function setLiquidationFactorBps(uint _liquidationFactorBps) public onlyGov {
    function setReplenismentIncentiveBps(uint _replenishmentIncentiveBps) public onlyGov {
    function setLiquidationIncentiveBps(uint _liquidationIncentiveBps) public onlyGov {
    function setLiquidationFeeBps(uint _liquidationFeeBps) public onlyGov {

// src/Oracle.sol
    function setPendingOperator(address newOperator_) public onlyOperator { pendingOperator = newOperator_; }
    function setFeed(address token, IChainlinkFeed feed, uint8 tokenDecimals) public onlyOperator { feeds[token] = FeedData(feed, tokenDecimals); }
    function setFixedPrice(address token, uint price) public onlyOperator { fixedPrices[token] = price; }

// src/Fed.sol
    function changeGov(address _gov) public {
        require(msg.sender == gov, "ONLY GOV");
        gov = _gov;
    }
    function changeSupplyCeiling(uint _supplyCeiling) public {
    function changeChair(address _chair) public {
    function resign() public {

Suggestion: Consider emitting events after sensitive changes take place, to facilitate tracking and notify off-chain clients following the contract’s activity.

Missing Equivalence Checks in Setters

Description: Setter functions are missing checks to validate if the new value being set is the same as the current value already set in the contract. Such checks will showcase mismatches between on-chain and off-chain states.

Suggestion: This may hinder detecting discrepancies between on-chain and off-chain states leading to flawed assumptions of on-chain state and protocol behavior.

Instances number of this issue: 18

// src/BorrowController.sol
    function setOperator(address _operator) public onlyOperator { operator = _operator; }

// src/DBR.sol
    function setPendingOperator(address newOperator_) public onlyOperator {
    function setReplenishmentPriceBps(uint newReplenishmentPriceBps_) public onlyOperator {

// src/Market.sol
    function setOracle(IOracle _oracle) public onlyGov { oracle = _oracle; }
    function setBorrowController(IBorrowController _borrowController) public onlyGov { borrowController = _borrowController; }
    function setGov(address _gov) public onlyGov { gov = _gov; }
    function setLender(address _lender) public onlyGov { lender = _lender; }
    function setPauseGuardian(address _pauseGuardian) public onlyGov { pauseGuardian = _pauseGuardian; }
    function setCollateralFactorBps(uint _collateralFactorBps) public onlyGov {
    function setLiquidationFactorBps(uint _liquidationFactorBps) public onlyGov {
    function setReplenismentIncentiveBps(uint _replenishmentIncentiveBps) public onlyGov {
    function setLiquidationIncentiveBps(uint _liquidationIncentiveBps) public onlyGov {
    function setLiquidationFeeBps(uint _liquidationFeeBps) public onlyGov {

// src/Oracle.sol
    function setPendingOperator(address newOperator_) public onlyOperator { pendingOperator = newOperator_; }
    function setFeed(address token, IChainlinkFeed feed, uint8 tokenDecimals) public onlyOperator { feeds[token] = FeedData(feed, tokenDecimals); }

// src/Fed.sol
    function changeGov(address _gov) public {
        require(msg.sender == gov, "ONLY GOV");
        gov = _gov;
    }
    function changeSupplyCeiling(uint _supplyCeiling) public {
    function changeChair(address _chair) public {

Missing Time locks

When critical parameters of systems need to be changed, it is required to broadcast the change via event emission and recommended to enforce the changes after a time-delay. This is to allow system users to be aware of such critical changes and give them an opportunity to exit or adjust their engagement with the system accordingly. None of the onlyOwner functions that change critical protocol addresses/parameters have a timelock for a time-delayed change to alert:

• users and give them a chance to engage/exit protocol if they are not agreeable to the changes
• team in case of compromised owner(s) and give them a chance to perform incident response.

Instances number of this issue: 20

// src/BorrowController.sol
    function setOperator(address _operator) public onlyOperator { operator = _operator; }

// src/DBR.sol
    function setPendingOperator(address newOperator_) public onlyOperator {
    function setReplenishmentPriceBps(uint newReplenishmentPriceBps_) public onlyOperator {

// src/Market.sol
    function setOracle(IOracle _oracle) public onlyGov { oracle = _oracle; }
    function setBorrowController(IBorrowController _borrowController) public onlyGov { borrowController = _borrowController; }
    function setGov(address _gov) public onlyGov { gov = _gov; }
    function setLender(address _lender) public onlyGov { lender = _lender; }
    function setPauseGuardian(address _pauseGuardian) public onlyGov { pauseGuardian = _pauseGuardian; }
    function setCollateralFactorBps(uint _collateralFactorBps) public onlyGov {
    function setLiquidationFactorBps(uint _liquidationFactorBps) public onlyGov {
    function setReplenismentIncentiveBps(uint _replenishmentIncentiveBps) public onlyGov {
    function setLiquidationIncentiveBps(uint _liquidationIncentiveBps) public onlyGov {
    function setLiquidationFeeBps(uint _liquidationFeeBps) public onlyGov {

// src/Oracle.sol
    function setPendingOperator(address newOperator_) public onlyOperator { pendingOperator = newOperator_; }
    function setFeed(address token, IChainlinkFeed feed, uint8 tokenDecimals) public onlyOperator { feeds[token] = FeedData(feed, tokenDecimals); }
    function setFixedPrice(address token, uint price) public onlyOperator { fixedPrices[token] = price; }

// src/Fed.sol
    function changeGov(address _gov) public {
        require(msg.sender == gov, "ONLY GOV");
        gov = _gov;
    }
    function changeSupplyCeiling(uint _supplyCeiling) public {
    function changeChair(address _chair) public {
    function resign() public {

Suggestion: Users may be surprised when critical parameters are changed. Furthermore, it can erode users' trust since they can’t be sure the protocol rules won’t be changed later on. Compromised owner keys may be used to change protocol addresses/parameters to benefit attackers. Without a time-delay, authorized owners have no time for any planned incident response. (May be medium since owner can change critical parameters at anytime that can affect the users poorly).

Code Structure Deviates From Best-Practice

src/Fed.sol src/DBR.sol

The best-practice layout for a contract should follow the following order:

• state variables
• events
• modifiers
• constructor
• functions

Function ordering helps readers identify which functions they can call and find constructor and fallback functions easier. Functions should be grouped according to their visibility and ordered:

• constructor
• receive function (if exists)
• fallback function (if exists)
• external
• public
• internal
• private
• within a grouping, place the view and pure functions last

Suggestion: Consider adopting recommended best-practice for code structure and layout.

Signature Malleability of EVM's ecrecover()

The function calls the Solidity ecrecover() function directly to verify the given signatures. However, the ecrecover() EVM opcode allows malleable (non-unique) signatures and thus is susceptible to replay attacks.

Although a replay attack seems not possible for DBR.sol and Market.sol since nonces are used to prevent re-voting. However, this may become a vulnerability if used elsewhere.

Suggestion: Use the ecrecover function from OpenZeppelin's ECDSA library for signature verification. (Ensure using a version > 4.7.3 for there was a critical bug >= 4.1.0 < 4.7.3).

Missing Zero-address Validation

Description: Lack of zero-address validation on address parameters may lead to transaction reverts, waste gas, require resubmission of transactions and may even force contract redeployments in certain cases within the protocol.

Suggestion: Consider adding explicit zero-address validation on input parameters of address type.

Instances number of this issue: 19

// src/BorrowController.sol
    function setOperator(address _operator) public onlyOperator { operator = _operator; }
    constructor(address _operator) {
        operator = _operator;
    }

// src/DBR.sol
    function setPendingOperator(address newOperator_) public onlyOperator {
    function setReplenishmentPriceBps(uint newReplenishmentPriceBps_) public onlyOperator {
    constructor(
        uint _replenishmentPriceBps,
        string memory _name,
        string memory _symbol,
        address _operator
    ) {
        replenishmentPriceBps = _replenishmentPriceBps;
        name = _name;
        symbol = _symbol;
        operator = _operator;
        // ...
    }

// src/Market.sol
    function setOracle(IOracle _oracle) public onlyGov { oracle = _oracle; }
    function setBorrowController(IBorrowController _borrowController) public onlyGov { borrowController = _borrowController; }
    function setGov(address _gov) public onlyGov { gov = _gov; }
    function setLender(address _lender) public onlyGov { lender = _lender; }
    function setPauseGuardian(address _pauseGuardian) public onlyGov { pauseGuardian = _pauseGuardian; }
    constructor (
        address _gov,
        address _lender,
        address _pauseGuardian,
        address _escrowImplementation,
        IDolaBorrowingRights _dbr,
        IERC20 _collateral,
        IOracle _oracle,
        uint _collateralFactorBps,
        uint _replenishmentIncentiveBps,
        uint _liquidationIncentiveBps,
        bool _callOnDepositCallback
    ) {
        // ...
        gov = _gov;
        lender = _lender;
        pauseGuardian = _pauseGuardian;
        escrowImplementation = _escrowImplementation;
        dbr = _dbr;
        collateral = _collateral;
        oracle = _oracle;
        collateralFactorBps = _collateralFactorBps;
        replenishmentIncentiveBps = _replenishmentIncentiveBps;
        liquidationIncentiveBps = _liquidationIncentiveBps;
        callOnDepositCallback = _callOnDepositCallback;
        // ...
    }

// src/Oracle.sol
    function setPendingOperator(address newOperator_) public onlyOperator { pendingOperator = newOperator_; }
    function setFeed(address token, IChainlinkFeed feed, uint8 tokenDecimals) public onlyOperator { feeds[token] = FeedData(feed, tokenDecimals); }
    constructor(address _operator) {
        operator = _operator;
    }

// src/Fed.sol
    function changeGov(address _gov) public {
        require(msg.sender == gov, "ONLY GOV");
        gov = _gov;
    }
    function changeSupplyCeiling(uint _supplyCeiling) public {
    function changeChair(address _chair) public {
    constructor (IDBR _dbr, IDola _dola, address _gov, address _chair, uint _supplyCeiling) {
        dbr = _dbr;
        dola = _dola;
        gov = _gov;
        chair = _chair;
        supplyCeiling = _supplyCeiling;
    }

// src/escrows/INVEscrow.sol
    constructor(IXINV _xINV) {
        xINV = _xINV; 
    }

Open TODO

Delete or implement open TODO:

// src/escrows/INVEscrow.sol
    constructor(IXINV _xINV) {
        xINV = _xINV; 
    }

Miss 0 amount check

Instances number of this issue: 11

// src/Market.sol
    function recall(uint amount) public {
    function deposit(uint amount) public {
    function deposit(address user, uint amount) public {
    function borrowInternal(address borrower, address to, uint amount) internal {
    function borrow(uint amount) public {
    function borrowOnBehalf(address from, uint amount, uint deadline, uint8 v, bytes32 r, bytes32 s) public {
    function withdrawInternal(address from, address to, uint amount) internal {
    function withdraw(uint amount) public {
    function withdrawOnBehalf(address from, uint amount, uint deadline, uint8 v, bytes32 r, bytes32 s) public {
    function repay(address user, uint amount) public {
    function forceReplenish(address user, uint amount) public {

X = X + Y / X = X - Y IS CHEAPER THAN X += Y / X -= Y

The demo of the gas comparison can be seen here.

Consider use X = X + Y / X = X - Y to save gas.

Instances number of this issue: 26

// src/DBR.sol
174    balances[to] += amount;
198    balances[to] += amount;
288    dueTokensAccrued[user] += accrued;
289    totalDueTokensAccrued += accrued;
304    debts[user] += additionalDebt;
332    debts[user] += replenishmentCost;
360    _totalSupply += amount;
362    balances[to] += amount;

172    balances[msg.sender] -= amount;
196    balances[from] -= amount;
316    debts[user] -= repaidDebt;
374    balances[from] -= amount;
376    _totalSupply -= amount;

// src/Fed.sol
91     supplies[market] += amount;
92     globalSupply += amount;

110    supplies[market] -= amount;
111    globalSupply -= amount;


// src/Market.sol
395    debts[borrower] += amount;
397    totalDebt += amount;
565    debts[user] += replenishmentCost;
568    totalDebt += replenishmentCost;
598    liquidatorReward += liquidatorReward * liquidationIncentiveBps / 10000;

534    debts[user] -= amount;
535    totalDebt -= amount;
599    debts[user] -= repaidDebt;
600    totalDebt -= repaidDebt;

Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.

Instances number of this issue: 12

// src/DBR.sol
19    mapping(address => uint256) public balances;
20    mapping(address => mapping(address => uint256)) public allowance;

23    mapping(address => uint256) public nonces;
26    mapping (address => uint) public debts; // user => debt across all tracked markets
27    mapping (address => uint) public dueTokensAccrued; // user => amount of due tokens accrued
28    mapping (address => uint) public lastUpdated; // user => last update timestamp

// src/Market.sol
57    mapping (address => IEscrow) public escrows; // user => escrow
58    mapping (address => uint) public debts; // user => debt
59    mapping(address => uint256) public nonces; // user => nonce

// src/Oracle.sol
25    mapping (address => FeedData) public feeds;
26    mapping (address => uint) public fixedPrices;
27    mapping (address => mapping(uint => uint)) public dailyLows; // token => day => price

Boolean comparison

Comparing to a constant (true or false) is a bit more expensive than directly checking the returned boolean value.

The demo of the gas comparison can be seen here.

Instances number of this issue: 2

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

// src/Fed.sol
89         require(market.borrowPaused() != true, "CANNOT EXPAND PAUSED MARKETS");

Suggestion: Change to:

// src/DBR.sol
350        require(minters[msg.sender] || msg.sender == operator, "ONLY MINTERS OR OPERATOR");

// src/Fed.sol
89         require(!market.borrowPaused(), "CANNOT EXPAND PAUSED MARKETS");

Add unchecked {} for subtractions where the operands cannot underflow

Some will not underflow because of a previous code or require() or if-statement

And block.timestamp can't be less than lastUpdated[user], due to line 290: lastUpdated[user] = block.timestamp;

Instances number of this issue: 22

// src/DBR.sol
110        if(totalDueTokensAccrued > _totalSupply) return 0;
111        return _totalSupply - totalDueTokensAccrued;

122-124
        uint accrued = (block.timestamp - lastUpdated[user]) * debt / 365 days;
        if(dueTokensAccrued[user] + accrued > balances[user]) return 0;
        return balances[user] - dueTokensAccrued[user] - accrued;

135-137
        uint accrued = (block.timestamp - lastUpdated[user]) * debt / 365 days;
* debt / 365 days;
        if(dueTokensAccrued[user] + accrued < balances[user]) return 0;
        return dueTokensAccrued[user] + accrued - balances[user];

148        uint accrued = (block.timestamp - lastUpdated[user]) * debt / 365 days;

287        uint accrued = (block.timestamp - lastUpdated[user]) * debt / 365 days;

171        require(balanceOf(msg.sender) >= amount, "Insufficient balance");
172        balances[msg.sender] -= amount;

195        require(balanceOf(from) >= amount, "Insufficient balance");
196        balances[from] -= amount;

360        _totalSupply += amount;

373        require(balanceOf(from) >= amount, "Insufficient balance");
374        balances[from] -= amount;

// src/Market.sol
532-535
        uint debt = debts[user];
        require(debt >= amount, "Insufficient debt");
        debts[user] -= amount;
        totalDebt -= amount;

// src/Fed.sol
123        if(supply >= marketValue) return 0;
124        return marketValue - supply;

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 replace 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.

Instances number of this issue: 4 // src/DBR.sol totalDueTokensAccrued and _totalSupply:

110        if(totalDueTokensAccrued > _totalSupply) return 0;
111        return _totalSupply - totalDueTokensAccrued;

dueTokensAccrued[user] and balances[user]:

123        if(dueTokensAccrued[user] + accrued > balances[user]) return 0;
124        return balances[user] - dueTokensAccrued[user] - accrued;

// src/Market.sol escrows[user]:

246        if(escrows[user] != IEscrow(address(0))) return escrows[user];

Functions only called outside of the contract can be declared as external

External call cost is less expensive than of public functions.

The difference is because in public functions, Solidity immediately copies array arguments to memory, while external functions can read directly from calldata. Memory allocation is expensive, whereas reading from calldata is cheap.

Instances number of this issue: 55

// src/BorrowController.sol
    function allow(address allowedContract) public onlyOperator { contractAllowlist[allowedContract] = true; }
    function deny(address deniedContract) public onlyOperator { contractAllowlist[deniedContract] = false; }
    function borrowAllowed(address msgSender, address, uint) public view returns (bool) {

// src/DBR.sol
    function setPendingOperator(address newOperator_) public onlyOperator {
    function setReplenishmentPriceBps(uint newReplenishmentPriceBps_) public onlyOperator {
    function claimOperator() public {
    function addMinter(address minter_) public onlyOperator {
    function removeMinter(address minter_) public onlyOperator {
    function addMarket(address market_) public onlyOperator {
    function approve() public virtual returns (bool) {
    function transfer() public virtual returns (bool) {
    function transferFrom() public virtual returns (bool) {
    function permit() public virtual {
    function invalidateNonce() public {
    function accrueDueTokens(address user) public {
    function onBorrow(address user, uint additionalDebt) public {
    function onRepay(address user, uint repaidDebt) public {
    function onForceReplenish(address user, uint amount) public {
    function burn(uint amount) public {
    function mint(address to, uint amount) public {

// src/Fed.sol
    function changeGov(address _gov) public {
    function changeSupplyCeiling(uint _supplyCeiling) public {
    function changeChair(address _chair) public {
    function resign() public {
    function expansion(IMarket market, uint amount) public {
    function contraction(IMarket market, uint amount) public {
    function takeProfit(IMarket market) public {

// src/Market.sol
    function setOracle(IOracle _oracle) public onlyGov { oracle = _oracle; }
    function setBorrowController(IBorrowController _borrowController) public onlyGov { borrowController = _borrowController; }
    function setGov(address _gov) public onlyGov { gov = _gov; }
    function setLender(address _lender) public onlyGov { lender = _lender; }
    function setPauseGuardian(address _pauseGuardian) public onlyGov { pauseGuardian = _pauseGuardian; }
    function setCollateralFactorBps(uint _collateralFactorBps) public onlyGov {
    function setLiquidationFactorBps(uint _liquidationFactorBps) public onlyGov {
    function setReplenismentIncentiveBps(uint _replenishmentIncentiveBps) public onlyGov {
    function setLiquidationIncentiveBps(uint _liquidationIncentiveBps) public onlyGov {
    function setLiquidationFeeBps(uint _liquidationFeeBps) public onlyGov {
    function recall(uint amount) public {
    function pauseBorrows(bool _value) public {
    function depositAndBorrow(uint amountDeposit, uint amountBorrow) public {
    function borrowOnBehalf(address from, uint amount, uint deadline, uint8 v, bytes32 r, bytes32 s) public {
    function withdrawOnBehalf(address from, uint amount, uint deadline, uint8 v, bytes32 r, bytes32 s) public {
    function invalidateNonce() public {
    function repayAndWithdraw(uint repayAmount, uint withdrawAmount) public {
    function forceReplenish(address user, uint amount) public {
    function liquidate(address user, uint repaidDebt) public {

// src/Oracle.sol
    function setPendingOperator(address newOperator_) public onlyOperator { pendingOperator = newOperator_; }
    function setFeed(address token, IChainlinkFeed feed, uint8 tokenDecimals) public onlyOperator { feeds[token] = FeedData(feed, tokenDecimals); }
    function setFixedPrice(address token, uint price) public onlyOperator { fixedPrices[token] = price; }
    function claimOperator() public {

// src/escrows/GovTokenEscrow.sol
    function pay(address recipient, uint amount) public {
    function delegate(address delegatee) public {

// src/escrows/INVEscrow.sol
    function pay(address recipient, uint amount) public {
    function delegate(address delegatee) public {

// src/escrows/SimpleERC20Escrow.sol
    function pay(address recipient, uint amount) public {

Duplicated functions can be written into a library

invalidateNonce() and DOMAIN_SEPARATOR():

// src/DBR.sol
// src/Market.sol
    function invalidateNonce() public {
        nonces[msg.sender]++;
    }

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

Splitting require() statements that use &&

require() statements with multiple conditions can be split.

See this issue which describes the fact that there is a larger deployment gas cost, but with enough runtime calls, the change ends up being cheaper.

The demo of the gas comparison can be seen here.

Instances number of this issue: 8

// src/DBR.sol
249    require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER");

// 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");

Code reuse

Some functions codes overlap, only a few lines of difference. Consider reuse the common part and save deployment gas.

• src/Oracle.sol viewPrice() and getPrice()
• src/Market.sol
  • getCollateralValue(address user) and getCollateralValueInternal(address user)
  • getCreditLimit(address user) and getCreditLimitInternal(address user)
  • getWithdrawalLimitInternal(address user) and getWithdrawalLimit(address user)

Recycle mapping storage

As time goes by, outdated dailyLows[token][day] in src/Oracle.sol will be useless. Consider delete the keys and get some gas refund.