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

feat[CL]: add MsgCreateConcentratedPool #3607

Closed

Conversation

czarcas7ic
Copy link
Member

@czarcas7ic czarcas7ic commented Dec 3, 2022

Closes: #3245

What is the purpose of the change

Implementation of MsgCreateConcentratedPool and required wiring

Brief Changelog

(for example:)

  • The metadata is stored in the blob store on job creation time as a persistent artifact
  • Deployments RPC transmits only the blob storage reference
  • Daemons retrieve the RPC data from the blob cache

@github-actions github-actions bot added 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/swaprouter labels Dec 3, 2022
@czarcas7ic czarcas7ic marked this pull request as ready for review December 3, 2022 01:04
@czarcas7ic czarcas7ic requested review from a team and p0mvn December 3, 2022 01:04
Copy link
Member

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

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

Nice work!

x/concentrated-liquidity/keeper_test.go Outdated Show resolved Hide resolved
x/concentrated-liquidity/lp.go Show resolved Hide resolved
x/concentrated-liquidity/lp.go Outdated Show resolved Hide resolved
x/concentrated-liquidity/lp.go Outdated Show resolved Hide resolved
x/concentrated-liquidity/lp.go Outdated Show resolved Hide resolved
x/concentrated-liquidity/msg_server.go Outdated Show resolved Hide resolved
x/concentrated-liquidity/msg_server.go Outdated Show resolved Hide resolved
x/swaprouter/creator.go Outdated Show resolved Hide resolved
x/swaprouter/creator.go Outdated Show resolved Hide resolved
x/swaprouter/creator.go Outdated Show resolved Hide resolved
@czarcas7ic czarcas7ic requested a review from p0mvn December 4, 2022 04:25
@czarcas7ic
Copy link
Member Author

@p0mvn addressed most of your comments with a few followups!

@p0mvn
Copy link
Member

p0mvn commented Dec 5, 2022

@czarcas7ic thanks, replied to the remaining unresolved discussions

@czarcas7ic
Copy link
Member Author

@p0mvn all concerns addressed!

@czarcas7ic
Copy link
Member Author

@p0mvn I got rid of the internal model package but for some reason proto check is failing on this as well as the go test. Is this normal?

@p0mvn
Copy link
Member

p0mvn commented Dec 5, 2022

@czarcas7ic I think there might be an import that is still referencing the old package

@czarcas7ic
Copy link
Member Author

@p0mvn Just double checked and there isnt :/

@czarcas7ic
Copy link
Member Author

I think that its a false error. It was passing before, and the changes I made dont effect this..

@czarcas7ic
Copy link
Member Author

Yeah, I reverted back to a point where it was passing and it still fails, so I think its something to do with CI

@@ -56,6 +57,14 @@ func (server msgServer) CreateStableswapPool(goCtx context.Context, msg *stables
return &stableswap.MsgCreateStableswapPoolResponse{PoolID: poolId}, nil
}

func (server msgServer) CreateConcentratedPool(goCtx context.Context, msg *cl.MsgCreateConcentratedPool) (*cl.MsgCreateConcentratedPoolResponse, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Based on discussions with Dev yesterday, we are going to move the "create pool" message servers to their respective modules. So this would go to CL. Can you try moving it please? It should still call the swaprouter's CreatePool keeper method

Stableswap and GAMM will get moved to gamm but let's do that in another PR.

@czarcas7ic
Copy link
Member Author

Closing in favor of #3687

@czarcas7ic czarcas7ic closed this Dec 11, 2022
@czarcas7ic czarcas7ic deleted the adam/msgCreateConcentratedPool branch August 7, 2023 23:52
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/swaprouter
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants