ICS-2 Implement Misbehavior (#5321)

* ibc client evidence route

* split evidence from misbehaviour

* clean up client events

* test misbehaviour and evidence

* remove comments

* remove frozen comments from demo

* Update x/ibc/02-client/types/tendermint/evidence_test.go

Co-Authored-By: Aditya <[email protected]>

* change evidence to detect malicious chain

* remove unnecessary sort

* fix evidence and persist committers to check misbehaviour

* minor fixes and remove incorrect tests

* add evidence tests

* remove debug statements

* cleanup evidence test

* start misbehaviour tests

* fix nondeterministic bug

* add same height and next height checks in misbehaviour

* fix bugs

* apply fede review suggestions

* finish code review changes

* fix GetCommitter and write keeper-level misbehaviour tests

* remove incorrect special case checking

* save

* final fixes

* save

* fix conflict

* fix conflicts and add back submit misbehaviour msg

* Apply suggestions from code review

Co-Authored-By: Federico Kunze <[email protected]>

* save

* add godocs and fix test

* fix test panics in other modules

* Update x/ibc/02-client/keeper/client.go

Co-Authored-By: Bot from GolangCI <[email protected]>

* add back aliases

simapp/app.go

@@ -24,6 +24,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/genutil"
 	"github.com/cosmos/cosmos-sdk/x/gov"
 	"github.com/cosmos/cosmos-sdk/x/ibc"
+	ibcclient "github.com/cosmos/cosmos-sdk/x/ibc/02-client"
 	ibctransfer "github.com/cosmos/cosmos-sdk/x/ibc/20-transfer"
 	"github.com/cosmos/cosmos-sdk/x/mint"
 	"github.com/cosmos/cosmos-sdk/x/params"
@@ -201,8 +202,9 @@ func NewSimApp(
 		app.cdc, keys[evidence.StoreKey], app.subspaces[evidence.ModuleName], evidence.DefaultCodespace,
 		&app.StakingKeeper, app.SlashingKeeper,
 	)
-	evidenceRouter := evidence.NewRouter()
-	// TODO: Register evidence routes.
+	evidenceRouter := evidence.NewRouter().
+		AddRoute(ibcclient.RouterKey, ibcclient.HandlerClientMisbehaviour(app.IBCKeeper.ClientKeeper))
+
 	evidenceKeeper.SetRouter(evidenceRouter)
 	app.EvidenceKeeper = *evidenceKeeper
 

x/evidence/exported/evidence.go

@@ -15,11 +15,17 @@ type Evidence interface {
 	Hash() cmn.HexBytes
 	ValidateBasic() error
 
-	// The consensus address of the malicious validator at time of infraction
-	GetConsensusAddress() sdk.ConsAddress
-
 	// Height at which the infraction occurred
 	GetHeight() int64
+}
+
+// ValidatorEvidence extends Evidence interface to define contract
+// for evidence against malicious validators
+type ValidatorEvidence interface {
+	Evidence
+
+	// The consensus address of the malicious validator at time of infraction
+	GetConsensusAddress() sdk.ConsAddress
 
 	// The total power of the malicious validator at time of infraction
 	GetValidatorPower() int64

x/ibc/02-client/alias.go

@@ -61,17 +61,16 @@ var (
 	KeyRoot                      = types.KeyRoot
 	NewMsgCreateClient           = types.NewMsgCreateClient
 	NewMsgUpdateClient           = types.NewMsgUpdateClient
-	NewMsgSubmitMisbehaviour     = types.NewMsgSubmitMisbehaviour
+	NewMsgSubmitMibehaviour      = types.NewMsgSubmitMisbehaviour
 	NewQueryClientStateParams    = types.NewQueryClientStateParams
 	NewQueryCommitmentRootParams = types.NewQueryCommitmentRootParams
 	NewClientState               = types.NewClientState
 
 	// variable aliases
-	SubModuleCdc                = types.SubModuleCdc
-	EventTypeCreateClient       = types.EventTypeCreateClient
-	EventTypeUpdateClient       = types.EventTypeUpdateClient
-	EventTypeSubmitMisbehaviour = types.EventTypeSubmitMisbehaviour
-	AttributeValueCategory      = types.AttributeValueCategory
+	SubModuleCdc           = types.SubModuleCdc
+	EventTypeCreateClient  = types.EventTypeCreateClient
+	EventTypeUpdateClient  = types.EventTypeUpdateClient
+	AttributeValueCategory = types.AttributeValueCategory
 )
 
 type (

x/ibc/02-client/client/cli/tx.go

@@ -16,6 +16,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/version"
 	"github.com/cosmos/cosmos-sdk/x/auth"
 	"github.com/cosmos/cosmos-sdk/x/auth/client/utils"
+	evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported"
 	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported"
 	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types"
 )
@@ -133,7 +134,7 @@ $ %s tx ibc client misbehaviour [client-id] [path/to/evidence.json] --from node0
 
 			clientID := args[0]
 
-			var evidence exported.Evidence
+			var evidence evidenceexported.Evidence
 			if err := cdc.UnmarshalJSON([]byte(args[1]), &evidence); err != nil {
 				fmt.Fprintf(os.Stderr, "failed to unmarshall input into struct, checking for file...")
 

x/ibc/02-client/client/rest/rest.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/client/context"
 	"github.com/cosmos/cosmos-sdk/types/rest"
+	evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported"
 	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported"
 )
 
@@ -35,5 +36,5 @@ type UpdateClientReq struct {
 // SubmitMisbehaviourReq defines the properties of a submit misbehaviour request's body.
 type SubmitMisbehaviourReq struct {
 	BaseReq  rest.BaseReq      `json:"base_req" yaml:"base_req"`
-	Evidence exported.Evidence `json:"evidence" yaml:"evidence"`
+	Evidence evidenceexported.Evidence `json:"evidence" yaml:"evidence"`
 }

x/ibc/02-client/exported/exported.go

@@ -4,10 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 
-	sdk "github.com/cosmos/cosmos-sdk/types"
-
-	cmn "github.com/tendermint/tendermint/libs/common"
-
+	evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported"
 	commitment "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment"
 )
 
@@ -20,41 +17,32 @@ type ConsensusState interface {
 	// which is used for key-value pair verification.
 	GetRoot() commitment.RootI
 
+	// GetCommitter returns the committer that committed the consensus state
+	GetCommitter() Committer
+
 	// CheckValidityAndUpdateState returns the updated consensus state
 	// only if the header is a descendent of this consensus state.
 	CheckValidityAndUpdateState(Header) (ConsensusState, error)
 }
 
-// Evidence from ADR 009: Evidence Module
-// TODO: use evidence module interface once merged
-type Evidence interface {
-	Route() string
-	Type() string
-	String() string
-	Hash() cmn.HexBytes
-	ValidateBasic() sdk.Error
-
-	// The consensus address of the malicious validator at time of infraction
-	GetConsensusAddress() sdk.ConsAddress
-
-	// Height at which the infraction occurred
-	GetHeight() int64
-
-	// The total power of the malicious validator at time of infraction
-	GetValidatorPower() int64
-
-	// The total validator set power at time of infraction
-	GetTotalPower() int64
-}
-
 // Misbehaviour defines a specific consensus kind and an evidence
 type Misbehaviour interface {
 	ClientType() ClientType
-	Evidence() Evidence
+	GetEvidence() evidenceexported.Evidence
 }
 
 // Header is the consensus state update information
 type Header interface {
+	ClientType() ClientType
+	GetCommitter() Committer
+	GetHeight() uint64
+}
+
+// Committer defines the type that is responsible for
+// updating the consensusState at a given height
+//
+// In Tendermint, this is the ValidatorSet at the given height
+type Committer interface {
 	ClientType() ClientType
 	GetHeight() uint64
 }

x/ibc/02-client/handler.go

@@ -1,8 +1,13 @@
 package client
 
 import (
+	"fmt"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	exported "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported"
+	"github.com/cosmos/cosmos-sdk/x/evidence"
+	evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported"
+	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported"
+	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/tendermint"
 )
 
 // HandleMsgCreateClient defines the sdk.Handler for MsgCreateClient
@@ -12,7 +17,6 @@ func HandleMsgCreateClient(ctx sdk.Context, k Keeper, msg MsgCreateClient) sdk.R
 		return sdk.ResultFromError(ErrInvalidClientType(DefaultCodespace, err.Error()))
 	}
 
-	// TODO: should we create an event with the new client state id ?
 	_, err = k.CreateClient(ctx, msg.ClientID, clientType, msg.ConsensusState)
 	if err != nil {
 		return sdk.ResultFromError(err)
@@ -55,24 +59,17 @@ func HandleMsgUpdateClient(ctx sdk.Context, k Keeper, msg MsgUpdateClient) sdk.R
 	return sdk.Result{Events: ctx.EventManager().Events()}
 }
 
-// HandleMsgSubmitMisbehaviour defines the sdk.Handler for MsgSubmitMisbehaviour
-func HandleMsgSubmitMisbehaviour(ctx sdk.Context, k Keeper, msg MsgSubmitMisbehaviour) sdk.Result {
-	err := k.CheckMisbehaviourAndUpdateState(ctx, msg.ClientID, msg.Evidence)
-	if err != nil {
-		return sdk.ResultFromError(err)
-	}
+// HandlerClientMisbehaviour defines the Evidence module handler for submitting a
+// light client misbehaviour.
+func HandlerClientMisbehaviour(k Keeper) evidence.Handler {
+	return func(ctx sdk.Context, evidence evidenceexported.Evidence) error {
+		switch e := evidence.(type) {
+		case tendermint.Misbehaviour:
+			return k.CheckMisbehaviourAndUpdateState(ctx, evidence)
 
-	ctx.EventManager().EmitEvents(sdk.Events{
-		sdk.NewEvent(
-			EventTypeSubmitMisbehaviour,
-			sdk.NewAttribute(AttributeKeyClientID, msg.ClientID),
-		),
-		sdk.NewEvent(
-			sdk.EventTypeMessage,
-			sdk.NewAttribute(sdk.AttributeKeyModule, AttributeValueCategory),
-			sdk.NewAttribute(sdk.AttributeKeySender, msg.Signer.String()),
-		),
-	})
-
-	return sdk.Result{Events: ctx.EventManager().Events()}
+		default:
+			errMsg := fmt.Sprintf("unrecognized IBC client evidence type: %T", e)
+			return sdk.ErrUnknownRequest(errMsg)
+		}
+	}
 }

x/ibc/02-client/keeper/client.go

@@ -5,9 +5,11 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported"
 	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported"
 	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types"
 	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors"
+	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/tendermint"
 )
 
 // CreateClient creates a new client state and populates it with a given consensus
@@ -27,6 +29,7 @@ func (k Keeper) CreateClient(
 	}
 
 	clientState := k.initialize(ctx, clientID, consensusState)
+	k.SetCommitter(ctx, clientID, consensusState.GetHeight(), consensusState.GetCommitter())
 	k.SetVerifiedRoot(ctx, clientID, consensusState.GetHeight(), consensusState.GetRoot())
 	k.SetClientState(ctx, clientState)
 	k.SetClientType(ctx, clientID, clientType)
@@ -66,30 +69,55 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H
 	}
 
 	k.SetConsensusState(ctx, clientID, consensusState)
+	k.SetCommitter(ctx, clientID, consensusState.GetHeight(), consensusState.GetCommitter())
 	k.SetVerifiedRoot(ctx, clientID, consensusState.GetHeight(), consensusState.GetRoot())
 	k.Logger(ctx).Info(fmt.Sprintf("client %s updated to height %d", clientID, consensusState.GetHeight()))
 	return nil
 }
 
 // CheckMisbehaviourAndUpdateState checks for client misbehaviour and freezes the
 // client if so.
-func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, clientID string, evidence exported.Evidence) error {
-	clientState, found := k.GetClientState(ctx, clientID)
+//
+// NOTE: In the first implementation, only Tendermint misbehaviour evidence is
+// supported.
+func (k Keeper) CheckMisbehaviourAndUpdateState(ctx sdk.Context, evidence evidenceexported.Evidence) error {
+	misbehaviour, ok := evidence.(tendermint.Misbehaviour)
+	if !ok {
+		return errors.ErrInvalidClientType(k.codespace, "consensus type is not Tendermint")
+	}
+
+	clientState, found := k.GetClientState(ctx, misbehaviour.ClientID)
 	if !found {
-		sdk.ResultFromError(errors.ErrClientNotFound(k.codespace, clientID))
+		return errors.ErrClientNotFound(k.codespace, misbehaviour.ClientID)
 	}
 
-	err := k.checkMisbehaviour(ctx, evidence)
-	if err != nil {
-		return err
+	committer, found := k.GetCommitter(ctx, misbehaviour.ClientID, uint64(misbehaviour.GetHeight()))
+	if !found {
+		return errors.ErrCommitterNotFound(k.codespace, fmt.Sprintf("committer not found for height %d", misbehaviour.GetHeight()))
+	}
+	tmCommitter, ok := committer.(tendermint.Committer)
+	if !ok {
+		return errors.ErrInvalidCommitter(k.codespace, "committer type is not Tendermint")
+	}
+
+	if err := tendermint.CheckMisbehaviour(tmCommitter, misbehaviour); err != nil {
+		return errors.ErrInvalidEvidence(k.codespace, err.Error())
 	}
 
-	clientState, err = k.freeze(ctx, clientState)
+	clientState, err := k.freeze(ctx, clientState)
 	if err != nil {
 		return err
 	}
 
 	k.SetClientState(ctx, clientState)
-	k.Logger(ctx).Info(fmt.Sprintf("client %s frozen due to misbehaviour", clientID))
+	k.Logger(ctx).Info(fmt.Sprintf("client %s frozen due to misbehaviour", misbehaviour.ClientID))
+
+	ctx.EventManager().EmitEvent(
+		sdk.NewEvent(
+			types.EventTypeSubmitMisbehaviour,
+			sdk.NewAttribute(types.AttributeKeyClientID, misbehaviour.ClientID),
+		),
+	)
+
 	return nil
 }