Simplify SendPacket API (backport #1703)

CHANGELOG.md

@@ -48,6 +48,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (transfer) [\#2034](https://github.com/cosmos/ibc-go/pull/2034) Transfer Keeper now expects a keeper which fulfills the expected `ScopedKeeper` interface for the capability keeper.
 * (05-port) [\#2025](https://github.com/cosmos/ibc-go/pull/2025) Port Keeper now expects a keeper which fulfills the expected `ScopedKeeper` interface for the capability keeper.
 * (04-channel) [\#2024](https://github.com/cosmos/ibc-go/pull/2024) Channel Keeper now expects a keeper which fulfills the expected `ScopedKeeper` interface for the capability keeper.
+* (core/04-channel)[\#1703](https://github.com/cosmos/ibc-go/pull/1703) Update `SendPacket` API to take in necessary arguments and construct rest of packet rather than taking in entire packet. The generated packet sequence is returned by the `SendPacket` function.
 * (modules/apps/27-interchain-accounts) [\#2433](https://github.com/cosmos/ibc-go/pull/2450) Renamed icatypes.PortPrefix to icatypes.ControllerPortPrefix & icatypes.PortID to icatypes.HostPortID
 
 ### State Machine Breaking

docs/ibc/apps.md

@@ -238,10 +238,21 @@ DecodePacketData(encoded []byte) (CustomPacketData) {
 Then a module must encode its packet data before sending it through IBC.
 
 ```go
+// retrieve the dynamic capability for this channel
+channelCap := scopedKeeper.GetCapability(ctx, channelCapName)
 // Sending custom application packet data
 data := EncodePacketData(customPacketData)
 packet.Data = data
-IBCChannelKeeper.SendPacket(ctx, packet)
+// Send packet to IBC, authenticating with channelCap
+sequence, err := IBCChannelKeeper.SendPacket(
+    ctx, 
+    channelCap, 
+    sourcePort, 
+    sourceChannel, 
+    timeoutHeight, 
+    timeoutTimestamp, 
+    data,
+)
 ```
 
 A module receiving a packet must decode the `PacketData` into a structure it expects so that it can
@@ -284,9 +295,16 @@ packet a module simply needs to call `SendPacket` on the `IBCChannelKeeper`.
 channelCap := scopedKeeper.GetCapability(ctx, channelCapName)
 // Sending custom application packet data
 data := EncodePacketData(customPacketData)
-packet.Data = data
 // Send packet to IBC, authenticating with channelCap
-IBCChannelKeeper.SendPacket(ctx, channelCap, packet)
+sequence, err := IBCChannelKeeper.SendPacket(
+    ctx, 
+    channelCap, 
+    sourcePort, 
+    sourceChannel, 
+    timeoutHeight, 
+    timeoutTimestamp, 
+    data,
+)
 ```
 
 ::: warning

docs/ibc/apps/ibcmodule.md

@@ -232,9 +232,16 @@ module must trigger execution on the port-bound module through the use of callba
 channelCap := scopedKeeper.GetCapability(ctx, channelCapName)
 // Sending custom application packet data
 data := EncodePacketData(customPacketData)
-packet.Data = data
 // Send packet to IBC, authenticating with channelCap
-IBCChannelKeeper.SendPacket(ctx, channelCap, packet)
+sequence, err := IBCChannelKeeper.SendPacket(
+    ctx, 
+    channelCap, 
+    sourcePort, 
+    sourceChannel, 
+    timeoutHeight, 
+    timeoutTimestamp, 
+    data,
+)
 ```
 
 ::: warning

docs/ibc/apps/packets_acks.md

@@ -43,10 +43,20 @@ DecodePacketData(encoded []byte) (CustomPacketData) {
 Then a module must encode its packet data before sending it through IBC.
 
 ```go
+// retrieve the dynamic capability for this channel
+channelCap := scopedKeeper.GetCapability(ctx, channelCapName)
 // Sending custom application packet data
 data := EncodePacketData(customPacketData)
-packet.Data = data
-IBCChannelKeeper.SendPacket(ctx, packet)
+// Send packet to IBC, authenticating with channelCap
+sequence, err := IBCChannelKeeper.SendPacket(
+    ctx, 
+    channelCap, 
+    sourcePort, 
+    sourceChannel, 
+    timeoutHeight, 
+    timeoutTimestamp, 
+    data,
+)
 ```
 
 A module receiving a packet must decode the `PacketData` into a structure it expects so that it can

docs/ibc/middleware/develop.md

@@ -47,9 +47,28 @@ type Middleware interface {
 // The base application will call `sendPacket` or `writeAcknowledgement` of the middleware directly above them
 // which will call the next middleware until it reaches the core IBC handler.
 type ICS4Wrapper interface {
-    SendPacket(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet exported.Packet) error
-    WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet exported.Packet, ack exported.Acknowledgement) error
-    GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool)
+    SendPacket(
+        ctx sdk.Context,
+        chanCap *capabilitytypes.Capability,
+        sourcePort string,
+        sourceChannel string,
+        timeoutHeight clienttypes.Height,
+        timeoutTimestamp uint64,
+        data []byte,
+    ) (sequence uint64, err error)
+
+    WriteAcknowledgement(
+        ctx sdk.Context,
+        chanCap *capabilitytypes.Capability,
+        packet exported.PacketI,
+        ack exported.Acknowledgement,
+    ) error
+
+    GetAppVersion(
+        ctx sdk.Context,
+        portID,
+        channelID string,
+    ) (string, bool)
 }
 ```
 
@@ -352,12 +371,24 @@ Middleware must also wrap ICS-4 so that any communication from the application t
 func SendPacket(
     ctx sdk.Context,
     chanCap *capabilitytypes.Capability,
-    appPacket exported.PacketI,
+    sourcePort string,
+    sourceChannel string,
+    timeoutHeight clienttypes.Height,
+    timeoutTimestamp uint64,
+    appData []byte,
 ) {
-    // middleware may modify packet
-    packet = doCustomLogic(appPacket)
-
-    return ics4Keeper.SendPacket(ctx, chanCap, packet)
+    // middleware may modify data
+    data = doCustomLogic(appData)
+
+    return ics4Keeper.SendPacket(
+        ctx, 
+        chanCap, 
+        sourcePort, 
+        sourceChannel, 
+        timeoutHeight, 
+        timeoutTimestamp, 
+        data,
+    )
 }
 ```
 

docs/migrations/v5-to-v6.md

@@ -0,0 +1,40 @@
+
+# Migrating from ibc-go v5 to v6
+
+This document is intended to highlight significant changes which may require more information than presented in the CHANGELOG.
+Any changes that must be done by a user of ibc-go should be documented here.
+
+There are four sections based on the four potential user groups of this document:
+- Chains
+- IBC Apps
+- Relayers
+- IBC Light Clients
+
+**Note:** ibc-go supports golang semantic versioning and therefore all imports must be updated to bump the version number on major releases.
+
+## Chains
+
+## IBC Apps
+
+### ICS04 - `SendPacket` API change
+
+The `SendPacket` API has been simplified:
+
+```diff
+// SendPacket is called by a module in order to send an IBC packet on a channel
+ func (k Keeper) SendPacket(
+        ctx sdk.Context,
+        channelCap *capabilitytypes.Capability,
+-       packet exported.PacketI,
+-) error {
++       sourcePort string,
++       sourceChannel string,
++       timeoutHeight clienttypes.Height,
++       timeoutTimestamp uint64,
++       data []byte,
++) (uint64, error) {
+```
+
+Callers no longer need to pass in a pre-constructed packet. 
+The destination port/channel identifiers and the packet sequence will be determined by core IBC.
+`SendPacket` will return the packet sequence.

modules/apps/27-interchain-accounts/controller/ibc_middleware.go

@@ -8,6 +8,7 @@ import (
 	"github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/controller/keeper"
 	"github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/controller/types"
 	icatypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/types"
+	clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
 	channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
 	porttypes "github.com/cosmos/ibc-go/v6/modules/core/05-port/types"
 	host "github.com/cosmos/ibc-go/v6/modules/core/24-host"
@@ -227,8 +228,12 @@ func (im IBCMiddleware) OnTimeoutPacket(
 func (im IBCMiddleware) SendPacket(
 	ctx sdk.Context,
 	chanCap *capabilitytypes.Capability,
-	packet ibcexported.PacketI,
-) error {
+	sourcePort string,
+	sourceChannel string,
+	timeoutHeight clienttypes.Height,
+	timeoutTimestamp uint64,
+	data []byte,
+) (uint64, error) {
 	panic("SendPacket not supported for ICA controller module. Please use SendTx")
 }
 

modules/apps/27-interchain-accounts/controller/keeper/keeper.go

@@ -16,6 +16,7 @@ import (
 	genesistypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/genesis/types"
 	icatypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/types"
 	channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
+	porttypes "github.com/cosmos/ibc-go/v6/modules/core/05-port/types"
 	host "github.com/cosmos/ibc-go/v6/modules/core/24-host"
 )
 
@@ -25,7 +26,7 @@ type Keeper struct {
 	cdc        codec.BinaryCodec
 	paramSpace paramtypes.Subspace
 
-	ics4Wrapper   icatypes.ICS4Wrapper
+	ics4Wrapper   porttypes.ICS4Wrapper
 	channelKeeper icatypes.ChannelKeeper
 	portKeeper    icatypes.PortKeeper
 
@@ -37,7 +38,7 @@ type Keeper struct {
 // NewKeeper creates a new interchain accounts controller Keeper instance
 func NewKeeper(
 	cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace paramtypes.Subspace,
-	ics4Wrapper icatypes.ICS4Wrapper, channelKeeper icatypes.ChannelKeeper, portKeeper icatypes.PortKeeper,
+	ics4Wrapper porttypes.ICS4Wrapper, channelKeeper icatypes.ChannelKeeper, portKeeper icatypes.PortKeeper,
 	scopedKeeper icatypes.ScopedKeeper, msgRouter icatypes.MessageRouter,
 ) Keeper {
 	// set KeyTable if it has not already been set

modules/apps/27-interchain-accounts/controller/keeper/relay.go

@@ -26,14 +26,6 @@ func (k Keeper) sendTx(ctx sdk.Context, connectionID, portID string, icaPacketDa
 		return 0, sdkerrors.Wrapf(icatypes.ErrActiveChannelNotFound, "failed to retrieve active channel on connection %s for port %s", connectionID, portID)
 	}
 
-	sourceChannelEnd, found := k.channelKeeper.GetChannel(ctx, portID, activeChannelID)
-	if !found {
-		return 0, sdkerrors.Wrap(channeltypes.ErrChannelNotFound, activeChannelID)
-	}
-
-	destinationPort := sourceChannelEnd.GetCounterparty().GetPortID()
-	destinationChannel := sourceChannelEnd.GetCounterparty().GetChannelID()
-
 	chanCap, found := k.scopedKeeper.GetCapability(ctx, host.ChannelCapabilityPath(portID, activeChannelID))
 	if !found {
 		return 0, sdkerrors.Wrapf(capabilitytypes.ErrCapabilityNotFound, "failed to find capability: %s", host.ChannelCapabilityPath(portID, activeChannelID))
@@ -43,45 +35,16 @@ func (k Keeper) sendTx(ctx sdk.Context, connectionID, portID string, icaPacketDa
 		return 0, icatypes.ErrInvalidTimeoutTimestamp
 	}
 
-	return k.createOutgoingPacket(ctx, portID, activeChannelID, destinationPort, destinationChannel, chanCap, icaPacketData, timeoutTimestamp)
-}
-
-func (k Keeper) createOutgoingPacket(
-	ctx sdk.Context,
-	sourcePort,
-	sourceChannel,
-	destinationPort,
-	destinationChannel string,
-	chanCap *capabilitytypes.Capability,
-	icaPacketData icatypes.InterchainAccountPacketData,
-	timeoutTimestamp uint64,
-) (uint64, error) {
 	if err := icaPacketData.ValidateBasic(); err != nil {
 		return 0, sdkerrors.Wrap(err, "invalid interchain account packet data")
 	}
 
-	// get the next sequence
-	sequence, found := k.channelKeeper.GetNextSequenceSend(ctx, sourcePort, sourceChannel)
-	if !found {
-		return 0, sdkerrors.Wrapf(channeltypes.ErrSequenceSendNotFound, "failed to retrieve next sequence send for channel %s on port %s", sourceChannel, sourcePort)
-	}
-
-	packet := channeltypes.NewPacket(
-		icaPacketData.GetBytes(),
-		sequence,
-		sourcePort,
-		sourceChannel,
-		destinationPort,
-		destinationChannel,
-		clienttypes.ZeroHeight(),
-		timeoutTimestamp,
-	)
-
-	if err := k.ics4Wrapper.SendPacket(ctx, chanCap, packet); err != nil {
+	sequence, err := k.ics4Wrapper.SendPacket(ctx, chanCap, portID, activeChannelID, clienttypes.ZeroHeight(), timeoutTimestamp, icaPacketData.GetBytes())
+	if err != nil {
 		return 0, err
 	}
 
-	return packet.Sequence, nil
+	return sequence, nil
 }
 
 // OnTimeoutPacket removes the active channel associated with the provided packet, the underlying channel end is closed

modules/apps/27-interchain-accounts/controller/keeper/relay_test.go

@@ -90,13 +90,6 @@ func (suite *KeeperTestSuite) TestSendTx() {
 			},
 			false,
 		},
-		{
-			"channel does not exist",
-			func() {
-				suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, "channel-100")
-			},
-			false,
-		},
 		{
 			"channel in INIT state - optimistic packet sends fail",
 			func() {

modules/apps/27-interchain-accounts/host/keeper/keeper.go

@@ -15,6 +15,7 @@ import (
 	"github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/host/types"
 	icatypes "github.com/cosmos/ibc-go/v6/modules/apps/27-interchain-accounts/types"
 	channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
+	porttypes "github.com/cosmos/ibc-go/v6/modules/core/05-port/types"
 	host "github.com/cosmos/ibc-go/v6/modules/core/24-host"
 )
 
@@ -24,7 +25,7 @@ type Keeper struct {
 	cdc        codec.BinaryCodec
 	paramSpace paramtypes.Subspace
 
-	ics4Wrapper   icatypes.ICS4Wrapper
+	ics4Wrapper   porttypes.ICS4Wrapper
 	channelKeeper icatypes.ChannelKeeper
 	portKeeper    icatypes.PortKeeper
 	accountKeeper icatypes.AccountKeeper
@@ -37,7 +38,7 @@ type Keeper struct {
 // NewKeeper creates a new interchain accounts host Keeper instance
 func NewKeeper(
 	cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace paramtypes.Subspace,
-	ics4Wrapper icatypes.ICS4Wrapper, channelKeeper icatypes.ChannelKeeper, portKeeper icatypes.PortKeeper,
+	ics4Wrapper porttypes.ICS4Wrapper, channelKeeper icatypes.ChannelKeeper, portKeeper icatypes.PortKeeper,
 	accountKeeper icatypes.AccountKeeper, scopedKeeper icatypes.ScopedKeeper, msgRouter icatypes.MessageRouter,
 ) Keeper {
 	// ensure ibc interchain accounts module account is set

modules/apps/27-interchain-accounts/types/expected_keepers.go

@@ -18,12 +18,6 @@ type AccountKeeper interface {
 	GetModuleAddress(name string) sdk.AccAddress
 }
 
-// ICS4Wrapper defines the expected ICS4Wrapper for middleware
-type ICS4Wrapper interface {
-	SendPacket(ctx sdk.Context, channelCap *capabilitytypes.Capability, packet ibcexported.PacketI) error
-	GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool)
-}
-
 // ChannelKeeper defines the expected IBC channel keeper
 type ChannelKeeper interface {
 	GetChannel(ctx sdk.Context, srcPort, srcChan string) (channel channeltypes.Channel, found bool)

modules/apps/29-fee/ibc_middleware.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/cosmos/ibc-go/v6/modules/apps/29-fee/keeper"
 	"github.com/cosmos/ibc-go/v6/modules/apps/29-fee/types"
+	clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
 	channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
 	porttypes "github.com/cosmos/ibc-go/v6/modules/core/05-port/types"
 	"github.com/cosmos/ibc-go/v6/modules/core/exported"
@@ -337,9 +338,13 @@ func (im IBCMiddleware) OnTimeoutPacket(
 func (im IBCMiddleware) SendPacket(
 	ctx sdk.Context,
 	chanCap *capabilitytypes.Capability,
-	packet exported.PacketI,
-) error {
-	return im.keeper.SendPacket(ctx, chanCap, packet)
+	sourcePort string,
+	sourceChannel string,
+	timeoutHeight clienttypes.Height,
+	timeoutTimestamp uint64,
+	data []byte,
+) (uint64, error) {
+	return im.keeper.SendPacket(ctx, chanCap, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, data)
 }
 
 // WriteAcknowledgement implements the ICS4 Wrapper interface