lightningnetwork / lnd

Lightning Network Daemon ⚡️
MIT License
7.7k stars 2.09k forks source link

Create separate go module for proto files #5402

Open joostjager opened 3 years ago

joostjager commented 3 years ago

Currently the proto files are part of the main lnd module. That means that if an application just needs to reference the proto files, it will inherit all the lnd dependencies such as etcd. This can sometimes cause difficult to resolve conflicts if the application uses the same dependencies but different versions.

One option may be to put the proto files in their own module, so that linking against it can remain very light.

gabbyprecious commented 2 years ago

Hi @Roasbeef can I work on this?

l0k18 commented 2 years ago

In the master branch, it seems to me that lnrpc already is a mostly independent unit.

Is the goal to consolidate all the generated files into one package and separate it from lnrpc?

Just trying to get a clear idea of what is intended for this task.

l0k18 commented 2 years ago

Ok, after more inspection and reading and checking I think I know what's intended here.

Currently I can see every submodule has build tags attached to it. Thus they can't be imported separately. They should have a separate subfolder which is imported by the tagged source files.

I am proposing to essentially make a proto folder in each one and isolating the generated files and the generation scripts.

l0k18 commented 2 years ago

I'm encountering some funny versioning issues with google protobuf packages. If you run the lnrpc/gen_protos.sh script now, you will get a non building output. Looking into how to fix this first.

all goes well until this point:

# github.com/lightningnetwork/lnd/lnrpc/verrpc
lnrpc/verrpc/server.go:82:51: cannot use mux (variable of type *"github.com/grpc-ecosystem/grpc-gateway/v2/runtime".ServeMux) as type *"github.com/grpc-ecosystem/grpc-gateway/runtime".ServeMux in argument to RegisterVersionerHandlerFromEndpoint
# github.com/lightningnetwork/lnd/lnrpc/wtclientrpc
lnrpc/wtclientrpc/wtclient.go:144:58: cannot use mux (variable of type *"github.com/grpc-ecosystem/grpc-gateway/v2/runtime".ServeMux) as type *"github.com/grpc-ecosystem/grpc-gateway/runtime".ServeMux in argument to RegisterWatchtowerClientHandlerFromEndpoint
github.com/lightningnetwork/lnd/chainreg
# github.com/lightningnetwork/lnd/lnrpc/routerrpc
lnrpc/routerrpc/router_server.go:273:48: cannot use mux (variable of type *"github.com/grpc-ecosystem/grpc-gateway/v2/runtime".ServeMux) as type *"github.com/grpc-ecosystem/grpc-gateway/runtime".ServeMux in argument to RegisterRouterHandlerFromEndpoint
github.com/lightningnetwork/lnd/funding
github.com/lightningnetwork/lnd/lnwallet/rpcwallet
github.com/lightningnetwork/lnd/peer
make: *** [Makefile:137: install] Error 2

The issue has to do with the gRPC gateway generator. Digging further...

guggero commented 2 years ago

You need to import the correct version of the grpc-gateway package. Notice there's a v2 and a non-v2 package.

l0k18 commented 2 years ago

ok, I seem to have found the right setup now, just want to record it here for future reference:

go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.26.0
go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest
go install github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2@v2.5.0
go install github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-grpc-gateway@v2.5.0

This set of versions when invoked via lnrpc/gen_protos.sh produces the same as the current state of the repository, with the minor difference that they now add the version number of the plugins into the *_grpc.pb.go and the filename of the source proto.

// versions:
// - protoc-gen-go-grpc v1.2.0
// - protoc             v3.6.1
// source: stateservice.proto

This should help, it should be in the build documentation. I will be able to make a lot more progress now as those commands need to be invoked and everything still compiles and runs.

l0k18 commented 2 years ago

I may need some help with making sure that the CI tests are right before merging, but this PR has only got changes of filesystem structure that necessitated some extra package dot references to account for. I hope this satisfies the requirements.

oh, I did miss one thing. The version number for google.golang.org/grpc/cmd/protoc-gen-go-grpc - it is v1.1.0

l0k18 commented 2 years ago

I'm gonna try and do this one again. chainrpc and neutrinorpc APIs changed between my branch and the master branch while I waited to get it merged and now I've done gone and deleted my fork. Should go smooth this time.