Cleanup remains of ServiceMsg (cosmos#9236)

* Remove ServiceMsgClientConn from CLI * Cleanup Operations * Remove some legacytx.LegacyMsg * Update CL * Remove mentions of ServiceMsg * Simply reflection MsgDescriptor * Simplify more * Add code comment Co-authored-by: technicallyty <[email protected]>
eltociear · May 7, 2021 · 56ab4e4 · 56ab4e4
1 parent 9899ae0
commit 56ab4e4
Show file tree

Hide file tree

Showing 25 changed files with 168 additions and 860 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -53,7 +53,6 @@ if input key is empty, or input data contains empty key.
 ### Client Breaking Changes
 
 * [\#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.
 * (crypto/ed25519) [\#8690] Adopt zip1215 ed2559 verification rules.
 * [\#8849](https://github.com/cosmos/cosmos-sdk/pull/8849) Upgrade module no longer supports time based upgrades.
 * [\#8880](https://github.com/cosmos/cosmos-sdk/pull/8880) The CLI `simd migrate v0.40 ...` command has been renamed to `simd migrate v0.42`.

diff --git a/client/tx/legacy_test.go b/client/tx/legacy_test.go
@@ -103,7 +103,7 @@ func (s *TestSuite) TestCopyTx() {
 	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
+	s.Require().Equal(aminoBuilder.GetTx().GetMsgs()[1], aminoBuilder2.GetTx().GetMsgs()[1])
 }
 
 func (s *TestSuite) TestConvertTxToStdTx() {

diff --git a/client/tx/tx.go b/client/tx/tx.go
@@ -34,6 +34,16 @@ func GenerateOrBroadcastTxCLI(clientCtx client.Context, flagSet *pflag.FlagSet,
 // GenerateOrBroadcastTxWithFactory will either generate and print and unsigned transaction
 // or sign it and broadcast it returning an error upon failure.
 func GenerateOrBroadcastTxWithFactory(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error {
+	// Validate all msgs before generating or broadcasting the tx.
+	// We were calling ValidateBasic separately in each CLI handler before.
+	// Right now, we're factorizing that call inside this function.
+	// ref: https://github.com/cosmos/cosmos-sdk/pull/9236#discussion_r623803504
+	for _, msg := range msgs {
+		if err := msg.ValidateBasic(); err != nil {
+			return err
+		}
+	}
+
 	if clientCtx.GenerateOnly {
 		return GenerateTx(clientCtx, txf, msgs...)
 	}

diff --git a/docs/core/proto-docs.md b/docs/core/proto-docs.md
@@ -151,7 +151,6 @@
     - [QueryMethodDescriptor](#cosmos.base.reflection.v2alpha1.QueryMethodDescriptor)
     - [QueryServiceDescriptor](#cosmos.base.reflection.v2alpha1.QueryServiceDescriptor)
     - [QueryServicesDescriptor](#cosmos.base.reflection.v2alpha1.QueryServicesDescriptor)
-    - [ServiceMsgDescriptor](#cosmos.base.reflection.v2alpha1.ServiceMsgDescriptor)
     - [SigningModeDescriptor](#cosmos.base.reflection.v2alpha1.SigningModeDescriptor)
     - [TxDescriptor](#cosmos.base.reflection.v2alpha1.TxDescriptor)
 
@@ -2449,7 +2448,7 @@ MsgDescriptor describes a cosmos-sdk message that can be delivered with a transa
 
 | Field | Type | Label | Description |
 | ----- | ---- | ----- | ----------- |
-| `service_msg` | [ServiceMsgDescriptor](#cosmos.base.reflection.v2alpha1.ServiceMsgDescriptor) |  | service_msg is used when the message is an sdk.ServiceMsg type |
+| `msg_type_url` | [string](#string) |  | msg_type_url contains the TypeURL of a sdk.Msg. |
 
 
 
@@ -2506,24 +2505,6 @@ QueryServicesDescriptor contains the list of cosmos-sdk queriable services
 
 
 
-<a name="cosmos.base.reflection.v2alpha1.ServiceMsgDescriptor"></a>
-
-### ServiceMsgDescriptor
-ServiceMsgDescriptor describes an sdk.ServiceMsg type
-
-
-| Field | Type | Label | Description |
-| ----- | ---- | ----- | ----------- |
-| `request_fullname` | [string](#string) |  | request_fullname is the protobuf fullname of the given sdk.ServiceMsg request this is the protobuf message type which should be used as google.protobuf.Any.value when delivering the msg to the DeliverTx endpoint |
-| `request_route` | [string](#string) |  | request_route is the sdk.ServiceMsg route, it is equal to type_url |
-| `request_type_url` | [string](#string) |  | request_type_url is the identifier that should be used as google.protobuf.Any.type_url when delivering the msg to the DeliverTx endpoint |
-| `response_fullname` | [string](#string) |  | response_fullname is the protobuf fullname of the given sdk.ServiceMsg response |
-
-
-
-
-
-
 <a name="cosmos.base.reflection.v2alpha1.SigningModeDescriptor"></a>
 
 ### SigningModeDescriptor
@@ -2553,7 +2534,7 @@ TxDescriptor describes the accepted transaction type
 | Field | Type | Label | Description |
 | ----- | ---- | ----- | ----------- |
 | `fullname` | [string](#string) |  | fullname is the protobuf fullname of the raw transaction type (for instance the tx.Tx type) it is not meant to support polymorphism of transaction types, it is supposed to be used by reflection clients to understand if they can handle a specific transaction type in an application. |
-| `msgs` | [MsgDescriptor](#cosmos.base.reflection.v2alpha1.MsgDescriptor) | repeated | msgs lists the accepted application messages (sdk.ServiceMsg, sdk.Msg) NOTE: not to be confused with proto.Message types |
+| `msgs` | [MsgDescriptor](#cosmos.base.reflection.v2alpha1.MsgDescriptor) | repeated | msgs lists the accepted application messages (sdk.Msg) |
 
 
 

diff --git a/proto/cosmos/base/reflection/v2alpha1/reflection.proto b/proto/cosmos/base/reflection/v2alpha1/reflection.proto
@@ -28,8 +28,7 @@ message TxDescriptor {
   // it is not meant to support polymorphism of transaction types, it is supposed to be used by
   // reflection clients to understand if they can handle a specific transaction type in an application.
   string fullname = 1;
-  // msgs lists the accepted application messages (sdk.ServiceMsg, sdk.Msg)
-  // NOTE: not to be confused with proto.Message types
+  // msgs lists the accepted application messages (sdk.Msg)
   repeated MsgDescriptor msgs = 2;
 }
 
@@ -106,28 +105,8 @@ message ConfigurationDescriptor {
 
 // MsgDescriptor describes a cosmos-sdk message that can be delivered with a transaction
 message MsgDescriptor {
-  // msg contains a descriptor of sdk.ServiceMsg, note: sdk.Msg is not supported
-  // as every sdk.Msg is already an sdk.ServiceMsg. It is defined as a oneof in case
-  // different representation of a msg will be implemented.
-  oneof msg {
-    // service_msg is used when the message is an sdk.ServiceMsg type
-    ServiceMsgDescriptor service_msg = 1;
-  }
-}
-
-// ServiceMsgDescriptor describes an sdk.ServiceMsg type
-message ServiceMsgDescriptor {
-  // request_fullname is the protobuf fullname of the given sdk.ServiceMsg request
-  // this is the protobuf message type which should be used as google.protobuf.Any.value
-  // when delivering the msg to the DeliverTx endpoint
-  string request_fullname = 1;
-  // request_route is the sdk.ServiceMsg route, it is equal to type_url
-  string request_route = 2;
-  // request_type_url is the identifier that should be used as google.protobuf.Any.type_url
-  // when delivering the msg to the DeliverTx endpoint
-  string request_type_url = 3;
-  // response_fullname is the protobuf fullname of the given sdk.ServiceMsg response
-  string response_fullname = 4;
+  // msg_type_url contains the TypeURL of a sdk.Msg.
+  string msg_type_url = 1;
 }
 
 // ReflectionService defines a service for application reflection.

diff --git a/server/grpc/reflection/v2alpha1/reflection.go b/server/grpc/reflection/v2alpha1/reflection.go
@@ -173,29 +173,13 @@ func newTxDescriptor(ir codectypes.InterfaceRegistry) (*TxDescriptor, error) {
 
 	msgsDesc := make([]*MsgDescriptor, 0, len(sdkMsgImplementers))
 
-	// process sdk.ServiceMsg
-	for _, svcMsg := range sdkMsgImplementers {
-		resolved, err := ir.Resolve(svcMsg)
-		if err != nil {
-			return nil, fmt.Errorf("unable to resolve sdk.ServiceMsg %s: %w", svcMsg, err)
-		}
-		pbName := proto.MessageName(resolved)
-		if pbName == "" {
-			return nil, fmt.Errorf("unable to get proto name for sdk.ServiceMsg %s", svcMsg)
-		}
-
-		msgsDesc = append(msgsDesc, &MsgDescriptor{Msg: &MsgDescriptor_ServiceMsg{
-			ServiceMsg: &ServiceMsgDescriptor{
-				RequestFullname: pbName,
-				RequestRoute:    svcMsg,
-				RequestTypeUrl:  svcMsg,
-				// NOTE(fdymylja): this cannot be filled as of now, the Configurator is not held inside the *BaseApp type
-				// but is local to specific applications, hence we have no way of getting the MsgServer's descriptors
-				// which contain response information.
-				ResponseFullname: "",
-			},
-		}})
+	// process sdk.Msg
+	for _, msgTypeURL := range sdkMsgImplementers {
+		msgsDesc = append(msgsDesc, &MsgDescriptor{
+			MsgTypeUrl: msgTypeURL,
+		})
 	}
+
 	return &TxDescriptor{
 		Fullname: txPbName,
 		Msgs:     msgsDesc,