diff --git a/baseapp/grpcrouter_test.go b/baseapp/grpcrouter_test.go index b381fe8a277e..cc7e0476b2b3 100644 --- a/baseapp/grpcrouter_test.go +++ b/baseapp/grpcrouter_test.go @@ -95,6 +95,31 @@ func TestGRPCRouterHybridHandlers(t *testing.T) { } assertRouterBehaviour(helper) }) + + t.Run("any cached value is not dropped", func(t *testing.T) { + // ref: https://github.com/cosmos/cosmos-sdk/issues/22779 + qr := baseapp.NewGRPCQueryRouter() + interfaceRegistry := testdata.NewTestInterfaceRegistry() + testdata.RegisterInterfaces(interfaceRegistry) + qr.SetInterfaceRegistry(interfaceRegistry) + testdata.RegisterQueryServer(qr, testdata.QueryImpl{}) + helper := &baseapp.QueryServiceTestHelper{ + GRPCQueryRouter: qr, + Ctx: sdk.Context{}.WithContext(context.Background()), + } + + anyMsg, err := types.NewAnyWithValue(&testdata.Dog{}) + require.NoError(t, err) + + handler := qr.HybridHandlerByRequestName("testpb.TestAnyRequest")[0] + + resp := new(testdata.TestAnyResponse) + err = handler(helper.Ctx, &testdata.TestAnyRequest{ + AnyAnimal: anyMsg, + }, resp) + require.NoError(t, err) + require.NotNil(t, resp.HasAnimal.Animal.GetCachedValue()) + }) } func TestRegisterQueryServiceTwice(t *testing.T) { diff --git a/baseapp/internal/protocompat/protocompat.go b/baseapp/internal/protocompat/protocompat.go index 004efe635e2a..43c32df35376 100644 --- a/baseapp/internal/protocompat/protocompat.go +++ b/baseapp/internal/protocompat/protocompat.go @@ -6,7 +6,6 @@ import ( "reflect" gogoproto "github.com/cosmos/gogoproto/proto" - "github.com/golang/protobuf/proto" //nolint: staticcheck // needed because gogoproto.Merge does not work consistently. See NOTE: comments. "google.golang.org/grpc" proto2 "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" @@ -124,19 +123,13 @@ 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! ref: https://github.com/cosmos/cosmos-sdk/issues/18003 - // NOTE: using gogoproto.Merge will fail for some reason unknown to me, but - // using proto.Merge with gogo messages seems to work fine. - proto.Merge(msg.(gogoproto.Message), inReq) + setPointer(msg, inReq) return nil }, nil) if err != nil { return err } - // merge resp, ref: https://github.com/cosmos/cosmos-sdk/issues/18003 - // NOTE: using gogoproto.Merge will fail for some reason unknown to me, but - // using proto.Merge with gogo messages seems to work fine. - proto.Merge(outResp.(gogoproto.Message), resp.(gogoproto.Message)) + setPointer(outResp, resp) return nil }, nil } @@ -168,20 +161,13 @@ 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 - asGogoProto := msg.(gogoproto.Message) - // NOTE: using gogoproto.Merge will fail for some reason unknown to me, but - // using proto.Merge with gogo messages seems to work fine. - proto.Merge(asGogoProto, m) + setPointer(msg, m) return nil }, nil) if err != nil { return err } - // merge on the resp, ref: https://github.com/cosmos/cosmos-sdk/issues/18003 - // NOTE: using gogoproto.Merge will fail for some reason unknown to me, but - // using proto.Merge with gogo messages seems to work fine. - proto.Merge(outResp.(gogoproto.Message), resp.(gogoproto.Message)) + setPointer(outResp, resp) return nil default: panic("unreachable") @@ -246,3 +232,9 @@ func ResponseFullNameFromMethodDesc(sd *grpc.ServiceDesc, method grpc.MethodDesc } return methodDesc.Output().FullName(), nil } + +// since proto.Merge breaks due to the custom cosmos sdk any, we are forced to do this ugly setPointer hack. +// ref: https://github.com/cosmos/cosmos-sdk/issues/22779 +func setPointer(dst, src any) { + reflect.ValueOf(dst).Elem().Set(reflect.ValueOf(src).Elem()) +}