Skip to content

Commit

Permalink
fix(baseapp): protocompat.go gogoproto.Merge does not work with custo…
Browse files Browse the repository at this point in the history
…m types (#18654)

Co-authored-by: unknown unknown <unknown@unknown>
  • Loading branch information
testinginprod and unknown unknown authored Dec 8, 2023
1 parent 6218b5b commit dabf72c
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (simulation) [#17911](https://github.com/cosmos/cosmos-sdk/pull/17911) Fix all problems with executing command `make test-sim-custom-genesis-fast` for simulation test.
* (simulation) [#18196](https://github.com/cosmos/cosmos-sdk/pull/18196) Fix the problem of `validator set is empty after InitGenesis` in simulation test.
* (baseapp) [#18551](https://github.com/cosmos/cosmos-sdk/pull/18551) Fix SelectTxForProposal the calculation method of tx bytes size is inconsistent with CometBFT
* (baseapp) [#18653](https://github.com/cosmos/cosmos-sdk/pull/18654) Fixes an issue in which gogoproto.Merge does not work with gogoproto messages with custom types.

### API Breaking Changes

Expand Down
18 changes: 14 additions & 4 deletions baseapp/internal/protocompat/protocompat.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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"
Expand Down Expand Up @@ -125,14 +126,18 @@ func makeGogoHybridHandler(prefMethod protoreflect.MethodDescriptor, cdc codec.B
}
resp, err := method.Handler(handler, ctx, func(msg any) error {
// merge! ref: https://github.com/cosmos/cosmos-sdk/issues/18003
gogoproto.Merge(msg.(gogoproto.Message), inReq)
// 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)
return nil
}, nil)
if err != nil {
return err
}
// merge resp, ref: https://github.com/cosmos/cosmos-sdk/issues/18003
gogoproto.Merge(outResp.(gogoproto.Message), resp.(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(outResp.(gogoproto.Message), resp.(gogoproto.Message))
return nil
}, nil
}
Expand Down Expand Up @@ -165,14 +170,19 @@ func makeGogoHybridHandler(prefMethod protoreflect.MethodDescriptor, cdc codec.B
// 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)
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)
return nil
}, nil)
if err != nil {
return err
}
// merge on the resp, ref: https://github.com/cosmos/cosmos-sdk/issues/18003
gogoproto.Merge(outResp.(gogoproto.Message), resp.(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(outResp.(gogoproto.Message), resp.(gogoproto.Message))
return nil
default:
panic("unreachable")
Expand Down

0 comments on commit dabf72c

Please sign in to comment.