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

[CL]: Remove notion of canonical ticks by increasing min tick/spot price #5550

Closed
AlpinYukseloglu opened this issue Jun 16, 2023 · 0 comments · Fixed by #5551
Closed

[CL]: Remove notion of canonical ticks by increasing min tick/spot price #5550

AlpinYukseloglu opened this issue Jun 16, 2023 · 0 comments · Fixed by #5551
Labels
F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board

Comments

@AlpinYukseloglu
Copy link
Contributor

Background

Canonical tick rounding is intended to solve the issue of many ticks mapping to the same sqrt price due to precision truncation during conversion. This leads to very unintuitive behavior that leads to ticks with much wider spacing than the pool's tick spacing.

Since the issue only arises near MinTick, one suggestion that came up was to simply increase the tick/price floor to avoid the class of issues altogether. While this still leaves some issues to be resolved in the future (namely that some bridged assets might face problems in CL pools depending on the precision they are bridged with), it seems an appropriate first step to minimize the surface area of inconsistent tick behavior.

Suggested Design

  • Increase min tick to -108000000 and min spot price to 10^-12 as shown in Find lower tick bound for canonical ticks #5525 (note that this was found via brute force & was confirmed to be consistent all the way to tick 0, but still lacks a mathematical proof)

Acceptance Criteria

  • All existing tests pass
  • Any tests around min/max ticks are checked to ensure nothing was broken
@AlpinYukseloglu AlpinYukseloglu added the F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board label Jun 16, 2023
@AlpinYukseloglu AlpinYukseloglu mentioned this issue Jun 16, 2023
Merged
6 tasks
@p0mvn p0mvn mentioned this issue Jun 16, 2023
Merged
6 tasks
mergify bot pushed a commit that referenced this issue Jun 19, 2023
@p0mvn
docs: precision issues around price; short and long term solution (#5552
5008d7b 
)

Closes: #XXX

## What is the purpose of the change

Related to: #5550

Documenting latest decisions around tick and price conversions

## Testing and Verifying

This change is a trivial rework / code cleanup without any test coverage.

## 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
mattverse pushed a commit that referenced this issue Jun 20, 2023
@p0mvn @mattverse
docs: precision issues around price; short and long term solution (#5552
2474c54 
)

Closes: #XXX

## What is the purpose of the change

Related to: #5550

Documenting latest decisions around tick and price conversions

## Testing and Verifying

This change is a trivial rework / code cleanup without any test coverage.

## 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
ValarDragon added a commit that referenced this issue Jun 20, 2023
@mattverse @ValarDragon
@AlpinYukseloglu @p0mvn @czarcas7ic
[CL] Changes from audit results(1/2) (#5568)
0cea7aa 
* Some optimizations

* Update x/concentrated-liquidity/math/math.go

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>

* [CL]: Fix incorrect bound check/chain halt vector (#5557)

* repro panic trigger and fix bound check

* fix comments

* docs: precision issues around price; short and long term solution (#5552)

Closes: #XXX

## What is the purpose of the change

Related to: #5550

Documenting latest decisions around tick and price conversions

## Testing and Verifying

This change is a trivial rework / code cleanup without any test coverage.

## 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

* Make go tests only run if relevant (#5569)

* CL: migration functional test (#5560)

Closes: #XXX

## What is the purpose of the change

The following PR introduces a functional test that:
- Creates every type of balancer position that can be created
  - Bonded superfluid
  - Unbonded superfluid (locked)
  - Unbonded superfluid (unlocking)
  - Vanilla lock (locked)
  - Vanilla lock (unlocking)
  - No lock
- It then migrates each position, asserting invariants after each position is migrated
- Finally, an overall invariant is run after all positions have been migrated, asserting that all funds are accounted for in some way

The next PR subsequent to this will be adding randomization to the inputs in order to make the test non deterministic. 

## Testing and Verifying

Functional test above added

## 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

* [CL]: Fix tick rounding bug and implement direct conversion from tick <> sqrt price (#5541)

Closes: #5516

> **Note to reviewer:** the only real state machine change is in `tick.go` and heavily mirrors @ValarDragon's PR here: #5522 
> The rest of the changes are related to function renames/test refactors.

## What is the purpose of the change

This PR expands on #5522 and updates all price to tick conversions to use SqrtPriceToTick instead.

## Testing and Verifying

The new function is tested in `math/tick_test.go`

The original attack vector is also converted into an edge case test in `position_test.go`

## 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

* Still fixing merge conlfict

* Revert lp.go change as commented

---------

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: alpo <62043214+AlpinYukseloglu@users.noreply.github.com>
Co-authored-by: Roman <roman@osmosis.team>
Co-authored-by: Adam Tucker <adam@osmosis.team>
This was referenced Aug 10, 2023
Closed
Closed
@github-actions github-actions bot mentioned this issue Nov 15, 2023
Closed
This was referenced Dec 1, 2023
Closed
Closed
This was referenced Dec 15, 2023
Closed
Closed
This was referenced Jan 1, 2024
Closed
Closed
This was referenced Jan 15, 2024
Closed
Closed
This was referenced Feb 1, 2024
Closed
Closed
This was referenced Feb 15, 2024
Closed
Closed
This was referenced Mar 1, 2024
Closed
Closed
This was referenced Mar 15, 2024
Closed
Closed
This was referenced Apr 1, 2024
Closed
Closed
This was referenced Apr 15, 2024
Closed
Closed
This was referenced May 1, 2024
Closed
Closed
This was referenced May 15, 2024
Closed
Closed
@github-actions github-actions bot mentioned this issue Jun 1, 2024
Closed
@github-actions github-actions bot mentioned this issue Jun 15, 2024
Closed
@github-actions github-actions bot mentioned this issue Jul 1, 2024
Closed
@github-actions github-actions bot mentioned this issue Jul 15, 2024
Closed
@github-actions github-actions bot mentioned this issue Aug 1, 2024
Closed
@github-actions github-actions bot mentioned this issue Aug 15, 2024
Closed
@github-actions github-actions bot mentioned this issue Sep 1, 2024
Closed
@github-actions github-actions bot mentioned this issue Sep 15, 2024
Closed
@github-actions github-actions bot mentioned this issue Oct 1, 2024
Closed
@github-actions github-actions bot mentioned this issue Nov 1, 2024
Closed
@github-actions github-actions bot mentioned this issue Nov 15, 2024
Closed
@github-actions github-actions bot mentioned this issue Dec 1, 2024
Closed
@github-actions github-actions bot mentioned this issue Dec 15, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[CL]: Increase min tick past canonical tick range osmosis-labs/osmosis
1 participant
@AlpinYukseloglu