From 4c4f657475ccc48b7cc5a0f58620d224b043f7a9 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 28 Jun 2022 12:42:01 -0400 Subject: [PATCH] feat: add query.GenericFilteredPaginated (backport #12253) (#12371) --- .github/workflows/lint.yml | 2 +- .github/workflows/sims.yml | 14 +-- .github/workflows/test.yml | 12 +-- CHANGELOG.md | 5 + README.md | 2 +- contrib/images/simd-dlv/Dockerfile | 2 +- contrib/images/simd-env/Dockerfile | 2 +- go.mod | 2 +- types/query/filtered_pagination.go | 161 +++++++++++++++++++++++++++-- x/authz/keeper/grpc_query.go | 93 +++++++---------- x/authz/keeper/grpc_query_test.go | 12 ++- 11 files changed, 223 insertions(+), 84 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 9ae8d0e80be6..8c9423e2b0d5 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -16,7 +16,7 @@ jobs: steps: - uses: actions/setup-go@v2.1.4 with: - go-version: 1.17 + go-version: 1.18 - uses: technote-space/get-diff-action@v5 id: git_diff with: diff --git a/.github/workflows/sims.yml b/.github/workflows/sims.yml index d3f5fcd45b30..7c5de4383b3e 100644 --- a/.github/workflows/sims.yml +++ b/.github/workflows/sims.yml @@ -21,7 +21,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: 1.17 + go-version: 1.18 - name: Display go version run: go version - run: make build @@ -32,11 +32,11 @@ jobs: steps: - uses: actions/setup-go@v2.1.3 with: - go-version: 1.17 + go-version: 1.18 - name: Display go version run: go version - name: Install runsim - run: export GO111MODULE="on" && go get github.com/cosmos/tools/cmd/runsim@v1.0.0 + run: export GO111MODULE="on" && go install github.com/cosmos/tools/cmd/runsim@v1.0.0 - uses: actions/cache@v2.1.6 with: path: ~/go/bin @@ -49,7 +49,7 @@ jobs: - uses: actions/checkout@v2 - uses: actions/setup-go@v2.1.3 with: - go-version: 1.17 + go-version: 1.18 - name: Display go version run: go version - uses: technote-space/get-diff-action@v4 @@ -76,7 +76,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: 1.17 + go-version: 1.18 - name: Display go version run: go version - uses: technote-space/get-diff-action@v4 @@ -104,7 +104,7 @@ jobs: - uses: actions/checkout@v2 - uses: actions/setup-go@v2.1.3 with: - go-version: 1.17 + go-version: 1.18 - name: Display go version run: go version - uses: technote-space/get-diff-action@v4 @@ -123,7 +123,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: 1.17 + go-version: 1.18 - name: Display go version run: go version - uses: technote-space/get-diff-action@v4 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index aeacb1ecd7cd..d754c3317ae9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,7 +20,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: 1.17 + go-version: 1.18 - name: Display go version run: go version - name: install tparse @@ -42,7 +42,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: 1.17 + go-version: 1.18 - uses: technote-space/get-diff-action@v4 id: git_diff with: @@ -73,7 +73,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: 1.17 + go-version: 1.18 - name: Display go version run: go version - uses: technote-space/get-diff-action@v4 @@ -133,7 +133,7 @@ jobs: steps: - uses: actions/checkout@v4 with: - go-version: 1.17 + go-version: 1.18 - uses: technote-space/get-diff-action@v4 with: PATTERNS: | @@ -525,7 +525,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: 1.17 + go-version: 1.18 - uses: technote-space/get-diff-action@v4 with: PATTERNS: | @@ -914,7 +914,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: 1.17 + go-version: 1.18 - uses: technote-space/get-diff-action@v4 id: git_diff with: diff --git a/CHANGELOG.md b/CHANGELOG.md index a569408decdc..c20c8d007966 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements * (simapp) [#12314](https://github.com/cosmos/cosmos-sdk/pull/12314) Increase `DefaultGenTxGas` from `1000000` to `10000000` +* [#12371](https://github.com/cosmos/cosmos-sdk/pull/12371) Update min required Golang version to 1.18. ### Bug Fixes @@ -50,6 +51,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/upgrade) [#12264](https://github.com/cosmos/cosmos-sdk/pull/12264) Fix `GetLastCompleteUpgrade` to properly return the latest upgrade. * (x/crisis) [#12208](https://github.com/cosmos/cosmos-sdk/pull/12208) Fix progress index of crisis invariant assertion logs. +### Features + +* (query) [#12253](https://github.com/cosmos/cosmos-sdk/pull/12253) Add `GenericFilteredPaginate` to the `query` package to improve UX. + ## [v0.45.5](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.45.5) - 2022-06-09 ### Improvements diff --git a/README.md b/README.md index 43ccc56cc255..b41c08b845c4 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ The Cosmos SDK is a framework for building blockchain applications. [Tendermint **WARNING**: The Cosmos SDK has mostly stabilized, but we are still making some breaking changes. -**Note**: Requires [Go 1.17+](https://golang.org/dl/) +**Note**: Requires [Go 1.18+](https://golang.org/dl/) ## Quick Start diff --git a/contrib/images/simd-dlv/Dockerfile b/contrib/images/simd-dlv/Dockerfile index 9d15d621d761..f55463b26e66 100644 --- a/contrib/images/simd-dlv/Dockerfile +++ b/contrib/images/simd-dlv/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.17-alpine AS build +FROM golang:1.18-alpine AS build RUN apk add build-base git linux-headers libc-dev RUN go install github.com/go-delve/delve/cmd/dlv@latest WORKDIR /work diff --git a/contrib/images/simd-env/Dockerfile b/contrib/images/simd-env/Dockerfile index f00d0e582238..78e393f3ec62 100644 --- a/contrib/images/simd-env/Dockerfile +++ b/contrib/images/simd-env/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.17-alpine AS build +FROM golang:1.18-alpine AS build RUN apk add build-base git linux-headers WORKDIR /work COPY go.mod go.sum /work/ diff --git a/go.mod b/go.mod index 3d24615184eb..3a792e16e623 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -go 1.17 +go 1.18 module github.com/cosmos/cosmos-sdk diff --git a/types/query/filtered_pagination.go b/types/query/filtered_pagination.go index 1eafe6321b33..ff4cb18b7411 100644 --- a/types/query/filtered_pagination.go +++ b/types/query/filtered_pagination.go @@ -3,11 +3,8 @@ package query import ( "errors" - "github.com/cosmos/gogoproto/proto" - - "cosmossdk.io/store/types" - "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/store/types" ) // FilteredPaginate does pagination of all the results in the PrefixStore based on the @@ -35,8 +32,18 @@ func FilteredPaginate( err error ) - iterator := getIterator(prefixStore, pageRequest.Key, pageRequest.Reverse) - defer iterator.Close() + // 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 + nextKey []byte + ) if len(pageRequest.Key) != 0 { accumulateFn := func(_ uint64) bool { return true } @@ -76,10 +83,10 @@ func FilteredPaginate( } } - res := &PageResponse{NextKey: nextKey} - if pageRequest.CountTotal { - res.Total = numHits - } + var ( + numHits uint64 + nextKey []byte + ) return res, nil } @@ -209,3 +216,137 @@ func GenericFilteredPaginate[T, F proto.Message]( return results, res, nil } + +// GenericFilteredPaginate does pagination of all the results in the PrefixStore based on the +// provided PageRequest. `onResult` should be used to filter or transform the results. +// `c` is a constructor function that needs to return a new instance of the type T (this is to +// workaround some generic pitfalls in which we can't instantiate a T struct inside the function). +// If key is provided, the pagination uses the optimized querying. +// If offset is used, the pagination uses lazy filtering i.e., searches through all the records. +// The resulting slice (of type F) can be of a different type than the one being iterated through +// (type T), so it's possible to do any necessary transformation inside the onResult function. +func GenericFilteredPaginate[T codec.ProtoMarshaler, F codec.ProtoMarshaler]( + cdc codec.BinaryCodec, + prefixStore types.KVStore, + pageRequest *PageRequest, + onResult func(key []byte, value T) (F, error), + constructor func() T, +) ([]F, *PageResponse, error) { + // if the PageRequest is nil, use default PageRequest + if pageRequest == nil { + pageRequest = &PageRequest{} + } + + 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 + nextKey []byte + ) + + for ; iterator.Valid(); iterator.Next() { + if numHits == limit { + nextKey = iterator.Key() + break + } + + if iterator.Error() != nil { + return nil, nil, iterator.Error() + } + + protoMsg := constructor() + + err := cdc.Unmarshal(iterator.Value(), protoMsg) + if err != nil { + return nil, nil, err + } + + val, err := onResult(iterator.Key(), protoMsg) + 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 + nextKey []byte + ) + + for ; iterator.Valid(); iterator.Next() { + if iterator.Error() != nil { + return nil, nil, iterator.Error() + } + + protoMsg := constructor() + + err := cdc.Unmarshal(iterator.Value(), protoMsg) + if err != nil { + return nil, nil, err + } + + val, err := onResult(iterator.Key(), protoMsg) + 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 +} diff --git a/x/authz/keeper/grpc_query.go b/x/authz/keeper/grpc_query.go index 3eebab1bec17..f2d9d438c181 100644 --- a/x/authz/keeper/grpc_query.go +++ b/x/authz/keeper/grpc_query.go @@ -7,10 +7,6 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - "cosmossdk.io/errors" - "cosmossdk.io/store/prefix" - "cosmossdk.io/x/authz" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/runtime" "github.com/cosmos/cosmos-sdk/types/query" @@ -34,6 +30,7 @@ func (k Keeper) Grants(ctx context.Context, req *authz.QueryGrantsRequest) (*aut if err != nil { return nil, err } + ctx := sdk.UnwrapSDKContext(c) if req.MsgTypeUrl != "" { grant, found := k.getGrant(ctx, grantStoreKey(grantee, granter, req.MsgTypeUrl)) @@ -58,19 +55,19 @@ func (k Keeper) Grants(ctx context.Context, req *authz.QueryGrantsRequest) (*aut }, nil } - store := runtime.KVStoreAdapter(k.KVStoreService.OpenKVStore(ctx)) + store := ctx.KVStore(k.storeKey) key := grantStoreKey(grantee, granter, "") grantsStore := prefix.NewStore(store, key) authorizations, pageRes, err := query.GenericFilteredPaginate(k.cdc, grantsStore, req.Pagination, func(key []byte, auth *authz.Grant) (*authz.Grant, error) { - auth1, err := auth.GetAuthorization() + auth1 := auth.GetAuthorization() if err != nil { return nil, err } authorizationAny, err := codectypes.NewAnyWithValue(auth1) if err != nil { - return nil, status.Error(codes.Internal, err.Error()) + return nil, status.Errorf(codes.Internal, err.Error()) } return &authz.Grant{ Authorization: authorizationAny, @@ -104,32 +101,29 @@ 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 := auth.GetAuthorization() if err != nil { - return false, err + return nil, err } - auth1 := auth.GetAuthorization() - 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, - }) + any, err := codectypes.NewAnyWithValue(auth1) + if err != nil { + return nil, status.Errorf(codes.Internal, err.Error()) } - return true, nil + + grantee := firstAddressFromGrantStoreKey(key) + return &authz.GrantAuthorization{ + Granter: granter.String(), + Grantee: grantee.String(), + Authorization: any, + Expiration: auth.Expiration, + }, nil + + }, func() *authz.Grant { + return &authz.Grant{} }) + if err != nil { return nil, err } @@ -154,35 +148,30 @@ 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 := auth.GetAuthorization() if err != nil { - return false, err + return nil, err } granter, g := addressesFromGrantStoreKey(append(GrantKey, key...)) if !g.Equals(grantee) { - return false, nil + return nil, nil } - auth1 := auth.GetAuthorization() - 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(), - }) + authorizationAny, err := codectypes.NewAnyWithValue(auth1) + if err != nil { + 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 + }, func() *authz.Grant { + return &authz.Grant{} }) if err != nil { return nil, err @@ -193,9 +182,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 -} diff --git a/x/authz/keeper/grpc_query_test.go b/x/authz/keeper/grpc_query_test.go index 467a88b84134..1e7156144234 100644 --- a/x/authz/keeper/grpc_query_test.go +++ b/x/authz/keeper/grpc_query_test.go @@ -339,7 +339,8 @@ func (suite *TestSuite) TestGRPCQueryGranterGrants() { }, { "valid case, pagination", - func() {}, + func() { + }, false, authz.QueryGranterGrantsRequest{ Granter: addrs[0].String(), @@ -398,6 +399,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() {