cosmos-proto
cosmos-proto copied to clipboard
Utility for dynamicpb.NewMessageType + known types
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.
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.