Skip to content

Fix the inversion order in OracleRef#829

Merged
thomas-waite merged 4 commits intodevelopfrom
fix-oracle-ref
Jun 1, 2022
Merged

Fix the inversion order in OracleRef#829
thomas-waite merged 4 commits intodevelopfrom
fix-oracle-ref

Conversation

@thomas-waite
Copy link
Contributor

@thomas-waite thomas-waite commented May 23, 2022 •
edited
Loading

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
View reviewed changes
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

Reviewers

2 more reviewers

@eswak eswak eswak approved these changes

@ElliotFriedman ElliotFriedman ElliotFriedman left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@thomas-waite thomas-waite

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@thomas-waite @eswak @ElliotFriedman

Comments