The intended way for a protocol parameter to change is by providing an interval of time in which the parameter will proportionally change and the percentage to shift compared with the current parameter value. That's how the upper and lower ranges and upper and lower thresholds works. However, when an admin wants to update the center price it does not work as expected. The function that admins call in order to change the center price is the following: ``` function updateCenterPriceAddress(uint centerPriceAddress_, uint percent_, uint time_) public _check _onlyDelegateCall { if ( (centerPriceAddress_ > X30) || (percent_ == 0) || (percent_ > X20) || (time_ == 0) || (time_ > X20) ) { revert FluidDexError(ErrorTypes.DexT1Admin__ConfigOverflow); } if (centerPriceAddress_ > 0) { IFluidDexT1.ConstantViews memory c_ = IFluidDexT1(address(this)).constantsView(); address centerPrice_ = AddressCalcs.addressCalc(c_.deployerContract, centerPriceAddress_); _checkIsContract(centerPrice_); // note: if address is made 0 then as well in the last swap currentPrice is updated on storage, so code will start using that automatically dexVariables2 = (dexVariables2 & 0xFeFFFFFFFFFFFFFFFFFFFFFFFFFFC0000000FFFFFFFFFFFFFFFFFFFFFFFFFFFF) | (centerPriceAddress_ << 112) | uint(1) << 248; _centerPriceShift = block.timestamp | (percent_ << 33) | (time_ << 53); } else { dexVariables2 = (dexVariables2 & 0xFeFFFFFFFFFFFFFFFFFFFFFFFFFFC0000000FFFFFFFFFFFFFFFFFFFFFFFFFFFF); _centerPriceShift = 0; } emit LogUpdateCenterPriceAddress(centerPriceAddress_, percent_, time_); } ``` As we can see, this function only sets the nonce to get the address of the external contract to obtain the new center price, and sets the price shift as active saving the timestamp when this function has been called, the percentage and time during this shift will be active. Let's now see the flow for a function: ``` function swapIn( bool swap0to1_, uint256 amountIn_, uint256 amountOutMin_, address to_ ) public payable returns (uint256 amountOut_) { ... PricesAndExchangePrice memory pex_ = _getPricesAndExchangePrices(dexVariables_, dexVariables2_); ... dexVariables = _updateOracle( s_.price, pex_.centerPrice, dexVariables_ ); emit Swap(swap0to1_, amountIn_, amountOut_, to_); } ``` It first computes prices and at the end of the swap, it calls the `_updateOracle` function. ``` function _getPricesAndExchangePrices( uint dexVariables_, uint dexVariables2_ ) internal returns ( PricesAndExchangePrice memory pex_ ) { uint centerPrice_; if (((dexVariables2_ >> 248) & 1) == 0) { ... } } else { // an active centerPrice_ shift is going on centerPrice_ = _calcCenterPrice(dexVariables_, dexVariables2_); } ... } function _calcCenterPrice(uint dexVariables_, uint dexVariables2_) internal returns (uint newCenterPrice_) { uint oldCenterPrice_ = (dexVariables_ >> 81) & X40; oldCenterPrice_ = (oldCenterPrice_ >> DEFAULT_EXPONENT_SIZE) << (oldCenterPrice_ & DEFAULT_EXPONENT_MASK); uint centerPriceShift_ = _centerPriceShift; uint startTimeStamp_ = centerPriceShift_ & X33; uint percent_ = (centerPriceShift_ >> 33) & X20; uint time_ = (centerPriceShift_ >> 53) & X20; uint fromTimeStamp_ = (dexVariables_ >> 121) & X33; // Maximum between start timestamp from the centerPriceShift and last interaction timestamp fromTimeStamp_ = fromTimeStamp_ > startTimeStamp_ ? fromTimeStamp_ : startTimeStamp_; newCenterPrice_ = ICenterPrice(AddressCalcs.addressCalc(DEPLOYER_CONTRACT, ((dexVariables2_ >> 112) & X30))).centerPrice(); uint priceShift_ = (oldCenterPrice_ * percent_ * (block.timestamp - fromTimeStamp_)) / (time_ * SIX_DECIMALS); if (newCenterPrice_ > oldCenterPrice_) { // shift on positive side oldCenterPrice_ += priceShift_; if (newCenterPrice_ > oldCenterPrice_) { newCenterPrice_ = oldCenterPrice_; } else { // shifting fully done delete _centerPriceShift; // making active shift as 0 because shift is over // fetching from storage and storing in storage, aside from admin module dexVariables2 only updates these shift function. dexVariables2 = dexVariables2 & ~uint(1 << 248); } } else { unchecked { oldCenterPrice_ = oldCenterPrice_ > priceShift_ ? oldCenterPrice_ - priceShift_ : 0; // In case of oldCenterPrice_ ending up 0, which could happen when a lot of time has passed (pool has no swaps for many days or weeks) // then below we get into the else logic which will fully conclude shifting and return newCenterPrice_ // as it was fetched from the external center price source. // not ideal that this would ever happen unless the pool is not in use and all/most users have left leaving not enough liquidity to trade on } if (newCenterPrice_ < oldCenterPrice_) { newCenterPrice_ = oldCenterPrice_; } else { // shifting fully done delete _centerPriceShift; // making active shift as 0 because shift is over // fetching from storage and storing in storage, aside from admin module dexVariables2 only updates these shift function. dexVariables2 = dexVariables2 & ~uint(1 << 248); } } } ``` One of the computations inside of the `_getPricesAndExchangePrices` is the center price. Inside it, we can see that it takes the old center price from the position 81 bit from the `dexVariables` and it applies to it the changes set by the admin. It takes the previous center price, applies the percentage towards the new center price fetched externally. After that it computes the amount of time between either the start of the shift of the last interaction timestamp and divides it by the total time that the shift should happen. After that, at the end of the swap we can see that the new computed center price is stored in the same position in storage: ``` function _updateOracle( uint newPrice_, uint centerPrice_, uint dexVariables_ ) internal returns (uint) { // time difference between last & current swap uint timeDiff_ = block.timestamp - ((dexVariables_ >> 121) & X33); uint temp_; uint temp2_; uint temp3_; ... // doing this due to stack too deep error when using params memory variables temp_ = newPrice_; // <-- temp2_ = centerPrice_; temp3_ = dexVariables_; // then update last price temp3_ = (temp3_ & 0xfffffffffffffff8000000000000000000000000000000000000000000000001) | (((temp3_ >> 41) & X40) << 1) | (temp_.toBigNumber(32, 8, BigMathMinified.ROUND_DOWN) << 41) | // <-- (temp2_.toBigNumber(32, 8, BigMathMinified.ROUND_DOWN) << 81) | (block.timestamp << 121) | (timeDiff_ << 154) | (nextOracleSlot_ << 176) | (oracleMap_ << 179); } else { ... } return temp3_; } ``` There is clearly an issue here because the percentage should be taken on the center price when the price shift started and propotionllay compute the price from the date it started. However, it computes the percentage from the previously updated price which will be updated in each swap. Furthermore the time elapsed should be computed always from the start of the price shift instead of the maximum between this and the last interaction timestamp. Examples of why this does not work as intended: - Current center price is 60000 USDC/WBTC - Admin calls the `updateCenterPriceAddress` with a +30% change during 1000 seconds The intended behavior should be: - If there is a swap after 200 seconds from the price shift start it should return: ``` 60000 + 60000 * 0.3 * 200 / 1000 = 63600 ``` - If there is a swap after 500 seconds from the price shift start it should return: ``` 60000 + 60000 * 0.3 * 500 / 1000 = 69000 ``` - If there is a swap after 900 seconds from the price shift start it should return: ``` 60000 + 60000 * 0.3 * 900 / 1000 = 76200 ``` At the end, the final price should be 78000. Each of these prices are proportional from the start of the shift until the end proportional to the price when the shift started. However, the current behavior is: - If there is a swap after 200 seconds from the price shift start it should return: ``` 60000 + 60000 * 0.3 * 200 / 1000 = 63600 ``` - If there is a swap after 500 seconds from the price shift start it should return: ``` 63600 + 63600 * 0.3 * (500 - 200) / 1000 = 69324 ``` - If there is a swap after 900 seconds from the price shift start it should return: ``` 69324 + 69324 * 0.3 * (900 - 500) / 1000 = 77642 ``` As we can see the prices in each timestamp are different from the expected ones. The impact of this issue could be considered medium and high likelihood due to happening in every swap. The impact is the price increasing too quickly and reaching the price before expected. Instead, it should compute the price proportional to the time elapsed. Example of price increasing too quick: Swap happens evey 50 seconds, the price will evolve as follows: ``` Timestamp 0: 60000 Timestamp 50: 60900 Timestamp 100: 61813 Timestamp 150: 62740 Timestamp 200: 63681 Timestamp 250: 64636 Timestamp 300: 65605 Timestamp 350: 66589 Timestamp 400: 67587 Timestamp 450: 68600 Timestamp 500: 69629 Timestamp 550: 70673 Timestamp 600: 71733 Timestamp 650: 72808 Timestamp 700: 73900 Timestamp 750: 75008 Timestamp 800: 76133 Timestamp 850: 77274 Timestamp 900: 78433 Timestamp 950: 79609 Timestamp 1000: 80803 ``` As we can see, after 900 seconds have elapsed it already reached the 78000 final price without reaching all the required time. There are 2 main issues here: 1. The percentage is computed from the updated value instead of the old value. The percentage that the admin set during the `updateCenterPriceAddress` call is based on the center price of this moment, hence it should not change until the shift has been completed. However, as we have seen, it updates the center price in the `dexVariables` and the percentage is computed based on the updated value of every swap. 2. The time that the code considers elapsed is wrong, it takes the maximum value between the initiation of the price shift and the last interaction of the dex as the initial value and it substracts it to the current timestamp. This is problematic because any activity on the dex after the `updateCenterPriceAddress` will make the time elapsed to be just the amount of time between now and the last interaction. Hence it will not work as expected.

Competition	Date	Submissions	Payout
Daao	Jan 27, 2025	9	$339
Infrared Finance	Jan 15, 2025	1	$0
Instadapp	Sep 11, 2024	2	$8,950

Draiakoo

Public reviews