Steakhouse Oracles

Description

In both initialize() and getDeviation(), currentBackupPrice is used as the denominator as we can see in the following code snippets:

// initialize()initialDeviation = (diff * 10**18) / initialBackupPrice;

// getDeviation()return (diff * 10**18) / currentBackupPrice;

The implicit assumption behind this code is that the expected deviation is always a drop in the price of backup oracle. While it might be the case in most deployments, it is still possible that in certain scenarios the price of the backup oracle will be greater than the price of the primary oracle. In this case, the deviation might not cross the predefined threshold although it should. To better understand, let's consider the following scenario:

1. The primary oracle tracks the price of BTC/USD, assuming it is $100,000.
2. The backup oracle tracks the price of cbBTC/USDC, assuming it is 100,000 USDC.
3. The deviation threshold is 5%.

If at some point USDC depegs while cbBTC does not, we expect cbBTC/USDC to increase, assuming it is 105,000 USDC at the moment, this will not be considered deviated since getDeviation() will return (105,000 - 100,000) / 105,000 ~ 4.76% < 5%.

Recommendation

Consider changing the code to divide by the minimum value of the two price instead.

Cantina

The issue was fixed by taking the average between the two prices (primary and backup) instead.

Description

The following logic in initialize() is duplicated from getDeviation():

uint256 initialPrimaryPrice = _primaryOracle.price();uint256 initialBackupPrice = _backupOracle.price();uint256 initialDeviation;if (initialBackupPrice == 0) {    initialDeviation = initialPrimaryPrice == 0 ? 0 : type(uint256).max;} else {    uint256 diff;    if (initialPrimaryPrice >= initialBackupPrice) {        diff = initialPrimaryPrice - initialBackupPrice;    } else {        diff = initialBackupPrice - initialPrimaryPrice;    }    initialDeviation = (diff * 10**18) / initialBackupPrice;}

Code duplication is considered bad practice as it increases maintenance effort, reduces readability, and expands the audit surface. It also risks inconsistencies when updating shared logic.

Recommendation

Refactor the shared logic into an internal view function (e.g., _getDeviation()) and call it from both initialize() and getDeviation() to improve maintainability while preserving gas efficiency.

Description

As communicated with the team, the intended use of this system is to use the primary oracle to track the price of an asset against USD (BTC/USD for example), and the backup oracle to track the price of a wrapped version of that asset with a wrapped version of USD (cbBTC/USDC for example).

In the unlikely scenario of a depegging of both cbBTC and USDC, the backup oracle price might not deviate from the primary oracle price while using the current getDeviation() function. Although this scenario is unlikely, in case both assets tracked by the backup oracle are backed by the same centralized entity it might be more likely than we think.

Recommendation

Consider adding a third oracle that will track the price of USDC/USD.

Cantina

Issue was acknowledged without a fix since the described scenario does not violate the expected behavior of the system as in case of double depegging the ratio will be maintained.