From aa7c0bee0406457c9e988c1e67e5db82b3a46aa1 Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Fri, 4 Dec 2020 13:56:02 +0100 Subject: [PATCH 1/4] increase client basic genesis validation --- x/ibc/core/02-client/types/genesis.go | 39 +++++- x/ibc/core/02-client/types/genesis_test.go | 138 +++++++++++++++++---- 2 files changed, 150 insertions(+), 27 deletions(-) diff --git a/x/ibc/core/02-client/types/genesis.go b/x/ibc/core/02-client/types/genesis.go index f7939c47893..f6eff19e5c5 100644 --- a/x/ibc/core/02-client/types/genesis.go +++ b/x/ibc/core/02-client/types/genesis.go @@ -103,11 +103,15 @@ func (gs GenesisState) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { // Validate performs basic genesis state validation returning an error upon any // failure. func (gs GenesisState) Validate() error { + // keep track of the max sequence to ensure it is less than + // the next sequence used in creating client identifers. + var maxSequence uint64 = 0 + if err := gs.Params.Validate(); err != nil { return err } - validClients := make(map[string]bool) + validClients := make(map[string]exported.ClientState) for i, client := range gs.Clients { if err := host.ClientIdentifierValidator(client.ClientId); err != nil { @@ -126,13 +130,31 @@ func (gs GenesisState) Validate() error { return fmt.Errorf("invalid client %v index %d: %w", client, i, err) } + clientType, sequence, err := ParseClientIdentifier(client.ClientId) + if err != nil { + return err + } + + if clientType != clientState.ClientType() { + return fmt.Errorf("client state type %s does not equal client type in client identifier %s", clientState.ClientType(), clientType) + } + + if err := ValidateClientType(clientType); err != nil { + return err + } + + if sequence > maxSequence { + maxSequence = sequence + } + // add client id to validClients map - validClients[client.ClientId] = true + validClients[client.ClientId] = clientState } for i, cc := range gs.ClientsConsensus { // check that consensus state is for a client in the genesis clients list - if !validClients[cc.ClientId] { + clientState, ok := validClients[cc.ClientId] + if !ok { return fmt.Errorf("consensus state in genesis has a client id %s that does not map to a genesis client", cc.ClientId) } @@ -149,12 +171,23 @@ func (gs GenesisState) Validate() error { if err := cs.ValidateBasic(); err != nil { return fmt.Errorf("invalid client consensus state %v index %d: %w", cs, i, err) } + + // ensure consensus state type matches client state type + if clientState.ClientType() != cs.ClientType() { + return fmt.Errorf("consensus state client type %s does not equal client state client type %s", cs.ClientType(), clientState.ClientType()) + } + } + } if gs.CreateLocalhost && !gs.Params.IsAllowedClient(exported.Localhost) { return fmt.Errorf("localhost client is not registered on the allowlist") } + if maxSequence != 0 && maxSequence >= gs.NextClientSequence { + return fmt.Errorf("next client identifier sequence %d must be greater than the maximum sequence used in the provided client identifiers %d", gs.NextClientSequence, maxSequence) + } + return nil } diff --git a/x/ibc/core/02-client/types/genesis_test.go b/x/ibc/core/02-client/types/genesis_test.go index 81eff78335d..12cfba69dd4 100644 --- a/x/ibc/core/02-client/types/genesis_test.go +++ b/x/ibc/core/02-client/types/genesis_test.go @@ -17,8 +17,11 @@ import ( ) const ( - chainID = "chainID" - clientID = "ethbridge" + chainID = "chainID" + tmClientID0 = "07-tendermint-0" + tmClientID1 = "07-tendermint-1" + invalidClientID = "myclient-0" + clientID = tmClientID0 height = 10 ) @@ -69,15 +72,15 @@ func (suite *TypesTestSuite) TestValidateGenesis() { genState: types.NewGenesisState( []types.IdentifiedClientState{ types.NewIdentifiedClientState( - clientID, ibctmtypes.NewClientState(chainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), + tmClientID0, ibctmtypes.NewClientState(chainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), ), types.NewIdentifiedClientState( - exported.Localhost, localhosttypes.NewClientState("chainID", clientHeight), + exported.Localhost+"-1", localhosttypes.NewClientState("chainID", clientHeight), ), }, []types.ClientConsensusStates{ types.NewClientConsensusStates( - clientID, + tmClientID0, []types.ConsensusStateWithHeight{ types.NewConsensusStateWithHeight( header.GetHeight().(types.Height), @@ -90,7 +93,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, types.NewParams(exported.Tendermint, exported.Localhost), false, - 0, + 2, ), expPass: true, }, @@ -99,7 +102,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { genState: types.NewGenesisState( []types.IdentifiedClientState{ types.NewIdentifiedClientState( - "/~@$*", ibctmtypes.NewClientState(chainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), + invalidClientID, ibctmtypes.NewClientState(chainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), ), types.NewIdentifiedClientState( exported.Localhost, localhosttypes.NewClientState("chainID", clientHeight), @@ -107,7 +110,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, []types.ClientConsensusStates{ types.NewClientConsensusStates( - "/~@$*", + invalidClientID, []types.ConsensusStateWithHeight{ types.NewConsensusStateWithHeight( header.GetHeight().(types.Height), @@ -129,7 +132,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { genState: types.NewGenesisState( []types.IdentifiedClientState{ types.NewIdentifiedClientState( - clientID, ibctmtypes.NewClientState(chainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), + tmClientID0, ibctmtypes.NewClientState(chainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), ), types.NewIdentifiedClientState(exported.Localhost, localhosttypes.NewClientState("chaindID", types.ZeroHeight())), }, @@ -145,7 +148,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { genState: types.NewGenesisState( []types.IdentifiedClientState{ types.NewIdentifiedClientState( - clientID, ibctmtypes.NewClientState(chainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), + tmClientID0, ibctmtypes.NewClientState(chainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), ), types.NewIdentifiedClientState( exported.Localhost, localhosttypes.NewClientState("chaindID", clientHeight), @@ -153,7 +156,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, []types.ClientConsensusStates{ types.NewClientConsensusStates( - "wrongclientid", + tmClientID1, []types.ConsensusStateWithHeight{ types.NewConsensusStateWithHeight( types.ZeroHeight(), @@ -175,7 +178,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { genState: types.NewGenesisState( []types.IdentifiedClientState{ types.NewIdentifiedClientState( - clientID, ibctmtypes.NewClientState(chainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), + tmClientID0, ibctmtypes.NewClientState(chainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), ), types.NewIdentifiedClientState( exported.Localhost, localhosttypes.NewClientState("chaindID", clientHeight), @@ -183,7 +186,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, []types.ClientConsensusStates{ types.NewClientConsensusStates( - clientID, + tmClientID0, []types.ConsensusStateWithHeight{ types.NewConsensusStateWithHeight( types.ZeroHeight(), @@ -205,7 +208,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { genState: types.NewGenesisState( []types.IdentifiedClientState{ types.NewIdentifiedClientState( - clientID, ibctmtypes.NewClientState(chainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), + tmClientID0, ibctmtypes.NewClientState(chainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), ), types.NewIdentifiedClientState( exported.Localhost, localhosttypes.NewClientState("chaindID", clientHeight), @@ -213,7 +216,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, []types.ClientConsensusStates{ types.NewClientConsensusStates( - clientID, + tmClientID0, []types.ConsensusStateWithHeight{ types.NewConsensusStateWithHeight( types.NewHeight(0, 1), @@ -235,7 +238,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { genState: types.NewGenesisState( []types.IdentifiedClientState{ types.NewIdentifiedClientState( - clientID, ibctmtypes.NewClientState(chainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), + tmClientID0, ibctmtypes.NewClientState(chainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), ), types.NewIdentifiedClientState( exported.Localhost, localhosttypes.NewClientState("chainID", clientHeight), @@ -243,7 +246,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, []types.ClientConsensusStates{ types.NewClientConsensusStates( - clientID, + tmClientID0, []types.ConsensusStateWithHeight{ types.NewConsensusStateWithHeight( header.GetHeight().(types.Height), @@ -265,7 +268,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { genState: types.NewGenesisState( []types.IdentifiedClientState{ types.NewIdentifiedClientState( - clientID, ibctmtypes.NewClientState(chainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), + tmClientID0, ibctmtypes.NewClientState(chainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), ), types.NewIdentifiedClientState( exported.Localhost, localhosttypes.NewClientState("chainID", clientHeight), @@ -273,7 +276,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, []types.ClientConsensusStates{ types.NewClientConsensusStates( - clientID, + tmClientID0, []types.ConsensusStateWithHeight{ types.NewConsensusStateWithHeight( header.GetHeight().(types.Height), @@ -295,7 +298,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { genState: types.NewGenesisState( []types.IdentifiedClientState{ types.NewIdentifiedClientState( - clientID, ibctmtypes.NewClientState(chainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), + tmClientID0, ibctmtypes.NewClientState(chainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), ), types.NewIdentifiedClientState( exported.Localhost, localhosttypes.NewClientState("chainID", clientHeight), @@ -303,7 +306,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, []types.ClientConsensusStates{ types.NewClientConsensusStates( - clientID, + tmClientID0, []types.ConsensusStateWithHeight{ types.NewConsensusStateWithHeight( header.GetHeight().(types.Height), @@ -325,15 +328,15 @@ func (suite *TypesTestSuite) TestValidateGenesis() { genState: types.NewGenesisState( []types.IdentifiedClientState{ types.NewIdentifiedClientState( - clientID, ibctmtypes.NewClientState(chainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), + tmClientID1, ibctmtypes.NewClientState(chainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), ), types.NewIdentifiedClientState( - exported.Localhost, localhosttypes.NewClientState("chainID", clientHeight), + exported.Localhost+"-0", localhosttypes.NewClientState("chainID", clientHeight), ), }, []types.ClientConsensusStates{ types.NewClientConsensusStates( - clientID, + tmClientID1, []types.ConsensusStateWithHeight{ types.NewConsensusStateWithHeight( header.GetHeight().(types.Height), @@ -346,10 +349,97 @@ func (suite *TypesTestSuite) TestValidateGenesis() { }, types.NewParams(exported.Tendermint), true, + 2, + ), + expPass: false, + }, + { + name: "next sequence too small", + genState: types.NewGenesisState( + []types.IdentifiedClientState{ + types.NewIdentifiedClientState( + tmClientID0, ibctmtypes.NewClientState(chainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), + ), + types.NewIdentifiedClientState( + exported.Localhost+"-1", localhosttypes.NewClientState("chainID", clientHeight), + ), + }, + []types.ClientConsensusStates{ + types.NewClientConsensusStates( + tmClientID0, + []types.ConsensusStateWithHeight{ + types.NewConsensusStateWithHeight( + header.GetHeight().(types.Height), + ibctmtypes.NewConsensusState( + header.GetTime(), commitmenttypes.NewMerkleRoot(header.Header.GetAppHash()), header.Header.NextValidatorsHash, + ), + ), + }, + ), + }, + types.NewParams(exported.Tendermint, exported.Localhost), + false, 0, ), expPass: false, }, + { + name: "failed to parse client identifier in client state loop", + genState: types.NewGenesisState( + []types.IdentifiedClientState{ + types.NewIdentifiedClientState( + "my-client", ibctmtypes.NewClientState(chainID, ibctesting.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), + ), + types.NewIdentifiedClientState( + exported.Localhost+"-1", localhosttypes.NewClientState("chainID", clientHeight), + ), + }, + []types.ClientConsensusStates{ + types.NewClientConsensusStates( + tmClientID0, + []types.ConsensusStateWithHeight{ + types.NewConsensusStateWithHeight( + header.GetHeight().(types.Height), + ibctmtypes.NewConsensusState( + header.GetTime(), commitmenttypes.NewMerkleRoot(header.Header.GetAppHash()), header.Header.NextValidatorsHash, + ), + ), + }, + ), + }, + types.NewParams(exported.Tendermint, exported.Localhost), + false, + 5, + ), + expPass: false, + }, + { + name: "consensus state different than client state type", + genState: types.NewGenesisState( + []types.IdentifiedClientState{ + types.NewIdentifiedClientState( + exported.Localhost+"-1", localhosttypes.NewClientState("chainID", clientHeight), + ), + }, + []types.ClientConsensusStates{ + types.NewClientConsensusStates( + exported.Localhost+"-1", + []types.ConsensusStateWithHeight{ + types.NewConsensusStateWithHeight( + header.GetHeight().(types.Height), + ibctmtypes.NewConsensusState( + header.GetTime(), commitmenttypes.NewMerkleRoot(header.Header.GetAppHash()), header.Header.NextValidatorsHash, + ), + ), + }, + ), + }, + types.NewParams(exported.Tendermint, exported.Localhost), + false, + 5, + ), + expPass: false, + }, } for _, tc := range testCases { From 464a4faa4394fd4ecdcbf2296d4c855d17beb8e3 Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Fri, 4 Dec 2020 15:02:13 +0100 Subject: [PATCH 2/4] add connection/channel genesis validation --- x/ibc/core/03-connection/types/genesis.go | 17 ++++ .../core/03-connection/types/genesis_test.go | 26 ++++++ x/ibc/core/04-channel/types/genesis.go | 17 ++++ x/ibc/core/04-channel/types/genesis_test.go | 80 ++++++++++++++++++- 4 files changed, 137 insertions(+), 3 deletions(-) diff --git a/x/ibc/core/03-connection/types/genesis.go b/x/ibc/core/03-connection/types/genesis.go index f5af9b8486b..b10c300a84a 100644 --- a/x/ibc/core/03-connection/types/genesis.go +++ b/x/ibc/core/03-connection/types/genesis.go @@ -38,7 +38,20 @@ func DefaultGenesisState() GenesisState { // Validate performs basic genesis state validation returning an error upon any // failure. func (gs GenesisState) Validate() error { + // keep track of the max sequence to ensure it is less than + // the next sequence used in creating connection identifers. + var maxSequence uint64 = 0 + for i, conn := range gs.Connections { + sequence, err := ParseConnectionSequence(conn.Id) + if err != nil { + return err + } + + if sequence > maxSequence { + maxSequence = sequence + } + if err := conn.ValidateBasic(); err != nil { return fmt.Errorf("invalid connection %v index %d: %w", conn, i, err) } @@ -55,5 +68,9 @@ func (gs GenesisState) Validate() error { } } + if maxSequence != 0 && maxSequence >= gs.NextConnectionSequence { + return fmt.Errorf("next connection sequence %d must be greater than maximum sequence used in connection identifier %d", gs.NextConnectionSequence, maxSequence) + } + return nil } diff --git a/x/ibc/core/03-connection/types/genesis_test.go b/x/ibc/core/03-connection/types/genesis_test.go index 343d69b08e2..8d343d1f031 100644 --- a/x/ibc/core/03-connection/types/genesis_test.go +++ b/x/ibc/core/03-connection/types/genesis_test.go @@ -74,6 +74,32 @@ func TestValidateGenesis(t *testing.T) { ), expPass: false, }, + { + name: "invalid connection identifier", + genState: types.NewGenesisState( + []types.IdentifiedConnection{ + types.NewIdentifiedConnection("conn-0", types.NewConnectionEnd(types.INIT, clientID, types.Counterparty{clientID2, connectionID2, commitmenttypes.NewMerklePrefix([]byte("prefix"))}, []*types.Version{ibctesting.ConnectionVersion})), + }, + []types.ConnectionPaths{ + {clientID, []string{connectionID}}, + }, + 0, + ), + expPass: false, + }, + { + name: "next connection sequence is not greater than maximum connection identifier sequence provided", + genState: types.NewGenesisState( + []types.IdentifiedConnection{ + types.NewIdentifiedConnection(types.FormatConnectionIdentifier(10), types.NewConnectionEnd(types.INIT, clientID, types.Counterparty{clientID2, connectionID2, commitmenttypes.NewMerklePrefix([]byte("prefix"))}, []*types.Version{ibctesting.ConnectionVersion})), + }, + []types.ConnectionPaths{ + {clientID, []string{connectionID}}, + }, + 0, + ), + expPass: false, + }, } for _, tc := range testCases { diff --git a/x/ibc/core/04-channel/types/genesis.go b/x/ibc/core/04-channel/types/genesis.go index 28ab32f21a5..2c431e97b33 100644 --- a/x/ibc/core/04-channel/types/genesis.go +++ b/x/ibc/core/04-channel/types/genesis.go @@ -74,12 +74,29 @@ func DefaultGenesisState() GenesisState { // Validate performs basic genesis state validation returning an error upon any // failure. func (gs GenesisState) Validate() error { + // keep track of the max sequence to ensure it is less than + // the next sequence used in creating connection identifers. + var maxSequence uint64 = 0 + for i, channel := range gs.Channels { + sequence, err := ParseChannelSequence(channel.ChannelId) + if err != nil { + return err + } + + if sequence > maxSequence { + maxSequence = sequence + } + if err := channel.ValidateBasic(); err != nil { return fmt.Errorf("invalid channel %v channel index %d: %w", channel, i, err) } } + if maxSequence != 0 && maxSequence >= gs.NextChannelSequence { + return fmt.Errorf("next channel sequence %d must be greater than maximum sequence used in channel identifier %d", gs.NextChannelSequence, maxSequence) + } + for i, ack := range gs.Acknowledgements { if err := ack.Validate(); err != nil { return fmt.Errorf("invalid acknowledgement %v ack index %d: %w", ack, i, err) diff --git a/x/ibc/core/04-channel/types/genesis_test.go b/x/ibc/core/04-channel/types/genesis_test.go index 05e154296e6..a0d21007a77 100644 --- a/x/ibc/core/04-channel/types/genesis_test.go +++ b/x/ibc/core/04-channel/types/genesis_test.go @@ -13,8 +13,8 @@ const ( testPort2 = "secondport" testConnectionIDA = "connectionidatob" - testChannel1 = "firstchannel" - testChannel2 = "secondchannel" + testChannel1 = "channel-0" + testChannel2 = "channel-1" testChannelOrder = types.ORDERED testChannelVersion = "1.0" @@ -66,7 +66,7 @@ func TestValidateGenesis(t *testing.T) { []types.PacketSequence{ types.NewPacketSequence(testPort2, testChannel2, 1), }, - 0, + 2, ), expPass: true, }, @@ -137,6 +137,80 @@ func TestValidateGenesis(t *testing.T) { }, expPass: false, }, + { + name: "invalid channel identifier", + genState: types.NewGenesisState( + []types.IdentifiedChannel{ + types.NewIdentifiedChannel( + testPort1, "chan-0", types.NewChannel( + types.INIT, testChannelOrder, counterparty2, []string{testConnectionIDA}, testChannelVersion, + ), + ), + types.NewIdentifiedChannel( + testPort2, testChannel2, types.NewChannel( + types.INIT, testChannelOrder, counterparty1, []string{testConnectionIDA}, testChannelVersion, + ), + ), + }, + []types.PacketState{ + types.NewPacketState(testPort2, testChannel2, 1, []byte("ack")), + }, + []types.PacketState{ + types.NewPacketState(testPort2, testChannel2, 1, []byte("")), + }, + []types.PacketState{ + types.NewPacketState(testPort1, testChannel1, 1, []byte("commit_hash")), + }, + []types.PacketSequence{ + types.NewPacketSequence(testPort1, testChannel1, 1), + }, + []types.PacketSequence{ + types.NewPacketSequence(testPort2, testChannel2, 1), + }, + []types.PacketSequence{ + types.NewPacketSequence(testPort2, testChannel2, 1), + }, + 0, + ), + expPass: false, + }, + { + name: "next channel sequence is less than maximum channel identifier sequence used", + genState: types.NewGenesisState( + []types.IdentifiedChannel{ + types.NewIdentifiedChannel( + testPort1, "channel-10", types.NewChannel( + types.INIT, testChannelOrder, counterparty2, []string{testConnectionIDA}, testChannelVersion, + ), + ), + types.NewIdentifiedChannel( + testPort2, testChannel2, types.NewChannel( + types.INIT, testChannelOrder, counterparty1, []string{testConnectionIDA}, testChannelVersion, + ), + ), + }, + []types.PacketState{ + types.NewPacketState(testPort2, testChannel2, 1, []byte("ack")), + }, + []types.PacketState{ + types.NewPacketState(testPort2, testChannel2, 1, []byte("")), + }, + []types.PacketState{ + types.NewPacketState(testPort1, testChannel1, 1, []byte("commit_hash")), + }, + []types.PacketSequence{ + types.NewPacketSequence(testPort1, testChannel1, 1), + }, + []types.PacketSequence{ + types.NewPacketSequence(testPort2, testChannel2, 1), + }, + []types.PacketSequence{ + types.NewPacketSequence(testPort2, testChannel2, 1), + }, + 0, + ), + expPass: false, + }, } for _, tc := range testCases { From 93e7edb1b93706a986254e347a73334d96156f07 Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Fri, 4 Dec 2020 15:36:52 +0100 Subject: [PATCH 3/4] fix tests --- x/ibc/core/genesis_test.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/x/ibc/core/genesis_test.go b/x/ibc/core/genesis_test.go index bc4c5834d3e..fa0200de79d 100644 --- a/x/ibc/core/genesis_test.go +++ b/x/ibc/core/genesis_test.go @@ -22,16 +22,17 @@ import ( ) const ( - connectionID = "connectionidone" - clientID = "clientidone" - connectionID2 = "connectionidtwo" - clientID2 = "clientidtwo" + connectionID = "connection-0" + clientID = "07-tendermint-0" + connectionID2 = "connection-1" + clientID2 = "07-tendermin-1" + localhostID = exported.Localhost + "-1" port1 = "firstport" port2 = "secondport" - channel1 = "firstchannel" - channel2 = "secondchannel" + channel1 = "channel-0" + channel2 = "channel-1" ) var clientHeight = clienttypes.NewHeight(0, 10) @@ -79,7 +80,7 @@ func (suite *IBCTestSuite) TestValidateGenesis() { clientID, ibctmtypes.NewClientState(suite.chainA.ChainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), ), clienttypes.NewIdentifiedClientState( - exported.Localhost, localhosttypes.NewClientState("chaindID", clientHeight), + localhostID, localhosttypes.NewClientState("chaindID", clientHeight), ), }, []clienttypes.ClientConsensusStates{ @@ -97,7 +98,7 @@ func (suite *IBCTestSuite) TestValidateGenesis() { }, clienttypes.NewParams(exported.Tendermint, exported.Localhost), true, - 0, + 2, ), ConnectionGenesis: connectiontypes.NewGenesisState( []connectiontypes.IdentifiedConnection{ @@ -149,13 +150,13 @@ func (suite *IBCTestSuite) TestValidateGenesis() { clientID, ibctmtypes.NewClientState(suite.chainA.ChainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false), ), clienttypes.NewIdentifiedClientState( - exported.Localhost, localhosttypes.NewClientState("(chaindID)", clienttypes.ZeroHeight()), + localhostID, localhosttypes.NewClientState("(chaindID)", clienttypes.ZeroHeight()), ), }, nil, clienttypes.NewParams(exported.Tendermint), false, - 0, + 2, ), ConnectionGenesis: connectiontypes.DefaultGenesisState(), }, From 4147b7e01558ca7baf3a66b29e6dfac47ab191e4 Mon Sep 17 00:00:00 2001 From: Colin Axner Date: Fri, 4 Dec 2020 16:06:29 +0100 Subject: [PATCH 4/4] apply @AdityaSripal suggestion --- x/ibc/core/02-client/types/genesis.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/x/ibc/core/02-client/types/genesis.go b/x/ibc/core/02-client/types/genesis.go index f6eff19e5c5..18f33160c28 100644 --- a/x/ibc/core/02-client/types/genesis.go +++ b/x/ibc/core/02-client/types/genesis.go @@ -111,7 +111,7 @@ func (gs GenesisState) Validate() error { return err } - validClients := make(map[string]exported.ClientState) + validClients := make(map[string]string) for i, client := range gs.Clients { if err := host.ClientIdentifierValidator(client.ClientId); err != nil { @@ -148,12 +148,12 @@ func (gs GenesisState) Validate() error { } // add client id to validClients map - validClients[client.ClientId] = clientState + validClients[client.ClientId] = clientState.ClientType() } for i, cc := range gs.ClientsConsensus { // check that consensus state is for a client in the genesis clients list - clientState, ok := validClients[cc.ClientId] + clientType, ok := validClients[cc.ClientId] if !ok { return fmt.Errorf("consensus state in genesis has a client id %s that does not map to a genesis client", cc.ClientId) } @@ -173,8 +173,8 @@ func (gs GenesisState) Validate() error { } // ensure consensus state type matches client state type - if clientState.ClientType() != cs.ClientType() { - return fmt.Errorf("consensus state client type %s does not equal client state client type %s", cs.ClientType(), clientState.ClientType()) + if clientType != cs.ClientType() { + return fmt.Errorf("consensus state client type %s does not equal client state client type %s", cs.ClientType(), clientType) } }