Skip to content
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
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
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If we merge this first I'll go update the api usage in the gov pr.

// 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`.
Comment on lines +34 to 37
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider adding more descriptive text to clarify the impact of the API breaking changes for developers. While the technical terms and references are correctly used, a brief explanation of why these changes were made and how developers can adapt would enhance the documentation. For instance:

+ * [#19637](https://github.com/cosmos/cosmos-sdk/pull/19637) The `NewKeeper` function signature has been updated: it no longer requires a message router parameter. Developers should now set the message router directly in the `appmodule.Environment`.
+ * [#19490](https://github.com/cosmos/cosmos-sdk/pull/19490) The `appmodule.Environment` is now explicitly passed to the Keeper, allowing access to various application services more directly.

This approach provides clearer guidance to developers on adapting to the new changes.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
* [#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`.
* [#19637](https://github.com/cosmos/cosmos-sdk/pull/19637) The `NewKeeper` function signature has been updated: it no longer requires a message router parameter. Developers should now set the message router directly in the `appmodule.Environment`.
* [#19490](https://github.com/cosmos/cosmos-sdk/pull/19490) The `appmodule.Environment` is now explicitly passed to the Keeper, allowing access to various application services more directly.
* [#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}
}