From dcd0681d8f07c624f53b9a9ffe9de2f122486207 Mon Sep 17 00:00:00 2001 From: Sean King Date: Tue, 10 May 2022 17:27:59 +0200 Subject: [PATCH 1/2] refactor: channel handshake version improvements (#1283) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: returning version from OnChanOpenInit * refactor: update tests and add version to proto resp * refactor: adding version to msg server resp * refactor: remove unncessary if & update version on Endpoint.Ack * fix: ics29 OnChanOpenInit remake versionMetaData before returning * chore: update godoc * test: adding check for expected version string * test: adding test case for passing empty version string to ics20 onChanOpenInit * Update modules/apps/29-fee/ibc_module.go Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> * chore: comment * chore: changelog * fix: ica now discards auth module version * chore: update changelog * adding default version for ics29 * fix: using transfer module directly rather than calling full middleware stack * fix testing bug * refactor: test now uses bool for isFeeEnabled rather than direct check * docs: updating comment and migration docs Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- CHANGELOG.md | 1 + docs/migrations/v3-to-v4.md | 6 ++- .../controller/ibc_middleware.go | 17 ++++--- .../controller/ibc_middleware_test.go | 19 ++++++-- .../27-interchain-accounts/host/ibc_module.go | 4 +- modules/apps/29-fee/ibc_middleware.go | 44 ++++++++++++++----- modules/apps/29-fee/ibc_middleware_test.go | 42 +++++++++++++++--- modules/apps/29-fee/transfer_test.go | 1 - modules/apps/transfer/ibc_module.go | 15 ++++--- modules/apps/transfer/ibc_module_test.go | 18 +++++--- modules/core/05-port/types/module.go | 16 ++++--- modules/core/keeper/msg_server.go | 7 +-- testing/endpoint.go | 4 ++ testing/mock/ibc_app.go | 2 +- testing/mock/ibc_module.go | 12 +++-- 15 files changed, 151 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b38ee315afc..7968f31ea84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking * (transfer) [\#1250](https://github.com/cosmos/ibc-go/pull/1250) Deprecate `GetTransferAccount` since the `transfer` module account is never used. +* (channel) [\#1283](https://github.com/cosmos/ibc-go/pull/1283) The `OnChanOpenInit` application callback now returns a version string in line with the latest [spec changes](https://github.com/cosmos/ibc/pull/629). * (modules/29-fee)[\#1338](https://github.com/cosmos/ibc-go/pull/1338) Renaming `Result` field in `IncentivizedAcknowledgement` to `AppAcknowledgement`. * (modules/29-fee)[\#1343](https://github.com/cosmos/ibc-go/pull/1343) Renaming `KeyForwardRelayerAddress` to `KeyRelayerAddressForAsyncAck`, and `ParseKeyForwardRelayerAddress` to `ParseKeyRelayerAddressForAsyncAck`. diff --git a/docs/migrations/v3-to-v4.md b/docs/migrations/v3-to-v4.md index 90e9af256d2..5a3d61deaae 100644 --- a/docs/migrations/v3-to-v4.md +++ b/docs/migrations/v3-to-v4.md @@ -1,4 +1,4 @@ -# Migrating from ibc-go v2 to v3 +# Migrating from ibc-go v3 to v4 This document is intended to highlight significant changes which may require more information than presented in the CHANGELOG. Any changes that must be done by a user of ibc-go should be documented here. @@ -23,4 +23,8 @@ No genesis or in-place migrations required when upgrading from v1 or v2 of ibc-g The `WriteAcknowledgement` API now takes the `exported.Acknowledgement` type instead of passing in the acknowledgement byte array directly. This is an API breaking change and as such IBC application developers will have to update any calls to `WriteAcknowledgement`. +The `OnChanOpenInit` application callback has been modified. +The return signature now includes the application version as detailed in the latest IBC [spec changes](https://github.com/cosmos/ibc/pull/629). + + diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index c12f5d3e9ce..9bf874fa8eb 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -45,18 +45,23 @@ func (im IBCMiddleware) OnChanOpenInit( chanCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string, -) error { +) (string, error) { if !im.keeper.IsControllerEnabled(ctx) { - return types.ErrControllerSubModuleDisabled + return "", types.ErrControllerSubModuleDisabled } if err := im.keeper.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version); err != nil { - return err + return "", err + } + + // call underlying app's OnChanOpenInit callback with the passed in version + // the version returned is discarded as the ica-auth module does not have permission to edit the version string. + // ics27 will always return the version string containing the Metadata struct which is created during the `RegisterInterchainAccount` call. + if _, err := im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version); err != nil { + return "", err } - // call underlying app's OnChanOpenInit callback with the appVersion - return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, - chanCap, counterparty, version) + return version, nil } // OnChanOpenTry implements the IBCMiddleware interface 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 ea1b5185af2..dd7849efca9 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -147,8 +147,8 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() { suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID, channelID string, chanCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string, - ) error { - return fmt.Errorf("mock ica auth fails") + ) (string, error) { + return "", fmt.Errorf("mock ica auth fails") } }, false, }, @@ -197,11 +197,24 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() { cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) suite.Require().True(ok) - err = cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), + version, err := cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, channel.Counterparty, channel.GetVersion(), ) if tc.expPass { + expMetadata := icatypes.NewMetadata( + icatypes.Version, + path.EndpointA.ConnectionID, + path.EndpointB.ConnectionID, + "", + icatypes.EncodingProtobuf, + icatypes.TxTypeSDKMultiMsg, + ) + + expBytes, err := icatypes.ModuleCdc.MarshalJSON(&expMetadata) + suite.Require().NoError(err) + + suite.Require().Equal(version, string(expBytes)) suite.Require().NoError(err) } else { suite.Require().Error(err) diff --git a/modules/apps/27-interchain-accounts/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go index fb403c71937..1598801601f 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module.go @@ -34,8 +34,8 @@ func (im IBCModule) OnChanOpenInit( chanCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string, -) error { - return sdkerrors.Wrap(icatypes.ErrInvalidChannelFlow, "channel handshake must be initiated by controller chain") +) (string, error) { + return "", sdkerrors.Wrap(icatypes.ErrInvalidChannelFlow, "channel handshake must be initiated by controller chain") } // OnChanOpenTry implements the IBCModule interface diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go index 96bb8be4030..3e6e7afb26d 100644 --- a/modules/apps/29-fee/ibc_middleware.go +++ b/modules/apps/29-fee/ibc_middleware.go @@ -1,6 +1,8 @@ package fee import ( + "strings" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" @@ -39,25 +41,44 @@ func (im IBCMiddleware) OnChanOpenInit( chanCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string, -) error { +) (string, error) { var versionMetadata types.Metadata - if err := types.ModuleCdc.UnmarshalJSON([]byte(version), &versionMetadata); err != nil { - // Since it is valid for fee version to not be specified, the above middleware version may be for a middleware - // lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying - // application. - return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, - chanCap, counterparty, version) + + if strings.TrimSpace(version) == "" { + // default version + versionMetadata = types.Metadata{ + FeeVersion: types.Version, + AppVersion: "", + } + } else { + if err := types.ModuleCdc.UnmarshalJSON([]byte(version), &versionMetadata); err != nil { + // Since it is valid for fee version to not be specified, the above middleware version may be for a middleware + // lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying + // application. + return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, + chanCap, counterparty, version) + } } if versionMetadata.FeeVersion != types.Version { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, versionMetadata.FeeVersion) + return "", sdkerrors.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, versionMetadata.FeeVersion) + } + + appVersion, err := im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, versionMetadata.AppVersion) + if err != nil { + return "", err + } + + versionMetadata.AppVersion = appVersion + versionBytes, err := types.ModuleCdc.MarshalJSON(&versionMetadata) + if err != nil { + return "", err } im.keeper.SetFeeEnabled(ctx, portID, channelID) // call underlying app's OnChanOpenInit callback with the appVersion - return im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, - chanCap, counterparty, versionMetadata.AppVersion) + return string(versionBytes), nil } // OnChanOpenTry implements the IBCMiddleware interface @@ -94,7 +115,6 @@ func (im IBCMiddleware) OnChanOpenTry( } versionMetadata.AppVersion = appVersion - versionBytes, err := types.ModuleCdc.MarshalJSON(&versionMetadata) if err != nil { return "", err @@ -116,7 +136,7 @@ func (im IBCMiddleware) OnChanOpenAck( if im.keeper.IsFeeEnabled(ctx, portID, channelID) { var versionMetadata types.Metadata if err := types.ModuleCdc.UnmarshalJSON([]byte(counterpartyVersion), &versionMetadata); err != nil { - return sdkerrors.Wrap(types.ErrInvalidVersion, "failed to unmarshal ICS29 counterparty version metadata") + return sdkerrors.Wrapf(err, "failed to unmarshal ICS29 counterparty version metadata: %s", counterpartyVersion) } if versionMetadata.FeeVersion != types.Version { diff --git a/modules/apps/29-fee/ibc_middleware_test.go b/modules/apps/29-fee/ibc_middleware_test.go index 8c76720ad3e..4b6d7f98959 100644 --- a/modules/apps/29-fee/ibc_middleware_test.go +++ b/modules/apps/29-fee/ibc_middleware_test.go @@ -25,34 +25,46 @@ var ( // Tests OnChanOpenInit on ChainA func (suite *FeeTestSuite) TestOnChanOpenInit() { testCases := []struct { - name string - version string - expPass bool + name string + version string + expPass bool + isFeeEnabled bool }{ { "success - valid fee middleware and mock version", string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: ibcmock.Version})), true, + true, }, { "success - fee version not included, only perform mock logic", ibcmock.Version, true, + false, }, { "invalid fee middleware version", string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: "invalid-ics29-1", AppVersion: ibcmock.Version})), false, + false, }, { "invalid mock version", string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: "invalid-mock-version"})), false, + false, }, { "mock version not wrapped", types.Version, false, + false, + }, + { + "passing an empty string returns default version", + "", + true, + true, }, } @@ -68,11 +80,11 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID, channelID string, chanCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string, - ) error { + ) (string, error) { if version != ibcmock.Version { - return fmt.Errorf("incorrect mock version") + return "", fmt.Errorf("incorrect mock version") } - return nil + return ibcmock.Version, nil } suite.path.EndpointA.ChannelID = ibctesting.FirstChannelID @@ -95,13 +107,29 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) suite.Require().True(ok) - err = cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), + version, err := cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, chanCap, counterparty, channel.Version) if tc.expPass { + // check if the channel is fee enabled. If so version string should include metaData + if tc.isFeeEnabled { + versionMetadata := types.Metadata{ + FeeVersion: types.Version, + AppVersion: ibcmock.Version, + } + + versionBytes, err := types.ModuleCdc.MarshalJSON(&versionMetadata) + suite.Require().NoError(err) + + suite.Require().Equal(version, string(versionBytes)) + } else { + suite.Require().Equal(ibcmock.Version, version) + } + suite.Require().NoError(err, "unexpected error from version: %s", tc.version) } else { suite.Require().Error(err, "error not returned for version: %s", tc.version) + suite.Require().Equal("", version) } }) } diff --git a/modules/apps/29-fee/transfer_test.go b/modules/apps/29-fee/transfer_test.go index 9d7557fd6c4..ba1f505d9c0 100644 --- a/modules/apps/29-fee/transfer_test.go +++ b/modules/apps/29-fee/transfer_test.go @@ -68,5 +68,4 @@ func (suite *FeeTestSuite) TestFeeTransfer() { fee.AckFee.Add(fee.TimeoutFee...), // ack fee paid, timeout fee refunded sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)).Sub(originalChainASenderAccountBalance), ) - } diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go index f5ed807d8b2..b9bfc2be189 100644 --- a/modules/apps/transfer/ibc_module.go +++ b/modules/apps/transfer/ibc_module.go @@ -3,6 +3,7 @@ package transfer import ( "fmt" "math" + "strings" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -70,21 +71,25 @@ func (im IBCModule) OnChanOpenInit( chanCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string, -) error { +) (string, error) { if err := ValidateTransferChannelParams(ctx, im.keeper, order, portID, channelID); err != nil { - return err + return "", err + } + + if strings.TrimSpace(version) == "" { + version = types.Version } if version != types.Version { - return sdkerrors.Wrapf(types.ErrInvalidVersion, "got %s, expected %s", version, types.Version) + return "", sdkerrors.Wrapf(types.ErrInvalidVersion, "got %s, expected %s", version, types.Version) } // Claim channel capability passed back by IBC module if err := im.keeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil { - return err + return "", err } - return nil + return version, nil } // OnChanOpenTry implements the IBCModule interface. diff --git a/modules/apps/transfer/ibc_module_test.go b/modules/apps/transfer/ibc_module_test.go index 3dcb5518cbb..95779ad2e8e 100644 --- a/modules/apps/transfer/ibc_module_test.go +++ b/modules/apps/transfer/ibc_module_test.go @@ -5,6 +5,7 @@ import ( capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" + "github.com/cosmos/ibc-go/v3/modules/apps/transfer" "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/v3/modules/core/24-host" @@ -28,6 +29,11 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() { { "success", func() {}, true, }, + { + "empty version string", func() { + channel.Version = "" + }, true, + }, { "max channels reached", func() { path.EndpointA.ChannelID = channeltypes.FormatChannelIdentifier(math.MaxUint32 + 1) @@ -74,25 +80,23 @@ func (suite *TransferTestSuite) TestOnChanOpenInit() { Version: types.Version, } - module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort) - suite.Require().NoError(err) - + var err error chanCap, err = suite.chainA.App.GetScopedIBCKeeper().NewCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(ibctesting.TransferPort, path.EndpointA.ChannelID)) suite.Require().NoError(err) - cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module) - suite.Require().True(ok) - tc.malleate() // explicitly change fields in channel and testChannel - err = cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), + transferModule := transfer.NewIBCModule(suite.chainA.GetSimApp().TransferKeeper) + version, err := transferModule.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, counterparty, channel.GetVersion(), ) if tc.expPass { suite.Require().NoError(err) + suite.Require().Equal(types.Version, version) } else { suite.Require().Error(err) + suite.Require().Equal(version, "") } }) diff --git a/modules/core/05-port/types/module.go b/modules/core/05-port/types/module.go index e6ba8f3449b..7ac57479dbd 100644 --- a/modules/core/05-port/types/module.go +++ b/modules/core/05-port/types/module.go @@ -11,10 +11,16 @@ import ( // IBCModule defines an interface that implements all the callbacks // that modules must define as specified in ICS-26 type IBCModule interface { - // OnChanOpenInit will verify that the relayer-chosen parameters are - // valid and perform any custom INIT logic.It may return an error if - // the chosen parameters are invalid in which case the handshake is aborted. - // OnChanOpenInit should return an error if the provided version is invalid. + // OnChanOpenInit will verify that the relayer-chosen parameters + // are valid and perform any custom INIT logic. + // It may return an error if the chosen parameters are invalid + // in which case the handshake is aborted. + // If the provided version string is non-empty, OnChanOpenInit should return + // the version string if valid or an error if the provided version is invalid. + // If the version string is empty, OnChanOpenInit is expected to + // return a default version string representing the version(s) it supports. + // If there is no default version string for the application, + // it should return an error if provided version is empty string. OnChanOpenInit( ctx sdk.Context, order channeltypes.Order, @@ -24,7 +30,7 @@ type IBCModule interface { channelCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string, - ) error + ) (string, error) // OnChanOpenTry will verify the relayer-chosen parameters along with the // counterparty-chosen version string and perform custom TRY logic. diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index e8b7c0fd254..a4aeb496f6f 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -185,16 +185,17 @@ func (k Keeper) ChannelOpenInit(goCtx context.Context, msg *channeltypes.MsgChan } // Perform application logic callback - if err = cbs.OnChanOpenInit(ctx, msg.Channel.Ordering, msg.Channel.ConnectionHops, msg.PortId, channelID, cap, msg.Channel.Counterparty, msg.Channel.Version); err != nil { + version, err := cbs.OnChanOpenInit(ctx, msg.Channel.Ordering, msg.Channel.ConnectionHops, msg.PortId, channelID, cap, msg.Channel.Counterparty, msg.Channel.Version) + if err != nil { return nil, sdkerrors.Wrap(err, "channel open init callback failed") } // Write channel into state - k.ChannelKeeper.WriteOpenInitChannel(ctx, msg.PortId, channelID, msg.Channel.Ordering, msg.Channel.ConnectionHops, msg.Channel.Counterparty, msg.Channel.Version) + k.ChannelKeeper.WriteOpenInitChannel(ctx, msg.PortId, channelID, msg.Channel.Ordering, msg.Channel.ConnectionHops, msg.Channel.Counterparty, version) return &channeltypes.MsgChannelOpenInitResponse{ ChannelId: channelID, - Version: msg.Channel.Version, + Version: version, }, nil } diff --git a/testing/endpoint.go b/testing/endpoint.go index 02c4e9aac39..94fa14f8b0d 100644 --- a/testing/endpoint.go +++ b/testing/endpoint.go @@ -279,6 +279,10 @@ func (endpoint *Endpoint) ChanOpenInit() error { endpoint.ChannelID, err = ParseChannelIDFromEvents(res.GetEvents()) require.NoError(endpoint.Chain.T, err) + // update version to selected app version + // NOTE: this update must be performed after SendMsgs() + endpoint.ChannelConfig.Version = endpoint.GetChannel().Version + return nil } diff --git a/testing/mock/ibc_app.go b/testing/mock/ibc_app.go index 77eb17b8c6f..0504c2df0de 100644 --- a/testing/mock/ibc_app.go +++ b/testing/mock/ibc_app.go @@ -23,7 +23,7 @@ type MockIBCApp struct { channelCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string, - ) error + ) (string, error) OnChanOpenTry func( ctx sdk.Context, diff --git a/testing/mock/ibc_module.go b/testing/mock/ibc_module.go index e58f6ae7156..69f3c388a8a 100644 --- a/testing/mock/ibc_module.go +++ b/testing/mock/ibc_module.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "strconv" + "strings" sdk "github.com/cosmos/cosmos-sdk/types" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" @@ -32,18 +33,21 @@ func NewIBCModule(appModule *AppModule, app *MockIBCApp) IBCModule { func (im IBCModule) OnChanOpenInit( ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID string, channelID string, chanCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string, -) error { +) (string, error) { + if strings.TrimSpace(version) == "" { + version = Version + } + if im.IBCApp.OnChanOpenInit != nil { return im.IBCApp.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version) - } // Claim channel capability passed back by IBC module if err := im.IBCApp.ScopedKeeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil { - return err + return "", err } - return nil + return version, nil } // OnChanOpenTry implements the IBCModule interface. From ab8ab42f45cf6ade6e08c44321aec82613344bf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 11 May 2022 12:28:19 +0200 Subject: [PATCH 2/2] fix: emit ics29 events on cacheCtx write after refunding on closed channel (#1344) ## Description closes: #1323 --- 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 --- modules/apps/29-fee/keeper/escrow.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/apps/29-fee/keeper/escrow.go b/modules/apps/29-fee/keeper/escrow.go index 0ca84684440..2c5b570320e 100644 --- a/modules/apps/29-fee/keeper/escrow.go +++ b/modules/apps/29-fee/keeper/escrow.go @@ -225,6 +225,9 @@ func (k Keeper) RefundFeesOnChannelClosure(ctx sdk.Context, portID, channelID st k.DeleteFeesInEscrow(cacheCtx, identifiedPacketFee.PacketId) } + // NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context. + ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events()) + // write the cache writeFn()