Closed tserg closed 7 months ago
Pinging @milancermak @bllu404 @0xBRM for initial comments.
I don't think we should use the TWAP even though it is available because then we are additionally treating the TWAP as a fallback to spot
So you're saying this approach in this draft PR is not the way to go?
I don't think we should use the TWAP even though it is available because then we are additionally treating the TWAP as a fallback to spot
So you're saying this approach in this draft PR is not the way to go?
Oops there is a slight typo, I meant that the entire price update should be rejected if spot price is invalid but TWAP is valid.
[EDIT - and it looks we cannot really validate the TWAP because it simply returns price and decimals]
Fair point on validating TWAP. Let's ask the Pragma guys on how they do it (I'm too lazy to read their code 😁).
You also make a good point about the spot validation (if spot is invalid, the function returns an error). Maybe with TWAP we should return that if spot is not valid? Or have a different interface like fetch_spot
and fetch_twap
and let the Seer determine what to do next? Soon we will also be able to integrate Switchboard, so we'll have more data points and the logic could be there.
Other than that, impl looks good. I agree with the approach of using min
. Given that even ETH and BTC have been trading like 💩 🪙 recently this is a good approach. With the risk of starting a bike shed discussion, why should we take the 7d TWAP? Why not 3 or 5 or 14? I don't have an answer, just interested in your thoughts overall.
You also make a good point about the spot validation (if spot is invalid, the function returns an error). Maybe with TWAP we should return that if spot is not valid? Or have a different interface like fetch_spot and fetch_twap and let the Seer determine what to do next? Soon we will also be able to integrate Switchboard, so we'll have more data points and the logic could be there.
I think it would be better for the pessimistic implementation to sit outside of the Seer, and to keep the Seer's logic simple - it either gets a single valid price from an oracle, or it fails to do so and moves to the next oracle. The main reason is that letting the Seer decide could lead to exponential complexity - what if an oracle has spot price only but not TWAP, would spot and TWAP have different fallback priorities etc.
My current thinking is that if the spot price is not valid for Pragma, then the TWAP is also likely to be affected by the lack of a valid spot price update, so then Seer should fall back to the next oracle.
I think another benefit of having the pessimistic implementation outside of Seer is that it lets us prioritize different goals based on each oracle's priority. For example, the pessimistic implementation of Pragma's oracle takes top priority due to price manipulation being the primary concern, but if it does not provide a valid price update, and assuming all other fallback oracles fail, then we may want to use the spot price from an AMM (which probably does not support TWAP) as the very last option where the primary concern shifts from price manipulation to liveness.
Other than that, impl looks good. I agree with the approach of using min. Given that even ETH and BTC have been trading like 💩 🪙 recently this is a good approach. With the risk of starting a bike shed discussion, why should we take the 7d TWAP? Why not 3 or 5 or 14? I don't have an answer, just interested in your thoughts overall.
I think it largely depends on the balance we want to strike between user experience (more responsive) and protocol safety (less responsive).
This article does suggest at the end that the window should be longer the more volatile an asset is. I think something cool would be to have the time window correspond to the threshold for a yang e.g. >=80%: 2 day TWAP, 60% - 80%, 5 day TWAP, <60%, 7 day TWAP. This could be hardcoded, and the Pragma adapter contract simply reads a yang's threshold from Shrine to decide what values to read from Pragma
This PR changes the Pragma adapter contract into a pessimistic oracle that takes the lower of the spot price and the TWAP price over 7 days, as per the first design in this Slack thread.
Since the call to fetch the TWAP price returns the price and decimals directly, data validation is performed on the spot price only. If the spot price is valid, then
pragma.fetch_price()
returns the lower of the spot price and TWAP. If the spot price is invalid, thenpragma.fetch_price()
throws - I don't think we should use the TWAP even though the spot price is available because then we are additionally treating the TWAP as a fallback to spot price (or is that ok?).