Use service Msgs in CLI tx commands (#8512)

* Use service Msgs in CLI tx commands

* Update comment

* Gracefully support amino signing

* CLI to use svc msg

* Fix lint

* Use fq method name in events

* Update tests

* Fix quering events

* Add docs

* Fix test build

* Fix events

* Fix search for events

* Handle old andd new event quering

* Use merge events

* Better encCfg

* Add page in search

* Fix tests

* Update test and comments

* Update x/auth/legacy/legacytx/stdsign.go

Co-authored-by: atheeshp <59333759+atheeshp@users.noreply.github.com>

* Fix conflict for weighted vote

* Make CopyTx actually run

* Fix CopyTx

* Fix test

* Add changelog entry

* Remove useless code

* Add msgs tests

* Remove testing proto Msg via CLI

* Fix lint

* Fix test

* Implement GetSignBytes on ServiceMsg

* Fix %T

Co-authored-by: atheeshp <59333759+atheeshp@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

CHANGELOG.md

@@ -38,7 +38,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Client Breaking Changes
 
-* [\#8363](https://github.com/cosmos/cosmos-sdk/issues/8363) Addresses no longer have a fixed 20-byte length. From the SDK modules' point of view, any 1-255 bytes-long byte array is a valid address.
+* [\#8363](https://github.com/cosmos/cosmos-sdk/pull/8363) Addresses no longer have a fixed 20-byte length. From the SDK modules' point of view, any 1-255 bytes-long byte array is a valid address.
+* [\#8346](https://github.com/cosmos/cosmos-sdk/pull/8346) All CLI `tx` commands generate ServiceMsgs by default. Graceful Amino support has been added to ServiceMsgs to support signing legacy Msgs.
 
 ### API Breaking Changes
 

client/tx/legacy_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/simapp/params"
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
 	"github.com/cosmos/cosmos-sdk/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	signing2 "github.com/cosmos/cosmos-sdk/types/tx/signing"
 	"github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx"
 	"github.com/cosmos/cosmos-sdk/x/auth/signing"
@@ -23,28 +24,32 @@ import (
 const (
 	memo          = "waboom"
 	gas           = uint64(10000)
-	timeoutHeight = 5
+	timeoutHeight = uint64(5)
 )
 
 var (
 	fee            = types.NewCoins(types.NewInt64Coin("bam", 100))
 	_, pub1, addr1 = testdata.KeyTestPubAddr()
 	_, _, addr2    = testdata.KeyTestPubAddr()
-	msg            = banktypes.NewMsgSend(addr1, addr2, types.NewCoins(types.NewInt64Coin("wack", 10000)))
 	sig            = signing2.SignatureV2{
 		PubKey: pub1,
 		Data: &signing2.SingleSignatureData{
 			SignMode:  signing2.SignMode_SIGN_MODE_LEGACY_AMINO_JSON,
 			Signature: []byte("dummy"),
 		},
 	}
+	msg0 = banktypes.NewMsgSend(addr1, addr2, types.NewCoins(types.NewInt64Coin("wack", 1)))
+	msg1 = sdk.ServiceMsg{
+		MethodName: "/cosmos.bank.v1beta1.Msg/Send",
+		Request:    banktypes.NewMsgSend(addr1, addr2, types.NewCoins(types.NewInt64Coin("wack", 2))),
+	}
 )
 
 func buildTestTx(t *testing.T, builder client.TxBuilder) {
 	builder.SetMemo(memo)
 	builder.SetGasLimit(gas)
 	builder.SetFeeAmount(fee)
-	err := builder.SetMsgs(msg)
+	err := builder.SetMsgs(msg0, msg1)
 	require.NoError(t, err)
 	err = builder.SetSignatures(sig)
 	require.NoError(t, err)
@@ -75,11 +80,15 @@ func (s *TestSuite) TestCopyTx() {
 	protoBuilder2 := s.protoCfg.NewTxBuilder()
 	err = tx2.CopyTx(aminoBuilder.GetTx(), protoBuilder2, false)
 	s.Require().NoError(err)
-	bz, err := s.protoCfg.TxEncoder()(protoBuilder.GetTx())
+	// Check sigs, signers and msgs.
+	sigsV2_1, err := protoBuilder.GetTx().GetSignaturesV2()
 	s.Require().NoError(err)
-	bz2, err := s.protoCfg.TxEncoder()(protoBuilder2.GetTx())
+	sigsV2_2, err := protoBuilder2.GetTx().GetSignaturesV2()
 	s.Require().NoError(err)
-	s.Require().Equal(bz, bz2)
+	s.Require().Equal(sigsV2_1, sigsV2_2)
+	s.Require().Equal(protoBuilder.GetTx().GetSigners(), protoBuilder2.GetTx().GetSigners())
+	s.Require().Equal(protoBuilder.GetTx().GetMsgs()[0], protoBuilder2.GetTx().GetMsgs()[0])
+	s.Require().Equal(protoBuilder.GetTx().GetMsgs()[1].(sdk.ServiceMsg).Request, protoBuilder2.GetTx().GetMsgs()[1]) // We lose the "ServiceMsg" information
 
 	// amino -> proto -> amino
 	aminoBuilder = s.aminoCfg.NewTxBuilder()
@@ -90,11 +99,15 @@ func (s *TestSuite) TestCopyTx() {
 	aminoBuilder2 := s.aminoCfg.NewTxBuilder()
 	err = tx2.CopyTx(protoBuilder.GetTx(), aminoBuilder2, false)
 	s.Require().NoError(err)
-	bz, err = s.aminoCfg.TxEncoder()(aminoBuilder.GetTx())
+	// Check sigs, signers, and msgs
+	sigsV2_1, err = aminoBuilder.GetTx().GetSignaturesV2()
 	s.Require().NoError(err)
-	bz2, err = s.aminoCfg.TxEncoder()(aminoBuilder2.GetTx())
+	sigsV2_2, err = aminoBuilder2.GetTx().GetSignaturesV2()
 	s.Require().NoError(err)
-	s.Require().Equal(bz, bz2)
+	s.Require().Equal(sigsV2_1, sigsV2_2)
+	s.Require().Equal(aminoBuilder.GetTx().GetSigners(), aminoBuilder2.GetTx().GetSigners())
+	s.Require().Equal(aminoBuilder.GetTx().GetMsgs()[0], aminoBuilder2.GetTx().GetMsgs()[0])
+	s.Require().Equal(aminoBuilder.GetTx().GetMsgs()[1], aminoBuilder2.GetTx().GetMsgs()[1]) // We lose the "ServiceMsg" information
 }
 
 func (s *TestSuite) TestConvertTxToStdTx() {
@@ -106,7 +119,8 @@ func (s *TestSuite) TestConvertTxToStdTx() {
 	s.Require().Equal(memo, stdTx.Memo)
 	s.Require().Equal(gas, stdTx.Fee.Gas)
 	s.Require().Equal(fee, stdTx.Fee.Amount)
-	s.Require().Equal(msg, stdTx.Msgs[0])
+	s.Require().Equal(msg0, stdTx.Msgs[0])
+	s.Require().Equal(msg1.Request, stdTx.Msgs[1])
 	s.Require().Equal(timeoutHeight, stdTx.TimeoutHeight)
 	s.Require().Equal(sig.PubKey, stdTx.Signatures[0].PubKey)
 	s.Require().Equal(sig.Data.(*signing2.SingleSignatureData).Signature, stdTx.Signatures[0].Signature)
@@ -125,7 +139,8 @@ func (s *TestSuite) TestConvertTxToStdTx() {
 	s.Require().Equal(memo, stdTx.Memo)
 	s.Require().Equal(gas, stdTx.Fee.Gas)
 	s.Require().Equal(fee, stdTx.Fee.Amount)
-	s.Require().Equal(msg, stdTx.Msgs[0])
+	s.Require().Equal(msg0, stdTx.Msgs[0])
+	s.Require().Equal(msg1.Request, stdTx.Msgs[1])
 	s.Require().Equal(timeoutHeight, stdTx.TimeoutHeight)
 	s.Require().Empty(stdTx.Signatures)
 
@@ -158,3 +173,7 @@ func (s *TestSuite) TestConvertAndEncodeStdTx() {
 	s.Require().NoError(err)
 	s.Require().Equal(stdTx, decodedTx)
 }
+
+func TestTestSuite(t *testing.T) {
+	suite.Run(t, new(TestSuite))
+}

docs/core/events.md

@@ -19,7 +19,7 @@ take the form of: `{eventType}.{eventAttribute}={value}`.
 
 Events contain:
 
-- A `type`, which is meant to categorize an event at a high-level (e.g. by module or action).
+- A `type`, which is meant to categorize an event at a high-level (e.g. by module (e.g. `module=bank`) or action (e.g. `action=/cosmos.bank.v1beta1.Msg/Send`)).
 - A list of `attributes`, which are key-value pairs that give more information about
   the categorized `event`.
   +++ https://github.com/cosmos/cosmos-sdk/blob/7d7821b9af132b0f6131640195326aa02b6751db/types/events.go#L51-L56

server/grpc/server_test.go

@@ -13,15 +13,13 @@ import (
 	"google.golang.org/grpc/metadata"
 	rpb "google.golang.org/grpc/reflection/grpc_reflection_v1alpha"
 
-	clienttx "github.com/cosmos/cosmos-sdk/client/tx"
 	"github.com/cosmos/cosmos-sdk/testutil/network"
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	grpctypes "github.com/cosmos/cosmos-sdk/types/grpc"
 	"github.com/cosmos/cosmos-sdk/types/tx"
 	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"
+	banktestutil "github.com/cosmos/cosmos-sdk/x/bank/client/testutil"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 )
 
@@ -37,6 +35,7 @@ func (s *IntegrationTestSuite) SetupSuite() {
 	s.T().Log("setting up integration test suite")
 
 	s.cfg = network.DefaultConfig()
+	s.cfg.NumValidators = 1
 	s.network = network.New(s.T(), s.cfg)
 	s.Require().NotNil(s.network)
 
@@ -130,42 +129,9 @@ 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()
-	s.Require().NoError(
-		txBuilder.SetMsgs(&banktypes.MsgSend{
-			FromAddress: val0.Address.String(),
-			ToAddress:   val0.Address.String(),
-			Amount:      sdk.Coins{sdk.NewInt64Coin(s.cfg.BondDenom, 10)},
-		}),
-	)
-	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)
-
-	txBytes, err := val0.ClientCtx.TxConfig.TxEncoder()(txBuilder.GetTx())
-	s.Require().NoError(err)
-
-	// Broadcast the tx via gRPC.
-	queryClient := tx.NewServiceClient(s.conn)
-	grpcRes, err := queryClient.BroadcastTx(
-		context.Background(),
-		&tx.BroadcastTxRequest{
-			Mode:    tx.BroadcastMode_BROADCAST_MODE_SYNC,
-			TxBytes: txBytes,
-		},
+	grpcRes, err := banktestutil.LegacyGRPCProtoMsgSend(val0.ClientCtx,
+		val0.Moniker, val0.Address, val0.Address,
+		sdk.Coins{sdk.NewInt64Coin(s.cfg.BondDenom, 10)}, sdk.Coins{sdk.NewInt64Coin(s.cfg.BondDenom, 10)},
 	)
 	s.Require().NoError(err)
 	s.Require().Equal(uint32(0), grpcRes.TxResponse.Code)

types/service_msg.go

@@ -1,6 +1,8 @@
 package types
 
 import (
+	"fmt"
+
 	"github.com/gogo/protobuf/proto"
 )
 
@@ -18,7 +20,7 @@ type MsgRequest interface {
 }
 
 // ServiceMsg is the struct into which an Any whose typeUrl matches a service
-// method format (ex. `/cosmos.gov.Msg/SubmitProposal`) unpacks.
+// method format (ex. `/cosmos.gov.v1beta1.Msg/SubmitProposal`) unpacks.
 type ServiceMsg struct {
 	// MethodName is the fully-qualified service method name.
 	MethodName string
@@ -44,7 +46,17 @@ func (msg ServiceMsg) ValidateBasic() error {
 
 // GetSignBytes implements Msg.GetSignBytes method.
 func (msg ServiceMsg) GetSignBytes() []byte {
-	panic("ServiceMsg does not have a GetSignBytes method")
+	// Here, we're gracefully supporting Amino JSON for service
+	// Msgs.
+	// ref: https://github.com/cosmos/cosmos-sdk/issues/8346
+	// If `msg` is a service Msg, then we cast its `Request` to a sdk.Msg
+	// and call GetSignBytes on the `Request`.
+	msgRequest, ok := msg.Request.(Msg)
+	if !ok {
+		panic(fmt.Errorf("cannot convert ServiceMsg request to sdk.Msg, got %T", msgRequest))
+	}
+
+	return msgRequest.GetSignBytes()
 }
 
 // GetSigners implements Msg.GetSigners method.

x/auth/client/cli/cli_test.go

@@ -230,23 +230,19 @@ func (s *IntegrationTestSuite) TestCLIQueryTxCmd() {
 	sendTokens := sdk.NewInt64Coin(s.cfg.BondDenom, 10)
 
 	// Send coins, try both with legacy Msg and with Msg service.
-	// Legacy Msg.
-	legacyMsgOut, err := bankcli.MsgSendExec(
-		val.ClientCtx,
+	// Legacy proto Msg.
+	legacyTxRes, err := bankcli.LegacyGRPCProtoMsgSend(
+		val.ClientCtx, val.Moniker,
 		val.Address,
 		account2.GetAddress(),
+		sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))),
 		sdk.NewCoins(sendTokens),
-		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()),
-		fmt.Sprintf("--gas=%d", flags.DefaultGasLimit),
 	)
 	s.Require().NoError(err)
-	var legacyMsgTxRes sdk.TxResponse
-	s.Require().NoError(val.ClientCtx.JSONMarshaler.UnmarshalJSON(legacyMsgOut.Bytes(), &legacyMsgTxRes))
+	s.Require().NoError(s.network.WaitForNextBlock())
 
 	// Service Msg.
-	out, err := bankcli.ServiceMsgSendExec(
+	out, err := bankcli.MsgSendExec(
 		val.ClientCtx,
 		val.Address,
 		account2.GetAddress(),
@@ -259,7 +255,6 @@ func (s *IntegrationTestSuite) TestCLIQueryTxCmd() {
 	s.Require().NoError(err)
 	var txRes sdk.TxResponse
 	s.Require().NoError(val.ClientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), &txRes))
-
 	s.Require().NoError(s.network.WaitForNextBlock())
 
 	testCases := []struct {
@@ -282,7 +277,7 @@ func (s *IntegrationTestSuite) TestCLIQueryTxCmd() {
 		},
 		{
 			"happy case (legacy Msg)",
-			[]string{legacyMsgTxRes.TxHash, fmt.Sprintf("--%s=json", tmcli.OutputFlag)},
+			[]string{legacyTxRes.TxResponse.TxHash, fmt.Sprintf("--%s=json", tmcli.OutputFlag)},
 			false,
 			"",
 		},

x/auth/client/rest/rest_test.go

@@ -269,7 +269,7 @@ func (s *IntegrationTestSuite) TestQueryTxWithStdTx() {
 	s.testQueryTx(s.stdTxRes.Height, s.stdTxRes.TxHash, val0.Address.String())
 }
 
-func (s *IntegrationTestSuite) TestQueryTxWithServiceMessage() {
+func (s *IntegrationTestSuite) TestQueryTxWithServiceMsg() {
 	val := s.network.Validators[0]
 
 	sendTokens := sdk.NewInt64Coin(s.cfg.BondDenom, 10)
@@ -278,7 +278,7 @@ func (s *IntegrationTestSuite) TestQueryTxWithServiceMessage() {
 	// Might need to wait a block to refresh sequences from previous setups.
 	s.Require().NoError(s.network.WaitForNextBlock())
 
-	out, err := bankcli.ServiceMsgSendExec(
+	out, err := bankcli.MsgSendExec(
 		val.ClientCtx,
 		val.Address,
 		addr,

x/auth/legacy/legacytx/stdsign.go

@@ -35,6 +35,9 @@ type StdSignDoc struct {
 func StdSignBytes(chainID string, accnum, sequence, timeout uint64, fee StdFee, msgs []sdk.Msg, memo string) []byte {
 	msgsBytes := make([]json.RawMessage, 0, len(msgs))
 	for _, msg := range msgs {
+		// If msg is a legacy Msg, then GetSignBytes is implemented.
+		// If msg is a ServiceMsg, then GetSignBytes has graceful support of
+		// calling GetSignBytes from its underlying Msg.
 		msgsBytes = append(msgsBytes, json.RawMessage(msg.GetSignBytes()))
 	}
 

x/auth/tx/service_test.go

@@ -175,14 +175,14 @@ func (s IntegrationTestSuite) TestGetTxEvents_GRPC() {
 		{
 			"without pagination",
 			&tx.GetTxsEventRequest{
-				Events: []string{"message.action=send"},
+				Events: []string{"message.action=/cosmos.bank.v1beta1.Msg/Send"},
 			},
 			false, "",
 		},
 		{
 			"with pagination",
 			&tx.GetTxsEventRequest{
-				Events: []string{"message.action=send"},
+				Events: []string{"message.action=/cosmos.bank.v1beta1.Msg/Send"},
 				Pagination: &query.PageRequest{
 					CountTotal: false,
 					Offset:     0,
@@ -194,7 +194,7 @@ func (s IntegrationTestSuite) TestGetTxEvents_GRPC() {
 		{
 			"with multi events",
 			&tx.GetTxsEventRequest{
-				Events: []string{"message.action=send", "message.module=bank"},
+				Events: []string{"message.action=/cosmos.bank.v1beta1.Msg/Send", "message.module=bank"},
 			},
 			false, "",
 		},
@@ -231,25 +231,25 @@ func (s IntegrationTestSuite) TestGetTxEvents_GRPCGateway() {
 		},
 		{
 			"without pagination",
-			fmt.Sprintf("%s/cosmos/tx/v1beta1/txs?events=%s", val.APIAddress, "message.action=send"),
+			fmt.Sprintf("%s/cosmos/tx/v1beta1/txs?events=%s", val.APIAddress, "message.action=/cosmos.bank.v1beta1.Msg/Send"),
 			false,
 			"",
 		},
 		{
 			"with pagination",
-			fmt.Sprintf("%s/cosmos/tx/v1beta1/txs?events=%s&pagination.offset=%d&pagination.limit=%d", val.APIAddress, "message.action=send", 0, 10),
+			fmt.Sprintf("%s/cosmos/tx/v1beta1/txs?events=%s&pagination.offset=%d&pagination.limit=%d", val.APIAddress, "message.action=/cosmos.bank.v1beta1.Msg/Send", 0, 10),
 			false,
 			"",
 		},
 		{
 			"expect pass with multiple-events",
-			fmt.Sprintf("%s/cosmos/tx/v1beta1/txs?events=%s&events=%s", val.APIAddress, "message.action=send", "message.module=bank"),
+			fmt.Sprintf("%s/cosmos/tx/v1beta1/txs?events=%s&events=%s", val.APIAddress, "message.action=/cosmos.bank.v1beta1.Msg/Send", "message.module=bank"),
 			false,
 			"",
 		},
 		{
 			"expect pass with escape event",
-			fmt.Sprintf("%s/cosmos/tx/v1beta1/txs?events=%s", val.APIAddress, "message.action%3Dsend"),
+			fmt.Sprintf("%s/cosmos/tx/v1beta1/txs?events=%s", val.APIAddress, "message.action%3D%2Fcosmos.bank.v1beta1.Msg%2FSend"),
 			false,
 			"",
 		},

x/auth/vesting/client/cli/tx.go

@@ -9,6 +9,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/client/tx"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/msgservice"
 	"github.com/cosmos/cosmos-sdk/x/auth/vesting/types"
 )
 
@@ -69,11 +70,14 @@ timestamp.`,
 			delayed, _ := cmd.Flags().GetBool(FlagDelayed)
 
 			msg := types.NewMsgCreateVestingAccount(clientCtx.GetFromAddress(), toAddr, amount, endTime, delayed)
-			if err := msg.ValidateBasic(); err != nil {
+			svcMsgClientConn := &msgservice.ServiceMsgClientConn{}
+			msgClient := types.NewMsgClient(svcMsgClientConn)
+			_, err = msgClient.CreateVestingAccount(cmd.Context(), msg)
+			if err != nil {
 				return err
 			}
 
-			return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
+			return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), svcMsgClientConn.GetMsgs()...)
 		},
 	}