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

Gauges queries #8563

Merged
merged 14 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
47 changes: 47 additions & 0 deletions proto/osmosis/incentives/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,21 @@ service Query {
"/osmosis/incentives/v1beta1/current_weight_by_group_gauge_id/"
"{group_gauge_id}";
}
rpc InternalGauges(QueryInternalGaugesRequest)
returns (QueryInternalGaugesResponse) {
option (google.api.http).get =
"/osmosis/incentives/v1beta1/internal_gauges";
}
rpc ExternalGauges(QueryExternalGaugesRequest)
returns (QueryExternalGaugesResponse) {
option (google.api.http).get =
"/osmosis/incentives/v1beta1/external_gauges";
}
rpc GaugesByPoolID(QueryGaugesByPoolIDRequest)
returns (QueryGaugesByPoolIDResponse) {
option (google.api.http).get =
"/osmosis/incentives/v1beta1/gauges_by_pool_id/{id}";
}
// Params returns incentives module params.
rpc Params(ParamsRequest) returns (ParamsResponse) {
option (google.api.http).get = "/osmosis/incentives/v1beta1/params";
Expand Down Expand Up @@ -243,5 +258,37 @@ message GaugeWeight {
];
}

message QueryInternalGaugesRequest {
// Pagination defines pagination for the request
cosmos.base.query.v1beta1.PageRequest pagination = 2;
najeal marked this conversation as resolved.
Show resolved Hide resolved
}
message QueryInternalGaugesResponse {
repeated Gauge gauges = 1 [ (gogoproto.nullable) = false ];
// Pagination defines pagination for the response
cosmos.base.query.v1beta1.PageResponse pagination = 2;
}

message QueryExternalGaugesRequest {
// Pagination defines pagination for the request
cosmos.base.query.v1beta1.PageRequest pagination = 2;
najeal marked this conversation as resolved.
Show resolved Hide resolved
}
message QueryExternalGaugesResponse {
repeated Gauge gauges = 1 [ (gogoproto.nullable) = false ];
// Pagination defines pagination for the response
cosmos.base.query.v1beta1.PageResponse pagination = 2;
}

message QueryGaugesByPoolIDRequest {
uint64 id = 1;
// Pagination defines pagination for the request
cosmos.base.query.v1beta1.PageRequest pagination = 2;
najeal marked this conversation as resolved.
Show resolved Hide resolved
}

message QueryGaugesByPoolIDResponse {
repeated Gauge gauges = 1 [ (gogoproto.nullable) = false ];
// Pagination defines pagination for the response
cosmos.base.query.v1beta1.PageResponse pagination = 2;
}

message ParamsRequest {}
message ParamsResponse { Params params = 1 [ (gogoproto.nullable) = false ]; }
40 changes: 40 additions & 0 deletions x/incentives/client/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,43 @@ func TestGetCmdUpcomingGaugesPerDenom(t *testing.T) {
}
osmocli.RunQueryTestCases(t, desc, tcs)
}

func TestGetCmdGaugesByPoolID(t *testing.T) {
desc, _ := GetCmdGaugesByPoolID()
tcs := map[string]osmocli.QueryCliTestCase[*types.QueryGaugesByPoolIDRequest]{
"basic test": {
Cmd: "1",
ExpectedQuery: &types.QueryGaugesByPoolIDRequest{
Id: 1,
Pagination: &query.PageRequest{Key: []uint8{}, Offset: 0, Limit: 100},
},
},
}
osmocli.RunQueryTestCases(t, desc, tcs)
}

func TestGetCmdExternalGauges(t *testing.T) {
desc, _ := GetCmdExternalGauges()
tcs := map[string]osmocli.QueryCliTestCase[*types.QueryExternalGaugesRequest]{
"basic test": {
Cmd: "",
ExpectedQuery: &types.QueryExternalGaugesRequest{
Pagination: &query.PageRequest{Key: []uint8{}, Offset: 0, Limit: 100},
},
},
}
osmocli.RunQueryTestCases(t, desc, tcs)
}

