Skip to content

Commit

Permalink
fix: assert previous channel identifer is empty in Msg ValidateBasic …
Browse files Browse the repository at this point in the history 
…(backport cosmos#1792) (cosmos#1827)

* fix: assert previous channel identifer is empty in Msg ValidateBasic (cosmos#1792)

* remove previous channel id from NewMsgChanOpenTry, assert previous channel id to be empty

* add changelog entry

* move changelog entry to correct location

* add migration docs

* Update docs/migrations/v3-to-v4.md

Co-authored-by: Carlos Rodriguez <[email protected]>

* Update CHANGELOG.md

Co-authored-by: Carlos Rodriguez <[email protected]>

Co-authored-by: Carlos Rodriguez <[email protected]>
(cherry picked from commit 4a781e5)

# Conflicts:
#	CHANGELOG.md

* fix conflicts

Co-authored-by: colin axnér <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
  • Loading branch information
@mergify @colin-axner
@crodriguezvega @ulbqb
3 people authored and ulbqb committed Jul 31, 2023
1 parent 1f26752 commit bd29d36
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 26 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 `NewMsgChannelOpenTry` arguments. `MsgChannelOpenTry.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.
Expand Down
2 changes: 2 additions & 0 deletions docs/migrations/v3-to-v4.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.

`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

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.
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
7 changes: 2 additions & 5 deletions modules/core/04-channel/types/msgs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,14 @@ 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 {
counterparty := NewCounterparty(counterpartyPortID, counterpartyChannelID)
channel := NewChannel(TRYOPEN, channelOrder, counterparty, connectionHops, version)
return &MsgChannelOpenTry{
PortId: portID,
PreviousChannelId: previousChannelID,
Channel: channel,
CounterpartyVersion: counterpartyVersion,
ProofInit: proofInit,
Expand All @@ -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")
Expand Down
36 changes: 17 additions & 19 deletions modules/core/04-channel/types/msgs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,25 +153,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 {
Expand Down
2 changes: 1 addition & 1 deletion testing/endpoint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit bd29d36

Please sign in to comment.