Grpc reflection

[sekulicd]

This adds gRPC Server Reflection which provides information about RPC's.

To test locally:
grpcurl -plaintext 127.0.0.1:9945 list
grpcurl -insecure 127.0.0.1:9000 list

This closes #679

@altafan please review.

[what-the-diff]

• Add Reflection to the list of entities in permissions.go
• Register reflection service for both grpcOperatorServer and grpcTradeServer

[altafan]

I just noticed that the reflection RPC is defined as a bidirectional stream and this would cause browser clients to not be able to make use of it basically.

I found out this grpc-gateway wrapper that exposes bidi streams as websockets. It might be what we're looking for, but still, we should make changes to the reflection RPC to add google annotations for the gateway.

Alternatively, we could just define our own reflection service with a unary RPC instead and related google annotations.

@tiero @sekulicd @Janaka-Steph please give a feedback.

[Janaka-Steph]

Bidirectional stream

Indeed the protocol is structured as a bidirectional stream. Here it describes why:
https://github.com/grpc/grpc/blob/master/doc/server-reflection.md#reverse-proxy-traversal
Mainly to ensure that all related requests go to a single server.

Client support

From https://github.com/deeplay-io/nice-grpc/tree/master/packages/nice-grpc-web#compatibility
"Most browsers do not support sending streams in fetch requests. This means that client streaming and bidirectional streaming will not work. The only browser that supports client streams is Chrome 105+ (and other Chromium-based browsers, see compatibility table), and only over http2, which in turn requires https. Client streams work in NodeJS native fetch implementation as well. Note, however, that fetch streams are currently whatwg/fetch#1254, which means that any response data will be buffered until the request stream is sent until the end. This unfortunately makes it impossible to use infinite bidirectional streaming. To overcome this limitation, it is recommended to design your API to use only unary and server streaming methods. If you still need to use client streams, you can use a Websocket transport with grpcwebproxy."

=> Here it says the API should use only unary and server streaming methods. But doesn't warn about bidirectional stream of Reflection protocol. So it's ok after all ?

How to use reflection from browser

I looked at how to call reflection endpoints from JS client. It can be done through @grpc/grpc-js directly or using libraries like https://github.com/redhoyasa/grpc-reflection-js and https://github.com/deeplay-io/nice-grpc

Use reflection v1, not v1 alpha

The PR is using v1 alpha but v1 has been released:
https://github.com/grpc/grpc/blob/master/src/proto/grpc/reflection/v1/reflection.proto

Conclusion

I would try in regtest if it works with this PR (updated to latest v1 if possible) and https://github.com/redhoyasa/grpc-reflection-js on client.
I am not sure if we need a proxy.

[sekulicd]

One option is that tdex-daemon exposes another unary RPC that would internally call reflection stream, PSB POC:

func (o operatorHandler) GetSvcInfo(
	ctx context.Context,
	request *daemonv1.GetSvcInfoRequest,
) (*daemonv1.GetSvcInfoResponse, error) {
	conn, err := grpc.Dial("127.0.0.1:9945", grpc.WithInsecure())
	if err != nil {
		return nil, err
	}
	defer conn.Close()

	stub := rpb.NewServerReflectionClient(conn)
	client, err := stub.ServerReflectionInfo(context.Background())
	if err != nil {
		return nil, err
	}
	defer client.CloseSend()

	if err := client.Send(&rpb.ServerReflectionRequest{
		MessageRequest: &rpb.ServerReflectionRequest_ListServices{},
	}); err != nil {
		return nil, err
	}

	resp, err := client.Recv()
	if err != nil {
		return nil, err
	}

	services := make([]string, 0, len(resp.GetListServicesResponse().Service))
	for _, service := range resp.GetListServicesResponse().Service {
		services = append(services, service.Name)
	}

	return &daemonv1.GetSvcInfoResponse{
		Services: services,
	}, nil
}

[tiero]

@sekulicd solutions works too

[sekulicd]

@tiero @altafan please review.

[Janaka-Steph]

I didn't get how the client should call List. It seems it still needs the protos to do that. But the purpose of List is to know which protos to use.

Also the PR is still using Reflection v1 alpha.

[sekulicd]

I didn't get how the client should call List. It seems it still needs the protos to do that. But the purpose of List is to know which protos to use.

Also the PR is still using Reflection v1 alpha.

@Janaka-Steph with latest commit u can call regular unary RPC ListProtoServices which is internally using reflecation.

[Janaka-Steph]

@sekulicd Yes I get that, it is part of Operator proto. Again, the goal here is to know which proto version a daemon is using. So List should be callable without instanciating a OperatorServiceClient.

[sekulicd]

.. goal here is to know which proto version a daemon is using. So List should be callable without instanciating a OperatorServiceClient.

I see, if i understand issue with using operator client is that u dont know version, right? @Janaka-Steph
If this is the true one option would be to put this code inside HTTP endpoint, but endpoint would be able to serve infos only after wallet is initialised.
If this is not acceptable then we need to re-implement reflection by ourself and expose it via HTTP that would be reachable even if wallet is not init.

@tiero @altafan

[Janaka-Steph]

@sekulicd That's right.

By the way, for that purpose of discovering daemon version I could also use a more hacky solution where I instanciate a v2 service and calling a method that only exist in v2. If it throw then the daemon is using protos v1.

[Janaka-Steph]

Why not setting up Envoy?

[altafan]

this RPC must be moved to a dedicated proto into a ReflectionService.
It can't stay here because it wouldn't be possible to call the reflection API without knowing the version of the operator proto (v1/v2).

[tiero]

Why not setting up Envoy?

We cant make assumption of the infrastructure where tdex daemons are run by other operators @Janaka-Steph

[tiero]

I am in favor of keep it simple here, and return an HTTP endpoint (like we do for transport) that tella us the version of protos used byt trader and operator interface

[altafan]

Let's make use of our fork of grpc-go/reflection that supports both grpc-web and gateway, making the reflection endpoint available with just one line reflection.Register(grpcServer) on every protocol.

I'm closing this since it's easier to just start from v1 branch for this one-line change instead of reverting all the work done here.

@sekulicd can you open another PR please?