From 70e6a3a7bfe3d87bca67c5dac6fb2ebc3afe7001 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 9 Feb 2022 13:25:38 +0100 Subject: [PATCH 1/6] refactor: allow the mock module to be used multiple times as base ibc application in middleware stack --- testing/mock/ibc_app.go | 4 ++++ testing/mock/ibc_module.go | 22 +++++++++++----------- testing/mock/mock.go | 20 +++++++++++--------- testing/simapp/app.go | 12 +++++++++--- 4 files changed, 35 insertions(+), 23 deletions(-) diff --git a/testing/mock/ibc_app.go b/testing/mock/ibc_app.go index a3f2db3bc6d..32fcc23aae8 100644 --- a/testing/mock/ibc_app.go +++ b/testing/mock/ibc_app.go @@ -2,6 +2,7 @@ package mock import ( sdk "github.com/cosmos/cosmos-sdk/types" + capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" @@ -10,6 +11,9 @@ import ( // MockIBCApp contains IBC application module callbacks as defined in 05-port. type MockIBCApp struct { + PortID string + ScopedKeeper capabilitykeeper.ScopedKeeper + OnChanOpenInit func( ctx sdk.Context, order channeltypes.Order, diff --git a/testing/mock/ibc_module.go b/testing/mock/ibc_module.go index fb46864709b..1ea1d3850ad 100644 --- a/testing/mock/ibc_module.go +++ b/testing/mock/ibc_module.go @@ -6,7 +6,6 @@ import ( "strconv" sdk "github.com/cosmos/cosmos-sdk/types" - capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" @@ -16,15 +15,16 @@ import ( // IBCModule implements the ICS26 callbacks for testing/mock. type IBCModule struct { - IBCApp *MockIBCApp // base application of an IBC middleware stack - scopedKeeper capabilitykeeper.ScopedKeeper + appModule *AppModule + IBCApp *MockIBCApp // base application of an IBC middleware stack } // NewIBCModule creates a new IBCModule given the underlying mock IBC application and scopedKeeper. -func NewIBCModule(app *MockIBCApp, scopedKeeper capabilitykeeper.ScopedKeeper) IBCModule { +func NewIBCModule(appModule *AppModule, app *MockIBCApp) IBCModule { + appModule.ibcApps = append(appModule.ibcApps, app) return IBCModule{ - IBCApp: app, - scopedKeeper: scopedKeeper, + appModule: appModule, + IBCApp: app, } } @@ -39,7 +39,7 @@ func (im IBCModule) OnChanOpenInit( } // Claim channel capability passed back by IBC module - if err := im.scopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil { + if err := im.IBCApp.ScopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil { return err } @@ -56,7 +56,7 @@ func (im IBCModule) OnChanOpenTry( } // Claim channel capability passed back by IBC module - if err := im.scopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil { + if err := im.IBCApp.ScopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil { return "", err } @@ -107,7 +107,7 @@ func (im IBCModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, re // set state by claiming capability to check if revert happens return capName := GetMockRecvCanaryCapabilityName(packet) - if _, err := im.scopedKeeper.NewCapability(ctx, capName); err != nil { + if _, err := im.IBCApp.ScopedKeeper.NewCapability(ctx, capName); err != nil { // application callback called twice on same packet sequence // must never occur panic(err) @@ -129,7 +129,7 @@ func (im IBCModule) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes } capName := GetMockAckCanaryCapabilityName(packet) - if _, err := im.scopedKeeper.NewCapability(ctx, capName); err != nil { + if _, err := im.IBCApp.ScopedKeeper.NewCapability(ctx, capName); err != nil { // application callback called twice on same packet sequence // must never occur panic(err) @@ -145,7 +145,7 @@ func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, } capName := GetMockTimeoutCanaryCapabilityName(packet) - if _, err := im.scopedKeeper.NewCapability(ctx, capName); err != nil { + if _, err := im.IBCApp.ScopedKeeper.NewCapability(ctx, capName); err != nil { // application callback called twice on same packet sequence // must never occur panic(err) diff --git a/testing/mock/mock.go b/testing/mock/mock.go index fd454aa80d9..a37536de37a 100644 --- a/testing/mock/mock.go +++ b/testing/mock/mock.go @@ -8,7 +8,6 @@ import ( codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" - capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" "github.com/gorilla/mux" "github.com/grpc-ecosystem/grpc-gateway/runtime" @@ -89,15 +88,14 @@ func (AppModuleBasic) GetQueryCmd() *cobra.Command { // AppModule represents the AppModule for the mock module. type AppModule struct { AppModuleBasic - scopedKeeper capabilitykeeper.ScopedKeeper - portKeeper PortKeeper + ibcApps []*MockIBCApp + portKeeper PortKeeper } // NewAppModule returns a mock AppModule instance. -func NewAppModule(sk capabilitykeeper.ScopedKeeper, pk PortKeeper) AppModule { +func NewAppModule(pk PortKeeper) AppModule { return AppModule{ - scopedKeeper: sk, - portKeeper: pk, + portKeeper: pk, } } @@ -124,9 +122,13 @@ func (am AppModule) RegisterServices(module.Configurator) {} // InitGenesis implements the AppModule interface. func (am AppModule) InitGenesis(ctx sdk.Context, cdc codec.JSONCodec, data json.RawMessage) []abci.ValidatorUpdate { - // bind mock port ID - cap := am.portKeeper.BindPort(ctx, ModuleName) - am.scopedKeeper.ClaimCapability(ctx, cap, host.PortPath(ModuleName)) + for _, ibcApp := range am.ibcApps { + if ibcApp.PortID != "" { + // bind mock portID + cap := am.portKeeper.BindPort(ctx, ibcApp.PortID) + ibcApp.ScopedKeeper.ClaimCapability(ctx, cap, host.PortPath(ModuleName)) + } + } return []abci.ValidatorUpdate{} } diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 7f361aa9b8a..49631b172fd 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -350,8 +350,11 @@ func NewSimApp( // 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(scopedIBCMockKeeper, &app.IBCKeeper.PortKeeper) - mockIBCModule := ibcmock.NewIBCModule(&ibcmock.MockIBCApp{}, scopedIBCMockKeeper) + mockModule := ibcmock.NewAppModule(&app.IBCKeeper.PortKeeper) + mockIBCModule := ibcmock.NewIBCModule(&mockModule, &ibcmock.MockIBCApp{ + PortID: ibcmock.ModuleName, + ScopedKeeper: scopedIBCMockKeeper, + }) app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper( appCodec, keys[icacontrollertypes.StoreKey], app.GetSubspace(icacontrollertypes.SubModuleName), @@ -369,7 +372,10 @@ func NewSimApp( icaModule := ica.NewAppModule(&app.ICAControllerKeeper, &app.ICAHostKeeper) // initialize ICA module with mock module as the authentication module on the controller side - icaAuthModule := ibcmock.NewIBCModule(&ibcmock.MockIBCApp{}, scopedICAMockKeeper) + icaAuthModule := ibcmock.NewIBCModule(&mockModule, &ibcmock.MockIBCApp{ + PortID: "", + ScopedKeeper: scopedICAMockKeeper, + }) app.ICAAuthModule = icaAuthModule icaControllerIBCModule := icacontroller.NewIBCModule(app.ICAControllerKeeper, icaAuthModule) From ba3d43d0d284822716d4b70648f6d9e5db5c62f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 9 Feb 2022 13:30:06 +0100 Subject: [PATCH 2/6] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index df182d73b6a..96b7f9211fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (modules/core/02-client) [\#536](https://github.com/cosmos/ibc-go/pull/536) `GetSelfConsensusState` return type changed from bool to error. * (channel) [\#644](https://github.com/cosmos/ibc-go/pull/644) Removes `CounterpartyHops` function from the ChannelKeeper. * (testing) [\#776](https://github.com/cosmos/ibc-go/pull/776) Adding helper fn to generate capability name for testing callbacks +* (testing) [\#892](https://github.com/cosmos/ibc-go/pull/892) IBC Mock modules store the scoped keeper and portID within the IBCMockApp. They also maintain reference to the AppModule to update the AppModule's list of IBC applications it references. Allows for the mock module to be reused as a base application in middleware stacks. ### State Machine Breaking From 174057e5317f7a03fabe2c2f5a7ea48304368587 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 9 Feb 2022 13:30:59 +0100 Subject: [PATCH 3/6] update testing docs --- testing/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/README.md b/testing/README.md index 97f540d5c4c..ad02a80e640 100644 --- a/testing/README.md +++ b/testing/README.md @@ -296,6 +296,7 @@ The mock module may also be leveraged to act as a base application in the instan The mock IBC module contains a `MockIBCApp`. This struct contains a function field for every IBC App Module callback. Each of these functions can be individually set to mock expected behaviour of a base application. +The portID and scoped keeper for the `MockIBCApp` should be set within `MockIBCApp` before calling `NewIBCModule`. For example, if one wanted to test that the base application cannot affect the outcome of the `OnChanOpenTry` callback, the mock module base application callback could be updated as such: ```go From a78bc3fb8de28275fb427b26dcf9f7e37a36ad81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 9 Feb 2022 13:35:43 +0100 Subject: [PATCH 4/6] chore: add and use 'NewMockIBCApp' --- testing/mock/ibc_app.go | 8 ++++++++ testing/simapp/app.go | 10 ++-------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/testing/mock/ibc_app.go b/testing/mock/ibc_app.go index 32fcc23aae8..15f77d02d5a 100644 --- a/testing/mock/ibc_app.go +++ b/testing/mock/ibc_app.go @@ -85,3 +85,11 @@ type MockIBCApp struct { relayer sdk.AccAddress, ) error } + +// NewMockIBCApp returns a MockIBCApp. An empty PortID indicates the mock app doesn't bind/claim ports. +func NewMockIBCApp(portID string, scopedKeeper capabilitykeeper.ScopedKeeper) *MockIBCApp { + return &MockIBCApp{ + PortID: portID, + ScopedKeeper: scopedKeeper, + } +} diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 49631b172fd..c5d6807862e 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -351,10 +351,7 @@ func NewSimApp( // 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) - mockIBCModule := ibcmock.NewIBCModule(&mockModule, &ibcmock.MockIBCApp{ - PortID: ibcmock.ModuleName, - ScopedKeeper: scopedIBCMockKeeper, - }) + mockIBCModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp(ibcmock.ModuleName, scopedIBCMockKeeper)) app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper( appCodec, keys[icacontrollertypes.StoreKey], app.GetSubspace(icacontrollertypes.SubModuleName), @@ -372,10 +369,7 @@ func NewSimApp( icaModule := ica.NewAppModule(&app.ICAControllerKeeper, &app.ICAHostKeeper) // initialize ICA module with mock module as the authentication module on the controller side - icaAuthModule := ibcmock.NewIBCModule(&mockModule, &ibcmock.MockIBCApp{ - PortID: "", - ScopedKeeper: scopedICAMockKeeper, - }) + icaAuthModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp("", scopedICAMockKeeper)) app.ICAAuthModule = icaAuthModule icaControllerIBCModule := icacontroller.NewIBCModule(app.ICAControllerKeeper, icaAuthModule) From f330f34f4bb46079be4142a279f9a8a09a5a9c4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 9 Feb 2022 17:07:38 +0100 Subject: [PATCH 5/6] Update CHANGELOG.md Co-authored-by: Sean King --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eaa336918a..6a530a4e91a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,7 +53,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (modules/core/02-client) [\#536](https://github.com/cosmos/ibc-go/pull/536) `GetSelfConsensusState` return type changed from bool to error. * (channel) [\#644](https://github.com/cosmos/ibc-go/pull/644) Removes `CounterpartyHops` function from the ChannelKeeper. * (testing) [\#776](https://github.com/cosmos/ibc-go/pull/776) Adding helper fn to generate capability name for testing callbacks -* (testing) [\#892](https://github.com/cosmos/ibc-go/pull/892) IBC Mock modules store the scoped keeper and portID within the IBCMockApp. They also maintain reference to the AppModule to update the AppModule's list of IBC applications it references. Allows for the mock module to be reused as a base application in middleware stacks. +* (testing) [\#892](https://github.com/cosmos/ibc-go/pull/892) IBC Mock modules store the scoped keeper and portID within the IBCMockApp. They also maintain reference to the AppModule to update the AppModule's list of IBC applications it references. This allows for the mock module to be reused as a base application in middleware stacks. ### State Machine Breaking From 662b5e7d680851b1d15c3fbdd23b0da47033d62e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 10 Feb 2022 11:49:49 +0100 Subject: [PATCH 6/6] Update CHANGELOG.md --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e9161e876c..13b0b202822 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,7 +56,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (testing) [\#892](https://github.com/cosmos/ibc-go/pull/892) IBC Mock modules store the scoped keeper and portID within the IBCMockApp. They also maintain reference to the AppModule to update the AppModule's list of IBC applications it references. Allows for the mock module to be reused as a base application in middleware stacks. * (channel) [\#882](https://github.com/cosmos/ibc-go/pull/882) The `WriteAcknowledgement` API now takes `exported.Acknowledgement` instead of a byte array - ### State Machine Breaking * (transfer) [\#818](https://github.com/cosmos/ibc-go/pull/818) Error acknowledgements returned from Transfer `OnRecvPacket` now include a deterministic ABCI code and error message.