# f(x) Review By [Dark Bible](https://paragraph.com/@dark-bible) ยท 2023-11-13 --- I am an independent crypto researcher, I was very interested in [f(x) protocol](https://fx.aladdin.club/) and decided to look into the codebase. Here are my notes of the code dive, I would like to shared with the community, so you know the risks involved when you use f(x) protocol. You can contact me on X at [@codephobic](https://twitter.com/codephobic) Review Date : 1 Nov 2023, any changes made after this date is not reviewed. Scope : * Market.sol * HarvestableTreasury.sol * FractionalToken.sol * StableCoinMath.sol * ReservePool.sol Non tech summaries for users ============================ 1. If 6 out of 9 current [multi sig](https://etherscan.io/address/0x26B2ec4E02ebe2F54583af25b647b1D619e67BbF#readProxyContract) owners collude, they can upgrade the protocol and steal everything. By using this system, you are trusting these 9 accounts. details in 1. 2. xToken holders might not be able to redeem for stETH under certain conditions, forcing them to stuck with a volatile risky asset. details in 4. 3. The system is designed to sacrifice fToken holder's interests to protect the system under certain situations, e.g. when the system collateral ration drops below stability ratio (130%) and when liquidation happens. during these times, the system will artificially drop fToken value in order to create incentives to reward users for bringing back the collateral ratio. Details in 5. 4. fToken and xToken holders should redeem their tokens asap if the protocol becomes insolvent. The fx protocol uses a strategy contract to invest user's stETH collateral, in case anything goes wrong causing the protocol to become insolvent, the redeem function will stop giving back stETH after it runs out, but it will still burn the redeemed fToken and xTokens just not giving anything back, late users could end up with nothing. Issues ====== 1\. Admin could steal everything by upgrading the protocol contracts -------------------------------------------------------------------- Fx's smart contracts like Market, Treasury are designed to be upgradable using a proxy pattern. This means the admin controlling the upgradability is able to do anything they want by upgrading the logic of these smart contracts including stealing all user funds. The current admin is a [multi sig](https://etherscan.io/address/0x26B2ec4E02ebe2F54583af25b647b1D619e67BbF#readProxyContract) with 6 out of 9 owners threshold meaning it would take 6 out of the 9 owners to collude in order to maliciously update the contract. It is a fairly common practice to use upgradable smart contracts with a multi sig handling upgradability, it does require a lot of trust tho. Recommendation - make the protocol immutable by renouncing admin. 2\. Proxy implementation storages are not initialized ----------------------------------------------------- Some current proxy implementation storages like [Market](https://etherscan.io/address/0x31bfaf256875a93b5d02cdef80b17ef780663d81#readContract) and [Treasury](https://etherscan.io/address/0x1951fb29ee3ed3026ed7ce2beee3194d90fbb2bc#readContract) are not initialized, anyone can call initialize to become owner of these implementation contracts. Although there are currently no immediate risks associated with this issue since these storages are not in use, it is not the best practice to leave implementations uninitialized, we have previously seen attack vectors associated with uninitialized implementations. Recommendation - initialize these unused storage to avoid any potential future attack vectors. 3\. Unused variable ------------------- `selfLiquidationRatio` in `Market.MarketConfig` struct is currently not used in the system, can be removed. 4\. User might not be able to redeem xTokens under certain conditions --------------------------------------------------------------------- The system is designed to not allow redeem of xTokens if both of the following conditions are met 1. the setting `xTokenRedeemInSystemStabilityModePaused` is set to true (currently set to false which could be turned on by the protocol) 2. system collateral ratio is <= 130% When this happens, any attempt to redeem xTokens to for stETH will be capped at the amount coresponding to 130% collateral ratio, further redemptions will not work. This design is a mechanism to prevent the system's collateral ratio from further falling below 130% at the cost of xToken holder's ability to exit. This is a very unfriendly design for xToken holders due to the leveraging nature of xTokens, guaranteed exit liquidity is important. This could lead to a bank run when collateral ratio drops close to 130%, with last redeemed users get stuck with the volatile xTokens. Recommendations - unlike restrictions on minting, restricting redemption is not a common design for Defi protocols, even if it is cents on a dollar, the users should have the option to exit at anytime. Consider change the design to allow redemption at anytime. 5\. fToken holders will be the ones paying when the collateral ratio falls below 130% or when liquidation happens ----------------------------------------------------------------------------------------------------------------- The system is designed to allow anyone to call the function `addBaseToken()` to mint xTokens at a discount and push up the collateral ratio when it falls below the stability ratio (currently 130%). The discount comes from artificially drop fToken's price during this function, this means in case of stETH price drop and pushed Collateral Ratio below 130%, all fToken holders will not only be paying for the 10% price drop agreed by fTokens, but also the incentives paid to anyone who mint xTokens with the function `addBaseToken()`. Similarly during a liquidation, similar design is also used to drop fToken value in order to create an incentive. This again is not a very friendly design for fToken holders as they are expecting to hold a relatively stable asset with limited exposure to the base asset. 6\. Treasury function `mint()` could fail early in order to save gas -------------------------------------------------------------------- The `Treasury.mint()` function checks if base token will exceed the allowance cap in line 367, require(_state.baseSupply + _baseIn <= baseTokenCap, "Exceed total cap"); This check can be placed higher in the function right after `_state` is out in line 332 in order to fail early and safe gas. --- *Originally published on [Dark Bible](https://paragraph.com/@dark-bible/f-x-review)*