Skip to content

Commit

Permalink
When a channel reopens the ordering and metadata must not change (#5562)
Browse files Browse the repository at this point in the history
  • Loading branch information
@chatton
chatton authored Jan 11, 2024
1 parent 3dedb40 commit 3f83cf6
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 19 deletions.
6 changes: 5 additions & 1 deletion modules/apps/27-interchain-accounts/controller/keeper/handshake.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,14 @@ func (k Keeper) OnChanOpenInit(
panic(fmt.Errorf("active channel mapping set for %s but channel does not exist in channel store", activeChannelID))
}

if channel.IsOpen() {
if channel.State == channeltypes.OPEN {
return "", errorsmod.Wrapf(icatypes.ErrActiveChannelAlreadySet, "existing active channel %s for portID %s is already OPEN", activeChannelID, portID)
}

if channel.Ordering != order {
return "", errorsmod.Wrapf(channeltypes.ErrInvalidChannelOrdering, "order cannot change when reopening a channel expected %s, got %s", channel.Ordering, order)
}

appVersion, found := k.GetAppVersion(ctx, portID, activeChannelID)
if !found {
panic(fmt.Errorf("active channel mapping set for %s, but channel does not exist in channel store", activeChannelID))
Expand Down
56 changes: 38 additions & 18 deletions modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
testCases := []struct {
name string
malleate func()
expPass bool
expError error
}{
{
"success",
func() {},
true,
nil,
},
{
"success: previous active channel closed",
Expand All @@ -48,14 +48,14 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {

path.EndpointA.SetChannel(channel)
},
true,
nil,
},
{
"success: empty channel version returns default metadata JSON string",
func() {
channel.Version = ""
},
true,
nil,
},
{
"success: channel reopening",
Expand All @@ -72,7 +72,25 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
path.EndpointA.ChannelID = ""
path.EndpointB.ChannelID = ""
},
true,
nil,
},
{
"failure: different ordering from previous channel",
func() {
suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)

counterparty := channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
channel := channeltypes.Channel{
State: channeltypes.CLOSED,
Ordering: channeltypes.UNORDERED,
Counterparty: counterparty,
ConnectionHops: []string{path.EndpointA.ConnectionID},
Version: TestVersion,
}

path.EndpointA.SetChannel(channel)
},
channeltypes.ErrInvalidChannelOrdering,
},
{
"invalid metadata - previous metadata is different",
Expand All @@ -96,30 +114,30 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
}
path.EndpointA.SetChannel(closedChannel)
},
false,
icatypes.ErrInvalidVersion,
},
{
"invalid port ID",
func() {
path.EndpointA.ChannelConfig.PortID = "invalid-port-id" //nolint:goconst
},
false,
icatypes.ErrInvalidControllerPort,
},
{
"invalid counterparty port ID",
func() {
path.EndpointA.SetChannel(*channel)
channel.Counterparty.PortId = "invalid-port-id"
},
false,
icatypes.ErrInvalidHostPort,
},
{
"invalid metadata bytestring",
func() {
path.EndpointA.SetChannel(*channel)
channel.Version = "invalid-metadata-bytestring"
},
false,
icatypes.ErrUnknownDataType,
},
{
"unsupported encoding format",
Expand All @@ -132,7 +150,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
channel.Version = string(versionBytes)
path.EndpointA.SetChannel(*channel)
},
false,
icatypes.ErrInvalidCodec,
},
{
"unsupported transaction type",
Expand All @@ -145,23 +163,23 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
channel.Version = string(versionBytes)
path.EndpointA.SetChannel(*channel)
},
false,
icatypes.ErrUnknownDataType,
},
{
"connection not found",
func() {
channel.ConnectionHops = []string{"invalid-connnection-id"}
path.EndpointA.SetChannel(*channel)
},
false,
connectiontypes.ErrConnectionNotFound,
},
{
"connection not found with default empty channel version",
func() {
channel.ConnectionHops = []string{"connection-10"}
channel.Version = ""
},
false,
connectiontypes.ErrConnectionNotFound,
},
{
"invalid controller connection ID",
Expand All @@ -174,7 +192,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
channel.Version = string(versionBytes)
path.EndpointA.SetChannel(*channel)
},
false,
connectiontypes.ErrInvalidConnection,
},
{
"invalid host connection ID",
Expand All @@ -187,7 +205,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
channel.Version = string(versionBytes)
path.EndpointA.SetChannel(*channel)
},
false,
connectiontypes.ErrInvalidConnection,
},
{
"invalid version",
Expand All @@ -200,7 +218,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
channel.Version = string(versionBytes)
path.EndpointA.SetChannel(*channel)
},
false,
icatypes.ErrInvalidVersion,
},
{
"channel is already active",
Expand All @@ -217,7 +235,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
}
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetChannel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, channel)
},
false,
icatypes.ErrActiveChannelAlreadySet,
},
}

Expand Down Expand Up @@ -261,11 +279,13 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, channel.Counterparty, channel.Version,
)

if tc.expPass {
expPass := tc.expError == nil
if expPass {
suite.Require().NoError(err)
suite.Require().Equal(string(versionBytes), version)
} else {
suite.Require().Error(err)
suite.Require().ErrorIs(err, tc.expError)
}
})
}
Expand Down

0 comments on commit 3f83cf6

Please sign in to comment.