diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go index a0ab6348dcf..a1646da9c6b 100644 --- a/modules/core/02-client/keeper/client.go +++ b/modules/core/02-client/keeper/client.go @@ -1,8 +1,6 @@ package keeper import ( - "encoding/hex" - "github.com/armon/go-metrics" "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" @@ -27,7 +25,6 @@ func (k Keeper) CreateClient( clientID := k.GenerateClientIdentifier(ctx, clientState.ClientType()) - k.SetClientState(ctx, clientID, clientState) k.Logger(ctx).Info("client created at height", "client-id", clientID, "height", clientState.GetLatestHeight().String()) // verifies initial consensus state against client state and initializes client store with any client-specific metadata @@ -36,17 +33,16 @@ func (k Keeper) CreateClient( return "", err } + k.SetClientState(ctx, clientID, clientState) k.SetClientConsensusState(ctx, clientID, clientState.GetLatestHeight(), consensusState) k.Logger(ctx).Info("client created at height", "client-id", clientID, "height", clientState.GetLatestHeight().String()) - defer func() { - telemetry.IncrCounterWithLabels( - []string{"ibc", "client", "create"}, - 1, - []metrics.Label{telemetry.NewLabel(types.LabelClientType, clientState.ClientType())}, - ) - }() + defer telemetry.IncrCounterWithLabels( + []string{"ibc", "client", "create"}, + 1, + []metrics.Label{telemetry.NewLabel(types.LabelClientType, clientState.ClientType())}, + ) EmitCreateClientEvent(ctx, clientID, clientState) @@ -76,17 +72,15 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg exporte k.Logger(ctx).Info("client frozen due to misbehaviour", "client-id", clientID) - defer func() { - telemetry.IncrCounterWithLabels( - []string{"ibc", "client", "misbehaviour"}, - 1, - []metrics.Label{ - telemetry.NewLabel(types.LabelClientType, clientState.ClientType()), - telemetry.NewLabel(types.LabelClientID, clientID), - telemetry.NewLabel(types.LabelMsgType, "update"), - }, - ) - }() + defer telemetry.IncrCounterWithLabels( + []string{"ibc", "client", "misbehaviour"}, + 1, + []metrics.Label{ + telemetry.NewLabel(types.LabelClientType, clientState.ClientType()), + telemetry.NewLabel(types.LabelClientID, clientID), + telemetry.NewLabel(types.LabelMsgType, "update"), + }, + ) EmitSubmitMisbehaviourEvent(ctx, clientID, clientState) @@ -97,25 +91,18 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg exporte k.Logger(ctx).Info("client state updated", "client-id", clientID, "heights", consensusHeights) - 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"), - }, - ) - }() - - // 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 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(), consensusHeights, clientMsgStr) + EmitUpdateClientEvent(ctx, clientID, clientState.ClientType(), consensusHeights, k.cdc, clientMsg) return nil } @@ -143,16 +130,14 @@ func (k Keeper) UpgradeClient(ctx sdk.Context, clientID string, upgradedClient e k.Logger(ctx).Info("client state upgraded", "client-id", clientID, "height", upgradedClient.GetLatestHeight().String()) - defer func() { - telemetry.IncrCounterWithLabels( - []string{"ibc", "client", "upgrade"}, - 1, - []metrics.Label{ - telemetry.NewLabel(types.LabelClientType, upgradedClient.ClientType()), - telemetry.NewLabel(types.LabelClientID, clientID), - }, - ) - }() + defer telemetry.IncrCounterWithLabels( + []string{"ibc", "client", "upgrade"}, + 1, + []metrics.Label{ + telemetry.NewLabel(types.LabelClientType, upgradedClient.ClientType()), + telemetry.NewLabel(types.LabelClientID, clientID), + }, + ) EmitUpgradeClientEvent(ctx, clientID, upgradedClient) diff --git a/modules/core/02-client/keeper/events.go b/modules/core/02-client/keeper/events.go index d49d41cb1cb..ec15ca41f4e 100644 --- a/modules/core/02-client/keeper/events.go +++ b/modules/core/02-client/keeper/events.go @@ -1,8 +1,10 @@ package keeper import ( + "encoding/hex" "strings" + "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" @@ -26,7 +28,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, clientMsgStr string) { +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() diff --git a/modules/light-clients/07-tendermint/types/update.go b/modules/light-clients/07-tendermint/types/update.go index 827d598d0bd..31d16799118 100644 --- a/modules/light-clients/07-tendermint/types/update.go +++ b/modules/light-clients/07-tendermint/types/update.go @@ -17,26 +17,6 @@ import ( "github.com/cosmos/ibc-go/v3/modules/core/exported" ) -// checkTrustedHeader checks that consensus state matches trusted fields of Header -func checkTrustedHeader(header *Header, consState *ConsensusState) error { - tmTrustedValidators, err := tmtypes.ValidatorSetFromProto(header.TrustedValidators) - if err != nil { - return sdkerrors.Wrap(err, "trusted validator set in not tendermint validator set type") - } - - // assert that trustedVals is NextValidators of last trusted header - // to do this, we check that trustedVals.Hash() == consState.NextValidatorsHash - tvalHash := tmTrustedValidators.Hash() - if !bytes.Equal(consState.NextValidatorsHash, tvalHash) { - return sdkerrors.Wrapf( - ErrInvalidValidatorSet, - "trusted validators %s, does not hash to latest trusted validators. Expected: %X, got: %X", - header.TrustedValidators, consState.NextValidatorsHash, tvalHash, - ) - } - return nil -} - // VerifyClientMessage checks if the clientMessage is of type Header or Misbehaviour and verifies the message func (cs *ClientState) VerifyClientMessage( ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, @@ -190,15 +170,13 @@ func (cs ClientState) pruneOldestConsensusState(ctx sdk.Context, cdc codec.Binar // so that we can delete consensus state and all associated metadata. var ( pruneHeight exported.Height - pruneError error ) pruneCb := func(height exported.Height) bool { consState, found := GetConsensusState(clientStore, cdc, height) // this error should never occur if !found { - pruneError = sdkerrors.Wrapf(clienttypes.ErrConsensusStateNotFound, "failed to retrieve consensus state at height: %s", height) - return true + panic(sdkerrors.Wrapf(clienttypes.ErrConsensusStateNotFound, "failed to retrieve consensus state at height: %s", height)) } if cs.IsExpired(consState.Timestamp, ctx.BlockTime()) { @@ -209,9 +187,6 @@ func (cs ClientState) pruneOldestConsensusState(ctx sdk.Context, cdc codec.Binar } IterateConsensusStateAscending(clientStore, pruneCb) - if pruneError != nil { - panic(pruneError) - } // if pruneHeight is set, delete consensus state and metadata if pruneHeight != nil { @@ -230,11 +205,11 @@ func (cs ClientState) CheckForMisbehaviour(ctx sdk.Context, cdc codec.BinaryCode // Check if the Client store already has a consensus state for the header's height // If the consensus state exists, and it matches the header then we return early // since header has already been submitted in a previous UpdateClient. - prevConsState, _ := GetConsensusState(clientStore, cdc, tmHeader.GetHeight()) - if prevConsState != nil { + existingConsState, _ := GetConsensusState(clientStore, cdc, tmHeader.GetHeight()) + if existingConsState != nil { // This header has already been submitted and the necessary state is already stored // in client store, thus we can return early without further validation. - if reflect.DeepEqual(prevConsState, tmHeader.ConsensusState()) { + if reflect.DeepEqual(existingConsState, tmHeader.ConsensusState()) { return false } @@ -272,3 +247,23 @@ func (cs ClientState) UpdateStateOnMisbehaviour(ctx sdk.Context, cdc codec.Binar clientStore.Set(host.ClientStateKey(), clienttypes.MustMarshalClientState(cdc, &cs)) } + +// checkTrustedHeader checks that consensus state matches trusted fields of Header +func checkTrustedHeader(header *Header, consState *ConsensusState) error { + tmTrustedValidators, err := tmtypes.ValidatorSetFromProto(header.TrustedValidators) + if err != nil { + return sdkerrors.Wrap(err, "trusted validator set in not tendermint validator set type") + } + + // assert that trustedVals is NextValidators of last trusted header + // to do this, we check that trustedVals.Hash() == consState.NextValidatorsHash + tvalHash := tmTrustedValidators.Hash() + if !bytes.Equal(consState.NextValidatorsHash, tvalHash) { + return sdkerrors.Wrapf( + ErrInvalidValidatorSet, + "trusted validators %s, does not hash to latest trusted validators. Expected: %X, got: %X", + header.TrustedValidators, consState.NextValidatorsHash, tvalHash, + ) + } + return nil +}