check clients against params and consensus against clients (#8016)

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
cosmos · Nov 24, 2020 · 53294f5 · 53294f5
1 parent def3c5b
commit 53294f5
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 10 deletions.
diff --git a/x/ibc/core/02-client/types/genesis.go b/x/ibc/core/02-client/types/genesis.go
@@ -101,6 +101,12 @@ 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 {
+	if err := gs.Params.Validate(); err != nil {
+		return err
+	}
+
+	validClients := make(map[string]bool)
+
 	for i, client := range gs.Clients {
 		if err := host.ClientIdentifierValidator(client.ClientId); err != nil {
 			return fmt.Errorf("invalid client consensus state identifier %s index %d: %w", client.ClientId, i, err)
@@ -110,14 +116,22 @@ func (gs GenesisState) Validate() error {
 		if !ok {
 			return fmt.Errorf("invalid client state with ID %s", client.ClientId)
 		}
+
+		if !gs.Params.IsAllowedClient(clientState.ClientType()) {
+			return fmt.Errorf("client type %s not allowed by genesis params", clientState.ClientType())
+		}
 		if err := clientState.Validate(); err != nil {
 			return fmt.Errorf("invalid client %v index %d: %w", client, i, err)
 		}
+
+		// add client id to validClients map
+		validClients[client.ClientId] = true
 	}
 
 	for i, cc := range gs.ClientsConsensus {
-		if err := host.ClientIdentifierValidator(cc.ClientId); err != nil {
-			return fmt.Errorf("invalid client consensus state identifier %s index %d: %w", cc.ClientId, i, err)
+		// check that consensus state is for a client in the genesis clients list
+		if !validClients[cc.ClientId] {
+			return fmt.Errorf("consensus state in genesis has a client id %s that does not map to a genesis client", cc.ClientId)
 		}
 
 		for _, consensusState := range cc.ConsensusStates {
@@ -136,10 +150,6 @@ func (gs GenesisState) Validate() error {
 		}
 	}
 
-	if err := gs.Params.Validate(); err != nil {
-		return err
-	}
-
 	if gs.CreateLocalhost && !gs.Params.IsAllowedClient(exported.Localhost) {
 		return fmt.Errorf("localhost client is not registered on the allowlist")
 	}

diff --git a/x/ibc/core/02-client/types/genesis_test.go b/x/ibc/core/02-client/types/genesis_test.go
@@ -88,7 +88,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() {
 						},
 					),
 				},
-				types.NewParams(exported.Tendermint),
+				types.NewParams(exported.Tendermint, exported.Localhost),
 				false,
 			),
 			expPass: true,
@@ -106,7 +106,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() {
 				},
 				[]types.ClientConsensusStates{
 					types.NewClientConsensusStates(
-						clientID,
+						"/~@$*",
 						[]types.ConsensusStateWithHeight{
 							types.NewConsensusStateWithHeight(
 								header.GetHeight().(types.Height),
@@ -138,7 +138,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() {
 			expPass: false,
 		},
 		{
-			name: "invalid consensus state client id",
+			name: "consensus state client id does not match client id in genesis clients",
 			genState: types.NewGenesisState(
 				[]types.IdentifiedClientState{
 					types.NewIdentifiedClientState(
@@ -150,7 +150,7 @@ func (suite *TypesTestSuite) TestValidateGenesis() {
 				},
 				[]types.ClientConsensusStates{
 					types.NewClientConsensusStates(
-						"(CLIENTID2)",
+						"wrongclientid",
 						[]types.ConsensusStateWithHeight{
 							types.NewConsensusStateWithHeight(
 								types.ZeroHeight(),
@@ -224,6 +224,35 @@ func (suite *TypesTestSuite) TestValidateGenesis() {
 			),
 			expPass: false,
 		},
+		{
+			name: "client in genesis clients is disallowed by params",
+			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),
+					),
+					types.NewIdentifiedClientState(
+						exported.Localhost, localhosttypes.NewClientState("chainID", clientHeight),
+					),
+				},
+				[]types.ClientConsensusStates{
+					types.NewClientConsensusStates(
+						clientID,
+						[]types.ConsensusStateWithHeight{
+							types.NewConsensusStateWithHeight(
+								header.GetHeight().(types.Height),
+								ibctmtypes.NewConsensusState(
+									header.GetTime(), commitmenttypes.NewMerkleRoot(header.Header.GetAppHash()), header.Header.NextValidatorsHash,
+								),
+							),
+						},
+					),
+				},
+				types.NewParams(exported.Solomachine),
+				false,
+			),
+			expPass: false,
+		},
 		{
 			name: "invalid params",
 			genState: types.NewGenesisState(