From 4bc70d627db6e62d995bd6d2b8221435a2dbcf63 Mon Sep 17 00:00:00 2001 From: Aditya Date: Thu, 5 Dec 2019 01:07:52 -0800 Subject: [PATCH] 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 * 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 <31522760+fedekunze@users.noreply.github.com> * 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 <42910462+golangcibot@users.noreply.github.com> * add back aliases --- simapp/app.go | 6 +- x/evidence/exported/evidence.go | 12 +- x/ibc/02-client/alias.go | 11 +- x/ibc/02-client/client/cli/tx.go | 3 +- x/ibc/02-client/client/rest/rest.go | 3 +- x/ibc/02-client/exported/exported.go | 42 ++-- x/ibc/02-client/handler.go | 39 ++-- x/ibc/02-client/keeper/client.go | 44 +++- x/ibc/02-client/keeper/client_test.go | 111 +++++++++- x/ibc/02-client/keeper/keeper.go | 74 ++++--- x/ibc/02-client/keeper/keeper_test.go | 51 ++++- x/ibc/02-client/types/codec.go | 3 +- x/ibc/02-client/types/errors/errors.go | 22 +- x/ibc/02-client/types/events.go | 6 +- x/ibc/02-client/types/keys.go | 22 +- x/ibc/02-client/types/msgs.go | 16 +- x/ibc/02-client/types/msgs_test.go | 55 +++-- x/ibc/02-client/types/tendermint/codec.go | 3 + x/ibc/02-client/types/tendermint/committer.go | 26 +++ .../types/tendermint/consensus_state.go | 21 +- .../types/tendermint/consensus_state_test.go | 13 +- x/ibc/02-client/types/tendermint/doc.go | 4 +- x/ibc/02-client/types/tendermint/evidence.go | 106 +++++++++ .../types/tendermint/evidence_test.go | 135 ++++++++++++ x/ibc/02-client/types/tendermint/header.go | 26 +++ .../types/tendermint/misbehaviour.go | 111 ++++------ .../types/tendermint/misbehaviour_test.go | 202 +++++++++++++----- .../types/tendermint/tendermint_test.go | 3 +- .../02-client/types/tendermint/test_utils.go | 37 +--- x/ibc/03-connection/keeper/handshake_test.go | 8 +- x/ibc/03-connection/keeper/keeper_test.go | 13 +- x/ibc/04-channel/keeper/handshake_test.go | 10 +- x/ibc/04-channel/keeper/keeper_test.go | 13 +- x/ibc/20-transfer/handler_test.go | 21 +- x/ibc/20-transfer/keeper/keeper_test.go | 13 +- x/ibc/20-transfer/keeper/relay_test.go | 8 +- x/ibc/handler.go | 3 - 37 files changed, 954 insertions(+), 342 deletions(-) create mode 100644 x/ibc/02-client/types/tendermint/committer.go create mode 100644 x/ibc/02-client/types/tendermint/evidence.go create mode 100644 x/ibc/02-client/types/tendermint/evidence_test.go diff --git a/simapp/app.go b/simapp/app.go index 81d7fd4b6e21..4a46d34e07dd 100644 --- a/simapp/app.go +++ b/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" @@ -208,8 +209,9 @@ func NewSimApp( evidenceKeeper := evidence.NewKeeper( app.cdc, keys[evidence.StoreKey], app.subspaces[evidence.ModuleName], &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 diff --git a/x/evidence/exported/evidence.go b/x/evidence/exported/evidence.go index 1c6adfdf8c0a..79ce2bec737f 100644 --- a/x/evidence/exported/evidence.go +++ b/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 diff --git a/x/ibc/02-client/alias.go b/x/ibc/02-client/alias.go index 00519d9da512..4713a4e77dee 100644 --- a/x/ibc/02-client/alias.go +++ b/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 ( diff --git a/x/ibc/02-client/client/cli/tx.go b/x/ibc/02-client/client/cli/tx.go index 197f7271b011..9e666f8f926b 100644 --- a/x/ibc/02-client/client/cli/tx.go +++ b/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...") diff --git a/x/ibc/02-client/client/rest/rest.go b/x/ibc/02-client/client/rest/rest.go index fb832f63a69a..51f8ce762bcb 100644 --- a/x/ibc/02-client/client/rest/rest.go +++ b/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"` } diff --git a/x/ibc/02-client/exported/exported.go b/x/ibc/02-client/exported/exported.go index e611b94301fa..9a5f2bc2e89f 100644 --- a/x/ibc/02-client/exported/exported.go +++ b/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 } diff --git a/x/ibc/02-client/handler.go b/x/ibc/02-client/handler.go index 1b1e1807b54a..595656a86c20 100644 --- a/x/ibc/02-client/handler.go +++ b/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) + } + } } diff --git a/x/ibc/02-client/keeper/client.go b/x/ibc/02-client/keeper/client.go index 3ee7a53143bf..9b72ef4a2c2c 100644 --- a/x/ibc/02-client/keeper/client.go +++ b/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,6 +69,7 @@ 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 @@ -73,23 +77,47 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, header exported.H // 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 } diff --git a/x/ibc/02-client/keeper/client_test.go b/x/ibc/02-client/keeper/client_test.go index cfc69e2f2245..c99757d3eacb 100644 --- a/x/ibc/02-client/keeper/client_test.go +++ b/x/ibc/02-client/keeper/client_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "bytes" "fmt" "github.com/stretchr/testify/require" @@ -19,7 +20,7 @@ const ( func (suite *KeeperTestSuite) TestCreateClient() { // Test Valid CreateClient state, err := suite.keeper.CreateClient(suite.ctx, testClientID, exported.Tendermint, suite.consensusState) - require.Nil(suite.T(), err, "CreateClient failed") + suite.NoError(err, "CreateClient failed") // Test ClientState stored correctly expectedState := types.State{ @@ -36,7 +37,7 @@ func (suite *KeeperTestSuite) TestCreateClient() { // Test that trying to CreateClient on existing client fails _, err = suite.keeper.CreateClient(suite.ctx, testClientID, exported.Tendermint, suite.consensusState) - require.NotNil(suite.T(), err, "CreateClient on existing client: %s passed", testClientID) + suite.Error(err, "CreateClient on existing client: %s passed", testClientID) } func (suite *KeeperTestSuite) TestUpdateClient() { @@ -61,10 +62,10 @@ func (suite *KeeperTestSuite) TestUpdateClient() { suite.keeper.SetClientState(suite.ctx, clientState) }, true}, {"past height", func() { - suite.header = tendermint.MakeHeader(2, suite.valSet, suite.valSet, []tmtypes.PrivValidator{suite.privVal}) + suite.header = tendermint.MakeHeader("gaia", 2, suite.valSet, suite.valSet, []tmtypes.PrivValidator{suite.privVal}) }, true}, {"validatorHash incorrect", func() { - suite.header = tendermint.MakeHeader(4, altValSet, suite.valSet, altSigners) + suite.header = tendermint.MakeHeader("gaia", 4, altValSet, suite.valSet, altSigners) }, true}, {"nextHash incorrect", func() { suite.header.NextValidatorSet = altValSet @@ -79,28 +80,30 @@ func (suite *KeeperTestSuite) TestUpdateClient() { } for _, tc := range cases { + tc := tc // pin for scopelint suite.Run(fmt.Sprintf("Case %s", tc.name), func() { // Reset suite on each subtest suite.SetupTest() _, err := suite.keeper.CreateClient(suite.ctx, testClientID, exported.Tendermint, suite.consensusState) - require.Nil(suite.T(), err, "CreateClient failed") + suite.NoError(err, "CreateClient failed") tc.malleate() err = suite.keeper.UpdateClient(suite.ctx, testClientID, suite.header) retrievedConsState, _ := suite.keeper.GetConsensusState(suite.ctx, testClientID) tmConsState, _ := retrievedConsState.(tendermint.ConsensusState) + tmConsState.ValidatorSet.TotalVotingPower() tmConsState.NextValidatorSet.TotalVotingPower() retrievedRoot, _ := suite.keeper.GetVerifiedRoot(suite.ctx, testClientID, suite.consensusState.GetHeight()+1) if tc.expErr { - require.NotNil(suite.T(), err, "Invalid UpdateClient passed", tc.name) + suite.Error(err, "Invalid UpdateClient passed", tc.name) // require no state changes occurred require.Equal(suite.T(), suite.consensusState, tmConsState, "Consensus state changed after invalid UpdateClient") require.Nil(suite.T(), retrievedRoot, "Root added for new height after invalid UpdateClient") } else { - require.Nil(suite.T(), err, "Valid UpdateClient failed", tc.name) + suite.NoError(err, "Valid UpdateClient failed", tc.name) // require state changes were performed correctly require.Equal(suite.T(), suite.header.GetHeight(), retrievedConsState.GetHeight(), "height not updated correctly") @@ -112,3 +115,97 @@ func (suite *KeeperTestSuite) TestUpdateClient() { }) } } + +func (suite *KeeperTestSuite) TestCheckMisbehaviourAndUpdateState() { + altPrivVal := tmtypes.NewMockPV() + altVal := tmtypes.NewValidator(altPrivVal.GetPubKey(), 4) + + // Create bothValSet with both suite validator and altVal + bothValSet := tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal)) + // Create alternative validator set with only altVal + altValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) + + // Create signer array and ensure it is in same order as bothValSet + var bothSigners []tmtypes.PrivValidator + if bytes.Compare(altPrivVal.GetPubKey().Address(), suite.privVal.GetPubKey().Address()) == -1 { + bothSigners = []tmtypes.PrivValidator{altPrivVal, suite.privVal} + } else { + bothSigners = []tmtypes.PrivValidator{suite.privVal, altPrivVal} + } + + altSigners := []tmtypes.PrivValidator{altPrivVal} + + _, err := suite.keeper.CreateClient(suite.ctx, "gaiamainnet", exported.Tendermint, suite.consensusState) + suite.NoError(err, "CreateClient failed") + + testCases := []struct { + name string + evidence *tendermint.Evidence + clientID string + expErr bool + }{ + { + "trusting period misbehavior should pass", + &tendermint.Evidence{ + Header1: tendermint.MakeHeader("gaia", 5, bothValSet, suite.valSet, bothSigners), + Header2: tendermint.MakeHeader("gaia", 5, bothValSet, bothValSet, bothSigners), + ChainID: "gaia", + }, + "gaiamainnet", + false, + }, + { + "first valset has too much change", + &tendermint.Evidence{ + Header1: tendermint.MakeHeader("gaia", 5, altValSet, bothValSet, altSigners), + Header2: tendermint.MakeHeader("gaia", 5, bothValSet, bothValSet, bothSigners), + ChainID: "gaia", + }, + "gaiamainnet", + true, + }, + { + "second valset has too much change", + &tendermint.Evidence{ + Header1: tendermint.MakeHeader("gaia", 5, bothValSet, bothValSet, bothSigners), + Header2: tendermint.MakeHeader("gaia", 5, altValSet, bothValSet, altSigners), + ChainID: "gaia", + }, + "gaiamainnet", + true, + }, + { + "both valsets have too much change", + &tendermint.Evidence{ + Header1: tendermint.MakeHeader("gaia", 5, altValSet, altValSet, altSigners), + Header2: tendermint.MakeHeader("gaia", 5, altValSet, bothValSet, altSigners), + ChainID: "gaia", + }, + "gaiamainnet", + true, + }, + } + + for _, tc := range testCases { + tc := tc // pin for scopelint + suite.Run(tc.name, func() { + misbehaviour := tendermint.Misbehaviour{ + Evidence: tc.evidence, + ClientID: tc.clientID, + } + + err = suite.keeper.CheckMisbehaviourAndUpdateState(suite.ctx, misbehaviour) + + if tc.expErr { + suite.Error(err, "CheckMisbehaviour passed unexpectedly") + } else { + suite.NoError(err, "CheckMisbehaviour failed unexpectedly: %v", err) + } + + // reset Frozen flag to false + clientState, _ := suite.keeper.GetClientState(suite.ctx, tc.clientID) + clientState.Frozen = false + suite.keeper.SetClientState(suite.ctx, clientState) + }) + } +} diff --git a/x/ibc/02-client/keeper/keeper.go b/x/ibc/02-client/keeper/keeper.go index 3911c1f98f0e..e5c4feb26263 100644 --- a/x/ibc/02-client/keeper/keeper.go +++ b/x/ibc/02-client/keeper/keeper.go @@ -12,7 +12,6 @@ import ( "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" commitment "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment" ibctypes "github.com/cosmos/cosmos-sdk/x/ibc/types" ) @@ -119,6 +118,32 @@ func (k Keeper) SetVerifiedRoot(ctx sdk.Context, clientID string, height uint64, store.Set(types.KeyRoot(clientID, height), bz) } +// GetCommitter will get the Committer of a particular client at the oldest height +// that is less than or equal to the height passed in +func (k Keeper) GetCommitter(ctx sdk.Context, clientID string, height uint64) (exported.Committer, bool) { + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixClient) + + var committer exported.Committer + + // TODO: Replace this for-loop with a ReverseIterator for efficiency + for i := height; i > 0; i-- { + bz := store.Get(types.KeyCommitter(clientID, i)) + if bz != nil { + k.cdc.MustUnmarshalBinaryLengthPrefixed(bz, &committer) + return committer, true + } + } + return nil, false +} + +// SetCommitter sets a committer from a particular height to +// a particular client +func (k Keeper) SetCommitter(ctx sdk.Context, clientID string, height uint64, committer exported.Committer) { + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixClient) + bz := k.cdc.MustMarshalBinaryLengthPrefixed(committer) + store.Set(types.KeyCommitter(clientID, height), bz) +} + // State returns a new client state with a given id as defined in // https://github.com/cosmos/ics/tree/master/spec/ics-002-client-semantics#example-implementation func (k Keeper) initialize(ctx sdk.Context, clientID string, consensusState exported.ConsensusState) types.State { @@ -127,24 +152,6 @@ func (k Keeper) initialize(ctx sdk.Context, clientID string, consensusState expo return clientState } -func (k Keeper) checkMisbehaviour(ctx sdk.Context, evidence exported.Evidence) error { - switch evidence.Type() { - case exported.ClientTypeTendermint: - var tmEvidence tendermint.Evidence - _, ok := evidence.(tendermint.Evidence) - if !ok { - return errors.ErrInvalidClientType(k.codespace, "consensus type is not Tendermint") - } - err := tendermint.CheckMisbehaviour(tmEvidence) - if err != nil { - return errors.ErrInvalidEvidence(k.codespace, err.Error()) - } - default: - panic(fmt.Sprintf("unregistered evidence type: %s", evidence.Type())) - } - return nil -} - // freeze updates the state of the client in the event of a misbehaviour func (k Keeper) freeze(ctx sdk.Context, clientState types.State) (types.State, error) { if clientState.Frozen { @@ -164,12 +171,14 @@ func (k Keeper) VerifyMembership( path commitment.PathI, value []byte, ) bool { - // XXX: commented out for demo - /* - if clientState.Frozen { - return false - } - */ + clientState, found := k.GetClientState(ctx, clientID) + if !found { + return false + } + + if clientState.Frozen { + return false + } root, found := k.GetVerifiedRoot(ctx, clientID, height) if !found { @@ -187,12 +196,15 @@ func (k Keeper) VerifyNonMembership( proof commitment.ProofI, path commitment.PathI, ) bool { - // XXX: commented out for demo - /* - if clientState.Frozen { - return false - } - */ + clientState, found := k.GetClientState(ctx, clientID) + if !found { + return false + } + + if clientState.Frozen { + return false + } + root, found := k.GetVerifiedRoot(ctx, clientID, height) if !found { return false diff --git a/x/ibc/02-client/keeper/keeper_test.go b/x/ibc/02-client/keeper/keeper_test.go index da81be0edb12..da79777dae50 100644 --- a/x/ibc/02-client/keeper/keeper_test.go +++ b/x/ibc/02-client/keeper/keeper_test.go @@ -4,6 +4,7 @@ import ( "testing" abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/crypto/tmhash" tmtypes "github.com/tendermint/tendermint/types" "github.com/cosmos/cosmos-sdk/codec" @@ -48,12 +49,13 @@ func (suite *KeeperTestSuite) SetupTest() { validator := tmtypes.NewValidator(suite.privVal.GetPubKey(), 1) suite.valSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{validator}) - suite.header = tendermint.MakeHeader(4, suite.valSet, suite.valSet, []tmtypes.PrivValidator{suite.privVal}) + suite.header = tendermint.MakeHeader("gaia", 4, suite.valSet, suite.valSet, []tmtypes.PrivValidator{suite.privVal}) suite.consensusState = tendermint.ConsensusState{ ChainID: testClientID, Height: 3, Root: commitment.NewRoot([]byte("hash")), + ValidatorSet: suite.valSet, NextValidatorSet: suite.valSet, } } @@ -87,6 +89,7 @@ func (suite *KeeperTestSuite) TestSetConsensusState() { require.True(suite.T(), ok, "GetConsensusState failed") tmConsState, _ := retrievedConsState.(tendermint.ConsensusState) // force recalculation of unexported totalVotingPower so we can compare consensusState + tmConsState.ValidatorSet.TotalVotingPower() tmConsState.NextValidatorSet.TotalVotingPower() require.Equal(suite.T(), suite.consensusState, tmConsState, "ConsensusState not stored correctly") } @@ -100,3 +103,49 @@ func (suite *KeeperTestSuite) TestSetVerifiedRoot() { require.True(suite.T(), ok, "GetVerifiedRoot failed") require.Equal(suite.T(), root, retrievedRoot, "Root stored incorrectly") } + +func (suite KeeperTestSuite) TestSetCommitter() { + committer := tendermint.Committer{ + ValidatorSet: suite.valSet, + Height: 3, + NextValSetHash: suite.valSet.Hash(), + } + nextCommitter := tendermint.Committer{ + ValidatorSet: suite.valSet, + Height: 6, + NextValSetHash: tmhash.Sum([]byte("next_hash")), + } + + suite.keeper.SetCommitter(suite.ctx, "gaia", 3, committer) + suite.keeper.SetCommitter(suite.ctx, "gaia", 6, nextCommitter) + + // fetch the commiter on each respective height + for i := 0; i < 3; i++ { + committer, ok := suite.keeper.GetCommitter(suite.ctx, "gaia", uint64(i)) + require.False(suite.T(), ok, "GetCommitter passed on nonexistent height: %d", i) + require.Nil(suite.T(), committer, "GetCommitter returned committer on nonexistent height: %d", i) + } + + for i := 3; i < 6; i++ { + recv, ok := suite.keeper.GetCommitter(suite.ctx, "gaia", uint64(i)) + tmRecv, _ := recv.(tendermint.Committer) + if tmRecv.ValidatorSet != nil { + // update validator set's power + tmRecv.ValidatorSet.TotalVotingPower() + } + require.True(suite.T(), ok, "GetCommitter failed on existing height: %d", i) + require.Equal(suite.T(), committer, recv, "GetCommitter returned committer on nonexistent height: %d", i) + } + + for i := 6; i < 9; i++ { + recv, ok := suite.keeper.GetCommitter(suite.ctx, "gaia", uint64(i)) + tmRecv, _ := recv.(tendermint.Committer) + if tmRecv.ValidatorSet != nil { + // update validator set's power + tmRecv.ValidatorSet.TotalVotingPower() + } + require.True(suite.T(), ok, "GetCommitter failed on existing height: %d", i) + require.Equal(suite.T(), nextCommitter, recv, "GetCommitter returned committer on nonexistent height: %d", i) + } + +} diff --git a/x/ibc/02-client/types/codec.go b/x/ibc/02-client/types/codec.go index d595f583baa9..ed923f5cbc89 100644 --- a/x/ibc/02-client/types/codec.go +++ b/x/ibc/02-client/types/codec.go @@ -12,7 +12,7 @@ var SubModuleCdc *codec.Codec // RegisterCodec registers the IBC client interfaces and types func RegisterCodec(cdc *codec.Codec) { cdc.RegisterInterface((*exported.ConsensusState)(nil), nil) - cdc.RegisterInterface((*exported.Evidence)(nil), nil) + cdc.RegisterInterface((*exported.Committer)(nil), nil) cdc.RegisterInterface((*exported.Header)(nil), nil) cdc.RegisterInterface((*exported.Misbehaviour)(nil), nil) @@ -20,6 +20,7 @@ func RegisterCodec(cdc *codec.Codec) { cdc.RegisterConcrete(MsgUpdateClient{}, "ibc/client/MsgUpdateClient", nil) cdc.RegisterConcrete(tendermint.ConsensusState{}, "ibc/client/tendermint/ConsensusState", nil) + cdc.RegisterConcrete(tendermint.Committer{}, "ibc/client/tendermint/Committer", nil) cdc.RegisterConcrete(tendermint.Header{}, "ibc/client/tendermint/Header", nil) cdc.RegisterConcrete(tendermint.Evidence{}, "ibc/client/tendermint/Evidence", nil) diff --git a/x/ibc/02-client/types/errors/errors.go b/x/ibc/02-client/types/errors/errors.go index 1e9510df6c44..1a7272ca1fcb 100644 --- a/x/ibc/02-client/types/errors/errors.go +++ b/x/ibc/02-client/types/errors/errors.go @@ -19,8 +19,10 @@ const ( CodeClientTypeNotFound sdk.CodeType = 205 CodeInvalidClientType sdk.CodeType = 206 CodeRootNotFound sdk.CodeType = 207 - CodeInvalidHeader sdk.CodeType = 207 + CodeInvalidHeader sdk.CodeType = 208 CodeInvalidEvidence sdk.CodeType = 209 + CodeCommitterNotFound sdk.CodeType = 210 + CodeInvalidCommitter sdk.CodeType = 211 ) // ErrClientExists implements sdk.Error @@ -112,3 +114,21 @@ func ErrInvalidEvidence(codespace sdk.CodespaceType, msg string) error { fmt.Sprintf("invalid evidence: %s", msg), ) } + +// ErrCommitterNotFound implements sdk.Error +func ErrCommitterNotFound(codespace sdk.CodespaceType, msg string) error { + return sdkerrors.New( + string(codespace), + uint32(CodeCommitterNotFound), + fmt.Sprintf("invalid evidence: %s", msg), + ) +} + +// ErrInvalidEvidence implements sdk.Error +func ErrInvalidCommitter(codespace sdk.CodespaceType, msg string) error { + return sdkerrors.New( + string(codespace), + uint32(CodeInvalidCommitter), + fmt.Sprintf("invalid evidence: %s", msg), + ) +} diff --git a/x/ibc/02-client/types/events.go b/x/ibc/02-client/types/events.go index 7e7f9a508cee..6b02bfa5eaf8 100644 --- a/x/ibc/02-client/types/events.go +++ b/x/ibc/02-client/types/events.go @@ -13,9 +13,9 @@ const ( // IBC client events vars var ( - EventTypeCreateClient = MsgCreateClient{}.Type() - EventTypeUpdateClient = MsgUpdateClient{}.Type() - EventTypeSubmitMisbehaviour = MsgSubmitMisbehaviour{}.Type() + EventTypeCreateClient = TypeMsgCreateClient + EventTypeUpdateClient = TypeMsgUpdateClient + EventTypeSubmitMisbehaviour = TypeClientMisbehaviour AttributeValueCategory = fmt.Sprintf("%s_%s", ibctypes.ModuleName, SubModuleName) ) diff --git a/x/ibc/02-client/types/keys.go b/x/ibc/02-client/types/keys.go index acebca3ee09a..6dc55a77baf0 100644 --- a/x/ibc/02-client/types/keys.go +++ b/x/ibc/02-client/types/keys.go @@ -6,16 +6,16 @@ import ( const ( // SubModuleName defines the IBC client name - SubModuleName = "client" + SubModuleName string = "client" // StoreKey is the store key string for IBC client - StoreKey = SubModuleName + StoreKey string = SubModuleName // RouterKey is the message route for IBC client - RouterKey = SubModuleName + RouterKey string = SubModuleName // QuerierRoute is the querier route for IBC client - QuerierRoute = SubModuleName + QuerierRoute string = SubModuleName ) // KVStore key prefixes @@ -44,11 +44,17 @@ func ConsensusStatePath(clientID string) string { } // RootPath takes an Identifier and returns a Path under which to -// store the consensus state of a client. +// store the root for a particular height of a client. func RootPath(clientID string, height uint64) string { return fmt.Sprintf("clients/%s/roots/%d", clientID, height) } +// CommitterPath takes an Identifier and returns a Path under which +// to store the committer of a client at a particular height +func CommitterPath(clientID string, height uint64) string { + return fmt.Sprintf("clients/%s/committer/%d", clientID, height) +} + // KeyClientState returns the store key for a particular client state func KeyClientState(clientID string) []byte { return []byte(ClientStatePath(clientID)) @@ -70,3 +76,9 @@ func KeyConsensusState(clientID string) []byte { func KeyRoot(clientID string, height uint64) []byte { return []byte(RootPath(clientID, height)) } + +// KeyValidatorSet returns the store key for a validator of a particular +// client at a given height. Key => clientID||height +func KeyCommitter(clientID string, height uint64) []byte { + return []byte(CommitterPath(clientID, height)) +} diff --git a/x/ibc/02-client/types/msgs.go b/x/ibc/02-client/types/msgs.go index f09e126666ff..6f0aa7959ac7 100644 --- a/x/ibc/02-client/types/msgs.go +++ b/x/ibc/02-client/types/msgs.go @@ -2,12 +2,20 @@ package types import ( sdk "github.com/cosmos/cosmos-sdk/types" + 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/errors" host "github.com/cosmos/cosmos-sdk/x/ibc/24-host" ibctypes "github.com/cosmos/cosmos-sdk/x/ibc/types" ) +// Message types for the IBC client +const ( + TypeMsgCreateClient string = "create_client" + TypeMsgUpdateClient string = "update_client" + TypeClientMisbehaviour string = "client_misbehaviour" +) + var _ sdk.Msg = MsgCreateClient{} // MsgCreateClient defines a message to create an IBC client @@ -35,7 +43,7 @@ func (msg MsgCreateClient) Route() string { // Type implements sdk.Msg func (msg MsgCreateClient) Type() string { - return "create_client" + return TypeMsgCreateClient } // ValidateBasic implements sdk.Msg @@ -90,7 +98,7 @@ func (msg MsgUpdateClient) Route() string { // Type implements sdk.Msg func (msg MsgUpdateClient) Type() string { - return "update_client" + return TypeMsgUpdateClient } // ValidateBasic implements sdk.Msg @@ -120,12 +128,12 @@ func (msg MsgUpdateClient) GetSigners() []sdk.AccAddress { // MsgSubmitMisbehaviour defines a message to update an IBC client type MsgSubmitMisbehaviour struct { ClientID string `json:"id" yaml:"id"` - Evidence exported.Evidence `json:"evidence" yaml:"evidence"` + Evidence evidenceexported.Evidence `json:"evidence" yaml:"evidence"` Signer sdk.AccAddress `json:"address" yaml:"address"` } // NewMsgSubmitMisbehaviour creates a new MsgSubmitMisbehaviour instance -func NewMsgSubmitMisbehaviour(id string, evidence exported.Evidence, signer sdk.AccAddress) MsgSubmitMisbehaviour { +func NewMsgSubmitMisbehaviour(id string, evidence evidenceexported.Evidence, signer sdk.AccAddress) MsgSubmitMisbehaviour { return MsgSubmitMisbehaviour{ ClientID: id, Evidence: evidence, diff --git a/x/ibc/02-client/types/msgs_test.go b/x/ibc/02-client/types/msgs_test.go index f7e022ecc89c..b29633420ed4 100644 --- a/x/ibc/02-client/types/msgs_test.go +++ b/x/ibc/02-client/types/msgs_test.go @@ -9,11 +9,38 @@ import ( cmn "github.com/tendermint/tendermint/libs/common" sdk "github.com/cosmos/cosmos-sdk/types" + 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/errors" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/tendermint" ) +var _ evidenceexported.Evidence = mockEvidence{} +var _ evidenceexported.Evidence = mockBadEvidence{} + +const mockStr = "mock" + +// mock GoodEvidence +type mockEvidence struct{} + +// Implement Evidence interface +func (me mockEvidence) Route() string { return mockStr } +func (me mockEvidence) Type() string { return mockStr } +func (me mockEvidence) String() string { return mockStr } +func (me mockEvidence) Hash() cmn.HexBytes { return cmn.HexBytes([]byte(mockStr)) } +func (me mockEvidence) ValidateBasic() error { return nil } +func (me mockEvidence) GetHeight() int64 { return 3 } + +// mock bad evidence +type mockBadEvidence struct { + mockEvidence +} + +// Override ValidateBasic +func (mbe mockBadEvidence) ValidateBasic() error { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "invalid evidence") +} + func TestMsgCreateClientValidateBasic(t *testing.T) { cs := tendermint.ConsensusState{} privKey := secp256k1.GenPrivKey() @@ -79,34 +106,6 @@ func TestMsgUpdateClient(t *testing.T) { } } -var _ exported.Evidence = mockEvidence{} - -const mockStr = "mock" - -// mock GoodEvidence -type mockEvidence struct{} - -// Implement Evidence interface -func (me mockEvidence) Route() string { return mockStr } -func (me mockEvidence) Type() string { return mockStr } -func (me mockEvidence) String() string { return mockStr } -func (me mockEvidence) Hash() cmn.HexBytes { return cmn.HexBytes([]byte(mockStr)) } -func (me mockEvidence) ValidateBasic() sdk.Error { return nil } -func (me mockEvidence) GetConsensusAddress() sdk.ConsAddress { return sdk.ConsAddress{} } -func (me mockEvidence) GetHeight() int64 { return 3 } -func (me mockEvidence) GetValidatorPower() int64 { return 3 } -func (me mockEvidence) GetTotalPower() int64 { return 5 } - -// mock bad evidence -type mockBadEvidence struct { - mockEvidence -} - -// Override ValidateBasic -func (mbe mockBadEvidence) ValidateBasic() sdk.Error { - return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, "invalid evidence")) -} - func TestMsgSubmitMisbehaviour(t *testing.T) { privKey := secp256k1.GenPrivKey() signer := sdk.AccAddress(privKey.PubKey().Address()) diff --git a/x/ibc/02-client/types/tendermint/codec.go b/x/ibc/02-client/types/tendermint/codec.go index 46814fec2479..a546ed313cef 100644 --- a/x/ibc/02-client/types/tendermint/codec.go +++ b/x/ibc/02-client/types/tendermint/codec.go @@ -8,8 +8,11 @@ var SubModuleCdc = codec.New() // RegisterCodec registers the Tendermint types func RegisterCodec(cdc *codec.Codec) { + codec.RegisterCrypto(cdc) + cdc.RegisterConcrete(Committer{}, "ibc/client/tendermint/Committer", nil) cdc.RegisterConcrete(ConsensusState{}, "ibc/client/tendermint/ConsensusState", nil) cdc.RegisterConcrete(Header{}, "ibc/client/tendermint/Header", nil) + cdc.RegisterConcrete(Misbehaviour{}, "ibc/client/tendermint/Misbehaviour", nil) cdc.RegisterConcrete(Evidence{}, "ibc/client/tendermint/Evidence", nil) } diff --git a/x/ibc/02-client/types/tendermint/committer.go b/x/ibc/02-client/types/tendermint/committer.go new file mode 100644 index 000000000000..ccf798feb570 --- /dev/null +++ b/x/ibc/02-client/types/tendermint/committer.go @@ -0,0 +1,26 @@ +package tendermint + +import ( + tmtypes "github.com/tendermint/tendermint/types" + + "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" +) + +var _ exported.Committer = Committer{} + +// Committer definites a Tendermint Committer +type Committer struct { + *tmtypes.ValidatorSet `json:"validator_set" yaml:"validator_set"` + Height uint64 `json:"height" yaml:"height"` + NextValSetHash []byte `json:"next_valset_hash" yaml:"next_valset_hash"` +} + +// Implement exported.Committer interface +func (c Committer) ClientType() exported.ClientType { + return exported.Tendermint +} + +// Implement exported.Committer interface +func (c Committer) GetHeight() uint64 { + return c.Height +} diff --git a/x/ibc/02-client/types/tendermint/consensus_state.go b/x/ibc/02-client/types/tendermint/consensus_state.go index f57887045a2f..25c2f8d990f8 100644 --- a/x/ibc/02-client/types/tendermint/consensus_state.go +++ b/x/ibc/02-client/types/tendermint/consensus_state.go @@ -20,6 +20,7 @@ type ConsensusState struct { ChainID string `json:"chain_id" yaml:"chain_id"` Height uint64 `json:"height" yaml:"height"` // NOTE: defined as 'sequence' in the spec Root commitment.RootI `json:"root" yaml:"root"` + ValidatorSet *tmtypes.ValidatorSet `json:"validator_set" yaml:"validator_set"` NextValidatorSet *tmtypes.ValidatorSet `json:"next_validator_set" yaml:"next_validator_set"` // contains the PublicKey } @@ -38,6 +39,15 @@ func (cs ConsensusState) GetRoot() commitment.RootI { return cs.Root } +// GetCommitter returns the commmitter that committed the ConsensusState +func (cs ConsensusState) GetCommitter() exported.Committer { + return Committer{ + ValidatorSet: cs.ValidatorSet, + Height: cs.Height, + NextValSetHash: cs.NextValidatorSet.Hash(), + } +} + // CheckValidityAndUpdateState checks if the provided header is valid and updates // the consensus state if appropriate func (cs ConsensusState) CheckValidityAndUpdateState(header exported.Header) (exported.ConsensusState, error) { @@ -64,6 +74,11 @@ func (cs ConsensusState) checkValidity(header Header) error { ) } + // basic consistency check + if err := header.ValidateBasic(cs.ChainID); err != nil { + return err + } + // check if the hash from the consensus set and header // matches nextHash := cs.NextValidatorSet.Hash() @@ -78,11 +93,6 @@ func (cs ConsensusState) checkValidity(header Header) error { return lerr.ErrUnexpectedValidators(header.NextValidatorsHash, nextHash) } - // basic consistency check - if err := header.ValidateBasic(cs.ChainID); err != nil { - return err - } - // abortTransactionUnless(consensusState.publicKey.verify(header.signature)) return header.ValidatorSet.VerifyFutureCommit( cs.NextValidatorSet, cs.ChainID, header.Commit.BlockID, header.Height, header.Commit, @@ -93,6 +103,7 @@ func (cs ConsensusState) checkValidity(header Header) error { func (cs ConsensusState) update(header Header) ConsensusState { cs.Height = header.GetHeight() cs.Root = commitment.NewRoot(header.AppHash) + cs.ValidatorSet = header.ValidatorSet cs.NextValidatorSet = header.NextValidatorSet return cs } diff --git a/x/ibc/02-client/types/tendermint/consensus_state_test.go b/x/ibc/02-client/types/tendermint/consensus_state_test.go index 07c1015a3067..778de45fa3a8 100644 --- a/x/ibc/02-client/types/tendermint/consensus_state_test.go +++ b/x/ibc/02-client/types/tendermint/consensus_state_test.go @@ -9,12 +9,12 @@ import ( func (suite *TendermintTestSuite) TestCheckValidity() { // valid header err := suite.cs.checkValidity(suite.header) - require.Nil(suite.T(), err, "validity failed") + suite.NoError(err, "validity failed") // switch out header ValidatorsHash suite.header.ValidatorsHash = tmhash.Sum([]byte("hello")) err = suite.cs.checkValidity(suite.header) - require.NotNil(suite.T(), err, "validator hash is wrong") + suite.Error(err, "validator hash is wrong") // reset suite and make header.NextValidatorSet different // from NextValidatorSetHash @@ -23,13 +23,13 @@ func (suite *TendermintTestSuite) TestCheckValidity() { val := tmtypes.NewValidator(privVal.GetPubKey(), 5) suite.header.NextValidatorSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{val}) err = suite.cs.checkValidity(suite.header) - require.NotNil(suite.T(), err, "header's next validator set is not consistent with hash") + suite.Error(err, "header's next validator set is not consistent with hash") // reset and make header fail validatebasic suite.SetupTest() suite.header.ChainID = "not_gaia" err = suite.cs.checkValidity(suite.header) - require.NotNil(suite.T(), err, "invalid header should fail ValidateBasic") + suite.Error(err, "invalid header should fail ValidateBasic") } func (suite *TendermintTestSuite) TestCheckUpdate() { @@ -40,13 +40,14 @@ func (suite *TendermintTestSuite) TestCheckUpdate() { require.Equal(suite.T(), suite.header.GetHeight(), cs.GetHeight(), "height not updated") require.Equal(suite.T(), suite.header.AppHash.Bytes(), cs.GetRoot().GetHash(), "root not updated") tmCS, _ := cs.(ConsensusState) - require.Equal(suite.T(), suite.header.NextValidatorSet, tmCS.NextValidatorSet, "validator set did not update") + require.Equal(suite.T(), suite.header.ValidatorSet, tmCS.ValidatorSet, "validator set did not update") + require.Equal(suite.T(), suite.header.NextValidatorSet, tmCS.NextValidatorSet, "next validator set did not update") // make header invalid so update should be unsuccessful suite.SetupTest() suite.header.ChainID = "not_gaia" cs, err = suite.cs.CheckValidityAndUpdateState(suite.header) - require.NotNil(suite.T(), err) + suite.Error(err) require.Nil(suite.T(), cs) } diff --git a/x/ibc/02-client/types/tendermint/doc.go b/x/ibc/02-client/types/tendermint/doc.go index f0e27c7696a7..26aa430a82da 100644 --- a/x/ibc/02-client/types/tendermint/doc.go +++ b/x/ibc/02-client/types/tendermint/doc.go @@ -1,5 +1,5 @@ /* -Package tendermint implements a concrete `ConsensusState`, `Header` and `Equivocation` -for the Tendermint consensus algorithm. +Package tendermint implements a concrete `ConsensusState`, `Header`, +`Misbehaviour` and `Equivocation` types for the Tendermint consensus light client. */ package tendermint diff --git a/x/ibc/02-client/types/tendermint/evidence.go b/x/ibc/02-client/types/tendermint/evidence.go new file mode 100644 index 000000000000..a8d1154a27ae --- /dev/null +++ b/x/ibc/02-client/types/tendermint/evidence.go @@ -0,0 +1,106 @@ +package tendermint + +import ( + "fmt" + + yaml "gopkg.in/yaml.v2" + + "github.com/tendermint/tendermint/crypto/tmhash" + cmn "github.com/tendermint/tendermint/libs/common" + tmtypes "github.com/tendermint/tendermint/types" + + evidenceexported "github.com/cosmos/cosmos-sdk/x/evidence/exported" + "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" +) + +var _ evidenceexported.Evidence = Evidence{} + +// Evidence is a wrapper over tendermint's DuplicateVoteEvidence +// that implements Evidence interface expected by ICS-02 +type Evidence struct { + Header1 Header `json:"header1" yaml:"header1"` + Header2 Header `json:"header2" yaml:"header2"` + ChainID string `json:"chain_id" yaml:"chain_id"` +} + +// Route implements Evidence interface +func (ev Evidence) Route() string { + return "client" +} + +// Type implements Evidence interface +func (ev Evidence) Type() string { + return "client_misbehaviour" +} + +// String implements Evidence interface +func (ev Evidence) String() string { + bz, err := yaml.Marshal(ev) + if err != nil { + panic(err) + } + return string(bz) +} + +// Hash implements Evidence interface +func (ev Evidence) Hash() cmn.HexBytes { + return tmhash.Sum(SubModuleCdc.MustMarshalBinaryBare(ev)) +} + +// GetHeight returns the height at which misbehaviour occurred +func (ev Evidence) GetHeight() int64 { + return ev.Header1.Height +} + +// ValidateBasic implements Evidence interface +func (ev Evidence) ValidateBasic() error { + // ValidateBasic on both validators + if err := ev.Header1.ValidateBasic(ev.ChainID); err != nil { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("Header1 failed ValidateBasic: %v", err)) + } + if err := ev.Header2.ValidateBasic(ev.ChainID); err != nil { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("Header2 failed ValidateBasic: %v", err)) + } + // Ensure that Heights are the same + if ev.Header1.Height != ev.Header2.Height { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "headers in evidence are on different heights") + } + // Ensure that Commit Hashes are different + if ev.Header1.Commit.BlockID.Equals(ev.Header2.Commit.BlockID) { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "Headers commit to same blockID") + } + + if err1 := ValidCommit(ev.ChainID, ev.Header1.Commit, ev.Header1.ValidatorSet); err1 != nil { + return err1 + } + if err2 := ValidCommit(ev.ChainID, ev.Header2.Commit, ev.Header2.ValidatorSet); err2 != nil { + return err2 + } + + return nil +} + +// ValidCommit checks if the given commit is a valid commit from the passed-in validatorset +// +// CommitToVoteSet will panic if the commit cannot be converted to a valid voteset given the validatorset +// This implies that someone tried to submit evidence that wasn't actually committed by the validatorset +// thus we should return an error here and reject the evidence rather than panicing. +func ValidCommit(chainID string, commit *tmtypes.Commit, valSet *tmtypes.ValidatorSet) (err error) { + defer func() { + if r := recover(); r != nil { + err = errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("invalid commit: %v", r)) + } + }() + + // Convert commits to vote-sets given the validator set so we can check if they both have 2/3 power + voteSet := tmtypes.CommitToVoteSet(chainID, commit, valSet) + + blockID, ok := voteSet.TwoThirdsMajority() + + // Check that ValidatorSet did indeed commit to blockID in Commit + if !ok || !blockID.Equals(commit.BlockID) { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "ValidatorSet did not commit to Header1") + } + + return nil +} diff --git a/x/ibc/02-client/types/tendermint/evidence_test.go b/x/ibc/02-client/types/tendermint/evidence_test.go new file mode 100644 index 000000000000..09f40499b458 --- /dev/null +++ b/x/ibc/02-client/types/tendermint/evidence_test.go @@ -0,0 +1,135 @@ +package tendermint + +import ( + "bytes" + + "github.com/tendermint/tendermint/crypto/tmhash" + tmtypes "github.com/tendermint/tendermint/types" +) + +func (suite *TendermintTestSuite) TestEvidenceValidateBasic() { + altPrivVal := tmtypes.NewMockPV() + altVal := tmtypes.NewValidator(altPrivVal.GetPubKey(), 4) + + // Create bothValSet with both suite validator and altVal + bothValSet := tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal)) + // Create alternative validator set with only altVal + altValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) + + signers := []tmtypes.PrivValidator{suite.privVal} + // Create signer array and ensure it is in same order as bothValSet + var bothSigners []tmtypes.PrivValidator + if bytes.Compare(altPrivVal.GetPubKey().Address(), suite.privVal.GetPubKey().Address()) == -1 { + bothSigners = []tmtypes.PrivValidator{altPrivVal, suite.privVal} + } else { + bothSigners = []tmtypes.PrivValidator{suite.privVal, altPrivVal} + } + + altSigners := []tmtypes.PrivValidator{altPrivVal} + + testCases := []struct { + name string + evidence Evidence + malleateEvidence func(ev *Evidence) + expErr bool + }{ + { + "valid evidence", + Evidence{ + Header1: suite.header, + Header2: MakeHeader("gaia", 4, suite.valSet, bothValSet, signers), + ChainID: "gaia", + }, + func(ev *Evidence) {}, + false, + }, + { + "wrong chainID on header1", + Evidence{ + Header1: suite.header, + Header2: MakeHeader("ethermint", 4, suite.valSet, bothValSet, signers), + ChainID: "ethermint", + }, + func(ev *Evidence) {}, + true, + }, + { + "wrong chainID on header2", + Evidence{ + Header1: suite.header, + Header2: MakeHeader("ethermint", 4, suite.valSet, bothValSet, signers), + ChainID: "gaia", + }, + func(ev *Evidence) {}, + true, + }, + { + "mismatched heights", + Evidence{ + Header1: suite.header, + Header2: MakeHeader("gaia", 6, suite.valSet, bothValSet, signers), + ChainID: "gaia", + }, + func(ev *Evidence) {}, + true, + }, + { + "same block id", + Evidence{ + Header1: suite.header, + Header2: suite.header, + ChainID: "gaia", + }, + func(ev *Evidence) {}, + true, + }, + { + "header doesn't have 2/3 majority", + Evidence{ + Header1: suite.header, + Header2: MakeHeader("gaia", 4, bothValSet, bothValSet, bothSigners), + ChainID: "gaia", + }, + func(ev *Evidence) { + // voteSet contains only altVal which is less than 2/3 of total power (4/14) + wrongVoteSet := tmtypes.NewVoteSet("gaia", ev.Header2.Height, 1, tmtypes.PrecommitType, altValSet) + var err error + ev.Header2.Commit, err = tmtypes.MakeCommit(ev.Header2.Commit.BlockID, ev.Header2.Height, ev.Header2.Commit.Round(), wrongVoteSet, altSigners) + if err != nil { + panic(err) + } + }, + true, + }, + { + "validators sign off on wrong commit", + Evidence{ + Header1: suite.header, + Header2: MakeHeader("gaia", 4, bothValSet, bothValSet, bothSigners), + ChainID: "gaia", + }, + func(ev *Evidence) { + ev.Header2.Commit.BlockID = makeBlockID(tmhash.Sum([]byte("other_hash")), 3, tmhash.Sum([]byte("other_partset"))) + }, + true, + }, + } + + for _, tc := range testCases { + tc := tc // pin for scopelint + suite.Run(tc.name, func() { + // reset suite for each subtest + suite.SetupTest() + + tc.malleateEvidence(&tc.evidence) + + err := tc.evidence.ValidateBasic() + + if tc.expErr { + suite.Error(err, "ValidateBasic did not throw error for invalid evidence") + } else { + suite.NoError(err, "ValidateBasic returned error on valid evidence: %s", err) + } + }) + } +} diff --git a/x/ibc/02-client/types/tendermint/header.go b/x/ibc/02-client/types/tendermint/header.go index c2d9cfebc9c5..133c1741e56e 100644 --- a/x/ibc/02-client/types/tendermint/header.go +++ b/x/ibc/02-client/types/tendermint/header.go @@ -3,7 +3,9 @@ package tendermint import ( tmtypes "github.com/tendermint/tendermint/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported" + clienterrors "github.com/cosmos/cosmos-sdk/x/ibc/02-client/types/errors" ) var _ exported.Header = Header{} @@ -20,9 +22,33 @@ func (h Header) ClientType() exported.ClientType { return exported.Tendermint } +// GetCommitter returns the ValidatorSet that committed header +func (h Header) GetCommitter() exported.Committer { + return Committer{ + ValidatorSet: h.ValidatorSet, + Height: uint64(h.Height), + NextValSetHash: h.NextValidatorsHash, + } +} + // GetHeight returns the current height // // NOTE: also referred as `sequence` func (h Header) GetHeight() uint64 { return uint64(h.Height) } + +// ValidateBasic calls the SignedHeader ValidateBasic function +// and checks that validatorsets are not nil +func (h Header) ValidateBasic(chainID string) error { + if err := h.SignedHeader.ValidateBasic(chainID); err != nil { + return err + } + if h.ValidatorSet == nil { + return sdkerrors.Wrap(clienterrors.ErrInvalidHeader(clienterrors.DefaultCodespace), "ValidatorSet is nil") + } + if h.NextValidatorSet == nil { + return sdkerrors.Wrap(clienterrors.ErrInvalidHeader(clienterrors.DefaultCodespace), "NextValidatorSet is nil") + } + return nil +} diff --git a/x/ibc/02-client/types/tendermint/misbehaviour.go b/x/ibc/02-client/types/tendermint/misbehaviour.go index 1edde8ec1c04..5b04b6e3ec0c 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour.go @@ -3,94 +3,65 @@ package tendermint import ( "fmt" - yaml "gopkg.in/yaml.v2" - - sdk "github.com/cosmos/cosmos-sdk/types" + 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/errors" - - "github.com/tendermint/tendermint/crypto/tmhash" - cmn "github.com/tendermint/tendermint/libs/common" - tmtypes "github.com/tendermint/tendermint/types" + host "github.com/cosmos/cosmos-sdk/x/ibc/24-host" ) -var _ exported.Evidence = Evidence{} +var _ exported.Misbehaviour = Misbehaviour{} +var _ evidenceexported.Evidence = Misbehaviour{} -// Evidence is a wrapper over tendermint's DuplicateVoteEvidence -// that implements Evidence interface expected by ICS-02 -type Evidence struct { - *tmtypes.DuplicateVoteEvidence - ChainID string `json:"chain_id" yaml:"chain_id"` - ValidatorPower int64 `json:"val_power" yaml:"val_power"` - TotalPower int64 `json:"total_power" yaml:"total_power"` +// Misbehaviour contains evidence that a light client submitted a different header from +// a full node at the same height. +type Misbehaviour struct { + *Evidence + ClientID string `json:"client_id" yaml:"client_id"` } -// Type implements exported.Evidence interface -func (ev Evidence) Route() string { - return exported.ClientTypeTendermint +// ClientType is Tendermint light client +func (m Misbehaviour) ClientType() exported.ClientType { + return exported.Tendermint } -// Type implements exported.Evidence interface -func (ev Evidence) Type() string { - return exported.ClientTypeTendermint +// GetEvidence returns the evidence to handle a light client misbehaviour +func (m Misbehaviour) GetEvidence() evidenceexported.Evidence { + return m.Evidence } -// String implements exported.Evidence interface -func (ev Evidence) String() string { - bz, err := yaml.Marshal(ev) - if err != nil { - panic(err) +// ValidateBasic performs the basic validity checks for the evidence and the +// client ID. +func (m Misbehaviour) ValidateBasic() error { + if m.Evidence == nil { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, "empty evidence") } - return string(bz) -} - -// Hash implements exported.Evidence interface -func (ev Evidence) Hash() cmn.HexBytes { - return tmhash.Sum(SubModuleCdc.MustMarshalBinaryBare(ev)) -} -// ValidateBasic implements exported.Evidence interface -func (ev Evidence) ValidateBasic() sdk.Error { - if ev.DuplicateVoteEvidence == nil { - return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, "duplicate evidence is nil")) - } - err := ev.DuplicateVoteEvidence.ValidateBasic() - if err != nil { - return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, err.Error())) - } - if ev.ChainID == "" { - return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, "chainID is empty")) - } - if ev.ValidatorPower <= 0 { - return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("Invalid Validator Power: %d", ev.ValidatorPower))) - } - if ev.TotalPower < ev.ValidatorPower { - return sdk.ConvertError(errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("Invalid Total Power: %d", ev.TotalPower))) + if err := m.Evidence.ValidateBasic(); err != nil { + return err } - return nil -} -// GetConsensusAddress implements exported.Evidence interface -func (ev Evidence) GetConsensusAddress() sdk.ConsAddress { - return sdk.ConsAddress(ev.DuplicateVoteEvidence.Address()) + return host.DefaultClientIdentifierValidator(m.ClientID) } -// GetHeight implements exported.Evidence interface -func (ev Evidence) GetHeight() int64 { - return ev.DuplicateVoteEvidence.Height() -} +// CheckMisbehaviour checks if the evidence provided is a valid light client misbehaviour +func CheckMisbehaviour(trustedCommitter Committer, m Misbehaviour) error { + if err := m.ValidateBasic(); err != nil { + return err + } -// GetValidatorPower implements exported.Evidence interface -func (ev Evidence) GetValidatorPower() int64 { - return ev.ValidatorPower -} + trustedValSet := trustedCommitter.ValidatorSet -// GetTotalPower implements exported.Evidence interface -func (ev Evidence) GetTotalPower() int64 { - return ev.TotalPower -} + // Evidence is within trusting period. ValidatorSet must have 2/3 similarity with trustedCommitter ValidatorSet + // check that the validator sets on both headers are valid given the last trusted validatorset + // less than or equal to evidence height + if err := trustedValSet.VerifyFutureCommit(m.Evidence.Header1.ValidatorSet, m.Evidence.ChainID, + m.Evidence.Header1.Commit.BlockID, m.Evidence.Header1.Height, m.Evidence.Header1.Commit); err != nil { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("validator set in Header1 has too much change from last known committer: %v", err)) + } + if err := trustedValSet.VerifyFutureCommit(m.Evidence.Header2.ValidatorSet, m.Evidence.ChainID, + m.Evidence.Header2.Commit.BlockID, m.Evidence.Header2.Height, m.Evidence.Header2.Commit); err != nil { + return errors.ErrInvalidEvidence(errors.DefaultCodespace, fmt.Sprintf("validator set in Header2 has too much change from last known committer: %v", err)) + } -// CheckMisbehaviour checks if the evidence provided is a misbehaviour -func CheckMisbehaviour(evidence Evidence) error { - return evidence.DuplicateVoteEvidence.Verify(evidence.ChainID, evidence.DuplicateVoteEvidence.PubKey) + return nil } diff --git a/x/ibc/02-client/types/tendermint/misbehaviour_test.go b/x/ibc/02-client/types/tendermint/misbehaviour_test.go index f5e75e12f785..f69a6d12884e 100644 --- a/x/ibc/02-client/types/tendermint/misbehaviour_test.go +++ b/x/ibc/02-client/types/tendermint/misbehaviour_test.go @@ -1,64 +1,170 @@ package tendermint import ( - "testing" + "bytes" - "github.com/stretchr/testify/require" - - yaml "gopkg.in/yaml.v2" + tmtypes "github.com/tendermint/tendermint/types" ) -func TestString(t *testing.T) { - dupEv := randomDuplicatedVoteEvidence() - ev := Evidence{ - DuplicateVoteEvidence: dupEv, - ChainID: "mychain", - ValidatorPower: 10, - TotalPower: 50, +func (suite *TendermintTestSuite) TestMisbehaviourValidateBasic() { + altPrivVal := tmtypes.NewMockPV() + altVal := tmtypes.NewValidator(altPrivVal.GetPubKey(), 4) + + // Create bothValSet with both suite validator and altVal + bothValSet := tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal)) + + signers := []tmtypes.PrivValidator{suite.privVal} + testCases := []struct { + name string + evidence *Evidence + clientID string + expErr bool + }{ + { + "valid misbehavior", + &Evidence{ + Header1: suite.header, + Header2: MakeHeader("gaia", 4, suite.valSet, bothValSet, signers), + ChainID: "gaia", + }, + "gaiamainnet", + false, + }, + { + "nil evidence", + nil, + "gaiamainnet", + true, + }, + { + "invalid evidence", + &Evidence{ + Header1: suite.header, + Header2: suite.header, + ChainID: "gaia", + }, + "gaiamainnet", + true, + }, + { + "invalid ClientID", + &Evidence{ + Header1: MakeHeader("gaia123??", 5, suite.valSet, suite.valSet, signers), + Header2: MakeHeader("gaia123?", 5, suite.valSet, suite.valSet, signers), + ChainID: "gaia123?", + }, + "gaia123?", + true, + }, } - byteStr, err := yaml.Marshal(ev) - require.Nil(t, err) - require.Equal(t, string(byteStr), ev.String(), "Evidence String method does not work as expected") + for _, tc := range testCases { + tc := tc // pin for scopelint + suite.Run(tc.name, func() { + misbehaviour := Misbehaviour{ + Evidence: tc.evidence, + ClientID: tc.clientID, + } + + err := misbehaviour.ValidateBasic() + if tc.expErr { + suite.Error(err, "Invalid Misbehaviour passed ValidateBasic") + } else { + suite.NoError(err, "Valid Misbehaviour failed ValidateBasic: %v", err) + } + }) + } } -func TestValidateBasic(t *testing.T) { - dupEv := randomDuplicatedVoteEvidence() +func (suite *TendermintTestSuite) TestCheckMisbehaviour() { + altPrivVal := tmtypes.NewMockPV() + altVal := tmtypes.NewValidator(altPrivVal.GetPubKey(), 4) + + // Create bothValSet with both suite validator and altVal + bothValSet := tmtypes.NewValidatorSet(append(suite.valSet.Validators, altVal)) + // Create alternative validator set with only altVal + altValSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{altVal}) - // good evidence - ev := Evidence{ - DuplicateVoteEvidence: dupEv, - ChainID: "mychain", - ValidatorPower: 10, - TotalPower: 50, + // Create signer array and ensure it is in same order as bothValSet + var bothSigners []tmtypes.PrivValidator + if bytes.Compare(altPrivVal.GetPubKey().Address(), suite.privVal.GetPubKey().Address()) == -1 { + bothSigners = []tmtypes.PrivValidator{altPrivVal, suite.privVal} + } else { + bothSigners = []tmtypes.PrivValidator{suite.privVal, altPrivVal} } - err := ev.ValidateBasic() - require.Nil(t, err, "good evidence failed on ValidateBasic: %v", err) - - // invalid duplicate evidence - ev.DuplicateVoteEvidence.VoteA = nil - err = ev.ValidateBasic() - require.NotNil(t, err, "invalid duplicate evidence passed on ValidateBasic") - - // reset duplicate evidence to be valid, and set empty chainID - dupEv = randomDuplicatedVoteEvidence() - ev.DuplicateVoteEvidence = dupEv - ev.ChainID = "" - err = ev.ValidateBasic() - require.NotNil(t, err, "invalid chain-id passed on ValidateBasic") - - // reset chainID and set 0 validator power - ev.ChainID = "mychain" - ev.ValidatorPower = 0 - err = ev.ValidateBasic() - require.NotNil(t, err, "invalid validator power passed on ValidateBasic") - - // reset validator power and set invalid total power - ev.ValidatorPower = 10 - ev.TotalPower = 9 - err = ev.ValidateBasic() - require.NotNil(t, err, "invalid total power passed on ValidateBasic") + altSigners := []tmtypes.PrivValidator{altPrivVal} + committer := Committer{ + ValidatorSet: suite.valSet, + Height: 3, + NextValSetHash: suite.valSet.Hash(), + } + + testCases := []struct { + name string + evidence *Evidence + clientID string + expErr bool + }{ + { + "trusting period misbehavior should pass", + &Evidence{ + Header1: MakeHeader("gaia", 5, bothValSet, suite.valSet, bothSigners), + Header2: MakeHeader("gaia", 5, bothValSet, bothValSet, bothSigners), + ChainID: "gaia", + }, + "gaiamainnet", + false, + }, + { + "first valset has too much change", + &Evidence{ + Header1: MakeHeader("gaia", 5, altValSet, bothValSet, altSigners), + Header2: MakeHeader("gaia", 5, bothValSet, bothValSet, bothSigners), + ChainID: "gaia", + }, + "gaiamainnet", + true, + }, + { + "second valset has too much change", + &Evidence{ + Header1: MakeHeader("gaia", 5, bothValSet, bothValSet, bothSigners), + Header2: MakeHeader("gaia", 5, altValSet, bothValSet, altSigners), + ChainID: "gaia", + }, + "gaiamainnet", + true, + }, + { + "both valsets have too much change", + &Evidence{ + Header1: MakeHeader("gaia", 5, altValSet, altValSet, altSigners), + Header2: MakeHeader("gaia", 5, altValSet, bothValSet, altSigners), + ChainID: "gaia", + }, + "gaiamainnet", + true, + }, + } + + for _, tc := range testCases { + tc := tc // pin for scopelint + suite.Run(tc.name, func() { + misbehaviour := Misbehaviour{ + Evidence: tc.evidence, + ClientID: tc.clientID, + } + + err := CheckMisbehaviour(committer, misbehaviour) + + if tc.expErr { + suite.Error(err, "CheckMisbehaviour passed unexpectedly") + } else { + suite.NoError(err, "CheckMisbehaviour failed unexpectedly: %v", err) + } + }) + } } diff --git a/x/ibc/02-client/types/tendermint/tendermint_test.go b/x/ibc/02-client/types/tendermint/tendermint_test.go index cb3e572d6289..7f8b8e0e863c 100644 --- a/x/ibc/02-client/types/tendermint/tendermint_test.go +++ b/x/ibc/02-client/types/tendermint/tendermint_test.go @@ -24,13 +24,14 @@ func (suite *TendermintTestSuite) SetupTest() { privVal := tmtypes.NewMockPV() val := tmtypes.NewValidator(privVal.GetPubKey(), 10) valSet := tmtypes.NewValidatorSet([]*tmtypes.Validator{val}) - suite.header = MakeHeader(3, valSet, valSet, []tmtypes.PrivValidator{privVal}) + suite.header = MakeHeader("gaia", 4, valSet, valSet, []tmtypes.PrivValidator{privVal}) root := commitment.NewRoot(tmhash.Sum([]byte("my root"))) cs := ConsensusState{ ChainID: "gaia", Height: 3, Root: root, + ValidatorSet: valSet, NextValidatorSet: valSet, } diff --git a/x/ibc/02-client/types/tendermint/test_utils.go b/x/ibc/02-client/types/tendermint/test_utils.go index ac25a13fabbf..bc2b998c2e71 100644 --- a/x/ibc/02-client/types/tendermint/test_utils.go +++ b/x/ibc/02-client/types/tendermint/test_utils.go @@ -21,42 +21,13 @@ func makeBlockID(hash []byte, partSetSize int, partSetHash []byte) tmtypes.Block } -func makeVote(val tmtypes.PrivValidator, chainID string, valIndex int, height int64, round, step int, blockID tmtypes.BlockID) *tmtypes.Vote { - addr := val.GetPubKey().Address() - v := &tmtypes.Vote{ - ValidatorAddress: addr, - ValidatorIndex: valIndex, - Height: height, - Round: round, - Type: tmtypes.SignedMsgType(step), - BlockID: blockID, - } - err := val.SignVote(chainID, v) - if err != nil { - panic(err) - } - return v -} - -func randomDuplicatedVoteEvidence() *tmtypes.DuplicateVoteEvidence { - val := tmtypes.NewMockPV() - blockID := makeBlockID(tmhash.Sum([]byte("blockhash")), 1000, tmhash.Sum([]byte("partshash"))) - blockID2 := makeBlockID(tmhash.Sum([]byte("blockhash2")), 1000, tmhash.Sum([]byte("partshash"))) - const chainID = "gaia" - return &tmtypes.DuplicateVoteEvidence{ - PubKey: val.GetPubKey(), - VoteA: makeVote(val, chainID, 0, 10, 2, 1, blockID), - VoteB: makeVote(val, chainID, 0, 10, 2, 1, blockID2), - } -} - -func MakeHeader(height int64, valSet *tmtypes.ValidatorSet, nextValSet *tmtypes.ValidatorSet, signers []tmtypes.PrivValidator) Header { +func MakeHeader(chainID string, height int64, valSet *tmtypes.ValidatorSet, nextValSet *tmtypes.ValidatorSet, signers []tmtypes.PrivValidator) Header { vsetHash := valSet.Hash() nextHash := nextValSet.Hash() timestamp := time.Date(math.MaxInt64, 0, 0, 0, 0, 0, math.MaxInt64, time.UTC) tmHeader := tmtypes.Header{ Version: version.Consensus{Block: 2, App: 2}, - ChainID: "gaia", + ChainID: chainID, Height: height, Time: timestamp, NumTxs: 100, @@ -70,11 +41,11 @@ func MakeHeader(height int64, valSet *tmtypes.ValidatorSet, nextValSet *tmtypes. AppHash: tmhash.Sum([]byte("app_hash")), LastResultsHash: tmhash.Sum([]byte("last_results_hash")), EvidenceHash: tmhash.Sum([]byte("evidence_hash")), - ProposerAddress: signers[0].GetPubKey().Address(), + ProposerAddress: valSet.Proposer.Address, } hhash := tmHeader.Hash() blockID := makeBlockID(hhash, 3, tmhash.Sum([]byte("part_set"))) - voteSet := tmtypes.NewVoteSet("gaia", height, 1, tmtypes.PrecommitType, valSet) + voteSet := tmtypes.NewVoteSet(chainID, height, 1, tmtypes.PrecommitType, valSet) commit, err := tmtypes.MakeCommit(blockID, height, 1, voteSet, signers) if err != nil { panic(err) diff --git a/x/ibc/03-connection/keeper/handshake_test.go b/x/ibc/03-connection/keeper/handshake_test.go index fede95f47fe9..4e39fbd8de23 100644 --- a/x/ibc/03-connection/keeper/handshake_test.go +++ b/x/ibc/03-connection/keeper/handshake_test.go @@ -267,9 +267,11 @@ func (suite *KeeperTestSuite) createClient(clientID string) { suite.ctx = suite.app.BaseApp.NewContext(false, abci.Header{}) consensusState := tendermint.ConsensusState{ - ChainID: chainID, - Height: uint64(commitID.Version), - Root: commitment.NewRoot(commitID.Hash), + ChainID: chainID, + Height: uint64(commitID.Version), + Root: commitment.NewRoot(commitID.Hash), + ValidatorSet: suite.valSet, + NextValidatorSet: suite.valSet, } _, err := suite.app.IBCKeeper.ClientKeeper.CreateClient(suite.ctx, clientID, clientType, consensusState) diff --git a/x/ibc/03-connection/keeper/keeper_test.go b/x/ibc/03-connection/keeper/keeper_test.go index bf6dfe0d7839..904f02ca16d8 100644 --- a/x/ibc/03-connection/keeper/keeper_test.go +++ b/x/ibc/03-connection/keeper/keeper_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/suite" abci "github.com/tendermint/tendermint/abci/types" + tmtypes "github.com/tendermint/tendermint/types" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/simapp" @@ -29,9 +30,10 @@ const ( type KeeperTestSuite struct { suite.Suite - cdc *codec.Codec - ctx sdk.Context - app *simapp.SimApp + cdc *codec.Codec + ctx sdk.Context + app *simapp.SimApp + valSet *tmtypes.ValidatorSet } func (suite *KeeperTestSuite) SetupTest() { @@ -41,6 +43,11 @@ func (suite *KeeperTestSuite) SetupTest() { suite.cdc = app.Codec() suite.ctx = app.BaseApp.NewContext(isCheckTx, abci.Header{}) suite.app = app + + privVal := tmtypes.NewMockPV() + + validator := tmtypes.NewValidator(privVal.GetPubKey(), 1) + suite.valSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{validator}) } func TestKeeperTestSuite(t *testing.T) { diff --git a/x/ibc/04-channel/keeper/handshake_test.go b/x/ibc/04-channel/keeper/handshake_test.go index efd704c28d21..98a39778e467 100644 --- a/x/ibc/04-channel/keeper/handshake_test.go +++ b/x/ibc/04-channel/keeper/handshake_test.go @@ -22,11 +22,15 @@ func (suite *KeeperTestSuite) createClient() { suite.ctx = suite.app.BaseApp.NewContext(false, abci.Header{}) consensusState := clienttypestm.ConsensusState{ - ChainID: testChainID, - Height: uint64(commitID.Version), - Root: commitment.NewRoot(commitID.Hash), + ChainID: testChainID, + Height: uint64(commitID.Version), + Root: commitment.NewRoot(commitID.Hash), + ValidatorSet: suite.valSet, + NextValidatorSet: suite.valSet, } + _ = consensusState.GetCommitter() + _, err := suite.app.IBCKeeper.ClientKeeper.CreateClient(suite.ctx, testClient, testClientType, consensusState) suite.NoError(err) } diff --git a/x/ibc/04-channel/keeper/keeper_test.go b/x/ibc/04-channel/keeper/keeper_test.go index 299b0ab9c5e5..fa011b3ae104 100644 --- a/x/ibc/04-channel/keeper/keeper_test.go +++ b/x/ibc/04-channel/keeper/keeper_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/suite" abci "github.com/tendermint/tendermint/abci/types" + tmtypes "github.com/tendermint/tendermint/types" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/simapp" @@ -32,9 +33,10 @@ const ( type KeeperTestSuite struct { suite.Suite - cdc *codec.Codec - ctx sdk.Context - app *simapp.SimApp + cdc *codec.Codec + ctx sdk.Context + app *simapp.SimApp + valSet *tmtypes.ValidatorSet } func (suite *KeeperTestSuite) SetupTest() { @@ -45,6 +47,11 @@ func (suite *KeeperTestSuite) SetupTest() { suite.ctx = app.BaseApp.NewContext(isCheckTx, abci.Header{}) suite.app = app + privVal := tmtypes.NewMockPV() + + validator := tmtypes.NewValidator(privVal.GetPubKey(), 1) + suite.valSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{validator}) + suite.createClient() } diff --git a/x/ibc/20-transfer/handler_test.go b/x/ibc/20-transfer/handler_test.go index 7e0f430ddaa9..2b07fbe5151a 100644 --- a/x/ibc/20-transfer/handler_test.go +++ b/x/ibc/20-transfer/handler_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/suite" abci "github.com/tendermint/tendermint/abci/types" + tmtypes "github.com/tendermint/tendermint/types" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/simapp" @@ -50,9 +51,10 @@ var ( type HandlerTestSuite struct { suite.Suite - cdc *codec.Codec - ctx sdk.Context - app *simapp.SimApp + cdc *codec.Codec + ctx sdk.Context + app *simapp.SimApp + valSet *tmtypes.ValidatorSet } func (suite *HandlerTestSuite) SetupTest() { @@ -63,6 +65,11 @@ func (suite *HandlerTestSuite) SetupTest() { suite.ctx = app.BaseApp.NewContext(isCheckTx, abci.Header{}) suite.app = app + privVal := tmtypes.NewMockPV() + + validator := tmtypes.NewValidator(privVal.GetPubKey(), 1) + suite.valSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{validator}) + suite.createClient() suite.createConnection(connection.OPEN) } @@ -75,9 +82,11 @@ func (suite *HandlerTestSuite) createClient() { suite.ctx = suite.app.BaseApp.NewContext(false, abci.Header{}) consensusState := clienttypestm.ConsensusState{ - ChainID: testChainID, - Height: uint64(commitID.Version), - Root: commitment.NewRoot(commitID.Hash), + ChainID: testChainID, + Height: uint64(commitID.Version), + Root: commitment.NewRoot(commitID.Hash), + ValidatorSet: suite.valSet, + NextValidatorSet: suite.valSet, } _, err := suite.app.IBCKeeper.ClientKeeper.CreateClient(suite.ctx, testClient, testClientType, consensusState) diff --git a/x/ibc/20-transfer/keeper/keeper_test.go b/x/ibc/20-transfer/keeper/keeper_test.go index b7370a34d29f..cc67265bb1f8 100644 --- a/x/ibc/20-transfer/keeper/keeper_test.go +++ b/x/ibc/20-transfer/keeper/keeper_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/suite" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/crypto" + tmtypes "github.com/tendermint/tendermint/types" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/simapp" @@ -46,9 +47,10 @@ var ( type KeeperTestSuite struct { suite.Suite - cdc *codec.Codec - ctx sdk.Context - app *simapp.SimApp + cdc *codec.Codec + ctx sdk.Context + app *simapp.SimApp + valSet *tmtypes.ValidatorSet } func (suite *KeeperTestSuite) SetupTest() { @@ -59,6 +61,11 @@ func (suite *KeeperTestSuite) SetupTest() { suite.ctx = app.BaseApp.NewContext(isCheckTx, abci.Header{}) suite.app = app + privVal := tmtypes.NewMockPV() + + validator := tmtypes.NewValidator(privVal.GetPubKey(), 1) + suite.valSet = tmtypes.NewValidatorSet([]*tmtypes.Validator{validator}) + suite.createClient() suite.createConnection(connection.OPEN) } diff --git a/x/ibc/20-transfer/keeper/relay_test.go b/x/ibc/20-transfer/keeper/relay_test.go index fe20d2b26062..c28b4cc9db00 100644 --- a/x/ibc/20-transfer/keeper/relay_test.go +++ b/x/ibc/20-transfer/keeper/relay_test.go @@ -23,9 +23,11 @@ func (suite *KeeperTestSuite) createClient() { suite.ctx = suite.app.BaseApp.NewContext(false, abci.Header{}) consensusState := clienttypestm.ConsensusState{ - ChainID: testChainID, - Height: uint64(commitID.Version), - Root: commitment.NewRoot(commitID.Hash), + ChainID: testChainID, + Height: uint64(commitID.Version), + Root: commitment.NewRoot(commitID.Hash), + ValidatorSet: suite.valSet, + NextValidatorSet: suite.valSet, } _, err := suite.app.IBCKeeper.ClientKeeper.CreateClient(suite.ctx, testClient, testClientType, consensusState) diff --git a/x/ibc/handler.go b/x/ibc/handler.go index 07fc9f831be8..1a1e2d0189be 100644 --- a/x/ibc/handler.go +++ b/x/ibc/handler.go @@ -23,9 +23,6 @@ func NewHandler(k Keeper) sdk.Handler { case client.MsgUpdateClient: return client.HandleMsgUpdateClient(ctx, k.ClientKeeper, msg) - case client.MsgSubmitMisbehaviour: - return client.HandleMsgSubmitMisbehaviour(ctx, k.ClientKeeper, msg) - // IBC connection msgs case connection.MsgConnectionOpenInit: return connection.HandleMsgConnectionOpenInit(ctx, k.ConnectionKeeper, msg)