Fraxlend (Frax Finance) contest - 0xA5DF's results

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/b58c9b72f5fe8fab81f7436504e7daf60fd124e3/src/contracts/FraxlendPairCore.sol#L995-L1015

Vulnerability details

While liquidateClean() does mark off bad debt from totalBorrow it doesn't mark it off from the borrower (userBorrowShares). This means that in case the borrower does repay for some reason (mistake / good will / wants to borrow again), the totalBorrow.shares will be less than the total of userBorrowShares values. Meaning that the last borrower can't repay his shares and get back his collateral.

Impact

Besides the last user which will be unable to repay his debt, this will also cause all borrowers to fleet the pair ASAP in order to not be the last borrower who can't repay. The pair will become unusable (since without borrowers the vault has no point), with 1 user who lost his collateral in exchange for asset-tokens which are worth less than the collateral.

Proof of Concept

In the following test, a user tries to repay his loan and fails after bad debt was repaid by another user.

Added to src/test/e2e/LiquidationPairTest.sol:

    function testBadDebtRepayBug() public {
        // Setup contracts
        defaultSetUp();
        // Sets price to 3200 USD per ETH

        uint256 _amountToBorrow = 16e20; // 1.5k
        uint256 _amountInPool = 15e23; // 1.5m

        // collateral is 1.5 times borrow amount
        oracleDivide.setPrice(3200, 1, vm);
        mineBlocks(1);
        (, uint256 _exchangeRate) = pair.exchangeRateInfo();
        uint256 _collateralAmount = (_amountToBorrow * _exchangeRate * 3) /
            (2 * 1e18);
        faucetFunds(asset, _amountInPool);
        faucetFunds(collateral, _collateralAmount);
        lendTokenViaDeposit(_amountInPool, users[0]);
        address liquidator = users[1];
        address victimBorrower = users[2];
        address badBorrower = users[3];

        borrowToken(
            uint128(_amountToBorrow),
            _collateralAmount,
            victimBorrower
        );
        borrowToken(uint128(_amountToBorrow), _collateralAmount, badBorrower);
        uint256 mxltv = pair.maxLTV();
        uint256 cleanLiquidationFee = pair.cleanLiquidationFee();


        // lowering collateral price, creating bad debt for both borrowers
        uint256 liquidation_price = ((((1e18 / _exchangeRate) * mxltv) / 1e5) *
            (1e5 + cleanLiquidationFee)) / 1e5;
        oracleDivide.setPrice(liquidation_price / 4, 1, vm);
        // update stored exchange rate acoordingaly
        (, _exchangeRate) = pair.exchangeRateInfo();

        mineBlocks(1);
        for (uint256 i = 0; i < 1; i++) {
            pair.addInterest();
            mineOneBlock();
        }
        {
            uint128 totalBBShares = pair.userBorrowShares(badBorrower).toUint128();

            // liquidator liquidates shares, to the amount the collateral can cover
            uint256 userCollateral = pair.userCollateralBalance(badBorrower);
            uint256 amountToLiquidate = (userCollateral * 1e18) / _exchangeRate  / 10;
            uint256 sharesToLiquidate = pair.toBorrowShares(
                amountToLiquidate,
                true
            );
            assertLt(sharesToLiquidate , totalBBShares);

            vm.startPrank(liquidator);
            pair.addInterest();

            asset.approve(
                address(pair),
                toBorrowAmount(sharesToLiquidate, true)
            );

            uint256 collateralGained = pair.liquidateClean(
                uint128(sharesToLiquidate),
                block.timestamp,
                badBorrower
            );
            // assertEq(pair.userBorrowShares(badBorrower), 0);
            vm.stopPrank();
        }

        {
            // bad borrower now repays his borrow shares
            vm.startPrank(badBorrower);
            uint256 borrowerShares = pair.userBorrowShares(badBorrower);

            asset.approve(address(pair), toBorrowAmount(borrowerShares, true) * 2);
            pair.repayAsset(borrowerShares * 5 / 10, badBorrower);
            vm.stopPrank();
        }
        {
            // victime borrower now tries to repay his debt, but fails 
            vm.startPrank(victimBorrower);

            uint256 borrowerShares = pair.userBorrowShares(victimBorrower);
 
            asset.approve(address(pair), toBorrowAmount(borrowerShares, true) * 2);
            pair.repayAsset(borrowerShares, victimBorrower);


            vm.stopPrank();
        }
    }

Recommended Mitigation Steps

If you wish to demand the user to repay his bad debt before being able to borrow again - mark this debt as bad debt and when it gets repaid - mark it off only from userBorrowShares and not from totalBorrow.shares (this might cost some extra gas, having to check that the user doesn't have bad debt). Otherwise, simply mark off the debt from userBorrowShares too during liquidateClean().

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/b58c9b72f5fe8fab81f7436504e7daf60fd124e3/src/contracts/FraxlendPairCore.sol#L911-L942

Vulnerability details

When there's bad debt which wasn't marked off yet, the totalAssets.amount is higher than the actual (solvent) amount the pair has, meaning that lenders who redeem their tokens earlier will get more in return, at the expense of lenders who leave later. Therefore bad debt should be marked off as soon as possible, the later it's done the more interest it accumulates and the higher the chances are that some of the lenders will notice and redeem their shares before the bad debt is subtracted from the total assets amount.

Having the option to liquidate via the liquidate() function (which doesn't mark off bad debt) can lead to users using that function and leaving bad debt alongside zero collateral or near-zero collateral (giving no motivation for other users to liquidate the rest). Marking off the remaining of the bad debt via liquidateClean() with 0 shares might be possible (not always, some tokens don't allow 0 transfers), however there's no motivation for outside users to do so. And as for the lenders - redeeming their tokens before the bad debt is subtracted from the total amount might be more profitable than staying and marking off the bad debt.

Impact

Some lenders might be able to dodge the loss of the bad debt (+ interest), while the last one(s) will have to absorb the lost of the lenders who left too.

Proof of Concept

Consider the following scenario:

• A pair has 10 lenders with 1K$ from each one (10K total)
• Borrower borrowed that 10K
• The collateral price went down and now it's worth only 7K
• A liquidator notices that and liquidates it via liquidate()
• the totalAssets.amount is 10K + interest, but the total asset amount is actually less than 7K (subtracting liquidator fees)
• 6 lenders notice that and redeem their shares, getting back their money + interest
• The 7th lender to redeem will only be able to get back part of his money
• The remaining 3 lenders will loose all of their money

Recommended Mitigation Steps

Mark off bad debt when remaining collateral reaches zero or near-zero value. (if only liquidateClean() was available then there would be a motivation to not leave near-zero collateral, but as long as this isn't the case consider marking off also in case of near-zero collateral left).

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/b58c9b72f5fe8fab81f7436504e7daf60fd124e3/src/contracts/FraxlendPairCore.sol#L447-L449

Vulnerability details

When _addInterest() is called post maturity, the penalty rate is applied since the last time _addInterest() was called, including the period before maturity.

Impact

Borrowers will be charged with higher interest rates, when maturity date passes. The longer the time since _addInterest() was last called before maturity date the more extra-interest they will be charged.

Sidenote: I think this should be considered high, since it comes at the cost of the assets of the borrowers (even though it stems from unnecessary fees being charged, similar to this Putty bug).

Proof of Concept

The test compares 2 scenarios, the 1st one where addInterest() isn't called for 30 days before maturity date, and in the 2nd it isn't called just for 1 day before. The debt (not interest) in the first scenario would be ~50% higher than the second, see test output below.

At src/test/e2e/BorrowPairTest.t.sol I've modified _fuzzySetupBorrowToken() to this and then ran source .env && forge test --fork-url $MAINNET_URL --fork-block-number $DEFAULT_FORK_BLOCK -m testFuzzyMaxLTVBorrowToken -vvv. And I've also modified src/test/e2e/BasePairTest.sol a bit so that each pair can have a different name for salt (see diff below).

    function _fuzzySetupBorrowToken(
        uint256 _minInterest,
        uint256 _vertexInterest,
        uint256 _maxInterest,
        uint256 _vertexUtilization,
        uint256 _maxLTV,
        uint256 _liquidationFee,
        uint256 _priceTop,
        uint256 _priceDiv
    ) public {
        asset = IERC20(FIL_ERC20);
        collateral = IERC20(MKR_ERC20);
        oracleMultiply = AggregatorV3Interface(CHAINLINK_MKR_ETH);
        oracleMultiply.setPrice(_priceTop, 1e8, vm);
        oracleDivide = AggregatorV3Interface(CHAINLINK_FIL_ETH);
        oracleDivide.setPrice(_priceDiv, 1e8, vm);
        uint256 _oracleNormalization = 1e18;
        (
            address _rateContract,
            bytes memory _rateInitData
        ) = fuzzyRateCalculator(
                2,
                _minInterest,
                _vertexInterest,
                _maxInterest,
                _vertexUtilization
            );
        startHoax(COMPTROLLER_ADDRESS);
        setWhitelistTrue();
        vm.stopPrank();

        address[] memory _borrowerWhitelist = _maxLTV >= LTV_PRECISION
            ? users
            : new address[](0);
        address[] memory _lenderWhitelist = _maxLTV >= LTV_PRECISION
            ? users
            : new address[](0);

        uint256 borrowerDebtWrong;
        {
            // wrong calculation, when addInterest isn't called before maturity date (30 days)
            deployFraxlendCustom(
                _oracleNormalization,
                _rateContract,
                _rateInitData,
                _maxLTV,
                _liquidationFee,
                block.timestamp + 30 days,
                1000 * DEFAULT_INT,
                _borrowerWhitelist,
                _lenderWhitelist
            );
            pair.updateExchangeRate();
            _borrowTest(_maxLTV, 15e20, 15e23);
            vm.roll(block.number + 1);


            vm.warp(block.timestamp + 29 days);
            vm.roll(block.number + 1);
            vm.warp(block.timestamp + 2 days);
            pair.addInterest();



            address borrower = users[2];
            uint256 borrowerShares = pair.userBorrowShares(borrower);
            borrowerDebtWrong = pair.toBorrowAmount(borrowerShares, false);
        }
        deploySaltName = "asdf2";
        uint256 borrowerDebtRight;
        {
            // relatively correct calculation, when addInterest is called close to maturity date 
            deployFraxlendCustom(
                _oracleNormalization,
                _rateContract,
                _rateInitData,
                _maxLTV,
                _liquidationFee,
                block.timestamp + 30 days,
                1000 * DEFAULT_INT,
                _borrowerWhitelist,
                _lenderWhitelist
            );
            pair.updateExchangeRate();
            _borrowTest(_maxLTV, 15e20, 15e23);
            vm.roll(block.number + 1);

            vm.warp(block.timestamp + 29 days);
            
            pair.addInterest();

            vm.roll(block.number + 1);
            vm.warp(block.timestamp + 2 days);
            pair.addInterest();


            address borrower = users[2];
            uint256 borrowerShares = pair.userBorrowShares(borrower);
            borrowerDebtRight = pair.toBorrowAmount(borrowerShares, false);
        }

        // compare final debt between both scenarios
        assertEq(borrowerDebtRight, borrowerDebtWrong);
    }

diff --git a/src/test/e2e/BasePairTest.sol b/src/test/e2e/BasePairTest.sol
index 25114d9..27321dc 100644
--- a/src/test/e2e/BasePairTest.sol
+++ b/src/test/e2e/BasePairTest.sol
@@ -67,6 +67,8 @@ contract BasePairTest is FraxlendPairConstants, Scenarios, Test {
     PairAccounting public initial;
     PairAccounting public final_;
     PairAccounting public net;
+    string public deploySaltName = "asdf";
+
 
     // Users
     address[] public users = [vm.addr(1), vm.addr(2), vm.addr(3), vm.addr(4), vm.addr(5)];
@@ -284,10 +286,12 @@ contract BasePairTest is FraxlendPairConstants, Scenarios, Test {
         address[] memory _approvedLenders
     ) public {
         startHoax(COMPTROLLER_ADDRESS);
+
+        
         {
             pair = FraxlendPair(
                 deployer.deployCustom(
-                    "testname",
+                    deploySaltName,
                     _encodeConfigData(_normalization, _rateContractAddress, _initRateData),
                     _maxLTV,
                     _liquidationFee,

Output:

  Error: a == b not satisfied [uint]
    Expected: 2135773332009600000000
      Actual: 1541017987253789777725

Recommended Mitigation Steps

When maturity date passes and the last time _addInterest() was called was before maturity date, calculate first the interest rate for that interval as normal interest rate, and then calculate penalty rate for the remaining time.

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/b58c9b72f5fe8fab81f7436504e7daf60fd124e3/src/contracts/FraxlendPairCore.sol#L409-L497

Vulnerability details

addInterest() always multiplies the interest rate by the time delta, and then multiplies the total borrow amount by the results (newTotalBorrowAmount = interestRate * timeDelta * totalBorrowAmount). This can lead to different interest amount, depending on how frequently it's called. This will mostly affect custom pairs with stable interest rates (e.g. using linear rate with same min/max/vertex interest) which don't have much activity (e.g. whitelisted borrowers and lenders with long term loan).

The higher the interest rate the higher the difference will be, for example:

• 15% annual rate would turn into 13.9% if called only once a year (compared to once a day)
• 30% will turn into 26.2%
• 50% will turn to 40%
• 100% to ~70%
• 200% to 110%

Impact

Lenders might end up gaining less interest than expected.

Proof of Concept

The following test shows a case of 50% annual rate, comparing calling it once in 3 days and only once after a year, the difference would be about 4.5% of the final debt.

At src/test/e2e/BorrowPairTest.t.sol I've modified _fuzzySetupBorrowToken() to this and then ran source .env && forge test --fork-url $MAINNET_URL --fork-block-number $DEFAULT_FORK_BLOCK -m testFuzzyMaxLTVBorrowToken -vvv. And I've also modified src/test/e2e/BasePairTest.sol a bit so that each pair can have a different name for salt (see diff below).

 function _fuzzySetupBorrowToken(
        uint256 _minInterest,
        uint256 _vertexInterest,
        uint256 _maxInterest,
        uint256 _vertexUtilization,
        uint256 _maxLTV,
        uint256 _liquidationFee,
        uint256 _priceTop,
        uint256 _priceDiv
    ) public {
        asset = IERC20(FIL_ERC20);
        collateral = IERC20(MKR_ERC20);
        oracleMultiply = AggregatorV3Interface(CHAINLINK_MKR_ETH);
        oracleMultiply.setPrice(_priceTop, 1e8, vm);
        oracleDivide = AggregatorV3Interface(CHAINLINK_FIL_ETH);
        oracleDivide.setPrice(_priceDiv, 1e8, vm);
        uint256 _oracleNormalization = 1e18;
        (
            address _rateContract,
            bytes memory _rateInitData
        ) = fuzzyRateCalculator(
                2,
                12864358183, // ~ 50% annual rate - setting this is as min/max/vertex rate
                12864358183,  
                12864358183, 
                _vertexUtilization
            );
        startHoax(COMPTROLLER_ADDRESS);
        setWhitelistTrue();
        vm.stopPrank();

        address[] memory _borrowerWhitelist = _maxLTV >= LTV_PRECISION
            ? users
            : new address[](0);
        address[] memory _lenderWhitelist = _maxLTV >= LTV_PRECISION
            ? users
            : new address[](0);

        uint256 borrowerDebtWrong;
        {
            // wrong calculation, when addInterest isn't called before maturity date (30 days)
            deployFraxlendCustom(
                _oracleNormalization,
                _rateContract,
                _rateInitData,
                _maxLTV,
                _liquidationFee,
                0,
                1000 * DEFAULT_INT,
                _borrowerWhitelist,
                _lenderWhitelist
            );
            pair.updateExchangeRate();
            _borrowTest(_maxLTV, 15e20, 15e23);
            vm.roll(block.number + 1);


            for(uint256 i =0; i < 100; i++){
                vm.warp(block.timestamp + 3 days);
                vm.roll(block.number + 1); 
            }
            pair.addInterest();



            address borrower = users[2];
            uint256 borrowerShares = pair.userBorrowShares(borrower);
            borrowerDebtWrong = pair.toBorrowAmount(borrowerShares, false);
        }
        deploySaltName = "asdf2";
        uint256 borrowerDebtRight;
        {
            // relatively correct calculation, when addInterest is called close to maturity date 
            deployFraxlendCustom(
                _oracleNormalization,
                _rateContract,
                _rateInitData,
                _maxLTV,
                _liquidationFee,
                0,
                1000 * DEFAULT_INT,
                _borrowerWhitelist,
                _lenderWhitelist
            );
            pair.updateExchangeRate();
            _borrowTest(_maxLTV, 15e20, 15e23);

            for(uint256 i =0; i < 100; i++){
                vm.warp(block.timestamp + 3 days);
                pair.addInterest();
                vm.roll(block.number + 1); 

            }
            pair.addInterest();


            address borrower = users[2];
            uint256 borrowerShares = pair.userBorrowShares(borrower);
            borrowerDebtRight = pair.toBorrowAmount(borrowerShares, false);
        }

        // compare final debt between both scenarios
        assertEq(borrowerDebtRight, borrowerDebtWrong);
    }

diff --git a/src/test/e2e/BasePairTest.sol b/src/test/e2e/BasePairTest.sol
index 25114d9..27321dc 100644
--- a/src/test/e2e/BasePairTest.sol
+++ b/src/test/e2e/BasePairTest.sol
@@ -67,6 +67,8 @@ contract BasePairTest is FraxlendPairConstants, Scenarios, Test {
     PairAccounting public initial;
     PairAccounting public final_;
     PairAccounting public net;
+    string public deploySaltName = "asdf";
+
 
     // Users
     address[] public users = [vm.addr(1), vm.addr(2), vm.addr(3), vm.addr(4), vm.addr(5)];
@@ -284,10 +286,12 @@ contract BasePairTest is FraxlendPairConstants, Scenarios, Test {
         address[] memory _approvedLenders
     ) public {
         startHoax(COMPTROLLER_ADDRESS);
+
+        
         {
             pair = FraxlendPair(
                 deployer.deployCustom(
-                    "testname",
+                    deploySaltName,
                     _encodeConfigData(_normalization, _rateContractAddress, _initRateData),
                     _maxLTV,
                     _liquidationFee,

Output:

  Error: a == b not satisfied [uint]
    Expected: 2000166246155040000000
      Actual: 2092489655740553483735

Also, here's a JS function I've used to calculate the rate depending on frequency:

function calculateInterest(annualRate, frequency){
    let dailyRate = (1+annualRate) ** (1/365);
    let ratePerFrequency = (dailyRate -1) * 365 / frequency;
    let finalRate = (1+ratePerFrequency) ** frequency;
    return finalRate - 1;
}

Recommended Mitigation Steps

• Let the users know that in some cases not calling addInterest() frequently enough can lead to lower interest rates.
• Consider dividing the interest calculation into small period of times in case a long period has passed since last time addInterest() ran. Something along these lines (this shouldn't cost much more gas, as it's mostly math operations):

            // Calculate interest accrued
            if(_deltaTime > THRESHOLD){
                uint256 iterations = min(MAX_ITERATIONS, _deltaTime / THRESHOLD);
                uint256 multiplier = 1e36;
                for(uint i=0; i < iterations; i++){
                _   multiplier =  (multiplier * _deltaTime  * _currentRateInfo.ratePerSec) / (1e18 * iterations);
                }
                _interestEarned = ( _totalBorrow.amount * multiplier) / 1e36;

            }else{
                _interestEarned = (_deltaTime * _totalBorrow.amount * _currentRateInfo.ratePerSec) / 1e18;
            }

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/FraxlendPairDeployer.sol#L170-L178 https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/FraxlendPairDeployer.sol#L207-L210

Vulnerability details

In case of setting the creation code to less than 13K the creation would fail at 2 points:

• BytesLib.slice(_creationCode, 0, 13000) would fail since slice() requires that _bytes.length would be at least start + length.
• SSTORE2.read(contractAddress2) at _deployFirst() would fail since calling this on address without code would cause a math underflow (due to pointer.code.length - DATA_OFFSET)

Impact

In case the code of the pair gets smaller (e.g. optimization, moving part of the code to a library etc.) to 13K or less, it'd be impossible to set it as the new creation code (or in the case of the 2nd issue, it'd be impossible to deploy it)

Proof of Concept

In the following test I try to set creation code to a smaller mock pair. The test was added to src/test/e2e/FraxlendPairDeployerTest.sol:

    function testChangeCreationCodeBug() public {
        // Setup contracts
        setExternalContracts();
        startHoax(COMPTROLLER_ADDRESS);
        setWhitelistTrue();
        vm.stopPrank();
        console2.log("Mock pair size:", type(MockPair).creationCode.length);
        
        vm.startPrank(deployer.owner());
        deployer.setCreationCode(type(MockPair).creationCode);

        // Set initial oracle prices
        bytes memory _rateInitData = defaultRateInitForLinear();
        deployer.deploy(
            abi.encode(
                address(asset),
                address(collateral),
                address(oracleMultiply),
                address(oracleDivide),
                1e10,
                address(linearRateContract),
                _rateInitData
            )
        );

    }

The mock pair was created as src/contracts/audit/MockPair.sol:

// SPDX-License-Identifier: ISC
pragma solidity ^0.8.15;

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "@openzeppelin/contracts/security/Pausable.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/utils/math/SafeCast.sol";
import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
import "../FraxlendPairConstants.sol";
import "../libraries/VaultAccount.sol";
import "../libraries/SafeERC20.sol";
import "../interfaces/IERC4626.sol";
import "../interfaces/IFraxlendWhitelist.sol";
import "../interfaces/IRateCalculator.sol";
import "../interfaces/ISwapper.sol";

/// @title FraxlendPairCore
/// @author Drake Evans (Frax Finance) https://github.com/drakeevans
/// @notice  An abstract contract which contains the core logic and storage for the FraxlendPair
contract MockPair is FraxlendPairConstants, ERC20, Ownable, Pausable, ReentrancyGuard {
    using VaultAccountingLibrary for VaultAccount;
    using SafeERC20 for IERC20;
    using SafeCast for uint256;

    string public version = "1.0.0";

    // ============================================================================================
    // Settings set by constructor() & initialize()
    // ============================================================================================

    // Asset and collateral contracts
    IERC20 internal immutable assetContract;
    IERC20 public immutable collateralContract;

    // Oracle wrapper contract and oracleData
    address public immutable oracleMultiply;
    address public immutable oracleDivide;
    uint256 public immutable oracleNormalization;

    // LTV Settings
    uint256 public immutable maxLTV;

    // Liquidation Fee
    uint256 public immutable cleanLiquidationFee;
    uint256 public immutable dirtyLiquidationFee;

    // Interest Rate Calculator Contract
    IRateCalculator public immutable rateContract; // For complex rate calculations
    bytes public rateInitCallData; // Optional extra data from init function to be passed to rate calculator

    // Swapper
    mapping(address => bool) public swappers; // approved swapper addresses

    // Deployer
    address public immutable DEPLOYER_ADDRESS;

    // Admin contracts
    address public immutable CIRCUIT_BREAKER_ADDRESS;
    address public immutable COMPTROLLER_ADDRESS;
    address public TIME_LOCK_ADDRESS;

    // Dependencies
    address public immutable FRAXLEND_WHITELIST_ADDRESS;

    // ERC20 token name, accessible via name()
    string internal nameOfContract;

    // Maturity Date & Penalty Interest Rate (per Sec)
    uint256 public immutable maturityDate;
    uint256 public immutable penaltyRate;

    // ============================================================================================
    // Storage
    // ============================================================================================

    /// @notice Stores information about the current interest rate
    /// @dev struct is packed to reduce SLOADs. feeToProtocolRate is 1e5 precision, ratePerSec is 1e18 precision
    CurrentRateInfo public currentRateInfo;
    struct CurrentRateInfo {
        uint64 lastBlock;
        uint64 feeToProtocolRate; // Fee amount 1e5 precision
        uint64 lastTimestamp;
        uint64 ratePerSec;
    }

    /// @notice Stores information about the current exchange rate. Collateral:Asset ratio
    /// @dev Struct packed to save SLOADs. Amount of Collateral Token to buy 1e18 Asset Token
    ExchangeRateInfo public exchangeRateInfo;
    struct ExchangeRateInfo {
        uint32 lastTimestamp;
        uint224 exchangeRate; // collateral:asset ratio. i.e. how much collateral to buy 1e18 asset
    }

    // Contract Level Accounting
    VaultAccount public totalAsset; // amount = total amount of assets, shares = total shares outstanding
    VaultAccount public totalBorrow; // amount = total borrow amount with interest accrued, shares = total shares outstanding
    uint256 public totalCollateral; // total amount of collateral in contract

    // User Level Accounting
    /// @notice Stores the balance of collateral for each user
    mapping(address => uint256) public userCollateralBalance; // amount of collateral each user is backed
    /// @notice Stores the balance of borrow shares for each user
    mapping(address => uint256) public userBorrowShares; // represents the shares held by individuals
    // NOTE: user shares of assets are represented as ERC-20 tokens and accessible via balanceOf()

    // Internal Whitelists
    bool public immutable borrowerWhitelistActive;
    mapping(address => bool) public approvedBorrowers;

    bool public immutable lenderWhitelistActive;
    mapping(address => bool) public approvedLenders;
    event Fine(uint256 line);
    // ============================================================================================
    // Initialize
    // ============================================================================================

    /// @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
    )  ERC20("", "") {
        // Handle Immutables Configuration
        {
            (
                address _circuitBreaker,
                address _comptrollerAddress,
                address _timeLockAddress,
                address _fraxlendWhitelistAddress
            ) = abi.decode(_immutables, (address, address, address, address));

            // Deployer contract
            DEPLOYER_ADDRESS = msg.sender;
            CIRCUIT_BREAKER_ADDRESS = _circuitBreaker;
            COMPTROLLER_ADDRESS = _comptrollerAddress;
            TIME_LOCK_ADDRESS = _timeLockAddress;
            FRAXLEND_WHITELIST_ADDRESS = _fraxlendWhitelistAddress;
        }
        emit Fine(177);
        {
            (
                address _asset,
                address _collateral,
                address _oracleMultiply,
                address _oracleDivide,
                uint256 _oracleNormalization,
                address _rateContract,

            ) = abi.decode(_configData, (address, address, address, address, uint256, address, bytes));

            // Pair Settings
            assetContract = IERC20(_asset);
            collateralContract = IERC20(_collateral);
            currentRateInfo.feeToProtocolRate = DEFAULT_PROTOCOL_FEE;
            cleanLiquidationFee = _liquidationFee;
            dirtyLiquidationFee = (_liquidationFee * 9000) / LIQ_PRECISION; // 90% of clean fee

            if (_maxLTV >= LTV_PRECISION && !_isBorrowerWhitelistActive) revert BorrowerWhitelistRequired();
            maxLTV = _maxLTV;

            // Swapper Settings
            swappers[FRAXSWAP_ROUTER_ADDRESS] = true;

            // Oracle Settings
            {
                IFraxlendWhitelist _fraxlendWhitelist = IFraxlendWhitelist(FRAXLEND_WHITELIST_ADDRESS);
                // Check that oracles are on the whitelist
                if (_oracleMultiply != address(0) && !_fraxlendWhitelist.oracleContractWhitelist(_oracleMultiply)) {
                    revert NotOnWhitelist(_oracleMultiply);
                }

                if (_oracleDivide != address(0) && !_fraxlendWhitelist.oracleContractWhitelist(_oracleDivide)) {
                    revert NotOnWhitelist(_oracleDivide);
                }

                // Write oracleData to storage
                oracleMultiply = _oracleMultiply;
                oracleDivide = _oracleDivide;
                oracleNormalization = _oracleNormalization;

                // Rate Settings
                if (!_fraxlendWhitelist.rateContractWhitelist(_rateContract)) {
                    revert NotOnWhitelist(_rateContract);
                }
            }

            rateContract = IRateCalculator(_rateContract);
        }
        emit Fine(177);


        // Set approved borrowers whitelist
        borrowerWhitelistActive = _isBorrowerWhitelistActive;

        // Set approved lenders whitlist active
        lenderWhitelistActive = _isLenderWhitelistActive;

        // Set maturity date & penalty interest rate
        maturityDate = _maturityDate;
        penaltyRate = _penaltyRate;
    }


    function initialize(
        string calldata _name,
        address[] calldata _approvedBorrowers,
        address[] calldata _approvedLenders,
        bytes calldata _rateInitCallData
    ) external {
    }

}

Recommended Mitigation Steps

If creation code length is 13K or less:

• Don't run BytesLib.slice()
• at deployment read only from the first address

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/b58c9b72f5fe8fab81f7436504e7daf60fd124e3/src/contracts/FraxlendPairCore.sol#L194

Vulnerability details

Comment says dirtyLiquidationFee is supposed to be 90% of clean fee, but it's actually only 9%.

Impact

Liquidation fee would be 10% of what it's actually supposed to be. Making dirty liquidation much less profitable.

Proof of Concept

The code pretty much says it all, but here's a PoC I've added to src/test/e2e/FraxlendPairDeployerTest.sol:

    function testIsDirtyLiqFee90() public {
        // Setup contracts
        setExternalContracts();
        startHoax(COMPTROLLER_ADDRESS);
        setWhitelistTrue();
        vm.stopPrank();

        // Set initial oracle prices
        bytes memory _rateInitData = defaultRateInitForLinear();
        address pairAddress = deployer.deploy(
            abi.encode(
                address(asset),
                address(collateral),
                address(oracleMultiply),
                address(oracleDivide),
                1e10,
                address(linearRateContract),
                _rateInitData
            )
        );
        FraxlendPair pair = FraxlendPair(pairAddress);
        uint256 cleanFee = pair.cleanLiquidationFee();
        uint256 dirtyFee = pair.dirtyLiquidationFee();
        assertEq(dirtyFee, cleanFee * 90 / 100);
    }

Test will fail with:

 Error: a == b not satisfied [uint]
    Expected: 9000
      Actual: 900

Recommended Mitigation Steps

Add the missing zero

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/b58c9b72f5fe8fab81f7436504e7daf60fd124e3/src/contracts/FraxlendPairCore.sol#L944-L1032

Vulnerability details

liquidateClean doesn't check that the amount of shares the liquidator repays are covered by the collateral available, if the user repays more shares than (equally worth) collateral available - he'll get only the amount of collateral available.

Impact

Liquidator might end up paying assets that are worth much more than the collateral he's received. Even though it's also the liquidator's responsibility to check that there's enough collateral available, mistakes are commonly done (wrong calculation, typos, deadline too large) and the protocol should prevent those kinds of errors if possible. Otherwise users might loose trust in the protocol when they loose their funds due to errors.

Proof of Concept

The following test was added to src/test/e2e/LiquidationPairTest.sol. In this scenario the user ends up with collateral that's worth only 1/3 of the asset-tokens he paid.

    function testCleanLiquidate() public {
        // Setup contracts
        defaultSetUp();
        // Sets price to 3200 USD per ETH

        uint256 _amountToBorrow = 16e20; // 1.5k
        uint256 _amountInPool = 15e23; // 1.5m

        // collateral is 1.5 times borrow amount
        oracleDivide.setPrice(3200, 1, vm);
        mineBlocks(1);
        (, uint256 _exchangeRate) = pair.exchangeRateInfo();
        uint256 _collateralAmount = (_amountToBorrow * _exchangeRate * 3) / (2 * 1e18);
        faucetFunds(asset, _amountInPool);
        faucetFunds(collateral, _collateralAmount);
        lendTokenViaDeposit(_amountInPool, users[0]);
        borrowToken(uint128(_amountToBorrow), _collateralAmount, users[2]);
        uint256 mxltv = pair.maxLTV();
        uint256 cleanLiquidationFee = pair.cleanLiquidationFee();
        uint256 liquidation_price = ((((1e18 / _exchangeRate) * mxltv) / 1e5) * (1e5 + cleanLiquidationFee)) / 1e5;
        liquidation_price /= 4; // Collateral price goes down enough so that it doesn't cover the loan 
        oracleDivide.setPrice(liquidation_price, 1, vm);
        mineBlocks(1);
        uint128 _shares = pair.userBorrowShares(users[2]).toUint128();
        for (uint256 i = 0; i < 1; i++) {
            pair.addInterest();
            mineOneBlock();
        }
        vm.startPrank(users[1]);
        pair.addInterest();
        asset.approve(address(pair), toBorrowAmount(_shares, true));

        uint256 liquidatorBalanceBefore = asset.balanceOf(users[1]);
        
        uint256 collateralGained = pair.liquidateClean(_shares, block.timestamp, users[2]);

        uint256 liquidatorBalanceAfter = asset.balanceOf(users[1]);

        uint256 balanceDiff = liquidatorBalanceBefore - liquidatorBalanceAfter;

        console2.log("Balance diff is: ", balanceDiff);
        console2.log("_exchangeRate is: ", _exchangeRate);
        console2.log("cleanLiquidationFee is: ", cleanLiquidationFee);

        (, _exchangeRate) = pair.exchangeRateInfo();
        // 11/10 is for liquidation fee, 1e18 is for exchange precision
        assertEq(collateralGained, balanceDiff *  _exchangeRate * 11 / 10 / 1e18);

        assertEq(pair.userBorrowShares(users[2]), 0);
        vm.stopPrank();
    }

Output:

  Balance diff is:  1600000011384589113999
  _exchangeRate is:  863759326621800
  cleanLiquidationFee is:  10000
  Error: a == b not satisfied [uint]
    Expected: 7394958035811125166
      Actual: 2073022383892320000

Recommended Mitigation Steps

Check that the shares intended to be repaid are worth the collateral + fee, if not reduce the amount of shares to be repaid accordingly.

Judge has assessed an item in Issue #220 as Medium risk. The relevant finding follows:

Nominate admin to manage whitelists

Lines: https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/FraxlendPair.sol#L276-L312

Currently the lenders and borrowers whitelists are managed by themselves - approved borrower can add/remove any borrower from the whitelist and the same goes for approved lender. This isn't the best practice, since it's mixing the role of managing the whitelists and being an approved lender/borrower. This can lead to an approved borrower/lender (might be one added by mistake, a friend of a friend of a friend of a member of the original whitelist or a compromised wallet) taking over and removing all other members from the list.

Mitigation

Consider creating a different role of an admin that would manage the whitelists.

Missing zero approval before approval

USDT requires the approved amount to be zero before approving a non-zero amount. Even though the approved amount is expected to always be zero at the beginning of tx (since the swapper always uses all of the approved amount), it's better to approve zero to avoid any kind of errors (even if there's 1 wei of USDT left approved due to some error or rounding with the swapper, it'll make the functions that use approval unusable).

Code: https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairCore.sol#L1184 https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairCore.sol#L1103

multiple instances of public pairs with same config can be created, due to _configData length not being checked

The deployer tried to prevent creation of multiple pairs with same config data by hashing the config data and checking if a pair with a similar hash has been created. However, this can by bypassed by padding the _configData with random data. Since abi.decode() doesn't check the input bytes size, this will still pass as a valid config and create pair with same config as without the padding (while the hash would be different due to the padding).

PoC

testCannotDeployTwicePublic() would fail with Call did not revert as expected when applying following diff:

diff --git a/src/test/e2e/FraxlendPairDeployerTest.sol b/src/test/e2e/FraxlendPairDeployerTest.sol
index 18eeb1e..e1c836c 100644
--- a/src/test/e2e/FraxlendPairDeployerTest.sol
+++ b/src/test/e2e/FraxlendPairDeployerTest.sol
@@ -39,7 +39,8 @@ contract FraxlendPairDeployerTest is BasePairTest {
                 address(oracleDivide),
                 1e10,
                 address(linearRateContract),
-                _rateInitData2
+                _rateInitData2,
+                address(oracleDivide)
             )
         );
     }

Mitigation

Check that the config data is the expected length (it contains encoded bytes of _rateInitData2, but that should have a fixed length too).

Admin can bypass timelock by replacing it

The ability to change the timelock address without any delay defeats the whole purpose of the timelock address. The idea of the timelock is to delay admin actions and prevent any surprises to users, however if the admin can replace the contract without any delay, he can simply replace it with a non-timelock address and execute actions immediately.

Mitigation

Make the setTimeLock a 2 step actions, with a delay (the same delay as the timelock contract) between.

Move division to after second multiplication, to get a more precise results

Code: https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairCore.sol#L929-L932 https://github.com/code-423n4/2022-08-frax/blob/b58c9b72f5fe8fab81f7436504e7daf60fd124e3/src/contracts/FraxlendPairCore.sol#L970-L991 https://github.com/code-423n4/2022-08-frax/blob/b58c9b72f5fe8fab81f7436504e7daf60fd124e3/src/contracts/FraxlendPairCore.sol#L314

In order to loose less to rounding it's better to move any division to after multiplication:

Mitigation diff:

diff --git a/src/contracts/FraxlendPairCore.sol b/src/contracts/FraxlendPairCore.sol
index a712d46..58cba44 100644
--- a/src/contracts/FraxlendPairCore.sol
+++ b/src/contracts/FraxlendPairCore.sol
@@ -311,7 +311,7 @@ abstract contract FraxlendPairCore is FraxlendPairConstants, IERC4626, ERC20, Ow
         uint256 _collateralAmount = userCollateralBalance[_borrower];
         if (_collateralAmount == 0) return false;
 
-        uint256 _ltv = (((_borrowerAmount * _exchangeRate) / EXCHANGE_PRECISION) * LTV_PRECISION) / _collateralAmount;
+        uint256 _ltv = ((_borrowerAmount * _exchangeRate) * LTV_PRECISION) / (_collateralAmount * EXCHANGE_PRECISION);
         return _ltv <= maxLTV;
     }
 
@@ -927,9 +927,9 @@ abstract contract FraxlendPairCore is FraxlendPairConstants, IERC4626, ERC20, Ow
 
         // Determine how much of the borrow and collateral will be repaid
         _collateralForLiquidator =
-            (((_totalBorrow.toAmount(_shares, false) * _exchangeRate) / EXCHANGE_PRECISION) *
+            ((_totalBorrow.toAmount(_shares, false) * _exchangeRate) *
                 (LIQ_PRECISION + cleanLiquidationFee)) /
-            LIQ_PRECISION;
+            (LIQ_PRECISION * EXCHANGE_PRECISION);
 
         // Effects & Interactions
         // NOTE: reverts if _shares > userBorrowShares
@@ -972,12 +972,12 @@ abstract contract FraxlendPairCore is FraxlendPairConstants, IERC4626, ERC20, Ow
             // Checks & Calculations
             // Determine the liquidation amount in collateral units (i.e. how much debt is liquidator going to repay)
             uint256 _liquidationAmountInCollateralUnits = ((_totalBorrow.toAmount(_sharesToLiquidate, false) *
-                _exchangeRate) / EXCHANGE_PRECISION);
+                _exchangeRate) );
 
             // We first optimistically calculate the amount of collateral to give the liquidator based on the higher clean liquidation fee
             // This fee only applies if the liquidator does a full liquidation
             uint256 _optimisticCollateralForLiquidator = (_liquidationAmountInCollateralUnits *
-                (LIQ_PRECISION + cleanLiquidationFee)) / LIQ_PRECISION;
+                (LIQ_PRECISION + cleanLiquidationFee)) / (LIQ_PRECISION * EXCHANGE_PRECISION);
 
             // 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)
@@ -987,7 +987,7 @@ abstract contract FraxlendPairCore is FraxlendPairConstants, IERC4626, ERC20, Ow
             // This will only be true when there liquidator is cleaning out the position
             _collateralForLiquidator = _leftoverCollateral <= 0
                 ? _userCollateralBalance
-                : (_liquidationAmountInCollateralUnits * (LIQ_PRECISION + dirtyLiquidationFee)) / LIQ_PRECISION;
+                : (_liquidationAmountInCollateralUnits * (LIQ_PRECISION + dirtyLiquidationFee)) / (LIQ_PRECISION * EXCHANGE_PRECISION);
         }
         // Calced here for use during repayment, grouped with other calcs before effects start
         uint128 _amountLiquidatorToRepay = (_totalBorrow.toAmount(_sharesToLiquidate, true)).toUint128();

Replace constant expression with its value

Code: https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairDeployer.sol#L329

Gas saved: ~140 units Replace keccak256(abi.encodePacked("public")) with its value:

diff:

diff --git a/src/contracts/FraxlendPairDeployer.sol b/src/contracts/FraxlendPairDeployer.sol
index f8d959e..1e634ce 100644
--- a/src/contracts/FraxlendPairDeployer.sol
+++ b/src/contracts/FraxlendPairDeployer.sol
@@ -326,7 +326,7 @@ contract FraxlendPairDeployer is Ownable {
         );
 
         _pairAddress = _deployFirst(
-            keccak256(abi.encodePacked("public")),
+            0x3c0c3a2537aaf75b853bbf2b595e872d3db0359b7e792ebd8907fb017c3b71a2,
             _configData,
             abi.encode(CIRCUIT_BREAKER_ADDRESS, COMPTROLLER_ADDRESS, TIME_LOCK_ADDRESS, FRAXLEND_WHITELIST_ADDRESS),
             DEFAULT_MAX_LTV,

gas report diff:

 ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
 │ Function Name                                                        ┆ min             ┆ avg     ┆ median  ┆ max     ┆ # calls │
 ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
-│ deploy                                                               ┆ 12860           ┆ 5372981 ┆ 5642803 ┆ 5710008 ┆ 20      │
+│ deploy                                                               ┆ 12728           ┆ 5372838 ┆ 5642660 ┆ 5709865 ┆ 20      │
 ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
 │ deployCustom                                                         ┆ 5424            ┆ 3597248 ┆ 5560292 ┆ 5818462 ┆ 8       │

Make storage vars immutable

https://github.com/code-423n4/2022-08-frax/blob/92a8d7d331cc718cd64de6b02515b554672fb0f3/src/contracts/FraxlendPairDeployer.sol#L56-L59

FraxLendPairDeployer

FraxLendPairDeployer doesn't change the following addresses, these can be changed to immutable: FraxLendPairDeployer.sol:

    // Admin contracts
    address public CIRCUIT_BREAKER_ADDRESS;
    address public COMPTROLLER_ADDRESS;
    address public TIME_LOCK_ADDRESS;

FraxlendPairCore

The rateInitCallData takes 5 storage slots (4 uint256 + one slot for bytes size), this will cost 10.5K every time it's loaded (= almost every time _addInterest() is called = almost every tx). bytes variable can't be immutable since it has a dynamic size, but it can be decoded into 4 immutable uint256 vars and then encoded back to bytes at runtime. This would be MUCH cheaper since encoding shouldn't cost more than a few hundred gas units.

Note: This gas optimization wouldn't show on forge tests, the reason is that forge considers each test as one tx (even when you change block number & timestamp). Therefore, rateInitCallData storage variable is still considered to be 'warm' (i.e. used in the last tx) which reduces the cost per sload from 2.1K to just 0.1K (see evm.codes). However, if you'd test it with Hardhat I'm certain you'd see the difference.

Optimization diff:

diff --git a/src/contracts/FraxlendPairCore.sol b/src/contracts/FraxlendPairCore.sol
index a712d46..c7be423 100644
--- a/src/contracts/FraxlendPairCore.sol
+++ b/src/contracts/FraxlendPairCore.sol
@@ -63,6 +63,11 @@ abstract contract FraxlendPairCore is FraxlendPairConstants, IERC4626, ERC20, Ow
     address public immutable oracleDivide;
     uint256 public immutable oracleNormalization;
 
+    uint256 immutable _minInterest;
+    uint256 immutable _vertexInterest;
+    uint256 immutable _maxInterest;
+    uint256 immutable _vertexUtilization;
+
     // LTV Settings
     uint256 public immutable maxLTV;
 
@@ -72,7 +77,7 @@ abstract contract FraxlendPairCore is FraxlendPairConstants, IERC4626, ERC20, Ow
 
     // Interest Rate Calculator Contract
     IRateCalculator public immutable rateContract; // For complex rate calculations
-    bytes public rateInitCallData; // Optional extra data from init function to be passed to rate calculator
+    // bytes public rateInitCallData; // Optional extra data from init function to be passed to rate calculator
 
     // Swapper
     mapping(address => bool) public swappers; // approved swapper addresses
@@ -183,9 +188,26 @@ abstract contract FraxlendPairCore is FraxlendPairConstants, IERC4626, ERC20, Ow
                 address _oracleDivide,
                 uint256 _oracleNormalization,
                 address _rateContract,
-
+                bytes memory _rateInitData
             ) = abi.decode(_configData, (address, address, address, address, uint256, address, bytes));
 
+            {
+                uint256 temp1;
+                uint256 temp2;
+                uint256 temp3;
+                uint256 temp4;
+                if(_rateInitData.length > 0)
+                {
+                    // (_minInterest,  _vertexInterest, _maxInterest, _vertexUtilization) = (5,6,7,8);
+                    (temp1, temp2, temp3, temp4) = abi.decode(
+                        _rateInitData,
+                        (uint256, uint256, uint256, uint256)
+                    );
+                }
+                (_minInterest,  _vertexInterest, _maxInterest, _vertexUtilization) = (temp1, temp2, temp3, temp4);
+            }
+
+
             // Pair Settings
             assetContract = IERC20(_asset);
             collateralContract = IERC20(_collateral);
@@ -275,7 +297,6 @@ abstract contract FraxlendPairCore is FraxlendPairConstants, IERC4626, ERC20, Ow
         IRateCalculator(rateContract).requireValidInitData(_rateInitCallData);
 
         // Set rate init Data
-        rateInitCallData = _rateInitCallData;
 
         // Instantiate Interest
         _addInterest();
@@ -300,6 +321,14 @@ abstract contract FraxlendPairCore is FraxlendPairConstants, IERC4626, ERC20, Ow
         return _totalAsset.amount - _totalBorrow.amount;
     }
 
+    // `testPreviewInterestRate` test breaks without this
+    // You can remove this once you get to the root of the issue
+    function rateInitCallData() public view returns(bytes memory){
+        return abi.encode(
+                    _minInterest, _vertexInterest,  _maxInterest, _vertexUtilization
+                ); 
+    }
+
     /// @notice The ```_isSolvent``` function determines if a given borrower is solvent given an exchange rate
     /// @param _borrower The borrower address to check
     /// @param _exchangeRate The exchange rate, i.e. the amount of collateral to buy 1e18 asset
@@ -447,6 +476,10 @@ abstract contract FraxlendPairCore is FraxlendPairConstants, IERC4626, ERC20, Ow
             if (_isPastMaturity()) {
                 _newRate = uint64(penaltyRate);
             } else {
+
+                bytes memory rateInitCallData = abi.encode(
+                    _minInterest, _vertexInterest,  _maxInterest, _vertexUtilization
+                );             
                 bytes memory _rateData = abi.encode(
                     _currentRateInfo.ratePerSec,
                     _deltaTime,

Custom errors instead of require-string

Even though custom errors are widely used, there are still some places where require-string is used (e.g. FraxlendPairDeployer.sol#L208). Replacing that with a custom error can save deployment gas / code size and gas in case of reverts on those requirements.