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

feat: add query.GenericFilteredPaginated #12253

Merged
merged 25 commits into from
Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
fa3014e
poc
facundomedica Jun 14, 2022
619f716
Merge branch 'main' into facu/filteredPaginated-generics
facundomedica Jun 14, 2022
a920332
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Jun 14, 2022
cb28171
progress
facundomedica Jun 14, 2022
b5b5a64
Merge branch 'facu/filteredPaginated-generics' of https://github.com/…
facundomedica Jun 14, 2022
3dd87d2
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Jun 14, 2022
4ebaaaa
fix when returned object is nil
facundomedica Jun 14, 2022
9eeb401
Merge branch 'main' into facu/filteredPaginated-generics
facundomedica Jun 14, 2022
6db3de3
use reflect to accept codec.ProtoMarshaler
facundomedica Jun 15, 2022
85fc29b
suggestion
facundomedica Jun 15, 2022
6353df0
try out constructor
facundomedica Jun 15, 2022
23b5fdc
rename
facundomedica Jun 15, 2022
fff311f
Merge branch 'main' into facu/filteredPaginated-generics
facundomedica Jun 15, 2022
bf01daa
docs
facundomedica Jun 16, 2022
1cf2af3
Merge branch 'facu/filteredPaginated-generics' of https://github.com/…
facundomedica Jun 16, 2022
314d622
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Jun 16, 2022
2c59802
docs
facundomedica Jun 16, 2022
a93666b
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into facu…
facundomedica Jun 27, 2022
40cd7b6
Apply suggestions from code review
facundomedica Jun 27, 2022
38f74ad
Merge branch 'facu/filteredPaginated-generics' of https://github.com/…
facundomedica Jun 27, 2022
61311bd
Merge branch 'main' into facu/filteredPaginated-generics
facundomedica Jun 27, 2022
51ca9b5
Merge branch 'facu/filteredPaginated-generics' of https://github.com/…
facundomedica Jun 27, 2022
baf7f7f
cl++, small fix
facundomedica Jun 27, 2022
b0070c2
cl++
facundomedica Jun 27, 2022
7e955a9
Merge branch 'main' into facu/filteredPaginated-generics
facundomedica Jun 27, 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
120 changes: 120 additions & 0 deletions types/query/filtered_pagination.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package query
import (
"fmt"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/store/types"
)

Expand Down Expand Up @@ -114,3 +115,122 @@ func FilteredPaginate(

return res, nil
}

func GenericFilteredPaginate[V any, F codec.ProtoMarshaler](
cdc codec.BinaryCodec,
prefixStore types.KVStore,
pageRequest *PageRequest,
onResult func(key []byte, value V) (F, error),
) ([]F, *PageResponse, error) {
// if the PageRequest is nil, use default PageRequest
if pageRequest == nil {
pageRequest = &PageRequest{}
}
facundomedica marked this conversation as resolved.
Show resolved Hide resolved

offset := pageRequest.Offset
key := pageRequest.Key
limit := pageRequest.Limit
countTotal := pageRequest.CountTotal
reverse := pageRequest.Reverse
results := []F{}

if offset > 0 && key != nil {
return results, nil, fmt.Errorf("invalid request, either offset or key is expected, got both")
}

if limit == 0 {
limit = DefaultLimit

// count total results when the limit is zero/not supplied
countTotal = true
}

if len(key) != 0 {
iterator := getIterator(prefixStore, key, reverse)
defer iterator.Close()

var numHits uint64
var nextKey []byte
facundomedica marked this conversation as resolved.
Show resolved Hide resolved

for ; iterator.Valid(); iterator.Next() {
if numHits == limit {
nextKey = iterator.Key()
break
}

if iterator.Error() != nil {
return nil, nil, iterator.Error()
}

protoMsg := any(new(V))
err := cdc.Unmarshal(iterator.Value(), protoMsg.(codec.ProtoMarshaler))
Copy link
Contributor

Choose a reason for hiding this comment

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

so V is an any, which is just an interface{}. Do we not need to type check here that V is a codec.ProtoMarshaler? Why not also make V of type codec.ProtoMarshaler?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was trying to find a way of instantiating V (now T; not sure what to name these), which for some reason was impossible for me (always ended up getting **authz.Grant instead of *authz.Grant without the possibility to dereference it). With reflectit was quite easy, but I'm not sure if we would like to use it here. See my last commit: 6db3de3

Copy link
Member Author

Choose a reason for hiding this comment

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

This was what I was trying to do before:

protoMsg := any(new(T)).(codec.ProtoMarshaler)
protoMsg := any(*new(T)).(codec.ProtoMarshaler)

None of these work... In short, we need to be able to initialize a variable with type T that is not nil. Is this possible with generics?

Copy link
Contributor

Choose a reason for hiding this comment

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

In Osmosis, I made V of type proto.Message for something similar here; https://github.com/osmosis-labs/osmosis/blob/main/tests/e2e/chain/config.go#L149-L162

Copy link
Member Author

Choose a reason for hiding this comment

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

codec.BinaryCodec calls for codec.ProtoMarshaler. It shouldn't be an issue, it seems like all of our generated protobuf structs are codec.ProtoMarshaler.
The issue we were discussing here has been solved by adding a constructor function.

if err != nil {
return nil, nil, err
}

val, err := onResult(iterator.Key(), *(protoMsg.(*V)))
if err != nil {
return nil, nil, err
}

if val.Size() != 0 {
results = append(results, val)
numHits++
}
}

return results, &PageResponse{
NextKey: nextKey,
}, nil
}

iterator := getIterator(prefixStore, nil, reverse)
defer iterator.Close()

end := offset + limit

var numHits uint64
var nextKey []byte

for ; iterator.Valid(); iterator.Next() {
if iterator.Error() != nil {
return nil, nil, iterator.Error()
}

protoMsg := any(new(V))
err := cdc.Unmarshal(iterator.Value(), protoMsg.(codec.ProtoMarshaler))
if err != nil {
return nil, nil, err
}

val, err := onResult(iterator.Key(), *(protoMsg.(*V)))
if err != nil {
return nil, nil, err
}

if val.Size() != 0 {
// Previously this was the "accumulate" flag
if numHits >= offset && numHits < end {
results = append(results, val)
}
numHits++
}

if numHits == end+1 {
if nextKey == nil {
nextKey = iterator.Key()
}

if !countTotal {
break
}
}
}

res := &PageResponse{NextKey: nextKey}
if countTotal {
res.Total = numHits
}

return results, res, nil
}
110 changes: 36 additions & 74 deletions x/authz/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ import (
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

proto "github.com/gogo/protobuf/proto"

"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/store/prefix"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -64,34 +61,20 @@ func (k Keeper) Grants(c context.Context, req *authz.QueryGrantsRequest) (*authz
key := grantStoreKey(grantee, granter, "")
grantsStore := prefix.NewStore(store, key)

var authorizations []*authz.Grant
pageRes, err := query.FilteredPaginate(grantsStore, req.Pagination, func(key []byte, value []byte, accumulate bool) (bool, error) {
auth, err := unmarshalAuthorization(k.cdc, value)
if err != nil {
return false, err
}

authorizations, pageRes, err := query.GenericFilteredPaginate(k.cdc, grantsStore, req.Pagination, func(key []byte, auth authz.Grant) (*authz.Grant, error) {
auth1, err := auth.GetAuthorization()
if err != nil {
return false, err
return nil, err
}

if accumulate {
msg, ok := auth1.(proto.Message)
if !ok {
return false, status.Errorf(codes.Internal, "can't protomarshal %T", msg)
}

authorizationAny, err := codectypes.NewAnyWithValue(msg)
if err != nil {
return false, status.Errorf(codes.Internal, err.Error())
}
authorizations = append(authorizations, &authz.Grant{
Authorization: authorizationAny,
Expiration: auth.Expiration,
})
authorizationAny, err := codectypes.NewAnyWithValue(auth1)
if err != nil {
return nil, status.Errorf(codes.Internal, err.Error())
}
return true, nil
return &authz.Grant{
Authorization: authorizationAny,
Expiration: auth.Expiration,
}, nil
})
if err != nil {
return nil, err
Expand All @@ -118,34 +101,27 @@ func (k Keeper) GranterGrants(c context.Context, req *authz.QueryGranterGrantsRe
store := ctx.KVStore(k.storeKey)
authzStore := prefix.NewStore(store, grantStoreKey(nil, granter, ""))

var grants []*authz.GrantAuthorization
pageRes, err := query.FilteredPaginate(authzStore, req.Pagination, func(key []byte, value []byte, accumulate bool) (bool, error) {
auth, err := unmarshalAuthorization(k.cdc, value)
grants, pageRes, err := query.GenericFilteredPaginate(k.cdc, authzStore, req.Pagination, func(key []byte, auth authz.Grant) (*authz.GrantAuthorization, error) {
auth1, err := auth.GetAuthorization()
if err != nil {
return false, err
return nil, err
}

auth1, err := auth.GetAuthorization()
any, err := codectypes.NewAnyWithValue(auth1)
if err != nil {
return false, err
return nil, status.Errorf(codes.Internal, err.Error())
}

if accumulate {
any, err := codectypes.NewAnyWithValue(auth1)
if err != nil {
return false, status.Errorf(codes.Internal, err.Error())
}

grantee := firstAddressFromGrantStoreKey(key)
grants = append(grants, &authz.GrantAuthorization{
Granter: granter.String(),
Grantee: grantee.String(),
Authorization: any,
Expiration: auth.Expiration,
})
}
return true, nil
grantee := firstAddressFromGrantStoreKey(key)
return &authz.GrantAuthorization{
Granter: granter.String(),
Grantee: grantee.String(),
Authorization: any,
Expiration: auth.Expiration,
}, nil

})

if err != nil {
return nil, err
}
Expand All @@ -170,36 +146,28 @@ func (k Keeper) GranteeGrants(c context.Context, req *authz.QueryGranteeGrantsRe
ctx := sdk.UnwrapSDKContext(c)
store := prefix.NewStore(ctx.KVStore(k.storeKey), GrantKey)

var authorizations []*authz.GrantAuthorization
pageRes, err := query.FilteredPaginate(store, req.Pagination, func(key []byte, value []byte, accumulate bool) (bool, error) {
auth, err := unmarshalAuthorization(k.cdc, value)
authorizations, pageRes, err := query.GenericFilteredPaginate(k.cdc, store, req.Pagination, func(key []byte, auth authz.Grant) (*authz.GrantAuthorization, error) {
auth1, err := auth.GetAuthorization()
if err != nil {
return false, err
return nil, err
}

granter, g, _ := parseGrantStoreKey(append(GrantKey, key...))
if !g.Equals(grantee) {
return false, nil
return nil, nil
}

auth1, err := auth.GetAuthorization()
authorizationAny, err := codectypes.NewAnyWithValue(auth1)
if err != nil {
return false, err
}
if accumulate {
any, err := codectypes.NewAnyWithValue(auth1)
if err != nil {
return false, status.Errorf(codes.Internal, err.Error())
}

authorizations = append(authorizations, &authz.GrantAuthorization{
Authorization: any,
Expiration: auth.Expiration,
Granter: granter.String(),
Grantee: grantee.String(),
})
return nil, status.Errorf(codes.Internal, err.Error())
}
return true, nil

return &authz.GrantAuthorization{
Authorization: authorizationAny,
Expiration: auth.Expiration,
Granter: granter.String(),
Grantee: grantee.String(),
}, nil
})
if err != nil {
return nil, err
Expand All @@ -210,9 +178,3 @@ func (k Keeper) GranteeGrants(c context.Context, req *authz.QueryGranteeGrantsRe
Pagination: pageRes,
}, nil
}

// unmarshal an authorization from a store value
func unmarshalAuthorization(cdc codec.BinaryCodec, value []byte) (v authz.Grant, err error) {
err = cdc.Unmarshal(value, &v)
return v, err
}
12 changes: 11 additions & 1 deletion x/authz/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ func (suite *TestSuite) TestGRPCQueryGranterGrants() {
},
{
"valid case, pagination",
func() {},
func() {
},
false,
authz.QueryGranterGrantsRequest{
Granter: addrs[0].String(),
Expand Down Expand Up @@ -253,6 +254,15 @@ func (suite *TestSuite) TestGRPCQueryGranteeGrants() {
},
1,
},
{
"valid case, no authorization found",
func() {},
false,
authz.QueryGranteeGrantsRequest{
Grantee: addrs[2].String(),
},
0,
},
{
"valid case, multiple authorization",
func() {
Expand Down