Computing probe prices are not suitable for stable pools

Summary

As docs states any uniswap pool can be used in Aloe. Stable pools which are high in tvl and also very high volatility can also be used for example USDC-USDT, DAI-USDC pools. However, the way how probe prices (price.a price.b) is computed is not suitable for these pools because the implied volatility is misunderstood in those pools. The price.an and price.b will be not realistic values for these pools and the borrowing operations are most likely to not make sense because liquidations can happen in very unrealistic priceA priceB values.

Vulnerability Detail

Say we have lenders and borrowers in USDC-USDT pool in Optimism. Which almost 95% of the liquidity is pretty densely concentrated in the area of 0.9-1.1. Now, let's calculate what would be the price.a and price.b for this pool given a random time we will test them with one with using oracle prepare and update and one without the oracle prepare and update here the results:

We will run those two tests:

Default, means that we prepared the oracle and updated. Volatility oracle is ON

function testDefault() public {
        address dummy = address(31);

        uint256 count = pools.length - 1;

        IUniswapV3Pool pool = IUniswapV3Pool(pools[count]);

        vm.startPrank(dummy);
        oracle.prepare(pool);
        oracle.update(pool, (1 << 32));
        vm.stopPrank();

        (uint56 metric, uint160 price, uint256 iv) = oracle.consult(pool, (1 << 32));

        console.log("Metric", metric);
        console.log("Price", price);
        console.log("iv", iv);

        uint8 nSigma = 50;
        uint8 manipulationThresholdDivisor = 12;

        (uint160 a, uint160 b, bool seemsLegit) = BalanceSheet.computeProbePrices(
            uint56(metric),
            uint256(price),
            uint256(iv),
            nSigma,
            manipulationThresholdDivisor
        );

        console.log("Price A", a);
        console.log("Price B", b);
        console.log("seemsLegit?", seemsLegit);

        // assertGt(metric, 0);
        // assertGt(price, 0);
        // assertEqDecimal(iv, 0, 12);
    }

Volatility oracle is not used, Volatility oracle is OFF:

function testWithoutOraclePrepareAndUpdate() public {
        address dummy = address(31);

        uint256 count = pools.length - 1;

        IUniswapV3Pool pool = IUniswapV3Pool(pools[count]);

        (uint56 metric, uint160 price, uint256 iv) = oracle.consult(pool, (1 << 32));

        console.log("Metric", metric);
        console.log("Price", price);
        console.log("iv", iv);

        uint8 nSigma = 50;
        uint8 manipulationThresholdDivisor = 12;

        (uint160 a, uint160 b, bool seemsLegit) = BalanceSheet.computeProbePrices(
            uint56(metric),
            uint256(price),
            uint256(iv),
            nSigma,
            manipulationThresholdDivisor
        );

        console.log("Price A", a);
        console.log("Price B", b);
        console.log("seemsLegit?", seemsLegit);

        // assertGt(metric, 0);
        // assertGt(price, 0);
        // assertEqDecimal(iv, 0, 12);
    }

Now the console results

for the first snippet, with oracle prepare and update: Metric 0 Price 79220240490215316061937756561 iv 127921282726 Price A 57537023128721537114198409333 Price B 109074925362183896224165233716 seemsLegit? true

for the second snippet, without oracle prepare and update: Metric 0 Price 79220240490215316061937756561 iv 0 Price A 77194016963225748098002027713 Price B 81299649250242852390862043329 seemsLegit? true

Now let's convert these prices to human readable form. Since both tokens are in same decimals this formula we will be using: "(sqrtRatio / 2^^96)^^2"

Now for the first snippet let's calculate the human readable prices: Twap price = 79220240490215316061937756561 Twap price human readable = 0.99980003 Ok, twap is giving us a good value which is suitable for the pools, now let's see the price a and price b

Price A = 57537023128721537114198409333 Price A human readable = 0.5273945158

Price B = 109074925362183896224165233716 Price B human readable = 1.895355507

As we can see these values are extremely off and probably these values never reached in the actual pool! Before we talk more on this let's do the second snippet (no prepare and no volatility oracle)

Second snippet: Twap price = 79220240490215316061937756561 Twap price human readable = 0.99980003 Same twap, since it is the same calculation and it is accurate with the pool.

Price A = 77194016963225748098002027713 Price A human readable = 0.9493101285

Price B = 81299649250242852390862043329 Price B human readable = 1.052975282

Well as we can see without the IV it is somehow better. However, it is still not fair because 0.94 and 1.05 pricing requires a serious depeg anyways. Also, initiating the volatility oracle is permissionless for pools which means the first snippet results will be applicable as soon as someone calls the prepare and update with the stable pool.

In general, the more the IV the more the spread between prices A,B respect to price C. Though it can be somehow ok with volatile pools it should not be considered as OK for stable pools because the 99% of the liquidity is concentrated in a very dense tick area.

I also calculated it via altering nSigma and the results are not very different which I will not put here but feel free to play around.

Impact

Stable pools are not suitable for the system because volatility is misinterpreted for them. Considering stable pools has lots of liquidity there can be a good opportunity to make lenders and borrowers to them however, with the given probe prices calculations this is not feasible because the pricesA and pricesB used for liquidations and these prices are not realistic to calculate a potential liquidation event.

Code Snippet

https://github.com/sherlock-audit/2023-10-aloe/blob/main/aloe-ii/core/src/libraries/BalanceSheet.sol#L94-L114

https://github.com/sherlock-audit/2023-10-aloe/blob/main/aloe-ii/core/src/libraries/BalanceSheet.sol#L48-L80

https://github.com/sherlock-audit/2023-10-aloe/blob/main/aloe-ii/core/src/VolatilityOracle.sol#L36-L94

Tool used

Manual Review

Recommendation

Interpret volatility in a different way for stable pools so borrowing operations make sense for those pools

sherlock-admin2 commented 1 year ago

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

panprog commented:

low, seems more like a useful feature (to have better IV estimation for stablecoin pools), not the core function

MohammedRizwan commented:

valid

haydenshively commented 1 year ago

We’ve done substantial backtesting to make sure Aloe II works for a variety of pools. The reason this whitehat is getting bad results is that the VolatilityOracle hasn’t been put through the requisite warm-up period. For any newly-prepared pool, the effective LTV starts at 50% and then gradually moves up or down depending on market conditions. The rate at which it moves is determined by IV_CHANGE_PER_UPDATE. After reaching equilibrium, the LTV of a stable-stable pool would be close to 90%, which is in line with other lending protocols.

This issue is invalid as it comes from a misunderstanding. It is not a duplicate of #38, which makes valid arguments about the EMH.

sherlock-audit / 2023-10-aloe-judging

mstpr-brainbot - Computing probe prices are not suitable for stable pools #129