Address reviews

CHANGELOG.md

@@ -60,7 +60,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
   * CLI: removed `--text` flag from `show-node-id` command; the text format for public keys is not used any more - instead we use ProtoJSON.
 * (types) [\#9079](https://github.com/cosmos/cosmos-sdk/issues/9079) Add `AddAmount`/`SubAmount` methods to `sdk.Coin`.
 * [\#8628](https://github.com/cosmos/cosmos-sdk/issues/8628) Commands no longer print outputs using `stderr` by default
-* [\#9139](https://github.com/cosmos/cosmos-sdk/pull/9139) Querying events via `ServiceMsg` TypeURLs (e.g. `/cosmos.bank.v1beta1.Msg/Send`) does not work anymore, please use concrete `Msg` TypeURLs instead (e.g. `/cosmos.bank.v1beta1/MsgSend`).
+* [\#9139](https://github.com/cosmos/cosmos-sdk/pull/9139) Querying events via `ServiceMsg` TypeURLs (e.g. `/cosmos.bank.v1beta1.Msg/Send`) does not work anymore, please use concrete `Msg` TypeURLs instead (e.g. `/cosmos.bank.v1beta1.MsgSend`).
 
 ### API Breaking Changes
 

baseapp/baseapp.go

@@ -704,19 +704,23 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*s
 		}
 
 		var (
-			msgResult      *sdk.Result
-			msgEventAction string // name to use as value in event `message.action`
-			err            error
+			msgResult    *sdk.Result
+			eventMsgName string // name to use as value in event `message.action`
+			err          error
 		)
 
 		if handler := app.msgServiceRouter.Handler(msg); handler != nil {
 			// ADR 031 request type routing
 			msgResult, err = handler(ctx, msg)
-			msgEventAction = sdk.MsgTypeURL(msg)
+			eventMsgName = sdk.MsgTypeURL(msg)
 		} else if legacyMsg, ok := msg.(legacytx.LegacyMsg); ok {
 			// legacy sdk.Msg routing
+			// Assuming that the app developer has migrated all their Msgs to
+			// proto messages and has registered all `Msg services`, then this
+			// path should never be called, because all those Msgs should be
+			// registered within the `msgServiceRouter` already.
 			msgRoute := legacyMsg.Route()
-			msgEventAction = legacyMsg.Type()
+			eventMsgName = legacyMsg.Type()
 			handler := app.router.Route(ctx, msgRoute)
 			if handler == nil {
 				return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized message route: %s; message index: %d", msgRoute, i)
@@ -732,7 +736,7 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*s
 		}
 
 		msgEvents := sdk.Events{
-			sdk.NewEvent(sdk.EventTypeMessage, sdk.NewAttribute(sdk.AttributeKeyAction, msgEventAction)),
+			sdk.NewEvent(sdk.EventTypeMessage, sdk.NewAttribute(sdk.AttributeKeyAction, eventMsgName)),
 		}
 		msgEvents = msgEvents.AppendEvents(msgResult.GetEvents())
 

baseapp/msg_service_router.go

@@ -71,10 +71,7 @@ func (msr *MsgServiceRouter) RegisterService(sd *grpc.ServiceDesc, handler inter
 
 			requestTypeName = sdk.MsgTypeURL(msg)
 			return nil
-		}, func(_ context.Context, _ interface{}, _ *grpc.UnaryServerInfo, _ grpc.UnaryHandler) (interface{}, error) {
-			return nil, nil
-		},
-		)
+		}, noopInterceptor)
 
 		// Check that the service Msg fully-qualified method name has already
 		// been registered (via RegisterInterfaces). If the user registers a
@@ -109,7 +106,7 @@ func (msr *MsgServiceRouter) RegisterService(sd *grpc.ServiceDesc, handler inter
 			)
 		}
 
