imp(02-client,03-connection)!: remove ValidateSelfClient

modules/core/02-client/keeper/keeper.go

@@ -308,14 +308,6 @@ func (k *Keeper) GetSelfConsensusState(ctx sdk.Context, height exported.Height)
 	return k.consensusHost.GetSelfConsensusState(ctx, height)
 }
 
-// ValidateSelfClient validates the client parameters for a client of the running chain.
-// This function is only used to validate the client state the counterparty stores for this chain.
-// NOTE: If the client type is not of type Tendermint then delegate to a custom client validator function.
-// This allows support for non-Tendermint clients, for example 08-wasm clients.
-func (k *Keeper) ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error {
-	return k.consensusHost.ValidateSelfClient(ctx, clientState)
-}
-
 // VerifyMembershipProof retrieves the light client module for the clientID and verifies the proof of the existence of a key-value pair at a specified height.
 func (k *Keeper) VerifyMembershipProof(ctx sdk.Context, clientID string, height exported.Height, delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, path exported.Path, value []byte) error {
 	clientModule, err := k.getLightClientModule(ctx, clientID)

modules/core/02-client/types/client.go

@@ -25,7 +25,6 @@ var (
 // ConsensusHost defines an interface used to validate an IBC ClientState and ConsensusState against the host chain's underlying consensus parameters.
 type ConsensusHost interface {
 	GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error)
-	ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error
 }
 
 // NewIdentifiedClientState creates a new IdentifiedClientState instance

modules/core/03-connection/keeper/handshake.go

@@ -88,11 +88,6 @@ func (k *Keeper) ConnOpenTry(
 		)
 	}
 
-	// validate client parameters of a chainB client stored on chainA
-	if err := k.clientKeeper.ValidateSelfClient(ctx, clientState); err != nil {
-		return "", err
-	}
-
 	expectedConsensusState, err := k.clientKeeper.GetSelfConsensusState(ctx, consensusHeight)
 	if err != nil {
 		return "", errorsmod.Wrapf(err, "self consensus state not found for height %s", consensusHeight.String())
@@ -199,11 +194,6 @@ func (k *Keeper) ConnOpenAck(
 		)
 	}
 
-	// validate client parameters of a chainA client stored on chainB
-	if err := k.clientKeeper.ValidateSelfClient(ctx, clientState); err != nil {
-		return err
-	}
-
 	// Retrieve chainA's consensus state at consensusheight
 	expectedConsensusState, err := k.clientKeeper.GetSelfConsensusState(ctx, consensusHeight)
 	if err != nil {

modules/core/03-connection/keeper/handshake_test.go

@@ -3,15 +3,12 @@ package keeper_test
 import (
 	"time"
 
-	sdk "github.com/cosmos/cosmos-sdk/types"
-
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
 	"github.com/cosmos/ibc-go/v8/modules/core/03-connection/types"
 	host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
 	"github.com/cosmos/ibc-go/v8/modules/core/exported"
 	ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
-	"github.com/cosmos/ibc-go/v8/testing/mock"
 )
 
 // TestConnOpenInit - chainA initializes (INIT state) a connection with
@@ -220,21 +217,6 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
 			err := path.EndpointA.ConnOpenInit()
 			suite.Require().NoError(err)
 		}, false},
-		{"override self consensus host", func() {
-			err := path.EndpointA.ConnOpenInit()
-			suite.Require().NoError(err)
-
-			// retrieve client state of chainA to pass as counterpartyClient
-			counterpartyClient = suite.chainA.GetClientState(path.EndpointA.ClientID)
-
-			mockValidator := mock.ConsensusHost{
-				ValidateSelfClientFn: func(ctx sdk.Context, clientState exported.ClientState) error {
-					return mock.MockApplicationCallbackError
-				},
-			}
-
-			suite.chainB.App.GetIBCKeeper().SetConsensusHost(&mockValidator)
-		}, false},
 	}
 
 	for _, tc := range testCases {

modules/core/03-connection/types/expected_keepers.go

@@ -15,7 +15,6 @@ type ClientKeeper interface {
 	GetClientState(ctx sdk.Context, clientID string) (exported.ClientState, bool)
 	GetClientConsensusState(ctx sdk.Context, clientID string, height exported.Height) (exported.ConsensusState, bool)
 	GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error)
-	ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error
 	VerifyMembershipProof(ctx sdk.Context, clientID string, height exported.Height, delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, path exported.Path, value []byte) error
 	VerifyNonMembership(ctx sdk.Context, clientID string, height exported.Height, delayTimePeriod uint64, delayBlockPeriod uint64, proof []byte, path exported.Path) error
 	IterateClientStates(ctx sdk.Context, prefix []byte, cb func(string, exported.ClientState) bool)

modules/light-clients/07-tendermint/consensus_host.go

@@ -2,17 +2,13 @@ package tendermint
 
 import (
 	"context"
-	"reflect"
 	"time"
 
 	errorsmod "cosmossdk.io/errors"
-	upgradetypes "cosmossdk.io/x/upgrade/types"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 
-	"github.com/cometbft/cometbft/light"
-
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
 	commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types"
 	ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
@@ -69,70 +65,3 @@ func (c *ConsensusHost) GetSelfConsensusState(ctx sdk.Context, height exported.H
 
 	return consensusState, nil
 }
-
-// ValidateSelfClient implements the 02-client clienttypes.ConsensusHost interface.
-func (c *ConsensusHost) ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error {
-	tmClient, ok := clientState.(*ClientState)
-	if !ok {
-		return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "client must be a Tendermint client, expected: %T, got: %T", &ClientState{}, tmClient)
-	}
-
-	if !tmClient.FrozenHeight.IsZero() {
-		return clienttypes.ErrClientFrozen
-	}
-
-	if ctx.ChainID() != tmClient.ChainId {
-		return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "invalid chain-id. expected: %s, got: %s",
-			ctx.ChainID(), tmClient.ChainId)
-	}
-
-	revision := clienttypes.ParseChainID(ctx.ChainID())
-
-	// client must be in the same revision as executing chain
-	if tmClient.LatestHeight.RevisionNumber != revision {
-		return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "client is not in the same revision as the chain. expected revision: %d, got: %d",
-			tmClient.LatestHeight.RevisionNumber, revision)
-	}
-
-	selfHeight := clienttypes.NewHeight(revision, uint64(ctx.BlockHeight()))
-	if tmClient.LatestHeight.GTE(selfHeight) {
-		return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "client has LatestHeight %d greater than or equal to chain height %d",
-			tmClient.LatestHeight, selfHeight)
-	}
-
-	expectedProofSpecs := commitmenttypes.GetSDKSpecs()
-	if !reflect.DeepEqual(expectedProofSpecs, tmClient.ProofSpecs) {
-		return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "client has invalid proof specs. expected: %v got: %v",
-			expectedProofSpecs, tmClient.ProofSpecs)
-	}
-
-	if err := light.ValidateTrustLevel(tmClient.TrustLevel.ToTendermint()); err != nil {
-		return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "trust-level invalid: %v", err)
-	}
-
-	expectedUbdPeriod, err := c.stakingKeeper.UnbondingTime(ctx)
-	if err != nil {
-		return errorsmod.Wrapf(err, "failed to retrieve unbonding period")
-	}
-
-	if expectedUbdPeriod != tmClient.UnbondingPeriod {
-		return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "invalid unbonding period. expected: %s, got: %s",
-			expectedUbdPeriod, tmClient.UnbondingPeriod)
-	}
-
-	if tmClient.UnbondingPeriod < tmClient.TrustingPeriod {
-		return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "unbonding period must be greater than trusting period. unbonding period (%d) < trusting period (%d)",
-			tmClient.UnbondingPeriod, tmClient.TrustingPeriod)
-	}
-
-	if len(tmClient.UpgradePath) != 0 {
-		// For now, SDK IBC implementation assumes that upgrade path (if defined) is defined by SDK upgrade module
-		expectedUpgradePath := []string{upgradetypes.StoreKey, upgradetypes.KeyUpgradedIBCState}
-		if !reflect.DeepEqual(expectedUpgradePath, tmClient.UpgradePath) {
-			return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "upgrade path must be the upgrade path defined by upgrade module. expected %v, got %v",
-				expectedUpgradePath, tmClient.UpgradePath)
-		}
-	}
-
-	return nil
-}

modules/light-clients/07-tendermint/consensus_host_test.go

@@ -5,11 +5,8 @@ import (
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
-	commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types"
 	"github.com/cosmos/ibc-go/v8/modules/core/exported"
 	solomachine "github.com/cosmos/ibc-go/v8/modules/light-clients/06-solomachine"
-	ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint"
-	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 	"github.com/cosmos/ibc-go/v8/testing/mock"
 )
 
@@ -111,139 +108,3 @@ func (suite *TendermintTestSuite) TestGetSelfConsensusState() {
 		})
 	}
 }
