Skip to content

Commit

Permalink
imp: Use json for marshalling/unmarshalling transfer packet data. (co…
Browse files Browse the repository at this point in the history
  • Loading branch information
DimitrisJim committed Feb 1, 2024
1 parent 116029f commit dbc11b5
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 43 deletions.
3 changes: 2 additions & 1 deletion modules/apps/callbacks/ibc_middleware_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ibccallbacks_test

import (
"encoding/json"
"fmt"

errorsmod "cosmossdk.io/errors"
Expand Down Expand Up @@ -477,7 +478,7 @@ func (s *CallbacksTestSuite) TestOnTimeoutPacket() {
s.Require().NoError(err)
s.Require().NotNil(packet)

err = transfertypes.ModuleCdc.UnmarshalJSON(packet.Data, &packetData)
err = json.Unmarshal(packet.Data, &packetData)
s.Require().NoError(err)

ctx = s.chainA.GetContext()
Expand Down
9 changes: 5 additions & 4 deletions modules/apps/transfer/ibc_module.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package transfer

import (
"encoding/json"
"fmt"
"math"
"strings"
Expand Down Expand Up @@ -182,7 +183,7 @@ func (im IBCModule) OnRecvPacket(

var data types.FungibleTokenPacketData
var ackErr error
if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
if err := json.Unmarshal(packet.GetData(), &data); err != nil {
ackErr = errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS-20 transfer packet data")
logger.Error(fmt.Sprintf("%s sequence %d", ackErr.Error(), packet.Sequence))
ack = channeltypes.NewErrorAcknowledgement(ackErr)
Expand Down Expand Up @@ -238,7 +239,7 @@ func (im IBCModule) OnAcknowledgementPacket(
return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err)
}
var data types.FungibleTokenPacketData
if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
if err := json.Unmarshal(packet.GetData(), &data); err != nil {
return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error())
}

Expand Down Expand Up @@ -286,7 +287,7 @@ func (im IBCModule) OnTimeoutPacket(
relayer sdk.AccAddress,
) error {
var data types.FungibleTokenPacketData
if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
if err := json.Unmarshal(packet.GetData(), &data); err != nil {
return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error())
}
// refund tokens
Expand Down Expand Up @@ -352,7 +353,7 @@ func (IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, pr
// PacketDataUnmarshaler interface required for ADR 008 support.
func (IBCModule) UnmarshalPacketData(bz []byte) (interface{}, error) {
var packetData types.FungibleTokenPacketData
if err := types.ModuleCdc.UnmarshalJSON(bz, &packetData); err != nil {
if err := json.Unmarshal(bz, &packetData); err != nil {
return nil, err
}

Expand Down
34 changes: 0 additions & 34 deletions modules/apps/transfer/types/codec.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
package types

import (
"bytes"

"github.com/cosmos/gogoproto/jsonpb"
"github.com/cosmos/gogoproto/proto"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/legacy"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
Expand Down Expand Up @@ -39,32 +34,3 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
// The actual codec used for serialization should be provided to x/ibc transfer and
// defined at the application level.
var ModuleCdc = codec.NewProtoCodec(codectypes.NewInterfaceRegistry())

// 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()
}
14 changes: 10 additions & 4 deletions modules/apps/transfer/types/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@ package types

import (
"encoding/json"
"errors"
"strings"
"time"

errorsmod "cosmossdk.io/errors"
sdkmath "cosmossdk.io/math"

sdk "github.com/cosmos/cosmos-sdk/types"

ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported"
)
Expand Down Expand Up @@ -67,9 +66,16 @@ func (ftpd FungibleTokenPacketData) ValidateBasic() error {
return ValidatePrefixedDenom(ftpd.Denom)
}

// GetBytes is a helper for serialising
// GetBytes is a helper for serialising the packet to bytes.
// The memo field of FungibleTokenPacketData is marked with the JSON omitempty tag
// ensuring that the memo field is not included in the marshalled bytes if one is not specified.
func (ftpd FungibleTokenPacketData) GetBytes() []byte {
return sdk.MustSortJSON(mustProtoMarshalJSON(&ftpd))
bz, err := json.Marshal(ftpd)
if err != nil {
panic(errors.New("cannot marshal FungibleTokenPacketData into bytes"))
}

return bz
}

// GetPacketSender returns the sender address embedded in the packet data.
Expand Down
25 changes: 25 additions & 0 deletions modules/apps/transfer/types/packet_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package types_test

import (
"encoding/json"
"fmt"
"testing"

Expand Down Expand Up @@ -134,3 +135,27 @@ func (suite *TypesTestSuite) TestPacketDataProvider() {
suite.Require().Equal(tc.expCustomData, customData)
}
}

func (suite *TypesTestSuite) TestFungibleTokenPacketDataOmitEmpty() {
// check that omitempty is present for the memo field
packetData := types.FungibleTokenPacketData{
Denom: denom,
Amount: amount,
Sender: sender,
Receiver: receiver,
// Default value for non-specified memo field is empty string
}

bz, err := json.Marshal(packetData)
suite.Require().NoError(err)

// check that the memo field is not present in the marshalled bytes
suite.Require().NotContains(string(bz), "memo")

packetData.Memo = "abc"
bz, err = json.Marshal(packetData)
suite.Require().NoError(err)

// check that the memo field is present in the marshalled bytes
suite.Require().Contains(string(bz), "memo")
}

0 comments on commit dbc11b5

Please sign in to comment.