-		handler := func(ctx sdk.Context, req sdk.Msg) (*sdk.Result, error) {
+		msr.routes[requestTypeName] = func(ctx sdk.Context, req sdk.Msg) (*sdk.Result, error) {
 			ctx = ctx.WithEventManager(sdk.NewEventManager())
 			interceptor := func(goCtx context.Context, _ interface{}, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
 				goCtx = context.WithValue(goCtx, sdk.SdkContextKey, ctx)
@@ -129,8 +126,6 @@ func (msr *MsgServiceRouter) RegisterService(sd *grpc.ServiceDesc, handler inter
 
 			return sdk.WrapServiceResult(ctx, resMsg, err)
 		}
-
-		msr.routes[requestTypeName] = handler
 	}
 }
 
@@ -140,3 +135,6 @@ func (msr *MsgServiceRouter) SetInterfaceRegistry(interfaceRegistry codectypes.I
 }
 
 func noopDecoder(_ interface{}) error { return nil }
+func noopInterceptor(_ context.Context, _ interface{}, _ *grpc.UnaryServerInfo, _ grpc.UnaryHandler) (interface{}, error) {
+	return nil, nil
+}

server/grpc/server_test.go

@@ -27,7 +27,6 @@ import (
 	txtypes "github.com/cosmos/cosmos-sdk/types/tx"
 	"github.com/cosmos/cosmos-sdk/types/tx/signing"
 	authclient "github.com/cosmos/cosmos-sdk/x/auth/client"
-	"github.com/cosmos/cosmos-sdk/x/bank/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 )
 
@@ -164,32 +163,7 @@ func (s *IntegrationTestSuite) TestGRPCServer_GetTxsEvent() {
 func (s *IntegrationTestSuite) TestGRPCServer_BroadcastTx() {
 	val0 := s.network.Validators[0]
 
-	// prepare txBuilder with msg
-	txBuilder := val0.ClientCtx.TxConfig.NewTxBuilder()
-	feeAmount := sdk.Coins{sdk.NewInt64Coin(s.cfg.BondDenom, 10)}
-	gasLimit := testdata.NewTestGasLimit()
-
-	// This sets a legacy Proto MsgSend.
-	err := txBuilder.SetMsgs(&types.MsgSend{
-		FromAddress: val0.Address.String(),
-		ToAddress:   val0.Address.String(),
-		Amount:      sdk.Coins{sdk.NewInt64Coin(s.cfg.BondDenom, 10)},
-	})
-	s.Require().NoError(err)
-
-	txBuilder.SetFeeAmount(feeAmount)
-	txBuilder.SetGasLimit(gasLimit)
-
-	// setup txFactory
-	txFactory := clienttx.Factory{}.
-		WithChainID(val0.ClientCtx.ChainID).
-		WithKeybase(val0.ClientCtx.Keyring).
-		WithTxConfig(val0.ClientCtx.TxConfig).
-		WithSignMode(signing.SignMode_SIGN_MODE_DIRECT)
-
-	// Sign Tx.
-	err = authclient.SignTx(txFactory, val0.ClientCtx, val0.Moniker, txBuilder, false, true)
-	s.Require().NoError(err)
+	txBuilder := s.mkTxBuilder()
 
 	txBytes, err := val0.ClientCtx.TxConfig.TxEncoder()(txBuilder.GetTx())
 	s.Require().NoError(err)
@@ -237,6 +211,40 @@ func (s *IntegrationTestSuite) TestGRPCServerInvalidHeaderHeights() {
 	}
 }
 
+// mkTxBuilder creates a TxBuilder containing a signed tx from validator 0.
+func (s IntegrationTestSuite) mkTxBuilder() client.TxBuilder {
+	val := s.network.Validators[0]
+	s.Require().NoError(s.network.WaitForNextBlock())
+
+	// prepare txBuilder with msg
+	txBuilder := val.ClientCtx.TxConfig.NewTxBuilder()
+	feeAmount := sdk.Coins{sdk.NewInt64Coin(s.cfg.BondDenom, 10)}
+	gasLimit := testdata.NewTestGasLimit()
+	s.Require().NoError(
+		txBuilder.SetMsgs(&banktypes.MsgSend{
+			FromAddress: val.Address.String(),
+			ToAddress:   val.Address.String(),
+			Amount:      sdk.Coins{sdk.NewInt64Coin(s.cfg.BondDenom, 10)},
+		}),
+	)
+	txBuilder.SetFeeAmount(feeAmount)
+	txBuilder.SetGasLimit(gasLimit)
+	txBuilder.SetMemo("foobar")
+
+	// setup txFactory
+	txFactory := clienttx.Factory{}.
+		WithChainID(val.ClientCtx.ChainID).
+		WithKeybase(val.ClientCtx.Keyring).
+		WithTxConfig(val.ClientCtx.TxConfig).
+		WithSignMode(signing.SignMode_SIGN_MODE_DIRECT)
+
+	// Sign Tx.
+	err := authclient.SignTx(txFactory, val.ClientCtx, val.Moniker, txBuilder, false, true)
+	s.Require().NoError(err)
+
+	return txBuilder
+}
+
 func TestIntegrationTestSuite(t *testing.T) {
 	suite.Run(t, new(IntegrationTestSuite))
 }

types/tx/types.go

@@ -25,7 +25,11 @@ func (t *Tx) GetMsgs() []sdk.Msg {
 	anys := t.Body.Messages
 	res := make([]sdk.Msg, len(anys))
 	for i, any := range anys {
-		res[i] = any.GetCachedValue().(sdk.Msg)
+		cached := any.GetCachedValue()
+		if cached == nil {
+			panic("Any cached value is nil. Transaction messages must be correctly packed Any values.")
+		}
+		res[i] = cached.(sdk.Msg)
 	}
 	return res
 }

x/authz/client/rest/grpc_query_test.go

@@ -190,7 +190,7 @@ func (s *IntegrationTestSuite) TestQueryAuthorizationsGRPC() {
 			"",
 			func() {},
 			func(authorizations *types.QueryAuthorizationsResponse) {
-				s.Require().Equal(1, len(authorizations.Authorizations))
+				s.Require().Len(authorizations.Authorizations), 1)
 			},
 		},
 		{