search

chronicleprotocol / aggor

Oracle aggregator distributing trust among different oracle providers
Other
4 stars 0 forks source link

Updates `setXXX` functions and increases test coverage #7

Closed pmerkleplant closed 1 year ago

pmerkleplant commented 1 year ago

@deep-quality-dev Ping, as only one reviewer can be requested via Github directly.

deep-quality-dev commented 1 year ago

Sorry for my late comment, @pmerkleplant , I should leave comment yesterday, I was late to find some examples of DeFi apps using TWAP length. But all LGTM, I was wondering about 5 min of secondsAgo in the last PR, recommended window length was 5min - 24hours. i.e. angle.money is using 10min, integral.link is using 30min, setUniswapSecondsAgo is good to me.

nit: snapshot for testIntegration_Mainnet is quite different on my local.

MainnetIntegrationTest:testIntegration_Mainnet() (gas: 204926)
jar-o commented 1 year ago

length was 5min - 24hours. i.e. angle.money is using 10min, integral.link is using 30min...

yeah there doesn't appear to be any real consensus (that I have found) around what should the appropriate timeframe be. my sense is that shorter is more accurate, but longer is more safe. obviously my feeling is that 5m is a reasonable default given that it can be changed to be more conservative as the situation warrants.