Move IBC packet data interpretation to the application module

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

@@ -192,22 +192,7 @@ 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.
-
-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`
-
-```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
-
-	ValidateBasic() error
-	Type() string
-}
-```
+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.
 
 Example application-side usage:
 
@@ -234,15 +219,17 @@ 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)
+      var data PacketDataTransfer
+      if err := types.ModuleCodec.UnmarshalBinaryBare(msg.GetData(), &data); err != nil {
+        return err
       }
-    case ibc.MsgTimeoutPacket: 
-      switch packet := msg.Packet.Data.(type) {
-      case PacketDataTransfer: // i.e fulfills the PacketDataI interface
-        return handleTimeoutPacketDataTransfer(ctx, k, msg.Packet)
+      return handlePacketDataTransfer(ctx, k, msg, data)
+    case ibc.MsgTimeoutPacket:
+      var data PacketDataTransfer
+      if err := types.ModuleCodec.UnmarshalBinaryBare(msg.GetData(), &data); err != nil {
+        return err
       }
+      return handleTimeoutPacketDataTransfer(ctx, k, packet)
     // interface { PortID() string; ChannelID() string; Channel() ibc.Channel }
     // MsgChanInit, MsgChanTry implements ibc.MsgChannelOpen
     case 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

@@ -100,15 +100,15 @@ func (k Keeper) SendPacket(
 
 	nextSequenceSend++
 	k.SetNextSequenceSend(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), nextSequenceSend)
-	k.SetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence(), types.CommitPacket(packet.GetData()))
+	k.SetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence(), types.CommitPacket(packet))
 
 	// Emit Event with Packet data along with other packet information for relayer to pick up
 	// and relay to other chain
 	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()),
@@ -179,7 +179,7 @@ func (k Keeper) RecvPacket(
 	if err := k.connectionKeeper.VerifyPacketCommitment(
 		ctx, connectionEnd, proofHeight, proof,
 		packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence(),
-		types.CommitPacket(packet.GetData()),
+		types.CommitPacket(packet),
 	); err != nil {
 		return nil, sdkerrors.Wrap(err, "couldn't verify counterparty packet commitment")
 	}
@@ -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) {
@@ -295,13 +295,13 @@ func (k Keeper) AcknowledgePacket(
 	commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 
 	// verify we sent the packet and haven't cleared it out yet
-	if !bytes.Equal(commitment, types.CommitPacket(packet.GetData())) {
+	if !bytes.Equal(commitment, types.CommitPacket(packet)) {
 		return nil, sdkerrors.Wrap(types.ErrInvalidPacket, "packet hasn't been sent")
 	}
 
 	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")
 	}
@@ -377,7 +377,7 @@ func (k Keeper) CleanupPacket(
 	commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 
 	// verify we sent the packet and haven't cleared it out yet
-	if !bytes.Equal(commitment, types.CommitPacket(packet.GetData())) {
+	if !bytes.Equal(commitment, types.CommitPacket(packet)) {
 		return nil, sdkerrors.Wrap(types.ErrInvalidPacket, "packet hasn't been sent")
 	}