Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add upgrade callback overrides for mock ibc app #3812

Merged
merged 2 commits into from
Jun 12, 2023
Merged

chore: add upgrade callback overrides for mock ibc app #3812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d0a452
adding testing app overrides for upgrade handshake app callback handlers
damiannolan Jun 11, 2023
c692ca3
Merge branch '04-channel-upgrades' into damian/upgrade-cbs-testing-ap…
damiannolan Jun 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions testing/mock/ibc_app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,42 @@ type IBCApp struct {
packet channeltypes.Packet,
relayer sdk.AccAddress,
) error

OnChanUpgradeInit func(
ctx sdk.Context,
portID, channelID string,
order channeltypes.Order,
connectionHops []string,
sequence uint64,
version, previousVersion string,
) (string, error)

OnChanUpgradeTry func(
ctx sdk.Context,
portID, channelID string,
order channeltypes.Order,
connectionHops []string,
counterpartyVersion string,
) (string, error)

OnChanUpgradeAck func(
ctx sdk.Context,
portID,
channelID,
counterpartyVersion string,
) error

OnChanUpgradeOpen func(
ctx sdk.Context,
portID,
channelID string,
) error

OnChanUpgradeRestore func(
ctx sdk.Context,
portID,
channelID string,
) error
}

// NewIBCApp returns a IBCApp. An empty PortID indicates the mock app doesn't bind/claim ports.
Expand Down
20 changes: 20 additions & 0 deletions testing/mock/ibc_module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,26 +164,46 @@ func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet,

// OnChanUpgradeInit implements the IBCModule interface
func (im IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, sequence uint64, version, previousVersion string) (string, error) {
if im.IBCApp.OnChanUpgradeInit != nil {
return im.IBCApp.OnChanUpgradeInit(ctx, portID, channelID, order, connectionHops, sequence, version, previousVersion)
}

return version, nil
}

// OnChanUpgradeTry implements the IBCModule interface
func (im IBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, counterpartyVersion string) (string, error) {
if im.IBCApp.OnChanUpgradeTry != nil {
return im.IBCApp.OnChanUpgradeTry(ctx, portID, channelID, order, connectionHops, counterpartyVersion)
}

return counterpartyVersion, nil
}

// OnChanUpgradeAck implements the IBCModule interface
func (im IBCModule) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error {
if im.IBCApp.OnChanUpgradeAck != nil {
return im.IBCApp.OnChanUpgradeAck(ctx, portID, channelID, counterpartyVersion)
}

return nil
}

// OnChanUpgradeOpen implements the IBCModule interface
func (im IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string) error {
if im.IBCApp.OnChanUpgradeOpen != nil {
return im.IBCApp.OnChanUpgradeOpen(ctx, portID, channelID)
}

return nil
}

// OnChanUpgradeRestore implements the IBCModule interface
func (im IBCModule) OnChanUpgradeRestore(ctx sdk.Context, portID, channelID string) error {
if im.IBCApp.OnChanUpgradeRestore != nil {
return im.IBCApp.OnChanUpgradeRestore(ctx, portID, channelID)
}

return nil
}

Expand Down
2 changes: 2 additions & 0 deletions testing/simapp/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ type SimApp struct {

// make IBC modules public for test purposes
// these modules are never directly routed to by the IBC Router
IBCMockModule ibcmock.IBCModule
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side question, how come this wasn't present earlier?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we originally added these overrides to test middleware. One initial point of testing was with interchain accounts (to mimic "icauth" modules, v6 improvement removed notion of "icaauth"). The mock module would be wrapped however as it acts as middleware, thus you reference ICAAuthModule instead of IBCMockModule

Referencing the unwrapped mock module is only required when you don't need to test the mock module as middleware but as a base application. This only occurs when you need to revert state of the IBC app callback. This occurs in OnRecvPacket handling. Because those tests were written before this feature was added, it uses a mock.MockFailPacketData. Then logic was added to mock.OnRecvPacket to return a certain ack based on the packet data. These tests could be updated to use the override app callback. It'd probably be more stable in the long term (as you won't need to pass in a certain packet data to create a certain result)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#432

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like I initially added GetMockModule() but I'm not sure when that was removed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was removed in this pr #380, but idk why. Might have been something as simple as a merge conflict mistake or unused func

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, must have been a merge conflict I assume. I was a little surprised when I noticed this was missing from the SimApp struct.

ICAAuthModule ibcmock.IBCModule
FeeMockModule ibcmock.IBCModule

Expand Down Expand Up @@ -455,6 +456,7 @@ func NewSimApp(

// The mock module is used for testing IBC
mockIBCModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp(ibcmock.ModuleName, scopedIBCMockKeeper))
app.IBCMockModule = mockIBCModule
ibcRouter.AddRoute(ibcmock.ModuleName, mockIBCModule)

// Create Transfer Stack
Expand Down