chore: Address linter findings on core/02-client (#6119)

* Address linter findings on core/02-client * Update modules/core/02-client/keeper/keeper.go Co-authored-by: Carlos Rodriguez <[email protected]> * Better panic messages. --------- Co-authored-by: Carlos Rodriguez <[email protected]>
cosmos · Apr 9, 2024 · 8547b0b · 8547b0b
1 parent b1c65fd
commit 8547b0b
Show file tree

Hide file tree

Showing 12 changed files with 62 additions and 32 deletions.
diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go
@@ -216,13 +216,15 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
 			suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientConsensusState(suite.chainA.GetContext(), clientID, updateHeader.GetHeight(), conflictConsState)
 		}, true, true},
 		{"misbehaviour detection: monotonic time violation", func() {
-			clientState := path.EndpointA.GetClientState().(*ibctm.ClientState)
+			clientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState)
+			suite.Require().True(ok)
 			clientID := path.EndpointA.ClientID
 			trustedHeight := clientState.LatestHeight
 
 			// store intermediate consensus state at a time greater than updateHeader time
 			// this will break time monotonicity
-			incrementedClientHeight := clientState.LatestHeight.Increment().(clienttypes.Height)
+			incrementedClientHeight, ok := clientState.LatestHeight.Increment().(clienttypes.Height)
+			suite.Require().True(ok)
 			intermediateConsState := &ibctm.ConsensusState{
 				Timestamp:          suite.coordinator.CurrentTime.Add(2 * time.Hour),
 				NextValidatorsHash: suite.chainB.Vals.Hash(),
@@ -238,7 +240,8 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
 			updateHeader = createFutureUpdateFn(trustedHeight)
 		}, true, true},
 		{"client state not found", func() {
-			clientState := path.EndpointA.GetClientState().(*ibctm.ClientState)
+			clientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState)
+			suite.Require().True(ok)
 			updateHeader = createFutureUpdateFn(clientState.LatestHeight)
 
 			path.EndpointA.ClientID = ibctesting.InvalidID
@@ -247,21 +250,25 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
 			clientState := path.EndpointA.GetClientState()
 			tmClient, ok := clientState.(*ibctm.ClientState)
 			suite.Require().True(ok)
-			tmClient.LatestHeight = tmClient.LatestHeight.Increment().(clienttypes.Height)
+			tmClient.LatestHeight, ok = tmClient.LatestHeight.Increment().(clienttypes.Height)
+			suite.Require().True(ok)
 
 			suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID, clientState)
 			updateHeader = createFutureUpdateFn(tmClient.LatestHeight)
 		}, false, false},
 		{"client is not active", func() {
-			clientState := path.EndpointA.GetClientState().(*ibctm.ClientState)
+			clientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState)
+			suite.Require().True(ok)
 			clientState.FrozenHeight = clienttypes.NewHeight(1, 1)
 			suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID, clientState)
 			updateHeader = createFutureUpdateFn(clientState.LatestHeight)
 		}, false, false},
 		{"invalid header", func() {
-			clientState := path.EndpointA.GetClientState().(*ibctm.ClientState)
+			clientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState)
+			suite.Require().True(ok)
 			updateHeader = createFutureUpdateFn(clientState.LatestHeight)
-			updateHeader.TrustedHeight = updateHeader.TrustedHeight.Increment().(clienttypes.Height)
+			updateHeader.TrustedHeight, ok = updateHeader.TrustedHeight.Increment().(clienttypes.Height)
+			suite.Require().True(ok)
 		}, false, false},
 	}
 
