refactor: cleanup middleware stacks (#1248)

* refactor: UX improvements for middleware * fix: move fee keeper * fix: remove unncessary wrapping * refactor: ica module stack setup * fix: define icaControllerStack as type porttypes.IBCModule * add back controller routing for port * refactor: remove unncessary SendPacket * refactor: panic instead of return nil * refactor: changing structure to init all keepers, then stacks & routing * docs: add comments * nit: comment * chore: rename files * chore: comment + nit * refactor: stacks * Update modules/apps/27-interchain-accounts/controller/ibc_middleware.go Co-authored-by: Aditya <adityasripal@gmail.com> * nits: nits * chore: comment * chore: chore Co-authored-by: Colin Axnér <25233464+colin-axner@users.noreply.github.com> Co-authored-by: Aditya <adityasripal@gmail.com>
cosmos · Apr 28, 2022 · cc4cd7b · cc4cd7b
1 parent 0a98ac6
commit cc4cd7b
Show file tree

Hide file tree

Showing 7 changed files with 191 additions and 103 deletions.
diff --git a/...erchain-accounts/controller/ibc_module.go → ...ain-accounts/controller/ibc_middleware.go b/...erchain-accounts/controller/ibc_module.go → ...ain-accounts/controller/ibc_middleware.go
@@ -13,27 +13,30 @@ import (
 	ibcexported "github.com/cosmos/ibc-go/v3/modules/core/exported"
 )
 
-// IBCModule implements the ICS26 interface for interchain accounts controller chains
-type IBCModule struct {
-	keeper keeper.Keeper
+var _ porttypes.Middleware = &IBCMiddleware{}
+
+// IBCMiddleware implements the ICS26 callbacks for the fee middleware given the
+// ICA controller keeper and the underlying application.
+type IBCMiddleware struct {
 	app    porttypes.IBCModule
+	keeper keeper.Keeper
 }
 
-// NewIBCModule creates a new IBCModule given the associated keeper and underlying application
-func NewIBCModule(k keeper.Keeper, app porttypes.IBCModule) IBCModule {
-	return IBCModule{
-		keeper: k,
+// IBCMiddleware creates a new IBCMiddleware given the associated keeper and underlying application
+func NewIBCMiddleware(app porttypes.IBCModule, k keeper.Keeper) IBCMiddleware {
+	return IBCMiddleware{
 		app:    app,
+		keeper: k,
 	}
 }
 
-// OnChanOpenInit implements the IBCModule interface
+// OnChanOpenInit implements the IBCMiddleware 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.
-func (im IBCModule) OnChanOpenInit(
+func (im IBCMiddleware) OnChanOpenInit(
 	ctx sdk.Context,
 	order channeltypes.Order,
 	connectionHops []string,
@@ -56,8 +59,8 @@ func (im IBCModule) OnChanOpenInit(
 		chanCap, counterparty, version)
 }
 
-// OnChanOpenTry implements the IBCModule interface
-func (im IBCModule) OnChanOpenTry(
+// OnChanOpenTry implements the IBCMiddleware interface
+func (im IBCMiddleware) OnChanOpenTry(
 	ctx sdk.Context,
 	order channeltypes.Order,
 	connectionHops []string,
@@ -70,13 +73,13 @@ func (im IBCModule) OnChanOpenTry(
 	return "", sdkerrors.Wrap(icatypes.ErrInvalidChannelFlow, "channel handshake must be initiated by controller chain")
 }
 
-// OnChanOpenAck implements the IBCModule interface
+// OnChanOpenAck implements the IBCMiddleware 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.
-func (im IBCModule) OnChanOpenAck(
+func (im IBCMiddleware) OnChanOpenAck(
 	ctx sdk.Context,
 	portID,
 	channelID string,
@@ -95,17 +98,17 @@ func (im IBCModule) OnChanOpenAck(
 	return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion)
 }
 
-// OnChanOpenAck implements the IBCModule interface
-func (im IBCModule) OnChanOpenConfirm(
+// OnChanOpenAck implements the IBCMiddleware interface
+func (im IBCMiddleware) OnChanOpenConfirm(
 	ctx sdk.Context,
 	portID,
 	channelID string,
 ) error {
 	return sdkerrors.Wrap(icatypes.ErrInvalidChannelFlow, "channel handshake must be initiated by controller chain")
 }
 
-// OnChanCloseInit implements the IBCModule interface
-func (im IBCModule) OnChanCloseInit(
+// OnChanCloseInit implements the IBCMiddleware interface
+func (im IBCMiddleware) OnChanCloseInit(
 	ctx sdk.Context,
 	portID,
 	channelID string,
@@ -114,26 +117,26 @@ func (im IBCModule) OnChanCloseInit(
 	return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "user cannot close channel")
 }
 
-// OnChanCloseConfirm implements the IBCModule interface
-func (im IBCModule) OnChanCloseConfirm(
+// OnChanCloseConfirm implements the IBCMiddleware interface
+func (im IBCMiddleware) OnChanCloseConfirm(
 	ctx sdk.Context,
 	portID,
 	channelID string,
 ) error {
 	return im.keeper.OnChanCloseConfirm(ctx, portID, channelID)
 }
 
-// OnRecvPacket implements the IBCModule interface
-func (im IBCModule) OnRecvPacket(
+// OnRecvPacket implements the IBCMiddleware interface
+func (im IBCMiddleware) OnRecvPacket(
 	ctx sdk.Context,
 	packet channeltypes.Packet,
 	_ sdk.AccAddress,
 ) ibcexported.Acknowledgement {
 	return channeltypes.NewErrorAcknowledgement("cannot receive packet on controller chain")
 }
 
-// OnAcknowledgementPacket implements the IBCModule interface
-func (im IBCModule) OnAcknowledgementPacket(
+// OnAcknowledgementPacket implements the IBCMiddleware interface
+func (im IBCMiddleware) OnAcknowledgementPacket(
 	ctx sdk.Context,
 	packet channeltypes.Packet,
 	acknowledgement []byte,
@@ -147,8 +150,8 @@ func (im IBCModule) OnAcknowledgementPacket(
 	return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
 }
 
-// OnTimeoutPacket implements the IBCModule interface
-func (im IBCModule) OnTimeoutPacket(
+// OnTimeoutPacket implements the IBCMiddleware interface
+func (im IBCMiddleware) OnTimeoutPacket(
 	ctx sdk.Context,
 	packet channeltypes.Packet,
 	relayer sdk.AccAddress,
@@ -164,7 +167,27 @@ func (im IBCModule) OnTimeoutPacket(
 	return im.app.OnTimeoutPacket(ctx, packet, relayer)
 }
 
+// SendPacket implements the ICS4 Wrapper interface
+// The ICA controller module builds the outgoing ICA packet and calls the core IBC SendPacket
+func (im IBCMiddleware) SendPacket(
+	ctx sdk.Context,
+	chanCap *capabilitytypes.Capability,
+	packet ibcexported.PacketI,
+) error {
+	panic("SendPacket not supported for ICA-auth module. Please use SendTx")
+}
+
+// WriteAcknowledgement implements the ICS4 Wrapper interface
+func (im IBCMiddleware) WriteAcknowledgement(
+	ctx sdk.Context,
+	chanCap *capabilitytypes.Capability,
+	packet ibcexported.PacketI,
+	ack ibcexported.Acknowledgement,
+) error {
+	panic("WriteAcknowledgement not supported for ICA controller module")
+}
+
 // GetAppVersion returns the interchain accounts metadata.
-func (im IBCModule) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
+func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
 	return im.keeper.GetAppVersion(ctx, portID, channelID)
 }
diff --git a/...in-accounts/controller/ibc_module_test.go → ...ccounts/controller/ibc_middleware_test.go b/...in-accounts/controller/ibc_module_test.go → ...ccounts/controller/ibc_middleware_test.go
@@ -717,7 +717,7 @@ func (suite *InterchainAccountsTestSuite) TestGetAppVersion() {
 	cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
 	suite.Require().True(ok)
 
-	controllerModule := cbs.(icacontroller.IBCModule)
+	controllerModule := cbs.(icacontroller.IBCMiddleware)
 
 	appVersion, found := controllerModule.GetAppVersion(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 	suite.Require().True(found)

diff --git a/modules/apps/27-interchain-accounts/module.go b/modules/apps/27-interchain-accounts/module.go
@@ -16,7 +16,6 @@ import (
 	abci "github.com/tendermint/tendermint/abci/types"
 
 	"github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/client/cli"
-	"github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller"
 	controllerkeeper "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller/keeper"
 	controllertypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/controller/types"
 	"github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/host"
@@ -31,7 +30,6 @@ var (
 	_ module.AppModule      = AppModule{}
 	_ module.AppModuleBasic = AppModuleBasic{}
 
-	_ porttypes.IBCModule = controller.IBCModule{}
 	_ porttypes.IBCModule = host.IBCModule{}
 )
 

diff --git a/modules/apps/29-fee/ibc_module.go → modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_module.go → modules/apps/29-fee/ibc_middleware.go
@@ -12,22 +12,25 @@ import (
 	"github.com/cosmos/ibc-go/v3/modules/core/exported"
 )
 
-// IBCModule implements the ICS26 callbacks for the fee middleware given the fee keeper and the underlying application.
-type IBCModule struct {
-	keeper keeper.Keeper
+var _ porttypes.Middleware = &IBCMiddleware{}
+
+// IBCMiddleware implements the ICS26 callbacks for the fee middleware given the
+// fee keeper and the underlying application.
+type IBCMiddleware struct {
 	app    porttypes.IBCModule
+	keeper keeper.Keeper
 }
 
-// NewIBCModule creates a new IBCModule given the keeper and underlying application
-func NewIBCModule(k keeper.Keeper, app porttypes.IBCModule) IBCModule {
-	return IBCModule{
-		keeper: k,
+// NewIBCMiddleware creates a new IBCMiddlware given the keeper and underlying application
+func NewIBCMiddleware(app porttypes.IBCModule, k keeper.Keeper) IBCMiddleware {
+	return IBCMiddleware{
 		app:    app,
+		keeper: k,
 	}
 }
 
-// OnChanOpenInit implements the IBCModule interface
-func (im IBCModule) OnChanOpenInit(
+// OnChanOpenInit implements the IBCMiddleware interface
+func (im IBCMiddleware) OnChanOpenInit(
 	ctx sdk.Context,
 	order channeltypes.Order,
 	connectionHops []string,
@@ -57,10 +60,10 @@ func (im IBCModule) OnChanOpenInit(
 		chanCap, counterparty, versionMetadata.AppVersion)
 }
 
-// OnChanOpenTry implements the IBCModule interface
+// OnChanOpenTry implements the IBCMiddleware interface
 // If the channel is not fee enabled the underlying application version will be returned
 // If the channel is fee enabled we merge the underlying application version with the ics29 version
-func (im IBCModule) OnChanOpenTry(
+func (im IBCMiddleware) OnChanOpenTry(
 	ctx sdk.Context,
 	order channeltypes.Order,
 	connectionHops []string,
@@ -100,8 +103,8 @@ func (im IBCModule) OnChanOpenTry(
 	return string(versionBytes), nil
 }
 
-// OnChanOpenAck implements the IBCModule interface
-func (im IBCModule) OnChanOpenAck(
+// OnChanOpenAck implements the IBCMiddleware interface
+func (im IBCMiddleware) OnChanOpenAck(
 	ctx sdk.Context,
 	portID,
 	channelID string,
@@ -128,8 +131,8 @@ func (im IBCModule) OnChanOpenAck(
 	return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion)
 }
 
-// OnChanOpenConfirm implements the IBCModule interface
-func (im IBCModule) OnChanOpenConfirm(
+// OnChanOpenConfirm implements the IBCMiddleware interface
+func (im IBCMiddleware) OnChanOpenConfirm(
 	ctx sdk.Context,
 	portID,
 	channelID string,
@@ -138,8 +141,8 @@ func (im IBCModule) OnChanOpenConfirm(
 	return im.app.OnChanOpenConfirm(ctx, portID, channelID)
 }
 
-// OnChanCloseInit implements the IBCModule interface
-func (im IBCModule) OnChanCloseInit(
+// OnChanCloseInit implements the IBCMiddleware interface
+func (im IBCMiddleware) OnChanCloseInit(
 	ctx sdk.Context,
 	portID,
 	channelID string,
@@ -155,8 +158,8 @@ func (im IBCModule) OnChanCloseInit(
 	return nil
 }
 
-// OnChanCloseConfirm implements the IBCModule interface
-func (im IBCModule) OnChanCloseConfirm(
+// OnChanCloseConfirm implements the IBCMiddleware interface
+func (im IBCMiddleware) OnChanCloseConfirm(
 	ctx sdk.Context,
 	portID,
 	channelID string,
@@ -172,9 +175,9 @@ func (im IBCModule) OnChanCloseConfirm(
 	return nil
 }
 
-// OnRecvPacket implements the IBCModule interface.
+// OnRecvPacket implements the IBCMiddleware interface.
 // If fees are not enabled, this callback will default to the ibc-core packet callback
-func (im IBCModule) OnRecvPacket(
+func (im IBCMiddleware) OnRecvPacket(
 	ctx sdk.Context,
 	packet channeltypes.Packet,
 	relayer sdk.AccAddress,
@@ -197,9 +200,9 @@ func (im IBCModule) OnRecvPacket(
 	return types.NewIncentivizedAcknowledgement(forwardRelayer, ack.Acknowledgement(), ack.Success())
 }
 
-// OnAcknowledgementPacket implements the IBCModule interface
+// OnAcknowledgementPacket implements the IBCMiddleware interface
 // If fees are not enabled, this callback will default to the ibc-core packet callback
-func (im IBCModule) OnAcknowledgementPacket(
+func (im IBCMiddleware) OnAcknowledgementPacket(
 	ctx sdk.Context,
 	packet channeltypes.Packet,
 	acknowledgement []byte,
@@ -239,9 +242,9 @@ func (im IBCModule) OnAcknowledgementPacket(
 	return im.app.OnAcknowledgementPacket(ctx, packet, ack.Result, relayer)
 }
 
-// OnTimeoutPacket implements the IBCModule interface
+// OnTimeoutPacket implements the IBCMiddleware interface
 // If fees are not enabled, this callback will default to the ibc-core packet callback
-func (im IBCModule) OnTimeoutPacket(
+func (im IBCMiddleware) OnTimeoutPacket(
 	ctx sdk.Context,
 	packet channeltypes.Packet,
 	relayer sdk.AccAddress,
@@ -268,7 +271,26 @@ func (im IBCModule) OnTimeoutPacket(
 	return im.app.OnTimeoutPacket(ctx, packet, relayer)
 }
 
+// SendPacket implements the ICS4 Wrapper interface
+func (im IBCMiddleware) SendPacket(
+	ctx sdk.Context,
+	chanCap *capabilitytypes.Capability,
+	packet exported.PacketI,
+) error {
+	return im.keeper.SendPacket(ctx, chanCap, packet)
+}
+
+// WriteAcknowledgement implements the ICS4 Wrapper interface
+func (im IBCMiddleware) WriteAcknowledgement(
+	ctx sdk.Context,
+	chanCap *capabilitytypes.Capability,
+	packet exported.PacketI,
+	ack exported.Acknowledgement,
+) error {
+	return im.keeper.WriteAcknowledgement(ctx, chanCap, packet, ack)
+}
+
 // GetAppVersion returns the application version of the underlying application
-func (im IBCModule) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
+func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) {
 	return im.keeper.GetAppVersion(ctx, portID, channelID)
 }
diff --git a/modules/apps/29-fee/ibc_module_test.go → modules/apps/29-fee/ibc_middleware_test.go b/modules/apps/29-fee/ibc_module_test.go → modules/apps/29-fee/ibc_middleware_test.go
@@ -896,7 +896,7 @@ func (suite *FeeTestSuite) TestGetAppVersion() {
 			cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
 			suite.Require().True(ok)
 
-			feeModule := cbs.(fee.IBCModule)
+			feeModule := cbs.(fee.IBCMiddleware)
 
 			appVersion, found := feeModule.GetAppVersion(suite.chainA.GetContext(), portID, channelID)
 

diff --git a/modules/apps/29-fee/module.go b/modules/apps/29-fee/module.go
@@ -20,12 +20,10 @@ import (
 	"github.com/cosmos/ibc-go/v3/modules/apps/29-fee/client/cli"
 	"github.com/cosmos/ibc-go/v3/modules/apps/29-fee/keeper"
 	"github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types"
-	porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types"
 )
 
 var (
 	_ module.AppModule      = AppModule{}
-	_ porttypes.IBCModule   = IBCModule{}
 	_ module.AppModuleBasic = AppModuleBasic{}
 )