diff --git a/CHANGELOG.md b/CHANGELOG.md index 68e425a272d..b7beb37f789 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go index fa68f896665..eb1fc87e354 100644 --- a/modules/core/02-client/keeper/client.go +++ b/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 } diff --git a/modules/core/02-client/keeper/events.go b/modules/core/02-client/keeper/events.go index ff8ae1c3acd..4e2f38941a0 100644 --- a/modules/core/02-client/keeper/events.go +++ b/modules/core/02-client/keeper/events.go @@ -24,14 +24,14 @@ func EmitCreateClientEvent(ctx sdk.Context, clientID string, clientState exporte } // EmitUpdateClientEvent emits an update client event -func EmitUpdateClientEvent(ctx sdk.Context, clientID string, clientState exported.ClientState, consensusHeight exported.Height, headerStr 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, clientState.ClientType()), + sdk.NewAttribute(types.AttributeKeyClientType, clientType), sdk.NewAttribute(types.AttributeKeyConsensusHeight, consensusHeight.String()), - sdk.NewAttribute(types.AttributeKeyHeader, headerStr), + sdk.NewAttribute(types.AttributeKeyHeader, clientMsgStr), ), sdk.NewEvent( sdk.EventTypeMessage, @@ -80,12 +80,12 @@ func EmitSubmitMisbehaviourEvent(ctx sdk.Context, clientID string, clientState e } // EmitSubmitMisbehaviourEventOnUpdate emits a client misbehaviour event on a client update event -func EmitSubmitMisbehaviourEventOnUpdate(ctx sdk.Context, clientID string, clientState exported.ClientState, consensusHeight exported.Height, headerStr string) { +func EmitSubmitMisbehaviourEventOnUpdate(ctx sdk.Context, clientID string, clientType string, consensusHeight exported.Height, headerStr string) { ctx.EventManager().EmitEvent( sdk.NewEvent( types.EventTypeSubmitMisbehaviour, sdk.NewAttribute(types.AttributeKeyClientID, clientID), - sdk.NewAttribute(types.AttributeKeyClientType, clientState.ClientType()), + sdk.NewAttribute(types.AttributeKeyClientType, clientType), sdk.NewAttribute(types.AttributeKeyConsensusHeight, consensusHeight.String()), sdk.NewAttribute(types.AttributeKeyHeader, headerStr), ), diff --git a/modules/core/02-client/legacy/v100/solomachine.go b/modules/core/02-client/legacy/v100/solomachine.go index 2ef2cc19eac..19e5459206c 100644 --- a/modules/core/02-client/legacy/v100/solomachine.go +++ b/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!") } diff --git a/modules/core/exported/client.go b/modules/core/exported/client.go index 643c254cbeb..5fa9b8b4ca2 100644 --- a/modules/core/exported/client.go +++ b/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 diff --git a/modules/light-clients/06-solomachine/types/update.go b/modules/light-clients/06-solomachine/types/update.go index 2bf3ea64f35..49dc6ebddfb 100644 --- a/modules/light-clients/06-solomachine/types/update.go +++ b/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)) diff --git a/modules/light-clients/06-solomachine/types/update_test.go b/modules/light-clients/06-solomachine/types/update_test.go index 0078c4718df..e4d33e2e7f6 100644 --- a/modules/light-clients/06-solomachine/types/update_test.go +++ b/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()) diff --git a/modules/light-clients/07-tendermint/types/update.go b/modules/light-clients/07-tendermint/types/update.go index 5b854730992..6fe7c6c5910 100644 --- a/modules/light-clients/07-tendermint/types/update.go +++ b/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)) diff --git a/modules/light-clients/07-tendermint/types/update_test.go b/modules/light-clients/07-tendermint/types/update_test.go index 788ccdb2bad..b903319a0e7 100644 --- a/modules/light-clients/07-tendermint/types/update_test.go +++ b/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())