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

x/incentives: index synthetic lockups by denom #2006

Closed
Tracked by #4622
czarcas7ic opened this issue Jul 9, 2022 · 6 comments
Closed
Tracked by #4622

x/incentives: index synthetic lockups by denom #2006

czarcas7ic opened this issue Jul 9, 2022 · 6 comments

Comments

@czarcas7ic
Copy link
Member

Background

Synthetic lockups aren't indexed by denom as expected

// getLocksToDistributionWithMaxDuration returns locks that match the provided lockuptypes QueryCondition,
// are greater than the provided minDuration, AND have yet to be distributed to.
func (k Keeper) getLocksToDistributionWithMaxDuration(ctx sdk.Context, distrTo lockuptypes.QueryCondition, minDuration time.Duration) []lockuptypes.PeriodLock {
switch distrTo.LockQueryType {
case lockuptypes.ByDuration:
// TODO: Create issue for this
// Confusingly, synthetic lockups aren't indexed by denom as expected.
// Thus you have to do this as a hack
denom := lockuptypes.NativeDenom(distrTo.Denom)
if distrTo.Duration > minDuration {
return k.lk.GetLocksLongerThanDurationDenom(ctx, denom, minDuration)
}
return k.lk.GetLocksLongerThanDurationDenom(ctx, distrTo.Denom, distrTo.Duration)
case lockuptypes.ByTime:
panic("Gauge by time is present, however is no longer supported. This should have been blocked in ValidateBasic")
default:
}
return []lockuptypes.PeriodLock{}
}

Suggested Design

Will need to come back to this to figure out a suggested design.

Acceptance Criteria

  • Synthetic lockups are indexed by denom
  • Hack no longer required in getLocksToDistributionWithMaxDuration
@stackman27
Copy link
Contributor

Hi @czarcas7ic is this issue being worked on? can i take a shot at it? Will follow up with @ValarDragon regarding the design

@czarcas7ic
Copy link
Member Author

@stackman27 please do, I will assign it to you!

@ValarDragon
Copy link
Member

Hey, just saw this issue. I think we should not index these, and instead be planning to delete the synthetic locks entirely.

Rather than maintain the current state, would rather put such energy into getting it deleted 💪

@stackman27
Copy link
Contributor

Are we trying to make it a part of period locks?

@ValarDragon
Copy link
Member

This is the design we should go towards imo #942

@ValarDragon
Copy link
Member

Were not going to pursue this.

@github-project-automation github-project-automation bot moved this from Needs Triage 🔍 to Done ✅ in Osmosis Chain Development Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

4 participants