morpho-org / morpho-blue

🟦 Morpho Blue Protocol
https://morpho.org
Other
128 stars 46 forks source link

Oracle could return more things #49

Closed pakim249CAL closed 1 year ago

pakim249CAL commented 1 year ago

The onchain team has discussed extensively the utility of having pause flags for things like borrow and liquidation returned by the oracle. This is especially important for less liquid markets to prevent short term price changes from rekting.

Rubilmax commented 1 year ago

Here's how I did it:

https://github.com/morpho-labs/blue/blob/63e8da9592869a8935a4dadba3b73063db19302f/src/interfaces/IOracle.sol#L4-L6

https://github.com/morpho-labs/blue/blob/63e8da9592869a8935a4dadba3b73063db19302f/src/Morpho.sol#L74-L75 https://github.com/morpho-labs/blue/blob/63e8da9592869a8935a4dadba3b73063db19302f/src/Morpho.sol#L129-L130 https://github.com/morpho-labs/blue/blob/63e8da9592869a8935a4dadba3b73063db19302f/src/Morpho.sol#L168-L169

makcandrov commented 1 year ago

Did we agree on a standard for the oracle? Because this POC is calling the oracle twice, for the price of the collateral and for the price of the borrowable asset. I think it would be better to call the oracle once to get the price of the borrowable asset in the collateral asset unit (or the inverse). Also some discussions were ongoing here about the formatting of the price

MathisGD commented 1 year ago

I think that this is less opti with this opti in mind (and I think that for a lot of markets, it applies).

opti idea for Blue: assume price = 1 when the address of the oracle is zero, to do one less call

This is useful when you lend the quote currency (which turns to happen quite a lot)

Nb: having only one oracle makes this idea more natural but also adds one call (3 calls in the general case, 2 in my example)

Rubilmax commented 1 year ago

opti idea for Blue: assume price = 1 when the address of the oracle is zero, to do one less call

This could be moved to the oracle wrapper in the general case too, right?

MathisGD commented 1 year ago

yes but it adds one cold call in any case

Nb: having only one oracle makes this idea more natural but also adds one call (3 calls in the general case, 2 in my example)

Rubilmax commented 1 year ago

Ah but we lose the possibility for oracles to define pause flags...

MathisGD commented 1 year ago

nb: one natural way for an oracle to define a pause flag is to revert (work only for withdrawCollat and borrow though)

MerlinEgalite commented 1 year ago

@QGarchery @MathisGD wdyt?

QGarchery commented 1 year ago

Having two oracles seems better in terms of gas efficiency, and more natural in most cases, because you can just use the chainlink/uniswap oracles. Also oracles are not called in every function anyway, so I don't think that they are the best suited to pause the main entry points. It seems to me like what we really want to do is to prevent to use oracles when they return shaky prices, and this can easily be done with reverting. In the end, having two oracles that do not return more things sounds enough.

Tell me if you have another reason to return more data in the oracles, I read the oracles milestone page but I may have missed a use case for pause flags

pakim249CAL commented 1 year ago

Using two oracles has the potential for price sync problems though, which is why I consider pause flags a necessity if we want to give flexibility in oracle selections.

Reference: https://governance.aave.com/t/arc-provide-better-oracle-mechanism-for-correlated-assets/4720 https://governance.aave.com/t/bgd-generalised-price-sync-adapters/11416

makcandrov commented 1 year ago

Having two oracles seems better in terms of gas efficiency

How can having two oracles be more gas-efficient than having one?

and more natural in most cases

I don't find it more natural. It leaves the uncertainty of: what is the base currency of the price returned by the oracles? If there is a single oracle, there is no such uncertainty - the returned price is the price of the collateral in the borrowable currency (or the inverse).

because you can just use the chainlink/uniswap oracles

Why wouldn't it be possible with just one oracle?

edit: Perhaps the debate between 1 vs 2 oracles should be considered a separate issue, as it is not directly related to the flags

Rubilmax commented 1 year ago

How can having two oracles be more gas-efficient than having one?

The canonic version when using chainlink oracles would be to have an oracle wrapper (a third contract), which wraps the calls to the 2 underlying chainlink oracles. This is indeed not an ideal usecase, but it seems canonic if one want to use a third-party oracle (because there's no oracle interface standard, there must be an intermediary contract that fetches, transforms and outputs the price, according to Blue's oracle interface). Ultimately, this leads to at least an additional CALL.

Using two oracles has the potential for price sync problems though

These are good resources arguing against an intermediary quote asset, so in favor of a single debt-in-collateral (or vice-versa) oracle.

which is why I consider pause flags a necessity if we want to give flexibility in oracle selections.

However, I don't understand this part. As suggested by @QGarchery, oracles could revert if they consider prices incorrect. Which is equivalent to returning pause flags and handling them at the protocol level? If building an oracle standard for Blue, I'd rather have oracles revert than return flags (a priori).

EDIT: this actually has the side-effect of not being able to query the price from this oracle. It may be fine for Blue-specific oracles.

pakim249CAL commented 1 year ago

Reverting in the oracle is too blunt of a solution IMO. It doesn't make a distinction between whether it should be okay to withdraw collateral, borrow, or liquidate.

makcandrov commented 1 year ago

Ultimately, this leads to at least an additional CALL

I agree that the additional call is bothersome, but I don't think we can avoid it because, as you said, there is currently no standard for oracles. That's why I believe it is not possible to have two oracles in Blue.

However, if we implement a single oracle for the price of any asset in any asset (the wrapper), Blue might establish a new oracle standard that could be widely adopted, even beyond Blue.

Rubilmax commented 1 year ago

I agree that the additional call is bothersome, but I don't think we can avoid it because, as you said, there is currently no standard for oracles.

Agreed. Because there is no standard for oracles, the additional CALL seems necessary (unless we conform to some Chainlink or Uniswap oracle convention). Would 2 oracles with an intermediary quote asset still be more efficient then? @MathisGD @QGarchery

Blue might establish a new oracle standard that could be widely adopted, even beyond Blue.

I am not convinced by this, because the usecase of pause flags only fits to lending (while oracles, as designed by Chainlink, works for anything). I don't think we should design or aim at designing an oracle standard that may prevent us to be specific to our usecase.

But still, I'm convinced a single oracle is better now, if we don't want to stick to a third-party oracle convention. To me, the question still holds whether we need pause flags or not (and in the case we need them: a single oracle is necessary).

MerlinEgalite commented 1 year ago

To me we can always use one oracle because you can always merge 2 oracles in one through a wrapper

QGarchery commented 1 year ago

