Skip to content

Commit

Permalink
refactor(x/authz)!: use router service (#19637)
Browse files Browse the repository at this point in the history
Co-authored-by: Likhita Polavarapu <[email protected]>
  • Loading branch information
julienrbrt and likhita-809 authored Mar 4, 2024
1 parent 7f84815 commit 8c12991
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 62 deletions.
2 changes: 1 addition & 1 deletion core/router/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type Service interface {
// Router is the interface that wraps the basic methods for a router.
type Router interface {
// CanInvoke returns an error if the given request cannot be invoked.
CanInvoke(ctx context.Context, req protoiface.MessageV1) error
CanInvoke(ctx context.Context, typeURL string) error
// InvokeTyped execute a message or query. It should be used when the called knows the type of the response.
InvokeTyped(ctx context.Context, req, res protoiface.MessageV1) error
// InvokeUntyped execute a message or query. It should be used when the called doesn't know the type of the response.
Expand Down
24 changes: 15 additions & 9 deletions runtime/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,14 @@ type msgRouterService struct {
}

// CanInvoke returns an error if the given message cannot be invoked.
func (m *msgRouterService) CanInvoke(ctx context.Context, msg protoiface.MessageV1) error {
messageName := msgTypeURL(msg)
handler := m.router.HybridHandlerByMsgName(messageName)
func (m *msgRouterService) CanInvoke(ctx context.Context, typeURL string) error {
if typeURL == "" {
return fmt.Errorf("missing type url")
}

handler := m.router.HybridHandlerByMsgName(typeURL)
if handler == nil {
return fmt.Errorf("unknown message: %s", messageName)
return fmt.Errorf("unknown message: %s", typeURL)
}

return nil
Expand Down Expand Up @@ -106,13 +109,16 @@ type queryRouterService struct {
}

// CanInvoke returns an error if the given request cannot be invoked.
func (m *queryRouterService) CanInvoke(ctx context.Context, req protoiface.MessageV1) error {
reqName := msgTypeURL(req)
handlers := m.router.HybridHandlerByRequestName(reqName)
func (m *queryRouterService) CanInvoke(ctx context.Context, typeURL string) error {
if typeURL == "" {
return fmt.Errorf("missing type url")
}

handlers := m.router.HybridHandlerByRequestName(typeURL)
if len(handlers) == 0 {
return fmt.Errorf("unknown request: %s", reqName)
return fmt.Errorf("unknown request: %s", typeURL)
} else if len(handlers) > 1 {
return fmt.Errorf("ambiguous request, query have multiple handlers: %s", reqName)
return fmt.Errorf("ambiguous request, query have multiple handlers: %s", typeURL)
}

return nil
Expand Down
2 changes: 1 addition & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ func NewSimApp(
app.CircuitKeeper = circuitkeeper.NewKeeper(runtime.NewEnvironment(runtime.NewKVStoreService(keys[circuittypes.StoreKey]), logger), appCodec, authtypes.NewModuleAddress(govtypes.ModuleName).String(), app.AuthKeeper.AddressCodec())
app.BaseApp.SetCircuitBreaker(&app.CircuitKeeper)

app.AuthzKeeper = authzkeeper.NewKeeper(runtime.NewEnvironment(runtime.NewKVStoreService(keys[authzkeeper.StoreKey]), logger), appCodec, app.MsgServiceRouter(), app.AuthKeeper)
app.AuthzKeeper = authzkeeper.NewKeeper(runtime.NewEnvironment(runtime.NewKVStoreService(keys[authzkeeper.StoreKey]), logger, runtime.EnvWithRouterService(app.GRPCQueryRouter(), app.MsgServiceRouter())), appCodec, app.AuthKeeper)

groupConfig := group.DefaultConfig()
/*
Expand Down
13 changes: 5 additions & 8 deletions x/authz/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,17 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

### Consens Breaking Changes

* [#19188](https://github.com/cosmos/cosmos-sdk/pull/19188) Remove creation of `BaseAccount` when sending a message to an account that does not exist

### Features

* [#18737](https://github.com/cosmos/cosmos-sdk/pull/18737) Added a limit of 200 grants pruned per `BeginBlock` and the `PruneExpiredGrants` message that prunes 75 expired grants on every run.

### Improvements

### API Breaking Changes

* [#19490](https://github.com/cosmos/cosmos-sdk/pull/19490) `appmodule.Environment` is received on the Keeper to get access to different application services
* [#19637](https://github.com/cosmos/cosmos-sdk/pull/19637) `NewKeeper` doesn't take a message router anymore. Set the message router in the `appmodule.Environment` instead.
* [#19490](https://github.com/cosmos/cosmos-sdk/pull/19490) `appmodule.Environment` is received on the Keeper to get access to different application services.
* [#18737](https://github.com/cosmos/cosmos-sdk/pull/18737) Update the keeper method `DequeueAndDeleteExpiredGrants` to take a limit argument for the number of grants to prune.
* [#16509](https://github.com/cosmos/cosmos-sdk/pull/16509) `AcceptResponse` has been moved to sdk/types/authz and the `Updated` field is now of the type `sdk.Msg` instead of `authz.Authorization`.

### Bug Fixes
### Consensus Breaking Changes

* [#19188](https://github.com/cosmos/cosmos-sdk/pull/19188) Remove creation of `BaseAccount` when sending a message to an account that does not exist.
4 changes: 2 additions & 2 deletions x/authz/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ func (suite *GenesisTestSuite) SetupTest() {
storeService := runtime.NewKVStoreService(key)
testCtx := testutil.DefaultContextWithDB(suite.T(), key, storetypes.NewTransientStoreKey("transient_test"))
suite.ctx = testCtx.Ctx.WithHeaderInfo(header.Info{Height: 1})
env := runtime.NewEnvironment(storeService, log.NewNopLogger())

suite.encCfg = moduletestutil.MakeTestEncodingConfig(authzmodule.AppModule{})

Expand All @@ -68,8 +67,9 @@ func (suite *GenesisTestSuite) SetupTest() {

msr := suite.baseApp.MsgServiceRouter()
msr.SetInterfaceRegistry(suite.encCfg.InterfaceRegistry)
env := runtime.NewEnvironment(storeService, log.NewNopLogger(), runtime.EnvWithRouterService(nil, msr))

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

func (suite *GenesisTestSuite) TestImportExportGenesis() {
Expand Down
31 changes: 4 additions & 27 deletions x/authz/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,16 @@ import (
"bytes"
"context"
"fmt"
"strconv"
"time"

"github.com/cosmos/gogoproto/proto"

"cosmossdk.io/core/appmodule"
"cosmossdk.io/core/event"
errorsmod "cosmossdk.io/errors"
"cosmossdk.io/log"
storetypes "cosmossdk.io/store/types"
"cosmossdk.io/x/authz"

"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"
Expand All @@ -32,16 +29,14 @@ const gasCostPerIteration = uint64(20)
type Keeper struct {
environment appmodule.Environment
cdc codec.Codec
router baseapp.MessageRouter
authKeeper authz.AccountKeeper
}

// NewKeeper constructs a message authorization Keeper
func NewKeeper(env appmodule.Environment, cdc codec.Codec, router baseapp.MessageRouter, ak authz.AccountKeeper) Keeper {
func NewKeeper(env appmodule.Environment, cdc codec.Codec, ak authz.AccountKeeper) Keeper {
return Keeper{
environment: env,
cdc: cdc,
router: router,
authKeeper: ak,
}
}
Expand Down Expand Up @@ -151,29 +146,11 @@ func (k Keeper) DispatchActions(ctx context.Context, grantee sdk.AccAddress, msg
}
}

handler := k.router.Handler(msg)
if handler == nil {
return nil, sdkerrors.ErrUnknownRequest.Wrapf("unrecognized message route: %s", sdk.MsgTypeURL(msg))
}

msgResp, err := handler(sdkCtx, msg)
// no need to use the branch service here, as if the transaction fails, the transaction will be reverted
_, err = k.environment.RouterService.MessageRouterService().InvokeUntyped(ctx, msg)
if err != nil {
return nil, errorsmod.Wrapf(err, "failed to execute message; message %v", msg)
return nil, fmt.Errorf("failed to execute message %d; message %v: %w", i, msg, err)
}

results[i] = msgResp.Data

// emit the events from the dispatched actions
for i = range msgResp.Events {
err := k.environment.EventService.EventManager(ctx).EmitKV(
"dispatch actions",
event.NewAttribute("authz_msg_index", strconv.Itoa(i)),
)
if err != nil {
return nil, err
}
}

}

return results, nil
Expand Down
4 changes: 2 additions & 2 deletions x/authz/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ func (s *TestSuite) SetupTest() {
testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test"))
s.ctx = testCtx.Ctx.WithHeaderInfo(header.Info{Time: time.Now().Round(0).UTC()})
s.encCfg = moduletestutil.MakeTestEncodingConfig(authzmodule.AppModule{})
env := runtime.NewEnvironment(storeService, log.NewNopLogger())

s.baseApp = baseapp.NewBaseApp(
"authz",
Expand All @@ -75,7 +74,8 @@ func (s *TestSuite) SetupTest() {
banktypes.RegisterInterfaces(s.encCfg.InterfaceRegistry)
banktypes.RegisterMsgServer(s.baseApp.MsgServiceRouter(), s.bankKeeper)

s.authzKeeper = authzkeeper.NewKeeper(env, s.encCfg.Codec, s.baseApp.MsgServiceRouter(), s.accountKeeper)
env := runtime.NewEnvironment(storeService, log.NewNopLogger(), runtime.EnvWithRouterService(s.baseApp.GRPCQueryRouter(), s.baseApp.MsgServiceRouter()))
s.authzKeeper = authzkeeper.NewKeeper(env, s.encCfg.Codec, s.accountKeeper)

queryHelper := baseapp.NewQueryServerTestHelper(s.ctx, s.encCfg.InterfaceRegistry)
authz.RegisterQueryServer(queryHelper, s.authzKeeper)
Expand Down
4 changes: 2 additions & 2 deletions x/authz/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ func (k Keeper) Grant(ctx context.Context, msg *authz.MsgGrant) (*authz.MsgGrant
}

t := authorization.MsgTypeURL()
if k.router.HandlerByTypeURL(t) == nil {
return nil, sdkerrors.ErrInvalidType.Wrapf("%s doesn't exist.", t)
if k.environment.RouterService.MessageRouterService().CanInvoke(ctx, t) == nil {
return nil, sdkerrors.ErrInvalidType.Wrapf("%s doesn't exist", t)
}

err = k.SaveGrant(ctx, grantee, granter, authorization, msg.Grant.Expiration)
Expand Down
4 changes: 2 additions & 2 deletions x/authz/module/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ func TestExpiredGrantsQueue(t *testing.T) {
testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test"))
encCfg := moduletestutil.MakeTestEncodingConfig(authzmodule.AppModule{})
ctx := testCtx.Ctx
env := runtime.NewEnvironment(storeService, log.NewNopLogger())

baseApp := baseapp.NewBaseApp(
"authz",
Expand Down Expand Up @@ -66,7 +65,8 @@ func TestExpiredGrantsQueue(t *testing.T) {

accountKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes()

authzKeeper := keeper.NewKeeper(env, encCfg.Codec, baseApp.MsgServiceRouter(), accountKeeper)
env := runtime.NewEnvironment(storeService, log.NewNopLogger(), runtime.EnvWithRouterService(baseApp.GRPCQueryRouter(), baseApp.MsgServiceRouter()))
authzKeeper := keeper.NewKeeper(env, encCfg.Codec, accountKeeper)

save := func(grantee sdk.AccAddress, exp *time.Time) {
err := authzKeeper.SaveGrant(ctx, grantee, granter, sendAuthz, exp)
Expand Down
14 changes: 6 additions & 8 deletions x/authz/module/depinject.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"cosmossdk.io/x/authz"
"cosmossdk.io/x/authz/keeper"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/codec"
cdctypes "github.com/cosmos/cosmos-sdk/codec/types"
)
Expand All @@ -28,12 +27,11 @@ func init() {
type ModuleInputs struct {
depinject.In

Cdc codec.Codec
AccountKeeper authz.AccountKeeper
BankKeeper authz.BankKeeper
Registry cdctypes.InterfaceRegistry
MsgServiceRouter baseapp.MessageRouter
Environment appmodule.Environment
Cdc codec.Codec
AccountKeeper authz.AccountKeeper
BankKeeper authz.BankKeeper
Registry cdctypes.InterfaceRegistry
Environment appmodule.Environment
}

type ModuleOutputs struct {
Expand All @@ -44,7 +42,7 @@ type ModuleOutputs struct {
}

func ProvideModule(in ModuleInputs) ModuleOutputs {
k := keeper.NewKeeper(in.Environment, in.Cdc, in.MsgServiceRouter, in.AccountKeeper)
k := keeper.NewKeeper(in.Environment, in.Cdc, in.AccountKeeper)
m := NewAppModule(in.Cdc, k, in.AccountKeeper, in.BankKeeper, in.Registry)
return ModuleOutputs{AuthzKeeper: k, Module: m}
}

0 comments on commit 8c12991

Please sign in to comment.