@@ -275,16 +282,19 @@ func (suite *KeeperTestSuite) TestUpdateClientTendermint() {
 			tc.malleate()
 
 			var clientState *ibctm.ClientState
+			var ok bool
 			if tc.expPass {
-				clientState = path.EndpointA.GetClientState().(*ibctm.ClientState)
+				clientState, ok = path.EndpointA.GetClientState().(*ibctm.ClientState)
+				suite.Require().True(ok)
 			}
 
 			err := suite.chainA.App.GetIBCKeeper().ClientKeeper.UpdateClient(suite.chainA.GetContext(), path.EndpointA.ClientID, updateHeader)
 
 			if tc.expPass {
 				suite.Require().NoError(err, err)
 
-				newClientState := path.EndpointA.GetClientState().(*ibctm.ClientState)
+				newClientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState)
+				suite.Require().True(ok)
 
 				if tc.expFreeze {
 					suite.Require().True(!newClientState.FrozenHeight.IsZero(), "client did not freeze after conflicting header was submitted to UpdateClient")
@@ -463,7 +473,8 @@ func (suite *KeeperTestSuite) TestUpgradeClient() {
 			path = ibctesting.NewPath(suite.chainA, suite.chainB)
 			path.SetupClients()
 
-			clientState := path.EndpointA.GetClientState().(*ibctm.ClientState)
+			clientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState)
+			suite.Require().True(ok)
 			revisionNumber := clienttypes.ParseChainID(clientState.ChainId)
 
 			newChainID, err := clienttypes.SetRevisionNumber(clientState.ChainId, revisionNumber+1)
@@ -589,7 +600,8 @@ func (suite *KeeperTestSuite) TestRecoverClient() {
 			func() {
 				tmClientState, ok := subjectClientState.(*ibctm.ClientState)
 				suite.Require().True(ok)
-				tmClientState.LatestHeight = substituteClientState.(*ibctm.ClientState).LatestHeight.Increment().(clienttypes.Height)
+				tmClientState.LatestHeight, ok = substituteClientState.(*ibctm.ClientState).LatestHeight.Increment().(clienttypes.Height)
+				suite.Require().True(ok)
 				suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), subject, tmClientState)
 			},
 			clienttypes.ErrInvalidHeight,
@@ -665,7 +677,8 @@ func (suite *KeeperTestSuite) TestRecoverClient() {
 
 				// Assert that client status is now Active
 				clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID)
-				tmClientState := subjectPath.EndpointA.GetClientState().(*ibctm.ClientState)
+				tmClientState, ok := subjectPath.EndpointA.GetClientState().(*ibctm.ClientState)
+				suite.Require().True(ok)
 				suite.Require().Equal(tmClientState.Status(suite.chainA.GetContext(), clientStore, suite.chainA.App.AppCodec()), exported.Active)
 
 			} else {

diff --git a/modules/core/02-client/keeper/events_test.go b/modules/core/02-client/keeper/events_test.go
@@ -18,7 +18,8 @@ func (suite *KeeperTestSuite) TestMsgCreateClientEvents() {
 	tmConfig, ok := path.EndpointA.ClientConfig.(*ibctesting.TendermintConfig)
 	suite.Require().True(ok)
 
-	height := path.EndpointA.Counterparty.Chain.LatestCommittedHeader.GetHeight().(clienttypes.Height)
+	height, ok := path.EndpointA.Counterparty.Chain.LatestCommittedHeader.GetHeight().(clienttypes.Height)
+	suite.Require().True(ok)
 	clientState := ibctm.NewClientState(
 		path.EndpointA.Counterparty.Chain.ChainID, tmConfig.TrustLevel, tmConfig.TrustingPeriod, tmConfig.UnbondingPeriod, tmConfig.MaxClockDrift,
 		height, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath)
@@ -56,7 +57,8 @@ func (suite *KeeperTestSuite) TestMsgUpdateClientEvents() {
 
 	suite.chainB.Coordinator.CommitBlock(suite.chainB)
 
-	clientState := path.EndpointA.GetClientState().(*ibctm.ClientState)
+	clientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState)
+	suite.Require().True(ok)
 	trustedHeight := clientState.LatestHeight
 	header, err := suite.chainB.IBCClientHeader(suite.chainB.LatestCommittedHeader, trustedHeight)
 	suite.Require().NoError(err)

diff --git a/modules/core/02-client/keeper/grpc_query_test.go b/modules/core/02-client/keeper/grpc_query_test.go
@@ -344,7 +344,8 @@ func (suite *KeeperTestSuite) TestQueryConsensusStates() {
 				path := ibctesting.NewPath(suite.chainA, suite.chainB)
 				path.SetupClients()
 
-				height1 := path.EndpointA.GetClientLatestHeight().(types.Height)
+				height1, ok := path.EndpointA.GetClientLatestHeight().(types.Height)
+				suite.Require().True(ok)
 				expConsensusStates = append(
 					expConsensusStates,
 					types.NewConsensusStateWithHeight(
@@ -354,8 +355,8 @@ func (suite *KeeperTestSuite) TestQueryConsensusStates() {
 
 				err := path.EndpointA.UpdateClient()
 				suite.Require().NoError(err)
-
-				height2 := path.EndpointA.GetClientLatestHeight().(types.Height)
+				height2, ok := path.EndpointA.GetClientLatestHeight().(types.Height)
+				suite.Require().True(ok)
 				expConsensusStates = append(
 					expConsensusStates,
 					types.NewConsensusStateWithHeight(
@@ -548,10 +549,12 @@ func (suite *KeeperTestSuite) TestQueryClientStatus() {
 			func() {
 				path := ibctesting.NewPath(suite.chainA, suite.chainB)
 				path.SetupClients()
-				clientState := path.EndpointA.GetClientState().(*ibctm.ClientState)
+				clientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState)
+				suite.Require().True(ok)
 
 				// increment latest height so no consensus state is stored
-				clientState.LatestHeight = clientState.LatestHeight.Increment().(types.Height)
+				clientState.LatestHeight, ok = clientState.LatestHeight.Increment().(types.Height)
+				suite.Require().True(ok)
 				path.EndpointA.SetClientState(clientState)
 
 				req = &types.QueryClientStatusRequest{
@@ -565,7 +568,8 @@ func (suite *KeeperTestSuite) TestQueryClientStatus() {
 			func() {
 				path := ibctesting.NewPath(suite.chainA, suite.chainB)
 				path.SetupClients()
-				clientState := path.EndpointA.GetClientState().(*ibctm.ClientState)
+				clientState, ok := path.EndpointA.GetClientState().(*ibctm.ClientState)
+				suite.Require().True(ok)
 
 				clientState.FrozenHeight = types.NewHeight(0, 1)
 				path.EndpointA.SetClientState(clientState)
@@ -636,7 +640,9 @@ func (suite *KeeperTestSuite) TestQueryUpgradedClientState() {
 				suite.Require().NoError(err)
 				suite.Require().NotNil(resp)
 
-				expClientState = clientState.(*ibctm.ClientState)
+				var ok bool
+				expClientState, ok = clientState.(*ibctm.ClientState)
+				suite.Require().True(ok)
 			},
 			nil,
 		},

diff --git a/modules/core/02-client/keeper/keeper.go b/modules/core/02-client/keeper/keeper.go
@@ -422,7 +422,12 @@ func (k Keeper) GetClientLatestHeight(ctx sdk.Context, clientID string) types.He
 		return types.ZeroHeight()
 	}
 
-	return clientModule.LatestHeight(ctx, clientID).(types.Height)
+	var latestHeight types.Height
+	latestHeight, ok := clientModule.LatestHeight(ctx, clientID).(types.Height)
+	if !ok {
+		panic(fmt.Errorf("cannot convert %T to %T", clientModule.LatestHeight, latestHeight))
+	}
+	return latestHeight
 }
 
 // GetClientTimestampAtHeight returns the timestamp in nanoseconds of the consensus state at the given height.

diff --git a/modules/core/02-client/keeper/migrations.go b/modules/core/02-client/keeper/migrations.go
@@ -3,7 +3,7 @@ package keeper
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
-	v7 "github.com/cosmos/ibc-go/v8/modules/core/02-client/migrations/v7"
+	"github.com/cosmos/ibc-go/v8/modules/core/02-client/migrations/v7"
 	"github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
 )
 

diff --git a/modules/core/02-client/migrations/v7/genesis_test.go b/modules/core/02-client/migrations/v7/genesis_test.go
@@ -7,7 +7,7 @@ import (
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
 
 	ibcclient "github.com/cosmos/ibc-go/v8/modules/core/02-client"
-	v7 "github.com/cosmos/ibc-go/v8/modules/core/02-client/migrations/v7"
+	"github.com/cosmos/ibc-go/v8/modules/core/02-client/migrations/v7"
 	"github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
 	host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
 	ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported"
@@ -67,7 +67,8 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() {
 		// set in store for ease of determining expected genesis
 		clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), sm.ClientID)
 
-		cdc := suite.chainA.App.AppCodec().(*codec.ProtoCodec)
+		cdc, ok := suite.chainA.App.AppCodec().(*codec.ProtoCodec)
+		suite.Require().True(ok)
 		v7.RegisterInterfaces(cdc.InterfaceRegistry())
 
 		bz, err := cdc.MarshalInterface(legacyClientState)

diff --git a/modules/core/02-client/migrations/v7/localhost_test.go b/modules/core/02-client/migrations/v7/localhost_test.go
@@ -1,7 +1,7 @@
 package v7_test
 
 import (
-	v7 "github.com/cosmos/ibc-go/v8/modules/core/02-client/migrations/v7"
+	"github.com/cosmos/ibc-go/v8/modules/core/02-client/migrations/v7"
 	host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
 	"github.com/cosmos/ibc-go/v8/modules/core/exported"
 )

diff --git a/modules/core/02-client/migrations/v7/store_test.go b/modules/core/02-client/migrations/v7/store_test.go
@@ -8,7 +8,7 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/codec"
 
-	v7 "github.com/cosmos/ibc-go/v8/modules/core/02-client/migrations/v7"
+	"github.com/cosmos/ibc-go/v8/modules/core/02-client/migrations/v7"
 	"github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
 	host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
 	ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported"
@@ -104,7 +104,8 @@ func (suite *MigrationsV7TestSuite) createSolomachineClients(solomachines []*ibc
 			AllowUpdateAfterProposal: true,
 		}
 
-		cdc := suite.chainA.App.AppCodec().(*codec.ProtoCodec)
+		cdc, ok := suite.chainA.App.AppCodec().(*codec.ProtoCodec)
+		suite.Require().True(ok)
 		v7.RegisterInterfaces(cdc.InterfaceRegistry())
 
 		bz, err := cdc.MarshalInterface(legacyClientState)

diff --git a/modules/core/02-client/types/client.go b/modules/core/02-client/types/client.go
@@ -6,7 +6,7 @@ import (
 	"sort"
 	"strings"
 
-	proto "github.com/cosmos/gogoproto/proto"
+	"github.com/cosmos/gogoproto/proto"
 
 	errorsmod "cosmossdk.io/errors"
 

diff --git a/modules/core/02-client/types/client_test.go b/modules/core/02-client/types/client_test.go
@@ -27,7 +27,8 @@ func (suite *TypesTestSuite) TestMarshalConsensusStateWithHeight() {
 				path := ibctesting.NewPath(suite.chainA, suite.chainB)
 				path.SetupClients()
 
-				latestHeight := path.EndpointA.GetClientLatestHeight().(types.Height)
+				latestHeight, ok := path.EndpointA.GetClientLatestHeight().(types.Height)
+				suite.Require().True(ok)
 				consensusState, ok := suite.chainA.GetConsensusState(path.EndpointA.ClientID, latestHeight)
 				suite.Require().True(ok)
 

diff --git a/modules/core/02-client/types/codec.go b/modules/core/02-client/types/codec.go
@@ -1,7 +1,7 @@
 package types
 
 import (
-	proto "github.com/cosmos/gogoproto/proto"
+	"github.com/cosmos/gogoproto/proto"
 
 	errorsmod "cosmossdk.io/errors"
 

diff --git a/modules/core/02-client/types/legacy_proposal_test.go b/modules/core/02-client/types/legacy_proposal_test.go
@@ -74,7 +74,8 @@ func (suite *TypesTestSuite) TestMarshalClientUpdateProposalProposal() {
 	cdc := codec.NewProtoCodec(ir)
 
 	// marshal message
-	content := proposal.(*types.ClientUpdateProposal)
+	content, ok := proposal.(*types.ClientUpdateProposal)
+	suite.Require().True(ok)
 	bz, err := cdc.MarshalJSON(content)
 	suite.Require().NoError(err)