Adding a parameter to limit variation to the median price #1490
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1490 +/- ##
========================================
Coverage 74.55% 74.55%
========================================
Files 278 278
Lines 7777 7777
Branches 707 991 +284
========================================
Hits 5798 5798
Misses 1868 1868
Partials 111 111
Continue to review full report at Codecov.
|
…-median-rate-change-529
…-monorepo into limit-median-rate-change-529
| recomputeRate(token); | ||
| } | ||
|
|
||
| function fracMulExp(FixidityLib.Fraction memory a, FixidityLib.Fraction memory b, uint256 exp) |
There was a problem hiding this comment.
Perhaps include in the fixidity lib?
There was a problem hiding this comment.
fixidity lib cannot inherit UsingPrecompiles...
| uint256 maxChange = fracMulExp( | ||
| FixidityLib.fixed1(), | ||
| maxMedianChangeRatePerSecond.add(FixidityLib.fixed1()), | ||
| td |
There was a problem hiding this comment.
This doesn't feel safe to do as the exponents could grow very large...
There was a problem hiding this comment.
So need to add something to make sure it won't overflow?
There was a problem hiding this comment.
Added some checks, now it shouldn't be able to overflow.
mrsmkl
changed the title
…-monorepo into limit-median-rate-change-529
There was a problem hiding this comment.
It seems to me, that the intent of the PR is to limit the variation of the median price, but in particular for short time periods.
For "long" time periods, it probably doesn't matter, nor we can anticipate those values. So, why limit it?
And if you check this for short time periods, we eliminate the whole overflow/checks issue
What do you think? @asaj @mrsmkl @rcroessmann
|
I think that's a good idea, changed the behavior to that. |
|
Still thinking about this... we are defining an artificial limit to the exchange rate variation; but since that limit doesn't exist outside the system (any centralized exchange); we create the opportunity to game the system and profit from arbitrage in a non productive way for us. In particular, taking the Max possible rate
Here we can see that if we use this max
|
|
If we change the max period from year to for example one day, then we get a bit different results. Then, if max is 0.0005, then the max change in hour would be 600% |
|
Yeah, the more i see the Basically, when asking for the Expiring only happens when somebody calls the Then i wonder what's the "mechanics" for it... If we have a long/medium expiration time, for example 1 hour.. we'll be obtaining a rate considering information from last minute and last 59 minute, without making any distinction. Both will be equally interesting to us; when arguably something that was informed an hour ago should not be as relevant as information from a minute ago. That means that it should be short... for example 2 minutes; and we need to call removeExpired constantly. Then there's the other problem... we have "hundreds" of oracles reporting in a 2 minute window... which thinking in terms of block tx space not sure if it's ideal. Maybe not hundreds then ;) But "tens of oracles" reporting... and you don't want the exchange rate to be change everytime a new report arrives... but changes happen due to a median computation, which it we assume a majority honest and up-to-date reports, it should be quite stable... so, i'm a bit confused about what are we looking here. But it is important, since this controls the usd/gold exchange rate which is used in several parts, like rewards for validators... |
|
closing after checking with sami |
Description
Added a parameter to limit variation to the reported prices. There is now a new variable where the current "median" is stored, and it is changed each time a new report is made.
Alternative approaches I can think of:
Tested
Added tests.
Other changes
Related issues
Backwards compatibility