Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup remains of ServiceMsg #9236

Merged
merged 14 commits into from
May 7, 2021
Merged
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ Ref: https://keepachangelog.com/en/1.0.0/
### 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`.
Expand Down
2 changes: 1 addition & 1 deletion client/tx/legacy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
7 changes: 7 additions & 0 deletions client/tx/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ 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.
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
for _, msg := range msgs {
if err := msg.ValidateBasic(); err != nil {
return err
}
}

if clientCtx.GenerateOnly {
return GenerateTx(clientCtx, txf, msgs...)
}
Expand Down
23 changes: 2 additions & 21 deletions docs/core/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,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)

Expand Down Expand Up @@ -2431,7 +2430,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. |



Expand Down Expand Up @@ -2488,24 +2487,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
Expand Down Expand Up @@ -2535,7 +2516,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) |



Expand Down
27 changes: 3 additions & 24 deletions proto/cosmos/base/reflection/v2alpha1/reflection.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fdymylja Does this simplification make sense to you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AmauryM perfect!

}

// ReflectionService defines a service for application reflection.
Expand Down
28 changes: 6 additions & 22 deletions server/grpc/reflection/v2alpha1/reflection.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading