feat: adding ConsensusHost interface for custom self client/consensus state validation (#6055)

Co-authored-by: chatton <[email protected]>

Author: damiannolan

CHANGELOG.md

@@ -53,6 +53,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### Features
 
 * (apps/27-interchain-accounts) [\#5785](https://github.com/cosmos/ibc-go/pull/5785) Introduce a new tx message that ICA host submodule can use to query the chain (only those marked with `module_query_safe`) and write the responses to the acknowledgement.
+* (core) [\#6055](https://github.com/cosmos/ibc-go/pull/6055) Introduce a new interface `ConsensusHost` used to validate an IBC `ClientState` and `ConsensusState` against the host chain's underlying consensus parameters.
 
 ### Bug Fixes
 

modules/core/02-client/abci.go

@@ -9,7 +9,7 @@ import (
 )
 
 // BeginBlocker is used to perform IBC client upgrades
-func BeginBlocker(ctx sdk.Context, k keeper.Keeper) {
+func BeginBlocker(ctx sdk.Context, k *keeper.Keeper) {
 	plan, err := k.GetUpgradePlan(ctx)
 	if err == nil {
 		// Once we are at the last block this chain will commit, set the upgraded consensus state

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

@@ -3,7 +3,6 @@ package keeper
 import (
 	"errors"
 	"fmt"
-	"reflect"
 	"strings"
 
 	errorsmod "cosmossdk.io/errors"
@@ -15,12 +14,8 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
-	"github.com/cometbft/cometbft/light"
-
 	"github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
-	commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types"
 	host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
-	ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
 	"github.com/cosmos/ibc-go/v8/modules/core/exported"
 	ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint"
 	localhost "github.com/cosmos/ibc-go/v8/modules/light-clients/09-localhost"
@@ -32,8 +27,8 @@ type Keeper struct {
 	storeKey       storetypes.StoreKey
 	cdc            codec.BinaryCodec
 	router         *types.Router
+	consensusHost  types.ConsensusHost
 	legacySubspace types.ParamSubspace
-	stakingKeeper  types.StakingKeeper
 	upgradeKeeper  types.UpgradeKeeper
 }
 
@@ -47,8 +42,8 @@ func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace ty
 		storeKey:       key,
 		cdc:            cdc,
 		router:         router,
+		consensusHost:  ibctm.NewConsensusHost(sk),
 		legacySubspace: legacySubspace,
-		stakingKeeper:  sk,
 		upgradeKeeper:  uk,
 	}
 }
@@ -88,6 +83,15 @@ func (k Keeper) UpdateLocalhostClient(ctx sdk.Context, clientState exported.Clie
 	return clientModule.UpdateState(ctx, exported.LocalhostClientID, nil)
 }
 
+// SetSelfConsensusHost sets a custom ConsensusHost for self client state and consensus state validation.
+func (k *Keeper) SetSelfConsensusHost(consensusHost types.ConsensusHost) {
+	if consensusHost == nil {
+		panic(fmt.Errorf("cannot set a nil self consensus host"))
+	}
+
+	k.consensusHost = consensusHost
+}
+
 // GenerateClientIdentifier returns the next client identifier.
 func (k Keeper) GenerateClientIdentifier(ctx sdk.Context, clientType string) string {
 	nextClientSeq := k.GetNextClientSequence(ctx)
@@ -99,7 +103,7 @@ func (k Keeper) GenerateClientIdentifier(ctx sdk.Context, clientType string) str
 }
 
 // GetClientState gets a particular client from the store
-func (k Keeper) GetClientState(ctx sdk.Context, clientID string) (exported.ClientState, bool) {
+func (k *Keeper) GetClientState(ctx sdk.Context, clientID string) (exported.ClientState, bool) {
 	store := k.ClientStore(ctx, clientID)
 	bz := store.Get(host.ClientStateKey())
 	if len(bz) == 0 {
@@ -111,13 +115,13 @@ func (k Keeper) GetClientState(ctx sdk.Context, clientID string) (exported.Clien
 }
 
 // SetClientState sets a particular Client to the store
-func (k Keeper) SetClientState(ctx sdk.Context, clientID string, clientState exported.ClientState) {
+func (k *Keeper) SetClientState(ctx sdk.Context, clientID string, clientState exported.ClientState) {
 	store := k.ClientStore(ctx, clientID)
 	store.Set(host.ClientStateKey(), k.MustMarshalClientState(clientState))
 }
 
 // GetClientConsensusState gets the stored consensus state from a client at a given height.
-func (k Keeper) GetClientConsensusState(ctx sdk.Context, clientID string, height exported.Height) (exported.ConsensusState, bool) {
+func (k *Keeper) GetClientConsensusState(ctx sdk.Context, clientID string, height exported.Height) (exported.ConsensusState, bool) {
 	store := k.ClientStore(ctx, clientID)
 	bz := store.Get(host.ConsensusStateKey(height))
 	if len(bz) == 0 {
@@ -308,96 +312,15 @@ func (k Keeper) GetLatestClientConsensusState(ctx sdk.Context, clientID string)
 // and returns the expected consensus state at that height.
 // For now, can only retrieve self consensus states for the current revision
 func (k Keeper) GetSelfConsensusState(ctx sdk.Context, height exported.Height) (exported.ConsensusState, error) {
-	selfHeight, ok := height.(types.Height)
-	if !ok {
-		return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", types.Height{}, height)
-	}
-	// check that height revision matches chainID revision
-	revision := types.ParseChainID(ctx.ChainID())
-	if revision != height.GetRevisionNumber() {
-		return nil, errorsmod.Wrapf(types.ErrInvalidHeight, "chainID revision number does not match height revision number: expected %d, got %d", revision, height.GetRevisionNumber())
-	}
-	histInfo, err := k.stakingKeeper.GetHistoricalInfo(ctx, int64(selfHeight.RevisionHeight))
-	if err != nil {
-		return nil, errorsmod.Wrapf(err, "height %d", selfHeight.RevisionHeight)
-	}
-
-	consensusState := &ibctm.ConsensusState{
-		Timestamp:          histInfo.Header.Time,
-		Root:               commitmenttypes.NewMerkleRoot(histInfo.Header.GetAppHash()),
-		NextValidatorsHash: histInfo.Header.NextValidatorsHash,
-	}
-
-	return consensusState, nil
+	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
-// Client must be in same revision as the executing chain
+// 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 {
-	tmClient, ok := clientState.(*ibctm.ClientState)
-	if !ok {
-		return errorsmod.Wrapf(types.ErrInvalidClient, "client must be a Tendermint client, expected: %T, got: %T",
-			&ibctm.ClientState{}, tmClient)
-	}
-
-	if !tmClient.FrozenHeight.IsZero() {
-		return types.ErrClientFrozen
-	}
-
-	if ctx.ChainID() != tmClient.ChainId {
-		return errorsmod.Wrapf(types.ErrInvalidClient, "invalid chain-id. expected: %s, got: %s",
-			ctx.ChainID(), tmClient.ChainId)
-	}
-
-	revision := types.ParseChainID(ctx.ChainID())
-
-	// client must be in the same revision as executing chain
-	if tmClient.LatestHeight.RevisionNumber != revision {
-		return errorsmod.Wrapf(types.ErrInvalidClient, "client is not in the same revision as the chain. expected revision: %d, got: %d",
-			tmClient.LatestHeight.RevisionNumber, revision)
-	}
-
-	selfHeight := types.NewHeight(revision, uint64(ctx.BlockHeight()))
-	if tmClient.LatestHeight.GTE(selfHeight) {
-		return errorsmod.Wrapf(types.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(types.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(types.ErrInvalidClient, "trust-level invalid: %v", err)
-	}
-
-	expectedUbdPeriod, err := k.stakingKeeper.UnbondingTime(ctx)
-	if err != nil {
-		return errorsmod.Wrapf(err, "failed to retrieve unbonding period")
-	}
-
-	if expectedUbdPeriod != tmClient.UnbondingPeriod {
-		return errorsmod.Wrapf(types.ErrInvalidClient, "invalid unbonding period. expected: %s, got: %s",
-			expectedUbdPeriod, tmClient.UnbondingPeriod)
-	}
-
-	if tmClient.UnbondingPeriod < tmClient.TrustingPeriod {
-		return errorsmod.Wrapf(types.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(types.ErrInvalidClient, "upgrade path must be the upgrade path defined by upgrade module. expected %v, got %v",
-				expectedUpgradePath, tmClient.UpgradePath)
-		}
-	}
-	return nil
+	return k.consensusHost.ValidateSelfClient(ctx, clientState)
 }
 
 // GetUpgradePlan executes the upgrade keeper GetUpgradePlan function.

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

@@ -25,7 +25,6 @@ import (
 	commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types"
 	host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
 	"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"
 	localhost "github.com/cosmos/ibc-go/v8/modules/light-clients/09-localhost"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
@@ -45,10 +44,7 @@ const (
 	maxClockDrift  time.Duration = time.Second * 10
 )
 
-var (
-	testClientHeight          = types.NewHeight(0, 5)
-	testClientHeightRevision1 = types.NewHeight(1, 5)
-)
+var testClientHeight = types.NewHeight(0, 5)
 
 type KeeperTestSuite struct {
 	testifysuite.Suite
@@ -85,7 +81,7 @@ func (suite *KeeperTestSuite) SetupTest() {
 
 	suite.cdc = app.AppCodec()
 	suite.ctx = app.BaseApp.NewContext(isCheckTx)
-	suite.keeper = &app.IBCKeeper.ClientKeeper
+	suite.keeper = app.IBCKeeper.ClientKeeper
 	suite.privVal = cmttypes.NewMockPV()
 	pubKey, err := suite.privVal.GetPubKey()
 	suite.Require().NoError(err)
@@ -145,90 +141,6 @@ func (suite *KeeperTestSuite) TestSetClientConsensusState() {
 	suite.Require().Equal(suite.consensusState, tmConsState, "ConsensusState not stored correctly")
 }
 
-func (suite *KeeperTestSuite) TestValidateSelfClient() {
-	testClientHeight := types.GetSelfHeight(suite.chainA.GetContext())
-	testClientHeight.RevisionHeight--
-
-	testCases := []struct {
-		name        string
-		clientState exported.ClientState
-		expPass     bool
-	}{
-		{
-			"success",
-			ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath),
-			true,
-		},
-		{
-			"success with nil UpgradePath",
-			ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), nil),
-			true,
-		},
-		{
-			"frozen client",
-			&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},
-			false,
-		},
-		{
-			"incorrect chainID",
-			ibctm.NewClientState("gaiatestnet", ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath),
-			false,
-		},
-		{
-			"invalid client height",
-			ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, types.GetSelfHeight(suite.chainA.GetContext()).Increment().(types.Height), commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath),
-			false,
-		},
-		{
-			"invalid client type",
-			solomachine.NewClientState(0, &solomachine.ConsensusState{PublicKey: suite.solomachine.ConsensusState().PublicKey, Diversifier: suite.solomachine.Diversifier, Timestamp: suite.solomachine.Time}),
-			false,
-		},
-		{
-			"invalid client revision",
-			ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeightRevision1, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath),
-			false,
-		},
-		{
-			"invalid proof specs",
-			ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, nil, ibctesting.UpgradePath),
-			false,
-		},
-		{
-			"invalid trust level",
-			ibctm.NewClientState(suite.chainA.ChainID, ibctm.Fraction{Numerator: 0, Denominator: 1}, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath), false,
-		},
-		{
-			"invalid unbonding period",
-			ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+10, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath),
-			false,
-		},
-		{
-			"invalid trusting period",
-			ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, ubdPeriod+10, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath),
-			false,
-		},
-		{
-			"invalid upgrade path",
-			ibctm.NewClientState(suite.chainA.ChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, testClientHeight, commitmenttypes.GetSDKSpecs(), []string{"bad", "upgrade", "path"}),
-			false,
-		},
-	}
-
-	for _, tc := range testCases {
-		tc := tc
-
-		suite.Run(tc.name, func() {
-			err := suite.chainA.App.GetIBCKeeper().ClientKeeper.ValidateSelfClient(suite.chainA.GetContext(), tc.clientState)
-			if tc.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)
-			}
-		})
-	}
-}
-
 func (suite *KeeperTestSuite) TestGetAllGenesisClients() {
 	clientIDs := []string{
 		exported.LocalhostClientID, testClientID2, testClientID3, testClientID,
@@ -308,32 +220,6 @@ func (suite *KeeperTestSuite) TestGetAllGenesisMetadata() {
 	})
 }
 
-func (suite *KeeperTestSuite) TestGetConsensusState() {
-	suite.ctx = suite.ctx.WithBlockHeight(10)
-	cases := []struct {
-		name    string
-		height  types.Height
-		expPass bool
-	}{
-		{"zero height", types.ZeroHeight(), false},
-		{"height > latest height", types.NewHeight(0, uint64(suite.ctx.BlockHeight())+1), false},
-		{"latest height - 1", types.NewHeight(0, uint64(suite.ctx.BlockHeight())-1), true},
-		{"latest height", types.GetSelfHeight(suite.ctx), true},
-	}
-
-	for i, tc := range cases {
-		tc := tc
-		cs, err := suite.keeper.GetSelfConsensusState(suite.ctx, tc.height)
-		if tc.expPass {
-			suite.Require().NoError(err, "Case %d should have passed: %s", i, tc.name)
-			suite.Require().NotNil(cs, "Case %d should have passed: %s", i, tc.name)
-		} else {
-			suite.Require().Error(err, "Case %d should have failed: %s", i, tc.name)
-			suite.Require().Nil(cs, "Case %d should have failed: %s", i, tc.name)
-		}
-	}
-}
-
 // 2 clients in total are created on chainA. The first client is updated so it contains an initial consensus state
 // and a consensus state at the update height.
 func (suite *KeeperTestSuite) TestGetAllConsensusStates() {

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

@@ -9,11 +9,11 @@ import (
 
 // Migrator is a struct for handling in-place store migrations.
 type Migrator struct {
-	keeper Keeper
+	keeper *Keeper
 }
 
 // NewMigrator returns a new Migrator.
-func NewMigrator(keeper Keeper) Migrator {
+func NewMigrator(keeper *Keeper) Migrator {
 	return Migrator{keeper: keeper}
 }
 

modules/core/02-client/migrations/v7/genesis_test.go

@@ -33,7 +33,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() {
 	solomachine := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, ibctesting.DefaultSolomachineClientID, "testing", 1)
 	solomachineMulti := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "06-solomachine-1", "testing", 4)
 
-	clientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper)
+	clientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), *suite.chainA.App.GetIBCKeeper().ClientKeeper)
 
 	// manually generate old proto buf definitions and set in genesis
 	// NOTE: we cannot use 'ExportGenesis' for the solo machines since we are
@@ -108,7 +108,7 @@ func (suite *MigrationsV7TestSuite) TestMigrateGenesisSolomachine() {
 	// NOTE: tendermint clients are not pruned in genesis so the test should not have expired tendermint clients
 	err := v7.MigrateStore(suite.chainA.GetContext(), suite.chainA.GetSimApp().GetKey(ibcexported.StoreKey), suite.chainA.App.AppCodec(), suite.chainA.GetSimApp().IBCKeeper.ClientKeeper)
 	suite.Require().NoError(err)
-	expectedClientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), suite.chainA.App.GetIBCKeeper().ClientKeeper)
+	expectedClientGenState := ibcclient.ExportGenesis(suite.chainA.GetContext(), *suite.chainA.App.GetIBCKeeper().ClientKeeper)
 
 	cdc, ok := suite.chainA.App.AppCodec().(codec.ProtoCodecMarshaler)
 	suite.Require().True(ok)

modules/core/02-client/proposal_handler.go

@@ -15,7 +15,7 @@ import (
 //
 // Deprecated: This function is deprecated and will be removed in a future release.
 // Please use MsgRecoverClient and MsgIBCSoftwareUpgrade in favour of this legacy Handler.
-func NewClientProposalHandler(k keeper.Keeper) govtypes.Handler { //nolint:staticcheck
+func NewClientProposalHandler(k *keeper.Keeper) govtypes.Handler { //nolint:staticcheck
 	return func(ctx sdk.Context, content govtypes.Content) error {
 		switch c := content.(type) {
 		case *types.ClientUpdateProposal: