diff --git a/CHANGELOG.md b/CHANGELOG.md index b09bc01d73e8..8dcf673cba23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,7 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking -* (AppModule) [\#7518](https://github.com/cosmos/cosmos-sdk/pull/7518) Rename `AppModule.RegisterQueryServices` to `AppModule.RegisterServices`, as this method now registers multiple services (the gRPC query service and the protobuf Msg service). A `Configurator` struct is used to hold the different services. +* (AppModule) [\#7518](https://github.com/cosmos/cosmos-sdk/pull/7518) [\#7584](https://github.com/cosmos/cosmos-sdk/pull/7584) Rename `AppModule.RegisterQueryServices` to `AppModule.RegisterServices`, as this method now registers multiple services (the gRPC query service and the protobuf Msg service). A `Configurator` struct is used to hold the different services. ### Features diff --git a/simapp/app.go b/simapp/app.go index aa1375ea7a74..5c3412f36003 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -360,7 +360,7 @@ func NewSimApp( app.mm.RegisterInvariants(&app.CrisisKeeper) app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), encodingConfig.Amino) - app.mm.RegisterServices(module.NewConfigurator(app.GRPCQueryRouter())) + app.mm.RegisterServices(module.NewConfigurator(app.MsgServiceRouter(), app.GRPCQueryRouter())) // add test gRPC service for testing gRPC queries in isolation testdata.RegisterQueryServer(app.GRPCQueryRouter(), testdata.QueryImpl{}) diff --git a/tests/mocks/account_retriever.go b/tests/mocks/account_retriever.go index f571fcbd1516..23b02b7dce2b 100644 --- a/tests/mocks/account_retriever.go +++ b/tests/mocks/account_retriever.go @@ -8,9 +8,89 @@ import ( client "github.com/cosmos/cosmos-sdk/client" types "github.com/cosmos/cosmos-sdk/types" gomock "github.com/golang/mock/gomock" + crypto "github.com/tendermint/tendermint/crypto" reflect "reflect" ) +// MockAccount is a mock of Account interface +type MockAccount struct { + ctrl *gomock.Controller + recorder *MockAccountMockRecorder +} + +// MockAccountMockRecorder is the mock recorder for MockAccount +type MockAccountMockRecorder struct { + mock *MockAccount +} + +// NewMockAccount creates a new mock instance +func NewMockAccount(ctrl *gomock.Controller) *MockAccount { + mock := &MockAccount{ctrl: ctrl} + mock.recorder = &MockAccountMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockAccount) EXPECT() *MockAccountMockRecorder { + return m.recorder +} + +// GetAddress mocks base method +func (m *MockAccount) GetAddress() types.AccAddress { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAddress") + ret0, _ := ret[0].(types.AccAddress) + return ret0 +} + +// GetAddress indicates an expected call of GetAddress +func (mr *MockAccountMockRecorder) GetAddress() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAddress", reflect.TypeOf((*MockAccount)(nil).GetAddress)) +} + +// GetPubKey mocks base method +func (m *MockAccount) GetPubKey() crypto.PubKey { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPubKey") + ret0, _ := ret[0].(crypto.PubKey) + return ret0 +} + +// GetPubKey indicates an expected call of GetPubKey +func (mr *MockAccountMockRecorder) GetPubKey() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPubKey", reflect.TypeOf((*MockAccount)(nil).GetPubKey)) +} + +// GetAccountNumber mocks base method +func (m *MockAccount) GetAccountNumber() uint64 { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAccountNumber") + ret0, _ := ret[0].(uint64) + return ret0 +} + +// GetAccountNumber indicates an expected call of GetAccountNumber +func (mr *MockAccountMockRecorder) GetAccountNumber() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAccountNumber", reflect.TypeOf((*MockAccount)(nil).GetAccountNumber)) +} + +// GetSequence mocks base method +func (m *MockAccount) GetSequence() uint64 { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSequence") + ret0, _ := ret[0].(uint64) + return ret0 +} + +// GetSequence indicates an expected call of GetSequence +func (mr *MockAccountMockRecorder) GetSequence() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSequence", reflect.TypeOf((*MockAccount)(nil).GetSequence)) +} + // MockAccountRetriever is a mock of AccountRetriever interface type MockAccountRetriever struct { ctrl *gomock.Controller @@ -34,6 +114,37 @@ func (m *MockAccountRetriever) EXPECT() *MockAccountRetrieverMockRecorder { return m.recorder } +// GetAccount mocks base method +func (m *MockAccountRetriever) GetAccount(clientCtx client.Context, addr types.AccAddress) (client.Account, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAccount", clientCtx, addr) + ret0, _ := ret[0].(client.Account) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetAccount indicates an expected call of GetAccount +func (mr *MockAccountRetrieverMockRecorder) GetAccount(clientCtx, addr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAccount", reflect.TypeOf((*MockAccountRetriever)(nil).GetAccount), clientCtx, addr) +} + +// GetAccountWithHeight mocks base method +func (m *MockAccountRetriever) GetAccountWithHeight(clientCtx client.Context, addr types.AccAddress) (client.Account, int64, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAccountWithHeight", clientCtx, addr) + ret0, _ := ret[0].(client.Account) + ret1, _ := ret[1].(int64) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// GetAccountWithHeight indicates an expected call of GetAccountWithHeight +func (mr *MockAccountRetrieverMockRecorder) GetAccountWithHeight(clientCtx, addr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAccountWithHeight", reflect.TypeOf((*MockAccountRetriever)(nil).GetAccountWithHeight), clientCtx, addr) +} + // EnsureExists mocks base method func (m *MockAccountRetriever) EnsureExists(clientCtx client.Context, addr types.AccAddress) error { m.ctrl.T.Helper() diff --git a/types/module/configurator.go b/types/module/configurator.go index efa83607de14..d561dd9eef0d 100644 --- a/types/module/configurator.go +++ b/types/module/configurator.go @@ -7,20 +7,33 @@ import "github.com/gogo/protobuf/grpc" // support module object capabilities isolation as described in // https://github.com/cosmos/cosmos-sdk/issues/7093 type Configurator interface { + // MsgServer returns a grpc.Server instance which allows registering services + // that will handle TxBody.messages in transactions. These Msg's WILL NOT + // be exposed as gRPC services. + MsgServer() grpc.Server + + // QueryServer returns a grpc.Server instance which allows registering services + // that will be exposed as gRPC services as well as ABCI query handlers. QueryServer() grpc.Server } type configurator struct { + msgServer grpc.Server queryServer grpc.Server } // NewConfigurator returns a new Configurator instance -func NewConfigurator(queryServer grpc.Server) Configurator { - return configurator{queryServer: queryServer} +func NewConfigurator(msgServer grpc.Server, queryServer grpc.Server) Configurator { + return configurator{msgServer: msgServer, queryServer: queryServer} } var _ Configurator = configurator{} +// MsgServer implements the Configurator.MsgServer method +func (c configurator) MsgServer() grpc.Server { + return c.msgServer +} + // QueryServer implements the Configurator.QueryServer method func (c configurator) QueryServer() grpc.Server { return c.queryServer diff --git a/types/module/module_test.go b/types/module/module_test.go index 67eac1695587..cc331cf58f2d 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -181,8 +181,9 @@ func TestManager_RegisterQueryServices(t *testing.T) { require.NotNil(t, mm) require.Equal(t, 2, len(mm.Modules)) + msgRouter := mocks.NewMockServer(mockCtrl) queryRouter := mocks.NewMockServer(mockCtrl) - cfg := module.NewConfigurator(queryRouter) + cfg := module.NewConfigurator(msgRouter, queryRouter) mockAppModule1.EXPECT().RegisterServices(cfg).Times(1) mockAppModule2.EXPECT().RegisterServices(cfg).Times(1) diff --git a/x/auth/vesting/module.go b/x/auth/vesting/module.go index b4997d60e4bd..7afab8d34b60 100644 --- a/x/auth/vesting/module.go +++ b/x/auth/vesting/module.go @@ -100,8 +100,10 @@ func (am AppModule) Route() sdk.Route { // functionality. func (AppModule) QuerierRoute() string { return "" } -// RegisterQueryService performs a no-op. -func (am AppModule) RegisterServices(_ module.Configurator) {} +// RegisterServices registers module services. +func (am AppModule) RegisterServices(cfg module.Configurator) { + types.RegisterMsgServer(cfg.MsgServer(), NewMsgServerImpl(am.accountKeeper, am.bankKeeper)) +} // LegacyQuerierHandler performs a no-op. func (am AppModule) LegacyQuerierHandler(_ *codec.LegacyAmino) sdk.Querier { diff --git a/x/bank/client/cli/cli_test.go b/x/bank/client/cli/cli_test.go index 67f967b44a38..c68be4ccd348 100644 --- a/x/bank/client/cli/cli_test.go +++ b/x/bank/client/cli/cli_test.go @@ -5,6 +5,13 @@ import ( "fmt" "testing" + "github.com/spf13/cobra" + + "github.com/cosmos/cosmos-sdk/client/tx" + + "github.com/gogo/protobuf/grpc" + grpc2 "google.golang.org/grpc" + "github.com/gogo/protobuf/proto" "github.com/stretchr/testify/suite" tmcli "github.com/tendermint/tendermint/libs/cli" @@ -295,6 +302,141 @@ func (s *IntegrationTestSuite) TestNewSendTxCmd() { } } +// serviceMsgClientConn is an instance of grpc.ClientConn that is used to test building +// transactions with MsgClient's. It is intended to be replaced by the work in +// https://github.com/cosmos/cosmos-sdk/issues/7541 when that is ready. +type serviceMsgClientConn struct { + msgs []sdk.Msg +} + +func (t *serviceMsgClientConn) Invoke(_ context.Context, method string, args, _ interface{}, _ ...grpc2.CallOption) error { + req, ok := args.(sdk.MsgRequest) + if !ok { + return fmt.Errorf("%T should implement %T", args, (*sdk.MsgRequest)(nil)) + } + + err := req.ValidateBasic() + if err != nil { + return err + } + + t.msgs = append(t.msgs, sdk.ServiceMsg{ + MethodName: method, + Request: req, + }) + + return nil +} + +func (t *serviceMsgClientConn) NewStream(context.Context, *grpc2.StreamDesc, string, ...grpc2.CallOption) (grpc2.ClientStream, error) { + return nil, fmt.Errorf("not supported") +} + +var _ grpc.ClientConn = &serviceMsgClientConn{} + +// newSendTxMsgServiceCmd is just for the purpose of testing ServiceMsg's in an end-to-end case. It is effectively +// NewSendTxCmd but using MsgClient. +func newSendTxMsgServiceCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "send [from_key_or_address] [to_address] [amount]", + Short: `Send funds from one account to another. Note, the'--from' flag is +ignored as it is implied from [from_key_or_address].`, + Args: cobra.ExactArgs(3), + RunE: func(cmd *cobra.Command, args []string) error { + cmd.Flags().Set(flags.FlagFrom, args[0]) + + clientCtx := client.GetClientContextFromCmd(cmd) + clientCtx, err := client.ReadTxCommandFlags(clientCtx, cmd.Flags()) + if err != nil { + return err + } + + toAddr, err := sdk.AccAddressFromBech32(args[1]) + if err != nil { + return err + } + + coins, err := sdk.ParseCoins(args[2]) + if err != nil { + return err + } + + msg := types.NewMsgSend(clientCtx.GetFromAddress(), toAddr, coins) + svcMsgClientConn := &serviceMsgClientConn{} + bankMsgClient := types.NewMsgClient(svcMsgClientConn) + _, err = bankMsgClient.Send(context.Background(), msg) + if err != nil { + return err + } + + return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), svcMsgClientConn.msgs...) + }, + } + + flags.AddTxFlagsToCmd(cmd) + + return cmd +} + +// TestBankMsgService does a basic test of whether or not service Msg's as defined +// in ADR 031 work in the most basic end-to-end case. +func (s *IntegrationTestSuite) TestBankMsgService() { + val := s.network.Validators[0] + + testCases := []struct { + name string + from, to sdk.AccAddress + amount sdk.Coins + args []string + expectErr bool + respType proto.Message + expectedCode uint32 + rawLogContains string + }{ + { + "valid transaction", + val.Address, + val.Address, + sdk.NewCoins( + sdk.NewCoin(fmt.Sprintf("%stoken", val.Moniker), sdk.NewInt(10)), + sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10)), + ), + []string{ + fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), + fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock), + fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()), + }, + false, + &sdk.TxResponse{}, + 0, + "/cosmos.bank.v1beta1.Msg/Send", // indicates we are using ServiceMsg and not a regular Msg + }, + } + + for _, tc := range testCases { + tc := tc + + s.Run(tc.name, func() { + clientCtx := val.ClientCtx + + args := []string{tc.from.String(), tc.to.String(), tc.amount.String()} + args = append(args, tc.args...) + + bz, err := clitestutil.ExecTestCLICmd(clientCtx, newSendTxMsgServiceCmd(), args) + if tc.expectErr { + s.Require().Error(err) + } else { + s.Require().NoError(err) + + s.Require().NoError(clientCtx.JSONMarshaler.UnmarshalJSON(bz.Bytes(), tc.respType), bz.String()) + txResp := tc.respType.(*sdk.TxResponse) + s.Require().Equal(tc.expectedCode, txResp.Code) + s.Require().Contains(txResp.RawLog, tc.rawLogContains) + } + }) + } +} + func TestIntegrationTestSuite(t *testing.T) { suite.Run(t, new(IntegrationTestSuite)) } diff --git a/x/bank/module.go b/x/bank/module.go index 1b3d68f4607d..9712636133da 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -94,9 +94,9 @@ type AppModule struct { accountKeeper types.AccountKeeper } -// RegisterQueryService registers a GRPC query service to respond to the -// module-specific GRPC queries. +// RegisterServices registers module services. func (am AppModule) RegisterServices(cfg module.Configurator) { + types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) types.RegisterQueryServer(cfg.QueryServer(), am.keeper) } diff --git a/x/crisis/module.go b/x/crisis/module.go index fdc2ffb03d6f..4066cb2ef9d7 100644 --- a/x/crisis/module.go +++ b/x/crisis/module.go @@ -112,9 +112,10 @@ func (AppModule) QuerierRoute() string { return "" } // LegacyQuerierHandler returns no sdk.Querier. func (AppModule) LegacyQuerierHandler(*codec.LegacyAmino) sdk.Querier { return nil } -// RegisterQueryService registers a GRPC query service to respond to the -// module-specific GRPC queries. -func (am AppModule) RegisterServices(module.Configurator) {} +// RegisterServices registers module services. +func (am AppModule) RegisterServices(cfg module.Configurator) { + types.RegisterMsgServer(cfg.MsgServer(), am.keeper) +} // InitGenesis performs genesis initialization for the crisis module. It returns // no validator updates. diff --git a/x/distribution/module.go b/x/distribution/module.go index 9c53d5619170..c32bcbc1ed41 100644 --- a/x/distribution/module.go +++ b/x/distribution/module.go @@ -139,9 +139,9 @@ func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc *codec.LegacyAmino) sd return keeper.NewQuerier(am.keeper, legacyQuerierCdc) } -// RegisterQueryService registers a GRPC query service to respond to the -// module-specific GRPC queries. +// RegisterServices registers module services. func (am AppModule) RegisterServices(cfg module.Configurator) { + types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) types.RegisterQueryServer(cfg.QueryServer(), am.keeper) } diff --git a/x/evidence/module.go b/x/evidence/module.go index 1ac32c2148b7..7ff49aabd583 100644 --- a/x/evidence/module.go +++ b/x/evidence/module.go @@ -148,9 +148,9 @@ func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc *codec.LegacyAmino) sd return keeper.NewQuerier(am.keeper, legacyQuerierCdc) } -// RegisterQueryService registers a GRPC query service to respond to the -// module-specific GRPC queries. +// RegisterServices registers module services. func (am AppModule) RegisterServices(cfg module.Configurator) { + types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) types.RegisterQueryServer(cfg.QueryServer(), am.keeper) } diff --git a/x/gov/module.go b/x/gov/module.go index 31c0e5c05d80..1183f6060e72 100644 --- a/x/gov/module.go +++ b/x/gov/module.go @@ -155,9 +155,9 @@ func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc *codec.LegacyAmino) sd return keeper.NewQuerier(am.keeper, legacyQuerierCdc) } -// RegisterQueryService registers a GRPC query service to respond to the -// module-specific GRPC queries. +// RegisterServices registers module services. func (am AppModule) RegisterServices(cfg module.Configurator) { + types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) types.RegisterQueryServer(cfg.QueryServer(), am.keeper) } diff --git a/x/slashing/module.go b/x/slashing/module.go index f74c4a84dfd2..3698bb6987de 100644 --- a/x/slashing/module.go +++ b/x/slashing/module.go @@ -137,9 +137,9 @@ func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc *codec.LegacyAmino) sd return keeper.NewQuerier(am.keeper, legacyQuerierCdc) } -// RegisterQueryService registers a GRPC query service to respond to the -// module-specific GRPC queries. +// RegisterServices registers module services. func (am AppModule) RegisterServices(cfg module.Configurator) { + types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) types.RegisterQueryServer(cfg.QueryServer(), am.keeper) } diff --git a/x/staking/module.go b/x/staking/module.go index 6a344265b74a..4fa17d4d45d8 100644 --- a/x/staking/module.go +++ b/x/staking/module.go @@ -133,9 +133,9 @@ func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc *codec.LegacyAmino) sd return keeper.NewQuerier(am.keeper, legacyQuerierCdc) } -// RegisterQueryService registers a GRPC query service to respond to the -// module-specific GRPC queries. +// RegisterServices registers module services. func (am AppModule) RegisterServices(cfg module.Configurator) { + types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) querier := keeper.Querier{Keeper: am.keeper} types.RegisterQueryServer(cfg.QueryServer(), querier) }