func TestGetCmdInternalGauges(t *testing.T) {
desc, _ := GetCmdInternalGauges()
tcs := map[string]osmocli.QueryCliTestCase[*types.QueryInternalGaugesRequest]{
"basic test": {
Cmd: "",
ExpectedQuery: &types.QueryInternalGaugesRequest{
Pagination: &query.PageRequest{Key: []uint8{}, Offset: 0, Limit: 100},
},
},
}
osmocli.RunQueryTestCases(t, desc, tcs)
}
30 changes: 30 additions & 0 deletions x/incentives/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ func GetQueryCmd() *cobra.Command {
osmocli.AddQueryCmd(cmd, qcGetter, GetCmdAllGroupsWithGauge)
osmocli.AddQueryCmd(cmd, qcGetter, GetCmdGroupByGroupGaugeID)
osmocli.AddQueryCmd(cmd, qcGetter, GetCmdCurrentWeightByGroupGaugeID)
osmocli.AddQueryCmd(cmd, qcGetter, GetCmdGaugesByPoolID)
osmocli.AddQueryCmd(cmd, qcGetter, GetCmdExternalGauges)
osmocli.AddQueryCmd(cmd, qcGetter, GetCmdInternalGauges)
cmd.AddCommand(
osmocli.GetParams[*types.ParamsRequest](
types.ModuleName, types.NewQueryClient),
Expand Down Expand Up @@ -265,3 +268,30 @@ func GetCmdGroupByGroupGaugeID() (*osmocli.QueryDescriptor, *types.QueryGroupByG
Long: `{{.Short}}`,
}, &types.QueryGroupByGroupGaugeIDRequest{}
}

// GetCmdGaugesByPoolID returns a gauges by PoolID.
func GetCmdGaugesByPoolID() (*osmocli.QueryDescriptor, *types.QueryGaugesByPoolIDRequest) {
return &osmocli.QueryDescriptor{
Use: "gauges-by-pool-id",
Short: "Query gauges by pool id.",
Long: `{{.Short}}`,
}, &types.QueryGaugesByPoolIDRequest{}
}

// GetCmdExternalGauges returns all external gauges.
func GetCmdExternalGauges() (*osmocli.QueryDescriptor, *types.QueryExternalGaugesRequest) {
return &osmocli.QueryDescriptor{
Use: "gauges-external",
Short: "Query external gauges.",
Long: `{{.Short}}`,
}, &types.QueryExternalGaugesRequest{}
}

// GetCmdInternalGauges returns all internal gauges.
func GetCmdInternalGauges() (*osmocli.QueryDescriptor, *types.QueryInternalGaugesRequest) {
return &osmocli.QueryDescriptor{
Use: "gauges-internal",
Short: "Query external gauges.",
Long: `{{.Short}}`,
}, &types.QueryInternalGaugesRequest{}
}
33 changes: 32 additions & 1 deletion x/incentives/keeper/gauge.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ var byGroupQueryCondition = lockuptypes.QueryCondition{LockQueryType: lockuptype

// 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 {
return k.getGaugesFromIteratorAndFilter(ctx, iterator, func(_ *types.Gauge) bool { return true })
}

// GaugeFilterFn is a function returning true if the Gauge has the expected values
type GaugeFilterFn func(*types.Gauge) bool

// getGaugesFromIterator iterates over everything in a gauge's iterator, until it reaches the end. Return all gauges iterated over.
func (k Keeper) getGaugesFromIteratorAndFilter(ctx sdk.Context, iterator db.Iterator, filter GaugeFilterFn) []types.Gauge {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this change right now? It seems like we're not calling this anymore

gauges := []types.Gauge{}
defer iterator.Close()
for ; iterator.Valid(); iterator.Next() {
Expand All @@ -40,7 +48,9 @@ func (k Keeper) getGaugesFromIterator(ctx sdk.Context, iterator db.Iterator) []t
if err != nil {
panic(err)
}
gauges = append(gauges, *gauge)
if filter(gauge) {
gauges = append(gauges, *gauge)
}
}
}
return gauges
Expand Down Expand Up @@ -310,6 +320,27 @@ func (k Keeper) GetGaugeFromIDs(ctx sdk.Context, gaugeIDs []uint64) ([]types.Gau
return gauges, nil
}

// GetInternalGauges returns internal gauges
func (k Keeper) GetInternalGauges(ctx sdk.Context) []types.Gauge {
return k.getGaugesFromIteratorAndFilter(ctx, k.GaugesIterator(ctx), func(g *types.Gauge) bool {
return g.IsInternalGauge()
})
}

// GetExternalGauges returns external gauges
func (k Keeper) GetExternalGauges(ctx sdk.Context) []types.Gauge {
return k.getGaugesFromIteratorAndFilter(ctx, k.GaugesIterator(ctx), func(g *types.Gauge) bool {
return g.IsExternalGauge()
})
}

// GetGaugesFromPoolID return gauges associated to poolID
func (k Keeper) GetGaugesFromPoolID(ctx sdk.Context, poolID uint64) []types.Gauge {
return k.getGaugesFromIteratorAndFilter(ctx, k.GaugesIterator(ctx), func(g *types.Gauge) bool {
return g.IsLinkedToPool(poolID)
})
}

// GetGauges returns upcoming, active, and finished gauges.
func (k Keeper) GetGauges(ctx sdk.Context) []types.Gauge {
return k.getGaugesFromIterator(ctx, k.GaugesIterator(ctx))
Expand Down
73 changes: 72 additions & 1 deletion x/incentives/keeper/gauge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,78 @@ func (s *KeeperTestSuite) TestInvalidDurationGaugeCreationValidation() {
s.Require().NoError(err)
}

func (s *KeeperTestSuite) TestGetGauges() {
s.SetupTest()

gaugesToCreate := []struct {
distrTo lockuptypes.QueryCondition
poolId uint64
}{
{
poolId: concentratedPoolId,
distrTo: lockuptypes.QueryCondition{
LockQueryType: lockuptypes.NoLock,
Denom: "",
Duration: time.Nanosecond,
},
},
{
poolId: concentratedPoolId,
distrTo: lockuptypes.QueryCondition{
LockQueryType: lockuptypes.NoLock,
Denom: "",
Duration: time.Nanosecond,
},
},
{
poolId: concentratedPoolId,
distrTo: lockuptypes.QueryCondition{
LockQueryType: lockuptypes.NoLock,
Denom: types.NoLockInternalGaugeDenom(concentratedPoolId),
Duration: s.App.IncentivesKeeper.GetEpochInfo(s.Ctx).Duration,
},
},
}

for _, coin := range defaultGaugeCreationCoins {
s.App.ProtoRevKeeper.SetPoolForDenomPair(s.Ctx, appparams.BaseCoinUnit, coin.Denom, 9999)
}
s.PrepareBalancerPool()
s.PrepareConcentratedPool()
s.FundAcc(s.TestAccs[0], defaultGaugeCreationCoins)

gaugeCreationCoins := sdk.NewCoins(
sdk.NewCoin(appparams.BaseCoinUnit, osmomath.NewInt(200)),
sdk.NewCoin("atom", osmomath.NewInt(200)),
)
for _, gaugeToCreate := range gaugesToCreate {
gaugeId, err := s.App.IncentivesKeeper.CreateGauge(s.Ctx, defaultIsPerpetualParam, s.TestAccs[0], gaugeCreationCoins, gaugeToCreate.distrTo, defaultTime, defaultNumEpochPaidOver, gaugeToCreate.poolId)
s.Require().NoError(err)
_ = gaugeId

}
gauges := s.App.IncentivesKeeper.GetGauges(s.Ctx)
s.Require().Len(gauges, 7)
s.Run("ExternalGauges", func() {
externalGauges := s.App.IncentivesKeeper.GetExternalGauges(s.Ctx)
s.Require().Len(externalGauges, 2)
s.Require().Equal(uint64(5), externalGauges[0].Id)
s.Require().Equal(uint64(6), externalGauges[1].Id)
})
s.Run("InternalGauges", func() {
internalGauges := s.App.IncentivesKeeper.GetInternalGauges(s.Ctx)
s.Require().Len(internalGauges, 2)
s.Require().Equal(uint64(7), internalGauges[0].Id)
s.Require().Equal(uint64(4), internalGauges[1].Id) // generated when initializing concentrated pool
})
s.Run("GaugesFromPoolID", func() {
gauges := s.App.IncentivesKeeper.GetGaugesFromPoolID(s.Ctx, concentratedPoolId)
s.Require().Len(gauges, 4)
gauges = s.App.IncentivesKeeper.GetGaugesFromPoolID(s.Ctx, balancerPoolId)
s.Require().Len(gauges, 3)
})
}

// TestNonExistentDenomGaugeCreation tests error handling for creating a gauge with an invalid denom.
func (s *KeeperTestSuite) TestNonExistentDenomGaugeCreation() {
s.SetupTest()
Expand Down Expand Up @@ -382,7 +454,6 @@ func (s *KeeperTestSuite) TestChargeFeeIfSufficientFeeDenomBalance() {
}

func (s *KeeperTestSuite) TestAddToGaugeRewards() {

defaultCoins := sdk.NewCoins(sdk.NewInt64Coin("stake", 12))

// since most of the same functionality and edge cases are tested by a higher level
Expand Down
83 changes: 83 additions & 0 deletions x/incentives/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,89 @@ func (q Querier) getGaugeFromIDJsonBytes(ctx sdk.Context, refValue []byte) ([]ty
return gauges, nil
}

// GaugeFilterFn is used to apply a filter on gauges
// It must be used in query.FilterPaginate as a condition to add the gauge to the response data
type GaugesFilterFn func(gauge *types.Gauge) bool

func (q Querier) GaugesByPoolID(goCtx context.Context, req *types.QueryGaugesByPoolIDRequest) (*types.QueryGaugesByPoolIDResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "empty request")
}
ctx := sdk.UnwrapSDKContext(goCtx)
pageRes, gauges, err := q.getGaugesWithFilter(ctx, types.KeyPrefixGauges, req.GetPagination(), func(gauge *types.Gauge) bool {
return gauge.IsLinkedToPool(req.GetId())
})
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
return &types.QueryGaugesByPoolIDResponse{
Gauges: gauges,
Pagination: pageRes,
}, nil
}

func (q Querier) InternalGauges(goCtx context.Context, req *types.QueryInternalGaugesRequest) (*types.QueryInternalGaugesResponse, error) {
mattverse marked this conversation as resolved.
Show resolved Hide resolved
if req == nil {
return nil, status.Error(codes.InvalidArgument, "empty request")
}
ctx := sdk.UnwrapSDKContext(goCtx)
pageRes, gauges, err := q.getGaugesWithFilter(ctx, types.KeyPrefixGauges, req.GetPagination(), func(gauge *types.Gauge) bool {
return gauge.IsInternalGauge()
})
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
return &types.QueryInternalGaugesResponse{
Gauges: gauges,
Pagination: pageRes,
}, nil
}

func (q Querier) ExternalGauges(goCtx context.Context, req *types.QueryExternalGaugesRequest) (*types.QueryExternalGaugesResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "empty request")
}
ctx := sdk.UnwrapSDKContext(goCtx)
pageRes, gauges, err := q.getGaugesWithFilter(ctx, types.KeyPrefixGauges, req.GetPagination(), func(gauge *types.Gauge) bool {
return gauge.IsExternalGauge()
})
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
return &types.QueryExternalGaugesResponse{
Gauges: gauges,
Pagination: pageRes,
}, nil
}

func (q Querier) getGaugesWithFilter(ctx sdk.Context, keyPrefix []byte, pageReq *query.PageRequest, filter GaugesFilterFn) (*query.PageResponse, []types.Gauge, error) {
gauges := []types.Gauge{}
store := ctx.KVStore(q.Keeper.storeKey)
valStore := prefix.NewStore(store, keyPrefix)

pageRes, err := query.FilteredPaginate(valStore, pageReq, func(key []byte, value []byte, accumulate bool) (bool, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we not filter than paginate? I'm curious if we paginate then filter inside, we wouldn't be getting correct pagination (e.g user wants 100 results, we do a filter inside and only returns 30 results). I might be wrong on this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Huum the way I understand how FilteredPaginate works it will continue to iterate until it reaches the 100 results.

I just missed one important point: to return true at the end of the closure when new element is added to the result.
When the closure returns true the FilteredPaginate function increments the numbered of hit elements and verifies that num_hit < request.Limit before continuing to iterate.
I will make the update.

// this may return multiple gauges at once if two gauges start at the same time.
// for now this is treated as an edge case that is not of importance
newGauges, err := q.getGaugeFromIDJsonBytes(ctx, value)
if err != nil {
return false, err
}
if accumulate {
for _, gauge := range newGauges {
if !filter(&gauge) {
continue
}
gauges = append(gauges, gauge)
}
}
return true, nil
})
if err != nil {
return nil, nil, err
}
return pageRes, gauges, nil
}

// filterByPrefixAndDenom filters gauges based on a given key prefix and denom
func (q Querier) filterByPrefixAndDenom(ctx sdk.Context, prefixType []byte, denom string, pagination *query.PageRequest) (*query.PageResponse, []types.Gauge, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function looks not correct if I am right about the way FilteredPaginate works. What do you think @mattverse

Copy link
Member

Choose a reason for hiding this comment

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

which part are you sus about?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Iterating over newGauges the closure returns false if the denom is not the expected one.
In some situations we can miss a gauge
If len(newGauges) == 2
Index 0 : denom is not expecting one
Index 1 : denom is the expecting one

we are going to exit at first iteration without adding the second gauge in the response.

Copy link
Member

Choose a reason for hiding this comment

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

yeah, this is an edge case as mentioned in the comment that we need to fix :(

gauges := []types.Gauge{}
Expand Down
Loading