From feaefd2f952778be18f499595bb602dc5a2f8b1a Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Thu, 28 Sep 2023 13:02:38 +0200 Subject: [PATCH 01/10] add protobuf hybrid handler --- baseapp/grpcrouter.go | 128 ++++++++++++------- baseapp/grpcrouter_test.go | 25 +++- baseapp/protocompat/protocompat.go | 191 +++++++++++++++++++++++++++++ 3 files changed, 298 insertions(+), 46 deletions(-) create mode 100644 baseapp/protocompat/protocompat.go diff --git a/baseapp/grpcrouter.go b/baseapp/grpcrouter.go index 5a127277feab..c976da0c3831 100644 --- a/baseapp/grpcrouter.go +++ b/baseapp/grpcrouter.go @@ -1,12 +1,16 @@ package baseapp import ( + "context" "fmt" abci "github.com/cometbft/cometbft/abci/types" + "github.com/cosmos/cosmos-sdk/baseapp/protocompat" gogogrpc "github.com/cosmos/gogoproto/grpc" + "github.com/cosmos/gogoproto/proto" "google.golang.org/grpc" "google.golang.org/grpc/encoding" + "google.golang.org/protobuf/reflect/protoreflect" "github.com/cosmos/cosmos-sdk/client/grpc/reflection" "github.com/cosmos/cosmos-sdk/codec" @@ -16,9 +20,11 @@ import ( // GRPCQueryRouter routes ABCI Query requests to GRPC handlers type GRPCQueryRouter struct { - routes map[string]GRPCQueryHandler - cdc encoding.Codec - serviceData []serviceData + routes map[string]GRPCQueryHandler + handlerByMessageName map[string][]func(ctx context.Context, msg proto.Message) (proto.Message, error) + sdkCodec codec.BinaryCodec + cdc encoding.Codec + serviceData []serviceData } // serviceData represents a gRPC service, along with its handler. @@ -32,7 +38,8 @@ var _ gogogrpc.Server = &GRPCQueryRouter{} // NewGRPCQueryRouter creates a new GRPCQueryRouter func NewGRPCQueryRouter() *GRPCQueryRouter { return &GRPCQueryRouter{ - routes: map[string]GRPCQueryHandler{}, + routes: map[string]GRPCQueryHandler{}, + handlerByMessageName: map[string][]func(ctx context.Context, msg proto.Message) (proto.Message, error){}, } } @@ -58,46 +65,13 @@ func (qrt *GRPCQueryRouter) Route(path string) GRPCQueryHandler { func (qrt *GRPCQueryRouter) RegisterService(sd *grpc.ServiceDesc, handler interface{}) { // adds a top-level query handler based on the gRPC service name for _, method := range sd.Methods { - fqName := fmt.Sprintf("/%s/%s", sd.ServiceName, method.MethodName) - methodHandler := method.Handler - - // Check that each service is only registered once. If a service is - // registered more than once, then we should error. Since we can't - // return an error (`Server.RegisterService` interface restriction) we - // panic (at startup). - _, found := qrt.routes[fqName] - if found { - panic( - fmt.Errorf( - "gRPC query service %s has already been registered. Please make sure to only register each service once. "+ - "This usually means that there are conflicting modules registering the same gRPC query service", - fqName, - ), - ) + err := qrt.registerABCIQueryHandler(sd, method, handler) + if err != nil { + panic(err) } - - qrt.routes[fqName] = func(ctx sdk.Context, req *abci.RequestQuery) (*abci.ResponseQuery, error) { - // call the method handler from the service description with the handler object, - // a wrapped sdk.Context with proto-unmarshaled data from the ABCI request data - res, err := methodHandler(handler, ctx, func(i interface{}) error { - return qrt.cdc.Unmarshal(req.Data, i) - }, nil) - if err != nil { - return nil, err - } - - // proto marshal the result bytes - var resBytes []byte - resBytes, err = qrt.cdc.Marshal(res) - if err != nil { - return nil, err - } - - // return the result bytes as the response value - return &abci.ResponseQuery{ - Height: req.Height, - Value: resBytes, - }, nil + err = qrt.registerHandlerByMessageName(sd, method, handler) + if err != nil { + panic(err) } } @@ -107,9 +81,77 @@ func (qrt *GRPCQueryRouter) RegisterService(sd *grpc.ServiceDesc, handler interf }) } +func (qrt *GRPCQueryRouter) registerABCIQueryHandler(sd *grpc.ServiceDesc, method grpc.MethodDesc, handler interface{}) error { + fqName := fmt.Sprintf("/%s/%s", sd.ServiceName, method.MethodName) + methodHandler := method.Handler + + // Check that each service is only registered once. If a service is + // registered more than once, then we should error. Since we can't + // return an error (`Server.RegisterService` interface restriction) we + // panic (at startup). + _, found := qrt.routes[fqName] + if found { + return fmt.Errorf( + "gRPC query service %s has already been registered. Please make sure to only register each service once. "+ + "This usually means that there are conflicting modules registering the same gRPC query service", + fqName, + ) + } + + qrt.routes[fqName] = func(ctx sdk.Context, req *abci.RequestQuery) (*abci.ResponseQuery, error) { + // call the method handler from the service description with the handler object, + // a wrapped sdk.Context with proto-unmarshaled data from the ABCI request data + res, err := methodHandler(handler, ctx, func(i interface{}) error { + return qrt.cdc.Unmarshal(req.Data, i) + }, nil) + if err != nil { + return nil, err + } + + // proto marshal the result bytes + var resBytes []byte + resBytes, err = qrt.cdc.Marshal(res) + if err != nil { + return nil, err + } + + // return the result bytes as the response value + return &abci.ResponseQuery{ + Height: req.Height, + Value: resBytes, + }, nil + } + return nil +} + +func (qrt *GRPCQueryRouter) HandlersByRequestName(name string) []func(ctx context.Context, req proto.Message) (resp proto.Message, err error) { + return qrt.handlerByMessageName[name] +} + +func (qrt *GRPCQueryRouter) registerHandlerByMessageName(sd *grpc.ServiceDesc, method grpc.MethodDesc, handler interface{}) error { + // extract message name from method descriptor + methodFullName := protoreflect.FullName(fmt.Sprintf("%s.%s", sd.ServiceName, method.MethodName)) + desc, err := proto.HybridResolver.FindDescriptorByName(methodFullName) + if err != nil { + return fmt.Errorf("cannot find method descriptor %s", methodFullName) + } + methodDesc, ok := desc.(protoreflect.MethodDescriptor) + if !ok { + return fmt.Errorf("invalid method descriptor %s", methodFullName) + } + inputName := methodDesc.Input().FullName() + methodHandler, err := protocompat.MakeHybridHandler(qrt.sdkCodec, sd, method, handler) + if err != nil { + return err + } + qrt.handlerByMessageName[string(inputName)] = append(qrt.handlerByMessageName[string(inputName)], methodHandler) + return nil +} + // SetInterfaceRegistry sets the interface registry for the router. This will // also register the interface reflection gRPC service. func (qrt *GRPCQueryRouter) SetInterfaceRegistry(interfaceRegistry codectypes.InterfaceRegistry) { + qrt.sdkCodec = codec.NewProtoCodec(interfaceRegistry) // instantiate the codec qrt.cdc = codec.NewProtoCodec(interfaceRegistry).GRPCCodec() // Once we have an interface registry, we can register the interface diff --git a/baseapp/grpcrouter_test.go b/baseapp/grpcrouter_test.go index 593125c692f8..849bc15d190c 100644 --- a/baseapp/grpcrouter_test.go +++ b/baseapp/grpcrouter_test.go @@ -5,11 +5,10 @@ import ( "sync" "testing" - dbm "github.com/cosmos/cosmos-db" - "github.com/stretchr/testify/require" - "cosmossdk.io/depinject" "cosmossdk.io/log" + dbm "github.com/cosmos/cosmos-db" + "github.com/stretchr/testify/require" "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/codec/types" @@ -51,6 +50,26 @@ func TestGRPCQueryRouter(t *testing.T) { require.NoError(t, err) require.NotNil(t, res3) require.Equal(t, spot, res3.HasAnimal.Animal.GetCachedValue()) + + // test getting the handler by name + handlers := qr.HandlersByRequestName("testpb.EchoRequest") + require.NotNil(t, handlers) + require.Len(t, handlers, 1) + handler := handlers[0] + // sending a protov2 message should work, and return a protov2 message + resp, err := handler(helper.Ctx, &testdata_pulsar.EchoRequest{Message: "hello"}) + require.Nil(t, err) + require.NotNil(t, resp) + protov2Resp, ok := resp.(*testdata_pulsar.EchoResponse) + require.True(t, ok) + require.Equal(t, "hello", protov2Resp.Message) + // also sending a protov1 message should work, and return a gogoproto message + resp, err = handler(helper.Ctx, &testdata.EchoRequest{Message: "hello"}) + require.Nil(t, err) + require.NotNil(t, resp) + gogoprotoResp, ok := resp.(*testdata.EchoResponse) + require.True(t, ok) + require.Equal(t, "hello", gogoprotoResp.Message) } func TestRegisterQueryServiceTwice(t *testing.T) { diff --git a/baseapp/protocompat/protocompat.go b/baseapp/protocompat/protocompat.go new file mode 100644 index 000000000000..cecf39e9105b --- /dev/null +++ b/baseapp/protocompat/protocompat.go @@ -0,0 +1,191 @@ +package protocompat + +import ( + "context" + "fmt" + "reflect" + + "github.com/cosmos/cosmos-sdk/codec" + gogoproto "github.com/cosmos/gogoproto/proto" + "google.golang.org/grpc" + proto2 "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/reflect/protoregistry" +) + +var ( + gogoType = reflect.TypeOf((*gogoproto.Message)(nil)).Elem() + protov2Type = reflect.TypeOf((*proto2.Message)(nil)).Elem() +) + +type Handler = func(ctx context.Context, message gogoproto.Message) (gogoproto.Message, error) + +func MakeHybridHandler(cdc codec.BinaryCodec, sd *grpc.ServiceDesc, method grpc.MethodDesc, handler interface{}) (Handler, error) { + methodFullName := protoreflect.FullName(fmt.Sprintf("%s.%s", sd.ServiceName, method.MethodName)) + desc, err := gogoproto.HybridResolver.FindDescriptorByName(methodFullName) + if err != nil { + return nil, err + } + methodDesc, ok := desc.(protoreflect.MethodDescriptor) + if !ok { + return nil, fmt.Errorf("invalid method descriptor %s", methodFullName) + } + + funcType, err := getFuncType(handler, method.MethodName) + if err != nil { + return nil, err + } + // the handler function takes two arguments: context.Context and proto.Message + // since it is a method, the first argument is the receiver, which we don't care about. + // the second argument is the proto.Message, which we need to know the type of. + inputType := funcType.In(2) + switch { + case inputType.Implements(protov2Type): + return makeProtoV2HybridHandler(methodDesc, cdc, method, handler) + case inputType.Implements(gogoType): + return makeGogoHybridHandler(methodDesc, cdc, method, handler) + default: + return nil, fmt.Errorf("invalid method handler type %T, input does not implement", method.Handler) + } +} + +// makeProtoV2HybridHandler returns a handler that can handle both gogo and protov2 messages. +func makeProtoV2HybridHandler(prefMethod protoreflect.MethodDescriptor, cdc codec.BinaryCodec, method grpc.MethodDesc, handler any) (Handler, error) { + // it's a protov2 handler, if a gogo counterparty is not found we cannot handle gogo messages. + gogoRespType := gogoproto.MessageType(string(prefMethod.Output().FullName())) + if gogoRespType == nil { + return func(ctx context.Context, request gogoproto.Message) (gogoproto.Message, error) { + protov2Request, ok := request.(proto2.Message) + if !ok { + return nil, fmt.Errorf("invalid request type %T, method %s does not accept gogoproto messages", request, prefMethod.FullName()) + } + resp, err := method.Handler(handler, ctx, func(msg any) error { + proto2.Merge(msg.(proto2.Message), protov2Request) + return nil + }, nil) + if err != nil { + return nil, err + } + return resp.(gogoproto.Message), nil + }, nil + } + gogoRespType = gogoRespType.Elem() // we need the non pointer type + return func(ctx context.Context, request gogoproto.Message) (gogoproto.Message, error) { + // we check if the request is a protov2 message. + switch m := request.(type) { + case proto2.Message: + // we can just call the handler after making a copy of the message, for safety reasons. + resp, err := method.Handler(handler, ctx, func(msg any) error { + proto2.Merge(msg.(proto2.Message), m) + return nil + }, nil) + if err != nil { + return nil, err + } + return resp.(gogoproto.Message), nil + case gogoproto.Message: + // we need to marshal and unmarshal the request. + requestBytes, err := cdc.Marshal(m) + if err != nil { + return nil, err + } + resp, err := method.Handler(handler, ctx, func(msg any) error { + // unmarshal request into the message. + return proto2.Unmarshal(requestBytes, msg.(proto2.Message)) + }, nil) + if err != nil { + return nil, err + } + // the response is a protov2 message, so we cannot just return it. + // since the request came as gogoproto, we expect the response + // to also be gogoproto. + respBytes, err := proto2.Marshal(resp.(proto2.Message)) + if err != nil { + return nil, err + } + + // unmarshal response into a gogo message. + gogoResp := reflect.New(gogoRespType).Interface().(gogoproto.Message) + return gogoResp, cdc.Unmarshal(respBytes, gogoResp) + default: + panic("unreachable") + } + }, nil +} + +func makeGogoHybridHandler(prefMethod protoreflect.MethodDescriptor, cdc codec.BinaryCodec, method grpc.MethodDesc, handler any) (Handler, error) { + // it's a gogo handler, we check if the existing protov2 counterparty exists. + protov2RespType, err := protoregistry.GlobalTypes.FindMessageByName(prefMethod.Output().FullName()) + if err != nil { + // this can only be a gogo message. + return func(ctx context.Context, request gogoproto.Message) (gogoproto.Message, error) { + _, ok := request.(proto2.Message) + if ok { + return nil, fmt.Errorf("invalid request type %T, method %s does not accept protov2 messages", request, prefMethod.FullName()) + } + resp, err := method.Handler(handler, ctx, func(msg any) error { + // merge! + gogoproto.Merge(msg.(gogoproto.Message), request) + return nil + }, nil) + if err != nil { + return nil, err + } + return resp.(gogoproto.Message), nil + }, nil + } + // this is a gogo handler, and we have a protov2 counterparty. + return func(ctx context.Context, request gogoproto.Message) (gogoproto.Message, error) { + switch m := request.(type) { + case proto2.Message: + // we need to marshal and unmarshal the request. + requestBytes, err := proto2.Marshal(m) + if err != nil { + return nil, err + } + resp, err := method.Handler(handler, ctx, func(msg any) error { + // unmarshal request into the message. + return cdc.Unmarshal(requestBytes, msg.(gogoproto.Message)) + }, nil) + if err != nil { + return nil, err + } + // the response is a gogo message, so we cannot just return it. + // since the request came as protov2, we expect the response + // to also be protov2. + respBytes, err := cdc.Marshal(resp.(gogoproto.Message)) + if err != nil { + return nil, err + } + // now we unmarshal back into a protov2 message. + protov2Resp := protov2RespType.New().Interface() + return protov2Resp.(gogoproto.Message), proto2.Unmarshal(respBytes, protov2Resp) + case gogoproto.Message: + // we can just call the handler after making a copy of the message, for safety reasons. + resp, err := method.Handler(handler, ctx, func(msg any) error { + gogoproto.Merge(msg.(gogoproto.Message), m) + return nil + }, nil) + if err != nil { + return nil, err + } + return resp.(gogoproto.Message), nil + default: + panic("unreachable") + } + }, nil +} + +// getFuncType returns the handler type for the given method. +// this is unfortunately hideous because we need to discover +// from the gRPC server implementer (handler) the method that +// matches the protobuf service method. This depends on the +// codegen semantics. +func getFuncType(handler any, method string) (reflect.Type, error) { + handlerType := reflect.TypeOf(handler) + methodType, exists := handlerType.MethodByName(method) + if !exists { + return nil, fmt.Errorf("method %s not found on handler", method) + } + return methodType.Type, nil +} From 8bad4875b325a0057848131eeac362c5c755c0c7 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Mon, 2 Oct 2023 12:53:22 +0200 Subject: [PATCH 02/10] address feedbacks --- baseapp/grpcrouter.go | 7 +-- baseapp/protocompat/protocompat.go | 69 +++++++++++++++++------------- 2 files changed, 43 insertions(+), 33 deletions(-) diff --git a/baseapp/grpcrouter.go b/baseapp/grpcrouter.go index c976da0c3831..9884eb8e8fb4 100644 --- a/baseapp/grpcrouter.go +++ b/baseapp/grpcrouter.go @@ -11,6 +11,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/encoding" "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/runtime/protoiface" "github.com/cosmos/cosmos-sdk/client/grpc/reflection" "github.com/cosmos/cosmos-sdk/codec" @@ -21,7 +22,7 @@ import ( // GRPCQueryRouter routes ABCI Query requests to GRPC handlers type GRPCQueryRouter struct { routes map[string]GRPCQueryHandler - handlerByMessageName map[string][]func(ctx context.Context, msg proto.Message) (proto.Message, error) + handlerByMessageName map[string][]func(ctx context.Context, msg protoiface.MessageV1) (protoiface.MessageV1, error) sdkCodec codec.BinaryCodec cdc encoding.Codec serviceData []serviceData @@ -39,7 +40,7 @@ var _ gogogrpc.Server = &GRPCQueryRouter{} func NewGRPCQueryRouter() *GRPCQueryRouter { return &GRPCQueryRouter{ routes: map[string]GRPCQueryHandler{}, - handlerByMessageName: map[string][]func(ctx context.Context, msg proto.Message) (proto.Message, error){}, + handlerByMessageName: map[string][]func(ctx context.Context, msg protoiface.MessageV1) (protoiface.MessageV1, error){}, } } @@ -124,7 +125,7 @@ func (qrt *GRPCQueryRouter) registerABCIQueryHandler(sd *grpc.ServiceDesc, metho return nil } -func (qrt *GRPCQueryRouter) HandlersByRequestName(name string) []func(ctx context.Context, req proto.Message) (resp proto.Message, err error) { +func (qrt *GRPCQueryRouter) HandlersByRequestName(name string) []func(ctx context.Context, req protoiface.MessageV1) (resp protoiface.MessageV1, err error) { return qrt.handlerByMessageName[name] } diff --git a/baseapp/protocompat/protocompat.go b/baseapp/protocompat/protocompat.go index cecf39e9105b..2a28053193d3 100644 --- a/baseapp/protocompat/protocompat.go +++ b/baseapp/protocompat/protocompat.go @@ -11,6 +11,7 @@ import ( proto2 "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/reflect/protoregistry" + "google.golang.org/protobuf/runtime/protoiface" ) var ( @@ -18,7 +19,7 @@ var ( protov2Type = reflect.TypeOf((*proto2.Message)(nil)).Elem() ) -type Handler = func(ctx context.Context, message gogoproto.Message) (gogoproto.Message, error) +type Handler = func(ctx context.Context, message protoiface.MessageV1) (protoiface.MessageV1, error) func MakeHybridHandler(cdc codec.BinaryCodec, sd *grpc.ServiceDesc, method grpc.MethodDesc, handler interface{}) (Handler, error) { methodFullName := protoreflect.FullName(fmt.Sprintf("%s.%s", sd.ServiceName, method.MethodName)) @@ -31,21 +32,14 @@ func MakeHybridHandler(cdc codec.BinaryCodec, sd *grpc.ServiceDesc, method grpc. return nil, fmt.Errorf("invalid method descriptor %s", methodFullName) } - funcType, err := getFuncType(handler, method.MethodName) + isProtov2Handler, err := isProtov2(method) if err != nil { return nil, err } - // the handler function takes two arguments: context.Context and proto.Message - // since it is a method, the first argument is the receiver, which we don't care about. - // the second argument is the proto.Message, which we need to know the type of. - inputType := funcType.In(2) - switch { - case inputType.Implements(protov2Type): + if isProtov2Handler { return makeProtoV2HybridHandler(methodDesc, cdc, method, handler) - case inputType.Implements(gogoType): + } else { return makeGogoHybridHandler(methodDesc, cdc, method, handler) - default: - return nil, fmt.Errorf("invalid method handler type %T, input does not implement", method.Handler) } } @@ -54,7 +48,7 @@ func makeProtoV2HybridHandler(prefMethod protoreflect.MethodDescriptor, cdc code // it's a protov2 handler, if a gogo counterparty is not found we cannot handle gogo messages. gogoRespType := gogoproto.MessageType(string(prefMethod.Output().FullName())) if gogoRespType == nil { - return func(ctx context.Context, request gogoproto.Message) (gogoproto.Message, error) { + return func(ctx context.Context, request protoiface.MessageV1) (protoiface.MessageV1, error) { protov2Request, ok := request.(proto2.Message) if !ok { return nil, fmt.Errorf("invalid request type %T, method %s does not accept gogoproto messages", request, prefMethod.FullName()) @@ -66,11 +60,11 @@ func makeProtoV2HybridHandler(prefMethod protoreflect.MethodDescriptor, cdc code if err != nil { return nil, err } - return resp.(gogoproto.Message), nil + return resp.(protoiface.MessageV1), nil }, nil } gogoRespType = gogoRespType.Elem() // we need the non pointer type - return func(ctx context.Context, request gogoproto.Message) (gogoproto.Message, error) { + return func(ctx context.Context, request protoiface.MessageV1) (protoiface.MessageV1, error) { // we check if the request is a protov2 message. switch m := request.(type) { case proto2.Message: @@ -82,7 +76,7 @@ func makeProtoV2HybridHandler(prefMethod protoreflect.MethodDescriptor, cdc code if err != nil { return nil, err } - return resp.(gogoproto.Message), nil + return resp.(protoiface.MessageV1), nil case gogoproto.Message: // we need to marshal and unmarshal the request. requestBytes, err := cdc.Marshal(m) @@ -118,7 +112,7 @@ func makeGogoHybridHandler(prefMethod protoreflect.MethodDescriptor, cdc codec.B protov2RespType, err := protoregistry.GlobalTypes.FindMessageByName(prefMethod.Output().FullName()) if err != nil { // this can only be a gogo message. - return func(ctx context.Context, request gogoproto.Message) (gogoproto.Message, error) { + return func(ctx context.Context, request protoiface.MessageV1) (protoiface.MessageV1, error) { _, ok := request.(proto2.Message) if ok { return nil, fmt.Errorf("invalid request type %T, method %s does not accept protov2 messages", request, prefMethod.FullName()) @@ -131,11 +125,11 @@ func makeGogoHybridHandler(prefMethod protoreflect.MethodDescriptor, cdc codec.B if err != nil { return nil, err } - return resp.(gogoproto.Message), nil + return resp.(protoiface.MessageV1), nil }, nil } // this is a gogo handler, and we have a protov2 counterparty. - return func(ctx context.Context, request gogoproto.Message) (gogoproto.Message, error) { + return func(ctx context.Context, request protoiface.MessageV1) (protoiface.MessageV1, error) { switch m := request.(type) { case proto2.Message: // we need to marshal and unmarshal the request. @@ -169,23 +163,38 @@ func makeGogoHybridHandler(prefMethod protoreflect.MethodDescriptor, cdc codec.B if err != nil { return nil, err } - return resp.(gogoproto.Message), nil + return resp.(protoiface.MessageV1), nil default: panic("unreachable") } }, nil } -// getFuncType returns the handler type for the given method. -// this is unfortunately hideous because we need to discover -// from the gRPC server implementer (handler) the method that -// matches the protobuf service method. This depends on the -// codegen semantics. -func getFuncType(handler any, method string) (reflect.Type, error) { - handlerType := reflect.TypeOf(handler) - methodType, exists := handlerType.MethodByName(method) - if !exists { - return nil, fmt.Errorf("method %s not found on handler", method) +// isProtov2 returns true if the given method accepts protov2 messages. +// Returns false if it does not. +// It uses the decoder function passed to the method handler to determine +// the type. Since the decoder function is passed in by the concrete implementer the expected +// message where bytes are unmarshaled to, we can use that to determine the type. +func isProtov2(md grpc.MethodDesc) (isV2Type bool, err error) { + pullRequestType := func(msg interface{}) error { + typ := reflect.TypeOf(msg) + switch { + case typ.Implements(protov2Type): + isV2Type = true + return nil + case typ.Implements(gogoType): + isV2Type = false + return nil + default: + return fmt.Errorf("invalid request type %T, expected protov2 or gogo message", msg) + } + } + // doNotExecute is a dummy handler that stops the request execution. + doNotExecute := func(_ context.Context, _ any, _ *grpc.UnaryServerInfo, _ grpc.UnaryHandler) (any, error) { + return nil, nil } - return methodType.Type, nil + // we are allowed to pass in a nil context and nil request, since we are not actually executing the request. + // this is made possible by the doNotExecute function which immediately returns without calling other handlers. + _, _ = md.Handler(nil, nil, pullRequestType, doNotExecute) + return } From a31f01ab13068a06fdee40363dff367b635edfdd Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Tue, 3 Oct 2023 12:02:16 +0200 Subject: [PATCH 03/10] move to internal pkg --- baseapp/grpcrouter.go | 2 +- baseapp/{ => internal}/protocompat/protocompat.go | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename baseapp/{ => internal}/protocompat/protocompat.go (100%) diff --git a/baseapp/grpcrouter.go b/baseapp/grpcrouter.go index 9884eb8e8fb4..41c434c13e98 100644 --- a/baseapp/grpcrouter.go +++ b/baseapp/grpcrouter.go @@ -5,7 +5,7 @@ import ( "fmt" abci "github.com/cometbft/cometbft/abci/types" - "github.com/cosmos/cosmos-sdk/baseapp/protocompat" + "github.com/cosmos/cosmos-sdk/baseapp/internal/protocompat" gogogrpc "github.com/cosmos/gogoproto/grpc" "github.com/cosmos/gogoproto/proto" "google.golang.org/grpc" diff --git a/baseapp/protocompat/protocompat.go b/baseapp/internal/protocompat/protocompat.go similarity index 100% rename from baseapp/protocompat/protocompat.go rename to baseapp/internal/protocompat/protocompat.go From c4169b9e097ae99e26166e5102773cbc4d3ea884 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Fri, 6 Oct 2023 12:00:14 +0200 Subject: [PATCH 04/10] refactor testing --- baseapp/grpcrouter.go | 2 +- baseapp/grpcrouter_test.go | 71 +++++++++++++++------ baseapp/internal/protocompat/protocompat.go | 3 +- 3 files changed, 53 insertions(+), 23 deletions(-) diff --git a/baseapp/grpcrouter.go b/baseapp/grpcrouter.go index 41c434c13e98..bf7539f70c77 100644 --- a/baseapp/grpcrouter.go +++ b/baseapp/grpcrouter.go @@ -5,7 +5,6 @@ import ( "fmt" abci "github.com/cometbft/cometbft/abci/types" - "github.com/cosmos/cosmos-sdk/baseapp/internal/protocompat" gogogrpc "github.com/cosmos/gogoproto/grpc" "github.com/cosmos/gogoproto/proto" "google.golang.org/grpc" @@ -13,6 +12,7 @@ import ( "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/runtime/protoiface" + "github.com/cosmos/cosmos-sdk/baseapp/internal/protocompat" "github.com/cosmos/cosmos-sdk/client/grpc/reflection" "github.com/cosmos/cosmos-sdk/codec" codectypes "github.com/cosmos/cosmos-sdk/codec/types" diff --git a/baseapp/grpcrouter_test.go b/baseapp/grpcrouter_test.go index 849bc15d190c..c8c10bef812e 100644 --- a/baseapp/grpcrouter_test.go +++ b/baseapp/grpcrouter_test.go @@ -5,11 +5,12 @@ import ( "sync" "testing" - "cosmossdk.io/depinject" - "cosmossdk.io/log" dbm "github.com/cosmos/cosmos-db" "github.com/stretchr/testify/require" + "cosmossdk.io/depinject" + "cosmossdk.io/log" + "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/runtime" @@ -50,26 +51,54 @@ func TestGRPCQueryRouter(t *testing.T) { require.NoError(t, err) require.NotNil(t, res3) require.Equal(t, spot, res3.HasAnimal.Animal.GetCachedValue()) +} - // test getting the handler by name - handlers := qr.HandlersByRequestName("testpb.EchoRequest") - require.NotNil(t, handlers) - require.Len(t, handlers, 1) - handler := handlers[0] - // sending a protov2 message should work, and return a protov2 message - resp, err := handler(helper.Ctx, &testdata_pulsar.EchoRequest{Message: "hello"}) - require.Nil(t, err) - require.NotNil(t, resp) - protov2Resp, ok := resp.(*testdata_pulsar.EchoResponse) - require.True(t, ok) - require.Equal(t, "hello", protov2Resp.Message) - // also sending a protov1 message should work, and return a gogoproto message - resp, err = handler(helper.Ctx, &testdata.EchoRequest{Message: "hello"}) - require.Nil(t, err) - require.NotNil(t, resp) - gogoprotoResp, ok := resp.(*testdata.EchoResponse) - require.True(t, ok) - require.Equal(t, "hello", gogoprotoResp.Message) +func TestGRPCRouterHybridHandlers(t *testing.T) { + assertRouterBehaviour := func(t *testing.T, helper *baseapp.QueryServiceTestHelper) { + // test getting the handler by name + handlers := helper.GRPCQueryRouter.HandlersByRequestName("testpb.EchoRequest") + require.NotNil(t, handlers) + require.Len(t, handlers, 1) + handler := handlers[0] + // sending a protov2 message should work, and return a protov2 message + resp, err := handler(helper.Ctx, &testdata_pulsar.EchoRequest{Message: "hello"}) + require.Nil(t, err) + require.NotNil(t, resp) + protov2Resp, ok := resp.(*testdata_pulsar.EchoResponse) + require.True(t, ok) + require.Equal(t, "hello", protov2Resp.Message) + // also sending a protov1 message should work, and return a gogoproto message + resp, err = handler(helper.Ctx, &testdata.EchoRequest{Message: "hello"}) + require.Nil(t, err) + require.NotNil(t, resp) + gogoprotoResp, ok := resp.(*testdata.EchoResponse) + require.True(t, ok) + require.Equal(t, "hello", gogoprotoResp.Message) + } + + t.Run("protov2 server", func(t *testing.T) { + qr := baseapp.NewGRPCQueryRouter() + interfaceRegistry := testdata.NewTestInterfaceRegistry() + qr.SetInterfaceRegistry(interfaceRegistry) + testdata_pulsar.RegisterQueryServer(qr, testdata_pulsar.QueryImpl{}) + helper := &baseapp.QueryServiceTestHelper{ + GRPCQueryRouter: qr, + Ctx: sdk.Context{}.WithContext(context.Background()), + } + assertRouterBehaviour(t, helper) + }) + + t.Run("gogoproto server", func(t *testing.T) { + qr := baseapp.NewGRPCQueryRouter() + interfaceRegistry := testdata.NewTestInterfaceRegistry() + qr.SetInterfaceRegistry(interfaceRegistry) + testdata.RegisterQueryServer(qr, testdata.QueryImpl{}) + helper := &baseapp.QueryServiceTestHelper{ + GRPCQueryRouter: qr, + Ctx: sdk.Context{}.WithContext(context.Background()), + } + assertRouterBehaviour(t, helper) + }) } func TestRegisterQueryServiceTwice(t *testing.T) { diff --git a/baseapp/internal/protocompat/protocompat.go b/baseapp/internal/protocompat/protocompat.go index 2a28053193d3..233187016cef 100644 --- a/baseapp/internal/protocompat/protocompat.go +++ b/baseapp/internal/protocompat/protocompat.go @@ -5,13 +5,14 @@ import ( "fmt" "reflect" - "github.com/cosmos/cosmos-sdk/codec" gogoproto "github.com/cosmos/gogoproto/proto" "google.golang.org/grpc" proto2 "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/reflect/protoregistry" "google.golang.org/protobuf/runtime/protoiface" + + "github.com/cosmos/cosmos-sdk/codec" ) var ( From 986b44e7dfc0ca3354f988e237028b06de3f2343 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Fri, 6 Oct 2023 12:14:06 +0200 Subject: [PATCH 05/10] los lintos --- baseapp/grpcrouter_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/baseapp/grpcrouter_test.go b/baseapp/grpcrouter_test.go index c8c10bef812e..f9dc3e9a9a2a 100644 --- a/baseapp/grpcrouter_test.go +++ b/baseapp/grpcrouter_test.go @@ -54,7 +54,7 @@ func TestGRPCQueryRouter(t *testing.T) { } func TestGRPCRouterHybridHandlers(t *testing.T) { - assertRouterBehaviour := func(t *testing.T, helper *baseapp.QueryServiceTestHelper) { + assertRouterBehaviour := func(helper *baseapp.QueryServiceTestHelper) { // test getting the handler by name handlers := helper.GRPCQueryRouter.HandlersByRequestName("testpb.EchoRequest") require.NotNil(t, handlers) @@ -85,7 +85,7 @@ func TestGRPCRouterHybridHandlers(t *testing.T) { GRPCQueryRouter: qr, Ctx: sdk.Context{}.WithContext(context.Background()), } - assertRouterBehaviour(t, helper) + assertRouterBehaviour(helper) }) t.Run("gogoproto server", func(t *testing.T) { @@ -97,7 +97,7 @@ func TestGRPCRouterHybridHandlers(t *testing.T) { GRPCQueryRouter: qr, Ctx: sdk.Context{}.WithContext(context.Background()), } - assertRouterBehaviour(t, helper) + assertRouterBehaviour(helper) }) } From cc65bd5f975659b2225883d88dd1b349cf5b9e57 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Fri, 6 Oct 2023 13:22:01 +0200 Subject: [PATCH 06/10] fix isProtov2 error handling --- baseapp/internal/protocompat/protocompat.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/baseapp/internal/protocompat/protocompat.go b/baseapp/internal/protocompat/protocompat.go index 233187016cef..bbe1581819d8 100644 --- a/baseapp/internal/protocompat/protocompat.go +++ b/baseapp/internal/protocompat/protocompat.go @@ -187,7 +187,8 @@ func isProtov2(md grpc.MethodDesc) (isV2Type bool, err error) { isV2Type = false return nil default: - return fmt.Errorf("invalid request type %T, expected protov2 or gogo message", msg) + err = fmt.Errorf("invalid request type %T, expected protov2 or gogo message", msg) + return nil } } // doNotExecute is a dummy handler that stops the request execution. From a76f1d36e1aa12423435887766a5cd385060b34e Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Mon, 9 Oct 2023 11:11:13 +0200 Subject: [PATCH 07/10] change signature to be more gRPC compliant --- baseapp/grpcrouter.go | 6 +- baseapp/grpcrouter_test.go | 17 ++--- baseapp/internal/protocompat/protocompat.go | 73 +++++++++++---------- 3 files changed, 48 insertions(+), 48 deletions(-) diff --git a/baseapp/grpcrouter.go b/baseapp/grpcrouter.go index bf7539f70c77..8836a5fd94ff 100644 --- a/baseapp/grpcrouter.go +++ b/baseapp/grpcrouter.go @@ -22,7 +22,7 @@ import ( // GRPCQueryRouter routes ABCI Query requests to GRPC handlers type GRPCQueryRouter struct { routes map[string]GRPCQueryHandler - handlerByMessageName map[string][]func(ctx context.Context, msg protoiface.MessageV1) (protoiface.MessageV1, error) + handlerByMessageName map[string][]func(ctx context.Context, req protoiface.MessageV1, resp protoiface.MessageV1) error sdkCodec codec.BinaryCodec cdc encoding.Codec serviceData []serviceData @@ -40,7 +40,7 @@ var _ gogogrpc.Server = &GRPCQueryRouter{} func NewGRPCQueryRouter() *GRPCQueryRouter { return &GRPCQueryRouter{ routes: map[string]GRPCQueryHandler{}, - handlerByMessageName: map[string][]func(ctx context.Context, msg protoiface.MessageV1) (protoiface.MessageV1, error){}, + handlerByMessageName: map[string][]func(ctx context.Context, req protoiface.MessageV1, resp protoiface.MessageV1) error{}, } } @@ -125,7 +125,7 @@ func (qrt *GRPCQueryRouter) registerABCIQueryHandler(sd *grpc.ServiceDesc, metho return nil } -func (qrt *GRPCQueryRouter) HandlersByRequestName(name string) []func(ctx context.Context, req protoiface.MessageV1) (resp protoiface.MessageV1, err error) { +func (qrt *GRPCQueryRouter) HandlersByRequestName(name string) []func(ctx context.Context, req protoiface.MessageV1, resp protoiface.MessageV1) error { return qrt.handlerByMessageName[name] } diff --git a/baseapp/grpcrouter_test.go b/baseapp/grpcrouter_test.go index f9dc3e9a9a2a..238deecc1be3 100644 --- a/baseapp/grpcrouter_test.go +++ b/baseapp/grpcrouter_test.go @@ -61,19 +61,14 @@ func TestGRPCRouterHybridHandlers(t *testing.T) { require.Len(t, handlers, 1) handler := handlers[0] // sending a protov2 message should work, and return a protov2 message - resp, err := handler(helper.Ctx, &testdata_pulsar.EchoRequest{Message: "hello"}) + v2Resp := new(testdata_pulsar.EchoResponse) + err := handler(helper.Ctx, &testdata_pulsar.EchoRequest{Message: "hello"}, v2Resp) require.Nil(t, err) - require.NotNil(t, resp) - protov2Resp, ok := resp.(*testdata_pulsar.EchoResponse) - require.True(t, ok) - require.Equal(t, "hello", protov2Resp.Message) + require.Equal(t, "hello", v2Resp.Message) // also sending a protov1 message should work, and return a gogoproto message - resp, err = handler(helper.Ctx, &testdata.EchoRequest{Message: "hello"}) - require.Nil(t, err) - require.NotNil(t, resp) - gogoprotoResp, ok := resp.(*testdata.EchoResponse) - require.True(t, ok) - require.Equal(t, "hello", gogoprotoResp.Message) + gogoResp := new(testdata.EchoResponse) + err = handler(helper.Ctx, &testdata.EchoRequest{Message: "hello"}, gogoResp) + require.Equal(t, "hello", gogoResp.Message) } t.Run("protov2 server", func(t *testing.T) { diff --git a/baseapp/internal/protocompat/protocompat.go b/baseapp/internal/protocompat/protocompat.go index bbe1581819d8..24395209c5f6 100644 --- a/baseapp/internal/protocompat/protocompat.go +++ b/baseapp/internal/protocompat/protocompat.go @@ -20,7 +20,7 @@ var ( protov2Type = reflect.TypeOf((*proto2.Message)(nil)).Elem() ) -type Handler = func(ctx context.Context, message protoiface.MessageV1) (protoiface.MessageV1, error) +type Handler = func(ctx context.Context, request protoiface.MessageV1, response protoiface.MessageV1) error func MakeHybridHandler(cdc codec.BinaryCodec, sd *grpc.ServiceDesc, method grpc.MethodDesc, handler interface{}) (Handler, error) { methodFullName := protoreflect.FullName(fmt.Sprintf("%s.%s", sd.ServiceName, method.MethodName)) @@ -47,27 +47,28 @@ func MakeHybridHandler(cdc codec.BinaryCodec, sd *grpc.ServiceDesc, method grpc. // makeProtoV2HybridHandler returns a handler that can handle both gogo and protov2 messages. func makeProtoV2HybridHandler(prefMethod protoreflect.MethodDescriptor, cdc codec.BinaryCodec, method grpc.MethodDesc, handler any) (Handler, error) { // it's a protov2 handler, if a gogo counterparty is not found we cannot handle gogo messages. - gogoRespType := gogoproto.MessageType(string(prefMethod.Output().FullName())) - if gogoRespType == nil { - return func(ctx context.Context, request protoiface.MessageV1) (protoiface.MessageV1, error) { - protov2Request, ok := request.(proto2.Message) + gogoExists := gogoproto.MessageType(string(prefMethod.Output().FullName())) != nil + if !gogoExists { + return func(ctx context.Context, inReq protoiface.MessageV1, outResp protoiface.MessageV1) error { + protov2Request, ok := inReq.(proto2.Message) if !ok { - return nil, fmt.Errorf("invalid request type %T, method %s does not accept gogoproto messages", request, prefMethod.FullName()) + return fmt.Errorf("invalid request type %T, method %s does not accept gogoproto messages", inReq, prefMethod.FullName()) } resp, err := method.Handler(handler, ctx, func(msg any) error { proto2.Merge(msg.(proto2.Message), protov2Request) return nil }, nil) if err != nil { - return nil, err + return err } - return resp.(protoiface.MessageV1), nil + // merge on the resp + proto2.Merge(outResp.(proto2.Message), resp.(proto2.Message)) + return nil }, nil } - gogoRespType = gogoRespType.Elem() // we need the non pointer type - return func(ctx context.Context, request protoiface.MessageV1) (protoiface.MessageV1, error) { + return func(ctx context.Context, inReq protoiface.MessageV1, outResp protoiface.MessageV1) error { // we check if the request is a protov2 message. - switch m := request.(type) { + switch m := inReq.(type) { case proto2.Message: // we can just call the handler after making a copy of the message, for safety reasons. resp, err := method.Handler(handler, ctx, func(msg any) error { @@ -75,33 +76,34 @@ func makeProtoV2HybridHandler(prefMethod protoreflect.MethodDescriptor, cdc code return nil }, nil) if err != nil { - return nil, err + return err } - return resp.(protoiface.MessageV1), nil + // merge on the resp + proto2.Merge(outResp.(proto2.Message), resp.(proto2.Message)) + return nil case gogoproto.Message: // we need to marshal and unmarshal the request. requestBytes, err := cdc.Marshal(m) if err != nil { - return nil, err + return err } resp, err := method.Handler(handler, ctx, func(msg any) error { // unmarshal request into the message. return proto2.Unmarshal(requestBytes, msg.(proto2.Message)) }, nil) if err != nil { - return nil, err + return err } // the response is a protov2 message, so we cannot just return it. // since the request came as gogoproto, we expect the response // to also be gogoproto. respBytes, err := proto2.Marshal(resp.(proto2.Message)) if err != nil { - return nil, err + return err } // unmarshal response into a gogo message. - gogoResp := reflect.New(gogoRespType).Interface().(gogoproto.Message) - return gogoResp, cdc.Unmarshal(respBytes, gogoResp) + return cdc.Unmarshal(respBytes, outResp.(gogoproto.Message)) default: panic("unreachable") } @@ -110,51 +112,52 @@ func makeProtoV2HybridHandler(prefMethod protoreflect.MethodDescriptor, cdc code func makeGogoHybridHandler(prefMethod protoreflect.MethodDescriptor, cdc codec.BinaryCodec, method grpc.MethodDesc, handler any) (Handler, error) { // it's a gogo handler, we check if the existing protov2 counterparty exists. - protov2RespType, err := protoregistry.GlobalTypes.FindMessageByName(prefMethod.Output().FullName()) + _, err := protoregistry.GlobalTypes.FindMessageByName(prefMethod.Output().FullName()) if err != nil { // this can only be a gogo message. - return func(ctx context.Context, request protoiface.MessageV1) (protoiface.MessageV1, error) { - _, ok := request.(proto2.Message) + return func(ctx context.Context, inReq protoiface.MessageV1, outResp protoiface.MessageV1) error { + _, ok := inReq.(proto2.Message) if ok { - return nil, fmt.Errorf("invalid request type %T, method %s does not accept protov2 messages", request, prefMethod.FullName()) + return fmt.Errorf("invalid request type %T, method %s does not accept protov2 messages", inReq, prefMethod.FullName()) } resp, err := method.Handler(handler, ctx, func(msg any) error { // merge! - gogoproto.Merge(msg.(gogoproto.Message), request) + gogoproto.Merge(msg.(gogoproto.Message), inReq) return nil }, nil) if err != nil { - return nil, err + return err } - return resp.(protoiface.MessageV1), nil + // merge resp + gogoproto.Merge(outResp.(gogoproto.Message), resp.(gogoproto.Message)) + return nil }, nil } // this is a gogo handler, and we have a protov2 counterparty. - return func(ctx context.Context, request protoiface.MessageV1) (protoiface.MessageV1, error) { - switch m := request.(type) { + return func(ctx context.Context, inReq protoiface.MessageV1, outResp protoiface.MessageV1) error { + switch m := inReq.(type) { case proto2.Message: // we need to marshal and unmarshal the request. requestBytes, err := proto2.Marshal(m) if err != nil { - return nil, err + return err } resp, err := method.Handler(handler, ctx, func(msg any) error { // unmarshal request into the message. return cdc.Unmarshal(requestBytes, msg.(gogoproto.Message)) }, nil) if err != nil { - return nil, err + return err } // the response is a gogo message, so we cannot just return it. // since the request came as protov2, we expect the response // to also be protov2. respBytes, err := cdc.Marshal(resp.(gogoproto.Message)) if err != nil { - return nil, err + return err } // now we unmarshal back into a protov2 message. - protov2Resp := protov2RespType.New().Interface() - return protov2Resp.(gogoproto.Message), proto2.Unmarshal(respBytes, protov2Resp) + return proto2.Unmarshal(respBytes, outResp.(proto2.Message)) case gogoproto.Message: // we can just call the handler after making a copy of the message, for safety reasons. resp, err := method.Handler(handler, ctx, func(msg any) error { @@ -162,9 +165,11 @@ func makeGogoHybridHandler(prefMethod protoreflect.MethodDescriptor, cdc codec.B return nil }, nil) if err != nil { - return nil, err + return err } - return resp.(protoiface.MessageV1), nil + // merge on the resp + gogoproto.Merge(outResp.(gogoproto.Message), resp.(gogoproto.Message)) + return nil default: panic("unreachable") } From bb645be376439b65da22725d4d80666db961d33c Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Mon, 9 Oct 2023 11:19:31 +0200 Subject: [PATCH 08/10] explain why docs --- baseapp/internal/protocompat/protocompat.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/baseapp/internal/protocompat/protocompat.go b/baseapp/internal/protocompat/protocompat.go index 24395209c5f6..0cae572b1e11 100644 --- a/baseapp/internal/protocompat/protocompat.go +++ b/baseapp/internal/protocompat/protocompat.go @@ -121,14 +121,14 @@ func makeGogoHybridHandler(prefMethod protoreflect.MethodDescriptor, cdc codec.B return fmt.Errorf("invalid request type %T, method %s does not accept protov2 messages", inReq, prefMethod.FullName()) } resp, err := method.Handler(handler, ctx, func(msg any) error { - // merge! + // merge! ref: https://github.com/cosmos/cosmos-sdk/issues/18003 gogoproto.Merge(msg.(gogoproto.Message), inReq) return nil }, nil) if err != nil { return err } - // merge resp + // merge resp, ref: https://github.com/cosmos/cosmos-sdk/issues/18003 gogoproto.Merge(outResp.(gogoproto.Message), resp.(gogoproto.Message)) return nil }, nil @@ -161,13 +161,14 @@ func makeGogoHybridHandler(prefMethod protoreflect.MethodDescriptor, cdc codec.B case gogoproto.Message: // we can just call the handler after making a copy of the message, for safety reasons. resp, err := method.Handler(handler, ctx, func(msg any) error { + // ref: https://github.com/cosmos/cosmos-sdk/issues/18003 gogoproto.Merge(msg.(gogoproto.Message), m) return nil }, nil) if err != nil { return err } - // merge on the resp + // merge on the resp, ref: https://github.com/cosmos/cosmos-sdk/issues/18003 gogoproto.Merge(outResp.(gogoproto.Message), resp.(gogoproto.Message)) return nil default: From 17319e2d2215bcb525701cf82e88115b0091c0fc Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Mon, 9 Oct 2023 12:32:11 +0200 Subject: [PATCH 09/10] lint --- baseapp/grpcrouter.go | 6 +++--- baseapp/grpcrouter_test.go | 1 + baseapp/internal/protocompat/protocompat.go | 10 +++++----- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/baseapp/grpcrouter.go b/baseapp/grpcrouter.go index 8836a5fd94ff..78a87c574e2f 100644 --- a/baseapp/grpcrouter.go +++ b/baseapp/grpcrouter.go @@ -22,7 +22,7 @@ import ( // GRPCQueryRouter routes ABCI Query requests to GRPC handlers type GRPCQueryRouter struct { routes map[string]GRPCQueryHandler - handlerByMessageName map[string][]func(ctx context.Context, req protoiface.MessageV1, resp protoiface.MessageV1) error + handlerByMessageName map[string][]func(ctx context.Context, req, resp protoiface.MessageV1) error sdkCodec codec.BinaryCodec cdc encoding.Codec serviceData []serviceData @@ -40,7 +40,7 @@ var _ gogogrpc.Server = &GRPCQueryRouter{} func NewGRPCQueryRouter() *GRPCQueryRouter { return &GRPCQueryRouter{ routes: map[string]GRPCQueryHandler{}, - handlerByMessageName: map[string][]func(ctx context.Context, req protoiface.MessageV1, resp protoiface.MessageV1) error{}, + handlerByMessageName: map[string][]func(ctx context.Context, req, resp protoiface.MessageV1) error{}, } } @@ -125,7 +125,7 @@ func (qrt *GRPCQueryRouter) registerABCIQueryHandler(sd *grpc.ServiceDesc, metho return nil } -func (qrt *GRPCQueryRouter) HandlersByRequestName(name string) []func(ctx context.Context, req protoiface.MessageV1, resp protoiface.MessageV1) error { +func (qrt *GRPCQueryRouter) HandlersByRequestName(name string) []func(ctx context.Context, req, resp protoiface.MessageV1) error { return qrt.handlerByMessageName[name] } diff --git a/baseapp/grpcrouter_test.go b/baseapp/grpcrouter_test.go index 238deecc1be3..a7e338daaf1b 100644 --- a/baseapp/grpcrouter_test.go +++ b/baseapp/grpcrouter_test.go @@ -68,6 +68,7 @@ func TestGRPCRouterHybridHandlers(t *testing.T) { // also sending a protov1 message should work, and return a gogoproto message gogoResp := new(testdata.EchoResponse) err = handler(helper.Ctx, &testdata.EchoRequest{Message: "hello"}, gogoResp) + require.NoError(t, err) require.Equal(t, "hello", gogoResp.Message) } diff --git a/baseapp/internal/protocompat/protocompat.go b/baseapp/internal/protocompat/protocompat.go index 0cae572b1e11..17ebac8ad97e 100644 --- a/baseapp/internal/protocompat/protocompat.go +++ b/baseapp/internal/protocompat/protocompat.go @@ -20,7 +20,7 @@ var ( protov2Type = reflect.TypeOf((*proto2.Message)(nil)).Elem() ) -type Handler = func(ctx context.Context, request protoiface.MessageV1, response protoiface.MessageV1) error +type Handler = func(ctx context.Context, request, response protoiface.MessageV1) error func MakeHybridHandler(cdc codec.BinaryCodec, sd *grpc.ServiceDesc, method grpc.MethodDesc, handler interface{}) (Handler, error) { methodFullName := protoreflect.FullName(fmt.Sprintf("%s.%s", sd.ServiceName, method.MethodName)) @@ -49,7 +49,7 @@ func makeProtoV2HybridHandler(prefMethod protoreflect.MethodDescriptor, cdc code // it's a protov2 handler, if a gogo counterparty is not found we cannot handle gogo messages. gogoExists := gogoproto.MessageType(string(prefMethod.Output().FullName())) != nil if !gogoExists { - return func(ctx context.Context, inReq protoiface.MessageV1, outResp protoiface.MessageV1) error { + return func(ctx context.Context, inReq, outResp protoiface.MessageV1) error { protov2Request, ok := inReq.(proto2.Message) if !ok { return fmt.Errorf("invalid request type %T, method %s does not accept gogoproto messages", inReq, prefMethod.FullName()) @@ -66,7 +66,7 @@ func makeProtoV2HybridHandler(prefMethod protoreflect.MethodDescriptor, cdc code return nil }, nil } - return func(ctx context.Context, inReq protoiface.MessageV1, outResp protoiface.MessageV1) error { + return func(ctx context.Context, inReq, outResp protoiface.MessageV1) error { // we check if the request is a protov2 message. switch m := inReq.(type) { case proto2.Message: @@ -115,7 +115,7 @@ func makeGogoHybridHandler(prefMethod protoreflect.MethodDescriptor, cdc codec.B _, err := protoregistry.GlobalTypes.FindMessageByName(prefMethod.Output().FullName()) if err != nil { // this can only be a gogo message. - return func(ctx context.Context, inReq protoiface.MessageV1, outResp protoiface.MessageV1) error { + return func(ctx context.Context, inReq, outResp protoiface.MessageV1) error { _, ok := inReq.(proto2.Message) if ok { return fmt.Errorf("invalid request type %T, method %s does not accept protov2 messages", inReq, prefMethod.FullName()) @@ -134,7 +134,7 @@ func makeGogoHybridHandler(prefMethod protoreflect.MethodDescriptor, cdc codec.B }, nil } // this is a gogo handler, and we have a protov2 counterparty. - return func(ctx context.Context, inReq protoiface.MessageV1, outResp protoiface.MessageV1) error { + return func(ctx context.Context, inReq, outResp protoiface.MessageV1) error { switch m := inReq.(type) { case proto2.Message: // we need to marshal and unmarshal the request. From 3e6c0436b73d24916b31d2d8f52220bfb8cc7850 Mon Sep 17 00:00:00 2001 From: unknown unknown Date: Tue, 10 Oct 2023 20:22:15 +0200 Subject: [PATCH 10/10] add docs to fields --- baseapp/grpcrouter.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/baseapp/grpcrouter.go b/baseapp/grpcrouter.go index 78a87c574e2f..2c3980ccbbac 100644 --- a/baseapp/grpcrouter.go +++ b/baseapp/grpcrouter.go @@ -21,11 +21,17 @@ import ( // GRPCQueryRouter routes ABCI Query requests to GRPC handlers type GRPCQueryRouter struct { - routes map[string]GRPCQueryHandler + // routes maps query handlers used in ABCIQuery. + routes map[string]GRPCQueryHandler + // handlerByMessageName maps the request name to the handler. It is a hybrid handler which seamlessly + // handles both gogo and protov2 messages. handlerByMessageName map[string][]func(ctx context.Context, req, resp protoiface.MessageV1) error - sdkCodec codec.BinaryCodec - cdc encoding.Codec - serviceData []serviceData + // binaryCodec is used to encode/decode binary protobuf messages. + binaryCodec codec.BinaryCodec + // cdc is the gRPC codec used by the router to correctly unmarshal messages. + cdc encoding.Codec + // serviceData contains the gRPC services and their handlers. + serviceData []serviceData } // serviceData represents a gRPC service, along with its handler. @@ -141,7 +147,7 @@ func (qrt *GRPCQueryRouter) registerHandlerByMessageName(sd *grpc.ServiceDesc, m return fmt.Errorf("invalid method descriptor %s", methodFullName) } inputName := methodDesc.Input().FullName() - methodHandler, err := protocompat.MakeHybridHandler(qrt.sdkCodec, sd, method, handler) + methodHandler, err := protocompat.MakeHybridHandler(qrt.binaryCodec, sd, method, handler) if err != nil { return err } @@ -152,7 +158,7 @@ func (qrt *GRPCQueryRouter) registerHandlerByMessageName(sd *grpc.ServiceDesc, m // SetInterfaceRegistry sets the interface registry for the router. This will // also register the interface reflection gRPC service. func (qrt *GRPCQueryRouter) SetInterfaceRegistry(interfaceRegistry codectypes.InterfaceRegistry) { - qrt.sdkCodec = codec.NewProtoCodec(interfaceRegistry) + qrt.binaryCodec = codec.NewProtoCodec(interfaceRegistry) // instantiate the codec qrt.cdc = codec.NewProtoCodec(interfaceRegistry).GRPCCodec() // Once we have an interface registry, we can register the interface