Time Monotonicity Enforcement

CHANGELOG.md

@@ -51,13 +51,17 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### State Machine Breaking
 
 * (modules/light-clients/07-tendermint) [\#99](https://github.com/cosmos/ibc-go/pull/99) Enforce maximum chain-id length for tendermint client. 
+* (modules/light-clients/07-tendermint) [\#141](https://github.com/cosmos/ibc-go/pull/141) Allow a new form of misbehaviour that proves counterparty chain breaks time monotonicity, automatically enforce monotonicity in UpdateClient and freeze client if monotonicity is broken.
+* (modules/light-clients/07-tendermint) [\#141](https://github.com/cosmos/ibc-go/pull/141) Freeze the client if there's a conflicting header submitted for an existing consensus state.
 * (modules/core/02-client) [\#8405](https://github.com/cosmos/cosmos-sdk/pull/8405) Refactor IBC client update governance proposals to use a substitute client to update a frozen or expired client.
 * (modules/core/02-client) [\#8673](https://github.com/cosmos/cosmos-sdk/pull/8673) IBC upgrade logic moved to 02-client and an IBC UpgradeProposal is added.
 
 ### Improvements
 
 * (modules/core/04-channel) [\#7949](https://github.com/cosmos/cosmos-sdk/issues/7949) Standardized channel `Acknowledgement` moved to its own file. Codec registration redundancy removed.
 * (modules/light-clients/07-tendermint) [\#125](https://github.com/cosmos/ibc-go/pull/125) Implement efficient iteration of consensus states and pruning of earliest expired consensus state on UpdateClient.
+* (modules/light-clients/07-tendermint) [\#141](https://github.com/cosmos/ibc-go/pull/141) Return early in case there's a duplicate update call to save Gas.
+
 
 ## IBC in the Cosmos SDK Repository
 

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

@@ -61,63 +61,97 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H
 		return sdkerrors.Wrapf(types.ErrClientNotFound, "cannot update client with ID %s", clientID)
 	}
 
-	// prevent update if the client is frozen before or at header height
-	if clientState.IsFrozen() && clientState.GetFrozenHeight().LTE(header.GetHeight()) {
+	// prevent update if the client is frozen
+	if clientState.IsFrozen() {
 		return sdkerrors.Wrapf(types.ErrClientFrozen, "cannot update client with ID %s", clientID)
 	}
 
-	clientState, consensusState, err := clientState.CheckHeaderAndUpdateState(ctx, k.cdc, k.ClientStore(ctx, clientID), header)
+	eventType := types.EventTypeUpdateClient
+
+	// Cache the context to ensure that any writes in CheckHeaderAndUpdateState are only written if
+	// the update is successful and the update is not evidence of misbehaviour.
+	cacheCtx, writeFn := ctx.CacheContext()
+	newClientState, newConsensusState, err := clientState.CheckHeaderAndUpdateState(cacheCtx, k.cdc, k.ClientStore(ctx, clientID), header)
 	if err != nil {
 		return sdkerrors.Wrapf(err, "cannot update client with ID %s", clientID)
 	}
 
-	k.SetClientState(ctx, clientID, clientState)
-
-	var consensusHeight exported.Height
-
-	// we don't set consensus state for localhost client
-	if header != nil && clientID != exported.Localhost {
-		k.SetClientConsensusState(ctx, clientID, header.GetHeight(), consensusState)
-		consensusHeight = header.GetHeight()
-	} else {
-		consensusHeight = types.GetSelfHeight(ctx)
-	}
-
-	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("client-type", clientState.ClientType()),
-				telemetry.NewLabel("client-id", clientID),
-				telemetry.NewLabel("update-type", "msg"),
-			},
-		)
-	}()
-
 	// emit the full header in events
-	var headerStr string
+	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.MustMarshalHeader(k.cdc, header))
+		// set default consensus height with header height
+		consensusHeight = header.GetHeight()
 
 	}
 
+	// 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 !newClientState.IsFrozen() {
+		// write any cached state changes from CheckHeaderAndUpdateState
+		// to store metadata in client store for new consensus state.
+		writeFn()
+
+		// if update is not misbehaviour then update the consensus state
+		// we don't set consensus state for localhost client
+		if header != nil && clientID != exported.Localhost {
+			k.SetClientConsensusState(ctx, clientID, header.GetHeight(), newConsensusState)
+		} else {
+			consensusHeight = types.GetSelfHeight(ctx)
+		}
+
+		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("client-type", clientState.ClientType()),
+					telemetry.NewLabel("client-id", clientID),
+					telemetry.NewLabel("update-type", "msg"),
+				},
+			)
+		}()
+
+	} else {
+		// set eventType to SubmitMisbehaviour
+		eventType = types.EventTypeSubmitMisbehaviour
+
+		k.Logger(ctx).Info("client frozen due to misbehaviour", "client-id", clientID, "height", header.GetHeight().String())
+
+		defer func() {
+			telemetry.IncrCounterWithLabels(
+				[]string{"ibc", "client", "misbehaviour"},
+				1,
+				[]metrics.Label{
+					telemetry.NewLabel("client-type", clientState.ClientType()),
+					telemetry.NewLabel("client-id", clientID),
+					telemetry.NewLabel("msg-type", "update"),
+				},
+			)
+		}()
+	}
+
 	// emitting events in the keeper emits for both begin block and handler client updates
 	ctx.EventManager().EmitEvent(
 		sdk.NewEvent(
-			types.EventTypeUpdateClient,
+			eventType,
 			sdk.NewAttribute(types.AttributeKeyClientID, clientID),
 			sdk.NewAttribute(types.AttributeKeyClientType, clientState.ClientType()),
 			sdk.NewAttribute(types.AttributeKeyConsensusHeight, consensusHeight.String()),
 			sdk.NewAttribute(types.AttributeKeyHeader, headerStr),
 		),
 	)
-
 	return nil
 }
 
@@ -178,8 +212,12 @@ func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, misbehaviour ex
 		return sdkerrors.Wrapf(types.ErrClientNotFound, "cannot check misbehaviour for client with ID %s", misbehaviour.GetClientID())
 	}
 
-	if clientState.IsFrozen() && clientState.GetFrozenHeight().LTE(misbehaviour.GetHeight()) {
-		return sdkerrors.Wrapf(types.ErrInvalidMisbehaviour, "client is already frozen at height ≤ misbehaviour height (%s ≤ %s)", clientState.GetFrozenHeight(), misbehaviour.GetHeight())
+	if clientState.IsFrozen() {
+		return sdkerrors.Wrap(types.ErrInvalidMisbehaviour, "client is already frozen")
+	}
+
+	if err := misbehaviour.ValidateBasic(); err != nil {
+		return err
 	}
 
 	clientState, err := clientState.CheckMisbehaviourAndUpdateState(ctx, k.cdc, k.ClientStore(ctx, misbehaviour.GetClientID()), misbehaviour)