From aca4a1bb898f0f51c09040d7118bea368068f36c 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 12:07:21 +0100 Subject: [PATCH 1/2] refactor: allow the mock module to be used multiple times as base ibc application in middleware stack (#892) ## Description Currently the `AppModule` assumes a single scoped keeper. This doesn't allow the mock module to be used as a base application for different middleware stack (ica stack, fee stack, etc) I broke the API because I think it is cleaner. If we want this to be non API breaking, I can try to readjust ref: #891 --- Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why. - [ ] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md). - [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing) - [ ] Updated relevant documentation (`docs/`) or specification (`x//spec/`) - [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [ ] Re-reviewed `Files changed` in the Github PR explorer - [ ] Review `Codecov Report` in the comment section below once CI passes (cherry picked from commit 8f62a47a28c9190a8e54782fdfa2a45b085a7b4a) # Conflicts: # CHANGELOG.md --- CHANGELOG.md | 5 +++++ testing/README.md | 1 + testing/mock/ibc_app.go | 12 ++++++++++++ testing/mock/ibc_module.go | 22 +++++++++++----------- testing/mock/mock.go | 20 +++++++++++--------- testing/simapp/app.go | 6 +++--- 6 files changed, 43 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index adb0e0243ea..1067d854358 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,7 +53,12 @@ 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 +<<<<<<< HEAD +======= +* (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 +>>>>>>> 8f62a47 (refactor: allow the mock module to be used multiple times as base ibc application in middleware stack (#892)) ### State Machine Breaking 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 diff --git a/testing/mock/ibc_app.go b/testing/mock/ibc_app.go index a3f2db3bc6d..15f77d02d5a 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, @@ -81,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/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..c5d6807862e 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -350,8 +350,8 @@ 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.NewMockIBCApp(ibcmock.ModuleName, scopedIBCMockKeeper)) app.ICAControllerKeeper = icacontrollerkeeper.NewKeeper( appCodec, keys[icacontrollertypes.StoreKey], app.GetSubspace(icacontrollertypes.SubModuleName), @@ -369,7 +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(&ibcmock.MockIBCApp{}, scopedICAMockKeeper) + icaAuthModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewMockIBCApp("", scopedICAMockKeeper)) app.ICAAuthModule = icaAuthModule icaControllerIBCModule := icacontroller.NewIBCModule(app.ICAControllerKeeper, icaAuthModule) From cc3949cc3e1ff15ff0b24c468b235d3fbea4684f 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 12:22:23 +0100 Subject: [PATCH 2/2] fix conflicts --- CHANGELOG.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1067d854358..2bcca89af2e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,12 +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 -<<<<<<< HEAD - -======= * (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 ->>>>>>> 8f62a47 (refactor: allow the mock module to be used multiple times as base ibc application in middleware stack (#892)) ### State Machine Breaking