LaconicNetwork/cosmos-sdk
7
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

fix(baseapp): hybrid handlers do not drop any (#22866)

Browse Source
This commit is contained in:
testinginprod 2024-12-13 16:46:03 +01:00 committed by GitHub
parent 957e241716
commit 25cbf98983
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 35 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
baseapp/grpcrouter_test.go
View File

@ -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) {

28
baseapp/internal/protocompat/protocompat.go
View File

@ -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())
}