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

[CL][Concentrated Liquidity Module]: create a separate file and folder for pool implementation #3243

Closed
p0mvn opened this issue Nov 4, 2022 · 3 comments
Assignees
Labels
C:x/concentrated-liquidity F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board

Comments

@p0mvn
Copy link
Member

p0mvn commented Nov 4, 2022

Background

Currently, we have keeper methods and pool methods both mixed up in pool.go.

We should separate pool methods from the keeper methods.

Suggested Design

  1. Create a concentratedpool package in the root of the module.
  2. Regenerate the concentratedPool.pb.go file to the above package
  3. Move all pool method implementations from into the concentratedpool package
  4. Port tests

Acceptance Criteria

  • design implemented
  • keeper and pool implementations are separated
@p0mvn p0mvn added C:x/concentrated-liquidity F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board labels Nov 4, 2022
@osmo-bot osmo-bot moved this to Needs Review 🔍 in Osmosis Chain Development Nov 4, 2022
@mattverse
Copy link
Member

Hrrm, srry if Im missing something, but I don't understand the reason to have a separated out structure for the cl module if cl is the only pool model within the module. Also, this way, we can't access state for the ticks if its a pool method. Are you thinking of this design to be swap-router compatible?

@mattverse mattverse self-assigned this Nov 7, 2022
@p0mvn
Copy link
Member Author

p0mvn commented Nov 7, 2022

I don't understand the reason to have a separated out structure for the cl module if cl is the only pool model within the module

Do you mean having a separate pool model?

Pool-model is a separate component so I think it makes sense to encapsulate it into its own package. It would also work well with the abstractions in x/gamm where each pool model belongs to their own package.

Also, this way, we can't access state for the ticks if its a pool method.

With the current API, what are the methods that would need to have access to ticks? We should be able to keep such methods on the keeper.

Are you thinking of this design to be swap-router compatible?

What do you mean exactly by swaprouter compatible? If it implements PoolI, it is compatible with the swaprouter

@p0mvn
Copy link
Member Author

p0mvn commented Nov 27, 2022

This is complete in #3296

@p0mvn p0mvn closed this as completed Nov 27, 2022
Repository owner moved this from Needs Review 🔍 to Done ✅ in Osmosis Chain Development Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/concentrated-liquidity F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board
Projects
Archived in project
Development

No branches or pull requests

2 participants