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

Demo: break spot price API to operate on BigDec #6371

Closed
wants to merge 1 commit into from

Conversation

p0mvn
Copy link
Member

@p0mvn p0mvn commented Sep 11, 2023

Closes: #XXX

What is the purpose of the change

This is the demo to decide on the best way of implementing a 36-decimal spot price query.

The suggestion in this PR is to avoid code duplication as much as possible and attempt to break PoolI and PoolModuleI interfaces to operate on 36 decimals in a state-compatible way.

In the querier, we can expose two APIs:

  1. Old V1: returns a string representing decimal with 18 precision
  2. New V2: returns a string representing decimal with 36 precision.

It is unclear to me whether it would be client-breaking if we were to start returning a 36 decimal string post-upgrade or not. As a result, we can end up duplicating the querier only

Note that there are probably state breaks present in this PR currently. However, I think that it would be possible to implement it in a state-compatible way incrementally:

  • First, break PoolI API, backport and test
  • Break PoolModuleI API, backport and test
  • Make the state-breaking change in CL to allow pools with larger spot prices.
  • Solve x/twap issue separately (in the future). Accept that pools with spot spot price below 10^-18 will not have twap functioning on-chain.

Pros

  • No code duplication for essentially the same methods operating on BigDec
  • Lesser maintenance burden for us long-term

Cons

  • Should implement carefully, ensuring that no unexpected state-breaks or client breakages

@github-actions github-actions bot added C:x/gamm Changes, features and bugs related to the gamm module. C:x/superfluid C:x/txfees C:app-wiring Changes to the app folder C:x/twap Changes to the twap module C:x/concentrated-liquidity C:x/poolmanager labels Sep 11, 2023
@github-actions
Copy link
Contributor

Important Notice

This PR includes modifications to the tests/e2e/initialization module.
Please follow the instructions below:

  1. Backport these changes to the previous Osmosis version's branch.
  2. Run the script inside a Docker container to update genesis and configs for pre-upgrade Osmosis.
  3. Merge the backported changes.
  4. The image will be built and uploaded to Docker Hub here.
  5. Grab the latest image and update it in the PR to the main branch replacing the previousVersionInitTag in the osmosis/tests/e2e/containers/config.go

Please let us know if you need any help.

@p0mvn p0mvn changed the title break spot price API to operate on BigDec Demo: break spot price API to operate on BigDec Sep 11, 2023
@p0mvn p0mvn closed this Sep 14, 2023
@github-actions github-actions bot mentioned this pull request Feb 15, 2024
@github-actions github-actions bot mentioned this pull request Apr 1, 2024
@github-actions github-actions bot mentioned this pull request Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:app-wiring Changes to the app folder C:x/concentrated-liquidity C:x/gamm Changes, features and bugs related to the gamm module. C:x/poolmanager C:x/superfluid C:x/twap Changes to the twap module C:x/txfees
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant