diff --git a/e2e/tests/core/client_test.go b/e2e/tests/core/client_test.go index 1f9cbb1cf4d..71f2c41727e 100644 --- a/e2e/tests/core/client_test.go +++ b/e2e/tests/core/client_test.go @@ -67,7 +67,8 @@ func (s *ClientTestSuite) TestClientUpdateProposal_Succeeds() { relayer ibc.Relayer subjectClientID string substituteClientID string - badTrustingPeriod = time.Duration(time.Second) + // set the trusting period to a value which will still be valid upon client creation, but invalid before the first update + badTrustingPeriod = time.Duration(time.Second * 10) ) t.Run("create substitute client with correct trusting period", func(t *testing.T) { diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go index 4935cc13299..1be50fdb820 100644 --- a/modules/core/02-client/keeper/client.go +++ b/modules/core/02-client/keeper/client.go @@ -35,6 +35,10 @@ func (k Keeper) CreateClient( return "", err } + if status := k.GetClientStatus(ctx, clientState, clientID); status != exported.Active { + return "", errorsmod.Wrapf(types.ErrClientNotActive, "cannot create client (%s) with status %s", clientID, status) + } + k.Logger(ctx).Info("client created at height", "client-id", clientID, "height", clientState.GetLatestHeight().String()) defer telemetry.IncrCounterWithLabels( diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go index ce338d25853..913d079def5 100644 --- a/modules/core/02-client/keeper/client_test.go +++ b/modules/core/02-client/keeper/client_test.go @@ -18,43 +18,71 @@ import ( ) func (suite *KeeperTestSuite) TestCreateClient() { - cases := []struct { - msg string + var ( clientState exported.ClientState consensusState exported.ConsensusState - expPass bool + ) + + testCases := []struct { + msg string + malleate func() + expPass bool }{ { "success: 07-tendermint client type supported", - ibctm.NewClientState(testChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath), - suite.consensusState, + func() { + clientState = ibctm.NewClientState(testChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) + consensusState = suite.consensusState + }, true, }, + { + "failure: 07-tendermint client status is not active", + func() { + clientState = ibctm.NewClientState(testChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) + tmcs, ok := clientState.(*ibctm.ClientState) + suite.Require().True(ok) + tmcs.FrozenHeight = ibctm.FrozenHeight + consensusState = suite.consensusState + }, + false, + }, { "success: 06-solomachine client type supported", - solomachine.NewClientState(0, &solomachine.ConsensusState{PublicKey: suite.solomachine.ConsensusState().PublicKey, Diversifier: suite.solomachine.Diversifier, Timestamp: suite.solomachine.Time}), - &solomachine.ConsensusState{PublicKey: suite.solomachine.ConsensusState().PublicKey, Diversifier: suite.solomachine.Diversifier, Timestamp: suite.solomachine.Time}, + func() { + clientState = solomachine.NewClientState(0, &solomachine.ConsensusState{PublicKey: suite.solomachine.ConsensusState().PublicKey, Diversifier: suite.solomachine.Diversifier, Timestamp: suite.solomachine.Time}) + consensusState = &solomachine.ConsensusState{PublicKey: suite.solomachine.ConsensusState().PublicKey, Diversifier: suite.solomachine.Diversifier, Timestamp: suite.solomachine.Time} + }, true, }, { "failure: 09-localhost client type not supported", - localhost.NewClientState(clienttypes.GetSelfHeight(suite.ctx)), - nil, + func() { + clientState = localhost.NewClientState(clienttypes.GetSelfHeight(suite.ctx)) + consensusState = nil + }, false, }, } - for i, tc := range cases { - clientID, err := suite.keeper.CreateClient(suite.ctx, tc.clientState, tc.consensusState) - if tc.expPass { - suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.msg) - suite.Require().NotNil(clientID, "valid test case %d failed: %s", i, tc.msg) - suite.Require().True(suite.keeper.ClientStore(suite.ctx, clientID).Has(host.ClientStateKey())) - } else { - suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.msg) - suite.Require().Equal("", clientID, "invalid test case %d passed: %s", i, tc.msg) - suite.Require().False(suite.keeper.ClientStore(suite.ctx, clientID).Has(host.ClientStateKey())) - } + for _, tc := range testCases { + tc := tc + + suite.Run(fmt.Sprintf("Case %s", tc.msg), func() { + suite.SetupTest() // reset + tc.malleate() + + clientID, err := suite.keeper.CreateClient(suite.ctx, clientState, consensusState) + if tc.expPass { + suite.Require().NoError(err) + suite.Require().NotEmpty(clientID) + suite.Require().True(suite.keeper.ClientStore(suite.ctx, clientID).Has(host.ClientStateKey())) + } else { + suite.Require().Error(err) + suite.Require().Empty(clientID) + suite.Require().False(suite.keeper.ClientStore(suite.ctx, clientID).Has(host.ClientStateKey())) + } + }) } }