Replace CheckHeaderAndUpdateState with new ClientState functions (cos…

…mos#1208)

* refactor: replace CheckHeaderAndUpdateState with VerifyClientMessage, CheckForMisbehaviour, UpdateStateOnMisbehaviour, and UpdateState

* add changelog entry

* fix tests

CHANGELOG.md

@@ -43,6 +43,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### State Machine Breaking
 
 ### Improvements
+
+* (02-client) [\#1208](https://github.com/cosmos/ibc-go/pull/1208) Replace `CheckHeaderAndUpdateState` usage in 02-client with calls to `VerifyClientMessage`, `CheckForMisbehaviour`, `UpdateStateOnMisbehaviour` and `UpdateState`. 
 * (modules/light-clients/09-localhost) [\#1187](https://github.com/cosmos/ibc-go/pull/1187/) Removing localhost light client implementation as it is not functional. 
 * [\#1186](https://github.com/cosmos/ibc-go/pull/1186/files) Removing `GetRoot` function from ConsensusState interface in `02-client`. `GetRoot` is unused by core IBC. 
 * (modules/core/02-client) [\#1196](https://github.com/cosmos/ibc-go/pull/1196) Adding VerifyClientMessage to ClientState interface. 

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

@@ -54,7 +54,7 @@ func (k Keeper) CreateClient(
 }
 
 // UpdateClient updates the consensus state and the state root from a provided header.
-func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.ClientMessage) error {
+func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg exported.ClientMessage) error {
 	clientState, found := k.GetClientState(ctx, clientID)
 	if !found {
 		return sdkerrors.Wrapf(types.ErrClientNotFound, "cannot update client with ID %s", clientID)
@@ -66,54 +66,21 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.C
 		return sdkerrors.Wrapf(types.ErrClientNotActive, "cannot update client (%s) with status %s", clientID, status)
 	}
 
-	// Any writes made in CheckHeaderAndUpdateState are persisted on both valid updates and misbehaviour updates.
-	// Light client implementations are responsible for writing the correct metadata (if any) in either case.
-	newClientState, newConsensusState, err := clientState.CheckHeaderAndUpdateState(ctx, k.cdc, clientStore, header)
-	if err != nil {
-		return sdkerrors.Wrapf(err, "cannot update client with ID %s", clientID)
-	}
-
-	// emit the full header in events
-	var (
-		headerStr       string
-		consensusHeight exported.Height
-	)
-	if header != nil {
-		// Marshal the Header as an Any and encode the resulting bytes to hex.
-		// This prevents the event value from containing invalid UTF-8 characters
-		// which may cause data to be lost when JSON encoding/decoding.
-		headerStr = hex.EncodeToString(types.MustMarshalClientMessage(k.cdc, header))
-		// set default consensus height with header height
-		consensusHeight = header.GetHeight()
-
+	if err := clientState.VerifyClientMessage(ctx, k.cdc, clientStore, clientMsg); err != nil {
+		return err
 	}
 
-	// set new client state regardless of if update is valid update or misbehaviour
-	k.SetClientState(ctx, clientID, newClientState)
-	// If client state is not frozen after clientState CheckHeaderAndUpdateState,
-	// then update was valid. Write the update state changes, and set new consensus state.
-	// Else the update was proof of misbehaviour and we must emit appropriate misbehaviour events.
-	if status := newClientState.Status(ctx, clientStore, k.cdc); status != exported.Frozen {
-		// if update is not misbehaviour then update the consensus state
-		k.SetClientConsensusState(ctx, clientID, header.GetHeight(), newConsensusState)
-
-		k.Logger(ctx).Info("client state updated", "client-id", clientID, "height", consensusHeight.String())
+	// Marshal the ClientMessage as an Any and encode the resulting bytes to hex.
+	// This prevents the event value from containing invalid UTF-8 characters
+	// which may cause data to be lost when JSON encoding/decoding.
+	clientMsgStr := hex.EncodeToString(types.MustMarshalClientMessage(k.cdc, clientMsg))
 
-		defer func() {
-			telemetry.IncrCounterWithLabels(
-				[]string{"ibc", "client", "update"},
-				1,
-				[]metrics.Label{
-					telemetry.NewLabel(types.LabelClientType, clientState.ClientType()),
-					telemetry.NewLabel(types.LabelClientID, clientID),
-					telemetry.NewLabel(types.LabelUpdateType, "msg"),
-				},
-			)
-		}()
+	// set default consensus height with header height
+	consensusHeight := clientMsg.GetHeight()
 
-		// emitting events in the keeper emits for both begin block and handler client updates
-		EmitUpdateClientEvent(ctx, clientID, newClientState, consensusHeight, headerStr)
-	} else {
+	foundMisbehaviour := clientState.CheckForMisbehaviour(ctx, k.cdc, clientStore, clientMsg)
+	if foundMisbehaviour {
+		clientState.UpdateStateOnMisbehaviour(ctx, k.cdc, clientStore, clientMsg)
 
 		k.Logger(ctx).Info("client frozen due to misbehaviour", "client-id", clientID)
 
@@ -129,9 +96,30 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.C
 			)
 		}()
 
-		EmitSubmitMisbehaviourEventOnUpdate(ctx, clientID, newClientState, consensusHeight, headerStr)
+		EmitSubmitMisbehaviourEventOnUpdate(ctx, clientID, clientState.ClientType(), consensusHeight, clientMsgStr)
+
+		return nil
 	}
 
+	clientState.UpdateState(ctx, k.cdc, clientStore, clientMsg)
+
+	k.Logger(ctx).Info("client state updated", "client-id", clientID, "height", consensusHeight.String())
+
+	defer func() {
+		telemetry.IncrCounterWithLabels(
+			[]string{"ibc", "client", "update"},
+			1,
+			[]metrics.Label{
+				telemetry.NewLabel(types.LabelClientType, clientState.ClientType()),
+				telemetry.NewLabel(types.LabelClientID, clientID),
+				telemetry.NewLabel(types.LabelUpdateType, "msg"),
+			},
+		)
+	}()
+
+	// emitting events in the keeper emits for both begin block and handler client updates
+	EmitUpdateClientEvent(ctx, clientID, clientState.ClientType(), consensusHeight, clientMsgStr)
+
 	return nil
 }
 

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

@@ -1,17 +1,10 @@
 package keeper
 
 import (
-	"encoding/hex"
-	"fmt"
-	"strconv"
-	"strings"
-
-	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
 
-	"github.com/cosmos/ibc-go/v5/modules/core/02-client/types"
-	"github.com/cosmos/ibc-go/v5/modules/core/exported"
+	"github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
+	"github.com/cosmos/ibc-go/v3/modules/core/exported"
 )
 
 // EmitCreateClientEvent emits a create client event
@@ -31,31 +24,13 @@ func EmitCreateClientEvent(ctx sdk.Context, clientID string, clientState exporte
 }
 
 // EmitUpdateClientEvent emits an update client event
-func EmitUpdateClientEvent(ctx sdk.Context, clientID string, clientType string, consensusHeights []exported.Height, cdc codec.BinaryCodec, clientMsg exported.ClientMessage) {
-	// Marshal the ClientMessage as an Any and encode the resulting bytes to hex.
-	// This prevents the event value from containing invalid UTF-8 characters
-	// which may cause data to be lost when JSON encoding/decoding.
-	clientMsgStr := hex.EncodeToString(types.MustMarshalClientMessage(cdc, clientMsg))
-
-	var consensusHeightAttr string
-	if len(consensusHeights) != 0 {
-		consensusHeightAttr = consensusHeights[0].String()
-	}
-
-	consensusHeightsAttr := make([]string, len(consensusHeights))
-	for i, height := range consensusHeights {
-		consensusHeightsAttr[i] = height.String()
-	}
-
+func EmitUpdateClientEvent(ctx sdk.Context, clientID string, clientType string, consensusHeight exported.Height, clientMsgStr string) {
 	ctx.EventManager().EmitEvents(sdk.Events{
 		sdk.NewEvent(
 			types.EventTypeUpdateClient,
 			sdk.NewAttribute(types.AttributeKeyClientID, clientID),
 			sdk.NewAttribute(types.AttributeKeyClientType, clientType),
-			// Deprecated: AttributeKeyConsensusHeight is deprecated and will be removed in a future release.
-			// Please use AttributeKeyConsensusHeights instead.
-			sdk.NewAttribute(types.AttributeKeyConsensusHeight, consensusHeightAttr),
-			sdk.NewAttribute(types.AttributeKeyConsensusHeights, strings.Join(consensusHeightsAttr, ",")),
+			sdk.NewAttribute(types.AttributeKeyConsensusHeight, consensusHeight.String()),
 			sdk.NewAttribute(types.AttributeKeyHeader, clientMsgStr),
 		),
 		sdk.NewEvent(
@@ -82,23 +57,13 @@ func EmitUpgradeClientEvent(ctx sdk.Context, clientID string, clientState export
 }
 
 // EmitUpdateClientProposalEvent emits an update client proposal event
-func EmitUpdateClientProposalEvent(ctx sdk.Context, clientID, clientType string) {
+func EmitUpdateClientProposalEvent(ctx sdk.Context, clientID string, clientState exported.ClientState) {
 	ctx.EventManager().EmitEvent(
 		sdk.NewEvent(
 			types.EventTypeUpdateClientProposal,
 			sdk.NewAttribute(types.AttributeKeySubjectClientID, clientID),
-			sdk.NewAttribute(types.AttributeKeyClientType, clientType),
-		),
-	)
-}
-
-// EmitUpgradeClientProposalEvent emits an upgrade client proposal event
-func EmitUpgradeClientProposalEvent(ctx sdk.Context, title string, height int64) {
-	ctx.EventManager().EmitEvent(
-		sdk.NewEvent(
-			types.EventTypeUpgradeClientProposal,
-			sdk.NewAttribute(types.AttributeKeyUpgradePlanTitle, title),
-			sdk.NewAttribute(types.AttributeKeyUpgradePlanHeight, fmt.Sprintf("%d", height)),
+			sdk.NewAttribute(types.AttributeKeyClientType, clientState.ClientType()),
+			sdk.NewAttribute(types.AttributeKeyConsensusHeight, clientState.GetLatestHeight().String()),
 		),
 	)
 }
@@ -114,13 +79,15 @@ func EmitSubmitMisbehaviourEvent(ctx sdk.Context, clientID string, clientState e
 	)
 }
 
-// EmitUpgradeChainEvent emits an upgrade chain event.
-func EmitUpgradeChainEvent(ctx sdk.Context, height int64) {
-	ctx.EventManager().EmitEvents(sdk.Events{
+// EmitSubmitMisbehaviourEventOnUpdate emits a client misbehaviour event on a client update event
+func EmitSubmitMisbehaviourEventOnUpdate(ctx sdk.Context, clientID string, clientType string, consensusHeight exported.Height, headerStr string) {
+	ctx.EventManager().EmitEvent(
 		sdk.NewEvent(
-			types.EventTypeUpgradeChain,
-			sdk.NewAttribute(types.AttributeKeyUpgradePlanHeight, strconv.FormatInt(height, 10)),
-			sdk.NewAttribute(types.AttributeKeyUpgradeStore, upgradetypes.StoreKey), // which store to query proof of consensus state from
+			types.EventTypeSubmitMisbehaviour,
+			sdk.NewAttribute(types.AttributeKeyClientID, clientID),
+			sdk.NewAttribute(types.AttributeKeyClientType, clientType),
+			sdk.NewAttribute(types.AttributeKeyConsensusHeight, consensusHeight.String()),
+			sdk.NewAttribute(types.AttributeKeyHeader, headerStr),
 		),
-	})
+	)
 }

modules/core/02-client/legacy/v100/solomachine.go

@@ -95,7 +95,7 @@ func (cs ClientState) CheckForMisbehaviour(ctx sdk.Context, cdc codec.BinaryCode
 
 // UpdateStateOnMisbehaviour panics!
 func (cs *ClientState) UpdateStateOnMisbehaviour(
-	_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore,
+	_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, _ exported.ClientMessage,
 ) {
 	panic("legacy solo machine is deprecated!")
 }

modules/core/exported/client.go

@@ -56,12 +56,12 @@ type ClientState interface {
 	ExportMetadata(sdk.KVStore) []GenesisMetadata
 
 	// UpdateStateOnMisbehaviour should perform appropriate state changes on a client state given that misbehaviour has been detected and verified
-	UpdateStateOnMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore)
+	UpdateStateOnMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg ClientMessage)
 
 	// VerifyClientMessage verifies a ClientMessage. A ClientMessage could be a Header, Misbehaviour, or batch update.
 	VerifyClientMessage(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg ClientMessage) error
 
-	// UpdateState updates and stores as necessary any associated information for an IBC client, such as the ClientState and corresponding ConsensusState. 
+	// UpdateState updates and stores as necessary any associated information for an IBC client, such as the ClientState and corresponding ConsensusState.
 	// An error is returned if ClientMessage is of type Misbehaviour
 	UpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, ClientMessage) error
 

modules/light-clients/06-solomachine/types/update.go

@@ -26,7 +26,7 @@ func (cs ClientState) CheckHeaderAndUpdateState(
 
 	foundMisbehaviour := cs.CheckForMisbehaviour(ctx, cdc, clientStore, msg)
 	if foundMisbehaviour {
-		cs.UpdateStateOnMisbehaviour(ctx, cdc, clientStore)
+		cs.UpdateStateOnMisbehaviour(ctx, cdc, clientStore, msg)
 		return &cs, cs.ConsensusState, nil
 	}
 
@@ -142,7 +142,7 @@ func (cs ClientState) CheckForMisbehaviour(_ sdk.Context, _ codec.BinaryCodec, _
 
 // UpdateStateOnMisbehaviour updates state upon misbehaviour. This method should only be called on misbehaviour
 // as it does not perform any misbehaviour checks.
-func (cs ClientState) UpdateStateOnMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore) {
+func (cs ClientState) UpdateStateOnMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, _ exported.ClientMessage) {
 	cs.IsFrozen = true
 
 	clientStore.Set(host.ClientStateKey(), clienttypes.MustMarshalClientState(cdc, &cs))

modules/light-clients/06-solomachine/types/update_test.go

@@ -708,7 +708,7 @@ func (suite *SoloMachineTestSuite) TestUpdateStateOnMisbehaviour() {
 
 				tc.malleate()
 
-				clientState.UpdateStateOnMisbehaviour(suite.chainA.GetContext(), suite.chainA.Codec, suite.store)
+				clientState.UpdateStateOnMisbehaviour(suite.chainA.GetContext(), suite.chainA.Codec, suite.store, nil)
 
 				if tc.expPass {
 					clientStateBz := suite.store.Get(host.ClientStateKey())

modules/light-clients/07-tendermint/types/update.go

@@ -358,7 +358,7 @@ func (cs ClientState) CheckForMisbehaviour(ctx sdk.Context, cdc codec.BinaryCode
 
 // UpdateStateOnMisbehaviour updates state upon misbehaviour, freezing the ClientState. This method should only be called when misbehaviour is detected
 // as it does not perform any misbehaviour checks.
-func (cs ClientState) UpdateStateOnMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore) {
+func (cs ClientState) UpdateStateOnMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, _ exported.ClientMessage) {
 	cs.FrozenHeight = FrozenHeight
 
 	clientStore.Set(host.ClientStateKey(), clienttypes.MustMarshalClientState(cdc, &cs))

modules/light-clients/07-tendermint/types/update_test.go

@@ -792,11 +792,7 @@ func (suite *TendermintTestSuite) TestUpdateStateOnMisbehaviour() {
 			clientState := path.EndpointA.GetClientState()
 			clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID)
 
-			// TODO: remove casting when 'UpdateState' is an interface function.
-			tmClientState, ok := clientState.(*types.ClientState)
-			suite.Require().True(ok)
-
-			tmClientState.UpdateStateOnMisbehaviour(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore)
+			clientState.UpdateStateOnMisbehaviour(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, nil)
 
 			if tc.expPass {
 				clientStateBz := clientStore.Get(host.ClientStateKey())