Merge PR #5603: Remove acknowledgement interface in favour of []byte

x/ibc/04-channel/alias.go

@@ -37,6 +37,7 @@ var (
 	ErrPacketTimeout             = types.ErrPacketTimeout
 	ErrInvalidChannel            = types.ErrInvalidChannel
 	ErrInvalidChannelState       = types.ErrInvalidChannelState
+	ErrAcknowledgementTooLong    = types.ErrAcknowledgementTooLong
 	NewMsgChannelOpenInit        = types.NewMsgChannelOpenInit
 	NewMsgChannelOpenTry         = types.NewMsgChannelOpenTry
 	NewMsgChannelOpenAck         = types.NewMsgChannelOpenAck

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

@@ -46,6 +46,11 @@ type PacketDataI interface {
 	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

@@ -178,7 +178,7 @@ func (k Keeper) RecvPacket(
 func (k Keeper) PacketExecuted(
 	ctx sdk.Context,
 	packet exported.PacketI,
-	acknowledgement exported.PacketDataI,
+	acknowledgement exported.PacketAcknowledgementI,
 ) error {
 	channel, found := k.GetChannel(ctx, packet.GetDestPort(), packet.GetDestChannel())
 	if !found {
@@ -231,7 +231,7 @@ func (k Keeper) PacketExecuted(
 func (k Keeper) AcknowledgePacket(
 	ctx sdk.Context,
 	packet exported.PacketI,
-	acknowledgement []byte,
+	acknowledgement exported.PacketAcknowledgementI,
 	proof commitment.ProofI,
 	proofHeight uint64,
 ) (exported.PacketI, error) {

x/ibc/04-channel/types/codec.go

@@ -22,6 +22,7 @@ func init() {
 func RegisterCodec(cdc *codec.Codec) {
 	cdc.RegisterInterface((*exported.PacketI)(nil), nil)
 	cdc.RegisterInterface((*exported.PacketDataI)(nil), nil)
+	cdc.RegisterInterface((*exported.PacketAcknowledgementI)(nil), nil)
 	cdc.RegisterConcrete(Channel{}, "ibc/channel/Channel", nil)
 	cdc.RegisterConcrete(Packet{}, "ibc/channel/Packet", nil)
 

x/ibc/04-channel/types/errors.go

@@ -18,4 +18,5 @@ var (
 	ErrInvalidPacket             = sdkerrors.Register(SubModuleName, 10, "invalid packet")
 	ErrPacketTimeout             = sdkerrors.Register(SubModuleName, 11, "packet timeout")
 	ErrTooManyConnectionHops     = sdkerrors.Register(SubModuleName, 12, "too many connection hops")
+	ErrAcknowledgementTooLong    = sdkerrors.Register(SubModuleName, 13, "acknowledgement too long")
 )

x/ibc/04-channel/types/msgs.go

@@ -504,14 +504,14 @@ var _ sdk.Msg = MsgAcknowledgement{}
 // MsgAcknowledgement receives incoming IBC acknowledgement
 type MsgAcknowledgement struct {
 	Packet          `json:"packet" yaml:"packet"`
-	Acknowledgement exported.PacketDataI `json:"acknowledgement" yaml:"acknowledgement"`
-	Proof           commitment.ProofI    `json:"proof" yaml:"proof"`
-	ProofHeight     uint64               `json:"proof_height" yaml:"proof_height"`
-	Signer          sdk.AccAddress       `json:"signer" yaml:"signer"`
+	Acknowledgement exported.PacketAcknowledgementI `json:"acknowledgement" yaml:"acknowledgement"`
+	Proof           commitment.ProofI               `json:"proof" yaml:"proof"`
+	ProofHeight     uint64                          `json:"proof_height" yaml:"proof_height"`
+	Signer          sdk.AccAddress                  `json:"signer" yaml:"signer"`
 }
 
 // NewMsgAcknowledgement constructs a new MsgAcknowledgement
-func NewMsgAcknowledgement(packet Packet, ack exported.PacketDataI, proof commitment.ProofI, proofHeight uint64, signer sdk.AccAddress) MsgAcknowledgement {
+func NewMsgAcknowledgement(packet Packet, ack exported.PacketAcknowledgementI, proof commitment.ProofI, proofHeight uint64, signer sdk.AccAddress) MsgAcknowledgement {
 	return MsgAcknowledgement{
 		Packet:          packet,
 		Acknowledgement: ack,
@@ -534,12 +534,12 @@ func (msg MsgAcknowledgement) ValidateBasic() error {
 	if err := msg.Proof.ValidateBasic(); err != nil {
 		return sdkerrors.Wrap(err, "proof ack cannot be nil")
 	}
+	if len(msg.Acknowledgement.GetBytes()) > 100 {
+		return sdkerrors.Wrap(ErrAcknowledgementTooLong, "acknowledgement cannot exceed 100 bytes")
+	}
 	if msg.ProofHeight == 0 {
 		return sdkerrors.Wrap(ibctypes.ErrInvalidHeight, "proof height must be > 0")
 	}
-	if err := msg.Acknowledgement.ValidateBasic(); err != nil {
-		return err
-	}
 	if msg.Signer.Empty() {
 		return sdkerrors.ErrInvalidAddress
 	}

x/ibc/04-channel/types/msgs_test.go

@@ -392,10 +392,19 @@ func (invalidPacketT) Type() string {
 	return "invalid"
 }
 
+var _ exported.PacketAcknowledgementI = invalidAckT{}
+
+type invalidAckT struct{}
+
+func (invalidAckT) GetBytes() []byte {
+	return []byte("123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890")
+}
+
 // define variables used for testing
 var (
 	packet        = NewPacket(validPacketT{}, 1, portid, chanid, cpportid, cpchanid)
 	invalidPacket = NewPacket(invalidPacketT{}, 0, portid, chanid, cpportid, cpchanid)
+	invalidAck    = invalidAckT{}
 
 	emptyProof     = commitment.Proof{Proof: nil}
 	invalidProofs1 = commitment.ProofI(nil)
@@ -521,7 +530,7 @@ func (suite *MsgTestSuite) TestMsgAcknowledgement() {
 		NewMsgAcknowledgement(packet, packet.GetData(), proof, 1, emptyAddr),
 		NewMsgAcknowledgement(packet, packet.GetData(), emptyProof, 1, addr),
 		NewMsgAcknowledgement(invalidPacket, packet.GetData(), proof, 1, addr),
-		NewMsgAcknowledgement(packet, invalidPacket.GetData(), proof, 1, addr),
+		NewMsgAcknowledgement(packet, invalidAck, proof, 1, addr),
 		NewMsgAcknowledgement(packet, packet.GetData(), invalidProofs1, 1, addr),
 	}
 

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

@@ -22,7 +22,7 @@ func CommitPacket(data exported.PacketDataI) []byte {
 }
 
 // CommitAcknowledgement returns the hash of commitment bytes
-func CommitAcknowledgement(data exported.PacketDataI) []byte {
+func CommitAcknowledgement(data exported.PacketAcknowledgementI) []byte {
 	return tmhash.Sum(data.GetBytes())
 }
 

x/ibc/20-transfer/alias.go

@@ -43,5 +43,5 @@ type (
 	SupplyKeeper            = types.SupplyKeeper
 	FungibleTokenPacketData = types.FungibleTokenPacketData
 	MsgTransfer             = types.MsgTransfer
-	PacketDataTransfer      = types.PacketDataTransfer
+	AckDataTransfer         = types.AckDataTransfer
 )

x/ibc/20-transfer/handler.go

@@ -64,6 +64,8 @@ func handlePacketDataTransfer(
 	if err := k.ReceiveTransfer(
 		ctx, packet.SourcePort, packet.SourceChannel, packet.DestinationPort, packet.DestinationChannel, data,
 	); err != nil {
+		// How do we want to handle this case? Maybe we should be more lenient, it's safe to leave the channel open I think.
+
 		// TODO: handle packet receipt that due to an error (specify)
 		// the receiving chain couldn't process the transfer
 
@@ -97,13 +99,14 @@ func handleTimeoutDataTransfer(ctx sdk.Context, k Keeper, msg channeltypes.MsgTi
 	packet := msg.Packet
 	err := k.TimeoutTransfer(ctx, packet.SourcePort, packet.SourceChannel, packet.DestinationPort, packet.DestinationChannel, data)
 	if err != nil {
-		// TODO: This chain sent invalid packet
+		// This shouldn't happen, since we've already validated that we've sent the packet.
 		panic(err)
 	}
 
 	err = k.TimeoutExecuted(ctx, packet)
 	if err != nil {
-		// TODO: This should not happen
+		// This shouldn't happen, since we've already validated that we've sent the packet.
+		// TODO: Figure out what happens if the capability authorisation changes.
 		panic(err)
 	}
 

x/ibc/20-transfer/keeper/keeper.go

@@ -65,7 +65,7 @@ func (k Keeper) GetTransferAccount(ctx sdk.Context) supplyexported.ModuleAccount
 
 // PacketExecuted defines a wrapper function for the channel Keeper's function
 // in order to expose it to the ICS20 transfer handler.
-func (k Keeper) PacketExecuted(ctx sdk.Context, packet channelexported.PacketI, acknowledgement channelexported.PacketDataI) error {
+func (k Keeper) PacketExecuted(ctx sdk.Context, packet channelexported.PacketI, acknowledgement channelexported.PacketAcknowledgementI) error {
 	return k.channelKeeper.PacketExecuted(ctx, packet, acknowledgement)
 }
 

x/ibc/20-transfer/keeper/relay.go

@@ -81,8 +81,6 @@ func (k Keeper) ReceiveTransfer(
 		return k.supplyKeeper.SendCoinsFromModuleToAccount(ctx, types.GetModuleAccountName(), data.Receiver, data.Amount)
 	}
 
-	// unescrow tokens
-
 	// check the denom prefix
 	prefix := types.GetDenomPrefix(sourcePort, sourceChannel)
 	coins := make(sdk.Coins, len(data.Amount))
@@ -96,6 +94,7 @@ func (k Keeper) ReceiveTransfer(
 		coins[i] = sdk.NewCoin(coin.Denom[len(prefix):], coin.Amount)
 	}
 
+	// unescrow tokens
 	escrowAddress := types.GetEscrowAddress(destinationPort, destinationChannel)
 	return k.bankKeeper.SendCoins(ctx, escrowAddress, data.Receiver, coins)
 

x/ibc/20-transfer/types/expected_keepers.go

@@ -19,7 +19,7 @@ type ChannelKeeper interface {
 	GetChannel(ctx sdk.Context, srcPort, srcChan string) (channel channel.Channel, found bool)
 	GetNextSequenceSend(ctx sdk.Context, portID, channelID string) (uint64, bool)
 	SendPacket(ctx sdk.Context, packet channelexported.PacketI) error
-	PacketExecuted(ctx sdk.Context, packet channelexported.PacketI, acknowledgement channelexported.PacketDataI) error
+	PacketExecuted(ctx sdk.Context, packet channelexported.PacketI, acknowledgement channelexported.PacketAcknowledgementI) error
 	ChanCloseInit(ctx sdk.Context, portID, channelID string) error
 	TimeoutExecuted(ctx sdk.Context, packet channelexported.PacketI) error
 }

x/ibc/20-transfer/types/packet.go

@@ -83,28 +83,13 @@ func (ftpd FungibleTokenPacketData) Type() string {
 	return "ics20/transfer"
 }
 
-var _ channelexported.PacketDataI = AckDataTransfer{}
+var _ channelexported.PacketAcknowledgementI = AckDataTransfer{}
 
 // AckDataTransfer is a no-op packet
 // See spec for onAcknowledgePacket: https://github.com/cosmos/ics/tree/master/spec/ics-020-fungible-token-transfer#packet-relay
 type AckDataTransfer struct{}
 
-// ValidateBasic implements channelexported.PacketDataI
-func (ack AckDataTransfer) ValidateBasic() error {
-	return nil
-}
-
-// GetBytes implements channelexported.PacketDataI
+// GetBytes implements channelexported.PacketAcknowledgementI
 func (ack AckDataTransfer) GetBytes() []byte {
-	return []byte("ok")
-}
-
-// GetTimeoutHeight implements channelexported.PacketDataI
-func (ack AckDataTransfer) GetTimeoutHeight() uint64 {
-	return 0
-}
-
-// Type implements channelexported.PacketDataI
-func (ack AckDataTransfer) Type() string {
-	return "ics20/transfer/ack"
+	return []byte("fungible token transfer ack")
 }