cosmos-proto icon indicating copy to clipboard operation
cosmos-proto copied to clipboard

Utility for dynamicpb.NewMessageType + known types

Open amaury1093 opened this issue 1 year ago • 1 comments

Summary

Write a new utililty function that's the same as dynamicpb.NewMessageType, but when a nested field is a known type, use that resolved type, instead of a new dynamicpb.Message.

ref: https://github.com/cosmos/cosmos-sdk/pull/15302#discussion_r1136452777

Problem

Consider a custom Msg, suppose it's only registered in gogo, and not in protov2:

message MsgCustom {
  google.protobuf.Any a = 1;
}

Then when our Unpack utility receives this message, it sees MsgCustom and will use dynamicpb.NewMessageType to generate the whole proto.Message, including the nested field. MsgCustom.A will have type dynamicpb.Message.

Is this expected, or is it more helpful to have MsgCustom.A be an anypb.Any?

Proposal 1: Keep using official dynamicpb.NewMessageType

Using this, we know that proto.Messages created with dynamicpb are dynamicpb all the way down. There are no statically-created types used.

However, this also means that callers in general cannot type-cast. For example, in Textual, we use a switch case to handle both cases (dynamicpb or known type): https://github.com/cosmos/cosmos-sdk/blob/9a83c202cd545265f54d15090c6b150dc692f0f6/x/tx/signing/textual/any.go#L134-L148

We probably can move some utility functions like this to cosmos-proto if they are repeated.

Proposal 2: Write an utility function that does dynamicpb.NewMessageType with known types

We should pass a file resolver / type resolver to this new function, but it shouldn't be too hard to write it. The positive point is that callers can always type-cast to known types.

amaury1093 avatar Mar 15 '23 09:03 amaury1093

Currently, in Textual, we decided to use a coerceToMessage utility, see here.

Usage is like this:

// Assume `givenCoin` is a dynamicpb.Message representing a Coin
coin := &basev1beta1.Coin{}
err := coerceToMessage(givenCoin, coin)
if err != nil { /* handler error */ }
fmt.Println(coin) // Will have the same field values as `givenCoin`

This way, after coercing, Textual always handles concrete types in its code. If coerceToMessage is useful for other packages, I propose to move it here, as it's really SDK-independent.

amaury1093 avatar Mar 20 '23 16:03 amaury1093