Ibc-go v1.0.0 issue

Summary of Bug

When receiving a packet with a packet data byte of an incorrect struct, the chain cannot unmarshal json from this line: https://github.com/cosmos/ibc-go/blob/v1.0.0/modules/apps/transfer/module.go#L331. Then the error acknowledgment will be created and saved to the chain state.

However, the behavior of types.ModuleCdc.UnmarshalJSON is nondeterministic making the acknowledgment containing different error messages. This will result in a consensus break. Below are the error acknowledgments from our two test nodes.

  • {"error":"cannot unmarshal ICS-20 transfer packet data: unknown field \"timeout_timestamp\" in types.FungibleTokenPacketData"}
  • {"error":"cannot unmarshal ICS-20 transfer packet data: unknown field \"source_channel\" in types.FungibleTokenPacketData"}

For the error above, we added a function called GetBytes() to MsgTransfer type. Then we also passed the msg to SendTransfer() function and called msg.GetBytes() instead of using packetdata.GetBytes() as usual (https://github.com/cosmos/ibc-go/blob/v1.0.0/modules/apps/transfer/keeper/relay.go#L151).

The nondeterminis error is from protobuf package. On this line https://github.com/gogo/protobuf/blob/master/jsonpb/jsonpb.go#L1116, if there are more than one unknown fields, looping through a map can return different orders of elements.

Version

ibc-go v1.0.0

Steps to Reproduce

  • start one chain with multiple validators
  • start another chain with a custom ibc-transfer module that sends incorrect packet data
  • Make sure relayer does not have CalculateGas (I modified go-relayer to skip CalculateGas step) so it will not fail in relayer
  • send a transaction from the second chain