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

Invalid FileDescriptorSet in 0.47 #14713

Closed
aaronc opened this issue Jan 20, 2023 · 1 comment · Fixed by #14838
Closed

Invalid FileDescriptorSet in 0.47 #14713

aaronc opened this issue Jan 20, 2023 · 1 comment · Fixed by #14838
Assignees
Labels

Comments

@aaronc
Copy link
Member

aaronc commented Jan 20, 2023

Summary of Bug

cosmos.reflection.v1 returns an invalid FileDescriptorSet that can't be built by protodesc.NewFiles.

Version

v0.47.0-rc1

Steps to Reproduce

In testing #14696, @julienrbrt ran into these issues

grpcurl v047.simapp.zone:9090 cosmos.auth.v1beta1.Query/Accounts
However, trying to use cosmcli with v047 gives me this:
julien:~/ $ cosmcli --init simapp
Configuring simapp
Unable to load data for simapp in the chain registry. You'll have to specify a gRPC endpoint manually.
Enter a gRPC endpoint that you trust: v047.simapp.zone:9090
Selected: v047.simapp.zone:9090
Error: error building protoregistry.Files: proto: file "gogo.proto" has a name conflict over gogoproto.goproto_enum_prefix
Usage:
[flags]

Examples:
cosmcli --init foochain

Flags:
-h, --help help for this command
--init string initialize a new chain with the specified name

panic: error building protoregistry.Files: proto: file "gogo.proto" has a name conflict over gogoproto.goproto_enum_prefix

goroutine 1 [running]:
main.main()
/home/julien/projects/cosmos/cosmos-sdk/client/v2/cmd/cosmcli/main.go:13 +0x51
Some other times I get some other errors like: Error: error building protoregistry.Files: proto: file "descriptor.proto" has a name conflict over google.protobuf.FileDescriptorSet

This indicates that the cosmos.reflection.v1 service we added in v.047 is returning a FileDescriptorSet with duplicate proto files that can't be turned into a valid instance of https://pkg.go.dev/google.golang.org/protobuf@v1.26.0/reflect/protoregistry#Files. This will cause problems for clients downstream, including but not limited to autocli.

Proposed Solution

  1. NewReflectionService should return an error whenever the FileDescriptorSet can't be built into protoregistry.Files with helpful error messages and links to documentation. In particular, @testinginprod suggests to return errors in the following cases:
  • two proto files have equal content but are registered with different paths
  • two proto files are registered with the same path but have different content
  1. Clean up any such issues originating in the SDK that will be passed on to downstream users - in particular the duplication of gogo.proto and maybe descriptor.proto
@aaronc
Copy link
Member Author

aaronc commented Jan 23, 2023

@AmauryM here's where tendermint/liquidity is messing up import paths:

@aaronc aaronc mentioned this issue Jan 24, 2023
54 tasks
@amaury1093 amaury1093 moved this from 📝 Todo to 💪 In Progress in Cosmos-SDK Jan 25, 2023
@amaury1093 amaury1093 moved this from 💪 In Progress to 👀 Needs Review in Cosmos-SDK Jan 30, 2023
@github-project-automation github-project-automation bot moved this from 👀 Needs Review to 👏 Done in Cosmos-SDK Jan 31, 2023
@tac0turtle tac0turtle removed the Q1:2023 label Apr 3, 2023
@tac0turtle tac0turtle removed this from Cosmos-SDK May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants