Skip to content

Check exponent range #179

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 29, 2022
Merged

Check exponent range #179

merged 1 commit into from
Jun 29, 2022

Conversation

jayantk
Copy link
Contributor

@jayantk jayantk commented Jun 28, 2022 •
edited
Loading

Bound the range on price account exponents 8 >= expo >= -8 such the the EMA computation can't lose too much precision.

I also verified that all existing price accounts have exponents between -3 and -8 (inclusive).

ptaffet
ptaffet approved these changes Jun 28, 2022
View reviewed changes
Copy link

@ptaffet ptaffet left a comment

Choose a reason for hiding this comment

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

Looks good. I didn't check all the existing prices, but this does prevent any new prices with strange exponents. The value is only used when creating a price, so I don't see any potential unintended consequences of this change.

@jayantk jayantk merged commit 467641d into main Jun 29, 2022
@jayantk jayantk deleted the exponent_range_check branch June 29, 2022 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@ptaffet ptaffet ptaffet approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jayantk @ptaffet