-
-func (suite *TendermintTestSuite) TestValidateSelfClient() {
-	testClientHeight := clienttypes.GetSelfHeight(suite.chainA.GetContext())
-	testClientHeight.RevisionHeight--
-
-	var clientState exported.ClientState
-
-	testCases := []struct {
-		name     string
-		malleate func()
-		expError error
-	}{
-		{
-			name: "success",
-			malleate: func() {
-				clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath)
-			},
-			expError: nil,
-		},
-		{
-			name: "success with nil UpgradePath",
-			malleate: func() {
-				clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), nil)
-			},
-			expError: nil,
-		},
-		{
-			name: "success with custom self validator: solomachine",
-			malleate: func() {
-				clientState = solomachine.NewClientState(1, &solomachine.ConsensusState{})
-
-				smConsensusHost := &mock.ConsensusHost{
-					ValidateSelfClientFn: func(ctx sdk.Context, clientState exported.ClientState) error {
-						smClientState, ok := clientState.(*solomachine.ClientState)
-						suite.Require().True(ok)
-						suite.Require().Equal(uint64(1), smClientState.Sequence)
-
-						return nil
-					},
-				}
-
-				// add mock validation logic
-				suite.chainA.App.GetIBCKeeper().SetConsensusHost(smConsensusHost)
-			},
-			expError: nil,
-		},
-		{
-			name: "frozen client",
-			malleate: func() {
-				clientState = &ibctm.ClientState{ChainId: suite.chainA.ChainID, TrustLevel: ibctm.DefaultTrustLevel, TrustingPeriod: trustingPeriod, UnbondingPeriod: ubdPeriod, MaxClockDrift: maxClockDrift, FrozenHeight: testClientHeight, LatestHeight: testClientHeight, ProofSpecs: commitmenttypes.GetSDKSpecs(), UpgradePath: ibctesting.UpgradePath}
-			},
-			expError: clienttypes.ErrClientFrozen,
-		},
-		{
-			name: "incorrect chainID",
-			malleate: func() {
-				clientState = ibctm.NewClientState("gaiatestnet", ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath)
-			},
-			expError: clienttypes.ErrInvalidClient,
-		},
-		{
-			name: "invalid client height",
-			malleate: func() {
-				clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, clienttypes.GetSelfHeight(suite.chainA.GetContext()).Increment().(clienttypes.Height), commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath)
-			},
-			expError: clienttypes.ErrInvalidClient,
-		},
-		{
-			name: "invalid client type",
-			malleate: func() {
-				clientState = solomachine.NewClientState(0, &solomachine.ConsensusState{})
-			},
-			expError: clienttypes.ErrInvalidClient,
-		},
-		{
-			name: "invalid client revision",
-			malleate: func() {
-				clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, clienttypes.NewHeight(1, 5), commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath)
-			},
-			expError: clienttypes.ErrInvalidClient,
-		},
-		{
-			name: "invalid proof specs",
-			malleate: func() {
-				clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, nil, ibctesting.UpgradePath)
-			},
-			expError: clienttypes.ErrInvalidClient,
-		},
-		{
-			name: "invalid trust level",
-			malleate: func() {
-				clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.Fraction{Numerator: 0, Denominator: 1}, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath)
-			},
-			expError: clienttypes.ErrInvalidClient,
-		},
-		{
-			name: "invalid unbonding period",
-			malleate: func() {
-				clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+10, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath)
-			},
-			expError: clienttypes.ErrInvalidClient,
-		},
-		{
-			name: "invalid trusting period",
-			malleate: func() {
-				clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, ubdPeriod+10, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath)
-			},
-			expError: clienttypes.ErrInvalidClient,
-		},
-		{
-			name: "invalid upgrade path",
-			malleate: func() {
-				clientState = ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), []string{"bad", "upgrade", "path"})
-			},
-			expError: clienttypes.ErrInvalidClient,
-		},
-	}
-
-	for _, tc := range testCases {
-		tc := tc
-		suite.Run(tc.name, func() {
-			suite.SetupTest()
-
-			tc.malleate()
-
-			err := suite.chainA.App.GetIBCKeeper().ClientKeeper.ValidateSelfClient(suite.chainA.GetContext(), clientState)
-
-			expPass := tc.expError == nil
-			if expPass {
-				suite.Require().NoError(err, "expected valid client for case: %s", tc.name)
-			} else {
-				suite.Require().Error(err, "expected invalid client for case: %s", tc.name)
-			}
-		})
-	}
-}

modules/light-clients/08-wasm/types/consensus_host.go

@@ -3,8 +3,6 @@ package types
 import (
 	"errors"
 
-	errorsmod "cosmossdk.io/errors"
-
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
@@ -20,7 +18,7 @@ type WasmConsensusHost struct {
 
 var _ clienttypes.ConsensusHost = (*WasmConsensusHost)(nil)
 
-// NewWasmConsensusHost creates and returns a new ConsensusHost for wasm wrapped consensus client state and consensus state self validation.
+// NewWasmConsensusHost creates and returns a new ConsensusHost for wasm wrapped consensus client state.
 func NewWasmConsensusHost(cdc codec.BinaryCodec, delegate clienttypes.ConsensusHost) (*WasmConsensusHost, error) {
 	if cdc == nil {
 		return nil, errors.New("wasm consensus host codec is nil")
@@ -55,23 +53,3 @@ func (w *WasmConsensusHost) GetSelfConsensusState(ctx sdk.Context, height export
 
 	return wasmConsensusState, nil
 }
-
-// ValidateSelfClient implements the 02-client types.ConsensusHost interface.
-func (w *WasmConsensusHost) ValidateSelfClient(ctx sdk.Context, clientState exported.ClientState) error {
-	wasmClientState, ok := clientState.(*ClientState)
-	if !ok {
-		return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "client must be a wasm client, expected: %T, got: %T", ClientState{}, wasmClientState)
-	}
-
-	if wasmClientState.Data == nil {
-		return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "wasm client state data is nil")
-	}
-
-	// unmarshal the wasmClientState bytes into the ClientState interface and call self validation
-	var unwrappedClientState exported.ClientState
-	if err := w.cdc.UnmarshalInterface(wasmClientState.Data, &unwrappedClientState); err != nil {
-		return err
-	}
-
-	return w.delegate.ValidateSelfClient(ctx, unwrappedClientState)
-}