-
Notifications
You must be signed in to change notification settings - Fork 105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RToken Oracle #849
RToken Oracle #849
Conversation
15a0727
to
3ae0eab
Compare
Sample RToken Oracle Result
|
7976e8c
to
fee0cb1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-reviewed all the non-FixLib stuff
will also need lint attention before going in
/// @param a First to multiply | ||
/// @param b Second to multiply | ||
/// @param c Denominator | ||
function safeMulDiv( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other reviewers, this is the function that needs your eyes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can see safeMulDiv
only diviates from mulDiv
in the overflow case if (hi >= c) return FIX_MAX;
- as is the intention.
I don't see any unintended behaviour introduced. LGTM
return; | ||
} | ||
|
||
(uint192 low, uint192 high) = price(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting out loud that when we bring the lotPrice changes over to this repo this should stay price()
and should not change to lotPrice()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uint256 result_256; | ||
unchecked { | ||
(uint256 hi, uint256 lo) = fullMul(a, b); | ||
if (hi >= c) return FIX_MAX; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line and L595 are the only 2 that feel a bit odd to me, but we already follow a similar pattern in safeMul
, so i think this is correct to return FIX_MAX instead of erroring.
Oracle implementation for RTokens based on internal pricing logic.