ICA Controller Side Middleware (#417)

* initial draft for ica middleware

* add capability handling, fix build and tests

* split module.go into ibc_module.go

* add middleware handshake tests

* fix app.go wiring and various tests

* godoc self nits

* remove unnecessary error

* update comment

* fix testing build

* split channel keeper interface

splits ChannelKeeper interface into ics4Wrapper and core ChannelKeeper

* fix tests

* remove comments

* add callback for timeouts

* Apply suggestions from code review

Co-authored-by: Sean King <seantking@users.noreply.github.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* fix test and update testing README

apply test fix pointed out by Sean.
Update testing README to reflect how to test with the mock module for middleware

* add OnRecvPacket test

Add test cases for OnRecvPacket, reused structure in relay_test.go

* add failing test case for NegotiateAppVersion

Co-authored-by: Sean King <seantking@users.noreply.github.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

Author: 3 people

modules/apps/27-interchain-accounts/ibc_module.go

@@ -1,20 +1,17 @@
 package interchain_accounts
 
 import (
-	"fmt"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
 
 	"github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/keeper"
-	"github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types"
 	channeltypes "github.com/cosmos/ibc-go/v2/modules/core/04-channel/types"
 	porttypes "github.com/cosmos/ibc-go/v2/modules/core/05-port/types"
 	ibcexported "github.com/cosmos/ibc-go/v2/modules/core/exported"
 )
 
-// IBCModule implements the ICS26 callbacks for interchain accounts given the
+// IBCModule implements the ICS26 interface for interchain accounts given the
 // interchain account keeper and underlying application.
 type IBCModule struct {
 	keeper keeper.Keeper
@@ -29,7 +26,13 @@ func NewIBCModule(k keeper.Keeper, app porttypes.IBCModule) IBCModule {
 	}
 }
 
-// OnChanOpenInit implements the IBCModule interface
+// OnChanOpenInit implements the IBCModule interface. Interchain Accounts is
+// implemented to act as middleware for connected authentication modules on
+// the controller side. The connected modules may not change the controller side portID or
+// version. They will be allowed to perform custom logic without changing
+// the parameters stored within a channel struct.
+//
+// Controller Chain
 func (im IBCModule) OnChanOpenInit(
 	ctx sdk.Context,
 	order channeltypes.Order,
@@ -40,10 +43,18 @@ func (im IBCModule) OnChanOpenInit(
 	counterparty channeltypes.Counterparty,
 	version string,
 ) error {
-	return im.keeper.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version)
+	if err := im.keeper.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version); err != nil {
+		return err
+	}
+
+	// call underlying app's OnChanOpenInit callback with the appVersion
+	return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID,
+		chanCap, counterparty, version)
 }
 
 // OnChanOpenTry implements the IBCModule interface
+//
+// Host Chain
 func (im IBCModule) OnChanOpenTry(
 	ctx sdk.Context,
 	order channeltypes.Order,
@@ -58,17 +69,30 @@ func (im IBCModule) OnChanOpenTry(
 	return im.keeper.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version, counterpartyVersion)
 }
 
-// OnChanOpenAck implements the IBCModule interface
+// OnChanOpenAck implements the IBCModule interface. Interchain Accounts is
+// implemented to act as middleware for connected authentication modules on
+// the controller side. The connected modules may not change the portID or
+// version. They will be allowed to perform custom logic without changing
+// the parameters stored within a channel struct.
+//
+// Controller Chain
 func (im IBCModule) OnChanOpenAck(
 	ctx sdk.Context,
 	portID,
 	channelID string,
 	counterpartyVersion string,
 ) error {
-	return im.keeper.OnChanOpenAck(ctx, portID, channelID, counterpartyVersion)
+	if err := im.keeper.OnChanOpenAck(ctx, portID, channelID, counterpartyVersion); err != nil {
+		return err
+	}
+
+	// call underlying app's OnChanOpenAck callback with the counterparty app version.
+	return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyVersion)
 }
 
-// OnChanOpenConfirm implements the IBCModule interface
+// OnChanOpenAck implements the IBCModule interface
+//
+// Host Chain
 func (im IBCModule) OnChanOpenConfirm(
 	ctx sdk.Context,
 	portID,
@@ -97,18 +121,15 @@ func (im IBCModule) OnChanCloseConfirm(
 }
 
 // OnRecvPacket implements the IBCModule interface
+//
+// Host Chain
 func (im IBCModule) OnRecvPacket(
 	ctx sdk.Context,
 	packet channeltypes.Packet,
 	_ sdk.AccAddress,
 ) ibcexported.Acknowledgement {
 	ack := channeltypes.NewResultAcknowledgement([]byte{byte(1)})
 
-	var data types.InterchainAccountPacketData
-	if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
-		ack = channeltypes.NewErrorAcknowledgement(fmt.Sprintf("cannot unmarshal ICS-27 interchain account packet data: %s", err.Error()))
-	}
-
 	// only attempt the application logic if the packet data
 	// was successfully decoded
 	if ack.Success() {
@@ -123,36 +144,31 @@ func (im IBCModule) OnRecvPacket(
 }
 
 // OnAcknowledgementPacket implements the IBCModule interface
+//
+// Controller Chain
 func (im IBCModule) OnAcknowledgementPacket(
 	ctx sdk.Context,
 	packet channeltypes.Packet,
 	acknowledgement []byte,
-	_ sdk.AccAddress,
+	relayer sdk.AccAddress,
 ) error {
-	var ack channeltypes.Acknowledgement
-
-	if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil {
-		return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-27 interchain account packet acknowledgment: %v", err)
-	}
-	var data types.InterchainAccountPacketData
-	if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
-		return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-27 interchain account packet data: %s", err.Error())
-	}
-
-	if err := im.keeper.OnAcknowledgementPacket(ctx, packet, data, ack); err != nil {
-		return err
-	}
-
-	return nil
+	// call underlying app's OnAcknowledgementPacket callback.
+	return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
 }
 
 // OnTimeoutPacket implements the IBCModule interface
+//
+// Controller Chain
 func (im IBCModule) OnTimeoutPacket(
 	ctx sdk.Context,
 	packet channeltypes.Packet,
-	_ sdk.AccAddress,
+	relayer sdk.AccAddress,
 ) error {
-	return im.keeper.OnTimeoutPacket(ctx, packet)
+	if err := im.keeper.OnTimeoutPacket(ctx, packet); err != nil {
+		return err
+	}
+
+	return im.app.OnTimeoutPacket(ctx, packet, relayer)
 }
 
 // NegotiateAppVersion implements the IBCModule interface