Skip to content

Commit

Permalink
refactor: allow the mock module to be used multiple times as base ibc…
Browse files Browse the repository at this point in the history
… 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/<module>/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
  • Loading branch information
colin-axner authored Feb 10, 2022
1 parent d5e2ba5 commit 8f62a47
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 23 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +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.
* (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
Expand Down
1 change: 1 addition & 0 deletions testing/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions testing/mock/ibc_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
Expand Down Expand Up @@ -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,
}
}
22 changes: 11 additions & 11 deletions testing/mock/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
}
}

Expand All @@ -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
}

Expand All @@ -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
}

Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
20 changes: 11 additions & 9 deletions testing/mock/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
}
}

Expand All @@ -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{}
}
Expand Down
6 changes: 3 additions & 3 deletions testing/simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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)
Expand Down

0 comments on commit 8f62a47

Please sign in to comment.