From ee9ebf3f381f693631d987fb6336c1056a0f932b Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 23 Jan 2024 17:00:06 +0100 Subject: [PATCH 01/10] docs: remove refs to OnChanUpgradeRestore in docs --- docs/docs/01-ibc/06-channel-upgrades.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/docs/docs/01-ibc/06-channel-upgrades.md b/docs/docs/01-ibc/06-channel-upgrades.md index 6f2b528225c..a663363f912 100644 --- a/docs/docs/01-ibc/06-channel-upgrades.md +++ b/docs/docs/01-ibc/06-channel-upgrades.md @@ -152,8 +152,6 @@ It is possible for a relayer to cancel an in-progress channel upgrade if the fol Upon cancelling a channel upgrade, an `ErrorReceipt` will be written with the channel's current upgrade sequence, and the channel will move back to `OPEN` state keeping its original parameters. -The application's `OnChanUpgradeRestore` callback method will be invoked. - It will then be possible to re-initiate an upgrade by sending a `MsgChannelOpenInit` message. ## Timing Out a Channel Upgrade @@ -204,8 +202,6 @@ type MsgChannelUpgradeTimeout struct { An `ErrorReceipt` will be written with the channel's current upgrade sequence, and the channel will move back to `OPEN` state keeping its original parameters. -The application's `OnChanUpgradeRestore` callback method will also be invoked. - Note that timing out a channel upgrade will end the upgrade process, and a new `MsgChannelUpgradeInit` will have to be submitted via governance in order to restart the upgrade process. ## Pruning Acknowledgements @@ -252,8 +248,6 @@ should be aware that callbacks will be invoked before any core ibc state changes `OnChanUpgradeOpen` should perform any logic associated with changing of the channel fields. -`OnChanUpgradeRestore` should perform any logic that needs to be executed when an upgrade attempt fails as is reverted. - > IBC applications should not attempt to process any packet data under the new conditions until after `OnChanUpgradeOpen` > has been executed, as up until this point it is still possible for the upgrade handshake to fail and for the channel > to remain in the pre-upgraded state. From 70284f125ab861e098ecd3b555d20828946384ad Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 23 Jan 2024 17:00:31 +0100 Subject: [PATCH 02/10] testing: rm OnChanUpgradeRestore callback from mock app test helpers --- testing/mock/ibc_app.go | 6 ------ testing/mock/ibc_module.go | 7 ------- 2 files changed, 13 deletions(-) diff --git a/testing/mock/ibc_app.go b/testing/mock/ibc_app.go index a49a834ef96..026a5d0873d 100644 --- a/testing/mock/ibc_app.go +++ b/testing/mock/ibc_app.go @@ -117,12 +117,6 @@ type IBCApp struct { connectionHops []string, version string, ) - - OnChanUpgradeRestore func( - ctx sdk.Context, - portID, - channelID string, - ) } // NewIBCApp returns a IBCApp. An empty PortID indicates the mock app doesn't bind/claim ports. diff --git a/testing/mock/ibc_module.go b/testing/mock/ibc_module.go index 9ca2b23c6d8..b3bd2432de3 100644 --- a/testing/mock/ibc_module.go +++ b/testing/mock/ibc_module.go @@ -217,13 +217,6 @@ func (im IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, } } -// OnChanUpgradeRestore implements the IBCModule interface -func (im IBCModule) OnChanUpgradeRestore(ctx sdk.Context, portID, channelID string) { - if im.IBCApp.OnChanUpgradeRestore != nil { - im.IBCApp.OnChanUpgradeRestore(ctx, portID, channelID) - } -} - // UnmarshalPacketData returns the MockPacketData. This function implements the optional // PacketDataUnmarshaler interface required for ADR 008 support. func (IBCModule) UnmarshalPacketData(bz []byte) (interface{}, error) { From f83d3ca38f157da91089e6e87161bbe011373638 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 23 Jan 2024 17:00:49 +0100 Subject: [PATCH 03/10] chore: rm OnChanUpgradeRestore application callback from ibc core and apps --- .../controller/ibc_middleware.go | 17 ----- .../controller/ibc_middleware_test.go | 61 --------------- .../27-interchain-accounts/host/ibc_module.go | 3 - modules/apps/29-fee/ibc_middleware.go | 10 --- modules/apps/callbacks/ibc_middleware.go | 10 --- modules/apps/transfer/ibc_module.go | 3 - modules/core/05-port/types/module.go | 11 --- modules/core/keeper/msg_server.go | 47 +----------- modules/core/keeper/msg_server_test.go | 76 +------------------ 9 files changed, 2 insertions(+), 236 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index ae05b7476b2..fe9ef65a026 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -309,23 +309,6 @@ func (im IBCMiddleware) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID str } } -// OnChanUpgradeRestore implements the IBCModule interface -func (im IBCMiddleware) OnChanUpgradeRestore(ctx sdk.Context, portID, channelID string) { - connectionID, err := im.keeper.GetConnectionID(ctx, portID, channelID) - if err != nil { - panic(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 { - panic(errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack")) - } - cbs.OnChanUpgradeRestore(ctx, portID, channelID) - } -} - // SendPacket implements the ICS4 Wrapper interface func (IBCMiddleware) SendPacket( ctx sdk.Context, diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index 1c7925e1842..93610b452ff 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -1066,67 +1066,6 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeOpen() { } } -func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeRestore() { - var ( - path *ibctesting.Path - isNilApp bool - ) - - testCases := []struct { - name string - malleate func() - }{ - { - "success", func() {}, - }, - { - "success: nil app", - func() { - isNilApp = true - }, - }, - { - "middleware disabled", func() { - suite.chainA.GetSimApp().ICAControllerKeeper.DeleteMiddlewareEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ConnectionID) - suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanUpgradeAck = func(ctx sdk.Context, portID, channelID string, counterpartyVersion string) error { - return ibcmock.MockApplicationCallbackError - } - }, - }, - } - - for _, tc := range testCases { - tc := tc - - suite.Run(tc.name, func() { - suite.SetupTest() // reset - isNilApp = false - - path = NewICAPath(suite.chainA, suite.chainB) - suite.coordinator.SetupConnections(path) - - err := SetupICAPath(path, TestOwnerAddress) - suite.Require().NoError(err) - - 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().Router.GetRoute(module) - suite.Require().True(ok) - cbs, ok := app.(porttypes.UpgradableModule) - suite.Require().True(ok) - - if isNilApp { - cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper) - } - - cbs.OnChanUpgradeRestore(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - }) - } -} - func (suite *InterchainAccountsTestSuite) TestSingleHostMultipleControllers() { var ( pathAToB *ibctesting.Path diff --git a/modules/apps/27-interchain-accounts/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go index 0ea90d4cddf..622bf284bd9 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module.go @@ -180,9 +180,6 @@ func (IBCModule) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpar func (IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) { } -// OnChanUpgradeRestore implements the IBCModule interface -func (IBCModule) OnChanUpgradeRestore(ctx sdk.Context, portID, channelID string) {} - // UnmarshalPacketData attempts to unmarshal the provided packet data bytes // into an InterchainAccountPacketData. This function implements the optional // PacketDataUnmarshaler interface required for ADR 008 support. diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go index becaa10b2ca..4ac1e7dd989 100644 --- a/modules/apps/29-fee/ibc_middleware.go +++ b/modules/apps/29-fee/ibc_middleware.go @@ -440,16 +440,6 @@ func (im IBCMiddleware) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID str cbs.OnChanUpgradeOpen(ctx, portID, channelID, proposedOrder, proposedConnectionHops, versionMetadata.AppVersion) } -// OnChanUpgradeRestore implements the IBCModule interface -func (im IBCMiddleware) OnChanUpgradeRestore(ctx sdk.Context, portID, channelID string) { - cbs, ok := im.app.(porttypes.UpgradableModule) - if !ok { - panic(errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack")) - } - - cbs.OnChanUpgradeRestore(ctx, portID, channelID) -} - // SendPacket implements the ICS4 Wrapper interface func (im IBCMiddleware) SendPacket( ctx sdk.Context, diff --git a/modules/apps/callbacks/ibc_middleware.go b/modules/apps/callbacks/ibc_middleware.go index d43ab06cff6..1f1da0b6cde 100644 --- a/modules/apps/callbacks/ibc_middleware.go +++ b/modules/apps/callbacks/ibc_middleware.go @@ -404,16 +404,6 @@ func (im IBCMiddleware) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID str cbs.OnChanUpgradeOpen(ctx, portID, channelID, proposedOrder, proposedConnectionHops, proposedVersion) } -// OnChanUpgradeRestore implements the IBCModule interface -func (im IBCMiddleware) OnChanUpgradeRestore(ctx sdk.Context, portID, channelID string) { - cbs, ok := im.app.(porttypes.UpgradableModule) - if !ok { - panic(errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack")) - } - - cbs.OnChanUpgradeRestore(ctx, portID, channelID) -} - // GetAppVersion implements the ICS4Wrapper interface. Callbacks has no version, // so the call is deferred to the underlying application. func (im IBCMiddleware) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) { diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go index 28ae28d8f65..b73dc6e8edb 100644 --- a/modules/apps/transfer/ibc_module.go +++ b/modules/apps/transfer/ibc_module.go @@ -347,9 +347,6 @@ func (IBCModule) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpar func (IBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) { } -// OnChanUpgradeRestore implements the IBCModule interface -func (IBCModule) OnChanUpgradeRestore(ctx sdk.Context, portID, channelID string) {} - // UnmarshalPacketData attempts to unmarshal the provided packet data bytes // into a FungibleTokenPacketData. This function implements the optional // PacketDataUnmarshaler interface required for ADR 008 support. diff --git a/modules/core/05-port/types/module.go b/modules/core/05-port/types/module.go index 77b6ae6fad7..71a1ee59ef2 100644 --- a/modules/core/05-port/types/module.go +++ b/modules/core/05-port/types/module.go @@ -152,17 +152,6 @@ type UpgradableModule interface { proposedConnectionHops []string, proposedVersion string, ) - - // OnChanUpgradeRestore enables additional custom logic to be executed when any of the following occur: - // - the channel upgrade is aborted. - // - the channel upgrade is cancelled. - // - the channel upgrade times out. - // Any logic set due to an upgrade attempt should be reverted in this callback. - OnChanUpgradeRestore( - ctx sdk.Context, - portID, - channelID string, - ) } // ICS4Wrapper implements the ICS4 interfaces that IBC applications use to send packets and acknowledgements. diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 028aceff881..da7f72d41db 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -875,7 +875,6 @@ func (k Keeper) ChannelUpgradeAck(goCtx context.Context, msg *channeltypes.MsgCh if err != nil { ctx.Logger().Error("channel upgrade ack failed", "error", errorsmod.Wrap(err, "channel upgrade ack failed")) if channeltypes.IsUpgradeError(err) { - cbs.OnChanUpgradeRestore(ctx, msg.PortId, msg.ChannelId) k.ChannelKeeper.MustAbortUpgrade(ctx, msg.PortId, msg.ChannelId, err) // NOTE: a FAILURE result is returned to the client and an error receipt is written to state. @@ -891,7 +890,6 @@ func (k Keeper) ChannelUpgradeAck(goCtx context.Context, msg *channeltypes.MsgCh err = cbs.OnChanUpgradeAck(cacheCtx, msg.PortId, msg.ChannelId, msg.CounterpartyUpgrade.Fields.Version) if err != nil { ctx.Logger().Error("channel upgrade ack callback failed", "port-id", msg.PortId, "channel-id", msg.ChannelId, "error", err.Error()) - cbs.OnChanUpgradeRestore(ctx, msg.PortId, msg.ChannelId) k.ChannelKeeper.MustAbortUpgrade(ctx, msg.PortId, msg.ChannelId, err) return &channeltypes.MsgChannelUpgradeAckResponse{Result: channeltypes.FAILURE}, nil @@ -935,7 +933,6 @@ func (k Keeper) ChannelUpgradeConfirm(goCtx context.Context, msg *channeltypes.M if err != nil { ctx.Logger().Error("channel upgrade confirm failed", "error", errorsmod.Wrap(err, "channel upgrade confirm failed")) if channeltypes.IsUpgradeError(err) { - cbs.OnChanUpgradeRestore(ctx, msg.PortId, msg.ChannelId) k.ChannelKeeper.MustAbortUpgrade(ctx, msg.PortId, msg.ChannelId, err) // NOTE: a FAILURE result is returned to the client and an error receipt is written to state. @@ -1015,32 +1012,11 @@ func (k Keeper) ChannelUpgradeOpen(goCtx context.Context, msg *channeltypes.MsgC // ChannelUpgradeTimeout defines a rpc handler method for MsgChannelUpgradeTimeout. func (k Keeper) ChannelUpgradeTimeout(goCtx context.Context, msg *channeltypes.MsgChannelUpgradeTimeout) (*channeltypes.MsgChannelUpgradeTimeoutResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) - module, _, err := k.ChannelKeeper.LookupModuleByChannel(ctx, msg.PortId, msg.ChannelId) - if err != nil { - ctx.Logger().Error("channel upgrade timeout failed", "port-id", msg.PortId, "error", errorsmod.Wrap(err, "could not retrieve module from port-id")) - return nil, errorsmod.Wrap(err, "could not retrieve module from port-id") - } - app, ok := k.Router.GetRoute(module) - if !ok { - err = errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", module) - ctx.Logger().Error("channel upgrade timeout failed", "port-id", msg.PortId, "error", err) - return nil, err - } - - cbs, ok := app.(porttypes.UpgradableModule) - if !ok { - err = errorsmod.Wrapf(porttypes.ErrInvalidRoute, "upgrade route not found to module: %s", module) - ctx.Logger().Error("channel upgrade timeout failed", "port-id", msg.PortId, "error", err) - return nil, err - } - - err = k.ChannelKeeper.ChanUpgradeTimeout(ctx, msg.PortId, msg.ChannelId, msg.CounterpartyChannel, msg.ProofChannel, msg.ProofHeight) - if err != nil { + if err := k.ChannelKeeper.ChanUpgradeTimeout(ctx, msg.PortId, msg.ChannelId, msg.CounterpartyChannel, msg.ProofChannel, msg.ProofHeight); err != nil { return nil, errorsmod.Wrapf(err, "could not timeout upgrade for channel: %s", msg.ChannelId) } - cbs.OnChanUpgradeRestore(ctx, msg.PortId, msg.ChannelId) channel, upgrade := k.ChannelKeeper.WriteUpgradeTimeoutChannel(ctx, msg.PortId, msg.ChannelId) ctx.Logger().Info("channel upgrade timeout callback succeeded: portID %s, channelID %s", msg.PortId, msg.ChannelId) @@ -1052,25 +1028,6 @@ func (k Keeper) ChannelUpgradeTimeout(goCtx context.Context, msg *channeltypes.M // ChannelUpgradeCancel defines a rpc handler method for MsgChannelUpgradeCancel. func (k Keeper) ChannelUpgradeCancel(goCtx context.Context, msg *channeltypes.MsgChannelUpgradeCancel) (*channeltypes.MsgChannelUpgradeCancelResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) - module, _, err := k.ChannelKeeper.LookupModuleByChannel(ctx, msg.PortId, msg.ChannelId) - if err != nil { - ctx.Logger().Error("channel upgrade cancel failed", "port-id", msg.PortId, "error", errorsmod.Wrap(err, "could not retrieve module from port-id")) - return nil, errorsmod.Wrap(err, "could not retrieve module from port-id") - } - - app, ok := k.Router.GetRoute(module) - if !ok { - err = errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", module) - ctx.Logger().Error("channel upgrade cancel failed", "port-id", msg.PortId, "error", err) - return nil, err - } - - cbs, ok := app.(porttypes.UpgradableModule) - if !ok { - err = errorsmod.Wrapf(porttypes.ErrInvalidRoute, "upgrade route not found to module: %s", module) - ctx.Logger().Error("channel upgrade cancel failed", "port-id", msg.PortId, err) - return nil, err - } channel, found := k.ChannelKeeper.GetChannel(ctx, msg.PortId, msg.ChannelId) if !found { @@ -1086,7 +1043,6 @@ func (k Keeper) ChannelUpgradeCancel(goCtx context.Context, msg *channeltypes.Ms return nil, errorsmod.Wrapf(channeltypes.ErrUpgradeNotFound, "failed to retrieve channel upgrade: port ID (%s) channel ID (%s)", msg.PortId, msg.ChannelId) } - cbs.OnChanUpgradeRestore(ctx, msg.PortId, msg.ChannelId) k.ChannelKeeper.WriteUpgradeCancelChannel(ctx, msg.PortId, msg.ChannelId, channel.UpgradeSequence) ctx.Logger().Info("channel upgrade cancel succeeded", "port-id", msg.PortId, "channel-id", msg.ChannelId) @@ -1107,7 +1063,6 @@ func (k Keeper) ChannelUpgradeCancel(goCtx context.Context, msg *channeltypes.Ms return nil, errorsmod.Wrapf(channeltypes.ErrUpgradeNotFound, "failed to retrieve channel upgrade: port ID (%s) channel ID (%s)", msg.PortId, msg.ChannelId) } - cbs.OnChanUpgradeRestore(ctx, msg.PortId, msg.ChannelId) k.ChannelKeeper.WriteUpgradeCancelChannel(ctx, msg.PortId, msg.ChannelId, msg.ErrorReceipt.Sequence) ctx.Logger().Info("channel upgrade cancel succeeded", "port-id", msg.PortId, "channel-id", msg.ChannelId) diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index 2251b546593..c09f9622437 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -2158,24 +2158,6 @@ func (suite *KeeperTestSuite) TestChannelUpgradeCancel() { suite.Require().Equal(uint64(1), channel.UpgradeSequence) }, }, - { - "capability not found", - func() { - msg.ChannelId = ibctesting.InvalidID - }, - func(res *channeltypes.MsgChannelUpgradeCancelResponse, events []abci.Event, err error) { - suite.Require().Error(err) - suite.Require().Nil(res) - - channel := path.EndpointA.GetChannel() - suite.Require().ErrorIs(err, capabilitytypes.ErrCapabilityNotFound) - suite.Require().Equal(channeltypes.OPEN, channel.State) - // Upgrade sequence should not be changed. - suite.Require().Equal(uint64(1), channel.UpgradeSequence) - - suite.Require().Empty(events) - }, - }, { "core handler fails: invalid proof", func() { @@ -2198,29 +2180,11 @@ func (suite *KeeperTestSuite) TestChannelUpgradeCancel() { // Upgrade sequence should not be changed. suite.Require().Equal(uint64(2), channel.UpgradeSequence) - 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 - - suite.coordinator.Setup(path) - - msg.PortId = path.EndpointB.ChannelConfig.PortID - msg.ChannelId = path.EndpointB.ChannelID - }, - func(res *channeltypes.MsgChannelUpgradeCancelResponse, events []abci.Event, err error) { - suite.Require().ErrorIs(err, porttypes.ErrInvalidRoute) - suite.Require().Nil(res) - suite.Require().Empty(events) }, }, } + for _, tc := range cases { tc := tc suite.Run(tc.name, func() { @@ -2359,25 +2323,6 @@ func (suite *KeeperTestSuite) TestChannelUpgradeTimeout() { ibctesting.AssertEventsLegacy(&suite.Suite, expEvents, events) }, }, - { - "capability not found", - func() { - msg.ChannelId = ibctesting.InvalidID - }, - func(res *channeltypes.MsgChannelUpgradeTimeoutResponse, events []abci.Event, err error) { - suite.Require().Error(err) - suite.Require().ErrorIs(err, capabilitytypes.ErrCapabilityNotFound) - - channel := path.EndpointA.GetChannel() - suite.Require().Equalf(channeltypes.FLUSHCOMPLETE, channel.State, "channel state should be %s", channeltypes.OPEN) - suite.Require().Equal(uint64(1), channel.UpgradeSequence, "channel upgrade sequence should not incremented") - - _, found := path.EndpointA.Chain.GetSimApp().IBCKeeper.ChannelKeeper.GetUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - suite.Require().True(found, "channel upgrade should not be nil") - - suite.Require().Empty(events) - }, - }, { "core handler fails: invalid proof", func() { @@ -2401,25 +2346,6 @@ func (suite *KeeperTestSuite) TestChannelUpgradeTimeout() { _, found := path.EndpointA.Chain.GetSimApp().IBCKeeper.ChannelKeeper.GetUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().True(found, "channel upgrade should not be nil") - 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 - - suite.coordinator.Setup(path) - - msg.PortId = path.EndpointB.ChannelConfig.PortID - msg.ChannelId = path.EndpointB.ChannelID - }, - func(res *channeltypes.MsgChannelUpgradeTimeoutResponse, events []abci.Event, err error) { - suite.Require().ErrorIs(err, porttypes.ErrInvalidRoute) - suite.Require().Nil(res) - suite.Require().Empty(events) }, }, From 1cb7661146ace8bd077d87724f06275e3a3a854f Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 23 Jan 2024 17:13:02 +0100 Subject: [PATCH 04/10] chore: discard app state changes by using a cacheCtx and discarding writeFn --- modules/core/keeper/msg_server.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index da7f72d41db..374d885490b 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -769,8 +769,9 @@ func (k Keeper) ChannelUpgradeInit(goCtx context.Context, msg *channeltypes.MsgC return nil, errorsmod.Wrap(err, "channel upgrade init failed") } + cacheCtx, _ := ctx.CacheContext() // NOTE: the writeFn is discarded and application state changes are not committed. upgradeVersion, err := cbs.OnChanUpgradeInit( - ctx, + cacheCtx, msg.PortId, msg.ChannelId, upgrade.Fields.Ordering, @@ -829,7 +830,8 @@ func (k Keeper) ChannelUpgradeTry(goCtx context.Context, msg *channeltypes.MsgCh return nil, errorsmod.Wrap(err, "channel upgrade try failed") } - upgradeVersion, err := cbs.OnChanUpgradeTry(ctx, msg.PortId, msg.ChannelId, upgrade.Fields.Ordering, upgrade.Fields.ConnectionHops, upgrade.Fields.Version) + cacheCtx, _ := ctx.CacheContext() // NOTE: the writeFn is discarded and application state changes are not committed. + upgradeVersion, err := cbs.OnChanUpgradeTry(cacheCtx, msg.PortId, msg.ChannelId, upgrade.Fields.Ordering, upgrade.Fields.ConnectionHops, upgrade.Fields.Version) if err != nil { ctx.Logger().Error("channel upgrade try callback failed", "port-id", msg.PortId, "channel-id", msg.ChannelId, "error", err.Error()) return nil, errorsmod.Wrapf(err, "channel upgrade try callback failed for port ID: %s, channel ID: %s", msg.PortId, msg.ChannelId) @@ -886,7 +888,7 @@ func (k Keeper) ChannelUpgradeAck(goCtx context.Context, msg *channeltypes.MsgCh return nil, errorsmod.Wrap(err, "channel upgrade ack failed") } - cacheCtx, writeFn := ctx.CacheContext() + cacheCtx, _ := ctx.CacheContext() // NOTE: the writeFn is discarded and application state changes are not committed. err = cbs.OnChanUpgradeAck(cacheCtx, msg.PortId, msg.ChannelId, msg.CounterpartyUpgrade.Fields.Version) if err != nil { ctx.Logger().Error("channel upgrade ack callback failed", "port-id", msg.PortId, "channel-id", msg.ChannelId, "error", err.Error()) @@ -895,8 +897,6 @@ func (k Keeper) ChannelUpgradeAck(goCtx context.Context, msg *channeltypes.MsgCh return &channeltypes.MsgChannelUpgradeAckResponse{Result: channeltypes.FAILURE}, nil } - writeFn() - channel, upgrade := k.ChannelKeeper.WriteUpgradeAckChannel(ctx, msg.PortId, msg.ChannelId, msg.CounterpartyUpgrade) ctx.Logger().Info("channel upgrade ack succeeded", "port-id", msg.PortId, "channel-id", msg.ChannelId) From 471cf8f6cc799d43fa6c20a390cc868f380eed8d Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 23 Jan 2024 17:33:38 +0100 Subject: [PATCH 05/10] docs: add godoc notes to app callbacks discarding state changes --- modules/core/05-port/types/module.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/core/05-port/types/module.go b/modules/core/05-port/types/module.go index 71a1ee59ef2..23fc5705233 100644 --- a/modules/core/05-port/types/module.go +++ b/modules/core/05-port/types/module.go @@ -112,7 +112,8 @@ type IBCModule interface { type UpgradableModule interface { // OnChanUpgradeInit enables additional custom logic to be executed when the channel upgrade is initialized. // It must validate the proposed version, order, and connection hops. - // Note: in the case of crossing hellos, this callback may be executed on both chains. + // NOTE: in the case of crossing hellos, this callback may be executed on both chains. + // NOTE: Any IBC application state changes made in this callback handler are not committed. OnChanUpgradeInit( ctx sdk.Context, portID, channelID string, @@ -124,6 +125,7 @@ type UpgradableModule interface { // OnChanUpgradeTry enables additional custom logic to be executed in the ChannelUpgradeTry step of the // channel upgrade handshake. It must validate the proposed version (provided by the counterparty), order, // and connection hops. + // NOTE: Any IBC application state changes made in this callback handler are not committed. OnChanUpgradeTry( ctx sdk.Context, portID, channelID string, @@ -134,6 +136,7 @@ type UpgradableModule interface { // OnChanUpgradeAck enables additional custom logic to be executed in the ChannelUpgradeAck step of the // channel upgrade handshake. It must validate the version proposed by the counterparty. + // NOTE: Any IBC application state changes made in this callback handler are not committed. OnChanUpgradeAck( ctx sdk.Context, portID, From 622bb96e45d0c9cfde3cad57cf6a3d87a7985d57 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 23 Jan 2024 17:33:52 +0100 Subject: [PATCH 06/10] test: adding unit tests for discarding app state changes --- modules/core/keeper/msg_server_test.go | 91 ++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index c09f9622437..3145146f4f2 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -23,6 +23,7 @@ import ( "github.com/cosmos/ibc-go/v8/modules/core/keeper" ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v8/testing" + "github.com/cosmos/ibc-go/v8/testing/mock" ibcmock "github.com/cosmos/ibc-go/v8/testing/mock" ) @@ -990,6 +991,41 @@ func (suite *KeeperTestSuite) TestChannelUpgradeInit() { suite.Require().Empty(events) }, }, + { + "ibc application does not commit state changes in callback", + func() { + msg = channeltypes.NewMsgChannelUpgradeInit( + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + path.EndpointA.GetProposedUpgrade().Fields, + path.EndpointA.Chain.GetSimApp().IBCKeeper.GetAuthority(), + ) + + suite.chainA.GetSimApp().IBCMockModule.IBCApp.OnChanUpgradeInit = func(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) { + storeKey := suite.chainA.GetSimApp().GetKey(exported.ModuleName) + store := ctx.KVStore(storeKey) + store.Set([]byte("test-key"), []byte("test-value")) + + ctx.EventManager().EmitEvent(sdk.NewEvent("test-event", sdk.NewAttribute("k", "v"))) + return mock.UpgradeVersion, nil + } + }, + func(res *channeltypes.MsgChannelUpgradeInitResponse, events []abci.Event, err error) { + suite.Require().NoError(err) + suite.Require().NotNil(res) + suite.Require().Equal(uint64(1), res.UpgradeSequence) + + storeKey := suite.chainA.GetSimApp().GetKey(exported.ModuleName) + store := suite.chainA.GetContext().KVStore(storeKey) + suite.Require().Nil(store.Get([]byte("test-key"))) + + for _, event := range events { + if event.GetType() == "test-event" { + suite.Fail("expected application callback events to be discarded") + } + } + }, + }, } for _, tc := range cases { @@ -1128,6 +1164,34 @@ func (suite *KeeperTestSuite) TestChannelUpgradeTry() { suite.Require().Empty(events) }, }, + { + "ibc application does not commit state changes in callback", + func() { + suite.chainA.GetSimApp().IBCMockModule.IBCApp.OnChanUpgradeTry = func(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, counterpartyVersion string) (string, error) { + storeKey := suite.chainA.GetSimApp().GetKey(exported.ModuleName) + store := ctx.KVStore(storeKey) + store.Set([]byte("test-key"), []byte("test-value")) + + ctx.EventManager().EmitEvent(sdk.NewEvent("test-event", sdk.NewAttribute("k", "v"))) + return mock.UpgradeVersion, nil + } + }, + func(res *channeltypes.MsgChannelUpgradeTryResponse, events []abci.Event, err error) { + suite.Require().NoError(err) + suite.Require().NotNil(res) + suite.Require().Equal(uint64(1), res.UpgradeSequence) + + storeKey := suite.chainA.GetSimApp().GetKey(exported.ModuleName) + store := suite.chainA.GetContext().KVStore(storeKey) + suite.Require().Nil(store.Get([]byte("test-key"))) + + for _, event := range events { + if event.GetType() == "test-event" { + suite.Fail("expected application callback events to be discarded") + } + } + }, + }, } for _, tc := range cases { @@ -1393,6 +1457,33 @@ func (suite *KeeperTestSuite) TestChannelUpgradeAck() { suite.Require().Empty(events) }, }, + { + "ibc application does not commit state changes in callback", + func() { + suite.chainA.GetSimApp().IBCMockModule.IBCApp.OnChanUpgradeAck = func(ctx sdk.Context, portID, channelID, counterpartyVersion string) error { + storeKey := suite.chainA.GetSimApp().GetKey(exported.ModuleName) + store := ctx.KVStore(storeKey) + store.Set([]byte("test-key"), []byte("test-value")) + + ctx.EventManager().EmitEvent(sdk.NewEvent("test-event", sdk.NewAttribute("k", "v"))) + return nil + } + }, + func(res *channeltypes.MsgChannelUpgradeAckResponse, events []abci.Event, err error) { + suite.Require().NoError(err) + suite.Require().NotNil(res) + + storeKey := suite.chainA.GetSimApp().GetKey(exported.ModuleName) + store := suite.chainA.GetContext().KVStore(storeKey) + suite.Require().Nil(store.Get([]byte("test-key"))) + + for _, event := range events { + if event.GetType() == "test-event" { + suite.Fail("expected application callback events to be discarded") + } + } + }, + }, } for _, tc := range cases { From 0d59063a29bb3cc09a33ea38f571c2d67091a4ad Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 23 Jan 2024 17:38:30 +0100 Subject: [PATCH 07/10] chore: clean imports --- modules/core/keeper/msg_server_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index 3145146f4f2..326313ef5f6 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -23,7 +23,6 @@ import ( "github.com/cosmos/ibc-go/v8/modules/core/keeper" ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v8/testing" - "github.com/cosmos/ibc-go/v8/testing/mock" ibcmock "github.com/cosmos/ibc-go/v8/testing/mock" ) @@ -1007,7 +1006,7 @@ func (suite *KeeperTestSuite) TestChannelUpgradeInit() { store.Set([]byte("test-key"), []byte("test-value")) ctx.EventManager().EmitEvent(sdk.NewEvent("test-event", sdk.NewAttribute("k", "v"))) - return mock.UpgradeVersion, nil + return ibcmock.UpgradeVersion, nil } }, func(res *channeltypes.MsgChannelUpgradeInitResponse, events []abci.Event, err error) { @@ -1173,7 +1172,7 @@ func (suite *KeeperTestSuite) TestChannelUpgradeTry() { store.Set([]byte("test-key"), []byte("test-value")) ctx.EventManager().EmitEvent(sdk.NewEvent("test-event", sdk.NewAttribute("k", "v"))) - return mock.UpgradeVersion, nil + return ibcmock.UpgradeVersion, nil } }, func(res *channeltypes.MsgChannelUpgradeTryResponse, events []abci.Event, err error) { From 28f55b414d28f702327ba9b3b4c53f6267f2b3cc Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 24 Jan 2024 10:12:58 +0100 Subject: [PATCH 08/10] chore: review suggestions for in-line docstring comments --- modules/core/keeper/msg_server.go | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 374d885490b..b9624f18ddc 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -769,15 +769,9 @@ func (k Keeper) ChannelUpgradeInit(goCtx context.Context, msg *channeltypes.MsgC return nil, errorsmod.Wrap(err, "channel upgrade init failed") } - cacheCtx, _ := ctx.CacheContext() // NOTE: the writeFn is discarded and application state changes are not committed. - upgradeVersion, err := cbs.OnChanUpgradeInit( - cacheCtx, - msg.PortId, - msg.ChannelId, - upgrade.Fields.Ordering, - upgrade.Fields.ConnectionHops, - upgrade.Fields.Version, - ) + // NOTE: a cached context is used to discard ibc application state changes and events. + cacheCtx, _ := ctx.CacheContext() + upgradeVersion, err := cbs.OnChanUpgradeInit(cacheCtx, msg.PortId, msg.ChannelId, upgrade.Fields.Ordering, upgrade.Fields.ConnectionHops, upgrade.Fields.Version) if err != nil { ctx.Logger().Error("channel upgrade init callback failed", "port-id", msg.PortId, "channel-id", msg.ChannelId, "error", err.Error()) return nil, errorsmod.Wrapf(err, "channel upgrade init callback failed for port ID: %s, channel ID: %s", msg.PortId, msg.ChannelId) @@ -830,7 +824,8 @@ func (k Keeper) ChannelUpgradeTry(goCtx context.Context, msg *channeltypes.MsgCh return nil, errorsmod.Wrap(err, "channel upgrade try failed") } - cacheCtx, _ := ctx.CacheContext() // NOTE: the writeFn is discarded and application state changes are not committed. + // NOTE: a cached context is used to discard ibc application state changes and events. + cacheCtx, _ := ctx.CacheContext() upgradeVersion, err := cbs.OnChanUpgradeTry(cacheCtx, msg.PortId, msg.ChannelId, upgrade.Fields.Ordering, upgrade.Fields.ConnectionHops, upgrade.Fields.Version) if err != nil { ctx.Logger().Error("channel upgrade try callback failed", "port-id", msg.PortId, "channel-id", msg.ChannelId, "error", err.Error()) @@ -888,7 +883,8 @@ func (k Keeper) ChannelUpgradeAck(goCtx context.Context, msg *channeltypes.MsgCh return nil, errorsmod.Wrap(err, "channel upgrade ack failed") } - cacheCtx, _ := ctx.CacheContext() // NOTE: the writeFn is discarded and application state changes are not committed. + // NOTE: a cached context is used to discard ibc application state changes and events. + cacheCtx, _ := ctx.CacheContext() err = cbs.OnChanUpgradeAck(cacheCtx, msg.PortId, msg.ChannelId, msg.CounterpartyUpgrade.Fields.Version) if err != nil { ctx.Logger().Error("channel upgrade ack callback failed", "port-id", msg.PortId, "channel-id", msg.ChannelId, "error", err.Error()) From 55ee06e7acef70bc39e3e27d2c503b799056c049 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 24 Jan 2024 10:47:33 +0100 Subject: [PATCH 09/10] chore: fix linting tests, add mock types for testing kv and events --- modules/core/keeper/msg_server_test.go | 24 ++++++++++++------------ testing/mock/events.go | 1 + testing/mock/mock.go | 5 +++++ 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index 326313ef5f6..bd99426063d 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -1003,9 +1003,9 @@ func (suite *KeeperTestSuite) TestChannelUpgradeInit() { suite.chainA.GetSimApp().IBCMockModule.IBCApp.OnChanUpgradeInit = func(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) { storeKey := suite.chainA.GetSimApp().GetKey(exported.ModuleName) store := ctx.KVStore(storeKey) - store.Set([]byte("test-key"), []byte("test-value")) + store.Set(ibcmock.TestKey, ibcmock.TestValue) - ctx.EventManager().EmitEvent(sdk.NewEvent("test-event", sdk.NewAttribute("k", "v"))) + ctx.EventManager().EmitEvent(sdk.NewEvent(ibcmock.MockEventType)) return ibcmock.UpgradeVersion, nil } }, @@ -1016,10 +1016,10 @@ func (suite *KeeperTestSuite) TestChannelUpgradeInit() { storeKey := suite.chainA.GetSimApp().GetKey(exported.ModuleName) store := suite.chainA.GetContext().KVStore(storeKey) - suite.Require().Nil(store.Get([]byte("test-key"))) + suite.Require().Nil(store.Get(ibcmock.TestKey)) for _, event := range events { - if event.GetType() == "test-event" { + if event.GetType() == ibcmock.MockEventType { suite.Fail("expected application callback events to be discarded") } } @@ -1169,9 +1169,9 @@ func (suite *KeeperTestSuite) TestChannelUpgradeTry() { suite.chainA.GetSimApp().IBCMockModule.IBCApp.OnChanUpgradeTry = func(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, counterpartyVersion string) (string, error) { storeKey := suite.chainA.GetSimApp().GetKey(exported.ModuleName) store := ctx.KVStore(storeKey) - store.Set([]byte("test-key"), []byte("test-value")) + store.Set(ibcmock.TestKey, ibcmock.TestValue) - ctx.EventManager().EmitEvent(sdk.NewEvent("test-event", sdk.NewAttribute("k", "v"))) + ctx.EventManager().EmitEvent(sdk.NewEvent(ibcmock.MockEventType)) return ibcmock.UpgradeVersion, nil } }, @@ -1182,10 +1182,10 @@ func (suite *KeeperTestSuite) TestChannelUpgradeTry() { storeKey := suite.chainA.GetSimApp().GetKey(exported.ModuleName) store := suite.chainA.GetContext().KVStore(storeKey) - suite.Require().Nil(store.Get([]byte("test-key"))) + suite.Require().Nil(store.Get(ibcmock.TestKey)) for _, event := range events { - if event.GetType() == "test-event" { + if event.GetType() == ibcmock.MockEventType { suite.Fail("expected application callback events to be discarded") } } @@ -1462,9 +1462,9 @@ func (suite *KeeperTestSuite) TestChannelUpgradeAck() { suite.chainA.GetSimApp().IBCMockModule.IBCApp.OnChanUpgradeAck = func(ctx sdk.Context, portID, channelID, counterpartyVersion string) error { storeKey := suite.chainA.GetSimApp().GetKey(exported.ModuleName) store := ctx.KVStore(storeKey) - store.Set([]byte("test-key"), []byte("test-value")) + store.Set(ibcmock.TestKey, ibcmock.TestValue) - ctx.EventManager().EmitEvent(sdk.NewEvent("test-event", sdk.NewAttribute("k", "v"))) + ctx.EventManager().EmitEvent(sdk.NewEvent(ibcmock.MockEventType)) return nil } }, @@ -1474,10 +1474,10 @@ func (suite *KeeperTestSuite) TestChannelUpgradeAck() { storeKey := suite.chainA.GetSimApp().GetKey(exported.ModuleName) store := suite.chainA.GetContext().KVStore(storeKey) - suite.Require().Nil(store.Get([]byte("test-key"))) + suite.Require().Nil(store.Get(ibcmock.TestKey)) for _, event := range events { - if event.GetType() == "test-event" { + if event.GetType() == ibcmock.MockEventType { suite.Fail("expected application callback events to be discarded") } } diff --git a/testing/mock/events.go b/testing/mock/events.go index e92592c0326..124705962a5 100644 --- a/testing/mock/events.go +++ b/testing/mock/events.go @@ -3,6 +3,7 @@ package mock import sdk "github.com/cosmos/cosmos-sdk/types" const ( + MockEventType = "mock-event-type" MockEventTypeRecvPacket = "mock-recv-packet" MockEventTypeAckPacket = "mock-ack-packet" MockEventTypeTimeoutPacket = "mock-timeout" diff --git a/testing/mock/mock.go b/testing/mock/mock.go index 86de269f48b..1d90491f43e 100644 --- a/testing/mock/mock.go +++ b/testing/mock/mock.go @@ -49,6 +49,11 @@ var ( MockApplicationCallbackError error = &applicationCallbackError{} ) +var ( + TestKey = []byte("test-key") + TestValue = []byte("test-value") +) + var ( _ module.AppModuleBasic = (*AppModuleBasic)(nil) _ appmodule.AppModule = (*AppModule)(nil) From 2857ac31e676e3a8a993df7c71ca5532cb56f6a2 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 24 Jan 2024 10:52:48 +0100 Subject: [PATCH 10/10] doc: add more context to in-line docstrings --- modules/core/keeper/msg_server.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index b9624f18ddc..e1f9a3c4a71 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -770,6 +770,7 @@ func (k Keeper) ChannelUpgradeInit(goCtx context.Context, msg *channeltypes.MsgC } // NOTE: a cached context is used to discard ibc application state changes and events. + // IBC applications must flush in-flight packets using the pre-upgrade channel parameters. cacheCtx, _ := ctx.CacheContext() upgradeVersion, err := cbs.OnChanUpgradeInit(cacheCtx, msg.PortId, msg.ChannelId, upgrade.Fields.Ordering, upgrade.Fields.ConnectionHops, upgrade.Fields.Version) if err != nil { @@ -825,6 +826,7 @@ func (k Keeper) ChannelUpgradeTry(goCtx context.Context, msg *channeltypes.MsgCh } // NOTE: a cached context is used to discard ibc application state changes and events. + // IBC applications must flush in-flight packets using the pre-upgrade channel parameters. cacheCtx, _ := ctx.CacheContext() upgradeVersion, err := cbs.OnChanUpgradeTry(cacheCtx, msg.PortId, msg.ChannelId, upgrade.Fields.Ordering, upgrade.Fields.ConnectionHops, upgrade.Fields.Version) if err != nil { @@ -884,6 +886,7 @@ func (k Keeper) ChannelUpgradeAck(goCtx context.Context, msg *channeltypes.MsgCh } // NOTE: a cached context is used to discard ibc application state changes and events. + // IBC applications must flush in-flight packets using the pre-upgrade channel parameters. cacheCtx, _ := ctx.CacheContext() err = cbs.OnChanUpgradeAck(cacheCtx, msg.PortId, msg.ChannelId, msg.CounterpartyUpgrade.Fields.Version) if err != nil {