ebtc-protocol / ebtc

GNU General Public License v3.0
48 stars 24 forks source link

Last Good Price Optimization #757

Closed dapp-whisperer closed 5 months ago

dapp-whisperer commented 6 months ago

Storing the lastGoodPrice in the priceFeed contract can be seen as redundant or undesirable in the setup with it as a primaryOracle of the ebtcFeed.

The priceFeed.lastGoodPrice() can also become out of date versus the EbtcFeed.lastGoodPrice().

So it could return success, with an outdated value vs the secondaryOracle on ebtcFeed.

rayeaster commented 6 months ago

lastGoodPrice is a key state variable used in original PriceFeed design, especially when both its primary (like ChainLink) and secondary feed (like Tellor) are not working as expected.

If we skip storing an valid price to lastGoodPrice variable, existing logic in PriceFeed will be broken.

https://docs.google.com/spreadsheets/d/18fdtTUoqgmsK3Mb6LBO-6na0oK-Y9LWBqnPCJRp5Hsg/edit#gid=316701608 image

On the other hand, EbtcFeed could check both lastGoodPrice and status from PriceFeed to decide what to do with it. For example, if the status is bothOraclesUntrusted then EbtcFeed could choose to failover to its own secondaryOracle regardless of the lastGoodPrice returned by PriceFeed.

I think this might be the minimal disruptive way to move forward

dapp-whisperer commented 6 months ago

Agree here - we're invalidating the priceFeed as a standalone component.

if we consider the priceFeeds as one unit, it looks similar to above however. We "return the last good price of the parent EbtcFeed"

my inclination is to adapt all tests to the new setup and consider the priceFeeds as one unit