Skip to content
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

Fix the inversion order in OracleRef #829

Merged
merged 4 commits into from
Jun 1, 2022
Merged

Conversation

thomas-waite
Copy link
Contributor

@thomas-waite thomas-waite commented May 23, 2022

Summary

The inversion and scalingFactor in OracleRef are applied in the wrong order, which in a situation where doInvert = true and scalingFactory != 0 would lead to an incorrect price. The issue was found by Elliott when he was developing a USDC PSM.

Info below from his fix in the Volt repo:

Creating this PSM exposed an issue with the OracleRef contract, which is that the order of operations for inverting and then scaling was backwards which necessitated this change.

The current flow is:

Read -> scale -> invert

1. Read 1000000000000000000
2. Scale 1000000000000000000 * 1000000000000000000 (scaled by 1e18)
3. Invert 1 / 1000000000000000000000000000000000000
Result: 1e -36

This flow is incorrect, as the scale should have been applied after the inversion, not before, and thus the price which should have been 1 is now 0.000000000000000000000000000000000001

Vs

Read -> invert -> scale

1. Read 1000000000000000000
2. Invert 1 / 1000000000000000000
3. Scale 1 / 1000000000000000000 * 1000000000000000000 (scaled by 1e18)
Result: 1

@thomas-waite thomas-waite added the bug Something isn't working label May 23, 2022
@thomas-waite thomas-waite self-assigned this May 23, 2022
@thomas-waite thomas-waite requested a review from a team as a code owner May 23, 2022 14:55
@thomas-waite thomas-waite changed the title Fix the inversion order in OracleRef [WIP] Fix the inversion order in OracleRef May 23, 2022
ElliotFriedman
ElliotFriedman previously approved these changes May 23, 2022
Copy link
Contributor

@ElliotFriedman ElliotFriedman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@thomas-waite thomas-waite changed the title [WIP] Fix the inversion order in OracleRef Fix the inversion order in OracleRef Jun 1, 2022
@eswak
Copy link
Contributor

eswak commented Jun 1, 2022

LGTM, waiting for e2e tests to be fixed so we can merge with a green tick

@thomas-waite thomas-waite merged commit 85a3e11 into develop Jun 1, 2022
@thomas-waite thomas-waite deleted the fix-oracle-ref branch June 1, 2022 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants