From 6fe9fbc375a50caf6f10cdffae494a49f5f52ad8 Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Mon, 26 Oct 2020 16:11:03 +0100 Subject: [PATCH 1/2] fix optimistic handshake bugs and add crossing hello test --- x/ibc/core/04-channel/keeper/handshake.go | 31 +++++++++--- .../core/04-channel/keeper/handshake_test.go | 6 +++ x/ibc/testing/coordinator.go | 48 +++++++++++++++++++ 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/x/ibc/core/04-channel/keeper/handshake.go b/x/ibc/core/04-channel/keeper/handshake.go index 900c234bec75..a812536b76f5 100644 --- a/x/ibc/core/04-channel/keeper/handshake.go +++ b/x/ibc/core/04-channel/keeper/handshake.go @@ -187,16 +187,33 @@ func (k Keeper) ChanOpenTry( return nil, err } - k.SetChannel(ctx, portID, desiredChannelID, channel) + var ( + capKey *capabilitytypes.Capability + err error + ) - capKey, err := k.scopedKeeper.NewCapability(ctx, host.ChannelCapabilityPath(portID, desiredChannelID)) - if err != nil { - return nil, sdkerrors.Wrapf(err, "could not create channel capability for port ID %s and channel ID %s", portID, desiredChannelID) + // Only create a capability and set the sequences if the previous channel does not exist + _, found = k.GetChannel(ctx, portID, desiredChannelID) + if !found { + capKey, err = k.scopedKeeper.NewCapability(ctx, host.ChannelCapabilityPath(portID, desiredChannelID)) + if err != nil { + return nil, sdkerrors.Wrapf(err, "could not create channel capability for port ID %s and channel ID %s", portID, desiredChannelID) + } + + k.SetNextSequenceSend(ctx, portID, desiredChannelID, 1) + k.SetNextSequenceRecv(ctx, portID, desiredChannelID, 1) + k.SetNextSequenceAck(ctx, portID, desiredChannelID, 1) + } else { + // capability initialized in ChanOpenInit + capKey, found = k.scopedKeeper.GetCapability(ctx, host.ChannelCapabilityPath(portID, desiredChannelID)) + if !found { + return nil, sdkerrors.Wrapf(types.ErrChannelCapabilityNotFound, + "capability not found for existing channel, portID (%s) channelID (%s)", portID, desiredChannelID, + ) + } } - k.SetNextSequenceSend(ctx, portID, desiredChannelID, 1) - k.SetNextSequenceRecv(ctx, portID, desiredChannelID, 1) - k.SetNextSequenceAck(ctx, portID, desiredChannelID, 1) + k.SetChannel(ctx, portID, desiredChannelID, channel) k.Logger(ctx).Info("channel state updated", "port-id", portID, "channel-id", desiredChannelID, "previous-state", previousChannel.State.String(), "new-state", "TRYOPEN") diff --git a/x/ibc/core/04-channel/keeper/handshake_test.go b/x/ibc/core/04-channel/keeper/handshake_test.go index 77bbb90ee435..0409b43ff690 100644 --- a/x/ibc/core/04-channel/keeper/handshake_test.go +++ b/x/ibc/core/04-channel/keeper/handshake_test.go @@ -153,6 +153,12 @@ func (suite *KeeperTestSuite) TestChanOpenTry() { suite.chainB.CreatePortCapability(connB.NextTestChannel(ibctesting.MockPort).PortID) portCap = suite.chainB.GetPortCapability(connB.NextTestChannel(ibctesting.MockPort).PortID) }, true}, + {"success with crossing hello", func() { + _, _, connA, connB = suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, ibctesting.Tendermint) + suite.coordinator.ChanOpenInit(suite.chainA, suite.chainB, connA, connB, ibctesting.MockPort, ibctesting.MockPort, types.ORDERED) + + portCap = suite.chainB.GetPortCapability(connB.NextTestChannel(ibctesting.MockPort).PortID) + }, true}, {"success with empty counterparty chosen channel id", func() { var clientA, clientB string clientA, clientB, connA, connB = suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, ibctesting.Tendermint) diff --git a/x/ibc/testing/coordinator.go b/x/ibc/testing/coordinator.go index 2b7c6a466d0f..a33924ef8ec8 100644 --- a/x/ibc/testing/coordinator.go +++ b/x/ibc/testing/coordinator.go @@ -524,6 +524,54 @@ func (coord *Coordinator) ChanOpenInit( return sourceChannel, counterpartyChannel, nil } +// ChanOpenInitOnBothChains initializes a channel on the source chain and counterparty chain +// with the state INIT using the OpenInit handshake call. +func (coord *Coordinator) ChanOpenInitOnBothChains( + source, counterparty *TestChain, + connection, counterpartyConnection *TestConnection, + sourcePortID, counterpartyPortID string, + order channeltypes.Order, +) (TestChannel, TestChannel, error) { + sourceChannel := connection.AddTestChannel(sourcePortID) + counterpartyChannel := counterpartyConnection.AddTestChannel(counterpartyPortID) + + // NOTE: only creation of a capability for a transfer or mock port is supported + // Other applications must bind to the port in InitGenesis or modify this code. + source.CreatePortCapability(sourceChannel.PortID) + counterparty.CreatePortCapability(counterpartyChannel.PortID) + coord.IncrementTime() + + // initialize channel on source + if err := source.ChanOpenInit(sourceChannel, counterpartyChannel, order, connection.ID); err != nil { + return sourceChannel, counterpartyChannel, err + } + coord.IncrementTime() + + // update source client on counterparty connection + if err := coord.UpdateClient( + counterparty, source, + counterpartyConnection.ClientID, Tendermint, + ); err != nil { + return sourceChannel, counterpartyChannel, err + } + + // initialize channel on counterparty + if err := counterparty.ChanOpenInit(counterpartyChannel, sourceChannel, order, counterpartyConnection.ID); err != nil { + return sourceChannel, counterpartyChannel, err + } + coord.IncrementTime() + + // update counterparty client on source connection + if err := coord.UpdateClient( + source, counterparty, + connection.ClientID, Tendermint, + ); err != nil { + return sourceChannel, counterpartyChannel, err + } + + return sourceChannel, counterpartyChannel, nil +} + // ChanOpenTry initializes a channel on the source chain with the state TRYOPEN // using the OpenTry handshake call. func (coord *Coordinator) ChanOpenTry( From fe3f256bfd86301e476a577c0f6268c3e8c9dc3b Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Mon, 26 Oct 2020 16:25:17 +0100 Subject: [PATCH 2/2] fix tests --- x/ibc/core/04-channel/keeper/handshake_test.go | 2 +- x/ibc/testing/coordinator.go | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/x/ibc/core/04-channel/keeper/handshake_test.go b/x/ibc/core/04-channel/keeper/handshake_test.go index 0409b43ff690..927bc3131bcb 100644 --- a/x/ibc/core/04-channel/keeper/handshake_test.go +++ b/x/ibc/core/04-channel/keeper/handshake_test.go @@ -155,7 +155,7 @@ func (suite *KeeperTestSuite) TestChanOpenTry() { }, true}, {"success with crossing hello", func() { _, _, connA, connB = suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, ibctesting.Tendermint) - suite.coordinator.ChanOpenInit(suite.chainA, suite.chainB, connA, connB, ibctesting.MockPort, ibctesting.MockPort, types.ORDERED) + suite.coordinator.ChanOpenInitOnBothChains(suite.chainA, suite.chainB, connA, connB, ibctesting.MockPort, ibctesting.MockPort, types.ORDERED) portCap = suite.chainB.GetPortCapability(connB.NextTestChannel(ibctesting.MockPort).PortID) }, true}, diff --git a/x/ibc/testing/coordinator.go b/x/ibc/testing/coordinator.go index a33924ef8ec8..6dbacee50f69 100644 --- a/x/ibc/testing/coordinator.go +++ b/x/ibc/testing/coordinator.go @@ -547,14 +547,6 @@ func (coord *Coordinator) ChanOpenInitOnBothChains( } coord.IncrementTime() - // update source client on counterparty connection - if err := coord.UpdateClient( - counterparty, source, - counterpartyConnection.ClientID, Tendermint, - ); err != nil { - return sourceChannel, counterpartyChannel, err - } - // initialize channel on counterparty if err := counterparty.ChanOpenInit(counterpartyChannel, sourceChannel, order, counterpartyConnection.ID); err != nil { return sourceChannel, counterpartyChannel, err @@ -569,6 +561,14 @@ func (coord *Coordinator) ChanOpenInitOnBothChains( return sourceChannel, counterpartyChannel, err } + // update source client on counterparty connection + if err := coord.UpdateClient( + counterparty, source, + counterpartyConnection.ClientID, Tendermint, + ); err != nil { + return sourceChannel, counterpartyChannel, err + } + return sourceChannel, counterpartyChannel, nil }