Add MsgServer to Configurator for ADR 031 wiring (#7584)

* Add MsgServer to Configurator for ADR 031 wiring

* Add docs, wire up evidence & staking

* Add integration test

* Add comments

* Doc strings

* Update types/module/configurator.go

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

* Update types/module/configurator.go

Co-authored-by: Cory <cjlevinson@gmail.com>

* Wire up vesting

* Update CHANGELOG.md

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: Amaury Martiny <amaury.martiny@protonmail.com>
Co-authored-by: Cory <cjlevinson@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

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
 

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{})

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()

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

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)
 

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 {

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))
 }

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)
 }
 

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.

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)
 }
 

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)
 }
 

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)
 }
 

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)
 }
 

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)
 }