feat(x/tx): Support gogo registry in Textual #15302
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,28 +1,27 @@ | ||
| package std | ||
|
|
||
| import ( | ||
| "cosmossdk.io/x/tx/signing" | ||
| "cosmossdk.io/x/tx/signing/direct" | ||
| "cosmossdk.io/x/tx/signing/textual" | ||
| ) | ||
|
|
||
| // SignModeOptions are options for configuring the standard sign mode handler map. | ||
| type SignModeOptions struct { | ||
| // Textual are options for SIGN_MODE_TEXTUAL | ||
| Textual textual.SignModeOptions | ||
| } | ||
|
|
||
| // HandlerMap returns a sign mode handler map that Cosmos SDK apps can use out | ||
| // of the box to support all "standard" sign modes. | ||
| func (s SignModeOptions) HandlerMap() (*signing.HandlerMap, error) { | ||
| txt, err := textual.NewSignModeHandler(s.Textual) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return signing.NewHandlerMap( | ||
| direct.SignModeHandler{}, | ||
| txt, | ||
| ), nil | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,10 +3,13 @@ package textual | |
| import ( | ||
| "context" | ||
| "fmt" | ||
| "strings" | ||
|
|
||
| "github.com/cosmos/cosmos-proto/anyutil" | ||
| "google.golang.org/protobuf/proto" | ||
| "google.golang.org/protobuf/reflect/protoreflect" | ||
| "google.golang.org/protobuf/reflect/protoregistry" | ||
| "google.golang.org/protobuf/types/dynamicpb" | ||
| "google.golang.org/protobuf/types/known/anypb" | ||
| ) | ||
|
|
||
|
|
@@ -26,17 +29,16 @@ func NewAnyValueRenderer(t *SignModeHandler) ValueRenderer { | |
| // Format implements the ValueRenderer interface. | ||
| func (ar anyValueRenderer) Format(ctx context.Context, v protoreflect.Value) ([]Screen, error) { | ||
| msg := v.Message().Interface() | ||
| anymsg, err := toAny(msg) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| internalMsg, err := anyutil.Unpack(anymsg, ar.tr.protoFiles, ar.tr.protoTypes) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| vr, err := ar.tr.GetMessageValueRenderer(internalMsg.ProtoReflect().Descriptor()) | ||
| if err != nil { | ||
| return nil, err | ||
|
|
@@ -48,6 +50,7 @@ func (ar anyValueRenderer) Format(ctx context.Context, v protoreflect.Value) ([] | |
| } | ||
|
|
||
| // The Any value renderer suppresses emission of the object header | ||
| omitHeader := 0 | ||
| _, isMsgRenderer := vr.(*messageValueRenderer) | ||
| if isMsgRenderer && subscreens[0].Content == fmt.Sprintf("%s object", internalMsg.ProtoReflect().Descriptor().Name()) { | ||
| omitHeader = 1 | ||
|
|
@@ -72,8 +75,22 @@ func (ar anyValueRenderer) Parse(ctx context.Context, screens []Screen) (protore | |
| return nilValue, fmt.Errorf("bad indentation: want 0, got %d", screens[0].Indent) | ||
| } | ||
|
|
||
| typeURL := screens[0].Content | ||
| msgType, err := ar.tr.protoTypes.FindMessageByURL(typeURL) | ||
| if err == protoregistry.NotFound { | ||
| // If the proto v2 registry doesn't have this message, then we use | ||
| // protoFiles (which can e.g. be initialized to gogo's MergedRegistry) | ||
| // to retrieve the message descriptor, and then use dynamicpb on that | ||
| // message descriptor to create a proto.Message | ||
| typeURL := strings.TrimPrefix(typeURL, "/") | ||
|
|
||
| msgDesc, err := ar.tr.protoFiles.FindDescriptorByName(protoreflect.FullName(typeURL)) | ||
| if err != nil { | ||
| return nilValue, fmt.Errorf("textual protoFiles does not have descriptor %s: %w", typeURL, err) | ||
| } | ||
|
|
||
| msgType = dynamicpb.NewMessageType(msgDesc.(protoreflect.MessageDescriptor)) | ||
| } else if err != nil { | ||
| return nilValue, err | ||
| } | ||
| vr, err := ar.tr.GetMessageValueRenderer(msgType.Descriptor()) | ||
|
|
@@ -113,3 +130,19 @@ func (ar anyValueRenderer) Parse(ctx context.Context, screens []Screen) (protore | |
|
|
||
| return protoreflect.ValueOfMessage(anyMsg.ProtoReflect()), nil | ||
| } | ||
|
|
||
| // toAny converts the proto Message to a anypb.Any. | ||
| // The input msg can be: | ||
| // - either a anypb.Any already (in which case there's nothing to do), | ||
| // - or a dynamicpb.Message. | ||
| func toAny(msg proto.Message) (*anypb.Any, error) { | ||
|
There was a problem hiding this comment. In the case of a Even better, how about a single function There was a problem hiding this comment. That's a good suggestion, thanks. I applied it in fbe2ce6, and the code looks less scattered. I'll see about moving |
||
| switch msg := msg.(type) { | ||
| case *anypb.Any: | ||
| return msg, nil | ||
| case *dynamicpb.Message: | ||
| t, v := getValueFromFieldName(msg, "type_url").String(), getValueFromFieldName(msg, "value").Bytes() | ||
| return &anypb.Any{TypeUrl: t, Value: v}, nil | ||
| default: | ||
| return nil, fmt.Errorf("expected anypb.Any or dynamicpb.Message, got %T", msg) | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and below for similar converters - I'm quite confused by this. Don't we know that
Any,Duration, etc, are in fact registered?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fact that we import
google.golang.org/protobuf/types/known/{any,duration...}pbindeed guarantees thatAny,Durationare registered.However, when they are nested in another message that's not registered, then they are dynamically created using dynamicpb. Consider a custom Msg, suppose it's only registered in gogo, and not in protov2:
Then when Textual receives this message, it sees
MsgCustomand will use dynamicpb to generate the whole proto.Message, including the nested field.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We currently use dynamicpb.NewMessageType to do this. We could consider writing our own
dynamicpb.NewMessageTypeV2that uses known types when generating a field with a known name. I'm not sure if that would be more helpful or confusing though.Created a new issue: cosmos/cosmos-proto#104
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation. It would be nice if we didn't have to complicate the textual code for this, but I don't know enough to second-guess the choices you've made here.