fix: move IBC packet data interpretation to the application module

This also adds a channelexported.NewOpaquePacketData(rawBytes)
implementation to assist apps in being able to manipulate the
raw packet data using the codec layer.

docs/architecture/adr-015-ibc-packet-receiver.md

@@ -192,7 +192,9 @@ func NewAnteHandler(
 }
 ```
 
-The implementation of this ADR will also change the `Data` field of the `Packet` type from `[]byte` (i.e. arbitrary data) to `PacketDataI`. We want to make application modules be able to register custom packet data type which is automatically unmarshaled at `TxDecoder` time and can be simply type switched inside the application handler. Also, by having `GetCommitment()` method instead of manually generate the commitment inside the IBC keeper, the applications can define their own commitment method, including bare bytes, hashing, etc.
+The implementation of this ADR will also create a `RawData` field of the `Packet` of type `[]byte`, which can be deserialised by the receiving module into the `Data` field of type `PacketDataI`. It is up to the application modules to do this according to their own interpretation, not by the IBC keeper.  This is crucial for dynamic IBC.
+
+By having the `GetCommitment()` method, the applications can define their own commitment method, including bare bytes, hashing, etc.
 
 This also removes the `Timeout` field from the `Packet` struct. This is because the `PacketDataI` interface now contains this information. You can see details about this in [ICS04](https://github.com/cosmos/ics/tree/master/spec/ics-004-channel-and-packet-semantics#definitions). 
 
@@ -234,9 +236,12 @@ func NewHandler(k Keeper) Handler {
     case MsgTransfer:
       return handleMsgTransfer(ctx, k, msg)
     case ibc.MsgPacket:
-      switch data := msg.Packet.Data.(type) {
+      msg.Data = k.MustUnmarshalPacketData(ctx, msg.RawData)
+      switch data := msg.Data.(type) {
       case PacketDataTransfer: // i.e fulfills the PacketDataI interface
-        return handlePacketDataTransfer(ctx, k, msg.Packet, data)
+        return handlePacketDataTransfer(ctx, k, msg, data)
+      default:
+        return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized ICS-20 transfer packet data type: %T", data)
       }
     case ibc.MsgTimeoutPacket: 
       switch packet := msg.Packet.Data.(type) {

x/ibc/04-channel/client/utils/utils.go

@@ -4,7 +4,6 @@ import (
 	abci "github.com/tendermint/tendermint/abci/types"
 
 	"github.com/cosmos/cosmos-sdk/client/context"
-	"github.com/cosmos/cosmos-sdk/x/ibc/04-channel/exported"
 	"github.com/cosmos/cosmos-sdk/x/ibc/04-channel/types"
 	ibctypes "github.com/cosmos/cosmos-sdk/x/ibc/types"
 )
@@ -33,23 +32,14 @@ func QueryPacket(
 	destPortID := channelRes.Channel.Counterparty.PortID
 	destChannelID := channelRes.Channel.Counterparty.ChannelID
 
-	var data exported.PacketDataI
-	// TODO: commitment data is stored, not the data
-	// but we are unmarshalling the commitment in a json format
-	// because the current ICS 20 implementation uses json commitment form
-	// need to be changed to use external source of packet(e.g. event logs)
-	err = ctx.Codec.UnmarshalJSON(res.Value, &data)
-	if err != nil {
-		return types.PacketResponse{}, err
-	}
-
 	packet := types.NewPacket(
-		data,
+		res.Value,
 		sequence,
 		portID,
 		channelID,
 		destPortID,
 		destChannelID,
+		timeout,
 	)
 
 	// FIXME: res.Height+1 is hack, fix later

x/ibc/04-channel/exported/exported.go

@@ -31,26 +31,10 @@ type PacketI interface {
 	GetSourceChannel() string
 	GetDestPort() string
 	GetDestChannel() string
-	GetData() PacketDataI
+	GetData() []byte
 	ValidateBasic() error
 }
 
-// PacketDataI defines the packet data interface for IBC packets
-// IBC application modules should define which data they want to
-// send and receive over IBC channels.
-type PacketDataI interface {
-	GetBytes() []byte         // GetBytes returns the serialised packet data (without timeout)
-	GetTimeoutHeight() uint64 // GetTimeoutHeight returns the timeout height defined specifically for each packet data type instance
-
-	ValidateBasic() error // ValidateBasic validates basic properties of the packet data, implements sdk.Msg
-	Type() string         // Type returns human readable identifier, implements sdk.Msg
-}
-
-// PacketAcknowledgementI defines the interface for IBC packet acknowledgements.
-type PacketAcknowledgementI interface {
-	GetBytes() []byte
-}
-
 // Order defines if a channel is ORDERED or UNORDERED
 type Order byte
 

x/ibc/04-channel/exported/opaque.go

@@ -0,0 +1,19 @@
+package exported
+
+// OpaquePacketData is useful as a struct for when a Keeper cannot
+// unmarshal an IBC packet.
+type OpaquePacketData struct {
+	data []byte
+}
+
+// NewOpaquePacketData creates a wrapper for rawData.
+func NewOpaquePacketData(data []byte) OpaquePacketData {
+	return OpaquePacketData{
+		data: data,
+	}
+}
+
+// GetData is a helper for serialising
+func (opd OpaquePacketData) GetData() []byte {
+	return opd.data
+}

x/ibc/04-channel/keeper/packet.go

@@ -107,8 +107,8 @@ func (k Keeper) SendPacket(
 	ctx.EventManager().EmitEvents(sdk.Events{
 		sdk.NewEvent(
 			types.EventTypeSendPacket,
-			sdk.NewAttribute(types.AttributeKeyData, string(packet.GetData().GetBytes())),
-			sdk.NewAttribute(types.AttributeKeyTimeout, fmt.Sprintf("%d", packet.GetData().GetTimeoutHeight())),
+			sdk.NewAttribute(types.AttributeKeyData, string(packet.GetData())),
+			sdk.NewAttribute(types.AttributeKeyTimeout, fmt.Sprintf("%d", packet.GetTimeoutHeight())),
 			sdk.NewAttribute(types.AttributeKeySequence, fmt.Sprintf("%d", packet.GetSequence())),
 			sdk.NewAttribute(types.AttributeKeySrcPort, packet.GetSourcePort()),
 			sdk.NewAttribute(types.AttributeKeySrcChannel, packet.GetSourceChannel()),
@@ -193,7 +193,7 @@ func (k Keeper) RecvPacket(
 func (k Keeper) PacketExecuted(
 	ctx sdk.Context,
 	packet exported.PacketI,
-	acknowledgement exported.PacketAcknowledgementI,
+	acknowledgement []byte,
 ) error {
 	channel, found := k.GetChannel(ctx, packet.GetDestPort(), packet.GetDestChannel())
 	if !found {
@@ -246,7 +246,7 @@ func (k Keeper) PacketExecuted(
 func (k Keeper) AcknowledgePacket(
 	ctx sdk.Context,
 	packet exported.PacketI,
-	acknowledgement exported.PacketAcknowledgementI,
+	acknowledgement []byte,
 	proof commitmentexported.Proof,
 	proofHeight uint64,
 ) (exported.PacketI, error) {
@@ -301,7 +301,7 @@ func (k Keeper) AcknowledgePacket(
 
 	if err := k.connectionKeeper.VerifyPacketAcknowledgement(
 		ctx, connectionEnd, proofHeight, proof, packet.GetDestPort(), packet.GetDestChannel(),
-		packet.GetSequence(), acknowledgement.GetBytes(),
+		packet.GetSequence(), acknowledgement,
 	); err != nil {
 		return nil, sdkerrors.Wrap(err, "invalid acknowledgement on counterparty chain")
 	}