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.

Sign up for GitHub

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

Fix incentives query filtering #1759

Merged
merged 10 commits into from
Jun 15, 2022
Merged

Fix incentives query filtering #1759

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f5292cd
Merge pull request #3 from osmosis-labs/main
hieuvubk Jun 10, 2022
62d4d47
fix filtering logic
hieuvubk Jun 10, 2022
66ef1c6
add test
hieuvubk Jun 10, 2022
d6811c0
format
hieuvubk Jun 10, 2022
ba75477
Merge pull request #32 from hieuvubk/issue-pagination
hieuvubk Jun 10, 2022
40a5c38
fix: append element in loop
hieuvubk Jun 10, 2022
b8539ed
use range
hieuvubk Jun 11, 2022
7d81622
optimize code, make filtering into a separate function
hieuvubk Jun 14, 2022
bc69184
format
hieuvubk Jun 14, 2022
9b9a0cf
add to changelog
hieuvubk Jun 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 29 additions & 11 deletions x/incentives/keeper/grpc_query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ func (q Querier) Gauges(goCtx context.Context, req *types.GaugesRequest) (*types
if err != nil {
panic(err)
}
gauges = append(gauges, newGauges...)
if accumulate {
gauges = append(gauges, newGauges...)
}

return true, nil
})
Expand All @@ -99,7 +101,9 @@ func (q Querier) ActiveGauges(goCtx context.Context, req *types.ActiveGaugesRequ
if err != nil {
panic(err)
}
gauges = append(gauges, newGauges...)
if accumulate {
gauges = append(gauges, newGauges...)
}

return true, nil
})
Expand All @@ -122,10 +126,16 @@ func (q Querier) ActiveGaugesPerDenom(goCtx context.Context, req *types.ActiveGa
valStore := prefix.NewStore(store, types.KeyPrefixActiveGauges)

pageRes, err := query.FilteredPaginate(valStore, req.Pagination, func(key []byte, value []byte, accumulate bool) (bool, error) {
activeGauges := q.Keeper.GetActiveGauges(ctx)
for _, gauge := range activeGauges {
if gauge.DistributeTo.Denom == req.Denom {
gauges = append(gauges, gauge)
newGauges, err := q.getGaugeFromIDJsonBytes(ctx, value)
if err != nil {
panic(err)
}
for i := 0; i < len(newGauges); i++ {
Copy link
Contributor

@alexanderbez alexanderbez Jun 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
}
for i := 0; i < len(newGauges); i++ {
}
for i, g := range newGauges {

if newGauges[i].DistributeTo.Denom != req.Denom {
return false, nil
}
if accumulate {
gauges = append(gauges, newGauges...)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we be appending a single element, i.e. newGauges[i] instead of the entire slice each time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes sure, my mistake

}
}
return true, nil
Expand Down Expand Up @@ -153,7 +163,9 @@ func (q Querier) UpcomingGauges(goCtx context.Context, req *types.UpcomingGauges
if err != nil {
panic(err)
}
gauges = append(gauges, newGauges...)
if accumulate {
gauges = append(gauges, newGauges...)
}

return true, nil
})
Expand All @@ -179,10 +191,16 @@ func (q Querier) UpcomingGaugesPerDenom(goCtx context.Context, req *types.Upcomi
prefixStore := prefix.NewStore(store, types.KeyPrefixUpcomingGauges)

pageRes, err := query.FilteredPaginate(prefixStore, req.Pagination, func(key []byte, value []byte, accumulate bool) (bool, error) {
upcomingGauges := q.Keeper.GetUpcomingGauges(ctx)
for _, gauge := range upcomingGauges {
if gauge.DistributeTo.Denom == req.Denom {
gauges = append(gauges, gauge)
newGauges, err := q.getGaugeFromIDJsonBytes(ctx, value)
if err != nil {
panic(err)
}
for i := 0; i < len(newGauges); i++ {
if newGauges[i].DistributeTo.Denom != req.Denom {
return false, nil
}
if accumulate {
gauges = append(gauges, newGauges...)
}
}
return true, nil
Expand Down
86 changes: 86 additions & 0 deletions x/incentives/keeper/grpc_query_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package keeper_test

import (
// "fmt"
"time"

sdk "github.com/cosmos/cosmos-sdk/types"

query "github.com/cosmos/cosmos-sdk/types/query"
"github.com/osmosis-labs/osmosis/v7/x/incentives/types"
lockuptypes "github.com/osmosis-labs/osmosis/v7/x/lockup/types"
pooltypes "github.com/osmosis-labs/osmosis/v7/x/pool-incentives/types"
Expand Down Expand Up @@ -72,6 +74,16 @@ func (suite *KeeperTestSuite) TestGRPCGauges() {
StartTime: startTime,
}
suite.Require().Equal(res.Data[0].String(), expectedGauge.String())

// filtering check
for i := 0; i < 10; i++ {
suite.SetupNewGauge(false, sdk.Coins{sdk.NewInt64Coin("stake", 3)})
suite.Ctx = suite.Ctx.WithBlockTime(startTime.Add(time.Second))
}

filter := query.PageRequest{Limit: 10}
res, err = suite.querier.Gauges(sdk.WrapSDKContext(suite.Ctx), &types.GaugesRequest{Pagination: &filter})
suite.Require().Len(res.Data, 10)
}

func (suite *KeeperTestSuite) TestGRPCActiveGauges() {
Expand Down Expand Up @@ -107,6 +119,23 @@ func (suite *KeeperTestSuite) TestGRPCActiveGauges() {
StartTime: startTime,
}
suite.Require().Equal(res.Data[0].String(), expectedGauge.String())

// filtering check
for i := 0; i < 20; i++ {
_, gauge, _, _ := suite.SetupNewGauge(false, sdk.Coins{sdk.NewInt64Coin("stake", 3)})
suite.Ctx = suite.Ctx.WithBlockTime(startTime.Add(time.Second))

// set up more 9 active gauges => 10
if i < 9 {
suite.querier.MoveUpcomingGaugeToActiveGauge(suite.Ctx, *gauge)
}
}

res, err = suite.querier.ActiveGauges(sdk.WrapSDKContext(suite.Ctx), &types.ActiveGaugesRequest{Pagination: &query.PageRequest{Limit: 5}})
suite.Require().Len(res.Data, 5)

res, err = suite.querier.ActiveGauges(sdk.WrapSDKContext(suite.Ctx), &types.ActiveGaugesRequest{Pagination: &query.PageRequest{Limit: 15}})
suite.Require().Len(res.Data, 10)
}

func (suite *KeeperTestSuite) TestGRPCActiveGaugesPerDenom() {
Expand Down Expand Up @@ -140,6 +169,26 @@ func (suite *KeeperTestSuite) TestGRPCActiveGaugesPerDenom() {
StartTime: startTime,
}
suite.Require().Equal(res.Data[0].String(), expectedGauge.String())

// filtering check
for i := 0; i < 20; i++ {
_, gauge, _, _ := suite.SetupNewGaugeWithDenom(false, sdk.Coins{sdk.NewInt64Coin("stake", 3)}, "pool")
suite.Ctx = suite.Ctx.WithBlockTime(startTime.Add(time.Second))

// set up 10 active gauges with "pool" denom
if i < 10 {
suite.querier.MoveUpcomingGaugeToActiveGauge(suite.Ctx, *gauge)
}
}

res, err = suite.querier.ActiveGaugesPerDenom(sdk.WrapSDKContext(suite.Ctx), &types.ActiveGaugesPerDenomRequest{Denom: "lptoken", Pagination: &query.PageRequest{Limit: 5}})
suite.Require().Len(res.Data, 1)

res, err = suite.querier.ActiveGaugesPerDenom(sdk.WrapSDKContext(suite.Ctx), &types.ActiveGaugesPerDenomRequest{Denom: "pool", Pagination: &query.PageRequest{Limit: 5}})
suite.Require().Len(res.Data, 5)

res, err = suite.querier.ActiveGaugesPerDenom(sdk.WrapSDKContext(suite.Ctx), &types.ActiveGaugesPerDenomRequest{Denom: "pool", Pagination: &query.PageRequest{Limit: 15}})
suite.Require().Len(res.Data, 10)
}

func (suite *KeeperTestSuite) TestGRPCUpcomingGauges() {
Expand Down Expand Up @@ -172,6 +221,23 @@ func (suite *KeeperTestSuite) TestGRPCUpcomingGauges() {
StartTime: startTime,
}
suite.Require().Equal(res.Data[0].String(), expectedGauge.String())

// filtering check
for i := 0; i < 20; i++ {
_, gauge, _, _ := suite.SetupNewGauge(false, sdk.Coins{sdk.NewInt64Coin("stake", 3)})
suite.Ctx = suite.Ctx.WithBlockTime(startTime.Add(time.Second))

// set up more 9 active gauges => Upcoming = 1 + (20 -9) = 12
if i < 9 {
suite.querier.MoveUpcomingGaugeToActiveGauge(suite.Ctx, *gauge)
}
}

res, err = suite.querier.UpcomingGauges(sdk.WrapSDKContext(suite.Ctx), &types.UpcomingGaugesRequest{Pagination: &query.PageRequest{Limit: 5}})
suite.Require().Len(res.Data, 5)

res, err = suite.querier.UpcomingGauges(sdk.WrapSDKContext(suite.Ctx), &types.UpcomingGaugesRequest{Pagination: &query.PageRequest{Limit: 15}})
suite.Require().Len(res.Data, 12)
}

func (suite *KeeperTestSuite) TestGRPCUpcomingGaugesPerDenom() {
Expand Down Expand Up @@ -210,6 +276,26 @@ func (suite *KeeperTestSuite) TestGRPCUpcomingGaugesPerDenom() {
res, err = suite.querier.UpcomingGaugesPerDenom(sdk.WrapSDKContext(suite.Ctx), &upcomingGaugeRequest)
suite.Require().NoError(err)
suite.Require().Len(res.UpcomingGauges, 0)

// filtering check
for i := 0; i < 20; i++ {
_, gauge, _, _ := suite.SetupNewGaugeWithDenom(false, sdk.Coins{sdk.NewInt64Coin("stake", 3)}, "pool")
suite.Ctx = suite.Ctx.WithBlockTime(startTime.Add(time.Second))

// set up 10 active gauges with "pool" denom => 10 upcoming
if i < 10 {
suite.querier.MoveUpcomingGaugeToActiveGauge(suite.Ctx, *gauge)
}
}

res, err = suite.querier.UpcomingGaugesPerDenom(sdk.WrapSDKContext(suite.Ctx), &types.UpcomingGaugesPerDenomRequest{Denom: "lptoken", Pagination: &query.PageRequest{Limit: 5}})
suite.Require().Len(res.UpcomingGauges, 0)

res, err = suite.querier.UpcomingGaugesPerDenom(sdk.WrapSDKContext(suite.Ctx), &types.UpcomingGaugesPerDenomRequest{Denom: "pool", Pagination: &query.PageRequest{Limit: 5}})
suite.Require().Len(res.UpcomingGauges, 5)

res, err = suite.querier.UpcomingGaugesPerDenom(sdk.WrapSDKContext(suite.Ctx), &types.UpcomingGaugesPerDenomRequest{Denom: "pool", Pagination: &query.PageRequest{Limit: 15}})
suite.Require().Len(res.UpcomingGauges, 10)
}

func (suite *KeeperTestSuite) TestGRPCRewardsEst() {
Expand Down
27 changes: 27 additions & 0 deletions x/incentives/keeper/suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,33 @@ func (suite *KeeperTestSuite) SetupNewGauge(isPerpetual bool, coins sdk.Coins) (
return suite.setupNewGaugeWithDuration(isPerpetual, coins, defaultLockDuration)
}

func (suite *KeeperTestSuite) setupNewGaugeWithDenom(isPerpetual bool, coins sdk.Coins, duration time.Duration, denom string) (
uint64, *types.Gauge, sdk.Coins, time.Time,
) {
addr := sdk.AccAddress([]byte("Gauge_Creation_Addr_"))
startTime2 := time.Now()
distrTo := lockuptypes.QueryCondition{
LockQueryType: lockuptypes.ByDuration,
Denom: denom,
Duration: duration,
}

// mints coins so supply exists on chain
mintCoins := sdk.Coins{sdk.NewInt64Coin(distrTo.Denom, 200)}
suite.FundAcc(addr, mintCoins)

numEpochsPaidOver := uint64(2)
if isPerpetual {
numEpochsPaidOver = uint64(1)
}
gaugeID, gauge := suite.CreateGauge(isPerpetual, addr, coins, distrTo, startTime2, numEpochsPaidOver)
return gaugeID, gauge, coins, startTime2
}

func (suite *KeeperTestSuite) SetupNewGaugeWithDenom(isPerpetual bool, coins sdk.Coins, denom string) (uint64, *types.Gauge, sdk.Coins, time.Time) {
return suite.setupNewGaugeWithDenom(isPerpetual, coins, defaultLockDuration, denom)
}

func (suite *KeeperTestSuite) SetupManyLocks(numLocks int, liquidBalance sdk.Coins, coinsPerLock sdk.Coins,
lockDuration time.Duration,
) []sdk.AccAddress {
Expand Down