From 570a56239222d1ab4a14090319ff1ade84289813 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 27 Jul 2022 14:06:41 +0200 Subject: [PATCH 1/6] remove previous channel id from NewMsgChanOpenTry, assert previous channel id to be empty --- .../controller/ibc_middleware_test.go | 2 +- modules/core/04-channel/types/msgs.go | 7 ++-- modules/core/04-channel/types/msgs_test.go | 36 +++++++++---------- testing/endpoint.go | 2 +- 4 files changed, 21 insertions(+), 26 deletions(-) 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 b326c0bb71f..a8b5fcc7094 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -244,7 +244,7 @@ func (suite *InterchainAccountsTestSuite) TestChanOpenTry() { proofInit, proofHeight := path.EndpointB.Chain.QueryProof(channelKey) // use chainA (controller) for ChanOpenTry - msg := channeltypes.NewMsgChannelOpenTry(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, TestVersion, channeltypes.ORDERED, []string{path.EndpointA.ConnectionID}, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, TestVersion, proofInit, proofHeight, icatypes.ModuleName) + msg := channeltypes.NewMsgChannelOpenTry(path.EndpointA.ChannelConfig.PortID, TestVersion, channeltypes.ORDERED, []string{path.EndpointA.ConnectionID}, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, TestVersion, proofInit, proofHeight, icatypes.ModuleName) handler := suite.chainA.GetSimApp().MsgServiceRouter().Handler(msg) _, err = handler(suite.chainA.GetContext(), msg) diff --git a/modules/core/04-channel/types/msgs.go b/modules/core/04-channel/types/msgs.go index 107afa08594..4ae18c5fe68 100644 --- a/modules/core/04-channel/types/msgs.go +++ b/modules/core/04-channel/types/msgs.go @@ -66,7 +66,7 @@ var _ sdk.Msg = &MsgChannelOpenTry{} // It is left as an argument for go API backwards compatibility. // nolint:interfacer func NewMsgChannelOpenTry( - portID, previousChannelID, version string, channelOrder Order, connectionHops []string, + portID, version string, channelOrder Order, connectionHops []string, counterpartyPortID, counterpartyChannelID, counterpartyVersion string, proofInit []byte, proofHeight clienttypes.Height, signer string, ) *MsgChannelOpenTry { @@ -74,7 +74,6 @@ func NewMsgChannelOpenTry( channel := NewChannel(TRYOPEN, channelOrder, counterparty, connectionHops, version) return &MsgChannelOpenTry{ PortId: portID, - PreviousChannelId: previousChannelID, Channel: channel, CounterpartyVersion: counterpartyVersion, ProofInit: proofInit, @@ -89,9 +88,7 @@ func (msg MsgChannelOpenTry) ValidateBasic() error { return sdkerrors.Wrap(err, "invalid port ID") } if msg.PreviousChannelId != "" { - if !IsValidChannelID(msg.PreviousChannelId) { - return sdkerrors.Wrap(ErrInvalidChannelIdentifier, "invalid previous channel ID") - } + return sdkerrors.Wrap(ErrInvalidChannelIdentifier, "previous channel identifier must be empty, this field has been deprecated as crossing hellos are no longer supported") } if len(msg.ProofInit) == 0 { return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty proof init") diff --git a/modules/core/04-channel/types/msgs_test.go b/modules/core/04-channel/types/msgs_test.go index dda7668e365..f35dfab3ed3 100644 --- a/modules/core/04-channel/types/msgs_test.go +++ b/modules/core/04-channel/types/msgs_test.go @@ -151,25 +151,23 @@ func (suite *TypesTestSuite) TestMsgChannelOpenTryValidateBasic() { msg *types.MsgChannelOpenTry expPass bool }{ - {"", types.NewMsgChannelOpenTry(portid, chanid, version, types.ORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), true}, - {"too short port id", types.NewMsgChannelOpenTry(invalidShortPort, chanid, version, types.ORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, - {"too long port id", types.NewMsgChannelOpenTry(invalidLongPort, chanid, version, types.ORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, - {"port id contains non-alpha", types.NewMsgChannelOpenTry(invalidPort, chanid, version, types.ORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, - {"too short channel id", types.NewMsgChannelOpenTry(portid, invalidShortChannel, version, types.ORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, - {"too long channel id", types.NewMsgChannelOpenTry(portid, invalidLongChannel, version, types.ORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, - {"channel id contains non-alpha", types.NewMsgChannelOpenTry(portid, invalidChannel, version, types.ORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, - {"", types.NewMsgChannelOpenTry(portid, chanid, version, types.ORDERED, connHops, cpportid, cpchanid, "", suite.proof, height, addr), true}, - {"proof height is zero", types.NewMsgChannelOpenTry(portid, chanid, version, types.ORDERED, connHops, cpportid, cpchanid, version, suite.proof, clienttypes.ZeroHeight(), addr), false}, - {"invalid channel order", types.NewMsgChannelOpenTry(portid, chanid, version, types.Order(4), connHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, - {"connection hops more than 1 ", types.NewMsgChannelOpenTry(portid, chanid, version, types.UNORDERED, invalidConnHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, - {"too short connection id", types.NewMsgChannelOpenTry(portid, chanid, version, types.UNORDERED, invalidShortConnHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, - {"too long connection id", types.NewMsgChannelOpenTry(portid, chanid, version, types.UNORDERED, invalidLongConnHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, - {"connection id contains non-alpha", types.NewMsgChannelOpenTry(portid, chanid, version, types.UNORDERED, []string{invalidConnection}, cpportid, cpchanid, version, suite.proof, height, addr), false}, - {"", types.NewMsgChannelOpenTry(portid, chanid, "", types.UNORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), true}, - {"invalid counterparty port id", types.NewMsgChannelOpenTry(portid, chanid, version, types.UNORDERED, connHops, invalidPort, cpchanid, version, suite.proof, height, addr), false}, - {"invalid counterparty channel id", types.NewMsgChannelOpenTry(portid, chanid, version, types.UNORDERED, connHops, cpportid, invalidChannel, version, suite.proof, height, addr), false}, - {"empty proof", types.NewMsgChannelOpenTry(portid, chanid, version, types.UNORDERED, connHops, cpportid, cpchanid, version, emptyProof, height, addr), false}, - {"channel not in TRYOPEN state", &types.MsgChannelOpenTry{portid, chanid, initChannel, version, suite.proof, height, addr}, false}, + {"", types.NewMsgChannelOpenTry(portid, version, types.ORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), true}, + {"too short port id", types.NewMsgChannelOpenTry(invalidShortPort, version, types.ORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, + {"too long port id", types.NewMsgChannelOpenTry(invalidLongPort, version, types.ORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, + {"port id contains non-alpha", types.NewMsgChannelOpenTry(invalidPort, version, types.ORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, + {"", types.NewMsgChannelOpenTry(portid, version, types.ORDERED, connHops, cpportid, cpchanid, "", suite.proof, height, addr), true}, + {"proof height is zero", types.NewMsgChannelOpenTry(portid, version, types.ORDERED, connHops, cpportid, cpchanid, version, suite.proof, clienttypes.ZeroHeight(), addr), false}, + {"invalid channel order", types.NewMsgChannelOpenTry(portid, version, types.Order(4), connHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, + {"connection hops more than 1 ", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, invalidConnHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, + {"too short connection id", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, invalidShortConnHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, + {"too long connection id", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, invalidLongConnHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, + {"connection id contains non-alpha", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, []string{invalidConnection}, cpportid, cpchanid, version, suite.proof, height, addr), false}, + {"", types.NewMsgChannelOpenTry(portid, "", types.UNORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), true}, + {"invalid counterparty port id", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, connHops, invalidPort, cpchanid, version, suite.proof, height, addr), false}, + {"invalid counterparty channel id", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, connHops, cpportid, invalidChannel, version, suite.proof, height, addr), false}, + {"empty proof", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, connHops, cpportid, cpchanid, version, emptyProof, height, addr), false}, + {"channel not in TRYOPEN state", &types.MsgChannelOpenTry{portid, "", initChannel, version, suite.proof, height, addr}, false}, + {"previous channel id is not empty", &types.MsgChannelOpenTry{portid, chanid, initChannel, version, suite.proof, height, addr}, false}, } for _, tc := range testCases { diff --git a/testing/endpoint.go b/testing/endpoint.go index ce8895ebb51..ba81708e56e 100644 --- a/testing/endpoint.go +++ b/testing/endpoint.go @@ -295,7 +295,7 @@ func (endpoint *Endpoint) ChanOpenTry() error { proof, height := endpoint.Counterparty.Chain.QueryProof(channelKey) msg := channeltypes.NewMsgChannelOpenTry( - endpoint.ChannelConfig.PortID, "", // does not support handshake continuation + endpoint.ChannelConfig.PortID, endpoint.ChannelConfig.Version, endpoint.ChannelConfig.Order, []string{endpoint.ConnectionID}, endpoint.Counterparty.ChannelConfig.PortID, endpoint.Counterparty.ChannelID, endpoint.Counterparty.ChannelConfig.Version, proof, height, From 3891be43bcec07f7a874406d186cad88603742cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 27 Jul 2022 14:09:47 +0200 Subject: [PATCH 2/6] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6739c792235..358c1e840f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -97,6 +97,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* (core/04-channel) [\#1792](https://github.com/cosmos/ibc-go/pull/1792) Remove `PreviousChannelID` from `NewMsgChanOpenTry` arguments. `MsgChanOpenTry.ValidateBasic()` returns error if the deprecated `PreviousChannelID` is not empty. * (transfer) [\#1342](https://github.com/cosmos/ibc-go/pull/1342) `DenomTrace` grpc now takes in either an `ibc denom` or a `hash` instead of only accepting a `hash`. * (modules/core/04-channel) [\#1160](https://github.com/cosmos/ibc-go/pull/1160) Improve `uint64 -> string` performance in `Logger`. * (modules/core/04-channel) [\#1279](https://github.com/cosmos/ibc-go/pull/1279) Add selected channel version to MsgChanOpenInitResponse and MsgChanOpenTryResponse. Emit channel version during OpenInit/OpenTry From 9a440e504dbcc657a60dfccdd54e5448f05d6efe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 27 Jul 2022 16:18:54 +0200 Subject: [PATCH 3/6] move changelog entry to correct location --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 358c1e840f5..107f755aed2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking +* (core/04-channel) [\#1792](https://github.com/cosmos/ibc-go/pull/1792) Remove `PreviousChannelID` from `NewMsgChanOpenTry` arguments. `MsgChanOpenTry.ValidateBasic()` returns error if the deprecated `PreviousChannelID` is not empty. * (modules/core/03-connection) [\#1672](https://github.com/cosmos/ibc-go/pull/1672) Remove crossing hellos from connection handshakes. The `PreviousConnectionId` in `MsgConnectionOpenTry` has been deprecated. * (modules/core/04-channel) [\#1317](https://github.com/cosmos/ibc-go/pull/1317) Remove crossing hellos from channel handshakes. The `PreviousChannelId` in `MsgChannelOpenTry` has been deprecated. * (transfer) [\#1250](https://github.com/cosmos/ibc-go/pull/1250) Deprecate `GetTransferAccount` since the `transfer` module account is never used. @@ -97,7 +98,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements -* (core/04-channel) [\#1792](https://github.com/cosmos/ibc-go/pull/1792) Remove `PreviousChannelID` from `NewMsgChanOpenTry` arguments. `MsgChanOpenTry.ValidateBasic()` returns error if the deprecated `PreviousChannelID` is not empty. * (transfer) [\#1342](https://github.com/cosmos/ibc-go/pull/1342) `DenomTrace` grpc now takes in either an `ibc denom` or a `hash` instead of only accepting a `hash`. * (modules/core/04-channel) [\#1160](https://github.com/cosmos/ibc-go/pull/1160) Improve `uint64 -> string` performance in `Logger`. * (modules/core/04-channel) [\#1279](https://github.com/cosmos/ibc-go/pull/1279) Add selected channel version to MsgChanOpenInitResponse and MsgChanOpenTryResponse. Emit channel version during OpenInit/OpenTry From ddba6dba72e3eb0bb725dfca6fe0148124f903ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 28 Jul 2022 11:38:50 +0200 Subject: [PATCH 4/6] add migration docs --- docs/migrations/v3-to-v4.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/migrations/v3-to-v4.md b/docs/migrations/v3-to-v4.md index bdf8654dcb7..d60ca2a9e6e 100644 --- a/docs/migrations/v3-to-v4.md +++ b/docs/migrations/v3-to-v4.md @@ -63,6 +63,8 @@ Crossing hellos have been removed from 04-channel handshake negotiation. IBC Applications no longer need to account from already claimed capabilities in the `OnChanOpenTry` callback. The capability provided by core IBC must be able to be claimed with error. `PreviousChannelId` in `MsgChannelOpenTry` has been deprecated and is no longer used by core IBC. +`NewMsgChanOpenTry` no longer takes in the `PreviousChannelId` as crossing hellos are no longer supported. A non-empty `PreviousChannelId` will fail basic validation for this message. + ### ICS27 - Interchain Accounts The `RegisterInterchainAccount` API has been modified to include an additional `version` argument. This change has been made in order to support ICS29 fee middleware, for relayer incentivization of ICS27 packets. From ba6394b839ba3569c94f8c3418f8bd8e9c31b59f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 28 Jul 2022 18:16:08 +0200 Subject: [PATCH 5/6] Update docs/migrations/v3-to-v4.md Co-authored-by: Carlos Rodriguez --- docs/migrations/v3-to-v4.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/migrations/v3-to-v4.md b/docs/migrations/v3-to-v4.md index d60ca2a9e6e..ec75d3be569 100644 --- a/docs/migrations/v3-to-v4.md +++ b/docs/migrations/v3-to-v4.md @@ -63,7 +63,7 @@ Crossing hellos have been removed from 04-channel handshake negotiation. IBC Applications no longer need to account from already claimed capabilities in the `OnChanOpenTry` callback. The capability provided by core IBC must be able to be claimed with error. `PreviousChannelId` in `MsgChannelOpenTry` has been deprecated and is no longer used by core IBC. -`NewMsgChanOpenTry` no longer takes in the `PreviousChannelId` as crossing hellos are no longer supported. A non-empty `PreviousChannelId` will fail basic validation for this message. +`NewMsgChannelOpenTry` no longer takes in the `PreviousChannelId` as crossing hellos are no longer supported. A non-empty `PreviousChannelId` will fail basic validation for this message. ### ICS27 - Interchain Accounts From 23ee89446cbdea4d242fa1971b115af6e1c232e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 28 Jul 2022 18:16:18 +0200 Subject: [PATCH 6/6] Update CHANGELOG.md Co-authored-by: Carlos Rodriguez --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 878512c1d57..a99e0de4416 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,7 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking -* (core/04-channel) [\#1792](https://github.com/cosmos/ibc-go/pull/1792) Remove `PreviousChannelID` from `NewMsgChanOpenTry` arguments. `MsgChanOpenTry.ValidateBasic()` returns error if the deprecated `PreviousChannelID` is not empty. +* (core/04-channel) [\#1792](https://github.com/cosmos/ibc-go/pull/1792) Remove `PreviousChannelID` from `NewMsgChannelOpenTry` arguments. `MsgChannelOpenTry.ValidateBasic()` returns error if the deprecated `PreviousChannelID` is not empty. * (core/04-channel) [\#1418](https://github.com/cosmos/ibc-go/pull/1418) `NewPacketId` has been renamed to `NewPacketID` to comply with go linting rules. * (core/ante) [\#1418](https://github.com/cosmos/ibc-go/pull/1418) `AnteDecorator` has been renamed to `RedundancyDecorator` to comply with go linting rules and to give more clarity to the purpose of the Decorator. * (testing) [\#1418](https://github.com/cosmos/ibc-go/pull/1418) `MockIBCApp` has been renamed to `IBCApp` and `MockEmptyAcknowledgement` has been renamed to `EmptyAcknowledgement` to comply with go linting rules