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.

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

Use new port router for OnChanUpgradeInit #7082

Original file line number Diff line number Diff line change
Expand Up @@ -288,20 +288,6 @@ func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID str
return "", err
}

connectionID, err := im.keeper.GetConnectionID(ctx, portID, channelID)
if err != nil {
return "", err
}

if im.app != nil && im.keeper.IsMiddlewareEnabled(ctx, portID, connectionID) {
// Only cast to UpgradableModule if the application is set.
cbs, ok := im.app.(porttypes.UpgradableModule)
if !ok {
return "", errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack")
}
return cbs.OnChanUpgradeInit(ctx, portID, channelID, proposedOrder, proposedConnectionHops, proposedVersion)
}

return proposedVersion, nil
Copy link
Member

Choose a reason for hiding this comment

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

I think we can return im.keeper.OnChanUpgradeInit() ?

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -777,12 +777,12 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
}, ibcmock.MockApplicationCallbackError,
},
{
"middleware disabled", func() {
"application in stack fails", func() {
suite.chainA.GetSimApp().ICAControllerKeeper.DeleteMiddlewareEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ConnectionID)
suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanUpgradeInit = func(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) {
return "", ibcmock.MockApplicationCallbackError
}
}, nil,
}, ibcmock.MockApplicationCallbackError,
},
}

Expand All @@ -804,10 +804,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {

tc.malleate() // malleate mutates test data

module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID)
suite.Require().NoError(err)

app, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
app, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.AppRouter.HandshakeRoute(types.SubModuleName)
suite.Require().True(ok)
cbs, ok := app.(porttypes.UpgradableModule)
suite.Require().True(ok)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -658,11 +658,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
err := SetupICAPath(path, TestOwnerAddress)
suite.Require().NoError(err)

// call application callback directly
module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
suite.Require().NoError(err)

app, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
app, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.AppRouter.HandshakeRoute(types.SubModuleName)
suite.Require().True(ok)
cbs, ok := app.(porttypes.UpgradableModule)
suite.Require().True(ok)
Expand Down
32 changes: 4 additions & 28 deletions modules/apps/29-fee/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,42 +301,18 @@ func (im IBCMiddleware) OnTimeoutPacket(
}

// OnChanUpgradeInit implements the IBCModule interface
func (im IBCMiddleware) OnChanUpgradeInit(
func (IBCMiddleware) OnChanUpgradeInit(
ctx sdk.Context,
portID string,
channelID string,
proposedOrder channeltypes.Order,
proposedConnectionHops []string,
proposedVersion string,
) (string, error) {
cbs, ok := im.app.(porttypes.UpgradableModule)
if !ok {
return "", errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack")
if proposedVersion != types.Version {
return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, proposedVersion)
}

versionMetadata, err := types.MetadataFromVersion(proposedVersion)
if err != nil {
// since it is valid for fee version to not be specified, the upgrade version may be for a middleware
// or application further down in the stack. Thus, pass through to next middleware or application in callstack.
return cbs.OnChanUpgradeInit(ctx, portID, channelID, proposedOrder, proposedConnectionHops, proposedVersion)
}

if versionMetadata.FeeVersion != types.Version {
return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, versionMetadata.FeeVersion)
}

appVersion, err := cbs.OnChanUpgradeInit(ctx, portID, channelID, proposedOrder, proposedConnectionHops, versionMetadata.AppVersion)
if err != nil {
return "", err
}

versionMetadata.AppVersion = appVersion
versionBz, err := types.ModuleCdc.MarshalJSON(&versionMetadata)
if err != nil {
return "", err
}

return string(versionBz), nil
return types.Version, nil
}

// OnChanUpgradeTry implements the IBCModule interface
Expand Down
9 changes: 2 additions & 7 deletions modules/apps/callbacks/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,13 +358,8 @@ func (im IBCMiddleware) OnChanCloseConfirm(ctx sdk.Context, portID, channelID st
}

// OnChanUpgradeInit implements the IBCModule interface
func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) (string, error) {
cbs, ok := im.app.(porttypes.UpgradableModule)
if !ok {
return "", errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack")
}

return cbs.OnChanUpgradeInit(ctx, portID, channelID, proposedOrder, proposedConnectionHops, proposedVersion)
func (IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) (string, error) {
return proposedVersion, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return proposedVersion, nil
return "", nil

to be explicit?

}

// OnChanUpgradeTry implements the IBCModule interface
Expand Down
73 changes: 55 additions & 18 deletions modules/core/05-port/types/ibc_legacy_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,22 +118,6 @@ func (im *LegacyIBCModule) OnChanOpenTry(
return reconstructVersion(im.cbs, negotiatedVersions)
}

// reconstructVersion will generate the channel version by applying any version wrapping as necessary.
// Version wrapping will only occur if the negotiated version is non=empty and the application is a VersionWrapper.
func reconstructVersion(cbs []ClassicIBCModule, negotiatedVersions []string) (string, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just moved this to the bottom of the file since most handlers are not using this.

version := negotiatedVersions[0] // base version
for i := 1; i < len(cbs); i++ { // iterate over the remaining callbacks
if strings.TrimSpace(negotiatedVersions[i]) != "" {
wrapper, ok := cbs[i].(VersionWrapper)
if !ok {
return "", ibcerrors.ErrInvalidVersion
}
version = wrapper.WrapVersion(negotiatedVersions[i], version)
}
}
return version, nil
}

// OnChanOpenAck implements the IBCModule interface
func (LegacyIBCModule) OnChanOpenAck(
ctx sdk.Context,
Expand Down Expand Up @@ -227,8 +211,45 @@ func (LegacyIBCModule) OnTimeoutPacket(
}

// OnChanUpgradeInit implements the IBCModule interface
func (LegacyIBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) (string, error) {
return "", nil
func (im *LegacyIBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) (string, error) {
negotiatedVersions := make([]string, len(im.cbs))

for i := len(im.cbs) - 1; i >= 0; i-- {
cbVersion := proposedVersion

// To maintain backwards compatibility, we must handle two cases:
// - relayer provides empty version (use default versions)
// - relayer provides version which chooses to not enable a middleware
//
// If an application is a VersionWrapper which means it modifies the version string
// and the version string is non-empty (don't use default), then the application must
// attempt to unmarshal the version using the UnwrapVersionUnsafe interface function.
// If it is unsuccessful, no callback will occur to this application as the version
// indicates it should be disabled.
if wrapper, ok := im.cbs[i].(VersionWrapper); ok && strings.TrimSpace(proposedVersion) != "" {
appVersion, underlyingAppVersion, err := wrapper.UnwrapVersionUnsafe(proposedVersion)
if err != nil {
// middleware disabled
negotiatedVersions[i] = ""
continue
}
cbVersion, proposedVersion = appVersion, underlyingAppVersion
}

// in order to maintain backwards compatibility, every callback in the stack must implement the UpgradableModule interface.
upgradableModule, ok := im.cbs[i].(UpgradableModule)
if !ok {
return "", errorsmod.Wrap(ErrInvalidRoute, "upgrade route not found to module in application callstack")
}

negotiatedVersion, err := upgradableModule.OnChanUpgradeInit(ctx, portID, channelID, proposedOrder, proposedConnectionHops, cbVersion)
if err != nil {
return "", errorsmod.Wrapf(err, "channel open init callback failed for port ID: %s, channel ID: %s", portID, channelID)
}
negotiatedVersions[i] = negotiatedVersion
}

return reconstructVersion(im.cbs, negotiatedVersions)
}

// OnChanUpgradeTry implements the IBCModule interface
Expand All @@ -251,3 +272,19 @@ func (LegacyIBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID stri
func (LegacyIBCModule) UnmarshalPacketData(ctx sdk.Context, portID, channelID string, bz []byte) (interface{}, error) {
return nil, nil
}

// reconstructVersion will generate the channel version by applying any version wrapping as necessary.
// Version wrapping will only occur if the negotiated version is non=empty and the application is a VersionWrapper.
func reconstructVersion(cbs []ClassicIBCModule, negotiatedVersions []string) (string, error) {
version := negotiatedVersions[0] // base version
for i := 1; i < len(cbs); i++ { // iterate over the remaining callbacks
if strings.TrimSpace(negotiatedVersions[i]) != "" {
wrapper, ok := cbs[i].(VersionWrapper)
if !ok {
return "", ibcerrors.ErrInvalidVersion
}
version = wrapper.WrapVersion(negotiatedVersions[i], version)
}
}
return version, nil
}
2 changes: 1 addition & 1 deletion modules/core/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ func (k *Keeper) ChannelUpgradeInit(goCtx context.Context, msg *channeltypes.Msg
return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "expected %s, got %s", k.GetAuthority(), msg.Signer)
}

app, ok := k.PortKeeper.Route(msg.PortId)
app, ok := k.PortKeeper.AppRouter.HandshakeRoute(msg.PortId)
if !ok {
ctx.Logger().Error("channel upgrade init failed", "port-id", msg.PortId, "error", errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", msg.PortId))
return nil, errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", msg.PortId)
Expand Down
23 changes: 0 additions & 23 deletions modules/core/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -974,29 +974,6 @@ func (suite *KeeperTestSuite) TestChannelUpgradeInit() {
suite.Require().Empty(events)
},
},
{
"ibc application does not implement the UpgradeableModule interface",
func() {
path = ibctesting.NewPath(suite.chainA, suite.chainB)
path.EndpointA.ChannelConfig.PortID = ibcmock.MockBlockUpgrade
path.EndpointB.ChannelConfig.PortID = ibcmock.MockBlockUpgrade

path.Setup()

msg = channeltypes.NewMsgChannelUpgradeInit(
path.EndpointA.ChannelConfig.PortID,
path.EndpointA.ChannelID,
path.EndpointA.GetProposedUpgrade().Fields,
path.EndpointA.Chain.GetSimApp().IBCKeeper.GetAuthority(),
)
},
func(res *channeltypes.MsgChannelUpgradeInitResponse, events []abci.Event, err error) {
suite.Require().ErrorIs(err, porttypes.ErrInvalidRoute)
suite.Require().Nil(res)

suite.Require().Empty(events)
},
},
{
"ibc application does not commit state changes in callback",
func() {
Expand Down
Loading