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

refactor: breaking CL tick to sqrt price math min spot price relaxation #6319

Merged
merged 6 commits into from
Sep 8, 2023

Conversation

p0mvn
Copy link
Member

@p0mvn p0mvn commented Sep 5, 2023

Closes: #6046

What is the purpose of the change

This PR is a follow-up to: #6317

It relaxes the tick-to-sqrt-price conversion to support the extended minimum range.

Testing and Verifying

The extended price range has been thoroughly tested in previous PRs related to #5726 while guarding state compatibility with the simple check that is removed here.

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes?
  • Changelog entry added to Unreleased section of CHANGELOG.md?

Where is the change documented?

  • Specification (x/{module}/README.md)
  • Osmosis documentation site
  • Code comments?
  • N/A

@p0mvn
Copy link
Member Author

p0mvn commented Sep 5, 2023

Please only merge after #6317 is merged

@p0mvn p0mvn force-pushed the roman/tick-to-sqrt-breaking branch from ed299b2 to 7db80ff Compare September 5, 2023 20:11
@p0mvn p0mvn marked this pull request as ready for review September 5, 2023 20:38
Copy link
Contributor

@AlpinYukseloglu AlpinYukseloglu left a comment

Choose a reason for hiding this comment

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

LGTM once #6317 is merged

Base automatically changed from roman/state-compat-tick-to-price to main September 8, 2023 15:43
@p0mvn p0mvn force-pushed the roman/tick-to-sqrt-breaking branch from 8e7774c to cc7ab11 Compare September 8, 2023 16:39
@p0mvn p0mvn requested a review from mattverse as a code owner September 8, 2023 16:39
@p0mvn p0mvn force-pushed the roman/tick-to-sqrt-breaking branch from cc7ab11 to 7db80ff Compare September 8, 2023 16:40
@p0mvn
Copy link
Member Author

p0mvn commented Sep 8, 2023

I added a test skip here post-approval: 74f96ef

It was in one of the future PRs anyway and I'm tracking removing that check.

Noticed that CI is being flaky from the last merge, and this should fix it.

@p0mvn p0mvn merged commit 6b3273f into main Sep 8, 2023
@p0mvn p0mvn deleted the roman/tick-to-sqrt-breaking branch September 8, 2023 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: tick conversion infrastructure and precompute logic to support [10^-30, 10^38] price range
2 participants