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 all 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 = 1;
}
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 = 1;
}
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;
}

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{}
}
1 change: 0 additions & 1 deletion x/incentives/keeper/gauge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,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
86 changes: 86 additions & 0 deletions x/incentives/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/cosmos/cosmos-sdk/types/query"

"github.com/osmosis-labs/osmosis/osmomath"
gammtypes "github.com/osmosis-labs/osmosis/v26/x/gamm/types"
"github.com/osmosis-labs/osmosis/v26/x/incentives/types"
lockuptypes "github.com/osmosis-labs/osmosis/v26/x/lockup/types"
)
Expand Down Expand Up @@ -269,6 +270,91 @@ func (q Querier) getGaugeFromIDJsonBytes(ctx sdk.Context, refValue []byte) ([]ty
return gauges, nil
}

// GaugesFilterFn 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) {
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([]string{gammtypes.GAMMTokenPrefix})
})
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
}
var hit bool
if accumulate {
for _, gauge := range newGauges {
if !filter(&gauge) {
continue
}
gauges = append(gauges, gauge)
hit = true
}
}
return hit, 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
116 changes: 116 additions & 0 deletions x/incentives/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
query "github.com/cosmos/cosmos-sdk/types/query"

"github.com/osmosis-labs/osmosis/osmomath"
appparams "github.com/osmosis-labs/osmosis/v26/app/params"
"github.com/osmosis-labs/osmosis/v26/x/incentives/types"
lockuptypes "github.com/osmosis-labs/osmosis/v26/x/lockup/types"
pooltypes "github.com/osmosis-labs/osmosis/v26/x/pool-incentives/types"
Expand Down Expand Up @@ -49,6 +50,121 @@ func (s *KeeperTestSuite) TestGRPCGaugeByID() {
s.Require().Equal(res.Gauge.String(), expectedGauge.String())
}

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

// ensure initially querying gauges returns no gauges
res, err := s.querier.Gauges(s.Ctx, &types.GaugesRequest{})
s.Require().NoError(err)
s.Require().Len(res.Data, 0)

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)),
)

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 _, 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
}

// ensure initially querying gauges returns 7 gauges
res, err = s.querier.Gauges(s.Ctx, &types.GaugesRequest{})
s.Require().NoError(err)
s.Require().Len(res.Data, 7)

// checkContainsGauges check each id has its equivalent gauge
// it returns ids not having their equivalent gauge
checkContainsGauges := func(gauges []types.Gauge, ids []uint64) []uint64 {
missing := []uint64{}
for _, id := range ids {
found := false
for _, gauge := range gauges {
if gauge.Id == id {
found = true
break
}
}
if !found {
missing = append(missing, id)
}
}
return missing
}

s.Run("ExternalGauges", func() {
res, err := s.querier.ExternalGauges(s.Ctx, &types.QueryExternalGaugesRequest{})
s.Require().NoError(err)
s.Require().Len(res.GetGauges(), 2)
missingGauges := checkContainsGauges(res.GetGauges(), []uint64{5, 6})
s.Require().Empty(missingGauges, "missing gauges %v", missingGauges)
})
s.Run("InternalGauges", func() {
res, err := s.querier.InternalGauges(s.Ctx, &types.QueryInternalGaugesRequest{})
s.Require().NoError(err)
s.Require().Len(res.GetGauges(), 5)
missingGauges := checkContainsGauges(res.GetGauges(), []uint64{1, 2, 3, 4, 7})
s.Require().Empty(missingGauges, "missing gauges %v", missingGauges)
})
s.Run("ByPoolID", func() {
res, err := s.querier.GaugesByPoolID(s.Ctx, &types.QueryGaugesByPoolIDRequest{
Id: concentratedPoolId,
Pagination: &query.PageRequest{
Limit: 10,
},
})
s.Require().NoError(err)
s.Require().Len(res.Gauges, 4)
missingGauges := checkContainsGauges(res.GetGauges(), []uint64{4, 5, 6, 7})
s.Require().Empty(missingGauges, "missing gauges %v", missingGauges)
res, err = s.querier.GaugesByPoolID(s.Ctx, &types.QueryGaugesByPoolIDRequest{
Id: balancerPoolId,
})
s.Require().NoError(err)
s.Require().Len(res.Gauges, 3)
missingGauges = checkContainsGauges(res.GetGauges(), []uint64{1, 2, 3})
s.Require().Empty(missingGauges, "missing gauges %v", missingGauges)
})
}

// TestGRPCGauges tests querying upcoming and active gauges via gRPC returns the correct response.
func (s *KeeperTestSuite) TestGRPCGauges() {
s.SetupTest()
Expand Down
Loading