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

@@ -359,7 +359,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{})

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