From 227f47b962f81f1c1796f17fa3ba605d391f3495 Mon Sep 17 00:00:00 2001 From: Sean King Date: Tue, 12 Apr 2022 12:02:21 +0200 Subject: [PATCH 01/18] refactor: UX improvements for middleware --- modules/apps/29-fee/ibc_module.go | 85 ++++++++++++-------- modules/apps/29-fee/module.go | 2 - testing/mock/module.go | 126 ++++++++++++++++++++++++++++++ testing/simapp/app.go | 33 +++++--- 4 files changed, 202 insertions(+), 44 deletions(-) create mode 100644 testing/mock/module.go diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index e4dab034b04..4545606475a 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/modules/apps/29-fee/ibc_module.go @@ -10,24 +10,28 @@ import ( channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types" "github.com/cosmos/ibc-go/v3/modules/core/exported" + ibcmock "github.com/cosmos/ibc-go/v3/testing/mock" ) -// IBCModule implements the ICS26 callbacks for the fee middleware given the fee keeper and the underlying application. -type IBCModule struct { +var _ porttypes.Middleware = &IBCMiddleware{} + +// IBCMiddlware implements the ICS26 callbacks for the fee middleware given the +// fee keeper and the underlying application. +type IBCMiddleware struct { + *ibcmock.Module keeper keeper.Keeper - app porttypes.IBCModule } -// NewIBCModule creates a new IBCModule given the keeper and underlying application -func NewIBCModule(k keeper.Keeper, app porttypes.IBCModule) IBCModule { - return IBCModule{ +// NewIBCMiddlware creates a new IBCMiddlware given the keeper and underlying application +func NewIBCMiddleware(app porttypes.IBCModule, k keeper.Keeper) IBCMiddleware { + return IBCMiddleware{ + Module: ibcmock.NewModule(app), keeper: k, - app: app, } } // OnChanOpenInit implements the IBCModule interface -func (im IBCModule) OnChanOpenInit( +func (im IBCMiddleware) OnChanOpenInit( ctx sdk.Context, order channeltypes.Order, connectionHops []string, @@ -42,7 +46,7 @@ func (im IBCModule) OnChanOpenInit( // Since it is valid for fee version to not be specified, the above middleware version may be for a middleware // lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying // application. - return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, + return im.Module.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version) } @@ -53,14 +57,14 @@ func (im IBCModule) OnChanOpenInit( im.keeper.SetFeeEnabled(ctx, portID, channelID) // call underlying app's OnChanOpenInit callback with the appVersion - return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, + return im.Module.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, versionMetadata.AppVersion) } // OnChanOpenTry implements the IBCModule 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, @@ -75,7 +79,7 @@ func (im IBCModule) OnChanOpenTry( // Since it is valid for fee version to not be specified, the above middleware version may be for a middleware // lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying // application. - return im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, counterpartyVersion) + return im.Module.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, counterpartyVersion) } if versionMetadata.FeeVersion != types.Version { @@ -85,7 +89,7 @@ func (im IBCModule) OnChanOpenTry( im.keeper.SetFeeEnabled(ctx, portID, channelID) // call underlying app's OnChanOpenTry callback with the app versions - appVersion, err := im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, versionMetadata.AppVersion) + appVersion, err := im.Module.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, versionMetadata.AppVersion) if err != nil { return "", err } @@ -101,7 +105,7 @@ func (im IBCModule) OnChanOpenTry( } // OnChanOpenAck implements the IBCModule interface -func (im IBCModule) OnChanOpenAck( +func (im IBCMiddleware) OnChanOpenAck( ctx sdk.Context, portID, channelID string, @@ -121,30 +125,30 @@ func (im IBCModule) OnChanOpenAck( } // call underlying app's OnChanOpenAck callback with the counterparty app version. - return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, versionMetadata.AppVersion) + return im.Module.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, versionMetadata.AppVersion) } // call underlying app's OnChanOpenAck callback with the counterparty app version. - return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion) + return im.Module.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion) } // OnChanOpenConfirm implements the IBCModule interface -func (im IBCModule) OnChanOpenConfirm( +func (im IBCMiddleware) OnChanOpenConfirm( ctx sdk.Context, portID, channelID string, ) error { // call underlying app's OnChanOpenConfirm callback. - return im.app.OnChanOpenConfirm(ctx, portID, channelID) + return im.Module.OnChanOpenConfirm(ctx, portID, channelID) } // OnChanCloseInit implements the IBCModule interface -func (im IBCModule) OnChanCloseInit( +func (im IBCMiddleware) OnChanCloseInit( ctx sdk.Context, portID, channelID string, ) error { - if err := im.app.OnChanCloseInit(ctx, portID, channelID); err != nil { + if err := im.Module.OnChanCloseInit(ctx, portID, channelID); err != nil { return err } @@ -167,7 +171,7 @@ func (im IBCModule) OnChanCloseInit( } // OnChanCloseConfirm implements the IBCModule interface -func (im IBCModule) OnChanCloseConfirm( +func (im IBCMiddleware) OnChanCloseConfirm( ctx sdk.Context, portID, channelID string, @@ -186,21 +190,21 @@ func (im IBCModule) OnChanCloseConfirm( if err != nil { im.keeper.DisableAllChannels(ctx) } - return im.app.OnChanCloseConfirm(ctx, portID, channelID) + return im.Module.OnChanCloseConfirm(ctx, portID, channelID) } // OnRecvPacket implements the IBCModule 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, ) exported.Acknowledgement { if !im.keeper.IsFeeEnabled(ctx, packet.DestinationPort, packet.DestinationChannel) { - return im.app.OnRecvPacket(ctx, packet, relayer) + return im.Module.OnRecvPacket(ctx, packet, relayer) } - ack := im.app.OnRecvPacket(ctx, packet, relayer) + ack := im.Module.OnRecvPacket(ctx, packet, relayer) // incase of async aknowledgement (ack == nil) store the relayer address for use later during async WriteAcknowledgement if ack == nil { @@ -216,14 +220,14 @@ func (im IBCModule) OnRecvPacket( // OnAcknowledgementPacket implements the IBCModule 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, relayer sdk.AccAddress, ) error { if !im.keeper.IsFeeEnabled(ctx, packet.SourcePort, packet.SourceChannel) { - return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) + return im.Module.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) } ack := new(types.IncentivizedAcknowledgement) @@ -241,18 +245,18 @@ func (im IBCModule) OnAcknowledgementPacket( } // call underlying callback - return im.app.OnAcknowledgementPacket(ctx, packet, ack.Result, relayer) + return im.Module.OnAcknowledgementPacket(ctx, packet, ack.Result, relayer) } // OnTimeoutPacket implements the IBCModule 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, ) error { if !im.keeper.IsFeeEnabled(ctx, packet.SourcePort, packet.SourceChannel) { - return im.app.OnTimeoutPacket(ctx, packet, relayer) + return im.Module.OnTimeoutPacket(ctx, packet, relayer) } packetID := channeltypes.NewPacketId(packet.SourceChannel, packet.SourcePort, packet.Sequence) @@ -265,5 +269,24 @@ func (im IBCModule) OnTimeoutPacket( } // call underlying callback - return im.app.OnTimeoutPacket(ctx, packet, relayer) + return im.Module.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) } diff --git a/modules/apps/29-fee/module.go b/modules/apps/29-fee/module.go index e493e047837..895a475b512 100644 --- 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{} ) diff --git a/testing/mock/module.go b/testing/mock/module.go new file mode 100644 index 00000000000..f44a46d2b65 --- /dev/null +++ b/testing/mock/module.go @@ -0,0 +1,126 @@ +package mock + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" + channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" + porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types" + "github.com/cosmos/ibc-go/v3/modules/core/exported" +) + +var _ porttypes.IBCModule = &Module{} + +// Module a concrete type for a module boilerplate. +type Module struct { + app porttypes.IBCModule +} + +// NewModule creates a new IBC Module boilerplate given the underlying IBC app +func NewModule(app porttypes.IBCModule) *Module { + return &Module{ + app: app, + } +} + +// OnChanOpenInit implements the Module interface +// It calls the underlying app's OnChanOpenInit callback. +func (im Module) OnChanOpenInit( + ctx sdk.Context, + order channeltypes.Order, + connectionHops []string, + portID string, + channelID string, + chanCap *capabilitytypes.Capability, + counterparty channeltypes.Counterparty, + version string, +) error { + return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version) +} + +// OnChanOpenTry implements the Module interface. +// It calls the underlying app's OnChanOpenTry callback. +func (im Module) OnChanOpenTry( + ctx sdk.Context, + order channeltypes.Order, + connectionHops []string, + portID, + channelID string, + chanCap *capabilitytypes.Capability, + counterparty channeltypes.Counterparty, + counterpartyVersion string, +) (version string, err error) { + return im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, counterpartyVersion) +} + +// OnChanOpenAck implements the Module interface. +// It calls the underlying app's OnChanOpenAck callback. +func (im Module) OnChanOpenAck( + ctx sdk.Context, + portID, + channelID, + counterpartyChannelID, + counterpartyVersion string, +) error { + return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion) +} + +// OnChanOpenConfirm implements the Module interface. +// It calls the underlying app's OnChanOpenConfirm callback. +func (im Module) OnChanOpenConfirm( + ctx sdk.Context, + portID, + channelID string, +) error { + return im.app.OnChanOpenConfirm(ctx, portID, channelID) +} + +// OnChanCloseInit implements the Module interface +// It calls the underlying app's OnChanCloseInit callback. +func (im Module) OnChanCloseInit( + ctx sdk.Context, + portID, + channelID string, +) error { + return im.app.OnChanCloseInit(ctx, portID, channelID) +} + +// OnChanCloseConfirm implements the Module interface. +// It calls the underlying app's OnChanCloseConfirm callback. +func (im Module) OnChanCloseConfirm( + ctx sdk.Context, + portID, + channelID string, +) error { + return im.app.OnChanCloseConfirm(ctx, portID, channelID) +} + +// OnRecvPacket implements the Module interface. +// It calls the underlying app's OnRecvPacket callback. +func (im Module) OnRecvPacket( + ctx sdk.Context, + packet channeltypes.Packet, + relayer sdk.AccAddress, +) exported.Acknowledgement { + return im.app.OnRecvPacket(ctx, packet, relayer) +} + +// OnAcknowledgementPacket implements the Module interface. +// It calls the underlying app's OnAcknowledgementPacket callback. +func (im Module) OnAcknowledgementPacket( + ctx sdk.Context, + packet channeltypes.Packet, + acknowledgement []byte, + relayer sdk.AccAddress, +) error { + return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) +} + +// OnTimeoutPacket implements the Module interface. +// It calls the underlying app's OnTimeoutPacket callback. +func (im Module) OnTimeoutPacket( + ctx sdk.Context, + packet channeltypes.Packet, + relayer sdk.AccAddress, +) error { + return im.app.OnTimeoutPacket(ctx, packet, relayer) +} diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 738e29cdd59..43c828bdaab 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -353,25 +353,33 @@ func NewSimApp( &stakingKeeper, govRouter, ) - app.IBCFeeKeeper = ibcfeekeeper.NewKeeper(appCodec, keys[ibcfeetypes.StoreKey], app.GetSubspace(ibcfeetypes.ModuleName), - app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, app.AccountKeeper, app.BankKeeper, - ) + // Create Transfer Stack // Create Transfer Keeper and pass IBCFeeKeeper as expected Channel and PortKeeper // since fee middleware will wrap the IBCKeeper for underlying application. app.TransferKeeper = ibctransferkeeper.NewKeeper( appCodec, keys[ibctransfertypes.StoreKey], app.GetSubspace(ibctransfertypes.ModuleName), - app.IBCFeeKeeper, app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, + app.IBCFeeKeeper, // ISC4 Wrapper: fee IBC middleware + app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, app.AccountKeeper, app.BankKeeper, scopedTransferKeeper, ) + transferModule := transfer.NewAppModule(app.TransferKeeper) - transferIBCModule := transfer.NewIBCModule(app.TransferKeeper) - // create fee-wrapped transfer module - feeTransferModule := ibcfee.NewIBCModule(app.IBCFeeKeeper, transferIBCModule) + app.IBCFeeKeeper = ibcfeekeeper.NewKeeper(appCodec, keys[ibcfeetypes.StoreKey], app.GetSubspace(ibcfeetypes.ModuleName), + app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, app.AccountKeeper, app.BankKeeper, + ) - feeModule := ibcfee.NewAppModule(app.IBCFeeKeeper) + // transfer stack contains (from top to bottom): + // - IBC Fee Middleware + // - Transfer + // create IBC module from bottom to top of stack + var transferStack porttypes.IBCModule + transferStack = transfer.NewIBCModule(app.TransferKeeper) + transferStack = ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper) + + // TODO: clean up remaining modules // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do // not replicate if you do not need to test core IBC or light clients. mockModule := ibcmock.NewAppModule(&app.IBCKeeper.PortKeeper) @@ -380,7 +388,7 @@ func NewSimApp( feeMockModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp(MockFeePort, scopedFeeMockKeeper)) app.FeeMockModule = feeMockModule - feeWithMockModule := ibcfee.NewIBCModule(app.IBCFeeKeeper, feeMockModule) + feeWithMockModule := ibcfee.NewIBCMiddleware(feeMockModule, app.IBCFeeKeeper) mockIBCModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp(ibcmock.ModuleName, scopedIBCMockKeeper)) @@ -412,7 +420,7 @@ func NewSimApp( ibcRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerIBCModule). AddRoute(icahosttypes.SubModuleName, icaHostIBCModule). AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerIBCModule). // ica with mock auth module stack route to ica (top level of middleware stack) - AddRoute(ibctransfertypes.ModuleName, feeTransferModule). + AddRoute(ibctransfertypes.ModuleName, transferStack). AddRoute(ibcmock.ModuleName, mockIBCModule). AddRoute(MockFeePort, feeWithMockModule) @@ -434,6 +442,7 @@ func NewSimApp( // NOTE: Any module instantiated in the module manager that is later modified // must be passed by reference here. app.mm = module.NewManager( + // SDK app modules genutil.NewAppModule( app.AccountKeeper, app.StakingKeeper, app.BaseApp.DeliverTx, encodingConfig.TxConfig, @@ -454,8 +463,10 @@ func NewSimApp( ibc.NewAppModule(app.IBCKeeper), params.NewAppModule(app.ParamsKeeper), authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AccountKeeper, app.BankKeeper, app.interfaceRegistry), + + // ibc modules transferModule, - feeModule, + ibcfee.NewAppModule(app.IBCFeeKeeper), icaModule, mockModule, ) From 725936f42a5d58656b105ceac66477536ab3b9bb Mon Sep 17 00:00:00 2001 From: Sean King Date: Tue, 12 Apr 2022 13:52:14 +0200 Subject: [PATCH 02/18] fix: move fee keeper --- testing/simapp/app.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 43c828bdaab..5b63b954d06 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -353,6 +353,10 @@ func NewSimApp( &stakingKeeper, govRouter, ) + app.IBCFeeKeeper = ibcfeekeeper.NewKeeper(appCodec, keys[ibcfeetypes.StoreKey], app.GetSubspace(ibcfeetypes.ModuleName), + app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, app.AccountKeeper, app.BankKeeper, + ) + // Create Transfer Stack // Create Transfer Keeper and pass IBCFeeKeeper as expected Channel and PortKeeper @@ -366,10 +370,6 @@ func NewSimApp( transferModule := transfer.NewAppModule(app.TransferKeeper) - app.IBCFeeKeeper = ibcfeekeeper.NewKeeper(appCodec, keys[ibcfeetypes.StoreKey], app.GetSubspace(ibcfeetypes.ModuleName), - app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, app.AccountKeeper, app.BankKeeper, - ) - // transfer stack contains (from top to bottom): // - IBC Fee Middleware // - Transfer From 700ad6a775d64ffc303c2376a215cf979217dd62 Mon Sep 17 00:00:00 2001 From: Sean King Date: Tue, 12 Apr 2022 14:04:21 +0200 Subject: [PATCH 03/18] fix: remove unncessary wrapping --- modules/apps/29-fee/ibc_module.go | 35 ++++----- testing/mock/module.go | 126 ------------------------------ 2 files changed, 17 insertions(+), 144 deletions(-) delete mode 100644 testing/mock/module.go diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_module.go index 4545606475a..80c1435ea4b 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/modules/apps/29-fee/ibc_module.go @@ -10,7 +10,6 @@ import ( channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types" "github.com/cosmos/ibc-go/v3/modules/core/exported" - ibcmock "github.com/cosmos/ibc-go/v3/testing/mock" ) var _ porttypes.Middleware = &IBCMiddleware{} @@ -18,14 +17,14 @@ var _ porttypes.Middleware = &IBCMiddleware{} // IBCMiddlware implements the ICS26 callbacks for the fee middleware given the // fee keeper and the underlying application. type IBCMiddleware struct { - *ibcmock.Module + app porttypes.IBCModule keeper keeper.Keeper } // NewIBCMiddlware creates a new IBCMiddlware given the keeper and underlying application func NewIBCMiddleware(app porttypes.IBCModule, k keeper.Keeper) IBCMiddleware { return IBCMiddleware{ - Module: ibcmock.NewModule(app), + app: app, keeper: k, } } @@ -46,7 +45,7 @@ func (im IBCMiddleware) OnChanOpenInit( // Since it is valid for fee version to not be specified, the above middleware version may be for a middleware // lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying // application. - return im.Module.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, + return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version) } @@ -57,7 +56,7 @@ func (im IBCMiddleware) OnChanOpenInit( im.keeper.SetFeeEnabled(ctx, portID, channelID) // call underlying app's OnChanOpenInit callback with the appVersion - return im.Module.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, + return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, versionMetadata.AppVersion) } @@ -79,7 +78,7 @@ func (im IBCMiddleware) OnChanOpenTry( // Since it is valid for fee version to not be specified, the above middleware version may be for a middleware // lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying // application. - return im.Module.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, counterpartyVersion) + return im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, counterpartyVersion) } if versionMetadata.FeeVersion != types.Version { @@ -89,7 +88,7 @@ func (im IBCMiddleware) OnChanOpenTry( im.keeper.SetFeeEnabled(ctx, portID, channelID) // call underlying app's OnChanOpenTry callback with the app versions - appVersion, err := im.Module.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, versionMetadata.AppVersion) + appVersion, err := im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, versionMetadata.AppVersion) if err != nil { return "", err } @@ -125,11 +124,11 @@ func (im IBCMiddleware) OnChanOpenAck( } // call underlying app's OnChanOpenAck callback with the counterparty app version. - return im.Module.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, versionMetadata.AppVersion) + return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, versionMetadata.AppVersion) } // call underlying app's OnChanOpenAck callback with the counterparty app version. - return im.Module.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion) + return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion) } // OnChanOpenConfirm implements the IBCModule interface @@ -139,7 +138,7 @@ func (im IBCMiddleware) OnChanOpenConfirm( channelID string, ) error { // call underlying app's OnChanOpenConfirm callback. - return im.Module.OnChanOpenConfirm(ctx, portID, channelID) + return im.app.OnChanOpenConfirm(ctx, portID, channelID) } // OnChanCloseInit implements the IBCModule interface @@ -148,7 +147,7 @@ func (im IBCMiddleware) OnChanCloseInit( portID, channelID string, ) error { - if err := im.Module.OnChanCloseInit(ctx, portID, channelID); err != nil { + if err := im.app.OnChanCloseInit(ctx, portID, channelID); err != nil { return err } @@ -190,7 +189,7 @@ func (im IBCMiddleware) OnChanCloseConfirm( if err != nil { im.keeper.DisableAllChannels(ctx) } - return im.Module.OnChanCloseConfirm(ctx, portID, channelID) + return im.app.OnChanCloseConfirm(ctx, portID, channelID) } // OnRecvPacket implements the IBCModule interface. @@ -201,10 +200,10 @@ func (im IBCMiddleware) OnRecvPacket( relayer sdk.AccAddress, ) exported.Acknowledgement { if !im.keeper.IsFeeEnabled(ctx, packet.DestinationPort, packet.DestinationChannel) { - return im.Module.OnRecvPacket(ctx, packet, relayer) + return im.app.OnRecvPacket(ctx, packet, relayer) } - ack := im.Module.OnRecvPacket(ctx, packet, relayer) + ack := im.app.OnRecvPacket(ctx, packet, relayer) // incase of async aknowledgement (ack == nil) store the relayer address for use later during async WriteAcknowledgement if ack == nil { @@ -227,7 +226,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket( relayer sdk.AccAddress, ) error { if !im.keeper.IsFeeEnabled(ctx, packet.SourcePort, packet.SourceChannel) { - return im.Module.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) + return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) } ack := new(types.IncentivizedAcknowledgement) @@ -245,7 +244,7 @@ func (im IBCMiddleware) OnAcknowledgementPacket( } // call underlying callback - return im.Module.OnAcknowledgementPacket(ctx, packet, ack.Result, relayer) + return im.app.OnAcknowledgementPacket(ctx, packet, ack.Result, relayer) } // OnTimeoutPacket implements the IBCModule interface @@ -256,7 +255,7 @@ func (im IBCMiddleware) OnTimeoutPacket( relayer sdk.AccAddress, ) error { if !im.keeper.IsFeeEnabled(ctx, packet.SourcePort, packet.SourceChannel) { - return im.Module.OnTimeoutPacket(ctx, packet, relayer) + return im.app.OnTimeoutPacket(ctx, packet, relayer) } packetID := channeltypes.NewPacketId(packet.SourceChannel, packet.SourcePort, packet.Sequence) @@ -269,7 +268,7 @@ func (im IBCMiddleware) OnTimeoutPacket( } // call underlying callback - return im.Module.OnTimeoutPacket(ctx, packet, relayer) + return im.app.OnTimeoutPacket(ctx, packet, relayer) } // SendPacket implements the ICS4 Wrapper interface diff --git a/testing/mock/module.go b/testing/mock/module.go deleted file mode 100644 index f44a46d2b65..00000000000 --- a/testing/mock/module.go +++ /dev/null @@ -1,126 +0,0 @@ -package mock - -import ( - sdk "github.com/cosmos/cosmos-sdk/types" - capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" - channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" - porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types" - "github.com/cosmos/ibc-go/v3/modules/core/exported" -) - -var _ porttypes.IBCModule = &Module{} - -// Module a concrete type for a module boilerplate. -type Module struct { - app porttypes.IBCModule -} - -// NewModule creates a new IBC Module boilerplate given the underlying IBC app -func NewModule(app porttypes.IBCModule) *Module { - return &Module{ - app: app, - } -} - -// OnChanOpenInit implements the Module interface -// It calls the underlying app's OnChanOpenInit callback. -func (im Module) OnChanOpenInit( - ctx sdk.Context, - order channeltypes.Order, - connectionHops []string, - portID string, - channelID string, - chanCap *capabilitytypes.Capability, - counterparty channeltypes.Counterparty, - version string, -) error { - return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version) -} - -// OnChanOpenTry implements the Module interface. -// It calls the underlying app's OnChanOpenTry callback. -func (im Module) OnChanOpenTry( - ctx sdk.Context, - order channeltypes.Order, - connectionHops []string, - portID, - channelID string, - chanCap *capabilitytypes.Capability, - counterparty channeltypes.Counterparty, - counterpartyVersion string, -) (version string, err error) { - return im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, counterpartyVersion) -} - -// OnChanOpenAck implements the Module interface. -// It calls the underlying app's OnChanOpenAck callback. -func (im Module) OnChanOpenAck( - ctx sdk.Context, - portID, - channelID, - counterpartyChannelID, - counterpartyVersion string, -) error { - return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion) -} - -// OnChanOpenConfirm implements the Module interface. -// It calls the underlying app's OnChanOpenConfirm callback. -func (im Module) OnChanOpenConfirm( - ctx sdk.Context, - portID, - channelID string, -) error { - return im.app.OnChanOpenConfirm(ctx, portID, channelID) -} - -// OnChanCloseInit implements the Module interface -// It calls the underlying app's OnChanCloseInit callback. -func (im Module) OnChanCloseInit( - ctx sdk.Context, - portID, - channelID string, -) error { - return im.app.OnChanCloseInit(ctx, portID, channelID) -} - -// OnChanCloseConfirm implements the Module interface. -// It calls the underlying app's OnChanCloseConfirm callback. -func (im Module) OnChanCloseConfirm( - ctx sdk.Context, - portID, - channelID string, -) error { - return im.app.OnChanCloseConfirm(ctx, portID, channelID) -} - -// OnRecvPacket implements the Module interface. -// It calls the underlying app's OnRecvPacket callback. -func (im Module) OnRecvPacket( - ctx sdk.Context, - packet channeltypes.Packet, - relayer sdk.AccAddress, -) exported.Acknowledgement { - return im.app.OnRecvPacket(ctx, packet, relayer) -} - -// OnAcknowledgementPacket implements the Module interface. -// It calls the underlying app's OnAcknowledgementPacket callback. -func (im Module) OnAcknowledgementPacket( - ctx sdk.Context, - packet channeltypes.Packet, - acknowledgement []byte, - relayer sdk.AccAddress, -) error { - return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer) -} - -// OnTimeoutPacket implements the Module interface. -// It calls the underlying app's OnTimeoutPacket callback. -func (im Module) OnTimeoutPacket( - ctx sdk.Context, - packet channeltypes.Packet, - relayer sdk.AccAddress, -) error { - return im.app.OnTimeoutPacket(ctx, packet, relayer) -} From 5852bdf2a102a8a1a4befab3898da0146bab3730 Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 13 Apr 2022 10:06:49 +0200 Subject: [PATCH 04/18] refactor: ica module stack setup --- .../controller/ibc_module.go | 71 ++++++++++++------- modules/apps/27-interchain-accounts/module.go | 2 - modules/apps/29-fee/ibc_module.go | 18 ++--- testing/mock/ibc_module.go | 9 +++ testing/simapp/app.go | 40 ++++++----- 5 files changed, 86 insertions(+), 54 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_module.go b/modules/apps/27-interchain-accounts/controller/ibc_module.go index c00c9f5d1c2..ce5b0c730f5 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_module.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_module.go @@ -10,30 +10,34 @@ import ( icatypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types" + "github.com/cosmos/ibc-go/v3/modules/core/exported" 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{} + +// IBCMiddlware 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{ +// IBCMiddleware creates a new IBCMiddleware given the associated keeper and underlying application +func NewIBCMiddleware(k keeper.Keeper, app porttypes.IBCModule) IBCMiddleware { + return IBCMiddleware{ keeper: k, app: app, } } -// 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 +60,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 +74,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,8 +99,8 @@ 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, @@ -104,8 +108,8 @@ func (im IBCModule) OnChanOpenConfirm( 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,8 +118,8 @@ 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, @@ -123,8 +127,8 @@ func (im IBCModule) OnChanCloseConfirm( 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, @@ -132,8 +136,8 @@ func (im IBCModule) OnRecvPacket( 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 +151,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, @@ -163,3 +167,22 @@ 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 nil +} + +// WriteAcknowledgement implements the ICS4 Wrapper interface +func (im IBCMiddleware) WriteAcknowledgement( + ctx sdk.Context, + chanCap *capabilitytypes.Capability, + packet exported.PacketI, + ack exported.Acknowledgement, +) error { + return nil +} diff --git a/modules/apps/27-interchain-accounts/module.go b/modules/apps/27-interchain-accounts/module.go index 1bb870fca5d..c7a5651baa6 100644 --- 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 b/modules/apps/29-fee/ibc_module.go index 80c1435ea4b..2af28ad0e9e 100644 --- a/modules/apps/29-fee/ibc_module.go +++ b/modules/apps/29-fee/ibc_module.go @@ -29,7 +29,7 @@ func NewIBCMiddleware(app porttypes.IBCModule, k keeper.Keeper) IBCMiddleware { } } -// OnChanOpenInit implements the IBCModule interface +// OnChanOpenInit implements the IBCMiddleware interface func (im IBCMiddleware) OnChanOpenInit( ctx sdk.Context, order channeltypes.Order, @@ -60,7 +60,7 @@ func (im IBCMiddleware) 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 IBCMiddleware) OnChanOpenTry( @@ -103,7 +103,7 @@ func (im IBCMiddleware) OnChanOpenTry( return string(versionBytes), nil } -// OnChanOpenAck implements the IBCModule interface +// OnChanOpenAck implements the IBCMiddleware interface func (im IBCMiddleware) OnChanOpenAck( ctx sdk.Context, portID, @@ -131,7 +131,7 @@ func (im IBCMiddleware) OnChanOpenAck( return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion) } -// OnChanOpenConfirm implements the IBCModule interface +// OnChanOpenConfirm implements the IBCMiddleware interface func (im IBCMiddleware) OnChanOpenConfirm( ctx sdk.Context, portID, @@ -141,7 +141,7 @@ func (im IBCMiddleware) OnChanOpenConfirm( return im.app.OnChanOpenConfirm(ctx, portID, channelID) } -// OnChanCloseInit implements the IBCModule interface +// OnChanCloseInit implements the IBCMiddleware interface func (im IBCMiddleware) OnChanCloseInit( ctx sdk.Context, portID, @@ -169,7 +169,7 @@ func (im IBCMiddleware) OnChanCloseInit( return nil } -// OnChanCloseConfirm implements the IBCModule interface +// OnChanCloseConfirm implements the IBCMiddleware interface func (im IBCMiddleware) OnChanCloseConfirm( ctx sdk.Context, portID, @@ -192,7 +192,7 @@ func (im IBCMiddleware) OnChanCloseConfirm( return im.app.OnChanCloseConfirm(ctx, portID, channelID) } -// 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 IBCMiddleware) OnRecvPacket( ctx sdk.Context, @@ -217,7 +217,7 @@ func (im IBCMiddleware) 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 IBCMiddleware) OnAcknowledgementPacket( ctx sdk.Context, @@ -247,7 +247,7 @@ func (im IBCMiddleware) 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 IBCMiddleware) OnTimeoutPacket( ctx sdk.Context, diff --git a/testing/mock/ibc_module.go b/testing/mock/ibc_module.go index e58f6ae7156..73f40e9678f 100644 --- a/testing/mock/ibc_module.go +++ b/testing/mock/ibc_module.go @@ -154,6 +154,15 @@ func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, return nil } +// SendPacket implements the ICS4 Wrapper interface +func (im IBCModule) SendPacket( + ctx sdk.Context, + chanCap *capabilitytypes.Capability, + packet exported.PacketI, +) error { + return nil +} + // GetMockRecvCanaryCapabilityName generates a capability name for testing OnRecvPacket functionality. func GetMockRecvCanaryCapabilityName(packet channeltypes.Packet) string { return fmt.Sprintf("%s%s%s%s", MockRecvCanaryCapabilityName, packet.GetDestPort(), packet.GetDestChannel(), strconv.Itoa(int(packet.GetSequence()))) diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 5b63b954d06..9e291726cf3 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -353,6 +353,7 @@ func NewSimApp( &stakingKeeper, govRouter, ) + // Fee Module keeper app.IBCFeeKeeper = ibcfeekeeper.NewKeeper(appCodec, keys[ibcfeetypes.StoreKey], app.GetSubspace(ibcfeetypes.ModuleName), app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, app.AccountKeeper, app.BankKeeper, ) @@ -379,19 +380,9 @@ func NewSimApp( transferStack = transfer.NewIBCModule(app.TransferKeeper) transferStack = ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper) - // TODO: clean up remaining modules - // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do - // not replicate if you do not need to test core IBC or light clients. - mockModule := ibcmock.NewAppModule(&app.IBCKeeper.PortKeeper) - - // create fee wrapped mock module - feeMockModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp(MockFeePort, scopedFeeMockKeeper)) - app.FeeMockModule = feeMockModule - - feeWithMockModule := ibcfee.NewIBCMiddleware(feeMockModule, app.IBCFeeKeeper) - - mockIBCModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp(ibcmock.ModuleName, scopedIBCMockKeeper)) + // Create Interchain Accounts Stack + // Controller app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper( appCodec, keys[icacontrollertypes.StoreKey], app.GetSubspace(icacontrollertypes.SubModuleName), app.IBCKeeper.ChannelKeeper, // may be replaced with middleware such as ics29 fee @@ -399,27 +390,38 @@ func NewSimApp( scopedICAControllerKeeper, app.MsgServiceRouter(), ) + // Host app.ICAHostKeeper = icahostkeeper.NewKeeper( appCodec, keys[icahosttypes.StoreKey], app.GetSubspace(icahosttypes.SubModuleName), app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, app.AccountKeeper, scopedICAHostKeeper, app.MsgServiceRouter(), ) - icaModule := ica.NewAppModule(&app.ICAControllerKeeper, &app.ICAHostKeeper) + // Mock Module setup for testing IBC + // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do + // not replicate if you do not need to test core IBC or light clients. + mockModule := ibcmock.NewAppModule(&app.IBCKeeper.PortKeeper) // initialize ICA module with mock module as the authentication module on the controller side - icaAuthModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp("", scopedICAMockKeeper)) - app.ICAAuthModule = icaAuthModule + icaControllerStack := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp("", scopedICAMockKeeper)) + app.ICAAuthModule = icaControllerStack + icaControllerStack = icacontroller.NewIBCMiddleware(app.ICAControllerKeeper, icaControllerStack) - icaControllerIBCModule := icacontroller.NewIBCModule(app.ICAControllerKeeper, icaAuthModule) icaHostIBCModule := icahost.NewIBCModule(app.ICAHostKeeper) + // create fee wrapped mock module + feeMockModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp(MockFeePort, scopedFeeMockKeeper)) + app.FeeMockModule = feeMockModule + feeWithMockModule := ibcfee.NewIBCMiddleware(feeMockModule, app.IBCFeeKeeper) + + mockIBCModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp(ibcmock.ModuleName, scopedIBCMockKeeper)) + // Create static IBC router, add app routes, then set and seal it // pass in top-level (fully-wrapped) IBCModules to IBC Router ibcRouter := porttypes.NewRouter() - ibcRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerIBCModule). + ibcRouter. AddRoute(icahosttypes.SubModuleName, icaHostIBCModule). - AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerIBCModule). // ica with mock auth module stack route to ica (top level of middleware stack) + AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack). // ica with mock auth module stack route to ica (top level of middleware stack) AddRoute(ibctransfertypes.ModuleName, transferStack). AddRoute(ibcmock.ModuleName, mockIBCModule). AddRoute(MockFeePort, feeWithMockModule) @@ -467,7 +469,7 @@ func NewSimApp( // ibc modules transferModule, ibcfee.NewAppModule(app.IBCFeeKeeper), - icaModule, + ica.NewAppModule(&app.ICAControllerKeeper, &app.ICAHostKeeper), mockModule, ) From 94f3e23333a20be79425dc690f9f96563e1e08ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 13 Apr 2022 14:20:40 +0200 Subject: [PATCH 05/18] fix: define icaControllerStack as type porttypes.IBCModule --- testing/simapp/app.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 9e291726cf3..3efe9f91837 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -403,8 +403,9 @@ func NewSimApp( mockModule := ibcmock.NewAppModule(&app.IBCKeeper.PortKeeper) // initialize ICA module with mock module as the authentication module on the controller side - icaControllerStack := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp("", scopedICAMockKeeper)) - app.ICAAuthModule = icaControllerStack + var icaControllerStack porttypes.IBCModule + icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp("", scopedICAMockKeeper)) + app.ICAAuthModule = icaControllerStack.(ibcmock.IBCModule) icaControllerStack = icacontroller.NewIBCMiddleware(app.ICAControllerKeeper, icaControllerStack) icaHostIBCModule := icahost.NewIBCModule(app.ICAHostKeeper) From e0e405e4c1bb6acc195de21248440da4230c59b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 13 Apr 2022 14:27:09 +0200 Subject: [PATCH 06/18] add back controller routing for port --- testing/simapp/app.go | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 3efe9f91837..7abda467897 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -421,6 +421,7 @@ func NewSimApp( // pass in top-level (fully-wrapped) IBCModules to IBC Router ibcRouter := porttypes.NewRouter() ibcRouter. + AddRoute(icacontrollertypes.SubModuleName, icaControllerStack). AddRoute(icahosttypes.SubModuleName, icaHostIBCModule). AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack). // ica with mock auth module stack route to ica (top level of middleware stack) AddRoute(ibctransfertypes.ModuleName, transferStack). From fd2360fcc5a01205c4a79dc226490a1c13f4f301 Mon Sep 17 00:00:00 2001 From: Sean King Date: Thu, 14 Apr 2022 14:34:44 +0200 Subject: [PATCH 07/18] refactor: remove unncessary SendPacket --- testing/mock/ibc_module.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/testing/mock/ibc_module.go b/testing/mock/ibc_module.go index 73f40e9678f..e58f6ae7156 100644 --- a/testing/mock/ibc_module.go +++ b/testing/mock/ibc_module.go @@ -154,15 +154,6 @@ func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, return nil } -// SendPacket implements the ICS4 Wrapper interface -func (im IBCModule) SendPacket( - ctx sdk.Context, - chanCap *capabilitytypes.Capability, - packet exported.PacketI, -) error { - return nil -} - // GetMockRecvCanaryCapabilityName generates a capability name for testing OnRecvPacket functionality. func GetMockRecvCanaryCapabilityName(packet channeltypes.Packet) string { return fmt.Sprintf("%s%s%s%s", MockRecvCanaryCapabilityName, packet.GetDestPort(), packet.GetDestChannel(), strconv.Itoa(int(packet.GetSequence()))) From 2793d737b348a74d6d989fb6024d1c5189c2b45a Mon Sep 17 00:00:00 2001 From: Sean King Date: Thu, 14 Apr 2022 14:39:58 +0200 Subject: [PATCH 08/18] refactor: panic instead of return nil --- modules/apps/27-interchain-accounts/controller/ibc_module.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_module.go b/modules/apps/27-interchain-accounts/controller/ibc_module.go index ce5b0c730f5..49b60144669 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_module.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_module.go @@ -174,7 +174,7 @@ func (im IBCMiddleware) SendPacket( chanCap *capabilitytypes.Capability, packet exported.PacketI, ) error { - return nil + panic("SendPacket not supported for ICA controller module") } // WriteAcknowledgement implements the ICS4 Wrapper interface @@ -184,5 +184,5 @@ func (im IBCMiddleware) WriteAcknowledgement( packet exported.PacketI, ack exported.Acknowledgement, ) error { - return nil + panic("WriteAcknowledgement not supported for ICA controller module") } From b419f8fdec8865875bf38e9ff11a7af181f3c970 Mon Sep 17 00:00:00 2001 From: Sean King Date: Thu, 14 Apr 2022 15:01:50 +0200 Subject: [PATCH 09/18] refactor: changing structure to init all keepers, then stacks & routing --- testing/simapp/app.go | 68 +++++++++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 28 deletions(-) diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 7abda467897..fcbdf290ad0 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -300,7 +300,8 @@ func NewSimApp( // seal capability keeper after scoping modules app.CapabilityKeeper.Seal() - // add keepers + // SDK module keepers + app.AccountKeeper = authkeeper.NewAccountKeeper( appCodec, keys[authtypes.StoreKey], app.GetSubspace(authtypes.ModuleName), authtypes.ProtoBaseAccount, maccPerms, ) @@ -334,13 +335,14 @@ func NewSimApp( stakingtypes.NewMultiStakingHooks(app.DistrKeeper.Hooks(), app.SlashingKeeper.Hooks()), ) - // Create IBC Keeper + app.AuthzKeeper = authzkeeper.NewKeeper(keys[authzkeeper.StoreKey], appCodec, app.BaseApp.MsgServiceRouter()) + + // IBC Keepers + app.IBCKeeper = ibckeeper.NewKeeper( appCodec, keys[ibchost.StoreKey], app.GetSubspace(ibchost.ModuleName), app.StakingKeeper, app.UpgradeKeeper, scopedIBCKeeper, ) - app.AuthzKeeper = authzkeeper.NewKeeper(keys[authzkeeper.StoreKey], appCodec, app.BaseApp.MsgServiceRouter()) - // register the proposal types govRouter := govtypes.NewRouter() govRouter.AddRoute(govtypes.RouterKey, govtypes.ProposalHandler). @@ -353,12 +355,30 @@ func NewSimApp( &stakingKeeper, govRouter, ) - // Fee Module keeper + // IBC Fee Module keeper app.IBCFeeKeeper = ibcfeekeeper.NewKeeper(appCodec, keys[ibcfeetypes.StoreKey], app.GetSubspace(ibcfeetypes.ModuleName), app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, app.AccountKeeper, app.BankKeeper, ) - // Create Transfer Stack + // ICA Controller keeper + app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper( + appCodec, keys[icacontrollertypes.StoreKey], app.GetSubspace(icacontrollertypes.SubModuleName), + app.IBCKeeper.ChannelKeeper, // may be replaced with middleware such as ics29 fee + app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, + scopedICAControllerKeeper, app.MsgServiceRouter(), + ) + + // ICA Host keeper + app.ICAHostKeeper = icahostkeeper.NewKeeper( + appCodec, keys[icahosttypes.StoreKey], app.GetSubspace(icahosttypes.SubModuleName), + app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, + app.AccountKeeper, scopedICAHostKeeper, app.MsgServiceRouter(), + ) + + // Create IBC Router + ibcRouter := porttypes.NewRouter() + + // Middleware Stacks // Create Transfer Keeper and pass IBCFeeKeeper as expected Channel and PortKeeper // since fee middleware will wrap the IBCKeeper for underlying application. @@ -369,6 +389,7 @@ func NewSimApp( app.AccountKeeper, app.BankKeeper, scopedTransferKeeper, ) + // Create Transfer Stack transferModule := transfer.NewAppModule(app.TransferKeeper) // transfer stack contains (from top to bottom): @@ -380,22 +401,11 @@ func NewSimApp( transferStack = transfer.NewIBCModule(app.TransferKeeper) transferStack = ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper) - // Create Interchain Accounts Stack - - // Controller - app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper( - appCodec, keys[icacontrollertypes.StoreKey], app.GetSubspace(icacontrollertypes.SubModuleName), - app.IBCKeeper.ChannelKeeper, // may be replaced with middleware such as ics29 fee - app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, - scopedICAControllerKeeper, app.MsgServiceRouter(), - ) + // Add transfer stack to IBC Router + ibcRouter. + AddRoute(ibctransfertypes.ModuleName, transferStack) - // Host - app.ICAHostKeeper = icahostkeeper.NewKeeper( - appCodec, keys[icahosttypes.StoreKey], app.GetSubspace(icahosttypes.SubModuleName), - app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, - app.AccountKeeper, scopedICAHostKeeper, app.MsgServiceRouter(), - ) + // Create Interchain Accounts Stack // Mock Module setup for testing IBC // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do @@ -410,6 +420,13 @@ func NewSimApp( icaHostIBCModule := icahost.NewIBCModule(app.ICAHostKeeper) + // Add host, controller & ica auth modules to IBC router + ibcRouter. + AddRoute(icacontrollertypes.SubModuleName, icaControllerStack). + AddRoute(icahosttypes.SubModuleName, icaHostIBCModule). + AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) // ica with mock auth module stack route to ica (top level of middleware stack) + + // Create Mock IBC Fee module stack for testing // create fee wrapped mock module feeMockModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp(MockFeePort, scopedFeeMockKeeper)) app.FeeMockModule = feeMockModule @@ -417,17 +434,12 @@ func NewSimApp( mockIBCModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp(ibcmock.ModuleName, scopedIBCMockKeeper)) - // Create static IBC router, add app routes, then set and seal it - // pass in top-level (fully-wrapped) IBCModules to IBC Router - ibcRouter := porttypes.NewRouter() + // Add Mock Module & Mock Fee Module to IBC Router ibcRouter. - AddRoute(icacontrollertypes.SubModuleName, icaControllerStack). - AddRoute(icahosttypes.SubModuleName, icaHostIBCModule). - AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack). // ica with mock auth module stack route to ica (top level of middleware stack) - AddRoute(ibctransfertypes.ModuleName, transferStack). AddRoute(ibcmock.ModuleName, mockIBCModule). AddRoute(MockFeePort, feeWithMockModule) + // Seal the IBC Router app.IBCKeeper.SetRouter(ibcRouter) // create evidence keeper with router From 4a60ff6eea095b0367f2c9fdce74f0b0fbd59864 Mon Sep 17 00:00:00 2001 From: Sean King Date: Thu, 14 Apr 2022 15:09:13 +0200 Subject: [PATCH 10/18] docs: add comments --- testing/simapp/app.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/testing/simapp/app.go b/testing/simapp/app.go index fcbdf290ad0..723de9cff79 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -390,6 +390,12 @@ func NewSimApp( ) // Create Transfer Stack + // SendPacket, since it is originating from the application to core IBC: + // transferKeeper.SendPacket -> fee.SendPacket -> channel.SendPacket + + // RecvPacket, message that originates from core IBC and goes down to app, the flow is the otherway + // channel.RecvPacket -> fee.OnRecvPacket -> transfer.OnRecvPacket + transferModule := transfer.NewAppModule(app.TransferKeeper) // transfer stack contains (from top to bottom): @@ -406,8 +412,10 @@ func NewSimApp( AddRoute(ibctransfertypes.ModuleName, transferStack) // Create Interchain Accounts Stack + // SendPacket, since it is originating from the application to core IBC: + // icaAuthModuleKeeper.SendPacket -> icaControllerKeeper.SendPacket -> channel.SendPacket - // Mock Module setup for testing IBC + // Mock Module setup for testing IBC and acts as the interchain accounts authentication module // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do // not replicate if you do not need to test core IBC or light clients. mockModule := ibcmock.NewAppModule(&app.IBCKeeper.PortKeeper) @@ -422,6 +430,9 @@ func NewSimApp( // Add host, controller & ica auth modules to IBC router ibcRouter. + // the ICA Controller middleware needs to be explicitly added to the IBC Router because the + // ICA controller module owns the port capability for ICA. The ICA authentication module + // owns the channel capability. AddRoute(icacontrollertypes.SubModuleName, icaControllerStack). AddRoute(icahosttypes.SubModuleName, icaHostIBCModule). AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) // ica with mock auth module stack route to ica (top level of middleware stack) From 31c3f93d1c0f41bfacfb026c628262df7fbae7fc Mon Sep 17 00:00:00 2001 From: Sean King Date: Thu, 14 Apr 2022 15:11:41 +0200 Subject: [PATCH 11/18] nit: comment --- testing/simapp/app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 723de9cff79..9872de1fe8e 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -491,7 +491,7 @@ func NewSimApp( params.NewAppModule(app.ParamsKeeper), authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AccountKeeper, app.BankKeeper, app.interfaceRegistry), - // ibc modules + // IBC modules transferModule, ibcfee.NewAppModule(app.IBCFeeKeeper), ica.NewAppModule(&app.ICAControllerKeeper, &app.ICAHostKeeper), From 40fbdcc594ef0c63f8599d991b0e9008764c28de Mon Sep 17 00:00:00 2001 From: Sean King Date: Tue, 19 Apr 2022 17:22:44 +0200 Subject: [PATCH 12/18] chore: rename files --- .../controller/{ibc_module.go => ibc_middleware.go} | 0 .../controller/{ibc_module_test.go => ibc_middleware_test.go} | 0 modules/apps/29-fee/{ibc_module.go => ibc_middleware.go} | 0 .../apps/29-fee/{ibc_module_test.go => ibc_middleware_test.go} | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename modules/apps/27-interchain-accounts/controller/{ibc_module.go => ibc_middleware.go} (100%) rename modules/apps/27-interchain-accounts/controller/{ibc_module_test.go => ibc_middleware_test.go} (100%) rename modules/apps/29-fee/{ibc_module.go => ibc_middleware.go} (100%) rename modules/apps/29-fee/{ibc_module_test.go => ibc_middleware_test.go} (100%) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_module.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go similarity index 100% rename from modules/apps/27-interchain-accounts/controller/ibc_module.go rename to modules/apps/27-interchain-accounts/controller/ibc_middleware.go diff --git a/modules/apps/27-interchain-accounts/controller/ibc_module_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go similarity index 100% rename from modules/apps/27-interchain-accounts/controller/ibc_module_test.go rename to modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go diff --git a/modules/apps/29-fee/ibc_module.go b/modules/apps/29-fee/ibc_middleware.go similarity index 100% rename from modules/apps/29-fee/ibc_module.go rename to modules/apps/29-fee/ibc_middleware.go diff --git a/modules/apps/29-fee/ibc_module_test.go b/modules/apps/29-fee/ibc_middleware_test.go similarity index 100% rename from modules/apps/29-fee/ibc_module_test.go rename to modules/apps/29-fee/ibc_middleware_test.go From 68f4843de757bcdc3c798239e51e855403ff6ce2 Mon Sep 17 00:00:00 2001 From: Sean King Date: Tue, 19 Apr 2022 17:31:48 +0200 Subject: [PATCH 13/18] chore: comment + nit --- .../controller/ibc_middleware.go | 2 +- modules/apps/29-fee/ibc_middleware.go | 2 +- testing/simapp/app.go | 20 +++++++++++++------ 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index 49b60144669..f2050b9f010 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -16,7 +16,7 @@ import ( var _ porttypes.Middleware = &IBCMiddleware{} -// IBCMiddlware implements the ICS26 callbacks for the fee middleware given the +// IBCMiddleware implements the ICS26 callbacks for the fee middleware given the // ICA controller keeper and the underlying application. type IBCMiddleware struct { app porttypes.IBCModule diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go index e7c6994706f..c6f65682a34 100644 --- a/modules/apps/29-fee/ibc_middleware.go +++ b/modules/apps/29-fee/ibc_middleware.go @@ -14,7 +14,7 @@ import ( var _ porttypes.Middleware = &IBCMiddleware{} -// IBCMiddlware implements the ICS26 callbacks for the fee middleware given the +// IBCMiddleware implements the ICS26 callbacks for the fee middleware given the // fee keeper and the underlying application. type IBCMiddleware struct { app porttypes.IBCModule diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 9872de1fe8e..f073c658ffb 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -408,18 +408,17 @@ func NewSimApp( transferStack = ibcfee.NewIBCMiddleware(transferStack, app.IBCFeeKeeper) // Add transfer stack to IBC Router - ibcRouter. - AddRoute(ibctransfertypes.ModuleName, transferStack) - - // Create Interchain Accounts Stack - // SendPacket, since it is originating from the application to core IBC: - // icaAuthModuleKeeper.SendPacket -> icaControllerKeeper.SendPacket -> channel.SendPacket + ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack) // Mock Module setup for testing IBC and acts as the interchain accounts authentication module // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do // not replicate if you do not need to test core IBC or light clients. mockModule := ibcmock.NewAppModule(&app.IBCKeeper.PortKeeper) + // Create Interchain Accounts Stack + // SendPacket, since it is originating from the application to core IBC: + // icaAuthModuleKeeper.SendPacket -> icaControllerKeeper.SendPacket -> channel.SendPacket + // initialize ICA module with mock module as the authentication module on the controller side var icaControllerStack porttypes.IBCModule icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp("", scopedICAMockKeeper)) @@ -438,6 +437,15 @@ func NewSimApp( AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) // ica with mock auth module stack route to ica (top level of middleware stack) // Create Mock IBC Fee module stack for testing + // OnAckPacket as this is where fee's are paid out + // mockModule.OnAcknowledgementPacket -> fee.OnAcknowledgementPacket -> channel.OnAcknowledgementPacket + + // RecvPacket, message that originates from core IBC and goes down to app, the flow is the otherway + // channel.RecvPacket -> fee.OnRecvPacket -> mockModule.OnRecvPacket + + // SendPacket, since it is originating from the application to core IBC: + // mockModule.SendPacket -> fee.SendPacket -> channel.SendPacket + // create fee wrapped mock module feeMockModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp(MockFeePort, scopedFeeMockKeeper)) app.FeeMockModule = feeMockModule From 6624b73478a236b11f3dec70ce0f0e0c69d5816f Mon Sep 17 00:00:00 2001 From: Sean King Date: Tue, 19 Apr 2022 18:25:21 +0200 Subject: [PATCH 14/18] refactor: stacks --- .../controller/ibc_middleware.go | 4 +- testing/simapp/app.go | 45 ++++++++++--------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index f2050b9f010..24e602227d4 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -24,10 +24,10 @@ type IBCMiddleware struct { } // IBCMiddleware creates a new IBCMiddleware given the associated keeper and underlying application -func NewIBCMiddleware(k keeper.Keeper, app porttypes.IBCModule) IBCMiddleware { +func NewIBCMiddleware(app porttypes.IBCModule, k keeper.Keeper) IBCMiddleware { return IBCMiddleware{ - keeper: k, app: app, + keeper: k, } } diff --git a/testing/simapp/app.go b/testing/simapp/app.go index f073c658ffb..1f088bd0683 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -356,8 +356,11 @@ func NewSimApp( ) // IBC Fee Module keeper - app.IBCFeeKeeper = ibcfeekeeper.NewKeeper(appCodec, keys[ibcfeetypes.StoreKey], app.GetSubspace(ibcfeetypes.ModuleName), - app.IBCKeeper.ChannelKeeper, app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, app.AccountKeeper, app.BankKeeper, + app.IBCFeeKeeper = ibcfeekeeper.NewKeeper( + appCodec, keys[ibcfeetypes.StoreKey], app.GetSubspace(ibcfeetypes.ModuleName), + app.IBCKeeper.ChannelKeeper, // may be replaced with IBC middleware + app.IBCKeeper.ChannelKeeper, + &app.IBCKeeper.PortKeeper, app.AccountKeeper, app.BankKeeper, ) // ICA Controller keeper @@ -389,6 +392,17 @@ func NewSimApp( app.AccountKeeper, app.BankKeeper, scopedTransferKeeper, ) + // Mock Module Stack + + // Mock Module setup for testing IBC and also acts as the interchain accounts authentication module + // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do + // not replicate if you do not need to test core IBC or light clients. + mockModule := ibcmock.NewAppModule(&app.IBCKeeper.PortKeeper) + + // The mock module is used for testing IBC + mockIBCModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp(ibcmock.ModuleName, scopedIBCMockKeeper)) + ibcRouter.AddRoute(ibcmock.ModuleName, mockIBCModule) + // Create Transfer Stack // SendPacket, since it is originating from the application to core IBC: // transferKeeper.SendPacket -> fee.SendPacket -> channel.SendPacket @@ -396,8 +410,6 @@ func NewSimApp( // RecvPacket, message that originates from core IBC and goes down to app, the flow is the otherway // channel.RecvPacket -> fee.OnRecvPacket -> transfer.OnRecvPacket - transferModule := transfer.NewAppModule(app.TransferKeeper) - // transfer stack contains (from top to bottom): // - IBC Fee Middleware // - Transfer @@ -410,20 +422,19 @@ func NewSimApp( // Add transfer stack to IBC Router ibcRouter.AddRoute(ibctransfertypes.ModuleName, transferStack) - // Mock Module setup for testing IBC and acts as the interchain accounts authentication module - // NOTE: the IBC mock keeper and application module is used only for testing core IBC. Do - // not replicate if you do not need to test core IBC or light clients. - mockModule := ibcmock.NewAppModule(&app.IBCKeeper.PortKeeper) - // Create Interchain Accounts Stack // SendPacket, since it is originating from the application to core IBC: - // icaAuthModuleKeeper.SendPacket -> icaControllerKeeper.SendPacket -> channel.SendPacket + // icaAuthModuleKeeper.SendPacket -> fee.SendPacket -> icaControllerKeeper.SendPacket -> channel.SendPacket + + // OnAckPacket as this is where fee's are paid out + // mockModule.OnAcknowledgementPacket -> fee.OnAcknowledgementPacket -> channel.OnAcknowledgementPacket // initialize ICA module with mock module as the authentication module on the controller side var icaControllerStack porttypes.IBCModule icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp("", scopedICAMockKeeper)) app.ICAAuthModule = icaControllerStack.(ibcmock.IBCModule) - icaControllerStack = icacontroller.NewIBCMiddleware(app.ICAControllerKeeper, icaControllerStack) + icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper) + icaControllerStack = icacontroller.NewIBCMiddleware(icaControllerStack, app.ICAControllerKeeper) icaHostIBCModule := icahost.NewIBCModule(app.ICAHostKeeper) @@ -450,13 +461,7 @@ func NewSimApp( feeMockModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp(MockFeePort, scopedFeeMockKeeper)) app.FeeMockModule = feeMockModule feeWithMockModule := ibcfee.NewIBCMiddleware(feeMockModule, app.IBCFeeKeeper) - - mockIBCModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp(ibcmock.ModuleName, scopedIBCMockKeeper)) - - // Add Mock Module & Mock Fee Module to IBC Router - ibcRouter. - AddRoute(ibcmock.ModuleName, mockIBCModule). - AddRoute(MockFeePort, feeWithMockModule) + ibcRouter.AddRoute(MockFeePort, feeWithMockModule) // Seal the IBC Router app.IBCKeeper.SetRouter(ibcRouter) @@ -500,7 +505,7 @@ func NewSimApp( authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AccountKeeper, app.BankKeeper, app.interfaceRegistry), // IBC modules - transferModule, + transfer.NewAppModule(app.TransferKeeper), ibcfee.NewAppModule(app.IBCFeeKeeper), ica.NewAppModule(&app.ICAControllerKeeper, &app.ICAHostKeeper), mockModule, @@ -562,7 +567,7 @@ func NewSimApp( evidence.NewAppModule(app.EvidenceKeeper), authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AccountKeeper, app.BankKeeper, app.interfaceRegistry), ibc.NewAppModule(app.IBCKeeper), - transferModule, + transfer.NewAppModule(app.TransferKeeper), ) app.sm.RegisterStoreDecoders() From 611c5db45ff5c29b98bb29655d329bc52c796207 Mon Sep 17 00:00:00 2001 From: Sean King Date: Tue, 26 Apr 2022 19:32:16 +0200 Subject: [PATCH 15/18] Update modules/apps/27-interchain-accounts/controller/ibc_middleware.go Co-authored-by: Aditya --- .../apps/27-interchain-accounts/controller/ibc_middleware.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index 24e602227d4..cc7ab2c3b4c 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -174,7 +174,7 @@ func (im IBCMiddleware) SendPacket( chanCap *capabilitytypes.Capability, packet exported.PacketI, ) error { - panic("SendPacket not supported for ICA controller module") + panic("SendPacket not supported for ICA-auth module. Please use SendTx") } // WriteAcknowledgement implements the ICS4 Wrapper interface From 5898bfc64905a4b858025609705fbefd141d8bee Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 27 Apr 2022 18:31:08 +0200 Subject: [PATCH 16/18] nits: nits --- .../controller/ibc_middleware.go | 4 ++-- modules/apps/29-fee/ibc_middleware.go | 2 +- testing/simapp/app.go | 16 ++++++---------- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index cc7ab2c3b4c..9ccfa339c4f 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -181,8 +181,8 @@ func (im IBCMiddleware) SendPacket( func (im IBCMiddleware) WriteAcknowledgement( ctx sdk.Context, chanCap *capabilitytypes.Capability, - packet exported.PacketI, - ack exported.Acknowledgement, + packet ibcexported.PacketI, + ack ibcexported.Acknowledgement, ) error { panic("WriteAcknowledgement not supported for ICA controller module") } diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go index c6f65682a34..c37e40bf555 100644 --- a/modules/apps/29-fee/ibc_middleware.go +++ b/modules/apps/29-fee/ibc_middleware.go @@ -21,7 +21,7 @@ type IBCMiddleware struct { keeper keeper.Keeper } -// NewIBCMiddlware creates a new IBCMiddlware given the keeper and underlying application +// NewIBCMiddleware creates a new IBCMiddlware given the keeper and underlying application func NewIBCMiddleware(app porttypes.IBCModule, k keeper.Keeper) IBCMiddleware { return IBCMiddleware{ app: app, diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 1f088bd0683..d30297b0e84 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -407,7 +407,7 @@ func NewSimApp( // SendPacket, since it is originating from the application to core IBC: // transferKeeper.SendPacket -> fee.SendPacket -> channel.SendPacket - // RecvPacket, message that originates from core IBC and goes down to app, the flow is the otherway + // RecvPacket, message that originates from core IBC and goes down to app, the flow is the other way // channel.RecvPacket -> fee.OnRecvPacket -> transfer.OnRecvPacket // transfer stack contains (from top to bottom): @@ -426,14 +426,10 @@ func NewSimApp( // SendPacket, since it is originating from the application to core IBC: // icaAuthModuleKeeper.SendPacket -> fee.SendPacket -> icaControllerKeeper.SendPacket -> channel.SendPacket - // OnAckPacket as this is where fee's are paid out - // mockModule.OnAcknowledgementPacket -> fee.OnAcknowledgementPacket -> channel.OnAcknowledgementPacket - // initialize ICA module with mock module as the authentication module on the controller side var icaControllerStack porttypes.IBCModule icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp("", scopedICAMockKeeper)) app.ICAAuthModule = icaControllerStack.(ibcmock.IBCModule) - icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper) icaControllerStack = icacontroller.NewIBCMiddleware(icaControllerStack, app.ICAControllerKeeper) icaHostIBCModule := icahost.NewIBCModule(app.ICAHostKeeper) @@ -448,14 +444,14 @@ func NewSimApp( AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) // ica with mock auth module stack route to ica (top level of middleware stack) // Create Mock IBC Fee module stack for testing - // OnAckPacket as this is where fee's are paid out - // mockModule.OnAcknowledgementPacket -> fee.OnAcknowledgementPacket -> channel.OnAcknowledgementPacket + // SendPacket, since it is originating from the application to core IBC: + // mockModule.SendPacket -> fee.SendPacket -> channel.SendPacket - // RecvPacket, message that originates from core IBC and goes down to app, the flow is the otherway + // OnRecvPacket, message that originates from core IBC and goes down to app, the flow is the otherway // channel.RecvPacket -> fee.OnRecvPacket -> mockModule.OnRecvPacket - // SendPacket, since it is originating from the application to core IBC: - // mockModule.SendPacket -> fee.SendPacket -> channel.SendPacket + // OnAcknowledgementPacket as this is where fee's are paid out + // mockModule.OnAcknowledgementPacket -> fee.OnAcknowledgementPacket -> channel.OnAcknowledgementPacket // create fee wrapped mock module feeMockModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp(MockFeePort, scopedFeeMockKeeper)) From 80816aaffacb79c031c2ec3c9e9b72cd5bbe9cf2 Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 27 Apr 2022 18:34:59 +0200 Subject: [PATCH 17/18] chore: comment --- .../apps/27-interchain-accounts/controller/ibc_middleware.go | 1 + testing/simapp/app.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index 9ccfa339c4f..80b5b5885f0 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -169,6 +169,7 @@ func (im IBCMiddleware) OnTimeoutPacket( } // 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, diff --git a/testing/simapp/app.go b/testing/simapp/app.go index d30297b0e84..f707ba6087b 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -424,7 +424,7 @@ func NewSimApp( // Create Interchain Accounts Stack // SendPacket, since it is originating from the application to core IBC: - // icaAuthModuleKeeper.SendPacket -> fee.SendPacket -> icaControllerKeeper.SendPacket -> channel.SendPacket + // icaAuthModuleKeeper.SendTx -> icaControllerKeeper.SendPacket -> channel.SendPacket // initialize ICA module with mock module as the authentication module on the controller side var icaControllerStack porttypes.IBCModule From ac937cb806f3212f6d02863d74930359f851210c Mon Sep 17 00:00:00 2001 From: Sean King Date: Thu, 28 Apr 2022 16:23:13 +0200 Subject: [PATCH 18/18] chore: chore --- .../apps/27-interchain-accounts/controller/ibc_middleware.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index ce3eb120407..c12f5d3e9ce 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -10,7 +10,6 @@ import ( icatypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types" - "github.com/cosmos/ibc-go/v3/modules/core/exported" ibcexported "github.com/cosmos/ibc-go/v3/modules/core/exported" ) @@ -173,7 +172,7 @@ func (im IBCMiddleware) OnTimeoutPacket( func (im IBCMiddleware) SendPacket( ctx sdk.Context, chanCap *capabilitytypes.Capability, - packet exported.PacketI, + packet ibcexported.PacketI, ) error { panic("SendPacket not supported for ICA-auth module. Please use SendTx") }