Fraxlend (Frax Finance) contest - delfin454000's results

Fraxlend: A permissionless lending platform and the final piece of the Frax Finance Defi Trinity.

General Information

Platform: Code4rena

Start Date: 12/08/2022

Pot Size: $50,000 USDC

Total HM: 15

Participants: 120

Period: 5 days

Judge: Justin Goro

Total Solo HM: 6

Id: 153

League: ETH

Frax Finance

Findings Distribution

Researcher Performance

Rank: 60/120

Findings: 2

Award: $67.01

🌟 Selected for report: 0

🚀 Solo Findings: 0

Missing @param statements


FraxlendPairCore.sol: L143-160

    /// @notice The ```constructor``` function is called on deployment
    /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)
    /// @param _maxLTV The Maximum Loan-To-Value for a borrower to be considered solvent (1e5 precision)
    /// @param _liquidationFee The fee paid to liquidators given as a % of the repayment (1e5 precision)
    /// @param _maturityDate The maturityDate date of the Pair
    /// @param _penaltyRate The interest rate after maturity date
    /// @param _isBorrowerWhitelistActive Enables borrower whitelist
    /// @param _isLenderWhitelistActive Enables lender whitelist
    constructor(
        bytes memory _configData,
        bytes memory _immutables,
        uint256 _maxLTV,
        uint256 _liquidationFee,
        uint256 _maturityDate,
        uint256 _penaltyRate,
        bool _isBorrowerWhitelistActive,
        bool _isLenderWhitelistActive
    ) {

@param statement is missing for _immutables


FraxlendPairCore.sol: L791-800

    /// @notice The ```RemoveCollateral``` event is emitted when collateral is removed from a borrower's position
    /// @param _sender The account from which funds are transferred
    /// @param _collateralAmount The amount of Collateral Token to be transferred
    /// @param _receiver The address to which Collateral Tokens will be transferred
    event RemoveCollateral(
        address indexed _sender,
        uint256 _collateralAmount,
        address indexed _receiver,
        address indexed _borrower
    );

@param statement is missing for _borrower


FraxlendPairCore.sol: L892-904

    /// @notice The ```Liquidate``` event is emitted when a liquidation occurs
    /// @param _borrower The borrower account for which the liquidation occured
    /// @param _collateralForLiquidator The amount of Collateral Token transferred to the liquidator
    /// @param _sharesToLiquidate The number of Borrow Shares the liquidator repaid on behalf of the borrower
    /// @param _sharesToAdjust The number of Borrow Shares that were adjusted on liabilites and assets (a writeoff)
    event Liquidate(
        address indexed _borrower,
        uint256 _collateralForLiquidator,
        uint256 _sharesToLiquidate,
        uint256 _amountLiquidatorToRepay,
        uint256 _sharesToAdjust,
        uint256 _amountToAdjust
    );

@param statements missing for _amountLiquidatorToRepay and _amountToAdjust


FraxlendPairDeployer.sol: L183-201

    /// @notice The ```_deployFirst``` function is an internal function with deploys the pair
    /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)
    /// @param _maxLTV The Maximum Loan-To-Value for a borrower to be considered solvent (1e5 precision)
    /// @param _liquidationFee The fee paid to liquidators given as a % of the repayment (1e5 precision)
    /// @param _maturityDate The maturityDate of the Pair
    /// @param _isBorrowerWhitelistActive Enables borrower whitelist
    /// @param _isLenderWhitelistActive Enables lender whitelist
    /// @return _pairAddress The address to which the Pair was deployed
    function _deployFirst(
        bytes32 _saltSeed,
        bytes memory _configData,
        bytes memory _immutables,
        uint256 _maxLTV,
        uint256 _liquidationFee,
        uint256 _maturityDate,
        uint256 _penaltyRate,
        bool _isBorrowerWhitelistActive,
        bool _isLenderWhitelistActive
    ) private returns (address _pairAddress) {

@param statements missing for _saltSeed, _immutables and _penaltyRate


FraxlendPairDeployer.sol: L345-364

    /// @notice The ```deployCustom``` function allows whitelisted users to deploy custom Term Sheets for OTC debt structuring
    /// @dev Caller must be added to FraxLedWhitelist
    /// @param _name The name of the Pair
    /// @param _configData abi.encode(address _asset, address _collateral, address _oracleMultiply, address _oracleDivide, uint256 _oracleNormalization, address _rateContract, bytes memory _rateInitData)
    /// @param _maxLTV The Maximum Loan-To-Value for a borrower to be considered solvent (1e5 precision)
    /// @param _liquidationFee The fee paid to liquidators given as a % of the repayment (1e5 precision)
    /// @param _maturityDate The maturityDate of the Pair
    /// @param _approvedBorrowers An array of approved borrower addresses
    /// @param _approvedLenders An array of approved lender addresses
    /// @return _pairAddress The address to which the Pair was deployed
    function deployCustom(
        string memory _name,
        bytes memory _configData,
        uint256 _maxLTV,
        uint256 _liquidationFee,
        uint256 _maturityDate,
        uint256 _penaltyRate,
        address[] memory _approvedBorrowers,
        address[] memory _approvedLenders
    ) external returns (address _pairAddress) {

@param statement is missing for _penaltyRate



Duplicate comment

FraxlendPairCore.sol: L1020-1022

        // NOTE: reverts if _collateralForLiquidator > userCollateralBalance
        // Collateral is removed on behalf of borrower and sent to liquidator
        // NOTE: reverts if _collateralForLiquidator > userCollateralBalance

Remove duplicate comment line



Typos


VariableInterestRate.sol: L32

/// @notice A Contract for calulcating interest rates as a function of utilization and time

Change calulcating to calculating


FraxlendWhitelist.sol: L47

    /// @notice The ```setOracleContractWhitelist``` function sets a given address to true/false for use as oralce

Change oralce to Oracle


FraxlendPairCore.sol: L231

        // Set approved lenders whitlist active

Change whitlist to whitelist


FraxlendPairCore.sol: L893

    /// @param _borrower The borrower account for which the liquidation occured

Change occured to occurred


FraxlendPairCore.sol: L896

    /// @param _sharesToAdjust The number of Borrow Shares that were adjusted on liabilites and assets (a writeoff)

Change liabilites to liabilities


FraxlendPairCore.sol: L1010

                    // Note: Ensure this memory stuct will be passed to _repayAsset for write to state

Change stuct to struct


FraxlendPairCore.sol: L1041

    /// @param _borrowAmount The amount of Asset Token to be borrowed to be borrowed

Remove repeated phrase to be borrowed


The same typo (black list) occurs in both lines below:

FraxlendPair.sol: L285

FraxlendPair.sol: L304

    /// @dev Cannot black list self

Change black list to blacklist in both cases


The same typo (whos) occurs in both lines below:

FraxlendPair.sol: L286

FraxlendPair.sol: L305

    /// @param _lenders The addresses whos status will be set

Change whos to whose


The same typo (approcal) occurs in both lines below:

FraxlendPair.sol: L287

FraxlendPair.sol: L306

    /// @param _approval The approcal status

Change approcal to approval


FraxlendPairDeployer.sol: L168

    /// @dev splits the data if necessary to accomodate creation code that is slightly larger than 24kb

Change accomodate to accommodate



Long single line comments

In theory, comments over 79 characters should wrap using multi-line comment syntax. Even if somewhat longer comments are acceptable, there are cases where very long comments interfere with readability. Below are five instances of extra-long comments whose readability could be improved by wrapping, as shown:


FraxlendPairCore.sol: L391

    /// @notice The ```addInterest``` function is a public implementation of _addInterest and allows 3rd parties to trigger interest accrual

Suggestion:

    /// @notice The ```addInterest``` function is a public implementation of _addInterest
    ///   and allows 3rd parties to trigger interest accrual.

FraxlendPairCore.sol: L406

    /// @notice The ```_addInterest``` function is invoked prior to every external function and is used to accrue interest and update interest rate

Suggestion:

    /// @notice The ```_addInterest``` function is invoked prior to every external function
    ///   and is used to accrue interest and update interest rate.

FraxlendPairCore.sol: L616-618

    /// @notice The ```_redeem``` function is an internal implementation which allows a Lender to pull their Asset Tokens out of the Pair
    /// @dev Caller must invoke ```ERC20.approve``` on the Asset Token contract prior to calling function
    /// @param _totalAsset An in-memory VaultAccount struct which holds the total amount of Asset Tokens and the total number of Asset Shares (fTokens)

Suggestion:

    /// @notice The ```_redeem``` function is an internal implementation 
    ///   which allows a Lender to pull their Asset Tokens out of the Pair.
    /// @dev Caller must invoke ```ERC20.approve``` on the Asset Token contract prior to calling function
    /// @param _totalAsset An in-memory VaultAccount struct which holds the total amount of Asset Tokens 
    ///   and the total number of Asset Shares (fTokens).

FraxlendPairCore.sol: L982-983

            // Because interest accrues every block, _liquidationAmountInCollateralUnits from a few lines up is an ever increasing value
            // This means that leftoverCollateral can occasionally go negative by a few hundred wei (cleanLiqFee premium covers this for liquidator)

Suggestion:

            // Because interest accrues every block, _liquidationAmountInCollateralUnits 
            //   from a few lines up is an ever increasing value.
            // This means that leftoverCollateral can occasionally go negative
            //   by a few hundred wei (cleanLiqFee premium covers this for liquidator).

FraxlendPairCore.sol: L1151-1156

    /// @notice The ```repayAssetWithCollateral``` function allows a borrower to repay their debt using existing collateral in contract
    /// @param _swapperAddress The address of the whitelisted swapper to use for token swaps
    /// @param _collateralToSwap The amount of Collateral Tokens to swap for Asset Tokens
    /// @param _amountAssetOutMin The minimum amount of Asset Tokens to receive during the swap
    /// @param _path An array containing the addresses of ERC20 tokens to swap.  Adheres to UniV2 style path params.
    /// @return _amountAssetOut The amount of Asset Tokens received for the Collateral Tokens, the amount the borrowers account was credited

Suggestion:

    /// @notice The ```repayAssetWithCollateral``` function allows a borrower
    ///   to repay their debt using existing collateral in contract.
    /// @param _swapperAddress The address of the whitelisted swapper to use for token swaps
    /// @param _collateralToSwap The amount of Collateral Tokens to swap for Asset Tokens
    /// @param _amountAssetOutMin The minimum amount of Asset Tokens to receive during the swap
    /// @param _path An array containing the addresses of ERC20 tokens to swap —
    ///   adheres to UniV2 style path params.
    /// @return _amountAssetOut The amount of Asset Tokens received for the Collateral Tokens,
    ///   the amount the borrowers account was credited.


Require revert string is too long

The revert strings below should be shortened to 32 characters or fewer to save gas, or else consider using custom error codes (which could save even more gas)


LinearInterestRate.sol: L57-60

        require(
            _minInterest < MAX_INT && _minInterest <= _vertexInterest && _minInterest >= MIN_INT,
            "LinearInterestRate: _minInterest < MAX_INT && _minInterest <= _vertexInterest && _minInterest >= MIN_INT"

LinearInterestRate.sol: L61-64

        require(
            _maxInterest <= MAX_INT && _vertexInterest <= _maxInterest && _maxInterest > MIN_INT,
            "LinearInterestRate: _maxInterest <= MAX_INT && _vertexInterest <= _maxInterest && _maxInterest > MIN_INT"
        );

LinearInterestRate.sol: L65-68

        require(
            _vertexUtilization < MAX_VERTEX_UTIL && _vertexUtilization > 0,
            "LinearInterestRate: _vertexUtilization < MAX_VERTEX_UTIL && _vertexUtilization > 0"
        );

FraxlendPairDeployer.sol: L205

            require(deployedPairsBySalt[salt] == address(0), "FraxlendPairDeployer: Pair already deployed");

FraxlendPairDeployer.sol: L228

            require(_pairAddress != address(0), "FraxlendPairDeployer: create2 failed");

FraxlendPairDeployer.sol: L365

        require(_maxLTV <= GLOBAL_MAX_LTV, "FraxlendPairDeployer: _maxLTV is too large");

FraxlendPairDeployer.sol: L366-369

        require(
            IFraxlendWhitelist(FRAXLEND_WHITELIST_ADDRESS).fraxlendDeployerWhitelist(msg.sender),
            "FraxlendPairDeployer: Only whitelisted addresses"
        );


Avoid use of '&&' within a require function

Splitting into separate require() statements saves gas


LinearInterestRate.sol: L57-60

        require(
            _minInterest < MAX_INT && _minInterest <= _vertexInterest && _minInterest >= MIN_INT,
            "LinearInterestRate: _minInterest < MAX_INT && _minInterest <= _vertexInterest && _minInterest >= MIN_INT"

Recommendation:

        require(_minInterest < MAX_INT, "Error message");
        require(_minInterest <= _vertexInterest, "Error message");
        require(_minInterest >= MIN_INT, "Error message");

Similarly for the additional instances of && within require functions referenced below:

LinearInterestRate.sol: L61-64

LinearInterestRate.sol: L65-68



Array length should not be looked up in every iteration of a for loop

Since calculating the array length costs gas, it's best to read the length of the array from memory before executing the loop (as in fact occurs in some of the for loops in Fraxlend)


FraxlendWhitelist.sol: L51-54

        for (uint256 i = 0; i < _addresses.length; i++) {
            oracleContractWhitelist[_addresses[i]] = _bool;
            emit SetOracleWhitelist(_addresses[i], _bool);
        }

Suggestion:

        uint256 totalAddressesLength = _addresses.length; 
        for (uint256 i = 0; i < totalAddressesLength; i++) {
            oracleContractWhitelist[_addresses[i]] = _bool;
            emit SetOracleWhitelist(_addresses[i], _bool);
        }

Similarly for the six additional for loops referenced below:

FraxlendWhitelist.sol: L66-69

FraxlendWhitelist.sol: L81-84

FraxlendPairCore.sol: L265-267

FraxlendPairCore.sol: L270-272

FraxlendPair.sol: L289-295

FraxlendPair.sol: L308-314



Use ++i instead of i++ to increase count in a for loop

Since use of i++ (or equivalent counter) costs more gas, it is better to use ++i


SafeERC20.sol: L27-29

            for (i = 0; i < 32 && data[i] != 0; i++) {
                bytesArray[i] = data[i];
            }

Suggestion:

            for (i = 0; i < 32 && data[i] != 0; ++i) {
                bytesArray[i] = data[i];
            }

Similarly for the seven for loops referenced below:

FraxlendWhitelist.sol: L51-54

FraxlendWhitelist.sol: L66-69

FraxlendWhitelist.sol: L81-84

FraxlendPair.sol: L289-295

FraxlendPair.sol: L308-314

FraxlendPairDeployer.sol: L127-132

FraxlendPairDeployer.sol: L152-160



Avoid use of default "checked" behavior in a for loop

Underflow/overflow checks are made every time ++i (or i++ or equivalent counter) is called. Such checks are unnecessary since i is already limited. Therefore, use unchecked {++i}/unchecked {i++} instead


FraxlendWhitelist.sol: L51-54

        for (uint256 i = 0; i < _addresses.length; i++) {
            oracleContractWhitelist[_addresses[i]] = _bool;
            emit SetOracleWhitelist(_addresses[i], _bool);
        }

Recommendation

        uint256 totalAddressesLength = _addresses.length; 
        for (uint256 i = 0; i < totalAddressesLength;) {
            oracleContractWhitelist[_addresses[i]] = _bool;
            emit SetOracleWhitelist(_addresses[i], _bool);

            unchecked {
              ++i;
          }
        }

Similarly for the seven following for loops:

FraxlendWhitelist.sol: L51-54

FraxlendWhitelist.sol: L66-69

FraxlendWhitelist.sol: L81-84

FraxlendPairCore.sol: L265-267

FraxlendPairCore.sol: L270-272

FraxlendPair.sol: L289-295

FraxlendPair.sol: L308-314


Note that while, for presentation purposes, I have been addressing the for loop-related gas savings opportunities separately, they should be combined. I've included the other two types of gas saving that were described in previous sections in the recommendation above. Also, the for loops in the contest are inconsistent, so that the lists of loops for each gas saving type presented above are not identical.



AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter