From 369847cfd3b0b081a03cbc7e15fe9bf3e22f4584 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 15 Apr 2024 17:41:11 +0200 Subject: [PATCH] feat(x/accounts): use router service from env (#20003) --- simapp/app.go | 4 +- .../auth/keeper/msg_server_test.go | 9 +- x/accounts/depinject.go | 10 +-- x/accounts/genesis_test.go | 2 - x/accounts/keeper.go | 48 ++--------- x/accounts/keeper_test.go | 54 +----------- x/accounts/msg_server_test.go | 10 --- x/accounts/query_server_test.go | 4 - x/accounts/utils_test.go | 82 +++++++++++++------ x/bank/types/codec.go | 2 + 10 files changed, 74 insertions(+), 151 deletions(-) diff --git a/simapp/app.go b/simapp/app.go index ea359a1a1ddb..27b7add32317 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -287,10 +287,8 @@ func NewSimApp( // add keepers accountsKeeper, err := accounts.NewKeeper( appCodec, - runtime.NewEnvironment(runtime.NewKVStoreService(keys[accounts.StoreKey]), logger), + runtime.NewEnvironment(runtime.NewKVStoreService(keys[accounts.StoreKey]), logger, runtime.EnvWithRouterService(app.GRPCQueryRouter(), app.MsgServiceRouter())), signingCtx.AddressCodec(), - app.MsgServiceRouter(), - app.GRPCQueryRouter(), appCodec.InterfaceRegistry(), // TESTING: do not add accountstd.AddAccount("counter", counter.NewAccount), diff --git a/tests/integration/auth/keeper/msg_server_test.go b/tests/integration/auth/keeper/msg_server_test.go index b7ae43e00c1d..f2c629becf01 100644 --- a/tests/integration/auth/keeper/msg_server_test.go +++ b/tests/integration/auth/keeper/msg_server_test.go @@ -2,6 +2,7 @@ package keeper_test import ( "context" + "fmt" "strings" "testing" @@ -84,10 +85,8 @@ func initFixture(t *testing.T) *fixture { account := baseaccount.NewAccount("base", signing.NewHandlerMap(handler)) accountsKeeper, err := accounts.NewKeeper( cdc, - runtime.NewEnvironment(runtime.NewKVStoreService(keys[accounts.StoreKey]), log.NewNopLogger()), + runtime.NewEnvironment(runtime.NewKVStoreService(keys[accounts.StoreKey]), log.NewNopLogger(), runtime.EnvWithRouterService(queryRouter, router)), addresscodec.NewBech32Codec("cosmos"), - router, - queryRouter, cdc.InterfaceRegistry(), account, ) @@ -158,7 +157,7 @@ func TestAsyncExec(t *testing.T) { addrs := simtestutil.CreateIncrementalAccounts(2) coins := sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))) - assert.NilError(t, testutil.FundAccount(f.ctx, f.bankKeeper, addrs[0], sdk.NewCoins(sdk.NewInt64Coin("stake", 50)))) + assert.NilError(t, testutil.FundAccount(f.ctx, f.bankKeeper, addrs[0], sdk.NewCoins(sdk.NewInt64Coin("stake", 500)))) msg := &banktypes.MsgSend{ FromAddress: addrs[0].String(), @@ -276,7 +275,7 @@ func TestAsyncExec(t *testing.T) { if tc.expErrMsg != "" { for _, res := range result.Results { if res.Error != "" { - assert.Assert(t, strings.Contains(res.Error, tc.expErrMsg)) + assert.Assert(t, strings.Contains(res.Error, tc.expErrMsg), fmt.Sprintf("res.Error %s does not contain %s", res.Error, tc.expErrMsg)) } continue } diff --git a/x/accounts/depinject.go b/x/accounts/depinject.go index 5f322b74c0ee..1c7e9919fdf8 100644 --- a/x/accounts/depinject.go +++ b/x/accounts/depinject.go @@ -34,9 +34,10 @@ type ModuleInputs struct { Cdc codec.Codec Environment appmodule.Environment AddressCodec address.Codec - ExecRouter MsgRouter - QueryRouter QueryRouter Registry cdctypes.InterfaceRegistry + + // TODO: Add a way to inject custom accounts. + // Currently only the base account is supported. } type ModuleOutputs struct { @@ -61,10 +62,7 @@ func (s directHandler) GetSignBytes(_ context.Context, _ signing.SignerData, _ s func ProvideModule(in ModuleInputs) ModuleOutputs { handler := directHandler{} account := baseaccount.NewAccount("base", signing.NewHandlerMap(handler)) - accountskeeper, err := NewKeeper( - in.Cdc, in.Environment, in.AddressCodec, - in.ExecRouter, in.QueryRouter, in.Registry, account, - ) + accountskeeper, err := NewKeeper(in.Cdc, in.Environment, in.AddressCodec, in.Registry, account) if err != nil { panic(err) } diff --git a/x/accounts/genesis_test.go b/x/accounts/genesis_test.go index f81bd669b565..0c5c25b13304 100644 --- a/x/accounts/genesis_test.go +++ b/x/accounts/genesis_test.go @@ -1,7 +1,6 @@ package accounts import ( - "context" "testing" "github.com/cosmos/gogoproto/types" @@ -16,7 +15,6 @@ func TestGenesis(t *testing.T) { acc, err := NewTestAccount(deps) return "test", acc, err }) - k.queryRouter = mockQuery(func(ctx context.Context, req, resp implementation.ProtoMsg) error { return nil }) // we init two accounts of the same type // we set counter to 10 diff --git a/x/accounts/keeper.go b/x/accounts/keeper.go index 676130fe6437..6dd4004d7b0f 100644 --- a/x/accounts/keeper.go +++ b/x/accounts/keeper.go @@ -35,19 +35,6 @@ var ( AccountByNumber = collections.NewPrefix(2) ) -// QueryRouter represents a router which can be used to route queries to the correct module. -// It returns the handler given the message name, if multiple handlers are returned, then -// it is up to the caller to choose which one to call. -type QueryRouter interface { - HybridHandlerByRequestName(name string) []func(ctx context.Context, req, resp implementation.ProtoMsg) error -} - -// MsgRouter represents a router which can be used to route messages to the correct module. -type MsgRouter interface { - HybridHandlerByMsgName(msgName string) func(ctx context.Context, req, resp implementation.ProtoMsg) error - ResponseNameByMsgName(name string) string -} - type InterfaceRegistry interface { RegisterInterface(name string, iface any, impls ...protoiface.MessageV1) RegisterImplementations(iface any, impls ...protoiface.MessageV1) @@ -57,18 +44,14 @@ func NewKeeper( cdc codec.Codec, env appmodule.Environment, addressCodec address.Codec, - execRouter MsgRouter, - queryRouter QueryRouter, ir InterfaceRegistry, accounts ...accountstd.AccountCreatorFunc, ) (Keeper, error) { sb := collections.NewSchemaBuilder(env.KVStoreService) keeper := Keeper{ environment: env, - addressCodec: addressCodec, - msgRouter: execRouter, codec: cdc, - queryRouter: queryRouter, + addressCodec: addressCodec, makeSendCoinsMsg: defaultCoinsTransferMsgFunc(addressCodec), Schema: collections.Schema{}, AccountNumber: collections.NewSequence(sb, AccountNumberKey, "account_number"), @@ -95,8 +78,6 @@ type Keeper struct { environment appmodule.Environment addressCodec address.Codec codec codec.Codec - msgRouter MsgRouter // todo use env - queryRouter QueryRouter // todo use env makeSendCoinsMsg coinsTransferMsgFunc accounts map[string]implementation.Implementation @@ -343,17 +324,11 @@ func (k Keeper) sendAnyMessages(ctx context.Context, sender []byte, anyMessages // SendModuleMessageUntyped can be used to send a message towards a module. // It should be used when the response type is not known by the caller. func (k Keeper) SendModuleMessageUntyped(ctx context.Context, sender []byte, msg implementation.ProtoMsg) (implementation.ProtoMsg, error) { - // we need to fetch the response type from the request message type. - // this is because the response type is not known. - respName := k.msgRouter.ResponseNameByMsgName(implementation.MessageName(msg)) - if respName == "" { - return nil, fmt.Errorf("could not find response type for message %T", msg) - } - // get response type - resp, err := implementation.FindMessageByName(respName) + resp, err := k.environment.RouterService.MessageRouterService().InvokeUntyped(ctx, msg) if err != nil { return nil, err } + // send the message return resp, k.sendModuleMessage(ctx, sender, msg, resp) } @@ -373,27 +348,14 @@ func (k Keeper) sendModuleMessage(ctx context.Context, sender []byte, msg, msgRe if !bytes.Equal(sender, wantSenders[0]) { return fmt.Errorf("%w: sender does not match expected sender", ErrUnauthorized) } - messageName := implementation.MessageName(msg) - handler := k.msgRouter.HybridHandlerByMsgName(messageName) - if handler == nil { - return fmt.Errorf("unknown message: %s", messageName) - } - return handler(ctx, msg, msgResp) + return k.environment.RouterService.MessageRouterService().InvokeTyped(ctx, msg, msgResp) } // queryModule is the entrypoint for an account to query a module. // It will try to find the query handler for the given query and execute it. // If multiple query handlers are found, it will return an error. func (k Keeper) queryModule(ctx context.Context, queryReq, queryResp implementation.ProtoMsg) error { - queryName := implementation.MessageName(queryReq) - handlers := k.queryRouter.HybridHandlerByRequestName(queryName) - if len(handlers) == 0 { - return fmt.Errorf("unknown query: %s", queryName) - } - if len(handlers) > 1 { - return fmt.Errorf("multiple handlers for query: %s", queryName) - } - return handlers[0](ctx, queryReq, queryResp) + return k.environment.RouterService.QueryRouterService().InvokeTyped(ctx, queryReq, queryResp) } // maybeSendFunds will send the provided coins between the provided addresses, if amt diff --git a/x/accounts/keeper_test.go b/x/accounts/keeper_test.go index 27642680ae8f..b0532cd766c4 100644 --- a/x/accounts/keeper_test.go +++ b/x/accounts/keeper_test.go @@ -1,15 +1,11 @@ package accounts import ( - "context" "testing" "github.com/cosmos/gogoproto/types" "github.com/stretchr/testify/require" - "google.golang.org/protobuf/proto" - bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1" - basev1beta1 "cosmossdk.io/api/cosmos/base/v1beta1" "cosmossdk.io/collections" "cosmossdk.io/x/accounts/accountstd" "cosmossdk.io/x/accounts/internal/implementation" @@ -17,13 +13,6 @@ import ( func TestKeeper_Init(t *testing.T) { m, ctx := newKeeper(t, accountstd.AddAccount("test", NewTestAccount)) - m.queryRouter = mockQuery(func(ctx context.Context, req, resp implementation.ProtoMsg) error { - _, ok := req.(*bankv1beta1.QueryBalanceRequest) - require.True(t, ok) - _, ok = resp.(*bankv1beta1.QueryBalanceResponse) - require.True(t, ok) - return nil - }) t.Run("ok", func(t *testing.T) { sender := []byte("sender") @@ -52,7 +41,6 @@ func TestKeeper_Init(t *testing.T) { func TestKeeper_Execute(t *testing.T) { m, ctx := newKeeper(t, accountstd.AddAccount("test", NewTestAccount)) - m.queryRouter = mockQuery(func(ctx context.Context, req, resp implementation.ProtoMsg) error { return nil }) // create account sender := []byte("sender") @@ -71,35 +59,14 @@ func TestKeeper_Execute(t *testing.T) { }) t.Run("exec module", func(t *testing.T) { - m.msgRouter = mockExec(func(ctx context.Context, msg, msgResp implementation.ProtoMsg) error { - concrete, ok := msg.(*bankv1beta1.MsgSend) - require.True(t, ok) - require.Equal(t, concrete.ToAddress, "recipient") - _, ok = msgResp.(*bankv1beta1.MsgSendResponse) - require.True(t, ok) - return nil - }) - - m.msgRouter = mockExec(func(ctx context.Context, msg, msgResp implementation.ProtoMsg) error { - concrete, ok := msg.(*bankv1beta1.MsgSend) - require.True(t, ok) - require.Equal(t, concrete.FromAddress, string(accAddr)) - _, ok = msgResp.(*bankv1beta1.MsgSendResponse) - require.True(t, ok) - - resp, err := m.Execute(ctx, accAddr, sender, &types.Int64Value{Value: 1000}, nil) - require.NoError(t, err) - require.True(t, implementation.Equal(&types.Empty{}, resp)) - return nil - }) + resp, err := m.Execute(ctx, accAddr, sender, &types.Int64Value{Value: 1000}, nil) + require.NoError(t, err) + require.True(t, implementation.Equal(&types.Empty{}, resp)) }) } func TestKeeper_Query(t *testing.T) { m, ctx := newKeeper(t, accountstd.AddAccount("test", NewTestAccount)) - m.queryRouter = mockQuery(func(ctx context.Context, req, resp implementation.ProtoMsg) error { - return nil - }) // create account sender := []byte("sender") @@ -118,21 +85,6 @@ func TestKeeper_Query(t *testing.T) { }) t.Run("query module", func(t *testing.T) { - // we inject the module query function, which accepts only a specific type of message - // we force the response - m.queryRouter = mockQuery(func(ctx context.Context, req, resp implementation.ProtoMsg) error { - concrete, ok := req.(*bankv1beta1.QueryBalanceRequest) - require.True(t, ok) - require.Equal(t, string(accAddr), concrete.Address) - require.Equal(t, concrete.Denom, "atom") - copyResp := &bankv1beta1.QueryBalanceResponse{Balance: &basev1beta1.Coin{ - Denom: "atom", - Amount: "1000", - }} - proto.Merge(resp.(proto.Message), copyResp) - return nil - }) - resp, err := m.Query(ctx, accAddr, &types.StringValue{Value: "atom"}) require.NoError(t, err) require.True(t, implementation.Equal(&types.Int64Value{Value: 1000}, resp)) diff --git a/x/accounts/msg_server_test.go b/x/accounts/msg_server_test.go index 1b96e415247c..23d4cc66c684 100644 --- a/x/accounts/msg_server_test.go +++ b/x/accounts/msg_server_test.go @@ -1,15 +1,12 @@ package accounts import ( - "context" "testing" "github.com/stretchr/testify/require" - "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/emptypb" "google.golang.org/protobuf/types/known/wrapperspb" - bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1" "cosmossdk.io/x/accounts/accountstd" "cosmossdk.io/x/accounts/internal/implementation" v1 "cosmossdk.io/x/accounts/v1" @@ -17,13 +14,6 @@ import ( func TestMsgServer(t *testing.T) { k, ctx := newKeeper(t, accountstd.AddAccount("test", NewTestAccount)) - k.queryRouter = mockQuery(func(ctx context.Context, req, resp implementation.ProtoMsg) error { - _, ok := req.(*bankv1beta1.QueryBalanceRequest) - require.True(t, ok) - proto.Merge(resp.(proto.Message), &bankv1beta1.QueryBalanceResponse{}) - return nil - }) - s := NewMsgServer(k) // create diff --git a/x/accounts/query_server_test.go b/x/accounts/query_server_test.go index bc4913458a28..91a14caba3d4 100644 --- a/x/accounts/query_server_test.go +++ b/x/accounts/query_server_test.go @@ -1,7 +1,6 @@ package accounts import ( - "context" "testing" "github.com/cosmos/gogoproto/types" @@ -16,9 +15,6 @@ import ( func TestQueryServer(t *testing.T) { k, ctx := newKeeper(t, accountstd.AddAccount("test", NewTestAccount)) - k.queryRouter = mockQuery(func(ctx context.Context, req, resp implementation.ProtoMsg) error { - return nil - }) ms := NewMsgServer(k) qs := NewQueryServer(k) diff --git a/x/accounts/utils_test.go b/x/accounts/utils_test.go index d9966acd7a0d..837b9b8ccfe3 100644 --- a/x/accounts/utils_test.go +++ b/x/accounts/utils_test.go @@ -4,16 +4,25 @@ import ( "context" "testing" + gogoproto "github.com/cosmos/gogoproto/proto" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/reflect/protoregistry" "google.golang.org/protobuf/runtime/protoiface" + bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1" + basev1beta1 "cosmossdk.io/api/cosmos/base/v1beta1" "cosmossdk.io/collections/colltest" "cosmossdk.io/core/address" "cosmossdk.io/core/event" "cosmossdk.io/log" "cosmossdk.io/x/accounts/internal/implementation" + "cosmossdk.io/x/tx/signing" + "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" ) var _ address.Codec = (*addressCodec)(nil) @@ -33,44 +42,63 @@ func (e eventService) EmitKV(eventType string, attrs ...event.Attribute) error { func (e eventService) EventManager(ctx context.Context) event.Manager { return e } -var _ InterfaceRegistry = (*interfaceRegistry)(nil) - -type interfaceRegistry struct{} - -func (i interfaceRegistry) RegisterInterface(string, any, ...protoiface.MessageV1) {} - -func (i interfaceRegistry) RegisterImplementations(any, ...protoiface.MessageV1) {} - func newKeeper(t *testing.T, accounts ...implementation.AccountCreatorFunc) (Keeper, context.Context) { t.Helper() + + addressCodec := addressCodec{} + ir, err := codectypes.NewInterfaceRegistryWithOptions(codectypes.InterfaceRegistryOptions{ + ProtoFiles: gogoproto.HybridResolver, + SigningOptions: signing.Options{ + FileResolver: gogoproto.HybridResolver, + TypeResolver: protoregistry.GlobalTypes, + AddressCodec: addressCodec, + ValidatorAddressCodec: addressCodec, + }, + }) + if err != nil { + t.Fatal(err) + } + msgRouter := baseapp.NewMsgServiceRouter() + msgRouter.SetInterfaceRegistry(ir) + queryRouter := baseapp.NewGRPCQueryRouter() + queryRouter.SetInterfaceRegistry(ir) + + ir.RegisterImplementations((*sdk.Msg)(nil), + &bankv1beta1.MsgSend{}, + &bankv1beta1.MsgBurn{}, + &bankv1beta1.MsgSetSendEnabled{}, + &bankv1beta1.MsgMultiSend{}, + &bankv1beta1.MsgUpdateParams{}, + ) + queryRouter.RegisterService(&bankv1beta1.Query_ServiceDesc, &bankQueryServer{}) + msgRouter.RegisterService(&bankv1beta1.Msg_ServiceDesc, &bankMsgServer{}) + ss, ctx := colltest.MockStore() - env := runtime.NewEnvironment(ss, log.NewNopLogger()) + env := runtime.NewEnvironment(ss, log.NewNopLogger(), runtime.EnvWithRouterService( + queryRouter, + msgRouter, + )) env.EventService = eventService{} - m, err := NewKeeper(nil, env, addressCodec{}, nil, nil, interfaceRegistry{}, accounts...) + m, err := NewKeeper(codec.NewProtoCodec(ir), env, addressCodec, ir, accounts...) require.NoError(t, err) return m, ctx } -var _ QueryRouter = (*mockQuery)(nil) - -type mockQuery func(ctx context.Context, req, resp implementation.ProtoMsg) error - -func (m mockQuery) HybridHandlerByRequestName(_ string) []func(ctx context.Context, req, resp implementation.ProtoMsg) error { - return []func(ctx context.Context, req, resp protoiface.MessageV1) error{func(ctx context.Context, req, resp protoiface.MessageV1) error { - return m(ctx, req, resp) - }} +type bankQueryServer struct { + bankv1beta1.UnimplementedQueryServer } -var _ MsgRouter = (*mockExec)(nil) - -type mockExec func(ctx context.Context, msg, msgResp implementation.ProtoMsg) error +func (b bankQueryServer) Balance(context.Context, *bankv1beta1.QueryBalanceRequest) (*bankv1beta1.QueryBalanceResponse, error) { + return &bankv1beta1.QueryBalanceResponse{Balance: &basev1beta1.Coin{ + Denom: "atom", + Amount: "1000", + }}, nil +} -func (m mockExec) HybridHandlerByMsgName(_ string) func(ctx context.Context, req, resp protoiface.MessageV1) error { - return func(ctx context.Context, req, resp protoiface.MessageV1) error { - return m(ctx, req, resp) - } +type bankMsgServer struct { + bankv1beta1.UnimplementedMsgServer } -func (m mockExec) ResponseNameByMsgName(name string) string { - return name + "Response" +func (b bankMsgServer) Send(context.Context, *bankv1beta1.MsgSend) (*bankv1beta1.MsgSendResponse, error) { + return &bankv1beta1.MsgSendResponse{}, nil } diff --git a/x/bank/types/codec.go b/x/bank/types/codec.go index eea6b27dbdcf..e63d77e583f0 100644 --- a/x/bank/types/codec.go +++ b/x/bank/types/codec.go @@ -26,6 +26,8 @@ func RegisterInterfaces(registrar registry.InterfaceRegistrar) { &MsgSend{}, &MsgMultiSend{}, &MsgUpdateParams{}, + &MsgBurn{}, + &MsgSetSendEnabled{}, ) msgservice.RegisterMsgServiceDesc(registrar, &_Msg_serviceDesc)