Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove crossings hello #1317

Merged
merged 24 commits into from
Jul 11, 2022
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4c2b39c
remove crossing hellos from ChanOpenTry
vuong177 Apr 30, 2022
5dac6fa
remove crossing hellos testcase
vuong177 Apr 30, 2022
f281033
revert single connectionHops check
vuong177 May 2, 2022
ef47c1c
add comment in MsgChannelOpenTry tx proto about deprecate field
vuong177 May 2, 2022
60195cc
Update proto/ibc/core/channel/v1/tx.proto
vuong177 May 3, 2022
b196ca8
minor fixes && UPDATE CHANGELOG.md
vuong177 May 3, 2022
7983209
Merge branch 'main' into remove-crossings-hello
vuong177 May 3, 2022
7890a9d
Update proto/ibc/core/channel/v1/tx.proto
vuong177 May 3, 2022
fa6e420
Merge branch 'main' into remove-crossings-hello
vuong177 May 4, 2022
2851cf7
Update proto/ibc/core/channel/v1/tx.proto
vuong177 May 10, 2022
94918d4
Merge branch 'main' into remove-crossings-hello
vuong177 May 16, 2022
0615575
Merge branch 'main' into remove-crossings-hello
faddat May 31, 2022
aa21130
Merge branch 'main' into remove-crossings-hello
faddat Jul 5, 2022
af6bcec
Merge branch 'main' of github.com:cosmos/ibc-go into colin/1311-remov…
colin-axner Jul 7, 2022
40771a6
apply remaining changes for crossing hello removal
colin-axner Jul 7, 2022
c2eecab
remove previous channel check in WriteChannelOpenTry
colin-axner Jul 7, 2022
4f7e88a
regenerate proto files
colin-axner Jul 7, 2022
9856d24
update documentation
colin-axner Jul 7, 2022
a75219d
add migration documentation
colin-axner Jul 7, 2022
dbb9b0e
remove unnecessary doc
colin-axner Jul 7, 2022
c890576
remove crossing hello notion from ChanOpenAck
colin-axner Jul 7, 2022
86d61ed
apply review suggestions
colin-axner Jul 7, 2022
4aa75e5
Merge pull request #68 from cosmos/colin/1311-remove-channel-crossing…
vuong177 Jul 7, 2022
17198d5
Merge branch 'main' into remove-crossings-hello
vuong177 Jul 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (modules/core/04-channel) [\#1232](https://github.com/cosmos/ibc-go/pull/1232) Updating params on `NewPacketId` and moving to bottom of file.
* (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
* (app/29-fee) [\#1305](https://github.com/cosmos/ibc-go/pull/1305) Change version string for fee module to `ics29-1`
* (modules/core/04-channel) [\#1317](https://github.com/cosmos/ibc-go/pull/1317) Remove crossing hellos from ChanOpenTry
colin-axner marked this conversation as resolved.
Show resolved Hide resolved

### Features

Expand Down
54 changes: 6 additions & 48 deletions modules/core/04-channel/keeper/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,51 +98,20 @@ func (k Keeper) ChanOpenTry(
ctx sdk.Context,
order types.Order,
connectionHops []string,
portID,
previousChannelID string,
portID string,
portCap *capabilitytypes.Capability,
counterparty types.Counterparty,
counterpartyVersion string,
proofInit []byte,
proofHeight exported.Height,
) (string, *capabilitytypes.Capability, error) {
var (
previousChannel types.Channel
previousChannelFound bool
)

channelID := previousChannelID

// connection hops only supports a single connection
if len(connectionHops) != 1 {
vuong177 marked this conversation as resolved.
Show resolved Hide resolved
return "", nil, sdkerrors.Wrapf(types.ErrTooManyConnectionHops, "expected 1, got %d", len(connectionHops))
}

// empty channel identifier indicates continuing a previous channel handshake
if previousChannelID != "" {
// channel identifier and connection hop length checked on msg.ValidateBasic()
// ensure that the previous channel exists
previousChannel, previousChannelFound = k.GetChannel(ctx, portID, previousChannelID)
if !previousChannelFound {
return "", nil, sdkerrors.Wrapf(types.ErrInvalidChannel, "previous channel does not exist for supplied previous channelID %s", previousChannelID)
}
// previous channel must use the same fields
if !(previousChannel.Ordering == order &&
previousChannel.Counterparty.PortId == counterparty.PortId &&
previousChannel.Counterparty.ChannelId == "" &&
previousChannel.ConnectionHops[0] == connectionHops[0] && // ChanOpenInit will only set a single connection hop
previousChannel.Version == counterpartyVersion) {
return "", nil, sdkerrors.Wrap(types.ErrInvalidChannel, "channel fields mismatch previous channel fields")
}

if previousChannel.State != types.INIT {
return "", nil, sdkerrors.Wrapf(types.ErrInvalidChannelState, "previous channel state is in %s, expected INIT", previousChannel.State)
}

} else {
// generate a new channel
channelID = k.GenerateChannelIdentifier(ctx)
}
// generate a new channel
channelID := k.GenerateChannelIdentifier(ctx)

if !k.portKeeper.Authenticate(ctx, portCap, portID) {
return "", nil, sdkerrors.Wrapf(porttypes.ErrInvalidPort, "caller does not own port capability for port ID %s", portID)
Expand Down Expand Up @@ -199,20 +168,9 @@ func (k Keeper) ChanOpenTry(
err error
)

if !previousChannelFound {
capKey, err = k.scopedKeeper.NewCapability(ctx, host.ChannelCapabilityPath(portID, channelID))
if err != nil {
return "", nil, sdkerrors.Wrapf(err, "could not create channel capability for port ID %s and channel ID %s", portID, channelID)
}

} else {
// capability initialized in ChanOpenInit
capKey, found = k.scopedKeeper.GetCapability(ctx, host.ChannelCapabilityPath(portID, channelID))
if !found {
return "", nil, sdkerrors.Wrapf(types.ErrChannelCapabilityNotFound,
"capability not found for existing channel, portID (%s) channelID (%s)", portID, channelID,
)
}
capKey, err = k.scopedKeeper.NewCapability(ctx, host.ChannelCapabilityPath(portID, channelID))
if err != nil {
return "", nil, sdkerrors.Wrapf(err, "could not create channel capability for port ID %s and channel ID %s", portID, channelID)
}

return channelID, capKey, nil
Expand Down
24 changes: 4 additions & 20 deletions modules/core/04-channel/keeper/handshake_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,9 @@ func (suite *KeeperTestSuite) TestChanOpenInit() {
// ChanOpenTry can succeed.
func (suite *KeeperTestSuite) TestChanOpenTry() {
var (
path *ibctesting.Path
previousChannelID string
portCap *capabilitytypes.Capability
heightDiff uint64
path *ibctesting.Path
portCap *capabilitytypes.Capability
heightDiff uint64
)

testCases := []testCase{
Expand All @@ -164,22 +163,8 @@ func (suite *KeeperTestSuite) TestChanOpenTry() {
err := suite.coordinator.ChanOpenInitOnBothChains(path)
suite.Require().NoError(err)

previousChannelID = path.EndpointB.ChannelID
portCap = suite.chainB.GetPortCapability(ibctesting.MockPort)
}, true},
{"previous channel with invalid version, crossing hello", func() {
suite.coordinator.SetupConnections(path)
path.SetChannelOrdered()

// modify channel version
path.EndpointA.ChannelConfig.Version = "invalid version"

err := suite.coordinator.ChanOpenInitOnBothChains(path)
suite.Require().NoError(err)

previousChannelID = path.EndpointB.ChannelID
portCap = suite.chainB.GetPortCapability(ibctesting.MockPort)
}, false},
{"previous channel with invalid state", func() {
suite.coordinator.SetupConnections(path)

Expand Down Expand Up @@ -268,7 +253,6 @@ func (suite *KeeperTestSuite) TestChanOpenTry() {
suite.Run(fmt.Sprintf("Case %s", tc.msg), func() {
suite.SetupTest() // reset
heightDiff = 0 // must be explicitly changed in malleate
previousChannelID = ""
path = ibctesting.NewPath(suite.chainA, suite.chainB)

tc.malleate()
Expand All @@ -286,7 +270,7 @@ func (suite *KeeperTestSuite) TestChanOpenTry() {

channelID, cap, err := suite.chainB.App.GetIBCKeeper().ChannelKeeper.ChanOpenTry(
suite.chainB.GetContext(), types.ORDERED, []string{path.EndpointB.ConnectionID},
path.EndpointB.ChannelConfig.PortID, previousChannelID, portCap, counterparty, path.EndpointA.ChannelConfig.Version,
path.EndpointB.ChannelConfig.PortID, portCap, counterparty, path.EndpointA.ChannelConfig.Version,
proof, malleateHeight(proofHeight, heightDiff),
)

Expand Down
2 changes: 1 addition & 1 deletion modules/core/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func (k Keeper) ChannelOpenTry(goCtx context.Context, msg *channeltypes.MsgChann
}

// Perform 04-channel verification
channelID, cap, err := k.ChannelKeeper.ChanOpenTry(ctx, msg.Channel.Ordering, msg.Channel.ConnectionHops, msg.PortId, msg.PreviousChannelId,
vuong177 marked this conversation as resolved.
Show resolved Hide resolved
channelID, cap, err := k.ChannelKeeper.ChanOpenTry(ctx, msg.Channel.Ordering, msg.Channel.ConnectionHops, msg.PortId,
portCap, msg.Channel.Counterparty, msg.CounterpartyVersion, msg.ProofInit, msg.ProofHeight,
)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions proto/ibc/core/channel/v1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ message MsgChannelOpenTry {
string port_id = 1 [(gogoproto.moretags) = "yaml:\"port_id\""];
// in the case of crossing hello's, when both chains call OpenInit, we need
// the channel identifier of the previous channel in state INIT
// this field has been deprecated and will be ignored by core IBC because now we don't need to protect against crossing hellos anymore.
vuong177 marked this conversation as resolved.
Show resolved Hide resolved
string previous_channel_id = 2 [(gogoproto.moretags) = "yaml:\"previous_channel_id\""];
// NOTE: the version field within the channel has been deprecated. Its value will be ignored by core IBC.
Channel channel = 3 [(gogoproto.nullable) = false];
Expand Down