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

Allow negative range accumulators #5854

Closed
Tracked by #5191
p0mvn opened this issue Jul 17, 2023 · 1 comment
Closed
Tracked by #5191

Allow negative range accumulators #5854

p0mvn opened this issue Jul 17, 2023 · 1 comment
Assignees
@p0mvn @czarcas7ic
Labels
F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board T:bug 🐛 Something isn't working

Comments

@p0mvn
Copy link
Member

p0mvn commented Jul 17, 2023 •
edited by osmosis-clickup
Loading

Background

We should allow negative range accumulators.

Currently, this is disallowed by the validation in DecCoins, leaving to a panic.

Specific way to reproduce this is by initializing the lower tick when the upper tick is already initialized and current tick > upper tick.

In that case, the lower tick's accumulator value is smaller than upper tick's accumulator value.

Suggested Design

  • Reproduce the problem in a test case
  • Find a way to resolve this without migration
    • Idea to validate: new NegDecCoins struct that serializes similarly to DecCoins into bytes but does not implement the negative coin validation.

Acceptance Criteria

  • reproduced with a test
  • solution found
  • no migration requires
@p0mvn p0mvn added T:bug 🐛 Something isn't working F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board labels Jul 17, 2023
@p0mvn p0mvn self-assigned this Jul 17, 2023
@github-project-automation github-project-automation bot moved this to Needs Triage 🔍 in Osmosis Chain Development Jul 17, 2023
This was referenced Jul 17, 2023
Merged
Closed
Merged
Merged
Merged
@czarcas7ic czarcas7ic self-assigned this Jul 24, 2023
@czarcas7ic czarcas7ic moved this from Needs Triage 🔍 to Needs PR Review in Osmosis Chain Development Jul 24, 2023
@p0mvn
Copy link
Member Author

p0mvn commented Aug 3, 2023

It looks like all relevant work is merged now

@p0mvn p0mvn closed this as completed Aug 3, 2023
@github-project-automation github-project-automation bot moved this from Needs PR Review to Done ✅ in Osmosis Chain Development Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@p0mvn p0mvn

@czarcas7ic czarcas7ic

Labels
F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board T:bug 🐛 Something isn't working
Projects
Osmosis Chain Development
Archived in project
Milestone
No milestone
Development

No branches or pull requests
2 participants
@p0mvn @czarcas7ic