From 91278f6602ebac9534fc19c2d803d23c9df78bac Mon Sep 17 00:00:00 2001 From: Facundo Medica <14063057+facundomedica@users.noreply.github.com> Date: Thu, 27 Apr 2023 12:15:45 -0300 Subject: [PATCH] refactor(x/authz)!: Use KVStoreService, context.Context and return errors instead of panic (#15962) --- CHANGELOG.md | 1 + UPGRADING.md | 3 +- simapp/app.go | 2 +- x/authz/authorizations.go | 4 +- x/authz/generic_authorization.go | 4 +- x/authz/keeper/genesis_test.go | 4 +- x/authz/keeper/grpc_query.go | 17 ++- x/authz/keeper/keeper.go | 162 +++++++++++++++++----------- x/authz/keeper/keeper_test.go | 4 +- x/authz/keeper/migrations.go | 2 +- x/authz/migrations/v2/store.go | 11 +- x/authz/migrations/v2/store_test.go | 20 +++- x/authz/module/abci_test.go | 4 +- x/authz/module/module.go | 6 +- x/bank/types/send_authorization.go | 7 +- x/staking/types/authz.go | 9 +- 16 files changed, 161 insertions(+), 99 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87a5726f695e..6eb06693e1ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -118,6 +118,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. diff --git a/UPGRADING.md b/UPGRADING.md index 92272976a8ba..ff41503e2e37 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -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` @@ -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`. diff --git a/simapp/app.go b/simapp/app.go index 6b68a75cc366..500b4ad33a4c 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -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() /* diff --git a/x/authz/authorizations.go b/x/authz/authorizations.go index 2df63a7d3404..75b2e0659097 100644 --- a/x/authz/authorizations.go +++ b/x/authz/authorizations.go @@ -1,6 +1,8 @@ package authz import ( + context "context" + "github.com/cosmos/gogoproto/proto" sdk "github.com/cosmos/cosmos-sdk/types" @@ -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. diff --git a/x/authz/generic_authorization.go b/x/authz/generic_authorization.go index 23853d250a7a..688211a6928d 100644 --- a/x/authz/generic_authorization.go +++ b/x/authz/generic_authorization.go @@ -1,6 +1,8 @@ package authz import ( + context "context" + sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -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 } diff --git a/x/authz/keeper/genesis_test.go b/x/authz/keeper/genesis_test.go index 52166cafbe82..99f901a72062 100644 --- a/x/authz/keeper/genesis_test.go +++ b/x/authz/keeper/genesis_test.go @@ -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" @@ -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{}) @@ -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() { diff --git a/x/authz/keeper/grpc_query.go b/x/authz/keeper/grpc_query.go index cb19e5760e3f..5278d5874b16 100644 --- a/x/authz/keeper/grpc_query.go +++ b/x/authz/keeper/grpc_query.go @@ -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" ) @@ -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") } @@ -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 { @@ -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) @@ -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") } @@ -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) { @@ -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") } @@ -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() diff --git a/x/authz/keeper/keeper.go b/x/authz/keeper/keeper.go index 4ca846406048..0177927113b6 100644 --- a/x/authz/keeper/keeper.go +++ b/x/authz/keeper/keeper.go @@ -1,6 +1,7 @@ package keeper import ( + "context" "fmt" "strconv" "time" @@ -9,12 +10,14 @@ import ( abci "github.com/cometbft/cometbft/abci/types" "github.com/cosmos/gogoproto/proto" + corestoretypes "cosmossdk.io/core/store" errorsmod "cosmossdk.io/errors" storetypes "cosmossdk.io/store/types" "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" + "github.com/cosmos/cosmos-sdk/runtime" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/authz" @@ -26,31 +29,37 @@ import ( const gasCostPerIteration = uint64(20) type Keeper struct { - storeKey storetypes.StoreKey - cdc codec.BinaryCodec - router baseapp.MessageRouter - authKeeper authz.AccountKeeper + storeService corestoretypes.KVStoreService + cdc codec.BinaryCodec + router baseapp.MessageRouter + authKeeper authz.AccountKeeper } // NewKeeper constructs a message authorization Keeper -func NewKeeper(storeKey storetypes.StoreKey, cdc codec.BinaryCodec, router baseapp.MessageRouter, ak authz.AccountKeeper) Keeper { +func NewKeeper(storeService corestoretypes.KVStoreService, cdc codec.BinaryCodec, router baseapp.MessageRouter, ak authz.AccountKeeper) Keeper { return Keeper{ - storeKey: storeKey, - cdc: cdc, - router: router, - authKeeper: ak, + storeService: storeService, + cdc: cdc, + router: router, + authKeeper: ak, } } // Logger returns a module-specific logger. -func (k Keeper) Logger(ctx sdk.Context) log.Logger { - return ctx.Logger().With("module", fmt.Sprintf("x/%s", authz.ModuleName)) +func (k Keeper) Logger(ctx context.Context) log.Logger { + sdkCtx := sdk.UnwrapSDKContext(ctx) + return sdkCtx.Logger().With("module", fmt.Sprintf("x/%s", authz.ModuleName)) } // getGrant returns grant stored at skey. -func (k Keeper) getGrant(ctx sdk.Context, skey []byte) (grant authz.Grant, found bool) { - store := ctx.KVStore(k.storeKey) - bz := store.Get(skey) +func (k Keeper) getGrant(ctx context.Context, skey []byte) (grant authz.Grant, found bool) { + store := k.storeService.OpenKVStore(ctx) + + bz, err := store.Get(skey) + if err != nil { + panic(err) + } + if bz == nil { return grant, false } @@ -58,7 +67,7 @@ func (k Keeper) getGrant(ctx sdk.Context, skey []byte) (grant authz.Grant, found return grant, true } -func (k Keeper) update(ctx sdk.Context, grantee, granter sdk.AccAddress, updated authz.Authorization) error { +func (k Keeper) update(ctx context.Context, grantee, granter sdk.AccAddress, updated authz.Authorization) error { skey := grantStoreKey(grantee, granter, updated.MsgTypeURL()) grant, found := k.getGrant(ctx, skey) if !found { @@ -76,7 +85,7 @@ func (k Keeper) update(ctx sdk.Context, grantee, granter sdk.AccAddress, updated } grant.Authorization = any - store := ctx.KVStore(k.storeKey) + store := k.storeService.OpenKVStore(ctx) store.Set(skey, k.cdc.MustMarshal(&grant)) return nil @@ -84,9 +93,10 @@ func (k Keeper) update(ctx sdk.Context, grantee, granter sdk.AccAddress, updated // DispatchActions attempts to execute the provided messages via authorization // grants from the message signer to the grantee. -func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs []sdk.Msg) ([][]byte, error) { +func (k Keeper) DispatchActions(ctx context.Context, grantee sdk.AccAddress, msgs []sdk.Msg) ([][]byte, error) { results := make([][]byte, len(msgs)) - now := ctx.BlockTime() + sdkCtx := sdk.UnwrapSDKContext(ctx) + now := sdkCtx.BlockTime() for i, msg := range msgs { signers := msg.GetSigners() @@ -115,7 +125,7 @@ func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs [] return nil, err } - resp, err := authorization.Accept(ctx, msg) + resp, err := authorization.Accept(sdkCtx, msg) if err != nil { return nil, err } @@ -139,7 +149,7 @@ func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs [] return nil, sdkerrors.ErrUnknownRequest.Wrapf("unrecognized message route: %s", sdk.MsgTypeURL(msg)) } - msgResp, err := handler(ctx, msg) + msgResp, err := handler(sdkCtx, msg) if err != nil { return nil, errorsmod.Wrapf(err, "failed to execute message; message %v", msg) } @@ -156,7 +166,7 @@ func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs [] sdkEvents = append(sdkEvents, sdk.Event(e)) } - ctx.EventManager().EmitEvents(sdkEvents) + sdkCtx.EventManager().EmitEvents(sdkEvents) } return results, nil @@ -165,12 +175,13 @@ func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs [] // SaveGrant method grants the provided authorization to the grantee on the granter's account // with the provided expiration time and insert authorization key into the grants queue. If there is an existing authorization grant for the // same `sdk.Msg` type, this grant overwrites that. -func (k Keeper) SaveGrant(ctx sdk.Context, grantee, granter sdk.AccAddress, authorization authz.Authorization, expiration *time.Time) error { - store := ctx.KVStore(k.storeKey) +func (k Keeper) SaveGrant(ctx context.Context, grantee, granter sdk.AccAddress, authorization authz.Authorization, expiration *time.Time) error { + sdkCtx := sdk.UnwrapSDKContext(ctx) msgType := authorization.MsgTypeURL() + store := k.storeService.OpenKVStore(ctx) skey := grantStoreKey(grantee, granter, msgType) - grant, err := authz.NewGrant(ctx.BlockTime(), authorization, expiration) + grant, err := authz.NewGrant(sdkCtx.BlockTime(), authorization, expiration) if err != nil { return err } @@ -193,10 +204,17 @@ func (k Keeper) SaveGrant(ctx sdk.Context, grantee, granter sdk.AccAddress, auth } } - bz := k.cdc.MustMarshal(&grant) - store.Set(skey, bz) + bz, err := k.cdc.Marshal(&grant) + if err != nil { + return err + } + + err = store.Set(skey, bz) + if err != nil { + return err + } - return ctx.EventManager().EmitTypedEvent(&authz.EventGrant{ + return sdkCtx.EventManager().EmitTypedEvent(&authz.EventGrant{ MsgTypeUrl: authorization.MsgTypeURL(), Granter: granter.String(), Grantee: grantee.String(), @@ -205,8 +223,8 @@ func (k Keeper) SaveGrant(ctx sdk.Context, grantee, granter sdk.AccAddress, auth // DeleteGrant revokes any authorization for the provided message type granted to the grantee // by the granter. -func (k Keeper) DeleteGrant(ctx sdk.Context, grantee, granter sdk.AccAddress, msgType string) error { - store := ctx.KVStore(k.storeKey) +func (k Keeper) DeleteGrant(ctx context.Context, grantee, granter sdk.AccAddress, msgType string) error { + store := k.storeService.OpenKVStore(ctx) skey := grantStoreKey(grantee, granter, msgType) grant, found := k.getGrant(ctx, skey) if !found { @@ -220,9 +238,13 @@ func (k Keeper) DeleteGrant(ctx sdk.Context, grantee, granter sdk.AccAddress, ms } } - store.Delete(skey) + err := store.Delete(skey) + if err != nil { + return err + } - return ctx.EventManager().EmitTypedEvent(&authz.EventRevoke{ + sdkCtx := sdk.UnwrapSDKContext(ctx) + return sdkCtx.EventManager().EmitTypedEvent(&authz.EventRevoke{ MsgTypeUrl: msgType, Granter: granter.String(), Grantee: grantee.String(), @@ -230,8 +252,8 @@ func (k Keeper) DeleteGrant(ctx sdk.Context, grantee, granter sdk.AccAddress, ms } // GetAuthorizations Returns list of `Authorizations` granted to the grantee by the granter. -func (k Keeper) GetAuthorizations(ctx sdk.Context, grantee, granter sdk.AccAddress) ([]authz.Authorization, error) { - store := ctx.KVStore(k.storeKey) +func (k Keeper) GetAuthorizations(ctx context.Context, grantee, granter sdk.AccAddress) ([]authz.Authorization, error) { + store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)) key := grantStoreKey(grantee, granter, "") iter := storetypes.KVStorePrefixIterator(store, key) defer iter.Close() @@ -259,9 +281,10 @@ func (k Keeper) GetAuthorizations(ctx sdk.Context, grantee, granter sdk.AccAddre // - No grant is found. // - A grant is found, but it is expired. // - There was an error getting the authorization from the grant. -func (k Keeper) GetAuthorization(ctx sdk.Context, grantee, granter sdk.AccAddress, msgType string) (authz.Authorization, *time.Time) { +func (k Keeper) GetAuthorization(ctx context.Context, grantee, granter sdk.AccAddress, msgType string) (authz.Authorization, *time.Time) { + sdkCtx := sdk.UnwrapSDKContext(ctx) grant, found := k.getGrant(ctx, grantStoreKey(grantee, granter, msgType)) - if !found || (grant.Expiration != nil && grant.Expiration.Before(ctx.BlockHeader().Time)) { + if !found || (grant.Expiration != nil && grant.Expiration.Before(sdkCtx.BlockHeader().Time)) { return nil, nil } @@ -277,10 +300,10 @@ func (k Keeper) GetAuthorization(ctx sdk.Context, grantee, granter sdk.AccAddres // This function should be used with caution because it can involve significant IO operations. // It should not be used in query or msg services without charging additional gas. // The iteration stops when the handler function returns true or the iterator exhaust. -func (k Keeper) IterateGrants(ctx sdk.Context, +func (k Keeper) IterateGrants(ctx context.Context, handler func(granterAddr, granteeAddr sdk.AccAddress, grant authz.Grant) bool, ) { - store := ctx.KVStore(k.storeKey) + store := runtime.KVStoreAdapter(k.storeService.OpenKVStore(ctx)) iter := storetypes.KVStorePrefixIterator(store, GrantKey) defer iter.Close() for ; iter.Valid(); iter.Next() { @@ -293,9 +316,13 @@ func (k Keeper) IterateGrants(ctx sdk.Context, } } -func (k Keeper) getGrantQueueItem(ctx sdk.Context, expiration time.Time, granter, grantee sdk.AccAddress) (*authz.GrantQueueItem, error) { - store := ctx.KVStore(k.storeKey) - bz := store.Get(GrantQueueKey(expiration, granter, grantee)) +func (k Keeper) getGrantQueueItem(ctx context.Context, expiration time.Time, granter, grantee sdk.AccAddress) (*authz.GrantQueueItem, error) { + store := k.storeService.OpenKVStore(ctx) + bz, err := store.Get(GrantQueueKey(expiration, granter, grantee)) + if err != nil { + return nil, err + } + if bz == nil { return &authz.GrantQueueItem{}, nil } @@ -307,43 +334,37 @@ func (k Keeper) getGrantQueueItem(ctx sdk.Context, expiration time.Time, granter return &queueItems, nil } -func (k Keeper) setGrantQueueItem(ctx sdk.Context, expiration time.Time, +func (k Keeper) setGrantQueueItem(ctx context.Context, expiration time.Time, granter, grantee sdk.AccAddress, queueItems *authz.GrantQueueItem, ) error { - store := ctx.KVStore(k.storeKey) + store := k.storeService.OpenKVStore(ctx) bz, err := k.cdc.Marshal(queueItems) if err != nil { return err } - store.Set(GrantQueueKey(expiration, granter, grantee), bz) - - return nil + return store.Set(GrantQueueKey(expiration, granter, grantee), bz) } // insertIntoGrantQueue inserts a grant key into the grant queue -func (k Keeper) insertIntoGrantQueue(ctx sdk.Context, granter, grantee sdk.AccAddress, msgType string, expiration time.Time) error { +func (k Keeper) insertIntoGrantQueue(ctx context.Context, granter, grantee sdk.AccAddress, msgType string, expiration time.Time) error { queueItems, err := k.getGrantQueueItem(ctx, expiration, granter, grantee) if err != nil { return err } - if len(queueItems.MsgTypeUrls) == 0 { - k.setGrantQueueItem(ctx, expiration, granter, grantee, &authz.GrantQueueItem{ - MsgTypeUrls: []string{msgType}, - }) - } else { - queueItems.MsgTypeUrls = append(queueItems.MsgTypeUrls, msgType) - k.setGrantQueueItem(ctx, expiration, granter, grantee, queueItems) - } - - return nil + queueItems.MsgTypeUrls = append(queueItems.MsgTypeUrls, msgType) + return k.setGrantQueueItem(ctx, expiration, granter, grantee, queueItems) } // removeFromGrantQueue removes a grant key from the grant queue -func (k Keeper) removeFromGrantQueue(ctx sdk.Context, grantKey []byte, granter, grantee sdk.AccAddress, expiration time.Time) error { - store := ctx.KVStore(k.storeKey) +func (k Keeper) removeFromGrantQueue(ctx context.Context, grantKey []byte, granter, grantee sdk.AccAddress, expiration time.Time) error { + store := k.storeService.OpenKVStore(ctx) key := GrantQueueKey(expiration, granter, grantee) - bz := store.Get(key) + bz, err := store.Get(key) + if err != nil { + return err + } + if bz == nil { return errorsmod.Wrap(authz.ErrNoGrantKeyFound, "can't remove grant from the expire queue, grant key not found") } @@ -356,8 +377,9 @@ func (k Keeper) removeFromGrantQueue(ctx sdk.Context, grantKey []byte, granter, _, _, msgType := parseGrantStoreKey(grantKey) queueItems := queueItem.MsgTypeUrls + sdkCtx := sdk.UnwrapSDKContext(ctx) for index, typeURL := range queueItems { - ctx.GasMeter().ConsumeGas(gasCostPerIteration, "grant queue") + sdkCtx.GasMeter().ConsumeGas(gasCostPerIteration, "grant queue") if typeURL == msgType { end := len(queueItem.MsgTypeUrls) - 1 @@ -377,10 +399,14 @@ func (k Keeper) removeFromGrantQueue(ctx sdk.Context, grantKey []byte, granter, } // DequeueAndDeleteExpiredGrants deletes expired grants from the state and grant queue. -func (k Keeper) DequeueAndDeleteExpiredGrants(ctx sdk.Context) error { - store := ctx.KVStore(k.storeKey) +func (k Keeper) DequeueAndDeleteExpiredGrants(ctx context.Context) error { + store := k.storeService.OpenKVStore(ctx) + sdkCtx := sdk.UnwrapSDKContext(ctx) - iterator := store.Iterator(GrantQueuePrefix, storetypes.InclusiveEndBytes(GrantQueueTimePrefix(ctx.BlockTime()))) + iterator, err := store.Iterator(GrantQueuePrefix, storetypes.InclusiveEndBytes(GrantQueueTimePrefix(sdkCtx.BlockTime()))) + if err != nil { + return err + } defer iterator.Close() for ; iterator.Valid(); iterator.Next() { @@ -394,10 +420,16 @@ func (k Keeper) DequeueAndDeleteExpiredGrants(ctx sdk.Context) error { return err } - store.Delete(iterator.Key()) + err = store.Delete(iterator.Key()) + if err != nil { + return err + } for _, typeURL := range queueItem.MsgTypeUrls { - store.Delete(grantStoreKey(grantee, granter, typeURL)) + err = store.Delete(grantStoreKey(grantee, granter, typeURL)) + if err != nil { + return err + } } } diff --git a/x/authz/keeper/keeper_test.go b/x/authz/keeper/keeper_test.go index b101566f8d81..c7bfa602fc6d 100644 --- a/x/authz/keeper/keeper_test.go +++ b/x/authz/keeper/keeper_test.go @@ -14,6 +14,7 @@ import ( storetypes "cosmossdk.io/store/types" "github.com/cosmos/cosmos-sdk/baseapp" + "github.com/cosmos/cosmos-sdk/runtime" "github.com/cosmos/cosmos-sdk/testutil" simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" sdk "github.com/cosmos/cosmos-sdk/types" @@ -48,6 +49,7 @@ type TestSuite struct { func (s *TestSuite) SetupTest() { key := storetypes.NewKVStoreKey(authzkeeper.StoreKey) + storeService := runtime.NewKVStoreService(key) testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test")) s.ctx = testCtx.Ctx.WithBlockHeader(cmtproto.Header{Time: cmttime.Now()}) s.encCfg = moduletestutil.MakeTestEncodingConfig(authzmodule.AppModuleBasic{}) @@ -77,7 +79,7 @@ func (s *TestSuite) SetupTest() { banktypes.RegisterInterfaces(s.encCfg.InterfaceRegistry) banktypes.RegisterMsgServer(s.baseApp.MsgServiceRouter(), s.bankKeeper) - s.authzKeeper = authzkeeper.NewKeeper(key, s.encCfg.Codec, s.baseApp.MsgServiceRouter(), s.accountKeeper) + s.authzKeeper = authzkeeper.NewKeeper(storeService, s.encCfg.Codec, s.baseApp.MsgServiceRouter(), s.accountKeeper) queryHelper := baseapp.NewQueryServerTestHelper(s.ctx, s.encCfg.InterfaceRegistry) authz.RegisterQueryServer(queryHelper, s.authzKeeper) diff --git a/x/authz/keeper/migrations.go b/x/authz/keeper/migrations.go index a4411d0919ee..32b3637d6795 100644 --- a/x/authz/keeper/migrations.go +++ b/x/authz/keeper/migrations.go @@ -17,5 +17,5 @@ func NewMigrator(keeper Keeper) Migrator { // Migrate1to2 migrates from version 1 to 2. func (m Migrator) Migrate1to2(ctx sdk.Context) error { - return v2.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc) + return v2.MigrateStore(ctx, m.keeper.storeService, m.keeper.cdc) } diff --git a/x/authz/migrations/v2/store.go b/x/authz/migrations/v2/store.go index 4090d76a43d8..e340af27fe04 100644 --- a/x/authz/migrations/v2/store.go +++ b/x/authz/migrations/v2/store.go @@ -1,11 +1,15 @@ package v2 import ( + "context" + + corestoretypes "cosmossdk.io/core/store" "cosmossdk.io/store/prefix" storetypes "cosmossdk.io/store/types" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/internal/conv" + "github.com/cosmos/cosmos-sdk/runtime" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/authz" ) @@ -15,9 +19,10 @@ import ( // // - pruning expired authorizations // - create secondary index for pruning expired authorizations -func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error { - store := ctx.KVStore(storeKey) - err := addExpiredGrantsIndex(ctx, store, cdc) +func MigrateStore(ctx context.Context, storeService corestoretypes.KVStoreService, cdc codec.BinaryCodec) error { + store := storeService.OpenKVStore(ctx) + sdkCtx := sdk.UnwrapSDKContext(ctx) + err := addExpiredGrantsIndex(sdkCtx, runtime.KVStoreAdapter(store), cdc) if err != nil { return err } diff --git a/x/authz/migrations/v2/store_test.go b/x/authz/migrations/v2/store_test.go index fd2a266fb3db..7e05f87b8080 100644 --- a/x/authz/migrations/v2/store_test.go +++ b/x/authz/migrations/v2/store_test.go @@ -9,6 +9,7 @@ import ( codectypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" + "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" @@ -99,7 +100,8 @@ func TestMigration(t *testing.T) { }, } - store := ctx.KVStore(authzKey) + storeService := runtime.NewKVStoreService(authzKey) + store := storeService.OpenKVStore(ctx) for _, g := range grants { grant := g.authorization() @@ -107,9 +109,17 @@ func TestMigration(t *testing.T) { } ctx = ctx.WithBlockTime(ctx.BlockTime().Add(1 * time.Hour)) - require.NoError(t, v2.MigrateStore(ctx, authzKey, cdc)) + require.NoError(t, v2.MigrateStore(ctx, storeService, cdc)) - require.NotNil(t, store.Get(v2.GrantStoreKey(grantee1, granter2, genericMsgType))) - require.NotNil(t, store.Get(v2.GrantStoreKey(grantee1, granter1, sendMsgType))) - require.Nil(t, store.Get(v2.GrantStoreKey(grantee2, granter2, genericMsgType))) + bz, err := store.Get(v2.GrantStoreKey(grantee1, granter2, genericMsgType)) + require.NoError(t, err) + require.NotNil(t, bz) + + bz, err = store.Get(v2.GrantStoreKey(grantee1, granter1, sendMsgType)) + require.NoError(t, err) + require.NotNil(t, bz) + + bz, err = store.Get(v2.GrantStoreKey(grantee2, granter2, genericMsgType)) + require.NoError(t, err) + require.Nil(t, bz) } diff --git a/x/authz/module/abci_test.go b/x/authz/module/abci_test.go index 78e8402e76ca..dde9f86bbebf 100644 --- a/x/authz/module/abci_test.go +++ b/x/authz/module/abci_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "github.com/cosmos/cosmos-sdk/baseapp" + "github.com/cosmos/cosmos-sdk/runtime" "github.com/cosmos/cosmos-sdk/testutil" simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" sdk "github.com/cosmos/cosmos-sdk/types" @@ -25,6 +26,7 @@ import ( func TestExpiredGrantsQueue(t *testing.T) { key := storetypes.NewKVStoreKey(keeper.StoreKey) + storeService := runtime.NewKVStoreService(key) testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test")) encCfg := moduletestutil.MakeTestEncodingConfig(authzmodule.AppModuleBasic{}) ctx := testCtx.Ctx.WithBlockHeader(types.Header{}) @@ -62,7 +64,7 @@ func TestExpiredGrantsQueue(t *testing.T) { accountKeeper.EXPECT().StringToBytes(granter.String()).Return(granter, nil).AnyTimes() accountKeeper.EXPECT().BytesToString(granter).Return(granter.String(), nil).AnyTimes() - authzKeeper := keeper.NewKeeper(key, encCfg.Codec, baseApp.MsgServiceRouter(), accountKeeper) + authzKeeper := keeper.NewKeeper(storeService, encCfg.Codec, baseApp.MsgServiceRouter(), accountKeeper) save := func(grantee sdk.AccAddress, exp *time.Time) { err := authzKeeper.SaveGrant(ctx, grantee, granter, sendAuthz, exp) diff --git a/x/authz/module/module.go b/x/authz/module/module.go index a6cc28efb96b..b0d3a16ae086 100644 --- a/x/authz/module/module.go +++ b/x/authz/module/module.go @@ -15,8 +15,8 @@ import ( "cosmossdk.io/depinject" + "cosmossdk.io/core/store" "cosmossdk.io/errors" - store "cosmossdk.io/store/types" "github.com/cosmos/cosmos-sdk/baseapp" sdkclient "github.com/cosmos/cosmos-sdk/client" @@ -173,12 +173,12 @@ func init() { type ModuleInputs struct { depinject.In - Key *store.KVStoreKey Cdc codec.Codec AccountKeeper authz.AccountKeeper BankKeeper authz.BankKeeper Registry cdctypes.InterfaceRegistry MsgServiceRouter baseapp.MessageRouter + StoreService store.KVStoreService } type ModuleOutputs struct { @@ -189,7 +189,7 @@ type ModuleOutputs struct { } func ProvideModule(in ModuleInputs) ModuleOutputs { - k := keeper.NewKeeper(in.Key, in.Cdc, in.MsgServiceRouter, in.AccountKeeper) + k := keeper.NewKeeper(in.StoreService, in.Cdc, in.MsgServiceRouter, in.AccountKeeper) m := NewAppModule(in.Cdc, k, in.AccountKeeper, in.BankKeeper, in.Registry) return ModuleOutputs{AuthzKeeper: k, Module: m} } diff --git a/x/bank/types/send_authorization.go b/x/bank/types/send_authorization.go index ba2796fe9d8f..22449301700b 100644 --- a/x/bank/types/send_authorization.go +++ b/x/bank/types/send_authorization.go @@ -1,6 +1,8 @@ package types import ( + context "context" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/authz" @@ -27,7 +29,7 @@ func (a SendAuthorization) MsgTypeURL() string { } // Accept implements Authorization.Accept. -func (a SendAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.AcceptResponse, error) { +func (a SendAuthorization) Accept(ctx context.Context, msg sdk.Msg) (authz.AcceptResponse, error) { mSend, ok := msg.(*MsgSend) if !ok { return authz.AcceptResponse{}, sdkerrors.ErrInvalidType.Wrap("type mismatch") @@ -41,8 +43,9 @@ func (a SendAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.AcceptRes isAddrExists := false toAddr := mSend.ToAddress allowedList := a.GetAllowList() + sdkCtx := sdk.UnwrapSDKContext(ctx) for _, addr := range allowedList { - ctx.GasMeter().ConsumeGas(gasCostPerIteration, "send authorization") + sdkCtx.GasMeter().ConsumeGas(gasCostPerIteration, "send authorization") if addr == toAddr { isAddrExists = true break diff --git a/x/staking/types/authz.go b/x/staking/types/authz.go index 911974c1e376..284be1fa5af8 100644 --- a/x/staking/types/authz.go +++ b/x/staking/types/authz.go @@ -1,6 +1,8 @@ package types import ( + context "context" + errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" @@ -74,7 +76,7 @@ func (a StakeAuthorization) ValidateBasic() error { // and, should the allowed list not be empty, if the validator is in the allowed list. // If these conditions are met, the authorization amount is validated and if successful, the // corresponding AcceptResponse is returned. -func (a StakeAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.AcceptResponse, error) { +func (a StakeAuthorization) Accept(ctx context.Context, msg sdk.Msg) (authz.AcceptResponse, error) { var ( validatorAddress string amount sdk.Coin @@ -99,8 +101,9 @@ func (a StakeAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.AcceptRe isValidatorExists := false allowedList := a.GetAllowList().GetAddress() + sdkCtx := sdk.UnwrapSDKContext(ctx) for _, validator := range allowedList { - ctx.GasMeter().ConsumeGas(gasCostPerIteration, "stake authorization") + sdkCtx.GasMeter().ConsumeGas(gasCostPerIteration, "stake authorization") if validator == validatorAddress { isValidatorExists = true break @@ -109,7 +112,7 @@ func (a StakeAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.AcceptRe denyList := a.GetDenyList().GetAddress() for _, validator := range denyList { - ctx.GasMeter().ConsumeGas(gasCostPerIteration, "stake authorization") + sdkCtx.GasMeter().ConsumeGas(gasCostPerIteration, "stake authorization") if validator == validatorAddress { return authz.AcceptResponse{}, sdkerrors.ErrUnauthorized.Wrapf("cannot delegate/undelegate to %s validator", validator) }