From c04345d69c1056d3cc3d2c303994b92d9169d198 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 3 Nov 2022 21:06:36 +0100 Subject: [PATCH] fix: skip emission of unpopulated memo field in ics20 (backport #2651) (#2654) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: skip emission of unpopulated memo field in ics20 (#2651) By setting `EmitDefaults` to false, we will not include an empty memo field in the marshaled json bytes closes: #2645 --- Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why. - [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules/10-structure.md). - [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing) - [ ] Updated relevant documentation (`docs/`) or specification (`x//spec/`) - [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [ ] Re-reviewed `Files changed` in the Github PR explorer - [ ] Review `Codecov Report` in the comment section below once CI passes (cherry picked from commit 393247bff61c225a29fab7aa002877e6653ab415) * Update modules/apps/transfer/types/codec_test.go * fix: merge conflict build Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> Co-authored-by: Damian Nolan --- CHANGELOG.md | 1 + modules/apps/transfer/types/codec.go | 33 +++++++++++++++++++++++ modules/apps/transfer/types/codec_test.go | 26 ++++++++++++++++++ modules/apps/transfer/types/packet.go | 2 +- 4 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 modules/apps/transfer/types/codec_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f0b7ddc1e8f..f7885b74f73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### State Machine Breaking +* (apps/transfer) [\#2651](https://github.com/cosmos/ibc-go/pull/2651) Introduce `mustProtoMarshalJSON` for ics20 packet data marshalling which will skip emission (marshalling) of the memo field if unpopulated (empty). * (27-interchain-accounts) [\#2580](https://github.com/cosmos/ibc-go/issues/2580) Removing port prefix requirement from the ICA host channel handshake * (transfer) [\#2377](https://github.com/cosmos/ibc-go/pull/2377) Adding `sequence` to `MsgTransferResponse`. diff --git a/modules/apps/transfer/types/codec.go b/modules/apps/transfer/types/codec.go index e9e5becc5b2..b7b866c9a10 100644 --- a/modules/apps/transfer/types/codec.go +++ b/modules/apps/transfer/types/codec.go @@ -1,10 +1,14 @@ package types import ( + "bytes" + "github.com/Finschia/finschia-sdk/codec" codectypes "github.com/Finschia/finschia-sdk/codec/types" sdk "github.com/Finschia/finschia-sdk/types" "github.com/Finschia/finschia-sdk/types/msgservice" + "github.com/gogo/protobuf/jsonpb" + "github.com/gogo/protobuf/proto" ) // RegisterLegacyAminoCodec registers the necessary x/ibc transfer interfaces and concrete types @@ -39,3 +43,32 @@ func init() { RegisterLegacyAminoCodec(amino) amino.Seal() } + +// mustProtoMarshalJSON provides an auxiliary function to return Proto3 JSON encoded +// bytes of a message. +// NOTE: Copied from https://github.com/cosmos/cosmos-sdk/blob/971c542453e0972ef1dfc5a80159ad5049c7211c/codec/json.go +// and modified in order to allow `EmitDefaults` to be set to false for ics20 packet marshalling. +// This allows for the introduction of the memo field to be backwards compatible. +func mustProtoMarshalJSON(msg proto.Message) []byte { + anyResolver := codectypes.NewInterfaceRegistry() + + // EmitDefaults is set to false to prevent marshalling of unpopulated fields (memo) + // OrigName and the anyResovler match the fields the original SDK function would expect + // in order to minimize changes. + + // OrigName is true since there is no particular reason to use camel case + // The any resolver is empty, but provided anyways. + jm := &jsonpb.Marshaler{OrigName: true, EmitDefaults: false, AnyResolver: anyResolver} + + err := codectypes.UnpackInterfaces(msg, codectypes.ProtoJSONPacker{JSONPBMarshaler: jm}) + if err != nil { + panic(err) + } + + buf := new(bytes.Buffer) + if err := jm.Marshal(buf, msg); err != nil { + panic(err) + } + + return buf.Bytes() +} diff --git a/modules/apps/transfer/types/codec_test.go b/modules/apps/transfer/types/codec_test.go new file mode 100644 index 00000000000..98d26a71255 --- /dev/null +++ b/modules/apps/transfer/types/codec_test.go @@ -0,0 +1,26 @@ +package types_test + +import ( + "strings" + + sdk "github.com/Finschia/finschia-sdk/types" + + "github.com/cosmos/ibc-go/v4/modules/apps/transfer/types" +) + +// TestMustMarshalProtoJSON tests that the memo field is only emitted (marshalled) if it is populated +func (suite *TypesTestSuite) TestMustMarshalProtoJSON() { + memo := "memo" + packetData := types.NewFungibleTokenPacketData(sdk.DefaultBondDenom, "1", suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String()) + packetData.Memo = memo + + bz := packetData.GetBytes() + exists := strings.Contains(string(bz), memo) + suite.Require().True(exists) + + packetData.Memo = "" + + bz = packetData.GetBytes() + exists = strings.Contains(string(bz), memo) + suite.Require().False(exists) +} diff --git a/modules/apps/transfer/types/packet.go b/modules/apps/transfer/types/packet.go index d5e45e6f210..f9883f05ecd 100644 --- a/modules/apps/transfer/types/packet.go +++ b/modules/apps/transfer/types/packet.go @@ -56,5 +56,5 @@ func (ftpd FungibleTokenPacketData) ValidateBasic() error { // GetBytes is a helper for serialising func (ftpd FungibleTokenPacketData) GetBytes() []byte { - return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&ftpd)) + return sdk.MustSortJSON(mustProtoMarshalJSON(&ftpd)) }