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.

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

Calculate SpotPrice with Ratios and SigFigs #1176

Merged
merged 4 commits into from
Mar 31, 2022
Merged

Calculate SpotPrice with Ratios and SigFigs #1176

merged 4 commits into from
Mar 31, 2022

Conversation

czarcas7ic
Copy link
Member

Closes: #1155

Description

  • Calculates SpotPrice with ratios to prevent trailing digits from giving incorrect results
  • Utilizes constant in x/gamm/types to determine how many significant figures we calculate SpotPrice to (other functions can also use this SigFigs constant now)
  • Added a test to show a circumstance in which the 9th digit is greater than 5, and with 8 sigfigs set should remove the 9th digit however still round the 8th digit up
  • Added comment to explain why this change was added

@czarcas7ic czarcas7ic requested a review from ValarDragon March 29, 2022 23:11
@czarcas7ic
Copy link
Member Author

Should I modify all the tests that failed to utilize the 8 sigfigs now?

@czarcas7ic
Copy link
Member Author

I basically would just need to change this test here
https://github.com/osmosis-labs/osmosis/blob/main/x/pool-incentives/keeper/keeper_test.go#L96
and here
https://github.com/osmosis-labs/osmosis/blob/main/x/gamm/keeper/keeper_test.go#L95

Just lmk what you would like and I will change accordingly

@ValarDragon
Copy link
Member

Yeah lets fix those other tests with the right value!

@czarcas7ic
Copy link
Member Author

Test modified to include sigfigs, now pass

alexanderbez
alexanderbez reviewed Mar 31, 2022
View reviewed changes
x/gamm/pool-models/balancer/amm.go
Comment on lines +125 to +126
// spot_price = (Base_supply / Weight_base) / (Quote_supply / Weight_quote)
// spot_price = (weight_quote / weight_base) * (base_supply / quote_supply)
Copy link
Contributor

Choose a reason for hiding this comment

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

wait, this says spot_price is two different things. Or are you suggesting there are two ways to get the value?

Copy link
Member

Choose a reason for hiding this comment

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

They are equivalent! Its intended to show that they're the same, if you simplify the equation in that way

Copy link
Member

Choose a reason for hiding this comment

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

The first equation is how its commonly quoted, but we're computing according to the second equation for the speed and improved precision due to the weight common denominator

ValarDragon
ValarDragon approved these changes Mar 31, 2022
View reviewed changes
Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

LGTM

@czarcas7ic czarcas7ic merged commit cc0ec41 into main Mar 31, 2022
@czarcas7ic czarcas7ic deleted the adam/sigfig-patch branch March 31, 2022 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexanderbez alexanderbez alexanderbez left review comments

@ValarDragon ValarDragon ValarDragon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Osmosis Chain Development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SpotPrice: Improve accuracy and reduce precision of quoted result.
3 participants
@czarcas7ic @ValarDragon @alexanderbez