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

refactor(x/authz)!: Use KVStoreService, context.Context and return errors instead of panic #15962

Merged
merged 6 commits into from
Apr 27, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* (x/authz) [#15962](https://github.com/cosmos/cosmos-sdk/issues/15962) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey`, methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context`. The `Authorization` interface's `Accept` method now takes a `context.Context` instead of a `sdk.Context`.
* (x/distribution) [#15948](https://github.com/cosmos/cosmos-sdk/issues/15948) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey` and methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context`. Keeper methods also now return an `error`.
* (x/bank) [#15891](https://github.com/cosmos/cosmos-sdk/issues/15891) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey` and methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context`. Also `FundAccount` and `FundModuleAccount` from the `testutil` package accept a `context.Context` instead of a `sdk.Context`, and it's position was moved to the first place.
* (x/bank) [#15818](https://github.com/cosmos/cosmos-sdk/issues/15818) `BaseViewKeeper`'s `Logger` method now doesn't require a context. `NewBaseKeeper`, `NewBaseSendKeeper` and `NewBaseViewKeeper` now also require a `log.Logger` to be passed in.
Expand Down
3 changes: 2 additions & 1 deletion UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ This is no longer the case, the assertion has been loosened to only require modu
The following modules `NewKeeper` function now take a `KVStoreService` instead of a `StoreKey`:

* `x/auth`
* `x/authz`
* `x/bank`
* `x/consensus`
* `x/distribution`
Expand All @@ -94,10 +95,10 @@ The following modules `NewKeeper` function now also take a `log.Logger`:

The following modules' `Keeper` methods now take in a `context.Context` instead of `sdk.Context`. Any module that has an interfaces for them (like "expected keepers") will need to update and re-generate mocks if needed:

* `x/authz`
* `x/bank`
* `x/distribution`


### depinject

For `depinject` users, now the logger must be supplied through the main `depinject.Inject` function instead of passing it to `appBuilder.Build`.
Expand Down
2 changes: 1 addition & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ func NewSimApp(
stakingtypes.NewMultiStakingHooks(app.DistrKeeper.Hooks(), app.SlashingKeeper.Hooks()),
)

app.AuthzKeeper = authzkeeper.NewKeeper(keys[authzkeeper.StoreKey], appCodec, app.MsgServiceRouter(), app.AccountKeeper)
app.AuthzKeeper = authzkeeper.NewKeeper(runtime.NewKVStoreService(keys[authzkeeper.StoreKey]), appCodec, app.MsgServiceRouter(), app.AccountKeeper)

groupConfig := group.DefaultConfig()
/*
Expand Down
4 changes: 3 additions & 1 deletion x/authz/authorizations.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package authz

import (
context "context"

"github.com/cosmos/gogoproto/proto"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -17,7 +19,7 @@ type Authorization interface {

// Accept determines whether this grant permits the provided sdk.Msg to be performed,
// and if so provides an upgraded authorization instance.
Accept(ctx sdk.Context, msg sdk.Msg) (AcceptResponse, error)
Accept(ctx context.Context, msg sdk.Msg) (AcceptResponse, error)

// ValidateBasic does a simple validation check that
// doesn't require access to any other information.
Expand Down
4 changes: 3 additions & 1 deletion x/authz/generic_authorization.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package authz

import (
context "context"

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

Expand All @@ -19,7 +21,7 @@ func (a GenericAuthorization) MsgTypeURL() string {
}

// Accept implements Authorization.Accept.
func (a GenericAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (AcceptResponse, error) {
func (a GenericAuthorization) Accept(ctx context.Context, msg sdk.Msg) (AcceptResponse, error) {
return AcceptResponse{Accept: true}, nil
}

Expand Down
4 changes: 3 additions & 1 deletion x/authz/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil"
sdk "github.com/cosmos/cosmos-sdk/types"
moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
Expand Down Expand Up @@ -42,6 +43,7 @@ type GenesisTestSuite struct {

func (suite *GenesisTestSuite) SetupTest() {
key := storetypes.NewKVStoreKey(keeper.StoreKey)
storeService := runtime.NewKVStoreService(key)
testCtx := testutil.DefaultContextWithDB(suite.T(), key, storetypes.NewTransientStoreKey("transient_test"))
suite.ctx = testCtx.Ctx.WithBlockHeader(cmtproto.Header{Height: 1})
suite.encCfg = moduletestutil.MakeTestEncodingConfig(authzmodule.AppModuleBasic{})
Expand All @@ -68,7 +70,7 @@ func (suite *GenesisTestSuite) SetupTest() {
msr := suite.baseApp.MsgServiceRouter()
msr.SetInterfaceRegistry(suite.encCfg.InterfaceRegistry)

suite.keeper = keeper.NewKeeper(key, suite.encCfg.Codec, msr, suite.accountKeeper)
suite.keeper = keeper.NewKeeper(storeService, suite.encCfg.Codec, msr, suite.accountKeeper)
}

func (suite *GenesisTestSuite) TestImportExportGenesis() {
Expand Down
17 changes: 7 additions & 10 deletions x/authz/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"cosmossdk.io/store/prefix"

codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/types/query"
"github.com/cosmos/cosmos-sdk/x/authz"
)
Expand All @@ -20,7 +20,7 @@ var _ authz.QueryServer = Keeper{}

// Grants implements the Query/Grants gRPC method.
// It returns grants for a granter-grantee pair. If msg type URL is set, it returns grants only for that msg type.
func (k Keeper) Grants(c context.Context, req *authz.QueryGrantsRequest) (*authz.QueryGrantsResponse, error) {
func (k Keeper) Grants(ctx context.Context, req *authz.QueryGrantsRequest) (*authz.QueryGrantsResponse, error) {
if req == nil {
return nil, status.Errorf(codes.InvalidArgument, "empty request")
}
Expand All @@ -35,7 +35,6 @@ func (k Keeper) Grants(c context.Context, req *authz.QueryGrantsRequest) (*authz
return nil, err
}

ctx := sdk.UnwrapSDKContext(c)
if req.MsgTypeUrl != "" {
grant, found := k.getGrant(ctx, grantStoreKey(grantee, granter, req.MsgTypeUrl))
if !found {
Expand All @@ -59,7 +58,7 @@ func (k Keeper) Grants(c context.Context, req *authz.QueryGrantsRequest) (*authz
}, nil
}

store := ctx.KVStore(k.storeKey)
store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx))
key := grantStoreKey(grantee, granter, "")
grantsStore := prefix.NewStore(store, key)

Expand Down Expand Up @@ -91,7 +90,7 @@ func (k Keeper) Grants(c context.Context, req *authz.QueryGrantsRequest) (*authz
}

// GranterGrants implements the Query/GranterGrants gRPC method.
func (k Keeper) GranterGrants(c context.Context, req *authz.QueryGranterGrantsRequest) (*authz.QueryGranterGrantsResponse, error) {
func (k Keeper) GranterGrants(ctx context.Context, req *authz.QueryGranterGrantsRequest) (*authz.QueryGranterGrantsResponse, error) {
if req == nil {
return nil, status.Errorf(codes.InvalidArgument, "empty request")
}
Expand All @@ -101,8 +100,7 @@ func (k Keeper) GranterGrants(c context.Context, req *authz.QueryGranterGrantsRe
return nil, err
}

ctx := sdk.UnwrapSDKContext(c)
store := ctx.KVStore(k.storeKey)
store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx))
authzStore := prefix.NewStore(store, grantStoreKey(nil, granter, ""))

grants, pageRes, err := query.GenericFilteredPaginate(k.cdc, authzStore, req.Pagination, func(key []byte, auth *authz.Grant) (*authz.GrantAuthorization, error) {
Expand Down Expand Up @@ -137,7 +135,7 @@ func (k Keeper) GranterGrants(c context.Context, req *authz.QueryGranterGrantsRe
}

// GranteeGrants implements the Query/GranteeGrants gRPC method.
func (k Keeper) GranteeGrants(c context.Context, req *authz.QueryGranteeGrantsRequest) (*authz.QueryGranteeGrantsResponse, error) {
func (k Keeper) GranteeGrants(ctx context.Context, req *authz.QueryGranteeGrantsRequest) (*authz.QueryGranteeGrantsResponse, error) {
if req == nil {
return nil, status.Errorf(codes.InvalidArgument, "empty request")
}
Expand All @@ -147,8 +145,7 @@ func (k Keeper) GranteeGrants(c context.Context, req *authz.QueryGranteeGrantsRe
return nil, err
}

ctx := sdk.UnwrapSDKContext(c)
store := prefix.NewStore(ctx.KVStore(k.storeKey), GrantKey)
store := prefix.NewStore(runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)), GrantKey)

authorizations, pageRes, err := query.GenericFilteredPaginate(k.cdc, store, req.Pagination, func(key []byte, auth *authz.Grant) (*authz.GrantAuthorization, error) {
auth1, err := auth.GetAuthorization()
Expand Down
Loading