The Auditor’s Digest is a series of articles dedicated to a comprehensive security outlook at various code components in Defi. In this article we elaborate mistakes made by developers when they implement TWAP algorithms on their own for fetching prices. More information about TWAP here.
Wrong units of TWAP interval
When implementing functions to calculate/fetch TWAP based on a dynamic interval passed as a parameter, make sure to validate the intended time unit of the parameter supplied. One way this can go wrong is when you want a 15 minute twap from a function so you pass in getPrice(15) but the function was supposed to handle it as 15 seconds, so now you have a much shorter twap window leading to inaccurate values. Mostly, this value should correspond to seconds.
Example : TWAP#1
function getPositionValue() public view returns (uint256) {
uint256 markPrice = getMarkPriceTwap(15); //@audit here it should be 900 seconds
int256 positionSize = IAccountBalance(clearingHouse.getAccountBalance())
.getTakerPositionSize(address(this), market);
return markPrice.mulWadUp(_abs(positionSize));
}
function getMarkPriceTwap(uint32 twapInterval)
public
view
returns (uint256)
{
IExchange exchange = IExchange(clearingHouse.getExchange());
uint256 markPrice = exchange
.getSqrtMarkTwapX96(market, twapInterval)
.formatSqrtPriceX96ToPriceX96()
.formatX96ToX10_18();
return markPrice;
}Recommendation is to verify by matching time value passed to a price function with expected units.
Incorrect weightage in double-oracle setup
When you use multiple sources of price and calculate the final using weighting of each source (ex. coupling of chainlink-balancer TWAP oracle), make sure that reasonable weightage is given to each value. It shouldn’t be too much or too less (carefully assess the risks), otherwise it will not calculate the reported price as desired and defeat the purpose of price feed stability. On the extreme side, extreme dependence on one source may lead to disastrous consequences if the overweighted source fails.
This type of setup is usually prone to mathematical mistakes and precision errors as the root problem.
Example : TWAP#2
uint256 internal constant BALANCER_ORACLE_WEIGHT_PRECISION = 1e8; //@audit
// this precision is incorrectly used to set the weightage
uint32 internal constant SLIPPAGE_LIMIT_PRECISION = 1e8;
/// @notice Precision for all percentages used by the vault
/// 1e4 = 100% (i.e. maxBalancerPoolShare)
uint16 internal constant VAULT_PERCENT_BASIS = 1e4;Recommendation is to employ fuzzing and careful mathematical analysis to make sure that weightage is correct in all situations to give the setup a meaningful value. Also, consider not hardcoding the weightage so that it can be adjusted based on market/pool conditions.
TWAP oracle update mechanism can’t be left to users
In unison with the uniswap twap methods, since most implementations calculate the average price from data points of many periods, there needs to be a guarantee that an oracle update will fire in all of these periods (mostly defined as 1 ethereum block).
The oracle update should be a part of a fixed functionality user-action (like how uniswap stores values on every swap/trade) to update the current price into the state. Discontinuous reporting will break the concept of TWAP and lead to unequal weightings to certain data points, meaning inaccurate prices.
Other ways of solving this problem are : Consider checking that the price has been updated in a reasonable timeframe, or employ a backup oracle update keeper (if sometime there is no corresponding user-action in a period). If you are not sure that the price will be kept updated by user actions, then maybe use moving averages instead of a fixed periodic update.
If your implementation doesn’t use fix time period but calculates dynamically, the specific durations a value existed for need to be used for every recorded entry to reach an accurate final price.
{TWAP = (Σ price*time duration)/ desired twap window}
Example : TWAP#3, one more, one more
File: /src/modules/PRICE.sol #1
134 // Calculate new moving average
135 if (currentPrice > earliestPrice) {
136 _movingAverage += (currentPrice - earliestPrice) / numObs;
137 } else {
138 _movingAverage -= (earliestPrice - currentPrice) / numObs;
139 }
140
141 // Push new observation into storage and store timestamp taken at
142 observations[nextObsIndex] = currentPrice;
143 lastObservationTime = uint48(block.timestamp);
144: nextObsIndex = (nextObsIndex + 1) % numObs;Recommendation is to have evenly-spaced time-weightage (that is explicitly defined) instead of observation-based calculation, or the calculation should take into account the duration between these observations.
TWAP oracle updates can be stuck
One type of implementation allows only one oracle update to fire within a defined period, at the end of it (like how uniswap updates twap state with a delay of 1 block). This helps with price-spamming problems and efficient calculation, also saves user gas. However, the time duration check can be used incorrectly and lead to opposite behavior ie. update only if the slot has not passed already. While this can work okay, oracle updates will be permanently stuck if the update fails in one slot.
Example : TWAP#4
// ensure that at least one full period has passed since the last update
- if (timeElapsed >= PERIOD) {
+ if (timeElapsed < PERIOD) {
return;
}Recommendation is to allow oracle value updates only after the slot has passed, instead of before the slot has passed.
TWAP can become a spot oracle
While we have been warned enough against spot price oracles by samczsun, the twap implementation maybe using small windows such that it ultimately resonates with the spot price. The twap interval needs to be carefully decided based on risk parameters (accuracy<>manipulation resistance).
The example here was using 1 block price as the twap, which is very bad usage. Remember that manipulation cost increases with the size of TWAP window. Also, the twap interval should be flexible(upgradeable) : to be readjusted according to market conditions, and adjusted across assets (based on liquidity and activity).
Example : TWAP#5, one more, one more
// Capture oracle reading every 0 minutes ?
uint constant periodSize = 0;Recommendation is to use a standard twap interval such as 1800 seconds or decide based on risk assessment of the asset/source. And do not hardcode it (for e.g 1-hour twap might lag considerably in sudden movements).
Uniswap V2 TWAPs cannot be inverted
If the Uni V2 TWAP of token0/token1 is x, then twap of token1/token0 can’t simply be calculated by inverting the mean ie. 1/x. The inverse prices in each period inside the window need to be tracked properly in the cumulative and directly calculated from there.
Inverting is great in spot prices but it causes large large error in twap because of how arithmetic mean works. This may lead to highly inaccurate prices leading to unfair liquidations etc. The more volatile the price of token0/token1, the more extreme this result is.
Recommendation is to take data points from the source and calculate the arithemtic mean yourself, instead of directly inverting the twap price.
TWAP of non-existent UniV3 pools
The TWAP of a pool with low-liquidity can easily be manipulated. If your protocol is fetching the twap price of a token from a UniV3 pool, first make sure that the pool exists and its twap cannot be easily manipulated. An attacker may create the pool with low liquidity and maintain a bad price ratio for the TWAP length (which is not hard for a new, unused pool), ultimately getting the twap calculation in his hands, thus leading to highly wrong pricing.
Twap is not so safe for each and every pool by default, you always need to do due diligence on the pools the twap is going to be fetched from. Liquidity is TWAP’s best friend. An external protocol should require the pool being used as an oracle to meet certain liquidity thresholds or have existed for a predefined period of time, only then returning the price.
Example : TWAP#7
uint256 unscaledAmountToBeneficiary = OracleLibrary.getQuoteAtTick({
tick: arithmeticMeanTick,
baseAmount: quoteParams_.baseAmount,
baseToken: cqp.cBase,
quoteToken: cqp.cQuote
});Recommendation is to do thorough liquidity analysis and risk assessment of any pools you are fetching twap prices from, and only use whitelisted pools.
Do not use lagging TWAPs
When integrating with protocols like Curve/balancer pools, developers often tend to trust the pool contract for the underlying assets’ price values, instead of calculating it themselves from different sources. The problem is that these pools maybe dependent on user action for oracle update firing (like uniswap) : the difference being that the trading activity in some of these these pools may be scarce because of low popularity.
For a decently accurate twap reporting, you need to make sure that the price gets updated at least once in a reasonable timeframe. Pools from curve/balancer may often have differing configuration from their other pools, so always check the pool contract for their twap implementations.
Do not rely on prices returned from these as they can lag behind even upto a few days due to no trading activity. Read the pool contract live data and give weightage accordingly.
File: BalancerUtils.sol
21: function _getTimeWeightedOraclePrice(
22: address pool,
23: IPriceOracle.Variable variable,
24: uint256 secs
25: ) internal view returns (uint256) {
26: IPriceOracle.OracleAverageQuery[]
27: memory queries = new IPriceOracle.OracleAverageQuery[](1);
28:
29: queries[0].variable = variable;
30: queries[0].secs = secs;
31: queries[0].ago = 0; // now
32:
33: // Gets the balancer time weighted average price denominated in the first token
34: return IPriceOracle(pool).getTimeWeightedAverage(queries)[0];
// @audit This "pool" has very low trading activity in reality
35: }Recommendation is to only use an oracle as a TWAP price source if it is updated frequently to avoid lagging prices.
TWAP as a fallback oracle
A possible design of the double-oracle setup is incorporating a fallback oracle whose reported prices are used if the primary oracle(Chainlink or other) fails due to some reason (feeder outages etc.). TWAP is included as the fallback because it is considered the next safe option after Chainlink etc., but what if there is no way the fallback can ever be used ?
The problem is that Chainlink or other oracles may revert on failure, so the logic of this fallback oracle for fetching prices has to be included in a try-catch block to ensure the execution can go on even if the primary oracle reverts inside the try block.
Example : TWAP#9
File: /paraspace-core/contracts/misc/ParaSpaceOracle.sol #4
114 /// @inheritdoc IPriceOracleGetter
115 function getAssetPrice(address asset)
116 public
117 view
118 override
119 returns (uint256)
120 {
121 if (asset == BASE_CURRENCY) {
122 return BASE_CURRENCY_UNIT;
123 }
124
125 uint256 price = 0;
126 IEACAggregatorProxy source = IEACAggregatorProxy(assetsSources[asset]);
127 if (address(source) != address(0)) {
128 @> price = uint256(source.latestAnswer());
//@audit execution will revert from here
129 }
130 if (price == 0 && address(_fallbackOracle) != address(0)) {
131 @> price = _fallbackOracle.getAssetPrice(asset);
132 }
133
134 require(price != 0, Errors.ORACLE_PRICE_NOT_READY);
135 return price;
136: }Recommendation is to use a try-catch structure whenever you want to include fallback mechanisms, because of possible reverts DoS’ing the execution.
Max deviation of prices in a double-oracle setup
Another popular design of the multi-oracle setup is the concept of keeping check of the current deviations between prices reported from multiple sources. For example, consider a chainlink-balancer twap coupling : if the price from chainlink is 5 or 10 % away from that reported by balancer twap, then it means one of them has failed or probably being manipulated, so the execution will revert and the system shall be inoperational until the deviation comes within the threshold again.
This protects against single-block manipulation (It has one risk though : continuous breaching of the max threshold deviation may completely halt the system). Especially used for pegged assets like WBTC for utmost safety against depeg risk.
The example here checked the deviation only if the price was last updated in the current block, which breaks the system if the price was manipulated (updated) at the end of the previous block and used in the first txn of current block without any txn updating it again (possible in PoS by multi-block MEV). Check that they are within an allowed delta regardless of when price was last updated.
Also, the deviation threshold needs to be dynamically configurable based on asset type and market conditions. Another pattern to use is involve 2+ sources and only use reported values from the 2 results that are most closely aligned with each other without much deviation.
// if difference between spot and twap is too big, check if the price may have been manipulated in this block
uint256 delta = (price > twap) ? price.sub(twap).mul(PRECISION).div(price) : twap.sub(price).mul(PRECISION).div(twap);
if (delta > hysteresis) require(checkHysteresis(), "IV.deposit: try later");Recommendation is to use the threshold deviation concept for weightage of values reported from different oracle sources and use it for all cases despite of when the price was last updated.
Source of price in TWAP calculation
TWAP is the average of spot price over a certain time window. How this spot price is structured can affect the twap calculation a lot. One problem is in how perpetual protocols’ prices are configured to react to new orders or open positions.
Basically you can affect the price reported by a perpetual AMM (differs by design) using arbitrarily placed orders because it requires no funds if the order is not executed. So these pending orders/positions can be used to manipulate the reported price very easily and hence influence the calculated TWAP over a longer window.
Place arbitrary large orders at arbitrary large or small price points => do this repeatedly to influence TWAP => use the manipulated twap => profit
Another similar risk is when underlying spot price is calculated from the price of the last filled orders. This whole category is a special case because there is little to no cost associated with it.
Recommendation is to always check where the twap’s underlying data is sourced from, and if it can be easily manipulated to arbitrary values.
Recency-based weighting for large-duration TWAPs
Some implementations may give undue weightage to the few recent prices or last recorded prices, or even use the one last recorded price if there has been no update in a desired timeslot. This may give away partial or complete control of the final twap to the more recent prices. If so, the concept of TWAP has been broken, now it is RWAP {Recency-Weighted Average Price}.
There can be many ways of attacking RWAP, based on specific logic. Either give equal weighting or ignore the last recorded trades (because they might have been manipulated) otherwise current orders can have undue influence on the TWAP due to the weighting of a period inside the full twap window. Such implementations fundamentally give more importance to the spot values.
uint256 currentPeriodStart = (_blockTimestamp() / spotPriceTwapInterval) * spotPriceTwapInterval;
uint256 lastPeriodStart = currentPeriodStart - spotPriceTwapInterval;
// If there is no trade in the last period, return the last trade price
if (markPriceTwapData.lastTimestamp <= lastPeriodStart) {
return markPriceTwapData.lastPrice;
} //@audit simply returns the last filled price
/**
* check if there is any trade after currentPeriodStart
* since this function will not be called before the nextFundingTime,
* we can use the lastPeriodAccumulator to calculate the twap if there is a trade after currentPeriodStart
*/
if (markPriceTwapData.lastTimestamp >= currentPeriodStart) {
// use the lastPeriodAccumulator to calculate the twap
twap = markPriceTwapData.lastPeriodAccumulator / spotPriceTwapInterval;
} else {
// use the accumulator to calculate the twap
uint256 currentAccumulator = markPriceTwapData.accumulator + (currentPeriodStart - markPriceTwapData.lastTimestamp) * markPriceTwapData.lastPrice;
twap = currentAccumulator / spotPriceTwapInterval;
}Recommendation is to avoid directly using the last price(or giving too much weight to it).
Conclusion
These kinds of issues arise when developers try to build a twap algorithm on their own. Apart from all these issues, here are some general use cases you should remember : Use twaps in LP token pricing ; when calculating on-the-go slippage like minOut parameters {instead of using current values} ; in reserve amounts, interest rates and everywhere you can replace spot values because time-averaged values are much more resitant to manipulation.
Follow and stay tuned for more security content :)
You can see my auditing portfolio here : https://github.com/chinmay-farkya
Reach out to me for Audits : Discord | Twitter | Telegram
If you want to donate me, head to my Wallet address
