Move IBC packet data interpretation to the application module

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

@@ -192,20 +192,22 @@ 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 `Data` field of the `Packet` of type `[]byte`, which can be deserialised by the receiving module into its own private type. 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.
 
-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). 
-
-The `PacketDataI` is the application specific interface that provides information for the execution of the application packet. In the case of ICS20 this would be `denom`, `amount` and `address`
+By convention, the application's `Keeper` can deserialise the `Data` with a `UnmarshalPacketData` method.
 
 ```go
-// PacketDataI defines the standard interface for IBC packet data
-type PacketDataI interface {
-	GetCommitment() []byte // Commitment form that will be stored in the state.
-	GetTimeoutHeight() uint64
+// UnmarshalPacketData tries to extract an application type from raw channel packet data.
+func (k Keeper) UnmarshalPacketData(ctx sdk.Context, rawData []byte) (types.PacketDataI, error) {
+  var pdt types.PacketDataTransfer
+
+  err := k.cdc.UnmarshalBinaryBare(rawData, &pdt)
+  if err == nil {
+    return pdt, nil
+  }
 
-	ValidateBasic() error
-	Type() string
+  // Cannot unmarshal, so return the error.
+  return nil, err
 }
 ```
 
@@ -234,14 +236,24 @@ func NewHandler(k Keeper) Handler {
     case MsgTransfer:
       return handleMsgTransfer(ctx, k, msg)
     case ibc.MsgPacket:
-      switch data := msg.Packet.Data.(type) {
-      case PacketDataTransfer: // i.e fulfills the PacketDataI interface
-        return handlePacketDataTransfer(ctx, k, msg.Packet, data)
+      pd, err = k.UnmarshalPacketData(ctx, msg.GetData())
+      if err {
+        return nil, err
+      }
+      switch data := pd.(type) {
+      case PacketDataTransfer: // any packet type known by the keeper
+        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:
+      pd, err = k.UnmarshalPacketData(ctx, msg.GetData())
+      if err {
+        return nil, err
       }
-    case ibc.MsgTimeoutPacket: 
-      switch packet := msg.Packet.Data.(type) {
-      case PacketDataTransfer: // i.e fulfills the PacketDataI interface
-        return handleTimeoutPacketDataTransfer(ctx, k, msg.Packet)
+      switch packet := pd.(type) {
+      case PacketDataTransfer: // any packet type known by the keeper
+        return handleTimeoutPacketDataTransfer(ctx, k, packet)
       }
     // interface { PortID() string; ChannelID() string; Channel() ibc.Channel }
     // MsgChanInit, MsgChanTry implements ibc.MsgChannelOpen

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/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")
 	}