As @MerlinEgalite suggests, theoretically everything you could do with one oracle, you can do with two: make one of them trivial and wrap your single oracle in the other one. This means that you could solve price sync this way, have pause flags this way, etc. It just gets more complicated to implement some features. That being said, I agree that if we have many such features, it may be worth it to not bother with 2 oracles (but I don't see it for now).

The efficiency loss with having only one oracle cannot be recovered though when you have a single oracle.

Reverting in the oracle is too blunt of a solution IMO. It doesn't make a distinction between whether it should be okay to withdraw collateral, borrow, or liquidate.

I don't see why you would want to do these specific actions when just knowing what the oracle returns. If the prices are not good, you only know that you don't want to use those prices. Pausing functions cannot be done with just the information that the oracle is not returning consistent prices, because the oracle is not called in all the functions, and because it may be that more information is needed. I think we are still far from having relevant information on oracles for that (like price volatility)

I don't find it more natural. It leaves the uncertainty of: what is the base currency of the price returned by the oracles? If there is a single oracle, there is no such uncertainty - the returned price is the price of the collateral in the borrowable currency (or the inverse).

What I find more natural is that you can actually use a true oracle (as we know them today, chainlink for example), for market.collateralOracle and for market.borrowableOracle.

Rubilmax commented 1 year ago

What I find more natural is that you can actually use a true oracle (as we know them today, chainlink for example)

Can you show an example in practice? It seems it requires to conform to Chainlink's oracle interface first (namely: query latestAnswer instead of price), which is what I suggest in my comments above (and I believe, also what @makcandrov highlighted).

Either we stick to existing oracle conventions, or we lose efficiency (additional CALL).

MathisGD commented 1 year ago

Using two oracles has the potential for price sync problems though, which is why I consider pause flags a necessity if we want to give flexibility in oracle selections.

Reference: https://governance.aave.com/t/arc-provide-better-oracle-mechanism-for-correlated-assets/4720 https://governance.aave.com/t/bgd-generalised-price-sync-adapters/11416

@pakim249CAL I don't think so. If you want to price the assets in of the {borrowable,collateral} you can just hardcode the price of it to 1 and price the other in it.

QGarchery commented 1 year ago

Can you show an example in practice? It seems it requires to conform to Chainlink's oracle interface first (namely: query latestAnswer instead of price), which is what I suggest in my comments above (and I believe, also what @makcandrov highlighted).

Yes that would be the most natural, but I agree that it's annoying to have to choose between standards. Or we can use an adapter, which would be less clean but still more efficient: here is an example of that idea

QGarchery commented 1 year ago

I changed my mind about oracles, in the most common use cases it seems too convoluted to try to keep 2 oracles: see https://github.com/morpho-labs/blue/pull/145#issuecomment-1643601530

MerlinEgalite commented 1 year ago

I don't get why, maybe we want to reuse oracles for different markets. I still think it's interesting to have a specific oracle for the collateral and another for the debt. Maybe I did not interpreted well what you said though.

pakim249CAL commented 1 year ago

I don't think it's beneficial to try and accommodate reusing oracles across markets. This would have a side effect of consolidating risk across reused oracles, and I don't think this has any substantial benefits during execution time since the oracles are passed in calldata anyway.

MerlinEgalite commented 1 year ago

The benefit it's that if you create multiple markets with WETH you don't need to rewrite an oracle for each pair WETH-X or X-WETH. I don't speak about gas reduction but more about maintainability and reduction of operational cost.

pakim249CAL commented 1 year ago

I'm thinking that if we are bothering to create new markets in the first place, it would actually be safer and easier to deploy new oracles from scratch anyways. This should not increase operation cost by much as long as we have good templates for oracles ready and we make the oracles immutable. Maybe we can build oracle factories that are adapted to specific oracles like chainlink or uniswap?

MerlinEgalite commented 1 year ago

it would actually be safer and easier to deploy new oracles from scratch anyways

I don't really get the reasoning here.

I was thinking about something similar for the oracles though and I agree it would be helpful

pakim249CAL commented 1 year ago

My reasoning only applies if we choose to move to 1 oracle instead of 2. https://github.com/morpho-labs/blue/issues/49#issuecomment-1643713482

I don't think it makes sense in this case to try and reuse oracles. Instead, each market should probably have its own oracle, and it can optionally be immutable to make them zero maintenance.

MerlinEgalite commented 1 year ago

Imagine we take chainlink as first oracle for the most common assets (with a wrapper so that the output is well formatted), what wouldn’t we reuse it for different markets?

Side note: I can organize a call with them once it's the right moment to have some tips about how to integrate oracles the best way.

makcandrov commented 1 year ago

I think that keeping 2 oracles and allowing people to reuse some of them is more favorable for potential mistakes.

For example, if someone wants to reuse the oracle of asset A and B, but they forget to check the base currency, one may be in ETH and the other in USD, or one in USDC and the other in USDT. Also, having a single oracle shifts the responsibility of price desync handling to the oracle instead of Blue.

MerlinEgalite commented 1 year ago

For example, if someone wants to reuse the oracle of asset A and B, but they forget to check the base currency, one may be in ETH and the other in USD, or one in USDC and the other in USDT.

I'd say it's on the market creator responsibility to check this + we can provide guidelines on how to build an oracle for blue. But anyway I think I'm the only one thinking that 2 different oracles is better so I surrender lol

Rubilmax commented 1 year ago

But anyway I think I'm the only one thinking that 2 different oracles is better so I surrender lol

I think your point on reusability of oracle wrappers across markets is valuable. But I still think that reusability (=> lesser market creation gas cost - need to be quantified) is less valuable than the benefits of a single oracle (=> can be responsible for market flags, => built-in price sync)...

Rubilmax commented 1 year ago

Seeing https://github.com/Uniswap/v3-periphery/blob/0.8/contracts/libraries/OracleLibrary.sol, I considered at some point having an oracle taking into input a borrowedAmount and returning the amount of collateral that can be exchanged for borrowedAmount of debt (or vice-versa). I thought it was a great idea to have more precise prices, but it actually turns out that such a market would be bound to a unique underlying exchange market, which may not be desirable. For borrowers, it'd be similar to be liquidatable with reference to a single market price. Instead, borrowers would prefer to be liquidated against a mark price.

MerlinEgalite commented 1 year ago

I'm not sure why it brings a better precision. Can you elaborate on that aspect?

Rubilmax commented 1 year ago

Having the oracle quote amounts instead of giving a price would enable the oracle to account for price impact, which in turns lead to more real prices (ie there's not a single notion of price that fits any size of trade).

MerlinEgalite commented 1 year ago

Having the oracle quote amounts instead of giving a price would enable the oracle to account for price impact, which in turns lead to more real prices (ie there's not a single notion of price that fits any size of trade).

Oh ok I get it but is it the price you want to use in a lending protocol? Actually this is a good question

pakim249CAL commented 1 year ago

Having the oracle quote amounts instead of giving a price would enable the oracle to account for price impact, which in turns lead to more real prices (ie there's not a single notion of price that fits any size of trade).

That's a very interesting observation. I think this feature could be leveraged to create lending markets with an oracle that makes the lending market safer by ensuring that a certain amount of assets can be liquidated at a certain volume at the oracle price.

makcandrov commented 1 year ago

Having the oracle quote amounts instead of giving a price would enable the oracle to account for price impact

Actually, it would just incentivize users to split their position into multiple smaller ones

pakim249CAL commented 1 year ago

Having the oracle quote amounts instead of giving a price would enable the oracle to account for price impact

Actually, it would just incentivize users to split their position into multiple smaller ones

Ah yes you're right. A good middle ground would be for the oracles themselves to account for a certain price impact in their price rather than trying to use amounts.

MerlinEgalite commented 1 year ago

Do you have an idea on how you would do that?

pakim249CAL commented 1 year ago

My idea is only applicable to using something like a uniswap price oracle. It would go something like this:

We can call the TWAP price during X period "P_0". We can call the drop in price "D" of the collateral when Y amount of collateral is swapped in uniswap for debt.

Then the oracle can use the price P_1 = P_0 * (1 - D).

This would have the following consequences:

I wouldn't take this idea too seriously yet though, since I think there are some manipulation vectors here by adding or removing liquidity.

MerlinEgalite commented 1 year ago

Ok clear, interesting idea. We could talk about about some oracle variants with uniswap later on

Rubilmax commented 1 year ago

I believe @pakim249CAL's comment adds a lot of value and context to the discussion here

MathisGD commented 1 year ago

Intersting discussion, but I'm closing because it does not fit Blue's current vision