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: get to distribute coins with single iteration #2005

Closed
czarcas7ic opened this issue Jul 9, 2022 · 0 comments · Fixed by #2014
Closed

x/incentives: get to distribute coins with single iteration #2005

czarcas7ic opened this issue Jul 9, 2022 · 0 comments · Fixed by #2014
Assignees

Comments

@czarcas7ic
Copy link
Member

Background

getToDistributeCoinsFromGauges currently requires multiple iterations over all gauges

// getToDistributeCoinsFromGauges returns coins that have not been distributed yet from the provided gauges
func (k Keeper) getToDistributeCoinsFromGauges(gauges []types.Gauge) sdk.Coins {
// TODO: Consider optimizing this in the future to only require one iteration over all gauges.
// TODO: Lets make issue for this, requires changing getGaugesFromIterator logic in gauge.go
coins := k.getCoinsFromGauges(gauges)
distributed := k.getDistributedCoinsFromGauges(gauges)
return coins.Sub(distributed)
}

Suggested Design

I believe this would require making changes to the getGaugesFromIterator logic in gauge.go found here

// getGaugesFromIterator iterates over everything in a gauge's iterator, until it reaches the end. Return all gauges iterated over.
func (k Keeper) getGaugesFromIterator(ctx sdk.Context, iterator db.Iterator) []types.Gauge {
gauges := []types.Gauge{}
defer iterator.Close()
for ; iterator.Valid(); iterator.Next() {
gaugeIDs := []uint64{}
err := json.Unmarshal(iterator.Value(), &gaugeIDs)
if err != nil {
panic(err)
}
for _, gaugeID := range gaugeIDs {
gauge, err := k.GetGaugeByID(ctx, gaugeID)
if err != nil {
panic(err)
}
gauges = append(gauges, *gauge)
}
}
return gauges
}

Acceptance Criteria

  • Getting toDistribute coins should only require one iteration over all gauges
  • All existing tests should pass
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants