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

[Incentives] Refactor createGauge to accomodate for CL gauges #4833

Closed
stackman27 opened this issue Apr 4, 2023 · 5 comments
Closed

[Incentives] Refactor createGauge to accomodate for CL gauges #4833

stackman27 opened this issue Apr 4, 2023 · 5 comments
Assignees
Labels
C:x/concentrated-liquidity C:x/incentives C:x/pool-incentives F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board

Comments

@stackman27
Copy link
Contributor

stackman27 commented Apr 4, 2023

Background

After merging #4526, we pass in dummy variables to lockuptypes.QueryCondition{} for CL gauges to pass createGauge function. While we do not use any lockuptypes.QueryCondition{} parameters we pass the values so that we do not have to modify the lower level createGauge function

Suggested Design

There are couple of ways of fixing this;

  1. modify createGauge to accomodate different type of pool Gauges, this will be modifying the queryCondition check in the createGauge function
  2. allow lockuptypes.QueryCondition{} to be empty in createGauge function

Acceptance Criteria

  • new tests as well as old tests should pass

Goals & criteria for success
e.g.

  • all existing and new tests should pass
@github-project-automation github-project-automation bot moved this to Needs Triage 🔍 in Osmosis Chain Development Apr 4, 2023
@mattverse mattverse added C:x/concentrated-liquidity F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board labels Apr 6, 2023
@p0mvn p0mvn moved this from Needs Triage 🔍 to On Hold in Osmosis Chain Development May 23, 2023
@czarcas7ic czarcas7ic moved this from On Hold to Todo 🕒 in Osmosis Chain Development May 30, 2023
@hieuvubk
Copy link
Contributor

hieuvubk commented May 31, 2023

Is this issue still available, I see we have CreateConcentratedLiquidityPoolGauge for CL gauge.
But looks like we're over checking this condition:
https://github.com/osmosis-labs/osmosis/blob/main/x/pool-incentives/keeper/keeper.go#L98-L101

@stackman27
Copy link
Contributor Author

stackman27 commented May 31, 2023

so this issues a suggestion to change this QueryCondition https://github.com/osmosis-labs/osmosis/blob/main/x/pool-incentives/keeper/keeper.go#L111

right now its

	lockuptypes.QueryCondition{
			LockQueryType: lockuptypes.ByTime,
			Denom:         sdk.DefaultBondDenom,
	},

which essentially is pretty useless, it was added so that we can skip this logic check (https://github.com/osmosis-labs/osmosis/blob/main/x/incentives/keeper/gauge.go#L100) because we donot really care about the lockup type for CL incentive gauges

one of the fix could be to allow lockuptypes.QueryCondition{} and still be able to run createGauge function seamlessly

@stackman27
Copy link
Contributor Author

lmk if that makes sense, happy to explain more if not

@hieuvubk
Copy link
Contributor

hieuvubk commented Jun 1, 2023

Make sense to me

@ValarDragon
Copy link
Member

I believe this was handled in some of Alpins recent work

@github-project-automation github-project-automation bot moved this from Todo 🕒 to Done ✅ in Osmosis Chain Development Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/concentrated-liquidity C:x/incentives C:x/pool-incentives F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants