diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index b0b6bd6767c..b7c65874bfa 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -50,7 +50,8 @@ func (im IBCMiddleware) OnChanOpenInit( return "", types.ErrControllerSubModuleDisabled } - if err := im.keeper.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version); err != nil { + version, err := im.keeper.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version) + if err != nil { return "", err } 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 1c3660c54b5..7656c665333 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -130,6 +130,18 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() { { "success", func() {}, true, }, + { + "ICA auth module modification of channel version is ignored", func() { + // NOTE: explicitly modify the channel version via the auth module callback, + // ensuring the expected JSON encoded metadata is not modified upon return + 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, + ) (string, error) { + return "invalid-version", nil + } + }, true, + }, { "controller submodule disabled", func() { suite.chainA.GetSimApp().ICAControllerKeeper.SetParams(suite.chainA.GetContext(), types.NewParams(false)) @@ -200,19 +212,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() { ) 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().Equal(icatypes.NewDefaultMetadataString(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID), version) suite.Require().NoError(err) } else { suite.Require().Error(err) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go index 6b84fb554ca..c9f72ae3380 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go @@ -28,26 +28,35 @@ func (k Keeper) OnChanOpenInit( chanCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string, -) error { +) (string, error) { if order != channeltypes.ORDERED { - return sdkerrors.Wrapf(channeltypes.ErrInvalidChannelOrdering, "expected %s channel, got %s", channeltypes.ORDERED, order) + return "", sdkerrors.Wrapf(channeltypes.ErrInvalidChannelOrdering, "expected %s channel, got %s", channeltypes.ORDERED, order) } if !strings.HasPrefix(portID, icatypes.PortPrefix) { - return sdkerrors.Wrapf(icatypes.ErrInvalidControllerPort, "expected %s{owner-account-address}, got %s", icatypes.PortPrefix, portID) + return "", sdkerrors.Wrapf(icatypes.ErrInvalidControllerPort, "expected %s{owner-account-address}, got %s", icatypes.PortPrefix, portID) } if counterparty.PortId != icatypes.PortID { - return sdkerrors.Wrapf(icatypes.ErrInvalidHostPort, "expected %s, got %s", icatypes.PortID, counterparty.PortId) + return "", sdkerrors.Wrapf(icatypes.ErrInvalidHostPort, "expected %s, got %s", icatypes.PortID, counterparty.PortId) } var metadata icatypes.Metadata - if err := icatypes.ModuleCdc.UnmarshalJSON([]byte(version), &metadata); err != nil { - return sdkerrors.Wrapf(icatypes.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata") + if strings.TrimSpace(version) == "" { + connection, err := k.channelKeeper.GetConnection(ctx, connectionHops[0]) + if err != nil { + return "", err + } + + metadata = icatypes.NewDefaultMetadata(connectionHops[0], connection.GetCounterparty().GetConnectionID()) + } else { + if err := icatypes.ModuleCdc.UnmarshalJSON([]byte(version), &metadata); err != nil { + return "", sdkerrors.Wrapf(icatypes.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata") + } } if err := icatypes.ValidateControllerMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil { - return err + return "", err } activeChannelID, found := k.GetActiveChannelID(ctx, connectionHops[0], portID) @@ -58,15 +67,15 @@ func (k Keeper) OnChanOpenInit( } if channel.State == channeltypes.OPEN { - return sdkerrors.Wrapf(icatypes.ErrActiveChannelAlreadySet, "existing active channel %s for portID %s is already OPEN", activeChannelID, portID) + return "", sdkerrors.Wrapf(icatypes.ErrActiveChannelAlreadySet, "existing active channel %s for portID %s is already OPEN", activeChannelID, portID) } if !icatypes.IsPreviousMetadataEqual(channel.Version, metadata) { - return sdkerrors.Wrap(icatypes.ErrInvalidVersion, "previous active channel metadata does not match provided version") + return "", sdkerrors.Wrap(icatypes.ErrInvalidVersion, "previous active channel metadata does not match provided version") } } - return nil + return string(icatypes.ModuleCdc.MustMarshalJSON(&metadata)), nil } // OnChanOpenAck sets the active channel for the interchain account/owner pair diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go index f2104e2032d..b0cb9c3a125 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -30,7 +30,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { true, }, { - "success - previous active channel closed", + "success: previous active channel closed", func() { suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) @@ -47,6 +47,13 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { }, true, }, + { + "success: empty channel version returns default metadata JSON string", + func() { + channel.Version = "" + }, + true, + }, { "invalid metadata - previous metadata is different", func() { @@ -138,6 +145,14 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { }, false, }, + { + "connection not found with default empty channel version", + func() { + channel.ConnectionHops = []string{"connection-10"} + channel.Version = "" + }, + false, + }, { "invalid controller connection ID", func() { @@ -214,7 +229,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { path.EndpointA.ChannelConfig.PortID = portID // default values - metadata = icatypes.NewMetadata(icatypes.Version, ibctesting.FirstConnectionID, ibctesting.FirstConnectionID, "", icatypes.EncodingProtobuf, icatypes.TxTypeSDKMultiMsg) + metadata = icatypes.NewMetadata(icatypes.Version, path.EndpointA.ConnectionID, path.EndpointB.ConnectionID, "", icatypes.EncodingProtobuf, icatypes.TxTypeSDKMultiMsg) versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) suite.Require().NoError(err) @@ -232,12 +247,13 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { tc.malleate() // malleate mutates test data - err = suite.chainA.GetSimApp().ICAControllerKeeper.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), + version, err := suite.chainA.GetSimApp().ICAControllerKeeper.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, channel.Counterparty, channel.Version, ) if tc.expPass { suite.Require().NoError(err) + suite.Require().Equal(string(versionBytes), version) } else { suite.Require().Error(err) } diff --git a/modules/apps/27-interchain-accounts/types/metadata.go b/modules/apps/27-interchain-accounts/types/metadata.go index 15f27314d47..dd14c4788ec 100644 --- a/modules/apps/27-interchain-accounts/types/metadata.go +++ b/modules/apps/27-interchain-accounts/types/metadata.go @@ -27,17 +27,25 @@ func NewMetadata(version, controllerConnectionID, hostConnectionID, accAddress, } } -// NewDefaultMetadataString creates and returns a new JSON encoded version string containing the default ICS27 Metadata values +// NewDefaultMetadata creates and returns a new ICS27 Metadata instance containing the default ICS27 Metadata values // with the provided controller and host connection identifiers -func NewDefaultMetadataString(controllerConnectionID, hostConnectionID string) string { +func NewDefaultMetadata(connectionConnectionID, hostConnectionID string) Metadata { metadata := Metadata{ - Version: Version, - ControllerConnectionId: controllerConnectionID, + ControllerConnectionId: connectionConnectionID, HostConnectionId: hostConnectionID, Encoding: EncodingProtobuf, TxType: TxTypeSDKMultiMsg, + Version: Version, } + return metadata +} + +// NewDefaultMetadataString creates and returns a new JSON encoded version string containing the default ICS27 Metadata values +// with the provided controller and host connection identifiers +func NewDefaultMetadataString(controllerConnectionID, hostConnectionID string) string { + metadata := NewDefaultMetadata(controllerConnectionID, hostConnectionID) + return string(ModuleCdc.MustMarshalJSON(&metadata)) }