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][e2e]: CreateConcentratedPosition #4056

Merged
merged 28 commits into from
Feb 2, 2023
Merged

Conversation

pysel
Copy link
Member

@pysel pysel commented Jan 18, 2023

  • CreatePosition
  • rename to CreateConcentratedPosition

Closes: #4055

What is the purpose of the change

This PR adds a function - CreateConcentratedPosition - that creates a concentrated position

Brief Changelog

CreateConcentratedPosition added to commands.go

Testing and Verifying

Testing blocked before #4038 is merged

Documentation and Release Note

no

@pysel pysel added C:e2e V:state/compatible/no_backport State machine compatible PR, depends on prior breaks F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board labels Jan 18, 2023
@mattverse
Copy link
Member

Wdyt about pointing this PR to ruslan/e2e-cl instead of main?

@p0mvn p0mvn marked this pull request as draft January 19, 2023 09:53
@pysel pysel force-pushed the ruslan/e2e-cl-create-position branch from 0976add to 59b3bf0 Compare January 27, 2023 04:25
@pysel
Copy link
Member Author

pysel commented Jan 27, 2023

@p0mvn Current positions:
Screen Shot 2023-01-27 at 11 15 39

minTick, maxTick := cl.GetMinAndMaxTicksFromExponentAtPriceOne(sdk.NewInt(precisionFactorAtPriceOne))

// Create 2 positions for node1: overlap together, overlap with 2 node3 positions)
node1.CreateConcentratedPosition(initialization.ValidatorWalletName, "[-1200]", "400", fmt.Sprintf("1000%s", denom0), fmt.Sprintf("1000%s", denom1), 0, 0, poolID)
Copy link
Member

Choose a reason for hiding this comment

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

What matters is using different addressed to create positions from.

We would like to have positions created from the same addresses as well as some different addresses. Overlapping and non-overlapping

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, completely makes sense, fixed that. It means there is not really any use anymore for GetNodeAtIndex I created for this PR. However, it is still a pretty nice function, shall we keep it? wdyt? @p0mvn

minTick, maxTick := cl.GetMinAndMaxTicksFromExponentAtPriceOne(sdk.NewInt(precisionFactorAtPriceOne))

// Create 2 positions for node1: overlap together, overlap with 2 node3 positions)
node1.CreateConcentratedPosition(initialization.ValidatorWalletName, "[-1200]", "400", fmt.Sprintf("1000%s", denom0), fmt.Sprintf("1000%s", denom1), 0, 0, poolID)
Copy link
Member Author

Choose a reason for hiding this comment

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

It is also possible to provide ticks as ints here, but I thought it would be better to make code a little dirtier here, but keep CreateConcentratedPosition clear by avoiding transforming int-ticks to strings

minTick, maxTick := cl.GetMinAndMaxTicksFromExponentAtPriceOne(sdk.NewInt(precisionFactorAtPriceOne))

// Create 2 positions for node1: overlap together, overlap with 2 node3 positions)
node1.CreateConcentratedPosition(initialization.ValidatorWalletName, "[-1200]", "400", fmt.Sprintf("1000%s", denom0), fmt.Sprintf("1000%s", denom1), 0, 0, poolID)
Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, completely makes sense, fixed that. It means there is not really any use anymore for GetNodeAtIndex I created for this PR. However, it is still a pretty nice function, shall we keep it? wdyt? @p0mvn

@pysel pysel force-pushed the ruslan/e2e-cl-create-position branch from 7fd36f9 to 338eb63 Compare February 2, 2023 05:45
@pysel pysel marked this pull request as ready for review February 2, 2023 08:00
@pysel pysel requested a review from p0mvn February 2, 2023 08:00
@@ -24,7 +24,7 @@ const (
// It should be uploaded to Docker Hub. OSMOSIS_E2E_SKIP_UPGRADE should be unset
// for this functionality to be used.
previousVersionOsmoRepository = "osmolabs/osmosis-dev"
previousVersionOsmoTag = "v14.x-de392d2b-1674667154"
previousVersionOsmoTag = "v14.x-6b742c84-1675172347"
Copy link
Member

Choose a reason for hiding this comment

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

How were you able to push the update? Is it correctly auto-pushing to Docker hub now?

Copy link
Member Author

Choose a reason for hiding this comment

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

we had #3503 this workflow to push osmosis-dev images

@p0mvn p0mvn merged commit d115076 into main Feb 2, 2023
@p0mvn p0mvn deleted the ruslan/e2e-cl-create-position branch February 2, 2023 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:e2e C:x/concentrated-liquidity F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board V:state/compatible/no_backport State machine compatible PR, depends on prior breaks
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

e2e: create concentrated position
3 participants