sherlock-audit / 2023-10-aloe-judging

9 stars 6 forks source link

roguereddwarf - VolatilityOracle skips implied volatility updates due to time constraints #91

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

roguereddwarf

medium

VolatilityOracle skips implied volatility updates due to time constraints

Summary

The VolatilityOracle.update function is supposed to be called by external parties to update the implied volatility (IV).

The problem is that the time constraints for this function are set up in a way that can lead to much less frequent IV updates, making IV react slower to sudden changes in market volatility.

Vulnerability Detail

VolatilityOracle.update can only update IV once an hour. If the hour hasn't passed, the old IV is returned.

Note that it's not realistic that there is an exact difference of one hour between IV updates because there may not be a transaction at exactly the hour mark.

So in reality this will be something like 60-70 minutes if there is really an interested party around to call this function for which there is no monetary incentive to do so.

In order to update IV, we need to find a timestamp from a past update that is FEE_GROWTH_AVG_WINDOW - FEE_GROWTH_SAMPLE_PERIOD / 2 (5.5 hours) to FEE_GROWTH_AVG_WINDOW + FEE_GROWTH_SAMPLE_PERIOD / 2 (6.5 hours) seconds in the past.

If we can't find that, IV won't be updated. The new LastWrite is created though and the feeGrowthGlobals array is updated. At this point we need to wait another hour to try to make an IV update, even though IV hasn't even been updated (this means we skip an IV update).

If we make a reasonable assumption that it takes 5 minutes for someone to call VolatilityOracle.update after it becomes possible again then there is a 5 / 65 = ~7% chance that we don't find a timestamp that is close enough and we skip an update. For a 10 minute delay, the chance is 10 / 65 = ~15%

Thereby IV can in fact react slower than it is supposed to.

Impact

IV is updated less often than it should due to the 1 hour restriction for making updates to the feeGrowthGlobals array.

Thereby IV reacts slower to sudden changes leading to an increased risk of bad debt or decreased capital efficiency.

Code Snippet

https://github.com/aloelabs/aloe-ii/blob/c71e7b0cfdec830b1f054486dfe9d58ce407c7a4/core/src/VolatilityOracle.sol#L45-L94

Tool used

Manual Review

Recommendation

The solution is to allow updating the feeGrowthGlobals array more frequently than after FEE_GROWTH_SAMPLE_PERIOD seconds.

I recommend FEE_GROWTH_SAMPLE_PERIOD / 2, i.e. 30 minutes.

Still only update the IV once an hour and since the feeGrowthGlobals array is updated more frequently, it will always be possible to make an IV update (even when there is a delay for updating the feeGrowthGlobals array) as opposed to skipping some updates.

sherlock-admin2 commented 1 year ago

3 comment(s) were left on this issue during the judging contest.

panprog commented:

low, because this is the parameters choice by the protocol team. Besides, IV and price probes calculation has a lot of safety margin built-in, so that slow IV update should not be a big problem anyway

tsvetanovv commented:

Design decision

MohammedRizwan commented:

seems intended design