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

Add convenience constructors to generated go code #2731

Closed
conorsch opened this issue Jun 22, 2023 · 2 comments
Closed

Add convenience constructors to generated go code #2731

conorsch opened this issue Jun 22, 2023 · 2 comments
Assignees

Comments

@conorsch
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Our generated golang code, based on protos and built via buf tooling, lacks convenient constructors present in the generated rust code. For example, we can call ViewProtocolServiceClient::new in rust:

ViewProtocolServiceClient::new(ViewProtocolServiceServer::new(self.clone()));

But there's no comparable method in the generated go code:

rg -il ViewProtocolServiceClient crates/proto/src/gen/ proto/go/gen/
crates/proto/src/gen/penumbra.view.v1alpha1.rs

Describe the solution you'd like
Other downstream consumers of our protos, notably Strangelove, use a different buf build module, protoc-gen-gocosmos, to generate these convenience methods. If we switch our CI to use that tooling, we can have access to the same methods in the generated golang we build automatically.

Describe alternatives you've considered
We can simply ignore this discrepancy. Depending on the upstream gocosmos tooling (which itself is a fork of the deprecated gogoprotobuf project) might cause problems down the road if it's no longer maintained.

Additional context

An example of using the convenient method in go
// This code adapted from the ongoing integration work in github.com/strangelove-ventures/interchaintest
pclientd_addr := "localhost:8081"
channel, err := grpc.Dial(
        pclientd_addr,
        grpc.WithTransportCredentials(insecure.NewCredentials()),
)
if err != nil {
        return false, err
}
defer channel.Close()

// Note the `NewViewProtocolServiceClient` convenience constructor.
viewClient := penumbraview.NewViewProtocolServiceClient(channel)
addressReq := &penumbraview.AddressByIndexRequest{
        AddressIndex: &penumbracrypto.AddressIndex{
                Account: 0,
        }}

ctx := context.TODO()
fmt.Println("Requesting address via pclientd...")
resp, err := viewClient.AddressByIndex(ctx, addressReq)
if err != nil {
        fmt.Println("Encountered error looking up address via pclientd", err)
        return false, err
}
@aubrika aubrika moved this to Next in Testnets Jun 23, 2023
@aubrika aubrika moved this from Next to Testnet 56: Callisto in Testnets Jun 23, 2023
@aubrika aubrika moved this from Testnet 56: Callisto to Next in Testnets Jun 23, 2023
@conorsch
Copy link
Contributor Author

One downside of this proposal is that it would require folks to download the protoc-gen-gocosmos tool locally in order to regenerate the protos as part of our tooling. That's suboptimal, but in my view, not a showstopper. Thoughts from others?

@conorsch
Copy link
Contributor Author

We're on the fence about utility here. Practically speaking, I'd like to let the dust settle on the golang integration with Penumbra that Strangelove has been working on. See some related challenges here: strangelove-ventures/interchaintest#658

@conorsch conorsch moved this from Next to Future in Testnets Jul 14, 2023
@aubrika aubrika added this to Penumbra Oct 30, 2023
@github-project-automation github-project-automation bot moved this to 🗄️ Backlog in Penumbra Oct 30, 2023
conorsch added a commit that referenced this issue Feb 6, 2024
We originally added the golang codegen to provide first-class support to
downstream integrators. This is no longer necessary, as those same
integrators, e.g. Strangelove, are performing their own codegen in the
respective repos. These codepaths in the Penumbra monorepo are unused
and largely unmaintained, so I'm culling them.

Refs #2184. Moots and therefore closes #2731.
conorsch added a commit that referenced this issue Feb 6, 2024
We originally added the golang codegen to provide first-class support to
downstream integrators. This is no longer necessary, as those same
integrators, e.g. Strangelove, are performing their own codegen in the
respective repos. These codepaths in the Penumbra monorepo are unused
and largely unmaintained, so I'm culling them.

Refs #2184. Moots and therefore closes #2731.
@github-project-automation github-project-automation bot moved this from 🗄️ Backlog to Done in Penumbra Feb 7, 2024
TalDerei pushed a commit that referenced this issue Feb 8, 2024
We originally added the golang codegen to provide first-class support to
downstream integrators. This is no longer necessary, as those same
integrators, e.g. Strangelove, are performing their own codegen in the
respective repos. These codepaths in the Penumbra monorepo are unused
and largely unmaintained, so I'm culling them.

Refs #2184. Moots and therefore closes #2731.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Status: Future
Development

No branches or pull requests

1 participant